refactor(runtime): register pi as a stub runtime

[waynesun09]

Summary

Registers pi (earendil-works/pi) as a stub runtime — step 1 of the pi runtime plan (#6464), mirroring the OpenCode stub precedent (#6035). The stub is resolvable via runtime.Resolve() for dev/testing but deliberately not in ValidRuntimes() (a selectable stub would burn pre-script side effects + a sandbox before failing), and the pi CLI is version-pinned into the sandbox image so subsequent fixture-recording and Bootstrap/Run work runs against a reviewed version.

Related Issue

Refs #6464

Changes

• internal/runtime/pi.go — PiRuntime implementing Runtime + TranscriptHandler as explicit not-implemented stubs. System() = "pi" (multi-provider, same rationale as OpenCode's merged precedent). ConfigDir() = /sandbox/pi-config (new sandbox.SandboxPiConfig) — runner-owned, outside the agent-writable workspace, applying the lesson flagged on the OpenCode stub's ConfigDir comment from day one. EnvExports() pins PI_CODING_AGENT_DIR/PI_CODING_AGENT_SESSION_DIR there and sets PI_OFFLINE=1 + PI_SKIP_VERSION_CHECK=1 (no startup network traffic; does not affect inference).
• internal/runtime/registry.go — case "pi"; ValidRuntimes() untouched.
• images/sandbox/Containerfile — ARG PI_VERSION=0.84.2 + npm install -g --ignore-scripts @earendil-works/pi-coding-agent@${PI_VERSION} (upstream-recommended install; package ships npm-shrinkwrap.json); renovate.json customManagers regex tracks the ARG. The comment records that pin bumps must re-verify the upcoming stream-parser fixtures — pi changed its --mode json wire shape within 0.84.
• docs/runtimes.md — registered-runtimes row + pi (stub) column (all N/A — stub) in the security feature matrix and config-key support matrix.
• Tests — pi_test.go (metadata incl. config-dir-outside-workspace invariant, env exports, not-implemented errors, no-ops, capability defaults: no CLAUDE.md bridge, default debug-log name); registry + config tests extended with pi resolvable-but-not-selectable cases.

Deliberately deferred to the Bootstrap/Run PR: the anthropic-vertex provider pin (upstream earendil-works/pi#5262 still open; community extension unchanged since July; and #6445 may change the image-install mechanism), the pi --mode json stream parser, and any runner wiring.

Note: the docs/runtimes.md matrix edits will need a trivial rebase if #6405 (which rewrites the Claude column cells) merges first — I'll rebase this branch when it lands.

Testing

• make lint passes (staged first)
• go test ./internal/{runtime,config,sandbox}/... green; pi.go functions at 100% coverage; registry/config stub-bypass cases covered
• python -c "json.load(open('renovate.json'))" — renovate config valid JSON; regex mirrors the CLAUDE_CODE_VERSION manager
• Verified @earendil-works/pi-coding-agent@0.84.2 is the current npm latest

Checklist

• PR title follows Conventional Commits (refactor(runtime) — internal registration, no user-visible capability; same type as the OpenCode stub PR refactor(runtime): register opencode as a stub runtime #6035)
• Commits are signed off (DCO) — human-directed session
• I wrote this contribution myself and can explain all changes in it

[qodo-code-review]

PR Summary by Qodo

Register pi as a stub runtime backend (resolvable, not user-selectable)

✨ Enhancement 🧪 Tests 📝 Documentation ⚙️ Configuration changes 🕐 40+ Minutes

AI Description

• Add a stub PiRuntime backend resolvable via runtime.Resolve("pi") for dev/testing.
• Keep pi out of config.ValidRuntimes() to avoid exposing a non-functional runtime.
• Pin the pi CLI into the sandbox image and document/runtime-matrix coverage with tests.

Diagram

graph TD
  caller["Runner / CLI"] --> resolve["runtime.Resolve()"] --> pi["PiRuntime (stub)"] --> env["EnvExports + ConfigDir"] --> cfg["/sandbox/pi-config"]
  image[("Sandbox image")] --> cli["pi CLI (pinned)"]
  pi -. "future: Bootstrap/Run" .-> cli
  docs["docs/runtimes.md"] --> matrices["Runtime matrices"]

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Expose pi via ValidRuntimes() behind an 'experimental' guard

• ➕ Allows selection via existing user-facing runtime selection paths
• ➕ Enables end-to-end wiring earlier (even if partially implemented)
• ➖ Still risks burning pre-script side effects + sandbox time before failing
• ➖ Requires more UX/CLI/docs surface area changes to communicate experimental semantics

2. Omit pi CLI from the sandbox image until Bootstrap/Run lands

• ➕ Reduces supply-chain surface area until the runtime is usable
• ➕ Avoids pin maintenance until fixtures/stream parsing are ready
• ➖ Blocks upcoming fixture recording / parser work from running against a reviewed, pinned binary
• ➖ Encourages ad-hoc installs that are harder to audit/reproduce

3. Gate pi runtime registration behind a build tag or dev-only flag

• ➕ Makes it impossible to accidentally resolve in production builds
• ➕ Keeps registry cleaner for production
• ➖ Diverges behavior between dev and CI/production builds
• ➖ Adds complexity to testing and to future rollout when enabling pi generally

Recommendation: Current approach (resolvable stub, not in ValidRuntimes(), pinned CLI in the sandbox image) is the best fit for incremental rollout: it mirrors the established OpenCode precedent, avoids exposing a non-functional runtime to user selection paths, and ensures future stream-parser/fixture work runs against a reproducible, reviewed pi CLI version.

Files changed (9) +229 / -23

Enhancement (3) +94 / -0

pi.goAdd PiRuntime stub implementing Runtime and TranscriptHandler +87/-0

Add PiRuntime stub implementing Runtime and TranscriptHandler

• Introduces 'PiRuntime' with runtime metadata ('Name', 'System', 'WorkspaceDir', 'ConfigDir') and sandbox-owned env exports for pi config/session directories and offline/version-check suppression. 'Bootstrap', 'Run', and transcript extraction return explicit not-implemented errors; parsing/emission methods are no-ops where appropriate.

internal/runtime/pi.go

registry.goRegister pi in runtime.Resolve() +3/-0

Register pi in runtime.Resolve()

• Adds a 'case "pi"' branch returning a Backend with 'PiRuntime' as both the Runtime and TranscriptHandler, making it resolvable for internal use.

internal/runtime/registry.go

sandbox.goAdd sandbox constant for pi config directory +4/-0

Add sandbox constant for pi config directory

• Introduces 'SandboxPiConfig = "/sandbox/pi-config"' with comments emphasizing it must remain outside the agent-writable workspace to prevent runtime config tampering.

internal/sandbox/sandbox.go

Tests (3) +86 / -0

config_test.goAssert pi is not user-selectable via ValidRuntimes() +1/-0

Assert pi is not user-selectable via ValidRuntimes()

• Extends the ValidRuntimes unit test to ensure 'pi' (like 'opencode') is excluded, preventing selection through user-facing runtime configuration until implemented.

internal/config/config_test.go

pi_test.goAdd tests for PiRuntime stub behavior and invariants +72/-0

Add tests for PiRuntime stub behavior and invariants

• Adds unit tests covering runtime metadata, env exports, config-dir-outside-workspace invariant, not-implemented failures for Bootstrap/Run/extraction, no-op transcript parsers, and default capability behavior (no CLAUDE.md bridge, default debug log name).

internal/runtime/pi_test.go

registry_test.goTest resolving pi directly and via per-repo config +13/-0

Test resolving pi directly and via per-repo config

• Extends registry tests to validate 'Resolve("pi")' returns a pi backend and that per-repo config can set 'runtime=pi' even though it is not in 'ValidRuntimes()'.

internal/runtime/registry_test.go

Documentation (1) +24 / -23

runtimes.mdDocument pi as a registered stub runtime across matrices +24/-23

Document pi as a registered stub runtime across matrices

• Adds 'pi' to the registered runtimes table and introduces a 'pi (stub)' column in the security feature matrix and harness-key mapping matrix. Marks all pi entries as N/A or stub to reflect non-functional Bootstrap/Run and hook wiring.

docs/runtimes.md

Other (2) +25 / -0

ContainerfileInstall pinned pi CLI into the sandbox image +15/-0

Install pinned pi CLI into the sandbox image

• Adds 'ARG PI_VERSION=0.84.2' and installs '@earendil-works/pi-coding-agent' with '--ignore-scripts'. Includes guidance noting the pin must be re-verified against future pi JSON streaming/fixture expectations when bumped.

images/sandbox/Containerfile

renovate.jsonTrack PI_VERSION pin via Renovate custom regex manager +10/-0

Track PI_VERSION pin via Renovate custom regex manager

• Adds a renovate 'customManagers' regex entry to detect and update the 'PI_VERSION' ARG in the sandbox Containerfile. Documents that bumps require re-verifying pi stream-parser fixtures due to historical wire-shape churn.

renovate.json

[github-actions]

Site preview

Preview: https://10b028a0-site.fullsend-ai.workers.dev

Commit: 6d3e6fe66d1c505511775f3236866ce4a55c52dd

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 9:17 PM UTC · Completed 9:33 PM UTC

Commit: a793653 · View workflow run →

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

1. Stub runtime selectable via config ✗ Dismissed 🐞 Bug ≡ Correctness

Description

Adding case "pi" to runtime.Resolve() makes runtime: pi in per-repo config resolve
successfully, even though ValidRuntimes() excludes it. Because config parsing for runtime
selection does not validate runtimes, a pi selection will proceed into sandbox bootstrap steps
before failing, contradicting the PR’s “not user-selectable” guarantee and wasting/side-effecting
runs.

Code

internal/runtime/registry.go[R23-25]

+	case "pi":
+		r := PiRuntime{}
+		return Backend{Runtime: r, Transcripts: r}, nil

Relevance

●●● Strong

Matches accepted precedent: stub runtimes must fail before sandbox bootstrap, not just at Run().

PR-#6035

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

pi becomes selectable because the CLI uses per-repo config parsing that does not validate
runtime, then resolves via ResolveFromPerRepoConfig → Resolve, which now includes a pi case;
the run then performs sandbox bootstrap work prior to the runtime stub failing.

internal/runtime/registry.go[10-28]
internal/cli/run.go[167-187]
internal/config/config.go[685-693]
internal/config/config.go[192-195]
internal/cli/run.go[1231-1254]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`pi` is intended to be resolvable for dev/testing but *not selectable* via config (`ValidRuntimes()` excludes it). However, the CLI’s runtime selection path parses per-repo config without calling `Validate()`, and then calls `runtime.ResolveFromPerRepoConfig()` → `runtime.Resolve()`. By adding `case "pi"` in `Resolve()`, `runtime: pi` in config becomes a valid selection and the runner will still create/prepare a sandbox before failing.

## Issue Context
- `config.ParsePerRepoConfig()` does not call `Validate()`, so `runtime:` values are not checked against `config.ValidRuntimes()` during runtime selection.
- `internal/cli/run.go` proceeds through sandbox bootstrap steps (common bootstrap + env wiring) before invoking `rt.Bootstrap()`, so selecting a stub runtime still burns work/side effects.

## Fix Focus Areas
- Enforce the “not selectable” rule at the config-resolution boundary:
 - internal/runtime/registry.go[31-47]
 - internal/cli/run.go[167-187]
- Ensure only `config.ValidRuntimes()` are allowed when selecting from org/per-repo config (while still allowing `runtime.Resolve("pi")` for explicit dev/testing codepaths):
 - internal/config/config.go[192-195]
 - internal/config/config.go[685-693]

### Suggested implementation direction
- In `runtime.ResolveFromConfig` and `runtime.ResolveFromPerRepoConfig`, reject any runtime not in `config.ValidRuntimes()` with a clear error (e.g., “runtime %q is not user-selectable; must be one of …”).
- Update/replace the newly-added tests that assert `pi` is resolvable via `ResolveFromPerRepoConfig`; instead assert it fails with the expected error.
- (Optional) If you still need config-based selection for internal testing, gate it behind an explicit opt-in (env var/flag) so default behavior remains non-selectable.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Context sources

✅ Compliance rules (platform): 58 rules

✅ Skills: writing-user-docs, writing-adrs

Tip of the day

💡 Did you know, you can commit Qodo's fix in one click with committable suggestions (GitHub & GitLab)

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

[fullsend-ai-review]

Review

Findings

Medium

• [protected-path] images/sandbox/Containerfile — This PR modifies a file under the protected path images/. The change adds pi CLI installation to the sandbox image, authorized by linked issue Track pi (earendil-works/pi) as a supported agent runtime #6464 (acceptance criterion 2: "pi CLI version-pinned into the sandbox image"). Human approval is always required for protected-path changes, regardless of context.

• [stale-reference] images/README.md:19 — The fullsend-sandbox image description lists pre-installed tools (Claude Code, jq, acli, gitleaks, tirith, pre-commit, gitlint, DeBERTa model) but does not mention pi, which is now installed via PI_VERSION ARG in the Containerfile.
  Remediation: Add 'pi' to the image description.

• [stale-reference] images/README.md:136 — The supply chain security table documents Claude Code's CLAUDE_CODE_VERSION ARG and npm version pin but does not include a corresponding row for pi (PI_VERSION ARG + @earendil-works/pi-coding-agent).
  Remediation: Add a pi row after the Claude Code row.

• [stale-reference] docs/contributing/sandbox-topology.md:15 — The "Key additions over parent" column for fullsend-sandbox lists "Claude Code, jq, gitleaks, acli, pre-commit, gitlint, tirith, ProtectAI DeBERTa model" but does not mention pi, which is now pre-installed in the sandbox image.
  Remediation: Add 'pi' to the key additions list.

Low

• [comment-style] internal/sandbox/sandbox.go:29 — SandboxPiConfig doc comment omits the trailing period that sibling constants SandboxWorkspace and SandboxClaudeConfig both use.
  Remediation: Change to // SandboxPiConfig is the pi config directory inside the sandbox.

Previous run

Review

Findings

Medium

• [protected-path] images/sandbox/Containerfile — This PR modifies a file under the protected images/ path. The change is justified by linked issue Track pi (earendil-works/pi) as a supported agent runtime #6464 (acceptance criterion: "pi CLI version-pinned into sandbox image") and the PR description explains the rationale. Human approval is always required for protected-path changes, regardless of context.

• [stale-enumeration] docs/contributing/sandbox-topology.md:15 — The "Key additions over parent" column for the fullsend-sandbox image lists Claude Code, jq, gitleaks, acli, pre-commit, gitlint, tirith, and ProtectAI DeBERTa model but does not mention pi (@earendil-works/pi-coding-agent), which this PR adds to the Containerfile. The table should reflect the updated image contents.

Low

• [API-shape-consistency] internal/runtime/pi.go:45 — EnvExports() uses raw string concatenation with sandbox.SandboxPiConfig instead of calling r.ConfigDir() via fmt.Sprintf like ClaudeRuntime.EnvExports(). This creates a second reference to the config-path constant; if the path changes, EnvExports() needs a separate update.

• [test-robustness] internal/runtime/pi_test.go:39 — TestPiRuntimeRun_NotImplemented passes ui.New(nil) (a non-nil Printer wrapping a nil Writer), while the equivalent OpenCode test passes nil directly. The stub returns immediately so this is safe today, but ui.New(nil) is a latent fragility if logging is added before the error return.

• [scope-boundary] images/sandbox/Containerfile — The Containerfile bakes PI_CODING_AGENT_DIR, PI_CODING_AGENT_SESSION_DIR, PI_OFFLINE, and PI_SKIP_VERSION_CHECK as image-level ENV vars, duplicating what PiRuntime.EnvExports() sets at runtime. The comment explains the rationale (defense-in-depth for ad-hoc Bash invocations of pi).

Previous run (2)

Review

Findings

Medium

• [protected-path] images/sandbox/Containerfile — This PR modifies images/sandbox/Containerfile, which is under the protected paths images/ and Containerfile. The PR links to issue Track pi (earendil-works/pi) as a supported agent runtime #6464 and explains the rationale (installing the pi CLI for fixture recording and upcoming Bootstrap/Run work). Human approval is always required for protected-path changes, regardless of context.

• [stale-enumeration] docs/contributing/sandbox-topology.md:15 — The sandbox image contents table lists "Claude Code, jq, gitleaks, acli, pre-commit, gitlint, tirith, ProtectAI DeBERTa model" as the key additions in fullsend-sandbox, but the Containerfile now also installs @earendil-works/pi-coding-agent (the pi CLI). Consider adding the pi CLI to the "Key additions over parent" column.

Low

• [supply-chain-integrity] images/sandbox/Containerfile — The pi CLI package (@earendil-works/pi-coding-agent) is installed with a version pin but no SHA256 checksum verification, unlike the checksum-verified binary installs (gitleaks, tirith, acli, ONNX Runtime, ProtectAI) elsewhere in the Containerfile. This follows the same pattern as the Claude Code npm install, where npm's own registry integrity checks provide the baseline verification.

• [test-adequacy] internal/config/config_test.go:411 — The PR adds a ValidRuntimes() exclusion assertion for "pi" but does not add explicit rejection tests in TestOrgConfigValidateRuntime or TestPerRepoConfigValidate_Runtime (unlike the opencode precedent which has both). The existing generic "invalid" test cases cover the rejection path, but explicit tests serve as documentation of the deliberate design choice.

Previous run (3)

Review

Findings

Medium

• [protected-path] images/sandbox/Containerfile — This PR modifies files under protected paths (images/sandbox/Containerfile matches images/ and Containerfile). The PR links to issue Track pi (earendil-works/pi) as a supported agent runtime #6464 and explains the rationale (installing pi CLI in the sandbox image for fixture recording and upcoming Bootstrap/Run work). Human approval is always required for protected-path changes, regardless of context.

Low

• [supply-chain] images/sandbox/Containerfile — The @earendil-works/pi-coding-agent npm package is installed without SHA256 checksum verification, unlike other binary tools in the Containerfile (gitleaks, tirith, ONNX Runtime, acli). This matches the existing Claude Code npm install pattern (also without checksums), so it is a pre-existing pattern rather than a regression. npm's registry-level integrity checking and --ignore-scripts partially compensate.

• [stale-code-reference] docs/guides/dev/cli-internals.md:517 — The "Sandbox Constants" code block lists SandboxWorkspace and SandboxClaudeConfig but omits the new SandboxPiConfig constant added by this PR.
  Remediation: Add SandboxPiConfig = "/sandbox/pi-config" to the code block.

• [stale-enumeration] docs/contributing/sandbox-topology.md:15 — The sandbox image topology table lists "Claude Code" as a key addition in fullsend-sandbox but does not mention the newly installed pi CLI (@earendil-works/pi-coding-agent).
  Remediation: Add "pi CLI" to the "Key additions over parent" column.

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 11:41 PM UTC · Completed 11:57 PM UTC

Commit: 2aefca1 · View workflow run →

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 12:02 AM UTC · Completed 12:19 AM UTC

Commit: c40f528 · View workflow run →

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 12:24 AM UTC · Completed 12:41 AM UTC

Commit: 6d3e6fe · View workflow run →

[fullsend-ai-retro]

🤖 Finished Retro · ✅ Success · Started 12:57 AM UTC · Completed 1:07 AM UTC

Commit: 6d3e6fe · View workflow run →

[fullsend-ai-retro]

Retro: PR #6466 — register pi as a stub runtime

Timeline

1. 21:06 UTC — waynesun09 opened issue #6464 to track pi as a supported runtime.
2. 21:07 UTC — Triage agent dispatched (run 32526865395), completed successfully at 21:13 UTC.
3. 21:14 UTC — waynesun09 opened human-authored PR #6466 (pi-runtime-stub branch) covering stub registration only (12 files, +267/−27).
4. 21:17 UTC — /fs-code dispatched the code agent (run 32527743227), which created a broader PR #6467 (+2004/−29, 22 files) including both the stub and a stream parser.
5. 21:22–00:41 UTC — PR refactor(runtime): register pi as a stub runtime #6466 went through 4 review cycles (runs 32527589298, 32537667377, 32538925408, 32540144513). The author addressed all LOW findings across 2 fix commits, filed follow-up #6471 for the architectural gap.
6. 00:55 UTC — waynesun09 closed PR refactor(runtime): register pi as a stub runtime #6466 without merge, superseded by PR feat(#6464): add the pi runtime (stream parser, Bootstrap/Run, Vertex provider, enablement) #6467.

What went well

• Review agent accuracy: Zero false positives across 4 reviews. All findings were legitimate.
• Iteration tracking: The review agent cleanly dropped findings that the author fixed and persisted unfixed ones. No finding churn.
• Stale-doc detection: The highest-value findings were cross-file documentation gaps (images/README.md, sandbox-topology.md) — exactly what humans miss on multi-file PRs.
• Severity calibration: MEDIUM for procedural/doc gaps, LOW for style/consistency — appropriate throughout.
• Author engagement: waynesun09 addressed all actionable findings and provided thorough investigation of the Qodo bot's HIGH finding.
• Human-agent convergence: The parallel workflow (human stub PR + agent comprehensive PR) resolved naturally when the author merged their work into the agent's branch.

Review gap

The fullsend-ai-review agent missed the Resolve() vs ValidRuntimes() architectural interaction — stub runtimes can be selected via config even though ValidRuntimes() excludes them. The Qodo bot caught this as a HIGH-severity finding. The author confirmed it as a pre-existing gap (shared with the OpenCode stub from #6035) and filed #6471. See the proposal below for a structural improvement.

Existing issues with new evidence

• #2176 (skip retro for closed-without-merge PRs with immediate successor): This retro ran on PR refactor(runtime): register pi as a stub runtime #6466, which was superseded by PR feat(#6464): add the pi runtime (stream parser, Bootstrap/Run, Vertex provider, enablement) #6467. Evidence that the skip logic would have saved a retro run here.
• #1213 (review agent completeness check for multi-site patterns): The Resolve/ValidRuntimes gap is a related instance, though the specific pattern (paired resolver + validation functions) is structurally different from Review agent: add completeness check for multi-site patterns #1213's cross-file focus.

Proposals filed

• Review agent: detect validation-resolver function pairing gaps (in fullsend-ai/agents)