From 4aba49e3163701da6881a6461f96b6a75c0c0477 Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Tue, 14 Apr 2026 12:47:03 -0700 Subject: [PATCH 1/5] feat(config): add planSave schema, resolver, and customPath guard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add planSave to PlannotatorConfig so the server can read user preferences at arrival-save time — before any HTTP request has arrived from the UI. resolvePlanSave follows the established resolveUseJina precedence (env > config > default). isSafeCustomPath rejects path-traversal segments at the /api/config boundary as a defense-in-depth measure against persisting a malicious customPath to disk. Refs #556. For provenance purposes, this commit was AI assisted. --- packages/server/config.ts | 3 ++ packages/shared/config.test.ts | 80 ++++++++++++++++++++++++++++++++ packages/shared/config.ts | 83 +++++++++++++++++++++++++++++++++- 3 files changed, 165 insertions(+), 1 deletion(-) create mode 100644 packages/shared/config.test.ts diff --git a/packages/server/config.ts b/packages/server/config.ts index a3e6b39f..792aacc1 100644 --- a/packages/server/config.ts +++ b/packages/server/config.ts @@ -3,6 +3,9 @@ export { saveConfig, detectGitUser, getServerConfig, + resolvePlanSave, + isSafeCustomPath, type PlannotatorConfig, + type PlanSaveConfig, type DiffOptions, } from "@plannotator/shared/config"; diff --git a/packages/shared/config.test.ts b/packages/shared/config.test.ts new file mode 100644 index 00000000..24a0f517 --- /dev/null +++ b/packages/shared/config.test.ts @@ -0,0 +1,80 @@ +/** + * Tests for config.ts — focuses on resolvePlanSave precedence + * and saveConfig nested merge semantics for planSave. + * + * Run: bun test packages/shared/config.test.ts + */ + +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { resolvePlanSave, type PlannotatorConfig } from "./config"; + +describe("resolvePlanSave", () => { + const ENV_KEYS = ["PLANNOTATOR_PLAN_SAVE", "PLANNOTATOR_PLAN_SAVE_ON_ARRIVAL"] as const; + const saved: Record = {}; + + beforeEach(() => { + for (const k of ENV_KEYS) { + saved[k] = process.env[k]; + delete process.env[k]; + } + }); + + afterEach(() => { + for (const k of ENV_KEYS) { + if (saved[k] === undefined) delete process.env[k]; + else process.env[k] = saved[k]; + } + }); + + test("returns defaults when config is empty and no env vars are set", () => { + expect(resolvePlanSave({})).toEqual({ + enabled: true, + customPath: null, + saveOnArrival: true, + }); + }); + + test("reads values from config when present", () => { + const cfg: PlannotatorConfig = { + planSave: { enabled: false, customPath: "/tmp/myplans", saveOnArrival: false }, + }; + expect(resolvePlanSave(cfg)).toEqual({ + enabled: false, + customPath: "/tmp/myplans", + saveOnArrival: false, + }); + }); + + test("env var PLANNOTATOR_PLAN_SAVE=false overrides config enabled=true", () => { + process.env.PLANNOTATOR_PLAN_SAVE = "false"; + expect(resolvePlanSave({ planSave: { enabled: true } }).enabled).toBe(false); + }); + + test("env var PLANNOTATOR_PLAN_SAVE=1 overrides config enabled=false", () => { + process.env.PLANNOTATOR_PLAN_SAVE = "1"; + expect(resolvePlanSave({ planSave: { enabled: false } }).enabled).toBe(true); + }); + + test("env var PLANNOTATOR_PLAN_SAVE_ON_ARRIVAL=false overrides config", () => { + process.env.PLANNOTATOR_PLAN_SAVE_ON_ARRIVAL = "false"; + expect(resolvePlanSave({ planSave: { saveOnArrival: true } }).saveOnArrival).toBe(false); + }); + + test("customPath has no env var override — always config-sourced", () => { + process.env.PLANNOTATOR_PLAN_SAVE = "true"; + process.env.PLANNOTATOR_PLAN_SAVE_ON_ARRIVAL = "true"; + const cfg: PlannotatorConfig = { + planSave: { customPath: "/tmp/custom" }, + }; + expect(resolvePlanSave(cfg).customPath).toBe("/tmp/custom"); + }); + + test("partial planSave config fills missing fields with defaults", () => { + const cfg: PlannotatorConfig = { planSave: { customPath: "/tmp/only" } }; + expect(resolvePlanSave(cfg)).toEqual({ + enabled: true, + customPath: "/tmp/only", + saveOnArrival: true, + }); + }); +}); diff --git a/packages/shared/config.ts b/packages/shared/config.ts index eeba1b95..c5466e1a 100644 --- a/packages/shared/config.ts +++ b/packages/shared/config.ts @@ -31,6 +31,24 @@ export interface CCLabelConfig { blocking: boolean; } +/** + * Plan save preferences. Controls whether plans are written to + * ~/.plannotator/plans/ (or a custom directory) both on arrival (server + * startup) and on approve/deny decisions. + * + * Migrated from browser cookies (`plannotator-save-enabled`, + * `plannotator-save-path`) to config.json so the server can honor user + * preferences during arrival save — before the UI has made any request. + */ +export interface PlanSaveConfig { + /** Master switch. When false, no decision snapshots are written. Default: true. */ + enabled?: boolean; + /** Custom directory for plan saves. null/undefined = default ~/.plannotator/plans/. */ + customPath?: string | null; + /** When true, writes plain {slug}.md on server startup. Default: true. */ + saveOnArrival?: boolean; +} + export interface PlannotatorConfig { displayName?: string; diffOptions?: DiffOptions; @@ -52,6 +70,8 @@ export interface PlannotatorConfig { * Set to false to always use plain fetch + Turndown. */ jina?: boolean; + /** Plan save preferences (migrated from browser cookies). */ + planSave?: PlanSaveConfig; } const CONFIG_DIR = join(homedir(), ".plannotator"); @@ -83,7 +103,15 @@ export function saveConfig(partial: Partial): void { const mergedDiffOptions = (current.diffOptions || partial.diffOptions) ? { ...current.diffOptions, ...partial.diffOptions } : undefined; - const merged = { ...current, ...partial, diffOptions: mergedDiffOptions }; + const mergedPlanSave = (current.planSave || partial.planSave) + ? { ...current.planSave, ...partial.planSave } + : undefined; + const merged = { + ...current, + ...partial, + diffOptions: mergedDiffOptions, + planSave: mergedPlanSave, + }; mkdirSync(CONFIG_DIR, { recursive: true }); writeFileSync(CONFIG_PATH, JSON.stringify(merged, null, 2) + "\n", "utf-8"); } catch (e) { @@ -114,6 +142,7 @@ export function getServerConfig(gitUser: string | null): { gitUser?: string; conventionalComments?: boolean; conventionalLabels?: CCLabelConfig[] | null; + planSave?: PlanSaveConfig; } { const cfg = loadConfig(); return { @@ -122,6 +151,7 @@ export function getServerConfig(gitUser: string | null): { gitUser: gitUser ?? undefined, ...(cfg.conventionalComments !== undefined && { conventionalComments: cfg.conventionalComments }), ...(cfg.conventionalLabels !== undefined && { conventionalLabels: cfg.conventionalLabels }), + ...(cfg.planSave !== undefined && { planSave: cfg.planSave }), }; } @@ -155,3 +185,54 @@ export function resolveUseJina(cliNoJina: boolean, config: PlannotatorConfig): b // Default: enabled return true; } + +function parseBoolEnv(value: string | undefined): boolean | undefined { + if (value === undefined) return undefined; + return value === "1" || value.toLowerCase() === "true"; +} + +/** + * Reject unsafe custom plan-save paths at the `/api/config` boundary. + * + * Rejects strings containing `..` path segments — defense-in-depth against a + * local process POSTing a traversal path that would later be read back via + * `loadConfig()` and used by the arrival save. The server binds to loopback + * by default so this is not a remote-reachable sink, but the two-step + * (write → restart → use) pattern warrants a boundary check. + * + * Returns true for `null` (reset to default) and for strings without `..` + * segments. Non-strings are rejected. Absolute paths and `~` are allowed — + * users can legitimately pick any directory they have write access to. + */ +export function isSafeCustomPath(value: unknown): value is string | null { + if (value === null) return true; + if (typeof value !== "string") return false; + const segments = value.replace(/\\/g, "/").split("/"); + return !segments.some((s) => s === ".."); +} + +/** + * Resolve effective plan-save settings from config + environment. + * + * Priority (highest wins): + * PLANNOTATOR_PLAN_SAVE / PLANNOTATOR_PLAN_SAVE_ON_ARRIVAL env vars + * → config.json planSave.{enabled, customPath, saveOnArrival} + * → defaults (enabled: true, customPath: null, saveOnArrival: true) + * + * customPath has no env var override (paths are too user-specific; config.json + * is the one source). Session-level overrides happen via the approve/deny + * request body, handled by the caller. + */ +export function resolvePlanSave(config: PlannotatorConfig): { + enabled: boolean; + customPath: string | null; + saveOnArrival: boolean; +} { + const envEnabled = parseBoolEnv(process.env.PLANNOTATOR_PLAN_SAVE); + const envOnArrival = parseBoolEnv(process.env.PLANNOTATOR_PLAN_SAVE_ON_ARRIVAL); + return { + enabled: envEnabled ?? config.planSave?.enabled ?? true, + customPath: config.planSave?.customPath ?? null, + saveOnArrival: envOnArrival ?? config.planSave?.saveOnArrival ?? true, + }; +} From bba7f0023306ea2b2442972f65d7730b37381b05 Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Tue, 14 Apr 2026 12:47:12 -0700 Subject: [PATCH 2/5] feat(server): arrival save and env-first planSave resolution MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Write a plain {slug}.md to ~/.plannotator/plans/ on server startup before the browser opens. Fixes #556 — users who exit Claude Code without approving/denying now see a file land in the plans directory they expect. - Resolve planSave on approve/deny by merging body.planSave into effective config and running resolvePlanSave, so PLANNOTATOR_PLAN_SAVE / PLANNOTATOR_PLAN_SAVE_ON_ARRIVAL env vars retain top precedence and a client default payload can't silently re-enable saves an operator disabled at the environment level. - Guard /api/config against persisting path-traversal segments to config.json in all three Bun servers (plan, review, annotate). - Test coexistence: arrival {slug}.md lives alongside -approved.md and stays invisible to listArchivedPlans. For provenance purposes, this commit was AI assisted. --- packages/server/annotate.ts | 10 ++- packages/server/index.ts | 118 ++++++++++++++++++-------------- packages/server/review.ts | 10 ++- packages/server/storage.test.ts | 22 +++++- 4 files changed, 102 insertions(+), 58 deletions(-) diff --git a/packages/server/annotate.ts b/packages/server/annotate.ts index 13865f2a..0ef36a02 100644 --- a/packages/server/annotate.ts +++ b/packages/server/annotate.ts @@ -18,7 +18,7 @@ import { handleImage, handleUpload, handleServerReady, handleDraftSave, handleDr import { handleDoc, handleFileBrowserFiles, handleObsidianVaults, handleObsidianFiles, handleObsidianDoc } from "./reference-handlers"; import { contentHash, deleteDraft } from "./draft"; import { createExternalAnnotationHandler } from "./external-annotations"; -import { saveConfig, detectGitUser, getServerConfig } from "./config"; +import { saveConfig, detectGitUser, getServerConfig, isSafeCustomPath } from "./config"; import { dirname } from "path"; import { isWSL } from "./browser"; @@ -158,12 +158,18 @@ export async function startAnnotateServer( // API: Update user config (write-back to ~/.plannotator/config.json) if (url.pathname === "/api/config" && req.method === "POST") { try { - const body = (await req.json()) as { displayName?: string; diffOptions?: Record; conventionalComments?: boolean; conventionalLabels?: unknown[] | null }; + const body = (await req.json()) as { displayName?: string; diffOptions?: Record; conventionalComments?: boolean; conventionalLabels?: unknown[] | null; planSave?: { enabled?: boolean; customPath?: string | null; saveOnArrival?: boolean } }; const toSave: Record = {}; if (body.displayName !== undefined) toSave.displayName = body.displayName; if (body.diffOptions !== undefined) toSave.diffOptions = body.diffOptions; if (body.conventionalComments !== undefined) toSave.conventionalComments = body.conventionalComments; if (body.conventionalLabels !== undefined) toSave.conventionalLabels = body.conventionalLabels; + if (body.planSave !== undefined) { + if (body.planSave.customPath !== undefined && !isSafeCustomPath(body.planSave.customPath)) { + return Response.json({ error: "Invalid planSave.customPath" }, { status: 400 }); + } + toSave.planSave = body.planSave; + } if (Object.keys(toSave).length > 0) saveConfig(toSave as Parameters[0]); return Response.json({ ok: true }); } catch { diff --git a/packages/server/index.ts b/packages/server/index.ts index d2c416aa..383a7190 100644 --- a/packages/server/index.ts +++ b/packages/server/index.ts @@ -38,7 +38,7 @@ import { } from "./storage"; import { getRepoInfo } from "./repo"; import { detectProjectName } from "./project"; -import { saveConfig, detectGitUser, getServerConfig } from "./config"; +import { loadConfig, saveConfig, detectGitUser, getServerConfig, resolvePlanSave, isSafeCustomPath, type PlanSaveConfig, type PlannotatorConfig } from "./config"; import { handleImage, handleUpload, handleAgents, handleServerReady, handleDraftSave, handleDraftLoad, handleDraftDelete, handleFavicon, type OpencodeClient } from "./shared-handlers"; import { contentHash, deleteDraft } from "./draft"; import { handleDoc, handleObsidianVaults, handleObsidianFiles, handleObsidianDoc, handleFileBrowserFiles } from "./reference-handlers"; @@ -176,6 +176,20 @@ export async function startPlannotatorServer( project = (await detectProjectName()) ?? "_unknown"; const historyResult = saveToHistory(project, slug, plan); currentPlanPath = historyResult.path; + + // Arrival save: write a plain {slug}.md to ~/.plannotator/plans/ (or + // custom path) before the UI opens. Respects user preferences from + // config.json — not cookies — because no HTTP request has arrived yet. + // Wrapped in try/catch so filesystem errors never block server startup. + try { + const planSaveCfg = resolvePlanSave(loadConfig()); + if (planSaveCfg.enabled && planSaveCfg.saveOnArrival) { + savePlan(slug, plan, planSaveCfg.customPath); + } + } catch (e) { + process.stderr.write(`[plannotator] Arrival save failed: ${e instanceof Error ? e.message : String(e)}\n`); + } + previousPlan = historyResult.version > 1 ? getPlanVersion(project, slug, historyResult.version - 1) @@ -285,12 +299,18 @@ export async function startPlannotatorServer( // API: Update user config (write-back to ~/.plannotator/config.json) if (url.pathname === "/api/config" && req.method === "POST") { try { - const body = (await req.json()) as { displayName?: string; diffOptions?: Record; conventionalComments?: boolean; conventionalLabels?: unknown[] | null }; + const body = (await req.json()) as { displayName?: string; diffOptions?: Record; conventionalComments?: boolean; conventionalLabels?: unknown[] | null; planSave?: PlanSaveConfig }; const toSave: Record = {}; if (body.displayName !== undefined) toSave.displayName = body.displayName; if (body.diffOptions !== undefined) toSave.diffOptions = body.diffOptions; if (body.conventionalComments !== undefined) toSave.conventionalComments = body.conventionalComments; if (body.conventionalLabels !== undefined) toSave.conventionalLabels = body.conventionalLabels; + if (body.planSave !== undefined) { + if (body.planSave.customPath !== undefined && !isSafeCustomPath(body.planSave.customPath)) { + return Response.json({ error: "Invalid planSave.customPath" }, { status: 400 }); + } + toSave.planSave = body.planSave; + } if (Object.keys(toSave).length > 0) saveConfig(toSave as Parameters[0]); return Response.json({ ok: true }); } catch { @@ -414,44 +434,37 @@ export async function startPlannotatorServer( // API: Approve plan if (url.pathname === "/api/approve" && req.method === "POST") { - // Check for note integrations and optional feedback - let feedback: string | undefined; - let agentSwitch: string | undefined; - let requestedPermissionMode: string | undefined; - let planSaveEnabled = true; // default to enabled for backwards compat - let planSaveCustomPath: string | undefined; + // Parse body first so we can treat body.planSave as a config + // override feeding into resolvePlanSave. That keeps the precedence + // chain intact: env var > body > config.json > defaults. A client + // default payload can't silently re-enable saves when the operator + // set PLANNOTATOR_PLAN_SAVE=false. + let body: { + obsidian?: ObsidianConfig; + bear?: BearConfig; + octarine?: OctarineConfig; + feedback?: string; + agentSwitch?: string; + planSave?: { enabled: boolean; customPath?: string | null }; + permissionMode?: string; + } = {}; try { - const body = (await req.json().catch(() => ({}))) as { - obsidian?: ObsidianConfig; - bear?: BearConfig; - octarine?: OctarineConfig; - feedback?: string; - agentSwitch?: string; - planSave?: { enabled: boolean; customPath?: string }; - permissionMode?: string; - }; + body = (await req.json()) as typeof body; + } catch { /* empty body → use config/env defaults */ } - // Capture feedback if provided (for "approve with notes") - if (body.feedback) { - feedback = body.feedback; - } + const feedback = body.feedback; + const agentSwitch = body.agentSwitch; + const requestedPermissionMode = body.permissionMode; - // Capture agent switch setting for OpenCode - if (body.agentSwitch) { - agentSwitch = body.agentSwitch; - } - - // Capture permission mode from client request (Claude Code) - if (body.permissionMode) { - requestedPermissionMode = body.permissionMode; - } - - // Capture plan save settings - if (body.planSave !== undefined) { - planSaveEnabled = body.planSave.enabled; - planSaveCustomPath = body.planSave.customPath; - } + const cfg = loadConfig(); + const effectiveCfg: PlannotatorConfig = body.planSave !== undefined + ? { ...cfg, planSave: { ...cfg.planSave, ...body.planSave } } + : cfg; + const planSaveResolved = resolvePlanSave(effectiveCfg); + const planSaveEnabled = planSaveResolved.enabled; + const planSaveCustomPath: string | undefined = planSaveResolved.customPath ?? undefined; + try { // Run integrations in parallel — they're independent const integrationResults: Record = {}; const integrationPromises: Promise[] = []; @@ -497,24 +510,23 @@ export async function startPlannotatorServer( // API: Deny with feedback if (url.pathname === "/api/deny" && req.method === "POST") { - let feedback = "Plan rejected by user"; - let planSaveEnabled = true; // default to enabled for backwards compat - let planSaveCustomPath: string | undefined; + let body: { + feedback?: string; + planSave?: { enabled: boolean; customPath?: string | null }; + } = {}; try { - const body = (await req.json()) as { - feedback?: string; - planSave?: { enabled: boolean; customPath?: string }; - }; - feedback = body.feedback || feedback; - - // Capture plan save settings - if (body.planSave !== undefined) { - planSaveEnabled = body.planSave.enabled; - planSaveCustomPath = body.planSave.customPath; - } - } catch { - // Use default feedback - } + body = (await req.json()) as typeof body; + } catch { /* empty body */ } + const feedback = body.feedback || "Plan rejected by user"; + + // See /api/approve for precedence rationale: env var > body > config. + const cfg = loadConfig(); + const effectiveCfg: PlannotatorConfig = body.planSave !== undefined + ? { ...cfg, planSave: { ...cfg.planSave, ...body.planSave } } + : cfg; + const planSaveResolved = resolvePlanSave(effectiveCfg); + const planSaveEnabled = planSaveResolved.enabled; + const planSaveCustomPath: string | undefined = planSaveResolved.customPath ?? undefined; // Save annotations and final snapshot (if enabled) let savedPath: string | undefined; diff --git a/packages/server/review.ts b/packages/server/review.ts index c6b98ac0..d267e18a 100644 --- a/packages/server/review.ts +++ b/packages/server/review.ts @@ -32,7 +32,7 @@ import { parseClaudeStreamOutput, transformClaudeFindings, } from "./claude-review"; -import { saveConfig, detectGitUser, getServerConfig } from "./config"; +import { saveConfig, detectGitUser, getServerConfig, isSafeCustomPath } from "./config"; import { type PRMetadata, type PRReviewFileComment, fetchPRFileContent, fetchPRContext, submitPRReview, fetchPRViewedFiles, markPRFilesViewed, getPRUser, prRefFromMetadata, getDisplayRepo, getMRLabel, getMRNumberLabel } from "./pr"; import { createAIEndpoints, ProviderRegistry, SessionManager, createProvider, type AIEndpoints, type PiSDKConfig } from "@plannotator/ai"; import { isWSL } from "./browser"; @@ -513,12 +513,18 @@ export async function startReviewServer( // API: Update user config (write-back to ~/.plannotator/config.json) if (url.pathname === "/api/config" && req.method === "POST") { try { - const body = (await req.json()) as { displayName?: string; diffOptions?: Record; conventionalComments?: boolean; conventionalLabels?: unknown[] | null }; + const body = (await req.json()) as { displayName?: string; diffOptions?: Record; conventionalComments?: boolean; conventionalLabels?: unknown[] | null; planSave?: { enabled?: boolean; customPath?: string | null; saveOnArrival?: boolean } }; const toSave: Record = {}; if (body.displayName !== undefined) toSave.displayName = body.displayName; if (body.diffOptions !== undefined) toSave.diffOptions = body.diffOptions; if (body.conventionalComments !== undefined) toSave.conventionalComments = body.conventionalComments; if (body.conventionalLabels !== undefined) toSave.conventionalLabels = body.conventionalLabels; + if (body.planSave !== undefined) { + if (body.planSave.customPath !== undefined && !isSafeCustomPath(body.planSave.customPath)) { + return Response.json({ error: "Invalid planSave.customPath" }, { status: 400 }); + } + toSave.planSave = body.planSave; + } if (Object.keys(toSave).length > 0) saveConfig(toSave as Parameters[0]); return Response.json({ ok: true }); } catch { diff --git a/packages/server/storage.test.ts b/packages/server/storage.test.ts index 8774d5e9..81befb86 100644 --- a/packages/server/storage.test.ts +++ b/packages/server/storage.test.ts @@ -8,7 +8,7 @@ import { afterEach, describe, expect, test } from "bun:test"; import { mkdtempSync, rmSync, readFileSync, readdirSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; -import { generateSlug, getPlanDir, savePlan, saveToHistory, getPlanVersion, getVersionCount, listVersions } from "./storage"; +import { generateSlug, getPlanDir, savePlan, saveFinalSnapshot, saveToHistory, getPlanVersion, getVersionCount, listVersions, listArchivedPlans } from "./storage"; const tempDirs: string[] = []; @@ -84,6 +84,26 @@ describe("savePlan", () => { expect(path).toBe(join(dir, "test-slug.md")); expect(readFileSync(path, "utf-8")).toBe("# Content"); }); + + test("arrival-save {slug}.md coexists with -approved.md snapshot", () => { + // Simulates: server starts → savePlan (arrival) → user approves → + // saveFinalSnapshot (decision). Both files should exist side by side, + // and listArchivedPlans should return only the decision snapshot. + const dir = makeTempDir(); + const slug = `coexist-${Date.now()}`; + const arrivalPath = savePlan(slug, "# Arrival", dir); + const approvedPath = saveFinalSnapshot(slug, "approved", "# Arrival", "feedback", dir); + + expect(readFileSync(arrivalPath, "utf-8")).toBe("# Arrival"); + expect(readFileSync(approvedPath, "utf-8")).toContain("# Arrival"); + expect(readFileSync(approvedPath, "utf-8")).toContain("feedback"); + + const archived = listArchivedPlans(dir); + expect(archived.length).toBe(1); + expect(archived[0].status).toBe("approved"); + // Plain {slug}.md should be invisible to archive listing + expect(archived.find(p => p.filename === `${slug}.md`)).toBeUndefined(); + }); }); describe("saveToHistory", () => { From d59a3793ff98afc5403214a3b2d6b3d5e22dd91a Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Tue, 14 Apr 2026 12:47:19 -0700 Subject: [PATCH 3/5] feat(pi): mirror arrival save and planSave resolution in Pi servers Pi's storage/config modules are copied from packages/shared/ at build time via vendor.sh, so schema + resolver changes propagate for free. This commit updates the hand-written Pi server code: - Arrival save block in serverPlan.ts (same try/catch as Bun). - Approve/deny handlers use effective-config merge so env vars remain authoritative. - /api/config handlers (plan, review, annotate) reject unsafe customPath values. For provenance purposes, this commit was AI assisted. --- apps/pi-extension/server/serverAnnotate.ts | 11 ++- apps/pi-extension/server/serverPlan.ts | 91 +++++++++++++++------- apps/pi-extension/server/serverReview.ts | 11 ++- 3 files changed, 79 insertions(+), 34 deletions(-) diff --git a/apps/pi-extension/server/serverAnnotate.ts b/apps/pi-extension/server/serverAnnotate.ts index 7b1e1177..cc14df8e 100644 --- a/apps/pi-extension/server/serverAnnotate.ts +++ b/apps/pi-extension/server/serverAnnotate.ts @@ -2,7 +2,7 @@ import { createServer } from "node:http"; import { dirname, resolve as resolvePath } from "node:path"; import { contentHash, deleteDraft } from "../generated/draft.js"; -import { saveConfig, detectGitUser, getServerConfig } from "../generated/config.js"; +import { saveConfig, detectGitUser, getServerConfig, isSafeCustomPath } from "../generated/config.js"; import { handleDraftRequest, @@ -94,11 +94,18 @@ export async function startAnnotateServer(options: { }); } else if (url.pathname === "/api/config" && req.method === "POST") { try { - const body = (await parseBody(req)) as { displayName?: string; diffOptions?: Record; conventionalComments?: boolean }; + const body = (await parseBody(req)) as { displayName?: string; diffOptions?: Record; conventionalComments?: boolean; planSave?: { enabled?: boolean; customPath?: string | null; saveOnArrival?: boolean } }; const toSave: Record = {}; if (body.displayName !== undefined) toSave.displayName = body.displayName; if (body.diffOptions !== undefined) toSave.diffOptions = body.diffOptions; if (body.conventionalComments !== undefined) toSave.conventionalComments = body.conventionalComments; + if (body.planSave !== undefined) { + if (body.planSave.customPath !== undefined && !isSafeCustomPath(body.planSave.customPath)) { + json(res, { error: "Invalid planSave.customPath" }, 400); + return; + } + toSave.planSave = body.planSave; + } if (Object.keys(toSave).length > 0) saveConfig(toSave as Parameters[0]); json(res, { ok: true }); } catch { diff --git a/apps/pi-extension/server/serverPlan.ts b/apps/pi-extension/server/serverPlan.ts index 78448877..a54e18bb 100644 --- a/apps/pi-extension/server/serverPlan.ts +++ b/apps/pi-extension/server/serverPlan.ts @@ -13,6 +13,7 @@ import { readArchivedPlan, saveAnnotations, saveFinalSnapshot, + savePlan, saveToHistory, } from "../generated/storage.js"; import { createEditorAnnotationHandler } from "./annotations.js"; @@ -36,7 +37,7 @@ import { } from "./integrations.js"; import { listenOnPort } from "./network.js"; -import { saveConfig, detectGitUser, getServerConfig } from "../generated/config.js"; +import { loadConfig, saveConfig, detectGitUser, getServerConfig, resolvePlanSave, isSafeCustomPath, type PlannotatorConfig } from "../generated/config.js"; import { detectProjectName, getRepoInfo } from "./project.js"; import { handleDocRequest, @@ -112,6 +113,21 @@ export async function startPlanReviewServer(options: { options.mode !== "archive" ? saveToHistory(project, slug, options.plan) : { version: 0, path: "", isNew: false }; + + // Arrival save: write a plain {slug}.md to ~/.plannotator/plans/ (or + // custom path) before the UI opens. Respects user preferences from + // config.json — not cookies — because no HTTP request has arrived yet. + // Wrapped in try/catch so filesystem errors never block server startup. + if (options.mode !== "archive") { + try { + const planSaveCfg = resolvePlanSave(loadConfig()); + if (planSaveCfg.enabled && planSaveCfg.saveOnArrival) { + savePlan(slug, options.plan, planSaveCfg.customPath); + } + } catch (e) { + process.stderr.write(`[plannotator] Arrival save failed: ${e instanceof Error ? e.message : String(e)}\n`); + } + } const previousPlan = options.mode !== "archive" && historyResult.version > 1 ? getPlanVersion(project, slug, historyResult.version - 1) @@ -225,11 +241,18 @@ export async function startPlanReviewServer(options: { } } else if (url.pathname === "/api/config" && req.method === "POST") { try { - const body = (await parseBody(req)) as { displayName?: string; diffOptions?: Record; conventionalComments?: boolean }; + const body = (await parseBody(req)) as { displayName?: string; diffOptions?: Record; conventionalComments?: boolean; planSave?: { enabled?: boolean; customPath?: string | null; saveOnArrival?: boolean } }; const toSave: Record = {}; if (body.displayName !== undefined) toSave.displayName = body.displayName; if (body.diffOptions !== undefined) toSave.diffOptions = body.diffOptions; if (body.conventionalComments !== undefined) toSave.conventionalComments = body.conventionalComments; + if (body.planSave !== undefined) { + if (body.planSave.customPath !== undefined && !isSafeCustomPath(body.planSave.customPath)) { + json(res, { error: "Invalid planSave.customPath" }, 400); + return; + } + toSave.planSave = body.planSave; + } if (Object.keys(toSave).length > 0) saveConfig(toSave as Parameters[0]); json(res, { ok: true }); } catch { @@ -342,22 +365,29 @@ export async function startPlanReviewServer(options: { json(res, { ok: true, duplicate: true }); return; } - let feedback: string | undefined; - let agentSwitch: string | undefined; - let requestedPermissionMode: string | undefined; - let planSaveEnabled = true; - let planSaveCustomPath: string | undefined; + // Parse body first so body.planSave acts as a config override fed + // into resolvePlanSave — env > body > config.json > defaults. A + // client default payload can't silently re-enable saves when the + // operator set PLANNOTATOR_PLAN_SAVE=false. + let body: Record = {}; + try { + body = await parseBody(req); + } catch { /* empty body */ } + + const feedback = body.feedback as string | undefined; + const agentSwitch = body.agentSwitch as string | undefined; + const requestedPermissionMode = body.permissionMode as string | undefined; + const bodyPlanSave = body.planSave as { enabled?: boolean; customPath?: string | null } | undefined; + + const approveCfg = loadConfig(); + const approveEffectiveCfg: PlannotatorConfig = bodyPlanSave !== undefined + ? { ...approveCfg, planSave: { ...approveCfg.planSave, ...bodyPlanSave } } + : approveCfg; + const approveResolved = resolvePlanSave(approveEffectiveCfg); + const planSaveEnabled = approveResolved.enabled; + const planSaveCustomPath: string | undefined = approveResolved.customPath ?? undefined; + try { - const body = await parseBody(req); - if (body.feedback) feedback = body.feedback as string; - if (body.agentSwitch) agentSwitch = body.agentSwitch as string; - if (body.permissionMode) - requestedPermissionMode = body.permissionMode as string; - if (body.planSave !== undefined) { - const ps = body.planSave as { enabled: boolean; customPath?: string }; - planSaveEnabled = ps.enabled; - planSaveCustomPath = ps.customPath; - } // Run note integrations in parallel const integrationResults: Record = {}; const integrationPromises: Promise[] = []; @@ -421,20 +451,21 @@ export async function startPlanReviewServer(options: { json(res, { ok: true, duplicate: true }); return; } - let feedback = "Plan rejected by user"; - let planSaveEnabled = true; - let planSaveCustomPath: string | undefined; + let denyBody: Record = {}; try { - const body = await parseBody(req); - feedback = (body.feedback as string) || feedback; - if (body.planSave !== undefined) { - const ps = body.planSave as { enabled: boolean; customPath?: string }; - planSaveEnabled = ps.enabled; - planSaveCustomPath = ps.customPath; - } - } catch { - /* use default feedback */ - } + denyBody = await parseBody(req); + } catch { /* empty body */ } + const feedback = (denyBody.feedback as string) || "Plan rejected by user"; + const denyBodyPlanSave = denyBody.planSave as { enabled?: boolean; customPath?: string | null } | undefined; + + // See /api/approve for precedence rationale: env > body > config. + const denyCfg = loadConfig(); + const denyEffectiveCfg: PlannotatorConfig = denyBodyPlanSave !== undefined + ? { ...denyCfg, planSave: { ...denyCfg.planSave, ...denyBodyPlanSave } } + : denyCfg; + const denyResolved = resolvePlanSave(denyEffectiveCfg); + const planSaveEnabled = denyResolved.enabled; + const planSaveCustomPath: string | undefined = denyResolved.customPath ?? undefined; let savedPath: string | undefined; if (planSaveEnabled) { saveAnnotations(slug, feedback, planSaveCustomPath); diff --git a/apps/pi-extension/server/serverReview.ts b/apps/pi-extension/server/serverReview.ts index c69180aa..0bbc2d94 100644 --- a/apps/pi-extension/server/serverReview.ts +++ b/apps/pi-extension/server/serverReview.ts @@ -6,7 +6,7 @@ import os from "node:os"; import { Readable } from "node:stream"; import { contentHash, deleteDraft } from "../generated/draft.js"; -import { saveConfig, detectGitUser, getServerConfig } from "../generated/config.js"; +import { saveConfig, detectGitUser, getServerConfig, isSafeCustomPath } from "../generated/config.js"; export type { DiffOption, @@ -592,11 +592,18 @@ export async function startReviewServer(options: { json(res, { error: "No file access available" }, 400); } else if (url.pathname === "/api/config" && req.method === "POST") { try { - const body = (await parseBody(req)) as { displayName?: string; diffOptions?: Record; conventionalComments?: boolean }; + const body = (await parseBody(req)) as { displayName?: string; diffOptions?: Record; conventionalComments?: boolean; planSave?: { enabled?: boolean; customPath?: string | null; saveOnArrival?: boolean } }; const toSave: Record = {}; if (body.displayName !== undefined) toSave.displayName = body.displayName; if (body.diffOptions !== undefined) toSave.diffOptions = body.diffOptions; if (body.conventionalComments !== undefined) toSave.conventionalComments = body.conventionalComments; + if (body.planSave !== undefined) { + if (body.planSave.customPath !== undefined && !isSafeCustomPath(body.planSave.customPath)) { + json(res, { error: "Invalid planSave.customPath" }, 400); + return; + } + toSave.planSave = body.planSave; + } if (Object.keys(toSave).length > 0) saveConfig(toSave as Parameters[0]); json(res, { ok: true }); } catch { From 95068d06b26d3b468d9bf1fd6937b7d77678ba9c Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Tue, 14 Apr 2026 12:47:32 -0700 Subject: [PATCH 4/5] feat(ui): migrate planSave from cookies to config.json MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Source of truth for plan-save preferences moves from browser cookies to ~/.plannotator/config.json, delivered to the client in the /api/plan response as serverConfig.planSave. - getPlanSaveSettings prefers serverConfig over legacy cookies, with a one-shot migration POST to /api/config that clears cookies only after the server confirms the write. - savePlanSaveSettings chains POSTs through a module-level promise queue so rapid changes (keystrokes in the custom-path input) land on the server in call order. - Settings.tsx takes a serverPlanSave prop and an onServerPlanSaveChange callback so mid-session changes propagate to App state — approve/deny and the archive browser see the new values without waiting for a reload. - Approve/deny bodies send customPath explicitly as string|null so a cleared path doesn't leave a stale config path winning the server's body-over-config merge. - useArchive.init marks hasFetched=true and App.tsx drops the redundant archive.fetchPlans() call that used to race with the initial setServerPlanSave update and clobber the archive list with defaults. Accepts a one-session discrepancy for legacy users upgrading with saves-disabled cookies: decision snapshots that session land in the default location alongside the arrival file, then config.json is authoritative forever after. Documented inline in planSave.ts. Refs #556. For provenance purposes, this commit was AI assisted. --- packages/editor/App.tsx | 33 +++++-- packages/ui/components/Settings.tsx | 18 +++- packages/ui/hooks/useArchive.ts | 14 ++- packages/ui/utils/planSave.ts | 136 ++++++++++++++++++++++++---- 4 files changed, 166 insertions(+), 35 deletions(-) diff --git a/packages/editor/App.tsx b/packages/editor/App.tsx index 5425d145..ac3b79c6 100644 --- a/packages/editor/App.tsx +++ b/packages/editor/App.tsx @@ -27,7 +27,7 @@ import { getBearSettings } from '@plannotator/ui/utils/bear'; import { getOctarineSettings, isOctarineConfigured } from '@plannotator/ui/utils/octarine'; import { getDefaultNotesApp } from '@plannotator/ui/utils/defaultNotesApp'; import { getAgentSwitchSettings, getEffectiveAgentName } from '@plannotator/ui/utils/agentSwitch'; -import { getPlanSaveSettings } from '@plannotator/ui/utils/planSave'; +import { getPlanSaveSettings, type ServerPlanSave } from '@plannotator/ui/utils/planSave'; import { getUIPreferences, type UIPreferences, type PlanWidth } from '@plannotator/ui/utils/uiPreferences'; import { getEditorMode, saveEditorMode } from '@plannotator/ui/utils/editorMode'; import { getInputMethod, saveInputMethod } from '@plannotator/ui/utils/inputMethod'; @@ -130,6 +130,7 @@ const App: React.FC = () => { const [isApiMode, setIsApiMode] = useState(false); const [origin, setOrigin] = useState(null); const [gitUser, setGitUser] = useState(); + const [serverPlanSave, setServerPlanSave] = useState(); const [isWSL, setIsWSL] = useState(false); const [globalAttachments, setGlobalAttachments] = useState([]); const [annotateMode, setAnnotateMode] = useState(false); @@ -227,6 +228,7 @@ const App: React.FC = () => { const archive = useArchive({ markdown, viewerRef, linkedDocHook, setMarkdown, setAnnotations, setSelectedAnnotationId, setSubmitted, + serverPlanSave, }); // Markdown file browser (also handles vault dirs via isVault flag) @@ -526,16 +528,21 @@ const App: React.FC = () => { if (!res.ok) throw new Error('Not in API mode'); return res.json(); }) - .then((data: { plan: string; origin?: Origin; mode?: 'annotate' | 'annotate-last' | 'annotate-folder' | 'archive'; filePath?: string; sourceInfo?: string; sharingEnabled?: boolean; shareBaseUrl?: string; pasteApiUrl?: string; repoInfo?: { display: string; branch?: string }; previousPlan?: string | null; versionInfo?: { version: number; totalVersions: number; project: string }; archivePlans?: ArchivedPlan[]; projectRoot?: string; isWSL?: boolean; serverConfig?: { displayName?: string; gitUser?: string } }) => { + .then((data: { plan: string; origin?: Origin; mode?: 'annotate' | 'annotate-last' | 'annotate-folder' | 'archive'; filePath?: string; sourceInfo?: string; sharingEnabled?: boolean; shareBaseUrl?: string; pasteApiUrl?: string; repoInfo?: { display: string; branch?: string }; previousPlan?: string | null; versionInfo?: { version: number; totalVersions: number; project: string }; archivePlans?: ArchivedPlan[]; projectRoot?: string; isWSL?: boolean; serverConfig?: { displayName?: string; gitUser?: string; planSave?: ServerPlanSave } }) => { // Initialize config store with server-provided values (config file > cookie > default) configStore.init(data.serverConfig); // gitUser drives the "Use git name" button in Settings; stays undefined (button hidden) when unavailable setGitUser(data.serverConfig?.gitUser); + // planSave is authoritative source for plan save preferences (config.json > cookie legacy) + setServerPlanSave(data.serverConfig?.planSave); if (data.mode === 'archive') { - // Archive mode: show first archived plan or clear demo content + // Archive mode: show first archived plan or clear demo content. + // /api/plan already delivered the archivePlans list and first plan + // content using the server-side customPath; don't call + // archive.fetchPlans() here — it would race with setServerPlanSave + // above and re-fetch against the default directory. setMarkdown(data.plan || ''); if (data.archivePlans) archive.init(data.archivePlans); - archive.fetchPlans(); setSharingEnabled(false); sidebar.open('archive'); } else if (data.mode === 'annotate-folder') { @@ -761,13 +768,13 @@ const App: React.FC = () => { const bearSettings = getBearSettings(); const octarineSettings = getOctarineSettings(); const agentSwitchSettings = getAgentSwitchSettings(); - const planSaveSettings = getPlanSaveSettings(); + const planSaveSettings = getPlanSaveSettings(serverPlanSave); const autoSaveResults = bearSettings.autoSave && autoSavePromiseRef.current ? await autoSavePromiseRef.current : autoSaveResultsRef.current; // Build request body - include integrations if enabled - const body: { obsidian?: object; bear?: object; octarine?: object; feedback?: string; agentSwitch?: string; planSave?: { enabled: boolean; customPath?: string }; permissionMode?: string } = {}; + const body: { obsidian?: object; bear?: object; octarine?: object; feedback?: string; agentSwitch?: string; planSave?: { enabled: boolean; customPath: string | null }; permissionMode?: string } = {}; // Include permission mode for Claude Code if (origin === 'claude-code') { @@ -780,10 +787,12 @@ const App: React.FC = () => { body.agentSwitch = effectiveAgent; } - // Include plan save settings + // Include plan save settings. Always send customPath explicitly (string + // or null) — the server merges this over config, so omitting would let + // a stale config path survive when the user just cleared it. body.planSave = { enabled: planSaveSettings.enabled, - ...(planSaveSettings.customPath && { customPath: planSaveSettings.customPath }), + customPath: planSaveSettings.customPath, }; const effectiveVaultPath = getEffectiveVaultPath(obsidianSettings); @@ -837,15 +846,17 @@ const App: React.FC = () => { const handleDeny = async () => { setIsSubmitting(true); try { - const planSaveSettings = getPlanSaveSettings(); + const planSaveSettings = getPlanSaveSettings(serverPlanSave); await fetch('/api/deny', { method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ feedback: annotationsOutput, + // Always send customPath explicitly (string or null) so a cleared + // path doesn't leave a stale config path winning the server merge. planSave: { enabled: planSaveSettings.enabled, - ...(planSaveSettings.customPath && { customPath: planSaveSettings.customPath }), + customPath: planSaveSettings.customPath, }, }) }); @@ -1400,6 +1411,8 @@ const App: React.FC = () => { externalOpen={mobileSettingsOpen} onExternalClose={() => setMobileSettingsOpen(false)} gitUser={gitUser} + serverPlanSave={serverPlanSave} + onServerPlanSaveChange={setServerPlanSave} /> diff --git a/packages/ui/components/Settings.tsx b/packages/ui/components/Settings.tsx index fbb62092..7decba1a 100644 --- a/packages/ui/components/Settings.tsx +++ b/packages/ui/components/Settings.tsx @@ -35,6 +35,7 @@ import { getPlanSaveSettings, savePlanSaveSettings, type PlanSaveSettings, + type ServerPlanSave, } from '../utils/planSave'; import { getUIPreferences, @@ -86,6 +87,10 @@ interface SettingsProps { aiProviders?: Array<{ id: string; name: string; capabilities: Record }>; /** Git user name from `git config user.name`, for quick identity set */ gitUser?: string; + /** Plan save config from /api/plan serverConfig — authoritative source over legacy cookies */ + serverPlanSave?: ServerPlanSave; + /** Notify the parent when the user changes plan-save settings, so App state stays in sync and approve/deny sees the latest values. */ + onServerPlanSaveChange?: (updated: ServerPlanSave) => void; } // --- Review-mode Display tab (diff display options) --- @@ -542,7 +547,7 @@ const CommentsTab: React.FC = () => { ); }; -export const Settings: React.FC = ({ taterMode, onTaterModeChange, onIdentityChange, origin, mode = 'plan', onUIPreferencesChange, externalOpen, onExternalClose, aiProviders = [], gitUser }) => { +export const Settings: React.FC = ({ taterMode, onTaterModeChange, onIdentityChange, origin, mode = 'plan', onUIPreferencesChange, externalOpen, onExternalClose, aiProviders = [], gitUser, serverPlanSave, onServerPlanSaveChange }) => { const [showDialog, setShowDialog] = useState(false); const [activeTab, setActiveTab] = useState('general'); const [identity, setIdentity] = useState(''); @@ -625,7 +630,7 @@ export const Settings: React.FC = ({ taterMode, onTaterModeChange setBear(getBearSettings()); setOctarine(getOctarineSettings()); setAgent(getAgentSwitchSettings()); - setPlanSave(getPlanSaveSettings()); + setPlanSave(getPlanSaveSettings(serverPlanSave)); setUiPrefs(getUIPreferences()); setPermissionMode(getPermissionModeSettings().mode); setAutoCloseDelayState(getAutoCloseDelay()); @@ -639,7 +644,7 @@ export const Settings: React.FC = ({ taterMode, onTaterModeChange setAgentWarning(getAgentWarning()); } } - }, [showDialog, availableAgents, origin, getAgentWarning]); + }, [showDialog, availableAgents, origin, getAgentWarning, serverPlanSave]); useEffect(() => { if (!showDialog) return; @@ -726,6 +731,13 @@ export const Settings: React.FC = ({ taterMode, onTaterModeChange const newSettings = { ...planSave, ...updates }; setPlanSave(newSettings); savePlanSaveSettings(newSettings); + // Keep parent App state in sync so approve/deny and the archive browser + // pick up mid-session changes without waiting for a reload. + onServerPlanSaveChange?.({ + ...serverPlanSave, + enabled: newSettings.enabled, + customPath: newSettings.customPath, + }); }; const handleUIPrefsChange = (updates: Partial) => { diff --git a/packages/ui/hooks/useArchive.ts b/packages/ui/hooks/useArchive.ts index 9bcf2e10..6f2d0ff2 100644 --- a/packages/ui/hooks/useArchive.ts +++ b/packages/ui/hooks/useArchive.ts @@ -10,7 +10,7 @@ import type { ArchivedPlan } from "@plannotator/shared/storage"; import type { UseLinkedDocReturn } from "./useLinkedDoc"; import type { ViewerHandle } from "../components/Viewer"; import type { Annotation } from "../types"; -import { getPlanSaveSettings } from "../utils/planSave"; +import { getPlanSaveSettings, type ServerPlanSave } from "../utils/planSave"; export interface UseArchiveOptions { markdown: string; @@ -20,6 +20,8 @@ export interface UseArchiveOptions { setAnnotations: (anns: Annotation[]) => void; setSelectedAnnotationId: (id: string | null) => void; setSubmitted: (s: "approved" | "denied" | null) => void; + /** serverConfig.planSave delivered via /api/plan — source of truth for customPath */ + serverPlanSave?: ServerPlanSave; } export interface UseArchiveReturn { @@ -56,6 +58,7 @@ export function useArchive(options: UseArchiveOptions): UseArchiveReturn { setAnnotations, setSelectedAnnotationId, setSubmitted, + serverPlanSave, } = options; const [archiveMode, setArchiveMode] = useState(false); @@ -64,7 +67,10 @@ export function useArchive(options: UseArchiveOptions): UseArchiveReturn { const [isLoading, setIsLoading] = useState(false); const hasFetched = useRef(false); - const customPath = useMemo(() => getPlanSaveSettings().customPath || undefined, []); + const customPath = useMemo( + () => getPlanSaveSettings(serverPlanSave).customPath || undefined, + [serverPlanSave], + ); const currentInfo = useMemo(() => { if (!selectedFile) return null; @@ -75,6 +81,10 @@ export function useArchive(options: UseArchiveOptions): UseArchiveReturn { const init = useCallback((archivePlans: ArchivedPlan[]) => { setArchiveMode(true); setPlans(archivePlans); + // /api/plan already returned the list using the server-side customPath, + // so block any later fetchPlans() call from re-requesting with a stale + // closure (would race with setServerPlanSave and clobber with defaults). + hasFetched.current = true; if (archivePlans.length > 0) { setSelectedFile(archivePlans[0].filename); } diff --git a/packages/ui/utils/planSave.ts b/packages/ui/utils/planSave.ts index 3786792e..d90f2bae 100644 --- a/packages/ui/utils/planSave.ts +++ b/packages/ui/utils/planSave.ts @@ -1,11 +1,15 @@ /** * Plan Save Settings Utility * - * Manages settings for automatic plan saving after approval/denial. - * Users can configure custom save path or disable saving entirely. + * Controls automatic plan saving to ~/.plannotator/plans/ (or a custom + * directory) on both arrival (server startup) and approve/deny. * - * Uses cookies (not localStorage) because each hook invocation runs on a - * random port, and localStorage is scoped by origin including port. + * Source of truth is ~/.plannotator/config.json, delivered to the client in + * the /api/plan response as `serverConfig.planSave`. This utility also + * transparently migrates legacy browser-cookie settings + * (`plannotator-save-enabled`, `plannotator-save-path`) the first time a + * user loads the UI after upgrade: read cookies → POST to /api/config → + * clear cookies once the server confirms. */ import { storage } from './storage'; @@ -18,32 +22,124 @@ export interface PlanSaveSettings { customPath: string | null; } +export interface ServerPlanSave { + enabled?: boolean; + customPath?: string | null; + saveOnArrival?: boolean; +} + const DEFAULT_SETTINGS: PlanSaveSettings = { enabled: true, - customPath: null, // null means use default ~/.plannotator/plans/ + customPath: null, }; -/** - * Get current plan save settings from storage - */ -export function getPlanSaveSettings(): PlanSaveSettings { - const enabled = storage.getItem(STORAGE_KEY_ENABLED); - const customPath = storage.getItem(STORAGE_KEY_PATH); +// One-shot guard so concurrent getPlanSaveSettings() calls only fire a single +// migration POST per page load. Cleared in the fetch .finally() so a failed +// migration retries on the next call. +let migrationInFlight = false; +function readLegacyCookies(): PlanSaveSettings | null { + const cookieEnabled = storage.getItem(STORAGE_KEY_ENABLED); + const cookiePath = storage.getItem(STORAGE_KEY_PATH); + if (cookieEnabled === null && cookiePath === null) return null; return { - enabled: enabled !== 'false', // default to true - customPath: customPath || null, + enabled: cookieEnabled !== 'false', + customPath: cookiePath || null, }; } +function clearLegacyCookies(): void { + storage.removeItem(STORAGE_KEY_ENABLED); + storage.removeItem(STORAGE_KEY_PATH); +} + +function postPlanSave(settings: PlanSaveSettings): Promise { + return fetch('/api/config', { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ planSave: settings }), + }); +} + /** - * Save plan save settings to storage + * Get current plan save settings. + * + * Precedence: serverConfig.planSave (source of truth) → legacy cookies + * (with one-shot migration) → defaults. + * + * @param serverPlanSave The `serverConfig.planSave` value from /api/plan. + * Pass undefined when the server response hasn't + * arrived yet; the function will fall back to cookies. */ -export function savePlanSaveSettings(settings: PlanSaveSettings): void { - storage.setItem(STORAGE_KEY_ENABLED, String(settings.enabled)); - if (settings.customPath) { - storage.setItem(STORAGE_KEY_PATH, settings.customPath); - } else { - storage.removeItem(STORAGE_KEY_PATH); +export function getPlanSaveSettings( + serverPlanSave?: ServerPlanSave, +): PlanSaveSettings { + // Server config is authoritative whenever it's present + if (serverPlanSave !== undefined && serverPlanSave !== null) { + return { + enabled: serverPlanSave.enabled ?? true, + customPath: serverPlanSave.customPath ?? null, + }; } + + // Legacy cookie fallback + one-shot migration. + // + // Accepted trade-off (see plan "one-time discrepancy window"): on the first + // post-upgrade session for a user with legacy cookies, this function fires + // the migration POST and clears the cookies — but it returns the migrated + // value only to *this* caller. Any subsequent caller in the same session + // that doesn't yet have `serverPlanSave` from /api/plan (App state is not + // updated by this migration path) will read cookies-gone → serverPlanSave + // undefined → defaults. For a user who had saves disabled, this means that + // session's `-approved.md` / `-denied.md` snapshots land in the default + // `~/.plannotator/plans/` alongside the arrival `{slug}.md` that was also + // written with defaults before the UI mounted. Three harmless files on + // disk in the wrong place, one session, then config.json is authoritative + // forever after. Same blast radius as the arrival-save trade-off already + // signed off on — not worth the complexity of a module-level cache or + // parent-state callback plumbing to close a one-shot window. + const legacy = readLegacyCookies(); + if (legacy !== null) { + if (!migrationInFlight) { + migrationInFlight = true; + postPlanSave(legacy) + .then((r) => { + if (r.ok) clearLegacyCookies(); + }) + .catch(() => { /* keep cookies; retry next load */ }) + .finally(() => { migrationInFlight = false; }); + } + return legacy; + } + + return DEFAULT_SETTINGS; +} + +// Serialize POSTs so rapid changes (e.g. keystrokes in the custom-path input) +// land on the server in call order. Without this, an earlier POST arriving +// after a later one could persist a stale prefix in config.json even though +// the UI shows the final value. +let saveQueue: Promise = Promise.resolve(); + +/** + * Persist plan save settings. + * + * Writes to ~/.plannotator/config.json via POST /api/config. Also clears + * legacy cookies defensively so a user editing settings post-migration + * doesn't leave stale cookie values behind. Writes are chained so calls + * issued in quick succession are processed in order. + */ +export function savePlanSaveSettings(settings: PlanSaveSettings): Promise { + const next = saveQueue.then(async () => { + try { + const res = await postPlanSave(settings); + if (res.ok) clearLegacyCookies(); + } catch { + // Silent — Settings UI already reflects the change locally; next load + // will retry via the cookie fallback path if the POST never landed. + } + }); + // Swallow errors on the queue itself so a single failure doesn't break the chain. + saveQueue = next.catch(() => {}); + return next; } From 5c69503463524a8dfd13cf0e2316235c80563dc6 Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Tue, 14 Apr 2026 12:47:36 -0700 Subject: [PATCH 5/5] docs: document PLANNOTATOR_PLAN_SAVE env vars For provenance purposes, this commit was AI assisted. --- AGENTS.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/AGENTS.md b/AGENTS.md index 8e9f37bd..61f95cd3 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -106,6 +106,8 @@ claude --plugin-dir ./apps/hook | `PLANNOTATOR_PASTE_URL` | Base URL of the paste service API for short URL sharing. Default: `https://plannotator-paste.plannotator.workers.dev`. | | `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`. | | `JINA_API_KEY` | Optional Jina Reader API key for higher rate limits (500 RPM vs 20 RPM unauthenticated). Free keys include 10M tokens. | +| `PLANNOTATOR_PLAN_SAVE` | Set to `0` / `false` to disable all plan saves (arrival + approve/deny snapshots), or `1` / `true` to enable. Default: enabled. Overrides `~/.plannotator/config.json` (`{ "planSave": { "enabled": false } }`) and the client session override — cannot be re-enabled by the UI when set to disabled. | +| `PLANNOTATOR_PLAN_SAVE_ON_ARRIVAL` | Set to `0` / `false` to skip the plain `{slug}.md` write on server startup (decision snapshots still happen on approve/deny). Default: enabled. Overrides `~/.plannotator/config.json` (`{ "planSave": { "saveOnArrival": false } }`). | | `PLANNOTATOR_VERIFY_ATTESTATION` | **Read by the install scripts only**, not by the runtime binary. Set to `1` / `true` to have `scripts/install.sh` / `install.ps1` / `install.cmd` run `gh attestation verify` on every install. Off by default. Can also be set persistently via `~/.plannotator/config.json` (`{ "verifyAttestation": true }`) or per-invocation via `--verify-attestation`. Requires `gh` installed and authenticated. | **Legacy:** `SSH_TTY` and `SSH_CONNECTION` are still detected when `PLANNOTATOR_REMOTE` is unset. Set `PLANNOTATOR_REMOTE=1` / `true` to force remote mode or `0` / `false` to force local mode.