Repository navigation
Conversation
|
Caution Review failedAn error occurred during the review process. Please try again later. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
ejc3
force-pushed
the
remote-tmux-fuzz-can-see-reveal
branch
2 times, most recently
from
July 16, 2026 09:55
af0e608 to
9fe1365
Compare
ejc3
marked this pull request as ready for review
July 16, 2026 11:15
Contributor
Greptile SummaryThis is a harness-only change with no production file modifications. It fixes three categories of blind spots in the remote-tmux sizing fuzz: a settlement judge that ANDed on the flag it was testing (shrinking the judged set instead of reddening it), a pane census blind to zoom state, and a mirror-tab-switch op that was never exercised because no operation ever switched cmux tabs.
Confidence Score: 5/5Safe to merge — all changes are test harness and debug diagnostics only, with no production logic modified. Every changed file is either a debug-only Swift extension (inside an existing #if DEBUG block in Sources/Debug/) or a fuzz harness shell script. The Swift changes make the settlement judge more accurate by reading view state directly rather than through the flag under test, and by tightening the derivation-parity and settled computations. The shell script changes add a missing op, fix zoom-blind pane census, and assert marathon-level coverage. No production behavior is altered. No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Settlement Judge per window] --> B{isTornDown?}
B -- yes --> SKIP[skip]
B -- no --> C[Read onScreen from hostedView directly\nnot through mirror flag]
C --> D{onScreen AND not isVisibleForSizing?}
D -- yes --> M1[append mismatch: on-screen but not visible-for-sizing]
D -- no --> E{onScreen AND claimed == nil?}
E -- yes --> M2[append mismatch: on-screen but never claimed]
E -- no --> F{onScreen?}
M1 --> F
M2 --> F
F -- no --> G{mismatches empty?}
G -- yes --> SKIP2[continue - off-screen, no report]
G -- no --> R1[append window entry settled=false]
F -- yes --> H[derivationSettled: false if missing grid/claim]
H --> I[settled = mismatches.isEmpty AND all terms true]
I --> R2[append window entry with why dict]
subgraph Op10[Op 10 - switch_mirror_tab]
O1[get pane_grids mirrored windows]
O1 --> O2[get shown window from pane_surfaces]
O2 --> O3{shown is mirrored?}
O3 -- no --> OSKIP[skip]
O3 -- yes --> O4[find target: off-screen mirrored window != shown using IN not inside]
O4 --> O5[surface.focus target]
O5 --> O6[poll up to 3s for shown == target_window]
O6 -- match --> O7[OP10_SWITCHES++]
O6 -- no match --> O8[note_fail: mirror tab never switched]
end
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
A[Settlement Judge per window] --> B{isTornDown?}
B -- yes --> SKIP[skip]
B -- no --> C[Read onScreen from hostedView directly\nnot through mirror flag]
C --> D{onScreen AND not isVisibleForSizing?}
D -- yes --> M1[append mismatch: on-screen but not visible-for-sizing]
D -- no --> E{onScreen AND claimed == nil?}
E -- yes --> M2[append mismatch: on-screen but never claimed]
E -- no --> F{onScreen?}
M1 --> F
M2 --> F
F -- no --> G{mismatches empty?}
G -- yes --> SKIP2[continue - off-screen, no report]
G -- no --> R1[append window entry settled=false]
F -- yes --> H[derivationSettled: false if missing grid/claim]
H --> I[settled = mismatches.isEmpty AND all terms true]
I --> R2[append window entry with why dict]
subgraph Op10[Op 10 - switch_mirror_tab]
O1[get pane_grids mirrored windows]
O1 --> O2[get shown window from pane_surfaces]
O2 --> O3{shown is mirrored?}
O3 -- no --> OSKIP[skip]
O3 -- yes --> O4[find target: off-screen mirrored window != shown using IN not inside]
O4 --> O5[surface.focus target]
O5 --> O6[poll up to 3s for shown == target_window]
O6 -- match --> O7[OP10_SWITCHES++]
O6 -- no match --> O8[note_fail: mirror tab never switched]
end
Reviews (3): Last reviewed commit: "remote-tmux: close the review bots' thre..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
334
to
343
| if [ -n "${missing// /}" ]; then | ||
| note_fail "$iter" "visible $window: tmux panes [$missing] are not on screen in the app (coverage would silently drop them)" | ||
| # Report both sides and the zoom flag, not just the difference. Whether a | ||
| # pane is EXPECTED on screen turns entirely on zoom — a sibling of a zoomed | ||
| # pane is absent by design — so naming the missing pane alone accuses the | ||
| # app in a sentence that reads identically when the harness is the one that | ||
| # is wrong. | ||
| zoom_state=off | ||
| printf '%s\n' "$window_panes" | grep -q '^1 ' && zoom_state=on | ||
| note_fail "$iter" "visible $window: tmux panes [$missing] are not on screen in the app (coverage would silently drop them) zoom=$zoom_state expected=[$(printf '%s' "$tmux_panes" | tr '\n' ' ')] app_on_screen=[$(printf '%s' "$app_panes" | tr '\n' ' ')]" | ||
| fi |
Contributor
There was a problem hiding this comment.
zoom_state is already set correctly by the outer if/else block just above this section (lines 323–325). The two lines inside the failure block re-run the identical grep -q '^1 ' and re-assign the same value, so they are dead computation. If the intent was defensive clarity the comment above already explains the context; the duplicate assignment just adds noise and risks drifting from the outer logic if either branch is later edited.
Suggested change
| if [ -n "${missing// /}" ]; then | |
| note_fail "$iter" "visible $window: tmux panes [$missing] are not on screen in the app (coverage would silently drop them)" | |
| # Report both sides and the zoom flag, not just the difference. Whether a | |
| # pane is EXPECTED on screen turns entirely on zoom — a sibling of a zoomed | |
| # pane is absent by design — so naming the missing pane alone accuses the | |
| # app in a sentence that reads identically when the harness is the one that | |
| # is wrong. | |
| zoom_state=off | |
| printf '%s\n' "$window_panes" | grep -q '^1 ' && zoom_state=on | |
| note_fail "$iter" "visible $window: tmux panes [$missing] are not on screen in the app (coverage would silently drop them) zoom=$zoom_state expected=[$(printf '%s' "$tmux_panes" | tr '\n' ' ')] app_on_screen=[$(printf '%s' "$app_panes" | tr '\n' ' ')]" | |
| fi | |
| if [ -n "${missing// /}" ]; then | |
| # Report both sides and the zoom flag, not just the difference. Whether a | |
| # pane is EXPECTED on screen turns entirely on zoom — a sibling of a zoomed | |
| # pane is absent by design — so naming the missing pane alone accuses the | |
| # app in a sentence that reads identically when the harness is the one that | |
| # is wrong. | |
| note_fail "$iter" "visible $window: tmux panes [$missing] are not on screen in the app (coverage would silently drop them) zoom=$zoom_state expected=[$(printf '%s' "$tmux_panes" | tr '\n' ' ')] app_on_screen=[$(printf '%s' "$app_panes" | tr '\n' ' ')]" | |
| fi |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@scripts/remote-tmux-live-fuzz.sh`:
- Around line 330-342: Update the visibility validation around the existing
missing comparison to compute and reject both set differences: tmux panes absent
from app_on_screen and app_on_screen panes absent from tmux. Use the structured
pane IDs as the authoritative sets, and include unexpected panes in the existing
failure report so a zoomed sibling left visible cannot produce a false pass.
In `@Sources/Debug/RemoteTmux/TerminalController`+RemoteTmuxTestSupport.swift:
- Around line 430-435: Update the sizing-owner validation in the remote tmux
test-support judge to use the workspace’s selected tab and its selected mirrored
window as the authoritative owner. Require exactly that mirror to be present in
owners; when the selected tab has no mirror, require owners to be empty,
preserving fail-closed behavior instead of accepting a sole hidden stale owner.
🪄 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: 1b964c5b-e6e1-4ea1-a2b2-97c83661880d
📒 Files selected for processing (3)
Sources/Debug/RemoteTmux/TerminalController+RemoteTmuxTestSupport.swiftscripts/remote-tmux-fuzz-marathon.shscripts/remote-tmux-live-fuzz.sh
ejc3
force-pushed
the
remote-tmux-fuzz-can-see-reveal
branch
from
July 16, 2026 14:10
39eb767 to
e43b8e0
Compare
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
The live fuzz reported 125 clean iterations against a mirror bug that was
reproducible by hand: select a tab in a mirrored tmux session and its panes keep
the size they had before they were hidden. Every part of the harness that should
have caught it was structurally unable to.
`sizing_settled` opened with `guard mirror.isEffectivelyVisibleForSizing else
{ continue }`. That ANDs the mirror's own visibility flag into the judge's entry
condition, so when the flag is the thing that is wrong the window is dropped from
the report rather than failing it — the defect blinds the judge instead of
reddening it. It now reads view state directly, compares that against the flag,
and reports the disagreement; `settled` consumes `mismatches`, so a report can
actually fail something. A stale flag on a switched-away mirror is invisible
per-window (its panes are parked offscreen, so judging their grids reports
phantoms), so "at most one mirror per session owns sizing" is asserted at the
session level, where two owners are countable.
The fuzz had never switched a mirror tab. Op 1's `select-window` is tmux-side and
cmux deliberately never follows tmux's current window, so the whole reveal path
was unreachable from here. Op 10 switches the cmux tab through the same route a
click takes and then polls until a different window is shown — focus can be
swallowed mid-mutation, and an op that quietly does nothing while reporting
coverage is how this class stayed hidden. Op 6 keeps two multi-pane windows alive
so op 10 has somewhere to switch to; only multi-pane windows have mirrors.
The coverage census read `list-panes` as the displayed set. Zoom hides panes
without closing them, so every sibling of a zoomed pane counted as coverage the
app had dropped — five spurious failures in one seed. It now gates on
`#{window_zoomed_flag}` and expects the pane tmux actually draws.
Two smaller things the last debugging round wanted:
Refuse to run when TMUX_TMPDIR does not exist. tmux ignores a missing directory
and falls back to /tmp/tmux-$UID — the developer's own server, which this harness
kills and resizes freely.
Report the terms `settled` is made of. An unsettled window whose claim, derivable
and layout all agreed with zero mismatches gave no clue which term was false.
No product change: the reveal bug does not reproduce on main. Verified by taking
main's sources, flipping contentViewLifecycle back to .recreateOnSwitch, and
asserting the defect's own signature (panes on screen while visible_for_sizing is
false) across four tab switches — green. RemoteTmuxSizingUITests stays 15/15.
A settle failure reported an iteration number and 300 characters of a pretty-printed payload, which is mostly indentation and stops inside the first window — before the terms that say which one is false. Chasing one meant re-running the seed and counting ops by hand. Three things, all reporting: The payload is compacted with jq and every window kept whole, so the `why` terms survive into the log. The cap is now 4000 characters and says what it dropped instead of trailing off mid-token. Each iteration records the ops behind it, so a failure names its cause (`iter=10/25 ops=3 [op4,op1,op8]`) rather than leaving the first question about any fuzz failure unanswerable. `why` gains the mirror's banked render frame and its live container. A native-geometry mismatch says the panes disagree with a plan, and the plan is only as good as the parent it was built from; without both, a judge planning from a drifted parent is indistinguishable from an app that failed to lay out.
A native-geometry mismatch named the pane, the plan and the view and stopped there, which leaves the only useful question unanswered: the size was lost somewhere, and the message points at everything and nothing. Each mismatch now carries the hosted view's enclosing containers with their own sizes, and for a split how many panes it still arranges. The first run of it answered a question two marathons had not: the panes are portal-hosted at the window level, so a mismatch is the portal's frame against the plan and not bonsplit's layout at all — and the split the pane is synced to still arranges a sibling the tmux layout no longer has, which is why nothing corrects it. With the sibling gone there is no divider to move, so re-imposing geometry is a no-op. An arranged count beside the tree's own child count is what makes that visible rather than inferred.
The judge compared a pane's frame against a plan built from the tmux layout and left the obvious question open: do the app and bonsplit even agree on the tree? Every answer to that was a guess, and this judge kept inviting the guess. `why` now carries `tree_matches_layout` — bonsplit's own logical tree checked against the layout the plan came from. It earns its place immediately. On the deterministic seed-2 failure it reports true while a pane sits at half its planned width, which rules out the whole family of stale-tree explanations and says the divergence is downstream of the tree: the panes are portal-hosted at the window level, so a frame tracks an anchor inside the split tree, and the anchor can be stale while the tree is right. That also explains why nothing corrected it. The reconcile consults this same predicate to decide whether to rebuild, so a correct tree means it declines — correctly — and no divider remains in that tree to impose against.
Five ways this harness could still report more than it delivered, four of them the same fault it was written to remove. The judge turned missing derivation evidence into success. `derivable` is nil when a mirror has no container, and `?? true` then called derivation settled — while every other term sat true on a cached plan, so precisely the window that could not re-derive its claim settled green. It also keyed off `isVisibleForSizing`, the flag under test, which hands the defect a way to excuse itself. It now keys off the independent view-state read and requires a grid for anything on screen. Op 10 could bank a switch it never landed. It kept only the window it started from and accepted any *other* mirrored window as proof, so a third mirrored window or a concurrent selection satisfied it without the target ever being shown. It now records the target's window and requires that one. Its target filter used jq `inside`, which compares strings by containment: `["@3"] | inside(["@30","@1"])` is true. Once tmux hands out @10 and up — routine across a marathon — @1 passed as mirrored on the strength of @10. Exact membership now. The census skipped a window tmux could not list, and another window's success kept the iteration non-vacuous, so a mirror left on screen for a closed window was compared against nothing. That is the finding, so it says so. The digest capped at 4000 characters, which cuts mid-window and can drop the failing window's `why` outright depending on dictionary order, leaving output that is no longer JSON; and it discarded jq's status, so one valid value followed by garbage read as a clean digest. It keeps the unsettled windows whole instead, and honors the status. Also: zero mirror-tab switches is no longer a per-seed failure. Whether a seed draws op 10 is the RNG's business — one seed in thirty never draws it — and filing that beside real defects is how a gate gets ignored. The seed reports its coverage; the marathon asserts the floor across seeds, where the draw evens out, and fails only if no seed landed one. Dropped the ancestry diagnostic: panes are portal-hosted onto one flat host view, so a hosted view's ancestors are the same three views for every pane and its NSSplitView branch could never print. It answered its one question — the miss is in the portal, not bonsplit's layout — and the honest version reports the ANCHOR's chain, which needs a registry API this does not have.
Both review bots and an independent pass converged on the same two holes, which is a fair signal they were real. The census compared one direction only: panes tmux draws that the app lacks. The zoom gate narrows the expected set to the zoomed pane alone, and it narrows it in exactly the direction the one-sided diff cannot see — an app that keeps painting a zoomed pane's siblings produces a superset and the check comes back empty. Both differences are rejected now, and the failure names both sides. A SOLE stale owner escaped the owner count: one mirror keeps the flag while hidden, the tab now selected is a single-pane window with no mirror, so the count is one, nothing is on screen, and no per-window check fires. Each owner is now checked against the product's own rule rather than counted. `panelVisibleInUI` is `isWorkspaceVisible && (isSelectedInPane || isFocused)`; its first input is view-level state this judge cannot reach, so the full equality is not recomputable — but the implication is: holding the flag REQUIRES the panel be selected in its pane or focused, and both are model state. An owner that is neither is stale. Necessary-condition only, so it cannot fire on the legitimately hidden-but-selected mirror that makes the on-screen assertion directional. Also dropped the duplicate zoom_state derivation the outer branch already did.
ejc3
force-pushed
the
remote-tmux-fuzz-can-see-reveal
branch
from
July 16, 2026 22:23
e43b8e0 to
05e2781
Compare
Contributor
Author
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.
The sizing fuzz could not fail on a mirror it wasn't showing, so the operation that
reveals one was never exercised, and when it did fail it could not say why. This is
harness-only — no product file changes.
What it could not see
The coverage census read
list-panes, which is blind to zoom: it reports the wholebase tree, so the census compared the app against panes tmux wasn't drawing. It now
gates on
window_zoomed_flagand, for a zoomed window, expects the pane tmuxactually draws (
select-paneunzooms, sozoom=1 && pane_active=1names ituniquely). It also reports both sides and the flag it compared, because whether a
pane is expected on screen turns entirely on zoom — a sibling of a zoomed pane is
absent by design, and naming the missing pane alone reads as an accusation either
way.
Nothing ever switched between two mirrored tabs, which is the whole reveal path.
Op 10 does, drawing targets only from mirrored windows, and it has to prove the
window it asked for is the one now shown: keeping only the window it started from
and accepting any other mirrored window let a third window or a concurrent
selection satisfy it without the target appearing at all.
TMUX_TMPDIRpointing at a directory that does not exist makes tmux fall back tothe default socket, so a run could quietly drive the wrong server. It is checked
before anything mutates.
What a failure now says
It used to print an iteration number and 300 characters of a pretty-printed
payload — mostly indentation, cut off inside the first window, before the terms
that say which one is false. Now each iteration records the ops behind it
(
iter=10/25 ops=3 [op4,op1,op8]), the digest keeps the unsettled windows whole,and
whycarries the termssettledis made of, including the banked render framebeside the live container and whether bonsplit's own tree agrees with the tmux
layout.
That reporting earned itself immediately. It located a real defect that two
marathons had only pointed at: the panes are portal-hosted at the window level, so
a pane's frame tracks an anchor inside the split tree;
tree_matches_layoutreportstrue while a pane sits at half its planned width, which rules out every stale-tree
explanation and puts the divergence downstream of the tree.
The judge asserts rather than filters
It opened by skipping any mirror that was not visible-for-sizing — an AND that
could only shrink the judged set, so the defect blinded the judge instead of
reddening it. It now reads view state directly rather than through the mirror's own
notion of visibility, which is the thing under test. Same fault, twice more:
derivation parity keyed off that flag and defaulted to settled when no grid could
be derived, so the one window that could not re-derive its claim passed while every
other term sat true on a cached plan.
The assertions are directional on purpose: a mirror can legitimately hold the flag
while off screen (a selected tab in a background window), so only the reverse is a
defect. A known gap is stated rather than implied — a sole stale owner is not
caught; two owners, and a shown mirror that owns nothing, are.
Zero mirror-tab switches is reported per seed but asserted across the marathon.
Whether a seed draws op 10 is the RNG's business — about one seed in thirty never
draws it — and filing that beside real defects is how a gate gets ignored.
Status
An adversarial pass over this diff found five more ways it could report more than it
delivered, four of them the same fault above; all are fixed in the last commit and
verified by running them. Among them a real one worth naming: the op-10 target
filter used jq's
inside, which compares strings by containment, so["@3"] | inside(["@30","@1"])is true and@1passed as mirrored on the strengthof
@10once tmux hands out double digits.The marathon this improves is not green, and the reason is a confirmed defect
this PR neither introduces nor can fix. After a pane closes, bonsplit's logical tree
collapses correctly —
bonsplitTreeMatchesreports true — while its AppKitcontainer hierarchy keeps the old arrangement, so the portal syncs the surviving
pane to a stale anchor at its former width. cmux cannot detect it: every check it
owns asks the tree whether the views are right, so it cannot fail when the views
diverge from the tree. The fix belongs in bonsplit's
closePaneteardown and needsits own change there.
Measured on this branch, 125 iterations per arm: the mirror-identity fix removes a
nine-failure wedge on one seed outright and durably, and two seeds fail on the
bonsplit divergence in every arrangement tried. The fuzz is not run in CI, so
nothing here gates the build;
RemoteTmuxSizingUITestsis 12/14, which is whatpristine main scores on the same machine (its two failures are a
lab server never startedsetup flake whose failing pair shuffles between runs).Summary by CodeRabbit