fix(#4594): one canonical dispatch-identity owner — the emitted format and the parser that reads it back (#4693)
* fix(#4594): give dispatch identity one owner for the emitted format and its parser The isolation guards decided whether a run-scoped sentinel applied to a dispatch by regex-scraping model-authored prose. The scrape returned values in a different namespace from the ones the sentinel records, so the comparison could never succeed: sentinel { phase: "03", plan: "03-02-hardening" } <- $PHASE_NUMBER / $plan_id prose "Execute plan 02 of phase 03-auth." scraped { phase: "03-auth.", plan: "02" } <- greedy (\S+), both wrong #4594 reports only the phase half. Measured against a real phase-plan-index run, plans[].id is phase-prefixed, plan-numbered AND slugged, while the prose carries a bare in-phase plan number — so the plan field mismatches too, and the Claude path is dead rather than latent. A fresh sentinel was therefore discarded on every executor dispatch and every legitimate ISOLATION=none degrade was denied, leaving the work unrun. hooks/lib/dispatch-identity.js is now the single owner of both halves. The two prompt-body producers emit a canonical marker carrying the same shell values the sentinel records, so producer and consumer agree by construction. The prose frame stays as a fallback, bounded by the phase-token grammar ADR-2121 owns and deliberately reporting no plan — an absent identifier means "cannot compare" and is safe; a wrong one is a false mismatch and is not. The prose sentence itself is byte-identical: the executor agent reads it too, so the marker is purely additive (Hyrum's Law). An inapplicable sentinel is now named in the guards' deny reason instead of being dropped silently — the silence is why this survived three producers and two consumers unnoticed. Interpolated values come from a sentinel file and from prompt text, so both are length-bounded and stripped of control characters. ADR-4630 locks the seam and maps the epic's three phases. Refs #4630 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(#4594): resolve eight review findings across the dispatch-identity seam Three orthogonal review engines ran on 43418af144 — the code-review skill's Standards and Spec axes, and an isolated adversarial security pass — plus a self-review of the committed diff. Every finding is fixed here; none deferred. F1 (major, reproduced). A keyless or unknown-key-only marker — the literal "[gsd:dispatch]" or "[gsd:dispatch run=..]" — matched the marker grammar and returned source:'marker' with both fields null, suppressing the prose fallback entirely. Any prompt text containing that literal silently disabled identity narrowing, so a fresh sentinel applied to a dispatch it was never scoped to, defeating #3045 SECURITY F2. Prompt text is attacker-influenceable. A marker that yields neither recognized key is no longer a marker: the scan continues to later markers, then later texts, then prose. Forward-compatible tolerance of unknown keys is unchanged. F2/F3 (major). The first cut duplicated sanitizeForReason, describeSentinelDiscard and REASON_INTERPOLATION_MAX_LEN byte-for-byte across both guards — the exact defect class this epic exists to delete, and with no cold-load justification, since both hooks already require hooks/lib/. They now live in hooks/lib/isolation-deny-reason.js, and buildSentinelDiscard lives in isolation-sentinel.js beside the comparison it mirrors, returning the nested {sentinel:{phase,plan}, dispatch:{phase,plan}} shape instead of a bespoke four-field bag that renamed the pairs already flowing through the seam. F4 (hard violation). The visibility test asserted on the deny reason's prose. CONTRIBUTING.md prohibits raw text matching on hook output, which is why every deny carries a stable reason_code. The discard is now a structured sentinel_discarded field on each hook's stdout JSON, and the test asserts that; the sentence stays for the operator but is no longer the contract. F5 (hard violation). The 64-character truncation limit had no boundary coverage. 63/64/65 are now exercised against the single consolidated helper. F6 (minor). sanitizeForReason stripped C0/C1 controls but not U+2028/U+2029 or the bidi overrides, so a crafted value could still reflow or reverse the message. Both classes are stripped, with a test each. F7 (major). The producer/template parity test was vacuous — it rendered a marker and re-parsed its own output, and would have passed with both templates deleted. It now reads the two workflow templates, extracts each marker line, substitutes the measured values and asserts the owner's parser returns them. Proven red by deleting one template's marker line before being proven green. F8 (doc). ADR-4630 and the design notes claimed the marker is guaranteed on the orchestrator-worktree path because that prompt is built in shell. It is not: executor-isolation-dispatch.md:131 says plainly that those are template placeholders, not shell variables, so {plan_id} is model-substituted there too. A false guarantee in a design lock is worse than a stated limit. Both documents now say the marker is model-substituted on both paths and that the prose fallback is the real floor everywhere. The "3 workflow templates" count was also wrong — 3 prose sites across 2 files, 2 of which carry the marker. Refs #4630 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * chore(#4594): refresh the compact-content baseline and acknowledge execute-phase.md growth Refs #4630. The dispatch-identity marker and its substitution note grew gsd-core/workflows/execute-phase.md by 525 bytes (91846 -> 92371), which drifts two real-tree guards that lint:ci does not run: - tests/benchmark-compact-content.test.cjs asserts the committed baseline is "up to date"; the split for execute-phase.md moved off 25827 -> 25952 and on 23576 -> 23701, taking its compaction reduction 8.72% -> 8.67%. Baseline regenerated with scripts/benchmark-compact-content.cjs --write. - tests/emitted-attribution.test.cjs requires a growth acknowledgment trailer for any emitted file that grows, keyed on the bare filename. Added below. The growth is two additions and no rewrites: the [gsd:dispatch ...] marker line inside the Agent() prompt's <objective>, and the note telling the orchestrator to substitute {plan_id} with the plan's id verbatim. Both are load-bearing -- the marker is what lets a guard hook match a dispatch to the sentinel the per-plan gate wrote, and without the note the orchestrator has no instruction telling it the value must not be paraphrased. Emitted-Drift-Ack-Growth: execute-phase.md — adds the canonical [gsd:dispatch] identity marker and its {plan_id} substitution note, which the isolation guards compare verbatim against the run-scoped sentinel (#4594) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * chore(#4594): set changeset fragment pr to 4693 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: sim <sim@local> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Tom Boucher
5
.changeset/patient-tunas-howl.md
Normal file
5
.changeset/patient-tunas-howl.md
Normal file
@@ -0,0 +1,5 @@
|
||||
---
|
||||
type: Fixed
|
||||
pr: 4693
|
||||
---
|
||||
**Executor dispatches are no longer refused when a phase correctly degrades to sequential execution.** The isolation guards identified a dispatch by regex-scraping model-authored prose, which returned identifiers in a different namespace from the ones the run-scoped sentinel records — so a fresh decision was discarded on every executor dispatch and every legitimate `ISOLATION=none` degrade was denied, leaving the work unrun. Dispatch identity now has one owner for both the emitted format and the parser that reads it back. (#4594)
|
||||
211
docs/adr/4630-dispatch-identity-and-isolation-decision-seam.md
Normal file
211
docs/adr/4630-dispatch-identity-and-isolation-decision-seam.md
Normal file
@@ -0,0 +1,211 @@
|
||||
# ADR-4630: One Canonical Dispatch-Identity Owner and a Recorded Isolation Decision
|
||||
|
||||
- **Status:** Accepted
|
||||
- **Date:** 2026-09-13
|
||||
- **Issue:** [#4630](https://github.com/open-gsd/gsd-core/issues/4630) — epic (`type: chore`, `area: agents`)
|
||||
- **Absorbs:** [#4222](https://github.com/open-gsd/gsd-core/issues/4222), [#4561](https://github.com/open-gsd/gsd-core/issues/4561), [#4594](https://github.com/open-gsd/gsd-core/issues/4594) — three `confirmed-bug` issues that are the same missing owner reported at two ends of one wire
|
||||
- **Framed by:** [ADR-1239](1239-gsd-embeddable-orchestration-engine.md) (host-integration interface — agent dispatch is interface point 2)
|
||||
- **Pattern copied from:** [ADR-2121](2121-phase-identifier-parsing-consolidation.md) (phase-identifier consolidation: seam + migration + anti-divergence guard). This ADR reuses its phase-token grammar and its drift-lint shape deliberately.
|
||||
- **Scope note:** unlike ADR-2121, this ADR does **not** land as a code-free Phase 0. It ships alongside Phase 1's implementation because the epic's phase sub-issues could not be created in the session that executed it; the decisions below were nonetheless fixed before that implementation was written, and Phases 2 and 3 execute against this file.
|
||||
|
||||
## Context
|
||||
|
||||
A GSD phase execution decides, per dispatch, whether the executor runs isolated in a git
|
||||
worktree or sequentially in the primary checkout. That decision is:
|
||||
|
||||
1. **produced** in workflow shell (`execute-phase/steps/executor-isolation-dispatch.md`, `per-plan-worktree-gate.md`, `references/dispatch-isolation-gate.md`, `execute-plan.md`),
|
||||
2. **transported** through a run-scoped sentinel at `.gsd/dispatch-isolation-sentinel.json`,
|
||||
3. **consumed** by two guard hooks (`hooks/gsd-agent-isolation-guard.js` for Claude-shaped hosts, `hooks/gsd-cursor-subagent-start.js` for Cursor).
|
||||
|
||||
At every hop, both sides hand-roll their own copy of the format. #4561 states the census
|
||||
directly: the dispatch-isolation membership is **hand-written at eight uncross-checked
|
||||
sites**. The consequence is not a style complaint — it is three confirmed bugs:
|
||||
|
||||
| Issue | Defect |
|
||||
|---|---|
|
||||
| #4222 | A plain `dispatch-isolation` re-query re-resolves the host *capability* and re-persists it, clobbering every shell-computed degrade but the #3737 opt-out. The guard then denies the mandated sequential dispatch with `exit 2`. |
|
||||
| #4561 | #4232's re-derivation fix reaches only producers whose degrade is re-derivable from a file the resolver reads. Three producers compute their degrade from call-site context that is **not on disk** and are structurally unreachable that way. |
|
||||
| #4594 | `extractDispatchIdentifiers` cannot parse the canonical prompt-body dispatch text, so the sentinel match never succeeds. |
|
||||
|
||||
### What #4594 actually is, measured
|
||||
|
||||
The reported symptom is a greedy capture: `/execute\s+plan\s+(\S+)\s+of\s+phase\s+(\S+)/i`
|
||||
swallows the `-{phase_name}` suffix and the sentence-terminating period, so
|
||||
`Execute plan 02 of phase 03-auth.` yields phase `03-auth.` and can never equal the
|
||||
sentinel's `03`.
|
||||
|
||||
That is true and incomplete. Running `phase-plan-index` against a real fixture shows
|
||||
`plans[].id` is `03-02-hardening` — phase-prefixed, plan-numbered **and slugged** — and
|
||||
`per-plan-worktree-gate.md:17` records exactly that value as the sentinel's `plan`. The
|
||||
prose, meanwhile, carries a bare in-phase plan number (`02`). The two fields therefore live
|
||||
in **different namespaces**, and the plan comparison mismatches on *every* dispatch, on
|
||||
*both* hosts. The issue calls the Claude path "latent rather than dead"; it is dead too, for
|
||||
a second reason the issue does not name. Fixing only the greedy capture would have shipped
|
||||
as a green, fully-tested no-op.
|
||||
|
||||
### Why this recurs rather than staying fixed
|
||||
|
||||
Repairing each symptom where it was reported leaves the missing owner missing, and the next
|
||||
occurrence is already being written elsewhere in the tree. The recurrence has a mechanical
|
||||
root, and it is the same one ADR-2121 identified for phase identifiers: **a format with no
|
||||
single owner is re-derived at each site, and two copies that agree today are the same defect
|
||||
as two that disagree.**
|
||||
|
||||
## Decision
|
||||
|
||||
### Decision 1 — one module owns the emitted format *and* the parser that reads it back
|
||||
|
||||
`hooks/lib/dispatch-identity.js` owns both halves together:
|
||||
|
||||
- `renderDispatchIdentityMarker({phase, plan})` — the format producers emit.
|
||||
- `parseDispatchIdentity(...texts)` — the pattern consumers read back.
|
||||
|
||||
Producers emit a structured marker carrying the **same shell values the sentinel records**:
|
||||
|
||||
```text
|
||||
[gsd:dispatch phase="03" plan="03-02-hardening"]
|
||||
```
|
||||
|
||||
`$PHASE_NUMBER` and `$plan_id` are already in scope at every producer site. Producer and
|
||||
consumer then agree *by construction*, independently of how the surrounding prose reads or
|
||||
whether a model paraphrases it.
|
||||
|
||||
The prose frame remains as a fallback, with the greedy `(\S+)` replaced by the phase-token
|
||||
grammar ADR-2121 already owns, and with `plan` deliberately **not** reported — the prose
|
||||
plan token is a different namespace from `plan_id`, and reporting it is the bug. The
|
||||
fallback is therefore **correct-or-absent** in both fields: an absent identifier means
|
||||
"cannot compare" and is safe by the existing contract; a wrong one is a false mismatch and
|
||||
is not.
|
||||
|
||||
**The owner lives in `hooks/lib/`, not `src/`.** The guard hooks must load on a raw
|
||||
plugin-marketplace install where the compiled `gsd-core/bin/lib/` is absent and the
|
||||
self-healing build seam has not run — a hook that dies at module load is worse than one
|
||||
carrying a mirror. A `src/*.cts` owner would force either a top-level require of the
|
||||
compiled lib (breaking that install) or a second mirrored copy (the exact defect this epic
|
||||
removes). Nothing in `src/**` needs this format: the producers are workflow *markdown* and
|
||||
the consumers are *hooks*.
|
||||
|
||||
### Decision 2 — the isolation decision is a recorded fact, not a re-derived one
|
||||
|
||||
A producer writes an `IsolationDecision` (the value, which producer decided it, and its run
|
||||
scope); consumers **read** it. A plain re-query must not overwrite a recorded decision.
|
||||
|
||||
Re-derivation is the wrong primitive for the three producers #4561 names, because each
|
||||
computes its degrade from call-site context that does not exist on disk:
|
||||
|
||||
| Producer | Why re-derivation cannot reach it |
|
||||
|---|---|
|
||||
| `gsd-core/references/dispatch-isolation-gate.md` | the `orchestrator-worktree` fallback fires only at a subagent-tool-only dispatch site; the resolver cannot see which tool the caller has |
|
||||
| `gsd-core/workflows/execute-phase/steps/per-plan-worktree-gate.md` | the #2474 per-plan submodule intersection is a property of the plan set, not the repo |
|
||||
| `gsd-core/workflows/execute-plan.md` | Pattern B's unisolated segments are by design; which segment is executing is not on disk |
|
||||
|
||||
**A decision that cannot be recomputed must be recorded and honoured, not recomputed and
|
||||
overwritten.** #4232's base-check re-derivation stays — it is correct for the producers it
|
||||
covers, and this decision builds on it rather than replacing it.
|
||||
|
||||
### Decision 3 — a copy that must exist is pinned by a parity test, never merely tolerated
|
||||
|
||||
Decision 1's cold-load constraint means `hooks/lib/` carries deliberate mirrors of two
|
||||
things it cannot import: the phase-token grammar (from `src/phase-id.cts`) and the isolation
|
||||
vocabulary (from the Phase 2 owner). Each mirror is **pinned by a parity assertion that reds
|
||||
when the owner changes**.
|
||||
|
||||
This matters more than it looks. A drifted grammar mirror fails as a **non-match** — nothing
|
||||
throws, no test that only feeds the old shape notices, and the symptom is a guard that
|
||||
quietly stops recognizing valid input. That is the failure mode that produced #4594 and it
|
||||
is invisible without a pin.
|
||||
|
||||
A mirror's justification is part of the contract: **do not "clean up" a pinned mirror
|
||||
without replacing the reason it exists.**
|
||||
|
||||
### Decision 4 — the anti-divergence lint is part of the deliverable, not a follow-up
|
||||
|
||||
A drift lint asserts that (a) every dispatch-identifier template in
|
||||
`gsd-core/workflows/**/*.md` and every parsing regex in `hooks/**` derives from the single
|
||||
exported token source, and (b) every dispatch-isolation producer site appears in the
|
||||
canonical membership list. It is modelled on `scripts/lint-phase-id-drift.cjs`.
|
||||
|
||||
Two lessons from #2121 are binding:
|
||||
|
||||
- Compare the regex derived from the **source string** (`.source` / `.flags` against the
|
||||
canonical token), not the rendered literal.
|
||||
- A sanctioned exemption is a **dedicated comment line** that states which half is
|
||||
deliberate — never a line-level substring escape. Partial ownership is drift, and a
|
||||
line-level escape waves through exactly the case under review.
|
||||
|
||||
Decisions 1 and 2 without Decision 4 un-consolidate quietly as soon as someone who has not
|
||||
read this ADR touches the area. **The lint must be demonstrated red before it is
|
||||
demonstrated green** — a drift guard that has never failed is not evidence.
|
||||
|
||||
### Decision 5 — an inapplicable sentinel is reported, never silently discarded
|
||||
|
||||
When a fresh sentinel exists but does not apply to a dispatch, the guard states so in its
|
||||
deny reason and names both sides' identifiers.
|
||||
|
||||
This is Postel's Law's "be liberal but *visible*" half, and its absence is why the defect
|
||||
survived three producers and two consumers unnoticed: the guard discarded a fresh sentinel
|
||||
and then denied with a message about registry resolution that never mentioned the sentinel
|
||||
existed. Values interpolated into that message come from a sentinel file and from
|
||||
model-authored prompt text — both untrusted — so each is length-bounded and stripped of
|
||||
control characters first, matching the escaping discipline `phase-plan-index` already
|
||||
applies to a `depends_on` token.
|
||||
|
||||
## Alternatives rejected
|
||||
|
||||
1. **Repair the three absorbed issues at their existing call sites.** This is the pattern
|
||||
that produced them. It leaves the epic open with every symptom gone and CI green — the
|
||||
specific outcome the epic's scoping exists to prevent.
|
||||
2. **Normalize the plan comparison** — strip the phase prefix and slug and compare the
|
||||
middle segment. Invents a cross-namespace equivalence no producer guarantees, and
|
||||
silently restores wrong-value-instead-of-no-value the moment plan-id naming changes.
|
||||
Returning `null` is honest; a heuristic match is not.
|
||||
3. **Carry a structured per-dispatch kwarg through the Agent/Task protocol.** This is the
|
||||
correct end state and `hooks/lib/isolation-sentinel.js` has named it as such since #3045.
|
||||
Rejected *for now* under Gall's Law: it is a 19-runtime descriptor change and an
|
||||
ADR-1239-level negotiation change. The marker is the simple version that works, and
|
||||
growing one into the other is the sanctioned path.
|
||||
4. **Own the format in `src/dispatch-identity.cts`.** Breaks the cold-tree hook load
|
||||
contract, or forces the second mirror this epic exists to delete.
|
||||
5. **Delete `sentinelAppliesToDispatch`'s plan/phase narrowing**, on the grounds that it has
|
||||
never worked so removing it changes nothing observable. It is #3045 SECURITY F2's defence
|
||||
against a stale sentinel from an earlier phase authorizing a later dispatch. The marker
|
||||
makes it work for the first time; deleting it would trade one silent hole for another.
|
||||
|
||||
## Consequences
|
||||
|
||||
**Accepted cost.** `[gsd:dispatch k="v"]` is a homegrown key/value mini-syntax, and
|
||||
Greenspun's Tenth Rule is the standing warning against letting one grow. It is therefore
|
||||
deliberately inert: exactly two recognized keys, flat, no nesting, no conditionals, no
|
||||
variables, no escaping rules beyond quoted values, and unrecognized keys ignored so a later
|
||||
addition cannot break a deployed parser. **Revisit condition, stated so it is actionable:
|
||||
the moment this marker needs a third semantic key or any structure, stop and implement
|
||||
alternative 3 instead.**
|
||||
|
||||
**Accepted limit.** The marker is emitted by a model following a template on BOTH paths —
|
||||
the harness path AND the orchestrator-worktree path. Per
|
||||
`executor-isolation-dispatch.md:131`, the orchestrator-worktree path's `{plan_number}` and
|
||||
`{phase_number}` "are template placeholders, not shell variables," and `{plan_id}` is
|
||||
model-substituted there exactly as on the harness path — there is no shell-built prompt on
|
||||
either path, so the marker is NOT guaranteed on either. A model that drops it degrades to
|
||||
the prose fallback everywhere. That fallback is now correct-or-absent, which is sufficient
|
||||
to resolve #4594 on the phase field alone, but plan-level scoping is exact only when the
|
||||
marker survives. The prose fallback is therefore the real floor on both paths.
|
||||
|
||||
**Hyrum's Law constraint.** The prose sentence `Execute plan X of phase Y` is read by the
|
||||
executor agent itself, not only by the guards. It stays **byte-identical**; the marker is
|
||||
purely additive. Any future change to that sentence is a behavior change to every executor
|
||||
dispatch, not a formatting edit.
|
||||
|
||||
**Out of scope.** The sentinel remains a `cwd`-derived, gitignored file; #3045 SECURITY F3's
|
||||
accepted forgery risk is unchanged. This ADR changes how the decision is transported, not
|
||||
when a degrade should occur.
|
||||
|
||||
## Phase mapping
|
||||
|
||||
| Phase | Delivers | Decisions |
|
||||
|---|---|---|
|
||||
| 1 | `hooks/lib/dispatch-identity.js`; producers emit the marker; both guards parse through the owner; #4594 regression | 1, 3 (grammar mirror), 5 |
|
||||
| 2 | Isolation-vocabulary owner; recorded `IsolationDecision` that a re-query holds; #4222 / #4561 regressions | 2, 3 (vocabulary mirror) |
|
||||
| 3 | `scripts/lint-dispatch-identity-drift.cjs`, demonstrated red then green | 4 |
|
||||
|
||||
Each deliverable is claimed by exactly one phase. The epic stays open until Phase 3 lands.
|
||||
@@ -269,6 +269,7 @@ These govern the system as it stands. Cite these.
|
||||
| [ADR-3806](3806-review-dispositions-ledger.md) | Review Dispositions Ledger canonizes where and how reviews-mode records incorporate/defer decisions in PLAN.md | Accepted | — |
|
||||
| [ADR-4139](4139-compact-content-seam.md) | The compact-content seam — shrink the eager window, never the guarantee | Accepted | — |
|
||||
| [ADR-4593](4593-macos-conformance-tier-architecture.md) | A macOS-specific conformance-tier classifier, separate from the Windows-oriented one | Accepted | — |
|
||||
| [ADR-4630](4630-dispatch-identity-and-isolation-decision-seam.md) | One Canonical Dispatch-Identity Owner and a Recorded Isolation Decision | Accepted | — |
|
||||
| [ADR-4641](4641-windows-selector-consolidation.md) | One Windows test selector, and a proportional ceiling on the conformance tier | Accepted | — |
|
||||
|
||||
### Proposed
|
||||
|
||||
@@ -591,6 +591,8 @@ increases monotonically across waves. `{status}` is `complete` (success),
|
||||
|
||||
Pass paths only — executors read files themselves.
|
||||
|
||||
**Substitute `{plan_id}` in the prompt below with this plan's `id` field** from the `phase-plan-index` JSON loaded in step 1 (the same field referred to elsewhere in this workflow as `plan.id`) — unmodified and un-truncated, never a paraphrase. The guard hooks compare this value verbatim against the sentinel the per-plan gate wrote (`per-plan-worktree-gate.md`'s `plan_id`); a paraphrase or an omission costs the dispatch its recorded isolation decision.
|
||||
|
||||
**Executor routing (#1689/#3370).** Per plan, run `gsd-core/workflows/execute-phase/steps/per-plan-executor-routing.md` to set `EXECUTOR_TYPE` for `subagent_type="{EXECUTOR_TYPE}"` below.
|
||||
|
||||
**TDD-applicability resolution (#4266/#4272).** Run `gsd-core/workflows/execute-phase/steps/tdd-applicability-resolution.md`.
|
||||
@@ -641,6 +643,7 @@ increases monotonically across waves. `{status}` is `complete` (success),
|
||||
prompt="
|
||||
<objective>
|
||||
Execute plan {plan_number} of phase {phase_number}-{phase_name}.
|
||||
[gsd:dispatch phase="{phase_number}" plan="{plan_id}"]
|
||||
Commit each task atomically. Create SUMMARY.md.
|
||||
Do NOT update STATE.md or ROADMAP.md — the orchestrator owns those writes after all worktree agents in the wave complete.
|
||||
</objective>
|
||||
|
||||
@@ -149,8 +149,12 @@ Assign the composed prompt to a shell variable so it can be passed as one argume
|
||||
# An unreadable source file is a halt condition (#3637 fail-closed),
|
||||
# never a skip — a child without these texts is not a gsd-executor.
|
||||
# 2. Substitute this plan's {plan_number}, {phase_number}, {phase_name},
|
||||
# {phase_dir}, and {plan_file} placeholders (same values the harness
|
||||
# path substitutes into its Agent() prompt).
|
||||
# {phase_dir}, {plan_file}, and {plan_id} placeholders (same values the
|
||||
# harness path substitutes into its Agent() prompt). {plan_id} is this
|
||||
# plan's `id` field from the phase-plan-index JSON — the guard hooks
|
||||
# compare it verbatim against the sentinel the per-plan gate wrote, so a
|
||||
# paraphrase or omission costs the dispatch its recorded isolation
|
||||
# decision.
|
||||
# 3. Inline the gsd-executor ROLE DEFINITION: read `agents/gsd-executor.md`
|
||||
# (resolved against the install root the same way the harness runtime
|
||||
# resolves subagent types) and inline it verbatim at the provenance
|
||||
@@ -175,6 +179,7 @@ TDD_APPLICABLE="$_TDD_APPLICABLE_RAW"
|
||||
|
||||
EXECUTOR_PROMPT='<objective>
|
||||
Execute plan {plan_number} of phase {phase_number}-{phase_name}.
|
||||
[gsd:dispatch phase="{phase_number}" plan="{plan_id}"]
|
||||
Commit each task atomically. Create SUMMARY.md.
|
||||
Do NOT update STATE.md or ROADMAP.md — the orchestrator owns those writes after all worktree agents in the wave complete.
|
||||
</objective>
|
||||
|
||||
@@ -63,8 +63,8 @@
|
||||
const fs = require('fs');
|
||||
const path = require('path');
|
||||
const os = require('os');
|
||||
const { readSentinel, VALID_ISOLATION, extractDispatchIdentifiers, sentinelAppliesToDispatch } = require('./lib/isolation-sentinel.js');
|
||||
const { REASON_CODE } = require('./lib/isolation-deny-reason.js');
|
||||
const { readSentinel, VALID_ISOLATION, extractDispatchIdentifiers, sentinelAppliesToDispatch, buildSentinelDiscard } = require('./lib/isolation-sentinel.js');
|
||||
const { REASON_CODE, describeSentinelDiscard } = require('./lib/isolation-deny-reason.js');
|
||||
const { HOOK_ON_CRASH, allow, deny, crash } = require('./lib/hook-exit.js');
|
||||
|
||||
// Required at module top, alongside the other ./lib requires — NOT behind
|
||||
@@ -385,16 +385,20 @@ function resolveIsolationState(cwd, { clock = Date, dispatchIds = null } = {}) {
|
||||
projectExists = false;
|
||||
}
|
||||
if (!projectExists) {
|
||||
return { gsdProject: false, isolation: null, harnessFlag: null, error: null };
|
||||
return { gsdProject: false, isolation: null, harnessFlag: null, error: null, sentinelDiscarded: null };
|
||||
}
|
||||
|
||||
const sentinel = readSentinel(cwd, { clock });
|
||||
if (sentinel.present && !sentinel.stale && sentinelAppliesToDispatch(sentinel, dispatchIds)) {
|
||||
// Hoisted so the "sentinel was present/fresh but did not apply" case below
|
||||
// (#4594 row 15 — Postel's-Law finding) can distinguish itself from
|
||||
// "absent"/"stale" without re-deriving applicability.
|
||||
const applies = sentinelAppliesToDispatch(sentinel, dispatchIds);
|
||||
if (sentinel.present && !sentinel.stale && applies) {
|
||||
if (sentinel.isolation !== 'harness-worktree') {
|
||||
return { gsdProject: true, isolation: sentinel.isolation, harnessFlag: null, error: null };
|
||||
return { gsdProject: true, isolation: sentinel.isolation, harnessFlag: null, error: null, sentinelDiscarded: null };
|
||||
}
|
||||
if (sentinel.harnessFlag) {
|
||||
return { gsdProject: true, isolation: 'harness-worktree', harnessFlag: sentinel.harnessFlag, error: null };
|
||||
return { gsdProject: true, isolation: 'harness-worktree', harnessFlag: sentinel.harnessFlag, error: null, sentinelDiscarded: null };
|
||||
}
|
||||
// #3045 BLOCKER 2 fix: the sentinel already PROVED this dispatch requires
|
||||
// isolation (it resolved harness-worktree) but carries no usable flag —
|
||||
@@ -423,6 +427,7 @@ function resolveIsolationState(cwd, { clock = Date, dispatchIds = null } = {}) {
|
||||
'dispatch-isolation sentinel resolved "harness-worktree" but recorded no harness_flag — ' +
|
||||
'cannot verify what parameter the dispatch must carry.'
|
||||
),
|
||||
sentinelDiscarded: null,
|
||||
};
|
||||
}
|
||||
|
||||
@@ -430,11 +435,19 @@ function resolveIsolationState(cwd, { clock = Date, dispatchIds = null } = {}) {
|
||||
// conservative fallback (#3045 finding — must still cover fail-closed case
|
||||
// (a): a project that opted out of worktrees entirely via
|
||||
// workflow.use_worktrees).
|
||||
//
|
||||
// #4594 row 15: a PRESENT, FRESH sentinel that simply does not apply to
|
||||
// THIS dispatch (identifiers disagree) is a distinct case from "absent" or
|
||||
// "stale" — record what was discarded so evaluateDispatch can name it in a
|
||||
// block reason instead of silently falling through to a registry-resolution
|
||||
// message that never mentions the sentinel existed.
|
||||
const sentinelDiscarded = buildSentinelDiscard(sentinel, dispatchIds);
|
||||
|
||||
try {
|
||||
const { isolation, harnessFlag } = resolveRegistryIsolation(cwd, configPath);
|
||||
return { gsdProject: true, isolation, harnessFlag, error: null };
|
||||
return { gsdProject: true, isolation, harnessFlag, error: null, sentinelDiscarded };
|
||||
} catch (err) {
|
||||
return { gsdProject: true, isolation: null, harnessFlag: null, error: err };
|
||||
return { gsdProject: true, isolation: null, harnessFlag: null, error: err, sentinelDiscarded };
|
||||
}
|
||||
}
|
||||
|
||||
@@ -464,10 +477,16 @@ function evaluateDispatch(data, { clock = Date } = {}) {
|
||||
}
|
||||
|
||||
const cwd = data.cwd || process.cwd();
|
||||
// #3045 SECURITY F2: best-effort plan/phase extraction from this
|
||||
// dispatch's own description text, so a fresh sentinel that disagrees
|
||||
// with THIS dispatch is treated as inapplicable rather than trusted.
|
||||
const dispatchIds = extractDispatchIdentifiers(toolInput.description);
|
||||
// #3045 SECURITY F2 / #4594: best-effort plan/phase extraction from this
|
||||
// dispatch's own text, so a fresh sentinel that disagrees with THIS
|
||||
// dispatch is treated as inapplicable rather than trusted. PROMPT FIRST:
|
||||
// `description` is short, model-authored free text that only carries usable
|
||||
// identity when the model happens to reproduce the dispatch template
|
||||
// verbatim, while the canonical `[gsd:dispatch phase="…" plan="…"]` marker
|
||||
// (or, failing that, the prose fallback) lives in the prompt body itself —
|
||||
// `description` is kept only as a fallback for a marker/prose match that
|
||||
// exists solely in it.
|
||||
const dispatchIds = extractDispatchIdentifiers(toolInput.prompt, toolInput.description);
|
||||
const state = resolveIsolationState(cwd, { clock, dispatchIds });
|
||||
|
||||
if (!state.gsdProject) return { action: 'allow' };
|
||||
@@ -493,7 +512,8 @@ function evaluateDispatch(data, { clock = Date } = {}) {
|
||||
`required — a guard that cannot verify must not answer "safe" (#3050). Retry once the ` +
|
||||
`project configuration is readable.`;
|
||||
const reasonCode = isBuildFailure ? REASON_CODE.RUNTIME_BUILD_FAILED : REASON_CODE.CONFIG_UNREADABLE;
|
||||
return { action: 'block', reason, reasonCode };
|
||||
const fullReason = state.sentinelDiscarded ? reason + describeSentinelDiscard(state.sentinelDiscarded) : reason;
|
||||
return { action: 'block', reason: fullReason, reasonCode, sentinelDiscarded: state.sentinelDiscarded };
|
||||
}
|
||||
|
||||
if (state.isolation !== 'harness-worktree') return { action: 'allow' };
|
||||
@@ -503,13 +523,14 @@ function evaluateDispatch(data, { clock = Date } = {}) {
|
||||
|
||||
if (toolInput[parsed.param] === parsed.value) return { action: 'allow' };
|
||||
|
||||
const reason =
|
||||
let reason =
|
||||
`Agent isolation guard: this project's dispatch isolation resolves to "harness-worktree", ` +
|
||||
`but the Agent() dispatch for subagent_type="${subagentType}" is missing ` +
|
||||
`${parsed.param}="${parsed.value}". Add ${parsed.param}="${parsed.value}" to the Agent() ` +
|
||||
`call so the executor runs in an isolated worktree instead of the primary checkout ` +
|
||||
`(gsd-core/workflows/execute-phase/steps/executor-isolation-dispatch.md).`;
|
||||
return { action: 'block', reason, reasonCode: REASON_CODE.HARNESS_FLAG_MISSING };
|
||||
if (state.sentinelDiscarded) reason += describeSentinelDiscard(state.sentinelDiscarded);
|
||||
return { action: 'block', reason, reasonCode: REASON_CODE.HARNESS_FLAG_MISSING, sentinelDiscarded: state.sentinelDiscarded };
|
||||
}
|
||||
|
||||
/* istanbul ignore next -- stdin adapter, exercised via spawnSync in tests */
|
||||
@@ -524,7 +545,12 @@ function main() {
|
||||
const data = JSON.parse(input);
|
||||
const decision = evaluateDispatch(data);
|
||||
if (decision.action === 'block') {
|
||||
const out = { decision: 'block', reason: decision.reason, reason_code: decision.reasonCode };
|
||||
const out = {
|
||||
decision: 'block',
|
||||
reason: decision.reason,
|
||||
reason_code: decision.reasonCode,
|
||||
sentinel_discarded: decision.sentinelDiscarded ?? null,
|
||||
};
|
||||
// Kimi feeds stderr (not stdout) back to the model on exit 2.
|
||||
deny(out, decision.reason);
|
||||
}
|
||||
|
||||
@@ -62,8 +62,8 @@ const { allow } = require('./lib/hook-exit.js');
|
||||
// hooks/lib/cursor-workspace.js. Staged next to these scripts by
|
||||
// writeCursorHooksJson so the require always resolves post-install.
|
||||
const { resolveStatePath } = require('./lib/cursor-workspace.js');
|
||||
const { readSentinel, VALID_ISOLATION, extractDispatchIdentifiers, sentinelAppliesToDispatch } = require('./lib/isolation-sentinel.js');
|
||||
const { REASON_CODE } = require('./lib/isolation-deny-reason.js');
|
||||
const { readSentinel, VALID_ISOLATION, extractDispatchIdentifiers, sentinelAppliesToDispatch, buildSentinelDiscard } = require('./lib/isolation-sentinel.js');
|
||||
const { REASON_CODE, describeSentinelDiscard } = require('./lib/isolation-deny-reason.js');
|
||||
// #3582: gsd-core/bin/lib/*.cjs (runtime-homes.cjs, worktree-safety.cjs,
|
||||
// runtime-name-policy.cjs, capability-registry.cjs — required below, inside
|
||||
// resolveIsolationEvidence and resolveFallbackIsolation) are tsc build
|
||||
@@ -491,18 +491,25 @@ function evaluateRootIsolation(root, subagentType, { clock = Date, dispatchIds =
|
||||
`Refusing to allow this subagent to spawn until the runtime library is built — a guard ` +
|
||||
`that cannot verify must not answer "safe" (#3050).`,
|
||||
reasonCode: REASON_CODE.RUNTIME_BUILD_FAILED,
|
||||
sentinelDiscarded: null,
|
||||
};
|
||||
}
|
||||
|
||||
// #3045 BLOCKER fix: a fresh sentinel is authoritative for THIS dispatch's
|
||||
// actual resolved isolation — see the doc comment above.
|
||||
// #3045 SECURITY F2: a fresh sentinel that names a DIFFERENT plan/phase
|
||||
// than this dispatch is not applicable to it — fall through to the
|
||||
// conservative fallback exactly as a stale sentinel would.
|
||||
// Hoisted (readSentinel never throws) so the "present, fresh, but did not
|
||||
// apply" case (#4594 row 15) can be reported on every deny path below
|
||||
// instead of silently discarded.
|
||||
const sentinel = readSentinel(root, { clock });
|
||||
const applies = sentinelAppliesToDispatch(sentinel, dispatchIds);
|
||||
const sentinelDiscarded = buildSentinelDiscard(sentinel, dispatchIds);
|
||||
|
||||
let declaredIsolation;
|
||||
try {
|
||||
// #3045 BLOCKER fix: a fresh sentinel is authoritative for THIS
|
||||
// dispatch's actual resolved isolation — see the doc comment above.
|
||||
// #3045 SECURITY F2: a fresh sentinel that names a DIFFERENT
|
||||
// plan/phase than this dispatch is not applicable to it — fall through
|
||||
// to the conservative fallback exactly as a stale sentinel would.
|
||||
const sentinel = readSentinel(root, { clock });
|
||||
declaredIsolation = (sentinel.present && !sentinel.stale && sentinelAppliesToDispatch(sentinel, dispatchIds))
|
||||
declaredIsolation = (sentinel.present && !sentinel.stale && applies)
|
||||
? sentinel.isolation
|
||||
: resolveFallbackIsolation(root, configPath);
|
||||
} catch {
|
||||
@@ -513,8 +520,10 @@ function evaluateRootIsolation(root, subagentType, { clock = Date, dispatchIds =
|
||||
`dispatch-isolation configuration ('.planning/config.json' exists under "${root}"). ` +
|
||||
`Refusing to allow this subagent to spawn without being able to verify whether ` +
|
||||
`isolation is required — a guard that cannot verify must not answer "safe" (#3050). ` +
|
||||
`Retry once the project configuration is readable.`,
|
||||
`Retry once the project configuration is readable.` +
|
||||
(sentinelDiscarded ? describeSentinelDiscard(sentinelDiscarded) : ''),
|
||||
reasonCode: REASON_CODE.CONFIG_UNREADABLE,
|
||||
sentinelDiscarded,
|
||||
};
|
||||
}
|
||||
|
||||
@@ -530,8 +539,10 @@ function evaluateRootIsolation(root, subagentType, { clock = Date, dispatchIds =
|
||||
`GSD subagent isolation guard: this project's dispatch isolation resolves to ` +
|
||||
`"harness-worktree", but the subagentStart payload for this dispatch carries no usable ` +
|
||||
`subagent_type. Refusing to allow it to spawn without being able to confirm whether it ` +
|
||||
`is a GSD executor — a guard that cannot verify must not answer "safe" (#3050).`,
|
||||
`is a GSD executor — a guard that cannot verify must not answer "safe" (#3050).` +
|
||||
(sentinelDiscarded ? describeSentinelDiscard(sentinelDiscarded) : ''),
|
||||
reasonCode: REASON_CODE.NO_SUBAGENT_TYPE,
|
||||
sentinelDiscarded,
|
||||
};
|
||||
}
|
||||
|
||||
@@ -547,8 +558,10 @@ function evaluateRootIsolation(root, subagentType, { clock = Date, dispatchIds =
|
||||
`"harness-worktree", but whether "${root}" is running in an isolated Cursor worktree ` +
|
||||
`could not be determined (git did not respond). Refusing to allow subagent_type=` +
|
||||
`"${subagentType}" to spawn without being able to verify isolation — a guard that ` +
|
||||
`cannot verify must not answer "safe" (#3050). Retry once git is responsive.`,
|
||||
`cannot verify must not answer "safe" (#3050). Retry once git is responsive.` +
|
||||
(sentinelDiscarded ? describeSentinelDiscard(sentinelDiscarded) : ''),
|
||||
reasonCode: REASON_CODE.CANNOT_DETERMINE_ISOLATION,
|
||||
sentinelDiscarded,
|
||||
};
|
||||
}
|
||||
|
||||
@@ -560,8 +573,10 @@ function evaluateRootIsolation(root, subagentType, { clock = Date, dispatchIds =
|
||||
`which is not an isolated Cursor worktree — it would edit the user's primary checkout ` +
|
||||
`directly, with no consent and no warning. Start an isolated session first (the ` +
|
||||
`"--worktree" CLI flag or the "/worktree" chat command; Cursor manages these worktrees ` +
|
||||
`under "~/.cursor/worktrees/") and retry.`,
|
||||
`under "~/.cursor/worktrees/") and retry.` +
|
||||
(sentinelDiscarded ? describeSentinelDiscard(sentinelDiscarded) : ''),
|
||||
reasonCode: REASON_CODE.NOT_ISOLATED_WORKTREE,
|
||||
sentinelDiscarded,
|
||||
};
|
||||
}
|
||||
|
||||
@@ -611,7 +626,12 @@ function main() {
|
||||
decision = { action: 'allow' };
|
||||
}
|
||||
if (decision.action === 'deny') {
|
||||
const out = { permission: 'deny', user_message: decision.reason, reason_code: decision.reasonCode };
|
||||
const out = {
|
||||
permission: 'deny',
|
||||
user_message: decision.reason,
|
||||
reason_code: decision.reasonCode,
|
||||
sentinel_discarded: decision.sentinelDiscarded ?? null,
|
||||
};
|
||||
if (additionalContext !== null) out.additional_context = additionalContext;
|
||||
process.stdout.write(JSON.stringify(out));
|
||||
return;
|
||||
|
||||
187
hooks/lib/dispatch-identity.js
Normal file
187
hooks/lib/dispatch-identity.js
Normal file
@@ -0,0 +1,187 @@
|
||||
'use strict';
|
||||
// hooks/lib/dispatch-identity.js — the ONE canonical owner of the
|
||||
// `[gsd:dispatch phase="…" plan="…"]` marker format and its prose fallback
|
||||
// (#4594, epic #4630 Phase 1). See
|
||||
// `.gsd/phase/fix-4594-dispatch-identity-seam/40-design.md` for the full
|
||||
// rationale (behavior table, negative space, rejected alternatives).
|
||||
//
|
||||
// COLD-LOAD CONSTRAINT (load-bearing, not a style choice): this module MUST
|
||||
// require NOTHING — no `fs`, no `path`, and above all nothing under
|
||||
// `gsd-core/bin/lib/` or `ensure-runtime-build`. The guard hooks that consume
|
||||
// this module must load on a raw plugin-marketplace install where the
|
||||
// compiled lib is absent and the self-healing build seam has not run — a
|
||||
// hook that dies at module load is worse than one carrying a mirror. See
|
||||
// `tests/dispatch-identity.test.cjs`'s "cold tree load" test, which asserts
|
||||
// this by monkeypatching `Module._load` to throw on exactly those paths.
|
||||
|
||||
// DISPATCH_PHASE_TOKEN_SOURCE is a DELIBERATE MIRROR of
|
||||
// `CASE_FLEXIBLE_PHASE_NUMBER_TOKEN_SOURCE` in `src/phase-id.cts`
|
||||
// (ADR-2121 owns the phase-token grammar; `gsd-core/bin/lib/phase-id.cjs` is
|
||||
// its compiled form). It cannot be an `require()`-based import: importing the
|
||||
// compiled lib here would violate the cold-load constraint above (either a
|
||||
// direct dependency on `gsd-core/bin/lib/` or a forced self-heal via
|
||||
// `ensure-runtime-build`), which is exactly the failure mode this module
|
||||
// exists to avoid for the guard hooks that consume it.
|
||||
//
|
||||
// Because this is a hand-copied mirror and not a shared reference, a
|
||||
// hand-edited grammar change on one side that is not mirrored on the other
|
||||
// does NOT fail loudly — both sides remain independently valid regex
|
||||
// sources, so the failure mode is a silent, invisible non-match (a dispatch
|
||||
// whose phase token the two owners now parse differently), never a thrown
|
||||
// error. `tests/dispatch-identity.test.cjs`'s "templates: prose token source
|
||||
// matches the case-flexible phase-id grammar" test pins this string equal to
|
||||
// the compiled source at test time specifically to turn that silent drift
|
||||
// into a loud, in-CI failure.
|
||||
const DISPATCH_PHASE_TOKEN_SOURCE = '\\d+[A-Za-z]?(?:\\.\\d+)*';
|
||||
|
||||
// Bounded marker grammar: `[gsd:dispatch key="value" key2="value2"]`.
|
||||
// - Key names bounded to {1,31} — no key we emit or expect is anywhere near
|
||||
// that long; this is purely a backstop against pathological input.
|
||||
// - Values bounded to {0,200} and forbidden from containing `"`, `]`, or any
|
||||
// control character that could either close the marker early or forge a
|
||||
// sibling key — enforced structurally by the negated character class, not
|
||||
// by a separate validation pass.
|
||||
// Global flag so `findMarker` can scan forward through a large prompt.
|
||||
const MARKER_RE = /\[gsd:dispatch((?:\s+[A-Za-z][A-Za-z0-9_-]{0,31}="[^"\]\r\n]{0,200}")*)\s*\]/g;
|
||||
const MARKER_KV_RE = /([A-Za-z][A-Za-z0-9_-]{0,31})="([^"\]\r\n]{0,200})"/g;
|
||||
|
||||
// Prose fallback frame: `execute plan <token> of phase <PHASE TOKEN>`.
|
||||
// - Case-insensitive, `\s+` throughout so CRLF (and any run of whitespace)
|
||||
// parses identically to LF.
|
||||
// - The plan token is bounded (`\S{1,80}`) but its VALUE is deliberately
|
||||
// discarded by the caller (see `parseDispatchIdentity` below) — captured
|
||||
// only so the phase token can be anchored correctly after it.
|
||||
// - The phase token is bounded by `DISPATCH_PHASE_TOKEN_SOURCE`, replacing
|
||||
// the old greedy `(\S+)`, so a directory-name suffix (`-auth`) or a
|
||||
// sentence-terminating period is never swept into the token.
|
||||
const PROSE_RE = new RegExp(
|
||||
`execute\\s+plan\\s+(\\S{1,80})\\s+of\\s+phase\\s+(${DISPATCH_PHASE_TOKEN_SOURCE})`,
|
||||
'i',
|
||||
);
|
||||
|
||||
/**
|
||||
* Render the `[gsd:dispatch phase="…" plan="…"]` marker for a producer to
|
||||
* embed verbatim in a dispatch description/prompt. Never throws.
|
||||
*
|
||||
* Emits only keys whose value is a non-empty string not containing `"`, `]`,
|
||||
* `\r`, or `\n` — such a value is UNUSABLE and that key is omitted entirely,
|
||||
* because embedding it could close the marker early and forge a second,
|
||||
* attacker-controlled field. Key order is always `phase` then `plan`.
|
||||
* Returns `''` when neither key is usable (nothing worth emitting).
|
||||
*/
|
||||
function renderDispatchIdentityMarker(input) {
|
||||
const value = input && typeof input === 'object' ? input : {};
|
||||
const parts = [];
|
||||
for (const key of ['phase', 'plan']) {
|
||||
const v = value[key];
|
||||
if (typeof v === 'string' && v.length > 0 && !/["\]\r\n]/.test(v)) {
|
||||
parts.push(`${key}="${v}"`);
|
||||
}
|
||||
}
|
||||
if (parts.length === 0) return '';
|
||||
return `[gsd:dispatch ${parts.join(' ')}]`;
|
||||
}
|
||||
|
||||
function emptyResult() {
|
||||
return { phase: null, plan: null, source: null };
|
||||
}
|
||||
|
||||
/**
|
||||
* Scan `texts` in order for the first well-formed marker that yields at
|
||||
* least one recognized key (`phase` and/or `plan`). Returns `{ phase, plan }`
|
||||
* (one of the two possibly null, never both) or `null` if no QUALIFYING
|
||||
* marker was found in any text. Unrecognized keys inside a marker are
|
||||
* ignored (forward compatibility — a later `run=`/`wave=` key must not break
|
||||
* a deployed parser).
|
||||
*
|
||||
* #4594 F1 fix: a marker that matches `MARKER_RE` but carries neither
|
||||
* `phase=` nor `plan=` (e.g. only unrecognized keys, or an empty kv block)
|
||||
* is NOT treated as "found" — it is skipped and scanning continues (later
|
||||
* markers in the same text, then subsequent texts), falling through to the
|
||||
* prose fallback if nothing qualifying turns up. Without this, prompt text
|
||||
* that merely CONTAINS the literal marker syntax with no usable identifiers
|
||||
* silently suppressed the prose fallback entirely, since the old
|
||||
* implementation returned on the first syntactic match regardless of
|
||||
* content.
|
||||
*/
|
||||
function findMarker(texts) {
|
||||
for (const text of texts) {
|
||||
if (typeof text !== 'string' || text.length === 0) continue;
|
||||
MARKER_RE.lastIndex = 0;
|
||||
let match;
|
||||
while ((match = MARKER_RE.exec(text)) !== null) {
|
||||
const kvBlock = match[1] || '';
|
||||
let phase = null;
|
||||
let plan = null;
|
||||
MARKER_KV_RE.lastIndex = 0;
|
||||
let kv;
|
||||
while ((kv = MARKER_KV_RE.exec(kvBlock)) !== null) {
|
||||
const [, key, val] = kv;
|
||||
if (key === 'phase' && phase === null) phase = val;
|
||||
else if (key === 'plan' && plan === null) plan = val;
|
||||
}
|
||||
if (phase === null && plan === null) continue;
|
||||
return { phase, plan };
|
||||
}
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Scan `texts` in order for the first `execute plan <token> of phase
|
||||
* <PHASE TOKEN>` frame. Returns `{ phase }` or `null`.
|
||||
*
|
||||
* The prose fallback returns `plan: null` DELIBERATELY (enforced by the
|
||||
* caller, not here): the prose plan token (e.g. `02`) is a bare in-phase
|
||||
* plan number, a different namespace from the sentinel's `plan_id` (e.g.
|
||||
* `03-02-hardening`, which is phase-prefixed AND slugged). Reporting the
|
||||
* prose plan token as the parsed `plan` is exactly the false-mismatch bug
|
||||
* this module exists to fix — an absent value is honestly "cannot compare",
|
||||
* while a wrong value silently forges a mismatch on every dispatch. Do not
|
||||
* "fix" this by threading the plan token through — that was the bug.
|
||||
*/
|
||||
function findProse(texts) {
|
||||
for (const text of texts) {
|
||||
if (typeof text !== 'string' || text.length === 0) continue;
|
||||
const match = PROSE_RE.exec(text);
|
||||
if (match) {
|
||||
return { phase: match[2] };
|
||||
}
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Parse dispatch identity out of one or more texts (a short description, a
|
||||
* full prompt body, etc). Never throws for any input type; non-string
|
||||
* entries are skipped.
|
||||
*
|
||||
* Pass 1: scan all texts in order for a well-formed marker; the first one
|
||||
* wins outright (marker beats prose even if prose appears earlier in the
|
||||
* same text — see the design doc's row 8).
|
||||
* Pass 2 (only if no marker was found anywhere): scan all texts in order for
|
||||
* the prose frame; the first match wins. `plan` is always `null` from this
|
||||
* path (see `findProse`'s doc comment).
|
||||
*
|
||||
* Returns `{ phase, plan, source }` where `source` is `'marker' | 'prose' |
|
||||
* null`.
|
||||
*/
|
||||
function parseDispatchIdentity(...texts) {
|
||||
const marker = findMarker(texts);
|
||||
if (marker) {
|
||||
return { phase: marker.phase, plan: marker.plan, source: 'marker' };
|
||||
}
|
||||
|
||||
const prose = findProse(texts);
|
||||
if (prose) {
|
||||
return { phase: prose.phase, plan: null, source: 'prose' };
|
||||
}
|
||||
|
||||
return emptyResult();
|
||||
}
|
||||
|
||||
module.exports = {
|
||||
DISPATCH_PHASE_TOKEN_SOURCE,
|
||||
renderDispatchIdentityMarker,
|
||||
parseDispatchIdentity,
|
||||
};
|
||||
@@ -36,4 +36,56 @@ const REASON_CODE = Object.freeze({
|
||||
NOT_ISOLATED_WORKTREE: 'not_isolated_worktree',
|
||||
});
|
||||
|
||||
module.exports = { REASON_CODE };
|
||||
// #4594 row 15 / F2: values interpolated into a deny reason (sentinel/dispatch
|
||||
// phase and plan) come from a sentinel file on disk and, transitively, from
|
||||
// model-authored prompt text, neither of which is trusted — bound length and
|
||||
// strip control characters/newlines so a crafted value cannot forge extra
|
||||
// lines or otherwise inject content into the guard's stdout/stderr message
|
||||
// (same discipline as escaping an untrusted token before embedding it in a
|
||||
// message, e.g. phase-plan-index's `depends_on` warning).
|
||||
//
|
||||
// Originally duplicated byte-for-byte in hooks/gsd-agent-isolation-guard.js
|
||||
// and hooks/gsd-cursor-subagent-start.js (#4594 F2 review finding) — both
|
||||
// hooks already require this dependency-free module, so there is no
|
||||
// cold-load justification for the duplication the way there is for
|
||||
// hooks/lib/dispatch-identity.js's grammar mirror. Moved here as the single
|
||||
// owner; both hooks now import it.
|
||||
const REASON_INTERPOLATION_MAX_LEN = 64;
|
||||
|
||||
// F6: also strip Unicode line/paragraph separators (U+2028/U+2029) and the
|
||||
// bidi-override/isolate control characters (U+202A-U+202E, U+2066-U+2069) —
|
||||
// none of these are in `[\x00-\x1f\x7f]`, so a crafted sentinel or dispatch
|
||||
// value carrying them could still visually reflow the deny message onto a
|
||||
// new line or reverse/hide part of it despite the ASCII control-char strip.
|
||||
const REASON_UNSAFE_CHARS_RE = new RegExp(
|
||||
// eslint-disable-next-line no-control-regex -- deliberately stripping control chars/newlines
|
||||
'[\\x00-\\x1f\\x7f\\u2028\\u2029\\u202a-\\u202e\\u2066-\\u2069]',
|
||||
'g',
|
||||
);
|
||||
|
||||
function sanitizeForReason(value) {
|
||||
if (typeof value !== 'string' || value.length === 0) return '(none)';
|
||||
const stripped = value.replace(REASON_UNSAFE_CHARS_RE, '');
|
||||
return stripped.length > REASON_INTERPOLATION_MAX_LEN
|
||||
? `${stripped.slice(0, REASON_INTERPOLATION_MAX_LEN)}…`
|
||||
: stripped;
|
||||
}
|
||||
|
||||
function describeSentinelDiscard(sentinelDiscarded) {
|
||||
const sentinelPhase = sanitizeForReason(sentinelDiscarded.sentinel.phase);
|
||||
const sentinelPlan = sanitizeForReason(sentinelDiscarded.sentinel.plan);
|
||||
const dispatchPhase = sanitizeForReason(sentinelDiscarded.dispatch.phase);
|
||||
const dispatchPlan = sanitizeForReason(sentinelDiscarded.dispatch.plan);
|
||||
return (
|
||||
` A fresh dispatch-isolation sentinel was present but did not apply to this dispatch ` +
|
||||
`(sentinel phase="${sentinelPhase}" plan="${sentinelPlan}"; dispatch phase="${dispatchPhase}" ` +
|
||||
`plan="${dispatchPlan}"), so it was not consulted.`
|
||||
);
|
||||
}
|
||||
|
||||
module.exports = {
|
||||
REASON_CODE,
|
||||
REASON_INTERPOLATION_MAX_LEN,
|
||||
sanitizeForReason,
|
||||
describeSentinelDiscard,
|
||||
};
|
||||
|
||||
@@ -56,6 +56,7 @@
|
||||
|
||||
const fs = require('fs');
|
||||
const path = require('path');
|
||||
const { parseDispatchIdentity } = require('./dispatch-identity.js');
|
||||
|
||||
// Isolation modes ADR-1239 declares (mirrors gsd-tools.cjs
|
||||
// routeDispatchIsolation / routeRecordDispatchIsolation).
|
||||
@@ -223,26 +224,40 @@ function readSentinel(cwd, { clock = Date } = {}) {
|
||||
}
|
||||
|
||||
/**
|
||||
* #3045 SECURITY F2: extract the `{plan, phase}` a specific Agent()/Task()
|
||||
* dispatch is FOR, from the one place that data is reliably embedded today —
|
||||
* the dispatch prompt/description text (`execute-phase.md`'s Agent() block
|
||||
* uses the literal shape `description="Execute plan {plan_number} of phase
|
||||
* {phase_number}"`, and the prompt body's `<objective>` repeats "Execute plan
|
||||
* {plan_number} of phase {phase_number}-{phase_name}." verbatim — the SAME
|
||||
* text the orchestrator-worktree EXECUTOR_PROMPT template and Cursor's `task`
|
||||
* field carry, since Cursor dispatches the same prompt content). There is no
|
||||
* structured per-dispatch kwarg carrying plan/phase identifiers today (#3045
|
||||
* would need a larger dispatch-protocol change to add one) — this is
|
||||
* therefore a best-effort, NOT a guaranteed, extraction: a dispatch whose
|
||||
* text doesn't match the expected shape returns `{ plan: null, phase: null }`
|
||||
* and the caller must NOT treat that as a mismatch (see
|
||||
* `sentinelAppliesToDispatch`).
|
||||
* #3045 SECURITY F2 / #4594: extract the `{plan, phase}` a specific
|
||||
* Agent()/Task() dispatch is FOR. Variadic — accepts any number of text
|
||||
* sources (short description, full prompt body, etc) and delegates to
|
||||
* `hooks/lib/dispatch-identity.js::parseDispatchIdentity`, the one canonical
|
||||
* owner of both the `[gsd:dispatch phase="…" plan="…"]` marker format and its
|
||||
* prose fallback (see `.gsd/phase/fix-4594-dispatch-identity-seam/40-design.md`).
|
||||
*
|
||||
* MARKER-FIRST CONTRACT: producers embed a structured marker carrying the
|
||||
* exact shell values the sentinel itself records (`$PHASE_NUMBER`,
|
||||
* `$plan_id`), so producer and consumer agree by construction, independent
|
||||
* of how the prose reads or whether a model paraphrases the dispatch
|
||||
* sentence. Only when no marker is found anywhere in the supplied texts does
|
||||
* this fall back to scanning for the prose frame "execute plan <token> of
|
||||
* phase <PHASE TOKEN>".
|
||||
*
|
||||
* The prose fallback is now CORRECT-OR-ABSENT rather than possibly-wrong:
|
||||
* the phase token is bounded by the same grammar `src/phase-id.cts` owns
|
||||
* (ADR-2121), so a directory-name slug or trailing punctuation can no longer
|
||||
* leak into the phase value, and the prose plan token is never reported at
|
||||
* all (it lives in a different namespace than the sentinel's phase-prefixed,
|
||||
* slugged `plan_id` — reporting it was the #4594 false-mismatch bug).
|
||||
*
|
||||
* This is still a best-effort, NOT a guaranteed, extraction: a dispatch that
|
||||
* carries neither a marker nor a matching prose frame in ANY supplied text
|
||||
* returns `{ plan: null, phase: null }`, and the caller MUST NEVER treat
|
||||
* that as a mismatch — see `sentinelAppliesToDispatch`, whose whole
|
||||
* contract depends on "missing" and "wrong" being distinguishable.
|
||||
*
|
||||
* Returns only the two-field `{ plan, phase }` shape existing callers
|
||||
* depend on — `parseDispatchIdentity`'s `source` field is discarded here.
|
||||
*/
|
||||
function extractDispatchIdentifiers(text) {
|
||||
if (typeof text !== 'string' || text.length === 0) return { plan: null, phase: null };
|
||||
const m = /execute\s+plan\s+(\S+)\s+of\s+phase\s+(\S+)/i.exec(text);
|
||||
if (!m) return { plan: null, phase: null };
|
||||
return { plan: m[1], phase: m[2] };
|
||||
function extractDispatchIdentifiers(...texts) {
|
||||
const { phase, plan } = parseDispatchIdentity(...texts);
|
||||
return { plan, phase };
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -265,6 +280,29 @@ function sentinelAppliesToDispatch(sentinel, dispatchIds) {
|
||||
return true;
|
||||
}
|
||||
|
||||
/**
|
||||
* #4594 F3: build the structured "a fresh sentinel was present but did not
|
||||
* apply to this dispatch" descriptor, mirroring the exact comparison
|
||||
* `sentinelAppliesToDispatch` performs. Returns `null` when the sentinel is
|
||||
* absent, stale, malformed, or DOES apply — i.e. exactly when there is
|
||||
* nothing to report as discarded. Otherwise returns the nested
|
||||
* `{ sentinel: {phase, plan}, dispatch: {phase, plan} }` shape, reusing the
|
||||
* `{phase, plan}` pair already flowing through this module end to end rather
|
||||
* than renaming its fields into an ad hoc `sentinelPhase`/`dispatchPlan` bag
|
||||
* (previously rebuilt identically at two call sites in the guard hooks).
|
||||
*/
|
||||
function buildSentinelDiscard(sentinel, dispatchIds) {
|
||||
if (!sentinel || !sentinel.present || sentinel.stale) return null;
|
||||
if (sentinelAppliesToDispatch(sentinel, dispatchIds)) return null;
|
||||
return {
|
||||
sentinel: { phase: sentinel.phase ?? null, plan: sentinel.plan ?? null },
|
||||
dispatch: {
|
||||
phase: dispatchIds ? (dispatchIds.phase ?? null) : null,
|
||||
plan: dispatchIds ? (dispatchIds.plan ?? null) : null,
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
module.exports = {
|
||||
VALID_ISOLATION,
|
||||
SENTINEL_RELATIVE_PATH,
|
||||
@@ -274,4 +312,5 @@ module.exports = {
|
||||
readSentinel,
|
||||
extractDispatchIdentifiers,
|
||||
sentinelAppliesToDispatch,
|
||||
buildSentinelDiscard,
|
||||
};
|
||||
|
||||
@@ -68,6 +68,7 @@ module.exports = {
|
||||
"tests/cursor-hook-workspace-roots.test.cjs",
|
||||
"tests/cursor-hooks.test.cjs",
|
||||
"tests/cursor-subagent-isolation.test.cjs",
|
||||
"tests/dispatch-identity.test.cjs",
|
||||
"tests/dispatcher.test.cjs",
|
||||
"tests/drift-detection.test.cjs",
|
||||
"tests/effort-surface-axis.test.cjs",
|
||||
|
||||
@@ -854,6 +854,31 @@ describe('gsd-cursor-subagent-start.js: #3045 SECURITY F2 — sentinel bound to
|
||||
});
|
||||
});
|
||||
|
||||
describe('gsd-cursor-subagent-start.js: #4594 row 30 — sentinel-discard reporting parity with the Claude guard', () => {
|
||||
let harnessProject;
|
||||
|
||||
before(() => {
|
||||
harnessProject = makeGitProject('gsd-cs-4594-', JSON.stringify({ runtime: 'cursor' }));
|
||||
});
|
||||
|
||||
after(() => {
|
||||
cleanup(harnessProject);
|
||||
});
|
||||
|
||||
test('row 30: `data.task` in prose form + a fresh matching {isolation:"none"} sentinel -> ALLOW (was: discarded and denied)', (t) => {
|
||||
// Measured production shape: sentinel plan is phase-prefixed-and-slugged
|
||||
// (phase-plan-index's `plans[].id`); the prose fallback never reports a
|
||||
// plan (see hooks/lib/dispatch-identity.js's findProse doc comment), so
|
||||
// only phase is compared here — exactly the real-world match path.
|
||||
writeSentinel(harnessProject, { isolation: 'none', phase: '03', plan: '03-02-hardening' });
|
||||
t.after(() => cleanup(path.join(harnessProject, '.gsd')));
|
||||
const r = runHook(subagentPayload([harnessProject], { task: 'Execute plan 02 of phase 03-auth.' }));
|
||||
assert.equal(r.status, 0, `stdout: ${r.stdout} stderr: ${r.stderr}`);
|
||||
const out = JSON.parse(r.stdout);
|
||||
assert.equal(out.permission, undefined, `expected allow, got: ${r.stdout}`);
|
||||
});
|
||||
});
|
||||
|
||||
describe('gsd-cursor-subagent-start.js: #3045 MAJOR — clock seam boundary coverage (in-process, no subprocess wall-clock race)', () => {
|
||||
const cursorHookModule = require('../hooks/gsd-cursor-subagent-start.js');
|
||||
|
||||
|
||||
445
tests/dispatch-identity.test.cjs
Normal file
445
tests/dispatch-identity.test.cjs
Normal file
@@ -0,0 +1,445 @@
|
||||
'use strict';
|
||||
|
||||
/**
|
||||
* Tests for the (not-yet-created) `hooks/lib/dispatch-identity.js` — the one
|
||||
* canonical owner of the `[gsd:dispatch phase="…" plan="…"]` marker format
|
||||
* and its prose fallback (#4594, epic #4630 Phase 1).
|
||||
*
|
||||
* See `.gsd/phase/fix-4594-dispatch-identity-seam/40-design.md` (behavior
|
||||
* table + negative space) and `50-test-matrix.md` (row -> test-name mapping)
|
||||
* for the full rationale. This file intentionally covers matrix rows 1-27
|
||||
* and 33-34 only; rows 28-32 are guard end-to-end rows that belong in
|
||||
* `tests/gsd-agent-isolation-guard.test.cjs` / `tests/cursor-subagent-isolation.test.cjs`.
|
||||
*
|
||||
* RED BY DESIGN: `hooks/lib/dispatch-identity.js` does not exist yet. This
|
||||
* whole file fails to load with MODULE_NOT_FOUND until that module is
|
||||
* written — that failure is the expected, correct state of this commit.
|
||||
*/
|
||||
|
||||
const { test, describe } = require('node:test');
|
||||
const assert = require('node:assert/strict');
|
||||
const fs = require('node:fs');
|
||||
const path = require('node:path');
|
||||
const { execFileSync } = require('node:child_process');
|
||||
const fc = require('fast-check');
|
||||
const { GENERATOR_SCRIPT_TIMEOUT_MS } = require('./helpers/timeouts.cjs');
|
||||
|
||||
const {
|
||||
DISPATCH_PHASE_TOKEN_SOURCE,
|
||||
renderDispatchIdentityMarker,
|
||||
parseDispatchIdentity,
|
||||
} = require('../hooks/lib/dispatch-identity.js');
|
||||
|
||||
const {
|
||||
extractDispatchIdentifiers,
|
||||
sentinelAppliesToDispatch,
|
||||
} = require('../hooks/lib/isolation-sentinel.js');
|
||||
|
||||
const REPO_ROOT = path.resolve(__dirname, '..');
|
||||
|
||||
// Measured production shapes (40-design.md): the sentinel's own {phase, plan}
|
||||
// as recorded by a real `phase-plan-index` run in this worktree, and the two
|
||||
// verbatim dispatch-prose templates that embed the phase/plan numbers.
|
||||
const MEASURED_SENTINEL = { phase: '03', plan: '03-02-hardening' };
|
||||
const DESCRIPTION_FORM = 'Execute plan 02 of phase 03';
|
||||
const PROMPT_BODY_FORM = 'Execute plan 02 of phase 03-auth.';
|
||||
|
||||
/**
|
||||
* Row 33's cold-tree probe: a single `node -e` script, spawned directly (no
|
||||
* fan-out), doing an in-process require plus two synchronous calls against a
|
||||
* small fixture path. Reuses `GENERATOR_SCRIPT_TIMEOUT_MS` (30000ms) rather
|
||||
* than declaring a new literal — that constant's own doc comment describes
|
||||
* "a single ... script, spawned directly ... against a small temp fixture
|
||||
* repo -- no fan-out", which is this call shape as well, even though the
|
||||
* spawned script here is an inline `-e` probe rather than a `scripts/*.cjs`
|
||||
* file.
|
||||
*/
|
||||
const COLD_TREE_PROBE_TIMEOUT_MS = GENERATOR_SCRIPT_TIMEOUT_MS;
|
||||
|
||||
describe('hooks/lib/dispatch-identity.js', () => {
|
||||
describe('marker format grammar', () => {
|
||||
test('marker: parses both identifiers', () => {
|
||||
const result = parseDispatchIdentity('[gsd:dispatch phase="03" plan="03-02-hardening"]');
|
||||
assert.deepEqual(result, { phase: '03', plan: '03-02-hardening', source: 'marker' });
|
||||
});
|
||||
|
||||
test('marker: found anywhere in a large prompt', () => {
|
||||
const text = `some preamble\n\nmore lines here\n[gsd:dispatch phase="03" plan="03-02-hardening"]\n\ntrailing text`;
|
||||
const result = parseDispatchIdentity(text);
|
||||
assert.equal(result.phase, '03');
|
||||
assert.equal(result.plan, '03-02-hardening');
|
||||
assert.equal(result.source, 'marker');
|
||||
});
|
||||
|
||||
test('render → parse round-trips', () => {
|
||||
const marker = renderDispatchIdentityMarker({ phase: '03', plan: '03-02-hardening' });
|
||||
assert.equal(marker, '[gsd:dispatch phase="03" plan="03-02-hardening"]');
|
||||
const parsed = parseDispatchIdentity(marker);
|
||||
assert.deepEqual(parsed, { phase: '03', plan: '03-02-hardening', source: 'marker' });
|
||||
});
|
||||
|
||||
test('property: render/parse is a bijection on valid tokens', () => {
|
||||
// Only values a real producer could emit: filesystem-derived phase/plan
|
||||
// identifiers, so no quotes, brackets, or newlines (those are exactly
|
||||
// the hostile-value case covered separately by row 21).
|
||||
const safeToken = fc
|
||||
.stringMatching(/^[A-Za-z0-9._-]+$/)
|
||||
.filter((s) => s.length > 0 && s.length <= 64);
|
||||
|
||||
fc.assert(
|
||||
fc.property(safeToken, safeToken, (phase, plan) => {
|
||||
const marker = renderDispatchIdentityMarker({ phase, plan });
|
||||
const parsed = parseDispatchIdentity(marker);
|
||||
assert.equal(parsed.phase, phase);
|
||||
assert.equal(parsed.plan, plan);
|
||||
assert.equal(parsed.source, 'marker');
|
||||
}),
|
||||
{ seed: 4594, numRuns: 300 },
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('prose fallback', () => {
|
||||
test('prose: bare phase number, plan deliberately null', () => {
|
||||
const result = parseDispatchIdentity(DESCRIPTION_FORM);
|
||||
assert.deepEqual(result, { phase: '03', plan: null, source: 'prose' });
|
||||
});
|
||||
|
||||
test('prose: slug suffix and trailing period are not part of the token', () => {
|
||||
const result = parseDispatchIdentity(PROMPT_BODY_FORM);
|
||||
assert.deepEqual(result, { phase: '03', plan: null, source: 'prose' });
|
||||
});
|
||||
|
||||
test('prose: decimal sub-phase', () => {
|
||||
const result = parseDispatchIdentity('Execute plan 02 of phase 3.2-thing.');
|
||||
assert.equal(result.phase, '3.2');
|
||||
assert.equal(result.plan, null);
|
||||
});
|
||||
|
||||
test('prose: variant-letter phase', () => {
|
||||
const result = parseDispatchIdentity('Execute plan 02 of phase 12A-thing.');
|
||||
assert.equal(result.phase, '12A');
|
||||
assert.equal(result.plan, null);
|
||||
});
|
||||
|
||||
test('prose: multi-segment sub-phase', () => {
|
||||
const result = parseDispatchIdentity('Execute plan 02 of phase 3.2.1');
|
||||
assert.equal(result.phase, '3.2.1');
|
||||
assert.equal(result.plan, null);
|
||||
});
|
||||
|
||||
test('prose: token at end of input', () => {
|
||||
const result = parseDispatchIdentity('Execute plan 02 of phase 03');
|
||||
assert.equal(result.phase, '03');
|
||||
});
|
||||
|
||||
test('prose: requires the full execute-plan-of-phase frame', () => {
|
||||
const lookalikes = [
|
||||
'run phase-plan-index for this project',
|
||||
'we are still in this phase 03 of the rollout',
|
||||
'the phase_dir helper resolves the directory',
|
||||
];
|
||||
for (const text of lookalikes) {
|
||||
assert.deepEqual(
|
||||
parseDispatchIdentity(text),
|
||||
{ phase: null, plan: null, source: null },
|
||||
`unexpected match for lookalike: ${text}`,
|
||||
);
|
||||
}
|
||||
});
|
||||
|
||||
test('prose: anchors on the first frame only', () => {
|
||||
const text = 'Execute plan 02 of phase 03-auth. Note: wait until after plan 03-01 completes.';
|
||||
const result = parseDispatchIdentity(text);
|
||||
assert.equal(result.phase, '03');
|
||||
assert.equal(result.plan, null);
|
||||
});
|
||||
});
|
||||
|
||||
describe('precedence and malformed markers', () => {
|
||||
test('marker takes precedence over prose', () => {
|
||||
const text = 'Execute plan 02 of phase 03-auth. [gsd:dispatch phase="04" plan="04-01-setup"]';
|
||||
const result = parseDispatchIdentity(text);
|
||||
assert.equal(result.phase, '04');
|
||||
assert.equal(result.plan, '04-01-setup');
|
||||
assert.equal(result.source, 'marker');
|
||||
});
|
||||
|
||||
test('marker: unquoted value is not accepted', () => {
|
||||
const result = parseDispatchIdentity('[gsd:dispatch phase=03]');
|
||||
// Never a partial wrong value: either falls back to null/null (no
|
||||
// prose present here) or, if this text also had prose, to the prose
|
||||
// result — but never a phase of '03' read out of the malformed marker.
|
||||
assert.notEqual(result.source, 'marker');
|
||||
assert.deepEqual(result, { phase: null, plan: null, source: null });
|
||||
});
|
||||
|
||||
test('marker: unknown keys are ignored', () => {
|
||||
const result = parseDispatchIdentity('[gsd:dispatch phase="03" plan="03-02-hardening" run="abc"]');
|
||||
assert.equal(result.phase, '03');
|
||||
assert.equal(result.plan, '03-02-hardening');
|
||||
assert.equal(result.source, 'marker');
|
||||
});
|
||||
|
||||
test('marker: key order does not matter', () => {
|
||||
const result = parseDispatchIdentity('[gsd:dispatch plan="03-02-hardening" phase="03"]');
|
||||
assert.equal(result.phase, '03');
|
||||
assert.equal(result.plan, '03-02-hardening');
|
||||
});
|
||||
|
||||
test('marker: phase-only is valid', () => {
|
||||
const result = parseDispatchIdentity('[gsd:dispatch phase="03"]');
|
||||
assert.deepEqual(result, { phase: '03', plan: null, source: 'marker' });
|
||||
});
|
||||
|
||||
// #4594 F1: a syntactically well-formed marker that carries NEITHER
|
||||
// `phase=` nor `plan=` must not suppress the prose fallback — it is not
|
||||
// a marker at all for this parser's purposes.
|
||||
test('F1: keyless marker (no recognized keys) falls through to prose', () => {
|
||||
const result = parseDispatchIdentity('Execute plan 02 of phase 03-auth.\n[gsd:dispatch]');
|
||||
assert.deepEqual(result, { phase: '03', plan: null, source: 'prose' });
|
||||
});
|
||||
|
||||
test('F1: marker with only an unrecognized key falls through to prose', () => {
|
||||
const result = parseDispatchIdentity('Execute plan 02 of phase 03-auth.\n[gsd:dispatch run="x"]');
|
||||
assert.deepEqual(result, { phase: '03', plan: null, source: 'prose' });
|
||||
});
|
||||
|
||||
test('F1: keyless marker with no prose anywhere yields the empty result', () => {
|
||||
const result = parseDispatchIdentity('[gsd:dispatch]');
|
||||
assert.deepEqual(result, { phase: null, plan: null, source: null });
|
||||
});
|
||||
|
||||
test('F1: mixed case — a keyless marker precedes a later QUALIFYING marker, which wins', () => {
|
||||
const result = parseDispatchIdentity(
|
||||
'[gsd:dispatch] some text [gsd:dispatch phase="03" plan="03-02-hardening"]',
|
||||
);
|
||||
assert.deepEqual(result, { phase: '03', plan: '03-02-hardening', source: 'marker' });
|
||||
});
|
||||
|
||||
test('F1: unknown-key tolerance is preserved for a marker that ALSO carries a recognized key', () => {
|
||||
const result = parseDispatchIdentity('[gsd:dispatch phase="03" run="x"]');
|
||||
assert.deepEqual(result, { phase: '03', plan: null, source: 'marker' });
|
||||
});
|
||||
});
|
||||
|
||||
describe('hostile and edge inputs', () => {
|
||||
test('parse: non-string and empty inputs', () => {
|
||||
const hostileInputs = [undefined, null, '', 0, 42, true, false, [], {}, NaN];
|
||||
for (const input of hostileInputs) {
|
||||
assert.doesNotThrow(() => parseDispatchIdentity(input));
|
||||
assert.deepEqual(
|
||||
parseDispatchIdentity(input),
|
||||
{ phase: null, plan: null, source: null },
|
||||
`unexpected result for hostile input: ${String(input)}`,
|
||||
);
|
||||
}
|
||||
// Zero-argument call must also never throw and yield the same empty shape.
|
||||
assert.doesNotThrow(() => parseDispatchIdentity());
|
||||
assert.deepEqual(parseDispatchIdentity(), { phase: null, plan: null, source: null });
|
||||
});
|
||||
|
||||
test('parse: unrelated prose', () => {
|
||||
const result = parseDispatchIdentity('this text has nothing to do with any dispatch');
|
||||
assert.deepEqual(result, { phase: null, plan: null, source: null });
|
||||
});
|
||||
|
||||
test('CRLF variants parse identically', () => {
|
||||
const lf = parseDispatchIdentity('preamble\n[gsd:dispatch phase="03" plan="03-02-hardening"]\ntrailer');
|
||||
const crlf = parseDispatchIdentity('preamble\r\n[gsd:dispatch phase="03" plan="03-02-hardening"]\r\ntrailer');
|
||||
assert.deepEqual(crlf, lf);
|
||||
|
||||
const lfProse = parseDispatchIdentity('Execute plan 02 of phase 03-auth.\nmore text');
|
||||
const crlfProse = parseDispatchIdentity('Execute plan 02 of phase 03-auth.\r\nmore text');
|
||||
assert.deepEqual(crlfProse, lfProse);
|
||||
});
|
||||
|
||||
test('marker: quote/bracket in a value cannot forge a field', () => {
|
||||
// A value containing '"' or ']' is unusable per the render contract; a
|
||||
// hand-crafted malicious marker string attempting the same must not
|
||||
// let the parser manufacture a second, forged identifier either.
|
||||
const hostileMarker = '[gsd:dispatch phase="03" plan="03-02"]" phase="99"]';
|
||||
const result = parseDispatchIdentity(hostileMarker);
|
||||
assert.notEqual(result.phase, '99');
|
||||
});
|
||||
|
||||
test('parse: large prompt terminates', (t) => {
|
||||
const filler = 'x'.repeat(100 * 1024);
|
||||
const text = `${filler}[gsd:dispatch phase="03" plan="03-02-hardening"]`;
|
||||
const result = parseDispatchIdentity(text);
|
||||
assert.equal(result.phase, '03');
|
||||
assert.equal(result.plan, '03-02-hardening');
|
||||
// Backstop against catastrophic backtracking, not a timing assertion:
|
||||
// this test's own node:test timeout is the enforcement mechanism.
|
||||
t.diagnostic('parse completed within the test timeout on a 100KB input');
|
||||
});
|
||||
|
||||
test('parse: scans every supplied text in order', () => {
|
||||
const result = parseDispatchIdentity('', '[gsd:dispatch phase="03" plan="03-02-hardening"]');
|
||||
assert.equal(result.phase, '03');
|
||||
assert.equal(result.plan, '03-02-hardening');
|
||||
assert.equal(result.source, 'marker');
|
||||
});
|
||||
});
|
||||
|
||||
describe('render: omission rules', () => {
|
||||
test('render: unusable values (quote or bracket) are omitted', () => {
|
||||
assert.equal(renderDispatchIdentityMarker({ phase: '03', plan: 'has"quote' }), '[gsd:dispatch phase="03"]');
|
||||
assert.equal(renderDispatchIdentityMarker({ phase: '03', plan: 'has]bracket' }), '[gsd:dispatch phase="03"]');
|
||||
assert.equal(renderDispatchIdentityMarker({ phase: 'ba"d', plan: '03-02' }), '[gsd:dispatch plan="03-02"]');
|
||||
});
|
||||
|
||||
test('render: absent or empty values are omitted, and both-absent renders empty string', () => {
|
||||
assert.equal(renderDispatchIdentityMarker({ phase: '03' }), '[gsd:dispatch phase="03"]');
|
||||
assert.equal(renderDispatchIdentityMarker({ plan: '03-02-hardening' }), '[gsd:dispatch plan="03-02-hardening"]');
|
||||
assert.equal(renderDispatchIdentityMarker({ phase: '', plan: '' }), '');
|
||||
assert.equal(renderDispatchIdentityMarker({}), '');
|
||||
});
|
||||
});
|
||||
|
||||
describe('sentinelAppliesToDispatch end-state behavior (hooks/lib/isolation-sentinel.js)', () => {
|
||||
test('applies: prose dispatch no longer false-mismatches on plan', () => {
|
||||
// THE regression, #4594 rows 2/4: a fresh sentinel carrying the
|
||||
// phase-prefixed, slugged plan id must still apply to a dispatch whose
|
||||
// only identity source is the prose prompt-body form.
|
||||
const dispatchIds = extractDispatchIdentifiers(PROMPT_BODY_FORM);
|
||||
assert.equal(sentinelAppliesToDispatch(MEASURED_SENTINEL, dispatchIds), true);
|
||||
});
|
||||
|
||||
test('applies: marker for another plan is rejected', () => {
|
||||
const dispatchIds = parseDispatchIdentity('[gsd:dispatch phase="03" plan="03-01-setup"]');
|
||||
assert.equal(sentinelAppliesToDispatch(MEASURED_SENTINEL, dispatchIds), false);
|
||||
});
|
||||
|
||||
test('applies: marker for another phase is rejected', () => {
|
||||
const dispatchIds = parseDispatchIdentity('[gsd:dispatch phase="04" plan="03-02-hardening"]');
|
||||
assert.equal(sentinelAppliesToDispatch(MEASURED_SENTINEL, dispatchIds), false);
|
||||
});
|
||||
|
||||
test('applies: a missing value is never a mismatch', () => {
|
||||
assert.equal(sentinelAppliesToDispatch(MEASURED_SENTINEL, { phase: null, plan: null }), true);
|
||||
assert.equal(sentinelAppliesToDispatch({ phase: '03', plan: null }, { phase: '03', plan: '03-02-hardening' }), true);
|
||||
assert.equal(sentinelAppliesToDispatch({ phase: null, plan: '03-02-hardening' }, { phase: '99', plan: null }), true);
|
||||
});
|
||||
|
||||
test('extractDispatchIdentifiers still returns exactly {plan, phase}', () => {
|
||||
const result = extractDispatchIdentifiers(DESCRIPTION_FORM);
|
||||
assert.deepEqual(Object.keys(result).sort(), ['phase', 'plan']);
|
||||
});
|
||||
});
|
||||
|
||||
describe('cold-tree load (#3582-style standing constraint)', () => {
|
||||
test('cold tree: the owner module has no compiled-lib dependency', () => {
|
||||
const probe = `
|
||||
const Module = require('module');
|
||||
const originalLoad = Module._load;
|
||||
Module._load = function patchedLoad(request, parent, isMain) {
|
||||
if (typeof request === 'string' && (
|
||||
request.includes('gsd-core/bin/lib') ||
|
||||
request.includes('ensure-runtime-build')
|
||||
)) {
|
||||
throw new Error('FORBIDDEN cold-tree require: ' + request);
|
||||
}
|
||||
return originalLoad.call(this, request, parent, isMain);
|
||||
};
|
||||
const owner = require(${JSON.stringify(path.join(REPO_ROOT, 'hooks', 'lib', 'dispatch-identity.js'))});
|
||||
const marker = owner.renderDispatchIdentityMarker({ phase: '03', plan: '03-02-hardening' });
|
||||
const parsed = owner.parseDispatchIdentity(marker);
|
||||
if (parsed.phase !== '03' || parsed.plan !== '03-02-hardening') {
|
||||
throw new Error('cold-tree round-trip failed: ' + JSON.stringify(parsed));
|
||||
}
|
||||
process.stdout.write('OK');
|
||||
`;
|
||||
|
||||
const output = execFileSync(process.execPath, ['-e', probe], {
|
||||
cwd: REPO_ROOT,
|
||||
encoding: 'utf8',
|
||||
timeout: COLD_TREE_PROBE_TIMEOUT_MS,
|
||||
});
|
||||
assert.equal(output, 'OK');
|
||||
});
|
||||
});
|
||||
|
||||
describe('producer/consumer parity', () => {
|
||||
// #4594 F7: this reads the REAL workflow templates and runs their
|
||||
// marker literal through the owner's real parser after substituting the
|
||||
// measured placeholders — a template that loses its `[gsd:dispatch …]`
|
||||
// line, or whose grammar the owner's parser can no longer read, reds
|
||||
// this test. The prior version only called `renderDispatchIdentityMarker`
|
||||
// and re-parsed its own output (a duplicate of the round-trip test
|
||||
// above) and would have passed even if both templates below were
|
||||
// deleted. There are 3 prose "execute plan ... of phase ..." sites
|
||||
// across these 2 files; only 2 of them carry the `[gsd:dispatch …]`
|
||||
// marker (execute-phase.md's `description=` field is prose-only) — see
|
||||
// `.gsd/phase/fix-4594-dispatch-identity-seam/40-design.md`'s "Known
|
||||
// limits" section.
|
||||
const TEMPLATE_FILES = [
|
||||
path.join(REPO_ROOT, 'gsd-core', 'workflows', 'execute-phase.md'),
|
||||
path.join(REPO_ROOT, 'gsd-core', 'workflows', 'execute-phase', 'steps', 'executor-isolation-dispatch.md'),
|
||||
];
|
||||
|
||||
/**
|
||||
* Extract every `[gsd:dispatch ...]` marker LINE from a template's raw
|
||||
* text, verbatim (still containing its `{phase_number}`/`{plan_id}`
|
||||
* placeholders) — not a string match on content, a line-oriented
|
||||
* extraction feeding the real parser below.
|
||||
*/
|
||||
function extractMarkerLines(text) {
|
||||
return text.split(/\r?\n/).filter((line) => line.includes('[gsd:dispatch'));
|
||||
}
|
||||
|
||||
function substitutePlaceholders(line) {
|
||||
return line
|
||||
.replace(/\{phase_number\}/g, MEASURED_SENTINEL.phase)
|
||||
.replace(/\{plan_id\}/g, MEASURED_SENTINEL.plan);
|
||||
}
|
||||
|
||||
test('templates: every producer\'s marker parses', () => {
|
||||
let totalMarkerLines = 0;
|
||||
for (const file of TEMPLATE_FILES) {
|
||||
assert.equal(fs.existsSync(file), true, `template file not found: ${file}`);
|
||||
const text = fs.readFileSync(file, 'utf-8');
|
||||
const markerLines = extractMarkerLines(text);
|
||||
assert.equal(
|
||||
markerLines.length,
|
||||
1,
|
||||
`expected exactly one [gsd:dispatch ...] marker line in ${file}, found ${markerLines.length}`,
|
||||
);
|
||||
totalMarkerLines += markerLines.length;
|
||||
|
||||
const substituted = substitutePlaceholders(markerLines[0]);
|
||||
const parsed = parseDispatchIdentity(substituted);
|
||||
assert.deepEqual(
|
||||
parsed,
|
||||
{ phase: MEASURED_SENTINEL.phase, plan: MEASURED_SENTINEL.plan, source: 'marker' },
|
||||
`template marker in ${file} did not parse to the expected identifiers (got ${JSON.stringify(parsed)})`,
|
||||
);
|
||||
}
|
||||
// 3 prose sites exist across these 2 files; exactly 2 carry the marker.
|
||||
assert.equal(totalMarkerLines, 2);
|
||||
});
|
||||
|
||||
test('templates: prose token source matches the case-flexible phase-id grammar', () => {
|
||||
// Parity guard, matrix row 34: DISPATCH_PHASE_TOKEN_SOURCE is a hand
|
||||
// mirror of gsd-core/bin/lib/phase-id.cjs's
|
||||
// CASE_FLEXIBLE_PHASE_NUMBER_TOKEN_SOURCE. The mirror exists because
|
||||
// hooks/lib/dispatch-identity.js must load on a cold tree with no
|
||||
// compiled `gsd-core/bin/lib/` present (see the cold-tree test above),
|
||||
// so it cannot `require()` the compiled module directly — it restates
|
||||
// the same regex source string instead. A hand-edited grammar change
|
||||
// on one side that is not mirrored on the other does NOT fail loudly:
|
||||
// both sides remain independently valid regex sources, so the failure
|
||||
// mode is a silent, invisible non-match (a dispatch whose phase token
|
||||
// the two owners now parse differently), not a thrown error. Pinning
|
||||
// this equality is the only thing that turns that drift into a loud,
|
||||
// in-CI failure. Loaded only here, not at module scope, so this
|
||||
// file's own MODULE_NOT_FOUND failure mode (before
|
||||
// hooks/lib/dispatch-identity.js exists) is not masked by a require
|
||||
// ordering accident.
|
||||
const { CASE_FLEXIBLE_PHASE_NUMBER_TOKEN_SOURCE } = require(
|
||||
path.join(REPO_ROOT, 'gsd-core', 'bin', 'lib', 'phase-id.cjs'),
|
||||
);
|
||||
assert.equal(DISPATCH_PHASE_TOKEN_SOURCE, CASE_FLEXIBLE_PHASE_NUMBER_TOKEN_SOURCE);
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -18,9 +18,9 @@
|
||||
"reductionPct": 16.51
|
||||
},
|
||||
"execute-phase": {
|
||||
"offTokens": 25827,
|
||||
"onTokens": 23576,
|
||||
"reductionPct": 8.72
|
||||
"offTokens": 25952,
|
||||
"onTokens": 23701,
|
||||
"reductionPct": 8.67
|
||||
},
|
||||
"new-project": {
|
||||
"offTokens": 14279,
|
||||
@@ -39,8 +39,8 @@
|
||||
}
|
||||
},
|
||||
"aggregate": {
|
||||
"offTokens": 107286,
|
||||
"onTokens": 90638,
|
||||
"reductionPct": 15.52
|
||||
"offTokens": 107411,
|
||||
"onTokens": 90763,
|
||||
"reductionPct": 15.5
|
||||
}
|
||||
}
|
||||
|
||||
1
tests/fixtures/install-tree/antigravity.json
vendored
1
tests/fixtures/install-tree/antigravity.json
vendored
@@ -594,6 +594,7 @@
|
||||
"hooks/gsd-write-guard.js",
|
||||
"hooks/lib/cli-exit.js",
|
||||
"hooks/lib/cursor-workspace.js",
|
||||
"hooks/lib/dispatch-identity.js",
|
||||
"hooks/lib/exit-code-registry.js",
|
||||
"hooks/lib/filename-classification.js",
|
||||
"hooks/lib/git-cmd.js",
|
||||
|
||||
1
tests/fixtures/install-tree/augment.json
vendored
1
tests/fixtures/install-tree/augment.json
vendored
@@ -666,6 +666,7 @@
|
||||
"hooks/gsd-write-guard.js",
|
||||
"hooks/lib/cli-exit.js",
|
||||
"hooks/lib/cursor-workspace.js",
|
||||
"hooks/lib/dispatch-identity.js",
|
||||
"hooks/lib/exit-code-registry.js",
|
||||
"hooks/lib/filename-classification.js",
|
||||
"hooks/lib/git-cmd.js",
|
||||
|
||||
@@ -530,6 +530,7 @@
|
||||
"hooks/gsd-write-guard.js",
|
||||
"hooks/lib/cli-exit.js",
|
||||
"hooks/lib/cursor-workspace.js",
|
||||
"hooks/lib/dispatch-identity.js",
|
||||
"hooks/lib/exit-code-registry.js",
|
||||
"hooks/lib/filename-classification.js",
|
||||
"hooks/lib/git-cmd.js",
|
||||
|
||||
1
tests/fixtures/install-tree/claude.json
vendored
1
tests/fixtures/install-tree/claude.json
vendored
@@ -594,6 +594,7 @@
|
||||
"hooks/gsd-write-guard.js",
|
||||
"hooks/lib/cli-exit.js",
|
||||
"hooks/lib/cursor-workspace.js",
|
||||
"hooks/lib/dispatch-identity.js",
|
||||
"hooks/lib/exit-code-registry.js",
|
||||
"hooks/lib/filename-classification.js",
|
||||
"hooks/lib/git-cmd.js",
|
||||
|
||||
1
tests/fixtures/install-tree/codebuddy.json
vendored
1
tests/fixtures/install-tree/codebuddy.json
vendored
@@ -666,6 +666,7 @@
|
||||
"hooks/gsd-write-guard.js",
|
||||
"hooks/lib/cli-exit.js",
|
||||
"hooks/lib/cursor-workspace.js",
|
||||
"hooks/lib/dispatch-identity.js",
|
||||
"hooks/lib/exit-code-registry.js",
|
||||
"hooks/lib/filename-classification.js",
|
||||
"hooks/lib/git-cmd.js",
|
||||
|
||||
1
tests/fixtures/install-tree/cursor.json
vendored
1
tests/fixtures/install-tree/cursor.json
vendored
@@ -572,6 +572,7 @@
|
||||
"hooks/gsd-cursor-subagent-stop.js",
|
||||
"hooks/lib/cli-exit.js",
|
||||
"hooks/lib/cursor-workspace.js",
|
||||
"hooks/lib/dispatch-identity.js",
|
||||
"hooks/lib/exit-code-registry.js",
|
||||
"hooks/lib/hook-exit.js",
|
||||
"hooks/lib/isolation-deny-reason.js",
|
||||
|
||||
1
tests/fixtures/install-tree/hermes.json
vendored
1
tests/fixtures/install-tree/hermes.json
vendored
@@ -594,6 +594,7 @@
|
||||
"hooks/gsd-write-guard.js",
|
||||
"hooks/lib/cli-exit.js",
|
||||
"hooks/lib/cursor-workspace.js",
|
||||
"hooks/lib/dispatch-identity.js",
|
||||
"hooks/lib/exit-code-registry.js",
|
||||
"hooks/lib/filename-classification.js",
|
||||
"hooks/lib/git-cmd.js",
|
||||
|
||||
1
tests/fixtures/install-tree/kilo.json
vendored
1
tests/fixtures/install-tree/kilo.json
vendored
@@ -666,6 +666,7 @@
|
||||
"hooks/gsd-write-guard.js",
|
||||
"hooks/lib/cli-exit.js",
|
||||
"hooks/lib/cursor-workspace.js",
|
||||
"hooks/lib/dispatch-identity.js",
|
||||
"hooks/lib/exit-code-registry.js",
|
||||
"hooks/lib/filename-classification.js",
|
||||
"hooks/lib/git-cmd.js",
|
||||
|
||||
1
tests/fixtures/install-tree/kimi-code.json
vendored
1
tests/fixtures/install-tree/kimi-code.json
vendored
@@ -595,6 +595,7 @@
|
||||
"hooks/gsd-write-guard.js",
|
||||
"hooks/lib/cli-exit.js",
|
||||
"hooks/lib/cursor-workspace.js",
|
||||
"hooks/lib/dispatch-identity.js",
|
||||
"hooks/lib/exit-code-registry.js",
|
||||
"hooks/lib/filename-classification.js",
|
||||
"hooks/lib/git-cmd.js",
|
||||
|
||||
1
tests/fixtures/install-tree/opencode.json
vendored
1
tests/fixtures/install-tree/opencode.json
vendored
@@ -666,6 +666,7 @@
|
||||
"hooks/gsd-write-guard.js",
|
||||
"hooks/lib/cli-exit.js",
|
||||
"hooks/lib/cursor-workspace.js",
|
||||
"hooks/lib/dispatch-identity.js",
|
||||
"hooks/lib/exit-code-registry.js",
|
||||
"hooks/lib/filename-classification.js",
|
||||
"hooks/lib/git-cmd.js",
|
||||
|
||||
1
tests/fixtures/install-tree/pi.json
vendored
1
tests/fixtures/install-tree/pi.json
vendored
@@ -396,6 +396,7 @@
|
||||
"gsd-hooks/gsd-write-guard.js",
|
||||
"gsd-hooks/lib/cli-exit.js",
|
||||
"gsd-hooks/lib/cursor-workspace.js",
|
||||
"gsd-hooks/lib/dispatch-identity.js",
|
||||
"gsd-hooks/lib/exit-code-registry.js",
|
||||
"gsd-hooks/lib/filename-classification.js",
|
||||
"gsd-hooks/lib/git-cmd.js",
|
||||
|
||||
1
tests/fixtures/install-tree/qwen.json
vendored
1
tests/fixtures/install-tree/qwen.json
vendored
@@ -594,6 +594,7 @@
|
||||
"hooks/gsd-write-guard.js",
|
||||
"hooks/lib/cli-exit.js",
|
||||
"hooks/lib/cursor-workspace.js",
|
||||
"hooks/lib/dispatch-identity.js",
|
||||
"hooks/lib/exit-code-registry.js",
|
||||
"hooks/lib/filename-classification.js",
|
||||
"hooks/lib/git-cmd.js",
|
||||
|
||||
@@ -55,7 +55,7 @@ const { toLegacyResult, gitOrThrow } = require('./helpers/git-fixture.cjs');
|
||||
const { PROBE_TIMEOUT_MS } = require('./helpers/timeouts.cjs');
|
||||
const { createTempDir, createTempProject, runGsdTools, cleanup } = require('./helpers.cjs');
|
||||
const { SENTINEL_RELATIVE_PATH, SENTINEL_STALE_MS, readSentinel } = require('../hooks/lib/isolation-sentinel.js');
|
||||
const { REASON_CODE } = require('../hooks/lib/isolation-deny-reason.js');
|
||||
const { REASON_CODE, REASON_INTERPOLATION_MAX_LEN, sanitizeForReason, describeSentinelDiscard } = require('../hooks/lib/isolation-deny-reason.js');
|
||||
const { runtimes } = require('../gsd-core/bin/lib/capability-registry.cjs');
|
||||
|
||||
const HOOK_PATH = path.join(__dirname, '..', 'hooks', 'gsd-agent-isolation-guard.js');
|
||||
@@ -568,6 +568,93 @@ describe('gsd-agent-isolation-guard.js: #3045 SECURITY F2 — sentinel bound to
|
||||
});
|
||||
});
|
||||
|
||||
describe('gsd-agent-isolation-guard.js: #4594 rows 15/28-32 — prompt-first extraction + sentinel-discard reporting', () => {
|
||||
let harnessProject;
|
||||
|
||||
before(() => {
|
||||
harnessProject = mkProject('gsd-aig-4594-');
|
||||
writeConfig(harnessProject, JSON.stringify({ runtime: 'claude' }));
|
||||
});
|
||||
|
||||
after(() => {
|
||||
cleanup(harnessProject);
|
||||
});
|
||||
|
||||
test('row 29 (THE REGRESSION): fresh sentinel matches a real prose dispatch carried only in tool_input.prompt -> ALLOW', (t) => {
|
||||
// Measured production shapes (not simplified): sentinel plan is
|
||||
// phase-prefixed-and-slugged (`03-02-hardening`, phase-plan-index's
|
||||
// `plans[].id`), the dispatch prose is the verbatim frame the workflow
|
||||
// actually emits. `description` is deliberately OMITTED so this only
|
||||
// passes when evaluateDispatch scans `prompt` — before this change the
|
||||
// guard read only `description` and never saw this text at all.
|
||||
writeSentinel(harnessProject, { isolation: 'none', phase: '03', plan: '03-02-hardening' });
|
||||
t.after(() => cleanup(path.join(harnessProject, '.gsd')));
|
||||
const r = runHook(
|
||||
agentPayload({ tool_input: { subagent_type: 'gsd-executor', prompt: 'Execute plan 02 of phase 03-auth.' } }),
|
||||
harnessProject,
|
||||
);
|
||||
assert.equal(r.status, 0, `stdout: ${r.stdout} stderr: ${r.stderr}`);
|
||||
assert.equal(r.stdout, '');
|
||||
});
|
||||
|
||||
test('row 28: unusable description + marker-bearing prompt, sentinel matches -> ALLOW', (t) => {
|
||||
writeSentinel(harnessProject, { isolation: 'none', phase: '03', plan: '03-02-hardening' });
|
||||
t.after(() => cleanup(path.join(harnessProject, '.gsd')));
|
||||
const r = runHook(
|
||||
agentPayload({
|
||||
tool_input: {
|
||||
subagent_type: 'gsd-executor',
|
||||
description: 'Run the executor',
|
||||
prompt: '[gsd:dispatch phase="03" plan="03-02-hardening"] Execute the plan.',
|
||||
},
|
||||
}),
|
||||
harnessProject,
|
||||
);
|
||||
assert.equal(r.status, 0, `stdout: ${r.stdout} stderr: ${r.stderr}`);
|
||||
assert.equal(r.stdout, '');
|
||||
});
|
||||
|
||||
test('row 31: fresh sentinel for a DIFFERENT plan, isolation harness-worktree, kwarg missing -> DENY naming the discarded sentinel and both identifiers', (t) => {
|
||||
writeSentinel(harnessProject, {
|
||||
isolation: 'harness-worktree',
|
||||
harnessFlag: 'isolation="worktree"',
|
||||
phase: '03',
|
||||
plan: '03-02-hardening',
|
||||
});
|
||||
t.after(() => cleanup(path.join(harnessProject, '.gsd')));
|
||||
const r = runHook(
|
||||
agentPayload({
|
||||
tool_input: {
|
||||
subagent_type: 'gsd-executor',
|
||||
prompt: '[gsd:dispatch phase="03" plan="07-01-x"] Execute the plan.',
|
||||
},
|
||||
}),
|
||||
harnessProject,
|
||||
);
|
||||
assert.equal(r.status, 2, `stdout: ${r.stdout} stderr: ${r.stderr}`);
|
||||
const out = JSON.parse(r.stdout);
|
||||
assert.equal(out.decision, 'block');
|
||||
assert.match(out.reason, /sentinel/i);
|
||||
// #4594 F4: the reason PROSE is not the contract (CONTRIBUTING.md
|
||||
// "Prohibited: Raw Text Matching on Test Outputs") — assert the
|
||||
// STRUCTURED `sentinel_discarded` field instead.
|
||||
assert.deepEqual(out.sentinel_discarded, {
|
||||
sentinel: { phase: '03', plan: '03-02-hardening' },
|
||||
dispatch: { phase: '03', plan: '07-01-x' },
|
||||
});
|
||||
});
|
||||
|
||||
test('row 32: no sentinel at all -> unchanged conservative fallback (DENY, registry resolves harness-worktree)', () => {
|
||||
const r = runHook(agentPayload(), harnessProject);
|
||||
assert.equal(r.status, 2, `stdout: ${r.stdout} stderr: ${r.stderr}`);
|
||||
const out = JSON.parse(r.stdout);
|
||||
assert.equal(out.decision, 'block');
|
||||
// No sentinel existed, so there is nothing to discard/report.
|
||||
assert.doesNotMatch(out.reason, /was not consulted/);
|
||||
assert.equal(out.sentinel_discarded, null);
|
||||
});
|
||||
});
|
||||
|
||||
describe('gsd-agent-isolation-guard.js: #3045 MAJOR — clock seam boundary coverage (in-process, no subprocess wall-clock race)', () => {
|
||||
const guardModule = require('../hooks/gsd-agent-isolation-guard.js');
|
||||
|
||||
@@ -1630,3 +1717,51 @@ describe('worktreesOptedOut — ladder unit semantics (#3972)', () => {
|
||||
assert.equal(worktreesOptedOut(dir), true, 'unreadable scoped config falls to the root view under the gate');
|
||||
});
|
||||
});
|
||||
|
||||
describe('hooks/lib/isolation-deny-reason.js — sanitizeForReason (#4594 F2/F5/F6)', () => {
|
||||
test('boundary: 63 chars is not truncated', () => {
|
||||
const value = 'a'.repeat(63);
|
||||
assert.equal(sanitizeForReason(value), value);
|
||||
assert.equal(REASON_INTERPOLATION_MAX_LEN, 64);
|
||||
});
|
||||
|
||||
test('boundary: exactly 64 chars (the limit) is NOT truncated', () => {
|
||||
const value = 'a'.repeat(64);
|
||||
assert.equal(sanitizeForReason(value), value);
|
||||
assert.equal(sanitizeForReason(value).includes('…'), false);
|
||||
});
|
||||
|
||||
test('boundary: 65 chars is truncated to 64 chars plus an ellipsis', () => {
|
||||
const value = 'a'.repeat(65);
|
||||
const result = sanitizeForReason(value);
|
||||
assert.equal(result, `${'a'.repeat(64)}…`);
|
||||
assert.equal(result.length, 65);
|
||||
});
|
||||
|
||||
test('F6: strips Unicode line/paragraph separators (U+2028/U+2029)', () => {
|
||||
assert.equal(sanitizeForReason('before
after'), 'beforeafter');
|
||||
assert.equal(sanitizeForReason('before
after'), 'beforeafter');
|
||||
});
|
||||
|
||||
test('F6: strips bidi override/isolate control characters (U+202A-U+202E, U+2066-U+2069)', () => {
|
||||
assert.equal(sanitizeForReason('evil'), 'evil');
|
||||
assert.equal(sanitizeForReason('evil'), 'evil');
|
||||
assert.equal(sanitizeForReason('evil'), 'evil');
|
||||
});
|
||||
|
||||
test('empty/non-string values render "(none)"', () => {
|
||||
assert.equal(sanitizeForReason(''), '(none)');
|
||||
assert.equal(sanitizeForReason(null), '(none)');
|
||||
assert.equal(sanitizeForReason(undefined), '(none)');
|
||||
});
|
||||
|
||||
test('describeSentinelDiscard consumes the {sentinel, dispatch} shape from buildSentinelDiscard (#4594 F3)', () => {
|
||||
const discard = {
|
||||
sentinel: { phase: '03', plan: '03-02-hardening' },
|
||||
dispatch: { phase: '03', plan: '07-01-x' },
|
||||
};
|
||||
const message = describeSentinelDiscard(discard);
|
||||
assert.match(message, /sentinel phase="03" plan="03-02-hardening"/);
|
||||
assert.match(message, /dispatch phase="03" plan="07-01-x"/);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user