Repository navigation
[AI-894] Host Pi as an unattended review-flow reviewer - #1102
Conversation
ReapVerdictGate carries its own line-collapsing sanitizer instead of calling AcpHostedAgentRuntime.SanitizeForForward, so it names no ACP type — a later task composes it from the Pi runtime too. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
ReapVerdictGate's SanitizeReason dropped SanitizeForForward's surrogate-safe 500-char cap, leaving reap reasons (agent-influenced text) uncapped on the report/log path. SanitizeForForward now delegates to the gate's copy instead of duplicating it, so ACP keeps its one sanitizer and the gate stays vendor-neutral. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
For partitioned servers by role instead of trusting the caller to list the result channel first, so a launcher's own server ordering can never silently reorder the advertised tool list. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
CreateOwnerOnly trusted CreateDirectory's mode argument without a read-back, unlike the Kiro/Antigravity/OpenCode siblings it mirrors — an existing root or dir with leftover looser bits would silently re-narrow and proceed instead of refusing. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The npm-offline claim and 8 of 10 planted text-token sources had no positive control, so a dead observable and a real suppression looked identical. Also splits PiContainmentRun into its own file. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… floors Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Composes ReapVerdictGate on PiRpcHostedAgentRuntime and wires PiRoundCeiling into it, guarded on a new PiReviewerGuards being non-null so an interactive launch keeps today's behaviour in every case. Three review-flow-only guards share one reap routine: an unanswerable dialog, a round past its ceiling, and a command-shaped prompt. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The reviewer ladder is read by both advertisement and launch, so a daemon cannot offer a reviewer it would refuse to start. A Pi reap is a deliberate coded reason, so its exception omits the transport suffix ACP adds. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Widens CaptureServerConnection.VerdictCaptureRuntime from AcpHostedAgentRuntime to ITerminationVerdictSource so the shared check-to-send race test can drive it with a Pi runtime too. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Code Review by Qodo
1.
|
PR Summary by QodoAdd contained Pi reviewer lane for unattended review flows
AI Description
Diagram
High-Level Assessment
Files changed (61)
|
read_file streamed a whole hostile file into memory before applying its line/byte caps and counted bytes as UTF-16 length; it now reads through a fixed buffer, refuses on a bounded binary prefix, and measures returned size with Buffer.byteLength. globMatches backtracked combinatorially on a crafted "**" run; it is now memoised by (glob-seg, path-seg), refuses an over-complex pattern, and carries the search deadline so a match aborts with the budget. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
A normal Pi reviewer death did not claim the reap slot, so a racing round timer, a buffered dialog, or a command-shaped input could still publish a verdict over an ordinary teardown, and dispose sampled the reap once and could miss a later claim. The gate now seals under its lock — EnterTerminal seals before disarming the ceiling, dispose awaits the reap the seal returns — so a claim after teardown is refused. The ceiling's UserEcho rode the 16-entry transcript-dedup cache, so past 16 queued rounds an evicted echo left `owed` stuck and reaped a settled reviewer. Round accounting now rides its own uncapped tally. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
A null/empty/whitespace prompt slipped past reviewer-context validation, so the launch skipped the first-prompt send and returned an idle runtime with no round started and no ceiling armed — the round then hung forever. Validation now refuses a blank prompt with the launch-context-incomplete code before the launch directory is created, and the first-prompt send is unconditional once validation passes. The launch tests also move the env test onto EnvScope.Exclusive so it no longer discards inherited values. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Use the shared JsonElement.IsArray helper in the containment bench, bound the two real-child cert classes to the shared subprocess budget with ParallelLimiter<SubprocessLimit>, inject TempDir into the launch-dir tests instead of constructing one per test, and rewrite the fixture doc comments to describe the containment purpose rather than a dated probe path. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
A line-based offset is honoured by scanning to it, so a large offset on a large file traversed the whole file regardless of the returned-byte cap, pinning the reviewer's event loop and delaying its abort. read_file now refuses a file over a 1 MB read ceiling before opening it and caps total bytes scanned per call as a backstop against a file that grew past its stat size. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
DisposeAsync abandoned an already-won reap after a fixed 5s wait, so a slow-but-legitimate teardown could be cut short before the promised terminate ran, and a fault after the timeout went unobserved. It now attaches an unconditional fault-observing continuation before the wait and sizes the wait to the reap's own bounded ladder (abort grace + stop grace), so a normal teardown is not cut short while a stuck one still cannot hang. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Part of AI-894 (no GitHub issue in this repo; the issue closes after rollout, so no closing keyword).
What & why
Pi is the one supported harness with no ACP endpoint, so it could not ride the ACP reviewer work. This adds an unattended review-flow lane over a contained
pi --mode rpcchild:start_review_flow(vendor: "pi")advertisespiand launches a reviewer that reports throughsubmit_review_result.Containment is by argv plus one per-launch TypeScript extension that bridges the flow-result MCP server and supplies worktree-scoped read-only file tools; no Pi built-in tool is reachable. One ordered tool list renders both the extension manifest and the
--toolsallowlist, and the host verifies the child's live tool set from aready.jsonbefore any prompt is sent. Coded failure reasons reach the server through a reap-verdict gate lifted out of the ACP runtime and now shared with Pi. Every recordedpiversion is a minimum (floors, not pins).Interactive Pi and ACP behaviour are unchanged, except ACP's post-registration
Runningis now gated (closing the same interval for ACP).Where to look
PiReviewerExtension(component-wise realpath containment, no symlink following, literal search),PiReviewerToolSurface(one list → manifest + allowlist), and host-verifiedready.json.ReapVerdictGatelift: ACP delegates to it unchanged;PiRpcHostedAgentRuntimecomposes the same gate. Interactive launches never consult the reviewer ladder.Verification
pi0.85.1 (KCAP_PI_REVIEWER_CONTAINMENT=1): 14/14 pass, and 14/14 skip cleanly without the gate. The three live certs (hosted, memory, reviewer) are written and skip until run at deploy.🤖 Generated with Claude Code