Skip to content

test: keep terminal startup out of the sidebar unread invalidation window - #14258

Merged
teamleaderleo merged 1 commit into
manaflow-ai:mainfrom
teamleaderleo:fix/sidebar-unread-row-invalidation
Sep 24, 2026
Merged

teamleaderleo merged 1 commit into
manaflow-ai:mainfrom
teamleaderleo:fix/sidebar-unread-row-invalidation

Conversation

@teamleaderleo

@teamleaderleo teamleaderleo commented Sep 24, 2026 •

Copy link
Copy Markdown
Collaborator

testUnreadChangeUpdatesOnlyAffectedSidebarRow failed on main run 36022521492 (674b92f) and passed on 36021096031 (1ecdcf9). Nothing in Sources/ or cmuxTests/ changed between them, so the failure is a timing flake in the fixture, not a product regression.

Cause

The test built its fixture with TabManager() and a default addWorkspace, so it had two real terminal workspaces. During the measured interval, the selected terminal starts a login shell (the shard log shows io_exec: started subcommand path=/usr/bin/login inside the test), and each terminal workspace schedules a git metadata probe (autoRefreshMetadata defaults to true). Either can publish a workspace change after counts.reset(), and a workspace change legitimately re-renders VerticalTabsSidebar. Whether it lands inside the window depends on runner speed.

The unread change itself does not touch the sidebar body: the badge goes through SidebarUnreadModel to the AppKit row, and the per-row assertions passed in the failing run.

Change (test only)

  • Build the fixture the way the sibling testMinimalModeToggleDoesNotReevaluateChromeHeavyBodies does since Make app-host unit tests green on main #13643: no initial or welcome workspace, and two .cloudVMLoading workspaces. That fixture passed the same body-count checks on main runs 36021096031 and 36022521492.
  • Enable _printChanges tracing only during the measured interval, so any future failure names what re-rendered the sidebar.
  • Tear the workspaces down at the end. Assertions are unchanged.

Residual risk: the test no longer covers unread routing for terminal-backed workspaces. The badge path is keyed by workspace id, so the panel type should not matter.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Tests
    • Updated validation for unread-status changes in the sidebar, using a controlled workspace setup and more focused measurement cleanup. No end-user behavior changes are included in this update.

…test

testUnreadChangeUpdatesOnlyAffectedSidebarRow failed once on main
(run 36022521492, 674b92f, :400 verticalTabsSidebarBody 1 == 0) and
passed on the run just before it (36021096031, 1ecdcf9) and on the four
runs before that. The commits between the passing and failing SHAs change
only CI workflows, docs and CI tests, so the failure is timing, not a
product regression.

The fixture used TabManager() and a default addWorkspace, which gives
two terminal workspaces. The selected one starts a login shell during the
test (io_exec lines in the shard log) and each schedules an initial git
metadata probe. Either can publish a workspace change after counts.reset(),
and workspace changes legitimately re-evaluate VerticalTabsSidebar, so the
count depended on runner speed.

Build the fixture the way testMinimalModeToggleDoesNotReevaluateChromeHeavyBodies
does since manaflow-ai#13643: no initial or welcome workspace, loading-card workspaces,
and finalize them on teardown. Arm _printChanges tracing for the measured
interval so a future failure names what invalidated the sidebar. Every
assertion is unchanged.

Co-Authored-By: Claude Opus 5.5 <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 24, 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: 42ed7fc8-d912-43e7-94b8-8267b08435ef

📥 Commits

Reviewing files that changed from the base of the PR and between 7cf4e10 and 9ba357d.

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

Included review availability: Your plan provides up to 10 included reviews per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The unread sidebar invalidation test now creates explicit loading-card workspaces, measures probe invalidations only during the unread update, and finalizes all workspaces during cleanup.

Changes

Unread sidebar invalidation test

Layer / File(s) Summary
Workspace setup and scoped invalidation measurement
cmuxTests/WorkspaceContentViewVisibilityTests.swift
The test creates two loading-card workspaces and uses the added selected workspace’s ID. It scopes probe tracing to the unread-update measurement and finalizes workspaces in the cleanup defer.

Estimated code review effort: 2 (Simple) | ~8 minutes

Suggested reviewers: lawrencecchen

Merge Risk: ⚪ Minimal · up to 9ba35

The fixture preserves the shared unread-row invalidation path and verifies both affected and unaffected rows. No actionable merge risk is established.

🚥 Pre-merge checks | ✅ 24 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (24 passed)
Check name Status Explanation
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 pull request changes only cmuxTests/WorkspaceContentViewVisibilityTests.swift. The diff replaces live terminal fixtures with .cloudVMLoading test workspaces, changes invalidation tracing…
Cmux Swift Actor Isolation ✅ Passed PASS: The review-scoped diff changes only cmuxTests/WorkspaceContentViewVisibilityTests.swift. The changes are confined to the test fixture, invalidation probe, and cleanup for `testUnreadChangeUpda…
Cmux Swift Blocking Runtime ✅ Passed PASS. The authoritative diff changes only cmuxTests/WorkspaceContentViewVisibilityTests.swift. It adds deterministic test fixture setup, tracing gates, and cleanup. It adds no blocking or timing pri…
Cmux Browser Automation Off-Main ✅ Passed PASS: The pull request changes only cmuxTests/WorkspaceContentViewVisibilityTests.swift. The diff updates a test fixture, invalidation tracing, and workspace cleanup. It adds no browser socket comma…
Cmux Expensive Synchronous Load ✅ Passed PASS: The pull request changes only cmuxTests/WorkspaceContentViewVisibilityTests.swift. The diff modifies test fixtures, tracing, and cleanup. It adds no production Swift code and no synchronous ag…
Cmux Cache Substitution Correctness ✅ Passed The pull request changes only cmuxTests/WorkspaceContentViewVisibilityTests.swift. It updates a test fixture and cleanup behavior. It does not change production code or replace an authoritative read…
Cmux No Hacky Sleeps ✅ Passed PASS. The PR changes only cmuxTests/WorkspaceContentViewVisibilityTests.swift, which is Swift test code. The diff adds no sleep, timer, polling loop, delayed dispatch, or wall-clock wait. It repla…
Cmux Algorithmic Complexity ✅ Passed PASS. The PR changes only cmuxTests/WorkspaceContentViewVisibilityTests.swift; no production file changes are present. The modified code is test fixture and measurement scaffolding, which the algori…
Cmux Swift Concurrency ✅ Passed The PR changes only a Swift XCTest fixture. The added code uses existing async test infrastructure and await Self.drainMainRunLoop; it adds no DispatchQueue, DispatchGroup, Combine state, comple…
Cmux Swift @Concurrent ✅ Passed PASS — The diff changes only a @MainActor test fixture and measurement cleanup. It adds no @concurrent or nonisolated async function and does not add a CPU-, file-, or network-heavy async helper…
Cmux Swift Package Boundaries ✅ Passed The authoritative diff changes only cmuxTests/WorkspaceContentViewVisibilityTests.swift. It changes test fixture setup and cleanup; it introduces no production Swift feature or reusable domain logic…
Cmux Swiftpm Lockfiles ✅ Passed The PR changes only cmuxTests/WorkspaceContentViewVisibilityTests.swift. It does not change a Package.swift, Package.resolved, .gitignore, workflow, or Xcode project package reference. Therefo…
Cmux Swift Logging ✅ Passed PASS: The pull request changes only cmuxTests/WorkspaceContentViewVisibilityTests.swift. It adds no print, debugPrint, dump, NSLog, file logging, or sensitive-data logging. The new `shouldTr…
Cmux User-Facing Error Privacy ✅ Passed PASS: The pull request changes only cmuxTests/WorkspaceContentViewVisibilityTests.swift. It adds test fixture setup, tracing, cleanup, and developer-only comments. No production user-facing error, a…
Cmux Full Internationalization ✅ Passed PASS. The authoritative diff changes only cmuxTests/WorkspaceContentViewVisibilityTests.swift. The changes are test fixture setup, tracing, cleanup, and test comments. The custom rule explicitly all…
Cmux Swiftui State Layout ✅ Passed PASS. The PR changes only cmuxTests/WorkspaceContentViewVisibilityTests.swift. It adjusts test fixture setup, invalidation tracing, and cleanup. The diff introduces no ObservableObject, `@Publishe…
Cmux Architecture Rethink ✅ Passed PASS. The PR changes only cmuxTests/WorkspaceContentViewVisibilityTests.swift. It replaces live terminal workspaces with explicit .cloudVMLoading fixtures, gates existing invalidation tracing arou…
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed PASS. The PR changes only cmuxTests/WorkspaceContentViewVisibilityTests.swift. It adjusts test workspaces, tracing, and cleanup. The NSWindow remains a test-only fixture, which the rule explicitly…
Cmux Source Artifacts ✅ Passed PASS. The PR changes only cmuxTests/WorkspaceContentViewVisibilityTests.swift, a hand-written test source file. The diff contains no artifact-like path, binary content, logs, caches, build output, s…
Cmux No Test Or Debug Seam In Production Source ✅ Passed The authoritative diff changes only cmuxTests/WorkspaceContentViewVisibilityTests.swift. It contains no Swift file under a production Sources/ path, and the pull request adds no production test/de…
Title check ✅ Passed The title clearly and concisely identifies the main change: preventing terminal startup from affecting the sidebar unread invalidation measurement window.
Description check ✅ Passed The description clearly explains the flaky test failure, root cause, fixture changes, unchanged assertions, and residual risk. It does not use the template headings or include the checklist, but it pr…
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • 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 enabled auto-merge (squash) September 24, 2026 17:06
@teamleaderleo
teamleaderleo merged commit e3ac98d into manaflow-ai:main Sep 24, 2026
51 checks passed
rustybret pushed a commit to rustybret/bmux that referenced this pull request Sep 24, 2026
40adc27 ci: drop compile admission's reads of the retired persistent-restore step (manaflow-ai#14260)
9415c2d fix(nushell): stop hiding the claude wrapper for every session (manaflow-ai#14263)
710ea01 Pace mobile render-grid frames per surface: dynamic ~11fps floor with keystroke-echo bypass (manaflow-ai#14031)
c6f41e7 ci(e2e): wait for an earlier dispatch's compile of the same revision (manaflow-ai#14240)
e3ac98d test: keep live terminals out of the unread sidebar-row invalidation test (manaflow-ai#14258)
464fe13 ci: compare build inputs by content so an adopted seed rebuilds only real changes (manaflow-ai#14262)
ee95353 test: judge renderer retention after the async release lands (manaflow-ai#14247)
eeb5d53 ci: skip a main seed build only when the nearest seed has the same inputs (manaflow-ai#14261)
55d9b75 ci: clone the canonical build root instead of rsyncing it (manaflow-ai#14254)

# Conflicts:
#	.github/workflows/ci-guards.yml
#	.github/workflows/ci-macos.yml
#	.github/workflows/seed-derived-data.yml
#	.github/workflows/test-e2e.yml
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