Repository navigation
Clean up the leftovers from the steering PR (#35 follow-up) #40
Description
Activity
- changed the title
[-]Remove the web thread outbox — #35 §0: the queue does not survive[/-][+]Clean up the leftovers from the steering PR (#35 follow-up)[/+]on Aug 2, 2026 Triage 2026-08-07 — real defects, but must land after #49
Sequencing, not scope, is the issue here. The A-items live in
apps/web/src/components/ChatView.tsx(A3, A4) anduseThreadOutboxDrain.ts/threadSettled.ts(A1, A2).ChatView.tsxtakes 20 upstream commits in the 103-commit backlog #49 has not merged — including48aa875c(removes the Build/Plan toggle from the composer),a8cd2ad2(plans fold into chat),a2ca89aa(+116 lines), and80720ad59(+53). It is also the file the last three failed sync runs stopped on, and the ledger's highest-risk row at 11466.Every line number in section A is against the 2026-08-02 base and will need re-deriving. Doing this work first means doing it twice.
Exception — C2 should move into the sync itself. "
SEAMS.md's Parallel paths table has one row… the steering path is a second submit path into a running turn and is not in it." Whoever resolves #49 has to reason through theChatView.tsxconflicts anyway, anda2ca89aaadds a second parallel-path candidate at the same time. Registering the steering path during that rebase costs almost nothing; doing it afterwards means re-reading the same diff.Ranking within section A, once unblocked
- A4 is the only correctness bug with a wrong-provider outcome — keying
canSteerActiveThreadon the composer'sselectedProviderrather thanactiveThread.session.providerNamecan classify a thread steer-capable and take the immediate-send path into Codex, which is the exact case the allowlist exists to prevent. It is also the one that makes the "fail closed" promise inb9d94d96cactually hold. Do this first. - A1 + A2 are one fix (stamp
createdAtat dispatch) and clear both the reload-order inversion and the expired anti-double-send gate. - A3 is a one-line move and matters mostly because the breadcrumb is the only client-side evidence during triage.
- A5 / A6 are data-loss-shaped but narrower.
Section B needs decisions rather than code and can be split off — B1 in particular (the honest fix is a separate
canDispatchQueuedTurnpredicate, not weakeningcanSettle) is a design call worth its own issue.C1 note:
48aa875canda8cd2ad2change the composer on web while mobile is untouched, so the web/mobile divergence recorded in C1 will look different after the sync. Re-check before recording it.Priority: P4, immediately after #49.
- A4 is the only correctness bug with a wrong-provider outcome — keying
PR #81 — section A items 1–4, plus C2
Unblocked: #49 has landed, so the "do this after the sync" sequencing no longer applies. Every line number in section A was re-derived against current
mainbefore touching anything.Landed in #81, in the order this issue's triage ranked them:
- A4 —
canSteerActiveThreadnow keys on the thread's persistedsession.providerName(new puresteerProviderBinding) instead of the composer'sselectedProvider. Confirmed the failure path exactly as described:selectedProvider=deriveLockedProvider(...) ?? resolveSelectableProvider(...), andresolveSelectableProvider's fallback is the first enabled provider, so a Codex thread with a disabled/unknown instance presented as steerable. Pinned by a test that asserts the contrast — same inputs, binding says no, picker says yes. - A1 + A2 — one fix, as predicted: the three outgoing commands are stamped at dispatch. The queue's own
createdAtis untouched and still orders the list. Worth recording:threadSettled.tsneeded no edit at all — it is not currently a fork row (git diff <merge-base> origin/main -- packages/client-runtime/src/state/threadSettled.tsis empty), so A2 cost zero seam. - A3 — the breadcrumb moved to each path's real dispatch point. It also gained a
steerProviderfield besideprovider, so the picker-vs-binding divergence that caused A4 shows up in the log line rather than having to be inferred from it. - C2 — the steering path now has its own row in the "Parallel paths" table, with the re-check instruction (there is no steering capability flag in the contracts, so a new adapter or a changed
sendTurnis invisible: no conflict, no type error, no failing test).
Seam:
ChatView.tsx+186/-1 → +210/-1. Deletions unchanged, so the additive invariant holds. Regenerating the header with the ledger's own recipe surfaced a pre-existing 13-line drift (+1957 documented vs +1970 measured, thepnpm-lock.yamlrow); corrected to +1994/-912 and noted inline.Still open on this issue: A5, A6, all of section B, and C1 (which the triage asked to re-check after the sync before recording). B1 in particular still reads like it wants its own issue rather than a patch here.
- A4 —
C1 re-checked after the sync — recording the divergence, not closing it
The triage asked for this to be re-derived post-#49 because
48aa875canda8cd2ad2changed the web composer while mobile was untouched. Done, against currentmain(merge-base78f462c4e).The divergence is real and it is wider than "mobile still queues". Mobile has no immediate-send path at all to steer into:
File Owner State apps/mobile/src/state/use-thread-composer-state.tsupstream, fork-untouched onSendMessagecallsenqueueThreadOutboxMessageunconditionally. There is no branch — every mobile submit is a queue submit, busy or idle.apps/mobile/src/features/threads/ThreadComposer.tsxupstream, already a fork row (+27/-4) renders the label apps/mobile/src/features/threads/composerSendLabel.tsfork-created connectionState !== "connected" || activeThreadBusy || queueCount > 0→"Queue". No steering escape.So the fork owns the label but not the path. The label is not the divergence — it is an accurate description of what mobile does.
Two consequences worth having on the record:
- Porting steering to mobile is a new fork seam on an upstream file, not a label change: it means adding an immediate-send branch to
use-thread-composer-state.ts, which the fork currently does not touch at all. On a file upstream is actively building its offline pending-task flow on. That is a decision, not a cleanup, which is why I have not made it. - Mobile's busy test is also narrower than web's. It is
session.status === "running" || "starting"— nolatestTurn.stateand noisRevertingCheckpoint, both of which web'sphase/composerActiveThreadBusyconsider. Any future port has to reconcile that too, or mobile will classify some running turns as idle.
Note also that
apps/web/src/outbox/composerSendLabel.logic.ts:5already carries a comment saying the two have diverged and pointing atcanSteerActiveThread, so the divergence is at least discoverable in-tree today.Not recorded in
SEAMS.md— the ledger tracks upstream-owned files the fork edits, and the substance here is a file the fork deliberately does not edit. If you want this written down somewhere durable, the natural home is this issue or a short note in the mobile composer's own file, and the choice between "port it" and "close it as intended divergence" is yours.- Porting steering to mobile is a new fork seam on an upstream file, not a label change: it means adding an immediate-send branch to
- added 9 commits that reference this issue
on Aug 12, 2026
Follow-up to #35. The steering work (
b9d94d96c,7e7f91ac6) changed what "Queue" means —from "the thread is busy" to "this cannot go out now" — and settled #35 §0: the queue is no
longer the primary mid-turn path, steering is.
This issue is the cleanup that transition left behind. Nothing is being reverted and no
commits are being dropped. The outbox stays, the queue UI stays, steering stays. What follows
is residue: defects the new path introduced, and places that still assume the old
queue-first model.
Line numbers are against
mainafter the 2026-08-02 upstream sync.A. Defects the steering path introduced or left behind
A1. Queued messages carry their enqueue-time
createdAtinto the transcript.useThreadOutboxDrain.ts:185, :201, :224all shipcreatedAt: queuedMessage.createdAt, and thesnapshot sorts
ORDER BY thread_id, created_at ASC, message_id. The live session looks correct(the reducer appends in arrival order), so the inversion only appears on reload, reconnect
resync, or reopening the thread — and then permanently. Reordering the queue makes it
reproducible: rows delivered in the new order, transcript in the old one.
Fix: stamp
createdAtat dispatch, which is what upstream's own immediate-send path does.A2. The same stale timestamp disables the anti-double-send gate.
packages/client-runtime/src/state/threadSettled.ts:37setsQUEUED_TURN_START_GRACE_MS = 2 * 60 * 1_000, and:64rejects anything outside that window.Any message that waited longer than two minutes — the normal case for a queued message — drains
with the guard already expired. Benign on steer-capable drivers; on Codex it can produce two
turn/startcalls, the second orphaning the first turn's events. Fixed by the same one-linechange as A1.
A3. The dispatch breadcrumb fires before every guard.
ChatView.tsx:4714callslogComposerDispatchnear the top ofonSend, above thethreadDetailLoading/isSendBusy/hasSendableContentchecks. Pressing Enter on an emptycomposer logs
composer dispatch: steerfor a submit that never happened. That matters morethan it sounds: per
outboxDiagnostics.tsthis log is the only client-side evidence ofwhether a message steered or queued, so a noisy one is actively misleading during triage.
Fix: move the call below the guards.
A4. The steer allowlist keys off the wrong provider.
ChatView.tsx:2150feedscanSteerActiveThreadthe composer'sselectedProvider, whosefallback resolves to the first enabled provider, while the server routes by the thread's
persisted binding. A thread whose provider instance was disabled or removed can be classified
steer-capable and take the immediate-send path into Codex — the exact case the allowlist exists
to prevent. Fix: key on
activeThread.session.providerName. This is what makes the"fail closed" promise in
b9d94d96cactually hold.A5. An edit saved during dispatch is silently discarded. The editing hold is only consulted
at head-selection time, so an edit started after
beginDispatchingQueuedMessageproceeds tosend;
threadOutboxManager.updatereturnsfalseandThreadOutboxQueueList.saveEditingignores the return, closing the editor as if it saved.
A6. A refused steer loses the composer text. Not invisible —
recoverTurnStartFailuresetsa session error, appends
provider.turn.start.failed, and unsticks the composer — but the textis gone from the composer, the draft store, and everywhere else.
B. Still-open behaviour worth a decision (no action proposed)
B1. The queue holds while the agent is blocked on an approval.
canSettlerefuses whilehasPendingApprovals, so a queued message waits while the agent is waiting on you. Steeringnarrowed this to Codex and non-empty queues, but it is the case most likely to read as "the
queue is broken." The honest fix is a separate
canDispatchQueuedTurnpredicate rather thanweakening
canSettle, which is shared with settle/snooze and mirrored server-side.B2. Steering into an approval-blocked turn is new and untested.
canSteerActiveThreadrequires only
phase === "running", and a session stays running while an approval is pending.The fork now steers into a state it still refuses to drain into.
B3. One mistimed second message poisons the rest of the turn into queue mode.
queueCount > 0is evaluated before the steer branch, andisSendBusystays true across theserver ack round trip (~100–500ms). Type again inside that window and everything queues for the
rest of the turn, signalled only by the glyph changing.
B4. "Type + Enter, then Stop" now loses the message. The muscle memory the previous build
taught (Enter queues → Stop interrupts → queue drains into a fresh turn) now enters the live
turn and then interrupts the turn that just received it. Probably costs a sentence of visible
copy on the running-turn Send button, which today carries only an aria-label.
C. Divergence and docs
C1. Web and mobile now disagree. Mobile still queues everything mid-turn
(
apps/mobile/src/features/threads/composerSendLabel.ts) while web steers. Note that the mobileoutbox is upstream's, not the fork's — all seven
apps/mobile/src/state/thread-outbox*.tsexist in
upstream/mainand upstream builds its offline pending-task flow on them — so this isnot a simple "port the web behaviour" job. Record the divergence or close it deliberately.
C2.
docs/t3x/SEAMS.md's "Parallel paths" table has one row. The steering path is a secondsubmit path into a running turn and is not in it. It should be, along with the upstream guards
it must mirror.
C3. `#35 §2 (queued images) and §3 (send-and-interrupt) are closed as won't-do — recorded
here so the decision is not re-litigated.
Note on seam cost
A1/A2 land in fork-owned files (
useThreadOutboxDrain.ts,threadSettled.tsis already a forkrow). A3/A4 are edits to lines the fork already owns inside
ChatView.tsx, so they do notenlarge the seam surface — but they are new commits touching the ledger's highest-risk file, so
keep them tight and update the row if the delta moves.