feat(ci): execute workflow steps in a sibling container, not the app #5595
ccantynzAI Reviewcommented 23h ago
AI Triage
(no summary)
Priority: medium Risk area: mixed
Suggested labels: (no label suggestions) Suggested reviewers: (no reviewer suggestions)
Suggestions only — nothing has been applied. The PR author stays in control.
gluecron[bot]🤖 botAI Reviewcommented 23h ago
AI review unavailable
The platform's AI balance is exhausted, so AI generation is temporarily unavailable. Nothing was lost. You can queue this as a repair for the internal agent from the repository's Health page, or try again once the balance is restored. The PR is otherwise unchanged.
Cross-repo impact
See what breaks downstream if this PR merges.
⮌ Merged
This pull request was merged into main.
c comment · e edit title · m merge · a approve · r request changes · ? shortcuts
A workflow
run:step has been aBun.spawninside the app container. Three problems follow from that one fact, and none is a bug that can be fixed in place:tscon 2026-08-29 and took CI down platform-wide. The fix was raising the app's ceiling to 6g — which the compose file itself records as a stopgap..env.buildRunnerEnvwithholds those variables from the step's own env;/proc/1/environhands them straight back./data/reposis mounted read-write.What this does
A sibling
runnercontainer: same image, different entrypoint, deliberately impoverished — noenv_file, no repo mount, no database import. The app clones into a sharedci-workvolume (it has the repositories and the credentials) and the runner only ever sees that directory.The guarantee is structural rather than a denylist: the runner cannot leak
DATABASE_URLbecause its process never had it, and cannot reach a repository because no such path is mounted.Off by default
RUNNER_URLandRUNNER_TOKENare both empty. A URL without a token counts as no runner, so a half-filled.envfalls back to the working in-process path instead of routing every build on the platform to a container that answers 401. The runner comes up alongside and can be verified before anything routes to it.What this does NOT close
Said plainly, because an isolation story that overstates itself gets trusted:
workflow-exec-policy.tstherefore stays, with its header rewritten to say exactly why and what must be true before it goes.ci-workare untouched.This is what makes CI safe to offer the first customer. Per-job ephemeral containers are what make it safe to offer the tenth.
The trap worth naming
requeueRunsOrphanedByRestartassumed the executing process dies when the app does. With a sibling runner it does not — a redeploy would leave a step running while the sweep marked the runrunner_restartedand enqueued a second attempt: two executions racing over one checkout. Both sweeps now ask the runner what it is actually executing. An unreachable runner reports nothing live, which is today's behaviour and the correct failure direction.Tests
They assert absences, which is unusual and deliberate: the value here is entirely in what the runner does not have, and each property dies to one well-meaning line.
Mutation-verified — adding
env_file, aports:mapping, agit-reposmount, or animport { db }each fails a specific test. The first draft passed against its own subject because it was reading the comments that explain the rules; it strips comments now.Verification before enabling
Design and rollout steps:
docs/CI-ISOLATION-NEXT-INCREMENT.md. The acceptance test is run inside a live step, not against the compose file: