diff --git a/AGENTS.md b/AGENTS.md index 89c666c4e..340a582da 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -298,6 +298,12 @@ User annotates content, provides feedback Send Annotations → feedback sent to agent session ``` +### Submit with a note + +Every annotate surface (file, folder, `annotate-last` message, URL, live app) has a split Send control: the primary button is the incumbent Send Feedback, and the caret beside it opens a one-line "Add a note..." field whose Enter submits the note TOGETHER with any annotations already queued, in one interaction. With nothing queued the primary button opens that field instead of staying hidden, which is what it did before (submitting an empty review was never useful). Escape closes the field without submitting and keeps the typed text for the rest of the session; an unsent note is deliberately not drafted, because it becomes a real, drafted annotation the moment it is sent. + +The note is created as a `GLOBAL_COMMENT` at SUBMIT time and committed into `annotations` (`commitSubmitNote` in `packages/editor/App.tsx`), so it rides `exportAnnotations` and the `/api/feedback` annotations array exactly like a composer-made global comment: **zero server change on either runtime** — both `/api/feedback` handlers take a pre-rendered feedback string plus an opaque `unknown[]`. Committing it into state rather than threading it through the payload builders is what makes `annotate-last`'s multi-message export pick it up, since those entries are rebuilt from the live linked-doc session snapshot rather than from `allAnnotations`; the submit therefore waits one render for the commit (an effect keyed on the pending note id). It is NOT recorded in the annotation undo/redo history: it exists for the duration of one submit. On HTML and live-app surfaces the comment-only clamp does not apply — that clamp sits on the iframe's postMessage ingest, and this note is created in the parent. Plan mode is untouched: the control is annotate-only. The compact touch shell has no header Send control, so the field lives in its "Review and finish" surface instead. The affordance is `packages/editor/components/AnnotateSendControl.tsx`; the chords are documented by the `annotate-note` shortcut scope. + ### Tolerant argument resolution Slash-command hosts forward raw user words to `plannotator annotate` verbatim (on Claude Code through a bash-substitution prefix that runs before the model sees anything), so non-strict invocations resolve their arguments in three tiers. The shared logic lives in `packages/shared/annotate-target.ts` (vendored to Pi) and is wired into the CLI's annotate branch plus the OpenCode and Pi command parsers: diff --git a/packages/editor/App.submitNote.test.tsx b/packages/editor/App.submitNote.test.tsx new file mode 100644 index 000000000..34074ac27 --- /dev/null +++ b/packages/editor/App.submitNote.test.tsx @@ -0,0 +1,353 @@ +/** + * One-step "submit with a note" on the annotate surfaces (DOM-gated). + * + * Regressions each test guards: + * - The note must reach the agent AS A GLOBAL COMMENT, in the exported + * feedback string AND in the `/api/feedback` annotations array. If it were + * only spliced into the feedback text, the annotations array (which both + * servers persist as the durable submission record) would lose it; if it + * were only put in the array, the agent-facing text would not mention it. + * - It must be sent ALONGSIDE annotations already in the session, not + * instead of them. The note is committed into state and submitted a render + * later, so a regression that submits in the same tick would send the + * pre-note payload and silently drop the note. + * - The zero-annotation fast path: the incumbent header hid Send entirely + * with nothing to send, so Send must open the note field instead of + * submitting an empty review. + * - Escape must close the field WITHOUT submitting: on HTML surfaces Escape + * also walks the pinpoint ladder, so a missed stopPropagation would both + * submit and disarm. + * - Plan mode must not grow the control at all: it has its own + * approve/deny-with-feedback semantics and is deliberately untouched. + */ +import { afterAll, afterEach, describe, expect, test } from "bun:test"; +import React, { act } from "react"; +import { createRoot, type Root } from "react-dom/client"; +import { + resetStorageBackend, + setStorageBackend, + type StorageBackend, +} from "@plannotator/ui/utils/storage"; + +const hasDom = typeof document !== "undefined"; + +if (hasDom) { + document.cookie = "plannotator-look-feel-announcement-seen=2; path=/"; + document.cookie = "plannotator-vim-mode-announcement-seen=2; path=/"; + document.cookie = "plannotator-plan-ai-announcement-seen=1; path=/"; +} + +const appModule = hasDom ? await import("./App") : null; +const App = appModule?.default as typeof import("./App")["default"]; +const originalFetch = globalThis.fetch; +const originalEventSource = globalThis.EventSource; + +const memory = new Map(); +const memoryBackend: StorageBackend = { + getItem: (key) => memory.get(key) ?? null, + setItem: (key, value) => void memory.set(key, value), + removeItem: (key) => void memory.delete(key), +}; + +function seedAnnouncementsSeen(): void { + memory.set("plannotator-look-feel-announcement-seen", "2"); + memory.set("plannotator-vim-mode-announcement-seen", "2"); + memory.set("plannotator-plan-ai-announcement-seen", "1"); +} + +/** External annotations to deliver as the stream's opening snapshot, so a test + * can seed the session with pre-existing feedback without driving the DOM + * annotation flow. Read by the EventSource double at construction time. */ +let seededExternalAnnotations: unknown[] = []; + +class StubEventSource { + static readonly CONNECTING = 0; + static readonly OPEN = 1; + static readonly CLOSED = 2; + + readonly CONNECTING = 0; + readonly OPEN = 1; + readonly CLOSED = 2; + readonly readyState = StubEventSource.OPEN; + readonly url: string; + readonly withCredentials = false; + onerror: ((event: Event) => void) | null = null; + onmessage: ((event: MessageEvent) => void) | null = null; + onopen: ((event: Event) => void) | null = null; + + constructor(url: string | URL) { + this.url = String(url); + const payload = seededExternalAnnotations; + if (payload.length > 0) { + // Handlers are assigned right after construction; deliver on the next + // task, the way a real stream's first snapshot arrives. + setTimeout(() => { + this.onmessage?.({ + data: JSON.stringify({ type: "snapshot", annotations: payload }), + } as MessageEvent); + }, 0); + } + } + + addEventListener(): void {} + close(): void {} + dispatchEvent(): boolean { return true; } + removeEventListener(): void {} +} + +interface SubmittedFeedback { + feedback?: string; + annotations?: Array<{ type?: string; text?: string; originalText?: string }>; +} + +let submissions: SubmittedFeedback[] = []; + +const MARKDOWN = "# Notes\n\nSome body text.\n"; +const RAW_HTML = "

Rendered page

Body copy.

"; + +function annotatePlan(extra: Record = {}) { + return { + plan: MARKDOWN, + origin: "codex", + mode: "annotate", + filePath: "/tmp/notes.md", + sharingEnabled: false, + serverConfig: {}, + ...extra, + }; +} + +const planReviewPlan = { + plan: MARKDOWN, + origin: "claude-code", + sharingEnabled: false, + serverConfig: {}, +}; + +function makeFetch(plan: unknown): typeof fetch { + // SAFETY: the app only ever calls fetch(input, init); the double implements + // that call signature and not `fetch.preconnect`. + const impl = async (input: RequestInfo | URL, init?: RequestInit) => { + const rawUrl = input instanceof Request ? input.url : String(input); + if (rawUrl.startsWith("https://api.github.com/")) return new Response(null, { status: 404 }); + + const url = new URL(rawUrl, "http://localhost"); + if (url.pathname === "/api/plan") return Response.json(plan); + if (url.pathname === "/api/ai/capabilities") return Response.json({ available: false, providers: [] }); + if (url.pathname === "/api/draft") return Response.json({ error: "Not found" }, { status: 404 }); + if (url.pathname === "/api/feedback" || url.pathname === "/api/deny") { + submissions.push(JSON.parse(String(init?.body ?? "{}")) as SubmittedFeedback); + return Response.json({ ok: true }); + } + return Response.json({}); + }; + return impl as unknown as typeof fetch; +} + +let root: Root | null = null; +let host: HTMLElement | null = null; + +function sendButton(): HTMLButtonElement | undefined { + return Array.from(document.querySelectorAll("button")) + .find((button) => button.title.startsWith("Send Feedback")); +} + +function noteToggle(): HTMLButtonElement | null { + return document.querySelector("[data-annotate-note-toggle]"); +} + +function noteInput(): HTMLTextAreaElement | null { + return document.querySelector("[data-annotate-note-input]"); +} + +async function settle(): Promise { + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 0)); + }); +} + +async function mount(plan: unknown, waitFor: () => unknown): Promise { + globalThis.fetch = makeFetch(plan); + // SAFETY: the App only uses EventSource's constructor, handlers, and close. + globalThis.EventSource = StubEventSource as unknown as typeof EventSource; + host = document.createElement("div"); + document.body.appendChild(host); + root = createRoot(host); + await act(async () => { + root?.render(); + }); + for (let attempt = 0; attempt < 20 && !waitFor(); attempt += 1) { + await settle(); + } + if (!waitFor()) throw new Error("app did not finish mounting"); +} + +const mountAnnotate = (extra: Record = {}) => + mount(annotatePlan(extra), () => noteToggle()); + +async function typeNote(text: string): Promise { + const input = noteInput(); + if (!input) throw new Error("note field is not open"); + await act(async () => { + const setter = Object.getOwnPropertyDescriptor(HTMLTextAreaElement.prototype, "value")!.set!; + setter.call(input, text); + input.dispatchEvent(new Event("input", { bubbles: true })); + }); +} + +async function pressKey(key: string, init: KeyboardEventInit = {}): Promise { + const input = noteInput(); + if (!input) throw new Error("note field is not open"); + await act(async () => { + input.dispatchEvent(new KeyboardEvent("keydown", { key, bubbles: true, ...init })); + }); + await settle(); +} + +afterEach(async () => { + if (root) await act(async () => root?.unmount()); + root = null; + host?.remove(); + host = null; + globalThis.fetch = originalFetch; + globalThis.EventSource = originalEventSource; + submissions = []; + seededExternalAnnotations = []; + memory.clear(); + resetStorageBackend(); + if (hasDom) document.body.replaceChildren(); +}); + +afterAll(() => { + resetStorageBackend(); +}); + +describe.if(hasDom)("annotate submit-with-note", () => { + test("zero annotations: Send opens the note field and submits nothing", async () => { + setStorageBackend(memoryBackend); + seedAnnouncementsSeen(); + await mountAnnotate(); + + // The incumbent header hid Send with nothing to send; the fast path + // replaces that absence rather than restoring a dead button. + expect(noteInput()).toBeNull(); + const send = sendButton(); + expect(send).toBeDefined(); + + await act(async () => send!.click()); + await settle(); + + expect(noteInput()).not.toBeNull(); + expect(submissions).toHaveLength(0); + }); + + test("Mod+Enter sends the note as a GLOBAL_COMMENT in one step", async () => { + setStorageBackend(memoryBackend); + seedAnnouncementsSeen(); + await mountAnnotate(); + + await act(async () => noteToggle()!.click()); + await settle(); + await typeNote("looks fine, watch the migration"); + // The field is multi-line now: a bare Enter is a newline, never a send. + await pressKey("Enter"); + expect(submissions).toHaveLength(0); + await pressKey("Enter", { metaKey: true }); + + expect(submissions).toHaveLength(1); + const body = submissions[0]!; + const notes = (body.annotations ?? []).filter((a) => a.type === "GLOBAL_COMMENT"); + expect(notes).toHaveLength(1); + expect(notes[0]!.text).toBe("looks fine, watch the migration"); + // The exported feedback is what the agent actually reads. + expect(body.feedback).toContain("looks fine, watch the migration"); + }); + + test("the note rides alongside annotations already in the session", async () => { + setStorageBackend(memoryBackend); + seedAnnouncementsSeen(); + seededExternalAnnotations = [{ + id: "ext-1", + blockId: "", + startOffset: 0, + endOffset: 0, + type: "COMMENT", + text: "existing finding", + originalText: "Some body text.", + createdA: 1, + source: "eslint", + }]; + await mountAnnotate(); + // Wait for the seeded snapshot to land before opening the composer. + await settle(); + await settle(); + + await act(async () => noteToggle()!.click()); + await settle(); + await typeNote("also: ship it"); + await pressKey("Enter", { metaKey: true }); + + expect(submissions).toHaveLength(1); + const annotations = submissions[0]!.annotations ?? []; + expect(annotations.some((a) => a.text === "existing finding")).toBe(true); + expect(annotations.some((a) => a.type === "GLOBAL_COMMENT" && a.text === "also: ship it")).toBe(true); + expect(submissions[0]!.feedback).toContain("existing finding"); + expect(submissions[0]!.feedback).toContain("also: ship it"); + }); + + test("Escape closes the field without submitting and keeps the typed text", async () => { + setStorageBackend(memoryBackend); + seedAnnouncementsSeen(); + await mountAnnotate(); + + await act(async () => noteToggle()!.click()); + await settle(); + await typeNote("not ready to send"); + await pressKey("Escape"); + + expect(noteInput()).toBeNull(); + expect(submissions).toHaveLength(0); + + // Reopening restores the text rather than discarding a half-typed note. + await act(async () => noteToggle()!.click()); + await settle(); + expect(noteInput()?.value).toBe("not ready to send"); + }); + + test("HTML surface: the same one-step note reaches the agent", async () => { + setStorageBackend(memoryBackend); + seedAnnouncementsSeen(); + // Raw-HTML sessions are comment-only, but the clamp lives on the iframe's + // postMessage ingest — a GLOBAL_COMMENT made in the parent is unaffected. + await mountAnnotate({ + filePath: "/tmp/page.html", + renderAs: "html", + rawHtml: RAW_HTML, + plan: "", + }); + + await act(async () => noteToggle()!.click()); + await settle(); + await typeNote("the header spacing is off"); + await pressKey("Enter", { ctrlKey: true }); + + expect(submissions).toHaveLength(1); + const notes = (submissions[0]!.annotations ?? []).filter((a) => a.type === "GLOBAL_COMMENT"); + expect(notes).toHaveLength(1); + expect(notes[0]!.text).toBe("the header spacing is off"); + }); + + test("plan review is untouched: no note control, Send Feedback still submits", async () => { + setStorageBackend(memoryBackend); + seedAnnouncementsSeen(); + await mount(planReviewPlan, () => sendButton()); + + expect(noteToggle()).toBeNull(); + expect(noteInput()).toBeNull(); + // Plan mode's Send with no feedback opens its own prompt, not a note field. + await act(async () => sendButton()!.click()); + await settle(); + expect(noteInput()).toBeNull(); + expect(submissions).toHaveLength(0); + }); +}); diff --git a/packages/editor/App.tsx b/packages/editor/App.tsx index c9dbd5013..2e8688817 100644 --- a/packages/editor/App.tsx +++ b/packages/editor/App.tsx @@ -38,6 +38,7 @@ import { getCallbackConfig, CallbackAction, executeCallback } from '@plannotator import { useAgents } from '@plannotator/ui/hooks/useAgents'; import { useActiveSection } from '@plannotator/ui/hooks/useActiveSection'; import { storage } from '@plannotator/ui/utils/storage'; +import { getIdentity } from '@plannotator/ui/utils/identity'; import { copyTextToClipboard } from '@plannotator/ui/utils/clipboard'; import { configStore, useConfigValue } from '@plannotator/ui/config'; import { CompletionOverlay } from '@plannotator/ui/components/CompletionOverlay'; @@ -174,6 +175,7 @@ import { CompactPlanReview, type CompactPlanReviewAction, } from './components/CompactPlanReview'; +import type { AnnotateSubmitNoteControl } from './components/AnnotateSendControl'; import { COMPACT_PLAN_ARTIFACT, openCompactPlanNavigator, @@ -521,6 +523,12 @@ const App: React.FC = () => { const [annotateMode, setAnnotateMode] = useState(false); const [gate, setGate] = useState(false); const [approvalNotesSupported, setApprovalNotesSupported] = useState(false); + // One-step "submit with a note" (annotate surfaces). The typed text lives in + // the control itself; App only tracks the note it has committed and is + // waiting to submit. An unsent note is deliberately NOT persisted: it becomes + // a real, drafted annotation the moment it is sent, and a one-liner is + // cheaper to retype than a second draft channel is to maintain. + const [pendingSubmitNoteId, setPendingSubmitNoteId] = useState(null); const [clientLease, setClientLease] = useState(null); const [annotateSource, setAnnotateSource] = useState<'file' | 'message' | 'folder' | null>(null); const [recentMessages, setRecentMessages] = useState([]); @@ -4958,6 +4966,75 @@ const App: React.FC = () => { sendFeedback(); }, [maybeConfirmUnsavedSourceFileEdits]); + // --- One-step submit with a note (annotate surfaces only) ----------------- + // Reading an agent's message and wanting to reply "that's fine, but watch the + // migration" took four interactions (open the global-comment composer, type, + // save, Send). The note is created as a GLOBAL_COMMENT at SUBMIT time, so it + // rides exportAnnotations and the /api/feedback annotations array exactly + // like a composer-made global comment: zero server change on either runtime. + // + // It is committed into `annotations` (not threaded through the payload + // builders) so every annotate export path picks it up for free — including + // annotate-last's multi-message export, whose entries are rebuilt from the + // live linked-doc session snapshot rather than from `allAnnotations`. + // The submit itself waits a render for that commit; see the effect below. + const commitSubmitNote = useCallback((text: string): string | null => { + const trimmed = text.trim(); + if (!trimmed) return null; + const note: Annotation = { + id: `global-note-${Date.now()}`, + blockId: '', + startOffset: 0, + endOffset: 0, + type: AnnotationType.GLOBAL_COMMENT, + text: trimmed, + originalText: '', + createdA: Date.now(), + author: getIdentity(), + }; + // Deliberately NOT annotationHistory.record: the note exists for the + // duration of one submit, and an undo of it after the send would restore + // nothing the agent has not already been told. + annotationsRef.current = [...annotationsRef.current, note]; + setAnnotations(annotationsRef.current); + return note.id; + }, []); + + const handleSubmitNote = useCallback((text: string) => { + if (isSubmitting || isExiting) return; + const noteId = commitSubmitNote(text); + if (!noteId) { + // Empty field: fall back to the plain send when something is queued, + // otherwise there is genuinely nothing to submit. + if (hasFeedbackToSend) handleHeaderAnnotateFeedback(); + return; + } + setPendingSubmitNoteId(noteId); + }, [ + commitSubmitNote, + handleHeaderAnnotateFeedback, + hasFeedbackToSend, + isExiting, + isSubmitting, + ]); + + // The commit above is a state write, so the feedback payload builders (which + // close over `allAnnotations`) only see the note on the NEXT render. Submit + // from an effect once the note is actually in state rather than guessing. + useEffect(() => { + if (!pendingSubmitNoteId) return; + if (!annotations.some((a) => a.id === pendingSubmitNoteId)) return; + setPendingSubmitNoteId(null); + handleHeaderAnnotateFeedback(); + }, [annotations, handleHeaderAnnotateFeedback, pendingSubmitNoteId]); + + const submitNoteControl = useMemo( + () => (annotateMode && isApiMode && !documentReadOnly + ? { onSubmit: handleSubmitNote } + : undefined), + [annotateMode, documentReadOnly, handleSubmitNote, isApiMode], + ); + const handleHeaderAnnotateApprove = useCallback(() => { if (maybeConfirmUnsavedSourceFileEdits('approve', requestAnnotateApprove)) return; requestAnnotateApprove(); @@ -5508,6 +5585,7 @@ const App: React.FC = () => { onGoalSetupExit={handleGoalSetupExit} onGoalSetupSubmit={handleGoalSetupSubmit} onAnnotateFeedback={handleHeaderAnnotateFeedback} + submitNote={submitNoteControl} onAnnotateApprove={handleHeaderAnnotateApprove} onFeedback={handleHeaderFeedback} onApprove={handleHeaderApprove} @@ -5588,6 +5666,8 @@ const App: React.FC = () => { primaryActionId={compactPrimaryReviewActionId} onOpenAnnotations={() => switchCompactPlanSurface('annotations')} onOpenAI={canUseAskAI ? () => switchCompactPlanSurface('ai') : undefined} + submitNote={submitNoteControl} + submitNoteDisabled={compactActionBusy} /> )} diff --git a/packages/editor/components/AnnotateSendControl.tsx b/packages/editor/components/AnnotateSendControl.tsx new file mode 100644 index 000000000..95c3abe9d --- /dev/null +++ b/packages/editor/components/AnnotateSendControl.tsx @@ -0,0 +1,281 @@ +import React, { useCallback, useEffect, useRef, useState } from 'react'; +import { Send } from 'lucide-react'; +import { Button } from '@plannotator/ui/components/ui/button'; +import { submitHint } from '@plannotator/ui/utils/platform'; + +/** + * One-step "send with a note" for the annotate surfaces. + * + * The note itself is not owned here: App creates it as a GLOBAL_COMMENT at + * submit time so it rides `exportAnnotations` and the `/api/feedback` + * annotations array with no server change. This component only owns the + * affordance and the typed text (deliberately local, so a keystroke never + * re-renders the whole header). + * + * Interaction contract: + * - The control is ONE joined split button: [Send Feedback | v]. The left + * segment is always the incumbent plain send, unchanged; the caret is a + * separate segment of the same pill. + * - The caret opens a panel BELOW the pill with a multi-line note field and + * its own distinct action, "Submit with feedback", which sends the note + * together with everything already in the session. The two actions never + * share a button. + * - The note field: Enter inserts a newline, Mod+Enter submits with feedback, + * Esc closes the panel and keeps the half-typed text. + */ +export interface AnnotateSubmitNoteControl { + /** Submit this note together with any annotations already in the session. */ + onSubmit: (text: string) => void; +} + +export const ANNOTATE_NOTE_PLACEHOLDER = 'Add a note...'; + +/** One line tall at rest, grows with content, then scrolls. */ +const NOTE_MAX_HEIGHT_PX = 144; + +function useAutoGrow(text: string) { + const ref = useRef(null); + useEffect(() => { + const el = ref.current; + if (!el) return; + el.style.height = 'auto'; + el.style.height = `${Math.min(el.scrollHeight, NOTE_MAX_HEIGHT_PX)}px`; + }, [text]); + return ref; +} + +interface AnnotateNoteFieldProps { + text: string; + onTextChange: (value: string) => void; + onSubmit: (text: string) => void; + onClose: () => void; + disabled?: boolean; + autoFocus?: boolean; +} + +/** The multi-line note field. Controlled: the owner keeps the text so closing + * and reopening does not discard a half-typed note. */ +const AnnotateNoteField: React.FC = ({ + text, + onTextChange, + onSubmit, + onClose, + disabled = false, + autoFocus = true, +}) => { + const ref = useAutoGrow(text); + + useEffect(() => { + if (autoFocus) ref.current?.focus(); + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [autoFocus]); + + const canSend = text.trim().length > 0; + + const handleKeyDown = useCallback( + (event: React.KeyboardEvent) => { + if (event.key === 'Escape') { + // Stop here: the surrounding surfaces (the HTML pinpoint ladder, + // popovers, the plan-diff exit) all treat a bare Escape as theirs. + event.preventDefault(); + event.stopPropagation(); + onClose(); + return; + } + if (event.key === 'Enter' && (event.metaKey || event.ctrlKey)) { + event.preventDefault(); + event.stopPropagation(); + if (!disabled && canSend) onSubmit(text); + } + // Plain Enter falls through: the field is multi-line and Enter's job is + // a newline. + }, + [canSend, disabled, onClose, onSubmit, text], + ); + + return ( +