From da7dfa9eed2b53a5a4cb5d1406206a6af1a9914d Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Fri, 22 May 2026 08:10:32 -0700 Subject: [PATCH 01/35] Add session persistence foundation: suspend, reactivate, awaiting-resubmission MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Protocol: add "awaiting-resubmission" status (non-terminal), add "session-revision" event family, bump protocol version to 2. Session store: add suspend() (resolves waiters WITHOUT disposing resources — session stays alive), reactivate() (transitions back to active), and matchKey field on records. Server: skip the 2-second deletion timer on result endpoint for awaiting-resubmission sessions — they need to stay alive for the agent to resubmit. --- packages/server/daemon/server.ts | 6 ++++-- packages/server/daemon/session-store.ts | 27 +++++++++++++++++++++++++ packages/shared/daemon-protocol.ts | 4 +++- 3 files changed, 34 insertions(+), 3 deletions(-) diff --git a/packages/server/daemon/server.ts b/packages/server/daemon/server.ts index 944d22ca..15217370 100644 --- a/packages/server/daemon/server.ts +++ b/packages/server/daemon/server.ts @@ -444,8 +444,10 @@ export function createDaemonFetchHandler(options: DaemonServerOptions): DaemonFe requestContext?.disableIdleTimeout?.(); const completed = await store.waitForResult(id); const response = json({ ok: true, session: store.summary(completed), result: completed.result ?? null }); - const timer = setTimeout(() => void store.delete(id), RESULT_DELETE_GRACE_MS); - timer.unref?.(); + if (completed.status !== "awaiting-resubmission") { + const timer = setTimeout(() => void store.delete(id), RESULT_DELETE_GRACE_MS); + timer.unref?.(); + } return response; } diff --git a/packages/server/daemon/session-store.ts b/packages/server/daemon/session-store.ts index 50b1c90f..30fa8240 100644 --- a/packages/server/daemon/session-store.ts +++ b/packages/server/daemon/session-store.ts @@ -19,6 +19,7 @@ export interface DaemonSessionRecord { cwd?: string; label: string; origin?: string; + matchKey?: string; createdAt: string; updatedAt: string; expiresAt?: string; @@ -130,6 +131,7 @@ const TERMINAL_STATUSES = new Set([ "failed", ]); const TERMINAL_SESSION_TTL_MS = 60_000; +const AWAITING_RESUBMISSION_TTL_MS = 10 * 60_000; function iso(ms: number): string { return new Date(ms).toISOString(); @@ -253,6 +255,31 @@ export class DaemonSessionStore { return record; } + suspend(id: string, result: TResult, ttlMs = AWAITING_RESUBMISSION_TTL_MS): DaemonSessionRecord | undefined { + const record = this.sessions.get(id) as DaemonSessionRecord | undefined; + if (!record || record.status !== "active") return record; + record.status = "awaiting-resubmission"; + record.result = result; + const now = this.now(); + record.updatedAt = iso(now); + record.expiresAt = iso(now + ttlMs); + this.resolveWaiters(record); + this.emit("session-updated", record); + return record; + } + + reactivate(id: string): DaemonSessionRecord | undefined { + const record = this.sessions.get(id); + if (!record || record.status !== "awaiting-resubmission") return record; + record.status = "active"; + record.result = undefined; + const now = this.now(); + record.updatedAt = iso(now); + record.expiresAt = undefined; + this.emit("session-updated", record); + return record; + } + async cancel(id: string, reason = "Session cancelled."): Promise { const record = this.sessions.get(id); if (!record || TERMINAL_STATUSES.has(record.status)) return record; diff --git a/packages/shared/daemon-protocol.ts b/packages/shared/daemon-protocol.ts index d7caf790..c2b751b3 100644 --- a/packages/shared/daemon-protocol.ts +++ b/packages/shared/daemon-protocol.ts @@ -1,7 +1,7 @@ import type { PluginRequest, PluginSessionMode } from "./plugin-protocol"; export const PLANNOTATOR_DAEMON_PROTOCOL = "plannotator-daemon"; -export const PLANNOTATOR_DAEMON_PROTOCOL_VERSION = 1; +export const PLANNOTATOR_DAEMON_PROTOCOL_VERSION = 2; export const PLANNOTATOR_DAEMON_MIN_CLIENT_VERSION = 1; export const PLANNOTATOR_DAEMON_FEATURES = [ @@ -24,6 +24,7 @@ export const PLANNOTATOR_DAEMON_EVENT_FAMILIES = [ "daemon", "external-annotations", "agent-jobs", + "session-revision", ] as const; export const PLANNOTATOR_DAEMON_SESSION_VIEWS = [ @@ -40,6 +41,7 @@ export type DaemonSessionMode = PluginSessionMode; export type DaemonSessionView = (typeof PLANNOTATOR_DAEMON_SESSION_VIEWS)[number]; export type DaemonSessionStatus = | "active" + | "awaiting-resubmission" | "completed" | "cancelled" | "expired" From 167ee5f26d45622ea5e5c6e0c6cfea0fa2258485 Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Fri, 22 May 2026 08:14:24 -0700 Subject: [PATCH 02/35] Add cycle-based decision model and updateContent to plan server MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace one-shot resolveDecision with a cycle system — each deny resolves the current cycle and starts a new one. Approve resolves the final cycle. Agent-originated sessions return awaitingResubmission in the deny response. Add updateContent(newPlan) method that updates plan content, saves to version history, resets draft state, and publishes a session-revision event via the WebSocket hub. Add slug and getSnapshot to PlannotatorSession interface so the factory can match resubmissions and serve correct snapshots. Add session-revision to SessionEventFamily type. --- packages/server/index.ts | 59 ++++++++++++++++++++---------- packages/server/session-handler.ts | 2 +- 2 files changed, 41 insertions(+), 20 deletions(-) diff --git a/packages/server/index.ts b/packages/server/index.ts index 82728769..14e73883 100644 --- a/packages/server/index.ts +++ b/packages/server/index.ts @@ -118,6 +118,9 @@ export interface PlannotatorSession { waitForDecision: ServerResult["waitForDecision"]; waitForDone?: () => Promise; dispose: () => void; + slug?: string; + updateContent?: (newPlan: string) => void; + getSnapshot?: () => unknown; } // --- Server Implementation --- @@ -137,7 +140,8 @@ const RETRY_DELAY_MS = 500; export async function createPlannotatorSession( options: ServerOptions ): Promise { - const { cwd = process.cwd(), plan, origin, htmlContent, permissionMode, sharingEnabled = true, shareBaseUrl, pasteApiUrl, mode, customPlanPath } = options; + const { cwd = process.cwd(), plan: initialPlan, origin, htmlContent, permissionMode, sharingEnabled = true, shareBaseUrl, pasteApiUrl, mode, customPlanPath } = options; + let plan = initialPlan; const resolvePlanStoragePath = (customPath?: string | null): string | undefined => { if (!customPath?.trim()) return undefined; return resolveUserPath(customPath, cwd); @@ -166,7 +170,7 @@ export async function createPlannotatorSession( } // --- Plan review mode setup (skip in archive mode) --- - const draftKey = mode !== "archive" ? contentHash(plan) : ""; + let draftKey = mode !== "archive" ? contentHash(plan) : ""; const editorAnnotations = mode !== "archive" ? createEditorAnnotationHandler() : null; const externalAnnotations = mode !== "archive" ? createExternalAnnotationHandler("plan", { publishEvent: (event) => options.sessionEvents?.publishEvent("external-annotations", event), @@ -185,20 +189,19 @@ export async function createPlannotatorSession( let previousPlan: string | null = null; let versionInfo = { version: 0, totalVersions: 0, project: "" }; - let resolveDecision: (result: { + type DecisionResult = { approved: boolean; feedback?: string; savedPath?: string; agentSwitch?: string; permissionMode?: string; - }) => void; - let decisionPromise: Promise<{ - approved: boolean; - feedback?: string; - savedPath?: string; - agentSwitch?: string; - permissionMode?: string; - }>; + }; + let currentCycle: { promise: Promise; resolve: (result: DecisionResult) => void }; + function startNewCycle() { + let resolve: (result: DecisionResult) => void; + const promise = new Promise((r) => { resolve = r; }); + currentCycle = { promise, resolve: resolve! }; + } if (mode !== "archive") { repoInfo = await getRepoInfo(cwd); @@ -215,12 +218,10 @@ export async function createPlannotatorSession( project, }; - decisionPromise = new Promise((resolve) => { - resolveDecision = resolve; - }); + startNewCycle(); } else { // Never-resolving promise — archive mode uses waitForDone instead - decisionPromise = new Promise(() => {}); + startNewCycle(); } const handleRequest: SessionRequestHandler = async (req, url, context) => { @@ -541,7 +542,7 @@ export async function createPlannotatorSession( // Use permission mode from client request if provided, otherwise fall back to hook input const effectivePermissionMode = requestedPermissionMode || permissionMode; - resolveDecision({ approved: true, feedback, savedPath, agentSwitch, permissionMode: effectivePermissionMode }); + currentCycle.resolve({ approved: true, feedback, savedPath, agentSwitch, permissionMode: effectivePermissionMode }); return Response.json({ ok: true, savedPath }); } @@ -578,8 +579,10 @@ export async function createPlannotatorSession( } deleteDraft(draftKey); - resolveDecision({ approved: false, feedback, savedPath }); - return Response.json({ ok: true, savedPath }); + const isAgentSession = !!origin; + currentCycle.resolve({ approved: false, feedback, savedPath }); + if (isAgentSession) startNewCycle(); + return Response.json({ ok: true, savedPath, ...(isAgentSession && { awaitingResubmission: true }) }); } // Favicon @@ -594,11 +597,29 @@ export async function createPlannotatorSession( return { htmlContent, handleRequest, - waitForDecision: () => decisionPromise, + waitForDecision: () => currentCycle.promise, ...(donePromise && { waitForDone: () => donePromise }), dispose: () => { externalAnnotations?.dispose(); }, + slug: mode !== "archive" ? slug : undefined, + getSnapshot: mode !== "archive" ? () => ({ plan, origin }) : undefined, + updateContent: mode !== "archive" ? (newPlan: string) => { + plan = newPlan; + const historyResult = saveToHistory(project, slug, newPlan); + currentPlanPath = historyResult.path; + previousPlan = historyResult.version > 1 + ? getPlanVersion(project, slug, historyResult.version - 1) + : null; + versionInfo = { + version: historyResult.version, + totalVersions: getVersionCount(project, slug), + project, + }; + deleteDraft(draftKey); + draftKey = contentHash(newPlan); + options.sessionEvents?.publishEvent("session-revision", { plan: newPlan, previousPlan, versionInfo }); + } : undefined, }; } diff --git a/packages/server/session-handler.ts b/packages/server/session-handler.ts index f8bfcd57..98749c7f 100644 --- a/packages/server/session-handler.ts +++ b/packages/server/session-handler.ts @@ -3,7 +3,7 @@ export interface SessionRequestContext { upgradeWebSocket?: (data: unknown) => Response | undefined; } -export type SessionEventFamily = "external-annotations" | "agent-jobs"; +export type SessionEventFamily = "external-annotations" | "agent-jobs" | "session-revision"; export type SessionEventPublisher = ( family: SessionEventFamily, From e5afae8281b4a865b52d9db1ced49f139ebd83a1 Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Fri, 22 May 2026 08:52:07 -0700 Subject: [PATCH 03/35] Add session matching, persistent decision loop, and CLI support MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Session factory: add sessionRefs registry and findAwaitingSession() matching by matchKey. Plan sessions compute matchKey as plan:project:slug. On resubmission, existing awaiting session is matched and reactivated instead of creating a new one. Add registerPersistentDecision() — loops on waitForDecision(), suspending on deny and completing on approve. Replaces one-shot registerSessionDecision for plan sessions. Fix unhandled promise rejection: add .catch() to disposed promise in both registerSessionDecision and registerPersistentDecision. CLI: accept "awaiting-resubmission" as valid non-error status so denied sessions output feedback and exit 0 instead of failing. --- apps/hook/server/index.ts | 2 +- packages/server/daemon/session-factory.ts | 68 ++++++++++++++++++++++- packages/server/daemon/session-store.ts | 2 + 3 files changed, 68 insertions(+), 4 deletions(-) diff --git a/apps/hook/server/index.ts b/apps/hook/server/index.ts index b189d120..53a6a71d 100644 --- a/apps/hook/server/index.ts +++ b/apps/hook/server/index.ts @@ -689,7 +689,7 @@ async function runDaemonSessionRequest(request: PluginRequest, options: { plugin await cancelCreatedSession(); fail(completed.error.code, completed.error.message); } - if (completed.session.status !== "completed") { + if (completed.session.status !== "completed" && completed.session.status !== "awaiting-resubmission") { fail( completed.session.status, completed.session.error ?? `Plannotator session ${completed.session.id} ended with status ${completed.session.status}.`, diff --git a/packages/server/daemon/session-factory.ts b/packages/server/daemon/session-factory.ts index eca9b02e..0c07a1ae 100644 --- a/packages/server/daemon/session-factory.ts +++ b/packages/server/daemon/session-factory.ts @@ -24,7 +24,8 @@ import type { PluginReviewRequest, } from "@plannotator/shared/plugin-protocol"; import { normalizeGoalSetupBundle } from "@plannotator/shared/goal-setup"; -import { createPlannotatorSession } from "../index"; +import { createPlannotatorSession, type PlannotatorSession } from "../index"; +import { generateSlug } from "../storage"; import { createAnnotateSession } from "../annotate"; import { createGoalSetupSession } from "../goal-setup"; import { createReviewSession } from "../review"; @@ -107,6 +108,7 @@ function registerSessionDecision( const disposed = new Promise((_, reject) => { releaseDecisionWait = () => reject(new Error("Session disposed.")); }); + disposed.catch(() => {}); void Promise.race([waitForDecision(), disposed]) .then((result) => context.store.complete(id, mapResult(result))) @@ -123,6 +125,42 @@ function registerSessionDecision( }; } +function registerPersistentDecision( + context: DaemonFetchContext, + id: string, + session: { waitForDecision: () => Promise<{ approved: boolean; [key: string]: unknown }>; dispose: () => void | Promise }, +): () => void | Promise { + let releaseDecisionWait: (() => void) | undefined; + const disposed = new Promise((_, reject) => { + releaseDecisionWait = () => reject(new Error("Session disposed.")); + }); + disposed.catch(() => {}); + + const decisionLoop = async () => { + while (true) { + const result = await Promise.race([session.waitForDecision(), disposed]); + if (result.approved) { + context.store.complete(id, result); + return; + } + context.store.suspend(id, result); + } + }; + + void decisionLoop().catch((err) => { + const record = context.store.get(id); + if (record && record.status === "active" || record?.status === "awaiting-resubmission") { + context.store.fail(id, err instanceof Error ? err.message : String(err)); + } + }); + + return () => { + releaseDecisionWait?.(); + releaseDecisionWait = undefined; + return session.dispose(); + }; +} + function resolvePlanFilePath(planFilePath: string, cwd: string): string { const requestedPath = isAbsolute(planFilePath) ? planFilePath @@ -482,6 +520,19 @@ async function prepareReviewInput(request: PluginReviewRequest, cwd: string) { } export function createDaemonSessionFactory(options: DaemonSessionFactoryOptions) { + const sessionRefs = new Map(); + + function findAwaitingSession(store: DaemonFetchContext["store"], matchKey: string) { + for (const [sessionId, ref] of sessionRefs) { + const record = store.get(sessionId); + if (!record) { sessionRefs.delete(sessionId); continue; } + if (record.status !== "awaiting-resubmission") continue; + if (record.matchKey !== matchKey) continue; + return { record, session: ref.session }; + } + return null; + } + return async function createSession( createRequest: DaemonCreateSessionRequest, context: DaemonFetchContext, @@ -513,6 +564,15 @@ export function createDaemonSessionFactory(options: DaemonSessionFactoryOptions) if (request.action === "plan") { const plan = await readPlanRequest(request, cwd); + const matchKey = `plan:${project}:${generateSlug(plan)}`; + + const existing = findAwaitingSession(context.store, matchKey); + if (existing && existing.session.updateContent) { + existing.session.updateContent(plan); + context.store.reactivate(existing.record.id); + return existing.record; + } + const remoteShare = context.endpoint.isRemote && sharingEnabled ? await createRemoteShareNotice(plan, shareBaseUrl, "review the plan", "plan only").catch(() => undefined) : undefined; @@ -538,12 +598,14 @@ export function createDaemonSessionFactory(options: DaemonSessionFactoryOptions) cwd, label: branch ? `plugin-plan-${request.origin}-${project}-${branch}` : `plugin-plan-${request.origin}-${project}`, origin: request.origin, + matchKey, ttlMs, handleRequest: session.handleRequest, - dispose: registerSessionDecision(context, id, () => session.waitForDecision(), () => session.dispose()), + dispose: registerPersistentDecision(context, id, session), remoteShare, - snapshot: () => ({ plan, origin: request.origin }), + snapshot: session.getSnapshot ?? (() => ({ plan, origin: request.origin })), }); + sessionRefs.set(id, { matchKey, session }); return record; } diff --git a/packages/server/daemon/session-store.ts b/packages/server/daemon/session-store.ts index 30fa8240..a91ecb00 100644 --- a/packages/server/daemon/session-store.ts +++ b/packages/server/daemon/session-store.ts @@ -41,6 +41,7 @@ export interface CreateDaemonSessionInput { cwd?: string; label: string; origin?: string; + matchKey?: string; ttlMs?: number; now?: number; htmlContent?: string; @@ -172,6 +173,7 @@ export class DaemonSessionStore { label: input.label, ...(input.cwd && { cwd: input.cwd }), ...(input.origin && { origin: input.origin }), + ...(input.matchKey && { matchKey: input.matchKey }), createdAt: iso(now), updatedAt: iso(now), ...(input.ttlMs !== undefined && { expiresAt: iso(now + input.ttlMs) }), From d1e80b9d89fcfd431a1068ad16fd0a2df2a9b11e Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Fri, 22 May 2026 08:55:10 -0700 Subject: [PATCH 04/35] Add awaiting-resubmission UI state to plan review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extend CompletionBanner with 'awaiting' variant — amber spinner with "Feedback sent — waiting for agent to revise..." message and optional cancel button. Plan review deny handler now checks response for awaitingResubmission flag. When true, shows the awaiting banner instead of the completion overlay. Subscribe to session-revision events via daemon WebSocket. When the agent resubmits and the session reactivates, the event carries the new plan content. The UI updates: markdown refreshes, previousPlan updates for diff, all annotations clear, awaiting state resets. --- packages/plannotator-plan-review/App.tsx | 43 +++++++++++++++-- packages/ui/components/CompletionBanner.tsx | 53 ++++++++++++++------- 2 files changed, 76 insertions(+), 20 deletions(-) diff --git a/packages/plannotator-plan-review/App.tsx b/packages/plannotator-plan-review/App.tsx index 55992b33..90088cfb 100644 --- a/packages/plannotator-plan-review/App.tsx +++ b/packages/plannotator-plan-review/App.tsx @@ -59,6 +59,7 @@ import { useArchive } from '@plannotator/ui/hooks/useArchive'; import { useEditorAnnotations } from '@plannotator/ui/hooks/useEditorAnnotations'; import { useExternalAnnotations } from '@plannotator/ui/hooks/useExternalAnnotations'; import { useExternalAnnotationHighlights } from '@plannotator/ui/hooks/useExternalAnnotationHighlights'; +import { subscribeToDaemonSessionFamily } from '@plannotator/ui/utils/daemonHub'; import { buildPlanAgentInstructions } from '@plannotator/ui/utils/planAgentInstructions'; import { useFileBrowser } from '@plannotator/ui/hooks/useFileBrowser'; import { isVaultBrowserEnabled } from '@plannotator/ui/utils/obsidian'; @@ -184,6 +185,7 @@ const App: React.FC<{ __embedded?: boolean; headerLeft?: React.ReactNode; onOpen const [isSubmitting, setIsSubmitting] = useState(false); const [isExiting, setIsExiting] = useState(false); const [submitted, setSubmitted] = useState<'approved' | 'denied' | 'exited' | null>(null); + const [awaitingResubmission, setAwaitingResubmission] = useState(false); const [pendingPasteImage, setPendingPasteImage] = useState<{ file: File; blobUrl: string; initialName: string } | null>(null); const [showPermissionModeSetup, setShowPermissionModeSetup] = useState(false); const [permissionMode, setPermissionMode] = useState('bypassPermissions'); @@ -588,6 +590,29 @@ const App: React.FC<{ __embedded?: boolean; headerLeft?: React.ReactNode; onOpen const { editorAnnotations, deleteEditorAnnotation } = useEditorAnnotations(); const { externalAnnotations, updateExternalAnnotation, deleteExternalAnnotation } = useExternalAnnotations({ enabled: isApiMode && !goalSetupMode }); + // Listen for session-revision events (plan resubmission after deny) + useEffect(() => { + if (!isApiMode || !awaitingResubmission) return; + const unsubscribe = subscribeToDaemonSessionFamily("session-revision", (msg) => { + if (msg.type !== "event" || !msg.payload) return; + const revision = msg.payload as { plan?: string; previousPlan?: string | null; versionInfo?: { version: number; totalVersions: number; project: string } }; + if (revision.plan) { + setMarkdown(revision.plan); + if (revision.previousPlan !== undefined) setPreviousPlan(revision.previousPlan); + if (revision.versionInfo) setVersionInfo(revision.versionInfo); + setAnnotations([]); + setCodeAnnotations([]); + setGlobalAttachments([]); + setSelectedAnnotationId(null); + setSelectedCodeAnnotationId(null); + setAwaitingResubmission(false); + setSubmitted(null); + setIsSubmitting(false); + } + }); + return unsubscribe; + }, [isApiMode, awaitingResubmission]); + // Drive DOM highlights for SSE-delivered external annotations. Disabled // while a linked doc overlay is open (Viewer DOM is hidden) and while the // plan diff view is active (diff view has its own annotation surface). @@ -1077,7 +1102,7 @@ const App: React.FC<{ __embedded?: boolean; headerLeft?: React.ReactNode; onOpen setIsSubmitting(true); try { const planSaveSettings = getPlanSaveSettings(); - await fetch('/api/deny', { + const response = await fetch('/api/deny', { method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ @@ -1088,7 +1113,13 @@ const App: React.FC<{ __embedded?: boolean; headerLeft?: React.ReactNode; onOpen }, }) }); - setSubmitted('denied'); + const data = await response.json().catch(() => ({})); + if (data.awaitingResubmission) { + setAwaitingResubmission(true); + setIsSubmitting(false); + } else { + setSubmitted('denied'); + } } catch { setIsSubmitting(false); } @@ -1803,7 +1834,13 @@ const App: React.FC<{ __embedded?: boolean; headerLeft?: React.ReactNode; onOpen /> {/* Embedded completion banner — inline, non-blocking (skipped in legacy tab mode) */} - {__embedded && !legacyTabMode && } + {__embedded && !legacyTabMode && ( + + )} {/* Linked document error banner */} {linkedDocHook.error && ( diff --git a/packages/ui/components/CompletionBanner.tsx b/packages/ui/components/CompletionBanner.tsx index 6f9c68b9..894b825f 100644 --- a/packages/ui/components/CompletionBanner.tsx +++ b/packages/ui/components/CompletionBanner.tsx @@ -1,37 +1,56 @@ interface CompletionBannerProps { - submitted: 'approved' | 'denied' | 'feedback' | 'exited' | null | false; + submitted: 'approved' | 'denied' | 'feedback' | 'exited' | 'awaiting' | null | false; title: string; subtitle: string; + onCancel?: () => void; } -export function CompletionBanner({ submitted, title, subtitle }: CompletionBannerProps) { +export function CompletionBanner({ submitted, title, subtitle, onCancel }: CompletionBannerProps) { if (!submitted) return null; const isApproved = submitted === 'approved'; + const isAwaiting = submitted === 'awaiting'; return (
- - {isApproved ? ( - - ) : ( - - )} - -
+ {isAwaiting ? ( + + + + ) : ( + + {isApproved ? ( + + ) : ( + + )} + + )} +
{title} {subtitle}
+ {isAwaiting && onCancel && ( + + )}
); } From 27bf0ff7dd09978c9bff777e94b80fc3ad37ca99 Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Fri, 22 May 2026 08:56:22 -0700 Subject: [PATCH 05/35] Document session persistence: AGENTS.md and backlog updates Add session persistence and resubmission section to AGENTS.md covering the awaiting-resubmission status, matchKey matching, and session-revision event family. Update backlog: mark #3 (live plan updates) and #4 (session persistence after completion) as done. --- AGENTS.md | 12 +++++++-- goals/frontend-session-lifecycle/backlog.md | 27 +++------------------ 2 files changed, 14 insertions(+), 25 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 46baf6ae..1dafcdc9 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -245,11 +245,19 @@ The daemon is the single long-running Bun server used by normal plan/review/anno | `/daemon/projects/prs` | GET | List open PRs for a project (`?cwd=`) | | `/daemon/projects/prs/detailed` | GET | List PRs with review metadata for dashboard (`?cwd=`) | | `/daemon/fs/list` | GET | List directory contents (`?path=`) | -| `/daemon/ws` | WebSocket | Multiplex daemon lifecycle events, session-scoped external annotation events, agent job events, and correlated session actions | +| `/daemon/ws` | WebSocket | Multiplex daemon lifecycle events, session-scoped external annotation events, agent job events, session revision events, and correlated session actions | | `/s/:id` | GET | Serve the browser HTML for a session | | `/s/:id/api/...` | Any | Route browser API requests to that session's plan/review/annotate handler | -Runtime live updates for daemon lifecycle events, external annotations, and agent jobs are delivered through `/daemon/ws`. Session-scoped updates subscribe by `{ family, sessionId }`. HTTP endpoints below remain for snapshots, mutations, uploads, and large payloads. AI query token streaming remains on `/api/ai/query`. +Runtime live updates for daemon lifecycle events, external annotations, agent jobs, and session revisions are delivered through `/daemon/ws`. Session-scoped updates subscribe by `{ family, sessionId }`. HTTP endpoints below remain for snapshots, mutations, uploads, and large payloads. AI query token streaming remains on `/api/ai/query`. + +### Session Persistence and Resubmission + +When a user denies a plan (or sends feedback on a review/annotation), the session enters `awaiting-resubmission` status instead of completing. The session's HTTP handler stays alive. When the agent replans and submits again via `POST /daemon/sessions`, the daemon matches the new submission to the existing session by a match key (`plan:project:slug` for plans, `review:project:branch` for reviews, `annotate:filepath` for annotations). The session reactivates in place — the frontend receives a `session-revision` event via WebSocket with the updated content. + +**Session statuses:** `active` → `awaiting-resubmission` (on deny) → `active` (on resubmit) → `completed` (on approve). The `awaiting-resubmission` status is non-terminal — the session stays routable and its handler keeps serving requests. If the agent doesn't resubmit within 10 minutes, the session expires. + +**Event families:** `daemon`, `external-annotations`, `agent-jobs`, `session-revision`. ### Plan Server (`packages/server/index.ts`) diff --git a/goals/frontend-session-lifecycle/backlog.md b/goals/frontend-session-lifecycle/backlog.md index 3ba79516..c6f16eff 100644 --- a/goals/frontend-session-lifecycle/backlog.md +++ b/goals/frontend-session-lifecycle/backlog.md @@ -26,34 +26,15 @@ Some users will prefer each session opening in a new browser tab with auto-close --- -## 3. Live plan updates across deny/replan cycles +## ~~3. Live plan updates across deny/replan cycles~~ DONE -**Priority:** High — most-requested feature -**Size:** Large - -When the agent resubmits a plan after denial, the existing session should reactivate in-place rather than spawning a new session. - -**Desired behavior:** -- User denies plan, sends feedback, agent replans -- Agent calls ExitPlanMode again — daemon matches it to the existing session -- Session status flips from "completed" back to "active" -- Frontend receives a push notification via WebSocket -- Plan diff system shows what changed between versions -- The plan→deny→replan→approve cycle happens in one persistent session - -**Open questions:** -- How does the daemon match a new plan submission to an existing session? By project + plan slug? By a correlation ID from the agent? -- Does the session status reset to "active" on resubmission, or show "updated"? -- How does this interact with the version history system already in `~/.plannotator/history/`? +Implemented in `feat/session-persistence`. Sessions enter `awaiting-resubmission` status on deny. Agent resubmission is matched by `plan:project:slug` and the session reactivates in place. Frontend receives `session-revision` WebSocket event with updated content. --- -## 4. Session persistence after completion - -**Priority:** High — current behavior is broken -**Size:** Medium, tied to #3 +## ~~4. Session persistence after completion~~ DONE -**Current bug:** When a session is approved/denied, the daemon disposes the session handler. The session disappears from the sidebar even though the route still resolves. API calls fail, so the plan content is gone. +Implemented in `feat/session-persistence`. Denied sessions stay alive (handler not disposed) in `awaiting-resubmission` state with a 10-minute TTL. The session remains routable and serves API requests while waiting. **Required behavior:** - Completed sessions stay in the sidebar with a status badge (approved/denied) From f5af86ae5b8ac72097e994e601930e92d3d68df7 Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Fri, 22 May 2026 08:59:30 -0700 Subject: [PATCH 06/35] Self-review fixes: operator precedence bug and sessionRefs cleanup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix operator precedence in registerPersistentDecision catch handler — was evaluating as (A && B) || C instead of A && (B || C). Extend findAwaitingSession to prune completed/expired/failed/cancelled entries from sessionRefs map, preventing slow memory growth over daemon lifetime. --- packages/server/daemon/session-factory.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/server/daemon/session-factory.ts b/packages/server/daemon/session-factory.ts index 0c07a1ae..c48f7e7e 100644 --- a/packages/server/daemon/session-factory.ts +++ b/packages/server/daemon/session-factory.ts @@ -149,7 +149,7 @@ function registerPersistentDecision( void decisionLoop().catch((err) => { const record = context.store.get(id); - if (record && record.status === "active" || record?.status === "awaiting-resubmission") { + if (record && (record.status === "active" || record.status === "awaiting-resubmission")) { context.store.fail(id, err instanceof Error ? err.message : String(err)); } }); @@ -525,7 +525,10 @@ export function createDaemonSessionFactory(options: DaemonSessionFactoryOptions) function findAwaitingSession(store: DaemonFetchContext["store"], matchKey: string) { for (const [sessionId, ref] of sessionRefs) { const record = store.get(sessionId); - if (!record) { sessionRefs.delete(sessionId); continue; } + if (!record || record.status === "completed" || record.status === "expired" || record.status === "failed" || record.status === "cancelled") { + sessionRefs.delete(sessionId); + continue; + } if (record.status !== "awaiting-resubmission") continue; if (record.matchKey !== matchKey) continue; return { record, session: ref.session }; From 83344d15c0dc8954d8d6085c5cfc99eea8dc41fb Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Fri, 22 May 2026 10:19:21 -0700 Subject: [PATCH 07/35] Wire all three session types for persistence Extract createDecisionScope helper to eliminate duplication between registerSessionDecision and registerPersistentDecision. Define SessionDecisionResult type for explicit contracts. Annotate server: cycle-based decisions, updateContent for file-based modes, awaitingResubmission in /api/feedback response. Review server: cycle-based decisions, updateContent(rawPatch, gitRef), awaitingResubmission for non-approved feedback. Factory: generalize sessionRefs to PersistableSession interface. Add matchKey computation and matching for annotate (annotate:filepath) and review (review:project:branch or review:prUrl). Wire both with registerPersistentDecision. URL and annotate-last modes keep one-shot behavior (no persistent source to refresh). --- packages/server/annotate.ts | 52 +++++++----- packages/server/daemon/session-factory.ts | 99 ++++++++++++++++------- packages/server/review.ts | 54 ++++++++----- 3 files changed, 130 insertions(+), 75 deletions(-) diff --git a/packages/server/annotate.ts b/packages/server/annotate.ts index aaa6155c..2fcfba36 100644 --- a/packages/server/annotate.ts +++ b/packages/server/annotate.ts @@ -92,6 +92,8 @@ export interface AnnotateSession { handleRequest: SessionRequestHandler; waitForDecision: AnnotateServerResult["waitForDecision"]; dispose: () => void; + updateContent?: (newMarkdown: string) => void; + getSnapshot?: () => unknown; } // --- Server Implementation --- @@ -104,7 +106,7 @@ export async function createAnnotateSession( ): Promise { const { cwd = process.cwd(), - markdown, + markdown: initialMarkdown, filePath, htmlContent, origin, @@ -119,6 +121,7 @@ export async function createAnnotateSession( rawHtml, renderHtml = false, } = options; + let markdown = initialMarkdown; // Side-channel pre-warm so /api/doc/exists POSTs land on warm cache. void warmFileListCache(cwd, "code"); @@ -129,7 +132,7 @@ export async function createAnnotateSession( mode === "annotate-folder" && folderPath ? `folder:${resolvePath(folderPath)}` : renderHtml && rawHtml ? rawHtml : markdown; - const draftKey = contentHash(draftSource); + let draftKey = contentHash(draftSource); const externalAnnotations = createExternalAnnotationHandler("plan", { publishEvent: (event) => options.sessionEvents?.publishEvent("external-annotations", event), registerSnapshotProvider: (provider) => @@ -139,21 +142,15 @@ export async function createAnnotateSession( // Detect repo info (cached for this session) const repoInfo = await getRepoInfo(cwd); - // Decision promise - let resolveDecision: (result: { - feedback: string; - annotations: unknown[]; - exit?: boolean; - approved?: boolean; - }) => void; - const decisionPromise = new Promise<{ - feedback: string; - annotations: unknown[]; - exit?: boolean; - approved?: boolean; - }>((resolve) => { - resolveDecision = resolve; - }); + // Decision cycle — each deny/feedback starts a new cycle; approve/exit is final + type AnnotateDecisionResult = { feedback: string; annotations: unknown[]; exit?: boolean; approved?: boolean }; + let currentCycle: { promise: Promise; resolve: (result: AnnotateDecisionResult) => void }; + function startNewCycle() { + let resolve: (result: AnnotateDecisionResult) => void; + const promise = new Promise((r) => { resolve = r; }); + currentCycle = { promise, resolve: resolve! }; + } + startNewCycle(); const handleRequest: SessionRequestHandler = async (req, url, context) => { @@ -258,14 +255,14 @@ export async function createAnnotateSession( // API: Exit annotation session without feedback if (url.pathname === "/api/exit" && req.method === "POST") { deleteDraft(draftKey); - resolveDecision({ feedback: "", annotations: [], exit: true }); + currentCycle.resolve({ feedback: "", annotations: [], exit: true }); return Response.json({ ok: true }); } // API: Approve the annotation session (review-gate UX, #570) if (url.pathname === "/api/approve" && req.method === "POST") { deleteDraft(draftKey); - resolveDecision({ feedback: "", annotations: [], approved: true }); + currentCycle.resolve({ feedback: "", annotations: [], approved: true }); return Response.json({ ok: true }); } @@ -278,12 +275,14 @@ export async function createAnnotateSession( }; deleteDraft(draftKey); - resolveDecision({ + const isAgentSession = !!origin; + currentCycle.resolve({ feedback: body.feedback || "", annotations: body.annotations || [], }); + if (isAgentSession) startNewCycle(); - return Response.json({ ok: true }); + return Response.json({ ok: true, ...(isAgentSession && { awaitingResubmission: true }) }); } catch (err) { const message = err instanceof Error @@ -302,13 +301,22 @@ export async function createAnnotateSession( }); }; + const isFileBased = mode === "annotate" || mode === "annotate-folder"; + return { htmlContent, handleRequest, - waitForDecision: () => decisionPromise, + waitForDecision: () => currentCycle.promise, dispose: () => { externalAnnotations.dispose(); }, + getSnapshot: isFileBased ? () => ({ plan: markdown, filePath, mode, sourceInfo }) : undefined, + updateContent: isFileBased ? (newMarkdown: string) => { + markdown = newMarkdown; + deleteDraft(draftKey); + draftKey = contentHash(newMarkdown); + options.sessionEvents?.publishEvent("session-revision", { plan: newMarkdown }); + } : undefined, }; } diff --git a/packages/server/daemon/session-factory.ts b/packages/server/daemon/session-factory.ts index c48f7e7e..d1df40e0 100644 --- a/packages/server/daemon/session-factory.ts +++ b/packages/server/daemon/session-factory.ts @@ -97,6 +97,26 @@ function createSessionEventBridge(context: DaemonFetchContext, id: string): Sess }; } +interface SessionDecisionResult { + approved?: boolean; + feedback?: string; + [key: string]: unknown; +} + +function createDecisionScope(dispose: () => void | Promise) { + let releaseDecisionWait: (() => void) | undefined; + const disposed = new Promise((_, reject) => { + releaseDecisionWait = () => reject(new Error("Session disposed.")); + }); + disposed.catch(() => {}); + const cleanup = () => { + releaseDecisionWait?.(); + releaseDecisionWait = undefined; + return dispose(); + }; + return { disposed, cleanup }; +} + function registerSessionDecision( context: DaemonFetchContext, id: string, @@ -104,11 +124,7 @@ function registerSessionDecision( dispose: () => void | Promise, mapResult: (result: TResult) => TStored = (result) => result as unknown as TStored, ): () => void | Promise { - let releaseDecisionWait: (() => void) | undefined; - const disposed = new Promise((_, reject) => { - releaseDecisionWait = () => reject(new Error("Session disposed.")); - }); - disposed.catch(() => {}); + const { disposed, cleanup } = createDecisionScope(dispose); void Promise.race([waitForDecision(), disposed]) .then((result) => context.store.complete(id, mapResult(result))) @@ -118,23 +134,15 @@ function registerSessionDecision( } }); - return () => { - releaseDecisionWait?.(); - releaseDecisionWait = undefined; - return dispose(); - }; + return cleanup; } function registerPersistentDecision( context: DaemonFetchContext, id: string, - session: { waitForDecision: () => Promise<{ approved: boolean; [key: string]: unknown }>; dispose: () => void | Promise }, + session: { waitForDecision: () => Promise; dispose: () => void | Promise }, ): () => void | Promise { - let releaseDecisionWait: (() => void) | undefined; - const disposed = new Promise((_, reject) => { - releaseDecisionWait = () => reject(new Error("Session disposed.")); - }); - disposed.catch(() => {}); + const { disposed, cleanup } = createDecisionScope(session.dispose); const decisionLoop = async () => { while (true) { @@ -154,11 +162,7 @@ function registerPersistentDecision( } }); - return () => { - releaseDecisionWait?.(); - releaseDecisionWait = undefined; - return session.dispose(); - }; + return cleanup; } function resolvePlanFilePath(planFilePath: string, cwd: string): string { @@ -520,7 +524,13 @@ async function prepareReviewInput(request: PluginReviewRequest, cwd: string) { } export function createDaemonSessionFactory(options: DaemonSessionFactoryOptions) { - const sessionRefs = new Map(); + interface PersistableSession { + waitForDecision: () => Promise; + dispose: () => void | Promise; + // eslint-disable-next-line @typescript-eslint/no-explicit-any + updateContent?: (...args: any[]) => void; + } + const sessionRefs = new Map(); function findAwaitingSession(store: DaemonFetchContext["store"], matchKey: string) { for (const [sessionId, ref] of sessionRefs) { @@ -647,6 +657,18 @@ export function createDaemonSessionFactory(options: DaemonSessionFactoryOptions) if (request.action === "annotate" || request.action === "annotate-last") { const input = await resolveAnnotateInput(request, cwd, request.action); + const isFileBased = input.mode === "annotate" || input.mode === "annotate-folder"; + const matchKey = isFileBased ? `annotate:${input.filePath}` : undefined; + + if (matchKey) { + const existing = findAwaitingSession(context.store, matchKey); + if (existing && existing.session.updateContent) { + (existing.session.updateContent as (m: string) => void)(input.markdown); + context.store.reactivate(existing.record.id); + return existing.record; + } + } + const remoteShare = context.endpoint.isRemote && sharingEnabled && input.markdown ? await createRemoteShareNotice(input.markdown, shareBaseUrl, "annotate", "document only").catch(() => undefined) : undefined; @@ -670,21 +692,34 @@ export function createDaemonSessionFactory(options: DaemonSessionFactoryOptions) ? `plugin-annotate-${request.origin}-${basename(input.folderPath)}${branch ? `-${branch}` : ""}` : `plugin-annotate-${request.origin}-${input.mode === "annotate-last" ? "last" : basename(input.filePath)}${branch ? `-${branch}` : ""}`, origin: request.origin, + matchKey, ttlMs, handleRequest: session.handleRequest, - dispose: registerSessionDecision(context, id, () => session.waitForDecision(), () => session.dispose(), (result) => ({ - ...result, - filePath: input.filePath, - mode: input.mode, - })), + dispose: isFileBased + ? registerPersistentDecision(context, id, session) + : registerSessionDecision(context, id, () => session.waitForDecision(), () => session.dispose(), (result) => ({ + ...result, filePath: input.filePath, mode: input.mode, + })), remoteShare, - snapshot: () => ({ plan: input.markdown, filePath: input.filePath, mode: input.mode, sourceInfo: input.sourceInfo }), + snapshot: session.getSnapshot ?? (() => ({ plan: input.markdown, filePath: input.filePath, mode: input.mode, sourceInfo: input.sourceInfo })), }); + if (matchKey) sessionRefs.set(id, { matchKey, session }); return record; } if (request.action === "review") { const input = await prepareReviewInput(request, cwd); + const reviewMatchKey = input.prMetadata + ? `review:${input.prMetadata.url}` + : branch ? `review:${project}:${branch}` : `review:${project}`; + + const existingReview = findAwaitingSession(context.store, reviewMatchKey); + if (existingReview && existingReview.session.updateContent) { + (existingReview.session.updateContent as (rawPatch: string, gitRef: string) => void)(input.rawPatch, input.gitRef); + context.store.reactivate(existingReview.record.id); + return existingReview.record; + } + const sessionError = [input.error, input.localWarning].filter(Boolean).join("\n\n") || undefined; const remoteShare = context.endpoint.isRemote && sharingEnabled ? await createRemoteShareNotice(input.rawPatch, shareBaseUrl, "review changes", "diff only").catch(() => undefined) @@ -727,18 +762,20 @@ export function createDaemonSessionFactory(options: DaemonSessionFactoryOptions) ? `plugin-${getMRLabel(input.prMetadata).toLowerCase()}-review-${getDisplayRepo(input.prMetadata)}${getMRNumberLabel(input.prMetadata)}` : branch ? `plugin-review-${request.origin}-${project}-${branch}` : `plugin-review-${request.origin}-${project}`, origin: request.origin, + matchKey: reviewMatchKey, ttlMs, handleRequest: session.handleRequest, - dispose: registerSessionDecision(context, id, () => session.waitForDecision(), () => session.dispose()), + dispose: registerPersistentDecision(context, id, session), remoteShare, - snapshot: () => ({ + snapshot: session.getSnapshot ?? (() => ({ rawPatch: input.rawPatch, gitRef: input.gitRef, origin: request.origin, diffType: input.gitContext ? (input.diffType ?? "unstaged") : undefined, gitContext: input.gitContext ? { currentBranch: input.gitContext.currentBranch, base: input.base } : undefined, - }), + })), }); + sessionRefs.set(id, { matchKey: reviewMatchKey, session }); return record; } diff --git a/packages/server/review.ts b/packages/server/review.ts index b26b5e5e..67bd5d72 100644 --- a/packages/server/review.ts +++ b/packages/server/review.ts @@ -130,6 +130,8 @@ export interface ReviewSession { waitForDecision: ReviewServerResult["waitForDecision"]; setServerUrl: (url: string) => void; dispose: () => void; + updateContent?: (rawPatch: string, gitRef: string) => void; + getSnapshot?: () => unknown; } export interface ResolveReviewScopedAgentCwdOptions { @@ -503,22 +505,14 @@ export async function createReviewSession( } // Decision promise - let resolveDecision: (result: { - approved: boolean; - feedback: string; - annotations: unknown[]; - agentSwitch?: string; - exit?: boolean; - }) => void; - const decisionPromise = new Promise<{ - approved: boolean; - feedback: string; - annotations: unknown[]; - agentSwitch?: string; - exit?: boolean; - }>((resolve) => { - resolveDecision = resolve; - }); + type ReviewDecisionResult = { approved: boolean; feedback: string; annotations: unknown[]; agentSwitch?: string; exit?: boolean }; + let currentCycle: { promise: Promise; resolve: (result: ReviewDecisionResult) => void }; + function startNewCycle() { + let resolve: (result: ReviewDecisionResult) => void; + const promise = new Promise((r) => { resolve = r; }); + currentCycle = { promise, resolve: resolve! }; + } + startNewCycle(); const handleRequest: SessionRequestHandler = async (req, url, context) => { @@ -1050,7 +1044,7 @@ export async function createReviewSession( // API: Exit review session without feedback if (url.pathname === "/api/exit" && req.method === "POST") { deleteDraft(draftKey); - resolveDecision({ approved: false, feedback: "", annotations: [], exit: true }); + currentCycle.resolve({ approved: false, feedback: "", annotations: [], exit: true }); return Response.json({ ok: true }); } @@ -1065,14 +1059,17 @@ export async function createReviewSession( }; deleteDraft(draftKey); - resolveDecision({ - approved: body.approved ?? false, + const isApproved = body.approved ?? false; + const isAgentSession = !!origin; + currentCycle.resolve({ + approved: isApproved, feedback: body.feedback || "", annotations: body.annotations || [], agentSwitch: body.agentSwitch, }); + if (!isApproved && isAgentSession) startNewCycle(); - return Response.json({ ok: true }); + return Response.json({ ok: true, ...(!isApproved && isAgentSession && { awaitingResubmission: true }) }); } catch (err) { const message = err instanceof Error ? err.message : "Failed to process feedback"; @@ -1184,7 +1181,7 @@ export async function createReviewSession( return { htmlContent, handleRequest, - waitForDecision: () => decisionPromise, + waitForDecision: () => currentCycle.promise, setServerUrl: (url) => { serverUrl = url; }, @@ -1194,7 +1191,6 @@ export async function createReviewSession( agentJobs.dispose(); aiSessionManager.disposeAll(); aiRegistry.disposeAll(); - // Invoke cleanup callback (e.g., remove temp worktree) if (options.onCleanup) { try { const result = options.onCleanup(); @@ -1202,6 +1198,20 @@ export async function createReviewSession( } catch { /* best effort */ } } }, + getSnapshot: () => ({ + rawPatch: currentPatch, + gitRef: currentGitRef, + origin, + diffType: currentDiffType, + gitContext: gitContext ? { currentBranch: gitContext.currentBranch, base: currentBase } : undefined, + }), + updateContent: (rawPatch: string, gitRef: string) => { + currentPatch = rawPatch; + currentGitRef = gitRef; + deleteDraft(draftKey); + draftKey = contentHash(rawPatch); + options.sessionEvents?.publishEvent("session-revision", { rawPatch, gitRef }); + }, }; } From 7feff893cf6869750568bae8ac98003a3a0aecf8 Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Fri, 22 May 2026 10:21:47 -0700 Subject: [PATCH 08/35] Add awaiting-resubmission frontend state for annotate and code review Annotate: handleAnnotateFeedback now checks response for awaitingResubmission flag, same pattern as plan deny handler. Session-revision subscription already handles both plan and annotate since they share App.tsx. Code review: handleSendFeedback checks awaitingResubmission response. Add session-revision event subscription that refreshes diff data, clears annotations, and resets awaiting state. CompletionBanner shows awaiting variant for all three surfaces. --- packages/plannotator-code-review/App.tsx | 40 ++++++++++++++++++++++-- packages/plannotator-plan-review/App.tsx | 10 ++++-- 2 files changed, 46 insertions(+), 4 deletions(-) diff --git a/packages/plannotator-code-review/App.tsx b/packages/plannotator-code-review/App.tsx index d0902e8f..74d68fa2 100644 --- a/packages/plannotator-code-review/App.tsx +++ b/packages/plannotator-code-review/App.tsx @@ -36,6 +36,7 @@ import { isTypingTarget, useReviewSearch, type ReviewSearchMatch } from './hooks import { useEditorAnnotations } from '@plannotator/ui/hooks/useEditorAnnotations'; import { useExternalAnnotations } from '@plannotator/ui/hooks/useExternalAnnotations'; import { useAgentJobs } from '@plannotator/ui/hooks/useAgentJobs'; +import { subscribeToDaemonSessionFamily } from '@plannotator/ui/utils/daemonHub'; import { exportEditorAnnotations } from '@plannotator/ui/utils/parser'; import { ResizeHandle } from '@plannotator/ui/components/ResizeHandle'; import { DockviewReact, type DockviewReadyEvent, type DockviewApi } from 'dockview-react'; @@ -238,6 +239,7 @@ const ReviewApp: React.FC<{ __embedded?: boolean; headerLeft?: React.ReactNode; const [isApproving, setIsApproving] = useState(false); const [isExiting, setIsExiting] = useState(false); const [submitted, setSubmitted] = useState<'approved' | 'feedback' | 'exited' | false>(false); + const [awaitingResubmission, setAwaitingResubmission] = useState(false); const [showApproveWarning, setShowApproveWarning] = useState(false); const [showExitWarning, setShowExitWarning] = useState(false); const [sharingEnabled, setSharingEnabled] = useState(true); @@ -308,6 +310,28 @@ const ReviewApp: React.FC<{ __embedded?: boolean; headerLeft?: React.ReactNode; const { externalAnnotations, updateExternalAnnotation, deleteExternalAnnotation } = useExternalAnnotations({ enabled: !!origin }); const agentJobs = useAgentJobs({ enabled: !!origin }); + // Listen for session-revision events (diff refresh after feedback) + useEffect(() => { + if (!origin || !awaitingResubmission) return; + const unsubscribe = subscribeToDaemonSessionFamily("session-revision", (msg) => { + if (msg.type !== "event" || !msg.payload) return; + const revision = msg.payload as { rawPatch?: string; gitRef?: string }; + if (revision.rawPatch) { + const newFiles = parseDiffToFiles(revision.rawPatch); + setDiffData(prev => prev ? { ...prev, rawPatch: revision.rawPatch!, gitRef: revision.gitRef ?? prev.gitRef } : prev); + storeApi.getState().setFiles(newFiles); + storeApi.getState().setFocusedFile(0); + storeApi.getState().setLocalAnnotations([]); + storeApi.getState().selectAnnotation(null); + storeApi.getState().setPendingSelection(null); + setAwaitingResubmission(false); + setSubmitted(false); + setIsSendingFeedback(false); + } + }); + return unsubscribe; + }, [origin, awaitingResubmission, storeApi]); + // Tour dialog state — opens as an overlay instead of a dock panel const [tourDialogJobId, setTourDialogJobId] = useState(null); @@ -1528,7 +1552,13 @@ const ReviewApp: React.FC<{ __embedded?: boolean; headerLeft?: React.ReactNode; }), }); if (res.ok) { - setSubmitted('feedback'); + const data = await res.json().catch(() => ({})); + if (data.awaitingResubmission) { + setAwaitingResubmission(true); + setIsSendingFeedback(false); + } else { + setSubmitted('feedback'); + } } else { throw new Error('Failed to send'); } @@ -2153,7 +2183,13 @@ const ReviewApp: React.FC<{ __embedded?: boolean; headerLeft?: React.ReactNode; {/* Embedded completion banner — inline, non-blocking */} - {__embedded && !legacyTabMode && } + {__embedded && !legacyTabMode && ( + + )} {/* Main content */}
diff --git a/packages/plannotator-plan-review/App.tsx b/packages/plannotator-plan-review/App.tsx index 90088cfb..bc530184 100644 --- a/packages/plannotator-plan-review/App.tsx +++ b/packages/plannotator-plan-review/App.tsx @@ -1129,7 +1129,7 @@ const App: React.FC<{ __embedded?: boolean; headerLeft?: React.ReactNode; onOpen const handleAnnotateFeedback = async () => { setIsSubmitting(true); try { - await fetch('/api/feedback', { + const response = await fetch('/api/feedback', { method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ @@ -1138,7 +1138,13 @@ const App: React.FC<{ __embedded?: boolean; headerLeft?: React.ReactNode; onOpen codeAnnotations, }), }); - setSubmitted('denied'); // reuse 'denied' state for "feedback sent" overlay + const data = await response.json().catch(() => ({})); + if (data.awaitingResubmission) { + setAwaitingResubmission(true); + setIsSubmitting(false); + } else { + setSubmitted('denied'); + } } catch { setIsSubmitting(false); } From db80f9557b4ca664a7f9e5c284f5c61560629824 Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Fri, 22 May 2026 10:22:48 -0700 Subject: [PATCH 09/35] Quality fixes: extract updateContent, document findAwaitingSession Move plan server's inline updateContent closure to a named function handleUpdateContent for readability. Document findAwaitingSession's side effect of pruning terminal sessions during search. --- packages/server/daemon/session-factory.ts | 2 ++ packages/server/index.ts | 34 ++++++++++++----------- 2 files changed, 20 insertions(+), 16 deletions(-) diff --git a/packages/server/daemon/session-factory.ts b/packages/server/daemon/session-factory.ts index d1df40e0..2594b54d 100644 --- a/packages/server/daemon/session-factory.ts +++ b/packages/server/daemon/session-factory.ts @@ -532,6 +532,8 @@ export function createDaemonSessionFactory(options: DaemonSessionFactoryOptions) } const sessionRefs = new Map(); + // Search for an awaiting-resubmission session with the given matchKey. + // Side effect: prunes terminal sessions from sessionRefs during the scan. function findAwaitingSession(store: DaemonFetchContext["store"], matchKey: string) { for (const [sessionId, ref] of sessionRefs) { const record = store.get(sessionId); diff --git a/packages/server/index.ts b/packages/server/index.ts index 14e73883..076f061f 100644 --- a/packages/server/index.ts +++ b/packages/server/index.ts @@ -594,6 +594,23 @@ export async function createPlannotatorSession( }); }; + function handleUpdateContent(newPlan: string) { + plan = newPlan; + const historyResult = saveToHistory(project, slug, newPlan); + currentPlanPath = historyResult.path; + previousPlan = historyResult.version > 1 + ? getPlanVersion(project, slug, historyResult.version - 1) + : null; + versionInfo = { + version: historyResult.version, + totalVersions: getVersionCount(project, slug), + project, + }; + deleteDraft(draftKey); + draftKey = contentHash(newPlan); + options.sessionEvents?.publishEvent("session-revision", { plan: newPlan, previousPlan, versionInfo }); + } + return { htmlContent, handleRequest, @@ -604,22 +621,7 @@ export async function createPlannotatorSession( }, slug: mode !== "archive" ? slug : undefined, getSnapshot: mode !== "archive" ? () => ({ plan, origin }) : undefined, - updateContent: mode !== "archive" ? (newPlan: string) => { - plan = newPlan; - const historyResult = saveToHistory(project, slug, newPlan); - currentPlanPath = historyResult.path; - previousPlan = historyResult.version > 1 - ? getPlanVersion(project, slug, historyResult.version - 1) - : null; - versionInfo = { - version: historyResult.version, - totalVersions: getVersionCount(project, slug), - project, - }; - deleteDraft(draftKey); - draftKey = contentHash(newPlan); - options.sessionEvents?.publishEvent("session-revision", { plan: newPlan, previousPlan, versionInfo }); - } : undefined, + updateContent: mode !== "archive" ? handleUpdateContent : undefined, }; } From b4ffc95dcdfa449681245ff885054102c6beead0 Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Fri, 22 May 2026 12:24:49 -0700 Subject: [PATCH 10/35] Extract shared decision cycle helper, consistent updateContent naming MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add createDecisionCycle() and resolveAndCycle() to session-handler.ts. All three servers (plan, annotate, review) now use the shared helper instead of copy-pasting the cycle setup, resolve, and startNewCycle pattern. Deny handlers collapse to a single resolveAndCycle() call. Extract updateContent to named handleUpdateContent functions in all three servers for consistency — inline closures in return blocks replaced with named functions defined above the return. --- packages/server/annotate.ts | 38 +++++++++++--------------- packages/server/index.ts | 22 +++++---------- packages/server/review.ts | 43 +++++++++++++----------------- packages/server/session-handler.ts | 36 +++++++++++++++++++++++++ 4 files changed, 77 insertions(+), 62 deletions(-) diff --git a/packages/server/annotate.ts b/packages/server/annotate.ts index 2fcfba36..22bb9b20 100644 --- a/packages/server/annotate.ts +++ b/packages/server/annotate.ts @@ -22,6 +22,7 @@ import { createExternalAnnotationHandler } from "./external-annotations"; import { saveConfig, detectGitUser, getServerConfig } from "./config"; import { dirname, resolve as resolvePath } from "path"; import { isWSL } from "./browser"; +import { createDecisionCycle, resolveAndCycle } from "./session-handler"; import type { SessionEventBridge, SessionRequestHandler } from "./session-handler"; // Re-export utilities @@ -142,15 +143,8 @@ export async function createAnnotateSession( // Detect repo info (cached for this session) const repoInfo = await getRepoInfo(cwd); - // Decision cycle — each deny/feedback starts a new cycle; approve/exit is final type AnnotateDecisionResult = { feedback: string; annotations: unknown[]; exit?: boolean; approved?: boolean }; - let currentCycle: { promise: Promise; resolve: (result: AnnotateDecisionResult) => void }; - function startNewCycle() { - let resolve: (result: AnnotateDecisionResult) => void; - const promise = new Promise((r) => { resolve = r; }); - currentCycle = { promise, resolve: resolve! }; - } - startNewCycle(); + const decisionCycle = createDecisionCycle(); const handleRequest: SessionRequestHandler = async (req, url, context) => { @@ -255,14 +249,14 @@ export async function createAnnotateSession( // API: Exit annotation session without feedback if (url.pathname === "/api/exit" && req.method === "POST") { deleteDraft(draftKey); - currentCycle.resolve({ feedback: "", annotations: [], exit: true }); + decisionCycle.resolve({ feedback: "", annotations: [], exit: true }); return Response.json({ ok: true }); } // API: Approve the annotation session (review-gate UX, #570) if (url.pathname === "/api/approve" && req.method === "POST") { deleteDraft(draftKey); - currentCycle.resolve({ feedback: "", annotations: [], approved: true }); + decisionCycle.resolve({ feedback: "", annotations: [], approved: true }); return Response.json({ ok: true }); } @@ -275,14 +269,12 @@ export async function createAnnotateSession( }; deleteDraft(draftKey); - const isAgentSession = !!origin; - currentCycle.resolve({ + const resubmit = resolveAndCycle(decisionCycle, { feedback: body.feedback || "", annotations: body.annotations || [], - }); - if (isAgentSession) startNewCycle(); + }, origin); - return Response.json({ ok: true, ...(isAgentSession && { awaitingResubmission: true }) }); + return Response.json({ ok: true, ...resubmit }); } catch (err) { const message = err instanceof Error @@ -303,20 +295,22 @@ export async function createAnnotateSession( const isFileBased = mode === "annotate" || mode === "annotate-folder"; + function handleUpdateContent(newMarkdown: string) { + markdown = newMarkdown; + deleteDraft(draftKey); + draftKey = contentHash(newMarkdown); + options.sessionEvents?.publishEvent("session-revision", { plan: newMarkdown }); + } + return { htmlContent, handleRequest, - waitForDecision: () => currentCycle.promise, + waitForDecision: () => decisionCycle.promise(), dispose: () => { externalAnnotations.dispose(); }, getSnapshot: isFileBased ? () => ({ plan: markdown, filePath, mode, sourceInfo }) : undefined, - updateContent: isFileBased ? (newMarkdown: string) => { - markdown = newMarkdown; - deleteDraft(draftKey); - draftKey = contentHash(newMarkdown); - options.sessionEvents?.publishEvent("session-revision", { plan: newMarkdown }); - } : undefined, + updateContent: isFileBased ? handleUpdateContent : undefined, }; } diff --git a/packages/server/index.ts b/packages/server/index.ts index 076f061f..f0391a34 100644 --- a/packages/server/index.ts +++ b/packages/server/index.ts @@ -50,6 +50,7 @@ import { resolveUserPath, warmFileListCache } from "@plannotator/shared/resolve- import { createEditorAnnotationHandler } from "./editor-annotations"; import { createExternalAnnotationHandler } from "./external-annotations"; import { isWSL } from "./browser"; +import { createDecisionCycle, resolveAndCycle } from "./session-handler"; import type { SessionEventBridge, SessionRequestHandler } from "./session-handler"; // Re-export utilities @@ -196,12 +197,7 @@ export async function createPlannotatorSession( agentSwitch?: string; permissionMode?: string; }; - let currentCycle: { promise: Promise; resolve: (result: DecisionResult) => void }; - function startNewCycle() { - let resolve: (result: DecisionResult) => void; - const promise = new Promise((r) => { resolve = r; }); - currentCycle = { promise, resolve: resolve! }; - } + const decisionCycle = createDecisionCycle(); if (mode !== "archive") { repoInfo = await getRepoInfo(cwd); @@ -218,10 +214,8 @@ export async function createPlannotatorSession( project, }; - startNewCycle(); } else { - // Never-resolving promise — archive mode uses waitForDone instead - startNewCycle(); + // Archive mode: decision cycle exists but is never resolved (uses waitForDone instead) } const handleRequest: SessionRequestHandler = async (req, url, context) => { @@ -542,7 +536,7 @@ export async function createPlannotatorSession( // Use permission mode from client request if provided, otherwise fall back to hook input const effectivePermissionMode = requestedPermissionMode || permissionMode; - currentCycle.resolve({ approved: true, feedback, savedPath, agentSwitch, permissionMode: effectivePermissionMode }); + decisionCycle.resolve({ approved: true, feedback, savedPath, agentSwitch, permissionMode: effectivePermissionMode }); return Response.json({ ok: true, savedPath }); } @@ -579,10 +573,8 @@ export async function createPlannotatorSession( } deleteDraft(draftKey); - const isAgentSession = !!origin; - currentCycle.resolve({ approved: false, feedback, savedPath }); - if (isAgentSession) startNewCycle(); - return Response.json({ ok: true, savedPath, ...(isAgentSession && { awaitingResubmission: true }) }); + const resubmit = resolveAndCycle(decisionCycle, { approved: false, feedback, savedPath }, origin); + return Response.json({ ok: true, savedPath, ...resubmit }); } // Favicon @@ -614,7 +606,7 @@ export async function createPlannotatorSession( return { htmlContent, handleRequest, - waitForDecision: () => currentCycle.promise, + waitForDecision: () => decisionCycle.promise(), ...(donePromise && { waitForDone: () => donePromise }), dispose: () => { externalAnnotations?.dispose(); diff --git a/packages/server/review.ts b/packages/server/review.ts index 67bd5d72..c3e85171 100644 --- a/packages/server/review.ts +++ b/packages/server/review.ts @@ -49,6 +49,7 @@ import { type PRMetadata, type PRReviewFileComment, type PRStackTree, type PRLis import { createAIEndpoints, ProviderRegistry, SessionManager, createProvider, type AIEndpoints, type PiSDKConfig } from "@plannotator/ai"; import { isWSL } from "./browser"; import { handleCodeNavResolve, extractChangedFiles } from "./code-nav"; +import { createDecisionCycle, resolveAndCycle } from "./session-handler"; import type { SessionEventBridge, SessionRequestHandler } from "./session-handler"; // Re-export utilities @@ -506,13 +507,7 @@ export async function createReviewSession( // Decision promise type ReviewDecisionResult = { approved: boolean; feedback: string; annotations: unknown[]; agentSwitch?: string; exit?: boolean }; - let currentCycle: { promise: Promise; resolve: (result: ReviewDecisionResult) => void }; - function startNewCycle() { - let resolve: (result: ReviewDecisionResult) => void; - const promise = new Promise((r) => { resolve = r; }); - currentCycle = { promise, resolve: resolve! }; - } - startNewCycle(); + const decisionCycle = createDecisionCycle(); const handleRequest: SessionRequestHandler = async (req, url, context) => { @@ -1044,7 +1039,7 @@ export async function createReviewSession( // API: Exit review session without feedback if (url.pathname === "/api/exit" && req.method === "POST") { deleteDraft(draftKey); - currentCycle.resolve({ approved: false, feedback: "", annotations: [], exit: true }); + decisionCycle.resolve({ approved: false, feedback: "", annotations: [], exit: true }); return Response.json({ ok: true }); } @@ -1060,16 +1055,12 @@ export async function createReviewSession( deleteDraft(draftKey); const isApproved = body.approved ?? false; - const isAgentSession = !!origin; - currentCycle.resolve({ - approved: isApproved, - feedback: body.feedback || "", - annotations: body.annotations || [], - agentSwitch: body.agentSwitch, - }); - if (!isApproved && isAgentSession) startNewCycle(); + const result = { approved: isApproved, feedback: body.feedback || "", annotations: body.annotations || [], agentSwitch: body.agentSwitch }; + const resubmit = isApproved + ? (decisionCycle.resolve(result), {}) + : resolveAndCycle(decisionCycle, result, origin); - return Response.json({ ok: true, ...(!isApproved && isAgentSession && { awaitingResubmission: true }) }); + return Response.json({ ok: true, ...resubmit }); } catch (err) { const message = err instanceof Error ? err.message : "Failed to process feedback"; @@ -1178,10 +1169,18 @@ export async function createReviewSession( const exitHandler = () => agentJobs.killAll(); process.once("exit", exitHandler); + function handleUpdateContent(rawPatch: string, gitRef: string) { + currentPatch = rawPatch; + currentGitRef = gitRef; + deleteDraft(draftKey); + draftKey = contentHash(rawPatch); + options.sessionEvents?.publishEvent("session-revision", { rawPatch, gitRef }); + } + return { htmlContent, handleRequest, - waitForDecision: () => currentCycle.promise, + waitForDecision: () => decisionCycle.promise(), setServerUrl: (url) => { serverUrl = url; }, @@ -1205,13 +1204,7 @@ export async function createReviewSession( diffType: currentDiffType, gitContext: gitContext ? { currentBranch: gitContext.currentBranch, base: currentBase } : undefined, }), - updateContent: (rawPatch: string, gitRef: string) => { - currentPatch = rawPatch; - currentGitRef = gitRef; - deleteDraft(draftKey); - draftKey = contentHash(rawPatch); - options.sessionEvents?.publishEvent("session-revision", { rawPatch, gitRef }); - }, + updateContent: handleUpdateContent, }; } diff --git a/packages/server/session-handler.ts b/packages/server/session-handler.ts index 98749c7f..c2f74791 100644 --- a/packages/server/session-handler.ts +++ b/packages/server/session-handler.ts @@ -27,3 +27,39 @@ export type SessionRequestHandler = ( url: URL, context?: SessionRequestContext, ) => Response | Promise; + +/** + * Manages a resolvable decision cycle for session servers. + * Each deny/feedback starts a new cycle; approve/exit is final. + */ +export function createDecisionCycle() { + let current: { promise: Promise; resolve: (result: T) => void }; + function start() { + let resolve: (result: T) => void; + const promise = new Promise((r) => { resolve = r; }); + current = { promise, resolve: resolve! }; + } + start(); + return { + promise: () => current.promise, + resolve: (result: T) => current.resolve(result), + startNew: () => start(), + }; +} + +/** + * Resolve the current decision cycle. If the session has an agent origin, + * start a new cycle and include `awaitingResubmission: true` in the response. + */ +export function resolveAndCycle( + cycle: ReturnType>, + result: T, + origin: string | undefined, +): { awaitingResubmission?: true } { + cycle.resolve(result); + if (origin) { + cycle.startNew(); + return { awaitingResubmission: true }; + } + return {}; +} From d46c92e9e9c0b04f3821b348bba1e41ed0e94d63 Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Fri, 22 May 2026 16:56:46 -0700 Subject: [PATCH 11/35] Fix 5 review findings: snapshot provider, origin check, exit handling, empty content, TTL MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 1. Register no-op snapshot provider for session-revision in all three servers — prevents WebSocket disconnect when frontend subscribes. 2. Exclude plannotator-frontend from agent origins in resolveAndCycle — dashboard-created sessions complete on deny instead of hanging. 3. Complete session on exit: true in registerPersistentDecision — Exit button now properly terminates instead of suspending. 4. Check revision.plan !== undefined instead of truthiness — empty diffs and empty annotate content no longer ignored. 5. Store ttlMs on session record and restore it on reactivate() — reactivated sessions expire normally if abandoned. --- packages/plannotator-code-review/App.tsx | 2 +- packages/plannotator-plan-review/App.tsx | 2 +- packages/server/annotate.ts | 1 + packages/server/daemon/session-factory.ts | 2 +- packages/server/daemon/session-store.ts | 4 +++- packages/server/index.ts | 1 + packages/server/review.ts | 1 + packages/server/session-handler.ts | 4 +++- 8 files changed, 12 insertions(+), 5 deletions(-) diff --git a/packages/plannotator-code-review/App.tsx b/packages/plannotator-code-review/App.tsx index 74d68fa2..31b53a47 100644 --- a/packages/plannotator-code-review/App.tsx +++ b/packages/plannotator-code-review/App.tsx @@ -316,7 +316,7 @@ const ReviewApp: React.FC<{ __embedded?: boolean; headerLeft?: React.ReactNode; const unsubscribe = subscribeToDaemonSessionFamily("session-revision", (msg) => { if (msg.type !== "event" || !msg.payload) return; const revision = msg.payload as { rawPatch?: string; gitRef?: string }; - if (revision.rawPatch) { + if (revision.rawPatch !== undefined) { const newFiles = parseDiffToFiles(revision.rawPatch); setDiffData(prev => prev ? { ...prev, rawPatch: revision.rawPatch!, gitRef: revision.gitRef ?? prev.gitRef } : prev); storeApi.getState().setFiles(newFiles); diff --git a/packages/plannotator-plan-review/App.tsx b/packages/plannotator-plan-review/App.tsx index bc530184..44404f8d 100644 --- a/packages/plannotator-plan-review/App.tsx +++ b/packages/plannotator-plan-review/App.tsx @@ -596,7 +596,7 @@ const App: React.FC<{ __embedded?: boolean; headerLeft?: React.ReactNode; onOpen const unsubscribe = subscribeToDaemonSessionFamily("session-revision", (msg) => { if (msg.type !== "event" || !msg.payload) return; const revision = msg.payload as { plan?: string; previousPlan?: string | null; versionInfo?: { version: number; totalVersions: number; project: string } }; - if (revision.plan) { + if (revision.plan !== undefined) { setMarkdown(revision.plan); if (revision.previousPlan !== undefined) setPreviousPlan(revision.previousPlan); if (revision.versionInfo) setVersionInfo(revision.versionInfo); diff --git a/packages/server/annotate.ts b/packages/server/annotate.ts index 22bb9b20..ab53e9c3 100644 --- a/packages/server/annotate.ts +++ b/packages/server/annotate.ts @@ -139,6 +139,7 @@ export async function createAnnotateSession( registerSnapshotProvider: (provider) => options.sessionEvents?.registerSnapshotProvider("external-annotations", provider), }); + options.sessionEvents?.registerSnapshotProvider("session-revision", () => null); // Detect repo info (cached for this session) const repoInfo = await getRepoInfo(cwd); diff --git a/packages/server/daemon/session-factory.ts b/packages/server/daemon/session-factory.ts index 2594b54d..6ae3a866 100644 --- a/packages/server/daemon/session-factory.ts +++ b/packages/server/daemon/session-factory.ts @@ -147,7 +147,7 @@ function registerPersistentDecision( const decisionLoop = async () => { while (true) { const result = await Promise.race([session.waitForDecision(), disposed]); - if (result.approved) { + if (result.approved || result.exit) { context.store.complete(id, result); return; } diff --git a/packages/server/daemon/session-store.ts b/packages/server/daemon/session-store.ts index a91ecb00..9522a317 100644 --- a/packages/server/daemon/session-store.ts +++ b/packages/server/daemon/session-store.ts @@ -20,6 +20,7 @@ export interface DaemonSessionRecord { label: string; origin?: string; matchKey?: string; + ttlMs?: number; createdAt: string; updatedAt: string; expiresAt?: string; @@ -174,6 +175,7 @@ export class DaemonSessionStore { ...(input.cwd && { cwd: input.cwd }), ...(input.origin && { origin: input.origin }), ...(input.matchKey && { matchKey: input.matchKey }), + ...(input.ttlMs !== undefined && { ttlMs: input.ttlMs }), createdAt: iso(now), updatedAt: iso(now), ...(input.ttlMs !== undefined && { expiresAt: iso(now + input.ttlMs) }), @@ -277,7 +279,7 @@ export class DaemonSessionStore { record.result = undefined; const now = this.now(); record.updatedAt = iso(now); - record.expiresAt = undefined; + record.expiresAt = record.ttlMs !== undefined ? iso(now + record.ttlMs) : undefined; this.emit("session-updated", record); return record; } diff --git a/packages/server/index.ts b/packages/server/index.ts index f0391a34..7321c81b 100644 --- a/packages/server/index.ts +++ b/packages/server/index.ts @@ -178,6 +178,7 @@ export async function createPlannotatorSession( registerSnapshotProvider: (provider) => options.sessionEvents?.registerSnapshotProvider("external-annotations", provider), }) : null; + if (mode !== "archive") options.sessionEvents?.registerSnapshotProvider("session-revision", () => null); const slug = mode !== "archive" ? generateSlug(plan) : ""; // Lazy cache for in-session archive browsing (plan review sidebar tab) diff --git a/packages/server/review.ts b/packages/server/review.ts index c3e85171..2c5dc120 100644 --- a/packages/server/review.ts +++ b/packages/server/review.ts @@ -174,6 +174,7 @@ export async function createReviewSession( registerSnapshotProvider: (provider) => options.sessionEvents?.registerSnapshotProvider("external-annotations", provider), }); + options.sessionEvents?.registerSnapshotProvider("session-revision", () => null); const tour = createTourSession(); diff --git a/packages/server/session-handler.ts b/packages/server/session-handler.ts index c2f74791..9a1acaf5 100644 --- a/packages/server/session-handler.ts +++ b/packages/server/session-handler.ts @@ -51,13 +51,15 @@ export function createDecisionCycle() { * Resolve the current decision cycle. If the session has an agent origin, * start a new cycle and include `awaitingResubmission: true` in the response. */ +const NON_AGENT_ORIGINS = new Set(["plannotator-frontend"]); + export function resolveAndCycle( cycle: ReturnType>, result: T, origin: string | undefined, ): { awaitingResubmission?: true } { cycle.resolve(result); - if (origin) { + if (origin && !NON_AGENT_ORIGINS.has(origin)) { cycle.startNew(); return { awaitingResubmission: true }; } From b2477d10232c6f2c0ab611888cea3097aec2d768 Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Sun, 24 May 2026 09:30:18 -0700 Subject: [PATCH 12/35] Add session persistence design docs and decisions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Captures the overview of what session persistence does, the technical architecture, and active design decisions from PR #770 triage — notably removing persistence from code review and redesigning the awaiting state. --- goals/session-persistence/decisions.md | 96 +++++++ goals/session-persistence/overview.html | 327 ++++++++++++++++++++++++ goals/session-persistence/overview.md | 164 ++++++++++++ 3 files changed, 587 insertions(+) create mode 100644 goals/session-persistence/decisions.md create mode 100644 goals/session-persistence/overview.html create mode 100644 goals/session-persistence/overview.md diff --git a/goals/session-persistence/decisions.md b/goals/session-persistence/decisions.md new file mode 100644 index 00000000..d2e28a59 --- /dev/null +++ b/goals/session-persistence/decisions.md @@ -0,0 +1,96 @@ +# Session Persistence — Design Decisions + +Tracking decisions made during PR #770 review and triage (2026-05-23). + +## Decision 1: Remove persistence from code review + +**Status:** Decided + +Code review persistence (awaiting-resubmission + session matching + content refresh) is being removed. The plan review persistence model doesn't fit code review. + +**Why:** Plan persistence is "same document, new version" — the agent revises a plan and the user reviews the revision in context. Code review is different — a diff is a snapshot of file changes. When the agent modifies code based on feedback, it produces an entirely new diff. Keeping the same session and "refreshing" it conflates two separate review rounds. + +**What changes:** +- Review sessions complete normally on feedback (one-shot decision, not a cycle) +- Session stays alive in the browser after feedback is sent (not completed/closed) +- Annotations flush when feedback is sent +- The session becomes a live review workspace, not part of a decision loop + +**Open question: what happens when diffs change?** + +After feedback is sent, the agent makes code changes. The files on disk change. The user may want to see the updated diff. Options under consideration: + +- **A.** Session becomes read-only after feedback. User runs `/plannotator-review` again for a fresh review. (Simplest, but friction.) +- **B.** Session has a "Refresh Diff" button. User pulls the latest diff on demand. Can annotate and send feedback again — but the agent isn't listening. Subsequent feedback needs a delivery mechanism. (Middle ground.) +- **C.** Session stays live and pushes feedback directly into the agent conversation via hook system, like external annotations. (Most ambitious, agent doesn't need to explicitly trigger review.) + +**Open question: where does subsequent feedback go?** + +Once the first feedback resolves the decision promise and the agent unblocks, there's no listener for more feedback. If the session stays alive and the user sends again: +- Write to a file the agent picks up? +- Push into the conversation via hooks? +- Just don't allow — one feedback per agent invocation? + +--- + +## Decision 2: "Feedback sent" state should be calm, not loading + +**Status:** Decided + +After sending feedback (plan or code review), the UI should NOT show a spinner or "waiting" state. The session should feel settled — like archive mode. The user's annotations were sent. The plan/diff is still readable. The user can scroll, review what they wrote. + +If a new version arrives (plan resubmission), the content refreshes and the session comes back to life. + +**What this means for the current code:** +- Replace the amber spinner `CompletionBanner` awaiting variant with a calm "Feedback sent" banner +- Don't disable buttons during awaiting — the session is readable, not blocked +- The `onCancel` escape hatch becomes less urgent since the user isn't trapped behind a spinner + +--- + +## Decision 3: Fix the hot loop for frontend-originated sessions + +**Status:** Decided — will fix + +When a session is created from the Plannotator dashboard (origin: `"plannotator-frontend"`), `resolveAndCycle` resolves the promise but doesn't start a new cycle. The `registerPersistentDecision` loop then spins on the already-resolved promise. Each iteration is a no-op (suspend guard catches it), but it burns CPU. + +**Fix:** In `registerPersistentDecision`, if `suspend()` returns a record with status !== `"awaiting-resubmission"`, break the loop and complete. + +--- + +## Decision 4: Clear external annotations on review resubmission + +**Status:** Decided — will fix + +`handleUpdateContent` in review.ts swaps the patch but keeps stale external annotations (lint results, agent comments) from the previous diff. Line numbers and file paths may no longer match. + +**Fix:** Call `externalAnnotations.clear()` inside `handleUpdateContent`. + +*Note: If we remove persistence from code review (Decision 1), this becomes moot — there's no resubmission path. Keeping this noted in case we preserve any refresh mechanism.* + +--- + +## Decision 5: Wire the cancel escape hatch + +**Status:** Deferred — depends on Decision 2 + +If the awaiting state becomes calm/archive-like (Decision 2), the urgency drops. The user isn't stuck behind a spinner. But we still need to handle session expiry gracefully — when the 10-minute TTL fires server-side, the frontend should know. + +**Options:** +- Subscribe to `session-updated` daemon events (the expiry event family) +- Client-side timeout that matches server TTL +- Just let the session sit — user navigates away naturally + +--- + +## Bugs confirmed from external review (PR #770) + +| # | Finding | Severity | Action | +|---|---------|----------|--------| +| 1-2 | Frontend sessions spin in registerPersistentDecision | P1 | Fix (Decision 3) | +| 3+9 | --render-html resubmission shows stale content | P2 | Skip — narrow edge case | +| 4 | OpenCode folder annotation mislabeled after persistence | nit | Skip — cosmetic | +| 5 | Stale external annotations after review resubmission | P2 | Fix (Decision 4), possibly moot after Decision 1 | +| 6 | waitForResult doesn't short-circuit on awaiting | P2 | Skip — current behavior is correct | +| 7-8 | Actions not disabled during awaiting | P2 | Redesign (Decision 2 — calm state, not disabled) | +| 10 | onCancel never wired, stuck spinner | nit | Deferred (Decision 5) | diff --git a/goals/session-persistence/overview.html b/goals/session-persistence/overview.html new file mode 100644 index 00000000..67f32744 --- /dev/null +++ b/goals/session-persistence/overview.html @@ -0,0 +1,327 @@ + + + + + +Session Persistence — What You Need To Know + + + + + + + +
+

+ feat/session-persistence +

+ +
+ +
+ +
+

+ Session Persistence +

+

+ Denied sessions stay alive. The agent revises. The same session updates in place. No more starting over. +

+
+ + +
+

User Experience

+ +
+
+
+ +

Before

+
+
    +
  1. 1 You review a plan, leave annotations, deny
    2. +
  2. 2 Session dies — completion screen, done
    3. +
  3. 3 Agent revises and resubmits
    4. +
  4. 4 Brand new session — new tab, no context, no diff
    5. +
  5. 5 Start over from scratch
    6. +
+
+
+
+ +

After

+
+
    +
  1. 1 You review a plan, leave annotations, deny
    2. +
  2. 2 Banner: "Waiting for agent to revise..."
    3. +
  3. 3 Agent revises and resubmits
    4. +
  4. 4 Same session updates — diff shows what changed
    5. +
  5. 5 Review again — approve or deny with more feedback
    6. +
+
+
+
+ + +
+

Works across all modes

+
+
+
📋
+

Plan Review

+

Agent revises the plan, session shows plan diff

+
+
+
🔍
+

Code Review

+

Agent makes changes, session refreshes with new diff

+
+
+
✏️
+

Annotate

+

Agent edits the file, session refreshes with updated content

+
+
+
+ + + + + +
+

Session lifecycle

+
+ + + + active + + + deny + + + awaiting-resubmission + + + agent resubmits + + + 10 min TTL + + expired + + + approve + + completed + + + + + + +
+
+ + +
+

How sessions are matched

+
+ + + + + + + + + + + + + + + + + + + + + + + + + +
ModeMatch Key
Planplan:project:slug
Reviewreview:project:branch
Annotateannotate:filepath
+
+

No match (heading changed, different branch, different file) → new session as before.

+
+ + +
+

Files changed — 14 files, +423 / -143

+
+ + Show file list → + +
+ + + + + + + + + + + + + + + + +
daemon-protocol.tsNew status, event family, protocol v2
session-handler.tscreateDecisionCycle, resolveAndCycle helpers
session-store.tssuspend(), reactivate(), matchKey field
session-factory.tsMatching, persistent loop, all three types wired
server.ts (daemon)Skip deletion timer for awaiting sessions
index.ts (plan)Cycle model, updateContent, slug
annotate.tsCycle model, updateContent for files
review.tsCycle model, updateContent for diffs
hook/index.ts (CLI)Accept awaiting-resubmission status
plan-review App.tsxAwaiting state + revision subscription
code-review App.tsxAwaiting state + revision subscription
CompletionBanner.tsxAwaiting variant with spinner
AGENTS.mdDocumentation
+
+
+
+ + +
+

What does NOT persist

+
+
+ ✗ + URL-based annotations +
+
+ ✗ + "Annotate last message" sessions +
+
+ ✗ + Archive sessions (read-only) +
+
+ ✗ + Standalone / demo sessions +
+
+
+ + +
+

Recap

+
+
    +
  1. Denied sessions stay alive instead of dying
    2. +
  2. Agent resubmits → same session updates in place
    3. +
  3. Works for plan, code review, and file-based annotate
    4. +
  4. Matching by project+slug, project+branch, or filepath
    5. +
  5. 10-minute timeout if the agent doesn't come back
    6. +
  6. Agent doesn't need to know — matching is server-side
    7. +
  7. Shared createDecisionCycle eliminates duplication
    8. +
  8. Frontend shows amber "waiting" banner with cancel
    9. +
  9. No changes to approve, exit, or standalone flows
    10. +
+
+
+ + +
+

Knowledge check

+
+
+
+ +
+ + + + + diff --git a/goals/session-persistence/overview.md b/goals/session-persistence/overview.md new file mode 100644 index 00000000..666cb87a --- /dev/null +++ b/goals/session-persistence/overview.md @@ -0,0 +1,164 @@ +# Session Persistence — What You Need To Know + +## The User Experience + +### Before + +You review a plan. You leave annotations. You click Deny. Your feedback gets sent to the agent. The session dies. Completion screen. Done. + +The agent reads your feedback, revises the plan, and submits again. A completely new session appears — new tab or new sidebar entry. No connection to the one you just closed. No awareness that this is a revision of the same plan. You start from scratch every time. + +Same story for code review and annotate. Send feedback, session dies, agent makes changes, new session. Every deny-resubmit cycle is a fresh start. + +### After + +You deny a plan. Instead of the completion screen, you see: **"Feedback sent — waiting for agent to revise..."** The session stays alive. Your browser tab stays open. The sidebar shows a pulsing amber indicator. + +The agent revises the plan and submits again. The **same session** updates in place. You see the new plan with a diff showing what changed. Your annotations are cleared (the agent already has them). You review the fresh version. Approve or deny again. Repeat until you're satisfied. + +This works for all three session types: +- **Plan review** — agent revises the plan, session updates with plan diff +- **Code review** — agent makes code changes, session refreshes with new diff +- **Annotate** — agent edits the file, session refreshes with updated content + +### What stays the same + +- Approve works exactly as before +- Exit works exactly as before +- Auto-close works exactly as before +- Sessions opened without an agent (standalone, demo) behave exactly as before — deny is still final +- The session expires after 10 minutes if the agent doesn't resubmit + +--- + +## Why This Was Needed + +Users and community members repeatedly asked for this. The linear "deny → wait → new session" flow was friction-heavy. Every cycle required the user to re-orient: find the new session, remember what they asked for, compare mentally against the previous version. The plan diff system already existed but couldn't show diffs across sessions — only within a session's version history. + +The deny-resubmit cycle is the core feedback loop of plan-driven development. Making it seamless makes the entire product more useful. + +--- + +## Technical Overview + +### New Session Status: `awaiting-resubmission` + +A non-terminal status in the daemon session lifecycle. The session stays alive — its HTTP handler keeps serving requests, the WebSocket connection stays open, and the frontend connection persists. After 10 minutes without resubmission, the session expires normally. + +``` +active → awaiting-resubmission → active → completed + ↓ + expired (10 min TTL) +``` + +### Decision Cycle Model + +Each server (plan, annotate, review) previously used a one-shot promise for the user's decision. Now they use a **cycle model**: each deny resolves the current cycle and starts a new one. The approve/exit path resolves the cycle as final. + +Shared helper in `packages/server/session-handler.ts`: +- `createDecisionCycle()` — creates a resolvable cycle with `promise()`, `resolve()`, `startNew()` +- `resolveAndCycle(cycle, result, origin)` — resolves current cycle, starts new one if agent-originated, returns `{ awaitingResubmission: true }` flag + +### Session Matching + +When the agent resubmits, the daemon matches the new request to the existing suspended session using a **match key**: + +| Session Type | Match Key | Example | +|-------------|-----------|---------| +| Plan | `plan:${project}:${slug}` | `plan:plannotator:implementation-plan-2026-05-22` | +| Code Review | `review:${project}:${branch}` or `review:${prUrl}` | `review:plannotator:feat/session-persistence` | +| Annotate | `annotate:${filePath}` | `annotate:/path/to/README.md` | + +If a match is found: the session's `updateContent` method pushes new content, the store reactivates the session, and a `session-revision` WebSocket event notifies the frontend. + +If no match (different slug, different branch, different file): a new session is created as before. + +### Content Update + +Each server exposes a `handleUpdateContent` function that: +- Replaces the content in the server's closure (plan text, diff patch, markdown) +- Resets draft state +- Publishes a `session-revision` event to the frontend + +### Frontend + +All three surfaces (plan review, code review, annotate) handle the `awaitingResubmission` response from their feedback endpoints. When received: +- Show the "Feedback sent — waiting for agent to revise..." banner +- Subscribe to `session-revision` WebSocket events +- On revision: refresh content, clear annotations, reset awaiting state + +### CLI + +The CLI binary accepts `awaiting-resubmission` as a valid non-error status. It outputs the denial feedback and exits with code 0 — the agent reads the feedback and replans, same as always. The matching happens server-side; the agent doesn't know about session persistence. + +--- + +## Files Changed + +| File | What changed | +|------|-------------| +| `packages/shared/daemon-protocol.ts` | New `awaiting-resubmission` status, `session-revision` event family, protocol v2 | +| `packages/server/session-handler.ts` | `createDecisionCycle()` and `resolveAndCycle()` shared helpers | +| `packages/server/daemon/session-store.ts` | `suspend()`, `reactivate()` methods, `matchKey` field | +| `packages/server/daemon/session-factory.ts` | `createDecisionScope`, `registerPersistentDecision`, `findAwaitingSession`, matching + reactivation for all three types | +| `packages/server/daemon/server.ts` | Skip deletion timer for awaiting-resubmission sessions | +| `packages/server/index.ts` | Cycle model, `handleUpdateContent`, slug/getSnapshot on session | +| `packages/server/annotate.ts` | Cycle model, `handleUpdateContent` for file-based modes | +| `packages/server/review.ts` | Cycle model, `handleUpdateContent(rawPatch, gitRef)` | +| `apps/hook/server/index.ts` | Accept `awaiting-resubmission` status (exit 0, not error) | +| `packages/plannotator-plan-review/App.tsx` | `awaitingResubmission` state, deny handler check, `session-revision` subscription | +| `packages/plannotator-code-review/App.tsx` | Same as plan review, adapted for diff refresh | +| `packages/ui/components/CompletionBanner.tsx` | `awaiting` variant with spinner and cancel button | +| `AGENTS.md` | Documentation for new status, event family, resubmission flow | + +--- + +## What Does NOT Persist + +- **URL-based annotations** — the source URL might change, can't refresh +- **"Annotate last message" sessions** — ephemeral, no persistent source +- **Archive sessions** — read-only, no feedback cycle +- **Goal setup sessions** — one-shot Q&A, not a review cycle +- **Standalone/demo sessions** — no agent to resubmit + +--- + +## Recap + +1. Denied sessions stay alive instead of dying +2. The agent resubmits → same session updates in place +3. Works for plan, code review, and file-based annotate +4. Matching is by project+slug (plan), project+branch (review), or filepath (annotate) +5. 10-minute timeout if the agent doesn't come back +6. Agent doesn't need to know — matching is server-side +7. Shared `createDecisionCycle` helper eliminates duplication across three servers +8. Frontend shows amber "waiting" banner with cancel option +9. No changes to approve, exit, or standalone flows + +--- + +## Quiz + +**1.** What happens to a denied session's HTTP handler? +> It stays alive. `suspend()` does NOT call `disposeResources()` or clear `handleRequest`. + +**2.** How does the daemon know a new plan submission is a revision of an existing session? +> It computes a match key (`plan:${project}:${slug}`) and searches for an `awaiting-resubmission` session with the same key. + +**3.** What happens if the agent changes the plan's heading when resubmitting? +> Different heading → different slug → no match → new session. The old session expires after 10 minutes. + +**4.** Does the agent need to track session IDs or know about persistence? +> No. The CLI binary runs fresh each time. Matching is entirely server-side. + +**5.** What's the difference between `suspend()` and `complete()`? +> `complete()` sets terminal status, disposes resources, clears the HTTP handler. `suspend()` sets `awaiting-resubmission`, resolves waiters (so the CLI gets feedback), but keeps everything alive. + +**6.** How does the frontend know the content changed? +> A `session-revision` WebSocket event carrying the new content. The frontend subscribes when `awaitingResubmission` is true. + +**7.** What happens to a URL annotation session when denied? +> It completes normally (no persistence). URL sources can't be refreshed, so no match key is set. + +**8.** How long does the session wait for the agent to resubmit? +> 10 minutes. Then it expires via `cleanupExpired()`. From 17c77e1881f0bf1fbc45181dd7759e0d88e6c8d2 Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Sun, 24 May 2026 09:31:18 -0700 Subject: [PATCH 13/35] Remove redundant HTML overview from tracking --- goals/session-persistence/overview.html | 327 ------------------------ 1 file changed, 327 deletions(-) delete mode 100644 goals/session-persistence/overview.html diff --git a/goals/session-persistence/overview.html b/goals/session-persistence/overview.html deleted file mode 100644 index 67f32744..00000000 --- a/goals/session-persistence/overview.html +++ /dev/null @@ -1,327 +0,0 @@ - - - - - -Session Persistence — What You Need To Know - - - - - - - -
-

- feat/session-persistence -

- -
- -
- -
-

- Session Persistence -

-

- Denied sessions stay alive. The agent revises. The same session updates in place. No more starting over. -

-
- - -
-

User Experience

- -
-
-
- -

Before

-
-
    -
  1. 1 You review a plan, leave annotations, deny
    2. -
  2. 2 Session dies — completion screen, done
    3. -
  3. 3 Agent revises and resubmits
    4. -
  4. 4 Brand new session — new tab, no context, no diff
    5. -
  5. 5 Start over from scratch
    6. -
-
-
-
- -

After

-
-
    -
  1. 1 You review a plan, leave annotations, deny
    2. -
  2. 2 Banner: "Waiting for agent to revise..."
    3. -
  3. 3 Agent revises and resubmits
    4. -
  4. 4 Same session updates — diff shows what changed
    5. -
  5. 5 Review again — approve or deny with more feedback
    6. -
-
-
-
- - -
-

Works across all modes

-
-
-
📋
-

Plan Review

-

Agent revises the plan, session shows plan diff

-
-
-
🔍
-

Code Review

-

Agent makes changes, session refreshes with new diff

-
-
-
✏️
-

Annotate

-

Agent edits the file, session refreshes with updated content

-
-
-
- - - - - -
-

Session lifecycle

-
- - - - active - - - deny - - - awaiting-resubmission - - - agent resubmits - - - 10 min TTL - - expired - - - approve - - completed - - - - - - -
-
- - -
-

How sessions are matched

-
- - - - - - - - - - - - - - - - - - - - - - - - - -
ModeMatch Key
Planplan:project:slug
Reviewreview:project:branch
Annotateannotate:filepath
-
-

No match (heading changed, different branch, different file) → new session as before.

-
- - -
-

Files changed — 14 files, +423 / -143

-
- - Show file list → - -
- - - - - - - - - - - - - - - - -
daemon-protocol.tsNew status, event family, protocol v2
session-handler.tscreateDecisionCycle, resolveAndCycle helpers
session-store.tssuspend(), reactivate(), matchKey field
session-factory.tsMatching, persistent loop, all three types wired
server.ts (daemon)Skip deletion timer for awaiting sessions
index.ts (plan)Cycle model, updateContent, slug
annotate.tsCycle model, updateContent for files
review.tsCycle model, updateContent for diffs
hook/index.ts (CLI)Accept awaiting-resubmission status
plan-review App.tsxAwaiting state + revision subscription
code-review App.tsxAwaiting state + revision subscription
CompletionBanner.tsxAwaiting variant with spinner
AGENTS.mdDocumentation
-
-
-
- - -
-

What does NOT persist

-
-
- ✗ - URL-based annotations -
-
- ✗ - "Annotate last message" sessions -
-
- ✗ - Archive sessions (read-only) -
-
- ✗ - Standalone / demo sessions -
-
-
- - -
-

Recap

-
-
    -
  1. Denied sessions stay alive instead of dying
    2. -
  2. Agent resubmits → same session updates in place
    3. -
  3. Works for plan, code review, and file-based annotate
    4. -
  4. Matching by project+slug, project+branch, or filepath
    5. -
  5. 10-minute timeout if the agent doesn't come back
    6. -
  6. Agent doesn't need to know — matching is server-side
    7. -
  7. Shared createDecisionCycle eliminates duplication
    8. -
  8. Frontend shows amber "waiting" banner with cancel
    9. -
  9. No changes to approve, exit, or standalone flows
    10. -
-
-
- - -
-

Knowledge check

-
-
-
- -
- - - - - From 1c1f720907cc3bb752037be211295018a6168184 Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Sun, 24 May 2026 21:21:41 -0700 Subject: [PATCH 14/35] Add version history and diff support to annotate sessions Reuses the plan review's version infrastructure for file-based annotate sessions. Revisions now save to history, the session-revision event carries previousPlan and versionInfo, and the frontend's diff badge, diff view, and version browser activate automatically. Also updates decisions.md with product facts and revised design decisions. --- goals/session-persistence/decisions.md | 115 +++++++++++++++---------- packages/server/annotate.ts | 52 ++++++++++- 2 files changed, 118 insertions(+), 49 deletions(-) diff --git a/goals/session-persistence/decisions.md b/goals/session-persistence/decisions.md index d2e28a59..dee8330a 100644 --- a/goals/session-persistence/decisions.md +++ b/goals/session-persistence/decisions.md @@ -2,53 +2,83 @@ Tracking decisions made during PR #770 review and triage (2026-05-23). -## Decision 1: Remove persistence from code review +--- + +## Product Facts + +### Annotate Mode + +- A user can annotate a document. +- A user can annotate a URL. +- A user can annotate a folder. +- A user can annotate any of the above asynchronously across multiple docs/agents. +- A user can submit/flush annotations through to the agent. +- An agent can, but may not, create new versions of the document. +- If an agent does, a user should be notified. +- Agent revisions may change the state of a document. If it does, a user is notified. +- The user should be able to see those new document versions. Diff mode should allow them to see previous. +- Annotation mode has a gating process, by default it is not used. If it is used, we should assume the agent will iterate with the user until an approval. +- The gating process is similar to the planning flow. +- If an agent gates a document the user already has open, the gate buttons would appear. +- Otherwise, the normal button set appears. + +### Code Review Mode + +- A code review session can be associated with a project (local dir) or GitHub PR or GitLab MR. +- In local mode, there is the possibility of the diffs changing under the session. +- When I review code, I can make annotations. +- I want to send/flush the annotations to the agent, or I can publish to GitHub/GitLab comments (if in PR mode). +- Code review no longer needs to end. +- If an agent session initiates a new code review session from same directory, ideally it would open in the existing session. +- But I would need to be notified of this. +- I would need to be notified if diffs change. + +--- + +## Decisions + +### Decision 1: Code review sessions are long-lived **Status:** Decided -Code review persistence (awaiting-resubmission + session matching + content refresh) is being removed. The plan review persistence model doesn't fit code review. +Code review sessions no longer end after feedback. The session stays alive indefinitely. Memory/compute cost is negligible — each session is a JS closure in a Map, a few KB, zero CPU when idle. -**Why:** Plan persistence is "same document, new version" — the agent revises a plan and the user reviews the revision in context. Code review is different — a diff is a snapshot of file changes. When the agent modifies code based on feedback, it produces an entirely new diff. Keeping the same session and "refreshing" it conflates two separate review rounds. +The "awaiting-resubmission" persistence model (suspend → match → reactivate cycle) is being removed from code review. It doesn't fit — a diff is a moving target, not an iterative document. Instead: -**What changes:** -- Review sessions complete normally on feedback (one-shot decision, not a cycle) -- Session stays alive in the browser after feedback is sent (not completed/closed) -- Annotations flush when feedback is sent -- The session becomes a live review workspace, not part of a decision loop +- Feedback is sent (annotations flush, agent unblocks). +- Session stays open. User can keep viewing the diff. +- If diffs change (agent makes code changes, or a new review is triggered from the same directory), the user is notified and the session updates. +- If an agent initiates a new code review from the same directory, it should open in the existing session rather than creating a new one. -**Open question: what happens when diffs change?** +**Open questions:** +- Notification mechanism when diffs change under the session (file watcher? agent-triggered event? manual refresh?) +- Where does subsequent feedback go if the agent isn't listening? (Hook-based push? Queued for next agent invocation?) +- Cleanup strategy for long-lived sessions (daemon restart? idle timeout of hours? explicit close only?) -After feedback is sent, the agent makes code changes. The files on disk change. The user may want to see the updated diff. Options under consideration: +### Decision 2: Annotate sessions keep persistence -- **A.** Session becomes read-only after feedback. User runs `/plannotator-review` again for a fresh review. (Simplest, but friction.) -- **B.** Session has a "Refresh Diff" button. User pulls the latest diff on demand. Can annotate and send feedback again — but the agent isn't listening. Subsequent feedback needs a delivery mechanism. (Middle ground.) -- **C.** Session stays live and pushes feedback directly into the agent conversation via hook system, like external annotations. (Most ambitious, agent doesn't need to explicitly trigger review.) +**Status:** Decided -**Open question: where does subsequent feedback go?** +Annotate mode is similar to plan mode. Agent revisions may change the document. The user should be notified and see new versions. Diff mode should show previous versions. The gating process (when used) iterates until approval, same as planning. -Once the first feedback resolves the decision promise and the agent unblocks, there's no listener for more feedback. If the session stays alive and the user sends again: -- Write to a file the agent picks up? -- Push into the conversation via hooks? -- Just don't allow — one feedback per agent invocation? +Persistence stays as-is for annotate. The awaiting-resubmission model fits because the document is an iterative artifact. ---- +**Open question: folder annotation with multiple file changes.** Currently `updateContent` takes a single markdown string and the matchKey is `annotate:${filePath}` (the folder path). If the agent edits multiple files in the folder, we need a way to push per-file updates or trigger a re-fetch of the file list. Current design doesn't handle this. -## Decision 2: "Feedback sent" state should be calm, not loading +### Decision 3: "Feedback sent" state should be calm, not loading **Status:** Decided -After sending feedback (plan or code review), the UI should NOT show a spinner or "waiting" state. The session should feel settled — like archive mode. The user's annotations were sent. The plan/diff is still readable. The user can scroll, review what they wrote. +After sending feedback (plan deny, annotate, or code review), the UI should NOT show a spinner or "waiting" state. The session should feel settled. The user's annotations were sent. The content is still readable. The user can scroll around and review what they wrote. -If a new version arrives (plan resubmission), the content refreshes and the session comes back to life. +If a new version arrives (plan/annotate resubmission, or diff change in code review), the content refreshes and the session comes back to life. **What this means for the current code:** -- Replace the amber spinner `CompletionBanner` awaiting variant with a calm "Feedback sent" banner -- Don't disable buttons during awaiting — the session is readable, not blocked -- The `onCancel` escape hatch becomes less urgent since the user isn't trapped behind a spinner +- Replace the amber spinner `CompletionBanner` awaiting variant with a calm confirmation +- For plan/annotate: actions should be disabled until the revision arrives (the agent already has the feedback and is working — re-submitting before the revision arrives doesn't make sense) +- For code review: different model, TBD based on Decision 1 ---- - -## Decision 3: Fix the hot loop for frontend-originated sessions +### Decision 4: Fix the hot loop for frontend-originated sessions **Status:** Decided — will fix @@ -56,30 +86,23 @@ When a session is created from the Plannotator dashboard (origin: `"plannotator- **Fix:** In `registerPersistentDecision`, if `suspend()` returns a record with status !== `"awaiting-resubmission"`, break the loop and complete. ---- +### Decision 5: Clear external annotations on review resubmission -## Decision 4: Clear external annotations on review resubmission - -**Status:** Decided — will fix +**Status:** Decided — will fix (if any refresh mechanism is kept for reviews) `handleUpdateContent` in review.ts swaps the patch but keeps stale external annotations (lint results, agent comments) from the previous diff. Line numbers and file paths may no longer match. **Fix:** Call `externalAnnotations.clear()` inside `handleUpdateContent`. -*Note: If we remove persistence from code review (Decision 1), this becomes moot — there's no resubmission path. Keeping this noted in case we preserve any refresh mechanism.* - ---- +*May become moot depending on how Decision 1's refresh mechanism works.* -## Decision 5: Wire the cancel escape hatch +### Decision 6: Cancel / session expiry handling -**Status:** Deferred — depends on Decision 2 +**Status:** Deferred — depends on Decision 3 -If the awaiting state becomes calm/archive-like (Decision 2), the urgency drops. The user isn't stuck behind a spinner. But we still need to handle session expiry gracefully — when the 10-minute TTL fires server-side, the frontend should know. +If the feedback-sent state is calm (Decision 3), the user isn't trapped. But we still need to handle session expiry gracefully for plan/annotate — when the 10-minute TTL fires server-side, the frontend should know. -**Options:** -- Subscribe to `session-updated` daemon events (the expiry event family) -- Client-side timeout that matches server TTL -- Just let the session sit — user navigates away naturally +For code review (Decision 1), sessions live indefinitely, so expiry is a different question (idle cleanup, not agent-didn't-resubmit). --- @@ -87,10 +110,10 @@ If the awaiting state becomes calm/archive-like (Decision 2), the urgency drops. | # | Finding | Severity | Action | |---|---------|----------|--------| -| 1-2 | Frontend sessions spin in registerPersistentDecision | P1 | Fix (Decision 3) | +| 1-2 | Frontend sessions spin in registerPersistentDecision | P1 | Fix (Decision 4) | | 3+9 | --render-html resubmission shows stale content | P2 | Skip — narrow edge case | | 4 | OpenCode folder annotation mislabeled after persistence | nit | Skip — cosmetic | -| 5 | Stale external annotations after review resubmission | P2 | Fix (Decision 4), possibly moot after Decision 1 | +| 5 | Stale external annotations after review resubmission | P2 | Fix (Decision 5), possibly moot after Decision 1 | | 6 | waitForResult doesn't short-circuit on awaiting | P2 | Skip — current behavior is correct | -| 7-8 | Actions not disabled during awaiting | P2 | Redesign (Decision 2 — calm state, not disabled) | -| 10 | onCancel never wired, stuck spinner | nit | Deferred (Decision 5) | +| 7-8 | Actions not disabled during awaiting | P2 | Redesign per Decision 3 | +| 10 | onCancel never wired, stuck spinner | nit | Deferred (Decision 6) | diff --git a/packages/server/annotate.ts b/packages/server/annotate.ts index ab53e9c3..1d7ea906 100644 --- a/packages/server/annotate.ts +++ b/packages/server/annotate.ts @@ -20,6 +20,8 @@ import { warmFileListCache } from "@plannotator/shared/resolve-file"; import { contentHash, deleteDraft } from "./draft"; import { createExternalAnnotationHandler } from "./external-annotations"; import { saveConfig, detectGitUser, getServerConfig } from "./config"; +import { generateSlug, saveToHistory, getPlanVersion, getVersionCount, listVersions } from "./storage"; +import { detectProjectName } from "./project"; import { dirname, resolve as resolvePath } from "path"; import { isWSL } from "./browser"; import { createDecisionCycle, resolveAndCycle } from "./session-handler"; @@ -144,6 +146,25 @@ export async function createAnnotateSession( // Detect repo info (cached for this session) const repoInfo = await getRepoInfo(cwd); + // Version history (file-based modes only) + const isFileBased = mode === "annotate" || mode === "annotate-folder"; + const project = isFileBased ? ((await detectProjectName(cwd)) ?? "_unknown") : ""; + const slug = isFileBased ? generateSlug(markdown) : ""; + let previousPlan: string | null = null; + let versionInfo: { version: number; totalVersions: number; project: string } | null = null; + + if (isFileBased && markdown.trim()) { + const historyResult = saveToHistory(project, slug, markdown); + previousPlan = historyResult.version > 1 + ? getPlanVersion(project, slug, historyResult.version - 1) + : null; + versionInfo = { + version: historyResult.version, + totalVersions: getVersionCount(project, slug), + project, + }; + } + type AnnotateDecisionResult = { feedback: string; annotations: unknown[]; exit?: boolean; approved?: boolean }; const decisionCycle = createDecisionCycle(); @@ -165,12 +186,30 @@ export async function createAnnotateSession( shareBaseUrl, pasteApiUrl, repoInfo, + previousPlan, + versionInfo, projectRoot: folderPath || cwd, isWSL: wslFlag, serverConfig: getServerConfig(gitUser), }); } + // API: Get a specific version from history + if (url.pathname === "/api/plan/version" && isFileBased) { + const vParam = url.searchParams.get("v"); + if (!vParam) return new Response("Missing v parameter", { status: 400 }); + const v = parseInt(vParam, 10); + if (isNaN(v) || v < 1) return new Response("Invalid version number", { status: 400 }); + const content = getPlanVersion(project, slug, v); + if (content === null) return Response.json({ error: "Version not found" }, { status: 404 }); + return Response.json({ plan: content, version: v }); + } + + // API: List all versions + if (url.pathname === "/api/plan/versions" && isFileBased) { + return Response.json({ project, slug, versions: listVersions(project, slug) }); + } + // API: Update user config (write-back to ~/.plannotator/config.json) if (url.pathname === "/api/config" && req.method === "POST") { try { @@ -294,13 +333,20 @@ export async function createAnnotateSession( }); }; - const isFileBased = mode === "annotate" || mode === "annotate-folder"; - function handleUpdateContent(newMarkdown: string) { markdown = newMarkdown; + const historyResult = saveToHistory(project, slug, newMarkdown); + previousPlan = historyResult.version > 1 + ? getPlanVersion(project, slug, historyResult.version - 1) + : null; + versionInfo = { + version: historyResult.version, + totalVersions: getVersionCount(project, slug), + project, + }; deleteDraft(draftKey); draftKey = contentHash(newMarkdown); - options.sessionEvents?.publishEvent("session-revision", { plan: newMarkdown }); + options.sessionEvents?.publishEvent("session-revision", { plan: newMarkdown, previousPlan, versionInfo }); } return { From 9d1f92117637be9abb71f11d1b4852c71df93076 Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Mon, 25 May 2026 10:36:00 -0700 Subject: [PATCH 15/35] Long-lived code review sessions with idle status MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace the awaiting-resubmission persistence model for code review with a new "idle" session status. After sending feedback, the session stays alive and interactive — the user can annotate and send again. Agent reviews from the same directory attach to the idle session instead of creating a new one. The frontend shows a calm "Feedback sent" banner instead of a spinner. --- apps/hook/server/index.ts | 2 +- packages/plannotator-code-review/App.tsx | 26 +++++++------ packages/server/daemon/server.ts | 2 +- packages/server/daemon/session-factory.ts | 42 +++++++++++++++++---- packages/server/daemon/session-store.ts | 15 +++++++- packages/server/review.ts | 8 ++-- packages/shared/daemon-protocol.ts | 1 + packages/ui/components/CompletionBanner.tsx | 7 ++-- 8 files changed, 73 insertions(+), 30 deletions(-) diff --git a/apps/hook/server/index.ts b/apps/hook/server/index.ts index 53a6a71d..adf400f6 100644 --- a/apps/hook/server/index.ts +++ b/apps/hook/server/index.ts @@ -689,7 +689,7 @@ async function runDaemonSessionRequest(request: PluginRequest, options: { plugin await cancelCreatedSession(); fail(completed.error.code, completed.error.message); } - if (completed.session.status !== "completed" && completed.session.status !== "awaiting-resubmission") { + if (completed.session.status !== "completed" && completed.session.status !== "awaiting-resubmission" && completed.session.status !== "idle") { fail( completed.session.status, completed.session.error ?? `Plannotator session ${completed.session.id} ended with status ${completed.session.status}.`, diff --git a/packages/plannotator-code-review/App.tsx b/packages/plannotator-code-review/App.tsx index 31b53a47..afd36d0d 100644 --- a/packages/plannotator-code-review/App.tsx +++ b/packages/plannotator-code-review/App.tsx @@ -239,7 +239,7 @@ const ReviewApp: React.FC<{ __embedded?: boolean; headerLeft?: React.ReactNode; const [isApproving, setIsApproving] = useState(false); const [isExiting, setIsExiting] = useState(false); const [submitted, setSubmitted] = useState<'approved' | 'feedback' | 'exited' | false>(false); - const [awaitingResubmission, setAwaitingResubmission] = useState(false); + const [feedbackSent, setFeedbackSent] = useState(false); const [showApproveWarning, setShowApproveWarning] = useState(false); const [showExitWarning, setShowExitWarning] = useState(false); const [sharingEnabled, setSharingEnabled] = useState(true); @@ -310,9 +310,9 @@ const ReviewApp: React.FC<{ __embedded?: boolean; headerLeft?: React.ReactNode; const { externalAnnotations, updateExternalAnnotation, deleteExternalAnnotation } = useExternalAnnotations({ enabled: !!origin }); const agentJobs = useAgentJobs({ enabled: !!origin }); - // Listen for session-revision events (diff refresh after feedback) + // Listen for session-revision events (agent pushed a new diff) useEffect(() => { - if (!origin || !awaitingResubmission) return; + if (!origin) return; const unsubscribe = subscribeToDaemonSessionFamily("session-revision", (msg) => { if (msg.type !== "event" || !msg.payload) return; const revision = msg.payload as { rawPatch?: string; gitRef?: string }; @@ -324,13 +324,13 @@ const ReviewApp: React.FC<{ __embedded?: boolean; headerLeft?: React.ReactNode; storeApi.getState().setLocalAnnotations([]); storeApi.getState().selectAnnotation(null); storeApi.getState().setPendingSelection(null); - setAwaitingResubmission(false); + setFeedbackSent(false); setSubmitted(false); setIsSendingFeedback(false); } }); return unsubscribe; - }, [origin, awaitingResubmission, storeApi]); + }, [origin, storeApi]); // Tour dialog state — opens as an overlay instead of a dock panel const [tourDialogJobId, setTourDialogJobId] = useState(null); @@ -1553,9 +1553,13 @@ const ReviewApp: React.FC<{ __embedded?: boolean; headerLeft?: React.ReactNode; }); if (res.ok) { const data = await res.json().catch(() => ({})); - if (data.awaitingResubmission) { - setAwaitingResubmission(true); + if (data.feedbackDelivered) { + setFeedbackSent(true); setIsSendingFeedback(false); + storeApi.getState().setLocalAnnotations([]); + storeApi.getState().selectAnnotation(null); + storeApi.getState().setPendingSelection(null); + setTimeout(() => setFeedbackSent(false), 5000); } else { setSubmitted('feedback'); } @@ -1568,7 +1572,7 @@ const ReviewApp: React.FC<{ __embedded?: boolean; headerLeft?: React.ReactNode; setTimeout(() => setCopyFeedback(null), 2000); setIsSendingFeedback(false); } - }, [totalAnnotationCount, feedbackMarkdown, allAnnotations]); + }, [totalAnnotationCount, feedbackMarkdown, allAnnotations, storeApi]); // Exit review session without sending any feedback const handleExit = useCallback(async () => { @@ -2185,9 +2189,9 @@ const ReviewApp: React.FC<{ __embedded?: boolean; headerLeft?: React.ReactNode; {/* Embedded completion banner — inline, non-blocking */} {__embedded && !legacyTabMode && ( )} diff --git a/packages/server/daemon/server.ts b/packages/server/daemon/server.ts index 15217370..21829db8 100644 --- a/packages/server/daemon/server.ts +++ b/packages/server/daemon/server.ts @@ -444,7 +444,7 @@ export function createDaemonFetchHandler(options: DaemonServerOptions): DaemonFe requestContext?.disableIdleTimeout?.(); const completed = await store.waitForResult(id); const response = json({ ok: true, session: store.summary(completed), result: completed.result ?? null }); - if (completed.status !== "awaiting-resubmission") { + if (completed.status !== "awaiting-resubmission" && completed.status !== "idle") { const timer = setTimeout(() => void store.delete(id), RESULT_DELETE_GRACE_MS); timer.unref?.(); } diff --git a/packages/server/daemon/session-factory.ts b/packages/server/daemon/session-factory.ts index 6ae3a866..f27b01ce 100644 --- a/packages/server/daemon/session-factory.ts +++ b/packages/server/daemon/session-factory.ts @@ -165,6 +165,31 @@ function registerPersistentDecision( return cleanup; } +function registerReviewDecision( + context: DaemonFetchContext, + id: string, + session: { waitForDecision: () => Promise; dispose: () => void | Promise }, +): () => void | Promise { + const { disposed, cleanup } = createDecisionScope(session.dispose); + + const decisionLoop = async () => { + while (true) { + const result = await Promise.race([session.waitForDecision(), disposed]); + const record = context.store.idle(id, result); + if (!record || record.status !== "idle") return; + } + }; + + void decisionLoop().catch((err) => { + const record = context.store.get(id); + if (record && (record.status === "active" || record.status === "idle")) { + context.store.fail(id, err instanceof Error ? err.message : String(err)); + } + }); + + return cleanup; +} + function resolvePlanFilePath(planFilePath: string, cwd: string): string { const requestedPath = isAbsolute(planFilePath) ? planFilePath @@ -532,16 +557,17 @@ export function createDaemonSessionFactory(options: DaemonSessionFactoryOptions) } const sessionRefs = new Map(); - // Search for an awaiting-resubmission session with the given matchKey. - // Side effect: prunes terminal sessions from sessionRefs during the scan. - function findAwaitingSession(store: DaemonFetchContext["store"], matchKey: string) { + const RESUBMIT_STATUSES = new Set(["awaiting-resubmission"]); + const RESUBMIT_OR_IDLE_STATUSES = new Set(["awaiting-resubmission", "idle"]); + + function findMatchingSession(store: DaemonFetchContext["store"], matchKey: string, matchStatuses = RESUBMIT_STATUSES) { for (const [sessionId, ref] of sessionRefs) { const record = store.get(sessionId); if (!record || record.status === "completed" || record.status === "expired" || record.status === "failed" || record.status === "cancelled") { sessionRefs.delete(sessionId); continue; } - if (record.status !== "awaiting-resubmission") continue; + if (!matchStatuses.has(record.status)) continue; if (record.matchKey !== matchKey) continue; return { record, session: ref.session }; } @@ -581,7 +607,7 @@ export function createDaemonSessionFactory(options: DaemonSessionFactoryOptions) const plan = await readPlanRequest(request, cwd); const matchKey = `plan:${project}:${generateSlug(plan)}`; - const existing = findAwaitingSession(context.store, matchKey); + const existing = findMatchingSession(context.store, matchKey); if (existing && existing.session.updateContent) { existing.session.updateContent(plan); context.store.reactivate(existing.record.id); @@ -663,7 +689,7 @@ export function createDaemonSessionFactory(options: DaemonSessionFactoryOptions) const matchKey = isFileBased ? `annotate:${input.filePath}` : undefined; if (matchKey) { - const existing = findAwaitingSession(context.store, matchKey); + const existing = findMatchingSession(context.store, matchKey); if (existing && existing.session.updateContent) { (existing.session.updateContent as (m: string) => void)(input.markdown); context.store.reactivate(existing.record.id); @@ -715,7 +741,7 @@ export function createDaemonSessionFactory(options: DaemonSessionFactoryOptions) ? `review:${input.prMetadata.url}` : branch ? `review:${project}:${branch}` : `review:${project}`; - const existingReview = findAwaitingSession(context.store, reviewMatchKey); + const existingReview = findMatchingSession(context.store, reviewMatchKey, RESUBMIT_OR_IDLE_STATUSES); if (existingReview && existingReview.session.updateContent) { (existingReview.session.updateContent as (rawPatch: string, gitRef: string) => void)(input.rawPatch, input.gitRef); context.store.reactivate(existingReview.record.id); @@ -767,7 +793,7 @@ export function createDaemonSessionFactory(options: DaemonSessionFactoryOptions) matchKey: reviewMatchKey, ttlMs, handleRequest: session.handleRequest, - dispose: registerPersistentDecision(context, id, session), + dispose: registerReviewDecision(context, id, session), remoteShare, snapshot: session.getSnapshot ?? (() => ({ rawPatch: input.rawPatch, diff --git a/packages/server/daemon/session-store.ts b/packages/server/daemon/session-store.ts index 9522a317..3b8910d5 100644 --- a/packages/server/daemon/session-store.ts +++ b/packages/server/daemon/session-store.ts @@ -272,9 +272,22 @@ export class DaemonSessionStore { return record; } + idle(id: string, result?: TResult): DaemonSessionRecord | undefined { + const record = this.sessions.get(id) as DaemonSessionRecord | undefined; + if (!record || TERMINAL_STATUSES.has(record.status) || record.status === "idle") return undefined; + record.status = "idle"; + if (result !== undefined) record.result = result as TResult; + const now = this.now(); + record.updatedAt = iso(now); + record.expiresAt = record.ttlMs !== undefined ? iso(now + record.ttlMs) : undefined; + this.resolveWaiters(record); + this.emit("session-updated", record); + return record; + } + reactivate(id: string): DaemonSessionRecord | undefined { const record = this.sessions.get(id); - if (!record || record.status !== "awaiting-resubmission") return record; + if (!record || (record.status !== "awaiting-resubmission" && record.status !== "idle")) return record; record.status = "active"; record.result = undefined; const now = this.now(); diff --git a/packages/server/review.ts b/packages/server/review.ts index 2c5dc120..0abf5d9d 100644 --- a/packages/server/review.ts +++ b/packages/server/review.ts @@ -1040,7 +1040,7 @@ export async function createReviewSession( // API: Exit review session without feedback if (url.pathname === "/api/exit" && req.method === "POST") { deleteDraft(draftKey); - decisionCycle.resolve({ approved: false, feedback: "", annotations: [], exit: true }); + resolveAndCycle(decisionCycle, { approved: false, feedback: "", annotations: [], exit: true }, origin); return Response.json({ ok: true }); } @@ -1057,11 +1057,9 @@ export async function createReviewSession( deleteDraft(draftKey); const isApproved = body.approved ?? false; const result = { approved: isApproved, feedback: body.feedback || "", annotations: body.annotations || [], agentSwitch: body.agentSwitch }; - const resubmit = isApproved - ? (decisionCycle.resolve(result), {}) - : resolveAndCycle(decisionCycle, result, origin); + resolveAndCycle(decisionCycle, result, origin); - return Response.json({ ok: true, ...resubmit }); + return Response.json({ ok: true, feedbackDelivered: !isApproved || undefined }); } catch (err) { const message = err instanceof Error ? err.message : "Failed to process feedback"; diff --git a/packages/shared/daemon-protocol.ts b/packages/shared/daemon-protocol.ts index c2b751b3..531979e1 100644 --- a/packages/shared/daemon-protocol.ts +++ b/packages/shared/daemon-protocol.ts @@ -41,6 +41,7 @@ export type DaemonSessionMode = PluginSessionMode; export type DaemonSessionView = (typeof PLANNOTATOR_DAEMON_SESSION_VIEWS)[number]; export type DaemonSessionStatus = | "active" + | "idle" | "awaiting-resubmission" | "completed" | "cancelled" diff --git a/packages/ui/components/CompletionBanner.tsx b/packages/ui/components/CompletionBanner.tsx index 894b825f..c80c613e 100644 --- a/packages/ui/components/CompletionBanner.tsx +++ b/packages/ui/components/CompletionBanner.tsx @@ -1,5 +1,5 @@ interface CompletionBannerProps { - submitted: 'approved' | 'denied' | 'feedback' | 'exited' | 'awaiting' | null | false; + submitted: 'approved' | 'denied' | 'feedback' | 'exited' | 'awaiting' | 'feedback-sent' | null | false; title: string; subtitle: string; onCancel?: () => void; @@ -10,13 +10,14 @@ export function CompletionBanner({ submitted, title, subtitle, onCancel }: Compl const isApproved = submitted === 'approved'; const isAwaiting = submitted === 'awaiting'; + const isFeedbackSent = submitted === 'feedback-sent'; return (
) : ( - {isApproved ? ( + {isApproved || isFeedbackSent ? ( ) : ( Date: Mon, 25 May 2026 17:15:22 -0700 Subject: [PATCH 16/35] Hide submit buttons while idle, no auto-dismiss After feedback is sent and the session is idle (no agent listening), the Send Feedback and Approve buttons disappear. They reappear when the agent reactivates the session via session-revision. Keyboard shortcuts are also blocked while idle. --- packages/plannotator-code-review/App.tsx | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/packages/plannotator-code-review/App.tsx b/packages/plannotator-code-review/App.tsx index afd36d0d..16f95a3a 100644 --- a/packages/plannotator-code-review/App.tsx +++ b/packages/plannotator-code-review/App.tsx @@ -1559,7 +1559,6 @@ const ReviewApp: React.FC<{ __embedded?: boolean; headerLeft?: React.ReactNode; storeApi.getState().setLocalAnnotations([]); storeApi.getState().selectAnnotation(null); storeApi.getState().setPendingSelection(null); - setTimeout(() => setFeedbackSent(false), 5000); } else { setSubmitted('feedback'); } @@ -1770,7 +1769,7 @@ const ReviewApp: React.FC<{ __embedded?: boolean; headerLeft?: React.ReactNode; // If the platform post dialog is open, Cmd+Enter submits it if (platformCommentDialog) { - if (submitted || isPlatformActioning) return; + if (submitted || feedbackSent || isPlatformActioning) return; const isApproveAction = platformCommentDialog.action === 'approve'; const hasTargets = platformCommentDialog.plan.targets.length > 0; const canSubmit = isApproveAction || hasTargets || platformGeneralComment.trim(); @@ -1783,7 +1782,7 @@ const ReviewApp: React.FC<{ __embedded?: boolean; headerLeft?: React.ReactNode; const tag = (e.target as HTMLElement)?.tagName; if (tag === 'INPUT' || tag === 'TEXTAREA') return; if (showExportModal || showNoAnnotationsDialog || showApproveWarning || showExitWarning) return; - if (submitted || isSendingFeedback || isApproving || isExiting || isPlatformActioning) return; + if (submitted || feedbackSent || isSendingFeedback || isApproving || isExiting || isPlatformActioning) return; if (!origin) return; // Demo mode e.preventDefault(); @@ -1811,7 +1810,7 @@ const ReviewApp: React.FC<{ __embedded?: boolean; headerLeft?: React.ReactNode; }, [ showExportModal, showNoAnnotationsDialog, showApproveWarning, showExitWarning, platformCommentDialog, platformGeneralComment, - submitted, isSendingFeedback, isApproving, isExiting, isPlatformActioning, + submitted, feedbackSent, isSendingFeedback, isApproving, isExiting, isPlatformActioning, origin, platformMode, platformLabel, platformUser, prMetadata, totalAnnotationCount, openPlatformDialog, handleApprove, handleSendFeedback, handlePlatformAction ]); @@ -1957,7 +1956,7 @@ const ReviewApp: React.FC<{ __embedded?: boolean; headerLeft?: React.ReactNode;
- {origin && !submitted ? ( + {origin && !submitted && !feedbackSent ? ( <> {/* Destination dropdown (PR mode only) */} {prMetadata && ( From f26fa655a1fd8911e535d2544e0256ae7897ed09 Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Mon, 25 May 2026 17:58:22 -0700 Subject: [PATCH 17/35] Update decisions.md with implemented code review lifecycle Document the actual idle flow, resolve open questions, and mark Decision 1 and 3 as implemented. --- goals/session-persistence/decisions.md | 38 ++++++++++++++++---------- 1 file changed, 23 insertions(+), 15 deletions(-) diff --git a/goals/session-persistence/decisions.md b/goals/session-persistence/decisions.md index dee8330a..3ea13e32 100644 --- a/goals/session-persistence/decisions.md +++ b/goals/session-persistence/decisions.md @@ -39,21 +39,28 @@ Tracking decisions made during PR #770 review and triage (2026-05-23). ### Decision 1: Code review sessions are long-lived -**Status:** Decided +**Status:** Implemented -Code review sessions no longer end after feedback. The session stays alive indefinitely. Memory/compute cost is negligible — each session is a JS closure in a Map, a few KB, zero CPU when idle. +Code review sessions use a new `"idle"` daemon status. The flow: -The "awaiting-resubmission" persistence model (suspend → match → reactivate cycle) is being removed from code review. It doesn't fit — a diff is a moving target, not an iterative document. Instead: +``` +agent → plannotator review (CLI opens, blocks) → session active +session → user annotates → sends feedback → submit (CLI closes) +session → idle (user can browse and annotate, but no submit buttons — nobody is listening) +agent → plannotator review again (CLI opens) → reactivates the idle session +(repeats indefinitely) +``` -- Feedback is sent (annotations flush, agent unblocks). -- Session stays open. User can keep viewing the diff. -- If diffs change (agent makes code changes, or a new review is triggered from the same directory), the user is notified and the session updates. -- If an agent initiates a new code review from the same directory, it should open in the existing session rather than creating a new one. +Key behaviors: +- After feedback: session transitions to `idle` via `store.idle()`. The HTTP handler stays alive, resources stay alive. The user can browse the diff and make annotations, but Send Feedback / Approve buttons are hidden (no agent to receive them). +- On reactivation: agent triggers `plannotator review` from the same directory/branch. The daemon finds the idle session by matchKey, pushes the new diff via `updateContent`, and calls `store.reactivate()`. The frontend receives a `session-revision` WebSocket event, updates the diff, and re-shows the submit buttons. +- Infinite cycle: this repeats as many times as needed. No counter, no limit. +- Cleanup: idle sessions use the original session TTL (hours, not the 10-minute awaiting TTL). They expire via normal TTL cleanup or daemon restart. -**Open questions:** -- Notification mechanism when diffs change under the session (file watcher? agent-triggered event? manual refresh?) -- Where does subsequent feedback go if the agent isn't listening? (Hook-based push? Queued for next agent invocation?) -- Cleanup strategy for long-lived sessions (daemon restart? idle timeout of hours? explicit close only?) +**Resolved questions:** +- Notification when diffs change: agent-triggered via `session-revision` event. No file watcher (user can manually switch diff type to refresh). +- Subsequent feedback without agent: not possible — submit buttons are hidden while idle. +- Cleanup: normal TTL expiry. ### Decision 2: Annotate sessions keep persistence @@ -67,14 +74,15 @@ Persistence stays as-is for annotate. The awaiting-resubmission model fits becau ### Decision 3: "Feedback sent" state should be calm, not loading -**Status:** Decided +**Status:** Implemented (code review), pending (plan/annotate) -After sending feedback (plan deny, annotate, or code review), the UI should NOT show a spinner or "waiting" state. The session should feel settled. The user's annotations were sent. The content is still readable. The user can scroll around and review what they wrote. +**Code review:** After sending feedback, the `CompletionBanner` shows a green checkmark with "Feedback sent / Your annotations were delivered to the agent." The banner persists until the agent reactivates (no auto-dismiss). Submit buttons disappear. The session stays browsable. -If a new version arrives (plan/annotate resubmission, or diff change in code review), the content refreshes and the session comes back to life. +**Plan/annotate:** Still uses the amber spinner "Waiting for agent to revise..." variant. This should eventually be made calmer, but it's lower priority because plan/annotate persistence works correctly (agent WILL resubmit). **What this means for the current code:** -- Replace the amber spinner `CompletionBanner` awaiting variant with a calm confirmation +- Code review uses `'feedback-sent'` CompletionBanner variant (green checkmark, not spinner) +- Plan/annotate still uses `'awaiting'` variant (amber spinner) — acceptable for now - For plan/annotate: actions should be disabled until the revision arrives (the agent already has the feedback and is working — re-submitting before the revision arrives doesn't make sense) - For code review: different model, TBD based on Decision 1 From 714847181c3645d15ca66de2e1d1008a86558e87 Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Mon, 25 May 2026 18:55:26 -0700 Subject: [PATCH 18/35] Fix 4 review findings: idle TTL, late waiter, self-refresh, temp cleanup 1. Idle sessions with no ttlMs now get a fallback TTL instead of living forever (uses AWAITING_RESUBMISSION_TTL_MS as fallback) 2. waitForResult returns immediately for idle sessions with results instead of hanging forever on late agent checks 3. Review handleUpdateContent re-runs the diff with current user settings instead of accepting a pre-computed patch, and clears currentError. Preserves user's diff type and base branch choice. 4. Temp worktree cleanup on the review session reuse path --- packages/server/daemon/session-factory.ts | 5 +++-- packages/server/daemon/session-store.ts | 6 +++--- packages/server/review.ts | 17 +++++++++++------ 3 files changed, 17 insertions(+), 11 deletions(-) diff --git a/packages/server/daemon/session-factory.ts b/packages/server/daemon/session-factory.ts index f27b01ce..155b041f 100644 --- a/packages/server/daemon/session-factory.ts +++ b/packages/server/daemon/session-factory.ts @@ -553,7 +553,7 @@ export function createDaemonSessionFactory(options: DaemonSessionFactoryOptions) waitForDecision: () => Promise; dispose: () => void | Promise; // eslint-disable-next-line @typescript-eslint/no-explicit-any - updateContent?: (...args: any[]) => void; + updateContent?: (...args: any[]) => void | Promise; } const sessionRefs = new Map(); @@ -743,7 +743,8 @@ export function createDaemonSessionFactory(options: DaemonSessionFactoryOptions) const existingReview = findMatchingSession(context.store, reviewMatchKey, RESUBMIT_OR_IDLE_STATUSES); if (existingReview && existingReview.session.updateContent) { - (existingReview.session.updateContent as (rawPatch: string, gitRef: string) => void)(input.rawPatch, input.gitRef); + await Promise.resolve(input.onCleanup?.()).catch(() => {}); + await existingReview.session.updateContent(); context.store.reactivate(existingReview.record.id); return existingReview.record; } diff --git a/packages/server/daemon/session-store.ts b/packages/server/daemon/session-store.ts index 3b8910d5..2ddb552c 100644 --- a/packages/server/daemon/session-store.ts +++ b/packages/server/daemon/session-store.ts @@ -279,7 +279,7 @@ export class DaemonSessionStore { if (result !== undefined) record.result = result as TResult; const now = this.now(); record.updatedAt = iso(now); - record.expiresAt = record.ttlMs !== undefined ? iso(now + record.ttlMs) : undefined; + record.expiresAt = iso(now + (record.ttlMs ?? AWAITING_RESUBMISSION_TTL_MS)); this.resolveWaiters(record); this.emit("session-updated", record); return record; @@ -292,7 +292,7 @@ export class DaemonSessionStore { record.result = undefined; const now = this.now(); record.updatedAt = iso(now); - record.expiresAt = record.ttlMs !== undefined ? iso(now + record.ttlMs) : undefined; + record.expiresAt = iso(now + (record.ttlMs ?? AWAITING_RESUBMISSION_TTL_MS)); this.emit("session-updated", record); return record; } @@ -314,7 +314,7 @@ export class DaemonSessionStore { waitForResult(id: string): Promise> { const record = this.sessions.get(id) as DaemonSessionRecord | undefined; if (!record) return Promise.reject(new Error(`Session not found: ${id}`)); - if (TERMINAL_STATUSES.has(record.status)) return Promise.resolve(record); + if (TERMINAL_STATUSES.has(record.status) || (record.status === "idle" && record.result !== undefined)) return Promise.resolve(record); return new Promise((resolve, reject) => { const waiters = this.waiters.get(id) ?? []; waiters.push({ resolve: resolve as (record: DaemonSessionRecord) => void, reject }); diff --git a/packages/server/review.ts b/packages/server/review.ts index 0abf5d9d..9b086e00 100644 --- a/packages/server/review.ts +++ b/packages/server/review.ts @@ -131,7 +131,7 @@ export interface ReviewSession { waitForDecision: ReviewServerResult["waitForDecision"]; setServerUrl: (url: string) => void; dispose: () => void; - updateContent?: (rawPatch: string, gitRef: string) => void; + updateContent?: () => Promise; getSnapshot?: () => unknown; } @@ -1168,12 +1168,17 @@ export async function createReviewSession( const exitHandler = () => agentJobs.killAll(); process.once("exit", exitHandler); - function handleUpdateContent(rawPatch: string, gitRef: string) { - currentPatch = rawPatch; - currentGitRef = gitRef; + async function handleUpdateContent() { + const defaultCwd = gitContext?.cwd; + const result = await runVcsDiff(currentDiffType, currentBase, defaultCwd, { + hideWhitespace: currentHideWhitespace, + }); + currentPatch = result.patch; + currentGitRef = result.label; + currentError = result.error; deleteDraft(draftKey); - draftKey = contentHash(rawPatch); - options.sessionEvents?.publishEvent("session-revision", { rawPatch, gitRef }); + draftKey = contentHash(result.patch); + options.sessionEvents?.publishEvent("session-revision", { rawPatch: result.patch, gitRef: result.label }); } return { From caa4865ec5b8fe79a1a6dd19f940a326fceb9180 Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Mon, 25 May 2026 19:34:17 -0700 Subject: [PATCH 19/35] Fix PR mode review reuse: pass fetched patch instead of local diff For PR reviews, the diff comes from the GitHub/GitLab API, not from local git. handleUpdateContent now accepts an optional pre-computed patch for PR mode, falling back to self-refresh for local reviews. --- packages/server/daemon/session-factory.ts | 5 +++- packages/server/review.ts | 31 +++++++++++++++-------- 2 files changed, 24 insertions(+), 12 deletions(-) diff --git a/packages/server/daemon/session-factory.ts b/packages/server/daemon/session-factory.ts index 155b041f..27760b6d 100644 --- a/packages/server/daemon/session-factory.ts +++ b/packages/server/daemon/session-factory.ts @@ -744,7 +744,10 @@ export function createDaemonSessionFactory(options: DaemonSessionFactoryOptions) const existingReview = findMatchingSession(context.store, reviewMatchKey, RESUBMIT_OR_IDLE_STATUSES); if (existingReview && existingReview.session.updateContent) { await Promise.resolve(input.onCleanup?.()).catch(() => {}); - await existingReview.session.updateContent(); + await existingReview.session.updateContent( + input.prMetadata ? input.rawPatch : undefined, + input.prMetadata ? input.gitRef : undefined, + ); context.store.reactivate(existingReview.record.id); return existingReview.record; } diff --git a/packages/server/review.ts b/packages/server/review.ts index 9b086e00..b8d3476c 100644 --- a/packages/server/review.ts +++ b/packages/server/review.ts @@ -131,7 +131,7 @@ export interface ReviewSession { waitForDecision: ReviewServerResult["waitForDecision"]; setServerUrl: (url: string) => void; dispose: () => void; - updateContent?: () => Promise; + updateContent?: (precomputedPatch?: string, precomputedGitRef?: string) => Promise; getSnapshot?: () => unknown; } @@ -1168,17 +1168,26 @@ export async function createReviewSession( const exitHandler = () => agentJobs.killAll(); process.once("exit", exitHandler); - async function handleUpdateContent() { - const defaultCwd = gitContext?.cwd; - const result = await runVcsDiff(currentDiffType, currentBase, defaultCwd, { - hideWhitespace: currentHideWhitespace, - }); - currentPatch = result.patch; - currentGitRef = result.label; - currentError = result.error; + async function handleUpdateContent(precomputedPatch?: string, precomputedGitRef?: string) { + let patch: string; + let label: string; + if (precomputedPatch !== undefined) { + patch = precomputedPatch; + label = precomputedGitRef ?? currentGitRef; + currentError = undefined; + } else { + const result = await runVcsDiff(currentDiffType, currentBase, gitContext?.cwd, { + hideWhitespace: currentHideWhitespace, + }); + patch = result.patch; + label = result.label; + currentError = result.error; + } + currentPatch = patch; + currentGitRef = label; deleteDraft(draftKey); - draftKey = contentHash(result.patch); - options.sessionEvents?.publishEvent("session-revision", { rawPatch: result.patch, gitRef: result.label }); + draftKey = contentHash(patch); + options.sessionEvents?.publishEvent("session-revision", { rawPatch: patch, gitRef: label }); } return { From 1a56bd081a181d8fc1df2e4783ff872c73d71a5f Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Mon, 25 May 2026 20:28:53 -0700 Subject: [PATCH 20/35] Fix plan diff base tracking and linked-doc annotation leak 1. usePlanDiff now updates the diff base on every revision, not just the first. Previously the diff always compared against v1 after multiple deny/resubmit cycles. 2. Clear linked-doc annotation cache on session revision so stale annotations from linked files don't persist into the next plan version. --- packages/plannotator-plan-review/App.tsx | 1 + packages/ui/hooks/useLinkedDoc.ts | 3 +++ packages/ui/hooks/usePlanDiff.ts | 7 +++---- 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/packages/plannotator-plan-review/App.tsx b/packages/plannotator-plan-review/App.tsx index 44404f8d..3ad80e14 100644 --- a/packages/plannotator-plan-review/App.tsx +++ b/packages/plannotator-plan-review/App.tsx @@ -605,6 +605,7 @@ const App: React.FC<{ __embedded?: boolean; headerLeft?: React.ReactNode; onOpen setGlobalAttachments([]); setSelectedAnnotationId(null); setSelectedCodeAnnotationId(null); + linkedDocHook.clearCache(); setAwaitingResubmission(false); setSubmitted(null); setIsSubmitting(false); diff --git a/packages/ui/hooks/useLinkedDoc.ts b/packages/ui/hooks/useLinkedDoc.ts index 9993f35b..008ec3c8 100644 --- a/packages/ui/hooks/useLinkedDoc.ts +++ b/packages/ui/hooks/useLinkedDoc.ts @@ -64,6 +64,8 @@ export interface UseLinkedDocReturn { getDocAnnotations: () => Map; /** Reactive count of annotations on non-active documents (updates on open() and back()) */ docAnnotationCount: number; + /** Clear all cached linked-doc annotations (used on session revision) */ + clearCache: () => void; } const HIGHLIGHT_REAPPLY_DELAY = 100; @@ -297,5 +299,6 @@ export function useLinkedDoc(options: UseLinkedDocOptions): UseLinkedDocReturn { dismissError, getDocAnnotations, docAnnotationCount, + clearCache: () => { docCache.current.clear(); setDocAnnotationCount(0); }, }; } diff --git a/packages/ui/hooks/usePlanDiff.ts b/packages/ui/hooks/usePlanDiff.ts index 735df927..13e0c51f 100644 --- a/packages/ui/hooks/usePlanDiff.ts +++ b/packages/ui/hooks/usePlanDiff.ts @@ -66,16 +66,15 @@ export function usePlanDiff( const [isSelectingVersion, setIsSelectingVersion] = useState(false); const [fetchingVersion, setFetchingVersion] = useState(null); - // Sync diffBasePlan when initialPreviousPlan arrives after mount (API response) + // Sync diff base when previousPlan or versionInfo changes (initial load + resubmissions) useEffect(() => { - if (initialPreviousPlan && !diffBasePlan) { + if (initialPreviousPlan) { setDiffBasePlan(initialPreviousPlan); } }, [initialPreviousPlan]); - // Sync diffBaseVersion when versionInfo arrives after mount useEffect(() => { - if (versionInfo && versionInfo.version > 1 && diffBaseVersion === null) { + if (versionInfo && versionInfo.version > 1) { setDiffBaseVersion(versionInfo.version - 1); } }, [versionInfo]); From 893172f47c0add2352cb5f69d214ec189a941044 Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Mon, 25 May 2026 21:34:58 -0700 Subject: [PATCH 21/35] Exclude folder annotate from persistence, fix review loop survival MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 1. Folder-mode annotate sessions no longer participate in session matching or resubmission — they have no single document to track. Prevents empty content being pushed on resubmission. 2. Review decision loop uses promise identity tracking instead of idle() return value to detect cycle exhaustion. Survives double- submissions while still exiting cleanly for non-agent origins. --- packages/server/annotate.ts | 4 ++-- packages/server/daemon/session-factory.ts | 15 +++++++++------ 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/packages/server/annotate.ts b/packages/server/annotate.ts index 1d7ea906..702922d2 100644 --- a/packages/server/annotate.ts +++ b/packages/server/annotate.ts @@ -146,8 +146,8 @@ export async function createAnnotateSession( // Detect repo info (cached for this session) const repoInfo = await getRepoInfo(cwd); - // Version history (file-based modes only) - const isFileBased = mode === "annotate" || mode === "annotate-folder"; + // Version history (single-file annotate only — folders have no single document to track) + const isFileBased = mode === "annotate"; const project = isFileBased ? ((await detectProjectName(cwd)) ?? "_unknown") : ""; const slug = isFileBased ? generateSlug(markdown) : ""; let previousPlan: string | null = null; diff --git a/packages/server/daemon/session-factory.ts b/packages/server/daemon/session-factory.ts index 27760b6d..28d9447d 100644 --- a/packages/server/daemon/session-factory.ts +++ b/packages/server/daemon/session-factory.ts @@ -173,10 +173,13 @@ function registerReviewDecision( const { disposed, cleanup } = createDecisionScope(session.dispose); const decisionLoop = async () => { + let lastPromise: Promise | null = null; while (true) { - const result = await Promise.race([session.waitForDecision(), disposed]); - const record = context.store.idle(id, result); - if (!record || record.status !== "idle") return; + const currentPromise = session.waitForDecision(); + if (currentPromise === lastPromise) return; + lastPromise = currentPromise; + const result = await Promise.race([currentPromise, disposed]); + context.store.idle(id, result); } }; @@ -685,8 +688,8 @@ export function createDaemonSessionFactory(options: DaemonSessionFactoryOptions) if (request.action === "annotate" || request.action === "annotate-last") { const input = await resolveAnnotateInput(request, cwd, request.action); - const isFileBased = input.mode === "annotate" || input.mode === "annotate-folder"; - const matchKey = isFileBased ? `annotate:${input.filePath}` : undefined; + const isSingleFile = input.mode === "annotate"; + const matchKey = isSingleFile ? `annotate:${input.filePath}` : undefined; if (matchKey) { const existing = findMatchingSession(context.store, matchKey); @@ -723,7 +726,7 @@ export function createDaemonSessionFactory(options: DaemonSessionFactoryOptions) matchKey, ttlMs, handleRequest: session.handleRequest, - dispose: isFileBased + dispose: isSingleFile ? registerPersistentDecision(context, id, session) : registerSessionDecision(context, id, () => session.waitForDecision(), () => session.dispose(), (result) => ({ ...result, filePath: input.filePath, mode: input.mode, From 7204c54b7ec667ad6f753da5b69b6f96d9a4aac5 Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Tue, 26 May 2026 07:29:10 -0700 Subject: [PATCH 22/35] Scope annotate matchKey by project to prevent cross-project collisions The "document" filePath fallback created a global collision bucket for fileless annotate sessions. Now scoped as annotate:project:path. --- packages/server/daemon/session-factory.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/server/daemon/session-factory.ts b/packages/server/daemon/session-factory.ts index 28d9447d..43eff5a3 100644 --- a/packages/server/daemon/session-factory.ts +++ b/packages/server/daemon/session-factory.ts @@ -689,7 +689,7 @@ export function createDaemonSessionFactory(options: DaemonSessionFactoryOptions) if (request.action === "annotate" || request.action === "annotate-last") { const input = await resolveAnnotateInput(request, cwd, request.action); const isSingleFile = input.mode === "annotate"; - const matchKey = isSingleFile ? `annotate:${input.filePath}` : undefined; + const matchKey = isSingleFile ? `annotate:${project}:${input.filePath}` : undefined; if (matchKey) { const existing = findMatchingSession(context.store, matchKey); From 7b499f7bc3a8816c63e477e405b55ffd18e65ca6 Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Tue, 26 May 2026 09:00:49 -0700 Subject: [PATCH 23/35] Update decisions.md with cross-cutting facts and current open items Add cross-cutting requirements from review triage: external annotation flushing, waitForResult consistency, plan action disabling, snapshot provider, and architectural gaps (HTML pipeline, PR metadata). Replace outdated bug table with current open items reflecting what's fixed, what's deferred, and what's accepted. --- goals/session-persistence/decisions.md | 37 ++++++++++++++++++-------- 1 file changed, 26 insertions(+), 11 deletions(-) diff --git a/goals/session-persistence/decisions.md b/goals/session-persistence/decisions.md index 3ea13e32..5f1dcf4d 100644 --- a/goals/session-persistence/decisions.md +++ b/goals/session-persistence/decisions.md @@ -33,6 +33,17 @@ Tracking decisions made during PR #770 review and triage (2026-05-23). - But I would need to be notified of this. - I would need to be notified if diffs change. +### Cross-Cutting + +- When a revision arrives (plan, annotate, or review), any external annotations (lint results, agent comments) from the previous version must be cleared. They reference old content with wrong positions. +- `waitForResult` must return immediately if the result is already available — for both `idle` and `awaiting-resubmission` sessions. No consistency gaps. +- Plan/annotate actions (Approve, Deny, Send Feedback) must be disabled while awaiting resubmission. The agent already has the feedback — submitting again against stale content is wrong. Code review already handles this (buttons hidden when idle). +- Late WebSocket subscribers (tab refresh during awaiting) should receive the current state. The snapshot provider for `session-revision` must return the latest content, not null. +- HTML and markdown annotation should go through the same functional pipeline. The `--render-html` path diverges from markdown in a way that `updateContent` can't reach. This is an architectural gap, not a quick fix. +- PR review sessions that get reactivated need updated PR metadata (head SHA, etc.). The current implementation serves the correct diff but posts platform actions against the stale commit. Needs a bigger fix to make PR metadata updatable inside the session closure. +- Annotate history slug is computed once from the initial heading and doesn't update if the heading changes. Acceptable — versions stay intact, just filed under the old name on disk. +- Session collisions across worktrees of the same repo are not a real concern. This is a local app — one daemon per machine. + --- ## Decisions @@ -114,14 +125,18 @@ For code review (Decision 1), sessions live indefinitely, so expiry is a differe --- -## Bugs confirmed from external review (PR #770) - -| # | Finding | Severity | Action | -|---|---------|----------|--------| -| 1-2 | Frontend sessions spin in registerPersistentDecision | P1 | Fix (Decision 4) | -| 3+9 | --render-html resubmission shows stale content | P2 | Skip — narrow edge case | -| 4 | OpenCode folder annotation mislabeled after persistence | nit | Skip — cosmetic | -| 5 | Stale external annotations after review resubmission | P2 | Fix (Decision 5), possibly moot after Decision 1 | -| 6 | waitForResult doesn't short-circuit on awaiting | P2 | Skip — current behavior is correct | -| 7-8 | Actions not disabled during awaiting | P2 | Redesign per Decision 3 | -| 10 | onCancel never wired, stuck spinner | nit | Deferred (Decision 6) | +## Open Items + +| Item | Severity | Status | +|------|----------|--------| +| External annotations not cleared on revision (all surfaces) | P2 | Must fix — stale line numbers | +| Plan/annotate actions not disabled during awaiting | P2 | Must fix — stale content submission | +| `waitForResult` missing `awaiting-resubmission` short-circuit | P2 | Must fix — consistency with idle | +| `session-revision` snapshot provider returns null | P2 | Must fix — tab refresh during awaiting loses content | +| `registerPersistentDecision` hot loop for non-agent origins | P1 | Must fix — currently unreachable but latent | +| `--render-html` resubmission shows stale HTML | P2 | Deferred — architectural gap (HTML/markdown pipeline divergence) | +| PR reviews keep stale metadata on reuse | P1 | Deferred — needs PR metadata updatable in session closure | +| `onCancel` never wired on awaiting banner | nit | Deferred (Decision 6) | +| Session collisions across same-repo worktrees | nit | Not a concern — local app, one daemon per machine | +| Annotate slug doesn't update on heading change | nit | Accepted — cosmetic, versions work correctly | +| `sessionRefs` lazy cleanup | nit | Accepted — negligible memory | From 23dd2fab0c2471224c6045c460e6e8b0012dd2ac Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Tue, 26 May 2026 09:06:37 -0700 Subject: [PATCH 24/35] Fix 5 cross-cutting issues: external annotations, waitForResult, plan actions, snapshots, docs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 1. Clear external annotations on revision for all three servers (plan, annotate, review) — stale lint/agent comments no longer persist with wrong line numbers after content refresh. 2. waitForResult short-circuits for awaiting-resubmission sessions with results, matching the existing idle behavior. 3. Plan/annotate actions disabled during awaitingResubmission — keyboard shortcuts and header buttons gated. 4. session-revision snapshot providers return current content instead of null — late WebSocket subscribers get the latest state. 5. Comment on registerReviewDecision explaining intentional idle lifecycle (reviews stay alive, cleanup via TTL). --- packages/plannotator-plan-review/App.tsx | 6 +++--- packages/server/annotate.ts | 3 ++- packages/server/daemon/session-factory.ts | 3 +++ packages/server/daemon/session-store.ts | 2 +- packages/server/external-annotations.ts | 6 ++++++ packages/server/index.ts | 3 ++- packages/server/review.ts | 3 ++- 7 files changed, 19 insertions(+), 7 deletions(-) diff --git a/packages/plannotator-plan-review/App.tsx b/packages/plannotator-plan-review/App.tsx index 3ad80e14..6ee41482 100644 --- a/packages/plannotator-plan-review/App.tsx +++ b/packages/plannotator-plan-review/App.tsx @@ -1213,8 +1213,8 @@ const App: React.FC<{ __embedded?: boolean; headerLeft?: React.ReactNode; onOpen if (showExport || showImport || showFeedbackPrompt || showClaudeCodeWarning || showExitWarning || showAgentWarning || showPermissionModeSetup || pendingPasteImage) return; - // Don't intercept if already submitted, submitting, or exiting - if (submitted || isSubmitting || isExiting || goalSetupAction.isSubmitting) return; + // Don't intercept if already submitted, submitting, exiting, or awaiting resubmission + if (submitted || isSubmitting || isExiting || awaitingResubmission || goalSetupAction.isSubmitting) return; // Don't intercept in demo/share mode (no API) if (!isApiMode) return; @@ -1792,7 +1792,7 @@ const App: React.FC<{ __embedded?: boolean; headerLeft?: React.ReactNode; onOpen gate={gate} isSharedSession={isSharedSession} origin={origin} - submitted={!!submitted} + submitted={!!submitted || awaitingResubmission} isSubmitting={isSubmitting} isExiting={isExiting} isPanelOpen={isPanelOpen} diff --git a/packages/server/annotate.ts b/packages/server/annotate.ts index 702922d2..0d196012 100644 --- a/packages/server/annotate.ts +++ b/packages/server/annotate.ts @@ -141,7 +141,7 @@ export async function createAnnotateSession( registerSnapshotProvider: (provider) => options.sessionEvents?.registerSnapshotProvider("external-annotations", provider), }); - options.sessionEvents?.registerSnapshotProvider("session-revision", () => null); + options.sessionEvents?.registerSnapshotProvider("session-revision", () => ({ plan: markdown, previousPlan, versionInfo })); // Detect repo info (cached for this session) const repoInfo = await getRepoInfo(cwd); @@ -344,6 +344,7 @@ export async function createAnnotateSession( totalVersions: getVersionCount(project, slug), project, }; + externalAnnotations.clearAll(); deleteDraft(draftKey); draftKey = contentHash(newMarkdown); options.sessionEvents?.publishEvent("session-revision", { plan: newMarkdown, previousPlan, versionInfo }); diff --git a/packages/server/daemon/session-factory.ts b/packages/server/daemon/session-factory.ts index 43eff5a3..d7c94af2 100644 --- a/packages/server/daemon/session-factory.ts +++ b/packages/server/daemon/session-factory.ts @@ -165,6 +165,9 @@ function registerPersistentDecision( return cleanup; } +// Review sessions stay alive (idle) after every decision — including approve/exit. +// Cleanup is via TTL expiry, not terminal completion. This is intentional: the user +// keeps the session as a diff viewer, and the agent can reactivate it later. function registerReviewDecision( context: DaemonFetchContext, id: string, diff --git a/packages/server/daemon/session-store.ts b/packages/server/daemon/session-store.ts index 2ddb552c..01c26810 100644 --- a/packages/server/daemon/session-store.ts +++ b/packages/server/daemon/session-store.ts @@ -314,7 +314,7 @@ export class DaemonSessionStore { waitForResult(id: string): Promise> { const record = this.sessions.get(id) as DaemonSessionRecord | undefined; if (!record) return Promise.reject(new Error(`Session not found: ${id}`)); - if (TERMINAL_STATUSES.has(record.status) || (record.status === "idle" && record.result !== undefined)) return Promise.resolve(record); + if (TERMINAL_STATUSES.has(record.status) || ((record.status === "idle" || record.status === "awaiting-resubmission") && record.result !== undefined)) return Promise.resolve(record); return new Promise((resolve, reject) => { const waiters = this.waiters.get(id) ?? []; waiters.push({ resolve: resolve as (record: DaemonSessionRecord) => void, reject }); diff --git a/packages/server/external-annotations.ts b/packages/server/external-annotations.ts index b2c51170..8879d647 100644 --- a/packages/server/external-annotations.ts +++ b/packages/server/external-annotations.ts @@ -32,6 +32,8 @@ export interface ExternalAnnotationHandler { ) => Promise; /** Push annotations directly into the store (bypasses HTTP, reuses same validation). */ addAnnotations: (body: unknown) => { ids: string[] } | { error: string }; + /** Remove all annotations from the store. */ + clearAll: () => void; dispose: () => void; } @@ -222,6 +224,10 @@ export function createExternalAnnotationHandler( return null; }, + clearAll(): void { + store.clearAll(); + }, + dispose(): void { disposed = true; for (const cleanup of Array.from(streamCleanups)) cleanup(); diff --git a/packages/server/index.ts b/packages/server/index.ts index 7321c81b..c2741f77 100644 --- a/packages/server/index.ts +++ b/packages/server/index.ts @@ -178,7 +178,7 @@ export async function createPlannotatorSession( registerSnapshotProvider: (provider) => options.sessionEvents?.registerSnapshotProvider("external-annotations", provider), }) : null; - if (mode !== "archive") options.sessionEvents?.registerSnapshotProvider("session-revision", () => null); + if (mode !== "archive") options.sessionEvents?.registerSnapshotProvider("session-revision", () => ({ plan, previousPlan, versionInfo })); const slug = mode !== "archive" ? generateSlug(plan) : ""; // Lazy cache for in-session archive browsing (plan review sidebar tab) @@ -599,6 +599,7 @@ export async function createPlannotatorSession( totalVersions: getVersionCount(project, slug), project, }; + externalAnnotations?.clearAll(); deleteDraft(draftKey); draftKey = contentHash(newPlan); options.sessionEvents?.publishEvent("session-revision", { plan: newPlan, previousPlan, versionInfo }); diff --git a/packages/server/review.ts b/packages/server/review.ts index b8d3476c..b1138f1c 100644 --- a/packages/server/review.ts +++ b/packages/server/review.ts @@ -174,7 +174,7 @@ export async function createReviewSession( registerSnapshotProvider: (provider) => options.sessionEvents?.registerSnapshotProvider("external-annotations", provider), }); - options.sessionEvents?.registerSnapshotProvider("session-revision", () => null); + options.sessionEvents?.registerSnapshotProvider("session-revision", () => ({ rawPatch: currentPatch, gitRef: currentGitRef })); const tour = createTourSession(); @@ -1185,6 +1185,7 @@ export async function createReviewSession( } currentPatch = patch; currentGitRef = label; + externalAnnotations.clearAll(); deleteDraft(draftKey); draftKey = contentHash(patch); options.sessionEvents?.publishEvent("session-revision", { rawPatch: patch, gitRef: label }); From 0e9fd2e4d2c96ae15a51fd85ba5e30eaf2abc1f7 Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Tue, 26 May 2026 11:34:59 -0700 Subject: [PATCH 25/35] Collapse duplicate decision loops into shared registerDecisionLoop The persistent and review decision handlers shared ~80% of their structure. Extract a parameterized registerDecisionLoop that takes an onResult callback and activeStatuses set. Also drop an unnecessary cast and extract a named boolean in waitForResult for readability. --- packages/server/daemon/session-factory.ts | 69 +++++++++++------------ packages/server/daemon/session-store.ts | 3 +- 2 files changed, 36 insertions(+), 36 deletions(-) diff --git a/packages/server/daemon/session-factory.ts b/packages/server/daemon/session-factory.ts index d7c94af2..27bd7d94 100644 --- a/packages/server/daemon/session-factory.ts +++ b/packages/server/daemon/session-factory.ts @@ -137,27 +137,29 @@ function registerSessionDecision( return cleanup; } -function registerPersistentDecision( +function registerDecisionLoop( context: DaemonFetchContext, id: string, session: { waitForDecision: () => Promise; dispose: () => void | Promise }, + onResult: (result: SessionDecisionResult) => "continue" | "done", + activeStatuses: Set, ): () => void | Promise { const { disposed, cleanup } = createDecisionScope(session.dispose); - const decisionLoop = async () => { + const loop = async () => { + let lastPromise: Promise | null = null; while (true) { - const result = await Promise.race([session.waitForDecision(), disposed]); - if (result.approved || result.exit) { - context.store.complete(id, result); - return; - } - context.store.suspend(id, result); + const currentPromise = session.waitForDecision(); + if (currentPromise === lastPromise) return; + lastPromise = currentPromise; + const result = await Promise.race([currentPromise, disposed]); + if (onResult(result) === "done") return; } }; - void decisionLoop().catch((err) => { + void loop().catch((err) => { const record = context.store.get(id); - if (record && (record.status === "active" || record.status === "awaiting-resubmission")) { + if (record && activeStatuses.has(record.status)) { context.store.fail(id, err instanceof Error ? err.message : String(err)); } }); @@ -165,35 +167,32 @@ function registerPersistentDecision( return cleanup; } +const PERSISTENT_ACTIVE = new Set(["active", "awaiting-resubmission"]); +const REVIEW_ACTIVE = new Set(["active", "idle"]); + +function registerPersistentDecision( + context: DaemonFetchContext, + id: string, + session: { waitForDecision: () => Promise; dispose: () => void | Promise }, +) { + return registerDecisionLoop(context, id, session, (result) => { + if (result.approved || result.exit) { context.store.complete(id, result); return "done"; } + context.store.suspend(id, result); + return "continue"; + }, PERSISTENT_ACTIVE); +} + // Review sessions stay alive (idle) after every decision — including approve/exit. -// Cleanup is via TTL expiry, not terminal completion. This is intentional: the user -// keeps the session as a diff viewer, and the agent can reactivate it later. +// Cleanup is via TTL expiry, not terminal completion. function registerReviewDecision( context: DaemonFetchContext, id: string, session: { waitForDecision: () => Promise; dispose: () => void | Promise }, -): () => void | Promise { - const { disposed, cleanup } = createDecisionScope(session.dispose); - - const decisionLoop = async () => { - let lastPromise: Promise | null = null; - while (true) { - const currentPromise = session.waitForDecision(); - if (currentPromise === lastPromise) return; - lastPromise = currentPromise; - const result = await Promise.race([currentPromise, disposed]); - context.store.idle(id, result); - } - }; - - void decisionLoop().catch((err) => { - const record = context.store.get(id); - if (record && (record.status === "active" || record.status === "idle")) { - context.store.fail(id, err instanceof Error ? err.message : String(err)); - } - }); - - return cleanup; +) { + return registerDecisionLoop(context, id, session, (result) => { + context.store.idle(id, result); + return "continue"; + }, REVIEW_ACTIVE); } function resolvePlanFilePath(planFilePath: string, cwd: string): string { @@ -697,7 +696,7 @@ export function createDaemonSessionFactory(options: DaemonSessionFactoryOptions) if (matchKey) { const existing = findMatchingSession(context.store, matchKey); if (existing && existing.session.updateContent) { - (existing.session.updateContent as (m: string) => void)(input.markdown); + existing.session.updateContent(input.markdown); context.store.reactivate(existing.record.id); return existing.record; } diff --git a/packages/server/daemon/session-store.ts b/packages/server/daemon/session-store.ts index 01c26810..e40102d2 100644 --- a/packages/server/daemon/session-store.ts +++ b/packages/server/daemon/session-store.ts @@ -314,7 +314,8 @@ export class DaemonSessionStore { waitForResult(id: string): Promise> { const record = this.sessions.get(id) as DaemonSessionRecord | undefined; if (!record) return Promise.reject(new Error(`Session not found: ${id}`)); - if (TERMINAL_STATUSES.has(record.status) || ((record.status === "idle" || record.status === "awaiting-resubmission") && record.result !== undefined)) return Promise.resolve(record); + const hasIntermediateResult = (record.status === "idle" || record.status === "awaiting-resubmission") && record.result !== undefined; + if (TERMINAL_STATUSES.has(record.status) || hasIntermediateResult) return Promise.resolve(record); return new Promise((resolve, reject) => { const waiters = this.waiters.get(id) ?? []; waiters.push({ resolve: resolve as (record: DaemonSessionRecord) => void, reject }); From 61dbe331cc4c4e4d6bab2af6c0be083e7a5c3e42 Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Tue, 26 May 2026 12:28:39 -0700 Subject: [PATCH 26/35] Fix 3 review findings: protocol test, legacy overlay, smart viewed-files retention MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 1. Update daemon-protocol test to expect session-revision event family 2. Wire feedbackSent to CompletionOverlay so legacy tab mode shows the full-screen close experience after sending review feedback 3. Add retainUnchangedViewedFiles() — on diff revision, only un-hide files whose patch actually changed; unchanged files stay hidden 4. Document viewed-files and legacy-mode facts in decisions.md --- goals/session-persistence/decisions.md | 2 ++ packages/plannotator-code-review/App.tsx | 10 ++++++---- packages/plannotator-code-review/types.ts | 20 ++++++++++++++++++++ packages/shared/daemon-protocol.test.ts | 1 + packages/ui/components/CompletionOverlay.tsx | 2 +- 5 files changed, 30 insertions(+), 5 deletions(-) diff --git a/goals/session-persistence/decisions.md b/goals/session-persistence/decisions.md index 5f1dcf4d..41d6ec4e 100644 --- a/goals/session-persistence/decisions.md +++ b/goals/session-persistence/decisions.md @@ -32,6 +32,8 @@ Tracking decisions made during PR #770 review and triage (2026-05-23). - If an agent session initiates a new code review session from same directory, ideally it would open in the existing session. - But I would need to be notified of this. - I would need to be notified if diffs change. +- In legacy tab mode, code review should show the full-screen completion overlay (countdown + close tab) after sending feedback, same as plan review. The inline banner is for embedded mode only. +- When a new diff arrives, files I've already viewed should stay hidden — unless the file actually changed in the new diff. Only show it again if the content is different. ### Cross-Cutting diff --git a/packages/plannotator-code-review/App.tsx b/packages/plannotator-code-review/App.tsx index 16f95a3a..0e4f12aa 100644 --- a/packages/plannotator-code-review/App.tsx +++ b/packages/plannotator-code-review/App.tsx @@ -73,7 +73,7 @@ import { REVIEW_ALL_FILES_PANEL_ID, REVIEW_CODE_NAV_PANEL_ID, } from './dock/reviewPanelTypes'; -import type { DiffFile } from './types'; +import { retainUnchangedViewedFiles, type DiffFile } from './types'; import type { DiffOption, WorktreeInfo, GitContext } from '@plannotator/shared/types'; import type { PRMetadata } from '@plannotator/shared/pr-types'; import type { PRDiffScope, PRDiffScopeOption, PRStackInfo, PRStackTree } from '@plannotator/shared/pr-stack'; @@ -317,6 +317,7 @@ const ReviewApp: React.FC<{ __embedded?: boolean; headerLeft?: React.ReactNode; if (msg.type !== "event" || !msg.payload) return; const revision = msg.payload as { rawPatch?: string; gitRef?: string }; if (revision.rawPatch !== undefined) { + const oldFiles = storeApi.getState().files; const newFiles = parseDiffToFiles(revision.rawPatch); setDiffData(prev => prev ? { ...prev, rawPatch: revision.rawPatch!, gitRef: revision.gitRef ?? prev.gitRef } : prev); storeApi.getState().setFiles(newFiles); @@ -324,6 +325,7 @@ const ReviewApp: React.FC<{ __embedded?: boolean; headerLeft?: React.ReactNode; storeApi.getState().setLocalAnnotations([]); storeApi.getState().selectAnnotation(null); storeApi.getState().setPendingSelection(null); + setViewedFiles(prev => retainUnchangedViewedFiles(oldFiles, newFiles, prev)); setFeedbackSent(false); setSubmitted(false); setIsSendingFeedback(false); @@ -2502,9 +2504,9 @@ const ReviewApp: React.FC<{ __embedded?: boolean; headerLeft?: React.ReactNode; {/* Full-screen overlay: standalone mode, or legacy tab mode even when embedded */} {(!__embedded || legacyTabMode) && ( )} diff --git a/packages/plannotator-code-review/types.ts b/packages/plannotator-code-review/types.ts index 5e44f14f..3835aed0 100644 --- a/packages/plannotator-code-review/types.ts +++ b/packages/plannotator-code-review/types.ts @@ -5,3 +5,23 @@ export interface DiffFile { additions: number; deletions: number; } + +/** + * Keep files marked "viewed" only if their patch is identical in the new diff. + * Files that changed or were added get removed from the set so they reappear. + */ +export function retainUnchangedViewedFiles( + oldFiles: DiffFile[], + newFiles: DiffFile[], + viewedFiles: Set, +): Set { + if (viewedFiles.size === 0) return viewedFiles; + const oldPatches = new Map(oldFiles.map(f => [f.path, f.patch])); + const retained = new Set(); + for (const file of newFiles) { + if (viewedFiles.has(file.path) && oldPatches.get(file.path) === file.patch) { + retained.add(file.path); + } + } + return retained; +} diff --git a/packages/shared/daemon-protocol.test.ts b/packages/shared/daemon-protocol.test.ts index 2b5c59d5..18be196c 100644 --- a/packages/shared/daemon-protocol.test.ts +++ b/packages/shared/daemon-protocol.test.ts @@ -35,6 +35,7 @@ describe("daemon protocol", () => { "daemon", "external-annotations", "agent-jobs", + "session-revision", ]); expect(PLANNOTATOR_DAEMON_SESSION_VIEWS).toEqual([ "plan", diff --git a/packages/ui/components/CompletionOverlay.tsx b/packages/ui/components/CompletionOverlay.tsx index 21868096..309cff06 100644 --- a/packages/ui/components/CompletionOverlay.tsx +++ b/packages/ui/components/CompletionOverlay.tsx @@ -25,7 +25,7 @@ const ChatBubbleIcon = () => ( // --------------------------------------------------------------------------- interface CompletionOverlayProps { - submitted: 'approved' | 'denied' | 'feedback' | 'exited' | null | false; + submitted: 'approved' | 'denied' | 'feedback' | 'feedback-sent' | 'exited' | null | false; title: string; subtitle: string; agentLabel: string; From fe9cde1c98e6ee6b32671a445918c23cc5871da5 Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Tue, 26 May 2026 12:38:45 -0700 Subject: [PATCH 27/35] Fix CompletionOverlay feedback-sent visual to match CompletionBanner MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both components now show success colors and checkmark icon for feedback-sent state. Previously the overlay used accent/chat-bubble while the banner used success/check — inconsistent for the same action. --- packages/ui/components/CompletionOverlay.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/ui/components/CompletionOverlay.tsx b/packages/ui/components/CompletionOverlay.tsx index 309cff06..51708250 100644 --- a/packages/ui/components/CompletionOverlay.tsx +++ b/packages/ui/components/CompletionOverlay.tsx @@ -36,17 +36,17 @@ export function CompletionOverlay({ submitted, title, subtitle, agentLabel }: Co if (!submitted) return null; - const isApproved = submitted === 'approved'; + const isSuccess = submitted === 'approved' || submitted === 'feedback-sent'; return (
- {isApproved ? : } + {isSuccess ? : }
From 206104f42b1252550cb363a020c18bdac58a6957 Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Tue, 26 May 2026 14:19:50 -0700 Subject: [PATCH 28/35] Code quality: exit cycle, util location, docs, decision cycle comment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 1. Review /api/exit uses direct resolve instead of resolveAndCycle — no dangling cycle left for the decision loop to block on 2. Move retainUnchangedViewedFiles from types.ts to utils/diffFiles.ts 3. Document createDecisionCycle promise identity invariant 4. Update AGENTS.md: correct annotate match key format, add idle status lifecycle for code review, add version history endpoints to annotate server table 5. Add session lifetime and timeout facts to decisions.md --- AGENTS.md | 10 ++++++--- goals/session-persistence/decisions.md | 6 +++++- packages/plannotator-code-review/App.tsx | 3 ++- packages/plannotator-code-review/types.ts | 20 ------------------ .../utils/diffFiles.ts | 21 +++++++++++++++++++ packages/server/review.ts | 2 +- packages/server/session-handler.ts | 4 ++++ 7 files changed, 40 insertions(+), 26 deletions(-) create mode 100644 packages/plannotator-code-review/utils/diffFiles.ts diff --git a/AGENTS.md b/AGENTS.md index 1dafcdc9..2673d90b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -253,9 +253,11 @@ Runtime live updates for daemon lifecycle events, external annotations, agent jo ### Session Persistence and Resubmission -When a user denies a plan (or sends feedback on a review/annotation), the session enters `awaiting-resubmission` status instead of completing. The session's HTTP handler stays alive. When the agent replans and submits again via `POST /daemon/sessions`, the daemon matches the new submission to the existing session by a match key (`plan:project:slug` for plans, `review:project:branch` for reviews, `annotate:filepath` for annotations). The session reactivates in place — the frontend receives a `session-revision` event via WebSocket with the updated content. +When a user denies a plan (or sends feedback on a review/annotation), the session enters `awaiting-resubmission` status instead of completing. The session's HTTP handler stays alive. When the agent replans and submits again via `POST /daemon/sessions`, the daemon matches the new submission to the existing session by a match key (`plan:project:slug` for plans, `review:project:branch` for reviews, `annotate:project:filePath` for single-file annotations). The session reactivates in place — the frontend receives a `session-revision` event via WebSocket with the updated content. -**Session statuses:** `active` → `awaiting-resubmission` (on deny) → `active` (on resubmit) → `completed` (on approve). The `awaiting-resubmission` status is non-terminal — the session stays routable and its handler keeps serving requests. If the agent doesn't resubmit within 10 minutes, the session expires. +**Session statuses (plan/annotate):** `active` → `awaiting-resubmission` (on deny) → `active` (on resubmit) → `completed` (on approve). The `awaiting-resubmission` status is non-terminal — the session stays routable and its handler keeps serving requests. + +**Session statuses (code review):** `active` → `idle` (on feedback/approve/exit) → `active` (on agent resubmit) → `idle` ... repeating. Review sessions never complete — they stay alive as diff viewers and expire via TTL. The `idle` status is non-terminal — the session is fully interactive but no agent is blocking on it. **Event families:** `daemon`, `external-annotations`, `agent-jobs`, `session-revision`. @@ -328,7 +330,9 @@ When a user denies a plan (or sends feedback on a review/annotation), the sessio | Endpoint | Method | Purpose | | --------------------- | ------ | ------------------------------------------ | -| `/api/plan` | GET | Returns `{ plan, origin, mode: "annotate", filePath, sourceInfo?, gate, renderAs?, rawHtml? }` | +| `/api/plan` | GET | Returns `{ plan, origin, mode: "annotate", filePath, sourceInfo?, gate, renderAs?, rawHtml?, previousPlan, versionInfo }` | +| `/api/plan/version` | GET | Fetch specific version (`?v=N`) — single-file annotate only | +| `/api/plan/versions` | GET | List all versions — single-file annotate only | | `/api/feedback` | POST | Submit annotations (body: feedback, annotations) | | `/api/approve` | POST | Approve without feedback (review-gate UX, `--gate`) | | `/api/exit` | POST | Close session without feedback | diff --git a/goals/session-persistence/decisions.md b/goals/session-persistence/decisions.md index 41d6ec4e..42b68846 100644 --- a/goals/session-persistence/decisions.md +++ b/goals/session-persistence/decisions.md @@ -37,11 +37,15 @@ Tracking decisions made during PR #770 review and triage (2026-05-23). ### Cross-Cutting +- Every annotate session lives forever once created — single file, folder, last message, URL. No one-shot sessions. The tab stays open and interactive after feedback is sent. There are no exceptions. +- Legacy tab mode is the only case where the tab closes after feedback — that's the full-screen overlay with countdown, and it's the expected legacy behavior. +- Sessions do not time out. A session, once created, lives until the daemon restarts. We do not kill sessions on a countdown. If a user submits feedback and the agent never comes back, that's for the user to see and decide — not for us to silently clean up. +- We should collect the right data (timestamps, feedback-sent-at, last-agent-contact) so we can eventually show the user: "you submitted feedback but it never came back." But that's a future UI concern, not a reason to expire sessions. - When a revision arrives (plan, annotate, or review), any external annotations (lint results, agent comments) from the previous version must be cleared. They reference old content with wrong positions. - `waitForResult` must return immediately if the result is already available — for both `idle` and `awaiting-resubmission` sessions. No consistency gaps. - Plan/annotate actions (Approve, Deny, Send Feedback) must be disabled while awaiting resubmission. The agent already has the feedback — submitting again against stale content is wrong. Code review already handles this (buttons hidden when idle). - Late WebSocket subscribers (tab refresh during awaiting) should receive the current state. The snapshot provider for `session-revision` must return the latest content, not null. -- HTML and markdown annotation should go through the same functional pipeline. The `--render-html` path diverges from markdown in a way that `updateContent` can't reach. This is an architectural gap, not a quick fix. +- HTML and markdown annotation should go through the same functional pipeline. The `--render-html` path diverges from markdown in a way that `updateContent` can't reach — `updateContent` must also update `rawHtml` for HTML sessions. - PR review sessions that get reactivated need updated PR metadata (head SHA, etc.). The current implementation serves the correct diff but posts platform actions against the stale commit. Needs a bigger fix to make PR metadata updatable inside the session closure. - Annotate history slug is computed once from the initial heading and doesn't update if the heading changes. Acceptable — versions stay intact, just filed under the old name on disk. - Session collisions across worktrees of the same repo are not a real concern. This is a local app — one daemon per machine. diff --git a/packages/plannotator-code-review/App.tsx b/packages/plannotator-code-review/App.tsx index 0e4f12aa..38e370e8 100644 --- a/packages/plannotator-code-review/App.tsx +++ b/packages/plannotator-code-review/App.tsx @@ -73,7 +73,8 @@ import { REVIEW_ALL_FILES_PANEL_ID, REVIEW_CODE_NAV_PANEL_ID, } from './dock/reviewPanelTypes'; -import { retainUnchangedViewedFiles, type DiffFile } from './types'; +import type { DiffFile } from './types'; +import { retainUnchangedViewedFiles } from './utils/diffFiles'; import type { DiffOption, WorktreeInfo, GitContext } from '@plannotator/shared/types'; import type { PRMetadata } from '@plannotator/shared/pr-types'; import type { PRDiffScope, PRDiffScopeOption, PRStackInfo, PRStackTree } from '@plannotator/shared/pr-stack'; diff --git a/packages/plannotator-code-review/types.ts b/packages/plannotator-code-review/types.ts index 3835aed0..5e44f14f 100644 --- a/packages/plannotator-code-review/types.ts +++ b/packages/plannotator-code-review/types.ts @@ -5,23 +5,3 @@ export interface DiffFile { additions: number; deletions: number; } - -/** - * Keep files marked "viewed" only if their patch is identical in the new diff. - * Files that changed or were added get removed from the set so they reappear. - */ -export function retainUnchangedViewedFiles( - oldFiles: DiffFile[], - newFiles: DiffFile[], - viewedFiles: Set, -): Set { - if (viewedFiles.size === 0) return viewedFiles; - const oldPatches = new Map(oldFiles.map(f => [f.path, f.patch])); - const retained = new Set(); - for (const file of newFiles) { - if (viewedFiles.has(file.path) && oldPatches.get(file.path) === file.patch) { - retained.add(file.path); - } - } - return retained; -} diff --git a/packages/plannotator-code-review/utils/diffFiles.ts b/packages/plannotator-code-review/utils/diffFiles.ts new file mode 100644 index 00000000..039df59a --- /dev/null +++ b/packages/plannotator-code-review/utils/diffFiles.ts @@ -0,0 +1,21 @@ +import type { DiffFile } from '../types'; + +/** + * Keep files marked "viewed" only if their patch is identical in the new diff. + * Files that changed or were added get removed from the set so they reappear. + */ +export function retainUnchangedViewedFiles( + oldFiles: DiffFile[], + newFiles: DiffFile[], + viewedFiles: Set, +): Set { + if (viewedFiles.size === 0) return viewedFiles; + const oldPatches = new Map(oldFiles.map(f => [f.path, f.patch])); + const retained = new Set(); + for (const file of newFiles) { + if (viewedFiles.has(file.path) && oldPatches.get(file.path) === file.patch) { + retained.add(file.path); + } + } + return retained; +} diff --git a/packages/server/review.ts b/packages/server/review.ts index b1138f1c..9258d07a 100644 --- a/packages/server/review.ts +++ b/packages/server/review.ts @@ -1040,7 +1040,7 @@ export async function createReviewSession( // API: Exit review session without feedback if (url.pathname === "/api/exit" && req.method === "POST") { deleteDraft(draftKey); - resolveAndCycle(decisionCycle, { approved: false, feedback: "", annotations: [], exit: true }, origin); + decisionCycle.resolve({ approved: false, feedback: "", annotations: [], exit: true }); return Response.json({ ok: true }); } diff --git a/packages/server/session-handler.ts b/packages/server/session-handler.ts index 9a1acaf5..19e07d7a 100644 --- a/packages/server/session-handler.ts +++ b/packages/server/session-handler.ts @@ -31,6 +31,10 @@ export type SessionRequestHandler = ( /** * Manages a resolvable decision cycle for session servers. * Each deny/feedback starts a new cycle; approve/exit is final. + * + * Invariant: promise() returns a new Promise object after startNew(). + * The decision loop in session-factory uses reference identity to detect + * when no new cycle was started (same promise === loop should exit). */ export function createDecisionCycle() { let current: { promise: Promise; resolve: (result: T) => void }; From 1616421a773da42d54e6344152a723fe4fb0258e Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Tue, 26 May 2026 14:49:30 -0700 Subject: [PATCH 29/35] Sessions never die: remove TTL, persistent all annotate types, fix 8 divergences MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 1. Remove AWAITING_RESUBMISSION_TTL_MS — suspend(), idle(), reactivate() no longer set expiresAt. Non-terminal sessions live until daemon restart. 2. registerPersistentDecision never calls store.complete() — approve and exit suspend the session like deny does. The agent still gets the result via resolveWaiters. 3. All annotate types (folder, last, URL) use registerPersistentDecision instead of one-shot registerSessionDecision. 4. Annotate /api/feedback returns feedbackSent instead of awaitingResubmission for non-revisable sessions (folder, last). 5. Plan review frontend handles feedbackSent state with feedback-sent banner. 6. Annotate handleUpdateContent accepts optional rawHtml for --render-html revision support. 7. Review /api/feedback ties feedbackDelivered to resolveAndCycle return value — dashboard sessions no longer get stuck. 8. Update decisions.md and AGENTS.md with sessions-never-die principle. --- AGENTS.md | 6 ++++-- goals/session-persistence/decisions.md | 12 +++++------ packages/plannotator-plan-review/App.tsx | 25 +++++++++++++++-------- packages/server/annotate.ts | 19 +++++++++++++---- packages/server/daemon/session-factory.ts | 7 +------ packages/server/daemon/session-store.ts | 9 ++++---- packages/server/review.ts | 4 ++-- 7 files changed, 49 insertions(+), 33 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 2673d90b..a570a9ff 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -255,9 +255,11 @@ Runtime live updates for daemon lifecycle events, external annotations, agent jo When a user denies a plan (or sends feedback on a review/annotation), the session enters `awaiting-resubmission` status instead of completing. The session's HTTP handler stays alive. When the agent replans and submits again via `POST /daemon/sessions`, the daemon matches the new submission to the existing session by a match key (`plan:project:slug` for plans, `review:project:branch` for reviews, `annotate:project:filePath` for single-file annotations). The session reactivates in place — the frontend receives a `session-revision` event via WebSocket with the updated content. -**Session statuses (plan/annotate):** `active` → `awaiting-resubmission` (on deny) → `active` (on resubmit) → `completed` (on approve). The `awaiting-resubmission` status is non-terminal — the session stays routable and its handler keeps serving requests. +**Sessions never die.** No session type calls `store.complete()` from its decision handler. All sessions survive feedback, approve, and exit — the HTTP handler stays alive and the tab keeps working. `registerPersistentDecision` always calls `store.suspend()`. `registerReviewDecision` always calls `store.idle()`. Non-terminal sessions have no expiry timer. -**Session statuses (code review):** `active` → `idle` (on feedback/approve/exit) → `active` (on agent resubmit) → `idle` ... repeating. Review sessions never complete — they stay alive as diff viewers and expire via TTL. The `idle` status is non-terminal — the session is fully interactive but no agent is blocking on it. +**Session statuses (plan/annotate):** `active` → `awaiting-resubmission` (on any decision) → `active` (on resubmit) → `awaiting-resubmission` ... repeating. + +**Session statuses (code review):** `active` → `idle` (on any decision) → `active` (on agent resubmit) → `idle` ... repeating. **Event families:** `daemon`, `external-annotations`, `agent-jobs`, `session-revision`. diff --git a/goals/session-persistence/decisions.md b/goals/session-persistence/decisions.md index 42b68846..4640af4b 100644 --- a/goals/session-persistence/decisions.md +++ b/goals/session-persistence/decisions.md @@ -72,22 +72,22 @@ Key behaviors: - After feedback: session transitions to `idle` via `store.idle()`. The HTTP handler stays alive, resources stay alive. The user can browse the diff and make annotations, but Send Feedback / Approve buttons are hidden (no agent to receive them). - On reactivation: agent triggers `plannotator review` from the same directory/branch. The daemon finds the idle session by matchKey, pushes the new diff via `updateContent`, and calls `store.reactivate()`. The frontend receives a `session-revision` WebSocket event, updates the diff, and re-shows the submit buttons. - Infinite cycle: this repeats as many times as needed. No counter, no limit. -- Cleanup: idle sessions use the original session TTL (hours, not the 10-minute awaiting TTL). They expire via normal TTL cleanup or daemon restart. +- Cleanup: idle sessions have no expiry. They live until daemon restart. **Resolved questions:** - Notification when diffs change: agent-triggered via `session-revision` event. No file watcher (user can manually switch diff type to refresh). - Subsequent feedback without agent: not possible — submit buttons are hidden while idle. - Cleanup: normal TTL expiry. -### Decision 2: Annotate sessions keep persistence +### Decision 2: All annotate sessions are persistent -**Status:** Decided +**Status:** Implemented -Annotate mode is similar to plan mode. Agent revisions may change the document. The user should be notified and see new versions. Diff mode should show previous versions. The gating process (when used) iterates until approval, same as planning. +Every annotate session lives forever — single file, folder, URL, last message. No one-shot sessions. All annotate types use `registerPersistentDecision`, which never calls `store.complete()`. The session always suspends and the loop continues. -Persistence stays as-is for annotate. The awaiting-resubmission model fits because the document is an iterative artifact. +Single-file annotate is revisable: it has a matchKey, updateContent, and version history. The frontend shows "Waiting for agent to revise..." after feedback. -**Open question: folder annotation with multiple file changes.** Currently `updateContent` takes a single markdown string and the matchKey is `annotate:${filePath}` (the folder path). If the agent edits multiple files in the folder, we need a way to push per-file updates or trigger a re-fetch of the file list. Current design doesn't handle this. +Folder, annotate-last, and URL annotate are non-revisable: no matchKey, no updateContent. The frontend shows "Feedback sent" after feedback. The session stays interactive — the user can keep browsing and send more feedback. ### Decision 3: "Feedback sent" state should be calm, not loading diff --git a/packages/plannotator-plan-review/App.tsx b/packages/plannotator-plan-review/App.tsx index 6ee41482..173bf0c4 100644 --- a/packages/plannotator-plan-review/App.tsx +++ b/packages/plannotator-plan-review/App.tsx @@ -186,6 +186,7 @@ const App: React.FC<{ __embedded?: boolean; headerLeft?: React.ReactNode; onOpen const [isExiting, setIsExiting] = useState(false); const [submitted, setSubmitted] = useState<'approved' | 'denied' | 'exited' | null>(null); const [awaitingResubmission, setAwaitingResubmission] = useState(false); + const [feedbackSent, setFeedbackSent] = useState(false); const [pendingPasteImage, setPendingPasteImage] = useState<{ file: File; blobUrl: string; initialName: string } | null>(null); const [showPermissionModeSetup, setShowPermissionModeSetup] = useState(false); const [permissionMode, setPermissionMode] = useState('bypassPermissions'); @@ -595,8 +596,12 @@ const App: React.FC<{ __embedded?: boolean; headerLeft?: React.ReactNode; onOpen if (!isApiMode || !awaitingResubmission) return; const unsubscribe = subscribeToDaemonSessionFamily("session-revision", (msg) => { if (msg.type !== "event" || !msg.payload) return; - const revision = msg.payload as { plan?: string; previousPlan?: string | null; versionInfo?: { version: number; totalVersions: number; project: string } }; + const revision = msg.payload as { plan?: string; previousPlan?: string | null; versionInfo?: { version: number; totalVersions: number; project: string }; rawHtml?: string }; if (revision.plan !== undefined) { + if (revision.rawHtml !== undefined) { + setRawHtml(revision.rawHtml); + setRenderAs('html'); + } setMarkdown(revision.plan); if (revision.previousPlan !== undefined) setPreviousPlan(revision.previousPlan); if (revision.versionInfo) setVersionInfo(revision.versionInfo); @@ -607,6 +612,7 @@ const App: React.FC<{ __embedded?: boolean; headerLeft?: React.ReactNode; onOpen setSelectedCodeAnnotationId(null); linkedDocHook.clearCache(); setAwaitingResubmission(false); + setFeedbackSent(false); setSubmitted(null); setIsSubmitting(false); } @@ -1143,6 +1149,9 @@ const App: React.FC<{ __embedded?: boolean; headerLeft?: React.ReactNode; onOpen if (data.awaitingResubmission) { setAwaitingResubmission(true); setIsSubmitting(false); + } else if (data.feedbackSent) { + setFeedbackSent(true); + setIsSubmitting(false); } else { setSubmitted('denied'); } @@ -1214,7 +1223,7 @@ const App: React.FC<{ __embedded?: boolean; headerLeft?: React.ReactNode; onOpen showExitWarning || showAgentWarning || showPermissionModeSetup || pendingPasteImage) return; // Don't intercept if already submitted, submitting, exiting, or awaiting resubmission - if (submitted || isSubmitting || isExiting || awaitingResubmission || goalSetupAction.isSubmitting) return; + if (submitted || isSubmitting || isExiting || awaitingResubmission || feedbackSent || goalSetupAction.isSubmitting) return; // Don't intercept in demo/share mode (no API) if (!isApiMode) return; @@ -1843,9 +1852,9 @@ const App: React.FC<{ __embedded?: boolean; headerLeft?: React.ReactNode; onOpen {/* Embedded completion banner — inline, non-blocking (skipped in legacy tab mode) */} {__embedded && !legacyTabMode && ( )} @@ -2289,9 +2298,9 @@ const App: React.FC<{ __embedded?: boolean; headerLeft?: React.ReactNode; onOpen {/* Full-screen overlay: standalone mode, or legacy tab mode even when embedded */} {(!__embedded || legacyTabMode) && ( )} diff --git a/packages/server/annotate.ts b/packages/server/annotate.ts index 0d196012..c715fbda 100644 --- a/packages/server/annotate.ts +++ b/packages/server/annotate.ts @@ -121,10 +121,11 @@ export async function createAnnotateSession( shareBaseUrl, pasteApiUrl, gate = false, - rawHtml, + rawHtml: initialRawHtml, renderHtml = false, } = options; let markdown = initialMarkdown; + let rawHtml = initialRawHtml; // Side-channel pre-warm so /api/doc/exists POSTs land on warm cache. void warmFileListCache(cwd, "code"); @@ -141,7 +142,10 @@ export async function createAnnotateSession( registerSnapshotProvider: (provider) => options.sessionEvents?.registerSnapshotProvider("external-annotations", provider), }); - options.sessionEvents?.registerSnapshotProvider("session-revision", () => ({ plan: markdown, previousPlan, versionInfo })); + options.sessionEvents?.registerSnapshotProvider("session-revision", () => ({ + plan: markdown, previousPlan, versionInfo, + ...(rawHtml !== undefined && { rawHtml }), + })); // Detect repo info (cached for this session) const repoInfo = await getRepoInfo(cwd); @@ -314,6 +318,9 @@ export async function createAnnotateSession( annotations: body.annotations || [], }, origin); + if (resubmit.awaitingResubmission && !isFileBased) { + return Response.json({ ok: true, feedbackSent: true }); + } return Response.json({ ok: true, ...resubmit }); } catch (err) { const message = @@ -333,8 +340,9 @@ export async function createAnnotateSession( }); }; - function handleUpdateContent(newMarkdown: string) { + function handleUpdateContent(newMarkdown: string, newRawHtml?: string) { markdown = newMarkdown; + if (newRawHtml !== undefined) rawHtml = newRawHtml; const historyResult = saveToHistory(project, slug, newMarkdown); previousPlan = historyResult.version > 1 ? getPlanVersion(project, slug, historyResult.version - 1) @@ -347,7 +355,10 @@ export async function createAnnotateSession( externalAnnotations.clearAll(); deleteDraft(draftKey); draftKey = contentHash(newMarkdown); - options.sessionEvents?.publishEvent("session-revision", { plan: newMarkdown, previousPlan, versionInfo }); + options.sessionEvents?.publishEvent("session-revision", { + plan: newMarkdown, previousPlan, versionInfo, + ...(rawHtml !== undefined && { rawHtml }), + }); } return { diff --git a/packages/server/daemon/session-factory.ts b/packages/server/daemon/session-factory.ts index 27bd7d94..6ef5b814 100644 --- a/packages/server/daemon/session-factory.ts +++ b/packages/server/daemon/session-factory.ts @@ -176,7 +176,6 @@ function registerPersistentDecision( session: { waitForDecision: () => Promise; dispose: () => void | Promise }, ) { return registerDecisionLoop(context, id, session, (result) => { - if (result.approved || result.exit) { context.store.complete(id, result); return "done"; } context.store.suspend(id, result); return "continue"; }, PERSISTENT_ACTIVE); @@ -728,11 +727,7 @@ export function createDaemonSessionFactory(options: DaemonSessionFactoryOptions) matchKey, ttlMs, handleRequest: session.handleRequest, - dispose: isSingleFile - ? registerPersistentDecision(context, id, session) - : registerSessionDecision(context, id, () => session.waitForDecision(), () => session.dispose(), (result) => ({ - ...result, filePath: input.filePath, mode: input.mode, - })), + dispose: registerPersistentDecision(context, id, session), remoteShare, snapshot: session.getSnapshot ?? (() => ({ plan: input.markdown, filePath: input.filePath, mode: input.mode, sourceInfo: input.sourceInfo })), }); diff --git a/packages/server/daemon/session-store.ts b/packages/server/daemon/session-store.ts index e40102d2..b5ea0cdc 100644 --- a/packages/server/daemon/session-store.ts +++ b/packages/server/daemon/session-store.ts @@ -133,7 +133,6 @@ const TERMINAL_STATUSES = new Set([ "failed", ]); const TERMINAL_SESSION_TTL_MS = 60_000; -const AWAITING_RESUBMISSION_TTL_MS = 10 * 60_000; function iso(ms: number): string { return new Date(ms).toISOString(); @@ -259,14 +258,14 @@ export class DaemonSessionStore { return record; } - suspend(id: string, result: TResult, ttlMs = AWAITING_RESUBMISSION_TTL_MS): DaemonSessionRecord | undefined { + suspend(id: string, result: TResult): DaemonSessionRecord | undefined { const record = this.sessions.get(id) as DaemonSessionRecord | undefined; if (!record || record.status !== "active") return record; record.status = "awaiting-resubmission"; record.result = result; const now = this.now(); record.updatedAt = iso(now); - record.expiresAt = iso(now + ttlMs); + delete record.expiresAt; this.resolveWaiters(record); this.emit("session-updated", record); return record; @@ -279,7 +278,7 @@ export class DaemonSessionStore { if (result !== undefined) record.result = result as TResult; const now = this.now(); record.updatedAt = iso(now); - record.expiresAt = iso(now + (record.ttlMs ?? AWAITING_RESUBMISSION_TTL_MS)); + delete record.expiresAt; this.resolveWaiters(record); this.emit("session-updated", record); return record; @@ -292,7 +291,7 @@ export class DaemonSessionStore { record.result = undefined; const now = this.now(); record.updatedAt = iso(now); - record.expiresAt = iso(now + (record.ttlMs ?? AWAITING_RESUBMISSION_TTL_MS)); + delete record.expiresAt; this.emit("session-updated", record); return record; } diff --git a/packages/server/review.ts b/packages/server/review.ts index 9258d07a..42f4f900 100644 --- a/packages/server/review.ts +++ b/packages/server/review.ts @@ -1057,9 +1057,9 @@ export async function createReviewSession( deleteDraft(draftKey); const isApproved = body.approved ?? false; const result = { approved: isApproved, feedback: body.feedback || "", annotations: body.annotations || [], agentSwitch: body.agentSwitch }; - resolveAndCycle(decisionCycle, result, origin); + const resubmit = resolveAndCycle(decisionCycle, result, origin); - return Response.json({ ok: true, feedbackDelivered: !isApproved || undefined }); + return Response.json({ ok: true, feedbackDelivered: resubmit.awaitingResubmission || undefined }); } catch (err) { const message = err instanceof Error ? err.message : "Failed to process feedback"; From 0b5ab632b15841b0fe0dd93e5ef67c01351ad9b5 Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Tue, 26 May 2026 14:54:37 -0700 Subject: [PATCH 30/35] Self-review: pass rawHtml through annotate reuse path, fix interface and comment The factory's annotate reuse path called updateContent(input.markdown) without forwarding input.rawHtml. Updated to pass both. Also updated the AnnotateSession interface to match the new signature and fixed a stale comment about TTL expiry. --- packages/server/annotate.ts | 2 +- packages/server/daemon/session-factory.ts | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/server/annotate.ts b/packages/server/annotate.ts index c715fbda..bffc5a94 100644 --- a/packages/server/annotate.ts +++ b/packages/server/annotate.ts @@ -95,7 +95,7 @@ export interface AnnotateSession { handleRequest: SessionRequestHandler; waitForDecision: AnnotateServerResult["waitForDecision"]; dispose: () => void; - updateContent?: (newMarkdown: string) => void; + updateContent?: (newMarkdown: string, newRawHtml?: string) => void; getSnapshot?: () => unknown; } diff --git a/packages/server/daemon/session-factory.ts b/packages/server/daemon/session-factory.ts index 6ef5b814..309f1571 100644 --- a/packages/server/daemon/session-factory.ts +++ b/packages/server/daemon/session-factory.ts @@ -182,7 +182,7 @@ function registerPersistentDecision( } // Review sessions stay alive (idle) after every decision — including approve/exit. -// Cleanup is via TTL expiry, not terminal completion. +// Sessions persist until daemon restart. function registerReviewDecision( context: DaemonFetchContext, id: string, @@ -695,7 +695,7 @@ export function createDaemonSessionFactory(options: DaemonSessionFactoryOptions) if (matchKey) { const existing = findMatchingSession(context.store, matchKey); if (existing && existing.session.updateContent) { - existing.session.updateContent(input.markdown); + existing.session.updateContent(input.markdown, input.rawHtml); context.store.reactivate(existing.record.id); return existing.record; } From fae1d206dd21925351c718b3b2d178cef30c5beb Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Tue, 26 May 2026 15:10:30 -0700 Subject: [PATCH 31/35] Clean up decisions.md: mark fixed items, update stale decisions, add backlog - Mark 5 open items as Fixed (external annotations, actions disabled, waitForResult, snapshot providers, render-html) - Update Decisions 4/5/6 to reflect current implementation - Fix Decision 1 cleanup line (sessions persist, no TTL) - Add gate flag resubmission gap to backlog - Add provenance data collection to backlog --- goals/session-persistence/decisions.md | 45 +++++++++++--------------- 1 file changed, 19 insertions(+), 26 deletions(-) diff --git a/goals/session-persistence/decisions.md b/goals/session-persistence/decisions.md index 4640af4b..fd4148e5 100644 --- a/goals/session-persistence/decisions.md +++ b/goals/session-persistence/decisions.md @@ -77,7 +77,7 @@ Key behaviors: **Resolved questions:** - Notification when diffs change: agent-triggered via `session-revision` event. No file watcher (user can manually switch diff type to refresh). - Subsequent feedback without agent: not possible — submit buttons are hidden while idle. -- Cleanup: normal TTL expiry. +- Cleanup: sessions persist until daemon restart (no TTL on non-terminal sessions). ### Decision 2: All annotate sessions are persistent @@ -103,31 +103,23 @@ Folder, annotate-last, and URL annotate are non-revisable: no matchKey, no updat - For plan/annotate: actions should be disabled until the revision arrives (the agent already has the feedback and is working — re-submitting before the revision arrives doesn't make sense) - For code review: different model, TBD based on Decision 1 -### Decision 4: Fix the hot loop for frontend-originated sessions +### Decision 4: Hot loop prevention for non-agent origins -**Status:** Decided — will fix +**Status:** Resolved -When a session is created from the Plannotator dashboard (origin: `"plannotator-frontend"`), `resolveAndCycle` resolves the promise but doesn't start a new cycle. The `registerPersistentDecision` loop then spins on the already-resolved promise. Each iteration is a no-op (suspend guard catches it), but it burns CPU. +The `registerDecisionLoop` spin guard uses promise identity (`currentPromise === lastPromise`) to detect when no new cycle was started. When a non-agent origin calls `resolveAndCycle`, it resolves without calling `startNew()`, so the loop sees the same promise and exits cleanly. No hot loop. -**Fix:** In `registerPersistentDecision`, if `suspend()` returns a record with status !== `"awaiting-resubmission"`, break the loop and complete. +### Decision 5: Clear external annotations on revision -### Decision 5: Clear external annotations on review resubmission - -**Status:** Decided — will fix (if any refresh mechanism is kept for reviews) - -`handleUpdateContent` in review.ts swaps the patch but keeps stale external annotations (lint results, agent comments) from the previous diff. Line numbers and file paths may no longer match. - -**Fix:** Call `externalAnnotations.clear()` inside `handleUpdateContent`. - -*May become moot depending on how Decision 1's refresh mechanism works.* +**Status:** Implemented -### Decision 6: Cancel / session expiry handling +All three `handleUpdateContent` functions (plan, annotate, review) call `externalAnnotations.clearAll()` before publishing the `session-revision` event. -**Status:** Deferred — depends on Decision 3 +### Decision 6: Session expiry -If the feedback-sent state is calm (Decision 3), the user isn't trapped. But we still need to handle session expiry gracefully for plan/annotate — when the 10-minute TTL fires server-side, the frontend should know. +**Status:** Resolved — sessions don't expire -For code review (Decision 1), sessions live indefinitely, so expiry is a different question (idle cleanup, not agent-didn't-resubmit). +Non-terminal sessions (`awaiting-resubmission`, `idle`, `active` after first decision) have `expiresAt` deleted. `cleanupExpired()` skips them. Sessions live until daemon restart or explicit cancellation. --- @@ -135,14 +127,15 @@ For code review (Decision 1), sessions live indefinitely, so expiry is a differe | Item | Severity | Status | |------|----------|--------| -| External annotations not cleared on revision (all surfaces) | P2 | Must fix — stale line numbers | -| Plan/annotate actions not disabled during awaiting | P2 | Must fix — stale content submission | -| `waitForResult` missing `awaiting-resubmission` short-circuit | P2 | Must fix — consistency with idle | -| `session-revision` snapshot provider returns null | P2 | Must fix — tab refresh during awaiting loses content | -| `registerPersistentDecision` hot loop for non-agent origins | P1 | Must fix — currently unreachable but latent | -| `--render-html` resubmission shows stale HTML | P2 | Deferred — architectural gap (HTML/markdown pipeline divergence) | +| External annotations not cleared on revision (all surfaces) | P2 | Fixed | +| Plan/annotate actions not disabled during awaiting | P2 | Fixed | +| `waitForResult` missing `awaiting-resubmission` short-circuit | P2 | Fixed | +| `session-revision` snapshot provider returns null | P2 | Fixed | +| `--render-html` resubmission shows stale HTML | P2 | Fixed — `handleUpdateContent` now accepts and updates `rawHtml` | | PR reviews keep stale metadata on reuse | P1 | Deferred — needs PR metadata updatable in session closure | -| `onCancel` never wired on awaiting banner | nit | Deferred (Decision 6) | -| Session collisions across same-repo worktrees | nit | Not a concern — local app, one daemon per machine | +| Gate flag not updated on resubmission | P2 | Deferred — if session was created ungated and agent resubmits with `--gate`, Approve button won't appear (user still sees Send Annotations + Close). Reverse also true: gated session stays gated even if agent resubmits without `--gate`. Fix: `updateContent` should accept and update the `gate` flag. | +| Provenance data for stale sessions | P3 | Deferred — collect timestamps (feedback-sent-at, last-agent-contact) so we can show "you submitted feedback but it never came back." Future UI concern. | +| `onCancel` never wired on awaiting banner | nit | Deferred | +| Session collisions across same-repo worktrees | nit | Accepted — local app, one daemon per machine | | Annotate slug doesn't update on heading change | nit | Accepted — cosmetic, versions work correctly | | `sessionRefs` lazy cleanup | nit | Accepted — negligible memory | From 8f4afcc42b1a7591068d616ab014f2883ed63025 Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Tue, 26 May 2026 15:55:24 -0700 Subject: [PATCH 32/35] Fix zombie listener, folder reuse, and button state on refresh - Approve/exit paths now use resolveAndCycle() so the decision loop stays alive for future resubmissions (fixes agent hang after approve) - Folder annotate sessions get a match key so re-running the same folder reuses the existing session instead of creating duplicates - All three servers track lastDecision and include it in the initial API response so frontends restore correct button state on refresh - Session-revision listeners accept snapshots and guard annotation clearing behind a content-change check to prevent data loss on WebSocket reconnect - Annotate updateContent is always provided (not just for file-based) so folder sessions can publish reactivation events --- goals/frontend-session-lifecycle/backlog.md | 2 +- goals/session-persistence/decisions.md | 3 ++ packages/plannotator-code-review/App.tsx | 26 +++++++--- packages/plannotator-plan-review/App.tsx | 50 +++++++++++++------ packages/server/annotate.ts | 34 ++++++++----- .../server/daemon/session-factory.test.ts | 2 +- packages/server/daemon/session-factory.ts | 13 +++-- packages/server/index.ts | 8 ++- packages/server/review.ts | 7 ++- 9 files changed, 102 insertions(+), 43 deletions(-) diff --git a/goals/frontend-session-lifecycle/backlog.md b/goals/frontend-session-lifecycle/backlog.md index c6f16eff..b16442f9 100644 --- a/goals/frontend-session-lifecycle/backlog.md +++ b/goals/frontend-session-lifecycle/backlog.md @@ -34,7 +34,7 @@ Implemented in `feat/session-persistence`. Sessions enter `awaiting-resubmission ## ~~4. Session persistence after completion~~ DONE -Implemented in `feat/session-persistence`. Denied sessions stay alive (handler not disposed) in `awaiting-resubmission` state with a 10-minute TTL. The session remains routable and serves API requests while waiting. +Implemented in `feat/session-persistence`. Denied sessions stay alive (handler not disposed) in `awaiting-resubmission` state with no expiry. Sessions persist until daemon restart. **Required behavior:** - Completed sessions stay in the sidebar with a status badge (approved/denied) diff --git a/goals/session-persistence/decisions.md b/goals/session-persistence/decisions.md index fd4148e5..6b46e4fc 100644 --- a/goals/session-persistence/decisions.md +++ b/goals/session-persistence/decisions.md @@ -38,6 +38,8 @@ Tracking decisions made during PR #770 review and triage (2026-05-23). ### Cross-Cutting - Every annotate session lives forever once created — single file, folder, last message, URL. No one-shot sessions. The tab stays open and interactive after feedback is sent. There are no exceptions. +- Folder annotate sessions are reusable. If the user annotates the same folder twice, the daemon should find the existing session and reactivate it — not create a new one. The match key is the folder path. There's no content to update (it's a file browser), but the session is reused as-is. +- Annotate-last is not reusable — "last message" has no stable identity across invocations. Each annotate-last creates a new session. This is fine; the command is rarely run twice in a row. - Legacy tab mode is the only case where the tab closes after feedback — that's the full-screen overlay with countdown, and it's the expected legacy behavior. - Sessions do not time out. A session, once created, lives until the daemon restarts. We do not kill sessions on a countdown. If a user submits feedback and the agent never comes back, that's for the user to see and decide — not for us to silently clean up. - We should collect the right data (timestamps, feedback-sent-at, last-agent-contact) so we can eventually show the user: "you submitted feedback but it never came back." But that's a future UI concern, not a reason to expire sessions. @@ -48,6 +50,7 @@ Tracking decisions made during PR #770 review and triage (2026-05-23). - HTML and markdown annotation should go through the same functional pipeline. The `--render-html` path diverges from markdown in a way that `updateContent` can't reach — `updateContent` must also update `rawHtml` for HTML sessions. - PR review sessions that get reactivated need updated PR metadata (head SHA, etc.). The current implementation serves the correct diff but posts platform actions against the stale commit. Needs a bigger fix to make PR metadata updatable inside the session closure. - Annotate history slug is computed once from the initial heading and doesn't update if the heading changes. Acceptable — versions stay intact, just filed under the old name on disk. +- The decision listener must stay alive after every user action — approve, deny, exit, send feedback. If the listener shuts down after approve/exit, the session looks alive but can't respond to future resubmissions. The agent hangs forever. - Session collisions across worktrees of the same repo are not a real concern. This is a local app — one daemon per machine. --- diff --git a/packages/plannotator-code-review/App.tsx b/packages/plannotator-code-review/App.tsx index 38e370e8..987b8a10 100644 --- a/packages/plannotator-code-review/App.tsx +++ b/packages/plannotator-code-review/App.tsx @@ -315,18 +315,22 @@ const ReviewApp: React.FC<{ __embedded?: boolean; headerLeft?: React.ReactNode; useEffect(() => { if (!origin) return; const unsubscribe = subscribeToDaemonSessionFamily("session-revision", (msg) => { - if (msg.type !== "event" || !msg.payload) return; + if (!msg.payload) return; const revision = msg.payload as { rawPatch?: string; gitRef?: string }; if (revision.rawPatch !== undefined) { const oldFiles = storeApi.getState().files; const newFiles = parseDiffToFiles(revision.rawPatch); - setDiffData(prev => prev ? { ...prev, rawPatch: revision.rawPatch!, gitRef: revision.gitRef ?? prev.gitRef } : prev); - storeApi.getState().setFiles(newFiles); - storeApi.getState().setFocusedFile(0); - storeApi.getState().setLocalAnnotations([]); - storeApi.getState().selectAnnotation(null); - storeApi.getState().setPendingSelection(null); - setViewedFiles(prev => retainUnchangedViewedFiles(oldFiles, newFiles, prev)); + const contentChanged = newFiles.length !== oldFiles.length || + newFiles.some((f, i) => f.patch !== oldFiles[i]?.patch); + if (contentChanged) { + setDiffData(prev => prev ? { ...prev, rawPatch: revision.rawPatch!, gitRef: revision.gitRef ?? prev.gitRef } : prev); + storeApi.getState().setFiles(newFiles); + storeApi.getState().setFocusedFile(0); + storeApi.getState().setLocalAnnotations([]); + storeApi.getState().selectAnnotation(null); + storeApi.getState().setPendingSelection(null); + setViewedFiles(prev => retainUnchangedViewedFiles(oldFiles, newFiles, prev)); + } setFeedbackSent(false); setSubmitted(false); setIsSendingFeedback(false); @@ -830,6 +834,7 @@ const ReviewApp: React.FC<{ __embedded?: boolean; headerLeft?: React.ReactNode; error?: string; isWSL?: boolean; serverConfig?: { displayName?: string; gitUser?: string }; + lastDecision?: 'approved' | 'feedback' | 'exited' | null; }) => { configStore.init(data.serverConfig); setGitUser(data.serverConfig?.gitUser); @@ -877,6 +882,11 @@ const ReviewApp: React.FC<{ __embedded?: boolean; headerLeft?: React.ReactNode; if (data.diffType && !data.prMetadata && data.gitContext?.vcsType !== 'p4' && data.gitContext?.vcsType !== 'jj' && needsDiffTypeSetup()) { setDiffTypeSetupPending(true); } + if (data.lastDecision) { + if (data.lastDecision === 'approved') setSubmitted('approved'); + else if (data.lastDecision === 'feedback') setFeedbackSent(true); + else if (data.lastDecision === 'exited') setSubmitted('exited'); + } }) .catch(() => { // Not in API mode - use demo content diff --git a/packages/plannotator-plan-review/App.tsx b/packages/plannotator-plan-review/App.tsx index 173bf0c4..30dcd82d 100644 --- a/packages/plannotator-plan-review/App.tsx +++ b/packages/plannotator-plan-review/App.tsx @@ -124,6 +124,8 @@ const App: React.FC<{ __embedded?: boolean; headerLeft?: React.ReactNode; onOpen return getComputedStyle(rootRef.current).visibility !== 'hidden'; }, []); const [markdown, setMarkdown] = useState(DEMO_PLAN_CONTENT); + const markdownRef = useRef(markdown); + markdownRef.current = markdown; const [annotations, setAnnotations] = useState([]); const [codeAnnotations, setCodeAnnotations] = useState([]); const [selectedAnnotationId, setSelectedAnnotationId] = useState(null); @@ -591,26 +593,29 @@ const App: React.FC<{ __embedded?: boolean; headerLeft?: React.ReactNode; onOpen const { editorAnnotations, deleteEditorAnnotation } = useEditorAnnotations(); const { externalAnnotations, updateExternalAnnotation, deleteExternalAnnotation } = useExternalAnnotations({ enabled: isApiMode && !goalSetupMode }); - // Listen for session-revision events (plan resubmission after deny) + // Listen for session-revision events (plan/annotate resubmission or reactivation) useEffect(() => { - if (!isApiMode || !awaitingResubmission) return; + if (!isApiMode) return; const unsubscribe = subscribeToDaemonSessionFamily("session-revision", (msg) => { - if (msg.type !== "event" || !msg.payload) return; + if (!msg.payload) return; const revision = msg.payload as { plan?: string; previousPlan?: string | null; versionInfo?: { version: number; totalVersions: number; project: string }; rawHtml?: string }; if (revision.plan !== undefined) { + const contentChanged = revision.plan !== markdownRef.current; if (revision.rawHtml !== undefined) { setRawHtml(revision.rawHtml); setRenderAs('html'); } - setMarkdown(revision.plan); - if (revision.previousPlan !== undefined) setPreviousPlan(revision.previousPlan); - if (revision.versionInfo) setVersionInfo(revision.versionInfo); - setAnnotations([]); - setCodeAnnotations([]); - setGlobalAttachments([]); - setSelectedAnnotationId(null); - setSelectedCodeAnnotationId(null); - linkedDocHook.clearCache(); + if (contentChanged) { + setMarkdown(revision.plan); + if (revision.previousPlan !== undefined) setPreviousPlan(revision.previousPlan); + if (revision.versionInfo) setVersionInfo(revision.versionInfo); + setAnnotations([]); + setCodeAnnotations([]); + setGlobalAttachments([]); + setSelectedAnnotationId(null); + setSelectedCodeAnnotationId(null); + linkedDocHook.clearCache(); + } setAwaitingResubmission(false); setFeedbackSent(false); setSubmitted(null); @@ -618,7 +623,7 @@ const App: React.FC<{ __embedded?: boolean; headerLeft?: React.ReactNode; onOpen } }); return unsubscribe; - }, [isApiMode, awaitingResubmission]); + }, [isApiMode]); // Drive DOM highlights for SSE-delivered external annotations. Disabled // while a linked doc overlay is open (Viewer DOM is hidden) and while the @@ -796,7 +801,7 @@ const App: React.FC<{ __embedded?: boolean; headerLeft?: React.ReactNode; onOpen if (!res.ok) throw new Error('Not in API mode'); return res.json(); }) - .then((data: { plan: string; origin?: Origin; mode?: 'annotate' | 'annotate-last' | 'annotate-folder' | 'archive' | 'goal-setup'; goalSetup?: GoalSetupBundle; filePath?: string; sourceInfo?: string; sourceConverted?: boolean; gate?: boolean; renderAs?: 'html' | 'markdown'; rawHtml?: string; sharingEnabled?: boolean; shareBaseUrl?: string; pasteApiUrl?: string; repoInfo?: { display: string; branch?: string; host?: string }; previousPlan?: string | null; versionInfo?: { version: number; totalVersions: number; project: string }; archivePlans?: ArchivedPlan[]; projectRoot?: string; isWSL?: boolean; serverConfig?: { displayName?: string; gitUser?: string } }) => { + .then((data: { plan: string; origin?: Origin; mode?: 'annotate' | 'annotate-last' | 'annotate-folder' | 'archive' | 'goal-setup'; goalSetup?: GoalSetupBundle; filePath?: string; sourceInfo?: string; sourceConverted?: boolean; gate?: boolean; renderAs?: 'html' | 'markdown'; rawHtml?: string; sharingEnabled?: boolean; shareBaseUrl?: string; pasteApiUrl?: string; repoInfo?: { display: string; branch?: string; host?: string }; previousPlan?: string | null; versionInfo?: { version: number; totalVersions: number; project: string }; archivePlans?: ArchivedPlan[]; projectRoot?: string; isWSL?: boolean; serverConfig?: { displayName?: string; gitUser?: string }; lastDecision?: 'approved' | 'denied' | 'exited' | 'feedback' | null }) => { // Initialize config store with server-provided values (config file > cookie > default) configStore.init(data.serverConfig); setGitUser(data.serverConfig?.gitUser); @@ -875,6 +880,23 @@ const App: React.FC<{ __embedded?: boolean; headerLeft?: React.ReactNode; onOpen if (data.isWSL) { setIsWSL(true); } + if (data.lastDecision) { + const isAnnotate = data.mode === 'annotate' || data.mode === 'annotate-last' || data.mode === 'annotate-folder'; + if (data.lastDecision === 'approved') { + setSubmitted('approved'); + } else if (data.lastDecision === 'denied') { + setAwaitingResubmission(true); + } else if (data.lastDecision === 'exited') { + setSubmitted('exited'); + } else if (data.lastDecision === 'feedback') { + const isFileBased = data.mode === 'annotate'; + if (isAnnotate && !isFileBased) { + setFeedbackSent(true); + } else { + setAwaitingResubmission(true); + } + } + } }) .catch(() => { // Not in API mode - use default content diff --git a/packages/server/annotate.ts b/packages/server/annotate.ts index bffc5a94..c4178233 100644 --- a/packages/server/annotate.ts +++ b/packages/server/annotate.ts @@ -95,7 +95,7 @@ export interface AnnotateSession { handleRequest: SessionRequestHandler; waitForDecision: AnnotateServerResult["waitForDecision"]; dispose: () => void; - updateContent?: (newMarkdown: string, newRawHtml?: string) => void; + updateContent: (newMarkdown: string, newRawHtml?: string) => void; getSnapshot?: () => unknown; } @@ -171,6 +171,7 @@ export async function createAnnotateSession( type AnnotateDecisionResult = { feedback: string; annotations: unknown[]; exit?: boolean; approved?: boolean }; const decisionCycle = createDecisionCycle(); + let lastDecision: 'approved' | 'exited' | 'feedback' | null = null; const handleRequest: SessionRequestHandler = async (req, url, context) => { @@ -195,6 +196,7 @@ export async function createAnnotateSession( projectRoot: folderPath || cwd, isWSL: wslFlag, serverConfig: getServerConfig(gitUser), + lastDecision, }); } @@ -293,14 +295,16 @@ export async function createAnnotateSession( // API: Exit annotation session without feedback if (url.pathname === "/api/exit" && req.method === "POST") { deleteDraft(draftKey); - decisionCycle.resolve({ feedback: "", annotations: [], exit: true }); + lastDecision = 'exited'; + resolveAndCycle(decisionCycle, { feedback: "", annotations: [], exit: true }, origin); return Response.json({ ok: true }); } // API: Approve the annotation session (review-gate UX, #570) if (url.pathname === "/api/approve" && req.method === "POST") { deleteDraft(draftKey); - decisionCycle.resolve({ feedback: "", annotations: [], approved: true }); + lastDecision = 'approved'; + resolveAndCycle(decisionCycle, { feedback: "", annotations: [], approved: true }, origin); return Response.json({ ok: true }); } @@ -313,6 +317,7 @@ export async function createAnnotateSession( }; deleteDraft(draftKey); + lastDecision = 'feedback'; const resubmit = resolveAndCycle(decisionCycle, { feedback: body.feedback || "", annotations: body.annotations || [], @@ -342,16 +347,19 @@ export async function createAnnotateSession( function handleUpdateContent(newMarkdown: string, newRawHtml?: string) { markdown = newMarkdown; + lastDecision = null; if (newRawHtml !== undefined) rawHtml = newRawHtml; - const historyResult = saveToHistory(project, slug, newMarkdown); - previousPlan = historyResult.version > 1 - ? getPlanVersion(project, slug, historyResult.version - 1) - : null; - versionInfo = { - version: historyResult.version, - totalVersions: getVersionCount(project, slug), - project, - }; + if (isFileBased && newMarkdown.trim()) { + const historyResult = saveToHistory(project, slug, newMarkdown); + previousPlan = historyResult.version > 1 + ? getPlanVersion(project, slug, historyResult.version - 1) + : null; + versionInfo = { + version: historyResult.version, + totalVersions: getVersionCount(project, slug), + project, + }; + } externalAnnotations.clearAll(); deleteDraft(draftKey); draftKey = contentHash(newMarkdown); @@ -369,7 +377,7 @@ export async function createAnnotateSession( externalAnnotations.dispose(); }, getSnapshot: isFileBased ? () => ({ plan: markdown, filePath, mode, sourceInfo }) : undefined, - updateContent: isFileBased ? handleUpdateContent : undefined, + updateContent: handleUpdateContent, }; } diff --git a/packages/server/daemon/session-factory.test.ts b/packages/server/daemon/session-factory.test.ts index 1a0f02ec..723d59e8 100644 --- a/packages/server/daemon/session-factory.test.ts +++ b/packages/server/daemon/session-factory.test.ts @@ -100,7 +100,7 @@ describe("createDaemonSessionFactory", () => { ); const completed = await store.waitForResult<{ approved: boolean }>(record.id); - expect(completed.status).toBe("completed"); + expect(completed.status).toBe("awaiting-resubmission"); expect(completed.result?.approved).toBe(true); }); diff --git a/packages/server/daemon/session-factory.ts b/packages/server/daemon/session-factory.ts index 309f1571..51a8e8be 100644 --- a/packages/server/daemon/session-factory.ts +++ b/packages/server/daemon/session-factory.ts @@ -690,12 +690,19 @@ export function createDaemonSessionFactory(options: DaemonSessionFactoryOptions) if (request.action === "annotate" || request.action === "annotate-last") { const input = await resolveAnnotateInput(request, cwd, request.action); const isSingleFile = input.mode === "annotate"; - const matchKey = isSingleFile ? `annotate:${project}:${input.filePath}` : undefined; + const isFolder = input.mode === "annotate-folder"; + const matchKey = isSingleFile + ? `annotate:${project}:${input.filePath}` + : isFolder && input.folderPath + ? `annotate:${project}:folder:${input.folderPath}` + : undefined; if (matchKey) { const existing = findMatchingSession(context.store, matchKey); - if (existing && existing.session.updateContent) { - existing.session.updateContent(input.markdown, input.rawHtml); + if (existing) { + if (existing.session.updateContent) { + existing.session.updateContent(input.markdown, input.rawHtml); + } context.store.reactivate(existing.record.id); return existing.record; } diff --git a/packages/server/index.ts b/packages/server/index.ts index c2741f77..4b539769 100644 --- a/packages/server/index.ts +++ b/packages/server/index.ts @@ -199,6 +199,7 @@ export async function createPlannotatorSession( permissionMode?: string; }; const decisionCycle = createDecisionCycle(); + let lastDecision: 'approved' | 'denied' | null = null; if (mode !== "archive") { repoInfo = await getRepoInfo(cwd); @@ -289,7 +290,7 @@ export async function createPlannotatorSession( serverConfig: getServerConfig(gitUser), }); } - return Response.json({ plan, origin, permissionMode, sharingEnabled, shareBaseUrl, pasteApiUrl, repoInfo, previousPlan, versionInfo, projectRoot: cwd, isWSL: wslFlag, serverConfig: getServerConfig(gitUser) }); + return Response.json({ plan, origin, permissionMode, sharingEnabled, shareBaseUrl, pasteApiUrl, repoInfo, previousPlan, versionInfo, projectRoot: cwd, isWSL: wslFlag, serverConfig: getServerConfig(gitUser), lastDecision }); } // API: Serve a linked markdown document @@ -537,7 +538,8 @@ export async function createPlannotatorSession( // Use permission mode from client request if provided, otherwise fall back to hook input const effectivePermissionMode = requestedPermissionMode || permissionMode; - decisionCycle.resolve({ approved: true, feedback, savedPath, agentSwitch, permissionMode: effectivePermissionMode }); + lastDecision = 'approved'; + resolveAndCycle(decisionCycle, { approved: true, feedback, savedPath, agentSwitch, permissionMode: effectivePermissionMode }, origin); return Response.json({ ok: true, savedPath }); } @@ -574,6 +576,7 @@ export async function createPlannotatorSession( } deleteDraft(draftKey); + lastDecision = 'denied'; const resubmit = resolveAndCycle(decisionCycle, { approved: false, feedback, savedPath }, origin); return Response.json({ ok: true, savedPath, ...resubmit }); } @@ -589,6 +592,7 @@ export async function createPlannotatorSession( function handleUpdateContent(newPlan: string) { plan = newPlan; + lastDecision = null; const historyResult = saveToHistory(project, slug, newPlan); currentPlanPath = historyResult.path; previousPlan = historyResult.version > 1 diff --git a/packages/server/review.ts b/packages/server/review.ts index 42f4f900..193394cc 100644 --- a/packages/server/review.ts +++ b/packages/server/review.ts @@ -509,6 +509,7 @@ export async function createReviewSession( // Decision promise type ReviewDecisionResult = { approved: boolean; feedback: string; annotations: unknown[]; agentSwitch?: string; exit?: boolean }; const decisionCycle = createDecisionCycle(); + let lastDecision: 'approved' | 'feedback' | 'exited' | null = null; const handleRequest: SessionRequestHandler = async (req, url, context) => { @@ -562,6 +563,7 @@ export async function createReviewSession( ...(isPRMode && initialViewedFiles.length > 0 && { viewedFiles: initialViewedFiles }), ...(currentError && { error: currentError }), serverConfig: getServerConfig(gitUser), + lastDecision, }); } @@ -1040,7 +1042,8 @@ export async function createReviewSession( // API: Exit review session without feedback if (url.pathname === "/api/exit" && req.method === "POST") { deleteDraft(draftKey); - decisionCycle.resolve({ approved: false, feedback: "", annotations: [], exit: true }); + lastDecision = 'exited'; + resolveAndCycle(decisionCycle, { approved: false, feedback: "", annotations: [], exit: true }, origin); return Response.json({ ok: true }); } @@ -1056,6 +1059,7 @@ export async function createReviewSession( deleteDraft(draftKey); const isApproved = body.approved ?? false; + lastDecision = isApproved ? 'approved' : 'feedback'; const result = { approved: isApproved, feedback: body.feedback || "", annotations: body.annotations || [], agentSwitch: body.agentSwitch }; const resubmit = resolveAndCycle(decisionCycle, result, origin); @@ -1185,6 +1189,7 @@ export async function createReviewSession( } currentPatch = patch; currentGitRef = label; + lastDecision = null; externalAnnotations.clearAll(); deleteDraft(draftKey); draftKey = contentHash(patch); From d3dadb12f1b885690b100f11545f558a7a2bd668 Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Tue, 26 May 2026 16:16:18 -0700 Subject: [PATCH 33/35] Fix snapshot state wipe, editor annotations, remote share, HTML draft key - Session-revision handlers only reset awaiting/feedback/submitted state when content actually changed, preventing WebSocket snapshots from wiping lastDecision-restored state on tab refresh - Add feedbackSent to annotate action bar disabled state so buttons hide after folder/annotate-last feedback - Add clearAll() to editor annotation handler; call it in plan and review handleUpdateContent so VS Code annotations don't survive across revisions - Regenerate remoteShare URL in all three session reuse paths (plan, annotate, review) so remote users get current share links - Fix raw-HTML annotate draft key to hash rawHtml instead of empty markdown, preventing cross-session draft collisions - Update overview.md: remove all stale 10-minute TTL references --- goals/session-persistence/decisions.md | 3 +++ goals/session-persistence/overview.md | 14 ++++++------- packages/plannotator-code-review/App.tsx | 6 +++--- packages/plannotator-plan-review/App.tsx | 25 ++++++++++------------- packages/server/annotate.ts | 2 +- packages/server/daemon/session-factory.ts | 9 ++++++++ packages/server/editor-annotations.ts | 2 ++ packages/server/index.ts | 1 + packages/server/review.ts | 1 + 9 files changed, 37 insertions(+), 26 deletions(-) diff --git a/goals/session-persistence/decisions.md b/goals/session-persistence/decisions.md index 6b46e4fc..e1c21b45 100644 --- a/goals/session-persistence/decisions.md +++ b/goals/session-persistence/decisions.md @@ -141,4 +141,7 @@ Non-terminal sessions (`awaiting-resubmission`, `idle`, `active` after first dec | `onCancel` never wired on awaiting banner | nit | Deferred | | Session collisions across same-repo worktrees | nit | Accepted — local app, one daemon per machine | | Annotate slug doesn't update on heading change | nit | Accepted — cosmetic, versions work correctly | +| VS Code editor annotations not cleared on revision | P2 | Fixed — `editorAnnotations.clearAll()` added to `handleUpdateContent` in plan and review servers | +| PR diff scope/baseline not reset on reuse | P2 | Deferred — part of the broader PR metadata staleness issue. `originalPRPatch`, `currentPRDiffScope` not updated in `handleUpdateContent`. | +| Remote share link stale on session reuse | P2 | Fixed — all three reuse paths regenerate `remoteShare` before returning the record | | `sessionRefs` lazy cleanup | nit | Accepted — negligible memory | diff --git a/goals/session-persistence/overview.md b/goals/session-persistence/overview.md index 666cb87a..0bdfb7a8 100644 --- a/goals/session-persistence/overview.md +++ b/goals/session-persistence/overview.md @@ -27,7 +27,7 @@ This works for all three session types: - Exit works exactly as before - Auto-close works exactly as before - Sessions opened without an agent (standalone, demo) behave exactly as before — deny is still final -- The session expires after 10 minutes if the agent doesn't resubmit +- Sessions do not expire — they persist until daemon restart --- @@ -43,12 +43,10 @@ The deny-resubmit cycle is the core feedback loop of plan-driven development. Ma ### New Session Status: `awaiting-resubmission` -A non-terminal status in the daemon session lifecycle. The session stays alive — its HTTP handler keeps serving requests, the WebSocket connection stays open, and the frontend connection persists. After 10 minutes without resubmission, the session expires normally. +A non-terminal status in the daemon session lifecycle. The session stays alive — its HTTP handler keeps serving requests, the WebSocket connection stays open, and the frontend connection persists. Sessions do not expire; they persist until daemon restart. ``` -active → awaiting-resubmission → active → completed - ↓ - expired (10 min TTL) +active → awaiting-resubmission → active → awaiting-resubmission → ... ``` ### Decision Cycle Model @@ -129,7 +127,7 @@ The CLI binary accepts `awaiting-resubmission` as a valid non-error status. It o 2. The agent resubmits → same session updates in place 3. Works for plan, code review, and file-based annotate 4. Matching is by project+slug (plan), project+branch (review), or filepath (annotate) -5. 10-minute timeout if the agent doesn't come back +5. Sessions persist until daemon restart — no timeout 6. Agent doesn't need to know — matching is server-side 7. Shared `createDecisionCycle` helper eliminates duplication across three servers 8. Frontend shows amber "waiting" banner with cancel option @@ -146,7 +144,7 @@ The CLI binary accepts `awaiting-resubmission` as a valid non-error status. It o > It computes a match key (`plan:${project}:${slug}`) and searches for an `awaiting-resubmission` session with the same key. **3.** What happens if the agent changes the plan's heading when resubmitting? -> Different heading → different slug → no match → new session. The old session expires after 10 minutes. +> Different heading → different slug → no match → new session. The old session persists until daemon restart. **4.** Does the agent need to track session IDs or know about persistence? > No. The CLI binary runs fresh each time. Matching is entirely server-side. @@ -161,4 +159,4 @@ The CLI binary accepts `awaiting-resubmission` as a valid non-error status. It o > It completes normally (no persistence). URL sources can't be refreshed, so no match key is set. **8.** How long does the session wait for the agent to resubmit? -> 10 minutes. Then it expires via `cleanupExpired()`. +> Indefinitely. Sessions persist until daemon restart — no timeout. diff --git a/packages/plannotator-code-review/App.tsx b/packages/plannotator-code-review/App.tsx index 987b8a10..b46c113d 100644 --- a/packages/plannotator-code-review/App.tsx +++ b/packages/plannotator-code-review/App.tsx @@ -330,10 +330,10 @@ const ReviewApp: React.FC<{ __embedded?: boolean; headerLeft?: React.ReactNode; storeApi.getState().selectAnnotation(null); storeApi.getState().setPendingSelection(null); setViewedFiles(prev => retainUnchangedViewedFiles(oldFiles, newFiles, prev)); + setFeedbackSent(false); + setSubmitted(false); + setIsSendingFeedback(false); } - setFeedbackSent(false); - setSubmitted(false); - setIsSendingFeedback(false); } }); return unsubscribe; diff --git a/packages/plannotator-plan-review/App.tsx b/packages/plannotator-plan-review/App.tsx index 30dcd82d..99658746 100644 --- a/packages/plannotator-plan-review/App.tsx +++ b/packages/plannotator-plan-review/App.tsx @@ -599,23 +599,20 @@ const App: React.FC<{ __embedded?: boolean; headerLeft?: React.ReactNode; onOpen const unsubscribe = subscribeToDaemonSessionFamily("session-revision", (msg) => { if (!msg.payload) return; const revision = msg.payload as { plan?: string; previousPlan?: string | null; versionInfo?: { version: number; totalVersions: number; project: string }; rawHtml?: string }; - if (revision.plan !== undefined) { - const contentChanged = revision.plan !== markdownRef.current; + if (revision.plan !== undefined && revision.plan !== markdownRef.current) { if (revision.rawHtml !== undefined) { setRawHtml(revision.rawHtml); setRenderAs('html'); } - if (contentChanged) { - setMarkdown(revision.plan); - if (revision.previousPlan !== undefined) setPreviousPlan(revision.previousPlan); - if (revision.versionInfo) setVersionInfo(revision.versionInfo); - setAnnotations([]); - setCodeAnnotations([]); - setGlobalAttachments([]); - setSelectedAnnotationId(null); - setSelectedCodeAnnotationId(null); - linkedDocHook.clearCache(); - } + setMarkdown(revision.plan); + if (revision.previousPlan !== undefined) setPreviousPlan(revision.previousPlan); + if (revision.versionInfo) setVersionInfo(revision.versionInfo); + setAnnotations([]); + setCodeAnnotations([]); + setGlobalAttachments([]); + setSelectedAnnotationId(null); + setSelectedCodeAnnotationId(null); + linkedDocHook.clearCache(); setAwaitingResubmission(false); setFeedbackSent(false); setSubmitted(null); @@ -1823,7 +1820,7 @@ const App: React.FC<{ __embedded?: boolean; headerLeft?: React.ReactNode; onOpen gate={gate} isSharedSession={isSharedSession} origin={origin} - submitted={!!submitted || awaitingResubmission} + submitted={!!submitted || awaitingResubmission || feedbackSent} isSubmitting={isSubmitting} isExiting={isExiting} isPanelOpen={isPanelOpen} diff --git a/packages/server/annotate.ts b/packages/server/annotate.ts index c4178233..6ea70ae0 100644 --- a/packages/server/annotate.ts +++ b/packages/server/annotate.ts @@ -362,7 +362,7 @@ export async function createAnnotateSession( } externalAnnotations.clearAll(); deleteDraft(draftKey); - draftKey = contentHash(newMarkdown); + draftKey = contentHash(renderHtml && rawHtml ? rawHtml : newMarkdown); options.sessionEvents?.publishEvent("session-revision", { plan: newMarkdown, previousPlan, versionInfo, ...(rawHtml !== undefined && { rawHtml }), diff --git a/packages/server/daemon/session-factory.ts b/packages/server/daemon/session-factory.ts index 51a8e8be..926c9447 100644 --- a/packages/server/daemon/session-factory.ts +++ b/packages/server/daemon/session-factory.ts @@ -614,6 +614,9 @@ export function createDaemonSessionFactory(options: DaemonSessionFactoryOptions) const existing = findMatchingSession(context.store, matchKey); if (existing && existing.session.updateContent) { existing.session.updateContent(plan); + if (context.endpoint.isRemote && sharingEnabled) { + existing.record.remoteShare = await createRemoteShareNotice(plan, shareBaseUrl, "review the plan", "plan only").catch(() => undefined); + } context.store.reactivate(existing.record.id); return existing.record; } @@ -703,6 +706,9 @@ export function createDaemonSessionFactory(options: DaemonSessionFactoryOptions) if (existing.session.updateContent) { existing.session.updateContent(input.markdown, input.rawHtml); } + if (context.endpoint.isRemote && sharingEnabled && input.markdown) { + existing.record.remoteShare = await createRemoteShareNotice(input.markdown, shareBaseUrl, "annotate", "document only").catch(() => undefined); + } context.store.reactivate(existing.record.id); return existing.record; } @@ -755,6 +761,9 @@ export function createDaemonSessionFactory(options: DaemonSessionFactoryOptions) input.prMetadata ? input.rawPatch : undefined, input.prMetadata ? input.gitRef : undefined, ); + if (context.endpoint.isRemote && sharingEnabled) { + existingReview.record.remoteShare = await createRemoteShareNotice(input.rawPatch, shareBaseUrl, "review changes", "diff only").catch(() => undefined); + } context.store.reactivate(existingReview.record.id); return existingReview.record; } diff --git a/packages/server/editor-annotations.ts b/packages/server/editor-annotations.ts index cd01f2fc..97d74bd9 100644 --- a/packages/server/editor-annotations.ts +++ b/packages/server/editor-annotations.ts @@ -12,12 +12,14 @@ export type { EditorAnnotation }; export interface EditorAnnotationHandler { handle: (req: Request, url: URL) => Promise; + clearAll: () => void; } export function createEditorAnnotationHandler(): EditorAnnotationHandler { const annotations: EditorAnnotation[] = []; return { + clearAll() { annotations.length = 0; }, async handle(req: Request, url: URL): Promise { // GET /api/editor-annotations — return all if (url.pathname === "/api/editor-annotations" && req.method === "GET") { diff --git a/packages/server/index.ts b/packages/server/index.ts index 4b539769..90288fac 100644 --- a/packages/server/index.ts +++ b/packages/server/index.ts @@ -604,6 +604,7 @@ export async function createPlannotatorSession( project, }; externalAnnotations?.clearAll(); + editorAnnotations?.clearAll(); deleteDraft(draftKey); draftKey = contentHash(newPlan); options.sessionEvents?.publishEvent("session-revision", { plan: newPlan, previousPlan, versionInfo }); diff --git a/packages/server/review.ts b/packages/server/review.ts index 193394cc..632b4e4a 100644 --- a/packages/server/review.ts +++ b/packages/server/review.ts @@ -1191,6 +1191,7 @@ export async function createReviewSession( currentGitRef = label; lastDecision = null; externalAnnotations.clearAll(); + editorAnnotations.clearAll(); deleteDraft(draftKey); draftKey = contentHash(patch); options.sessionEvents?.publishEvent("session-revision", { rawPatch: patch, gitRef: label }); From dc63a1ac9f23754d34b18bf2b0ed6e99ec9f52ba Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Tue, 26 May 2026 16:19:12 -0700 Subject: [PATCH 34/35] Fix session-revision handler: distinguish snapshots from live events State resets (feedbackSent, awaitingResubmission, submitted) now fire on content change OR live events, but not on snapshots with unchanged content. This fixes two competing requirements: - Tab refresh: snapshot has same content, skip state reset (preserves lastDecision-restored state) - Folder reactivation: live event has same content, reset state (re-enables buttons after feedbackSent) --- packages/plannotator-code-review/App.tsx | 2 ++ packages/plannotator-plan-review/App.tsx | 6 +++++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/packages/plannotator-code-review/App.tsx b/packages/plannotator-code-review/App.tsx index b46c113d..413b9ac8 100644 --- a/packages/plannotator-code-review/App.tsx +++ b/packages/plannotator-code-review/App.tsx @@ -330,6 +330,8 @@ const ReviewApp: React.FC<{ __embedded?: boolean; headerLeft?: React.ReactNode; storeApi.getState().selectAnnotation(null); storeApi.getState().setPendingSelection(null); setViewedFiles(prev => retainUnchangedViewedFiles(oldFiles, newFiles, prev)); + } + if (contentChanged || msg.type === "event") { setFeedbackSent(false); setSubmitted(false); setIsSendingFeedback(false); diff --git a/packages/plannotator-plan-review/App.tsx b/packages/plannotator-plan-review/App.tsx index 99658746..21058bd0 100644 --- a/packages/plannotator-plan-review/App.tsx +++ b/packages/plannotator-plan-review/App.tsx @@ -599,7 +599,9 @@ const App: React.FC<{ __embedded?: boolean; headerLeft?: React.ReactNode; onOpen const unsubscribe = subscribeToDaemonSessionFamily("session-revision", (msg) => { if (!msg.payload) return; const revision = msg.payload as { plan?: string; previousPlan?: string | null; versionInfo?: { version: number; totalVersions: number; project: string }; rawHtml?: string }; - if (revision.plan !== undefined && revision.plan !== markdownRef.current) { + if (revision.plan === undefined) return; + const contentChanged = revision.plan !== markdownRef.current; + if (contentChanged) { if (revision.rawHtml !== undefined) { setRawHtml(revision.rawHtml); setRenderAs('html'); @@ -613,6 +615,8 @@ const App: React.FC<{ __embedded?: boolean; headerLeft?: React.ReactNode; onOpen setSelectedAnnotationId(null); setSelectedCodeAnnotationId(null); linkedDocHook.clearCache(); + } + if (contentChanged || msg.type === "event") { setAwaitingResubmission(false); setFeedbackSent(false); setSubmitted(null); From 8ba95767b4108c149d9035690c8784f15acb36cf Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Tue, 26 May 2026 16:45:50 -0700 Subject: [PATCH 35/35] =?UTF-8?q?Fix=20dead=20import,=20HTML=E2=86=92markd?= =?UTF-8?q?own=20switching,=20standalone=20awaiting=20overlay?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Remove unused PlannotatorSession type import from session-factory - Always assign rawHtml in annotate updateContent so clearing works when a file switches from --render-html to plain markdown - Wire awaitingResubmission into CompletionOverlay for standalone and legacy tab mode so deny shows a waiting screen instead of nothing --- packages/plannotator-plan-review/App.tsx | 6 +++--- packages/server/annotate.ts | 2 +- packages/server/daemon/session-factory.ts | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/plannotator-plan-review/App.tsx b/packages/plannotator-plan-review/App.tsx index 21058bd0..25c44899 100644 --- a/packages/plannotator-plan-review/App.tsx +++ b/packages/plannotator-plan-review/App.tsx @@ -2321,9 +2321,9 @@ const App: React.FC<{ __embedded?: boolean; headerLeft?: React.ReactNode; onOpen {/* Full-screen overlay: standalone mode, or legacy tab mode even when embedded */} {(!__embedded || legacyTabMode) && ( )} diff --git a/packages/server/annotate.ts b/packages/server/annotate.ts index 6ea70ae0..f420f031 100644 --- a/packages/server/annotate.ts +++ b/packages/server/annotate.ts @@ -348,7 +348,7 @@ export async function createAnnotateSession( function handleUpdateContent(newMarkdown: string, newRawHtml?: string) { markdown = newMarkdown; lastDecision = null; - if (newRawHtml !== undefined) rawHtml = newRawHtml; + rawHtml = newRawHtml; if (isFileBased && newMarkdown.trim()) { const historyResult = saveToHistory(project, slug, newMarkdown); previousPlan = historyResult.version > 1 diff --git a/packages/server/daemon/session-factory.ts b/packages/server/daemon/session-factory.ts index 926c9447..0f75634f 100644 --- a/packages/server/daemon/session-factory.ts +++ b/packages/server/daemon/session-factory.ts @@ -24,7 +24,7 @@ import type { PluginReviewRequest, } from "@plannotator/shared/plugin-protocol"; import { normalizeGoalSetupBundle } from "@plannotator/shared/goal-setup"; -import { createPlannotatorSession, type PlannotatorSession } from "../index"; +import { createPlannotatorSession } from "../index"; import { generateSlug } from "../storage"; import { createAnnotateSession } from "../annotate"; import { createGoalSetupSession } from "../goal-setup";