Skip to content

fix(#1617 roots): server status seq + client floor/reconcile + relay reconnect cursor (#1636/#1637/#1638) - #1642

Merged
jeonghun-jj-lee merged 10 commits into
feature/free-tier-fleetfrom
fix/1617-staleness-roots
Sep 28, 2026
Merged

jeonghun-jj-lee merged 10 commits into
feature/free-tier-fleetfrom
fix/1617-staleness-roots

Conversation

@jeonghun-jj-lee

Copy link
Copy Markdown
Contributor

Closes #1636, closes #1637, closes #1638. Part of the #1617 staleness family.

What this is

The three root fixes for the recurring "session transcript + rail go stale while the agent is still running, heals only on manual reload" symptom. The three prior #1617 commits (d353ea57/1717c791/54e86222, already in feature/free-tier-fleet) were real robustifications of the backpressure/delta path but never touched these roots — this PR does. Surfaced by a full-path event-delivery sweep + adversarial deliberation; each issue survived a four-critic review that corrected two BLOCKING contradictions and several wrong-seam errors before implementation.

The three roots

Verification (all director-run, not self-reported)

  • Extension fast suite: 5134 pass / 0 fail (the 3 fleet-data-plane timeout-flakes confirmed 114/114 in isolation + clean on full re-run).
  • Engine: typecheck gate exit 0 (only the Base-pin drift: manifest upstream_base_sha stale vs absorbed overlay #1229 base-drift allowlist); seq/status effect tests 4/0; run-process 13/0 (the hang-regression guard).
  • App suite: 1475 pass / 18 fail — the known pre-existing set (i18n parity, editor-dom, sync-optimistic, desktop-native, session-relay routes, observe-element-offset, vscode-icon-theme); the new #1637 suites pass 44/0; zero new regressions.
  • Combined manifest verified at 900 overlay files; DAG honored (S2 built on the integrated S1 so honor-seq is genuinely exercised).

Notes for the reviewer

amicode-ci added 10 commits September 28, 2026 16:45
… composite-only reconnect gap

Step 0 (blocking prerequisite) resolved: NO REPLAY. The upstream /global/event
route does not honor ?lastEventID — its engine exercise contract (httpapi-exercise
index.ts:69-79) declares only a fresh-connect server.connected + live stream, no
replay param; and the aggregator/driver's own #1617 machinery exists precisely
because overflow is unrecoverable (no replay cursor). So the no-replay branch
applies: advance the per-namespace resume cursor AND emit a composite-only gap.

- Add SseFanInAggregator.cursorFor(ns): the live last-delivered id per namespace
  (advances per delivered frame on BOTH composite + verbatim paths; id-less frames
  never advance it). The driver reads it at re-open so a re-opened arm resumes from
  last-delivered, not the stale connect-time id (#1617 cursor invariant).
- Add SseFanInAggregator.reopenArm(ns): a NON-INITIAL re-open in composite mode
  (peerArms>0) emits exactly one id-less amicode.sync.gap naming that namespace to
  force a client refetch. Fleet-of-one (zero-peer) emits NONE — a synthetic frame
  in the verbatim stream would violate the #1264 byte-identity guard. An initial
  open (never-delivered arm) emits no gap.
- Route ALL control writes (gap/focus/honest-comment) through one return-checked,
  order-preserving helper backed by a single ordered pending stream (replacing the
  per-namespace buffers). A control frame written during backpressure never jumps
  ahead of buffered data frames nor is swallowed into a full sink; per-namespace
  overflow isolation is preserved by counting live per-ns data entries.
- resume() guard is now no-op WHEN ALREADY FLOWING (never 'when the sink is full').
  It still sets flowing=true and flushes the verbatim + pending buffers on drain —
  #1617's drain-flush behavior is unchanged.

Driver: reconcile-time re-opens (peer + local #1601 paths) go through reopenAndTrack
— it calls agg.reopenArm(ns) then opens from agg.cursorFor(ns) ?? connect id.

Tests: +9 aggregator (cursorFor advance/id-less; composite gap / fleet-of-one none /
initial-open none; resume no-op-when-flowing + drain-flush; control-after-data order)
and +4 driver (last-delivered resume local+solo; composite gap; fleet-of-one none +
byte-identical). Full extension suite 5124 pass / 0 fail; tsc clean.
…op-older guard

Session status transitions carried no per-session ordering key and several
callers of the single publish funnel (SessionStatus.set) race on value/order,
so a completing turn's idle could supersede a newer turn's busy on the wire —
a stale idle produced upstream with nothing to reject it.

- SessionStatus.set now stamps a monotonic per-session `seq` (service-owned,
  persistent across turns; never the per-turn runner) on every published
  transition — busy, idle, AND retry — and puts it on the event data.
- Drop-older guard: the turn owner (run-state, one runner per turn) draws a
  per-turn `generation` via SessionStatus.nextGeneration and threads it through
  that turn's busy and idle. An idle whose generation is strictly older than the
  session's high-water generation is dropped (a newer turn's busy superseded it),
  so a still-running session's last published status is never a stale idle. A
  busy following a busy within one turn (run-state onBusy + processor busy) shares
  the turn generation, so a legitimate idle is never lost.
- cancel with no runner publishes a reconcile idle only when the session is not
  currently busy, so it cannot supersede a newer generation.
- schema: session.status event data gains `seq` (NonNegativeInt); SDK v2 types
  regenerated so downstream (#1637) sees `seq`.
- tests: wire-asserted via EventV2Bridge.listen (not SessionStatus.get, which
  synthesizes idle and deletes on idle); transport/acp status fixtures carry seq.
…lears + honor status seq

- session_working() rises on session.execution.started (brackets every turn
  shape incl. no-part turns) via a turnActive flag, not just the #1617 per-delta
  floor; clears on terminal execution events OR fallbacks (session.error,
  eviction/session.deleted, an idle status frame, a bounded timeout).
- session.error and the bounded timeout also settle a stuck-busy status to idle
  so neither floor wedges the rail 'working' after a swallowed terminal.
- honor #1636 seq: a session.status frame with seq <= last seen for the session
  is discarded (strictly-greater wins); absent seq is applied (pre-#1636 compat).
- shared sessionWorkingFromStore predicate: working when turn flag up OR status
  not idle; both session_working impls read it.
- State.session_turn_active; child-store session_working honors it.
- applyDirectoryEvent handles session.execution.{started,succeeded,failed,
  interrupted} and session.error to set/clear the flag, and honors status seq
  (drops a stale idle). dropSessionCaches clears the flag with the session.
… session_working

- message-timeline workingTurn reads sync().data.session_working(id) not raw
  status type — a stray idle during a live turn (incl. no-part) no longer flips
  the per-turn indicator to done.
- session-context-tab busy memo reads session_working likewise.
- the diff-refetch (session.tsx:1040) + todo-refetch (session.tsx:1344) readers
  stay on raw status (they want the real idle edge) — deliberately NOT migrated.
…watchdog resync bump

- ServerSession.reconcileStatuses(statuses, {fetchedAt}) corrects a stale idle
  AND a stale busy from the tri-state /session/status map; recency-guarded so an
  out-of-order snapshot (older than the last local status mutation) cannot
  downgrade a live busy to idle. Status writers stamp lastStatusMutationAt.
- server-sync reconcileFromStatus() calls it on server.connected + amicode.sync.gap
  (using /session/status, NOT running-only /session/active).
- watchdog forceReconnect bumps resyncCount when it aborts a parked attempt, so
  the rail self-heals a stranded view on a silent-socket forced resync.
@coderabbitai

coderabbitai Bot commented Sep 28, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 0359cd35-74c9-4873-aaa5-0b534a8eb5e8

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant