2306 Commits

Author	SHA1	Message	Date
Tom Boucher	0d08c32048	fix(#2590): emit Workflow scripts the Workflow tool accepts; make the backend reachable (#2681) ... * fix(#2590): emit Workflow scripts the Workflow tool accepts; make the backend reachable Every emitted script was rejected. Four invalid constructs, the first fatal on its own, so the Workflow backend could never dispatch a wave: 1. no `export const meta = {…}` first statement -> whole script rejected 2. resumeFromRunId("<id>") -> "resumeFromRunId is not defined". It is a Workflow TOOL INPUT parameter, not a script function. The run id still reaches the caller via summary.resumeRunId, to pass as that input. 3. budget(<n>) -> "budget is not a function". `budget` is a read-only object { total, spent(), remaining() } fed by the caller's token directive; a script cannot set it. Recorded as intent in a comment. 4. parallel(agent(…), agent(…)) -> "parallel() expects an array of functions". Now parallel([() => agent(…), …]) — passing agent() results directly also started every agent eagerly, before parallel() could bound concurrency. The single-plan stage had its own branch with the same parallel() defect; both branches are now one array-emitting path. Waves also emit phase() calls whose titles match meta.phases exactly, so progress groups correctly. Two secondary defects kept the script from ever being REACHED — which is why this shipped undetected: 5. NOTHING resolved the Agent SDK version. The fragment claimed there was "no scriptable way" to introspect it and told callers to omit the flag, so gate 5 returned agent_sdk_version_unknown on every automated run while `capability state` still reported active:true. True for bash, false for Node: the router now reads the installed @anthropic-ai/claude-agent-sdk version, walking node_modules up the tree and reading package.json directly — require.resolve throws ERR_PACKAGE_PATH_NOT_EXPORTED because the SDK's exports map does not expose ./package.json. Precedence: explicit flag > GSD_AGENT_SDK_VERSION > installed. Fail-closed is preserved; an unresolvable version still declines to inline. A too-old SDK now reports the truthful agent_sdk_version_below_floor instead of unknown. 6. The runtime fallback was `--runtime > GSD_RUNTIME > 'unknown'`, diverging from the canonical `GSD_RUNTIME > config.runtime > 'claude'`, so any invocation without --runtime reported runtime_not_claude on an ordinary Claude project. Now delegates to runtime-slash.resolveRuntime. The fragment's `${AGENT_SDK_VERSION:+--agent-sdk-version "$AGENT_SDK_VERSION"}` snippet is removed rather than repaired: it was also shell-dependent — zsh does not word-split unquoted parameter expansions, so it collapsed to a single argv element, argValue() never matched, and the run failed into the same agent_sdk_version_unknown, indistinguishable from genuinely unknown. Auto- resolution removes the need for the construct entirely. Verified with the issue's own repro: no flags now reaches the version gate; an SDK above the floor yields backend:"workflow" with a script that parses as a real ES module. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015TCwhbMuY37DzRMCfzTABJ * fix(#2590): sync generated registry, repair sibling tests, reject duplicate wave ids Findings from the isolated review, all fixed. HIGH — gsd-core/bin/lib/capability-registry.cjs was stale, and `lint:ci` was already RED because of it. The registry embeds the fragment text INLINE, so the shipped/installed copy still taught the exact broken contract this PR fixes: the old `${AGENT_SDK_VERSION:+…}` bash line and the "OMIT the flag when unknown" guidance. Regenerated. (I had read `lint:ci` by grepping its output instead of checking its exit code, so I recorded a red chain as green — checking $? now.) HIGH — three existing tests asserted the OLD broken shape and would have failed CI; none was touched by the first commit: tests/fix-2285-claude-orchestration-wiring.test.cjs — matched resumeFromRunId("…") tests/claude-orchestration.test.cjs — .includes('budget(') tests/claude-orchestration-command-router.test.cjs — .includes('budget(') Each now asserts the corrected contract: the id/pool reaches the caller via summary, and neither construct is ever CALLED. Two sibling assertions had also gone vacuous — `.includes('resumeFromRunId')` still passed, but only because the new explanatory COMMENT contains that substring, not because anything is wired. Rewritten to assert the real property. MEDIUM — duplicate wave ids were never rejected. Plan-id uniqueness was checked within a wave, but nothing checked wave ids across waves. That was harmless before; it is not now, because each wave emits a `phase("Wave <id>")` call plus a matching meta.phases entry and the tool matches titles by exact string — two waves sharing an id would collapse into one progress group and misattribute the second wave's agents to the first. Rejected at validation, with tests either side of the boundary. MEDIUM — the fragment contradicted itself (its "Manifest construction" header still listed $AGENT_SDK_VERSION as orchestrator-built) and, more seriously, never told the orchestrator to pass summary.resumeRunId as the Workflow tool's resumeFromRunId INPUT. Since this PR moves resume from a broken in-script call to a tool-invocation input, an implementer following only the fragment would have silently regressed phase-resume to a no-op. Both fixed. MEDIUM — docs/how-to/enable-claude-orchestration-workflow-backend.md and docs/explanation/claude-orchestration-capability.md documented `resumeFromRunId("<id>")` and `budget(<tokens>)` as current correct output — teaching the bug as the feature. Updated to the real contract, including the required meta block and the thunk-array parallel() form. (The changeset is `Fixed`, so the docs gate exempts this; it is corrected because it is wrong, not because a gate demanded it.) LOW — the router's top-of-file comment still described the divergent `--runtime > GSD_RUNTIME > 'unknown'` chain as current, ninety lines above the fix; and inserting resolveInstalledAgentSdkVersion had orphaned resolveDetectionArgs' JSDoc above the wrong function. Both repaired. lint:ci now exits 0 (verified by exit code, not by reading output). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015TCwhbMuY37DzRMCfzTABJ * chore(#2590): backfill changeset pr number (#2681) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-26 20:49:38 -04:00
Tom Boucher	c3958018dd	docs(#2674): amend ADR-1411 — corrupt is not absent (epic #1879 Phase 0) (#2678) ... * docs(#2674): amend adr-1411 with the corrupt-is-not-absent house pattern ADR-1411 reasons only about a resolution miss. It is silent on input that is present but not usable, which is how five engine read paths (#1879) could fold an unusable input into the value meaning 'genuinely absent' without contradicting an Accepted ADR. Read together, ADR-1411 and ADR-227 converge and do not license throwing as the cluster's answer: ADR-227 requires malformed input to be coerced rather than propagated and carves out only genuinely-fatal fields, while ADR-1411 already permits a fallback provided it is 'a visible value, not a silent substitution'. The defect in these five sites is therefore not that they fall back but that they fall back invisibly. Records the pattern that follows: every current return value is preserved, and the cause is made visible in-band where the result already carries a provenance envelope, or out-of-band via a deduplicated stderr diagnostic where it returns a bare value it cannot extend. Throwing stays confined to ADR-227's genuinely-fatal carve-out, decided per call. Also names the per-applier caller audit and the lint-resolution-provenance registry gap. Refs #1879 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(#2674): prove the warning-state reset misses the unknown-key dedup set The two existing cases in this suite only pass because each picks a key name no other case reuses, so neither can observe whether the reset the beforeEach calls actually runs. Failing-first: asserts the exported _warnedUnknownConfigKeys is empty after _resetRuntimeWarningCacheForTests(). It is not - the helper clears only _warnedConfigKeys despite documenting itself as resetting per-process warning state. Refs #1879 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(#2674): reset the unknown-key dedup set with the runtime warning cache _resetRuntimeWarningCacheForTests documents itself as resetting per-process warning state but cleared only _warnedConfigKeys, leaving _warnedUnknownConfigKeys populated across cases. The suite that exists to test that set - 'loadConfig - unknown-key warning dedup' - calls the helper in beforeEach expecting exactly this, so the reset was a silent no-op for it; both cases passed only because each picked a key name the other never reused. Any later case reusing a key would have had its warning suppressed by leaked state. Found while amending ADR-1411, which names this dedup guard as the pattern five downstream PRs (#1880-#1884) will adopt - shipping the ADR without the fix would have propagated the footgun to each of them. Folded in here per CLAUDE.md's no-defer rule rather than filed. RED verified on 3c4895841 (test only, no fix): linux-node22 reported 'FAIL tests/config-loader.test.cjs - the documented per-process warning-state reset must clear the unknown-key dedup set too'. Refs #1879 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(#2674): document src/ in the changeset-lint trigger list CONTRIBUTING.md presented the Changeset Required trigger list as bin/, gsd-core/, agents/, commands/, hooks/, sdk/src/ - omitting src/, which scripts/changeset/lint.cjs has in USER_FACING_PREFIXES. src/ is the TypeScript source of truth compiled into gsd-core/bin/lib/*.cjs, so it is the most-edited user-facing path in the repo and the omission sends any contributor who touches it into a CI failure the doc says cannot happen. Also documents that the lint reads GITHUB_BASE_REF, which only CI sets, so running it bare locally reports success without evaluating the branch. This PR hit exactly that: a local run said ok_fragment_present and CI failed fail_missing_fragment on the same diff. Found while opening this PR; folded in per the no-defer rule. Refs #1879 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(#2674): add Fixed changeset for the src/ trigger-list and reset fixes Refs #1879 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(#2674): restore the round-2 review corrections to the amendment These edits were made in response to the second isolated review pass but never staged: later commits used targeted `git add <file>` for the test and the source fix, so the two markdown files stayed dirty and shipped nothing. The branch carried the round-1 text, including the ADR-227 misquote the reviewer raised as a blocker. Restores: the unconditional-diagnostic clause (ADR-227's GSD_DEBUG opt-in was never implemented, so citing it as the precedent was wrong), the dedup key, #1882 folded into the out-of-band mechanism instead of a fourth mechanism-less category, the narrowed caller-audit rationale, and the test-methodology clause. Refs #1879 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>	2026-07-26 20:25:39 -04:00
Tom Boucher	c7c2fe3c2b	fix(#2587): resolve cursor hook workspace from workspace_roots, not cwd (#2680) ... * fix(#2587): resolve cursor hook workspace from workspace_roots, not cwd gsd-cursor-session-start.js and gsd-cursor-stop.js both resolved the project as path.join(process.cwd(), '.planning', 'STATE.md'). Under the cursor-agent CLI, hooks are invoked with cwd set to the Cursor config dir (~/.cursor), not the workspace — so the lookup always missed. sessionStart could only ever emit the "no .planning/ workflow found" nudge and stop's verify-work reminder could never fire, even with .planning/STATE.md sitting in the workspace. Slash commands were unaffected, which is why only the hook layer looked blind. Both hooks already buffered stdin into `raw` and never parsed it; the payload's workspace_roots carries the real path. Multi-root was left open in the report ("first root vs any root"). Resolved forward: prefer the first root that actually carries .planning/STATE.md, so a workspace whose GSD project is not the first root still resolves — strictly better than first-root-only and identical to it in the single-root CLI case. Falls back to roots[0], then to cwd, keeping IDE behavior unchanged if the IDE ever invokes hooks from the workspace. The resolver is duplicated verbatim across the two scripts rather than shared via hooks/lib/: these hooks ship standalone, and a new hooks/lib/ file must be registered in the GENERATED installer's GSD_HOOK_LIB_FILES allowlist — the installer-omits-shipped-file class that yields MODULE_NOT_FOUND at runtime. Per CLAUDE.md "Generative Fix Divergence", the duplication carries a parity assertion so the copies cannot drift. Failing-first, demonstrated by direct invocation with cwd != workspace: pre-fix sessionStart -> "no .planning/ workflow found" stop -> {} post-fix sessionStart -> ".planning/STATE.md is present" stop -> reminder tests/fix-2587-cursor-hook-workspace-roots.test.cjs spawns the real scripts as child processes with a cwd lacking .planning/ and workspace_roots pointing at it. Boundary coverage on the roots array (0 / 1 / 2 entries), plus malformed-JSON fail-open, junk-entry filtering, the parity assertion, and a guard that neither script resolves .planning from cwd again. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015TCwhbMuY37DzRMCfzTABJ * fix(#2587): extend workspace_roots fix to subagentStart; keep cwd a candidate Three findings from the isolated review, all fixed. 1. MISSED SITE (high). gsd-cursor-subagent-start.js carried the identical defect at line 43 — its own header documents workspace_roots in the input schema, but it resolved .planning/ from process.cwd() anyway. Under the cursor-agent CLI that meant every Cursor subagent (planner, executor, verifier) started with "no .planning/ workflow found" and no phase context. The report named only sessionStart and stop; the defect class was wider. Verified pre-fix vs post-fix by direct invocation with cwd != workspace. 2. SEMANTIC NARROWING (medium). The first cut searched only workspace_roots and fell back to cwd solely when the array was EMPTY. So when roots were supplied but none carried .planning/ while cwd did, the hook reported absent — where the pre-fix code, which always used cwd, reported present. That contradicted the fallback's own stated intent of preserving IDE behavior. cwd is now a CANDIDATE in the search (`[...roots, process.cwd()]`), so the fix is a strict superset of both the old behavior and the CLI fix, never a narrowing. 3. STALE GOLDEN FIXTURES (high, would have failed CI). The golden-install-parity fixtures store a content hash per installed file; these three hooks appear in 13 of the 19 runtime fixtures. Regenerated via `npm run gen:golden` — the diff is exactly the three hook hashes in exactly those 13 runtimes. Tests extended: subagentStart resolution via workspace_roots; the stop hook's absent branch (previously only session-start's was covered); an explicit regression guard that a project at cwd is still found when roots miss; parity now asserts all THREE copies byte-identical; and the cwd guard sweeps the whole RESOLVING_HOOKS list so a future hook in this family cannot be left on cwd. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015TCwhbMuY37DzRMCfzTABJ * refactor(#2587): extract cursor workspace resolution to a shared hooks/lib module The duplicate-plus-parity-test approach was the wrong call. The reported issue named two hooks; a third (subagentStart) had the identical defect. That is the signature of a systemic problem, and three copies of a resolver guarded by a parity assertion is a divergence risk maintained by hand rather than a fix. hooks/lib/cursor-workspace.js is now the single implementation. All three Cursor hooks require it; none defines a local copy. Divergence is prevented structurally instead of by asserting three copies stay byte-identical. The reason duplication looked necessary was real, and is fixed properly here rather than worked around: Cursor sets hostBehaviors.skipSharedHooksInstall (#2089), so it never reaches the installer's bulk hooks/lib copy — it was the ONE runtime shipping these hooks WITHOUT hooks/lib (verified against all 19 golden fixtures: cursor had the hook scripts, no lib). A naive require would have thrown MODULE_NOT_FOUND at load, BEFORE each hook's own try/catch, wedging every session on precisely the runtime this bug is about. writeCursorHooksJson (src/runtime-hooks-surface.cts) now stages the hooks/lib helpers the staged scripts actually require, discovered by scanning their require('./lib/…') calls rather than a hardcoded name — so a future helper cannot be silently omitted. This is narrower than flipping skipSharedHooksInstall, which would wrongly pull in every shared hook. cursor-workspace.js is also added to GSD_HOOK_LIB_FILES so uninstall and the manifest manage it for the runtimes that do receive hooks/lib. Verified against a REAL install (runMinimalInstall, cursor/global): the helper is staged, and all three INSTALLED hooks resolve the workspace end-to-end from a cwd that is not the project. Also closes the review gap that the stop hook was excluded from the cwd-candidate regression loop — it now sweeps RESOLVING_HOOKS. The byte-parity test is replaced by a structural guard (every hook requires the shared module, none redefines it) plus a new install test asserting the helper is staged and the installed hook actually loads against it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015TCwhbMuY37DzRMCfzTABJ * fix(#2587): fail loud on a missing hook lib source; drop unsubstituted version marker Two findings from the installer-focused review. H1 — the staging step's `if (!fs.existsSync(libSrc)) continue;` silently defeated the very guarantee it was added for. Reproduced: delete hooks/lib/cursor-workspace.js from source, run the cursor install — it exits 0, prints "Done!", and ships the three hook scripts with an EMPTY hooks/lib/. The installed hook then throws `Cannot find module './lib/cursor-workspace.js'` at load, before its own try/catch, wedging every session — and nothing surfaces until a user hits it. The scan protected against a required-but-UNLISTED helper while leaving required-but-MISSING wide open (typo, bad rebase, an accidental delete). It now throws: a missing helper source is a packaging bug and aborts the install. M1 — hooks/lib/cursor-workspace.js carried a `gsd-hook-version: <placeholder>` marker that NOTHING substitutes: copyLibDir stamps .sh files only, and writeCursorHooksJson's staging applies just the colon-to-dash rewrite. Verified the literal was reaching disk on both the bulk (--claude) and Cursor (--cursor) paths. hooks/lib/git-cmd.js — the only pre-existing hooks/lib/*.js — carries no such marker, so this was newly introduced, not inherited. Marker removed, matching that precedent, with a note on why. (The explanatory comment deliberately does not spell the token out, or it would reintroduce the literal.) M2 — the require-scan regex demanded the exact compact form, so `require( "./lib/x.js" )` would silently fail to stage its helper and compound H1. Now tolerant of interior whitespace and either quote style. Regression test added for H1 — the reviewer confirmed the invariant had zero coverage repo-wide: a source tree carrying the hooks but no hooks/lib/ must make writeCursorHooksJson throw rather than produce a broken install. Re-verified end to end: the missing-source case throws, no unsubstituted literal ships, and the installed hook still resolves the workspace from a foreign cwd. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015TCwhbMuY37DzRMCfzTABJ * chore(#2587): backfill changeset pr number (#2680) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-26 19:57:58 -04:00
Tom Boucher	c87f6f358e	enhance(#1854): offer restore for user-added files backed up on update (#2679) ... * test(#1854): failing-first coverage for user-files-backup restore Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat(#1854): offer restore for user-added files backed up on update Adds a restore-custom-files gsd-tools verb and wires it into update.md as a restore_custom_files step: plan, compatibility-check against the newly installed release, then restore only on explicit opt-in. The backup is never deleted, a shipped path is never overwritten, and a single unwritable entry does not abort the rest. Also drops the jq pipe from update-context field extraction (#2589 class, missed by that sweep) and repairs a broken code fence in docs/CLI-TOOLS.md. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(#1854): reject symlinked restore destinations and backup roots Self-review of the restore path found two write-through holes: copyFileSync follows a symlinked destination, so a link planted at the restore target wrote outside the config dir with every ancestor still a real directory; and statSync on the backup root followed a link, letting the walk read arbitrary files and present them as the user's own backup. Both now lstat. Also marks the report's path/detail strings as untrusted data in update.md so the rendered step cannot carry instructions into the runtime model. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(#1854): move the update-context jq guard into the #2589 sweep update.md joins the AUDITED list rather than carrying a duplicate assertion in the backup-restore suite, and the guard gains a negative-proof companion so 'no jq pipe' cannot pass by the fields simply no longer being read. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(#1854): validate manifest files map shape before trusting it Security review flagged that Object.keys on a non-plain-object files field yields numeric-index keys matching nothing, so the managed-path check dies silently while manifest_found still reports true. Shape, not just type (ADR-227): an array or scalar files map is now an unusable manifest. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(#1854): size the restore prompt by eligible_count Spec review found the prompt was driven by entries.length, so a backup holding only blocked entries asked "Restore 1 file(s)?" when accepting would restore zero. The question now reads eligible_count, and an all-blocked backup reports its reasons instead of offering a choice that cannot be honored. The decline path names the resolved backup_dir rather than the bare directory name. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(#1854): use t.skip on hosts without symlink support A bare return in a node:test body registers as a PASS, so the four symlink guards silently reported green on unprivileged Windows instead of skipping. Adds the dangling-link destination case the security review called out, and moves outside-dir teardown to t.after so a failing assert cannot leak it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(#1854): unfence restore hint, regen goldens, widen install timeout Three gate failures from the c99d612a5 run, all root-caused: 1. capability-registry (3): update.md's decline message put an instructional 'gsd-tools ...' line in an UNTAGGED fence, and the guard treats untagged fences as shell blocks. Retagged both display blocks as text and switched the hint to the resolved 'node <config-dir>/.../gsd-tools.cjs' form users can actually paste. 2. golden-install-parity (19): update.md and gsd-tools.cjs ship, so every runtime fixture moved. Regenerated; the diff is exactly those two hashes per fixture, no other drift. 3. install.test.cjs (5): one real failure, four cascades. The Cursor suite's before hook died on 'spawnSync ETIMEDOUT' at the 60s cap while the node22 lane passed the SAME commit in 12.7s. A full install measures 13-30s idle, so 60s was under 2x headroom and shrinks with every file added to the payload. Raised to 120s, matching the heavy case already in this file. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * chore(#1854): backfill changeset pr number to 2679 --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>	2026-07-26 19:50:56 -04:00
Tom Boucher	bd570618d4	feat(#2632): executor actuals and the closed estimate-calibration loop (#2672) ... * feat(#2632): record executor actuals and close the estimate calibration loop * fix(#2632): calibrate against the raw projection so the loop converges * test(#2632): add closed-loop convergence guard and codify the feedback-loop rule * fix(#2632): pair calibration samples per plan; atomic write; amend adr * chore(#2632): backfill changeset pr to 2672 * fix(#2632): retry renameSync on transient windows errnos and clean up the temp	2026-07-26 16:28:56 -04:00
Tom Boucher	920a5f3f06	fix(#2589): use --raw/--pick for config/model/verify lookups, drop jq dep (#2673) ... * fix(#2589): use --raw/--pick for config/model/verify lookups, drop jq dep The reviewer/workflow config lookups resolved scalars and object fields with a `gsd_run query <cmd> … | jq … 2>/dev/null || <default>` shape. On any machine without jq (the default on Windows/Git-Bash) the jq stage fails with exit 127, the failure is swallowed by 2>/dev/null + the trailing || default, and the variable comes back EMPTY — the configured per-lane model/host/budget is silently dropped and the lane falls back to CLI defaults with no diagnostic. gsd-tools ships native flags that do the same job with no external dep: config-get <key> --raw (strips JSON quotes off a scalar) resolve-model <id> --pick model (descends an object) resolve-execution … --pick <f> (same) verification.status … --pick status Replaced every jq-piped config/model/verify lookup across review.md (×23), plan-phase.md, ship.md, debug.md (incl. the redundant boolean coercion — --raw returns true/false as bare tokens natively), autonomous.md (×2), ai-integration-phase.md (×4), and eval-review.md. The legitimate structured-JSON jq sites that parse HTTP curl responses (.choices[0], jq -rs, jq -n --rawfile) are untouched — only the jq-replaceable lookups moved to the native flags. Adds tests/fix-2589-config-get-no-jq.test.cjs: a source-invariant guard asserting no audited workflow pipes config-get/resolve-model/resolve-execution/verification.status to jq (fails-first on the pre-fix text, passes after). * test(#2589): update autonomous-converge jq assertion to --pick; regen golden fixtures Two test consequences of the workflow-doc edits in the prior commit: 1. tests/autonomous-converge.test.cjs pinned the OLD jq-dependent shape (`verification.status … | jq -r '.status//empty'`) as the canonical routing contract. The test's INTENT is correct (route human validation through canonical verification.status) but it over-specified the MECHANISM (the jq pipe). Updated the assertion to match the new native --pick status shape; the contract being guarded (canonical verification.status read before the human_needed branch) is unchanged. 2. The golden-install-parity fixtures (19 runtimes) record a content hash of every installed workflow .md; the 7 edited workflows changed those hashes. Regenerated via `npm run gen:golden` (the test's own failure message instructs this). Only the 7 edited workflow hashes changed in each fixture. * fix(#2589): declare jq a prerequisite for the lanes that still need it; repair test file Three defects in the first cut of the #2589 fix: 1. tests/autonomous-converge.test.cjs was a JavaScript syntax error. The regex literal /...2>\/dev/null .../ left the second slash unescaped, terminating the literal early and parsing `null` as regex flags: SyntaxError: Invalid regular expression flags The whole file failed to load, so every assertion in it — including the #1522 and #1526 guards — silently stopped running. Replaced with the string-compare form already used at line 202 for the sibling shell-snippet assertion. 2. lint:ci failed. tests/fix-2589-config-get-no-jq.test.cjs buckets into the capped `config` production module via its `config-get-...` effective prefix, making it a novel offender against the 2-file cap. The test is about workflow documents, not the config module, so it is renamed to fix-2589-workflow-jq-dependency.test.cjs (free prefix) rather than growing the allowlist with a module that does not actually need a 5th test file. 3. The fix deleted the repo's only jq-prerequisite declaration. review.md:244 ("install jq if missing") was the anchor plan-review-convergence.md cites by line number, and it went away with the jq pipes — while the ollama, lm_studio, llama_cpp, opencode, and agy lanes still hard-require jq to parse HTTP /v1/chat/completions responses, opencode's JSONL event stream, and agy's conversation cache. On a jq-less host those five lanes swallow exit 127 into empty output: the same silent-degradation class #2589 exists to close. detect_clis now probes jq alongside the other prerequisites and emits jq:available / jq:missing, and the five dependent lanes are treated as undetected when it is absent, with an install hint. The six lanes that do not need jq stay selectable. plan-review-convergence.md now cites the section by name instead of a line number that moves. Regression guards added to the renamed test file: review.md must keep the jq probe and must name all five dependent lanes, and no workflow may cite review.md by line number. Workflow-size baseline and the 19 install-parity goldens regenerated for the review.md / plan-review-convergence.md edits. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015TCwhbMuY37DzRMCfzTABJ * fix(#2589): decide the ship verification gate on a single verification.status read Isolated review finding (medium). Pre-fix, ship.md captured verification.status ONCE into $VERIFICATION and picked status / next_action / next_command off that cached JSON with three jq calls. --pick takes a single dot-path field, so the mechanical conversion issued three separate queries up front: three node spawns that each re-read the phase VERIFICATION.md and re-derive the commit-time vs mtime staleness comparison, on every ship — including the common passing path that never uses the two message fields. It also meant the gate's verdict and the message shown to the user were derived from three reads with no guarantee they observed the same state. The gate now reads `status` once and decides. The two message-only fields are read on the blocking path only, after PHASE_VERIFICATION_INCOMPLETE is already determined — so the passing path costs one query instead of three, and a concurrent write between reads can no longer make the gate and its message disagree, because the block/allow decision no longer depends on them. Adding a multi-field --pick to gsd-tools would have collapsed this to one query, but that changes the flag's output contract and belongs in its own change. Regression guard in tests/fix-2589-workflow-jq-dependency.test.cjs: ship.md must read verification.status exactly three times total, the block decision must follow the status read, and next_action / next_command must both appear after the blocking prose so they cannot drift back onto the passing path. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015TCwhbMuY37DzRMCfzTABJ * docs(#2589): document the jq prerequisite for the five reviewer lanes that need it /gsd-review's ollama, lm_studio, llama_cpp, opencode, and agy lanes parse JSON GSD does not produce (OpenAI-compatible /v1/chat/completions responses, OpenCode's JSONL event stream, Antigravity's conversation cache), so they require jq on PATH. Nothing in docs/ said so. Records which five lanes need it, which six do not, that reading configured models/hosts/budgets no longer requires jq at all, and what /gsd-review now does when jq is absent. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015TCwhbMuY37DzRMCfzTABJ * chore(#2589): backfill changeset pr number (#2673) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-26 16:17:02 -04:00
Tom Boucher	89b673e40d	feat(#2631): planner emits estimate and plan-checker surfaces the over-budget flag (#2670) ... * test(#2631): failing-first planner estimate emission and over-budget surfacing * feat(#2631): emit plan estimate and surface the over-budget split recommendation * fix(#2631): extract sizing prose to references to fit planner and plan-phase caps * fix(#2631): move estimate check to plan-checker; fix template regex and caps * fix(#2631): restore ALWAYS split literal and keep gsd_run after the launcher preamble * fix(#2631): invoke estimate-check after the launcher preamble in plan-checker * fix(#2631): stop double-applying calibration; repair COMMANDS table and stale reference * chore(#2631): backfill changeset pr to 2670 * chore(#2631): backfill changeset pr to 2670	2026-07-26 14:29:02 -04:00
Tom Boucher	115433bba6	fix(#2539): anchor commit phase-token detection; drop silent wrong-branch switch (#2669) ... * fix(#2539): anchor commit phase-token extraction to the phases/ segment; drop silent switch-to-existing cmdCommit auto-detected the commit's phase from --files with an unanchored `match(/(\d+(?:\.\d+)*)-/)`, which returns the leftmost digit-run-then-hyphen anywhere in the joined path. A project_code ending in a digit (PROJECT_V2) made `.planning/phases/PROJECT_V2-07-name/…` match the `2-` inside `V2-` before the real `07-` token, resolving phase 2. findPhaseInternal also searches archived milestones, so an existing archived phase 2 produced a real branch name and the silent `git checkout <existing-branch>` fallback switched the whole working tree onto the wrong branch in the same call that then committed. The extraction now anchors to the directory segment immediately under `.planning/phases/` (or `.planning/milestones/<v>-phases/`) and runs it through the existing project-code-aware extractPhaseToken helper — the single owner shared by the other 6 call sites — rather than introducing a fourth independent copy of phase-token-matching logic. The auto-switch keeps create-if-absent only (the #1278 intent: ensure the branch exists before the first commit on it); it no longer force-switches an already-checked-out working branch onto a different existing branch. Adds two regression fixtures: a digit-suffixed project_code + an archived phase whose number collides with the trailing digit (the silent-wrong-branch case), and a pre-existing phase branch that must not be silently switched onto. * test(#2539): assert non-silent warning; hoist execFileSync; normalizePhaseName guard Address orthogonal-review findings on the #2539 fix: - Spec AC2 ('an auto-checkout mid-commit must never happen silently'): the no-switch path now writes a 'Warning: resolved phase branch X already exists; committing on Y instead' line to stderr when checkout -b fails because the branch already exists. The regression test captures stderr via spawnSync and asserts the warning, so neither direction of the branching resolution is silent. - Spec AC3 ('reuse normalizePhaseName/extractPhaseToken/stripProjectCodePrefix'): the token-acceptance guard now runs the candidate token through normalizePhaseName and accepts it only when it normalizes to a numeric phase form, rather than the brittle 'token !== phaseDir && /\d/.test(token)' check that leaned on extractPhaseToken's undocumented dirName fallback. - Standards (Duplicated Code): hoist execFileSync/spawnSync requires to the top of the 'commit command' describe block instead of inlining them per test. * fix(#2539): build phase-token shape from PHASE_NUMBER_TOKEN_SOURCE (#2128 guard) The acceptance guard regex in detectPhaseNumberFromFiles was a hardcoded `/^\d+[A-Z]?(?:\.\d+)*$/i` — a literal re-derivation of the canonical phase-number grammar, which the #2128 phase-id drift guard (tests/phase-id-drift-guard.test.cjs) rejects unless sanctioned with a `// phase-id-owner:` marker. Build it from the single-owner PHASE_NUMBER_TOKEN_SOURCE export instead, so this read-side acceptance check cannot drift from every other phase-token reader. gsd-test reported this as 2 failures (linux-node22 + linux-node24) on the prior commit. * docs(#2539): backfill changeset pr: 2669	2026-07-26 14:14:28 -04:00
Tom Boucher	46ba02acde	feat(#2630): phase-estimation module, smart-zone config key, and cli verbs (#2661) ... * feat(#2630): add phase-estimation module, smart-zone config key, and cli verbs * fix(#2630): document smart_zone_tokens, refresh golden fixtures, fix null-proto property assertions * fix(#2630): align smart_zone_tokens write/read validation and harden estimation tests * chore(#2630): backfill changeset pr to 2661	2026-07-26 01:42:47 -04:00
Tom Boucher	bf127b7d25	fix(#2565): route generate-claude-profile target through runtime policy (#2659) ... * test(#2565): add failing regression for generate-claude-profile runtime target cmdGenerateClaudeProfile hardcodes .claude/CLAUDE.md (project + global), ignoring the runtime policy that #3163 wired into the sibling cmdGenerateClaudeMd handler. These tests pin the parity contract for project scope (codex -> AGENTS.md, env precedence, --output override, claude preserved), global scope (codex -> <CODEX_HOME>/AGENTS.md, claude preserved), and a divergence guard asserting both handlers agree on the project instruction path for the same runtime. Failing-first: all seven tests reproduce the bug on unmodified next. * fix(#2565): route generate-claude-profile target through runtime policy cmdGenerateClaudeProfile hardcoded .claude/CLAUDE.md for both project and global scope, ignoring the runtime-aware resolution that #3163 wired into the sibling cmdGenerateClaudeMd handler. The #3163 fix diverged when it did not propagate here, so /gsd-profile-user kept writing Claude instruction files on Codex installs (and other AGENTS-native runtimes: opencode, kilo, kimi, antigravity, copilot). Fix mirrors the proven #3163 pattern using existing policy primitives: - Project scope resolves through getProjectInstructionFile(runtime) and a non-claude runtime wins over a stale claude_md_path (AGENTS-native projects must never write to CLAUDE.md). - Global scope derives ~/.<config-home>/<instruction-basename> via getGlobalConfigDir + basename(getProjectInstructionFile), so codex lands at ~/.codex/AGENTS.md. Claude global is preserved byte-for-byte (no env-var drift beyond the prior hardcoded path). - GSD_RUNTIME env var takes precedence over config.runtime. A parity test asserts both handlers agree on the project instruction path for the same runtime, guarding against future re-divergence (CLAUDE.md 'Generative Fix Divergence' rule). * docs(#2565): add changeset fragment for generate-claude-profile runtime fix * docs(#2565): remove parenthetical product description from changeset The product-name purity guard (#1777) flags 'ProductName (description)' patterns in changeset fragments because the prose renders verbatim into CHANGELOG.md at release time. The original fragment had 'Codex (and other AGENTS-native runtimes)' in the bold header, which matched the banned pattern. Reworded to drop the parenthetical; also trimmed the per-runtime mapping (belongs in code comments, not changelog prose). * docs(#2565): backfill PR number in changeset fragment * test(#2565): isolate os.homedir() cross-platform in claude global test The 'global scope: claude runtime writes to ~/.claude/CLAUDE.md' test set only HOME to redirect os.homedir() at a tmpDir. On Windows, Node's os.homedir() reads USERPROFILE (not HOME), so the child process still resolved the real user profile and the path assertion failed (#2659 CI). Set USERPROFILE alongside HOME so the isolation holds on both POSIX and Windows. Production code is unchanged — it uses os.homedir() exactly as the prior hardcoded path did.	2026-07-26 01:06:35 -04:00
Tom Boucher	ec681e3c21	fix(#2567): scope Paused At to ## Session + guard Last Activity date regression (#2660) ... * test(#2567): add failing regression for stale state field overwrites buildStateFrontmatter extracts Last Activity and Paused At from the full STATE.md body via stateExtractField, which matches the first 'Field:' line anywhere. Historical archive sections containing stale field-shaped lines silently overwrite the correct frontmatter value on every sync, and because the poisoning line stays in the body it regresses on the next write. Same divergence class as Bug #2444 (which scoped Stopped At to ## Session but did not propagate). Failing-first: all three tests reproduce the bug on unmodified next (verified via the dedicated red run on the test-only commit). * fix(#2567): scope Paused At to ## Session + guard Last Activity date Two complementary fixes for the stale-archive-overwrites-frontmatter bug class, chosen per field semantics: - Paused At is a session field: scope extraction to ## Session (via the existing matchSessionSection helper), exactly mirroring the #2444 fix for Stopped At. A stale 'Paused At:' line in an archive section can no longer win over the current value. Falls back to full body when no ## Session. - Last Activity has no single canonical section (it appears in the preamble, ## Configuration, and ## Current Position across STATE.md layouts), so a section scope cannot reliably exclude archive copies. Instead guard the information-losing direction: when the body-derived date is OLDER than the existing frontmatter date, keep the existing value and description (preferNewerLastActivity). Applied at both the write seam (syncStateFrontmatter) and the read seam (cmdStateJson) so they agree. Non-date values pass through unchanged. A first attempt scoped ALL current-state fields to the body preamble, but that broke STATE.md variants where the fields legitimately live inside ## Configuration / ## Current Position (regressed 4 frontmatter.test.cjs suites). This minimal fix targets only the two fields the issue names. * docs(#2567): add changeset fragment for stale state field overwrite fix * docs(#2567): backfill PR number in changeset fragment	2026-07-26 00:55:00 -04:00
Tom Boucher	b897070de3	fix(#2653): regenerate stale api-coverage.cjs + add artifact-sync guard (#2656) ... * fix(#2653): regenerate stale api-coverage.cjs + add artifact-sync guard The tracked build artifact gsd-core/bin/lib/api-coverage.cjs had drifted four days behind src/api-coverage.cts: PR 2551 landed the #2366 fix in the source without regenerating the compiled output, so the module that actually ships still carried none of it. Regenerate the artifact, and add scripts/lint-compiled-artifact-sync.cjs to lint:generated-sync so a tracked compiled artifact can never again silently diverge from its source. The check derives its file set from git ls-files rather than a hand-maintained list, and is regime-agnostic: if these artifacts are later untracked and gitignored per ADR-457, the tracked set becomes empty and the check passes trivially. Verified fail-first: the guard exits 1 against the previously-committed artifact (37731 bytes vs 38634 expected) and 0 after regeneration. Also fixes a defect this change surfaced in tests/no-phantom-issue-refs: its PHANTOM list still banned 2551 and 2361, but GitHub numbers issues and PRs from one shared counter and this repo has since reached 2654, so both now resolve to merged PRs. The guard was rejecting accurate citations of them — it failed this very commit for naming PR 2551 as the drift's provenance. Verified by replaying the guard's scan: 1 offender under the old list, 0 under the pruned one. Only 3182 is still a 404. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * chore(#2653): backfill changeset pr number to 2656 --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>	2026-07-25 23:45:11 -04:00
Behruz Nassre Esfahani	a40ee8a5e7	fix(#2479): drop the codex hook-trust bypass flag and its capability probe (#2536) ... * fix(#2479): drop the codex hook-trust bypass flag and its capability probe The /gsd-review codex lane emitted a hook-trust bypass flag via a capability-probed variable (#1115). Host-harness safety classifiers deny commands carrying the flag (23/33 sampled invocations), one denial citing the probe itself as intent, while flagless retries succeeded 32/32 — the flag only bypasses persisted hook trust, a first-run condition with no steady-state value. Remove both the flag and the probe per maintainer direction (no config key). #1115's diagnosability half — stderr to .err, folded into the lane on empty output — is untouched; its version-gate becomes vacuous with no flag to gate. The regression test inverts: the literal flag is now banned file-wide in review.md (covers continuation lines, carrier variables, and probes), alongside bans on the carrier variable and any codex help-grep probe shape. Fixes #2479 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * chore(#2479): add changeset for PR #2536 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * chore(#2479): changeset body in house format (bold lead) + post-rebase fixture regen Review round 2 Minor: wrap the changeset lead clause in the required **bold** span (reviewer-supplied text, applied verbatim). Rebased onto current next; golden-install-parity (19 runtimes) + size baseline regenerated — delta confined to review.md's hash/size. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>	2026-07-25 20:15:49 -04:00
Tom Boucher	6ee4349272	fix(#2537): extract offer_next step to references/ (~3.3KB headroom restored) (#2642) ... * fix(#2537): extract offer_next step to references/ (~3.3KB headroom restored) * chore(#2537): backfill changeset pr to 2642	2026-07-25 05:30:20 -04:00
Tom Boucher	e4dd0cbdd5	fix(#2523): normalize --files to repo-relative; reject out-of-repo; gate push on git-add exit (#2638) ... * test(#2523): absolute + mixed + out-of-repo --files paths * fix(#2523): normalize --files to repo-relative; reject out-of-repo; gate push on git-add exit * chore(#2523): backfill changeset pr to 2638	2026-07-25 01:50:36 -04:00
Tom Boucher	6ad30f74b6	feat(#2584): Phase 3 — scheduler consumer + isolation adapters (#2635) ... Final phase of #2584 (ADR-1239 Codex-binding amendment). execute-phase now negotiates dispatch.isolation and dispatches through the matching adapter, so a wave's independent plans run concurrently on six runtimes instead of one — with no runtime=== branch in the scheduler. harness-worktree passes the host's declared isolation flag (claude, cursor); orchestrator-worktree creates the worktree via the Phase-2 verb and spawns the executor into it with the resolved argv/cwd (codex, opencode, kimi, kimi-code); none stays sequential. Undeclared/unknown/unresolvable isolation degrades to none — never an unisolated parallel run. Fixes two shipped Phase-2 descriptors that per-host research found would fail at spawn: kimi lacked its headless flag (would launch the interactive TUI and hang the orchestrator), and kimi-code named a non-existent binary (Kimi Code installs as 'kimi'). Adds the worktree-path root confinement Phase 2 deferred here, and leading-dash guards on the resolver's prompt/cwd matching the existing git-argument guard. Closes #2627 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-07-25 01:50:20 -04:00
Tom Boucher	461c744c31	fix(#2522): fold wrapped success-criteria lines into their criterion (#2637) ... * test(#2522): wrapped + blank-line success-criteria parse * fix(#2522): fold wrapped success-criteria lines into their criterion * chore(#2522): backfill changeset pr to 2637	2026-07-25 00:36:05 -04:00
Tom Boucher	b97b3dad21	fix(#2517): plan-phase omits model= when *_model is inherit/empty (port execute-phase fix) (#2634) ... * test(#2517): guard plan/execute-phase omit model= when *_model inherit/empty * fix(#2517): plan-phase omits model= when *_model is inherit/empty (port execute-phase fix) * chore(#2517): backfill changeset pr to 2634	2026-07-24 23:44:12 -04:00
Tom Boucher	c2b498e452	fix(#2498): pass --no-track when branching off origin/$DEFAULT_BRANCH (#2628) ... * test(#2498): guard branch-creation uses --no-track (all workflows) * fix(#2498): pass --no-track when branching off origin/$DEFAULT_BRANCH * chore(#2498): backfill changeset pr to 2628	2026-07-24 22:49:29 -04:00
Tom Boucher	57b2bd8368	fix(#2491): finish todos/done -> todos/completed rename (14 stale refs) + guard (#2626) ... * test(#2491): add todos/done rename under-sweep guard * fix(#2491): finish todos/done -> todos/completed rename (14 stale refs) * chore(#2491): backfill changeset pr to 2626 * test(#2491): fix lint-legacy-dir-name + allow-test-rule-refs (split legacy token, add issue ref)	2026-07-24 21:36:33 -04:00
Tom Boucher	4a66d62d10	feat(#2584): Phase 2 — worktree create verb + orchestrator-exec resolver (#2625) ... * feat(#2584): Phase 2 — worktree create verb + orchestrator-exec resolver Phase 2 of the negotiated executor-isolation feature (ADR-1239 Codex-binding amendment). Two building blocks for `dispatch.isolation: orchestrator-worktree` hosts, both unconsumed — no scheduler wires them yet (that is Phase 3), so no runtime behavior changes. worktree create verb (planWorktreeCreate / executeWorktreeCreatePlan / cmdWorktreeCreate in worktree-safety.cts, routed via routeWorktree in gsd-tools.cjs): validates the wave base, creates a bounded branch+worktree, records it in the run manifest reusing record-agent 4-field entry shape, returns the executor working directory. Bounded git (10s timeout, degrade-not-throw); all manifest read/parse/validate/dedupe precedes the single git side effect (no unmanifested-orphan on a bad manifest); timeout-only best-effort partial rollback (a clean collision-exit never removes a live peer worktree); fail-closed on bad base, unsafe leading-dash / .. inputs, and malformed/mis-shaped manifest. resolveOrchestratorExec (host-integration.cts): pure descriptor->argv resolver reading the new runtime.orchestratorExec descriptor field (codex/opencode/kimi/kimi-code), fail-closed on missing/invalid shape. Validator (capability-validator.cjs) + a parity guard asserting every orchestrator-worktree host declares a resolvable orchestratorExec. Adding the create route edits the installed gsd-core/bin/gsd-tools.cjs, so the golden-install-parity fixtures for all 19 runtimes are regenerated (npm run gen:golden) — the only changed hash is gsd-tools.cjs. CONTEXT.md glossary updated; capability-registry regenerated. Behavioral tests (worktree-safety + host-integration) incl. a fast-check property test and the parity sweep. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * chore: rebuild tracked state-transition.cjs to match #2400 source The tracked compiled artifact drifted from src/state-transition.cts: #2400 (commit 2bcfaa2e2) added the progress.total_plans frontmatter sync to source but the tracked bin/lib/state-transition.cjs was never rebuilt, so the fix was not shipping to consumers of the compiled artifact. The mandatory build:lib step for Phase 2 surfaced the drift; recompiling makes the already-merged, already-changelogged #2400 fix effective. Artifact-only resync (no source/test change); drift class tracked by #2591. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>	2026-07-24 21:23:06 -04:00
Tom Boucher	7e1c736a3e	fix(#2556): rescue SUMMARY when cat-file reports absent (exit 128, not 1) (#2611) ... * test(#2556): correct cat-file stubs to exit 128 + rewrite fail-closed tests to fail-open * fix(#2556): rescue SUMMARY when cat-file reports absent (exit 128, not 1) * chore(#2556): backfill changeset pr to 2611	2026-07-24 13:35:02 -04:00
Tom Boucher	ec7978c0b4	feat(#2584): add dispatch.isolation sub-field, descriptors, validator + negotiation (#2604)	2026-07-24 12:51:42 -04:00
0xdhx	bd18a593a9	fix(#2494): capture stderr and guard empty output on the claude and gemini reviewer legs (#2592) ... * fix(#2494): capture stderr and guard empty output on the claude and gemini reviewer legs The gemini and claude blocks in gsd-core/workflows/review.md were the only two of the ten prompt-fed reviewer legs with both `2>/dev/null` and no empty-output guard. Any failure that wrote no stdout — CLI missing, unauthenticated, rate-limited, crashed — left a zero-byte review file with the only diagnostic evidence already discarded. write_reviews substitutes each file's raw content into a `## <Reviewer> Review` section with no marker separating "empty/failed" from "ran cleanly, nothing to report", so a failed lane silently degraded the advertised N-reviewer consensus to N-1 while present_results reported success. Both /gsd:review and /gsd:plan-review-convergence share the invoke_reviewers step, so both were affected. Both legs now redirect stderr to a `.err` sidecar and write a diagnostic stub with the captured stderr appended when the review file comes back empty — the same shape the codex and cursor legs already use. Scope limit, noted in the block comment: the guard only runs if the block itself completes. A host Bash-tool timeout that kills the whole block skips it, the same hard bound the OpenCode block already documents. The existing timeout guidance (#2194) covers that case and is unchanged. Regression test extracts the two dispatch blocks verbatim from the workflow and runs them under bash against a failing CLI stub, asserting the review file is non-empty and carries a diagnosable message plus the captured stderr. It fails against pre-fix review.md (4 of 5 cases). Golden install-parity fixtures and the workflow size baseline are regenerated: one review.md hash per fixture, one size entry. Fixes #2494 * fix(#2494): stamp the changeset fragment with the filed PR number The fragment was authored with the documented `pr: 0` placeholder because the PR number does not exist until `gh pr create` returns. Now that this PR is #2592, stamp it — scripts/changeset/parse.cjs requires pr > 0, so the placeholder would fail the Changeset Required check.	2026-07-24 12:50:34 -04:00
BeeHiggs	bf9fe4630d	feat(#2249): bracket phase-id core grammar — parse/render/toDir round-trip pair (epic #612 PR-1) (#2258) ... * feat(#2249): bracket phase-id core grammar — parse/render/toDir + READING-B + guards PR-1 of epic #612 (ADR-612, in-tree at docs/adr/612-bracket-phase-id-convention.md). Adds the bracket-convention grammar INSIDE src/phase-id.cts — the ADR-2121 single canonical owner — as a pure, additive extension. The 17 locked exports and PHASE_NUMBER_TOKEN_SOURCE are untouched, and normalizePhaseName is byte-identical, so the PR-0 collision anchor (tests/adr-612-collision-characterization.test.cjs) stays green. New pure round-trippable model (ADR Decision 4): - PhaseId { project, milestone, phase, subphase?, plan? }. - parsePhaseId(input): accepts display `[GSD.02] 05.03-01`, dir/token `GSD.02-05.03-slug`, or bare `GSD.02-05`; rejects ambiguous non-bracket tokens (`02-04`, `05`) rather than guessing. The rejection lives ONLY in this new parser — normalizePhaseName and every legacy reader keep accepting those tokens unchanged (conservative default; no existing path gains a throw). - renderPhaseId(id) -> `[GSD.02] 05.03-01`; toDir(id, slug) -> `GSD.02-05.03-slug` with a slug guard that sanitizes path-traversal input. - getMilestoneFromPhaseId(phaseId, convention?): READING-B derives the milestone from the `[PROJECT.MM]` prefix, gated on convention === 'bracket' and returning the `vN.0` form (parity with READING-A). The optional parameter keeps the helper pure (no config read) and byte-compatible — every existing single-arg caller resolves to the unchanged READING-A body (ADR Decision 6). - extractPhaseToken(dirName, convention?): bracket dir branch GATED on convention === 'bracket'. A bracket dir `{CODE}.{MM}-{PP}` is string-indistinguishable from the legacy #2043/#1324 letter-prefixed-decimal family (`P0.3-2`, `P0.12-34`) whenever the code ends in a digit, so no string-only discriminator is complete — an ungated auto-detect silently reinterpreted legacy reads on this CRITICAL 6-caller helper. The explicit convention signal keeps every existing convention-less call site byte-identical (pinned by a #2043 numeric-tail characterization in tests/phase-id.test.cjs). - comparator: no new code — comparePhaseNum already orders the dot-decimal `PP[.SS]` tokens extractPhaseToken yields; milestone-qualified ordering is a PR-2 resolution concern (bracketQualifiedKey), not core grammar. - SENTINEL_RANGES / isSentinelPhaseId(phaseId, convention?): {0, 999} non-milestone guard; the bracket-prefix reading is gated the same way (an ungated read called `P0.0-foundation` a sentinel), legacy leading-int form unchanged. - BRACKET_PHASE_TOKEN_SOURCE (dot-or-dash `[.-]` sub-separator; deliberately more permissive than parsePhaseId — a read-tolerance source for PR-2, not the emit grammar) and PHASE_HEADING_PREFIX_SRC exported from the drift-guard-exempt owner so PR-2 builds every bracket read regex from the canonical source and check:phase-id-drift stays green stack-wide. The bracket project code follows the repo's config-validated `[A-Z][A-Z0-9_]*` grammar (not the ADR §1 illustration's `[A-Z]{1,6}`), so every project_code the config permits parses. parsePhaseId has no live callers in PR-1, so this grammar choice is forward-facing for PR-2 with zero PR-1 behavior impact. Tests: tests/adr-612-bracket-grammar.test.cjs (28) — ADR §3 example round-trips, full 5-tuple parse, READING-B (+ legacy-unchanged and sentinel cases), extractPhaseToken bracket ON/OFF, comparator ordering of extracted tokens, sentinel + slug guards, bare-token rejection, exported-source behavioral assertions, and two generative fast-check properties: render∘parse identity over well-formed displays, and the toDir/disk↔display bijection. Plus a #2043 numeric-tail characterization (single- AND multi-digit rows) in tests/phase-id.test.cjs pinning the convention-less reading byte-identical. The compiled gsd-core/bin/lib/phase-id.cjs is gitignored (ADR-457 build-at-publish) and rebuilt by CI, so it is intentionally not committed. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * chore(#2249): changeset fragment for PR #2258 (docs-exempt: internal grammar behind flag) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(#2249): reject non-canonical phase-id input + harden toDir (review B1/M1-M3) PR-1 CHANGES_REQUESTED follow-up (epic #612, ADR-612 Decision 4). B1 (blocker): parsePhaseId accepted non-canonical input (unpadded numbers, over-padded numbers, multi-space separators, stray whitespace), so render(parse(x)) === x did not hold for every well-formed x as ADR-612 Decision 4 requires. Both branches now enforce canonicality by construction: parse permissively, rebuild the canonical string via the same emit path (renderPhaseId for display, a hand-rebuilt token for dir/token), and throw "parsePhaseId: not canonical" on any mismatch. The .trim() at the parser's entry is removed — the match anchors now reject leading/trailing whitespace outright, folding into the existing "not a bracket phase id" rejection. M1 (major): toDir only ever guarded the slug; project/milestone/phase/ subphase were interpolated unsanitized, so a hand-built PhaseId (a structural, not nominal, type) could smuggle a path-traversal segment onto disk. Every field is now validated against the exact shape parsePhaseId itself would produce before use. M2 (major): a slug that sanitized to empty (e.g. '!!!') left a dangling trailing hyphen in the emitted dir name. toDir now throws in that case. M3 (major): an all-digit slug (e.g. '2026') was string-indistinguishable from the dir-branch's plan tail, so it silently broke the disk<->identity bijection on read-back. toDir now rejects all-digit slugs. Nits: toDir now rejects a non-string slug instead of coercing it to the literal token 'undefined'/'null'; sentinel boundary tests added for milestones 1/998/1000 (SENTINEL_RANGES is the two discrete values {0, 999}, not an inclusive range — these were already correct, now locked by test). Test-first: every new assertion (concrete examples + fast-check mutation property for B1; concrete cases for M1-M3 and the nits) was written and confirmed red before the implementation changes, per repo TDD convention. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * chore(#2249): reformat changeset body to house convention (review Mi2) The fragment added in ab26190a was a plain paragraph — no bold headline, no trailing issue reference. Reformat to the repo's `**Bold headline** — symptom/explanation. (#issue)` body shape (see e.g. .changeset/agile-pandas-dance.md, .changeset/fierce-pumas-gather.md). Uses (#2249), the issue every commit on this branch references, not the PR number already carried in frontmatter (`pr: 2258`) — the changelog serializer appends `(#{pr})` unconditionally, so a body also ending in `(#2258)` would double-render as `(#2258) (#2258)`. Verified the rendered bullet directly via parseFragment + serializeChangelog: it now reads `... (#2249) (#2258)`, matching the dominant convention across the other fragments (frontmatter pr = merged PR, body reference = originating issue). Also moved the docs-exempt marker back before the paragraph -> after it (matching the file's original order): the marker sits on its own line and is stripped before the body is used, but placing it first left a leading blank line in front of the bold headline once reformatted. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(#2249): widen property generators — 3+-digit numerics + subphase-pad mutation (re-review Minor 1/2) PR-1 re-review follow-up (epic #612, ADR-612 Decision 4). Test-only: closes two property-generator coverage gaps the reviewer flagged; no source change (src/phase-id.cts and gsd-core/bin/lib/phase-id.cjs are byte-unchanged). Minor 1 (3+-digit numerics never exercised): numArb capped at 99, so no property fed a 3+-digit milestone/phase/subphase/plan through parse/render/ toDir despite CANONICAL_NUMERIC_RE's dedicated `[1-9]\d{2,}` branch. Widen numArb to 1–999 so the round-trip and disk↔display bijection properties both span 3-digit widths (pad2 passes ≥3-digit values through un-truncated with no leading zero, so canonicality still holds). Add a concrete regression pinning the reviewer's hand-traced example: '[GSD.100] 05' round-trips, renders, and toDirs to 'GSD.100-05-feature' without truncation. Minor 2 (no subphase-pad mutation): the B1 mutation-rejection property covered milestone/phase pad + whitespace mutations but never a subphase pad. Add unpad-subphase / overpad-subphase to the mutation set and a generated `includeSub` boolean that decides whether the canonical carries a `.SS` (forced in for the subphase mutations so there is always a `.SS` to mutate); non-subphase mutations keep their original no-subphase coverage. Non-vacuity verified against the compiled lib by temporarily probing each widened/new property and confirming it fails: round-trip counterexample ["A",100,1,…] and bijection counterexample ["A",1,100,…,"a"] prove 3-digit tokens are genuinely generated and reach the body; a no-op unpad-subphase mutation trips the mutated===canonical guard (counterexample ["A",1,1,1,false,"unpad-subphase"]), proving the subphase branch is reached with a subphase present. Probes reverted; numRuns unchanged. Gates: tests/adr-612-bracket-grammar.test.cjs 44 pass / 0 fail; `npm run test:unit` 1079 pass / 0 fail; `npm run lint:ci` exit 0. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(#2249): consume the #2232 continuation seam at the bracket token's slug-adjacent position (review Major) BRACKET_PHASE_TOKEN_SOURCE was a sixth continuation-recognition site that re-derived the grammar as an unbounded `\d+` literal instead of consuming PHASE_CONTINUATION_SEGMENT_SOURCE, re-opening the #2232 bug class on the bracket path: a PR-2 reader interpolating it over dir `PROJ.01-14-2026-photos-…` (a slug whose first word is a year) over-collected the token as `01-14-2026` instead of `01-14`. Interpolating the cap verbatim at every position was rejected on evidence: the bracket run is `MM-PP[.SS][-LL]` and only the LAST position is slug-adjacent. The exactly-2 cap at the others would under-collect ids toDir itself emits — `PROJ.02-105-slug` (3-digit phase) reads as `02`, `[GSD.02] 05.100` (3-digit sub-phase) as `05` — because CANONICAL_NUMERIC_RE admits `[1-9]\d{2,}` and `[GSD.100] 05` is a pinned regression. Those positions are delimiter- disambiguated (a required field separator; a dot a slug can never contain), not heuristically recognized, so they have no year collision to defend against. Upstream draws the same line for the same reason: core-utils/phase cap the paired PLAN component while the leading phase component stays unbounded. So the run is now positional rather than a free `(?:[.-]\d+)*` repetition, and each position takes the width its delimiter affords: leading unbounded, dash-1 and dot canonical, and the slug-adjacent dash-2 interpolating the single-owner seam. The accepted trade-off is #2232's policy verbatim: a PLAN ≥100 is out of the token grammar. Also derives CANONICAL_NUMERIC_RE from the new BRACKET_CANONICAL_NUMERIC_SOURCE instead of re-spelling it as a literal, so the emit-side gate and the read-side token source are one rule — the same single-owner discipline this fix is about. Behaviour-identical (the anchors make the source's `(?!\d)` guard redundant). Refs #2249 * test(#2249): pin the bracket/#2232 reconciliation — parity surface 6 + divergence gate + property (review Major) The comment block alone cannot hold the divergence: src/phase-id.cts is exempt from the #2128 drift guard by construction, so lint-phase-id-drift.cjs would not catch the bracket token source drifting from the seam. Per the Generative Fix Divergence rule, the divergence is pinned behaviorally instead. Surface 6 joins the existing #2232 parity gate rather than starting a rival one: the review named the bracket token source "a sixth continuation-recognition site", and continuation-grammar-parity.test.cjs is already the invariant-named home where the five #2043 sites agree with the owner on a shared width corpus. Surface 6 asserts the same contract at the bracket run's slug-adjacent position (`01-14-<seg>-photos-…`, mirroring surface 1 with the extra milestone level), so the bracket path now fails the same gate the other five do. A second block pins the DELIBERATE half — the wider canonical width at the delimiter-disambiguated positions, plus the accepted bound (a plan >=100 is out of the grammar). Without it, "unifying" bracket onto the exactly-2 cap would look like a cleanup rather than a regression. The generative property ties the READ side to the EMIT side metamorphically: for every id toDir can produce, BRACKET_PHASE_TOKEN_SOURCE must collect exactly that id's numeric run — no more, no less. It needed a new arbitrary: the existing slugArb generates one [a-z0-9] word and so can never produce the number-leading slug the collision requires. Probe-falsified, both directions (probes reverted): - reverting the source to the old unbounded `\d+` fails 8: the parity gate reports `"01-14-2026-photos-performance" collected "01-14-2026"` — the review's scenario verbatim — and the property shrinks to ["A",1,1,undefined,"100-a"]. - interpolating the seam at EVERY position (the rejected verbatim option) leaves the repro and parity green but fails the divergence gate `'02' !== '02-105'` and the property at ["A",1,1,100,"100-a"] (3-digit sub-phase), which is the evidence that a verbatim cap under-collects ids toDir emits. Width 2 stays green under both probes — the corpus agrees with the owner exactly where the old and new rules coincide, so the gate discriminates rather than merely mirroring the regex. Refs #2249 * docs(#2249): add the new phase-id exports to the CONTEXT.md glossary bullet (round-4 Major) * test(#2249): pin deterministic grammar boundary cases (re-review m1) PR-1 re-review follow-up (epic #612, ADR-612 Decision 4). Test-only: closes the m1 proof gap — the grammar's bounds were exercised only incidentally through the fast-check domain (1-999, [a-z0-9] slugs). No source change (src/phase-id.cts and gsd-core/bin/lib/phase-id.cjs byte-unchanged). Adds a deterministic boundary block (7 describe groups, +22 tests) pinning the compiled lib's CURRENT behavior — a proof gap, not a behavior gap: - m1.1 numeric-width 99/100/101 at milestone/phase/subphase/plan: parse (display + dir) -> render/toDir round-trip byte-equality. The plan position is identity-symmetric (parse/render accept 99/100/101) but toDir drops it (filename-surface dimension only). - m1.2 read-token width is POSITIONAL: BRACKET_PHASE_TOKEN_SOURCE absorbs 99/100/101 at milestone/phase/subphase (delimiter-disambiguated) but caps the slug-adjacent plan (dash-2) at exactly 2 digits — plan >=100 is out of the token grammar (#2232 seam). Pinned as asymmetry, NOT symmetry. - m1.3 leading-zero 007 -> not-canonical rejection at every position/form. - m1.4 slug abuse: parse DROPS a null-byte/control/unicode/emoji trailing slug (never stored, never mis-read as a plan) and rejects a line terminator; toDir's allow-list sanitizer collapses each to a safe [a-z0-9-] token or rejects sanitize-to-empty. - m1.5 absolute-path slug sanitizes (next to the ../../etc traversal test); an absolute-path project on a hand-built id is rejected by PROJECT_ID_RE; an abs-path string is not a bracket id; an abs-path dir slug is dropped to a clean tuple. - m1.6 whitespace-only -> not-a-bracket-phase-id. - m1.7 very-long input (10k) resolves promptly (ReDoS smoke, behavioral): garbage/partial-prefix throw; a 10k-char slug parses (dropped)/sanitizes. No accept-not-reject case is a src bug: parse never STORES an abusive slug (dropped from the identity tuple) and toDir independently re-sanitizes on emit, so the only slug reaching disk is allow-listed. Plan >=100 accepted by parse is the documented positional design (toDir drops the plan; the read-token caps it) — divergence pinned, not papered over. Probe-falsify: corrupted one assertion in each of the 7 groups (m1.4 both its parse-side and emit-side), ran -> 8 distinct named failures, reverted -> 66/66 green. Confirms every new group executes and can fail. Gates: tests/adr-612-bracket-grammar.test.cjs 66 pass / 0 fail; grammar + continuation-grammar-parity + collision-characterization + phase-id family 175 pass / 0 fail; `npm run lint:ci` exit 0. `npm run test:unit` is green except one pre-existing, unrelated env failure (npm-integrity-gate: a live npm-audit advisory in the production dep tree — reproduces with this change stashed; no package.json/lock change here). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-07-24 12:49:53 -04:00
Tom Boucher	3b15a1e3cc	fix(#2576): normalize padded vs unpadded resolves_phase in close_phase_todos (#2597) ... * test(#2576): add failing regression for padded resolves_phase compare * fix(#2576): normalize padded vs unpadded resolves_phase in close_phase_todos * chore(#2576): backfill changeset pr to 2597 * test(#2576): drop fast-check property test (cross-platform-fragile on Windows CI)	2026-07-24 12:10:32 -04:00
Tom Boucher	a5180d96a3	fix: add regression-test-presence gate + missing tests for #2429/#2279 (#2563) ... lint-fix-has-regression-test.cjs: new gate that fails if a fix(#NNNN) or feat(#NNNN) commit has zero behavioral test files (*.test.cjs, excluding auto-generated fixtures/baselines) in its diff. Wired into lint:ci so it runs before PR creation. Missing regression tests added: - #2429: codex local scope does not set $HOME/.agents skills home; global scope does (tests/runtime-artifact-layout.test.cjs) - #2279: map-codebase instructions say to overwrite existing dates, not just replace [YYYY-MM-DD] placeholders (tests/commands.test.cjs)	2026-07-23 12:13:12 -04:00
Tom Boucher	7d298d6d4d	fix(#2474): gate worktree dispatch on project-level USE_WORKTREES too (#2561) ... * test(#2474): update dispatch gate test for dual-gate behavior The #2772 test asserted the gate reads USE_WORKTREES_FOR_PLAN only. Update to accept the dual-gate (USE_WORKTREES + USE_WORKTREES_FOR_PLAN). * fix(#2474): gate worktree dispatch on project-level USE_WORKTREES too The per-plan dispatch condition checked only USE_WORKTREES_FOR_PLAN (submodule-derived), ignoring the project-level USE_WORKTREES flag. Add USE_WORKTREES to the gate. Net-negative edit: compress two nearby prose lines to offset the added shell condition (93353 bytes, down from 93368). Closes #2474 * docs(#2474): backfill changeset PR number (2561) * fix: merge coverage gate into single-process check (#2474) The test:coverage:unit script chained two c8 invocations with &&: the first ran tests and wrote coverage data to .nyc_output/, the second read that data for per-file branch checks. On fast CI runners (ubuntu/24), the second process started before the filesystem flushed the first process's writes — a classic TOCTOU race that caused intermittent coverage gate failures. Replace the two-process chain with a single c8 invocation that generates both text and json-summary reports, followed by a Node script (scripts/check-coverage-gate.cjs) that reads the JSON summary once and checks both overall and per-file thresholds. No filesystem race is possible because the JSON report is fully written before the check script reads it.	2026-07-23 09:14:02 -04:00
Tom Boucher	1482dc5ce0	fix(#2366): scope parseCoverageMatrix to recognized coverage tables (#2551) ... * test(#2366): regression tests for parseCoverageMatrix scoping bugs Bug 1: summary table outside matrix not parsed as data Bug 2: multi-section matrix with repeated headers parses correctly Bug 3: markdown emphasis on decision cell is stripped * fix(#2366): scope parseCoverageMatrix to recognized coverage tables Replace latching sawHeader with contextual inMatrix tracking that resets on non-pipe lines, preventing summary tables from being parsed as data (bug 1). Allow multiple headers for multi-section matrices (bug 2). Strip markdown emphasis from decision cells before validation (bug 3). Closes #2366 * fix(#2366): update representative-corpus test to expect correct behavior The test previously documented the known-buggy parseCoverageMatrix behavior. Now that the fix is in place, test against the expected correct output (expectedBlock, expectedCounts, expectedErrorCount) instead of the currentBuggyOutput snapshot. * docs(#2366): backfill changeset PR number (2551)	2026-07-23 07:38:00 -04:00
Tom Boucher	0ad3c5dd14	fix(#2279): refresh date stamps on map-codebase Update runs (#2550) ... * fix(#2279): reword date stamping to overwrite existing dates on Update runs The map-codebase agent and workflow instructions only said to replace [YYYY-MM-DD] placeholders, but Update-path files already contain concrete dates from the prior run. Reword to SET the date stamps unconditionally, overwriting whatever date is already there. Closes #2279 * docs(#2279): backfill changeset PR number (2550)	2026-07-23 07:37:37 -04:00
Tom Boucher	200daa456c	fix(#2269): add --files to three unscoped query commit call sites (#2549) ... * test(#2269): regression test for query commit --files scoping Verify secure-phase.md, validate-phase.md, and next.md all pass --files to their query commit calls. * fix(#2269): add --files to three unscoped query commit call sites secure-phase.md, validate-phase.md, and next.md were the only 3 of 65 query commit call sites that omitted --files, causing blanket staging of .planning/ and committing unrelated files. Add --files with the specific artifact path to each. Closes #2269 * docs(#2269): backfill changeset PR number (2549)	2026-07-23 07:37:15 -04:00
Tom Boucher	77bf21b3a6	fix(#1995): widen worktree branch regex to accept agent-<id> namespace (#2548) ... * test(#1995): regression test for agent-<id> branch namespace Add failing-first tests proving that normalizeCleanupManifestEntry and planWorktreeRecordAgent reject Claude Code's current agent-<id> isolation branches (only worktree-agent-<id> is accepted). Boundary tests cover both namespaces plus rejection cases. * fix(#1995): widen worktree branch regex to accept agent-<id> namespace Claude Code's isolation="worktree" branch naming changed from worktree-agent-<id> to agent-<id>. Widen the regex in all 7 locations from ^worktree-agent-[A-Za-z0-9._/-]+$ to ^(worktree-)?agent-[A-Za-z0-9._/-]+$ so both namespaces are accepted. Introduce a shared WORKTREE_AGENT_BRANCH_RE constant in src/worktree-safety.cts to prevent future drift. Closes #1995 * fix(#1995): update workflow guards, test assertions, and baselines Widen the branch-check regex in execute-phase.md and execute-plan.md. Update all test assertions that checked for ^worktree-agent- to expect the widened ^(worktree-)?agent- pattern. Regenerate golden-install-parity fixtures, agent-size-baseline, and workflow-size-baseline. Closes #1995 * fix(#1995): update extractCwdGuardBash sanity check for widened regex The e2e test's sanity check verified the extracted bash block contained 'worktree-agent-'. After widening to '(worktree-)?agent-', update the check to match the new pattern. * fix(#1995): widen missed workflow-guard branch check + changeset + lint fixes - hooks/gsd-workflow-guard.js: widen startsWith('worktree-agent-') to /^(worktree-)?agent-/ regex — same defect class, was missed in prior commit - tests/worktree.test.cjs: fix indentation regression from prior edit - Add .changeset/1995-worktree-agent-branch-namespace.md (pr:0 placeholder) Found by orthogonal code review (Step 4). * fix(#1995): regenerate golden + size baselines for workflow-guard change * docs(#1995): backfill changeset PR number (2548)	2026-07-23 07:36:53 -04:00
Tom Boucher	4fc89497d0	fix(#2505): set USERPROFILE in kimi-variant test env + add issue refs on allow-test-rule exemptions (#2545) ... The kimi-variant-disambiguation test set only HOME in the spawnSync env, but os.homedir() on Windows resolves USERPROFILE — so the installer never found the probe config files and the 'variant mismatch' warning never fired. Every other installer test in the repo sets both HOME and USERPROFILE (agent-skills, augment-upgrades, antigravity-upgrades, etc.); this one was newly written for Phase 5 and missed the pattern. Also: both new test files carried allow-test-rule exemptions without the 'see #NNN' issue reference required by ADR-456, failing lint-allow-test-rule-refs.	2026-07-22 20:58:29 -04:00
Tom Boucher	aa0f7dee99	fix(#2460): pi before_provider_request fail-opens without explicit model_profile_overrides (#2499) ... * fix(#2460): pi before_provider_request fail-opens without explicit override pi/gsd.cjs's buildBeforeProviderRequestHandler unconditionally rewrote payload.model to the built-in pi/sonnet tier default (claude-sonnet-5) via resolveTierEntry's catalog fallback. For any pi user on a non-Anthropic provider (kimi-coding, zai, openrouter, openai-codex, minimax, ...), this silently broke every request: pi's chosen model was replaced with one the active provider did not know. The fix inspects model_profile_overrides.pi[tier] explicitly BEFORE calling resolveTierEntry (which falls back to the built-in catalog and would mask the 'user did not opt in' signal). When the user has not set an override (or set it to null), the handler returns undefined — fail-open — and pi's chosen model flows through untouched. Only an explicit opt-in via model_profile_overrides.pi[tier] steers. Tests: - the ACTUALLY-REGISTERED handler fail-opens when no override configured (was: 'steers to default-tier model-catalog pi id' — encoded the bug). - new test reproducing the reporter's exact repro (model: 'k3' → undefined). - override path: explicit model_profile_overrides.pi.sonnet config steers to the user-configured model id. - defensive: explicit null override also fail-opens. Per the reporter's suggested fix #1 of #2460. * test(#2460): regen pi golden parity + install tree fixtures pi/gsd.cjs changed → pi install hash changed → regenerate the parity + install-tree fixtures via UPDATE_GOLDEN=1 + UPDATE_INSTALL_TREE=1. * fix(#2460): treat empty-string override as fail-open (M1 review) Per code-review M1 + security M1: an explicit empty-string override (`{ pi: { sonnet: "" } }`) silently bypassed the fail-open guard because the check was `=== undefined || === null` only. resolveTierEntry's falsy `if (userRaw)` then fell back to the built-in catalog and rewrote payload.model to claude-sonnet-5 — re-introducing the exact bug this PR fixes, via a degenerate config shape. Fix: widen the guard to also reject `''`. The test now exercises both null and '' in a loop, asserting fail-open for both. * fix(#2460): clear hono/@hono/node-server moderate advisories via npm override GHSA-v422-hmwv-36x6-class advisories (3 moderate) appeared during this PR's session: - @hono/node-server <2.0.5 (path traversal on Windows via encoded paths) - hono 4.3.3 - 4.12.26 (API Gateway v1 adapter drops distinct repeated request header values) - both transitively via @anthropic-ai/claude-agent-sdk -> @modelcontextprotocol /sdk@1.29.0 The npm audit fix re-resolved hono to 4.12.31 (within the existing ^4.11.4 range declared by MCP SDK), clearing the hono advisory without an override. The @hono/node-server advisory cannot be re-resolved the same way: MCP SDK pins @hono/node-server@^1.19.9, and the fix requires 2.0.5+. There is no MCP SDK release that allows @hono/node-server@2.x (latest 1.29.0 is the most recent), and bumping @anthropic-ai/claude-agent-sdk to 0.3.x does not help (its peerDependency is still @modelcontextprotocol/sdk@^1.29.0). The override (sibling to the existing 'qs' and 'body-parser' entries) is therefore the only available tool — distinct from the body-parser case in re-resolution. Verified: npm audit --omit=dev reports 0/0/0/0 advisories. Also: regenerated the pi golden-install-parity fixture (the pi/gsd.cjs change in this PR altered the pi install hash). * docs(changeset): add Fixed fragment for #2460 PR The two-otters-jog.md changeset was created earlier but lost during the cherry-pick detour to fix #2454 PR 1's npm advisory cascade. Recreating here with PR number 2499 backfilled (no placeholder cycle needed).	2026-07-22 16:05:41 -04:00
Tom Boucher	d579daa3ed	docs(#2505): Phase 6 — migration guide + built-in-only subagent-toolkit enum (#2538) ... * docs(#2512): Phase 6 — migration guide + built-in-only subagent-toolkit enum * fix #2512: update CONTRACT-PIN for built-in-only subagentToolkit value * docs(changeset): backfill PR #2538 for Phase 6 (#2512)	2026-07-22 14:30:26 -04:00
Tom Boucher	28f79e6e22	feat(#2505): Phase 5 — Kimi variant install-time disambiguation (#2535) ... * feat(#2513): Phase 5 — Kimi variant install-time disambiguation (descriptions + mismatch warning) * fix #2513: use helpers.cleanup (Windows-EBUSY retry budget) not raw fs.rmSync * fix #2513: spawnSync captures stderr (execFileSync drops it on exit-0) * docs(changeset): backfill PR #2535 for Phase 5 (#2513)	2026-07-22 13:49:31 -04:00
Tom Boucher	f654c24a3e	feat(#2505): Phase 4 — runtime-aware subagent dispatch (Option A; resolve-dispatch-type query) (#2525) ... * feat(#2508): Phase 4 Option A — runtime-aware subagent dispatch via resolve-dispatch-type query (#2505) * fix(#2508): prose-variant preamble (avoid scanner-tripping literals) + namedDispatch===false-only mapping * fix(#2508): remove leftover old-preamble lines (keep prose variant only) * fix #2508: prose-only reference file * test #2508: regen golden install parity after workflow preamble additions * fix #2508: remove preamble from plan-phase.md (Phase 6 capstone ceiling); regen size+golden baselines * docs(changeset): backfill PR #2525 for Phase 4 (#2508)	2026-07-22 10:22:42 -04:00
Tom Boucher	c2a305c44d	feat(#2505): Phase 2 — kimi-code Agent Skills install layout (#2520) ... * feat(#2454): PR 2 — kimi-code Agent Skills converter + install layout PR 1 registered the kimi-code EoS descriptor with empty artifactLayout (SKIP_INSTALL_CONTRACT excluded it from the end-to-end install test). PR 2 fills in the install surface: - src/runtime-artifact-conversion.cts: new convertClaudeCommandToKimiCodeSkill function. Today it delegates to convertClaudeCommandToKimiSkill (Python kimi-cli) because Kimi Code uses the same Agent Skills format + /skill: invocation per official docs. The distinct function name lets a future divergence land cleanly if Kimi Code's skill format evolves independently. - gsd-core/bin/lib/capability-validator.cjs: add to ALLOWED_SKILLS_CONVERTERS. - capabilities/kimi-code/capability.json: artifactLayout.global now declares the skills kind with converter='convertClaudeCommandToKimiCodeSkill' + home='.kimi-code' (auto-discovered at ~/.kimi-code/skills/ per Kimi Code docs: merge_all_available_skills = true default). - tests/installer-migration-install.integration.test.cjs: REMOVE the SKIP_INSTALL_CONTRACT exclusion — kimi-code now has a full install surface. - Regenerated capability-registry + capability-matrix + golden install parity + install tree fixtures for kimi-code. * fix(#2454): wire kimi-code converter into SKILLS_CONVERTER_REGISTRY + count bump - src/install-engine.cts: add convertClaudeCommandToKimiCodeSkill to SKILLS_CONVERTER_REGISTRY so the layout-driven skills install path can dispatch off the descriptor's converter string. - tests/capability-registry.test.cjs: bump VALID_CONVERTER_NAMES count 26 → 27 (added convertClaudeCommandToKimiCodeSkill). * fix(#2454): remove home override from kimi-code skills (inherit configDir) The home:'.kimi-code' override made the install plan resolve skills dest to ~/.kimi-code/skills instead of <configDir>/skills, causing the test's temp configDir to miss the install. Removing it lets skills inherit configDir like most runtimes. * fix(#2454): kimi-code install contract surface is flat-skills (no agents) Kimi Code has NO custom named subagents (per official docs: 3 built-in coder/explore/plan only). The kimi-skills-agents surface expects agents/ gsd.yaml + subagents/*.yaml which kimi-code does not produce. Changed to flat-skills which only checks for skills/gsd-* dirs. * docs(changeset): Phase 2 kimi-code install layout Added (#2509) * docs(changeset): backfill PR #2520 for Phase 2 (#2509)	2026-07-22 00:58:58 -04:00
Tom Boucher	bf8f320083	feat(#2505): Phase 1 — EoS descriptor split (kimi-code capability.json + drift-guard registration) (#2519) ... * feat(#2454): add kimi-code as an EoS capability (Node Kimi Code CLI) PR 1 of N for #2454. Establishes the EoS descriptor foundation for splitting GSD's kimi support into two distinct products per the user's directive: - kimi (existing): Moonshot's Python kimi-cli (~/.kimi, runtime: python) - kimi-code (new): Moonshot's Node Kimi Code CLI (~/.kimi-code, runtime: node, KIMI_CODE_HOME env) Per ADR-1239 EoS, runtime behavior is driven by capabilities/<id>/capability.json descriptors, not hardcoded branches in install.js. The new descriptor uses the existing primitives (dot-home configHome, skills artifactLayout, kimi-hooks-toml hooksSurface — same TOML [[hooks]] format Kimi Code reads per its docs). Critical Kimi Code constraint reflected in the descriptor: hostIntegration.dispatch.namedDispatch: false hostIntegration.dispatch.builtInSubagents: ['coder', 'explore', 'plan'] hostBehaviors.namedSubagentsSupported: false Kimi Code's official docs confirm only 3 built-in subagents with NO custom- subagent registration (the [subagent] table only has timeout_ms). The kimi-agents YAML layout (used by Python kimi-cli) is therefore NOT in kimi-code's artifactLayout. Schema adjustments: - subagentToolkit set to 'undocumented' (the existing escape hatch); the schema enum (full/read-only) lacks a 'limited'/'built-in-only' value. A follow-up PR can extend the schema enum to add 'built-in-only' as a first-class axis value reflecting Kimi Code's documented model. Registration: - capabilities/kimi-code/capability.json (new descriptor, modeled on codex) - bin/install.js: allRuntimes array + --all list + --kimi-code flag - gsd-core/bin/shared/runtime-aliases.manifest.json: kimi-code aliases (kimi-code, kimicode, kimi_code) - src/runtime-name-policy.cts: FALLBACK_ALIASES map - gsd-core/bin/lib/capability-registry.cjs: regenerated via scripts/gen-capability-registry.cjs --write Tests: - tests/multi-runtime-select.test.cjs updated for the new runtime count (18) + new --kimi-code flag test + 'All' shortcut renumbered 18 → 19. Out of scope for PR 1 (follow-up PRs in the sequence): - Install-time decision logic (kimi vs kimi-code detection / prompt) - agent-install-check semantics for kimi-code (verify Agent Skills presence) - cmdAgentSkills fallback returning subagent prompt content - Workflow template mapping (named agents → built-in coder/explore/plan) - Migration guidance for users currently on 'kimi' who are actually on Kimi Code - Schema enum extension for subagentToolkit: 'built-in-only' Refs #2454, #2095 (EoS/kimi migration epic), ADR-1239 (EoS). * fix(#2454): complete drift-guard registrations for kimi-code runtime The drift guards caught every surface that pins runtime enumeration. Each update is mechanical, driven by the guard's named failure mode: - src/runtime-name-policy.cts RUNTIME_LABELS: 'Kimi Code' label for kimi-code - src/runtime-name-policy.cts RUNTIME_FLAG_IDS: add kimi-code to the isKimiCode predicate generator - bin/install.js runtimeMap: option '11' → 'kimi-code', renumber downstream entries (11..17 → 12..18), ALL_RUNTIMES_OPTION 18 → 19 - gsd-core/bin/shared/model-catalog.json runtimeTierDefaults: kimi-code entry (null/null/null — same as kimi, no model tier defaults until configured) - docs/reference/capability-matrix.md: regenerated via scripts/gen-capability-matrix.cjs --write (kimi-code row added) - tests/global-config-home-fragment.test.cjs GOLDEN_FRAGMENT_MAP: kimi-code → '.kimi-code' - tests/fixtures/golden-install-parity/*.json: regenerated via npm run gen:golden (the runtime-aliases.manifest.json hash changed; all 17 runtime fixtures updated) The capability-registry is already regenerated from the prior commit. * test(#2454): update drift-guard tests for kimi-code runtime registration Multiple drift guards pin runtime enumeration counts and option numbering. Each update is mechanical, driven by the guard's named failure mode: - tests/runtime-flags.test.cjs: EXPECTED_FLAGS gains isKimiCode (16 → 17); 'all 16 flags' → 'all 17 flags' in test names + messages. - tests/multi-runtime-select.test.cjs: parseRuntimeInput option renumbering cascade — kilo moves 11→12, opencode 12→13, pi 13→14, qwen 14→15, trae 15→16, windsurf 16→17, zcode 17→18, All 18→19. New single-choice test for kimi-code (option 11). Prompt test updated for new numbering. - tests/host-integration-descriptors.test.cjs: EXPECTED_PROFILES gains kimi-code → 'programmatic-cli' (terminal CLI per Kimi Code docs); EXPECTED_FLATTEN gains kimi-code → false (backgroundDispatch:true per docs, same as Python kimi/opencode). - tests/global-config-home-fragment.test.cjs: table-count test renamed 13 → 14 table runtimes (kimi-code added to GOLDEN_FRAGMENT_MAP earlier). * fix(#2454): empty artifactLayout for kimi-code (PR 1 scope) The skills kind requires a converter (existing converters are per-runtime like convertClaudeCommandToKimiSkill). PR 1 of this multi-PR sequence only registers the descriptor; the actual Agent Skills converter (and a new 'convertClaudeCommandToKimiCodeSkill' function) lands in PR 2 alongside the install-time decision logic. Empty artifactLayout.global is valid and means 'nothing to install yet via the layout seam'. Also: added kimi-code to RUNTIME_META in tests/helpers/install-shared.cjs (localDir .kimi-code, globalSuffix .kimi-code), and added Kimi Code as option 11 in install.js's buildRuntimePromptText (renumbered downstream options 11..17 → 12..18, All 18 → 19). * fix(#2454): camelCase runtimeFlags for hyphenated ids (kimi-code → isKimiCode) The runtimeFlags generator previously produced 'isKimi-code' (hyphen preserved) for the new kimi-code runtime id. Property names with hyphens are awkward for consumers (flags['isKimi-code'] instead of flags.isKimiCode). The new runtimeIdToFlagName helper folds -[a-z] boundaries to uppercase, producing the conventional PascalCase flag name. The 16 prior single-word runtime ids are unaffected (the regex finds no hyphens). * fix(#2454): update remaining drift-guard tests + gen kimi-code fixtures - tests/runtime-flags.test.cjs drift guard: use proper kebab-case conversion (isKimiCode → kimi-code, not 'kimicode') so the registry comparison doesn't false-positive on hyphenated runtime ids. - tests/multi-runtime-select.test.cjs: fix kilo/opencode/pi/qwen/trae single-choice tests for the renumbered options (kilo 11→12, opencode 12→13, pi 13→14, qwen 14→15, trae 15→16). - tests/install.test.cjs: Kilo integration option 11→12, prompt test regex updated. - tests/fixtures/golden-install-parity/kimi-code.json + install-tree/ kimi-code.json: generated via UPDATE_GOLDEN=1 + UPDATE_INSTALL_TREE=1. The kimi-code install produces the standard GSD install layout (skills, contexts, references, etc.) — 436 paths, same shape as other runtimes that have no custom converter yet. * fix(#2454): add kimi-code install contract + global config home fragment - src/runtime-name-policy.cts GLOBAL_CONFIG_HOME_FRAGMENTS: add kimi-code → '.kimi-code' so getGlobalConfigHomeFragment returns the correct path instead of falling through to the default '.claude'. - tests/installer-migration-install.integration.test.cjs RUNTIME_INSTALL_CONTRACTS: kimi-code entry (same surface as kimi for PR 1; PR 2 will specialize once the Agent Skills converter lands). - tests/multi-runtime-select.test.cjs: fix space-separated-choices test for the renumbered kilo option (11 → 12). - tests/fixtures/golden-install-parity/kimi-code.json + install-tree/ kimi-code.json: regenerated after rebasing onto current next (new planner-reversibility.md from #2471 etc. now included). * test(#2454): skip kimi-code install contract until PR 2 ships install layout The end-to-end install test (tests/installer-migration-install.integration .test.cjs) asserts every allRuntimes entry installs a runtime-specific artifact surface. PR 1 of #2454 registers kimi-code in allRuntimes + the capability descriptor + flags + labels, but the install LAYOUT (Agent Skills converter + global AGENTS.md at $KIMI_CODE_HOME/AGENTS.md) lands in PR 2. The SKIP_INSTALL_CONTRACT set marks this exclusion explicit and self-removing — PR 2 removes the entry alongside adding the install surface, restoring the contract loop to full coverage. * fix(#2454): restore compact model-catalog.json format (M1 review) Per code-review M1: my prior 'fix(#2454): complete drift-guard registrations' commit used python json.dump(indent=2) which inflated the file from 165→607 lines (every nested entry got expanded) and lost the trailing newline. The semantic change was just a 3-line kimi-code entry. Restored the original hybrid format (top-level indent=2 + inner entries' one-line style) and added kimi-code in matching form. Regenerated golden install parity + install tree fixtures since the model-catalog.json hash changed. * fix(#2454): update CONTEXT.md allRuntimes glossary (17 → 18, add kimi-code) CI lint-tests job failed on the glossary drift guard (scripts/check-glossary-refs.cjs --check): ✗ CONTEXT.md's allRuntimes enum-count sentence claims 17 values but bin/install.js's allRuntimes array has 18. ✗ CONTEXT.md's allRuntimes member list has drifted from bin/install.js (missing from CONTEXT.md's list: kimi-code). Missed in the prior commits because gsd-test does not run the glossary check (it's a CI lint-tests-only check). Updating CONTEXT.md's two claims to 18 values + kimi-code in the member list. * chore(#2505): regen capability-registry + stamp kimi-code version 1.8.0 (#2511) * docs(changeset): Phase 1 kimi-code runtime Added (#2511) * test(#2511): regen kimi-code golden parity fixture after Phase 0 guard normalization lands * docs(changeset): backfill PR #2519 for Phase 1 (#2511)	2026-07-22 00:27:35 -04:00
Tom Boucher	7e905aa137	feat(#2505): Phase 0 — Kimi PreToolUse guard vocabulary normalization (precondition; carries PR #2326 forward) (#2518) ... * fix(#2304): normalize Kimi tool vocabulary in PreToolUse guard payload checks The Kimi [[hooks]] registrations translate the matcher to Kimi's tool vocabulary (WriteFile|StrReplaceFile) but the guard scripts early-exit unless the payload's tool_name is a Claude name (Write/Edit/MultiEdit), so every guard was dormant on Kimi: the matcher fired, the script saw WriteFile, and exit(0)'d. Normalize the payload's tool_name at the top of each guard (WriteFile -> Write, StrReplaceFile -> Edit; bare or module-qualified kimi_cli.tools.file:* forms) before the check. Inlined per guard rather than a hooks/lib/ helper because hook scripts are staged as standalone files on every hook surface, and a sibling require is a staging dependency that can fail silently. Regression tests pipe Kimi-vocabulary payloads at each guard and assert it engages (typed fields: exit status, decision, hookSpecificOutput) — verified red against the pre-fix scripts, green after. * fix(#2304): normalize Kimi tool_input fields and route block reasons to stderr Cross-AI review of the initial fix, verified against kimi-cli source, found the tool_name normalization alone leaves the guards dormant on a real Kimi runtime: kimi-cli forwards tool_input verbatim (src/kimi_cli/hooks/events.py), and its tool schemas (src/kimi_cli/tools/file/{write,replace}.py) use path/content and edit.old/edit.new (single Edit or list) — not Claude's file_path/old_string/new_string. The guards read file_path, got '', and exited 0 past the now-open tool_name gate. Extend the per-guard normalization to the payload fields (path -> file_path, edit -> old_string/new_string with list flattening), and write the worktree guard's block reason to stderr as well as the stdout JSON — Kimi feeds stderr, not stdout, back to the model on exit 2 (docs/en/customization/hooks.md exit-code table). Regression tests rewritten to Kimi's actual payload shapes (plus an edit-list case and a stderr-reason assertion) — verified red against the name-only fix, green after. * fix(#2304): join all edit[] entries into old_string, matching new_string Review nit on #2326: old_string took only edits[0].old while new_string joined the whole list. Symmetric join removes the latent trap for any future consumer sizing before/after content (e.g. the #2255 write guard). * fix(#2304): normalize Kimi ReadFile vocabulary in read-injection scanner Review Major 2 on #2326: gsd-read-injection-scanner.js had the identical dormancy — its Kimi matcher fires on 'ReadFile' but the SCANNED_TOOLS check only knew 'Read', so injected content in read files was never flagged on Kimi installs. Folds the same inlined normalization block into the scanner and extends the shared KIMI_TOOL_NAMES map with ReadFile:'Read' in all four copies so they stay byte-identical. Harmless in the three write guards: a normalized 'Read' falls out of their Write/Edit allowlist exactly as the unmapped name did. Field mapping verified against kimi-cli upstream (src/kimi_cli/tools/file/read.py Params.path); the existing path->file_path copy covers the scanner's file_path read. * test(#2304): parity test binding the four inlined Kimi normalization copies Review Major 1 on #2326: KIMI_TOOL_NAMES + normalizeKimiPayload is deliberately inlined in four hook scripts (staging-dependency rationale, unchanged), with the inverse table in bin/install.js — five hand-maintained surfaces and nothing binding them. Static binding, zero runtime coupling: - the four inlined blocks must be byte-identical; - each guard-map entry must be the value-inverse of convertKimiToolName() for its Claude name; - every guard-relevant Claude tool (Write/Edit/MultiEdit/Read) must have a reverse entry — a vocabulary rename or extension that updates the installer without updating the guards now fails in CI instead of leaving a guard silently dormant (the #2304 recurrence door). Negative-controlled: diverging one copy or dropping a map entry fails the suite against the fixed code. * test(#2304): regenerate golden parity fixtures for guard hook changes CI red on #2326: all 10 golden-parity failures were the staged guard hooks drifting from their fixtures. Regenerated with npm run gen:golden (after npm run build) under throwaway HOME/CLAUDE_CONFIG_DIR; diff verified to change exactly the four PR-touched guard entries per surface, nothing else. * test(#2304): regression tests for Kimi ReadFile engaging the scanner Mirrors the per-guard Kimi vocabulary tests the PR added for the three write guards: bare and module-qualified ReadFile produce the advisory, path exclusions still apply post-normalization, unknown Kimi names stay fail-open. Negative-controlled against the pre-fold scanner (the two positive cases fail there; exclusion/fall-through correctly pass on both sides). * fix(#2304): normalize Kimi Shell vocabulary in workflow guard Withdraws the disclosed out-of-scope split: verification showed the Bash->Shell case needs NO different mapping — kimi-cli's Shell.Params names its field `command` (src/kimi_cli/tools/shell/__init__.py), same as Claude's Bash — and the guard's write branch (Write/Edit/MultiEdit allowlist) was ALSO dormant on Kimi under its Shell|WriteFile| StrReplaceFile matcher. Same defect class as the other four hooks. Folds the identical inlined block into gsd-workflow-guard.js and extends the shared map with Shell:'Bash' in all five copies (harmless outside the workflow guard: a normalized Bash falls out of the other guards' checks as before). Parity test now binds five copies and adds Bash to the dormancy alarm. New workflow-guard test file exercises the observable block (force-add on a worktree-agent branch): Shell bare and module-qualified block with WORKTREE_AGENT_FORCE_ADD_FORBIDDEN, benign Shell passes, Claude Bash unchanged — negative-controlled against the pre-fold guard (the two Kimi cases fail there). Golden parity fixtures regenerated; diff verified to change exactly the five guard entries per surface. * fix(#2304): map Kimi tool_output and route workflow-guard block to stderr Third-party review (cross-AI verifier) caught two gaps in the revision: 1. Kimi PostToolUse events carry `tool_output`, not `tool_response` (kimi-cli src/kimi_cli/hooks/events.py post_tool_use()), so the read-injection scanner — which reads data.tool_response — was STILL dormant on real Kimi payloads; the earlier tests passed because they sent Claude-shaped payloads. The shared normalization block now maps tool_output -> tool_response (inert in PreToolUse guards, where the field is absent), and the scanner's Kimi tests send the real shape. 2. The workflow guard's force-add block wrote its reason to stdout only. Kimi's exit-2 protocol feeds stderr back to the model — the exact fix this PR already applied to the other blocking guard — so the newly-awakened block would have been a silent denial. Reason now also routed to stderr, asserted in the test. Also: the scanner's "unknown name" test now uses a genuinely unmapped name (FetchURL) — Shell stopped qualifying when it entered the map — and the workflow guard's write branch (WriteFile advisory, StrReplaceFile .planning pass) gains behavioral coverage. All five copies stay byte-identical (parity test green); golden fixtures regenerated, diff verified to the five guard entries per surface. Negative-controlled: 3 new assertions fail against the pre-fix hooks. * docs(#2304): update changeset to cover the full five-guard fix Review round 2 (2026-07-18) flagged the changeset as stale: it was written for the first commit and still described only the three guards named in the issue. The shipped diff grew to five guards plus two payload dimensions the original body never mentioned. The body now names gsd-read-injection-scanner and gsd-workflow-guard, the ReadFile and Shell vocabulary entries, the tool_output -> tool_response mapping, and the workflow guard's stderr block-reason routing. * test(#2304): regenerate kilo golden fixture after #2305 landed on next The branch's fixture sweep predates 50efae13 (fix(#2305), PR #2327), which made Kilo ship the five shared guard hooks. Rebased onto next and re-ran the full generator sweep (gen:golden, size:baseline, and the four registry/contract generators); the only delta across all of them is kilo.json's five guard-hook hashes, matching this PR's hook edits. * fix(#2304): fold Kimi normalization into the two shell hooks The 2026-07-19 review found the last two guards with the #2304 dormancy: - hooks/gsd-graphify-update.sh gated on tool_name == "Bash" but is registered on Kimi with matcher 'Shell' — Gate 1 never matched and the auto-rebuild was silently dormant. kimi-cli's Shell.Params names its field `command` (src/kimi_cli/tools/shell/__init__.py), same as Claude Bash, so only the name needs mapping: strip the module-path prefix, map Shell -> Bash. - hooks/gsd-phase-boundary.sh read only tool_input.file_path, but Kimi's file tools name the field `path` (src/kimi_cli/tools/file/write.py + replace.py) — the hook read '' and .planning/ writes went undetected. Falls back to tool_input.path when file_path is absent, mirroring normalizeKimiPayload's precedence in the JS guards. The normalization is reimplemented in shell — a byte-identity assertion cannot span the JS<->shell boundary, so the parity test gains a shell-guard vocabulary block that pins both scripts' mapping facts to convertKimiToolName's live vocabulary instead of faking a byte binding. Behavior is covered by negative-controlled tests beside each hook's existing suite (verified red against the pre-fix scripts): Kimi Shell dispatch (bare + module-qualified) with a WriteFile negative control in graphify-auto-update.slow.test.cjs, and Kimi path detection, file_path precedence, and a non-.planning negative control in hooks-opt-in.test.cjs. Changeset updated to name all seven guards; golden install-parity fixtures regenerated (diff is exactly the two hook entries per runtime; size baselines unchanged). * fix(#2304): use a Map for KIMI_TOOL_NAMES so prototype keys cannot pass the guard fall-through A bare bracket lookup on an object literal resolves 'constructor', '__proto__', 'toString', 'valueOf' and 'hasOwnProperty' through Object.prototype to truthy functions/objects, so `if (!mapped)` failed to short-circuit and data.tool_name was assigned a non-string. Map.get returns undefined for those keys — the same shape the repo already uses in canonicalizeRuntimeName (src/runtime-name-policy.cts). Applied identically to all five inlined copies (review M1, PR #2326). No new bypass class: unrecognized strings already fail open by design; this fixes the lookup being wrong, not the posture. * test(#2304): enumerate normalized guards by scanning hooks/, not a hardcoded list The parity test's file list was a literal five-entry array — a sixth guard with its own copy-pasted normalization block would be silently uncovered, the exact divergence mode the test exists to prevent (review M2). Now the list is a scan of hooks/*.js for the KIMI_TOOL_NAMES marker, with a floor assertion so a scan that finds nothing fails instead of passing vacuously. Also parses the Map declaration introduced by the M1 fix, and carries the allow-test-rule annotation documenting the source-text scanning (review m4). * test(#2304): parse hook JSON output instead of substring-matching raw stdout workflow-guard.test.cjs asserted on unparsed stdout while read-guard.test.cjs in the same PR parses the JSON envelope first — match the better pattern at all four assertion sites (review m5). * test(#2304): regenerate golden parity fixtures after Map conversion in the five guards * docs(#2304): reset changeset pr:0 placeholder for Phase 0 PR (#2507) The closed PR #2326's changeset carried pr:2326. Phase 0 of epic #2505 re-lands this fix on a fresh branch; the pr: field will be backfilled to the real Phase 0 PR number immediately after gh pr create returns. * docs(changeset): backfill PR #2518 for Phase 0 (#2507) --------- Co-authored-by: 0xdhx <darkhawkx@gmail.com>	2026-07-21 23:42:02 -04:00
Tom Boucher	9181c77df5	fix(#2515): auto-merge the release/hotfix -> main PR when clean (#2516) ... The finalize job created the release/hotfix -> main merge-back PR but never merged it, so every release and hotfix needed a manual merge click on main. The opposite direction (main -> next) is already admin-merged by auto-backmerge.yml; this direction was the asymmetric manual step where the recurring divergence got hand-reconciled. Adds a finalize step (after "Verify publish", so main only absorbs a confirmed-published release) that finds the open merge-back PR, polls until GitHub settles its mergeability, and admin-merges it with a merge commit ONLY when MERGEABLE. A CONFLICTING PR is left open for manual resolution rather than force-merged. Non-fatal (continue-on-error): the tag + npm publish already happened, so a merge-back that can't complete (org PR policy, token) must not fail the release. With the main-is-ancestor-of-next invariant restored (#2504), this merge is clean every release -- verified: a simulated 1.8.1 hotfix merges to main producing [1.8.1]->[1.8.0]->[1.7.0]->[1.6.1] and version 1.8.1 with no conflict, and main's hardened auto-backmerge.yml survives the merge. Guarded by three assertions in release-backmerge-invariants.test.cjs (step present + admin-merge, gates on MERGEABLE, continue-on-error); verified they fail when --admin or the MERGEABLE guard or the continue-on-error is removed. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>	2026-07-21 23:23:45 -04:00
Tom Boucher	bcdfd21c61	fix(#2504): make auto-backmerge survive a broken workflow copy + gate the invariants (#2506) ... The main->next auto-backmerge fails after nearly every release, leaving main not an ancestor of next, so the following release->main merge-back conflicts. Root cause is a copy-shuffling loop: auto-backmerge.yml must be identical on main and next, but `-s ours` (main->next) and the release-tree merge-back (release->main) each overwrite one copy wholesale, so a fix applied to one copy is repeatedly overwritten by the copy that lacks it. The build:lib step proves it: added to main (329233fc8), overwritten by the 1.7.0 merge-back, re-added to next (#2281), never on old-main -> the 1.7.0 backmerge ran on main's broken copy and failed at "Sync next's version" (npm version -> gen-capability-registry needs the gitignored capability-ledger from build:lib -> absent -> step fails -> "Open PR" skipped -> no PR -> main never becomes an ancestor of next). Two-part durable fix: 1. Blast-radius containment: mark the version-sync steps continue-on-error. The job's load-bearing purpose is opening + admin-merging the back-merge PR (the ancestry that keeps release->main clean). A version-sync failure (missing build:lib after a copy regression, or any npm-version lifecycle hiccup) can no longer abort that PR. A sync failure now costs only a stale next version, trivially re-synced -- never a broken back-merge. 2. Required-steps gate: tests/release-backmerge-invariants.test.cjs parses the workflow YAML and asserts build:lib runs before the version-sync, both steps are continue-on-error, the ancestry steps exist, and the finalize timeout is >= 30 (sibling #2281 regression). It runs on every branch, so a PR shipping a fix-less copy fails at PR time instead of at release time -- which is exactly what #1855/#1928/#1990 did undetected. Verified the test fails on both regression modes (continue-on-error removed; build:lib step removed) and passes on the fixed workflow. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>	2026-07-21 21:27:29 -04:00
Tom Boucher	d39ca0c004	Merge pull request #2490 from open-gsd/docs/2481-adr-1239-effort-axis ... feat(#2481): add a negotiated effortSurface axis and wire invocation-time effort	2026-07-21 20:14:27 -04:00
Tom Boucher	9fe9da9830	fix(#2488): strip leading terminators so changeset bullets survive re-parse (#2492) ... * fix(#2488): strip leading terminators so changeset bullets survive re-parse A fragment body beginning with a line terminator rendered as an empty `- ` bullet followed by an orphaned paragraph. `parseChangelog` treats a non-indented line as terminating a bullet, so `github-release-notes.cjs` silently dropped the entry when re-parsing CHANGELOG.md to build the GitHub Release body. Two independent causes, both in scripts/changeset/parse.cjs: 1. `extractDocsExempt` stripped trailing terminators but not leading ones. `DOCS_EXEMPT_RE` is `^...$` under /m, so removing a first-line `<!-- docs-exempt -->` marker left the `\n` that `$` does not consume. 2. `parseFragment` preserved the post-frontmatter body verbatim, so a blank line between the closing `---` and the first content line produced the same leading `\n` with no marker involved. 8 of 256 pending fragments were affected, split 4/4 across the two causes — including the OpenCode MCP binding, the pi extension, and the EoS adapters, all of which would have vanished from the v1.8.0 release notes. Regression tests cover both causes in LF and CRLF form, plus an end-to-end serializeChangelog -> parseChangelog round-trip that pins the user-visible defect. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * chore(#2488): regenerate golden install fixtures for parse.cjs scripts/ ships in the npm package and the installer, so the golden install-parity fixtures record a content hash for every shipped file. Editing scripts/changeset/parse.cjs drifts that hash and fails all 18 per-runtime parity tests. Regenerated via `npm run gen:golden`. The diff is exactly one line per fixture — the scripts/changeset/parse.cjs hash — with no unrelated drift. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>	2026-07-21 19:32:51 -04:00
Tom Boucher	ecef1e1670	test(#2481): add tracking ref to allow-test-rule exemption ... lint-allow-test-rule-refs (ADR-456) requires a #NNN issue ref on the SAME line as allow-test-rule:. CI-only lint (lint:ci), so it passed the local lint gate. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>	2026-07-21 19:10:22 -04:00
Tom Boucher	09b535ac00	feat(#2481): add a negotiated effortSurface axis and wire invocation-time effort ... ADR-1239 gains a ninth negotiated axis, effortSurface (argv | none), declaring how a host accepts reasoning effort. ADR-443 is amended in the same change because its recorded deferral is what the axis resolves: its Unblock condition offered paths (a) and (b) and stated the choice was 'a maintainer call this file records but does not make'. Path (a) is selected and satisfied here. Before this, effort reached a runtime only through install-time channels (EFFORT_RENDERING's frontmatter/api), so reviewer CLIs spawned as subprocesses silently inherited whatever effort sat in the user's own global CLI config. The review lane now resolves one universal effort through the ADR-443 cascade and renders it per host through the negotiated descriptor. Every per-host value is documentation-sourced, never inferred: - claude argv -- verified via 'claude --help' (--effort <level>) - opencode argv -- verified via 'opencode run --help' (--variant) - codex argv -- codex-rs/exec/src/cli.rs: model_reasoning_effort is NOT a CLI flag (config.toml key only), so the global -c override is the only argv route - 15 hosts undocumented -- their docs state no reasoning setting; the sentinel fails closed rather than inheriting a profile baseline No config-file vocabulary member: the only host that ever had one (Gemini CLI's thinkingConfig) was removed as a sunset runtime by 8f2ebbe9b (#1928, PR #1996), and neither Antigravity CLI nor ZCode documents a reasoning setting. Closes #2481 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>	2026-07-21 19:10:22 -04:00
Tom Boucher	c19d3d7bda	chore(#2453): resolve the uniform-router-param conflict and clear the warning floor (#2489) ... npx eslint . reported 0 errors, 53 warnings. The floor eroded the signal: a genuinely new warning had to be spotted against noise, so 'lint is clean' was not usable as a check. This takes it to zero. Fifty of the 53 were one category in gsd-core/bin/gsd-tools.cjs — all unused ARGUMENTS (error x30, cwd x10, raw x7, args x3), never unused variables. They are the Command Routing Hub's uniform handler signature, function routeX({ args, cwd, raw, error }), declared identically whether or not a handler uses all four members. argsIgnorePattern: '^_' is structurally in conflict with that convention: satisfying it would mean _-prefixing ~50 parameters and making the signature non-uniform across the table. Option 1 of #2453: disable args checking for that file only, keeping varsIgnorePattern intact so genuinely dead variables (the #2379 class) still surface — verified by injecting an unused variable, which still warns. This is the config decision #732 explicitly deferred ('Severities stay warn (no config change in this pass)'). The remaining three predate the issue's count of 51 and are real defects, not suppressions: - tests/workflow-compat.test.cjs — the step-9 lookahead used (?=\*\*10\.|\z). \z is a Perl/Ruby end-of-input anchor with NO meaning in JavaScript; it matched a literal 'z', so the lazy span silently stopped at the first z whenever **10. was absent, truncating the captured step and letting the assertion pass against a partial block. Corrected to $. - tests/debugger-prevention.test.cjs — an unused RegExp built one line above the one actually used. Removed. - tests/installer-migrations.test.cjs — try/finally inside a test body, which CONTRIBUTING prohibits ('verbose, masks test failures, not an approved pattern'); the unused t was the symptom. Converted to t.after(). Closes #2453 Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>	2026-07-21 18:38:51 -04:00
Tom Boucher	3435218089	fix(#2452): stop shallow-fetching the base ref in three-dot-diff CI gates (#2485) ... * fix(#2452): stop shallow-fetching the base ref in three-dot-diff CI gates `mutation.yml` re-fetched the base branch with `--depth=1` after checking out with `fetch-depth: 0`. The shallow re-fetch truncates the base ref's ancestry, so `git diff --name-only origin/<base>...HEAD` in scripts/mutation-matrix.cjs can no longer compute a merge base and aborts with `fatal: origin/next...HEAD: no merge base` (exit 2). The `detect` job then fails and the `mutate` shards never run — so the 80% mutation-score threshold went UNVERIFIED rather than enforced. The failure is branch-position dependent, which is why it went unnoticed: a branch already level with the base incidentally passes (its merge base IS the single fetched commit), while a branch that is BEHIND fails. Observed on PRs #2436 and #2005. `changeset-required.yml` and `docs-required.yml` shallow-fetched the base ref too (`--depth=50`), shrinking the same window further. All three now fetch the BASE REF unshallowed. Their shallow *checkout* depth is left at 50: that is a separate, deliberate cost control with fail-closed semantics, owned by tests/policy-lint-shallow-checkout.test.cjs. Only the base-ref fetch changes. The two `${{ }}` interpolations in mutation.yml's run: blocks now pass the base name through `env:`, matching the sibling workflows. Regression coverage in tests/mutation-workflow-base-ref.test.cjs: - a per-workflow contract guard asserting the base fetch carries no --depth (RED on origin/next for all three files, GREEN here). The YAML step parser handles block scalars and skips commented-out steps, so a future refactor to a multi-line `run:` cannot silently degrade the guard. - a real-git mechanism proof with boundary coverage at the shallow edge: with the base advanced 60 commits past the branch point, --depth=1 and --depth=60 both fail with `no merge base`, --depth=61 (merge base exactly at the boundary) succeeds, and an unbounded fetch succeeds. Each variant uses an independent clone, because a plain fetch does not un-shallow a repo that already carries a .git/shallow boundary. Also fixes a startup race in tests/run-with-timeout.test.cjs surfaced by this branch's gsd-test run (C1, linux-node24). The heartbeat file only appeared ~100ms after the grandchild's runtime was up, but the window is 1s spanning two cold node starts, so on a loaded runner the timeout fired before any heartbeat existed and the precondition failed for reasons unrelated to reaping. The child now writes its heartbeat once synchronously at startup; the frozen-vs-ticking comparison that actually proves reaping is unchanged. Closes #2452 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(changeset): backfill PR number to 2485 --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>	2026-07-21 11:14:29 -04:00
Tom Boucher	c5e0371775	feat(#1951): reversibility tagging — gate one-way-door decisions (#2471) ... * test(#1951): add failing-first tests for reversibility tagging Red phase for issue #1951 (reversibility tagging: classify decisions by undo cost, gate one-way doors behind a checkpoint:decision). Tests assert, per the issue's acceptance criteria: - discuss-phase CONTEXT.md template records a **Reversibility:** field with a rationale on captured decisions, and states it is optional - gsd-planner @-references planner-reversibility.md and stays under the 49152-char agent cap (LARGE_CAP, tests/agent-size-budget.test.cjs) - a one-way rating inserts a checkpoint:decision before the dependent task; reversible inserts none; costly is flagged but never blocks - the taxonomy defaults to reversible when unsure (checkpoint-fatigue guard) and inserting a checkpoint implies autonomous: false - docs/reference/plan-md.md documents <reversibility> as optional with all three ratings - --no-reversibility-gates parses to REVERSIBILITY_GATES=false, is injected into the planner prompt, and is advertised in the command argument-hint and help full mode (argument-hint parity) - the override suppresses the gate but still persists the rating - cmdVerifyPlanStructure accepts every rating and the absent case (additive-validator guarantee, behavioral via runGsdTools) - parity: thinking-models-planning.md #4 adopts the canonical three-level taxonomy and the binary REVERSIBLE/IRREVERSIBLE vocabulary is gone - no content loss from the planner extraction made to fit under the cap Prose-contract assertions are Red until the implementation lands. The behavioral validator assertions pass immediately — regression guards proving the validator already accepts unknown optional tags. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(#1951): reversibility tagging — gate one-way-door decisions Classify planning decisions by what undoing them would cost, and give a one-way door a human beat before the agent walks through it (issue #1951, The Pragmatic Programmer Topic 15 'Reversibility'; Bezos's one-way/two-way door framing). Acceptance criteria met: - discuss-phase records an optional reversibility rating with a rationale on <decisions> entries in the phase CONTEXT.md template. Unrated decisions are treated as reversible, so existing phases are unaffected. - a one-way rating makes gsd-planner insert a checkpoint:decision before the task that implements the decision, reusing the existing checkpoint mechanism -- no new checkpoint machinery. - reversible ratings trigger no checkpoint; costly ratings are flagged in the plan but never block. - the rating persists on the task as the optional <reversibility rating=> element. cmdVerifyPlanStructure accepts every rating and the absent case; the structural validator does not reject unknown optional tags. - --no-reversibility-gates (REVERSIBILITY_GATES=false) suppresses checkpoint insertion for intentionally-unattended runs while still recording ratings -- the override changes what stops the run, not what the plan remembers. Single taxonomy, not two: references/thinking-models-planning.md #4 already shipped a binary REVERSIBLE/IRREVERSIBLE classification and is loaded by both gsd-planner and gsd-plan-checker. It is rewritten onto the canonical three-level vocabulary and now points at planner-reversibility.md as the taxonomy owner, with a parity test that fails if the surfaces diverge (DEFECT.GENERATIVE-FIX-DIVERGENCE). agents/gsd-planner.md sat 47 chars under the 49152 LARGE_CAP, so the checkpoint DO/DON'T guidance was relocated verbatim into planner-antipatterns.md -- already @-referenced from the same section for the same topic, so the planner still loads it and nothing was dropped. A test guards the relocation against content loss. Files: gsd-core/references/planner-reversibility.md (NEW, canonical taxonomy + emission rules + anti-patterns), gsd-planner.md, plan-phase workflow/command/help (flag wiring + parity), plan-md.md schema, discuss-phase context template, CONTEXT.md glossary, INVENTORY + manifest, size baselines, install goldens, plugin skills regen, changeset. Closes #1951 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(#1951): address orthogonal review findings Two isolated reviewers (correctness + security), neither of which authored the change. Every finding fixed: Security — the rationale is untrusted input (ADR-1577). It originates in conversation and flows CONTEXT.md -> planner -> PLAN.md -> executor, each hop an LLM reading the previous hop's output, with no validation on the path. planner-reversibility.md and the discuss-phase template now state it is data and never instructions, and name the </reversibility> early-termination hazard explicitly -- a rationale that closes its own element injects sibling structure the executor reads as real tasks. Four tests guard it. Correctness 1 — nothing machine-enforced the feature's own promise: a task rated one-way with no preceding checkpoint:decision validated as fully clean, so a planner error silently reopened the gap this feature exists to close. cmdVerifyPlanStructure now warns on an ungated one-way rating. A warning, not an error: <reversibility> stays additive and the plan stays valid. Four tests cover ungated (warns), gated (silent), still-valid, and reversible/costly never flagged. Correctness 2 — pass-always test. The --no-reversibility-gates parse test substring-matched the whole workflow file, and plan-phase.md prose mentions both tokens in one sentence, so it passed with the bash conditional deleted: it was testing the documentation, not the parser. Now scoped to the fenced bash blocks and matched as one physical line, with a negative control confirming prose alone cannot satisfy it. Correctness 3 — costly had no itemized emission rule, only one-way did, so two agents could diverge on whether to tag costly at all. Correctness 4 — template convention break: the example ratings were bare while every sibling field uses [...] to signal substitution, inviting an LLM to copy one-way/costly forward as boilerplate. Now bracketed. Correctness 5 — latent false-green: .includes('reversible') also matches inside irreversible/irreversibility, which appear in anti-pattern prose, so a surface that dropped the real taxonomy entry would still pass. Now word-boundary matched. ADR-857 phase-6 ceiling — the first gsd-test run caught plan-phase.md 1216 bytes over its frozen 94519 ceiling (it had 49 bytes of headroom on next). The ceiling may only rise for privileged host machinery, and reversibility gating is optional-feature logic, so the wiring was slimmed to its minimum and the explanatory prose moved to the reference files the planner already loads. plan-phase.md is now 94400 bytes -- 119 under the ceiling and 70 bytes SMALLER than on next, so the host loop shrank while gaining the feature, which is what phase 6 ratchets toward. The tracer contract (tests/tracer-bullet.test.cjs) is unchanged. Lint — fixed an unnecessary non-null assertion in verify.cts and a CRLF-fragile bare \n regex in the new test (DEFECT.WINDOWS-CRLF-TEST- PORTABILITY, the #1658/#1668/#2206/#2449/#2450 class). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(#1951): checkpoint fixture must carry the common task elements The gated-one-way fixture built a checkpoint:decision task from the abbreviated skeleton in gsd-planner.md, which shows only the checkpoint-specific elements (<decision>/<context>/<resume-signal>). cmdVerifyPlanStructure requires <name> and <action> on EVERY task regardless of type, so the fixture failed validation for reasons that had nothing to do with reversibility: errors: ["Task missing <name> element", "Task 'unnamed' missing <action>"] Caught by gsd-test on 14d14a39 (2 failures, both this fixture). The canonical shape is in tests/verify.test.cjs:266 — a checkpoint task carries <name>/<files>/<action>/<verify> like any other. Fixture corrected to match. Verified behaviorally against the real gsd-tools CLI across all four cases: gated one-way (valid, silent), ungated one-way (valid, warns), costly (valid, silent), absent (valid, silent). Not a product defect: the validator's every-task contract is intentional and pre-existing, and docs/reference/plan-md.md scopes its required-element list to type=auto/tracer only because those are the elements a planner must author, not because checkpoints are exempt from <name>. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * chore(#1951): backfill changeset pr number to 2471 * fix(#1951): CodeQL incomplete-sanitization + prompt-injection scan collision Both CI failures were real defects in code this PR added, not false positives. CodeQL js/incomplete-sanitization (high), reversibility-tagging.test.cjs:46 — the namesRating helper built its regex with `rating.replace(/[-]/g, '\\-')`, which escapes the hyphen but not backslash, so the escape was incomplete. It was also unnecessary: `-` carries no special meaning outside a character class. Replaced with a complete metacharacter escape (backslash included). Word-boundary behavior verified unchanged across all three ratings — notably that "irreversible" prose still does not satisfy a "reversible" match, which is the false-green this helper exists to prevent. Prompt injection scan — the checkpoint fixture used the human-verification child element inside <verify>. That tag name is a fake-instruction-boundary pattern in scripts/prompt-injection-scan.sh, and the scan runs over changed files, so copying the shape from tests/verify.test.cjs (unflagged only because it is not in this diff) tripped the gate. Switched to the documented plain-prose <verify> form. The first attempt at that fix failed the same gate a second time: the comment explaining the collision quoted the offending tag literally. The comment now names it in prose instead — the scanner does not care whether a match is code or commentary, which is the whole point of the DEFECT.PROMPT-INJECTION-SCAN-COLLISION note in CLAUDE.md. Verified locally before push: scan reports 0 findings across 57 changed files, eslint clean, and both fixtures still validate as designed (gated one-way silent, ungated one-way warns, neither errors). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(#1951): record measured cost and halve gsd-tools spawns The Windows shard 1/3 job timeout was traced to the sharding layer, not to this PR's assertions — see #2472. Two contributing factors were this file's own, and are fixed here. 1. tests/test-timings.json had no entry for reversibility-tagging.test.cjs, so scripts/run-tests.cjs weighted it at the table's median fallback (~315ms) for LPT chunk packing. It actually measures 5595ms — an 18x under-weight. Recorded the measured value from the green gsd-test run (max across the node22/node24 lanes, per gen-test-timings.cjs's convention). Only this one entry: a full regen churns 634 entries of run-to-run drift, and the table is explicitly advisory and un-gated, so a 637-line diff does not belong in a feature PR. 2. Each verifyPlan() spawns gsd-tools, which dominates this file's cost. Spawns cut from 9 to 6 with no coverage lost: - the ungated-one-way warning and its stays-valid assertion now share one plan instead of building the same plan twice; - the reversible/costly never-flagged-as-ungated test was strictly subsumed by the additive suite, which already runs those two ratings ungated and asserts no /reversibilit/ warning at all — and the gate warning's text contains both "reversibility" and "one-way", so the broader assertion catches it. It only re-spawned gsd-tools twice to prove the same thing. Both are symptom fixes. The shard imbalance itself (19/11/10 minutes against a 20-minute cap, from a cost-blind round-robin partition that also reshuffles downstream files whenever one is inserted) is tracked in #2472. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(#1951): checkpoint fixture adopts the #2444 type-branched contract Surfaced by rebasing onto next, which gained #2444 (branch plan-structure validation on task type=checkpoint:*) while this PR was in review. cmdVerifyPlanStructure no longer applies one required-element set to every task. A checkpoint:decision now requires <name> + <resume-signal> + <decision> + <options>, and is exempt from the <action>/<verify>/<done>/ <files> set that auto and tracer tasks carry. The gated-one-way fixture predated that split and failed on the new requirement: errors: ["Task 'Task 0: Confirm the on-disk format' missing <options>"] Fixture rewritten to mirror the checkpoint:decision contract exactly — real <options> with two <option> children — rather than padding it with fields checkpoints no longer need. That also drops the plain-prose <verify> the earlier revision carried purely to dodge the prompt-injection scan; a checkpoint task has no <verify> requirement at all, so the workaround is moot. Verified against the real gsd-tools CLI across all four cases: gated one-way (valid, silent), ungated one-way (valid, warns), costly (valid, silent), absent (valid, silent). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>	2026-07-21 10:44:55 -04:00
Tom Boucher	04a0eb8d63	fix(#2450): CRLF-tolerant session-section rewrite + no-op-detection guard (#2482) ... * fix(#2444): re-resolve body-parser to 2.3.0 in lockfile (GHSA-v422-hmwv-36x6) GHSA-v422-hmwv-36x6 (body-parser DoS via invalid limit value, low severity, published 2026-07-20T23:23:26Z) made tests/npm-integrity-gate.test.cjs (#3588: root workspace production tree has no advisories) fail any subsequent npm audit --omit=dev. The advisory affects body-parser >=2.0.0 <2.3.0 pulled transitively via @anthropic-ai/claude-agent-sdk -> @modelcontextprotocol/sdk -> express -> body-parser@2.2.2. express@5.2.1 already declares body-parser as ^2.2.1, so 2.3.0 is a valid re-resolution within express's own compatibility range — no override needed. Regenerated the lockfile via 'npm audit fix --omit=dev' which re-resolves transitive deps within their declared ranges; package.json is unchanged. Verified: npm audit --omit=dev reports 0/0/0/0/0 advisories; body-parser now reads as 2.3.0 in 'npm ls body-parser --omit=dev'. * test(#2450): failing-first CRLF regression for record-session insert path cmdStateRecordSession's section-rewrite regexes (src/state.cts:1166, :1195) used literal \\n which cannot match CRLF STATE.md delimiters. The detector regex (CRLF-tolerant via $ under /m) entered the rewrite branch, the writer regex silently no-op'd, but updated.push(...)/ sessionCreated=true ran unconditionally. Result: caller reported recorded:true with 'Resume File' in updated, but the field was never written to disk. With core.autocrlf=input, the CRLF working-tree file produces no git diff, so the bug was invisible. Adds three regression tests covering all three rewrite paths: - CRLF STATE.md with ## Session and Resume file absent - CRLF STATE.md with ## Session and Stopped at absent - CRLF STATE.md with ## Session Continuity (bootstrap shape) Each asserts the field IS on disk (the bug discriminator: the pre-fix command's JSON output looked identical to a successful write). * fix(#2450): CRLF-tolerant session-section rewrite + no-op-detection guard Two regexes in cmdStateRecordSession used literal \\n which cannot match CRLF STATE.md, silently no-op'ing the section rewrite while the CRLF- tolerant detector above entered the branch. The reporter's exact repro: on a CRLF STATE.md with one canonical session field absent, the command returned recorded:true + updated:['Resume File'] but the field was never written to disk. Three changes: 1. src/state.cts:1166 (canonical ## Session rewrite regex): \\n -> \\r?\\n 2. src/state.cts:1195 (## Session Continuity insert regex): \\n -> \\r?\\n 3. Defensive invariant (#2450 class fix per reporter's suggestion): track whether the chosen branch's replace actually matched via callback flag. Only set sessionCreated=true and push to updated when rewriteMatched. Unreachable post-fix, but fail-loud is the right posture for a silent- success gate. If a future drift between the detector and writer regexes reintroduces the asymmetry, the caller will not see false updated entries. Same canonical CRLF-tolerant form already in use at check-command-router.cts :205 (extractPlanDesignatedSections). Same bug class previously fixed in #1658, #1668, #2206, #2449. * fix(#2450): address review followups + add changeset Code-review + security-review both flagged the unreachable else at the Session Continuity branch (defaulted rewriteMatched=true in dead code, re-arming the bug class for future drift). Removed the else; the remaining code path leaves rewriteMatched=false if linesToInsert is empty, preserving the fail-loud posture. Added scope-limitation doc to the rewriteMatched gate: it covers the INSERT path only, not the earlier in-place stateReplaceField successes (which DID land on disk and correctly push to updated unconditionally). Tests: - Normalized STATE_CRLF_SESSION_MISSING_RESUME fixture to match the canonical 6-key frontmatter of STATE_WITH_SESSION (code-review I2). - Added mixed-ending test (LF frontmatter + CRLF body) to close CONTRIBUTING.md:490 'Mixed CRLF/LF newlines' requirement (I1). Added Fixed changeset (code-review H1). * docs(changeset): backfill PR number to 2482	2026-07-21 09:53:14 -04:00