Repository navigation
test(pi): gate the installed Pi 1.0.0 RPC runtime [#639] - #646
Merged
Merged
Conversation
zzwong
added this pull request to stack #647
October 3, 2026 17:19
This was referenced Oct 8, 2026
Base automatically changed from
zzwong/issue-638/pi-rpc-settlement-accounting
to
main
October 9, 2026 01:54
Most pi_rpc tests use a fake Pi, so they stay green when Pi's real completion and event contracts drift. Add tests that run the adapter against the installed Pi with an isolated agent directory and home, offline mode, and a scripted loopback OpenAI-compatible provider with a dummy key. Reviewer tool calls go through the real __pi-review-tool helper, which TestMain dispatches the way the cr binary does. The tests cover structured JSON without tools, a 503 followed by a successful retry, permanent provider failures, a multi-turn reviewer run with the diff-first gate, a path escape, and exact usage, cache, and cost totals, a denied shell tool, and caller cancellation and task deadlines with process-group cleanup. Hostile global and project instructions, extensions, MCP servers, and skills must not load. Ordinary go test skips them. make test-pi-runtime sets CR_PI_RUNTIME_VERSION from the Makefile pin, so a missing or different Pi fails. A new non-required pi-runtime CI job installs that version with npm --ignore-scripts on pinned Node 22.22.3 and runs the target.
A blank PI_RUNTIME_VERSION made make test-pi-runtime export an empty CR_PI_RUNTIME_VERSION, so every runtime test skipped and the target reported ok. The same value made pi-runtime-version print nothing, which the CI install step would turn into an unpinned, latest Pi install. Both Make targets now require exactly one version word before doing anything. The runtime tests also fail, rather than skip, when the opt-in variable is set but blank. The CI step reads the version in its own assignment so a failed lookup stops the step before npm runs. A focused test runs both targets against a fake go and checks that blank, whitespace, and multi-word pins fail without dispatching go test. The development guide now states that the runtime fixture's isolation does not extend to production agent directories (#642).
zzwong
force-pushed
the
zzwong/issue-639/pi-runtime-smoke
branch
from
October 9, 2026 01:54
e9d463e to
ae1cd95
Compare
monit-reviewer
previously approved these changes
Oct 9, 2026
monit-reviewer
left a comment
monit-reviewer
left a comment
There was a problem hiding this comment.
Automated PR Review
Reviewed commit: ae1cd95d0403
Profile: open-cli-collective-review - Posting as: monit-reviewer
Summary
| Reviewer | Findings |
|---|---|
| automation:ci-release | 0 |
| documentation:docs | 1 |
| go:implementation-tests | 0 |
| policies:conventions | 0 |
| structure:repo-health | 0 |
documentation:docs (1 finding)
Minor - docs/development.md:116
The installation example does not stop if the version lookup fails. In an ordinary interactive Bash session, a blank pin or failed make command leaves pi_version empty, and the next command installs the latest Pi rather than the pinned runtime. Guard the lookup and installation with an && chain (or an explicitly fail-fast subshell) so this documented workflow fails closed like CI.
Reviewer Coverage
Complete coverage details are retained in the local coverage.json artifact.
automation:ci-release— complete (broad); scope: 2 files; examples:.github/workflows/ci.yml,Makefile; body-inspected: 2 files; examples:.github/workflows/ci.yml,Makefile; skipped: none; context (not coverage): 2 files; examples:docs/development.md,internal/llmadapters/pi_rpc_runtime_unix_test.go; constraints: Static inspection only; tests and GitHub Actions were not executed. External shared action implementations were not available in repository context.documentation:docs— complete (broad); scope: 1 file; examples:docs/development.md; body-inspected: 1 file; examples:docs/development.md; skipped: none; context (not coverage): 2 files; examples:.github/workflows/ci.yml,Makefile; constraints: Documentation-only review; commands and runtime tests were inspected, not executed.go:implementation-tests— complete (broad); scope: 2 files; examples:internal/llmadapters/pi_rpc_runtime_test.go,internal/llmadapters/pi_rpc_runtime_unix_test.go; body-inspected: 2 files; examples:internal/llmadapters/pi_rpc_runtime_test.go,internal/llmadapters/pi_rpc_runtime_unix_test.go; skipped: none; context (not coverage): 5 files; examples:docs/development.md,internal/llm/subprocess.go,internal/llmadapters/pi_rpc.go,internal/llmadapters/pi_rpc_test.go,internal/llmadapters/subprocess_test.go; constraints: Review was limited to static inspection through read-only repository tools; installed-Pi tests were not executed.policies:conventions— complete (broad); scope: 3 files; examples:.github/workflows/ci.yml,Makefile,docs/development.md; body-inspected: 3 files; examples:.github/workflows/ci.yml,Makefile,docs/development.md; skipped: none; constraints: Review limited to the assigned CI, Makefile, and development-documentation changes. Shared standards linked in the documentation were not available in the provided review context. Static inspection only; no tests or workflows were executed.structure:repo-health— complete (broad); scope: 5 files; examples:.github/workflows/ci.yml,Makefile,docs/development.md,internal/llmadapters/pi_rpc_runtime_test.go,internal/llmadapters/pi_rpc_runtime_unix_test.go; body-inspected: 5 files; examples:.github/workflows/ci.yml,Makefile,docs/development.md,internal/llmadapters/pi_rpc_runtime_test.go,internal/llmadapters/pi_rpc_runtime_unix_test.go; skipped: none; context (not coverage): 1 file; examples:internal/llmadapters/pi_rpc_test.go; constraints: Static review only; tests and CI were not executed.
0 PR discussion threads considered. 0 summarized; 0 resolved.
Completed in 56s | $0.08 | openai-codex/gpt-6.1-sol | cr 0.10.328
| Field | Value |
|---|---|
| Model | openai-codex/gpt-6.1-sol |
| Reviewers | automation:ci-release, documentation:docs, go:implementation-tests, policies:conventions, structure:repo-health |
| Engine | pi_rpc · openai-codex/gpt-6.1-sol |
| Reviewed by | cr · monit-reviewer |
| Duration | 56s wall · 2m 10s compute |
| Cost | $0.08 |
| Tokens | 26.3k in / 1.2k out |
Per-workstream usage
orchestrator-selection— openai-codex/gpt-6.1-sol- In: 5.6k
- Out: 420
- Cache read: 0
- Cache create: 0
- Cost: $0.02
- Duration: 12s
automation:ci-release— openai-codex/gpt-6.1-sol- In: 2.1k
- Out: 92
- Cache read: 19.6k
- Cache create: 0
- Cost: $0.01
- Duration: 25s
documentation:docs— openai-codex/gpt-6.1-sol- In: 538
- Out: 169
- Cache read: 22.4k
- Cache create: 0
- Cost: $0.01
- Duration: 22s
go:implementation-tests— openai-codex/gpt-6.1-sol- In: 5.3k
- Out: 195
- Cache read: 30.5k
- Cache create: 0
- Cost: $0.02
- Duration: 28s
policies:conventions— openai-codex/gpt-6.1-sol- In: 5.6k
- Out: 130
- Cache read: 13.4k
- Cache create: 0
- Cost: $0.01
- Duration: 11s
structure:repo-health— openai-codex/gpt-6.1-sol- In: 3.2k
- Out: 144
- Cache read: 22.4k
- Cache create: 0
- Cost: $0.01
- Duration: 24s
orchestrator-rollup— openai-codex/gpt-6.1-sol- In: 4.1k
- Out: 88
- Cache read: 0
- Cache create: 0
- Cost: $0.01
- Duration: 5s
monit-reviewer
approved these changes
Oct 9, 2026
monit-reviewer
left a comment
monit-reviewer
left a comment
There was a problem hiding this comment.
Automated PR Review
Reviewed commit: ca3d6fe2ba89
Profile: open-cli-collective-review - Posting as: monit-reviewer
Summary
| Reviewer | Findings |
|---|---|
| automation:ci-release | 0 |
| documentation:docs | 0 |
| go:implementation-tests | 0 |
| policies:conventions | 0 |
| structure:repo-health | 0 |
Reviewer Coverage
Complete coverage details are retained in the local coverage.json artifact.
automation:ci-release— complete (constrained); scope: 2 files; examples:.github/workflows/ci.yml,Makefile; body-inspected: 2 files; examples:.github/workflows/ci.yml,Makefile; skipped: none; context (not coverage): 1 file; examples:docs/development.md; constraints: External shared action implementations were not inspected. Static review only; workflows and Make targets were not executed.documentation:docs— complete (constrained); scope: 1 file; examples:docs/development.md; body-inspected: 1 file; examples:docs/development.md; skipped: none; context (not coverage): 3 files; examples:.github/workflows/ci.yml,Makefile,internal/llmadapters/pi_rpc_runtime_test.go; constraints: Review limited to docs/development.md; implementation and CI were inspected as context. Commands and runtime tests were not executed.go:implementation-tests— complete (constrained); scope: 2 files; examples:internal/llmadapters/pi_rpc_runtime_test.go,internal/llmadapters/pi_rpc_runtime_unix_test.go; body-inspected: 2 files; examples:internal/llmadapters/pi_rpc_runtime_test.go,internal/llmadapters/pi_rpc_runtime_unix_test.go; skipped: none; context (not coverage): 2 files; examples:docs/development.md,internal/llmadapters/pi_rpc.go; constraints: Read-only review; tests and the installed Pi runtime were not executed.policies:conventions— complete (constrained); scope: 3 files; examples:.github/workflows/ci.yml,Makefile,docs/development.md; body-inspected: 3 files; examples:.github/workflows/ci.yml,Makefile,docs/development.md; skipped: none; constraints: Inspection only; no tests or workflows were executed. Review limited to the three assigned files and visible repo-local conventions; external shared standards were not available in the review context.structure:repo-health— complete (constrained); scope: 5 files; examples:.github/workflows/ci.yml,Makefile,docs/development.md,internal/llmadapters/pi_rpc_runtime_test.go,internal/llmadapters/pi_rpc_runtime_unix_test.go; body-inspected: 5 files; examples:.github/workflows/ci.yml,Makefile,docs/development.md,internal/llmadapters/pi_rpc_runtime_test.go,internal/llmadapters/pi_rpc_runtime_unix_test.go; skipped: none; context (not coverage): 1 file; examples:internal/llmadapters/pi_rpc.go; constraints: Read-only review; runtime tests and CI were not executed.
0 PR discussion threads considered. 0 summarized; 0 resolved.
Completed in 35s | $0.10 | openai-codex/gpt-6.1-sol | cr 0.10.328
| Field | Value |
|---|---|
| Model | openai-codex/gpt-6.1-sol |
| Reviewers | automation:ci-release, documentation:docs, go:implementation-tests, policies:conventions, structure:repo-health |
| Engine | pi_rpc · openai-codex/gpt-6.1-sol |
| Reviewed by | cr · monit-reviewer |
| Duration | 35s wall · 1m 12s compute |
| Cost | $0.10 |
| Tokens | 39.6k in / 623 out |
Per-workstream usage
automation:ci-release— openai-codex/gpt-6.1-sol- In: 5.2k
- Out: 107
- Cache read: 14.1k
- Cache create: 0
- Cost: $0.01
- Duration: 9s
documentation:docs— openai-codex/gpt-6.1-sol- In: 6.9k
- Out: 88
- Cache read: 19.3k
- Cache create: 0
- Cost: $0.02
- Duration: 14s
go:implementation-tests— openai-codex/gpt-6.1-sol- In: 8.6k
- Out: 120
- Cache read: 31.0k
- Cache create: 0
- Cost: $0.02
- Duration: 15s
policies:conventions— openai-codex/gpt-6.1-sol- In: 5.5k
- Out: 118
- Cache read: 13.7k
- Cache create: 0
- Cost: $0.01
- Duration: 11s
structure:repo-health— openai-codex/gpt-6.1-sol- In: 9.6k
- Out: 138
- Cache read: 17.3k
- Cache create: 0
- Cost: $0.02
- Duration: 16s
orchestrator-rollup— openai-codex/gpt-6.1-sol- In: 3.9k
- Out: 52
- Cache read: 0
- Cache create: 0
- Cost: $0.01
- Duration: 4s
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
2 participants
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.
Fixes #639. Stacked on #641; merge #641 first.
Summary
Most
pi_rpctests use a fake Pi, so they can miss changes in Pi's real contracts. Add credential-free installed-Pi tests and a pinned CI check. No production code changed.HOME, offline flags, a dummy key, and loopback. Reviewer calls dispatch the real__pi-review-toolhelper throughTestMain.pi-runtimeCI with pinned Node 22.22.3/action revisions andnpm install --ignore-scripts. A failed version lookup stops before installation. Existing check identities remain unchanged.Test plan
make test-pi-runtimepasses on macOS and in a Linux arm64/root container; five repeated race runs pass.GOFLAGS=-tags=keyring_nopassage go test ./..., Go 1.26.3 lint,actionlint, andgit diff --checkpass.pi-runtimeon x64/non-root.Limits
Evidence is local/mock, not deployed. Process-group assertions are Unix-only; Windows has no runtime coverage.
Hostile instructions at the default HOME location and in project resources are tested under a fresh selected agent directory. This does not establish production ambient-instruction isolation: selected-agent
AGENTS.mdaffects non-reviewers, andAPPEND_SYSTEM.mdaffects both modes. Follow-up #642 tracks that fix. Codemode remains separate in #640.