[luv-299] fix: enforce require-*-before-stop on GitHub Copilot CLI

[NiveditJain]

Summary

• Empirically verified (Copilot CLI 1.0.41, via ~/.copilot/logs/process-*.log + session-state events.jsonl): exit-2 + stderr from a Stop hook on Copilot's agentStop is logged as [WARNING] Hook warning: … and surfaced to the user, but the agent stops cleanly without retrying — same observation-only failure mode as Cursor/OpenCode/Pi, except Copilot at least shows the message.
• Per Copilot's documented agentStop semantics, the retry shape is {decision: "block", reason} JSON on stdout (exit 0); reason becomes the next-turn prompt. New cli === "copilot" branches in src/hooks/policy-evaluator.ts emit that shape on both the deny path (line ~267) and the instruct path (line ~457), mirroring the existing Gemini AfterAgent branches. Without these branches, all five require-*-before-stop builtins (commit / push / PR / no-conflicts / CI-green) were silently observation-only on Copilot.
• Adds SubagentStop to COPILOT_HOOK_EVENT_TYPES so custom policies subscribing to that event are reachable from Copilot subagent boundaries (Copilot's subagentStop aliases to PascalCase via the same VS Code-compat alias that maps agentStop ↔ Stop).
• Both cli === "copilot" branches (deny + instruct) match Stop and SubagentStop so those custom subagent policies get the correct retry shape too. The 5 require-*-before-stop builtins still match Stop only by design — they are session-completion gates (commit/push/PR/conflicts/CI), not subagent-return gates — so the SubagentStop widening does not change builtin behavior.
• Updates CLAUDE.md per-CLI capability matrix and the Copilot section in src/hooks/types.ts with the verified Stop block semantics.

Test plan

• bun run test:run — 1524 passing (was 1517 before; +7 from new Copilot Stop deny, Copilot SubagentStop deny, Claude SubagentStop deny, SubagentStop config assertion, plus the three new instruct-path tests added by the latest commit: Claude SubagentStop instruct, Copilot Stop instruct, Copilot SubagentStop instruct)
• bun run test:e2e — 292 passing (was 291; +1 from new Copilot Stop deny e2e regression)
• bun run lint — clean (only pre-existing unrelated @next/next/no-img-element warning)
• bunx tsc --noEmit — clean
• Empirical verification against real Copilot CLI 1.0.41 in a sandbox: confirmed exit-2 path produces [WARNING] Hook warning: … with no retry; confirmed agentStop and Stop PascalCase aliasing works for user-scope hooks
• CodeRabbit review: addressed (1) the deny-branch SubagentStop widening on the first review pass and (2) the parallel instruct-branch fix on the second pass — the deny path had a Copilot arm but the instruct path's eventType === "Stop" arm was missing both the SubagentStop widening AND the Copilot arm, so instruct verdicts on Copilot still fell through to exit-2

Open follow-ups (not blocking this PR)

• .github/hooks/failproofai.json project-scope appears NOT to be loaded by Copilot CLI 1.0.41 in my sandbox tests — only user-scope ~/.copilot/hooks/ was dispatched. I tried multiple schema variants (PascalCase + matcher-wrap, flat camelCase) and filenames (failproofai.json, canonical hooks.json), with files committed to git. None fired. Worth filing a separate investigation issue.
• The Cursor followup_message lift (policy-evaluator.ts:147 Stop+deny → {followup_message} instead of {permission:"deny"}) is still pending. Lower stakes; covered in the PR [luv-299] fix: enforce require-*-before-stop on GitHub Copilot CLI #299 review thread.

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features

  • Copilot stop now surfaces mandatory-action block messages and supports SubagentStop parity.
  • Per-session download/export expanded (including OpenCode export format) and transcripts populate across activity dashboard harnesses.

• Bug Fixes

  • Improved transcript path resolution, tool-name canonicalization, and session-log rendering stability.
  • Better install/launch error messaging and PATH shadowing diagnostics/path protection.

• Documentation

  • Added Copilot Stop semantics.

• Tests

  • Expanded integration and end-to-end coverage for Copilot/Stop/SubagentStop and OpenCode exports.

[coderabbitai]

📝 Walkthrough

Walkthrough

The PR adds Copilot-aware Stop handling: Stop and SubagentStop now emit a Copilot-specific JSON "block" payload on stdout with exitCode 0 when the session CLI is "copilot". It extends Copilot event types to include "SubagentStop", updates docs, and adds tests and a test helper to validate the new behavior.

Changes

Copilot Stop Event Handling & SubagentStop Support

Layer / File(s)	Summary
Type Definitions & Documentation src/hooks/types.ts, CLAUDE.md	COPILOT_HOOK_EVENT_TYPES now includes "SubagentStop". Added docs describing Copilot Stop block semantics and the stdout JSON block shape used to enforce denials.
Core Policy Evaluator Logic src/hooks/policy-evaluator.ts	Stop and SubagentStop handling now branch on session.cli === "copilot": for Copilot return exitCode: 0 with stdout JSON { decision: "block", reason: ... } (and empty stderr); non-Copilot behavior continues to return exitCode: 2 with reason on stderr. This behavior applies to both deny and instruct paths.
Test Helpers __tests__/e2e/helpers/hook-runner.ts	Added assertCopilotStopBlock(result: HookRunResult) asserting exitCode === 0, parsed decision === "block", and reason contains MANDATORY ACTION REQUIRED.
Tests & Validation __tests__/e2e/hooks/copilot-integration.e2e.test.ts, __tests__/hooks/policy-evaluator.test.ts, __tests__/hooks/integrations.test.ts	New and updated tests validate Copilot Stop JSON output, SubagentStop inclusion in event types, and Copilot-specific deny/instruct flows producing the stdout JSON with expected reason prefix; e2e and unit expectations updated accordingly.
Changelog CHANGELOG.md	Unreleased notes updated to document Copilot require-*-before-stop policy enforcement, SubagentStop parity, transcript/export and download-session mentions, and related test/doc updates.

Sequence Diagram

sequenceDiagram
  participant Copilot as Copilot CLI
  participant HookRunner as Hook Runner
  participant Policy as Policy Evaluator
  participant Std as Stdout/Stderr

  Copilot->>HookRunner: invoke Stop/SubagentStop hook (session.cli="copilot")
  HookRunner->>Policy: evaluatePolicies(session, eventType)
  Policy-->>HookRunner: { decision: "instruct/deny", reasonText }
  alt session.cli == "copilot" and eventType is Stop/SubagentStop
    HookRunner->>Std: write stdout JSON { decision: "block", reason: reasonText }
    HookRunner-->>Copilot: exitCode 0
  else non-Copilot Stop/SubagentStop
    HookRunner->>Std: write stderr reasonText
    HookRunner-->>Copilot: exitCode 2
  end

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

• exospherehost/failproofai#63: Modifies evaluatePolicies in src/hooks/policy-evaluator.ts and related stdout JSON behavior; touches the same function and output conventions.
• exospherehost/failproofai#109: Related Stop-event handling changes that introduce or standardize the "MANDATORY ACTION REQUIRED" framing used in deny responses.
• exospherehost/failproofai#64: Also modifies Stop behavior in src/hooks/policy-evaluator.ts; related to where Stop denies write their reason and output semantics.

Poem

🐰 I hopped through hooks with careful cheer,
Copilot stops now speak plain and clear,
Subagent joins, a JSON block so bright,
Tests nod, docs glow in the springtime light,
Hop, patch, assert — the rabbit claps in delight.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main change: enforcing require-*-before-stop policies on Copilot CLI via documented Stop block semantics.
Description check	✅ Passed	The description provides comprehensive context including empirical verification, technical rationale, test results, and checklist completion, well exceeding template requirements.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 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 `@src/hooks/policy-evaluator.ts`:
- Around line 267-287: Builtin policies registered with match: { events:
["Stop"] } and the runtime check if (eventType === "Stop") both miss
SubagentStop, so Copilot subagent denials fall through to exitCode 2 instead of
returning the Copilot block payload; update the policy registrations that
currently use match: { events: ["Stop"] } to include "SubagentStop" as well, and
change the runtime condition in policy-evaluator.ts from if (eventType ===
"Stop") to check both Stop and SubagentStop (e.g., eventType === "Stop" ||
eventType === "SubagentStop") so the Copilot branch that returns
JSON.stringify({ decision: "block", reason: reasonText }) (and the surrounding
session?.cli === "copilot" handling, policy.name, reason, decision: "deny"
fields) is executed for both event types.

🪄 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: 2e1207c2-5f3e-4f64-a2cd-97b6540ffdbb

📥 Commits

Reviewing files that changed from the base of the PR and between e5f3eb7 and 5959e6d.

📒 Files selected for processing (8)

• CHANGELOG.md
• CLAUDE.md
• __tests__/e2e/helpers/hook-runner.ts
• __tests__/e2e/hooks/copilot-integration.e2e.test.ts
• __tests__/hooks/integrations.test.ts
• __tests__/hooks/policy-evaluator.test.ts
• src/hooks/policy-evaluator.ts
• src/hooks/types.ts

[coderabbitai]

Actionable comments posted: 2

🤖 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`:
- Line 9: Update the CHANGELOG language to avoid implying the five builtins are
affected: clarify that adding SubagentStop to COPILOT_HOOK_EVENT_TYPES and
widening the cli === "copilot" branch in policy-evaluator.ts to handle both Stop
and SubagentStop enables custom policies to fire on Copilot subagent boundaries,
while the existing require-*-before-stop builtins still intentionally match only
Stop; reword the earlier sentence to explicitly state this distinction
(reference symbols: SubagentStop, COPILOT_HOOK_EVENT_TYPES,
require-*-before-stop, policy-evaluator.ts cli === "copilot" branch).

In `@src/hooks/policy-evaluator.ts`:
- Around line 267-288: The instruct-path for policy enforcement misses the
Copilot-specific `{decision: "block", reason}` response for both "Stop" and
"SubagentStop" events; update the instruct branch that currently only checks
`eventType === "Stop"` to also handle `eventType === "SubagentStop"` and add a
`session?.cli === "copilot"` branch that returns the same shape as the deny-path
(exitCode: 0, stdout: JSON.stringify({decision: "block", reason: reasonText}),
stderr: "", policyName: policy.name, reason, decision: "deny") before falling
through to the generic Exit-2 + stderr handler so Copilot retries on both Stop
and SubagentStop for instruct policies.

🪄 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: f7e44b9e-f852-4e73-bd26-b80ebec943f4

📥 Commits

Reviewing files that changed from the base of the PR and between 5959e6d and e80d8ce.

📒 Files selected for processing (3)

• CHANGELOG.md
• __tests__/hooks/policy-evaluator.test.ts
• src/hooks/policy-evaluator.ts

[coderabbitai]

🧹 Nitpick comments (1)

CHANGELOG.md (1)

9-9: ⚡ Quick win

Condense this changelog item to the repo’s single-line format.

This reads like release notes rather than an Unreleased changelog bullet. Please trim it to a short one-line summary with the PR reference.

As per coding guidelines, CHANGELOG.md: “Add every PR update under the ## Unreleased section with subsections for Features, Fixes, Docs, and Dependencies; format as single-line entries like - Add foo support (#123).”

🤖 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 `@CHANGELOG.md` at line 9, Replace the long multi-paragraph entry in
CHANGELOG.md with a single-line Unreleased bullet in the repo's required format:
condense the content to one short sentence like "- Enforce require-*-before-stop
policies and add Copilot SubagentStop parity (`#PR`)" (use the actual PR number in
place of `#PR`), place it under the appropriate Unreleased subsection (Features or
Fixes), and remove the verbose release-note style paragraph entirely.

🤖 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.

Nitpick comments:
In `@CHANGELOG.md`:
- Line 9: Replace the long multi-paragraph entry in CHANGELOG.md with a
single-line Unreleased bullet in the repo's required format: condense the
content to one short sentence like "- Enforce require-*-before-stop policies and
add Copilot SubagentStop parity (`#PR`)" (use the actual PR number in place of
`#PR`), place it under the appropriate Unreleased subsection (Features or Fixes),
and remove the verbose release-note style paragraph entirely.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 4fa93ef5-6ae9-4cc1-8175-ac083735f38c

📥 Commits

Reviewing files that changed from the base of the PR and between e80d8ce and de88fea.

📒 Files selected for processing (3)

• CHANGELOG.md
• __tests__/hooks/policy-evaluator.test.ts
• src/hooks/policy-evaluator.ts

🚧 Files skipped from review as they are similar to previous changes (1)

• tests/hooks/policy-evaluator.test.ts