Files
msd-core/docs/adr
0xdhx 0396d9cab1 enhance(#2483): stop the claude reviewer lane from inheriting CLAUDE.md + auto-memory (#2493)
* enhance(#2483): env-guard the claude reviewer leg against CLAUDE.md injection

The claude reviewer in workflows/review.md was a bare headless `claude -p`
spawn run from the project cwd, so it inherited the invoking user's global
CLAUDE.md, the project CLAUDE.md, and Claude Code auto-memory.

That made it the only reviewer leg seeing anything beyond the prompt file.
gather_context assembles PROJECT.md, the roadmap section, every PLAN file,
CONTEXT.md, RESEARCH.md and REQUIREMENTS.md into the prompt before any
reviewer runs; the gemini leg receives only that prompt and the codex leg
runs --ephemeral. Beyond the measured ~4k tokens/spawn, the asymmetry cuts
at the workflow's own premise: "independent review" meant something
different for the claude leg than for the other two.

Guard both dispatch lines with a per-invocation
`env CLAUDE_CODE_DISABLE_CLAUDE_MDS=1`. `env`, never `export` — the flag
must not leak into the orchestrating session (which may itself be Claude
Code on the SELF_CLI="auto" path) or into any later spawn.

review.md is the only claude -p call site in the installed tree, so this is
two lines on one surface. The self-skip logic is untouched.

* enhance(#2483): fix CRLF-fragile split and regenerate workflow baselines

Two CI failures from the first push, both mine:

1. lint-tests: the new regression test split readFileSync content on a
   literal "\n". On a Windows git-autocrlf checkout that leaves a trailing
   "\r" on every line (local/no-crlf-fragile-split). Use .split(/\r?\n/).

2. golden-install-parity / workflow-size-budget / workflow-compat: editing
   gsd-core/workflows/review.md changes its content hash and byte size, and
   both are pinned in committed baselines. Regenerated via the repo's own
   generators (npm run size:baseline, npm run gen:golden).

The regenerated diffs are review.md-only: exactly one hash line per
golden-install-parity fixture and one size entry in workflow-size-baseline
— no unrelated drift swept in.

Full suite now green locally: 2113 pass, 0 fail, 3 skipped (run with HOME
and CLAUDE_CONFIG_DIR overridden to throwaway dirs; live profile verified
untouched afterward).

* enhance(#2483): adapt guard-test matcher to the effort-args dispatch reshape

The effortSurface wiring (#2481) reshaped the bare-model dispatch to
`claude $CLAUDE_EFFORT_ARGS -p -`; the invocation matcher's dash-first
form could no longer see it, and the count assertion failed exactly as
designed. The matcher now tolerates variable expansions between `claude`
and its first literal flag. Negative-controlled both ways: a stripped
guard and a deleted dispatch line each still fail.

* enhance(#2483): also guard the claude leg against auto-memory injection

CLAUDE_CODE_DISABLE_CLAUDE_MDS suppresses CLAUDE.md file loading;
auto-memory is an independently-toggled mechanism with its own flag.
Add CLAUDE_CODE_DISABLE_AUTO_MEMORY=1 to both dispatch lines, correct
the docs/COMMANDS.md and changeset claims that credited the first flag
with covering auto-memory, and extend the regression test to require
both flags on every claude invocation (negative-controlled: 2/4
assertions fail with the new flag removed).

* enhance(#2483): match the claude binary in command position, not argument position

The line-oriented invocation matcher counted any line where the token
`claude` was followed by a flag. #2589 (landed on next as 920a5f3f)
reshaped the effort-args lookup from

  --host claude 2>/dev/null | jq -r '.effort_argv_string // ""'

to

  --host claude --pick effort_argv_string

which put a flag immediately after `claude` and made the config query
read as a third claude dispatch, failing the count assertion.

The defect class is a binary name in *argument* position being read as a
command. Fixed at the class rather than the instance: tokenise the line
and skip any `claude` whose preceding token is a flag. That also covers
the latent sibling one line away in review.md (`command -v claude`),
which escaped today only because its next token is a redirect.

Negative-controlled four ways: stripping CLAUDE_CODE_DISABLE_AUTO_MEMORY=1
fails, stripping the whole env guard fails, adding a genuine third
unguarded dispatch (`timeout 900 claude --output-format text -p -`) still
fails — so the narrowing did not blind the matcher to reshapes, which is
the property the count assertion exists for — and the pre-#2589 jq form of
the lookup still passes, so the matcher is not pinned to today's base.

* enhance(#2483): carry the claude reviewer's memory guard as declared lane data

ADR-2782 Phase 5b replaced the hand-authored per-CLI dispatch legs in
review.md with the declared lane table, so the two `env`-prefixed shell
lines this PR previously added no longer have a surface to live on. The
guard is reimplemented where the lane contract now lives.

`SpawnInvoke` gains an optional `env`, the claude lane declares the pair,
the resolver folds own string-valued entries into `SpawnPlan.env` (absent
or empty resolves to null, so the runner has one shape to test), and the
runner passes it to spawn. Production merges it OVER `process.env` into a
fresh object for that one child, so nothing reaches the orchestrating
session or any other lane in the run.

Declared data rather than a handler (D6): the pairs are static per lane,
which is precisely what the manifest vocabulary is for. The capability
manifest carries the same field, because the lane-fidelity test compares
manifest and descriptor over the union of `invoke`'s keys.

The regression test is rewritten against the resolver and runner rather
than review.md's text. It gains the property the source-text assertions
could only approximate: that `process.env` is never mutated.

Scope boundary, asserted rather than left in prose: `env` is not part of
the trust-disclosure surface, which is safe only while no manifest body
reaches the resolver — the registry's reviewer bodies contribute slugs to
the parity check and execution resolves from `REVIEWER_LANES`. The new
test fails first if that ever changes.

* enhance(#2483): restate the guard's mechanism in the docs and changeset

Both described the fix as two `env`-prefixed dispatch lines, which is the
surface ADR-2782 Phase 5b removed. The user-visible behaviour is
unchanged; the carrier is not, and a changeset that ships a description
of a mechanism the tree does not have is a CHANGELOG entry nobody can
verify against the code.

* enhance(#2483): cover the production spawn wiring end to end

The unit tests stop at the runner's `deps.spawn` seam — every one injects
a spy. Production supplies that seam in `gsd-core/bin/gsd-tools.cjs` as a
hand-written object no test constructs, so the chain could be correct all
the way to `SpawnPlan.env` and the merge could still be wrong or absent
with the suite green. Deleting those four lines was the one mutation that
left every other control silent.

This runs the real `spawnSync` through `gsd-tools review-lane invoke`,
with a `claude` shim on PATH that records the environment it was handed.
It asserts both halves in one test: the pair arrives, and an unrelated
inherited variable survives — a wiring that REPLACED the environment
rather than merging over it would satisfy the first and break every
lane's PATH and HOME.

POSIX-only; mediating a Windows `.cmd` shim is a separate concern the
repo already tests on its own.

Noted rather than fixed: `timeout`, `killSignal`, `maxBuffer` and
`shell: false` on that same object are equally uncovered. That is the
epic's gap, not this change's, and closing it is not in scope here.

* enhance(#2483): validate the invoke.env shape and register it as spawn-only

`env` was the one spawn-invoke field with no shape enforcement: every sibling in
`validateSpawnInvoke` is checked, and a manifest declaring `env` as an array, a
string, a number, or an object with non-string values passed validation in
silence. That matters more than an ordinary schema gap here, because
`resolveLanePlan` DROPS a non-string value rather than coercing it — so an
unvalidated manifest declares a pair that never reaches the spawn, which is the
failure a memory guard can least afford.

Two registrations, not one. `env` was also absent from
`SPAWN_ONLY_INVOKE_FIELDS`, which is the list the openai-http arm rejects
against — so `invoke.env` was accepted on a transport that issues an HTTP POST
and has no child environment at all. It was the only spawn-shaped field accepted
there; the other six each produce two errors. Self-found while sweeping the
class, not raised in review.

Keys are held to the portable POSIX environment-name grammar. That is a policy,
not a claim about what an environment can hold: measured, only NUL is actually
rejected by `spawnSync`, while `=`, a leading digit, a dash and a space are all
carried through to the child (an `A=B` key arrives as the raw entry `A=B=value`).
They are refused because a name outside the grammar is not portably addressable
by the program meant to read it.

`__proto__` is refused for a different and concrete reason. It passes that
grammar and is a real own key once a manifest is JSON-parsed, but assigning it
onto a plain accumulator goes through the inherited `__proto__` setter rather
than creating an own property — and for the string values this field permits the
setter is a no-op that does not even change the prototype. The pair would
validate and then simply vanish before the spawn. (An environment CAN carry a
literal `__proto__` entry; this is about the resolver's accumulator, and the
error message says so.)

Deliberately narrower than the sibling reserved-name guards in this file, which
also reject `constructor`/`prototype`: those guard bracket lookups that resolve
prototype members, whereas this reads via `Object.keys` plus an own-value read,
where `constructor` assigns as an ordinary key the spawn could carry.

`effortChannel` is deliberately left in neither field list: ADR-2782 D2 defines
it for both transports, so it is shared rather than spawn-only.

Reversion-controlled, three mutations, all three fire a named test: dropping
`env` from the discriminator fails `httpTransportRejectsEnv`; removing the
`__proto__` arm fails `envRejectsProtoKeyThatWouldSilentlyVanish`; disabling
the block fails four.

(#2483)

* enhance(#2483): amend ADR-2782 D2 for the invoke.env vocabulary widening

D2 records the spawn `invoke` shape as a closed vocabulary, and its Amendments
section carries a dated entry for every prior widening (Phase 1 #2794, Phase 2
corrections #2795, Phase 5b #2799). This change extended that vocabulary in code
without touching the ADR governing it, so the ADR contradicted the
implementation — and the repo's own convention, recorded in CONTEXT.md, is that
the ADR is amended in the same PR precisely because the prior widenings did it
correctly.

Adds the `invoke.env` row to the D2 table and a dated Amendments entry.

The entry also corrects the authority this change cited. The source comment
pointed at D6, which governs the closed `handler` enum — imperative behavior
admitted first-party — and says nothing about the `invoke` field vocabulary.
That is D2's territory, so the citation never covered the gap.

Two claims are corrected rather than restated, both about the trust boundary
that justifies leaving `env` out of the D5 disclosure signature:

- The regression test does not enforce that boundary. On one forged lane it
  shows the resolver folds whatever it is handed, so a future path feeding it
  manifest lanes would not make any assertion in that test fail. Its comment
  claimed it "will fail first"; that was wrong, and both the comment and the
  ADR now say the boundary is a property of the production call chain instead.
- The ADR is internally inconsistent on whether third-party manifest lanes
  execute at all: Consequences says adding a reviewer needs "no core patch",
  while `gsd-tools.cjs` rejects every slug absent from the first-party
  REVIEWER_LANES map. CONTEXT.md, `workflows/review.md` and the resolver's own
  header take the first view. #2483 did not create that inconsistency and does
  not resolve it; the entry records it rather than settling it in its own favour.

(#2483)

* enhance(#2483): document invoke.env in the capability-manifest reference

ADR-2782 points capability and plugin authors at
`docs/reference/capability-manifest.md` as where the lane vocabulary must be
visible, and its `invoke` row enumerates the spawn sub-shape field by field.
`env` was absent from that table while being part of the real shape, so the one
document a third-party capability author would actually consult to learn the
field exists did not mention it.

Squarely Diataxis reference material — a field-by-field schema description — so
it goes here rather than in the user-facing prose, which was already updated.
States the constraints a manifest author can actually trip, and is explicit that
the name grammar is a portability policy rather than an OS limit, so a reader
does not take it for a claim about what an environment can hold.

(#2483)

* enhance(#2483): disclose and sign the reviewer lane's env and residual invoke fields

`invoke.env` was undisclosed at install time. That was defensible while manifest
lanes could not execute — the premise this PR's own ADR amendment recorded — and
#2927/#3062 retired it: `routeReviewLane` now merges installed overlay `reviewer`
bodies into its lane map via `mergeReviewerLanes`, which is a field-identical merge
by ADR-2782 D1 and deliberately does not deep-validate. An overlay's whole `invoke`
therefore reaches `resolveLanePlan`, and `env` reaches the spawned child. A consented
third-party capability could set `NODE_OPTIONS=--require ./evil.js` on a reviewer lane
with no install-time disclosure and no re-consent.

The same file already decided what `env` means in a manifest: MCP servers fold it into
the disclosure signature and render each key and value in the consent prompt, with an
inline rationale naming this exact shape. Reviewer lanes get the identical treatment.

`env` was the ninth unsigned invoke field, not the first. `defaultHost` (the manifest's
OWN fallback egress host, used whenever the config key resolves to nothing),
`path`, `outputChannel`/`outputArg`, `modelArg`, `effortChannel` and `modelDiscovery`
all reach `resolveLanePlan` and none was bound. Enumerating a ninth name leaves the
tenth open, so the lane signature carries a RESIDUAL of every other declared `invoke`
key — the completeness backstop `rawConfig` already gives the MCP line (#1459 finding 5),
and the "sign the whole object" remedy the recorded decision on this class prefers.

`defaultHost` is also rendered: `resolvedHost` comes from user config, so a lane whose
key is unset displayed "(unresolved …)" — which reads as "no destination" — while the
runtime egresses the plan and review text to the address the manifest picked.

D4.5 is preserved one level down: the extra element is appended ONLY when the lane
declares something beyond the eight already-bound fields, so an env-free lane's
signature stays byte-identical and no already-consented capability is re-prompted for
a field it does not use. A lane that does declare one re-consents, which is the point.

Execution-primitive env names are FLAGGED in the prompt, not refused in the validator.
A denylist cannot be the boundary here: `PATH` alone is a complete execution primitive
for a spawn lane and can never be refused, the child is an arbitrary third-party binary
so the true set spans every interpreter's injection vars, and the MCP `env` this mirrors
refuses nothing and discloses everything. Missing a name costs a quieter line, never a
boundary.

Refs #2483.

* enhance(#2483): exercise the real overlay merge path in the guard test

The test named for the manifest/first-party boundary did not test it. It built a
forged lane locally, handed it straight to `resolveLanePlan`, and asserted that
`REVIEWER_LANES` did not contain it — so no assertion in it depended on the claim its
name made, and a code path that fed manifest lanes to the resolver would not have made
it fail. Its own comment said as much, and named the production chain as the real
carrier of the guarantee: "gsd-tools.cjs builds its lane map solely from REVIEWER_LANES".

That sentence is now false. #3062 merged overlay reviewer bodies into that map, so the
test's premise and its subject both moved.

The replacement routes through `mergeReviewerLanes` — the real helper the production
path calls — and asserts the overlay lane is admitted, resolves, and carries its `env`
into `SpawnPlan.env`. That makes the security property falsifiable instead of narrated.
It then asserts what now backs it: the env is disclosed on the surface, rendered key
and value in the consent prompt, flagged when the name is an execution primitive, and
bound to the signature so a value change, an addition, or a removal each force
re-consent.

Three further cases, because the finding's generative half is what stops it recurring:
the residual backstop is asserted against five fields including one that does not exist
(`aFieldThatDoesNotExistYet`), so a future vocabulary widening cannot silently re-open
this; a fully-enumerated lane is pinned to its original 8-tuple, which is what keeps the
fix from re-prompting every consented capability; and an http lane's manifest-declared
`defaultHost` is asserted to reach both the prompt and the signature.

Reversion-controlled, seven mutations, all seven fail a named test: env dropped from the
surface, the prompt's env line removed, the execution-primitive warning removed, the
signature's extra element never appended, the residual emptied, the defaultHost line
removed, and the declares-something test un-widened. The last of those was SILENT on its
first run and its test was written in response, then the control re-run.

Refs #2483.

* enhance(#2483): correct the ADR amendment's manifest-lane premise

The amendment argued `env` needed no D5 disclosure because a manifest's `invoke`
fields never reach `resolveLanePlan`. That was true when written and #3062 retired it
22 hours after this branch's last commit: `routeReviewLane` now builds its lane map
from `mergeReviewerLanes(REVIEWER_LANES, loadRegistry({includeInstalled: true}))`, and
D1's no-translation-layer rule makes that a field-identical merge, so an overlay's
whole `invoke` reaches the resolver and executes.

The entry had named this exact trigger — "were manifest lanes ever made executable,
`env` must join the disclosed surface in that change, and nothing here will trip if it
does not." Nothing tripped. The premise is rewritten to current truth rather than
annotated, because an ADR is read in fragments and a superseded paragraph left standing
reads as live reasoning to the next author; a one-line dated tombstone points at git for
the withdrawn text.

The rewritten entry records four things the first draft could not: that the enumeration
itself was the defect (`env` was the ninth unbound `invoke` field, and `defaultHost` and
`path` are egress-relevant on their own), that the residual is what closes the class,
that D4.5's byte-identical-signature property is preserved by appending the residual only
when a lane declares something beyond the eight bound fields, and that consent — not
shape validation — is the boundary, since no honest env denylist can exclude `PATH`.

It also closes the internal inconsistency the previous entry could only record. This ADR,
`CONTEXT.md`, `gsd-core/workflows/review.md` and `resolveLanePlan`'s own header all said
overlay lanes reach the resolver while the runtime said otherwise; #3062 resolved that in
the documents' favour, which is what makes the disclosure mandatory rather than defensive.

Refs #2483.

* enhance(#2483): record in the manifest reference that invoke fields are consent-bound

`docs/reference/capability-manifest.md` is the field table ADR-2782 points capability
authors at, and it described `invoke` purely as a schema. A third-party author reading it
could not learn that everything they declare there is shown to the user at install and
bound to the consent signature — which is exactly what they need to know now that an
overlay reviewer lane executes (#2927/#3062).

States the two things the schema alone cannot: that `env` and `defaultHost` are named in
the consent prompt and the rest is covered by a residual, so any change to a declared
`invoke` field forces re-consent; and that `env`'s validation is a portability policy
rather than a safety boundary, since `PATH` is a complete execution primitive and cannot
be refused. Names that are execution primitives are highlighted in the prompt instead.

Refs #2483.

* enhance(#2483): add a Security changeset for the reviewer-lane disclosure

The existing fragment describes the enhancement this PR was opened for and stays as it
is. The disclosure fix is a separate user-visible change of a different type: a
capability declaring `invoke.env` or `invoke.defaultHost` will ask for consent once
more, and users are entitled to read why in the changelog rather than discover it as an
unexplained prompt.

Type is `Security` rather than `Changed` because the entry describes a closed
code-execution disclosure gap, not a behaviour adjustment.

Refs #2483.

* enhance(#2483): correct this round's own claim about who gets re-prompted

Self-found while auditing the round's claims before publishing them. The changeset and
the ADR entry both stated that a capability declaring `invoke.env` or `defaultHost`
"will ask for consent once more". That is wrong, and it overstated the cost of the fix
in the one direction a maintainer would have had to take on trust.

A code change to `disclosureSignature` re-prompts nobody. `hasProjectConsent` matches on
the recomputed bundle `contentHash` — the signature has not been the security binding
since #1459 CB-1/CB-2 — and the upgrade path's `executableSetChanged(old, new)` compares
two disclosures both computed by the CURRENT code, so widening the signature moves both
sides of that comparison equally. First-party capabilities never reach the path at all:
the install flow blocks a first-party id before trust evaluation.

What the widening actually buys is forward-looking, and is the real argument for it: an
upgrade whose manifest edits a declared `invoke` field now registers as an
executable-surface change and re-consents, where before it could change what the lane
runs in silence.

Also measured and recorded, because the D4.5 property was stated more strongly than it
deserved: of the twelve first-party reviewer capabilities, ZERO are in the
byte-identical-signature class — every real lane declares at least `effortChannel`. The
property is a guarantee about minimal lanes, not a description of the fleet, and the ADR
now says so.

Refs #2483.

* enhance(#2483): sign and disclose the probe binary and the lane's outer fields

Found by this round's own adversarial review, and it is the same defect one level out:
the `invoke` residual cannot reach the lane body's OUTER fields, and `probeLane` SPAWNS
`probe.binary` with `--help` before dispatch (`review-lane-runner.cts`, the
`command-exists`/`command-capability` arms). An overlay naming an arbitrary probe binary
therefore executes it — unsigned and undisclosed, exactly as `invoke.env` was, and
reachable on the same #3062 path.

The lane element now carries a second residual over the outer fields, and the probe
binary is shown in the consent prompt when it differs from the dispatch binary — it is a
program that runs, and the user is entitled to see it.

TWO fields stay excluded, and that is a decision rather than an omission:
`reviewsSection` and `timeoutFloorMs` are ADR-2782's cosmetic carve-outs (matrix
A10/A13), where re-consenting would present a prompt carrying no security information.
A test pins that they remain excluded, so a later widening cannot quietly reverse D4.5
while claiming to complete this fix.

Also corrects a miscount introduced by the previous commit: the source comment said the
enumeration had fallen behind by "seven fields" and omitted `fallbackModel`, while
asserting `env` was the ninth. `resolveLanePlan` reads twelve `inv.*` fields and four
were bound, so the number is eight. The comment now states the derivation rather than
just the total.

Reversion-controlled: emptying the outer residual fails "repointing the probe binary must
force re-consent"; removing the render line fails its own named assertion.

Refs #2483.

* enhance(#2483): refuse execution-primitive env names as defence in depth

Adopts the review's B5 after this round's own adversarial pass refuted my reason for
declining it. I had argued a denylist was worthless because `PATH` can never be refused.
That was wrong on the facts: no shipped reviewer manifest declares `PATH`, so it can be
refused, and it is the most complete primitive in the set — repoint it at a directory
holding a fake binary and the declared `invoke.binary` is irrelevant. A list that cannot
be exhaustive can still close the highest-confidence, lowest-legitimacy routes.

So the validator now rejects `PATH`, `NODE_OPTIONS`, `LD_PRELOAD`, `DYLD_INSERT_LIBRARIES`,
`BASH_ENV`, `PYTHONPATH`, `PERL5OPT`, `RUBYOPT`, `GIT_SSH_COMMAND`, `JAVA_TOOL_OPTIONS`
and their siblings on a reviewer lane. A lane needing a specific executable declares an
absolute `invoke.binary` instead of reshaping the child's environment.

The comment states plainly that this is defence in depth and NOT the boundary — the
boundary is install-time consent, which discloses every declared pair and binds it to the
signature, so an unlisted name is still SEEN before it runs. That framing is load-bearing:
a future reader who mistakes the denylist for the control will under-invest in the one
that is, which is the failure mode I was trying to avoid by declining it outright.

Two tests: the rejection itself across ten names, and a guard asserting no shipped
reviewer capability declares a denied key — so if the list ever outgrows its evidence,
that surfaces as a decision rather than a silent removal.

Refs #2483.

* enhance(#2483): fix two stale D5 enumerations elsewhere in the ADR

The previous commit rewrote the amendment's premise but swept only the amendment. Two
normative passages earlier in the same ADR still enumerated the old closed field list and
now contradicted it: the `executableSetChanged` trigger list, and the split-binding note
asserting the seven manifest-derived fields were "everything that is SHA-pinned".

That is the failure the rewrite-don't-annotate rule exists to prevent, one section over —
an ADR is read in fragments, and a fragment carries no supersession marker, so a reader
landing on either passage would have taken the superseded enumeration as current.

Both now name the residual as the mechanism rather than restating a list, which is also
what stops them going stale the next time the vocabulary widens.

Found by this round's adversarial review, which grepped the whole document rather than
the section under edit.

Refs #2483.

* enhance(#2483): stop the probe disclosure claiming a spawn that does not happen

The probe line added one commit ago rendered "probes by running: <binary> --help" for
every lane. That is false for `kind: "command-exists"`, which only calls `hasBinary` — a
PATH/filesystem scan that starts no process. Only `command-capability` spawns.

A false statement in a consent prompt is worse than a missing one: the prompt is the
surface a user is asked to trust, and this one overstated what a lane does. Worse, the
test I wrote to prove the fix used `command-exists` — the kind that does NOT spawn — so
it pinned the wrong claim and would have kept the error green forever.

The surface now carries `probeKind` and the two kinds render differently: a spawn is
described as a spawn, a presence check as a presence check. The test exercises both, and
asserts the `command-exists` path never emits the spawn wording.

Also corrects the field-count parenthetical to state its derivation unambiguously —
`resolveLanePlan` reads thirteen `inv.*` fields including `env` (twelve before this PR),
four were bound, so eight were unbound before `env` and nine including it. The bare
"twelve" was true only of the pre-PR tree and read as a claim about the current one.

And retires two comments that argued AGAINST the validator denylist this round then
shipped. Leaving them would have handed the next reader the reasoning for removing it.

Reversion-controlled: conflating the two probe kinds fails a named test.

Refs #2483.

* enhance(#2483): match the reviewer-lane env denylist case-insensitively

The denylist added one commit ago compared exact case, so `Path`, `path`, `node_options`
and `Node_Options` all passed it. Windows environment lookup is case-insensitive, so
those reach the child as `PATH` and `NODE_OPTIONS` — the exact inputs the list names.

An exactly-cased denylist is worse than none: it reads as a control while admitting the
input it was written to refuse, and the next reader has no reason to doubt it. Members
are stored uppercase and the key is folded before lookup; the name grammar already
constrains keys to ASCII, so a plain fold is sufficient.

Reversion-controlled: restoring the exact-case compare fails `envDenylistIsCaseInsensitive`
on `Path`.

Refs #2483.

* enhance(#2483): correct the docs that still described the denylist as absent

Both the ADR and the manifest reference still said `env` carries no denylist and that
`PATH` "can never be refused" — written when that was this round's position, and left
standing after the round reversed it. A reader landing on either passage would have taken
the superseded argument as current, which is precisely the failure the rewrite-don't-
annotate rule exists to prevent.

Both now describe the denylist, name `PATH`'s inclusion and the case-insensitive match,
and keep the limit explicit: the list cannot be complete against an arbitrary child and
disclosure runs before validation, so consent remains the boundary.

The ADR's byte-identical-signature claim is also corrected rather than softened. With the
outer residual in place, a lane producing no residual is one the validator rejects — it
declares no `flags`, `probe`, `emptyOutput`, `evidenceClass`, `requiresBinaries` or
`promptBudgetKey`. So the property is about the ENCODING, not a claim that any real
signature is unchanged, and it is not the argument for the change being safe. That
argument is that consent binds to the bundle contentHash and no existing consent is
invalidated at all.

Refs #2483.

* test(#2483): cover the three new lane disclosure fields in the injection-safety parity guard

The PARITY test in section N exists to catch a renderer field that skips
`renderValueForPrompt` (#3248). Its payload manifest is hand-maintained, so it
covers the fields that existed when it was written — slug, binary, args,
hostConfigKey, handler — and none of the fields this PR adds.

This PR renders three further manifest-supplied values into consent-prompt
lines: `invoke.env` (keys and values), `invoke.defaultHost` and `probe.binary`.
The gap was silent rather than theoretical: with the lane env line reverted to
the pre-#3248 raw form, the whole 948-test lane/capability/trust-disclosure
suite stayed green.

Two manifests, because the shapes render disjoint lines — `defaultHost` only on
the openai-http branch, `env`/`probe` only where declared, and the probe line
only when the probe binary differs from the dispatch binary.

Non-vacuity is asserted on the typed disclosure object and on structural line
counts, not by substring-matching rendered prose: CONTRIBUTING.md forbids raw
text matching on test output, and this section's own header promises structural
assertions only, so a prose match here would have made that promise false.

Negative-controlled three ways against the merged tree, each producing exactly
one named failure: env rendered raw, defaultHost rendered raw, probe binary
rendered raw.

* docs(#2483): extend the #3248 render-site comment to the fields this PR adds

The comment enumerates every manifest-supplied value that must pass through
`renderValueForPrompt`, and it stopped at `handler` — the reviewer-lane fields
that existed when #3248 landed. This PR renders three more (`defaultHost`, the
probe binary, and the env keys and values), so the list understated its own
contract in the one place a future author would check before adding a fourth.

A comment enumerating a closed set is a set that can silently fall behind the
code it describes; the parity test added alongside is what makes the omission
fail loudly rather than read as deliberate.

---------

Co-authored-by: Tom Boucher <trekkie@nomorestars.com>
2026-08-11 17:42:41 -04:00
..

Architecture Decision Records

This directory contains Architecture Decision Records (ADRs) for GSD.

Each ADR documents one architectural decision: what was decided, why, and what consequences follow. ADRs are append-only. Amendments extend existing ADRs with a dated section rather than replacing them.

Reading this corpus

Start with the index below, and respect the status. The index is grouped so that the first table — Active decisions — is the set that governs the system as it stands. An ADR in Superseded, Retired, and Legacy is historical: it records what was once decided and names what replaced it. Do not cite it as current architecture.

Two things the index makes explicit, because getting them wrong has actually misled readers here:

  • "Read first" on an active ADR points at a broader ADR that now frames it. A decision can be entirely correct and still not be the whole picture. The runtime capability descriptor (ADR-1016) is live and load-bearing, but ADR-1239 (EoS — GSD as an Embeddable Orchestration Engine) subsumes it as the declarative adapter and inverts its direction: GSD is the engine a host embeds, not an installer that projects onto a host. For how GSD meets a host, EoS is the current frame.
  • Proposed means not ratified — and it is kept honest. On 2026-07-17 the corpus was audited against the shipped tree and nine ADRs whose decisions had demonstrably shipped were ratified to Accepted, each carrying a dated Ratification section with the evidence (see ADR-857 for the fullest example). The ADRs that remain Proposed are Proposed for a reason recorded in the file — an unmet acceptance criterion, an outstanding phase, or a successor ADR already planned — not through neglect. Trust the label; if you think it is wrong, prove it in a dated section and see Ratifying a stale Proposed.

Naming Convention

New ADRs use issue#-prefix slug naming:

docs/adr/<issue#>-<kebab-slug>.md

Examples: 2264-golden-parity-redesign.md, 1239-gsd-embeddable-orchestration-engine.md.

Why

Two developers computing "next ADR number" locally against main will independently pick the same integer and both ship. The collision is already on disk — 0010-* exists twice and 0011-* exists three times. GitHub issue numbers are server-assigned and atomic: the moment you open an issue, that number is reserved globally. Two PRs that both edit the ### Fixed block of CHANGELOG.md always conflict on merge — two PRs that each use a distinct issue# as their ADR prefix never collide. Same shape, same solution.

Legacy naming is not Legacy status

Files 0001-* through 0012-* are preserved as immutable historical record of the old local-compute numbering. The duplicate 0010-* and the three-way 0011-* are documented residue of that convention — not patterns to imitate. Do not renumber them.

This is the single authoritative statement of the legacy range. docs/contributor-standards.md references it rather than restating it, so the two cannot drift.

Two other zero-padded files look legacy but are not: 0174-retire-gsd-sdk-package-boundary.md (issue #174) and 0656-research-module-seam.md (issue #656) are mis-padded modern ADRs — modern, issue-numbered files whose four-digit padding is a mistake. They are NOT part of the legacy sequential set above and are not "old local-compute numbering" residue.

This is a statement about filenames only. Many of those ADRs are Accepted and load-bearing today (ADR-0002, ADR-0004, ADR-0008, ADR-0009). An old filename says nothing about whether a decision still holds. The Legacy status in the table below is a separate claim — see the vocabulary.

Because 0010-* and 0011-* each resolve to more than one file, a bare cross-reference like "ADR-0011" is genuinely ambiguous. Link the file (see Lifecycle rules).

Full process

See CONTRIBUTING.md — "Proposing an ADR or PRD" for the end-to-end workflow: opening the issue, waiting for approval, naming the file, and submitting the PR.

PRDs live in docs/prd/, not here. (0011-review-default-reviewers-prd.md predates that directory and is kept in place as frozen historical record.)

Lifecycle rules

These are enforced by scripts/gen-adr-index.cjs, which runs in CI via npm run lint:generated-sync. A violation fails the build with the exact file and fix.

1. Every ADR declares one status from the canonical vocabulary

The first word of the Status field must be one of:

Status Means Obligation
Accepted Decided and in force. Cite it. —
Proposed Decided in principle, not ratified. Do not cite as settled. If the work has demonstrably shipped, ratify it (below) — do not leave the label lying.
Superseded A specific newer ADR replaced this decision. Must name the successor as a file link.
Retired What this ADR decided no longer exists at all, and no single ADR replaced it. Say what was removed and when.
Legacy Frozen historical record, kept for provenance; not a pattern to follow. Say why it is frozen.

Prose may follow the token (Superseded by [ADR-0174](0174-retire-gsd-sdk-package-boundary.md) (2026-05-23); originally Accepted (2026-05-09)). Both the bullet form (- **Status:** Accepted) and the table form (| **Status** | Accepted |) are accepted.

Write [ADR-0011](0011-skill-surface-budget-module.md), not ADR-0011. Bare ids are ambiguous for 0010/0011, and unlinked references cannot be checked.

If you mean an issue, write #857 — not ADR-857. (An ADR and its owning issue often share a number; that is intentional and not a conflict.)

3. Supersession and subsumption are symmetric

These are different relations. Do not conflate them:

  • Supersedes / Superseded by — the target is replaced. Its status becomes Superseded.
  • Subsumes / Subsumed by — the target still holds, but a broader ADR now frames it. Its status is unchanged; it becomes a component of the larger decision.

If A declares either relation toward B, B must record the reciprocal. A one-way pointer is the failure this corpus actually suffered: ADR-1239 declared it subsumed four ADRs, none of which said so, and none of which pointed back — so a reader landing on any of them concluded the superseded frame was the way forward.

Only an Accepted ADR is owed the back-link. A Proposed ADR's claim is prospective: it has not taken effect, so its target is not marked. On ratification, the check begins demanding the back-links.

4. The declared id matches the filename

An H1 of # ADR-0175: … in a file named 218-*.md is a rename that never finished. The id in the title must match the filename's prefix.

5. A trailing H1 status bracket must agree with the Status field

Many ADRs restate their status in the H1 — # ADR-1610: … [Accepted]. That bracket is the first thing a reader sees, and the index strips it when rendering the title, so a stale one used to be invisible to everyone but the reader it misled.

If the H1 ends in a bracket holding a status token, it must name the same status as the Status field. Comparison is case-insensitive and against the parsed token, so [Superseded] agrees with Status: Superseded by [ADR-0174](0174-retire-gsd-sdk-package-boundary.md) (2026-05-23).

A trailing bracket that is not a status token — [Draft], [WIP] — is treated as part of the title and left alone. If you want a bracket the gate ignores, do not spell it like a status.

A link whose target does not exist on disk fails the check, naming the file, the line, and the unresolved target. This covers every markdown file in this directory, including this README and any file whose name breaks the convention above.

Written as Treated as
[t](900-beta.md), [t](../prd/) resolved — a directory counts
[t](900-beta.md#section) the file is resolved; the #fragment is not checked
[t](https://…), [t](mailto:…), [t](//host/x) out of scope — absolute destinations are never fetched
[t](#lifecycle-rules) out of scope — a same-document anchor is not a file reference
[t](/docs/adr/x.md) resolved against the repository root, as GitHub does
a link inside a ``` fence or `backticks` not a link — markdown does not render one there, so it is never resolved
[text][ref] reference-style, <a href>, bare autolinks not supported; write an inline link

Two consequences worth stating outright:

  • Case matters, on every platform. [t](0001-Alpha.md) pointing at 0001-alpha.md fails even on macOS and Windows, because it 404s on github.com and reds the Linux CI lane. The failure names the entry it found so the fix is obvious.
  • A link to a generated or ignored path fails. Nothing here consults .gitignore; the question is only whether a reader following the link lands somewhere. Cite the hand-authored source rather than the build artifact.

If the gate rejects something you wrote

Reproduce it locally first — it is the same command CI runs, and it names the file, the line, and the target:

node scripts/gen-adr-index.cjs --check

Then work from the reason:

What it says What to do
does not resolve — no such file or directory at … Fix the path. It is relative to docs/adr/, so a sibling ADR is just 900-slug.md. If the target genuinely does not exist yet, drop the link rather than leaving it pointing nowhere.
…Did you mean X? — link targets are case-sensitive on github.com Match the on-disk name exactly. Your machine may open the file regardless; github.com and the Linux CI lane will not.
escapes the repository The path resolves outside the repo. Link something inside it, or use an absolute URL — those are out of scope and never checked.
is a symlink that escapes the repository An ADR file itself is a symlink pointing outside the repo. Commit a real file.
H1 status bracket […] contradicts the Status field (…) Update whichever of the two is stale so they agree. The Status field is authoritative; the bracket is a restatement for the reader.

A link that is an example, not a destination, belongs in backticks. The gate skips fenced blocks and inline code entirely, because markdown does not render a link there. That is the escape hatch for illustrative syntax — the table above is written that way, which is why it does not fail this check. An indented code block (four spaces) is not skipped; use backticks.

To consume the result from a script rather than by eye, use --json (below) and branch on each violation's stable reason code.

Ratifying a stale Proposed

A stale Proposed is not cosmetic: it tells contributors and agents that live architecture is an unbuilt idea. Fix it — but on evidence, not vibes.

The bar. All four must hold before flipping to Accepted:

  1. The decided mechanism demonstrably exists in the tree — name the files, symbols, and tests.
  2. The owning issue is closed as completed. A closed issue is not proof: stateReason of not planned / duplicate means the decision was dropped (that is Legacy or Retired, not Accepted).
  3. No material part is unshipped. If the ADR defines phases and one is outstanding, or states its own bar for acceptance and that bar is unmet, it stays Proposed.
  4. No later ADR supersedes it, and no approved issue already plans its graduation as separate work.

The procedure. Set the status to Accepted — ratified <date> (originally Proposed <date>), add a dated ## Ratification section holding the evidence, then run node scripts/gen-adr-index.cjs --write. If the ADR claims to supersede or subsume others, the gate will now demand their back-links — that is the point. Ratify deliberately.

Two traps worth knowing, both hit during the 2026-07-17 audit:

  • Shipped code is necessary, not sufficient. Eight ADRs had every named module, symbol, and test present and their epics closed — and still failed the bar: ADR-2264's own headline acceptance criterion is unmet in the tree, ADR-230's decided branch protection does not match the live API, ADR-660's namesake mechanism is performed by hand, and ADR-959 has an approved issue planning its graduation as its own ADR. Verify the decision, not just the code.
  • "Supersedes" is often "subsumes". Read what the ADR means before the gate makes you act on what it says. ADR-857 said "Supersedes (generalizes)"; taken literally, ratifying it would have stamped two live seams (ADR-0011, ADR-58) as dead. The parenthetical was the truth; the field name was wrong.

Maintaining the index

The index is generated. Do not hand-edit it. Everything between the ADR-INDEX:START / ADR-INDEX:END markers is derived from the ADR files themselves:

node scripts/gen-adr-index.cjs            # print the index
node scripts/gen-adr-index.cjs --write    # regenerate it into this file
node scripts/gen-adr-index.cjs --check    # CI: fail if stale or invalid
node scripts/gen-adr-index.cjs --json     # same checks, machine-readable report

After adding an ADR, or changing any ADR's status or relations, run --write and commit the result. npm run lint:generated-sync runs --check in CI, so a missing or stale row fails the build rather than rotting silently.

--json runs the same validation as --check and writes a report to stdout instead of prose to stderr, with the same exit code. Each violation carries a stable reason code, so a tool consuming this never has to pattern-match an error message:

{
  "ok": false,
  "adrCount": 76,
  "indexStale": false,
  "violations": [
    { "file": "2704-example.md", "line": 41, "reason": "link_unresolved",
      "target": "reference/x.md", "resolved": "docs/adr/reference/x.md" }
  ]
}

An unrecognized flag is rejected rather than ignored.

This replaces a hand-maintained table that had drifted to 40 of 65 ADRs — the entire capability family and EoS itself were missing from it, which is precisely why the ADRs a reader most needed were the ones they could not find.

Index

Active decisions

These govern the system as it stands. Cite these.

ADR Title Status Read first
ADR-0001 Dispatch policy module as single seam for query execution outcomes Accepted —
ADR-0002 Command Contract Validation Module Accepted —
ADR-0003 Model Catalog Module as single source of truth for agent profiles and runtime tier defaults Accepted —
ADR-0004 Planning Workspace Module as single seam for worktree and workstream state Accepted —
ADR-0006 Planning Path Projection Module for SDK query handlers Accepted —
ADR-0008 Installer Migration Module owns install-time upgrade safety Accepted —
ADR-0009 Shell Command Projection Module owns runtime-aware OS command rendering Accepted —
ADR-0011 review.default_reviewers config key scopes the no-flag /gsd-review fan-out Accepted —
ADR-0011 Skill Surface Budget Module owns install-time profile staging and runtime surface control Accepted ADR-857
ADR-15 Cross-AI Plan Convergence via Existing Orchestration Commands Accepted —
ADR-22 Plan-vs-codebase drift guard: defaults and symbol-resolver seam Accepted —
ADR-58 Runtime Install Policy Module owns the typed install-plan projection Accepted ADR-1239, ADR-857
ADR-0174 Retire @opengsd/gsd-sdk package boundary — single-runtime collapse Accepted —
ADR-218 Harden release-workflow version validation — reject leading zeros and pre-check npm Accepted —
ADR-227 Input validation must check semantic shape, not just type Accepted —
ADR-415 Prevent stale-base reintroduction of retired runtime tokens Accepted —
ADR-452 Adopt standard ESLint flat-config lint harness Accepted —
ADR-456 Test-rigor architecture — deterministic scheduling, antagonistic tier, typed-surface mandate, and delete-bad-tests policy Accepted —
ADR-457 Generation model for bin/lib/*.cjs type safety Accepted —
ADR-550 spec-phase probe pattern and prohibition contract Accepted —
ADR-0656 Research Module — L2-hybrid seam for cached, curated-first research Accepted —
ADR-766 Claude Code Plugin Manifest Module owns the projection of gsd-core surfaces onto the Claude Code plugin contract Accepted —
ADR-857 Capability system — five-step loop as core, features as plug-ins behind Loop Extension Points Accepted —
ADR-894 Capability declaration format + registry generation Accepted ADR-1239
ADR-959 Capability Command Contribution Accepted —
ADR-1016 Runtime Capability Descriptor Accepted ADR-1239
ADR-1235 Migrate agent conversion to the descriptor-driven install path Accepted —
ADR-1239 GSD as an Embeddable Orchestration Engine Accepted —
ADR-1244 Capability Ecosystem: third-party authoring, versioned manifests, and URL import/upgrade/remove Accepted —
ADR-1372 Canonical markdown-structure parsing — the markdown-sectionizer seam Accepted —
ADR-1411 Resolution must report provenance, not fall open silently Accepted —
ADR-1508 Runtime Artifact Conversion Module owns per-runtime content rewriting Accepted —
ADR-1517 Reviewer instances — bounded config surface for same-adapter multi-model review Accepted —
ADR-1577 Untrusted-input boundary + opt-in injection blocking Accepted —
ADR-1593 Skill mapping & converter methodology across runtimes Accepted —
ADR-1610 workflow & agent size-budget ratchet (per-file byte baseline + tier hard caps) Accepted —
ADR-1703 Cross-platform portability enforcement as AST ESLint rules Accepted —
ADR-1769 STATE.md Transition Module — intent-based transitions over scattered RMW callbacks Accepted —
ADR-1787 /gsd:next smart-entry front door delegates advancement to /gsd:progress --next Accepted —
ADR-1817 STATE.md rebuild — derivability contract (capstone transition) Accepted —
ADR-1820 Spec-Optional Predicate Rail — the Spec-Section Detection Module, the fallback toggle, and the SPEC↔probe precedence contract Accepted —
ADR-1866 agent_skills dual injection — orchestrator-side + agent-side self-load Accepted —
ADR-1990 Existing Code Onboarding Module owns deterministic repo-state detection and onboarding route selection Accepted —
ADR-2008 Generic gate-predicate evaluator Accepted —
ADR-2121 Phase-Identifier Parsing Consolidation Accepted —
ADR-2143 Markdown Table Model, Bounded Mutation, and Fail-Loud Consolidation (#1372 part 2) Accepted —
ADR-2164 Statusline draws its data boundary at local, read-only sources Accepted —
ADR-2207 STATE.md Status lifecycle — phase-completion writes an intermediate state; milestone-close owns termination Accepted —
ADR-2313 Codex Adopts the Passive / Session-Only Model Posture Accepted —
ADR-2346 Command Dispatch Completion Accepted —
ADR-2363 A capability's skill body is an instruction surface — trusted, unscanned, and disclosed Accepted —
ADR-2619 Observability and shareable diagnostics — wire the dispatch seam, add the outbound trust boundary Accepted —
ADR-2629 Phase effort is estimated against a calibrated smart-zone budget, not a static heuristic Accepted —
ADR-2719 Emitted-artifact attribution — replace the committed parity fixtures with a computed conservation law Accepted —
ADR-2782 Reviewer Lane — the cross-AI reviewer handoff becomes a declared capability surface Accepted —
ADR-2866 Install-surface resolution — the install pipeline resolves (runtime × scope × trigger) as a value Accepted —
ADR-2966 Test the five-step loop as a continuous walk, not isolated points Accepted —
ADR-2980 A payload-carried error key is a degraded result, not a fault Accepted —
ADR-3180 Planning Semantic Model — Single Owner per Derivation Accepted —
ADR-3212 The Lexical Seam — Safe Pattern Construction, Line-Terminator Normalization, and Tokenizer-First Stateful Grammars Accepted —
ADR-3660 Runtime Artifact Layout Module owns per-runtime artifact placement Accepted ADR-1239

Proposed

Decided in principle, not yet ratified. Do not cite as settled architecture.

ADR Title Status Read first
ADR-230 Introduce next as a long-lived integration branch Proposed —
ADR-443 Unified cross-provider effort controls and fast-mode-aware routing Proposed —
ADR-612 Bracket Phase-ID Convention Proposed —
ADR-660 Release from the head of next; immutable release tags; @next dist-tag as the RC surface Proposed —
ADR-1143 Claude orchestration capability — Workflow tool (ultracode) as a runtime-gated loop execution backend Proposed —
ADR-1213 Capability write side — the Capability State Writer Proposed —
ADR-1606 prohibition-enforcement verify-time seam Proposed —
ADR-1671 Dynamic context management platform Proposed —
ADR-1953 Complexity-triggered refactor — the loop measures the entropy it just added Proposed —
ADR-3128 Adaptive runtime evidence for GSD Debug Proposed —

Superseded, Retired, and Legacy

Historical record. Do not follow these — each names what replaced it, or why it was retired.

ADR Title Status Replaced by
ADR-0005 SDK Architecture seam map for query/runtime surfaces Superseded ADR-0174
ADR-0007 SDK Package Seam Module owns SDK-to-get-shit-done-redux compatibility Superseded ADR-0174
ADR-0010 File Operation Engine Module owns safe runtime/config file mutations Superseded ADR-0009
ADR-0010 Skill Surface Budget Module owns install-time skill listing curation Superseded ADR-0011
ADR-0011 PRD — review.default_reviewers config key for /gsd-review reviewer selection Legacy —
ADR-0012 CommandRoutingHub as single dispatch seam for CJS command families Superseded ADR-0174
ADR-2264 Redesign golden-install-parity — single-source manifest builder + split invariant Superseded ADR-2719
ADR-3524 CJS↔SDK hard seam — one source of truth per Shared Module Superseded ADR-0174

Generated by scripts/gen-adr-index.cjs — run --write after adding or restatusing an ADR.

Seam map

Orientation for the module-ownership ADRs. This section is prose and hand-maintained; the index above is the authority on status.

How GSD meets a host — start at ADR-1239 (EoS). It is the current frame and subsumes the descriptor/projection ADRs (ADR-1016, ADR-58, ADR-3660, ADR-894) as adapters beneath it.

The SDK seam map is gone. ADR-0005 was once the entry point for SDK module ownership; it is superseded by ADR-0174, which retired the @opengsd/gsd-sdk package boundary entirely. There is no sdk/ tree. Read ADR-0174 for the single-runtime collapse; the seam-Module vocabulary survives under one src/.

ADR-0006 documents how query handlers project planning paths (cwd → effectiveRoot → .planning/<project>/...). Cross-reference the Planning Workspace Module (ADR-0004) for workstream pointer policy.

ADR-0008 documents the Installer Migration Module for safe install-time moves, removals, config rewrites, and user-data preservation.

ADR-0009 documents the Shell Command Projection Module seam for runtime-aware projection of installer-owned command text and projection IR. Its Phases 3–4 absorbed the File Operation Engine Module (ADR-0010).

ADR-0011 documents the Skill Surface Budget Module for install-time skill/agent profile staging (--profile=<name>, .gsd-profile marker, requires: closure) and the Phase 2 runtime /gsd:surface command.

ADR-1411 establishes the Resolution Provenance principle: context resolution (config loading, project-root anchoring, workstream resolution) must report its provenance rather than fall open silently to defaults. It is the resolution-side analog of ADR-227 (input-validation shape).