Repository navigation
cmux-next agent pane: close the transcript gaps and stuck states left after #16426 - #16452
Merged
teamleaderleo merged 5 commits intoOct 1, 2026
Merged
Conversation
…ry 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
- 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
… 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
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 |
Collaborator
Author
|
Subagent review at 9497cbc: APPROVE. Checked against acpmux
Non-blocking:
|
…ext-agent-pane-client-fixes # Conflicts: # Packages/macOS/CmuxNext/Sources/CmuxNextAgentPane/Resources/agent-pane/index.html
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
5 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.test.ts">
<violation number="1" location="webviews/src/agent-session/acpmux/direct.test.ts:112">
P3: `dropAndReconnect` sleeps a fixed 300ms to outlast the client's first reconnect delay, which is currently 250ms in `scheduleReconnect` (direct.ts). The helper duplicates an internal constant, so changing `reconnectDelay` makes these three reconnect tests flaky or failing for a reason unrelated to the behavior under test. Wait for the actual reconnection instead: poll until `ScriptedSocket.current` is a new socket (bounded by a deadline), then drain microtasks.</violation>
</file>
<file name="webviews/src/agent-session/acpmux/transcript.test.tsx">
<violation number="1" location="webviews/src/agent-session/acpmux/transcript.test.tsx:16">
P3: The mock's `disconnect()` never removes callbacks, so `resizeCallbacks` keeps one entry per mounted pane for the module's lifetime; `resize()` in later tests would then fire stale callbacks against unmounted roots (which is why this test must reset `resizeCallbacks.length = 0` defensively), and unmount no longer mirrors real `ResizeObserver` behavior. Make `disconnect()` remove its own callback and drop `resizeCallbacks.length = 0`.</violation>
</file>
<file name="webviews/src/agent-session/acpmux/direct.ts">
<violation number="1" location="webviews/src/agent-session/acpmux/direct.ts:161">
P1: `AcpmuxApp` always supplies `onLost`, whose callback discards the old client and creates a new one with `lastSeq = 0`. The real pane therefore never fetches from its previous cursor; after a gap beyond the 400-event attach page, the replacement snapshot loses the previously visible transcript. Carry the cursor/history across the host handshake.</violation>
<violation number="2" location="webviews/src/agent-session/acpmux/direct.ts:277">
P2: This branch still publishes a stale watch result after the selection changes. A session created or selected while `_acpmux/watch` is pending can disappear from the picker; discard stale responses or merge them with the newer session list before emitting.</violation>
<violation number="3" location="webviews/src/agent-session/acpmux/direct.ts:351">
P2: Preserving a failed row while deleting its prompt correlation can render one accepted prompt twice after a socket loss. Keep the failed prompt correlation until a matching `user_message` settles it, or remove the local failed row when replay finds that prompt ID.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| this.selectedSessionId = initialSession(this.selectedSessionId, this.sessions, this.host.newSession); | ||
| if (this.selectedSessionId) await this.attach(this.selectedSessionId); | ||
| // A reconnect to the same session keeps its transcript; the attach page holds only the newest events. | ||
| const resumeAfter = this.lastSeq; |
There was a problem hiding this comment.
P1: AcpmuxApp always supplies onLost, whose callback discards the old client and creates a new one with lastSeq = 0. The real pane therefore never fetches from its previous cursor; after a gap beyond the 400-event attach page, the replacement snapshot loses the previously visible transcript. Carry the cursor/history across the host handshake.
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 161:
<comment>`AcpmuxApp` always supplies `onLost`, whose callback discards the old client and creates a new one with `lastSeq = 0`. The real pane therefore never fetches from its previous cursor; after a gap beyond the 400-event attach page, the replacement snapshot loses the previously visible transcript. Carry the cursor/history across the host handshake.</comment>
<file context>
@@ -154,10 +153,19 @@ export class AcpmuxDirectClient {
this.selectedSessionId = initialSession(this.selectedSessionId, this.sessions, this.host.newSession);
- if (this.selectedSessionId) await this.attach(this.selectedSessionId);
+ // A reconnect to the same session keeps its transcript; the attach page holds only the newest events.
+ const resumeAfter = this.lastSeq;
+ const sessionId = this.selectedSessionId;
+ const generation = this.selectionGeneration;
</file context>
| this.rows.clear(); for (const row of inFlight) this.rows.set(row.id, row); this.firstSeq = undefined; this.lastSeq = 0; this.turnOpen = false; this.streamingAssistant = undefined; this.streamingAssistantMessageId = undefined; this.streamingActivity = undefined; this.supersededMessageIds.clear(); this.messageRows.clear(); this.pendingPermission = undefined; | ||
| // A prompt still in flight keeps its optimistic row until an event settles it; a failed one stays to show it was not sent. | ||
| const inFlight = new Set(this.optimisticPromptRows.values()); | ||
| const local = [...this.rows.values()].filter((row) => row.failed || inFlight.has(row.id)); |
There was a problem hiding this comment.
P2: Preserving a failed row while deleting its prompt correlation can render one accepted prompt twice after a socket loss. Keep the failed prompt correlation until a matching user_message settles it, or remove the local failed row when replay finds that prompt ID.
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 351:
<comment>Preserving a failed row while deleting its prompt correlation can render one accepted prompt twice after a socket loss. Keep the failed prompt correlation until a matching `user_message` settles it, or remove the local failed row when replay finds that prompt ID.</comment>
<file context>
@@ -324,13 +346,37 @@ export class AcpmuxDirectClient {
- this.rows.clear(); for (const row of inFlight) this.rows.set(row.id, row); this.firstSeq = undefined; this.lastSeq = 0; this.turnOpen = false; this.streamingAssistant = undefined; this.streamingAssistantMessageId = undefined; this.streamingActivity = undefined; this.supersededMessageIds.clear(); this.messageRows.clear(); this.pendingPermission = undefined;
+ // A prompt still in flight keeps its optimistic row until an event settles it; a failed one stays to show it was not sent.
+ const inFlight = new Set(this.optimisticPromptRows.values());
+ const local = [...this.rows.values()].filter((row) => row.failed || inFlight.has(row.id));
+ this.rows.clear(); for (const row of local) this.rows.set(row.id, row); this.firstSeq = undefined; this.lastSeq = 0; this.turnOpen = false; this.streamingAssistant = undefined; this.streamingAssistantMessageId = undefined; this.streamingActivity = undefined; this.supersededMessageIds.clear(); this.messageRows.clear(); this.pendingPermission = undefined;
const events = [...this.events].sort((a, b) => a.seq - b.seq);
</file context>
| this.emit("session changed"); | ||
| const missing = this.selectedSessionId !== undefined && !this.sessions.some((session) => session.sessionId === this.selectedSessionId); | ||
| if (missing && generation === this.selectionGeneration) this.selectFallbackSession("session changed"); | ||
| else this.emit("session changed"); |
There was a problem hiding this comment.
P2: This branch still publishes a stale watch result after the selection changes. A session created or selected while _acpmux/watch is pending can disappear from the picker; discard stale responses or merge them with the newer session list before emitting.
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 277:
<comment>This branch still publishes a stale watch result after the selection changes. A session created or selected while `_acpmux/watch` is pending can disappear from the picker; discard stale responses or merge them with the newer session list before emitting.</comment>
<file context>
@@ -236,28 +244,46 @@ export class AcpmuxDirectClient {
- this.emit("session changed");
+ const missing = this.selectedSessionId !== undefined && !this.sessions.some((session) => session.sessionId === this.selectedSessionId);
+ if (missing && generation === this.selectionGeneration) this.selectFallbackSession("session changed");
+ else this.emit("session changed");
+ }
+
</file context>
| const dropAndReconnect = async () => { | ||
| const dropped = ScriptedSocket.current; | ||
| dropped.drop(); | ||
| await new Promise((resolve) => setTimeout(resolve, 300)); |
There was a problem hiding this comment.
P3: dropAndReconnect sleeps a fixed 300ms to outlast the client's first reconnect delay, which is currently 250ms in scheduleReconnect (direct.ts). The helper duplicates an internal constant, so changing reconnectDelay makes these three reconnect tests flaky or failing for a reason unrelated to the behavior under test. Wait for the actual reconnection instead: poll until ScriptedSocket.current is a new socket (bounded by a deadline), then drain microtasks.
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.test.ts, line 112:
<comment>`dropAndReconnect` sleeps a fixed 300ms to outlast the client's first reconnect delay, which is currently 250ms in `scheduleReconnect` (direct.ts). The helper duplicates an internal constant, so changing `reconnectDelay` makes these three reconnect tests flaky or failing for a reason unrelated to the behavior under test. Wait for the actual reconnection instead: poll until `ScriptedSocket.current` is a new socket (bounded by a deadline), then drain microtasks.</comment>
<file context>
@@ -91,12 +91,28 @@ class ScriptedSocket {
+const dropAndReconnect = async () => {
+ const dropped = ScriptedSocket.current;
+ dropped.drop();
+ await new Promise((resolve) => setTimeout(resolve, 300));
+ for (let pass = 0; pass < 5 && ScriptedSocket.current === dropped; pass += 1) await settle();
+ for (let pass = 0; pass < 5; pass += 1) await settle();
</file context>
| navigator: dom.window.navigator, | ||
| HTMLElement: dom.window.HTMLElement, | ||
| ResizeObserver: class { observe() {} disconnect() {} }, | ||
| ResizeObserver: class { constructor(callback: () => void) { resizeCallbacks.push(callback); } observe() {} disconnect() {} }, |
There was a problem hiding this comment.
P3: The mock's disconnect() never removes callbacks, so resizeCallbacks keeps one entry per mounted pane for the module's lifetime; resize() in later tests would then fire stale callbacks against unmounted roots (which is why this test must reset resizeCallbacks.length = 0 defensively), and unmount no longer mirrors real ResizeObserver behavior. Make disconnect() remove its own callback and drop resizeCallbacks.length = 0.
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/transcript.test.tsx, line 16:
<comment>The mock's `disconnect()` never removes callbacks, so `resizeCallbacks` keeps one entry per mounted pane for the module's lifetime; `resize()` in later tests would then fire stale callbacks against unmounted roots (which is why this test must reset `resizeCallbacks.length = 0` defensively), and unmount no longer mirrors real `ResizeObserver` behavior. Make `disconnect()` remove its own callback and drop `resizeCallbacks.length = 0`.</comment>
<file context>
@@ -5,13 +5,15 @@ import type { AcpmuxRow } from "./model";
navigator: dom.window.navigator,
HTMLElement: dom.window.HTMLElement,
- ResizeObserver: class { observe() {} disconnect() {} },
+ ResizeObserver: class { constructor(callback: () => void) { resizeCallbacks.push(callback); } observe() {} disconnect() {} },
requestAnimationFrame: (callback: FrameRequestCallback) => setTimeout(() => callback(0), 0) as unknown as number,
cancelAnimationFrame: (handle: number) => clearTimeout(handle),
</file context>
Suggested change
| ResizeObserver: class { constructor(callback: () => void) { resizeCallbacks.push(callback); } observe() {} disconnect() {} }, | |
| ResizeObserver: class { readonly callback: () => void; constructor(callback: () => void) { this.callback = callback; resizeCallbacks.push(callback); } observe() {} disconnect() { const index = resizeCallbacks.indexOf(this.callback); if (index !== -1) resizeCallbacks.splice(index, 1); } }, |
Collaborator
Author
Collaborator
Author
|
Added commit
Validation: |
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.
Summary
These are nine client fixes for the React agent pane, from the review notes collected in #16424.
Transcript and session state (
direct.ts):permission_decisionstill clears the permission.loadOlderuses the samerebuildKeepingLiveStatehelper.failed(ordisconnectedonce the socket is gone) instead of staying atresyncing.lastSeqand the re-attach page through_acpmux/events, instead of keeping only the newest 400 events. This and the next item only apply to clients created withoutonLost. The shipped pane passesonLostand builds a fresh client after a dropped socket, so today they cover the client's own reconnect path, not the app.close(): it rejects pending requests instead of leaving them unsettled.Transcript view (
App.tsx):register(): re-registering the same renderer with the samemeasureno longer re-renders the pane, so aregister()call inside render code doesn't loop.The bundled
agent-pane/index.htmlis rebuilt.Verification
bun test src/agent-session/acpmux/direct.test.ts src/agent-session/acpmux/transcript.test.tsxinwebviews/:bun run test(279 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
Closes the transcript gaps and stuck states the cmux-next agent pane had from #16426, adds regression tests for each case, and rebuilds the bundled
agent-pane/index.html.Session state
permission_decisionstill clears the permission.failed(ordisconnectedonce the socket is gone) instead of staying atresyncing.close()rejects pending requests, and failed prompt rows survive a rebuild.Transcript view
Written for commit b18e54b. Summary will update on new commits.