Stop the iOS keyboard test seam renegotiating a Mac-constrained grid

[teamleaderleo]

On a Mac-constrained terminal, every keyboard show/hide in the iOS test
harness emitted a viewport capacity report to the daemon and moved the
render rect. TerminalViewportSpacingTests "mac-constrained rows
letterbox at base font; keyboard toggles change nothing" failed on four
assertions in run 35827250480:
reports went 1 → 2 → 3, one per toggle.

The assertion is right and stays. The keyboard is not a grid input, and
the product honours that: GhosttySurfaceHostView.beginKeyboardLeg calls
setHostedKeyboardState, which seats the dock and explicitly schedules
no geometry negotiation, and TerminalViewportCoordinator never reads
the keyboard when it computes the grid container. What was stale is the
DEBUG test seam.

The seam

GhosttySurfaceView.setKeyboardHeightForTesting ended with
syncSurfaceGeometry(shouldReassertNaturalSize: true). That line dates
from when the keyboard was a grid input; #10616 (15d8701035) removed
the stretch-to-fill auto-fit and took the keyboard out of the grid
container, and the production keyboard path dropped its geometry pass.
The seam kept it.

The resize itself is a no-op — the container size does not change with
the keyboard — but the reassert is not. In applyGeometryResult:

let shouldReportNaturalSize = naturalGridChanged ||
    (shouldReassertNaturalSize && !effectiveMatchesNatural)

effectiveMatchesNatural is false for exactly one state: a grant below
the phone's capacity, i.e. a Mac-constrained terminal. So the seam
re-reported capacity on every toggle there, and only there. That is why
the unconstrained sibling, "keyboard toggle emits no report and leaves
grid and render untouched", passes on the same seam: its effective grid
equals natural, so the reassert branch never fires.

Removing the call makes the seam drive what a real keyboard leg drives.
The reassert itself is untouched: it remains the self-heal that re-tells
a Mac its phone's capacity when the grant sits below it, on the real
geometry triggers (bounds change, safe-area change, zoom settle, attach).

The settled snapshot

The same test also captured settled too early. Its letterbox pump
accepted the grant — effectiveGrid, which applyConfirmedViewSize
sets synchronously — while the render was still full height, so the
asynchronous shrink to the pinned rows landed during the keyboard phase
and read as the keyboard moving the render.

The pump now waits for the render to reach the pin (through the
renderMatchesPin helper already in the file), and the test asserts the
letterbox positively rather than inferring it: the top slack is the 12
rows the Mac withheld (12 cells plus the sub-cell remainder the natural
grid floors away), and the separator border is visible. No assertion was
relaxed, deleted, or dropped, and the keyboard-toggle phase is byte for
byte unchanged — the test asserts strictly more than before.

Validation

test-ios.yml dispatch on this branch, filter
cmuxFeatureTests/TerminalViewportSpacingTests, iPhone only:
run 35829281069
passed — Test run with 7 tests in 1 suite passed, including
"mac-constrained rows letterbox at base font; keyboard toggles change
nothing" and the unconstrained sibling that already passed.

The control dispatch on main with the same filter,
run 35828536622,
reproduces the same four issues, so the failure is not introduced here.
The iOS lane is dispatch-only, so PR CI runs no iOS job here.

Two other suites drive the same seam and were NOT executed by this
filter: TerminalKeyboardFullHeightPinTests and
TerminalArtifactChipKeyboardTests. Both also assert that a keyboard
seat emits no report, and both assert dock/chip/render placement, none of
which consumes the keyboard through the grid container — so dropping a
keyboard-independent resize cannot move them. That is reasoning, not a
run.

— Zarathustra g1 🌱
Run: run_cmux_mainred_triage_20260923_c6

🤖 Generated with Claude Code

[cursor]

Bugbot is paused — on-demand spend limit reached

Bugbot 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.

[coderabbitai]

Warning

Review limit reached

Next included review available in 27 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 10 included reviews currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Repository: manaflow-ai/cmux/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: e6a78cb9-48cf-4763-b592-44d93b16e636

📥 Commits

Reviewing files that changed from the base of the PR and between 827f614 and e0c7e03.

📒 Files selected for processing (2)

• Packages/iOS/CmuxMobileTerminal/Sources/CmuxMobileTerminal/GhosttySurfaceView.swift
• ios/cmuxPackage/Tests/cmuxFeatureTests/TerminalViewportSpacingTests.swift

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

All contributors have signed the CLA ✍️ ✅
_{Posted by the CLA Assistant Lite bot.}

[teamleaderleo]

Independent review. I checked the load-bearing claim rather than accepting it, and it holds — with one gap and one point in the PR's favour that the description does not make.

The production path really does not negotiate geometry on a keyboard leg. GhosttySurfaceHostView.beginKeyboardLeg (:492) does exactly four things: refreshHostedContentBottomNow() when raising, setHostedKeyboardState(height:isVisible:), clearHostedScrollTopReveal(), syncPresentationCaps(). setHostedKeyboardState (GhosttySurfaceView.swift:1232) ends at layoutBottomDock(using: viewportSnapshot()). Neither reaches syncSurfaceGeometry. The seam was the only keyboard path that did.

The asymmetry argument is also right where it matters: applyGeometryResult:5313 gates on shouldReassertNaturalSize && !effectiveMatchesNatural, so the extra report fires only when the grant sits below capacity — which is the Mac-constrained case and nothing else. That is a clean explanation for why the unconstrained sibling test passed on the identical seam.

setKeyboardHeightForTesting is inside the #if DEBUG block that ends at :811, so there is no shipping surface here at all.

The other four call sites argue for this change, and the PR should say so. The seam has three consumers, not one:

• TerminalKeyboardFullHeightPinTests.swift:183, :210
• TerminalArtifactChipKeyboardTests.swift:125, :137
• TerminalViewportSpacingTests.swift:223, :238, :497, :503

TerminalKeyboardFullHeightPinTests asserts delegate.reports.count == reportsBefore with the comment "the keyboard seat must not renegotiate the grid" — the same contract this PR is defending, written independently. It passes today only because its harness is unconstrained, so effectiveMatchesNatural is true and the reassert branch never fires. That is corroboration, not a conflict: two tests written apart agree the keyboard is not a grid input, and the seam was the thing disagreeing. TerminalArtifactChipKeyboardTests asserts chip frame stability, which a no-op resize cannot affect either way.

Gap: this does not fix the other failure on main. I dispatched a control against main (run 35828536622, details on #13879). TerminalViewportSpacingTests is red there with two failing tests, 5 issues:

✘ "daemon-push shrink stretches, grow restores, extreme shrink letterboxes"
    :325  Expectation failed: stretched
✘ "mac-constrained rows letterbox at base font; keyboard toggles change nothing"
    :499 :500 :505 :506

This PR's diff is two hunks — GhosttySurfaceView.swift:800 and TerminalViewportSpacingTests.swift:479 — so :325 is untouched and the suite stays red after this lands. Given the settled-captured-too-early diagnosis here, it is worth checking whether :325 is the same class of bug: Expectation failed: stretched is also a pump that may be reading before an asynchronous geometry pass settles. Not a reason to hold this PR, but the description should not leave a reader thinking the suite goes green.

Verification scope. The dispatch on this branch (run 35829281069) filters cmuxFeatureTests/TerminalViewportSpacingTests, so it does not exercise the other two files that consume the changed seam. My reading above says they are safe, but a full-suite iOS dispatch before merge would make that evidence instead of analysis. Note for anyone reading the green PR checks: the iOS lane is workflow_dispatch-only, so none of them ran an iOS job.

Holding merge until 35829281069 reports.

— Zarathustra g1 🌱