Commit Graph
2 Commits
Author SHA1 Message Date
Brennan Benson 533b0bd02e fix(native-chat): count a turn from the send that opened it (#21086)
* fix(native-chat): count a turn from the send that opened it

The live turn indicator switched on at the submission but anchored its clock at
the provider turn-open, so it jumped back by exactly the dispatch latency the
moment the turn opened. Measured on a real Claude session: the counter climbed to
"Working for 25s", reset to "Working for 0s", then settled "Worked for 26s" —
three readings of one turn, from two different instants.

The host now resolves the send that opened a turn and publishes it as an additive
optional `requestedAt` on the turn lifecycle row. `startedAt` keeps its exact
meaning, the provider turn-open, and is never rewritten, so clients that cannot be
upgraded see no change to any value they already read. Both providers write it;
it is omitted when no send can be named (provider-resumed turns, replayed history).

Readers take one origin, `requestedAt ?? startedAt`, for both the live counter and
the settled host interval, so the two cannot disagree. The provider's own reported
duration keeps outranking the host interval, unchanged.

The host-to-local clock conversion is now latched once per turn rather than
re-derived per render. `receivedAt - hostNow` carries that sample's one-way
delivery latency as well as skew, and the reducer replaces the sample on every
frame, so re-deriving imported fresh jitter and could move the anchor later — the
same class of backwards jump this change removes. With the conversion fixed, an
origin that improves moves the anchor earlier by exactly that much, so displayed
elapsed only grows. No monotonicity guard is added; the ordering is structural.

Desktop and mobile drove byte-identical copies of the timing hook, so both are
collapsed onto one React-free helper in shared.

Regression tests drive the origin resolution rather than an already-resolved
anchor, assert in milliseconds because second-flooring hides the sub-second case,
and include a deliberate host/client skew so a raw timestamp assignment cannot
pass on a machine where the two clocks agree.

* fix(native-chat): correlate Codex turn origins by echo

* fix(native-chat): preserve causal turn timing ownership

* fix(native-chat): keep settled turn timing continuous
2026-09-16 15:54:04 -07:00
Brennan BensonandMerge Sim 955051ded0 fix(codex): settle a structured send on admission, and stop minting a colliding identity (#20138)
* fix(codex): settle a structured send on admission, and stop minting a colliding identity

Two sends could be written into the journal under one durable identity.

Codex coalesces a mid-turn `turn/start` into the running turn rather than
refusing it -- measured against real `codex app-server` builds 0.147.0,
0.150.1 and 0.153.4, none of which refuse and none of which fire a second
`turn/started`. The dispatch path read the turn id from the turn/start
response and stamped every accepted send `ordinal: 0`. Since a coalesced
send gets the running turn's id back, two submissions persisted the same
`providerItemId`. That string is durable, and it is the key a restore uses
to match a submission against provider history, so the second message's real
history row matched nothing and rendered as an extra bubble on replay.

On 0.147.0 it is worse than a collision: the coalesced response returns a
turn id that never starts and never completes, so the persisted key named a
turn absent from history and NEITHER message could match.

Identity is now minted from the echoed user message at `identityFor` -- the
single point that mints the journal row's own identity -- so the settled key
is by construction the one replay computes, rather than a parallel
calculation that can drift.

Dispatch returns `admitted` when the transport write completes; identity
settles on the echo through a channel that did not previously exist for
Codex. Waiters are keyed by client message id instead of being shifted off
the front of an array by arrival order, and they are cleared on session
close and child exit -- previously a timeout was the only thing that ever
ended one.

`TURN_ID_WAIT_MS` is deleted. It was never reachable on any build measured:
`readCodexTurnId` returns non-null on all three, so the 10s wait never
fired. The comment justifying it claimed older builds acknowledge before the
id exists, which no tested build does.

Three comments asserting Codex answers a mid-turn send with `turn already
running` are corrected. Their only backing was a test fixture inventing that
error string. The correction is factual only -- every changed line in
`src/main/runtime/orchestration/` is a comment, and mid-turn delivery is
still refused for both providers. Whether that policy is right is a separate
question; it was resting on a false premise.

Known gap, stated rather than implied: this prevents new collisions and does
not repair journals already written with a colliding or phantom key. Those
conversations keep duplicating on restore. Repairing them means re-matching
persisted submissions against provider history and rewriting
`providerItemId` -- which is what `journal-submission-reconciler.ts` is
written for, and it still has no production caller.

* test(codex): drop the synchronous-accept contract and the colliding `:0` from the integration fakes

Three tests in the structured-session integration suites encoded the dispatch
contract this branch replaces, and two of them pinned the defect it fixes.

They asserted `agentSession.send` answers `dispatchState: 'accepted'` carrying
`providerItemId: codex:<thread>:<turn>:0` at send time. That ordinal was never
observed; it was stamped on every accepted send, which is exactly the collision
this branch removes -- a send coalesced into a running turn is answered with the
running turn's id, so two submissions persisted one durable key.

The visible failure was a 30s timeout rather than a failed assertion. The fake
client advertised no `agent-session.pending-send-result.v1`, and without it the
host holds the reply until the send settles: a shim for clients too old to
render a pending bubble. The fake provider then echoed the user message with no
`clientId`, so nothing could correlate that echo back to the submission, and the
wait ran to its own 30s ceiling. Real Codex sends `clientId` on that echo, and
the fake now does too, which is what makes it a model of the provider rather
than a sketch of one.

The identity assertion is kept rather than dropped. Each send now asserts
`pending` with no identity at admission, then asserts the submission settles
`accepted` at `codex:<thread>:<turn>:0` once the echo lands. Same ordinal, but
earned from `identityFor` on the echo -- the key a replay recomputes -- instead
of guessed from the turn/start response. Ablated: removing `clientId` from the
two echoes leaves both submissions `pending` and fails both assertions, so the
assertion is load-bearing and not satisfied by something incidental.

Both suites' client fixtures now advertise the capability set the desktop
renderer sends in `src/main/ipc/runtime.ts`, which is what these suites mean by
a client. The older-client settlement wait keeps its own coverage in
`src/main/runtime/rpc/methods/structured-agent-session.test.ts`.

`structured-agent-session-runtime-exit.test.ts` asserts `pending` for the same
reason; it drives the host directly, so it never took the compatibility path,
and what proves delivery there is still the turn the reacquired provider starts.

The replay suite's "without dispatching it twice" property is untouched: one
`turn/start` call, one replayed ledger row.

* fix(codex): preserve unsettled dispatch correlations

* test(codex): type the dispatch fixtures instead of asserting over them

main's new casting gate (#20367 base) flags type assertions on changed
lines. Replace them with checked types: the recording sink already
satisfies its interface, both CodexSession fixtures are now annotated and
carry real collaborators, the settlement assertion compares whole
identities, and the integration helper reads submissions through the
host's public journalSnapshot instead of its private session map.

* fix(test): merge the duplicate doubt-reasons import the merge left behind

Both sides added an import from journal-dispatch-doubt-reasons and the
merge kept both statements, which the whole-repo native plugin gate
refuses under --deny-warnings.

* test(codex): a Fast mode turn is admitted, not accepted

#20506 landed its Fast mode tests against the dispatch contract this
branch replaces: a Codex send now returns admitted and settles its
identity on the provider echo. The tier assertions the test exists for
are untouched.

---------

Co-authored-by: Merge Sim <sim@local>
2026-09-14 13:37:13 -07:00