Merge remote-tracking branch 'origin/main' into garrytan/learn-from-reviews

Resolved conflicts:
- VERSION: bumped to 0.13.10.0 (our changes on top of main's 0.13.9.0)
- CHANGELOG.md: kept both entries, ours on top with updated version
- plan-ceo-review/SKILL.md.tmpl: took main's INVOKE_SKILL resolver
- scripts/resolvers/review.ts: took main's invokeBlock pattern
- scripts/resolvers/preamble.ts: wrapped JSONL writes in telemetry conditional
- test/skill-validation.test.ts: removed contributor-mode tests (feature removed)
- test/touchfiles.test.ts: updated test refs from contributor-mode to session-awareness
- Regenerated all SKILL.md files from merged templates

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

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

@@ -1162,6 +1162,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 ---
@@ -1802,11 +1934,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', () => {

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

@@ -735,45 +735,8 @@ describe('investigate skill structure', () => {
   }
 });
 
-// --- Contributor mode preamble structure validation ---
-
-describe('Contributor mode preamble structure', () => {
-  const skillsWithPreamble = [
-    'SKILL.md', 'browse/SKILL.md', 'qa/SKILL.md',
-    'qa-only/SKILL.md',
-    'setup-browser-cookies/SKILL.md',
-    'ship/SKILL.md', 'review/SKILL.md',
-    'plan-ceo-review/SKILL.md', 'plan-eng-review/SKILL.md',
-    'retro/SKILL.md',
-    'plan-design-review/SKILL.md',
-    'design-review/SKILL.md',
-    'design-consultation/SKILL.md',
-    'document-release/SKILL.md',
-    'canary/SKILL.md',
-    'benchmark/SKILL.md',
-    'land-and-deploy/SKILL.md',
-    'setup-deploy/SKILL.md',
-  ];
-
-  for (const skill of skillsWithPreamble) {
-    test(`${skill} has 0-10 rating in contributor mode`, () => {
-      const content = fs.readFileSync(path.join(ROOT, skill), 'utf-8');
-      expect(content).toContain('0-10');
-      expect(content).toContain('Rating');
-    });
-
-    test(`${skill} has "what would make this a 10" field`, () => {
-      const content = fs.readFileSync(path.join(ROOT, skill), 'utf-8');
-      expect(content).toContain('What would make this a 10');
-    });
-
-    test(`${skill} uses periodic reflection (not per-command)`, () => {
-      const content = fs.readFileSync(path.join(ROOT, skill), 'utf-8');
-      expect(content).toContain('workflow step');
-      expect(content).not.toContain('After you use gstack-provided CLIs');
-    });
-  }
-});
+// Contributor mode was removed in v0.13.10.0 — replaced by operational self-improvement.
+// Tests for contributor mode preamble structure are no longer applicable.
 
 describe('Enum & Value Completeness in review checklist', () => {
   const checklist = fs.readFileSync(path.join(ROOT, 'review', 'checklist.md'), 'utf-8');
@@ -1409,13 +1372,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);
     });
   }
 });

test/touchfiles.test.ts

@@ -101,7 +101,7 @@ describe('selectTests', () => {
     expect(result.reason).toBe('diff');
     // Should include tests that depend on gen-skill-docs.ts
     expect(result.selected).toContain('skillmd-setup-discovery');
-    expect(result.selected).toContain('contributor-mode');
+    expect(result.selected).toContain('session-awareness');
     expect(result.selected).toContain('journey-ideation');
     // Should NOT include tests that don't depend on it
     expect(result.selected).not.toContain('retro');
@@ -144,7 +144,7 @@ describe('selectTests', () => {
     const result = selectTests(['SKILL.md.tmpl'], E2E_TOUCHFILES);
     // Should select the 7 tests that depend on root SKILL.md
     expect(result.selected).toContain('skillmd-setup-discovery');
-    expect(result.selected).toContain('contributor-mode');
+    expect(result.selected).toContain('session-awareness');
     expect(result.selected).toContain('session-awareness');
     // Also selects journey routing tests (SKILL.md.tmpl in their touchfiles)
     expect(result.selected).toContain('journey-ideation');