Skip to content

[AI-3313] Let a Pi reviewer borrow the requester's checkout as a snapshot - #1225

Merged
realtonyyoung merged 3 commits into
mainfrom
tonyyoung/ai-3313-pi-reviewer-snapshot-borrow
Sep 29, 2026
Merged

realtonyyoung merged 3 commits into
mainfrom
tonyyoung/ai-3313-pi-reviewer-snapshot-borrow

Conversation

@realtonyyoung

Copy link
Copy Markdown
Collaborator

AI-3313 (no GitHub issue — filed in Linear only)

What & why

Every Pi review flow fell back to an owned worktree at HEAD (borrow_capability_missing), so the requester's uncommitted and untracked work never reached the reviewer. The Pi factory now advertises the existing independent-snapshot borrow — the one Cursor uses without a process sandbox: the daemon copies the requester's tracked, modified and untracked files into its own git clone, rebuilds it before every round, and the reviewer runs in that copy, never the live checkout. The reviewer's tools are rooted at the snapshot root (the working directory can be a subdirectory of it), the snapshot's kcap-review-context server gets its one read-only tool on Pi's surface, and the system prompt and README describe the borrowed case.

Where to look

PiRpcHostedAgentRuntimeFactory: advertisement follows SupportsUnattended, ReviewRoot picks the snapshot root, and a snapshot launch without a snapshot root or for an interactive agent is refused (borrowed_snapshot_containment_mismatch). The live-checkout (BorrowedCwd) refusal is unchanged. The server needs no change — independent-snapshot is already a recognized containment.

Verification

  • A_borrowed_snapshot_review_roots_the_reviewer_at_the_snapshot_and_offers_the_review_context drives the real reviewer launch with a snapshot context and reads the manifest it wrote: root is the snapshot root, and the review-context server is bridged. Rooting at the working directory instead fails it.
  • Gated real-Pi certs still pass: containment 15/15, git tools 4/4.
  • Pi* daemon tests: 237 passed, 21 gated skips.

🤖 Generated with Claude Code

…shot

Pi has no process sandbox, so it borrows the way Cursor does: a daemon-owned copy of the
working tree, rebuilt every round, with the reviewer's tools rooted at the copy.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@linear-code

linear-code Bot commented Sep 29, 2026

Copy link
Copy Markdown

AI-3313

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Enable Pi reviewers to review borrowed checkout snapshots

✨ Enhancement 🧪 Tests 📝 Documentation 🕐 40+ Minutes

Grey Divider

AI Description

• Let Pi reviewers see tracked, modified, and untracked work through daemon-owned snapshots, never
 the live checkout.
• Root reviewer tools at the snapshot and expose its read-only review-context tool.
• Refuse invalid snapshot launches and document the borrowed-review behavior.
Diagram

graph TD
  O["Review orchestrator"] --> S["Owned snapshot"] --> F["Pi factory"] --> M["Launch manifest"] --> P["Pi reviewer"]
  C["Review context"] --> M
Loading
High-Level Assessment

Reuse the existing independent-snapshot path. It delivers uncommitted work without giving Pi access to the live checkout or requiring a new process sandbox; the launch checks preserve that boundary.

Files changed (6) +101 / -4

Enhancement (3) +29 / -2
PiReviewerSystemPrompt.csExplain snapshot files versus committed history +1/-1

Explain snapshot files versus committed history

• Tells reviewers that borrowed reviews include copied working-tree changes while git history tools show committed history.

src/Capacitor.Cli.Core/Harness/Pi/PiReviewerSystemPrompt.cs

PiReviewerToolSurface.csExpose the borrowed review-context tool +9/-0

Expose the borrowed review-context tool

• Adds the review-context server's single read-only tool to Pi's reviewer surface instead of rejecting its server as unclassified.

src/Capacitor.Cli.Daemon/Harness/Pi/PiReviewerToolSurface.cs

PiRpcHostedAgentRuntimeFactory.csEnable snapshot borrowing for supported Pi reviewers +19/-1

Enable snapshot borrowing for supported Pi reviewers

• Advertises independent-snapshot borrowing when unattended reviews are supported. Refuses snapshot launches without a root or reviewer flow, and roots reviewer tools at the snapshot root rather than a possible subdirectory.

src/Capacitor.Cli.Daemon/Harness/Pi/PiRpcHostedAgentRuntimeFactory.cs

Tests (2) +69 / -0
PiReviewerLaunchTests.csTest borrowed snapshot advertisement and launch containment +58/-0

Test borrowed snapshot advertisement and launch containment

• Checks conditional borrowing support, verifies the launched manifest's snapshot root and review-context server, and covers invalid snapshot launches.

test/Capacitor.Cli.Daemon.Tests.Unit/Harness/Pi/PiReviewerLaunchTests.cs

PiReviewerToolSurfaceTests.csTest review-context tool exposure +11/-0

Test review-context tool exposure

• Verifies the borrowed review-context server contributes its expected Pi and MCP tool names.

test/Capacitor.Cli.Daemon.Tests.Unit/Harness/Pi/PiReviewerToolSurfaceTests.cs

Documentation (1) +3 / -2
README.mdDocument Pi's borrowed working-tree reviews +3/-2

Document Pi's borrowed working-tree reviews

• Explains that Pi reviews use a daemon-owned copy containing uncommitted and untracked work, rebuilt before each round, instead of the live checkout.

README.md

@qodo-code-review

qodo-code-review Bot commented Sep 29, 2026 •

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Grey Divider


Remediation recommended

1. A built-in warning labels the wrong tool ✓ Resolved
Description
ReviewContextServer now has two consecutive <summary> tags, including the existing explanation
of PiBuiltInNames. In generated documentation, that built-in collision warning attaches to the
review-context server rather than the built-in name list it describes.
Code

src/Capacitor.Cli.Daemon/Harness/Pi/PiReviewerToolSurface.cs[R18-20]

+    /// <summary>The one tool the borrowed-snapshot review-context server serves.</summary>
+    internal const string ReviewContextServer = "kcap-review-context";
+    internal const string ReviewContextTool   = "get_branch_authored_mcp_configs";
Relevance

●●● Strong

Exact precedent accepted fixing consecutive XML summaries so each documents the intended
declaration.

PR-#421

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The added summary and constants sit between the existing built-in warning and PiBuiltInNames, so
the warning documents the wrong declaration. This violates the requirement that comments remain
accurate and useful when read in the current file.

Rule 2762993: Restrict comments to documenting non-obvious, behavior‑critical constraints
src/Capacitor.Cli.Daemon/Harness/Pi/PiReviewerToolSurface.cs[16-24]

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

## Issue description
The new review-context constants separate the existing built-in warning from the field it documents.

## Fix Focus Areas
- src/Capacitor.Cli.Daemon/Harness/Pi/PiReviewerToolSurface.cs[16-24]

## Recommended Fix
Move the existing built-in warning immediately above `PiBuiltInNames`. Keep any documentation for the review-context constants separate and specific to their constraint.

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


Grey Divider

Context sources
✅ Compliance rules (platform): 64 rules
✅ Cross-repo context — repo relationships
  Explored: repo: kurrent-io/kcap-server (sha: da763ffb) — View relationship
Review mode: ⚖️ Balanced: This changes borrowed-review containment, filesystem roots, tool exposure, and launch validation across runtime and tests, creating meaningful security and behavior risk despite the localized scope.

Grey Divider

Tip of the day
💡 Did you know, you can add REVIEW.md to your repo root and Qodo follows it on every PR

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread src/Capacitor.Cli.Daemon/Harness/Pi/PiReviewerToolSurface.cs
realtonyyoung and others added 2 commits September 29, 2026 10:15
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ative PATH

A written prompt now counts as busy until its turn starts, so a round's refresh cannot rewrite
files under it; the reviewer child keeps only absolute PATH entries.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@realtonyyoung
realtonyyoung merged commit a9ea868 into main Sep 29, 2026
8 checks passed
@realtonyyoung
realtonyyoung deleted the tonyyoung/ai-3313-pi-reviewer-snapshot-borrow branch September 29, 2026 14:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant