Repository navigation
Conversation
|
@ejc3 is attempting to deploy a commit to the Manaflow Team on Vercel. A member of the Team first needs to authorize it. |
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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:
📝 WalkthroughWalkthroughRemote tmux mirror sizing now uses quantized native split plans, applies planned Bonsplit divider extents, tracks pane render grids, and exposes DEBUG settlement diagnostics. Tests and shell harnesses validate resizing, layout stability, pane rendering, and seeded fuzz behavior. ChangesRemote tmux layout pipeline
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant RemoteTmuxWindowMirror
participant RemoteTmuxNativeSplitLayout
participant RemoteTmuxNativeLayoutMetrics
participant BonsplitTree
RemoteTmuxWindowMirror->>RemoteTmuxNativeSplitLayout: plan measured tree for container size
RemoteTmuxNativeSplitLayout->>RemoteTmuxNativeLayoutMetrics: allocate rails and child sizes
RemoteTmuxNativeLayoutMetrics-->>RemoteTmuxNativeSplitLayout: return quantized extents
RemoteTmuxNativeSplitLayout-->>RemoteTmuxWindowMirror: return recursive Plan
RemoteTmuxWindowMirror->>BonsplitTree: apply planned extents and fractions
BonsplitTree-->>RemoteTmuxWindowMirror: return actual divider positions
sequenceDiagram
participant LiveFuzzer
participant TerminalController
participant RemoteTmuxWindowMirror
LiveFuzzer->>TerminalController: request remote.tmux.sizing_settled
TerminalController->>RemoteTmuxWindowMirror: inspect visible mirror windows
RemoteTmuxWindowMirror-->>TerminalController: return claimed sizes and pane mismatches
TerminalController-->>LiveFuzzer: return settlement response
Possibly related PRs
Suggested reviewers: Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (3 errors, 2 warnings)
✅ Passed checks (20 passed)
✨ 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 |
70254eb to
f1340c0
Compare
Greptile SummaryThis PR makes mirrored tmux panes render at their assigned cell sizes. The main changes are:
Confidence Score: 5/5This looks safe to merge.
Important Files Changed
Reviews (8): Last reviewed commit: "portal: skip geometry-sync passes whose ..." | Re-trigger Greptile |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
f1340c0 to
34c83bf
Compare
34c83bf to
04167c7
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
🤖 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/RemoteTmuxNativeMirrorLayoutFuzzTests.swift`:
- Around line 105-125: Track how many regimes pass the minimum-size guard and
execute their assertions in the fuzz loop around clientGrid, incrementing the
counter only when a regime is actually tested. After each trial, assert that the
expected coverage was reached so all regimes being skipped cannot produce a
passing test; preserve the existing continue behavior for legitimately
undersized claims.
In `@scripts/remote-tmux-fuzz-marathon.sh`:
- Around line 24-31: Update relaunch_app and the evidence-capture flow to
replace fixed sleep delays with deadline-bounded checks: wait until the previous
app process exits, then wait until the relaunched app’s socket is ready, and
wait for sample to signal completion before relaunching. Apply the same
readiness/completion handling to the additional lines referenced by the review,
while preserving timeout behavior so the script cannot wait indefinitely.
- Around line 53-67: Update the result-classification order in the marathon loop
so the app-pid check occurs before the rc 99 hang branch. When app_pid is
absent, classify the run as a crash, capture crash evidence, and relaunch; only
classify rc 99 as a hang when the application is still running.
- Around line 77-82: Update the final exit logic in the marathon script to
return a boolean failure status: exit 0 when hangs, crashes, and fails are all
zero, and exit nonzero otherwise. Do not use the aggregate count directly,
preserving the existing summary output and failure categories.
In `@scripts/remote-tmux-layout-repro.sh`:
- Around line 175-191: Update attach_mirror so the structured window= value
returned by cli ssh-tmux is authoritative: remove the window_ids fallback and
return failure when MIRROR_WINDOW is empty, while preserving the existing
nonzero-status handling and successful identifier assignment.
- Around line 194-210: Update panel_id_for_title to request the list-panels
identifier format that provides the fields it consumes, such as adding
--id-format both to the cli invocation, or alternatively update the parser to
consume the returned stable ref field. Ensure matching panels produce an
identifier instead of being reported missing when id is suppressed.
In `@scripts/remote-tmux-live-fuzz.sh`:
- Around line 83-94: Update the setup flow around CONNECT_OUT and WINDOW_ID,
plus the corresponding reconnect blocks, to fail immediately when ssh-tmux
cannot provide the required attach identifiers, including WINDOW_ID and
WORKSPACE_REF. Preserve DRY mode behavior, but validate non-dry connection
output and stop with a clear error instead of continuing with empty identifiers
or discarding reconnect failures.
- Around line 154-155: Update random_window and random_pane to select entries
using the script’s seeded rand function instead of sort -R. Materialize the
listed windows or panes, choose an indexed entry, and return the selection
through the established global variable so command substitution does not discard
RNG state and seeded runs remain reproducible.
- Around line 109-129: Update the settlement loop around the sizing_settled RPC
to verify the timeout command succeeds and the response is valid JSON before
evaluating settlement. Distinguish a structurally valid empty window list from
timeout, unsupported-method, or malformed responses; only accept the former as
settled, and keep retrying or reporting failure for invalid RPC results.
- Around line 130-151: Update the pane-validation loop to implement the
mirror-text oracle: resolve each tmux pane through its structured panel
reference for the selected workspace, obtain that panel’s app output via the CLI
read-screen operation, normalize it consistently with want, and compare the two
values. Keep the existing ruler-width confirmation as a separate check, and
report mismatches through note_fail.
- Around line 267-268: Update the final exit-status handling in the fuzz script
so the raw failure count is never returned directly: reserve exit codes 97 and
99 for their documented meanings, and map ordinary failure counts to a safe
non-reserved nonzero status without allowing shell exit-code wrapping to produce
zero. Preserve zero for successful runs and continue reporting the failure count
in the completion message.
In `@Sources/RemoteTmuxNativeLayoutMetrics.swift`:
- Around line 17-23: Remove the unused pixelsPerPoint property and its
device-pixel quantization documentation from RemoteTmuxNativeLayoutMetrics,
unless the layout’s rounding calculations are updated to actually consume it.
Keep the existing extent and cell-span behavior unchanged.
In `@Sources/RemoteTmuxWindowMirror`+Bonsplit.swift:
- Around line 136-147: Update the guard failure in the plan/tree matching logic
to emit the DEBUG mismatch log for either shape discrepancy: a split tree with a
leaf plan or a leaf tree with a split plan. Preserve the existing early return
and avoid logging when both shapes are compatible.
In `@Sources/TerminalController`+RemoteTmuxTestSupport.swift:
- Around line 153-219: In v2RemoteTmuxSizingSettled, leave the diagnostic logic
unchanged; the O(n²) pane traversal is acceptable because this is DEBUG-only
fuzz-harness polling and does not affect production behavior.
In `@vendor/bonsplit`:
- Line 1: Publish the Bonsplit submodule commit to origin/main before updating
the parent repository’s pinned SHA. Ensure the submodule reference resolves to
the commit on origin/main rather than refs/pull/176/head, then pin the parent
repository to that published commit.
🪄 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: ca83bb80-35e1-4bb0-8128-9593251429a7
📒 Files selected for processing (24)
Packages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Wire/ControlCommandExecutionPolicy.swiftSources/RemoteTmuxLayoutNode.swiftSources/RemoteTmuxMirrorFrames.swiftSources/RemoteTmuxMirrorGeometry.swiftSources/RemoteTmuxNativeLayoutMetrics.swiftSources/RemoteTmuxNativeSplitLayout.swiftSources/RemoteTmuxNativeSplitTree.swiftSources/RemoteTmuxWindowMirror+Bonsplit.swiftSources/RemoteTmuxWindowMirror+BonsplitLayout.swiftSources/RemoteTmuxWindowMirror+DividerSizing.swiftSources/RemoteTmuxWindowMirror.swiftSources/TerminalController+DebugMethodNames.swiftSources/TerminalController+RemoteTmuxTestSupport.swiftSources/TerminalController.swiftcmux.xcodeproj/project.pbxprojcmuxTests/RemoteTmuxMirrorFeedForwardTests.swiftcmuxTests/RemoteTmuxMirrorGeometryTests.swiftcmuxTests/RemoteTmuxMirrorLayoutMathTests.swiftcmuxTests/RemoteTmuxNativeMirrorLayoutFuzzTests.swiftcmuxTests/RemoteTmuxRectPublicationTests.swiftscripts/remote-tmux-fuzz-marathon.shscripts/remote-tmux-layout-repro.shscripts/remote-tmux-live-fuzz.shvendor/bonsplit
💤 Files with no reviewable changes (2)
- Sources/RemoteTmuxMirrorFrames.swift
- cmuxTests/RemoteTmuxMirrorGeometryTests.swift
| relaunch_app() { | ||
| local pid; pid=$(app_pid) | ||
| [ -n "$pid" ] && kill -9 "$pid" 2>/dev/null | ||
| sleep 2 | ||
| # Launch through the login shell so the app runs outside any sandbox. | ||
| ssh cmux-srvA "zsh -lc 'open \"$APP\"'" >/dev/null 2>&1 | ||
| sleep 6 | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Wait for real completion instead of fixed durations.
The 2/6-second relaunch waits do not prove that the old process exited or the new socket is ready. Likewise, the 8-second evidence wait can relaunch the app before sample finishes. Use deadline-bounded process/socket checks and a completion signal for sample.
Also applies to: 37-40
🧰 Tools
🪛 Shellcheck (0.11.0)
[info] 29-29: Note that, unescaped, this expands on the client side.
(SC2029)
🤖 Prompt for 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.
In `@scripts/remote-tmux-fuzz-marathon.sh` around lines 24 - 31, Update
relaunch_app and the evidence-capture flow to replace fixed sleep delays with
deadline-bounded checks: wait until the previous app process exits, then wait
until the relaunched app’s socket is ready, and wait for sample to signal
completion before relaunching. Apply the same readiness/completion handling to
the additional lines referenced by the review, while preserving timeout behavior
so the script cannot wait indefinitely.
| @@ -1 +1 @@ | |||
| Subproject commit cd4e4c27e7b34cd8840c02f96b99c4b442715b94 | |||
| Subproject commit cc818aebed46e31bd7aafd3aacff8b8a17f81b0e | |||
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
git -C vendor/bonsplit fetch origin
git -C vendor/bonsplit cat-file -e cc818aebed46e31bd7aafd3aacff8b8a17f81b0e^{commit}
git -C vendor/bonsplit merge-base --is-ancestor \
cc818aebed46e31bd7aafd3aacff8b8a17f81b0e origin/main
git -C vendor/bonsplit show --stat --oneline \
cc818aebed46e31bd7aafd3aacff8b8a17f81b0eRepository: manaflow-ai/cmux
Length of output: 331
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== remote refs containing the SHA =="
git -C vendor/bonsplit ls-remote origin | awk '$1 == "cc818aebed46e31bd7aafd3aacff8b8a17f81b0e" {print}'
echo "== local reachability from fetched refs =="
git -C vendor/bonsplit branch -r --contains cc818aebed46e31bd7aafd3aacff8b8a17f81b0e || true
echo "== ancestry check against origin/main =="
if git -C vendor/bonsplit merge-base --is-ancestor cc818aebed46e31bd7aafd3aacff8b8a17f81b0e origin/main; then
echo "ancestor=yes"
else
echo "ancestor=no"
fiRepository: manaflow-ai/cmux
Length of output: 482
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== resolve remote main tip =="
git -C vendor/bonsplit rev-parse origin/main
echo "== show remote branches =="
git -C vendor/bonsplit for-each-ref --format='%(refname:short) %(objectname)' refs/remotes/originRepository: manaflow-ai/cmux
Length of output: 8318
Push the Bonsplit submodule commit to origin/main before pinning it
The new SHA only appears on refs/pull/176/head; origin/main is still at 020c768472e2fb291f785296e61841d21cbecd94, so this parent repo is pointing at a PR ref instead of a published submodule commit.
🤖 Prompt for 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.
In `@vendor/bonsplit` at line 1, Publish the Bonsplit submodule commit to
origin/main before updating the parent repository’s pinned SHA. Ensure the
submodule reference resolves to the commit on origin/main rather than
refs/pull/176/head, then pin the parent repository to that published commit.
Source: Path instructions
There was a problem hiding this comment.
The pointer targets the branch behind manaflow-ai/bonsplit#176 (this PR depends on it, noted in the body); the two merge together, at which point the pointer is an ancestor of bonsplit main. Happy to hold this PR until #176 lands if you prefer the pointer to only ever reference main.
There was a problem hiding this comment.
@ejc3, that makes sense — thanks for the context. Since this PR depends on manaflow-ai/bonsplit#176, pinning to that branch commit is a reasonable interim state. A couple of practical notes to weigh against holding this PR:
- Anyone else who clones/CI's this branch before
#176merges will need access to that PR ref (or the fork/branch) to resolve the submodule, which can be brittle depending on your CI's submodule fetch configuration (e.g., if it only fetchesrefs/heads/*or shallow-clonesmain). - If
#176changes further before merging (force-push, rebase), the pinned SHA here could go stale or become unreachable.
Given those tradeoffs, holding this PR until #176 lands and then re-pinning to the merged main commit is the safer path if your CI can tolerate the wait — but pinning to the PR-branch commit temporarily is a defensible call if you need this to keep moving and are confident #176 will merge as-is. I'll leave the choice to you; no action needed on my end either way.
Pull the mirror's divider-fraction walk out of the bonsplit applier into RemoteTmuxNativeSplitLayout.plan, a pure function from (measured tree, metrics, container size) to per-split fractions and per-pane outer sizes, modeling the native split view's whole-point division. The mirror now just zips the plan onto the bonsplit tree. Add a seeded fuzz that drives random layouts and containers through the claim (clientGrid), a tmux-style cell assignment, and this exact walk, then derives each pane's rendered grid from its outer size the way the terminal surface does. It asserts every pane renders at least its assigned span — one column short means every full-width line in that pane wraps. The fuzz FAILS on this commit: proportional ideal-over-ideal fractions let whole-point rounding compound down the binary split chain, so in a near-exact container the deepest panes come up short. The fix follows.
04167c7 to
9aa047d
Compare
| if printf '%s' "$settled_json" | grep -q '"settled" : true' \ | ||
| && ! printf '%s' "$settled_json" | grep -q '"settled" : false'; then | ||
| break | ||
| fi |
There was a problem hiding this comment.
The RPC now records an unsampled pane as a no-sample mismatch, but this condition only checks the separate settled flags. Those flags compare the claimed and assigned window sizes without considering pane mismatches. During pane creation or attachment churn, every window can therefore report settled: true while a new pane still has no sample, causing the loop to accept an unchecked transition. Require the response to contain neither missing samples nor rendered-grid mismatches before breaking.
| if printf '%s' "$settled_json" | grep -q '"settled" : true' \ | |
| && ! printf '%s' "$settled_json" | grep -q '"settled" : false'; then | |
| break | |
| fi | |
| if printf '%s' "$settled_json" | grep -q '"settled" : true' \ | |
| && ! printf '%s' "$settled_json" | grep -q '"settled" : false' \ | |
| && ! printf '%s' "$settled_json" | grep -q 'no-sample' \ | |
| && ! printf '%s' "$settled_json" | grep -q 'rendered='; then | |
| break | |
| fi |
There was a problem hiding this comment.
Real hole, thanks — the break keyed off the settled flags alone, and the post-break judgment greps only rendered=, so a no-sample entry at settle would slip through unchecked. The break now also waits out no-sample (a pane that has not reported is still in transition, same as an unsettled window). rendered= stays out of the break condition on purpose: a pane wrong AT settle is exactly the bug this harness exists to capture, and blocking the break on it would misreport every real failure as "never settled".
There was a problem hiding this comment.
You're right on the rendered= part — blocking the break on it would turn every real rendering bug into a false "never settled" timeout. The break condition's job is to answer "are we done transitioning?", and a wrong-but-stable grid is done; the post-break check is what turns it into a FUZZ FAIL. My suggestion conflated the two.
The refined fix — no-sample blocks the break, rendered= does not:
| if printf '%s' "$settled_json" | grep -q '"settled" : true' \ | |
| && ! printf '%s' "$settled_json" | grep -q '"settled" : false'; then | |
| break | |
| fi | |
| if printf '%s' "$settled_json" | grep -q '"settled" : true' \ | |
| && ! printf '%s' "$settled_json" | grep -q '"settled" : false' \ | |
| && ! printf '%s' "$settled_json" | grep -q 'no-sample'; then | |
| break | |
| fi |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (10)
Sources/RemoteTmuxWindowMirror+Bonsplit.swift (1)
142-167: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winMismatch logging is still asymmetric — one drift direction stays silent.
This is the same gap flagged in a prior review: the guard only logs when
treeNodeis provably.splitbutplanresolved to.leaf. WhentreeNodeis a leaf butplanexpected a.split(about to imposefirstExtent/fractiononfirst/second),if case .split = treeNodeisfalse, so nothing is logged — yet the function still returns silently, leaving dividers below it stale. The doc comment above the function explicitly says this should be loud in DEBUG for a mismatch, but only one of the two drift directions currently is.🐛 Proposed fix to close the asymmetric mismatch-logging gap
else { - if case .split = treeNode { - `#if` DEBUG - cmuxDebugLog("remote.divider.plan mismatch: plan leaf vs bonsplit split") - `#endif` - } + `#if` DEBUG + if case .split = treeNode { + cmuxDebugLog("remote.divider.plan mismatch: plan leaf vs bonsplit split") + } else if case .split = plan { + cmuxDebugLog("remote.divider.plan mismatch: bonsplit leaf vs plan split") + } + `#endif` return }🤖 Prompt for 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. In `@Sources/RemoteTmuxWindowMirror`+Bonsplit.swift around lines 142 - 167, Update applyDividerPositions so its DEBUG mismatch logging covers both plan/treeNode drift directions: log when plan is a split but treeNode is a leaf, as well as the existing treeNode-split/plan-leaf case. Keep the early return behavior unchanged and preserve the existing diagnostic context.scripts/remote-tmux-layout-repro.sh (2)
194-211: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winRequest an identifier format
panel_id_for_titleactually consumes.
list-panelsmay suppressidwhen a stable ref exists, but this parser only readsidorhandle. Same unresolved concern as the prior review pass.Proposed fix
- cli --json list-panels --window "$MIRROR_WINDOW" 2>/dev/null | "$PYTHON_BIN" -c ' + cli --json --id-format both list-panels --window "$MIRROR_WINDOW" 2>/dev/null | "$PYTHON_BIN" -c '🤖 Prompt for 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. In `@scripts/remote-tmux-layout-repro.sh` around lines 194 - 211, Update panel_id_for_title to extract the stable reference field emitted by list-panels when id is suppressed, while retaining the existing id and handle fallbacks. Ensure the function returns an identifier format accepted by its callers.
175-192: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winFallback heuristic for
MIRROR_WINDOWhas resurfaced — fail closed instead.This is the same concern raised previously (and marked addressed in commits 6fc44b0–9aa047d), but the current revision still falls back to
window_ids | tail -1when the structuredwindow=field is empty. A heuristic "newest cmux window" pick can validate an unrelated window and produce false-positive/false-negative results silently.🐛 Proposed fix
MIRROR_WINDOW="$(printf '%s\n' "$output" | awk -F'window=' '/^OK host=/{split($2,a," "); print a[1]; exit}')" - if [ -z "$MIRROR_WINDOW" ]; then - # Fallback for older CLI output: pick the key/newest cmux window. - MIRROR_WINDOW="$(window_ids | tail -1)" - fi - [ -n "$MIRROR_WINDOW" ] + [ -n "$MIRROR_WINDOW" ] || { + echo "ERROR: ssh-tmux did not return a mirror window identifier" >&2 + return 1 + } }Once removed,
window_ids()(lines 175-177) has no remaining caller and can be deleted too.As per coding guidelines, "a missing reliable signal must fail closed ... rather than guessing."
🤖 Prompt for 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. In `@scripts/remote-tmux-layout-repro.sh` around lines 175 - 192, Remove the `window_ids | tail -1` fallback from `attach_mirror`; when the structured `window=` field is absent, return failure instead of selecting a guessed window. Delete the now-unused `window_ids` function as well.Source: Coding guidelines
scripts/remote-tmux-fuzz-marathon.sh (3)
53-67: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winCrash still gets misclassified as a hang before the app-gone check runs.
Unresolved:
rc -eq 99is still checked before[ -z "$(app_pid)" ], so a crash whose childpingnaturally exits 99 records "HANG" instead of "CRASH".Proposed fix
if [ "$rc" -eq 97 ]; then ... - elif [ "$rc" -eq 99 ]; then - hangs=$((hangs + 1)) - echo "seed=$seed HANG (evidence: seed-$seed-hang-*)" - capture_evidence "$seed" hang - relaunch_app elif [ -z "$(app_pid)" ]; then crashes=$((crashes + 1)) echo "seed=$seed CRASH (app gone; evidence: seed-$seed-crash-*)" capture_evidence "$seed" crash relaunch_app + elif [ "$rc" -eq 99 ]; then + hangs=$((hangs + 1)) + echo "seed=$seed HANG (evidence: seed-$seed-hang-*)" + capture_evidence "$seed" hang + relaunch_app🤖 Prompt for 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. In `@scripts/remote-tmux-fuzz-marathon.sh` around lines 53 - 67, Reorder the result handling in the marathon loop so the app-gone check runs before the rc=99 hang branch. Ensure a missing app increments crashes, captures crash evidence, and relaunches the app even when rc is 99; retain the existing hang behavior only when the app is still running.
77-82: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winExit status still wraps modulo 256.
Unresolved:
exit $((hangs + crashes + fails))can wrap to 0 (false success) for large aggregate counts.Proposed fix
-exit $((hangs + crashes + fails)) +if [ $((hangs + crashes + fails)) -gt 0 ]; then + exit 1 +fi +exit 0🤖 Prompt for 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. In `@scripts/remote-tmux-fuzz-marathon.sh` around lines 77 - 82, Update the final exit-status handling in the marathon script so aggregate hangs, crashes, and failed seeds cannot wrap modulo 256 and produce a false success. Preserve the summary output, but normalize any nonzero aggregate result to a conventional nonzero exit status while retaining zero for a completely successful run.
24-44: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winFixed sleeps still stand in for real completion signals.
Same unresolved concern as before (which explicitly also called out lines 37-40, now
capture_evidence'ssleep 8at line 40): the relaunch waits don't confirm the old process exited or the new socket is ready, and the evidence-capturesleep 8can let the marathon relaunch the app beforesamplefinishes (compounded by the hardcodedtmux send-keys -t '1:0'target silently no-oping via2>/dev/nullif that session doesn't exist).🤖 Prompt for 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. In `@scripts/remote-tmux-fuzz-marathon.sh` around lines 24 - 44, The relaunch and evidence-capture flows rely on fixed sleeps instead of completion checks. Update relaunch_app to wait until the old PID exits and the newly launched app is ready, and update capture_evidence to wait for the sample command to finish successfully; avoid the hardcoded tmux target and silently ignored failures by using a validated session or direct execution. Preserve the existing evidence outputs and timeout safely if readiness or sampling does not complete.scripts/remote-tmux-live-fuzz.sh (4)
109-134: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winEmpty/failed settlement RPC is still accepted as settled without validating the response.
Unresolved: a
timeout'd, unsupported-method, or malformed response has no"window"substring and takes the same early-exit path as a genuinely empty (single-pane) window list, without checkingtimeout's exit status or parsing valid JSON first.🤖 Prompt for 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. In `@scripts/remote-tmux-live-fuzz.sh` around lines 109 - 134, The settlement polling logic around settled_json must distinguish a valid empty window list from timeout, unsupported-method, and malformed RPC responses. Capture and validate the timeout/CLI exit status and parse the response as valid JSON before allowing the single-pane empty-list path to break; otherwise treat the response as failed, reselect the workspace as appropriate, and continue polling.
289-290: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winFinal exit status still collides with reserved hang/inert codes.
Unresolved:
exit "$fail"can produce 97 or 99, whichremote-tmux-fuzz-marathon.shreserves for "inert" and "hang" respectively, and can also wrap to 0.Proposed fix
echo "FUZZ DONE seed=$SEED iters=$ITERS failures=$fail" -exit "$fail" +if [ "$fail" -gt 0 ]; then + exit 1 +fi +exit 0🤖 Prompt for 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. In `@scripts/remote-tmux-live-fuzz.sh` around lines 289 - 290, Update the final status handling after the “FUZZ DONE” output so the script’s exit code cannot equal the reserved inert or hang codes 97 and 99, and cannot wrap to 0 when failures exceed the valid range. Preserve direct failure reporting for ordinary counts while mapping reserved or overflowing values to a non-reserved failure status.
162-163: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winWindow/pane selection still uses unseeded
sort -Rinstead of the deterministicrand.Unresolved: this breaks reproducibility of target selection (and therefore the resulting layout) for a given
$SEED, undermining the "a seed reproduces the exact op sequence" guarantee stated in the header comment.🤖 Prompt for 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. In `@scripts/remote-tmux-live-fuzz.sh` around lines 162 - 163, Update random_window and random_pane to replace each unseeded sort -R selection with the script’s deterministic rand mechanism, ensuring both window and pane selection derive from $SEED while preserving their existing tmux target formats and single-item output behavior.
89-94: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winSetup still doesn't fail closed when attach identifiers are missing.
Unresolved: a failed
ssh-tmuxpipeline still leavesWINDOW_IDempty and setup continues;app_resize(165-170) silently no-ops on emptyWINDOW_IDrather than failing, and the reconnect op (line 230) still swallows errors (>/dev/null 2>&1), so a broken attach can produce a "passing" run that never actually exercised resize/reconnect.🤖 Prompt for 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. In `@scripts/remote-tmux-live-fuzz.sh` around lines 89 - 94, The setup flow around CONNECT_OUT and WINDOW_ID must fail closed when ssh-tmux cannot provide a valid attach identifier: validate the resolved WINDOW_ID and exit nonzero before continuing if it is empty or invalid, ensure app_resize does not silently accept an empty identifier, and make the reconnect operation propagate errors instead of swallowing them so failed attach/resize/reconnect runs cannot be reported as passing.
🤖 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 141-159: Extend the pane-validation loop around the tmux ruler
check to compare each pane’s mirror/read-screen output against the corresponding
tmux capture-pane text, using the existing `want` or mirror `read-screen`
mechanism described by the script header. Ensure every pane surface is checked
and call `note_fail` when the normalized mirror text differs from tmux output,
while preserving the existing ruler-width confirmation logic.
---
Duplicate comments:
In `@scripts/remote-tmux-fuzz-marathon.sh`:
- Around line 53-67: Reorder the result handling in the marathon loop so the
app-gone check runs before the rc=99 hang branch. Ensure a missing app
increments crashes, captures crash evidence, and relaunches the app even when rc
is 99; retain the existing hang behavior only when the app is still running.
- Around line 77-82: Update the final exit-status handling in the marathon
script so aggregate hangs, crashes, and failed seeds cannot wrap modulo 256 and
produce a false success. Preserve the summary output, but normalize any nonzero
aggregate result to a conventional nonzero exit status while retaining zero for
a completely successful run.
- Around line 24-44: The relaunch and evidence-capture flows rely on fixed
sleeps instead of completion checks. Update relaunch_app to wait until the old
PID exits and the newly launched app is ready, and update capture_evidence to
wait for the sample command to finish successfully; avoid the hardcoded tmux
target and silently ignored failures by using a validated session or direct
execution. Preserve the existing evidence outputs and timeout safely if
readiness or sampling does not complete.
In `@scripts/remote-tmux-layout-repro.sh`:
- Around line 194-211: Update panel_id_for_title to extract the stable reference
field emitted by list-panels when id is suppressed, while retaining the existing
id and handle fallbacks. Ensure the function returns an identifier format
accepted by its callers.
- Around line 175-192: Remove the `window_ids | tail -1` fallback from
`attach_mirror`; when the structured `window=` field is absent, return failure
instead of selecting a guessed window. Delete the now-unused `window_ids`
function as well.
In `@scripts/remote-tmux-live-fuzz.sh`:
- Around line 109-134: The settlement polling logic around settled_json must
distinguish a valid empty window list from timeout, unsupported-method, and
malformed RPC responses. Capture and validate the timeout/CLI exit status and
parse the response as valid JSON before allowing the single-pane empty-list path
to break; otherwise treat the response as failed, reselect the workspace as
appropriate, and continue polling.
- Around line 289-290: Update the final status handling after the “FUZZ DONE”
output so the script’s exit code cannot equal the reserved inert or hang codes
97 and 99, and cannot wrap to 0 when failures exceed the valid range. Preserve
direct failure reporting for ordinary counts while mapping reserved or
overflowing values to a non-reserved failure status.
- Around line 162-163: Update random_window and random_pane to replace each
unseeded sort -R selection with the script’s deterministic rand mechanism,
ensuring both window and pane selection derive from $SEED while preserving their
existing tmux target formats and single-item output behavior.
- Around line 89-94: The setup flow around CONNECT_OUT and WINDOW_ID must fail
closed when ssh-tmux cannot provide a valid attach identifier: validate the
resolved WINDOW_ID and exit nonzero before continuing if it is empty or invalid,
ensure app_resize does not silently accept an empty identifier, and make the
reconnect operation propagate errors instead of swallowing them so failed
attach/resize/reconnect runs cannot be reported as passing.
In `@Sources/RemoteTmuxWindowMirror`+Bonsplit.swift:
- Around line 142-167: Update applyDividerPositions so its DEBUG mismatch
logging covers both plan/treeNode drift directions: log when plan is a split but
treeNode is a leaf, as well as the existing treeNode-split/plan-leaf case. Keep
the early return behavior unchanged and preserve the existing diagnostic
context.
🪄 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: b0486706-3b4d-487d-980c-6bb90f97d6da
📒 Files selected for processing (29)
Packages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Wire/ControlCommandExecutionPolicy.swiftSources/AppDelegate.swiftSources/GhosttyTerminalView.swiftSources/RemoteTmuxLayoutNode.swiftSources/RemoteTmuxMirrorFrames.swiftSources/RemoteTmuxMirrorGeometry.swiftSources/RemoteTmuxNativeLayoutMetrics.swiftSources/RemoteTmuxNativeSplitLayout.swiftSources/RemoteTmuxNativeSplitTree.swiftSources/RemoteTmuxWindowMirror+Bonsplit.swiftSources/RemoteTmuxWindowMirror+BonsplitLayout.swiftSources/RemoteTmuxWindowMirror+DividerSizing.swiftSources/RemoteTmuxWindowMirror.swiftSources/RemoteTmuxWindowMirrorSplitView.swiftSources/TerminalController+DebugMethodNames.swiftSources/TerminalController+RemoteTmuxTestSupport.swiftSources/TerminalController.swiftcmux.xcodeproj/project.pbxprojcmuxTests/AppDelegateDisplayConfigRestoreTests.swiftcmuxTests/RemoteTmuxMirrorFeedForwardTests.swiftcmuxTests/RemoteTmuxMirrorGeometryTests.swiftcmuxTests/RemoteTmuxMirrorLayoutMathTests.swiftcmuxTests/RemoteTmuxNativeMirrorLayoutFuzzTests.swiftcmuxTests/RemoteTmuxRectPublicationTests.swiftscripts/remote-tmux-fuzz-host.shscripts/remote-tmux-fuzz-marathon.shscripts/remote-tmux-layout-repro.shscripts/remote-tmux-live-fuzz.shvendor/bonsplit
💤 Files with no reviewable changes (2)
- Sources/RemoteTmuxMirrorFrames.swift
- cmuxTests/RemoteTmuxMirrorGeometryTests.swift
| # Oracle 2: every ruler redrew to its actual pane size on the tmux side | ||
| # (a stale ruler would make on-screen text look mangled without any | ||
| # rendering bug — see op 8's comment). | ||
| for pane in $(t list-panes -s -t $SESSION -F '#{pane_id}'); do | ||
| local got | ||
| got=$(t display -t "$pane" -p '#{pane_width}') | ||
| local first | ||
| first=$(t capture-pane -p -J -t "$pane" 2>/dev/null | grep -m1 "^[0-9]*x[0-9]* 01" | wc -c | tr -d ' ') | ||
| if [ -n "$first" ] && [ "$first" -gt 1 ] && [ $((first - 1)) -ne "$got" ]; then | ||
| # The ruler redraws every 2s; confirm with a second read before | ||
| # calling it a failure, and tolerate the pane dying mid-check. | ||
| sleep 3 | ||
| got=$(t display -t "$pane" -p '#{pane_width}' 2>/dev/null) || continue | ||
| first=$(t capture-pane -p -J -t "$pane" 2>/dev/null | grep -m1 "^[0-9]*x[0-9]* 01" | wc -c | tr -d ' ') | ||
| if [ -n "$first" ] && [ "$first" -gt 1 ] && [ -n "$got" ] && [ $((first - 1)) -ne "$got" ]; then | ||
| note_fail "$iter" "pane $pane tmux-side ruler ${first}c != width ${got} after confirm" | ||
| fi | ||
| fi | ||
| done |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Mirror-text oracle is still not implemented — only tmux is compared against itself.
Unresolved: want/mirror read-screen comparison described in the file's own header comment ("per-pane text: read-screen output equals tmux capture-pane for every pane surface") is not present in this loop; it only checks tmux's ruler width against tmux's own pane width, so mirror-side text corruption still can't be caught here.
🤖 Prompt for 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.
In `@scripts/remote-tmux-live-fuzz.sh` around lines 141 - 159, Extend the
pane-validation loop around the tmux ruler check to compare each pane’s
mirror/read-screen output against the corresponding tmux capture-pane text,
using the existing `want` or mirror `read-screen` mechanism described by the
script header. Ensure every pane surface is checked and call `note_fail` when
the normalized mirror text differs from tmux output, while preserving the
existing ruler-width confirmation logic.
9aa047d to
dd1a8cb
Compare
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/RemoteTmuxWindowMirror.swift`:
- Around line 90-95: Update isEffectivelyVisibleForSizing to determine the
hosted window from a pane ID in renderedLayout.paneIDsInOrder rather than
panelsByPaneId.values.first. Only use the rendered pane’s hostedView.window for
the visibility check, preserving false when no rendered pane or window is
available.
🪄 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: 7a77aa2c-6e9d-4f60-a18b-8579d1139343
📒 Files selected for processing (27)
Packages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Wire/ControlCommandExecutionPolicy.swiftSources/AppDelegate.swiftSources/GhosttyTerminalView.swiftSources/RemoteTmuxLayoutNode.swiftSources/RemoteTmuxMirrorFrames.swiftSources/RemoteTmuxMirrorGeometry.swiftSources/RemoteTmuxNativeLayoutMetrics.swiftSources/RemoteTmuxNativeSplitLayout.swiftSources/RemoteTmuxNativeSplitTree.swiftSources/RemoteTmuxWindowMirror+Bonsplit.swiftSources/RemoteTmuxWindowMirror+BonsplitLayout.swiftSources/RemoteTmuxWindowMirror+DividerSizing.swiftSources/RemoteTmuxWindowMirror.swiftSources/RemoteTmuxWindowMirrorSplitView.swiftSources/TerminalController+DebugMethodNames.swiftSources/TerminalController+RemoteTmuxTestSupport.swiftSources/TerminalController.swiftcmux.xcodeproj/project.pbxprojcmuxTests/AppDelegateDisplayConfigRestoreTests.swiftcmuxTests/RemoteTmuxMirrorFeedForwardTests.swiftcmuxTests/RemoteTmuxMirrorGeometryTests.swiftcmuxTests/RemoteTmuxMirrorLayoutMathTests.swiftcmuxTests/RemoteTmuxNativeMirrorLayoutFuzzTests.swiftcmuxTests/RemoteTmuxRectPublicationTests.swiftscripts/remote-tmux-fuzz-marathon.shscripts/remote-tmux-live-fuzz.shvendor/bonsplit
💤 Files with no reviewable changes (3)
- Sources/RemoteTmuxMirrorFrames.swift
- cmuxTests/RemoteTmuxMirrorGeometryTests.swift
- cmux.xcodeproj/project.pbxproj
| var isEffectivelyVisibleForSizing: Bool { | ||
| guard isVisibleForSizing else { return false } | ||
| guard let window = panelsByPaneId.values.first?.hostedView.window else { | ||
| return false | ||
| } | ||
| return window.isVisible |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Judge visibility from a rendered pane.
panelsByPaneId.values.first is arbitrary and can select a retained, offscreen pane while zoomed. That makes a visible mirror appear hidden and skips its settlement/mismatch validation. Restrict the check to renderedLayout.paneIDsInOrder.
Proposed fix
var isEffectivelyVisibleForSizing: Bool {
guard isVisibleForSizing else { return false }
- guard let window = panelsByPaneId.values.first?.hostedView.window else {
- return false
- }
- return window.isVisible
+ return renderedLayout.paneIDsInOrder.contains { paneID in
+ panelsByPaneId[paneID]?.hostedView.window?.isVisible == true
+ }
}As per path instructions, correctness-critical validation facts must use a reliable source of truth rather than an arbitrary fallback.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| var isEffectivelyVisibleForSizing: Bool { | |
| guard isVisibleForSizing else { return false } | |
| guard let window = panelsByPaneId.values.first?.hostedView.window else { | |
| return false | |
| } | |
| return window.isVisible | |
| var isEffectivelyVisibleForSizing: Bool { | |
| guard isVisibleForSizing else { return false } | |
| return renderedLayout.paneIDsInOrder.contains { paneID in | |
| panelsByPaneId[paneID]?.hostedView.window?.isVisible == true | |
| } | |
| } |
🤖 Prompt for 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.
In `@Sources/RemoteTmuxWindowMirror.swift` around lines 90 - 95, Update
isEffectivelyVisibleForSizing to determine the hosted window from a pane ID in
renderedLayout.paneIDsInOrder rather than panelsByPaneId.values.first. Only use
the rendered pane’s hostedView.window for the visibility check, preserving false
when no rendered pane or window is available.
Source: Path instructions
dd1a8cb to
6b4bfe7
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (5)
scripts/remote-tmux-fuzz-marathon.sh (3)
94-111: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winCrash still misclassified as hang —
rc==99checked before theapp_pidliveness check.A crash makes the child's
pingfail with the same rc=99 a genuine hang uses; this branch order records that ashangs(and mislabels its evidencehang, while skippingsamplecapture sinceapp_pidis empty) before ever checking whether the app process still exists. This is the exact ordering an earlier review flagged and that was reportedly fixed — the code currently shown still has the old order.🐛 Proposed fix: check app_pid before classifying rc==99 as a hang
if [ "$rc" -eq 97 ]; then fails=$((fails + 1)) echo "seed=$seed INERT — fuzzer bug, aborting marathon (fix the fuzzer first)" capture_evidence "$seed" inert break - elif [ "$rc" -eq 99 ]; then - hangs=$((hangs + 1)) - echo "seed=$seed HANG (evidence: seed-$seed-hang-*)" - capture_evidence "$seed" hang - relaunch_app - wait_app_ready elif [ -z "$(app_pid)" ]; then crashes=$((crashes + 1)) echo "seed=$seed CRASH (app gone; evidence: seed-$seed-crash-*)" capture_evidence "$seed" crash relaunch_app wait_app_ready + elif [ "$rc" -eq 99 ]; then + hangs=$((hangs + 1)) + echo "seed=$seed HANG (evidence: seed-$seed-hang-*)" + capture_evidence "$seed" hang + relaunch_app + wait_app_ready elif [ "$rc" -ne 0 ]; then🤖 Prompt for 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. In `@scripts/remote-tmux-fuzz-marathon.sh` around lines 94 - 111, Reorder the status checks in the marathon loop so the app_pid liveness check runs before the rc==99 hang branch. This ensures a failed ping with no running app increments crashes and captures crash evidence, while rc==99 is classified as a hang only when the app is still alive.
35-55: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winFixed sleeps still used as synchronization for relaunch/evidence capture.
relaunch_appwaits a flat 2s then 6s with no check that the old process actually exited or the new socket is ready;capture_evidencewaits a flat 8s and can let the caller's nextrelaunch_appkill the app beforesamplefinishes writing. As per coding guidelines, "Production non-Swift app and runtime code must not use fixed sleeps... to mask lifecycle... process... teardown, startup... races," and as per path instructions,runtime-no-hacky-sleeps.mdrequires flagging "fixed sleeps... used as synchronization" in production shell scripts.🔧 Proposed fix: poll for real completion instead of fixed waits
relaunch_app() { local pid; pid=$(app_pid) - [ -n "$pid" ] && kill -9 "$pid" 2>/dev/null - sleep 2 + if [ -n "$pid" ]; then + kill -9 "$pid" 2>/dev/null + local tries=0 + while [ -n "$(app_pid)" ] && [ "$tries" -lt 15 ]; do sleep 1; tries=$((tries + 1)); done + fi # Launch through the login shell so the app runs outside any sandbox. ssh cmux-srvA "zsh -lc 'open \"$APP\"'" >/dev/null 2>&1 - sleep 6 } capture_evidence() { local seed=$1 kind=$2 local pid; pid=$(app_pid) local out="$DIR/seed-$seed-$kind" if [ -n "$pid" ] && [ "$kind" = hang ]; then tmux send-keys -t '1:0' "sample $pid 5 -file $out-sample.txt >/dev/null 2>&1" Enter 2>/dev/null - sleep 8 + local tries=0 + while [ ! -s "$out-sample.txt" ] && [ "$tries" -lt 20 ]; do sleep 1; tries=$((tries + 1)); done fi🤖 Prompt for 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. In `@scripts/remote-tmux-fuzz-marathon.sh` around lines 35 - 55, Replace the fixed synchronization sleeps in relaunch_app and capture_evidence with polling based on real lifecycle state: wait until the old app_pid exits, then wait until the relaunched app process or socket is ready, and wait until the sample command completes or its output file is finalized before returning. Preserve the existing timeouts, logging, and evidence capture behavior while ensuring subsequent relaunch_app calls cannot interrupt an active sample.Sources: Coding guidelines, Path instructions
125-125: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAggregate exit code can wrap or collide with sentinel codes.
exit $((hangs + crashes + fails))wraps modulo 256 (a 256+ failure marathon exits 0/success) and can coincidentally land on 97/98/99, colliding with this same script's/sibling script's reserved sentinel meanings.🐛 Proposed fix: boolean exit status
-exit $((hangs + crashes + fails)) +if [ $((hangs + crashes + fails)) -gt 0 ]; then + exit 1 +fi +exit 0🤖 Prompt for 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. In `@scripts/remote-tmux-fuzz-marathon.sh` at line 125, Replace the aggregate arithmetic exit in the marathon script with a boolean failure status: return success only when hangs, crashes, and fails are all zero, otherwise return a nonzero status. Avoid encoding the count in the exit code so large totals cannot wrap to zero or collide with reserved sentinel codes.scripts/remote-tmux-live-fuzz.sh (1)
96-174: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftMirror-text oracle (
read-screenvs tmuxcapture-pane) still not implemented.The header (lines 13-14) advertises comparing per-pane
read-screenoutput against tmuxcapture-panetext for every pane surface, butcheck_iter's only per-pane check (155-173) only reads tmux's own ruler output against tmux's ownpane_width— nothing here resolves a panel ref and calls the app'sread-screenRPC, so mirror-side text corruption (the class of bug this PR targets) is still invisible to this harness. This mirrors an unresolved finding from a prior pass on this same gap.Want me to sketch the panel-ref resolution +
read-screencomparison, following thepanel_id_for_title-style JSON lookup already established inscripts/remote-tmux-layout-repro.sh?🤖 Prompt for 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. In `@scripts/remote-tmux-live-fuzz.sh` around lines 96 - 174, Add the missing mirror-text oracle to check_iter: for every tmux pane, resolve its corresponding panel reference using the existing panel_id_for_title-style JSON lookup, call the app’s read-screen RPC, and compare its text with tmux capture-pane output. Record a failure through note_fail when the outputs differ, while preserving the existing ruler-versus-pane-width check as a separate validation.cmuxTests/RemoteTmuxNativeMirrorLayoutFuzzTests.swift (1)
105-125: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winFuzz loop can still silently execute zero assertions.
Unresolved from a prior pass: the
guard claim.columns >= minimum.cols, claim.rows >= minimum.rows else { continue }can skip all four regimes for a trial (or all trials for a seed) with nothing tallying it — a regression that makes everyclaimfall short ofminimumwould pass this suite green while testing nothing.♻️ Proposed fix: track and assert regime coverage
`@Test`(arguments: seeds) func everyPaneRendersAtLeastItsAssignedSpan(seed: UInt64) throws { var rng = SplitMix64(seed: seed) + var regimesExecuted = 0 for trial in 0..<80 { ... guard claim.columns >= minimum.cols, claim.rows >= minimum.rows else { continue } + regimesExecuted += 1 ... } + `#expect`(regimesExecuted > 0, "fuzz loop never exercised a regime — assertions never ran: seed=0x\(String(seed, radix: 16))") }🤖 Prompt for 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. In `@cmuxTests/RemoteTmuxNativeMirrorLayoutFuzzTests.swift` around lines 105 - 125, Update the fuzz loop around the minimum-size guard to track whether each trial executes at least one eligible regime, and assert that coverage after the regime loop (or equivalent at the seed level). Preserve skipping individual undersized claims, but make an all-skipped trial fail instead of silently performing zero assertions; use the existing fuzz test symbols such as claim, minimum, regime, trial, and seed.
🤖 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/RemoteTmuxRectPublicationTests.swift`:
- Around line 513-523: Update the test around patchingLeafRects to use distinct
x and width values for the known panes, then assert x, y, width, and height for
both top[0] and right[0]. Keep the existing structure check and untouched-pane
assertion for right[1].
In `@Sources/TerminalController`+RemoteTmuxTestSupport.swift:
- Around line 154-161: Update the documentation for the
remote.tmux.sizing_settled predicate to state that each pane renders at least
its assigned span, or has no shortfall, rather than exactly matching it. Keep
the description of mismatches aligned with the implementation’s shortfall-only
behavior.
---
Duplicate comments:
In `@cmuxTests/RemoteTmuxNativeMirrorLayoutFuzzTests.swift`:
- Around line 105-125: Update the fuzz loop around the minimum-size guard to
track whether each trial executes at least one eligible regime, and assert that
coverage after the regime loop (or equivalent at the seed level). Preserve
skipping individual undersized claims, but make an all-skipped trial fail
instead of silently performing zero assertions; use the existing fuzz test
symbols such as claim, minimum, regime, trial, and seed.
In `@scripts/remote-tmux-fuzz-marathon.sh`:
- Around line 94-111: Reorder the status checks in the marathon loop so the
app_pid liveness check runs before the rc==99 hang branch. This ensures a failed
ping with no running app increments crashes and captures crash evidence, while
rc==99 is classified as a hang only when the app is still alive.
- Around line 35-55: Replace the fixed synchronization sleeps in relaunch_app
and capture_evidence with polling based on real lifecycle state: wait until the
old app_pid exits, then wait until the relaunched app process or socket is
ready, and wait until the sample command completes or its output file is
finalized before returning. Preserve the existing timeouts, logging, and
evidence capture behavior while ensuring subsequent relaunch_app calls cannot
interrupt an active sample.
- Line 125: Replace the aggregate arithmetic exit in the marathon script with a
boolean failure status: return success only when hangs, crashes, and fails are
all zero, otherwise return a nonzero status. Avoid encoding the count in the
exit code so large totals cannot wrap to zero or collide with reserved sentinel
codes.
In `@scripts/remote-tmux-live-fuzz.sh`:
- Around line 96-174: Add the missing mirror-text oracle to check_iter: for
every tmux pane, resolve its corresponding panel reference using the existing
panel_id_for_title-style JSON lookup, call the app’s read-screen RPC, and
compare its text with tmux capture-pane output. Record a failure through
note_fail when the outputs differ, while preserving the existing
ruler-versus-pane-width check as a separate validation.
🪄 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: 9f10962b-2758-4809-b865-1ce4010735ea
📒 Files selected for processing (28)
Packages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Wire/ControlCommandExecutionPolicy.swiftSources/AppDelegate.swiftSources/GhosttyTerminalView.swiftSources/RemoteTmuxLayoutNode.swiftSources/RemoteTmuxMirrorFrames.swiftSources/RemoteTmuxMirrorGeometry.swiftSources/RemoteTmuxNativeLayoutMetrics.swiftSources/RemoteTmuxNativeSplitLayout.swiftSources/RemoteTmuxNativeSplitTree.swiftSources/RemoteTmuxWindowMirror+Bonsplit.swiftSources/RemoteTmuxWindowMirror+BonsplitLayout.swiftSources/RemoteTmuxWindowMirror+DividerSizing.swiftSources/RemoteTmuxWindowMirror.swiftSources/RemoteTmuxWindowMirrorSplitView.swiftSources/TerminalController+DebugMethodNames.swiftSources/TerminalController+RemoteTmuxTestSupport.swiftSources/TerminalController.swiftcmux.xcodeproj/project.pbxprojcmuxTests/AppDelegateDisplayConfigRestoreTests.swiftcmuxTests/RemoteTmuxMirrorFeedForwardTests.swiftcmuxTests/RemoteTmuxMirrorGeometryTests.swiftcmuxTests/RemoteTmuxMirrorLayoutMathTests.swiftcmuxTests/RemoteTmuxNativeMirrorLayoutFuzzTests.swiftcmuxTests/RemoteTmuxRectPublicationTests.swiftscripts/remote-tmux-fuzz-marathon.shscripts/remote-tmux-layout-repro.shscripts/remote-tmux-live-fuzz.shvendor/bonsplit
💤 Files with no reviewable changes (3)
- cmuxTests/RemoteTmuxMirrorGeometryTests.swift
- Sources/RemoteTmuxMirrorFrames.swift
- cmux.xcodeproj/project.pbxproj
| let patched = tree.patchingLeafRects([ | ||
| 1: (x: 0, y: 1, width: 50, height: 19), | ||
| 2: (x: 51, y: 1, width: 49, height: 8), | ||
| ]) | ||
| guard case let .horizontal(top) = patched.content, | ||
| case let .vertical(right) = top[1].content else { | ||
| Issue.record("structure must be preserved"); return | ||
| } | ||
| #expect(top[0].y == 1 && top[0].height == 19) | ||
| #expect(right[0].y == 1 && right[0].height == 8) | ||
| #expect(right[1].y == 10 && right[1].height == 10) // unknown: untouched |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Assert the complete rectangle for known leaves.
The supplied rectangles only change y/height, and the assertions omit x/width. A regression that stops applying fetched horizontal geometry would still pass. Use distinct x/width values and assert all four fields for panes 1 and 2.
🤖 Prompt for 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.
In `@cmuxTests/RemoteTmuxRectPublicationTests.swift` around lines 513 - 523,
Update the test around patchingLeafRects to use distinct x and width values for
the known panes, then assert x, y, width, and height for both top[0] and
right[0]. Keep the existing structure check and untouched-pane assertion for
right[1].
| /// `remote.tmux.sizing_settled` (DEBUG only) — answers "has every | ||
| /// mirrored window finished settling, and does every pane render exactly | ||
| /// its assigned span?" in one call. Harnesses poll this instead of | ||
| /// guessing with timers: a timer too short misreads transitions as bugs, | ||
| /// too long crawls. `settled` means each mirror's tmux layout matches the | ||
| /// size it claimed; `mismatches` lists panes whose last rendered grid | ||
| /// differs from their assigned span. A mismatch while `settled` is true | ||
| /// is a real rendering bug, no ambiguity. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Align the settlement comment with the actual predicate.
The comment says every pane must render “exactly” its assigned span, but the implementation only reports shortfalls and intentionally accepts surplus space. Change this to “at least its assigned span” or “has no shortfall” to avoid misleading harness and diagnostic users.
🤖 Prompt for 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.
In `@Sources/TerminalController`+RemoteTmuxTestSupport.swift around lines 154 -
161, Update the documentation for the remote.tmux.sizing_settled predicate to
state that each pane renders at least its assigned span, or has no shortfall,
rather than exactly matching it. Keep the description of mismatches aligned with
the implementation’s shortfall-only behavior.
6b4bfe7 to
b1ed20a
Compare
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 `@scripts/remote-tmux-layout-repro.sh`:
- Line 205: Update the identifier extraction in the item parser to also fall
back to item.get("pane_ref") after the existing id, handle, and ref fields.
Preserve the current precedence while ensuring entries containing only pane_ref
resolve successfully instead of being reported missing.
🪄 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: 0b0ff3d8-dd23-4bae-82b7-9f9ae8e7cb5d
📒 Files selected for processing (28)
Packages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Wire/ControlCommandExecutionPolicy.swiftSources/AppDelegate.swiftSources/GhosttyTerminalView.swiftSources/RemoteTmuxLayoutNode.swiftSources/RemoteTmuxMirrorFrames.swiftSources/RemoteTmuxMirrorGeometry.swiftSources/RemoteTmuxNativeLayoutMetrics.swiftSources/RemoteTmuxNativeSplitLayout.swiftSources/RemoteTmuxNativeSplitTree.swiftSources/RemoteTmuxWindowMirror+Bonsplit.swiftSources/RemoteTmuxWindowMirror+BonsplitLayout.swiftSources/RemoteTmuxWindowMirror+DividerSizing.swiftSources/RemoteTmuxWindowMirror.swiftSources/RemoteTmuxWindowMirrorSplitView.swiftSources/TerminalController+DebugMethodNames.swiftSources/TerminalController+RemoteTmuxTestSupport.swiftSources/TerminalController.swiftcmux.xcodeproj/project.pbxprojcmuxTests/AppDelegateDisplayConfigRestoreTests.swiftcmuxTests/RemoteTmuxMirrorFeedForwardTests.swiftcmuxTests/RemoteTmuxMirrorGeometryTests.swiftcmuxTests/RemoteTmuxMirrorLayoutMathTests.swiftcmuxTests/RemoteTmuxNativeMirrorLayoutFuzzTests.swiftcmuxTests/RemoteTmuxRectPublicationTests.swiftscripts/remote-tmux-fuzz-marathon.shscripts/remote-tmux-layout-repro.shscripts/remote-tmux-live-fuzz.shvendor/bonsplit
💤 Files with no reviewable changes (3)
- cmuxTests/RemoteTmuxMirrorGeometryTests.swift
- Sources/RemoteTmuxMirrorFrames.swift
- cmux.xcodeproj/project.pbxproj
| sys.exit(1) | ||
| for item in data.get("surfaces", []): | ||
| if item.get("title") == title: | ||
| value = item.get("id") or item.get("handle") or item.get("ref") |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Also accept pane_ref as an identifier field.
The parser now consumes ref, but per the retained learning the CLI emits identifiers under ref/pane_ref. Surfaces that carry only pane_ref would still resolve to nothing and be reported as missing.
Proposed fix
- value = item.get("id") or item.get("handle") or item.get("ref")
+ value = item.get("id") or item.get("handle") or item.get("ref") or item.get("pane_ref")Based on learnings: "treat panel identifiers as coming from the ref/pane_ref fields ... accept ref (and pane_ref if applicable) as the canonical identifier field."
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| value = item.get("id") or item.get("handle") or item.get("ref") | |
| value = item.get("id") or item.get("handle") or item.get("ref") or item.get("pane_ref") |
🤖 Prompt for 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.
In `@scripts/remote-tmux-layout-repro.sh` at line 205, Update the identifier
extraction in the item parser to also fall back to item.get("pane_ref") after
the existing id, handle, and ref fields. Preserve the current precedence while
ensuring entries containing only pane_ref resolve successfully instead of being
reported missing.
Source: Learnings
Panes in a mirrored tmux window could render narrower than the width tmux assigned them. A pane even one column short wraps every full-width line, so multi-column layouts looked shredded; a window whose claimed size exactly filled the container garbled every pane at once. The terminal surface floors its size to whole cells, so a pane's pixels must never come out below its assigned span — but pane sizes were made by translating exact point targets into normalized divider fractions, and the fraction machinery cannot carry that precision: its drift deadbands eat sub-1% changes (two or three columns at terminal sizes), rounding compounds down the binary split chain, and the claim reserved no margin, so every pane sat exactly on the boundary where one lost point costs one column. Stop translating. Bonsplit now accepts an imposed first-child extent in points per split and applies it verbatim: clamped only by pane minimums, the equivalent fraction mirrored back for ratio readers, cleared by a user drag, and convergence memo-based so a target AppKit refuses cannot spin the main thread. The plan computes each split's extent directly — assigned cells times cell size plus measured chrome, including tmux's own pane title rows when pane-border-status is active — scaled evenly when the container genuinely cannot fit, quantized upward to the device-pixel grid with an axis-tagged running remainder so error never accumulates with depth or leaks across axes. The claim leaves one device pixel per axis unclaimed: exactly the quantum round-up can accumulate to, not a tuning constant. Container resizes re-impose the plan. DEBUG observability so harnesses ask instead of guessing with timers: a remote.grid.mismatch log line whenever a pane settles on a grid different from its assignment, and a remote.tmux.sizing_settled socket verb reporting per visible window whether sizing settled and which panes fall short. The native layout fuzz from the previous commit goes green: random trees, metrics, and containers — including exact-fit and one-extra-cell regimes — every pane renders at least its assigned span, degradation is even, and the other axis stays exact.
RemoteTmuxMirrorGeometry.clientCells/frames and the mirror's framesForRender had no callers outside their own tests: the native chrome rewrite replaced the live pipeline with RemoteTmuxNativeLayoutMetrics for the claim and bonsplit divider fractions for the render, but left the old walk in the tree with doc comments still describing it as the sizing authority. That cost us a real debugging detour — the comments point at the wrong code when the mirror misrenders. Keep the struct itself: it carries the measured render constants (cell/padding/scale) the native metrics are built from, plus the claim floors. Drop the dead functions, their frames value type, their test suite, and the one feed-forward test that exercised the dead entry point, and rewrite the mirror's header to describe the pipeline that actually runs. The one still-live function the deleted suite covered (patchingLeafRects) keeps its regression test.
A window larger than its own display is never a state a user created — interactive resizing and zoom are both display-bounded — yet exact-frame preservation restored such frames verbatim whenever the saved display still matched. A layout feedback bug (fixed separately in this branch) once persisted a 13,000-point-wide window, and every launch after that restored the giant frame and re-poisoned everything derived from window geometry. Restore now clamps a saved frame's size to its display's visible frame; position handling is unchanged.
b1ed20a to
19cf069
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 19cf069. Configure here.
| metrics: metrics, | ||
| parentSize: containerSizePt | ||
| ) | ||
| applyDividerPositions(plan: plan, treeNode: treeNode) |
There was a problem hiding this comment.
Plan idempotence blocks retry
Medium Severity
refreshDividerPositions stores lastPlanInputs before applyDividerPositions finishes. If the bonsplit snapshot and computed plan disagree at any split, apply returns early (DEBUG log only) while inputs are already cached. Identical layout, container, and metrics then skip every later imposition pass, leaving divider extents stale until something else changes those inputs.
Reviewed by Cursor Bugbot for commit 19cf069. Configure here.
A sync pass lays out hosted split views and writes the host frame, and the notifications those emit can be delivered after the pass ends, so no in-pass reentrancy flag catches them all. The echo then re-runs the sync forever on identical geometry, pinning the main thread. Each pass now fingerprints everything it reads or writes — window size, container, reference, host, and hosted frames — and an incoming pass with an identical fingerprint is a no-op, so echoes die in one cheap comparison while any real change still syncs fully.
19cf069 to
c57be5d
Compare
|
Closing this in favor of a rework. The sizing math held up — rounding absolute split boundaries with a point of slack per pane kept every pane within its assigned span across thousands of randomized layouts in the seeded fuzz — but making the view layers honor that math reliably took a growing pile of anti-feedback guards: an imposition epoch, a distance check, retry budgets, an input gate, geometry fingerprints. Each one suppressed another loop between SwiftUI, AppKit, and the portal layer, and needing that many of them is the design telling us the interaction model is wrong. The replacement moves sizing into a single transaction per input change: one owner that plans, applies, and verifies exactly once, where geometry events may only update inputs and can never re-enter layout. That makes the feedback loops structurally impossible instead of individually suppressed. A new PR will follow with that shape, and the repro harness, the live fuzz gate, and the window/restore hardening from this branch will carry over. |
1 participant
Superseded by #7938, which keeps this branch's harness, fuzz gate, and hardening commits and replaces the sizing machinery with a single-transaction design.
Panes in a mirrored tmux window could render narrower than the width tmux assigned them. A pane that is even one column short wraps every full-width line, so multi-column layouts looked shredded. The worst case was a window whose claimed size exactly filled the container — every pane garbled at once. To reproduce, mirror a six-column window whose panes each run a full-width ruler (
scripts/remote-tmux-layout-repro.shbuilds one) and watch the deepest panes wrap their last characters.Depends on manaflow-ai/bonsplit#176 (the submodule bump in this PR).
Before (six uneven columns, every pane wrapping):
(screenshot: wrapping-before)
After (same layout, every pane exact at every depth):
(screenshot: wrapping-after)
Root cause
The terminal surface floors its size to whole cells, so a pane's pixel size must never come out below its assigned span. But pane sizes were produced by translating exact point targets into normalized divider fractions, and the fraction machinery cannot carry that precision: its layout code damps sub-1% changes to stop drift, and 1% of a large split is two or three terminal columns; the leftover rounding compounded down the binary split chain, worst for the deepest panes; and the claim reserved no margin, so every pane sat exactly on the cell boundary where losing one point costs one column.
Fix
Stop translating. Bonsplit (our submodule) now accepts an imposed first-child extent in points per split and applies it verbatim, clamped only by pane minimums, with the equivalent fraction mirrored back for anything that reads ratios. A user divider drag clears the imposition — the gesture takes ownership, the drag syncs cells to tmux as before, and the new assignment is re-imposed at settle.
On the cmux side the whole computation is one pure function,
RemoteTmuxNativeSplitLayout.plan: each split's first child gets exactly its assigned cells times the cell size plus its measured chrome, scaled down evenly when the container genuinely cannot fit (mid-resize, a co-attached client, tmux briefly ahead of the claim), and quantized by rounding each split boundary's absolute position to the nearest whole point. Rounding boundaries rather than per-split extents is what makes the error depth-independent: every boundary lands within half a point of its exact position no matter how deep the tree nests, so every pane lands within one point of its ideal. Whole points matter too: a fractional-point extent leaves nested view edges on half pixels, where AppKit's backing alignment can shave a device pixel — one column — off a pane. The claim reserves one point per pane per axis, which is the quantization granularity rather than a tuning constant: a pane's two boundaries can each be off by half a point, and a point below an exact cell boundary would cost a whole column, so one point covers the worst case exactly. A container resize re-imposes the plan.Hidden tabs never impose and keep their last visible geometry: their trees live in portal hosts that no window clamps, where an absolute divider position grows the container instead of dividing it, and the grown bounds would feed the next plan. Bonsplit's split and pane views also stop answering size proposals with content-derived ideals — a split tree is space-filling and has no meaningful size of its own — so layout can never inflate whatever sizes itself from content, including the window. Showing a tab re-imposes against its real, window-clamped bounds.
Tests
The commits are ordered so CI shows the fuzz failing before the fix: the first commit adds the repro harness, the second extracts the plan and adds a seeded fuzz that fails on the fraction-based code (red), the third lands the fix (green). The last commit hardens session restore: a saved window frame larger than its own display can only be programmatic growth (the feedback bug above once persisted a 13,000-point-wide window, and every launch restored it), so restore now clamps a saved frame's size to its display.
Three layers of coverage. The unit fuzz drives random trees, metrics, and four container regimes — loose, exact-fit, and one-extra-cell per axis — through the real claim and the real plan, deriving each pane's grid in the renderer's own integer arithmetic; every pane must render at least its assigned span, and an over-assigned axis must degrade evenly without touching the other. In the app, a debug probe logs whenever any pane settles on a grid different from its assignment, and a debug socket call reports per window whether sizing has finished settling and which panes mismatch — so harnesses ask instead of guessing with timers. On top of those,
scripts/remote-tmux-fuzz-host.shstands up the whole "remote" side locally — a loopback-only sshd with generated keys whose logins land in an isolated tmux server, capped at one session like hosts that limit concurrent ssh — so the live gate runs on any machine with no real network.scripts/remote-tmux-live-fuzz.shthen drives the running app mirroring that server through seeded random churn (pane and window churn, zoom, pane title rows, app-window resizes, output floods, reconnects, burst sequences), failing with the seed and iteration if anything mismatches once settled, never settles, or stops responding; a marathon wrapper runs many seeds unattended and captures a process sample if the app ever hangs.The last commit removes the superseded pure-frame layout functions: they had no callers outside their own tests since the native chrome rewrite, but their doc comments still described them as the sizing authority, which misdirects debugging. The one still-live function the deleted suite covered keeps its regression test.
Verified live against a real tmux server over ssh: uneven six-column rows, five-deep alternating nests, and multi-hour randomized runs, every pane rendering exactly its assigned span at settle.
Verification in progress
A 60-seed randomized live run (the marathon wrapper above) is going right now against this exact build — results will be posted here when it completes. Seeds 1–3 are clean so far, and the specific sequences that exposed every bug fixed in this PR replay clean. An adversarial model review of the final diff is also running; anything it surfaces will be addressed on this PR.
Summary by CodeRabbit
New Features
remote.tmux.sizing_settled) to report per-pane render/mismatch status.Bug Fixes
Testing
Note
Medium Risk
Large change to remote tmux mirror sizing, window geometry, and portal sync on critical UI paths, mitigated by extensive fuzz and live harnesses but still touches layout loops that previously caused runaway sizing.
Overview
Mirrored tmux panes were ending up one or more columns narrow than tmux’s assignment because divider positions were driven by normalized fractions that lost precision (drift deadbands, rounding down deep splits, no slack on the claim). This PR replaces that path with
RemoteTmuxNativeSplitLayout.plan: whole-point rail allocation from measured chrome (including pane title rows and per-pane quantization slack), applied via BonsplitsetImposedFirstExtentinstead of fraction-only positioning. Hidden tabs skip imposition and ignore poisoned portal geometry; becoming visible re-claims and re-imposes.The old device-pixel frame imposition (
RemoteTmuxMirrorFrames/framesForRender) is removed; sizing authority lives in the native split plan plus updatedRemoteTmuxNativeLayoutMetrics.Feedback-loop hardening (observed runaway window/container growth):
CmuxMainWindow.setFrameand session restore clamp frames to the display;GhosttyTerminalView.sizeThatFitsaccepts the proposed size; mirror container recording clamps to the window and ignores limbo geometry; portal geometry sync dedupes identical signatures and ignores re-entrant notifications; divider refresh idempotence and drag baselines read actual Bonsplit fractions after impose.DEBUG:
remote.tmux.sizing_settledfor harness polling; per-pane rendered grid tracking and mismatch logging;flushPendingManualSizeReportIfAttachedon portal attach.Tests & scripts: seeded native mirror layout fuzz, updated feed-forward expectations, live fuzz / layout repro / fuzz host / marathon shell harnesses.
Reviewed by Cursor Bugbot for commit 19cf069. Bugbot is set up for automated code reviews on this repo. Configure here.