* test(#3412): failing-first suite for the pattern-construction seam
Phase 1 of epic #3212 (ADR-3212 §1/§2/§7). Tests only — src/pattern.cts
and eslint-rules/no-adhoc-regex-escape.cjs do not exist yet, so both
suites fail with MODULE_NOT_FOUND, which is the intended RED.
Locks the measured behavior rather than the assumed behavior:
RegExp.escape hex-escapes the leading character of nearly every string
("abc" -> "\x61bc"), so the suite asserts match-equivalence against an
inlined historical oracle (the implementation being deleted) rather
than byte-equivalence of pattern text — 200 seeded fast-check runs plus
a fixed corpus, 0 mismatches. Also locks the latent character-class
range bug this phase fixes as a side effect: a hyphen-bearing value
interpolated into [...] currently forms a real range and matches an
unintended character; post-migration it must not.
* chore(#3412): src/pattern.cts owns runtime-value regex construction
Phase 1 of epic #3212 (ADR-3212 §1/§2/§6/§7). Adds the pattern seam
delegating to the built-in RegExp.escape, deletes every hand-rolled
copy, and raises the Node floor to the Active LTS line.
The census was low, three times over. ADR-3212 counted 10 copies; a
graph query found 12; the new lint rule — once live — found 27 more.
The difference is that the census counted named helper FUNCTIONS while
the rule counts the escape SHAPE, so inline .replace(<class>, '\$&')
copies were never in scope. ADR §1's actual requirement is that no
module outside the seam escapes a value for regex use, so all of them
are, and CLAUDE.md's no-defer rule makes them this change's work.
Fourth consecutive epic here whose copy count was low — the argument
for ADR-3180 Amendment 3's "state N found by the guard" rule.
Also corrected mid-implementation: the survey reported phase-id.cts's
escapeRegex had 0 external importers. It had 8 production importers,
making its removal a public-surface change to an ADR-2121-owned module
and requiring an update to that ADR's locked-surface test. Blast
radius revised Medium-High -> High.
RegExp.escape is match-equivalent but NOT text-equivalent: it
hex-escapes the leading char of nearly every string ("abc" ->
"\x61bc"). Equivalence is proven by a seeded fast-check property test
against the deleted implementation as oracle. It also fixes a latent
bug: a hyphen-bearing value interpolated into a character class
previously formed a real range and matched an unintended character.
Node floor 22 -> 24 (RegExp.escape is Node 24+), across engines,
.nvmrc, package-lock, 9 CI matrix entries, and 5 docs. The aggregate
`required-tests` context is unchanged and no job was added or removed,
so branch protection cannot be orphaned by the dropped lanes.
Enforced by eslint-rules/no-adhoc-regex-escape.cjs (shape-matched, with
structural provenance for reviewed pattern-fragment constants rather
than a name heuristic) plus a whole-tree companion guard covering the
directories ESLint's globs miss.
* fix(#3412): close the _SOURCE guard evasion, correct two false claims
Three findings from the orthogonal review pass, all fixed.
1. The ESLint rule's `_SOURCE` provenance fallback was pure identifier-
name matching with no binding check, so `new RegExp(userInput_SOURCE)`
— a function parameter — sailed past the guard. That is the same
rename-evasion class issue #3410 documents, reopened by the very
fallback meant to complement the structural check. Now bound to the
identifier's actual binding kind: import, require-derived const, or
module-scope const; parameters, `let`/`var`, and unresolvable
bindings fail closed. Four RuleTester cases cover the evasion and
prove the legitimate cross-module case still passes.
2. src/pattern.cts's own header carried the stale pre-correction counts
(12 copies / 17 call sites) while CONTEXT.md and the design doc
carried the corrected ones (~39 / ~44) — a self-contradiction inside
the PR whose entire purpose is deleting divergent copies. Rewritten,
preserving the durable lesson: a named-function census cannot see
inline copies; only a shape-matching guard can.
3. The claim that all deleted copies threw TypeError on non-string was
false. phase-id.cts's copy — the one with 8 external importers — did
String(value).replace(...) and never threw. The seam's locked
signature does not coerce, so this is a real, now-disclosed behavior
change rather than the pure preservation the tests asserted. Audited
all 32 invocations across the 8 importers and 6 in-file callers:
every one is safe by construction (upstream truthy guard or a
string-producing derivation), verified by runtime probe against the
compiled modules rather than by TS compilation, which cannot see a
runtime undefined. Corrected the false claim in both the test comment
and the design doc, and added it to Known limits.
* docs(#3412): add Changed changeset for the Node 24 floor
The only user-visible break in this phase. The escape-behavior change
is internal and match-equivalent, so it carries no user-facing note.
* fix(#3412): resolve the seam's require graph in script fixtures and packaging
Checkpoint 2 came back red with 90 failures on the node24 lane. Three
distinct defects, all introduced by routing scripts/ through the new
pattern seam, none reproducible by any local gate:
1. ~82 failures — tests/adr-index-gate.test.cjs and
tests/removed-but-needed-lint.test.cjs copy a scripts/*.cjs into an
mkdtemp fixture and spawn it there (necessary: those scripts resolve
their scan root from __dirname/.., so running the real script would
scan the real repo). Each harness hand-listed the dependencies to
copy alongside. Adding require('../gsd-core/bin/lib/pattern.cjs') to
gen-adr-index.cjs made both lists silently incomplete ->
MODULE_NOT_FOUND, plus 17 downstream 'did not emit parseable JSON'
failures from the same crash.
Fixed as a class, not an instance: new tests/helpers/copy-script-
fixture.cjs walks a script's transitive static relative-require graph
and copies it, so dependencies are derived and never re-declared. It
throws (naming the unbuilt artifact) instead of letting the child die
with a bare MODULE_NOT_FOUND. Verified for all four seam-consuming
scripts: gen-adr-index, lint-removed-but-needed, gen-loop-host-
contract, sync-runtime-launcher.
2. 2 failures — scripts/ ships wholesale but eslint-rules/ does not, so
the new scripts/lint-no-adhoc-regex-escape.cjs would be
MODULE_NOT_FOUND in a published install (#2858 guard). Excluded from
the tarball, matching the existing precedent for gen-emitted-
baseline.cjs, which is excluded for the identical reason, and locked
with a test modeled on that one. Confirmed against a real npm pack:
890 files, 0 from eslint-rules/, and gsd-core/bin/lib/pattern.cjs
present (so the other four scripts' requires are legitimate).
3. 6 failures — tests/phase-id.test.cjs asserted the literal escaped
source text ('0*29', 'PROJ-42'). RegExp.escape is match-equivalent to
the retired hand-rolled escaper but NOT text-equivalent: it hex-
escapes the leading character and all hyphens ('0*\x329',
'\x50ROJ\x2d42'). Verified NOT a behavior change — 576 match
decisions across all three real interpolation prefixes, zero
divergence. Those tests now compile each source into the same heading
regex src/roadmap.cts's searchPhaseInContent builds and assert what
matches and what does not, including the 'i'-flag canonicalization
the hex escape has to preserve. Re-pinning the new literals would
have rebuilt the same brittleness one layer down. Adds a test for the
property the escape exists for: a dot in '1.2' must not act as a
wildcard.
Also shares one definition of 'a require' between the packaging guard
and the fixture copier, so the two cannot disagree about what they scan.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(#3412): refuse to copy a fixture dependency outside the fixture root
copyScriptWithDeps resolved each relative require and joined the
repo-relative result onto fixtureRoot. A require resolving OUTSIDE the
repo yields a '../'-prefixed relative path, so path.join climbed out of
the fixture and wrote into the surrounding temp dir (verified:
repoRoot=/repo + depAbs=/etc/passwd wrote /tmp/etc/passwd).
No script in the tree does this today, so this closes an available
escape rather than an active one. Refuses via the existing unresolved-
require path so the failure names the offending specifier. Covered by a
negative proof that the guard fires and that nothing lands outside the
fixture.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(#3412): parse requires instead of pattern-matching them; restore the foreign-prefix contract
Applies all findings from the second orthogonal review round, re-run
because real code changed after round 1.
HIGH (security) — extractRequires stripped BLOCK comments before LINE
comments, so a '//' comment containing '/*' opened a phantom block
comment, and a '//' inside a string literal truncated the line. Both
hid real requires: 'const u="http://x"; require("./real.cjs")'
returned [], and four real requires in gsd-core/bin/gsd-tools.cjs were
invisible. Replaced with a real AST parse via espree.
This is ADR-3212's own Decision 4 — tokenizer-first for stateful
grammars — applied to the case it describes; comment/string/regex
nesting is exactly such a grammar, which is why the regex version was
wrong. The function was moved byte-identical out of the #2858 packaging
guard, so the bug PRE-DATES this branch and has been a live blind spot
there: a shipped script could have required an unshipped path
undetected. Fixing it makes that guard strictly stronger than on next.
espree is promoted from a transitive eslint dependency to an explicit
devDependency rather than relying on hoisting. The script parse attempt
sets ecmaFeatures.globalReturn because Node wraps CommonJS bodies in a
function, making a top-level return legal — scripts/check-coverage-gate
.cjs relies on it, and without the flag the guard throws on a file it
is supposed to scan. Verified 0 unparseable across all 324 .cjs/.js
under scripts/, bin/, and gsd-core/bin/, and 0 new violations against a
real npm pack, so the exact extractor does not newly fail the guard.
MEDIUM (security) — the repo-containment check guarded dependencies but
not the entry path. One escapesContainment predicate now guards both.
LOW (security) — containment was lexical while fs follows symlinks, and
a directory symlink could mint a fresh dedupe key per level. realpath
now resolves both repoRoot and each dependency before the decision, and
the realpath-derived path is the dedupe key. Destination layout still
uses the original repo-relative path, so copied trees are unchanged.
MAJOR (standards) — the round-1 behavioral rewrite of phase-id tests
lost the foreign-prefix contract: every assertion was satisfied by an
impl returning [A-Z]+\x2d42, i.e. ANY project code — the exact #3599
bug class the exact-source prevents. The literal assertions it replaced
were catching this. Now asserts the compiled regex REJECTS a different
prefix with the same number.
MAJOR (standards) — the test hand-duplicated production's heading regex
with no parity guard (CLAUDE.md's 'Generative Fix Divergence'). Removed
the parallel surface instead of policing it: src/roadmap.cts exports
buildPhaseHeadingRegex, searchPhaseInContent calls it, the test imports
it. Byte-identical .source and .flags verified for both escaped forms.
MINOR — '..foo' no longer false-flagged as an escape; the inverted
spurious-vs-missing doc claim corrected; the dead allow-test-rule
header removed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* chore(#3412): backfill changeset pr number to 3416
* fix(#3412): make the escape guard's own regex linear, reword an injection-scan collision
Two CI failures on PR #3416, both in code this branch added.
CodeQL js/redos (high) — REPLACE_CALL_RE's outer alternation let a
bracket run be consumed EITHER by the character-class branch OR one
character at a time by the trailing catch-all, so a failing match
explored both parses of every pair. Measured on the real regex:
n=26 -> 204ms, n=28 -> 791ms, n=30 -> 3475ms, a clean 2^n. This script
scans repo source, so a file with a long bracket run after '.replace(/'
would hang CI outright — a guard against undisciplined pattern
construction was itself the worst pattern in the diff.
Fixed the way ADR-3212 already prescribes: the catch-all branch now
excludes '[' and ']' so a bracket can only be consumed by the class
branch (this is what makes it linear), and every quantifier is bounded
(the locked bounded-quantifiers decision) as a second line of defense.
Now 0ms at n=2000. Disclosed coverage tradeoff, recorded at the
constant: a regex literal with a BARE unescaped ']' outside a class is
no longer matched by this backstop. No census shape has that form, and
the AST rule remains the primary detector.
Verified the guard did not go blind doing it: a real census-shape
violation is still reported, and an allow-adhoc-regex-escape
suppression comment is still honored.
Regression test drives the exported findViolations on a
2000-repetition adversarial input and asserts the RESULT. It makes no
wall-clock assertion — elapsed-time tests are forbidden — so a
regression surfaces as a harness timeout, which is the correct signal.
Prompt injection scan — 'must not act as a regex wildcard' in a test
comment matched the scanner's jailbreak pattern act\s+as\s+(a|an|if|
my). Reworded to 'behave as'. Deliberately NOT allowlisted: silencing a
whole test file over one phrase would blunt the scanner permanently,
and the comment has nothing to do with injection.
Neither failure was reachable from the remote runner — CodeQL and the
injection scan are not in that matrix, so the sha it passed was green
and still wrong.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---------
Co-authored-by: sim <sim@local>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
* fix(#3025): refuse cross-runtime skill sync in sync-skills
Skill content and directory layout are runtime-specific — the installer
applies per-runtime converters, adapter headers, brand swaps, and layout
rules at install time, and grok/gemini resolve to ANOTHER runtime's skills
root. A verbatim cross-runtime cp -r therefore produces content the installer
would never have written for the destination, and can damage a runtime the
user never named. #3024 (closed) un-masked this, making the corruption live.
Fix (option b, user decision): add a functional Step 1 guard that refuses
any --to != --from with an actionable installer pointer, before any
resolution or copy. Identity sync (--from == --to) remains a no-op. The
non-functional Step 5 comment is replaced; Arguments/Limitations updated.
Regression: tests/sync-skills-cross-runtime-refuse.test.cjs (source-text-
is-the-product) asserts the guard exits non-zero for cross-runtime, points
at the installer, precedes the cp -r copy, and preserves identity.
* docs(#3025): backfill changeset PR number (#3404)
---------
Co-authored-by: sim <sim@local>
CI caught what the bench run didn't: "row 12" (every NONE-risk action
must actually apply) called applyRepairs('/fake/cwd', ...) — a literal
path that doesn't exist on disk. This was fine when applyRepairs's
handlers were stubs (pre-migration skeleton), but real handlers now
read/write actual files: createConfig writes config.json,
addNyquistKey/addAiIntegrationPhaseKey read it before patching. Against
a genuinely non-existent path these now correctly fail (ENOENT), and
an earlier fix in this same PR (applied only receives a code on real
success) correctly surfaces that as a failure instead of masking it —
so 3 of 4 codes stopped landing in `applied`, deterministically, on
any environment that actually enforces ENOENT against /fake/cwd.
Uses a real temp project (createTempProject + a valid config.json)
instead. Row 11 (DESTRUCTIVE refusal) and the ADVISE-skip test are
unaffected — both paths return before any handler touches the
filesystem, confirmed by reading applyRepairs's dispatch order.
REQ-HEALTH-05 said --repair auto-fixes recoverable issues without
qualification — now inaccurate since DESTRUCTIVE-risk remedies are
reported but never auto-applied. Adds REQ-HEALTH-06 for --backfill,
previously unmentioned in this requirements register.
docs/COMMANDS.md's /gsd-health section never documented --backfill at
all, and predates this phase's breaking changes: --repair no longer
auto-applies resetConfig/regenerateState (both destructive), and W021/
W017 split into W026/W027 for their previously-conflated second
subjects. Required by lint:docs, which needs a docs/ touch alongside
any Changed-type changeset fragment.
Both tests were written against the pre-fix behavior and never updated
once the real fixes landed:
- state-consistency.test.cjs's "KNOWN GAP" test hardcoded the
expectation that W002 incorrectly fires for a STATE.md phase
reference whose only home is an archived milestone — that gap is
now closed (0 diagnostics, confirmed against real buildPlanningSnapshot
output), so the test is renamed and its expectation flipped.
- worktree-health.test.cjs's two W020 tests asserted the OLD single
combined "timed out or failed" message/remedy — verified against
the real pre-migration src/verify.cts:2204-2219 that git_timed_out
and git_list_failed always had distinct messages; the fix that
restored this distinction is correct, these tests just never
caught up to it.
gsd-test found two independent gaps around the newly-generated
health.md tables:
- emitted-attribution.test.cjs requires an acknowledgment for
health.md's 2271-byte growth (16-code hand-maintained table -> 34-row
generated table, this phase's explicit acceptance criterion). Adds
tests/emitted-drift-acks/3309-health-docs-generated.json. Removes
2573-state-head-freshness.json's now-inert health.md entry (that
fragment's growth already landed on origin/next, the diff base this
branch is compared against, so it has nothing left to acknowledge —
and the guard forbids two fragments naming the same path).
- runtime-converters.test.cjs's health.md content-consistency checks
asserted stale text from the old hand-written table: a regex that
false-positived on the new table's own unrelated W020 row (worktree
scan degradation, a different diagnostic than the W025 isolation
warning it was meant to detect), and anchors expecting the old
table's exact last row / footnote wording. Narrowed the regex to
require the literal use_worktrees config key, and updated the
anchors to the new table's real shape (I001/I010 as the last rows,
the new generated-table footnote).
gsd-test found buildWorktreeHealthField collapsed every
inspectWorktreeHealth failure reason (git_timed_out, git_list_failed,
not_a_git_repo) into one UNREADABLE scope, discarding which one. The
migrated checkW020 then warned unconditionally on any UNREADABLE
scope — but the original (verify.cts:2202-2217) only warned on
git_timed_out/git_list_failed, staying silent on not_a_git_repo (a
.planning/-only fixture with no git repo at all is not a degraded
scan, just the absence of one). This spuriously degraded every test
fixture that isn't a real git repo.
planning-snapshot.cts's worktreeHealth field now carries `reason`
through instead of discarding it; checkW020 branches on it exactly
like the pre-migration code did.
gsd-test found the migrated W023 dropped a piece of information the
original message included: each colliding phase directory's overall
status (e.g. "Complete"), not just its raw plan/summary/verification
counts. Adds derivePhaseStatusLabel, reconstructing the status label
from already-exposed PhaseSnapshot fields (planCount/summaryCount/
complete/verificationStatus) — no new ambient I/O, no new snapshot
field.
Also fixes a non-conforming test fixture found while verifying:
tests/health-validation.test.cjs's "05-real" fixture used a bare
VERIFICATION.md, which readVerificationStatus never matches (the real
convention, and every other fixture in this repo, use the
*-VERIFICATION.md suffix) — the status was always reading as "missing"
regardless of message formatting. Renamed to 05-real-VERIFICATION.md.
gsd-test found three real regressions in the migrated STATE.md checks:
- W002 didn't exempt phase refs whose only directory lives in an
archived milestone (#3652) — now consults planning-snapshot.cts's
archivedPhaseTokens field (added alongside this fix, shared with
W006's identical need).
- W011 (STATE/ROADMAP cross-validation) never fired: currentPhaseLabel
only read the current template's bare "Phase:" field, silently
missing legacy STATE.md fixtures that use the older bold
"**Current Phase:**" field (mirrors state.cts's own resolveStatePhase
fallback ladder, which the migration didn't carry over).
- W026 (STATE milestone-complete vs. unstarted ROADMAP phases) had two
independent defects: roadmapDeclaredPhases's milestone attribution
can't see <details>/<summary>-shaped ROADMAP sections, and current-
milestone resolution could go null — both silently emptied the
"unstarted" set every time. Fixed by scoping ROADMAP.md to the
current milestone via the same <details>-tolerant extractCurrentMilestone
every other milestone-aware consumer uses, in a new dedicated
planning-snapshot.cts field (currentMilestoneRoadmapPhaseIds) rather
than reusing roadmapDeclaredPhases, which exists for a narrower,
<details>-blind derivation (W021's own original logic) and would
have regressed it if repurposed.
Also fixes an unrelated drift-guard violation this same rule file
introduced: its own phase-token regex was independently re-derived
instead of built from the canonical PHASE_NUMBER_TOKEN_SOURCE.
gsd-test found two real regressions in the migrated W006/W007:
1. A phase whose directory lives under an archived milestone
(.planning/milestones/v*-phases/<phase>/) instead of the active
phases/ dir read as "in ROADMAP but no directory on disk" — the
original forEachArchivedPhaseToken(planBase, ...) fed archived
tokens into the same existence check (verify.cts:2038); the
migrated rule's allPhaseDirNames never included them.
2. Comparing a ROADMAP-declared phase id against a disk directory name
dropped phaseVariants() normalization the original ran as a second,
independent check (verify.cts:2071-2073/2092-2093) — a ROADMAP
"01A" and a disk "1A-..." read as mismatched instead of the same
phase, since matchPhaseDirs's own token comparison never unifies
that padding/letter-suffix difference.
Adds planning-snapshot.cts's archivedPhaseTokens field (mirrors
forEachArchivedPhaseToken/listMilestoneArchiveDirs exactly, no new
regex derivation) and a phaseVariants()-based fallback in
dirsForPhase when matchPhaseDirs finds nothing.
Three user-visible changes disclosed per CONTRIBUTING.md's changeset
convention: --repair no longer auto-applies DESTRUCTIVE remedies
(Changed), W021/W017 split into W026/W027 for their previously-
conflated second subjects (Changed), and --backfill alone now actually
works (Fixed, a latent-bug fix). pr:0 placeholder, backfilled once the
PR number is known.
Closes the issue's explicit acceptance criterion: "health.md's tables
are generated rather than hand-maintained, closing the 16-vs-30+
documentation gap structurally." The published roster listed 16 codes
against 30+ actually emitted; W010-W017 and W020-W023 had never been
documented.
Adds description/repairable as static fields on Rule (health-diagnostic-types.cts)
— generation needs a fixed, human-readable summary per code, distinct
from the dynamic per-instance Diagnostic.message a rule's check()
produces. repairable is true only when --repair will actually apply
the remedy: false for ADVISE-only rules AND for DESTRUCTIVE-risk rules
(regenerateState/resetConfig), which are described but never
auto-applied — matches verify.cts's diagnosticToIssueEntry semantics
exactly, after fixing E004/E005's static field to agree with it (both
were wrongly true, an inconsistency caught during this same commit's
own review, not left for later).
New scripts/gen-health-docs.cjs (--write/--check, wired into
lint:generated-sync) regenerates the two tagged table regions in
gsd-core/workflows/health.md from RULES (31 rules) plus the 3
pre-checks that stay outside the rule table by design (E001, E010,
I010) plus a small static Effect/Risk lookup for the 6 real repair
actions — including addAiIntegrationPhaseKey, live in code since an
earlier phase but never documented until now. 34 error-code rows, 6
repair-action rows. The table's old "grep verify.cts for the next free
number" footnote is rewritten to point at the rule table and its lint
guard instead.
Still said RULES ships empty and repair handlers are stubs — true when
the skeleton batch first wrote this glossary entry, false since the
migration landed (RULES holds 31 wired rules, applyRepairs has real
per-action handlers). Found by the Standards-axis orthogonal review.
W024's committed rule (state-consistency.cts) is a documented permanent
no-op — its real check runs in cmdValidateHealth itself, outside the
rule table, since readStateHeadFreshness needs a git-log shell-out no
Rule.check may perform. The guard's §8.5 fixture-proof check previously
"passed" for W024 only because some test file's title happened to
contain the string "W024" — not because any fixture actually proves it
fires, which it structurally never can. Found by the Spec-axis
orthogonal review.
Adds an explicit PERMANENTLY_INERT_CODES map (currently just W024,
with its reason recorded) that checkFixtureProofInvariant reports
separately from real coverage. The guard's PASS output now says
"30 covered by a real fixture, 1 exempted" instead of implying uniform
proof — a code with no coverage and no exemption entry still fails.
adviseRemedy() was defined identically in two of the eight rule-group
files (config-validation.cts, agent-install.cts) while the other six
repeated the same {action: ADVISE, risk: NONE, args: {command}} object
literal inline ~20+ times. Found by the Standards-axis orthogonal
review (Duplicated Code smell).
Moves the one-line helper into health-diagnostic-types.cts, the leaf
module every rule-group file already imports for its enums/types, and
uses it consistently across all 8 files. Pure mechanical refactor — no
ADVISE remedy's command text, code, or action changed.
applyRepairs pushed a diagnostic's code onto applied unconditionally
after the try/catch around runRepairAction, even when the handler
threw (caught, recorded in details with success:false) or otherwise
failed — making applied mean "attempted" rather than "succeeded," with
no test exercising the failure path. Found by the Spec-axis orthogonal
review.
applied now only receives a code when the repair actually succeeded;
a failed attempt is still fully recorded in details (success:false,
the error message) but no longer misreported as applied. Adds a
regression test forcing addNyquistKey to throw (ENOENT on a config.json
that doesn't exist) and asserts it lands in details, not applied.
The migrated checkW027 (stale worktree) dropped the pre-migration
exclusion of the CLI's own current worktree, since a Rule.check(snapshot)
has no cwd access (§8.1 rule 1 forbids ambient I/O) — flagged as a
disclosed regression during this phase's own design work, then
confirmed as a real, fixable gap by the Spec-axis orthogonal review
rather than an inherent limitation.
Fixes it properly instead of accepting the regression: buildPlanningSnapshot(cwd)
already receives cwd as its own input, so exposing it as snapshot.cwd
is not new ambient I/O, just surfacing an existing parameter — fully
consistent with §8.1 rule 2's "parsed value" allowance. checkW027 now
excludes the entry matching snapshot.cwd before flagging, matching the
original verify.cts:2233-2242 behavior exactly.
gen-inventory-manifest.cjs's cli_modules family did a flat readdirSync
of gsd-core/bin/lib/, invisible to anything shipped in a subdirectory.
Found while registering this phase's 8 health-diagnostic-rules/*.cjs
files in docs/INVENTORY.md (Standards-axis review) — the automated
manifest cross-check couldn't see them even though the manual
INVENTORY.md rows were correct.
Adds collectOneLevelSubdirs (mirrors the existing collectNested's
defensive statOrNull style) and merges flat + one-level-subdirectory
results into cli_modules's single sorted array, using the same
<subdir>/<file>.cjs key format INVENTORY.md's rows already use.
Regenerating the manifest surfaced that three OTHER existing
subdirectories (installer-migrations/, host-integration-adapters/,
observability/ — pre-existing, unrelated to this phase) were equally
invisible and had zero docs/INVENTORY.md rows at all. Added all 15
missing rows rather than leave a gap the fix itself just exposed.
Also fixes 3 pre-existing lint-legacy-dir-name violations in the
installer-migrations rows (legitimate references to the historical
get-shit-done -> gsd-core rename these migrations clean up — marked
with the guard's own gsd-allow-legacy-name exemption) and a stale
health-diagnostic.cjs row that still said "RULES ships empty."
root-existence.cts (E002, E003) and phase-structure.cts (W009) hardcode
a canonical hyphen-form slash command in their ADVISE remedy, same as
config-validation.cts's already-disclosed W016 — forced by §8.1 rule 1
(a Rule's check(snapshot) cannot call the runtime-resolved slash()
formatter, which needs cwd). Only config-validation.cts's own header
disclosed this tradeoff; the other two sites had it happen without
recording it in their own file. Adds the same disclosure to both,
matching the established convention. No behavior change.
Replaces cmdValidateHealth's hand-rolled addIssue/switch accumulation
(961 lines) with buildPlanningSnapshot -> evaluateRules -> map to the
legacy {code, message, fix, repairable} shape, bucketed by severity.
Two pre-checks (home-dir E010/I010, .planning/-root-missing E001) stay
outside the rule table entirely, per ADR-3180 §8.2 rule 4 ("no
precedence system") — building "some rules suppress others" into the
table would itself be the forbidden precedence system.
W024 (STATE.md commit-age freshness) also stays outside the table:
its committed rule is a documented permanent no-op (readStateHeadFreshness's
git-log shell-out is ambient I/O a Rule.check may never perform, and no
PlanningSnapshot field carries a commits-behind count). Migrating onto
the rule table as designed would have silently regressed 7 passing
tests in tests/health-validation.test.cjs — found while wiring this
function, kept as a real check in the wrapper instead (same I/O
license applyRepairs already relies on), fixed inline per this repo's
no-defer policy rather than accepted as a silent loss.
Ports the real repair-handler bodies (createConfig/resetConfig,
regenerateState, addNyquistKey/addAiIntegrationPhaseKey,
backfillMilestones) into health-diagnostic.cts's applyRepairs,
replacing the skeleton's stub. DESTRUCTIVE-risk remedies
(resetConfig/regenerateState) are refused by --repair — a disclosed
breaking change; repairable now means "an automatic repair will
actually run," not merely "a remedy exists to describe," so E004/E005
now report repairable:false. --backfill alone now actually triggers
backfillMilestones, fixing a latent bug where its gate was unreachable
without --repair also being set (verify.cts:2504, confirmed dead code
pre-migration).
Test updates distinguish the two explicitly-authorized behavior
changes (DESTRUCTIVE refusal, backfill-alone fix, W021->W026 split)
from preservation — every changed assertion is commented with why, and
new regression tests were added for both changes plus W021/W026
mutual independence. Drift-guard bookkeeping (bypass-baseline shrunk
to the one disclosed W024 exception, milestone-window and
phase-enumeration exemptions, test-file-count allowlist) updated for
the relocated/new functions this migration introduces.
Enforces ADR-3180 §8.2's 1:1 rule-code invariant (every code unique,
every severity a property of the Rule) and §8.5's fixture-proof
invariant (every code has a describe()/test() block naming it,
verified statically against tests/health-diagnostic-rules/*.test.cjs
and tests/health-diagnostic.test.cjs) for the new RULES table.
Adapted from the design doc's original plan of separate
tests/fixtures/health-diagnostic/<code>.* files: implementation used
inline temp-dir fixtures instead (mirrors tests/planning-snapshot.test.cjs),
so coverage is checked statically against test-file structure, mirroring
lint-fix-has-regression-test.cjs's house style. Wired into lint:ci
adjacent to lint-planning-snapshot-bypass-drift.cjs, its closest sibling.
Passes clean against the real tree: 31 codes, all unique, all covered.
Wiring all 8 rule-group files into health-diagnostic.cts's RULES array
created a genuine CJS circular dependency: each group file required
health-diagnostic.cjs back for the shared enums, and health-diagnostic.cjs
now required the group files forward, so the enums were undefined
mid-load (destructuring health-diagnostic.cjs's still-unassigned
exports).
Fixes it by splitting the enums/types (SEVERITY, REMEDY_ACTION,
REMEDY_RISK, Remedy, Diagnostic, Rule) into a dependency-free leaf
module, health-diagnostic-types.cts, that both sides import instead of
each other. health-diagnostic.cts re-exports the enums for existing
consumers. RULES is now the real concatenation of all 8 groups (31
codes — E001 intentionally stays a pre-check outside the table).
W020 (3 conditions), W017, W027 — git worktree list degradation,
orphan and stale worktree checks, migrated onto the frozen rule table
per ADR-3180 §8.2. W027 has a documented fidelity reduction: rules
have no cwd access, so it can no longer exclude the active worktree.
W024 (deliberately inert, no snapshot field yet for stale state_head),
W002, W011, W021, W026 — STATE.md cross-checks against ROADMAP/config,
migrated onto the frozen rule table per ADR-3180 §8.2.
E002, E003, E004, W001 — PROJECT.md/ROADMAP.md/STATE.md existence and
PROJECT.md section-completeness checks, migrated onto the frozen rule
table per ADR-3180 §8.2.
Mirrors the existing health-diagnostic.cjs / planning-snapshot.cjs
pattern: gitignore the compiled output and exclude it from eslint so
the generated JS isn't linted as hand-written source. Also adds the
CONTEXT.md glossary entry and INVENTORY.md rows for the new
src/health-diagnostic-rules/ directory.
W007 (orphan disk dir with no ROADMAP entry) cannot be sourced from
phaseDirs, which is windowed to ROADMAP-declared phases only — an
orphan dir can never appear in an already-ROADMAP-filtered set. Adds
an unwindowed allPhaseDirNames field so the rule can actually fire.
Phase 11 of epic #3180 (ADR-3180 §8.1 rule 2). PlanningSnapshot grows from
7 fields to 15: projectSections, statePhaseTokens, stateStatus,
roadmapDeclaredPhases, roadmapPhaseCheckboxes, researchValidationStatus,
milestoneArchiveStatus, planningRootFiles.
Every field is a reused owner (buildRoadmapPhaseVariants/
buildNotStartedPhaseVariants from src/validate.cts, stateFieldValue) or a
small relocation of already-working verify.cts logic (PHASE_NUMBER_TOKEN_SOURCE
scanning, the checkMilestonePrefixMismatches sectionRx walk, W009/W018's
file-existence checks) — never a new algorithm, and never raw document text:
§8.1 rule 2 forbids exposing raw text, not exposing a parsed list or boolean
derived from it once by the snapshot builder.
roadmapPhaseCheckboxes deliberately reads the same ROADMAP checkbox
isPhaseComplete (§7.4, disk-strict) refuses to consult — that owner decides
completion and must not read it; this field only exposes what the checkbox
says, for a diagnostic (W011) whose whole purpose is flagging disagreement.
Not a re-derivation of §7.4, recorded explicitly to prevent that reading.
Adds PROJECT_UNREADABLE to UNUSABLE_REASON (ninth #1879 site), closing a
gap the implementing agent correctly flagged rather than silently leaving
absent-vs-corrupt collapsed for PROJECT.md, matching the STATE_UNREADABLE/
CONFIG_UNREADABLE precedent from this same effort's prior commits.
currentPhaseLabel/statePhaseTokens/stateStatus share one STATE.md read
(buildStateFields) rather than three independent reads.
Additive only — all prior fields and worstScope/buildPhaseSnapshot
unchanged.
Phase 11 of epic #3180 (ADR-3180 §8.2/§8.3/§8.5). New src/health-diagnostic.cts:
SEVERITY/REMEDY_ACTION (7 members: 6 real repair actions + ADVISE)/REMEDY_RISK
(NONE/DESTRUCTIVE) frozen enums, Diagnostic/Remedy/Rule types, an empty RULES
table (rules land in the next commits), evaluateRules (with a duplicate-code
defense-in-depth check ahead of the lint guard), and applyRepairs (the
DESTRUCTIVE-risk-refusal dispatcher — §8.3 rule 3 — with stub handlers; real
repair bodies port in the migration step).
Six-gate .cts ripple: .gitignore, eslint.config.mjs, docs/INVENTORY.md +
manifest, CONTEXT.md glossary entry.
Phase 11 of epic #3180 (ADR-3180 §8.2/§8.3/§8.5) foundation. Extends the
already-merged Phase-10 PlanningSnapshot additively with three fields the
upcoming health-diagnostic rule table needs and Phase 10 never required:
- config: {value, scope, exists} — parsed .planning/config.json. `exists`
distinguishes absent (no diagnostic, non-answer) from present-but-invalid
(CONFIG_UNREADABLE diagnostic, corruption) — both collapse to scope
UNREADABLE, so a rule needs the extra bit to tell "not configured yet"
apart from "config.json is broken."
- agentInstall / worktreeHealth — not .planning/-sourced, wrap the existing
checkAgentsInstalled/inspectWorktreeHealth owners with the same arguments
cmdValidateHealth already passes them, so a later migration step reads
these fields instead of calling the owners itself.
Adds CONFIG_UNREADABLE to src/unusable-input.cts's UNUSABLE_REASON (eighth
#1879 site), mirroring STATE_UNREADABLE's exact shape from Phase 10.
Additive only — the four Phase-10 fields and worstScope/buildPhaseSnapshot
are unchanged; existing tests for them are untouched.
PR #3402's real Windows CI (windows-latest node22/24) caught what gsd-test's
Linux-only lanes structurally cannot: tests/planning-snapshot-bypass-drift.test.cjs
compared the guard's own POSIX-normalized output (findSnapshotBypassDrift's
`file` field, dedupeViolationsForBaseline/sortEntries entries, a written
baseline read back from disk) against REGISTERED_FILE, which is built via
path.join('src', 'verify.cts') and is therefore backslash-separated on
Windows. The guard always normalizes its OUTPUT to POSIX via toPosixRel
regardless of the input separator form, so the comparison only ever
coincidentally passed on POSIX.
Adds REGISTERED_FILE_POSIX for every assertion against a guard-PRODUCED
value (including baseline fixtures fed into diffAgainstBaseline, which are
matched by exact string key against the guard's normalized output).
REGISTERED_FILE itself is unchanged and still used, correctly, everywhere it
is the relPath INPUT to findSnapshotBypassDrift or a DIAGNOSTIC_RULE_FUNCTIONS
Map-key lookup — both need the platform-native form to match the guard's own
Map key, which is also path.join-constructed.
No behavior change on POSIX (both constants are byte-identical there).
* test(#3170): extractOneLinerFromBody must anchor to a summary-shaped heading
Regression for #3170: the function matched the first heading's first bold
run, so an incidental first heading (rule list, deviation notes) contributed
its bold text as the milestone accomplishment. Rows 1/2 fail RED on next; rows
3/4 guard Overview recognition and the #2660 Summary-heading form.
* fix(#3170): anchor milestone one-liner extraction to a summary-shaped heading
extractOneLinerFromBody matched the first heading's first bold run regardless
of section, so an incidental first heading (rule list, deviation notes)
contributed its bold text as the milestone accomplishment written into
MILESTONES.md. Iterate headings and extract from the first Summary/Overview/
Accomplishments one with a bold run, falling back to null when none exists.
The #2660 Summary-heading forms and the frontmatter one-liner precedence are
preserved.
* docs(#3170): add changeset
* test(#3170): align extractOneLinerFromBody unit fixtures with summary-heading contract
The core-utils unit fixtures used generic # Title headings encoding the old
'any first heading' contract; the #3170 fix anchors to a Summary/Overview/
Accomplishments heading (the function is summary-specific). Update the
heading text to Summary-shaped; the extraction assertions (bold, frontmatter
strip, colon-label, CRLF, unicode) are unchanged.
* docs(#3170): backfill changeset PR number (3401)
---------
Co-authored-by: sim <sim@local>
Found while running gsd-test for #3308: tests/commit-files-pathspec.test.cjs's
repo-wide `--files` scan runs `git ls-files -z -- *.md` directly against the
checked-out repo root (not a createTempGitProject() fixture, unlike every
other gitOrThrow call in this file). Inside a container-provisioned test
runner the checkout's on-disk owner can legitimately differ from the running
UID, tripping git's CVE-2022-24765 dubious-ownership guard and failing the
scan closed (exitCode 128) rather than reporting a real file-list result —
reproduced on gsd-test's linux-node22 and linux-node24 lanes.
Adds `-c safe.directory=*` to that ONE invocation only, so the bypass is
scoped to this call rather than a global `git config` write that would leak
into every other git call in the process.
No source behavior changed; test-infrastructure resilience only.
Updates docs/adr/3180-planning-semantic-model-single-owner.md to
reflect Phase 10 shipping: §8.1 status Required -> Enforced (Phase 10,
#3308), guard-roster row contract only -> enforced, phase-index table
issue/status backfilled, and a new Amendment 9 recording the guard's
real baseline (15 distinct raw-read sites, 21 total acknowledged
occurrences in cmdValidateHealth) against the issue's own vaguer
estimate, per Amendment 4a's standing "N found by the guard, never
per the epic" rule. Also records the intended reading of an absent
STATE.md as UNREADABLE-without-diagnostic, symmetric with every other
§7 owner's absence-vs-corruption distinction.
Phase 10 of epic #3180. src/planning-snapshot.cts is a new parsed
projection of .planning/, composed exclusively from the already-
consolidated §7 owners (getMilestoneInfo, listMilestonePhaseDirs,
isPhaseComplete, scanPhasePlans, stateFieldValue, planningPaths) plus
the frozen SCOPE enum. No new semantic derivation is introduced beyond
worstScope, a pure combinator folding several independently-scoped
owner answers into one composite signal.
Adds STATE_UNREADABLE to src/unusable-input.cts's UNUSABLE_REASON
(seventh #1879 site) for STATE.md exists-but-unreadable, distinct
from absent.
Adds scripts/lint-planning-snapshot-bypass-drift.cjs, a ratcheted
drift guard (ADR-3180 Decision 4(e)) scoped to DIAGNOSTIC_RULE_FUNCTIONS
(currently cmdValidateHealth in src/verify.cts only) preventing new
raw .planning/ reads from bypassing the snapshot, while acknowledging
cmdValidateHealth's existing 15 raw-read sites as debt owned by
Phase 11 (#3309).
Six-gate .cts ripple: .gitignore, eslint.config.mjs,
docs/INVENTORY.md + manifest regen, CONTEXT.md glossary entry.
Breaking changes: none. This phase adds the subject only; Phase 11
migrates cmdValidateHealth onto it.
ADR-3180 epic #3180 Phase 10 (§8.1): tests for the not-yet-existing
src/planning-snapshot.cts (buildPlanningSnapshot, worstScope), the
not-yet-existing scripts/lint-planning-snapshot-bypass-drift.cjs guard,
and the new STATE_UNREADABLE reason on tests/unusable-input.test.cjs's
already-shipped UNUSABLE_REASON enum. RED by construction: the modules
under test do not exist yet.
* test(#3163): phase add must insert in the active milestone, not the trailing archive
Regression for #3163: cmdPhaseAdd/cmdPhaseAddBatch pick the insertion point
via rawContent.lastIndexOf('\n---'), the file's last horizontal rule — which
on a roadmap with shipped/history material after the active phase list sits
deep in archive. Rows 1/2/4 fail RED on next (entry lands after the archive
heading); row 3 guards the no-milestone legacy fallback.
* fix(#3163): scope phase.add insertion to the current milestone window
cmdPhaseAdd and cmdPhaseAddBatch picked the insertion point via
rawContent.lastIndexOf('\n---') — the file's last horizontal rule, which on
a roadmap with shipped/history material after the active phase list sits deep
in archive. Extract phaseEntryInsertOffset(rawContent, cwd): scope the search
to currentMilestoneRawRanges' primary window so the entry lands at the end of
the active phase list. Fall back to the legacy whole-file heuristic when no
current milestone resolves, preserving simple no-milestone roadmaps. Applies
to both cmdPhaseAdd and cmdPhaseAddBatch (identical expression); the decimal
insert path was already header-anchored and is untouched.
* docs(#3163): add changeset
* docs(#3163): backfill changeset PR number (3400)
---------
Co-authored-by: sim <sim@local>
The has_work=false branch previously exit-0'd from a step that ran,
so the job's conclusion was `success` -- identical to a PR that
actually ran mutation testing and passed. Moved the trivial-pass
condition to the job's own `if:`, so the job is SKIPPED (not run)
when has_work=false, matching the `coverage-gate` precedent in
test.yml and confirmed (via job-level `if:` research plus this
repo's own live PR #3392) that a skipped required check does not
block merge while still rendering visibly distinct from a pass.
First design attempt (splitting into two step-level `if:`-gated
steps) was verified WRONG before landing: a job whose every step is
individually skipped via step-level `if:` still reports `success`,
not `skipped` -- confirmed against documented GitHub Actions
behavior, not assumed.
Empirically verified via `workflow_dispatch`:
pre-fix (run 31652963610, on next): mutation-gate conclusion = success
post-fix (see PR): mutation-gate conclusion = skipped
Co-authored-by: sim <sim@local>
