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

CHANGELOG.md

@@ -6,6 +6,7 @@
 - Activity dashboard: enable the `Download Logs` button for every harness, not just Claude Code. The five external file-backed CLIs (Codex, Copilot, Cursor, Pi, Gemini) already had per-CLI session loaders with discovered on-disk JSONL paths via `lib/<cli>-sessions.ts#find*Transcript(sessionId)`, but `app/api/download/[project]/[session]/route.ts` only resolved Claude paths via `lib/projects.ts#resolveSessionFilePath`, and `app/project/[name]/session/[sessionId]/page.tsx` gated the button behind `!isExternal`. New `lib/download-session.ts` dispatcher returns either `{kind:"file", path}` (claude / codex / copilot / cursor / pi / gemini — streamed verbatim as `application/x-ndjson` so replay tooling sees the original CLI bytes) or `{kind:"synthesized", body, contentType, extension}` (opencode only — see below). The route validates the `?cli=<id>` query param against `isKnownCli`, validates `sessionId` per-CLI shape (UUID for six, `^ses_[A-Za-z0-9]+$` for opencode mirroring `lib/opencode-sessions.ts`'s SQL guard), and extends `logActivity` to record the originating CLI. Session page drops the `!isExternal` gate and appends `?cli=${cli}` to the href. For OpenCode (transcripts live in SQLite at `~/.local/share/opencode/opencode.db` across three tables — `session` / `message` / `part` — with no on-disk file), the export emits a JSON document mirroring that structure: `{"session": …, "messages": [...], "parts": [...]}`, served as `application/json` with a `<sessionId>.json` filename. The `data` JSON columns on each message/part are parsed for readability; malformed JSON survives as the raw string. A new `getOpenCodeSessionExport(sessionId)` in `lib/opencode-sessions.ts` performs the export query (the existing viewer-facing `getOpenCodeSessionLog` collapses parts into per-message log entries, which is wrong for export). Mid-flight DB query failures return null (→ HTTP 404) rather than empty `messages: []` / `parts: []` arrays, so a broken `opencode db` call is never indistinguishable from a genuinely empty session. Test matrix: `__tests__/lib/download-session.test.ts` (12 cases — happy path + 404 for all 7 CLIs, 400 on bad cli / malformed session id, real on-disk fixture for codex), `__tests__/api/download-route.test.ts` (6 route-level cases including the back-compat default of `cli=claude`), and 4 new cases in `__tests__/lib/opencode-sessions.test.ts` for `getOpenCodeSessionExport` (structure preservation, malformed-JSON fallback, unknown id, SQL-injection guard) (#297).
 
 ### Fixes
+- Make `require-*-before-stop` policies actually enforce on GitHub Copilot CLI (and add `SubagentStop` parity). Empirically verified against Copilot CLI 1.0.41 (`~/.copilot/logs/process-*.log` + `~/.copilot/session-state/<id>/events.jsonl`): exit-2 + stderr from a Stop hook on Copilot is logged as `[WARNING] Hook warning: …` and surfaced to the user, but the agent stops cleanly without retrying — exact same failure mode as Cursor/OpenCode/Pi, except Copilot at least shows the message. The documented retry shape per https://docs.github.com/en/copilot/reference/copilot-cli-reference/cli-hooks-reference is `{decision: "block", reason}` JSON on stdout (exit 0), with the reason becoming the next-turn prompt. New `cli === "copilot"` branch in `policy-evaluator.ts:267` emits that shape on Stop deny, mirroring the existing Gemini AfterAgent branch at `:188`. Without this branch, all 5 `require-*-before-stop` builtins (commit / push / PR / no-conflicts / CI-green) were observation-only on Copilot — the agent saw the policy violation in the log but happily stopped anyway. Also adds `SubagentStop` to `COPILOT_HOOK_EVENT_TYPES` so the same policies fire when Copilot subagents finish (Copilot's `subagentStop` aliases to PascalCase via the same VS Code-compat alias that maps `agentStop` ↔ `Stop`), and widens the runtime `cli === "copilot"` branch in `policy-evaluator.ts` to match both `Stop` and `SubagentStop` so custom policies subscribing to subagent boundaries get the correct retry shape too (without this, SubagentStop denies on Copilot would fall through to exit-2 — silently observation-only). 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. New unit tests pin the Copilot Stop and SubagentStop response shapes; new e2e regression confirms the integration round-trip; existing Copilot install/uninstall tests assert SubagentStop is now written. Updated `CLAUDE.md` per-CLI capability matrix and the Copilot section in `src/hooks/types.ts` with the verified Stop block semantics (#299).
 - `block-work-on-main` deny message named the wrong git subcommand for chained commands. The policy correctly trips on the `commit`/`merge`/`rebase`/`cherry-pick` part of a chained `git checkout -b feat/x && git commit -m "y"`, but the deny-message formatter ran a second, looser `/git\s+(\S+)/` regex that captured the *first* git subcommand in the string — `checkout` — producing the misleading "Git checkout on main is blocked. Create a feature branch first." log line. Reusers reasonably read this as "the policy blocks branch creation from main", which it does not. The fix changes the existing `GIT_COMMIT_MERGE_RE` capture group from non-capturing to capturing and reuses `match[1]` in the deny message, so the rendered subcommand is always the actual offender. Also corrects `docs/built-in-policies.mdx`, which described the policy as denying `git checkout` on main and listed `protectedBranches` as "branch names that cannot be checked out directly" — both wrong, and reinforced the same misconception. New regression tests cover the chained `checkout && commit` case (must name `commit`, must not name `checkout`) and lock in the user-visible guarantee that standalone `git checkout -b new-branch` on main is allowed (#296).
 - Activity dashboard: populate the `Transcript:` field across every harness, not just Claude. Only Claude's hook stdin reliably carries `transcript_path`; Codex/Copilot/Cursor don't include one, the OpenCode and Pi shims don't forward one, and Gemini's coverage is uneven across versions, so before this fix the `/policies` activity detail panel rendered `Transcript: —` for nearly every non-Claude row even though every harness *except* OpenCode has the transcript on disk and the repo already shipped per-CLI `find*Transcript(sessionId)` helpers (`lib/codex-sessions.ts`, `lib/copilot-sessions.ts`, `lib/cursor-sessions.ts`, `lib/pi-sessions.ts`, `lib/gemini-sessions.ts`). New `src/hooks/resolve-transcript-path.ts` mirrors the existing `resolve-permission-mode.ts` dispatch pattern: trust `parsed.transcript_path` from stdin first, then fall back to the per-CLI helper when sessionId is known. OpenCode (transcripts in `~/.local/share/opencode/opencode.db`, no on-disk file) gets a synthetic `opencode-db://<sessionId>` marker so the field is non-empty, distinguishable from a genuine miss, and parseable by tooling — both detail panels (`app/components/session-hooks-panel.tsx`, `app/policies/hooks-client.tsx`) render an extra muted "(stored in opencode DB)" suffix when the value carries that scheme. The handler-side fallback covers OpenCode and Pi without touching their shims (no duplicated disk-walk; the Pi shim already discovers session IDs from disk for related reasons in `pi-extension/index.ts:discoverPiSessionId`). New `__tests__/hooks/resolve-transcript-path.test.ts` is a 23-case matrix: stdin-passthrough for all 7 CLIs, missing-sessionId returns undefined for all 7, per-CLI fallback dispatch, and stdin-precedence-beats-fallback (#296).
 - Extend per-CLI tool-name canonicalization across the four CLIs PR #293 left incomplete: Copilot's `view` (used for both file reads and directory listings — empirically confirmed against Copilot CLI 1.0.39 with `{"toolName":"view","arguments":{"path":"/some/dir"}}`), Cursor's `Shell` (Cursor's name for what Claude calls `Bash`; PR #293 left Cursor as passthrough), Codex's `apply_patch` and `write_stdin` (Codex was passthrough), plus OpenCode's `apply_patch` and `websearch`. User-reported regression: under Copilot CLI, listing `$HOME` via `view` ran successfully despite an enabled `block-read-outside-cwd` policy (the same `ls -la` flow PR #293 already fixed for Bash). Adds `CURSOR_TOOL_MAP` and `CODEX_TOOL_MAP` (handler-side; mirror `COPILOT_TOOL_MAP` / `GEMINI_TOOL_MAP`) and extends `COPILOT_TOOL_MAP` with the full Copilot CLI tool surface — `view`/`show_file` → `Read`, `create` → `Write`, `apply_patch` → `Edit`, `web_fetch` → `WebFetch`, `powershell` and the eight `*_bash` / `*_powershell` session-management tools → `Bash`, `rg` → `Grep`. New e2e regression test in `__tests__/e2e/hooks/copilot-integration.e2e.test.ts` pins the `view` fix; new unit-test blocks in `__tests__/hooks/handler.test.ts` cover every Cursor and Codex map entry plus passthrough for unmapped tools (#295).

CLAUDE.md

@@ -69,6 +69,23 @@ which writes a portable `npx -y failproofai --hook ... --cli copilot` command.
 Same self-reference caveat applies — do **not** install the standard `npx`
 form from inside this repo.
 
+**Stop block semantics** (verified against Copilot CLI 1.0.41, May 2026, via
+`~/.copilot/logs/process-*.log` and `~/.copilot/session-state/<id>/events.jsonl`):
+
+| Channel                                   | Effect                                                          |
+|-------------------------------------------|-----------------------------------------------------------------|
+| `{decision: "block", reason}` JSON stdout (exit 0) | ✅ Forces another turn — `reason` becomes the next-turn prompt |
+| Exit 2 + stderr (Claude convention)       | ❌ Logged as `[WARNING] Hook warning: ...`; agent does NOT retry |
+
+policy-evaluator.ts has a `cli === "copilot"` Stop branch that emits the
+JSON-block shape so the 5 `require-*-before-stop` builtins actually enforce
+on Copilot. Without this branch, the deny would be a user-visible warning
+only — the agent would stop without remediation. Same shape applies to
+SubagentStop (Copilot fires `subagentStop` when a subagent finishes; we
+subscribe to it for parity with `agentStop`).
+
+Ref: <https://docs.github.com/en/copilot/reference/copilot-cli-reference/cli-hooks-reference>
+
 ### Cursor hooks (`.cursor/hooks.json`)
 
 This repo also ships a `.cursor/hooks.json` for Cursor Agent CLI sessions,

__tests__/e2e/helpers/hook-runner.ts

@@ -193,6 +193,17 @@ export function assertGeminiStopBlock(result: HookRunResult): void {
   expect(result.parsed?.reason).toMatch(/MANDATORY ACTION REQUIRED/);
 }
 
+export function assertCopilotStopBlock(result: HookRunResult): void {
+  // Copilot's `agentStop` honors `{decision: "block", reason}` JSON on stdout
+  // (exit 0) — the reason becomes the next-turn prompt and the agent retries.
+  // Exit-2 is logged as `[WARNING] Hook warning: ...` but does NOT trigger
+  // retry (verified empirically against Copilot CLI 1.0.41 events.jsonl).
+  expect(result.exitCode).toBe(0);
+  expect(result.parsed?.decision).toBe("block");
+  expect(typeof result.parsed?.reason).toBe("string");
+  expect(result.parsed?.reason).toMatch(/MANDATORY ACTION REQUIRED/);
+}
+
 export function assertGeminiInstruct(result: HookRunResult, hookEventName: string): void {
   expect(result.exitCode).toBe(0);
   const output = result.parsed?.hookSpecificOutput as Record<string, unknown> | undefined;

__tests__/e2e/hooks/copilot-integration.e2e.test.ts

@@ -16,6 +16,7 @@ import {
   assertAllow,
   assertPreToolUseDeny,
   assertPostToolUseDeny,
+  assertCopilotStopBlock,
 } from "../helpers/hook-runner";
 import { CopilotPayloads } from "../helpers/payloads";
 
@@ -179,6 +180,35 @@ describe("E2E: Copilot integration — hook protocol", () => {
     }
   });
 
+  // Stop hook on Copilot honors `{decision: "block", reason}` JSON, NOT
+  // exit-2+stderr (verified empirically against Copilot CLI 1.0.41:
+  // ~/.copilot/logs/process-*.log shows `[WARNING] Hook warning: <stderr>`
+  // for exit-2 with no agent retry, while the documented agentStop schema
+  // accepts `{decision:"block",reason}` to force another turn). The
+  // `cli === "copilot"` branch in policy-evaluator.ts emits this shape.
+  it("Stop deny emits {decision:'block', reason} JSON (Copilot agentStop retry shape)", () => {
+    const env = createCopilotEnv();
+    try {
+      writeConfig(env.cwd, ["require-commit-before-stop"]);
+      // require-commit-before-stop denies when the cwd has uncommitted changes.
+      // env.cwd has no .git/ at all, so the policy short-circuits to allow —
+      // we need to plant uncommitted state. Easiest: init a real git repo with
+      // a file that's modified-not-staged.
+      execSync("git init -q && git config user.email t@t && git config user.name t && touch tracked && git add tracked && git commit -q -m initial && echo dirty > tracked", {
+        cwd: env.cwd,
+        env: { ...process.env, GIT_TERMINAL_PROMPT: "0" },
+      });
+      const result = runHook(
+        "Stop",
+        CopilotPayloads.stop(env.cwd),
+        { homeDir: env.home, cli: "copilot" },
+      );
+      assertCopilotStopBlock(result);
+    } finally {
+      env.cleanup();
+    }
+  });
+
   // Regression for #295: Copilot uses a single `view` tool for both file
   // reads AND directory listings. Without `view → Read` in COPILOT_TOOL_MAP
   // the case-sensitive registry filter skips `block-read-outside-cwd`
@@ -222,6 +252,11 @@ describe("E2E: Copilot integration — install/uninstall", () => {
       expect(hooks.SessionEnd).toBeDefined();
       expect(hooks.UserPromptSubmit).toBeDefined();
       expect(hooks.Stop).toBeDefined();
+      // SubagentStop is what Copilot fires when a subagent finishes (camelCase
+      // `subagentStop` aliases to PascalCase via VS Code-compat mode). Without
+      // this entry, require-*-before-stop policies silently no-op on subagent
+      // completion — relevant for cloud-agent flows that delegate via subagents.
+      expect(hooks.SubagentStop).toBeDefined();
       // camelCase keys should not be present
       expect(hooks.preToolUse).toBeUndefined();
     } finally {

__tests__/hooks/integrations.test.ts

@@ -296,6 +296,7 @@ describe("GitHub Copilot integration", () => {
     expect(copilot.eventTypes).toContain("SessionStart");
     expect(copilot.eventTypes).toContain("SessionEnd");
     expect(copilot.eventTypes).toContain("Stop");
+    expect(copilot.eventTypes).toContain("SubagentStop");
   });
 
   it("buildHookEntry uses bash + powershell keys with --cli copilot", () => {

__tests__/hooks/policy-evaluator.test.ts

@@ -379,6 +379,69 @@ describe("hooks/policy-evaluator", () => {
       expect(result.reason).toBe("changes not committed");
     });
 
+    it("Copilot Stop deny emits {decision:'block', reason} JSON on stdout (NOT exit 2)", async () => {
+      // Copilot CLI 1.0.41 logs exit-2 from agentStop as `[WARNING] Hook
+      // warning: ...` but does NOT retry the agent. The documented retry
+      // shape is `{decision: "block", reason}` JSON on stdout (exit 0) — the
+      // reason becomes the next-turn prompt. The cli==="copilot" branch in
+      // policy-evaluator.ts emits this shape; this test pins it.
+      registerPolicy("stop-blocker", "desc", () => ({
+        decision: "deny",
+        reason: "changes not committed",
+      }), { events: ["Stop"] });
+
+      const result = await evaluatePolicies("Stop", {}, { cli: "copilot" });
+      expect(result.exitCode).toBe(0);
+      expect(result.stderr).toBe("");
+      const parsed = JSON.parse(result.stdout) as Record<string, unknown>;
+      expect(parsed.decision).toBe("block");
+      expect(parsed.reason).toContain("MANDATORY ACTION REQUIRED");
+      expect(parsed.reason).toContain("changes not committed");
+      expect(result.decision).toBe("deny");
+    });
+
+    it("Copilot SubagentStop deny also emits {decision:'block', reason} JSON (subagent retry)", async () => {
+      // CodeRabbit catch on PR #299: the cli==="copilot" branch initially only
+      // matched eventType==="Stop", so SubagentStop denies fell through to
+      // exit-2 — which Copilot ignores for stop-style events, leaving subagent
+      // policies as observation-only. Custom policies matching SubagentStop
+      // need the same JSON-block shape as Stop. (Builtin require-*-before-stop
+      // policies still match Stop only by design — they are session-completion
+      // gates, not subagent-return gates.)
+      registerPolicy("subagent-blocker", "desc", () => ({
+        decision: "deny",
+        reason: "subagent left work undone",
+      }), { events: ["SubagentStop"] });
+
+      const result = await evaluatePolicies("SubagentStop", {}, { cli: "copilot" });
+      expect(result.exitCode).toBe(0);
+      expect(result.stderr).toBe("");
+      const parsed = JSON.parse(result.stdout) as Record<string, unknown>;
+      expect(parsed.decision).toBe("block");
+      expect(parsed.reason).toContain("MANDATORY ACTION REQUIRED");
+      expect(parsed.reason).toContain("subagent left work undone");
+      expect(result.decision).toBe("deny");
+    });
+
+    it("Claude SubagentStop deny still uses exit-2+stderr (regression: non-copilot path unchanged)", async () => {
+      // The Stop branch was widened to include SubagentStop; verify the
+      // non-copilot path still emits exit-2 with the MANDATORY ACTION wrapper
+      // (was previously falling through to the bare-reason "other events"
+      // fallback at the function tail — pre-existing minor inconsistency that
+      // the widening also fixed for SubagentStop on Claude).
+      registerPolicy("subagent-blocker", "desc", () => ({
+        decision: "deny",
+        reason: "subagent left work undone",
+      }), { events: ["SubagentStop"] });
+
+      const result = await evaluatePolicies("SubagentStop", {});
+      expect(result.exitCode).toBe(2);
+      expect(result.stdout).toBe("");
+      expect(result.stderr).toContain("MANDATORY ACTION REQUIRED");
+      expect(result.stderr).toContain("subagent left work undone");
+      expect(result.decision).toBe("deny");
+    });
+
     it("Stop deny short-circuits subsequent policies", async () => {
       const secondPolicyCalled = { value: false };
       registerPolicy("blocker", "desc", () => ({

src/hooks/policy-evaluator.ts

@@ -264,11 +264,32 @@ export async function evaluatePolicies(
         };
       }
 
-      if (eventType === "Stop") {
+      if (eventType === "Stop" || eventType === "SubagentStop") {
+        const reasonText = `MANDATORY ACTION REQUIRED from failproofai (policy: ${policy.name}): ${reason}\n\nYou MUST complete the above action NOW. Do NOT ask the user for confirmation — execute the required action, then attempt to finish your task again.`;
+        // Copilot CLI: `agentStop` and `subagentStop` both honor
+        // `{decision: "block", reason}` JSON on stdout — the reason becomes the
+        // next-turn prompt and the agent (or subagent) retries. Exit-2 is logged
+        // as `[WARNING] Hook warning: ...` (verified empirically against Copilot
+        // CLI 1.0.41 events.jsonl) but does NOT trigger retry. We branch on both
+        // event types so that custom policies matching SubagentStop also enforce
+        // on Copilot subagent boundaries; the 5 builtin require-*-before-stop
+        // policies still match Stop only by design — they are session-completion
+        // gates (commit/push/PR/conflicts/CI), not subagent-return gates.
+        // Ref: https://docs.github.com/en/copilot/reference/copilot-cli-reference/cli-hooks-reference
+        if (session?.cli === "copilot") {
+          return {
+            exitCode: 0,
+            stdout: JSON.stringify({ decision: "block", reason: reasonText }),
+            stderr: "",
+            policyName: policy.name,
+            reason,
+            decision: "deny",
+          };
+        }
         return {
           exitCode: 2,
           stdout: "",
-          stderr: `MANDATORY ACTION REQUIRED from failproofai (policy: ${policy.name}): ${reason}\n\nYou MUST complete the above action NOW. Do NOT ask the user for confirmation — execute the required action, then attempt to finish your task again.`,
+          stderr: reasonText,
           policyName: policy.name,
           reason,
           decision: "deny",