diff --git a/AGENTS.md b/AGENTS.md index 3f101ab8ce..edfd5c2937 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -139,6 +139,7 @@ claude --plugin-dir ./apps/hook | `PLANNOTATOR_ORIGIN` | Explicit agent-origin override at the top of the detection chain. Valid values: `claude-code`, `amp`, `droid`, `opencode`, `codex`, `copilot-cli`, `gemini-cli`, `kiro-cli`, `pi`. Invalid values silently fall through to env-based detection. Unset by default. | | `PLANNOTATOR_JINA` | Set to `0` / `false` to disable Jina Reader for URL annotation, or `1` / `true` to enable. Default: enabled. Can also be set via `~/.plannotator/config.json` (`{ "jina": false }`) or per-invocation via `--no-jina`. | | `PLANNOTATOR_ANNOTATE_HISTORY` | Set to `0` / `false` to disable per-file version history in annotate mode (no copies of annotated files are written to the data dir; the annotate version diff is unavailable). Default: enabled. Can also be set via `~/.plannotator/config.json` (`{ "annotateHistory": false }`); the env var takes precedence. | +| `PLANNOTATOR_GUIDE_HISTORY` | Set to `0` / `false` to disable persisting successful Guided Reviews (no guide copies are written to the data dir; the "Previous guides" list is then never populated, though already-saved guides remain readable and listed). Default: enabled. Can also be set via `~/.plannotator/config.json` (`{ "guideHistory": false }`); the env var takes precedence. | | `PLANNOTATOR_CURSOR_SANDBOX` | Set to `0` / `false` / `disabled` to stop passing `--sandbox enabled` when launching Cursor's `agent` CLI for review jobs — the flag pair is omitted entirely, deferring to the user's own Cursor Agent sandbox configuration. For systems where Cursor's sandbox cannot start (NixOS, AppArmor-restricted Linux). Default: enabled (`--sandbox enabled` is passed). Can also be set via `~/.plannotator/config.json` (`{ "cursorSandbox": false }`); the env var takes precedence. Note: opting out means the review job's write protection relies on `--mode ask` plus the user's own Cursor configuration. | | `JINA_API_KEY` | Optional Jina Reader API key for higher rate limits (500 RPM vs 20 RPM unauthenticated). Free keys include 10M tokens. | | `PLANNOTATOR_DATA_DIR` | Override the base data directory. Supports `~` expansion. Default: `~/.plannotator`. When unset, an existing `~/.plannotator` always wins; if it doesn't exist and `$XDG_DATA_HOME` is set to an absolute path, `$XDG_DATA_HOME/plannotator` is used; otherwise `~/.plannotator` (the XDG spec's implicit `~/.local/share` default is deliberately not applied). All data (plans, history, drafts, config, hooks, sessions, debug logs, IPC registry) is stored under this directory. | @@ -364,8 +365,10 @@ During normal plan review, an Archive sidebar tab provides the same browsing via | `/api/pr-switch` | POST | Switch to a different PR in-place (body: `{ url }`). Response includes `semanticDiff?`. | | `/api/tour/:jobId` | GET | Fetch Code Tour result (greeting, stops, checklist) for a completed tour job | | `/api/tour/:jobId/checklist` | PUT | Persist checklist item state for a Code Tour | -| `/api/guide/:jobId` | GET | Fetch Guided Review result (ordered sections with overviews + file refs) for a completed guide job | -| `/api/guide/:jobId/reviewed` | PUT | Persist per-section reviewed state for a guide | +| `/api/guide/:jobId` | GET | Fetch Guided Review result (ordered sections with overviews + file refs) for a completed guide job, or a persisted guide via the `saved:{id}` pseudo job id | +| `/api/guide/:jobId/reviewed` | PUT | Persist per-section reviewed state for a guide (live job ids write through to the job's autosaved file; `saved:{id}` ids persist directly) | +| `/api/guides` | GET | List persisted guides for the current repo: `[{ id, label, title, savedAt, progress: { reviewed, total }, moved }]` — `moved` flags a stored head sha that differs from the head currently under review | +| `/api/guides/:id` | DELETE | Delete a persisted guide | | `/api/guide/:jobId/output` | GET | Fetch a failed guide job's captured raw output for manual repair (404 if none captured) | | `/api/guide/:jobId/submit` | POST | Manually submit corrected guide JSON for a failed job (body: `{ payload }`) | | `/api/code-nav/resolve` | POST | Search for symbol definitions and references via ripgrep (body: `{ symbol, filePath, line, charStart, side, language? }`) | diff --git a/apps/pi-extension/server/agent-jobs.ts b/apps/pi-extension/server/agent-jobs.ts index 315cfbff0c..4ec35be09e 100644 --- a/apps/pi-extension/server/agent-jobs.ts +++ b/apps/pi-extension/server/agent-jobs.ts @@ -113,6 +113,10 @@ export interface AgentJobHandlerOptions { * model planned section placement against, not whatever patch is on * screen when the job happens to finish. */ changedFilesSnapshot?: string[]; + /** Launch-time review-target snapshot (guide provider only) — stored on + * AgentJobInfo so guide persistence labels the envelope with the context + * the guide was generated against (see AgentJobInfo.guideContext). */ + guideContext?: AgentJobInfo["guideContext"]; } | null>; /** Called when a job completes successfully — parse results and push annotations. */ onJobComplete?: (job: AgentJobInfo, meta: { outputPath?: string; stdout?: string; cwd?: string; changedFilesSnapshot?: string[] }) => void | Promise; @@ -214,7 +218,7 @@ export function createAgentJobHandler(options: AgentJobHandlerOptions) { command: string[], label: string, outputPath?: string, - spawnOptions?: { captureStdout?: boolean; stdinPrompt?: string; cwd?: string; prompt?: string; engine?: string; model?: string; effort?: string; reasoningEffort?: string; fastMode?: boolean; thinking?: string; prUrl?: string; diffScope?: string; diffContext?: AgentJobInfo["diffContext"]; reviewProfileId?: string; reviewProfileLabel?: string; changedFilesSnapshot?: string[] }, + spawnOptions?: { captureStdout?: boolean; stdinPrompt?: string; cwd?: string; prompt?: string; engine?: string; model?: string; effort?: string; reasoningEffort?: string; fastMode?: boolean; thinking?: string; prUrl?: string; diffScope?: string; diffContext?: AgentJobInfo["diffContext"]; reviewProfileId?: string; reviewProfileLabel?: string; changedFilesSnapshot?: string[]; guideContext?: AgentJobInfo["guideContext"] }, ): AgentJobInfo { const source = jobSource(id); @@ -236,6 +240,7 @@ export function createAgentJobHandler(options: AgentJobHandlerOptions) { ...(spawnOptions?.prUrl && { prUrl: spawnOptions.prUrl }), ...(spawnOptions?.diffScope && { diffScope: spawnOptions.diffScope }), ...(spawnOptions?.diffContext && { diffContext: spawnOptions.diffContext }), + ...(spawnOptions?.guideContext && { guideContext: spawnOptions.guideContext }), ...(spawnOptions?.reviewProfileId && { reviewProfileId: spawnOptions.reviewProfileId }), ...(spawnOptions?.reviewProfileLabel && { reviewProfileLabel: spawnOptions.reviewProfileLabel }), }; @@ -621,6 +626,7 @@ export function createAgentJobHandler(options: AgentJobHandlerOptions) { let jobReviewProfileId: string | undefined; let jobReviewProfileLabel: string | undefined; let jobChangedFilesSnapshot: string[] | undefined; + let jobGuideContext: AgentJobInfo["guideContext"] | undefined; const jobId = crypto.randomUUID(); if (options.buildCommand) { // Thread config from POST body to buildCommand @@ -654,6 +660,7 @@ export function createAgentJobHandler(options: AgentJobHandlerOptions) { jobReviewProfileId = built.reviewProfileId; jobReviewProfileLabel = built.reviewProfileLabel; jobChangedFilesSnapshot = built.changedFilesSnapshot; + jobGuideContext = built.guideContext; } } @@ -679,6 +686,7 @@ export function createAgentJobHandler(options: AgentJobHandlerOptions) { reviewProfileId: jobReviewProfileId, reviewProfileLabel: jobReviewProfileLabel, changedFilesSnapshot: jobChangedFilesSnapshot, + guideContext: jobGuideContext, }); json(res, { job }, 201); } catch (err) { diff --git a/apps/pi-extension/server/serverReview.ts b/apps/pi-extension/server/serverReview.ts index bddf0ac111..c25a06ba25 100644 --- a/apps/pi-extension/server/serverReview.ts +++ b/apps/pi-extension/server/serverReview.ts @@ -6,7 +6,7 @@ import { basename, resolve as resolvePath } from "node:path"; import { SingleFlight } from "../generated/single-flight.ts"; import { contentHash, deleteDraft } from "../generated/draft.ts"; -import { loadConfig, saveConfig, detectGitUser, getServerConfig, resolveSharingEnabled, resolveCursorSandbox } from "../generated/config.ts"; +import { loadConfig, saveConfig, detectGitUser, getServerConfig, resolveSharingEnabled, resolveCursorSandbox, resolveGuideHistory } from "../generated/config.ts"; export type { DiffOption, @@ -122,6 +122,7 @@ import { } from "../generated/claude-review.ts"; import { createTourSession, TOUR_EMPTY_OUTPUT_ERROR } from "../generated/tour-review.ts"; import { createGuideSession, GUIDE_EMPTY_OUTPUT_ERROR } from "../generated/guide-review.ts"; +import { createGuideStoreSession, SAVED_GUIDE_ID_PREFIX } from "../generated/guide-store.ts"; import { MARKER_ENGINES, composeMarkerReviewPrompt, @@ -767,6 +768,34 @@ export async function startReviewServer(options: { } const tour = createTourSession(); const guide = createGuideSession(); + // Durable guide persistence (#1112): autosaves validated guides to + // ${PLANNOTATOR_DATA_DIR}/guides/{repo-key}/ and serves them back through + // the existing guide endpoints as `saved:{id}` pseudo job ids. All getters + // are late-bound — prMeta/currentDiffType can change mid-session. + // Mirrors packages/server/review.ts's guideStore wiring. + const guideStore = createGuideStoreSession({ + runGit: async (args, cwd) => { + const result = await reviewRuntime.runGit(args, { cwd }); + return result.exitCode === 0 ? result.stdout : null; + }, + getGitCwd: () => + isPRMode || isWorkspaceMode + ? undefined + : options.gitContext + ? resolveVcsCwd(currentDiffType as DiffType, options.gitContext.cwd) ?? options.gitContext.cwd ?? process.cwd() + : undefined, + getPRInfo: () => + prMeta + ? { + url: prMeta.url, + headSha: prMeta.headSha, + label: `${getMRLabel(prMeta)} ${getMRNumberLabel(prMeta)}`, + } + : null, + getBranchLabel: () => clientGitContext?.currentBranch || options.gitContext?.currentBranch, + getFallbackDir: () => workspace?.root ?? options.agentCwd ?? process.cwd(), + writesEnabled: () => resolveGuideHistory(loadConfig()), + }); const semanticDiffScratchCwd = getSemanticDiffScratchCwd(); function resolveSemanticDiffCwd(diffType: DiffType = currentDiffType as DiffType): string { if (workspace) return workspace.root; @@ -1048,6 +1077,15 @@ export async function startReviewServer(options: { const changedFilesSnapshot = repairOf ? guide.getLaunchChangedFiles(repairOf) ?? changedFiles.map((f) => f.path) : changedFiles.map((f) => f.path); + // Snapshot the launch-time review-target context (#1112): guide jobs + // run for minutes while the session supports mid-generation PR/diff + // switching, so the persisted envelope must be labeled with the + // context this guide is GENERATED against — captured now, carried on + // the job (guideContext), and read back at completion instead of the + // live session state. Repairs reuse the FAILED job's own snapshot, + // same as changedFilesSnapshot above. Mirrors packages/server/review.ts. + const guideContext = (repairOf ? agentJobs.getJob(repairOf)?.guideContext : undefined) + ?? await guideStore.captureLaunchContext(); return { ...built, prUrl: launchPrUrl, @@ -1056,6 +1094,7 @@ export async function startReviewServer(options: { reviewProfileId: reviewProfile.id, reviewProfileLabel: reviewProfile.label, changedFilesSnapshot, + guideContext, }; } @@ -1292,6 +1331,13 @@ export async function startReviewServer(options: { const { summary, error } = await guide.onJobComplete({ job, meta, changedFiles }); if (summary) { job.summary = summary; + // Autosave (#1112): only guides that passed validateGuideOutput + // ever reach guideResults, so a getGuide hit here IS the + // validation gate. Failed/invalid guides never write. The job's + // launch-time context snapshot labels the envelope — never the + // live session state, which may have PR/diff-switched mid-run. + const validated = guide.getGuide(job.id); + if (validated) await guideStore.saveForJob(job, validated, job.guideContext); } else { // Same fail-closed precedent as Tour: an exit-0 job with empty, // malformed, or fully-invalidated output must not look like a @@ -1357,25 +1403,47 @@ export async function startReviewServer(options: { return; } - // API: Get guide result + // API: Get guide result — live job ids, or `saved:{id}` for a + // persisted guide loaded from the on-disk store (#1112). if (url.pathname.match(/^\/api\/guide\/[^/]+$/) && req.method === "GET") { - const jobId = url.pathname.slice("/api/guide/".length); + const jobId = decodeURIComponent(url.pathname.slice("/api/guide/".length)); + if (jobId.startsWith(SAVED_GUIDE_ID_PREFIX)) { + const saved = await guideStore.getSavedGuideData(jobId.slice(SAVED_GUIDE_ID_PREFIX.length)); + if (!saved) { + json(res, { error: "Guide not found" }, 404); + return; + } + json(res, saved); + return; + } const result = guide.getGuide(jobId); if (!result) { json(res, { error: "Guide not found" }, 404); return; } - json(res, result); + json(res, { ...result, ...(guideStore.isJobSaved(jobId) ? { saved: true } : {}) }); return; } - // API: Save guide reviewed state + // API: Save guide reviewed state. Live job ids also write through to + // the job's autosaved file; `saved:{id}` ids persist directly. const reviewedMatch = url.pathname.match(/^\/api\/guide\/([^/]+)\/reviewed$/); if (reviewedMatch && req.method === "PUT") { - const jobId = reviewedMatch[1]; + const jobId = decodeURIComponent(reviewedMatch[1]); try { const body = await parseBody(req) as { reviewed: boolean[] }; - if (Array.isArray(body.reviewed)) guide.saveReviewed(jobId, body.reviewed); + if (Array.isArray(body.reviewed)) { + if (jobId.startsWith(SAVED_GUIDE_ID_PREFIX)) { + const ok = await guideStore.updateSavedReviewed(jobId.slice(SAVED_GUIDE_ID_PREFIX.length), body.reviewed); + if (!ok) { + json(res, { error: "Guide not found" }, 404); + return; + } + } else { + guide.saveReviewed(jobId, body.reviewed); + await guideStore.writeThroughReviewed(jobId, body.reviewed); + } + } json(res, { ok: true }); } catch { json(res, { error: "Invalid JSON" }, 400); @@ -1383,6 +1451,24 @@ export async function startReviewServer(options: { return; } + // API: List saved guides for the current repo (#1112) + if (url.pathname === "/api/guides" && req.method === "GET") { + json(res, await guideStore.listSaved()); + return; + } + + // API: Delete a saved guide (#1112) + const savedGuideDeleteMatch = url.pathname.match(/^\/api\/guides\/([^/]+)$/); + if (savedGuideDeleteMatch && req.method === "DELETE") { + const ok = await guideStore.deleteSaved(decodeURIComponent(savedGuideDeleteMatch[1])); + if (!ok) { + json(res, { error: "Guide not found" }, 404); + return; + } + json(res, { ok: true }); + return; + } + // API: Get a failed guide job's captured raw output for manual repair const guideOutputMatch = url.pathname.match(/^\/api\/guide\/([^/]+)\/output$/); if (guideOutputMatch && req.method === "GET") { @@ -1427,6 +1513,11 @@ export async function startReviewServer(options: { explanation: `${sections} section${sections !== 1 ? "s" : ""}, ${files} file${files !== 1 ? "s" : ""} placed (manually repaired)`, confidence: 1, }); + // A manually repaired guide passed the same validateGuideOutput + // gate as an automatic one — persist it too (#1112), labeled + // with the job's own launch-time context snapshot. + const repaired = guide.getGuide(jobId); + if (repaired) await guideStore.saveForJob(existingJob, repaired, existingJob.guideContext); json(res, { ok: true, sections, files }); } catch { json(res, { error: "Invalid JSON" }, 400); diff --git a/apps/pi-extension/vendor.sh b/apps/pi-extension/vendor.sh index 2657fb8474..b31cf5d4d5 100755 --- a/apps/pi-extension/vendor.sh +++ b/apps/pi-extension/vendor.sh @@ -29,7 +29,7 @@ for f in config-types storage-types workspace-status-types; do done # Everything else in the original flat list stays sourced from packages/shared. -for f in prompts review-core diff-paths cli-pagination jj-core gitbutler-core vcs-core review-args draft pr-types pr-context-live pr-artifact-document pr-provider pr-stack pr-github pr-gitlab checklist integrations-common repo reference-common resolve-file annotate-reference-roots-node worktree worktree-pool html-to-markdown html-diff html-assets html-assets-node url-to-markdown tour annotate-args at-reference review-workspace-node review-workspace pfm-reminder improvement-hooks code-nav data-dir semantic-diff-types semantic-diff single-flight source-save-node review-profiles guide commit-avatars commit-history port-range; do +for f in prompts review-core diff-paths cli-pagination jj-core gitbutler-core vcs-core review-args draft pr-types pr-context-live pr-artifact-document pr-provider pr-stack pr-github pr-gitlab checklist integrations-common repo reference-common resolve-file annotate-reference-roots-node worktree worktree-pool html-to-markdown html-diff html-assets html-assets-node url-to-markdown tour annotate-args at-reference review-workspace-node review-workspace pfm-reminder improvement-hooks code-nav data-dir semantic-diff-types semantic-diff single-flight source-save-node review-profiles guide guide-store commit-avatars commit-history port-range; do src="../../packages/shared/$f.ts" printf '// @generated — DO NOT EDIT. Source: packages/shared/%s.ts\n' "$f" | cat - "$src" > "generated/$f.ts" done diff --git a/packages/core/agent-jobs.ts b/packages/core/agent-jobs.ts index d5492a8188..60ae1149df 100644 --- a/packages/core/agent-jobs.ts +++ b/packages/core/agent-jobs.ts @@ -111,6 +111,19 @@ export interface AgentJobInfo { reviewProfileId?: string; /** Resolved review profile label — rides on findings so the UI can show a profile tag. */ reviewProfileLabel?: string; + /** + * Launch-time review-target snapshot for guide jobs (#1112). Guide jobs run + * for minutes while the session supports mid-generation PR/diff switching, + * so the persisted envelope must be labeled with the context the guide was + * GENERATED against — captured here at build time (same discipline as + * changedFilesSnapshot), never re-read from live session state at + * completion. Structurally matches guide-store's GuideLaunchContext. + */ + guideContext?: { + pr?: { url: string; headSha: string; label: string } | null; + branchLabel?: string; + headSha?: string; + }; } export interface AgentCapability { diff --git a/packages/review-editor/App.tsx b/packages/review-editor/App.tsx index 75dae579db..1a24d85fc4 100644 --- a/packages/review-editor/App.tsx +++ b/packages/review-editor/App.tsx @@ -997,6 +997,17 @@ const ReviewApp: React.FC = () => { [prMetadata, activeWorktreePath], ); + // A `saved:{id}` guide has no AgentJobInfo, so GuideScreen's context match + // passes trivially (unknown ids are tolerated by design, for the demo path). + // Without this, switching PR or worktree while a saved guide is open would + // leave it mounted over the NEW context's diff with a stale `moved` flag. + // Clear it on any review-context switch — simplest and predictable; the + // user can reopen it from the Previous guides list. Live job ids are left + // alone: GuideScreen's own matchesContext already handles them. + useEffect(() => { + setActiveGuideJobId(prev => (prev?.startsWith('saved:') ? null : prev)); + }, [prMetadata?.url, activeWorktreePath]); + // Auto-open tour dialog when a tour job completes — scoped to the current // review context, same rule and same deferred-open semantics as the guide // effect below (an away-context tour stays unmarked so it opens when the @@ -3365,6 +3376,7 @@ const ReviewApp: React.FC = () => { killJob={agentJobs.killJob} onClose={() => setGuideOpen(false)} onOpenFixedGuide={handleOpenGuide} + onOpenSavedGuide={handleOpenGuide} /> )} diff --git a/packages/review-editor/components/guide/GuideEmptyState.tsx b/packages/review-editor/components/guide/GuideEmptyState.tsx index 15f20627f8..b51f5e6b52 100644 --- a/packages/review-editor/components/guide/GuideEmptyState.tsx +++ b/packages/review-editor/components/guide/GuideEmptyState.tsx @@ -1,8 +1,8 @@ import React, { useEffect, useState } from 'react'; import { ChevronDown, ChevronRight } from 'lucide-react'; import type { AgentCapabilities } from '@plannotator/ui/types'; +import type { SavedGuideListEntry } from '@plannotator/shared/guide'; import type { AgentLaunchParams } from '@plannotator/ui/hooks/useAgentJobs'; -import { useAgentSettings } from '@plannotator/ui/hooks/useAgentSettings'; import type { ReviewEngine } from '@plannotator/ui/hooks/useAgentSettings'; // Same catalogs AgentsTab's launch panel uses — one source of truth for both // guide launch surfaces (this page and the sidebar's Guided Review mode). @@ -15,18 +15,25 @@ import { REVIEW_ENGINE_LABEL, } from '@plannotator/ui/components/AgentsTab'; import { groupModelOptions, labelWithinGroup, SEARCHABLE_THRESHOLD } from '@plannotator/ui/components/AgentControls'; - -const GUIDE_ENGINES = Object.keys(REVIEW_ENGINE_LABEL) as ReviewEngine[]; - -// Marker-engine fallbacks until the server delivers the live catalogs on the -// capability entries (mirrors AgentsTab's fallbacks). -const CURSOR_FALLBACK = [{ value: 'auto', label: 'Auto' }]; -const OPENCODE_FALLBACK = [{ value: '', label: 'Default' }]; -const PI_FALLBACK = [{ value: '', label: 'Default' }]; -const COPILOT_FALLBACK = [{ value: '', label: 'Default' }]; +import { useGuideLaunch } from '../../hooks/guide/useGuideLaunch'; type Option = { value: string; label: string }; +/** "2d ago" style age for the previous-guides rows (local convention — same + * shape as PRCommentsTab's formatRelativeTime, taking epoch ms instead). */ +function formatSavedAge(savedAt: number): string { + if (!savedAt) return ''; + const diff = Date.now() - savedAt; + const minutes = Math.floor(diff / 60000); + const hours = Math.floor(minutes / 60); + const days = Math.floor(hours / 24); + if (minutes < 1) return 'just now'; + if (minutes < 60) return `${minutes}m ago`; + if (hours < 24) return `${hours}h ago`; + if (days < 30) return `${days}d ago`; + return new Date(savedAt).toLocaleDateString(undefined, { month: 'short', day: 'numeric' }); +} + /** Inline "Label: Value ▾" picker — the wireframe's compact select pill. * Long catalogs (marker-engine model lists can run to hundreds) automatically * gain a type-to-filter input, a wider popover, and provider group headers. */ @@ -150,6 +157,9 @@ interface GuideEmptyStateProps { /** Navigate to a guide by job id once a manually-fixed output is accepted by * the server (POST /api/guide/:jobId/submit → 200). */ onOpenFixedGuide?: (jobId: string) => void; + /** Open a persisted guide from the previous-guides list — receives the + * `saved:{id}` pseudo job id understood by the guide endpoints (#1112). */ + onOpenSavedGuide?: (jobId: string) => void; } /** @@ -159,18 +169,17 @@ interface GuideEmptyStateProps { * (the same persisted guide settings AgentsTab's launch panel edits), and a * primary Generate button. */ -export const GuideEmptyState: React.FC = ({ capabilities, launchJob, onBack, failure, onOpenFixedGuide }) => { +export const GuideEmptyState: React.FC = ({ capabilities, launchJob, onBack, failure, onOpenFixedGuide, onOpenSavedGuide }) => { + // Engine/model resolution + launch-param shapes are shared with GuideView's + // "Regenerate" hint via useGuideLaunch — this component keeps only the + // picker UI on top of it. + const launch = useGuideLaunch(capabilities); const { - guideEngine, guideClaudeModel, guideClaudeEffort, guideCodexModel, guideCodexReasoning, - guideCursorModel, - guideOpencodeModel, - guidePiModel, guidePiThinking, - guideCopilotModel, setGuideEngine, setGuideClaudeModel, setGuideClaudeEffort, @@ -181,11 +190,38 @@ export const GuideEmptyState: React.FC = ({ capabilities, setGuidePiModel, setGuidePiThinking, setGuideCopilotModel, - } = useAgentSettings(); + } = launch.settings; const [launching, setLaunching] = useState(false); const [launchError, setLaunchError] = useState(null); + // Previous guides persisted for this repo (#1112). Null until the list + // resolves; an empty list hides the section entirely. Standalone/demo mode + // has no backend — the fetch failure leaves the section hidden. + const [savedGuides, setSavedGuides] = useState(null); + useEffect(() => { + let alive = true; + fetch('/api/guides') + .then((res) => (res.ok ? res.json() : null)) + .then((data: SavedGuideListEntry[] | null) => { + if (alive && Array.isArray(data)) setSavedGuides(data); + }) + .catch(() => {}); + return () => { + alive = false; + }; + }, []); + + const handleDeleteSaved = async (id: string) => { + // Optimistic removal — a failed delete just resurfaces on the next open. + setSavedGuides((prev) => (prev ? prev.filter((g) => g.id !== id) : prev)); + try { + await fetch(`/api/guides/${encodeURIComponent(id)}`, { method: 'DELETE' }); + } catch { + // Best-effort. + } + }; + // Failure-recovery panel state (only relevant when `failure` is set — see // GuideScreen, which remounts this component fresh, keyed on jobId, for // each failed guide job so this state never leaks across failures). @@ -264,54 +300,19 @@ export const GuideEmptyState: React.FC = ({ capabilities, } }; - const providerAvailable = (id: string) => - capabilities?.providers.some((p) => p.id === id && p.available) ?? false; - const guideAvailable = providerAvailable('guide'); - const availableEngines = GUIDE_ENGINES.filter(providerAvailable); - // A persisted engine can be unavailable on this machine — fall back to the - // first available one rather than a dead selection. - const engine: ReviewEngine = providerAvailable(guideEngine) ? guideEngine : (availableEngines[0] ?? guideEngine); - - // Marker model catalogs are discovered server-side and delivered on the - // capability entry; fall back to the engine-default option until then. - // Mirrors AgentsTab's per-engine catalog semantics exactly: opencode/pi - // PREPEND their engine-managed "Default" ('' value) to the discovered list — - // the discovered catalogs are real models only, and dropping Default left a - // saved-default user with a blank pill and no way back after picking a - // concrete model. Cursor REPLACES: its discovered list natively includes - // 'auto', so prepending would duplicate it. - const markerModels = (id: 'cursor' | 'opencode' | 'pi' | 'copilot', fallback: Option[]): Option[] => { - const models = capabilities?.providers.find((p) => p.id === id)?.models; - if (!models || models.length === 0) return fallback; - const discovered = models.map((m) => ({ value: m.id, label: m.label })); - return id === 'cursor' ? discovered : [...fallback, ...discovered]; - }; - - // Compute each marker engine's catalog once so the picker and the launch - // params below read the exact same lists. - const cursorOptions = markerModels('cursor', CURSOR_FALLBACK); - const opencodeOptions = markerModels('opencode', OPENCODE_FALLBACK); - const piOptions = markerModels('pi', PI_FALLBACK); - const copilotOptions = markerModels('copilot', COPILOT_FALLBACK); - - // AgentsTab reconciles the saved guide Cursor/OpenCode/Pi model back to the - // catalog head via effects when the live catalog no longer contains it (see - // AgentsTab.tsx's reconcile effects ~712-745, which snap both the review - // AND guide fields against the same catalog). This launcher reads the same - // persisted cookie values but is deliberately stateless — no effects, no - // cookie writes — so instead of reconciling in the background, a stale - // saved id is reconciled right here at read time: if it's not in the - // current catalog, fall back to the catalog's first entry rather than - // POSTing a dead model id to the server. ('' / Default is present in the - // opencode/pi catalogs after the round-5 prepend fix, so a saved '' stays - // valid and is never treated as stale.) - const effectiveModel = (saved: string, options: Option[]): string => - options.some((o) => o.value === saved) ? saved : (options[0]?.value ?? saved); - - const effectiveCursorModel = effectiveModel(guideCursorModel, cursorOptions); - const effectiveOpencodeModel = effectiveModel(guideOpencodeModel, opencodeOptions); - const effectivePiModel = effectiveModel(guidePiModel, piOptions); - const effectiveCopilotModel = effectiveModel(guideCopilotModel, copilotOptions); + const { + guideAvailable, + availableEngines, + engine, + cursorOptions, + opencodeOptions, + piOptions, + copilotOptions, + effectiveCursorModel, + effectiveOpencodeModel, + effectivePiModel, + effectiveCopilotModel, + } = launch; const modelPicker: { value: string; options: Option[]; onChange: (v: string) => void } = engine === 'claude' @@ -326,53 +327,15 @@ export const GuideEmptyState: React.FC = ({ capabilities, ? { value: effectiveCopilotModel, options: copilotOptions, onChange: setGuideCopilotModel } : { value: effectivePiModel, options: piOptions, onChange: setGuidePiModel }; - const canLaunch = guideAvailable && availableEngines.length > 0 && !launching; + const canLaunch = launch.canLaunch && !launching; const handleGenerate = async () => { if (!canLaunch) return; setLaunching(true); setLaunchError(null); - // Config shapes mirror AgentsTab's buildGuideLaunch exactly — one shape - // per engine, so the server sees identical launches from both surfaces. - const params: AgentLaunchParams = - engine === 'cursor' - ? { - provider: 'guide', - label: 'Guided Review', - engine: 'cursor', - ...(effectiveCursorModel && effectiveCursorModel.toLowerCase() !== 'auto' ? { model: effectiveCursorModel } : {}), - } - : engine === 'opencode' - ? { - provider: 'guide', - label: 'Guided Review', - engine: 'opencode', - ...(effectiveOpencodeModel ? { model: effectiveOpencodeModel } : {}), - } - : engine === 'pi' - ? { - provider: 'guide', - label: 'Guided Review', - engine: 'pi', - ...(effectivePiModel ? { model: effectivePiModel } : {}), - thinking: guidePiThinking, - } - : engine === 'copilot' - ? { - provider: 'guide', - label: 'Guided Review', - engine: 'copilot', - ...(effectiveCopilotModel ? { model: effectiveCopilotModel } : {}), - } - : { - provider: 'guide', - label: 'Guided Review', - engine, - model: engine === 'claude' ? guideClaudeModel : guideCodexModel, - ...(engine === 'claude' - ? { effort: guideClaudeEffort } - : { reasoningEffort: guideCodexReasoning }), - }; + // Launch-param shapes live in useGuideLaunch (shared with GuideView's + // Regenerate hint) and mirror AgentsTab's buildGuideLaunch exactly. + const params: AgentLaunchParams = launch.buildParams(); try { await launchJob(params); } catch (err) { @@ -499,6 +462,51 @@ export const GuideEmptyState: React.FC = ({ capabilities, {launchError &&

{launchError}

} )} + + {savedGuides && savedGuides.length > 0 && ( +
+
+ Previous guides +
+
+ {savedGuides.map((entry) => ( +
+ + +
+ ))} +
+
+ )} ); }; diff --git a/packages/review-editor/components/guide/GuideScreen.tsx b/packages/review-editor/components/guide/GuideScreen.tsx index e8be94c73d..b7f05db5ee 100644 --- a/packages/review-editor/components/guide/GuideScreen.tsx +++ b/packages/review-editor/components/guide/GuideScreen.tsx @@ -6,6 +6,7 @@ import type { ReviewEngine } from '@plannotator/ui/hooks/useAgentSettings'; import { REVIEW_ENGINE_LABEL } from '@plannotator/ui/components/AgentsTab'; import { isTerminalStatus } from '@plannotator/shared/agent-jobs'; import { useGuideData } from '../../hooks/guide/useGuideData'; +import { useGuideLaunch } from '../../hooks/guide/useGuideLaunch'; import { useReviewState } from '../../dock/ReviewStateContext'; import { GuideEmptyState } from './GuideEmptyState'; import { GuideGenerating } from './GuideGenerating'; @@ -26,6 +27,10 @@ interface GuideScreenProps { * panel successfully submits a manually-fixed output (POST .../submit → * 200). Wired to the same handler ReviewSidebar's onOpenGuide uses. */ onOpenFixedGuide?: (jobId: string) => void; + /** Open a persisted guide from GuideEmptyState's previous-guides list — + * receives the `saved:{id}` pseudo job id (#1112). Wired to the same + * handler as onOpenFixedGuide; ids are opaque to this screen. */ + onOpenSavedGuide?: (jobId: string) => void; } /** @@ -46,6 +51,7 @@ export const GuideScreen: React.FC = ({ killJob, onClose, onOpenFixedGuide, + onOpenSavedGuide, }) => { const [cancelling, setCancelling] = useState(false); // GuideScreen renders inside ReviewStateProvider (ActiveGuide below already @@ -153,6 +159,7 @@ export const GuideScreen: React.FC = ({ capabilities={capabilities} launchJob={launchJob} onOpenFixedGuide={onOpenFixedGuide} + onOpenSavedGuide={onOpenSavedGuide} /> ); } @@ -182,11 +189,12 @@ export const GuideScreen: React.FC = ({ error: failedGuideJob.error ?? 'Guide generation failed.', }} onOpenFixedGuide={onOpenFixedGuide} + onOpenSavedGuide={onOpenSavedGuide} /> ); } - return ; + return ; }; function ActiveGuide({ @@ -196,6 +204,7 @@ function ActiveGuide({ capabilities, launchJob, onOpenFixedGuide, + onOpenSavedGuide, }: { jobId: string; jobs: AgentJobInfo[]; @@ -206,10 +215,26 @@ function ActiveGuide({ capabilities: AgentCapabilities | null; launchJob: (params: AgentLaunchParams) => Promise; onOpenFixedGuide?: (jobId: string) => void; + onOpenSavedGuide?: (jobId: string) => void; }) { const { guide, loading, error, reviewed, toggleReviewed, retry } = useGuideData(jobId); const [focusedFile, setFocusedFile] = useState(null); const state = useReviewState(); + // Shared guide-launch defaults, for the outdated-saved-guide "Regenerate" + // hint (#1112) — same params GuideEmptyState's Generate button would send. + const guideLaunch = useGuideLaunch(capabilities); + const [regenerating, setRegenerating] = useState(false); + const handleRegenerate = async () => { + if (!guideLaunch.canLaunch || regenerating) return; + setRegenerating(true); + try { + await launchJob(guideLaunch.buildParams()); + // A successful launch lands as a new running guide job — GuideScreen's + // running-job branch takes over from here automatically. + } finally { + setRegenerating(false); + } + }; // "Details" switches this screen over to the full failure-recovery panel // (GuideEmptyState's failure branch) for `failedJob`, instead of duplicating @@ -281,6 +306,7 @@ function ActiveGuide({ error: failedJob.error ?? 'Guide generation failed.', }} onOpenFixedGuide={onOpenFixedGuide} + onOpenSavedGuide={onOpenSavedGuide} /> ); } @@ -355,6 +381,7 @@ function ActiveGuide({ engineLabel={engine ? REVIEW_ENGINE_LABEL[engine as ReviewEngine] ?? engine : undefined} focusedFile={focusedFile} onFocusFile={setFocusedFile} + onRegenerate={guideLaunch.canLaunch && !regenerating ? handleRegenerate : undefined} /> ); diff --git a/packages/review-editor/components/guide/GuideView.test.tsx b/packages/review-editor/components/guide/GuideView.test.tsx new file mode 100644 index 0000000000..dd790b041c --- /dev/null +++ b/packages/review-editor/components/guide/GuideView.test.tsx @@ -0,0 +1,98 @@ +import { afterEach, describe, expect, mock, test } from 'bun:test'; +import React, { act } from 'react'; +import { createRoot, type Root } from 'react-dom/client'; +import type { CodeGuideData } from '@plannotator/shared/guide'; +import { ReviewStateProvider, type ReviewState } from '../../dock/ReviewStateContext'; + +// DiffViewer (imported via GuideDiffSection) loads its diff worker through a +// Vite `?worker&inline` virtual module that bun's test resolver can't parse; +// stub it before the component graph loads (hence the dynamic import below). +mock.module('@pierre/diffs/worker/worker.js?worker&inline', () => ({ default: class {} })); +const { GuideView } = await import('./GuideView'); + +const hasDom = typeof document !== 'undefined'; + +function makeGuide(overrides: Partial = {}): CodeGuideData { + return { + title: 'Persisted guide', + intent: 'Test intent.', + sections: [{ title: 'Core', overview: 'The heart.', diffs: [{ file: 'a.ts' }] }], + reviewed: [false], + ...overrides, + }; +} + +function makeState(): ReviewState { + return { + files: [], + guideRevealFile: null, + } as unknown as ReviewState; +} + +let root: Root | null = null; +let host: HTMLElement | null = null; + +afterEach(async () => { + if (root !== null) { + await act(async () => root?.unmount()); + root = null; + } + host?.remove(); + host = null; + if (hasDom) document.body.innerHTML = ''; +}); + +async function renderView(guide: CodeGuideData, onRegenerate?: () => void) { + host = document.createElement('div'); + document.body.appendChild(host); + await act(async () => { + root = createRoot(host!); + root.render( + + {}} + focusedFile={null} + onFocusFile={() => {}} + onRegenerate={onRegenerate} + /> + , + ); + }); +} + +describe('GuideView persistence affordances (#1112)', () => { + test.skipIf(!hasDom)('renders no Saved chip and no outdated hint by default', async () => { + await renderView(makeGuide()); + expect(host!.textContent).not.toContain('Saved'); + expect(host!.textContent).not.toContain('Generated on a different version'); + }); + + test.skipIf(!hasDom)('renders the Saved chip when the guide is persisted', async () => { + await renderView(makeGuide({ saved: true })); + expect(host!.textContent).toContain('Saved'); + expect(host!.textContent).not.toContain('Generated on a different version'); + }); + + test.skipIf(!hasDom)('renders the outdated hint with a wired Regenerate action when moved', async () => { + let regenerated = 0; + await renderView(makeGuide({ saved: true, moved: true }), () => { + regenerated += 1; + }); + expect(host!.textContent).toContain('Generated on a different version of this branch'); + + const regenerate = [...host!.querySelectorAll('button')].find((b) => b.textContent === 'Regenerate'); + expect(regenerate).not.toBeNull(); + await act(async () => { + regenerate!.click(); + }); + expect(regenerated).toBe(1); + }); + + test.skipIf(!hasDom)('moved without a regenerate handler renders the hint without a button', async () => { + await renderView(makeGuide({ saved: true, moved: true })); + expect(host!.textContent).toContain('Generated on a different version of this branch'); + expect([...host!.querySelectorAll('button')].find((b) => b.textContent === 'Regenerate')).toBeUndefined(); + }); +}); diff --git a/packages/review-editor/components/guide/GuideView.tsx b/packages/review-editor/components/guide/GuideView.tsx index 7dc2da133e..a28d662f9a 100644 --- a/packages/review-editor/components/guide/GuideView.tsx +++ b/packages/review-editor/components/guide/GuideView.tsx @@ -14,6 +14,10 @@ interface GuideViewProps { engineLabel?: string; focusedFile: string | null; onFocusFile: (filePath: string) => void; + /** Launch a fresh guide for the current changeset — wired to the shared + * guide-launch defaults (useGuideLaunch). Only rendered when the guide is + * outdated (`guide.moved`) and a launch is possible. */ + onRegenerate?: () => void; } /** @@ -22,7 +26,7 @@ interface GuideViewProps { * files the model didn't place anywhere (the server's coverage guarantee — * every changed file appears exactly once across sections + unplacedFiles). */ -export const GuideView: React.FC = ({ guide, reviewed, onToggleReviewed, engineLabel, focusedFile, onFocusFile }) => { +export const GuideView: React.FC = ({ guide, reviewed, onToggleReviewed, engineLabel, focusedFile, onFocusFile, onRegenerate }) => { const total = guide.sections.length; const reviewedCount = reviewed.filter(Boolean).length; const state = useReviewState(); @@ -56,11 +60,38 @@ export const GuideView: React.FC = ({ guide, reviewed, onToggleR {guide.intent && (

{renderInlineMarkdown(guide.intent)}

)} -

- {total} section{total !== 1 ? 's' : ''} - {reviewedCount > 0 && ` · ${reviewedCount}/${total} reviewed`} - {engineLabel && ` · generated by ${engineLabel}`} +

+ + {total} section{total !== 1 ? 's' : ''} + {reviewedCount > 0 && ` · ${reviewedCount}/${total} reviewed`} + {engineLabel && ` · generated by ${engineLabel}`} + + {guide.saved && ( + + Saved + + )}

+ {guide.moved && ( +

+ Generated on a different version of this branch + {onRegenerate && ( + <> + {' · '} + + + )} +

+ )}
diff --git a/packages/review-editor/hooks/guide/useGuideLaunch.ts b/packages/review-editor/hooks/guide/useGuideLaunch.ts new file mode 100644 index 0000000000..73832e505a --- /dev/null +++ b/packages/review-editor/hooks/guide/useGuideLaunch.ts @@ -0,0 +1,160 @@ +import type { AgentCapabilities } from '@plannotator/ui/types'; +import type { AgentLaunchParams } from '@plannotator/ui/hooks/useAgentJobs'; +import { useAgentSettings } from '@plannotator/ui/hooks/useAgentSettings'; +import type { ReviewEngine } from '@plannotator/ui/hooks/useAgentSettings'; +import { REVIEW_ENGINE_LABEL } from '@plannotator/ui/components/AgentsTab'; + +export const GUIDE_ENGINES = Object.keys(REVIEW_ENGINE_LABEL) as ReviewEngine[]; + +// Marker-engine fallbacks until the server delivers the live catalogs on the +// capability entries (mirrors AgentsTab's fallbacks). +const CURSOR_FALLBACK = [{ value: 'auto', label: 'Auto' }]; +const OPENCODE_FALLBACK = [{ value: '', label: 'Default' }]; +const PI_FALLBACK = [{ value: '', label: 'Default' }]; +const COPILOT_FALLBACK = [{ value: '', label: 'Default' }]; + +export type GuideModelOption = { value: string; label: string }; + +export interface GuideLaunchState { + /** The persisted agent settings bundle (pickers read/write through this). */ + settings: ReturnType; + guideAvailable: boolean; + availableEngines: ReviewEngine[]; + /** Effective engine: the persisted choice, snapped to an available one. */ + engine: ReviewEngine; + cursorOptions: GuideModelOption[]; + opencodeOptions: GuideModelOption[]; + piOptions: GuideModelOption[]; + copilotOptions: GuideModelOption[]; + effectiveCursorModel: string; + effectiveOpencodeModel: string; + effectivePiModel: string; + effectiveCopilotModel: string; + /** Whether a guide launch can be attempted at all on this machine. */ + canLaunch: boolean; + /** Build the launch params for the current effective engine + models. */ + buildParams: () => AgentLaunchParams; +} + +/** + * Shared guide-launch defaults: resolves the persisted engine/model settings + * against the live capability catalogs and builds the exact AgentLaunchParams + * shape the server expects (one shape per engine, mirroring AgentsTab's + * buildGuideLaunch). Used by GuideEmptyState's Generate button and by + * GuideView's "Regenerate" hint on an outdated saved guide, so both launch + * surfaces stay in lockstep. + */ +export function useGuideLaunch(capabilities: AgentCapabilities | null): GuideLaunchState { + const settings = useAgentSettings(); + const { + guideEngine, + guideClaudeModel, + guideClaudeEffort, + guideCodexModel, + guideCodexReasoning, + guideCursorModel, + guideOpencodeModel, + guidePiModel, + guidePiThinking, + guideCopilotModel, + } = settings; + + const providerAvailable = (id: string) => + capabilities?.providers.some((p) => p.id === id && p.available) ?? false; + const guideAvailable = providerAvailable('guide'); + const availableEngines = GUIDE_ENGINES.filter(providerAvailable); + // A persisted engine can be unavailable on this machine — fall back to the + // first available one rather than a dead selection. + const engine: ReviewEngine = providerAvailable(guideEngine) ? guideEngine : (availableEngines[0] ?? guideEngine); + + // Marker model catalogs are discovered server-side and delivered on the + // capability entry; fall back to the engine-default option until then. + // Mirrors AgentsTab's per-engine catalog semantics exactly: opencode/pi + // PREPEND their engine-managed "Default" ('' value) to the discovered list — + // the discovered catalogs are real models only, and dropping Default left a + // saved-default user with a blank pill and no way back after picking a + // concrete model. Cursor REPLACES: its discovered list natively includes + // 'auto', so prepending would duplicate it. + const markerModels = (id: 'cursor' | 'opencode' | 'pi' | 'copilot', fallback: GuideModelOption[]): GuideModelOption[] => { + const models = capabilities?.providers.find((p) => p.id === id)?.models; + if (!models || models.length === 0) return fallback; + const discovered = models.map((m) => ({ value: m.id, label: m.label })); + return id === 'cursor' ? discovered : [...fallback, ...discovered]; + }; + + const cursorOptions = markerModels('cursor', CURSOR_FALLBACK); + const opencodeOptions = markerModels('opencode', OPENCODE_FALLBACK); + const piOptions = markerModels('pi', PI_FALLBACK); + const copilotOptions = markerModels('copilot', COPILOT_FALLBACK); + + // A stale saved model id is reconciled at read time: if it's not in the + // current catalog, fall back to the catalog's first entry rather than + // POSTing a dead model id to the server. ('' / Default is present in the + // opencode/pi catalogs after the prepend above, so a saved '' stays valid.) + const effectiveModel = (saved: string, options: GuideModelOption[]): string => + options.some((o) => o.value === saved) ? saved : (options[0]?.value ?? saved); + + const effectiveCursorModel = effectiveModel(guideCursorModel, cursorOptions); + const effectiveOpencodeModel = effectiveModel(guideOpencodeModel, opencodeOptions); + const effectivePiModel = effectiveModel(guidePiModel, piOptions); + const effectiveCopilotModel = effectiveModel(guideCopilotModel, copilotOptions); + + // Config shapes mirror AgentsTab's buildGuideLaunch exactly — one shape + // per engine, so the server sees identical launches from every surface. + const buildParams = (): AgentLaunchParams => + engine === 'cursor' + ? { + provider: 'guide', + label: 'Guided Review', + engine: 'cursor', + ...(effectiveCursorModel && effectiveCursorModel.toLowerCase() !== 'auto' ? { model: effectiveCursorModel } : {}), + } + : engine === 'opencode' + ? { + provider: 'guide', + label: 'Guided Review', + engine: 'opencode', + ...(effectiveOpencodeModel ? { model: effectiveOpencodeModel } : {}), + } + : engine === 'pi' + ? { + provider: 'guide', + label: 'Guided Review', + engine: 'pi', + ...(effectivePiModel ? { model: effectivePiModel } : {}), + thinking: guidePiThinking, + } + : engine === 'copilot' + ? { + provider: 'guide', + label: 'Guided Review', + engine: 'copilot', + ...(effectiveCopilotModel ? { model: effectiveCopilotModel } : {}), + } + : { + provider: 'guide', + label: 'Guided Review', + engine, + model: engine === 'claude' ? guideClaudeModel : guideCodexModel, + ...(engine === 'claude' + ? { effort: guideClaudeEffort } + : { reasoningEffort: guideCodexReasoning }), + }; + + return { + settings, + guideAvailable, + availableEngines, + engine, + cursorOptions, + opencodeOptions, + piOptions, + copilotOptions, + effectiveCursorModel, + effectiveOpencodeModel, + effectivePiModel, + effectiveCopilotModel, + canLaunch: guideAvailable && availableEngines.length > 0, + buildParams, + }; +} diff --git a/packages/server/agent-jobs.ts b/packages/server/agent-jobs.ts index 8f65ef04a1..3fd8434a1c 100644 --- a/packages/server/agent-jobs.ts +++ b/packages/server/agent-jobs.ts @@ -128,6 +128,10 @@ export interface AgentJobHandlerOptions { * model planned section placement against, not whatever patch is on * screen when the job happens to finish. */ changedFilesSnapshot?: string[]; + /** Launch-time review-target snapshot (guide provider only) — stored on + * AgentJobInfo so guide persistence labels the envelope with the context + * the guide was generated against (see AgentJobInfo.guideContext). */ + guideContext?: AgentJobInfo["guideContext"]; } | null>; /** * Called after a job process exits with exit code 0. @@ -238,7 +242,7 @@ export function createAgentJobHandler(options: AgentJobHandlerOptions): AgentJob command: string[], label: string, outputPath?: string, - spawnOptions?: { captureStdout?: boolean; stdinPrompt?: string; cwd?: string; prompt?: string; engine?: string; model?: string; effort?: string; reasoningEffort?: string; fastMode?: boolean; thinking?: string; prUrl?: string; diffScope?: string; diffContext?: AgentJobInfo["diffContext"]; reviewProfileId?: string; reviewProfileLabel?: string; changedFilesSnapshot?: string[] }, + spawnOptions?: { captureStdout?: boolean; stdinPrompt?: string; cwd?: string; prompt?: string; engine?: string; model?: string; effort?: string; reasoningEffort?: string; fastMode?: boolean; thinking?: string; prUrl?: string; diffScope?: string; diffContext?: AgentJobInfo["diffContext"]; reviewProfileId?: string; reviewProfileLabel?: string; changedFilesSnapshot?: string[]; guideContext?: AgentJobInfo["guideContext"] }, ): AgentJobInfo { const source = jobSource(id); @@ -260,6 +264,7 @@ export function createAgentJobHandler(options: AgentJobHandlerOptions): AgentJob ...(spawnOptions?.prUrl && { prUrl: spawnOptions.prUrl }), ...(spawnOptions?.diffScope && { diffScope: spawnOptions.diffScope }), ...(spawnOptions?.diffContext && { diffContext: spawnOptions.diffContext }), + ...(spawnOptions?.guideContext && { guideContext: spawnOptions.guideContext }), ...(spawnOptions?.reviewProfileId && { reviewProfileId: spawnOptions.reviewProfileId }), ...(spawnOptions?.reviewProfileLabel && { reviewProfileLabel: spawnOptions.reviewProfileLabel }), }; @@ -664,6 +669,7 @@ export function createAgentJobHandler(options: AgentJobHandlerOptions): AgentJob let jobReviewProfileId: string | undefined; let jobReviewProfileLabel: string | undefined; let jobChangedFilesSnapshot: string[] | undefined; + let jobGuideContext: AgentJobInfo["guideContext"] | undefined; const jobId = crypto.randomUUID(); if (options.buildCommand) { // Thread config from POST body to buildCommand @@ -697,6 +703,7 @@ export function createAgentJobHandler(options: AgentJobHandlerOptions): AgentJob jobReviewProfileId = built.reviewProfileId; jobReviewProfileLabel = built.reviewProfileLabel; jobChangedFilesSnapshot = built.changedFilesSnapshot; + jobGuideContext = built.guideContext; } } @@ -724,6 +731,7 @@ export function createAgentJobHandler(options: AgentJobHandlerOptions): AgentJob reviewProfileId: jobReviewProfileId, reviewProfileLabel: jobReviewProfileLabel, changedFilesSnapshot: jobChangedFilesSnapshot, + guideContext: jobGuideContext, }); return Response.json({ job }, { status: 201 }); } catch (err) { diff --git a/packages/server/config.ts b/packages/server/config.ts index 6b4441ef00..263789efb7 100644 --- a/packages/server/config.ts +++ b/packages/server/config.ts @@ -5,6 +5,7 @@ export { getServerConfig, resolveAnnotateHistory, resolveCursorSandbox, + resolveGuideHistory, type PlannotatorConfig, type DiffOptions, } from "@plannotator/shared/config"; diff --git a/packages/server/guide-persistence.test.ts b/packages/server/guide-persistence.test.ts new file mode 100644 index 0000000000..36e529884d --- /dev/null +++ b/packages/server/guide-persistence.test.ts @@ -0,0 +1,204 @@ +/** + * Endpoint wiring for durable guide persistence (#1112), against BOTH server + * runtimes (Bun packages/server/review.ts and the Pi mirror + * apps/pi-extension/server/serverReview.ts): + * + * GET /api/guides — repo-scoped list + * GET /api/guide/saved:{id} — serve a persisted guide + * PUT /api/guide/saved:{id}/reviewed — persist reviewed state + * DELETE /api/guides/:id — remove a saved guide + * + * Both servers are started with no gitContext/PR, so the guide store derives + * its repo key via the no-remote fallback (process.cwd()) — the tests seed the + * store through @plannotator/shared/guide-store under that same key. + * + * Requires `bash apps/pi-extension/vendor.sh` to have been run (same as the + * other cross-runtime tests). + */ + +import { afterAll, afterEach, beforeAll, describe, expect, test } from "bun:test"; +import { mkdtempSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import type { CodeGuideOutput } from "@plannotator/shared/guide"; +import { + deriveGuideRepoKeyFallback, + listGuides, + loadGuide, + saveGuide, + type SavedGuideEnvelope, +} from "@plannotator/shared/guide-store"; +import { startReviewServer as startBunReviewServer } from "./review"; +import { startReviewServer as startPiReviewServer } from "../../apps/pi-extension/server"; + +const SPA_HTML = "SPA fallback"; + +const GUIDE: CodeGuideOutput = { + title: "Persisted guide", + intent: "Round-trips through the saved: endpoints.", + sections: [ + { title: "Core", overview: "The heart.", diffs: [{ file: "a.ts" }] }, + { title: "Glue", overview: "Wiring.", diffs: [{ file: "b.ts" }] }, + ], +}; + +function envelope(overrides: Partial = {}): SavedGuideEnvelope { + return { + version: 1, + savedAt: 1000, + label: "feature/x", + title: GUIDE.title, + guide: GUIDE, + reviewed: [false, false], + ...overrides, + }; +} + +interface RunningServer { + readonly url: string; + stop(): void; +} + +const serverCases = [ + { + name: "Bun review", + start: () => + startBunReviewServer({ + rawPatch: "", + gitRef: "HEAD", + origin: "claude-code", + htmlContent: SPA_HTML, + }), + }, + { + name: "Pi review", + start: () => + startPiReviewServer({ + rawPatch: "", + gitRef: "HEAD", + origin: "pi", + htmlContent: SPA_HTML, + }), + }, +] as const; + +let dataDir = ""; +let previousDataDir: string | undefined; +let previousPort: string | undefined; +let previousRemote: string | undefined; +// Both servers run with no gitContext/PR/workspace, so the session's repo key +// is the fallback derivation over process.cwd(). +const repoKey = deriveGuideRepoKeyFallback(process.cwd()); + +beforeAll(() => { + previousPort = process.env.PLANNOTATOR_PORT; + previousRemote = process.env.PLANNOTATOR_REMOTE; + delete process.env.PLANNOTATOR_PORT; + process.env.PLANNOTATOR_REMOTE = "0"; +}); + +afterAll(() => { + if (previousPort === undefined) delete process.env.PLANNOTATOR_PORT; + else process.env.PLANNOTATOR_PORT = previousPort; + if (previousRemote === undefined) delete process.env.PLANNOTATOR_REMOTE; + else process.env.PLANNOTATOR_REMOTE = previousRemote; +}); + +// Fresh data dir per test so seeded guides never leak across cases. +function useTempDataDir() { + dataDir = mkdtempSync(join(tmpdir(), "plannotator-guide-endpoints-")); + previousDataDir = process.env.PLANNOTATOR_DATA_DIR; + process.env.PLANNOTATOR_DATA_DIR = dataDir; +} + +afterEach(() => { + if (previousDataDir === undefined) delete process.env.PLANNOTATOR_DATA_DIR; + else process.env.PLANNOTATOR_DATA_DIR = previousDataDir; + if (dataDir) rmSync(dataDir, { recursive: true, force: true }); + dataDir = ""; +}); + +for (const serverCase of serverCases) { + describe(`${serverCase.name} guide persistence endpoints`, () => { + test("lists, serves, updates reviewed (persisted across restart), and deletes saved guides", async () => { + useTempDataDir(); + saveGuide(repoKey, "1000-persisted-guide", envelope()); + + let server = await serverCase.start(); + try { + // GET /api/guides — the seeded guide is listed with progress + moved. + const listRes = await fetch(`${server.url}/api/guides`); + expect(listRes.status).toBe(200); + const list = await listRes.json() as Array>; + expect(list.length).toBe(1); + expect(list[0].id).toBe("1000-persisted-guide"); + expect(list[0].label).toBe("feature/x"); + expect(list[0].title).toBe(GUIDE.title); + expect(list[0].progress).toEqual({ reviewed: 0, total: 2 }); + expect(list[0].moved).toBe(false); // no headSha stored → never flagged + + // GET /api/guide/saved:{id} — full guide + reviewed + saved/moved. + const getRes = await fetch(`${server.url}/api/guide/saved:1000-persisted-guide`); + expect(getRes.status).toBe(200); + const data = await getRes.json() as Record; + expect(data.title).toBe(GUIDE.title); + expect((data.sections as unknown[]).length).toBe(2); + expect(data.reviewed).toEqual([false, false]); + expect(data.saved).toBe(true); + expect(data.moved).toBe(false); + + // Unknown saved id → 404. + const missingRes = await fetch(`${server.url}/api/guide/saved:2000-missing`); + expect(missingRes.status).toBe(404); + + // PUT /api/guide/saved:{id}/reviewed persists to disk. + const putRes = await fetch(`${server.url}/api/guide/saved:1000-persisted-guide/reviewed`, { + method: "PUT", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ reviewed: [true, false] }), + }); + expect(putRes.status).toBe(200); + expect(loadGuide(repoKey, "1000-persisted-guide")!.reviewed).toEqual([true, false]); + } finally { + server.stop(); + } + + // Server restart — the reviewed state survives on the new session. + server = await serverCase.start(); + try { + const data = await (await fetch(`${server.url}/api/guide/saved:1000-persisted-guide`)).json() as Record; + expect(data.reviewed).toEqual([true, false]); + + // DELETE /api/guides/:id removes it; a repeat delete 404s. + const delRes = await fetch(`${server.url}/api/guides/1000-persisted-guide`, { method: "DELETE" }); + expect(delRes.status).toBe(200); + expect(listGuides(repoKey).length).toBe(0); + const delAgain = await fetch(`${server.url}/api/guides/1000-persisted-guide`, { method: "DELETE" }); + expect(delAgain.status).toBe(404); + const listAfter = await (await fetch(`${server.url}/api/guides`)).json() as unknown[]; + expect(listAfter).toEqual([]); + } finally { + server.stop(); + } + }); + + test("traversal-shaped ids are rejected and corrupt files load as no guide", async () => { + useTempDataDir(); + const server = await serverCase.start(); + try { + // Traversal-shaped ids never reach the disk layer. + const evil = await fetch(`${server.url}/api/guide/saved:..%2F..%2Fescape`); + expect(evil.status).toBe(404); + const evilDelete = await fetch(`${server.url}/api/guides/..%2Fescape`, { method: "DELETE" }); + expect(evilDelete.status).toBe(404); + + // A live (non-saved) job id still routes to the in-memory session. + const live = await fetch(`${server.url}/api/guide/some-live-job-id`); + expect(live.status).toBe(404); + expect(await live.json()).toEqual({ error: "Guide not found" }); + } finally { + server.stop(); + } + }); + }); +} diff --git a/packages/server/review.ts b/packages/server/review.ts index d48519d3b7..9a69dd0423 100644 --- a/packages/server/review.ts +++ b/packages/server/review.ts @@ -81,6 +81,7 @@ import { } from "./claude-review"; import { createTourSession, TOUR_EMPTY_OUTPUT_ERROR } from "./tour/tour-review"; import { createGuideSession, GUIDE_EMPTY_OUTPUT_ERROR } from "./guide/guide-review"; +import { createGuideStoreSession, SAVED_GUIDE_ID_PREFIX } from "@plannotator/shared/guide-store"; import { MARKER_ENGINES, composeMarkerReviewPrompt, @@ -92,7 +93,7 @@ import { extractMarkerNonce, type MarkerEngineId, } from "./marker-review"; -import { loadConfig, saveConfig, detectGitUser, getServerConfig, resolveCursorSandbox } from "./config"; +import { loadConfig, saveConfig, detectGitUser, getServerConfig, resolveCursorSandbox, resolveGuideHistory } from "./config"; import { type PRMetadata, type PRRef, type PRReviewFileComment, type PRStackTree, type PRListItem, fetchPR, fetchPRFileContent, fetchPRContext, submitPRReview, fetchPRViewedFiles, markPRFilesViewed, fetchPRStack, fetchPRList, getPRUser, parsePRUrl, prRefFromMetadata, isSameProject, getDisplayRepo, getMRLabel, getMRNumberLabel, prCommandRuntime } from "./pr"; import { PR_CONTEXT_HEARTBEAT_COMMENT, @@ -227,6 +228,33 @@ export async function startReviewServer( const tour = createTourSession(); const guide = createGuideSession(); + // Durable guide persistence (#1112): autosaves validated guides to + // ${PLANNOTATOR_DATA_DIR}/guides/{repo-key}/ and serves them back through + // the existing guide endpoints as `saved:{id}` pseudo job ids. All getters + // are late-bound — prMetadata/currentDiffType can change mid-session. + const guideStore = createGuideStoreSession({ + runGit: async (args, cwd) => { + const result = await gitRuntime.runGit(args, { cwd }); + return result.exitCode === 0 ? result.stdout : null; + }, + getGitCwd: () => + isPRMode || isWorkspaceMode + ? undefined + : gitContext + ? resolveVcsCwd(currentDiffType as DiffType, gitContext.cwd) ?? gitContext.cwd ?? process.cwd() + : undefined, + getPRInfo: () => + prMetadata + ? { + url: prMetadata.url, + headSha: prMetadata.headSha, + label: `${getMRLabel(prMetadata)} ${getMRNumberLabel(prMetadata)}`, + } + : null, + getBranchLabel: () => clientGitContext?.currentBranch || gitContext?.currentBranch, + getFallbackDir: () => workspace?.root ?? options.agentCwd ?? process.cwd(), + writesEnabled: () => resolveGuideHistory(loadConfig()), + }); // Mutable state for diff switching let currentPatch = options.rawPatch; @@ -1010,6 +1038,15 @@ export async function startReviewServer( const changedFilesSnapshot = repairOf ? guide.getLaunchChangedFiles(repairOf) ?? changedFiles.map((f) => f.path) : changedFiles.map((f) => f.path); + // Snapshot the launch-time review-target context (#1112): guide jobs + // run for minutes while the session supports mid-generation PR/diff + // switching, so the persisted envelope must be labeled with the + // context this guide is GENERATED against — captured now, carried on + // the job (guideContext), and read back at completion instead of the + // live session state. Repairs reuse the FAILED job's own snapshot, + // same as changedFilesSnapshot above. + const guideContext = (repairOf ? agentJobs.getJob(repairOf)?.guideContext : undefined) + ?? await guideStore.captureLaunchContext(); return { ...built, prUrl: launchPrUrl, @@ -1018,6 +1055,7 @@ export async function startReviewServer( reviewProfileId: reviewProfile.id, reviewProfileLabel: reviewProfile.label, changedFilesSnapshot, + guideContext, }; } @@ -1261,6 +1299,13 @@ export async function startReviewServer( const { summary, error } = await guide.onJobComplete({ job, meta, changedFiles }); if (summary) { job.summary = summary; + // Autosave (#1112): only guides that passed validateGuideOutput ever + // reach guideResults, so a getGuide hit here IS the validation gate. + // Failed/invalid guides never write. The job's launch-time context + // snapshot labels the envelope — never the live session state, which + // may have PR/diff-switched while the job ran. + const validated = guide.getGuide(job.id); + if (validated) await guideStore.saveForJob(job, validated, job.guideContext); } else { // Same fail-closed precedent as Tour: an exit-0 job with empty, // malformed, or fully-invalidated output must not look like a @@ -1386,27 +1431,55 @@ export async function startReviewServer( } } - // API: Get guide result + // API: Get guide result — live job ids, or `saved:{id}` for a + // persisted guide loaded from the on-disk store (#1112). if (url.pathname.match(/^\/api\/guide\/[^/]+$/) && req.method === "GET") { - const jobId = url.pathname.slice("/api/guide/".length); + const jobId = decodeURIComponent(url.pathname.slice("/api/guide/".length)); + if (jobId.startsWith(SAVED_GUIDE_ID_PREFIX)) { + const saved = await guideStore.getSavedGuideData(jobId.slice(SAVED_GUIDE_ID_PREFIX.length)); + if (!saved) return Response.json({ error: "Guide not found" }, { status: 404 }); + return Response.json(saved); + } const result = guide.getGuide(jobId); if (!result) return Response.json({ error: "Guide not found" }, { status: 404 }); - return Response.json(result); + return Response.json({ ...result, ...(guideStore.isJobSaved(jobId) ? { saved: true } : {}) }); } - // API: Save guide reviewed state + // API: Save guide reviewed state. Live job ids also write through to + // the job's autosaved file; `saved:{id}` ids persist directly. const reviewedMatch = url.pathname.match(/^\/api\/guide\/([^/]+)\/reviewed$/); if (reviewedMatch && req.method === "PUT") { - const jobId = reviewedMatch[1]; + const jobId = decodeURIComponent(reviewedMatch[1]); try { const body = await req.json() as { reviewed: boolean[] }; - if (Array.isArray(body.reviewed)) guide.saveReviewed(jobId, body.reviewed); + if (Array.isArray(body.reviewed)) { + if (jobId.startsWith(SAVED_GUIDE_ID_PREFIX)) { + const ok = await guideStore.updateSavedReviewed(jobId.slice(SAVED_GUIDE_ID_PREFIX.length), body.reviewed); + if (!ok) return Response.json({ error: "Guide not found" }, { status: 404 }); + } else { + guide.saveReviewed(jobId, body.reviewed); + await guideStore.writeThroughReviewed(jobId, body.reviewed); + } + } return Response.json({ ok: true }); } catch { return Response.json({ error: "Invalid JSON" }, { status: 400 }); } } + // API: List saved guides for the current repo (#1112) + if (url.pathname === "/api/guides" && req.method === "GET") { + return Response.json(await guideStore.listSaved()); + } + + // API: Delete a saved guide (#1112) + const savedGuideDeleteMatch = url.pathname.match(/^\/api\/guides\/([^/]+)$/); + if (savedGuideDeleteMatch && req.method === "DELETE") { + const ok = await guideStore.deleteSaved(decodeURIComponent(savedGuideDeleteMatch[1])); + if (!ok) return Response.json({ error: "Guide not found" }, { status: 404 }); + return Response.json({ ok: true }); + } + // API: Get a failed guide job's captured raw output for manual repair const guideOutputMatch = url.pathname.match(/^\/api\/guide\/([^/]+)\/output$/); if (guideOutputMatch && req.method === "GET") { @@ -1440,6 +1513,11 @@ export async function startReviewServer( explanation: `${sections} section${sections !== 1 ? "s" : ""}, ${files} file${files !== 1 ? "s" : ""} placed (manually repaired)`, confidence: 1, }); + // A manually repaired guide passed the same validateGuideOutput + // gate as an automatic one — persist it too (#1112), labeled + // with the job's own launch-time context snapshot. + const repaired = guide.getGuide(jobId); + if (repaired) await guideStore.saveForJob(existingJob, repaired, existingJob.guideContext); return Response.json({ ok: true, sections, files }); } catch { return Response.json({ error: "Invalid JSON" }, { status: 400 }); diff --git a/packages/shared/config.test.ts b/packages/shared/config.test.ts index e9c9457412..76ee11dc87 100644 --- a/packages/shared/config.test.ts +++ b/packages/shared/config.test.ts @@ -3,6 +3,7 @@ import { resolveCursorSandbox, resolveUseGlimpse, resolveAnnotateHistory, + resolveGuideHistory, resolveUseJina, } from "./config"; import type { PlannotatorConfig } from "./config"; @@ -74,6 +75,12 @@ describe("config.json boolean coercion", () => { key: "annotateHistory", resolve: resolveAnnotateHistory, }, + { + name: "resolveGuideHistory", + envVar: "PLANNOTATOR_GUIDE_HISTORY", + key: "guideHistory", + resolve: resolveGuideHistory, + }, { name: "resolveUseJina", envVar: "PLANNOTATOR_JINA", diff --git a/packages/shared/config.ts b/packages/shared/config.ts index ebb9f22357..689462897b 100644 --- a/packages/shared/config.ts +++ b/packages/shared/config.ts @@ -113,6 +113,13 @@ export interface PlannotatorConfig { * annotate sessions fully stateless. Default: true. */ annotateHistory?: boolean; + /** + * Persist successful Guided Reviews (guide content + per-section reviewed + * state) under ~/.plannotator/guides/ (or PLANNOTATOR_DATA_DIR) so they + * survive closing Plannotator. Set to false to disable writes; already-saved + * guides remain readable and listed. Default: true. + */ + guideHistory?: boolean; /** * Inject a Plannotator Flavored Markdown reminder into every EnterPlanMode * call so the agent is aware it can enrich plans with code-file links, @@ -284,6 +291,20 @@ export function resolveAnnotateHistory(config: PlannotatorConfig): boolean { return coerceConfigBoolean(config.annotateHistory, true); } +/** + * Resolve whether successful Guided Reviews are persisted to disk. + * + * Priority (highest wins): + * PLANNOTATOR_GUIDE_HISTORY env var → config.guideHistory → default true + */ +export function resolveGuideHistory(config: PlannotatorConfig): boolean { + const envVal = process.env.PLANNOTATOR_GUIDE_HISTORY; + if (envVal !== undefined) { + return envVal === "1" || envVal.toLowerCase() === "true"; + } + return coerceConfigBoolean(config.guideHistory, true); +} + export function resolveUseJina(cliNoJina: boolean, config: PlannotatorConfig): boolean { // CLI flag has highest priority if (cliNoJina) return false; diff --git a/packages/shared/guide-store.test.ts b/packages/shared/guide-store.test.ts new file mode 100644 index 0000000000..9202f51496 --- /dev/null +++ b/packages/shared/guide-store.test.ts @@ -0,0 +1,407 @@ +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { mkdtempSync, readdirSync, readFileSync, rmSync, writeFileSync, mkdirSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import type { CodeGuideOutput } from "./guide"; +import { + createGuideStoreSession, + deleteGuide, + deriveGuideRepoKeyFallback, + deriveGuideRepoKeyFromPRUrl, + deriveGuideRepoKeyFromRemote, + isValidGuideId, + listGuides, + loadGuide, + makeGuideId, + saveGuide, + updateGuideReviewed, + type SavedGuideEnvelope, +} from "./guide-store"; + +const REPO_KEY = "github.com__acme__widgets"; + +const GUIDE: CodeGuideOutput = { + title: "Add payment localization", + intent: "Localizes checkout for three new markets.", + sections: [ + { title: "Locale module", overview: "The core change.", diffs: [{ file: "src/locale.ts" }] }, + { title: "Wiring", overview: "Glue.", diffs: [{ file: "src/index.ts" }] }, + ], +}; + +function envelope(overrides: Partial = {}): SavedGuideEnvelope { + return { + version: 1, + savedAt: 1000, + label: "feature/locales", + title: GUIDE.title, + engine: "claude", + headSha: "abc1234", + guide: GUIDE, + reviewed: [false, false], + ...overrides, + }; +} + +let dataDir = ""; +let previousDataDir: string | undefined; + +beforeEach(() => { + dataDir = mkdtempSync(join(tmpdir(), "plannotator-guide-store-")); + previousDataDir = process.env.PLANNOTATOR_DATA_DIR; + process.env.PLANNOTATOR_DATA_DIR = dataDir; +}); + +afterEach(() => { + if (previousDataDir === undefined) delete process.env.PLANNOTATOR_DATA_DIR; + else process.env.PLANNOTATOR_DATA_DIR = previousDataDir; + rmSync(dataDir, { recursive: true, force: true }); +}); + +describe("repo key derivation", () => { + test("SSH, HTTPS, and ssh:// remotes land on the same host__owner__repo key", () => { + const expected = "github.com__acme__widgets"; + expect(deriveGuideRepoKeyFromRemote("git@github.com:acme/widgets.git")).toBe(expected); + expect(deriveGuideRepoKeyFromRemote("https://github.com/acme/widgets.git")).toBe(expected); + expect(deriveGuideRepoKeyFromRemote("https://github.com/acme/widgets")).toBe(expected); + expect(deriveGuideRepoKeyFromRemote("ssh://git@github.com:22/acme/widgets.git")).toBe(expected); + }); + + test("GitLab subgroup paths keep every segment", () => { + expect(deriveGuideRepoKeyFromRemote("git@gitlab.com:group/sub/proj.git")).toBe( + "gitlab.com__group__sub__proj", + ); + }); + + test("PR urls land on the same key as the matching origin remote", () => { + expect(deriveGuideRepoKeyFromPRUrl("https://github.com/acme/widgets/pull/12")).toBe( + deriveGuideRepoKeyFromRemote("git@github.com:acme/widgets.git"), + ); + expect(deriveGuideRepoKeyFromPRUrl("https://gitlab.com/group/sub/proj/-/merge_requests/3")).toBe( + deriveGuideRepoKeyFromRemote("git@gitlab.com:group/sub/proj.git"), + ); + }); + + test("unparseable remotes return null", () => { + expect(deriveGuideRepoKeyFromRemote("")).toBeNull(); + expect(deriveGuideRepoKeyFromRemote("not a url")).toBeNull(); + expect(deriveGuideRepoKeyFromPRUrl("https://example.com/not-a-pr")).toBeNull(); + }); + + test("no-remote fallback is dir name + path hash, stable per path, distinct across paths", () => { + const a = deriveGuideRepoKeyFallback("/tmp/projects/widgets"); + const b = deriveGuideRepoKeyFallback("/tmp/elsewhere/widgets"); + expect(a).toMatch(/^widgets-[0-9a-f]{8}$/); + expect(a).toBe(deriveGuideRepoKeyFallback("/tmp/projects/widgets")); + expect(a).not.toBe(b); + }); +}); + +describe("guide ids", () => { + test("makeGuideId is timestamp + title slug", () => { + expect(makeGuideId("Add payment localization", 1234)).toBe("1234-add-payment-localization"); + expect(makeGuideId("###", 1234)).toBe("1234-guide"); + }); + + test("isValidGuideId rejects traversal and separator characters", () => { + expect(isValidGuideId("1234-add-payment")).toBe(true); + expect(isValidGuideId("../escape")).toBe(false); + expect(isValidGuideId("a/b")).toBe(false); + expect(isValidGuideId("a\\b")).toBe(false); + expect(isValidGuideId(".hidden")).toBe(false); + expect(isValidGuideId("")).toBe(false); + }); +}); + +describe("save / load / list / delete", () => { + test("round-trips an envelope", () => { + expect(saveGuide(REPO_KEY, "1000-test", envelope())).toBe(true); + const loaded = loadGuide(REPO_KEY, "1000-test"); + expect(loaded).not.toBeNull(); + expect(loaded!.title).toBe(GUIDE.title); + expect(loaded!.guide.sections.length).toBe(2); + expect(loaded!.reviewed).toEqual([false, false]); + }); + + test("atomic write leaves no .tmp file behind", () => { + saveGuide(REPO_KEY, "1000-test", envelope()); + const files = readdirSync(join(dataDir, "guides", REPO_KEY)); + expect(files).toEqual(["1000-test.json"]); + }); + + test("atomic replace: updateGuideReviewed rewrites the whole file consistently", () => { + saveGuide(REPO_KEY, "1000-test", envelope()); + expect(updateGuideReviewed(REPO_KEY, "1000-test", [true, false])).toBe(true); + const loaded = loadGuide(REPO_KEY, "1000-test"); + expect(loaded!.reviewed).toEqual([true, false]); + // Untouched fields survive the rewrite. + expect(loaded!.headSha).toBe("abc1234"); + expect(loaded!.label).toBe("feature/locales"); + // Still exactly one file on disk — no stray tmp artifacts. + expect(readdirSync(join(dataDir, "guides", REPO_KEY))).toEqual(["1000-test.json"]); + }); + + test("reviewed arrays are clamped to the section count", () => { + saveGuide(REPO_KEY, "1000-test", envelope()); + updateGuideReviewed(REPO_KEY, "1000-test", [true, true, true, true]); + expect(loadGuide(REPO_KEY, "1000-test")!.reviewed).toEqual([true, true]); + }); + + test("corrupt or invalid files load as no saved guide and are skipped by list", () => { + const dir = join(dataDir, "guides", REPO_KEY); + mkdirSync(dir, { recursive: true }); + writeFileSync(join(dir, "1-corrupt.json"), "{ not json", "utf-8"); + writeFileSync(join(dir, "2-wrong-version.json"), JSON.stringify({ version: 2, guide: GUIDE }), "utf-8"); + writeFileSync(join(dir, "3-no-sections.json"), JSON.stringify(envelope({ guide: { ...GUIDE, sections: [] } })), "utf-8"); + saveGuide(REPO_KEY, "4-good", envelope()); + + expect(loadGuide(REPO_KEY, "1-corrupt")).toBeNull(); + expect(loadGuide(REPO_KEY, "2-wrong-version")).toBeNull(); + expect(loadGuide(REPO_KEY, "3-no-sections")).toBeNull(); + expect(listGuides(REPO_KEY).map((g) => g.id)).toEqual(["4-good"]); + }); + + test("list sorts newest first and delete removes exactly one guide", () => { + saveGuide(REPO_KEY, "1-old", envelope({ savedAt: 1 })); + saveGuide(REPO_KEY, "2-new", envelope({ savedAt: 2 })); + expect(listGuides(REPO_KEY).map((g) => g.id)).toEqual(["2-new", "1-old"]); + + expect(deleteGuide(REPO_KEY, "2-new")).toBe(true); + expect(deleteGuide(REPO_KEY, "2-new")).toBe(false); // already gone + expect(listGuides(REPO_KEY).map((g) => g.id)).toEqual(["1-old"]); + }); + + test("guides are scoped per repo key", () => { + saveGuide("github.com__acme__widgets", "1-a", envelope()); + saveGuide("github.com__acme__gadgets", "1-b", envelope()); + expect(listGuides("github.com__acme__widgets").map((g) => g.id)).toEqual(["1-a"]); + expect(listGuides("github.com__acme__gadgets").map((g) => g.id)).toEqual(["1-b"]); + }); + + test("invalid ids are rejected without touching the disk", () => { + expect(saveGuide(REPO_KEY, "../escape", envelope())).toBe(false); + expect(loadGuide(REPO_KEY, "../escape")).toBeNull(); + expect(deleteGuide(REPO_KEY, "../escape")).toBe(false); + }); +}); + +describe("createGuideStoreSession", () => { + const branchSession = (overrides: Partial[0]> = {}) => + createGuideStoreSession({ + runGit: async (args) => { + if (args[0] === "remote") return "git@github.com:acme/widgets.git\n"; + if (args[0] === "rev-parse" && args[1] === "HEAD") return "abc1234\n"; + return null; + }, + getGitCwd: () => "/repo", + getPRInfo: () => null, + getBranchLabel: () => "feature/locales", + getFallbackDir: () => "/repo", + writesEnabled: () => true, + ...overrides, + }); + + test("saveForJob persists under the remote-derived repo key and lists back", async () => { + const session = branchSession(); + await session.saveForJob({ id: "job-1", engine: "claude", model: "sonnet" }, { ...GUIDE, reviewed: [] }); + + expect(session.isJobSaved("job-1")).toBe(true); + const entries = listGuides(REPO_KEY); + expect(entries.length).toBe(1); + expect(entries[0].envelope.label).toBe("feature/locales"); + expect(entries[0].envelope.headSha).toBe("abc1234"); + expect(entries[0].envelope.engine).toBe("claude"); + + const listed = await session.listSaved(); + expect(listed.length).toBe(1); + expect(listed[0].progress).toEqual({ reviewed: 0, total: 2 }); + expect(listed[0].moved).toBe(false); // head still matches + }); + + test("moved flags a guide whose stored head differs from the current head", async () => { + const session = branchSession(); + await session.saveForJob({ id: "job-1" }, { ...GUIDE }); + + let head = "abc1234"; + const later = branchSession({ + runGit: async (args) => { + if (args[0] === "remote") return "git@github.com:acme/widgets.git\n"; + if (args[0] === "rev-parse" && args[1] === "HEAD") return `${head}\n`; + return null; + }, + }); + expect((await later.listSaved())[0].moved).toBe(false); + + head = "def4567"; + const moved = branchSession({ + runGit: async (args) => { + if (args[0] === "remote") return "git@github.com:acme/widgets.git\n"; + if (args[0] === "rev-parse" && args[1] === "HEAD") return `${head}\n`; + return null; + }, + }); + const rows = await moved.listSaved(); + expect(rows[0].moved).toBe(true); + const data = await moved.getSavedGuideData(rows[0].id); + expect(data!.moved).toBe(true); + expect(data!.saved).toBe(true); + }); + + test("reviewed write-through updates a live job's saved file", async () => { + const session = branchSession(); + await session.saveForJob({ id: "job-1" }, { ...GUIDE }); + await session.writeThroughReviewed("job-1", [true, false]); + + const rows = await session.listSaved(); + expect(rows[0].progress).toEqual({ reviewed: 1, total: 2 }); + + // A fresh session (server restart) still sees the persisted state. + const restarted = branchSession(); + const restartedRows = await restarted.listSaved(); + expect(restartedRows[0].progress).toEqual({ reviewed: 1, total: 2 }); + }); + + test("updateSavedReviewed persists and getSavedGuideData reflects it", async () => { + const session = branchSession(); + await session.saveForJob({ id: "job-1" }, { ...GUIDE }); + const [row] = await session.listSaved(); + expect(await session.updateSavedReviewed(row.id, [true, true])).toBe(true); + expect((await session.getSavedGuideData(row.id))!.reviewed).toEqual([true, true]); + expect(await session.updateSavedReviewed("missing-id", [true])).toBe(false); + }); + + test("opt-out blocks autosave writes while reads keep working", async () => { + // Seed a guide while enabled. + const enabled = branchSession(); + await enabled.saveForJob({ id: "job-1" }, { ...GUIDE }); + + const disabled = branchSession({ writesEnabled: () => false }); + await disabled.saveForJob({ id: "job-2" }, { ...GUIDE, title: "Should not persist" }); + + expect(disabled.isJobSaved("job-2")).toBe(false); + const rows = await disabled.listSaved(); + expect(rows.length).toBe(1); // only the pre-existing guide + expect(rows[0].title).toBe(GUIDE.title); + expect((await disabled.getSavedGuideData(rows[0].id))!.title).toBe(GUIDE.title); + }); + + test("PR mode derives the key from the PR url and stamps prUrl + PR head", async () => { + const session = createGuideStoreSession({ + getGitCwd: () => undefined, + getPRInfo: () => ({ url: "https://github.com/acme/widgets/pull/7", headSha: "pr-head-1", label: "PR #7" }), + getBranchLabel: () => undefined, + getFallbackDir: () => "/unused", + writesEnabled: () => true, + }); + await session.saveForJob({ id: "job-1", engine: "codex" }, { ...GUIDE }); + + // Same shelf as the branch-mode key for the matching origin remote. + const entries = listGuides(REPO_KEY); + expect(entries.length).toBe(1); + expect(entries[0].envelope.prUrl).toBe("https://github.com/acme/widgets/pull/7"); + expect(entries[0].envelope.headSha).toBe("pr-head-1"); + expect(entries[0].envelope.label).toBe("PR #7"); + }); + + test("no-remote session falls back to the fallback-dir key", async () => { + const session = createGuideStoreSession({ + runGit: async () => null, // no origin, not a repo + getGitCwd: () => "/some/where", + getPRInfo: () => null, + getBranchLabel: () => undefined, + getFallbackDir: () => "/some/where", + writesEnabled: () => true, + }); + await session.saveForJob({ id: "job-1" }, { ...GUIDE }); + + const key = deriveGuideRepoKeyFallback("/some/where"); + expect(listGuides(key).length).toBe(1); + // Label falls back to "local" when no branch is known. + expect(listGuides(key)[0].envelope.label).toBe("local"); + }); + + test("a job launched under PR A completing after a switch to PR B keeps A's label, url, headSha, and shelf", async () => { + // Mutable live state: what the session getters return at any moment — + // exactly what the servers wire in. The launch snapshot is captured while + // PR A is active; the "user" then switches to PR B before the multi-minute + // job completes and saveForJob runs against the live-getters-say-B state. + let livePR: { url: string; headSha: string; label: string } | null = { + url: "https://github.com/acme/widgets/pull/1", + headSha: "aaaaaaa1", + label: "PR #1", + }; + const session = createGuideStoreSession({ + getGitCwd: () => undefined, + getPRInfo: () => livePR, + getBranchLabel: () => undefined, + getFallbackDir: () => "/unused", + writesEnabled: () => true, + }); + + const launchContext = await session.captureLaunchContext(); + expect(launchContext.pr?.url).toBe("https://github.com/acme/widgets/pull/1"); + + // Simulate /api/pr-switch to a DIFFERENT PR (different repo, even) while + // the job runs. + livePR = { url: "https://github.com/other/gadgets/pull/9", headSha: "bbbbbbb2", label: "PR #9" }; + + await session.saveForJob({ id: "job-1", engine: "claude" }, { ...GUIDE }, launchContext); + + // The envelope is labeled with PR A — the context the guide was GENERATED + // against — and filed under A's repository shelf, not B's. + const entries = listGuides(REPO_KEY); + expect(entries.length).toBe(1); + expect(entries[0].envelope.label).toBe("PR #1"); + expect(entries[0].envelope.prUrl).toBe("https://github.com/acme/widgets/pull/1"); + expect(entries[0].envelope.headSha).toBe("aaaaaaa1"); + expect(listGuides("github.com__other__gadgets")).toEqual([]); + + // Reviewed write-through follows the launch-time shelf too, even though + // the live session is still pointed at PR B. + await session.writeThroughReviewed("job-1", [true, false]); + expect(listGuides(REPO_KEY)[0].envelope.reviewed).toEqual([true, false]); + }); + + test("branch-mode launch context survives a mid-run branch/head change", async () => { + let head = "abc1234"; + let branch = "feature/locales"; + const session = branchSession({ + runGit: async (args) => { + if (args[0] === "remote") return "git@github.com:acme/widgets.git\n"; + if (args[0] === "rev-parse" && args[1] === "HEAD") return `${head}\n`; + return null; + }, + getBranchLabel: () => branch, + }); + + const launchContext = await session.captureLaunchContext(); + head = "def4567"; + branch = "hotfix/other"; + + await session.saveForJob({ id: "job-1" }, { ...GUIDE }, launchContext); + const [entry] = listGuides(REPO_KEY); + expect(entry.envelope.label).toBe("feature/locales"); + expect(entry.envelope.headSha).toBe("abc1234"); + // The moved flag now reflects that the branch has advanced past the + // launch-time head the guide was generated on. + expect((await session.listSaved())[0].moved).toBe(true); + }); + + test("saveForJob without a launch snapshot falls back to the live getters", async () => { + const session = branchSession(); + await session.saveForJob({ id: "job-1" }, { ...GUIDE }); + const [entry] = listGuides(REPO_KEY); + expect(entry.envelope.label).toBe("feature/locales"); + expect(entry.envelope.headSha).toBe("abc1234"); + }); + + test("saving twice for the same job overwrites the same file", async () => { + const session = branchSession(); + await session.saveForJob({ id: "job-1" }, { ...GUIDE }); + await session.saveForJob({ id: "job-1" }, { ...GUIDE, intent: "Updated intent" }); + + const entries = listGuides(REPO_KEY); + expect(entries.length).toBe(1); + expect(entries[0].envelope.guide.intent).toBe("Updated intent"); + }); +}); diff --git a/packages/shared/guide-store.ts b/packages/shared/guide-store.ts new file mode 100644 index 0000000000..91489c55d0 --- /dev/null +++ b/packages/shared/guide-store.ts @@ -0,0 +1,467 @@ +/** + * Guide Store + * + * Persists successful Guided Reviews to disk so they survive server restarts + * (issue #1112). Files live at: + * + * ${PLANNOTATOR_DATA_DIR}/guides/{repo-key}/{id}.json + * + * repo-key is a sanitized `host__owner__repo` derived from the repository's + * origin remote (or the PR url in PR mode), so a PR review and a local review + * of the same repository share the same shelf while same-named branches in + * different repositories never collide. When no parseable remote exists, the + * key falls back to `{dir-name}-{hash8-of-resolved-path}` — the same pattern + * annotate mode uses for its per-file history slugs. + * + * Runtime-agnostic (node:fs / node:path / node:crypto only), following the + * storage.ts / draft.ts patterns. Vendored to Pi via vendor.sh. Corrupt or + * unreadable files load as "no saved guide" (the review-skill-loader + * skip-and-log discipline); writes are atomic (tmp + rename). + */ + +import { join, resolve, basename } from "path"; +import { mkdirSync, writeFileSync, readFileSync, readdirSync, renameSync, unlinkSync, existsSync } from "fs"; +import { createHash } from "crypto"; +import { getPlannotatorDataDir } from "./data-dir"; +import { parseRemoteUrl, parseRemoteHost } from "./repo"; +import { parsePRUrl } from "./pr-types"; +import type { CodeGuideOutput, SavedGuideListEntry } from "./guide"; + +export type { SavedGuideListEntry }; + +/** Client-facing pseudo job-id prefix for persisted guides ("saved:{id}"). */ +export const SAVED_GUIDE_ID_PREFIX = "saved:"; + +/** On-disk envelope for one persisted guide. */ +export interface SavedGuideEnvelope { + version: 1; + /** Epoch ms when the guide was first persisted. */ + savedAt: number; + /** Review-target label shown in the list — "PR #1082" or the branch name. */ + label: string; + /** The guide's own title (envelope-level copy for listing without re-parse). */ + title: string; + /** Engine that generated the guide (e.g. "claude", "codex"). */ + engine?: string; + /** Model used, when known. */ + model?: string; + /** Repo HEAD (or PR head) sha at generation time — drives the `moved` flag. */ + headSha?: string; + /** PR/MR url when the guide was generated in PR mode. */ + prUrl?: string; + guide: CodeGuideOutput; + reviewed: boolean[]; +} + +// --------------------------------------------------------------------------- +// Repo-key derivation +// --------------------------------------------------------------------------- + +function sanitizeKeySegment(segment: string): string { + const cleaned = segment.replace(/[^a-zA-Z0-9._-]+/g, "-").replace(/^[.\-]+|[.\-]+$/g, ""); + return cleaned || "x"; +} + +function hash8(content: string): string { + return createHash("sha256").update(content).digest("hex").slice(0, 8); +} + +/** + * Derive a repo key from a git remote URL (SSH, HTTPS, ssh://…), e.g. + * `git@github.com:owner/repo.git` → `github.com__owner__repo`. + * Subgroup paths keep every segment (`gitlab.com__group__sub__proj`). + * Returns null when the URL doesn't parse as a known remote form. + */ +export function deriveGuideRepoKeyFromRemote(remoteUrl: string): string | null { + const host = parseRemoteHost(remoteUrl); + const path = parseRemoteUrl(remoteUrl); + if (!host || !path) return null; + const segments = path.split("/").filter(Boolean); + if (segments.length === 0) return null; + return [host, ...segments].map(sanitizeKeySegment).join("__"); +} + +/** + * Derive a repo key from a PR/MR URL — lands on the same key as + * deriveGuideRepoKeyFromRemote for the matching origin remote, so PR-mode and + * branch-mode sessions of one repository share a shelf. + */ +export function deriveGuideRepoKeyFromPRUrl(prUrl: string): string | null { + const ref = parsePRUrl(prUrl); + if (!ref) return null; + const path = ref.platform === "github" ? `${ref.owner}/${ref.repo}` : ref.projectPath; + return [ref.host, ...path.split("/").filter(Boolean)].map(sanitizeKeySegment).join("__"); +} + +/** + * No-remote fallback: repo-root (or cwd) directory name + hash8 of the + * resolved path — mirrors annotate mode's `annotate-{basename}-{hash8}` slug. + */ +export function deriveGuideRepoKeyFallback(dirPath: string): string { + const resolved = resolve(dirPath); + const name = sanitizeKeySegment(basename(resolved) || "repo"); + return `${name}-${hash8(resolved)}`; +} + +// --------------------------------------------------------------------------- +// Ids +// --------------------------------------------------------------------------- + +const GUIDE_ID_RE = /^[A-Za-z0-9][A-Za-z0-9._-]*$/; + +/** Guard against path traversal — ids arrive from the client on the + * `saved:{id}` endpoints, so only a conservative charset is accepted. */ +export function isValidGuideId(id: string): boolean { + return id.length > 0 && id.length <= 160 && GUIDE_ID_RE.test(id); +} + +/** `{timestamp}-{slug-of-title}` — sortable-by-name ≙ sortable-by-time. */ +export function makeGuideId(title: string, now: number = Date.now()): string { + const slug = title + .toLowerCase() + .replace(/[^a-z0-9]+/g, "-") + .replace(/^-+|-+$/g, "") + .slice(0, 60) + .replace(/-+$/g, ""); + return `${now}-${slug || "guide"}`; +} + +// --------------------------------------------------------------------------- +// Disk primitives +// --------------------------------------------------------------------------- + +function guidesDir(repoKey: string): string { + return join(getPlannotatorDataDir(), "guides", repoKey); +} + +function guidePath(repoKey: string, id: string): string { + return join(guidesDir(repoKey), `${id}.json`); +} + +function coerceReviewed(value: unknown, sectionCount: number): boolean[] { + const out = new Array(sectionCount).fill(false); + if (Array.isArray(value)) { + for (let i = 0; i < sectionCount; i++) out[i] = value[i] === true; + } + return out; +} + +/** Minimal shape check — anything that fails loads as "no saved guide". */ +function parseEnvelope(raw: string): SavedGuideEnvelope | null { + let parsed: unknown; + try { + parsed = JSON.parse(raw); + } catch { + return null; + } + if (!parsed || typeof parsed !== "object") return null; + const obj = parsed as Record; + if (obj.version !== 1) return null; + const guide = obj.guide as Record | undefined; + if (!guide || typeof guide !== "object") return null; + if (typeof guide.title !== "string") return null; + if (!Array.isArray(guide.sections) || guide.sections.length === 0) return null; + const sectionCount = guide.sections.length; + return { + version: 1, + savedAt: typeof obj.savedAt === "number" ? obj.savedAt : 0, + label: typeof obj.label === "string" ? obj.label : "", + title: typeof obj.title === "string" ? obj.title : (guide.title as string), + ...(typeof obj.engine === "string" ? { engine: obj.engine } : {}), + ...(typeof obj.model === "string" ? { model: obj.model } : {}), + ...(typeof obj.headSha === "string" && obj.headSha ? { headSha: obj.headSha } : {}), + ...(typeof obj.prUrl === "string" && obj.prUrl ? { prUrl: obj.prUrl } : {}), + guide: guide as unknown as CodeGuideOutput, + reviewed: coerceReviewed(obj.reviewed, sectionCount), + }; +} + +/** + * Atomically write a guide envelope. Returns false (never throws) on invalid + * id or write failure — persistence must never break the review session. + */ +export function saveGuide(repoKey: string, id: string, envelope: SavedGuideEnvelope): boolean { + if (!isValidGuideId(id)) return false; + try { + mkdirSync(guidesDir(repoKey), { recursive: true }); + const finalPath = guidePath(repoKey, id); + const tmpPath = `${finalPath}.tmp`; + writeFileSync(tmpPath, JSON.stringify(envelope, null, 2), "utf-8"); + renameSync(tmpPath, finalPath); + return true; + } catch (e) { + console.error(`[guide-store] Failed to save guide ${id}: ${e}`); + return false; + } +} + +/** Load one saved guide. Missing/corrupt/invalid → null. */ +export function loadGuide(repoKey: string, id: string): SavedGuideEnvelope | null { + if (!isValidGuideId(id)) return null; + try { + const filePath = guidePath(repoKey, id); + if (!existsSync(filePath)) return null; + return parseEnvelope(readFileSync(filePath, "utf-8")); + } catch { + return null; + } +} + +/** Update the reviewed array of a saved guide in place (atomic rewrite). */ +export function updateGuideReviewed(repoKey: string, id: string, reviewed: boolean[]): boolean { + const envelope = loadGuide(repoKey, id); + if (!envelope) return false; + return saveGuide(repoKey, id, { + ...envelope, + reviewed: coerceReviewed(reviewed, envelope.guide.sections.length), + }); +} + +/** List all saved guides for a repo, newest first. Corrupt files are skipped. */ +export function listGuides(repoKey: string): Array<{ id: string; envelope: SavedGuideEnvelope }> { + let names: string[]; + try { + names = readdirSync(guidesDir(repoKey)); + } catch { + return []; + } + const out: Array<{ id: string; envelope: SavedGuideEnvelope }> = []; + for (const name of names) { + if (!name.endsWith(".json")) continue; + const id = name.slice(0, -".json".length); + if (!isValidGuideId(id)) continue; + const envelope = loadGuide(repoKey, id); + if (envelope) out.push({ id, envelope }); + } + out.sort((a, b) => b.envelope.savedAt - a.envelope.savedAt); + return out; +} + +/** Delete a saved guide. Returns true when a file was actually removed. */ +export function deleteGuide(repoKey: string, id: string): boolean { + if (!isValidGuideId(id)) return false; + try { + const filePath = guidePath(repoKey, id); + if (!existsSync(filePath)) return false; + unlinkSync(filePath); + return true; + } catch { + return false; + } +} + +// --------------------------------------------------------------------------- +// Session glue — shared by the Bun review server and the Pi mirror so the +// repo-key/headSha/label resolution and the jobId→savedId mapping are written +// exactly once. All getters are late-bound because prMetadata / gitContext / +// config can change mid-session (pr-switch, config edits). +// --------------------------------------------------------------------------- + +export interface GuideStoreSessionOptions { + /** Run `git ` in `cwd`, returning stdout on exit 0, else null. + * Omit when the session has no local git access. */ + runGit?: (args: string[], cwd?: string) => Promise; + /** Cwd for git commands (branch mode). Undefined disables git lookups. */ + getGitCwd: () => string | undefined; + /** Current PR identity, or null outside PR mode. */ + getPRInfo: () => { url: string; headSha: string; label: string } | null; + /** Branch label for non-PR sessions (gitContext.currentBranch). */ + getBranchLabel: () => string | undefined; + /** Directory for the no-remote fallback key (workspace root / agent cwd). */ + getFallbackDir: () => string; + /** Autosave gate — resolveGuideHistory(loadConfig()), evaluated per write. */ + writesEnabled: () => boolean; +} + +/** + * Review-target snapshot taken when a guide job LAUNCHES. Guide jobs run for + * minutes while the session supports mid-generation PR switching + * (/api/pr-switch) and diff switches — reading the live getters at completion + * would permanently label the envelope with the WRONG context (launched on PR + * A, switched to B, completed: A's content stamped with B's label/url/head). + * Captured via captureLaunchContext() at build time and carried on the job + * itself (AgentJobInfo.guideContext), same discipline as changedFilesSnapshot. + */ +export interface GuideLaunchContext { + pr?: { url: string; headSha: string; label: string } | null; + branchLabel?: string; + headSha?: string; +} + +export interface GuideStoreSession { + /** Snapshot the CURRENT review-target context for a job being launched now. + * The caller stamps the result onto the job; saveForJob prefers it over + * the live getters at completion time. */ + captureLaunchContext(): Promise; + /** Persist a successfully validated guide for a completed job. Prefers the + * job's launch-time context snapshot; falls back to the live getters only + * when no snapshot exists (defensive). Never throws. */ + saveForJob( + job: { id: string; engine?: string; model?: string }, + data: CodeGuideOutput & { reviewed?: boolean[] }, + launchContext?: GuideLaunchContext, + ): Promise; + /** True when this live job id has already been autosaved this session. */ + isJobSaved(jobId: string): boolean; + /** Write a live job's reviewed-state change through to its saved file. */ + writeThroughReviewed(jobId: string, reviewed: boolean[]): Promise; + /** Serve a saved guide as guide data (guide + reviewed + saved/moved flags). */ + getSavedGuideData( + id: string, + ): Promise<(CodeGuideOutput & { reviewed: boolean[]; saved: true; moved: boolean }) | null>; + /** Persist reviewed state for a saved guide (`saved:{id}` PUT). */ + updateSavedReviewed(id: string, reviewed: boolean[]): Promise; + /** GET /api/guides rows, newest first. */ + listSaved(): Promise; + /** DELETE /api/guides/:id. */ + deleteSaved(id: string): Promise; +} + +export function createGuideStoreSession(options: GuideStoreSessionOptions): GuideStoreSession { + const { runGit, getGitCwd, getPRInfo, getBranchLabel, getFallbackDir, writesEnabled } = options; + + /** jobId → saved location, for reviewed write-through and the `saved` flag. + * Carries the repo key the save actually landed under (launch-time PR key + * when one was captured), so write-through never chases the session key. */ + const savedIdByJob = new Map(); + + let repoKeyPromise: Promise | null = null; + const resolveRepoKey = (): Promise => { + if (!repoKeyPromise) { + repoKeyPromise = (async () => { + const pr = getPRInfo(); + if (pr) { + const key = deriveGuideRepoKeyFromPRUrl(pr.url); + if (key) return key; + } + const cwd = getGitCwd(); + if (cwd && runGit) { + const remote = await runGit(["remote", "get-url", "origin"], cwd); + if (remote?.trim()) { + const key = deriveGuideRepoKeyFromRemote(remote.trim()); + if (key) return key; + } + const toplevel = await runGit(["rev-parse", "--show-toplevel"], cwd); + if (toplevel?.trim()) return deriveGuideRepoKeyFallback(toplevel.trim()); + } + return deriveGuideRepoKeyFallback(getFallbackDir()); + })().catch(() => deriveGuideRepoKeyFallback(getFallbackDir())); + } + return repoKeyPromise; + }; + + /** The head the CURRENT session is looking at — PR head in PR mode, else + * local HEAD. Compared against each envelope's stored headSha for `moved`. */ + const currentHeadSha = async (): Promise => { + const pr = getPRInfo(); + if (pr && pr.headSha) return pr.headSha; + const cwd = getGitCwd(); + if (cwd && runGit) { + const out = await runGit(["rev-parse", "HEAD"], cwd); + const sha = out?.trim(); + if (sha && /^[0-9a-f]{7,64}$/i.test(sha)) return sha; + } + return undefined; + }; + + const isMoved = (stored: string | undefined, current: string | undefined): boolean => + !!(stored && current && stored !== current); + + return { + async captureLaunchContext() { + const pr = getPRInfo(); + return { + ...(pr ? { pr } : {}), + ...(getBranchLabel() ? { branchLabel: getBranchLabel() } : {}), + ...(await currentHeadSha().then((sha) => (sha ? { headSha: sha } : {}))), + }; + }, + + async saveForJob(job, data, launchContext) { + try { + if (!writesEnabled()) return; + const { reviewed, ...guide } = data; + const existingId = savedIdByJob.get(job.id); + // Launch-time snapshot wins over the live getters: the envelope must + // describe the changeset the guide was GENERATED against, not the + // PR/diff the reviewer switched to while the job ran. Live getters are + // the defensive fallback for jobs launched without a snapshot. + const pr = launchContext ? launchContext.pr ?? null : getPRInfo(); + const headSha = launchContext ? launchContext.headSha : await currentHeadSha(); + const branchLabel = launchContext ? launchContext.branchLabel : getBranchLabel(); + // Anchor the shelf to the launch-time PR too — a cross-project + // pr-switch must not file PR A's guide under B's repository. + const repoKey = existingId?.repoKey + ?? (pr?.url ? deriveGuideRepoKeyFromPRUrl(pr.url) : null) + ?? (await resolveRepoKey()); + const id = existingId?.id ?? makeGuideId(guide.title); + const envelope: SavedGuideEnvelope = { + version: 1, + savedAt: Date.now(), + label: pr?.label ?? branchLabel ?? "local", + title: guide.title, + ...(job.engine ? { engine: job.engine } : {}), + ...(job.model ? { model: job.model } : {}), + ...(pr?.headSha ? { headSha: pr.headSha } : headSha ? { headSha } : {}), + ...(pr?.url ? { prUrl: pr.url } : {}), + guide: guide as CodeGuideOutput, + reviewed: coerceReviewed(reviewed, guide.sections.length), + }; + if (saveGuide(repoKey, id, envelope)) savedIdByJob.set(job.id, { repoKey, id }); + } catch (e) { + // Persistence must never break job completion. + console.error(`[guide-store] Autosave failed for job ${job.id}: ${e}`); + } + }, + + isJobSaved(jobId) { + return savedIdByJob.has(jobId); + }, + + async writeThroughReviewed(jobId, reviewed) { + const saved = savedIdByJob.get(jobId); + if (!saved) return; + try { + updateGuideReviewed(saved.repoKey, saved.id, reviewed); + } catch { + // Best-effort — the in-memory state is still authoritative this session. + } + }, + + async getSavedGuideData(id) { + const envelope = loadGuide(await resolveRepoKey(), id); + if (!envelope) return null; + const head = await currentHeadSha(); + return { + ...envelope.guide, + reviewed: coerceReviewed(envelope.reviewed, envelope.guide.sections.length), + saved: true, + moved: isMoved(envelope.headSha, head), + }; + }, + + async updateSavedReviewed(id, reviewed) { + return updateGuideReviewed(await resolveRepoKey(), id, reviewed); + }, + + async listSaved() { + const repoKey = await resolveRepoKey(); + const head = await currentHeadSha(); + return listGuides(repoKey).map(({ id, envelope }) => ({ + id, + label: envelope.label, + title: envelope.title, + savedAt: envelope.savedAt, + progress: { + reviewed: envelope.reviewed.filter(Boolean).length, + total: envelope.guide.sections.length, + }, + moved: isMoved(envelope.headSha, head), + })); + }, + + async deleteSaved(id) { + return deleteGuide(await resolveRepoKey(), id); + }, + }; +} diff --git a/packages/shared/guide.ts b/packages/shared/guide.ts index 90d1d920cd..056f1f397e 100644 --- a/packages/shared/guide.ts +++ b/packages/shared/guide.ts @@ -32,5 +32,23 @@ export interface CodeGuideOutput { unplacedFiles?: string[]; } -/** UI-side guide shape: server output extended with persisted per-section reviewed state. */ -export type CodeGuideData = CodeGuideOutput & { reviewed: boolean[] }; +/** One row of GET /api/guides — a persisted guide for the current repo + * (#1112). Loaded through the guide endpoints as the `saved:{id}` pseudo + * job id. Browser-safe (types only); the store lives in guide-store.ts. */ +export interface SavedGuideListEntry { + id: string; + /** Review-target label — "PR #1082" or the branch name. */ + label: string; + title: string; + /** Epoch ms when the guide was first persisted. */ + savedAt: number; + progress: { reviewed: number; total: number }; + /** True when the stored head sha differs from the current head. */ + moved: boolean; +} + +/** UI-side guide shape: server output extended with persisted per-section reviewed state. + * `saved` is set when the guide is persisted on disk (autosaved live job, or a + * `saved:{id}` load); `moved` is set only on `saved:` loads whose stored head + * sha differs from the head currently under review. */ +export type CodeGuideData = CodeGuideOutput & { reviewed: boolean[]; saved?: boolean; moved?: boolean }; diff --git a/packages/shared/package.json b/packages/shared/package.json index 06b2d8d342..c383043ba2 100644 --- a/packages/shared/package.json +++ b/packages/shared/package.json @@ -52,6 +52,7 @@ "./html-assets-node": "./html-assets-node.ts", "./tour": "./tour.ts", "./guide": "./guide.ts", + "./guide-store": "./guide-store.ts", "./annotate-args": "./annotate-args.ts", "./at-reference": "./at-reference.ts", "./code-nav": "./code-nav.ts",