From 26dcd1a27c25a71fe160ec56d39c49871abb6913 Mon Sep 17 00:00:00 2001 From: Adam Bowker Date: Mon, 20 Jul 2026 11:59:13 -0400 Subject: [PATCH 1/4] feat(canvas): edit canvases as scratch files via canvas_checkout/publish MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Canvas generation previously required the agent to regenerate and emit the entire React source every turn. Add a canvas_checkout / canvas_publish local tool pair (available to both the Claude and Codex adapters) that moves the file I/O tool-side: checkout writes the live source to a scratch path and records the base version, the agent applies targeted edits with its native file tools, and publish reads the file from disk and appends a version — refusing when the canvas moved past the checkout base (concurrent edit or undo) instead of clobbering it. The generation prompt now routes through checkout → edit → publish and no longer embeds the current source. Storage stays wholesale full-file snapshots; a server-side base_version check on the desktop-fs PATCH remains a follow-up. Generated-By: PostHog Code Task-Id: 2b3d176e-3023-41d2-92e8-81eda7c12a8c --- .../agent/src/adapters/local-tools/index.ts | 3 + .../adapters/local-tools/tools/canvas.test.ts | 132 ++++++++ .../src/adapters/local-tools/tools/canvas.ts | 294 ++++++++++++++++++ packages/agent/src/posthog-api.ts | 37 +++ packages/core/src/canvas/canvasTemplates.ts | 5 +- packages/core/src/canvas/freeformSchemas.ts | 3 +- packages/ui/src/features/canvas/AGENTS.md | 13 +- .../features/canvas/freeformPrompt.test.ts | 36 ++- .../ui/src/features/canvas/freeformPrompt.ts | 73 ++++- 9 files changed, 570 insertions(+), 26 deletions(-) create mode 100644 packages/agent/src/adapters/local-tools/tools/canvas.test.ts create mode 100644 packages/agent/src/adapters/local-tools/tools/canvas.ts diff --git a/packages/agent/src/adapters/local-tools/index.ts b/packages/agent/src/adapters/local-tools/index.ts index c2aab7139d..716bbb478e 100644 --- a/packages/agent/src/adapters/local-tools/index.ts +++ b/packages/agent/src/adapters/local-tools/index.ts @@ -1,4 +1,5 @@ import type { LocalTool, LocalToolCtx, LocalToolGateMeta } from "./registry"; +import { canvasCheckoutTool, canvasPublishTool } from "./tools/canvas"; import { cloneRepoTool } from "./tools/clone-repo"; import { listReposTool } from "./tools/list-repos"; import { signedCommitTool } from "./tools/signed-commit"; @@ -23,6 +24,8 @@ export const LOCAL_TOOLS: LocalTool[] = [ listReposTool, cloneRepoTool, speakTool, + canvasCheckoutTool, + canvasPublishTool, ]; /** Tools whose gate passes for the given context — the set to actually expose. */ diff --git a/packages/agent/src/adapters/local-tools/tools/canvas.test.ts b/packages/agent/src/adapters/local-tools/tools/canvas.test.ts new file mode 100644 index 0000000000..bf563c6c5c --- /dev/null +++ b/packages/agent/src/adapters/local-tools/tools/canvas.test.ts @@ -0,0 +1,132 @@ +import { describe, expect, it } from "vitest"; +import { + canvasScratchDir, + canvasScratchFile, + composePublishedMeta, +} from "./canvas"; + +describe("canvas scratch paths", () => { + it("keys the scratch dir and file by canvas id, outside any workspace", () => { + expect(canvasScratchDir("dash-1")).toBe("/tmp/posthog-canvas/dash-1"); + expect(canvasScratchFile("dash-1")).toBe( + "/tmp/posthog-canvas/dash-1/canvas.tsx", + ); + }); +}); + +describe("composePublishedMeta", () => { + const v1 = { id: "v1", code: "one", createdAt: 1 }; + const v2 = { id: "v2", code: "two", createdAt: 2 }; + const v3 = { id: "v3", code: "three", createdAt: 3 }; + + it("appends a new head version when the base matches", () => { + const result = composePublishedMeta({ + freshMeta: { code: "two", versions: [v1, v2], currentVersionId: "v2" }, + baseVersionId: "v2", + code: "edited", + prompt: "tweak the chart", + now: 10, + }); + expect(result.ok).toBe(true); + if (!result.ok) return; + expect(result.meta.code).toBe("edited"); + expect(result.meta.currentVersionId).toBe(result.versionId); + expect(result.meta.versions).toHaveLength(3); + expect(result.meta.versions?.at(-1)).toMatchObject({ + code: "edited", + prompt: "tweak the chart", + createdAt: 10, + }); + expect(result.meta.updatedAt).toBe(10); + }); + + it("preserves unrelated meta keys", () => { + const result = composePublishedMeta({ + freshMeta: { + code: "one", + versions: [v1], + currentVersionId: "v1", + templateId: "freeform", + pinnedAt: 123, + }, + baseVersionId: "v1", + code: "edited", + now: 10, + }); + expect(result.ok).toBe(true); + if (!result.ok) return; + expect(result.meta.templateId).toBe("freeform"); + expect(result.meta.pinnedAt).toBe(123); + }); + + it("rejects when the canvas moved past the base (concurrent edit)", () => { + const result = composePublishedMeta({ + freshMeta: { + code: "three", + versions: [v1, v2, v3], + currentVersionId: "v3", + }, + baseVersionId: "v2", + code: "edited", + now: 10, + }); + expect(result.ok).toBe(false); + }); + + it("rejects a based publish onto a canvas with no versions", () => { + const result = composePublishedMeta({ + freshMeta: {}, + baseVersionId: "v1", + code: "edited", + now: 10, + }); + expect(result.ok).toBe(false); + }); + + it("truncates the redo tail when publishing from an undone version", () => { + // The user undid to v1 (pointer mid-history), then the agent edited from + // that checkout: the redo tail (v2, v3) is discarded — the same + // linear-discard the client's undo/redo uses. + const result = composePublishedMeta({ + freshMeta: { + code: "one", + versions: [v1, v2, v3], + currentVersionId: "v1", + }, + baseVersionId: "v1", + code: "edited", + now: 10, + }); + expect(result.ok).toBe(true); + if (!result.ok) return; + expect(result.meta.versions?.map((v) => v.id)).toEqual([ + "v1", + result.versionId, + ]); + }); + + it("seeds an empty canvas as the first version (no base)", () => { + const result = composePublishedMeta({ + freshMeta: { templateId: "freeform" }, + baseVersionId: undefined, + code: "first build", + now: 10, + }); + expect(result.ok).toBe(true); + if (!result.ok) return; + expect(result.meta.versions).toHaveLength(1); + expect(result.meta.currentVersionId).toBe(result.versionId); + expect(result.meta.code).toBe("first build"); + }); + + it("rejects an un-based publish onto a canvas that gained versions", () => { + // Checked out empty, but a concurrent first build published meanwhile. + const result = composePublishedMeta({ + freshMeta: { code: "one", versions: [v1], currentVersionId: "v1" }, + baseVersionId: undefined, + code: "edited", + now: 10, + }); + expect(result.ok).toBe(false); + }); +}); diff --git a/packages/agent/src/adapters/local-tools/tools/canvas.ts b/packages/agent/src/adapters/local-tools/tools/canvas.ts new file mode 100644 index 0000000000..ee0465fc1e --- /dev/null +++ b/packages/agent/src/adapters/local-tools/tools/canvas.ts @@ -0,0 +1,294 @@ +import { randomUUID } from "node:crypto"; +import { mkdirSync, readFileSync, writeFileSync } from "node:fs"; +import * as path from "node:path"; +import { z } from "zod"; +import { PostHogAPIClient } from "../../../posthog-api"; +import { resolveSandboxPosthogApi } from "../../../signed-commit-artefacts"; +import { defineLocalTool, type LocalToolResult } from "../registry"; + +/** + * Local tools for working on freeform canvases (desktop-fs `dashboard` rows) + * as scratch files, so the agent edits the source incrementally with its + * native file tools instead of regenerating (or transcribing) the whole file: + * + * - `canvas_checkout` fetches the canvas, writes `meta.code` to a + * deterministic scratch path tool-side (no model transcription), and stashes + * the fetched `currentVersionId` as the publish-time concurrency base. + * - `canvas_publish` reads the scratch file from disk (again, no + * transcription), verifies the canvas hasn't moved past the stashed base + * (a concurrent edit or user undo), appends a full-file version snapshot, + * and PATCHes the merged meta — mirroring `dashboardsService.saveFreeform` + * in `@posthog/core`, including the linear-discard of any redo tail. + * + * The check-then-PATCH guard is tool-side and therefore not atomic; it shrinks + * the clobber window from "the whole generation turn" to milliseconds. A + * server-side `base_version` check on the desktop-fs PATCH remains the + * follow-up that closes it completely. + * + * Credentials come from the sandbox environment (see + * `resolveSandboxPosthogApi`), so this works identically from the Claude + * in-process server and the Codex stdio child. + */ + +// Deliberately outside any workspace: scratch files never show up in +// changed-file diff panels or `git status` (a canvas task can lazily attach a +// repo), and the canvas id keeps concurrent generations from colliding. +export const CANVAS_SCRATCH_ROOT = "/tmp/posthog-canvas"; + +export function canvasScratchDir(canvasId: string): string { + return path.join(CANVAS_SCRATCH_ROOT, canvasId); +} + +export function canvasScratchFile(canvasId: string): string { + return path.join(canvasScratchDir(canvasId), "canvas.tsx"); +} + +function baseVersionMarkerFile(canvasId: string): string { + return path.join(canvasScratchDir(canvasId), ".base-version.json"); +} + +interface CanvasVersion { + id: string; + code: string; + context?: string; + prompt?: string; + createdAt: number; +} + +interface CanvasMeta { + code?: string; + versions?: CanvasVersion[]; + currentVersionId?: string; + updatedAt?: number; + [key: string]: unknown; +} + +interface CanvasFsEntry { + id: string; + path: string; + type?: string; + meta?: CanvasMeta | null; +} + +interface BaseVersionMarker { + versionId?: string; + fetchedAt: number; +} + +function createClient(): PostHogAPIClient | undefined { + const api = resolveSandboxPosthogApi(); + if (!api) return undefined; + return new PostHogAPIClient({ + apiUrl: api.apiUrl, + projectId: api.projectId, + getApiKey: () => api.apiKey, + }); +} + +async function fetchCanvasEntry(canvasId: string): Promise { + const client = createClient(); + if (!client) { + throw new Error("No PostHog credentials available in this session."); + } + const entry = await client.getDesktopFsEntry(canvasId); + if (!entry) { + throw new Error( + `Canvas ${canvasId} not found. Check the id — it should be a desktop-fs dashboard row id.`, + ); + } + if (entry.type !== "dashboard") { + throw new Error( + `Entry ${canvasId} is type "${entry.type}", not a canvas ("dashboard").`, + ); + } + return entry; +} + +async function patchCanvasMeta( + canvasId: string, + meta: CanvasMeta, +): Promise { + const client = createClient(); + if (!client) { + throw new Error("No PostHog credentials available in this session."); + } + await client.patchDesktopFsEntryMeta(canvasId, meta); +} + +function readMarker(canvasId: string): BaseVersionMarker | undefined { + try { + return JSON.parse( + readFileSync(baseVersionMarkerFile(canvasId), "utf8"), + ) as BaseVersionMarker; + } catch { + return undefined; + } +} + +function writeMarker(canvasId: string, marker: BaseVersionMarker): void { + mkdirSync(canvasScratchDir(canvasId), { recursive: true }); + writeFileSync(baseVersionMarkerFile(canvasId), JSON.stringify(marker)); +} + +/** + * Compose the published meta from a freshly-fetched entry: verify the base, + * truncate any redo tail past the current pointer (linear-discard, matching + * the client's undo semantics in `freeformSchemas.ts`), and append the new + * full-file snapshot. Pure — exported for tests. + */ +export function composePublishedMeta(input: { + freshMeta: CanvasMeta; + baseVersionId: string | undefined; + code: string; + prompt?: string; + now: number; +}): { ok: true; meta: CanvasMeta; versionId: string } | { ok: false } { + const { freshMeta, baseVersionId, code, prompt, now } = input; + if ((freshMeta.currentVersionId ?? undefined) !== baseVersionId) { + return { ok: false }; + } + const versions = freshMeta.versions ?? []; + const pointer = baseVersionId + ? versions.findIndex((v) => v.id === baseVersionId) + : -1; + const kept = pointer >= 0 ? versions.slice(0, pointer + 1) : []; + const version: CanvasVersion = { + id: randomUUID(), + code, + ...(prompt ? { prompt } : {}), + createdAt: now, + }; + return { + ok: true, + versionId: version.id, + meta: { + ...freshMeta, + code, + versions: [...kept, version], + currentVersionId: version.id, + updatedAt: now, + }, + }; +} + +function errorResult(text: string): LocalToolResult { + return { content: [{ type: "text", text }], isError: true }; +} + +const CONFLICT_MESSAGE = (canvasId: string) => + `version-conflict: the canvas changed since your checkout (a concurrent edit, or the user's undo). ` + + `Recover: call canvas_checkout with id "${canvasId}" again (it re-seeds the scratch file from the live source), ` + + `re-apply your edits to it, then call canvas_publish again.`; + +export const canvasCheckoutTool = defineLocalTool({ + name: "canvas_checkout", + description: + "Check out a PostHog canvas (a freeform React desktop-fs dashboard) for editing: fetches the live " + + "source, writes it to a local scratch file, and records the version your edits are based on. " + + "Returns the file path. Edit that file with your normal file-editing tools (targeted edits — do not " + + "regenerate it wholesale), then call canvas_publish to save. Always start canvas work with this tool.", + schema: { + id: z.string().describe("The canvas (desktop-fs dashboard row) id."), + }, + alwaysLoad: true, + isEnabled: () => resolveSandboxPosthogApi() !== undefined, + handler: async (_ctx, args): Promise => { + try { + const entry = await fetchCanvasEntry(args.id); + const file = canvasScratchFile(args.id); + const code = entry.meta?.code ?? ""; + mkdirSync(canvasScratchDir(args.id), { recursive: true }); + writeFileSync(file, code); + writeMarker(args.id, { + versionId: entry.meta?.currentVersionId, + fetchedAt: Date.now(), + }); + const lines = code ? code.split("\n").length : 0; + const text = code + ? `Checked out canvas "${entry.path}" to ${file} (${lines} lines, base version ${ + entry.meta?.currentVersionId ?? "none" + }).\nApply your changes by EDITING that file (targeted edits), then call canvas_publish with id "${args.id}".` + : `Canvas "${entry.path}" is empty. Author the complete single-file React app at ${file}, then call canvas_publish with id "${args.id}".`; + return { content: [{ type: "text", text }] }; + } catch (err) { + return errorResult( + `canvas_checkout failed: ${err instanceof Error ? err.message : String(err)}`, + ); + } + }, +}); + +export const canvasPublishTool = defineLocalTool({ + name: "canvas_publish", + description: + "Publish the checked-out canvas: reads the scratch file written by canvas_checkout, verifies the " + + "canvas hasn't changed since checkout, and saves the file as the canvas's new live version. Call " + + "exactly once when the edit is complete. On a version-conflict error, re-run canvas_checkout, " + + "re-apply your edits, and publish again.", + schema: { + id: z.string().describe("The canvas (desktop-fs dashboard row) id."), + prompt: z + .string() + .optional() + .describe( + "One short sentence describing the change, stored on the version history entry.", + ), + }, + alwaysLoad: true, + isEnabled: () => resolveSandboxPosthogApi() !== undefined, + handler: async (_ctx, args): Promise => { + let code: string; + try { + code = readFileSync(canvasScratchFile(args.id), "utf8"); + } catch { + return errorResult( + `canvas_publish failed: no scratch file for canvas "${args.id}". Call canvas_checkout first, edit the file it returns, then publish.`, + ); + } + if (!code.trim()) { + return errorResult( + `canvas_publish failed: the scratch file for canvas "${args.id}" is empty.`, + ); + } + const marker = readMarker(args.id); + if (!marker) { + return errorResult( + `canvas_publish failed: no checkout record for canvas "${args.id}". Call canvas_checkout first.`, + ); + } + try { + const fresh = await fetchCanvasEntry(args.id); + const composed = composePublishedMeta({ + freshMeta: fresh.meta ?? {}, + baseVersionId: marker.versionId, + code, + prompt: args.prompt, + now: Date.now(), + }); + if (!composed.ok) { + return errorResult( + `canvas_publish failed: ${CONFLICT_MESSAGE(args.id)}`, + ); + } + await patchCanvasMeta(args.id, composed.meta); + // Advance the base so a follow-up publish in the same session works + // without a re-checkout. + writeMarker(args.id, { + versionId: composed.versionId, + fetchedAt: Date.now(), + }); + return { + content: [ + { + type: "text", + text: `Published canvas "${args.id}" (new version ${composed.versionId}). The canvas is live; do not paste the code into chat.`, + }, + ], + }; + } catch (err) { + return errorResult( + `canvas_publish failed: ${err instanceof Error ? err.message : String(err)}`, + ); + } + }, +}); diff --git a/packages/agent/src/posthog-api.ts b/packages/agent/src/posthog-api.ts index b3596fe424..65421a6b91 100644 --- a/packages/agent/src/posthog-api.ts +++ b/packages/agent/src/posthog-api.ts @@ -338,6 +338,43 @@ export class PostHogAPIClient { .filter((artifact): artifact is TaskRunArtifact => !!artifact); } + /** + * Fetch a desktop file system entry (e.g. a canvas "dashboard" row) by id. + * Returns null on 404 so callers can produce a friendly not-found message. + */ + async getDesktopFsEntry(entryId: string): Promise { + const teamId = this.getTeamId(); + const response = await this.performRequestWithRetry( + `/api/projects/${teamId}/desktop_file_system/${encodeURIComponent(entryId)}/`, + ); + if (response.status === 404) { + return null; + } + if (!response.ok) { + throw new Error(`Failed to fetch desktop-fs entry (${response.status})`); + } + return response.json() as Promise; + } + + /** + * PATCH a desktop file system entry's meta blob (the endpoint stores the + * sent meta verbatim, so callers merge into the previously-fetched meta — + * the same read-modify-write contract as `dashboardsService` in core). + */ + async patchDesktopFsEntryMeta( + entryId: string, + meta: Record, + ): Promise { + const teamId = this.getTeamId(); + return this.apiRequest( + `/api/projects/${teamId}/desktop_file_system/${encodeURIComponent(entryId)}/`, + { + method: "PATCH", + body: JSON.stringify({ meta }), + }, + ); + } + /** Signal reports the given task is associated with (via report task associations). */ async getSignalReportIdsForTask(taskId: string): Promise { const teamId = this.getTeamId(); diff --git a/packages/core/src/canvas/canvasTemplates.ts b/packages/core/src/canvas/canvasTemplates.ts index d75cfba72b..7eb864c803 100644 --- a/packages/core/src/canvas/canvasTemplates.ts +++ b/packages/core/src/canvas/canvasTemplates.ts @@ -28,10 +28,9 @@ const FREEFORM_WHITELIST_NAMES = FREEFORM_WHITELIST.map((e) => e.name).join( const FREEFORM_BASE = [ "You are PostHog Canvas, an agent that builds a freeform React app for the user's current PostHog project. The app runs in a sandboxed iframe.", "", - "OUTPUT FORMAT — every turn:", - "- Write a SHORT sentence of prose, then the COMPLETE app as ONE fenced code block tagged tsx (```tsx ... ```).", - "- FULL-FILE REWRITE: always output the entire file, even for a tiny change. Never output a partial file, a diff, or multiple code blocks.", + "OUTPUT FORMAT — the app is ONE complete file:", "- The file MUST `export default` a single React component that takes no props.", + "- Maintain it as a working file with your file-editing tools: seed it once, then apply changes as TARGETED edits. Do not regenerate the whole file for a small change, and do not paste the source into chat — a short sentence describing the change is enough.", "", "IMPORTS — allowed packages ONLY:", `- You may import ONLY from: ${FREEFORM_WHITELIST_NAMES}.`, diff --git a/packages/core/src/canvas/freeformSchemas.ts b/packages/core/src/canvas/freeformSchemas.ts index 8f683bfa33..8a2592285c 100644 --- a/packages/core/src/canvas/freeformSchemas.ts +++ b/packages/core/src/canvas/freeformSchemas.ts @@ -5,7 +5,8 @@ import { z } from "zod"; export const FREEFORM_TEMPLATE_ID = "freeform"; // A single point in a freeform canvas's edit history. Every agent turn appends -// one full-file snapshot (Q7: full-file rewrite); the user can revert to any of +// one full-file snapshot (the agent edits a local scratch copy incrementally, +// then publishes the finished file wholesale); the user can revert to any of // them and the `currentVersionId` pointer is what publishes. We keep whole-file // snapshots rather than diffs because canvases are small and a snapshot can // never fail to reconstruct. diff --git a/packages/ui/src/features/canvas/AGENTS.md b/packages/ui/src/features/canvas/AGENTS.md index 0ea928dfbf..ba42a82f2b 100644 --- a/packages/ui/src/features/canvas/AGENTS.md +++ b/packages/ui/src/features/canvas/AGENTS.md @@ -76,9 +76,16 @@ The root `AGENTS.md` architecture rules still apply. and channel names in sync with the backend — the same surface that owns channels (top-level `folder` rows, see `hooks/useChannels.ts`). - `meta` is **last-write-wins, unversioned** at the fs layer (no `base_version`). - Freeform autosaves the whole file each agent turn, so a concurrent edit from - another client can clobber. Acceptable for now; revisit with optimistic - concurrency if multi-client editing becomes real. + The **agent publish path is guarded**: generation works the source as a local + scratch file via the `canvas_checkout` / `canvas_publish` local tools + (`@posthog/agent`, `adapters/local-tools/tools/canvas.ts`) — checkout records + the fetched `currentVersionId`, and publish re-fetches and refuses when the + canvas moved past that base (a concurrent edit or undo), instead of + clobbering it. The check is tool-side (check-then-PATCH, not atomic); a + server-side `base_version` check on the fs PATCH is the follow-up that closes + it completely. **User-side saves** (`saveFreeform`) are still last-write-wins; + revisit with the same optimistic concurrency if multi-client editing becomes + real. ## Channel sidebar preloading diff --git a/packages/ui/src/features/canvas/freeformPrompt.test.ts b/packages/ui/src/features/canvas/freeformPrompt.test.ts index 647f4cb465..cb7b3fdae2 100644 --- a/packages/ui/src/features/canvas/freeformPrompt.test.ts +++ b/packages/ui/src/features/canvas/freeformPrompt.test.ts @@ -21,19 +21,49 @@ describe("buildFreeformGenerationPrompt", () => { expect(extracted?.stripped).toBe("add a retention chart"); // The authoring contract + publishing rules are collapsed into the tag body. expect(extracted?.body).toContain("PUBLISHING"); + expect(extracted?.body).toContain("canvas_publish"); + }); + + it("routes all builds through checkout → edit file → publish", () => { + const prompt = buildFreeformGenerationPrompt(base); + const extracted = extractCanvasInstructions(prompt); + expect(extracted?.body).toContain("Build a freeform React canvas"); + expect(extracted?.body).toContain("canvas_checkout"); + expect(extracted?.body).toContain(`id: "dash-1"`); + // Publishing goes through the local tool; the remote MCP publish tool is + // named only to steer the agent away from it. + expect(extracted?.body).toContain("canvas_publish"); expect(extracted?.body).toContain( - "desktop-file-system-canvas-partial-update", + "`desktop-file-system-canvas-partial-update` directly", ); }); - it("folds the current code into the tag when editing", () => { + it("has edits work the scratch file instead of embedding the source", () => { const prompt = buildFreeformGenerationPrompt({ ...base, currentCode: "export const App = () => null;", }); const extracted = extractCanvasInstructions(prompt); expect(extracted?.stripped).toBe("add a retention chart"); - expect(extracted?.body).toContain("export const App = () => null;"); expect(extracted?.body).toContain("Edit the freeform React canvas"); + // The source is no longer folded into the prompt — canvas_checkout fetches + // the live code tool-side and the agent edits the scratch file in place. + expect(extracted?.body).not.toContain("export const App = () => null;"); + expect(extracted?.body).toContain("canvas_checkout"); + expect(extracted?.body).toContain("TARGETED edits"); + // The stale-publish recovery loop is spelled out. + expect(extracted?.body).toContain("version-conflict"); + }); + + it("seeds the starter scaffold into the checked-out file on a first build", () => { + const prompt = buildFreeformGenerationPrompt({ + ...base, + useStarter: true, + }); + const extracted = extractCanvasInstructions(prompt); + expect(extracted?.body).toContain("[Starter scaffold]"); + expect(extracted?.body).toContain("canvas_checkout"); + // The scaffold rides in the prompt (there is nothing to fetch yet). + expect(extracted?.body).toContain("```tsx"); }); }); diff --git a/packages/ui/src/features/canvas/freeformPrompt.ts b/packages/ui/src/features/canvas/freeformPrompt.ts index 32631fc0c7..1f49d1b97d 100644 --- a/packages/ui/src/features/canvas/freeformPrompt.ts +++ b/packages/ui/src/features/canvas/freeformPrompt.ts @@ -7,16 +7,24 @@ import { FREEFORM_STARTER_CODE } from "@posthog/core/canvas/freeformStarter"; // authoring contract // (imports, the `ph` data shim, Quill/style rules) therefore has to live in the // task's content (its first user message). The canvas is not a file on disk — it -// lives in PostHog — so the agent publishes the result via the PostHog MCP tool -// `desktop-file-system-canvas-partial-update` rather than replying with code or -// writing a file. +// lives in PostHog — but the agent WORKS on it as a local scratch file through +// the `canvas_checkout` / `canvas_publish` local tools (posthog-code-tools): +// checkout writes the live source to a scratch path tool-side and records the +// base version; the agent applies the change with its native file-editing tools +// (targeted edits, not a full regeneration — and no transcribing the source +// through chat); publish reads the file from disk and saves it as the new +// version, rejecting a publish whose base is stale (a concurrent edit or undo) +// instead of silently clobbering it. export function buildFreeformGenerationPrompt(input: { dashboardId: string; name: string; channelName: string; templateId?: string; instruction: string; - // The current source, when editing an existing canvas. Omitted for a first build. + // Present when editing an existing canvas; omitted for a first build. Only + // its presence matters — `canvas_checkout` fetches the authoritative source + // itself (fresher than anything embedded at task-creation time), so the + // content is no longer folded into the prompt. currentCode?: string; // Default on (opt out via the generate bar): seed a known-good starter // scaffold as the agent's baseline on a FIRST build, so it edits a compiling @@ -43,8 +51,17 @@ export function buildFreeformGenerationPrompt(input: { ? `Edit the freeform React canvas "${name}" in the channel "${channelName}", per the user's request at the start of this message.` : `Build a freeform React canvas "${name}" for the channel "${channelName}", per the user's request at the start of this message.`; - const currentBlock = isEdit - ? `\n[Current code] — the canvas as it stands now. Rewrite the WHOLE file with the change applied; do not output a partial file.\n\n\`\`\`tsx\n${currentCode}\n\`\`\`\n` + const checkoutStep = ` +[Working copy] — FIRST, check out the canvas: call the \`canvas_checkout\` tool +(posthog-code-tools) with id "${dashboardId}". It writes the canvas's live +source to a local scratch file and returns the path.`; + + const editStep = isEdit + ? ` +Then apply the user's request by EDITING that file with your file-editing +tools. Make TARGETED edits — do not rewrite the whole file for a small change, +and do not paste the source into chat. +` : ""; // First-build only: hand the agent a working scaffold to build ON instead of @@ -52,7 +69,26 @@ export function buildFreeformGenerationPrompt(input: { // picker, theme tokens, loading skeletons, typed-node result reading). const starterBlock = !isEdit && useStarter - ? `\n[Starter scaffold] — begin from this WORKING baseline instead of authoring from scratch. It already wires the things that are easy to get wrong: the date picker, theme-aware tokens, per-card loading skeletons, and reading a typed-node result correctly. KEEP that wiring; replace the sample "total events" metric and the layout with what the user asked for, and output the COMPLETE rewritten file.\n\n\`\`\`tsx\n${FREEFORM_STARTER_CODE}\n\`\`\`\n` + ? ` +[Starter scaffold] — the canvas is empty, so write this WORKING baseline to the +checked-out path, then build by EDITING that file. It already wires the things +that are easy to get wrong: the date picker, theme-aware tokens, per-card +loading skeletons, and reading a typed-node result correctly. KEEP that wiring; +replace the sample "total events" metric and the layout with what the user +asked for. + +\`\`\`tsx +${FREEFORM_STARTER_CODE} +\`\`\` +` + : ""; + + const freshBuildStep = + !isEdit && !useStarter + ? ` +The canvas is empty: author the complete app at the checked-out path and +iterate on it there with your file-editing tools. +` : ""; // The standing authoring contract + publishing/data rules are the same @@ -62,22 +98,27 @@ export function buildFreeformGenerationPrompt(input: { // inline (see extractCanvasInstructions). Kept after the user's instruction so // the request leads, mirroring how channel CONTEXT.md is appended. const instructions = `${header} -${currentBlock}${starterBlock} +${checkoutStep}${editStep}${starterBlock}${freshBuildStep} Follow this authoring contract for the canvas (imports, the \`ph\` data shim, and style rules): ${contract} -PUBLISHING — this OVERRIDES any instruction above about replying with the code in -a fenced \`\`\`tsx block. In this task you do NOT reply with the code. When the -canvas is ready, PUBLISH it by calling the PostHog MCP tool -\`desktop-file-system-canvas-partial-update\` exactly once with: +PUBLISHING — the scratch file is only your working copy; the canvas lives in +PostHog. When it is ready, call the \`canvas_publish\` tool (posthog-code-tools) +exactly once with: - id: "${dashboardId}" -- code: the COMPLETE single-file React source for the canvas. +- prompt: one short sentence describing the change. + +It reads the scratch file from disk and saves it as the canvas's new version — +do not paste the code into the tool call or into chat, and do not call +\`desktop-file-system-canvas-partial-update\` directly. If it fails with a +version-conflict error, the canvas changed while you worked (another edit, or +the user's undo): call \`canvas_checkout\` again, re-apply your edits to the +re-seeded file, then publish again. -The canvas lives in PostHog, not on disk — calling that MCP tool is what saves it. -Do not write a local file. Verify event/property names via the PostHog MCP before -using them, and operate only on this project. +Verify event/property names via the PostHog MCP before using them, and operate +only on this project. DATA — for each metric, first SAVE an insight via the PostHog MCP insight tools (prefer an insight query type — Trends, Funnels, Retention, web-analytics kinds — From 650cab764125e48c13dc72c0d1b1fe2b4bfc20d7 Mon Sep 17 00:00:00 2001 From: Adam Bowker Date: Mon, 20 Jul 2026 12:21:36 -0400 Subject: [PATCH 2/4] refactor(canvas): delegate publish composition to the desktop-fs canvas action canvas_publish now calls the desktop-fs canvas action, which owns version composition server-side, passing the checkout's version as expected_current_version_id so a stale publish is rejected atomically (409 version_conflict) instead of guarded by a non-atomic tool-side check-then-PATCH. Deletes the tool-side compose logic and the raw meta PATCH; backends predating the guard field ignore it and publish unguarded, so the tool degrades gracefully. Server side: PostHog/posthog#72365. Generated-By: PostHog Code Task-Id: 2b3d176e-3023-41d2-92e8-81eda7c12a8c --- .../adapters/local-tools/tools/canvas.test.ts | 126 +----------------- .../src/adapters/local-tools/tools/canvas.ts | 120 ++++------------- packages/agent/src/posthog-api.ts | 58 ++++++-- packages/ui/src/features/canvas/AGENTS.md | 24 ++-- 4 files changed, 93 insertions(+), 235 deletions(-) diff --git a/packages/agent/src/adapters/local-tools/tools/canvas.test.ts b/packages/agent/src/adapters/local-tools/tools/canvas.test.ts index bf563c6c5c..7cf32f9e2c 100644 --- a/packages/agent/src/adapters/local-tools/tools/canvas.test.ts +++ b/packages/agent/src/adapters/local-tools/tools/canvas.test.ts @@ -1,10 +1,9 @@ import { describe, expect, it } from "vitest"; -import { - canvasScratchDir, - canvasScratchFile, - composePublishedMeta, -} from "./canvas"; +import { canvasScratchDir, canvasScratchFile } from "./canvas"; +// Version composition and the stale-base rejection live server-side in the +// desktop-fs canvas action (and are tested there); the tool's own logic is +// the scratch-file plumbing. describe("canvas scratch paths", () => { it("keys the scratch dir and file by canvas id, outside any workspace", () => { expect(canvasScratchDir("dash-1")).toBe("/tmp/posthog-canvas/dash-1"); @@ -13,120 +12,3 @@ describe("canvas scratch paths", () => { ); }); }); - -describe("composePublishedMeta", () => { - const v1 = { id: "v1", code: "one", createdAt: 1 }; - const v2 = { id: "v2", code: "two", createdAt: 2 }; - const v3 = { id: "v3", code: "three", createdAt: 3 }; - - it("appends a new head version when the base matches", () => { - const result = composePublishedMeta({ - freshMeta: { code: "two", versions: [v1, v2], currentVersionId: "v2" }, - baseVersionId: "v2", - code: "edited", - prompt: "tweak the chart", - now: 10, - }); - expect(result.ok).toBe(true); - if (!result.ok) return; - expect(result.meta.code).toBe("edited"); - expect(result.meta.currentVersionId).toBe(result.versionId); - expect(result.meta.versions).toHaveLength(3); - expect(result.meta.versions?.at(-1)).toMatchObject({ - code: "edited", - prompt: "tweak the chart", - createdAt: 10, - }); - expect(result.meta.updatedAt).toBe(10); - }); - - it("preserves unrelated meta keys", () => { - const result = composePublishedMeta({ - freshMeta: { - code: "one", - versions: [v1], - currentVersionId: "v1", - templateId: "freeform", - pinnedAt: 123, - }, - baseVersionId: "v1", - code: "edited", - now: 10, - }); - expect(result.ok).toBe(true); - if (!result.ok) return; - expect(result.meta.templateId).toBe("freeform"); - expect(result.meta.pinnedAt).toBe(123); - }); - - it("rejects when the canvas moved past the base (concurrent edit)", () => { - const result = composePublishedMeta({ - freshMeta: { - code: "three", - versions: [v1, v2, v3], - currentVersionId: "v3", - }, - baseVersionId: "v2", - code: "edited", - now: 10, - }); - expect(result.ok).toBe(false); - }); - - it("rejects a based publish onto a canvas with no versions", () => { - const result = composePublishedMeta({ - freshMeta: {}, - baseVersionId: "v1", - code: "edited", - now: 10, - }); - expect(result.ok).toBe(false); - }); - - it("truncates the redo tail when publishing from an undone version", () => { - // The user undid to v1 (pointer mid-history), then the agent edited from - // that checkout: the redo tail (v2, v3) is discarded — the same - // linear-discard the client's undo/redo uses. - const result = composePublishedMeta({ - freshMeta: { - code: "one", - versions: [v1, v2, v3], - currentVersionId: "v1", - }, - baseVersionId: "v1", - code: "edited", - now: 10, - }); - expect(result.ok).toBe(true); - if (!result.ok) return; - expect(result.meta.versions?.map((v) => v.id)).toEqual([ - "v1", - result.versionId, - ]); - }); - - it("seeds an empty canvas as the first version (no base)", () => { - const result = composePublishedMeta({ - freshMeta: { templateId: "freeform" }, - baseVersionId: undefined, - code: "first build", - now: 10, - }); - expect(result.ok).toBe(true); - if (!result.ok) return; - expect(result.meta.versions).toHaveLength(1); - expect(result.meta.currentVersionId).toBe(result.versionId); - expect(result.meta.code).toBe("first build"); - }); - - it("rejects an un-based publish onto a canvas that gained versions", () => { - // Checked out empty, but a concurrent first build published meanwhile. - const result = composePublishedMeta({ - freshMeta: { code: "one", versions: [v1], currentVersionId: "v1" }, - baseVersionId: undefined, - code: "edited", - now: 10, - }); - expect(result.ok).toBe(false); - }); -}); diff --git a/packages/agent/src/adapters/local-tools/tools/canvas.ts b/packages/agent/src/adapters/local-tools/tools/canvas.ts index ee0465fc1e..2e391deea8 100644 --- a/packages/agent/src/adapters/local-tools/tools/canvas.ts +++ b/packages/agent/src/adapters/local-tools/tools/canvas.ts @@ -1,8 +1,10 @@ -import { randomUUID } from "node:crypto"; import { mkdirSync, readFileSync, writeFileSync } from "node:fs"; import * as path from "node:path"; import { z } from "zod"; -import { PostHogAPIClient } from "../../../posthog-api"; +import { + DesktopCanvasVersionConflictError, + PostHogAPIClient, +} from "../../../posthog-api"; import { resolveSandboxPosthogApi } from "../../../signed-commit-artefacts"; import { defineLocalTool, type LocalToolResult } from "../registry"; @@ -15,15 +17,11 @@ import { defineLocalTool, type LocalToolResult } from "../registry"; * deterministic scratch path tool-side (no model transcription), and stashes * the fetched `currentVersionId` as the publish-time concurrency base. * - `canvas_publish` reads the scratch file from disk (again, no - * transcription), verifies the canvas hasn't moved past the stashed base - * (a concurrent edit or user undo), appends a full-file version snapshot, - * and PATCHes the merged meta — mirroring `dashboardsService.saveFreeform` - * in `@posthog/core`, including the linear-discard of any redo tail. - * - * The check-then-PATCH guard is tool-side and therefore not atomic; it shrinks - * the clobber window from "the whole generation turn" to milliseconds. A - * server-side `base_version` check on the desktop-fs PATCH remains the - * follow-up that closes it completely. + * transcription) and publishes through the desktop-fs canvas action, which + * owns version composition server-side. The stashed base rides along as + * `expected_current_version_id`, so a publish based on a stale read (a + * concurrent edit, or a user's undo) is rejected atomically with a version + * conflict instead of clobbering the newer head. * * Credentials come from the sandbox environment (see * `resolveSandboxPosthogApi`), so this works identically from the Claude @@ -47,19 +45,9 @@ function baseVersionMarkerFile(canvasId: string): string { return path.join(canvasScratchDir(canvasId), ".base-version.json"); } -interface CanvasVersion { - id: string; - code: string; - context?: string; - prompt?: string; - createdAt: number; -} - interface CanvasMeta { code?: string; - versions?: CanvasVersion[]; currentVersionId?: string; - updatedAt?: number; [key: string]: unknown; } @@ -104,17 +92,6 @@ async function fetchCanvasEntry(canvasId: string): Promise { return entry; } -async function patchCanvasMeta( - canvasId: string, - meta: CanvasMeta, -): Promise { - const client = createClient(); - if (!client) { - throw new Error("No PostHog credentials available in this session."); - } - await client.patchDesktopFsEntryMeta(canvasId, meta); -} - function readMarker(canvasId: string): BaseVersionMarker | undefined { try { return JSON.parse( @@ -130,47 +107,6 @@ function writeMarker(canvasId: string, marker: BaseVersionMarker): void { writeFileSync(baseVersionMarkerFile(canvasId), JSON.stringify(marker)); } -/** - * Compose the published meta from a freshly-fetched entry: verify the base, - * truncate any redo tail past the current pointer (linear-discard, matching - * the client's undo semantics in `freeformSchemas.ts`), and append the new - * full-file snapshot. Pure — exported for tests. - */ -export function composePublishedMeta(input: { - freshMeta: CanvasMeta; - baseVersionId: string | undefined; - code: string; - prompt?: string; - now: number; -}): { ok: true; meta: CanvasMeta; versionId: string } | { ok: false } { - const { freshMeta, baseVersionId, code, prompt, now } = input; - if ((freshMeta.currentVersionId ?? undefined) !== baseVersionId) { - return { ok: false }; - } - const versions = freshMeta.versions ?? []; - const pointer = baseVersionId - ? versions.findIndex((v) => v.id === baseVersionId) - : -1; - const kept = pointer >= 0 ? versions.slice(0, pointer + 1) : []; - const version: CanvasVersion = { - id: randomUUID(), - code, - ...(prompt ? { prompt } : {}), - createdAt: now, - }; - return { - ok: true, - versionId: version.id, - meta: { - ...freshMeta, - code, - versions: [...kept, version], - currentVersionId: version.id, - updatedAt: now, - }, - }; -} - function errorResult(text: string): LocalToolResult { return { content: [{ type: "text", text }], isError: true }; } @@ -221,8 +157,8 @@ export const canvasCheckoutTool = defineLocalTool({ export const canvasPublishTool = defineLocalTool({ name: "canvas_publish", description: - "Publish the checked-out canvas: reads the scratch file written by canvas_checkout, verifies the " + - "canvas hasn't changed since checkout, and saves the file as the canvas's new live version. Call " + + "Publish the checked-out canvas: reads the scratch file written by canvas_checkout and saves it as " + + "the canvas's new live version, guarded against the canvas having changed since checkout. Call " + "exactly once when the edit is complete. On a version-conflict error, re-run canvas_checkout, " + "re-apply your edits, and publish again.", schema: { @@ -256,36 +192,38 @@ export const canvasPublishTool = defineLocalTool({ `canvas_publish failed: no checkout record for canvas "${args.id}". Call canvas_checkout first.`, ); } + const client = createClient(); + if (!client) { + return errorResult( + "canvas_publish failed: no PostHog credentials available in this session.", + ); + } try { - const fresh = await fetchCanvasEntry(args.id); - const composed = composePublishedMeta({ - freshMeta: fresh.meta ?? {}, - baseVersionId: marker.versionId, + const entry = await client.publishDesktopCanvas(args.id, { code, prompt: args.prompt, - now: Date.now(), + expectedCurrentVersionId: marker.versionId ?? null, }); - if (!composed.ok) { - return errorResult( - `canvas_publish failed: ${CONFLICT_MESSAGE(args.id)}`, - ); - } - await patchCanvasMeta(args.id, composed.meta); // Advance the base so a follow-up publish in the same session works // without a re-checkout. - writeMarker(args.id, { - versionId: composed.versionId, - fetchedAt: Date.now(), - }); + const newVersionId = entry.meta?.currentVersionId; + writeMarker(args.id, { versionId: newVersionId, fetchedAt: Date.now() }); return { content: [ { type: "text", - text: `Published canvas "${args.id}" (new version ${composed.versionId}). The canvas is live; do not paste the code into chat.`, + text: `Published canvas "${args.id}"${ + newVersionId ? ` (new version ${newVersionId})` : "" + }. The canvas is live; do not paste the code into chat.`, }, ], }; } catch (err) { + if (err instanceof DesktopCanvasVersionConflictError) { + return errorResult( + `canvas_publish failed: ${CONFLICT_MESSAGE(args.id)}`, + ); + } return errorResult( `canvas_publish failed: ${err instanceof Error ? err.message : String(err)}`, ); diff --git a/packages/agent/src/posthog-api.ts b/packages/agent/src/posthog-api.ts index 65421a6b91..9d5f425a66 100644 --- a/packages/agent/src/posthog-api.ts +++ b/packages/agent/src/posthog-api.ts @@ -63,6 +63,14 @@ export type TaskRunUpdate = Partial< state_remove_keys?: string[]; }; +/** A guarded canvas publish was based on a stale version (409 from the API). */ +export class DesktopCanvasVersionConflictError extends Error { + constructor(readonly currentVersionId: string | null) { + super("Canvas version conflict: the canvas changed since it was read."); + this.name = "DesktopCanvasVersionConflictError"; + } +} + export class PostHogAPIClient { private config: PostHogAPIConfig; @@ -357,22 +365,50 @@ export class PostHogAPIClient { } /** - * PATCH a desktop file system entry's meta blob (the endpoint stores the - * sent meta verbatim, so callers merge into the previously-fetched meta — - * the same read-modify-write contract as `dashboardsService` in core). + * Publish a freeform canvas's source via the desktop-fs canvas action. The + * server owns version composition (appends the full-file snapshot, moves + * `currentVersionId`, truncates any redo tail) and rejects a publish whose + * `expectedCurrentVersionId` no longer matches the live head with a 409 — + * surfaced as DesktopCanvasVersionConflictError. Backends predating the + * guard ignore the field and publish unguarded, so this degrades gracefully. */ - async patchDesktopFsEntryMeta( + async publishDesktopCanvas( entryId: string, - meta: Record, + input: { + code: string; + prompt?: string; + /** The head version the code was based on; null when it was empty. */ + expectedCurrentVersionId: string | null; + }, ): Promise { const teamId = this.getTeamId(); - return this.apiRequest( - `/api/projects/${teamId}/desktop_file_system/${encodeURIComponent(entryId)}/`, - { - method: "PATCH", - body: JSON.stringify({ meta }), - }, + const body: Record = { + code: input.code, + expected_current_version_id: input.expectedCurrentVersionId, + }; + if (input.prompt) { + body.prompt = input.prompt; + } + const response = await this.performRequestWithRetry( + `/api/projects/${teamId}/desktop_file_system/${encodeURIComponent(entryId)}/canvas/`, + { method: "PATCH", body: JSON.stringify(body) }, ); + if (response.status === 409) { + let currentVersionId: string | null = null; + try { + const parsed = (await response.json()) as { + current_version_id?: string | null; + }; + currentVersionId = parsed.current_version_id ?? null; + } catch { + // Conflict body unavailable — the status alone carries the signal. + } + throw new DesktopCanvasVersionConflictError(currentVersionId); + } + if (!response.ok) { + throw new Error(`Failed to publish canvas (${response.status})`); + } + return response.json() as Promise; } /** Signal reports the given task is associated with (via report task associations). */ diff --git a/packages/ui/src/features/canvas/AGENTS.md b/packages/ui/src/features/canvas/AGENTS.md index ba42a82f2b..a21bc3a65a 100644 --- a/packages/ui/src/features/canvas/AGENTS.md +++ b/packages/ui/src/features/canvas/AGENTS.md @@ -75,17 +75,19 @@ The root `AGENTS.md` architecture rules still apply. documented as `DashboardFileMeta` in `dashboardSchemas.ts`. This keeps canvas and channel names in sync with the backend — the same surface that owns channels (top-level `folder` rows, see `hooks/useChannels.ts`). -- `meta` is **last-write-wins, unversioned** at the fs layer (no `base_version`). - The **agent publish path is guarded**: generation works the source as a local - scratch file via the `canvas_checkout` / `canvas_publish` local tools - (`@posthog/agent`, `adapters/local-tools/tools/canvas.ts`) — checkout records - the fetched `currentVersionId`, and publish re-fetches and refuses when the - canvas moved past that base (a concurrent edit or undo), instead of - clobbering it. The check is tool-side (check-then-PATCH, not atomic); a - server-side `base_version` check on the fs PATCH is the follow-up that closes - it completely. **User-side saves** (`saveFreeform`) are still last-write-wins; - revisit with the same optimistic concurrency if multi-client editing becomes - real. +- `meta` is **last-write-wins, unversioned** at the fs layer (no `base_version`) + — except the **canvas publish action** (`PATCH …/:id/canvas/`), which owns + version composition server-side and accepts an optional + `expected_current_version_id`: a publish based on a stale version (a + concurrent edit or undo) is rejected 409 inside the row lock instead of + clobbering the newer head. The **agent publish path uses that guard**: + generation works the source as a local scratch file via the + `canvas_checkout` / `canvas_publish` local tools (`@posthog/agent`, + `adapters/local-tools/tools/canvas.ts`) — checkout records the fetched + `currentVersionId`, publish passes it as the expected version (backends + predating the field ignore it and publish unguarded). **User-side saves** + (`saveFreeform`) are still last-write-wins; adopt the same guard if + multi-client editing becomes real. ## Channel sidebar preloading From 40df5417351c4abfd13acac3b9b90922e1cf641d Mon Sep 17 00:00:00 2001 From: Adam Bowker Date: Mon, 20 Jul 2026 13:05:09 -0400 Subject: [PATCH 3/4] refactor(canvas): drop vestigial anti-rewrite prompting from canvas tools The "do not regenerate wholesale" phrasing in the checkout tool description, its runtime message, and the generation prompt warned against a workflow a fresh agent never sees, and could bias against legitimately large rewrites. The one forward-looking economy nudge (prefer targeted edits over rewriting the whole file for a small change) stays in the authoring contract in canvasTemplates.ts, stated once. Generated-By: PostHog Code Task-Id: 2b3d176e-3023-41d2-92e8-81eda7c12a8c --- packages/agent/src/adapters/local-tools/tools/canvas.ts | 6 +++--- packages/ui/src/features/canvas/freeformPrompt.test.ts | 2 +- packages/ui/src/features/canvas/freeformPrompt.ts | 5 ++--- 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/packages/agent/src/adapters/local-tools/tools/canvas.ts b/packages/agent/src/adapters/local-tools/tools/canvas.ts index 2e391deea8..d846678dfd 100644 --- a/packages/agent/src/adapters/local-tools/tools/canvas.ts +++ b/packages/agent/src/adapters/local-tools/tools/canvas.ts @@ -121,8 +121,8 @@ export const canvasCheckoutTool = defineLocalTool({ description: "Check out a PostHog canvas (a freeform React desktop-fs dashboard) for editing: fetches the live " + "source, writes it to a local scratch file, and records the version your edits are based on. " + - "Returns the file path. Edit that file with your normal file-editing tools (targeted edits — do not " + - "regenerate it wholesale), then call canvas_publish to save. Always start canvas work with this tool.", + "Returns the file path. Edit that file with your normal file-editing tools, then call " + + "canvas_publish to save. Always start canvas work with this tool.", schema: { id: z.string().describe("The canvas (desktop-fs dashboard row) id."), }, @@ -143,7 +143,7 @@ export const canvasCheckoutTool = defineLocalTool({ const text = code ? `Checked out canvas "${entry.path}" to ${file} (${lines} lines, base version ${ entry.meta?.currentVersionId ?? "none" - }).\nApply your changes by EDITING that file (targeted edits), then call canvas_publish with id "${args.id}".` + }).\nApply your changes by editing that file, then call canvas_publish with id "${args.id}".` : `Canvas "${entry.path}" is empty. Author the complete single-file React app at ${file}, then call canvas_publish with id "${args.id}".`; return { content: [{ type: "text", text }] }; } catch (err) { diff --git a/packages/ui/src/features/canvas/freeformPrompt.test.ts b/packages/ui/src/features/canvas/freeformPrompt.test.ts index cb7b3fdae2..741a16c874 100644 --- a/packages/ui/src/features/canvas/freeformPrompt.test.ts +++ b/packages/ui/src/features/canvas/freeformPrompt.test.ts @@ -50,7 +50,7 @@ describe("buildFreeformGenerationPrompt", () => { // the live code tool-side and the agent edits the scratch file in place. expect(extracted?.body).not.toContain("export const App = () => null;"); expect(extracted?.body).toContain("canvas_checkout"); - expect(extracted?.body).toContain("TARGETED edits"); + expect(extracted?.body).toContain("editing that file"); // The stale-publish recovery loop is spelled out. expect(extracted?.body).toContain("version-conflict"); }); diff --git a/packages/ui/src/features/canvas/freeformPrompt.ts b/packages/ui/src/features/canvas/freeformPrompt.ts index 1f49d1b97d..caa9ab30ec 100644 --- a/packages/ui/src/features/canvas/freeformPrompt.ts +++ b/packages/ui/src/features/canvas/freeformPrompt.ts @@ -58,9 +58,8 @@ source to a local scratch file and returns the path.`; const editStep = isEdit ? ` -Then apply the user's request by EDITING that file with your file-editing -tools. Make TARGETED edits — do not rewrite the whole file for a small change, -and do not paste the source into chat. +Then apply the user's request by editing that file with your file-editing +tools. Do not paste the source into chat. ` : ""; From 3c615278bbcad1a1a95e9548eab9c02c429b1f70 Mon Sep 17 00:00:00 2001 From: Adam Bowker Date: Mon, 20 Jul 2026 14:40:16 -0400 Subject: [PATCH 4/4] feat(canvas): comment-mode annotation overlay with element and text targets MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Canvas feedback was whole-document chat: describing the target in prose was the only way to point at something. Add a comment mode to freeform canvases: a host-authored overlay in the sandbox iframe captures element clicks and text selections as structured, LLM-locatable targets (stable-attribute selector, bounded text, attributes), queued annotations render as numbered pins in the canvas and editable comment chips above the composer, and on submit they fold into the generation instruction as a numbered [Annotations] block the agent resolves into targeted edits of the checked-out scratch file. Selector inference follows the toolbar's product-tours approach (stable attribute allowlist, nth-of-type fallback) in a dependency-free form — precision is secondary since the consumer is the agent locating spots in source it wrote itself. Generated-By: PostHog Code Task-Id: 2b3d176e-3023-41d2-92e8-81eda7c12a8c --- packages/core/src/canvas/freeformSchemas.ts | 65 +++++++ .../canvas/freeform/CanvasFrameHost.tsx | 3 + .../freeform/CanvasFramePlaceholder.tsx | 14 ++ .../canvas/freeform/FreeformCanvas.tsx | 65 ++++++- .../canvas/freeform/FreeformCanvasView.tsx | 59 ++++++- .../canvas/freeform/FreeformGenerateBar.tsx | 138 +++++++++++++-- .../canvas/freeform/canvasFrameStore.ts | 5 + .../canvas/freeform/sandboxRuntime.ts | 167 +++++++++++++++++- .../features/canvas/freeformPrompt.test.ts | 46 +++++ .../ui/src/features/canvas/freeformPrompt.ts | 45 ++++- .../canvas/hooks/useGenerateFreeformCanvas.ts | 9 +- .../canvas/stores/canvasAnnotationsStore.ts | 66 +++++++ 12 files changed, 662 insertions(+), 20 deletions(-) create mode 100644 packages/ui/src/features/canvas/stores/canvasAnnotationsStore.ts diff --git a/packages/core/src/canvas/freeformSchemas.ts b/packages/core/src/canvas/freeformSchemas.ts index 8a2592285c..be44d942d9 100644 --- a/packages/core/src/canvas/freeformSchemas.ts +++ b/packages/core/src/canvas/freeformSchemas.ts @@ -164,6 +164,51 @@ export type CanvasAnalyticsConfig = z.infer; export const canvasThemeSchema = z.enum(["light", "dark"]); export type CanvasTheme = z.infer; +// --------------------------------------------------------------------------- +// Annotation targets captured by the comment-mode overlay (structured canvas +// feedback). Deliberately LLM-locatable rather than machine-precise: the +// consumer is the generation agent locating the spot in source IT wrote, so +// the bounded text + stable attributes matter more than selector exactness. +// --------------------------------------------------------------------------- +export const canvasElementTargetSchema = z.object({ + type: z.literal("element"), + // Best-effort CSS selector for the rendered element (stable attributes + // preferred, nth-of-type fallback). + selector: z.string(), + tag: z.string(), + // Bounded innerText snippet — usually the strongest locator. + text: z.string(), + ariaLabel: z.string().nullable().optional(), + // The stable attributes present on the element (data-attr, id, role, …). + attributes: z.record(z.string(), z.string()), +}); +export type CanvasElementTarget = z.infer; + +export const canvasTextRangeTargetSchema = z.object({ + type: z.literal("text-range"), + // The selected text, bounded. + text: z.string(), + ancestorSelector: z.string(), + ancestorTag: z.string(), +}); +export type CanvasTextRangeTarget = z.infer; + +export const canvasAnnotationTargetSchema = z.discriminatedUnion("type", [ + canvasElementTargetSchema, + canvasTextRangeTargetSchema, +]); +export type CanvasAnnotationTarget = z.infer< + typeof canvasAnnotationTargetSchema +>; + +// A queued annotation the host asks the overlay to render as a numbered pin. +export const canvasAnnotationPinSchema = z.object({ + id: z.string(), + n: z.number(), + selector: z.string(), +}); +export type CanvasAnnotationPin = z.infer; + // host -> iframe export const hostToCanvasMessageSchema = z.discriminatedUnion("type", [ // First frame: hand the iframe its source + the run mode. The iframe does not @@ -199,6 +244,19 @@ export const hostToCanvasMessageSchema = z.discriminatedUnion("type", [ result: z.unknown().optional(), error: z.string().optional(), }), + // Toggle the annotation ("comment mode") overlay: crosshair cursor, hover + // highlight, and click-to-capture targets. + z.object({ + channel: z.literal(CANVAS_CHANNEL), + type: z.literal("set-annotation-mode"), + enabled: z.boolean(), + }), + // The queued annotations to render as numbered pins over their targets. + z.object({ + channel: z.literal(CANVAS_CHANNEL), + type: z.literal("annotation-pins"), + pins: z.array(canvasAnnotationPinSchema), + }), ]); export type HostToCanvasMessage = z.infer; @@ -254,5 +312,12 @@ export const canvasToHostMessageSchema = z.discriminatedUnion("type", [ type: z.literal("navigate"), nav: canvasNavIntentSchema, }), + // A comment-mode click captured an annotation target. The comment itself is + // typed host-side; only the target crosses the boundary. + z.object({ + channel: z.literal(CANVAS_CHANNEL), + type: z.literal("annotation-target"), + target: canvasAnnotationTargetSchema, + }), ]); export type CanvasToHostMessage = z.infer; diff --git a/packages/ui/src/features/canvas/freeform/CanvasFrameHost.tsx b/packages/ui/src/features/canvas/freeform/CanvasFrameHost.tsx index 56aaa920d9..45f6888cac 100644 --- a/packages/ui/src/features/canvas/freeform/CanvasFrameHost.tsx +++ b/packages/ui/src/features/canvas/freeform/CanvasFrameHost.tsx @@ -56,6 +56,9 @@ export function CanvasFrameHost() { onError={slot.inputs.onError} onRendered={slot.inputs.onRendered} onNavigate={slot.inputs.onNavigate} + annotationMode={slot.inputs.annotationMode} + annotationPins={slot.inputs.annotationPins} + onAnnotationTarget={slot.inputs.onAnnotationTarget} /> diff --git a/packages/ui/src/features/canvas/freeform/CanvasFramePlaceholder.tsx b/packages/ui/src/features/canvas/freeform/CanvasFramePlaceholder.tsx index fb56f90e44..b442739bfb 100644 --- a/packages/ui/src/features/canvas/freeform/CanvasFramePlaceholder.tsx +++ b/packages/ui/src/features/canvas/freeform/CanvasFramePlaceholder.tsx @@ -1,5 +1,7 @@ import type { CanvasAnalyticsConfig, + CanvasAnnotationPin, + CanvasAnnotationTarget, CanvasNavIntent, } from "@posthog/core/canvas/freeformSchemas"; import { useEffect, useLayoutEffect, useMemo, useRef } from "react"; @@ -19,6 +21,9 @@ export function CanvasFramePlaceholder({ onError, onRendered, onNavigate, + annotationMode, + annotationPins, + onAnnotationTarget, }: { dashboardId: string; code: string; @@ -27,6 +32,9 @@ export function CanvasFramePlaceholder({ onError?: (message: string, stack?: string) => void; onRendered?: () => void; onNavigate?: (intent: CanvasNavIntent) => void; + annotationMode?: boolean; + annotationPins?: CanvasAnnotationPin[]; + onAnnotationTarget?: (target: CanvasAnnotationTarget) => void; }) { const ref = useRef(null); const refreshKey = useCanvasRefreshNonce(`dashboard:${dashboardId}`); @@ -45,6 +53,9 @@ export function CanvasFramePlaceholder({ onError, onRendered, onNavigate, + annotationMode, + annotationPins, + onAnnotationTarget, }), [ code, @@ -54,6 +65,9 @@ export function CanvasFramePlaceholder({ onError, onRendered, onNavigate, + annotationMode, + annotationPins, + onAnnotationTarget, ], ); diff --git a/packages/ui/src/features/canvas/freeform/FreeformCanvas.tsx b/packages/ui/src/features/canvas/freeform/FreeformCanvas.tsx index 9a64218654..23578edb63 100644 --- a/packages/ui/src/features/canvas/freeform/FreeformCanvas.tsx +++ b/packages/ui/src/features/canvas/freeform/FreeformCanvas.tsx @@ -1,5 +1,7 @@ import { type CanvasAnalyticsConfig, + type CanvasAnnotationPin, + type CanvasAnnotationTarget, type CanvasNavIntent, type CanvasToHostMessage, canvasToHostMessageSchema, @@ -51,6 +53,12 @@ export interface FreeformCanvasProps { * reloads the frame via the existing reload path. */ refreshKey?: number; + /** Comment mode: the overlay captures element/text targets on click. */ + annotationMode?: boolean; + /** Queued annotations rendered as numbered pins over their targets. */ + annotationPins?: CanvasAnnotationPin[]; + /** Called when the overlay captures a target (the comment is typed host-side). */ + onAnnotationTarget?: (target: CanvasAnnotationTarget) => void; } // Renders a freeform-React canvas inside a null-origin sandboxed iframe and @@ -65,6 +73,9 @@ export function FreeformCanvas({ onNavigate, analytics, refreshKey = 0, + annotationMode = false, + annotationPins, + onAnnotationTarget, }: FreeformCanvasProps) { const iframeRef = useRef(null); // The canvas mirrors the host's light/dark theme. Passed via `init` (not the @@ -92,25 +103,32 @@ export function FreeformCanvas({ onError, onRendered, onNavigate, + onAnnotationTarget, code, mode, analytics, theme, + annotationMode, + annotationPins, }); latest.current = { onDataRequest, onError, onRendered, onNavigate, + onAnnotationTarget, code, mode, analytics, theme, + annotationMode, + annotationPins, }; const postInit = useCallback(() => { const p = latest.current; - iframeRef.current?.contentWindow?.postMessage( + const target = iframeRef.current?.contentWindow; + target?.postMessage( { channel: "posthog-canvas", type: "init", @@ -121,6 +139,24 @@ export function FreeformCanvas({ }, "*", ); + // A reload resets the overlay's state; re-sync comment mode + pins so they + // survive srcDoc changes exactly like the theme survives via init. + target?.postMessage( + { + channel: "posthog-canvas", + type: "set-annotation-mode", + enabled: p.annotationMode, + }, + "*", + ); + target?.postMessage( + { + channel: "posthog-canvas", + type: "annotation-pins", + pins: p.annotationPins ?? [], + }, + "*", + ); }, []); // The iframe reloads only when srcDoc changes (mode / analytics host); on @@ -182,6 +218,9 @@ export function FreeformCanvas({ // msg.nav is already allowlist-validated by safeParse below. latest.current.onNavigate?.(msg.nav); break; + case "annotation-target": + latest.current.onAnnotationTarget?.(msg.target); + break; } }; @@ -232,6 +271,30 @@ export function FreeformCanvas({ ); }, [theme]); + // Live comment-mode + pin updates (no remount; postInit re-syncs on reload). + useEffect(() => { + if (!readyRef.current) return; + iframeRef.current?.contentWindow?.postMessage( + { + channel: "posthog-canvas", + type: "set-annotation-mode", + enabled: annotationMode, + }, + "*", + ); + }, [annotationMode]); + useEffect(() => { + if (!readyRef.current) return; + iframeRef.current?.contentWindow?.postMessage( + { + channel: "posthog-canvas", + type: "annotation-pins", + pins: annotationPins ?? [], + }, + "*", + ); + }, [annotationPins]); + return (