Repository navigation
cmux-next agent pane: keep the session picker, history and lag recovery in sync with the daemon - #16426
Conversation
…readiness, lag recovery and select reset Seven client tests drive AcpmuxDirectClient over a scripted socket. All fail on this commit: an unselected purge leaves the picker stale, history reattaches instead of paging _acpmux/events, a request on a closed socket resolves silently, _acpmux/lagged is ignored, and select keeps the old session's queue, summary and permission. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RYQHfug1ZVQDp4eWgwVUtD
sessionChanged emitted only when the selected session changed, so a session created, updated or purged elsewhere left the picker stale until some other event. It now emits after every list mutation, including the unselected purge branch. Ported from #16203 (0b8b161). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RYQHfug1ZVQDp4eWgwVUtD
select cleared only rows, sequence, turn, streaming and permission, so the old session's queue, summary, optimistic prompt rows, superseded IDs, message mapping and assistant message ID lingered until the new attach replaced some of them. One resetSessionState helper now does the full reset for select, the open missing-session path and the selected purge path. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RYQHfug1ZVQDp4eWgwVUtD
request sent through socket?.send and waited forever when the socket was absent or not open. It now rejects at once. The client also handles _acpmux/lagged: it fetches _acpmux/events after the last seen sequence, drops the reply if the session or selection generation changed, and merges the missed events into the transcript instead of clearing it. rebuild keeps optimistic rows for prompts still in flight, so a lag replay no longer blanks a prompt the user just sent; the lag test now covers that. Ported from #16203 (a974f73, 8fc1387). The reconnect test's fake socket gains readyState so the guard sees an open socket. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RYQHfug1ZVQDp4eWgwVUtD
loadOlder reattached with beforeSeq, which replaced the live summary and queue with the attach reply and rebuilt away the pending permission. It now asks _acpmux/events for the page before the first loaded sequence, captures the session and selection generation first and drops the reply if either changed, merges the older events, and restores the live summary, queue and permission after the rebuild. History stops when the daemon reports no more, returns an empty page, or reaches sequence 1. attach loses its now-unused beforeSeq parameter. Ported from #16203 (8fc1387, guard from 4d91ad2). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RYQHfug1ZVQDp4eWgwVUtD
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RYQHfug1ZVQDp4eWgwVUtD
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository: manaflow-ai/cmux/.coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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. Comment |
…apes
The acpmux daemon sends _acpmux/lagged as {sessionIds, watch, dropped}
(agent-gui) or {dropped} (older daemons), never {sessionId}, and ends
_acpmux/events pages with `more`, not `hasMore`. With the real shapes the
client ignores every lag notice, stops lag recovery after one page,
leaves the picker stale after a watch lag, and keeps offering older
history after `more: false`. Six tests fail here.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RYQHfug1ZVQDp4eWgwVUtD
Lag recovery read a sessionId the daemon never sends, so it never ran. It now resyncs the selected session when the notice lists it, or when the notice has no sessionIds (older daemons send only dropped), pages _acpmux/events until `more` is false, and rereads the session list when the watch stream lagged. History paging reads `more` instead of `hasMore`. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RYQHfug1ZVQDp4eWgwVUtD
|
Subagent review at 7eed115: REQUEST_CHANGES. |
…ore attach Lag recovery pages from this.lastSeq, which live events advance between pages, so a live event that lands mid-resync makes the next page skip the rest of the gap. A lag notice that arrives while a newly selected session's attach is in flight resyncs from seq 0. Both tests fail here. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RYQHfug1ZVQDp4eWgwVUtD
The resync loop now advances a local cursor to the newest seq of each page instead of reading lastSeq, which live events move ahead mid-resync. A lag notice is ignored until the selected session's attach reply has landed, since that reply already carries the latest events and there is no cursor before it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RYQHfug1ZVQDp4eWgwVUtD
|
Subagent review at 4d6b7f5: REQUEST_CHANGES. The lag-resync loop paged from
|
|
Subagent review at 276e7e8: APPROVE.
Non-blocking, added to #16424:
|
There was a problem hiding this comment.
2 issues found and verified against the latest diff
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="webviews/src/agent-session/acpmux/direct.ts">
<violation number="1" location="webviews/src/agent-session/acpmux/direct.ts:249">
P1: Lag recovery can erase a live permission prompt when the prompt exists only in the `_acpmux/permission_pending` notification: `rebuild()` clears `pendingPermission`, and this path does not preserve it as `loadOlder()` does. Preserve or reconcile the current permission while replaying missed events.</violation>
<violation number="2" location="webviews/src/agent-session/acpmux/direct.ts:259">
P2: A watch lag that dropped the selected session’s purge leaves that deleted session selected after this refresh. Reconcile a missing `selectedSessionId` using the same fallback/reset/attach behavior as the purge handler.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| if (generation !== this.selectionGeneration || this.selectedSessionId !== sessionId) return; | ||
| const missed: EventRecord[] = result?.events ?? []; | ||
| this.events = mergeEventRecords(this.events, missed); | ||
| this.rebuild(); |
There was a problem hiding this comment.
P1: Lag recovery can erase a live permission prompt when the prompt exists only in the _acpmux/permission_pending notification: rebuild() clears pendingPermission, and this path does not preserve it as loadOlder() does. Preserve or reconcile the current permission while replaying missed events.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At webviews/src/agent-session/acpmux/direct.ts, line 245:
<comment>Lag recovery can erase a live permission prompt when the prompt exists only in the `_acpmux/permission_pending` notification: `rebuild()` clears `pendingPermission`, and this path does not preserve it as `loadOlder()` does. Preserve or reconcile the current permission while replaying missed events.</comment>
<file context>
@@ -213,21 +220,53 @@ export class AcpmuxDirectClient {
+ if (generation !== this.selectionGeneration || this.selectedSessionId !== sessionId) return;
+ const missed: EventRecord[] = result?.events ?? [];
+ this.events = mergeEventRecords(this.events, missed);
+ this.rebuild();
+ if (result?.more !== true || missed.length === 0) break;
+ }
</file context>
| /// session_changed notices dropped by a watch lag: reread the whole list. | ||
| private async refreshSessions(): Promise<void> { | ||
| const watched = await this.request("_acpmux/watch", { enabled: true }); | ||
| this.sessions = (watched?.sessions ?? []).filter((session: Session) => session.sessionId); |
There was a problem hiding this comment.
P2: A watch lag that dropped the selected session’s purge leaves that deleted session selected after this refresh. Reconcile a missing selectedSessionId using the same fallback/reset/attach behavior as the purge handler.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At webviews/src/agent-session/acpmux/direct.ts, line 254:
<comment>A watch lag that dropped the selected session’s purge leaves that deleted session selected after this refresh. Reconcile a missing `selectedSessionId` using the same fallback/reset/attach behavior as the purge handler.</comment>
<file context>
@@ -213,21 +220,53 @@ export class AcpmuxDirectClient {
+ /// session_changed notices dropped by a watch lag: reread the whole list.
+ private async refreshSessions(): Promise<void> {
+ const watched = await this.request("_acpmux/watch", { enabled: true });
+ this.sessions = (watched?.sessions ?? []).filter((session: Session) => session.sessionId);
+ this.emit("session changed");
}
</file context>
…ext-agent-pane-ts-followups # Conflicts: # Packages/macOS/CmuxNext/Sources/CmuxNextAgentPane/Resources/agent-pane/index.html
|
89fb8df merges feat-cmux-next (#16425) into the reviewed head 276e7e8. The only conflict was the generated agent-pane/index.html, rebuilt from the merged sources; |
|
Merging with only |
… after #16426 (#16452) * cmux-next agent pane: add failing tests for lag, reconnect and registry gaps Covers the #16424 follow-ups: lag resync dropping the live permission, a failed lag fetch stuck on "resyncing", a reconnect that switches the missing selection without bumping the generation, the event gap after a reconnect re-attach, a watch lag dropping the selected session, close() leaving requests pending, failed prompt rows lost on rebuild, the open-at-latest jump on a height-only shrink, and register() re-rendering for an unchanged renderer. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RYQHfug1ZVQDp4eWgwVUtD * cmux-next agent pane: close the direct client's lag and reconnect gaps - Lag resync keeps the live summary, queue and permission through the rebuild (shared with loadOlder) and replays the missed mux events on top, so a missed permission decision or status still lands. - A failed lag fetch ends "resyncing" with "failed", or "disconnected" when the socket is gone. - A connect that finds the selected session missing bumps the selection generation, so a lag notice before the new attach waits for it. - A reconnect re-attach of the same session fetches the events between the old cursor and the attach page. - A watch-lag refresh that no longer lists the selected session falls back to the most recent one, like a purge. - close() rejects pending requests; onclose ignores a released socket. - rebuild() keeps failed optimistic prompt rows. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RYQHfug1ZVQDp4eWgwVUtD * cmux-next agent pane: open at latest on a height-only shrink and skip no-op registers The open-at-latest effect also runs when the viewport height changes, so rows that fit and then overflow after a vertical shrink still jump to the latest row once. register() returns early when the kind already has that renderer and measure, so a user renderer registering from an effect does not loop. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RYQHfug1ZVQDp4eWgwVUtD * cmux-next agent pane: rebuild the bundled pane Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RYQHfug1ZVQDp4eWgwVUtD --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Summary
These are the four client-side fixes from #16203 that the React agent pane (#16231) didn't include, ported to
webviews/src/agent-session/acpmux/direct.ts:_acpmux/eventsinstead of reattaching, and stops when the daemon repliesmore: false. Reattaching replaced the live summary and queue and dropped a pending permission. A page that lands after the selection changed is discarded._acpmux/lagged: it fetches the missed events and merges them, instead of being ignored. It follows the daemon's real notice shapes:{sessionIds, watch, dropped}from agent-gui daemons and{dropped}from older ones, where nosessionIdsmeans the selected session. It pages from its own cursor untilmoreis false, waits for the selected session's attach before resyncing, and rereads the session list when the watch stream lagged. Prompts still in flight keep their optimistic rows.resetSessionStatehelper.The bundled
agent-pane/index.htmlis rebuilt.Verification
bun test src/agent-session/acpmux/inwebviews/:sessionIdthe daemon never sends, the fixtures switched to the real shapes and 6 tests failed. 4d6b7f5 fixes them. A second review found that lag paging skipped ranges when live events landed mid-resync; 2c57221 (2 red tests) and 276e7e8 fix that.bun run test(268 pass), typecheck andlint:cipass.build-agent-pane-web.sh --checkandbuild-webviews-app.sh --checkpass.Changelog
none
🤖 Generated with Claude Code
https://claude.ai/code/session_01RYQHfug1ZVQDp4eWgwVUtD
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Summary by cubic
Keeps the cmux-next agent pane's session picker, history paging, and lag recovery in sync with the daemon, porting four client-side fixes from #16203 into
AcpmuxDirectClientand matching the daemon's actual wire shapes.Behavior changes
_acpmux/eventsinstead of reattaching, preserving the live summary, queue, and pending permission; a page landing after a selection change is discarded, and paging stops whenmoreis false._acpmux/laggedfollows the daemon's shapes ({sessionIds, watch, dropped}or{dropped}only): it resyncs the selected session, merges missed events, and keeps in-flight prompts' optimistic rows.lastSeq, so a live event landing mid-resync no longer makes the next page skip the rest of the gap, and it ignores a lag notice until the selected session's attach reply has landed.resetSessionStatehelper.agent-pane/index.html.The branch is merged up to date with
feat-cmux-next; the only change from that merge is the rebuilt bundle.Written for commit 89fb8df. Summary will update on new commits.