Repository navigation
ci(owned-pool guard): a RUNNER_POOLS read isn't an owned pool - #32636
austinywang wants to merge 2 commits into
Conversation
feat-cmux-next's #32457 made nearly every runner selection read vars.RUNNER_POOLS, and the guard counted each as able to take an owned Mac, so CI fast guards failed on main and on every pull request, and the macOS admission gate then refused them. runner_pools.py rejects any pool member that isn't an ephemeral Blacksmith label, so these reads can't reach one. fast-checks.yml was watched only for that read, so its entry goes too. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
All contributors have signed the CLA ✍️ ✅ |
|
No actionable comments were generated in the recent review.
|
| Layer / File(s) | Summary |
|---|---|
Update owned-pool classification scripts/ci/owned_pool_sources.py, scripts/ci/owned_pool_rescue.py |
The matcher excludes RUNNER_POOLS from owned-variable detection. The side-workflow set no longer includes .github/workflows/fast-checks.yml. |
Estimated code review effort: 2 (Simple) | ~5 minutes
Change: Bug fix
Possibly related issues
- Issue 30131 — Reports a
test_every_owned_workflow_is_watchedfailure for workflows that readvars.RUNNER_POOLS. This change excludes that read from owned-pool detection. - Issue 31981 — Reports a persistent-pool rescue guard failure for workflows that read
vars.RUNNER_POOLS. This change excludes that read from owned-pool detection.
Possibly related PRs
- manaflow-ai/cmux#27129 — Introduces
RUNNER_POOLSand workflow reads that this change alters how the owned-pool guard classifies. - manaflow-ai/cmux#26585 — Adds the runner-pool declaration and reconciler related to this classification change.
- manaflow-ai/cmux#30840 — Adds a rescue guard exception for a workflow that reads
RUNNER_POOLS.
Suggested reviewers: teamleaderleo
Merge Risk: 
Minimal · up to 681b9
-
The classification change removes false owned-runner matches without an established loss of applicable rescue behavior. Mergeable subject to normal checks.
Pre-merge checks: 
24 · 
1
-
Failed checks: 1 warning Passed checks: 24 passed-
Full details: Description check
-
Explanation
The description explains the failure, intended behavior, implementation, testing results, known local test limitation, and changelog status. It omits the template’s Proof and Checklist sections, but the missing sections are non-critical for this CI-only change.
Full details: Cmux Cloud Persistent Session And Early Input
-
Explanation
PASS. The review-scoped diff changes only CI owned-pool classification: it removes
fast-checks.ymlfromSIDE_WORKFLOW_PATHSand excludesRUNNER_POOLSfromMarkers.variablesinscripts/ci/owned_pool_sources.py. It does not change Cloud terminal creation, cmux-tui transport, renderer readiness, input routing, authentication, idempotency, revision fences, or leases. The custom rule therefore does not apply to a failing behavior.
Full details: Cmux Swift Actor Isolation
-
Explanation
The PR changes only two Python CI scripts:
scripts/ci/owned_pool_rescue.pyandscripts/ci/owned_pool_sources.py. The reviewed diff contains no Swift files or Swift actor-isolation changes, so this check is not applicable.
Full details: Cmux Swift Blocking Runtime
-
Explanation
The pull request changes only two Python CI scripts. The authoritative diff contains no Swift files and introduces no semaphores, waits, sleeps, delayed dispatch, polling, main-queue synchronization, or manual locks.
Full details: Cmux Browser Automation Off-Main
-
Explanation
PASS: The reviewed diff changes only
scripts/ci/owned_pool_rescue.pyandscripts/ci/owned_pool_sources.py. It does not modifySources/TerminalController.swift,ControlCommandExecutionPolicy.swift, browser socket commands, worker routing, or policy tests. The browser automation WebKit-waits rule is therefore not applicable.
Full details: Cmux Expensive Synchronous Load
-
Explanation
The pull request changes only
scripts/ci/owned_pool_rescue.pyandscripts/ci/owned_pool_sources.py. The diff contains no Swift files, agent-history loads, workspace scans, or interactive-path changes. The custom check is therefore not applicable.
Full details: Cmux Cache Substitution Correctness
-
Explanation
PASS: The authoritative PR diff changes only
scripts/ci/owned_pool_rescue.pyandscripts/ci/owned_pool_sources.py, both Python CI guard code. It does not change production Swift, TypeScript, or JavaScript code, and it does not substitute a cached value in a persistence, history, undo, or snapshot path.
Full details: Cmux No Hacky Sleeps
-
Explanation
PASS: The pull request changes only CI guard classification in two Python scripts. The diff removes one workflow path and excludes
RUNNER_POOLSfrom owned-variable matching. It introduces no sleep, timer, polling loop, fixed delay, or wall-clock wait. Existing timing code is not worsened.
Full details: Cmux Algorithmic Complexity
-
Explanation
The diff only changes two Python CI scripts. It removes one fixed workflow-path entry and subtracts the fixed
BLACKSMITH_ONLY_VARIABLESset from a marker set. It introduces no nested scalable-collection scan, batch rescan, hot-path sort/filter, in-memory join, or slower algorithm for user-owned records. The existingsorted(...)call is unchanged.
Full details: Cmux Swift Concurrency
-
Explanation
The pull request changes only two Python CI scripts:
scripts/ci/owned_pool_rescue.pyandscripts/ci/owned_pool_sources.py. The diff contains no Swift files or Swift concurrency code, so it does not introduce or expand any listed legacy async pattern.
Full details: Cmux Swift `@Concurrent`
-
Explanation
PASS: The pull request changes only two Python files (
scripts/ci/owned_pool_rescue.pyandscripts/ci/owned_pool_sources.py). It introduces no Swift code,@concurrentannotation,nonisolated asyncwork, or Swift call-site changes covered by the rule.
Full details: Cmux Swift Package Boundaries
-
Explanation
The pull request changes only two Python CI scripts:
scripts/ci/owned_pool_rescue.pyandscripts/ci/owned_pool_sources.py. The diff contains no Swift changes, so the Swift package-boundary check does not apply.
Full details: Cmux Swiftpm Lockfiles
-
Explanation
PASS. The reviewed diff changes only two CI Python scripts. It changes no Package.swift, Package.resolved, .gitignore, Xcode project, workspace, workflow, or dependency file, and adds no SwiftPM or Xcode package-reference change. Therefore the SwiftPM lockfile failure conditions do not apply.
Full details: Cmux Swift Logging
-
Explanation
PASS: The pull request changes only two Python files:
scripts/ci/owned_pool_rescue.pyandscripts/ci/owned_pool_sources.py. The reviewed range contains no Swift files and adds or changes no Swift logging.
Full details: Cmux User-Facing Error Privacy
-
Explanation
The diff changes only internal CI rescue and workflow-analysis logic. It adds internal comments and excludes
vars.RUNNER_POOLSfrom owned-pool detection, and removes one CI workflow path. The changed text is not an app UI, product CLI, or product API message. The repository shows these scripts run from internal GitHub Actions and CI guard tests, so the user-facing error privacy rule does not apply.
Full details: Cmux Full Internationalization
-
Explanation
PASS. The PR changes only
scripts/ci/owned_pool_rescue.pyandscripts/ci/owned_pool_sources.py. The changes remove a workflow path and alter CI source-detection logic and developer comments. They add no user-facing Swift, web, API, markdown, changelog, or localization text, and they require no locale or catalog updates.
Full details: Cmux Swiftui State Layout
-
Explanation
PASS: The pull request changes only two Python CI scripts:
scripts/ci/owned_pool_rescue.pyandscripts/ci/owned_pool_sources.py. The diff contains no SwiftUI or Swift files, so the SwiftUI state-layout rules do not apply.
Full details: Cmux Architecture Rethink
-
Explanation
The pull request changes only Python CI scripts:
scripts/ci/owned_pool_rescue.pyandscripts/ci/owned_pool_sources.py. The diff contains no Swift, AppKit, or SwiftUI changes and introduces none of the listed Swift architecture patterns.
Full details: Cmux Swift Auxiliary Window Close Shortcuts
-
Explanation
The pull request changes only
scripts/ci/owned_pool_rescue.pyandscripts/ci/owned_pool_sources.py. The diff contains no Swift, NSWindow, NSPanel, NSWindowController, SwiftUIWindow, orWindowGroupcode. The auxiliary-window shortcut rule is therefore not applicable.
Full details: Cmux Source Artifacts
-
Explanation
The diff changes only two hand-written Python CI scripts:
scripts/ci/owned_pool_rescue.pyandscripts/ci/owned_pool_sources.py. It removes one workflow entry and updates owned-pool policy logic. No logs, screenshots, caches, build output, temporary directories, dependency checkouts, or other artifact paths enter source control.
Full details: Cmux No Test Or Debug Seam In Production Source
-
Explanation
The pull request changes only two Python files:
scripts/ci/owned_pool_rescue.pyandscripts/ci/owned_pool_sources.py. The diff contains no Swift file under a productionSources/path, so it cannot introduce a test or debug seam covered by this check.
-
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 1 functions across 1 files.
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
-
Finishing Touches
-
Generate docstrings
-
- Commit to this branch
- Create a new PR
Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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.
CodeRabbit Verify
- Run CodeRabbit Verify to test the changes in this pull request.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Correctness review (subagent; replayed both branches' guard code against both trees, not run in CI on a
Clean: no owned route through a pool, no rescue behaviour change from removing |
|
Combined into #32585 at the requester's ask. |
Summary
CI fast guardshas failed since #32457 landed onfeat-cmux-next, which made nearly every runner selection readvars.RUNNER_POOLS. The guard countsvars.RUNNER_POOLSas able to take an owned pool, so it reported 116 workflows as unwatched. That failure makes the macOS admission gate refuse app builds and app tests on pull requests to main.feat-cmux-nexthas its own copy of the guard: #32649.scripts/ci/runner_pools.pyrejects any pool member that isn't an ephemeral Blacksmith label, so these reads can't land on an owned Mac. This stops counting them.fast-checks.ymlwas watched only because of that read, so its entry is removed, as the guard's other check requires. The rescue does nothing for a run with no owned job, so no routing changes.Testing
python3 tests/test_ci_owned_pool_sources.pywithorigin/feat-cmux-nextat 25db344:test_every_owned_workflow_is_watched.python3 tests/test_ci_owned_pool_rescue.pyhas the same 2SpendLesserrors locally on base and head (KeyError: 'file', this Mac's Python 3.9), so CI is the check for those.The
_POOL_READ_WHYentries infeat-cmux-next's copy of the rescue become unnecessary; they do no harm and are left for that branch.Changelog
No entry: CI only.
Base 21dcc2b, head 681b9dd. Agent: Claude Code.
🤖 Generated with Claude Code
Summary by CodeRabbit