* test(#3148): bound the long tail and delete the allowlist
Migrates the final 170 unbounded sync spawn sites across 49 files, then
removes the allowlist entirely. local/no-unbounded-spawn now runs with no
exemption surface across tests/**: there is no file to add a name to.
drift-detection's throw-native git() helper routes to gitOrThrow -- bare
runGit would have taken 16 call sites quiet on failure. commands.test.cjs
has two independently-scoped runGsdTools/runCli helpers, one already bounded
and one not; they are kept distinct rather than unified, the same trap as the
two same-named git() helpers in Wave 1.
runNpm's bound was erasable. Its options spread callerOptions after the
defaults, so an explicit timeout:undefined silently dropped the 180000ms
bound -- the rule flagged it and was right; it was not a false positive. Fixed
by destructuring with a default, with a test that fails when the default is
removed.
Two sites stay on a raw spawn with an explicit timeout because the seam
cannot express them: one needs shell:true for npm.cmd on Windows, one
redirects stdout to a real fd. Both are the rule's own documented second
option, not an escape from it.
Closure verified rather than asserted: the derivation scan reports 0 unbounded
spawn helpers and 0 unbounded direct git call sites, and a temporary file
carrying an unbounded spawn still errors with the allowlist gone.
Closes#3064.
* test(#3148): close a hole in the guard's own eslint-disable ban
The ban listed only the top level of tests/, so it was blind to 37 .cjs
files under tests/helpers, qa, observability, fixtures and dispatch. With the
allowlist deleted this test is the sole remaining way to detect someone
silencing the rule inline, so the gap was load-bearing: a nested file could
carry an unbounded spawn plus an eslint-disable and pass everything.
Proven before and after. A probe planted under tests/helpers with both was
invisible to the guard and clean under eslint; after making the listing
recursive the guard fails on it. The scanned set goes from 771 files to 808.
Pre-existing since the guard shipped, but this wave is what promoted it to
sole defense, so it is fixed here rather than filed.
Also converts the last hand-rolled throw check to throwIfFailed and the last
re-derived legacy shape to compose toLegacyResult, which makes the epic's
none-remain claim true rather than nearly true. toLegacyResult itself is not
widened -- eight callers depend on its shape and one consumer does not
justify changing a shared contract.
* fix(#3148): correct seam incoherence at the bound and a slow review-lane error path
Two real failures from the remote runner, both fixed at the cause.
The seam could return outcome TIMED_OUT together with exitCode 0. At the
exact bound spawnSync reports ETIMEDOUT while the child has already exited
with a real status, and toSeamResult classified on the error code while
passing status straight through -- an incoherent pair its own boundary test
was written to catch, and did. A status that is not null is direct evidence
the child exited on its own, so it now decides the outcome before the
error-code branches run. process-seam.cjs was deliberately untouched by every
earlier wave; this is a defect in the module itself, kept surgical, with a
unit test that fails against the old logic.
review-lane with an unknown subcommand fell through to its usage error only
after loading the capability registry and building a per-lane plan, which
spawns one child process per lane -- up to twelve. The error path took
~1288ms instead of ~119ms, and under bench load it outran a caller's spawn
timeout and was killed before writing anything, which is the empty stdout and
stderr CI saw. It now fails fast before any of that work begins.
This is the epic's first production change. It is user-facing, so it carries
a changeset rather than a no-changelog label.
* test(#3148): replace a real-race timeout test with a deterministic one
E9 raced git rev-parse against a 1ms bound and assumed git always lost. On a
warm container git finishes first, spawnSync returns status 0 with no error
at all, the seam correctly classifies EXITED, and gitOrThrow correctly does
not throw -- so the test failed on both lanes. A probe confirms a genuine
timeout always carries status null, so this was never the seam misbehaving.
Raising the bound would only lengthen the odds, which is the same defect with
better luck. The test now drives gitOrThrow against a stubbed runGit that
returns a synthetic TIMED_OUT result, so it asserts exactly what it always
meant to -- that a timeout propagates as a throw -- with no timing
dependence. Five consecutive runs are identical where the old one varied.
I wrote this test in Wave 0; it is a real-race test by construction and
CLAUDE.md says to replace those rather than re-run them.
* chore(#3148): backfill changeset PR number 3192
---------
Co-authored-by: sim <sim@local>
* docs(#3182): ADR-3180 — planning semantic model single owner
Phase 0 design lock for epic #3180. Names one canonical owner per
semantic derivation, specifies the frozen-enum scope contract that
distinguishes a genuinely-empty computation from a truncated or
unscoped one, and locks the drift-guard contract.
Ships no production code. Phases 1-5 execute against this ADR.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012qYy4ZWif3sscQyMsup6Ma
* test(#3182): prune real issue 3182 from phantom-ref guard, fix empty-list regex
The guard's own header documents that its list rots: entries are phantom
only until the repo's shared issue/PR counter reaches them, and once the
counter passes an entry it must be deleted. Creating the Phase-0 sub-issue
advanced the counter past 3182, so the guard began rejecting a legitimate
citation of a real issue - the failure its header already records happening
twice, with PRs 2551 and 2361.
3182 was the last entry, and removing it exposed a latent bug: the regex
builder interpolated the list unconditionally, so an empty list yields
(?:#(?:)\b)|(?:issues/(?:)\b), whose empty alternation matches every issue
reference in the repo. Following the file's own maintenance instruction
would have turned a green guard into one failing on nearly every file.
buildRefRe() now returns null for an empty list and is exported, with
boundary coverage at 0/1/2 entries plus word-boundary and bare-digit
negative cases.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012qYy4ZWif3sscQyMsup6Ma
---------
Co-authored-by: sim <sim@local>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(#3024): sync-skills workflow uses gsd-tools query skills-root instead of unshipped install.js
The sync-skills workflow Step 2 shelled out to gsd-core/bin/install.js --skills-root,
but install.js is not shipped in installed trees (only in the npm tarball root bin/).
Every /gsd-update --sync invocation failed with MODULE_NOT_FOUND.
Fix: added 'gsd-tools query skills-root <runtime>' subcommand (gsd-tools IS shipped)
that calls the same getGlobalSkillsBase function install.js used. Updated the
workflow to call gsd_run query skills-root instead of the dead install.js path.
Also documented the #3025 verbatim-cp limitation in Step 5 with a workaround.
* test(#3024): failing-first guards for the three defects in the adopted fix
The cherry-picked commit came from an aborted run that never executed its own
tests. Its raw-path assertion fails as written, which is the clearest evidence
the work never reached verification.
Covers:
- --raw must emit a bare path, not JSON (output() takes a third rawValue arg
that routeSkillsRoot omits, so the raw branch never fires)
- an unknown, empty, whitespace, traversing, or metacharacter-bearing runtime
must be rejected, not silently resolved to claude's skills root
- sync-skills.md must contain zero references to the unshipped install.js,
including the guard's remediation text — the issue's second reported defect
- parity across every runtime in the registry, not three hardcoded ones, so the
two entry points cannot drift
Also converts the adopted tests off a hand-rolled spawnSync onto the bounded
process seam, per CONTRIBUTING.
Fails before the fix. Verified via the remote runner.
* fix(#3024): make the skills-root query actually work and reach non-Claude runtimes
The cherry-picked commit never ran its own tests. Six defects, all fixed here.
--raw was ignored: output() is output(result, raw, rawValue) and the third
argument was omitted, so the raw branch never fired and the workflow captured a
JSON blob as SRC_SKILLS_ROOT. Every downstream cp -r then resolved against a
nonexistent path — the command would have shipped still broken.
An unknown runtime silently resolved to claude's skills root, because
getGlobalSkillsBase falls back rather than returning null, leaving the existing
=== null guard dead. The runtime id is now validated at the CLI boundary against
the shipped registry, so a typo'd --from/--to fails instead of reading from or
writing into the wrong runtime's tree.
getGlobalSkillsBase('vscode') threw a raw TypeError. vscode is non-installable
by descriptor, so it has no skills root — null is the answer, not a crash. The
resolver now short-circuits configHome.kind 'none', which also fixes the same
latent crash in install.js --skills-root vscode. Every caller already gates on
=== null.
sync-skills.md used gsd_run WITHOUT the canonical launcher preamble, so gsd_run
was undefined on non-Claude runtimes — the fix would have been dead in exactly
the place the original bug bit. Preamble propagated via sync-runtime-launcher.
Also registers skills-root in TOP_LEVEL_USAGE (the help/dispatch parity guard
caught it), removes the last two install.js references including the guard's
remediation text (the issue's second reported defect), and updates the stale
assertion that still described the removed contract.
Verified on the remote runner.
* fix(#3024): align the documented runtime list with the registry and gate both entry points
Isolated review returned BLOCK on two findings.
The workflow's Supported-runtimes list and its --to all expansion named grok and
gemini, neither of which is a registered runtime. Once this branch added
validation, --to all — a documented first-class feature — aborted. The list was
hand-copied prose shadowing the registry, so correcting it alone would drift
again; a parity assertion now fails in BOTH directions if the doc and the
registry disagree. vscode is excluded by name: it is installSurface 'none', so
syncing skills to it is meaningless and would abort.
bin/install.js --skills-root reached getGlobalSkillsBase with no own-property
gate, so --skills-root __proto__ silently resolved to claude's skills root. This
branch had just hardened the OTHER entry point to the same function; leaving one
of two parallel surfaces open is the same divergence class as the first finding.
Both now call one shared isRegisteredRuntimeId() rather than a copied check, and
the parity test covers the hostile ids so the two can never disagree again.
Also guards the workflow's root resolution: neither command substitution checked
its exit status and only the source had an existence guard, so a failed
destination resolution left DEST_ROOT empty and turned rm -rf "$DEST_ROOT/$SKILL"
into an absolute path at filesystem root. Both resolutions are now checked, and
Step 5 requires both roots to be non-empty and absolute before any destructive
command.
Verified on the remote runner.
* test(#3024): anchor the runtime-list parity extractor to the list span
The extractor captured (.+) to end of line, so it swallowed the em-dash prose
that explains the vscode exclusion — and that sentence contains backticked
`runtimes` and `null`, which is where the three phantom ids came from. The
documented list was correct; the test was reading its own explanation back as
data. Anchored to the id-list span.
Both directions still fail as intended: proven by injecting a bogus id and by
removing a registered one.
* test(#3024): anchor the --to all extractor and fail loudly on empty captures
The workflow has three TO_RUNTIMES= assignments and the regex matched the first
one — an empty array initializer at line 28 — so the extractor captured nothing
and the assertion diffed [] against 18 ids as if that were data.
That is the same failure twice, so the fix is the general one: every extractor
in this test now asserts it captured a plausible list before comparing, naming
which extractor found nothing and what it was looking for. An extractor that
silently yields [] is a confident wrong answer, and a parity guard that reports
it as a data mismatch teaches the reader to loosen the assertion.
Verified against the real workflow and against doctored copies with each target
construct removed, plus both teeth directions.
* fix(#3024): merge duplicate process-seam import after rebase
The rebase applied cleanly but left runNode declared twice: next had gained its
own import of the seam while this branch added one carrying OUTCOME. A clean
rebase is not a correct one — the file no longer parsed. Merged into a single
import providing both.
* fix(#3024): bind DEST_ROOT per destination instead of a dangling map
Step 2 stored each destination's root into DEST_SKILLS_ROOTS, which nothing ever
read, while Steps 3 and 5 used a scalar DEST_ROOT that nothing ever assigned. The
array was also never declare -A'd, so on bash 3.2 — macOS system bash, which this
repo supports — every destination collapsed onto index 0.
The absolute-path guard added earlier was the only thing standing between that and
rm -rf "/$SKILL"; it turned a silent disaster into a hard stop, but the feature
still could not complete. Each destination now binds its own DEST_ROOT where it is
used, and the unread map is gone rather than replaced.
Step 2 keeps eager validation, so a bad runtime id in a multi-destination --to
aborts before any destination is written rather than after some already have been.
Verified on bash 3.2 with a two-destination run binding distinct roots, and with a
bad id aborting before any destructive call.
* fix(#3024): restore grok support broken by the registry gate
The registry gate added earlier rejected grok, and that was my error. I confirmed
grok was absent from the capability registry and concluded the hardcoded branch
was dead — without checking what it resolved to. It resolves to ~/.agents/skills,
a real grok-specific path, exactly as the pre-fix workflow documented ('grok uses
the ~/.agents layout'), and there is a support discussion doc for it. So a
working, documented runtime silently lost --skills-root and sync-skills support
as a side effect of prototype-pollution hardening — and the parity test I added
locked that in as correct.
gemini is the one that really was dead: it fell through to CLAUDE's skills root,
so rejecting it is right and it stays rejected, as do bogus ids, __proto__,
empty, whitespace and traversal.
The validator's real question is 'does this id have a genuine runtime-specific
resolution', not 'is it in the registry map'. Registry membership was a proxy
that happened to miss grok. Legacy non-registry runtimes with dedicated
resolution branches are now a named, documented set; enumerating every hardcoded
branch in getGlobalConfigDir against the registry confirms grok is the only one.
The new tests assert grok resolves UNDER .agents and specifically not to claude's
root. Allow-listing an id proves nothing about whether it resolves correctly —
that assertion is what would have caught my mistake.
Also uses the shared PROBE_TIMEOUT_MS instead of a duplicate literal, and guards
Step 3's DEST_ROOT re-resolution, which contradicted the file's own stated
guarantee.
Verified on the remote runner.
* test(#3024): guard against LEGACY_NON_REGISTRY_RUNTIME_IDS drifting
The named legacy set is a second hand-maintained proxy for the same predicate
the registry check got wrong — 'does this id resolve runtime-specifically'.
Nothing stopped a third hardcoded branch being added to getGlobalConfigDir
without updating the Set, reproducing the exact class of bug that broke grok.
Production stays explicit and greppable; the test derives the truth instead. It
resolves a sentinel id to learn the generic fallback, classifies every candidate
against it, and fails in both directions — an id resolving runtime-specifically
that is in neither the registry nor the Set, or a Set entry that no longer earns
its exemption. The failure message names the remedy.
Confirms grok resolves runtime-specifically and gemini does not, which is the
distinction the original registry check could not see.
Also reverts the shared-timeout swap: SKILLS_ROOT_PROBE_TIMEOUT_MS is
pre-existing on next and arrived by rebase, so changing it here was scope creep
into another issue's territory.
Verified on the remote runner.
* chore(#3024): backfill changeset PR number
---------
Co-authored-by: sim <sim@local>
* test(#3147): bound the lint/changeset/docs cluster onto the process seam
Migrates 69 unbounded sync spawn sites across 24 files. Allowlist 73 to 49.
Two shared helpers move: tests/helpers/graphify.cjs (6 importing suites) and
tests/fixtures/index.cjs, whose three quoted-argument shell strings became
single argv elements rather than whitespace splits.
changeset-lint's throw-native git() helper routes to gitOrThrow; migrating it
to bare runGit would have silently swallowed a failure that is loud today.
ingest-docs goes the other way -- its catch never rethrew, it degraded failure
into data every call site asserts on, so throwIfFailed would have thrown where
the original returned. The design doc said otherwise and was corrected.
tsconfig-noemit runs a real tsc --noEmit and takes a bespoke 180000ms per the
ensure-runtime-build precedent, not the 30000ms build-hooks norm -- that norm
is for a file copy, and sizing against a label rather than the work is the
same error in the opposite direction.
* test(#3147): add toLegacyResult and settle review findings
The seam exposed a throwing adapter (throwIfFailed) but no non-throwing one,
so eight files independently re-derived the same unwrap back to the legacy
{status, stdout, stderr} shape. That is the third time this epic produced N
copies of one mechanism -- seven throw wrappers in Wave 1, fifty-two timeout
constants in Wave 2, eight result adapters here. The pattern is that whenever
the seam does not expose a mechanism, every suite re-derives it.
toLegacyResult now sits beside throwIfFailed, with its own tests.
Two sites are deliberately NOT converted: changeset-cli's runRender and
runRenderIn return {status, report, stderr} from parsed JSON and never a raw
stdout, so they are a different shape family. lint-legacy-dir-name keeps its
local GUARD_TIMEOUT_MS: 30000 matches the build norm numerically but bounds a
lint probe, not hooks bundling, and importing it would encode a coincidence
as a relationship.
---------
Co-authored-by: sim <sim@local>
* test(#3145): bound the installer/runtime cluster onto the process seam
Migrates 156 unbounded sync spawn sites across 47 files. Allowlist 120 to 73.
Timeouts are sized from evidence already in the tree rather than a house
default, because this wave spawns installers rather than git plumbing and an
undersized bound does not catch a hang -- it manufactures CI flake, which is
worse, since a flake gets re-run instead of investigated. install.test.cjs
records a real spawnSync ETIMEDOUT at a 60000ms cap on a loaded bench while
another lane passed the same commit in 12.7s, so full installs are bound at
120000ms against that recorded incident.
Also adds an auditable escape to the guard's timeout ceiling. The 600000ms
cap was set in #3143 from partial evidence, but fragment-single-edit-
propagation carries a documented, load-tested 900000ms bound on a run that
chains a full build plus eight generators -- the guard would have rejected a
correct timeout the moment that file left the allowlist. A value above the
ceiling is now permitted only with an inline allow-spawn-timeout-ceiling
marker carrying a non-empty reason. It raises the ceiling; it never waives
the requirement for a bound, which is asserted directly.
install-shared.cjs keeps its hand-rolled assert rather than routing through
throwIfFailed: its message embeds both streams, and throwIfFailed carries
only a trimmed stderr. The message now also names the outcome, so a bounded
timeout reads as such across its 38 importers instead of as
expected null to equal 0.
* test(#3145): extract class-norm timeouts and correct the build-hooks sizing
A pre-PR review found 52 copies of four class-norm timeout constants across
this wave. These are not per-suite fixture bindings -- they are shared facts
about how long a class of subprocess takes, derived from a recorded bench
incident. That norm already moved once (60000 to 120000 after a real
ETIMEDOUT), and 52 copies would have drifted the next time it moved.
Extracts tests/helpers/timeouts.cjs, where each norm is justified once, and
converts the copies. A site that genuinely differs -- a real tsc compile, or
regen:derived -- keeps its own local constant with its own justification.
Also corrects a misclassification: scripts/build-hooks.js was sized as a
build at 120000 in twelve places and 60000 in another, but it compiles and
bundles nothing. Its own header says no bundling needed; it copies pre-built
files and syntax-checks them with vm. Three different values bounded one
script; now there is one.
* test(#3145): fix red CI — lint self-match and a Windows chunk overrun
Two failures on PR 3176.
lint-allow-test-rule-refs read a RuleTester fixture as a real exemption. The
fixture exists to prove an unrelated marker does NOT suppress the rule, so it
carries that marker's literal text as test data. Split via concatenation, the
same idiom no-unbounded-spawn-allowlist.test.cjs already uses for its own
self-match problem. The explanatory comment needed the same treatment.
The Windows shard 3/3 chunk was killed at its 600000ms budget. Output stopped
seven minutes before the kill, so this was an overrun rather than a slow
chunk: regenDerivedPropagatesSingleFragmentEditWithNoSecondSourceSurface runs
regen:derived bounded at 900000ms, which is larger than the whole chunk
budget, so the chunk killer always fires first and it can never complete
there. Both the test and that bound predate this change; modifying the file
pulled it into the Windows targeted set and exposed it. Skipped on Windows
with the reason recorded; the Linux lanes cover it. The 900000 bound and its
ceiling marker are unchanged -- they are correct.
* test(#3145): refresh the stale test-timings cost table
The Windows shard was killed at its 600000ms per-chunk budget. run-tests.cjs
packs chunks by measured duration from tests/test-timings.json, and an
unknown file falls back to the table's median weight -- advisory by design,
but it silently underweights exactly the files that matter.
Four of the failing chunk's 22 files were absent from the table, including
the two heaviest: fragment-single-edit-propagation.install.test.cjs at 230s
(it runs regen:derived) and agent-fragments-emission.install.test.cjs at 79s.
Both were weighted as average, so the chunk's total weight read 53.68 against
a budget of 60 and the packer produced a single chunk.
Regenerated from a passing full-suite run, per the remedy the script itself
documents. 700 to 770 entries, 70 added, 0 dropped -- verified, since
gen-test-timings.cjs replaces the table wholesale rather than merging.
Proven against the real packer: the same 22 files now weigh 103.91 and split
into two chunks. No logic, budget, or timeout was changed; raising a budget
to make a red gate pass is not a fix.
---------
Co-authored-by: sim <sim@local>
* 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>
