feat(specs): add custom runner image specification#1563
Open
jbpratt wants to merge 7 commits into
Open
Conversation
✅ Deploy Preview for cheerful-kitten-f556a0 canceled.
|
Contributor
📝 WalkthroughWalkthroughAdds a stable "custom runner image" specification describing required AG-UI HTTP endpoints, Python/runtime and filesystem constraints, non-root and exec/signal expectations, ProjectSettings-based runner image override with validation and pull-secret support, failure modes, security assumptions, and base-image contract labeling. ChangesRunner image contract & ProjectSettings override
🚥 Pre-merge checks | ✅ 8✅ Passed checks (8 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
specs/agents/runner-image.spec.md (2)
93-93: 💤 Low valueClarify path description to avoid confusion.
The phrase "MUST contain installed
ambient_runnerpackage" could be misread to mean the pip package must be installed at/app/ambient-runner, when it actually means this directory contains the application code (main.py) that imports the package installed elsewhere in site-packages.📝 Clearer phrasing
-| `/app/ambient-runner` | Runner package source and working directory | MUST contain installed `ambient_runner` package | +| `/app/ambient-runner` | Runner application root and working directory | MUST contain main.py and application code; requires `ambient_runner` package installed via pip |🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@specs/agents/runner-image.spec.md` at line 93, The spec line for `/app/ambient-runner` is ambiguous about where the pip-installed ambient_runner resides; update the wording so it clearly states that `/app/ambient-runner` contains the application source (e.g., main.py) which imports the `ambient_runner` package installed in site-packages, not that the pip package itself is installed at that path; reference the `/app/ambient-runner` directory, the application entrypoint `main.py`, and the `ambient_runner` package in the revised sentence to make this distinction explicit.
461-461: ⚡ Quick winConsider blocking contract version mismatches by default.
The spec makes version checking advisory-only (CP logs warning but creates pod anyway). However, if a custom image uses contract v2 with breaking changes and the CP expects v1, the session will fail unpredictably at runtime rather than being rejected upfront.
💡 Alternative design
Make blocking the default with operator opt-in for mismatches:
-The CP MAY read this label at pod creation time and log a warning if the contract version does not match the expected version. This is advisory — the CP SHALL NOT block pod creation based on contract version mismatch. +The CP SHALL read this label at pod creation time. If the contract version does not match the expected version, the CP SHALL transition the session to `Failed` with a condition describing the mismatch UNLESS the operator has set `ALLOW_CONTRACT_VERSION_MISMATCH=true`.This preserves flexibility for operators who explicitly opt in while preventing accidental incompatibilities.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@specs/agents/runner-image.spec.md` at line 461, Update the sentence about contract-version handling so the Control Plane (CP) SHALL by default reject pod creation on a contract version mismatch instead of merely warning; add a clear operator-configurable override (e.g., an "allowContractMismatch" opt-in flag) that, when enabled, permits the previous advisory behavior and logs a warning; ensure the wording references the "contract version" label and the CP's behavior at "pod creation" so readers can locate and implement the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@specs/agents/runner-image.spec.md`:
- Around line 274-276: Document that ProjectSettings.runner_image can override
image but not agent-type config (RUNNER_TYPE, resource limits, state dir) and
add a Failure Modes entry describing the cryptic Python import error when a
custom image lacks the required bridge implementation (e.g., ClaudeBridge,
GeminiCLIBridge, LangGraphBridge) for the session's runner type; update the
recommendations to advise building custom images FROM the standard base to
inherit all bridges and add a runtime validation step in the session creation
flow (where ProjectSettings.runner_image is applied) that inspects the image or
performs a quick probe to confirm the presence of the required bridge for the
requested RUNNER_TYPE and surface a clear, actionable error if missing.
---
Nitpick comments:
In `@specs/agents/runner-image.spec.md`:
- Line 93: The spec line for `/app/ambient-runner` is ambiguous about where the
pip-installed ambient_runner resides; update the wording so it clearly states
that `/app/ambient-runner` contains the application source (e.g., main.py) which
imports the `ambient_runner` package installed in site-packages, not that the
pip package itself is installed at that path; reference the
`/app/ambient-runner` directory, the application entrypoint `main.py`, and the
`ambient_runner` package in the revised sentence to make this distinction
explicit.
- Line 461: Update the sentence about contract-version handling so the Control
Plane (CP) SHALL by default reject pod creation on a contract version mismatch
instead of merely warning; add a clear operator-configurable override (e.g., an
"allowContractMismatch" opt-in flag) that, when enabled, permits the previous
advisory behavior and logs a warning; ensure the wording references the
"contract version" label and the CP's behavior at "pod creation" so readers can
locate and implement the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 34cd54f5-c174-486c-a499-0113c9af9cf5
📒 Files selected for processing (1)
specs/agents/runner-image.spec.md
Define the stable runner contract and a ProjectSettings-driven image override so workspace admins can layer tools onto the base runner via Dockerfile FROM. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
jbpratt
force-pushed
the
spec/custom-runner-image
branch
from
b49e8eb to
2308ab4
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@specs/agents/runner-image.spec.md`:
- Around line 154-164: The spec enforces a contradictory UID requirement: it
mandates a fixed UID 1001 via Dockerfile `USER 1001` while also recommending
OpenShift arbitrary-UID compatibility (e.g. `chmod -R g=u`), which conflicts
under restrictive SCCs; change the normative contract to require non-root
runtime behavior (`runAsNonRoot: true`, `allowPrivilegeEscalation: false`,
`drop: ["ALL"]` and no root at runtime) and demote `UID 1001`/`Dockerfile USER
1001` to a base-image default or recommendation, keeping the OpenShift
compatibility guidance (`chmod -R g=u` on writable paths) as a SHOULD rather
than a SHALL so implementations can satisfy `runAsNonRoot` without a fixed UID.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 7fd1e84c-8a85-4262-a291-b54a4719e4c9
📒 Files selected for processing (1)
specs/agents/runner-image.spec.md
Comment on lines
+154
to
+164
| A custom runner image SHALL run as UID 1001 with no root privileges. | ||
|
|
||
| | Constraint | Enforced by | | ||
| |------------|-------------| | ||
| | UID 1001 | Dockerfile `USER 1001` | | ||
| | `runAsNonRoot: true` | Pod SecurityContext | | ||
| | `allowPrivilegeEscalation: false` | Pod SecurityContext | | ||
| | `drop: ["ALL"]` capabilities | Pod SecurityContext | | ||
|
|
||
| Custom images MAY use `USER 0` during build stages for installing system packages, provided the final `USER` directive sets UID 1001. Custom images SHOULD include OpenShift arbitrary-UID compatibility (`chmod -R g=u` on writeable paths). | ||
|
|
Contributor
There was a problem hiding this comment.
Resolve UID contract contradiction for OpenShift compatibility
Line 154 mandates a fixed UID (1001), but Line 163 simultaneously recommends OpenShift arbitrary-UID compatibility. These are mutually inconsistent as a normative contract and can lead to incompatible implementations under restricted SCC.
Use a non-root contract as normative (runAsNonRoot, no privilege escalation, dropped caps), and make 1001 a base-image default rather than a hard runtime requirement.
Proposed spec wording change
-A custom runner image SHALL run as UID 1001 with no root privileges.
+A custom runner image SHALL run as non-root with no root privileges. The base image default runtime user is UID 1001, but deployments MAY run with an arbitrary non-root UID (e.g., OpenShift restricted SCC).
| Constraint | Enforced by |
|------------|-------------|
-| UID 1001 | Dockerfile `USER 1001` |
+| Non-root runtime user (default: UID 1001) | Dockerfile `USER 1001` + Pod SecurityContext |
| `runAsNonRoot: true` | Pod SecurityContext |
| `allowPrivilegeEscalation: false` | Pod SecurityContext |
| `drop: ["ALL"]` capabilities | Pod SecurityContext |🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@specs/agents/runner-image.spec.md` around lines 154 - 164, The spec enforces
a contradictory UID requirement: it mandates a fixed UID 1001 via Dockerfile
`USER 1001` while also recommending OpenShift arbitrary-UID compatibility (e.g.
`chmod -R g=u`), which conflicts under restrictive SCCs; change the normative
contract to require non-root runtime behavior (`runAsNonRoot: true`,
`allowPrivilegeEscalation: false`, `drop: ["ALL"]` and no root at runtime) and
demote `UID 1001`/`Dockerfile USER 1001` to a base-image default or
recommendation, keeping the OpenShift compatibility guidance (`chmod -R g=u` on
writable paths) as a SHOULD rather than a SHALL so implementations can satisfy
`runAsNonRoot` without a fixed UID.
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.
Summary
specs/agents/runner-image.spec.mddefining the stable runner contract and a workspace-level custom image overrideDockerfile FROMon a published base image — no init hooksrunner_imageandrunner_image_pull_secretfields on ProjectSettings let workspace admins configure a custom runner per projectDetails
The spec establishes the boundary between "what the platform guarantees" and "what custom images can change." Key design decisions:
Test plan
runner.spec.mdandcontrol-plane.spec.md🤖 Generated with Claude Code
Summary by CodeRabbit
runner_imageandrunner_image_pull_secretwith precedence, validation, pull-policy rules, RBAC guidance, failure modes, and security/isolation expectations.