Skip to content

ci: run the dedicated step when a PR edits an env-gated test - #14381

Merged
teamleaderleo merged 3 commits into
mainfrom
ci/changed-suites-test-only
Sep 25, 2026
Merged

teamleaderleo merged 3 commits into
mainfrom
ci/changed-suites-test-only

Conversation

@teamleaderleo

@teamleaderleo teamleaderleo commented Sep 25, 2026 •

Copy link
Copy Markdown
Collaborator

A PR that edits a test only a dedicated CI step can run could pass without running that test.

The five-tab renderer memory test in GhosttySurfaceOverlayTests skips itself unless its step sets CMUX_RENDERER_MEMORY_REGRESSION=1. That step ran only on shard 6. A test-only PR that edited the test got a changed-suites run of GhosttySurfaceOverlayTests inside compile admission, where the test reported "skipped" and CI went green.

strict_steps() in choose_ci_suite.py now also counts any shard step that names a suite with -only-testing:. A changed-suites run of that suite runs the step too, on the shard 8 worker. If such a step's if: cannot be selected through unit_strict_steps, routing falls back to all seven shards, so the step still runs on its own shard. The renderer memory step now accepts unit_strict_steps.

#14366 was not a case of this. Its compile admission log shows all three edited suites ran and passed: KeyboardShortcutSettingsFileStoreTests (33 XCTest cases), plus SurfacePaneFactoryFocusTests and SurfaceSelectionTests (23 Swift Testing tests), 56 typed results. Shard 8 was skipped because compile admission ran those suites itself.

Proof on this PR: comment-only edits to SurfaceSelectionTests and to the renderer memory test. This PR's CI should run SurfaceSelectionTests and the "Run five-tab renderer memory regression" step on the changed-suites worker.

Tests: tests/test_ci_change_areas.py gains a regression test that fails on main, plus one that pins #14366's routing and checks that an untraceable test edit runs every suite. actionlint is clean.

🤖 Generated with Claude Code


Summary by cubic

Fixes a CI gap where a PR editing a test that runs only in a dedicated CI step could pass without executing that test.

The five-tab renderer memory test skips itself unless its step sets CMUX_RENDERER_MEMORY_REGRESSION=1, but that step only ran on shard 6. A test-only PR editing the test got a changed-suites run where it reported "skipped" and CI went green.

  • strict_steps() now counts any shard step that names a suite with -only-testing:, quoted or unquoted, so a changed-suites run of that suite also runs the step on shard 8.
  • If a step that must run can't be selected through unit_strict_steps, routing falls back to all seven shards so the step still runs on its own shard.
  • The renderer memory step now accepts unit_strict_steps.
  • Adds regression tests: one pinning that test: fix three app-host tests failing on main #14366 was not affected by this gap, one proving a looped -only-testing:$suite step lists only strict suites, and one covering the unselectable-step fallback.

Written for commit 3d628ac. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

  • Improvements

    • CI more reliably routes test changes to the relevant test suites and dedicated checks, including the renderer memory regression check.
    • Changes that cannot be attributed to a specific suite continue to receive broader test coverage.
  • Tests

    • Added coverage for test-change routing, dedicated checks, and fallback behavior when changes cannot be matched to a suite.

teamleaderleo and others added 2 commits September 25, 2026 00:36
The five-tab renderer memory test skips itself unless its dedicated step sets
CMUX_RENDERER_MEMORY_REGRESSION=1. A PR that edits it gets a changed-suites
run of GhosttySurfaceOverlayTests that only runs the shared batch, so the
edited test reports "skipped" and the run passes. This test fails on main.

Also pins that #14366's test-only diff selects each edited suite and that an
untraceable test edit runs every suite.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
strict_steps() now treats any shard step that names a suite with
-only-testing: as a step a changed-suites run of that suite must run, and
returns None (every shard runs) when such a step cannot be selected through
unit_strict_steps. The renderer memory step now accepts unit_strict_steps.

Comment-only edits to SurfaceSelectionTests and the renderer memory test prove
the routing on this PR's own CI.

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 25, 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: bfe4db46-a2de-43ef-a520-78f448127710

📥 Commits

Reviewing files that changed from the base of the PR and between 1036ca8 and 3d628ac.

📒 Files selected for processing (2)
  • scripts/ci/choose_ci_suite.py
  • tests/test_ci_change_areas.py

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


📝 Walkthrough

Walkthrough

CI suite selection maps changed test suites to eligible strict workflow steps. The renderer-memory regression step also runs when changed-suite routing selects it.

Changes

Changed-suite CI routing

Layer / File(s) Summary
Suite attribution and strict-step selection
scripts/ci/choose_ci_suite.py, tests/test_ci_change_areas.py, cmuxTests/SurfaceSelectionTests.swift, cmuxTests/TerminalAndGhosttyTests.swift
The routing logic maps test suites to owning strict steps that can be selected through inputs.unit_strict_steps. Tests cover suite attribution and selection outcomes. Comments identify suites that use this routing.
Regression step selection
.github/workflows/ci-macos.yml
The renderer-memory regression step runs for its focused-regression shard or when selected through unit_strict_steps.

Priority: ➖ Normal

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

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 3d628

Changed suites can select their owning CI steps, and uncertain ownership broadens testing to all seven shards. No concrete coverage regression remains, so the PR is mergeable subject to normal checks.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 3d628

The change closes a path where CI could pass after skipping an edited test. No new security exposure was identified, but the assessment does not include a live CI run.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The identified security-relevant outcome is CI coverage for PR-edited tests. The flagged public entrypoint is a test-only caller, with no identified expansion of a production service or authorization boundary.

Trust Boundaries and Controls

  • inferred — A PR's changed-file information influences test selection, but an unselectable required step broadens execution to the full shard set rather than allowing a narrowed run to omit it.

Resilience and Maintainability Implications

  • observed — The fallback regression removes the step's selection condition and verifies that suite narrowing is abandoned, preserving the broader execution path.
🚥 Pre-merge checks | ✅ 24 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 62.50% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 8 functions across 4 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (24 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main CI routing change for environment-gated tests.
Description check ✅ Passed The description clearly explains the CI gap, the routing fix, the fallback behavior, and the added regression tests. It provides testing information, although it does not use the template's separate T…
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 macOS CI routing, test comments, and choose_ci_suite.py plus routing tests. The diff adds no Cloud terminal creation, persistent cmux-tui transport, manual-pane admission, att…
Cmux Swift Actor Isolation ✅ Passed The pull request changes only comments in two Swift test files. It introduces no production Swift declarations, actor-isolation changes, Sendable reference types, or background UI-store access. The re…
Cmux Swift Blocking Runtime ✅ Passed PASS. The pull request changes only comments in the two Swift test files. The added Swift lines introduce no semaphores, waits, sleeps, delayed dispatch, polling, main-queue sync, or manual locks. The…
Cmux Browser Automation Off-Main ✅ Passed The PR does not change browser socket automation routing. The rule-scoped files Sources/TerminalController.swift and ControlCommandExecutionPolicy.swift are unchanged, and no added or removed line…
Cmux Expensive Synchronous Load ✅ Passed PASS: The PR does not add or move an expensive synchronous agent-history load. The only Swift changes are comments in cmuxTests/SurfaceSelectionTests.swift and `cmuxTests/TerminalAndGhosttyTests.swi…
Cmux Cache Substitution Correctness ✅ Passed PASS: The pull request does not introduce a cache substitution. Its Swift changes only add comments; the other changes are Python, YAML, and test code. No production Swift, TypeScript, or JavaScript p…
Cmux No Hacky Sleeps ✅ Passed PASS. The PR adds no fixed sleeps, timers, polling, delayed dispatch, or wall-clock synchronization in covered runtime scripts. The Python changes only route CI suites, and the added Python tests are …
Cmux Algorithmic Complexity ✅ Passed The changed production code only extends strict_steps() to inspect CI workflow configuration. Its nested scan covers 54 fixed workflow steps and 38 fixed focused selectors, not user-owned records or…
Cmux Swift Concurrency ✅ Passed PASS: The only changed Swift lines are comments in SurfaceSelectionTests.swift and TerminalAndGhosttyTests.swift. They add no Dispatch, Combine, completion-handler, or fire-and-forget Task cod…
Cmux Swift @Concurrent ✅ Passed PASS: The Swift diff adds only comments in SurfaceSelectionTests.swift and TerminalAndGhosttyTests.swift. It does not add or modify async, nonisolated, @concurrent, actor isolation, or async…
Cmux Swift Package Boundaries ✅ Passed The PR changes only comments in two Swift test files. It introduces no production Swift logic, app-target feature, or reusable domain code. The Swift package-boundary rule is therefore not triggered.
Cmux Swiftpm Lockfiles ✅ Passed PASS. The PR changes CI routing, Swift test comments, and Python routing tests only. The authoritative diff contains no Package.swift, Package.resolved, .gitignore, or Xcode project package-reference …
Cmux Swift Logging ✅ Passed PASS. The only changed Swift lines are comments in cmuxTests test files. They add no print, debugPrint, dump, NSLog, file logging, Logger declaration, or sensitive-data logging. The logging rule permi…
Cmux User-Facing Error Privacy ✅ Passed PASS: The diff changes only GitHub Actions CI routing, a CI selector script, tests, and developer-only comments. The script output is consumed as GitHub Actions job outputs, and no changed text reache…
Cmux Full Internationalization ✅ Passed PASS. The diff changes only CI workflow logic, CI tooling, tests, and developer-only comments. It adds no production user-facing text, app localization catalog entries, or web locale/message files. Th…
Cmux Swiftui State Layout ✅ Passed PASS: The pull request does not introduce SwiftUI changes. The only Swift changes add comments in test files; the remaining changes are CI and Python logic. The diff adds no ObservableObject, @Publish…
Cmux Architecture Rethink ✅ Passed PASS: The pull request does not introduce a Swift architecture change. The only Swift additions are comments in two test files. The executable changes are CI workflow, Python routing, and Python tests…
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed PASS — The only Swift changes add comments in cmuxTests/SurfaceSelectionTests.swift and cmuxTests/TerminalAndGhosttyTests.swift. The diff adds no NSWindow, NSPanel, NSWindowController, SwiftUI Win…
Cmux Source Artifacts ✅ Passed The diff changes only tracked workflow configuration, Swift tests, and Python CI/test scripts. No logs, media, caches, build output, temporary directories, dependency checkouts, or other artifact-like…
Cmux No Test Or Debug Seam In Production Source ✅ Passed The PR changes no Swift file under a production Sources/ path. The only changed Swift files are under cmuxTests/, and their changes are comments. Therefore, the production test/debug seam rule doe…
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 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.

…uites

strict_steps() now reads `-only-testing:"cmuxTests/Suite"` as well as the
unquoted form. A test checks that every shard step looping over
`-only-testing:"cmuxTests/$suite"` lists only FOCUSED_GATE_SELECTORS suites,
which strict_steps() finds by name, and the unselectable-step test first
proves its fixture selects the suite.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@teamleaderleo
teamleaderleo enabled auto-merge (squash) September 25, 2026 05:12
@teamleaderleo
teamleaderleo merged commit f7c94b1 into main Sep 25, 2026
69 checks passed
@teamleaderleo
teamleaderleo deleted the ci/changed-suites-test-only branch September 25, 2026 05:26
rustybret pushed a commit to rustybret/bmux that referenced this pull request Sep 25, 2026
5a81d71 Merge pull request manaflow-ai#14122 from manaflow-ai/issue-14037-window-display-hang
1a5be43 Merge pull request manaflow-ai#13020 from manaflow-ai/13016-sidebar-new-local-workspace
9e61fc2 ci: rebalance app-host shards from measured timings on all seven workers (manaflow-ai#14393)
8848a92 Merge pull request manaflow-ai#14044 from manaflow-ai/13648-ssh-switch-latency
caae250 Merge pull request manaflow-ai#13055 from manaflow-ai/13049-computer-use-onboarding
55dcb23 ci: let a warm owned Mac adopt a near seed instead of its kept build (manaflow-ai#14385)
e4e3d88 fix: harden warm reveal and CI array guards
ac5bdd9 Merge branch 'main' of https://github.com/manaflow-ai/cmux into 13049-computer-use-onboarding
d3865b3 Merge pull request manaflow-ai#14371 from manaflow-ai/14294-helper-staging-leak
bd018b1 ci: run unsigned iOS jobs on owned minis with counted simulator capacity (manaflow-ai#14389)
7ca7818 fix: avoid inheriting SSH cloud directories locally
f7c94b1 ci: run the dedicated step when a PR edits an env-gated test (manaflow-ai#14381)
566c83f ci: follow changed string literals in the reverse test impact report (manaflow-ai#14387)
87bf6ae fix(ci): skip installing the test module when emission is disabled
28147df Merge pull request manaflow-ai#14382 from manaflow-ai/14273-accessibility-children-cycle
4ff4cde Merge pull request manaflow-ai#14384 from manaflow-ai/12925-split-hint-stuck
bf65819 test: assert mounted sidebar and project AX reachability
55e4147 project: group drag tests beside their existing suite
2867b94 test: release MainActor while awaiting hint dismissal
5847394 fix(ci): handle empty app-host output batches
2c52835 Merge branch 'main' of https://github.com/manaflow-ai/cmux into 13648-ssh-switch-latency
e8dd4e0 test: update sidebar regression for scoped Cloud creation
63608eb Merge branch 'main' of https://github.com/manaflow-ai/cmux into 13016-sidebar-new-local-workspace
38ca93f Merge remote-tracking branch 'origin/main' into 14294-helper-staging-leak
209a484 Merge remote-tracking branch 'origin/main' into 14273-accessibility-children-cycle
9193381 fix: compare helper inventory independently of URL normalization
1c2fda2 fix: make sidebar AX queries preserve readable text without setters
373ed39 test: cover upgrades from legacy read-only helper generations
c7a8d40 fix: normalize managed helper directory modes before atomic publication
69827d4 Merge branch 'main' of https://github.com/manaflow-ai/cmux into 12925-split-hint-stuck
088d07e test: preserve sidebar text and forbid AX getter writes
245daa1 refactor: preserve helper installation errors for diagnostics
ce1d8bc test: isolate helper copy failure fixtures within tasks
b7cf47b Merge remote-tracking branch 'origin/main' into 12925-split-hint-stuck
53f6251 fix: scope split hints to the native drag lifetime
c2db719 fix: make helper replacement atomic and bound retries
2724342 Merge remote-tracking branch 'origin/main' into 13049-computer-use-onboarding
8280bd3 fix: handle optional restore bindings in workspace liveness
1f98d5e fix: prepare helper directory parent
fbad4c1 Merge remote-tracking branch 'origin/main' into 14273-accessibility-children-cycle
c59df55 fix: keep sidebar accessibility children acyclic
646d361 Merge remote-tracking branch 'origin/main' into 14294-helper-staging-leak
b9ab24f test: reproduce sidebar accessibility children cycle
7464b12 Merge remote-tracking branch 'origin/main' into 13049-computer-use-onboarding
b690bb1 test: keep canonical build guard stable across CI recipe changes
c58c708 test: cover file-drop hint lifecycle teardown
266fbc7 Merge remote-tracking branch 'origin/main' into 13016-sidebar-new-local-workspace
81f274f fix: make helper cleanup event driven
cd4a936 fix: reject malformed helper staging names
bdd08d0 Merge branch 'main' of https://github.com/manaflow-ai/cmux into 13648-ssh-switch-latency
0c23d10 Merge remote-tracking branch 'origin/main' into 13049-computer-use-onboarding
cfe4407 Merge remote-tracking branch 'origin/main' into 14294-helper-staging-leak
b1f4f89 fix: bound Computer Use helper staging
cdc90c7 test: reproduce helper staging leak
fdbcb3c Merge origin/main into 13049-computer-use-onboarding
c80b7be fix: avoid AppKit frame constrain reentry
21983c5 fix: guard display frame reconciliation against reentry
343b4fb chore: keep renderer changes within file budgets
d8e7469 fix: use warm reveal refresh policy in production path
a1baca1 chore: keep renderer extension within file budget
772d634 fix: retain warm frame state across terminal hides
b92dfdd fix: avoid redundant terminal refresh on warm workspace reveal
ff4795d Merge remote-tracking branch 'origin/main' into 13016-sidebar-new-local-workspace
f8c4493 fix: allow CUA from tagged dev Codex sessions
38e7acf fix: resolve post-merge restore build errors
5fd391f Merge origin/main into 13049-computer-use-onboarding
c9f363f fix: preserve nonblocking scoped feed telemetry
63378ac test: keep first-use Computer Use telemetry nonblocking and scoped
ed9c946 Merge branch 'main' of https://github.com/manaflow-ai/cmux into 13049-computer-use-onboarding
16bfc27 fix: consume relay origin before serializing the ordering environment
3321b39 test: validate relay barriers with the host admission parser
dcf085a fix: retain filtering for remote hook transports
3d60cd8 fix: keep relayed hook ordering out of local process routing
88cca73 test: retain relay origin in feed ordering barriers
126b2db Merge branch 'main' of https://github.com/manaflow-ai/cmux into 13049-computer-use-onboarding
75dde56 Merge remote-tracking branch 'origin/main' into 13049-computer-use-onboarding
cbf9edb fix: preserve relay origin through feed target resolution
24a7c8c test: cover relay-origin feed admission and first-use attachment
e167935 Merge remote-tracking branch 'origin/main' into 13049-computer-use-onboarding
23b1dfc docs: brand the provider as cmux Computer Use
12fd7b0 Merge remote-tracking branch 'origin/main' into 13049-computer-use-onboarding
c4f611a fix: restore Swift parameter separators
203bec4 fix: validate both helper profiles before readiness
a31f9a6 fix: refresh revocation before first-use admission
9e9df10 fix: keep first-use credentials and revocation state current
d5e9bf4 fix: preserve readiness and relay feed admission
d3e906b fix: restore scoped completion before daemon readiness
e17514c test: use the direct capture outcome API
ec9b6e8 fix: report stale capture verification as unavailable
1260836 fix: keep relay routing and Swift 6 compatibility fail closed
267168e Merge remote-tracking branch 'origin/main' into 13049-computer-use-onboarding
edf1834 fix: expose shared feed target resolver to CLI extensions
b4f816d fix: bind onboarding work to view task lifecycle
55ffd3e fix: close Computer Use onboarding admission races
03772a9 test: expose Computer Use onboarding review regressions
5ddf044 Merge remote-tracking branch 'origin/main' into 13049-computer-use-onboarding
a61e770 fix: restore Computer Use test API visibility
5b8d894 fix: expose setup status for Settings snapshot
811990b fix: restore onboarding completion key compatibility
797bbac fix: wire Computer Use Settings host actions
78128dd fix: expose onboarding completion status to UI
b40ea20 fix: expose package transport to capture verification
b1d2670 fix: expose helper startup to capture admission
f8ea3c8 fix: expose runtime seams to package onboarding adapters
138d703 fix: import package transport types for capture verification
72966a2 fix: resolve feed targets through live delivery
e7231ca test: restore Computer Use onboarding target wiring
f0c6c1e fix: adapt Computer Use onboarding to current main architecture
66de110 Merge origin/main into 13049-computer-use-onboarding
bb1d403 fix: close remaining onboarding review findings
d8cff69 docs: document Computer Use core contracts
f9d1a3c docs: describe automatic Computer Use setup
a12ef64 fix: close Computer Use onboarding review gaps
6cb9cf2 Merge origin/main into 13049-computer-use-onboarding
1021c83 fix: notify Computer Use directly at feed ingress
ba61825 fix: present onboarding before helper provisioning
7c0443f fix: accept live owned surface for first-use onboarding
7db2f4b fix: recognize all owned terminal surfaces for first-use setup
73e3144 fix: opt into Computer Use setup from first explicit request
dce767a test: reproduce lost Computer Use hook surface in built CLI
4b40d82 fix: present setup before live session indexing
a8d61c5 fix: make cmux-cua the only Codex computer provider
c0773d9 fix: await live session indexing before first-use setup
82efcbf fix: open Computer Use setup on the first functional tool request
53256c9 test: require setup presentation on the first Computer Use tool
b6625cd fix: recheck grants through the shared daemon control protocol
ac17c49 fix: invalidate stale capture proof and roll back partial admission
1c21a39 fix: keep Computer Use setup status live through completion
1146f41 fix: make Computer Use setup completion runtime-owned and recoverable
3c44d5c test: reject stale Computer Use onboarding completion after disable
b144586 fix: make sidebar New Workspace explicitly local
3e77548 test: cover local workspace creation from sidebar plus menu

# Conflicts:
#	.github/workflows/ci-macos.yml
#	.github/workflows/ci-owned-pool-rescue.yml
#	.github/workflows/ci.yml
#	.github/workflows/ios-screenshots.yml
#	.github/workflows/ios-streamed-validate.yml
#	.github/workflows/iroh-release-gate.yml
#	.github/workflows/test-ios.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