Skip to content

Say why the portal fixture's scrollback wait failed - #14898

Merged
teamleaderleo merged 1 commit into
mainfrom
test/portal-scrollback-wait-diagnostics
Sep 27, 2026
Merged

teamleaderleo merged 1 commit into
mainfrom
test/portal-scrollback-wait-diagnostics

Conversation

@teamleaderleo

@teamleaderleo teamleaderleo commented Sep 27, 2026 •

Copy link
Copy Markdown
Collaborator

TerminalWindowPortalCommittedGeometryTests/legacyScrollerCommitsClipWidthWithoutPaneResize() fails on main now and then. When it does, the only message is "Expected shell output to create real scrollback". That doesn't say whether the command never produced output, or produced it and the scrollbar packet never reached the view. Now the failure also records the view's scrollbar, the runtime's own scrollbar (ghostty_surface_scrollbar), the TTY, and the last 600 characters of visible text. The next failure will show which of the two it was.

This PR changes only the failure message. It doesn't fix the flake, because I couldn't find its cause:

  • Not a regression. The test and the code it exercises haven't changed since 2026-09-16. It failed at 26a1e883, a docs-only commit (36239073356, shard 5, cmuxs-mac-mini-5), and passed at the commit before it (4061427e, cmux11s). It failed again at 022cc2dc (36271922019, shard 5, cmux7s) and passed at the next commit, c5e72222. That's 2 of the 16 full-suite main runs on 2026-09-26.
  • Doesn't reproduce on demand. It passed 100 of 100 repeats in one process on the owned lane (36290060597, cmux11s). It passed 20 of 20 in each of two full-suite runs, 36287042630 (cmux13s) and 36288708517 (cmux12s). Each repeat got scrollback 34–86 ms after sending the command.
  • What the 022cc2d failure shows. The system log from that failure's xcresult shows login(1) and zsh starting normally, 30 ms after spawn. zsh stayed alive until teardown 2 s later, and Ghostty logged no input or IO errors. The unified log can't show whether /bin/sh ran.

Validation: this head's tree passed the focused suite (11 tests, including this one) on the owned lane in 36291078295 (cmux14). That run was at 8913e4ca, which has the same tree with a different commit message.

🤖 Generated with Claude Code


Summary by cubic

Adds diagnostics to the portal scrollback test's failure message so the next flake shows whether the command never produced output or its scrollbar packet never reached the view. The failure now records the view's and runtime's scrollbars, the TTY, and the last 600 characters of visible text. The change only affects the failure message; the cause of the flake wasn't found—the test and the code it exercises haven't changed since 2026-09-16, and it doesn't reproduce on demand (100/100 repeats passed).

Written for commit 8424612. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

  • Tests
    • Improved diagnostic details when terminal scrollback checks fail, including scrollbar state, terminal information, and recent visible text.

legacyScrollerCommitsClipWidthWithoutPaneResize() fails on main now and
then (2 of the 16 full-suite main runs on 2026-09-26) with only
"Expected shell output to create real scrollback". That can't tell a
command that never produced output from output whose scrollbar packet
never reached the view. The failure now records the view's scrollbar,
the runtime's own scrollbar, the TTY and the visible text.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

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

@coderabbitai

coderabbitai Bot commented Sep 27, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

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

Review profile: ASSERTIVE

Plan: Advanced

Run ID: f24c6a58-4a63-4f37-87f6-8c8f55e70794

📥 Commits

Reviewing files that changed from the base of the PR and between a3baec3 and 8424612.

📒 Files selected for processing (1)
  • cmuxTests/TerminalPortalGeometryFixture.swift

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 2 remain after this review.


📝 Walkthrough

Walkthrough

The scrollback test assertion now reports view and runtime scrollbar values, the controlling TTY, and up to 600 characters of visible text when it fails.

Changes

Scrollback diagnostics

Layer / File(s) Summary
Add scrollback failure details
cmuxTests/TerminalPortalGeometryFixture.swift
requireScrollback now adds scrollbar, TTY, and visible-text diagnostics to its failure message.

Priority: ⬇️ Low

Estimated code review effort: 1 (Trivial) | ~5 minutes

Change: Other

Suggested reviewers: austinywang

Merge Risk: ⚪ Minimal · up to 84246

This change adds diagnostic details to an existing scrollback test failure without a supported impact on passing tests or terminal state.

🚥 Pre-merge checks | ✅ 25
✅ Passed checks (25 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Cmux Cloud Persistent Session And Early Input ✅ Passed PASS. The PR changes only the failure message in cmuxTests/TerminalPortalGeometryFixture.swift. It adds read-only diagnostics for the view scrollbar, runtime scrollbar, TTY, and visible text after t…
Cmux Swift Actor Isolation ✅ Passed PASS. The authoritative diff changes only cmuxTests/TerminalPortalGeometryFixture.swift, a test fixture. It updates the test failure message and adds a private diagnostic helper. The custom check ex…
Cmux Swift Blocking Runtime ✅ Passed PASS. The only changed file is cmuxTests/TerminalPortalGeometryFixture.swift, which belongs to the cmuxTests unit-test target. The diff adds diagnostic reads and changes the failure message. It do…
Cmux Browser Automation Off-Main ✅ Passed PASS. The PR changes only cmuxTests/TerminalPortalGeometryFixture.swift. It adds scrollback diagnostics to a test failure message. The diff adds no browser.* socket command, WebKit/AppKit wait, wo…
Cmux Expensive Synchronous Load ✅ Passed PASS. The PR changes only cmuxTests/TerminalPortalGeometryFixture.swift, a test fixture. It adds diagnostic formatting to requireScrollback() after the test wait; it does not add or move an agent-…
Cmux Cache Substitution Correctness ✅ Passed PASS. The PR changes only cmuxTests/TerminalPortalGeometryFixture.swift, not production Swift, TypeScript, or JavaScript. The diff adds failure diagnostics and does not replace a fresh authoritative…
Cmux No Hacky Sleeps ✅ Passed PASS: The review-scoped diff changes only cmuxTests/TerminalPortalGeometryFixture.swift, which is Swift test code. It adds diagnostic formatting and a helper; it does not change TypeScript, JavaScri…
Cmux Algorithmic Complexity ✅ Passed PASS. The only changed file is cmuxTests/TerminalPortalGeometryFixture.swift. The new scrollbackDiagnostics() helper runs only when the test wait fails and adds bounded suffix(600) text to a dia…
Cmux Swift Concurrency ✅ Passed The pull request changes only cmuxTests/TerminalPortalGeometryFixture.swift. The added scrollbackDiagnostics() helper performs synchronous diagnostic reads and string formatting. The diff adds no …
Cmux Swift @Concurrent ✅ Passed PASS. The diff adds only the synchronous @MainActor-isolated scrollbackDiagnostics() helper and changes the diagnostic message in existing requireScrollback(). It adds no nonisolated async fun…
Cmux Swift Package Boundaries ✅ Passed PASS. The PR changes only cmuxTests/TerminalPortalGeometryFixture.swift. The added scrollbackDiagnostics() helper supports a test failure message and remains inside test fixture code. The diff add…
Cmux Swiftpm Lockfiles ✅ Passed The pull request changes only cmuxTests/TerminalPortalGeometryFixture.swift. The diff contains no Package.swift, Package.resolved, .gitignore, workflow, dependency, or cmux.xcodeproj package…
Cmux Swift Logging ✅ Passed PASS: The only changed file is cmuxTests/TerminalPortalGeometryFixture.swift, a test fixture. The diff adds diagnostic text to a test failure comment and does not add print, debugPrint, dump, …
Cmux User-Facing Error Privacy ✅ Passed PASS. The diff changes only cmuxTests/TerminalPortalGeometryFixture.swift, which is compiled into the cmuxTests unit-test target. The added text is a test failure diagnostic used by `legacyScrolle…
Cmux Full Internationalization ✅ Passed PASS. The authoritative diff changes only cmuxTests/TerminalPortalGeometryFixture.swift, a test fixture. The added strings are failure diagnostics for a test assertion, not production user-facing te…
Cmux Swiftui State Layout ✅ Passed PASS. The PR changes only the AppKit-based test fixture TerminalPortalGeometryFixture. It adds diagnostic string construction in scrollbackDiagnostics() and updates the failure message. The diff a…
Cmux Architecture Rethink ✅ Passed PASS: The PR changes only cmuxTests/TerminalPortalGeometryFixture.swift. It replaces a generic test failure message with scrollbackDiagnostics(), which reads existing view state, authoritative run…
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed PASS. The PR changes only cmuxTests/TerminalPortalGeometryFixture.swift, adding failure diagnostics to requireScrollback. The NSWindow in this file belongs to TerminalPortalGeometryFixture, a …
Cmux Source Artifacts ✅ Passed The PR changes only cmuxTests/TerminalPortalGeometryFixture.swift, a hand-written test source file. The diff adds diagnostic code to a test failure message and adds no logs, screenshots, recordings,…
Cmux No Test Or Debug Seam In Production Source ✅ Passed The pull request changes only cmuxTests/TerminalPortalGeometryFixture.swift. It adds a private scrollbackDiagnostics() helper and updates a test failure message. No changed Swift file is under a p…
Title check ✅ Passed The title clearly identifies the main change: adding information that explains why the portal fixture's scrollback wait failed.
Description check ✅ Passed The description includes a concrete problem statement, resulting behavior, investigation context, and detailed testing results. It omits the Demo Video and Checklist sections, but these are not critic…
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

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

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

@teamleaderleo
teamleaderleo merged commit f10c5ce into main Sep 27, 2026
54 of 55 checks passed
@teamleaderleo
teamleaderleo deleted the test/portal-scrollback-wait-diagnostics branch September 27, 2026 03:46
@github-actions

Copy link
Copy Markdown
Contributor

Merge receipt for 84246129cc: every check was green at merge (16 verified; 14 skipped by policy). Full suite runs on main after merge.

rustybret pushed a commit to rustybret/bmux that referenced this pull request Sep 27, 2026
f77bfdd Show a password input indicator while echo is off (manaflow-ai#14867)
ec04960 web tests: spawn client-config-env children asynchronously (manaflow-ai#14899)
f10c5ce test: say why the portal fixture's scrollback wait failed (manaflow-ai#14898)
2e6a5ba ci: give each virtual display helper its own serial (manaflow-ai#14897)
208a6bf Keep the unfocused-pane dim in step with focus when a pane is revealed (manaflow-ai#14892)
33b4c92 fix: finish a detach-induced checklist popover close without its animation (manaflow-ai#14895)
a3baec3 test: free the remaining hosted test terminals and scope the portal leak check (manaflow-ai#14888)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant