Merge remote-tracking branch 'origin/main' into garrytan/design-html-skill

test/audit-compliance.test.ts

@@ -45,15 +45,17 @@ describe('Audit compliance', () => {
     expect(completionSection).toContain('_TEL" != "off"');
   });
 
-  // Fix 3: W012 — Bun install is version-pinned
-  test('bun install commands use version pinning', () => {
+  // Round 2 Fix 1: W012 — Bun install uses checksum verification
+  test('bun install uses checksum-verified method', () => {
     const browseResolver = readFileSync(join(ROOT, 'scripts/resolvers/browse.ts'), 'utf-8');
-    expect(browseResolver).toContain('BUN_VERSION');
-    // Should not have unpinned curl|bash (without BUN_VERSION on same line)
-    const lines = browseResolver.split('\n');
+    expect(browseResolver).toContain('shasum -a 256');
+    expect(browseResolver).toContain('BUN_INSTALL_SHA');
+    const setup = readFileSync(join(ROOT, 'setup'), 'utf-8');
+    // Setup error message should not have unverified curl|bash
+    const lines = setup.split('\n');
     for (const line of lines) {
-      if (line.includes('bun.sh/install') && line.includes('bash') && !line.includes('BUN_VERSION') && !line.includes('command -v')) {
-        throw new Error(`Unpinned bun install found: ${line.trim()}`);
+      if (line.includes('bun.sh/install') && line.includes('| bash') && !line.includes('shasum')) {
+        throw new Error(`Unverified bun install found: ${line.trim()}`);
       }
     }
   });
@@ -69,6 +71,17 @@ describe('Audit compliance', () => {
     expect(between.toLowerCase()).toContain('untrusted');
   });
 
+  // Round 2 Fix 2: Trust boundary markers + helper + wrapping in all paths
+  test('browse wraps untrusted content with trust boundary markers', () => {
+    const commands = readFileSync(join(ROOT, 'browse/src/commands.ts'), 'utf-8');
+    expect(commands).toContain('PAGE_CONTENT_COMMANDS');
+    expect(commands).toContain('wrapUntrustedContent');
+    const server = readFileSync(join(ROOT, 'browse/src/server.ts'), 'utf-8');
+    expect(server).toContain('wrapUntrustedContent');
+    const meta = readFileSync(join(ROOT, 'browse/src/meta-commands.ts'), 'utf-8');
+    expect(meta).toContain('wrapUntrustedContent');
+  });
+
   // Fix 5: Data flow documentation in review.ts
   test('review.ts has data flow documentation', () => {
     const review = readFileSync(join(ROOT, 'scripts/resolvers/review.ts'), 'utf-8');
@@ -76,6 +89,20 @@ describe('Audit compliance', () => {
     expect(review).toContain('Data NOT sent');
   });
 
+  // Round 2 Fix 3: Extension sender validation + message type allowlist
+  test('extension background.js validates message sender', () => {
+    const bg = readFileSync(join(ROOT, 'extension/background.js'), 'utf-8');
+    expect(bg).toContain('sender.id !== chrome.runtime.id');
+    expect(bg).toContain('ALLOWED_TYPES');
+  });
+
+  // Round 2 Fix 4: Chrome CDP binds to localhost only
+  test('chrome-cdp binds to localhost only', () => {
+    const cdp = readFileSync(join(ROOT, 'bin/chrome-cdp'), 'utf-8');
+    expect(cdp).toContain('--remote-debugging-address=127.0.0.1');
+    expect(cdp).toContain('--remote-allow-origins=');
+  });
+
   // Fix 2+6: All generated SKILL.md files with telemetry are conditional
   test('all generated SKILL.md files with telemetry calls use conditional pattern', () => {
     const skills = getAllSkillMds();

test/gen-skill-docs.test.ts

@@ -1155,6 +1155,138 @@ describe('BENEFITS_FROM resolver', () => {
     expect(ceoContent).toContain('office-hours/SKILL.md');
     expect(engContent).toContain('office-hours/SKILL.md');
   });
+
+  test('BENEFITS_FROM delegates to INVOKE_SKILL pattern', () => {
+    // Should contain the INVOKE_SKILL-style loading prose (not the old manual skip list)
+    expect(engContent).toContain('Follow its instructions from top to bottom');
+    expect(engContent).toContain('skipping these sections');
+    expect(ceoContent).toContain('Follow its instructions from top to bottom');
+  });
+});
+
+// --- {{INVOKE_SKILL}} resolver tests ---
+
+describe('INVOKE_SKILL resolver', () => {
+  const ceoContent = fs.readFileSync(path.join(ROOT, 'plan-ceo-review', 'SKILL.md'), 'utf-8');
+
+  test('plan-ceo-review uses INVOKE_SKILL for mid-session office-hours fallback', () => {
+    // The mid-session detection path should use INVOKE_SKILL-generated prose
+    expect(ceoContent).toContain('office-hours/SKILL.md');
+    expect(ceoContent).toContain('Follow its instructions from top to bottom');
+  });
+
+  test('INVOKE_SKILL output includes default skip list', () => {
+    expect(ceoContent).toContain('Preamble (run first)');
+    expect(ceoContent).toContain('Telemetry (run last)');
+    expect(ceoContent).toContain('AskUserQuestion Format');
+  });
+
+  test('INVOKE_SKILL output includes error handling', () => {
+    expect(ceoContent).toContain('If unreadable');
+    expect(ceoContent).toContain('Could not load');
+  });
+
+  test('template uses {{INVOKE_SKILL:office-hours}} placeholder', () => {
+    const tmpl = fs.readFileSync(path.join(ROOT, 'plan-ceo-review', 'SKILL.md.tmpl'), 'utf-8');
+    expect(tmpl).toContain('{{INVOKE_SKILL:office-hours}}');
+  });
+});
+
+// --- {{CHANGELOG_WORKFLOW}} resolver tests ---
+
+describe('CHANGELOG_WORKFLOW resolver', () => {
+  const shipContent = fs.readFileSync(path.join(ROOT, 'ship', 'SKILL.md'), 'utf-8');
+
+  test('ship SKILL.md contains changelog workflow', () => {
+    expect(shipContent).toContain('CHANGELOG (auto-generate)');
+    expect(shipContent).toContain('git log <base>..HEAD --oneline');
+  });
+
+  test('changelog workflow includes cross-check step', () => {
+    expect(shipContent).toContain('Cross-check');
+    expect(shipContent).toContain('Every commit must map to at least one bullet point');
+  });
+
+  test('changelog workflow includes voice guidance', () => {
+    expect(shipContent).toContain('Lead with what the user can now **do**');
+  });
+
+  test('template uses {{CHANGELOG_WORKFLOW}} placeholder', () => {
+    const tmpl = fs.readFileSync(path.join(ROOT, 'ship', 'SKILL.md.tmpl'), 'utf-8');
+    expect(tmpl).toContain('{{CHANGELOG_WORKFLOW}}');
+    // Should NOT contain the old inline changelog content
+    expect(tmpl).not.toContain('Group commits by theme');
+  });
+
+  test('changelog workflow includes keep-changelog format', () => {
+    expect(shipContent).toContain('### Added');
+    expect(shipContent).toContain('### Fixed');
+  });
+});
+
+// --- Parameterized resolver infrastructure tests ---
+
+describe('parameterized resolver support', () => {
+  test('gen-skill-docs regex handles colon-separated args', () => {
+    // Verify the template containing {{INVOKE_SKILL:office-hours}} was processed
+    // without leaving unresolved placeholders
+    const ceoContent = fs.readFileSync(path.join(ROOT, 'plan-ceo-review', 'SKILL.md'), 'utf-8');
+    expect(ceoContent).not.toMatch(/\{\{INVOKE_SKILL:[^}]+\}\}/);
+  });
+
+  test('templates with parameterized resolvers pass unresolved check', () => {
+    // All generated SKILL.md files should have no unresolved {{...}} placeholders
+    const skillDirs = fs.readdirSync(ROOT).filter(d =>
+      fs.existsSync(path.join(ROOT, d, 'SKILL.md'))
+    );
+    for (const dir of skillDirs) {
+      const content = fs.readFileSync(path.join(ROOT, dir, 'SKILL.md'), 'utf-8');
+      const unresolved = content.match(/\{\{[A-Z_]+(?::[^}]*)?\}\}/g);
+      if (unresolved) {
+        throw new Error(`${dir}/SKILL.md has unresolved placeholders: ${unresolved.join(', ')}`);
+      }
+    }
+  });
+});
+
+// --- Preamble routing injection tests ---
+
+describe('preamble routing injection', () => {
+  const shipContent = fs.readFileSync(path.join(ROOT, 'ship', 'SKILL.md'), 'utf-8');
+
+  test('preamble bash checks for routing section in CLAUDE.md', () => {
+    expect(shipContent).toContain('grep -q "## Skill routing" CLAUDE.md');
+    expect(shipContent).toContain('HAS_ROUTING');
+  });
+
+  test('preamble bash reads routing_declined config', () => {
+    expect(shipContent).toContain('routing_declined');
+    expect(shipContent).toContain('ROUTING_DECLINED');
+  });
+
+  test('preamble includes routing injection AskUserQuestion', () => {
+    expect(shipContent).toContain('Add routing rules to CLAUDE.md');
+    expect(shipContent).toContain("I'll invoke skills manually");
+  });
+
+  test('routing injection respects prior decline', () => {
+    expect(shipContent).toContain('ROUTING_DECLINED');
+    expect(shipContent).toMatch(/routing_declined.*true/);
+  });
+
+  test('routing injection only fires when all conditions met', () => {
+    // Must be: HAS_ROUTING=no AND ROUTING_DECLINED=false AND PROACTIVE_PROMPTED=yes
+    expect(shipContent).toContain('HAS_ROUTING');
+    expect(shipContent).toContain('ROUTING_DECLINED');
+    expect(shipContent).toContain('PROACTIVE_PROMPTED');
+  });
+
+  test('routing section content includes key routing rules', () => {
+    expect(shipContent).toContain('invoke office-hours');
+    expect(shipContent).toContain('invoke investigate');
+    expect(shipContent).toContain('invoke ship');
+    expect(shipContent).toContain('invoke qa');
+  });
 });
 
 // --- {{DESIGN_OUTSIDE_VOICES}} resolver tests ---
@@ -1795,11 +1927,12 @@ describe('setup script validation', () => {
   });
 
   test('link_claude_skill_dirs creates relative symlinks', () => {
-    // Claude links should be relative: ln -snf "gstack/skill_name"
+    // Claude links should be relative: ln -snf "gstack/$dir_name"
+    // Uses dir_name (not skill_name) because symlink target must point to the physical directory
     const fnStart = setupContent.indexOf('link_claude_skill_dirs()');
     const fnEnd = setupContent.indexOf('}', setupContent.indexOf('linked[@]}', fnStart));
     const fnBody = setupContent.slice(fnStart, fnEnd);
-    expect(fnBody).toContain('ln -snf "gstack/$skill_name"');
+    expect(fnBody).toContain('ln -snf "gstack/$dir_name"');
   });
 
   test('setup supports --host auto|claude|codex|kiro', () => {
@@ -2038,6 +2171,100 @@ describe('telemetry', () => {
   });
 });
 
+describe('community fixes wave', () => {
+  // Helper to get all generated SKILL.md files
+  function getAllSkillMds(): Array<{ name: string; content: string }> {
+    const results: Array<{ name: string; content: string }> = [];
+    const rootPath = path.join(ROOT, 'SKILL.md');
+    if (fs.existsSync(rootPath)) {
+      results.push({ name: 'root', content: fs.readFileSync(rootPath, 'utf-8') });
+    }
+    for (const entry of fs.readdirSync(ROOT, { withFileTypes: true })) {
+      if (!entry.isDirectory() || entry.name.startsWith('.') || entry.name === 'node_modules') continue;
+      const skillPath = path.join(ROOT, entry.name, 'SKILL.md');
+      if (fs.existsSync(skillPath)) {
+        results.push({ name: entry.name, content: fs.readFileSync(skillPath, 'utf-8') });
+      }
+    }
+    return results;
+  }
+
+  // #594 — Discoverability: every SKILL.md.tmpl description contains "gstack"
+  test('every SKILL.md.tmpl description contains "gstack"', () => {
+    for (const skill of ALL_SKILLS) {
+      const tmplPath = skill.dir === '.' ? path.join(ROOT, 'SKILL.md.tmpl') : path.join(ROOT, skill.dir, 'SKILL.md.tmpl');
+      const content = fs.readFileSync(tmplPath, 'utf-8');
+      const desc = extractDescription(content);
+      expect(desc.toLowerCase()).toContain('gstack');
+    }
+  });
+
+  // #594 — Discoverability: first line of each description is under 120 chars
+  test('every SKILL.md.tmpl description first line is under 120 chars', () => {
+    for (const skill of ALL_SKILLS) {
+      const tmplPath = skill.dir === '.' ? path.join(ROOT, 'SKILL.md.tmpl') : path.join(ROOT, skill.dir, 'SKILL.md.tmpl');
+      const content = fs.readFileSync(tmplPath, 'utf-8');
+      const desc = extractDescription(content);
+      const firstLine = desc.split('\n')[0];
+      expect(firstLine.length).toBeLessThanOrEqual(120);
+    }
+  });
+
+  // #573 — Feature signals: ship/SKILL.md contains feature signal detection
+  test('ship/SKILL.md contains feature signal detection in Step 4', () => {
+    const content = fs.readFileSync(path.join(ROOT, 'ship', 'SKILL.md'), 'utf-8');
+    expect(content.toLowerCase()).toContain('feature signal');
+  });
+
+  // #510 — Context warnings: no SKILL.md contains "running low on context"
+  test('no generated SKILL.md contains "running low on context"', () => {
+    const skills = getAllSkillMds();
+    for (const { name, content } of skills) {
+      expect(content).not.toContain('running low on context');
+    }
+  });
+
+  // #510 — Context warnings: plan-eng-review has explicit anti-warning
+  test('plan-eng-review/SKILL.md contains "Do not preemptively warn"', () => {
+    const content = fs.readFileSync(path.join(ROOT, 'plan-eng-review', 'SKILL.md'), 'utf-8');
+    expect(content).toContain('Do not preemptively warn');
+  });
+
+  // #474 — Safety Net: no SKILL.md uses find with -delete
+  test('no generated SKILL.md contains find with -delete flag', () => {
+    const skills = getAllSkillMds();
+    for (const { name, content } of skills) {
+      // Match find commands that use -delete (but not prose mentioning the word "delete")
+      const lines = content.split('\n');
+      for (const line of lines) {
+        if (line.includes('find ') && line.includes('-delete')) {
+          throw new Error(`${name}/SKILL.md contains find with -delete: ${line.trim()}`);
+        }
+      }
+    }
+  });
+
+  // #467 — Telemetry: preamble JSONL writes are gated by telemetry setting
+  test('preamble JSONL writes are inside telemetry conditional', () => {
+    const preamble = fs.readFileSync(path.join(ROOT, 'scripts/resolvers/preamble.ts'), 'utf-8');
+    // Find all skill-usage.jsonl write lines
+    const lines = preamble.split('\n');
+    for (let i = 0; i < lines.length; i++) {
+      if (lines[i].includes('skill-usage.jsonl') && lines[i].includes('>>')) {
+        // Look backwards for a telemetry conditional within 5 lines
+        let foundConditional = false;
+        for (let j = i - 1; j >= Math.max(0, i - 5); j--) {
+          if (lines[j].includes('_TEL') && lines[j].includes('off')) {
+            foundConditional = true;
+            break;
+          }
+        }
+        expect(foundConditional).toBe(true);
+      }
+    }
+  });
+});
+
 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

test/relink.test.ts

@@ -0,0 +1,152 @@
+import { describe, test, expect, beforeEach, afterEach } from 'bun:test';
+import { execSync } 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 skillsDir: string;
+let installDir: string;
+
+function run(cmd: string, env: Record<string, string> = {}, expectFail = false): string {
+  try {
+    return execSync(cmd, {
+      cwd: ROOT,
+      env: { ...process.env, GSTACK_STATE_DIR: tmpDir, ...env },
+      encoding: 'utf-8',
+      timeout: 10000,
+      stdio: ['pipe', 'pipe', 'pipe'],
+    }).trim();
+  } catch (e: any) {
+    if (expectFail) return (e.stderr || e.stdout || '').toString().trim();
+    throw e;
+  }
+}
+
+// Create a mock gstack install directory with skill subdirs
+function setupMockInstall(skills: string[]): void {
+  installDir = path.join(tmpDir, 'gstack-install');
+  skillsDir = path.join(tmpDir, 'skills');
+  fs.mkdirSync(installDir, { recursive: true });
+  fs.mkdirSync(skillsDir, { recursive: true });
+
+  // Copy the real gstack-config and gstack-relink to the mock install
+  const mockBin = path.join(installDir, 'bin');
+  fs.mkdirSync(mockBin, { recursive: true });
+  fs.copyFileSync(path.join(BIN, 'gstack-config'), path.join(mockBin, 'gstack-config'));
+  fs.chmodSync(path.join(mockBin, 'gstack-config'), 0o755);
+  if (fs.existsSync(path.join(BIN, 'gstack-relink'))) {
+    fs.copyFileSync(path.join(BIN, 'gstack-relink'), path.join(mockBin, 'gstack-relink'));
+    fs.chmodSync(path.join(mockBin, 'gstack-relink'), 0o755);
+  }
+
+  // Create mock skill directories
+  for (const skill of skills) {
+    fs.mkdirSync(path.join(installDir, skill), { recursive: true });
+    fs.writeFileSync(path.join(installDir, skill, 'SKILL.md'), `# ${skill}`);
+  }
+}
+
+beforeEach(() => {
+  tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-relink-test-'));
+});
+
+afterEach(() => {
+  fs.rmSync(tmpDir, { recursive: true, force: true });
+});
+
+describe('gstack-relink (#578)', () => {
+  // Test 11: prefixed symlinks when skill_prefix=true
+  test('creates gstack-* symlinks when skill_prefix=true', () => {
+    setupMockInstall(['qa', 'ship', 'review']);
+    // Set config to prefix mode
+    run(`${path.join(installDir, 'bin', 'gstack-config')} set skill_prefix true`);
+    // Run relink with env pointing to the mock install
+    const output = run(`${path.join(installDir, 'bin', 'gstack-relink')}`, {
+      GSTACK_INSTALL_DIR: installDir,
+      GSTACK_SKILLS_DIR: skillsDir,
+    });
+    // Verify gstack-* symlinks exist
+    expect(fs.existsSync(path.join(skillsDir, 'gstack-qa'))).toBe(true);
+    expect(fs.existsSync(path.join(skillsDir, 'gstack-ship'))).toBe(true);
+    expect(fs.existsSync(path.join(skillsDir, 'gstack-review'))).toBe(true);
+    expect(output).toContain('gstack-');
+  });
+
+  // Test 12: flat symlinks when skill_prefix=false
+  test('creates flat symlinks when skill_prefix=false', () => {
+    setupMockInstall(['qa', 'ship', 'review']);
+    run(`${path.join(installDir, 'bin', 'gstack-config')} set skill_prefix false`);
+    const output = run(`${path.join(installDir, 'bin', 'gstack-relink')}`, {
+      GSTACK_INSTALL_DIR: installDir,
+      GSTACK_SKILLS_DIR: skillsDir,
+    });
+    expect(fs.existsSync(path.join(skillsDir, 'qa'))).toBe(true);
+    expect(fs.existsSync(path.join(skillsDir, 'ship'))).toBe(true);
+    expect(fs.existsSync(path.join(skillsDir, 'review'))).toBe(true);
+    expect(output).toContain('flat');
+  });
+
+  // Test 13: cleans stale symlinks from opposite mode
+  test('cleans up stale symlinks from opposite mode', () => {
+    setupMockInstall(['qa', 'ship']);
+    // Create prefixed symlinks first
+    run(`${path.join(installDir, 'bin', 'gstack-config')} set skill_prefix true`);
+    run(`${path.join(installDir, 'bin', 'gstack-relink')}`, {
+      GSTACK_INSTALL_DIR: installDir,
+      GSTACK_SKILLS_DIR: skillsDir,
+    });
+    expect(fs.existsSync(path.join(skillsDir, 'gstack-qa'))).toBe(true);
+
+    // Switch to flat mode
+    run(`${path.join(installDir, 'bin', 'gstack-config')} set skill_prefix false`);
+    run(`${path.join(installDir, 'bin', 'gstack-relink')}`, {
+      GSTACK_INSTALL_DIR: installDir,
+      GSTACK_SKILLS_DIR: skillsDir,
+    });
+
+    // Flat symlinks should exist, prefixed should be gone
+    expect(fs.existsSync(path.join(skillsDir, 'qa'))).toBe(true);
+    expect(fs.existsSync(path.join(skillsDir, 'gstack-qa'))).toBe(false);
+  });
+
+  // Test 14: error when install dir missing
+  test('prints error when install dir missing', () => {
+    const output = run(`${BIN}/gstack-relink`, {
+      GSTACK_INSTALL_DIR: '/nonexistent/path/gstack',
+      GSTACK_SKILLS_DIR: '/nonexistent/path/skills',
+    }, true);
+    expect(output).toContain('setup');
+  });
+
+  // Test: gstack-upgrade does NOT get double-prefixed
+  test('does not double-prefix gstack-upgrade directory', () => {
+    setupMockInstall(['qa', 'ship', 'gstack-upgrade']);
+    run(`${path.join(installDir, 'bin', 'gstack-config')} set skill_prefix true`);
+    run(`${path.join(installDir, 'bin', 'gstack-relink')}`, {
+      GSTACK_INSTALL_DIR: installDir,
+      GSTACK_SKILLS_DIR: skillsDir,
+    });
+    // gstack-upgrade should keep its name, NOT become gstack-gstack-upgrade
+    expect(fs.existsSync(path.join(skillsDir, 'gstack-upgrade'))).toBe(true);
+    expect(fs.existsSync(path.join(skillsDir, 'gstack-gstack-upgrade'))).toBe(false);
+    // Regular skills still get prefixed
+    expect(fs.existsSync(path.join(skillsDir, 'gstack-qa'))).toBe(true);
+  });
+
+  // Test 15: gstack-config set skill_prefix triggers relink
+  test('gstack-config set skill_prefix triggers relink', () => {
+    setupMockInstall(['qa', 'ship']);
+    // Run gstack-config set which should auto-trigger relink
+    run(`${path.join(installDir, 'bin', 'gstack-config')} set skill_prefix true`, {
+      GSTACK_INSTALL_DIR: installDir,
+      GSTACK_SKILLS_DIR: skillsDir,
+    });
+    // If relink was triggered, symlinks should exist
+    expect(fs.existsSync(path.join(skillsDir, 'gstack-qa'))).toBe(true);
+    expect(fs.existsSync(path.join(skillsDir, 'gstack-ship'))).toBe(true);
+  });
+});

test/skill-routing-e2e.test.ts

@@ -93,11 +93,30 @@ function installSkills(tmpDir: string) {
     }
   }
 
-  // Copy CLAUDE.md so Claude has project context for skill routing.
-  const claudeMdSrc = path.join(ROOT, 'CLAUDE.md');
-  if (fs.existsSync(claudeMdSrc)) {
-    fs.copyFileSync(claudeMdSrc, path.join(tmpDir, 'CLAUDE.md'));
-  }
+  // Write a CLAUDE.md with explicit routing instructions.
+  // The skill descriptions in system-reminder aren't strong enough to override
+  // Claude's default behavior of answering directly. A CLAUDE.md instruction
+  // puts routing rules in project context which Claude weighs more heavily.
+  fs.writeFileSync(path.join(tmpDir, 'CLAUDE.md'), `# Project Instructions
+
+## Skill routing
+
+When the user's request matches an available skill, ALWAYS invoke it using the Skill
+tool as your FIRST action. Do NOT answer directly, do NOT use other tools first.
+The skill has specialized workflows that produce better results than ad-hoc answers.
+
+Key routing rules:
+- Product ideas, "is this worth building", brainstorming → invoke office-hours
+- Bugs, errors, "why is this broken", 500 errors → invoke investigate
+- Ship, deploy, push, create PR → invoke ship
+- QA, test the site, find bugs → invoke qa
+- Code review, check my diff → invoke review
+- Update docs after shipping → invoke document-release
+- Weekly retro → invoke retro
+- Design system, brand → invoke design-consultation
+- Visual audit, design polish → invoke design-review
+- Architecture review → invoke plan-eng-review
+`);
 }
 
 /** Init a git repo with config */

test/skill-validation.test.ts

@@ -1409,13 +1409,13 @@ describe('Skill trigger phrases', () => {
   ];
 
   for (const skill of SKILLS_REQUIRING_PROACTIVE) {
-    test(`${skill}/SKILL.md has "Proactively suggest" phrase`, () => {
+    test(`${skill}/SKILL.md has proactive routing phrase`, () => {
       const skillPath = path.join(ROOT, skill, 'SKILL.md');
       if (!fs.existsSync(skillPath)) return;
       const content = fs.readFileSync(skillPath, 'utf-8');
       const frontmatterEnd = content.indexOf('---', 4);
       const frontmatter = content.slice(0, frontmatterEnd);
-      expect(frontmatter).toMatch(/Proactively suggest/i);
+      expect(frontmatter).toMatch(/Proactively (suggest|invoke)/i);
     });
   }
 });
@@ -1547,3 +1547,30 @@ describe('Test failure triage in ship skill', () => {
     expect(content).toContain('In-branch test failures');
   });
 });
+
+describe('sidebar agent (#584)', () => {
+  // #584 — Sidebar Write: sidebar-agent.ts allowedTools includes Write
+  test('sidebar-agent.ts allowedTools includes Write', () => {
+    const content = fs.readFileSync(path.join(ROOT, 'browse', 'src', 'sidebar-agent.ts'), 'utf-8');
+    // Find the allowedTools line in the askClaude function
+    const match = content.match(/--allowedTools['"]\s*,\s*['"]([^'"]+)['"]/);
+    expect(match).not.toBeNull();
+    expect(match![1]).toContain('Write');
+  });
+
+  // #584 — Server Write: server.ts allowedTools includes Write (DRY parity)
+  test('server.ts allowedTools includes Write', () => {
+    const content = fs.readFileSync(path.join(ROOT, 'browse', 'src', 'server.ts'), 'utf-8');
+    // Find the sidebar allowedTools in the headed-mode path
+    const match = content.match(/--allowedTools['"]\s*,\s*['"]([^'"]+)['"]/);
+    expect(match).not.toBeNull();
+    expect(match![1]).toContain('Write');
+  });
+
+  // #584 — Sidebar stderr: stderr handler is not empty
+  test('sidebar-agent.ts stderr handler is not empty', () => {
+    const content = fs.readFileSync(path.join(ROOT, 'browse', 'src', 'sidebar-agent.ts'), 'utf-8');
+    // The stderr handler should NOT be an empty arrow function
+    expect(content).not.toContain("proc.stderr.on('data', () => {})");
+  });
+});

test/telemetry.test.ts

@@ -396,3 +396,25 @@ describe('gstack-community-dashboard', () => {
     expect(output).not.toContain('Supabase not configured');
   });
 });
+
+describe('preamble telemetry gating (#467)', () => {
+  test('preamble source does not write JSONL unconditionally', () => {
+    const preamble = fs.readFileSync(path.join(ROOT, 'scripts', 'resolvers', 'preamble.ts'), 'utf-8');
+    const lines = preamble.split('\n');
+    for (let i = 0; i < lines.length; i++) {
+      if (lines[i].includes('skill-usage.jsonl') && lines[i].includes('>>')) {
+        // Each JSONL write must be inside a _TEL conditional (within 5 lines above)
+        let foundConditional = false;
+        for (let j = i - 1; j >= Math.max(0, i - 5); j--) {
+          if (lines[j].includes('_TEL') && lines[j].includes('off')) {
+            foundConditional = true;
+            break;
+          }
+        }
+        if (!foundConditional) {
+          throw new Error(`Unconditional JSONL write at preamble.ts line ${i + 1}: ${lines[i].trim()}`);
+        }
+      }
+    }
+  });
+});