From c1b23011a8335f3076213d90e1bc9365d44b7231 Mon Sep 17 00:00:00 2001 From: Daniel Hanold Date: Wed, 7 Oct 2026 21:40:01 -0400 Subject: [PATCH 1/7] Add design spec: A finished run left active in run.json blocks set-visibility and cannot be cancelled --- ...n-run-json-blocks-set-visibility-design.md | 83 +++++++++++++++++++ 1 file changed, 83 insertions(+) create mode 100644 docs/superpowers/specs/2026-10-08-a-finished-run-left-active-in-run-json-blocks-set-visibility-design.md diff --git a/docs/superpowers/specs/2026-10-08-a-finished-run-left-active-in-run-json-blocks-set-visibility-design.md b/docs/superpowers/specs/2026-10-08-a-finished-run-left-active-in-run-json-blocks-set-visibility-design.md new file mode 100644 index 000000000..33f80fa7f --- /dev/null +++ b/docs/superpowers/specs/2026-10-08-a-finished-run-left-active-in-run-json-blocks-set-visibility-design.md @@ -0,0 +1,83 @@ +# A keyed run-done verdict retires its run: design + +Change 0540 — A finished run left active in run.json blocks set-visibility and cannot be cancelled + +Change #540, groomed interactively on 2026-10-07. Related: #541 (the child claims without `--run-context`, the most common cause of the symptom fixed here), #345 (the command-launched attribution gap), #532 (`set-visibility`, whose live-run scan surfaced it). ADRs: 0124 (successful-run closeout), 0128 (run.cancel ownership proofs). + +## Summary + +Every dispatched implement-next run leaves two files under `/run-tracker//`: `record.json` (the run tracker's verdict and retry accounting) and `run.json` (the run's lifecycle state, which decides whether the run is live). Today only a keyed `run-done … run-complete` verdict moves `run.json` out of `active`. Every other terminal verdict marks `record.json` `terminal: true` and leaves `run.json` `active`. For a run that claimed nothing, that state can never be cleared: `docket run cancel` refuses it (`claim-unconfirmed`), and `docket repository set-visibility` refuses while it is live and names `run cancel` as the remedy. + +This change makes the keyed verdict retire the run on **every** `run-done` outcome (`run-complete`, `no-attributable-claim`, `run-unclaimed`) through the existing observation-only closeout, so `run.json` ends `completed`. It also makes the live-run scan name a remedy that will work: `docket run verdict ` when `run cancel` would refuse. + +## Evidence gathered at grooming + +- **Only run-complete reaches the closeout.** In `RunVerdict` (`internal/app/runtracker_verdict.go`), `case VerdictRunComplete` calls `runTrackerCompleteRun`, which drives `completeSuccessfulRun` (`runtracker_complete.go`): success fence `active→completing`, settle retry-proven publications, stop-free accounting of participants, the launch census by the run's context hash and the mutation journal, then CAS `completing→completed`. The other outcomes call `persistRunVerdict` or `runTrackerOwnershipDone`, which write only `record.json` (`Disposition`, `Terminal`, a best-effort mirror). +- **`no-attributable-claim` is decided before `RunVerify`.** `resolveRunTrackerOwnership` returns `runTrackerOwnershipDone` when (a) an unconfirmed reservation has no committed receipt, or (b) there is no binding file and zero committed claim proofs match the record's `ChildContextHash`. +- **Cancel refuses exactly these runs, on purpose.** `runCancel` (`runtracker_cancel.go`) accepts a confirmed claim binding or the resume-verified shape (`RunTrackerRecord.resumeAttributed`); anything else is `claim-unconfirmed`. ADR-0128 Decision 1 keeps "an unconfirmed reservation still refuses claim-unconfirmed". +- **The repository-wide scan reads only `run.json`.** `liveRunsUnder` (`runtracker_live.go`) treats `active`, `completing`, `cancelling`, and unknown states as live and hard-codes `runCancelCommand(key)` as the remedy for `active`/`completing`. Its only caller is `visibilityLiveRunLines` in `repository_set_visibility_probe.go`. +- **Resume depends on halted runs staying `active`.** `run start --resume` (`runtracker_start.go`) answers a prior `RunCompleted` run with "there is nothing to resume" and supersedes only a `RunCancelled` one, so the halt → cancel → resume flow needs a halted run to remain `active` until it is cancelled. +- **It is not rare.** At grooming the development machine held three `active` runs with no claim binding and no participants: two `run-done … no-attributable-claim` runs (`implement-next-20261004t222959z-96475-7c95`, started 2026-10-04 22:29Z, and `implement-next-20261007t104957z-97517-71ff`, started 2026-10-07 10:49Z) and one `gate-armed` run that never got a verdict (`implement-next-20260929t153306z-56752-95e7`, started 2026-09-29). A private test repository had a fourth, hand-edited to `completed`. Two of these runs did claim and build a change but claimed without `--run-context` (#541), so the attribution failure is common, and every one of them strands an `active` `run.json` today. + +## Design + +### 1. The keyed verdict retires every run-done run + +`RunVerdict` routes every keyed `run-done` outcome through the closeout that `run-complete` already uses: + +| Verdict | Today | After | +|---|---|---| +| `run-done run-complete ` | closeout → `completed` | unchanged | +| `run-done no-attributable-claim` | `record.json` only; `run.json` stays `active` | closeout → `completed` | +| `run-done run-unclaimed ` | `record.json` only; `run.json` stays `active` | closeout → `completed` | +| `run-stop … run-halted`, `run-stop … run-incomplete`, every `run-stop … run-tracker-unavailable` ownership/load failure | `run.json` stays `active` | unchanged | +| `run-retry-once`, `run-continue` | nonterminal | unchanged | + +- Generalize `runTrackerCompleteRun` to carry the outcome token (and the attributed id, `0` for `no-attributable-claim`) instead of hard-coding `VerdictRunComplete`. `runTrackerOwnershipDone` and the `VerdictRunUnclaimed` case call it rather than `persistRunVerdict`. +- **Report lines are unchanged on success.** A successful closeout reports exactly today's line (`run-done no-attributable-claim`, `run-done run-unclaimed `), with `CompletionFindings` as for `run-complete`. +- **A blocked closeout fails closed through the existing channel.** If accounting finds a live, pending, or unreadable obligation, the verdict reports `run-stop run-tracker-unavailable completion-unaccounted` (or the engine's other bounded reason tokens) and the run stays `completing`. Repeating the same keyed verdict once the obligation settles replays the closeout to `completed`. No retry is consumed. `run-stop` and `run-done` both forbid re-dispatch, so the parent's obligations do not change. +- **Never relabel.** A run already `cancelling`, `cancelled`, or `superseded` is never marked `completed`. The closeout's existing refusal applies (`run-cancelled` / `run-superseded` reason tokens). +- **No run beside the record** (keyless or legacy): keep today's behavior — mirror the report line, write nothing else. +- **Unattributed observe mode** (`run verdict --unattributed`) stays read-only and never reaches the closeout. +- No new run state, no schema change, no new field. `completed` means "retired by its keyed verdict with full accounting". Whether the implementation succeeded remains `RunVerify`'s verdict, which the report line carries. + +### 2. The live-run remedy names a command that works + +`liveRunsUnder` chooses each run's remedy with the same ownership predicate `runCancel` uses (confirmed claim binding, or `resumeAttributed`), read from the run's own key directory. Extract that predicate so the two callers cannot drift. + +| `run.json` state | cancel would accept | cancel would refuse | +|---|---|---| +| `active`, `completing` | `docket run cancel --key --reason ` (today) | `docket run verdict ` | +| `cancelling` | re-run the same cancel until it reports cancelled (today) | inspect or remove `` by hand | +| unknown or unreadable | by hand (today) | by hand (today) | + +The `cancelling` + refuse cell covers a run that the death guardian fenced but that has no claim to cancel under. No documented command settles that state, so the honest remedy is the by-hand one. + +### 3. Decision record + +At build, record a new ADR (via `docket-adr`) that extends ADR-0124 Rule 1 from "only a verified `run-complete` drives the successful-run closeout" to "every keyed `run-done` verdict drives the closeout". It relates to ADR-0124 and ADR-0128. It supersedes neither: Rules 2–5 of ADR-0124 and ADR-0128's cancel proofs are unchanged. + +### 4. Existing stuck runs + +No migration. After install, running `docket run verdict ` on a stranded run re-resolves ownership, gets `no-attributable-claim`, and closes it out. The results file tells the human, as a human action, how to find stranded runs (a `run.json` still `active` beside a `record.json` with `terminal: true`) and to run the keyed verdict on each. It also names the three runs found at grooming (`implement-next-20260929t153306z-56752-95e7`, `implement-next-20261004t222959z-96475-7c95`, `implement-next-20261007t104957z-97517-71ff`) as known cases; re-check they are still `active` first. + +## Tests + +Each assert must go red when the code it guards is removed (mutation-test it). + +1. **The incident.** Fixture: `record.json` with a child context hash and no binding file, zero matching claim proofs, `run.json` `active`, no participants. Keyed verdict → `run-done no-attributable-claim`, `run.json` `completed`, and `liveRunsUnder` returns nothing (so `set-visibility` is no longer refused for it). +2. **Unconfirmed-reservation variant.** Reservation present, no committed receipt → same outcome. +3. **run-unclaimed.** Confirmed binding, change back to `proposed` → `run-done run-unclaimed `, `run.json` `completed`. +4. **Blocked closeout.** A live participant or launch under the run's context hash → `run-stop run-tracker-unavailable completion-unaccounted`, `run.json` `completing`. Settle it, repeat the verdict → `run-done … no-attributable-claim`, `completed`. +5. **Halted stays active.** A `run-halted` verdict leaves `run.json` `active` and resume still works after cancel. +6. **Never relabel.** A `cancelling` run with no claim → the verdict does not mark it `completed`. +7. **Remedy text.** Live-run lines for an `active` run: with a confirmed binding → the cancel command; without → `docket run verdict `; `cancelling` without a claim → the by-hand remedy. +8. **Observe mode.** `run verdict --unattributed` on the incident fixture leaves `run.json` `active`. + +## Out of scope + +- Why attribution fails: the child claiming without `--run-context` (#541) and command-launched dispatch (#345). +- `run cancel`'s ownership proofs; ADR-0128 stays as is. +- Halted and stop verdicts, which keep `run.json` `active`. +- Any new run state, run-tracker schema change, or change to the lock and generation scheme. +- Changing the resume-active-run remedy text in `run start --resume`. From 9abcf1409a8c9b2fe680648d1d9226f55455d166 Mon Sep 17 00:00:00 2001 From: Daniel Hanold Date: Wed, 7 Oct 2026 21:51:16 -0400 Subject: [PATCH 2/7] refactor(run-tracker): extract run cancel's ownership proof into runCancelOwner --- internal/app/runtracker_cancel.go | 73 ++++++++++++-------- internal/app/runtracker_cancel_owner_test.go | 46 ++++++++++++ 2 files changed, 90 insertions(+), 29 deletions(-) create mode 100644 internal/app/runtracker_cancel_owner_test.go diff --git a/internal/app/runtracker_cancel.go b/internal/app/runtracker_cancel.go index f1c5d29cc..bfaaeea89 100644 --- a/internal/app/runtracker_cancel.go +++ b/internal/app/runtracker_cancel.go @@ -335,35 +335,12 @@ func runCancel(seams cancelSeams, repoDir, key, reason string) RunCancelResult { return cancelRefused(cancelRunReason(err)) } - // (2) Validate the remaining authority conditions: the record must carry a - // parent-held authority; a CONFIRMED claim binding for the run's change must - // exist, or the resume-verified proof (ADR-0128 Decision 1). - if rec.ParentCap == "" { - return cancelRefused("authority-unavailable") - } - binding, ok, berr := LoadRunTrackerClaimBinding(repoDir, key) - if berr != nil { - return cancelRefused("claim-unreadable") - } - ownerID := 0 - switch { - case ok && binding.Confirmed: - ownerID = binding.ChangeID - case !ok && rec.resumeAttributed(): - // Resume-verified authority (change 0463): `run start --resume` pre-binds - // AttributedID through WorkspaceInspect identity and never gets a claim binding - // (change.claim requires a proposed change). It is the same shape - // resolveRunTrackerOwnership accepts as ownership. Without it, the run a resume start - // 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(ownerID) != ep.ChangeID { - return cancelRefused("claim-mismatch") + // (2) Validate the remaining authority conditions through the shared ownership + // proof (runCancelOwner): a parent-held authority, and a CONFIRMED claim binding + // for the run's change or the resume-verified proof (ADR-0128 Decision 1). + binding, hasBinding, berr := LoadRunTrackerClaimBinding(repoDir, key) + if _, refusal := runCancelOwner(rec, binding, hasBinding, berr, ep.ChangeID); refusal != "" { + return cancelRefused(refusal) } // (3) State gate + the durable fence. A terminal run is already-cancelled; an @@ -438,6 +415,44 @@ func runCancel(seams cancelSeams, repoDir, key, reason string) RunCancelResult { return cancelResult(CancelDispositionCancelled, findings) } +// runCancelOwner is run.cancel's ownership proof (ADR-0128 Decision 1) as one +// pure decision: the record must carry a parent-held authority, the claim binding +// must be readable, and the run must be owned by a CONFIRMED claim binding or by +// the resume-verified shape (no binding file at all and rec.resumeAttributed()). +// When the run already names its change, the owner must be that change. It returns +// the owning change id, or the exact refusal token runCancel reports. runCancel +// and the repository-wide live-run scan (liveRunCancelAuthority) both call it, so +// a printed `docket run cancel` remedy names cancel only when cancel would accept +// (change 0540). +func runCancelOwner(rec RunTrackerRecord, binding RunTrackerClaimBinding, hasBinding bool, bindingErr error, runChangeID string) (ownerID int, refusal string) { + if rec.ParentCap == "" { + return 0, "authority-unavailable" + } + if bindingErr != nil { + return 0, "claim-unreadable" + } + switch { + case hasBinding && binding.Confirmed: + ownerID = binding.ChangeID + case !hasBinding && rec.resumeAttributed(): + // Resume-verified authority (change 0463): `run start --resume` pre-binds + // AttributedID through WorkspaceInspect identity and never gets a claim binding + // (change.claim requires a proposed change). It is the same shape + // resolveRunTrackerOwnership accepts as ownership. Without it, the run a resume + // start 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. + ownerID = rec.AttributedID + default: + return 0, "claim-unconfirmed" + } + if runChangeID != "" && strconv.Itoa(ownerID) != runChangeID { + return 0, "claim-mismatch" + } + return ownerID, "" +} + // verifyTerminalRunQuiescence revalidates a terminal (cancelled/superseded) // run's EXISTING launch and mutation evidence using the same bounded accounting // cancellation uses — the launch census plus the admitted-mutation journal diff --git a/internal/app/runtracker_cancel_owner_test.go b/internal/app/runtracker_cancel_owner_test.go new file mode 100644 index 000000000..b07863359 --- /dev/null +++ b/internal/app/runtracker_cancel_owner_test.go @@ -0,0 +1,46 @@ +package app + +import ( + "errors" + "testing" +) + +// TestRunCancelOwnerMatchesCancelProofs pins run.cancel's ownership proof +// (ADR-0128 Decision 1) as one pure decision, shared by runCancel and the +// live-run scan (liveRunCancelAuthority) so the two cannot drift (change 0540). +func TestRunCancelOwnerMatchesCancelProofs(t *testing.T) { + confirmed := RunTrackerClaimBinding{Schema: bindingSchemaVersion, ChangeID: 7, RequestID: "r", Confirmed: true} + reserved := RunTrackerClaimBinding{Schema: bindingSchemaVersion, ChangeID: 7, RequestID: "r"} + withCap := RunTrackerRecord{ParentCap: "cap"} + resume := RunTrackerRecord{ParentCap: "cap", AttributedID: 7} + none := RunTrackerClaimBinding{} + cases := []struct { + name string + rec RunTrackerRecord + binding RunTrackerClaimBinding + has bool + berr error + runChange string + wantID int + wantRefusal string + }{ + {"confirmed binding", withCap, confirmed, true, nil, "7", 7, ""}, + {"confirmed binding, run not yet bound", withCap, confirmed, true, nil, "", 7, ""}, + {"resume-verified record", resume, none, false, nil, "7", 7, ""}, + {"no parent capability", RunTrackerRecord{}, confirmed, true, nil, "7", 0, "authority-unavailable"}, + {"capability checked before binding", RunTrackerRecord{}, none, false, errors.New("corrupt"), "7", 0, "authority-unavailable"}, + {"unreadable binding", withCap, none, false, errors.New("corrupt"), "7", 0, "claim-unreadable"}, + {"unconfirmed reservation", withCap, reserved, true, nil, "7", 0, "claim-unconfirmed"}, + {"reservation beside a resume shape", resume, reserved, true, nil, "7", 0, "claim-unconfirmed"}, + {"no binding and no claim", withCap, none, false, nil, "", 0, "claim-unconfirmed"}, + {"owner differs from the run", withCap, confirmed, true, nil, "8", 0, "claim-mismatch"}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + id, refusal := runCancelOwner(c.rec, c.binding, c.has, c.berr, c.runChange) + if id != c.wantID || refusal != c.wantRefusal { + t.Fatalf("runCancelOwner = (%d, %q), want (%d, %q)", id, refusal, c.wantID, c.wantRefusal) + } + }) + } +} From 87cd33593d85c8b92611171f56ccf9e8a72d0f9f Mon Sep 17 00:00:00 2001 From: Daniel Hanold Date: Wed, 7 Oct 2026 21:57:40 -0400 Subject: [PATCH 3/7] fix(run-tracker): name a live run's remedy from run cancel's own ownership proof --- ...ository_set_visibility_integration_test.go | 14 +- internal/app/runtracker_live.go | 68 +++++++++- internal/app/runtracker_live_test.go | 124 ++++++++++++++++++ internal/app/runtracker_store.go | 52 +++++--- 4 files changed, 235 insertions(+), 23 deletions(-) diff --git a/internal/app/repository_set_visibility_integration_test.go b/internal/app/repository_set_visibility_integration_test.go index b9e054446..67337e957 100644 --- a/internal/app/repository_set_visibility_integration_test.go +++ b/internal/app/repository_set_visibility_integration_test.go @@ -134,16 +134,26 @@ func TestIntegrationRepoVisibilitySwitchPreviewWritesNothing(t *testing.T) { } // TestIntegrationRepoVisibilitySwitchRefusesLiveRun proves an active run refuses the -// switch, naming the cancel command for its key. +// switch, naming the cancel command for its key. The run carries the authority run +// cancel accepts (a parent capability and a confirmed claim on its change), since +// the live-run scan names cancel only for a run cancel would act on. func TestIntegrationRepoVisibilitySwitchRefusesLiveRun(t *testing.T) { r := newSharedVisibilityRepo(t) - key, err := MintRunTrackerRecord(r.invocation, sampleRunTrackerRecord()) + rec := sampleRunTrackerRecord() + rec.ParentCap = "parent-cap-raw" + key, err := MintRunTrackerRecord(r.invocation, rec) if err != nil { t.Fatalf("MintRunTrackerRecord: %v", err) } if _, err := MintRunRecord(r.invocation, key, "375"); err != nil { t.Fatalf("MintRunRecord: %v", err) } + if err := ReserveRunTrackerClaim(r.invocation, key, 375, "req-1"); err != nil { + t.Fatalf("ReserveRunTrackerClaim: %v", err) + } + if err := ConfirmRunTrackerClaim(r.invocation, key, 375, "req-1", "rev-1", ""); err != nil { + t.Fatalf("ConfirmRunTrackerClaim: %v", err) + } res := r.runSetVisibility(t, SetVisibilityOptions{Target: "private"}) requireVisibilityRefusal(t, res, "docket run cancel --key "+key) } diff --git a/internal/app/runtracker_live.go b/internal/app/runtracker_live.go index e63aa381c..e19c5eb66 100644 --- a/internal/app/runtracker_live.go +++ b/internal/app/runtracker_live.go @@ -27,6 +27,11 @@ type liveRunLocator struct{ Key, State, ChangeID, Remedy string } // State liveRunStateUnreadable. A key directory without run.json holds no run and // is skipped, as is any non-directory entry. A missing root is (nil, nil); any // other failure to list the root is an error. +// +// Each locator's Remedy follows liveRunCancelAuthority, so it names a command that +// will act on the run in its current state: run cancel only where runCancelOwner +// accepts, the keyed run verdict for a readable run cancel would refuse, and the +// by-hand remedy where neither command can load the run. func liveRunsUnder(stateDir string) ([]liveRunLocator, error) { root := filepath.Join(stateDir, runTrackerDirName) entries, err := os.ReadDir(root) @@ -58,9 +63,24 @@ func liveRunsUnder(stateDir string) ([]liveRunLocator, error) { case RunCompleted, RunCancelled, RunSuperseded: continue case RunActive, RunCompleting: - loc.Remedy = runCancelCommand(key) + switch liveRunCancelAuthority(dir, rec.ChangeID) { + case liveRunCancelAccepts: + loc.Remedy = runCancelCommand(key) + case liveRunCancelRefuses: + // Cancel refuses a run it cannot prove owned (ADR-0128); the keyed + // verdict re-resolves ownership and retires a run that claimed nothing + // (change 0540). + loc.Remedy = runVerdictCommand(key) + default: + loc.Remedy = byHandRemedy(dir) + } case RunCancelling: - loc.Remedy = "re-run the same " + runCancelCommand(key) + " until it reports cancelled" + if liveRunCancelAuthority(dir, rec.ChangeID) == liveRunCancelAccepts { + loc.Remedy = "re-run the same " + runCancelCommand(key) + " until it reports cancelled" + } else { + // Fenced cancelling with no claim to cancel under: no command settles it. + loc.Remedy = byHandRemedy(dir) + } default: // run cancel refuses a state it does not know, so only a person can settle it. loc.Remedy = byHandRemedy(dir) @@ -77,6 +97,50 @@ func runCancelCommand(key string) string { return "docket run cancel --key " + key + " --reason " } +// runVerdictCommand is the ready-to-run keyed verdict for key: for a run cancel +// would refuse, it re-resolves ownership and, on any run-done, retires the run. +func runVerdictCommand(key string) string { + return "docket run verdict " + key +} + +// liveRunAuthority is what run cancel would decide about one live run, read from +// its own key directory. +type liveRunAuthority int + +const ( + // liveRunAuthorityUnknown: record.json is missing or unreadable, or the claim + // binding is unreadable. Neither run cancel nor run verdict can act on the run. + liveRunAuthorityUnknown liveRunAuthority = iota + // liveRunCancelAccepts: runCancelOwner accepts — run cancel would act. + liveRunCancelAccepts + // liveRunCancelRefuses: the run is readable but runCancelOwner refuses. + liveRunCancelRefuses +) + +// liveRunCancelAuthority asks runCancelOwner — the exact ownership proof runCancel +// applies — about the run in dir, whose run.json names runChangeID. It reads the +// files only and starts no git. +func liveRunCancelAuthority(dir, runChangeID string) liveRunAuthority { + buf, err := os.ReadFile(filepath.Join(dir, runTrackerRecordFileName)) + if err != nil { + return liveRunAuthorityUnknown + } + rec, err := decodeRunTrackerRecord(buf, "live-runs") + if err != nil { + return liveRunAuthorityUnknown + } + binding, hasBinding, berr := readRunTrackerClaimBinding(dir, "live-runs") + if berr != nil { + // Cancel refuses claim-unreadable and the verdict stops binding-unreadable + // without retiring the run: neither command settles it. + return liveRunAuthorityUnknown + } + if _, refusal := runCancelOwner(rec, binding, hasBinding, nil, runChangeID); refusal != "" { + return liveRunCancelRefuses + } + return liveRunCancelAccepts +} + func byHandRemedy(dir string) string { return "inspect or remove " + dir + " by hand" } diff --git a/internal/app/runtracker_live_test.go b/internal/app/runtracker_live_test.go index 0fc4efe61..797ea0ad2 100644 --- a/internal/app/runtracker_live_test.go +++ b/internal/app/runtracker_live_test.go @@ -36,6 +36,49 @@ func writeRunFile(t *testing.T, stateDir, key string, buf []byte) string { return dir } +// writeTrackerRecordFixture writes a schema-current record.json into key's +// directory under stateDir (no git: Repo is left empty, which only the +// repository-identity check reads). +func writeTrackerRecordFixture(t *testing.T, stateDir, key string, rec RunTrackerRecord) { + t.Helper() + rec.Schema = runTrackerSchemaVersion + buf, err := json.Marshal(rec) + if err != nil { + t.Fatal(err) + } + writeKeyFile(t, stateDir, key, runTrackerRecordFileName, buf) +} + +// writeBindingFixture writes claim-binding.json into key's directory. +func writeBindingFixture(t *testing.T, stateDir, key string, b RunTrackerClaimBinding) { + t.Helper() + b.Schema = bindingSchemaVersion + buf, err := json.Marshal(b) + if err != nil { + t.Fatal(err) + } + writeKeyFile(t, stateDir, key, runTrackerClaimBindingName, buf) +} + +func writeKeyFile(t *testing.T, stateDir, key, name string, buf []byte) { + t.Helper() + dir := filepath.Join(stateDir, runTrackerDirName, key) + if err := os.MkdirAll(dir, 0o700); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, name), buf, 0o600); err != nil { + t.Fatal(err) + } +} + +// cancellableRun gives key the authority run cancel accepts: a parent +// capability and a confirmed binding for change 7 (writeRunFixture's change). +func cancellableRun(t *testing.T, stateDir, key string) { + t.Helper() + writeTrackerRecordFixture(t, stateDir, key, RunTrackerRecord{ParentCap: "cap", AttemptLimit: 2}) + writeBindingFixture(t, stateDir, key, RunTrackerClaimBinding{ChangeID: 7, RequestID: "r", Confirmed: true}) +} + func TestLiveRunsUnderReportsLiveAndUnreadable(t *testing.T) { stateDir := testsupport.TempDir(t) writeRunFixture(t, stateDir, "k-active", RunActive) @@ -44,6 +87,9 @@ func TestLiveRunsUnderReportsLiveAndUnreadable(t *testing.T) { writeRunFixture(t, stateDir, "k-completed", RunCompleted) writeRunFixture(t, stateDir, "k-cancelled", RunCancelled) writeRunFixture(t, stateDir, "k-superseded", RunSuperseded) + cancellableRun(t, stateDir, "k-active") + cancellableRun(t, stateDir, "k-completing") + cancellableRun(t, stateDir, "k-cancelling") badDir := writeRunFile(t, stateDir, "k-broken", []byte("{not json")) // A key directory without run.json (a run-tracker record only) is no run. if err := os.MkdirAll(filepath.Join(stateDir, runTrackerDirName, "k-norun"), 0o700); err != nil { @@ -88,3 +134,81 @@ func TestLiveRunsUnderMissingRoot(t *testing.T) { t.Fatalf("missing root = %#v, %v; want nil, nil", got, err) } } + +// TestLiveRunsUnderRemedyFollowsCancelAuthority: each live run's remedy is a +// command that will act on it in its current state (change 0540). Cancel is named +// only when runCancelOwner accepts; a readable run cancel would refuse gets the +// keyed verdict, which retires it; anything neither command can load is by hand. +func TestLiveRunsUnderRemedyFollowsCancelAuthority(t *testing.T) { + stateDir := testsupport.TempDir(t) + cancel := func(k string) string { return "docket run cancel --key " + k + " --reason " } + verdict := func(k string) string { return "docket run verdict " + k } + byHand := func(dir string) string { return "inspect or remove " + dir + " by hand" } + + // The incident: a record with no parent capability and no claim (no binding file). + writeRunFixture(t, stateDir, "a-noclaim", RunActive) + writeTrackerRecordFixture(t, stateDir, "a-noclaim", RunTrackerRecord{ChildContextHash: "h", AttemptLimit: 2, Terminal: true}) + + // A capability but only an unconfirmed reservation. + writeRunFixture(t, stateDir, "b-reserved", RunActive) + writeTrackerRecordFixture(t, stateDir, "b-reserved", RunTrackerRecord{ParentCap: "cap", AttemptLimit: 2}) + writeBindingFixture(t, stateDir, "b-reserved", RunTrackerClaimBinding{ChangeID: 7, RequestID: "r"}) + + // A confirmed binding but no parent capability: cancel refuses, the verdict does not need it. + writeRunFixture(t, stateDir, "c-nocap", RunCompleting) + writeTrackerRecordFixture(t, stateDir, "c-nocap", RunTrackerRecord{AttemptLimit: 2}) + writeBindingFixture(t, stateDir, "c-nocap", RunTrackerClaimBinding{ChangeID: 7, RequestID: "r", Confirmed: true}) + + // Cancellable: confirmed binding and capability. + writeRunFixture(t, stateDir, "d-owned", RunActive) + cancellableRun(t, stateDir, "d-owned") + + // No record.json at all: neither cancel nor verdict can load the run. + eDir := writeRunFixture(t, stateDir, "e-norecord", RunActive) + + // Unreadable record.json. + fDir := writeRunFixture(t, stateDir, "f-badrecord", RunActive) + writeKeyFile(t, stateDir, "f-badrecord", runTrackerRecordFileName, []byte("{not json")) + + // Corrupt binding: cancel refuses claim-unreadable, verdict stops binding-unreadable. + gDir := writeRunFixture(t, stateDir, "g-badbinding", RunActive) + writeTrackerRecordFixture(t, stateDir, "g-badbinding", RunTrackerRecord{ParentCap: "cap", AttemptLimit: 2}) + writeKeyFile(t, stateDir, "g-badbinding", runTrackerClaimBindingName, []byte("{not json")) + + // Cancelling with no claim (the death guardian fenced it): no command settles it. + hDir := writeRunFixture(t, stateDir, "h-cancelling-noclaim", RunCancelling) + writeTrackerRecordFixture(t, stateDir, "h-cancelling-noclaim", RunTrackerRecord{ParentCap: "cap", AttemptLimit: 2}) + + // Cancelling and cancellable: re-run the cancel. + writeRunFixture(t, stateDir, "i-cancelling-owned", RunCancelling) + cancellableRun(t, stateDir, "i-cancelling-owned") + + // Owner differs from the run's change: cancel refuses claim-mismatch; the verdict still acts. + writeRunFixture(t, stateDir, "j-mismatch", RunActive) + writeTrackerRecordFixture(t, stateDir, "j-mismatch", RunTrackerRecord{ParentCap: "cap", AttemptLimit: 2}) + writeBindingFixture(t, stateDir, "j-mismatch", RunTrackerClaimBinding{ChangeID: 8, RequestID: "r", Confirmed: true}) + + got, err := liveRunsUnder(stateDir) + if err != nil { + t.Fatal(err) + } + remedies := map[string]string{} + for _, l := range got { + remedies[l.Key] = l.Remedy + } + want := map[string]string{ + "a-noclaim": verdict("a-noclaim"), + "b-reserved": verdict("b-reserved"), + "c-nocap": verdict("c-nocap"), + "d-owned": cancel("d-owned"), + "e-norecord": byHand(eDir), + "f-badrecord": byHand(fDir), + "g-badbinding": byHand(gDir), + "h-cancelling-noclaim": byHand(hDir), + "i-cancelling-owned": "re-run the same " + cancel("i-cancelling-owned") + " until it reports cancelled", + "j-mismatch": verdict("j-mismatch"), + } + if !reflect.DeepEqual(remedies, want) { + t.Fatalf("remedies =\n%#v\nwant\n%#v", remedies, want) + } +} diff --git a/internal/app/runtracker_store.go b/internal/app/runtracker_store.go index c968010e3..699723eb1 100644 --- a/internal/app/runtracker_store.go +++ b/internal/app/runtracker_store.go @@ -427,6 +427,36 @@ func MintRunTrackerRecord(repoDir string, rec RunTrackerRecord) (string, error) return "", runTrackerErr(ErrRunTrackerIO, "mint", errors.New("could not mint a unique key")) } +// decodeRunTrackerRecord parses record.json bytes and applies every read-boundary +// check LoadRunTrackerRecord makes EXCEPT the repository-identity check (Repo vs the +// canonical common dir): an unparseable record, an unknown schema, a partial +// continuation triple, or a partial claim-binding mirror pair is a corrupt record. +// LoadRunTrackerRecord adds the identity check; the repository-wide live-run scan +// (liveRunCancelAuthority) reads a state folder that already selects the repository. +func decodeRunTrackerRecord(buf []byte, op string) (RunTrackerRecord, error) { + var rec RunTrackerRecord + if err := json.Unmarshal(buf, &rec); err != nil { + return RunTrackerRecord{}, runTrackerErr(ErrRunTrackerCorruptRecord, op, err) + } + if rec.Schema != runTrackerSchemaVersion { + return RunTrackerRecord{}, runTrackerErr(ErrRunTrackerCorruptRecord, op, + fmt.Errorf("schema version %d, want %d", rec.Schema, runTrackerSchemaVersion)) + } + // A partial continuation triple is a corrupt record: fail closed on read so a + // half-written continuation is never handed to the verdict path. + if !runTrackerContinuationTripleOK(rec) { + return RunTrackerRecord{}, runTrackerErr(ErrRunTrackerCorruptRecord, op, + errors.New("partial continuation triple")) + } + // A partial claim-binding mirror pair is a corrupt record: fail closed on read + // so a half-written mirror is never handed to the verdict path. + if !runTrackerBoundPairOK(rec) { + return RunTrackerRecord{}, runTrackerErr(ErrRunTrackerCorruptRecord, op, + errors.New("partial claim-binding mirror pair")) + } + return rec, nil +} + // LoadRunTrackerRecord reads the record for key from the repository's run-tracker root. It // validates the key before any filesystem or git touch, refuses a record whose // Repo does not match the current canonical common dir (wrong-repo), fails closed @@ -448,29 +478,13 @@ func LoadRunTrackerRecord(repoDir, key string) (RunTrackerRecord, error) { } return RunTrackerRecord{}, runTrackerErr(ErrRunTrackerIO, "load", err) } - var rec RunTrackerRecord - if err := json.Unmarshal(buf, &rec); err != nil { - return RunTrackerRecord{}, runTrackerErr(ErrRunTrackerCorruptRecord, "load", err) - } - if rec.Schema != runTrackerSchemaVersion { - return RunTrackerRecord{}, runTrackerErr(ErrRunTrackerCorruptRecord, "load", - fmt.Errorf("schema version %d, want %d", rec.Schema, runTrackerSchemaVersion)) + rec, err := decodeRunTrackerRecord(buf, "load") + if err != nil { + return RunTrackerRecord{}, err } if rec.Repo != common { return RunTrackerRecord{}, runTrackerErr(ErrRunTrackerWrongRepo, "load", nil) } - // A partial continuation triple is a corrupt record: fail closed on read so a - // half-written continuation is never handed to the verdict path. - if !runTrackerContinuationTripleOK(rec) { - return RunTrackerRecord{}, runTrackerErr(ErrRunTrackerCorruptRecord, "load", - errors.New("partial continuation triple")) - } - // A partial claim-binding mirror pair is a corrupt record: fail closed on read - // so a half-written mirror is never handed to the verdict path. - if !runTrackerBoundPairOK(rec) { - return RunTrackerRecord{}, runTrackerErr(ErrRunTrackerCorruptRecord, "load", - errors.New("partial claim-binding mirror pair")) - } // The markers are authority; reflect them into the readable mirror on read so a // crash between an O_EXCL create and the JSON flip still reads as consumed. // Consumed == at least one retry marker exists (bare legacy name or any From 1d270d39b0647e9e732782cd09282fa3ff9741b3 Mon Sep 17 00:00:00 2001 From: Daniel Hanold Date: Wed, 7 Oct 2026 22:04:26 -0400 Subject: [PATCH 4/7] fix(run-tracker): retire the run on every keyed run-done verdict run-unclaimed and no-attributable-claim now drive the same ownership closeout as run-complete, so run.json leaves active instead of stranding the run for the live-run scan. runTrackerCompleteRun carries the outcome token so success lines are unchanged; its one direct test caller passes VerdictRunComplete. --- docs/concepts/run-tracker.md | 4 + internal/app/runtracker_complete.go | 23 +- .../runtracker_complete_integration_test.go | 2 +- internal/app/runtracker_verdict.go | 75 +++--- .../runtracker_verdict_integration_test.go | 253 ++++++++++++++++++ 5 files changed, 311 insertions(+), 46 deletions(-) diff --git a/docs/concepts/run-tracker.md b/docs/concepts/run-tracker.md index 92f47b553..2f652ea3e 100644 --- a/docs/concepts/run-tracker.md +++ b/docs/concepts/run-tracker.md @@ -70,6 +70,10 @@ verdict, not the worker's report, says what may happen next. `docket run verdict --unattributed [...]` checks the named changes (or every in-progress change) and prints `run-observe` lines, holding no key and writing nothing. + A keyed `run-done` line also retires the run, so it no longer counts as + live; if something the run started is still settling, the verdict reports + `run-stop … completion-unaccounted` instead, and repeating the same verdict + once it settles retires the run. - **Only two lines authorize another dispatch.** `run-retry-once` grants one more attempt for the change id and unmet work it names. `run-continue` is nonterminal: the same attempt still owns tracked work, diff --git a/internal/app/runtracker_complete.go b/internal/app/runtracker_complete.go index cb9327c08..da06c715c 100644 --- a/internal/app/runtracker_complete.go +++ b/internal/app/runtracker_complete.go @@ -3,8 +3,9 @@ // coordinator's authoritative Stop, this is the authoritative "the run finished // successfully — release its run ownership so later workflow mutations on its // worktree are no longer fenced to it". It is driven ONLY from the attributed, keyed RunVerdict -// path on a verified run-complete (Task 8 wires the caller); RunVerify stays -// read-only and unattributed observe verdicts never reach it. +// path, on every run-done verdict (run-complete since change 0441; run-unclaimed and +// no-attributable-claim since change 0540); RunVerify stays read-only and unattributed +// observe verdicts never reach it. // // STOPS NOTHING. Closeout stops nothing and signals nothing: it never invokes // native cancellation and never process.Stop. Its two writes are @@ -31,10 +32,11 @@ // // FAIL CLOSED. A live, busy, pending, or unreadable obligation blocks completion; a // publication journal entry blocks only while its publisher may still be running -// (change 0494), because RunVerify's live probes behind the verified run-complete -// are the evidence a publication landed: missing terminal evidence is UNPROVEN, never implicitly complete. A -// blocked closeout leaves the run durably completing (the success fence holds) and -// returns completion-unaccounted with the bounded findings that name what to settle; +// (change 0494), because the journal is never the evidence a publication landed (on +// a run-complete, RunVerify's live probes are; no other run-done claims one): +// missing terminal evidence is UNPROVEN, never implicitly complete. A blocked +// closeout leaves the run durably completing (the success fence holds) and returns +// completion-unaccounted with the bounded findings that name what to settle; // the remedy is to settle the named evidence and repeat the same keyed verdict, or to // cancel explicitly. No retry, budget, or attempt is consumed by a blocked closeout. // @@ -124,8 +126,8 @@ func (o appLaunchObserver) observe(contextHash string) (gatedrive.RunLaunchRepor // completion-unaccounted, completion-unpersisted, run-record-unreadable), and the bounded // credential-free findings that name every unsettled obligation; findings may also // carry informational mutation-settled: or mutation-abandoned: tokens, even on a successful closeout. -// The caller (Task 8) -// has already resolved the confirmed claim binding and the run-complete verdict; this +// The caller +// has already resolved ownership and a run-done verdict; this // function owns only the run's closeout. See the file header for the // stops-nothing, fail-closed, never-relabel, and lock-ordering contracts. func completeSuccessfulRun(seams cancelSeams, repoDir, runKey string) (ok bool, reason string, findings []string) { @@ -329,8 +331,9 @@ func durableExecutionProof(seams cancelSeams, handle string) bool { // (classifyAdmittedMutation, change 0494), mirroring cancellation. A publisher that // may still be running, or an unprovable entry, blocks with mutation-pending:. // An uncertain entry, or one whose publisher provably exited, is accounted with the -// informational mutation-abandoned:. RunVerify's live probes behind the verified -// run-complete, not the journal, are the evidence that a publication landed. +// informational mutation-abandoned:. The journal is never the evidence that a +// publication landed: on a run-complete RunVerify's live probes are, and no other +// run-done claims one. func accountCompletionMutations(repoDir, runKey string, ep RunRecord) (bool, []string) { return accountRunMutations(repoDir, runKey, ep.AdmittedMutations) } diff --git a/internal/app/runtracker_complete_integration_test.go b/internal/app/runtracker_complete_integration_test.go index 62d2d9981..b7f753009 100644 --- a/internal/app/runtracker_complete_integration_test.go +++ b/internal/app/runtracker_complete_integration_test.go @@ -757,7 +757,7 @@ func TestIntegrationRunCompletionTreeSurvivesNeverBlocksCloseout(t *testing.T) { t.Fatalf("LoadRunTrackerRecord: %v", err) } - res := runTrackerCompleteRun(fx.repo, fx.key, rec, 42, fx.seams()) + res := runTrackerCompleteRun(fx.repo, fx.key, rec, VerdictRunComplete, 42, fx.seams()) if res.Decision != RunDecisionDone || res.Outcome != VerdictRunComplete { t.Fatalf("verdict = %s %s (reason %q, findings %v), want run-done run-complete", res.Decision, res.Outcome, res.Reason, res.CompletionFindings) diff --git a/internal/app/runtracker_verdict.go b/internal/app/runtracker_verdict.go index 5c3591b5d..67ae10d31 100644 --- a/internal/app/runtracker_verdict.go +++ b/internal/app/runtracker_verdict.go @@ -28,11 +28,13 @@ import ( // never authorizes a retry. // // COMPLETION (spec §successful-completion-flow, change 0441). The verdict remains -// the sole authority MAPPER — it never re-derives RunVerify's run-complete. On a -// keyed run-complete it additionally drives the successful-run ownership closeout -// (runTrackerCompleteRun → completeSuccessfulRun) so a standalone finalize gate can admit -// on the same worktree without a run-superseded refusal or a human cancellation. A -// BLOCKED or lost closeout maps to `run-stop run-tracker-unavailable ` on the +// the sole authority MAPPER — it never re-derives RunVerify's run-complete. On +// every keyed run-done — run-complete, run-unclaimed, and no-attributable-claim +// (change 0540) — it additionally drives the run ownership closeout +// (runTrackerCompleteRun → completeSuccessfulRun) so the run stops counting as live and a +// standalone finalize gate can admit on the same worktree without a run-superseded +// refusal or a human cancellation. A BLOCKED or lost closeout on any of those +// run-done verdicts maps to `run-stop run-tracker-unavailable ` on the // existing run-tracker-unavailable channel with the new bounded reason tokens (run-cancelled // / run-superseded / completion-unaccounted / completion-unpersisted / // report-unpersisted / run-record-unreadable) and never reports success — RunVerify's own @@ -131,8 +133,8 @@ const ( ReasonRunProofUnavailable = "proof-unavailable" ) -// The successful-run closeout reason tokens (change 0441). Each rides the existing -// `run-stop run-tracker-unavailable ` channel when the keyed run-complete +// The run closeout reason tokens (change 0441). Each rides the existing +// `run-stop run-tracker-unavailable ` channel when a keyed run-done // verdict cannot report success: the report-line vocabulary is unchanged; only these // bounded reason spellings are new. The first four are RE-USED verbatim from Task 7's // completeSuccessfulRun return values (which flow straight through as r.Reason), so a @@ -156,7 +158,7 @@ const ( // completed, but the terminal gate REPORT mirror could not be saved. The failure is // REPORTED, not hidden by the best-effort save (spec: "completion-path persistence // failures must be reported"); the run is already completed, so a repeat of the - // same keyed verdict replays to run-done run-complete once the fault clears. + // same keyed verdict replays to the same run-done line once the fault clears. ReasonRunReportUnpersisted = "report-unpersisted" // ReasonRunRecordUnreadable: the run record beside the key could not be read // (a store fault, corruption, or schema mismatch — anything but a clean absence, @@ -304,16 +306,14 @@ func RunVerdict(ctx context.Context, deps PlanningDeps, wdeps WorkspaceDeps, gde v := RunVerify(ctx, deps, wdeps, gdeps, repoDir, RunVerifyRequest{ID: id}) switch v.Verdict { - case VerdictRunComplete: - // A verified run-complete additionally drives the successful-run ownership - // closeout (change 0441) so a standalone finalize gate can admit on the same - // worktree. The seam bundle is injectable (unit tests fake the observers); - // production composes productionCancelSeams(repoDir). RunVerify's verdict is - // still reported as fact — runTrackerCompleteRun never re-derives it. - return runTrackerCompleteRun(repoDir, key, rec, id, verdictSeams(repoDir, wdeps)) - case VerdictRunUnclaimed: - return persistRunVerdict(repoDir, key, rec, - runVerdictLine(key, RunDecisionDone, VerdictRunUnclaimed, id, true, nil)) + case VerdictRunComplete, VerdictRunUnclaimed: + // Every keyed run-done drives the run's ownership closeout (change 0441 for + // run-complete, extended to every run-done by change 0540) so run.json leaves + // active and the run stops counting as live. The seam bundle is injectable + // (unit tests fake the observers); production composes + // productionCancelSeams(repoDir). RunVerify's verdict is reported as fact — + // runTrackerCompleteRun never re-derives it. + return runTrackerCompleteRun(repoDir, key, rec, v.Verdict, id, verdictSeams(repoDir, wdeps)) case VerdictRunHalted: return persistRunVerdict(repoDir, key, rec, runVerdictLine(key, RunDecisionStop, VerdictRunHalted, id, true, nil)) @@ -403,14 +403,15 @@ func RunVerdict(ctx context.Context, deps PlanningDeps, wdeps WorkspaceDeps, gde } } -// runTrackerCompleteRun maps a verified keyed run-complete onto the successful-run -// ownership closeout (change 0441). The caller (RunVerdict) has already resolved -// the confirmed claim binding and delegated the run predicate to RunVerify; this owns -// only the ownership retirement. +// runTrackerCompleteRun maps a keyed run-done verdict — run-complete, run-unclaimed, +// or no-attributable-claim (outcome; id is 0 for no-attributable-claim) — onto the +// run's ownership closeout (changes 0441, 0540). The caller has already resolved +// ownership and, for run-complete and run-unclaimed, delegated the run predicate to +// RunVerify; this owns only the ownership retirement. // // 1. Locate the run beside the run-tracker record. A clean ABSENCE (ErrRunNotFound) // is the keyless/standalone/legacy shape: EXACTLY the prior behavior — best-effort -// report mirror + run-done run-complete. Any OTHER load fault fails closed +// report mirror + the same run-done line. Any OTHER load fault fails closed // (run-record-unreadable): a record the store cannot read is never a free closeout. // 2. Drive completeSuccessfulRun. A blocked/lost closeout maps to run-stop // run-tracker-unavailable with the engine's bounded reason token and the diagnostic @@ -419,14 +420,15 @@ func RunVerdict(ctx context.Context, deps PlanningDeps, wdeps WorkspaceDeps, gde // 3. On success the run is durably completed. The terminal report mirror is saved // as a CHECKED write — a persistence failure is REPORTED (report-unpersisted), // never hidden by the best-effort save; the run is already completed, so the -// remedy is the idempotent replay (a repeat of the same keyed verdict). -func runTrackerCompleteRun(repoDir, key string, rec RunTrackerRecord, id int, seams cancelSeams) RunVerdictResult { +// remedy is the idempotent replay (a repeat of the same keyed verdict), which +// replays to the same run-done line. +func runTrackerCompleteRun(repoDir, key string, rec RunTrackerRecord, outcome string, id int, seams cancelSeams) RunVerdictResult { // (1) Locate the run. Absence is the keyless/standalone/legacy shape; any // other fault fails closed. if _, _, lerr := LoadRunRecord(repoDir, key); lerr != nil { if ee, ok := AsRunError(lerr); ok && ee.Kind == ErrRunNotFound { return persistRunVerdict(repoDir, key, rec, - runVerdictLine(key, RunDecisionDone, VerdictRunComplete, id, true, nil)) + runVerdictLine(key, RunDecisionDone, outcome, id, true, nil)) } return persistRunVerdict(repoDir, key, rec, runVerdictLine(key, RunDecisionStop, RunOutcomeUnavailable, id, true, func(r *RunVerdictResult) { @@ -450,8 +452,8 @@ func runTrackerCompleteRun(repoDir, key string, rec RunTrackerRecord, id int, se // (3) Closeout succeeded; the run is durably completed. Save the terminal report // mirror as a CHECKED write — a completion-path persistence failure is reported, not // hidden. On failure the run stays completed, so the same keyed verdict replays to - // run-done run-complete once the fault clears. - res := runVerdictLine(key, RunDecisionDone, VerdictRunComplete, id, true, func(r *RunVerdictResult) { + // the same run-done line once the fault clears. + res := runVerdictLine(key, RunDecisionDone, outcome, id, true, func(r *RunVerdictResult) { r.CompletionFindings = findings }) rec.Disposition = res.HumanText() @@ -666,7 +668,7 @@ func resolveRunTrackerOwnership(ctx context.Context, deps PlanningDeps, wdeps Wo if !found { // The reserved claim never committed. Leave the reservation refused — never // released, never confirmed — and report no attributable claim. - return runTrackerOwnershipDone(repoDir, key, *rec) + return runTrackerOwnershipDone(wdeps, repoDir, key, *rec) } // The committed receipt is authority, so a confirm error still proceeds on the // proof (best-effort mirror) — but the worktree it binds must be real: resolve @@ -690,7 +692,7 @@ func resolveRunTrackerOwnership(ctx context.Context, deps PlanningDeps, wdeps Wo matches := runTrackerProofsForContext(proofs, rec.ChildContextHash) switch len(matches) { case 0: - return runTrackerOwnershipDone(repoDir, key, *rec) + return runTrackerOwnershipDone(wdeps, repoDir, key, *rec) case 1: p := matches[0] // Adopt the sole proof: resolve the change's logical feature worktree @@ -817,11 +819,14 @@ func runTrackerOwnershipStop(repoDir, key string, rec RunTrackerRecord, reason s return &res } -// runTrackerOwnershipDone builds the terminal run-done no-attributable-claim report for a -// dispatch that provably claimed nothing, persists it, and returns it. -func runTrackerOwnershipDone(repoDir, key string, rec RunTrackerRecord) *RunVerdictResult { - res := persistRunVerdict(repoDir, key, rec, - runVerdictLine(key, RunDecisionDone, RunOutcomeNoAttributableClaim, 0, true, nil)) +// runTrackerOwnershipDone handles a dispatch that provably claimed nothing: it +// reports run-done no-attributable-claim and retires the run through the same +// closeout as every keyed run-done (change 0540), so a run that never claimed is +// never left active for the live-run scan to refuse on. A blocked closeout +// reports run-stop run-tracker-unavailable with the closeout's reason; no run beside +// the record keeps the prior mirror-only behavior. +func runTrackerOwnershipDone(wdeps WorkspaceDeps, repoDir, key string, rec RunTrackerRecord) *RunVerdictResult { + res := runTrackerCompleteRun(repoDir, key, rec, RunOutcomeNoAttributableClaim, 0, verdictSeams(repoDir, wdeps)) return &res } diff --git a/internal/app/runtracker_verdict_integration_test.go b/internal/app/runtracker_verdict_integration_test.go index 5b36332de..eb88e2862 100644 --- a/internal/app/runtracker_verdict_integration_test.go +++ b/internal/app/runtracker_verdict_integration_test.go @@ -778,6 +778,9 @@ func TestIntegrationRunVerdictVerdictNoBindingNoProofIsNoAttributableClaim(t *te if runTrackerRetryMarkerExists(t, repo, key) { t.Errorf("no-attributable-claim must never spend the retry") } + if _, _, err := LoadRunRecord(repo, key); !isRunKind(err, ErrRunNotFound) { + t.Fatalf("no run beside the record: the verdict must not fabricate one: %v", err) + } } // TestIntegrationRunVerdictVerdictUnconfirmedReservationRecoversFromExactReceipt: a reservation whose @@ -1432,3 +1435,253 @@ func TestIntegrationRunVerdictUnattributedNeverSettlesNeverLaunchedDrive(t *test t.Fatalf("an unattributed verdict settled a drive: outcome %s", out) } } + +// --- every keyed run-done retires its run (change 0540) ---------------------- +// +// Before 0540 only run-complete drove the closeout; no-attributable-claim and +// run-unclaimed wrote record.json alone and left run.json active forever, so the +// repository-wide live-run scan refused set-visibility and run cancel refused +// claim-unconfirmed. These tests pin the closeout on every run-done. + +// noClaimRunFixture is a keyed run that claimed nothing — no binding file and no +// committed proof under its context hash "ha" — with an active run.json beside its +// record and permissive, faked closeout seams. +type noClaimRunFixture struct { + repo, key string + wdeps WorkspaceDeps + observer *fakeProcessObserver + launchObserver *fakeLaunchObserver +} + +func newNoClaimRunFixture(t *testing.T) noClaimRunFixture { + t.Helper() + repo := newRunTrackerRepo(t) + key := runTrackerMintStarted(t, repo, nil, 1, "ha") + if _, err := MintRunRecord(repo, key, ""); err != nil { + t.Fatalf("MintRunRecord: %v", err) + } + observer := &fakeProcessObserver{defaultProven: true} + launchObserver := &fakeLaunchObserver{report: gatedrive.RunLaunchReport{Accounted: true}} + wdeps := WorkspaceDeps{ + // A sibling's proof under a DIFFERENT context hash: filtered out, so zero match. + ClaimProofs: &fakeProofScanner{proofs: []ClaimProof{{RequestID: "x", ChangeID: 9, RunContextHash: "OTHER"}}}, + CancelSeams: func(string) cancelSeams { + return cancelSeams{observer: observer, launchObserver: launchObserver} + }, + } + return noClaimRunFixture{repo: repo, key: key, wdeps: wdeps, observer: observer, launchObserver: launchObserver} +} + +// TestIntegrationRunVerdictNoAttributableClaimRetiresStrandedRun reproduces the +// #540 incident: record.json already terminal from an earlier verdict, run.json +// still active, no claim. The live-run scan lists it with the keyed-verdict remedy; +// re-running that verdict reports the unchanged line, closes the run out through +// the census for its own context hash, and the scan no longer lists it. +func TestIntegrationRunVerdictNoAttributableClaimRetiresStrandedRun(t *testing.T) { + fx := newNoClaimRunFixture(t) + rec, err := LoadRunTrackerRecord(fx.repo, fx.key) + if err != nil { + t.Fatalf("LoadRunTrackerRecord: %v", err) + } + rec.Terminal = true + rec.Disposition = "run-done " + fx.key + " no-attributable-claim" + must(t, SaveRunTrackerRecord(fx.repo, fx.key, rec)) + stateDir, err := runTrackerStateDir(fx.repo) + if err != nil { + t.Fatalf("runTrackerStateDir: %v", err) + } + before, err := liveRunsUnder(stateDir) + if err != nil { + t.Fatal(err) + } + if len(before) != 1 || before[0].Key != fx.key || before[0].Remedy != runVerdictCommand(fx.key) { + t.Fatalf("stranded run before the verdict = %#v, want one locator with remedy %q", before, runVerdictCommand(fx.key)) + } + + res := RunVerdict(context.Background(), PlanningDeps{}, fx.wdeps, GitHubDeps{}, fx.repo, fx.key) + if got, want := res.HumanText(), "run-done "+fx.key+" no-attributable-claim"; got != want { + t.Fatalf("HumanText = %q, want %q", got, want) + } + if st := loadRunState(t, fx.repo, fx.key); st != RunCompleted { + t.Fatalf("run state = %q, want completed", st) + } + if calls := fx.launchObserver.calls; len(calls) != 1 || calls[0] != "ha" { + t.Fatalf("census calls = %v, want [ha] (the closeout ran for the run's context hash)", calls) + } + after, err := liveRunsUnder(stateDir) + if err != nil { + t.Fatal(err) + } + if len(after) != 0 { + t.Fatalf("live runs after the verdict = %#v, want none", after) + } + if runTrackerRetryMarkerExists(t, fx.repo, fx.key) { + t.Error("retiring a no-claim run must never spend the retry") + } +} + +// TestIntegrationRunVerdictUnconfirmedReservationRetiresRun: the other +// no-attributable-claim leg — a reservation whose claim never committed — also +// closes the run out, and leaves the reservation refused (never confirmed). +func TestIntegrationRunVerdictUnconfirmedReservationRetiresRun(t *testing.T) { + fx := newNoClaimRunFixture(t) + must(t, ReserveRunTrackerClaim(fx.repo, fx.key, 3, "claim-3-v")) + fx.wdeps.ClaimProofs = &fakeProofScanner{proofs: nil} + + res := RunVerdict(context.Background(), PlanningDeps{}, fx.wdeps, GitHubDeps{}, fx.repo, fx.key) + if got, want := res.HumanText(), "run-done "+fx.key+" no-attributable-claim"; got != want { + t.Fatalf("HumanText = %q, want %q", got, want) + } + if st := loadRunState(t, fx.repo, fx.key); st != RunCompleted { + t.Fatalf("run state = %q, want completed", st) + } + b, ok, err := LoadRunTrackerClaimBinding(fx.repo, fx.key) + if err != nil || !ok || b.Confirmed { + t.Fatalf("binding = %+v ok=%v err=%v, want the unconfirmed reservation intact", b, ok, err) + } +} + +// TestIntegrationRunVerdictRunUnclaimedRetiresRun: a confirmed claim whose change +// went back to proposed reports the unchanged run-unclaimed line and closes the +// run out. +func TestIntegrationRunVerdictRunUnclaimedRetiresRun(t *testing.T) { + repo := newRunTrackerRepo(t) + key := runTrackerMintStarted(t, repo, nil, 1, "ha") + must(t, ReserveRunTrackerClaim(repo, key, 3, "claim-3-v")) + must(t, ConfirmRunTrackerClaim(repo, key, 3, "claim-3-v", "r1", "")) + if _, err := MintRunRecord(repo, key, "3"); err != nil { + t.Fatalf("MintRunRecord: %v", err) + } + launchObserver := &fakeLaunchObserver{report: gatedrive.RunLaunchReport{Accounted: true}} + wdeps := WorkspaceDeps{ + ClaimProofs: &fakeProofScanner{proofs: []ClaimProof{ + {RequestID: "claim-3-v", ChangeID: 3, RunContextHash: "ha", Revision: "r1"}, + }}, + CancelSeams: func(string) cancelSeams { + return cancelSeams{observer: &fakeProcessObserver{defaultProven: true}, launchObserver: launchObserver} + }, + } + deps := runTrackerLightDeps(t, []StatusBlob{runTrackerProposedBlob(3, rvSlug)}) + + res := RunVerdict(context.Background(), deps, wdeps, GitHubDeps{}, repo, key) + if got, want := res.HumanText(), "run-done "+key+" run-unclaimed 3"; got != want { + t.Fatalf("HumanText = %q, want %q", got, want) + } + if st := loadRunState(t, repo, key); st != RunCompleted { + t.Fatalf("run state = %q, want completed", st) + } + if len(launchObserver.calls) != 1 { + t.Fatalf("census calls = %v, want one (the closeout ran)", launchObserver.calls) + } +} + +// TestIntegrationRunVerdictNoClaimBlockedCloseoutReplays: an unproven obligation +// blocks the closeout — run-stop completion-unaccounted with findings, the run +// stays completing, no retry spent. Once it settles, repeating the same keyed +// verdict replays the closeout to the unchanged run-done line and completed. +func TestIntegrationRunVerdictNoClaimBlockedCloseoutReplays(t *testing.T) { + fx := newNoClaimRunFixture(t) + must(t, RegisterRunParticipant(fx.repo, fx.key, RunParticipant{Kind: "raw-run", NativeHandle: "exec-live"})) + fx.observer.defaultProven = false + + res := RunVerdict(context.Background(), PlanningDeps{}, fx.wdeps, GitHubDeps{}, fx.repo, fx.key) + if res.Decision != RunDecisionStop || res.Outcome != RunOutcomeUnavailable || res.Reason != ReasonRunCompletionUnaccounted { + t.Fatalf("decision/outcome/reason = %q/%q/%q, want run-stop/run-tracker-unavailable/completion-unaccounted", + res.Decision, res.Outcome, res.Reason) + } + if len(res.CompletionFindings) == 0 { + t.Fatal("a blocked closeout carried no findings to settle") + } + if st := loadRunState(t, fx.repo, fx.key); st != RunCompleting { + t.Fatalf("run state = %q, want completing (the fence holds)", st) + } + if runTrackerRetryMarkerExists(t, fx.repo, fx.key) { + t.Error("a blocked closeout must never spend the retry") + } + + fx.observer.defaultProven = true + res2 := RunVerdict(context.Background(), PlanningDeps{}, fx.wdeps, GitHubDeps{}, fx.repo, fx.key) + if got, want := res2.HumanText(), "run-done "+fx.key+" no-attributable-claim"; got != want { + t.Fatalf("replay HumanText = %q, want %q", got, want) + } + if st := loadRunState(t, fx.repo, fx.key); st != RunCompleted { + t.Fatalf("run state after replay = %q, want completed", st) + } +} + +// TestIntegrationRunVerdictHaltedLeavesRunActive: run-halted is a stop, not a +// run-done. The run stays active (so halt → cancel → resume still works) and the +// closeout never runs. +func TestIntegrationRunVerdictHaltedLeavesRunActive(t *testing.T) { + repo := newRunTrackerRepo(t) + key := runTrackerMintAttributedLimit(t, repo, 3, 2) + if _, err := MintRunRecord(repo, key, "3"); err != nil { + t.Fatalf("MintRunRecord: %v", err) + } + launchObserver := &fakeLaunchObserver{report: gatedrive.RunLaunchReport{Accounted: true}} + wdeps := WorkspaceDeps{CancelSeams: func(string) cancelSeams { + return cancelSeams{observer: &fakeProcessObserver{defaultProven: true}, launchObserver: launchObserver} + }} + deps := runTrackerLightDeps(t, []StatusBlob{runTrackerHaltedInProgressBlob(3, rvSlug)}) + + res := RunVerdict(context.Background(), deps, wdeps, GitHubDeps{}, repo, key) + if got, want := res.HumanText(), "run-stop "+key+" run-halted 3"; got != want { + t.Fatalf("HumanText = %q, want %q", got, want) + } + if st := loadRunState(t, repo, key); st != RunActive { + t.Fatalf("run state = %q, want active (a halted run waits for cancel or continue)", st) + } + if len(launchObserver.calls) != 0 { + t.Fatalf("a halt ran the closeout census: %v", launchObserver.calls) + } +} + +// TestIntegrationRunVerdictNoClaimNeverRelabelsCancellingOrSuperseded: a run +// already fenced cancelling (or superseded) is never marked completed by a +// no-attributable-claim verdict; the closeout's own refusal is reported. +func TestIntegrationRunVerdictNoClaimNeverRelabelsCancellingOrSuperseded(t *testing.T) { + for _, tc := range []struct { + state runState + reason string + }{ + {RunCancelling, ReasonRunCancelled}, + {RunSuperseded, ReasonRunSuperseded}, + } { + t.Run(string(tc.state), func(t *testing.T) { + fx := newNoClaimRunFixture(t) + forceRunState(t, fx.repo, fx.key, tc.state) + res := RunVerdict(context.Background(), PlanningDeps{}, fx.wdeps, GitHubDeps{}, fx.repo, fx.key) + if res.Decision != RunDecisionStop || res.Reason != tc.reason { + t.Fatalf("decision/reason = %q/%q, want run-stop/%s", res.Decision, res.Reason, tc.reason) + } + if st := loadRunState(t, fx.repo, fx.key); st != tc.state { + t.Fatalf("run state = %q, want %q (never relabelled)", st, tc.state) + } + }) + } +} + +// TestIntegrationRunVerdictObserveLeavesNoClaimRunActive: unattributed observe +// mode over the incident fixture holds no key, so it never reaches the closeout — +// the run stays active and byte-identical. +func TestIntegrationRunVerdictObserveLeavesNoClaimRunActive(t *testing.T) { + fx := newNoClaimRunFixture(t) + _, genBefore, err := LoadRunRecord(fx.repo, fx.key) + if err != nil { + t.Fatalf("load run before: %v", err) + } + res := RunVerdictObserve(context.Background(), runTrackerLightDeps(t, nil), fx.wdeps, GitHubDeps{}, fx.repo, nil) + if got, want := res.HumanText(), "run-observe no-current-run"; got != want { + t.Fatalf("observe HumanText = %q, want %q", got, want) + } + ep, genAfter, err := LoadRunRecord(fx.repo, fx.key) + if err != nil { + t.Fatalf("load run after: %v", err) + } + if ep.State != RunActive || genAfter != genBefore { + t.Fatalf("observe mode wrote the run: state %q, generation %q -> %q", ep.State, genBefore, genAfter) + } + if len(fx.launchObserver.calls) != 0 { + t.Fatalf("observe mode ran the closeout census: %v", fx.launchObserver.calls) + } +} From 354d0d6d0e3334108675f2d1e9d7071f76d7c670 Mon Sep 17 00:00:00 2001 From: Daniel Hanold Date: Wed, 7 Oct 2026 22:26:47 -0400 Subject: [PATCH 5/7] fix(run-tracker): let resume pass over a run retired without run-complete (review finding: run-unclaimed retirement dead-ends resume) Retiring a run-unclaimed run to completed left run.json naming its change, so a later run start --resume for that change found it and answered "nothing to resume", which run cancel cannot clear. FindRunByChange now skips a completed run whose run-tracker record's latest report line is not run-done ... run-complete; such a run owns no worktree and holds no reservation, so the resume mints a fresh run. A run-complete line, or an unreadable record, still refuses as before. --- internal/app/runtracker_run_record.go | 35 ++++- internal/app/runtracker_start.go | 5 +- ...untracker_start_resume_integration_test.go | 141 ++++++++++++++++++ 3 files changed, 178 insertions(+), 3 deletions(-) diff --git a/internal/app/runtracker_run_record.go b/internal/app/runtracker_run_record.go index 30fb53f57..4f24431b5 100644 --- a/internal/app/runtracker_run_record.go +++ b/internal/app/runtracker_run_record.go @@ -42,6 +42,7 @@ import ( "io/fs" "os" "path/filepath" + "strings" "syscall" "time" ) @@ -637,6 +638,15 @@ func SupersedeCancelledRun(repoDir, runKey, replacementKey string) error { // is returned, so a repeat resume still recovers the reservation. A missing run-tracker // root or no match is (found=false, nil); a corrupt/unreadable sibling run is // skipped, mirroring findRunByWorktree. +// +// A COMPLETED run is the change's run only while its run-tracker record's latest +// report line is `run-done run-complete …` (runRetiredWithoutRunComplete). +// Every keyed run-done verdict retires its run to completed (change 0540), and a +// run-unclaimed run stays bound to the change it once claimed; if such a run +// matched, a resume after a keyless re-claim of the same change would hit +// resume's "nothing to resume" refusal, which run cancel cannot clear. A run +// retired that way owns no worktree and holds no reservation, so it is skipped +// like a run that never named the change, and the resume mints a fresh one. func FindRunByChange(repoDir, changeID string) (runKey string, rec RunRecord, found bool, err error) { if changeID == "" { return "", RunRecord{}, false, nil @@ -666,9 +676,13 @@ func FindRunByChange(repoDir, changeID string) (runKey string, rec RunRecord, fo if lerr != nil { continue // no run.json here, or a corrupt/unreadable sibling: cannot match } - if r.ChangeID == changeID { - matches = append(matches, matchEntry{key: key, rec: r}) + if r.ChangeID != changeID { + continue + } + if r.State == RunCompleted && runRetiredWithoutRunComplete(repoDir, key) { + continue // retired by a non-success run-done: no longer the change's run } + matches = append(matches, matchEntry{key: key, rec: r}) } if len(matches) == 0 { return "", RunRecord{}, false, nil @@ -707,6 +721,23 @@ func FindRunByChange(repoDir, changeID string) (runKey string, rec RunRecord, fo return "", RunRecord{}, false, runErr(ErrRunAmbiguous, "find-by-change", nil) } +// runRetiredWithoutRunComplete reports whether the completed run at key was retired +// by something other than a run-complete verdict: its run-tracker record reads +// cleanly and its latest report line is not `run-done run-complete …`. The +// line is the latest one, not the retiring one — a later keyed verdict on the same +// key may have replaced run-unclaimed with, say, run-stop … run-halted — so only a +// run-complete line keeps the run as the change's finished run. A record that +// cannot be read proves nothing and returns false, keeping the run (resume then +// refuses as before). +func runRetiredWithoutRunComplete(repoDir, key string) bool { + rec, err := LoadRunTrackerRecord(repoDir, key) + if err != nil { + return false + } + f := strings.Fields(rec.Disposition) + return len(f) < 3 || f[0] != RunDecisionDone || f[2] != VerdictRunComplete +} + // runToken mints a random 32-hex-char token (16 crypto-random bytes) for the // physical generation. It is an opaque lookup token, never encoded state. func runToken() (string, error) { diff --git a/internal/app/runtracker_start.go b/internal/app/runtracker_start.go index efe82edc5..8e532b520 100644 --- a/internal/app/runtracker_start.go +++ b/internal/app/runtracker_start.go @@ -620,7 +620,10 @@ func RunStart(ctx context.Context, deps PlanningDeps, wdeps WorkspaceDeps, sdeps case RunCompleted: // The successful closeout finished (change 0441): terminal, nothing to // resume — never quiescence-checked into a supersede, never a replacement - // reservation. Verify and finalize instead. + // reservation. Verify and finalize instead. FindRunByChange returns a + // completed run only when its latest report line is run-done … run-complete + // (or its record cannot be read); a run retired by another run-done outcome + // falls through to the fresh mint below. return runUntrackedMsg(ReasonRunResumeRunCompleted, "change "+scopeChangeID+"'s run completed successfully (run key "+ oldKey+"); there is nothing to resume — verify with 'docket run "+ diff --git a/internal/app/runtracker_start_resume_integration_test.go b/internal/app/runtracker_start_resume_integration_test.go index c22502baa..ffc6fe709 100644 --- a/internal/app/runtracker_start_resume_integration_test.go +++ b/internal/app/runtracker_start_resume_integration_test.go @@ -133,6 +133,7 @@ func TestIntegrationRunStartResumeRefusesCompletedRunWithoutSuperseding(t *testi repoDir := newWorkingRepo(t, nil).invocation deps, wdeps := resumeRunDeps(t) priorKey := seedPriorRun(t, repoDir, RunCompleted) + seedRunDisposition(t, repoDir, priorKey, "run-done "+priorKey+" run-complete 5") sp := &fakeScopePrep{grant: sampleScopeGrant()} res := RunStart(context.Background(), deps, wdeps, sp.deps(), repoDir, "implement-next", 5) @@ -160,6 +161,146 @@ func TestIntegrationRunStartResumeRefusesCompletedRunWithoutSuperseding(t *testi } } +// seedRunDisposition overwrites the latest report line on key's run-tracker record — +// the line the keyed verdict persisted there. +func seedRunDisposition(t *testing.T, repoDir, key, disposition string) { + t.Helper() + rec, err := LoadRunTrackerRecord(repoDir, key) + if err != nil { + t.Fatalf("LoadRunTrackerRecord: %v", err) + } + rec.Disposition = disposition + rec.Terminal = true + if err := SaveRunTrackerRecord(repoDir, key, rec); err != nil { + t.Fatalf("SaveRunTrackerRecord: %v", err) + } +} + +// TestIntegrationRunStartResumeAfterRunUnclaimedRetirementStartsFreshRun: a run the +// keyed run-unclaimed verdict retired to completed (change 0540) still names its +// change, but it is not the change's prior run. After a keyless re-claim of the same +// change halts, run start --resume mints and binds a fresh run instead of answering +// "nothing to resume", and a repeat resume resolves that fresh run as the prior run +// (resume-active-run) rather than an ambiguity with the retired one. +func TestIntegrationRunStartResumeAfterRunUnclaimedRetirementStartsFreshRun(t *testing.T) { + ctx := context.Background() + repoDir := newWorkingRepo(t, nil).invocation + + // A keyed run claims change 5; the claim is then released, so the keyed verdict + // reports run-unclaimed and retires the run to completed, still bound to change 5. + oldKey := runTrackerMintStarted(t, repoDir, nil, 1, "ha") + must(t, ReserveRunTrackerClaim(repoDir, oldKey, 5, "claim-5-v")) + must(t, ConfirmRunTrackerClaim(repoDir, oldKey, 5, "claim-5-v", "r1", "")) + if _, err := MintRunRecord(repoDir, oldKey, "5"); err != nil { + t.Fatalf("MintRunRecord: %v", err) + } + vwdeps := WorkspaceDeps{ + ClaimProofs: &fakeProofScanner{proofs: []ClaimProof{ + {RequestID: "claim-5-v", ChangeID: 5, RunContextHash: "ha", Revision: "r1"}, + }}, + CancelSeams: func(string) cancelSeams { + return cancelSeams{ + observer: &fakeProcessObserver{defaultProven: true}, + launchObserver: &fakeLaunchObserver{report: gatedrive.RunLaunchReport{Accounted: true}}, + } + }, + } + vdeps := runTrackerLightDeps(t, []StatusBlob{runTrackerProposedBlob(5, "epsilon")}) + v := RunVerdict(ctx, vdeps, vwdeps, GitHubDeps{}, repoDir, oldKey) + if got, want := v.HumanText(), "run-done "+oldKey+" run-unclaimed 5"; got != want { + t.Fatalf("verdict = %q, want %q", got, want) + } + if st := loadRunState(t, repoDir, oldKey); st != RunCompleted { + t.Fatalf("retired run state = %q, want completed", st) + } + + // A keyless dispatch re-claims change 5 and halts: it is in-progress again with + // no run-tracker run of its own. + reader := &fakeReader{pin: mainPin(t), corpus: []StatusBlob{runTrackerHaltedInProgressBlob(5, "epsilon")}} + deps := workspaceDepsFor(t, reader) + wdeps := WorkspaceDeps{Service: resumeInspectService("/tmp/wt/epsilon")} + sp := &fakeScopePrep{grant: sampleScopeGrant()} + + res := RunStart(ctx, deps, wdeps, sp.deps(), repoDir, "implement-next", 5) + if !res.Started { + t.Fatalf("resume after a run-unclaimed retirement did not start: reason %q, %q", res.Reason, res.HumanText()) + } + if res.Key == "" || res.Key == oldKey || sp.calls != 1 { + t.Fatalf("resume must mint a fresh run: key=%q (retired %q) scope calls=%d", res.Key, oldKey, sp.calls) + } + ep, _, err := LoadRunRecord(repoDir, res.Key) + if err != nil { + t.Fatalf("LoadRunRecord(fresh): %v", err) + } + if ep.State != RunActive || ep.ChangeID != "5" || ep.Worktree == "" { + t.Fatalf("fresh run = state %q change %q worktree %q, want active, bound to change 5 and its worktree", + ep.State, ep.ChangeID, ep.Worktree) + } + if st := loadRunState(t, repoDir, oldKey); st != RunCompleted { + t.Fatalf("resume must leave the retired run completed, got %q", st) + } + + sp2 := &fakeScopePrep{grant: sampleScopeGrant()} + res2 := RunStart(ctx, deps, wdeps, sp2.deps(), repoDir, "implement-next", 5) + if res2.Started || res2.Reason != ReasonRunResumeActiveRun || !strings.Contains(res2.Message, "run key "+res.Key) { + t.Fatalf("repeat resume = started %v reason %q message %q, want resume-active-run naming run key %q", + res2.Started, res2.Reason, res2.Message, res.Key) + } +} + +// TestIntegrationRunStartResumeCompletedRunByLatestReportLine: only a completed run +// whose latest report line is run-done … run-complete is the change's finished prior +// run ("nothing to resume"). One retired by another run-done outcome — or whose +// run-done line a later keyed verdict replaced, such as run-stop … run-halted after a +// keyless re-claim halted — is passed over and the resume starts a fresh run. A +// record.json that cannot be read proves nothing, so it still refuses. +func TestIntegrationRunStartResumeCompletedRunByLatestReportLine(t *testing.T) { + for _, tc := range []struct { + name string + disposition func(key string) string // "" corrupts record.json instead + wantStarted bool + }{ + {"no-attributable-claim", func(k string) string { return "run-done " + k + " no-attributable-claim" }, true}, + {"run-unclaimed", func(k string) string { return "run-done " + k + " run-unclaimed 5" }, true}, + {"halted replay", func(k string) string { return "run-stop " + k + " run-halted 5" }, true}, + {"run-complete", func(k string) string { return "run-done " + k + " run-complete 5" }, false}, + {"unreadable record", nil, false}, + } { + t.Run(tc.name, func(t *testing.T) { + repoDir := newWorkingRepo(t, nil).invocation + deps, wdeps := resumeRunDeps(t) + priorKey := seedPriorRun(t, repoDir, RunCompleted) + if tc.disposition != nil { + seedRunDisposition(t, repoDir, priorKey, tc.disposition(priorKey)) + } else { + stateDir, err := runTrackerStateDir(repoDir) + if err != nil { + t.Fatalf("runTrackerStateDir: %v", err) + } + if err := os.WriteFile(filepath.Join(stateDir, runTrackerDirName, priorKey, runTrackerRecordFileName), + []byte("{not json"), 0o600); err != nil { + t.Fatalf("corrupt record.json: %v", err) + } + } + sp := &fakeScopePrep{grant: sampleScopeGrant()} + + res := RunStart(context.Background(), deps, wdeps, sp.deps(), repoDir, "implement-next", 5) + if res.Started != tc.wantStarted { + t.Fatalf("started = %v (reason %q, %q), want %v", res.Started, res.Reason, res.HumanText(), tc.wantStarted) + } + if !tc.wantStarted && res.Reason != ReasonRunResumeRunCompleted { + t.Fatalf("Reason = %q, want %q", res.Reason, ReasonRunResumeRunCompleted) + } + if tc.wantStarted && res.Key == priorKey { + t.Fatalf("resume reused the retired run key %q", priorKey) + } + if st := loadRunState(t, repoDir, priorKey); st != RunCompleted { + t.Fatalf("resume must leave the completed run untouched, got %q", st) + } + }) + } +} + // TestIntegrationRunStartResumeCancellingIsPending: a resume of a change whose prior run is CANCELLING // is refused cancellation-pending — cleanup is still in flight, no replacement. func TestIntegrationRunStartResumeCancellingIsPending(t *testing.T) { From 653708c69dc1bc95f37c52345539fe548d62b30a Mon Sep 17 00:00:00 2001 From: Daniel Hanold Date: Wed, 7 Oct 2026 22:28:03 -0400 Subject: [PATCH 6/7] fix(run-tracker): qualify the live-run verdict remedy to after the dispatch returns (review finding: verdict remedy can end a live unclaimed dispatch) liveRunsUnder cannot tell a finished run that claimed nothing from a live dispatch that has not claimed yet; running the keyed verdict on the live one makes the record terminal and the child's claim is refused. The remedy now reads "once its dispatch has returned, run `docket run verdict `". --- internal/app/runtracker_live.go | 13 +++++++++++-- internal/app/runtracker_live_test.go | 6 +++++- internal/app/runtracker_verdict_integration_test.go | 4 ++-- 3 files changed, 18 insertions(+), 5 deletions(-) diff --git a/internal/app/runtracker_live.go b/internal/app/runtracker_live.go index e19c5eb66..bf29de6e1 100644 --- a/internal/app/runtracker_live.go +++ b/internal/app/runtracker_live.go @@ -30,7 +30,8 @@ type liveRunLocator struct{ Key, State, ChangeID, Remedy string } // // Each locator's Remedy follows liveRunCancelAuthority, so it names a command that // will act on the run in its current state: run cancel only where runCancelOwner -// accepts, the keyed run verdict for a readable run cancel would refuse, and the +// accepts, the keyed run verdict (once the dispatch has returned) for a readable +// run cancel would refuse, and the // by-hand remedy where neither command can load the run. func liveRunsUnder(stateDir string) ([]liveRunLocator, error) { root := filepath.Join(stateDir, runTrackerDirName) @@ -70,7 +71,7 @@ func liveRunsUnder(stateDir string) ([]liveRunLocator, error) { // Cancel refuses a run it cannot prove owned (ADR-0128); the keyed // verdict re-resolves ownership and retires a run that claimed nothing // (change 0540). - loc.Remedy = runVerdictCommand(key) + loc.Remedy = runVerdictRemedy(key) default: loc.Remedy = byHandRemedy(dir) } @@ -103,6 +104,14 @@ func runVerdictCommand(key string) string { return "docket run verdict " + key } +// runVerdictRemedy names runVerdictCommand only for after the run's dispatch has +// returned. The scan cannot tell a finished run that claimed nothing from a live +// dispatch that has not claimed yet, and a verdict on the live one ends it: the +// record goes terminal and the child's later claim is refused. +func runVerdictRemedy(key string) string { + return "once its dispatch has returned, run `" + runVerdictCommand(key) + "`" +} + // liveRunAuthority is what run cancel would decide about one live run, read from // its own key directory. type liveRunAuthority int diff --git a/internal/app/runtracker_live_test.go b/internal/app/runtracker_live_test.go index 797ea0ad2..66246f0ad 100644 --- a/internal/app/runtracker_live_test.go +++ b/internal/app/runtracker_live_test.go @@ -139,10 +139,14 @@ func TestLiveRunsUnderMissingRoot(t *testing.T) { // command that will act on it in its current state (change 0540). Cancel is named // only when runCancelOwner accepts; a readable run cancel would refuse gets the // keyed verdict, which retires it; anything neither command can load is by hand. +// The verdict remedy is qualified: the scan cannot tell a finished unclaimed run +// from a live dispatch that has not claimed yet, and a verdict on the latter ends it. func TestLiveRunsUnderRemedyFollowsCancelAuthority(t *testing.T) { stateDir := testsupport.TempDir(t) cancel := func(k string) string { return "docket run cancel --key " + k + " --reason " } - verdict := func(k string) string { return "docket run verdict " + k } + verdict := func(k string) string { + return "once its dispatch has returned, run `docket run verdict " + k + "`" + } byHand := func(dir string) string { return "inspect or remove " + dir + " by hand" } // The incident: a record with no parent capability and no claim (no binding file). diff --git a/internal/app/runtracker_verdict_integration_test.go b/internal/app/runtracker_verdict_integration_test.go index eb88e2862..7853d9117 100644 --- a/internal/app/runtracker_verdict_integration_test.go +++ b/internal/app/runtracker_verdict_integration_test.go @@ -1494,8 +1494,8 @@ func TestIntegrationRunVerdictNoAttributableClaimRetiresStrandedRun(t *testing.T if err != nil { t.Fatal(err) } - if len(before) != 1 || before[0].Key != fx.key || before[0].Remedy != runVerdictCommand(fx.key) { - t.Fatalf("stranded run before the verdict = %#v, want one locator with remedy %q", before, runVerdictCommand(fx.key)) + if len(before) != 1 || before[0].Key != fx.key || before[0].Remedy != runVerdictRemedy(fx.key) { + t.Fatalf("stranded run before the verdict = %#v, want one locator with remedy %q", before, runVerdictRemedy(fx.key)) } res := RunVerdict(context.Background(), PlanningDeps{}, fx.wdeps, GitHubDeps{}, fx.repo, fx.key) From 75102ddf632911c849d941816281ed6cdd92944c Mon Sep 17 00:00:00 2001 From: Daniel Hanold Date: Wed, 7 Oct 2026 22:28:37 -0400 Subject: [PATCH 7/7] docs(run-tracker): stop promising the live-run verdict remedy settles every refused run (review finding: verdict remedy loops on a halted change) For a run cancel refuses (authority-unavailable, claim-mismatch), the keyed verdict retires the run only on a run-done outcome; a run-stop verdict leaves run.json active. Soften liveRunsUnder's comment to promise only that. --- internal/app/runtracker_live.go | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/internal/app/runtracker_live.go b/internal/app/runtracker_live.go index bf29de6e1..a31aebaee 100644 --- a/internal/app/runtracker_live.go +++ b/internal/app/runtracker_live.go @@ -28,11 +28,13 @@ type liveRunLocator struct{ Key, State, ChangeID, Remedy string } // is skipped, as is any non-directory entry. A missing root is (nil, nil); any // other failure to list the root is an error. // -// Each locator's Remedy follows liveRunCancelAuthority, so it names a command that -// will act on the run in its current state: run cancel only where runCancelOwner -// accepts, the keyed run verdict (once the dispatch has returned) for a readable -// run cancel would refuse, and the -// by-hand remedy where neither command can load the run. +// Each locator's Remedy follows liveRunCancelAuthority: run cancel only where +// runCancelOwner accepts, and the by-hand remedy where neither run cancel nor the +// verdict can load the run. For a readable run cancel would refuse it names the +// keyed run verdict (once the dispatch has returned), which re-resolves ownership +// and retires the run on any run-done outcome. It promises no more than that: a +// verdict that reports run-stop (a halted or incomplete change) leaves the run +// active, and settling it is then a human's call. func liveRunsUnder(stateDir string) ([]liveRunLocator, error) { root := filepath.Join(stateDir, runTrackerDirName) entries, err := os.ReadDir(root) @@ -69,8 +71,9 @@ func liveRunsUnder(stateDir string) ([]liveRunLocator, error) { loc.Remedy = runCancelCommand(key) case liveRunCancelRefuses: // Cancel refuses a run it cannot prove owned (ADR-0128); the keyed - // verdict re-resolves ownership and retires a run that claimed nothing - // (change 0540). + // verdict re-resolves ownership and retires the run on any run-done + // outcome, including one that claimed nothing (change 0540); a run-stop + // verdict leaves it active. loc.Remedy = runVerdictRemedy(key) default: loc.Remedy = byHandRemedy(dir)