[luv-342] feat: enforce Pi Stop policies via before_agent_start handoff#341
[luv-342] feat: enforce Pi Stop policies via before_agent_start handoff#341NiveditJain merged 2 commits intomainfrom
Conversation
Pi's `agent_end` event has no Result type — by the time the handler fires,
Pi's agent loop has already exited, so a deny return cannot keep Pi running
the way Claude's exit-2-from-Stop does. Empirically: a user on Pi with
`require-commit-before-stop` enabled saw the deny reason propagate but Pi
exited anyway.
Fix: the `pi-extension/index.ts` shim now captures the deny `reason` from
`agent_end` into a per-`sessionId` in-memory map and re-injects it as a
`systemPrompt` suffix on the next `before_agent_start` (Pi v0.73.x — fires
after a new user prompt, before the agent loop runs). The map is one-shot
per drain and is cleared on every `session_shutdown` reason (including
`quit`), so a stale gate cannot leak into a fresh session in the same
process. `policy-evaluator.ts` emits the `MANDATORY ACTION REQUIRED from
failproofai (policy: …)` wrapper inside `reason` for `cli === "pi" &&
eventType === "Stop"` (deny + instruct paths), mirroring the
Cursor/Gemini/Copilot/OpenCode Stop branches; non-Stop Pi events keep the
existing flat `{permission, reason}` shape.
Bounded by Pi process lifetime — same bound Claude has on exit-2-from-Stop.
Verified:
- Unit: 1603/1603 (incl. 7 new shim tests)
- E2E: 298/298 (incl. new dirty-repo Pi case)
- Live binary smoke in dirty repo emits the MANDATORY ACTION reason JSON
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughThis PR implements cross-turn Stop-policy enforcement for Pi by capturing deny reasons at the ChangesPi Cross-turn Stop-policy Enforcement
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/hooks/policy-evaluator.ts (1)
191-202: ⚡ Quick winPlease pin both new Pi Stop output branches with direct evaluator tests.
The new shim/e2e coverage in this PR exercises the deny handoff, but it doesn't directly lock down these two
cli === "pi" && eventType === "Stop"payload shapes—especially the instruct branch, which now also relies onpermission: "deny"to trigger next-turn enforcement. A smallpolicy-evaluatortest matrix here would catch regressions before they silently weaken Pi Stop enforcement.As per coding guidelines, "Always add unit tests for new behaviour. Place tests in tests/."
Also applies to: 438-452
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/hooks/policy-evaluator.ts` around lines 191 - 202, Add unit tests in __tests__/ for policy-evaluator.ts that explicitly cover the pi Stop branches where session?.cli === "pi" and eventType === "Stop": create tests that invoke the evaluator with a pi CLI session and Stop event and assert the returned object contains decision: "deny", policyName matching policy.name, reason matching input, and stdout JSON with permission: "deny" and the instruct-style reasonText; also add the parallel test covering the other Stop branch referenced around the other block (lines ~438-452) so both deny handoffs are pinned and regressions in permission: "deny" behavior are caught.
🤖 Prompt for all review comments with AI agents
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 `@CHANGELOG.md`:
- Around line 3-6: The changelog entry under the "## 0.0.10-beta.12 —
2026-05-10" header contains an unresolved placeholder "#PR"; replace that
placeholder with the actual PR number "#341" so the release note for the
pi-extension/agent_end change reads with the correct PR reference.
---
Nitpick comments:
In `@src/hooks/policy-evaluator.ts`:
- Around line 191-202: Add unit tests in __tests__/ for policy-evaluator.ts that
explicitly cover the pi Stop branches where session?.cli === "pi" and eventType
=== "Stop": create tests that invoke the evaluator with a pi CLI session and
Stop event and assert the returned object contains decision: "deny", policyName
matching policy.name, reason matching input, and stdout JSON with permission:
"deny" and the instruct-style reasonText; also add the parallel test covering
the other Stop branch referenced around the other block (lines ~438-452) so both
deny handoffs are pinned and regressions in permission: "deny" behavior are
caught.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3002b6ef-3112-45cb-9e30-2bac584a16a4
📒 Files selected for processing (6)
CHANGELOG.mdCLAUDE.md__tests__/e2e/hooks/pi-integration.e2e.test.ts__tests__/hooks/pi-extension-shim.test.tspi-extension/index.tssrc/hooks/policy-evaluator.ts
…elog PR ref CodeRabbit follow-ups on PR #341: 1. Direct policy-evaluator coverage for the new Pi-Stop branches. The shim and e2e tests already exercise the agent_end → before_agent_start handoff end-to-end, but neither directly locks the JSON shape that policy-evaluator emits. Adds 4 tests in `__tests__/hooks/policy-evaluator.test.ts`: - Pi Stop + deny: pins `{permission:"deny", reason: /MANDATORY ACTION REQUIRED.*policy: exospherehost\/stop-blocker.*tests not run/}`. - Pi non-Stop deny: regression guard — PreToolUse on Pi must keep the legacy `{permission:"deny", reason:"Blocked Bash by failproofai because: …"}` shape (the Stop split must not leak into tool events). - Pi Stop + instruct: pins `{permission:"deny"}` (NOT the regular Pi instruct `{permission:"allow"}`) so the shim's agent_end handler captures the reason — silently switching back to "allow" here would be invisible to the shim and turn Stop instructs into a no-op. - Pi non-Stop instruct: regression guard — PreToolUse instruct on Pi must keep `{permission:"allow", reason: "Instruction from failproofai: …"}`. 2. CHANGELOG #PR placeholder → #341. Verified: 1607/1607 unit tests pass (was 1603 + 4 new). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* [luv-343] docs: document per-CLI Stop semantics + Pi limitation Follow-up to #341: explain to end-users how `require-*-before-stop` behaves across the 7 supported CLIs, with a dedicated note for Pi. Adds a "Per-CLI Stop semantics" subsection to the Workflow chapter of docs/built-in-policies.mdx: - 7-row table covering Claude / Codex / Copilot / Cursor / Gemini / OpenCode / Pi, showing where the gate fires and what the user sees. - A `<Note>` callout walking through the Pi limitation: Pi's `AgentEndEvent` has no Result type, so failproofai shifts enforcement to `before_agent_start` (next user turn). Pi visibly stops between turns; the gate fires the moment you submit the next prompt. Bounds (Pi process lifetime, `session_shutdown` cleanup) are spelled out so users enabling Stop policies on Pi understand the behavior before filing a bug. Translated copies under docs/{ar,de,es,fr,he,hi,it,ja,ko,pt-br,ru,tr,vi,zh}/ will be regenerated by the translate-docs workflow on merge. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * [luv-343] docs: replace #PR placeholder with #342 in CHANGELOG CodeRabbit catch on #342 — same placeholder fixup as #341 had. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
Summary
AgentEndEventhas no Result type — by the time the handler fires, Pi's agent loop has already exited, so a deny return cannot keep Pi running the way Claude's exit-2-from-Stop does. Verified against pi-coding-agent v0.73.1 d.ts atpackages/coding-agent/src/core/extensions/types.ts:1112. Empirically: a user enabledrequire-commit-before-stopon Pi, the denyreasonpropagated, but Pi exited anyway.pi-extension/index.tsshim now captures the denyreasonfromagent_endinto a per-sessionIdin-memory map, then on the nextbefore_agent_start(Pi v0.73.x — fires after a new user prompt, before the agent loop runs) returns{systemPrompt: <event.systemPrompt> + "\n\n" + reason}so the LLM sees aMANDATORY ACTION REQUIREDdirective at the top of its next turn. The map is one-shot per drain and is cleared on everysession_shutdownreason (includingquit), so a stale gate cannot leak into a fresh session in the same Pi process.policy-evaluator.tsnow emits theMANDATORY ACTION REQUIRED from failproofai (policy: …)wrapper insidereasonforcli === "pi" && eventType === "Stop"(deny + instruct paths), mirroring the Cursor/Gemini/Copilot/OpenCode Stop branches; non-Stop Pi events keep the existing flat{permission, reason}shape.Files
pi-extension/index.ts—pendingStopBlockBySessionmap;agent_endcaptures deny; newbefore_agent_starthandler drains;session_shutdowncleanup on every reason (sessionId captured before cache reset to avoid disk re-discovery).src/hooks/policy-evaluator.ts— PiStopbranch in deny + instruct paths.__tests__/hooks/policy-evaluator.test.ts— 4 new tests pinning the Pi-Stop deny + instruct payload shapes (and regression guards confirming non-Stop Pi events keep the legacy shape) — added per CodeRabbit suggestion.__tests__/hooks/pi-extension-shim.test.ts— 7 new shim tests (capture/drain/one-shot/session_shutdown-clear/no-pending-noop/missing-systemPrompt/no-resolvable-sessionId).__tests__/e2e/hooks/pi-integration.e2e.test.ts— new dirty-repo Pi case asserting the binary's stdout JSON shape.CLAUDE.md— capability matrix (agent_end: shifted (next turn)+ newbefore_agent_startrow, version bumped to 0.73.1).CHANGELOG.md—0.0.10-beta.12 — 2026-05-10Features entry.Test plan
bun run test:run— 1607/1607 pass (incl. 4 new policy-evaluator + 7 new shim tests)bun run test:e2e— 298/298 pass (incl. new Pi dirty-repo case)bun run lint— 0 errors (1 pre-existing img-element warning)bunx tsc --noEmit— cleanbun build --target=node --format=cjs --outfile=dist/index.js src/index.ts+bun run build:cli— both succeed--hook agent_end --cli pi): emits{permission:"deny", reason:"MANDATORY ACTION REQUIRED from failproofai (policy: exospherehost/require-commit-before-stop): You have uncommitted changes …"}🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Tests