fix(#1882): distinguish unterminated frontmatter from absent frontmatter (#2712)

* fix(#1882): distinguish unterminated frontmatter from absent frontmatter

extractFrontmatter returned {} both for a document with no frontmatter and for
one whose fence was opened and never closed, so a file truncated mid-write was
byte-identical to a legitimate no-metadata file. Verified live through
`gsd-tools frontmatter get`: both printed {} with exit 0 and nothing on stderr.

Per ADR-1411's "corrupt is not absent" amendment the {} return is preserved
exactly -- no caller may break -- and the cause is surfaced out-of-band as a
deduplicated, unconditional stderr diagnostic. That mechanism lands as a shared
leaf module rather than a per-site copy because three sibling findings in the
same epic need it identically; four hand-rolled copies of one behaviour is the
generative-fix-divergence defect class.

The discriminator is deliberately not "opened but never closed". A Markdown
document whose first line is a thematic break takes that exact branch, so
flagging on the missing fence alone reports corruption on good Markdown -- the
failure mode this class of check has shipped with before. The unterminated
region is instead run through extractFrontmatter's own parser (extracted as
parseYamlRegion so the probe and the real parse can never diverge) and reported
only when it yields at least one key.

Also folds an inline defect found while working: src/config-loader.cts carried
two NUL bytes in the JSDoc added by this epic's Phase 1 (3eb1cede2), making it
the only non-text file under src. file(1) reported it as data and text tools
silently skipped it, defeating the audit rule that says to search the authored
source; tsc passed because the bytes sat inside a comment, so no gate caught it.
It is live on next.

Refs #1879

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* test(#1882): pin unterminated-frontmatter detection and its negative space

Covers the discriminator on both sides. The positive rows are the issue's own
repro (LF and CRLF) plus the key-count boundary 0/1/2 around the ">= 1 parsed
key" threshold. The negative rows are the documents that reach the same branch
and must stay silent -- above all a Markdown thematic break at byte 0, which is
how this class of check has previously shipped a false positive on valid
Markdown.

Deduplication is tested on both halves of the composite key: a repeat of the
same (path, cause) is suppressed, a genuine second failure in a different file
is not, and a Windows and POSIX spelling of one path resolve to a single key.
The reset seam is asserted to actually clear -- #2674 is the precedent where a
reset that silently failed to clear made every later dedup assertion a vacuous
pass, and the cases only passed because each happened to pick an unused key, so
every case here uses a path unique to itself.

Assertions are on typed surfaces throughout -- the frozen reason enum and the
dedup-set size -- never on diagnostic prose. The one CLI-level case asserts a
differential between two runs (whether stderr is empty) rather than matching a
message, and is the wired user-reachable surface for this fix. Stream failure is
injected by overriding process.stderr.write and restoring it, never chmod 0o000,
which root bypasses.

Two properties guard the ~50 call sites of the changed function: the new
optional path argument is inert with respect to the parsed value, and LF/CRLF
spellings of a document still parse identically.

Refs #1879

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(#1882): raise the truncation threshold and repair the dedup key

Isolated adversarial review found the one-key discriminator false-positives on
ordinary Markdown: a thematic break above a single labelled line -- `Note:`,
`Author:`, `TODO:`, `See:` -- parses as exactly one key and was reported as
corruption, which is the precise failure the design claimed to prevent and the
changeset promised was fixed. The threshold is now two keys. A file truncated
after exactly one key becomes a false negative; that is the same
precision-over-recall direction already taken at zero keys, and every GSD
artefact this guards carries two or more frontmatter keys.

Three dedup-key defects, each of which could silently swallow a real diagnostic:

- Backslash normalization is removed. A backslash is a legal filename character
  on Linux and macOS, so folding it to a forward slash made two genuinely
  different files share one key. Two spellings of one Windows path may now
  report twice; two distinct files can never silence each other. Lost signal is
  the worse failure.
- The key namespaces are tagged so a file literally named like the unnamed
  digest fallback can no longer collide with a path-less caller whose content
  hashes to that digest -- computable for any predictable content, no brute
  force needed.
- The source identity is computed once rather than hashed twice per emission.

Corrects the previous commit. The two NUL bytes in src/config-loader.cts were
NOT in a JSDoc comment as that message claimed; they were deliberate separators
in the live dedup key, and stripping them degraded it to bare concatenation.
They are restored as escape sequences -- byte-identical runtime string, and the
file is text again so grep can see it. The diagnostic script that misled me
indexed a character-offset string with a byte offset.

Also threads sourcePath through the STATE.md and PLAN.md readers so the two
artefacts epic #1879 is actually about name their file rather than reporting
under a content digest.

Refs #1879

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* test(#1882): correct fixtures and assertions left behind by the review fixes

The previous commit changed two behaviours deliberately and the suite still
encoded the old ones, so gsd-test came back red with six failures across both
lanes -- all of them mine.

Fixtures carrying a single frontmatter key no longer clear the two-key
truncation threshold, so the CLI differential and the two path-less dedup cases
were asserting a diagnostic that is now correctly withheld. They now carry two
keys, which is what a real interrupted write of a GSD artefact looks like.

The Windows/POSIX case asserted that two spellings of one path collapse to a
single key -- the exact folding that was removed because it also collapsed
genuinely distinct POSIX files whose names contain a backslash. Inverted to
assert they now report separately, with the reasoning recorded inline so the
trade is not silently reversed later: mild duplicate noise on one Windows path
is acceptable, a swallowed diagnostic is not.

Refs #1879

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(#1882): name the file at every read site, and report each file once

The diagnostic reached only the four frontmatter CLI verbs, so ~47 of 53 call
sites reported a truncated file under an anonymous content digest instead of
naming it. Since naming the file is the whole point -- it is what an operator
can act on -- that was a gap in the deliverable, not a scoping choice. 43 of 53
sites now pass the resolved path.

Closing it surfaced a defect the original design missed. A single truncated
STATE.md is parsed twice in a normal run: once by the read wrapper, which holds
the path, and again by a pure core downstream, which is handed only the string
and cannot know it. Those two parses keyed separately, so one file produced two
diagnostics -- and wiring more sites made the collision more likely, not less.
Every emission now registers both identities the input could be known by and
checks both before writing, so whichever caller arrives first speaks and the
other is suppressed. Distinct files with distinct content still report
separately, which is the property ADR-1411 actually requires; two files whose
truncated content is byte-identical collapse to one report, which stays the
documented limit.

Ten call sites deliberately keep no path. Two are frontmatter's own round-trip
checks during set and merge, where passing a path would report on every write.
The other eight are the state-transition pure cores, which ADR-1769 defines as
(content, intent, deps) -> newContent with injected I/O; threading a path
through them would contradict that recorded decision, so it is surfaced rather
than taken unilaterally. With the widened key they no longer double-report, and
in the normal flow the named parse runs first, so the file is still named.

Refs #1879

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(#1882): inject the STATE.md path into the transition cores

The six state-transition cores parsed STATE.md frontmatter without knowing
which file it came from, so a truncated STATE.md reached the operator as an
anonymous content digest on exactly the artefact epic #1879 is named for.

ADR-1769 section 3 shapes these as (content, intent, deps) -> newContent with
injected deps, and deps is the seam for precisely this: something the core
cannot derive without doing I/O. It already carries roadmapProvider and a
phase-inventory provider on that basis, each documented as injected rather than
imported so the core stays pure and testable without disk access. A resolved
path is data, not I/O, so an optional sourcePath member extends the established
pattern rather than contradicting it, and every existing stub keeps compiling
because the member is optional.

updateCore and reconcileCurrentPosition take no deps and are left alone. With
the widened dedup key they cannot double-report, and in the normal flow the read
wrapper has already named the file by the time they run.

Also regenerates gsd-core/bin/lib/state-transition.cjs. That artifact is tracked
rather than gitignored, unlike most of its siblings, so leaving it stale would
have shipped a runtime without this change to anyone reading the repo without
building. tsc had skipped the re-emit because its incremental build info still
recorded an emit that had since been reverted, so the stale output survived a
clean build; clearing tsconfig.build.tsbuildinfo forced it. The
compiled-artifact-sync gate is what surfaced the drift and now reports all nine
tracked artifacts matching their source.

Refs #1879

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(#1882): stop the widened dedup key from hiding a second file

The previous commit widened the dedup guard so one file parsed twice -- once by
a read wrapper holding the path, once by a pure core holding only the string --
reported once instead of twice. It did that by checking BOTH keys before
emitting, which silently traded one defect for a worse one: two DIFFERENT files
whose truncated content happened to be byte-identical now collided on the shared
content digest, and the second file's diagnostic was swallowed. That is the
over-coarse keying ADR-1411 explicitly forbids, reintroduced while fixing
something else.

The guard now checks only the key matching what the caller actually knows -- a
named read checks its path key, a path-less read checks its digest key -- while
still recording every key the input could later be identified by. The redundant
path-less re-parse of an already-named file stays silent, and two distinct files
always both report.

Verified across all six orderings: same file named-then-anonymous reports once;
two different files with identical content report twice; two different files
with different content report twice; the same path twice reports once; two
path-less parses of identical content report once; two path-less parses of
different content report twice.

The suite caught this -- twenty failures, all in the unusable-input tests that
reuse one truncated fixture across different paths. The local probe written
alongside the broken change did not, because it compared two files with
different content and could therefore only confirm the expected behaviour.

Refs #1879

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* test(#1882): count diagnostics emitted, not identities interned

The suite measured the size of the dedup set as a stand-in for "how many
diagnostics were emitted". That held only while one emission recorded exactly
one key. Once an emission began recording every identity the input could later
be matched by -- a path key and a content key for the same file -- the set grew
by two per write and twenty assertions read 2 where they expected 1.

The production behaviour was correct throughout; the proxy was not. Set size
counts identities, which is an implementation detail of the guard. The
behavioural claim these tests exist to make is how many diagnostics an operator
actually saw, so the module now exposes that directly as an emission counter and
the suite asserts on it. The set-size accessor stays for assertions genuinely
about key shape.

The local probe written alongside the change did not catch this because it
counted process.stderr.write calls -- the right thing -- while the suite counted
set growth. Verification now asserts both and requires them to agree, so a
future divergence between the counter and real writes fails immediately rather
than being discovered a bench run later.

Refs #1879

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* test(#1882): retire two assertions that outlived the behaviour they described

Both tests encoded assumptions the dedup fix invalidated, and both were caught
by the suite rather than by the probe written alongside the change.

The forged-path case asserted that a file named like the anonymous digest
fallback must not suppress a later path-less report. That premise is gone: an
emission now records every identity its input could be matched by, so ANY named
report of some content silences the anonymous re-parse of that same content --
which is the same-file guard working as intended, and has nothing to do with the
crafted name. The property still worth defending is that a crafted filename can
never silence a real file reported under its own path, so that is what the test
now asserts, with the deliberate suppression documented beside it.

The reset-seam case ended by reading the size of the dedup set and expecting 1.
Set size counts interned identities, not diagnostics written, and one emission
now interns two. It asserts the emission counter for the event and keeps a
weaker set-size check for the interning.

Refs #1879

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(#1882): close the review findings on the discriminator, dry-run and counter

Three orthogonal review passes ran against the final diff. Their findings:

A labelled preamble under a leading rule was still misreported. Raising the key
threshold to two only moved the boundary, because two colon-labelled lines are
as common in ordinary prose as one -- a document opening with a rule over an
Author and a Reviewed-by line, then prose, was called corrupt. Key count alone
cannot separate the two. What does is what follows: a write interrupted part way
through a frontmatter block ends mid-block, so every line of the region is still
frontmatter-shaped, whereas a document merely opening with a rule goes on to
prose. Both conditions are now required, and each closes a false-positive class
the other leaves open. Nested list values and indented continuations stay
frontmatter-shaped, so legitimate truncations are unaffected.

`state rebuild --dry-run` reported a truncated STATE.md anonymously. The write
path is named only because readModifyWriteStateMd parses with the path first;
the dry-run branch reads the file directly and never did. Dry-run is the
read-only mode an operator reaches for first when they suspect corruption, so it
is the one that most needed to name the file. reconcileCurrentPosition takes the
path as an optional argument now and rebuildCore passes it down. That function
was previously left alone on the grounds that a read wrapper always names the
file first -- this is the flow that disproves it.

The emission counter counted write attempts rather than writes, so on a broken
stderr it claimed a diagnostic had reached the operator when nothing had. It is
incremented only after a write that completed, and the broken-stderr test now
asserts the count as well as the return value.

Two documentation defects. The module described a guarantee it does not keep:
one file yields one diagnostic only when the named read comes first. The reverse
ordering emits twice, and that is deliberate -- a path-less caller cannot
identify its file, so suppressing the later named report would also suppress a
genuine second failure in a different file whenever two files share identical
truncated bytes, which ADR-1411 ranks the worse failure. The comment now states
the asymmetric guarantee and a test pins it. Separately, the CONTEXT.md glossary
entry still described backslash normalization that a later commit removed, and
asserted the opposite of what the tests pin; no lint checks prose against code,
so nothing caught it.

Also converts three body-level try/finally blocks to t.after(), per
CONTRIBUTING.md's rule that try/finally belongs only in helpers with no test
context -- the file's own emissionsDuring helper already did this correctly.

Refs #1879

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* docs(#1882): tell the operator what the truncated-frontmatter warning means

A user who has just seen the new warning is acting, not studying, so this lands
in the How-To quadrant beside the other "if you see X" branches in
debug-a-failed-execution, not in reference or explanation. It gives them what
the warning means for this run, three steps to restore the file, and the fact
that the warning changes no return value or exit code.

It also states the case that matters more than the warning itself: silence does
not prove the file is intact. GSD says nothing when the partial block carries
fewer than two fields or reads as prose, because a Markdown document opening
with a horizontal rule is indistinguishable from one of those. A reader chasing
missing metadata needs to know not to treat quiet as clean. Why that threshold
exists is explanation and deliberately stays out of a how-to.

Refs #1879

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* chore(#1882): backfill changeset pr number to 2712

* test(#1882): constrain each branch of the frontmatter-shape check

CI's mutation gate came in at 61.56 against a threshold of 62, and the surviving
mutants were concentrated in isFrontmatterShaped -- the function added last, in
response to review, and the only one never given tests of its own. It was
exercised solely through extractFrontmatter, which covers the composite decision
but leaves each branch of the predicate unconstrained: drop the blank-line
filter, or any one of the three shape alternatives, and every existing assertion
still passed.

Four cases now pin the halves independently. A blank line inside an interrupted
block must not disqualify it, which constrains the filter and its comparison. An
unindented list item and an indented folded-scalar continuation each exercise one
shape alternative that no other case reaches on its own -- the folded line is
neither a key nor a list item, so it is the only input that distinguishes the
indented branch. And two keys followed by prose must stay silent, which is the
negative half: it fails if the predicate is ever mutated to accept everything,
and it is the case that proves key count alone was never sufficient.

Refs #1879

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* test(#1882): register the unusable-input suite with the frontmatter mutation shard

The mutation gate reported an identical 61.56 across two runs whose only
difference was four added tests. That is the tell: the tests were never
executed. The frontmatter shard runs a fixed file list in stryker.config.mjs and
scripts/mutation-matrix.cjs, and tests/unusable-input.test.cjs was in neither, so
the entire suite covering the new unterminated-fence branch was invisible to the
gate while passing perfectly well in the normal run.

So the score was not measuring weak tests, it was measuring absent ones: #1882
added mutants to frontmatter.cjs and no test in the shard covered them. Both
lists gain the file; the config already notes they must stay in sync.

This is a registration ripple a new test file carries when it covers a
mutation-tracked module, alongside the .gitignore, eslint, inventory, glossary
and size-baseline ripples a new module carries. Nothing warned about it, which
is why two runs were spent before the identical score gave it away.

Refs #1879

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Tom Boucher
2026-07-27 16:50:12 -04:00
committed by GitHub
parent 90ba0ef10b
commit 9a76ca6783
29 changed files with 1058 additions and 89 deletions

View File

@@ -0,0 +1,5 @@
---
type: Fixed
pr: 2712
---
**A truncated or half-written frontmatter file is no longer silently read as "no metadata"** — a document whose `---` fence was opened and never closed used to return exactly the same empty result as a file that legitimately has no frontmatter, so a crash mid-write left every phase/state reader proceeding with empty contracts and no signal. GSD now names the offending file on stderr while returning the same value as before, so nothing that consumed the old result changes. A Markdown horizontal rule at the top of a document — including one above a labelled line such as `Note:` or `Author:` — is not mistaken for a truncated fence. (#1882)

1
.gitignore vendored
View File

@@ -177,6 +177,7 @@ build/
/gsd-core/bin/lib/phase-id.cjs /gsd-core/bin/lib/phase-id.cjs
/gsd-core/bin/lib/normalize-test-command.cjs /gsd-core/bin/lib/normalize-test-command.cjs
/gsd-core/bin/lib/config-loader.cjs /gsd-core/bin/lib/config-loader.cjs
/gsd-core/bin/lib/unusable-input.cjs
/gsd-core/bin/lib/model-resolver.cjs /gsd-core/bin/lib/model-resolver.cjs
/gsd-core/bin/lib/loop-resolver.cjs /gsd-core/bin/lib/loop-resolver.cjs
/gsd-core/bin/lib/capability-state.cjs /gsd-core/bin/lib/capability-state.cjs

View File

@@ -115,6 +115,9 @@ Cross-seam principle (ADR-1411, epic #1411): context resolution — config loadi
### Resolution Convention ### Resolution Convention
Diagnostic-output convention for the Resolution Provenance principle (ADR-1411 P3, #1416). Config-interpreting read verbs expose `Resolution<T> { value, configured, reason, warnings }` (`src/resolution.cts`); agent-skills is the first adopter, where `value = { block, skills_count }` and `source`/`degraded` remain config-provenance extras outside the envelope. Other read verbs expose at least `warnings[]` (e.g. capability-state `{ runtimeConfigDir, capabilities, warnings? }`) without `configured`/`reason`, which are meaningful only for config-interpreting verbs. Mutation verbs expose `warnings[]` (advisory) PLUS `errors[]` (operation-not-applied), e.g. capability-writer `{ capabilities, warnings, errors }`. The shared seam across all shapes is `warnings: string[]`; a single generic `Resolution<T>` across read+write verbs was rejected by the deletion test (`configured`/`reason` are meaningless for capability verbs; `errors[]` cannot fold into `warnings[]`) — ADR-1411 P3 amendment. Recurrence prevention is delivered by P4's CI guard (a configured input resolving empty must carry a `reason`), not by a shared envelope. A CI guard (`scripts/lint-resolution-provenance.cjs`, wired into `lint:ci`) enforces that every registered config-interpreting read verb keeps a `configured_empty`/`not_configured` contract test; the registry in that script is the registration point for future verbs (ADR-1411 P4 / #1417). Diagnostic-output convention for the Resolution Provenance principle (ADR-1411 P3, #1416). Config-interpreting read verbs expose `Resolution<T> { value, configured, reason, warnings }` (`src/resolution.cts`); agent-skills is the first adopter, where `value = { block, skills_count }` and `source`/`degraded` remain config-provenance extras outside the envelope. Other read verbs expose at least `warnings[]` (e.g. capability-state `{ runtimeConfigDir, capabilities, warnings? }`) without `configured`/`reason`, which are meaningful only for config-interpreting verbs. Mutation verbs expose `warnings[]` (advisory) PLUS `errors[]` (operation-not-applied), e.g. capability-writer `{ capabilities, warnings, errors }`. The shared seam across all shapes is `warnings: string[]`; a single generic `Resolution<T>` across read+write verbs was rejected by the deletion test (`configured`/`reason` are meaningless for capability verbs; `errors[]` cannot fold into `warnings[]`) — ADR-1411 P3 amendment. Recurrence prevention is delivered by P4's CI guard (a configured input resolving empty must carry a `reason`), not by a shared envelope. A CI guard (`scripts/lint-resolution-provenance.cjs`, wired into `lint:ci`) enforces that every registered config-interpreting read verb keeps a `configured_empty`/`not_configured` contract test; the registry in that script is the registration point for future verbs (ADR-1411 P4 / #1417).
### Unusable Input Diagnostic Module
Leaf module owning the **out-of-band** half of ADR-1411's "corrupt is not absent" amendment (epic #1879). Where a read already returns a provenance envelope the cause is named in-band (`ConfigResolution.reason`, #1880); where a read returns a bare sentinel or a plausible default it cannot extend, the return value is preserved exactly and the cause is surfaced here instead. Interface: `UNUSABLE_REASON` (frozen reason enum — one entry per condition that has an emitting call site; adding a reason is three coordinated changes: enum + call site + the test locking `Object.keys(...).sort()`), `warnUnusableInput({reason, source?, content?}) → boolean` (returns whether this call actually wrote, so tests assert emission *counts* on a typed surface rather than scraping stderr), plus the `_resetUnusableInputWarningsForTests` / `_unusableInputWarningCountForTests` seams. Dedup key is `<normalized source>\0<reason>` — **both halves are load-bearing**: keying on the path alone would let a second, different fault on the same file go unreported, and keying on message prose would couple the guard to wording (ADR-1411 dedup clause). Path separators are deliberately **not** normalized: an earlier revision folded backslashes to `/` so two spellings of one Windows path would not double-report, but a backslash is a legal filename character on Linux and macOS, so that folding collapsed two genuinely distinct POSIX files onto one key and swallowed the second file's diagnostic. The trade is now one-directional — two spellings of one Windows path may report twice (noise), but two distinct files can never silence each other (lost signal), and ADR-1411 ranks the swallow the worse failure; ASCII control characters are stripped from the source before it is keyed or written, because the key separator is NUL (a crafted path could otherwise forge a collision) and because a path carrying ANSI escapes would replay into the operator's terminal. Callers with no path (in-memory content) fall back to a short content digest so *different* bad inputs still key differently. The diagnostic is **unconditional** — a deliberate divergence from ADR-227's never-implemented `GSD_DEBUG` opt-in, since "an opt-in nobody sets is indistinguishable from the silence #1879 is about" — and **never throws**: a failed stderr write is swallowed so a degraded read is never escalated into a crash. First adopter is `extractFrontmatter` (#1882); `roadmap-parser` (#1881) and `planning-workspace`/`verify` (#1883) follow. Exists as a shared seam rather than a per-site copy because four sites need identical behavior and four hand-rolled copies is `DEFECT.GENERATIVE-FIX` by construction. Source of truth: `gsd-core/bin/lib/unusable-input.cjs` (generated from `src/unusable-input.cts`). Test anchor: `tests/unusable-input.test.cjs`. See Resolution Provenance, Config Loader Module.
### Worktree Safety Policy Module ### Worktree Safety Policy Module
CJS Module owning worktree lifecycle safety policy for the GSD orchestration layer. Interface: `resolveWorktreeContext(cwd, deps) → WorktreeContext` (linked-worktree root mapping), `parseWorktreePorcelain(output) → WorktreeEntry[]` (porcelain parser, skips detached HEAD), `planWorktreePrune(repoRoot, opts, deps) → PrunePlan` (metadata-prune plan, never destructive by default), `executeWorktreePrunePlan(plan, deps) → PruneResult` (executes prune; degrades gracefully on git timeout), `listLinkedWorktreePaths(repoRoot, deps) → LinkedPathsResult`, `inspectWorktreeHealth(repoRoot, opts, deps) → HealthResult` (orphan + stale detection), `snapshotWorktreeInventory(repoRoot, opts, deps) → InventoryResult`, `planWorktreeWaveCleanup(repoRoot, manifest) → CleanupPlan` (manifest-scoped, fail-closed), `executeWorktreeWaveCleanupPlan(plan, deps) → CleanupResult`, `planWorktreeRecordAgent(manifestRaw, fields) → RecordAgentPlan` (write-strict per-agent manifest append; validates each field at write time via the same `normalizeCleanupManifestEntry` rules the reader enforces; fail-closed on a missing/garbled field or a duplicate `(worktree_path, branch)` the reader would dedup away), `cmdWorktreeRecordAgent(cwd, args, deps) → RecordAgentCmdResult` (thin deps-injectable IO wrapper for the `worktree record-agent` verb), `planWorktreeCreate(fields) → WorktreeCreatePlan` (write-strict `worktree create` planner — same missing-field-hint and `normalizeCleanupManifestEntry` validation as `planWorktreeRecordAgent`, pure/no-git), `executeWorktreeCreatePlan(plan, repoRoot, deps) → WorktreeCreateResult` (bounded `git rev-parse --verify` base check THEN `git worktree add -b <branch> <path> <base>`; fail-closed `base_unresolved`/`git_timeout`/`worktree_add_failed`; returns `cwd` — the working directory an executor spawn would use), `cmdWorktreeCreate(cwd, args, deps) → WorktreeCreateCmdResult` (CLI verb: plans, creates the worktree, then appends the manifest entry so it is immediately manageable by cleanup-wave/reap-orphans; dedupes by `(worktree_path, branch)`). #2584 ADR-1239 Codex-binding amendment, Phase 2: `worktree create` is the git-worktree-creation primitive for `dispatch.isolation: orchestrator-worktree` hosts — declared and testable but UNCONSUMED (no scheduler calls it yet; Phase 3 wires it). Source of truth: `gsd-core/bin/lib/worktree-safety.cjs`. Timeout path: all git subprocess calls are bounded; callers receive `ok:false, reason:'git_timed_out'` rather than a thrown exception. Test anchor: `tests/worktree-safety.test.cjs`. The `core.cjs` re-export spine was retired in epic #1267: this module absorbed the two thin compositional wrappers that squatted in Core — `resolveWorktreeRoot(cwd, deps)` (a projection over `resolveWorktreeContext`) and `pruneOrphanedWorktrees(...)` (sequences `planWorktreePrune` + `executeWorktreePrunePlan` with a timeout warning) — so callers reach this single worktree-lifecycle seam directly. `gitWorktreeInfoInternal` did NOT move here — worktree-info detection belongs to the Git Query Module. CJS Module owning worktree lifecycle safety policy for the GSD orchestration layer. Interface: `resolveWorktreeContext(cwd, deps) → WorktreeContext` (linked-worktree root mapping), `parseWorktreePorcelain(output) → WorktreeEntry[]` (porcelain parser, skips detached HEAD), `planWorktreePrune(repoRoot, opts, deps) → PrunePlan` (metadata-prune plan, never destructive by default), `executeWorktreePrunePlan(plan, deps) → PruneResult` (executes prune; degrades gracefully on git timeout), `listLinkedWorktreePaths(repoRoot, deps) → LinkedPathsResult`, `inspectWorktreeHealth(repoRoot, opts, deps) → HealthResult` (orphan + stale detection), `snapshotWorktreeInventory(repoRoot, opts, deps) → InventoryResult`, `planWorktreeWaveCleanup(repoRoot, manifest) → CleanupPlan` (manifest-scoped, fail-closed), `executeWorktreeWaveCleanupPlan(plan, deps) → CleanupResult`, `planWorktreeRecordAgent(manifestRaw, fields) → RecordAgentPlan` (write-strict per-agent manifest append; validates each field at write time via the same `normalizeCleanupManifestEntry` rules the reader enforces; fail-closed on a missing/garbled field or a duplicate `(worktree_path, branch)` the reader would dedup away), `cmdWorktreeRecordAgent(cwd, args, deps) → RecordAgentCmdResult` (thin deps-injectable IO wrapper for the `worktree record-agent` verb), `planWorktreeCreate(fields) → WorktreeCreatePlan` (write-strict `worktree create` planner — same missing-field-hint and `normalizeCleanupManifestEntry` validation as `planWorktreeRecordAgent`, pure/no-git), `executeWorktreeCreatePlan(plan, repoRoot, deps) → WorktreeCreateResult` (bounded `git rev-parse --verify` base check THEN `git worktree add -b <branch> <path> <base>`; fail-closed `base_unresolved`/`git_timeout`/`worktree_add_failed`; returns `cwd` — the working directory an executor spawn would use), `cmdWorktreeCreate(cwd, args, deps) → WorktreeCreateCmdResult` (CLI verb: plans, creates the worktree, then appends the manifest entry so it is immediately manageable by cleanup-wave/reap-orphans; dedupes by `(worktree_path, branch)`). #2584 ADR-1239 Codex-binding amendment, Phase 2: `worktree create` is the git-worktree-creation primitive for `dispatch.isolation: orchestrator-worktree` hosts — declared and testable but UNCONSUMED (no scheduler calls it yet; Phase 3 wires it). Source of truth: `gsd-core/bin/lib/worktree-safety.cjs`. Timeout path: all git subprocess calls are bounded; callers receive `ok:false, reason:'git_timed_out'` rather than a thrown exception. Test anchor: `tests/worktree-safety.test.cjs`. The `core.cjs` re-export spine was retired in epic #1267: this module absorbed the two thin compositional wrappers that squatted in Core — `resolveWorktreeRoot(cwd, deps)` (a projection over `resolveWorktreeContext`) and `pruneOrphanedWorktrees(...)` (sequences `planWorktreePrune` + `executeWorktreePrunePlan` with a timeout warning) — so callers reach this single worktree-lifecycle seam directly. `gitWorktreeInfoInternal` did NOT move here — worktree-info detection belongs to the Git Query Module.

View File

@@ -453,6 +453,7 @@
"uat.cjs", "uat.cjs",
"ui-consideration-probe.cjs", "ui-consideration-probe.cjs",
"ui-safety-gate.cjs", "ui-safety-gate.cjs",
"unusable-input.cjs",
"update-context.cjs", "update-context.cjs",
"validate-command-router.cjs", "validate-command-router.cjs",
"validate.cjs", "validate.cjs",

View File

@@ -40,6 +40,32 @@ GSD detects this at the next run and surfaces a safe-resume gate with three opti
- **Re-execute from scratch** — revert or supersede the partial commits before dispatching a new executor. - **Re-execute from scratch** — revert or supersede the partial commits before dispatching a new executor.
- **Mark-and-skip** — record the anomaly and continue, only with your explicit confirmation. - **Mark-and-skip** — record the anomaly and continue, only with your explicit confirmation.
### If you see a "frontmatter opens with `---` but never closes" warning
```
gsd: warning — /path/.planning/STATE.md: frontmatter opens with "---" but never closes; metadata was NOT applied. (#1879)
```
The named file was written only partly — typically a crash, a full disk, or a killed process
between the opening fence and the closing one. GSD read it as having **no** metadata and carried
on, so any phase, plan, or state value that file was supposed to supply is missing from this run.
1. Open the named file and check whether its frontmatter block is closed by a `---` line of its
own.
2. If it is truncated, restore it — `git checkout -- <file>` if the file is tracked and the good
version is committed, or re-add the missing fields and the closing `---` by hand.
3. Re-run the command that produced the warning. The warning is reported once per file per run,
so a silent re-run means the file now reads cleanly.
The warning cannot be turned off, and it never changes what a command returns or its exit code —
it only tells you a file could not be used.
**A file can still be truncated without this warning.** GSD stays silent when the partial block
carries fewer than two fields, or when the text after the opening `---` reads as prose rather than
frontmatter, because a Markdown document that opens with a horizontal rule is indistinguishable
from one of those. If a run behaves as though metadata is missing and you see no warning, inspect
the file anyway.
--- ---
## Diagnose the root cause ## Diagnose the root cause

View File

@@ -72,6 +72,7 @@ export default tseslint.config(
'gsd-core/bin/lib/capability-consent.cjs', 'gsd-core/bin/lib/capability-consent.cjs',
'gsd-core/bin/lib/capability-lock.cjs', 'gsd-core/bin/lib/capability-lock.cjs',
'gsd-core/bin/lib/resolution.cjs', 'gsd-core/bin/lib/resolution.cjs',
'gsd-core/bin/lib/unusable-input.cjs',
'gsd-core/bin/lib/plan-drift-guard.cjs', 'gsd-core/bin/lib/plan-drift-guard.cjs',
'gsd-core/bin/lib/cli-exit.cjs', 'gsd-core/bin/lib/cli-exit.cjs',
'gsd-core/bin/lib/external-job.cjs', 'gsd-core/bin/lib/external-job.cjs',

View File

@@ -241,7 +241,7 @@ function beginPhaseCore(content, intent, deps) {
// #1255: body-field replacements operate on body only (frontmatter stripped), // #1255: body-field replacements operate on body only (frontmatter stripped),
// not on the full content. The YAML `status:` key matches `^Status:\s*` // not on the full content. The YAML `status:` key matches `^Status:\s*`
// before the body pipe-table row if full content is passed. // before the body pipe-table row if full content is passed.
const existingFm = extractFrontmatter(content); const existingFm = extractFrontmatter(content, deps.sourcePath);
const hasFrontmatter = Object.keys(existingFm).length > 0; const hasFrontmatter = Object.keys(existingFm).length > 0;
let body = stripFrontmatter(content); let body = stripFrontmatter(content);
const reassemble = (b) => hasFrontmatter const reassemble = (b) => hasFrontmatter
@@ -523,7 +523,7 @@ function advancePlanCore(content, deps) {
// not on the full content. The YAML `status:` key matches `^Status:\s*` // not on the full content. The YAML `status:` key matches `^Status:\s*`
// before the body field if full content is passed (codex Phase 2 review: // before the body field if full content is passed (codex Phase 2 review:
// HIGH blocking finding — same pattern beginPhaseCore already handles). // HIGH blocking finding — same pattern beginPhaseCore already handles).
const existingFm = extractFrontmatter(content); const existingFm = extractFrontmatter(content, deps.sourcePath);
const hasFrontmatter = Object.keys(existingFm).length > 0; const hasFrontmatter = Object.keys(existingFm).length > 0;
let body = stripFrontmatter(content); let body = stripFrontmatter(content);
const reassemble = (b) => hasFrontmatter const reassemble = (b) => hasFrontmatter
@@ -645,7 +645,7 @@ function completePhaseCore(content, intent, deps) {
} }
// #1255: body-field replacements operate on body only (frontmatter stripped), // #1255: body-field replacements operate on body only (frontmatter stripped),
// so the YAML `status:` / `current_phase:` keys cannot shadow the body fields. // so the YAML `status:` / `current_phase:` keys cannot shadow the body fields.
const existingFm = extractFrontmatter(content); const existingFm = extractFrontmatter(content, deps.sourcePath);
const hasFrontmatter = Object.keys(existingFm).length > 0; const hasFrontmatter = Object.keys(existingFm).length > 0;
let body = stripFrontmatter(content); let body = stripFrontmatter(content);
const reassemble = (b) => hasFrontmatter const reassemble = (b) => hasFrontmatter
@@ -789,7 +789,7 @@ function plannedPhaseCore(content, intent, deps) {
} }
} }
// #1255: body-field replacements operate on body only. // #1255: body-field replacements operate on body only.
const existingFm = extractFrontmatter(content); const existingFm = extractFrontmatter(content, deps.sourcePath);
const hasFrontmatter = Object.keys(existingFm).length > 0; const hasFrontmatter = Object.keys(existingFm).length > 0;
let body = stripFrontmatter(content); let body = stripFrontmatter(content);
const reassemble = (b) => hasFrontmatter const reassemble = (b) => hasFrontmatter
@@ -880,7 +880,7 @@ function milestoneSwitchCore(content, intent, deps) {
'progress', 'progress',
'Current Position', 'Current Position',
]; ];
const existingFm = extractFrontmatter(content); const existingFm = extractFrontmatter(content, deps.sourcePath);
const body = stripFrontmatter(content); const body = stripFrontmatter(content);
const resolvedName = (intent.name && intent.name.trim()) || 'milestone'; const resolvedName = (intent.name && intent.name.trim()) || 'milestone';
// ## Current Position reset body (mirrors state.cts:2371-2375). // ## Current Position reset body (mirrors state.cts:2371-2375).
@@ -1025,7 +1025,7 @@ function milestoneCompleteCore(content, intent, deps) {
} }
} }
// #1255: body-field replacements operate on body only. // #1255: body-field replacements operate on body only.
const existingFm = extractFrontmatter(content); const existingFm = extractFrontmatter(content, deps.sourcePath);
const hasFrontmatter = Object.keys(existingFm).length > 0; const hasFrontmatter = Object.keys(existingFm).length > 0;
let body = stripFrontmatter(content); let body = stripFrontmatter(content);
const reassemble = (b) => hasFrontmatter const reassemble = (b) => hasFrontmatter
@@ -1351,7 +1351,9 @@ function rebuildCore(content, _intent, deps) {
let modified = content; let modified = content;
// §2 Decision: re-derive derived sections, preserve others. Order is // §2 Decision: re-derive derived sections, preserve others. Order is
// oldest-section-first so log entries appear in body order. // oldest-section-first so log entries appear in body order.
modified = reconcileCurrentPosition(modified, timestamp, log); // sourcePath threaded so `state rebuild --dry-run` names the file: that branch reads STATE.md
// directly rather than through readModifyWriteStateMd, so nothing upstream has named it yet.
modified = reconcileCurrentPosition(modified, timestamp, log, deps.sourcePath);
modified = reconcileByPhaseTable(modified, deps, timestamp, log); modified = reconcileByPhaseTable(modified, deps, timestamp, log);
modified = stripTemplatePlaceholders(modified, timestamp, log); modified = stripTemplatePlaceholders(modified, timestamp, log);
modified = deduplicateSessionArchive(modified, timestamp, log); modified = deduplicateSessionArchive(modified, timestamp, log);
@@ -1385,8 +1387,8 @@ function rebuildCore(content, _intent, deps) {
* the key (Leaky-Abstractions guard — don't synthesize values the canonical * the key (Leaky-Abstractions guard — don't synthesize values the canonical
* source doesn't have). * source doesn't have).
*/ */
function reconcileCurrentPosition(content, timestamp, log) { function reconcileCurrentPosition(content, timestamp, log, sourcePath) {
const fm = extractFrontmatter(content); const fm = extractFrontmatter(content, sourcePath);
if (!fm || typeof fm !== 'object') if (!fm || typeof fm !== 'object')
return content; return content;
let modified = content; let modified = content;

View File

@@ -145,6 +145,10 @@ const COVERED = {
tests: [ tests: [
'tests/frontmatter.property.test.cjs', 'tests/frontmatter.property.test.cjs',
'tests/frontmatter.unit.test.cjs', 'tests/frontmatter.unit.test.cjs',
// #1882 added the unterminated-fence detection to frontmatter.cjs, and the tests that
// constrain it live here. Without this entry the mutants in that branch are covered by
// no test in the shard, so the module's score drops even though the behaviour is tested.
'tests/unusable-input.test.cjs',
], ],
minScore: 62, minScore: 62,
}, },

View File

@@ -160,7 +160,7 @@ function scanDebugSessions(planDir: string): DebugSessionItem[] {
const content = platformReadSync(safeFilePath); const content = platformReadSync(safeFilePath);
if (content === null) continue; if (content === null) continue;
const fm = extractFrontmatter(content); const fm = extractFrontmatter(content, safeFilePath);
const status = ((fm.status as string) || 'unknown').toLowerCase(); const status = ((fm.status as string) || 'unknown').toLowerCase();
if (status === 'resolved' || status === 'complete') continue; if (status === 'resolved' || status === 'complete') continue;
@@ -246,7 +246,7 @@ function scanQuickTasks(planDir: string): QuickTaskItem[] {
if (content === null) { if (content === null) {
status = 'unreadable'; status = 'unreadable';
} else { } else {
const fm = extractFrontmatter(content); const fm = extractFrontmatter(content, safeSum);
status = ((fm.status as string) || 'unknown').toLowerCase(); status = ((fm.status as string) || 'unknown').toLowerCase();
} }
} }
@@ -309,7 +309,7 @@ function scanThreads(planDir: string): ThreadItem[] {
const content = platformReadSync(safeFilePath); const content = platformReadSync(safeFilePath);
if (content === null) continue; if (content === null) continue;
const fm = extractFrontmatter(content); const fm = extractFrontmatter(content, safeFilePath);
let status = ((fm.status as string) || '').toLowerCase().trim(); let status = ((fm.status as string) || '').toLowerCase().trim();
// Fall back to scanning body for ## Status: OPEN / IN PROGRESS // Fall back to scanning body for ## Status: OPEN / IN PROGRESS
@@ -378,7 +378,7 @@ function scanTodos(planDir: string): TodoItem[] {
const content = platformReadSync(safeFilePath); const content = platformReadSync(safeFilePath);
if (content === null) continue; if (content === null) continue;
const fm = extractFrontmatter(content); const fm = extractFrontmatter(content, safeFilePath);
// Extract first line of body after frontmatter // Extract first line of body after frontmatter
const bodyMatch = content.replace(/^---[\s\S]*?---\n?/, ''); const bodyMatch = content.replace(/^---[\s\S]*?---\n?/, '');
@@ -436,7 +436,7 @@ function scanSeeds(planDir: string): SeedItem[] {
const content = platformReadSync(safeFilePath); const content = platformReadSync(safeFilePath);
if (content === null) continue; if (content === null) continue;
const fm = extractFrontmatter(content); const fm = extractFrontmatter(content, safeFilePath);
const status = ((fm.status as string) || 'dormant').toLowerCase(); const status = ((fm.status as string) || 'dormant').toLowerCase();
if (!unimplementedStatuses.has(status)) continue; if (!unimplementedStatuses.has(status)) continue;
@@ -509,7 +509,7 @@ function scanUatGaps(planDir: string): UatGapItem[] {
const content = platformReadSync(safeFilePath); const content = platformReadSync(safeFilePath);
if (content === null) continue; if (content === null) continue;
const fm = extractFrontmatter(content); const fm = extractFrontmatter(content, safeFilePath);
const status = ((fm.status as string) || 'unknown').toLowerCase(); const status = ((fm.status as string) || 'unknown').toLowerCase();
const result = ((fm.result as string) || '').toLowerCase(); const result = ((fm.result as string) || '').toLowerCase();
@@ -579,7 +579,7 @@ function scanVerificationGaps(planDir: string): VerificationGapItem[] {
const content = platformReadSync(safeFilePath); const content = platformReadSync(safeFilePath);
if (content === null) continue; if (content === null) continue;
const fm = extractFrontmatter(content); const fm = extractFrontmatter(content, safeFilePath);
const status = ((fm.status as string) || 'unknown').toLowerCase(); const status = ((fm.status as string) || 'unknown').toLowerCase();
if (status !== 'gaps_found' && status !== 'human_needed') continue; if (status !== 'gaps_found' && status !== 'human_needed') continue;
@@ -641,7 +641,7 @@ function scanContextQuestions(planDir: string): ContextQuestionItem[] {
const content = platformReadSync(safeFilePath); const content = platformReadSync(safeFilePath);
if (content === null) continue; if (content === null) continue;
const fm = extractFrontmatter(content); const fm = extractFrontmatter(content, safeFilePath);
// Check frontmatter open_questions field // Check frontmatter open_questions field
let questions: string[] = []; let questions: string[] = [];

View File

@@ -149,12 +149,13 @@ function determinePhaseStatus(plans: number, summaries: number, phaseDir: string
const files = fs.readdirSync(phaseDir); const files = fs.readdirSync(phaseDir);
const verificationFile = files.find(f => f === 'VERIFICATION.md' || f.endsWith('-VERIFICATION.md')); const verificationFile = files.find(f => f === 'VERIFICATION.md' || f.endsWith('-VERIFICATION.md'));
if (verificationFile) { if (verificationFile) {
const content = platformReadSync(path.join(phaseDir, verificationFile)) || ''; const verificationFilePath = path.join(phaseDir, verificationFile);
const content = platformReadSync(verificationFilePath) || '';
// #1159 (Defect A): read ONLY the frontmatter `status` key to avoid false // #1159 (Defect A): read ONLY the frontmatter `status` key to avoid false
// matches from historical body metadata such as `previous_status: gaps_found`. // matches from historical body metadata such as `previous_status: gaps_found`.
// Full-text regexes like /status:\s*gaps_found/ match the substring inside // Full-text regexes like /status:\s*gaps_found/ match the substring inside
// `previous_status: gaps_found`, producing incorrect phase status labels. // `previous_status: gaps_found`, producing incorrect phase status labels.
const fm = extractFrontmatter(content) as Record<string, unknown>; const fm = extractFrontmatter(content, verificationFilePath) as Record<string, unknown>;
// Normalise to lower-case to preserve the prior case-insensitive behaviour // Normalise to lower-case to preserve the prior case-insensitive behaviour
// while reading only the frontmatter `status` key (not the full body text). // while reading only the frontmatter `status` key (not the full body text).
const fmStatus = typeof fm['status'] === 'string' ? fm['status'].trim().toLowerCase() : ''; const fmStatus = typeof fm['status'] === 'string' ? fm['status'].trim().toLowerCase() : '';
@@ -319,7 +320,7 @@ function cmdListSeeds(cwd: string, statusFilter: string | undefined, raw: boolea
const content = platformReadSync(safeFilePath); const content = platformReadSync(safeFilePath);
if (content === null) continue; if (content === null) continue;
const fm = extractFrontmatter(content) as Record<string, unknown>; const fm = extractFrontmatter(content, safeFilePath) as Record<string, unknown>;
const status = (fmStr(fm.status) || 'dormant').toLowerCase().trim() || 'dormant'; const status = (fmStr(fm.status) || 'dormant').toLowerCase().trim() || 'dormant';
// Match on the raw lowercased status (both sides already normalized); // Match on the raw lowercased status (both sides already normalized);
@@ -423,10 +424,11 @@ function cmdHistoryDigest(cwd: string, raw: boolean): void {
const summaries = fs.readdirSync(dirPath).filter(f => f.endsWith('-SUMMARY.md') || f === 'SUMMARY.md'); const summaries = fs.readdirSync(dirPath).filter(f => f.endsWith('-SUMMARY.md') || f === 'SUMMARY.md');
for (const summary of summaries) { for (const summary of summaries) {
const content = platformReadSync(path.join(dirPath, summary)); const summaryFilePath = path.join(dirPath, summary);
const content = platformReadSync(summaryFilePath);
if (content === null) continue; if (content === null) continue;
try { try {
const fm = extractFrontmatter(content) as Record<string, unknown>; const fm = extractFrontmatter(content, summaryFilePath) as Record<string, unknown>;
const phaseNum = (fm['phase'] as string) || dir.split('-')[0]; const phaseNum = (fm['phase'] as string) || dir.split('-')[0];
@@ -1365,7 +1367,7 @@ function cmdSummaryExtract(cwd: string, summaryPath: string | undefined, fields:
} }
const content = fs.readFileSync(fullPath, 'utf-8'); const content = fs.readFileSync(fullPath, 'utf-8');
const fm = extractFrontmatter(content) as Record<string, unknown>; const fm = extractFrontmatter(content, fullPath) as Record<string, unknown>;
// Parse key-decisions into structured format // Parse key-decisions into structured format
const parseDecisions = (decisionsList: unknown) => { const parseDecisions = (decisionsList: unknown) => {

View File

@@ -530,7 +530,13 @@ const _warnedUnusableConfig = new Set<string>();
* silently discarded still gets no signal. That was the whole defect in #1880. * silently discarded still gets no signal. That was the whole defect in #1880.
*/ */
function _warnUnusableConfig(fault: ConfigFault): void { function _warnUnusableConfig(fault: ConfigFault): void {
const key = `${fault.path}${fault.reason}${fault.code}`; // The NUL separators are load-bearing: without them `path`+`reason`+`code` is bare
// concatenation and two distinct faults can key alike. They are written as escapes rather
// than literal 0x00 bytes because a literal NUL makes the whole file binary to file(1) and
// grep(1), which silently skipped it — RULESET.AUDIT.search-source-not-generated tells
// agents to search this exact source to confirm an invariant exists, and it was returning
// nothing. Same runtime string, still greppable.
const key = `${fault.path}\u0000${fault.reason}\u0000${fault.code}`;
if (_warnedUnusableConfig.has(key)) return; if (_warnedUnusableConfig.has(key)) return;
_warnedUnusableConfig.add(key); _warnedUnusableConfig.add(key);
const what = fault.reason === CONFIG_REASON.CONFIG_UNPARSEABLE const what = fault.reason === CONFIG_REASON.CONFIG_UNPARSEABLE

View File

@@ -12,6 +12,9 @@ import path from 'node:path';
import ioMod = require('./io.cjs'); import ioMod = require('./io.cjs');
const { output, error } = ioMod; const { output, error } = ioMod;
import { platformReadSync as safeReadFile, platformWriteSync } from './shell-command-projection.cjs'; import { platformReadSync as safeReadFile, platformWriteSync } from './shell-command-projection.cjs';
// eslint-disable-next-line @typescript-eslint/no-require-imports
import unusableInputMod = require('./unusable-input.cjs');
const { UNUSABLE_REASON, warnUnusableInput } = unusableInputMod;
// ─── Types ──────────────────────────────────────────────────────────────────── // ─── Types ────────────────────────────────────────────────────────────────────
@@ -52,18 +55,52 @@ function splitInlineArray(body: string): string[] {
return items; return items;
} }
function extractFrontmatter(content: string): Frontmatter { /**
* How many parsed keys an unterminated region must yield before it is reported as a
* truncated frontmatter rather than left alone as ordinary Markdown. See the rationale on
* `extractFrontmatter`; exported for tests so the boundary is asserted against the constant
* rather than a magic number duplicated in the suite.
*/
const UNTERMINATED_KEY_THRESHOLD = 2;
/**
* Does every non-empty line of an unterminated region look like frontmatter?
*
* The key count alone cannot separate a truncated write from ordinary Markdown, because a
* thematic break above a short labelled preamble parses as keys too:
*
* ---
* Author: Jane Doe
* Reviewed-by: John Smith
*
* Ordinary prose, and no second `---` anywhere.
*
* Raising the threshold only moves that boundary — two labelled lines are as common in prose as
* one. What actually distinguishes the two is what follows: a write interrupted part-way through
* a frontmatter block ends mid-block, so *every* line in the region is still frontmatter-shaped,
* whereas a document merely opening with a rule goes on to prose. So the region must be
* uniformly frontmatter-shaped AND carry enough keys to be worth reporting; either test alone
* has a false-positive class the other closes.
*/
function isFrontmatterShaped(region: string): boolean {
const lines = region.split(/\r?\n/).filter((line) => line.trim() !== '');
if (lines.length === 0) return false;
return lines.every((line) => (
/^\s*[a-zA-Z0-9_-]+:/.test(line) // key: value
|| /^\s*-\s+/.test(line) // - list item
|| /^\s+\S/.test(line) // indented continuation of a nested value
));
}
/**
* Parse one already-delimited YAML region into a Frontmatter object.
*
* Extracted from `extractFrontmatter` (#1882) so the truncation probe below and the real
* parse run the *same* parser. A second, simpler "does this look like YAML?" matcher would
* be a parallel surface that drifts — exactly the generative-fix-divergence class.
*/
function parseYamlRegion(yaml: string): Frontmatter {
const frontmatter: Frontmatter = {}; const frontmatter: Frontmatter = {};
// Match frontmatter only at byte 0 — a `---` block later in the document
// body (YAML examples, horizontal rules) must never be treated as frontmatter.
const headerEnd = content.startsWith('---\r\n') ? 5 : content.startsWith('---\n') ? 4 : -1;
if (headerEnd === -1) return frontmatter;
const closingLineStart = content.indexOf('\n---', headerEnd);
if (closingLineStart === -1) return frontmatter;
const yamlEnd = content[closingLineStart - 1] === '\r' ? closingLineStart - 1 : closingLineStart;
const yaml = content.slice(headerEnd, yamlEnd);
const lines = yaml.split(/\r?\n/); const lines = yaml.split(/\r?\n/);
// Stack to track nested objects: [{obj, key, indent}] // Stack to track nested objects: [{obj, key, indent}]
@@ -133,6 +170,62 @@ function extractFrontmatter(content: string): Frontmatter {
return frontmatter; return frontmatter;
} }
/**
* Extract frontmatter from a document.
*
* Returns `{}` when the document has no frontmatter — and, unchanged since #1882, also
* returns `{}` when the frontmatter fence was opened and never closed. That return value is
* deliberately preserved: ADR-1411's amendment requires the fallback to stay, because
* changing it would break callers that treat "absent" and "unusable" identically. What #1882
* adds is that the second case is no longer *silent*.
*
* The discriminator is the reason this is not simply "opened but never closed". A Markdown
* document whose first line is a thematic break (`---`) takes that exact branch, so flagging
* on the missing fence alone reports corruption on perfectly good Markdown. Instead the
* unterminated region is run through this module's own parser and reported only when it
* yields **two or more** keys.
*
* Two, not one, and the extra key is doing real work. A single `key: value` line is genuinely
* ambiguous: `---` followed by `Note: this is a paragraph.` — or `Author:`, `TODO:`, `See:` —
* is ordinary technical writing, a thematic break above a labelled line, and it parses as
* exactly one key. There is no textual signal that separates it from a write interrupted
* after its first key, so the threshold is set where the ambiguity ends. The cost is a false
* negative on a file truncated after exactly one key; the benefit is silence on a very common
* Markdown shape. That direction is deliberate and matches the choice already made at zero
* keys: a false positive on valid Markdown is worse than a missed edge, because the
* diagnostic is unconditional and cannot be turned off. Every GSD artefact this guards
* (STATE.md, PLAN.md, ROADMAP.md, SUMMARY.md, agent/command docs) carries two or more
* frontmatter keys, so the realistic interruption window stays covered.
*
* @param content Raw document text.
* @param sourcePath Optional resolved path, used to name the file in the diagnostic and to
* key its deduplication. Optional because this function has 50-odd call sites and several
* hold only an in-memory string; those dedup on a content digest instead.
*/
function extractFrontmatter(content: string, sourcePath?: string): Frontmatter {
// Match frontmatter only at byte 0 — a `---` block later in the document
// body (YAML examples, horizontal rules) must never be treated as frontmatter.
const headerEnd = content.startsWith('---\r\n') ? 5 : content.startsWith('---\n') ? 4 : -1;
if (headerEnd === -1) return {};
const closingLineStart = content.indexOf('\n---', headerEnd);
if (closingLineStart === -1) {
const region = content.slice(headerEnd);
const probe = parseYamlRegion(region);
if (Object.keys(probe).length >= UNTERMINATED_KEY_THRESHOLD && isFrontmatterShaped(region)) {
warnUnusableInput({
reason: UNUSABLE_REASON.FRONTMATTER_UNTERMINATED,
source: sourcePath,
content,
});
}
return {};
}
const yamlEnd = content[closingLineStart - 1] === '\r' ? closingLineStart - 1 : closingLineStart;
return parseYamlRegion(content.slice(headerEnd, yamlEnd));
}
/** /**
* Escape a string for emission inside a YAML double-quoted scalar (#1779). * Escape a string for emission inside a YAML double-quoted scalar (#1779).
* Backslash must be escaped first so the backslashes added for embedded quotes * Backslash must be escaped first so the backslashes added for embedded quotes
@@ -547,7 +640,9 @@ function cmdFrontmatterGet(cwd: string, filePath: string, field: string | undefi
const fullPath = path.isAbsolute(filePath) ? filePath : path.join(cwd, filePath); const fullPath = path.isAbsolute(filePath) ? filePath : path.join(cwd, filePath);
const content = safeReadFile(fullPath); const content = safeReadFile(fullPath);
if (!content) { output({ error: 'File not found', path: filePath }, raw, undefined); return; } if (!content) { output({ error: 'File not found', path: filePath }, raw, undefined); return; }
const fm = extractFrontmatter(content); // Pass the resolved path so a truncated file is named in the diagnostic and deduplicated
// per file rather than per content digest (#1882, ADR-1411 wiring clause).
const fm = extractFrontmatter(content, fullPath);
if (field) { if (field) {
const value = fm[field]; const value = fm[field];
if (value === undefined) { output({ error: 'Field not found', field }, raw, undefined); return; } if (value === undefined) { output({ error: 'Field not found', field }, raw, undefined); return; }
@@ -564,7 +659,9 @@ function cmdFrontmatterSet(cwd: string, filePath: string, field: string | undefi
const fullPath = path.isAbsolute(filePath) ? filePath : path.join(cwd, filePath); const fullPath = path.isAbsolute(filePath) ? filePath : path.join(cwd, filePath);
if (!fs.existsSync(fullPath)) { output({ error: 'File not found', path: filePath }, raw, undefined); return; } if (!fs.existsSync(fullPath)) { output({ error: 'File not found', path: filePath }, raw, undefined); return; }
const content = fs.readFileSync(fullPath, 'utf-8'); const content = fs.readFileSync(fullPath, 'utf-8');
const fm = extractFrontmatter(content); // Pass the resolved path so a truncated file is named in the diagnostic and deduplicated
// per file rather than per content digest (#1882, ADR-1411 wiring clause).
const fm = extractFrontmatter(content, fullPath);
let parsedValue: unknown; let parsedValue: unknown;
try { parsedValue = JSON.parse(value as string); } catch { parsedValue = value; } try { parsedValue = JSON.parse(value as string); } catch { parsedValue = value; }
fm[field as string] = parsedValue as FrontmatterValue; fm[field as string] = parsedValue as FrontmatterValue;
@@ -603,7 +700,9 @@ function cmdFrontmatterMerge(cwd: string, filePath: string, data: string | undef
const fullPath = path.isAbsolute(filePath) ? filePath : path.join(cwd, filePath); const fullPath = path.isAbsolute(filePath) ? filePath : path.join(cwd, filePath);
if (!fs.existsSync(fullPath)) { output({ error: 'File not found', path: filePath }, raw, undefined); return; } if (!fs.existsSync(fullPath)) { output({ error: 'File not found', path: filePath }, raw, undefined); return; }
const content = fs.readFileSync(fullPath, 'utf-8'); const content = fs.readFileSync(fullPath, 'utf-8');
const fm = extractFrontmatter(content); // Pass the resolved path so a truncated file is named in the diagnostic and deduplicated
// per file rather than per content digest (#1882, ADR-1411 wiring clause).
const fm = extractFrontmatter(content, fullPath);
let mergeData: Record<string, FrontmatterValue>; let mergeData: Record<string, FrontmatterValue>;
try { mergeData = JSON.parse(data as string) as Record<string, FrontmatterValue>; } catch { error('Invalid JSON for --data'); return; } try { mergeData = JSON.parse(data as string) as Record<string, FrontmatterValue>; } catch { error('Invalid JSON for --data'); return; }
Object.assign(fm, mergeData); Object.assign(fm, mergeData);
@@ -619,7 +718,9 @@ function cmdFrontmatterValidate(cwd: string, filePath: string, schemaName: strin
const fullPath = path.isAbsolute(filePath) ? filePath : path.join(cwd, filePath); const fullPath = path.isAbsolute(filePath) ? filePath : path.join(cwd, filePath);
const content = safeReadFile(fullPath); const content = safeReadFile(fullPath);
if (!content) { output({ error: 'File not found', path: filePath }, raw, undefined); return; } if (!content) { output({ error: 'File not found', path: filePath }, raw, undefined); return; }
const fm = extractFrontmatter(content); // Pass the resolved path so a truncated file is named in the diagnostic and deduplicated
// per file rather than per content digest (#1882, ADR-1411 wiring clause).
const fm = extractFrontmatter(content, fullPath);
const missing = schema.required.filter(f => fm[f] === undefined); const missing = schema.required.filter(f => fm[f] === undefined);
const present = schema.required.filter(f => fm[f] !== undefined); const present = schema.required.filter(f => fm[f] !== undefined);
output({ valid: missing.length === 0, missing, present, schema: schemaName }, raw, missing.length === 0 ? 'valid' : 'invalid'); output({ valid: missing.length === 0, missing, present, schema: schemaName }, raw, missing.length === 0 ? 'valid' : 'invalid');
@@ -627,6 +728,7 @@ function cmdFrontmatterValidate(cwd: string, filePath: string, schemaName: strin
export = { export = {
extractFrontmatter, extractFrontmatter,
UNTERMINATED_KEY_THRESHOLD,
// Additive alias (#644 prohibition-probe schema contract): the probe round-trip seam reads a // Additive alias (#644 prohibition-probe schema contract): the probe round-trip seam reads a
// frontmatter object via `parseFrontmatter` (the name the contract test pins). It is the SAME // frontmatter object via `parseFrontmatter` (the name the contract test pins). It is the SAME
// function as `extractFrontmatter` — a bare-object parse with no behavior change — exposed under // function as `extractFrontmatter` — a bare-object parse with no behavior change — exposed under

View File

@@ -2536,8 +2536,9 @@ function buildSkillManifest(cwd: string, skillsDir: string | null = null): Skill
// with explicit '/' separators rather than path.join). // with explicit '/' separators rather than path.join).
relPath: string, relPath: string,
content: string, content: string,
sourcePath?: string,
): boolean { ): boolean {
const frontmatter = extractFrontmatter(content); const frontmatter = extractFrontmatter(content, sourcePath);
const dirPart = relPath.replace(/\/SKILL\.md$/, ''); const dirPart = relPath.replace(/\/SKILL\.md$/, '');
const stem = dirPart.includes('/') ? dirPart.split('/').pop()! : dirPart; const stem = dirPart.includes('/') ? dirPart.split('/').pop()! : dirPart;
const name = (frontmatter['name'] as string) || stem; const name = (frontmatter['name'] as string) || stem;
@@ -2579,7 +2580,7 @@ function buildSkillManifest(cwd: string, skillsDir: string | null = null): Skill
const skillMdPath = path.join(rootPath, entry.name, 'SKILL.md'); const skillMdPath = path.join(rootPath, entry.name, 'SKILL.md');
const content = platformReadSync(skillMdPath); const content = platformReadSync(skillMdPath);
if (content !== null) { if (content !== null) {
if (pushSkillEntry(`${entry.name}/SKILL.md`, content)) skillCount++; if (pushSkillEntry(`${entry.name}/SKILL.md`, content, skillMdPath)) skillCount++;
} }
// Nested layout: <entry>/skills/<stem>/SKILL.md // Nested layout: <entry>/skills/<stem>/SKILL.md
@@ -2604,7 +2605,7 @@ function buildSkillManifest(cwd: string, skillsDir: string | null = null): Skill
// Use forward-slash separator explicitly so manifest paths are posix-style // Use forward-slash separator explicitly so manifest paths are posix-style
// on all platforms, matching the flat-layout behaviour above. // on all platforms, matching the flat-layout behaviour above.
const relPath = `${entry.name}/skills/${nested.name}/SKILL.md`; const relPath = `${entry.name}/skills/${nested.name}/SKILL.md`;
if (pushSkillEntry(relPath, nestedContent)) skillCount++; if (pushSkillEntry(relPath, nestedContent, nestedSkillMd)) skillCount++;
} }
} }

View File

@@ -317,8 +317,8 @@ function cmdRequirementsReadyIds(cwd: string, args: string[], raw: boolean): voi
siblingPlanFiles = []; siblingPlanFiles = [];
} }
const parseFrontmatterReqIds = (content: string): string[] => { const parseFrontmatterReqIds = (content: string, sourcePath?: string): string[] => {
const fm = extractFrontmatter(content); const fm = extractFrontmatter(content, sourcePath);
const fmReq = fm.requirements; const fmReq = fm.requirements;
if (Array.isArray(fmReq)) return fmReq.map((r) => String(r).trim()).filter(Boolean); if (Array.isArray(fmReq)) return fmReq.map((r) => String(r).trim()).filter(Boolean);
if (typeof fmReq === 'string') { if (typeof fmReq === 'string') {
@@ -346,7 +346,7 @@ function cmdRequirementsReadyIds(cwd: string, args: string[], raw: boolean): voi
continue; continue;
} }
const siblingReqIds = parseFrontmatterReqIds(siblingContent); const siblingReqIds = parseFrontmatterReqIds(siblingContent, siblingPath);
const siblingDeclaresId = siblingReqIds.some((id) => id.toLowerCase() === reqId.toLowerCase()); const siblingDeclaresId = siblingReqIds.some((id) => id.toLowerCase() === reqId.toLowerCase());
if (!siblingDeclaresId) continue; if (!siblingDeclaresId) continue;
@@ -594,7 +594,7 @@ function cmdMilestoneComplete(cwd: string, version: string, options: MilestoneCo
for (const s of summaries) { for (const s of summaries) {
try { try {
const content = fs.readFileSync(path.join(phasesDir, dir, s), 'utf-8'); const content = fs.readFileSync(path.join(phasesDir, dir, s), 'utf-8');
const fm = extractFrontmatter(content); const fm = extractFrontmatter(content, path.join(phasesDir, dir, s));
const rawOneLiner = fm['one-liner']; const rawOneLiner = fm['one-liner'];
const oneLiner = (typeof rawOneLiner === 'string' ? rawOneLiner : '') || extractOneLinerFromBody(content); const oneLiner = (typeof rawOneLiner === 'string' ? rawOneLiner : '') || extractOneLinerFromBody(content);
if (oneLiner) { if (oneLiner) {
@@ -734,7 +734,7 @@ function cmdMilestoneComplete(cwd: string, version: string, options: MilestoneCo
version, version,
nextMilestoneCommand: formatGsdSlash('new-milestone', resolveRuntime(cwd)) as string, nextMilestoneCommand: formatGsdSlash('new-milestone', resolveRuntime(cwd)) as string,
}, },
{ clock: realClock, progressProvider: () => null }, { clock: realClock, progressProvider: () => null, sourcePath: statePath },
); );
writeStateMd(statePath, result.content, cwd); writeStateMd(statePath, result.content, cwd);
} }

View File

@@ -621,7 +621,8 @@ function cmdPhasePlanIndex(cwd: string, phase: string, raw: boolean): void {
const planId = planFile.replace('-PLAN.md', '').replace('PLAN.md', ''); const planId = planFile.replace('-PLAN.md', '').replace('PLAN.md', '');
const planPath = path.join(phaseDir, planFile); const planPath = path.join(phaseDir, planFile);
const content = fs.readFileSync(planPath, 'utf-8'); const content = fs.readFileSync(planPath, 'utf-8');
const fm = extractFrontmatter(content); // Pass planPath so a truncated PLAN.md names the file in the #1882 diagnostic.
const fm = extractFrontmatter(content, planPath);
const xmlTasks = content.match(/<task[\s>]/gi) || []; const xmlTasks = content.match(/<task[\s>]/gi) || [];
const mdTasks = content.match(/##\s*Task\s*\d+/gi) || []; const mdTasks = content.match(/##\s*Task\s*\d+/gi) || [];
@@ -1735,13 +1736,14 @@ function cmdPhaseComplete(cwd: string, phaseNum: string, raw: boolean): void {
for (const file of phaseFiles.filter( for (const file of phaseFiles.filter(
(f) => f.includes('-VERIFICATION') && f.endsWith('.md'), (f) => f.includes('-VERIFICATION') && f.endsWith('.md'),
)) { )) {
const content = fs.readFileSync(path.join(phaseFullDir, file), 'utf-8'); const verificationFilePath = path.join(phaseFullDir, file);
const content = fs.readFileSync(verificationFilePath, 'utf-8');
// #1159 (Defect A): read ONLY the frontmatter `status` key to avoid false positives // #1159 (Defect A): read ONLY the frontmatter `status` key to avoid false positives
// from historical metadata in the file body (e.g. `previous_status: gaps_found`). // from historical metadata in the file body (e.g. `previous_status: gaps_found`).
// A full-text regex like /status: gaps_found/ matches the substring inside // A full-text regex like /status: gaps_found/ matches the substring inside
// `previous_status: gaps_found`, producing spurious warnings even when the // `previous_status: gaps_found`, producing spurious warnings even when the
// current frontmatter status is `passed`. // current frontmatter status is `passed`.
const verFm = extractFrontmatter(content) as Record<string, unknown>; const verFm = extractFrontmatter(content, verificationFilePath) as Record<string, unknown>;
// Normalise to lower-case so `status: Passed` (title-case) is not missed. // Normalise to lower-case so `status: Passed` (title-case) is not missed.
const verStatus = typeof verFm['status'] === 'string' ? verFm['status'].trim().toLowerCase() : ''; const verStatus = typeof verFm['status'] === 'string' ? verFm['status'].trim().toLowerCase() : '';
if (verStatus === 'human_needed') warnings.push(`${file}: needs human verification`); if (verStatus === 'human_needed') warnings.push(`${file}: needs human verification`);
@@ -2393,6 +2395,7 @@ function cmdPhaseComplete(cwd: string, phaseNum: string, raw: boolean): void {
clock: realClock, clock: realClock,
progressProvider: () => null, // completePhase derives progress from the roadmap, not disk progressProvider: () => null, // completePhase derives progress from the roadmap, not disk
roadmapProvider: () => roadmapContent, roadmapProvider: () => roadmapContent,
sourcePath: statePath,
}, },
); );
stateContent = completeResult.content; stateContent = completeResult.content;

View File

@@ -64,7 +64,7 @@ function isPlanSuperseded(planFullPath: string): boolean {
} catch { } catch {
return false; return false;
} }
const status = extractFrontmatter(content)['status']; const status = extractFrontmatter(content, planFullPath)['status'];
return typeof status === 'string' && status.trim().toLowerCase() === 'superseded'; return typeof status === 'string' && status.trim().toLowerCase() === 'superseded';
} }

View File

@@ -781,7 +781,7 @@ function cmdRoadmapAnnotateDependencies(cwd: string, phaseNum: string | null | u
const planPath = path.join(path.resolve(cwd, phaseInfo.directory), planFile); const planPath = path.join(path.resolve(cwd, phaseInfo.directory), planFile);
try { try {
const content = fs.readFileSync(planPath, 'utf-8'); const content = fs.readFileSync(planPath, 'utf-8');
const fm = extractFrontmatter(content); const fm = extractFrontmatter(content, planPath);
const wave = parseInt(fm.wave as string, 10) || 1; const wave = parseInt(fm.wave as string, 10) || 1;
const planId = planFile.replace(/-PLAN\.md$/i, '').replace(/PLAN\.md$/i, ''); const planId = planFile.replace(/-PLAN\.md$/i, '').replace(/PLAN\.md$/i, '');
const truths = parseMustHavesBlock(content, 'truths') || []; const truths = parseMustHavesBlock(content, 'truths') || [];

View File

@@ -280,7 +280,7 @@ function readStateFile(statePath: string): {
} catch { } catch {
return null; return null;
} }
const fm = extractFrontmatter(content) as Record<string, unknown>; const fm = extractFrontmatter(content, statePath) as Record<string, unknown>;
const body = content.replace(/^---[\s\S]*?---\s*/, ''); const body = content.replace(/^---[\s\S]*?---\s*/, '');
return { fm, body }; return { fm, body };
} }

View File

@@ -300,6 +300,13 @@ export type StateTransitionDeps = {
* guard — the core stays pure and testable without disk I/O). * guard — the core stays pure and testable without disk I/O).
*/ */
phaseInventoryProvider?: () => PhaseInventoryRecord[] | null; phaseInventoryProvider?: () => PhaseInventoryRecord[] | null;
/**
* Resolved path of the STATE.md the content was read from. Injected (not
* derived) so the core stays pure; used only to name the file in an
* unusable-input frontmatter diagnostic (#1882). Optional: existing
* callers and test stubs are unaffected when absent.
*/
sourcePath?: string;
}; };
/** /**
@@ -438,7 +445,7 @@ function beginPhaseCore(
// #1255: body-field replacements operate on body only (frontmatter stripped), // #1255: body-field replacements operate on body only (frontmatter stripped),
// not on the full content. The YAML `status:` key matches `^Status:\s*` // not on the full content. The YAML `status:` key matches `^Status:\s*`
// before the body pipe-table row if full content is passed. // before the body pipe-table row if full content is passed.
const existingFm = extractFrontmatter(content) as Record<string, unknown>; const existingFm = extractFrontmatter(content, deps.sourcePath) as Record<string, unknown>;
const hasFrontmatter = Object.keys(existingFm).length > 0; const hasFrontmatter = Object.keys(existingFm).length > 0;
let body = stripFrontmatter(content); let body = stripFrontmatter(content);
@@ -751,7 +758,7 @@ function advancePlanCore(content: string, deps: StateTransitionDeps): StateTrans
// not on the full content. The YAML `status:` key matches `^Status:\s*` // not on the full content. The YAML `status:` key matches `^Status:\s*`
// before the body field if full content is passed (codex Phase 2 review: // before the body field if full content is passed (codex Phase 2 review:
// HIGH blocking finding — same pattern beginPhaseCore already handles). // HIGH blocking finding — same pattern beginPhaseCore already handles).
const existingFm = extractFrontmatter(content) as Record<string, unknown>; const existingFm = extractFrontmatter(content, deps.sourcePath) as Record<string, unknown>;
const hasFrontmatter = Object.keys(existingFm).length > 0; const hasFrontmatter = Object.keys(existingFm).length > 0;
let body = stripFrontmatter(content); let body = stripFrontmatter(content);
const reassemble = (b: string): string => const reassemble = (b: string): string =>
@@ -898,7 +905,7 @@ function completePhaseCore(
// #1255: body-field replacements operate on body only (frontmatter stripped), // #1255: body-field replacements operate on body only (frontmatter stripped),
// so the YAML `status:` / `current_phase:` keys cannot shadow the body fields. // so the YAML `status:` / `current_phase:` keys cannot shadow the body fields.
const existingFm = extractFrontmatter(content) as Record<string, unknown>; const existingFm = extractFrontmatter(content, deps.sourcePath) as Record<string, unknown>;
const hasFrontmatter = Object.keys(existingFm).length > 0; const hasFrontmatter = Object.keys(existingFm).length > 0;
let body = stripFrontmatter(content); let body = stripFrontmatter(content);
const reassemble = (b: string): string => const reassemble = (b: string): string =>
@@ -1059,7 +1066,7 @@ function plannedPhaseCore(
} }
// #1255: body-field replacements operate on body only. // #1255: body-field replacements operate on body only.
const existingFm = extractFrontmatter(content) as Record<string, unknown>; const existingFm = extractFrontmatter(content, deps.sourcePath) as Record<string, unknown>;
const hasFrontmatter = Object.keys(existingFm).length > 0; const hasFrontmatter = Object.keys(existingFm).length > 0;
let body = stripFrontmatter(content); let body = stripFrontmatter(content);
const reassemble = (b: string): string => const reassemble = (b: string): string =>
@@ -1174,7 +1181,7 @@ function milestoneSwitchCore(
'Current Position', 'Current Position',
]; ];
const existingFm = extractFrontmatter(content) as Record<string, unknown>; const existingFm = extractFrontmatter(content, deps.sourcePath) as Record<string, unknown>;
const body = stripFrontmatter(content); const body = stripFrontmatter(content);
const resolvedName = (intent.name && intent.name.trim()) || 'milestone'; const resolvedName = (intent.name && intent.name.trim()) || 'milestone';
@@ -1332,7 +1339,7 @@ function milestoneCompleteCore(
} }
// #1255: body-field replacements operate on body only. // #1255: body-field replacements operate on body only.
const existingFm = extractFrontmatter(content) as Record<string, unknown>; const existingFm = extractFrontmatter(content, deps.sourcePath) as Record<string, unknown>;
const hasFrontmatter = Object.keys(existingFm).length > 0; const hasFrontmatter = Object.keys(existingFm).length > 0;
let body = stripFrontmatter(content); let body = stripFrontmatter(content);
const reassemble = (b: string): string => const reassemble = (b: string): string =>
@@ -1730,7 +1737,9 @@ function rebuildCore(
// §2 Decision: re-derive derived sections, preserve others. Order is // §2 Decision: re-derive derived sections, preserve others. Order is
// oldest-section-first so log entries appear in body order. // oldest-section-first so log entries appear in body order.
modified = reconcileCurrentPosition(modified, timestamp, log); // sourcePath threaded so `state rebuild --dry-run` names the file: that branch reads STATE.md
// directly rather than through readModifyWriteStateMd, so nothing upstream has named it yet.
modified = reconcileCurrentPosition(modified, timestamp, log, deps.sourcePath);
modified = reconcileByPhaseTable(modified, deps, timestamp, log); modified = reconcileByPhaseTable(modified, deps, timestamp, log);
modified = stripTemplatePlaceholders(modified, timestamp, log); modified = stripTemplatePlaceholders(modified, timestamp, log);
modified = deduplicateSessionArchive(modified, timestamp, log); modified = deduplicateSessionArchive(modified, timestamp, log);
@@ -1771,8 +1780,9 @@ function reconcileCurrentPosition(
content: string, content: string,
timestamp: string, timestamp: string,
log: RebuildLogEntry[], log: RebuildLogEntry[],
sourcePath?: string,
): string { ): string {
const fm = extractFrontmatter(content) as Record<string, unknown>; const fm = extractFrontmatter(content, sourcePath) as Record<string, unknown>;
if (!fm || typeof fm !== 'object') return content; if (!fm || typeof fm !== 'object') return content;
let modified = content; let modified = content;

View File

@@ -518,6 +518,7 @@ function cmdStateAdvancePlan(cwd: string, raw: boolean): void {
const deps: StateTransitionDeps = { const deps: StateTransitionDeps = {
clock: realClock, clock: realClock,
progressProvider: () => null, progressProvider: () => null,
sourcePath: statePath,
}; };
let resultData: Record<string, unknown> | undefined; let resultData: Record<string, unknown> | undefined;
@@ -1364,7 +1365,9 @@ function cmdStateSnapshot(cwd: string, raw: boolean): void {
// Bug #3265: prefer YAML frontmatter for canonical scalar fields so that a // Bug #3265: prefer YAML frontmatter for canonical scalar fields so that a
// body table cell containing **Status:** Y cannot shadow the authoritative // body table cell containing **Status:** Y cannot shadow the authoritative
// frontmatter value. Mirrors the fix in sdk/src/query/state.ts. // frontmatter value. Mirrors the fix in sdk/src/query/state.ts.
const fm = extractFrontmatter(content) as Record<string, unknown>; // Pass statePath so a truncated STATE.md is named in the #1882 diagnostic rather than
// reported under a content digest — STATE.md is one of the artefacts epic #1879 is about.
const fm = extractFrontmatter(content, statePath) as Record<string, unknown>;
const body = stripFrontmatter(content); const body = stripFrontmatter(content);
// Helper: return frontmatter scalar value when present and non-empty. // Helper: return frontmatter scalar value when present and non-empty.
@@ -1766,7 +1769,12 @@ function buildStateFrontmatter(bodyContent: string, cwd: string | undefined): Re
function syncStateFrontmatter(content: string, cwd: string | undefined): string { function syncStateFrontmatter(content: string, cwd: string | undefined): string {
// Read existing frontmatter BEFORE stripping — it may contain values // Read existing frontmatter BEFORE stripping — it may contain values
// that the body no longer has (e.g., Status field removed by an agent). // that the body no longer has (e.g., Status field removed by an agent).
const existingFm = extractFrontmatter(content) as Record<string, unknown>; // `cwd` already identifies the workspace this content came from, so the STATE.md path is
// derivable here without widening the signature (#1882).
const existingFm = extractFrontmatter(
content,
cwd ? planningPaths(cwd).state : undefined,
) as Record<string, unknown>;
const body = stripFrontmatter(content); const body = stripFrontmatter(content);
const derivedFm = buildStateFrontmatter(body, cwd); const derivedFm = buildStateFrontmatter(body, cwd);
@@ -2135,7 +2143,7 @@ function readModifyWriteStateMd(statePath: string, transformFn: (content: string
const content = platformReadSync(statePath) || ''; const content = platformReadSync(statePath) || '';
// Snapshot the existing progress block BEFORE the transform so we can // Snapshot the existing progress block BEFORE the transform so we can
// restore it when resync is false. // restore it when resync is false.
const preFm = resync ? null : extractFrontmatter(content) as Record<string, unknown>; const preFm = resync ? null : extractFrontmatter(content, statePath) as Record<string, unknown>;
// Bug #1230: delta heuristic — snapshot pre-transform body source fields so // Bug #1230: delta heuristic — snapshot pre-transform body source fields so
// we can detect whether THIS write changed them. syncStateFrontmatter // we can detect whether THIS write changed them. syncStateFrontmatter
@@ -2148,7 +2156,7 @@ function readModifyWriteStateMd(statePath: string, transformFn: (content: string
// Strip frontmatter before calling stateExtractField so the YAML `status:` // Strip frontmatter before calling stateExtractField so the YAML `status:`
// key in the frontmatter block cannot shadow the body field we are tracking. // key in the frontmatter block cannot shadow the body field we are tracking.
const preBody = stripFrontmatter(content); const preBody = stripFrontmatter(content);
const preFmSnapshot = extractFrontmatter(content) as Record<string, unknown>; const preFmSnapshot = extractFrontmatter(content, statePath) as Record<string, unknown>;
const preBodyStatus = stateExtractField(preBody, 'Status'); const preBodyStatus = stateExtractField(preBody, 'Status');
// Bug #1230 / Change B: scope stopped_at delta to the ## Session section, // Bug #1230 / Change B: scope stopped_at delta to the ## Session section,
// mirroring buildStateFrontmatter's sessionBodyScope logic (line ~1172). // mirroring buildStateFrontmatter's sessionBodyScope logic (line ~1172).
@@ -2202,7 +2210,7 @@ function readModifyWriteStateMd(statePath: string, transformFn: (content: string
// one policy source, not three drifting encodings. Behavior-identical to // one policy source, not three drifting encodings. Behavior-identical to
// the pre-#1796 inline block; this is the absorption ADR-1769 / CONTEXT.md // the pre-#1796 inline block; this is the absorption ADR-1769 / CONTEXT.md
// already claimed shipped. // already claimed shipped.
const postFm = extractFrontmatter(synced) as Record<string, unknown>; const postFm = extractFrontmatter(synced, statePath) as Record<string, unknown>;
const preservation = applyStatePreservation({ const preservation = applyStatePreservation({
preFm, postFm, preFmSnapshot, resync, preFm, postFm, preFmSnapshot, resync,
deriveProgressKeys: options?.deriveProgressKeys === true, deriveProgressKeys: options?.deriveProgressKeys === true,
@@ -2230,7 +2238,7 @@ function cmdStateJson(cwd: string, raw: boolean): void {
} }
const content = fs.readFileSync(statePath, 'utf-8'); const content = fs.readFileSync(statePath, 'utf-8');
const existingFm = extractFrontmatter(content) as Record<string, unknown>; const existingFm = extractFrontmatter(content, statePath) as Record<string, unknown>;
const body = stripFrontmatter(content); const body = stripFrontmatter(content);
// Always rebuild from body + disk so progress counters reflect current state. // Always rebuild from body + disk so progress counters reflect current state.
@@ -2303,6 +2311,7 @@ function cmdStateBeginPhase(cwd: string, phaseNumber: string | number, phaseName
const deps: StateTransitionDeps = { const deps: StateTransitionDeps = {
clock: realClock, clock: realClock,
progressProvider: () => null, // beginPhase doesn't consult disk progress; syncStateFrontmatter's scan is authoritative progressProvider: () => null, // beginPhase doesn't consult disk progress; syncStateFrontmatter's scan is authoritative
sourcePath: statePath,
}; };
let updated: string[] = []; let updated: string[] = [];
@@ -2564,6 +2573,7 @@ function cmdStatePlannedPhase(cwd: string, phaseNumber: string | number, planCou
const deps: StateTransitionDeps = { const deps: StateTransitionDeps = {
clock: realClock, clock: realClock,
progressProvider: () => null, progressProvider: () => null,
sourcePath: statePath,
}; };
let updated: string[] = []; let updated: string[] = [];
@@ -2600,7 +2610,7 @@ function cmdStateMilestoneSwitch(cwd: string, version: string | undefined, name:
// milestoneSwitch rebuilds frontmatter directly and must not run the // milestoneSwitch rebuilds frontmatter directly and must not run the
// steady-state syncStateFrontmatter post-sync. // steady-state syncStateFrontmatter post-sync.
const intent: StateTransitionIntent = { kind: 'milestoneSwitch', version, name: resolvedName }; const intent: StateTransitionIntent = { kind: 'milestoneSwitch', version, name: resolvedName };
const deps: StateTransitionDeps = { clock: realClock, progressProvider: () => null }; const deps: StateTransitionDeps = { clock: realClock, progressProvider: () => null, sourcePath: statePath };
const lockPath = acquireStateLock(statePath); const lockPath = acquireStateLock(statePath);
try { try {
@@ -2813,7 +2823,7 @@ function cmdStateSync(cwd: string, options: StateSyncOptions | undefined, raw: b
// set — leave Progress untouched (percent=null) rather than silently writing // set — leave Progress untouched (percent=null) rather than silently writing
// fallback-derived wrong values. Projects without a milestone version (the common // fallback-derived wrong values. Projects without a milestone version (the common
// sync-test shape) are unaffected: the gate only fires when a version is asserted. // sync-test shape) are unaffected: the gate only fires when a version is asserted.
const fmVersion = (extractFrontmatter(content) as Record<string, unknown>).milestone; const fmVersion = (extractFrontmatter(content, statePath) as Record<string, unknown>).milestone;
const versionStr = typeof fmVersion === 'string' && fmVersion.trim() ? fmVersion.trim() : null; const versionStr = typeof fmVersion === 'string' && fmVersion.trim() ? fmVersion.trim() : null;
let milestoneBounded = true; let milestoneBounded = true;
if (versionStr !== null && syncRoadmapRaw !== null) { if (versionStr !== null && syncRoadmapRaw !== null) {
@@ -2877,7 +2887,7 @@ function cmdStatePrune(cwd: string, options: StatePruneOptions, raw: boolean): v
// the explicit `Current Phase` field are unambiguous, so they stay document-wide; // the explicit `Current Phase` field are unambiguous, so they stay document-wide;
// the shared extractor is not narrowed for any other caller. // the shared extractor is not narrowed for any other caller.
const rawState = fs.readFileSync(statePath, 'utf-8'); const rawState = fs.readFileSync(statePath, 'utf-8');
const fm = extractFrontmatter(rawState) as Record<string, unknown>; const fm = extractFrontmatter(rawState, statePath) as Record<string, unknown>;
const body = stripFrontmatter(rawState); const body = stripFrontmatter(rawState);
// Mirror buildStateFrontmatter's fmScalar: only string/number/boolean // Mirror buildStateFrontmatter's fmScalar: only string/number/boolean
// frontmatter scalars are usable (an object/array `current_phase` is ignored, // frontmatter scalars are usable (an object/array `current_phase` is ignored,
@@ -3019,6 +3029,12 @@ function cmdStateRebuild(cwd: string, options: StateRebuildOptions, raw: boolean
progressProvider: () => null, progressProvider: () => null,
clock: realClock, clock: realClock,
phaseInventoryProvider, phaseInventoryProvider,
// Without this, `state rebuild --dry-run` reported a truncated STATE.md anonymously: the
// write path is named only because readModifyWriteStateMd parses with the path first, and
// the dry-run branch reads the file directly and never does. Dry-run is the read-only mode
// an operator reaches for first when they suspect corruption, so it is the one that most
// needs to name the file (#1882).
sourcePath: statePath,
}; };
const runRebuild = (content: string) => transitionCore(content, { kind: 'rebuild' }, deps); const runRebuild = (content: string) => transitionCore(content, { kind: 'rebuild' }, deps);
@@ -3134,7 +3150,7 @@ function cmdStateCompletePhase(cwd: string, raw: boolean, overridePhase?: string
// Bug #1255: operate on body only so the YAML frontmatter `status:` key // Bug #1255: operate on body only so the YAML frontmatter `status:` key
// cannot shadow the body Status field (pipe-table or inline). // cannot shadow the body Status field (pipe-table or inline).
const existingFm = extractFrontmatter(content) as Record<string, unknown>; const existingFm = extractFrontmatter(content, statePath) as Record<string, unknown>;
const hasFrontmatter = Object.keys(existingFm).length > 0; const hasFrontmatter = Object.keys(existingFm).length > 0;
let body = stripFrontmatter(content); let body = stripFrontmatter(content);

View File

@@ -237,9 +237,10 @@ function evaluateUatPassed(
// ── Process UAT files ────────────────────────────────────────────────────── // ── Process UAT files ──────────────────────────────────────────────────────
for (const file of uatFileNames) { for (const file of uatFileNames) {
uatFiles.push(file); uatFiles.push(file);
const uatFilePath = path.join(phaseFullDir, file);
let raw = ''; let raw = '';
try { try {
raw = fs.readFileSync(path.join(phaseFullDir, file), 'utf-8'); raw = fs.readFileSync(uatFilePath, 'utf-8');
} catch { } catch {
blockers.push(`${file}: could not read file`); blockers.push(`${file}: could not read file`);
continue; continue;
@@ -254,7 +255,7 @@ function evaluateUatPassed(
blockers.push(`${file}: malformed markdown (unterminated fence or comment)`); blockers.push(`${file}: malformed markdown (unterminated fence or comment)`);
} }
const fm = extractFrontmatter(raw) as Record<string, unknown>; const fm = extractFrontmatter(raw, uatFilePath) as Record<string, unknown>;
// File-level frontmatter status check // File-level frontmatter status check
if (fm['status'] && BLOCKING_UAT_FM_STATUSES.has(fm['status'] as string)) { if (fm['status'] && BLOCKING_UAT_FM_STATUSES.has(fm['status'] as string)) {
@@ -288,15 +289,16 @@ function evaluateUatPassed(
let hasPassingVerification = false; let hasPassingVerification = false;
for (const file of verFileNames) { for (const file of verFileNames) {
verificationFiles.push(file); verificationFiles.push(file);
const verificationFilePath = path.join(phaseFullDir, file);
let raw = ''; let raw = '';
try { try {
raw = fs.readFileSync(path.join(phaseFullDir, file), 'utf-8'); raw = fs.readFileSync(verificationFilePath, 'utf-8');
} catch { } catch {
blockers.push(`${file}: could not read verification file`); blockers.push(`${file}: could not read verification file`);
continue; continue;
} }
const vfm = extractFrontmatter(raw) as Record<string, unknown>; const vfm = extractFrontmatter(raw, verificationFilePath) as Record<string, unknown>;
const vStatus = vfm['status'] as string | undefined; const vStatus = vfm['status'] as string | undefined;
if (vStatus && BLOCKING_VERIFICATION_FM_STATUSES.has(vStatus)) { if (vStatus && BLOCKING_VERIFICATION_FM_STATUSES.has(vStatus)) {

View File

@@ -98,7 +98,8 @@ function cmdAuditUat(cwd: string, raw: boolean): void {
// Process UAT files // Process UAT files
for (const file of files.filter(f => f.includes('-UAT') && f.endsWith('.md'))) { for (const file of files.filter(f => f.includes('-UAT') && f.endsWith('.md'))) {
const content = fs.readFileSync(path.join(phaseDir, file), 'utf-8'); const uatFilePath = path.join(phaseDir, file);
const content = fs.readFileSync(uatFilePath, 'utf-8');
const items = parseUatItems(content); const items = parseUatItems(content);
if (items.length > 0) { if (items.length > 0) {
results.push({ results.push({
@@ -107,7 +108,7 @@ function cmdAuditUat(cwd: string, raw: boolean): void {
file, file,
file_path: toPosixPath(path.relative(cwd, path.join(phaseDir, file))), file_path: toPosixPath(path.relative(cwd, path.join(phaseDir, file))),
type: 'uat', type: 'uat',
status: (extractFrontmatter(content).status as string || 'unknown'), status: (extractFrontmatter(content, uatFilePath).status as string || 'unknown'),
items, items,
}); });
} }
@@ -115,10 +116,11 @@ function cmdAuditUat(cwd: string, raw: boolean): void {
// Process VERIFICATION files // Process VERIFICATION files
for (const file of files.filter(f => f.includes('-VERIFICATION') && f.endsWith('.md'))) { for (const file of files.filter(f => f.includes('-VERIFICATION') && f.endsWith('.md'))) {
const content = fs.readFileSync(path.join(phaseDir, file), 'utf-8'); const verificationFilePath = path.join(phaseDir, file);
const status = extractFrontmatter(content).status as string || 'unknown'; const content = fs.readFileSync(verificationFilePath, 'utf-8');
const status = extractFrontmatter(content, verificationFilePath).status as string || 'unknown';
if (status === 'human_needed' || status === 'gaps_found') { if (status === 'human_needed' || status === 'gaps_found') {
const items = parseVerificationItems(content, status); const items = parseVerificationItems(content, status, verificationFilePath);
if (items.length > 0) { if (items.length > 0) {
results.push({ results.push({
phase: phaseNum, phase: phaseNum,
@@ -743,7 +745,7 @@ function rawGapEntryText(entryLines: string[]): string {
// ─── parseVerificationItems ─────────────────────────────────────────────────── // ─── parseVerificationItems ───────────────────────────────────────────────────
function parseVerificationItems(content: string, status: string): UatItem[] { function parseVerificationItems(content: string, status: string, sourcePath?: string): UatItem[] {
const items: UatItem[] = []; const items: UatItem[] = [];
if (status === 'human_needed') { if (status === 'human_needed') {
// #2286: the frontmatter's structured `human_verification:` YAML array // #2286: the frontmatter's structured `human_verification:` YAML array
@@ -752,7 +754,7 @@ function parseVerificationItems(content: string, status: string): UatItem[] {
// whose frontmatter declares the array doesn't require any particular // whose frontmatter declares the array doesn't require any particular
// `## Human Verification` body shape at all. An absent or empty array // `## Human Verification` body shape at all. An absent or empty array
// (length 0) falls back to the body scan unchanged. // (length 0) falls back to the body scan unchanged.
const frontmatter = extractFrontmatter(content); const frontmatter = extractFrontmatter(content, sourcePath);
const humanVerification = frontmatter.human_verification; const humanVerification = frontmatter.human_verification;
if (Array.isArray(humanVerification) && humanVerification.length > 0) { if (Array.isArray(humanVerification) && humanVerification.length > 0) {
humanVerification.forEach((entry, idx) => { humanVerification.forEach((entry, idx) => {

236
src/unusable-input.cts Normal file
View File

@@ -0,0 +1,236 @@
/**
* Unusable Input Diagnostic — the out-of-band half of ADR-1411's
* "corrupt is not absent" amendment (epic #1879).
*
* ADR-1411 splits the amendment's mechanism in two. Where a read already returns a
* provenance envelope, the cause is named *in-band* — that is `ConfigResolution.reason`
* (#1880, shipped). Where a read returns a bare sentinel or a plausible default it cannot
* extend, the return value is preserved exactly and the cause is surfaced *out-of-band*,
* as a deduplicated diagnostic on stderr. This module owns that second mechanism.
*
* It exists as a shared seam rather than a pattern copied per site because four call sites
* across four modules need identical behaviour (#1882 frontmatter, #1881 roadmap-parser,
* #1883 planning-workspace/verify, #1884 planning lock). Four hand-rolled copies of one
* behaviour is `DEFECT.GENERATIVE-FIX` by construction; one seam with a frozen reason set
* is the documented cure.
*
* Two contracts this module must not break:
*
* - **Unconditional.** ADR-1411 diverges deliberately from ADR-227's never-implemented
* `GSD_DEBUG` opt-in: "an opt-in nobody sets is indistinguishable from the silence
* #1879 is about". There is no config gate here, by design.
* - **Never throws.** Callers are leaf readers that promised a total function. A failed
* stderr write (closed stream, EPIPE) must not turn a silent degradation into a crash.
*/
import crypto from 'node:crypto';
// ─── Reason vocabulary ────────────────────────────────────────────────────────
/**
* Frozen so tests assert a typed surface instead of diagnostic prose
* (CONTRIBUTING.md — Prohibited: Raw Text Matching on Test Outputs).
*
* Adding a reason is three coordinated changes, matching the repo's `REASON`-enum
* convention: the entry here, the emitting call site, and the test that locks
* `Object.keys(UNUSABLE_REASON).sort()`. Each epic-#1879 phase adds only its own —
* pre-declaring the later phases' reasons would be speculative generality and would
* leave values no call site emits.
*/
const UNUSABLE_REASON = Object.freeze({
/**
* A file opened a `---` frontmatter fence at byte 0, carried at least one parseable
* key, and never closed the fence — a truncated or half-written file, NOT a file that
* legitimately has no frontmatter. (#1882)
*/
FRONTMATTER_UNTERMINATED: 'frontmatter_unterminated',
} as const);
type UnusableReason = (typeof UNUSABLE_REASON)[keyof typeof UNUSABLE_REASON];
/** One human-readable clause per reason. Prose lives here, never in a test assertion. */
const REASON_PROSE: Readonly<Record<UnusableReason, string>> = Object.freeze({
[UNUSABLE_REASON.FRONTMATTER_UNTERMINATED]:
'frontmatter opens with "---" but never closes; metadata was NOT applied',
});
// ─── Dedup state ──────────────────────────────────────────────────────────────
/**
* Process-lifetime dedup set. Mirrors `config-loader.cjs`'s `_warnedUnknownConfigKeys`
* guard, which ADR-1411 names as the precedent to reuse.
*/
const _warnedUnusableInputs = new Set<string>();
/**
* Count of diagnostics actually WRITTEN, which is not the same as the size of the dedup set:
* one emission records every key the input could later be identified by, so set size counts
* identities while this counts events. Tests assert on this because the behavioural claim is
* "how many diagnostics did the operator see", not "how many keys are interned".
*/
let _unusableInputEmissions = 0;
/**
* ASCII control characters (including NUL) are stripped from any path before it is used
* as a key component or written to a terminal. Two reasons, both real:
*
* - the key separator is NUL, so a `sourcePath` containing NUL could otherwise forge a
* collision with a different (path, reason) pair and suppress a genuine second failure;
* - a path carrying ANSI escapes would be replayed verbatim into the operator's terminal.
*/
const CONTROL_CHARS = /[\u0000-\u001F\u007F]/g;
/**
* Strip control characters. Deliberately does NOT normalize path separators.
*
* An earlier revision folded backslashes to `/` unconditionally, reasoning that `C:\a\b.md`
* and `C:/a/b.md` are one file and should not report twice. That is true on Windows, and
* false — destructively — everywhere else: `\` is a legal filename character on Linux and
* macOS, so `/repo/weird\name/PLAN.md` and `/repo/weird/name/PLAN.md` are two genuinely
* different files that collapsed to one key, and the second one's diagnostic was silently
* swallowed. ADR-1411 forbids exactly that ("keying too coarsely suppresses a genuine second
* failure in a different file"), and this repo targets Linux/macOS/Windows alike.
*
* The trade is now explicit and one-directional: two spellings of one Windows path may
* report twice (mild noise), but two distinct files can never silence each other (lost
* signal). Dropping a real diagnostic is the strictly worse failure.
*/
function sanitizeSource(source: string): string {
return source.replace(CONTROL_CHARS, '');
}
/**
* Identify the offending input. A path is preferred because it is what an operator can act
* on. When the caller has only an in-memory string, fall back to a short content digest so
* that *different* bad inputs still produce *different* keys.
*
* The leading `p`/`d` tag is what keeps the two namespaces disjoint. Without it a caller
* whose file is literally named `<unnamed:8efa5269728e7271>` would key identically to a
* path-less caller whose content happens to hash to that digest — no brute force required,
* since the digest of any predictable content (a shared template, known boilerplate) can
* simply be computed and used as a filename to pre-seed suppression. Because control
* characters — including NUL — are stripped from `source`, a caller-supplied path can never
* contain the separator and so can never forge a key in the other namespace either.
*
* The digest is computed only on the emission path, which is rare, so it never costs
* anything on a healthy read.
*/
function sourceKey(source?: string, content?: string): string {
if (typeof source === 'string' && source.trim() !== '') {
return `p\u0000${sanitizeSource(source)}`;
}
const digest = crypto.createHash('sha256').update(content ?? '').digest('hex').slice(0, 16);
return `d\u0000${digest}`;
}
/** Human-facing name for the offending input, derived from the same key. */
function displaySource(key: string): string {
return key.startsWith('p\u0000') ? key.slice(2) : `<unnamed:${key.slice(2)}>`;
}
// ─── Emission ─────────────────────────────────────────────────────────────────
interface WarnUnusableInputArgs {
/** Which unusable-input condition fired. */
reason: UnusableReason;
/** Resolved path of the offending file, when the caller has one. */
source?: string;
/** Raw content, used only to derive a dedup key when `source` is absent. */
content?: string;
}
/**
* Emit a deduplicated diagnostic naming an input that exists but cannot be used.
*
* The key is `<normalized source>\0<reason>`. ADR-1411 requires the resolved path AND the
* distinguishing cause — keying on the path alone would let a second, different fault on
* the same file go unreported; keying on the message prose would couple the guard to
* wording.
*
* @returns `true` when this call actually wrote a diagnostic, `false` when it was
* deduplicated. Returning the decision is what lets tests assert emission *counts* on a
* typed surface rather than scraping stderr.
*/
function warnUnusableInput({ reason, source, content }: WarnUnusableInputArgs): boolean {
// Defensive: an unknown reason must not emit a diagnostic with `undefined` in it.
const prose = Object.prototype.hasOwnProperty.call(REASON_PROSE, reason)
? REASON_PROSE[reason]
: null;
if (prose === null) return false;
// The guarantee, stated precisely, because it is not symmetric:
//
// * a file reported BY NAME is reported at most once, and
// * an anonymous re-parse of content already reported by name stays silent, and
// * two DIFFERENT files always both report, even when their truncated bytes are identical.
//
// The asymmetry is the anonymous-FIRST ordering (a path-less parse, then a named parse of the
// same content), which emits twice. That is a deliberate limit, not an oversight. A path-less
// caller cannot identify its file, so suppressing the later named report would also suppress a
// genuine second failure in a DIFFERENT file whenever two files share byte-identical truncated
// content — the over-coarse keying ADR-1411 explicitly forbids. Between a duplicate line and a
// swallowed diagnostic the ADR ranks the swallow worse, so the duplicate is accepted; and the
// second line is the more useful of the two, because it carries the filename.
//
// Mechanically: check ONLY the key matching what this caller actually knows, but record every
// key the input could later be identified by.
const identity = sourceKey(source, content);
const keys = [`${identity}\u0000${reason}`];
if (typeof content === 'string' && identity.startsWith('p\u0000')) {
keys.push(`${sourceKey(undefined, content)}\u0000${reason}`);
}
if (_warnedUnusableInputs.has(keys[0])) return false;
for (const k of keys) _warnedUnusableInputs.add(k);
try {
process.stderr.write(`gsd: warning — ${displaySource(identity)}: ${prose}. (#1879)\n`);
// Counted only after a write that actually completed. Incrementing before the try counted
// attempts, so on a broken stderr the counter claimed a diagnostic had reached the operator
// when nothing had — a seam documented as "written" reporting something else.
_unusableInputEmissions += 1;
} catch {
/* a closed or broken stderr must never escalate a degraded read into a crash */
}
return true;
}
// ─── Test seams ───────────────────────────────────────────────────────────────
/**
* Clear the dedup state between cases.
*
* This exists because the set is process-global: without it, the second test to use a key
* silently observes the first test's suppression. #2674 is the cautionary precedent — a
* reset helper that cleared two of three sets was a silent no-op for the very suite that
* existed to test it, and the cases only passed because each happened to pick a key no
* other case reused.
*/
function _resetUnusableInputWarningsForTests(): void {
_warnedUnusableInputs.clear();
_unusableInputEmissions = 0;
}
/** Number of diagnostics written — the typed surface tests assert on instead of stderr prose. */
function _unusableInputEmissionCountForTests(): number {
return _unusableInputEmissions;
}
/** Size of the dedup set (identities interned, not events). Retained for key-shape assertions. */
function _unusableInputWarningCountForTests(): number {
return _warnedUnusableInputs.size;
}
/** Test seam: the sanitized form of a source, so control-char handling is asserted on a
* returned value instead of by scraping what reached stderr. */
function _sanitizeSourceForTests(source: string): string {
return sanitizeSource(source);
}
export = {
UNUSABLE_REASON,
_sanitizeSourceForTests,
warnUnusableInput,
_resetUnusableInputWarningsForTests,
_unusableInputWarningCountForTests,
_unusableInputEmissionCountForTests,
};

View File

@@ -407,7 +407,7 @@ function readVerificationStatus(
let rawStatus: string | null = null; let rawStatus: string | null = null;
try { try {
const content = fsImpl.readFileSync(filePath, 'utf-8'); const content = fsImpl.readFileSync(filePath, 'utf-8');
const fm = extractFrontmatter(content); const fm = extractFrontmatter(content, filePath);
const statusVal = fm['status']; const statusVal = fm['status'];
// status is always a scalar string in a well-formed VERIFICATION.md frontmatter; // status is always a scalar string in a well-formed VERIFICATION.md frontmatter;
// only accept string values — arrays and objects are not valid status values. // only accept string values — arrays and objects are not valid status values.

View File

@@ -724,7 +724,7 @@ function cmdVerifyPlanStructure(cwd: string, filePath: string, raw: boolean): vo
return; return;
} }
const fm = extractFrontmatter(content); const fm = extractFrontmatter(content, fullPath);
const errors: string[] = []; const errors: string[] = [];
const warnings: string[] = []; const warnings: string[] = [];
@@ -1021,7 +1021,7 @@ function collectPromisedFilesAtOrAfterWave(phaseDir: string, minWave: number): S
const planFullPath = path.join(phaseDir, planFile); const planFullPath = path.join(phaseDir, planFile);
const planContent = safeReadFile(planFullPath); const planContent = safeReadFile(planFullPath);
if (!planContent) continue; if (!planContent) continue;
const fm = extractFrontmatter(planContent); const fm = extractFrontmatter(planContent, planFullPath);
const waveRaw = fm['wave']; const waveRaw = fm['wave'];
const wave = typeof waveRaw === 'string' ? parseInt(waveRaw, 10) : (typeof waveRaw === 'number' ? waveRaw : NaN); const wave = typeof waveRaw === 'string' ? parseInt(waveRaw, 10) : (typeof waveRaw === 'number' ? waveRaw : NaN);
if (isNaN(wave) || wave < minWave) continue; if (isNaN(wave) || wave < minWave) continue;
@@ -1056,7 +1056,7 @@ function cmdVerifyKeyLinks(cwd: string, planFilePath: string, raw: boolean): voi
// Derive the current plan's wave number and phase directory for wave-aware // Derive the current plan's wave number and phase directory for wave-aware
// missing-file handling (fix #1202). // missing-file handling (fix #1202).
const currentFm = extractFrontmatter(content); const currentFm = extractFrontmatter(content, fullPath);
const currentWaveRaw = currentFm['wave']; const currentWaveRaw = currentFm['wave'];
const currentWave = typeof currentWaveRaw === 'string' const currentWave = typeof currentWaveRaw === 'string'
? parseInt(currentWaveRaw, 10) ? parseInt(currentWaveRaw, 10)
@@ -1364,8 +1364,9 @@ function cmdValidateConsistency(cwd: string, raw: boolean): void {
} }
for (const plan of plans) { for (const plan of plans) {
const content = fs.readFileSync(path.join(phasePath, plan), 'utf-8'); const planFilePath = path.join(phasePath, plan);
const fmData = extractFrontmatter(content); const content = fs.readFileSync(planFilePath, 'utf-8');
const fmData = extractFrontmatter(content, planFilePath);
if (!fmData['wave']) { if (!fmData['wave']) {
warnings.push(`${phaseLabel}/${plan}: missing 'wave' in frontmatter`); warnings.push(`${phaseLabel}/${plan}: missing 'wave' in frontmatter`);
} }

View File

@@ -62,7 +62,7 @@ const UNMUTATED = [
// Full test command used by local runs and as the fallback when CI does not // Full test command used by local runs and as the fallback when CI does not
// inject a per-shard command via MUTATION_TEST_CMD. // inject a per-shard command via MUTATION_TEST_CMD.
// Keep this list in sync with the tests arrays in scripts/mutation-matrix.cjs COVERED. // Keep this list in sync with the tests arrays in scripts/mutation-matrix.cjs COVERED.
const DEFAULT_TEST_CMD = 'node --test tests/context-utilization.property.test.cjs tests/prompt-budget.property.test.cjs tests/frontmatter.property.test.cjs tests/adr-parser.property.test.cjs tests/config-schema.property.test.cjs tests/adr-parser.test.cjs tests/active-workstream-store.test.cjs tests/active-workstream-store.unit.test.cjs tests/prompt-budget.unit.test.cjs tests/adr-parser.unit.test.cjs tests/frontmatter.unit.test.cjs tests/core-utils.test.cjs tests/broken-windows.test.cjs'; const DEFAULT_TEST_CMD = 'node --test tests/context-utilization.property.test.cjs tests/prompt-budget.property.test.cjs tests/frontmatter.property.test.cjs tests/adr-parser.property.test.cjs tests/config-schema.property.test.cjs tests/adr-parser.test.cjs tests/active-workstream-store.test.cjs tests/active-workstream-store.unit.test.cjs tests/prompt-budget.unit.test.cjs tests/adr-parser.unit.test.cjs tests/frontmatter.unit.test.cjs tests/unusable-input.test.cjs tests/core-utils.test.cjs tests/broken-windows.test.cjs';
/** @type {import('@stryker-mutator/core').PartialStrykerOptions} */ /** @type {import('@stryker-mutator/core').PartialStrykerOptions} */
export default { export default {

View File

@@ -18,6 +18,7 @@ const assert = require('node:assert/strict');
const fs = require('fs'); const fs = require('fs');
const path = require('path'); const path = require('path');
const os = require('os'); const os = require('os');
const cp = require('node:child_process');
const { runGsdTools, parseFrontmatter } = require('./helpers.cjs'); const { runGsdTools, parseFrontmatter } = require('./helpers.cjs');
// Track temp files for cleanup // Track temp files for cleanup
@@ -526,3 +527,47 @@ describe('#1778: thread workflow uses the 1.6 named-flag frontmatter.set form',
); );
}); });
}); });
// ─── #1882: the user-reachable surface actually distinguishes the two cases ───
describe('frontmatter get — truncated vs absent frontmatter (#1882)', () => {
const TOOLS = path.join(__dirname, '..', 'gsd-core', 'bin', 'gsd-tools.cjs');
function runCapturingStderr(file) {
const r = cp.spawnSync(process.execPath, [TOOLS, 'frontmatter', 'get', file, '--raw'], {
encoding: 'utf8',
env: { ...process.env, GSD_TEST_MODE: '1' },
});
return { status: r.status, stdout: (r.stdout || '').trim(), stderr: (r.stderr || '').trim() };
}
// This is the wired keystone for #1882: the diagnostic is only "delivered" if it reaches
// the surface a user actually invokes. The assertion is a DIFFERENTIAL between two runs —
// whether stderr is empty — which is a behavioural claim, not a match against the message
// wording, so it stays inside CONTRIBUTING.md's ban on raw text matching.
test('a truncated file is reported while an absent-frontmatter file stays silent', () => {
const truncated = writeTempFile('---\nphase: 01\nplan: half-written\n');
const absent = writeTempFile('plain body with no frontmatter\n');
const bad = runCapturingStderr(truncated);
const good = runCapturingStderr(absent);
// The contract every one of the ~50 callers depends on is unchanged for both.
assert.strictEqual(bad.status, 0, 'truncated file must not change the exit code');
assert.strictEqual(good.status, 0);
assert.deepStrictEqual(JSON.parse(bad.stdout), {}, 'return value must be preserved');
assert.deepStrictEqual(JSON.parse(good.stdout), {});
// ...and the only difference is that corruption is no longer silent.
assert.notStrictEqual(bad.stderr, '', 'a truncated frontmatter must be reported');
assert.strictEqual(good.stderr, '', 'a file with no frontmatter is not corrupt');
});
test('a Markdown thematic break at byte 0 is not reported as corruption', () => {
const thematicBreak = writeTempFile('---\nSome heading text\n\nA paragraph, no more dashes.\n');
const r = runCapturingStderr(thematicBreak);
assert.strictEqual(r.status, 0);
assert.deepStrictEqual(JSON.parse(r.stdout), {});
assert.strictEqual(r.stderr, '', 'a horizontal rule is valid Markdown, not a truncated file');
});
});

View File

@@ -293,3 +293,48 @@ describe('frontmatter: reconstructFrontmatter strict-YAML property (#1779)', ()
); );
}); });
}); });
// (g)(h) #1882 added an optional `sourcePath` argument to extractFrontmatter, used only to
// name and deduplicate a diagnostic. These two properties are what protect the ~50 call
// sites: whatever the argument does, it must never reach the parsed result, and the
// LF/CRLF equivalence the parser already promised must survive the new branch.
describe('frontmatter: extractFrontmatter sourcePath is parse-inert (#1882)', () => {
test('property: the optional path argument never changes the parsed result', (t) => {
const original = process.stderr.write;
t.after(() => { process.stderr.write = original; });
process.stderr.write = () => true;
fc.assert(
fc.property(
fc.oneof(
fc.string({ maxLength: 300 }),
fc.string({ unit: 'binary', maxLength: 300 }),
),
fc.stringMatching(/^\/[a-z0-9/_-]{1,40}\.md$/),
(content, somePath) => {
assert.deepEqual(
extractFrontmatter(content, somePath),
extractFrontmatter(content),
'sourcePath must be inert with respect to the parsed value',
);
}
)
);
});
test('property: a document and its CRLF twin parse identically', (t) => {
const original = process.stderr.write;
t.after(() => { process.stderr.write = original; });
process.stderr.write = () => true;
fc.assert(
fc.property(fc.string({ maxLength: 300 }), (content) => {
const lf = content.replace(/\r\n/g, '\n');
const crlf = lf.replace(/\n/g, '\r\n');
assert.deepEqual(
extractFrontmatter(crlf),
extractFrontmatter(lf),
'CRLF and LF spellings of one document must parse the same',
);
})
);
});
});

View File

@@ -0,0 +1,455 @@
'use strict';
/**
* Unusable-input diagnostic — the out-of-band half of ADR-1411's "corrupt is not absent"
* amendment (epic #1879), and its first adopter, extractFrontmatter (#1882).
*
* What is under test is a BEHAVIOUR CHANGE ON A SILENT CHANNEL: every return value is
* preserved exactly, and the only observable difference is that a genuinely-unusable input
* now produces one diagnostic. So the assertions here are all on typed surfaces — the
* frozen reason enum and the dedup-set size — never on the diagnostic prose, per
* CONTRIBUTING.md's ban on raw text matching against stdout/stderr/file content.
*
* Independence note: the dedup set is process-global. Every case below resets it AND uses a
* path unique to that case. #2674 is the cautionary precedent in this repo — a reset helper
* that cleared two of three sets was a silent no-op for the very suite that existed to test
* it, and the cases only passed because each happened to pick a key no other case reused.
*/
const { describe, test } = require('node:test');
const assert = require('node:assert/strict');
const {
UNUSABLE_REASON,
warnUnusableInput,
_resetUnusableInputWarningsForTests,
_unusableInputWarningCountForTests,
_unusableInputEmissionCountForTests,
_sanitizeSourceForTests,
} = require('../gsd-core/bin/lib/unusable-input.cjs');
const { extractFrontmatter, UNTERMINATED_KEY_THRESHOLD } = require('../gsd-core/bin/lib/frontmatter.cjs');
/**
* Run `fn` with stderr captured, and report how many NEW diagnostics it produced.
*
* The count comes from the dedup set, not from parsing what was written — that is the typed
* surface. stderr is stubbed only to keep the suite's own output clean; the stub is restored
* in a `finally` inside this standalone helper, which is the one place CONTRIBUTING.md
* permits try/finally (a helper with no access to test context).
*/
function emissionsDuring(fn) {
const before = _unusableInputEmissionCountForTests();
const original = process.stderr.write;
process.stderr.write = () => true;
try {
fn();
} finally {
process.stderr.write = original;
}
return _unusableInputEmissionCountForTests() - before;
}
/** Parse `content` under a path unique to the calling case, returning [result, emissions]. */
function parseUnder(content, sourcePath) {
let result;
const emitted = emissionsDuring(() => {
result = extractFrontmatter(content, sourcePath);
});
return [result, emitted];
}
const TRUNCATED_LF = '---\nphase: 01\nplan: 02\n';
const TRUNCATED_CRLF = '---\r\nphase: 01\r\nplan: 02\r\n';
// ─── The reason vocabulary is a contract ─────────────────────────────────────
describe('UNUSABLE_REASON', () => {
test('is frozen and holds exactly the reasons that have an emitting call site', () => {
_resetUnusableInputWarningsForTests();
assert.ok(Object.isFrozen(UNUSABLE_REASON), 'enum must be frozen');
// Locking the key set is what makes adding a reason three coordinated changes
// (enum + call site + this assertion) instead of a silent widening.
assert.deepStrictEqual(Object.keys(UNUSABLE_REASON).sort(), ['FRONTMATTER_UNTERMINATED']);
assert.strictEqual(UNUSABLE_REASON.FRONTMATTER_UNTERMINATED, 'frontmatter_unterminated');
});
test('an unrecognised reason emits nothing rather than a diagnostic naming undefined', () => {
_resetUnusableInputWarningsForTests();
const emitted = emissionsDuring(() => {
const wrote = warnUnusableInput({ reason: 'not_a_real_reason', source: '/u/unknown.md' });
assert.strictEqual(wrote, false, 'unknown reason must report that it wrote nothing');
});
assert.strictEqual(emitted, 0);
});
});
// ─── The discriminator: truncated vs. everything that merely looks like it ───
describe('extractFrontmatter — flags a genuinely truncated frontmatter', () => {
test('unterminated fence carrying two keys is reported, and still returns {}', () => {
_resetUnusableInputWarningsForTests();
const [result, emitted] = parseUnder(TRUNCATED_LF, '/u/truncated-lf.md');
assert.deepStrictEqual(result, {}, 'return value must be preserved exactly');
assert.strictEqual(emitted, 1);
});
test('CRLF unterminated fence is reported identically to LF', () => {
_resetUnusableInputWarningsForTests();
const [result, emitted] = parseUnder(TRUNCATED_CRLF, '/u/truncated-crlf.md');
assert.deepStrictEqual(result, {});
assert.strictEqual(emitted, 1);
});
test('an indented "---" is not a closing fence, so the file is still truncated', () => {
_resetUnusableInputWarningsForTests();
const [result, emitted] = parseUnder('---\nphase: 01\nplan: 02\n ---\n', '/u/indented-close.md');
assert.deepStrictEqual(result, {});
assert.strictEqual(emitted, 1);
});
});
describe('extractFrontmatter — stays silent on everything that is not corruption', () => {
// Each row here is a document that reaches, or nearly reaches, the same branch as a
// truncated file. A diagnostic on any of them is a false positive on valid input.
const silentCases = [
['a document with no frontmatter at all', 'plain body\nmore\n'],
['a Markdown thematic break at byte 0', '---\nSome heading text\n\nA paragraph, no more dashes.\n'],
['a thematic break followed by a second one', '---\nIntro\n\n---\n\nMore\n'],
['a well-formed but empty frontmatter block', '---\n---\nbody\n'],
['an opening fence with nothing after it', '---\n'],
['a bare "---" with no newline', '---'],
['an empty document', ''],
['a BOM before the fence', '\uFEFF---\ntitle: x\n---\nbody\n'],
['a blank line before the fence', '\n---\ntitle: x\n---\n'],
['an opening fence with a trailing space', '--- \ntitle: x\n---\n'],
// #1882 review blocker: a thematic break above ONE labelled prose line is ordinary
// technical writing and parses as exactly one key. Each of these was flagged as
// corruption before the threshold moved to two.
['a thematic break above a Note: paragraph', '---\nNote: this is just a markdown paragraph, not frontmatter.\n'],
['a thematic break above an Author byline', '---\nAuthor: Jane Doe\n'],
['a thematic break above a TODO line', '---\nTODO: fix this later\n'],
['a thematic break above a See: link', '---\nSee: https://example.com\n'],
];
silentCases.forEach(([label, content], caseIndex) => {
test(`${label} produces no diagnostic`, () => {
_resetUnusableInputWarningsForTests();
const [, emitted] = parseUnder(content, `/u/silent-${caseIndex}.md`);
assert.strictEqual(emitted, 0, `${label} must not be reported as corruption`);
});
});
test('a thematic break above TWO labelled lines followed by prose stays silent', () => {
// Review finding: raising the key threshold to 2 only moved the boundary — two labelled
// lines are as common in ordinary prose as one. What separates a truncated write from a
// document opening with a rule is that a truncated write ends mid-block, so EVERY line is
// still frontmatter-shaped; this document goes on to prose.
_resetUnusableInputWarningsForTests();
const [, emitted] = parseUnder(
'---\nAuthor: Jane Doe\nReviewed-by: John Smith\n\nOrdinary prose, no other --- anywhere.\n',
'/u/two-labelled-lines.md',
);
assert.strictEqual(emitted, 0, 'a labelled preamble above prose is not a truncated file');
});
// The shape check decides whether an unterminated region reads as an interrupted frontmatter
// block or as a document that merely opened with a rule. These four pin each half of that
// decision independently; without them the predicate's individual branches are unconstrained
// and a mutation that drops any one of them still passes.
test('a blank line inside an interrupted block does not disqualify it', () => {
_resetUnusableInputWarningsForTests();
const [, emitted] = parseUnder('---\nphase: 01\n\nplan: 02\n', '/u/shape-blank-line.md');
assert.strictEqual(emitted, 1, 'blank lines are skipped, not treated as non-frontmatter');
});
test('an unindented list item counts as frontmatter-shaped', () => {
_resetUnusableInputWarningsForTests();
const [, emitted] = parseUnder('---\nphase: 01\nmust_haves:\n- alpha\n', '/u/shape-flat-list.md');
assert.strictEqual(emitted, 1);
});
test('an indented folded-scalar continuation counts as frontmatter-shaped', () => {
// This line is neither a key nor a list item, so it is the only case that exercises the
// indented-continuation branch on its own.
_resetUnusableInputWarningsForTests();
const [, emitted] = parseUnder('---\nphase: 01\ndescription: >\n folded text\n', '/u/shape-folded.md');
assert.strictEqual(emitted, 1);
});
test('two keys followed by prose is NOT frontmatter-shaped', () => {
// The negative half: enough keys to clear the threshold, but the region goes on to prose,
// so it is a document opening with a rule rather than an interrupted write.
_resetUnusableInputWarningsForTests();
const [, emitted] = parseUnder(
'---\nphase: 01\nplan: 02\n\nOrdinary prose sentence here.\n',
'/u/shape-then-prose.md',
);
assert.strictEqual(emitted, 0, 'key count alone must not be sufficient');
});
test('a truncated block whose values are nested lists is still reported', () => {
// The shape check must not reject legitimate frontmatter: list items and indented
// continuations are frontmatter-shaped too.
_resetUnusableInputWarningsForTests();
const [, emitted] = parseUnder('---\nphase: 01\nmust_haves:\n - alpha\n - beta\n', '/u/nested.md');
assert.strictEqual(emitted, 1);
});
test('a well-formed document still parses its keys and stays silent', () => {
_resetUnusableInputWarningsForTests();
let parsed;
const emitted = emissionsDuring(() => {
parsed = extractFrontmatter('---\ntitle: x\nstatus: draft\n---\nbody\n', '/u/well-formed.md');
});
assert.deepStrictEqual(parsed, { title: 'x', status: 'draft' });
assert.strictEqual(emitted, 0);
});
test('a four-dash close keeps its pre-existing lenient parse and stays silent', () => {
_resetUnusableInputWarningsForTests();
let parsed;
const emitted = emissionsDuring(() => {
parsed = extractFrontmatter('---\ntitle: x\n----\n', '/u/four-dash.md');
});
assert.deepStrictEqual(parsed, { title: 'x' });
assert.strictEqual(emitted, 0);
});
});
// ─── Boundary: the discriminator's threshold is ">= 2 parsed keys" ──────────
describe('extractFrontmatter — key-count boundary around the >=2 threshold', () => {
test('below threshold: zero keys is silent', () => {
_resetUnusableInputWarningsForTests();
const [, emitted] = parseUnder('---\njust prose, no colon\n', '/u/boundary-0.md');
assert.strictEqual(emitted, 0);
});
test('limit-1: exactly one key is silent — a labelled line under a thematic break', () => {
_resetUnusableInputWarningsForTests();
const [, emitted] = parseUnder('---\na: 1\n', '/u/boundary-1.md');
assert.strictEqual(emitted, 0, 'one key is ambiguous with ordinary Markdown');
});
test('limit: exactly two keys is reported', () => {
_resetUnusableInputWarningsForTests();
const [, emitted] = parseUnder('---\na: 1\nb: 2\n', '/u/boundary-2.md');
assert.strictEqual(emitted, UNTERMINATED_KEY_THRESHOLD - 1);
});
test('limit+1: three keys is reported exactly once, not once per key', () => {
_resetUnusableInputWarningsForTests();
const [, emitted] = parseUnder('---\na: 1\nb: 2\nc: 3\n', '/u/boundary-3.md');
assert.strictEqual(emitted, 1);
});
});
// ─── Deduplication: both halves of the composite key ────────────────────────
describe('diagnostic deduplication', () => {
test('the same file reported twice yields one diagnostic', () => {
_resetUnusableInputWarningsForTests();
const [, first] = parseUnder(TRUNCATED_LF, '/u/dedup-same.md');
const [, second] = parseUnder(TRUNCATED_LF, '/u/dedup-same.md');
assert.strictEqual(first, 1);
assert.strictEqual(second, 0, 'a repeat of the same fault must be suppressed');
});
test('a genuine second failure in a DIFFERENT file is never suppressed', () => {
_resetUnusableInputWarningsForTests();
const [, a] = parseUnder(TRUNCATED_LF, '/u/dedup-fileA.md');
const [, b] = parseUnder(TRUNCATED_LF, '/u/dedup-fileB.md');
assert.strictEqual(a, 1);
assert.strictEqual(b, 1, 'keying too coarsely would hide a real second fault');
});
test('the key includes the cause, so one file can report two different causes', () => {
_resetUnusableInputWarningsForTests();
const source = '/u/dedup-two-causes.md';
const emitted = emissionsDuring(() => {
const first = warnUnusableInput({
reason: UNUSABLE_REASON.FRONTMATTER_UNTERMINATED,
source,
});
const repeat = warnUnusableInput({
reason: UNUSABLE_REASON.FRONTMATTER_UNTERMINATED,
source,
});
assert.strictEqual(first, true);
assert.strictEqual(repeat, false, 'same (path, cause) must dedup');
});
assert.strictEqual(emitted, 1);
});
test('two spellings of one Windows path may report twice — the accepted trade', () => {
// Separator folding was REMOVED: it collapsed genuinely distinct POSIX files whose
// names contain a backslash. The residual cost is that one Windows file written two
// ways can report twice. Mild noise is strictly preferable to a swallowed diagnostic,
// and this test pins the direction of that trade so it is not silently reversed.
_resetUnusableInputWarningsForTests();
const [, backslash] = parseUnder(TRUNCATED_LF, 'C:\\proj\\phases\\PLAN.md');
const [, forward] = parseUnder(TRUNCATED_LF, 'C:/proj/phases/PLAN.md');
assert.strictEqual(backslash, 1);
assert.strictEqual(forward, 1, 'noise is acceptable; a lost diagnostic is not');
});
test('path-less callers dedup on content, so identical content reports once', () => {
_resetUnusableInputWarningsForTests();
const [, first] = parseUnder(TRUNCATED_LF, undefined);
const [, second] = parseUnder(TRUNCATED_LF, undefined);
assert.strictEqual(first, 1);
assert.strictEqual(second, 0);
});
test('path-less callers with DIFFERENT content each report', () => {
_resetUnusableInputWarningsForTests();
const [, first] = parseUnder('---\nalpha: 1\na2: x\n', undefined);
const [, second] = parseUnder('---\nbeta: 2\nb2: y\n', undefined);
assert.strictEqual(first, 1);
assert.strictEqual(second, 1);
});
test('an empty-string path falls back to the content key rather than keying on ""', () => {
_resetUnusableInputWarningsForTests();
const [, first] = parseUnder('---\ngamma: 1\ng2: x\n', ' ');
const [, second] = parseUnder('---\ndelta: 2\nd2: y\n', ' ');
assert.strictEqual(first, 1);
assert.strictEqual(second, 1, 'blank paths must not collapse distinct files into one key');
});
test('only the offending file is reported when a good file is parsed alongside it', () => {
_resetUnusableInputWarningsForTests();
const emitted = emissionsDuring(() => {
extractFrontmatter('---\nok: 1\n---\nbody\n', '/u/mixed-good.md');
extractFrontmatter(TRUNCATED_LF, '/u/mixed-bad.md');
extractFrontmatter('---\nalso: 2\n---\nbody\n', '/u/mixed-good-2.md');
});
assert.strictEqual(emitted, 1);
});
test('anonymous-first then named reports twice — the documented asymmetry', () => {
// Pinned deliberately. A path-less caller cannot identify its file, so suppressing the
// later NAMED report would also suppress a genuine second failure in a DIFFERENT file
// whenever two files share byte-identical truncated content. ADR-1411 ranks that swallow
// the worse failure, so the duplicate is accepted and the named line carries the filename.
_resetUnusableInputWarningsForTests();
const [, anonymous] = parseUnder(TRUNCATED_LF, undefined);
const [, named] = parseUnder(TRUNCATED_LF, '/u/anon-then-named.md');
assert.strictEqual(anonymous, 1);
assert.strictEqual(named, 1, 'the named report must still name the file');
});
test('one file parsed both with and without a path reports exactly once', () => {
// A read wrapper knows the path; a pure core downstream (state-transition.cts, per
// ADR-1769) is handed only the string. Keying those two parses separately reported the
// SAME truncated file twice, under a path key and a digest key.
_resetUnusableInputWarningsForTests();
const [, named] = parseUnder(TRUNCATED_LF, '/u/both-identities.md');
const [, anonymous] = parseUnder(TRUNCATED_LF, undefined);
assert.strictEqual(named, 1);
assert.strictEqual(anonymous, 0, 'the same file must not report twice under two keys');
});
test('widening the key does not merge two genuinely different files', () => {
_resetUnusableInputWarningsForTests();
const [, a] = parseUnder('---\nphase: 01\nplan: 02\n', '/u/widen-a.md');
const [, b] = parseUnder('---\nphase: 09\nplan: 09\n', '/u/widen-b.md');
assert.strictEqual(a, 1);
assert.strictEqual(b, 1, 'distinct content in distinct files must still both report');
});
test('the reset seam actually clears state, so the same key can report again', () => {
// #2674 shape: a reset that silently fails to clear turns every later dedup assertion
// into a vacuous pass. Prove the seam by re-reporting a key that was just suppressed.
_resetUnusableInputWarningsForTests();
const [, first] = parseUnder(TRUNCATED_LF, '/u/reset-seam.md');
const [, suppressed] = parseUnder(TRUNCATED_LF, '/u/reset-seam.md');
_resetUnusableInputWarningsForTests();
const [, afterReset] = parseUnder(TRUNCATED_LF, '/u/reset-seam.md');
assert.strictEqual(first, 1);
assert.strictEqual(suppressed, 0);
assert.strictEqual(afterReset, 1, 'reset must genuinely empty the dedup set');
assert.strictEqual(_unusableInputEmissionCountForTests(), 1,
'exactly one diagnostic was written after the reset');
assert.ok(_unusableInputWarningCountForTests() >= 1,
'and at least one identity was interned for it');
});
});
// ─── Hostile input ───────────────────────────────────────────────────────────
describe('hostile input', () => {
test('a literal backslash in a POSIX filename does not collide with a real directory', () => {
// Review finding: folding backslashes to '/' unconditionally made these two GENUINELY
// different files share one key on Linux/macOS, where '\\' is a legal filename
// character, and silently swallowed the second diagnostic.
_resetUnusableInputWarningsForTests();
const [, withBackslash] = parseUnder(TRUNCATED_LF, '/repo/weird\\name/PLAN.md');
const [, withSlash] = parseUnder(TRUNCATED_LF, '/repo/weird/name/PLAN.md');
assert.strictEqual(withBackslash, 1);
assert.strictEqual(withSlash, 1, 'two distinct files must never silence each other');
});
test('a path spelled like the unnamed-digest fallback cannot pre-seed suppression', () => {
// Review finding: the digest of any predictable content can be computed and used as a
// filename, so the two key namespaces must be disjoint by construction.
_resetUnusableInputWarningsForTests();
const crypto = require('node:crypto');
const digest = crypto.createHash('sha256').update(TRUNCATED_LF).digest('hex').slice(0, 16);
const [, forged] = parseUnder(TRUNCATED_LF, `<unnamed:${digest}>`);
const [, realFile] = parseUnder(TRUNCATED_LF, '/u/forge-victim.md');
assert.strictEqual(forged, 1);
assert.strictEqual(realFile, 1,
'a crafted filename must never suppress a real file reported by its own path');
// The anonymous re-report of byte-identical content IS suppressed, deliberately: that is
// the same-file guard (named read first, path-less re-parse second). The forged name buys
// an attacker nothing there, because ANY path-ful report of that content does the same.
});
test('a NUL in the path cannot forge a collision with another key', () => {
_resetUnusableInputWarningsForTests();
// The key separator is NUL. If it were not stripped, "a\0frontmatter_unterminated"
// supplied as a *path* would collide with the real key for path "a".
const [, forged] = parseUnder(TRUNCATED_LF, '/u/collide\u0000frontmatter_unterminated');
const [, genuine] = parseUnder(TRUNCATED_LF, '/u/collide');
assert.strictEqual(forged, 1);
assert.strictEqual(genuine, 1, 'a crafted path must not suppress a real report');
});
test('control characters are stripped from the source before it is used', () => {
// Asserted on the sanitizer's RETURN VALUE, not by capturing what reached stderr.
// Scraping the rendered stream and regex-testing it is the shape CONTRIBUTING.md bans
// (Prohibited: Raw Text Matching on Test Outputs) — the rule targets the mechanism,
// not just prose-wording checks, so the typed surface is the correct fix.
_resetUnusableInputWarningsForTests();
const cleaned = _sanitizeSourceForTests('/u/ansi\u001b[31mred\u0007\u0000.md');
assert.strictEqual(cleaned, '/u/ansi[31mred.md');
for (const ch of cleaned) {
assert.ok(ch.charCodeAt(0) > 31 && ch.charCodeAt(0) !== 127,
'sanitized source must contain no C0 or DEL bytes');
}
});
test('a large unterminated region completes without pathological behaviour', () => {
_resetUnusableInputWarningsForTests();
const big = '---\n' + Array.from({ length: 5000 }, (_, i) => `k${i}: v${i}`).join('\n') + '\n';
const [result, emitted] = parseUnder(big, '/u/large-unterminated.md');
assert.deepStrictEqual(result, {}, 'still returns the preserved sentinel');
assert.strictEqual(emitted, 1);
});
test('a failing stderr write is swallowed and never escalates into a throw', (t) => {
// Fault injection by method override + restore, never chmod 0o000: root bypasses mode
// bits, so a permission-based version of this test would silently pass with zero
// coverage in root Docker/CI.
_resetUnusableInputWarningsForTests();
const original = process.stderr.write;
t.after(() => { process.stderr.write = original; });
process.stderr.write = () => { throw new Error('EPIPE injected'); };
const result = extractFrontmatter(TRUNCATED_LF, '/u/broken-stderr.md');
assert.deepStrictEqual(result, {}, 'a broken stderr must not change the return value');
assert.strictEqual(_unusableInputEmissionCountForTests(), 0,
'a write that threw must not be counted as a diagnostic the operator saw');
});
});