* fix(#1619): normalize pruned mise node execPath to the stable shim
resolveNodeRunner() bakes process.execPath into managed .js hook commands.
Node realpaths execPath, so under mise it resolves to a concrete
<data>/installs/node/<ver>/bin/node that mise prunes on `mise up`, after
which every managed hook 404s — the same ephemeral-path failure #977 fixed
for fnm and #3181 for Homebrew. normalizeNodePath now rewrites a mise
versioned install path to the stable sibling shim <data>/shims/node when it
exists (deriving <data> from execPath so a custom MISE_DATA_DIR works),
falling back to the raw execPath otherwise. Tests folded into
install.test.cjs per the regression test-name lint.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* chore(changeset): set pr number to 1621
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Joe Seymour <joese@iarx.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: Tom Boucher <trekkie@nomorestars.com>
Three related defects in cmdConfigSet, all 'config-set stores invalid values silently':
1. Missing guards: workflow.security_block_on (enum) and
workflow.security_asvs_level (integer 1-3) had no store-time validation.
2. Systemic JSON-coercion bypass: every string-enum guard used
VALID_X.includes(String(parsedValue)). Because the value is JSON-parsed
before validation, String(["member"]) === "member" let a JSON array
slip through and an array was stored in a scalar key. Reproduced on
human_verify_mode, statusline.context_position, context_guard_mode,
fallow.scope/profile, source_grounding_authority, drift_action, context.
3. Unvalidated capability keys: 32 capability-registry-owned keys (4 enum,
25 boolean, 2 number, 1 string) had no hardcoded guard, so any value —
including coerced arrays/objects and out-of-enum strings like
code_review_depth=garbage — was stored silently.
Fix: a type-safe assertEnumValue() helper (requires typeof === 'string'
before membership), routed through all nine central string-enum guards
(messages preserved byte-for-byte); plus a generic capability-registry
validation block that validates every capability key against its declared
type/values (enum via the registry's values — single source of truth —
boolean, number, string). Behavioral regression tests cover every central
enum key and representative capability keys (array + object coercion
rejected, out-of-enum rejected, valid accepted) with boundary coverage for
the security keys.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Codex adversarial orthogonal review of PR #1622 surfaced that applySurface (src/surface.cts) only called rewriteStagedSkillBodies for kind='skills', skipping kind='commands'. The gap meant /gsd-surface profile changes on any runtime with commands kinds (windsurf, opencode, kilo, cursor, augment, codebuddy, gemini) wrote raw @~/.claude/... references into synced command/workflow bodies, which fail at invocation time on non-Claude runtimes.
For Windsurf specifically, this left workflow files containing @~/.claude/gsd-core/commands/gsd/X.md after a profile change — paths that don't exist on a Windsurf install. Verified by the new regression test which fails before the fix (workflow bodies contained @~/.claude/) and passes after (workflow bodies reference the install target).
Captures the return value of rewriteStagedCommandBodies (temp dir path — commands rewrite uses copy-then-rewrite to avoid mutating the package source), syncs from the temp dir, then cleans up. Type annotations satisfy typescript-eslint strict mode.
Findings 2 (install ordering) and 3 (legacy .devin cleanup) from the same review are tracked in #1629 — both real but out of scope for #1615.
Codex peer review of PR #1622 surfaced that convertClaudeCommandToWindsurfWorkflow interpolated commandName unsanitized into a markdown body that Windsurf loads as an LLM-readable workflow. A plugin author who controls a commands/gsd/*.md filename could inject newlines, markdown structure, or path components (..) to manipulate the workflow body.
Validate commandName at function entry against /^(?:gsd-)?[a-z0-9](?:[a-z0-9-]*[a-z0-9])?$/ — rejects slashes, backslashes, spaces, dots, control chars, trailing dash. Pattern requires alphanumeric ending so gsd- alone (which would slice to empty stem) is also rejected. Throws with a JSON.stringify-escaped preview (no literal newlines in the error message).
Applied to both bin/install.js (where tests import from) and src/runtime-artifact-conversion.cts (production source). 18 positive + 22 negative test cases lock in the validation.
computePathPrefix returned a Windows-style path (with backslashes from path.join) into markdown @-references. Workflow file content on Windows ended up with mixed separators, breaking substring checks in install/install-runtime-artifacts tests on windows-latest CI only.
Normalize resolvedTarget and homeDir to forward slashes inside computePathPrefix. The prefix is always substituted into markdown body text, which uses POSIX paths universally. Idempotent on POSIX.
Also normalizes the two test assertions to forward-slash form so they pass on Windows. Adds a regression test for backslash-style input.
Documents DEFECT.WINDOWS-PATH-LEAK-IN-MARKDOWN-CONTENT + RULESET.CONTENT-PATH-NORMALIZATION in CONTEXT.md so this anti-pattern stops recurring.
Add an optional structured `coverage:` block to SUMMARY.md frontmatter and a
deterministic classifier that `verify-work` consumes to route deliverables to
auto-pass vs human-UAT — replacing the rejected #1598/#1599 post-hoc heuristic.
- New `src/coverage.cts` (→ bin/lib/coverage.cjs) parses the nested coverage
block (extractFrontmatter can't — its `-` items are scalars-only; this is a
focused parser, sibling of parseMustHavesBlock), validates each entry, and
classifies into auto_passed vs present. Frozen MODE/PRESENT_REASON/ERROR_CODE
typed-IR surface. Exposed via `uat classify-coverage --summary <f>`.
- Auto-pass is the narrow proven case only: strict-boolean human_judgment:false
AND non-empty all-`pass` verification AND zero validation errors. Everything
else — judgment, empty/failing verification, malformed entry — routes to the
human (fail-safe). A malformed block falls back to legacy prose extraction and
surfaces an error; an absent block is byte-identical to pre-#1602.
- execute-plan create_summary populates the block (fail-safe default
human_judgment:true); verify-work extract_tests consumes it; create_uat_file
marks auto-passed entries `source: automated`.
- Templates (summary + 3 variants), CONTEXT.md predicate + glossary, INVENTORY,
eslint/gitignore registration, and Diataxis docs (COMMANDS reference +
USER-GUIDE explanation) updated.
- Behavioral tests via the CLI (no source-grep); parser-robustness regressions
for the null-entry/comment-header/mis-indent cases found in adversarial review.
Closes#1602
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Per review (test-standard items):
- Property test (RULESET.TESTS.property-based-testing): extractRetiredPhaseNumbers
is the parsing core, so add a fast-check property — k of n checklist phases
struck → exactly the k canonical keys returned, across randomized phase counts
and numeric/zero-padded/project-code ID forms. Exposed via a `_`-prefixed test
seam (mirrors the existing _setLockProbes seams), no public API surface added.
- Boundary (RULESET.TESTS.boundary-coverage): all-retired case (k === n) →
total_phases 0, via state json.
No production behavior change; the exclusion logic is unchanged.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
GitHub Copilot reads repository-wide instructions only from
.github/copilot-instructions.md (confirmed via GitHub Docs), not a root
copilot-instructions.md. Aligns getProjectInstructionFile with the installer
(runtime-config-adapter-registry installSurface 'copilot-instructions') and
cites the docs source in the doc-comment.
A retired/folded phase (struck through in ROADMAP, marked [x], with a
directory but no completion artifact) was counted in the total_phases
denominator via max(phaseDirs.length, roadmapPhaseCount), yet could never
satisfy the numerator (no SUMMARY → never "completed"), freezing shipped
milestones below 100% (e.g. 5/6 = 83%).
Both STATE counting paths now read the current-milestone ROADMAP scope and
exclude retired phases from BOTH the disk phase-dir set and the heading
count, so a retired phase counts toward neither denominator nor numerator:
- buildStateFrontmatter (`state json`)
- cmdStateSync (`state sync --verify` / rebuild) — previously re-derived the
inflated denominator and reported "no drift", per the issue.
Retired detection (extractRetiredPhaseNumbers) is scoped to the lines that
canonically mark a phase retired — a checklist entry (`- [x] …`) or a phase
heading — and within those, only a struck span whose SUBJECT is the phase
(`~~**Phase 04: Delta**~~`). So struck prose, a struck goal line, and the fold
target ("folded into Phase 05") are not misread as retired.
Phase matching uses the canonical phase-id helpers (normalizePhaseName +
extractPhaseToken), so numeric, decimal, and project-code IDs (PROJ-42) match
consistently across ROADMAP tokens and on-disk dir names.
Scope boundaries (separate, pre-existing concerns left unchanged):
- `roadmap analyze` (src/roadmap.cts) intentionally trusts the [x] checkbox
(incl. externally-completed phases) — a different reporting surface.
- cmdStateSync does not apply the milestone phase-dir filter (so 999.x /
other-milestone dirs can still affect its count); that is the #1445 /
milestone-filter axis, independent of retired phases.
Same counting family as #549 / #500 / #1445.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* feat(#441): add /gsd-capture --list-seeds for seed listing and audit
Seeds (.planning/seeds/SEED-NNN-slug.md) could only be created (--seed),
enriched (--enrich), or auto-surfaced at /gsd-new-milestone. There was no way
to browse or audit parked seeds on demand. This adds a read-only listing,
following the established --list → workflow pattern (per the approved scope on
- gsd-tools `list-seeds [status]` (cmdListSeeds in src/commands.cts): scans the
seeds dir, returns { count, seeds[], summary } JSON with each seed's id,
slug, status, scope, trigger_when, planted, title. Optional case-insensitive
status filter. User-controlled content is sanitized (sanitizeForDisplay) and
every path validated (requireSafePath); read-only. Independent of
audit.scanSeeds, which only returns unimplemented seeds for the milestone surface.
- /gsd-capture --list-seeds routes to a new read-only list-seeds workflow that
renders the seed table.
Closes#441
* chore(#441): point changeset fragment at PR #722
* test(#441): allowlist list-seeds test in prompt-injection scan
The test asserts that list-seeds neutralizes injection payloads
(<system>, [INST]) embedded in seed content, so the fixtures legitimately
contain those patterns — same as the sibling security tests already on the
allowlist.
* fix(#441): use canonical /gsd:capture colon form in list-seeds workflow
Claude-facing source (commands/, agents/, gsd-core/workflows/, ...) must use
the /gsd:<cmd> colon form per ADR/CONTEXT.md; the hyphen /gsd-<cmd> form is
retired there (enforced by bug-2543-gsd-slash-namespace.test.cjs). The new
list-seeds workflow used the hyphen form.
* docs(#441): sync help full.md + INVENTORY for --list-seeds
Adds the --list-seeds entry to the help reference (help/modes/full.md, per
bug-2954 argument-hint↔help parity) and registers the new list-seeds workflow
in docs/INVENTORY.md (88→89) and the generated INVENTORY-MANIFEST.json.
* docs(#441): add --list-seeds how-to + drop phantom statuses
Addresses CHANGES_REQUESTED on PR #722 (two documentation blockers):
- USER-GUIDE.md Seeds section (how-to): extend the task to cover
auditing parked seeds on demand via --list-seeds, including the
status filter — kept task-oriented per Diataxis how-to mode.
- CLI-TOOLS.md (reference): drop phantom statuses implemented|rejected
from the list-seeds filter vocabulary; the system only produces
dormant|active|triggered (src/audit.cts scanSeeds). Reference must
be factually accurate and complete.
* fix(#441): guard non-scalar status frontmatter in cmdListSeeds
A seed with a bare `status:` line (extractFrontmatter yields {}) or a
`status: [a, b]` value (yields an array) crashed the whole audit list:
`(fm.status || 'dormant').toLowerCase()` throws a TypeError on a non-string.
Coerce every frontmatter read through a `fmStr` helper (mirrors the existing
`typeof fm.id === 'string'` guard), so a non-scalar status falls back to
dormant and non-scalar scope/trigger_when/title can no longer leak a raw
array/object into the JSON contract. Title is now capped symmetrically.
Adds regression coverage for empty and array `status:` and non-scalar fields.
Refs #441
* docs(#441): align list-seeds workflow status vocabulary
The load_seeds step listed `implemented` as an example status filter, but the
real seed vocabulary is dormant|active|triggered (src/audit.cts scanSeeds);
`implemented` has no producer. Matches the earlier CLI-TOOLS.md correction.
Refs #441
* refactor(#441): extract pure deriveSeedIdentity; match raw status in list-seeds
Pull the seed_id/slug derivation out of cmdListSeeds into a pure, exported
deriveSeedIdentity(stem, rawFmId) so the parsing contract can be property-tested
in-process (review minor #1). No behavior change.
Filter comparison now matches the raw lowercased status (both sides already
normalized) instead of sanitizeForDisplay(status); sanitization is for output,
not matching (review nit #3).
* test(#441): add fast-check property coverage and count=1 boundary for list-seeds
Adds tests/list-seeds.property.test.cjs with four fast-check properties over
deriveSeedIdentity (never-throws, string-only contract, canonical id->seed_id/slug
invariant, filename-prefix fallback) per RULESET.TESTS.property-based-testing
(review minor #1).
Adds an N==1 status-filter boundary case to list-seeds.test.cjs (review minor #2).
* chore(#441): sync runtime launcher snippet into list-seeds workflow
Propagate the current _runtime-launcher.snippet.sh (with non-Claude
runtime home probes) into the new list-seeds.md workflow via
scripts/sync-runtime-launcher.cjs, satisfying bug-891 (E) propagation.
* test(#441): record list-seeds.md in workflow size baseline (#1074)
---------
Co-authored-by: Tom Boucher <trekkie@nomorestars.com>
* fix(#1383): resolve GSD version without a top-level require of the runtime-root package.json
The extracted runtime-artifact-conversion module sits in the gsd-tools
loader chain and did a module-load `require('../../../package.json')`.
On Codex (whose runtime root has no package.json) that threw
`Cannot find module '../../../package.json'`, crashing every gsd-tools
command before it did anything. Even on Claude the synthetic
`{"type":"commonjs"}` has no `version`, so the sole consumer already
emitted `version: undefined`.
Resolve the version lazily and defensively instead: read the installed
gsd-core/VERSION, else lazily require the runtime-root package.json, else
degrade to '' so the caller omits the field. Both sources are validated
against the repo's semver-prefix convention (mirrors update-context.cts)
so a garbled VERSION is never emitted verbatim. install.js's dead
duplicate converter is intentionally left untouched (scoped to the crash).
Adds a #1383 regression block exercising resolveVersionFrom across
VERSION-only / package.json-only / neither / malformed-VERSION layouts,
asserting no-throw and the correct version string.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* chore(#1383): add changeset for the Codex gsd-tools crash fix
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* docs(#1383): record resolveVersionFrom export in CONTEXT.md glossary
Maintainer review gate on PR #1409: the lazy resolveVersionFrom seam added on
the Runtime Artifact Conversion Module must be recorded in CONTEXT.md so the
canonical glossary doesn't drift from the exported surface.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* chore(#1383): reword changeset to drop product-name parenthetical
product-name-purity (#1777) rejects 'Codex (…)' parentheticals that render
verbatim into CHANGELOG.md. Reword to a comma clause; no behavior change.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: Tom Boucher <trekkie@nomorestars.com>
* fix(#1551): match dash-separated milestone phase IDs in roadmap analyze checklist scan
The checklist scanner in cmdRoadmapAnalyze allowed only a dot separator
(?:\.\d+)* while the detail-heading scanner allows [.-], so milestone-prefixed
IDs (1-01) truncated at the dash (-> 1) and reported phantom missing detail
sections on every well-formed milestone roadmap. Widen the char class to
(?:[.-]\d+)* to match the detail scanner and the shared phaseMarkdownRegexSource
helper.
Fixes#1551
Claude-Session: https://claude.ai/code/session_01H96MxPGMJJUiJLV2NgzV16
* chore(changeset): Fixed fragment for #1552 (roadmap milestone-id checklist scan)
Claude-Session: https://claude.ai/code/session_01H96MxPGMJJUiJLV2NgzV16
---------
Co-authored-by: Tom Boucher <trekkie@nomorestars.com>
trek-e's review found the M1 PID-liveness backport dropped two pieces of
capability-lock.cts's steal-safety machinery, reopening the #500/#905/#1230
lost-update family:
- Empty-body window (state.cts): acquireStateLock creates the lock with O_EXCL
and writes the pid in a separate writeSync; a lock observed in that gap has an
empty body, reads as not-verified-live, and was stolen at age ~0 — robbing a
holder mid-creation. Add a fresh-create floor scoped to the unverifiable-body
case: an empty/unparseable body that is fresh is treated as mid-creation and is
NOT stolen, while a COMPLETE dead-pid body is still stolen promptly (preserves
the prompt-dead-steal contract). planning-workspace writes its body atomically
(flag:'wx') so it has no empty-body window.
- Double-steal (both locks): the steal was a bare fs.unlinkSync with no identity
re-confirm, so two waiters could both reclaim a dead holder and end up holding
concurrently. Replace with an atomic renameSync (only one racer wins the inode)
guarded by a (dev,ino,body) identity re-confirm immediately before the steal;
body content is part of the identity to defeat inode reuse.
Tests (seam-driven, no wall-clock, each proven RED-before-GREEN):
- clock-seam: fresh empty-body lock is not stolen at age ~0; a racer-recreated
live lock is not double-stolen (identity re-confirm). Adds a beforeSteal seam.
- planning-workspace: racer-recreated live lock is not double-stolen.
- Updated the two #1217 unlinkSync-failure tests to the renameSync steal path
(the bounded-backoff/no-busy-spin guarantee is preserved and re-asserted).
Uncontended acquire path is byte-for-byte unchanged.
Claude-Session: https://claude.ai/code/session_01R88n7Q54bAaVHFkDbbH1yz
* fix(core): roadmap upgrade rollback must restore .planning regardless of git tracking (#1542)
applyMigration rolled back a failed migration with git reset --hard + git clean
-fd .planning/phases/. For a commit_docs:false project (.planning gitignored —
the default) that restores NOTHING (reset ignores untracked, clean without -x
skips ignored), yet it threw 'Migration failed (rolled back to <sha>)' — a false
claim leaving .planning half-migrated. git reset --hard is also a whole-repo op.
Replace it with a surgical, git-independent rollback: record the exact renames
performed and snapshot each file before rewriting it, then on failure reverse the
renames and restore the snapshots (deleting files that did not previously exist).
Correct whether .planning is tracked or ignored; touches only what it changed.
Claude-Session: https://claude.ai/code/session_01R88n7Q54bAaVHFkDbbH1yz
* chore(changeset): Fixed fragment for #1543 (roadmap upgrade surgical rollback)
Claude-Session: https://claude.ai/code/session_01R88n7Q54bAaVHFkDbbH1yz
* test(core): update bug-685 execSync count floor after surgical rollback (#1542)
The #1542 surgical, git-independent rollback removed the rev-parse/reset/clean
git execSync calls from roadmap-upgrade.cts, leaving only the git status
precondition. bug-685 asserted calls.length >= 4; lower the floor to >= 1 — the
durable guard (every remaining git execSync sets windowsHide:true) is unchanged.
Claude-Session: https://claude.ai/code/session_01R88n7Q54bAaVHFkDbbH1yz
---------
Co-authored-by: Tom Boucher <trekkie@nomorestars.com>
* fix(core): platformWriteSync must retry transient rename locks, not truncate readers (#1540)
platformWriteSync fell back to a non-atomic fs.writeFileSync(filePath) on ANY
error from the temp+rename path. On Windows, renameSync onto a target a reader
holds open throws EPERM/EBUSY/EACCES (the common case for hot files like
STATE.md), so the fallback fired and a concurrent reader saw the file
mid-truncation.
Mirror the capability-ledger rename-retry idiom: retry transient lock errnos
(EPERM/EBUSY/EACCES) with a bounded Atomics.wait backoff; on a persistent lock,
surface the error rather than do the truncating non-atomic write. Genuinely
unrenameable cases (EXDEV cross-device) and tmp-write failures still fall back.
Claude-Session: https://claude.ai/code/session_01R88n7Q54bAaVHFkDbbH1yz
* chore(changeset): Fixed fragment for #1541 (platformWriteSync rename retry)
Claude-Session: https://claude.ai/code/session_01R88n7Q54bAaVHFkDbbH1yz
---------
Co-authored-by: Tom Boucher <trekkie@nomorestars.com>
* fix(core): roadmap upgrade must not process.exit inside the no-throw hub (#1538)
The upgrade handler called process.stderr.write + process.exit(1) on an
unsupported --convention, structurally bypassing the command-routing-hub's
no-throw contract (ADR-0012). It also parsed only the space-separated
--convention <value> form, so --convention=<value> was silently dropped and
defaulted to milestone-prefixed, running the migration the user did not request.
Throw instead of exit (the hub converts to HandlerFailure and the adapter routes
it through error()); parse both --convention forms and fail closed on any
missing/unsupported value.
Claude-Session: https://claude.ai/code/session_01R88n7Q54bAaVHFkDbbH1yz
* chore(changeset): Fixed fragment for #1539 (roadmap upgrade hub contract)
Claude-Session: https://claude.ai/code/session_01R88n7Q54bAaVHFkDbbH1yz
---------
Co-authored-by: Tom Boucher <trekkie@nomorestars.com>
* fix: make punctuated CANONICAL_HEADERS synonyms reachable (audit M7)
classifyHeader received an already-normalized header (via normalizeAdrHeader,
which collapses [\s:._-]+ to a space and strips [^\w\s]) but compared it against
the RAW synonym strings. So any synonym carrying a hyphen/apostrophe ('trade-offs',
'non-goals', 'anti-goals', 'follow-up', 'cross-cuts', 'post-grilling', "how we'll
know", "won't do/have") could never match — its ADR section silently went unmapped.
Nine synonyms across six buckets were dead; the repo had characterization tests
pinning that quirk ('unreachable synonym').
Root-cause fix (per ADR-1372's 'compound, don't accrete' guidance): normalize BOTH
sides via a module-load-precomputed index, instead of pre-baking 9 normalized
literals into the data table. Closes the abstraction asymmetry once for all current
and future synonyms; the table stays human-readable. Also de-dupes 'trade-offs' from
considered_options so it no longer shadows risks once both normalize to 'trade offs'.
Insertion order preserved → first-match-wins + exact-then-prefix precedence byte-
identical for already-normalized synonyms; only the 9 dead synonyms gain matching.
Updates the 7 characterization tests to the corrected behavior and adds a
reachability+no-collision invariant test guarding the whole class against regression.
Note: the audit's M7 premise (trade-offs *misclassified into considered_options*)
was factually wrong — it was unmapped, and tested as such. adr-parser is CLI-only
(ADR-1372 T2), so the behavior change cannot reach in-process gates.
Claude-Session: https://claude.ai/code/session_01R88n7Q54bAaVHFkDbbH1yz
* chore(changeset): Fixed fragment for #1536 (adr-parser punctuated synonyms)
Claude-Session: https://claude.ai/code/session_01R88n7Q54bAaVHFkDbbH1yz
---------
Co-authored-by: Tom Boucher <trekkie@nomorestars.com>
* fix: prototype-pollution guard in _deepMergeConfig (audit M4)
The root↔workstream config merge iterated Object.keys(overlay) with no
__proto__/constructor/prototype guard, while four sibling paths in the same
file (lines ~315/319/331/341/549) guard them. A workstream/root config.json
with {"__proto__": {...}} could pollute the merged object's prototype chain
and spoof unset config flags (per-object, not global Object.prototype).
Adds the same three-key continue guard at the top of the overlay loop plus a
regression test for __proto__/constructor/prototype overlay keys.
Closes a gap missed by the closed config proto-pollution hardening
(#751/#1406/#663).
Claude-Session: https://claude.ai/code/session_01R88n7Q54bAaVHFkDbbH1yz
* chore(changeset): Fixed fragment for #1534 (config proto-pollution guard)
Claude-Session: https://claude.ai/code/session_01R88n7Q54bAaVHFkDbbH1yz
---------
Co-authored-by: Tom Boucher <trekkie@nomorestars.com>
* fix(pr-branch): handle sub_repos from config with git -C (#666)
Adds a `handle_sub_repos` step between `detect_state` and
`analyze_commits`. When `planning.sub_repos` is set in config, the
workflow now:
- Reads sub-repo paths via `gsd_run query config-get sub_repos`
- Skips the step entirely when the list is empty/null/[]
- Scans each repo with `git -C "$REPO" status --porcelain`
- Offers the user all/select/skip choices
- For selected repos: creates a PR branch, commits all staged/unstaged
changes, pushes, and opens a companion PR via `gh pr create`
All git commands use `git -C "$REPO"` — never `cd "$REPO"` — because
shell state does not persist between agent-executed commands.
Closes#666
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* chore: update changeset pr number to 667
* fix(pr-branch): address maintainer review — correct seam, behavioral tests, robustness
Resolves all three blockers and seven robustness issues raised in PR #667 review:
Blockers:
- Use `planning.sub_repos` (not top-level `sub_repos`) so config-get actually resolves
- Replace prose grep test with behavioral fixture tests using runGsdTools + local bare repo
- Extract sub-repo git work into new `cmdPrSubrepo` seam in src/commands.cts;
never uses git add -A — stages explicit files only (universal-anti-patterns.md:44)
Robustness:
- Dirty-repo list persisted via mktemp/cat, not bash arrays (cross-block safe)
- Branch name embeds repo slug (${CURRENT_BRANCH}-${REPO_SAFE}-pr) to avoid collision
- push --set-upstream so gh pr create finds the branch
- Sub-repo base branch resolved via ls-remote with fallback to repo's default branch
- Remote slug parsed with /github\.com[:/]/ (handles SSH + HTTPS + .git-less URLs)
- rollback() cleans up branch on any mid-sequence failure
- node -e replaces jq (always available, no undeclared hard dep)
Refs: #666
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(pr-branch): security guard, push timeout, rollback fix, porcelain fix
Security (Blocker 1):
- Use security.cjs validatePath() in cmdPrSubrepo for symlink-safe workspace
containment check — rejects ../escape, absolute paths, and symlink traversal
- Add negative regression test: '../escape' repo path must be rejected
Robustness:
- Push uses timeout: 60_000 ms (network op needs more than the 10 s default)
- Capture prevBranchName before checkout -b so rollback uses explicit name
instead of git checkout - (fails on fresh single-branch repos)
- Porcelain path parse: line.trimStart().slice(2).trim() handles all XY
combinations and the execGit global-trim edge case uniformly
Tests: 17/17 pass, lint: 0 errors
Refs: #666
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(pr-branch): move regression tests to commands.test.cjs, add core.quotePath=false
- Move cmdPrSubrepo behavioral + workflow source-invariant tests from
standalone bug-666-*.test.cjs into tests/commands.test.cjs under
describe('pr-subrepo') per TESTING-SUITES.md policy (no new bug-* files).
Adds allow-test-rule: source-text-is-the-product see #666 for the
workflow-source-invariant suite.
- Add -c core.quotePath=false to git status --porcelain call so non-ASCII
filenames (e.g. café) are not C-escaped, keeping slice(2) parse correct.
* fix(pr-branch): remove obsolete regression tests for sub-repos handling
* fix(pr-branch): update workflow-size-baseline, add dirty-scan timeout
- Regenerate tests/workflow-size-baseline.json for pr-branch.md growth
(+handle_sub_repos step, +timeout addition).
- Add { timeout: 10_000 } to the execFileSync git status --porcelain
call in the handle_sub_repos dirty-scan (repo convention: every git
subprocess is bounded, never hangs).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* chore: regenerate INVENTORY-MANIFEST after rebase onto next
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(#666): handle rename staging and split changedFiles from filesToStage
For git mv renames, the old path no longer exists in the worktree after
the move — staging it with git add fails. Split parsing into changedFiles
(both paths, for result.files) and filesToStage (new path only for
renames; old is already staged by git mv). Also adds porcelain tests
for staged renames, non-ASCII filenames, and a fast-check property test.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(#666): rollback on push failure in cmdPrSubrepo
If push fails the branch only exists locally; rollback cleans it up so
the sub-repo is not left in a half-committed state.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(#666): do not rollback after commit on push failure; add push-fail regression test
Post-commit push failures are network/auth/policy issues — the user's work
is already committed on the local branch. Calling rollback() at that point
force-deletes the only ref holding the commit (data loss). Leave the branch
in place and emit a retry instruction instead.
Adds a regression test (pre-receive hook that rejects all pushes) asserting
the branch and commit survive a push rejection so the failure path stays
covered going forward.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* chore: regenerate INVENTORY-MANIFEST after rebase onto next
Rebased onto current next (#1267 retired core.cjs). Stale tsbuildinfo and
a leftover bin/lib/core.cjs build artifact were masking the drift — wiped
both, rebuilt clean, and regenerated the manifest. gen-inventory-manifest
--check now exits 0.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(#666): validate sub-repo paths before git invocation in pr-branch.md
The handle_sub_repos workflow ran git -C on raw planning.sub_repos config
values at two points before the pr-subrepo seam's validatePath guard ever
ran: the dirty-scan detection (git status) and the base-branch resolution
(git ls-remote / remote show). A traversal entry could point git outside
the workspace; an embedded newline could inject a spurious record into
the newline-joined dirty-file output and into the shell-interpolated
commit message.
Adds a containment check + character allowlist to the dirty-scan node
script (reject before any execFileSync), and a defense-in-depth shell
case guard on the same value before the second, independent git -C
invocation in the base-branch resolution block.
Adds a behavioral test that extracts and executes the actual shipped
node script from pr-branch.md (not a mirror) against a real traversal
target and an embedded-newline entry, asserting neither reaches git or
the dirty-file output.
Also updates the stale cmdPrSubrepo doc comment: push failures no longer
delete the branch (see prior commit), only stage/commit failures do.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* test(#666): make sub-repo traversal scan test genuinely fail-first
The outside repo's only change was an untracked file, which the ?? filter
excludes — so the repo looked clean even with the guard removed, making the
traversal assertion vacuous (it passed against a neutered guard). Commit the
file first, then modify it, so the outside repo has a tracked dirty change:
without the path guard it WOULD be reported dirty, so the test now fails-first.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(#666): symlink-safe (realpath) sub-repo containment in pr-branch.md
Finding A from re-review: the workflow guard used path.resolve, which only
normalizes '..' textually and does not follow symlinks — so an in-tree symlink
whose name has no '..' or '/' (e.g. "evil" -> /outside) passed both the charset
filter and the resolve+startsWith check, letting git status / ls-remote /
remote show run against a directory outside the workspace. The pr-subrepo seam
already used fs.realpathSync (validatePath); this brings the workflow layer to
parity.
- dirty-scan: realpathSync the root once, and realpathSync each candidate before
the containment check; skip on throw.
- base-branch resolution: replace the weak `case *..*|/*` guard with a realpath
containment check that yields a validated absolute SUB_REPO_DIR, and run git -C
against that instead of re-concatenating $ROOT/$REPO_REL.
- security test: add a symlink-escape entry and a positive control (legit in-root
backend must still be reported). Confirmed fails-first — regressing the scan to
path.resolve makes the symlink case leak.
Also fixes a misleading-fallback minor: the workflow now checks the seam's exit
status and skips the companion-PR step on failure, instead of printing
"branch pushed, open PR manually" after a real stage/commit/push failure.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(#666): harden pr-branch sub-repo flow against round-12 edge cases
Pre-emptive hardening of the workflow changes from the symlink fix:
- continue-outside-loop: the "skip companion PR on seam failure" block used a
bash `continue`, but the per-sub-repo iteration is prose-driven (the agent
loops, not a literal `for`), so `continue` would warn and no-op. Reframed as
prose-gated control flow keyed on $SUBREPO_EXIT — no bash loop assumption.
- Windows portability: the new symlink security case now degrades gracefully
(try/catch around fs.symlinkSync; skip just the symlink assertion when symlink
creation lacks privileges) so it doesn't hard-fail on Windows CI.
Verified: seam exits 1 on error / 0 on success (error() → process.exit(1),
propagated through the shim), so the $SUBREPO_EXIT check is meaningful; bash -n
clean on the touched blocks; commands 156/156; lint:ci green; manifest in sync.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---------
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-authored-by: Tom Boucher <trekkie@nomorestars.com>
Follow-up hygiene from the #1507 epic review (release blocker fixed in #1537):
- path-replacement.test.cjs: delete the hand-reimplemented computePathPrefix copy
(ADR-1508 said to); route all cases (incl. Windows + outside-home) through the
real _computePathPrefix so the test can no longer drift from the function.
- enh-1511: add an isWindowsHost no-op tripwire characterization test.
- enh-1511: add a deterministic (fs-method monkeypatch, root/OS-independent)
error-path test for applyRuntimeContentRewritesForCommandsInPlace — asserts the
temp dir is rm'd on a read failure with no orphaned gsd-cmd-rewrites-* leak.
- runtime-artifact-conversion.cts: @internal note on rewriteStagedCommandBodies
(deep-seam companion to rewriteStagedSkillBodies; no production caller today).
- ADR-1508 + CONTEXT.md: qualify the "single owner" claim with the deliberate
opencode/kilo applyOpencodeFamilyPathPrefix pre-conversion carve-out (#784).
No user-facing behavior change (tests + internal docs + one code comment).
Refs #1507.
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Adopt maintainer-recommended option (a): keep the convertedAgentsKind /
stageAgentsForRuntimeWithConverter scope-threading plumbing, but DEFER the
8 runtimes' capability.json `agents`-kind declarations to a follow-up that
first ships the ADR-1235 §0 byte-for-byte parity harness.
The declarations were a live regression: the second `layout.kinds` consumer,
applySurface / `/gsd:surface` / `--materialize` (src/surface.cts), does not
mirror the legacy agent pipeline (copilot `.agent.md` rename, path-prefix
rewrite + attribution, stale cleanup), so a `/gsd:surface` toggle deleted
installed copilot `gsd-*.agent.md` and path-unrewrote the other 7 runtimes.
trek-e + davesienkowski both flagged this.
- Revert the agents-kind entries from the 8 capability.json files and
regenerate capability-registry.cjs (now matches next; 0 converted agents
kinds declared).
- Revert the declaration-driven kind-count test bumps
(runtime-artifact-layout, descriptor-drive, bug-782, enh-789, enh-790).
- Keep the synthetic-descriptor seam tests for convertedAgentsKind dispatch;
add a synthetic scope-threading test so the kept isGlobal plumbing stays
covered without depending on real declarations.
- Update the convertedAgentsKind doc comment to state declarations are
deferred pending the ADR-1235 §0 parity harness.
- Move ADR-1235 to Accepted.
- Reword the changeset to plumbing-only (docs-exempt now honest: no runtime
declares the kind, legacy loop authoritative, installed output unchanged).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The descriptor-driven install path (resolveRuntimeArtifactLayout) installed no
agents for copilot/antigravity/cursor/windsurf/augment/trae/codebuddy/cline —
their per-runtime agent conversion ran only via the legacy bin/install.js loop.
This wires each runtime's agent converter into the descriptor so the new path
applies per-runtime conversion (follow-up to #1099; ADR-1235 cutover).
- capabilities/<rt>/capability.json: declare an `agents` kind with the runtime's
converter (global+local; cline global-only). Regenerated capability-registry.cjs.
- convertedAgentsKind threads install scope -> isGlobal so the scope-aware
copilot/antigravity converters choose global vs workspace-relative paths; the
six single-arg converters ignore the extra arg. stageAgentsForRuntimeWithConverter
passes isGlobal to the converter.
- Tests: feat-1173 gains a real-registry block asserting each runtime's descriptor
applies the correct converter (== conv(src, isGlobal), != raw copy) with scope
threading (fails-first on pristine next). The ADR-857 equivalence golden +
per-runtime kind-count assertions now include the agents kind, each annotated as
an intentional #1173 change.
Scope: this wires the per-runtime CONVERTER. The remaining byte-parity behaviors of
the legacy loop (copilot `.agent.md` rename, cross-cutting path/attribution rewrites,
config-reading) stay with the legacy loop -- which runs after installRuntimeArtifacts
and is authoritative for the real install -- and are tracked by ADR-1235's later
cutover steps. No user-facing change.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(#1521): resolve own runtime + worktrees-off for all non-Claude installs
Generalizes the Codex-only #1515/#1519 fix to every non-Claude runtime, and
wires it into the real install path (where it was previously dead-on-arrival).
Root causes:
1. The runtime-default stamping lived only in `_applyRuntimeRewrites`, but the
installer emits `gsd-core/workflows/*.md` via `copyWithPathReplacement`, which
never calls it — so a real `--codex`/`--cursor`/etc. install emitted
`--default claude` and worktrees-on. RUNTIME mis-resolved to claude and the
workflow ran executors unisolated against the main checkout. (#1515/#1519 were
also dead-on-arrival in real installs; this repairs them.)
2. Only `case 'codex'` was stamped; every other non-Claude runtime kept the
Claude default.
Fix:
- New `_stampNonClaudeRuntimeDefaults(content, runtime)` (single shared helper)
stamps `--default <runtime>` + `use_worktrees=false` for every `runtime !=
claude`; called from both `_applyRuntimeRewrites` and, crucially,
`copyWithPathReplacement` in bin/install.js (the real workflow emit path).
- Generalize the fail-closed worktree guard `= codex` -> `!= claude` in
execute-phase/quick/diagnose-issues (worktree isolation is Claude-Code-only).
- Flip manager/autonomous inline-vs-background gating to `codex -> background,
everything-else -> inline` (research: only Codex can background-nest the
pipeline's subagents; all others run inline, which they support).
Worktree-capability determination is research-backed (official docs for all 14
non-Claude runtimes: none honor GSD's isolation="worktree" mechanism, only Codex
background-nests). New end-to-end real-install test asserts the EMITTED workflow
is stamped — the regression guard that would have caught the dead-on-arrival bug.
Closes#1521
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013vX5eUtWa2wsZEyeMf5i3r
* chore(#1521): backfill changeset PR number (#1537)
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013vX5eUtWa2wsZEyeMf5i3r
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Root cause: in acquireStateLock, once openSync(O_CREAT|O_EXCL) created the
lock file, the subsequent writeSync(pid)/closeSync were unguarded. A
recoverable errno (EAGAIN etc., in ACQUIRE_LOCK_RETRY_ERRNOS) made the catch
do checkBudgetAndSleep + continue WITHOUT closing the fd or unlinking the
just-created empty lock — leaking a descriptor every occurrence and stranding
a content-less lock (the #500/#905/#1230 STATE.md write-corruption family).
Fix: wrap writeSync/closeSync in an inner try that guardedly closeSync(fd) +
unlinkSync(lockPath) then re-throws to the existing outer catch (DRY errno
classification). Recoverable errno retries from a clean slate; a FATAL errno
(e.g. ENOSPC, not recoverable) still propagates after cleanup — not masked.
Mirrors the already-shipped capability-lock.cts:415-425 pattern.
Extends the M8 test seam with a one-shot simulateWriteError errno + an
onLoopIteration snapshot hook so the orphan-before-retry is deterministically
observable. New tests prove RED (orphan stranded / fatal leaves orphan)
before the cleanup and GREEN after.
Source of truth src/state.cts (ADR-457); bin/lib/state.cjs is generated.
Claude-Session: https://claude.ai/code/session_01R88n7Q54bAaVHFkDbbH1yz
Root cause: writeStateMd computed its frontmatter disk scan
(syncStateFrontmatter — the READ half of a read-modify-write) BEFORE
acquireStateLock, leaving a TOCTOU window. A concurrent writer that
committed a new PLAN/SUMMARY between our scan and our lock made writeStateMd
stamp stale progress counts (lost update — the #500/#905/#1230 family). The
atomic sibling readModifyWriteStateMd already scans inside its lock.
Fix: move _diskScanCache.delete + syncStateFrontmatter inside the
acquireStateLock-held try, before platformWriteSync. Byte-for-behaviour
identical for single-threaded callers — only the concurrent-writer window
closes. Adds an afterAcquire test seam (mirrors the M1 _setLockProbes seam)
to make the window deterministic; new test proves RED (stale count) before
the reorder and GREEN after.
Source of truth src/state.cts (ADR-457); bin/lib/state.cjs is generated.
Claude-Session: https://claude.ai/code/session_01R88n7Q54bAaVHFkDbbH1yz
The M1/M2 lock fix (4903ee04) was asymmetric: acquireStateLock got a 60s deadman
ceiling (recovers a lock once its age crosses an absolute bound ABOVE the wait
budget) but withPlanningLock did not. The .lock body carries no startTime, so
_planningHolderVerifiedLive can only check pid liveness — it cannot detect pid
reuse. A false-alive holder (original holder crashed, pid recycled by an unrelated
live process) would therefore make withPlanningLock throw on every call with no
self-heal until the reused pid happens to die.
Mirror acquireStateLock: in the EEXIST branch, steal a verified-live holder anyway
once the lock ages past deadmanCeilingMs (60000 > lockTimeout 10000). mtime age is
measured from lock creation, so a stuck lock self-heals on a subsequent call.
Adds a clock+probe-seam regression test (false-alive holder past the ceiling IS
stolen). planning-workspace 13/13, clock-seam 34/34, locking suites green; lint clean.
Claude-Session: https://claude.ai/code/session_01R88n7Q54bAaVHFkDbbH1yz
The STATE.md write lock (acquireStateLock) and the .planning/ workspace lock
(withPlanningLock) stole contended locks on mtime age alone with no
process.kill(pid,0) liveness check, and mis-ordered stale-vs-wait so a
live-but-slow holder could be robbed mid-critical-section.
M1 (lost update / STATE.md corruption): a live writer whose critical section
ran past the stale threshold aged out and a waiter unlinked its lock and
acquired -> two writers in STATE.md's read-modify-write window. mtime is a
leaky proxy for "holder is alive"; it leaks under exactly the slow-holder
condition the lock guards against.
M2 (uncaught EEXIST): withPlanningLock's timeout fallback unconditionally
unlinked whatever lock existed (even a live holder's) and re-acquired OUTSIDE
any try -- a concurrent re-create raced a raw EEXIST out of the helper.
Fix backports capability-lock.cts's liveness gate (process.kill(pid,0) via a
_setLockProbes/_resetLockProbes test seam):
- acquireStateLock: steal when holder pid is DEAD (any age) OR age exceeds a
deadman ceiling (60000ms, ABOVE maxWaitMs=30000) so a verified-live holder is
never stolen within budget; garbage/legacy bodies stay recoverable.
- withPlanningLock: same gate in the EEXIST path (dead stolen promptly, live
waited on); removed the unconditional force-steal -> clear timeout throw,
which also closes M2 (no re-acquire outside try).
Uncontended path unchanged byte-for-behaviour; realClock + real process.kill
remain the defaults. Pid-reuse residual fails safe (waits/times out, never
corrupts) and recovers at the deadman ceiling.
Tests: TDD red->green via the clock + new pid-liveness probe seams (no
wall-clock; #453 deleted the race tests). 8 new behavioural tests across
tests/clock-seam.test.cjs and tests/planning-workspace.test.cjs; the prior
withPlanningLock timeout test rewritten to pin the no-force-steal contract.
Claude-Session: https://claude.ai/code/session_01R88n7Q54bAaVHFkDbbH1yz
* fix(#1515): make Codex installs resolve their own runtime and fail closed on worktrees
A Codex install with a runtime-neutral .planning/config.json resolved
RUNTIME=claude and enabled git worktree isolation, which Codex's
spawn_agent cannot honor. Two root causes:
1. Workflows read `config-get runtime` / `config-get workflow.use_worktrees`
without `--raw`, so config-get's JSON-quoted output ("codex") was captured
verbatim into the bash var and broke every `[ "$RUNTIME" = ... ]` check —
the Codex fail-closed guard was dead even when runtime:codex was explicit,
and Claude's own worktree degrade-check was dead too. Add `--raw` to those
reads across execute-phase, autonomous, manager, diagnose-issues, quick.
2. The conversion engine emitted `--default claude` for every runtime. Stamp
the codex-emitted workflows to `--default codex` (runtime) and
`--default false` (use_worktrees) in _applyRuntimeRewrites case 'codex', so
a neutral config on a Codex install resolves runtime=codex / worktrees off.
Also extend the Codex fail-closed worktree guard to quick.md and
diagnose-issues.md (they spawned isolation="worktree" with no runtime guard).
Regression test asserts source<->engine parity across all five workflows
(DEFECT.GENERATIVE-FIX) plus fast-check property coverage of the stamping.
Closes#1515
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013vX5eUtWa2wsZEyeMf5i3r
* chore(#1515): backfill changeset PR number (#1519)
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013vX5eUtWa2wsZEyeMf5i3r
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The #1279 node-test machine-proof confirmed a known-bad subject drives the
negative test RED, but could not distinguish a genuine content-violation from a
deceptive test that reds merely because GSD_PROHIB_SUBJECT is set. Add an
optional fifth flat scalar `check_clean_fixture` (-> CheckDescriptor.cleanFixture)
threading a KNOWN-CLEAN control subject through projectProhibitions +
descriptorFromProjection. When present, the prover also runs the check against
the clean subject and requires GREEN, so fail-first is proven only when the check
is RED on the violation AND GREEN on the clean subject (content-dependent).
Opt-in and additive: absent a clean fixture the prover behaves exactly as
post-#1314 (no control, documented residual), preserving the zero-authoring
compose path; the lint-rule kind needs no analog (its subject IS the linted
file, no env indirection). Coverage: RED-first deceptive case, positive,
missing-clean fail-closed, round-trip read-back/emit, fast-check property
extended to the 5th scalar, and an end-to-end COMPOSE capstone (honest vs
deceptive). Docs: ADR-550 dated addendum, prohibition-probe reference,
spec-phase + verify-phase workflows.
Closes#1346
Claude-Session: https://claude.ai/code/session_01GsPRb8zvpcT7Eat6vZw8PX
* refactor(#1511): move content-rewrite engine to conversion module, delete the install.js relay
Phase 2 of epic #1507 (ADR-1508). Behavior-preserving: makes the Runtime
Artifact Conversion Module the single owner of per-runtime content rewriting and
removes the last upward .cts -> bin/install.js dependency.
- src/runtime-artifact-conversion.cts now owns the engine (_applyRuntimeRewrites,
5-arg with INJECTED attribution), the staged-content walkers
(applyRuntimeContentRewritesInPlace / ...ForCommandsInPlace), computePathPrefix
(private, exported as _computePathPrefix for tests), and the deep public seam
rewriteStagedSkillBodies / rewriteStagedCommandBodies({runtime, configDir,
scope, homedir?, platform?, resolveAttribution?}).
- src/surface.cts:applySurface calls rewriteStagedSkillBodies directly (no
resolveAttribution -> undefined). Co-Authored-By is absent from ALL rewritten
content, so processAttribution is vacuous there and undefined is provably
behavior-identical. surface no longer imports getInstallExports.
- src/runtime-artifact-layout.cts: deleted getInstallExports / loadInstallExports
/ InstallExports + the GSD_TEST_MODE require('bin/install.js') relay.
- bin/install.js: binds computePathPrefix / the two walkers / _applyRuntimeRewrites
from the conversion module (single implementation, exports preserved for Hyrum);
install callsites pass getCommitAttribution(runtime) as the injected attribution.
getCommitAttribution stays here (impure install-time config I/O).
- DEFECT.GENERATIVE-FIX guard: tests assert install.X === conversion.X reference
identity for computePathPrefix + both walkers (no drift).
New tests/enh-1511-*.test.cjs (engine, attribution injection, deep seam, prefix,
layout-no-relay guard, reference-identity). 316 affected-suite tests green; lint clean.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0187qgypdy1wkWRpdaf2hRuD
* test(#1511): make rewrite-engine path assertions Windows-robust
The deep seam normalizes paths as path.resolve(configDir).replace(/\\/g,'/')
and compares homedir().replace(/\\/g,'/'). Three assertions in the new test
rebuilt expected paths without that normalization, so they passed on Mac/Linux
but failed on Windows CI (PR #1513):
- two absolute-branch asserts rebuilt resolvedTarget via path.resolve(configDir)
without the backslash→slash replace → mismatch on Windows.
- the $HOME-branch test fed a POSIX-literal /home/testuser, which Windows
path.resolve re-roots onto the cwd drive (D:/home/...), so the
resolvedTarget.startsWith(homeDir) check failed and the $HOME shorthand was
never produced.
Fix is test-only (engine unchanged, still behavior-preserving): mirror the
engine's .replace(/\\/g,'/') in the two absolute-branch asserts, and use a real
absolute path (path.resolve(os.tmpdir(), ...)) + platform: process.platform for
the $HOME-branch test so the comparison holds on all platforms.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0187qgypdy1wkWRpdaf2hRuD
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Phase 1 of epic #1507 (ADR-1508): behavior-preserving relocation of the pure
rewrite-engine helpers out of the hand-authored installer so the conversion
module can own them without importing bin/install.js.
- getDirName -> src/runtime-name-policy.cts (pure runtime->dir-name switch).
- processAttribution -> src/runtime-artifact-conversion.cts (pure Co-Authored-By
content transform).
- bin/install.js imports both back via destructure/binding and re-exports
getDirName unchanged (Hyrum: install.test.cjs + runtime install tests import
getDirName from bin/install.js).
Two refinements to ADR-1508's Phase 1 (verified against the source):
- getCommitAttribution STAYS in bin/install.js: it is impure install-time
config I/O (reads runtime settings.json, uses install-time config-dir state +
attributionCache), not a content transform. Phase 2 will inject the resolved
attribution into the engine rather than move this function.
- The convertClaudeToAugmentMarkdown dedup is deferred to Phase 2's cleanup: the
local copy is entangled with a converter cluster (convertSlashCommandsTo
AugmentSkillMentions is used only by it; the family is partly dead-local via
the ...runtimeArtifactConversion export spread), deduping only augment would be
arbitrary, and it is not required to unblock Phase 2 (the engine will call the
conversion module's own copy when it moves).
New tests/enh-1510-*.test.cjs: getDirName at its new home (all runtimes +
fallback + install.js re-export identity) and processAttribution
(null/undefined/string/$-escape/CRLF/global). 487 affected-suite tests green.
Claude-Session: https://claude.ai/code/session_0187qgypdy1wkWRpdaf2hRuD
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
cmdAgentSkills' plain (non-JSON) path wrote the <agent_skills> block with
process.stdout.write then immediately called process.exit(0). stdout.write is
async on pipes/files, so on Windows the process tore down before Node flushed
the buffer — the workflows' `$(gsd_run query agent-skills <type>)` capture
received 0 bytes and every ${AGENT_SKILLS_*} substitution expanded empty,
silently dropping configured per-agent skills.
Route the plain path through the existing synchronous output() helper
(writeAllSync, src/io.cts) + return — the same flush-safe mechanism the --json
branch already uses — instead of write + process.exit(0).
Adds a #1400 regression block to tests/agent-skills.test.cjs that captures
stdout via a real file descriptor and asserts the block is non-empty and
byte-identical to the --json .block.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
