Merge remote-tracking branch 'origin/main' into garrytan/lyon-v2

# Conflicts:
#	CHANGELOG.md
#	VERSION
#	package.json
#	test/gstack-next-version.test.ts

test/codex-hardening.test.ts

@@ -364,3 +364,66 @@ describe('gstack-codex-probe: telemetry event emission', () => {
     }
   });
 });
+
+// ── Step 2A argv guard ─────────────────────────────────────────────────────
+// Regression test for #1428: Codex CLI >=0.130.0 rejects passing a quoted
+// prompt argument together with `--base <branch>`. Step 2A must never combine
+// the two on the same line. Asserts across both the .tmpl source and the
+// generated SKILL.md so template drift can't silently re-introduce the bug.
+
+describe('codex SKILL.md.tmpl Step 2A: PROMPT + --base mutual exclusion guard', () => {
+  function extractStep2A(filePath: string): string {
+    const content = fs.readFileSync(filePath, 'utf-8');
+    const startIdx = content.indexOf('## Step 2A: Review Mode');
+    expect(startIdx).toBeGreaterThan(-1);
+    // End at next `## ` heading (skill section boundary).
+    const tail = content.slice(startIdx);
+    const nextHeading = tail.slice(2).search(/\n## /);
+    return nextHeading === -1 ? tail : tail.slice(0, nextHeading + 2);
+  }
+
+  for (const relPath of ['codex/SKILL.md.tmpl', 'codex/SKILL.md']) {
+    test(`${relPath}: no \`codex review\` line combines a quoted prompt argument with --base`, () => {
+      const section = extractStep2A(path.join(ROOT, relPath));
+      // Find all lines invoking `codex review` (any prefix wrapper allowed).
+      const lines = section.split('\n');
+      const offendingLines: string[] = [];
+      for (const line of lines) {
+        // Skip prose lines that just discuss codex review. Only inspect lines
+        // that look like an actual shell invocation (codex review followed by
+        // a non-prose token).
+        const match = line.match(/\bcodex\s+review\b(.*)$/);
+        if (!match) continue;
+        const rest = match[1];
+        // Two regression patterns:
+        //   codex review "..." --base <foo>
+        //   codex review $VAR --base <foo>
+        //   codex review -- "..." --base <foo>
+        // Acceptable: codex review --base <foo>   (bare, no prompt arg)
+        const hasBase = /--base\b/.test(rest);
+        if (!hasBase) continue;
+        // Strip --base <token> and any trailing -c/--enable flags so they
+        // don't look like positional args. Anything that remains BEFORE
+        // --base and looks like a positional is the regression.
+        const beforeBase = rest.split(/--base\b/)[0].trim();
+        // Empty (or just whitespace) before --base => bare review, safe.
+        if (beforeBase === '') continue;
+        // Allow `--` separator that introduces nothing else (rare). Anything
+        // that looks like a quoted string OR variable expansion is the bug.
+        if (/^["'$]|^--\s*["']/.test(beforeBase)) {
+          offendingLines.push(line);
+        }
+      }
+      expect(offendingLines).toEqual([]);
+    });
+
+    test(`${relPath}: Step 2A still contains at least one fix-path invocation`, () => {
+      const section = extractStep2A(path.join(ROOT, relPath));
+      // At least one of: bare `codex review --base` OR `codex exec ...` must
+      // remain. Guards against accidental deletion of both fix paths.
+      const bareReview = /codex\s+review\s+--base\b/.test(section);
+      const execRoute = /codex\s+exec\b/.test(section);
+      expect(bareReview || execRoute).toBe(true);
+    });
+  }
+});

test/gstack-artifacts-init.test.ts

@@ -11,12 +11,18 @@
  * auto-executes (no MCP probe). Per Finding #10: stored URL is HTTPS.
  */
 
-import { describe, test, expect, beforeEach, afterEach } from 'bun:test';
+import { describe, test as _test, expect, beforeEach, afterEach } from 'bun:test';
 import * as fs from 'fs';
 import * as os from 'os';
 import * as path from 'path';
 import { spawnSync } from 'child_process';
 
+// Integration tests spawn real git/gh/glab subprocesses. The default 5s
+// per-test timeout is tight on developer machines; raise to 30s to match
+// the brain-sync.test.ts pattern. The tests stay deterministic (fake bins,
+// no network), but subprocess fork+exec under bun adds non-trivial overhead.
+const test = (name: string, fn: any) => _test(name, fn, 30000);
+
 const ROOT = path.resolve(import.meta.dir, '..');
 const INIT_BIN = path.join(ROOT, 'bin', 'gstack-artifacts-init');
 

test/gstack-memory-helpers.test.ts

@@ -272,17 +272,36 @@ describe("withErrorContext", () => {
 
 describe("detectEngineTier", () => {
   let savedHome: string | undefined;
+  let savedGbrainHome: string | undefined;
+  let savedRealHome: string | undefined;
+  let savedPath: string | undefined;
   let testHome: string;
+  let testGbrainHome: string;
 
   beforeEach(() => {
     savedHome = process.env.GSTACK_HOME;
+    savedGbrainHome = process.env.GBRAIN_HOME;
+    savedRealHome = process.env.HOME;
+    savedPath = process.env.PATH;
     testHome = mkdtempSync(join(tmpdir(), "gstack-test-engine-"));
+    testGbrainHome = mkdtempSync(join(tmpdir(), "gstack-test-gbrain-"));
     process.env.GSTACK_HOME = testHome;
+    process.env.GBRAIN_HOME = testGbrainHome;
+    // Isolate HOME too — even though gbrainConfigPath() prefers GBRAIN_HOME
+    // when set, defense-in-depth against future code reading ~/.gbrain
+    // directly. See #1415 codex review finding #6.
+    process.env.HOME = testHome;
   });
 
   afterAll(() => {
     if (savedHome === undefined) delete process.env.GSTACK_HOME;
     else process.env.GSTACK_HOME = savedHome;
+    if (savedGbrainHome === undefined) delete process.env.GBRAIN_HOME;
+    else process.env.GBRAIN_HOME = savedGbrainHome;
+    if (savedRealHome === undefined) delete process.env.HOME;
+    else process.env.HOME = savedRealHome;
+    if (savedPath === undefined) delete process.env.PATH;
+    else process.env.PATH = savedPath;
   });
 
   it("returns a valid EngineDetect shape (engine, detected_at, schema_version)", () => {
@@ -307,4 +326,19 @@ describe("detectEngineTier", () => {
     const second = detectEngineTier();
     expect(second.detected_at).toBe(first.detected_at);
   });
+
+  it("falls back to GBRAIN_HOME/config.json when gbrain doctor omits engine (schema_version:2 case)", () => {
+    // Regression test for #1415: gbrain >=0.25 doctor output dropped the
+    // top-level `engine` field. The detect path must fall back to config.json.
+    // We force the doctor call to fail (PATH stripped of gbrain) and write a
+    // synthetic config to GBRAIN_HOME so the fallback path is deterministic.
+    process.env.PATH = "/nonexistent-no-gbrain-here";
+    writeFileSync(
+      join(testGbrainHome, "config.json"),
+      JSON.stringify({ engine: "postgres", database_url: "postgresql://test/example" }),
+      "utf-8"
+    );
+    const result = detectEngineTier();
+    expect(result.engine).toBe("supabase");
+  });
 });

test/gstack-next-version.test.ts

@@ -153,6 +153,9 @@ describe("markActiveSiblings", () => {
 // Integration smoke — only runs if gh is available and authenticated. Confirms
 // the CLI executes end-to-end against real APIs without crashing.
 describe("integration (smoke)", () => {
+  // Bumps timeout to 30s — the test spawns a real `bun run` subprocess that
+  // does a `gh pr list` against the live GitHub API to inspect claimed slots.
+  // Network latency makes 5s tight on developer machines.
   test("CLI runs against real repo and emits parseable JSON", async () => {
     const proc = Bun.spawnSync([
       "bun",
@@ -178,5 +181,5 @@ describe("integration (smoke)", () => {
     expect(Array.isArray(parsed.claimed)).toBe(true);
     expect(parsed).toHaveProperty("siblings");
     expect(parsed.siblings).toEqual([]); // --workspace-root null disabled scanning
-  }, 15_000); // Headroom over the 4-5s wall time of the spawned process under load
+  }, 30_000); // Headroom over the 4-5s wall time of the spawned process under load
 });

test/learnings.test.ts

@@ -102,6 +102,27 @@ describe('gstack-learnings-log', () => {
     const lines = fs.readFileSync(f!, 'utf-8').trim().split('\n');
     expect(lines.length).toBe(2);
   });
+
+  // Regression test for #1423: investigate skill emits type:"investigation"
+  // but ALLOWED_TYPES previously rejected it. Now accepted.
+  test('accepts type:"investigation" (regression: #1423)', () => {
+    const input = '{"skill":"investigate","type":"investigation","key":"root-cause","insight":"verified","confidence":9,"source":"observed"}';
+    const result = runLog(input);
+    expect(result.exitCode).toBe(0);
+    const f = findLearningsFile();
+    expect(f).not.toBeNull();
+    const parsed = JSON.parse(fs.readFileSync(f!, 'utf-8').trim());
+    expect(parsed.type).toBe('investigation');
+  });
+
+  // Caller contract: investigate/SKILL.md.tmpl must emit type:"investigation"
+  // verbatim. Guards against the template drifting to an invalid type and
+  // silently breaking the log path. See codex review finding for #1423.
+  test('investigate template emits type:"investigation" verbatim (caller contract)', () => {
+    const tmpl = fs.readFileSync(path.join(ROOT, 'investigate/SKILL.md.tmpl'), 'utf-8');
+    // The invocation line must include "type":"investigation" exactly.
+    expect(tmpl).toContain('"type":"investigation"');
+  });
 });
 
 describe('gstack-learnings-search', () => {