Skip to content

F1c: shutdown wiring and /api/runner - #57

Draft
mchwang wants to merge 1 commit into
feat/f1b-coordinatorfrom
feat/f1c-shutdown-wiring
Draft

mchwang wants to merge 1 commit into
feat/f1b-coordinatorfrom
feat/f1c-shutdown-wiring

Conversation

@mchwang

@mchwang mchwang commented Sep 26, 2026

Copy link
Copy Markdown
Contributor

Lane F, step F1, slice F1c: shutdown wiring and /api/runner. Stacked on #56 (F1b), which is stacked on #53 (F1a); merge those first. Related: #22, #51.

What this does

  • Store write gate (runner/store.ts). closeWrites() makes every write throw ShuttingDownError. The only exception is a write run through the shutdown capability, which the Store issues once, to the server. The server hands it only to coordinators' settlement and close code: the runner, questions' finishAnswer, and the merge coordinator's settlement after an irreversible command. Reads keep working. A user action refused by the gate is not recorded, so the UI may resend it.

  • Shutdown order (web/server.ts):

    1. Set stopping and make the runner reject admission, in one synchronous turn.
    2. Drain admitted requests.
    3. After the drain limit, abort what remains, then close the Store gate.
    4. Stop and await runner jobs.
    5. Then await server closure, the question coordinator and the Store close, as before.

    The server also closes idle keep-alive connections once shutdown starts and after each request finishes. Without this, a finished request's socket held server.close() open until the keep-alive timeout; main has the same behavior today.

  • Error mapping. ShuttingDownError maps to 503, never to the 409 used for review errors. A new BadRequest maps to 400; malformed action and attempt IDs use it and are never recorded.

  • Merge coordinator (runner/merge.ts). displayStatus() and #pollQueue() used to swallow every error into a status. They now rethrow ShuttingDownError first. Settlement after gateway.merge (recording a merge that GitHub completed, or a refusal) runs through the capability, so it still lands after the gate closes.

  • /api/runner.

    • GET reads only the task and attempt rows and returns stateVersion, retryable (computed by the server), stopRequested and unresolved.
    • POST handles cancel-attempt, retry and cancel-task. Each goes through Store.userAction, so it is replayed exactly by actionId.
    • The runner coordinator exists only when D is injected (the new runnerDeps argument). Until D follow-ups required by the F1 runner lifecycle contract #51 lands, runner-only actions answer "The runner is not available yet."
  • Coordinator. Admission after shutdown throws ShuttingDownError. A job starts one microtask after admission, so a user action whose transaction rolls back cannot leave an orphan job.

Contract correction found while implementing

The merged contract (#49) had shutdown step 1 rejecting every admitted write and closing the Store gate. That contradicts AGENTS.md ("Drain already-admitted HTTP requests"; recheck only "at the irreversible action boundary"). It also broke the existing browser regression "drains an in-flight question request before closing its agent manager" and, as a knock-on effect, "blocks a partially received merge request when shutdown starts".

This PR follows AGENTS.md and the existing tests:

  • admitted requests drain;
  • the post-body recheck applies only to merge;
  • the gate closes after the drain, in step 3.

The doc's step 1 and step 3 rows, the summary, and the affected race-regression rows are updated in this PR.

Validation (head 081750f)

  • npm run typecheck: passes.

  • CI's unit set: 501 passed, 0 failed. 13 of those are new, in test/runner-shutdown.test.ts:

    • the gate and its capability;
    • merge rethrow after the gate, and merge settlement through the capability;
    • an admitted request draining;
    • a request destroyed after the drain limit, writing nothing;
    • 503 after shutdown begins;
    • 503 (not 409) when the review load hits the gate;
    • /api/runner status, replay, reused IDs, 400 for bad IDs, cancel and retry, with shutdown waiting for a running attempt;
    • the step-1 barrier and the gate closing after the drain.
  • npm run test:browser: 53 passed on this head.

  • Mutation check: each of these broken versions was caught:

    • displayStatus swallowing the error;
    • #pollQueue swallowing the error;
    • merge settlement without the capability;
    • the gate never closing;
    • no 503 mapping;
    • no 400 mapping;
    • the runner not rejecting admission at step 1.

    The merge-only post-body recheck is covered by the existing browser regression "blocks a partially received merge request when shutdown starts".

Not in this slice

  • F1d: startup recovery, the requeue claim and the OS lock.
  • F1e: the planning endpoints, and feedback wiring into review actions.
  • E3's settleSuggestion using the capability. This comes with the planning wiring in F1e.

🤖 Generated with Claude Code

Store write gate with a one-time shutdown capability for settling
coordinators; ShuttingDownError maps to 503 and BadRequest to 400; the
merge coordinator rethrows ShuttingDownError instead of swallowing it and
settles irreversible merges through the capability; the runner rejects
admission at step 1; admitted requests drain (AGENTS.md) and the gate
closes after the drain; idle keep-alive sockets are closed during shutdown;
/api/runner status and replayable cancel-attempt, retry and cancel-task.

Corrects the contract's shutdown steps 1 and 3 to follow the AGENTS.md
drain rule.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
mchwang added a commit that referenced this pull request Sep 26, 2026
Merge main. Record the F1 lifecycle contract (#49) and open F1a-F1c
(#53, #56, #57); record the ranked Issues screen (H4a, #55) with H4b's
trust action remaining; Issues is now a menu link, not a placeholder.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
mchwang added a commit that referenced this pull request Sep 26, 2026
* docs: reconcile the design plan with the code

Record where the code differs from the approved plan and update stale status:

- Record Ask as an interim exception to R1: it runs the vendor CLI on the
  host with tools off until lane F moves it into the lane D container.
- Amend D20: there is no "Merge anyway"; to override a blocker, merge on
  GitHub. Matches docs/implementation/guarded-merge.md.
- Tick T1, T2, T4, T5, T10, T13, T14 with test evidence; point Files lines
  at core/linking.ts and core/approvals.ts instead of never-created modules.
- Mark increment 1 merged; add a lane status table (C, D, K done; E, F, H
  progress); record decided open questions (issue ranking, AgentDiff).
- Add a verified status note for design tasks DT2-DT15; none newly ticked.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* docs: note merge-queue support in the guarded merge doc

The guarded merge gate doc still said merge-queue branches stay blocked.
#46 (closing #24) added queue lifecycle support. Point to merge-queue.md,
and state that adapters without queue inspection still fail closed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* docs: bring plan status up to date with F1 and H4a

Merge main. Record the F1 lifecycle contract (#49) and open F1a-F1c
(#53, #56, #57); record the ranked Issues screen (H4a, #55) with H4b's
trust action remaining; Issues is now a menu link, not a placeholder.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* docs: add F1d and Ask PRs to lane status

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Align merge-queue wording with the merged K2/K3 support

README no longer says merge queues block merging; it describes the
enqueue-then-confirm behaviour. The plan's wave-3 note records the old
block as history instead of a live instruction.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Record the K-lane queue block as history in the task table

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* README: disclose that Ask runs the agent CLI on the host

The plan (R1 exception) says README states this limit; it did not.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Add F1e (#60) and the #51 merge condition to the F lane row

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* README: distinguish queue-removal retry from changed-head review

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Plan: mark the install preflight and npx entry as planned

The CLI checks only the Node version today; say so instead of describing
the git/gh/container/sign-in preflight as current.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant