test: add name patching + ship idempotency tests (#620, #649)

- 4 unit tests for name: patching in relink.test.ts (prefix on/off, gstack-upgrade not double-prefixed, no-frontmatter no-op) - 2 tests for gen-skill-docs prefix warning - 1 E2E test for ship idempotency (periodic tier) - Updated setupMockInstall to write SKILL.md with proper frontmatter - Added ship-idempotency touchfiles + tier classification Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-05-02 11:45:20 +02:00 · 2026-03-30 19:38:59 -07:00
parent cbced97d72
commit 238942b483
4 changed files with 224 additions and 2 deletions
@@ -2462,3 +2462,49 @@ describe('CONFIDENCE_CALIBRATION resolver', () => {
    }
  });
 });
+
+describe('gen-skill-docs prefix warning (#620/#578)', () => {
+  const { execSync } = require('child_process');
+
+  test('warns about skill_prefix when config has prefix=true', () => {
+    const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-prefix-warn-'));
+    try {
+      // Create a fake ~/.gstack/config.yaml with skill_prefix: true
+      const fakeHome = tmpDir;
+      const fakeGstack = path.join(fakeHome, '.gstack');
+      fs.mkdirSync(fakeGstack, { recursive: true });
+      fs.writeFileSync(path.join(fakeGstack, 'config.yaml'), 'skill_prefix: true\n');
+
+      const output = execSync('bun run scripts/gen-skill-docs.ts', {
+        cwd: ROOT,
+        env: { ...process.env, HOME: fakeHome },
+        encoding: 'utf-8',
+        timeout: 30000,
+      });
+      expect(output).toContain('skill_prefix is true');
+      expect(output).toContain('gstack-relink');
+    } finally {
+      fs.rmSync(tmpDir, { recursive: true, force: true });
+    }
+  });
+
+  test('no warning when skill_prefix is false or absent', () => {
+    const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-prefix-warn-'));
+    try {
+      const fakeHome = tmpDir;
+      const fakeGstack = path.join(fakeHome, '.gstack');
+      fs.mkdirSync(fakeGstack, { recursive: true });
+      fs.writeFileSync(path.join(fakeGstack, 'config.yaml'), 'skill_prefix: false\n');
+
+      const output = execSync('bun run scripts/gen-skill-docs.ts', {
+        cwd: ROOT,
+        env: { ...process.env, HOME: fakeHome },
+        encoding: 'utf-8',
+        timeout: 30000,
+      });
+      expect(output).not.toContain('skill_prefix is true');
+    } finally {
+      fs.rmSync(tmpDir, { recursive: true, force: true });
+    }
+  });
+});
@@ -82,6 +82,7 @@ export const E2E_TOUCHFILES: Record<string, string[]> = {
  'review-dashboard-via': ['ship/**', 'scripts/resolvers/review.ts', 'codex/**', 'autoplan/**', 'land-and-deploy/**'],
  'ship-plan-completion': ['ship/**', 'scripts/gen-skill-docs.ts'],
  'ship-plan-verification': ['ship/**', 'scripts/gen-skill-docs.ts'],
+  'ship-idempotency':      ['ship/**', 'scripts/resolvers/utility.ts'],

  // Retro
  'retro':             ['retro/**'],
@@ -122,6 +123,7 @@ export const E2E_TOUCHFILES: Record<string, string[]> = {
  // Plan completion audit + verification
  'ship-plan-completion': ['ship/**', 'scripts/gen-skill-docs.ts'],
  'ship-plan-verification': ['ship/**', 'qa-only/**', 'scripts/gen-skill-docs.ts'],
+  'ship-idempotency':       ['ship/**', 'scripts/resolvers/utility.ts'],
  'review-plan-completion': ['review/**', 'scripts/gen-skill-docs.ts'],

  // Design
@@ -228,6 +230,7 @@ export const E2E_TIERS: Record<string, 'gate' | 'periodic'> = {
  'ship-triage': 'gate',
  'ship-plan-completion': 'gate',
  'ship-plan-verification': 'gate',
+  'ship-idempotency': 'periodic',

  // Retro — gate for cheap branch detection, periodic for full Opus retro
  'retro': 'periodic',
@@ -42,11 +42,18 @@ function setupMockInstall(skills: string[]): void {
    fs.copyFileSync(path.join(BIN, 'gstack-relink'), path.join(mockBin, 'gstack-relink'));
    fs.chmodSync(path.join(mockBin, 'gstack-relink'), 0o755);
  }
+  if (fs.existsSync(path.join(BIN, 'gstack-patch-names'))) {
+    fs.copyFileSync(path.join(BIN, 'gstack-patch-names'), path.join(mockBin, 'gstack-patch-names'));
+    fs.chmodSync(path.join(mockBin, 'gstack-patch-names'), 0o755);
+  }

-  // Create mock skill directories
+  // Create mock skill directories with proper frontmatter
  for (const skill of skills) {
    fs.mkdirSync(path.join(installDir, skill), { recursive: true });
-    fs.writeFileSync(path.join(installDir, skill, 'SKILL.md'), `# ${skill}`);
+    fs.writeFileSync(
+      path.join(installDir, skill, 'SKILL.md'),
+      `---\nname: ${skill}\ndescription: test\n---\n# ${skill}`
+    );
  }
 }

@@ -150,3 +157,73 @@ describe('gstack-relink (#578)', () => {
    expect(fs.existsSync(path.join(skillsDir, 'gstack-ship'))).toBe(true);
  });
 });
+
+describe('gstack-patch-names (#620/#578)', () => {
+  // Helper to read name: from SKILL.md frontmatter
+  function readSkillName(skillDir: string): string | null {
+    const content = fs.readFileSync(path.join(skillDir, 'SKILL.md'), 'utf-8');
+    const match = content.match(/^name:\s*(.+)$/m);
+    return match ? match[1].trim() : null;
+  }
+
+  test('prefix=true patches name: field in SKILL.md', () => {
+    setupMockInstall(['qa', 'ship', 'review']);
+    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,
+    });
+    // Verify name: field is patched with gstack- prefix
+    expect(readSkillName(path.join(installDir, 'qa'))).toBe('gstack-qa');
+    expect(readSkillName(path.join(installDir, 'ship'))).toBe('gstack-ship');
+    expect(readSkillName(path.join(installDir, 'review'))).toBe('gstack-review');
+  });
+
+  test('prefix=false restores name: field in SKILL.md', () => {
+    setupMockInstall(['qa', 'ship']);
+    // First, prefix them
+    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(readSkillName(path.join(installDir, 'qa'))).toBe('gstack-qa');
+    // Now 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,
+    });
+    // Verify name: field is restored to unprefixed
+    expect(readSkillName(path.join(installDir, 'qa'))).toBe('qa');
+    expect(readSkillName(path.join(installDir, 'ship'))).toBe('ship');
+  });
+
+  test('gstack-upgrade name: not double-prefixed', () => {
+    setupMockInstall(['qa', '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(readSkillName(path.join(installDir, 'gstack-upgrade'))).toBe('gstack-upgrade');
+    // Regular skill should be prefixed
+    expect(readSkillName(path.join(installDir, 'qa'))).toBe('gstack-qa');
+  });
+
+  test('SKILL.md without frontmatter is a no-op', () => {
+    setupMockInstall(['qa']);
+    // Overwrite qa SKILL.md with no frontmatter
+    fs.writeFileSync(path.join(installDir, 'qa', 'SKILL.md'), '# qa\nSome content.');
+    run(`${path.join(installDir, 'bin', 'gstack-config')} set skill_prefix true`);
+    // Should not crash
+    run(`${path.join(installDir, 'bin', 'gstack-relink')}`, {
+      GSTACK_INSTALL_DIR: installDir,
+      GSTACK_SKILLS_DIR: skillsDir,
+    });
+    // Content should be unchanged (no name: to patch)
+    const content = fs.readFileSync(path.join(installDir, 'qa', 'SKILL.md'), 'utf-8');
+    expect(content).toBe('# qa\nSome content.');
+  });
+});
@@ -3313,6 +3313,102 @@ Write your summary to ${benefitsDir}/benefits-summary.md`,
  }, 180_000);
 });

+// --- Ship idempotency (#649) ---
+describeIfSelected('Ship idempotency', ['ship-idempotency'], () => {
+  let idempDir: string;
+  const gitRun = (args: string[], cwd: string) =>
+    spawnSync('git', args, { cwd, stdio: 'pipe', timeout: 5000 });
+
+  beforeAll(() => {
+    idempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'skill-e2e-ship-idemp-'));
+
+    // Create git repo with initial commit on main
+    gitRun(['init', '-b', 'main'], idempDir);
+    gitRun(['config', 'user.email', 'test@test.com'], idempDir);
+    gitRun(['config', 'user.name', 'Test'], idempDir);
+
+    fs.writeFileSync(path.join(idempDir, 'app.ts'), 'console.log("v1");\n');
+    fs.writeFileSync(path.join(idempDir, 'VERSION'), '0.1.0.0\n');
+    fs.writeFileSync(path.join(idempDir, 'CHANGELOG.md'), '# Changelog\n');
+    gitRun(['add', '.'], idempDir);
+    gitRun(['commit', '-m', 'initial'], idempDir);
+
+    // Create feature branch with changes
+    gitRun(['checkout', '-b', 'feat/my-feature'], idempDir);
+    fs.writeFileSync(path.join(idempDir, 'app.ts'), 'console.log("v2");\n');
+    gitRun(['add', 'app.ts'], idempDir);
+    gitRun(['commit', '-m', 'feat: update to v2'], idempDir);
+
+    // Simulate prior /ship run: bump VERSION and write CHANGELOG entry
+    fs.writeFileSync(path.join(idempDir, 'VERSION'), '0.2.0.0\n');
+    fs.writeFileSync(path.join(idempDir, 'CHANGELOG.md'),
+      '# Changelog\n\n## [0.2.0.0] — 2026-03-30\n\n- Updated app to v2\n');
+    gitRun(['add', 'VERSION', 'CHANGELOG.md'], idempDir);
+    gitRun(['commit', '-m', 'chore: bump version to 0.2.0.0'], idempDir);
+
+    // Extract just the idempotency-relevant sections from ship/SKILL.md
+    const full = fs.readFileSync(path.join(ROOT, 'ship', 'SKILL.md'), 'utf-8');
+    const step4Start = full.indexOf('## Step 4: Version bump');
+    const step4End = full.indexOf('\n---\n', step4Start);
+    const step7Start = full.indexOf('## Step 7: Push');
+    const step8End = full.indexOf('## Step 8.5');
+    const extracted = [
+      full.slice(step4Start, step4End > step4Start ? step4End : step4Start + 500),
+      full.slice(step7Start, step8End > step7Start ? step8End : step7Start + 500),
+    ].join('\n\n---\n\n');
+    fs.writeFileSync(path.join(idempDir, 'ship-steps.md'), extracted);
+  });
+
+  afterAll(() => {
+    try { fs.rmSync(idempDir, { recursive: true, force: true }); } catch {}
+  });
+
+  testIfSelected('ship-idempotency', async () => {
+    const result = await runSkillTest({
+      prompt: `You are in a git repo on branch feat/my-feature. A prior /ship run already:
+- Bumped VERSION from 0.1.0.0 to 0.2.0.0
+- Wrote a CHANGELOG entry for 0.2.0.0
+- But the push/PR step failed
+
+Read ship-steps.md for the idempotency check instructions from the ship workflow.
+
+Run ONLY the idempotency checks described in Steps 4 and 7. Do NOT actually push or create PRs (there is no remote).
+
+After running the checks, write a report to ${idempDir}/idemp-result.md containing:
+- Whether VERSION was detected as ALREADY_BUMPED or not
+- Whether the push was detected as ALREADY_PUSHED or PUSH_NEEDED
+- The current VERSION value (should still be 0.2.0.0)
+
+Do NOT modify VERSION or CHANGELOG. Only run the detection checks and report.`,
+      workingDirectory: idempDir,
+      maxTurns: 10,
+      timeout: 60_000,
+      testName: 'ship-idempotency',
+      runId,
+    });
+
+    logCost('/ship idempotency', result);
+    recordE2E('/ship idempotency guard', 'Ship idempotency', result);
+    expect(result.exitReason).toBe('success');
+
+    // Verify VERSION was NOT modified
+    const version = fs.readFileSync(path.join(idempDir, 'VERSION'), 'utf-8').trim();
+    expect(version).toBe('0.2.0.0');
+
+    // Verify CHANGELOG was NOT duplicated
+    const changelog = fs.readFileSync(path.join(idempDir, 'CHANGELOG.md'), 'utf-8');
+    const versionEntries = (changelog.match(/## \[0\.2\.0\.0\]/g) || []).length;
+    expect(versionEntries).toBe(1);
+
+    // Check the result report if it was written
+    const reportPath = path.join(idempDir, 'idemp-result.md');
+    if (fs.existsSync(reportPath)) {
+      const report = fs.readFileSync(reportPath, 'utf-8');
+      expect(report.toLowerCase()).toContain('already_bumped');
+    }
+  }, 120_000);
+});
+
 // Module-level afterAll — finalize eval collector after all tests complete
 afterAll(async () => {
  if (evalCollector) {