fix(chat): wait for what the person did, not the next host event - #63
Merged
Merged
Conversation
The per-turn cap stops a second block inside one reply, but each new turn hands out a fresh allowance — so an assistant that waits, times out, and waits again next turn blocks for the full timeout every time. A judged data-fair run spent 480s of its 567s in four such timeouts, writing a new "I'm still waiting" line after each one, while the timeout result was already telling it to end its reply and let the user act. That is the worst thing a person reads in that transcript, and it reproduces whenever anyone takes more than two minutes to click. The store now counts the events it has reported. While that count has not moved since a wait timed out, the person has done nothing at all, so blocking again can only run out another clock: the tool returns immediately and says so. Any event clears it, and the first wait of a conversation is untouched. A host that wires no turnId keeps the old behaviour, as before. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JGtNVPW8sF6PP2BHUvTX1u
wait_for_user_action resolved on the next event whatever it was. A judged run
showed why that cannot work: advance_to_confirmation reported {ready:false} on
its own result, then {ready:true} once a title-conflict API check came back —
the wizard was still catching up with the assistant's own action. The wait took
that refresh as the person acting, the model sensibly retried, and the retry
blocked for the full timeout: two thirds of the run.
Timing cannot separate that from an early click. The same refresh lands before
or after the wait depending on network latency, so "only events after the wait
started" would move the failure around rather than remove it. What distinguishes
the refresh from a click is what it is, not when it came — and the store already
separates the two kinds by design: an unkeyed transition is something that
happened, keyed state is what is true now.
So a wait now resolves on a transition, or on a `location` change (the one keyed
change that means the person left, cancelling a wait whatever it expected), and
on either whether already pending or arriving later. Other keyed state never
resolves a wait; it stays pending and reaches the model as a follower when the
wait completes. The documented path where the wait resolves on the creation
itself — never once exercised, because the refresh always got there first — is
now reachable.
The chat carries its own copy of the location key: this module is kept loadable
by the node unit runner, which has no built lib-vue entry to import the value
from. A unit test pins it to lib-vue's AGENT_LOCATION_KEY so the two cannot
drift — the same class of duplicated literal that recently had a sidecar record
the wrong persona model.
One earlier test encoded the old behaviour (a pending wizard refresh resolving a
wait) and is rewritten around its real intent with the right kind of event.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JGtNVPW8sF6PP2BHUvTX1u
… to end The unit tests pin the store's rule; this pins it through the real chat, the real page and the real channel, because a judged simulation cannot: whether the assistant declares a wait at all is the model's choice, and three runs on this build declared none. No new page code is needed. The dev wizard publishes its keyed state, and its Title field is bound to it, so typing there while a wait is armed is precisely the case that broke: a refresh the page emits on its own, not the person doing the thing the wait is for. The wait must stay armed through that refresh and through the step change behind Continue, resolve on the item-created transition behind Create, and deliver the state the page reached meanwhile alongside it. Verified to fail against the old "any event" rule — the wait resolved on the title refresh — and to pass with the new one. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JGtNVPW8sF6PP2BHUvTX1u
…nwhile A keyed refresh arriving during a wait is context, not the answer, so it stays pending. If the wait then timed out, the tool returned bare text and left that refresh in the buffer for the next carrier. In a judged run the wizard's ready:true, emitted one second after the wait was armed, reached the model two minutes later — folded into the person's next message — so the assistant announced the button was ready without ever having been told so. The timeout is a carrier too: it now delivers whatever arrived during the wait and empties the buffer. A bare timeout with nothing pending is unchanged. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JGtNVPW8sF6PP2BHUvTX1u
A judged run lost the race by about ten seconds. The assistant told the person the Create button was ready and declared a wait; the 120s window expired while they were still reading the proposal and finding the button. The creation event therefore never resolved a wait — it reached the model folded into the next turn's hidden context — and the person had to announce their own click and ask what had just been created, which is exactly the re-ask the mechanism exists to remove. 300s by default. A long wait costs little here: the composer stays usable during one, a message takes the turn back, and Stop is always reachable. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JGtNVPW8sF6PP2BHUvTX1u
The re-block guard asked "has anything been reported since my last timeout?" while the wait itself asks "did the person act?". So a late async refresh — advance_to_confirmation flipping ready:false to true when a title-conflict check comes back, the case resolvesWait was written for — cleared the guard and bought the next wait a full timeout on someone who was still away. That is the 480s-of-567s pathology re-entered through the front door, and at the new default window each round costs 300s. The branch's own argument settles it: timing cannot separate a refresh from a click, since the same refresh lands before or after the boundary with the network. eventSeq now counts by kind, like everything else here. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
# Conflicts: # ui/dts/auto-imports.d.ts
Nginx proxies to DEV_UI_PORT and nowhere else, so a vite that quietly moved to port+1 is not a degraded dev server, it is an invisible one: every request through the proxy answers 502 and the e2e suite fails as though the code were broken. A restart that races the instance it replaces hits this every time — the outgoing sockets are still bound a second later — and it cost most of an afternoon to recognise, because nothing in the failure points at a port. strictPort turns that into a refusal at startup, naming the port. It governs server.port only; an hmr.port collision can still surface as a hard start failure, which is the intended direction. Unrelated to the wait-turn work in the rest of the branch; kept as its own commit so it can be dropped or cherry-picked on its own. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
wait_for_user_actionnow tracks the person rather than the event stream.locationchange (they left). Other keyed state is a refresh — often the assistant's own tool call finishing late — and rides along as a follower instead of being mistaken for the answer.Why: waits were resolving on the wrong things and burning whole turns on timeouts. A judged run took a wizard's
{ready:true}refresh as the person clicking, retried, and blocked for the full timeout; another spent 480s of a 567s run in four consecutive timeouts, writing "I'm still waiting" after each. And at 120s the assistant timed out while the person was still reading the proposal and finding the button — 120s is a model's idea of a pause, not a person's.Heads-up: the window change cuts both ways — a wait that does get stuck now holds its turn for five minutes rather than two. It is bounded by
WAIT_MAX_SECONDS(600, unchanged), the composer stays usable throughout, and Stop is always reachable, but it is worth confirming nothing downstream assumes a shorter ceiling.Unrelated rider:
76eb103setsstrictPorton the dev-ui vite server, so a taken port fails at startup instead of drifting to the next one and leaving nginx proxying to nothing. Separate commit; drop it if you would rather it went in on its own.