fix(compaction): review round 6 — defer-and-drain send windows, workstream-scoped notify, ERROR badge survives /compact

Replace park-and-abandon /send semantics with defer-and-drain: a send
landing in a command window is answered {status: queued, msg_id}
immediately and dispatched full-fidelity by a per-workstream drain
thread when the window closes. Parking encoded client disconnect as
message retraction — true only for the composer's ✕-abort; every
bounded caller (coordinator client and console proxy at timeout=30,
SDKs, stock proxies) timed out and lost its message for the whole
window, and the compensating client machinery was racy (one-shot
sendAbortMs sample) and over-broad (_sendAbort fired on the
interjection path, dispatching dismissed messages while showing a
connection error). Dismissal is now uniformly bind() → DELETE, with a
fall-through that retracts pending entries; retracting an
attachment-bearing deferred send surfaces the discarded-attachments
consequence. The drain claims entries under ws._lock immediately
before dispatch (DELETE can never remove an in-flight message),
refuses the truncating interjection fallback for oversized or
attachment entries atomically inside the enqueue callback, and never
gives up while the workstream lives; durability is documented as
node-local at-most-once. sendAbortMs, _sendAbort, the 600s bound and
the park loop are deleted; route and drain share one dispatch
implementation (spawn metrics included).

Also: the initial-send completion notify is un-gated from slot
ownership (_fire_notify_targets has exactly one call site — successor
turns never notify, so the round-5 guard prevented a duplicate that
cannot exist while converting force-cancel into permanent notification
loss for scheduled workstreams); /compact on an ERROR workstream
restores the badge instead of stamping idle over it; duck-typed
SessionUIs without on_compaction get the classic on_info lines back
via a shared renderer (superseded OK ends included — a committed swap
must never be silent; pre-1.8 SSE clients are deliberately not
dual-emitted, documented as a 1.8 breaking change); failed-end notice
suppression is computed once by the emitter as a notice bool on the
end event (SDK py+ts), replacing the hand-synced cli/JS policy while
the panes keep their pane-local card-ownership clause.
This commit is contained in:
Patrick Buckley
2026-07-16 21:35:07 -07:00
parent 1dbf7f410c
commit e99673eb0c
20 changed files with 1395 additions and 509 deletions
+29 -20
View File
@@ -482,13 +482,16 @@ carries `before_tokens`/`after_tokens` and the produced `summary`;
`"cancelled"` / `"error"`) and a human-readable `message` — for
`reason: "error"` the same message is also emitted as a paired typed
`error` event (that is the renderable error surface; the end event is
card-teardown). Every end (ok or failed) carries `trigger`, and every
event carries `compaction_id` — an opaque integer correlating the
start/progress/end of one compaction run (a client that force-stopped one
compaction can use it to ignore stragglers from the abandoned run). End
events also carry `superseded`: `true` marks a force-abandoned compaction
retiring after a successor generation took over — skip failure notices for
those (an OK end's result card still stands: the history swap happened).
card-teardown). Failed ends also carry `notice`: the emitter-computed
display verdict — show `message` only when it is `true` (the server
suppresses error-reason, superseded, and cancelled-auto notices once,
centrally, so clients don't re-derive that policy). Every end (ok or
failed) carries `trigger`, and every event carries `compaction_id` — an
opaque integer correlating the start/progress/end of one compaction run (a
client that force-stopped one compaction can use it to ignore stragglers
from the abandoned run). End events also carry `superseded`: `true` marks
a force-abandoned compaction retiring after a successor generation took
over (an OK end's result card still stands: the history swap happened).
Superseded start/progress events are never emitted.
Exactly one `start` and one `end` are emitted per attempt,
so clients can key an in-progress affordance (progress bar) on the pair. A
@@ -875,19 +878,25 @@ synchronous:
endpoint executed commands unconditionally mid-turn; the 409 makes the
refusal loud for callers that only check the HTTP status.)
While a command holds the slot, `POST .../send` requests **park** server-side
and dispatch as ordinary full-fidelity sends when the command's window closes
— they are never routed through the mid-turn interjection queue (no length
cap, no cross-user rejection). A client that aborts a parked send before the
window closes abandons it: the message is not dispatched. Clients must
therefore bound parked sends generously: the bundled web composer uses a
10-minute abort while a compaction progress card is visible (its usual bound
is ~15 s, sized for wedged-node detection), and dismissing a queued bubble
aborts the in-flight POST so a dismissed message can't dispatch later.
Deployment note: a reverse proxy's read timeout bounds the effective park —
behind a stock 60 s proxy, sends parked longer than that fail at the proxy
(the park sees the disconnect and abandons; nothing is dispatched — resend
after the command completes, or raise the proxy read timeout).
While a command holds the slot, `POST .../send` requests are **deferred**:
the server answers `{"status": "queued", "msg_id": ...}` immediately and
dispatches the message as an ordinary full-fidelity send (attachments and
sender identity included) when the command's window closes — it is never
routed through the mid-turn interjection queue (no length cap, no cross-user
rejection). The response arrives within normal round-trip time, so
timeout-bounded clients (SDKs, proxies, the coordinator) need no special
handling. To retract a deferred send before it dispatches, issue the same
`DELETE .../send` with its `msg_id` used for queued interjections —
`{"status": "removed"}` confirms it will not dispatch; `"not_found"` means
it already dispatched (or is dispatching). Retracting a deferred send
discards any attachments it carried; re-attach to send them again.
Durability: deferred sends are **node-local and in-memory** (the same
lifetime as the interjection queue). `"queued"` is at-most-once intake, not
durable acceptance — if the workstream is closed or the node restarts before
the window ends, the message is dropped. Anything that must survive a
restart should be re-sent after confirming dispatch (the turn appears on the
SSE stream / in `/history`).
**Request body:**