diff --git a/docs/results/2026-09-27-resume-gate-armed-line-is-ambiguous-when-no-epoch-exists-dis-results.md b/docs/results/2026-09-27-resume-gate-armed-line-is-ambiguous-when-no-epoch-exists-dis-results.md new file mode 100644 index 000000000..b9272657c --- /dev/null +++ b/docs/results/2026-09-27-resume-gate-armed-line-is-ambiguous-when-no-epoch-exists-dis-results.md @@ -0,0 +1,54 @@ + +> ↩ **[Change 0463 — Resume gate-armed line is ambiguous when no epoch exists — dispatch context gets passed as --run-epoch](https://github.com/danielhanold/docket/blob/docket/docs/changes/active/0463-resume-gate-armed-line-is-ambiguous-when-no-epoch-exists-dis.md)** + +# Resume gate-armed line is ambiguous when no epoch exists — Results + +**Human action:** Review the PR. Look closely at one design point before merging: `run cancel` now accepts a resumed run's verified change id as cancel authority. Nothing else needs a human. + +## Outcome + +Before this change, resuming a change whose earlier run was never armed (`docket run gate-before implement-next --resume ` with no prior run epoch) printed a two-token line, `gate-armed `. The documented line has three tokens, `gate-armed `. Both values are 32-hex strings, so the parent read the dispatch context as the epoch and passed it as `--run-epoch`. The gate then refused with a generic `invalid-request`, and the resumed run finished with no epoch linkage and no cancel coverage. That is what happened on change 0382. + +What changed: + +- **A resume with no prior epoch now mints one.** The epoch is bound to the change id and the verified feature worktree, so every armed gate carries an epoch and the `gate-armed` line always has three tokens. +- **An unknown `--run-epoch` gets a named refusal,** `unknown-run-epoch`, with a hint that names which token of the armed line goes where. This covers `gate drive start`, `gate drive prepare-scope` (which checks only that the epoch exists, not that it is live), and `agent enter`. `agent enter` now checks before it launches anything, including when `--run-epoch` is passed without `--run-gate-key`. A mismatched epoch there reports the existing `stale-run-epoch`. +- **Changes beyond the spec, made because of review findings:** + - `run cancel` accepts the resume-verified owner. That is a gate record attributed to a change, with no claim-binding file, and the change must match the epoch's own change id. Without this, the new resume epoch could never be cancelled, and every later `--resume` of that change was stuck behind `resume-active-run`. Epochs from the older cancelled-run replacement path can now be cancelled too. An unconfirmed claim reservation still refuses `claim-unconfirmed`. + - An epochless resume now refuses `resume-active-run` when a live epoch already owns the feature worktree, before it mints anything. This stops two live epochs from blocking all fenced mutations (such as PR publish) in one worktree. + +## Human actions and testing + +### Important — sign off on the widened `run cancel` authority + +ADR-0111 made a confirmed claim binding the authority for cancelling a run. This change adds a second accepted shape: a gate record attributed to change N with no binding file, cancelling an epoch whose change id is N. Without it, resumed runs cannot be cancelled at all. It is still a policy change on a fail-closed boundary, and no separate ADR records it. If you skip this review, the risk is that the widening covers a record shape you did not intend. + +1. Read the authority step of `runCancel` in `internal/app/rungate_cancel.go`, and `TestIntegrationGateCancelRunCancelResumeAuthorityFailsClosed` in `internal/app/rungate_before_resume_integration_test.go`. + Expected: the resume shape is accepted only when no binding file exists and the attributed id equals the epoch's change id. Every other case refuses as before. +2. Decide whether this needs an ADR update note. If it does, record one with `docket adr` after merging. + +## Verification performed + +- The full build suite (`go run ./cmd/docket development test`) passed on the pre-review head `d9d0c4ac`: 54 of 54 files. The post-review certification run on the final head is recorded in the PR's build-evidence block. +- Every task and review fix ran a focused RED/GREEN cycle through the gate driver. Each new guard was mutation-checked: stripping the fix turned its test red. +- Review used the deep rung and returned 1 blocker, 3 important and 1 minor finding. All were fixed in the branch (see the PR's disposition table). There was no second review round after the fixes. + +## Post-review changes (2026-09-28) + +A human review of the widened `run cancel` authority found three problems. The branch was then rebased onto `main` (one test moved to `testsupport.TempDir` after change 0462 removed `gateTempDir`), and two were fixed: + +- **A stray claim no longer brings back the cancel wedge.** A claim made under a resume arm's dispatch context used to write its reservation before refusing, or could claim a different change. Either left the resume epoch uncancellable (`claim-unconfirmed` or `claim-mismatch`). `change claim` now refuses a resume context as `gate-context-conflict` before it writes anything. The resume-verified shape is one predicate, `GateRecord.resumeAttributed`, shared by claim, cancel, and the verdict. +- **Concurrent resume arms are serialized.** The race below was not small: 12 of 12 simultaneous arms each minted a live epoch in test. A per-change resume lock (under `/docket/rungate-resume/`) now covers the arm from the prior-epoch check through the bind, so exactly one arms and the rest refuse `resume-active-run`. +- **Kept on purpose: an undispatched resume arm blocks the next resume until cancelled.** The resume epoch is bound when armed so that a second agent cannot enter the worktree before the first reaches its first gate step. Binding later would reopen that window, and the dispatch context is never stored, so a repeat arm cannot reprint it. The earlier resume-after-cancel path already behaves this way. Nothing records whether an agent is using an epoch, so the `resume-active-run` refusal now names both remedies: cancel if it was never dispatched or its agent exited, `run gate-verdict` if it is still running. + +## Known issues and follow-ups + +### A small race window remains between concurrent epochless resumes + +**Fixed after review** (see *Post-review changes*): a per-change resume lock now serializes resume arms. The original finding follows. + +Two `--resume` arms of the same unarmed change started at the same instant can both pass the new worktree-owner check and each mint an epoch. Epoch locks are per gate key, not per change. If that happens, later resumes refuse as `resume-epoch-unreadable`, and fenced mutations in that worktree refuse until one epoch is cancelled with `docket run cancel`. After that cancel, the change's own later resumes still see both epochs and keep refusing. This is suspected, not observed, and needs a true simultaneous double arm. Suggested next step: a follow-up change adding a check-after-bind re-scan, in which a racer that sees two owners unbinds its own epoch. + +### Cancelled epochs that were never superseded still count as live + +`FindEpochByChange` counts a cancelled epoch that was never superseded as live. This predates the change and only matters after the double-mint race above. It needs no action unless that race is seen in practice. diff --git a/docs/superpowers/plans/2026-09-27-resume-gate-armed-line-is-ambiguous-when-no-epoch-exists-dis.md b/docs/superpowers/plans/2026-09-27-resume-gate-armed-line-is-ambiguous-when-no-epoch-exists-dis.md new file mode 100644 index 000000000..4e8450fd2 --- /dev/null +++ b/docs/superpowers/plans/2026-09-27-resume-gate-armed-line-is-ambiguous-when-no-epoch-exists-dis.md @@ -0,0 +1,1271 @@ + +> ↩ **[Change 0463 — Resume gate-armed line is ambiguous when no epoch exists — dispatch context gets passed as --run-epoch](https://github.com/danielhanold/docket/blob/docket/docs/changes/active/0463-resume-gate-armed-line-is-ambiguous-when-no-epoch-exists-dis.md)** + +# Resume Arm Always Binds a Run Epoch — Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: this plan is executed by `docket-build` (the resolved +> build skill) task-by-task, each task routed to a build-profile worker under the `docket-build-task` +> contract, with one whole-suite gate at the end. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Make `run.gate-before --resume ` mint and bind a run epoch when the change has none, so +the `gate-armed ` line always has three tokens, and give an unknown +`--run-epoch` the named refusal `unknown-run-epoch` instead of the catch-all `invalid-request`. + +**Architecture:** In `RunGateBefore` (`internal/app/rungate_before.go`), step (6a) mints an epoch on +every arm that reaches it (fresh arm or epochless resume). A resume binds `ChangeID` + `Worktree` in +one `epochCAS`. A new `armedGateResult` constructor refuses to arm without an epoch, so `HumanText` +can drop its optional-epoch branch. A new focused file `internal/app/rungate_epoch_refusal.go` +classifies `EpochError`s into stable reason tokens and next-action messages. It is shared by +`mapDriveFailure` (gate drive start), a new resolvability pre-check in `GateDriveService.PrepareScope`, +and a new `agent.enter` preflight (`CheckRunEpochLinkage`) that runs before Codex is spawned. + +**Tech Stack:** Go (`internal/app`, `internal/cli`, `internal/gatedrive`), `go test`, cobra CLI. + +**Spec:** `docs/superpowers/specs/2026-09-27-resume-gate-armed-line-is-ambiguous-when-no-epoch-exists-dis-design.md` +(on the `docket` metadata branch; read it alongside this plan). + +## Global Constraints + +- The new refusal token is spelled exactly `unknown-run-epoch` (exported constant `ReasonUnknownRunEpoch`). +- Existing tokens are unchanged: `stale-run-epoch`, `run-cancelled`, `run-completed`, `invalid-request`. +- A reason is always a fixed vocabulary token, never argv, a path, or record content. A next-action message never echoes the presented `--run-epoch` value. +- `run gate-before` JSON result shape is unchanged (`key`, `epoch`, `dispatch_context`, … — same field names and tags). +- A mint or bind failure on the arm path returns `gate-unarmed mint-failed` (`ReasonGateMintFailed`), and no key is returned. +- Parent-facing prose (`AGENTS.md`, `cursor-rules/run-gate.md`, `internal/assets/embedded/tree/cursor-rules/run-gate.md`) already documents the three-token form and is **not edited**. Point-in-time records (archived changes, results, specs, published plans under `docs/superpowers/plans/`) are never edited. +- No per-change resume lock and no credential-hash detection of a dispatch context passed as `--run-epoch` (spec Out of scope). +- Code comments anchor on symbol names or verbatim-quoted clauses, never line numbers (ADR-0054, `TestCommentAnchorStyle`). +- Every new test is mutation-checked: strip the fix and watch it go red. Run mutation probes with `go test -count=1` (the cache otherwise serves pre-mutation verdicts). Restore from a backup copy (`cp f f.bak; ; ; mv -f f.bak f`), never `git checkout --`, which throws away your uncommitted edit. +- The build gate runs the whole suite through `build.test_command` (`go run ./cmd/docket development test`), not only the tests named here. + +## Review Focus + +1. **Worktree spelled through a symlink** (macOS `/var/...` vs `/private/var/...`). The resume inspect path and the start's `--repo-dir` may name one directory under two spellings, and the start must still be admitted: `epochOwnsWorktree` canonicalizes at compare time. Pinned in Task 6 (inspect with the raw temp path, start with the `EvalSymlinks` path). +2. **The epoch a resume mints goes through the normal cancel → resume cycle.** After it is cancelled, the next resume must reserve exactly one replacement and supersede it, like any other epoch. Pinned in Task 1. +3. **Parent swaps the epoch and dispatch-context fields.** A dispatch context presented as `--run-epoch` against a gate that has a real epoch is refused `unknown-run-epoch` and never admitted. Pinned in Task 6 (the misrouted leg). +4. **`run gate-before --resume --json` consumers.** The `epoch` field is populated on the epochless path and the rest of the JSON shape is unchanged. Pinned in Task 1. +5. **Human-mode (non-`--json`) refusals.** `gate drive prepare-scope` and `agent enter` render the reason and remedy without echoing the presented value. Pinned in Tasks 4 and 5. + +--- + +## File Structure + +- `internal/app/rungate_before.go` (modify): step (6a) mints on epochless resume and binds ChangeID+Worktree. Adds the new `armedGateResult`. `HumanText` always prints three tokens. Stale comments are corrected. +- `internal/app/rungate_epoch_refusal.go` (create): run-epoch refusal vocabulary. Contains `ReasonUnknownRunEpoch`, `ClassifyRunEpochError`, `RunEpochNextAction`, `runEpochLocator`, and `CheckRunEpochLinkage`. +- `internal/app/gate_drive.go` (modify): `mapDriveFailure`/`mapDriveResult` classify `EpochError`. `GateScopeResult.Message`. `GateDriveService.epochLocate` plus the `PrepareScope` pre-check, wired in `NewCommandlessGateDriveService`. +- `internal/cli/agent.go` (modify): `agent enter` preflight, plus classification of registration errors. +- `internal/cli/run.go` (modify): `gate-before` comment + cobra `Short`. +- Tests: `internal/app/rungate_before_resume_test.go`, `internal/app/rungate_before_test.go`, `internal/app/rungate_epoch_refusal_test.go` (create), `internal/app/gate_drive_test.go`, `internal/app/rungate_epochless_resume_e2e_test.go` (create), `internal/cli/gate_test.go`, `internal/cli/agent_test.go`. + +--- + +### Task 1: Epochless resume mints and binds a run epoch + +**Files:** +- Modify: `internal/app/rungate_before.go` (function `RunGateBefore` steps (4a), (6a), (7); function `armResumeReplacement` final return; field comment on `RunGateBeforeResult.Epoch`; `RunGateBefore` doc comment) +- Test: `internal/app/rungate_before_resume_test.go` (append) + +**Interfaces:** +- Consumes: `MintEpochRecord(repoDir, gateKey, changeID string) (EpochRecord, error)`, `epochCAS(repoDir, gateKey string, mutate func(*EpochRecord) error) error`, `FindEpochByChange`, `LoadEpochRecord` (all existing, `rungate_epoch.go`). +- Produces: `func armedGateResult(key, epochID, dispatchContext string) RunGateBeforeResult` (package-private), which returns `gateUnarmed(ReasonGateMintFailed)` when `epochID == ""`. Tasks 2 and 6 rely on it: every armed result has a non-empty `Epoch`. + +- [ ] **Step 1: Write the failing tests** (append to `internal/app/rungate_before_resume_test.go`; add `"encoding/json"` to its imports) + +```go +// TestEpochlessResumeMintsBoundEpoch (change 0463): resuming an in-progress change +// that has NO prior run epoch (its first dispatch was never armed) mints one. The +// epoch is bound to the change and to the verified feature worktree, and the result +// carries its id, so the armed line is always `gate-armed `. +func TestEpochlessResumeMintsBoundEpoch(t *testing.T) { + repoDir := newWorkingRepo(t, nil).invocation + deps, wdeps := resumeEpochDeps(t) + sp := &fakeScopePrep{grant: sampleScopeGrant()} + if _, _, found, err := FindEpochByChange(repoDir, "5"); err != nil || found { + t.Fatalf("fixture must start epochless: found=%v err=%v", found, err) + } + + res := RunGateBefore(context.Background(), deps, wdeps, sp.deps(), repoDir, "implement-next", 5) + if !res.Armed || res.Key == "" { + t.Fatalf("an epochless resume must arm: %q", res.HumanText()) + } + if res.Epoch == "" { + t.Fatalf("an epochless resume armed with no epoch: %q", res.HumanText()) + } + ep, _, err := LoadEpochRecord(repoDir, res.Key) + if err != nil { + t.Fatalf("LoadEpochRecord: %v", err) + } + if ep.EpochID != res.Epoch { + t.Errorf("result Epoch = %q, want the minted epoch id %q", res.Epoch, ep.EpochID) + } + if ep.ChangeID != "5" { + t.Errorf("epoch ChangeID = %q, want \"5\" (bound to the resumed change)", ep.ChangeID) + } + if ep.State != EpochActive { + t.Errorf("epoch state = %q, want active", ep.State) + } + if ep.Worktree != "/tmp/wt/epsilon" { + t.Errorf("epoch Worktree = %q, want the verified feature worktree /tmp/wt/epsilon", ep.Worktree) + } + // JSON consumers see the epoch on this path too, and the shape is unchanged. + buf, err := json.Marshal(res) + if err != nil { + t.Fatalf("marshal: %v", err) + } + for _, want := range []string{`"epoch":"` + res.Epoch + `"`, `"key":"` + res.Key + `"`, `"dispatch_context":"` + scopeGrantChild + `"`} { + if !strings.Contains(string(buf), want) { + t.Errorf("JSON result missing %s: %s", want, buf) + } + } +} + +// TestRepeatEpochlessResumeRefusedActive (change 0463): after an epochless resume +// mints its epoch, a SECOND resume of the same change finds that epoch active and +// refuses resume-active-run with the safe locator, the same single-live-run +// protection every other epoch gets. It mints nothing and prepares no scope. +func TestRepeatEpochlessResumeRefusedActive(t *testing.T) { + repoDir := newWorkingRepo(t, nil).invocation + deps, wdeps := resumeEpochDeps(t) + sp := &fakeScopePrep{grant: sampleScopeGrant()} + first := RunGateBefore(context.Background(), deps, wdeps, sp.deps(), repoDir, "implement-next", 5) + if !first.Armed { + t.Fatalf("first epochless resume must arm: %q", first.HumanText()) + } + + deps2, wdeps2 := resumeEpochDeps(t) + sp2 := &fakeScopePrep{grant: sampleScopeGrant()} + second := RunGateBefore(context.Background(), deps2, wdeps2, sp2.deps(), repoDir, "implement-next", 5) + if second.Armed { + t.Fatalf("a repeat resume over a live minted epoch must not arm: %q", second.HumanText()) + } + if second.Reason != ReasonGateResumeActiveRun { + t.Fatalf("Reason = %q, want %q", second.Reason, ReasonGateResumeActiveRun) + } + if !strings.Contains(second.Message, first.Epoch) || !strings.Contains(second.Message, first.Key) { + t.Fatalf("locator must name epoch %q and key %q, got %q", first.Epoch, first.Key, second.Message) + } + if second.Key != "" || sp2.calls != 0 { + t.Fatalf("an active refusal mints nothing: key=%q calls=%d", second.Key, sp2.calls) + } +} + +// TestEpochlessResumeEpochJoinsCancelCycle (change 0463, Review Focus 2): the epoch +// an epochless resume mints goes through the ordinary lifecycle. Once confirmed +// cancelled, the next resume supersedes it and reserves exactly one replacement, +// which carries its own fresh epoch. +func TestEpochlessResumeEpochJoinsCancelCycle(t *testing.T) { + repoDir := newWorkingRepo(t, nil).invocation + deps, wdeps := resumeEpochDeps(t) + sp := &fakeScopePrep{grant: sampleScopeGrant()} + first := RunGateBefore(context.Background(), deps, wdeps, sp.deps(), repoDir, "implement-next", 5) + if !first.Armed { + t.Fatalf("first epochless resume must arm: %q", first.HumanText()) + } + // A confirmed run.cancel leaves the epoch cancelled. + if err := epochCAS(repoDir, first.Key, func(r *EpochRecord) error { + r.State = EpochCancelled + return nil + }); err != nil { + t.Fatalf("epochCAS cancel: %v", err) + } + + deps2, wdeps2 := resumeEpochDeps(t) + sp2 := &fakeScopePrep{grant: sampleScopeGrant()} + repl := RunGateBefore(context.Background(), deps2, wdeps2, sp2.deps(), repoDir, "implement-next", 5) + if !repl.Armed || repl.Epoch == "" || repl.Epoch == first.Epoch { + t.Fatalf("the resume after cancel must arm one replacement with a fresh epoch: %q (first epoch %q)", repl.HumanText(), first.Epoch) + } + prior, _, err := LoadEpochRecord(repoDir, first.Key) + if err != nil { + t.Fatalf("LoadEpochRecord(prior): %v", err) + } + if prior.State != EpochSuperseded || prior.ReplacementReserved != repl.Key { + t.Fatalf("prior epoch = (%q, reserved %q), want superseded reserving %q", prior.State, prior.ReplacementReserved, repl.Key) + } +} + +// TestConcurrentEpochlessResumeEpochsFailSafe (change 0463 decision 4, a documenting +// test): epoch locks are per gate key, so two epochless resumes that race past +// FindEpochByChange can each mint an active epoch for one change. The next resume +// must then fail closed as resume-epoch-unreadable and must never arm a third run. +func TestConcurrentEpochlessResumeEpochsFailSafe(t *testing.T) { + repoDir := newWorkingRepo(t, nil).invocation + seedPriorEpoch(t, repoDir, EpochActive) + seedPriorEpoch(t, repoDir, EpochActive) + deps, wdeps := resumeEpochDeps(t) + sp := &fakeScopePrep{grant: sampleScopeGrant()} + + res := RunGateBefore(context.Background(), deps, wdeps, sp.deps(), repoDir, "implement-next", 5) + if res.Armed { + t.Fatalf("an ambiguous pair of live epochs must never arm: %q", res.HumanText()) + } + if res.Reason != ReasonGateResumeEpochUnreadable { + t.Fatalf("Reason = %q, want %q", res.Reason, ReasonGateResumeEpochUnreadable) + } + if res.Key != "" || sp.calls != 0 { + t.Fatalf("a fail-closed refusal mints nothing: key=%q calls=%d", res.Key, sp.calls) + } +} + +// TestArmedGateResultRequiresEpoch (change 0463): the armed constructor refuses to +// arm without an epoch. That guarantee is what makes the positional three-token +// line unambiguous. +func TestArmedGateResultRequiresEpoch(t *testing.T) { + if got := armedGateResult("k", "", "ctx"); got.Armed || got.Reason != ReasonGateMintFailed || got.Key != "" || got.DispatchContext != "" { + t.Fatalf("an epochless armed result must fail closed as gate-unarmed mint-failed, got %+v", got) + } + got := armedGateResult("k", "e", "ctx") + if !got.Armed || got.Result != ResultApplied || got.Key != "k" || got.Epoch != "e" || got.DispatchContext != "ctx" || + got.Target != gateBeforeStoredTarget || got.OwnerLifecycle != ReasonOwnerLifecycleUnavailable { + t.Fatalf("armed result fields wrong: %+v", got) + } +} +``` + +- [ ] **Step 2: Run the tests and confirm they fail** + +Run: `go test -count=1 ./internal/app/ -run 'TestEpochlessResumeMintsBoundEpoch|TestRepeatEpochlessResumeRefusedActive|TestEpochlessResumeEpochJoinsCancelCycle|TestConcurrentEpochlessResumeEpochsFailSafe|TestArmedGateResultRequiresEpoch'` +Expected: build failure `undefined: armedGateResult`. With that test temporarily commented out, `TestEpochlessResumeMintsBoundEpoch` fails `armed with no epoch`, and `TestRepeatEpochlessResumeRefusedActive` fails because the second resume arms. `TestConcurrentEpochlessResumeEpochsFailSafe` already passes (it documents existing behavior). + +- [ ] **Step 3: Implement** + +In `internal/app/rungate_before.go`, add `armedGateResult` directly after `gateResumeObserve`: + +```go +// armedGateResult builds the armed report for key. Every armed gate carries a run +// epoch (change 0463): parents read the `gate-armed ` +// line positionally, and both tokens are 32-hex, so the line is unambiguous only +// when the epoch slot is always filled. An empty epoch therefore fails closed as +// gate-unarmed mint-failed. It never prints a two-token line whose dispatch context +// a parent would read as the epoch. +func armedGateResult(key, epochID, dispatchContext string) RunGateBeforeResult { + if epochID == "" { + return gateUnarmed(ReasonGateMintFailed) + } + return newRunGateBeforeResult(ResultApplied, RunGateBeforeResult{ + Armed: true, + Key: key, + Epoch: epochID, + Target: gateBeforeStoredTarget, + DispatchContext: dispatchContext, + OwnerLifecycle: ReasonOwnerLifecycleUnavailable, + }) +} +``` + +In `armResumeReplacement`, replace the final `return newRunGateBeforeResult(ResultApplied, RunGateBeforeResult{...})` literal (keep the comment above it) with: + +```go + return armedGateResult(key, epochRec.EpochID, grant.ChildCapability) +``` + +In `RunGateBefore`, replace the step (4a) comment's last sentence ("No prior epoch (a legacy/pre-epoch resume, or an unclaimed run that never bound one) falls through to the existing resume arm, which shares no epoch and reserves no replacement.") with: + +```go + // ... No prior epoch (a legacy/pre-epoch resume, or a first dispatch that was + // never armed) falls through to the ordinary arm below, which mints and binds a + // fresh epoch for the resumed change (step 6a, change 0463), so the armed line + // always carries one. +``` + +Replace the whole step (6a) block, from its comment through the closing `}` of `if resumeID == 0 { ... }`, and the step (7) return, with: + +```go + // (6a) Every armed gate binds a run epoch beside the just-minted gate record, + // keyed by the gate key (rungate_epoch.go). The epoch is the durable coordinator + // fence that a later human cancellation flips and a resume supersedes. Its EpochID + // travels onto each scoped start's worktree slot, so an omitted or stale epoch + // cannot detach the worktree. Two arms reach this step: a FRESH arm, and a RESUME + // whose change has no prior epoch (a legacy/pre-epoch run, or a first dispatch + // that was never armed; change 0463). A resume that found a prior epoch never gets + // here, because every found state returned above (a refusal, an observed + // reservation, or armResumeReplacement, which mints its own). + // + // The epoch is minted unbound. A fresh arm binds ChangeID and Worktree later, at + // claim confirmation (bindEpochChange / bindEpochWorktree). A resume has already + // claimed, so it binds both NOW, in one epochCAS, the same way + // armResumeReplacement binds its worktree. Why both are needed: + // - epochLaunchGate refuses an active epoch that has no Worktree, so an unbound + // resume epoch would be refused on first use. + // - With ChangeID bound, a later resume of the same change finds this epoch + // active and refuses resume-active-run. + // Binding both in one CAS means a failed bind leaves an UNBOUND orphan (inert, + // like a fresh arm's), never an orphan that names the change. A mint or bind + // failure unarms fail-closed; the orphan gate record left behind is inert, because + // no key is returned and nothing dispatches against it. + epochRec, eerr := MintEpochRecord(repoDir, key, "") + if eerr != nil { + return gateUnarmed(ReasonGateMintFailed) + } + if resumeID != 0 { + if werr := epochCAS(repoDir, key, func(rec *EpochRecord) error { + rec.ChangeID = scopeChangeID + rec.Worktree = worktree + return nil + }); werr != nil { + return gateUnarmed(ReasonGateMintFailed) + } + } + + // (7) Report the armed gate with its dispatch context, its run epoch id, and the + // honest owner-lifecycle caveat: the dispatched route has no automatic Stop, so a + // Stop is the explicit `run.cancel` operation keyed by this epoch (change 0375 + // Task 13). armedGateResult refuses an empty epoch, so the line is always three + // tokens (change 0463). + return armedGateResult(key, epochRec.EpochID, grant.ChildCapability) +``` + +Remove the now-unused `var epochID string` declaration. + +Update the `RunGateBeforeResult.Epoch` field comment: replace "Empty only on a legacy resume arm that shares no epoch; the resume-active locator already prints the epoch there." with "Never empty on an armed result (change 0463): armedGateResult refuses to arm without one, so the positional `gate-armed ` line always has three tokens." + +Update the `RunGateBefore` doc comment: replace "mints the durable record, and returns `gate-armed `" with "mints the durable record and its run epoch, and returns `gate-armed `". + +- [ ] **Step 4: Run the tests and confirm they pass** + +Run: `go test -count=1 ./internal/app/ -run 'TestEpochlessResume|TestRepeatEpochlessResumeRefusedActive|TestConcurrentEpochlessResumeEpochsFailSafe|TestArmedGateResultRequiresEpoch|TestGateBefore|TestResume|TestRepeatArm|TestMintSnapshotsRunMaxAttempts'` +Expected: PASS. The existing resume and fresh-arm tests stay green. + +- [ ] **Step 5: Mutation-check** (backup-copy restore; `-count=1`) + +- Restore `if resumeID == 0 {` around the mint (and the `epochID` var): `TestEpochlessResumeMintsBoundEpoch` reddens (the arm is refused `mint-failed` by the guard). +- Drop `rec.Worktree = worktree` from the resume CAS: `TestEpochlessResumeMintsBoundEpoch` reddens on Worktree. +- Drop `rec.ChangeID = scopeChangeID`: `TestRepeatEpochlessResumeRefusedActive` reddens (the second resume arms). +- Remove the `epochID == ""` guard in `armedGateResult`: `TestArmedGateResultRequiresEpoch` reddens. +- In `FindEpochByChange`, return the first live match instead of `ErrEpochAmbiguous`: `TestConcurrentEpochlessResumeEpochsFailSafe` reddens. + +Restore each mutation from the backup and confirm green again. + +- [ ] **Step 6: Commit** + +```bash +git add internal/app/rungate_before.go internal/app/rungate_before_resume_test.go +git commit -m "fix(rungate): an epochless resume arm mints and binds a run epoch (change 0463)" +``` + +--- + +### Task 2: The armed line is always three tokens + +**Files:** +- Modify: `internal/app/rungate_before.go` (`RunGateBeforeResult.HumanText` body and doc comment; file header comment block at the top of the file) +- Modify: `internal/cli/run.go` (the `gate-before` comment block and the cobra `Short`) +- Test: `internal/app/rungate_before_test.go` (append) + +**Interfaces:** +- Consumes: `armedGateResult` (Task 1); test helpers `resumeEpochDeps`, `seedPriorEpoch` (`rungate_before_resume_test.go`), and `gateBeforeReader`, `gateBeforeCorpus`, `sampleScopeGrant`, `newGateRepo`, `newWorkingRepo`. +- Produces: `HumanText()` whose first line is always `gate-armed ` (Task 6 parses it positionally). + +- [ ] **Step 1: Write the failing test** (append to `internal/app/rungate_before_test.go`) + +```go +// TestGateArmedLineIsAlwaysThreeTokens (change 0463): every armed result a real arm +// produces (fresh, epochless resume, cancelled-replacement resume) prints a first +// line of exactly four space-separated fields. Field 3 is the epoch and field 4 is +// the dispatch context, so a positional parser can never read the dispatch context +// as the epoch. +func TestGateArmedLineIsAlwaysThreeTokens(t *testing.T) { + check := func(t *testing.T, res RunGateBeforeResult) { + t.Helper() + if !res.Armed { + t.Fatalf("did not arm: %q", res.HumanText()) + } + first := strings.SplitN(res.HumanText(), "\n", 2)[0] + fields := strings.Fields(first) + if len(fields) != 4 || fields[0] != "gate-armed" { + t.Fatalf("armed line %q: want exactly `gate-armed `", first) + } + if fields[1] != res.Key || fields[2] != res.Epoch || fields[3] != res.DispatchContext { + t.Fatalf("armed line %q: fields (%q,%q,%q), want (key %q, epoch %q, dispatch context %q)", + first, fields[1], fields[2], fields[3], res.Key, res.Epoch, res.DispatchContext) + } + if res.Epoch == "" || res.Epoch == res.DispatchContext { + t.Fatalf("epoch %q must be a distinct non-empty token from the dispatch context %q", res.Epoch, res.DispatchContext) + } + } + t.Run("fresh arm", func(t *testing.T) { + repo := newGateRepo(t) + deps := PlanningDeps{Reader: gateBeforeReader(t, gateBeforeCorpus(), nil, nil), Clock: testClock()} + sp := &fakeScopePrep{grant: sampleScopeGrant()} + check(t, RunGateBefore(context.Background(), deps, WorkspaceDeps{}, sp.deps(), repo, "implement-next", 0)) + }) + t.Run("epochless resume", func(t *testing.T) { + repoDir := newWorkingRepo(t, nil).invocation + deps, wdeps := resumeEpochDeps(t) + sp := &fakeScopePrep{grant: sampleScopeGrant()} + check(t, RunGateBefore(context.Background(), deps, wdeps, sp.deps(), repoDir, "implement-next", 5)) + }) + t.Run("cancelled-replacement resume", func(t *testing.T) { + repoDir := newWorkingRepo(t, nil).invocation + seedPriorEpoch(t, repoDir, EpochCancelled) + deps, wdeps := resumeEpochDeps(t) + sp := &fakeScopePrep{grant: sampleScopeGrant()} + check(t, RunGateBefore(context.Background(), deps, wdeps, sp.deps(), repoDir, "implement-next", 5)) + }) +} +``` + +- [ ] **Step 2: Run it** + +Run: `go test -count=1 ./internal/app/ -run TestGateArmedLineIsAlwaysThreeTokens` +Expected: PASS already on top of Task 1 (the invariant now holds). Its value is as a regression pin. Step 5 proves it reddens on the original defect. + +- [ ] **Step 3: Implement** + +`HumanText` (in `internal/app/rungate_before.go`): replace + +```go + line := "gate-armed " + r.Key + if r.Epoch != "" { + line += " " + r.Epoch + } + line += " " + r.DispatchContext +``` + +with + +```go + line := "gate-armed " + r.Key + " " + r.Epoch + " " + r.DispatchContext +``` + +Replace the `HumanText` doc comment's first sentence ("An armed gate prints `gate-armed ` (the epoch is omitted only on a legacy resume arm that shares no epoch);") with: "An armed gate prints `gate-armed `. That is always three tokens, because every armed result carries an epoch (armedGateResult, change 0463), so a positional parser can never read the dispatch context as the epoch." + +File header block at the top of `rungate_before.go`: replace "`gate-armed ` on success" with "`gate-armed ` on success". + +`internal/cli/run.go`, the `gate-before` comment: replace "prints `gate-armed\n\t// ` (or `gate-unarmed `)" with "prints `gate-armed\n\t// ` (or `gate-unarmed `)". Set the cobra `Short` to: + +```go + Short: "Arm the run gate for a dispatched workflow and print gate-armed ", +``` + +- [ ] **Step 4: Prose audit (derive the sites; never hand-list them)** + +Run: +```bash +out=$(grep -rn -e 'gate-armed' . --include='*.go' --include='*.md' 2>/dev/null) +grep -v -e '^./docs/superpowers/' -e '^./docs/results/' -e '^./docs/changes/' -e '^./.git/' -e '_test.go:' <<<"$out" +``` +Expected: every remaining prose site documents the three-token form, or is a `Disposition: "gate-armed"` record literal. That means `AGENTS.md`, `cursor-rules/run-gate.md`, and `internal/assets/embedded/tree/cursor-rules/run-gate.md` stay unchanged, and `internal/app/rungate_before.go` plus `internal/cli/run.go` are now fixed. If any other maintained site documents a two-token or optional-epoch form, fix it in this task and list it in the commit body. + +Then run `go test -count=1 ./internal/app/ ./internal/cli/ -run 'TestGateBefore|TestGateArmedLine|TestCapability|TestCommentAnchorStyle'` and `go test -count=1 ./internal/repoguard/`. +Expected: PASS. + +- [ ] **Step 5: Mutation-check** (backup-copy restore) + +Reintroduce the original defect as a pair: restore `if resumeID == 0 {` around the Task 1 mint, remove the `epochID == ""` guard in `armedGateResult`, and restore the `if r.Epoch != ""` conditional in `HumanText`. The `epochless resume` subtest reddens with a 3-field line. Restore and confirm green. + +- [ ] **Step 6: Commit** + +```bash +git add internal/app/rungate_before.go internal/app/rungate_before_test.go internal/cli/run.go +git commit -m "fix(rungate): the gate-armed line always prints (change 0463)" +``` + +--- + +### Task 3: Named refusal for an unknown run epoch on gate drive start + +**Files:** +- Create: `internal/app/rungate_epoch_refusal.go` +- Create: `internal/app/rungate_epoch_refusal_test.go` +- Modify: `internal/app/gate_drive.go` (`mapDriveFailure`, `mapDriveResult`) +- Test: `internal/app/gate_drive_test.go` (append), `internal/cli/gate_test.go` (append) + +**Interfaces:** +- Consumes: `AsEpochError`, `EpochErrorKind` constants, `ErrStaleRunEpoch` (`*MutationFenceError`, `.Reason == "stale-run-epoch"`), `Result` constants. +- Produces (used by Tasks 4, 5, and 6): + - `const ReasonUnknownRunEpoch = "unknown-run-epoch"` + - `func ClassifyRunEpochError(err error) (Result, string, bool)`: `ok == false` when no `*EpochError` is in the chain. + - `func RunEpochNextAction(reason string) string`: a message for `unknown-run-epoch` and `stale-run-epoch`, `""` otherwise. + +- [ ] **Step 1: Write the failing tests** + +`internal/app/rungate_epoch_refusal_test.go`: + +```go +package app + +import ( + "errors" + "fmt" + "strings" + "testing" +) + +// TestClassifyRunEpochError (change 0463): every run-epoch registry failure maps to +// a stable protocol result and a fixed reason token. A not-found epoch is the named +// unknown-run-epoch. A presented value wrapped into the error chain never leaks into +// the reason. +func TestClassifyRunEpochError(t *testing.T) { + const presented = "0790b760e26444866ef2e156ba383326" + cases := []struct { + kind EpochErrorKind + res Result + reason string + }{ + {ErrEpochNotFound, ResultInvalidInput, "unknown-run-epoch"}, + {ErrEpochMismatch, ResultInvalidInput, "stale-run-epoch"}, + {ErrEpochAmbiguous, ResultInvalidInput, "epoch-ambiguous"}, + {ErrEpochNotActive, ResultInvalidInput, "epoch-not-active"}, + {ErrEpochOwnerAmbiguous, ResultInvalidInput, "epoch-owner-ambiguous"}, + {ErrEpochOwnerUnresolved, ResultInvalidInput, "epoch-owner-unresolved"}, + {ErrEpochCorrupt, ResultInternalError, "epoch-corrupt"}, + {ErrEpochIO, ResultInternalError, "epoch-io"}, + } + for _, tc := range cases { + err := fmt.Errorf("start refused for %s: %w", presented, epochErr(tc.kind, "find-dir-by-id", errors.New(presented))) + res, reason, ok := ClassifyRunEpochError(err) + if !ok || res != tc.res || reason != tc.reason { + t.Errorf("%s: got (%s, %q, %v), want (%s, %q, true)", tc.kind, res, reason, ok, tc.res, tc.reason) + } + if strings.Contains(reason, presented) { + t.Errorf("%s: reason leaked the presented value: %q", tc.kind, reason) + } + } + if _, _, ok := ClassifyRunEpochError(errors.New("plain failure")); ok { + t.Error("a non-epoch error must not classify") + } + if _, _, ok := ClassifyRunEpochError(ErrStaleRunEpoch); ok { + t.Error("a mutation-fence error is not an epoch-registry error") + } +} + +// TestRunEpochNextAction (change 0463): unknown-run-epoch tells the caller which +// gate-armed field goes where. Its text is distinct from the stale-linkage remedy. +// Other reasons carry no invented message. +func TestRunEpochNextAction(t *testing.T) { + unknown := RunEpochNextAction(ReasonUnknownRunEpoch) + for _, want := range []string{"--run-epoch", "--gate-context", "gate-armed "} { + if !strings.Contains(unknown, want) { + t.Errorf("unknown-run-epoch message must mention %q, got %q", want, unknown) + } + } + stale := RunEpochNextAction("stale-run-epoch") + if stale == "" || stale == unknown { + t.Errorf("stale-run-epoch needs its own message, got %q", stale) + } + if got := RunEpochNextAction("epoch-io"); got != "" { + t.Errorf("an unmapped reason must yield no message, got %q", got) + } +} +``` + +Append to `internal/app/gate_drive_test.go`: + +```go +// TestMapDriveFailureEpochErrors (change 0463): an EpochError chained through the +// gate-drive seam (the epoch launch gate refusing an unknown --run-epoch) surfaces +// its named token, never the catch-all invalid-request. The service attaches the +// next-action message, and neither the reason nor the message echoes the value. +func TestMapDriveFailureEpochErrors(t *testing.T) { + const presented = "0790b760e26444866ef2e156ba383326" + wrapped := fmt.Errorf("refused %s: %w", presented, epochErr(ErrEpochNotFound, "find-dir-by-id", nil)) + res, reason := mapDriveFailure(wrapped) + if res != ResultInvalidInput || reason != ReasonUnknownRunEpoch { + t.Fatalf("mapDriveFailure = (%s, %q), want (invalid-input, unknown-run-epoch)", res, reason) + } + if res, reason := mapDriveFailure(epochErr(ErrEpochIO, "find-by-id", nil)); res != ResultInternalError || reason != "epoch-io" { + t.Fatalf("an unreadable registry must be an internal error, got (%s, %q)", res, reason) + } + eng := &fakeDriveEngine{err: wrapped} + got := newGateDriveService(eng, 0, "", "").Advance("d1", "owner") + if got.Reason != ReasonUnknownRunEpoch { + t.Fatalf("service reason = %q, want unknown-run-epoch", got.Reason) + } + if !strings.Contains(got.Message, "--gate-context") { + t.Fatalf("service must attach the unknown-run-epoch next action, got %q", got.Message) + } + if strings.Contains(got.Message, presented) || strings.Contains(got.HumanText(), presented) { + t.Fatalf("the presented value leaked: message=%q human=%q", got.Message, got.HumanText()) + } +} +``` + +Append to `internal/cli/gate_test.go`: + +```go +// TestGateDriveStartUnknownRunEpochIsNamed (change 0463): the 0382 misuse, where a +// well-formed but unknown --run-epoch (a dispatch-context-shaped 32-hex token) goes +// through the REAL epoch launch gate, is refused invalid-input with the named +// unknown-run-epoch, never the catch-all invalid-request. The presented value is +// never echoed. +func TestGateDriveStartUnknownRunEpochIsNamed(t *testing.T) { + wt := gateDriveConfiguredRepo(t, "metadata_branch: main\n") + root := gateTempDir(t) + const bogus = "0790b760e26444866ef2e156ba383326" + out, _, _ := runCLI(t, "--json", "gate", "drive", "start", + "--repo-dir", wt, "--run-root", root, "--owner", "task", + "--change-id", "463", "--task-id", "task-3", "--phase", "build", "--branch", "fix/x", + "--run-epoch", bogus, "--", "/bin/echo", "hi") + doc := decodeOneJSON(t, out) + if doc["result"] != "invalid-input" || doc["reason"] != "unknown-run-epoch" { + t.Fatalf("unknown --run-epoch must refuse invalid-input/unknown-run-epoch, got %v", doc) + } + if _, ok := doc["drive"]; ok { + t.Fatalf("a refused start must carry no drive document: %v", doc) + } + if msg, _ := doc["message"].(string); !strings.Contains(msg, "--gate-context") { + t.Fatalf("refusal must carry the next action, got %q", msg) + } + if strings.Contains(out, bogus) { + t.Fatalf("the presented --run-epoch value leaked into the output: %s", out) + } +} +``` + +- [ ] **Step 2: Run the tests and confirm they fail** + +Run: `go test -count=1 ./internal/app/ -run 'TestClassifyRunEpochError|TestRunEpochNextAction|TestMapDriveFailureEpochErrors'` and `go test -count=1 ./internal/cli/ -run TestGateDriveStartUnknownRunEpochIsNamed` +Expected: app build fails with `undefined: ClassifyRunEpochError / ReasonUnknownRunEpoch / RunEpochNextAction`. The CLI test fails with `reason:invalid-request`. + +- [ ] **Step 3: Implement** + +Create `internal/app/rungate_epoch_refusal.go`: + +```go +package app + +// This file is the run-epoch refusal vocabulary (change 0463). The run epoch is a +// public locator (ADR-0111) that a caller threads into --run-epoch flags (gate drive +// start, gate drive prepare-scope, agent.enter). When the presented value cannot be +// resolved, the caller must learn WHICH mistake it made through a stable token. A +// catch-all invalid-request makes a misrouted token (0382: the dispatch context +// passed as the epoch) indistinguishable from a malformed request. Tokens are a +// fixed vocabulary; nothing here echoes the presented value, a path, or record +// content. + +// ReasonUnknownRunEpoch is the stable refusal token for a --run-epoch that names no +// run epoch in this repository. +const ReasonUnknownRunEpoch = "unknown-run-epoch" + +// ClassifyRunEpochError maps a run-epoch registry failure (an *EpochError anywhere +// in err's chain) to a protocol result and a bounded reason token: +// - not-found: unknown-run-epoch. +// - mismatch: the existing stale-linkage token, stale-run-epoch. +// - corrupt or unreadable: internal-error carrying the kind. +// - any other readable-but-unusable registry state: invalid-input carrying the kind. +// +// ok is false when err carries no *EpochError, so callers fall through to their +// own classification. +func ClassifyRunEpochError(err error) (Result, string, bool) { + ee, ok := AsEpochError(err) + if !ok { + return "", "", false + } + switch ee.Kind { + case ErrEpochNotFound: + return ResultInvalidInput, ReasonUnknownRunEpoch, true + case ErrEpochMismatch: + return ResultInvalidInput, ErrStaleRunEpoch.Reason, true + case ErrEpochCorrupt, ErrEpochIO: + return ResultInternalError, string(ee.Kind), true + default: + return ResultInvalidInput, string(ee.Kind), true + } +} + +// RunEpochNextAction maps a run-epoch refusal reason to a one-line, credential-free +// next action (the ownershipNextAction / fenceNextAction pattern). It never echoes +// the presented value. A reason with no specific remedy yields "", and callers then +// omit the message. +func RunEpochNextAction(reason string) string { + switch reason { + case ReasonUnknownRunEpoch: + return "the --run-epoch value names no run epoch in this repository; pass the field of the arm's " + + "`gate-armed ` line (the goes to --gate-context) — " + + "never drop --run-epoch and retry" + case ErrStaleRunEpoch.Reason: + return "the --run-epoch value is not the run epoch this gate key carries; pass the printed on the same gate-armed line as the key" + default: + return "" + } +} +``` + +In `internal/app/gate_drive.go` `mapDriveFailure`, insert directly after the `AsMutationFenceError` block (before `gatedrive.AsStoreError`): + +```go + // A run-epoch registry failure (the epoch launch gate could not resolve the + // presented --run-epoch) surfaces its named token rather than collapsing to the + // generic invalid-request (change 0463): unknown-run-epoch for a not-found epoch, + // the kind for any other registry fault. + if res, reason, ok := ClassifyRunEpochError(err); ok { + return res, reason + } +``` + +In `mapDriveResult`, extend the `if oe, ok := gatedrive.AsOwnershipError(err); ok { ... } else if fe, ok := AsMutationFenceError(err); ok { ... }` chain with: + +```go + } else if _, reason, ok := ClassifyRunEpochError(err); ok { + result.Message = RunEpochNextAction(reason) + } +``` + +- [ ] **Step 4: Run the tests and confirm they pass** + +Run: `go test -count=1 ./internal/app/ -run 'TestClassifyRunEpochError|TestRunEpochNextAction|TestMapDriveFailure|TestEpochLaunchGate'` and `go test -count=1 ./internal/cli/ -run 'TestGateDrive'` +Expected: PASS. + +- [ ] **Step 5: Mutation-check** (backup-copy restore) + +- Delete the `ClassifyRunEpochError` block from `mapDriveFailure`: `TestMapDriveFailureEpochErrors` and `TestGateDriveStartUnknownRunEpochIsNamed` redden (`invalid-request`). +- Delete the `mapDriveResult` branch: the message asserts redden. +- Map `ErrEpochNotFound` to `string(ee.Kind)`: `TestClassifyRunEpochError` reddens. + +- [ ] **Step 6: Commit** + +```bash +git add internal/app/rungate_epoch_refusal.go internal/app/rungate_epoch_refusal_test.go internal/app/gate_drive.go internal/app/gate_drive_test.go internal/cli/gate_test.go +git commit -m "fix(gate): an unknown --run-epoch refuses unknown-run-epoch, not invalid-request (change 0463)" +``` + +--- + +### Task 4: prepare-scope refuses an unresolvable --run-epoch + +`gatedrive.Driver.PrepareScope` stores `RunEpochID` without resolving it, so today a bogus epoch is baked silently into the scope and only fails later, at start. This task adds a resolvability (not liveness) pre-check in the app seam. + +**Files:** +- Modify: `internal/app/rungate_epoch_refusal.go` (add `runEpochLocator`; add imports `"path/filepath"`) +- Modify: `internal/app/gate_drive.go` (`GateScopeResult` + `HumanText`; `GateDriveService` field `epochLocate`; `PrepareScope`; `NewCommandlessGateDriveService`) +- Test: `internal/app/gate_drive_test.go`, `internal/cli/gate_test.go` (append) + +**Interfaces:** +- Consumes: `findEpochDirByID(rungateRoot, epochID string) (string, EpochRecord, error)` (existing), `ClassifyRunEpochError`, `RunEpochNextAction`, `ReasonUnknownRunEpoch` (Task 3), `mapDriveFailure`. +- Produces: `func runEpochLocator(gitCommonDir string) func(string) error`. Field `GateDriveService.epochLocate func(epochID string) error`. Field `GateScopeResult.Message string` (`json:"message,omitempty"`). + +- [ ] **Step 1: Write the failing tests** + +Append to `internal/app/gate_drive_test.go`: + +```go +// TestPrepareScopeRefusesUnknownRunEpoch (change 0463): a presented --run-epoch that +// the registry cannot resolve is refused before any scope is minted, with the named +// token and the next action and without echoing the value. A scope with no +// --run-epoch never consults the locator (standalone scopes are unchanged). +func TestPrepareScopeRefusesUnknownRunEpoch(t *testing.T) { + eng := &fakeDriveEngine{grant: gatedrive.ScopeGrant{ScopeID: "scope-1", ChildCapability: "c", ParentCapability: "p"}} + svc := newGateDriveService(eng, 0, "", "") + var asked string + svc.epochLocate = func(id string) error { + asked = id + return epochErr(ErrEpochNotFound, "find-dir-by-id", nil) + } + got := svc.PrepareScope(gatedrive.ScopeRequest{ChangeID: "463", RunEpochID: "bogus-epoch-value"}) + if got.Result != ResultInvalidInput || got.Reason != ReasonUnknownRunEpoch { + t.Fatalf("got (%s, %q), want (invalid-input, unknown-run-epoch)", got.Result, got.Reason) + } + if got.ScopeID != "" || got.ChildCapability != "" || got.ParentCapability != "" { + t.Fatalf("a refused prepare-scope must carry no grant: %+v", got) + } + if eng.lastScopeReq.ChangeID != "" { + t.Fatalf("an unresolvable epoch must mint no scope, engine saw %+v", eng.lastScopeReq) + } + if asked != "bogus-epoch-value" { + t.Fatalf("locator asked %q, want the presented id", asked) + } + if !strings.Contains(got.Message, "--gate-context") || !strings.Contains(got.HumanText(), "unknown-run-epoch") { + t.Fatalf("refusal must carry reason and next action: message=%q human=%q", got.Message, got.HumanText()) + } + if strings.Contains(got.Message, "bogus-epoch-value") || strings.Contains(got.HumanText(), "bogus-epoch-value") { + t.Fatalf("the presented value leaked: %q / %q", got.Message, got.HumanText()) + } + + asked = "" + if ok := svc.PrepareScope(gatedrive.ScopeRequest{ChangeID: "463"}); ok.Result != ResultApplied || asked != "" { + t.Fatalf("a scope without --run-epoch must skip the locator and apply: result=%s asked=%q", ok.Result, asked) + } +} +``` + +Append to `internal/cli/gate_test.go`: + +```go +// TestGateDrivePrepareScopeUnknownRunEpochIsNamed (change 0463): through the real +// wiring, prepare-scope with an unknown --run-epoch refuses unknown-run-epoch and +// mints no scope. In human mode it renders reason + remedy without the value. The +// cancelled-epoch prepare in TestGateDrivePrepareScopeRunEpochGatesTakeover must +// stay applied, because the pre-check is resolvability only, never liveness. +func TestGateDrivePrepareScopeUnknownRunEpochIsNamed(t *testing.T) { + wt := gateDriveRepo(t) + const bogus = "0790b760e26444866ef2e156ba383326" + args := []string{"gate", "drive", "prepare-scope", "--repo-dir", wt, "--change-id", "463", + "--task-id", "task-4", "--phase", "build", "--branch", "fix/x", "--worktree", wt, "--run-epoch", bogus} + + out, _, _ := runCLI(t, append([]string{"--json"}, args...)...) + doc := decodeOneJSON(t, out) + if doc["result"] != "invalid-input" || doc["reason"] != "unknown-run-epoch" { + t.Fatalf("got %v, want invalid-input/unknown-run-epoch", doc) + } + if id, _ := doc["scope_id"].(string); id != "" { + t.Fatalf("a refused prepare-scope minted scope %q", id) + } + if strings.Contains(out, bogus) { + t.Fatalf("JSON output leaked the presented value: %s", out) + } + + // A non-applied result may render on either stream; check both together. + hOut, hErr, _ := runCLI(t, args...) + human := hOut + hErr + if !strings.Contains(human, "unknown-run-epoch") || !strings.Contains(human, "--gate-context") || strings.Contains(human, bogus) { + t.Fatalf("human output must name reason + remedy and never the value, got %q", human) + } +} +``` + +- [ ] **Step 2: Run the tests and confirm they fail** + +Run: `go test -count=1 ./internal/app/ -run TestPrepareScopeRefusesUnknownRunEpoch` and `go test -count=1 ./internal/cli/ -run TestGateDrivePrepareScopeUnknownRunEpochIsNamed` +Expected: app build fails (`svc.epochLocate undefined`, `got.Message undefined`). The CLI test fails with `result:applied` (a scope was minted). + +- [ ] **Step 3: Implement** + +Append to `internal/app/rungate_epoch_refusal.go` (and add `import "path/filepath"`): + +```go +// runEpochLocator builds the existence check prepare-scope runs on a presented +// --run-epoch. It resolves the id to exactly one epoch record under gitCommonDir's +// run-epoch registry through findEpochDirByID (the same locator the epoch launch +// gate uses) and returns its typed EpochError (not-found, ambiguous, IO) unchanged. +// It checks RESOLVABILITY only, never liveness: a scope may legitimately carry a +// cancelled epoch (the takeover revocation gate reads it later), and the launch gate +// still enforces liveness and worktree ownership at start. +func runEpochLocator(gitCommonDir string) func(string) error { + rungateRoot := filepath.Join(gitCommonDir, "docket", "rungate") + return func(epochID string) error { + _, _, err := findEpochDirByID(rungateRoot, epochID) + return err + } +} +``` + +In `internal/app/gate_drive.go`: + +1. `GateScopeResult`: add after `Reason`: +```go + // Message is the one-line next action on a command failure (change 0463), e.g. + // the unknown-run-epoch remedy. It never carries a capability or the presented + // run-epoch value. + Message string `json:"message,omitempty"` +``` + In `GateScopeResult.HumanText`, append after the reason line: +```go + if r.Message != "" { + lines = append(lines, "message: "+r.Message) + } +``` +2. `GateDriveService`: add the field after `maxAttempts`: +```go + // epochLocate resolves a presented run-epoch id against the repository's run-epoch + // registry before PrepareScope mints a scope (change 0463). A non-nil error is a + // typed EpochError. Nil on the fake-engine test seam and on services that never + // serve prepare-scope. + epochLocate func(epochID string) error +``` +3. `PrepareScope`: insert at the top of the method: +```go + // A presented --run-epoch must resolve before it is baked into the scope (change + // 0463): an unresolvable one refuses now with its named token, instead of + // surfacing later at start as a refusal the caller cannot attribute. + if req.RunEpochID != "" && s.epochLocate != nil { + if lerr := s.epochLocate(req.RunEpochID); lerr != nil { + res, reason := mapDriveFailure(lerr) + return GateScopeResult{ + Envelope: NewEnvelope(OperationGateDrivePrepareScope, res), + Reason: reason, + Message: RunEpochNextAction(reason), + } + } + } +``` +4. `NewCommandlessGateDriveService`: replace `return newGateDriveService(engine, 0, "", ""), "", ""` with: +```go + svc := newGateDriveService(engine, 0, "", "") + // prepare-scope is served by this commandless service: resolve a presented + // --run-epoch against the same registry before minting a scope (change 0463). + svc.epochLocate = runEpochLocator(gitCommonDir) + return svc, "", "" +``` + +- [ ] **Step 4: Run the tests and confirm they pass** + +Run: `go test -count=1 ./internal/app/ -run 'TestPrepareScope'` and `go test -count=1 ./internal/cli/ -run 'TestGateDrivePrepareScope|TestGateDriveScopeBoundStartRoundTrips|TestGateDriveTakeover'` +Expected: PASS, including the existing `TestGateDrivePrepareScopeRunEpochGatesTakeover` (cancelled epoch still prepares). + +- [ ] **Step 5: Mutation-check** (backup-copy restore) + +- Remove the pre-check block from `PrepareScope`: both new tests redden (a scope is minted). +- Remove the `svc.epochLocate = ...` wiring: only the CLI test reddens (this proves the production wiring, not just the seam). +- Make `runEpochLocator` also refuse non-active epochs: `TestGateDrivePrepareScopeRunEpochGatesTakeover` reddens. That confirms the existence-only boundary is pinned. + +- [ ] **Step 6: Commit** + +```bash +git add internal/app/rungate_epoch_refusal.go internal/app/gate_drive.go internal/app/gate_drive_test.go internal/cli/gate_test.go +git commit -m "fix(gate): prepare-scope refuses an unresolvable --run-epoch as unknown-run-epoch (change 0463)" +``` + +--- + +### Task 5: agent.enter preflights its run-epoch linkage + +Traced: `agent enter --run-gate-key K --run-epoch E` registers the Codex thread through `RegisterEpochParticipant` (`epochCAS` on K) only after Codex's app-server has started and a thread exists. A missing epoch (`ErrEpochNotFound`) or a wrong id (`ErrEpochMismatch`) then surfaces as the generic `root-entry-failed` (`ResultExternalFailed`). This task adds a read-only preflight before anything is spawned (before the death guardian and before `client.Enter`), and classifies a registration-time `EpochError` the same way. + +**Files:** +- Modify: `internal/app/rungate_epoch_refusal.go` (add `CheckRunEpochLinkage`) +- Modify: `internal/cli/agent.go` (inside the `if runGateKey != "" && runEpoch != ""` block, and the `client.Enter` error branch) +- Test: `internal/app/rungate_epoch_refusal_test.go`, `internal/cli/agent_test.go` (append) + +**Interfaces:** +- Consumes: `LoadEpochRecord`, `AsGateStoreError`, `ErrGateNotFound`, `ErrGateMalformedKey`, `ClassifyRunEpochError`, `RunEpochNextAction` (Task 3). +- Produces: `func CheckRunEpochLinkage(repoDir, gateKey, epochID string) error`, which returns nil or always an `*EpochError`. + +- [ ] **Step 1: Write the failing tests** + +Append to `internal/app/rungate_epoch_refusal_test.go` (add `"os"` and `"path/filepath"` to its imports): + +```go +// TestCheckRunEpochLinkage (change 0463): the agent.enter preflight answers with a +// typed EpochError. Not-found covers both a gate key with no epoch and a gate key +// that does not exist (the pair names no epoch). Mismatch covers a different +// recorded id. A matching pair is nil. +func TestCheckRunEpochLinkage(t *testing.T) { + repo := newGateRepo(t) + bare := mintTestGateKey(t, repo) + if err := CheckRunEpochLinkage(repo, bare, "0790b760e26444866ef2e156ba383326"); !isEpochKind(err, ErrEpochNotFound) { + t.Fatalf("gate key without an epoch: got %v, want epoch-not-found", err) + } + + withEpoch := mintTestGateKey(t, repo) + ep, err := MintEpochRecord(repo, withEpoch, "463") + if err != nil { + t.Fatalf("MintEpochRecord: %v", err) + } + if err := CheckRunEpochLinkage(repo, withEpoch, ep.EpochID); err != nil { + t.Fatalf("matching pair must pass, got %v", err) + } + if err := CheckRunEpochLinkage(repo, withEpoch, "0790b760e26444866ef2e156ba383326"); !isEpochKind(err, ErrEpochMismatch) { + t.Fatalf("wrong epoch id: got %v, want epoch-mismatch", err) + } + + gone := mintTestGateKey(t, repo) + root, rerr := gateRoot(repo) + if rerr != nil { + t.Fatalf("gateRoot: %v", rerr) + } + if err := os.RemoveAll(filepath.Join(root, gone)); err != nil { + t.Fatalf("remove gate dir: %v", err) + } + if err := CheckRunEpochLinkage(repo, gone, ep.EpochID); !isEpochKind(err, ErrEpochNotFound) { + t.Fatalf("absent gate key: got %v, want epoch-not-found", err) + } +} + +func isEpochKind(err error, kind EpochErrorKind) bool { + ee, ok := AsEpochError(err) + return ok && ee.Kind == kind +} +``` + +Append to `internal/cli/agent_test.go`: + +```go +// TestAgentEnterRefusesBadRunEpochLinkageBeforeLaunch (change 0463): an agent.enter +// whose --run-gate-key/--run-epoch pair names no run epoch, or names a different one, +// is refused with a named token BEFORE Codex is spawned. A stub codex that records +// any invocation proves nothing launched. The presented value never appears in the +// JSON or human output. +func TestAgentEnterRefusesBadRunEpochLinkageBeforeLaunch(t *testing.T) { + seedAgentInstallation(t) + repo := gateDriveRepo(t) + bin := testsupport.TempDir(t) + marker := filepath.Join(bin, "codex-invoked") + stub := "#!/bin/sh\ntouch '" + strings.ReplaceAll(marker, "'", "'\\''") + "'\nexit 1\n" + if err := os.WriteFile(filepath.Join(bin, "codex"), []byte(stub), 0o755); err != nil { + t.Fatal(err) + } + t.Setenv("PATH", bin+string(os.PathListSeparator)+os.Getenv("PATH")) + + const bogus = "0790b760e26444866ef2e156ba383326" + mintKey := func() string { + key, err := app.MintGateRecord(repo, app.GateRecord{Target: "docket-implement-next", AttemptLimit: 1, Retry: app.RetryUnused, Disposition: "gate-armed"}) + if err != nil { + t.Fatalf("MintGateRecord: %v", err) + } + return key + } + bare := mintKey() + withEpoch := mintKey() + if _, err := app.MintEpochRecord(repo, withEpoch, "463"); err != nil { + t.Fatalf("MintEpochRecord: %v", err) + } + + for _, tc := range []struct { + name, key, wantReason string + }{ + {"gate key with no epoch", bare, "unknown-run-epoch"}, + {"epoch id not the key's", withEpoch, "stale-run-epoch"}, + } { + t.Run(tc.name, func(t *testing.T) { + base := []string{"agent", "enter", "--role", "docket-implement-next", "--request", "-", "--cwd", repo, + "--approval-policy", "never", "--sandbox", "workspace-write", "--run-gate-key", tc.key, "--run-epoch", bogus} + var out, stderr bytes.Buffer + Run(append(base, "--json"), strings.NewReader("req"), &out, &stderr, devInfo(), hostFacts()) + var res app.AgentEnterResult + if err := json.Unmarshal(out.Bytes(), &res); err != nil { + t.Fatalf("decode %q: %v (stderr %q)", out.String(), err, stderr.String()) + } + if res.Result != app.ResultInvalidInput || res.Reason != tc.wantReason { + t.Fatalf("got (%s, %q), want (invalid-input, %q): %+v", res.Result, res.Reason, tc.wantReason, res) + } + if strings.Contains(out.String(), bogus) { + t.Fatalf("JSON output leaked the presented value: %s", out.String()) + } + var human, herr bytes.Buffer + Run(base, strings.NewReader("req"), &human, &herr, devInfo(), hostFacts()) + if !strings.Contains(human.String()+herr.String(), "--run-epoch") || strings.Contains(human.String()+herr.String(), bogus) { + t.Fatalf("human output must name the remedy and never the value: out=%q err=%q", human.String(), herr.String()) + } + if _, err := os.Stat(marker); err == nil { + t.Fatalf("codex was launched despite a bad run-epoch linkage") + } + }) + } +} +``` + +(If a git-repository `--cwd` changes how the role contract resolves in this fixture, mirror the setup of `TestAgentEnterCLIUsesEffectiveRepositoryRoleBeforeGlobal`. The assertions stay the same.) + +- [ ] **Step 2: Run the tests and confirm they fail** + +Run: `go test -count=1 ./internal/app/ -run TestCheckRunEpochLinkage` and `go test -count=1 ./internal/cli/ -run TestAgentEnterRefusesBadRunEpochLinkageBeforeLaunch` +Expected: app build fails (`undefined: CheckRunEpochLinkage`). The CLI test fails with `root-entry-failed`, and the marker exists (codex was launched). + +- [ ] **Step 3: Implement** + +Append to `internal/app/rungate_epoch_refusal.go`: + +```go +// CheckRunEpochLinkage verifies, before agent.enter spawns anything, that the +// presented (--run-gate-key, --run-epoch) pair names a real run epoch (change 0463). +// It returns nil when the gate key's epoch record carries exactly epochID, and +// otherwise ALWAYS an *EpochError: +// - a gate key with no directory, a malformed key, or no epoch record: +// ErrEpochNotFound (the pair names no epoch); +// - a different recorded id: ErrEpochMismatch; +// - a corrupt record: keeps ErrEpochCorrupt; +// - any other resolution fault: ErrEpochIO. +// +// It only reads. It never checks liveness, because participant registration still +// refuses a non-active epoch. +func CheckRunEpochLinkage(repoDir, gateKey, epochID string) error { + rec, _, err := LoadEpochRecord(repoDir, gateKey) + if err != nil { + if _, ok := AsEpochError(err); ok { + return err + } + if ge, ok := AsGateStoreError(err); ok && (ge.Kind == ErrGateNotFound || ge.Kind == ErrGateMalformedKey) { + return epochErr(ErrEpochNotFound, "check-linkage", nil) + } + return epochErr(ErrEpochIO, "check-linkage", err) + } + if rec.EpochID != epochID { + return epochErr(ErrEpochMismatch, "check-linkage", nil) + } + return nil +} +``` + +In `internal/cli/agent.go`, add a helper below `epochParticipantRegistrar`: + +```go +// runEpochRefusal renders a typed run-epoch linkage failure as the agent.enter +// refusal (change 0463): the named reason token and a credential-free next action. +// It never includes the presented value. +func runEpochRefusal(role string, res app.Result, reason string) app.AgentEnterResult { + msg := app.RunEpochNextAction(reason) + if msg == "" { + msg = "run-epoch linkage refused (" + reason + ")" + } + return app.AgentEnterResult{Envelope: app.NewEnvelope(app.OperationAgentEnter, res), Role: role, Reason: reason, Message: msg} +} +``` + +Make the preflight the first statement inside `if runGateKey != "" && runEpoch != "" {`, before `kind := "task"`: + +```go + // Preflight the linkage BEFORE anything is spawned (change 0463). An unknown + // or mismatched epoch refuses with its named token, instead of surfacing as a + // generic root-entry failure after Codex already started a thread. + if lerr := app.CheckRunEpochLinkage(effectiveCWD, runGateKey, runEpoch); lerr != nil { + res, reason, _ := app.ClassifyRunEpochError(lerr) + setResult(runEpochRefusal(role, res, reason)) + return nil + } +``` + +In the `out, err := client.Enter(...)` error branch, classify first: + +```go + if err != nil { + // A registration-time epoch fault (e.g. the epoch was fenced after the + // preflight) keeps its named token (change 0463). + if res, reason, ok := app.ClassifyRunEpochError(err); ok { + setResult(runEpochRefusal(role, res, reason)) + return nil + } + setResult(app.AgentEnterResult{Envelope: app.NewEnvelope(app.OperationAgentEnter, app.ResultExternalFailed), Role: role, Reason: "root-entry-failed", Message: err.Error()}) + return nil + } +``` + +- [ ] **Step 4: Run the tests and confirm they pass** + +Run: `go test -count=1 ./internal/app/ -run 'TestCheckRunEpochLinkage|TestClassifyRunEpochError'` and `go test -count=1 ./internal/cli/ -run 'TestAgentEnter'` +Expected: PASS. All existing `TestAgentEnter*` tests stay green (none pass the linkage flags). + +- [ ] **Step 5: Mutation-check** (backup-copy restore) + +- Remove the preflight block: the CLI test reddens (marker created; reason `root-entry-failed`). +- In `CheckRunEpochLinkage`, drop the `ErrGateNotFound` mapping (fall to `ErrEpochIO`): the `absent gate key` leg of `TestCheckRunEpochLinkage` reddens. +- Drop the `rec.EpochID != epochID` check: the mismatch legs redden in both tests. + +- [ ] **Step 6: Commit** + +```bash +git add internal/app/rungate_epoch_refusal.go internal/app/rungate_epoch_refusal_test.go internal/cli/agent.go internal/cli/agent_test.go +git commit -m "fix(agent): agent.enter preflights --run-gate-key/--run-epoch before launch (change 0463)" +``` + +--- + +### Task 6: End-to-end reproduction of the 0382 sequence + +This test proves the whole chain. A change is claimed with no armed gate, so no epoch exists. `gate-before --resume` then prints three tokens. The positionally parsed `` is admitted by the REAL epoch launch gate and lands on the worktree slot, and the misrouted dispatch context is refused `unknown-run-epoch`. The test admits through `Admit` and never launches, so no supervisor process is spawned. + +**Files:** +- Create: `internal/app/rungate_epochless_resume_e2e_test.go` + +**Interfaces:** +- Consumes: `RunGateBefore`, `armedGateResult` invariant (Tasks 1–2); `mapDriveFailure`, `ReasonUnknownRunEpoch` (Task 3); `NewTaskGateDriveService`, `GateDriveService.startRequest`, `svc.engine` (`driveEngine`: `Admit`, `AbandonAdmission`); `gatedrive.OpenStore(common).PrepareScope` / `.LoadWorktreeExecution`; test helpers `newWorkingRepo`, `workspaceDepsFor`, `inProgressChangeBlob`, `resumeInspectService`, `mainPin`, `buildEffWithMaxAttempts`, `gateGitCommonDir`. +- Produces: none (a test only). + +- [ ] **Step 1: Write the test** + +```go +package app + +import ( + "context" + "path/filepath" + "strings" + "testing" + + "github.com/danielhanold/docket/internal/gatedrive" + "github.com/danielhanold/docket/internal/testsupport" +) + +// TestEpochlessResumeEndToEnd0382 reproduces change 0382's resumed run (change 0463). +// The change was claimed by an UNARMED first dispatch, so no run epoch exists. The +// resume arm must print `gate-armed `. Parsed +// positionally (as AGENTS.md tells a parent), the is admitted by the real +// epoch launch gate for the resumed worktree and recorded on its execution slot. +// The misrouted 0382 call (the dispatch context presented as the epoch) is refused +// with the named unknown-run-epoch. The resume inspect path uses the raw temp +// spelling and the start uses the symlink-resolved one (Review Focus 1). +func TestEpochlessResumeEndToEnd0382(t *testing.T) { + repoDir := newWorkingRepo(t, nil).invocation + worktree, err := filepath.EvalSymlinks(repoDir) + if err != nil { + t.Fatalf("EvalSymlinks: %v", err) + } + common, err := gateGitCommonDir(repoDir) + if err != nil { + t.Fatalf("gateGitCommonDir: %v", err) + } + if _, _, found, ferr := FindEpochByChange(repoDir, "5"); ferr != nil || found { + t.Fatalf("the unarmed claim must leave no epoch: found=%v err=%v", found, ferr) + } + + // Arm the resume through the REAL outer-scope store, as production composes it. + reader := &fakeReader{pin: mainPin(t), corpus: []StatusBlob{inProgressChangeBlob(5, "epsilon", "v5", "")}} + deps := workspaceDepsFor(t, reader) + wdeps := WorkspaceDeps{Service: resumeInspectService(repoDir)} + store := gatedrive.OpenStore(common) + arm := RunGateBefore(context.Background(), deps, wdeps, GateScopeDeps{Prepare: store.PrepareScope}, repoDir, "implement-next", 5) + if !arm.Armed { + t.Fatalf("the epochless resume must arm: %q", arm.HumanText()) + } + + fields := strings.Fields(strings.SplitN(arm.HumanText(), "\n", 2)[0]) + if len(fields) != 4 || fields[0] != "gate-armed" { + t.Fatalf("armed line %q must be `gate-armed `", arm.HumanText()) + } + key, epoch, dispatchCtx := fields[1], fields[2], fields[3] + if key != arm.Key || epoch != arm.Epoch || dispatchCtx != arm.DispatchContext { + t.Fatalf("positional fields (%q,%q,%q) disagree with the result (%q,%q,%q)", key, epoch, dispatchCtx, arm.Key, arm.Epoch, arm.DispatchContext) + } + + svc, res, reason := NewTaskGateDriveService(common, "/bin/true", buildEffWithMaxAttempts("go test ./...", 4), []string{"/bin/echo", "ok"}) + if svc == nil { + t.Fatalf("task service: %s (%s)", res, reason) + } + req := GateDriveStartRequest{ + RepoDir: common, Worktree: worktree, ChangeID: "5", TaskID: "task-6", Phase: "build", + RunRoot: testsupport.TempDir(t), Cwd: worktree, GateContext: dispatchCtx, RunEpochID: epoch, + } + + // The misrouted 0382 call: the dispatch context presented as the run epoch. + bad := req + bad.RunEpochID = dispatchCtx + if _, berr := svc.engine.Admit(svc.startRequest(bad)); berr == nil { + t.Fatalf("the dispatch context must never admit as a run epoch") + } else if r, why := mapDriveFailure(berr); r != ResultInvalidInput || why != ReasonUnknownRunEpoch { + t.Fatalf("misrouted epoch refused as (%s, %q), want (invalid-input, unknown-run-epoch)", r, why) + } + + // The correctly parsed epoch is admitted by the real launch gate. + ticket, aerr := svc.engine.Admit(svc.startRequest(req)) + if aerr != nil { + r, why := mapDriveFailure(aerr) + t.Fatalf("the parsed epoch must admit for the resumed worktree, got (%s, %q): %v", r, why, aerr) + } + t.Cleanup(func() { _ = svc.engine.AbandonAdmission(ticket) }) + slot, _, lerr := store.LoadWorktreeExecution(worktree) + if lerr != nil { + t.Fatalf("LoadWorktreeExecution: %v", lerr) + } + if slot.RunEpochID != epoch { + t.Fatalf("worktree slot RunEpochID = %q, want the armed epoch %q", slot.RunEpochID, epoch) + } +} +``` + +- [ ] **Step 2: Run it** + +Run: `go test -count=1 ./internal/app/ -run TestEpochlessResumeEndToEnd0382 -v` +Expected: PASS on top of Tasks 1–3. If `Admit` refuses for a fixture reason unrelated to the epoch (for example, the fingerprint needs a clean checkout), fix the fixture, never the assertion. The worktree must be a git checkout with a HEAD, which `newWorkingRepo(...).invocation` provides. + +- [ ] **Step 3: Mutation-check** (backup-copy restore) + +- Reintroduce the original defect (restore `if resumeID == 0` around the Task 1 mint, remove the `armedGateResult` guard, and restore the `HumanText` epoch conditional): the test reddens on the 4-field assertion. +- Drop the resume `rec.Worktree = worktree` bind: the good `Admit` reddens (`stale-run-epoch`). +- Delete the `ClassifyRunEpochError` block from `mapDriveFailure`: the misrouted leg reddens (`invalid-request`). + +- [ ] **Step 4: Commit** + +```bash +git add internal/app/rungate_epochless_resume_e2e_test.go +git commit -m "test(rungate): end-to-end reproduction of the 0382 epochless resume (change 0463)" +``` + +--- + +## Self-Review + +- **Spec coverage.** Decision 1 / Design §1: Task 1. Design §2 (HumanText, stale comments, CLI `Short`, prose grep): Tasks 1–2. Design §3 (`mapDriveFailure` `AsEpochError` case, the `unknown-run-epoch` token, next-action message, prepare-scope, agent.enter trace, token registration): Tasks 3–5. The token-registration grep over non-archival prose and skills found no enumerated gate-drive reason vocabulary outside Go (skills mention only `suite-attempts-exhausted`), so no prose registry edit is needed. Tests 1, 3, 7: Task 1. Test 2: Task 2. Test 4: Tasks 3–4. Test 5: Task 5. Test 6: Task 6. Decision 4 (race fail-safe): the Task 1 documenting test. Out-of-scope items are untouched. +- **Deliberate deviations, recorded.** + - The resume epoch is minted unbound, and ChangeID+Worktree are bound in one `epochCAS`, rather than minted with ChangeID. The end state matches the spec, and a failed bind can no longer leave an orphan that names the change and blocks later resumes. + - `agent.enter` maps a mismatch to the existing stale-linkage token `stale-run-epoch`, and maps an absent gate key to `unknown-run-epoch`, since the pair names no epoch. + - prepare-scope checks resolvability only, keeping the existing cancelled-epoch prepare green. +- **Type consistency.** `armedGateResult(key, epochID, dispatchContext string)`, `ClassifyRunEpochError(err) (Result, string, bool)`, `RunEpochNextAction(reason string) string`, `ReasonUnknownRunEpoch`, `runEpochLocator(gitCommonDir string) func(string) error`, `GateDriveService.epochLocate`, `GateScopeResult.Message`, and `CheckRunEpochLinkage(repoDir, gateKey, epochID string) error` are used identically across Tasks 1–6. diff --git a/internal/app/change_claim.go b/internal/app/change_claim.go index 850e71e66..dd7ca7488 100644 --- a/internal/app/change_claim.go +++ b/internal/app/change_claim.go @@ -204,7 +204,7 @@ func ChangeClaim(ctx context.Context, deps PlanningDeps, repoDir string, req Cha var gateKey, gateHash string if req.GateContext != "" { gateHash = gateHashToken(req.GateContext) - key, _, ferr := FindGateRecordByContextHash(repoDir, gateHash) + key, gateRec, ferr := FindGateRecordByContextHash(repoDir, gateHash) if ferr != nil { return newChangeClaimResult(OperationChangeClaim, ResultInvalidState, ChangeClaimResult{ Disposition: ClaimDispositionGateContextInvalid, @@ -212,6 +212,17 @@ func ChangeClaim(ctx context.Context, deps PlanningDeps, repoDir string, req Cha "supplied gate context matches no live armed gate in this repository; refusing — an invalid context is never an ungated claim: "+ferr.Error())}, }) } + // A resume arm's context is already bound to the resumed change and never + // claims (change 0463). Refuse BEFORE reserving: a leftover unconfirmed + // reservation, or a confirmed claim of another change, would make run.cancel + // refuse the resume epoch forever. + if gateRec.resumeAttributed() { + return newChangeClaimResult(OperationChangeClaim, ResultInvalidState, ChangeClaimResult{ + Disposition: ClaimDispositionGateContextConflict, + Findings: []StatusFinding{lifecycleFinding(FindingCode(ClaimDispositionGateContextConflict), + fmt.Sprintf("this dispatch context was armed to resume change %04d and cannot claim a change; a resumed run continues its existing claim", gateRec.AttributedID))}, + }) + } gateKey = key if rerr := ReserveGateClaim(repoDir, gateKey, req.ID, claimRequestID(req)); rerr != nil { return newChangeClaimResult(OperationChangeClaim, ResultInvalidState, ChangeClaimResult{ diff --git a/internal/app/change_claim_integration_test.go b/internal/app/change_claim_integration_test.go index 99c457533..77a04e3de 100644 --- a/internal/app/change_claim_integration_test.go +++ b/internal/app/change_claim_integration_test.go @@ -8,6 +8,7 @@ package app import ( "context" + "fmt" "github.com/danielhanold/docket/internal/repository/transaction" "path" "strings" @@ -373,3 +374,66 @@ func TestIntegrationRecordOpsChangeRefreshClaimUnrelatedInvalidRecordRefusals(t }) } } + +// TestIntegrationRecordOpsClaimResumeContextRefusedBeforeReserve: a `gate-before --resume` arm pre-binds +// the resumed change as AttributedID and never gets a claim binding (change 0463). +// A claim under that context, for the resumed change itself or for any other change, +// is refused gate-context-conflict BEFORE ReserveGateClaim writes a binding file: a +// stray unconfirmed reservation would make run.cancel refuse claim-unconfirmed, and a +// confirmed claim of a different change would make it refuse claim-mismatch, leaving +// the resume epoch uncancellable either way. +func TestIntegrationRecordOpsClaimResumeContextRefusedBeforeReserve(t *testing.T) { + for _, id := range []int{3, 4} { + t.Run(fmt.Sprintf("claim-%d", id), func(t *testing.T) { + repoDir := newGateRepo(t) + key, err := MintGateRecord(repoDir, GateRecord{ + Target: "docket-implement-next", Retry: RetryUnused, AttemptLimit: 2, + ChildContextHash: gateHashToken("tok"), AttributedID: 3, + }) + if err != nil { + t.Fatalf("MintGateRecord: %v", err) + } + corpus := []StatusBlob{ + changeBlob(3, "widget", "feat", "high", ""), + changeBlob(4, "gadget", "feat", "high", ""), + } + engine := &claimGateEngine{result: appliedGateResult(t, id)} + + res := ChangeClaim(context.Background(), gateClaimDeps(t, engine, corpus), repoDir, + ChangeClaimRequest{ID: id, Version: gateClaimVersion, GateContext: "tok"}) + + if res.Result != ResultInvalidState || res.Disposition != ClaimDispositionGateContextConflict { + t.Fatalf("result = %q disposition = %q, want invalid-state %q (findings %v)", + res.Result, res.Disposition, ClaimDispositionGateContextConflict, res.Findings) + } + if len(engine.calls) != 0 { + t.Errorf("engine called %d times under a resume context, want 0", len(engine.calls)) + } + if _, ok, berr := LoadGateClaimBinding(repoDir, key); berr != nil || ok { + t.Errorf("claim binding present=%v err=%v after refusal; want none written", ok, berr) + } + }) + } +} + +// TestIntegrationRecordOpsClaimGateContextRetryAfterConfirmAdmitted: the resume-context refusal keys on +// the resume-verified shape only. A fresh arm's record gains AttributedID at confirm +// time together with BoundRequestID, so an idempotent retry of the same confirmed +// claim must still reach the engine rather than being refused as a resume context. +func TestIntegrationRecordOpsClaimGateContextRetryAfterConfirmAdmitted(t *testing.T) { + repoDir := newGateRepo(t) + mintGateWithHash(t, repoDir, gateHashToken("tok"), false) + corpus := []StatusBlob{changeBlob(3, "widget", "feat", "high", "")} + + for i := 0; i < 2; i++ { + engine := &claimGateEngine{result: appliedGateResult(t, 3)} + res := ChangeClaim(context.Background(), gateClaimDeps(t, engine, corpus), repoDir, + ChangeClaimRequest{ID: 3, Version: gateClaimVersion, GateContext: "tok"}) + if res.Result != ResultApplied { + t.Fatalf("attempt %d result = %q disposition = %q, want applied (%v)", i+1, res.Result, res.Disposition, res.Findings) + } + if len(engine.calls) != 1 { + t.Fatalf("attempt %d engine calls = %d, want 1", i+1, len(engine.calls)) + } + } +} diff --git a/internal/app/gate_drive.go b/internal/app/gate_drive.go index 12fea38d8..f846b7f68 100644 --- a/internal/app/gate_drive.go +++ b/internal/app/gate_drive.go @@ -68,6 +68,10 @@ type GateScopeResult struct { ChildCapability string `json:"child_capability,omitempty"` ParentCapability string `json:"parent_capability,omitempty"` Reason string `json:"reason,omitempty"` + // Message is the one-line next action on a command failure (change 0463), e.g. + // the unknown-run-epoch remedy. It never carries a capability or the presented + // run-epoch value. + Message string `json:"message,omitempty"` } // HumanText renders GateScopeResult naming ONLY the scope id (and a bounded @@ -81,6 +85,9 @@ func (r GateScopeResult) HumanText() string { if r.Reason != "" { lines = append(lines, "reason: "+r.Reason) } + if r.Message != "" { + lines = append(lines, "message: "+r.Message) + } return strings.Join(lines, "\n") } @@ -142,6 +149,11 @@ type GateDriveService struct { // finalize service also stores a non-nil budgetStore. budgetStore *gatedrive.Store maxAttempts int + // epochLocate resolves a presented run-epoch id against the repository's run-epoch + // registry before PrepareScope mints a scope (change 0463). A non-nil error is a + // typed EpochError. Nil on the fake-engine test seam and on services that never + // serve prepare-scope. + epochLocate func(epochID string) error } // GateDriveStartRequest is the caller-supplied identity and launch context for a @@ -298,7 +310,11 @@ func NewCommandlessGateDriveService(gitCommonDir, exePath string) (*GateDriveSer // settled through exact-token retirement rather than refused stale-run-epoch // (change 0446): wire the settlement read over the same registry. engine.SetEpochSettledResolver(epochSettledResolver(gitCommonDir)) - return newGateDriveService(engine, 0, "", ""), "", "" + svc := newGateDriveService(engine, 0, "", "") + // prepare-scope is served by this commandless service: resolve a presented + // --run-epoch against the same registry before minting a scope (change 0463). + svc.epochLocate = runEpochLocator(gitCommonDir) + return svc, "", "" } // NewTaskGateDriveService composes the gate-drive seam for TASK-INTENT @@ -597,6 +613,19 @@ func (s *GateDriveService) Claim(id, handoffID string) GateDriveResult { // safe reason and no grant. The two capabilities travel ONLY in the JSON // document — never in the human text (GateScopeResult.HumanText). func (s *GateDriveService) PrepareScope(req gatedrive.ScopeRequest) GateScopeResult { + // A presented --run-epoch must resolve before it is baked into the scope (change + // 0463): an unresolvable one refuses now with its named token, instead of + // surfacing later at start as a refusal the caller cannot attribute. + if req.RunEpochID != "" && s.epochLocate != nil { + if lerr := s.epochLocate(req.RunEpochID); lerr != nil { + res, reason := mapDriveFailure(lerr) + return GateScopeResult{ + Envelope: NewEnvelope(OperationGateDrivePrepareScope, res), + Reason: reason, + Message: RunEpochNextAction(reason), + } + } + } grant, err := s.engine.PrepareScope(req) if err != nil { res, reason := mapDriveFailure(err) @@ -674,6 +703,8 @@ func mapDriveResult(op string, doc gatedrive.DriveDoc, err error) GateDriveResul } } else if fe, ok := AsMutationFenceError(err); ok { result.Message = fenceNextAction(fe.Reason) + } else if _, reason, ok := ClassifyRunEpochError(err); ok { + result.Message = RunEpochNextAction(reason) } return result } @@ -705,6 +736,13 @@ func mapDriveFailure(err error) (Result, string) { if fe, ok := AsMutationFenceError(err); ok { return ResultInvalidInput, fe.Reason } + // A run-epoch registry failure (the epoch launch gate could not resolve the + // presented --run-epoch) surfaces its named token rather than collapsing to the + // generic invalid-request (change 0463): unknown-run-epoch for a not-found epoch, + // the kind for any other registry fault. + if res, reason, ok := ClassifyRunEpochError(err); ok { + return res, reason + } if se, ok := gatedrive.AsStoreError(err); ok { switch se.Kind { case gatedrive.ErrInvalidID, gatedrive.ErrNotFound: diff --git a/internal/app/gate_drive_test.go b/internal/app/gate_drive_test.go index a2a26fcc5..276fd994a 100644 --- a/internal/app/gate_drive_test.go +++ b/internal/app/gate_drive_test.go @@ -1648,3 +1648,68 @@ func TestQuoteOperand(t *testing.T) { t.Fatalf("quoteOperand = %q", got) } } + +// TestMapDriveFailureEpochErrors (change 0463): an EpochError chained through the +// gate-drive seam (the epoch launch gate refusing an unknown --run-epoch) surfaces +// its named token, never the catch-all invalid-request. The service attaches the +// next-action message, and neither the reason nor the message echoes the value. +func TestMapDriveFailureEpochErrors(t *testing.T) { + const presented = "0790b760e26444866ef2e156ba383326" + wrapped := fmt.Errorf("refused %s: %w", presented, epochErr(ErrEpochNotFound, "find-dir-by-id", nil)) + res, reason := mapDriveFailure(wrapped) + if res != ResultInvalidInput || reason != ReasonUnknownRunEpoch { + t.Fatalf("mapDriveFailure = (%s, %q), want (invalid-input, unknown-run-epoch)", res, reason) + } + if res, reason := mapDriveFailure(epochErr(ErrEpochIO, "find-by-id", nil)); res != ResultInternalError || reason != "epoch-io" { + t.Fatalf("an unreadable registry must be an internal error, got (%s, %q)", res, reason) + } + eng := &fakeDriveEngine{err: wrapped} + got := newGateDriveService(eng, 0, "", "").Advance("d1", "owner") + if got.Reason != ReasonUnknownRunEpoch { + t.Fatalf("service reason = %q, want unknown-run-epoch", got.Reason) + } + if !strings.Contains(got.Message, "--gate-context") { + t.Fatalf("service must attach the unknown-run-epoch next action, got %q", got.Message) + } + if strings.Contains(got.Message, presented) || strings.Contains(got.HumanText(), presented) { + t.Fatalf("the presented value leaked: message=%q human=%q", got.Message, got.HumanText()) + } +} + +// TestPrepareScopeRefusesUnknownRunEpoch (change 0463): a presented --run-epoch that +// the registry cannot resolve is refused before any scope is minted, with the named +// token and the next action and without echoing the value. A scope with no +// --run-epoch never consults the locator (standalone scopes are unchanged). +func TestPrepareScopeRefusesUnknownRunEpoch(t *testing.T) { + eng := &fakeDriveEngine{grant: gatedrive.ScopeGrant{ScopeID: "scope-1", ChildCapability: "c", ParentCapability: "p"}} + svc := newGateDriveService(eng, 0, "", "") + var asked string + svc.epochLocate = func(id string) error { + asked = id + return epochErr(ErrEpochNotFound, "find-dir-by-id", nil) + } + got := svc.PrepareScope(gatedrive.ScopeRequest{ChangeID: "463", RunEpochID: "bogus-epoch-value"}) + if got.Result != ResultInvalidInput || got.Reason != ReasonUnknownRunEpoch { + t.Fatalf("got (%s, %q), want (invalid-input, unknown-run-epoch)", got.Result, got.Reason) + } + if got.ScopeID != "" || got.ChildCapability != "" || got.ParentCapability != "" { + t.Fatalf("a refused prepare-scope must carry no grant: %+v", got) + } + if eng.lastScopeReq.ChangeID != "" { + t.Fatalf("an unresolvable epoch must mint no scope, engine saw %+v", eng.lastScopeReq) + } + if asked != "bogus-epoch-value" { + t.Fatalf("locator asked %q, want the presented id", asked) + } + if !strings.Contains(got.Message, "--gate-context") || !strings.Contains(got.HumanText(), "unknown-run-epoch") { + t.Fatalf("refusal must carry reason and next action: message=%q human=%q", got.Message, got.HumanText()) + } + if strings.Contains(got.Message, "bogus-epoch-value") || strings.Contains(got.HumanText(), "bogus-epoch-value") { + t.Fatalf("the presented value leaked: %q / %q", got.Message, got.HumanText()) + } + + asked = "" + if ok := svc.PrepareScope(gatedrive.ScopeRequest{ChangeID: "463"}); ok.Result != ResultApplied || asked != "" { + t.Fatalf("a scope without --run-epoch must skip the locator and apply: result=%s asked=%q", ok.Result, asked) + } +} diff --git a/internal/app/rungate_before.go b/internal/app/rungate_before.go index a3c2468eb..6d457c2f4 100644 --- a/internal/app/rungate_before.go +++ b/internal/app/rungate_before.go @@ -6,6 +6,8 @@ import ( "encoding/hex" "errors" "fmt" + "os" + "path/filepath" "sort" "strconv" "strings" @@ -21,9 +23,10 @@ import ( // origin, reads the current in-progress claim set, captures a dispatch epoch // AFTER that read, and mints a durable gate record under the git common dir // (rungate_store.go). Its whole contract is the printed report line: -// `gate-armed ` on success, `gate-unarmed ` on any failure — -// both exit 0 (learning exit-code-encodes-a-non-failure). Only `implement-next` -// is an accepted target; anything else is a usage error that exits non-zero. +// `gate-armed ` on success, `gate-unarmed +// ` on any failure — both exit 0 (learning +// exit-code-encodes-a-non-failure). Only `implement-next` is an accepted +// target; anything else is a usage error that exits non-zero. // // It writes NO metadata: the fresh-origin re-sync and the change-file parse are // the SAME plumbing the claim path uses (PinContext advances the remote-tracking @@ -155,8 +158,9 @@ type RunGateBeforeResult struct { // the operator threads into `run.cancel --epoch ` — the primary human Stop — // and the dispatcher threads into each `--run-epoch` flag (agent.enter, gate drive // start, gate drive prepare-scope). Without it the documented Stop path names an - // epoch the arm never surfaced (change 0375). Empty only on a legacy resume arm - // that shares no epoch; the resume-active locator already prints the epoch there. + // epoch the arm never surfaced (change 0375). Never empty on an armed result + // (change 0463): armedGateResult refuses to arm without one, so the positional + // `gate-armed ` line always has three tokens. Epoch string `json:"epoch,omitempty"` Target string `json:"target,omitempty"` Reason string `json:"reason,omitempty"` @@ -170,19 +174,17 @@ type RunGateBeforeResult struct { } // HumanText renders the one report line. An armed gate prints `gate-armed -// ` (the epoch is omitted only on a legacy resume arm -// that shares no epoch); a gate-unarmed report prints `gate-unarmed -// `; a usage error (a non-applied result) names its reason instead -// of a report line. The parent capability never appears here — only the child -// dispatch context, which is meant for the child. +// `. That is always three tokens, because every armed +// result carries an epoch (armedGateResult, change 0463), so a positional +// parser can never read the dispatch context as the epoch. A gate-unarmed +// report prints `gate-unarmed `; a usage error (a non-applied +// result) names its reason instead of a report line. The parent capability +// never appears here — only the child dispatch context, which is meant for the +// child. func (r RunGateBeforeResult) HumanText() string { if r.Result == ResultApplied { if r.Armed { - line := "gate-armed " + r.Key - if r.Epoch != "" { - line += " " + r.Epoch - } - line += " " + r.DispatchContext + line := "gate-armed " + r.Key + " " + r.Epoch + " " + r.DispatchContext if r.OwnerLifecycle != "" { // Honest standing caveat: the dispatched route cancels no run on owner // death; a Stop is the explicit `run.cancel` operation. @@ -230,15 +232,106 @@ func gateResumeObserve(reservedKey string) RunGateBeforeResult { }) } +// armedGateResult builds the armed report for key. Every armed gate carries a run +// epoch (change 0463): parents read the `gate-armed ` +// line positionally, and both tokens are 32-hex, so the line is unambiguous only +// when the epoch slot is always filled. An empty epoch therefore fails closed as +// gate-unarmed mint-failed. It never prints a two-token line whose dispatch context +// a parent would read as the epoch. +func armedGateResult(key, epochID, dispatchContext string) RunGateBeforeResult { + if epochID == "" { + return gateUnarmed(ReasonGateMintFailed) + } + return newRunGateBeforeResult(ResultApplied, RunGateBeforeResult{ + Armed: true, + Key: key, + Epoch: epochID, + Target: gateBeforeStoredTarget, + DispatchContext: dispatchContext, + OwnerLifecycle: ReasonOwnerLifecycleUnavailable, + }) +} + // resumeActiveLocator renders the safe locator and explicit cancel/continue remedy // a resume prints when the prior run is still active (change 0375 Task 12). It names // only public locators — the change id, the public epoch id, and the gate key — never // a capability or reservation token. func resumeActiveLocator(gateKey string, ep EpochRecord) string { return "change " + ep.ChangeID + " has an active run (epoch " + ep.EpochID + - ", gate key " + gateKey + "); cancel it with 'docket run cancel --key " + gateKey + - " --epoch " + ep.EpochID + " --reason ' and resume after confirmed cancellation, " + - "or continue the live run via 'docket run gate-verdict'" + ", gate key " + gateKey + "); " + resumeIncumbentRemedy(gateKey, ep.EpochID) +} + +// resumeIncumbentRemedy renders the two remedies for a resume refused over a live +// incumbent epoch. An epochless resume arm binds its epoch when armed (change 0463), +// so the incumbent may be an arm that was never dispatched. Nothing records whether an +// agent is using the epoch, so the remedy names both cases rather than guessing. +func resumeIncumbentRemedy(gateKey, epochID string) string { + return "if it was never dispatched or its agent has exited, cancel it with 'docket run cancel --key " + + gateKey + " --epoch " + epochID + " --reason ' and resume after confirmed cancellation; " + + "if its agent is still running, continue the live run via 'docket run gate-verdict'" +} + +// acquireResumeLock takes the exclusive per-change resume lock that serializes +// `gate-before --resume` arms of one change (change 0463). It lives outside the +// rungate root, under /docket/rungate-resume/, so the +// scanners that walk gate-key directories never see it. Closing the returned file +// releases the lock. +func acquireResumeLock(repoDir, changeID string) (*os.File, error) { + common, err := gateGitCommonDir(repoDir) + if err != nil { + return nil, err + } + dir := filepath.Join(common, "docket", "rungate-resume", changeID) + if err := os.MkdirAll(dir, 0o700); err != nil { + return nil, epochErr(ErrEpochIO, "resume-lock-dir", err) + } + return acquireEpochLock(dir) +} + +// resumeWorktreeOwnerRefusal checks whether a live run epoch already owns the +// verified worktree an epochless resume is about to bind (change 0463 decision 4). +// It resolves the owner the same way the mutation fence does (findEpochByWorktree +// over the canonical path; an uncanonicalizable path is matched by its own +// spelling). An active, completing, or unrecognized owner refuses resume-active-run +// with that owner's locator. A fenced owner (cancelling, cancelled, superseded) is +// no live owner: a fresh active epoch outranks it, so it does not block. An +// ambiguous or unreadable owner set refuses resume-epoch-unreadable, fail-closed. +// It returns refused=false when the resume may mint. +func resumeWorktreeOwnerRefusal(repoDir, worktree string) (RunGateBeforeResult, bool) { + canon := worktree + if c, err := canonicalWorktree(worktree); err == nil { + canon = c + } + ownerKey, found, err := findEpochByWorktree(repoDir, canon) + if err != nil { + return gateUnarmedMsg(ReasonGateResumeEpochUnreadable, + "the run epoch owning worktree "+worktree+" could not be resolved: "+err.Error()), true + } + if !found { + return RunGateBeforeResult{}, false + } + owner, _, lerr := LoadEpochRecord(repoDir, ownerKey) + if lerr != nil { + return gateUnarmedMsg(ReasonGateResumeEpochUnreadable, + "the run epoch owning worktree "+worktree+" (gate key "+ownerKey+") could not be read"), true + } + switch owner.State { + case EpochCancelling, EpochCancelled, EpochSuperseded: + return RunGateBeforeResult{}, false + } + return gateUnarmedMsg(ReasonGateResumeActiveRun, resumeWorktreeOwnerLocator(worktree, ownerKey, owner)), true +} + +// resumeWorktreeOwnerLocator renders the safe locator and the cancel/continue remedy +// for a live epoch that owns the resume's worktree without naming the resumed +// change. Like resumeActiveLocator it names only public locators. +func resumeWorktreeOwnerLocator(worktree, gateKey string, ep EpochRecord) string { + owner := "a live run with no bound change" + if ep.ChangeID != "" { + owner = "a live run of change " + ep.ChangeID + } + return "worktree " + worktree + " is already owned by " + owner + " (state " + string(ep.State) + + ", epoch " + ep.EpochID + ", gate key " + gateKey + "); " + resumeIncumbentRemedy(gateKey, ep.EpochID) } // resumeReplacementParams carries the immutable arm facts armResumeReplacement mints @@ -317,14 +410,7 @@ func armResumeReplacement(repoDir string, sdeps GateScopeDeps, oldKey string, p // The replacement dispatch gets a fresh live epoch; surface its public id so the // resumed run's Stop path (`run.cancel --epoch`) and `--run-epoch` flags are // followable, exactly as a fresh arm's are (change 0375). - return newRunGateBeforeResult(ResultApplied, RunGateBeforeResult{ - Armed: true, - Key: key, - Epoch: epochRec.EpochID, - Target: gateBeforeStoredTarget, - DispatchContext: grant.ChildCapability, - OwnerLifecycle: ReasonOwnerLifecycleUnavailable, - }) + return armedGateResult(key, epochRec.EpochID, grant.ChildCapability) } // validateResumeQuiescence re-proves the OLD epoch's quiescence before resume may @@ -357,8 +443,9 @@ func validateResumeQuiescence(seams cancelSeams, repoDir string, ep EpochRecord, // usage error (non-zero exit); otherwise it re-syncs, reads the in-progress // claim set, captures the dispatch epoch after that read, optionally verifies an // explicit resume id, prepares the OUTER recovery scope (change 0359), mints the -// durable record, and returns `gate-armed ` — degrading -// any arming failure to a `gate-unarmed ` report line that still exits 0. +// durable record and its run epoch, and returns `gate-armed +// ` — degrading any arming failure to a `gate-unarmed ` +// report line that still exits 0. // // resumeID (0 = none) requests explicit resume attribution: the id is pre-bound // as the record's AttributedID ONLY when it is a verified in-progress change with @@ -445,12 +532,25 @@ func RunGateBefore(ctx context.Context, deps PlanningDeps, wdeps WorkspaceDeps, branch = insp.FeatureRef worktree = insp.Path + // Serialize every resume arm of this change from here to the end of the arm + // (change 0463). The checks below decide from the epochs that exist now, and + // the arm then mints and binds one, so two arms must never interleave between + // check and bind. The lock is keyed by change id; a resume's worktree is the + // change's own feature worktree, so it also covers step 4b's worktree check. + lock, lerr := acquireResumeLock(repoDir, scopeChangeID) + if lerr != nil { + return gateUnarmedMsg(ReasonGateResumeEpochUnreadable, + "the resume lock for change "+scopeChangeID+" could not be taken: "+lerr.Error()) + } + defer lock.Close() + // (4a) Resume SHARES the run epoch's admission (change 0375 Task 12, spec // "run.gate-before --resume and direct implement-next resume must share the same // admission path"). Locate the change's prior epoch; its state decides whether a - // replacement may be admitted. No prior epoch (a legacy/pre-epoch resume, or an - // unclaimed run that never bound one) falls through to the existing resume arm, - // which shares no epoch and reserves no replacement. + // replacement may be admitted. No prior epoch (a legacy/pre-epoch resume, or a + // first dispatch that was never armed) falls through to the ordinary arm below, + // which mints and binds a fresh epoch for the resumed change (step 6a, change + // 0463), so the armed line always carries one. oldKey, oldEp, foundEp, ferr := FindEpochByChange(repoDir, scopeChangeID) if ferr != nil { return gateUnarmedMsg(ReasonGateResumeEpochUnreadable, @@ -542,6 +642,16 @@ func RunGateBefore(ctx context.Context, deps PlanningDeps, wdeps WorkspaceDeps, "change "+scopeChangeID+" has an unrecognized run epoch state") } } + + // (4b) No epoch names the change, so step 6a will mint one and bind it to the + // verified worktree. FindEpochByChange cannot see a live epoch that owns this + // worktree under no change or another change. Minting over one would leave two + // live owners of one worktree, and findEpochByWorktree would then refuse every + // fenced mutation there (PR publish, workspace publish) as + // ErrEpochOwnerAmbiguous. Refuse with the incumbent's locator instead. + if refusal, refused := resumeWorktreeOwnerRefusal(repoDir, worktree); refused { + return refusal + } } // (5) Prepare the OUTER recovery scope. The grant's ChildCapability becomes the @@ -584,37 +694,51 @@ func RunGateBefore(ctx context.Context, deps PlanningDeps, wdeps WorkspaceDeps, return gateUnarmed(ReasonGateMintFailed) } - // (6a) A fresh (non-resume) arm binds a NEW run epoch beside the just-minted gate - // record, keyed by the gate key (rungate_epoch.go). The epoch is the durable - // coordinator fence a later human cancellation flips and a resume supersedes; its - // EpochID travels onto each scoped start's worktree slot so an omitted or stale - // epoch cannot detach the worktree. A mint failure unarms fail-closed: an armed - // gate must carry a live epoch (the orphan gate record left behind is inert — no - // key is returned, so nothing dispatches against it). A resume arm does NOT mint - // here: it shares the change's existing epoch, whose supersede-and-reserve is - // Task 12's; for change 0375 Task 9 only the fresh arm binds an epoch. - var epochID string - if resumeID == 0 { - epochRec, eerr := MintEpochRecord(repoDir, key, scopeChangeID) - if eerr != nil { + // (6a) Every armed gate binds a run epoch beside the just-minted gate record, + // keyed by the gate key (rungate_epoch.go). The epoch is the durable coordinator + // fence that a later human cancellation flips and a resume supersedes. Its EpochID + // travels onto each scoped start's worktree slot, so an omitted or stale epoch + // cannot detach the worktree. Two arms reach this step: a FRESH arm, and a RESUME + // whose change has no prior epoch (a legacy/pre-epoch run, or a first dispatch + // that was never armed; change 0463). A resume that found a prior epoch never gets + // here, because every found state has already returned above (a refusal, an + // observed reservation, or armResumeReplacement, which mints its own). + // + // The epoch is minted unbound. A fresh arm binds ChangeID and Worktree later, at + // claim confirmation (bindEpochChange / bindEpochWorktree). A resume has already + // claimed, so it binds both NOW, in one epochCAS, the same way + // armResumeReplacement binds its worktree. Why both are needed: + // - epochLaunchGate refuses an active epoch that has no Worktree, so an unbound + // resume epoch would be refused on first use. + // - With ChangeID bound, a later resume of the same change finds this epoch + // active and refuses resume-active-run. + // Binding both in one CAS means a failed bind leaves an UNBOUND orphan (inert, + // like a fresh arm's), never an orphan that names the change. A mint or bind + // failure unarms fail-closed; the orphan gate record left behind is inert, because + // no key is returned and nothing dispatches against it. + // + // A resume reaches this mint and bind still holding the per-change resume lock it + // took at step 4 (acquireResumeLock). Epoch locks are per gate key, so without it + // concurrent epochless resumes of one change would each pass step 4's checks and + // each mint and bind a live epoch here. + epochRec, eerr := MintEpochRecord(repoDir, key, "") + if eerr != nil { + return gateUnarmed(ReasonGateMintFailed) + } + if resumeID != 0 { + if werr := epochCAS(repoDir, key, func(rec *EpochRecord) error { + rec.ChangeID = scopeChangeID + rec.Worktree = worktree + return nil + }); werr != nil { return gateUnarmed(ReasonGateMintFailed) } - // Surface the just-minted public epoch id so the documented Stop path is - // followable: `run.cancel --epoch ` and every `--run-epoch` dispatch flag - // consume exactly this value (change 0375). - epochID = epochRec.EpochID } // (7) Report the armed gate with its dispatch context, its run epoch id, and the // honest owner-lifecycle caveat: the dispatched route has no automatic Stop, so a // Stop is the explicit `run.cancel` operation keyed by this epoch (change 0375 - // Task 13). - return newRunGateBeforeResult(ResultApplied, RunGateBeforeResult{ - Armed: true, - Key: key, - Epoch: epochID, - Target: gateBeforeStoredTarget, - DispatchContext: grant.ChildCapability, - OwnerLifecycle: ReasonOwnerLifecycleUnavailable, - }) + // Task 13). armedGateResult refuses an empty epoch, so the line is always three + // tokens (change 0463). + return armedGateResult(key, epochRec.EpochID, grant.ChildCapability) } diff --git a/internal/app/rungate_before_integration_test.go b/internal/app/rungate_before_integration_test.go index c38091daf..da360d45a 100644 --- a/internal/app/rungate_before_integration_test.go +++ b/internal/app/rungate_before_integration_test.go @@ -498,3 +498,48 @@ func writeRawGateRecord(t *testing.T, root, key, tmpl string) { t.Fatalf("write raw record: %v", err) } } + +// TestIntegrationGateArmArmedLineIsAlwaysThreeTokens (change 0463): every armed result a real arm +// produces (fresh, epochless resume, cancelled-replacement resume) prints a first +// line of exactly four space-separated fields. Field 3 is the epoch and field 4 is +// the dispatch context, so a positional parser can never read the dispatch context +// as the epoch. +func TestIntegrationGateArmArmedLineIsAlwaysThreeTokens(t *testing.T) { + check := func(t *testing.T, res RunGateBeforeResult) { + t.Helper() + if !res.Armed { + t.Fatalf("did not arm: %q", res.HumanText()) + } + first := strings.SplitN(res.HumanText(), "\n", 2)[0] + fields := strings.Fields(first) + if len(fields) != 4 || fields[0] != "gate-armed" { + t.Fatalf("armed line %q: want exactly `gate-armed `", first) + } + if fields[1] != res.Key || fields[2] != res.Epoch || fields[3] != res.DispatchContext { + t.Fatalf("armed line %q: fields (%q,%q,%q), want (key %q, epoch %q, dispatch context %q)", + first, fields[1], fields[2], fields[3], res.Key, res.Epoch, res.DispatchContext) + } + if res.Epoch == "" || res.Epoch == res.DispatchContext { + t.Fatalf("epoch %q must be a distinct non-empty token from the dispatch context %q", res.Epoch, res.DispatchContext) + } + } + t.Run("fresh arm", func(t *testing.T) { + repo := newGateRepo(t) + deps := PlanningDeps{Reader: gateBeforeReader(t, gateBeforeCorpus(), nil, nil), Clock: testClock()} + sp := &fakeScopePrep{grant: sampleScopeGrant()} + check(t, RunGateBefore(context.Background(), deps, WorkspaceDeps{}, sp.deps(), repo, "implement-next", 0)) + }) + t.Run("epochless resume", func(t *testing.T) { + repoDir := newWorkingRepo(t, nil).invocation + deps, wdeps := resumeEpochDeps(t) + sp := &fakeScopePrep{grant: sampleScopeGrant()} + check(t, RunGateBefore(context.Background(), deps, wdeps, sp.deps(), repoDir, "implement-next", 5)) + }) + t.Run("cancelled-replacement resume", func(t *testing.T) { + repoDir := newWorkingRepo(t, nil).invocation + seedPriorEpoch(t, repoDir, EpochCancelled) + deps, wdeps := resumeEpochDeps(t) + sp := &fakeScopePrep{grant: sampleScopeGrant()} + check(t, RunGateBefore(context.Background(), deps, wdeps, sp.deps(), repoDir, "implement-next", 5)) + }) +} diff --git a/internal/app/rungate_before_resume_integration_test.go b/internal/app/rungate_before_resume_integration_test.go index 3eab99c11..081cc1746 100644 --- a/internal/app/rungate_before_resume_integration_test.go +++ b/internal/app/rungate_before_resume_integration_test.go @@ -4,6 +4,7 @@ package app import ( "context" + "encoding/json" "os" "path/filepath" "strings" @@ -773,3 +774,389 @@ func TestIntegrationGateArmResumeSupersededChecksReplacementSlot(t *testing.T) { }) } } + +// TestIntegrationGateArmEpochlessResumeMintsBoundEpoch (change 0463): resuming an in-progress change +// that has NO prior run epoch (its first dispatch was never armed) mints one. The +// epoch is bound to the change and to the verified feature worktree, and the result +// carries its id, so the armed line is always `gate-armed `. +func TestIntegrationGateArmEpochlessResumeMintsBoundEpoch(t *testing.T) { + repoDir := newWorkingRepo(t, nil).invocation + deps, wdeps := resumeEpochDeps(t) + sp := &fakeScopePrep{grant: sampleScopeGrant()} + if _, _, found, err := FindEpochByChange(repoDir, "5"); err != nil || found { + t.Fatalf("fixture must start epochless: found=%v err=%v", found, err) + } + + res := RunGateBefore(context.Background(), deps, wdeps, sp.deps(), repoDir, "implement-next", 5) + if !res.Armed || res.Key == "" { + t.Fatalf("an epochless resume must arm: %q", res.HumanText()) + } + if res.Epoch == "" { + t.Fatalf("an epochless resume armed with no epoch: %q", res.HumanText()) + } + ep, _, err := LoadEpochRecord(repoDir, res.Key) + if err != nil { + t.Fatalf("LoadEpochRecord: %v", err) + } + if ep.EpochID != res.Epoch { + t.Errorf("result Epoch = %q, want the minted epoch id %q", res.Epoch, ep.EpochID) + } + if ep.ChangeID != "5" { + t.Errorf("epoch ChangeID = %q, want \"5\" (bound to the resumed change)", ep.ChangeID) + } + if ep.State != EpochActive { + t.Errorf("epoch state = %q, want active", ep.State) + } + if ep.Worktree != "/tmp/wt/epsilon" { + t.Errorf("epoch Worktree = %q, want the verified feature worktree /tmp/wt/epsilon", ep.Worktree) + } + // JSON consumers see the epoch on this path too, and the shape is unchanged. + buf, err := json.Marshal(res) + if err != nil { + t.Fatalf("marshal: %v", err) + } + for _, want := range []string{`"epoch":"` + res.Epoch + `"`, `"key":"` + res.Key + `"`, `"dispatch_context":"` + scopeGrantChild + `"`} { + if !strings.Contains(string(buf), want) { + t.Errorf("JSON result missing %s: %s", want, buf) + } + } +} + +// TestIntegrationGateArmRepeatEpochlessResumeRefusedActive (change 0463): after an epochless resume +// mints its epoch, a SECOND resume of the same change finds that epoch active and +// refuses resume-active-run with the safe locator, the same single-live-run +// protection every other epoch gets. It mints nothing and prepares no scope. +func TestIntegrationGateArmRepeatEpochlessResumeRefusedActive(t *testing.T) { + repoDir := newWorkingRepo(t, nil).invocation + deps, wdeps := resumeEpochDeps(t) + sp := &fakeScopePrep{grant: sampleScopeGrant()} + first := RunGateBefore(context.Background(), deps, wdeps, sp.deps(), repoDir, "implement-next", 5) + if !first.Armed { + t.Fatalf("first epochless resume must arm: %q", first.HumanText()) + } + + deps2, wdeps2 := resumeEpochDeps(t) + sp2 := &fakeScopePrep{grant: sampleScopeGrant()} + second := RunGateBefore(context.Background(), deps2, wdeps2, sp2.deps(), repoDir, "implement-next", 5) + if second.Armed { + t.Fatalf("a repeat resume over a live minted epoch must not arm: %q", second.HumanText()) + } + if second.Reason != ReasonGateResumeActiveRun { + t.Fatalf("Reason = %q, want %q", second.Reason, ReasonGateResumeActiveRun) + } + if !strings.Contains(second.Message, first.Epoch) || !strings.Contains(second.Message, first.Key) { + t.Fatalf("locator must name epoch %q and key %q, got %q", first.Epoch, first.Key, second.Message) + } + if second.Key != "" || sp2.calls != 0 { + t.Fatalf("an active refusal mints nothing: key=%q calls=%d", second.Key, sp2.calls) + } +} + +// TestIntegrationGateArmEpochlessResumeEpochJoinsCancelCycle (change 0463, Review Focus 2): the epoch +// an epochless resume mints goes through the ordinary lifecycle, driven by the REAL +// cancel path. The resume-active-run refusal names `run cancel` as its remedy, so +// that remedy must work with the arm's own key and epoch: a resume arm has no claim +// binding (change.claim requires a proposed change), and runCancel must accept its +// resume-verified attribution instead of refusing claim-unconfirmed forever. Once +// cancelled, the next resume supersedes the epoch and reserves exactly one +// replacement with a fresh epoch, and that replacement is cancellable the same way. +func TestIntegrationGateArmEpochlessResumeEpochJoinsCancelCycle(t *testing.T) { + repoDir := newWorkingRepo(t, nil).invocation + common, err := gateGitCommonDir(repoDir) + if err != nil { + t.Fatalf("gateGitCommonDir: %v", err) + } + seams := cancelSeams{store: gatedrive.OpenStore(common), stopper: &fakeCancelStopper{}, launches: okLaunchReconciler()} + mkSeams := func(string) cancelSeams { return seams } + + deps, wdeps := resumeEpochDeps(t) + sp := &fakeScopePrep{grant: sampleScopeGrant()} + d := sp.deps() + d.CancelSeams = mkSeams + first := RunGateBefore(context.Background(), deps, wdeps, d, repoDir, "implement-next", 5) + if !first.Armed { + t.Fatalf("first epochless resume must arm: %q", first.HumanText()) + } + + // The documented remedy: cancel with the arm's own key and epoch. + cancel := runCancel(seams, repoDir, first.Key, first.Epoch, "human stop") + if cancel.Disposition != CancelDispositionCancelled { + t.Fatalf("cancelling the epochless resume's epoch: disposition = %q, want cancelled (findings=%v)", cancel.Disposition, cancel.Findings) + } + if st := loadEpochState(t, repoDir, first.Key); st != EpochCancelled { + t.Fatalf("epoch state after cancel = %q, want cancelled", st) + } + + deps2, wdeps2 := resumeEpochDeps(t) + sp2 := &fakeScopePrep{grant: sampleScopeGrant()} + d2 := sp2.deps() + d2.CancelSeams = mkSeams + repl := RunGateBefore(context.Background(), deps2, wdeps2, d2, repoDir, "implement-next", 5) + if !repl.Armed || repl.Epoch == "" || repl.Epoch == first.Epoch { + t.Fatalf("the resume after cancel must arm one replacement with a fresh epoch: %q (first epoch %q)", repl.HumanText(), first.Epoch) + } + prior, _, err := LoadEpochRecord(repoDir, first.Key) + if err != nil { + t.Fatalf("LoadEpochRecord(prior): %v", err) + } + if prior.State != EpochSuperseded || prior.ReplacementReserved != repl.Key { + t.Fatalf("prior epoch = (%q, reserved %q), want superseded reserving %q", prior.State, prior.ReplacementReserved, repl.Key) + } + + // The replacement's epoch is armed by resume too, so it is cancellable the same way. + replCancel := runCancel(seams, repoDir, repl.Key, repl.Epoch, "human stop") + if replCancel.Disposition != CancelDispositionCancelled { + t.Fatalf("cancelling the replacement epoch: disposition = %q, want cancelled (findings=%v)", replCancel.Disposition, replCancel.Findings) + } +} + +// TestIntegrationGateCancelRunCancelResumeAuthorityFailsClosed (change 0463): the resume-verified +// authority runCancel accepts is narrow. A resume-shaped record whose epoch names a +// DIFFERENT change refuses claim-mismatch, and a record carrying an unconfirmed claim +// reservation refuses claim-unconfirmed even though its AttributedID is set. Neither +// refusal fences the epoch. +func TestIntegrationGateCancelRunCancelResumeAuthorityFailsClosed(t *testing.T) { + t.Run("epoch names another change", func(t *testing.T) { + repo := newGateRepo(t) + common, _ := gateGitCommonDir(repo) + key, err := MintGateRecord(repo, GateRecord{ + Target: gateBeforeStoredTarget, AttemptLimit: 2, Retry: RetryUnused, + Disposition: "gate-armed", ParentCap: "parent-cap-raw", AttributedID: 5, + }) + if err != nil { + t.Fatalf("MintGateRecord: %v", err) + } + ep, err := MintEpochRecord(repo, key, "6") + if err != nil { + t.Fatalf("MintEpochRecord: %v", err) + } + res := runCancel(cancelSeams{store: gatedrive.OpenStore(common), stopper: &fakeCancelStopper{}, launches: okLaunchReconciler()}, repo, key, ep.EpochID, "human stop") + if res.Disposition != CancelDispositionRefused || !hasFinding(res.Findings, "claim-mismatch") { + t.Fatalf("got (%q, %v), want refused claim-mismatch", res.Disposition, res.Findings) + } + if st := loadEpochState(t, repo, key); st != EpochActive { + t.Fatalf("a refused cancel must not fence: epoch state = %q", st) + } + }) + t.Run("unconfirmed reservation present", func(t *testing.T) { + repo := newGateRepo(t) + common, _ := gateGitCommonDir(repo) + key, err := MintGateRecord(repo, GateRecord{ + Target: gateBeforeStoredTarget, AttemptLimit: 2, Retry: RetryUnused, + Disposition: "gate-armed", ParentCap: "parent-cap-raw", AttributedID: 5, + }) + if err != nil { + t.Fatalf("MintGateRecord: %v", err) + } + ep, err := MintEpochRecord(repo, key, "5") + if err != nil { + t.Fatalf("MintEpochRecord: %v", err) + } + if err := ReserveGateClaim(repo, key, 5, "req-1"); err != nil { + t.Fatalf("ReserveGateClaim: %v", err) + } + res := runCancel(cancelSeams{store: gatedrive.OpenStore(common), stopper: &fakeCancelStopper{}, launches: okLaunchReconciler()}, repo, key, ep.EpochID, "human stop") + if res.Disposition != CancelDispositionRefused || !hasFinding(res.Findings, "claim-unconfirmed") { + t.Fatalf("got (%q, %v), want refused claim-unconfirmed", res.Disposition, res.Findings) + } + if st := loadEpochState(t, repo, key); st != EpochActive { + t.Fatalf("a refused cancel must not fence: epoch state = %q", st) + } + }) +} + +// TestIntegrationGateArmConcurrentEpochlessResumeEpochsFailSafe (change 0463 decision 4): the +// per-change resume lock keeps concurrent arms from minting two live epochs for one +// change (TestRaceIntegrationAppConcurrencyEpochlessResumesArmOnce), but such a pair can still exist, +// for example left by a binary that predates the lock. A resume over it must fail +// closed as resume-epoch-unreadable and must never arm a third run. Recovery is an +// explicit 'docket run cancel' of either epoch by its own key and epoch, which +// runCancel's resume-verified authority accepts (TestIntegrationGateArmEpochlessResumeEpochJoinsCancelCycle +// drives that cancel path); the surviving epoch is then the worktree's sole live owner. +func TestIntegrationGateArmConcurrentEpochlessResumeEpochsFailSafe(t *testing.T) { + repoDir := newWorkingRepo(t, nil).invocation + seedPriorEpoch(t, repoDir, EpochActive) + seedPriorEpoch(t, repoDir, EpochActive) + deps, wdeps := resumeEpochDeps(t) + sp := &fakeScopePrep{grant: sampleScopeGrant()} + + res := RunGateBefore(context.Background(), deps, wdeps, sp.deps(), repoDir, "implement-next", 5) + if res.Armed { + t.Fatalf("an ambiguous pair of live epochs must never arm: %q", res.HumanText()) + } + if res.Reason != ReasonGateResumeEpochUnreadable { + t.Fatalf("Reason = %q, want %q", res.Reason, ReasonGateResumeEpochUnreadable) + } + if res.Key != "" || sp.calls != 0 { + t.Fatalf("a fail-closed refusal mints nothing: key=%q calls=%d", res.Key, sp.calls) + } +} + +// TestRaceIntegrationAppConcurrencyEpochlessResumesArmOnce (change 0463, post-review): epochless resume +// arms of one change race from the "no prior epoch" check to the mint and bind. The +// per-change resume lock serializes that window, so exactly one arm wins; every other +// arm then sees the winner's live epoch and refuses resume-active-run. Exactly one +// live epoch may end up bound to the change. +func TestRaceIntegrationAppConcurrencyEpochlessResumesArmOnce(t *testing.T) { + const arms = 12 + repoDir := newWorkingRepo(t, nil).invocation + results := make([]RunGateBeforeResult, arms) + start := make(chan struct{}) + var wg sync.WaitGroup + for i := 0; i < arms; i++ { + deps, wdeps := resumeEpochDeps(t) + sp := &fakeScopePrep{grant: sampleScopeGrant()} + wg.Add(1) + go func(i int) { + defer wg.Done() + <-start + results[i] = RunGateBefore(context.Background(), deps, wdeps, sp.deps(), repoDir, "implement-next", 5) + }(i) + } + close(start) + wg.Wait() + + armed := 0 + for _, r := range results { + switch { + case r.Armed: + armed++ + case r.Reason != ReasonGateResumeActiveRun: + t.Errorf("a losing arm must refuse %q, got %q (%s)", ReasonGateResumeActiveRun, r.Reason, r.Message) + } + } + if armed != 1 { + t.Fatalf("armed = %d of %d concurrent epochless resumes, want exactly 1", armed, arms) + } + if _, _, found, err := FindEpochByChange(repoDir, "5"); err != nil || !found { + t.Fatalf("FindEpochByChange after the race: found=%v err=%v, want the single winner's epoch", found, err) + } +} + +// TestIntegrationGateArmResumeRefusalNamesAbandonedArmRemedy (change 0463, post-review): an epochless +// resume arm binds its epoch at arm time, so an arm that was never dispatched blocks +// the next resume until it is cancelled. Nothing records whether an agent is using +// the epoch, so the refusal cannot say which case applies; it names both remedies, +// including the abandoned-arm case, for either kind of incumbent (found by change or +// by worktree). +func TestIntegrationGateArmResumeRefusalNamesAbandonedArmRemedy(t *testing.T) { + for _, msg := range []string{ + resumeActiveLocator("k", EpochRecord{ChangeID: "5", EpochID: "e"}), + resumeWorktreeOwnerLocator("/tmp/wt/epsilon", "k", EpochRecord{EpochID: "e"}), + } { + for _, want := range []string{"never dispatched", "run cancel --key k --epoch e", "still running", "run gate-verdict"} { + if !strings.Contains(msg, want) { + t.Errorf("refusal must contain %q, got %q", want, msg) + } + } + } +} + +// TestIntegrationGateArmEpochlessResumeRefusesLiveWorktreeOwner (change 0463 decision 4, review fix): +// an epochless resume binds its fresh epoch to the verified worktree, so it must not +// mint over a live epoch that already owns that worktree under no change or another +// change. FindEpochByChange cannot see such an owner. Minting anyway would leave two +// active epochs on one worktree, and findEpochByWorktree would then refuse every +// fenced mutation there as ErrEpochOwnerAmbiguous. The resume refuses +// resume-active-run with the owner's locator instead and mints nothing. A fenced +// (cancelled) epoch on the worktree is not a live owner and does not block. +func TestIntegrationGateArmEpochlessResumeRefusesLiveWorktreeOwner(t *testing.T) { + // seedWorktreeOwner mints an epoch bound to worktree (and to changeID, when set) + // in the given state, returning its gate key and epoch id. + seedWorktreeOwner := func(t *testing.T, repoDir, changeID, worktree string, state epochState) (string, string) { + t.Helper() + key := mintTestGateKey(t, repoDir) + ep, err := MintEpochRecord(repoDir, key, changeID) + if err != nil { + t.Fatalf("MintEpochRecord: %v", err) + } + if err := epochCAS(repoDir, key, func(r *EpochRecord) error { + r.Worktree = worktree + r.State = state + return nil + }); err != nil { + t.Fatalf("epochCAS bind: %v", err) + } + return key, ep.EpochID + } + assertRefused := func(t *testing.T, res RunGateBeforeResult, sp *fakeScopePrep, ownerKey, ownerEpoch string) { + t.Helper() + if res.Armed { + t.Fatalf("resume armed over a live worktree owner: %q", res.HumanText()) + } + if res.Reason != ReasonGateResumeActiveRun { + t.Fatalf("Reason = %q, want %q", res.Reason, ReasonGateResumeActiveRun) + } + if !strings.Contains(res.Message, ownerEpoch) || !strings.Contains(res.Message, ownerKey) || !strings.Contains(res.Message, "run cancel") { + t.Fatalf("Message must name the owner's locator (epoch %q, key %q) and the cancel remedy, got %q", ownerEpoch, ownerKey, res.Message) + } + if res.Key != "" || sp.calls != 0 { + t.Fatalf("the refusal must mint no record and prepare no scope: key=%q calls=%d", res.Key, sp.calls) + } + } + + for _, tc := range []struct { + name, changeID string + state epochState + }{ + {"active owner naming no change", "", EpochActive}, + {"active owner naming another change", "7", EpochActive}, + {"completing owner naming another change", "7", EpochCompleting}, + } { + t.Run(tc.name, func(t *testing.T) { + repoDir := newWorkingRepo(t, nil).invocation + ownerKey, ownerEpoch := seedWorktreeOwner(t, repoDir, tc.changeID, "/tmp/wt/epsilon", tc.state) + deps, wdeps := resumeEpochDeps(t) + sp := &fakeScopePrep{grant: sampleScopeGrant()} + res := RunGateBefore(context.Background(), deps, wdeps, sp.deps(), repoDir, "implement-next", 5) + assertRefused(t, res, sp, ownerKey, ownerEpoch) + if st := loadEpochState(t, repoDir, ownerKey); st != tc.state { + t.Fatalf("the incumbent must be untouched: state = %q, want %q", st, tc.state) + } + }) + } + + t.Run("owner bound under a different spelling of the worktree", func(t *testing.T) { + repoDir := newWorkingRepo(t, nil).invocation + raw := filepath.Join(testsupport.TempDir(t), "wt") + if err := os.MkdirAll(raw, 0o755); err != nil { + t.Fatalf("mkdir: %v", err) + } + link := filepath.Join(testsupport.TempDir(t), "wt-link") + if err := os.Symlink(raw, link); err != nil { + t.Fatalf("symlink: %v", err) + } + ownerKey, ownerEpoch := seedWorktreeOwner(t, repoDir, "", raw, EpochActive) + reader := &fakeReader{pin: mainPin(t), corpus: []StatusBlob{inProgressChangeBlob(5, "epsilon", "v5", "")}} + deps := workspaceDepsFor(t, reader) + wdeps := WorkspaceDeps{Service: resumeInspectService(link)} + sp := &fakeScopePrep{grant: sampleScopeGrant()} + res := RunGateBefore(context.Background(), deps, wdeps, sp.deps(), repoDir, "implement-next", 5) + assertRefused(t, res, sp, ownerKey, ownerEpoch) + }) + + t.Run("cancelled epoch on the worktree does not block", func(t *testing.T) { + repoDir := newWorkingRepo(t, nil).invocation + seedWorktreeOwner(t, repoDir, "7", "/tmp/wt/epsilon", EpochCancelled) + deps, wdeps := resumeEpochDeps(t) + sp := &fakeScopePrep{grant: sampleScopeGrant()} + res := RunGateBefore(context.Background(), deps, wdeps, sp.deps(), repoDir, "implement-next", 5) + if !res.Armed || res.Epoch == "" { + t.Fatalf("a fenced epoch on the worktree is not a live owner; the resume must arm: %q", res.HumanText()) + } + }) +} + +// TestIntegrationGateArmArmedGateResultRequiresEpoch (change 0463): the armed constructor refuses to +// arm without an epoch. That guarantee is what makes the positional three-token +// line unambiguous. +func TestIntegrationGateArmArmedGateResultRequiresEpoch(t *testing.T) { + if got := armedGateResult("k", "", "ctx"); got.Armed || got.Reason != ReasonGateMintFailed || got.Key != "" || got.DispatchContext != "" { + t.Fatalf("an epochless armed result must fail closed as gate-unarmed mint-failed, got %+v", got) + } + got := armedGateResult("k", "e", "ctx") + if !got.Armed || got.Result != ResultApplied || got.Key != "k" || got.Epoch != "e" || got.DispatchContext != "ctx" || + got.Target != gateBeforeStoredTarget || got.OwnerLifecycle != ReasonOwnerLifecycleUnavailable { + t.Fatalf("armed result fields wrong: %+v", got) + } +} diff --git a/internal/app/rungate_cancel.go b/internal/app/rungate_cancel.go index 8d392442f..a8ddc9057 100644 --- a/internal/app/rungate_cancel.go +++ b/internal/app/rungate_cancel.go @@ -12,7 +12,9 @@ // pins: the record's repository must be the current repository (LoadGateRecord fails // closed on wrong-repo), the presented epoch id must equal the record's public // EpochID, the record must carry a parent-held authority (a non-empty ParentCap), -// and a CONFIRMED claim binding for the epoch's change must exist (LoadGateClaimBinding). +// and a CONFIRMED claim binding for the epoch's change must exist (LoadGateClaimBinding) +// — or, for a record armed by `gate-before --resume`, the resume-verified attribution +// (AttributedID set, no claim binding at all), the shape resolveGateOwnership accepts. // Any missing/mismatched conjunct is a `refused` disposition with a bounded finding — // never a fence, never a stop. // @@ -317,10 +319,24 @@ func runCancel(seams cancelSeams, repoDir, key, expectEpoch, reason string) RunC if berr != nil { return cancelRefused("claim-unreadable") } - if !ok || !binding.Confirmed { + ownerID := 0 + switch { + case ok && binding.Confirmed: + ownerID = binding.ChangeID + case !ok && rec.resumeAttributed(): + // Resume-verified authority (change 0463): `gate-before --resume` pre-binds + // AttributedID through WorkspaceInspect identity and never gets a claim binding + // (change.claim requires a proposed change). It is the same shape + // resolveGateOwnership accepts as ownership. Without it, the epoch a resume arm + // mints could never be cancelled, and the next resume would refuse + // resume-active-run with a remedy that always refuses. Only a record with NO + // binding file qualifies: a reservation that exists but is unconfirmed still + // refuses below. + ownerID = rec.AttributedID + default: return cancelRefused("claim-unconfirmed") } - if ep.ChangeID != "" && strconv.Itoa(binding.ChangeID) != ep.ChangeID { + if ep.ChangeID != "" && strconv.Itoa(ownerID) != ep.ChangeID { return cancelRefused("claim-mismatch") } diff --git a/internal/app/rungate_epoch_integration_test.go b/internal/app/rungate_epoch_integration_test.go index 754c4c592..6d3d20b05 100644 --- a/internal/app/rungate_epoch_integration_test.go +++ b/internal/app/rungate_epoch_integration_test.go @@ -432,3 +432,39 @@ func TestIntegrationGateEpochEpochSettledResolverStates(t *testing.T) { } }) } + +// TestIntegrationGateEpochCheckRunEpochLinkage (change 0463): the agent.enter preflight answers with a +// typed EpochError. Not-found covers both a gate key with no epoch and a gate key +// that does not exist (the pair names no epoch). Mismatch covers a different +// recorded id. A matching pair is nil. +func TestIntegrationGateEpochCheckRunEpochLinkage(t *testing.T) { + repo := newGateRepo(t) + bare := mintTestGateKey(t, repo) + if err := CheckRunEpochLinkage(repo, bare, "0790b760e26444866ef2e156ba383326"); !isEpochKind(err, ErrEpochNotFound) { + t.Fatalf("gate key without an epoch: got %v, want epoch-not-found", err) + } + + withEpoch := mintTestGateKey(t, repo) + ep, err := MintEpochRecord(repo, withEpoch, "463") + if err != nil { + t.Fatalf("MintEpochRecord: %v", err) + } + if err := CheckRunEpochLinkage(repo, withEpoch, ep.EpochID); err != nil { + t.Fatalf("matching pair must pass, got %v", err) + } + if err := CheckRunEpochLinkage(repo, withEpoch, "0790b760e26444866ef2e156ba383326"); !isEpochKind(err, ErrEpochMismatch) { + t.Fatalf("wrong epoch id: got %v, want epoch-mismatch", err) + } + + gone := mintTestGateKey(t, repo) + root, rerr := gateRoot(repo) + if rerr != nil { + t.Fatalf("gateRoot: %v", rerr) + } + if err := os.RemoveAll(filepath.Join(root, gone)); err != nil { + t.Fatalf("remove gate dir: %v", err) + } + if err := CheckRunEpochLinkage(repo, gone, ep.EpochID); !isEpochKind(err, ErrEpochNotFound) { + t.Fatalf("absent gate key: got %v, want epoch-not-found", err) + } +} diff --git a/internal/app/rungate_epoch_refusal.go b/internal/app/rungate_epoch_refusal.go new file mode 100644 index 000000000..dc0cbb04f --- /dev/null +++ b/internal/app/rungate_epoch_refusal.go @@ -0,0 +1,117 @@ +package app + +import "path/filepath" + +// This file is the run-epoch refusal vocabulary (change 0463). The run epoch is a +// public locator (ADR-0111) that a caller threads into --run-epoch flags (gate drive +// start, gate drive prepare-scope, agent.enter). When the presented value cannot be +// resolved, the caller must learn WHICH mistake it made through a stable token. A +// catch-all invalid-request makes a misrouted token (0382: the dispatch context +// passed as the epoch) indistinguishable from a malformed request. Tokens are a +// fixed vocabulary; nothing here echoes the presented value, a path, or record +// content. + +// ReasonUnknownRunEpoch is the stable refusal token for a --run-epoch that names no +// run epoch in this repository. +const ReasonUnknownRunEpoch = "unknown-run-epoch" + +// ClassifyRunEpochError maps a run-epoch registry failure (an *EpochError anywhere +// in err's chain) to a protocol result and a bounded reason token: +// - not-found: unknown-run-epoch. +// - mismatch: the existing stale-linkage token, stale-run-epoch. +// - corrupt or unreadable: internal-error carrying the kind. +// - any other readable-but-unusable registry state: invalid-input carrying the kind. +// +// ok is false when err carries no *EpochError, so callers fall through to their +// own classification. +func ClassifyRunEpochError(err error) (Result, string, bool) { + ee, ok := AsEpochError(err) + if !ok { + return "", "", false + } + switch ee.Kind { + case ErrEpochNotFound: + return ResultInvalidInput, ReasonUnknownRunEpoch, true + case ErrEpochMismatch: + return ResultInvalidInput, ErrStaleRunEpoch.Reason, true + case ErrEpochCorrupt, ErrEpochIO: + return ResultInternalError, string(ee.Kind), true + default: + return ResultInvalidInput, string(ee.Kind), true + } +} + +// RunEpochNextAction maps a run-epoch refusal reason to a one-line, credential-free +// next action (the ownershipNextAction / fenceNextAction pattern). It never echoes +// the presented value. A reason with no specific remedy yields "", and callers then +// omit the message. +func RunEpochNextAction(reason string) string { + switch reason { + case ReasonUnknownRunEpoch: + return "the --run-epoch value names no run epoch in this repository; pass the field of the arm's " + + "`gate-armed ` line (the goes to --gate-context) — " + + "never drop --run-epoch and retry" + case ErrStaleRunEpoch.Reason: + return "the --run-epoch value is not the run epoch this gate key carries; pass the printed on the same gate-armed line as the key" + default: + return "" + } +} + +// runEpochLocator builds the existence check prepare-scope runs on a presented +// --run-epoch. It resolves the id to exactly one epoch record under gitCommonDir's +// run-epoch registry through findEpochDirByID (the same locator the epoch launch +// gate uses) and returns its typed EpochError (not-found, ambiguous, IO) unchanged. +// It checks RESOLVABILITY only, never liveness: a scope may legitimately carry a +// cancelled epoch (the takeover revocation gate reads it later), and the launch gate +// still enforces liveness and worktree ownership at start. +func runEpochLocator(gitCommonDir string) func(string) error { + rungateRoot := filepath.Join(gitCommonDir, "docket", "rungate") + return func(epochID string) error { + _, _, err := findEpochDirByID(rungateRoot, epochID) + return err + } +} + +// CheckRunEpochExists verifies, before agent.enter spawns anything, that a lone +// --run-epoch (presented without --run-gate-key) resolves to exactly one run epoch in +// repoDir's repository (change 0463). It is the same resolvability check prepare-scope +// runs (runEpochLocator / findEpochDirByID): it never checks liveness, and it returns +// the locator's typed *EpochError (not-found, ambiguous, IO) unchanged. A repository +// whose git common dir cannot be resolved yields ErrEpochIO. +func CheckRunEpochExists(repoDir, epochID string) error { + common, err := gateGitCommonDir(repoDir) + if err != nil { + return epochErr(ErrEpochIO, "check-exists", err) + } + return runEpochLocator(common)(epochID) +} + +// CheckRunEpochLinkage verifies, before agent.enter spawns anything, that the +// presented (--run-gate-key, --run-epoch) pair names a real run epoch (change 0463). +// It returns nil when the gate key's epoch record carries exactly epochID, and +// otherwise ALWAYS an *EpochError: +// - a gate key with no directory, a malformed key, or no epoch record: +// ErrEpochNotFound (the pair names no epoch); +// - a different recorded id: ErrEpochMismatch; +// - a corrupt record: keeps ErrEpochCorrupt; +// - any other resolution fault: ErrEpochIO. +// +// It only reads. It never checks liveness, because participant registration still +// refuses a non-active epoch. +func CheckRunEpochLinkage(repoDir, gateKey, epochID string) error { + rec, _, err := LoadEpochRecord(repoDir, gateKey) + if err != nil { + if _, ok := AsEpochError(err); ok { + return err + } + if ge, ok := AsGateStoreError(err); ok && (ge.Kind == ErrGateNotFound || ge.Kind == ErrGateMalformedKey) { + return epochErr(ErrEpochNotFound, "check-linkage", nil) + } + return epochErr(ErrEpochIO, "check-linkage", err) + } + if rec.EpochID != epochID { + return epochErr(ErrEpochMismatch, "check-linkage", nil) + } + return nil +} diff --git a/internal/app/rungate_epoch_refusal_test.go b/internal/app/rungate_epoch_refusal_test.go new file mode 100644 index 000000000..c586cb13f --- /dev/null +++ b/internal/app/rungate_epoch_refusal_test.go @@ -0,0 +1,65 @@ +package app + +import ( + "errors" + "fmt" + "strings" + "testing" +) + +// TestClassifyRunEpochError (change 0463): every run-epoch registry failure maps to +// a stable protocol result and a fixed reason token. A not-found epoch is the named +// unknown-run-epoch. A presented value wrapped into the error chain never leaks into +// the reason. +func TestClassifyRunEpochError(t *testing.T) { + const presented = "0790b760e26444866ef2e156ba383326" + cases := []struct { + kind EpochErrorKind + res Result + reason string + }{ + {ErrEpochNotFound, ResultInvalidInput, "unknown-run-epoch"}, + {ErrEpochMismatch, ResultInvalidInput, "stale-run-epoch"}, + {ErrEpochAmbiguous, ResultInvalidInput, "epoch-ambiguous"}, + {ErrEpochNotActive, ResultInvalidInput, "epoch-not-active"}, + {ErrEpochOwnerAmbiguous, ResultInvalidInput, "epoch-owner-ambiguous"}, + {ErrEpochOwnerUnresolved, ResultInvalidInput, "epoch-owner-unresolved"}, + {ErrEpochCorrupt, ResultInternalError, "epoch-corrupt"}, + {ErrEpochIO, ResultInternalError, "epoch-io"}, + } + for _, tc := range cases { + err := fmt.Errorf("start refused for %s: %w", presented, epochErr(tc.kind, "find-dir-by-id", errors.New(presented))) + res, reason, ok := ClassifyRunEpochError(err) + if !ok || res != tc.res || reason != tc.reason { + t.Errorf("%s: got (%s, %q, %v), want (%s, %q, true)", tc.kind, res, reason, ok, tc.res, tc.reason) + } + if strings.Contains(reason, presented) { + t.Errorf("%s: reason leaked the presented value: %q", tc.kind, reason) + } + } + if _, _, ok := ClassifyRunEpochError(errors.New("plain failure")); ok { + t.Error("a non-epoch error must not classify") + } + if _, _, ok := ClassifyRunEpochError(ErrStaleRunEpoch); ok { + t.Error("a mutation-fence error is not an epoch-registry error") + } +} + +// TestRunEpochNextAction (change 0463): unknown-run-epoch tells the caller which +// gate-armed field goes where. Its text is distinct from the stale-linkage remedy. +// Other reasons carry no invented message. +func TestRunEpochNextAction(t *testing.T) { + unknown := RunEpochNextAction(ReasonUnknownRunEpoch) + for _, want := range []string{"--run-epoch", "--gate-context", "gate-armed "} { + if !strings.Contains(unknown, want) { + t.Errorf("unknown-run-epoch message must mention %q, got %q", want, unknown) + } + } + stale := RunEpochNextAction("stale-run-epoch") + if stale == "" || stale == unknown { + t.Errorf("stale-run-epoch needs its own message, got %q", stale) + } + if got := RunEpochNextAction("epoch-io"); got != "" { + t.Errorf("an unmapped reason must yield no message, got %q", got) + } +} diff --git a/internal/app/rungate_epochless_resume_e2e_integration_test.go b/internal/app/rungate_epochless_resume_e2e_integration_test.go new file mode 100644 index 000000000..0455bf075 --- /dev/null +++ b/internal/app/rungate_epochless_resume_e2e_integration_test.go @@ -0,0 +1,88 @@ +//go:build integration + +package app + +import ( + "context" + "path/filepath" + "strings" + "testing" + + "github.com/danielhanold/docket/internal/gatedrive" + "github.com/danielhanold/docket/internal/testsupport" +) + +// TestIntegrationGateArmEpochlessResumeEndToEnd0382 reproduces change 0382's resumed run (change 0463). +// The change was claimed by an UNARMED first dispatch, so no run epoch exists. The +// resume arm must print `gate-armed `. Parsed +// positionally (as AGENTS.md tells a parent), the is admitted by the real +// epoch launch gate for the resumed worktree and recorded on its execution slot. +// The misrouted 0382 call (the dispatch context presented as the epoch) is refused +// with the named unknown-run-epoch. The resume inspect path uses the raw temp +// spelling and the start uses the symlink-resolved one (Review Focus 1). +func TestIntegrationGateArmEpochlessResumeEndToEnd0382(t *testing.T) { + repoDir := newWorkingRepo(t, nil).invocation + worktree, err := filepath.EvalSymlinks(repoDir) + if err != nil { + t.Fatalf("EvalSymlinks: %v", err) + } + common, err := gateGitCommonDir(repoDir) + if err != nil { + t.Fatalf("gateGitCommonDir: %v", err) + } + if _, _, found, ferr := FindEpochByChange(repoDir, "5"); ferr != nil || found { + t.Fatalf("the unarmed claim must leave no epoch: found=%v err=%v", found, ferr) + } + + // Arm the resume through the REAL outer-scope store, as production composes it. + reader := &fakeReader{pin: mainPin(t), corpus: []StatusBlob{inProgressChangeBlob(5, "epsilon", "v5", "")}} + deps := workspaceDepsFor(t, reader) + wdeps := WorkspaceDeps{Service: resumeInspectService(repoDir)} + store := gatedrive.OpenStore(common) + arm := RunGateBefore(context.Background(), deps, wdeps, GateScopeDeps{Prepare: store.PrepareScope}, repoDir, "implement-next", 5) + if !arm.Armed { + t.Fatalf("the epochless resume must arm: %q", arm.HumanText()) + } + + fields := strings.Fields(strings.SplitN(arm.HumanText(), "\n", 2)[0]) + if len(fields) != 4 || fields[0] != "gate-armed" { + t.Fatalf("armed line %q must be `gate-armed `", arm.HumanText()) + } + key, epoch, dispatchCtx := fields[1], fields[2], fields[3] + if key != arm.Key || epoch != arm.Epoch || dispatchCtx != arm.DispatchContext { + t.Fatalf("positional fields (%q,%q,%q) disagree with the result (%q,%q,%q)", key, epoch, dispatchCtx, arm.Key, arm.Epoch, arm.DispatchContext) + } + + svc, res, reason := NewTaskGateDriveService(common, "/bin/true", buildEffWithMaxAttempts("go test ./...", 4), []string{"/bin/echo", "ok"}) + if svc == nil { + t.Fatalf("task service: %s (%s)", res, reason) + } + req := GateDriveStartRequest{ + RepoDir: common, Worktree: worktree, ChangeID: "5", TaskID: "task-6", Phase: "build", + RunRoot: testsupport.TempDir(t), Cwd: worktree, GateContext: dispatchCtx, RunEpochID: epoch, + } + + // The misrouted 0382 call: the dispatch context presented as the run epoch. + bad := req + bad.RunEpochID = dispatchCtx + if _, berr := svc.engine.Admit(svc.startRequest(bad)); berr == nil { + t.Fatalf("the dispatch context must never admit as a run epoch") + } else if r, why := mapDriveFailure(berr); r != ResultInvalidInput || why != ReasonUnknownRunEpoch { + t.Fatalf("misrouted epoch refused as (%s, %q), want (invalid-input, unknown-run-epoch)", r, why) + } + + // The correctly parsed epoch is admitted by the real launch gate. + ticket, aerr := svc.engine.Admit(svc.startRequest(req)) + if aerr != nil { + r, why := mapDriveFailure(aerr) + t.Fatalf("the parsed epoch must admit for the resumed worktree, got (%s, %q): %v", r, why, aerr) + } + t.Cleanup(func() { _ = svc.engine.AbandonAdmission(ticket) }) + slot, _, lerr := store.LoadWorktreeExecution(worktree) + if lerr != nil { + t.Fatalf("LoadWorktreeExecution: %v", lerr) + } + if slot.RunEpochID != epoch { + t.Fatalf("worktree slot RunEpochID = %q, want the armed epoch %q", slot.RunEpochID, epoch) + } +} diff --git a/internal/app/rungate_store.go b/internal/app/rungate_store.go index 1cdc3c013..c7e243e36 100644 --- a/internal/app/rungate_store.go +++ b/internal/app/rungate_store.go @@ -181,6 +181,17 @@ type GateRecord struct { BoundRevision string `json:"bound_revision,omitempty"` } +// resumeAttributed reports whether rec has the resume-verified shape: `gate-before +// --resume` pre-bound AttributedID through WorkspaceInspect identity, and no claim +// ever confirmed under it (BoundRequestID is still empty). A fresh arm gains +// AttributedID only at confirm time, together with BoundRequestID, so it never +// matches. resolveGateOwnership accepts this shape as ownership, runCancel accepts +// it as cancel authority, and ChangeClaim refuses to reserve a claim under it +// (change 0463). +func (rec GateRecord) resumeAttributed() bool { + return rec.AttributedID != 0 && rec.BoundRequestID == "" +} + // gateBoundPairOK reports whether rec's claim-binding mirror pair is well formed: // BoundRequestID and BoundRevision must be ALL-EMPTY or ALL-SET (0396's pair rule // applied to the change-0407 mirror). A partial pair is a corrupt record — the diff --git a/internal/app/rungate_verdict.go b/internal/app/rungate_verdict.go index 43d961fd9..19fe27aa4 100644 --- a/internal/app/rungate_verdict.go +++ b/internal/app/rungate_verdict.go @@ -593,7 +593,7 @@ func resolveGateOwnership(ctx context.Context, deps PlanningDeps, wdeps Workspac // Resume-verified shape: an AttributedID with no claim binding was pre-bound by // `gate-before --resume` through WorkspaceInspect identity. Continuity for it is // RunVerify's job, exactly as today — the proof continuity check never runs. - if rec.AttributedID != 0 && rec.BoundRequestID == "" { + if rec.resumeAttributed() { return nil } diff --git a/internal/assets/embedded/manifest.json b/internal/assets/embedded/manifest.json index 7f27deaf3..2913fc4a0 100644 --- a/internal/assets/embedded/manifest.json +++ b/internal/assets/embedded/manifest.json @@ -1,7 +1,7 @@ { "format_version": 1, "asset_protocol": 1, - "asset_set_id": "sha256:2bcf93ecad4fae175b3bfba5657d2abe90bbaf431b324da317ef451c9e1b5a1e", + "asset_set_id": "sha256:27f68e20e117b2fed928af4bacd062f0ee2ee752e6e59196ab0c4e3b2af50e3c", "entries": [ { "path": ".docket.example.yml", @@ -414,7 +414,7 @@ "role": "skill", "mode": 420, "size": 10479, - "sha256": "6a9b02a9991d934954e2a3c7c462b2e9428ae481f09a2d5251b9fa07559a7865" + "sha256": "315b9876ff9acdb72f658a20a61459e43c894c5f238bea487631ab7cdf282f6d" }, { "path": "skills/docket-implement-next/references/fix-loop.md", diff --git a/internal/assets/embedded/tree/skills/docket-implement-next/references/edge-paths.md b/internal/assets/embedded/tree/skills/docket-implement-next/references/edge-paths.md index eb3d4b528..a83849833 100644 --- a/internal/assets/embedded/tree/skills/docket-implement-next/references/edge-paths.md +++ b/internal/assets/embedded/tree/skills/docket-implement-next/references/edge-paths.md @@ -57,11 +57,11 @@ this resume path with its marker gone. worktree carries at most one live run. When the caller arms the resume (`run.gate-before … --resume `), the arm refuses to open a second run over one that has not verifiably stopped: -- Prior epoch still **active** → refused `resume-active-run`, with a locator naming the change, - epoch, and gate key and the remedy: cancel the prior run via the `run.cancel` operation (`--key - --epoch --reason `) and resume after confirmed cancellation, or continue the live - run via `run.gate-verdict`. **Never** force a fresh claim over a possibly-live run — that is the - claim-theft the gate exists to prevent. +- Prior epoch still **active** (an undispatched earlier resume arm counts) → refused + `resume-active-run`, naming the change, epoch, and gate key, with the remedy: cancel the prior run + via the `run.cancel` operation (`--key --epoch --reason `) and resume after + confirmed cancellation, or continue the live run via `run.gate-verdict`. **Never** force a fresh + claim over a possibly-live run — that is claim theft. - Cancellation still finishing → refused `cancellation-pending`; the resume observes that cleanup only. Finish the cancel first, then resume. - Prior epoch confirmed-cancelled and superseded → the arm reserves **exactly one** replacement diff --git a/internal/cli/agent.go b/internal/cli/agent.go index 877ca527b..12104de83 100644 --- a/internal/cli/agent.go +++ b/internal/cli/agent.go @@ -93,7 +93,26 @@ func newAgentCommand(info buildinfo.Info, setResult func(app.OperationResult)) * // confer. The epoch id is a public locator; the dispatch-context child // capability continues to carry authority. isRootCoordinator := contract.LaunchPosture == harness.LaunchRootCoordinator + if runGateKey == "" && runEpoch != "" { + // A lone --run-epoch (the shape AGENTS.md documents) carries no gate key to + // register against, but it is still preflighted for existence so a misrouted + // token (0382: the dispatch context passed as the epoch) refuses with + // unknown-run-epoch instead of proceeding silently unlinked (change 0463). + if lerr := app.CheckRunEpochExists(effectiveCWD, runEpoch); lerr != nil { + res, reason, _ := app.ClassifyRunEpochError(lerr) + setResult(runEpochRefusal(role, res, reason)) + return nil + } + } if runGateKey != "" && runEpoch != "" { + // Preflight the linkage BEFORE anything is spawned (change 0463). An unknown + // or mismatched epoch refuses with its named token, instead of surfacing as a + // generic root-entry failure after Codex already started a thread. + if lerr := app.CheckRunEpochLinkage(effectiveCWD, runGateKey, runEpoch); lerr != nil { + res, reason, _ := app.ClassifyRunEpochError(lerr) + setResult(runEpochRefusal(role, res, reason)) + return nil + } kind := "task" if isRootCoordinator { kind = "coordinator" @@ -109,6 +128,12 @@ func newAgentCommand(info buildinfo.Info, setResult func(app.OperationResult)) * } out, err := client.Enter(c.Context(), codexentry.Request{Contract: contract, UserRequest: string(request), CWD: effectiveCWD, ApprovalPolicy: approval, Sandbox: sandbox, Skills: skills}) if err != nil { + // A registration-time epoch fault (e.g. the epoch was fenced after the + // preflight) keeps its named token (change 0463). + if res, reason, ok := app.ClassifyRunEpochError(err); ok { + setResult(runEpochRefusal(role, res, reason)) + return nil + } setResult(app.AgentEnterResult{Envelope: app.NewEnvelope(app.OperationAgentEnter, app.ResultExternalFailed), Role: role, Reason: "root-entry-failed", Message: err.Error()}) return nil } @@ -153,6 +178,17 @@ func (r epochParticipantRegistrar) RegisterParticipant(handle string) error { }) } +// runEpochRefusal renders a typed run-epoch linkage failure as the agent.enter +// refusal (change 0463): the named reason token and a credential-free next action. +// It never includes the presented value. +func runEpochRefusal(role string, res app.Result, reason string) app.AgentEnterResult { + msg := app.RunEpochNextAction(reason) + if msg == "" { + msg = "run-epoch linkage refused (" + reason + ")" + } + return app.AgentEnterResult{Envelope: app.NewEnvelope(app.OperationAgentEnter, res), Role: role, Reason: reason, Message: msg} +} + // epochTerminalRecorder adapts app.RecordEpochParticipantTerminal to codexentry's // TerminalRecorder (change 0441 Task 9): after the entry's turn settles, it stamps // the exact terminal observation onto the matching run-epoch participant. The epoch diff --git a/internal/cli/agent_test.go b/internal/cli/agent_test.go index 9828278d0..ec467aee9 100644 --- a/internal/cli/agent_test.go +++ b/internal/cli/agent_test.go @@ -427,3 +427,133 @@ func TestAgentEnterRequiresClosedExecutionContext(t *testing.T) { }) } } + +// TestAgentEnterRefusesBadRunEpochLinkageBeforeLaunch (change 0463): an agent.enter +// whose --run-gate-key/--run-epoch pair names no run epoch, or names a different one, +// is refused with a named token BEFORE Codex is spawned. A stub codex that records +// any invocation proves nothing launched. The presented value never appears in the +// JSON or human output. +func TestAgentEnterRefusesBadRunEpochLinkageBeforeLaunch(t *testing.T) { + seedAgentInstallation(t) + repo := gateDriveRepo(t) + bin := testsupport.TempDir(t) + marker := filepath.Join(bin, "codex-invoked") + stub := "#!/bin/sh\ntouch '" + strings.ReplaceAll(marker, "'", "'\\''") + "'\nexit 1\n" + if err := os.WriteFile(filepath.Join(bin, "codex"), []byte(stub), 0o755); err != nil { + t.Fatal(err) + } + t.Setenv("PATH", bin+string(os.PathListSeparator)+os.Getenv("PATH")) + + const bogus = "0790b760e26444866ef2e156ba383326" + mintKey := func() string { + key, err := app.MintGateRecord(repo, app.GateRecord{Target: "docket-implement-next", AttemptLimit: 1, Retry: app.RetryUnused, Disposition: "gate-armed"}) + if err != nil { + t.Fatalf("MintGateRecord: %v", err) + } + return key + } + bare := mintKey() + withEpoch := mintKey() + if _, err := app.MintEpochRecord(repo, withEpoch, "463"); err != nil { + t.Fatalf("MintEpochRecord: %v", err) + } + + for _, tc := range []struct { + name, key, wantReason string + }{ + {"gate key with no epoch", bare, "unknown-run-epoch"}, + {"epoch id not the key's", withEpoch, "stale-run-epoch"}, + } { + t.Run(tc.name, func(t *testing.T) { + base := []string{"agent", "enter", "--role", "docket-implement-next", "--request", "-", "--cwd", repo, + "--approval-policy", "never", "--sandbox", "workspace-write", "--run-gate-key", tc.key, "--run-epoch", bogus} + var out, stderr bytes.Buffer + Run(append(base, "--json"), strings.NewReader("req"), &out, &stderr, devInfo(), hostFacts()) + var res app.AgentEnterResult + if err := json.Unmarshal(out.Bytes(), &res); err != nil { + t.Fatalf("decode %q: %v (stderr %q)", out.String(), err, stderr.String()) + } + if res.Result != app.ResultInvalidInput || res.Reason != tc.wantReason { + t.Fatalf("got (%s, %q), want (invalid-input, %q): %+v", res.Result, res.Reason, tc.wantReason, res) + } + if strings.Contains(out.String(), bogus) { + t.Fatalf("JSON output leaked the presented value: %s", out.String()) + } + var human, herr bytes.Buffer + Run(base, strings.NewReader("req"), &human, &herr, devInfo(), hostFacts()) + if !strings.Contains(human.String()+herr.String(), "--run-epoch") || strings.Contains(human.String()+herr.String(), bogus) { + t.Fatalf("human output must name the remedy and never the value: out=%q err=%q", human.String(), herr.String()) + } + if _, err := os.Stat(marker); err == nil { + t.Fatalf("codex was launched despite a bad run-epoch linkage") + } + }) + } +} + +// TestAgentEnterLoneRunEpochIsPreflightedBeforeLaunch (change 0463, review fix): +// AGENTS.md threads only --run-epoch into agent.enter, so a lone --run-epoch (no +// --run-gate-key) must still be checked for existence before Codex is spawned. A +// misrouted token (0382: the dispatch context passed as the epoch) refuses with +// unknown-run-epoch and launches nothing; a lone epoch that DOES exist passes the +// preflight and reaches the launch (the stub codex records the invocation). +func TestAgentEnterLoneRunEpochIsPreflightedBeforeLaunch(t *testing.T) { + seedAgentInstallation(t) + repo := gateDriveRepo(t) + bin := testsupport.TempDir(t) + marker := filepath.Join(bin, "codex-invoked") + stub := "#!/bin/sh\ntouch '" + strings.ReplaceAll(marker, "'", "'\\''") + "'\nexit 1\n" + if err := os.WriteFile(filepath.Join(bin, "codex"), []byte(stub), 0o755); err != nil { + t.Fatal(err) + } + t.Setenv("PATH", bin+string(os.PathListSeparator)+os.Getenv("PATH")) + + key, err := app.MintGateRecord(repo, app.GateRecord{Target: "docket-implement-next", AttemptLimit: 1, Retry: app.RetryUnused, Disposition: "gate-armed"}) + if err != nil { + t.Fatalf("MintGateRecord: %v", err) + } + rec, err := app.MintEpochRecord(repo, key, "463") + if err != nil { + t.Fatalf("MintEpochRecord: %v", err) + } + enter := func(epoch string, extra ...string) []string { + return append([]string{"agent", "enter", "--role", "docket-implement-next", "--request", "-", "--cwd", repo, + "--approval-policy", "never", "--sandbox", "workspace-write", "--run-epoch", epoch}, extra...) + } + + const bogus = "0790b760e26444866ef2e156ba383326" + var out, stderr bytes.Buffer + Run(enter(bogus, "--json"), strings.NewReader("req"), &out, &stderr, devInfo(), hostFacts()) + var res app.AgentEnterResult + if err := json.Unmarshal(out.Bytes(), &res); err != nil { + t.Fatalf("decode %q: %v (stderr %q)", out.String(), err, stderr.String()) + } + if res.Result != app.ResultInvalidInput || res.Reason != app.ReasonUnknownRunEpoch { + t.Fatalf("lone unknown --run-epoch: got (%s, %q), want (invalid-input, %q): %+v", res.Result, res.Reason, app.ReasonUnknownRunEpoch, res) + } + if strings.Contains(out.String(), bogus) { + t.Fatalf("JSON output leaked the presented value: %s", out.String()) + } + var human, herr bytes.Buffer + Run(enter(bogus), strings.NewReader("req"), &human, &herr, devInfo(), hostFacts()) + if !strings.Contains(human.String()+herr.String(), "--run-epoch") || strings.Contains(human.String()+herr.String(), bogus) { + t.Fatalf("human output must name the remedy and never the value: out=%q err=%q", human.String(), herr.String()) + } + if _, err := os.Stat(marker); err == nil { + t.Fatalf("codex was launched despite an unknown lone --run-epoch") + } + + out.Reset() + stderr.Reset() + Run(enter(rec.EpochID, "--json"), strings.NewReader("req"), &out, &stderr, devInfo(), hostFacts()) + res = app.AgentEnterResult{} + if err := json.Unmarshal(out.Bytes(), &res); err != nil { + t.Fatalf("decode %q: %v (stderr %q)", out.String(), err, stderr.String()) + } + if res.Reason == app.ReasonUnknownRunEpoch { + t.Fatalf("a lone --run-epoch that exists was refused: %+v", res) + } + if _, err := os.Stat(marker); err != nil { + t.Fatalf("a lone existing --run-epoch must pass the preflight and reach launch; codex not invoked (result %+v)", res) + } +} diff --git a/internal/cli/gate_test.go b/internal/cli/gate_test.go index 6d1daf1e7..d5c4e87c2 100644 --- a/internal/cli/gate_test.go +++ b/internal/cli/gate_test.go @@ -1050,3 +1050,62 @@ func TestGateLaunchInsideWorktreeSecondRefused(t *testing.T) { t.Fatalf("refused launch produced a run_dir %q", rd) } } + +// TestGateDriveStartUnknownRunEpochIsNamed (change 0463): the 0382 misuse, where a +// well-formed but unknown --run-epoch (a dispatch-context-shaped 32-hex token) goes +// through the REAL epoch launch gate, is refused invalid-input with the named +// unknown-run-epoch, never the catch-all invalid-request. The presented value is +// never echoed. +func TestGateDriveStartUnknownRunEpochIsNamed(t *testing.T) { + wt := gateDriveConfiguredRepo(t, "metadata_branch: main\n") + root := testsupport.TempDir(t) + const bogus = "0790b760e26444866ef2e156ba383326" + out, _, _ := runCLI(t, "--json", "gate", "drive", "start", + "--repo-dir", wt, "--run-root", root, "--owner", "task", + "--change-id", "463", "--task-id", "task-3", "--phase", "build", "--branch", "fix/x", + "--run-epoch", bogus, "--", "/bin/echo", "hi") + doc := decodeOneJSON(t, out) + if doc["result"] != "invalid-input" || doc["reason"] != "unknown-run-epoch" { + t.Fatalf("unknown --run-epoch must refuse invalid-input/unknown-run-epoch, got %v", doc) + } + if _, ok := doc["drive"]; ok { + t.Fatalf("a refused start must carry no drive document: %v", doc) + } + if msg, _ := doc["message"].(string); !strings.Contains(msg, "--gate-context") { + t.Fatalf("refusal must carry the next action, got %q", msg) + } + if strings.Contains(out, bogus) { + t.Fatalf("the presented --run-epoch value leaked into the output: %s", out) + } +} + +// TestGateDrivePrepareScopeUnknownRunEpochIsNamed (change 0463): through the real +// wiring, prepare-scope with an unknown --run-epoch refuses unknown-run-epoch and +// mints no scope. In human mode it renders reason + remedy without the value. The +// cancelled-epoch prepare in TestGateDrivePrepareScopeRunEpochGatesTakeover must +// stay applied, because the pre-check is resolvability only, never liveness. +func TestGateDrivePrepareScopeUnknownRunEpochIsNamed(t *testing.T) { + wt := gateDriveRepo(t) + const bogus = "0790b760e26444866ef2e156ba383326" + args := []string{"gate", "drive", "prepare-scope", "--repo-dir", wt, "--change-id", "463", + "--task-id", "task-4", "--phase", "build", "--branch", "fix/x", "--worktree", wt, "--run-epoch", bogus} + + out, _, _ := runCLI(t, append([]string{"--json"}, args...)...) + doc := decodeOneJSON(t, out) + if doc["result"] != "invalid-input" || doc["reason"] != "unknown-run-epoch" { + t.Fatalf("got %v, want invalid-input/unknown-run-epoch", doc) + } + if id, _ := doc["scope_id"].(string); id != "" { + t.Fatalf("a refused prepare-scope minted scope %q", id) + } + if strings.Contains(out, bogus) { + t.Fatalf("JSON output leaked the presented value: %s", out) + } + + // A non-applied result may render on either stream; check both together. + hOut, hErr, _ := runCLI(t, args...) + human := hOut + hErr + if !strings.Contains(human, "unknown-run-epoch") || !strings.Contains(human, "--gate-context") || strings.Contains(human, bogus) { + t.Fatalf("human output must name reason + remedy and never the value, got %q", human) + } +} diff --git a/internal/cli/run.go b/internal/cli/run.go index 67b7dc274..b9c50fe2a 100644 --- a/internal/cli/run.go +++ b/internal/cli/run.go @@ -66,14 +66,14 @@ func newRunCommand(setResult func(app.OperationResult)) *cobra.Command { _ = verify.MarkFlagRequired("id") // gate-before arms the implement-next run gate: it re-syncs, records the - // before-set + dispatch epoch in a durable record, and prints `gate-armed - // ` (or `gate-unarmed `). The sole positional argument is the - // gate target; only `implement-next` is accepted, and any other value is an - // invalid-input result (non-zero exit) the app layer owns. It reuses the same - // read-only planning seams as verify. + // before-set + dispatch epoch in a durable record, and prints `gate-armed + // ` (or `gate-unarmed `). The sole positional + // argument is the gate target; only `implement-next` is accepted, and any other + // value is an invalid-input result (non-zero exit) the app layer owns. It + // reuses the same read-only planning seams as verify. gateBefore := &cobra.Command{ Use: "gate-before ", - Short: "Arm the run gate for a dispatched workflow and print gate-armed ", + Short: "Arm the run gate for a dispatched workflow and print gate-armed ", Args: cobra.ExactArgs(1), // local-write: mints the durable rungate record AND the outer recovery-scope // record under the Git common dir; the re-sync is a read-only fetch. diff --git a/skills/docket-implement-next/references/edge-paths.md b/skills/docket-implement-next/references/edge-paths.md index eb3d4b528..a83849833 100644 --- a/skills/docket-implement-next/references/edge-paths.md +++ b/skills/docket-implement-next/references/edge-paths.md @@ -57,11 +57,11 @@ this resume path with its marker gone. worktree carries at most one live run. When the caller arms the resume (`run.gate-before … --resume `), the arm refuses to open a second run over one that has not verifiably stopped: -- Prior epoch still **active** → refused `resume-active-run`, with a locator naming the change, - epoch, and gate key and the remedy: cancel the prior run via the `run.cancel` operation (`--key - --epoch --reason `) and resume after confirmed cancellation, or continue the live - run via `run.gate-verdict`. **Never** force a fresh claim over a possibly-live run — that is the - claim-theft the gate exists to prevent. +- Prior epoch still **active** (an undispatched earlier resume arm counts) → refused + `resume-active-run`, naming the change, epoch, and gate key, with the remedy: cancel the prior run + via the `run.cancel` operation (`--key --epoch --reason `) and resume after + confirmed cancellation, or continue the live run via `run.gate-verdict`. **Never** force a fresh + claim over a possibly-live run — that is claim theft. - Cancellation still finishing → refused `cancellation-pending`; the resume observes that cleanup only. Finish the cancel first, then resume. - Prior epoch confirmed-cancelled and superseded → the arm reserves **exactly one** replacement