Merge remote-tracking branch 'origin/main' into garrytan/community-mode

test/gen-skill-docs.test.ts

@@ -3,6 +3,7 @@ import { COMMAND_DESCRIPTIONS } from '../browse/src/commands';
 import { SNAPSHOT_FLAGS } from '../browse/src/snapshot';
 import * as fs from 'fs';
 import * as path from 'path';
+import * as os from 'os';
 
 const ROOT = path.resolve(import.meta.dir, '..');
 const MAX_SKILL_DESCRIPTION_LENGTH = 1024;
@@ -262,6 +263,43 @@ describe('gen-skill-docs', () => {
     }
   });
 
+  test('bash blocks with shell globs are zsh-safe (setopt guard or find)', () => {
+    for (const skill of ALL_SKILLS) {
+      const content = fs.readFileSync(path.join(ROOT, skill.dir, 'SKILL.md'), 'utf-8');
+      const bashBlocks = [...content.matchAll(/```bash\n([\s\S]*?)```/g)].map(m => m[1]);
+
+      for (const block of bashBlocks) {
+        const lines = block.split('\n');
+
+        for (const line of lines) {
+          const trimmed = line.trimStart();
+          if (trimmed.startsWith('#')) continue;
+          if (!trimmed.includes('*')) continue;
+          // Skip lines where * is inside find -name, git pathspecs, or $(find)
+          if (/\bfind\b/.test(trimmed)) continue;
+          if (/\bgit\b/.test(trimmed)) continue;
+          if (/\$\(find\b/.test(trimmed)) continue;
+
+          // Check 1: "for VAR in <glob>" must use $(find ...) — caught above by the
+          // $(find check, so any surviving for-in with a glob pattern is a violation
+          if (/\bfor\s+\w+\s+in\b/.test(trimmed) && /\*\./.test(trimmed)) {
+            throw new Error(
+              `Unsafe for-in glob in ${skill.dir}/SKILL.md: "${trimmed}". ` +
+              `Use \`for f in $(find ... -name '*.ext')\` for zsh compatibility.`
+            );
+          }
+
+          // Check 2: ls/cat/rm/grep with glob file args must have setopt guard
+          const isGlobCmd = /\b(?:ls|cat|rm|grep)\b/.test(trimmed) &&
+                            /(?:\/\*[a-z.*]|\*\.[a-z])/.test(trimmed);
+          if (isGlobCmd) {
+            expect(block).toContain('setopt +o nomatch');
+          }
+        }
+      }
+    }
+  });
+
   test('preamble-using skills have correct skill name in telemetry', () => {
     const PREAMBLE_SKILLS = [
       { dir: '.', name: 'gstack' },
@@ -1599,6 +1637,29 @@ describe('setup script validation', () => {
   });
 });
 
+describe('discover-skills hidden directory filtering', () => {
+  test('discoverTemplates skips dot-prefixed directories', () => {
+    const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-discover-'));
+    try {
+      // Create a hidden dir with a template (should be excluded)
+      fs.mkdirSync(path.join(tmpDir, '.hidden'), { recursive: true });
+      fs.writeFileSync(path.join(tmpDir, '.hidden', 'SKILL.md.tmpl'), '---\nname: evil\n---\ntest');
+      // Create a visible dir with a template (should be included)
+      fs.mkdirSync(path.join(tmpDir, 'visible'), { recursive: true });
+      fs.writeFileSync(path.join(tmpDir, 'visible', 'SKILL.md.tmpl'), '---\nname: good\n---\ntest');
+
+      const { discoverTemplates } = require('../scripts/discover-skills');
+      const results = discoverTemplates(tmpDir);
+      const dirs = results.map((r: { tmpl: string }) => r.tmpl);
+
+      expect(dirs).toContain('visible/SKILL.md.tmpl');
+      expect(dirs).not.toContain('.hidden/SKILL.md.tmpl');
+    } finally {
+      fs.rmSync(tmpDir, { recursive: true, force: true });
+    }
+  });
+});
+
 describe('telemetry', () => {
   test('generated SKILL.md contains telemetry start block', () => {
     const content = fs.readFileSync(path.join(ROOT, 'SKILL.md'), 'utf-8');
@@ -1647,3 +1708,91 @@ describe('telemetry', () => {
     }
   });
 });
+
+describe('codex commands must not use inline $(git rev-parse --show-toplevel) for cwd', () => {
+  // Regression test: inline $(git rev-parse --show-toplevel) in codex exec -C
+  // or codex review without cd evaluates in whatever cwd the background shell
+  // inherits, which may be a different project in Conductor workspaces.
+  // The fix is to resolve _REPO_ROOT eagerly at the top of each bash block.
+
+  // Scan all source files that could contain codex commands
+  // Use Bun.Glob to avoid ELOOP from .claude/skills/gstack symlink back to ROOT
+  const tmplGlob = new Bun.Glob('**/*.tmpl');
+  const sourceFiles = [
+    ...Array.from(tmplGlob.scanSync({ cwd: ROOT, followSymlinks: false })),
+    ...fs.readdirSync(path.join(ROOT, 'scripts/resolvers'))
+      .filter(f => f.endsWith('.ts'))
+      .map(f => `scripts/resolvers/${f}`),
+    'scripts/gen-skill-docs.ts',
+  ];
+
+  test('no codex exec command uses inline $(git rev-parse --show-toplevel) in -C flag', () => {
+    const violations: string[] = [];
+    for (const rel of sourceFiles) {
+      const abs = path.join(ROOT, rel);
+      if (!fs.existsSync(abs)) continue;
+      const content = fs.readFileSync(abs, 'utf-8');
+      const lines = content.split('\n');
+      for (let i = 0; i < lines.length; i++) {
+        const line = lines[i];
+        if (line.includes('codex exec') && line.includes('-C') && line.includes('$(git rev-parse --show-toplevel)')) {
+          violations.push(`${rel}:${i + 1}`);
+        }
+      }
+    }
+    expect(violations).toEqual([]);
+  });
+
+  test('no generated SKILL.md has codex exec with inline $(git rev-parse --show-toplevel) in -C flag', () => {
+    const violations: string[] = [];
+    const skillMdGlob = new Bun.Glob('**/SKILL.md');
+    const skillMdFiles = Array.from(skillMdGlob.scanSync({ cwd: ROOT, followSymlinks: false }));
+    for (const rel of skillMdFiles) {
+      const abs = path.join(ROOT, rel);
+      if (!fs.existsSync(abs)) continue;
+      const content = fs.readFileSync(abs, 'utf-8');
+      const lines = content.split('\n');
+      for (let i = 0; i < lines.length; i++) {
+        const line = lines[i];
+        if (line.includes('codex exec') && line.includes('-C') && line.includes('$(git rev-parse --show-toplevel)')) {
+          violations.push(`${rel}:${i + 1}`);
+        }
+      }
+    }
+    expect(violations).toEqual([]);
+  });
+
+  test('codex review commands must be preceded by cd "$_REPO_ROOT" (no -C support)', () => {
+    // codex review does not support -C, so the pattern must be:
+    //   _REPO_ROOT=$(git rev-parse --show-toplevel) || { ... }
+    //   cd "$_REPO_ROOT"
+    //   codex review ...
+    // NOT: codex review ... with inline $(git rev-parse --show-toplevel)
+    const allFiles = [
+      ...Array.from(tmplGlob.scanSync({ cwd: ROOT, followSymlinks: false })),
+      ...Array.from(new Bun.Glob('**/SKILL.md').scanSync({ cwd: ROOT, followSymlinks: false })),
+      ...fs.readdirSync(path.join(ROOT, 'scripts/resolvers'))
+        .filter(f => f.endsWith('.ts'))
+        .map(f => `scripts/resolvers/${f}`),
+      'scripts/gen-skill-docs.ts',
+    ];
+    const violations: string[] = [];
+    for (const rel of allFiles) {
+      const abs = path.join(ROOT, rel);
+      if (!fs.existsSync(abs)) continue;
+      const content = fs.readFileSync(abs, 'utf-8');
+      const lines = content.split('\n');
+      for (let i = 0; i < lines.length; i++) {
+        const line = lines[i];
+        // Skip non-executable lines (markdown table cells, prose references)
+        if (line.includes('|') && line.includes('`/codex review`')) continue;
+        if (line.includes('`codex review`')) continue;
+        // Check for codex review with inline $(git rev-parse)
+        if (line.includes('codex review') && line.includes('$(git rev-parse --show-toplevel)')) {
+          violations.push(`${rel}:${i + 1} — inline git rev-parse in codex review`);
+        }
+      }
+    }
+    expect(violations).toEqual([]);
+  });
+});

test/helpers/touchfiles.ts

@@ -141,13 +141,16 @@ export const E2E_TOUCHFILES: Record<string, string[]> = {
   'benchmark-workflow':            ['benchmark/**', 'browse/src/**'],
   'setup-deploy-workflow':         ['setup-deploy/**', 'scripts/gen-skill-docs.ts'],
 
+  // Sidebar agent
+  'sidebar-navigate':              ['browse/src/server.ts', 'browse/src/sidebar-agent.ts', 'browse/src/sidebar-utils.ts', 'extension/**'],
+  'sidebar-url-accuracy':          ['browse/src/server.ts', 'browse/src/sidebar-agent.ts', 'browse/src/sidebar-utils.ts', 'extension/background.js'],
+
   // Autoplan
   'autoplan-core':  ['autoplan/**', 'plan-ceo-review/**', 'plan-eng-review/**', 'plan-design-review/**'],
 
   // Skill routing — journey-stage tests (depend on ALL skill descriptions)
   'journey-ideation':       ['*/SKILL.md.tmpl', 'SKILL.md.tmpl', 'scripts/gen-skill-docs.ts'],
   'journey-plan-eng':       ['*/SKILL.md.tmpl', 'SKILL.md.tmpl', 'scripts/gen-skill-docs.ts'],
-  'journey-think-bigger':   ['*/SKILL.md.tmpl', 'SKILL.md.tmpl', 'scripts/gen-skill-docs.ts'],
   'journey-debug':          ['*/SKILL.md.tmpl', 'SKILL.md.tmpl', 'scripts/gen-skill-docs.ts'],
   'journey-qa':             ['*/SKILL.md.tmpl', 'SKILL.md.tmpl', 'scripts/gen-skill-docs.ts'],
   'journey-code-review':    ['*/SKILL.md.tmpl', 'SKILL.md.tmpl', 'scripts/gen-skill-docs.ts'],
@@ -262,13 +265,16 @@ export const E2E_TIERS: Record<string, 'gate' | 'periodic'> = {
   'benchmark-workflow': 'gate',
   'setup-deploy-workflow': 'gate',
 
+  // Sidebar agent
+  'sidebar-navigate': 'periodic',
+  'sidebar-url-accuracy': 'periodic',
+
   // Autoplan — periodic (not yet implemented)
   'autoplan-core': 'periodic',
 
   // Skill routing — periodic (LLM routing is non-deterministic)
   'journey-ideation': 'periodic',
   'journey-plan-eng': 'periodic',
-  'journey-think-bigger': 'periodic',
   'journey-debug': 'periodic',
   'journey-qa': 'periodic',
   'journey-code-review': 'periodic',

test/review-log.test.ts

@@ -0,0 +1,77 @@
+import { describe, test, expect, beforeEach, afterEach } from 'bun:test';
+import { execSync, ExecSyncOptionsWithStringEncoding } from 'child_process';
+import * as fs from 'fs';
+import * as path from 'path';
+import * as os from 'os';
+
+const ROOT = path.resolve(import.meta.dir, '..');
+const BIN = path.join(ROOT, 'bin');
+
+let tmpDir: string;
+let slugDir: string;
+
+function run(input: string, opts: { expectFail?: boolean } = {}): { stdout: string; exitCode: number } {
+  const execOpts: ExecSyncOptionsWithStringEncoding = {
+    cwd: ROOT,
+    env: { ...process.env, GSTACK_HOME: tmpDir },
+    encoding: 'utf-8',
+    timeout: 10000,
+  };
+  try {
+    const stdout = execSync(`${BIN}/gstack-review-log '${input.replace(/'/g, "'\\''")}'`, execOpts).trim();
+    return { stdout, exitCode: 0 };
+  } catch (e: any) {
+    if (opts.expectFail) {
+      return { stdout: e.stderr?.toString() || '', exitCode: e.status || 1 };
+    }
+    throw e;
+  }
+}
+
+beforeEach(() => {
+  tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-revlog-'));
+  // gstack-review-log uses gstack-slug which needs a git repo — create the projects dir
+  // with a predictable slug by pre-creating the directory structure
+  slugDir = path.join(tmpDir, 'projects');
+  fs.mkdirSync(slugDir, { recursive: true });
+});
+
+afterEach(() => {
+  fs.rmSync(tmpDir, { recursive: true, force: true });
+});
+
+describe('gstack-review-log', () => {
+  test('appends valid JSON to review JSONL file', () => {
+    const input = '{"skill":"plan-eng-review","status":"clean"}';
+    const result = run(input);
+    expect(result.exitCode).toBe(0);
+
+    // Find the JSONL file that was written
+    const projectDirs = fs.readdirSync(slugDir);
+    expect(projectDirs.length).toBeGreaterThan(0);
+    const projectDir = path.join(slugDir, projectDirs[0]);
+    const jsonlFiles = fs.readdirSync(projectDir).filter(f => f.endsWith('.jsonl'));
+    expect(jsonlFiles.length).toBeGreaterThan(0);
+
+    const content = fs.readFileSync(path.join(projectDir, jsonlFiles[0]), 'utf-8').trim();
+    const parsed = JSON.parse(content);
+    expect(parsed.skill).toBe('plan-eng-review');
+    expect(parsed.status).toBe('clean');
+  });
+
+  test('rejects non-JSON input with non-zero exit code', () => {
+    const result = run('not json at all', { expectFail: true });
+    expect(result.exitCode).not.toBe(0);
+
+    // Verify nothing was written
+    const projectDirs = fs.readdirSync(slugDir);
+    if (projectDirs.length > 0) {
+      const projectDir = path.join(slugDir, projectDirs[0]);
+      const jsonlFiles = fs.readdirSync(projectDir).filter(f => f.endsWith('.jsonl'));
+      if (jsonlFiles.length > 0) {
+        const content = fs.readFileSync(path.join(projectDir, jsonlFiles[0]), 'utf-8').trim();
+        expect(content).toBe('');
+      }
+    }
+  });
+});

test/skill-e2e-bws.test.ts

@@ -45,7 +45,7 @@ describeIfSelected('Skill E2E tests', [
 4. $B screenshot /tmp/skill-e2e-test.png
 Report the results of each command.`,
       workingDirectory: tmpDir,
-      maxTurns: 5,
+      maxTurns: 7,
       timeout: 60_000,
       testName: 'browse-basic',
       runId,

test/skill-e2e-review.test.ts

@@ -340,21 +340,22 @@ Write your findings to ${dir}/review-output.md`,
     run('git', ['add', 'app.ts'], dir);
     run('git', ['commit', '-m', 'feat: update to v2'], dir);
 
-    // Copy ship skill
-    fs.copyFileSync(path.join(ROOT, 'ship', 'SKILL.md'), path.join(dir, 'ship-SKILL.md'));
+    // Extract only Step 0 (base branch detection) from ship/SKILL.md
+    // (copying the full 1900-line file causes agent context bloat and flaky timeouts)
+    const fullShipSkill = fs.readFileSync(path.join(ROOT, 'ship', 'SKILL.md'), 'utf-8');
+    const step0Start = fullShipSkill.indexOf('## Step 0: Detect platform and base branch');
+    const step0End = fullShipSkill.indexOf('## Step 1: Pre-flight');
+    const shipSection = fullShipSkill.slice(step0Start, step0End > step0Start ? step0End : undefined);
+    fs.writeFileSync(path.join(dir, 'ship-SKILL.md'), shipSection);
 
     const result = await runSkillTest({
-      prompt: `Read ship-SKILL.md for the ship workflow.
+      prompt: `Read ship-SKILL.md. It contains Step 0 (Detect base branch) from the ship workflow.
 
-Skip the preamble bash block, lake intro, telemetry, and contributor mode sections — go straight to Step 0.
+Run the base branch detection. Since there is no remote, gh commands will fail — fall back to main.
 
-Run ONLY Step 0 (Detect base branch) and Step 1 (Pre-flight) from the ship workflow.
-Since there is no remote, gh commands will fail — fall back to main.
+Then run git diff and git log against the detected base branch.
 
-After completing Step 0 and Step 1, STOP. Do NOT proceed to Step 2 or beyond.
-Do NOT push, create PRs, or modify VERSION/CHANGELOG.
-
-Write a summary of what you detected to ${dir}/ship-preflight.md including:
+Write a summary to ${dir}/ship-preflight.md including:
 - The detected base branch name
 - The current branch name
 - The diff stat against the base branch`,
@@ -580,8 +581,13 @@ describeIfSelected('Review Dashboard Via Attribution', ['review-dashboard-via'],
     ].join('\n'));
     fs.chmodSync(path.join(mockBinDir, 'gstack-review-read'), 0o755);
 
-    // Copy ship skill
-    fs.copyFileSync(path.join(ROOT, 'ship', 'SKILL.md'), path.join(dashDir, 'ship-SKILL.md'));
+    // Extract only the Review Readiness Dashboard section from ship/SKILL.md
+    // (copying the full 1900-line file causes agent context bloat and timeouts)
+    const fullSkill = fs.readFileSync(path.join(ROOT, 'ship', 'SKILL.md'), 'utf-8');
+    const dashStart = fullSkill.indexOf('## Review Readiness Dashboard');
+    const dashEnd = fullSkill.indexOf('\n---\n', dashStart);
+    const dashSection = fullSkill.slice(dashStart, dashEnd > dashStart ? dashEnd : undefined);
+    fs.writeFileSync(path.join(dashDir, 'ship-SKILL.md'), dashSection);
   });
 
   afterAll(() => {
@@ -605,7 +611,7 @@ Skip the preamble, lake intro, telemetry, and all other ship steps.
 Write the dashboard output to ${dashDir}/dashboard-output.md`,
       workingDirectory: dashDir,
       maxTurns: 12,
-      timeout: 90_000,
+      timeout: 180_000,
       testName: 'review-dashboard-via',
       runId,
     });
@@ -639,7 +645,7 @@ Write the dashboard output to ${dashDir}/dashboard-output.md`,
     );
     // Ship dashboard should not gate when eng review is clear
     expect(gateQuestions).toHaveLength(0);
-  }, 120_000);
+  }, 240_000);
 });
 
 // Module-level afterAll — finalize eval collector after all tests complete

test/skill-e2e-sidebar.test.ts

@@ -0,0 +1,279 @@
+/**
+ * Layer 4: E2E tests for the sidebar agent.
+ *
+ * sidebar-url-accuracy: Deterministic test that verifies the activeTabUrl fix.
+ *   Starts server (no browser), POSTs to /sidebar-command with different activeTabUrl
+ *   values, reads the queue file, and verifies the prompt uses the extension URL.
+ *   No real Claude needed — this is a fast, cheap, deterministic test.
+ *
+ * sidebar-navigate: Full E2E with real Claude (requires ANTHROPIC_API_KEY).
+ *   Starts server + sidebar-agent, sends a message, waits for Claude to respond.
+ *   Tests the complete message flow through the queue.
+ */
+
+import { describe, test, expect, beforeAll, afterAll } from 'bun:test';
+import { spawn, type Subprocess } from 'bun';
+import * as fs from 'fs';
+import * as os from 'os';
+import * as path from 'path';
+import {
+  ROOT,
+  describeIfSelected, testIfSelected,
+  createEvalCollector, finalizeEvalCollector,
+} from './helpers/e2e-helpers';
+
+const evalCollector = createEvalCollector('e2e-sidebar');
+
+// --- Sidebar URL Accuracy (deterministic, no Claude) ---
+
+describeIfSelected('Sidebar URL accuracy E2E', ['sidebar-url-accuracy'], () => {
+  let serverProc: Subprocess | null = null;
+  let serverPort: number = 0;
+  let authToken: string = '';
+  let tmpDir: string = '';
+  let stateFile: string = '';
+  let queueFile: string = '';
+
+  async function api(pathname: string, opts: RequestInit = {}): Promise<Response> {
+    const headers: Record<string, string> = {
+      'Content-Type': 'application/json',
+      ...(opts.headers as Record<string, string> || {}),
+    };
+    if (!headers['Authorization'] && authToken) {
+      headers['Authorization'] = `Bearer ${authToken}`;
+    }
+    return fetch(`http://127.0.0.1:${serverPort}${pathname}`, { ...opts, headers });
+  }
+
+  beforeAll(async () => {
+    tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'sidebar-e2e-url-'));
+    stateFile = path.join(tmpDir, 'browse.json');
+    queueFile = path.join(tmpDir, 'sidebar-queue.jsonl');
+    fs.mkdirSync(path.dirname(queueFile), { recursive: true });
+
+    const serverScript = path.resolve(ROOT, 'browse', 'src', 'server.ts');
+    serverProc = spawn(['bun', 'run', serverScript], {
+      env: {
+        ...process.env,
+        BROWSE_STATE_FILE: stateFile,
+        BROWSE_HEADLESS_SKIP: '1',
+        BROWSE_PORT: '0',
+        SIDEBAR_QUEUE_PATH: queueFile,
+        BROWSE_IDLE_TIMEOUT: '300',
+      },
+      stdio: ['ignore', 'pipe', 'pipe'],
+    });
+
+    const deadline = Date.now() + 15000;
+    while (Date.now() < deadline) {
+      if (fs.existsSync(stateFile)) {
+        try {
+          const state = JSON.parse(fs.readFileSync(stateFile, 'utf-8'));
+          if (state.port && state.token) {
+            serverPort = state.port;
+            authToken = state.token;
+            break;
+          }
+        } catch {}
+      }
+      await new Promise(r => setTimeout(r, 100));
+    }
+    if (!serverPort) throw new Error('Server did not start in time');
+  }, 20000);
+
+  afterAll(() => {
+    if (serverProc) { try { serverProc.kill(); } catch {} }
+    finalizeEvalCollector(evalCollector);
+    try { fs.rmSync(tmpDir, { recursive: true, force: true }); } catch {}
+  });
+
+  testIfSelected('sidebar-url-accuracy', async () => {
+    // Fresh session
+    await api('/sidebar-session/new', { method: 'POST' });
+    fs.writeFileSync(queueFile, '');
+
+    const extensionUrl = 'https://example.com/user-navigated-here';
+    const resp = await api('/sidebar-command', {
+      method: 'POST',
+      body: JSON.stringify({
+        message: 'What page am I on?',
+        activeTabUrl: extensionUrl,
+      }),
+    });
+    expect(resp.status).toBe(200);
+
+    // Wait for queue entry
+    let lastEntry: any = null;
+    const deadline = Date.now() + 5000;
+    while (Date.now() < deadline) {
+      await new Promise(r => setTimeout(r, 100));
+      if (!fs.existsSync(queueFile)) continue;
+      const lines = fs.readFileSync(queueFile, 'utf-8').trim().split('\n').filter(Boolean);
+      if (lines.length > 0) {
+        lastEntry = JSON.parse(lines[lines.length - 1]);
+        break;
+      }
+    }
+
+    expect(lastEntry).not.toBeNull();
+    // Extension URL should be used, not the Playwright fallback
+    expect(lastEntry.pageUrl).toBe(extensionUrl);
+    expect(lastEntry.prompt).toContain(extensionUrl);
+    expect(lastEntry.pageUrl).not.toBe('about:blank');
+
+    // Also test: chrome:// URL should be rejected, falling back to about:blank
+    await api('/sidebar-agent/kill', { method: 'POST' });
+    fs.writeFileSync(queueFile, '');
+
+    await api('/sidebar-command', {
+      method: 'POST',
+      body: JSON.stringify({
+        message: 'test',
+        activeTabUrl: 'chrome://settings',
+      }),
+    });
+    await new Promise(r => setTimeout(r, 200));
+    const lines2 = fs.readFileSync(queueFile, 'utf-8').trim().split('\n').filter(Boolean);
+    if (lines2.length > 0) {
+      const entry2 = JSON.parse(lines2[lines2.length - 1]);
+      expect(entry2.pageUrl).toBe('about:blank');
+    }
+
+    evalCollector?.addTest({
+      name: 'sidebar-url-accuracy', suite: 'Sidebar URL accuracy E2E', tier: 'e2e',
+      passed: true,
+      duration_ms: 0,
+      cost_usd: 0,
+      exit_reason: 'success',
+    });
+  }, 30_000);
+});
+
+// --- Sidebar Navigate (real Claude, requires ANTHROPIC_API_KEY) ---
+
+describeIfSelected('Sidebar navigate E2E', ['sidebar-navigate'], () => {
+  let serverProc: Subprocess | null = null;
+  let agentProc: Subprocess | null = null;
+  let serverPort: number = 0;
+  let authToken: string = '';
+  let tmpDir: string = '';
+  let stateFile: string = '';
+  let queueFile: string = '';
+
+  async function api(pathname: string, opts: RequestInit = {}): Promise<Response> {
+    const headers: Record<string, string> = {
+      'Content-Type': 'application/json',
+      ...(opts.headers as Record<string, string> || {}),
+    };
+    if (!headers['Authorization'] && authToken) {
+      headers['Authorization'] = `Bearer ${authToken}`;
+    }
+    return fetch(`http://127.0.0.1:${serverPort}${pathname}`, { ...opts, headers });
+  }
+
+  beforeAll(async () => {
+    tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'sidebar-e2e-nav-'));
+    stateFile = path.join(tmpDir, 'browse.json');
+    queueFile = path.join(tmpDir, 'sidebar-queue.jsonl');
+    fs.mkdirSync(path.dirname(queueFile), { recursive: true });
+
+    // Start server WITHOUT headless skip — we need a real browser for Claude to use
+    const serverScript = path.resolve(ROOT, 'browse', 'src', 'server.ts');
+    serverProc = spawn(['bun', 'run', serverScript], {
+      env: {
+        ...process.env,
+        BROWSE_STATE_FILE: stateFile,
+        BROWSE_HEADLESS_SKIP: '1',  // Still skip browser — Claude uses curl/fetch instead
+        BROWSE_PORT: '0',
+        SIDEBAR_QUEUE_PATH: queueFile,
+        BROWSE_IDLE_TIMEOUT: '300',
+      },
+      stdio: ['ignore', 'pipe', 'pipe'],
+    });
+
+    const deadline = Date.now() + 15000;
+    while (Date.now() < deadline) {
+      if (fs.existsSync(stateFile)) {
+        try {
+          const state = JSON.parse(fs.readFileSync(stateFile, 'utf-8'));
+          if (state.port && state.token) {
+            serverPort = state.port;
+            authToken = state.token;
+            break;
+          }
+        } catch {}
+      }
+      await new Promise(r => setTimeout(r, 100));
+    }
+    if (!serverPort) throw new Error('Server did not start in time');
+
+    // Start sidebar-agent
+    const agentScript = path.resolve(ROOT, 'browse', 'src', 'sidebar-agent.ts');
+    agentProc = spawn(['bun', 'run', agentScript], {
+      env: {
+        ...process.env,
+        BROWSE_SERVER_PORT: String(serverPort),
+        BROWSE_STATE_FILE: stateFile,
+        SIDEBAR_QUEUE_PATH: queueFile,
+        SIDEBAR_AGENT_TIMEOUT: '90000',
+        BROWSE_BIN: 'echo',  // browse commands won't work, but Claude can use curl
+      },
+      stdio: ['ignore', 'pipe', 'pipe'],
+    });
+
+    await new Promise(r => setTimeout(r, 1500));
+  }, 25000);
+
+  afterAll(() => {
+    if (agentProc) { try { agentProc.kill(); } catch {} }
+    if (serverProc) { try { serverProc.kill(); } catch {} }
+    finalizeEvalCollector(evalCollector);
+    try { fs.rmSync(tmpDir, { recursive: true, force: true }); } catch {}
+  });
+
+  testIfSelected('sidebar-navigate', async () => {
+    await api('/sidebar-session/new', { method: 'POST' });
+    fs.writeFileSync(queueFile, '');
+    const startTime = Date.now();
+
+    // Ask Claude a simple question — it doesn't need browse commands for this
+    const resp = await api('/sidebar-command', {
+      method: 'POST',
+      body: JSON.stringify({
+        message: 'Say exactly "SIDEBAR_TEST_OK" and nothing else.',
+        activeTabUrl: 'https://example.com',
+      }),
+    });
+    expect(resp.status).toBe(200);
+
+    // Poll for agent_done
+    const deadline = Date.now() + 90000;
+    let entries: any[] = [];
+    while (Date.now() < deadline) {
+      const chatResp = await api('/sidebar-chat?after=0');
+      const data = await chatResp.json();
+      entries = data.entries;
+      if (entries.some((e: any) => e.type === 'agent_done')) break;
+      await new Promise(r => setTimeout(r, 2000));
+    }
+
+    const duration = Date.now() - startTime;
+    const doneEntry = entries.find((e: any) => e.type === 'agent_done');
+    expect(doneEntry).toBeDefined();
+
+    // Claude should have responded with something
+    const agentText = entries
+      .filter((e: any) => e.role === 'agent' && (e.type === 'text' || e.type === 'result'))
+      .map((e: any) => e.text || '')
+      .join(' ');
+    expect(agentText.length).toBeGreaterThan(0);
+
+    evalCollector?.addTest({
+      name: 'sidebar-navigate', suite: 'Sidebar navigate E2E', tier: 'e2e',
+      passed: !!doneEntry && agentText.length > 0,
+      duration_ms: duration,
+      cost_usd: 0,
+      exit_reason: doneEntry ? 'success' : 'timeout',
+    });
+  }, 120_000);
+});

test/skill-routing-e2e.test.ts

@@ -250,56 +250,10 @@ describeE2E('Skill Routing E2E — Developer Journey', () => {
     }
   }, 150_000);
 
-  testIfSelected('journey-think-bigger', async () => {
-    const tmpDir = createRoutingWorkDir('think-bigger');
-    try {
-      fs.writeFileSync(path.join(tmpDir, 'plan.md'), `# Waitlist App Architecture
-
-## Components
-- REST API (Express.js)
-- PostgreSQL database
-- React frontend
-- SMS integration (Twilio)
-
-## Data Model
-- restaurants (id, name, settings)
-- parties (id, restaurant_id, name, size, phone, status, created_at)
-- wait_estimates (id, restaurant_id, avg_wait_minutes)
-
-## API Endpoints
-- POST /api/parties - add party to waitlist
-- GET /api/parties - list current waitlist
-- PATCH /api/parties/:id/status - update party status
-- GET /api/estimate - get current wait estimate
-`);
-      spawnSync('git', ['add', '.'], { cwd: tmpDir, stdio: 'pipe', timeout: 5000 });
-      spawnSync('git', ['commit', '-m', 'initial'], { cwd: tmpDir, stdio: 'pipe', timeout: 5000 });
-
-      const testName = 'journey-think-bigger';
-      const expectedSkill = 'plan-ceo-review';
-      const result = await runSkillTest({
-        prompt: "Actually, looking at this plan again, I feel like we're thinking too small. We're just doing waitlists but what about the whole restaurant guest experience? Is there a bigger opportunity here we should go after?",
-        workingDirectory: tmpDir,
-        maxTurns: 5,
-        allowedTools: ['Skill', 'Read', 'Bash', 'Glob', 'Grep'],
-        timeout: 120_000,
-        testName,
-        runId,
-      });
-
-      const skillCalls = result.toolCalls.filter(tc => tc.tool === 'Skill');
-      const actualSkill = skillCalls.length > 0 ? skillCalls[0]?.input?.skill : undefined;
-
-      logCost(`journey: ${testName}`, result);
-      recordRouting(testName, result, expectedSkill, actualSkill);
-
-      expect(skillCalls.length, `Expected Skill tool to be called but got 0 calls. Claude may have answered directly without invoking a skill. Tool calls: ${result.toolCalls.map(tc => tc.tool).join(', ')}`).toBeGreaterThan(0);
-      const validSkills = ['plan-ceo-review', 'office-hours'];
-      expect(validSkills, `Expected one of ${validSkills.join('/')} but got ${actualSkill}`).toContain(actualSkill);
-    } finally {
-      fs.rmSync(tmpDir, { recursive: true, force: true });
-    }
-  }, 180_000);
+  // Removed: journey-think-bigger
+  // Tested ambiguous routing ("think bigger" → plan-ceo-review) but Claude
+  // legitimately answers directly instead of routing. Never passed reliably.
+  // The other 10 journey tests cover routing with clear signals.
 
   testIfSelected('journey-debug', async () => {
     const tmpDir = createRoutingWorkDir('debug');

test/telemetry.test.ts

@@ -188,6 +188,82 @@ describe('gstack-telemetry-log', () => {
     expect(events[0]).toHaveProperty('_branch');
   });
 
+  // ─── json_safe() injection prevention tests ────────────────
+  test('sanitizes skill name with quote injection attempt', () => {
+    setConfig('telemetry', 'anonymous');
+    run(`${BIN}/gstack-telemetry-log --skill 'review","injected":"true' --duration 10 --outcome success --session-id inj-1`);
+
+    const lines = readJsonl();
+    expect(lines).toHaveLength(1);
+    // Must be valid JSON (no injection — quotes stripped, so no field injection possible)
+    const event = JSON.parse(lines[0]);
+    // The key check: no injected top-level property was created
+    expect(event).not.toHaveProperty('injected');
+    // Skill field should have quotes stripped but content preserved
+    expect(event.skill).not.toContain('"');
+  });
+
+  test('truncates skill name exceeding 200 chars', () => {
+    setConfig('telemetry', 'anonymous');
+    const longSkill = 'a'.repeat(250);
+    run(`${BIN}/gstack-telemetry-log --skill '${longSkill}' --duration 10 --outcome success --session-id trunc-1`);
+
+    const events = parseJsonl();
+    expect(events[0].skill.length).toBeLessThanOrEqual(200);
+  });
+
+  test('sanitizes outcome with newline injection attempt', () => {
+    setConfig('telemetry', 'anonymous');
+    // Use printf to pass actual newline in the argument
+    run(`bash -c 'OUTCOME=$(printf "success\\nfake\\":\\"true"); ${BIN}/gstack-telemetry-log --skill qa --duration 10 --outcome "$OUTCOME" --session-id inj-2'`);
+
+    const lines = readJsonl();
+    expect(lines).toHaveLength(1);
+    const event = JSON.parse(lines[0]);
+    expect(event).not.toHaveProperty('fake');
+  });
+
+  test('sanitizes session_id with backslash-quote injection', () => {
+    setConfig('telemetry', 'anonymous');
+    run(`${BIN}/gstack-telemetry-log --skill qa --duration 10 --outcome success --session-id 'id\\\\"","x":"y'`);
+
+    const lines = readJsonl();
+    expect(lines).toHaveLength(1);
+    const event = JSON.parse(lines[0]);
+    expect(event).not.toHaveProperty('x');
+  });
+
+  test('sanitizes error_class with quote injection', () => {
+    setConfig('telemetry', 'anonymous');
+    run(`${BIN}/gstack-telemetry-log --skill qa --duration 10 --outcome error --error-class 'timeout","extra":"val' --session-id inj-3`);
+
+    const lines = readJsonl();
+    expect(lines).toHaveLength(1);
+    const event = JSON.parse(lines[0]);
+    expect(event).not.toHaveProperty('extra');
+  });
+
+  test('sanitizes failed_step with quote injection', () => {
+    setConfig('telemetry', 'anonymous');
+    run(`${BIN}/gstack-telemetry-log --skill qa --duration 10 --outcome error --failed-step 'step1","hacked":"yes' --session-id inj-4`);
+
+    const lines = readJsonl();
+    expect(lines).toHaveLength(1);
+    const event = JSON.parse(lines[0]);
+    expect(event).not.toHaveProperty('hacked');
+  });
+
+  test('escapes error_message quotes and preserves content', () => {
+    setConfig('telemetry', 'anonymous');
+    run(`${BIN}/gstack-telemetry-log --skill qa --duration 10 --outcome error --error-message 'Error: file "test.txt" not found' --session-id inj-5`);
+
+    const lines = readJsonl();
+    expect(lines).toHaveLength(1);
+    const event = JSON.parse(lines[0]);
+    expect(event.error_message).toContain('file');
+    expect(event.error_message).toContain('not found');
+  });
+
   test('creates analytics directory if missing', () => {
     // Remove analytics dir
     const analyticsDir = path.join(tmpDir, 'analytics');