From 63cd311842c4c1ed07942a25daa8a8237b1c4fca Mon Sep 17 00:00:00 2001 From: Twest2 Date: Thu, 29 Oct 2026 10:29:15 -0500 Subject: [PATCH 1/7] Let AI agents create, delete, reorder and replace guide content, snapshotting the guide before each change --- app/main.js | 15 +- app/renderer/app.js | 19 +- core/agent-tools.js | 377 ++++++++++++++++++++++++++++++++++- core/snapshots.js | 25 ++- core/store.js | 9 +- docs/AI_AGENTS.md | 45 +++-- docs/ARCHITECTURE.md | 10 +- docs/PRIVACY.md | 12 +- tests/unit/agent-mcp.test.js | 210 ++++++++++++++++++- tests/unit/store.test.js | 6 + 10 files changed, 690 insertions(+), 38 deletions(-) diff --git a/app/main.js b/app/main.js index 449d526c..edf7db5c 100644 --- a/app/main.js +++ b/app/main.js @@ -687,7 +687,13 @@ function watchAgentChanges() { } seenAt = changed.latest; for (const guideId of changed.guideIds) { - if (!store.guideExists(guideId)) continue; + if (!store.guideExists(guideId)) { + // The agent moved it to the Trash. Like any local Trash, that stays + // on this computer: cloud sync neither deletes nor restores it. + searchIndex.removeGuide(guideId); + sendToRenderer('guide:changed-by-agent', { guideId, deleted: true }); + continue; + } reindex(guideId); sendToRenderer('guide:changed-by-agent', { guideId }); } @@ -860,7 +866,13 @@ function setupIpc() { h('guide:quality', ({ guideId }) => qualityFindings(buildRenderAst(store, guideId, { globals: settings.getGlobalPlaceholders(), reviewDays: exportReviewDays(), }), stepNumberMap(orderedSteps(guideId))), { validate: (a) => c.id(a.guideId) }); + // An AI agent may have moved the guide to the Trash meanwhile; a late save + // from the editor must not recreate it, which would also block its restore. + const requireGuide = (guideId) => { + if (!store.guideExists(guideId)) throw new Error('This guide was moved to the Trash.'); + }; h('guide:save', ({ guide }) => { + requireGuide(guide.guideId); const saved = store.saveGuide(guide); reindex(guide.guideId); return saved; @@ -875,6 +887,7 @@ function setupIpc() { validate: (a) => c.id(a.guideId) && c.optionalBase64(a.imageBase64) && c.optionalNumber(a.position, 0, 100000), }); h('step:save', ({ guideId, step }) => { + requireGuide(guideId); const saved = store.saveStep(guideId, step); reindex(guideId); return saved; diff --git a/app/renderer/app.js b/app/renderer/app.js index 3e5af005..8ef19aa8 100644 --- a/app/renderer/app.js +++ b/app/renderer/app.js @@ -144,10 +144,25 @@ class StepForgeApp { await ensurePendingBrandRasters(api); } - /** An AI agent rewrote a guide's text: show it without losing the user's unsaved edits. */ + /** An AI agent changed a guide: show it without losing the user's unsaved edits. */ async onGuideChangedByAgent(payload) { if (!payload || !payload.guideId) return; - if (this.state.view === 'editor' && this.editor.guideId === payload.guideId) { + const open = this.state.view === 'editor' && this.editor.guideId === payload.guideId; + if (payload.deleted) { + if (open) { + // Saving now would write into a guide that is in the Trash. + this.editor.saveStepDebounced.cancel(); + this.editor.saveGuideDebounced.cancel(); + this.editor.pendingSave = false; + this.editor.pendingGuideSave = false; + await this.showLibrary(); + toast('An AI agent moved this guide to the Trash. You can restore it from there.', { ms: 6000 }); + } else if (this.state.view === 'library') { + await this.refreshLibrary(); + } + return; + } + if (open) { await this.editor.reloadAfterOutsideChange(); toast('An AI agent updated this guide.'); } else if (this.state.view === 'library') { diff --git a/core/agent-tools.js b/core/agent-tools.js index b0cc38a2..fea7b908 100644 --- a/core/agent-tools.js +++ b/core/agent-tools.js @@ -1,5 +1,7 @@ 'use strict'; +const fs = require('node:fs'); +const path = require('node:path'); const { htmlToMarkdown } = require('../exporters/htmlmd'); const { markdownHtml } = require('./placeholder-markdown'); const { sanitizeHtml } = require('./sanitize'); @@ -8,8 +10,11 @@ const { orderedBlocks, blockText } = require('./blocks'); const { placeholderText } = require('./placeholder-markdown'); const { ANNOTATION_TYPES, normalizeAnnotation } = require('./schema'); const { BUILT_IN_NAMES, cleanPlaceholderName, isValidPlaceholderName } = require('./ai-placeholders'); -const { withParent, parentMap } = require('./step-outline'); +const { withParent, parentMap, isValidOutline, moveSubtree } = require('./step-outline'); const { Settings } = require('./settings'); +const { decodePng } = require('./png'); +const { createAgentSnapshot } = require('./snapshots'); +const { forgetScreenshotChecks } = require('./redaction'); const { createBrandTools, BRAND_INSTRUCTIONS } = require('./agent-brand-tools'); const { normalizeAiBlock, @@ -22,8 +27,10 @@ const { /** * The tools AI agents (Claude, Codex, any MCP app) get through `StepForge --mcp`. * Agents read guides, rewrite their text, make substeps, add placeholders and - * draw annotations. They can't create or delete guides, steps, blocks, - * placeholders, annotations or screenshots: every step comes from a real capture. + * draw annotations. They can also create and delete guides and steps, move + * steps, replace or remove screenshots, and delete blocks and annotations; + * before each of those StepForge snapshots the guide, so the user can undo it. + * They can't change or delete placeholders, or delete blurs. */ // An agent may add this many blocks to a step per call; it can always call again. @@ -33,13 +40,16 @@ const MAX_DESCRIPTION_CHARS = 20000; const MAX_BLOCKS_PER_CALL = 20; const MAX_ANNOTATIONS_PER_CALL = 20; const MAX_PLACEHOLDER_VALUE_CHARS = 1000; +const MAX_IDS_PER_CALL = 50; +const MAX_SCREENSHOT_BYTES = 50 * 1024 * 1024; const COLOR_RE = /^#(?:[0-9a-fA-F]{3}|[0-9a-fA-F]{6})$/; const INSTRUCTIONS = [ 'StepForge holds step-by-step guides a person recorded: each step is a screenshot of one click, with a marker where they clicked.', 'You can read guides and rewrite their text: the guide title and description, and each step\'s title, description and blocks (notes, warnings, tips, code, tables).', 'You can also make a step a substep of an earlier step (update_step parent_step_id), add placeholders (create_placeholder), and draw annotations such as rectangles, arrows and callouts on a screenshot (add_annotations).', - 'You cannot reorder steps, or create or delete guides, steps, blocks, placeholders, annotations or screenshots.', + 'You can also create and delete guides and steps (create_guide, delete_guide, create_step, delete_step), move steps (move_step), replace or remove a step\'s screenshot with a PNG file on this computer (set_screenshot, remove_screenshot), and delete blocks and annotations (delete_blocks, delete_annotations). StepForge snapshots the guide before each of these, so the user can undo it.', + 'Steps normally come from a real capture, so prefer rewriting what is there over deleting it. You cannot change or delete placeholders, or delete blurs.', 'To write a guide: call get_guide, then get_step for each step to see its screenshot, then update_step. Finish with update_guide for the guide\'s title and description.', 'Step titles are short imperative actions ("Click Save", "Open Settings"). Descriptions are Markdown, 1–2 sentences per step.', 'Placeholders: get_guide lists them. In descriptions and blocks, write [[Name]] where a placeholder\'s value would appear; keep titles in plain words. Create a placeholder only for a specific value that repeats, such as a product name, version, URL or course code.', @@ -136,6 +146,13 @@ function stepForAgent(step, number) { const text = (value) => ({ type: 'text', text: typeof value === 'string' ? value : JSON.stringify(value, null, 2) }); +/** The kind of a step with no screenshot: content when it has text, else empty. */ +function kindWithoutScreenshot(step) { + const hasBody = Boolean(step.title || htmlToMarkdown(step.descriptionHtml || '').trim() + || orderedBlocks(step).length); + return hasBody ? 'content' : 'empty'; +} + function checkLength(value, max, what) { if (value !== undefined && (typeof value !== 'string' || value.length > max)) { throw new AgentError(`${what} must be text of at most ${max} characters.`); @@ -232,6 +249,47 @@ function createAgentTools({ store, agentSettings, onChange = () => {}, onBrandin } }; + /** The guide's step order and each step's parent. */ + const outlineOf = (guide) => { + const steps = store.listSteps(guide.guideId); + const order = [...(guide.stepsOrder || [])]; + return { order, parentOf: new Map(order.map((id) => [id, steps.get(id)?.parentStepId || null])) }; + }; + + /** Take a snapshot before a change the user may want to undo, and say so to the agent. */ + const snapshotBefore = (guideId, action) => { + createAgentSnapshot(store, guideId, action); + return 'StepForge saved a snapshot of the guide first, so the user can undo this in More → Backups & snapshots.'; + }; + + /** A PNG file on this computer, named by its full path, as screenshot bytes and size. */ + const readScreenshot = (filePath) => { + if (typeof filePath !== 'string' || !path.isAbsolute(filePath)) { + throw new AgentError('image_path must be the full path to a PNG file on this computer.'); + } + let stat; + try { stat = fs.statSync(filePath); } catch { throw new AgentError(`There's no file at ${filePath}.`); } + if (!stat.isFile()) throw new AgentError(`${filePath} isn't a file.`); + if (stat.size > MAX_SCREENSHOT_BYTES) throw new AgentError(`${filePath} is larger than ${MAX_SCREENSHOT_BYTES / 1024 / 1024} MB.`); + const png = fs.readFileSync(filePath); + let image; + try { + image = decodePng(png); + } catch { + throw new AgentError(`StepForge can't read ${filePath} as a PNG. Convert it to PNG and try again.`); + } + return { png, size: { width: image.width, height: image.height } }; + }; + + /** Ids that aren't in `known`, as an error the agent can act on. */ + const checkIds = (ids, known, what, where) => { + if (!Array.isArray(ids) || !ids.length || ids.length > MAX_IDS_PER_CALL || !ids.every((id) => typeof id === 'string')) { + throw new AgentError(`Give 1 to ${MAX_IDS_PER_CALL} ${what} ids.`); + } + const unknown = ids.filter((id) => !known.has(id)); + if (unknown.length) throw new AgentError(`${where} has no ${what} ${unknown.map((id) => `"${id}"`).join(', ')}. Call get_step for its ${what} ids.`); + }; + const tools = [ { name: 'list_guides', @@ -316,6 +374,32 @@ function createAgentTools({ store, agentSettings, onChange = () => {}, onBrandin return { content }; }, }, + { + name: 'create_guide', + description: 'Create a new, empty guide. Add its steps with create_step.', + annotations: { readOnlyHint: false, destructiveHint: false, idempotentHint: false }, + inputSchema: { + type: 'object', + properties: { + title: { type: 'string', description: 'What the reader will get done, e.g. "Submit a lab in Canvas".' }, + description: { type: 'string', description: 'Markdown. 1–3 sentences introducing the guide.' }, + }, + required: ['title'], + additionalProperties: false, + }, + handler({ title, description } = {}) { + requireEnabled(); + checkLength(title, MAX_TITLE_CHARS, 'title'); + checkLength(description, MAX_DESCRIPTION_CHARS, 'description'); + if (!normalizeWhitespace(title || '')) throw new AgentError('Give the guide a title.'); + const guide = store.createGuide({ + title: normalizeWhitespace(title), + descriptionHtml: description ? sanitizeHtml(markdownHtml(description)) : '', + }); + onChange(guide.guideId); + return { content: [text({ guide_id: guide.guideId, title: guide.title, description: descriptionMarkdown(guide.descriptionHtml) })] }; + }, + }, { name: 'update_guide', description: 'Rewrite the guide\'s title and/or description (Markdown). Leave a field out to keep it.', @@ -351,10 +435,92 @@ function createAgentTools({ store, agentSettings, onChange = () => {}, onBrandin return { content: [text({ guide_id: saved.guideId, title: saved.title, description: descriptionMarkdown(saved.descriptionHtml) })] }; }, }, + { + name: 'delete_guide', + description: 'Move a guide, with all its steps, to StepForge\'s Trash. The user can restore it from there for 30 days.', + annotations: { readOnlyHint: false, destructiveHint: true, idempotentHint: false }, + inputSchema: { + type: 'object', + properties: { guide_id: { type: 'string' } }, + required: ['guide_id'], + additionalProperties: false, + }, + handler({ guide_id: guideId } = {}) { + requireEnabled(); + const guide = loadGuide(guideId); + // The snapshot goes to the Trash with the guide, and comes back with it. + createAgentSnapshot(store, guideId, 'deletes the guide'); + store.deleteGuide(guideId); + onChange(guideId); + return { content: [text(`Moved "${guide.title}" to the Trash. The user can restore it from there.`)] }; + }, + }, + { + name: 'create_step', + description: 'Add a step to a guide, optionally with a screenshot from a PNG file on this computer. Add blocks with update_step and annotations with add_annotations.', + annotations: { readOnlyHint: false, destructiveHint: false, idempotentHint: false }, + inputSchema: { + type: 'object', + properties: { + guide_id: { type: 'string' }, + title: { type: 'string', description: 'A short imperative action, e.g. "Click Save".' }, + description: { type: 'string', description: 'Markdown. 1–2 sentences on what the person does.' }, + after_step_id: { + type: ['string', 'null'], + description: 'Put the new step right after this step, or null for the very top. Leave out to add it at the end.', + }, + parent_step_id: { + type: ['string', 'null'], + description: 'Make the new step a substep of this step, or null for the top level. The parent must be the step right above it or one of that step\'s parents. ' + + 'Leave out to nest it like the step above, or under that step when its substeps follow.', + }, + image_path: { type: 'string', description: 'Full path to a PNG file to use as the screenshot.' }, + }, + required: ['guide_id'], + additionalProperties: false, + }, + handler({ guide_id: guideId, title, description, after_step_id: afterStepId, parent_step_id: parentStepId, image_path: imagePath } = {}) { + requireEnabled(); + const guide = loadGuide(guideId); + checkLength(title, MAX_TITLE_CHARS, 'title'); + checkLength(description, MAX_DESCRIPTION_CHARS, 'description'); + if (afterStepId !== undefined && afterStepId !== null) loadStep(guide, afterStepId); + if (parentStepId !== undefined && parentStepId !== null) loadStep(guide, parentStepId); + const { order, parentOf } = outlineOf(guide); + const at = afterStepId === undefined ? order.length : afterStepId === null ? 0 : order.indexOf(afterStepId) + 1; + const NEW = '\0new'; + const nextOrder = [...order.slice(0, at), NEW, ...order.slice(at)]; + const fits = (parentId) => isValidOutline(nextOrder, new Map(parentOf).set(NEW, parentId)); + let parent; + if (parentStepId !== undefined) { + parent = parentStepId || null; + if (!fits(parent)) { + throw new AgentError('That parent would break the step numbering. A substep\'s parent must be the step right above it, or one of that step\'s parents.'); + } + } else { + // Beside the step above, or under it when its substeps follow, or under one of its parents. + const above = at > 0 ? order[at - 1] : null; + const candidates = [parentOf.get(above) || null, above]; + for (let p = parentOf.get(above); p; p = parentOf.get(p)) candidates.push(parentOf.get(p) || null); + parent = candidates.find(fits) ?? null; + } + const screenshot = imagePath !== undefined ? readScreenshot(imagePath) : null; + const fields = { + title: normalizeWhitespace(title || ''), + descriptionHtml: description ? sanitizeHtml(markdownHtml(description)) : '', + parentStepId: parent, + }; + if (!screenshot) fields.kind = kindWithoutScreenshot(fields); + const note = snapshotBefore(guideId, 'adds a step'); + const step = store.addStep(guideId, fields, screenshot?.png || null, screenshot?.size || null, { position: at }); + onChange(guideId); + return { content: [text(stepForAgent(step, numberOf(store.getGuide(guideId), step.stepId))), text(note)] }; + }, + }, { name: 'update_step', description: 'Rewrite a step\'s title, description (Markdown) and blocks, or make it a substep. Leave a field out to keep it. ' - + 'Blocks with an "id" from get_step are rewritten in place; blocks without one are added (at most 5 per call). Blocks are never deleted.', + + 'Blocks with an "id" from get_step are rewritten in place; blocks without one are added (at most 5 per call). Use delete_blocks to delete one.', annotations: { readOnlyHint: false, destructiveHint: false, idempotentHint: false }, inputSchema: { type: 'object', @@ -433,6 +599,167 @@ function createAgentTools({ store, agentSettings, onChange = () => {}, onBrandin return { content: [text(stepForAgent(saved, numberOf(store.getGuide(guideId), stepId)))] }; }, }, + { + name: 'move_step', + description: 'Move a step, with its substeps, to right after another step (or to the top). StepForge nests it so the numbering still reads top to bottom; ' + + 'give parent_step_id to choose its parent at the new place.', + annotations: { readOnlyHint: false, destructiveHint: false, idempotentHint: true }, + inputSchema: { + type: 'object', + properties: { + guide_id: { type: 'string' }, + step_id: { type: 'string' }, + after_step_id: { + type: ['string', 'null'], + description: 'Put the step right after this one, or null for the very top. To go after a step and all its substeps, give its last substep.', + }, + parent_step_id: { type: ['string', 'null'], description: 'Its parent at the new place, or null for the top level.' }, + }, + required: ['guide_id', 'step_id', 'after_step_id'], + additionalProperties: false, + }, + handler({ guide_id: guideId, step_id: stepId, after_step_id: afterStepId, parent_step_id: parentStepId } = {}) { + requireEnabled(); + const guide = loadGuide(guideId); + loadStep(guide, stepId); + if (afterStepId === undefined) throw new AgentError('Give after_step_id: a step id, or null for the very top.'); + if (afterStepId !== null) loadStep(guide, afterStepId); + if (parentStepId !== undefined && parentStepId !== null) loadStep(guide, parentStepId); + const { order, parentOf } = outlineOf(guide); + const moved = afterStepId === stepId ? null : moveSubtree(order, parentOf, stepId, afterStepId === null ? 0 : order.indexOf(afterStepId) + 1); + if (!moved) throw new AgentError('A step can\'t move into its own substeps.'); + if (parentStepId !== undefined) { + moved.parentOf = withParent(moved.order, moved.parentOf, stepId, parentStepId); + if (!moved.parentOf) { + throw new AgentError('That parent would break the step numbering there. A substep\'s parent must be the step right above it, or one of that step\'s parents.'); + } + } + const note = snapshotBefore(guideId, `moves step ${numberOf(guide, stepId)}`); + store.reorderSteps(guideId, moved.order, Object.fromEntries(moved.parentOf)); + onChange(guideId); + const now = store.getGuide(guideId); + return { + content: [text({ + steps: numberedSteps(store, now).map(({ step, number }) => ({ step_id: step.stepId, number, title: step.title || '' })), + }), text(note)], + }; + }, + }, + { + name: 'delete_step', + description: 'Delete a step and its screenshot. Its substeps move up a level.', + annotations: { readOnlyHint: false, destructiveHint: true, idempotentHint: false }, + inputSchema: { + type: 'object', + properties: { guide_id: { type: 'string' }, step_id: { type: 'string' } }, + required: ['guide_id', 'step_id'], + additionalProperties: false, + }, + handler({ guide_id: guideId, step_id: stepId } = {}) { + requireEnabled(); + const guide = loadGuide(guideId); + const step = loadStep(guide, stepId); + const note = snapshotBefore(guideId, `deletes step ${numberOf(guide, stepId)}`); + store.deleteStep(guideId, stepId); + onChange(guideId); + return { content: [text(`Deleted "${step.title || 'Untitled step'}". ${note}`)] }; + }, + }, + { + name: 'delete_blocks', + description: 'Delete blocks (notes, warnings, tips, code, tables) from a step, by the ids get_step gives.', + annotations: { readOnlyHint: false, destructiveHint: true, idempotentHint: true }, + inputSchema: { + type: 'object', + properties: { + guide_id: { type: 'string' }, + step_id: { type: 'string' }, + block_ids: { type: 'array', items: { type: 'string' }, minItems: 1, maxItems: MAX_IDS_PER_CALL }, + }, + required: ['guide_id', 'step_id', 'block_ids'], + additionalProperties: false, + }, + handler({ guide_id: guideId, step_id: stepId, block_ids: blockIds } = {}) { + requireEnabled(); + const guide = loadGuide(guideId); + const step = loadStep(guide, stepId); + checkIds(blockIds, new Set(orderedBlocks(step).map((block) => block.id)), 'block', `Step "${stepId}"`); + const doomed = new Set(blockIds); + const note = snapshotBefore(guideId, `deletes blocks in step ${numberOf(guide, stepId)}`); + const saved = saveWithRetry( + () => store.getStep(guideId, stepId), + (current) => ({ + ...current, + textBlocks: (current.textBlocks || []).filter((b) => !doomed.has(b.id)), + codeBlocks: (current.codeBlocks || []).filter((b) => !doomed.has(b.id)), + tableBlocks: (current.tableBlocks || []).filter((b) => !doomed.has(b.id)), + }), + (current, revision) => store.saveStep(guideId, current, { expectedRevision: revision }), + ); + onChange(guideId); + return { content: [text(stepForAgent(saved, numberOf(store.getGuide(guideId), stepId))), text(note)] }; + }, + }, + { + name: 'set_screenshot', + description: 'Replace a step\'s screenshot (or give a step without one a screenshot) with a PNG file on this computer. ' + + 'The step\'s text and annotations stay; check the annotations still line up, and delete any that don\'t with delete_annotations.', + annotations: { readOnlyHint: false, destructiveHint: true, idempotentHint: true }, + inputSchema: { + type: 'object', + properties: { + guide_id: { type: 'string' }, + step_id: { type: 'string' }, + image_path: { type: 'string', description: 'Full path to a PNG file.' }, + }, + required: ['guide_id', 'step_id', 'image_path'], + additionalProperties: false, + }, + handler({ guide_id: guideId, step_id: stepId, image_path: imagePath } = {}) { + requireEnabled(); + const guide = loadGuide(guideId); + loadStep(guide, stepId); + const { png, size } = readScreenshot(imagePath); + const note = snapshotBefore(guideId, `replaces the screenshot of step ${numberOf(guide, stepId)}`); + const step = store.getStep(guideId, stepId); + // Private-detail blurs matched the old screenshot; the new one is checked again. + forgetScreenshotChecks(step); + const saved = store.replaceImages(guideId, stepId, { original: png }, size, step); + onChange(guideId); + return { content: [text(stepForAgent(saved, numberOf(guide, stepId))), text(note)] }; + }, + }, + { + name: 'remove_screenshot', + description: 'Remove a step\'s screenshot, with the annotations drawn on it. The step keeps its text and blocks.', + annotations: { readOnlyHint: false, destructiveHint: true, idempotentHint: true }, + inputSchema: { + type: 'object', + properties: { guide_id: { type: 'string' }, step_id: { type: 'string' } }, + required: ['guide_id', 'step_id'], + additionalProperties: false, + }, + handler({ guide_id: guideId, step_id: stepId } = {}) { + requireEnabled(); + const guide = loadGuide(guideId); + const step = loadStep(guide, stepId); + if (!step.image) throw new AgentError('This step has no screenshot.'); + const note = snapshotBefore(guideId, `removes the screenshot of step ${numberOf(guide, stepId)}`); + const { originalPath, workingPath } = step.image; + const saved = saveWithRetry( + () => store.getStep(guideId, stepId), + (current) => { + const next = { ...current, image: null, annotations: [] }; + forgetScreenshotChecks(next); + return { ...next, kind: kindWithoutScreenshot(next) }; + }, + (current, revision) => store.saveStep(guideId, current, { expectedRevision: revision }), + ); + for (const file of new Set([originalPath, workingPath])) fs.rmSync(path.join(store.stepDir(guideId, stepId), file), { force: true }); + onChange(guideId); + return { content: [text(stepForAgent(saved, numberOf(guide, stepId))), text(note)] }; + }, + }, { name: 'create_placeholder', description: 'Add a placeholder that guides can use as [[Name]]. "guide" placeholders belong to one guide; "global" ones are shared by every guide. Existing placeholders are never changed.', @@ -482,7 +809,7 @@ function createAgentTools({ store, agentSettings, onChange = () => {}, onBrandin { name: 'add_annotations', description: 'Draw annotations on a step\'s screenshot: rect, oval, line, arrow, text, tooltip (a callout box), number (a numbered badge), blur, highlight, magnify, cursor. ' - + 'Coordinates are pixels on the screenshot from get_step (see screenshot_size). Annotations are added, never removed.', + + 'Coordinates are pixels on the screenshot from get_step (see screenshot_size). Annotations are added beside what is there; delete_annotations removes them.', annotations: { readOnlyHint: false, destructiveHint: false, idempotentHint: false }, inputSchema: { type: 'object', @@ -540,6 +867,44 @@ function createAgentTools({ store, agentSettings, onChange = () => {}, onBrandin }; }, }, + { + name: 'delete_annotations', + description: 'Delete annotations from a step\'s screenshot, by the ids get_step gives. Blurs can\'t be deleted: they hide what the person wants kept private.', + annotations: { readOnlyHint: false, destructiveHint: true, idempotentHint: true }, + inputSchema: { + type: 'object', + properties: { + guide_id: { type: 'string' }, + step_id: { type: 'string' }, + annotation_ids: { type: 'array', items: { type: 'string' }, minItems: 1, maxItems: MAX_IDS_PER_CALL }, + }, + required: ['guide_id', 'step_id', 'annotation_ids'], + additionalProperties: false, + }, + handler({ guide_id: guideId, step_id: stepId, annotation_ids: annotationIds } = {}) { + requireEnabled(); + const guide = loadGuide(guideId); + const step = loadStep(guide, stepId); + const annotations = step.annotations || []; + checkIds(annotationIds, new Set(annotations.map((ann) => ann.id)), 'annotation', `Step "${stepId}"`); + // Removing a blur would show the agent what it hides on the next get_step. + const isBlur = (ann) => ann.type === 'blur' || ann.redact; + const blurs = annotations.filter((ann) => annotationIds.includes(ann.id) && isBlur(ann)); + if (blurs.length) { + throw new AgentError(`${blurs.map((ann) => `"${ann.id}"`).join(', ')} ${blurs.length === 1 ? 'is a blur' : 'are blurs'}, which hide what the person wants kept private. Agents can't delete blurs.`); + } + const doomed = new Set(annotationIds); + const note = snapshotBefore(guideId, `deletes annotations in step ${numberOf(guide, stepId)}`); + const saved = saveWithRetry( + () => store.getStep(guideId, stepId), + // A blur the person drew meanwhile, with an id the agent named, still stays. + (current) => ({ ...current, annotations: (current.annotations || []).filter((ann) => !doomed.has(ann.id) || isBlur(ann)) }), + (current, revision) => store.saveStep(guideId, current, { expectedRevision: revision }), + ); + onChange(guideId); + return { content: [text({ deleted: annotationIds.length, annotations_on_step: (saved.annotations || []).length }), text(note)] }; + }, + }, ...createBrandTools({ settingsDir: store.settingsDir, requireEnabled, agentSettings, onChange: onBrandingChange, AgentError, text }), ]; diff --git a/core/snapshots.js b/core/snapshots.js index 1ff4424d..b63e736f 100644 --- a/core/snapshots.js +++ b/core/snapshots.js @@ -24,6 +24,11 @@ const SNAPSHOT_EXT = '.snapshot'; // A file saved for a snapshot whose manifest isn't written yet is this new; // cleanup leaves files this young alone. const FRESH_FILE_MS = 10 * 60 * 1000; +// Snapshots taken before an AI agent's change start their label with this. +// Pruning the others leaves them alone, so a run of agent changes can always +// be undone back to its start; only the newest AGENT_KEEP_LAST are kept. +const AGENT_LABEL = 'before_agent_'; +const AGENT_KEEP_LAST = 50; function snapshotsDir(store, guideId) { return path.join(store.guideDir(guideId), 'history', 'snapshots'); @@ -34,6 +39,7 @@ function filesDir(store, guideId) { } const isSnapshotName = (name) => name.endsWith(SNAPSHOT_EXT) || name.endsWith('.zip'); +const isAgentSnapshot = (name) => name.includes(`Z-${AGENT_LABEL}`); const notHistory = (rel) => rel !== 'history' && !rel.startsWith('history/'); let lastSnapshotMs = 0; @@ -75,6 +81,20 @@ function createSnapshot(store, guideId, { label = '', keepLast = 0, background = return take(); } +/** + * Snapshot a guide before an AI agent changes it in a way the user may want + * to undo, labeled with what the agent is about to do ("deletes step 3"). + */ +function createAgentSnapshot(store, guideId, action) { + const name = createSnapshot(store, guideId, { label: `${AGENT_LABEL}${action}` }); + const agents = listSnapshots(store, guideId).filter(isAgentSnapshot); + if (agents.length > AGENT_KEEP_LAST) { + for (const old of agents.slice(AGENT_KEEP_LAST)) fs.rmSync(path.join(snapshotsDir(store, guideId), old), { force: true }); + removeUnusedFiles(store, guideId); + } + return name; +} + function listSnapshots(store, guideId) { const dir = snapshotsDir(store, guideId); if (!fs.existsSync(dir)) return []; @@ -105,8 +125,9 @@ function removeUnusedFiles(store, guideId, now = Date.now()) { } } +/** Keep the newest `keepLast` snapshots, not counting agent ones (see createAgentSnapshot). */ function pruneSnapshots(store, guideId, keepLast) { - const all = listSnapshots(store, guideId); + const all = listSnapshots(store, guideId).filter((name) => !isAgentSnapshot(name)); for (const name of all.slice(keepLast)) { fs.rmSync(path.join(snapshotsDir(store, guideId), name), { force: true }); } @@ -321,6 +342,6 @@ async function autoSnapshotIfDue(store, guideId, settings) { } module.exports = { - createSnapshot, listSnapshots, pruneSnapshots, restoreSnapshot, snapshotsDir, + createSnapshot, createAgentSnapshot, listSnapshots, pruneSnapshots, restoreSnapshot, snapshotsDir, autoSnapshotIfDue, convertZipSnapshots, removeUnusedFiles, compareSnapshot, snapshotFile, }; diff --git a/core/store.js b/core/store.js index c557aad2..12ec6f35 100644 --- a/core/store.js +++ b/core/store.js @@ -398,10 +398,13 @@ class GuideStore { deleteStep(guideId, stepId) { const guide = this.getGuide(guideId); - // Re-parent substeps of the deleted step to the top level. - for (const [, step] of this.listSteps(guideId)) { + // Substeps of the deleted step move up a level, to its parent (or the + // top level), so the outline still reads top to bottom. + const steps = this.listSteps(guideId); + const parentId = steps.get(stepId)?.parentStepId || null; + for (const [, step] of steps) { if (step.parentStepId === stepId) { - step.parentStepId = null; + step.parentStepId = parentId; writeJsonSync(path.join(this.stepDir(guideId, step.stepId), 'step.json'), step); } } diff --git a/docs/AI_AGENTS.md b/docs/AI_AGENTS.md index a78daf71..47d6fdb3 100644 --- a/docs/AI_AGENTS.md +++ b/docs/AI_AGENTS.md @@ -17,18 +17,36 @@ window opens, and nothing runs when no agent is connected. | An agent can | An agent can't | | --- | --- | -| List and read your guides | Create or delete guides or steps | -| See each step's screenshot, with your blurs already filled in | Reorder steps | -| Rewrite a guide's title and description | Change or delete screenshots | -| Rewrite each step's title, description, and blocks, and add notes, warnings, tips, and code | Delete blocks, annotations, or placeholders | -| Make a step a substep of an earlier step | Change a placeholder you already have | -| Add guide and global placeholders, and use them as `[[Name]]` | Read anything outside your StepForge library, except an image or font file you ask it to add to a brand profile | -| Draw on screenshots: rectangles, ovals, lines, arrows, text, callouts, numbered badges, blurs, highlights, magnifiers, and cursors | Delete brand profiles | +| List and read your guides | Change a placeholder you already have, or delete one | +| See each step's screenshot, with your blurs already filled in | Delete blurs, or see what they hide | +| Create guides, and move them to the Trash | Delete brand profiles | +| Rewrite a guide's title and description | Read anything outside your StepForge library, except a PNG you ask it to use as a screenshot, or an image or font file you ask it to add to a brand profile | +| Add, delete, and reorder steps, and make a step a substep of an earlier step | | +| Rewrite each step's title, description, and blocks, add notes, warnings, tips, and code, and delete blocks | | +| Replace a step's screenshot with a PNG file you point it to, or remove the screenshot | | +| Add guide and global placeholders, and use them as `[[Name]]` | | +| Draw on screenshots: rectangles, ovals, lines, arrows, text, callouts, numbered badges, blurs, highlights, magnifiers, and cursors. Delete what's drawn, except blurs | | | If you allow it: make and change [brand profiles](BRANDING.md#let-an-ai-agent-set-up-your-brand), add a logo, banner or font file you point it to, and choose the active profile | | -Every step comes from a real capture, so an agent works around your -screenshots: it writes the text, organizes the steps, and adds annotations on -top, but the screenshots themselves never change. +Most of the time an agent works around your screenshots: it writes the text, +organizes the steps, and adds annotations on top. When it does more than that, +StepForge keeps a way back. + +### Undoing what an agent did + +Before an agent deletes, moves, or adds a step, replaces or removes a +screenshot, or deletes blocks or annotations, StepForge saves a snapshot of the +guide. To undo the change, open the guide and choose **More → Backups & +snapshots**. Each agent snapshot is named for what the agent was about to do, +for example "before agent deletes step 3". **Restore** the one from just before +the change you want to undo. Restoring snapshots the guide first, so a restore +can be undone too. + +Automatic backups don't push agent snapshots out. StepForge keeps the newest 50 +agent snapshots of each guide. + +A guide an agent deletes goes to the Trash, like one you delete. Restore it +from there. ## 1. Turn on agent access @@ -71,7 +89,8 @@ of seconds. If you're typing in the same step at that moment, your edit wins. Before you share the guide, read what the agent wrote. To undo a big rewrite, use **More → Backups & snapshots**. Annotations an agent drew can be moved or -deleted like your own. +deleted like your own. If you have the guide open when an agent moves it to +the Trash, StepForge takes you back to the library. ## Privacy @@ -81,7 +100,9 @@ its AI provider**, such as Anthropic for Claude or OpenAI for Codex. Their privacy terms apply to that data. - Screenshots are shared with your blurs filled in, so the agent never sees - what you hid. + what you hid. Agents can't delete blurs. +- An agent reads a file outside your library only when it uses one as a + screenshot or adds one to a brand profile, and only the file it names. - Agents never get the text you typed while recording, field values, or the text StepForge read from the screen. - Turning off **Let AI agents edit your guides** cuts off every connected diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 8428cc04..b67814cf 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -49,8 +49,10 @@ Internal working storage is **folder-based**; sharing/backups use a step.json original.png never mutated after capture working.png only once cropped; annotations stay vector JSON - history/snapshots/*.snapshot automated + manual backups: gzipped - manifests listing each file by SHA-256 + history/snapshots/*.snapshot automated, manual and agent backups: + gzipped manifests listing each file by + SHA-256. Agent ones (*-before_agent_*) are + pruned apart from the rest, to the newest 50 history/snapshot-files/ screenshots, stored once and shared by every snapshot that contains them index/search-index.json inverted full-text index @@ -128,7 +130,9 @@ Export dialog choice. In the profile editor, **Pick from logo** runs `core/brand-palette.js` on a small canvas drawing of the logo, and **Write with AI** asks Ollama for CSS (`core/ai-brand-css.js`, which also strips `@import` and remote `url()`s); it's shown only when `ai:status` reports AI on -and the model installed. AI agents reach profiles through +and the model installed. AI agents edit guides through `core/agent-tools.js`; +each tool that deletes, moves or replaces something first calls +`createAgentSnapshot` (`core/snapshots.js`). AI agents reach profiles through `core/agent-brand-tools.js`, behind `ai.agents.branding`. The `--mcp` process records each change in `agent-changes.json`; the running app saves an agent's choice of active profile itself (it holds Settings in memory) and has the diff --git a/docs/PRIVACY.md b/docs/PRIVACY.md index ca1e7436..3a96a6f9 100644 --- a/docs/PRIVACY.md +++ b/docs/PRIVACY.md @@ -107,9 +107,15 @@ agents edit your guides** and connect an agent such as Claude Code or Codex - Screenshots are shared with your blurs filled in. Agents never get the text you typed while recording, field values, or the text StepForge read from the screen. -- Agents can rewrite text, make substeps, add placeholders, and draw - annotations. They can't reorder steps, change screenshots, or delete - anything. +- Agents can rewrite text, add placeholders, draw annotations, create guides + and steps, reorder steps, replace or remove screenshots, delete steps, + blocks and annotations, and move guides to the Trash. StepForge snapshots a + guide before each change that deletes, moves or replaces something, and keeps + those snapshots in the guide's folder. Agents can't change or delete + placeholders, or delete blurs. +- To use a PNG file as a screenshot, an agent names it by its full path, and + StepForge copies it into the guide. StepForge reads no other file outside + your library for an agent, apart from brand files (below). - With **Let AI agents edit brand profiles** on (it's off by default), agents can also read and change brand profiles, see the logo, and add an image or font file they name on your computer to a profile. They can't delete a diff --git a/tests/unit/agent-mcp.test.js b/tests/unit/agent-mcp.test.js index 6cf9d050..480403db 100644 --- a/tests/unit/agent-mcp.test.js +++ b/tests/unit/agent-mcp.test.js @@ -6,7 +6,7 @@ const { spawn } = require('node:child_process'); const test = require('node:test'); const assert = require('node:assert/strict'); -const { makeTmpDir, rmrf } = require('./helpers'); +const { makeTmpDir, rmrf, TINY_PNG } = require('./helpers'); const { GuideStore } = require('../../core/store'); const { Settings } = require('../../core/settings'); const { encodePng, decodePng } = require('../../core/png'); @@ -14,6 +14,7 @@ const { createImage, fillRect } = require('../../core/raster'); const { createMcpServer, PROTOCOL_VERSIONS } = require('../../core/mcp-server'); const { createAgentTools, INSTRUCTIONS } = require('../../core/agent-tools'); const { recordAgentChange, agentChangesSince } = require('../../core/agent-changes'); +const { listSnapshots, restoreSnapshot, pruneSnapshots, createSnapshot, createAgentSnapshot } = require('../../core/snapshots'); const ROOT = path.resolve(__dirname, '..', '..'); @@ -49,18 +50,22 @@ function makeLibrary(t, agents = { enabled: true, screenshots: true }) { return { dir, store, settings, guide, first, second, tools, call, json, changes }; } -test('agents get read, rewrite and add tools only: nothing deletes, reorders, or creates steps', (t) => { +test('agents get every guide tool except changing or deleting placeholders', (t) => { const { tools } = makeLibrary(t); assert.deepEqual(tools.map((tool) => tool.name), [ - 'list_guides', 'get_guide', 'get_step', 'update_guide', 'update_step', 'create_placeholder', 'add_annotations', + 'list_guides', 'get_guide', 'get_step', 'create_guide', 'update_guide', 'delete_guide', + 'create_step', 'update_step', 'move_step', 'delete_step', 'delete_blocks', 'set_screenshot', 'remove_screenshot', + 'create_placeholder', 'add_annotations', 'delete_annotations', // Behind their own switch (tests/unit/agent-brand-tools.test.js). 'list_brand_profiles', 'get_brand_profile', 'create_brand_profile', 'update_brand_profile', 'set_brand_file', 'set_active_brand_profile', ]); for (const tool of tools) { assert.equal(tool.inputSchema.type, 'object'); - assert.ok(!/delete|remove|add_step|create_guide|create_step|reorder/i.test(tool.name)); + assert.ok(!/placeholder/.test(tool.name) || tool.name === 'create_placeholder'); + assert.equal(tool.annotations.destructiveHint === true, /^(delete|remove|set_screenshot)/.test(tool.name), tool.name); } - assert.match(INSTRUCTIONS, /cannot reorder steps, or create or delete guides, steps/); + assert.match(INSTRUCTIONS, /snapshots the guide before each of these/); + assert.match(INSTRUCTIONS, /cannot change or delete placeholders/); }); test('every tool refuses, with a fix the user can act on, while agent access is off', async (t) => { @@ -106,7 +111,7 @@ test('a step screenshot comes with its blurs filled, and not at all when screens assert.match(textOnly.content[1].text, /Screenshots are turned off/); }); -test('agents rewrite step and guide text; blocks merge by id and are never deleted', async (t) => { +test('agents rewrite step and guide text; update_step merges blocks by id and never deletes them', async (t) => { const { call, json, guide, first, store, changes } = makeLibrary(t); const before = store.getStep(guide.guideId, first.stepId); const result = json(await call('update_step', { @@ -329,3 +334,196 @@ test('on Windows, `--mcp` reruns StepForge as plain Node so piped stdin works', assert.deepEqual(run({ platform: 'win32', electron: false }).required, ['./mcp']); assert.deepEqual(run({ platform: 'win32', electron: true, argv: ['StepForge.exe'] }).required, ['./main']); }); + +/** The guide's agent snapshots, oldest first. */ +const agentSnapshots = (store, guideId) => listSnapshots(store, guideId).filter((name) => name.includes('before_agent_')).reverse(); + +/** A PNG file of `width` x `height` in `color`, in a fresh folder outside the library. */ +function pngFile(t, width, height, color) { + const dir = makeTmpDir('agent-png'); + t.after(() => rmrf(dir)); + const file = path.join(dir, 'shot.png'); + fs.writeFileSync(file, encodePng(createImage(width, height, color))); + return file; +} + +test('agents create guides and steps, nested where they land, with a snapshot before each step', async (t) => { + const { call, json, store, changes } = makeLibrary(t); + const made = json(await call('create_guide', { title: ' Reset a password ', description: 'For **staff**.' })); + assert.equal(made.title, 'Reset a password'); + assert.equal(store.getGuide(made.guide_id).descriptionHtml, '

For staff.

'); + assert.match((await call('create_guide', { title: ' ' })).content[0].text, /Give the guide a title/); + + const id = made.guide_id; + const shot = pngFile(t, 30, 20, [0, 128, 0, 255]); + const first = json(await call('create_step', { guide_id: id, title: 'Open Settings', image_path: shot })); + assert.equal(first.number, '1'); + assert.deepEqual(first.screenshot_size, { width: 30, height: 20 }); + assert.ok(store.getStep(id, first.step_id).image, 'the PNG became the screenshot'); + assert.ok(decodePng(fs.readFileSync(store.stepImagePath(id, first.step_id, 'original'))).width === 30); + const sub = json(await call('create_step', { guide_id: id, title: 'Choose Security', parent_step_id: first.step_id })); + assert.equal(sub.number, '1.1'); + assert.equal(store.getStep(id, sub.step_id).kind, 'content', 'a text-only step'); + // Right after a step whose substeps follow, a new step goes under it. + assert.equal(json(await call('create_step', { guide_id: id, title: 'Sign in', after_step_id: first.step_id })).number, '1.1'); + // After the last substep, it's another substep; at the top, a top-level step. + assert.equal(json(await call('create_step', { guide_id: id, title: 'Save', after_step_id: sub.step_id })).number, '1.3'); + assert.equal(json(await call('create_step', { guide_id: id, title: 'Go to the portal', after_step_id: null })).number, '1'); + + const broken = await call('create_step', { guide_id: id, title: 'x', after_step_id: null, parent_step_id: first.step_id }); + assert.match(broken.content[0].text, /break the step numbering/); + assert.match((await call('create_step', { guide_id: id, title: 'x', image_path: 'shot.png' })).content[0].text, /full path/); + const notPng = path.join(path.dirname(shot), 'notes.txt'); + fs.writeFileSync(notPng, 'hello'); + assert.match((await call('create_step', { guide_id: id, title: 'x', image_path: notPng })).content[0].text, /can't read .* as a PNG/); + + // One snapshot per step added, none for refused calls or the new guide. + const snaps = agentSnapshots(store, id); + assert.equal(snaps.length, 5); + assert.ok(snaps.every((name) => name.endsWith('-before_agent_adds_a_step.snapshot'))); + restoreSnapshot(store, id, snaps[0]); + assert.deepEqual(store.getGuide(id).stepsOrder, [], 'the first snapshot is the empty guide'); + assert.ok(changes.every((changed) => changed === id)); +}); + +test('agents delete steps after a snapshot that brings them back, screenshot and all', async (t) => { + const { call, guide, first, second, store, changes } = makeLibrary(t); + const id = guide.guideId; + const third = store.addStep(id, { title: 'Grandchild' }); + await call('update_step', { guide_id: id, step_id: second.stepId, parent_step_id: first.stepId }); + await call('update_step', { guide_id: id, step_id: third.stepId, parent_step_id: second.stepId }); + + const result = await call('delete_step', { guide_id: id, step_id: second.stepId }); + assert.match(result.content[0].text, /Deleted "Paste the answer"\. StepForge saved a snapshot/); + assert.deepEqual(store.getGuide(id).stepsOrder, [first.stepId, third.stepId]); + assert.equal(store.getStep(id, third.stepId).parentStepId, first.stepId, 'its substep moves up one level, not to the top'); + + await call('delete_step', { guide_id: id, step_id: first.stepId }); + assert.equal(fs.existsSync(store.stepDir(id, first.stepId)), false); + const [beforeSecond, beforeFirst] = agentSnapshots(store, id); + assert.match(beforeSecond, /before_agent_deletes_step_1_1\.snapshot$/); + assert.match(beforeFirst, /before_agent_deletes_step_1\.snapshot$/); + + restoreSnapshot(store, id, beforeFirst); + assert.deepEqual(store.getGuide(id).stepsOrder, [first.stepId, third.stepId]); + assert.equal(store.getStep(id, first.stepId).title, 'Screen capture'); + assert.ok(fs.existsSync(store.stepImagePath(id, first.stepId, 'original')), 'the screenshot is back'); + assert.ok(changes.length >= 2); +}); + +test('agents move steps with their substeps, with a snapshot before each move', async (t) => { + const { call, json, guide, first, second, store } = makeLibrary(t); + const id = guide.guideId; + const third = store.addStep(id, { title: 'Third' }); + await call('update_step', { guide_id: id, step_id: second.stepId, parent_step_id: first.stepId }); + + // Step 1 (with 1.1) goes after step 2. + const moved = json(await call('move_step', { guide_id: id, step_id: first.stepId, after_step_id: third.stepId })); + assert.deepEqual(moved.steps.map((s) => [s.step_id, s.number]), [[third.stepId, '1'], [first.stepId, '2'], [second.stepId, '2.1']]); + // A substep goes to the top, made top-level. + json(await call('move_step', { guide_id: id, step_id: second.stepId, after_step_id: null, parent_step_id: null })); + assert.deepEqual(store.getGuide(id).stepsOrder, [second.stepId, third.stepId, first.stepId]); + assert.equal(store.getStep(id, second.stepId).parentStepId, null); + + assert.match((await call('move_step', { guide_id: id, step_id: first.stepId })).content[0].text, /Give after_step_id/); + assert.match((await call('move_step', { guide_id: id, step_id: first.stepId, after_step_id: first.stepId })).content[0].text, /own substeps/); + assert.match((await call('move_step', { guide_id: id, step_id: third.stepId, after_step_id: null, parent_step_id: first.stepId })).content[0].text, /break the step numbering/); + + const snaps = agentSnapshots(store, id); + assert.equal(snaps.length, 2); + restoreSnapshot(store, id, snaps[0]); + assert.deepEqual(store.getGuide(id).stepsOrder, [first.stepId, second.stepId, third.stepId]); + assert.equal(store.getStep(id, second.stepId).parentStepId, first.stepId); +}); + +test('agents replace and remove screenshots, and a snapshot gets the old one back', async (t) => { + const { call, json, guide, first, second, store } = makeLibrary(t); + const id = guide.guideId; + const oldBytes = fs.readFileSync(store.stepImagePath(id, first.stepId, 'original')); + await call('add_annotations', { guide_id: id, step_id: first.stepId, annotations: [{ type: 'rect', x: 12, y: 2, width: 4, height: 4 }] }); + + const replaced = json(await call('set_screenshot', { guide_id: id, step_id: first.stepId, image_path: pngFile(t, 40, 30, [0, 0, 0, 255]) })); + assert.deepEqual(replaced.screenshot_size, { width: 40, height: 30 }); + assert.equal(replaced.title, 'Screen capture', 'the text stays'); + assert.deepEqual(replaced.annotations.map((a) => a.type), ['blur', 'rect'], 'drawn annotations stay'); + // A step without a screenshot gets one. + assert.equal(json(await call('set_screenshot', { guide_id: id, step_id: second.stepId, image_path: pngFile(t, 8, 8, [9, 9, 9, 255]) })).has_screenshot, true); + assert.equal(store.getStep(id, second.stepId).kind, 'image'); + + const removed = json(await call('remove_screenshot', { guide_id: id, step_id: first.stepId })); + assert.equal(removed.has_screenshot, false); + assert.equal(removed.annotations, undefined); + const bare = store.getStep(id, first.stepId); + assert.deepEqual([bare.image, bare.annotations, bare.kind], [null, [], 'content']); + assert.deepEqual(fs.readdirSync(store.stepDir(id, first.stepId)), ['step.json']); + assert.match((await call('remove_screenshot', { guide_id: id, step_id: first.stepId })).content[0].text, /no screenshot/); + assert.match((await call('set_screenshot', { guide_id: id, step_id: first.stepId, image_path: '/no/such/file.png' })).content[0].text, /There's no file/); + + const snaps = agentSnapshots(store, id); + assert.equal(snaps.length, 3); + assert.match(snaps[2], /before_agent_removes_the_screenshot_of_step_1\.snapshot$/); + restoreSnapshot(store, id, snaps[0]); + const back = store.getStep(id, first.stepId); + assert.ok(fs.readFileSync(store.stepImagePath(id, first.stepId, 'original')).equals(oldBytes)); + assert.deepEqual(back.annotations.map((a) => a.type), ['blur', 'rect']); +}); + +test('agents delete blocks and annotations, but never blurs, with a snapshot first', async (t) => { + const { call, json, guide, first, store } = makeLibrary(t); + const id = guide.guideId; + await call('update_step', { guide_id: id, step_id: first.stepId, blocks: [{ kind: 'code', code: 'ls' }] }); + const drawn = json(await call('add_annotations', { + guide_id: id, step_id: first.stepId, + annotations: [{ type: 'rect', x: 12, y: 2, width: 4, height: 4 }, { type: 'arrow', from: [0, 0], to: [10, 5] }], + })).added; + const codeId = store.getStep(id, first.stepId).codeBlocks[0].id; + + const afterBlocks = json(await call('delete_blocks', { guide_id: id, step_id: first.stepId, block_ids: ['tb1'] })); + assert.deepEqual(afterBlocks.blocks.map((b) => b.id), [codeId]); + assert.match((await call('delete_blocks', { guide_id: id, step_id: first.stepId, block_ids: ['tb1'] })).content[0].text, /has no block "tb1"/); + + const blurRefused = await call('delete_annotations', { guide_id: id, step_id: first.stepId, annotation_ids: [drawn[0].id, 'b1'] }); + assert.equal(blurRefused.isError, true); + assert.match(blurRefused.content[0].text, /"b1" is a blur/); + assert.equal(store.getStep(id, first.stepId).annotations.length, 3, 'nothing is deleted when one is a blur'); + const afterAnnotations = json(await call('delete_annotations', { guide_id: id, step_id: first.stepId, annotation_ids: [drawn[0].id] })); + assert.equal(afterAnnotations.annotations_on_step, 2); + assert.deepEqual(store.getStep(id, first.stepId).annotations.map((a) => a.type), ['blur', 'arrow']); + + // Refused calls take no snapshot. + const snaps = agentSnapshots(store, id); + assert.equal(snaps.length, 2); + restoreSnapshot(store, id, snaps[0]); + const back = store.getStep(id, first.stepId); + assert.deepEqual(back.textBlocks.map((b) => b.id), ['tb1']); + assert.equal(back.annotations.length, 3); +}); + +test('agents move guides to the Trash with a snapshot, and the app hears about it', async (t) => { + const { call, guide, store, changes } = makeLibrary(t); + const result = await call('delete_guide', { guide_id: guide.guideId }); + assert.match(result.content[0].text, /Moved "Lab 5" to the Trash/); + assert.equal(store.guideExists(guide.guideId), false); + assert.deepEqual(changes, [guide.guideId]); + const [trashed] = store.listTrash(); + assert.equal(store.restoreFromTrash(trashed), guide.guideId); + assert.equal(agentSnapshots(store, guide.guideId).length, 1, 'the snapshot comes back with the guide'); + assert.equal(store.getGuide(guide.guideId).stepsOrder.length, 2); + assert.match((await call('delete_guide', { guide_id: 'nope' })).content[0].text, /No guide with id/); +}); + +test('automatic pruning keeps agent snapshots, which keep their own newest 50', (t) => { + const dir = makeTmpDir('agent-snapshots'); + t.after(() => rmrf(dir)); + const store = new GuideStore(dir); + const { guideId } = store.createGuide({ title: 'G' }); + store.addStep(guideId, { title: 'S' }, TINY_PNG, { width: 1, height: 1 }); + createAgentSnapshot(store, guideId, 'deletes step 1'); + for (let i = 0; i < 3; i++) createSnapshot(store, guideId, { label: 'auto' }); + pruneSnapshots(store, guideId, 1); + assert.equal(listSnapshots(store, guideId).length, 2, 'one automatic snapshot and the agent one'); + for (let i = 0; i < 55; i++) createAgentSnapshot(store, guideId, `deletes step ${i}`); + const agents = agentSnapshots(store, guideId); + assert.equal(agents.length, 50); + assert.match(agents[0], /deletes_step_5\.snapshot$/, 'the oldest go first'); +}); diff --git a/tests/unit/store.test.js b/tests/unit/store.test.js index f98f8042..b39a7d4e 100644 --- a/tests/unit/store.test.js +++ b/tests/unit/store.test.js @@ -67,6 +67,12 @@ test('step reorder, delete with substep reparenting, and order integrity', (t) = const after = store.getGuide(guide.guideId); assert.deepEqual(after.stepsOrder, [c.stepId, a.stepId]); assert.equal(store.getStep(guide.guideId, c.stepId).parentStepId, null); + + // Deleting a substep moves its own substeps up to its parent, not to the top. + const d = store.addStep(guide.guideId, { kind: 'empty', title: 'D', parentStepId: a.stepId }); + const e = store.addStep(guide.guideId, { kind: 'empty', title: 'E', parentStepId: d.stepId }); + store.deleteStep(guide.guideId, d.stepId); + assert.equal(store.getStep(guide.guideId, e.stepId).parentStepId, a.stepId); }); test('restoreStep recreates a deleted step with its original id, data, and images', (t) => { From 954c05de7972ad61aeabbbe72e541ea4c1b70d9e Mon Sep 17 00:00:00 2001 From: Twest2 Date: Thu, 29 Oct 2026 10:53:13 -0500 Subject: [PATCH 2/7] Protect agent screenshot privacy and preserve concurrent guide structure --- app/main.js | 10 ++-- app/renderer/editor.js | 13 ++++- core/agent-tools.js | 17 +++++-- core/editor-save.js | 26 ++++++++++ core/store.js | 1 + docs/AI_AGENTS.md | 12 +++-- tests/unit/agent-editor-save.test.js | 74 ++++++++++++++++++++++++++++ tests/unit/agent-mcp.test.js | 41 +++++++++++++++ 8 files changed, 178 insertions(+), 16 deletions(-) create mode 100644 core/editor-save.js create mode 100644 tests/unit/agent-editor-save.test.js diff --git a/app/main.js b/app/main.js index edf7db5c..9b2b963c 100644 --- a/app/main.js +++ b/app/main.js @@ -11,6 +11,7 @@ const { const { GuideStore } = require('../core/store'); const { libraryLocationFor } = require('./data-dir'); const { Settings } = require('../core/settings'); +const { saveGuideFromEditor, saveStepFromEditor } = require('../core/editor-save'); const { GoogleDrive } = require('./google-drive'); const { OneDrive } = require('./onedrive'); const { Dropbox } = require('./dropbox'); @@ -868,12 +869,8 @@ function setupIpc() { }), stepNumberMap(orderedSteps(guideId))), { validate: (a) => c.id(a.guideId) }); // An AI agent may have moved the guide to the Trash meanwhile; a late save // from the editor must not recreate it, which would also block its restore. - const requireGuide = (guideId) => { - if (!store.guideExists(guideId)) throw new Error('This guide was moved to the Trash.'); - }; h('guide:save', ({ guide }) => { - requireGuide(guide.guideId); - const saved = store.saveGuide(guide); + const saved = saveGuideFromEditor(store, guide); reindex(guide.guideId); return saved; }, { validate: (a) => security.isPlainArgs(a.guide) && a.guide && c.id(a.guide.guideId) }); @@ -887,8 +884,7 @@ function setupIpc() { validate: (a) => c.id(a.guideId) && c.optionalBase64(a.imageBase64) && c.optionalNumber(a.position, 0, 100000), }); h('step:save', ({ guideId, step }) => { - requireGuide(guideId); - const saved = store.saveStep(guideId, step); + const saved = saveStepFromEditor(store, guideId, step); reindex(guideId); return saved; }, { validate: (a) => c.id(a.guideId) && security.isPlainArgs(a.step) && a.step && c.id(a.step.stepId) }); diff --git a/app/renderer/editor.js b/app/renderer/editor.js index 04b1574e..da8d510b 100644 --- a/app/renderer/editor.js +++ b/app/renderer/editor.js @@ -527,7 +527,18 @@ class GuideEditor { * Pending edits are saved first, so the user's own unsaved work always wins. */ async reloadAfterOutsideChange() { - if (this.pendingSave) await this.flushStep(); + if (this.pendingSave) { + const { guide } = await api.guide.get({ guideId: this.guideId }); + if (!guide.stepsOrder.includes(this.selectedStepId)) { + this.saveStepDebounced.cancel(); + this.pendingSave = false; + this.saveError = null; + this.onToast('An AI agent deleted the step you were editing. Its saved version is in Backups & snapshots.'); + } else { + await this.flushStep(); + if (this.pendingSave) return; // Keep unsaved edits if the write failed. + } + } if (this.pendingGuideSave) await this.flushGuide(); await this.reload(this.selectedStepId); } diff --git a/core/agent-tools.js b/core/agent-tools.js index fea7b908..bf690904 100644 --- a/core/agent-tools.js +++ b/core/agent-tools.js @@ -267,11 +267,17 @@ function createAgentTools({ store, agentSettings, onChange = () => {}, onBrandin if (typeof filePath !== 'string' || !path.isAbsolute(filePath)) { throw new AgentError('image_path must be the full path to a PNG file on this computer.'); } - let stat; - try { stat = fs.statSync(filePath); } catch { throw new AgentError(`There's no file at ${filePath}.`); } + let stat, resolved; + try { resolved = fs.realpathSync(filePath); stat = fs.statSync(resolved); } catch { throw new AgentError(`There's no file at ${filePath}.`); } + // Raw library images include originals hidden by blurs, old snapshots and + // Trash. Importing one as a new step would expose those pixels to get_step. + const relative = path.relative(fs.realpathSync(store.root), resolved); + if (!relative || (relative !== '..' && !relative.startsWith(`..${path.sep}`) && !path.isAbsolute(relative))) { + throw new AgentError('Use a PNG file outside the StepForge library. Library screenshots can only be read through get_step, with blurs filled in.'); + } if (!stat.isFile()) throw new AgentError(`${filePath} isn't a file.`); if (stat.size > MAX_SCREENSHOT_BYTES) throw new AgentError(`${filePath} is larger than ${MAX_SCREENSHOT_BYTES / 1024 / 1024} MB.`); - const png = fs.readFileSync(filePath); + const png = fs.readFileSync(resolved); let image; try { image = decodePng(png); @@ -722,8 +728,9 @@ function createAgentTools({ store, agentSettings, onChange = () => {}, onBrandin const { png, size } = readScreenshot(imagePath); const note = snapshotBefore(guideId, `replaces the screenshot of step ${numberOf(guide, stepId)}`); const step = store.getStep(guideId, stepId); - // Private-detail blurs matched the old screenshot; the new one is checked again. - forgetScreenshotChecks(step); + // Agents cannot remove any blur, including one added by a private-detail + // check. Even a replacement can contain the same private pixels. + if (step.redaction) step.redaction = { ...step.redaction, checked: '' }; const saved = store.replaceImages(guideId, stepId, { original: png }, size, step); onChange(guideId); return { content: [text(stepForAgent(saved, numberOf(guide, stepId))), text(note)] }; diff --git a/core/editor-save.js b/core/editor-save.js new file mode 100644 index 00000000..2701c96a --- /dev/null +++ b/core/editor-save.js @@ -0,0 +1,26 @@ +'use strict'; + +/** + * Renderer saves edit existing content. Step creation, deletion and ordering + * have their own operations, and may have changed in the agent process since + * the editor loaded its copy. + */ +function saveGuideFromEditor(store, guide) { + if (!store.guideExists(guide.guideId)) throw new Error('This guide was moved to the Trash.'); + const current = store.getGuide(guide.guideId); + return store.saveGuide({ ...guide, revision: current.revision, stepsOrder: current.stepsOrder }); +} + +function saveStepFromEditor(store, guideId, step) { + if (!store.guideExists(guideId)) throw new Error('This guide was moved to the Trash.'); + const guide = store.getGuide(guideId); + if (!guide.stepsOrder.includes(step.stepId)) throw new Error('This step was deleted.'); + const current = store.getStep(guideId, step.stepId); // Refuse vanished files. + // A dirty text edit must not undo an agent's move or the promotion of a + // deleted parent's children. A fresh edit may still choose a new parent. + const fields = step.revision !== current.revision + ? { ...step, parentStepId: current.parentStepId } : step; + return store.saveStep(guideId, { ...fields, revision: current.revision }); +} + +module.exports = { saveGuideFromEditor, saveStepFromEditor }; diff --git a/core/store.js b/core/store.js index 12ec6f35..6066bbc9 100644 --- a/core/store.js +++ b/core/store.js @@ -405,6 +405,7 @@ class GuideStore { for (const [, step] of steps) { if (step.parentStepId === stepId) { step.parentStepId = parentId; + step.revision = (Number.isInteger(step.revision) ? step.revision : 0) + 1; writeJsonSync(path.join(this.stepDir(guideId, step.stepId), 'step.json'), step); } } diff --git a/docs/AI_AGENTS.md b/docs/AI_AGENTS.md index 47d6fdb3..73038608 100644 --- a/docs/AI_AGENTS.md +++ b/docs/AI_AGENTS.md @@ -84,8 +84,11 @@ Record a guide in StepForge as usual, then ask your agent, for example: The agent finds the guide, looks at each step's screenshot, and rewrites the text. It uses your placeholders where their values would appear, and can add -new ones for values that repeat, such as a course code or product name. If the guide is open in StepForge, it updates on screen within a couple -of seconds. If you're typing in the same step at that moment, your edit wins. +new ones for values that repeat, such as a course code or product name. If +the guide is open in StepForge, it updates on screen within a couple of +seconds. If you're typing in the same step at that moment, your edit wins. If +the agent deleted that step, StepForge drops its pending save and shows the +remaining steps instead of recreating it. Before you share the guide, read what the agent wrote. To undo a big rewrite, use **More → Backups & snapshots**. Annotations an agent drew can be moved or @@ -100,7 +103,10 @@ its AI provider**, such as Anthropic for Claude or OpenAI for Codex. Their privacy terms apply to that data. - Screenshots are shared with your blurs filled in, so the agent never sees - what you hid. Agents can't delete blurs. + what you hid. Agents can't delete blurs. Screenshot imports must come from + outside the library; raw originals, Trash and snapshot files cannot be + imported to bypass the blurs. Replacing a screenshot keeps its blurs, + including those added by Find private details. - An agent reads a file outside your library only when it uses one as a screenshot or adds one to a brand profile, and only the file it names. - Agents never get the text you typed while recording, field values, or the diff --git a/tests/unit/agent-editor-save.test.js b/tests/unit/agent-editor-save.test.js new file mode 100644 index 00000000..3fabbd42 --- /dev/null +++ b/tests/unit/agent-editor-save.test.js @@ -0,0 +1,74 @@ +'use strict'; +const test = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); +const vm = require('node:vm'); +const { GuideStore } = require('../../core/store'); +const { saveGuideFromEditor, saveStepFromEditor } = require('../../core/editor-save'); +const { makeTmpDir, rmrf } = require('./helpers'); + +function library(t) { + const root = makeTmpDir('agent-editor'); + t.after(() => rmrf(root)); + const store = new GuideStore(root); + const guide = store.createGuide({ title: 'Guide' }); + const a = store.addStep(guide.guideId, { title: 'A' }); + const b = store.addStep(guide.guideId, { title: 'B' }); + return { store, guideId: guide.guideId, a, b }; +} + +test('saving stale guide details preserves steps added, moved and deleted outside the editor', (t) => { + const { store, guideId, a, b } = library(t); + const stale = store.getGuide(guideId); + const c = store.addStep(guideId, { title: 'C' }); + store.reorderSteps(guideId, [c.stepId, b.stepId, a.stepId]); + store.deleteStep(guideId, a.stepId); + stale.title = 'User typed title'; + const revision = store.getGuide(guideId).revision; + const saved = saveGuideFromEditor(store, stale); + assert.equal(saved.revision, revision + 1); + assert.equal(saved.title, 'User typed title'); + assert.deepEqual(saved.stepsOrder, [c.stepId, b.stepId]); +}); + +test('a late editor save cannot recreate a deleted step or a trashed guide', (t) => { + const { store, guideId, a } = library(t); + const guide = store.getGuide(guideId); + store.deleteStep(guideId, a.stepId); + assert.throws(() => saveStepFromEditor(store, guideId, a), /step was deleted/); + assert.equal(fs.existsSync(store.stepDir(guideId, a.stepId)), false); + store.deleteGuide(guideId); + assert.throws(() => saveStepFromEditor(store, guideId, a), /Trash/); + assert.throws(() => saveGuideFromEditor(store, guide), /Trash/); + assert.equal(store.guideExists(guideId), false); +}); + +test('agent reload cancels an unsaved deleted step rather than saving it onto another step', async () => { + const context = { window: { stepforge: { guide: { get: async () => ({ guide: { stepsOrder: ['remaining'] } }) } } } }; + vm.runInNewContext(fs.readFileSync(path.join(__dirname, '../../app/renderer/editor.js'), 'utf8'), context); + const editor = Object.create(context.window.GuideEditor.prototype); + let cancelled = false, reloaded = false; + Object.assign(editor, { guideId: 'g', selectedStepId: 'deleted', pendingSave: true, + saveStepDebounced: { cancel() { cancelled = true; } }, + flushStep() { assert.fail('must not save a deleted step'); }, + onToast() {}, reload: async () => { reloaded = true; } }); + await editor.reloadAfterOutsideChange(); + assert.equal(cancelled, true); + assert.equal(editor.pendingSave, false); + assert.equal(reloaded, true); +}); + + +test('a dirty step save preserves the parent changed by an agent move or deletion', (t) => { + const { store, guideId, a, b } = library(t); + store.reorderSteps(guideId, [a.stepId, b.stepId], { [b.stepId]: a.stepId }); + const dirty = store.getStep(guideId, b.stepId); + dirty.title = 'Unsaved user text'; + store.deleteStep(guideId, a.stepId); + const revision = store.getStep(guideId, b.stepId).revision; + const saved = saveStepFromEditor(store, guideId, dirty); + assert.equal(saved.revision, revision + 1); + assert.equal(saved.title, 'Unsaved user text'); + assert.equal(saved.parentStepId, null, 'do not restore a deleted parent'); +}); diff --git a/tests/unit/agent-mcp.test.js b/tests/unit/agent-mcp.test.js index 480403db..07bc3c3c 100644 --- a/tests/unit/agent-mcp.test.js +++ b/tests/unit/agent-mcp.test.js @@ -527,3 +527,44 @@ test('automatic pruning keeps agent snapshots, which keep their own newest 50', assert.equal(agents.length, 50); assert.match(agents[0], /deletes_step_5\.snapshot$/, 'the oldest go first'); }); + +test('screenshot imports cannot expose raw library images, including through a symbolic link', async (t) => { + const { call, guide, first, store } = makeLibrary(t); + const original = store.stepImagePath(guide.guideId, first.stepId, 'original'); + const outside = makeTmpDir('agent-protected-image'); + t.after(() => rmrf(outside)); + const inputs = [original]; + const link = path.join(outside, 'linked.png'); + try { fs.symlinkSync(original, link); inputs.push(link); } catch (error) { + if (!['EPERM', 'EACCES'].includes(error.code)) throw error; + } + for (const image_path of inputs) { + for (const tool of ['create_step', 'set_screenshot']) { + const result = await call(tool, { guide_id: guide.guideId, step_id: first.stepId, image_path }); + assert.equal(result.isError, true); + assert.match(result.content[0].text, /outside the StepForge library/); + } + } + assert.equal(agentSnapshots(store, guide.guideId).length, 0, 'refused imports do not take snapshots'); + assert.equal(store.getGuide(guide.guideId).stepsOrder.length, 2); +}); + +test('replacing a screenshot preserves automatic private-detail blurs too', async (t) => { + const { call, guide, first, store } = makeLibrary(t); + const step = store.getStep(guide.guideId, first.stepId); + step.annotations[0].redact = { kind: 'secret', key: 'secret-key' }; + step.redaction = { checked: 'old-check' }; + store.saveStep(guide.guideId, step); + const replacement = pngFile(t, 20, 10, [0, 0, 255, 255]); + fs.copyFileSync(store.stepImagePath(guide.guideId, first.stepId, 'original'), replacement); + await call('set_screenshot', { guide_id: guide.guideId, step_id: first.stepId, image_path: replacement }); + const saved = store.getStep(guide.guideId, first.stepId); + assert.equal(saved.annotations.length, 1); + assert.equal(saved.annotations[0].type, 'blur'); + assert.equal(saved.redaction?.checked || '', ''); + assert.equal(saved.annotations[0].redact.key, 'secret-key'); + const read = await call('get_step', { guide_id: guide.guideId, step_id: first.stepId }); + const png = decodePng(Buffer.from(read.content.find((part) => part.type === 'image').data, 'base64')); + const p = (5 * png.width + 5) * 4; + assert.notDeepEqual([...png.data.subarray(p, p + 3)], [255, 0, 0], 'private pixels remain hidden'); +}); From 6099b83e5f7d45ab1602352a64c718912bacfd0e Mon Sep 17 00:00:00 2001 From: Twest2 Date: Thu, 29 Oct 2026 10:54:50 -0500 Subject: [PATCH 3/7] Describe destructive agent permissions accurately in Settings --- app/renderer/dialogs.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/renderer/dialogs.js b/app/renderer/dialogs.js index bda5ed84..073c6e99 100644 --- a/app/renderer/dialogs.js +++ b/app/renderer/dialogs.js @@ -889,7 +889,7 @@ function showSettingsDialog({ ollamaModel), el('div.settings-test', {}, aiStatus, testAiBtn, ollamaModels)), settingsCard('AI agents (Claude, Codex)', - el('div.setting-desc', {}, 'AI apps such as Claude and Codex can connect to StepForge and write your guides for you. They can rewrite text, make substeps, add placeholders, and draw on screenshots, but they can’t reorder steps or delete anything.'), + el('div.setting-desc', {}, 'AI apps such as Claude and Codex can connect to StepForge and edit your guides. They can create and delete guides and steps, reorder steps, replace or remove screenshots, edit text, add placeholders, and add or delete annotations except blurs. StepForge saves snapshots before structural or destructive changes; restore them from More → Backups & snapshots. Deleted guides go to the Trash.'), settingRow('Let AI agents edit your guides', 'Turn off to cut off every connected agent at once.', makeSwitch(agentsEnabled, 'Let AI agents edit your guides')), settingRow('Agents can see screenshots', 'Blurred areas are filled in first. Turn off to share step text only.', makeSwitch(agentsScreenshots, 'Agents can see screenshots')), settingRow('Let AI agents edit brand profiles', 'Agents can make and change brand profiles: logo, colors, fonts and CSS, and choose the one exports use. They can’t delete a profile.', makeSwitch(agentsBranding, 'Let AI agents edit brand profiles')), From f99d4715310265a7e2b3176838fe9f2f7b16fbdc Mon Sep 17 00:00:00 2001 From: Twest2 Date: Thu, 29 Oct 2026 10:55:26 -0500 Subject: [PATCH 4/7] Allow agent editor regression tests to use renderer translations --- tests/unit/agent-editor-save.test.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/unit/agent-editor-save.test.js b/tests/unit/agent-editor-save.test.js index 3fabbd42..86b77eb3 100644 --- a/tests/unit/agent-editor-save.test.js +++ b/tests/unit/agent-editor-save.test.js @@ -6,7 +6,7 @@ const path = require('node:path'); const vm = require('node:vm'); const { GuideStore } = require('../../core/store'); const { saveGuideFromEditor, saveStepFromEditor } = require('../../core/editor-save'); -const { makeTmpDir, rmrf } = require('./helpers'); +const { makeTmpDir, rmrf, rendererI18n } = require('./helpers'); function library(t) { const root = makeTmpDir('agent-editor'); @@ -45,7 +45,7 @@ test('a late editor save cannot recreate a deleted step or a trashed guide', (t) }); test('agent reload cancels an unsaved deleted step rather than saving it onto another step', async () => { - const context = { window: { stepforge: { guide: { get: async () => ({ guide: { stepsOrder: ['remaining'] } }) } } } }; + const context = { ...(rendererI18n ? rendererI18n() : {}), window: { stepforge: { guide: { get: async () => ({ guide: { stepsOrder: ['remaining'] } }) } } } }; vm.runInNewContext(fs.readFileSync(path.join(__dirname, '../../app/renderer/editor.js'), 'utf8'), context); const editor = Object.create(context.window.GuideEditor.prototype); let cancelled = false, reloaded = false; From 02a6b3f4f3a2fdcd6c0087704cac584288c85a0f Mon Sep 17 00:00:00 2001 From: Twest2 Date: Thu, 29 Oct 2026 10:57:59 -0500 Subject: [PATCH 5/7] Preserve agent screenshot changes during stale editor saves --- core/editor-save.js | 15 ++++++-- tests/unit/agent-editor-save.test.js | 54 +++++++++++++++++++++++++++- 2 files changed, 66 insertions(+), 3 deletions(-) diff --git a/core/editor-save.js b/core/editor-save.js index 2701c96a..6a2f8b49 100644 --- a/core/editor-save.js +++ b/core/editor-save.js @@ -1,5 +1,7 @@ 'use strict'; +const { isDeepStrictEqual } = require('node:util'); + /** * Renderer saves edit existing content. Step creation, deletion and ordering * have their own operations, and may have changed in the agent process since @@ -18,8 +20,17 @@ function saveStepFromEditor(store, guideId, step) { const current = store.getStep(guideId, step.stepId); // Refuse vanished files. // A dirty text edit must not undo an agent's move or the promotion of a // deleted parent's children. A fresh edit may still choose a new parent. - const fields = step.revision !== current.revision - ? { ...step, parentStepId: current.parentStepId } : step; + const stale = step.revision !== current.revision; + const imageChanged = !isDeepStrictEqual(step.image, current.image); + // Screenshot operations have dedicated routes. A normal save cannot add + // back a removed image, overwrite its dimensions, or restore its old kind. + const fields = { ...step, image: current.image, kind: current.kind }; + if (stale) { + fields.parentStepId = current.parentStepId; + fields.redaction = current.redaction; + } + // Coordinates and privacy state belong to the current screenshot. + if (imageChanged || !current.image) fields.annotations = current.annotations; return store.saveStep(guideId, { ...fields, revision: current.revision }); } diff --git a/tests/unit/agent-editor-save.test.js b/tests/unit/agent-editor-save.test.js index 86b77eb3..ab446860 100644 --- a/tests/unit/agent-editor-save.test.js +++ b/tests/unit/agent-editor-save.test.js @@ -6,7 +6,11 @@ const path = require('node:path'); const vm = require('node:vm'); const { GuideStore } = require('../../core/store'); const { saveGuideFromEditor, saveStepFromEditor } = require('../../core/editor-save'); -const { makeTmpDir, rmrf, rendererI18n } = require('./helpers'); +const { makeTmpDir, rmrf, rendererI18n, TINY_PNG } = require('./helpers'); +const { Settings } = require('../../core/settings'); +const { createAgentTools } = require('../../core/agent-tools'); +const { encodePng } = require('../../core/png'); +const { createImage } = require('../../core/raster'); function library(t) { const root = makeTmpDir('agent-editor'); @@ -72,3 +76,51 @@ test('a dirty step save preserves the parent changed by an agent move or deletio assert.equal(saved.title, 'Unsaved user text'); assert.equal(saved.parentStepId, null, 'do not restore a deleted parent'); }); + + +test('stale text saves preserve screenshots removed, replaced or added by an agent', async (t) => { + const { store, guideId, a } = library(t); + const dir = makeTmpDir('agent-editor-image'); + t.after(() => rmrf(dir)); + const png = path.join(dir, 'replacement.png'); + fs.writeFileSync(png, encodePng(createImage(40, 30, [0, 128, 0, 255]))); + const settings = new Settings(store.settingsDir); + settings.set('ai.agents', { enabled: true }); + const tools = createAgentTools({ store, agentSettings: () => settings.get('ai.agents') }); + const call = (name, args = {}) => tools.find((tool) => tool.name === name).handler({ guide_id: guideId, step_id: a.stepId, ...args }); + store.replaceImages(guideId, a.stepId, { original: TINY_PNG }, { width: 1, height: 1 }); + + const beforeRemoval = store.getStep(guideId, a.stepId); + beforeRemoval.title = 'User kept typing'; + beforeRemoval.redaction = { checked: 'check-of-the-old-image' }; + await call('remove_screenshot'); + const removed = saveStepFromEditor(store, guideId, beforeRemoval); + assert.equal(removed.title, 'User kept typing'); + assert.equal(removed.image, null, 'a text save cannot recreate the removed screenshot'); + assert.equal(removed.kind, 'content'); + assert.deepEqual(removed.annotations, []); + assert.equal(removed.redaction, null); + assert.equal(store.stepImagePath(guideId, a.stepId), null); + + const beforeAddition = store.getStep(guideId, a.stepId); + beforeAddition.title = 'Typed while an image was added'; + await call('set_screenshot', { image_path: png }); + const added = saveStepFromEditor(store, guideId, beforeAddition); + assert.equal(added.title, beforeAddition.title); + assert.equal(added.kind, 'image'); + assert.deepEqual(added.image.size, { width: 40, height: 30 }); + + const beforeReplacement = store.getStep(guideId, a.stepId); + beforeReplacement.title = 'Typed during replacement'; + beforeReplacement.annotations = [{ id: 'old', type: 'rect', x: 0, y: 0, w: 1, h: 1 }]; + beforeReplacement.redaction = { checked: 'stale-check' }; + fs.writeFileSync(png, TINY_PNG); + await call('set_screenshot', { image_path: png }); + const replacement = store.getStep(guideId, a.stepId); + const saved = saveStepFromEditor(store, guideId, beforeReplacement); + assert.equal(saved.title, beforeReplacement.title); + assert.deepEqual(saved.image, replacement.image, 'keep new dimensions and paths'); + assert.deepEqual(saved.annotations, replacement.annotations, 'do not restore annotations for the old geometry'); + assert.deepEqual(saved.redaction, replacement.redaction, 'do not restore a check for the old pixels'); + assert.deepEqual(saved.image.size, { width: 1, height: 1 }); +}); From aa0f5a760541277c5b2153e42675b4166670ab63 Mon Sep 17 00:00:00 2001 From: Twest2 Date: Thu, 29 Oct 2026 11:40:58 -0500 Subject: [PATCH 6/7] Merge concurrent agent edits and reject late writes to trashed guides --- app/main.js | 7 +- app/renderer/dialogs.js | 2 +- app/renderer/editor.js | 12 +++- core/agent-tools.js | 4 +- core/editor-save.js | 12 +++- core/store.js | 14 +++- docs/AI_AGENTS.md | 9 ++- tests/unit/agent-editor-save.test.js | 95 ++++++++++++++++++++++++++-- tests/unit/agent-mcp.test.js | 2 +- 9 files changed, 137 insertions(+), 20 deletions(-) diff --git a/app/main.js b/app/main.js index 9b2b963c..a4db4bf7 100644 --- a/app/main.js +++ b/app/main.js @@ -883,11 +883,12 @@ function setupIpc() { maxChars: IMAGE_BUDGET, validate: (a) => c.id(a.guideId) && c.optionalBase64(a.imageBase64) && c.optionalNumber(a.position, 0, 100000), }); - h('step:save', ({ guideId, step }) => { - const saved = saveStepFromEditor(store, guideId, step); + h('step:save', ({ guideId, step, baseStep }) => { + const saved = saveStepFromEditor(store, guideId, step, baseStep); reindex(guideId); return saved; - }, { validate: (a) => c.id(a.guideId) && security.isPlainArgs(a.step) && a.step && c.id(a.step.stepId) }); + }, { validate: (a) => c.id(a.guideId) && security.isPlainArgs(a.step) && a.step && c.id(a.step.stepId) + && (a.baseStep == null || (security.isPlainArgs(a.baseStep) && a.baseStep.stepId === a.step.stepId)) }); h('step:delete', ({ guideId, stepId }) => { store.deleteStep(guideId, stepId); reindex(guideId); diff --git a/app/renderer/dialogs.js b/app/renderer/dialogs.js index 073c6e99..b620ff2d 100644 --- a/app/renderer/dialogs.js +++ b/app/renderer/dialogs.js @@ -889,7 +889,7 @@ function showSettingsDialog({ ollamaModel), el('div.settings-test', {}, aiStatus, testAiBtn, ollamaModels)), settingsCard('AI agents (Claude, Codex)', - el('div.setting-desc', {}, 'AI apps such as Claude and Codex can connect to StepForge and edit your guides. They can create and delete guides and steps, reorder steps, replace or remove screenshots, edit text, add placeholders, and add or delete annotations except blurs. StepForge saves snapshots before structural or destructive changes; restore them from More → Backups & snapshots. Deleted guides go to the Trash.'), + el('div.setting-desc', {}, 'AI apps such as Claude and Codex can connect to StepForge and edit your guides. They can create and delete guides and steps, reorder steps, replace or remove screenshots, edit text, add placeholders, and add or delete annotations except blurs. StepForge saves snapshots before step creation, moves, deletions and screenshot changes; restore them from More → Backups & snapshots. Deleted guides go to the Trash.'), settingRow('Let AI agents edit your guides', 'Turn off to cut off every connected agent at once.', makeSwitch(agentsEnabled, 'Let AI agents edit your guides')), settingRow('Agents can see screenshots', 'Blurred areas are filled in first. Turn off to share step text only.', makeSwitch(agentsScreenshots, 'Agents can see screenshots')), settingRow('Let AI agents edit brand profiles', 'Agents can make and change brand profiles: logo, colors, fonts and CSS, and choose the one exports use. They can’t delete a profile.', makeSwitch(agentsBranding, 'Let AI agents edit brand profiles')), diff --git a/app/renderer/editor.js b/app/renderer/editor.js index da8d510b..6e369618 100644 --- a/app/renderer/editor.js +++ b/app/renderer/editor.js @@ -550,6 +550,7 @@ class GuideEditor { this.guide = guide; this.steps = steps; this.stepMap = new Map(steps.map((step) => [step.stepId, step])); + this.savedStepBaselines = new Map(steps.map((step) => [step.stepId, clone(step)])); this.selectedSteps = new Set([...this.selectedSteps].filter((id) => this.stepMap.has(id))); if (!this.stepMap.has(this.stepSelectionAnchor)) this.stepSelectionAnchor = null; if (!this.shellMounted) this.mountShell(); @@ -1750,6 +1751,9 @@ class GuideEditor { async restoreHistoryRecord(record) { if (!record || !record.step) return; const step = clone(record.step); + // Undo is an intentional new edit of the current copy, not an autosave + // based on the old revision captured in the history record. + step.revision = this.stepMap.get(step.stepId)?.revision ?? step.revision; this.selectedStepId = step.stepId; this.beforeCanvasSnapshot = null; this.saveStepDebounced.cancel(); @@ -1878,7 +1882,7 @@ class GuideEditor { // surface it, and retry on the next edit or explicit save. let saved; try { - saved = await api.step.save({ guideId: this.guideId, step }); + saved = await api.step.save({ guideId: this.guideId, step, baseStep: this.savedStepBaselines?.get(step.stepId) }); } catch (err) { this.pendingSave = true; this.saveError = (err && err.message) || 'Save failed'; @@ -1888,7 +1892,9 @@ class GuideEditor { } this.pendingSave = false; this.saveError = null; - const committed = this.commitSavedStep(saved); + const mergedOutsideChange = saved.revision > (step.revision || 0) + 1; + const committed = this.commitSavedStep(saved, { replaceBlocks: mergedOutsideChange }); + if (mergedOutsideChange) this.renderAll(); if (this.selectedStepId === committed.stepId) { this.renderStepList(); this.syncStepFields(); @@ -1905,6 +1911,8 @@ class GuideEditor { commitSavedStep(saved, { replaceBlocks = false } = {}) { if (!saved || !saved.stepId) return saved; + this.savedStepBaselines ||= new Map(); + this.savedStepBaselines.set(saved.stepId, clone(saved)); const existing = this.stepMap.get(saved.stepId); if (!existing || existing === saved) { this.stepMap.set(saved.stepId, saved); diff --git a/core/agent-tools.js b/core/agent-tools.js index bf690904..1ebc3ffd 100644 --- a/core/agent-tools.js +++ b/core/agent-tools.js @@ -29,7 +29,7 @@ const { * Agents read guides, rewrite their text, make substeps, add placeholders and * draw annotations. They can also create and delete guides and steps, move * steps, replace or remove screenshots, and delete blocks and annotations; - * before each of those StepForge snapshots the guide, so the user can undo it. + * the deletion, step creation/move and screenshot tools snapshot the guide first. * They can't change or delete placeholders, or delete blurs. */ @@ -48,7 +48,7 @@ const INSTRUCTIONS = [ 'StepForge holds step-by-step guides a person recorded: each step is a screenshot of one click, with a marker where they clicked.', 'You can read guides and rewrite their text: the guide title and description, and each step\'s title, description and blocks (notes, warnings, tips, code, tables).', 'You can also make a step a substep of an earlier step (update_step parent_step_id), add placeholders (create_placeholder), and draw annotations such as rectangles, arrows and callouts on a screenshot (add_annotations).', - 'You can also create and delete guides and steps (create_guide, delete_guide, create_step, delete_step), move steps (move_step), replace or remove a step\'s screenshot with a PNG file on this computer (set_screenshot, remove_screenshot), and delete blocks and annotations (delete_blocks, delete_annotations). StepForge snapshots the guide before each of these, so the user can undo it.', + 'You can also create and delete guides and steps (create_guide, delete_guide, create_step, delete_step), move steps (move_step), replace or remove a step\'s screenshot with a PNG file on this computer (set_screenshot, remove_screenshot), and delete blocks and annotations (delete_blocks, delete_annotations). StepForge snapshots existing guides before deletions, create_step, move_step and screenshot changes. Creating a guide, rewriting text, changing nesting with update_step, adding placeholders and drawing annotations do not create agent snapshots.', 'Steps normally come from a real capture, so prefer rewriting what is there over deleting it. You cannot change or delete placeholders, or delete blurs.', 'To write a guide: call get_guide, then get_step for each step to see its screenshot, then update_step. Finish with update_guide for the guide\'s title and description.', 'Step titles are short imperative actions ("Click Save", "Open Settings"). Descriptions are Markdown, 1–2 sentences per step.', diff --git a/core/editor-save.js b/core/editor-save.js index 6a2f8b49..ea714516 100644 --- a/core/editor-save.js +++ b/core/editor-save.js @@ -1,6 +1,7 @@ 'use strict'; const { isDeepStrictEqual } = require('node:util'); +const { mergeValue } = require('./guide-merge'); /** * Renderer saves edit existing content. Step creation, deletion and ordering @@ -13,7 +14,7 @@ function saveGuideFromEditor(store, guide) { return store.saveGuide({ ...guide, revision: current.revision, stepsOrder: current.stepsOrder }); } -function saveStepFromEditor(store, guideId, step) { +function saveStepFromEditor(store, guideId, step, baseStep = null) { if (!store.guideExists(guideId)) throw new Error('This guide was moved to the Trash.'); const guide = store.getGuide(guideId); if (!guide.stepsOrder.includes(step.stepId)) throw new Error('This step was deleted.'); @@ -22,9 +23,16 @@ function saveStepFromEditor(store, guideId, step) { // deleted parent's children. A fresh edit may still choose a new parent. const stale = step.revision !== current.revision; const imageChanged = !isDeepStrictEqual(step.image, current.image); + if (stale && (!baseStep || baseStep.stepId !== step.stepId || baseStep.revision !== step.revision)) { + throw new Error('This step changed outside the editor. Your unsaved edits are kept; reopen the guide after copying any text you want to keep.'); + } + // Merge each field and each block/annotation by id against the copy the + // editor loaded. Untouched local lists cannot resurrect deleted blocks or + // erase annotations added by an agent. The user wins a same-field conflict. + const content = stale ? mergeValue(baseStep, [step, current], { conflicts: 0 }) : step; // Screenshot operations have dedicated routes. A normal save cannot add // back a removed image, overwrite its dimensions, or restore its old kind. - const fields = { ...step, image: current.image, kind: current.kind }; + const fields = { ...content, image: current.image, kind: current.kind }; if (stale) { fields.parentStepId = current.parentStepId; fields.redaction = current.redaction; diff --git a/core/store.js b/core/store.js index 6066bbc9..05487bbb 100644 --- a/core/store.js +++ b/core/store.js @@ -369,6 +369,9 @@ class GuideStore { * nothing). Its revision still moves, so caches keyed on it refresh. */ saveStep(guideId, step, { expectedRevision = null, touch = true, imagePaths = false } = {}) { + // A capture/image/background operation can finish after an agent trashed + // the guide. Refuse before creating any files in the old location. + const guide = this.getGuide(guideId); // Optimistic concurrency for background/AI/capture writes: refuse to // overwrite a step that changed since it was read. Direct user edits pass // no expectedRevision (last-write-wins — the user is the authority). @@ -391,7 +394,6 @@ class GuideStore { stored.revision = (Number.isInteger(stored.revision) ? stored.revision : 0) + 1; validateStep(stored); writeJsonSync(path.join(this.stepDir(guideId, step.stepId), 'step.json'), stored); - const guide = this.getGuide(guideId); this.saveGuide(guide, { touch }); // bump updatedAt return stored; } @@ -514,7 +516,10 @@ class GuideStore { /** Replace the working image (crop result). The original is never touched. */ setWorkingImage(guideId, stepId, pngBuffer, size, stepPatch = null) { - const step = stepPatch ? this.currentImagePaths(guideId, deepClone(stepPatch)) : this.getStep(guideId, stepId); + this.getGuide(guideId); + const current = this.getStep(guideId, stepId); + const step = stepPatch ? this.currentImagePaths(guideId, deepClone(stepPatch)) : current; + step.revision = current.revision; if (!step.image) throw new Error('step has no image'); const unused = this.placeWorkingImage(this.stepDir(guideId, stepId), step.image, pngBuffer); step.image.size = size; @@ -530,7 +535,10 @@ class GuideStore { * `stepPatch` is saved with it, so text and annotations go in one write. */ replaceImages(guideId, stepId, images, size, stepPatch = null) { - const step = stepPatch ? this.currentImagePaths(guideId, deepClone(stepPatch)) : this.getStep(guideId, stepId); + this.getGuide(guideId); + const current = this.getStep(guideId, stepId); + const step = stepPatch ? this.currentImagePaths(guideId, deepClone(stepPatch)) : current; + step.revision = current.revision; if (!images?.original && !images?.working) throw new Error('No screenshot to use.'); if (!step.image) { step.image = { originalPath: ORIGINAL_FILE, workingPath: ORIGINAL_FILE, size }; diff --git a/docs/AI_AGENTS.md b/docs/AI_AGENTS.md index 73038608..efea293a 100644 --- a/docs/AI_AGENTS.md +++ b/docs/AI_AGENTS.md @@ -43,7 +43,10 @@ the change you want to undo. Restoring snapshots the guide first, so a restore can be undone too. Automatic backups don't push agent snapshots out. StepForge keeps the newest 50 -agent snapshots of each guide. +agent snapshots of each guide. Text rewrites, changing substep nesting with +`update_step`, adding placeholders and drawing annotations do not create agent +snapshots; create a manual snapshot first if you want a recovery point before +those edits. A guide an agent deletes goes to the Trash, like one you delete. Restore it from there. @@ -88,7 +91,9 @@ new ones for values that repeat, such as a course code or product name. If the guide is open in StepForge, it updates on screen within a couple of seconds. If you're typing in the same step at that moment, your edit wins. If the agent deleted that step, StepForge drops its pending save and shows the -remaining steps instead of recreating it. +remaining steps instead of recreating it. For a surviving step, changes to +different fields, blocks or annotations combine; your edit wins when both +sides changed the same field. Before you share the guide, read what the agent wrote. To undo a big rewrite, use **More → Backups & snapshots**. Annotations an agent drew can be moved or diff --git a/tests/unit/agent-editor-save.test.js b/tests/unit/agent-editor-save.test.js index ab446860..0ff7d5dc 100644 --- a/tests/unit/agent-editor-save.test.js +++ b/tests/unit/agent-editor-save.test.js @@ -68,10 +68,11 @@ test('a dirty step save preserves the parent changed by an agent move or deletio const { store, guideId, a, b } = library(t); store.reorderSteps(guideId, [a.stepId, b.stepId], { [b.stepId]: a.stepId }); const dirty = store.getStep(guideId, b.stepId); + const baseline = structuredClone(dirty); dirty.title = 'Unsaved user text'; store.deleteStep(guideId, a.stepId); const revision = store.getStep(guideId, b.stepId).revision; - const saved = saveStepFromEditor(store, guideId, dirty); + const saved = saveStepFromEditor(store, guideId, dirty, baseline); assert.equal(saved.revision, revision + 1); assert.equal(saved.title, 'Unsaved user text'); assert.equal(saved.parentStepId, null, 'do not restore a deleted parent'); @@ -91,10 +92,11 @@ test('stale text saves preserve screenshots removed, replaced or added by an age store.replaceImages(guideId, a.stepId, { original: TINY_PNG }, { width: 1, height: 1 }); const beforeRemoval = store.getStep(guideId, a.stepId); + const removalBase = structuredClone(beforeRemoval); beforeRemoval.title = 'User kept typing'; beforeRemoval.redaction = { checked: 'check-of-the-old-image' }; await call('remove_screenshot'); - const removed = saveStepFromEditor(store, guideId, beforeRemoval); + const removed = saveStepFromEditor(store, guideId, beforeRemoval, removalBase); assert.equal(removed.title, 'User kept typing'); assert.equal(removed.image, null, 'a text save cannot recreate the removed screenshot'); assert.equal(removed.kind, 'content'); @@ -103,24 +105,109 @@ test('stale text saves preserve screenshots removed, replaced or added by an age assert.equal(store.stepImagePath(guideId, a.stepId), null); const beforeAddition = store.getStep(guideId, a.stepId); + const additionBase = structuredClone(beforeAddition); beforeAddition.title = 'Typed while an image was added'; await call('set_screenshot', { image_path: png }); - const added = saveStepFromEditor(store, guideId, beforeAddition); + const added = saveStepFromEditor(store, guideId, beforeAddition, additionBase); assert.equal(added.title, beforeAddition.title); assert.equal(added.kind, 'image'); assert.deepEqual(added.image.size, { width: 40, height: 30 }); const beforeReplacement = store.getStep(guideId, a.stepId); + const replacementBase = structuredClone(beforeReplacement); beforeReplacement.title = 'Typed during replacement'; beforeReplacement.annotations = [{ id: 'old', type: 'rect', x: 0, y: 0, w: 1, h: 1 }]; beforeReplacement.redaction = { checked: 'stale-check' }; fs.writeFileSync(png, TINY_PNG); await call('set_screenshot', { image_path: png }); const replacement = store.getStep(guideId, a.stepId); - const saved = saveStepFromEditor(store, guideId, beforeReplacement); + const saved = saveStepFromEditor(store, guideId, beforeReplacement, replacementBase); assert.equal(saved.title, beforeReplacement.title); assert.deepEqual(saved.image, replacement.image, 'keep new dimensions and paths'); assert.deepEqual(saved.annotations, replacement.annotations, 'do not restore annotations for the old geometry'); assert.deepEqual(saved.redaction, replacement.redaction, 'do not restore a check for the old pixels'); assert.deepEqual(saved.image.size, { width: 1, height: 1 }); }); + + +test('three-way editor saves keep agent block deletions and new annotations alongside local text', async (t) => { + const { store, guideId, a } = library(t); + store.replaceImages(guideId, a.stepId, { original: TINY_PNG }, { width: 1, height: 1 }); + let step = store.getStep(guideId, a.stepId); + step.textBlocks = [{ id: 'delete-me', order: 1, title: 'Gone' }, { id: 'keep-me', order: 2, title: 'Original' }]; + step.codeBlocks = [{ id: 'code', order: 3, code: 'old' }]; + step.tableBlocks = [{ id: 'table', order: 4, rows: [['old']] }]; + step = store.saveStep(guideId, step); + const baseline = structuredClone(step); + const local = structuredClone(step); + local.title = 'Unsaved user title'; + local.textBlocks[1].title = 'User changed this block'; + const external = structuredClone(step); + external.textBlocks = external.textBlocks.filter((b) => b.id !== 'delete-me'); + external.codeBlocks = []; + external.tableBlocks = []; + external.annotations = [{ id: 'new', type: 'blur', x: 0, y: 0, w: .5, h: 1 }]; + store.saveStep(guideId, external); + + const api = { step: { save: async ({ step, baseStep }) => saveStepFromEditor(store, guideId, step, baseStep) } }; + const context = { ...(rendererI18n ? rendererI18n() : {}), window: { stepforge: api } }; + vm.runInNewContext(fs.readFileSync(path.join(__dirname, '../../app/renderer/editor.js'), 'utf8'), context); + const editor = Object.create(context.window.GuideEditor.prototype); + Object.assign(editor, { guideId, selectedStepId: null, steps: [local], stepMap: new Map([[a.stepId, local]]), + savedStepBaselines: new Map([[a.stepId, baseline]]), pendingSave: true, + onToast: (message) => assert.fail(message), emitMeta() {}, renderAll() {} }); + const saved = await editor.flushStep(local); + assert.equal(saved.title, local.title); + assert.deepEqual(saved.textBlocks.map((b) => [b.id, b.title]), [['keep-me', 'User changed this block']]); + assert.deepEqual(saved.codeBlocks, []); + assert.deepEqual(saved.tableBlocks, []); + assert.deepEqual(saved.annotations.map((a) => a.id), ['new']); + assert.deepEqual(editor.stepMap.get(a.stepId).textBlocks.map((b) => b.id), ['keep-me'], 'live blocks adopt the merged result'); + assert.equal(editor.savedStepBaselines.get(a.stepId).revision, saved.revision); +}); + +test('a stale editor save without a baseline refuses rather than losing external changes', (t) => { + const { store, guideId, a } = library(t); + const local = store.getStep(guideId, a.stepId); + store.saveStep(guideId, { ...local, title: 'Agent title' }); + assert.throws(() => saveStepFromEditor(store, guideId, { ...local, title: 'Local title' }), /unsaved edits are kept/); + assert.equal(store.getStep(guideId, a.stepId).title, 'Agent title'); +}); + +test('late image and background saves cannot recreate files after a guide moves to Trash', (t) => { + const { store, guideId, a } = library(t); + store.replaceImages(guideId, a.stepId, { original: TINY_PNG }, { width: 1, height: 1 }); + const stale = store.getStep(guideId, a.stepId); + store.deleteGuide(guideId); + for (const lateWrite of [ + () => store.saveStep(guideId, stale), + () => store.replaceImages(guideId, a.stepId, { original: TINY_PNG }, { width: 1, height: 1 }, stale), + () => store.setWorkingImage(guideId, a.stepId, TINY_PNG, { width: 1, height: 1 }, stale), + () => store.addStep(guideId, {}, TINY_PNG, { width: 1, height: 1 }), + () => store.restoreStep(guideId, stale, { original: TINY_PNG }, 0), + ]) assert.throws(lateWrite, /ENOENT/); + assert.equal(fs.existsSync(store.guideDir(guideId)), false, 'no ghost guide directory blocks restoration'); + const [trashed] = store.listTrash(); + assert.equal(store.restoreFromTrash(trashed), guideId); + assert.deepEqual(fs.readFileSync(store.stepImagePath(guideId, a.stepId)), TINY_PNG); +}); + + +test('undoing a saved edit uses the current revision and remains available after baseline-aware saving', async (t) => { + const { store, guideId, a } = library(t); + const before = store.getStep(guideId, a.stepId); + const current = saveStepFromEditor(store, guideId, { ...before, title: 'Saved user edit' }); + const priorRevision = current.revision; + const api = { step: { save: async ({ step, baseStep }) => saveStepFromEditor(store, guideId, step, baseStep) } }; + const context = { ...(rendererI18n ? rendererI18n() : {}), document: { activeElement: null }, window: { stepforge: api } }; + vm.runInNewContext(fs.readFileSync(path.join(__dirname, '../../app/renderer/editor.js'), 'utf8'), context); + const editor = Object.create(context.window.GuideEditor.prototype); + Object.assign(editor, { guideId, steps: [current], stepMap: new Map([[a.stepId, current]]), + savedStepBaselines: new Map([[a.stepId, structuredClone(current)]]), + saveStepDebounced: { cancel() {} }, onToast: (message) => assert.fail(message), + emitMeta() {}, renderAll() {}, renderStepList() {}, syncStepFields() {}, renderAnnotationPanel() {}, + canvas: { setAnnotations() {} }, dom: { annotationEditor: { contains: () => false } } }); + await editor.restoreHistoryRecord({ step: before }); + assert.equal(store.getStep(guideId, a.stepId).title, before.title); + assert.equal(store.getStep(guideId, a.stepId).revision, priorRevision + 1); +}); diff --git a/tests/unit/agent-mcp.test.js b/tests/unit/agent-mcp.test.js index 07bc3c3c..3bf82af4 100644 --- a/tests/unit/agent-mcp.test.js +++ b/tests/unit/agent-mcp.test.js @@ -64,7 +64,7 @@ test('agents get every guide tool except changing or deleting placeholders', (t) assert.ok(!/placeholder/.test(tool.name) || tool.name === 'create_placeholder'); assert.equal(tool.annotations.destructiveHint === true, /^(delete|remove|set_screenshot)/.test(tool.name), tool.name); } - assert.match(INSTRUCTIONS, /snapshots the guide before each of these/); + assert.match(INSTRUCTIONS, /snapshots existing guides before deletions, create_step, move_step and screenshot changes/); assert.match(INSTRUCTIONS, /cannot change or delete placeholders/); }); From 1d98bca0e6fab6708cb5bbfbbe03b82ecec4e951 Mon Sep 17 00:00:00 2001 From: Twest2 Date: Thu, 29 Oct 2026 11:43:15 -0500 Subject: [PATCH 7/7] Reconcile concurrent guide details and document agent deletion scope --- app/main.js | 7 ++++--- app/renderer/app.js | 3 ++- app/renderer/editor.js | 16 +++++++++++++--- core/editor-save.js | 9 +++++++-- docs/AI_AGENTS.md | 5 ++++- tests/unit/agent-editor-save.test.js | 24 +++++++++++++++++++++++- 6 files changed, 53 insertions(+), 11 deletions(-) diff --git a/app/main.js b/app/main.js index a4db4bf7..708e50b3 100644 --- a/app/main.js +++ b/app/main.js @@ -869,11 +869,12 @@ function setupIpc() { }), stepNumberMap(orderedSteps(guideId))), { validate: (a) => c.id(a.guideId) }); // An AI agent may have moved the guide to the Trash meanwhile; a late save // from the editor must not recreate it, which would also block its restore. - h('guide:save', ({ guide }) => { - const saved = saveGuideFromEditor(store, guide); + h('guide:save', ({ guide, baseGuide }) => { + const saved = saveGuideFromEditor(store, guide, baseGuide); reindex(guide.guideId); return saved; - }, { validate: (a) => security.isPlainArgs(a.guide) && a.guide && c.id(a.guide.guideId) }); + }, { validate: (a) => security.isPlainArgs(a.guide) && a.guide && c.id(a.guide.guideId) + && (a.baseGuide == null || (security.isPlainArgs(a.baseGuide) && a.baseGuide.guideId === a.guide.guideId)) }); h('step:add', ({ guideId, fields, imageBase64, size, position }) => { const buf = imageBase64 ? Buffer.from(imageBase64, 'base64') : null; const step = store.addStep(guideId, fields || {}, buf, size || null, { position }); diff --git a/app/renderer/app.js b/app/renderer/app.js index 8ef19aa8..fcc8a43c 100644 --- a/app/renderer/app.js +++ b/app/renderer/app.js @@ -1178,8 +1178,9 @@ class StepForgeApp { const title = await dialogs.promptText({ title: 'Rename guide', label: 'Title', value: guide.title }); if (title == null || !title.trim()) return; const fullGuide = (await api.guide.get({ guideId: guide.guideId })).guide; + const baseGuide = JSON.parse(JSON.stringify(fullGuide)); fullGuide.title = title.trim(); - await api.guide.save({ guide: fullGuide }); + await api.guide.save({ guide: fullGuide, baseGuide }); if (this.state.view === 'editor' && this.editor.guideId === fullGuide.guideId) { await this.editor.reload(this.editor.selectedStepId); } diff --git a/app/renderer/editor.js b/app/renderer/editor.js index 6e369618..29a7595e 100644 --- a/app/renderer/editor.js +++ b/app/renderer/editor.js @@ -548,6 +548,7 @@ class GuideEditor { const { guide, steps } = await api.guide.get({ guideId: this.guideId }); if (token !== this.stepLoadToken) return; this.guide = guide; + this.savedGuideBaseline = clone(guide); this.steps = steps; this.stepMap = new Map(steps.map((step) => [step.stepId, step])); this.savedStepBaselines = new Map(steps.map((step) => [step.stepId, clone(step)])); @@ -1939,11 +1940,20 @@ class GuideEditor { return existing; } + async saveGuideEdits() { + const saved = await api.guide.save({ guide: this.guide, baseGuide: this.savedGuideBaseline }); + if (saved?.guideId) { + this.guide = saved; + this.savedGuideBaseline = clone(saved); + } + return saved; + } + async flushGuide() { while (this.linkedSaveInFlight) await this.linkedSaveInFlight; // see flushStep if (!this.guide) return; try { - await api.guide.save({ guide: this.guide }); + await this.saveGuideEdits(); } catch (err) { this.pendingGuideSave = true; this.saveError = (err && err.message) || 'Save failed'; @@ -2560,7 +2570,7 @@ class GuideEditor { values: this.guide.placeholders || {}, onSave: async (values) => { this.guide.placeholders = values; - await api.guide.save({ guide: this.guide }); + await this.saveGuideEdits(); this.onToast('Placeholders saved.'); }, }); @@ -2579,7 +2589,7 @@ class GuideEditor { this.guide.metadata = metadata; this.guide.descriptionHtml = textToHtml(description); this.guide.cloud = { ...(this.guide.cloud || {}), sharingEnabled }; - await api.guide.save({ guide: this.guide }); + await this.saveGuideEdits(); this.onToast('Guide information saved.'); }, onCloudSnapshots: async () => { diff --git a/core/editor-save.js b/core/editor-save.js index ea714516..d8a1f11e 100644 --- a/core/editor-save.js +++ b/core/editor-save.js @@ -8,10 +8,15 @@ const { mergeValue } = require('./guide-merge'); * have their own operations, and may have changed in the agent process since * the editor loaded its copy. */ -function saveGuideFromEditor(store, guide) { +function saveGuideFromEditor(store, guide, baseGuide = null) { if (!store.guideExists(guide.guideId)) throw new Error('This guide was moved to the Trash.'); const current = store.getGuide(guide.guideId); - return store.saveGuide({ ...guide, revision: current.revision, stepsOrder: current.stepsOrder }); + const stale = guide.revision !== current.revision; + if (stale && (!baseGuide || baseGuide.guideId !== guide.guideId || baseGuide.revision !== guide.revision)) { + throw new Error('This guide changed outside the editor. Your unsaved edits are kept; reopen the guide after copying any text you want to keep.'); + } + const content = stale ? mergeValue(baseGuide, [guide, current], { conflicts: 0 }) : guide; + return store.saveGuide({ ...content, revision: current.revision, stepsOrder: current.stepsOrder, linkedSource: current.linkedSource }); } function saveStepFromEditor(store, guideId, step, baseStep = null) { diff --git a/docs/AI_AGENTS.md b/docs/AI_AGENTS.md index efea293a..100f731f 100644 --- a/docs/AI_AGENTS.md +++ b/docs/AI_AGENTS.md @@ -49,7 +49,10 @@ snapshots; create a manual snapshot first if you want a recovery point before those edits. A guide an agent deletes goes to the Trash, like one you delete. Restore it -from there. +from there. Its snapshot and Trash copy contain saved content; unsaved edits +in an open editor are not included when the agent deletes the guide. The +agent-access switch permits all the guide tools listed above; it is not a +separate approval for each deletion. ## 1. Turn on agent access diff --git a/tests/unit/agent-editor-save.test.js b/tests/unit/agent-editor-save.test.js index 0ff7d5dc..91a7a751 100644 --- a/tests/unit/agent-editor-save.test.js +++ b/tests/unit/agent-editor-save.test.js @@ -25,12 +25,13 @@ function library(t) { test('saving stale guide details preserves steps added, moved and deleted outside the editor', (t) => { const { store, guideId, a, b } = library(t); const stale = store.getGuide(guideId); + const baseline = structuredClone(stale); const c = store.addStep(guideId, { title: 'C' }); store.reorderSteps(guideId, [c.stepId, b.stepId, a.stepId]); store.deleteStep(guideId, a.stepId); stale.title = 'User typed title'; const revision = store.getGuide(guideId).revision; - const saved = saveGuideFromEditor(store, stale); + const saved = saveGuideFromEditor(store, stale, baseline); assert.equal(saved.revision, revision + 1); assert.equal(saved.title, 'User typed title'); assert.deepEqual(saved.stepsOrder, [c.stepId, b.stepId]); @@ -211,3 +212,24 @@ test('undoing a saved edit uses the current revision and remains available after assert.equal(store.getStep(guideId, a.stepId).title, before.title); assert.equal(store.getStep(guideId, a.stepId).revision, priorRevision + 1); }); + + +test('guide autosaves combine an unsaved description with an external title and reject unknown stale baselines', async (t) => { + const { store, guideId } = library(t); + const baseline = store.getGuide(guideId); + const local = { ...structuredClone(baseline), descriptionHtml: '

Unsaved user introduction

' }; + store.saveGuide({ ...baseline, title: 'Agent title' }); + const api = { guide: { save: async ({ guide, baseGuide }) => saveGuideFromEditor(store, guide, baseGuide) } }; + const context = { ...(rendererI18n ? rendererI18n() : {}), window: { stepforge: api } }; + vm.runInNewContext(fs.readFileSync(path.join(__dirname, '../../app/renderer/editor.js'), 'utf8'), context); + const editor = Object.create(context.window.GuideEditor.prototype); + Object.assign(editor, { guide: local, savedGuideBaseline: baseline, pendingGuideSave: true, + onToast: (message) => assert.fail(message), emitMeta() {} }); + await editor.flushGuide(); + assert.equal(editor.pendingGuideSave, false); + assert.equal(editor.guide.title, 'Agent title'); + assert.equal(editor.guide.descriptionHtml, local.descriptionHtml); + assert.equal(editor.savedGuideBaseline.revision, store.getGuide(guideId).revision); + assert.throws(() => saveGuideFromEditor(store, { ...baseline, title: 'Another stale title' }), /unsaved edits are kept/); + assert.equal(store.getGuide(guideId).title, 'Agent title'); +});