Repository navigation
ci: keep backend credentials on bounded ephemeral Linux routes - #30761
Conversation
|
cmux status (one comment, edited in place; each line is updated by the job that owns it) Review App of this head (app.zip, kept 1 day)
red: cmux-next checks (god files, concurrency, crash safety, l10n); no finished base run on 3154610 to compare; do not merge until green; see lane-rules Shipped in:
|
There was a problem hiding this comment.
3 issues found across 3 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="tests/test_ci_runner_pool_reconcile.py">
<violation number="1" location="tests/test_ci_runner_pool_reconcile.py:59">
P2: The credentialed-job detection misses jobs that pass secrets via `secrets: inherit` or a `secrets:` block, because it only searches for `secrets.` and `environment:`. Also match a job-level `secrets:` key so a new reusable-workflow job falls under the boundary check.</violation>
<violation number="2" location="tests/test_ci_runner_pool_reconcile.py:83">
P2: This substring check also matches other pool keys that start with `linux`, such as `linux_ephemeral`, so a route moved onto the secret-bearing pool would still pass. Anchor the match to the closing brace.</violation>
</file>
<file name=".github/workflows/backend.yml">
<violation number="1" location=".github/workflows/backend.yml:182">
P2: These 11 credentialed jobs ignore any non-allowlisted `LINUX_RUNNER`, including the `ubuntu-24.04` that the overflow switch writes during a Blacksmith outage. They then fall back to `blacksmith-8vcpu-ubuntu-2404` and can queue indefinitely on the stalled pool. If pinning to Blacksmith is intended, document that these jobs are excluded from overflow; otherwise use the repo's `!startsWith(vars.LINUX_RUNNER, 'blacksmith-') && vars.LINUX_RUNNER` form.</violation>
</file>
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
| credentialed = {job for job in declared | ||
| if "secrets." in self._job_block("\n" + jobs_text, job) | ||
| or re.search(r"^ environment:", self._job_block("\n" + jobs_text, job), re.M)} |
There was a problem hiding this comment.
P2: The credentialed-job detection misses jobs that pass secrets via secrets: inherit or a secrets: block, because it only searches for secrets. and environment:. Also match a job-level secrets: key so a new reusable-workflow job falls under the boundary check.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At tests/test_ci_runner_pool_reconcile.py, line 59:
<comment>The credentialed-job detection misses jobs that pass secrets via `secrets: inherit` or a `secrets:` block, because it only searches for `secrets.` and `environment:`. Also match a job-level `secrets:` key so a new reusable-workflow job falls under the boundary check.</comment>
<file context>
@@ -37,6 +50,38 @@ def test_routing_keys_never_follow_the_secretless_pool(self):
+ text = path.read_text()
+ jobs_text = text.split("\njobs:\n", 1)[1]
+ declared = re.findall(r"^ ([A-Za-z0-9_-]+):$", jobs_text, re.M)
+ credentialed = {job for job in declared
+ if "secrets." in self._job_block("\n" + jobs_text, job)
+ or re.search(r"^ environment:", self._job_block("\n" + jobs_text, job), re.M)}
</file context>
| credentialed = {job for job in declared | |
| if "secrets." in self._job_block("\n" + jobs_text, job) | |
| or re.search(r"^ environment:", self._job_block("\n" + jobs_text, job), re.M)} | |
| credentialed = {job for job in declared | |
| if re.search(r"secrets\.|^ secrets:|^ environment:", self._job_block("\n" + jobs_text, job), re.M)} |
| text = path.read_text() | ||
| block = self._job_block(text, "migrations-push-guard") | ||
| route = next(line.strip() for line in block.splitlines() if line.strip().startswith("runs-on:")) | ||
| self.assertIn("fromJSON(vars.RUNNER_POOLS).linux", route) |
There was a problem hiding this comment.
P2: This substring check also matches other pool keys that start with linux, such as linux_ephemeral, so a route moved onto the secret-bearing pool would still pass. Anchor the match to the closing brace.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At tests/test_ci_runner_pool_reconcile.py, line 83:
<comment>This substring check also matches other pool keys that start with `linux`, such as `linux_ephemeral`, so a route moved onto the secret-bearing pool would still pass. Anchor the match to the closing brace.</comment>
<file context>
@@ -37,6 +50,38 @@ def test_routing_keys_never_follow_the_secretless_pool(self):
+ text = path.read_text()
+ block = self._job_block(text, "migrations-push-guard")
+ route = next(line.strip() for line in block.splitlines() if line.strip().startswith("runs-on:"))
+ self.assertIn("fromJSON(vars.RUNNER_POOLS).linux", route)
+
</file context>
| self.assertIn("fromJSON(vars.RUNNER_POOLS).linux", route) | |
| self.assertTrue(route.endswith("fromJSON(vars.RUNNER_POOLS).linux }}")) |
| needs: test | ||
| if: github.event_name == 'push' && github.ref == 'refs/heads/feat-cmux-next' | ||
| runs-on: ${{ github.repository_owner != 'manaflow-ai' && 'ubuntu-24.04' || github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name != github.repository && 'blacksmith-4vcpu-ubuntu-2404' || fromJSON(vars.RUNNER_POOLS).linux }} | ||
| runs-on: ${{ github.repository_owner != 'manaflow-ai' && 'ubuntu-24.04' || github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name != github.repository && 'blacksmith-4vcpu-ubuntu-2404' || contains(fromJSON('["blacksmith-8vcpu-ubuntu-2404","blacksmith-16vcpu-ubuntu-2404","blacksmith-4vcpu-ubuntu-2404"]'), vars.LINUX_RUNNER) && vars.LINUX_RUNNER || 'blacksmith-8vcpu-ubuntu-2404' }} |
There was a problem hiding this comment.
P2: These 11 credentialed jobs ignore any non-allowlisted LINUX_RUNNER, including the ubuntu-24.04 that the overflow switch writes during a Blacksmith outage. They then fall back to blacksmith-8vcpu-ubuntu-2404 and can queue indefinitely on the stalled pool. If pinning to Blacksmith is intended, document that these jobs are excluded from overflow; otherwise use the repo's !startsWith(vars.LINUX_RUNNER, 'blacksmith-') && vars.LINUX_RUNNER form.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At .github/workflows/backend.yml, line 182:
<comment>These 11 credentialed jobs ignore any non-allowlisted `LINUX_RUNNER`, including the `ubuntu-24.04` that the overflow switch writes during a Blacksmith outage. They then fall back to `blacksmith-8vcpu-ubuntu-2404` and can queue indefinitely on the stalled pool. If pinning to Blacksmith is intended, document that these jobs are excluded from overflow; otherwise use the repo's `!startsWith(vars.LINUX_RUNNER, 'blacksmith-') && vars.LINUX_RUNNER` form.</comment>
<file context>
@@ -179,7 +179,7 @@ jobs:
needs: test
if: github.event_name == 'push' && github.ref == 'refs/heads/feat-cmux-next'
- runs-on: ${{ github.repository_owner != 'manaflow-ai' && 'ubuntu-24.04' || github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name != github.repository && 'blacksmith-4vcpu-ubuntu-2404' || fromJSON(vars.RUNNER_POOLS).linux }}
+ runs-on: ${{ github.repository_owner != 'manaflow-ai' && 'ubuntu-24.04' || github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name != github.repository && 'blacksmith-4vcpu-ubuntu-2404' || contains(fromJSON('["blacksmith-8vcpu-ubuntu-2404","blacksmith-16vcpu-ubuntu-2404","blacksmith-4vcpu-ubuntu-2404"]'), vars.LINUX_RUNNER) && vars.LINUX_RUNNER || 'blacksmith-8vcpu-ubuntu-2404' }}
environment: cmux-next-vitess-ci
# A stuck connection to the shared branch must not hold a runner for the 6 h default.
</file context>
|
Review (subagent, independent Codex gpt-6-astra high): SHIP at 7888788. All 11 credentialed backend jobs retain bounded ephemeral routes and fork guards. The secretless migration guard retains its pool route. All 22 integration tests pass; no findings. |
…d-ephemeral-pool-boundary
|
Review (subagent, independent Codex gpt-6-astra high): SHIP at 532297a. The refreshed PR diff remains exactly the same three reviewed files: credentialed backend jobs stay on bounded BS4/8/16 routes, while migrations-push-guard retains the fleet pool. All 22 integration tests pass against the actual main schema; no findings. The base update includes fixture repair #30795. |
The Linux pool is becoming fleet-first, so credentialed backend jobs must retain their existing ephemeral-runner boundary. Follow-up to #30685: 11 jobs now accept only the measured Blacksmith Linux members through the legacy mirror. The secretless migration guard keeps its fleet-capable pool route.
Leo: “FOR OUR NIGHTLIES ARE THEY END TO END ON OUR FLEET? WE CAN'T BE RELYING ON BLACKSMITH THAT CUTS OUT.” This preserves the existing secret and runner-trust boundary while enabling that migration.
The scheduled writer's existing integration test now covers every backend job with secrets or an environment and rejects a newly added credentialed job missing from the boundary. It also verifies that the secretless guard retains its pool route. Workflow environments, secrets, permissions and deployment behavior are unchanged.
Validation: 22 integration tests pass against the actual main policy checkout, including all backend boundaries; workflow YAML and diff checks pass. No compile or cache change. Prior art: #30685, #30360, #30714.
Agent session: agent session
🤖 Generated with Claude Code
Need help on this PR? Tag
@codesmith-botwith what you need.Summary by cubic
Keeps credentialed backend jobs on a bounded set of ephemeral Linux runners instead of the fleet-capable pool route, preserving the existing secret and runner-trust boundary while the Linux pool becomes fleet-first.
runs-onto the ephemeralblacksmith-4vcpu/8vcpu/16vcpu-ubuntu-2404mirror viavars.LINUX_RUNNER, defaulting toblacksmith-8vcpu-ubuntu-2404; PRs from forks keepubuntu-24.04.migrations-push-guardon the fleet-capableRUNNER_POOLS.linuxroute.backend.ymlto theci-fast-guardssparse checkout so those tests validate the actual workflow.Written for commit 532297a. Summary will update on new commits.