Add external presenter support

AGENTS.md

@@ -133,6 +133,7 @@ claude --plugin-dir ./apps/hook
 | `PLANNOTATOR_AGENT_TERMINAL_REMOTE` | Set to `1` / `true` to enable the annotate-mode agent terminal while `PLANNOTATOR_REMOTE` is active. Off by default because remote mode binds beyond localhost. |
 | `PLANNOTATOR_PORT` | Fixed port to use. Default: random locally, `19432` for remote sessions. |
 | `PLANNOTATOR_BROWSER` | Custom browser to open plans in. macOS: app name or path. Linux/Windows: executable path. |
+| `PLANNOTATOR_PRESENTER` | Path to an executable implementing Plannotator's external presenter protocol: the server sends one JSON record on the child's stdin (`present` with the session URL and kind; `dismiss` with the handle on session stop) and reads one JSON record from stdout, presenting the review URL instead of opening the browser. Used by terminal hosts (Herdr). An explicitly-set env var wins over the config-file presenter and may run anywhere; an explicit empty value disables it. Can also be set via `~/.plannotator/config.json` (`{ "presenter": { "command": "/path/to/presenter", "when": "herdr" } }`) — config-file presenters default to `"when": "herdr"` (active only inside a Herdr pane, `HERDR_ENV=1`); set `"when": "always"` to enable everywhere. `PLANNOTATOR_SKIP_BROWSER_OPEN=1` wins over the presenter: when the host opens the URL itself, neither the presenter nor a browser is invoked. No presenter = existing browser behavior. Unset by default. |
 | `PLANNOTATOR_AI` | Set to `disabled` to disable Ask AI and the Review Agents / Guided Review execution surfaces, including provider and agent-job endpoints. Persisted guide data is retained and its server APIs remain available, but the in-app history browser is hidden while AI is disabled. External agents can still open reviews and submit annotations. The explicit annotate-mode agent terminal is separate and remains controlled by its own settings. Default: enabled. |
 | `PLANNOTATOR_SHARE` | Set to `disabled` to turn off URL sharing entirely. Default: enabled. Can also be set via `~/.plannotator/config.json` (`{ "share": "disabled" }`); the env var takes precedence. |
 | `PLANNOTATOR_SHARE_URL` | Custom base URL for share links (self-hosted portal). Default: `https://share.plannotator.ai`. |

README.md

@@ -363,6 +363,7 @@ implementation architecture.
 | `PLANNOTATOR_REMOTE` | `1`/`true` for remote mode, `0`/`false` for local, unset for SSH auto-detection |
 | `PLANNOTATOR_PORT` | Fixed port (default: random locally, `19432` remote) |
 | `PLANNOTATOR_BROWSER` | Custom browser to open plans in |
+| `PLANNOTATOR_PRESENTER` | Executable implementing the one-request JSON presenter protocol; overrides `config.json` and may run outside Herdr |
 | `PLANNOTATOR_AI` | `disabled` to disable Ask AI, Review Agents, and Guided Review; the annotate agent terminal is separate |
 | `PLANNOTATOR_SHARE` | `disabled` to turn off URL sharing |
 | `PLANNOTATOR_SHARE_URL` | Custom base URL for share links (self-hosted portal) |
@@ -378,6 +379,24 @@ All Plannotator data lives in a single directory — `~/.plannotator` by default
 export PLANNOTATOR_DATA_DIR=~/.local/share/plannotator
 ```
 
+Host integrations can persist an external presenter in
+`~/.plannotator/config.json`:
+
+```json
+{
+  "presenter": {
+    "command": "/absolute/path/to/presenter",
+    "when": "herdr"
+  }
+}
+```
+
+The command is executed directly without a shell. `when` defaults to
+`"herdr"`, so the presenter is selected only when `HERDR_ENV=1`; use
+`"always"` to enable it everywhere. An explicitly set
+`PLANNOTATOR_PRESENTER` takes priority, and an empty value disables the
+configured presenter for that invocation.
+
 ---
 
 ## Development

apps/hook/server/annotate-command.test.ts

@@ -61,6 +61,55 @@ async function runCompletion(
 }
 
 describe("completeAnnotateCommand", () => {
+  test("waits for server cleanup before publishing or exiting", async () => {
+    const events: string[] = [];
+    let finishCleanup!: () => void;
+    const cleanupFinished = new Promise<void>((resolve) => {
+      finishCleanup = resolve;
+    });
+    let confirmCleanupStarted!: () => void;
+    const cleanupStarted = new Promise<void>((resolve) => {
+      confirmCleanupStarted = resolve;
+    });
+
+    const completion = completeAnnotateCommand({
+      waitForDecision: async () => {
+        events.push("decision");
+        return { approved: true, feedback: "" };
+      },
+      settleAfterDecision: async () => {
+        events.push("settle");
+      },
+      stopServer: async () => {
+        events.push("stop:start");
+        confirmCleanupStarted();
+        await cleanupFinished;
+        events.push("stop:done");
+      },
+      requireApproval: false,
+      emitLegacyOutcome: () => {
+        events.push("legacy");
+      },
+      exit: (code) => {
+        events.push(`exit:${code}`);
+      },
+    });
+
+    await cleanupStarted;
+    expect(events).toEqual(["decision", "settle", "stop:start"]);
+
+    finishCleanup();
+    await completion;
+    expect(events).toEqual([
+      "decision",
+      "settle",
+      "stop:start",
+      "stop:done",
+      "legacy",
+      "exit:0",
+    ]);
+  });
+
   test("publishes approved feedback to matching stdout and result bytes", async () => {
     const result = await runCompletion(
       {
@@ -77,12 +126,14 @@ describe("completeAnnotateCommand", () => {
       '{"decision":"approved","feedback":"Keep the cache bounded."}\n';
     expect(result.resultBytes).toEqual([expected]);
     expect(result.stdout).toEqual([expected]);
+    // Strict publication runs BEFORE cleanup so a failing stopServer() can
+    // never erase the reviewer's completed decision record.
     expect(result.events).toEqual([
       "decision",
-      "settle",
-      "stop",
       "stdout",
       "result",
+      "settle",
+      "stop",
       "exit:0",
     ]);
   });
@@ -101,18 +152,24 @@ describe("completeAnnotateCommand", () => {
       '{"decision":"annotated","feedback":"revise"}\n',
     ]);
     expect(annotated.resultBytes).toEqual(annotated.stdout);
-    expect(annotated.events.slice(-3)).toEqual([
+    expect(annotated.events).toEqual([
+      "decision",
       "stdout",
       "result",
+      "settle",
+      "stop",
       "exit:1",
     ]);
     expect(dismissed.stdout).toEqual([
       '{"decision":"dismissed"}\n',
     ]);
     expect(dismissed.resultBytes).toEqual(dismissed.stdout);
-    expect(dismissed.events.slice(-3)).toEqual([
+    expect(dismissed.events).toEqual([
+      "decision",
       "stdout",
       "result",
+      "settle",
+      "stop",
       "exit:1",
     ]);
   });
@@ -137,13 +194,22 @@ describe("completeAnnotateCommand", () => {
       { requireApproval: true },
     );
 
-    expect(resultFileOnly.events.slice(-3)).toEqual([
+    expect(resultFileOnly.events).toEqual([
+      "decision",
       "stdout",
       "result",
+      "settle",
+      "stop",
       "exit:0",
     ]);
     expect(resultFileOnly.resultBytes).toEqual(resultFileOnly.stdout);
-    expect(approvalOnly.events.slice(-2)).toEqual(["stdout", "exit:1"]);
+    expect(approvalOnly.events).toEqual([
+      "decision",
+      "stdout",
+      "settle",
+      "stop",
+      "exit:1",
+    ]);
     expect(approvalOnly.resultBytes).toEqual([]);
   });
 
@@ -301,4 +367,50 @@ describe("completeAnnotateCommand", () => {
       await rm(directory, { recursive: true, force: true });
     }
   });
+
+  test("publishes the decision record before cleanup and exits 2 when cleanup fails", async () => {
+    const events: string[] = [];
+    const errors: string[] = [];
+    const stdout: string[] = [];
+    const resultBytes: string[] = [];
+
+    await completeAnnotateCommand({
+      waitForDecision: async () => ({ approved: true, feedback: "" }),
+      settleAfterDecision: async () => {
+        events.push("settle");
+      },
+      stopServer: async () => {
+        events.push("stop");
+        throw new Error("presenter cleanup exploded");
+      },
+      requireApproval: true,
+      resultFile: "/result.json",
+      writeResultFile: async (_path, serialized) => {
+        events.push("result");
+        resultBytes.push(`${serialized}\n`);
+      },
+      writeStdout: async (bytes) => {
+        events.push("stdout");
+        stdout.push(bytes);
+      },
+      emitLegacyOutcome: () => {
+        events.push("legacy");
+      },
+      exit: (code) => {
+        events.push(`exit:${code}`);
+      },
+      logError: (message) => {
+        errors.push(message);
+      },
+    });
+
+    // The decision record and result file are already published when cleanup
+    // rejects, and the failure routes to the documented environment-failure
+    // exit 2 — never a bare unhandled rejection, and never exit 1's "the
+    // reviewer did not approve".
+    expect(events).toEqual(["stdout", "result", "settle", "stop", "exit:2"]);
+    expect(stdout).toEqual(['{"decision":"approved"}\n']);
+    expect(resultBytes).toEqual(stdout);
+    expect(errors).toEqual(["presenter cleanup exploded"]);
+  });
 });

apps/hook/server/annotate-command.ts

@@ -9,7 +9,7 @@ import {
 export interface CompleteAnnotateCommandOptions {
   waitForDecision: () => Promise<AnnotateOutcome>;
   settleAfterDecision: () => Promise<void>;
-  stopServer: () => void;
+  stopServer: () => void | Promise<void>;
   requireApproval: boolean;
   resultFile?: string;
   writeResultFile?: (
@@ -44,34 +44,50 @@ export async function completeAnnotateCommand({
   logError = (message) => console.error(message),
 }: CompleteAnnotateCommandOptions): Promise<void> {
   const result = await waitForDecision();
-  await settleAfterDecision();
-  stopServer();
 
   if (requireApproval || resultFile) {
+    // Publish before cleanup: the reviewer's autosaved draft is already gone
+    // by the time we get here, so their completed decision must reach at
+    // least one channel before anything else can abort the run. A rejected
+    // stopServer() must not leave the process exiting without a record —
+    // under --require-approval that would masquerade as "the reviewer did
+    // not approve" instead of the documented environment-failure exit 2.
+    let exitCode: number;
     const serialized = serializeStrictAnnotateResult(result);
     try {
-      // stdout first: the reviewer's autosaved draft is already gone by the
-      // time we get here, so their completed decision must reach at least one
-      // channel before a result-file publication failure can abort the run.
-      // Result-file publication is best-effort on top of that record.
+      // stdout first; result-file publication is best-effort on top of that
+      // record.
       await outputWriter(`${serialized}\n`);
       if (resultFile) {
         await writeResultFile(resultFile, serialized);
       }
+      exitCode = annotateOutcomeExitCode(result, requireApproval);
     } catch (error) {
       // The result file was not published (or stdout itself was unwritable):
       // an environment error, not a reviewer outcome. Exit 2 — fail-closed, but
       // distinct from exit 1's "gate ran and the reviewer did not approve".
       // The stdout decision record has already been emitted unless stdout was
       // the thing that failed.
       logError(error instanceof Error ? error.message : String(error));
-      exit(STRICT_GATE_ERROR_EXIT_CODE);
-      return;
+      exitCode = STRICT_GATE_ERROR_EXIT_CODE;
+    }
+    try {
+      await settleAfterDecision();
+      await stopServer();
+    } catch (error) {
+      // Cleanup failed after the decision record was published: also an
+      // environment error, never a reviewer outcome. Exit 1 stays reserved
+      // for "the gate ran and the reviewer did not approve", and only exit 0
+      // may report approval, so a failed teardown routes to exit 2.
+      logError(error instanceof Error ? error.message : String(error));
+      exitCode = STRICT_GATE_ERROR_EXIT_CODE;
     }
-    exit(annotateOutcomeExitCode(result, requireApproval));
+    exit(exitCode);
     return;
   }
 
+  await settleAfterDecision();
+  await stopServer();
   emitLegacyOutcome(result);
   exit(0);
 }