Commit Graph

19 Commits

Author SHA1 Message Date
Tom Boucher
52b11ee811 fix(#3763): pass --raw at every shipped config-get bash call site (#3961)
* test(#3763): guard every shipped config-get substitution on --raw

* fix(#3763): pass --raw at every shipped config-get bash call site

config-get without --raw prints JSON.stringify(value), so string-typed values
reach bash with literal quotes and every string comparison silently never
matches (#3763). --raw added at 75 command-substitution sites across shipped
content; four JSON consumers (default_reviewers, sub_repos, pr_body_sections,
code_review_depth_overrides) deliberately keep default JSON output.

Emitted-Drift-Ack-Growth: ai-integration-phase.md — #3763: bytes from '--raw' at config-get call sites so string-typed config values reach bash comparisons unquoted
Emitted-Drift-Ack-Growth: audit-fix.md — #3763: bytes from '--raw' at config-get call sites so string-typed config values reach bash comparisons unquoted
Emitted-Drift-Ack-Growth: autonomous.md — #3763: bytes from '--raw' at config-get call sites so string-typed config values reach bash comparisons unquoted
Emitted-Drift-Ack-Growth: cleanup.md — #3763: bytes from '--raw' at config-get call sites so string-typed config values reach bash comparisons unquoted
Emitted-Drift-Ack-Growth: code-review.md — #3763: bytes from '--raw' at config-get call sites so string-typed config values reach bash comparisons unquoted
Emitted-Drift-Ack-Growth: complete-milestone.md — #3763: bytes from '--raw' at config-get call sites so string-typed config values reach bash comparisons unquoted
Emitted-Drift-Ack-Growth: discuss-phase-assumptions.md — #3763: bytes from '--raw' at config-get call sites so string-typed config values reach bash comparisons unquoted
Emitted-Drift-Ack-Growth: do.md — #3763: bytes from '--raw' at config-get call sites so string-typed config values reach bash comparisons unquoted
Emitted-Drift-Ack-Growth: eval-review.md — #3763: bytes from '--raw' at config-get call sites so string-typed config values reach bash comparisons unquoted
Emitted-Drift-Ack-Growth: execute-phase.md — #3763: bytes from '--raw' at config-get call sites so string-typed config values reach bash comparisons unquoted
Emitted-Drift-Ack-Growth: execute-plan.md — #3763: bytes from '--raw' at config-get call sites so string-typed config values reach bash comparisons unquoted
Emitted-Drift-Ack-Growth: fast.md — #3763: bytes from '--raw' at config-get call sites so string-typed config values reach bash comparisons unquoted
Emitted-Drift-Ack-Growth: graduation.md — #3763: bytes from '--raw' at config-get call sites so string-typed config values reach bash comparisons unquoted
Emitted-Drift-Ack-Growth: gsd-executor.md — #3763: bytes from '--raw' at config-get call sites so string-typed config values reach bash comparisons unquoted
Emitted-Drift-Ack-Growth: health.md — #3763: bytes from '--raw' at config-get call sites so string-typed config values reach bash comparisons unquoted
Emitted-Drift-Ack-Growth: import.md — #3763: bytes from '--raw' at config-get call sites so string-typed config values reach bash comparisons unquoted
Emitted-Drift-Ack-Growth: inbox.md — #3763: bytes from '--raw' at config-get call sites so string-typed config values reach bash comparisons unquoted
Emitted-Drift-Ack-Growth: ingest-docs.md — #3763: bytes from '--raw' at config-get call sites so string-typed config values reach bash comparisons unquoted
Emitted-Drift-Ack-Growth: mvp-phase.md — #3763: bytes from '--raw' at config-get call sites so string-typed config values reach bash comparisons unquoted
Emitted-Drift-Ack-Growth: new-milestone.md — #3763: bytes from '--raw' at config-get call sites so string-typed config values reach bash comparisons unquoted
Emitted-Drift-Ack-Growth: next.md — #3763: bytes from '--raw' at config-get call sites so string-typed config values reach bash comparisons unquoted
Emitted-Drift-Ack-Growth: plan-phase.md — #3763: bytes from '--raw' at config-get call sites so string-typed config values reach bash comparisons unquoted
Emitted-Drift-Ack-Growth: plan-review-convergence.md — #3763: bytes from '--raw' at config-get call sites so string-typed config values reach bash comparisons unquoted
Emitted-Drift-Ack-Growth: plant-seed.md — #3763: bytes from '--raw' at config-get call sites so string-typed config values reach bash comparisons unquoted
Emitted-Drift-Ack-Growth: profile-user.md — #3763: bytes from '--raw' at config-get call sites so string-typed config values reach bash comparisons unquoted
Emitted-Drift-Ack-Growth: progress.md — #3763: bytes from '--raw' at config-get call sites so string-typed config values reach bash comparisons unquoted
Emitted-Drift-Ack-Growth: quick.md — #3763: bytes from '--raw' at config-get call sites so string-typed config values reach bash comparisons unquoted
Emitted-Drift-Ack-Growth: remove-workspace.md — #3763: bytes from '--raw' at config-get call sites so string-typed config values reach bash comparisons unquoted
Emitted-Drift-Ack-Growth: secure-phase.md — #3763: bytes from '--raw' at config-get call sites so string-typed config values reach bash comparisons unquoted
Emitted-Drift-Ack-Growth: settings-integrations.md — #3763: bytes from '--raw' at config-get call sites so string-typed config values reach bash comparisons unquoted
Emitted-Drift-Ack-Growth: settings.md — #3763: bytes from '--raw' at config-get call sites so string-typed config values reach bash comparisons unquoted
Emitted-Drift-Ack-Growth: ship.md — #3763: bytes from '--raw' at config-get call sites so string-typed config values reach bash comparisons unquoted
Emitted-Drift-Ack-Growth: sketch-wrap-up.md — #3763: bytes from '--raw' at config-get call sites so string-typed config values reach bash comparisons unquoted
Emitted-Drift-Ack-Growth: sketch.md — #3763: bytes from '--raw' at config-get call sites so string-typed config values reach bash comparisons unquoted
Emitted-Drift-Ack-Growth: smart-entry.md — #3763: bytes from '--raw' at config-get call sites so string-typed config values reach bash comparisons unquoted
Emitted-Drift-Ack-Growth: spike-wrap-up.md — #3763: bytes from '--raw' at config-get call sites so string-typed config values reach bash comparisons unquoted
Emitted-Drift-Ack-Growth: spike.md — #3763: bytes from '--raw' at config-get call sites so string-typed config values reach bash comparisons unquoted
Emitted-Drift-Ack-Growth: ui-phase.md — #3763: bytes from '--raw' at config-get call sites so string-typed config values reach bash comparisons unquoted
Emitted-Drift-Ack-Growth: ui-review.md — #3763: bytes from '--raw' at config-get call sites so string-typed config values reach bash comparisons unquoted
Emitted-Drift-Ack-Growth: undo.md — #3763: bytes from '--raw' at config-get call sites so string-typed config values reach bash comparisons unquoted
Emitted-Drift-Ack-Growth: validate-phase.md — #3763: bytes from '--raw' at config-get call sites so string-typed config values reach bash comparisons unquoted

* chore(#3763): changeset fragment (pr number backfilled after PR creation)

* chore(#3763): backfill changeset PR number (3961)

---------

Co-authored-by: sim <sim@local>
2026-08-27 19:55:34 -04:00
Tom Boucher
832dcbb751 fix(#3707): surface UAT rows audit-uat silently dropped, and never report a clean result for a file it could not read (#3887)
* test(#3707): failing-first coverage for the three parseUatItems false negatives

Nine tests that must be red and three controls that must already be green.

The controls are the point of the split. `result: pass` staying unsurfaced is
what stops the fix inverting the filter so eagerly that every passing test
becomes an outstanding item, and the classic single-line shape is the no-churn
control for rewriting the adjacency regex. Both were confirmed green against
the current build before being written down; a control that is red today would
be a second bug, not a control.

Each failing fixture was run through the built parser first and returns []
for its stated cause — the issue row matched then filtered, the block-scalar
and wrapped rows never matched at all, the all-unparseable file vanishing whole.
That evidence is in 50-test-matrix.md rather than asserted.

Tests target ../gsd-core/bin/lib/uat.cjs, the built live module, and drive the
real CLI through runGsdTools. #3706 lost a full RED/GREEN cycle to tests that
imported a different copy of the function under test, so the import target was
verified before anything was written.

* fix(#3707): stop parseUatItems dropping outstanding UAT rows

Three independent false negatives, all in the audit path, plus one the issue
did not mention.

The matcher no longer requires `expected:` and `result:` to be adjacent single
lines. It slices each `### N.` block to the next heading and reads the first
`result:` line within it, taking `expected:` from parseExpectedFromTestBlock —
the seam that already parsed both the block-scalar and inline forms correctly
and was sitting unused two hundred lines away. Two parsers in one module read
the same field with different grammars; now there is one.

The result filter is inverted from an inclusion list of three to an exclusion
of a minimal PASS set. This was the issue's one open design question, which the
reporter explicitly declined to answer for the maintainer; it was asked and
decided deliberately. The fail-safe direction is what parseGapsItems documents
seventy lines below for this same false-negative class (#2286): a token nobody
recognised surfaces rather than vanishing. The trade is a visible, correctable
false positive if a project invents a novel pass-word, against today's silent
and invisible drop.

`issue` also needed a category. It is template-sanctioned with its own `issues:`
counter, but categorizeItem fell through to `unknown` — surfacing it in the
wrong bucket would have been a half-fix.

Finally, a file parsing to zero items no longer vanishes with its frontmatter
`status:`. One with a non-terminal status is reported with `parse_gap: true`,
so the reader gets a cue to look; a `complete` one stays omitted as before.
That is what made the first two defects dangerous rather than merely lossy —
the audit omitted the phase instead of under-counting it.

* fix(#3707): close the review blockers, including a regression I introduced

The remote suite was RED on the previous commit and both reviews found real
defects. Everything below was verified by execution, not by reading.

I introduced a regression against origin/next. The rewritten result matcher was
END-anchored where the old one was not, so `result: pending (blocked on
staging)`, `result: [skipped] # no device` and `result: blocked - waiting` all
returned a row before this branch and returned nothing on it — me reproducing
the exact defect class this issue exists to kill, in the fix for it. The anchor
is gone and each shape has a regression test; trailing text now falls back to
`reason` when the block has none.

`parse_gap` was inferred from the wrong signal. It fired for ANY zero-item file
whose status was not `complete`, which asserted something false about a
perfectly-parsed all-pass file, swept in archived phases left at `testing`, and
is what turned the #2286 Gaps tests red — a control this change was supposed to
keep green. It now derives from headings SEEN BUT UNYIELDED, reported by a new
parseUatItemsWithStats, so an all-pass file and a Gaps-only file are not parse
gaps and a file whose blocks carry no `result:` line is.

The fix was also invisible end to end, which both reviewers caught
independently. parse_gap entries carry no items, and both audit-uat.md and
progress.md gate on `total_items === 0` — so the headline symptom, the phase
vanishing, still reproduced for a user and only the raw JSON had changed. There
is now a `parse_gap_files` counter and both workflows gate and report on it.

Also: categorizeItem compared case-sensitively while the new PASS check
lowercased, so `result: PENDING` surfaced as `unknown`; blocks are bounded at
the next heading of any level, so a trailing `## Gaps` entry no longer bleeds
its `reason` onto the preceding test; dead unreachable fallbacks removed; and
the all-pass control was strengthened, since asserting only `total_items === 0`
let it stay green through the bogus parse_gap entry.

* chore(#3707): acknowledge the workflow growth the fix required

The emitted-attribution guard went red because audit-uat.md and progress.md
grew, and it is right to ask: runtime-loaded workflow prose is the product,
so growth there is a real change to what an executing agent reads.

The growth is not incidental to this fix, it IS the fix reaching a user. Both
reviewers found independently that emitting `parse_gap` in the JSON changed
nothing observable, because both workflows gated their output on
`total_items === 0` and parse-gap entries carry no items — so a phase whose
rows could not be parsed still printed "All Clear" and still vanished from the
progress report. The widened gates and the branches that name the unparsed
files with their phase and path are what close that.

Acks exactly the two paths the guard reported, keyed on the bare filename. The
three spent acknowledgments it also listed are inert by its own description —
the base already absorbs them — so they are left alone rather than swept up
here, where they would just add unrelated churn to this diff.

* fix(#3707): close the mixed-file blocker and the second false-clean surface

The suite was GREEN and the isolated review still found a blocker, which is
the useful part: none of this was covered by a test.

A MIXED file dropped its unparseable rows silently. `parse_gap` sat behind an
`else if` on `items.length > 0`, so one parseable row was enough to discard
`headingsSeen` entirely — a file with one pending row and two unreadable blocks
reported one item and no gap. That is the exact class this issue exists to kill,
reappearing inside its own fix for the third time. The flag is now set
independently of item count and the entry carries `unparsed_blocks`, so the
count is quantified rather than merely flagged.

A `result:` inside a fenced code block was being read as real, so a PASSING
test could be reported as outstanding from a value in a code sample — another
regression against origin/next, whose adjacency regex ignored it. Field scans
now run against a fence-stripped copy while `expected:` still reads the raw
block, since a block scalar may legitimately contain fenced-looking text.

The workflow report was still unreachable whenever anything else was
outstanding: the unparsed table lived in the all-clear branch, so a project with
one pending row in phase 01 and an unreadable phase 02 rendered phase 02
nowhere. It now fires on `parse_gap_files > 0` from the `present` step.

planning-inspect was the second surface making a false-clean claim — for
exactly the files audit-uat now flags it emitted `scope: 'complete'` with an
empty unresolved list, positively asserting completeness over a file it could
not read. It consumes the stats now and reports SCOPE.TRUNCATED with a
`uat_unreadable` diagnostic, reusing the vocabulary already used two lines above
for an unreadable file rather than inventing a token.

Also: headings with no name no longer vanish whole; the trailing-text-to-reason
synthesis I had added is removed, since it was never required by the blocker and
silently changed categorization for `result: [skipped] # no device`; the emitted
`result` token is normalized to lower case so it agrees with `category` in a
published contract; and an O(n^2) indexOf is gone from the heading loop.

* fix(#3707): stop rows stealing each other's fields, on all three surfaces

The suite was green when the security review found these. Two are
blocker-severity and one of them is a direct hit on my own verification.

A `### N.` line indented two spaces inside an `expected: |` scalar is a valid
ATX heading, so it became a phantom row that STOLE the real row's result token
while the real row vanished. I had probed this shape and declared it fixed — my
probe asserted the item COUNT and the result token, both of which the phantom
satisfied, so it passed for exactly the reason it should have failed. Block
scalar bodies are now masked to blank lines (line count preserved, so offsets
still line up) before headings are tokenized, and the tests assert row IDENTITY
— number and name — not presence.

Feeding parseExpectedFromTestBlock the raw slice let one row publish another
row's `expected:` from inside a fence the stripped view had correctly excluded.
Blocks handed to it are now clipped at the first fence opener. This was not
cosmetic on the render-checkpoint path: a checkpoint banner a HUMAN reads and
answers was rendering a different row's expected text.

A balanced fence pair straddling a test block made that heading invisible, so
an outstanding row disappeared with no item, no gap and no count — the exact
false-clean this issue exists to close, and a regression against origin/next.
Suppressed `### N.` lines now count toward headingsSeen so the file is flagged.

An unterminated fence swallowed the rest of the document including `## Gaps`,
producing a whole-file false clean. Such a file is now treated as a parse gap,
following what uat-predicate already does.

Found and fixed inline while there: parseExpectedFromTestBlock's scalar opener
required a bare newline, so a CRLF `expected: |` fell through to the inline arm
and published `expected: "|"`, silently discarding the entire value. The same
fall-through hit `|-` and `|+`.

parseFirstPendingTest had the identical exposure on the render-checkpoint path
and now shares the same masking and clipping. Five legitimate fixtures — inline
expected, a real block scalar, CRLF, bracketed pending, and a first-pending
that is not the first test — are byte-identical before and after.

Also from the code review: the admit condition disagreed with the terminal
status guard, so a `status: complete` file with an unparseable block was
emitted as an empty entry that rendered nowhere but inflated total_files; a
control test was vacuous because its fixture filename did not match its phase
dir, so #3511 scoping meant the file was never opened — and that vacuity is why
the admit regression shipped green; an unterminated fence discarded the flag
that would have caught it; `### 1.2.3` parsed as test 1; and planning-inspect
did not share the terminal-status rule.

* test(#3707): assert what the render-checkpoint fix actually does

The suite went red on three of my own tests and the source was right — the
assertions were wrong, in a way worth naming.

One forbade the rendered checkpoint from containing `### 3. Fake Row`. But in
that fixture the string IS row 1's legitimate `expected:` block-scalar value; a
heading-shaped line inside a scalar is inert text and rendering it is correct.
The test was forbidding correct output. It now asserts row IDENTITY — the
checkpoint is for test 1 named Alpha and never test 3 named Fake Row — which is
the property that actually distinguishes the fix from the bug.

The other expected success where the correct outcome is a clean error: row 1 in
that fixture has no `expected:` of its own and only ever appeared to have one by
stealing row 2's from inside a fence. Depending on the bug to produce a pass is
how a test ends up pinning the defect. The fixture now gives row 1 its own
value and asserts the checkpoint carries it and never the fence-hidden text, and
the error path gets its own test asserting it fails cleanly without leaking.

All three were checked against the real rendered output before the assertion was
written, and each was reasoned through for whether it can fail: the identity
test breaks if a phantom row is parsed, the clipping test breaks if the raw
block is read again, and the error test would pass-not-fail under the old
stealing behavior.

* fix(#3707): correct the scalar masking frame and cover every YAML block opener

Two reviews independently found the same blocker, and it is the sharpest defect
on this issue: maskBlockScalarBodies computed line offsets in UTF-16 units but
spliced them into Array.from(content), a CODE POINT array. One emoji anywhere
earlier in the file shifted every later mask write, so the mask blanked the
wrong characters and spilled past line ends. Measured: at two astral characters
a result token truncated `pending` to `pendi` and recategorized to unknown; at
six the real row vanished; at twelve the FOLLOWING row's `result: blocked`
disappeared and the file reported clean. That is the false-clean class this
issue exists to close, reintroduced by the mitigation written to prevent it, and
defeating both new detectors at once. The mask is rebuilt line by line now,
which is frame-agnostic and length-preserving by construction.

The opener grammar was also incomplete. YAML block scalar headers take an
optional indentation indicator and an optional chomping indicator in either
order, so `|2`, `|2-`, `|-2`, `>2`, `>2+` are all valid — and none were matched.
An unmasked `expected: |2` body meant a `### N.` line inside the value became a
real heading: reproduced, row 1 disappeared and a fabricated row 2 named
"Phantom" took its identity.

Fixing that exposed a third instance of the same family, found by my own probe
rather than by review: the value extractor understood only the `|` openers, so
every `>` folded scalar published the LITERAL OPENER as its value — `expected`
came back as ">" or ">2+" and the whole scalar was discarded. The extractor now
shares the opener grammar and implements real folding, joining paragraph lines
with a space and turning a blank line into a newline, rather than pretending `>`
means `|`.

Also from the reviews: the shortfall counter scanned the masked copy but not a
fence-stripped one, so a `### N.`-shaped line inside a properly closed
documentation fence — the ordinary way to document the row format inside a UAT
file — counted as a suppressed row and flagged the file against nothing; and
clipping at the first fence discarded a legitimate `expected:` that appeared
after a closed fence, which is silent field loss.

Every opener now verified for both row identity and exact extracted value, in
LF and CRLF, alongside the emoji fixtures at 1/2/6/12.

* refactor(#3707): replace the scalar masking with a column-0 heading rule

The fix had grown to five helpers whose only job was undoing one
over-permissive rule: tokenizeHeadings treats a heading indented up to three
spaces as real, so a `### N.` inside an `expected: |` body was parsed as a row
and stole the real row's identity. Every blocker in the last three review
rounds came out of that machinery rather than the reported bug — worst of all
a UTF-16-versus-code-point frame mismatch that corrupted any document
containing an emoji.

A UAT test heading is at column 0. The shipped template puts all of them there,
no `*UAT*.md` in the repo has an indented one, and the only indented `### N.`
lines in the tree are the adversarial fixtures that must not parse. Requiring
column 0 makes a scalar-interior heading a non-heading by construction, so
maskBlockScalarBodies, indentWidthOf and BLOCK_SCALAR_OPENER_RE are gone along
with the mask-invariant test that existed only to guard them. The frame bug is
now structurally unreachable: no code-point array or offset splicing remains.

The premise was incomplete and the reviewer caught it rather than forcing it
through. Masking had been doing double duty — it also hid indented FENCE
delimiters from the tokenizer, so removing it let a two-space fence inside a
scalar body swallow a later column-0 row. The alternative on offer was to
rewrite that test to assert the row is merely counted, which is a behavior
regression dressed as a passing suite. Instead there is a small line-based pass
that blanks only indented fence delimiters — same "column 0 is structure" rule
extended consistently, no YAML knowledge, and line-based by construction so the
frame bug cannot come back. It was proven load-bearing by a negative control:
reverting the wiring reproduces the regression exactly.

Kept, because they fix defects column-0 does not touch: the fence clipping that
stops one row reading another's `expected:` from inside a fence, and the folded
scalar handling that stopped `expected: >` publishing the literal ">".

Also corrects a comment left pointing at a symbol this commit deletes.

* fix(#3707): blank neutralized fence bodies, and give both parse paths one grammar

Reviewing the simplification found two more, and the first is row theft again —
the sixth time this class has surfaced on this issue, and the second time
inside a mitigation written to stop it.

Neutralizing an indented fence blanked only its DELIMITER lines. If the block's
body held a column-0 `### N.`, un-hiding the delimiters made that line a real
heading, which then took the preceding row's fields: an `### 1. Alpha` document
came back as a single row 9 named Phantom, with Alpha gone. Neutralized blocks
are now blanked open-to-close, body included, which is the honest reading of
the intent — an indented fence inside a scalar is content, so nothing in it
should be able to produce structure. The raw block is still what the expected
extractor reads, so a legitimate `expected: |` carrying a fenced code sample
keeps its full text.

The two parse paths also disagreed about what a test row IS.
parseFirstPendingTest filtered on `^\d+\.\s+` while parseUatItemsWithStats used
`^\d+\.(?!\d)`, so `### 3.Foo` was a row when audited and not a row when
resumed. Both now share one predicate and one extractor. The extractor mattered
as much as the filter: the checkpoint path's name-mandatory pattern would have
skipped exactly the shapes the widened filter admits, so fixing the filter alone
would have moved the divergence down a line rather than closing it.

Also from the security pass: parseUatItems had become an export with no callers
and no direct test once both consumers moved to the stats form. It stays, since
deleting an exported symbol from a shipped module is a contract change and not
this issue's business, but it is now documented as the items-only wrapper and
has a test. And the PASS check lowercased a value that extraction had already
lowercased; normalization now happens once.

* fix(#3707): revert the whole-block fence blanking, and pin the rule instead

My previous commit over-corrected and the suite caught it. Blanking a
neutralized fence block open-to-close destroys content legitimately living
between the delimiters, and on an UNTERMINATED opener it blanks to EOF and
deletes every later row. No framing makes that correct, and it is what turned
two earlier tests red.

The "blocker" that prompted it was my own misreading. This change adopted the
rule that column 0 is structure and indentation is content. Under that rule an
indented fence delimiter is not a fence, so a column-0 `### 9.` sitting between
two indented delimiters genuinely IS a heading, and a `result:` after it
genuinely belongs to it. That document is malformed and the parser reading it
that way is consistent, not stealing. Nothing is silently lost either: the row
whose result was taken surfaces as the parse gap.

So the blanking is back to delimiters only, and rather than leaving the
question open, the behavior is now pinned by a test asserting the rows by
identity, with the rule stated at the site — so the next person does not
oscillate the way I just did.

Kept from the reverted commit: the shared row-heading grammar and extractor
across both parse paths, the parseUatItems wrapper documentation and its test,
and the single point of lowercasing.

* fix(#3707): scope the shortfall scan, and stop neutralized content becoming structure

The security review found a HIGH that is the earlier frame-mismatch bug wearing
different clothes. The shortfall scan compared a SECTION-scoped raw line count —
the `## Tests` body — against a DOCUMENT-wide token count. So a single legal
`### N.` row anywhere outside `## Tests` decremented the shortfall and switched
the fence-straddle detector off: an identical `## Tests` section went from
`headingsSeen 1, parse_gap true` to `headingsSeen 0, no gap` purely because a
`## Prior` section existed. A document whose rows render as ordinary blocked rows
in any CommonMark renderer audited as totally clean. Both sides of the
comparison now come from the same surface, by filtering tokens to the scan
span rather than re-tokenizing, so there is no second offset basis to keep in
step.

Neutralizing a fence could also promote its former CONTENT into structure: a
column-0 delimiter run inside an indented pair became an opener once the
enclosing delimiters were blanked, hiding every later heading to EOF. Column-0
delimiter-shaped lines inside a neutralized block are now blanked too — and only
those, so a column-0 heading between neutralized delimiters is still a heading
(the pinned behaviour) and the field lines of a row living between two scalars
still survive. The reviewer corrected my repro while fixing it: an even number of
inner runs re-pairs and hides nothing, so the live shape needs an odd one, and
both are now tests.

Two more from that pass. A legal scalar header carrying a trailing comment
(`expected: | # sample`) failed the end-anchored grammar, publishing the literal
header and raising a false gap. And the indented-row counter walked backwards
per row: 3.6 seconds at sixteen thousand rows, now 15ms, via one forward pass —
though the reviewer also established my example was not the quadratic shape,
which needs an uninterrupted scalar body.

Carried in from the previous round: the indented-row counter keys on any block
scalar rather than only `expected:`, so a template-sanctioned `reported: |`
holding user prose with a heading-shaped line no longer raises a false gap; and
`reason:`/`blocked_by:` read block scalars through the same shared extractor
instead of publishing the literal `"|"`, which also means a multi-line reason
can finally reach categorizeItem — a `reason:` mentioning a server now
categorizes as server_blocked, which was impossible while the value was thrown
away.

* fix(#3707): make the shortfall scan whole-document on both sides

Second HIGH in this area, and the diagnosis is the useful part: I closed the
first one by making the two sides agree, but I did it by NARROWING the token
side to the `## Tests` span while the parse side stayed whole-document. Rows
outside that section are still parsed and surfaced when visible, so when a fence
straddled one it fell through both sides of the comparison — no item, no gap,
file never entered the results at all. A `## Regression Tests` section, or a
second `## Tests` (collectSection takes the first), audited as totally clean
while origin/next surfaced those rows.

Both sides are whole-document now. Symmetry is the property that matters here;
every attempt to be clever about which scope to compare has produced one of
these, twice at HIGH severity.

That reinstates a known over-report, deliberately: a `### N.`-shaped line inside
a closed fence in a `## Notes` section — the ordinary way to document the row
format — counts as a suppressed row and raises a gap on a file with nothing
missing. Noisy, but visible and fail-safe, against two silent false-cleans on
the other side of the trade. This issue exists to eliminate false cleans, so the
trade goes that way, and the reasoning is written at the site so it does not get
optimized back.

Three existing tests encoded the retired scoping and are replaced rather than
worked around: two now assert the accepted over-report, and one asserting a
4-space row is "not counted" was already contradicted by widening the counter to
any indentation — refusing to PARSE a 4-space heading is right, refusing to
COUNT it reopened the hole the counter exists to close.

Also in this commit, from the same review round: the inner-delimiter sweep tested
a column-0-anchored pattern, so an INDENTED delimiter inside a neutralized block
was still promoted to structure and lost a row; it is indent-tolerant now.

A refinement was identified and deliberately not taken — keying the
documentation-sample exemption on the fence info string rather than on section
scope. It is content-based and symmetric, so it would not reintroduce the
asymmetry, but it belongs in its own change rather than riding this one.

* docs(#3707): correct two claims in the over-report justification

Both from review, both comment-only, and both matter because they would
mislead the next person into "fixing" something correct.

The over-report note called the triggering shape "the ordinary way to document
the row format". It is narrower than that: the scan requires literal digits, so
the conventional placeholder `### N. Name` does not trigger it at all — only a
sample written with real numbers does, and no phase UAT file in-tree has one,
only the shipped template, which selectPhaseUatFiles never scans. A maintainer
who tested the documented placeholder form would find no over-report and could
reasonably conclude the pin was stale. That is now stated, and it also makes the
trade look better than I claimed: the real-world frequency is lower.

The attribution guard is described as structural rather than positional. It is
positional in one respect: the walk stops at the nearest column-0 line, so a
block scalar nested inside a `## Gaps` bullet is transparent to it and a
heading-shaped line in that value gets counted. Same accepted over-report,
reached by a path the comment did not mention — recorded so it is not later
mistaken for a new defect.

* fix(#3707): a complete status no longer switches off the parse-gap detector

The security review named this as the last silent-clean path in the change, and
its phrase is the right one: a self-declared kill switch over the very detector
this issue built. A file whose frontmatter said `status: complete` was omitted
unconditionally, so one containing a fence-straddled `result: blocked` computed
headingsSeen = 1 — the detector fired — and then emitted no entry at all. The
audit reported nothing.

The predicate is now status-independent: a file is surfaced when blocks were
seen but yielded nothing, whatever it claims about itself. A terminal status is
an assertion by the author, and an assertion is exactly what must not be allowed
to suppress the signal that would contradict it. What does not change is the
thing the status is actually for — a complete file with nothing to parse, and a
complete file whose rows all parse and all pass, both stay silent, verified
through the real CLI.

I replaced a control test of mine, and it is worth saying why that is not a
weakening: its name was already false. "A zero-item file with a complete status
is still omitted" used a fixture with a `### 1.` block carrying no `result:`
line, so headingsSeen was 1 — it was never a zero-item file, it was the kill
switch itself, pinned. The intent it claimed is now covered by two stricter
tests, one for a file with no blocks and one for a file where every row parses
and passes, each asserting both that no entry exists and that no items are
counted, where the old test asserted only the former.

Everything else is byte-identical: 61 regression cases and the non-complete
equivalents of all four shapes produce exactly the same output as before, with
the delta confined to the two cells this change is meant to move.

* fix(#3707): close the moved kill switch, and keep the archive out of the live gate

Both reviewers independently found that closing the kill switch on one surface
left it standing on the other. cmdAuditUat dropped the terminal-status guard,
but buildUatRows in planning-inspect kept it — and its comment justified that by
claiming to mirror a guard cmdAuditUat no longer had. One byte-identical file
with `status: complete` and a fence-straddled `result: blocked` reported
parse_gap through audit-uat while planning-inspect published
`uat.scope: "complete"` with no diagnostic at all. That is the repo's own
generative-fix-divergence class, and no test pinned that arm, which is why it
survived. The clause and the false comment are gone and the arm now has tests.

Removing it exposed a MAJOR the security pass had not reached: archived phase
dirs are deliberately not milestone-filtered and archived UAT files are
`complete` by definition, so status-independence newly admitted the entire
project archive. One live pending row plus four signed-off milestones produced
parse_gap_files 4 — and since progress.md gates Verification Debt on that
counter, a mature project would have warned on every run, forever, about closed
history no user action can clear. Warning fatigue that buries the next real gap
is the feature defeating itself.

So the counter is split rather than suppressed: `parse_gap_files` counts live
phases only and remains the gate, `archived_parse_gap_files` carries the rest,
and every archived entry stays in `results` with its parse_gap and its
milestone. Nothing became silent; the live signal stayed actionable. Both
workflows report the archived bucket as closed history rather than as something
to act on.

The scope cascade is also decoupled, on the security reviewer's advice that it
is load-bearing here rather than a follow-up: `uat.scope` still reports
TRUNCATED honestly so no completeness is claimed over an unread row, while the
accepted fence-shortfall over-report no longer flips the aggregate fold that
withholds a phase's percentage. A genuinely unreadable file degrades as before.

Also corrects a frequency claim of mine: "no phase UAT file in-tree triggers
this" was true over a sample of zero, since the only UAT file in the tree is the
shipped template. The comment now says the shape is uncommon, which is what I
can actually support.

* fix(#3707): state that the live/archived split does not extend to outstanding_debt

Review MINOR: the split's rationale read as though it governed every counter, but
`summary.total_items` was never split — so a single archived `result: pending` row
re-trips the same Verification Debt warning the split exists to stop. The asymmetry
is deliberate: an archived parse gap is a row nobody can read, so the warning can
never be cleared, whereas an archived pending row is legible work someone can still
pay down by retesting. Debt that can be settled stays counted. The prose now says so
at the point of the claim, instead of leaving the next reader to file it as a miss.

Also rewrites the changeset, which described only the secondary fixes and omitted all
three defects the issue actually reports: `result: issue` dropped, any wrapped or
block-scalar `expected:` never matched at all, and the phase vanishing outright.

* fix(#3707): count every parse gap, dropping the live/archived split

The split had two regressions, both reproduced through the real CLI, and its
premise was false.

uat.cts carries #2766's rationale ~190 lines above the code I added: 'Outstanding
UAT items do not stop mattering when a milestone closes: a deferred human-UAT
scenario or a skipped live-stack test is exactly what gets archived still-open.'
So 'archived UAT files are complete by definition' was never true, and the split
rested on it.

Regression 1: archived-ness was inferred from path shape alone. A phase in the
CURRENT milestone, status in_progress, filed under .planning/milestones/v1.1-phases/
was classified archived and demoted out of the gate — live work reported as closed
history that needs no action.

Regression 2: the split was one-sided. total_items has no archived split, so an
archived outstanding row that PARSES gates Verification Debt while the identical
row that fails to parse was informational. The parse failure was what buried the
debt — the exact bug class this issue exists to fix, re-created one surface over.

parse_gap_files counts every parse_gap entry again, archived or not, so it agrees
with total_items on what archived means. The pre-existing archived_milestone field
and archived-phase scanning are untouched. Regression tests added for both cases.

* fix(#3707): correct the changeset clause left behind by the split revert

The changeset was rewritten before the split was removed, so its final clause
still claimed archived parse gaps are counted separately because signed-off
history is not work anyone can act on. There is one counter now, and that premise
is the one uat.cts refutes and the revert was made over. This text lands in
CHANGELOG.md verbatim, so it would have shipped a description of behavior the
code does not have.

* chore(#3707): backfill changeset pr number

---------

Co-authored-by: sim <sim@local>
2026-08-26 09:16:22 -04:00
Tom Boucher
fb2d122d7f feat(#3841): assert gsd-tools identity on every state-mutating verb (#3848)
* feat(#3841): assert gsd-tools identity before any state-mutating verb

only this package publishes. The path-based branches — a project-local install,
a runtime config directory — had no such guarantee; they trusted their
configured location. This closes them.

Mechanism: once resolution finishes, and before any verb runs, the preamble
probes the tool it picked with `runtime-identity --raw` and matches the answer
with a shell `case` pattern ANCHORED to the start of the compact payload
(`{"packageName":"@opengsd/gsd-core"`). An unanchored substring match accepts
the decoy `{"packageName":"get-shit-done-cc","note":"@opengsd/gsd-core"}`, which
any colliding package could publish. The outcome is exported as the two-valued
`GSD_IDENTITY_STATUS` (`ok`/`unverified`), so the gate is asserted on a VALUE
rather than on warning prose. Rollout is warn-then-fail per the #3146 ruling:
`unverified` prints one line naming BOTH causes and continues, because
`no_identity_verb` cannot tell a foreign package from an `@opengsd/gsd-core`
older than the verb, and at rollout the old-version case is the common one.

The blocker was byte budget, not design. The preamble is inlined into 112
shipped files and several sat within single-digit bytes of frozen ceilings
(`gsd-verifier.md` 16 bytes, `gsd-executor.md` 33, `execute-phase.md` 234); a
first attempt broke five of them. What made room was collapsing the resolver's
twenty near-identical `elif [ -f … ]` arms into one candidate-list helper
(`_gsd_at`), which buys far more than the assertion costs. The preamble is now
2,624 bytes against 4,500 — a net 1,876 bytes SMALLER per inlined file, so every
capped file moved away from its ceiling rather than toward it. No cap raised, no
size-budget exception added, no override token emitted.

Resolution order, every runtime-home probe, the `unset -f gsd_run` re-source
fix, the fail-closed `exit 1`, and the `CLAUDE_ENV_FILE` persistence are all
preserved byte-for-byte in substring terms; the snippet still begins with
`_GSD_SHIM_NAME=` and still ends with `fi`, which the parity extractors anchor
on. `gsd-core/references/gsd-run-resolver.md` is re-synced byte-equal.

Also fixes two stale claims found in passing: CONTEXT.md and FEATURES.md both
described an `[ -x ]` guard as the load-bearing re-source defense. That guard
was tried and REMOVED in #3831 — it rejected the bare function name, fell
through every branch, and hit `exit 1`, which kills a sourced caller's shell.
`unset -f gsd_run` is the actual mechanism.

Refs #3841

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

* fix(#3841): pair the anchor's brace by requiring a closed identity payload

The matrix went red on `tests/new-project-mvp-prompt.test.cjs` — "new-project.md
has unbalanced braces: net depth 2" — plus a knock-on report from its parent
`bug #1516` describe, which is the same failure counted once at the child and
once at the block.

Root cause: that guard (:182-189, mirroring #3784 bd53925f) walks characters and
increments on `{`, decrements on `}`, with no awareness of shell quoting. It
scans `new-project.md` PLUS every `new-project/steps/*.md`, and both
`new-project.md` and `steps/auto-mode-config.md` carry one inlined preamble copy
— hence net 2 from a snippet that was off by exactly one. The unpaired brace was
the `{` inside the single-quoted `case` pattern of the identity anchor, which is
correct shell and invisible to a text scanner.

Fix in the snippet, not the guard. The pattern now anchors at BOTH ends:
`'{"packageName":"@opengsd/gsd-core"'*'}'`. That balances 51/51 with a brace that
does real work rather than a cosmetic pair — a truncated payload whose prefix
matches now fails too, where before it verified. Safe for any future additive
field: a JSON object's own closing brace is always the last character, whatever
type the last value has, which is pinned by two negative-space tests (a nested
object and an array-valued last key must both still verify). Cost: +3 bytes,
against the 1,873 the resolver fold already gave back.

The alternative considered and rejected was dropping the literal `{` for a `?`
glob. It balances too, but weakens the anchor from "must be an opening brace" to
"must be any one character", and the anchor is the entire point.

Two guards added so this cannot recur silently:
- runtime-launcher-parity (F0) pins brace balance at the SNIPPET, so the next
  edit to that pattern fails on the file it broke instead of surfacing three
  files downstream in a test whose name mentions neither the launcher nor this
  issue. It also asserts depth never goes negative, since a `}` preceding its
  `{` nets to zero while being unbalanced at every prefix.
- runtime-identity gains behavioral truncated-payload and trailing-garbage
  fixtures, so the added `}` is proven load-bearing rather than merely present.

Verified: snippet 51/51 braces; new-project combined net depth 0; the seven
other preamble-bearing files with nonzero depth are unchanged from merged next
(their own prose, not the preamble, and not in any guard's scan set); all 112
inlined copies and the resolver reference re-synced byte-equal; sync:launcher
idempotent on the second run.

Refs #3841

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

* chore(#3841): backfill changeset PR number

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

---------

Co-authored-by: sim <sim@local>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
2026-08-25 01:05:53 -04:00
Tom Boucher
63abcface9 feat(#3146): resolve gsd_run so workflows cannot reach a foreign gsd-tools (#3831)
* feat(#3146): resolve gsd_run so workflows cannot reach a foreign gsd-tools

The predecessor package get-shit-done-cc publishes a colliding gsd-tools bin whose phases.clear DELETES where this package's ARCHIVES, and both print success-shaped output against a gitignored .planning/ -- which is how #3129 cost a user 43 phase directories with no error and nothing recoverable from git.

The launcher's PATH branch now resolves gsd_run, published only by this package and self-locating via its own symlink chain to the sibling shim, instead of the colliding gsd-tools. A foreign handler becomes unreachable from PATH, and when no gsd_run is reachable the resolver fails closed rather than falling back -- that fallback was the vulnerability. This is smaller than the branch it replaces, which matters: the preamble is inlined into 113 shipped files and agents/gsd-verifier.md sits 2 bytes under a red-line size cap.

unset -f gsd_run leads the preamble so a re-source is idempotent. Without it, command -v finds the shell function, returns a bare name, and the resolver falls through to an exit 1 that kills a sourced caller's shell.

Adds gsd-tools runtime-identity, a manual diagnostic reporting this runtime's package coordinates over the baked package-identity (#498) and readHostVersion, with a strict total classifier: only a JSON object with an exact packageName verifies, since JSON.parse admits 0/"str"/[]/null/true.

An inlined identity assertion was built and reviewed first, then withdrawn -- it breaks five frozen size ceilings and no assertion fits in 2 bytes.

Closes #3146

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

* fix(#3146): stop sync:launcher relocating a deliberate preamble placement

Pre-existing defect, surfaced by this PR because sync is a no-op unless the snippet content actually changes. transformFile inserts the preamble into the first block that CALLS gsd_run, but gsd-core/workflows/explore.md deliberately places it in a bootstrap-only block that DEFINES gsd_run without calling it -- its own comment explains why: declining the research offer must not leave Step 5's commit call unbootstrapped. Stripping empties that block of calls, so the preamble migrated forward and broke the define-before-use invariant tests/explore-command.test.cjs pins.

Reproduced on a pristine origin/next checkout with the base snippet and base file, so this was not introduced here. The insertion target now honours a block that already carried the preamble, falling back to the first calling block for files that have none yet. Adds a behavioral regression test over a two-block fixture.

Also updates three runtime-launcher-parity tests that pinned the removed PATH fallback to gsd-tools. Their intent is preserved -- the PATH stub is renamed gsd_run so it is reachable by the new resolver, and the RUNTIME_DIR-wins test still asserts the stub is never invoked. Fixture shebangs move to an absolute /bin/sh, because the fixture PATH is deliberately restricted and #!/usr/bin/env sh could not resolve.

Refs #3146

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

* chore(#3146): backfill changeset PR number

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

* docs(#3146): document the FEATURES.md section-numbering practice

The monotonically increasing section number in docs/FEATURES.md is the most frequent merge-conflict source in this repo, and it has TWO conflict cells, not one: the ### N. heading and the hand-maintained table of contents. Two PRs adding differently numbered features still collide on the TOC, so renumbering alone does not make a branch safe. This branch alone was renumbered 165 -> 166 -> 167 -> 168 across successive rebases.

Adds a CONTRIBUTING section stating the practice: allocate the number last, never pre-emptively renumber, take max+1 after a rebase and update the TOC in the same commit, and never renumber someone else's section. Fork contributors are told explicitly they may leave the number to a maintainer at merge rather than chasing the counter. Agents are told to lease the allocation and to include the file in their published touched set.

Records the durable fix as planned rather than pretending it exists: FEATURES.md should be generated from per-feature fragments the way CHANGELOG.md is generated from .changeset/, and the way tests/emitted-drift-acks/ works (#2914).

Also renumbers this branch's own section to 168, leaving 167 to the PR already in flight.

Refs #3146

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

---------

Co-authored-by: sim <sim@local>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
2026-08-24 20:57:16 -04:00
Tom Boucher
8442d984b9 fix(#3809): route runtime-loaded markdown through the gsd_run launcher (#3815)
* test(#3809): generalize dead-ref guard into a rule table (failing first)

The #2020 guard hardcoded `sdk/(src|dist|handlers)/` — the three dead paths
that had caused that storm. That proved those three paths were gone and said
nothing about the class, so #3809 reproduced the identical Windows find.exe
storm under a different token and the guard could not see it.

Replaces the single regex with a rule table over the same runtime-loaded
markdown surface, adds `commands/` to the scan set (previously uncovered),
and adds rule B: the runtime shim filename must never appear in command
position, because it is not a PATH command and an agent that meets it falls
back to locating the file.

Rule B's matcher is deliberately lenient — the launcher's own resolver
assignment, `node <path>/<shim>` calls, bare paths, and prose that names the
file all stay unflagged, each pinned by a negative-space row.

This commit is expected to FAIL: 50 offenders across 23 files remain in the
tree. The remediation lands next.

Refs #3809

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

* fix(#3809): route every workflow call through the gsd_run launcher

50 places across 23 runtime-loaded workflow, agent, reference, and command
files instructed the agent to run the runtime shim by filename. That filename
is not on PATH under any name -- package.json ships gsd-core, gsd-tools,
gsd_run and gsd-mcp-server -- so the call exited 127, the file-shaped token
sent the agent looking for the file, and on Git Bash for Windows the resulting
`find /` walked the entire drive (7268 CPU-seconds in the report) until
somebody killed it by hand.

CONTEXT.md -> Runtime Launcher Module already makes gsd_run the single entry
point: "Canonical space-safe shell preamble (`gsd_run`) used by every workflow
bash block to invoke the GSD runtime CLI." These sites predate that rule --
they trace to 0e6907050 (docs(#195): migrate workflow markdown off gsd-sdk
query), which swapped one non-PATH token for another.

Two further instances of the same class surfaced during remediation and are
fixed here rather than left for later:

  - references/model-profiles.md prescribed `node <shim> effort sync` with no
    path at all; node resolves a bare filename against cwd, so it fails the
    same way.
  - references/universal-anti-patterns.md rule 25 instructed every agent to
    "use <shim>" when shelling out. That rule did not contain the defect, it
    prescribed it repo-wide.

Five "(or legacy <shim>)" parentheticals left dangling by the substitution are
removed; after the rewrite they offered the non-resolving form as an
alternative.

The guard from the previous commit now passes. Its node-prefix exemption was
tightened to require a path separator, which is what exposed model-profiles.

Fixes #3809

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

* fix(#3809): key the guard on the CLI's whole verb roster, not observed usage

Review found the first cut of rule B repeating the very mistake it exists to
prevent. Its verb set held query, commit and effort -- the verbs that happened
to appear in the tree -- so it could not see `<shim> phase add`,
`<shim> state load`, `<shim> verify ...` or twenty-odd other real single-word
subcommands. A guard that only recognises yesterday's offenders is not a guard.

The set is now the CLI's full advertised roster, unioned from the usage banner
and HOST_COMMAND_ROUTERS (which carries verification, planning, uat, stats,
todo and windows, all absent from the banner).

Widening it immediately caught a live offender the first pass had missed:
references/planning-config.md prescribed `node <shim> worktree set-baseref`
with no path. Fixed here.

Also drops the "a hyphen or a dot means subcommand" heuristic, which was
unsound for prose -- it flagged `built-in` and `v1.2`. Detection now keys
entirely on the roster, testing the first dot-segment so that phase.add and
state.patch still match while prose does not. Both false positives are pinned
as negative-space rows.

Guard verified against the pre-fix tree at origin/next: 52 offenders across 25
files, and 0 after this branch's remediation.

Refs #3809

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

* fix(#3809): derive the verb roster from the router; repair launcher parity

Standards review caught the guard repeating the defect it exists to prevent.
Its verb list was a hand-copied literal -- and worse, transcribed from an
INSTALLED older binary, so it was missing 22 verbs this tree actually ships
(websearch, windows, state-snapshot, context-predicates and the dispatch-*
family among them). gsd-tools.cjs already carries three hand-maintained
rosters whose drift is a named defect pinned by the parity test in
tests/commands.test.cjs; a hand-copied fourth was that same defect wearing a
guard's clothes.

The roster is now derived from HOST_COMMAND_ROUTERS + TOP_LEVEL_USAGE, lazily
and memoised, with `query` supplemented explicitly -- it dispatches through
the routing hub ahead of the host-router table, so it appears in neither
export, yet 45 of the 50 offenders used it. A parity test pins the derivation.

Two regressions this branch introduced, both caught by the remote runner:

  - runtime-launcher-parity: rewriting a comment in gsd-research-synthesizer.md
    put a `gsd_run` token at line 65 while the canonical preamble sits at 158,
    breaking "exactly ONE preamble, before the first gsd_run call". The comment
    is descriptive and needs no command token at all; it now names none.
  - The #2751 guard's PROSE_ALLOWLIST entry for that same line went stale once
    the line stopped carrying a bare mention. Pruned, exactly as that guard's
    own stale-entry test instructs.

Also corrects git-planning-commit.md, where the first pass rewrote only the
trailing "legacy" clause and left the sentence reading backwards.

Note the #2751 guard and this one are complementary, not duplicates: its regex
requires whitespace immediately after `gsd-tools`, so it cannot match the
`.cjs` form, and this one only matches the `.cjs` form.

Refs #3809

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

* fix(#2751): extend the bare-command guard to references/ and commands/

The #2751 guard has only ever scanned agents/ and gsd-core/workflows/. Two
runtime-loaded directories were never in its scan set, and 47 bare
`gsd-tools <verb>` calls had accumulated there unseen -- the same defect that
guard exists to catch, in the rooms it never entered.

  - gsd-core/references/: 37 calls, all rewritten to gsd_run. references are
    fragments inlined into a parent that defines the launcher, which is why 21
    of the 22 files already using gsd_run carry no local preamble.
  - commands/gsd/: 10 operative calls rewritten. The remaining 10 are
    descriptive prose ("resolved inside the workflow via ...") and are
    allowlisted with reasons, bringing PROSE_ALLOWLIST to 15.

commands/ also came under launcher propagation. sync-runtime-launcher.cjs
walked only WORKFLOWS_DIR and AGENTS_DIR, so every preamble under commands/
was a hand-pasted copy nothing propagated and no test checked -- graphify.md
had accumulated five. It now walks COMMANDS_DIR too, which collapses those
five to the canonical one-per-file, and runtime-launcher-parity gains a
(B-commands) arm mirroring (B-agents) exactly so the placement stays honest.

The parity arm keys on shell blocks, so commands/gsd/workstreams.md and
config.md -- which name gsd_run only in inline backtick prose -- are exempt,
as they should be. gsd_run is itself a shipped npm bin, so those inline
instructions resolve from PATH exactly as the gsd-tools form they replace did.

skills/ is deliberately NOT added to either guard's scan set: it is generated
from commands/ and pinned by lint:generated-sync, so guarding the source
guards both, and scanning the mirror would double-report every future
offender. Regenerated here.

Refs #2751, #3809

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

* test(#3809): acknowledge the one emitted file this change grows

The emitted-attribution gate failed on the previous sha: gsd-research-synthesizer.md
grew 3 bytes (13847 -> 13850) with no acknowledgment. The substitution SHRANK the
other 19 emitted files, which is why the growth arm was not expected to fire at all.

The 3 bytes are unavoidable. Line 65 is a descriptive comment inside a fenced block;
naming any command there puts a gsd_run token ahead of the file's canonical preamble
at line 158, which runtime-launcher-parity's (B-agents) arm correctly rejects. So the
comment names no command and says where the config is actually loaded instead, which
reads longer than the token it replaced.

Acks only the path the gate reported, per the fragment rules.

Refs #3809

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

* revert(#2751): drop the commands/ half — three contracts pin it in place

The remote runner refuted the commands/ extension outright. Reverting it and
keeping the references/ conversion, which passed.

What broke, all of it caused by bringing commands/ under launcher propagation:

  - graphify.md's five per-block preambles are LOAD-BEARING, not accumulated
    drift. tests/graphify-visualization.test.cjs extracts individual Step-3
    shell chains and executes them standalone, so each fenced block needs its
    own definition of gsd_run. Collapsing them to the canonical one-per-file
    produced `bash: gsd_run: command not found`, exit 127, across four tests.
    The "define once per file" contract holds for workflows and agents because
    nothing extracts their blocks in isolation; commands/ is not like that.
  - explore.md broke "the preamble that DEFINES gsd_run must appear before the
    first USE of gsd_run anywhere in the file".
  - tests/gsd-tools-path-refs.test.cjs (#1766) ASSERTS that
    commands/gsd/workstreams.md contains the literal string
    `gsd-tools query workstream.list`. Rewriting it to gsd_run contradicts a
    test that pins the opposite, so the two guards disagree about that file by
    construction.

So commands/ is not a scan-set widening. It needs those contracts reconciled
first, and that is its own change. SCAN_DIRS keeps gsd-core/references/ and
drops commands/, the ten commands/ allowlist entries go with it (back to 5),
and the reasoning is recorded in the guard itself so the next person does not
rediscover it by burning a matrix run.

commands/gsd/import.md keeps its #3809 fix — that one is the .cjs form this
PR exists to remove, and it is untouched by any of the above.

Refs #2751, #3809

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

* revert(#3809): restore explore.md's Step 1 preamble placement

Running the launcher sync script processed workflows/ and agents/ too, not
just the commands/ directory the run was aimed at, and it MOVED
gsd-core/workflows/explore.md's preamble from Step 1 down to Step 3.

The script inserts into the first bash block that USES gsd_run. explore.md's
Step 1 block only DEFINES it, and that placement is deliberate -- the file
says so on the line above: "Placed in Step 1 rather than Step 3 so declining
the research offer cannot leave Step 5's commit call unbootstrapped."
tests/explore-command.test.cjs pins it.

explore.md carried no #3809 offender, so reverting it costs this fix nothing.
This was collateral from invoking the sync script at all, not from the
COMMANDS_DIR change, which is why the earlier commands/ revert did not catch it.

Refs #3809

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

* chore(#3809): backfill PR number into changeset fragments

pr:0 -> pr:3815 for both fragments now that the PR exists.

Refs #3809

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

* fix(#3809): drop the hand-rolled regex escaper CodeQL flagged

CodeQL raised js/incomplete-sanitization (HIGH) on the guard's pattern build:
`SHIM.replace(/\./g, '\\.')` escapes the dot and nothing else, so it does not
escape backslashes. It blocked PR #3815.

The repo already bans this shape -- local/no-adhoc-regex-escape exists exactly
to stop hand-rolled escapers, with the canonical one in src/pattern.cts. Rather
than reach for that helper, the pattern now carries no escaping logic at all:
SHIM is a compile-time constant whose only metacharacter is the dot, so the
regex source is spelled out literally. The generated source string is
byte-identical to what the replace() produced, verified before and after --
0 offenders on this tree, 52 against origin/next, unchanged.

A drift pin asserts SHIM_PATTERN still matches SHIM exactly, and that the dot
is escaped rather than acting as a wildcard, so the two cannot separate.

Refs #3809

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

---------

Co-authored-by: sim <sim@local>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
2026-08-24 11:47:37 -04:00
Tom Boucher
2537c286f9 refactor(#1762): clarify verification-missing/unknown routing text (#3439)
* test(#1762): assert reassuring verification-routing wording (RED)

Regression test for #1762 — asserts readVerificationStatus's missing/
unknown next_action text reassures the user that execute-phase resumes
at the verification gates without redoing work, instead of reading as
a blind re-run instruction. Fails against current wording; the fix
lands in the next commit.

* fix(#1762): reassure verification-missing/unknown routing text is safe to run

readVerificationStatus's 'missing' and 'unknown' next_action text read as
"redo the implementation", when execute-phase's own discover_and_group_plans
step (#2868) already resumes at the verification gates and skips
execute_waves/checkpoint_handling entirely when every plan already has a
SUMMARY.md. Reword both to say so explicitly, and soften the 'unknown'
message to acknowledge a non-standard status may be an intentional marker
rather than presuming re-verification is always the fix.

Also updates progress.md's Route V.missing / V.unknown to consume the
dynamic $VERIFICATION_NEXT_ACTION (matching V.gaps/V.human) instead of a
hand-duplicated string, so the reassurance lands there too without a second
copy to keep in sync.

No change to next_command routing, status values, or the isPhaseComplete
completeness predicate (still gated on status === 'passed' per the #2957
DISK-STRICT decision) — advisory text only.

* fix(#1762): drop duplicated reassurance clause in progress.md routing text

Review finding: the SUMMARY.md reassurance was stated once via the
interpolated $VERIFICATION_NEXT_ACTION and again as a parenthetical on
the command line, in both Route V.missing and Route V.unknown. Trimmed
the parenthetical so $VERIFICATION_NEXT_ACTION stays the single source
of truth.

* docs(#1762): add changeset fragment

Type Changed with a docs-exempt marker — advisory routing text only,
no docs page documents this specific message today.

* test(#1762): acknowledge progress.md emitted-content growth

Route V.missing/V.unknown now render a small block referencing
${VERIFICATION_NEXT_ACTION} instead of a bare command line, growing
progress.md by 376 bytes. Deliberate per the differential attribution
check (ADR-2719).

* chore: drop spent progress.md ack from #3218's emitted-drift fragment

The #3218 growth it described is already on next (inert, per the
lint's own note that spent acks 'can no longer clear anything').
Its presence collided with the new #1762 fragment naming the same
bare filename, which lint-emitted-drift-ack rejects outright ('two
ack sources may never name the same path'). #3218's other two
acks (plan-phase.md, plan-review-convergence.md) are untouched.

* docs(#1762): backfill changeset PR number

---------

Co-authored-by: sim <sim@local>
2026-08-14 01:49:49 -04:00
Tom Boucher
8b4545f3c0 feat(#3218): the prompt layer asks the CLI for plan counts (#3327)
* feat(#3218): the prompt layer asks the CLI for plan counts

Seven sites across four workflows counted plans with ls and wc -l instead of
asking the CLI. A shell glob is not scanPhasePlans, so every fix that landed on
the owner missed all seven: they counted superseded plans as live, reported zero
for the nested plans layout, and missed loosely-named files. The 1762 figure of
30 plans and 24 summaries came from here.

phase find is extended rather than a verb added - 3218 is an enhancement whose
own checklist says it adds no new command, and CONTRIBUTING makes a new verb a
feature needing approved-feature. It gains plan_count and summary_count for the
live set and plan_count_all for the physical one, additively; the existing arrays
are untouched.

Both sets are exposed because the sites need different ones. Amendment 1 names
two cases; three of these sites ask a third - did the planner write files to disk
- and take the physical set, because a superseded plan is still a file the
planner wrote.

The progress.md dead route is fixed and was worse than the issue said. It read
.plans and .summaries arrays that roadmap.analyze has never emitted, so the
fallback always fired, both counts were always zero, and Route 0's
resume-incomplete-phase check had never fired at all.

The ratchet baseline is empty. Its own stale-entry check makes that
self-enforcing.

Verified on the remote runner.

* test(#3218): acknowledge the workflow growth and update the stale guard

The emitted-attribution gate named its own remedy, so it was followed rather
than pre-guessed: four workflow files grew between 200 and 770 bytes because each
replaced a shell glob with a find-phase call plus its jq extraction. plan-phase
grew most - two sites, and it takes the physical count for its did-the-planner-
write-files question. progress also carries the Route 0 dead-path fix.

plan-phase-drift-guard asserted the literal old ls shape. Updated rather than
deleted: what it protects is that a filesystem fallback exists and is reachable,
and that is intact. It is not a regression - gsd_run is already load-bearing
throughout plan-phase.md long before step 9, so the 9a and 11a fallback never
existed to survive gsd_run being unavailable; it guards against the planner
subagent's return hanging.

Three ack sources collided with the new fragment, which the gate treats as a hard
error rather than last-wins. Only the three colliding keys were removed, not the
421 spent entries, and two fragments left entryless were deleted per the
convention that an empty fragment signals nothing.

Verified on the remote runner.

* docs(#3218): document the live and physical plan counts

docs/CLI-TOOLS.md gains a find-phase counts section covering plan_count and
summary_count for the live set against plan_count_all for the physical one, plus
the null-not-zero not-found behavior. The live-versus-physical distinction is
spelled out because a caller picking the wrong one gets a plausible number, which
is the trap Amendment 1 records.

Changeset leads with what a user sees: progress and execute-plan stop counting
superseded plans as outstanding, a nested plans layout stops reporting zero, and
Route 0 resume routing starts working after never having worked.

No how-to. Nothing is enabled and nothing is sequenced - the user runs the same
command and the number is simply correct. The one new distinction is field
semantics, which is what a reference entry is for.

* chore(#3218): backfill changeset PR number

pr:0 placeholder replaced with the real number now that #3327 exists.

---------

Co-authored-by: sim <sim@local>
2026-08-10 12:56:38 -04:00
Tom Boucher
ff4a57b78c chore(#1671): migrate the remaining 13 LARGE/XL workflows to the fragment model — Phase 6.3 (#3030)
* chore(#2994): fragmentize progress.md forensic audit onto the fragment model

Extract the --forensic-gated forensic_audit step to
workflows/progress/steps/forensic-audit.md behind a section marker, and
repair progress.md's init line to forward --forensic so the atom is
actually true in production rather than only under direct CLI tests.

progress.md shrinks 32630 -> 27207 bytes.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* chore(#2994): fragmentize the four manifest-wired workflows

new-project, quick, new-milestone and progress each already had a
dedicated cmdInit* entry point but zero marked sections. Extract nine
gated bodies to workflows/<wf>/steps/ behind section markers and repair
each init line to forward its flags.

Fold --full into the discuss/research/validate facts inside cmdInitQuick
so the when= grammar never sees an OR, per the chunked-mode precedent.

Fixes found while working, per the no-defer rule:
- cmdInitProgress passed no phase info to buildSectionManifestField, so
  state:phase-mvp-mode was permanently false — an atom in the vocabulary
  whose fact could never be computed.
- the quick init router folded flag tokens into the free-text
  description, which the new forwarding would have corrupted.
- a #2508 dispatch note was nested inside quick.md's Agent(prompt=)
  fence, leaking orchestrator guidance into the subagent prompt.
- progress.md had a 3-vs-4 backtick outer-fence imbalance.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* chore(#2994): fragmentize verify-work.md and admit state:ui-phase-active

Wire cmdInitVerifyWork to buildSectionManifestField — it was a dedicated
entry point that never emitted a manifest — and mark two sections.

state:ui-phase-active folds (plan:pre hooks include an active ui step) OR
(the phase dir holds a *-UI-SPEC.md) into one boolean in init.cts, so the
grammar still sees a single operator-free atom. The inner Playwright-MCP
check stays as prose inside the fragment: it is live session state and no
init seam can precompute it.

The MVP false-branch note is a real fallback, not redundant prose, so it
sits outside the marker — gating it away would delete the text needed
precisely when MVP mode is off.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* test(#2994): follow moved workflow content in drift guards

Retarget every guard that asserted on content this branch moved into
workflows/<wf>/steps/, mirroring 815b3d897. Each retargeted assertion was
verified to still fail when its step file is blanked, so none was
weakened into vacuity.

Three assertions in verify-mvp-uat were genuinely red. Three more were
worse than red — passing for the wrong reason:
- quick-commit-boundary and worktree-cleanup anchored on indexOf('Step
  5.6'), which matched a later cross-reference and sliced 16069 chars
  that coincidentally held the asserted substrings. Replaced with an
  expandWorkflowSections helper that splices step content back in place.
- phase6-review-capabilities lost its end boundary and widened to EOF.
- playwright-ui-verify matched 'UI' in an unrelated bullet and 'fall
  back' in a subagent-dispatch line after the real content moved.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* chore(#2994): fragmentize code-review and complete-milestone, admit three atoms

Add dedicated cmdInitCodeReview and cmdInitCompleteMilestone entry points
alongside the shared generic ones rather than modifying them — init.phase-op
and init.manager carry a CRITICAL blast radius (179 dependents, 24
processes) and stay byte-identical for their other callers.

Admit flag:--fix, state:fallow-enabled and state:git-create-tag, each with
a consuming section and a fact its own entry point computes.

Both sections had the resolver-in-body hazard: the fallow config-gate and
the git.create_tag check each sat inside the very block being gated, so
gating would have disabled the resolver that decides the gate. Both are
hoisted into init and the bodies now consume the resolved fact.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* test(#2994): retarget code-review and milestone drift guards, fix two red tests

Retarget guards that asserted on content moved into steps/, proving
non-vacuity by blanking each step file and confirming failure.

Also fixes two genuinely red tests found while working, per the no-defer
rule:
- workflow-fragments' frozen-vocabulary lock was missing
  state:ui-phase-active, so commit 7ef7f8336 shipped red. Lint and build
  both passed over it, which is why neither is sufficient verification.
- code-review's quick.md capability-hook assertion carried a stale
  delimiter after the 18ff35d20 extraction.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* chore(#2994): fragmentize autonomous.md and admit state:plan-strategy-converge

Five sections share one atom, the pattern plan-phase already uses for
flag:--research-phase. The atom folds --converge OR --cross-ai into a
single boolean in cmdInitAutonomous so the grammar stays operator-free.

cmdInitAutonomous is additive; init.milestone-op, init.manager and
init.phase-op are untouched and still consumed. The $PLAN_STRATEGY bash
resolver is deliberately retained — ungated local-planning bullets still
read it, so the init-side fact supplements it rather than replacing it.

converge-fail-fast required splitting one bash fence so the always-run
CONVERGENCE_ARGS construction stays outside the marker. All three
flag-absent fallbacks were left outside their markers.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* chore(#2994): fragmentize review and discuss-phase-assumptions

Admit state:reviewer-instances-configured (two peripheral notes share it;
the core reviewer-lane dispatch stays unmarked — it is the workflow's
primary always-evaluated logic, not an optional branch) and
state:auto-advance-active, which folds --auto OR two config keys into one
boolean so the grammar stays operator-free.

discuss-phase-assumptions was the highest-risk edit in this PR. Its
auto_advance step is a full if/elif/else; gating it whole would have
deleted the flag-absent fallback needed exactly when --auto is off. Split
verified exact: resolvers 636-651 and the 'End here' fallback 668-669 both
stay outside the marker; only 653-667 is gated.

Adds emitted-drift acks for the two files that grew — review.md (+55 B)
and autonomous.md (+737 B from 80799211c, which had none and would have
red-gated the push.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* chore(#2994): fragmentize docs-update, update, transition and new-milestone Part A

Completes the 13-workflow rollout. Three of these had no init call at all
and gained a dedicated entry point plus their first gsd_run query line.

Admits state:is-monorepo and adds state:next-channel, state:workstream-active
and state:flat-mode. Vocabulary 26 -> 30 atoms.

Part A of new-milestone applies when NO workstream is active — the negation
of state:workstream-active. Rather than teach the grammar negation, which is
the Greenspun drift the frozen list exists to prevent, it gets a separate
positively-phrased atom whose fact is the inverse. Part B, which always runs,
stays outside the marker.

flag:--verify-only is deliberately NOT admitted: docs-update has no
contiguous purely-additive region for it, and an atom without a consuming
section is dead vocabulary. Evidence recorded in the slice report.

update.md reuses its existing resolved $GSD_TOOLS rather than prepending the
canonical preamble, which would have clobbered it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix(#2994): stop automated-ui-verification re-resolving its own gate, retire dead vocabulary

Two defects the new tests caught.

The automated-ui-verification step re-ran gsd_run loop render-hooks and
recomputed UI_PHASE_ACTIVE inside a body that is only read when that fact
is already true — the circular self-disabling pattern this design forbids,
introduced by 3c654b168. cmdInitVerifyWork now exposes ui_phase_active and
the step consumes it. Its launcher preamble goes too: no gsd_run remains.
The Playwright-MCP check stays as prose — that is live session state.

Dead vocabulary predating this PR: flag:--full and state:needs-codebase-map
were admitted with a gate-1 claim that never materialized. flag:--full is
removed, redundant once quick folds it into discuss/research/validate.
state:needs-codebase-map gets the real consumer it always lacked, gating
new-project's codebase-map offer. Vocabulary 30 -> 29, and no atom is now
without a consuming section.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* test(#2994): add the atom-admission, inversion and resolver-hoist gates

The two existing parity guards prove vocabulary/predicate symmetry but
never that a fact is computed — an atom no cmdInit* assembles evaluates
false forever. These close that hole:

- per-atom satisfiability for all 29 atoms, plus an anti-vacuity assertion
  so the loop cannot silently cover zero atoms
- dead-vocabulary check against the shipped manifest
- inversion guard: the flag-absent fallbacks in discuss-phase-assumptions
  and verify-work must stay outside their markers
- data-driven resolver-hoist guard over the shipped manifest, so a future
  extraction cannot reintroduce the circular class
- compound-fold coverage (--full, --cross-ai, --rc, config-only --auto)
- null-vs-[] degraded/computed distinction, and flag value shapes

Also repairs the frozen-vocabulary lock, which was stale and red for the
seven atoms earlier commits on this branch shipped.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* docs(#2994): add changeset for the fragment-model rollout

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* test(#2994): cite the issue on the two new allow-test-rule exemptions

ADR-456 requires an issue ref on the same line as the annotation.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* docs(#2994): correct the atom-count claims after retiring flag:--full

The vocabulary doc comments still said 30 entries; it is 29 since
flag:--full was removed as dead vocabulary.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix(#2994): dedupe the phase-fallback block and harden --ws parsing

Review findings.

MAJOR: the three new init entry points each pasted a verbatim copy of the
guardedFindPhase/guardedGetRoadmapPhase fallback, taking the repo from four
copies to seven — DEFECT.GENERATIVE-FIX. Extracted applyRoadmapFallback and
folded six of the seven; each call site keeps its own field-set via a
closure. Duplication removed rather than papered over with a parity test.
cmdInitPhaseOp stays out: its fallback omits has_reviews, so it is not a
byte-identical copy, and it is CRITICAL-radius.

LOW, pre-existing: GSD_WS captured [^[:space:]]+ and expands unquoted, so a
workstream name holding glob metacharacters would expand against the
filesystem. Narrowed to [A-Za-z0-9._-]+. The unquoted expansion is kept —
it must word-split into two args and vanish when empty.

Also restores the vocabulary ordering convention, and fixes a masked test
bug the mandated run surfaced: the flag-forwarding guard checked only the
first init line per workflow, but new-milestone has two, so a real failure
was reporting exit 0.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix(#2994): drop the stale new-milestone emitted-drift ack

new-milestone.md was acked for a +406 B growth measured against an
intermediate commit. Net against origin/next it SHRANK by 8 bytes, so
nothing needed the ack and it explained nothing — which the differential
attribution check reports as a stale acknowledgment, not a pass.

update.md's entry stays: it genuinely grew +703 B.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix(#2994): resolve the 15 failures from the full matrix run

All 15 were real and identical on both lanes.

REAL REGRESSION: autonomous.md hit 41479 chars against the #2196 guard's
40960 cap — a CHARS cap distinct from the LARGE tier byte cap, which the
five section stubs pushed it over. Extracted the 3a.5 UI Design Contract
body to references/; now 39968 chars, and the file nets -795 B vs base, so
its growth ack is deleted rather than left stale.

REAL DEFECT: docs referenced /gsd-transition, which is not a live
registered command. Reworded.

STALE FIXTURE: the emission byte-identity test hardcoded two marked
workflows; this branch legitimately marks fifteen. Fixture corrected — the
source was right.

The rest were drift guards over the eight workflows the earlier sweep did
not cover, retargeted at where the content now lives with non-vacuity
proven by blanking each step file and confirming failure. The GSD_WS
forwarding guard was checked as a possible real break and is not one: the
charclass narrowing is intact and forwarding works end to end.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix(#2994): drop the ack for a newly-added reference file

A new file's emitted ripple is attributable to the diff that adds it, so
the acknowledgment explained nothing and the differential check reports it
as stale. Removing the last entry removes the fragment — an empty one
signals nothing.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix(#2994): retarget the UI-contract guards and clear two transitive advisories

The §3a.5 extraction that brought autonomous.md under the #2196 char cap
moved its body to references/autonomous-ui-design-contract.md, so ten
guards in autonomous-ui-steps and check-ui-safety-gate were asserting it
against the host. Retargeted via a combined read, each proven non-vacuous
by blanking the reference file and confirming failure.

This class had already bitten twice on this branch because each sweep was
scoped to the workflows touched at that moment, so this one was
exhaustive: ~70 test files across all 13 workflows, zero further broken or
vacuous assertions found.

Also clears two high transitive advisories the matrix flagged on one lane
— fast-uri GHSA-7p8r-x3mc-p8w7 and three ip-address SSRF/trust-boundary
issues. Both pre-date this branch: package-lock.json was untouched until
now, so the production tree was byte-identical to the base. Lockfile-only,
package.json unchanged, verified against a real npm ci install.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* chore(#2994): backfill changeset pr number to 3030

---------

Co-authored-by: sim <sim@local>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-03 19:59:58 -04:00
Tom Boucher
b36e3b7e1f fix(#2751): normalize bare gsd-tools command-position calls to gsd_run in shipped source (#2851)
* test(#2751): regression guard — no command-position bare gsd-tools calls

Agents/workflows instructed bare `gsd-tools <verb>` invocations that fail with
'command not found' on a shim-only install (#725 fixed only the Codex conversion
pipeline; the Claude-facing source shipped them verbatim). Adds a source-text
guard (allow-test-rule: source-text-is-the-product) scanning agents/*.md +
gsd-core/workflows/*.md for the operative shape `gsd-tools <verb> <arg>`,
excluding command -v probes / resolver definitions, with a documented
PROSE_ALLOWLIST for descriptive mentions that name the command without
instructing literal invocation. A stale-allowlist check ensures entries stay
real. RED first; source fix lands next commit.

* fix(#2751): normalize bare gsd-tools calls to gsd_run in Claude-facing source

The 12 command-position bare `gsd-tools <verb>` instructions across agents/ and
gsd-core/workflows/ failed with 'command not found' on a shim-only install (no
gsd-tools binary on PATH). #725 fixed this only for the Codex install-conversion
pipeline; the Claude-facing SOURCE shipped the bare calls verbatim, and new ones
kept accumulating (new-project.md:114 landed 12 days AFTER #725 closed).

Rewrite each operative site to the portable `gsd_run` resolver that the same
files already define (3-18x each) — a pure command-position token swap
preserving all arguments, flags, --files, and surrounding prose. Every runtime
now benefits from one source change instead of each needing its own converter.

Touched sites (12): gsd-intel-updater (validate/snapshot/extract-exports),
gsd-code-fixer (query commit), gsd-planner (learnings.query), gsd-project-
researcher (websearch/research-plan/classify-confidence), gsd-phase-researcher
(websearch/research-plan/classify-confidence), new-project (project-instruction-
file / commit --files), new-milestone (commit --files). Preserves command -v
gsd-tools probes, resolver-snippet definitions, and the 4 descriptive prose
mentions that NAME the command without instructing invocation. RED @ 1bb12ba1.

* chore(#2751): allow-test-rule issue ref + changeset fragment

Add the (#2751) tracking ref to the source-text-is-the-product annotation per
ADR-456, and the .changeset Fixed fragment (pr:0, backfilled post-PR).

* fix(#2751): convert remaining command-position bare gsd-tools calls (verify-summary, windows, worktree, smart-entry, quick-tasks-append)

Isolated adversarial review (Step 4) found the first pass missed genuine
command-position bare calls because the regression test's hand-maintained
6-verb list silently false-passed verify-summary (the 'verify' branch matched
the prefix then died on the hyphen) and omitted windows/worktree/smart-entry/
quick-tasks-append entirely. Convert these 8 additional operative sites across
new-project.md, new-milestone.md, ship.md, execute-phase.md, progress.md,
smart-entry.md, quick.md.

* chore(#2751): backfill changeset PR number (2851)

* fix(#2751): normalize allowlist paths to forward slashes — Windows path-separator false-flag

The PROSE_ALLOWLIST is keyed by file:line using forward-slash paths, but
path.relative() returns backslash separators on Windows, so the allowlist
lookup failed and the 6 descriptive mentions were flagged as offenders on
the windows-latest CI lane. Normalize rel to forward slashes before the
lookup so the allowlist matches identically on every OS.

---------

Co-authored-by: Test <test@example.com>
2026-07-30 11:18:07 -04:00
Tom Boucher
d16a66479a feat(#1950): broken-windows ledger — cross-phase defect register gating ship (#2441)
* feat(#1950): broken-windows ledger — cross-phase defect register gating ship

Adds a new  capability (#1950) that operationalizes GSD's
no-defer discipline as a tracked, enforced artifact:
accumulates stubs, TODOs, skipped tests, unrun verifies, and unmet truths
across phases, and /gsd-ship blocks while any entry is open.

Implementation:
- src/broken-windows.cts → gsd-core/bin/lib/broken-windows.cjs: typed IR +
  I/O entry points (parseLedger/renderLedger/appendWindow/markWaived/markFixed
  + cmdWindowsStatus/Append/Waive/MarkFixed). Frozen REASON enum for typed
  error assertions. Windows-safe atomic rename with retry on transient
  EPERM/EBUSY/EACCES.
- gsd-tools.cjs: new  subcommand (status | append | waive | fixed),
  wired via routeWindows + HOST_COMMAND_ROUTERS.windows.
- capabilities/broken-windows/capability.json: one ship:pre gate with
  artifact-frontmatter-equals predicate on WINDOWS.md open_count == 0.
  activationKey windows.enabled (default true) + sibling windows.enforce
  (default true, separate so tracking can precede enforcement).
- gsd-core/workflows/ship.md: capId==broken-windows branch in preflight,
  sibling to security — reads gsd_run windows status --raw, fails closed
  on open_count > 0 or unreadable ledger.
- agents/gsd-executor.md: extends the existing ## Known Stubs instruction
  to also append to WINDOWS.md via gsd_run windows append (best-effort,
  never blocks execution).
- agents/gsd-verifier.md: new Step 8b — record unmet truths + human-verify
  items in WINDOWS.md.
- gsd-core/workflows/progress.md: surfaces open + waived counts.
- docs/COMMANDS.md + CONTEXT.md glossary entry + docs/INVENTORY.md:
  document the gate, waiver mechanism, and new module.
- tests/broken-windows.test.cjs: pure + CLI behavioral coverage + fast-check
  roundtrip property; fail-closed on malformed ledger; security boundary on
  path traversal in --file.

Backward-compatible: a project with no .planning/WINDOWS.md reports
open_count: 0 and ships cleanly. Disable enforcement per-project with
gsd config-set windows.enforce false (tracking continues, gate stays open).

* chore(#1950): ratchet size baselines, defer verifier integration

- Workflow size baseline: ship.md 25575→27928, progress.md 31789→32632
  (broken-windows preflight branch + open-windows surface).
- Agent size baseline: gsd-executor.md 46644→47951 (Known Stubs → also
  appends to WINDOWS.md). gsd-verifier.md unchanged.
- LARGE_CAP (49152) preempted the planned verifier integration
  (gsd-verifier.md was at 49140 pre-PR — 12 bytes of headroom, not the
  documented 'real headroom'). Verifier integration deferred to a follow-up
  PR that extracts the VERIFICATION.md template (lines 739-859) to
  gsd-core/references/ — a pre-existing cap-tightness defect this PR
  exposed but does not expand scope to fix. Verifier integration is not in
  the issue's acceptance criteria (executor writes is; unmet-truths
  recording was an enhancement, not a gate).

* fix(#1950): gate default-off, rename to workflow.windows_enforce, regen goldens

Test-failure-driven fixes after first gsd-test run on db8733c8f failed 44
cases (pre-existing structural tests encoded 'ship:pre has 1 gate' / 'all
caps off → empty hooks'):

- capability manifest: rename windows.enabled+windows.enforce (default
  true) → single federated key workflow.windows_enforce (default FALSE,
  opt-in). Matches security's workflow.security_enforce convention and
  makes the adr857 all-caps-off test pass without modification (the test's
  buildAllFalseConfig handles workflow.* out of the box). Default-OFF keeps
  the gate out of the registry's default ship:pre resolution so existing
  loop-hooks-ship-pre-e2e structural assertions (exactly 1 gate, capId
  'security') stay valid; users opt in via
  gsd config-set workflow.windows_enforce true.
- drop activationKey (security doesn't have one either; workflow.* key
  doubles as the activation toggle).
- regenerate docs/reference/capability-matrix.md to include broken-windows
  (capability-matrix-sync test).
- regenerate tests/fixtures/golden-install-parity/*.json (18 runtimes) —
  installer now emits the new capability + lib file.
- update CONTEXT.md, docs/COMMANDS.md, docs/FEATURES.md, ship.md,
  agents/gsd-executor.md to use the new key name and /gsd:colon slash
  syntax (slash-command-namespace test).
- restore accidentally-regressed /gsd:capture in progress.md.

Tracking-only by default; enforcement is opt-in. Acceptance criterion
'/gsd-ship fails while any ledger entry is open' is met when
workflow.windows_enforce=true (test fixture enables it).

* test(#1950): update ship:pre structural invariants for 2-gate registry

- loop-hooks-ship-pre-e2e: the registry now declares 2 gates at ship:pre
  (security + broken-windows), regardless of activation. Activation tests
  above still pin security-only or empty behavior via fixtures; these
  structural tests pin the REGISTRY shape, which has 2 gates as of #1950.
- workflow-size-baseline: ship.md 27928→27945 (workflow.windows_enforce
  rename added 17 bytes).

* fix(#1950): review H1+H2+M1+M2+M3 — fence-injection, EACCES fail-closed, cleanup, strict line, stryker

Adversarial isolated review (Step 6.3) found 2 HIGH findings that block
the PR and 3 mediums. All addressed:

H1 (HIGH): description containing the markdown 3-backtick fence would
terminate the ledger's JSON code block early inside JSON.stringify output
(JSON doesn't escape backticks), corrupting the file and bricking the
next parse. Fix: use a 4-backtick fence (json ... ) which
JSON.stringify cannot produce on its own, AND validate that no entry
text field contains a 4-backtick run (reject at append time with new
WINDOWS_INVALID_TEXT reason code). Locked by a regression test.

H2 (HIGH): readLedgerOrNull swallowed ALL fs errors as 'no ledger',
silently returning open_count:0 on EACCES/EPERM/EIO. The ship gate
would then pass on an unreadable ledger — the precise vector the
workflow doc claims is impossible. Fix: only ENOENT returns null;
every other fs error propagates as WINDOWS_LEDGER_MALFORMED so the
gate blocks and the operator sees a real diagnostic. Locked by a
regression test that chmod 000s a ledger with open_count=1 and
asserts the result is never a false-green 0.

M1: writeLedgerAtomic left an orphaned .tmp file on rename failure.
Wrapped renameWithRetry in try/catch with best-effort unlink.

M2: validateLine silently coerced 'abc' → NaN → null, hiding type
drift. Removed the line === 0 special case (was undocumented) and
made the error message match the strict check. Now any non-positive-
integer line value throws, including strings.

M3: tests/broken-windows.test.cjs (with its fast-check property test)
was not in stryker.config.mjs DEFAULT_TEST_CMD — Stryker would mutate
src/broken-windows.cts but no test would catch the mutations,
producing false surviving-mutant scores. Added to the list.

L1 (dead throw e after error()), L7 (line boundary tests, H1/H2
regression tests, 4-backtick CLI test) also addressed.

* docs(#1950): inline concurrency + busy-wait notes (review L2+L3)

* fix(#1950): regen goldens against latest gsd-tools; correct --line 0 boundary test

gsd-test v4 caught two issues:
- goldens I regenerated earlier (commit 526682084) predated the L1
  routeWindows catch-block cleanup (commit dd844d565). Regenerated
  via 'npm run gen:golden' against current HEAD so the install
  parity hash for gsd-tools.cjs matches.
- 'append --line boundary' test expected --line 0 to succeed with
  null entry.line, but the M2 fix correctly rejects 0 (lines are
  1-indexed; 0 is not a valid source line). Updated the boundary
  test to assert --line 0 fails alongside -1 and 'abc'.

* chore(#1950): regen goldens after rebase onto next

* chore(#1950): quick.md baseline 50699→50993 (correct resolution from next rebase)

* chore(changeset): backfill pr:2441 in .changeset/broken-windows-ledger.md

* fix(#1950): renderTable escapes backslash before pipe (CodeQL incomplete-sanitization)

CodeQL flagged the markdown-table cell escaper:
  String(s ?? '').replace(/\|/g, '\\|')
— it escapes pipe but not backslash first. A description containing '\|'
would render as '\\|' which markdown parses as 'literal backslash' +
'cell separator', splitting the column.

Fix: escape backslash FIRST (each \ → \\), then pipe (each | → \|).
Now a description with '\|' renders as '\\\\|' (literal '\\' + escaped
pipe), which markdown renders as a single '\|' inside the cell. The JSON
code block (the parse source-of-truth) was already correctly escaped via
JSON.stringify; only the display-only table was affected.

Locked by a regression test that:
1. Verifies the JSON block reparses with the description intact.
2. Walks the rendered table row counting unescaped pipes — must be
   exactly 11 (the row separators for 10 cells), proving no in-cell
   pipe added a split.
2026-07-19 20:24:21 -04:00
Tom Boucher
b041f101fb fix(#2287): surface unresolved deferred-items.md entries in progress + audit-uat (#2318)
The executor SCOPE BOUNDARY convention (agents/gsd-executor.md) logs
out-of-scope discoveries to a phase directory's deferred-items.md, but no
reader ever consumed it — forensic_audit, cmdAuditUat, and capture --list
all skipped it — so deferred items were permanently invisible.

cmdAuditUat (src/uat.cts) now scans each phase dir's deferred-items.md via
a new parseDeferredItems (reusing the collectSection/splitGapsEntries/
extractGapEntryFields seams) and surfaces entries whose status != resolved
(fail-safe: a missing/garbled status surfaces rather than hides, matching
the false-negative-averse posture of #2286). forensic_audit
(gsd-core/workflows/progress.md) gains Check 7 that globs
.planning/phases/*/deferred-items.md and reports unresolved entries.

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-15 21:40:59 -04:00
Tom Boucher
8b70db343b fix(#2204): phase-completion writes 'All phases complete' per ADR-2207 (#2259)
* fix(#2204): phase-completion writes 'All phases complete' per ADR-2207

completePhaseCore was writing the overloaded bare 'Milestone complete' on the
last phase — the same string space the milestone-close verb owns for terminal
state. Per ADR-2207, phase-completion now writes the existing intermediate
value 'All phases complete' (already used in gsd2-import.cts). Milestone
termination ('<version> milestone complete' / 'Awaiting next milestone')
remains solely with milestoneCompleteCore.

Status lifecycle: Ready to plan → All phases complete → <version> milestone
complete → Awaiting next milestone.

Changes:
- src/state-transition.cts: completePhaseCore status value
- src/phase.cts: #2028 guard comment
- tests/state-transition.test.cjs: assertion + test name
- tests/phase.test.cjs: 8 assertion updates (positive + negative)
- tests/state.test.cjs: normalizeStateStatus test case + reset regex
- tests/workstream.test.cjs: fixture status to terminal value
- gsd-core/workflows/progress.md: Route D label
- gsd-core/workflows/transition.md: Route B label
- CONTEXT.md: Status lifecycle glossary entry (ADR-2207)
- .changeset/brave-geese-jump.md

* test(#2204): regenerate golden-install-parity fixtures + workflow-size baseline

Workflow file edits (progress.md, transition.md) changed install payload
hashes and pushed past the committed workflow-size baseline. Regenerated
all 17 golden-install-parity fixtures + claude-local via the standalone gen
script (which now also covers the local-scope claude layout). Updated
workflow-size-baseline.json and agent-size-baseline.json via size:baseline.

* fix(#2204): correct claude-local golden hashes + document gen-script limitation

The gen-script's claude-local generation produces macOS-specific hashes
incompatible with Linux CI (local-scope install embeds platform-varying
node-runner paths). Reverted to manual update using Linux FAILURES.md
+actual hashes for the 2 changed workflow files. Added explanatory
comment in the gen script.

* test(#2204): add isCompletedInventory coverage + clarify CONTEXT.md glossary

Addresses orthogonal code-review findings (Medium #1 + #2):
- Add isCompletedInventory test cases for ADR-2207 status lifecycle
  (terminal 'milestone complete' → true; intermediate 'All phases
  complete' → false; archived → true; active statuses → false)
- Clarify CONTEXT.md glossary: note that isCompletedInventory
  intentionally excludes the intermediate value

* docs: backfill changeset PR number (#2259)

* docs(#2204): add Status lifecycle table to state-md reference (ADR-2207)
2026-07-14 14:47:03 -04:00
Tom Boucher
a62079b2da fix(#1865): runtime launcher honors CLAUDE_CONFIG_DIR (#2024)
* fix(#1865): runtime launcher honors CLAUDE_CONFIG_DIR

The gsd_run preamble resolved the Claude global install only at
$HOME/.claude/gsd-core/bin/, but the installer honors CLAUDE_CONFIG_DIR —
so a global install redirected via CLAUDE_CONFIG_DIR was invisible to every
gsd_run call (every command failed with 'gsd-tools.cjs not found').

The Claude resolver arm now uses ${CLAUDE_CONFIG_DIR:-$HOME/.claude},
matching the installer + the other runtimes' ${VAR:-default} pattern.
Default $HOME/.claude behavior is unchanged.

- _runtime-launcher.snippet.sh: Claude arm honors CLAUDE_CONFIG_DIR.
- sync-runtime-launcher.cjs re-run: 95 workflows/agents re-synced.
- review.md / discuss-phase.md: trimmed to stay under their byte budgets.
- runtime-launcher-parity.test.cjs: (A) substring updated for the new form
  + explicit #1865 assertion that the snippet honors CLAUDE_CONFIG_DIR.
- goldens + size baselines recaptured.

Closes #1865

* docs(#1865): backfill changeset pr 2024
2026-07-05 14:20:52 -04:00
Jeremy McSpadden
77c7b4fc9d fix(#1522): enforce canonical verification before phase transition (#1548)
* fix: require fresh phase verification before transition

* no-mistakes(review): Fix canonical verification closeout gates

* no-mistakes(review): Fix verify-work frontmatter promotion command

* no-mistakes(review): Fix stale verification gates

* no-mistakes(review): Fix canonical verification routing gates

* no-mistakes(review): Fix verification dependency and runtime routing gates

* no-mistakes(review): Block stale verification bypasses

* fix: handle large init manager outputs in verification workflows

* chore: update changeset pr number

* fix(verify-work): use fresh verification.status for stale gate

The stale check after UAT used phase_completion.verification_status from
session-start INIT while human_needed promotion already queried fresh
verification.status. Align the stale gate with the canonical query so
mid-session verification refresh is not ignored.

* fix(init): skip roadmap-checked phases when selecting next_phase

Roadmap-only phases without a disk directory were still promoted to
next_phase when their checkbox was already checked. Exclude
checkboxComplete phases so progress routing does not point at work the
roadmap already marks done.

* fix: gaps_found not overridden by stale, transition uses canonical verification

- verification.cts: check gaps_found before stale so gap-closure routing
  is not masked by a newer summary mtime
- phase.cts: remove redundant findStaleVerificationSummary — readVerificationStatus
  already handles stale detection
- transition.md: replace raw grep on file content with verification.status query
  to avoid false-positive blocks from body text matching

* ci: retrigger tests after rebase

* fix(transition): replace gsd_run advisory check with awk frontmatter extraction

The runtime launcher is not defined until the update_roadmap_and_state step
bash block (~line 165). The early verify_completion block used gsd_run to
query verification.status, which violated the runtime-launcher-parity test:
'preamble appears AFTER the first gsd_run reference'.

Replace the gsd_run call with an awk-based frontmatter extractor that reads
only the status: field between the two --- fences. This avoids both the
preamble-ordering constraint and the original false-positive grep bug where
body text like 'previous_status: gaps_found' would match a full-text regex.

The phase.complete gate at update_roadmap_and_state is the canonical
enforcement point; this early check is advisory only.

Also update workflow-size-baseline.json for the updated transition.md size.

Fixes: runtime-launcher-parity test (B)

Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>

* fix: re-check verification under planning lock in phase complete

Move readVerificationStatus into withPlanningLock so stale verification
cannot slip through when a SUMMARY.md is written between the gate and
the roadmap/state mutation. Return the blocked status from the lock
callback and emit the error after release to avoid leaving .lock behind.

* fix(transition): gate on canonical verification.status including stale

Replace awk frontmatter read with verification.status query so transition
blocks when summaries are newer than VERIFICATION.md, matching phase.complete
and other workflows (autonomous, progress, verify-work).

* Fix workflow verification gates for yolo transition and stale routing

Require VERIFY_STATUS passed before yolo/interactive transition advance.
Route stale verification recovery to verify-work, matching canonical projection.

* fix(transition): use verification.status query for stale-aware advisory check

The awk-based check read raw frontmatter status: passed, which misses the
stale case where summaries are newer than the VERIFICATION.md file even
though the frontmatter still says passed. The stale status is computed from
file modification times, not stored in frontmatter.

Move the preamble to the verify_completion bash block (the first block with
a gsd_run call) so gsd_run query verification.status can be used for the
advisory check. This gives the full readVerificationStatus logic including
mtime-based staleness detection, matching the enforcement gate at phase.complete.

Capture full JSON (VERIFY_JSON) so next_action can be included in the
advisory output alongside the status.

Also update workflow-size-baseline.json for the updated transition.md size.

Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>

* ci: trigger test matrix for 525b946

Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>

* fix(transition): restore awk frontmatter extraction for pre-shim verification check

The gsd_run launcher shim is not defined until line ~163 of transition.md,
so the verification debt check at line ~80 cannot use gsd_run. Restore the
awk-based frontmatter extraction that correctly reads status without needing
the runtime, and restore the shim at its proper location before
phase.complete.

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

* fix(#1522): clarify transition verification gate wording

* fix(#1522): update transition workflow size baseline

* fix(#1522): update workflow-size-baseline after rebase onto next

Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>

* fix(#1522): guard findStaleVerificationSummary FS calls + thread opts.fs seam (review)

Address review blocker B1 on #1548: findStaleVerificationSummary ran fs.readdirSync
and two fs.statSync calls unguarded between readVerificationStatus's try/catch sections,
so a TOCTOU race (a SUMMARY listed by scanPhasePlans then removed before statSync) or any
FS error threw uncaught into callers NOT under the planning lock (init.manager /
init.progress / uat-predicate). Wrap the body in try/catch degrading to 'not stale', and
thread the injectable opts.fs seam (add statSync to FsLike, pass fsImpl from the caller)
for parity with readVerificationStatus's no-throw contract and testability. Also adds the
Verification Module glossary entry to CONTEXT.md (review B3).

---------

Co-authored-by: Cursor Agent <cursoragent@cursor.com>
Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Co-authored-by: Tom Boucher <trekkie@nomorestars.com>
2026-06-24 13:19:10 -04:00
Tom Boucher
1c073c1c81 fix(#1107): progress consults verification.status before reporting a phase complete (#1116)
/gsd-progress derived phase completeness from plan/summary counts only and never
consulted the verification.status query (the #651 seam), so a phase whose
VERIFICATION.md ended human_needed or gaps_found was reported complete and
routing skipped to the next phase. Add Step 1.7 (consult verification.status for
the current phase) and routing rows that send gaps_found to plan-phase --gaps
(Route V.gaps) and human_needed to verify-work (Route V.human) before the generic
complete row. passed/missing/unknown still route as complete so unverified phases
are not falsely blocked.

Closes #1107

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-12 15:55:01 -04:00
Tom Boucher
7c07fce70f fix(#381): make gsd_run launcher reachable in fresh-shell-per-block runtimes (#1084)
* fix(#381): make gsd_run launcher reachable in fresh-shell-per-block runtimes

On runtimes that execute each fenced bash block in a separate shell process
(e.g. Claude Code — documented behavior: each Bash command is a separate
process; inline shell functions and exported vars do not persist between
calls), the once-per-file gsd_run() function was undefined in every block
after the preamble block, and the call was swallowed by
`2>/dev/null || echo "{}"` into silent empty state.

Fix (budget-neutral session-level resolution):
- Ship gsd-core/bin/gsd_run, a POSIX sh wrapper that symlink-resolves its own
  location and execs the co-located gsd-tools.cjs. Exposed on PATH via the npm
  `bin` field (global installs) and shipped to local installs via the recursive
  gsd-core/ copy.
- The per-file launcher preamble now appends `export PATH='<bindir>':"$PATH"`
  to the file named by $CLAUDE_ENV_FILE (Claude Code's documented
  env-persistence mechanism) so later fresh-shell blocks resolve gsd_run from
  PATH. Guarded as a strict no-op when CLAUDE_ENV_FILE is unset; the inline
  gsd_run() definition remains the fallback for all other runtimes. The
  single-quoted dir neutralizes shell metacharacters at source time.
- Propagated via scripts/sync-runtime-launcher.cjs to all launcher-using files.
- XL workflow byte budget 93000 -> 93200 (the ~130B clause pushes plan-phase.md
  to 93135; legitimate content growth, ratchet-up per #717).

Regression tests (I)/(J) in runtime-launcher-parity.test.cjs cover wrapper
delegation and end-to-end PATH persistence (sourcing the env file with a
space-bearing install path).

Known limitation: an install path containing a literal single-quote yields a
malformed env-file line and falls back to the status quo (no regression);
rare on sanitized home directories.

Closes #381

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

* docs(#381): add changeset for gsd_run fresh-shell reachability fix

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

* test(#381): scope test (J) bare-PATH execution to POSIX (Windows Git Bash exec bit)

Windows Git Bash (msys2) does not honor Node's chmod exec bit for
PATH-executing extension-less scripts, so the bare `gsd_run` command lookup
failed there even though the env-file PATH persistence was correct. The
env-file content assertions (the fix's actual cross-platform logic) still run
on every platform; only the final source-and-execute sub-step is gated to
non-win32. Global installs on Windows are covered by npm's generated bin shim.

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

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-11 21:42:52 -04:00
Jeremy McSpadden
fb37fa7dd5 fix(#725): route Codex gsd-tools calls through shim (#731)
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-10 14:44:43 -04:00
Tom Boucher
a90c654745 fix(#891): probe non-Claude runtime homes in gsd-tools launcher shim detection (#911)
- Updated `gsd-core/workflows/_runtime-launcher.snippet.sh` with 15 new
  `elif` arms covering Hermes, Cursor, Codex, Gemini, Copilot, Windsurf,
  Augment, Trae, Qwen, CodeBuddy, Cline, Grok, Antigravity, OpenCode, and
  Kilo (respecting each runtime's env-var override with a `$HOME`-relative
  default).
- Re-ran `scripts/sync-runtime-launcher.cjs` to propagate the expanded
  snippet into all `gsd-core/workflows/*.md` files (~70 files).
- Manually applied the same snippet update to `commands/gsd/import.md`
  (1 occurrence) and `commands/gsd/graphify.md` (5 occurrences) — these
  are not covered by the sync script.
- Updated `tests/workflow-size-budget.test.cjs` budgets (XL/LARGE/DEFAULT
  + discuss-phase target) to account for the ~3 KB snippet expansion.
- Added regression test `tests/bug-891-non-claude-runtime-home-fallback.test.cjs`
  (6 tests: structural probe presence, ordering, behavioral HERMES_HOME
  env-var + default-path stubs, resolution order, and workflow propagation).
- Added `.changeset/891-launcher-non-claude-runtime-homes.md` (Fixed).

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-08 22:51:40 -04:00
Tom Boucher
463cffd894 chore(#604): rename get-shit-done/ runtime directory to gsd-core/ (#615)
* chore(#604): rename get-shit-done/ runtime directory to gsd-core/

Renames the installed runtime directory `get-shit-done/` to `gsd-core/` so the
on-disk name matches the package (`@opengsd/gsd-core`), repo, and binary
(`gsd-tools`). The npm package name and binary are unchanged; npx/npm consumers
are unaffected.

Mechanical (bulk, ~90% of the diff):
- `git mv get-shit-done gsd-core`
- Swept path/identifier references across the repo via
  `perl -pe 's/get-shit-done(?!-\w)/gsd-core/g'`. The negative lookahead
  preserves the five legitimate slug variants that are NOT the directory:
  get-shit-done-{OLD,cc,classic,cli,redux} (old package/repo names).
- Build/manifest wiring: package.json (bin, files, coverage globs),
  tsconfig.build.json (outDir), ~86 .gitignore build-output entries,
  stryker.config.mjs, scan-ignore files, install.js path strings.
- Frozen (not rewritten): CHANGELOG.md history; translated docs
  (README.<locale>.md and docs/{ja-JP,ko-KR,pt-BR,zh-CN}/).

New logic (review here):
- src/installer-migrations/003-rename-get-shit-done-to-gsd-core.cts: a proper
  ADR-0008 installer migration. On upgrade it walks the legacy
  `~/.claude/get-shit-done/` tree, classifies each file via the prior install
  manifest, and emits remove-managed / backup-and-remove for managed files
  while PRESERVING unknown user-added files. Symlink-safe (skips a symlinked
  root and symlinked entries; bounds-checks every path under configDir). The
  framework rolls back on install failure. Emptied dirs may remain (framework
  has no recursive dir-removal primitive) — documented.
- scripts/lint-legacy-dir-name.cjs: CI regression guard forbidding the bare
  `get-shit-done` directory token (split token to avoid self-match; case-
  insensitive; `(?!-\w)` lookahead allows the slug variants; allowlists
  CHANGELOG, translated docs, and `gsd-allow-legacy-name` marker lines).
  Wired into the lint-tests CI job.
- Restored scripts/lint-package-identity-drift.cjs detection regexes (the
  mechanical sweep had wrongly rewritten the old-name patterns it exists to
  detect) and marked them as intentional legacy references.
- TDD tests for the migration and the guard; do.md slash-command guard regex
  tightened so a `/gsd-core/bin` path segment is not mistaken for a command;
  changeset + docs/installer-migrations.md row added.

Breaking: the installed runtime path moves `~/.claude/get-shit-done/` ->
`~/.claude/gsd-core/`. Migration 003 removes the stale legacy dir's managed
files (preserving user files) on upgrade. Users with custom hooks/configs
hardcoding the old path must update them.

Closes #604

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

* fix(#604): unsweep pending changesets + allowlist injection-example docs

CI fixes for the rename PR:
- Do not sweep pending .changeset/*.md (ephemeral release-note fragments,
  like CHANGELOG); reverted those body edits so 5 pre-existing malformed
  fragments (missing type/pr) no longer enter the PR diff and trip docs-lint.
  Allowlisted .changeset/ in the legacy-name guard accordingly.
- Allowlisted TEST-EXAMPLES.md and docs/explanation/security-model.md in
  prompt-injection-scan.sh: they contain intentional injection examples /
  security-model prose; the path-reference rewrites are kept.

CodeQL alerts on this PR are pre-existing (alert lines unchanged by this PR;
none in the new migration/guard) and are out of scope for the rename.

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

* fix(#604): resolve CodeQL alerts surfaced on this PR

The rename diff touched files carrying pre-existing CodeQL findings; per the
no-pre-existing-dismissal rule, fixing every surfaced alert rather than waving
them off. All behavior-preserving:

- scripts/ci-test-scope.cjs: build the config-path match from string
  .includes() instead of a RegExp over an arg-derived value (js/regex-injection).
- src/profile-output.cts: escape backslashes before pipe-escaping desc/safeName
  so the table-cell escape is complete (js/incomplete-sanitization).
- tests/{bug-2643,bug-2808,docs-parity-live-registry}: two-pass HTML-comment
  strip so a bare/unclosed `<!--` cannot survive (js/incomplete-multi-character-sanitization).
- tests/inline-plan-threshold: drop the no-op `\s`->`\s` identity replace,
  keep the meaningful POSIX-class conversion (js/identity-replacement).

Verified: build:lib green; the touched test files + ci-test-scope + profile-output
suites pass; lint:legacy-name clean.

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

* fix(#604): correctly resolve remaining CodeQL alerts (regex-injection + sanitization)

The prior commit's fixes for two alerts were ineffective:
- ci-test-scope.cjs js/regex-injection: the alert is the CLI-arg-derived `file`
  reaching static regex `.test(file)` calls (not the config rule). Removed ALL
  regex over file/t — startsWith/includes/=== string checks + an isWindowsHint
  helper — so there is no regex sink for the tainted value.
- js/incomplete-multi-character-sanitization (3 test files): a single
  `.replace(/<!--...-->/g,'')` can let `<!--` re-form. Replaced with a fixpoint
  loop (replace until stable) plus a final bare-opener strip.

Verified: no regex over file/t remains; ci-test-scope + the 3 test suites pass;
lint:legacy-name clean.

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

* fix(#604): make ci-test-scope + comment-strippers regex-free to clear CodeQL

CodeQL flags the regex PATTERNS syntactically (regex-injection on the
--files arg split; incomplete-multi-character-sanitization on the <!--...-->
replace), so loop fixes do not satisfy it. Made these paths regex-free:
- ci-test-scope.cjs splitFiles: char-by-char separator tokenizer (no /[,\\s]+/).
- 3 test files: indexOf/slice HTML-comment stripper (no .replace(/<!--/)).
Behavior preserved; ci-test-scope + the 3 suites pass; guard clean.

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

* fix(#604): unblock security base64 scan on the large rename diff

The security job hit its 10m timeout: base64-scan.sh choked on the binary
test fixture tests/feat-3594-parser-property-style.test.cjs (embedded NUL/
non-UTF8 bytes -> thousands of bogus blobs + "ignored null byte" warnings),
and the ~800-file rename diff is slow to scan regardless.

- scripts/base64-scan.sh: skip binary-by-content files (grep -Iq .) — they
  can't carry base64-obfuscated *text* and feeding NUL bytes through the
  per-line scanner is pathologically slow. collect_files already filtered
  binary *extensions*; this catches binary *content* in text extensions.
- .github/workflows/security-scan.yml: raise the security job timeout 10m->30m
  to accommodate very large diffs (the scan itself is unchanged).

Verified locally: scan skips the fixture, 0 "ignored null byte" warnings,
0 findings, exit 0.

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

* fix(#604): sweep get-shit-done refs introduced by merging next

The branch was updated with next (#614/#384/#618 etc.), which reference the
get-shit-done/ dir (still named that on next). Swept the stale references in
the merged files to gsd-core so the rename stays consistent and lint:legacy-name
passes:
- commands/gsd/discuss-phase.md (runtime-launcher shim paths)
- src/core.cts (getAgentsDir layout comments)
- tests/bug-384-agents-runtime-aware.test.cjs (require path to runtime lib)

Verified: guard 0 violations; build green.

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

* fix(#604): exclude gsd-core/ path segments from bug-3683 command cross-ref invariant

The #614 runtime-launcher shim added to discuss-phase.md references
`${_GSD_RUNTIME_ROOT}/gsd-core/bin/...`. bug-3683's REF_PATTERN excluded path-y
refs only via lookbehind, but `}` precedes `/gsd-core/` in the shim, so it
mis-read the directory path as a dangling `/gsd-core` command ref (same class as
the #604 bug-2954 fix). Added a trailing `(?![\w-]*\/)` so `/gsd-<x>/...` path
segments are not treated as slash-command references.

Verified locally on BOTH platforms before pushing:
- mac (node 26) full suite: 0 failures
- gsd-test-runner (linux, node22 image) full suite: 0 failures
- bug-3683 + bug-2954 pass.

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

* fix(#604): lazily resolve findProjectRoot in gsd-tools (harden flaky CI)

CI intermittently failed state.test's gsd-tools subprocess with
"findProjectRoot is not a function" (flip-flopping across legs; not reproducible
on mac full suite, gsd-test linux full suite, test:unit, or state.test x8).
findProjectRoot is a re-export from core.cjs (sourced from project-root.cjs);
binding it via destructure at module-load can be undefined under a load-ordering
edge. Resolve it lazily at call time via a small wrapper so the lookup happens
after core.cjs is fully initialized.

Verified green on BOTH platforms before pushing:
- mac (node 26) full suite: 0 failures
- gsd-test-runner (linux, node22) full suite: 0 failures
- state.test.cjs: 106/106; gsd-tools loads cleanly.

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

* fix(#604): allowlist verification-patterns.md placeholder examples in secret scan

The rename git-mv'd references/verification-patterns.md into gsd-core/, pulling
it into the secret-scan diff. It documents stub/placeholder RED-FLAG env-var
examples (illustrative Stripe test-key / database-URL / API-key placeholders) —
not real credentials. Added it to .secretscanignore with the strict annotation,
mirroring the existing gsd-core/workflows/plan-phase.md exception.

Verified locally: secret-scan-lint --strict OK; secret-scan --diff origin/next
exits 0 with 0 findings.

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

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-02 18:35:29 -04:00