Skip to content

Commit c750427

Browse files
authored
feat(annotate): dismiss abandoned gate sessions (#1143)
A direct local `plannotator annotate --gate --json` waits for one authoritative decision. If every review surface disappears without approving, sending feedback, or exiting, the caller blocks forever: the server has no notion of whether a client ever connected, whether another tab is still open, or whether a disconnect is a reload. Page lifecycle events cannot answer that. `pagehide` and `beforeunload` also fire on reload and navigation, so dismissing from them ends reviews the user expects to resume. Use connection presence instead, which is exactly what the transport can observe. Local direct structured gates advertise a client lease in /api/plan and serve /api/annotate/client-lease as SSE. One open stream is one connected review surface. The server heartbeats every 5s and, only after 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 through the same path as explicit Close, so it produces an ordinary `dismissed` decision and inherits the strict-result contract unchanged. Approve, feedback, explicit exit, and server stop all cancel a pending expiry. Presence lives in two runtime-independent pieces so Bun and Pi cannot drift. createAnnotateClientLeaseTracker owns first-client, active-count, reconnect, cancellation, and one-shot expiry. createAnnotateClientLease- StreamSession owns one connected client: acquire the slot, write the ready comment, heartbeat, release exactly once. Each server passes only its own write primitive (a ReadableStream controller for Bun, res.write for Pi). A write that fails closes the session, because a stream that can no longer be written to is a client that is no longer present; holding the slot there would make the gate un-dismissable for the rest of the run, which is reachable only through a half-open connection and so is covered by unit tests rather than an integration test. Scope is deliberately narrow. The capability stays off for remote and shared sessions, where tunnel disconnects would read as abandonment, and off for hook transport, legacy plaintext, archive, plan, review, and folder-picker sessions. A session that never receives its first client never auto-dismisses, so browser-launch failures still need a caller-side timeout. Decision settlement is explicit for the same reason: a connected surface and the lease can both try to settle the session, and the awaited promise ignoring the second resolve was not enough. The loser still deleted the reviewer's draft and answered ok, so a tab reported success for a decision the caller never received. createAnnotateDecisionSettler makes the winner explicit; a loser changes nothing and answers 409. Expiry deliberately keeps the saved draft, unlike explicit Close, so an abandoned review stays recoverable. Stopping the server closes live lease streams instead of only releasing their slots, so a long-lived host process does not retain a heartbeat timer and an open response for every finished session.
1 parent 5d1544f commit c750427

19 files changed

Lines changed: 1745 additions & 14 deletions

AGENTS.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -276,6 +276,10 @@ Direct `plannotator annotate` invocations may add `--require-approval` and/or `-
276276

277277
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.
278278

279+
### Abandoned strict gate sessions
280+
281+
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.
282+
279283
## Archive Flow
280284

281285
```
@@ -400,6 +404,7 @@ During normal plan review, an Archive sidebar tab provides the same browsing via
400404
| `/api/doc` | GET | Serve linked .md/.mdx/.html file or code file (`?path=<path>&base=<dir>`) |
401405
| `/api/doc/exists` | POST | Batch-validate code-file paths (body: `{ paths: string[], base?: string }`) |
402406
| `/api/draft` | GET/POST/DELETE | Auto-save annotation drafts to survive server crashes |
407+
| `/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. |
403408
| `/api/agent-terminal/pty/<token>` | WebSocket | Tokenized PTY bridge for the optional annotate-mode agent terminal |
404409
| `/api/ai/capabilities` | GET | Check if AI features are available |
405410
| `/api/ai/session` | POST | Create or fork an AI session |

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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { describe, expect, test } from "bun:test";
22
import {
33
formatAnnotateOutcome,
44
supportsAnnotateApprovalNotes,
5+
supportsAnnotateClientLease,
56
} from "./annotate-output";
67

78
describe("annotate stdout", () => {
@@ -48,4 +49,12 @@ describe("annotate stdout", () => {
4849
expect(supportsAnnotateApprovalNotes({ gate: true, json: false, hook: false })).toBe(false);
4950
expect(supportsAnnotateApprovalNotes({ gate: true, json: true, hook: true })).toBe(false);
5051
});
52+
53+
test("advertises client-lease only for gated direct JSON, local sessions", () => {
54+
expect(supportsAnnotateClientLease({ gate: true, json: true, hook: false, isRemote: false })).toBe(true);
55+
expect(supportsAnnotateClientLease({ gate: false, json: true, hook: false, isRemote: false })).toBe(false);
56+
expect(supportsAnnotateClientLease({ gate: true, json: false, hook: false, isRemote: false })).toBe(false);
57+
expect(supportsAnnotateClientLease({ gate: true, json: true, hook: true, isRemote: false })).toBe(false);
58+
expect(supportsAnnotateClientLease({ gate: true, json: true, hook: false, isRemote: true })).toBe(false);
59+
});
5160
});

apps/hook/server/annotate-output.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,11 @@ export interface AnnotateApprovalCapabilityOptions extends AnnotateOutputOptions
1111
gate: boolean;
1212
}
1313

14+
export interface AnnotateClientLeaseCapabilityOptions extends AnnotateApprovalCapabilityOptions {
15+
/** True for remote/shared sessions, where a lost tab connection is expected and not abandonment. */
16+
isRemote: boolean;
17+
}
18+
1419
const APPROVED_PLAINTEXT_MARKER = "The user approved.";
1520

1621
export function supportsAnnotateApprovalNotes(
@@ -19,6 +24,19 @@ export function supportsAnnotateApprovalNotes(
1924
return options.gate && options.json && !options.hook;
2025
}
2126

27+
/**
28+
* Local direct structured annotate gates (`--gate --json`, not `--hook`, not
29+
* a remote/shared session) are the only transport where a tab's abandonment
30+
* can be safely resolved automatically — the caller is already blocked on a
31+
* structured decision and no other protocol (hook JSON, plaintext) depends on
32+
* the exact timing of the response.
33+
*/
34+
export function supportsAnnotateClientLease(
35+
options: AnnotateClientLeaseCapabilityOptions,
36+
): boolean {
37+
return options.gate && options.json && !options.hook && !options.isRemote;
38+
}
39+
2240
export function formatAnnotateOutcome(
2341
result: AnnotateOutcome,
2442
options: AnnotateOutputOptions,

apps/hook/server/index.ts

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,7 @@ import {
7878
import {
7979
startAnnotateServer,
8080
handleAnnotateServerReady,
81+
isRemoteSession,
8182
} from "@plannotator/server/annotate";
8283
import {
8384
startGoalSetupServer,
@@ -157,6 +158,7 @@ import { buildLocalWorkspaceReview, type WorkspaceDiffType } from "@plannotator/
157158
import {
158159
createAnnotateOutcomeEmitter,
159160
supportsAnnotateApprovalNotes,
161+
supportsAnnotateClientLease,
160162
} from "./annotate-output";
161163

162164
// Embed the built HTML at compile time
@@ -1067,6 +1069,12 @@ if (args[0] === "sessions") {
10671069
json: jsonFlag,
10681070
hook: hookFlag,
10691071
}),
1072+
clientLeaseSupported: supportsAnnotateClientLease({
1073+
gate: gateFlag,
1074+
json: jsonFlag,
1075+
hook: hookFlag,
1076+
isRemote: isRemoteSession(),
1077+
}),
10701078
rawHtml,
10711079
renderHtml: !!rawHtml,
10721080
convertHtml: renderMarkdownFlag,
@@ -1303,6 +1311,12 @@ if (args[0] === "sessions") {
13031311
json: jsonFlag,
13041312
hook: hookFlag,
13051313
}),
1314+
clientLeaseSupported: supportsAnnotateClientLease({
1315+
gate: gateFlag,
1316+
json: jsonFlag,
1317+
hook: hookFlag,
1318+
isRemote: isRemoteSession(),
1319+
}),
13061320
htmlContent: planHtmlContent,
13071321
recentMessages: pickerMessages,
13081322
onReady: async (url, isRemote, port) => {
@@ -1811,6 +1825,12 @@ if (args[0] === "sessions") {
18111825
json: jsonFlag,
18121826
hook: hookFlag,
18131827
}),
1828+
clientLeaseSupported: supportsAnnotateClientLease({
1829+
gate: gateFlag,
1830+
json: jsonFlag,
1831+
hook: hookFlag,
1832+
isRemote: isRemoteSession(),
1833+
}),
18141834
htmlContent: planHtmlContent,
18151835
onReady: async (url, isRemote, port) => {
18161836
handleAnnotateServerReady(url, isRemote, port);

apps/marketing/src/content/docs/guides/annotate-gates-and-json-responses.md

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,13 @@ Two caveats on the publication guarantees:
133133

134134
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.
135135

136-
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.
136+
Clicking Close publishes `{"decision":"dismissed"}`.
137+
138+
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.
139+
140+
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.
141+
142+
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.
137143

138144
## Primary use cases
139145

apps/pi-extension/plannotator-browser.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -590,6 +590,7 @@ export async function startMarkdownAnnotationSession(
590590
sourceConverted,
591591
gate,
592592
approvalNotesSupported: true,
593+
clientLeaseSupported: gate === true && !isRemoteSession(),
593594
rawHtml,
594595
renderHtml,
595596
convertHtml,

0 commit comments

Comments
 (0)