From 6d922307223d12f436b229d4c4b29b8835b93b6c Mon Sep 17 00:00:00 2001 From: soryu Date: Thu, 30 Apr 2026 23:26:10 +0100 Subject: fix(doc-mode): root-walk goal serializer + roundtrip-confirmed draft drop, plus richer context menus (#114) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## The data-loss bug User reported "even after clicking Save now I have lost my doc". Two causes: 1. **GoalChangePlugin only read children[1]** — it captured edits to the single goal paragraph but silently dropped any typing that landed in the trailing paragraph below the StepsBlock (or in extra paragraphs the user had inserted). pendingGoalRef stayed at the persisted value, Save now fired empty/stale content, the doc was overwritten. 2. **fireSave dropped localStorage immediately on save success.** If the save persisted the wrong/empty content, the draft (which had the real content) was already gone — no recovery path. ## Fixes ### Capture all body content New `serializeGoalFromRoot` walks the entire root, skips only the H1 title and the StepsBlock decorator, and emits multi-paragraph markdown joined by blank lines. `GoalChangePlugin` and `fireSave` both call it now. Seed code splits an existing multi-paragraph goal back into ParagraphNodes on load. ### Read directly from the editor at save time `fireSave` now reads pendingGoalRef AND consults the live editor state via `editor.getEditorState().read()`. If anything went wrong with OnChangePlugin (which is rare, but possible — and was eating typing for many users), the save still picks up the actual document body. ### Defensive pre-save flush Before talking to the backend, `fireSave` writes the value to localStorage. If the network fails, the page closes mid-flight, or anything else goes sideways, the draft survives. ### Roundtrip-confirmed draft cleanup We no longer drop the localStorage draft inside `fireSave`. Instead a new effect watches `directive.goal` and clears the draft only when: lastSavedValueRef === directive.goal === pendingGoalRef.current i.e. only when the polled state confirms our save persisted AND the user hasn't typed anything new in the meantime. ## Question notifications respect document mode `SupervisorQuestionNotification` and `PhaseConfirmationToast` now route to `/directives/?task=` (the doc-mode surface) when the user has documentMode on AND the question carries a directiveId. Falls back to the old `/exec/:taskId` route for non-doc-mode users. ## Richer directive folder context menu In addition to start/pause/archive/delete/go-to-PR/new-draft we now expose: - Advance DAG - Plan orders - Clean up - Create / Update PR All optional callbacks; the legacy tabular UI is unaffected. ## Richer task row context menu In addition to interrupt/complete/fail/skip we now expose: - Send message — browser prompt → sendTaskMessage (same wire as the inline comment box) - Open in task page — navigates to /exec/:taskId for the full task UI (worktree diff viewer, checkpoint controls, etc.) Co-authored-by: Claude Opus 4.7 (1M context) --- .../components/directives/DirectiveContextMenu.tsx | 91 ++++++++++++--- .../src/components/directives/DocumentEditor.tsx | 125 +++++++++++++++++---- 2 files changed, 178 insertions(+), 38 deletions(-) (limited to 'makima/frontend/src/components/directives') diff --git a/makima/frontend/src/components/directives/DirectiveContextMenu.tsx b/makima/frontend/src/components/directives/DirectiveContextMenu.tsx index 3f24ce1..eda7e7f 100644 --- a/makima/frontend/src/components/directives/DirectiveContextMenu.tsx +++ b/makima/frontend/src/components/directives/DirectiveContextMenu.tsx @@ -17,6 +17,14 @@ interface DirectiveContextMenuProps { * tabular UI doesn't have to wire it up. */ onNewDraft?: () => void; + /** Trigger a fresh PR creation from the current contract state. */ + onCreatePR?: () => void; + /** Manually advance the DAG (find newly-ready steps). */ + onAdvance?: () => void; + /** Run the cleanup task to prune merged/stale steps. */ + onCleanup?: () => void; + /** Pick up linked orders (queue them as new steps). */ + onPickUpOrders?: () => void; } export function DirectiveContextMenu({ @@ -30,6 +38,10 @@ export function DirectiveContextMenu({ onDelete, onGoToPR, onNewDraft, + onCreatePR, + onAdvance, + onCleanup, + onPickUpOrders, }: DirectiveContextMenuProps) { const menuRef = useRef(null); @@ -153,21 +165,72 @@ export function DirectiveContextMenu({ )} - {/* Go to PR link */} + {/* Orchestration actions — Advance / Pick up orders / Cleanup. */} + {(onAdvance || onPickUpOrders || onCleanup) && ( +
+ )} + {onAdvance && ( + + )} + {onPickUpOrders && ( + + )} + {onCleanup && ( + + )} + + {/* PR actions — Create / Update / Go to PR. */} + {(onCreatePR || showGoToPR) &&
} + {onCreatePR && ( + + )} {showGoToPR && ( - <> -
- - + )}
diff --git a/makima/frontend/src/components/directives/DocumentEditor.tsx b/makima/frontend/src/components/directives/DocumentEditor.tsx index 08e4d73..981cab1 100644 --- a/makima/frontend/src/components/directives/DocumentEditor.tsx +++ b/makima/frontend/src/components/directives/DocumentEditor.tsx @@ -175,6 +175,32 @@ function appendInlineMarkdownTo(parent: ElementNode, markdown: string): void { } } +/** + * Walk the editor root and serialise every non-decorator paragraph between + * the H1 title and the end of the document into the goal markdown. This + * captures user typing wherever it lands — the goal paragraph proper, but + * also any extra paragraphs the user inserted above or below the StepsBlock. + * Previously we read only `children[1]` and lost anything outside it. + */ +function serializeGoalFromRoot(root: ElementNode): string { + const parts: string[] = []; + const children = root.getChildren(); + for (let i = 0; i < children.length; i++) { + const node = children[i]; + // Skip the H1 title (always at index 0 by construction). + if (i === 0 && node.getType() === "heading") continue; + // Skip the StepsBlock decorator and any other non-element nodes. + if ($isStepsBlockNode(node)) continue; + if (!$isElementNode(node)) continue; + parts.push(serializeInlineMarkdown(node)); + } + // Trim trailing empties; collapse runs of >2 blank lines. + return parts + .join("\n\n") + .replace(/\n{3,}/g, "\n\n") + .replace(/^\s+|\s+$/g, ""); +} + /** * Walk the goal paragraph's children and emit inline markdown. Only TextNodes * are emitted — anything else falls back to its plain text content. We always @@ -296,20 +322,28 @@ function SeedContentPlugin({ heading.append($createTextNode(directive.title)); root.append(heading); - // Paragraph: goal (editable). The persisted goal may contain inline - // markdown — parse it into formatted TextNodes so users see their - // bold/italic/code formatting on load. - const goalPara = $createParagraphNode(); - if (initialGoal.length > 0) { - appendInlineMarkdownTo(goalPara, initialGoal); + // Goal body. The persisted goal may contain multiple paragraphs + // (separated by blank lines) and inline markdown — split by blank + // lines so each block becomes its own ParagraphNode, and parse the + // inline formatting per paragraph. Always emit at least one + // paragraph so users have a place to type even when the goal is + // empty. + const blocks = + initialGoal.length > 0 ? initialGoal.split(/\n{2,}/) : [""]; + for (const block of blocks) { + const p = $createParagraphNode(); + if (block.length > 0) { + appendInlineMarkdownTo(p, block); + } + root.append(p); } - root.append(goalPara); // Steps block (decorator — non-editable). root.append($createStepsBlockNode()); // Trailing empty paragraph so the cursor has somewhere to land below - // the steps block. + // the steps block. The trailing area is also captured as goal + // content by serializeGoalFromRoot, so any typing here is preserved. root.append($createParagraphNode()); }, { tag: "history-merge" }, @@ -388,15 +422,11 @@ function GoalChangePlugin({ ignoreSelectionChange onChange={(editorState) => { editorState.read(() => { - const root = $getRoot(); - const children = root.getChildren(); - // The goal lives at index 1 (after the H1 title). - const goalNode = children[1]; - if (!goalNode || !$isElementNode(goalNode)) return; - // Serialize the goal paragraph as INLINE MARKDOWN so bold/italic/code - // formatting round-trips through `directive.goal` (a plain TEXT - // column on the backend). - onGoalChange(serializeInlineMarkdown(goalNode)); + // Walk the WHOLE root (minus title H1 and StepsBlock decorator) so + // typing anywhere in the document body is captured. Previously we + // only read children[1] and silently discarded edits placed in the + // trailing area below the StepsBlock. + onGoalChange(serializeGoalFromRoot($getRoot())); }); }} /> @@ -873,6 +903,12 @@ export function DocumentEditor({ const tickRef = useRef(null); const deadlineRef = useRef(0); const editorRef = useRef(null); + // Tracks the most recent value the backend was asked to save. Once a poll + // confirms `directive.goal === lastSavedValueRef.current` AND + // `pendingGoalRef.current` still matches (i.e. user hasn't typed more), + // the draft is safe to drop from localStorage. Until then we keep the + // draft so an interrupted save doesn't lose user content. + const lastSavedValueRef = useRef(null); // Timestamp of the most recent localStorage draft write — drives the // "Draft saved Xs ago" indicator so users can SEE that drafts are working. const [draftSavedAt, setDraftSavedAt] = useState(null); @@ -922,17 +958,38 @@ export function DocumentEditor({ }, [directive.goal]); const fireSave = useCallback(async () => { - const next = pendingGoalRef.current; + // Read DIRECTLY from the editor so we don't trust pendingGoalRef alone. + // If the OnChangePlugin missed an event for any reason, this still + // captures the user's current document state. + let next = pendingGoalRef.current; + const editor = editorRef.current; + if (editor) { + editor.getEditorState().read(() => { + next = serializeGoalFromRoot($getRoot()); + }); + } + pendingGoalRef.current = next; + + // DEFENSE IN DEPTH: write the draft to localStorage BEFORE talking to + // the backend. If the save errors, the page closes mid-flight, or the + // network drops, the user's content survives in the draft. + try { + window.localStorage.setItem(DRAFT_KEY(directive.id), next); + setDraftSavedAt(Date.now()); + } catch (err) { + // eslint-disable-next-line no-console + console.warn("[makima] pre-save draft flush failed", err); + } + cancelTimers(); setSaveState("saving"); try { await onUpdateGoal(next); + lastSavedValueRef.current = next; setSaveState("saved"); - try { - window.localStorage.removeItem(DRAFT_KEY(directive.id)); - } catch { - /* ignore */ - } + // NOTE: we deliberately do NOT clear localStorage here. The roundtrip + // effect below clears it once the polled directive.goal confirms our + // save persisted AND the user hasn't kept typing past it. window.setTimeout(() => { // Only fade if no new edit has reopened a pending state in the meantime. setSaveState((s) => (s === "saved" ? "idle" : s)); @@ -943,10 +1000,30 @@ export function DocumentEditor({ setSaveState("error"); window.setTimeout(() => { setSaveState((s) => (s === "error" ? "idle" : s)); - }, 2500); + }, 4000); } }, [onUpdateGoal, directive.id]); + // Roundtrip-confirmed draft cleanup. Only drops the localStorage draft + // when the polled directive.goal matches what we just saved AND the + // user hasn't typed anything new in the meantime. Keeps the draft alive + // through every "we hit save but the page reloads before the poll lands" + // edge case. + useEffect(() => { + if ( + lastSavedValueRef.current !== null && + directive.goal === lastSavedValueRef.current && + pendingGoalRef.current === lastSavedValueRef.current + ) { + try { + window.localStorage.removeItem(DRAFT_KEY(directive.id)); + } catch { + /* ignore */ + } + lastSavedValueRef.current = null; + } + }, [directive.goal, directive.id]); + const startOrExtendCountdown = useCallback(() => { cancelTimers(); deadlineRef.current = Date.now() + countdownMs; -- cgit v1.2.3