diff --git a/.changeset/brave-geese-sing.md b/.changeset/brave-geese-sing.md new file mode 100644 index 000000000..bd16c3fef --- /dev/null +++ b/.changeset/brave-geese-sing.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 4618 +--- +**`validate.health` no longer flags `.planning/PATTERNS.md` as an unrecognized file.** The graduation workflow (`/gsd-extract-learnings`) writes this file on gsd-core's own instruction, but the artifact registry was never updated to recognize it -- every repo that had run the graduation scan sat permanently at `status: degraded`. (#4282) diff --git a/gsd-core/templates/README.md b/gsd-core/templates/README.md index 0968235b6..b7168c370 100644 --- a/gsd-core/templates/README.md +++ b/gsd-core/templates/README.md @@ -23,6 +23,12 @@ These files live directly at `.planning/` — not inside phase subdirectories. | `config.json` | `config.json` | `/gsd:new-project`, `/gsd:health --repair` | Project-specific GSD configuration | | `CLAUDE.md` | *(inline)* | `/gsd-profile` | Auto-assembled Claude Code context file | | `RETROSPECTIVE.md` | *(inline)* | `/gsd:complete-milestone` | Living milestone retrospective updated at each milestone close | +| `WINDOWS.md` | *(none)* | broken-windows ledger (`src/broken-windows.cts`) | Tracked known-broken items pending resolution (#3224) | +| `STATE-ARCHIVE.md` | *(none)* | `state.cts`'s `cmdStatePrune` | Pruned historical STATE.md entries | +| `milestone.lock` | *(none)* | `src/milestone-lock.cts` | Persistent milestone (phase + session) claim, unlike the transient `STATE.md.lock`/`WAITING.json` (#3311) | +| `state.json` | *(none)* | `src/state-contract.cts` | Machine-readable state contract published at step boundaries (#3227) | +| `skill-manifest.json` | *(none)* | `init.cts`'s `cmdSkillManifest --write` | Project-scoped skill manifest (#3964) | +| `PATTERNS.md` | *(inline)* | `/gsd:extract-learnings` (graduation, `workflows/graduation.md`, `patterns` target) | Graduated cross-phase patterns -- distinct from the per-phase `NN-PATTERNS.md` below (#4282) | ### Version-stamped artifacts (pattern: `vX.Y-*.md`) diff --git a/scripts/lint-vendored-deps.cjs b/scripts/lint-vendored-deps.cjs index 7aa73260c..3307aa083 100644 --- a/scripts/lint-vendored-deps.cjs +++ b/scripts/lint-vendored-deps.cjs @@ -233,13 +233,17 @@ function checkHandAuthoredTwin(row) { * installed version. Shared by checkRow (compares) and fixRow (rewrites) so * the two can never silently diverge on how a pin is read. * @param {VendoredPackage} row + * @param {string} [pkgRoot] Override for testing -- defaults to the real repo ROOT. Lets a + * test point fixRow's pin-rewrite at an isolated temp package.json instead of writing the + * real, shared one, which other concurrently-running node --test files read at module + * top-level (the same race class already fixed for the vendored .cjs copy). * @returns {{pinnedSpec: string | undefined, installedVersion: string | undefined}} */ -function readPinState(row) { - const pkgPath = path.join(ROOT, 'package.json'); +function readPinState(row, pkgRoot = ROOT) { + const pkgPath = path.join(pkgRoot, 'package.json'); const pkg = JSON.parse(fs.readFileSync(pkgPath, 'utf8')); const pinnedSpec = pkg.devDependencies && pkg.devDependencies[row.name]; - const installedPkgPath = path.join(ROOT, 'node_modules', row.name, 'package.json'); + const installedPkgPath = path.join(pkgRoot, 'node_modules', row.name, 'package.json'); let installedVersion; if (fs.existsSync(installedPkgPath)) { installedVersion = JSON.parse(fs.readFileSync(installedPkgPath, 'utf8')).version; @@ -250,9 +254,13 @@ function readPinState(row) { /** * Run all applicable freshness checks for one vendored package row. * @param {VendoredPackage} row + * @param {string} [pkgRoot] Override for testing -- defaults to the real repo ROOT. Lets a + * test point fixRow's pin-rewrite at an isolated temp package.json instead of writing the + * real, shared one, which other concurrently-running node --test files read at module + * top-level (the same race class already fixed for the vendored .cjs copy). * @returns {string[]} findings (empty when the row is fresh) */ -function checkRow(row) { +function checkRow(row, pkgRoot = ROOT) { const findings = []; const cjsDrift = compareFiles(row.vendoredCjs, row.upstreamCjs); @@ -271,7 +279,7 @@ function checkRow(row) { findings.push(...checkHandAuthoredTwin(row)); } - const { pinnedSpec, installedVersion } = readPinState(row); + const { pinnedSpec, installedVersion } = readPinState(row, pkgRoot); if (!pinnedSpec) { findings.push(`package.json devDependencies.${row.name} is missing`); } else if (installedVersion === undefined) { @@ -303,9 +311,13 @@ function checkRow(row) { * after this runs, that is by design: the caller must not treat it as * fixed. * @param {VendoredPackage} row + * @param {string} [pkgRoot] Override for testing -- defaults to the real repo ROOT. Lets a + * test point fixRow's pin-rewrite at an isolated temp package.json instead of writing the + * real, shared one, which other concurrently-running node --test files read at module + * top-level (the same race class already fixed for the vendored .cjs copy). * @returns {string[]} findings remaining after the fix (empty when fully resolved) */ -function fixRow(row) { +function fixRow(row, pkgRoot = ROOT) { fs.copyFileSync(resolvePath(row.upstreamCjs), resolvePath(row.vendoredCjs)); if (row.twinKind === 'upstream-verbatim' && row.upstreamDts) { @@ -313,18 +325,18 @@ function fixRow(row) { if (row.srcTwin) fs.copyFileSync(resolvePath(row.upstreamDts), resolvePath(row.srcTwin)); } - const { pinnedSpec, installedVersion } = readPinState(row); + const { pinnedSpec, installedVersion } = readPinState(row, pkgRoot); if (pinnedSpec && installedVersion !== undefined) { const newPin = `${pinOperatorPrefix(pinnedSpec)}${installedVersion}`; if (newPin !== pinnedSpec) { - const pkgPath = path.join(ROOT, 'package.json'); + const pkgPath = path.join(pkgRoot, 'package.json'); const pkg = JSON.parse(fs.readFileSync(pkgPath, 'utf8')); pkg.devDependencies[row.name] = newPin; fs.writeFileSync(pkgPath, `${JSON.stringify(pkg, null, 2)}\n`); } } - return checkRow(row); + return checkRow(row, pkgRoot); } function main() { diff --git a/src/artifacts.cts b/src/artifacts.cts index 0bcfc9d59..4a17a0139 100644 --- a/src/artifacts.cts +++ b/src/artifacts.cts @@ -28,7 +28,8 @@ export const CANONICAL_EXACT: ReadonlySet = new Set([ 'STATE-ARCHIVE.md', // state.cts's cmdStatePrune writes this at the .planning/ root 'milestone.lock', // #3311: milestone (phase + session) claim (src/milestone-lock.cts); persistent, unlike the transient STATE.md.lock/WAITING.json 'state.json', // #3227: machine-readable state contract published at step boundaries (src/state-contract.cts) - 'skill-manifest.json', // init.cts routeSkillManifest --write (project-scoped planning root, #3964) + 'skill-manifest.json', // init.cts cmdSkillManifest --write (project-scoped planning root, #3964) + 'PATTERNS.md', // #4282: graduated cross-phase patterns (workflows/graduation.md, `patterns` target) -- distinct from the per-phase NN-PATTERNS.md (templates/README.md) ]); // Pattern-match canonical file names (regex tests on the basename) diff --git a/tests/artifacts.test.cjs b/tests/artifacts.test.cjs index 05cd0f25d..4ca187194 100644 --- a/tests/artifacts.test.cjs +++ b/tests/artifacts.test.cjs @@ -76,6 +76,17 @@ describe('isCanonicalPlanningFile', () => { assert.strictEqual(isCanonicalPlanningFile('WINDOWS.md'), true); }); + test('#4282: PATTERNS.md (graduated cross-phase patterns) is a canonical .planning/ artifact', () => { + // workflows/graduation.md's own graduation-target table instructs the agent to + // append to .planning/PATTERNS.md for the `patterns` category. Before #4282 it + // was absent from the registry, so validate health flagged it W019 + // "Unrecognized" with advice to archive/delete a file gsd-core itself produces. + // Distinct from the per-phase NN-PATTERNS.md (templates/README.md), which lives + // under a phase subdirectory and was never subject to W019 in the first place. + assert.ok(CANONICAL_EXACT.has('PATTERNS.md'), 'PATTERNS.md must be in CANONICAL_EXACT'); + assert.strictEqual(isCanonicalPlanningFile('PATTERNS.md'), true); + }); + test('returns false for unrecognized file', () => { assert.strictEqual(isCanonicalPlanningFile('random-file.md'), false); }); diff --git a/tests/lint-vendored-deps-manifest.test.cjs b/tests/lint-vendored-deps-manifest.test.cjs index 5dd7b4540..aa622bc9c 100644 --- a/tests/lint-vendored-deps-manifest.test.cjs +++ b/tests/lint-vendored-deps-manifest.test.cjs @@ -22,6 +22,7 @@ const assert = require('node:assert/strict'); const fs = require('node:fs'); const os = require('node:os'); const path = require('node:path'); +const { cleanup } = require('./helpers.cjs'); const { VENDORED, @@ -287,30 +288,46 @@ describe('#4573: pinOperatorPrefix / fixRow — mechanical --fix for Dependabot- }); test('fixRow resolves mechanical .cjs drift: mutated vendored js-yaml.cjs is byte-restored to match node_modules', (t) => { + // Operates on an ISOLATED TEMP COPY, never the real gsd-core/bin/lib/vendor/js-yaml.cjs. + // node --test runs files concurrently; the real file is `require()`-able by other test + // files at any moment, and fs.copyFileSync's write is not atomic against a concurrent + // reader on every filesystem. A prior version of this test wrote directly to the real + // file and a concurrent require() elsewhere caught it mid-overwrite, reading a truncated + // file and crashing an unrelated test with a SyntaxError -- confirmed via real CI logs, + // not a hypothetical. upstreamCjs stays pointed at the real node_modules copy (read-only, + // nothing writes to node_modules during tests, so sharing it is safe); only the + // destination is redirected to a private temp path. const row = jsYamlRow(); - const vendoredAbs = path.join(REPO_ROOT, row.vendoredCjs); const upstreamAbs = path.join(REPO_ROOT, row.upstreamCjs); - const original = fs.readFileSync(vendoredAbs, 'utf8'); - fs.writeFileSync(vendoredAbs, `${original}\n// mutated for test\n`); - t.after(() => { - // Safety net: fixRow copies FROM upstream, so the vendored file should - // already be back in its original clean state — but re-copy from - // upstream regardless in case an assertion above threw before fixRow - // completed, so this test never leaves the tree dirty. - fs.copyFileSync(upstreamAbs, vendoredAbs); - }); + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'lint-vendored-deps-fixrow-')); + const tempVendoredAbs = path.join(tmpDir, 'js-yaml.cjs'); + t.after(() => cleanup(tmpDir)); - const findings = fixRow(row); + const original = fs.readFileSync(upstreamAbs, 'utf8'); + fs.writeFileSync(tempVendoredAbs, `${original}\n// mutated for test\n`); + const tempRow = { ...row, vendoredCjs: tempVendoredAbs }; + + const findings = fixRow(tempRow); assert.deepEqual(findings, [], `expected fixRow to leave zero findings, got: ${JSON.stringify(findings)}`); assert.ok( - fs.readFileSync(vendoredAbs).equals(fs.readFileSync(upstreamAbs)), + fs.readFileSync(tempVendoredAbs).equals(fs.readFileSync(upstreamAbs)), 'expected the vendored .cjs to byte-equal node_modules/js-yaml/dist/js-yaml.js after fixRow', ); }); test('fixRow does NOT mask a genuine hand-authored-twin incompatibility: a fake declared export still surfaces after --fix', (t) => { + // fixRow's first line unconditionally copies onto row.vendoredCjs regardless of what this + // test is exercising -- redirect it to a private temp path too, same reasoning as the + // preceding test (avoid ANY write to the real, shared, concurrently-`require()`-able + // gsd-core/bin/lib/vendor/js-yaml.cjs). srcTwin (a .d.cts type-only file, never require()'d + // at runtime) is mutated in place as before -- this test's actual subject. const row = jsYamlRow(); const srcTwinAbs = path.join(REPO_ROOT, row.srcTwin); + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'lint-vendored-deps-fixrow-')); + const tempVendoredAbs = path.join(tmpDir, 'js-yaml.cjs'); + t.after(() => cleanup(tmpDir)); + const tempRow = { ...row, vendoredCjs: tempVendoredAbs }; + const original = fs.readFileSync(srcTwinAbs, 'utf8'); fs.writeFileSync( srcTwinAbs, @@ -320,7 +337,7 @@ describe('#4573: pinOperatorPrefix / fixRow — mechanical --fix for Dependabot- fs.writeFileSync(srcTwinAbs, original); }); - const findings = fixRow(row); + const findings = fixRow(tempRow); assert.ok( findings.some((f) => f.includes('thisFixRowExportDoesNotExistAtRuntime')), `expected the hand-authored-twin finding to survive fixRow, got: ${JSON.stringify(findings)}`, @@ -328,25 +345,39 @@ describe('#4573: pinOperatorPrefix / fixRow — mechanical --fix for Dependabot- }); test('fixRow preserves the pin\'s original range-operator style when rewriting package.json', (t) => { + // Isolates BOTH real-file writes fixRow makes: the vendoredCjs copy (redirected to a temp + // path, same reasoning as the two tests above) AND the package.json pin rewrite this test + // specifically exercises (redirected via fixRow's pkgRoot parameter to an isolated temp + // root containing its own package.json + node_modules/js-yaml/package.json). Neither the + // real vendored .cjs nor the real package.json is touched -- both are readable at module + // top-level by other concurrently-running node --test files, the same race class already + // fixed for the vendored .cjs. const row = jsYamlRow(); - const pkgPath = path.join(REPO_ROOT, 'package.json'); - const installedPkgPath = path.join(REPO_ROOT, 'node_modules', 'js-yaml', 'package.json'); - const installedVersion = JSON.parse(fs.readFileSync(installedPkgPath, 'utf8')).version; + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'lint-vendored-deps-fixrow-')); + t.after(() => cleanup(tmpDir)); + const tempVendoredAbs = path.join(tmpDir, 'js-yaml.cjs'); + const tempRow = { ...row, vendoredCjs: tempVendoredAbs }; - const originalContent = fs.readFileSync(pkgPath, 'utf8'); - t.after(() => { - fs.writeFileSync(pkgPath, originalContent); - }); + const realInstalledPkgPath = path.join(REPO_ROOT, 'node_modules', 'js-yaml', 'package.json'); + const installedVersion = JSON.parse(fs.readFileSync(realInstalledPkgPath, 'utf8')).version; - const pkg = JSON.parse(originalContent); + const tempPkgRoot = path.join(tmpDir, 'pkgroot'); + const tempNodeModulesJsYamlDir = path.join(tempPkgRoot, 'node_modules', 'js-yaml'); + fs.mkdirSync(tempNodeModulesJsYamlDir, { recursive: true }); const stalePin = installedVersion === '4.0.0' ? '~4.0.1' : '~4.0.0'; - pkg.devDependencies['js-yaml'] = stalePin; - fs.writeFileSync(pkgPath, `${JSON.stringify(pkg, null, 2)}\n`); + fs.writeFileSync( + path.join(tempPkgRoot, 'package.json'), + `${JSON.stringify({ devDependencies: { 'js-yaml': stalePin } }, null, 2)}\n`, + ); + fs.writeFileSync( + path.join(tempNodeModulesJsYamlDir, 'package.json'), + `${JSON.stringify({ name: 'js-yaml', version: installedVersion }, null, 2)}\n`, + ); - const findings = fixRow(row); + const findings = fixRow(tempRow, tempPkgRoot); assert.deepEqual(findings, [], `expected fixRow to leave zero findings, got: ${JSON.stringify(findings)}`); - const after = JSON.parse(fs.readFileSync(pkgPath, 'utf8')); + const after = JSON.parse(fs.readFileSync(path.join(tempPkgRoot, 'package.json'), 'utf8')); const pinnedAfter = after.devDependencies['js-yaml']; assert.equal( pinnedAfter,