Repository navigation
browser: expose webview lifecycle state in top - #4243
austinywang merged 5 commits into
Conversation
|
@lidge-jun is attempting to deploy a commit to the Manaflow Team on Vercel. A member of the Team first needs to authorize it. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds persistent WebView lifecycle tracking to BrowserPanel (new enum, visibility metadata, payload export), observes UI and portal visibility in BrowserPanelView, reports lifecycle data in TerminalController workspace payloads, and adds unit tests for lifecycle transitions. ChangesWebView Lifecycle State Tracking
Sequence DiagramsequenceDiagram
participant BrowserPanelView
participant BrowserPanel
participant TerminalController
BrowserPanelView->>BrowserPanel: noteWebViewVisibility(visible, reason)
BrowserPanel->>BrowserPanel: refreshWebViewLifecycleState()
TerminalController->>BrowserPanel: webViewLifecycleTopPayload()
BrowserPanel-->>TerminalController: lifecycle payload (state, visible_in_ui, should_render, timestamps, hidden_duration_ms)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (1 error, 1 warning)
✅ Passed checks (14 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
Greptile SummaryThis PR adds WKWebView lifecycle tracking to browser panels, recording visibility state (
Confidence Score: 3/5The lifecycle tracking machinery is mostly correct but has multiple unresolved issues that can make the core hidden_duration_ms metric unreliable in production — the same metric the discard policy in follow-up PRs will depend on. Two issues from prior review rounds remain in the code: resetWebViewLifecycleMetadata fires on the no-op branch of resetForWorkspaceContextChange clearing timestamp history for panels that do not need a reset, and noteWebViewVisibility is called synchronously inside updateNSView mutating a @published property during the SwiftUI view-update pass. Together they mean hidden_duration_ms can silently reset to zero or trigger runtime diagnostics on every workspace churn event. The new finding in this pass — null hidden_duration for background tabs that were never visible — adds a third gap in the observable this PR is meant to provide before the discard policy lands. Sources/Panels/BrowserPanel.swift and Sources/Panels/BrowserPanelView.swift both need attention: the lifecycle timestamp reset placement in resetForWorkspaceContextChange and the synchronous visibility mutation inside updateNSView are the two paths most likely to corrupt the hidden-duration metric in real use. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[BrowserPanel init] --> B{renderInitialNavigation?}
B -- false --> C[shouldRenderWebView = false\nstate = .newTab]
B -- true --> D[shouldRenderWebView = true\nstate = .liveHidden\nwebViewLastHiddenAt = nil]
E[onAppear / onChange isVisibleInUI] -->|noteWebViewVisibility| F{changed or first record?}
G[hideBrowserPortalView] -->|noteWebViewVisibility false| F
H[updateNSView portal accept/release] -->|noteWebViewVisibility sync| F
F -- yes --> I[update isWebViewVisibleInUI\nset webViewLastVisibleAt or webViewLastHiddenAt\ncall refreshWebViewLifecycleState]
F -- no --> J[refreshWebViewLifecycleState only]
I --> K{isClosingWebViewLifecycle?}
K -- yes --> L[.closing]
K -- no --> M{shouldRenderWebView?}
M -- no --> N[.newTab]
M -- yes --> O{isWebViewVisibleInUI?}
O -- yes --> P[.liveVisible]
O -- no --> Q[.liveHidden\nhidden_duration_ms = now - webViewLastHiddenAt]
D -.->|webViewLastHiddenAt is nil\nhidden_duration_ms = NSNull| Q
R[close] -->|isClosingWebViewLifecycle = true| L
S[resetForWorkspaceContextChange needsReset=false] -->|resetWebViewLifecycleMetadata wipes timestamps| N
T[resetForWorkspaceContextChange needsReset=true] -->|resetWebViewLifecycleMetadata| N
Reviews (5): Last reviewed commit: "browser: preserve lifecycle visibility o..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| isWebViewVisibleInUI = visible | ||
| if visible { | ||
| webViewLastVisibleAt = now | ||
| } else { | ||
| webViewLastHiddenAt = now | ||
| } | ||
| webViewLastVisibilityChangeAt = now | ||
| webViewLastVisibilityChangeReason = reason |
There was a problem hiding this comment.
webViewLastHiddenAt and webViewLastVisibilityChangeAt are unconditionally overwritten even when recordIfUnchanged: true and the view was already hidden. hideBrowserPortalView always passes recordIfUnchanged: true, so any subsequent AppKit portal-hide call — window animations, pane reflows, workspace churn — resets webViewLastHiddenAt to now. hidden_duration_ms is then computed as now − webViewLastHiddenAt, so it will read as near-zero for a tab that has been hidden for minutes. This defeats the stated purpose of the PR (identifying long-hidden retained WebContent processes before adding discard policy).
| isWebViewVisibleInUI = visible | |
| if visible { | |
| webViewLastVisibleAt = now | |
| } else { | |
| webViewLastHiddenAt = now | |
| } | |
| webViewLastVisibilityChangeAt = now | |
| webViewLastVisibilityChangeReason = reason | |
| if changed { | |
| isWebViewVisibleInUI = visible | |
| if visible { | |
| webViewLastVisibleAt = now | |
| } else { | |
| webViewLastHiddenAt = now | |
| } | |
| webViewLastVisibilityChangeAt = now | |
| } | |
| webViewLastVisibilityChangeReason = reason |
| private static func webViewLifecycleTimestamp(_ date: Date?) -> Any { | ||
| guard let date else { return NSNull() } | ||
| let formatter = ISO8601DateFormatter() | ||
| formatter.formatOptions = [.withInternetDateTime, .withFractionalSeconds] | ||
| return formatter.string(from: date) | ||
| } |
There was a problem hiding this comment.
webViewLifecycleTopPayload calls webViewLifecycleTimestamp up to three times per invocation, and each call allocates a fresh ISO8601DateFormatter. Formatter initialisation is non-trivial; since cmux top polls this for every browser panel, it creates avoidable per-poll allocations. A static cached instance is the standard fix.
| private static func webViewLifecycleTimestamp(_ date: Date?) -> Any { | |
| guard let date else { return NSNull() } | |
| let formatter = ISO8601DateFormatter() | |
| formatter.formatOptions = [.withInternetDateTime, .withFractionalSeconds] | |
| return formatter.string(from: date) | |
| } | |
| private static let lifecycleTimestampFormatter: ISO8601DateFormatter = { | |
| let f = ISO8601DateFormatter() | |
| f.formatOptions = [.withInternetDateTime, .withFractionalSeconds] | |
| return f | |
| }() | |
| private static func webViewLifecycleTimestamp(_ date: Date?) -> Any { | |
| guard let date else { return NSNull() } | |
| return Self.lifecycleTimestampFormatter.string(from: date) | |
| } |
| defer { panel.close() } | ||
|
|
||
| XCTAssertEqual(panel.webViewLifecycleState, .liveHidden) |
There was a problem hiding this comment.
defer { panel.close() } is set at the top of the test, and then panel.close() is also called explicitly just before the closing-state assertion. When the test function exits, the defer fires a second close() call. If any of the teardown helpers (GlobalSearchCoordinator.shared.purgePanel, closeDeveloperToolsForTeardown, etc.) are not idempotent, this will produce a double-teardown. Remove the defer since the test manages the lifetime manually.
| defer { panel.close() } | |
| XCTAssertEqual(panel.webViewLifecycleState, .liveHidden) | |
| XCTAssertEqual(panel.webViewLifecycleState, .liveHidden) |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cmuxTests/GhosttyConfigTests.swift`:
- Line 1201: The test currently calls defer { panel.close() } while also
explicitly calling panel.close() later to assert the .closing state, causing
close() to run twice; remove the defer cleanup from this test so the explicit
panel.close() is the sole cleanup/verification step (look for the defer {
panel.close() } and the explicit panel.close() and remove the defer to avoid
double invocation and rely on the assertion of the .closing state).
In `@Sources/Panels/BrowserPanel.swift`:
- Around line 2506-2512: The lifecycle metadata properties
(webViewLifecycleState, webViewLastVisibleAt, webViewLastHiddenAt,
webViewLastVisibilityChangeAt, webViewLastVisibilityChangeReason,
isWebViewVisibleInUI, isClosingWebViewLifecycle) are not cleared when a panel is
reused; update resetForWorkspaceContextChange(reason:) to reinitialize these
fields to their default values (e.g., .newTab for webViewLifecycleState, nil for
date/reason properties, false for booleans) so the panel does not carry stale
visibility/timestamp/reason data from the previous workspace.
- Around line 2701-2721: The method noteWebViewVisibility currently overwrites
hidden timestamps even when called with recordIfUnchanged while the web view is
already hidden; change it so webViewLastHiddenAt and
webViewLastVisibilityChangeAt are only set on an actual state transition to
hidden (i.e., when changed is true and visible is false). Keep updating
webViewLastVisibleAt when visible is true as before, and only update
webViewLastVisibilityChangeReason and webViewLastVisibilityChangeAt when changed
is true (or if webViewLastVisibilityChangeReason is nil on first record) so
repeated hideBrowserPortalView() calls do not reset hidden_duration_ms; adjust
logic in noteWebViewVisibility accordingly using the existing
isWebViewVisibleInUI, recordIfUnchanged, changed, webViewLastVisibleAt,
webViewLastHiddenAt, webViewLastVisibilityChangeAt, and
webViewLastVisibilityChangeReason symbols.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 91b2f22f-c82d-4f4c-8c4a-6ed1bb5eea00
📒 Files selected for processing (4)
Sources/Panels/BrowserPanel.swiftSources/Panels/BrowserPanelView.swiftSources/TerminalController.swiftcmuxTests/GhosttyConfigTests.swift
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 4 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="Sources/Panels/BrowserPanel.swift">
<violation number="1" location="Sources/Panels/BrowserPanel.swift:2713">
P1: Only update hidden/visible timestamps when visibility actually changes. Repeated hide events currently reset `webViewLastHiddenAt`, which under-reports `hidden_duration_ms` for retained hidden webviews.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Re-trigger cubic
| DispatchQueue.main.async { [weak panel] in | ||
| panel?.noteWebViewVisibility(lifecycleVisibleInUI, reason: lifecycleReason) | ||
| } |
There was a problem hiding this comment.
updateNSView
updateNSView is called on the main thread on every SwiftUI state change, including layout churn. The DispatchQueue.main.async defers each call to noteWebViewVisibility to the next run-loop cycle, which means multiple blocks accumulate in the queue with stale captured values. If onChange(of: isVisibleInUI) has already fired synchronously and set isWebViewVisibleInUI = false (with the correct webViewLastHiddenAt), a subsequent stale async block carrying lifecycleVisibleInUI = true from an older updateNSView pass will see changed = true and overwrite webViewLastHiddenAt with now, resetting the hidden-clock to zero. The hidden_duration_ms metric — the core observable this PR adds — can read near-zero for a tab that has been hidden for minutes.
The async hop is needed to avoid SwiftUI's "modifying state during view update" diagnostic (since noteWebViewVisibility can flip @Published var webViewLifecycleState). A safer shape is to snapshot visibility from isVisibleInUI inside a .task(id:) or onChange chain that already owns the transition, rather than racing an async block from the representable's imperative update path against the declarative SwiftUI path.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Sources/Panels/BrowserPanel.swift`:
- Around line 2506-2512: When webViewInstanceID is rotated the per-webview
lifecycle metadata must be reset; update switchToProfile(...) and
replaceWebViewPreservingState(...) to re-seed/clear webView lifecycle fields
(webViewLifecycleState, webViewLastVisibleAt, webViewLastHiddenAt,
webViewLastVisibilityChangeAt, webViewLastVisibilityChangeReason,
isWebViewVisibleInUI, isClosingWebViewLifecycle) immediately after
creating/assigning the new WKWebView and updating webViewInstanceID so the new
instance starts with fresh lifecycle values tied to the new ID.
- Around line 2714-2723: The bug is that webViewLastVisibilityChangeReason is
set unconditionally, causing it to drift from webViewLastVisibilityChangeAt when
recordIfUnchanged is true; only update webViewLastVisibilityChangeReason at the
same time you update webViewLastVisibilityChangeAt (i.e., inside the same
changed || isFirstVisibilityRecord branch) so the reason and timestamp always
refer to the same event—modify the block around
isWebViewVisibleInUI/webViewLastVisibleAt/webViewLastHiddenAt/webViewLastVisibilityChangeAt
to also set webViewLastVisibilityChangeReason there and remove the unconditional
assignment afterwards.
In `@Sources/Panels/BrowserPanelView.swift`:
- Around line 781-784: The call to panel.noteWebViewVisibility uses visibleInUI
to set the reason string but passes the effective visibility (visibleInUI &&
isCurrentPaneOwner) as the state, causing mismatched reason when the pane is not
owner; change the reason to reflect the actual emitted state by computing a
single effectiveVisibility = visibleInUI && isCurrentPaneOwner and pass that for
both the first argument and for selecting the reason (e.g., reason:
effectiveVisibility ? "view.visible" : "view.hidden") when calling
panel.noteWebViewVisibility so the reason matches the recorded visibility.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 07f8963d-cf90-4c3a-80df-64896aa5e046
📒 Files selected for processing (3)
Sources/Panels/BrowserPanel.swiftSources/Panels/BrowserPanelView.swiftcmuxTests/GhosttyConfigTests.swift
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="Sources/Panels/BrowserPanelView.swift">
<violation number="1" location="Sources/Panels/BrowserPanelView.swift:6706">
P2: Avoid dispatching captured visibility values asynchronously from `updateNSView`; queued stale values can be applied out of order and overwrite lifecycle timestamps, producing incorrect `hidden_duration_ms` telemetry.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Re-trigger cubic
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Sources/Panels/BrowserPanel.swift`:
- Line 4631: The reset currently only runs after needsWorkspaceContextReset,
letting lifecycle-only tabs keep
webViewLastVisibleAt/webViewLastVisibilityChangeReason when shouldRenderWebView
== false, currentURL == nil, and webView.superview == nil; move or invoke
resetWebViewLifecycleMetadata() in the early-return branch (or before the guard
that checks those blank-tab conditions) so lifecycle metadata is cleared even
when the tab is non-rendering, ensuring stale webViewLastVisibleAt and
webViewLastVisibilityChangeReason are reset regardless of
needsWorkspaceContextReset.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 9a1bde70-6e3e-4373-96f9-6c8b995bc104
📒 Files selected for processing (3)
Sources/Panels/BrowserPanel.swiftSources/Panels/BrowserPanelView.swiftcmuxTests/GhosttyConfigTests.swift
| if portalHostAccepted || didReleasePortalHost { | ||
| let lifecycleVisibleInUI = portalHostAccepted && coordinator.desiredPortalVisibleInUI | ||
| let lifecycleReason = lifecycleVisibleInUI ? "portal.update.visible" : "portal.update.hidden" | ||
| panel.noteWebViewVisibility(lifecycleVisibleInUI, reason: lifecycleReason) | ||
| } |
There was a problem hiding this comment.
@Published mutation still inside updateNSView
The PR description states "Deferred portal update lifecycle recording out of NSViewRepresentable render/update," but panel.noteWebViewVisibility is still called synchronously inside updateNSView. noteWebViewVisibility calls refreshWebViewLifecycleState(), which writes to @Published var webViewLifecycleState. Mutating an ObservableObject's published property synchronously during a SwiftUI view-update pass triggers the "Publishing changes from within view updates is not allowed" runtime diagnostic. The prior async-dispatch approach deferred that mutation but introduced the stale-capture race described in the earlier review thread; neither variant is safe here. The invariant needs a different shape — tracking the last-accepted portal visibility on the coordinator so the lifecycle update can be driven by a side-channel that is not tied to the render pass.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Sources/Panels/BrowserPanel.swift`:
- Around line 3420-3424: The replacement webview can be left permanently
`.live_hidden` because resetWebViewLifecycleMetadata() clears
isWebViewVisibleInUI and refreshWebViewLifecycleState() doesn't force
noteWebViewVisibility() when claimPortalHost() fails; updateUsingWindowPortal()
currently guards noteWebViewVisibility() behind (portalHostAccepted ||
didReleasePortalHost). To fix, ensure noteWebViewVisibility() runs after a
replacement even if claimPortalHost() returns false by adding an unconditional
fallback path: after calling resetWebViewLifecycleMetadata() and assigning
webView/currentURL in the replacement code path (where webViewInstanceID
changes), schedule a deferred call (async dispatch/Task or short timer) to
noteWebViewVisibility() or invoke refreshWebViewLifecycleState() to trigger
visibility reconciliation; alternatively add a dedicated recovery hook (onChange
for webViewInstanceID or a background monitor) that calls
noteWebViewVisibility() when claimPortalHost() fails (e.g., when
shouldUseLocalInlineDeveloperToolsHosting() is true) so new instances always
receive a visibility event.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: c75cb871-3f87-46f5-8b92-49d504a2aa5a
📒 Files selected for processing (1)
Sources/Panels/BrowserPanel.swift
|
Want your agent to iterate on Greptile's feedback? Try greploops. |
|
@coderabbitai resolve |
✅ Actions performedComments resolved and changes approved. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
austinywang
left a comment
austinywang
left a comment
There was a problem hiding this comment.
Summary
This is pointed in a useful direction: putting browser WebContent visibility into system.top is the right diagnostic surface, and the payload changes are mostly additive. I do not see a new WKWebView retain cycle, delegate-cycle, or observer teardown problem in the diff. I do see a correctness hole in the lifecycle state machine: attached-DevTools/local-inline hosting can keep reporting a hidden retained webview as live_visible, which is exactly the class of case this telemetry is supposed to distinguish.
Strengths
- The top payload change preserves the existing
webviewsarray andbrowser_web_content_pidfields, so existing JSON consumers should not break on missing old keys. BrowserPanel.close()now marks the lifecycle asclosingbefore delegate/KVO teardown, and workspace-context reset clears the new metadata.- The model-level duplicate-hide behavior does avoid resetting
last_hidden_at; the direct unit test covers that helper-level invariant. - The new code does not add NotificationCenter observers, KVO observers, WK delegates, or strong self-capturing WKWebView closures.
Concerns
-
Sources/Panels/BrowserPanelView.swift:6482:updateUsingLocalInlineHostingnever records lifecycle visibility, and that makes the state wrong when an attached-DevTools browser is hidden. The visible path is gated into local-inline mode atSources/Panels/BrowserPanelView.swift:1328, thenupdateUsingLocalInlineHostingclears/suppresses portal ownership aroundSources/Panels/BrowserPanelView.swift:6494. When the same panel later loses pane ownership or visibility,updateNSViewfalls back to the window-portal path, but the hidden lifecycle update atSources/Panels/BrowserPanelView.swift:6738only runs if a portal host was accepted or released. After local-inline hosting there may be no active portal lease to release, sonoteWebViewVisibility(false, ...)is skipped andcmux topkeeps showinglive_visiblefor a hidden retained WKWebView. That hides the exact memory-diagnostic case this PR is trying to expose. The fix should make effective webview visibility a single transition independent of whether the current host is window-portal or local-inline. -
Sources/Panels/BrowserPanel.swift:2701: lifecycle ownership is split acrossBrowserPanelView.onAppear(Sources/Panels/BrowserPanelView.swift:717), SwiftUIisVisibleInUIchanges (Sources/Panels/BrowserPanelView.swift:780), portal acceptance/release (Sources/Panels/BrowserPanelView.swift:6738), and workspace/tab cleanup (Sources/Panels/BrowserPanel.swift:6335). Architecturally this is still bolted onto several ad-hoc lifecycle signals rather than derived from one owner of "effective browser visibility." The local-inline miss above is a symptom of that split. I would expect one@MainActorowner onBrowserPanelto receive explicit lifecycle events or snapshots from the view/portal layer and derivenew_tab/live_visible/live_hidden/closingin one place, with portal/local-inline as implementation details rather than separate truth sources. -
cmuxTests/GhosttyConfigTests.swift:1175: the new tests callBrowserPanel.noteWebViewVisibilitydirectly, so they prove the helper behavior but not the real call paths that can disagree: selected-tab changes, workspace retirement, portal host replacement, local-inline DevTools hosting, andsystem.topserialization. Add at least one behavior-level test or small runtime seam that drives an effective visibility transition through the same path used byBrowserPanelView/portal ownership, plus a top-payload assertion. Otherwise the current tests would pass while the attached-DevTools hidden-state bug above remains.
Suggestions
- If the goal is human memory diagnostics from
cmux top, consider also rendering the lifecycle in the default tree label inCLI/cmux.swift:12486; right now it is only visible to JSON consumers. - Consider using truncation instead of rounding for
hidden_duration_msinSources/Panels/BrowserPanel.swift:2773if downstream tooling treats it as elapsed time rather than a displayed approximation.
Verdict
Request changes. The payload shape is fine, but the lifecycle source of truth is not durable yet, and at least one real browser hosting mode can report hidden retained WebViews as visible.
This branch was successfully deployed
2 participants
Summary
Why
Hidden browser tabs/workspaces currently keep their WKWebView and WebContent process alive, but cmux top does not distinguish visible webviews from hidden retained ones. This PR adds the observability needed before adding discard/reclaim policy in follow-up PRs.
Review follow-up
Validation
git diff --checkswiftc -parse Sources/Panels/BrowserPanel.swift Sources/Panels/BrowserPanelView.swift Sources/TerminalController.swift cmuxTests/GhosttyConfigTests.swiftNot run:
./scripts/test-unit.sh -only-testing:cmuxTests/BrowserPanelWebViewLifecycleTestsbecause this machine has only Command Line Tools selected and no Xcode.app install; xcodebuild exits with: tool xcodebuild requires Xcode.Summary by CodeRabbit
New Features
Behavior
Tests