From 26773bd6698d753cd16f4e8a8b6219f55d32bddc Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Wed, 20 May 2026 23:10:09 -0400 Subject: [PATCH] fix(3735): add surface to PROFILES.core (restore ADR-0011 expand contract) (#3744) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * test(3735): add failing test that PROFILES.core includes surface Regression test asserting that resolveProfile({ modes: ['core'] }) includes 'surface' in its transitive closure — the ADR-0011 contract that --profile=core users can expand via /gsd:surface enable . Also updates stale hardcoded skill-count assertions (7→8) across install-minimal*.test.cjs and install-profiles-resolve.test.cjs to reflect the correct post-fix baseline. Co-Authored-By: Claude Sonnet 4.6 * fix(3735): add 'surface' to PROFILES.core to restore ADR-0011 expand contract PROFILES.core omitted 'surface', silently breaking the documented contract in ADR-0011 that --profile=core users can expand their skill surface via /gsd:surface enable . The sub-command is only available if surface.md is staged — which requires it to appear in the resolved set for the core profile. Added 'surface' to both PROFILES.core and PROFILES.standard (standard is a documented superset of core; omitting it from standard would break the "standard must include all core skills" invariant and the resolveProfile tests). The MINIMAL_SKILL_ALLOWLIST back-compat shim is derived from PROFILES.core so it picks up surface automatically, ensuring --minimal and --core-only installs also stage surface.md. Co-Authored-By: Claude Sonnet 4.6 * chore(3735): update changeset to reference PR #3744 The fix commit landed with the linked-issue number as the pr: field placeholder. Updating to the actual PR number now that the PR is open. * docs(install-profiles): fix stale 'six skills' count in module header comment After #3735 added surface to PROFILES.core the skill count became eight, but the module-level comment still said "six skills covering the main project loop". Update the description to reflect the correct count and reference the ADR-0011 expand contract that surface fulfils. Co-Authored-By: Claude Sonnet 4.6 --------- Co-authored-by: Claude Sonnet 4.6 --- .changeset/kind-tunas-gather.md | 5 ++ get-shit-done/bin/lib/install-profiles.cjs | 4 +- ...35-profiles-core-includes-surface.test.cjs | 49 +++++++++++++++++++ tests/install-minimal-hooks.test.cjs | 18 +++---- ...-artifact-layout-install-profiles.test.cjs | 8 +-- 5 files changed, 71 insertions(+), 13 deletions(-) create mode 100644 .changeset/kind-tunas-gather.md create mode 100644 tests/bug-3735-profiles-core-includes-surface.test.cjs diff --git a/.changeset/kind-tunas-gather.md b/.changeset/kind-tunas-gather.md new file mode 100644 index 000000000..8142cd17f --- /dev/null +++ b/.changeset/kind-tunas-gather.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 3744 +--- +`gsd:surface` is now part of the `core` install profile — the ADR-0011 `core → expand via /gsd:surface enable` contract was silently broken. diff --git a/get-shit-done/bin/lib/install-profiles.cjs b/get-shit-done/bin/lib/install-profiles.cjs index 8e6035c57..24769c786 100644 --- a/get-shit-done/bin/lib/install-profiles.cjs +++ b/get-shit-done/bin/lib/install-profiles.cjs @@ -9,7 +9,7 @@ * dropped skills when users stack multiple plugins (#3408). * * Profile model: three named profiles replace the old minimal/full binary: - * - core — six skills covering the main project loop + * - core — eight skills covering the main project loop (includes surface for ADR-0011 expand contract) * - standard — core + phase management and workspace skills * - full — all skills (previous default, '*' sentinel) * Profiles compose: --profile=core,audit resolves to union(closure(core), closure(audit)). @@ -64,6 +64,7 @@ const PROFILES = Object.freeze({ 'phase', 'help', 'update', + 'surface', ]), standard: Object.freeze([ // Core loop @@ -73,6 +74,7 @@ const PROFILES = Object.freeze({ 'execute-phase', 'help', 'update', + 'surface', // Phase management (hot nodes from audit — required by 38+ skills) 'phase', 'review', diff --git a/tests/bug-3735-profiles-core-includes-surface.test.cjs b/tests/bug-3735-profiles-core-includes-surface.test.cjs new file mode 100644 index 000000000..6f53c87a4 --- /dev/null +++ b/tests/bug-3735-profiles-core-includes-surface.test.cjs @@ -0,0 +1,49 @@ +'use strict'; +/** + * Regression test for #3735: PROFILES.core must include 'surface' in its + * resolved closure so that --profile=core users can expand via + * /gsd:surface enable — the advertised use-case from ADR-0011. + * + * Stage 2 (RED): This test must fail before the fix is applied. + * Stage 3 (GREEN): This test must pass after 'surface' is added to PROFILES.core. + */ + +const { test, describe } = require('node:test'); +const assert = require('node:assert/strict'); +const path = require('path'); + +const { + PROFILES, + resolveProfile, + loadSkillsManifest, +} = require('../get-shit-done/bin/lib/install-profiles.cjs'); + +const REAL_COMMANDS_DIR = path.join(__dirname, '..', 'commands', 'gsd'); + +describe('PROFILES.core — ADR-0011 expand contract', () => { + test("PROFILES.core includes 'surface' so users can expand via /gsd:surface enable", () => { + const manifest = loadSkillsManifest(REAL_COMMANDS_DIR); + const result = resolveProfile({ modes: ['core'], manifest }); + + assert.ok(result.skills instanceof Set, + 'resolveProfile must return a skills Set for core profile'); + + // The primary assertion: surface must be in the resolved closure. + // ADR-0011 documents that --profile=core users expand via /gsd:surface enable . + // That sub-command is only available if surface.md is staged — which requires it to be + // in the resolved set for the core profile. + assert.ok(result.skills.has('surface'), + `PROFILES.core resolved closure must include 'surface'; got: [${[...result.skills].sort().join(', ')}]`); + }); + + // Counter-test: 'forensics' is NOT in core — proves the assertion above is selective, + // not vacuously true for all skills. + test("PROFILES.core does NOT include 'forensics' (selective assertion counter-check)", () => { + const manifest = loadSkillsManifest(REAL_COMMANDS_DIR); + const result = resolveProfile({ modes: ['core'], manifest }); + + assert.ok(result.skills instanceof Set); + assert.ok(!result.skills.has('forensics'), + `'forensics' should NOT be in core closure — it is a specialist skill, not a core loop skill`); + }); +}); diff --git a/tests/install-minimal-hooks.test.cjs b/tests/install-minimal-hooks.test.cjs index 729a5f4e0..f35265e13 100644 --- a/tests/install-minimal-hooks.test.cjs +++ b/tests/install-minimal-hooks.test.cjs @@ -69,7 +69,7 @@ describe('install-profiles: MINIMAL_SKILL_ALLOWLIST', () => { test('contains exactly the main-loop core (frozen)', () => { assert.deepStrictEqual( [...MINIMAL_SKILL_ALLOWLIST].sort(), - ['discuss-phase', 'execute-phase', 'help', 'new-project', 'phase', 'plan-phase', 'update'], + ['discuss-phase', 'execute-phase', 'help', 'new-project', 'phase', 'plan-phase', 'surface', 'update'], ); assert.ok(Object.isFrozen(MINIMAL_SKILL_ALLOWLIST)); }); @@ -128,7 +128,7 @@ describe('install-profiles: stageSkillsForMode', () => { function createFixtureSkillsDir() { const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-stage-fixture-')); for (const name of ['plan-phase', 'execute-phase', 'autonomous', 'do', 'help', - 'new-project', 'phase', 'discuss-phase', 'update', 'progress']) { + 'new-project', 'phase', 'discuss-phase', 'update', 'progress', 'surface']) { fs.writeFileSync(path.join(tmp, `${name}.md`), `# ${name}\n`); } return tmp; @@ -152,7 +152,7 @@ describe('install-profiles: stageSkillsForMode', () => { assert.deepStrictEqual( fs.readdirSync(staged).sort(), ['discuss-phase.md', 'execute-phase.md', 'help.md', 'new-project.md', - 'phase.md', 'plan-phase.md', 'update.md'], + 'phase.md', 'plan-phase.md', 'surface.md', 'update.md'], ); } finally { fs.rmSync(src, { recursive: true, force: true }); @@ -395,23 +395,23 @@ describe('install: manifest records mode for both profiles', () => { assert.ok(r.agentCount > 0); }); - test('--minimal records mode: "minimal" with exactly 7 skills and 0 agents', () => { + test('--minimal records mode: "minimal" with exactly 8 skills and 0 agents', () => { const r = manifestModeAfterInstall(['--minimal']); assert.strictEqual(r.mode, 'minimal'); - assert.strictEqual(r.skillCount, 7); + assert.strictEqual(r.skillCount, 8); assert.strictEqual(r.agentCount, 0); }); test('--core-only is an alias for --minimal', () => { const r = manifestModeAfterInstall(['--core-only']); assert.strictEqual(r.mode, 'minimal'); - assert.strictEqual(r.skillCount, 7); + assert.strictEqual(r.skillCount, 8); assert.strictEqual(r.agentCount, 0); }); }); describe('install-minimal-backcompat: PROFILES.core matches MINIMAL_SKILL_ALLOWLIST', () => { - test('PROFILES.core contains the same 7 skills as MINIMAL_SKILL_ALLOWLIST', () => { + test('PROFILES.core contains the same 8 skills as MINIMAL_SKILL_ALLOWLIST', () => { assert.deepStrictEqual( [...PROFILES.core].sort(), [...MINIMAL_SKILL_ALLOWLIST].sort(), @@ -442,10 +442,10 @@ describe('install-minimal-backcompat: --minimal and --profile=core produce same } } - test('--minimal produces mode "minimal" with exactly 7 skills', () => { + test('--minimal produces mode "minimal" with exactly 8 skills', () => { const r = installAndGetManifest(['--minimal']); assert.strictEqual(r.mode, 'minimal'); - assert.strictEqual(r.skillCount, 7); + assert.strictEqual(r.skillCount, 8); }); test('--minimal writes .gsd-profile marker "core"', () => { diff --git a/tests/runtime-artifact-layout-install-profiles.test.cjs b/tests/runtime-artifact-layout-install-profiles.test.cjs index b547feea4..42970b686 100644 --- a/tests/runtime-artifact-layout-install-profiles.test.cjs +++ b/tests/runtime-artifact-layout-install-profiles.test.cjs @@ -313,7 +313,7 @@ describe('PROFILES map', () => { assert.ok('full' in PROFILES, 'PROFILES.full missing'); }); - test('PROFILES.core contains the 7 main-loop skills (including phase)', () => { + test('PROFILES.core contains the 8 main-loop skills (including phase and surface)', () => { const core = PROFILES.core; assert.ok(Array.isArray(core), 'core should be an array'); const sorted = [...core].sort(); @@ -324,6 +324,7 @@ describe('PROFILES map', () => { 'new-project', 'phase', 'plan-phase', + 'surface', 'update', ]); }); @@ -354,12 +355,13 @@ describe('resolveProfile', () => { assert.strictEqual(result.skills, '*'); }); - test('resolves core profile — returns 7+ skills, all base stems present', () => { + test('resolves core profile — returns 8+ skills, all base stems present', () => { const manifest = loadSkillsManifest(REAL_COMMANDS_DIR); const result = resolveProfile({ modes: ['core'], manifest }); assert.strictEqual(result.name, 'core'); assert.ok(result.skills instanceof Set, 'skills should be a Set'); - assert.ok(result.skills.size >= 7, `core closure should have >=7 skills, got ${result.skills.size}`); + // core has 8 base skills (includes surface as of #3735). + assert.ok(result.skills.size >= 8, `core closure should have >=8 skills, got ${result.skills.size}`); for (const s of PROFILES.core) { assert.ok(result.skills.has(s), `core closure should include ${s}`); }