Repository navigation
Fix browser pane refreshes on split and resize churn - #1224
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughRefactors hosted WebView update flow to classify updates as none, geometryOnly, or refresh; adds a geometry-only invalidation path and a reattachRenderingState flag to refresh passes. Tests add counters/hooks to verify resizes repaint without forcing WebKit reattachment. CI checkout/install steps updated. Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
Greptile SummaryThis PR fixes excessive WebKit rendering reattaches that were firing on every browser-pane resize or split-divider drag by introducing a two-tier update path: geometry-only changes (frame, bounds, webFrame) now go through
Confidence Score: 4/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[sync detects changes\ncollects refreshReasons] --> B{reasons empty?}
B -- yes --> Z[.none\nno update]
B -- no --> C{any reason in\nrefreshReasons set?\nreveal / transientRecovery\nanchor / syncAttach*}
C -- yes --> D[.refresh]
C -- no --> E{all reasons subset of\ngeometryOnlyReasons?\nframe / bounds\nwebFrame / webFrameBottomDock}
E -- yes --> F[.geometryOnly]
E -- no --> G[.refresh\nfallthrough default]
D --> H{hostedInspectorAdjusted\n&& !recoveredFromTransient?}
H -- yes --> Z2[skip update]
H -- no --> I[refreshHostedWebViewPresentation\nbrowserPortalReattachRenderingState ×3]
F --> J[invalidateHostedWebViewGeometry\ndisplayIfNeeded only — no reattach]
G --> H
Last reviewed commit: 7c29652 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if reattachRenderingState { | ||
| webView.browserPortalReattachRenderingState(reason: "\(reason):\(phase)") | ||
| containerView.displayIfNeeded() | ||
| webView.displayIfNeeded() | ||
| (webView.window ?? hostView.window)?.displayIfNeeded() | ||
| } else { | ||
| containerView.displayIfNeeded() | ||
| webView.displayIfNeeded() | ||
| (webView.window ?? hostView.window)?.displayIfNeeded() | ||
| } |
There was a problem hiding this comment.
Redundant else branch, divergence risk
The else block contains the exact same three displayIfNeeded() calls as the end of the if block. The only meaningful difference between the two branches is the browserPortalReattachRenderingState call, so the display work can be lifted out of the conditional entirely. As written, any future addition to one branch (e.g. an extra flush or guard) must be duplicated in the other or a silent regression is introduced.
| if reattachRenderingState { | |
| webView.browserPortalReattachRenderingState(reason: "\(reason):\(phase)") | |
| containerView.displayIfNeeded() | |
| webView.displayIfNeeded() | |
| (webView.window ?? hostView.window)?.displayIfNeeded() | |
| } else { | |
| containerView.displayIfNeeded() | |
| webView.displayIfNeeded() | |
| (webView.window ?? hostView.window)?.displayIfNeeded() | |
| } | |
| if reattachRenderingState { | |
| webView.browserPortalReattachRenderingState(reason: "\(reason):\(phase)") | |
| } | |
| containerView.displayIfNeeded() | |
| webView.displayIfNeeded() | |
| (webView.window ?? hostView.window)?.displayIfNeeded() |
| static func resolve(refreshReasons: [String]) -> Self { | ||
| guard !refreshReasons.isEmpty else { return .none } | ||
| let reasonSet = Set(refreshReasons) | ||
| if !reasonSet.isDisjoint(with: Self.refreshReasons) { |
There was a problem hiding this comment.
Parameter name shadows the static property of the same name
The parameter refreshReasons: [String] has the same name as the private static property refreshReasons: Set<String>. Inside this function, bare refreshReasons resolves to the parameter while Self.refreshReasons resolves to the static set, which is correct but makes the code harder to read at a glance — a reader who misses the Self. qualifier on line 2512 may think the set is being compared against itself. Renaming either the parameter or the static property would eliminate the ambiguity.
| static func resolve(refreshReasons: [String]) -> Self { | |
| guard !refreshReasons.isEmpty else { return .none } | |
| let reasonSet = Set(refreshReasons) | |
| if !reasonSet.isDisjoint(with: Self.refreshReasons) { | |
| static func resolve(reasons: [String]) -> Self { | |
| guard !reasons.isEmpty else { return .none } | |
| let reasonSet = Set(reasons) | |
| if !reasonSet.isDisjoint(with: Self.refreshReasons) { |
| override func viewDidUnhide() { | ||
| reattachRenderingStateCount += 1 | ||
| super.viewDidUnhide() | ||
| } |
There was a problem hiding this comment.
viewDidUnhide fires for all un-hide transitions, not only reattach paths
viewDidUnhide is a standard NSView lifecycle callback that fires whenever the view transitions from hidden to visible — it is not exclusive to browserPortalReattachRenderingState. If a future test hides and then re-shows the WKWebView for an unrelated reason, reattachRenderingStateCount will be incremented even though no reattach occurred, producing a misleading failure (or false pass).
The two ObjC SPI hooks (_enterInWindow, _endDeferringViewInWindowChangesSync) are already precise proxies for the reattach path. Keeping viewDidUnhide separate from reattachRenderingStateCount (e.g. with its own unhideCount counter) would make the assertion semantics explicit and resilient against hide/show churn in future test scenarios.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Sources/BrowserWindowPortal.swift (1)
2099-2108:⚠️ Potential issue | 🟠 MajorAvoid a second invalidate on the external-geometry hot path.
Line 2097already drives each entry throughsynchronizeWebView(..., source: "externalGeometry"), and that path now callsinvalidateHostedWebViewGeometry(...)itself when the reasons resolve to.geometryOnly. This extra loop adds another synchronous repaint/display pass for the same resize tick, which is exactly the churn path this PR is trying to make cheaper. Consider lettingsynchronizeWebViewown the invalidate, or only running this fallback when the sync pass ended up with.none.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/BrowserWindowPortal.swift` around lines 2099 - 2108, The loop over entriesByWebViewId is redundantly calling invalidateHostedWebViewGeometry after synchronizeWebView already invalidates when the resolved reason is .geometryOnly; remove the extra invalidate by letting synchronizeWebView(..., source: "externalGeometry") own invalidation or only call invalidateHostedWebViewGeometry from this loop when synchronizeWebView returned .none; update the code around synchronizeWebView and the for-entry loop (referencing synchronizeWebView, invalidateHostedWebViewGeometry, entriesByWebViewId, entry.webView, entry.containerView) so a single invalidate occurs per resize tick (either move invalidate into synchronizeWebView or conditionalize this fallback to run only when sync reported .none).
🧹 Nitpick comments (1)
Sources/BrowserWindowPortal.swift (1)
2489-2519: Prefer typed presentation reasons over raw strings.This classifier depends on string literals that are appended from several branches. A typo or a future geometry-only reason that is not added to
geometryOnlyReasonswill silently fall back to.refreshand reintroduce the WebKit reattach path. Carrying a small enum instead of[String]would make that regression compile-time visible.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/BrowserWindowPortal.swift` around lines 2489 - 2519, Replace the string-based classifier with a typed enum to avoid silent fallbacks: introduce a PresentationReason enum (cases like frame, bounds, webFrame, webFrameBottomDock, syncAttachContainer, syncAttachWebView, reveal, transientRecovery, anchor) and change HostedWebViewPresentationUpdateKind.resolve to accept [PresentationReason] instead of [String]; update the internal sets geometryOnlyReasons/refreshReasons to be Set<PresentationReason> and perform the same subset/disjoint logic on the enum values; adjust all call sites that currently append string reasons to convert to PresentationReason at the API boundary (e.g., where strings are collected from WebKit/other branches) so the rest of the code uses the typed enum and compilation will catch missing/typoed reasons.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cmuxTests/CmuxWebViewKeyEquivalentTests.swift`:
- Around line 11444-11464: The reattach probe lacks a positive-control: capture
the initial value of reattachRenderingStateCount on the test helper instance,
trigger the known full-refresh scenario by calling
testPortalRevealRefreshesHostedWebViewWithoutFrameDelta (or invoking the same
portal-reveal flow used there), and assert that reattachRenderingStateCount has
increased afterwards; update the test to reference the helper's
reattachRenderingStateCount and ensure the increment is observed (also apply the
same positive-control assertion pattern to the other similar tests around the
reattach probes that you noted).
---
Outside diff comments:
In `@Sources/BrowserWindowPortal.swift`:
- Around line 2099-2108: The loop over entriesByWebViewId is redundantly calling
invalidateHostedWebViewGeometry after synchronizeWebView already invalidates
when the resolved reason is .geometryOnly; remove the extra invalidate by
letting synchronizeWebView(..., source: "externalGeometry") own invalidation or
only call invalidateHostedWebViewGeometry from this loop when synchronizeWebView
returned .none; update the code around synchronizeWebView and the for-entry loop
(referencing synchronizeWebView, invalidateHostedWebViewGeometry,
entriesByWebViewId, entry.webView, entry.containerView) so a single invalidate
occurs per resize tick (either move invalidate into synchronizeWebView or
conditionalize this fallback to run only when sync reported .none).
---
Nitpick comments:
In `@Sources/BrowserWindowPortal.swift`:
- Around line 2489-2519: Replace the string-based classifier with a typed enum
to avoid silent fallbacks: introduce a PresentationReason enum (cases like
frame, bounds, webFrame, webFrameBottomDock, syncAttachContainer,
syncAttachWebView, reveal, transientRecovery, anchor) and change
HostedWebViewPresentationUpdateKind.resolve to accept [PresentationReason]
instead of [String]; update the internal sets geometryOnlyReasons/refreshReasons
to be Set<PresentationReason> and perform the same subset/disjoint logic on the
enum values; adjust all call sites that currently append string reasons to
convert to PresentationReason at the API boundary (e.g., where strings are
collected from WebKit/other branches) so the rest of the code uses the typed
enum and compilation will catch missing/typoed reasons.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0eaaecfb-24f2-48a9-9170-245d2722f5eb
📒 Files selected for processing (2)
Sources/BrowserWindowPortal.swiftcmuxTests/CmuxWebViewKeyEquivalentTests.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 (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=".github/workflows/ci.yml">
<violation number="1" location=".github/workflows/ci.yml:40">
P1: Avoid `curl | bash` for tool installation in CI. This runs an unverified remote script and weakens supply-chain safety compared with a pinned setup action.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| - name: Install Bun | ||
| run: | | ||
| set -euo pipefail | ||
| curl -fsSL https://bun.sh/install | bash -s -- bun-v1.3.10 | ||
| echo "$HOME/.bun/bin" >> "$GITHUB_PATH" |
There was a problem hiding this comment.
P1: Avoid curl | bash for tool installation in CI. This runs an unverified remote script and weakens supply-chain safety compared with a pinned setup action.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .github/workflows/ci.yml, line 40:
<comment>Avoid `curl | bash` for tool installation in CI. This runs an unverified remote script and weakens supply-chain safety compared with a pinned setup action.</comment>
<file context>
@@ -35,10 +35,13 @@ jobs:
- - name: Setup Bun
- uses: oven-sh/setup-bun@3d267786b128fe76c2f16a390aa2448b815359f3 # v2
+ - name: Install Bun
+ run: |
+ set -euo pipefail
</file context>
| - name: Install Bun | |
| run: | | |
| set -euo pipefail | |
| curl -fsSL https://bun.sh/install | bash -s -- bun-v1.3.10 | |
| echo "$HOME/.bun/bin" >> "$GITHUB_PATH" | |
| - name: Setup Bun | |
| uses: oven-sh/setup-bun@3d267786b128fe76c2f16a390aa2448b815359f3 # v2 | |
| with: | |
| bun-version: 1.3.10 |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Sources/BrowserWindowPortal.swift (1)
2091-2109:⚠️ Potential issue | 🟠 MajorAvoid the second invalidate pass on external geometry changes.
Line 2097 already re-synchronizes every entry. For any entry that picks up
frame,bounds,webFrame, orwebFrameBottomDock,synchronizeWebView(...)now takes the.geometryOnlypath and callsinvalidateHostedWebViewGeometry(...)itself. The unconditional loop on Lines 2099-2108 then invalidates the same visible entry again, so live split/window resize does two repaint passes instead of one. Consider limiting this fallback loop to entries whose sync stayed.none, or havesynchronizeWebView(...)report whether it already invalidated/refreshed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/BrowserWindowPortal.swift` around lines 2091 - 2109, synchronizeAllEntriesFromExternalGeometryChange is doing a second unconditional invalidate pass over entries after calling synchronizeAllWebViews(excluding:source:), causing duplicate repaints; change the flow so the fallback loop only invalidates entries that were not already refreshed by synchronizeWebView(...). Update synchronizeAllWebViews/existing synchronizeWebView(...) to either return a Set of webView ids (or mark a property on entriesByWebViewId entries) for which it performed a geometry invalidation/refresh (or whose sync result != .none), then in synchronizeAllEntriesFromExternalGeometryChange iterate entriesByWebViewId and call invalidateHostedWebViewGeometry(...) only for entries not present in that returned Set (or whose marked sync result is .none), referencing the functions synchronizeAllEntriesFromExternalGeometryChange, synchronizeAllWebViews(excluding:source:), synchronizeWebView(...), invalidateHostedWebViewGeometry(...), and entriesByWebViewId to locate and implement the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 40-44: Replace the current "Install Bun" step that pipes
https://bun.sh/install into bash with the official pinned GitHub Action; stop
using the curl | bash approach and instead use the oven-sh/setup-bun action
(e.g., oven-sh/setup-bun@v2.1.3) and pass the bun-version input set to "1.3.10"
so the workflow is immutable and auditable; update the step name if needed and
remove the echo to GITHUB_PATH since the action manages installation paths.
In `@cmuxTests/CmuxWebViewKeyEquivalentTests.swift`:
- Around line 11444-11459: The test currently overrides WKWebView lifecycle
selectors (displayIfNeeded, cmuxUnitTestEnterInWindow,
cmuxUnitTestEndDeferringViewInWindowChangesSync) and increments
reattachRenderingStateCount but does not forward to the original implementation;
change these hooks to wrap the real implementations instead of replacing them by
invoking super (or forwarding to the original IMP) after or before incrementing
reattachRenderingStateCount so the real WebKit lifecycle work still runs while
preserving the probe; locate the overrides in
cmuxTests/CmuxWebViewKeyEquivalentTests.swift (functions displayIfNeeded,
cmuxUnitTestEnterInWindow, cmuxUnitTestEndDeferringViewInWindowChangesSync) and
ensure each wrapper calls the original implementation and then increments the
counter (or increments then forwards if timing matters).
---
Outside diff comments:
In `@Sources/BrowserWindowPortal.swift`:
- Around line 2091-2109: synchronizeAllEntriesFromExternalGeometryChange is
doing a second unconditional invalidate pass over entries after calling
synchronizeAllWebViews(excluding:source:), causing duplicate repaints; change
the flow so the fallback loop only invalidates entries that were not already
refreshed by synchronizeWebView(...). Update synchronizeAllWebViews/existing
synchronizeWebView(...) to either return a Set of webView ids (or mark a
property on entriesByWebViewId entries) for which it performed a geometry
invalidation/refresh (or whose sync result != .none), then in
synchronizeAllEntriesFromExternalGeometryChange iterate entriesByWebViewId and
call invalidateHostedWebViewGeometry(...) only for entries not present in that
returned Set (or whose marked sync result is .none), referencing the functions
synchronizeAllEntriesFromExternalGeometryChange,
synchronizeAllWebViews(excluding:source:), synchronizeWebView(...),
invalidateHostedWebViewGeometry(...), and entriesByWebViewId to locate and
implement the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8398e3d2-63ea-4734-a746-f3b9c15e7137
📒 Files selected for processing (4)
.github/workflows/ci-macos-compat.yml.github/workflows/ci.ymlSources/BrowserWindowPortal.swiftcmuxTests/CmuxWebViewKeyEquivalentTests.swift
| - name: Install Bun | ||
| run: | | ||
| set -euo pipefail | ||
| curl -fsSL https://bun.sh/install | bash -s -- bun-v1.3.10 | ||
| echo "$HOME/.bun/bin" >> "$GITHUB_PATH" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Read the workflow file to see current implementation and context
cat -n .github/workflows/ci.yml | head -60Repository: manaflow-ai/cmux
Length of output: 2011
🏁 Script executed:
# Check if there's git history showing what was previously used for Bun setup
git log --oneline -20 .github/workflows/ci.ymlRepository: manaflow-ai/cmux
Length of output: 109
🏁 Script executed:
# Search for any Bun-related setup actions or references in the repo
rg -i "bun" --type yamlRepository: manaflow-ai/cmux
Length of output: 122
🌐 Web query:
official Bun GitHub Action setup-bun version pinning
💡 Result:
To pin Bun’s version with the official GitHub Action (oven-sh/setup-bun), set bun-version (or point to a version file):
- uses: oven-sh/setup-bun@v2
with:
bun-version: "1.3.3" # also supports "latest", "canary", "1.3.x"[2]
Or:
- uses: oven-sh/setup-bun@v2
with:
bun-version-file: ".bun-version" # also supports package.json / .tool-versions[1]
If you don’t specify a version, the action will auto-detect from package.json (packageManager first, then engines.bun), otherwise it installs latest. [1]
To pin the action itself (recommended for reproducibility), use a specific tag (example latest shown in the repo UI as v2.1.3, released Mar 4, 2026):
- uses: oven-sh/setup-bun@v2.1.3
with:
bun-version: "1.3.3"[1][3]
Sources: official action README [1], Bun docs CI/CD guide [2], action releases/tags [3].
Switch to the official pinned Bun action instead of piping an installer into bash.
The current implementation executes a mutable remote script at job time (https://bun.sh/install), which loses the immutability and auditability of a pinned action. Use the official oven-sh/setup-bun@v2.1.3 action with an explicit version instead:
- uses: oven-sh/setup-bun@v2.1.3
with:
bun-version: "1.3.10"This provides commit-level immutability, reviewed/auditable code, and proper version pinning for both the action and the Bun runtime.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/ci.yml around lines 40 - 44, Replace the current "Install
Bun" step that pipes https://bun.sh/install into bash with the official pinned
GitHub Action; stop using the curl | bash approach and instead use the
oven-sh/setup-bun action (e.g., oven-sh/setup-bun@v2.1.3) and pass the
bun-version input set to "1.3.10" so the workflow is immutable and auditable;
update the step name if needed and remove the echo to GITHUB_PATH since the
action manages installation paths.
| private(set) var reattachRenderingStateCount = 0 | ||
|
|
||
| override func displayIfNeeded() { | ||
| displayIfNeededCount += 1 | ||
| super.displayIfNeeded() | ||
| } | ||
|
|
||
| @objc(_enterInWindow) | ||
| func cmuxUnitTestEnterInWindow() { | ||
| reattachRenderingStateCount += 1 | ||
| } | ||
|
|
||
| @objc(_endDeferringViewInWindowChangesSync) | ||
| func cmuxUnitTestEndDeferringViewInWindowChangesSync() { | ||
| reattachRenderingStateCount += 1 | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "TrackingPortalWebView hook definitions:"
sed -n '11442,11460p' cmuxTests/CmuxWebViewKeyEquivalentTests.swift
echo
echo "Search for any wrapper/swizzle that forwards these selectors:"
rg -n -C2 '_enterInWindow|_endDeferringViewInWindowChangesSync|method_exchangeImplementations|class_replaceMethod|class_getInstanceMethod|forwardInvocation|objc_msgSend' cmuxTests/CmuxWebViewKeyEquivalentTests.swiftRepository: manaflow-ai/cmux
Length of output: 1348
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Usage of reattachRenderingStateCount in tests ==="
rg -n 'reattachRenderingStateCount' cmuxTests/CmuxWebViewKeyEquivalentTests.swift
echo
echo "=== Context around the hook usage (test assertions) ==="
# Look for tests that use these counters
rg -n -B5 -A5 'reattachRenderingStateCount' cmuxTests/CmuxWebViewKeyEquivalentTests.swift | head -60
echo
echo "=== Check if there are comments explaining the hook design ==="
sed -n '11440,11465p' cmuxTests/CmuxWebViewKeyEquivalentTests.swiftRepository: manaflow-ai/cmux
Length of output: 3913
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Full test showing the assertion context ==="
sed -n '11815,11850p' cmuxTests/CmuxWebViewKeyEquivalentTests.swift
echo
echo "=== Second test using reattachRenderingStateCount ==="
sed -n '11895,11930p' cmuxTests/CmuxWebViewKeyEquivalentTests.swift
echo
echo "=== Third test using reattachRenderingStateCount ==="
sed -n '12212,12245p' cmuxTests/CmuxWebViewKeyEquivalentTests.swift
echo
echo "=== Search for any tests that EXPECT the counter to increment ==="
rg -n -B3 'reattachRenderingStateCount.*+|XCTAssertGreater.*reattachRenderingStateCount|reattachRenderingStateCount.*initialReattachCount' cmuxTests/CmuxWebViewKeyEquivalentTests.swiftRepository: manaflow-ai/cmux
Length of output: 6828
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Search for _enterInWindow and _endDeferringViewInWindowChangesSync in production code ==="
rg -t swift '_enterInWindow|_endDeferringViewInWindowChangesSync' --type-list | head -20
rg '_enterInWindow|_endDeferringViewInWindowChangesSync' Sources/ 2>/dev/null || echo "No matches in Sources/"
echo
echo "=== Check if these are documented WebKit private methods ==="
echo "Checking WebKit documentation references or patterns..."
rg -n 'enterInWindow|endDeferringViewInWindowChangesSync' cmuxTests/ | grep -v 'cmuxUnitTest' || echo "Only test-specific references found"
echo
echo "=== Look for any other WKWebView subclasses in the codebase ==="
rg -n 'class.*WKWebView' Sources/ | head -20Repository: manaflow-ai/cmux
Length of output: 560
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Search for _enterInWindow or _endDeferringViewInWindowChangesSync in production code ==="
rg -t swift '_enterInWindow|_endDeferringViewInWindowChangesSync' Sources/ 2>&1 || echo "No matches in Sources/"
echo
echo "=== Check if these selectors appear anywhere outside the test file ==="
rg '_enterInWindow|_endDeferringViewInWindowChangesSync' --glob '!CmuxWebViewKeyEquivalentTests.swift' 2>&1 || echo "Only in test file"
echo
echo "=== List all WKWebView subclasses in production ==="
rg -n 'class.*:\s*WKWebView' Sources/ -A 5 | head -40
echo
echo "=== Search for any method swizzling or ObjC runtime manipulation in tests ==="
rg -n 'method_setImplementation|method_exchangeImplementations|class_replaceMethod|class_addMethod|forwardingTargetForSelector' cmuxTests/ || echo "No swizzling found"Repository: manaflow-ai/cmux
Length of output: 1290
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Context around _enterInWindow and _endDeferringViewInWindowChangesSync in BrowserWindowPortal.swift ==="
rg -n -B10 -A10 '_enterInWindow|_endDeferringViewInWindowChangesSync' Sources/BrowserWindowPortal.swift | head -100Repository: manaflow-ai/cmux
Length of output: 902
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Implementation of browserPortalCallVoidIfAvailable ==="
rg -n -B3 -A15 'func browserPortalCallVoidIfAvailable' Sources/BrowserWindowPortal.swift
echo
echo "=== Full context of browserPortalReattachRenderingState ==="
rg -n -B5 -A25 'func browserPortalReattachRenderingState' Sources/BrowserWindowPortal.swiftRepository: manaflow-ai/cmux
Length of output: 1837
Wrap these hooks instead of replacing the WebKit lifecycle methods.
These @objc methods override private WKWebView selectors without forwarding to the original implementation. When production code invokes browserPortalReattachRenderingState(), the test's hooks increment the counter but skip the real WebKit lifecycle work. The test measures invocation without validating behavior, which weakens regression coverage for the reattach path.
The hooks should call super or forward to the original IMP to preserve the real behavior while maintaining the probe.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cmuxTests/CmuxWebViewKeyEquivalentTests.swift` around lines 11444 - 11459,
The test currently overrides WKWebView lifecycle selectors (displayIfNeeded,
cmuxUnitTestEnterInWindow, cmuxUnitTestEndDeferringViewInWindowChangesSync) and
increments reattachRenderingStateCount but does not forward to the original
implementation; change these hooks to wrap the real implementations instead of
replacing them by invoking super (or forwarding to the original IMP) after or
before incrementing reattachRenderingStateCount so the real WebKit lifecycle
work still runs while preserving the probe; locate the overrides in
cmuxTests/CmuxWebViewKeyEquivalentTests.swift (functions displayIfNeeded,
cmuxUnitTestEnterInWindow, cmuxUnitTestEndDeferringViewInWindowChangesSync) and
ensure each wrapper calls the original implementation and then increments the
counter (or increments then forwards if timing matters).
Roll back actions/checkout v6.0.2 to v4, restore oven-sh/setup-bun action, and remove explicit ref parameter from compat workflow. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…-browser-refresh Fix browser pane refreshes on split and resize churn
Roll back actions/checkout v6.0.2 to v4, restore oven-sh/setup-bun action, and remove explicit ref parameter from compat workflow. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This branch was successfully deployed
1 participant
Summary
Closes #1223.
Verification
./scripts/reload.sh --tag fix-1223-browser-refreshSummary by cubic
Fixes browser pane flicker during split/resize churn by repainting on geometry-only changes and avoiding unnecessary WebKit reattach. Full refresh is preserved for reveal, sync attach, anchor, and transient recovery. Closes #1223.
Bug Fixes
invalidateHostedWebViewGeometry.reattachRenderingStateflag; skips refresh while the inspector divider is dragged.Dependencies
actions/checkoutto v6.0.2 and ensure PR head SHA is checked out in macOS compat CI.oven-sh/setup-bunwith a pinnedBuninstall script (bun-v1.3.10) and set PATH.Written for commit 27e598c. Summary will update on new commits.
Summary by CodeRabbit
Improvements
Tests
Chores