diff --git a/test/gen-skill-docs.test.ts b/test/gen-skill-docs.test.ts index 95f4bc9c..4f24daf3 100644 --- a/test/gen-skill-docs.test.ts +++ b/test/gen-skill-docs.test.ts @@ -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 }); + } + }); +}); diff --git a/test/helpers/touchfiles.ts b/test/helpers/touchfiles.ts index b475daad..182356aa 100644 --- a/test/helpers/touchfiles.ts +++ b/test/helpers/touchfiles.ts @@ -82,6 +82,7 @@ export const E2E_TOUCHFILES: Record = { '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 = { // 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 = { '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', diff --git a/test/relink.test.ts b/test/relink.test.ts index 39af8891..b368d2bf 100644 --- a/test/relink.test.ts +++ b/test/relink.test.ts @@ -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.'); + }); +}); diff --git a/test/skill-e2e.test.ts b/test/skill-e2e.test.ts index 91c95f7a..2f92d095 100644 --- a/test/skill-e2e.test.ts +++ b/test/skill-e2e.test.ts @@ -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) {