diff --git a/AGENTS.md b/AGENTS.md index 971609f5..5dac5e3c 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -256,8 +256,10 @@ During normal plan review, an Archive sidebar tab provides the same browsing via | Endpoint | Method | Purpose | | --------------------- | ------ | ------------------------------------------ | -| `/api/plan` | GET | Returns `{ plan, origin, mode: "annotate", filePath, sourceInfo? }` | +| `/api/plan` | GET | Returns `{ plan, origin, mode: "annotate", filePath, sourceInfo?, gate }` | | `/api/feedback` | POST | Submit annotations (body: feedback, annotations) | +| `/api/approve` | POST | Approve without feedback (review-gate UX, `--gate`) | +| `/api/exit` | POST | Close session without feedback | | `/api/image` | GET | Serve image by path query param | | `/api/upload` | POST | Upload image, returns `{ path, originalName }` | | `/api/draft` | GET/POST/DELETE | Auto-save annotation drafts to survive server crashes | diff --git a/apps/copilot/commands/plannotator-annotate.md b/apps/copilot/commands/plannotator-annotate.md index 56dac088..b0c1ca79 100644 --- a/apps/copilot/commands/plannotator-annotate.md +++ b/apps/copilot/commands/plannotator-annotate.md @@ -9,4 +9,8 @@ allowed-tools: shell(plannotator:*) ## Your task -Address the annotation feedback above. The user has reviewed the markdown file and provided specific annotations and comments. +The output above will be one of: + +1. The exact text `The user approved.`, OR a JSON object with `"decision": "approved"`. The user approved the markdown file. Acknowledge with a single sentence ("Approved.") and stop. Do not begin any work. +2. Empty, OR a JSON object with `"decision": "dismissed"`. The user closed the session without requesting changes. Acknowledge with a single sentence ("Annotation session closed.") and stop. Do not begin any work. +3. Plaintext annotation feedback, OR a JSON object with `"decision": "annotated"` and a `"feedback"` field. Address the feedback. The user has reviewed the markdown file and provided specific annotations and comments. diff --git a/apps/copilot/commands/plannotator-last.md b/apps/copilot/commands/plannotator-last.md index b241a478..06b507d3 100644 --- a/apps/copilot/commands/plannotator-last.md +++ b/apps/copilot/commands/plannotator-last.md @@ -5,8 +5,12 @@ allowed-tools: shell(plannotator:*) ## Message Annotations -!`plannotator copilot-last` +!`plannotator copilot-last $ARGUMENTS` ## Your task -Address the annotation feedback above. The user has reviewed your last message and provided specific annotations and comments. +The output above will be one of: + +1. The exact text `The user approved.`, OR a JSON object with `"decision": "approved"`. The user approved your last message. Acknowledge with a single sentence ("Approved.") and stop. Do not begin any work. +2. Empty, OR a JSON object with `"decision": "dismissed"`. The user closed the session without requesting changes. Acknowledge with a single sentence ("Annotation session closed.") and stop. Do not begin any work. +3. Plaintext annotation feedback, OR a JSON object with `"decision": "annotated"` and a `"feedback"` field. Address the feedback. The user has reviewed your last message and provided specific annotations and comments. diff --git a/apps/gemini/commands/plannotator-annotate.toml b/apps/gemini/commands/plannotator-annotate.toml index e6552dbe..1ed2a9af 100644 --- a/apps/gemini/commands/plannotator-annotate.toml +++ b/apps/gemini/commands/plannotator-annotate.toml @@ -6,5 +6,9 @@ prompt = """ ## Your task -Address the annotation feedback above. The user has reviewed the markdown file and provided specific annotations and comments. +The output above will be one of: + +1. The exact text `The user approved.`, OR a JSON object with `"decision": "approved"`. The user approved the markdown file. Acknowledge with a single sentence ("Approved.") and stop. Do not begin any work. +2. Empty, OR a JSON object with `"decision": "dismissed"`. The user closed the session without requesting changes. Acknowledge with a single sentence ("Annotation session closed.") and stop. Do not begin any work. +3. Plaintext annotation feedback, OR a JSON object with `"decision": "annotated"` and a `"feedback"` field. Address the feedback. The user has reviewed the markdown file and provided specific annotations and comments. """ diff --git a/apps/hook/commands/plannotator-annotate.md b/apps/hook/commands/plannotator-annotate.md index 218e270a..4da50d9e 100644 --- a/apps/hook/commands/plannotator-annotate.md +++ b/apps/hook/commands/plannotator-annotate.md @@ -10,4 +10,8 @@ disable-model-invocation: true ## Your task -Address the annotation feedback above. The user has reviewed the markdown file(s) and provided specific annotations and comments. +The output above will be one of: + +1. The exact text `The user approved.`, OR a JSON object with `"decision": "approved"`. The user approved the markdown file(s). Acknowledge with a single sentence ("Approved.") and stop. Do not begin any work. +2. Empty, OR a JSON object with `"decision": "dismissed"`. The user closed the session without requesting changes. Acknowledge with a single sentence ("Annotation session closed.") and stop. Do not begin any work. +3. Plaintext annotation feedback, OR a JSON object with `"decision": "annotated"` and a `"feedback"` field. Address the feedback. The user has reviewed the markdown file(s) and provided specific annotations and comments. diff --git a/apps/hook/commands/plannotator-last.md b/apps/hook/commands/plannotator-last.md index f8f5d8ad..51f8b5f4 100644 --- a/apps/hook/commands/plannotator-last.md +++ b/apps/hook/commands/plannotator-last.md @@ -6,8 +6,12 @@ disable-model-invocation: true ## Message Annotations -!`plannotator annotate-last` +!`plannotator annotate-last $ARGUMENTS` ## Your task -Address the annotation feedback above. The user has reviewed your last message and provided specific annotations and comments. +The output above will be one of: + +1. The exact text `The user approved.`, OR a JSON object with `"decision": "approved"`. The user approved your last message. Acknowledge with a single sentence ("Approved.") and stop. Do not begin any work. +2. Empty, OR a JSON object with `"decision": "dismissed"`. The user closed the session without requesting changes. Acknowledge with a single sentence ("Annotation session closed.") and stop. Do not begin any work. +3. Plaintext annotation feedback, OR a JSON object with `"decision": "annotated"` and a `"feedback"` field. Address the feedback. The user has reviewed your last message and provided specific annotations and comments. diff --git a/apps/hook/server/cli.ts b/apps/hook/server/cli.ts index b39ef544..5b20ad8f 100644 --- a/apps/hook/server/cli.ts +++ b/apps/hook/server/cli.ts @@ -15,7 +15,7 @@ export function formatTopLevelHelp(): string { " plannotator --help", " plannotator [--browser ]", " plannotator review [PR_URL]", - " plannotator annotate [--no-jina]", + " plannotator annotate [--no-jina] [--gate] [--json] [--silent-approve]", " plannotator last", " plannotator archive", " plannotator sessions", diff --git a/apps/hook/server/index.ts b/apps/hook/server/index.ts index 26e9c986..ace64c0c 100644 --- a/apps/hook/server/index.ts +++ b/apps/hook/server/index.ts @@ -65,6 +65,7 @@ import { } from "@plannotator/server/annotate"; import { type DiffType, getVcsContext, runVcsDiff, gitRuntime } from "@plannotator/server/vcs"; import { loadConfig, resolveDefaultDiffType, resolveUseJina } from "@plannotator/shared/config"; +import { stripAtPrefix, resolveAtReference } from "@plannotator/shared/at-reference"; import { htmlToMarkdown } from "@plannotator/shared/html-to-markdown"; import { urlToMarkdown } from "@plannotator/shared/url-to-markdown"; import { fetchRef, createWorktree, removeWorktree, ensureObjectAvailable } from "@plannotator/shared/worktree"; @@ -124,6 +125,55 @@ const noJinaIdx = args.indexOf("--no-jina"); const cliNoJina = noJinaIdx !== -1; if (cliNoJina) args.splice(noJinaIdx, 1); +// Annotate review-gate flags (#570): --gate adds an Approve button, +// --json switches stdout to structured decision output, --silent-approve +// suppresses the plaintext approve marker (naive hooks that treat any +// stdout as a block signal opt in here to keep silence-is-permission). +const gateIdx = args.indexOf("--gate"); +const gateFlag = gateIdx !== -1; +if (gateFlag) args.splice(gateIdx, 1); +const jsonIdx = args.indexOf("--json"); +const jsonFlag = jsonIdx !== -1; +if (jsonFlag) args.splice(jsonIdx, 1); +const silentApproveIdx = args.indexOf("--silent-approve"); +const silentApproveFlag = silentApproveIdx !== -1; +if (silentApproveFlag) args.splice(silentApproveIdx, 1); + +// Stdout matrix for annotate / annotate-last / copilot annotate-last (#570). +// Plaintext mode: +// - Close emits empty stdout (naive PostToolUse / Stop hooks: empty = allow). +// - Approve emits "The user approved." so agents and templates can +// distinguish approval from close without needing --json. With +// --silent-approve, Approve also emits empty stdout (hook-friendly). +// - Send Annotations emits the plaintext feedback markdown. +// --json switches to structured output across all three decisions; +// --silent-approve has no effect in --json mode (JSON always routes by +// decision field, so there's no ambiguity to silence). +export const APPROVED_PLAINTEXT_MARKER = "The user approved."; + +function emitAnnotateOutcome(result: { + feedback: string; + exit?: boolean; + approved?: boolean; +}): void { + if (jsonFlag) { + if (result.approved) { + console.log(JSON.stringify({ decision: "approved" })); + } else if (result.exit) { + console.log(JSON.stringify({ decision: "dismissed" })); + } else { + console.log(JSON.stringify({ decision: "annotated", feedback: result.feedback || "" })); + } + return; + } + if (result.exit) return; // empty stdout on close + if (result.approved) { + if (!silentApproveFlag) console.log(APPROVED_PLAINTEXT_MARKER); + return; + } + if (result.feedback) console.log(result.feedback); +} + if (isTopLevelHelpInvocation(args)) { console.log(formatTopLevelHelp()); process.exit(0); @@ -475,16 +525,16 @@ if (args[0] === "sessions") { // ANNOTATE MODE // ============================================ - let filePath = args[1]; - if (!filePath) { - console.error("Usage: plannotator annotate [--no-jina]"); + const rawFilePath = args[1]; + if (!rawFilePath) { + console.error("Usage: plannotator annotate [--no-jina] [--gate] [--json] [--silent-approve]"); process.exit(1); } - // Strip @ prefix if present (Claude Code file reference syntax) - if (filePath.startsWith("@")) { - filePath = filePath.slice(1); - } + // Primary resolution strips the `@` reference marker; rawFilePath is + // preserved so each branch can fall back to the literal form below + // (scoped-package-style names). + let filePath = stripAtPrefix(rawFilePath); // Use PLANNOTATOR_CWD if set (original working directory before script cd'd) const projectRoot = process.env.PLANNOTATOR_CWD || process.cwd(); @@ -519,16 +569,14 @@ if (args[0] === "sessions") { absolutePath = filePath; // Use URL as the "path" for display sourceInfo = filePath; // Full URL for source attribution } else { - // Check if the argument is a directory (folder annotation mode) - const resolvedArg = resolveUserPath(filePath, projectRoot); - let isFolder = false; - try { - isFolder = statSync(resolvedArg).isDirectory(); - } catch { - // Not a directory, fall through to file resolution - } - - if (isFolder) { + // Folder check with literal-@ fallback for scoped-package-style names. + const folderCandidate = resolveAtReference(rawFilePath, (c) => { + try { return statSync(resolveUserPath(c, projectRoot)).isDirectory(); } + catch { return false; } + }); + + if (folderCandidate !== null) { + const resolvedArg = resolveUserPath(folderCandidate, projectRoot); // Folder annotation mode (markdown + HTML files) if (!hasMarkdownFiles(resolvedArg, FILE_BROWSER_EXCLUDED, /\.(mdx?|html?)$/i)) { console.error(`No markdown or HTML files found in ${resolvedArg}`); @@ -539,41 +587,49 @@ if (args[0] === "sessions") { markdown = ""; annotateMode = "annotate-folder"; console.error(`Folder: ${resolvedArg}`); - } else if (/\.html?$/i.test(resolvedArg)) { - // HTML file annotation mode — convert to markdown via Turndown - if (!existsSync(resolvedArg)) { - console.error(`File not found: ${filePath}`); - process.exit(1); - } - const htmlFile = Bun.file(resolvedArg); - if (htmlFile.size > 10 * 1024 * 1024) { - console.error(`File too large (${Math.round(htmlFile.size / 1024 / 1024)}MB, max 10MB): ${resolvedArg}`); - process.exit(1); - } - const html = await htmlFile.text(); - markdown = htmlToMarkdown(html); - absolutePath = resolvedArg; - sourceInfo = path.basename(resolvedArg); - console.error(`Converted: ${absolutePath}`); } else { - // Single markdown file annotation mode - const resolved = resolveMarkdownFile(filePath, projectRoot); + // HTML check with the same literal-@ fallback semantics. + const htmlCandidate = resolveAtReference(rawFilePath, (c) => { + const abs = resolveUserPath(c, projectRoot); + return /\.html?$/i.test(abs) && existsSync(abs); + }); + + if (htmlCandidate !== null) { + const resolvedArg = resolveUserPath(htmlCandidate, projectRoot); + const htmlFile = Bun.file(resolvedArg); + if (htmlFile.size > 10 * 1024 * 1024) { + console.error(`File too large (${Math.round(htmlFile.size / 1024 / 1024)}MB, max 10MB): ${resolvedArg}`); + process.exit(1); + } + const html = await htmlFile.text(); + markdown = htmlToMarkdown(html); + absolutePath = resolvedArg; + sourceInfo = path.basename(resolvedArg); + console.error(`Converted: ${absolutePath}`); + } else { + // Single markdown file annotation mode + // Strip-first with literal-@ fallback (scoped-package-style names). + let resolved = resolveMarkdownFile(filePath, projectRoot); + if (resolved.kind === "not_found" && rawFilePath !== filePath) { + resolved = resolveMarkdownFile(rawFilePath, projectRoot); + } - if (resolved.kind === "ambiguous") { - console.error(`Ambiguous filename "${resolved.input}" — found ${resolved.matches.length} matches:`); - for (const match of resolved.matches) { - console.error(` ${match}`); + if (resolved.kind === "ambiguous") { + console.error(`Ambiguous filename "${resolved.input}" — found ${resolved.matches.length} matches:`); + for (const match of resolved.matches) { + console.error(` ${match}`); + } + process.exit(1); + } + if (resolved.kind === "not_found") { + console.error(`File not found: ${resolved.input}`); + process.exit(1); } - process.exit(1); - } - if (resolved.kind === "not_found") { - console.error(`File not found: ${resolved.input}`); - process.exit(1); - } - absolutePath = resolved.path; - markdown = await Bun.file(absolutePath).text(); - console.error(`Resolved: ${absolutePath}`); + absolutePath = resolved.path; + markdown = await Bun.file(absolutePath).text(); + console.error(`Resolved: ${absolutePath}`); + } } } @@ -590,6 +646,7 @@ if (args[0] === "sessions") { sharingEnabled, shareBaseUrl, pasteApiUrl, + gate: gateFlag, htmlContent: planHtmlContent, onReady: async (url, isRemote, port) => { handleAnnotateServerReady(url, isRemote, port); @@ -622,11 +679,7 @@ if (args[0] === "sessions") { server.stop(); // Output feedback (captured by slash command) - if (result.exit) { - console.log("Annotation session closed without feedback."); - } else { - console.log(result.feedback || "No feedback provided."); - } + emitAnnotateOutcome(result); process.exit(0); } else if (args[0] === "annotate-last" || args[0] === "last") { @@ -724,6 +777,7 @@ if (args[0] === "sessions") { sharingEnabled, shareBaseUrl, pasteApiUrl, + gate: gateFlag, htmlContent: planHtmlContent, onReady: async (url, isRemote, port) => { handleAnnotateServerReady(url, isRemote, port); @@ -750,11 +804,7 @@ if (args[0] === "sessions") { server.stop(); - if (result.exit) { - console.log("Annotation session closed without feedback."); - } else { - console.log(result.feedback || "No feedback provided."); - } + emitAnnotateOutcome(result); process.exit(0); } else if (args[0] === "archive") { @@ -915,6 +965,7 @@ if (args[0] === "sessions") { mode: "annotate-last", sharingEnabled, shareBaseUrl, + gate: gateFlag, htmlContent: planHtmlContent, onReady: async (url, isRemote, port) => { handleAnnotateServerReady(url, isRemote, port); @@ -939,11 +990,7 @@ if (args[0] === "sessions") { await Bun.sleep(1500); server.stop(); - if (result.exit) { - console.log("Annotation session closed without feedback."); - } else { - console.log(result.feedback || "No feedback provided."); - } + emitAnnotateOutcome(result); process.exit(0); } else if (args[0] === "improve-context") { diff --git a/apps/marketing/src/content/docs/commands/annotate-last.md b/apps/marketing/src/content/docs/commands/annotate-last.md index 315f7734..d4fc02f1 100644 --- a/apps/marketing/src/content/docs/commands/annotate-last.md +++ b/apps/marketing/src/content/docs/commands/annotate-last.md @@ -73,6 +73,18 @@ The annotation UI in `annotate-last` mode works the same as `/plannotator-annota - Completion screen says "annotations on the message" - Feedback export is titled "Message Feedback" instead of "Plan Feedback" +## Flags + +`plannotator annotate-last` accepts the same `--gate`, `--json`, and `--silent-approve` flags as `plannotator annotate`. See [Annotate → Flags](/docs/commands/annotate/#flags) for the full matrix. + +The common use case for `--gate` on annotate-last is a turn-by-turn review gate wired to a Stop hook: + +```bash +plannotator annotate-last --gate +``` + +Paired with a Claude Code `Stop` hook, this pauses every agent turn for human review. Approve lets the turn end; Send Annotations re-prompts the agent with feedback. See [Hook integration recipes](/docs/guides/hook-integration/). + ## Server API The annotate-last mode reuses the same annotate server endpoints. See the [annotate docs](/docs/commands/annotate/#server-api). diff --git a/apps/marketing/src/content/docs/commands/annotate.md b/apps/marketing/src/content/docs/commands/annotate.md index a7b82f36..beab8ab7 100644 --- a/apps/marketing/src/content/docs/commands/annotate.md +++ b/apps/marketing/src/content/docs/commands/annotate.md @@ -94,13 +94,57 @@ When annotating an HTML file or URL (not plain markdown), a small badge appears The annotation UI in annotate mode works the same as plan review, with a few changes: -- The "Approve" button is hidden (there's nothing to approve) +- The "Approve" button is hidden by default (there's nothing to approve for most use cases). Pass `--gate` to enable it as a review decision. - "Send Feedback" becomes **"Send Annotations"** - `Cmd/Ctrl+Enter` sends annotations instead of approving - The completion screen says "Annotations Sent" instead of "Plan Approved" All annotation types work identically: deletions, replacements, comments, insertions, global comments, and image attachments. +## Flags + +Three opt-in flags turn annotate into a review gate for hook integrations (spec-driven frameworks, turn-by-turn review, and so on). They compose: use any alone or combine them. + +### `--gate` + +Adds a third **Approve** button to the UI. The reviewer now has three exits: + +- **Approve** — the artifact looks good; the agent should proceed. +- **Send Annotations** — changes requested; feedback goes back to the agent. +- **Close** — dismissed without deciding. + +### `--json` + +Switches stdout to a structured decision object so hooks can route programmatically: + +```json +{ "decision": "approved" | "annotated" | "dismissed", "feedback": "..." } +``` + +`feedback` is only present when `decision === "annotated"`. + +### `--silent-approve` + +Suppresses the plaintext approve marker so Approve emits empty stdout instead of `The user approved.`. Use this with naive hooks that treat any non-empty stdout as a block signal. Approve and Close both become silent, and only Send Annotations blocks with feedback (silence-is-permission). + +`--silent-approve` only affects plaintext mode. In `--json` mode, Approve continues to emit `{"decision":"approved"}` — JSON callers route on the `decision` field, so there's no ambiguity to silence. + +### Stdout matrix + +| Flags | UX | Approve | Close | Send Annotations | +|---|---|---|---|---| +| *(none)* | 2-button | n/a | empty | feedback (plaintext) | +| `--gate` | 3-button | `The user approved.` | empty | feedback (plaintext) | +| `--gate --silent-approve` | 3-button | empty | empty | feedback (plaintext) | +| `--json` | 2-button | n/a | `{"decision":"dismissed"}` | `{"decision":"annotated","feedback":"..."}` | +| `--gate --json` | 3-button | `{"decision":"approved"}` | `{"decision":"dismissed"}` | `{"decision":"annotated","feedback":"..."}` | + +**Key property:** `--gate` plaintext output is unambiguous across three decisions — Close is empty, Send Annotations is feedback markdown, Approve is the line `The user approved.`. Drop the marker with `--silent-approve` when your hook treats any stdout as a block. Use `--json` when you want machine-readable decision objects instead of string matching. + +On OpenCode and Pi, `--json` and `--silent-approve` are silently accepted because those harnesses write back into the session directly rather than via stdout. The `--gate` flag behaves identically across all three harnesses. + +See [Hook integration recipes](/docs/guides/hook-integration/) for ready-to-use PostToolUse and Stop hook examples. + ## Feedback format When you send annotations, they're exported as structured markdown: @@ -128,8 +172,9 @@ The agent receives this and can act on each annotation. | Endpoint | Method | Purpose | |----------|--------|---------| -| `/api/plan` | GET | Returns `{ plan, mode: "annotate", filePath, sourceInfo }` | +| `/api/plan` | GET | Returns `{ plan, mode: "annotate", filePath, sourceInfo, gate }` | | `/api/feedback` | POST | Submit annotations | +| `/api/approve` | POST | Approve without feedback (`--gate` UX) | | `/api/exit` | POST | Close session without feedback | | `/api/image` | GET | Serve image by path | | `/api/upload` | POST | Upload image attachment | diff --git a/apps/marketing/src/content/docs/guides/annotate-gates-and-json-responses.md b/apps/marketing/src/content/docs/guides/annotate-gates-and-json-responses.md new file mode 100644 index 00000000..1f0a71e5 --- /dev/null +++ b/apps/marketing/src/content/docs/guides/annotate-gates-and-json-responses.md @@ -0,0 +1,119 @@ +--- +title: "Annotate Gates and JSON Responses" +description: "The --gate, --json, and --silent-approve flags extend plannotator annotate from a feedback tool into a structured review gate with machine-readable decisions. Use them to wire Plannotator into spec-driven workflows, Stop hooks, and agent pipelines." +sidebar: + order: 28 +section: "Guides" +--- + +`plannotator annotate` and `plannotator annotate-last` accept three flags that turn markdown annotation into a full review gate with structured output. + +## Capabilities + +- **`--gate`** adds an Approve button to the annotation UI. The reviewer picks one of three decisions: approve, send annotations, or close. +- **`--json`** emits every decision as a structured JSON object on stdout so hooks and plugins can route on the outcome without parsing free text. +- **`--silent-approve`** suppresses the plaintext approve marker so Approve emits empty stdout. Naive hooks that treat any stdout as a block signal opt in here to keep silence-is-permission intact. +- The flags compose. Use any alone or together. +- Identical semantics across every supported harness: Claude Code, Copilot CLI, Gemini CLI, OpenCode, Pi, and Codex. + +## Stdout contract + +``` + Flags │ UX │ Approve │ Close │ Annotate +──────────────────────────┼──────────────────┼─────────────────────────┼──────────────────────────┼─────────────────────────────────────────────── + (none) │ 2-button │ n/a │ empty │ feedback (plaintext) + --gate │ 3-button │ `The user approved.` │ empty │ feedback (plaintext) + --gate --silent-approve │ 3-button │ empty │ empty │ feedback (plaintext) + --json │ 2-button │ n/a │ {"decision":"dismissed"}│ {"decision":"annotated","feedback":"..."} + --gate --json │ 3-button │ {"decision":"approved"}│ {"decision":"dismissed"}│ {"decision":"annotated","feedback":"..."} +``` + +### JSON schema + +```json +{ + "decision": "approved" | "annotated" | "dismissed", + "feedback": "string (present only when decision is 'annotated')" +} +``` + +### Example outputs + +**Approved** (reviewer clicked Approve, `--gate --json`): + +```json +{"decision":"approved"} +``` + +**Dismissed** (reviewer clicked Close, `--json` or `--gate --json`): + +```json +{"decision":"dismissed"} +``` + +**Annotated** (reviewer sent annotations, `--json` or `--gate --json`). The `feedback` field is the same markdown Plannotator emits in plaintext mode: + +```json +{ + "decision": "annotated", + "feedback": "# File Feedback\n\nI've reviewed this file and have 2 pieces of feedback:\n\n## 1. Remove this\n`the selected text`\n> I don't want this.\n\n## 2. Feedback on: \"some highlighted text\"\n> This needs more detail.\n\n---" +} +``` + +The object is emitted as a single line of JSON per invocation. One invocation, one decision, one line on stdout. + +## `--gate` + +A three-way review decision. The annotation UI adds an Approve button alongside Close and Send Annotations. The reviewer declares intent explicitly: + +- **Approve.** The artifact is good as written. The agent should proceed. +- **Send Annotations.** The reviewer has specific changes. The feedback is returned verbatim. +- **Close.** The session ends without a decision. Neither a signal to the agent nor an instruction set. + +In plaintext mode, Approve emits the single line `The user approved.` on stdout so templates and agents can distinguish approval from close without needing `--json`. Close emits nothing. Send Annotations emits the feedback markdown. Hook authors who treat any non-empty stdout as a block signal can add `--silent-approve` to suppress the marker, or use `--json` for structured routing. + +## `--json` + +Structured stdout. Every decision is emitted as a JSON object with a `decision` field and optionally a `feedback` payload. Hooks and plugins that need explicit routing (log approvals separately from dismissals, gate on decision type, accumulate telemetry) use this. + +`--json` is orthogonal to `--gate`: + +- `--json` alone keeps the two-button UI. Only `annotated` and `dismissed` decisions are emitted. +- `--gate --json` unlocks all three decisions in structured form. +- On OpenCode and Pi, `--json` is accepted silently. Those harnesses write back to the session directly rather than via stdout, so the flag has no effect there. Recipes remain portable. + +## `--silent-approve` + +Suppresses the plaintext approve marker. With `--gate --silent-approve`, Approve emits empty stdout (instead of `The user approved.`), matching Close. Send Annotations still emits feedback. + +This is the shape naive hooks want — the ones that treat any non-empty stdout as a block signal: + +- Approve → empty → hook passes → agent proceeds. +- Close → empty → hook passes → agent proceeds. +- Send Annotations → feedback → hook blocks with that feedback as the reason. + +`--silent-approve` only affects plaintext mode. In `--json` mode, Approve still emits `{"decision":"approved"}` — JSON callers route on the `decision` field, so there's no ambiguity to silence. The flag is accepted silently on OpenCode and Pi for the same reason `--json` is: those harnesses don't use stdout as the signal channel. + +## Primary use cases + +### Spec-driven development frameworks + +Spec-driven development frameworks like spec-kit, kiro, and openspec generate multiple markdown artifacts per feature: `spec.md`, `plan.md`, `tasks.md`, `research.md`, `data-model.md`. Each goes through clarify, review, and approve cycles. Plannotator's annotation UI is a first-class fit for reviewing these artifacts: inline, targeted feedback on markdown is exactly what these workflows need. + +With `--gate`, a PostToolUse hook on Write triggers a full review gate every time the agent produces a spec artifact. The reviewer approves, annotates, or dismisses. The agent proceeds, revises, or skips accordingly. + +### Turn-by-turn review + +`plannotator annotate-last --gate` wired into a Claude Code Stop hook pauses every agent turn for human review. Approve closes the turn cleanly. Send Annotations re-prompts the agent with the reviewer's feedback. Close ends the turn without injecting anything. + +### Programmatic decision routing + +When a hook or plugin needs to distinguish approve from dismiss, `--json` provides a single-line, stable contract. One-shot decisions become machine-readable events. No stdout parsing, no fragility. + +## Hook integration recipes + +See [Hook Integration](/docs/guides/hook-integration/) for copy-paste recipes that wire these flags into PostToolUse and Stop hooks on Claude Code, plus portable variants for OpenCode and Pi. + +## Exit codes + +Every decision exits `0`. Signals live on stdout. This keeps Plannotator composable with harnesses that use exit codes for their own purposes. diff --git a/apps/marketing/src/content/docs/guides/hook-integration.md b/apps/marketing/src/content/docs/guides/hook-integration.md new file mode 100644 index 00000000..dc1150b9 --- /dev/null +++ b/apps/marketing/src/content/docs/guides/hook-integration.md @@ -0,0 +1,147 @@ +--- +title: "Hook Integration" +description: "Use plannotator annotate and annotate-last as review gates from agent hooks — spec-driven workflows, turn-by-turn review, and more." +sidebar: + order: 27 +section: "Guides" +--- + +The `--gate`, `--json`, and `--silent-approve` flags on `plannotator annotate` and `plannotator annotate-last` turn annotation into a structured review decision. This guide shows how to wire them into agent hooks so a human can gate the agent at specific points in a workflow. + +See [Annotate → Flags](/docs/commands/annotate/#flags) for the full stdout matrix. The short version: + +- `--gate` adds a three-button UX (Approve / Send Annotations / Close). +- Plaintext default: Approve emits the line `The user approved.`, Close emits empty stdout, Send Annotations emits the feedback markdown. Three distinguishable outputs without parsing JSON. +- `--silent-approve` collapses Approve to empty stdout, matching Close. Use this with naive "any stdout = block" hooks so silence means permission. +- `--json` emits every decision as a structured `{ "decision": "approved" | "annotated" | "dismissed", "feedback": "..." }` object. + +## Recipe 1: PostToolUse spec gate + +Spec-driven frameworks (spec-kit, kiro, openspec) generate multiple markdown artifacts per feature — spec.md, plan.md, tasks.md, and so on — each needing human review before the agent builds from it. A PostToolUse hook on Write turns plannotator into a reviewer in the loop. + +### Plaintext (naive) + +Add to `.claude/hooks.json`: + +```json +{ + "hooks": { + "PostToolUse": [ + { + "matcher": "Write", + "hooks": [ + { + "type": "command", + "command": "plannotator annotate \"$CLAUDE_TOOL_INPUT_file_path\" --gate", + "timeout": 345600 + } + ] + } + ] + } +} +``` + +Behavior: + +- **Approve** → `The user approved.` on stdout. Claude Code reports the line back and proceeds. +- **Send Annotations** → feedback markdown on stdout. Claude Code reports the feedback back. +- **Close** → empty stdout. Claude Code proceeds silently. + +### Silence-is-permission (`--silent-approve`) + +If your hook treats any non-empty stdout as a block signal (spec-kit and similar naive PostToolUse hooks), add `--silent-approve` so Approve also emits empty stdout: + +```json +"command": "plannotator annotate \"$CLAUDE_TOOL_INPUT_file_path\" --gate --silent-approve" +``` + +Behavior with the flag: + +- **Approve** → empty stdout → hook passes → agent proceeds. +- **Close** → empty stdout → hook passes → agent proceeds. +- **Send Annotations** → feedback on stdout → hook blocks with feedback as the reason. + +Approve and Close collapse into the same "silent = allow" cell, which is what this class of hook expects. Only Send Annotations carries content the agent needs to react to. + +### Structured (`--json`) + +If you want to route on decision type explicitly — for example, only re-prompt on `annotated` and log `approved` vs `dismissed` separately — pipe through `jq` or a small shell wrapper: + +```bash +#!/usr/bin/env bash +# .claude/hooks/spec-gate.sh +result=$(plannotator annotate "$CLAUDE_TOOL_INPUT_file_path" --gate --json) +decision=$(echo "$result" | jq -r '.decision') +feedback=$(echo "$result" | jq -r '.feedback // ""') + +case "$decision" in + approved|dismissed) + # empty stdout — hook passes through, agent proceeds + ;; + annotated) + # emit feedback on stdout so the hook blocks with it as the reason + echo "$feedback" + ;; +esac +``` + +Exit code stays `0` for all three branches; signaling happens via stdout (empty = pass, non-empty = block). This mirrors the `--gate`-without-`--json` mode exactly — JSON just gives you a parsed decision for logging or conditional routing without changing the block contract. + +## Recipe 2: Stop-hook turn gate + +Wire `annotate-last` to Claude Code's Stop hook to pause every agent turn for human review. + +### Plaintext + +```json +{ + "hooks": { + "Stop": [ + { + "matcher": "", + "hooks": [ + { + "type": "command", + "command": "plannotator annotate-last --gate", + "timeout": 345600 + } + ] + } + ] + } +} +``` + +Behavior: + +- **Approve** → `The user approved.` on stdout. Turn ends; Claude Code reports the marker. +- **Send Annotations** → feedback on stdout → Claude Code re-prompts with the feedback. +- **Close** → empty stdout → turn ends. + +Add `--silent-approve` if your Stop hook treats any stdout as a re-prompt trigger — Approve then emits empty stdout too, so only Send Annotations re-fires the turn with feedback. + +### Structured + +Same pattern as the PostToolUse recipe — pipe `--gate --json` through a shell wrapper if you want distinct handling per decision. + +## OpenCode and Pi + +The same `--gate` flag works in OpenCode's `/plannotator-annotate` and Pi's `/plannotator-annotate` slash commands: + +``` +/plannotator-annotate spec.md --gate +``` + +On those harnesses there is no stdout channel back to the agent — the plugin writes back via `session.prompt` (OpenCode) or `sendUserMessage` (Pi). Approve and Close both result in no session injection; Send Annotations injects the feedback. `--json` and `--silent-approve` are accepted silently on these harnesses so recipes stay portable. + +Third-party Pi or OpenCode plugins that want explicit decision routing can read `approved` directly from the server's decision object: + +- OpenCode plugin: `server.waitForDecision()` returns `{ feedback, annotations, exit?, approved? }`. +- Pi: `openMarkdownAnnotation()` and `openLastMessageAnnotation()` return `{ feedback, exit?, approved? }`. + +## Notes + +- Exit code is always `0`. Gate decisions are signaled via stdout, not exit code. +- Folder annotation with `--gate` applies one decision to the whole session (not per-file). The user navigates the file browser inside the UI, annotates across files, and submits once. +- The `--gate` UX is fully opt-in. Users running `/plannotator-annotate README.md` interactively without the flag still see the 2-button experience. diff --git a/apps/opencode-plugin/commands.ts b/apps/opencode-plugin/commands.ts index 0618e41f..26b2ae5a 100644 --- a/apps/opencode-plugin/commands.ts +++ b/apps/opencode-plugin/commands.ts @@ -24,6 +24,7 @@ import { loadConfig, resolveDefaultDiffType, resolveUseJina } from "@plannotator import { resolveMarkdownFile, resolveUserPath, hasMarkdownFiles } from "@plannotator/shared/resolve-file"; import { FILE_BROWSER_EXCLUDED } from "@plannotator/shared/reference-common"; import { htmlToMarkdown } from "@plannotator/shared/html-to-markdown"; +import { parseAnnotateArgs } from "@plannotator/shared/annotate-args"; import { urlToMarkdown } from "@plannotator/shared/url-to-markdown"; import { statSync } from "fs"; import path from "path"; @@ -152,17 +153,18 @@ export async function handleAnnotateCommand( const { client, htmlContent, getSharingEnabled, getShareBaseUrl, getPasteApiUrl, directory } = deps; // @ts-ignore - Event properties contain arguments - let filePath = event.properties?.arguments || event.arguments || ""; + const rawArgs = event.properties?.arguments || event.arguments || ""; + // #570: split --gate / --json out of the args; rest is the file path. + // --json is accepted silently (OpenCode writes to session, not stdout). + // parseAnnotateArgs strips leading @ on filePath (reference-mode convention). + // `rawFilePath` preserves it for the scoped-package markdown fallback. + const { filePath, rawFilePath, gate } = parseAnnotateArgs(rawArgs); if (!filePath) { - client.app.log({ level: "error", message: "Usage: /plannotator-annotate " }); + client.app.log({ level: "error", message: "Usage: /plannotator-annotate [--gate] [--json]" }); return; } - if (filePath.startsWith("@")) { - filePath = filePath.slice(1); - } - let markdown: string; let absolutePath: string; let folderPath: string | undefined; @@ -206,7 +208,6 @@ export async function handleAnnotateCommand( annotateMode = "annotate-folder"; client.app.log({ level: "info", message: `Opening annotation UI for folder ${resolvedArg}...` }); } else if (/\.html?$/i.test(resolvedArg)) { - // HTML file annotation — convert to markdown via Turndown let fileSize: number; try { fileSize = statSync(resolvedArg).size; @@ -226,7 +227,11 @@ export async function handleAnnotateCommand( } else { // Markdown file annotation client.app.log({ level: "info", message: `Opening annotation UI for ${filePath}...` }); - const resolved = await resolveMarkdownFile(filePath, projectRoot); + // Strip-first with literal-@ fallback (scoped-package-style names). + let resolved = await resolveMarkdownFile(filePath, projectRoot); + if (resolved.kind === "not_found" && rawFilePath !== filePath) { + resolved = await resolveMarkdownFile(rawFilePath, projectRoot); + } if (resolved.kind === "ambiguous") { client.app.log({ @@ -256,6 +261,7 @@ export async function handleAnnotateCommand( sharingEnabled: await getSharingEnabled(), shareBaseUrl: getShareBaseUrl(), pasteApiUrl: getPasteApiUrl(), + gate, htmlContent, onReady: handleAnnotateServerReady, }); @@ -264,7 +270,8 @@ export async function handleAnnotateCommand( await Bun.sleep(1500); server.stop(); - if (result.exit) { + // Both exit and approve are "no-op for the agent" — skip session injection. + if (result.exit || result.approved) { return; } @@ -301,6 +308,11 @@ export async function handleAnnotateLastCommand( ): Promise { const { client, htmlContent, getSharingEnabled, getShareBaseUrl, getPasteApiUrl } = deps; + // @ts-ignore - Event properties contain arguments + const rawArgs = event.properties?.arguments || event.arguments || ""; + // #570: support --gate on /plannotator-last (Stop-hook review-gate pattern). + const { gate } = parseAnnotateArgs(rawArgs); + // @ts-ignore - Event properties contain sessionID const sessionId = event.properties?.sessionID; if (!sessionId) { @@ -346,6 +358,7 @@ export async function handleAnnotateLastCommand( sharingEnabled: await getSharingEnabled(), shareBaseUrl: getShareBaseUrl(), pasteApiUrl: getPasteApiUrl(), + gate, htmlContent, onReady: handleAnnotateServerReady, }); @@ -354,7 +367,8 @@ export async function handleAnnotateLastCommand( await Bun.sleep(1500); server.stop(); - if (result.exit) { + // Both exit and approve signal "don't inject feedback" — return null. + if (result.exit || result.approved) { return null; } diff --git a/apps/opencode-plugin/index.ts b/apps/opencode-plugin/index.ts index 91e20df2..f8678e55 100644 --- a/apps/opencode-plugin/index.ts +++ b/apps/opencode-plugin/index.ts @@ -376,7 +376,9 @@ Do NOT proceed with implementation until your plan is approved.`); }; const feedback = await handleAnnotateLastCommand( - { properties: { sessionID: input.sessionID } }, + // input.arguments is the raw tail string from OpenCode's command dispatcher — + // needed so --gate / --json reach handleAnnotateLastCommand's parseAnnotateArgs (#570). + { properties: { sessionID: input.sessionID, arguments: input.arguments } }, deps ); diff --git a/apps/pi-extension/index.ts b/apps/pi-extension/index.ts index 220e5b25..bb95fc06 100644 --- a/apps/pi-extension/index.ts +++ b/apps/pi-extension/index.ts @@ -37,6 +37,8 @@ import { FILE_BROWSER_EXCLUDED } from "./generated/reference-common.js"; import { htmlToMarkdown } from "./generated/html-to-markdown.js"; import { urlToMarkdown } from "./generated/url-to-markdown.js"; import { loadConfig, resolveUseJina } from "./generated/config.js"; +import { parseAnnotateArgs } from "./generated/annotate-args.js"; +import { resolveAtReference } from "./generated/at-reference.js"; import { getLastAssistantMessageText, hasPlanBrowserHtml, @@ -337,9 +339,13 @@ export default function plannotator(pi: ExtensionAPI): void { pi.registerCommand("plannotator-annotate", { description: "Open markdown file or folder in annotation UI", handler: async (args, ctx) => { - const filePath = args?.trim(); + // #570: split --gate / --json from the path. --json is silently + // accepted (Pi writes back via sendUserMessage, not stdout). + // `rawFilePath` keeps any leading `@` for the literal-@ fallback + // (scoped-package-style names). + const { filePath, rawFilePath, gate } = parseAnnotateArgs(args ?? ""); if (!filePath) { - ctx.ui.notify("Usage: /plannotator-annotate ", "error"); + ctx.ui.notify("Usage: /plannotator-annotate [--gate] [--json]", "error"); return; } if (!hasPlanBrowserHtml()) { @@ -373,11 +379,20 @@ export default function plannotator(pi: ExtensionAPI): void { absolutePath = filePath; sourceInfo = filePath; } else { - absolutePath = resolveUserPath(filePath, ctx.cwd); - if (!existsSync(absolutePath)) { + // Pick the interpretation of the user input that actually exists: + // stripped form first (reference-mode primary), literal as fallback + // for scoped-package-style names. Falls back to the stripped form + // for the error message if neither exists. + const resolvedCandidate = resolveAtReference(rawFilePath, (c) => { + const abs = resolveUserPath(c, ctx.cwd); + return existsSync(abs); + }); + if (resolvedCandidate === null) { + absolutePath = resolveUserPath(filePath, ctx.cwd); ctx.ui.notify(`File not found: ${absolutePath}`, "error"); return; } + absolutePath = resolveUserPath(resolvedCandidate, ctx.cwd); try { isFolder = statSync(absolutePath).isDirectory(); @@ -413,8 +428,10 @@ export default function plannotator(pi: ExtensionAPI): void { } try { - const result = await openMarkdownAnnotation(ctx, absolutePath, markdown, mode ?? "annotate", folderPath, sourceInfo); - if (result.exit) { + const result = await openMarkdownAnnotation(ctx, absolutePath, markdown, mode ?? "annotate", folderPath, sourceInfo, gate); + if (result.approved) { + ctx.ui.notify("Annotation approved.", "info"); + } else if (result.exit) { ctx.ui.notify("Annotation session closed.", "info"); } else if (result.feedback) { const header = isFolder @@ -437,7 +454,10 @@ export default function plannotator(pi: ExtensionAPI): void { pi.registerCommand("plannotator-last", { description: "Annotate the last assistant message", - handler: async (_args, ctx) => { + handler: async (args, ctx) => { + // #570: support --gate on /plannotator-last for Stop-hook review gate. + const { gate } = parseAnnotateArgs(args ?? ""); + if (!hasPlanBrowserHtml()) { ctx.ui.notify( "Annotation UI not available. Run 'bun run build' in the pi-extension directory.", @@ -455,8 +475,10 @@ export default function plannotator(pi: ExtensionAPI): void { ctx.ui.notify("Opening annotation UI for last message...", "info"); try { - const result = await openLastMessageAnnotation(ctx, lastText); - if (result.exit) { + const result = await openLastMessageAnnotation(ctx, lastText, gate); + if (result.approved) { + ctx.ui.notify("Message approved.", "info"); + } else if (result.exit) { ctx.ui.notify("Annotation session closed.", "info"); } else if (result.feedback) { pi.sendUserMessage( diff --git a/apps/pi-extension/plannotator-browser.ts b/apps/pi-extension/plannotator-browser.ts index 4032959f..db4a7a4b 100644 --- a/apps/pi-extension/plannotator-browser.ts +++ b/apps/pi-extension/plannotator-browser.ts @@ -370,7 +370,8 @@ export async function openMarkdownAnnotation( mode: AnnotateMode, folderPath?: string, sourceInfo?: string, -): Promise<{ feedback: string; exit?: boolean }> { + gate?: boolean, +): Promise<{ feedback: string; exit?: boolean; approved?: boolean }> { if (!ctx.hasUI || !planHtmlContent) { throw new Error("Plannotator annotation browser is unavailable in this session."); } @@ -394,6 +395,7 @@ export async function openMarkdownAnnotation( mode, folderPath, sourceInfo, + gate, htmlContent: planHtmlContent, sharingEnabled: process.env.PLANNOTATOR_SHARE !== "disabled", shareBaseUrl: process.env.PLANNOTATOR_SHARE_URL || undefined, @@ -406,8 +408,9 @@ export async function openMarkdownAnnotation( export async function openLastMessageAnnotation( ctx: ExtensionContext, lastText: string, -): Promise<{ feedback: string; exit?: boolean }> { - return openMarkdownAnnotation(ctx, "last-message", lastText, "annotate-last"); + gate?: boolean, +): Promise<{ feedback: string; exit?: boolean; approved?: boolean }> { + return openMarkdownAnnotation(ctx, "last-message", lastText, "annotate-last", undefined, undefined, gate); } export async function openArchiveBrowserAction( diff --git a/apps/pi-extension/plannotator-events.ts b/apps/pi-extension/plannotator-events.ts index defe87d4..2b94d3d5 100644 --- a/apps/pi-extension/plannotator-events.ts +++ b/apps/pi-extension/plannotator-events.ts @@ -100,10 +100,16 @@ export interface PlannotatorAnnotatePayload { markdown?: string; mode?: "annotate" | "annotate-folder" | "annotate-last"; folderPath?: string; + /** Enable review-gate UX (Approve / Annotate / Close), #570 */ + gate?: boolean; } export interface PlannotatorAnnotationResult { feedback: string; + /** True when the reviewer closed the session without providing feedback. */ + exit?: boolean; + /** True when the reviewer clicked Approve in review-gate mode, #570 */ + approved?: boolean; } export interface PlannotatorArchivePayload { @@ -272,6 +278,8 @@ export function registerPlannotatorEventListeners(pi: ExtensionAPI): void { payload.markdown ?? "", payload.mode ?? "annotate", payload.folderPath, + undefined, + payload.gate, ); request.respond({ status: "handled", result }); return; @@ -283,7 +291,7 @@ export function registerPlannotatorEventListeners(pi: ExtensionAPI): void { request.respond({ status: "unavailable", error: "No assistant message found in session." }); return; } - const result = await openLastMessageAnnotation(ctx, lastText); + const result = await openLastMessageAnnotation(ctx, lastText, payload?.gate); request.respond({ status: "handled", result }); return; } diff --git a/apps/pi-extension/server/serverAnnotate.ts b/apps/pi-extension/server/serverAnnotate.ts index 617d474c..75e3e9ad 100644 --- a/apps/pi-extension/server/serverAnnotate.ts +++ b/apps/pi-extension/server/serverAnnotate.ts @@ -28,7 +28,7 @@ export interface AnnotateServerResult { port: number; portSource: "env" | "remote-default" | "random"; url: string; - waitForDecision: () => Promise<{ feedback: string; annotations: unknown[]; exit?: boolean }>; + waitForDecision: () => Promise<{ feedback: string; annotations: unknown[]; exit?: boolean; approved?: boolean }>; stop: () => void; } @@ -43,6 +43,7 @@ export async function startAnnotateServer(options: { shareBaseUrl?: string; pasteApiUrl?: string; sourceInfo?: string; + gate?: boolean; }): Promise { const gitUser = detectGitUser(); const sharingEnabled = @@ -56,11 +57,13 @@ export async function startAnnotateServer(options: { feedback: string; annotations: unknown[]; exit?: boolean; + approved?: boolean; }) => void; const decisionPromise = new Promise<{ feedback: string; annotations: unknown[]; exit?: boolean; + approved?: boolean; }>((r) => { resolveDecision = r; }); @@ -89,6 +92,7 @@ export async function startAnnotateServer(options: { mode: options.mode || "annotate", filePath: options.filePath, sourceInfo: options.sourceInfo, + gate: options.gate ?? false, sharingEnabled, shareBaseUrl, pasteApiUrl, @@ -135,6 +139,10 @@ export async function startAnnotateServer(options: { deleteDraft(draftKey); resolveDecision({ feedback: "", annotations: [], exit: true }); json(res, { ok: true }); + } else if (url.pathname === "/api/approve" && req.method === "POST") { + deleteDraft(draftKey); + resolveDecision({ feedback: "", annotations: [], approved: true }); + json(res, { ok: true }); } else if (url.pathname === "/api/feedback" && req.method === "POST") { try { const body = await parseBody(req); diff --git a/apps/pi-extension/vendor.sh b/apps/pi-extension/vendor.sh index 51dbc426..48cb847c 100755 --- a/apps/pi-extension/vendor.sh +++ b/apps/pi-extension/vendor.sh @@ -6,7 +6,7 @@ cd "$(dirname "$0")" mkdir -p generated generated/ai/providers -for f in feedback-templates review-core storage draft project pr-provider pr-github pr-gitlab checklist integrations-common repo reference-common favicon resolve-file config external-annotation agent-jobs worktree html-to-markdown url-to-markdown tour; do +for f in feedback-templates review-core storage draft project pr-provider pr-github pr-gitlab checklist integrations-common repo reference-common favicon resolve-file config external-annotation agent-jobs worktree html-to-markdown url-to-markdown tour annotate-args at-reference; 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/bun.lock b/bun.lock index 2ff0a627..c87c0427 100644 --- a/bun.lock +++ b/bun.lock @@ -64,7 +64,7 @@ }, "apps/opencode-plugin": { "name": "@plannotator/opencode", - "version": "0.18.0", + "version": "0.19.0", "dependencies": { "@opencode-ai/plugin": "^1.1.10", }, @@ -86,7 +86,7 @@ }, "apps/pi-extension": { "name": "@plannotator/pi-extension", - "version": "0.18.0", + "version": "0.19.0", "dependencies": { "@joplin/turndown-plugin-gfm": "^1.0.64", "turndown": "^7.2.4", @@ -186,7 +186,7 @@ }, "packages/server": { "name": "@plannotator/server", - "version": "0.18.0", + "version": "0.19.0", "dependencies": { "@plannotator/ai": "workspace:*", "@plannotator/shared": "workspace:*", diff --git a/packages/editor/App.tsx b/packages/editor/App.tsx index 5ade5dca..f984f138 100644 --- a/packages/editor/App.tsx +++ b/packages/editor/App.tsx @@ -99,6 +99,8 @@ const App: React.FC = () => { const [showFeedbackPrompt, setShowFeedbackPrompt] = useState(false); const [showClaudeCodeWarning, setShowClaudeCodeWarning] = useState(false); const [showExitWarning, setShowExitWarning] = useState(false); + // When the warning dialog confirms, route to the handler matching the button that opened it. + const [exitWarningAction, setExitWarningAction] = useState<'close' | 'approve'>('close'); const [showAgentWarning, setShowAgentWarning] = useState(false); const [agentWarningMessage, setAgentWarningMessage] = useState(''); const [isPanelOpen, setIsPanelOpen] = useState(() => window.innerWidth >= 768); @@ -147,6 +149,7 @@ const App: React.FC = () => { const [isWSL, setIsWSL] = useState(false); const [globalAttachments, setGlobalAttachments] = useState([]); const [annotateMode, setAnnotateMode] = useState(false); + const [gate, setGate] = useState(false); const [annotateSource, setAnnotateSource] = useState<'file' | 'message' | 'folder' | null>(null); const [sourceInfo, setSourceInfo] = useState(); const [sourceFilePath, setSourceFilePath] = useState(); @@ -540,6 +543,15 @@ const App: React.FC = () => { // Plan diff state — memoize filtered annotation lists to avoid new references per render const diffAnnotations = useMemo(() => allAnnotations.filter(a => !!a.diffContext), [allAnnotations]); const viewerAnnotations = useMemo(() => allAnnotations.filter(a => !a.diffContext), [allAnnotations]); + // Any-annotations flag used by Close/Approve/Send guards. Consolidates the + // four-term check that was inlined across the annotate-mode header + keyboard paths. + const hasAnyAnnotations = useMemo( + () => allAnnotations.length > 0 + || editorAnnotations.length > 0 + || linkedDocHook.docAnnotationCount > 0 + || globalAttachments.length > 0, + [allAnnotations.length, editorAnnotations.length, linkedDocHook.docAnnotationCount, globalAttachments.length], + ); // URL-based sharing const { @@ -642,7 +654,7 @@ 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; host?: string }; previousPlan?: string | null; versionInfo?: { version: number; totalVersions: number; project: string }; archivePlans?: ArchivedPlan[]; projectRoot?: string; isWSL?: boolean; serverConfig?: { displayName?: string; gitUser?: string } }) => { + .then((data: { plan: string; origin?: Origin; mode?: 'annotate' | 'annotate-last' | 'annotate-folder' | 'archive'; filePath?: string; sourceInfo?: string; gate?: boolean; sharingEnabled?: boolean; shareBaseUrl?: string; pasteApiUrl?: string; repoInfo?: { display: string; branch?: string; host?: string }; previousPlan?: string | null; versionInfo?: { version: number; totalVersions: number; project: string }; archivePlans?: ArchivedPlan[]; projectRoot?: string; isWSL?: boolean; serverConfig?: { displayName?: string; gitUser?: string } }) => { // 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 @@ -663,6 +675,7 @@ const App: React.FC = () => { setIsApiMode(true); if (data.mode === 'annotate' || data.mode === 'annotate-last' || data.mode === 'annotate-folder') { setAnnotateMode(true); + setGate(data.gate ?? false); } if (data.mode === 'annotate-folder') { sidebar.open('files'); @@ -989,6 +1002,17 @@ const App: React.FC = () => { } }; + // Annotate gate-mode handler — approves the artifact without feedback (#570) + const handleAnnotateApprove = async () => { + setIsSubmitting(true); + try { + await fetch('/api/approve', { method: 'POST' }); + setSubmitted('approved'); + } catch { + setIsSubmitting(false); + } + }; + // Exit annotation session without sending feedback const handleAnnotateExit = useCallback(async () => { setIsExiting(true); @@ -1029,8 +1053,13 @@ const App: React.FC = () => { e.preventDefault(); - // Annotate mode: always send feedback (empty = "no feedback" message) + // Annotate mode: gate-enabled + no annotations → approve (empty stdout). + // Otherwise: send feedback. if (annotateMode) { + if (gate && !hasAnyAnnotations) { + handleAnnotateApprove(); + return; + } handleAnnotateFeedback(); return; } @@ -1062,6 +1091,7 @@ const App: React.FC = () => { showExport, showImport, showFeedbackPrompt, showClaudeCodeWarning, showExitWarning, showAgentWarning, showPermissionModeSetup, pendingPasteImage, submitted, isSubmitting, isExiting, isApiMode, linkedDocHook.isActive, annotations.length, externalAnnotations.length, annotateMode, + gate, hasAnyAnnotations, origin, getAgentWarning, ]); @@ -1422,14 +1452,22 @@ const App: React.FC = () => { {isApiMode && (!linkedDocHook.isActive || annotateMode) && !archive.archiveMode && ( <> {annotateMode ? ( - // Annotate mode: Close always visible, Send Annotations when annotations exist + // Annotate mode: Close always visible, Send Annotations when annotations exist, + // Approve only when gate (review) mode is enabled (#570). <> (allAnnotations.length > 0 || editorAnnotations.length > 0 || linkedDocHook.docAnnotationCount > 0 || globalAttachments.length > 0) ? setShowExitWarning(true) : handleAnnotateExit()} + onClick={() => { + if (hasAnyAnnotations) { + setExitWarningAction('close'); + setShowExitWarning(true); + } else { + handleAnnotateExit(); + } + }} disabled={isSubmitting || isExiting} isLoading={isExiting} /> - {(allAnnotations.length > 0 || editorAnnotations.length > 0 || linkedDocHook.docAnnotationCount > 0 || globalAttachments.length > 0) && ( + {hasAnyAnnotations && ( { /> )} - {!annotateMode &&
+ {(!annotateMode || gate) &&
{ + // Annotate gate mode: guard against dropping annotations via the existing + // showExitWarning dialog (routed via exitWarningAction='approve'). + if (annotateMode) { + if (hasAnyAnnotations) { + setExitWarningAction('approve'); + setShowExitWarning(true); + return; + } + handleAnnotateApprove(); + return; + } + // Plan mode: existing Claude-Code / OpenCode guards. if (origin === 'claude-code' && allAnnotations.length > 0) { setShowClaudeCodeWarning(true); return; @@ -1477,11 +1527,12 @@ const App: React.FC = () => { } handleApprove(); }} - disabled={isSubmitting} + disabled={isSubmitting || (annotateMode && isExiting)} isLoading={isSubmitting} - dimmed={(origin === 'claude-code' || origin === 'gemini-cli') && allAnnotations.length > 0} + dimmed={!annotateMode && (origin === 'claude-code' || origin === 'gemini-cli') && allAnnotations.length > 0} + title={annotateMode ? 'Approve — no changes requested' : undefined} /> - {(origin === 'claude-code' || origin === 'gemini-cli') && allAnnotations.length > 0 && ( + {!annotateMode && (origin === 'claude-code' || origin === 'gemini-cli') && allAnnotations.length > 0 && (
@@ -1879,18 +1930,19 @@ const App: React.FC = () => { showCancel /> - {/* Exit with annotations warning dialog */} + {/* Unsaved-annotations warning dialog — reused by Close and (in gate mode) Approve */} setShowExitWarning(false)} onConfirm={() => { setShowExitWarning(false); - handleAnnotateExit(); + if (exitWarningAction === 'approve') handleAnnotateApprove(); + else handleAnnotateExit(); }} title="Annotations Won't Be Sent" - message={<>You have {allAnnotations.length + editorAnnotations.length + linkedDocHook.docAnnotationCount + globalAttachments.length} annotation{(allAnnotations.length + editorAnnotations.length + linkedDocHook.docAnnotationCount + globalAttachments.length) !== 1 ? 's' : ''} that will be lost if you close.} + message={<>You have {allAnnotations.length + editorAnnotations.length + linkedDocHook.docAnnotationCount + globalAttachments.length} annotation{(allAnnotations.length + editorAnnotations.length + linkedDocHook.docAnnotationCount + globalAttachments.length) !== 1 ? 's' : ''} that will be lost if you {exitWarningAction === 'approve' ? 'approve' : 'close'}.} subMessage="To send your annotations, use Send Annotations instead." - confirmText="Close Anyway" + confirmText={exitWarningAction === 'approve' ? 'Approve Anyway' : 'Close Anyway'} cancelText="Cancel" variant="warning" showCancel @@ -1944,8 +1996,9 @@ const App: React.FC = () => { title={ archive.archiveMode ? 'Archive Closed' : submitted === 'exited' ? 'Session Closed' - : submitted === 'approved' ? 'Plan Approved' - : annotateMode ? 'Annotations Sent' + : submitted === 'approved' + ? (annotateMode ? 'Approved' : 'Plan Approved') + : annotateMode ? 'Annotations Sent' : 'Feedback Sent' } subtitle={ @@ -1954,7 +2007,9 @@ const App: React.FC = () => { : archive.archiveMode ? 'You can reopen with plannotator archive.' : submitted === 'approved' - ? `${agentName} will proceed with the implementation.` + ? (annotateMode + ? `${agentName} will proceed.` + : `${agentName} will proceed with the implementation.`) : annotateMode ? `${agentName} will address your annotations on the ${annotateSource === 'message' ? 'message' : annotateSource === 'folder' ? 'files' : 'file'}.` : `${agentName} will revise the plan based on your annotations.` diff --git a/packages/server/annotate.ts b/packages/server/annotate.ts index 0daa9fcf..2370a35c 100644 --- a/packages/server/annotate.ts +++ b/packages/server/annotate.ts @@ -50,6 +50,8 @@ export interface AnnotateServerOptions { pasteApiUrl?: string; /** Source attribution: original URL or filename (e.g. "https://..." or "index.html") */ sourceInfo?: string; + /** Enable review-gate UX: adds an Approve button alongside Close/Send Annotations (#570) */ + gate?: boolean; /** Called when server starts with the URL, remote status, and port */ onReady?: (url: string, isRemote: boolean, port: number) => void; } @@ -66,6 +68,7 @@ export interface AnnotateServerResult { feedback: string; annotations: unknown[]; exit?: boolean; + approved?: boolean; }>; /** Stop the server */ stop: () => void; @@ -98,6 +101,7 @@ export async function startAnnotateServer( sharingEnabled = true, shareBaseUrl, pasteApiUrl, + gate = false, onReady, } = options; @@ -120,11 +124,13 @@ export async function startAnnotateServer( feedback: string; annotations: unknown[]; exit?: boolean; + approved?: boolean; }) => void; const decisionPromise = new Promise<{ feedback: string; annotations: unknown[]; exit?: boolean; + approved?: boolean; }>((resolve) => { resolveDecision = resolve; }); @@ -149,6 +155,7 @@ export async function startAnnotateServer( mode, filePath, sourceInfo, + gate, sharingEnabled, shareBaseUrl, pasteApiUrl, @@ -237,6 +244,13 @@ export async function startAnnotateServer( return Response.json({ ok: true }); } + // API: Approve the annotation session (review-gate UX, #570) + if (url.pathname === "/api/approve" && req.method === "POST") { + deleteDraft(draftKey); + resolveDecision({ feedback: "", annotations: [], approved: true }); + return Response.json({ ok: true }); + } + // API: Submit annotation feedback if (url.pathname === "/api/feedback" && req.method === "POST") { try { diff --git a/packages/shared/annotate-args.test.ts b/packages/shared/annotate-args.test.ts new file mode 100644 index 00000000..ad566972 --- /dev/null +++ b/packages/shared/annotate-args.test.ts @@ -0,0 +1,287 @@ +import { describe, test, expect } from "bun:test"; +import { parseAnnotateArgs } from "./annotate-args"; + +describe("parseAnnotateArgs", () => { + test("path only", () => { + expect(parseAnnotateArgs("spec.md")).toEqual({ + filePath: "spec.md", + rawFilePath: "spec.md", + gate: false, + json: false, + silentApprove: false, + }); + }); + + test("path with --gate at end", () => { + expect(parseAnnotateArgs("spec.md --gate")).toEqual({ + filePath: "spec.md", + rawFilePath: "spec.md", + gate: true, + json: false, + silentApprove: false, + }); + }); + + test("--gate before path", () => { + expect(parseAnnotateArgs("--gate spec.md")).toEqual({ + filePath: "spec.md", + rawFilePath: "spec.md", + gate: true, + json: false, + silentApprove: false, + }); + }); + + test("path with both flags", () => { + expect(parseAnnotateArgs("spec.md --gate --json")).toEqual({ + filePath: "spec.md", + rawFilePath: "spec.md", + gate: true, + json: true, + silentApprove: false, + }); + }); + + test("flags only, no path", () => { + expect(parseAnnotateArgs("--gate --json")).toEqual({ + filePath: "", + rawFilePath: "", + gate: true, + json: true, + silentApprove: false, + }); + }); + + test("path with spaces rejoins with single space", () => { + expect(parseAnnotateArgs("my file.md --gate")).toEqual({ + filePath: "my file.md", + rawFilePath: "my file.md", + gate: true, + json: false, + silentApprove: false, + }); + }); + + // `@` is the reference-mode marker (Claude Code / OpenCode / Pi convention), + // not part of the filename. The parser strips it on `filePath` as the primary + // behavior — that's the common case. `rawFilePath` preserves the original + // for callers that want to try the literal form as a fallback (scoped-package- + // style names). See at-reference.ts for the combined helper. + + test("leading @ is stripped (reference-mode primary) and rawFilePath preserves it", () => { + expect(parseAnnotateArgs("@spec.md --gate")).toEqual({ + filePath: "spec.md", + rawFilePath: "@spec.md", + gate: true, + json: false, + silentApprove: false, + }); + }); + + test("scoped-package-style path: filePath stripped, rawFilePath literal", () => { + expect(parseAnnotateArgs("@plannotator/ui/README.md")).toEqual({ + filePath: "plannotator/ui/README.md", + rawFilePath: "@plannotator/ui/README.md", + gate: false, + json: false, + silentApprove: false, + }); + }); + + test("@ stripped on filePath when combined with --gate --json, raw preserved", () => { + expect(parseAnnotateArgs("@docs/spec.md --gate --json")).toEqual({ + filePath: "docs/spec.md", + rawFilePath: "@docs/spec.md", + gate: true, + json: true, + silentApprove: false, + }); + }); + + test("URL passes through", () => { + expect(parseAnnotateArgs("https://example.com/docs --gate")).toEqual({ + filePath: "https://example.com/docs", + rawFilePath: "https://example.com/docs", + gate: true, + json: false, + silentApprove: false, + }); + }); + + test("extra whitespace is collapsed", () => { + expect(parseAnnotateArgs(" spec.md --gate ")).toEqual({ + filePath: "spec.md", + rawFilePath: "spec.md", + gate: true, + json: false, + silentApprove: false, + }); + }); + + test("empty string produces empty result", () => { + expect(parseAnnotateArgs("")).toEqual({ + filePath: "", + rawFilePath: "", + gate: false, + json: false, + silentApprove: false, + }); + }); + + test("nullish input is tolerated", () => { + expect(parseAnnotateArgs(undefined as unknown as string)).toEqual({ + filePath: "", + rawFilePath: "", + gate: false, + json: false, + silentApprove: false, + }); + }); + + test("folder path with trailing slash", () => { + expect(parseAnnotateArgs("./specs/ --gate --json")).toEqual({ + filePath: "./specs/", + rawFilePath: "./specs/", + gate: true, + json: true, + silentApprove: false, + }); + }); + + // Regressions from the initial parser: the tokenize-and-rejoin approach + // collapsed consecutive whitespace in file paths. Before this branch, + // OpenCode and Pi passed the raw args string straight through, so files + // with double-spaces or tabs in their names worked fine. These tests pin + // that behavior so we don't regress it again. + + test("double-space inside a file path is preserved (flag at end)", () => { + expect(parseAnnotateArgs("My Notes.md --gate")).toEqual({ + filePath: "My Notes.md", + rawFilePath: "My Notes.md", + gate: true, + json: false, + silentApprove: false, + }); + }); + + test("double-space inside a file path is preserved (flag at start)", () => { + expect(parseAnnotateArgs("--gate My Notes.md")).toEqual({ + filePath: "My Notes.md", + rawFilePath: "My Notes.md", + gate: true, + json: false, + silentApprove: false, + }); + }); + + test("tab inside a file path is preserved", () => { + expect(parseAnnotateArgs("My\tNotes.md --gate")).toEqual({ + filePath: "My\tNotes.md", + rawFilePath: "My\tNotes.md", + gate: true, + json: false, + silentApprove: false, + }); + }); + + test("multi-whitespace path with no flags passes through untouched", () => { + expect(parseAnnotateArgs("/tmp/My Notes.md")).toEqual({ + filePath: "/tmp/My Notes.md", + rawFilePath: "/tmp/My Notes.md", + gate: false, + json: false, + silentApprove: false, + }); + }); + + // OpenCode and Pi don't go through a shell, so users who quote paths + // (shell muscle memory, copy-paste from docs) have literal quote + // characters reach the parser. Strip them at the tokenization layer + // so downstream callers don't have to reason about quoting. + + test("wrapping double quotes are stripped from both filePath and rawFilePath", () => { + expect(parseAnnotateArgs(`"@foo.md" --gate`)).toEqual({ + filePath: "foo.md", + rawFilePath: "@foo.md", + gate: true, + json: false, + silentApprove: false, + }); + }); + + test("wrapping single quotes are stripped", () => { + expect(parseAnnotateArgs(`'@foo.md' --gate`)).toEqual({ + filePath: "foo.md", + rawFilePath: "@foo.md", + gate: true, + json: false, + silentApprove: false, + }); + }); + + test("wrapping quotes around a path with spaces", () => { + expect(parseAnnotateArgs(`"@My Notes.md" --gate`)).toEqual({ + filePath: "My Notes.md", + rawFilePath: "@My Notes.md", + gate: true, + json: false, + silentApprove: false, + }); + }); + + test("wrapping quotes without @ still get stripped", () => { + expect(parseAnnotateArgs(`"My Notes.md"`)).toEqual({ + filePath: "My Notes.md", + rawFilePath: "My Notes.md", + gate: false, + json: false, + silentApprove: false, + }); + }); + + // --silent-approve (issue #570 follow-up) opts the plaintext Approve out of + // emitting the "The user approved." marker. It parses identically to the + // other flags and is recognized in any position, including without --gate + // (where it's a documented no-op — the parser still strips it so it doesn't + // leak into the path). + + test("--silent-approve alongside --gate", () => { + expect(parseAnnotateArgs("spec.md --gate --silent-approve")).toEqual({ + filePath: "spec.md", + rawFilePath: "spec.md", + gate: true, + json: false, + silentApprove: true, + }); + }); + + test("--silent-approve with all three flags", () => { + expect(parseAnnotateArgs("spec.md --gate --json --silent-approve")).toEqual({ + filePath: "spec.md", + rawFilePath: "spec.md", + gate: true, + json: true, + silentApprove: true, + }); + }); + + test("--silent-approve alone is stripped from path (no-op but not leaked)", () => { + expect(parseAnnotateArgs("spec.md --silent-approve")).toEqual({ + filePath: "spec.md", + rawFilePath: "spec.md", + gate: false, + json: false, + silentApprove: true, + }); + }); + + test("--silent-approve before path", () => { + expect(parseAnnotateArgs("--silent-approve --gate spec.md")).toEqual({ + filePath: "spec.md", + rawFilePath: "spec.md", + gate: true, + json: false, + silentApprove: true, + }); + }); +}); diff --git a/packages/shared/annotate-args.ts b/packages/shared/annotate-args.ts new file mode 100644 index 00000000..0658a34d --- /dev/null +++ b/packages/shared/annotate-args.ts @@ -0,0 +1,101 @@ +/** + * Parse CLI-style args arriving as a single whitespace-delimited string. + * + * Extracts the `--gate`, `--json`, and `--silent-approve` flags (issue #570) + * from the remainder, which is treated as the target path. Leading `@` is + * stripped via the shared at-reference helper — reference-mode is primary. + * Scoped-package-style literal `@` paths are handled by a fallback that the + * downstream resolver opts into (see at-reference.ts). + * + * Used by the OpenCode plugin and Pi extension, where the whole args string + * arrives pre-joined from the harness slash-command dispatcher. The Claude + * Code binary parses argv directly with indexOf/splice and does not use + * this helper. + * + * Implementation: walks the raw string once, preserving whitespace runs and + * non-whitespace tokens as separate segments. Only known flag tokens + * (whole-word match) plus one adjacent whitespace run are removed. + * This keeps double-spaces and tabs inside file paths intact — which + * matches the pre-PR behavior on `main`, where OpenCode and Pi passed + * the raw args string straight through to the filesystem resolver. + * + * Remaining edge: if a path literally contains a known flag as a standalone + * whitespace-separated token (e.g. `"Feature --gate spec.md"`), that token + * is stripped. Supporting this would need shell-style quoting, which isn't + * worth the complexity for a vanishingly rare naming pattern. + */ + +import { stripAtPrefix } from "./at-reference"; +import { stripWrappingQuotes } from "./resolve-file"; + +export interface ParsedAnnotateArgs { + /** + * Primary resolution path with any leading `@` stripped (reference-mode + * convention). Most call sites should use this directly. + */ + filePath: string; + /** + * Raw path with the `@` prefix preserved (if the user supplied one). + * Callers that want the literal-`@` fallback for scoped-package-style + * paths pair this with `resolveAtReference` from at-reference.ts. + */ + rawFilePath: string; + gate: boolean; + json: boolean; + silentApprove: boolean; +} + +type Segment = { type: "ws" | "tok"; text: string }; + +const FLAG_MAP = { + "--gate": "gate", + "--json": "json", + "--silent-approve": "silentApprove", +} as const satisfies Record>; + +export function parseAnnotateArgs(raw: string): ParsedAnnotateArgs { + const s = (raw ?? "").trim(); + const flags = { gate: false, json: false, silentApprove: false }; + + const segments: Segment[] = []; + for (let i = 0; i < s.length;) { + const isWs = /\s/.test(s[i]); + const start = i; + while (i < s.length && /\s/.test(s[i]) === isWs) i++; + segments.push({ type: isWs ? "ws" : "tok", text: s.slice(start, i) }); + } + + const keep = segments.map(() => true); + for (let j = 0; j < segments.length; j++) { + const seg = segments[j]; + if (seg.type !== "tok") continue; + const key = FLAG_MAP[seg.text as keyof typeof FLAG_MAP]; + if (!key) continue; + + flags[key] = true; + keep[j] = false; + + // Drop one adjacent whitespace run so removed flags don't leave dangling + // spaces. Prefer trailing whitespace; fall back to leading if at the end. + if (j + 1 < segments.length && segments[j + 1].type === "ws") { + keep[j + 1] = false; + } else if (j > 0 && segments[j - 1].type === "ws") { + keep[j - 1] = false; + } + } + + // Trim covers the case where two adjacent flags (`... --gate --json`) + // both claim the single whitespace between them, leaving a trailing space + // after the kept token. Wrapping quotes come from OpenCode/Pi users who + // quote paths with spaces (shell muscle memory); strip them here so + // downstream callers never see tokenization artifacts. + const rawFilePath = stripWrappingQuotes( + segments + .filter((_, j) => keep[j]) + .map((seg) => seg.text) + .join("") + .trim(), + ); + + return { filePath: stripAtPrefix(rawFilePath), rawFilePath, ...flags }; +} diff --git a/packages/shared/at-reference.test.ts b/packages/shared/at-reference.test.ts new file mode 100644 index 00000000..fbcf7f93 --- /dev/null +++ b/packages/shared/at-reference.test.ts @@ -0,0 +1,99 @@ +import { describe, test, expect } from "bun:test"; +import { stripAtPrefix, resolveAtReference } from "./at-reference"; + +// The `@foo.md` convention — popularised by Claude Code but supported by +// several harnesses — treats `@` as a reference marker, not part of the +// filename. These helpers exist so every harness strips the same way and +// optionally falls back to the literal path for scoped-package-style names. + +describe("stripAtPrefix", () => { + test("removes a single leading @", () => { + expect(stripAtPrefix("@foo.md")).toBe("foo.md"); + }); + + test("removes only one @ (does not recurse)", () => { + expect(stripAtPrefix("@@foo.md")).toBe("@foo.md"); + }); + + test("leaves paths without @ unchanged", () => { + expect(stripAtPrefix("foo.md")).toBe("foo.md"); + }); + + test("leaves @ that is not at the start unchanged", () => { + expect(stripAtPrefix("dir/@foo.md")).toBe("dir/@foo.md"); + }); + + test("strips @ from scoped-package-style paths", () => { + expect(stripAtPrefix("@scope/pkg/README.md")).toBe("scope/pkg/README.md"); + }); + + test("handles empty string", () => { + expect(stripAtPrefix("")).toBe(""); + }); + + // Wrapping quotes come from harnesses that tokenize on whitespace (OpenCode, + // Pi). Users have to quote paths with spaces: `"@My Notes.md"`. Without + // unwrapping the quotes first, stripAtPrefix would never see the `@`. + test("strips wrapping double quotes before stripping @", () => { + expect(stripAtPrefix(`"@foo.md"`)).toBe("foo.md"); + }); + + test("strips wrapping single quotes before stripping @", () => { + expect(stripAtPrefix(`'@foo.md'`)).toBe("foo.md"); + }); + + test("strips wrapping quotes around a path with spaces", () => { + expect(stripAtPrefix(`"@My Notes.md"`)).toBe("My Notes.md"); + }); + + test("strips wrapping quotes when no @ present", () => { + expect(stripAtPrefix(`"foo.md"`)).toBe("foo.md"); + }); + + test("leaves mismatched quotes alone (not wrapping)", () => { + expect(stripAtPrefix(`"@foo.md`)).toBe(`"@foo.md`); + expect(stripAtPrefix(`@foo.md"`)).toBe(`foo.md"`); + }); +}); + +describe("resolveAtReference", () => { + // Primary behavior: stripped form wins if it resolves. + test("returns the stripped path when it resolves", () => { + const exists = (p: string) => p === "foo.md"; + expect(resolveAtReference("@foo.md", exists)).toBe("foo.md"); + }); + + // Fallback: literal path used only when stripped form doesn't resolve. + test("falls back to the literal path when only that resolves", () => { + const exists = (p: string) => p === "@scope/pkg/README.md"; + expect(resolveAtReference("@scope/pkg/README.md", exists)).toBe("@scope/pkg/README.md"); + }); + + // When BOTH resolve, stripped form wins (reference-mode primacy). + test("prefers the stripped path when both resolve (reference wins)", () => { + const exists = (_p: string) => true; + expect(resolveAtReference("@foo.md", exists)).toBe("foo.md"); + }); + + // Returns null when neither candidate resolves — caller handles the error. + test("returns null when neither candidate resolves", () => { + const exists = (_p: string) => false; + expect(resolveAtReference("@nope.md", exists)).toBeNull(); + }); + + // Inputs without @ have no fallback, just a single existence check. + test("handles non-@ inputs with a single check", () => { + let calls = 0; + const exists = (p: string) => { calls++; return p === "plain.md"; }; + expect(resolveAtReference("plain.md", exists)).toBe("plain.md"); + expect(calls).toBe(1); + }); + + // Non-@ input that doesn't resolve returns null without a retry. + test("returns null for non-@ input that doesn't resolve", () => { + let calls = 0; + const exists = (_p: string) => { calls++; return false; }; + expect(resolveAtReference("missing.md", exists)).toBeNull(); + expect(calls).toBe(1); + }); +}); diff --git a/packages/shared/at-reference.ts b/packages/shared/at-reference.ts new file mode 100644 index 00000000..892d6df2 --- /dev/null +++ b/packages/shared/at-reference.ts @@ -0,0 +1,52 @@ +/** + * `@`-reference handling for user-provided paths. + * + * Several agent harnesses (Claude Code, OpenCode, Pi) let users reference + * files with an `@` prefix, e.g. `@README.md`. The `@` is the team's + * reference marker, not part of the filename. Stripping it is the primary + * resolution path — that's the common case and it's supported first-class. + * + * The secondary path handles scoped-package-style names like + * `@scope/pkg/README.md`: if the stripped form doesn't resolve, fall back + * to the literal form so those paths still open. + * + * Both functions are pure and take any filesystem-ish predicate via a + * callback, so they're trivial to unit-test without stubbing anything. + */ + +import { stripWrappingQuotes } from "./resolve-file"; + +/** + * Normalize a user-typed path reference by unwrapping matching `"..."` or + * `'...'` quotes and removing a single leading `@`. Quotes come from + * harnesses that tokenize on whitespace (OpenCode, Pi), where paths + * containing spaces have to be quoted. The quote-stripping has to run + * first so the `@` check sees the real first character. + * + * Non-`@` inputs are returned unchanged except for quote unwrapping. + * Does not recurse: `@@foo` becomes `@foo`, not `foo`. + */ +export function stripAtPrefix(input: string): string { + const unquoted = stripWrappingQuotes(input); + return unquoted.startsWith("@") ? unquoted.slice(1) : unquoted; +} + +/** + * Resolve an `@`-prefixed user input by trying the stripped form first + * (reference mode, primary) and falling back to the literal form if the + * stripped form doesn't resolve. Returns the candidate that resolves, or + * null if neither does. + * + * `exists` defines what "resolves" means — use `existsSync` for a bare + * filesystem check, or wrap `resolveMarkdownFile` / `statSync` for richer + * predicates. The helper itself is filesystem-agnostic. + */ +export function resolveAtReference( + input: string, + exists: (candidate: string) => boolean, +): string | null { + const stripped = stripAtPrefix(input); + if (exists(stripped)) return stripped; + if (stripped !== input && exists(input)) return input; + return null; +} diff --git a/packages/shared/package.json b/packages/shared/package.json index a94b401e..f86bbf6f 100644 --- a/packages/shared/package.json +++ b/packages/shared/package.json @@ -26,7 +26,9 @@ "./worktree": "./worktree.ts", "./html-to-markdown": "./html-to-markdown.ts", "./url-to-markdown": "./url-to-markdown.ts", - "./tour": "./tour.ts" + "./tour": "./tour.ts", + "./annotate-args": "./annotate-args.ts", + "./at-reference": "./at-reference.ts" }, "dependencies": { "@joplin/turndown-plugin-gfm": "^1.0.64", diff --git a/packages/shared/resolve-file.ts b/packages/shared/resolve-file.ts index 2ef9644b..c56c3930 100644 --- a/packages/shared/resolve-file.ts +++ b/packages/shared/resolve-file.ts @@ -55,7 +55,7 @@ export function expandHomePath(input: string, home = homedir()): string { return input; } -function stripWrappingQuotes(input: string): string { +export function stripWrappingQuotes(input: string): string { if (input.length < 2) { return input; }