test(ci): stop the nightly guard leaving scripts/ci on the import path

[teamleaderleo]

Follow-up to #13899, addressing the one actionable CodeRabbit finding on it.

tests/test_ci_nightly_push_throttle.py ran sys.path.insert(0, scripts/ci) inside a test and never took it back out. main() runs every test_* in this file in a single process, so that entry sat at the front of the import path for every test that ran afterwards — whatever they imported would have resolved out of scripts/ci first. Nothing collides today; the point is that nothing stops one from colliding.

The import now goes through a helper that removes the entry in a finally.

Why not CodeRabbit's suggestion. It asked for monkeypatch.syspath_prepend. There is no pytest in this lane — ci-guards.yml runs the file as python3 tests/test_ci_nightly_push_throttle.py, and the repo has no pytest config, no conftest.py, and no pytest dependency — so there is no fixture to restore anything. The bot also marked the finding "✅ Addressed in commit 32381d8"; it was not, the line was still on main.

Tradeoff: a helper instead of two plain import lines. That is the cost of not depending on a test runner this repo does not use.

Guard sweep: 172 of the 177 commands in ci-guards.yml pass locally; the 5 that fail need $RUNNER_TEMP, the ghostty/vendor/bonsplit submodules, or CI-provided arguments, and fail the same way on main.

🤖 Generated with Claude Code

---

^{Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.}

---

Summary by cubic

Stops the nightly-guard test from leaving scripts/ci on the import path, which could make later tests in the same process resolve imports from the wrong directory. The CI scripts now load through a helper that restores the original path in a finally block.

• Because nightly_build_inputs prepends the same directory on import, the helper saves and restores the whole path list instead of removing one entry.
• The lane runs the file as plain python3 rather than under pytest, so there is no fixture to restore the path.

^{Written for commit a99fb21. Summary will update on new commits.}

Summary by CodeRabbit

• Tests
  • Improved isolation between CI-related tests by preventing temporary import-path changes from carrying over into later tests.
  • No user-facing behavior changes.

[coderabbitai]

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: 81681a03-4cf5-4259-a055-9acb1ef00e05

📥 Commits

Reviewing files that changed from the base of the PR and between 6000c0b and a99fb21.

📒 Files selected for processing (1)

• tests/test_ci_nightly_push_throttle.py

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

---

📝 Walkthrough

Walkthrough

The nightly push throttle test now imports CI scripts through a helper that restores the original sys.path after importing them.

Changes

CI test import isolation

Layer / File(s)	Summary
Scoped CI script imports tests/test_ci_nightly_push_throttle.py	A helper imports the CI modules and restores sys.path in a finally block. The test uses the helper instead of inserting the scripts directory inline.

Priority: ⬇️ Low

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

Change: Other

Merge Risk: ⚪ Minimal · up to a99fb

This test-only import-isolation change presents no supported merge risk beyond normal checks.

🚥 Pre-merge checks | ✅ 24 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 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	This check is not applicable. The diff changes only tests/test_ci_nightly_push_throttle.py: it saves and restores sys.path around two test imports. It changes no Cloud terminal creation or transpo…
Cmux Swift Actor Isolation	✅ Passed	The pull request changes only tests/test_ci_nightly_push_throttle.py. It introduces a Python helper and updates a Python test. It contains no production Swift changes, so the Swift actor-isolation f…
Cmux Swift Blocking Runtime	✅ Passed	The reviewed diff changes only tests/test_ci_nightly_push_throttle.py. It adds Python import-path cleanup and changes one test call. The diff contains no production Swift changes or blocking/timing-ba…
Cmux Browser Automation Off-Main	✅ Passed	PASS: The pull request changes only tests/test_ci_nightly_push_throttle.py. Its diff adds ci_scripts() to restore sys.path and updates one test import. It does not modify browser socket commands…
Cmux Expensive Synchronous Load	✅ Passed	PASS: The authoritative PR diff changes only tests/test_ci_nightly_push_throttle.py. It adds a Python helper that restores sys.path; it adds or moves no production Swift code and no synchronous ag…
Cmux Cache Substitution Correctness	✅ Passed	The check applies only to production Swift, TypeScript, and JavaScript changes. The authoritative PR diff changes only tests/test_ci_nightly_push_throttle.py, a Python test file. It introduces no pr…
Cmux No Hacky Sleeps	✅ Passed	The check does not apply to this PR. The authoritative diff changes only tests/test_ci_nightly_push_throttle.py, a Python test file. It adds a helper that saves and restores sys.path around import…
Cmux Algorithmic Complexity	✅ Passed	The diff changes only tests/test_ci_nightly_push_throttle.py. The added helper is Python test scaffolding, not production Swift, TypeScript, JavaScript, shell, or runtime code. The custom check expl…
Cmux Swift Concurrency	✅ Passed	The check is not applicable to this pull request. The authoritative diff changes only tests/test_ci_nightly_push_throttle.py, a Python test file, and includes no Swift files or Swift concurrency cha…
Cmux Swift @Concurrent	✅ Passed	PASS. The reviewed diff changes only tests/test_ci_nightly_push_throttle.py, a Python file. It contains no Swift changes, so the Swift @concurrent check does not apply.
Cmux Swift Package Boundaries	✅ Passed	PASS. The authoritative diff changes only tests/test_ci_nightly_push_throttle.py, a Python test file. It contains no production Swift changes, so the Swift package-boundaries check does not apply.
Cmux Swiftpm Lockfiles	✅ Passed	The pull request changes only tests/test_ci_nightly_push_throttle.py. It does not change a SwiftPM package, Xcode project, .gitignore, workflow, or dependency, so none of the lockfile failure cond…
Cmux Swift Logging	✅ Passed	PASS. The reviewed diff changes only tests/test_ci_nightly_push_throttle.py, a Python test file. It contains no Swift production changes or added logging covered by this check.
Cmux User-Facing Error Privacy	✅ Passed	PASS. The authoritative diff changes only tests/test_ci_nightly_push_throttle.py. It adds a test helper that restores sys.path and updates a test to use that helper. The custom check explicitly pa…
Cmux Full Internationalization	✅ Passed	The pull request changes only tests/test_ci_nightly_push_throttle.py. The diff adds a test helper and updates a test import. The custom check exempts test changes, and the pull request adds no produ…
Cmux Swiftui State Layout	✅ Passed	The pull request changes only tests/test_ci_nightly_push_throttle.py. It introduces no SwiftUI changes, ObservableObject or @Published state, GeometryReader layout measurement, lazy/list row store r…
Cmux Architecture Rethink	✅ Passed	The check applies to Swift architecture changes. The reviewed diff changes only tests/test_ci_nightly_push_throttle.py; it adds a Python helper that restores sys.path and updates one test to use i…
Cmux Swift Auxiliary Window Close Shortcuts	✅ Passed	The check is not applicable. The reviewed diff changes only tests/test_ci_nightly_push_throttle.py and adds no Swift or window code. Therefore, it does not introduce an auxiliary window or alter close…
Cmux Source Artifacts	✅ Passed	The only changed path is tests/test_ci_nightly_push_throttle.py. The diff adds a hand-written test helper and updates a test to use it. The source-control-artifacts rule allows intentional test file…
Cmux No Test Or Debug Seam In Production Source	✅ Passed	The PR changes only tests/test_ci_nightly_push_throttle.py. It adds a Python helper that restores sys.path; it changes no Swift file under a production Sources/ path. The custom check does not a…
Title check	✅ Passed	The title clearly describes the main change: restoring the import path after the nightly guard test loads CI scripts. It is specific and concise.
Description check	✅ Passed	The description explains the change, its rationale, the choice not to use pytest, and local test results. It does not use the template headings or complete the checklist, but it provides enough releva…

• 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

• 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 agent review of 2cbd283685, posted once as required. It rejected the first version of this PR, so the commit now on the branch is a99fb21725.

What it found. The helper did not do what its commit message claimed. scripts/ci/nightly_build_inputs.py:38 prepends the same directory at its own import time, so _ci_scripts() left two equal entries on sys.path and list.remove() deleted one. Measured sys.path.count("…/scripts/ci") in a fresh interpreter: 2 before the PR, 1 with remove(), 0 with the save/restore form — which is the idiom admission_helper in tests/test_ci_change_areas.py already uses. Save/restore also cannot raise out of the finally and mask a real ImportError, which remove() can.

It also scoped the problem down. main() sorts globals(), the target test lands at index 19 of 21, and the single test after it imports nothing. No stem or subdirectory in scripts/ci/ collides with sys.stdlib_module_names or yaml. So this is defense-in-depth, and the original commit message overstated it — reworded per STYLE.md.

Claims it checked and upheld: the guard lane runs this file as python3 tests/... (ci-guards.yml:641-644, and scripts/ci/run_python_test_lane.py:71-76 for the linux-guard lane registered at tests/test-execution.toml:377-379); no conftest.py, pytest config, or pytest dependency anywhere. Both routes bypass pytest, so the rebuttal to CodeRabbit stands. It confirmed main() cannot pick the helper up, that the finally runs on import failure, that module caching hides nothing here, and that import sys is still required.

Verified after the fix: sys.path.count returns to 0, a second call returns the identical cached module objects with the count still 0, and python3 tests/test_ci_nightly_push_throttle.py passes. Guard sweep re-run on the amended commit: 172 of 177 pass, the same 5 environmental failures as main ($RUNNER_TEMP, the ghostty and vendor/bonsplit submodules, pip, and a script needing CI-supplied arguments).

Not taken: a return annotation on the helper, which the file omits on comparison() and run_decide() too, and no Python linter config in the repo enforces one.

— NightPetrel g1 🪁