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.

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.

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 334-347: Add a conformance test that verifies the runner bridge
for each supported RUNNER_TYPE is present and loadable in custom images: for
each RUNNER_TYPE, start a session using a custom image lacking that bridge and
assert the pod logs contain an error naming the missing bridge module, the pod
exits non-zero, and the control plane marks the session Failed; update the
conformance checklist/spec sections referencing RUNNER_TYPE and the "Bridge
Mismatch" requirement so the suite explicitly covers bridge
availability/loadability and log assertion for missing modules.
- Line 86: Update the requirement that "/app MUST exist; writeable by UID 1001"
to be compatible with arbitrary non-root UIDs: replace the hardcoded "UID 1001"
reference with wording that /app must be writeable by the container's effective
non-root UID (for example "writeable by the container's effective non-root UID
or a group writable location when running under an arbitrary UID such as
OpenShift SCC"), and apply the same change to the related statements referenced
around lines 164-167 so the spec consistently allows arbitrary non-root UIDs
instead of a fixed UID.

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 178-323: The spec adds ProjectSettings.runner_image and
runner_image_pull_secret without gating; add a new Requirement that these fields
are gated by the Unleash flag custom-runner-image: when disabled, PATCH/PUT
attempts to set runner_image or runner_image_pull_secret must be rejected with a
descriptive error (403/validation), and session provisioning logic (where image
selection occurs) must ignore ProjectSettings.runner_image and fall back to
normal precedence; reference ProjectSettings.runner_image,
ProjectSettings.runner_image_pull_secret, the session creation/path that selects
runner image, and the flag name custom-runner-image in the new paragraph.
- Around line 13-21: Add an "Implementation Impact" section to the spec that
enumerates all components that must change for ProjectSettings runner image
support and point to relevant files: list Backend (ProjectSettings handler
including validation, RBAC, registry allowlist check — reference
internal/api/project_settings.go), Operator (image selection precedence and pod
spec generation — reference pkg/operator/session_controller.go), CLI (acpctl
commands to set/get runner_image and runner_image_pull_secret), SDKs (client
library updates for new ProjectSettings fields), and Frontend (UI for workspace
admins to configure custom images); for each bullet include one-line notes on
what to update (e.g., validation rules, RBAC checks, precedence logic, new CLI
flags/commands, SDK model fields, UI form and permissions).
- Around line 454-467: Under the "Requirement: Conformance Test Suite" section
add explicit metadata: declare a repository location (e.g.,
scripts/runner-conformance/ or tests/conformance/runner/) and the exact
command/Make target to run the suite (e.g., make test-conformance
IMAGE=<custom-image>), name the owning team (e.g., platform-agents) responsible
for maintenance and updates, describe the CI integration (e.g., a GitHub Actions
workflow that runs on base-image builds and pull requests touching runner
images), and specify the shipping milestone (e.g., "must ship with the first
base image public release"); place this text directly beneath the "Requirement:
Conformance Test Suite" heading so the spec explicitly covers location,
ownership, CI workflow, and delivery timing.
- Around line 180-184: The spec adds ProjectSettings fields `runner_image` and
`runner_image_pull_secret` but omits the API surface; update the document by
either adding a cross-reference to the ProjectSettings API in
control-plane.spec.md or ambient-model.spec.md or by embedding an explicit API
contract: declare the endpoint (e.g., PATCH /api/projects/{project}/settings),
the JSON payload keys (`runner_image`, `runner_image_pull_secret`), validation
rules (400 for invalid image/ref syntax, 403 for disallowed registry, 404 for
missing secret), and expected responses (200 on success, 400/403/404 as above),
and ensure the ProjectSettings resource definition includes these two string
fields so implementers can locate `runner_image` and `runner_image_pull_secret`.
- Line 9: Update the incorrect relative link string in runner-image.spec.md:
replace the reference `../security/security.spec.md` with the correct path
`../standards/security/security.spec.md` so the spec in specs/agents points to
specs/standards/security/security.spec.md.