From 213a2fff63925c5775439caffaf68060d77c609e Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Sat, 29 Aug 2026 07:44:22 -0400 Subject: [PATCH] chore(#3813): delete the caller-less listMilestoneArchiveDirs seam; #1883 contract now pins the live path (#4029) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * test(#3813): pin the #1883 unreadable-milestones contract on the live planning-snapshot path * fix(#3813): delete the caller-less listMilestoneArchiveDirs seam; #1883 contract now pins the live path * chore(#3813): changeset fragment (pr number backfilled after PR creation) * chore(#3813): backfill changeset PR number (4029) * chore(#3813): docs-exempt marker — internal dead-code removal --------- Co-authored-by: sim --- .changeset/graceful-bears-caper.md | 6 ++ scripts/lint-test-file-count.allowlist.json | 3 +- src/verify.cts | 32 +-------- tests/verify-archive-dirs-live-path.test.cjs | 75 ++++++++++++++++++++ tests/verify.test.cjs | 59 --------------- 5 files changed, 84 insertions(+), 91 deletions(-) create mode 100644 .changeset/graceful-bears-caper.md create mode 100644 tests/verify-archive-dirs-live-path.test.cjs diff --git a/.changeset/graceful-bears-caper.md b/.changeset/graceful-bears-caper.md new file mode 100644 index 000000000..e43db0aaa --- /dev/null +++ b/.changeset/graceful-bears-caper.md @@ -0,0 +1,6 @@ +--- +type: Changed +pr: 4029 +--- +**Removed a dead code path** — `listMilestoneArchiveDirs` (caller-less since the Phase-12 snapshot migration) and its test seam are gone; the #1883 unreadable-milestones regression suite now pins the live planning-snapshot path. (#3813) + diff --git a/scripts/lint-test-file-count.allowlist.json b/scripts/lint-test-file-count.allowlist.json index bc3c880a5..cb907d7be 100644 --- a/scripts/lint-test-file-count.allowlist.json +++ b/scripts/lint-test-file-count.allowlist.json @@ -129,6 +129,7 @@ }, "verify": { "files": [ + "verify-archive-dirs-live-path.test.cjs", "verify-health.test.cjs", "verify-mvp-uat.test.cjs", "verify-npm-publish.test.cjs", @@ -136,7 +137,7 @@ "verify-work-auto-transition.test.cjs", "verify.test.cjs" ], - "issue": "3767", + "issue": "#3813 \u2014 the live-path #1883 suite drives buildPlanningSnapshot in-process with an fs fault; the verify module suites are CLI-driving and cannot host an fs monkeypatch", "justification": "verify-command-grounding added by #2401/#3678; allowlist follow-up landed with #3606 (base was red on this lane)." }, "install": { diff --git a/src/verify.cts b/src/verify.cts index 292fbc95d..12b4155cb 100644 --- a/src/verify.cts +++ b/src/verify.cts @@ -9,7 +9,7 @@ import fs from 'node:fs'; import path from 'node:path'; import os from 'node:os'; -import { MILESTONE_ARCHIVE_DIR_RE, textEncodingError } from './validate.cjs'; +import { textEncodingError } from './validate.cjs'; // eslint-disable-next-line @typescript-eslint/no-require-imports -- planning-workspace.cjs is an export= CommonJS module import planningWorkspace = require('./planning-workspace.cjs'); // eslint-disable-next-line @typescript-eslint/no-require-imports -- frontmatter.cjs is an export= CommonJS module @@ -1428,31 +1428,6 @@ function cmdVerifyKeyLinks(cwd: string, planFilePath: string, raw: boolean): voi ); } -function listMilestoneArchiveDirs(planBase: string): string[] { - const milestonesDir = path.join(planBase, 'milestones'); - try { - return fs - .readdirSync(milestonesDir, { withFileTypes: true }) - .filter((e) => e.isDirectory() && MILESTONE_ARCHIVE_DIR_RE.test(e.name)) - .map((e) => path.join(milestonesDir, e.name)) - .sort((a, b) => - path.basename(a).localeCompare(path.basename(b), undefined, { numeric: true }), - ); - } catch (err) { - // #1883: distinguish genuine absence from a permission/I-O failure. ENOENT - // (no milestones/ dir yet) keeps the long-standing [] contract callers of - // this function depend on for "no archives"; every other error (EACCES, - // EIO, …) must propagate — otherwise an unreadable milestones/ dir is - // silently reported as "no archives" and archived-phase resolution - // misbehaves. As of Phase 12 (#3310), `cmdValidateConsistency`'s own - // caller of this function (`collectPhaseRoots` -> `getActiveMilestoneArchiveDir`) - // was migrated onto `buildPlanningSnapshot` and deleted; this function is - // retained solely for its `_listMilestoneArchiveDirs` test seam below. - if ((err as NodeJS.ErrnoException).code === 'ENOENT') return []; - throw err; - } -} - interface IssueEntry { code: string; message: string; @@ -2026,9 +2001,4 @@ export = { cmdVerifySchemaDrift, cmdVerifyCodebaseDrift, STATE_HEAD_ADVISORY_COMMITS, - // Test seam (#1883): listMilestoneArchiveDirs is private and exercised through - // the validate command, which runs in a subprocess — an fs monkeypatch in the - // test process cannot reach it. Exposed under a leading underscore so the - // permission-error path can be unit-tested directly (no chmod 0o000). - _listMilestoneArchiveDirs: listMilestoneArchiveDirs, }; diff --git a/tests/verify-archive-dirs-live-path.test.cjs b/tests/verify-archive-dirs-live-path.test.cjs new file mode 100644 index 000000000..28d008668 --- /dev/null +++ b/tests/verify-archive-dirs-live-path.test.cjs @@ -0,0 +1,75 @@ +'use strict'; + +// ───────────────────────────────────────────────────────────────────────────── +// #1883 regression coverage on the LIVE path (#3813 re-point). +// +// The original #1883 suite asserted EACCES/EIO propagation on +// verify.cts's listMilestoneArchiveDirs — which lost its last production +// caller in Phase 12 (#3310) and survived only as a test seam, so the +// suite guarded dead code while the LIVE archived-phase enumeration +// (planning-snapshot.cts's buildArchivedPhaseTokensField) answered the +// same contract with a different, deliberate shape: a non-ENOENT +// milestones/ read failure yields scope UNREADABLE — never a silent +// COMPLETE "no archives". This suite pins the live shape. +// ───────────────────────────────────────────────────────────────────────────── + +const { test, describe } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); +const os = require('node:os'); + +const { buildPlanningSnapshot } = require('../gsd-core/bin/lib/planning-snapshot.cjs'); + +function fsError(code, targetPath) { + const err = new Error(`${code}: operation failed. scandir '${targetPath}'`); + err.code = code; + err.syscall = 'scandir'; + err.path = targetPath; + return err; +} + +// Inject a readdirSync fault scoped to the milestones/ path under test. +// t.mock auto-restores after each test — no chmod 0o000 (root bypasses mode bits). +function injectMilestonesFault(t, code, targetPath) { + const originalReaddirSync = fs.readdirSync; + t.mock.method(fs, 'readdirSync', function (p, ...rest) { + if (typeof p === 'string' && p.endsWith(path.join('milestones'))) { + throw fsError(code, targetPath); + } + return originalReaddirSync.call(this, p, ...rest); + }); +} + +function scopeOf(snapshot) { + const field = snapshot && snapshot.archivedPhaseTokens; + return field ? field.scope : undefined; +} + +describe('#1883 (live path, #3813): an unreadable milestones/ dir is UNREADABLE, never silent emptiness', () => { + test('a permission (EACCES) failure yields scope UNREADABLE, not COMPLETE', (t) => { + const planBase = path.join(os.tmpdir(), 'gsd-1883live-eacces-' + process.pid); + injectMilestonesFault(t, 'EACCES', path.join(planBase, 'milestones')); + const snap = buildPlanningSnapshot(planBase); + assert.equal( + scopeOf(snap), + 'unreadable', + '#1883/#3813: an unreadable milestones/ dir must never read as genuine absence (COMPLETE)', + ); + }); + + test('any non-ENOENT failure (EIO) yields scope UNREADABLE', (t) => { + const planBase = path.join(os.tmpdir(), 'gsd-1883live-eio-' + process.pid); + injectMilestonesFault(t, 'EIO', path.join(planBase, 'milestones')); + const snap = buildPlanningSnapshot(planBase); + assert.equal(scopeOf(snap), 'unreadable', 'every non-ENOENT error must flag UNREADABLE'); + }); + + test('an absent milestones/ dir (ENOENT) is a genuine empty: COMPLETE + []', () => { + const planBase = path.join(os.tmpdir(), 'gsd-1883live-absent-' + process.pid); + const snap = buildPlanningSnapshot(planBase); + const field = snap.archivedPhaseTokens; + assert.equal(field.scope, 'complete', 'genuine absence stays COMPLETE'); + assert.deepEqual(field.value, [], 'genuine absence yields no archived tokens'); + }); +}); diff --git a/tests/verify.test.cjs b/tests/verify.test.cjs index 941fd0a8b..e21e57561 100644 --- a/tests/verify.test.cjs +++ b/tests/verify.test.cjs @@ -3656,65 +3656,6 @@ describe('verifySummaryCore — reusable structured contract (#2572)', () => { }); }); -// ─── bug #1883: listMilestoneArchiveDirs must not swallow permission/I-O errors ── -// The private helper catch-alled every readdirSync error into [], so an unreadable -// milestones/ dir was silently reported as "no archives" (active-milestone -// resolution / archived-phase filtering misbehaved). The narrowed catch re-throws -// every non-ENOENT error and keeps [] only for genuine absence. Tested in-process -// via the _listMilestoneArchiveDirs test seam (the validate command runs in a -// subprocess, so an fs monkeypatch in the test process cannot reach it). -describe('bug #1883 — listMilestoneArchiveDirs distinguishes a permission error from emptiness', () => { - const verifyLib = require('../gsd-core/bin/lib/verify.cjs'); - const listMilestoneArchiveDirs = verifyLib._listMilestoneArchiveDirs; - const os = require('os'); - - function fsError(code, targetPath) { - const err = new Error(`${code}: operation failed, scandir '${targetPath}'`); - err.code = code; - err.syscall = 'scandir'; - err.path = targetPath; - return err; - } - - // Inject a readdirSync fault scoped to the milestones/ path under test. - // t.mock auto-restores after each test — no chmod 0o000 (root bypasses mode bits). - function injectMilestonesFault(t, code, targetPath) { - const originalReaddirSync = fs.readdirSync; - t.mock.method(fs, 'readdirSync', function (p, ...rest) { - if (typeof p === 'string' && p.endsWith(path.join('milestones'))) { - throw fsError(code, targetPath); - } - return originalReaddirSync.call(this, p, ...rest); - }); - } - - test('listMilestoneArchiveDirs re-throws a permission (EACCES) error instead of returning []', (t) => { - const planBase = path.join(os.tmpdir(), 'gsd-1883-eacces-' + process.pid); - injectMilestonesFault(t, 'EACCES', path.join(planBase, 'milestones')); - assert.throws( - () => listMilestoneArchiveDirs(planBase), - (err) => err.code === 'EACCES', - 'an unreadable milestones/ dir must propagate EACCES, not return [] as if empty', - ); - }); - - test('listMilestoneArchiveDirs re-throws any non-ENOENT error (EIO)', (t) => { - const planBase = path.join(os.tmpdir(), 'gsd-1883-eio-' + process.pid); - injectMilestonesFault(t, 'EIO', path.join(planBase, 'milestones')); - assert.throws( - () => listMilestoneArchiveDirs(planBase), - (err) => err.code === 'EIO', - 'every non-ENOENT error must propagate', - ); - }); - - test('listMilestoneArchiveDirs returns [] for an absent milestones/ dir (ENOENT) — empty path unchanged', () => { - const planBase = path.join(os.tmpdir(), 'gsd-1883-absent-' + process.pid); - // No milestones/ dir created → real OS readdirSync throws ENOENT. - assert.deepStrictEqual(listMilestoneArchiveDirs(planBase), [], - 'an absent milestones/ dir (ENOENT) must still return [] — Hyrum: empty path unchanged'); - }); -}); // ──────────────────────────────────────────────────────────────────────── // Folded from tests/issue-2701-nul-corrupted-validators.test.cjs — test-hygiene sweep #3338 (H3 wave 6)