Repository navigation
Commit e6557fe
F1a: persist the runner lifecycle in the Store (#53)
* F1a: persist the runner lifecycle in the Store
Schema v6 with a v5 backfill: tasks, attempts, user_actions and
feedback_events. Guarded attempt transitions, where the Store chooses
the terminal state from the first reason, D's result and context
currency. State version and context generation counters, cancel task,
merge closure, replayable user actions (including refusals) and
feedback events.
Implements the Store slice of docs/implementation/runner-lifecycle.md.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Complete only into a running task (contract round 35)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Refresh a merge click's replay response when the attempt changes
Codex review of #53: finishMergeAttempt updated only merge_attempts, so a
merge click resent after a lost response replayed the saved "in progress"
answer (runner-lifecycle.md: the outcome transaction updates the saved
response). beginMergeAttempt now takes the starting action ID, and every
attempt change refreshes that user_actions row in the same transaction
with mergeActionResponse(attempt). Regression fails without the refresh.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Refuse a merge for a closed, cancelling or changed task
Codex review of #53 (P1): beginMergeAttempt checked only the review state,
so a stale merge click after Cancel task could start a merge, and its
confirmation would reopen the cancelled task as merged with a second
task-closed event.
The admission transaction now refuses a closed task, a pending cancel,
and (when given) a task state version other than the expected one.
MergeCoordinator captures the task state version when the merge request
arrives and passes it after the final await, as runner-lifecycle.md
"Irreversible actions" requires. Regressions fail without each guard.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Merge only from review with no active attempt; bind events to their action
Codex review of #53, pass 3:
- P1: a merge could start while an attempt was pending or running, because
the captured task version already included it. Merge admission now
requires MERGEABLE_STATUSES ('in review', 'approved but merge blocked')
and no active attempt, in the admission transaction.
- P2: recordFeedback checked only that some transaction was open, so a
callback could commit an event under another action ID with no
user_actions row. userAction now records the running action, and
recordFeedback requires the same plan key and action ID.
Regressions fail without each guard.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Route merge clicks through the replayable user action
Codex review of #53, pass 4 (P2): the Store could replay a merge click, but
the production path passed no action ID, so a merge resent after a lost
response failed as "already active" instead of reporting the outcome.
- The browser sends a UUID actionId per merge click and keeps it until a
definite response arrives (a TypeError from fetch means none did).
- The server passes it to MergeCoordinator.merge(token, actionId).
- The coordinator replays a saved action before any validation (using the
cached display status), and otherwise begins the attempt inside
Store.userAction, so the attempt and its saved response commit together.
- Store.savedAction is the shared replay lookup, factored out of userAction.
Regression fails without the replay.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Replay merge clicks before the active guard, save early refusals and URLs
Codex review of #53, pass 5 (three P2s in the merge replay path):
- A resend that arrived while the first merge was still running hit the
"already running" guard before the replay lookup. merge() now replays a
saved click first.
- A refusal before admission (blocked checks, stale token, changed
requirements) was not saved, so reusing the action ID later could merge.
Definite pre-admission refusals are now recorded through userAction;
aborts, the deadline and shutdown stay resendable.
- A direct merge did not store its URL, so a replay returned an empty one.
finishMergeAttempt accepts the URL and the direct path passes it.
Each regression fails without its fix.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Refuse admission once the whole-task budget has run out
Codex review of #53, pass 6 (P2): admitAttempt checked only the attempt
deadline, so after budget_deadline passed a retry or fresh attempt could
still start. Admission now moves an expired task to 'needs human' (the
time-limit mapping in runner-lifecycle.md) in a committed transaction and
refuses. The existing "keeps the budget across attempts" test retried after
its budget expired, so it now retries inside the budget.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Require a UUID v4 actionId on every merge request
Codex review of #53, pass 7 (P2): the server accepted a merge without an
actionId, which bypassed replay, so a refused click resent after a state
change could merge. runner-lifecycle.md "User actions" requires the server
to refuse a missing or invalid key with HTTP 400 before any work. The merge
route now does; the two browser tests that post merges send a key, and a new
browser test checks the 400 and that nothing was stored.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Keep the budget handoff inside user actions; replay refused merges as errors
Codex review of #53, pass 8:
- P1: inside userAction (the retry path), admitAttempt's move to 'needs
human' was written in the outer transaction and rolled back by its own
refusal, leaving the task running. The refusal is now a
RefusalWithEffect: userAction commits its effect with the saved refusal,
and a direct call commits it in its own transaction. The effect re-checks
that the task is still open, idle and past its budget.
- P2: a failed or removed merge replayed as a 200 "submitted" with an empty
URL. Replay now rejects with the saved reason, like the first response.
Both regressions fail without their fixes.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Return a committed merge on replay even if the review cannot reload
Codex review of #53, pass 9 (P2): replay called service.load() outside the
displayStatus fallback, so a failing local refresh reported an already
merged click as blocked (409) instead of its saved URL (AGENTS.md: never
convert a committed irreversible result into action failure). A failing
reload or status read now yields an unavailable "refresh to confirm"
status alongside the saved result. Regression fails without the fix.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Add AGENTS.md rules from the #53 review-lesson audit
Ten Codex passes on #53 found gaps in merge replay (required key, lookup
before guards, saving pre-admission refusals, current saved responses,
failures replayed as failures, complete result fields) and a refusal whose
state change was rolled back. Two rules capture both patterns.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Block runner work during a merge; replay saved merges during shutdown
Codex review of #53, pass 11 (two P2s):
- transitionTask and admitAttempt ignored an in-flight merge attempt, so
runner work could start while GitHub might still merge the reviewed head.
Both now refuse while the latest merge is submitting or queued.
- A resend of a saved merge after close() began was refused as "shutting
down" before replay. The coordinator now replays first. The server's
HTTP 503 during shutdown stays (runner-lifecycle.md), and the browser now
keeps the action key on a 503 as well as on a lost response, so the next
click resends it. The AGENTS.md rule names that one exception.
Regressions (store, coordinator, browser) fail without their fixes.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Join a resend of the in-flight merge click instead of refusing it
Codex review of #53, pass 12 (P1): a resend that arrived while the first
click was still validating, before its action was saved, missed replay and
got "already running"; the browser dropped the key while the original could
still merge. The coordinator now remembers the active click's key and
request, returns the same promise to a matching resend, and refuses the same
key with a different request as a reused ID. Regression fails without it.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Join the in-progress merge click before replaying its saved state
Codex review of #53, pass 13 (P2): for a direct merge still running, a
resend returned the saved "submitting" state instead of joining the active
merge; direct attempts are not polled, so the UI stayed on "Submitting".
The coordinator now joins a matching active click first and replays only
when no merge for that click is in progress. The regression now asserts the
resend gets the final result, and fails with the old order.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Show non-review tasks as merge blockers; new key for a resolved click
Codex review of #53, pass 14 (two P2s; the P1 to route review feedback
through userAction is F1e's scope, #60, and stays out of this slice):
- Merge readiness ignored task status, so Merge PR could render ready for
a queued, running or human-gated task. Status now adds a 'task' blocker
unless the task is in a mergeable status.
- After a lost response the browser kept its key, and once that attempt
ended, Retry resent it and only replayed the old failure. Merge status now
exposes the attempt's actionId, and the browser drops its retained key
when the attempt that key started is merged, removed or failed.
Both regressions fail without their fixes.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Answer 503 for a merge stopped by its deadline or shutdown
Codex review of #53, pass 15 (P2): a deadline abort left the action
unsaved on purpose, but the server answered 409 and the browser dropped the
key, so the next click was a new action instead of a resend. The
coordinator now raises MergeNotApplied for an abort (deadline or shutdown)
and for a coordinator that is shutting down, and the server maps it to 503;
the browser already keeps its key on 503. Coordinator and server
regressions fail without the change.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Keep the merge key unless a parsed answer resolves the click
Codex review of #53, pass 16 (P2): an empty or truncated JSON body made
response.json() throw a SyntaxError with no status, and the browser dropped
the key although the outcome was unknown. The browser now drops the key only
for a parsed server answer with a definite status (anything but 503); no
response, an unreadable body, or a 503 keeps it. The browser regression adds
the unreadable-body case and fails with the old rule.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Refuse to reassign work on a closed task
Codex review of #53, pass 17 (P2): setAssignment accepted a merged or
cancelled task with a current state version and changed its assignment and
referenced-code hash after task-closed. It now refuses a closed task, like
transitionTask and admission. Regression fails without the guard.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Report 503 only for pre-admission aborts; save concurrent-click refusals
Codex review of #53, pass 18:
- P1: an abort during the GitHub merge command raised MergeNotApplied (503,
"nothing applied") although the attempt was durable and its outcome
unknown. Only an abort before admission is MergeNotApplied now; after
admission the original error is reported and the attempt stays in flight
to reconcile, as merge-queue.md documents.
- P2: "A merge attempt is already running" was not saved under the second
click's key, so a resend after a lost 409 could start a merge later. It is
now recorded like other pre-admission refusals (shared #recordRefusal).
Both regressions fail without their fixes.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Answer with a click's saved outcome when another coordinator saved it
Codex review of #53, pass 19 (P2): two coordinators sharing a database
could both validate the same actionId; the second missed the first replay
lookup and answered 409 instead of the saved outcome. When validation or
admission refuses a click that now has a saved outcome, the coordinator
returns that outcome instead of the refusal. Two regressions (save during
validation, save just before admission) fail without it.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Report an unsaved merge refusal as resendable
Codex review of #53, pass 20 (P1): #recordRefusal swallowed every error, so
a busy or failed database left a pre-admission refusal unsaved while the
server still answered a definite 409; a resend after a lost response could
then be evaluated as a new merge. Only the re-raised refusal and an already
saved outcome are treated as settled now; any other failure becomes
MergeNotApplied (503), so the browser keeps its key. Regression fails
without it.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Fail on any D stop reason; guard and version merge-time task changes
Copilot review of #53:
- classifySettlement published 'completed' for a D stop reason such as
cancelled or shutdown with exit 0 and no first reason. Any D stop reason
now ends 'failed' (a missing stopReason is the only normal finish).
- setAssignment could change the context while a merge was submitting or
queued; it now refuses during an active merge, like transitionTask.
- Merge-attempt inserts and changes did not bump the task state version.
They now do, in the same transaction, without changing the context
generation (runner-lifecycle.md "State version").
Each regression fails without its fix.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Guard plan edits during merges; keep merge URL and key through unknowns
Copilot review of #53 (round 2):
- Plan edits (importRevision, applySuggestion) are refused while a merge is
submitting or queued, and a confirmed merge closes the task with the
merged attempt's revision and snapshot. HEAD observation (recordHistory)
stays allowed: the merge is pinned to the reviewed head.
- A direct merge reconciled after a lost response now records GitHub's PR
URL (read with `gh pr view --json url`), so a replayed click reports it.
- After admission, an unknown GitHub outcome raises MergeOutcomeUnknown;
the server answers 409 with outcomeUnknown, and the browser keeps its key
until that attempt ends. Only GitHub's confirmed refusal stays definite.
Each regression fails without its fix.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Pin that a locked database is not saved as a refusal
Copilot review of #53 (round 3) asked whether busy or locked SQLite errors
escape userAction's storage check. node:sqlite reports them as
ERR_SQLITE_ERROR (errcode 5, "database is locked"), the code userAction
already treats as a storage error. This regression hits a real locked
database from a second connection and checks that nothing is saved and
the same action can be resent; it fails if the check is removed.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Keep closed tasks unchanged by plan writes and HEAD observation
Copilot review of #53 (round 4): importRevision and applySuggestion could
still change a merged or cancelled task, and a snapshot recorded after
closing bumped its counters. Plan writes now refuse a closed task. HEAD
observation (recordHistory) still records the snapshot, because the review
screen reads it after a merge, but no longer bumps a closed task's state
version or context generation. Regression fails without either guard.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Answer resend (503) when a saved merge outcome cannot be read
Copilot review of #53 (round 5, "previously missed"): a storage failure
while reading a click's saved outcome escaped #replay as an ordinary error,
so the server answered 409 and the browser dropped its key although the
click may already have applied. Both replay paths now map SQLite storage
errors to MergeNotApplied (503); saved refusals and reused keys still pass
through as definite answers. Regression fails without the mapping.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Fix four merge-replay and migration gaps from Codex review
/codex review of #53 at 83451f3 (four P2s, each reproduced by Codex):
- A replay answered from the outcome read before its own status refresh,
which can reconcile the attempt. It now re-reads the saved outcome after
the refresh, so a reconciled merge reports its URL and a failure fails.
- A SQLite error while storing the attempt at admission was treated as a
saved refusal (409). #recordRefusal now checks for a storage error first
and answers MergeNotApplied (503), so the browser keeps its key.
- After a lost click, a review edit and Refresh changed data.token, so the
retained key went out with a different request and was refused as
reused. The browser now keeps the original request with its key.
- The v6 backfill attributed a merged task's task-closed event to the
plan's current snapshot and revision. It now uses the merged attempt's.
Each regression fails without its fix.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Answer with a click saved concurrently instead of a stale refusal
Copilot review of #53 ("previously missed"): after the replay lookup
missed, another coordinator could save the same click before this one
saved its refusal. userAction then replayed that saved outcome, but
#recordRefusal discarded it and the caller answered with its own stale
refusal. #recordRefusal now reports what happened: this refusal saved
(caller answers with it), a concurrently saved success (returned as a
replay), or a saved refusal or reused key (thrown as the answer). Both
callers use the result. Two regressions fail without it.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Refuse rebase and ledger writes during a merge or after closing
Copilot review of #53: #snapshot's exception for the review screen's HEAD
observation also covered recordRebase and recordHistory with ledger entries,
which change the reviewed context. They could land while a merge was in
flight, or on a closed task. Both now refuse during an active merge and on
a closed task; recordHistory without ledger entries (the observation from
ReviewService.load) is still allowed. Regression fails without either guard.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Answer resend when a replay cannot re-read its saved outcome
Copilot review of #53 ("previously missed"): after the replay's status
refresh, a storage error while re-reading the saved outcome was swallowed
and the older record was returned as current (HTTP 200), so the browser
dropped its key without a durable answer. The re-read now propagates:
read() maps a storage error to MergeNotApplied (503). Regression fails
without it.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Keep unresolved merge clicks resendable; freeze reviews of closed tasks
Copilot review of #53:
- A replayed click whose attempt is still submitting answered 200 with an
empty URL, so the browser dropped its key. It now answers
MergeOutcomeUnknown (409 outcomeUnknown), matching the first answer; the
three concurrency tests that replayed another coordinator's in-flight
click now expect that too.
- A confirmed GitHub refusal was answered as definite even when its failed
state could not be saved. It is definite only once finishMergeAttempt
commits; otherwise the answer is MergeOutcomeUnknown.
- saveReview and addReviewNote now refuse a merged or cancelled task, so
approvals, choices and notes no longer change a closed task's review.
Each regression fails without its fix.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Refuse feedback events after a task closes
Copilot review of #53 ("previously missed"): recordFeedback could append a
feedback event after #closeTask wrote task-closed, so lane J could read
feedback from after closing. It now refuses a merged or cancelled task,
like the other writers. Regression fails without the guard.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Answer with a poll's committed outcome after a successful merge command
Copilot review of #53: after gateway.merge succeeded, the queued/merged
transition could return false because a concurrent poll already moved the
attempt, and the result was ignored. A poll-committed removed or failed
state was answered as a 200 success, and a poll-observed queued state
blocked the URL from ever reaching the attempt and its saved answer.
queueMergeAttempt is now idempotent from queued, and when a transition
does not apply the coordinator re-reads the attempt and answers with a
committed failure. Two regressions fail without the fix.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Fail closed on a newer merge attempt; refuse approvals during a merge
Copilot review of #53:
- If a newer merge attempt exists when the old command's transition does
not apply (possible only across processes until the single-runner lock),
the old command now answers MergeOutcomeUnknown instead of success.
- saveReview (approvals and segment choices) now refuses while a merge is
submitting or queued, so the reviewed state cannot change under it.
Notes stay allowed: they change no reviewed code, and an existing browser
test requests a change during a queued merge.
Both regressions fail without their fixes.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>1 parent 15ee8fa commit e6557fe
10 files changed
Lines changed: 1770 additions & 42 deletions
File tree
- github
- runner
- test
- browser
- web
- public
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
81 | 81 | | |
82 | 82 | | |
83 | 83 | | |
| 84 | + | |
| 85 | + | |
84 | 86 | | |
85 | 87 | | |
86 | 88 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
19 | 19 | | |
20 | 20 | | |
21 | 21 | | |
| 22 | + | |
| 23 | + | |
22 | 24 | | |
23 | 25 | | |
24 | 26 | | |
| |||
139 | 141 | | |
140 | 142 | | |
141 | 143 | | |
142 | | - | |
| 144 | + | |
143 | 145 | | |
144 | 146 | | |
145 | 147 | | |
| |||
233 | 235 | | |
234 | 236 | | |
235 | 237 | | |
| 238 | + | |
236 | 239 | | |
237 | 240 | | |
238 | 241 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
0 commit comments