chore(#3813): delete the caller-less listMilestoneArchiveDirs seam; #1883 contract now pins the live path (#4029)
* 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 <sim@local>
This commit is contained in:
Tom Boucher
6
.changeset/graceful-bears-caper.md
Normal file
6
.changeset/graceful-bears-caper.md
Normal file
@@ -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)
|
||||
<!-- docs-exempt: internal dead-code removal — the deleted helper had no production caller and no documented behavior; the replacement regression suite pins the live path's existing documented UNREADABLE scope contract -->
|
||||
@@ -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": {
|
||||
|
||||
@@ -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,
|
||||
};
|
||||
|
||||
75
tests/verify-archive-dirs-live-path.test.cjs
Normal file
75
tests/verify-archive-dirs-live-path.test.cjs
Normal file
@@ -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');
|
||||
});
|
||||
});
|
||||
@@ -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)
|
||||
|
||||
Reference in New Issue
Block a user