Read an oversized body before refusing it, and close on early refusals (#157)

docs/implementation/runner-lifecycle.md

@@ -254,7 +254,7 @@ codeboost never writes the user's repository. The commits it makes live in a **r
 | 1 | Set `stopping` on the server and make the runner coordinator reject admission, in the same synchronous turn, before any active-work list is copied. New API requests get HTTP 503, and new runner work throws `ShuttingDownError`. **Admitted requests are not rejected:** AGENTS.md says to drain already-admitted HTTP requests, and to recheck the flag only at irreversible boundaries. So after reading a body, the server rechecks `stopping` only for `merge`. The question coordinator also stops admission in this turn (`questions.stopAdmission()`): a question admitted before shutdown still saves its note, but starts no agent and gets a retryable "Server stopped" answer (`markStopped`). **Changed while implementing F1c:** an earlier revision of this row rejected every admitted write and closed the Store write gate here. That broke the drain rule and the existing browser regression "drains an in-flight question request before closing its agent manager", so the gate now closes in step 3. | server flag, merge recheck and question admission stop: yes. Runner barrier: new. |
 | 2 | Stop accepting connections, and wait for admitted requests up to the drain limit (at most 14.5 s, below the 15 s request timeout). | yes |
 | 2a | Abort every publish and PR close in progress (`TaskPublishing.close()`, #103, #111) in the same turn as step 1, and await it here, at the end of the drain, before the write gate closes: the publisher already refuses every later push and opening through the coordinator's `closing` flag, and a publish's last records (an abandoned opening, the outcome) must still land. A publish stopped this way is owed, so the next start publishes it again. | new (#103) |
-| 3 | After the drain limit, abort the signals of the remaining requests, destroy requests that are still reading a body, then **close the `Store` write gate**, then await request-owned work (the merge coordinator). From here on, every `Store` write throws `ShuttingDownError` unless the caller passes the shutdown capability. The server hands that capability at construction only to coordinators' settlement and close code (the runner, questions' `finishAnswer`, merge settlement after an irreversible command, and E3's `settleSuggestion` when it is wired), and to startup recovery; HTTP handlers and `ReviewService` never receive it. So a request-path write still pending after the abort (an aborted `/api/review` whose `load()` reaches `recordHistory`, or merge reconciliation and queue polling after a GitHub await) fails. The server maps `ShuttingDownError` to HTTP 503 "The review server is shutting down.", never to the 409 used for review errors, and there is no partial view. **Today the merge coordinator catches every error in `displayStatus()` and `#pollQueue()`**, which would swallow the gate error and answer 200; F1 makes each of those catch blocks rethrow `ShuttingDownError` first. The next startup checks GitHub again, so a skipped merge observation is recovered. The server also closes idle keep-alive connections once shutdown starts and after each request finishes, so a finished request's socket cannot hold `server.close()` open until its keep-alive timeout. | abort: yes. Gate, capability, rethrow and idle-connection close: new. |
+| 3 | After the drain limit, abort the signals of the remaining requests, destroy requests that are still reading a body, then **close the `Store` write gate**, then await request-owned work (the merge coordinator). From here on, every `Store` write throws `ShuttingDownError` unless the caller passes the shutdown capability. The server hands that capability at construction only to coordinators' settlement and close code (the runner, questions' `finishAnswer`, merge settlement after an irreversible command, and E3's `settleSuggestion` when it is wired), and to startup recovery; HTTP handlers and `ReviewService` never receive it. So a request-path write still pending after the abort (an aborted `/api/review` whose `load()` reaches `recordHistory`, or merge reconciliation and queue polling after a GitHub await) fails. The server maps `ShuttingDownError` to HTTP 503 "The review server is shutting down.", never to the 409 used for review errors, and there is no partial view. **Today the merge coordinator catches every error in `displayStatus()` and `#pollQueue()`**, which would swallow the gate error and answer 200; F1 makes each of those catch blocks rethrow `ShuttingDownError` first. The next startup checks GitHub again, so a skipped merge observation is recovered. The server also closes idle keep-alive connections once shutdown starts and after each request finishes, so a finished request's socket cannot hold `server.close()` open until its keep-alive timeout. For the same reason (#157), a reply to a request that declares a body (`Content-Length` above 0, or `Transfer-Encoding`) sent before that body has been read to its end, such as a 403 or 405, carries `Connection: close`. Every other reply keeps the connection. Node discards a body the handler never read, but the connection is busy while it does, so shutdown's idle-connection close skips it and nothing closes it once it goes idle. Node does not discard the rest of a body the handler has started reading, so a reply sent mid-body held the connection until the keep-alive timeout, or reset the client. A refusal that closes the connection can still reset a client sending a large body before it reads the reply; that is accepted, and the UI keeps an import's action ID for a resend. A body over its limit is therefore read to its end and discarded before the 413 goes out, which also means a client still sending is not reset before it reads the reply; a request still sending at the drain limit is destroyed like any other. | abort: yes. Gate, capability, rethrow and idle-connection close: new. |
 | 4 | For every `pending` and `running` attempt that has an in-memory job, record the first reason `shutdown` **only if no first reason is set yet**. A job already marked `cancelled`, `stale` or `time-limit` keeps its reason. For `running`, call `cancel('shutdown')` and await `settled`. For `pending`, await its preparation promise (the D start call itself returns at once, so it is never in progress), remove only its host-side preparation files (task storage waits for the terminal write, as in step 5), and cancel any handle that start returned, then await its `settled` (the "Launch" table). F does not abandon a job after a timer (decision 4). **This ends:** D's supervisor escalates to a forced kill, and retries unfinished container, network or setup cleanup every second for at most 60 seconds after the first failure (`CLEANUP_RETRY_WINDOW_MS`, `agents/adapters/supervisor.ts`). Retries after the first get only what is left of the window, and every cleanup subprocess is killed at its deadline, so settlement ends about 90 seconds after cleanup starts at worst (up to 30 seconds for the first, failed attempt, then the 60-second window). Then `settled` resolves with a `stopReason` and an `unreleased` list, even if Docker is unreachable or hangs. F records `unreleased` durably before it frees the slot. See the prerequisite below. | new |
 | 5 | For each attempt from step 4, in the order set by the task-storage holder row: the partial-output export (writable attempts), then the terminal state from its first reason (a `shutdown` reason gives `cancelled` "Stopped by shutdown") with its `diagnostic_ref`, then `removeTaskFilesystems`, and only then free its slot. A failed write leaves the row non-terminal, and a failed removal keeps the slot held. Startup recovery handles both. | new |
 | 5a | Await every plan run in progress (`ItemExecutor.close()`, #91). A run resumes after its attempt settles and may then pause for amendment or move the task to needs human: settlement writes, made with the capability, that must land before step 7 closes the `Store`. Admission is closed, so no run starts another item. | new (#91) |
@@ -495,6 +495,7 @@ Each case needs a test that fails before the fix and passes after it. Each test
 | Reason write fails (review round 9) | The first-reason write throws → settlement → the terminal write stores `cancelled` with the reason from memory |
 | Time limit, then crash (review round 9) | `time-limit` saved → process killed → restart → row `cancelled`, task `needs human`, retry refused |
 | Request still reading after the drain limit (review round 9, revised) | Body half-sent → shutdown → the drain limit passes → the request is destroyed → no `Store` change. A request arriving after shutdown began gets 503 |
+| Oversized body (#157) | Body over the limit → the 413 goes out only after the body ends, also during the drain, and shutdown then settles without waiting for the keep-alive timeout. Still sending at the drain limit → destroyed, no reply. A large body refused before it is read (403, 405) → the connection closes and shutdown is prompt |
 | Stale, then shutdown (review round 9) | Attempt marked `stale` → shutdown → row `stale`, not requeued |
 | Copied database (review round 9) | Copy the database file → start the copy → new token; recovery for the copy leaves the original's resources alone |
 | Review edit during the final merge check (review round 10) | Merge passes its GitHub checks → approval changed before the final re-read → merge refused |

docs/plan-format.md

@@ -95,7 +95,7 @@ questions: []
 
 Import accepts UTF-8 JSON or one YAML 1.2 document representing JSON-compatible data. Reject invalid UTF-8, input above 1 MiB, and nesting deeper than 50 containers before producing a plan. Parse into an intermediate syntax tree with duplicate-key detection; do not convert an unchecked YAML graph into application objects.
 
-`POST /api/plan/import` sends the source as one JSON string in a request body of at most 6 MiB + 16 KiB (6,307,840 bytes, `MAX_IMPORT_REQUEST_BYTES` in `web/server.ts`). Any JSON encoding writes each UTF-8 byte of the source as at most 6 bytes: YAML accepts a raw control character in a quoted scalar, and JSON writes it as `\u0001`. So every source within the 1 MiB parser limit fits, with 16 KiB left for the rest of the request. A larger body is refused with HTTP 413 before it is parsed. A body within the limit whose source is above 1 MiB gets the parser's refusal. Every other `POST` keeps its 16 KiB limit (#146).
+`POST /api/plan/import` sends the source as one JSON string in a request body of at most 6 MiB + 16 KiB (6,307,840 bytes, `MAX_IMPORT_REQUEST_BYTES` in `web/server.ts`). Any JSON encoding writes each UTF-8 byte of the source as at most 6 bytes: YAML accepts a raw control character in a quoted scalar, and JSON writes it as `\u0001`. So every source within the 1 MiB parser limit fits, with 16 KiB left for the rest of the request. A larger body is read to its end, then refused with HTTP 413 before it is parsed (#157). A body within the limit whose source is above 1 MiB gets the parser's refusal. Every other `POST` keeps its 16 KiB limit (#146).
 
 For both formats, object keys must be strings and unique after decoding at every level; reject duplicates rather than choosing the first or last value. JSON follows RFC 8259 syntax. YAML allows mappings, sequences, strings (including quoted and block strings), the exact plain literals `true`, `false`, and `null`, and numbers spelled using the JSON number grammar. Reject empty implicit values, non-finite values (`.nan`/`.inf`), non-JSON number spellings such as hex/octal or numeric separators, complex/non-string keys, every anchor and alias, merge keys (`<<`), explicit tags (including custom tags), extra documents, and parser warnings/errors before schema validation. Ordinary YAML 1.2 plain strings remain strings; do not infer dates, functions, or application-specific types. Integers used for issue, revision, or indexes must also be exactly representable safe integers in the implementation.