Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,10 @@ Direct `plannotator annotate` invocations may add `--require-approval` and/or `-

Strict decisions use one newline-terminated JSON record on stdout and, when requested, identical bytes in the result file. Exit codes follow the grep convention: approval exits `0`; with `--require-approval`, annotated and dismissed decisions are published before exiting `1` (negative human outcome); usage/startup/validation failures — bad flag combinations, strict flags outside `annotate --gate --json`, a missing `--result-file` parent, a pre-existing or dangling-symlink destination, and every annotate startup failure (missing path, unreachable URL, empty folder, ambiguous name, missing file, oversized file) — exit `2` (the gate itself was misconfigured or could not start). Those startup sites exit `1` as before for non-strict invocations; under a strict flag `1` is reserved for "the reviewer did not approve", so a typo'd path must never masquerade as a rejection. Post-decision publication failures (destination appears between validation and publish, hard links unavailable) also exit `2`: the result *file* was not published, so they present as environment errors — "the gate could not publish its result" — never as a reviewer outcome, and never as approval (still fail-closed, since only `0` means approved). The stdout decision record is written **before** result-file publication and is still emitted whenever the decision itself completed; only a stdout write failure leaves no record anywhere. Signal deaths keep `128+n`. Result paths resolve from the invocation working directory, require an existing parent and absent destination, and publish via a flushed/closed `0600` same-directory temporary file plus an atomic no-clobber hard link—never copy or overwrite fallback (the `0600` mode is a no-op on Windows, and the atomic link/rename is not followed by a parent-directory fsync, so publication is atomic but not crash-durable). Keep reviewed sources at stable project paths; unique result and diagnostic log files may use a narrow temporary directory. Explicit Close emits `dismissed`; missing results or process/browser failures are recovery cases, never approval.

### Abandoned strict gate sessions

Local direct structured gates (`--gate --json`, not `--hook`, not remote) advertise a client lease in `/api/plan` and serve `/api/annotate/client-lease` (SSE, `ANNOTATE_CLIENT_LEASE_STREAM_PATH`). Each open stream is one connected review surface; the server heartbeats every 5s and, once at least one client has connected, starts a 30s reconnect grace when the last one disconnects. A reconnect inside the grace continues the same review; expiry resolves the gate as the same `dismissed` decision an explicit Close produces, except that it keeps the saved annotation draft so an abandoned review can still be recovered. Approve, feedback, explicit exit, and server stop all cancel a pending expiry. Whichever producer settles the session first wins: every one of them (each connected surface and the expiry itself) goes through a single one-shot settlement, so a decision arriving after the session already resolved is rejected with `409` rather than deleting the draft and reporting success for an outcome the caller never received. Page lifecycle events are deliberately not used: `pagehide`/`beforeunload` also fire on reload and navigation, so they cannot distinguish abandonment from a reconnect. A session that never receives its first client never auto-dismisses, so browser-launch failures still need a caller-side timeout, and remote/shared sessions keep the capability off because tunnel disconnects would read as abandonment.

## Archive Flow

```
Expand Down Expand Up @@ -400,6 +404,7 @@ During normal plan review, an Archive sidebar tab provides the same browsing via
| `/api/doc` | GET | Serve linked .md/.mdx/.html file or code file (`?path=<path>&base=<dir>`) |
| `/api/doc/exists` | POST | Batch-validate code-file paths (body: `{ paths: string[], base?: string }`) |
| `/api/draft` | GET/POST/DELETE | Auto-save annotation drafts to survive server crashes |
| `/api/annotate/client-lease` | GET (SSE) | Client lease for local direct structured gates: each open stream is one connected review surface. 404 when the capability is not advertised. |
| `/api/agent-terminal/pty/<token>` | WebSocket | Tokenized PTY bridge for the optional annotate-mode agent terminal |
| `/api/ai/capabilities` | GET | Check if AI features are available |
| `/api/ai/session` | POST | Create or fork an AI session |
Expand Down
9 changes: 9 additions & 0 deletions apps/hook/server/annotate-output.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { describe, expect, test } from "bun:test";
import {
formatAnnotateOutcome,
supportsAnnotateApprovalNotes,
supportsAnnotateClientLease,
} from "./annotate-output";

describe("annotate stdout", () => {
Expand Down Expand Up @@ -48,4 +49,12 @@ describe("annotate stdout", () => {
expect(supportsAnnotateApprovalNotes({ gate: true, json: false, hook: false })).toBe(false);
expect(supportsAnnotateApprovalNotes({ gate: true, json: true, hook: true })).toBe(false);
});

test("advertises client-lease only for gated direct JSON, local sessions", () => {
expect(supportsAnnotateClientLease({ gate: true, json: true, hook: false, isRemote: false })).toBe(true);
expect(supportsAnnotateClientLease({ gate: false, json: true, hook: false, isRemote: false })).toBe(false);
expect(supportsAnnotateClientLease({ gate: true, json: false, hook: false, isRemote: false })).toBe(false);
expect(supportsAnnotateClientLease({ gate: true, json: true, hook: true, isRemote: false })).toBe(false);
expect(supportsAnnotateClientLease({ gate: true, json: true, hook: false, isRemote: true })).toBe(false);
});
});
18 changes: 18 additions & 0 deletions apps/hook/server/annotate-output.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,11 @@ export interface AnnotateApprovalCapabilityOptions extends AnnotateOutputOptions
gate: boolean;
}

export interface AnnotateClientLeaseCapabilityOptions extends AnnotateApprovalCapabilityOptions {
/** True for remote/shared sessions, where a lost tab connection is expected and not abandonment. */
isRemote: boolean;
}

const APPROVED_PLAINTEXT_MARKER = "The user approved.";

export function supportsAnnotateApprovalNotes(
Expand All @@ -19,6 +24,19 @@ export function supportsAnnotateApprovalNotes(
return options.gate && options.json && !options.hook;
}

/**
* Local direct structured annotate gates (`--gate --json`, not `--hook`, not
* a remote/shared session) are the only transport where a tab's abandonment
* can be safely resolved automatically — the caller is already blocked on a
* structured decision and no other protocol (hook JSON, plaintext) depends on
* the exact timing of the response.
*/
export function supportsAnnotateClientLease(
options: AnnotateClientLeaseCapabilityOptions,
): boolean {
return options.gate && options.json && !options.hook && !options.isRemote;
}

export function formatAnnotateOutcome(
result: AnnotateOutcome,
options: AnnotateOutputOptions,
Expand Down
20 changes: 20 additions & 0 deletions apps/hook/server/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ import {
import {
startAnnotateServer,
handleAnnotateServerReady,
isRemoteSession,
} from "@plannotator/server/annotate";
import {
startGoalSetupServer,
Expand Down Expand Up @@ -157,6 +158,7 @@ import { buildLocalWorkspaceReview, type WorkspaceDiffType } from "@plannotator/
import {
createAnnotateOutcomeEmitter,
supportsAnnotateApprovalNotes,
supportsAnnotateClientLease,
} from "./annotate-output";

// Embed the built HTML at compile time
Expand Down Expand Up @@ -1067,6 +1069,12 @@ if (args[0] === "sessions") {
json: jsonFlag,
hook: hookFlag,
}),
clientLeaseSupported: supportsAnnotateClientLease({
gate: gateFlag,
json: jsonFlag,
hook: hookFlag,
isRemote: isRemoteSession(),
}),
rawHtml,
renderHtml: !!rawHtml,
convertHtml: renderMarkdownFlag,
Expand Down Expand Up @@ -1271,6 +1279,12 @@ if (args[0] === "sessions") {
json: jsonFlag,
hook: hookFlag,
}),
clientLeaseSupported: supportsAnnotateClientLease({
gate: gateFlag,
json: jsonFlag,
hook: hookFlag,
isRemote: isRemoteSession(),
}),
htmlContent: planHtmlContent,
recentMessages: pickerMessages,
onReady: async (url, isRemote, port) => {
Expand Down Expand Up @@ -1772,6 +1786,12 @@ if (args[0] === "sessions") {
json: jsonFlag,
hook: hookFlag,
}),
clientLeaseSupported: supportsAnnotateClientLease({
gate: gateFlag,
json: jsonFlag,
hook: hookFlag,
isRemote: isRemoteSession(),
}),
htmlContent: planHtmlContent,
onReady: async (url, isRemote, port) => {
handleAnnotateServerReady(url, isRemote, port);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,13 @@ Two caveats on the publication guarantees:

Keep the reviewed source at a stable project path so revisions and version history continue to refer to the same artifact. Result and diagnostic log files can instead live in a narrowly scoped temporary directory.

Clicking Close publishes `{"decision":"dismissed"}`. Closing or crashing the browser outside that explicit action is not guaranteed to produce a decision; callers should treat a missing result or failed process as a recovery case, never as approval.
Clicking Close publishes `{"decision":"dismissed"}`.

Abandoning the review publishes the same `dismissed` decision. A local direct structured gate tracks its connected review surfaces: once at least one has connected, losing the last one starts a 30-second reconnect grace period, and expiry resolves the gate as `dismissed`. Reloading the page, navigating away and back, or closing one of several tabs all reconnect or leave another surface connected, so none of them dismiss the review. Approve, Send Annotations, and Close still win over a pending expiry.

An abandoned review keeps its saved annotation draft, so nothing you wrote is lost. If a stale tab comes back after the gate already resolved, its Approve, Send Annotations, or Close reports an error instead of pretending to apply: the decision the caller received is the one that counts.

Two cases remain caller-side recovery, never approval: a session where no review client ever connected (a browser that failed to launch keeps waiting, so pass your own startup timeout), and a half-open transport loss, where the connection is only proven dead by a failing heartbeat write and can take longer than the grace period to notice. Remote and shared sessions keep the behavior off entirely, because a tunnel or proxy disconnect is not abandonment.

## Primary use cases

Expand Down
1 change: 1 addition & 0 deletions apps/pi-extension/plannotator-browser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -590,6 +590,7 @@ export async function startMarkdownAnnotationSession(
sourceConverted,
gate,
approvalNotesSupported: true,
clientLeaseSupported: gate === true && !isRemoteSession(),
rawHtml,
renderHtml,
convertHtml,
Expand Down
Loading