* 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>
walk() collected dirents by extension only, then readFileSync'd each — so a
broken symlink named *.md (e.g. the gitignored CLAUDE.md worktree symlink whose
target is absent in a gsd-test Docker copy) crashed the #1073 guard with ENOENT.
It passed on a developer Mac (the symlink resolves) and in GitHub CI (CLAUDE.md
is gitignored/absent), failing only on the gsd-test-docker-from-worktree path —
a real error hiding behind the local environment, not a flake.
Guard the collection with entry.isFile() so symlinks (and other non-regular
dirents) are skipped deterministically on every platform; gitignored files are
not shipped repo text, so excluding CLAUDE.md is correct. Add a deterministic
regression test (dangling *.md symlink fixture, Windows-symlink-guarded with a
genuine t.skip) asserting walk() excludes the broken symlink and the read loop
never throws ENOENT.
Closes#1545.
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
lint-allow-test-rule-refs (ADR-456) flagged the two architectural-invariant
exemptions as novel offenders lacking a #NNN reference, failing lint-tests. Add
(see #1531) to both annotation lines; lint:ci now green.
Claude-Session: https://claude.ai/code/session_01R88n7Q54bAaVHFkDbbH1yz
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>
Phase 0 of epic #1507. Records the decision to make the Runtime Artifact
Conversion Module the single owner of per-runtime content rewriting, flip the
dependency direction to installer/layout -> conversion, and close the
surface.cts -> bin/install.js getInstallExports relay. Doc-only.
Resolves ADR-3660 Initial-Scope deferral; distinct from epic #1258.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0187qgypdy1wkWRpdaf2hRuD
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>
Proactive checkpoint guard fires at each wave boundary before spawning agents.
Self-assesses context pressure against context-budget.md degradation tiers and
warns (warn, default) or auto-invokes /gsd:pause-work (auto) when POOR tier
(70%+) is detected. Config key validated; defaults to \"warn\".
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
loadConfig() returns a flattened object with no nested `workflow` key, so
config?.workflow was always undefined, making drift_action permanently 'warn'
and drift_threshold permanently 3 regardless of .planning/config.json. Fixes
by reading the raw config.json directly (matching the pattern in
check-command-router.cts:readWorkflowConfig). Adds two behavioral regression
tests that fail under the old code and pass under the fix.
Closes#1493
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
product-name-purity (#1777) rejects 'Gemini (…)' parentheticals that render
verbatim into CHANGELOG.md. Reword to 'Gemini and Gemini-backed Antigravity';
no behavior change. (Reword was left uncommitted in the prior rebase push.)
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Addresses review: the regression unit test required the tsc build artifact
(gsd-core/bin/lib/runtime-artifact-conversion.cjs) instead of the live install
path. Destructure convertGeminiToolName from the existing ../bin/install.js
import so a stale build can't false-green while the live copy is broken, and
assert the full AskUserQuestion/ask_user exclusion group.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
convertGeminiToolName lowercased any unmapped Claude tool, so Skill and
SlashCommand became an invalid `skill`/`slashcommand` tool name. Gemini CLI
has no such built-in tool, so frontmatter validation failed (tools.N: Invalid
tool name) and aborted the entire agent load — 22 of 34 GSD agents were dead
on Gemini.
Add Skill and SlashCommand to the same `return null` exclusion branch that
already handles AskUserQuestion, in both the canonical src converter and the
hand-maintained bin/install.js copy that runs on the live --gemini install
path. Antigravity reuses this converter (it runs on the Gemini backend) and is
intentionally covered by the same exclusion — it surfaces GSD skills via the
skill surface (SKILL.md), not the agent tools: allowlist — locked by an added
Antigravity regression test.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(#1498): regenerate capability-registry.cjs in npm version lifecycle hook
The version npm lifecycle script now runs gen-capability-registry.cjs --write
after sync-manifest-versions.cjs stamps all capability/*.json version fields.
Without this, every npm version X.Y.Z call left capability-registry.cjs stale
(capability JSONs got the new version string, but the registry still embedded
the old ones), causing gen-capability-registry.cjs --check to fail in the RC
test suite.
- package.json version script: add gen-capability-registry --write + git add
- tests/issue-844-manifest-version-sync.test.cjs: regression guard (describe F)
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix: backfill changeset pr: 1499
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
---------
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(#1369): refresh wave manifest and re-check base before each wave in execute-phase
Two compounding issues caused wave N+1 worktrees to fork from the stale
pre-wave-N commit, immediately tripping the worktree_branch_check FATAL
guard in every executor:
1. worktree.base-check auto-degrade only ran once at initialize time.
After wave N merges advanced orchestrator HEAD past origin/HEAD, new
worktrees were still forked from origin/HEAD (Claude Code "fresh" base).
2. WAVE_WORKTREE_MANIFEST was never unset between waves, so wave N+1
reused the consumed wave-N manifest file, which would have blocked the
step 5.5 manifest guard (#3384) on subsequent waves.
Fix: add two safeguards in execute-phase.md —
- Step 0.5 (start of each wave): re-runs worktree.base-check; auto-degrades
USE_WORKTREES=false for that wave when HEAD has diverged from origin/HEAD.
- Step 7c (end of each wave): unsets WAVE_WORKTREE_MANIFEST so wave N+1
creates a fresh per-wave manifest; re-asserts worktree.set-baseref
(idempotent) and re-evaluates base degradation after wave merges land.
17 regression tests added in tests/bug-1369-wave-stale-base.test.cjs.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(#1369): rename test to fix-NNN convention; update workflow size baseline
Rename tests/bug-1369-wave-stale-base.test.cjs → tests/fix-1369-wave-stale-base.test.cjs
to satisfy the lint-regression-test-names gate (new files cannot use bug-NNN prefix).
Update tests/workflow-size-baseline.json for execute-phase.md: 93157 → 97393
(LF-normalized byte count after adding step 0.5 inter-wave base re-check and
step 7c between-wave manifest reset).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(#1369): add issue reference to allow-test-rule comment
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(#1369): extract new execute-phase steps to references; satisfy ADR-857 cap
Step 0.5 (inter-wave worktree base re-check) and steps 7b–7c (pre-wave
dependency check + between-wave manifest reset/base refresh) added by this
PR grew execute-phase.md to 97393 bytes, violating the ADR-857 phase-6
architectural mandate that host-loop bodies remain strictly below the
pre-phase-6 baseline of 93166 bytes.
Extract both new blocks into dedicated reference files:
- gsd-core/references/execute-phase-wave-guard.md (step 0.5)
- gsd-core/references/execute-phase-between-wave-reset.md (steps 7b + 7c)
Replace inline prose with @-reference pointers. File now measures 92851
bytes (LF-normalized), satisfying the ADR-857 capstone conformance gate.
Also update tests/workflow-size-baseline.json to 92851 and add both new
reference files to docs/INVENTORY-MANIFEST.json.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(#1369): update regression tests to read from extracted reference files
Steps 0.5 and 7b+7c were moved to reference files to satisfy the ADR-857
size cap on execute-phase.md. Tests now check @-reference pointers in the
workflow for ordering and read content assertions from the reference files.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
---------
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
* test(#1464): add behavioral manifest-validation test for capability tutorial docs
Extracts JSON capability manifests from tutorial/reference docs and validates
them through the real validateCapability — closing the test gap that let
issue #1464's broken tutorial manifest (step missing ref) pass undetected.
Adds fix-1464-docs-manifest-validation.test.cjs with:
- Suite 1: build/install/reference docs' complete manifests pass validateCapability
- Suite 2: adversarial fixtures prove the original #1464 bug shapes are caught
(step without ref → "steps[0].ref must be an object…"; id/folderId mismatch)
- Suite 3: extractManifests helper unit tests (complete vs. partial block filtering)
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* test(#1496): allowlist docs module for 3-file test cluster
docs-parity-live-registry, docs-update, and the new
fix-1464-docs-manifest-validation sit on the same docs production
module; add the docs entry to lint-test-file-count.allowlist.json
so the novel-offender CI gate passes.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
---------
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(#1453): clean up stale get-shit-done paths in Codex skill mirror on upgrade
Extends planLegacyCleanup in gsd-core/bin/lib/legacy-cleanup.cjs to scan
skills/gsd-* subdirectories for .md files that still embed the pre-rename
get-shit-done/ path (e.g. ~/.agents/skills/gsd-docs-update/SKILL.md).
These stale copies are removed by cleanupLegacyGsdCc during the next
install/upgrade so Codex can no longer discover and select them.
Adds 5 regression tests to tests/issue-607-legacy-cleanup.test.cjs covering
the stale path detection, non-flagging of fresh skills and user-owned dirs,
and the end-to-end ~/.agents/skills scenario from the issue.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(#1453): add gsd-allow-legacy-name markers to intentional legacy name references
Comments and test descriptions in legacy-cleanup.cjs and its test file
legitimately cite the old 'get-shit-done' directory name to explain what
the cleanup logic removes. Add the lint-exemption marker to each line so
lint-legacy-dir-name passes.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
---------
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
