Skip to content

Server: session status has no ordering key + multiple emitters → stale idle while agent runs #1636

Description

@jeonghun-jj-lee

Important

Problem — The session status a client renders (working spinner, per-turn thought-rail dot) can go stale — showing idle while the agent is still running — because status transitions for a session carry no per-session ordering key and are produced by several callers racing on value/order. A finishing turn's idle and a new turn's busy are set from different fibers with nothing that lets a consumer tell a late idle has been superseded. (Note: there is already exactly one publish funnel — SessionStatus.set → the session.status event; the race is among its callers, not multiple emitters. The fix therefore stamps ordering at that funnel.) This is the root that survives every client/relay mitigation of the #1617 family: a stale idle is genuinely produced upstream, and the client is given nothing to reject it with.
Approach — Give session status a monotonic per-session sequence stamped in the one place all callers already funnel through — SessionStatus.set — plus a drop-older guard at that same funnel. (1) The SessionStatus service owns a persistent per-session counter in its instance state (which survives across turns — the runner does not, it is deleted every turn, so it cannot own the counter). (2) set increments seq on every transition and includes it on the published event. (3) set drops a transition whose generation/seq context is older than the latest recorded for that session, so a completing turn's idle cannot overwrite a newer turn's busy. (4) Regenerate the client SDK/protocol so the new field is typed downstream. The client-side half — honoring seq to discard stale status — is the sibling issue (#1637), which depends on this for the field to exist.
Scope — in: the SessionStatus service (set/publish and its per-session state gaining a seq counter + drop-older guard covering all transition types — busy, idle, and retry); the status-event schema gaining seq on the event data; the SDK/protocol regeneration; the server-side emission test asserting on the wire. out: the fan-in relay (#1638); the client's consumption of seq (#1637); message/part event ordering (unchanged); how idle/busy/retry are computed — only how they are ordered and stamped at the funnel. Explicitly NOT scoped as "route everything through the runner": that is infeasible (see Key Decisions) and is replaced by "stamp at SessionStatus.set."
Assumptions — SessionStatus.set is the single funnel every status caller already passes through (processor busy/idle/retry, run-state onBusy/onIdle/cancel, prompt-loop busy), so stamping there covers all of them without collapsing call sites; the event is serialized whole to the SSE stream (a field added to the event data rides to the client without a field-mapper change); the SDK is generated from the schema and must be regenerated; the per-session counter belongs in the service's persistent instance state, never in the per-turn runner.

Acceptance Criteria

  • Every published session.status event — for every transition type including retry — carries a seq that is strictly increasing per session across all transitions for that session's lifetime (not reset per turn).
  • The seq counter and the drop-older guard live in SessionStatus.set / its persistent per-session state; no per-turn structure (the runner) owns the counter.
  • A completing turn's idle presented to set after a newer turn's busy (older generation) is dropped at the funnel, so the last status published for a still-running session is never idle.
  • Given two turns racing for one session, the final published status is busy with a seq greater than the dropped idle, in every interleaving the test exercises — asserted on the emitted event stream, not via SessionStatus.get (which synthesizes a default idle and deletes the entry on idle, so get mis-measures).
  • cancel when no runner is present does not publish an idle that supersedes a newer generation (guarded by the same drop-older rule).
  • The generated client SDK/protocol types include seq on the status event.

Testing Decisions

  • Reuse-first: extend the processor's effect-level test that already owns a controllable event sink via EventV2Bridge.Service.listen (it asserts on the emitted session.status event stream — the correct place, since SessionStatus.get synthesizes idle and deletes on idle). The retry path has an existing effect test to extend for the retry-carries-seq criterion. (There is no standalone status/run-state unit suite — do not claim one.)
  • New ordering test: drive two overlapping turn lifecycles for one session; assert the emitted status seq is strictly monotonic and never ends on a stale idle; assert retry transitions also carry seq.
  • Assert the schema round-trips seq and the SDK regen picks it up.

Key Decisions

Constraints & Invariants

  • Status ordering is enforceable end-to-end: a client tracking max seq per session can discard any status with seq <= what it has seen.
  • No change to message/part event ordering or ids.
  • Idle/busy/retry semantics are unchanged; only ordering, stamping, and the drop-older guard change.
  • An errored / overflow-aborted / cancelled turn still reaches idle exactly once (a retained emitter path, now stamped) — the fix must not lose a legitimate idle while suppressing a stale one.

Prior Art

Source

Notes

Activity

  1. added
    bugSomething isn't working
    afkImplementable without human interaction
    on Sep 28, 2026
  2. jeonghun-jj-lee commented on Sep 28, 2026

    @jeonghun-jj-lee
    ContributorAuthor

    Client sibling that consumes this seq: #1637. Relay sibling (independent): #1638. Part of the #1617 staleness family.

  3. jeonghun-jj-lee commented on Sep 28, 2026

    @jeonghun-jj-lee
    ContributorAuthor

    Deliberated (adversarial review, by hand — amico spec review tooling unavailable, so no mechanical tier-1 gate; a weaker claim than a tooled review). Two independent critics (coverage lens + schema/contradiction lens) reading only this issue and the raw engine code. Body revised to close their findings:

    • Missed emitters: there are 8 status writers, not 3 — the fix now covers processor busy (turn entry) and retry (which is not a busy/idle transition and cannot be a runner hook).
    • Wrong seam (the load-bearing correction): the runner is deleted every turn, so a seq counter there resets — it cannot be monotonic across turns, and its counter is closure-private and unreadable from the processor fiber. The counter + drop-older guard now live in SessionStatus.set, the single publish funnel all callers already pass through.
    • Premise corrected: there is already exactly one emitter; the race is value/ordering among its callers. Reframed accordingly.
    • Test target: assert on the emitted event stream (EventV2Bridge.listen), not SessionStatus.get (which synthesizes a default idle and deletes on idle). The claimed 'existing status/run-state suite' does not exist; pointed at the real effect-level tests.
    • SDK/protocol regeneration added as an explicit step. No blocking contradiction found.
  4. added a commit that references this issue on Sep 28, 2026
  5. jeonghun-jj-lee commented on Sep 28, 2026

    @jeonghun-jj-lee
    ContributorAuthor

    Landed via #1642 (merge commit f4518fe) into feature/free-tier-fleet. Director-run gates + CI all green (engine-tests, fast, build-binary, schema-roundtrip, boot-smoke ×3, vsix-gate). Not auto-closed because the PR targets feature/free-tier-fleet, not the default branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    afkImplementable without human interactionbugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions