From cd529c2edce7bd13f0e65ec8c98d01d7ce921faf Mon Sep 17 00:00:00 2001 From: "David W. Keith" Date: Wed, 12 Aug 2026 13:46:27 -0700 Subject: [PATCH] fix(#1422): persist image asset bytes with insert-image/replace-image-src MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit processImageDrop writes the optimized primary WebP + responsive variants under public/images/, but apply-edit-dispatcher.mjs's onApplied call only ever passed {file, range} for the patched .astro source — the asset bytes never made it into the anglesite/edits commit (or, for container-backed runtimes, the host repo it gets persisted onto). A "successful" image edit left the host with a source patch pointing at images that don't exist. Reuse the multi-file {files: [...]} onApplied shape extract-component already established, listing the source file plus every asset processImageDrop wrote, so recordEdit commits them all onto one commit. server/index-tools.mjs's onApplied wiring already branches on `files` generically, so no wiring change was needed there beyond the doc comment. Part of Anglesite/Anglesite#1422 (paired PR — the app PR consuming this release is tracked separately). Does not address that issue's other, unconfirmed defect: why the guest reply's commit can come back nil for insert-image in the first place. --- server/apply-edit-dispatcher.mjs | 31 +++++++- server/index-tools.mjs | 6 +- test/apply-edit-dispatcher.test.js | 120 ++++++++++++++++++++++++++++- 3 files changed, 150 insertions(+), 7 deletions(-) diff --git a/server/apply-edit-dispatcher.mjs b/server/apply-edit-dispatcher.mjs index 04ae2a4..fffcf7f 100644 --- a/server/apply-edit-dispatcher.mjs +++ b/server/apply-edit-dispatcher.mjs @@ -17,7 +17,10 @@ * a sibling temp file then `rename`), invoke an optional `onApplied` hook so #298's * `edit-history.mjs` can commit to the hidden `anglesite/edits` branch and thread its SHA * back as `commit`, then return `edit-applied` — with `result: {src, srcset}` for the - * image-drop path. + * image-drop path. Image ops pass `onApplied` the multi-file `{files: [...]}` shape (source + * patch + every optimized asset `processImageDrop` wrote) rather than the single-file + * `{file, range}` shape every other op uses, so the commit actually carries the bytes the + * patched `` references (#1422). * 5. On any filesystem error: return `edit-failed` with reason `write-failed`. * * The handler stays a pure async function so it's unit-testable independent of the MCP @@ -118,7 +121,14 @@ function mimeToExt(mime) { * Falls back to the dropped filename's stem when the target src is external * (http(s)://…) or otherwise can't be parsed to a /images/ path. * - * @returns {Promise<{ src: string, srcset: string }>} + * `assets` lists the optimized binaries' paths relative to `projectRoot` — the primary WebP + * plus every responsive variant. The caller threads these into `onApplied({files: [...]})` so + * the hidden `anglesite/edits` commit (and, for container-backed runtimes, the host repo it's + * persisted onto) actually carries the bytes the patched `` now references, not just the + * source-file patch (#1422 — a "successful" image edit previously committed a source file + * pointing at asset bytes that never made it to the host). + * + * @returns {Promise<{ src: string, srcset: string, assets: string[] }>} */ async function processImageDrop(projectRoot, edit) { const { selector, value } = edit; @@ -180,7 +190,11 @@ async function processImageDrop(projectRoot, edit) { const srcset = optimized.variants .map((v) => `/images/${v.file} ${v.width}w`) .join(", "); - return { src, srcset }; + const assets = [ + `public/images/${optimized.primary}`, + ...optimized.variants.map((v) => `public/images/${v.file}`), + ]; + return { src, srcset, assets }; } /** @@ -379,7 +393,16 @@ export async function applyEdit(projectRoot, edit, opts = {}) { let commit; if (opts.onApplied) { try { - commit = await opts.onApplied({ file, range, projectRoot }); + // Image ops (imageResult set) must commit the optimized binaries alongside the source + // patch — see processImageDrop's `assets` doc comment (#1422). Reuse extract-component's + // multi-file `{files: [...]}` shape rather than a third onApplied payload variant. + commit = imageResult + ? await opts.onApplied({ + files: [file, ...imageResult.assets], + projectRoot, + message: `anglesite: ${edit.op === "insert-image" ? "insert" : "replace"} image in ${file}`, + }) + : await opts.onApplied({ file, range, projectRoot }); } catch (err) { // Patch landed on disk but history-keeping failed. Surface as a successful apply with no // commit SHA — the user-visible source change is real; #298 can decide its own policy. diff --git a/server/index-tools.mjs b/server/index-tools.mjs index fab438c..0df1991 100644 --- a/server/index-tools.mjs +++ b/server/index-tools.mjs @@ -121,8 +121,10 @@ export function buildServer(projectRoot) { // invokes `onApplied` after a successful patch — `recordEdit` commits onto refs/heads/anglesite/edits // without touching HEAD/index/working-tree and returns the SHA, which the dispatcher threads // back as `commit` on the edit-applied response. `onApplied` is called with `{file, range}` for - // every single-file op, or `{files, message}` for extract-component's two-file write (Component - // Editor slice 5, Anglesite-app#495) — `recordEdit`'s `files` mode commits both onto ONE commit. + // every single-file op, or `{files, message}` for ops that touch more than one path on disk: + // extract-component's two-file write (Component Editor slice 5, Anglesite-app#495), and + // insert-image/replace-image-src's source patch + optimized asset bytes (Anglesite-app#1422) + // — `recordEdit`'s `files` mode commits every listed path onto ONE commit. server.registerTool( "apply_edit", { diff --git a/test/apply-edit-dispatcher.test.js b/test/apply-edit-dispatcher.test.js index 3faba3b..1a10494 100644 --- a/test/apply-edit-dispatcher.test.js +++ b/test/apply-edit-dispatcher.test.js @@ -2,9 +2,10 @@ import { describe, it, expect, beforeEach, afterEach } from "vitest"; import { mkdtempSync, cpSync, readFileSync, rmSync, chmodSync, statSync, mkdirSync, writeFileSync, existsSync } from "node:fs"; import { tmpdir } from "node:os"; import { join, resolve as resolvePath } from "node:path"; -import { execSync } from "node:child_process"; +import { execSync, execFileSync } from "node:child_process"; import sharp from "sharp"; import { applyEdit } from "../server/apply-edit-dispatcher.mjs"; +import { recordEdit } from "../server/edit-history.mjs"; const FIXTURE = resolvePath(import.meta.dirname, "fixtures/patcher"); let root; @@ -24,6 +25,15 @@ function parseContent(response) { return JSON.parse(response.content[0].text); } +// Mirrors server/index-tools.mjs's production `onApplied` wiring: single-file ops record +// `{file, range}`, multi-file ops (extract-component, and image ops per #1422) record `{files}`. +function wireOnApplied(projectRoot) { + return ({ file, range, files, message }) => + files + ? recordEdit(projectRoot, { files, message }) + : recordEdit(projectRoot, { file, range, message: `anglesite: edit ${file}` }); +} + beforeEach(() => { root = mkdtempSync(join(tmpdir(), "dispatcher-")); cpSync(FIXTURE, root, { recursive: true }); @@ -228,6 +238,64 @@ describe("replace-image-src", () => { expect(preserved.width).toBe(100); }); + it("commits the optimized image assets alongside the source patch, not just the source patch (#1422)", async () => { + mkdirSync(join(projectRoot, "src/pages"), { recursive: true }); + mkdirSync(join(projectRoot, "public/images"), { recursive: true }); + writeFileSync( + join(projectRoot, "src/pages/about.astro"), + `Hero`, + ); + await sharp({ create: { width: 100, height: 100, channels: 3, background: { r: 0, g: 0, b: 255 } } }) + .jpeg() + .toFile(join(projectRoot, "public/images/hero.jpg")); + execSync("git add .", { cwd: projectRoot }); + execSync("git commit -q -m fixture", { cwd: projectRoot }); + + const dropped = await sharp({ create: { width: 2000, height: 1500, channels: 3, background: { r: 255, g: 128, b: 0 } } }) + .jpeg() + .toBuffer(); + const dataURL = `data:image/jpeg;base64,${dropped.toString("base64")}`; + + let captured; + const result = await applyEdit( + projectRoot, + { + id: "e-img-commit", + path: "/about/", + selector: { tag: "IMG", classes: [], nthChild: 1, textContent: "/images/hero.jpg" }, + op: "replace-image-src", + value: { filename: "vacation.jpg", mimeType: "image/jpeg", dataURL }, + }, + { + onApplied: (info) => { + captured = info; + return wireOnApplied(projectRoot)(info); + }, + }, + ); + + // onApplied got the multi-file shape, not the single-file {file, range} shape — that's the + // bug: passing {file, range} silently drops the asset bytes from the commit. + expect(captured.file).toBeUndefined(); + expect(captured.files).toEqual( + expect.arrayContaining(["src/pages/about.astro", "public/images/hero.webp", "public/images/hero-480w.webp"]), + ); + + const reply = JSON.parse(result.content[0].text); + expect(reply.commit).toBeTruthy(); + + // The commit's tree actually carries the asset bytes, not just the source patch — verifying + // via `git show`, not just the in-memory `files` array, in case recordEdit's tree-building + // silently dropped one. + const committedFiles = execFileSync("git", ["ls-tree", "-r", "--name-only", reply.commit], { + cwd: projectRoot, + encoding: "utf-8", + }).split("\n"); + expect(committedFiles).toEqual( + expect.arrayContaining(["src/pages/about.astro", "public/images/hero.webp", "public/images/hero-480w.webp"]), + ); + }); + it("falls back to dropped filename when target src is external", async () => { mkdirSync(join(projectRoot, "src/pages"), { recursive: true }); mkdirSync(join(projectRoot, "public/images"), { recursive: true }); @@ -334,6 +402,56 @@ describe("insert-image", () => { expect(existsSync(join(projectRoot, "public/images/garden.webp"))).toBe(true); }); + it("commits the optimized image assets alongside the source patch, not just the source patch (#1422)", async () => { + writeFileSync( + join(projectRoot, "src/pages/index.astro"), + `---\nimport BaseLayout from "../layouts/BaseLayout.astro";\n---\n\n\n

Welcome

\n\n`, + ); + execSync("git init -q -b main", { cwd: projectRoot }); + execSync("git config user.email test@example.com", { cwd: projectRoot }); + execSync("git config user.name Test", { cwd: projectRoot }); + execSync("git add .", { cwd: projectRoot }); + execSync("git commit -q -m fixture", { cwd: projectRoot }); + + const dropped = await sharp({ create: { width: 2000, height: 1500, channels: 3, background: { r: 10, g: 200, b: 10 } } }) + .jpeg() + .toBuffer(); + const dataURL = `data:image/jpeg;base64,${dropped.toString("base64")}`; + + let captured; + const result = await applyEdit( + projectRoot, + { + id: "e-insert-commit", + path: "/", + op: "insert-image", + value: { filename: "garden.jpg", mimeType: "image/jpeg", dataURL }, + }, + { + onApplied: (info) => { + captured = info; + return wireOnApplied(projectRoot)(info); + }, + }, + ); + + expect(captured.file).toBeUndefined(); + expect(captured.files).toEqual( + expect.arrayContaining(["src/pages/index.astro", "public/images/garden.webp", "public/images/garden-480w.webp"]), + ); + + const reply = JSON.parse(result.content[0].text); + expect(reply.commit).toBeTruthy(); + + const committedFiles = execFileSync("git", ["ls-tree", "-r", "--name-only", reply.commit], { + cwd: projectRoot, + encoding: "utf-8", + }).split("\n"); + expect(committedFiles).toEqual( + expect.arrayContaining(["src/pages/index.astro", "public/images/garden.webp", "public/images/garden-480w.webp"]), + ); + }); + it("uses the dropped filename's stem — there is no existing image to derive one from", async () => { writeFileSync( join(projectRoot, "src/pages/index.astro"),