feat(runner): isolate structured workflows in one-attempt Kubernetes Pods - #287
Closed
chrisleekr wants to merge 1 commit into
Closed
feat(runner): isolate structured workflows in one-attempt Kubernetes Pods#287chrisleekr wants to merge 1 commit into
chrisleekr wants to merge 1 commit into
Conversation
|
Important Review skippedToo many files! This PR contains 214 files, which is 114 over the limit of 100. To get a review, reduce the PR to 100 files or fewer by splitting it into smaller PRs or changing its base branch. Upgrade to a paid plan to raise the limit. This review couldn't start because sufficient usage credits or metered capacity aren't available. Add credits or update usage-based reviews in the billing tab, then retry. ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Team Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (214)
You can disable this status message by setting the 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. Comment |
chrisleekr
force-pushed
the
feat/isolated-workflow-runner
branch
from
September 1, 2026 10:12
83743b4 to
8807bd5
Compare
chrisleekr
force-pushed
the
feat/isolated-workflow-runner
branch
from
September 1, 2026 10:27
8807bd5 to
f75dafa
Compare
This was referenced Sep 1, 2026
chrisleekr
force-pushed
the
feat/isolated-workflow-runner
branch
from
September 1, 2026 10:46
f75dafa to
a67b143
Compare
chrisleekr
force-pushed
the
feat/isolated-workflow-runner
branch
from
September 1, 2026 10:50
a67b143 to
c701344
Compare
…Pods A `workflow-run` no longer enters the shared-daemon job protocol. One exact attempt owns one Pod and one capability Secret, and every state mutation is fenced by run ID, attempt ID, owner ID, lease, and command receipt. Runner side: - Add `src/runner/` (entry, WebSocket client, workflow executor, output sanitizer, token deadline, process boundary), replacing the deleted `src/daemon/workflow-executor.ts`. - Add `native/daemon-process-guard.c` and `src/daemon/process-boundary.ts` so a worker cannot outlive its attempt. - The runner deny set rejects App, PAT, database, Valkey, Kubernetes, Context7, global GitHub, and daemon-auth credentials. Controller side: - Add `src/orchestrator/workflow-runner-*.ts` (controller, dispatch, payload, store, result, reconciler, resources, capability, output) and `src/k8s/workflow-runner-spawner.ts`, which owns the per-attempt Secret. - Add `src/shared/workflow-runner-messages.ts`: a separate protocol from the shared-daemon schema, deliberately not shared with it. - Add migration `017_workflow_run_leases.sql`, which introduces the `attempt_id` / `offer_id` columns that `src/orchestrator/history.ts` and the daemon-disconnect fencing path query. - The RPC output scanner is stricter than the general one: disablement, failure, or timeout rejects a command and converts a result to a fixed safe failure rather than failing open. Also in this change: - Orchestrator resilience: liveness reaper, queue worker, ws-server split into `ws-connection.ts`, dispatch outbox, completion reconciler, expiry notifier. - The remaining `src/config.ts` surface, including the runner capability secret and namespace validation that only applies once the runner exists. - Workflow handler and ship-rail updates that consume the Gate-2 policy. - Move the last colocated `src/**/*.test.ts` files under `test/` and tighten `check:test-globs` to reject tests in production source. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KUPpJPtxAaHWrBsjytRGyM
chrisleekr
force-pushed
the
feat/isolated-workflow-runner
branch
from
September 1, 2026 12:56
c701344 to
44de644
Compare
Owner
Author
|
Superseded by a three-PR stack. Same change, same final tree, split so each PR is reviewable and under CodeRabbit's 100-file cap.
The stack's tip tree is byte-identical to this PR's head ( |
chrisleekr
added a commit
that referenced
this pull request
Sep 2, 2026
Slice-boundary artifacts introduced by the #287 split: - .gitlab-ci.yml: drop the admission-policy job and its .docker_release `needs:` entry. `bun run test:admission` and the kind harness land with the isolated workflow runner, so on this branch the job could not run and its `needs:` edge would reject pipeline creation on any ref where the rule excluded it. - Revert the `scoped-job:offer` / `scoped-job:completion` colon renames in test/daemon/scoped-offer-evaluator.test.ts, src/daemon/scoped-rebase-executor.ts and the scheduled-actions runbook. The wire schema on this branch still declares the hyphen form, so the test fixture was a type error. - docs/operate/configuration.md: merge the two `## Per-repo config file` sections into one, keeping the deprecated-alias and trim-at-load notes. - CLAUDE.md: tests live under test/ only. scripts/test-isolated.sh globs test/**/*.test.ts, and check:test-globs derives its glob set from that script, so a colocated src/ test now trips the guard. Regressions against main: - src/mcp/servers/inline-comment.ts: claim `pendingLocations` before the commit-SHA fetch and re-read the set after the dedup await. The claim had moved below two suspension points, reopening the duplicate-comment race the set exists to close. - src/utils/bot-identity.ts: restore the `fetchSelfLogin` async helper in place of the inlined `.then(onFulfilled, onRejected)` chain. - test/mcp/servers/inline-comment-dedup.test.ts: restore `SELF` in the three negative cases, which had been weakened to `null` and stopped exercising the author-matching branch. - src/daemon/process-boundary.ts: the docstring claimed to fail daemon startup, but only the image smoke entrypoint calls it on this branch. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KUPpJPtxAaHWrBsjytRGyM
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stack 3 of 3 · base
feat/repo-config-surface(#286) · after #285What this does
Structured workflows previously ran on the shared daemon fleet: a long-lived daemon claimed a job over the fleet WebSocket and executed it in-process via
src/daemon/workflow-executor.ts, alongside other tenants' work, holding fleet-wide credentials. This PR removes that path entirely and gives every structuredworkflow-runits own Kubernetes Pod and its own capability Secret, for exactly one attempt.flowchart TD Disp["dispatcher<br/>workflow-run claimed by controller"]:::ctl Payload["workflow-runner-payload.ts<br/>resolves Gate-2 policy, bounds input"]:::ctl Cap["workflow-runner-capability.ts<br/>mints per-attempt capability Secret"]:::sec Spawn["workflow-runner-spawner.ts<br/>creates ONE bare Pod + Secret"]:::sec Pod["src/runner/main.ts<br/>one attempt, one Pod"]:::pod Guard["process-boundary.ts<br/>+ native/daemon-process-guard.c<br/>worker cannot outlive its attempt"]:::pod Scan["output-sanitizer.ts<br/>STRICT: disable, fail or timeout<br/>rejects the command"]:::stop Result["workflow-runner-result.ts<br/>terminal payload stored<br/>BEFORE projections and ACK"]:::ctl Recon["workflow-runner-reconciler.ts<br/>+ liveness reaper, expiry notifier"]:::ctl Clean["workflow-runner-resources.ts<br/>Pod + Secret torn down"]:::sec Fence["fenced by run id, attempt id,<br/>owner id, lease, command receipt"]:::gate Disp --> Payload --> Cap --> Spawn --> Pod Pod --> Guard Pod -->|"every RPC command"| Scan Scan --> Result --> Recon --> Clean Fence -.->|"gates every mutation"| Result classDef ctl fill:#2c3e50,color:#ffffff classDef sec fill:#8e44ad,color:#ffffff classDef pod fill:#1e8449,color:#ffffff classDef stop fill:#c0392b,color:#ffffff classDef gate fill:#ecf0f1,color:#2c3e50The isolation contract
A
workflow-runmust never enter the shared-daemon job protocol.src/shared/workflow-runner-messages.tsis a deliberately separate schema fromws-messages.ts, not a superset of it, so the two protocols cannot drift into each other by accident. Legacy and scoped jobs stay on the shared fleet unchanged.The runner's deny set rejects App credentials, PAT, database URL, Valkey URL, Kubernetes config, Context7 key, global GitHub token, and daemon-auth tokens. What the Pod gets instead is a short-lived App installation token scoped to its one target repository. This is also why structured dispatch fails closed in PAT mode: a PAT cannot be narrowed to one repo.
Stricter output scanning than the general path
The general GitHub-output scanner in
safePostToGitHubis fail-open: a provider outage falls back to the regex pass rather than blocking a comment. The runner RPC scanner inverts that. Scanner disablement, failure, or timeout rejects the command and converts a result into a fixed safe failure, loggingworkflow_runner_output_scan_unavailableaterror. The runner handles attacker-influenced repository content with a repo-scoped write token, so silently degrading to regex-only there is not an acceptable trade.Ordering that matters
The terminal payload is stored before projections and before the ACK. A crash between store and ACK replays into an idempotent projection; the reverse order would lose the result outright.
Also in this PR
ws-serversplit intows-connection.ts, dispatch outbox, completion reconciler, expiry notifier017workflow_run_leases— addsattempt_id,offer_id, lease/deadline columns and theworkflow_attempt_commandstablesrc/config.tssrc/**/*.test.tsfiles undertest/and tightenscheck:test-globsto reject tests in production sourceVerification
typecheck·lint·formatcheck:*guardstestmainbaseline is 137New CI job
admission-policyvalidates the runner admission spec against Kubernetes 1.30 viabun run test:admission.Review notes
src/orchestrator/history.tsand its two tests moved here from feat(core): extract HTTP router and add output-safety primitives #285 during review: they queryattempt_id/offer_id, so they must land in the same PR as migration017. Without that they would abort at runtime, not merely under test.bunfig.tomldrops the global 30s test timeout, which the runner tests exceed.🤖 Generated with Claude Code
https://claude.ai/code/session_01KUPpJPtxAaHWrBsjytRGyM