4 Commits

Author	SHA1	Message	Date
Tom Boucher	9a76ca6783	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>	2026-07-27 16:50:12 -04:00
Tom Boucher	6140627f5c	fix(#2427): ground smart-entry completion in ROADMAP-derived counts + tighten status regex (#2466) ... * fix(#2427): ground smart-entry completion in ROADMAP-derived counts + tighten status regex Two coupled defects in isComplete (src/smart-entry.cts): 1. Two-scale comparison: isComplete compared global current_phase (from STATE.md body 'Phase: N') against milestone-scoped total_phases (from STATE.md frontmatter progress.total_phases, written once at milestone- switch time and going stale as soon as new phases are appended to the roadmap). When current_phase >= stale total_phases (e.g. 7 >= 4), isComplete tripped true even though later phases were still unchecked in ROADMAP.md. 2. Over-broad status regex: /\bcomplete(d)?|done|shipped\b/i matched any 'shipped' or 'done' substring — including per-phase status like 'Phase X shipped — PR #N' — and falsely satisfied the status side of the completion check. Fix: - Added two new SmartEntrySignals fields: roadmap_total_phases and roadmap_completed_phases, populated by calling the existing deriveProgressFromRoadmap helper (from phase-lifecycle.cts:60) when ROADMAP.md exists. These are global, authoritative counts from the Progress table — never stale. - isComplete now prefers the roadmap-derived counts when available (completed >= total) and falls back to the legacy STATE.md comparison only when the roadmap has no parseable Progress table (backward compat for fresh or non-standard projects). - Tightened the status regex to /\b(milestone\s+complete|all\s+phases\s+complete|complete(d)?)\b/i. Drops 'done' and 'shipped' (per-phase language). Keeps milestone-level signals per ADR-2207 (milestone complete, all phases complete) plus the legacy short form 'complete'/'completed'. Tests (tests/smart-entry.unit.test.cjs gains a #2427 describe block): - Mid-milestone with stale total_phases=4, current_phase=7, per-phase 'shipped' status, and 3 unchecked roadmap phases → NOT complete (the core bug scenario). - All roadmap phases complete classifies as complete even with stale cached total_phases (roadmap wins). - Per-phase 'shipped' or 'done' status alone does NOT satisfy completion when roadmap phases are unchecked. - Legacy fallback: empty roadmap (no Progress table) still classifies via STATE.md comparison (backward compat). The makeProject test helper now accepts a string for the 'roadmap' parameter (written verbatim) in addition to the boolean shorthand, so tests can supply a real Progress table. References: #2427; ADR-2207 (milestone status lifecycle); ADR-2143 (column-name-driven Progress table parsing via deriveProgressFromRoadmap); triage note that this is a read-side fix only (the milestone-switch write path in state.cjs is out of scope). * chore(#2427): backfill pr:2466 in .changeset/curious-rams-run.md	2026-07-20 17:26:20 -04:00
Tom Boucher	c1885df9e5	chore(#2143): prohibition-with-teeth + migrate remaining ad-hoc table sites — Phase 4 (final) (#2253) ... * chore(#2143): prohibition-with-teeth + migrate remaining table sites — Phase 4 Phase 4 of epic #2143 (ADR-2143 §7). Completes the markdown table/mutation consolidation by (a) giving the ad-hoc-parsing prohibition teeth and (b) migrating the last ad-hoc table sites onto the shared seam. - src/markdown-table.cts: new formatting-preserving `updateTableCell` primitive (self-contained, ragged-row-tolerant header/delimiter/cell-range scan; splices only the target cell's raw span, preserving all other bytes incl. padding/CRLF; no-op-preserves-padding when a transformer returns the current value). Exports splitTableRow/isDelimiterRow/findTableStartOffset for tolerant reuse. - eslint-rules/no-adhoc-markdown-parsing.cjs: TABLE-REGEX detector extended to `new RegExp(<literal|static-template>)`; new `.replace()`-mutation detector for roadmap/state/content receivers with a table/section-shaped pattern. - scripts/lint-table-schema-drift.cjs (wired into lint:ci): fails if a TABLE_SCHEMA header drifts from its authored table; tests import its logic (single source). - Migrated onto the seam (behaviour-preserving vs pre-Phase-4 HEAD, verified byte-diff old-vs-new): roadmap.cts cmdRoadmapUpdatePlanProgress, phase.cts cmdPhaseComplete + traceability, milestone.cts cmdRequirementsMarkComplete, uat.cts read path, state.cts metrics/decisions/By-Phase. - Incidental correctness gains from the migration: a decoy table can no longer swallow a phase-progress update (## Progress scoping); a ragged neighbouring row no longer silently aborts an edit; completing integer phase N no longer touches a decimal sub-phase N.x row; record-metric no longer drops trailing section content or duplicates the ## Performance Metrics section. - Kept justified allow-adhoc-markdown markers only where genuinely not a table (security.cts <|role|> token) or a loose non-GFM section (uat human-verify). Two orthogonal isolated reviews (correctness/adversarial + security) passed; correctness found 4 behaviour regressions in the first migration pass, all fixed and re-verified byte-identical-or-better vs OLD. Surfaced for maintainer (pre-existing, ambiguous domain logic, NOT changed here): templates/state.md places a By-Phase table under ## Performance Metrics while cmdStateRecordMetric assumes a Plan|Duration|Tasks|Files table. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(#2143): match traceability row by first-cell value, not Requirement header Phase 4's migration matched the REQUIREMENTS.md traceability row by a column literally named `Requirement` (`row['Requirement']`), but real tables head that column `REQ-ID`. The by-name lookup found nothing, so `phase complete` and `requirements mark-complete` left the Status cell `Pending` (regressed #2769 / #2203, caught by gsd-test — 8 failures, both node 22/24). - src/phase.cts, src/milestone.cts: match the row by its FIRST cell's value (the requirement-ID column) regardless of that column's HEADER name, via `Object.values(row)[0]` (updateTableCell builds the record in header order). This mirrors OLD's first-cell `\|\s*<id>\s*\|` anchor, restoring header-name independence while keeping the seam. - src/milestone.cts hasTable: broadened from `Requirement`-only to also recognize `Requirement ID` / `REQ-ID` / `REQ ID` headers, kept in sync with the now-positional rowMatch/hasRow so a REQ-ID-headed table participates in the ADR-2143 §6 write-set and the #2140 table_unmatched drift check (it was silently omitted before — a checkbox-only partial reconcile against a REQ-ID table could report as fully reconciled). The `Requirement`-headed path is byte-identical to OLD. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(#2143): replace stale structural milestone guards with behavioural suite The `milestone.cjs regex global state fix` block was a source-structure guard (allow-test-rule: structural-regression-guard) — it readFileSync'd the compiled milestone.cjs and asserted removed regex idioms (`tablePattern.test`, `afterTable !== reqContent`, `doneTable = new RegExp(...)`). Phase 4's migration deleted those regexes (table update is now updateTableCell), making the assertions obsolete. Per the Test Cleanup rule, replace them in-PR with a behavioural suite driving the compiled CLI: - multi-ID mark-complete flips all IDs (guards the lastIndex/global-state class), - Pending->Complete flip under both `REQ-ID` and `Requirement` headers (#2769), - idempotent already_complete detection with no corruption, - REQ-ID-headed table participates in write_set (traceability entry, applied), - REQ-ID-headed table trips #2140 table_unmatched drift on a missing row. Pruned the now-nonexistent structural-regression-guard entry from the lint-allow-test-rule-refs allowlist (the source-text-is-the-product entry for the same file remains valid). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * chore(changeset): backfill PR number 2253 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(#2143): record-metric targets its own metrics table, not By-Phase velocity `state record-metric` appended its per-plan row (`| Phase 1 P1 | 5min | 3 tasks | 4 files |`) into the FIRST table under `## Performance Metrics` — which on a real template-derived STATE.md is the By-Phase velocity table `| Phase | Plans | Total | Avg/Plan |`, polluting it on EVERY plan completion (execute-plan.md:414 is a per-plan call). The command's own metrics table is `| Plan | Duration | Tasks | Files |`, which the template does not ship, so the row never reached it; the scaffold branch also emitted a wrong `| Phase | Plan | Duration | Notes |` header matching neither the row nor the canonical table. Pre-existing (predates Phase 4); surfaced while migrating this site and fixed here per no-defer, on the user's explicit go-ahead. - src/state.cts cmdStateRecordMetric: locate the metrics table by its own header shape (`Plan|Duration|Tasks|Files`, via splitTableRow/isDelimiterRow) rather than "first table in the section". When the section exists but has no metrics table (only the By-Phase table), self-heal by appending a fresh **Per-Plan Metrics:** table to the END of the section body — By-Phase table, Recent Trend and footer preserved verbatim, no duplicate `## Performance Metrics` heading, created stays false. Absent-section scaffold header corrected to the canonical `| Plan | Duration | Tasks | Files |`. Ragged-tolerance + None-yet preserved. - Not touching templates/state.md (golden-install-parity hashed) — record-metric self-creates the table on first use instead. Failing-first regression test (tests/state.test.cjs) demonstrates the By-Phase pollution on the pre-fix build, then green after. Verified: no pollution, self- heal idempotency, both-tables isolation, content/heading preservation, flags, None-yet, corrected scaffold header (23-check adversarial harness + all existing record-metric scenarios). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(#2143): deleteSection seam primitive (level-bounded whole-section removal) ADR-2143 §4 shipped withSection/collectSection (replace a section BODY) but no way to DELETE a section (heading + body). Phase 4 suppressed the phase-remove section delete instead of building it. deleteSection(content, predicate, opts) locates the section via the collectSection machinery and splices out from the heading's start offset to the next same-or-higher-level heading — so a level-3 `### Phase N` delete stops at a following level-2 `## Progress`, never past it. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(#2143): phase remove no longer deletes ## Progress on last-phase removal updateRoadmapAfterPhaseRemoval deleted a `### Phase N` detail section with a greedy raw regex whose lazy scan, on the LAST phase, ran to EOF and destroyed the following `## Progress` heading and its entire tracking table — silent data loss, uncovered by tests (removal tests only exercised a middle phase). Migrated onto the new deleteSection seam (level-bounded, stops at `## Progress`); dropped the allow-adhoc-markdown SECTION-DELETION suppression. Failing-first regression (tests/phase.test.cjs) removes the LAST phase and asserts the ## Progress heading + table survive; middle-phase removal is byte-identical. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(#2143): deleteTableRow seam primitive (row removal, ragged-tolerant) Sibling of updateTableCell: locates the first GFM table, matches a DATA row by predicate (ragged-tolerant record build, header order), and splices out that row's whole line preserving every other byte. Returns {ok:false,reason} on no table / no match. Enables migrating the phase-remove Progress-table row delete off its ad-hoc regex (ADR-2143 §7 — the "future row-delete seam" Phase 4 punted). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(#2143): phase remove deletes the Progress row via deleteTableRow The Progress-table row delete used a whole-document regex with two defects: (a) `\.?\s` required whitespace after the phase number, so a COMPACT row `|2|Beta|` was never deleted (stale row left behind); (b) unscoped — it could strike a row in a different table (e.g. an earlier `| Phase | Requirements |` table). Migrated onto deleteTableRow, scoped to the `## Progress` section (mirrors deriveProgressFromRoadmap), matching the row by first-cell phase number (integer zero-pad-insensitive; decimal exact; removing `2` never touches `2.5`). Both allow-adhoc-markdown suppressions removed. New behavioural tests: compact unpadded row deleted; padded byte-parity on the surviving rows (their ordinal correctly renumbers via the pre-existing renumber block). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(#2143): deleteTableRow leaves no dangling newline on last EOL-less row Deleting the final row of a table with no trailing EOL sliced from the row's start to end-of-string, stranding the newline that terminated the previous line. Back rowStart over the preceding \r?\n in that branch so the table ends cleanly. (Caught by the primitive's own unit test on gsd-test; local scenario checks missed the no-trailing-EOL edge.) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(#2143): migrate read-only section-collects onto collectSection Six hand-rolled `## Section` read-extract regexes replaced by the collectSection seam (behaviour-preserving; extracted bodies feed the same downstream parsers): state.cts matchSessionSection (## Session / ## Session Continuity) + ## Blockers, smart-entry.cts ## Blockers, audit.cts ## Current Focus + ## Open Questions. Removes 6 allow-adhoc-markdown "pending #1372" suppressions. Incidental fix: the old Session regex `## Session[ \t]*\n` silently failed on a CRLF `## Session\r\n` heading (Windows STATE.md), nulling all session fields; collectSection is CRLF-safe, so session state now resolves on Windows. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(#2143): fence-safe state-transition section writes + dedup stripFrontmatter - milestoneCompleteCore's `## Current Position` and `## Operator Next Steps` section resets used fence-blind raw regexes that a fenced `##` inside the body could truncate/mis-target (#2130/#2067/#2080 class). Migrated onto a fence-aware tokenizeHeadings-based helper (resetSectionVerbatim) that is byte-identical to the old output on the canonical path (9/9 fixtures) and correctly ignores a fenced fake heading (proven robustness gain). - mutateCurrentPositionFirstTime: hand-rolled locate+splice → collectSection + replaceSection (byte-parity). - stripFrontmatter was inlined byte-identically in state.cts AND state-transition.cts; hoisted the single canonical copy into frontmatter.cts (both call sites now import it) + unit tests — eliminates the divergence risk per CLAUDE.md "Generative Fix Divergence". Removes 3 allow-adhoc-markdown / #1372 markers. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(#2143): name-address By-Phase sum + uat parse, eslint recall hole, catches - state.cts By-Phase "Total plans completed" sum: positional 2nd-cell regex → name-addressed splitTableRow read (correct on a reordered header, where the old code silently summed the wrong column). Marker removed. - uat.cts parseVerificationItems: loose pipe regex → splitTableRow within the existing table/numbered/bullet union scan (item list byte-identical; does NOT reintroduce the reverted strict-parseMarkdownTable item-drop). Marker removed. - eslint no-adhoc-markdown-parsing: close the `new RegExp(identifier)` recall hole — resolve a const-declared table-shaped regex identifier (mirrors the .replace() detector) + RuleTester cases; param/call args stay out (boundary). - commands.cts: delete a lying comment that claimed the scaffold date "stays on raw UTC / deferred" — #2136 already moved it to realClock.localToday(). - Empty catches (classified, not blind-swept): removed 4 dead try/catch; fixed 3 error-hiding (phase-insert decimal-dir I/O collision now fails loud; phase-remove rename partial-failure surfaced; milestone-archive true count via finally); left best-effort swallows with justification comments. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(#2143): extractFencedBlock seam + migrate api-coverage named fence parseCoverageMatrix extracted its ```coverage fenced block with an ad-hoc regex (the last real allow-adhoc-markdown suppression). Added extractFencedBlock to the markdown-sectionizer seam (reuses stripFencedCode's CommonMark fence engine — info-string match, ~~~/backtick, nesting, indent) and migrated onto it; byte- parity on the parsed CoverageMatrix across 8 fixtures. Only security.cts:367 (a genuine `<|role|>` protocol-token false-positive, not a GFM table) remains marked in src/ — the "prohibition with teeth" goal (nothing grandfathered but a true FP) is met. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(#2143): By-Phase row insert is name-addressed (insertTableRow seam) updatePerformanceMetricsSection's INSERT-new-row branch located the By-Phase table with a canonical-column-order-only regex + a hardcoded positional row literal, so on a reordered header it silently inserted nothing — inconsistent with the now name-addressed UPDATE and SUM halves of the same function. Added insertTableRow (markdown-table seam sibling of updateTableCell/deleteTableRow: name-addressed, header-order-agnostic, EOL-preserving) and migrated the branch onto it, mapping By-Phase values by column NAME. Canonical-order output is byte-identical; a reordered header now inserts a correctly-mapped row; a pre-existing CRLF mixed-EOL splice glitch is incidentally fixed. Retired the now-dead byPhaseTablePattern const. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(#2143): phase-list checkbox flip via updateBullet seam Added updateBullet (markdown-sectionizer): a fence-aware, offset-tracked single-bullet write primitive (GFM 1–4-space marker tolerance) — the write counterpart to read-only iterateBullets. Migrated mutateMilestonePhase's phase-list checkbox flip (`- [ ] Phase N …` → `- [x] … (completed <date>)`) off its whole-slice regex onto it, same milestone-slice scope + clock seam. Byte-identical across simple / idempotent / metachar-title / double-space / CRLF scenarios. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(#2143): scope the Progress-ordinal renumber to ## Progress via seam phase remove's integer-renumber decremented Progress-table phase ordinals with a whole-document `content.replace(/(\|\s*)(\d+)(\.\s)/g, …)` — unscoped, so it also rewrote any `| N. …` cell in an unrelated/decoy table (same class as the batch-2 row-delete scoping bug). Migrated onto updateTableCell, scoped to the ## Progress section, decrementing each affected row's leading phase ordinal by column name. Byte-identical on canonical Progress tables + multi-row + decimal-sibling cases; a decoy `| 3. … |` row before ## Progress is now correctly left untouched. The sibling heading / checkbox-bullet / PLAN.md-filename / Depends-on-prose renumbers are not GFM-table mutations (outside ADR-2143's table/section mandate) — left as-is. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(#2143): review fixes — scope traceability write, restore Current Position H3-stop Adversarial review of the remediation (BLOCK verdict) — all 9 findings fixed: - F1 (BLOCKER): requirements mark-complete / phase complete flipped the checkbox but NOT the traceability row on the shipped template, because updateTableCell bound to the FIRST table (## Out of Scope, no Status column) instead of the ## Traceability table — the #2140 silent-divergence class, re-introduced by the seam migration and missed by tests (fixtures had Traceability first). Scoped the write + hasRow probe to the ## Traceability section slice (updateTraceability Cell helper) in milestone.cts + phase.cts. Failing-first tests on the Out-of-Scope-before-Traceability layout; the #2769 first-cell match preserved. - F2 (MAJOR): mutateCurrentPositionFirstTime restored to locateCurrentPosition (STOP_H2_PLUS) — collectSection's default H2-stop swallowed a level-3 subsection and the field regexes clobbered it (#2130 class). - F3/F8: Progress-ordinal renumber re-escapes via escapeCell + keys padding recovery by row index (was de-escaping `\|` and losing padding on dup values). - F4: insertTableRow escapes cell values internally. - F5: updateBullet accepts a tab after the marker (`[ \t]{1,4}`). - F7: resetSectionVerbatim consumes CRLF blank lines (byte-parity on CRLF). - F6/F9: corrected two misleading comments. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * chore(changeset): data-loss + CRLF-session user-facing fixes (#2253) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(#2143): de-flake the G10 windsurf ReDoS-guard wall-clock assertion The G10 test asserted `elapsedMs < 1000` for a 200k-char payload — a wall-clock assertion (CLAUDE.md: never assert on wall-clock time) that flaked on a loaded node24 bench at ~1.1s. It was redundant: runHook's spawnSync `timeout: 10000` already SIGKILLs a catastrophic-backtracking hook, so the exit-0 assertion is the real ReDoS guard. Removed the timing assertion; kept exit-0 + documented the subprocess-timeout mechanism. Surfaced (not caused) by this branch's gsd-test runs loading the bench; unrelated to the markdown-parsing changes but fixed in place per the no-flaky-tests rule. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>	2026-07-14 14:25:44 -04:00
Jeremy McSpadden	e5ef323b15	feat(#1787): add /gsd:next smart entry workflow (#1798) ... * docs: design spec for /gsd smart-entry command Hybrid approach porting gsd-pi's smart-entry wizard to gsd-core: deterministic classifier (gsd-tools smart-entry --json) + markdown command/workflow with AskUserQuestion + --text fallback. Routing-first ('what now?' menu), 10 situations redesigned for gsd-core's phase loop. * feat: add /gsd-start smart-entry command State-aware front door adapted from gsd-pi's smart-entry wizard, redesigned for gsd-core's markdown-first, multi-runtime architecture. - src/smart-entry.cts: deterministic situation classifier (no-project, paused, blocked, verify-failed, needs-first-phase, planning, executing, verify-pending, idle-stranded, complete, unknown). Reads STATE.md, ROADMAP.md, git, and verify signals; emits JSON the workflow consumes. - gsd-tools.cjs: wire case + help listing. - commands/gsd/start.md + gsd-core/workflows/gsd.md: thin markdown dispatcher presenting an AskUserQuestion menu (with --text fallback for non-Claude runtimes) and dispatching to existing commands. Falls back to /gsd:progress if detection is unavailable. - help.md: document /gsd:start (parity with bug-2954). - tests: smart-entry.unit.test.cjs (classifier behavior across all situations + priority + JSON shape) and gsd-workflow.structure.test.cjs (markdown-layer invariants + every emitted command resolves to a real slash command). Spec: docs/superpowers/specs/2026-06-27-gsd-smart-entry-design.md Note: command-contract (ADR-0002) requires a gsd:* prefix, so the bare /gsd from the spec surfaces as /gsd-start. * refactor: rename smart-entry command to /gsd:next Rename the command from /gsd:start to /gsd:next per feedback. The command file is now commands/gsd/next.md (name: gsd:next) and the backing workflow is gsd-core/workflows/smart-entry.md (named for the smart-entry classifier and gsd-tools smart-entry subcommand; does not collide with the existing workflows/next.md, which is the progress --next sub-workflow). help.md and the spec updated to match. All affected tests (188) pass; lint:ci clean. * fix: smart-entry reads real STATE.md schema (nested progress YAML + body Phase field) Codex review found the classifier misread this repo's own STATE.md: it looked only for scalar current_phase/total_phases frontmatter and body fields named 'Current Phase'/'Total Phases', but real STATE.md stores the phase as body 'Phase: N' and total_phases/percent under a nested 'progress:' YAML object. Both came back null, so active projects (e.g. this repo at Phase 3 / verifying) wrongly classified as needs-first-phase. - detectSignals now reads total_phases + percent from nested progress{} first, then scalar fm, then body; current_phase falls back to the body 'Phase:' field (parseProsePhaseField lineage). - Add regression tests against the real schema (nested progress YAML + body Phase field) covering verify-pending + executing situations. Verified against this repo: now classifies verify-pending (was needs-first-phase). Coverage 93.25% lines / 86.99% branches. * fix(workflow): tiered fallback when gsd-tools is broken (not just smart-entry) Live test exposed a self-defeating fallback: when smart-entry --json failed because gsd-tools itself was broken (missing markdown-sectionizer.cjs), the workflow fell back to /gsd:progress — which also depends on gsd-tools and would dead-end too. Replace the single /gsd:progress fallback with a tiered recovery: 1. Probe gsd_run state-snapshot. If it ALSO errors, the whole tool layer is down — read .planning/STATE.md directly with the Read tool and synthesize a minimal situation + actions menu so /gsd:next stays useful. Surface a rebuild hint. 2. Only if smart-entry alone is missing (older gsd-core), fall back to /gsd:progress as before. Matches the direct-read resilience the live agent already did by hand. * docs: add gsd-next skill surface * chore: trigger no-mistakes validation * no-mistakes(review): Fix smart-entry phase ordering * no-mistakes(review): Fix decimal smart-entry phase ordering * no-mistakes(test): Fix smart-entry next test contracts * no-mistakes(document): Docs synced for smart entry * chore: add changeset fragment for #1798 (/gsd:next smart-entry workflow) Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com> * fix: shorten next.md description and update golden install parity fixtures Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com> * fix: update /gsd-next refs to /gsd:next in docs and add Smart Entry topic alias Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com> * chore: trigger no-mistakes validation * fix: regenerate INVENTORY-MANIFEST.json for new /gsd-next files Full CI caught that adding commands/gsd/next.md + gsd-core/workflows/smart-entry.md left docs/INVENTORY-MANIFEST.json stale (not in the affected-test scope that no-mistakes' test gate runs, so it surfaced in CI). Regenerated via node scripts/gen-inventory-manifest.cjs --write; inventory-manifest-sync test now passes. * fix: add 'next' to core_loop cluster, update INVENTORY-MANIFEST, fix gates.md ref Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com> * fix: regenerate golden install parity fixtures for /gsd:next Full CI (shard 3/3) caught that adding commands/gsd/next.md + the smart-entry workflow/lib made the per-runtime golden install parity fixtures stale across all 16 runtimes. Regenerated via UPDATE_GOLDEN=1 node --test tests/golden-install-parity.test.cjs. All 16 fixtures + inventory-manifest-sync now pass. * Fix smart-entry verify-failed phase scoping and empty resolve shim step Scope detectVerifyFailed to STATE.md's current phase so leftover higher phase directories cannot force verify-failed routing. Move the gsd_run shim resolver into the workflow resolve step so agents define gsd_run before the detect step runs smart-entry. * fix: recapture golden fixtures with updated gates.md hash (/gsd:next) Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com> * fix: recapture all 16 golden fixtures with updated smart-entry.md hash Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com> * chore: regenerate fixtures + inventory manifest after rebase onto next Rebased onto next which adopted #1837 (package-version normalization to <VERSION> in golden-install-parity hashes). Recaptured the golden fixture that needed it (hermes), re-sorted INVENTORY-MANIFEST.json, and regenerated the gsd-next / ns-workflow skill descriptions to match the command surface. Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com> * refactor(#1787): delegate /gsd:next in-project advancement to gated /gsd:progress --next Reconciles the /gsd:next smart-entry front door with the existing /gsd:progress --next engine (davesienkowski review on PR #1798). The classifier previously recommended /gsd:execute-phase directly for the `executing` situation, bypassing workflows/next.md Route 0 (resume-incomplete-phase invariant, #160) and Gates 1-3 — reproducing the duplication that got the old flat /gsd-next removed (#3054), plus a correctness hazard (executing the recorded current phase while an earlier phase is silently incomplete). Now planning/executing/verify-pending recommend `/gsd:progress --next` (single gated engine); the specific command stays an explicit secondary. Off-path states (no-project, paused, blocked, verify-failed, idle-stranded, complete) keep direct recommendations — smart-entry's distinct value over --next. Adds docs/adr/1787-gsd-next-smart-entry.md and a regression test locking the delegation contract. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(#1787): avoid literal /gsd-next token in ADR (bug-3054 guard) The repo-invariants #3054 guard bans the removed /gsd-next slash form in docs surfaces. Refer to the removed command as `gsd-next` (prose) — the historical reference is unchanged, just the banned token is dropped. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * chore: gitignore compiled host-integration-sdk + handshake-serialized .cjs Pre-existing gap from #1683: these two src/*.cts modules compile to gsd-core/bin/lib/*.cjs but were omitted from the per-file ignore list, so `npm run build`/`npm test` left them as untracked build artifacts (dirty tree + accidental-commit footgun). Adds them alongside their siblings (host-integration.cjs, mcp-server.cjs, …). Found while finishing #1798. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(#1787): lock per-situation action invariants for all 11 situations + ADR typo Adversarial-review follow-ups: - Add a test asserting every situation's action set has exactly one recommended action, 1-4 unique-id /gsd:* actions (previously the one-recommended/1-4 invariant was only sampled for 6 of 11 situations). - Fix ADR typo: /gsd-progress → /gsd:progress. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(#1798): split oversized test chunks so a slow shard can't trip the per-chunk timeout Root-cause of the intermittent `full test (windows-latest, 22, shard 1/3)` failure. It was NOT a leaked handle (the runner's kill message guesses that, but --test-force-exit already exits leaks cleanly). Diagnosis: - Ran every shard-1/3 file WITHOUT --test-force-exit + a 45s kill-timer: zero hangs, zero leaks — every file self-exits. So no leaked handle / hang. - CI activity profile: output kept flowing (slowly) right up to the 600.0s kill — a dead hang would go silent. => pure slowness. - Per-file timing: install-minimal-hooks.test.cjs is a 4987-line / 250-case consolidation file doing dozens of real installs — 41s even on a fast Mac (much worse on the slow Windows I/O path), plus an install-heavy cluster. Mechanism: MAX_FILES_PER_CHUNK=180 packed the whole ~171-file shard into ONE `node --test` chunk, so the entire shard's wall-clock ran against a single 600s per-chunk backstop. On slow Windows runners that single chunk crossed 600s and was killed mid-run — an intermittent false-negative gate that also hits `next` directly. Fix: lower MAX_FILES_PER_CHUNK 180 -> 90 so each shard splits into ~2 chunks, each with its own fresh 600s budget and a fresh node process (also relieves per-process memory pressure). Verified locally: shard 1/3 now runs as chunk 1/2 (90 files) + chunk 2/2 (81 files), 5323 tests, 0 fail. Also made the timeout kill-message name slowness as a cause instead of asserting a leak, so the next debugger isn't sent hunting a nonexistent handle leak. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com> Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Tom Boucher <trekkie@nomorestars.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>	2026-07-03 12:18:25 -04:00