Repository navigation
cmux-next agent pane: cap crash reloads; a closed pane is not held by its handshake - #16463
Conversation
…ne held by its handshake Red first. A page whose web content process keeps crashing reloads forever, and the bridge keeps a closed pane (and its web view) alive for as long as the handshake waits on acpmux. The bridge's reply moves into reply(to:) unchanged so a test can drive it; AgentPaneCrashReloads is a stub that always reloads, today's behavior. 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 |
…ide #expect Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RYQHfug1ZVQDp4eWgwVUtD
…ake is pending Nothing held the pane strongly, so it was freed before the reply asked the host, and the test waited forever for a handshake that never came. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RYQHfug1ZVQDp4eWgwVUtD
… its handshake - After 3 web content crashes within 60 seconds, the pane stops reloading and shows "The agent pane crashed repeatedly." with a Reload button, which resets the count. - The bridge checks trust and replays the customization synchronously, then awaits only the model, so closing a tab frees its view and web view while acpmux is still starting for the handshake. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RYQHfug1ZVQDp4eWgwVUtD
|
Red at
The first bridge test (15bc3f7) didn't hold the pane, so it hung until the lane's 20-minute cap; d33888d fixed the test. The fix is pushed; the green result will follow. |
- The reload window is 10 minutes. A load-to-crash cycle can take 20 seconds or more, and with a 60 second window that slow loop never hit the cap. - The notice takes the pane's theme (appearance and secondary text color), not the system appearance, and is re-themed with the pane. - Its width inset yields in panes narrower than the inset. - The bridge test has a time limit and says which path it drives. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RYQHfug1ZVQDp4eWgwVUtD
|
Subagent review at ef89546: LGTM.
|
There was a problem hiding this comment.
3 issues found across 8 files
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="Packages/macOS/CmuxNext/Sources/CmuxNextAgentPane/AgentPaneNavigation.swift">
<violation number="1" location="Packages/macOS/CmuxNext/Sources/CmuxNextAgentPane/AgentPaneNavigation.swift:27">
P3: This comment still describes an unconditional reload, but `webContentProcessDidTerminate()` shows a notice after the crash cap instead. Document the capped behavior so this delegate does not promise a handshake when the pane is waiting for the user.</violation>
</file>
<file name="Packages/macOS/CmuxNext/Sources/CmuxNextAgentPane/AgentPaneCrashReloads.swift">
<violation number="1" location="Packages/macOS/CmuxNext/Sources/CmuxNextAgentPane/AgentPaneCrashReloads.swift:16">
P2: This elapsed-time cap uses wall-clock `Date`, so system-clock corrections can suppress reloads for too long or allow them too soon. Track crashes with `ContinuousClock.Instant` (or uptime) instead.</violation>
</file>
<file name="Packages/macOS/CmuxNext/Tests/CmuxNextAgentPaneTests/AgentPaneBridgeTests.swift">
<violation number="1" location="Packages/macOS/CmuxNext/Tests/CmuxNextAgentPaneTests/AgentPaneBridgeTests.swift:43">
P3: The test computes a temp path `agent-pane-bridge-test.html` and passes it as `AgentPaneSource.bundled(page)`, but never writes the file. `AgentPaneView.init` calls `source.load(into: webView)` → `loadFileURL(page, ...)` on a nonexistent file, so the pane tries to load a page that does not exist. The assertions pass only because the test drives `reply(to:)` directly and nothing reads the file, but the fixture is misleading and load behavior on a missing file is left to WebKit. Create the file (or load the real `AgentPaneView.bundledPage`).</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
|
|
||
| /// Records a crash at `now`; true when the page should reload. | ||
| mutating func shouldReload(at now: Date) -> Bool { | ||
| crashes.removeAll { now.timeIntervalSince($0) >= Self.window } |
There was a problem hiding this comment.
P2: This elapsed-time cap uses wall-clock Date, so system-clock corrections can suppress reloads for too long or allow them too soon. Track crashes with ContinuousClock.Instant (or uptime) instead.
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 Packages/macOS/CmuxNext/Sources/CmuxNextAgentPane/AgentPaneCrashReloads.swift, line 16:
<comment>This elapsed-time cap uses wall-clock `Date`, so system-clock corrections can suppress reloads for too long or allow them too soon. Track crashes with `ContinuousClock.Instant` (or uptime) instead.</comment>
<file context>
@@ -0,0 +1,20 @@
+
+ /// Records a crash at `now`; true when the page should reload.
+ mutating func shouldReload(at now: Date) -> Bool {
+ crashes.removeAll { now.timeIntervalSince($0) >= Self.window }
+ crashes.append(now)
+ return crashes.count <= Self.limit
</file context>
| /// A crashed web content process leaves a blank pane; the view reloads | ||
| /// the page, which asks for a fresh handshake and reattaches the session. |
There was a problem hiding this comment.
P3: This comment still describes an unconditional reload, but webContentProcessDidTerminate() shows a notice after the crash cap instead. Document the capped behavior so this delegate does not promise a handshake when the pane is waiting for the user.
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 Packages/macOS/CmuxNext/Sources/CmuxNextAgentPane/AgentPaneNavigation.swift, line 27:
<comment>This comment still describes an unconditional reload, but `webContentProcessDidTerminate()` shows a notice after the crash cap instead. Document the capped behavior so this delegate does not promise a handshake when the pane is waiting for the user.</comment>
<file context>
@@ -24,10 +24,10 @@ final class AgentPaneNavigation: NSObject, WKNavigationDelegate {
- /// A crashed web content process leaves a blank pane; reload the page,
- /// which asks for a fresh handshake and reattaches the session.
+ /// A crashed web content process leaves a blank pane; the view reloads
+ /// the page, which asks for a fresh handshake and reattaches the session.
func webViewWebContentProcessDidTerminate(_ webView: WKWebView) {
</file context>
| /// A crashed web content process leaves a blank pane; the view reloads | |
| /// the page, which asks for a fresh handshake and reattaches the session. | |
| /// A crashed web content process leaves a blank pane; the view reloads | |
| /// within the cap, then shows a Reload notice instead. |
| /// its trust check (a test cannot make a WKScriptMessage). | ||
| @Test(.timeLimit(.minutes(1))) func aClosedPaneIsFreedWhileItsHandshakeIsPending() async throws { | ||
| let host = GatedHost() | ||
| let page = FileManager.default.temporaryDirectory.appendingPathComponent("agent-pane-bridge-test.html") |
There was a problem hiding this comment.
P3: The test computes a temp path agent-pane-bridge-test.html and passes it as AgentPaneSource.bundled(page), but never writes the file. AgentPaneView.init calls source.load(into: webView) → loadFileURL(page, ...) on a nonexistent file, so the pane tries to load a page that does not exist. The assertions pass only because the test drives reply(to:) directly and nothing reads the file, but the fixture is misleading and load behavior on a missing file is left to WebKit. Create the file (or load the real AgentPaneView.bundledPage).
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 Packages/macOS/CmuxNext/Tests/CmuxNextAgentPaneTests/AgentPaneBridgeTests.swift, line 43:
<comment>The test computes a temp path `agent-pane-bridge-test.html` and passes it as `AgentPaneSource.bundled(page)`, but never writes the file. `AgentPaneView.init` calls `source.load(into: webView)` → `loadFileURL(page, ...)` on a nonexistent file, so the pane tries to load a page that does not exist. The assertions pass only because the test drives `reply(to:)` directly and nothing reads the file, but the fixture is misleading and load behavior on a missing file is left to WebKit. Create the file (or load the real `AgentPaneView.bundledPage`).</comment>
<file context>
@@ -0,0 +1,60 @@
+ /// its trust check (a test cannot make a WKScriptMessage).
+ @Test(.timeLimit(.minutes(1))) func aClosedPaneIsFreedWhileItsHandshakeIsPending() async throws {
+ let host = GatedHost()
+ let page = FileManager.default.temporaryDirectory.appendingPathComponent("agent-pane-bridge-test.html")
+ let weak = Weak()
+ var view: AgentPaneView? = try autoreleasepool {
</file context>
…ni (#16479) * test(ci): a stuck side job must not cancel a sibling running on a mini Fails on main: the rescue cancels the whole cmux-next run, including a swift test already running on a mini (#16463). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * ci: never rescue a stuck job while a sibling runs on a persistent runner The rescue cancels the whole run to move a job that never got a runner, so a sibling already running on a mini died with it and re-ran on Blacksmith (#16463, #16468). The stuck job now waits until no job of the run is running on a persistent runner; if the watch ends first it stays queued for the mini. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * ci: a sibling on a mini no longer hides a refused or held job The hold for a stuck job returned before the refused and held-in-setup checks, so a refusal next to a running mini job was never re-run and a held job was not rescued at the watch's end. Judge those first. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Summary
Two agent pane lifecycle fixes from the Swift lifecycle batch.
AgentPaneNavigationreloaded the page every time. A page that crashes as it loads looped forever.AgentPaneCrashReloadsnow allows 3 reloads per 10 minutes. After that the pane shows a native notice with a Reload button, which resets the count.AgentPaneBridgeheld a strong reference to the view acrossmodel.respond(to: .ready). That call can wait up to 20 seconds while acpmux starts, so a closed tab kept itsAgentPaneViewandWKWebViewalive until then. The bridge now checks trust and replays customization synchronously, then awaits only the model.Testing
Red first (commit
15bc3f73529), then the fix. Both run in thecmux-next swift testlane:AgentPaneCrashReloadsTests.aPageThatKeepsCrashingStopsReloadingAgentPaneBridgeTests.aClosedPaneIsFreedWhileItsHandshakeIsPendingRed and green results are in the comments below. The lane's known failures (RemoteMachineCompat, DaemonStartupState, ColumnCloseScroll) are unrelated and owned elsewhere.
Changelog
none
🤖 Generated with Claude Code
https://claude.ai/code/session_01RYQHfug1ZVQDp4eWgwVUtD