* test(#3023): failing-first guard — pi must not stage hooks in its reserved dir
pi reserves <configDir>/hooks as its deprecated extension location and warns
on every startup when it exists. Assert a pi install stages the shared hook
bundle under gsd-hooks/ instead, manifests it there, and never creates hooks/.
Also adds pi to the local-scope dir table in install-shared.cjs: pi was in
RUNTIME_META but not LOCAL_DIR_NAME, so scope:'local' resolved
path.join(root, undefined) and no local pi install could be exercised.
Fails before the fix. Verified via the remote runner.
* fix(#3023): stage pi's shared hook bundle outside pi's reserved hooks/ dir
pi reserves <configDir>/hooks as its now-deprecated extension location and
warns on every startup when that directory merely exists — checkDeprecatedExtensionDirs()
guards the warning with a bare existsSync(), unlike its tools/ sibling. GSD staged
its shared hook bundle exactly there, and pi's advised remediation (move it to
extensions/) would break the adapter's paths and expose GSD's .js helpers to pi's
extension auto-discovery.
The bundle directory name is now runtime-descriptor-driven: hostBehaviors
.sharedHooksDirName, defaulting to 'hooks' so all 18 other runtimes are
byte-identical. pi sets 'gsd-hooks'. The name is validated as a single path
segment — separators, dot-only segments, trailing dots, absolute paths, NUL,
and Windows reserved device names all fall back to the default, because the
value is joined onto a user's config root and written to.
Renamed in place rather than relocated: hook scripts resolve siblings via
__dirname/.., so a depth change would silently break them.
- install / uninstall / manifest sites all read the resolved name
- pi/gsd.cjs probes gsd-hooks then hooks, so dev checkouts and half-upgraded
trees still resolve; the never-throws contract is preserved
- new migration 009 retires the legacy pi hooks/ dir on upgrade, using a new
non-recursive remove-empty-dir engine primitive (rmdirSync only,
symlink-refusing, containment-guarded); ADR-0008 amended accordingly
- fixes two latent name-dependencies the rename exposed: the stale-hook scan
and the injection scanner's self-exclusion both hardcoded 'hooks'
Verified on the remote runner.
Closes#3023
* fix(#3023): close review findings and align emitted provenance with the rename
Adversarial review found two defects, and the remote runner found four
failure clusters. All fixed here.
Review BLOCKER — detect-custom-files was blind to the renamed bundle.
GSD_PREFIX_MANAGED_DIRS in gsd-tools.cjs hardcoded 'hooks', so for pi the
whole gsd-hooks/ tree was invisible to the custom-file scan and user-added
files there were never backed up before the next update's clean-install wipe.
The dir set now resolves via the .gsd-runtime marker plus the shipped
capability registry (never bin/install.js, which is not shipped into installed
trees), and falls back to scanning every known candidate when the runtime
cannot be determined — over-scanning is safe, under-scanning is the data loss.
Review MAJOR — the pi adapter bound to an empty bundle. resolveSharedHooksDir
accepted any directory, so an interrupted install left gsd-hooks/ winning over
a fully-staged legacy hooks/ and every hook silently no-opped. A candidate now
qualifies only if it is non-empty.
Remote-runner clusters:
- emitted-provenance had no rule for the gsd-hooks/ family; added two pi-scoped
rules pointing at the same sources the existing hooks/ rules use. The table is
total, so an unattributed family is a hard failure by design.
- pi tests in install-minimal-hooks and the install integration suite asserted
the old layout; updated to derive the dir name from the descriptor rather than
hardcoding either name.
- 19 unrelated-looking failures on node22 only were a leaked fs mock: t.after()
runs in registration order, cleanup was registered before mock.restoreAll(),
and node22's JS rimraf calls the public fs.rmdirSync while node24's native
path does not — so the EACCES stub leaked process-wide on one lane. Restore
now runs first.
Verified on the remote runner.
* fix(#3023): honor PI_CODING_AGENT_DIR, ack the rename ripple, fix expandTilde
pi resolves its agent dir as PI_CODING_AGENT_DIR ?? ~/<CONFIG_DIR_NAME>/agent
(packages/coding-agent/src/config.ts). GSD's pi descriptor declared an empty
configHome.env, so a user with that variable set had GSD installed where pi
never looks. Added the env name; the dot-home-nested resolver already handled
the override, so no resolver logic changed.
Also fixes expandTilde in the shared runtime-homes resolver, found while adding
that: it hardcoded os.homedir() and ignored the opts.home every caller threads,
so EVERY runtime's tilde-valued env override (claude, antigravity, windsurf, pi)
silently resolved against the real home. That is a correctness bug and a
test-escape hazard — a sandboxed test asserting on a tilde override reached the
developer's actual home directory. Now threaded through every branch; behavior
with no injected home is unchanged.
Adds the emitted-drift ack fragment for the 58 pi paths whose emitted location
moved with the rename. The provenance rules satisfy the totality gate; the
differential gate needs the ack because the hook sources are byte-unchanged —
only the installer's target directory moved. The two hook files this branch
genuinely edits stay attributed and are not double-acked.
Note on piConfig.configDir: it is read from pi's OWN installed package.json
(getPackageDir walks up from pi's __dirname), alongside piConfig.name — a
white-label setting for a redistributed pi fork, not a per-project user setting.
Documented accordingly rather than treated as an unsupported override.
Verified on the remote runner.
* fix(#3023): reject blank env overrides, pin adapter/descriptor parity
Three review findings, all fixed.
A whitespace-only config-dir override was accepted verbatim: the guard was
`if (val)`, falsy only for the empty string, so PI_CODING_AGENT_DIR=' '
resolved to a literal three-space directory name instead of falling back to the
descriptor default. Fixed across every env-consuming branch — dot-home,
dot-home-nested, all three xdg steps, and generic-agents-root — not just pi's.
Non-blank values are still never trimmed, so '~/My Agent Dir' keeps working.
pi/gsd.cjs's probe list and the descriptor were two independent sources of truth
for the bundle directory name; a future rename would have desynced them silently
and left every pi hook quiet with no error. The probe list stays deliberate — it
must resolve in a dev checkout and a half-upgraded tree, where the registry's
answer would be wrong — so this adds the parity assertion the repo's
generative-fix-divergence rule calls for: the descriptor value must be the FIRST
candidate, and the default must remain present.
Changeset body rewritten to cover the two later user-facing fixes it had not
caught up with.
Verified on the remote runner.
* chore(#3023): backfill changeset PR number
* fix(#3023): anchor injection-scan patterns and fix a macOS detection hole
CI's security job flagged CONTEXT.md:124 — pre-existing prose reading 'not the
same fact as a genuinely empty or absent one'. The match was the 'act as a'
INSIDE 'f-act as a': the pattern had no left word boundary, so any word ending
in act tripped it (fact, impact, contract, artifact, interact, redact,
abstract). My four-line CONTEXT.md edit dragged the latent false positive into
this PR because the scan is diff-scoped by file but reads whole files. Anchored
with (^|[^[:alnum:]]) rather than rewording maintainer-owned prose, which would
have left the class alive for the next PR touching any file saying 'fact as a'.
Auditing the rest of the list for the same class surfaced a real detection hole:
the eval/exec/Function patterns matched a quote via \x27, a GNU-grep-only hex
escape. BSD/macOS grep reads it as four literal characters, so single-quoted
eval('...')/exec('...') payloads were NEVER detected there while passing on
GNU-grep CI. Replaced with a literal apostrophe class.
Boundaries were added only where a real word-suffix collision exists; exec,
jailbreak, developer mode and the role-manipulation family were audited and
deliberately left unanchored. 22 new cases cover both directions — the false
positives now scan clean, and every real payload still fires, including the
quote/punctuation/start-of-line boundary forms.
Also builds this branch's injection test fixture at runtime instead of carrying
the literal phrase, so the payload keeps its teeth without tripping the scan.
Verified on the remote runner.
---------
Co-authored-by: sim <sim@local>
The maintainer decided the default after the ADR first merged, by
consistency with the shipped workflow config: verification gates default
on (research, plan_check, verifier, nyquist_validation,
security_enforcement), agent autonomy defaults off (auto_advance,
research_before_questions, plan_bounce, cross_ai_execution). Installing
probes into tracked source without a second confirmation is autonomy,
not a gate.
Adds Decision 8, flips the legacy/absent-section default and the
precedence tail to off, and marks Open question 2 resolved.
Decision 1's justification is corrected rather than deleted. It rested
on 'adaptive carries no flag', which the new default makes false. The
conclusion is unchanged and the real reason is stronger: precedence
includes the saved session policy, so a resumed session that persisted
adaptive passes no flag either, and a flag-keyed atom would exclude the
section from exactly the sessions already running the protocol.
Closes#3155
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Records the design decisions #3128's maintainer approval made a
condition: schema v1, the probe/artifact ownership model, and the
cleanup state machine that gates terminal transitions.
Also amends ADR-1671 with a RESERVED atom rather than a widening. The
vocabulary stays at 29 until #3128's implementation lands; the
reservation exists so the widening is a coordinated decision rather than
an organic edit found in review.
The load-bearing decision is the atom's shape. #3128's probe policy is
tri-state (adaptive|force|off), so gating on flag:--runtime-probes would
exclude the protocol section from every default invocation -- adaptive
carries no flag -- and the feature's primary mode could never activate.
That is admission gate (2)'s silent-exclusion failure arriving through a
different door: not a fact nobody computes, but a fact computed for only
one of three policies. The atom is therefore a resolved boolean folded
in cmdInitDebug.
Closes#3155
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* test(#3144): bound the git/worktree cluster onto the process seam
Migrates 180 unbounded sync spawn sites across 19 files. Every previously
unbounded call now carries an explicit timeout with a comment giving the
number and why.
The migration is not a callee swap. execSync and execFileSync throw on a
non-zero exit and the seam never does, so each site was classified first:
sites that rely on the throw route to gitOrThrow, and sites that already read
.status to detect an EXPECTED non-zero -- an intended cherry-pick conflict, a
rev-parse outside a repo driving a skip -- route to the never-throwing runGit
instead, which would otherwise throw on exactly the exit being probed for.
Two same-named git() helpers in worktree-cleanup.test.cjs have different
return contracts, one trimmed and one raw; both are preserved rather than
unified.
Collapses five hand-rolled throw wrappers onto one throwIfFailed in
git-fixture.cjs, which gitOrThrow now also uses so the shape cannot drift.
Allowlist drops 139 to 120; BASELINE lowered to match.
* test(#3144): fix pre-PR review findings
Documents throwIfFailed in the CONTEXT.md glossary and CONTRIBUTING.md --
it became the shared throw mechanism without either doc naming it.
Routes the sixth and seventh hand-rolled copies of the throw shape through
throwIfFailed (worktree-baseref-install, worktree-safety-reap); the first
consolidation missed both.
Converts ci-rebase-check's 8 fixture-setup calls from unchecked runGit to
gitOrThrow so a failed setup step aborts where it fails rather than
surfacing later as a confusing failure against the wrong subject.
Adds 12 direct unit tests for throwIfFailed, which until now was only
exercised transitively.
Splits verify.test.cjs's non-git grep/sed bound off GIT_TIMEOUT_MS.
---------
Co-authored-by: sim <sim@local>
docs/ is never passed through the install-time slash-form converters, so
the colon form names a command no runtime registers. Caught by
lint-docs-command-form.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two failures from the remote runner on 654b2cc10, both introduced here.
1. tests/mcp-catalog-parity.install.test.cjs greps emitted workflow files
for the bare substring 'gsd:section' and treats its presence in a
composed file as an un-stripped marker. debug.md's new Step 0 prose
documented the field by writing that token literally, so the emitted
file tripped the gate even though the parser correctly ignored it as
prose. Reworded to 'applicability-section markers'. Same class as
DEFECT.PROMPT-INJECTION-SCAN-COLLISION.
2. tests/debug-session-management.test.cjs asserted debug.md contains the
literal 'config-get workflow.tdd_mode'. That call is gone. The
invariant it protected -- tdd_mode comes from the workflow.tdd_mode
key, never a bare top-level one -- is unchanged, and now has a
stronger behavioral home: init-debug.test.cjs row A9 asserts a bare
key is ignored and the canonical key is honored, whatever the read
mechanism.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* test(#3143): add no-unbounded-spawn guard and throw-preserving git fixture
Adds the ESLint rule local/no-unbounded-spawn, wired into the tests/**/*.cjs
block, plus an allowlist that only ratchets down: a listed file with zero
violations reports its own entry as stale.
The rule resolves renamed destructures and chained requires rather than
matching literal callee names -- both forms exist in the suite today and a
name-only matcher leaves them permanently invisible. It resolves an options
object held in a single-write const, which is what keeps process-seam.cjs,
the bounded reference implementation, from flagging itself.
timeout: 0 and anything above the 600000ms ceiling are rejected as only
nominally bounded.
Adds tests/helpers/git-fixture.cjs so a migrated execSync call site keeps
its throw-on-non-zero contract; process-seam.cjs is unchanged.
* test(#3143): prove the allowlist guards can actually fail
Extracts the D4/D6/D7/D8 checks into pure helpers and drives each against a
synthetic fixture carrying an injected violation. Without this the suite only
proved that today's clean data passes, which a deleted check would also
satisfy.
* fix(#3143): close two ceiling and alias escapes found in review
Nested arithmetic bypassed the ceiling entirely: the numeric evaluator only
resolved a flat literal, so `timeout: 60 * 60 * 1000` (3600000ms, six times
the ceiling) fell through to trusted and reported nothing. The evaluator now
recurses through arithmetic and unary signs with a depth cap.
Alias resolution was traversal-order dependent, not scope dependent: a call
textually above its own require destructure saw an empty alias map and
reported clean. The map is now built in a Program pre-pass.
Also: an explicit timeoutMs:undefined no longer overwrites the git fixture
default via spread, adds the missing seam-routed rule test, and de-duplicates
the repeated try/catch in the fixture tests.
---------
Co-authored-by: sim <sim@local>
The debug_dir change introduced a second planningPaths(cwd) call in the
same function. Bind the struct once and read both .planning and .debug
from it; emitted output is byte-identical.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
/gsd:debug was one of the last workflows with no cmdInit* of its own: its
Step 0 made three separate round-trips (state.load, resolve-model
gsd-debugger, config-get workflow.tdd_mode) to assemble one context. Because
no debug-scoped fact was computed at any entry point, ADR-1671 admission gate
(2) could never be satisfied for debug — an applicability atom naming such a
fact would evaluate FALSE forever and silently exclude its section.
Adds cmdInitDebug (init.debug), registers it in the init router and the
command-alias table, and collapses debug.md Step 0 to one call. Every field
resolves through the same primitive the call it replaces used: loadConfig for
commit_docs, withProjectRoot for response_language (#2402), planningPaths for
debug_dir, resolveModelInternal for debugger_model, and the existing
Boolean(workflow.tdd_mode) idiom for tdd_mode.
PlanningPaths gains a debug field so state.load and init.debug share ONE
debug-directory expression rather than two kept in sync by hand. state.load
keeps emitting debug_dir: it is a shipped query surface with its own test
anchor, so narrowing it would break unseen consumers for no gain.
No WHEN_VOCABULARY atom and no gsd:section marker: gate (1), a consuming
section of at least 400 bytes, belongs to the change that adds the section.
Closes#3149
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Adds the behavioral matrix for a dedicated init.debug handler before the
handler exists: equivalence cross-checks against the three calls debug.md
makes today (state.load, resolve-model, config-get workflow.tdd_mode),
bundle shape, the CLI negative/hostile argv matrix, section_manifest
null-vs-[] degradation, planningPaths.debug, and a guard that
WHEN_VOCABULARY stays closed at 29 entries.
Currently RED: the init router reports "Unknown init workflow: debug".
Matrix: .gsd/phase/feat-3149-cmdinitdebug/50-test-matrix.md
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(#3086): apply #2667 .cmd-shim gate to deps.spawn + surface errorCode in review lanes
deps.spawn used shell:false with a bare binary name — on Windows, npm-installed
CLIs (gemini, codex, etc.) are .cmd shims that CreateProcess cannot start,
producing ENOENT + empty stderr. The review path then wrote an empty err file
and emitted a generic 'failed or returned empty output' stub.
Two fixes:
1. deps.spawn: detect .cmd/.bat on win32 and mediate through cmd.exe /d /s /c
(same gate as runWithTimeout #2667, same explicit argv array).
2. runSpawnLane: surface errorCode (ENOENT, ETIMEDOUT) in the err file so the
stub explains WHY the lane produced nothing.
* chore(#3086): backfill changeset PR number 3142
---------
Co-authored-by: sim <sim@local>
* fix(#3079: query commit no longer resurrects deleted phase branches via silent switch
git checkout -b both created AND switched HEAD, so when branching_strategy:
'phase' deletes its branch on merge, a later query commit silently
recreated it and moved HEAD there. The commit landed on the wrong branch.
Fix: replace checkout -b with git rev-parse --verify + git branch (create-
only, no switch). The commit always lands on the current branch. Callers
that want to be on the phase branch use execute-phase's handle_branching.
Updated 3 existing tests that asserted the old switch behavior.
* chore(#3079): backfill changeset PR number 3141
---------
Co-authored-by: sim <sim@local>
* fix(#3052): preserve frontmatter last_activity_desc on same-date body prose conflict
preferNewerLastActivity only preserved last_activity_desc when the derived
date was OLDER than the existing frontmatter date. When the dates matched
(same-date), the derived body prose (potentially stale) overwrote the
authoritative frontmatter desc.
Fix: when derDate === exDate, also preserve the frontmatter desc.
* chore(#3052): backfill changeset PR number 3140
---------
Co-authored-by: sim <sim@local>
* fix(#3099): emit LAST_ACTIVITY_UNPARSEABLE diagnostic when last_activity is present but unparseable
parseActivityTimestamp returned null for both absent AND present-but-unusable
last_activity values, silently suppressing the idle-stranded recommendation.
Per ADR-1411's amendment (corrupt is not absent), the fallback stays but a
diagnostic is now emitted via the warnUnusableInput seam.
- Added LAST_ACTIVITY_UNPARSEABLE to UNUSABLE_REASON enum + prose
- Wired warnUnusableInput into detectSignals when lastActivityRaw is truthy
but parseActivityTimestamp returned null
- Updated UNUSABLE_REASON lock test
- Added 5 regression tests (unusable→diagnostic, absent→silent, well-formed→silent, dedup)
* chore(#3099): add changeset fragment
* chore(#3099): backfill changeset PR number 3139
---------
Co-authored-by: sim <sim@local>
* fix(#3132): realign spec/plan/ui-phase workflow prose from retired covered/backstop-as-status to resolved+verification
The edge-probe resolution model splits status (resolved|dismissed|unresolved)
from verification (explicit|backstop). The workflow prose in three files still
used the pre-re-cut covered/backstop-as-status vocabulary that validateResolution
rejects.
Swept all three prose surfaces:
- spec-phase.md: Step 5.5 resolution options, --auto mode + log line, comment, Step 6 row list
- plan-phase.md: lift rule (L778/L780), comments (L564/L706), quality gate (L826-827)
- ui-phase.md: resolution loop (L391), --auto mode (L405-409), write-back format (L415)
Added regression test in edge-probe-spec-phase-contract.test.cjs asserting the
retired vocab is absent and resolved+verification is used instead.
* chore(#3132): add changeset + emitted-drift ack for workflow vocab realignment
* fix(#3132): update planner contract tests for resolved+verification vocabulary
RR-02 and RR-03 tests asserted the old covered/backstop-as-status vocab.
Updated to match the realigned prose (resolved edge → must_haves).
* fix(#3132): fix specless-probe-fallback test assertion + merge duplicate ack
Test assertion was too strict (expected auto-resolved + verification:explicit
on same line). Split into two independent assertions.
Merged plan-phase.md ack into existing #2658 fragment to resolve duplicate-path
rule violation.
* fix(#3132): use bare filenames in ack keys (size map keys are bare, not full paths)
* fix(#3132): amend existing acks instead of duplicating — remove plan-phase from #2658, spec-phase from #3132, append #3132 reason to #0000 and #2650
* chore(#3132): backfill changeset PR number 3138
---------
Co-authored-by: sim <sim@local>
* test(#3116): failing-first — parseLedger throws on CRLF WINDOWS.md
On repos with core.autocrlf=true (Windows default), .planning/WINDOWS.md
is checked out CRLF. The \n--- close-fence scan leaves the last frontmatter
line's CR attached, and the key:value regex's . doesn't match CR, so the
parser throws WINDOWS_LEDGER_MALFORMED on the last key.
* fix(#3116): strip trailing CR per line in parseFrontmatterStrict
The \n--- close-fence scan lands on the LF of the last frontmatter line's
CRLF, so yamlBody ends with a bare \r. split(/\r?\n/) strips CR from
interior lines but the last line's \r survives. The key:value regex fails
because . doesn't match CR.
Fix: strip \r per line (rawLine.replace(/\r$/, '')) rather than normalizing
raw — the writer round-trips raw byte-exact.
* chore(#3116): add changeset fragment
* chore(#3116): backfill changeset PR number 3137
---------
Co-authored-by: sim <sim@local>
* test(#3118): failing-first coverage for the dead injectables and the shell projection
Adds the counter-tests Wave 4 closes against, before any fix:
- antigravityWatermark had zero test references. The four existing tests
that look like watermark coverage hand the fallback a literal mark and
never call the producer, so nothing pinned whether a real run's mark is
correct. Covers all six branches plus the non-object cache classes.
- Pins the fail-open: a transcript read that throws reports lines:0,
indistinguishable from a genuinely empty transcript, and the consumer
then replays a previous run's review as this run's.
- Pins the export-line escaping across the repair, persist and win32 bash
lanes, including the parity assertion that they must not diverge.
- sliceCurrentPositionSection: empty-vs-absent, fenced heading, second
occurrence, H3, CRLF.
- Proves deps.progressProvider is inert by supplying a throwing stub to
all ten transition intents.
Verification through the remote runner only.
Refs #3118
* fix(#3118): distinguish an unreadable transcript from an empty one
antigravityWatermark's final read can throw on a transcript that
indisputably exists. It returned lines:0, which is the same value a
genuinely empty transcript produces, so the caller could not tell the
two apart.
antigravityTranscriptFallback derives its skip from that count. A mark
of {convId:'c1', lines:0} for a conversation that pre-dates the run
makes it skip nothing and return the last PLANNER_RESPONSE in a
transcript written before this run started — a previous review
presented as this one's, which is exactly what the function's own
'never stale' docstring promises cannot happen.
The unreadable case now sets unreadable:true and the fallback declines
for a same-conv-id unreadable mark. An absent or empty transcript is
untouched: those genuinely have zero prior lines.
* fix(#3118): escape the export line for the file it lands in, not the echo
Three lanes emit export PATH="<dir>:$PATH". repair escaped it with
escapePosixDoubleQuoted; persist and the win32 Git Bash lane escaped it
with escapeSingleQuotedShellLiteral instead.
The single-quoting is correct for the echo, so nothing runs when the
user pastes the command. But the bytes appended to ~/.bashrc are the
export line itself, and inside double quotes in an rc file a $(...) or
a backtick in the directory name is command substitution that runs on
every new shell. Those characters are legal in a path on both POSIX and
Windows, so the path was reachable.
projectPathExportLine is now the single source of that line and escapes
for its final rc-file context; each lane still applies its own transport
escaping on top. fish keeps the single-quote escaper — its value really
does stay single-quoted.
The cmd.exe lane interpolated into a cmd double-quoted string with no
cmd-level escaping, so a quote closed the region and &cmd& ran. A quote
is reserved on Windows and cannot appear in a real path, so there is no
correct command to suggest: the win32 lanes now fail closed for one.
Metacharacter-free paths render byte-identically on every lane.
* fix(#3118): drop a stray carriage return and a deps field nobody reads
locateCurrentPosition subtracted a fixed one byte to exclude the newline
before the next heading, which assumes LF. On a CRLF document the slice
kept an unpaired trailing carriage return. It now walks back over the
newline and over a preceding carriage return if there is one.
StateTransitionDeps also required a progressProvider that 33 sites
supplied and no site ever called. A required field nothing reads widens
the module's interface without changing its implementation, which is the
shape epic #3051 cites as its reason for refusing blanket injection.
Removed along with the ProgressRecord alias that existed only as its
return type; state-document.cts's unrelated interface of the same name
is untouched.
* fix(#3118): stop an empty span duplicating bytes, and name the empty results
Three findings from the isolated review pass.
locateCurrentPosition could return end < start when the section was
empty and the next heading followed with no blank line between. Every
mutator splices with slice(0,start) + body + slice(end), so an inverted
span duplicated the region between them — a blank line silently
inserted into STATE.md on every transition, two bytes on CRLF. The span
is now clamped, and an empty section is a zero-length span, which is
what it always meant.
The win32 fail-closed path left the installer printing 'Add it with one
of:' with nothing under it. An empty shellActions folded two different
facts together, so projectPathActionProjection now carries a frozen
PATH_ACTION_REASON and the installer branches on it. Two empty results
with different causes staying distinguishable is the subject of the
epic this belongs to.
fish_add_path parses a leading dash as an option, so a directory named
-v printed 'No paths to add' instead of being added. Verified against
fish 4.8.1: the end-of-options separator fixes it.
Replaces the console-prose test the second fix first arrived with — a
regex over captured stdout is what CONTRIBUTING prohibits, and the
typed reason is the surface it asks for instead.
* fix(#3118): escape TOML control characters, and stop a test name overstating
Five findings from the two review axes.
escapeTomlDoubleQuotedString escaped only backslash and quote. TOML
basic strings also require U+0000-U+0008, U+000A-U+001F and U+007F to be
escaped, so a value carrying a raw newline or NUL wrote a config.toml no
parser accepts — rejecting the whole file, not just that value. Four of
its call sites write real config. Tab stays raw; the grammar exempts it.
The byte-identity test claimed every lane was unchanged for an ordinary
path, which is false: fish now takes the end-of-options separator on
every path, not only hostile ones. Renamed, and the one intended delta
now has its own named test instead of hiding inside a claim that read
as broader than it was.
Also: exact-equality assertions in place of substring checks that could
pass on a subtly wrong escape, newline and null-byte cases for all five
quoting primitives, and a temp dir registered with t.after so it is
removed when an assertion fails.
* docs(#3118): add the changeset fragments
* fix(#3118): degrade instead of throwing on a null conversation cache
A cache file whose whole content is the literal null — what a truncated
or zeroed write leaves behind — made both antigravityWatermark and
antigravityTranscriptFallback throw. JSON.parse('null') succeeds, so the
try/catch wrapping the parse never fired, and resolveConvId then called
hasOwnProperty on null.
Both functions advertise the opposite; the existing test next to them is
named 'a missing cache or transcript degrades to empty, never throws'.
Parsing successfully is not the same fact as the payload being usable,
and a guard that only wraps the parse cannot tell them apart.
resolveConvId is now total for any non-object input, so one guard covers
both callers. Caught by the null case in this wave's own cache matrix.
* test(#3118): correct a stale fish expectation and a parity comparison
The pre-existing 'POSIX persist mode escapes single quotes' test pinned
fish_add_path without the end-of-options separator this wave adds, so it
asserted behavior that is no longer correct. A repo-wide scan found one
such hardcoded expectation; every other site derives its expectation
from the projection.
The new parity test compared the token from a POSIX path against the
win32 lane, which posix-normalizes its input first — two different
inputs, so the tokens differed for a reason that had nothing to do with
the parity it claims to check. It now derives the win32 expectation from
the same input the lane receives.
* docs(#3118): reword a comment the injection scanner reads as an instruction
The scanner pattern act\s+as\s+(?:a|an|the)\s+ carries no word
boundary, so 'the same fact as the payload' matched on the tail of
'fact'. Reworded per the documented remedy for this collision.
The missing boundary is a scanner defect rather than a prose problem —
any contributor writing 'fact as the' trips it — but the pattern is
gate plumbing, which the sibling epic owns, so it is surfaced rather
than changed here.
* chore(#3118): backfill changeset pr number to 3124
* chore(#3118): backfill changeset pr number to 3124
* fix(#2784): make the negation scan single-pass and index it correctly
Three defects in the negation suppression added by #3127, all in one
block, none of which had a test.
The pair scan was verbs.some(nouns.some(...)) with a slice and a split
per pair, so it grew cubically with clause length: 1.1ms before that PR
and 8462ms after, on 800 verb+noun pairs in one clause. api-coverage's
property test generates documents large enough to reach the runner's
600s file cap, which is why it hangs as 'fail 0, cancelled 1' rather
than failing an assertion. Every (verb, noun) window is a subset of the
single widest one, so one scan of that window answers the same question
in a linear pass. Verified equivalent against the old predicate over
20,000 generated clauses.
Both checks also subtracted clause.start from offsets that collectTerm-
Matches already returns clause-local. The first clause on a line has
start 0 so it worked there and nowhere else: later clauses went
negative, and slice reads a negative index from the end, so suppression
silently examined unrelated text.
The comment claimed 'without any API integration' was suppressed. It is
not — the qualifier sits outside the two-word lookback and the noun
precedes the verb. Widening the window would trade a false positive
that costs one declaration line for a false negative that slips a real
integration past a blocking gate, so the behavior stands and the
comment now says so. Pinned by a test.
The qualifier sets were also rebuilt for every line of every document.
* fix(#2665): scrub config-location env vars in TEST_ENV_BASE
TEST_ENV_BASE scrubbed 14 session-identity vars but omitted CLAUDE_CONFIG_DIR,
GSD_RUNTIME, and CODEX_HOME. The config-home resolver (runtime-homes.cts)
consults these env vars BEFORE the HOME-derived fallback, so an ambient
value won unconditionally over a sandboxed HOME. npm test wrote fixtures
into the developer's live config directory when any of these were set.
One leaked fixture was a registered skill (gsd-dev-preferences/SKILL.md)
carrying behavioral directives that loaded into subsequent sessions.
All three config-location vars are now blanked in TEST_ENV_BASE. Per-site
overrides still win (env is spread last in the child-env merge).
* chore(#2665): backfill changeset PR number 3134
* ci: retry shard timeout flake (#2665)
* ci: retry shard-2 timeout flake (#2665)
---------
Co-authored-by: sim <sim@local>
* fix(#2786): skip sentinel phase ids in phase-complete stage 2 heading scan
Stage 2 of the next-phase cascade accepted any higher-numbered roadmap
heading without checking the 999.x backlog sentinel convention that stage 1
already checks. A Phase 999.1: Backlog Item heading was treated as the next
real phase, advancing STATE.md into the backlog and making the milestone
perpetually 'Ready to plan' instead of 'All phases complete'.
Added isSentinelPhaseId(pm[1]) guard (mirrors stage 1's /^999(?:\.|$)/ check)
so both sentinel ranges (0.x drafts, 999.x backlog) are skipped.
* chore(#2786): backfill changeset PR number 3130
---------
Co-authored-by: sim <sim@local>
* fix(#2784): suppress detectApiIntegration on negated prose clauses
The compound verb+noun rule had no negation awareness. A clause like "This
phase integrates no external API" matched verb=integrates + noun=API and
fired a false positive, halting the blocking verify:pre gate and forcing a
coverage matrix for a non-existent API.
Added clause-local negation suppression: if the clause contains an
unambiguous negation qualifier (no, not, without, zero, neither, nor,
none, and common contractions), the pair is suppressed. True positives
("integrate the Stripe API") are unaffected. The human override (COVERAGE.md
"no integration" declaration) remains valid.
* chore(#2784): backfill changeset PR number 3125
* chore(#2784): fix changeset PR number to 3127
* chore(#2784): trigger CI re-run
---------
Co-authored-by: sim <sim@local>
* fix(#3044): add zh-CN verification-patterns.md to secret-scan exclusions
The translated document carries the same illustrative placeholder examples
(Stripe test-key, database-URL, API-key) as the English source, which is
already excluded. The exclusion was never extended to the zh-CN translation,
causing the strict-mode scan to fail. No other locale has a translation of
this file (verified: ja-JP, ko-KR, pt-BR do not have it).
* chore(#3044): backfill changeset PR number 3122
---------
Co-authored-by: sim <sim@local>
* docs(#3043): add caution blocks for --dangerously-skip-permissions
The flag was presented without a caveat in docs/USER-GUIDE.md,
docs/tutorials/onboarding-an-existing-codebase.md, and all four translated
locales. Only the English first-project tutorial carried a proper [!CAUTION]
block. All 10 uncaveated occurrences now carry the same caution block
(optional flag, throwaway/low-stakes use, how to keep confirmations, link
to security model).
* chore(#3043): backfill changeset PR number 3121
---------
Co-authored-by: sim <sim@local>
* fix(#3039): clamp max/xhigh effort to high for Claude-runtime skills
effort: max in plan-phase, execute-phase, and autonomous SKILL.md frontmatter
was passed through as output_config.effort, which the Anthropic API rejects
when extended thinking is disabled (400: effort 'max' is not supported when
thinking is disabled on this model). The frontmatter is static at install
time and the installer cannot know whether thinking will be on or off at
invocation.
normalizeClaudeSkillEffort now clamps both 'max' and 'xhigh' to 'high' —
the maximum value that works in both thinking states on all supported models.
Applied in both src/runtime-artifact-conversion.cts and bin/install.js.
* chore(#3039): backfill changeset PR number 3119
* fix(#3039): regenerate skills with clamped effort: high
---------
Co-authored-by: sim <sim@local>
* fix(#3036): accept non-numeric-leading phase ids in roadmap.analyze
roadmap.analyze's phase-heading discovery and checklist discovery regexes
required a digit-first id (\d+...). A project using letter-prefixed ids
(e.g. B7, P0.3-2) got phase_count: 0, current_phase: null, next_phase:
null — even though get-phase and execute-phase resolved the same ids fine.
Widened all three regex sites (phasePattern, checklistPattern, nextHeader
section boundary) to accept an optional leading letter prefix
([A-Za-z]?\d+...). Existing numeric-leading ids are unchanged.
* chore(#3036): backfill changeset PR number 3117
---------
Co-authored-by: sim <sim@local>
* fix(#3035): add kimi-code detection and flag to review workflow
The kimi-code reviewer lane was declared in REVIEWER_LANES, documented in
docs/COMMANDS.md, resolved via --kimi-code, and functional when reached —
but review.md's detect_clis hardcoded 11 of 12 lanes (no kimi probe) and
the flag-parse list omitted --kimi-code. /gsd:review --kimi-code could
never reach SELECTED_REVIEWERS.
Added command -v kimi detection and the --kimi-code flag to the review
workflow's CLI detection and flag-parse steps.
* chore(#3035): backfill changeset PR number 3115
---------
Co-authored-by: sim <sim@local>
* fix(#3033): resolve zero-plan split-parent phase as complete when roadmap checkbox is checked
A phase split into sub-phases (parent kept as shared context, zero plans
by design) was permanently stuck as 'researched' because the roadmap-
checkbox override at line 2266 required completion.phase_complete (derived
from plan/summary counts), which is always false for zero-plan phases.
The parent was permanently eligible for current-phase selection and
re-planning recommendations.
The override now fires when roadmapComplete AND planCount === 0 (the
split-parent shape), treating it as complete regardless of the plan-count
derivation. A zero-plan phase whose checkbox is still unchecked stays
in-progress (researched). Ordinary phases with plans are unchanged.
* chore(#3033): backfill changeset PR number 3114
---------
Co-authored-by: sim <sim@local>
* fix(#3029): remove redundant hooks declaration from plugin manifest
.claude-plugin/plugin.json declared "hooks": "./hooks/hooks.json"
explicitly. Claude Code already auto-loads hooks/hooks.json by default,
so the explicit declaration caused a duplicate-rejection that silently
disabled every hook the plugin ships — security guards, monitors,
injection scanners. The failure had no visible signal in normal use.
Removed the redundant hooks field. Updated the manifest test to assert
ABSENCE of the field and that the auto-loaded file still exists on disk.
* chore(#3029): backfill changeset PR number 3113
---------
Co-authored-by: sim <sim@local>
* fix(#3026): document --pi and --gemini in installer --help
--help documented 16 runtime flags but the installer accepts 18: --pi
(named only in banner prose) and --gemini (invisible everywhere). Both
install correctly when passed but are undiscoverable via --help.
Added both to the Options section. Added a parity test that asserts every
accepted runtime flag appears in --help output, with an exclusion set for
legacy aliases (--both, --kimi-code).
* chore(#3026): backfill changeset PR number 3112
---------
Co-authored-by: sim <sim@local>
* fix(#3022): suppress spurious typebox fallback warning on Pi startup
The Pi adapter's buildGsdInvokeParameters attempts require('typebox') and
falls back to a plain JSON-Schema object when it's unavailable (typebox is
NOT a gsd-core dependency). The fallback is the normal, expected path —
typebox is never installed in a Pi extension context. But the code emitted
a process.stderr.write warning on every startup, alarming users.
Removed the warning; the catch comment now explains the fallback is normal.
* chore(#3022): backfill changeset PR number 3111
---------
Co-authored-by: sim <sim@local>
* fix(#3021): recognize worktree-wf_* branch namespace in all guards
The Claude-orchestration Workflow backend (#1143) creates per-plan
worktrees on branches named worktree-wf_<runid>-<n>. Four independent
copies of the agent branch allow-list regex (^(worktree-)?agent-...) never
learned this namespace:
- hooks/gsd-worktree-path-guard.js:176 — FAILED OPEN (process.exit(0)),
silently disabling path containment for exactly the concurrent dispatch
mode where cross-worktree writes are most likely
- src/worktree-safety.cts:21 — silently dropped cleanup-wave manifest
entries
- agents/gsd-executor.md:503 — FATAL halt on branch check
- gsd-core/references/worktree-branch-check.md:33 — same FATAL halt
Extended all four to ^((worktree-)?agent-|worktree-wf_)[A-Za-z0-9._/-]+$.
The path guard now correctly blocks cross-worktree writes for Workflow-
backend branches instead of no-op'ing.
* chore(#3021): backfill changeset PR number 3109
---------
Co-authored-by: sim <sim@local>
* fix(#3020): verify graphify tool identity before reporting compatibility
checkGraphifyVersion ran 'graphify --version' on PATH and trusted any
plausible version string — a foreign binary named 'graphify' that happened
to print a version would silently report compatible:true with no warning.
Downstream graphify work would then proceed against the wrong tool and fail
later in ways that looked like GSD defects.
After getting a version from the binary, verify the graphifyy Python package
via importlib.metadata. If the package cannot be confirmed, emit a warning
naming the mismatch regardless of version-range compatibility. The
compatible flag is now correctly false for unverified tools (it was
previously read by nobody — the warning is what surfaces).
* chore(#3020): backfill changeset PR number 3107
---------
Co-authored-by: sim <sim@local>
The Windows shard failed on the worktree-root assertion:
actual C:/Users/runneradmin/AppData/Local/Temp/gsd-wt-info-Nn4hj6
expected C:\Users\runneradmin\AppData\Local\Temp\gsd-wt-info-Nn4hj6
The value comes straight from `git rev-parse --show-toplevel`, and git reports
POSIX separators on every platform. The expected side was built with the native
realpath, so the test encoded the separator convention of the machine it was
written on. Normalising the expected side keeps the assertion exact — on POSIX
the replacement is a no-op, so nothing is weakened where it already passed.
This is the same assertion that was strengthened earlier today from a
typeof-string-and-non-empty shape check. Pinning the exact path was right; the
weaker version would have passed on Windows precisely because it asserted
almost nothing. Getting a real assertion wrong on one platform is the better
failure, and the Linux-only matrix could not see it — the platform shards
caught it, as they did twice in the previous wave.
Every other path comparison in the two new test files was swept for the same
mistake. The remaining ones are safe: the base-branch tests compare against
literal POSIX strings supplied to a mocked git, and the reap tests put both
sides through one canonicalising helper, so they cannot disagree on separators.
Drive-letter case can differ in principle at the fixed site; it did not here and
no case-folding was added on speculation.
Refs #3057
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The fragment carries the real PR number because changeset-lint rejects the
pr: 0 placeholder, so it can only be written once the PR is open.
Refs #3057
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(#3017): scope state write's disk scan to stored milestone
buildStateFrontmatter called getMilestonePhaseFilter(cwd) WITHOUT the
stored milestone version, so it auto-derived from ROADMAP.md — and when
getMilestoneInfo mis-bound (the stored milestone had no matching non-✅
heading), it picked a confidently-wrong milestone and clobbered the stored
value + rewrote progress with whole-project counts on every state.* write.
Pass the stored milestone from STATE.md frontmatter through to
buildStateFrontmatter and use it as the explicit versionOverride for
getMilestonePhaseFilter. When the stored milestone is available, the filter
scopes to it instead of auto-deriving.
* chore(#3017): backfill changeset PR number 3105
---------
Co-authored-by: sim <sim@local>
Review found the previous commit fixed the wrong cliff. process.kill accepts a
pid up to 2147483647 and throws a TypeError above it, so 2147483648 — an
ordinary finite number — sailed past the finite check, threw, and was
classified dead. Measured here: 2147483646 and 2147483647 raise ESRCH,
2147483648 and above raise ERR_INVALID_ARG_TYPE. The digit-length boundary the
last commit pinned was a different, earlier gate, and its tests implied it was
the meaningful one.
The liveness helper now treats only ESRCH as dead. EPERM, a type error from an
out-of-range pid, an error with no code at all — every outcome it does not
recognise returns alive, because the value feeds a forced worktree removal and
an unrecognised failure must never read as permission to delete. Against a
build with the old catch, a lock holding 2147483648 reaps the worktree; with
this one it is skipped and the directory survives.
The finite check stays. It no longer carries the safety, but it still names
garbage input accurately rather than reporting it as a live owner, and its
comment now says that so it is not removed as redundant.
The freshness verdict is split. An unreadable lock mtime and a genuinely recent
lock both reported lock_too_fresh, which tells an operator to wait when waiting
cannot help — the same conflation this module already separated for parse
failures. The unreadable case is now lock_age_unknown. Only this module and its
tests read these strings; no workflow or command consumes them.
The dependency spread in the prune path is kept as it is, with a test that
kills it. A caller-supplied porcelain parser displacing the default is the
intended seam, not an accident: the parser is a declared member of the
dependency type and the planner already prefers an injected one, so the
hard-coded key restates the same default rather than guarding against
override. Previously nothing exercised that line at all.
Refs #3057
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A lock file whose PID is a digit string longer than 308 characters parses to
Infinity, not NaN. The default liveness helper then calls process.kill with it,
which throws a TypeError rather than an errno error, and that helper's catch
only recognises EPERM — so it returns false, meaning "the owner is dead", and
the worktree becomes eligible to be removed.
The reaper already fails closed for this exact situation. It wraps the liveness
call in a catch that sets alive and does not reap, with a comment saying
liveness could not be determined. That protection never fires here, because the
inner catch swallowed the error first and answered confidently instead of
admitting it did not know. A guard that cannot verify safety reporting success
is the defect this whole epic is named for, and it was sitting inside the one
function in the tree that deletes things.
The guard deleted earlier on this branch tested for NaN. That test really was
dead — a digits-only capture cannot parseInt to NaN — but the reachable failure
is non-finite, so removing it without correcting the predicate left the hole
open. The check is now for a finite value, and a malformed PID reports the same
lock_owner_unknown skip as an unparseable one, since both mean the same thing:
the owner is unknown, so nothing is removed.
Measured, not assumed: 308 nines still parse finite, 309 are Infinity. All
three of that boundary are covered, along with a 400-digit case that asserts
the worktree is still on disk afterwards — the consequence, not just the
verdict. Against a build with the guard removed, that case reports
pid_dead_and_merged and the worktree is gone.
The liveness helper's own catch still maps every non-EPERM error to "dead". The
fix belongs where the value stops being trustworthy rather than at the far end
of it, but that helper is worth revisiting on its own terms.
Refs #3057
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Thirty-four tests covering every branch in the reaping path that no test
reached, which was all of the fail-closed ones. The function has always
accepted an injectable dependency bag; nothing used it. Every existing test
drove real git and injected only the clock and the liveness probe, so each
guard that exists for a failure — an unreadable git dir, a null directory
listing, an unresolvable remote ref, a missing pointer file, an unlocked
sibling, an ambiguous remote — had never executed. They are now driven by
injecting exactly the fault that selects them, and each asserts its specific
status and reason rather than that something happened.
The last one needed no new mechanism, only the right one. It was reported as
unreachable without a cross-user PID, but the default liveness helper is
reachable by not injecting over it and patching process.kill, which is the
deterministic injection this repo requires over real OS conditions. Its three
outcomes — EPERM, ESRCH, and a clean return — now assert their verdicts.
The assertions were kill-tested rather than assumed. Against mutated copies of
the built module, renaming the six reason strings fails eighteen tests,
neutralising the fail-closed returns fails seven more, dropping the
ambiguous-remote guard fails one, and removing the prune catch and its timeout
guard fails both prune tests. Flipping the EPERM arm to false turns a skip into
a reap and fails that test.
Four places where production folds distinct causes into one verdict are
recorded in the tests rather than papered over. A lock is too fresh whether its
mtime is unreadable or merely recent; a branch tip fails to resolve for three
different reasons; a PID reads as alive whether the owner lives or the probe
threw. Where the return value cannot separate them the tests assert the git
call sequence instead, and where even that cannot, the test says so.
Six weak assertions already in the older file are replaced rather than left
beside the new ones: five guarded their assertions behind `if (entry)`, so a
missing entry skipped the check and passed, and one asserted only that the
reaper returned a non-empty array. Three JSON parses wrapped in doesNotThrow
now parse directly, so a malformed payload reports its own syntax error
instead of a generic message.
Refs #3057
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(#3004): update Codex adapter collaboration-tool vocabulary
The generated Codex skill adapter documented stale tool vocabulary:
- wait(ids) → collaboration.wait_agent(timeout_ms=...) (the real tool), with
explicit disambiguation from the unrelated exec-cell functions.wait
- close_agent(id) unconditional → gated on tool visibility (same schema-
detection pattern already used for spawn_agent's agent_type field)
- Missing required task_name field and fork_turns parameter → added
alongside the existing fork_context guidance (coexist, not replace)
Updated the regression test to assert the new vocabulary (wait_agent not
wait(ids), functions.wait disambiguation, task_name, fork_turns, tool_search
gate on close_agent).
* chore(#3004): backfill changeset PR number 3104
---------
Co-authored-by: sim <sim@local>
Thirty-eight tests, covering every previously unreached branch in this module:
malformed and non-object config at each nesting level, the origin/HEAD path
returning a bare remote prefix, the remote-show parse missing its HEAD line and
its "(unknown)" sentinel, three catch arms, the spawn-failure path that reports
unverified, the worktree probe answering something other than "true", a
non-zero toplevel query, a blank toplevel, and the default diagnostic sink on
both arms.
The one worth naming is the local-branch fallback that was deleted earlier
today as unreachable and restored. It is hard to pin because it and the
empty-stdout guard above it both return null, so the return value cannot tell
them apart. The test drives it through a result object whose stdout is a getter
that counts reads: one read means the guard fired, two means the guard passed
and the fallback ran. Deleting the line again fails the value assertion —
the function would return undefined — and changing the guard to swallow
whitespace fails the read count, which is the other way that branch dies.
Tier attribution is now pinned. Four tests rig the lower tiers to answer with
DIFFERENT branch names and assert the recorded call sequence, so a result from
symref, remote-show, local-branch, or config can no longer be mistaken for
another. Previously any of them could have produced the answer and the test
would not have noticed. Each tier's argv and timeout are asserted exactly.
The boundary trio applies to the branch listing rather than a numeric limit:
zero matching lines, one, and two.
Three assertions already in this file were removed rather than left beside the
new ones. A doesNotThrow around the worktree probe became an exact-value test
of the catch arm; a typeof-string-and-non-empty shape check on the resolved
root became an equality check against the realpath; and the unverified-fallback
diagnostic was asserted only to be non-empty, so it now asserts the exact
sentence, which fails if the reason changes rather than only if it vanishes.
Two folded bodies used try/finally for teardown and now use t.after.
Refs #3057
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Twenty-two branches in the orphan-reaping path are unreachable from the test
suite, and the reason is not that they are hard to reach — it is that nothing
can reach them. The reaper already accepts an injectable dependency bag, but
every test drives real git and injects only the clock and the liveness probe,
so each fail-closed return inside it has never executed under test.
The two entry points above it took no dependencies at all, so a test could not
drive them even if it wanted to. Both now accept the same bag and thread it
down, with stdout and stderr writers defaulting to the process streams. The
worktree-info probe in the base-branch resolver called its git seam directly
while a sibling function in the same file already modelled the injectable
form; it now follows that sibling rather than inventing a second convention.
Every parameter defaults to today's real implementation, so no existing caller
changes behavior. This is a testability seam, not a redesign.
Two guards are removed as genuinely dead, each excluded by a check a few lines
above it. A NaN test on a value captured by a digits-only pattern cannot fire,
because parseInt of digits is never NaN. An emptiness test on a capture group
that matched one-or-more non-space characters cannot fire either. Each site
keeps a one-line note naming the guard that excludes it, so neither gets
restored by a future reader.
A third guard was proposed for deletion on the same grounds and is NOT removed,
because the claim was wrong. The local-branch fallback returns null when git
prints output that is non-empty but names neither branch — the emptiness check
above it only catches the empty string, so a single newline reaches the
fallback with both flags false. Deleting it would have changed which branch the
resolver reports. It stays, and it gets a test.
Refs #3057
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The scoped Windows lane reached exactly 15m05s and was cancelled on four
consecutive shas of PR #3094. A job that exceeds timeout-minutes reports as
CANCELLED rather than FAILURE, which is why it first read as infrastructure
noise; the giveaway is that the duration equals the cap. The Required tests
rollup fans that job in, so it red-blocked merge while every other lane —
including all three sharded full-Windows shards — was green.
The trigger was a change to the shared test helper, which scopes the
install-heavy suites into the selected list. The lane normally runs about eight
minutes; with that list it does not fit. It was the only unsharded lane left in
this job, so it was the only one without headroom to absorb a large scoped
list.
Issue #869 hit this exact cliff on the sibling lane and named the durable answer
in its own follow-up: a timeout bump moves the cliff, sharding removes it.
#2952 then sharded the full lane. This finishes that work.
The runner already supports it — the shard partition is applied after scope
selection, so it composes with a selected file list rather than only with a
suite, and the partition is cost-weighted from the measured timings table. The
job name template already renders a shard suffix when one is present, so the
three entries name themselves. No individual matrix job is a required status
check; the rollup is, and it is name-independent, so renaming these jobs does
not touch branch protection.
timeout-minutes stays at 15. Each shard now does roughly a third of the work,
so the cap goes from binding to backstop without being raised.
The lane-shape tests were generalized rather than relaxed: the complete-shard-set
invariant now runs per sharded scope instead of only over the full lane, and
"only the full lane is sharded" became "targeted is the only unsharded lane". A
new assertion pins the shard through to the runner — without it the three shards
would each run the entire selected list, triple the cost and no speedup, and
every check would stay green.
No LANE_COSTS entry is added for the new shards. The only recorded cost for that
lane is the pre-sharding run that hit the cap, and inventing a post-sharding
number would be exactly the kind of unmeasured claim the rest of that table
avoids. The estimate and the reason are written down instead, to be replaced by
a real measurement.
Refs #3057
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(#3001): indent continuation lines in serializeChangelog
serializeChangelog interpolated bullet bodies verbatim into a single
- ${body} (#${pr}) line. Any embedded newline became a column-0 line;
parseChangelog's continuation-fold (/^\s+/) didn't pick it up, so
flushBullet terminated the bullet early — dropping the continuation text
and the (#NNNN) PR trailer (recorded as pr: null). The round-trip property
serialize(IR) → parse(text) === IR was false for any body containing \n.
Fix: indent continuation lines (body.replace(/\n/g, '\n ')) so the
parser folds them correctly. Round-trip test asserts both paragraphs'
content AND the PR number survive.
* chore(#3001): backfill changeset PR number 3101
---------
Co-authored-by: sim <sim@local>
* fix(#2998): point bug template at runtime-home version, not npm list -g
The bug-report template instructed reporters to run npm list -g for their
GSD version, but /gsd-update installs into the runtime home
(~/.claude/gsd-core/) without touching the global npm package — so a
correctly-updated machine could report a stale or nonexistent version.
gsd-tools also rejects --version, so 'npx ... --version' was wrong too.
Changed both the version-field description and the diagnostics section to
point at cat ~/.claude/gsd-core/gsd-file-manifest.json (the version field
the installer writes).
* chore(#2998): backfill changeset PR number 3100
---------
Co-authored-by: sim <sim@local>
The Windows node-24 scoped-test job hit its 15-minute ceiling twice on this
branch and GitHub reported both as cancelled, which is how a job timeout
surfaces. The same job finishes in about two minutes twenty on next, across
four consecutive runs. cleanup() is the only hot-path change here.
It was doing up to seven filesystem calls per invocation: three probes of the
temp root, two more for each conventional temp dir, then an existence check and
a realpath of the target. On Windows fs.realpathSync.native opens a file handle
and Defender charges for each one, and this runs in the teardown of effectively
every test.
The root candidates are now memoized on the live os.tmpdir() value. The key
matters: two files in the suite override TMPDIR mid-run and restore it, so a
plain module-level hoist would go stale for them, while re-reading os.tmpdir()
costs an env lookup. Measured over 20,000 calls: 546ms unmemoized, 10ms
memoized.
The symlink-escape check is removed rather than optimized, because it was
guarding something that cannot happen. Verified directly on node v26.5.1:
fs.rmSync(link, {recursive: true, force: true})
link exists false
victim exists true
file exists true
rmSync unlinks a top-level symlink and leaves its target alone, and a symlink
nested inside a tree being recursively removed is also unlinked rather than
followed. The check cost two filesystem calls per teardown on the slowest
platform in the matrix and bought nothing. Its test asserted the victim
survived, which was true with or without the guard.
What closed the original defect is untouched: a target outside the known temp
roots is still refused, before the chdir and before the rmSync, with the roots
named in the message.
Refs #3057
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
