Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 27 additions & 4 deletions server/apply-edit-dispatcher.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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 `<img>` 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
Expand Down Expand Up @@ -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 `<img>` 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;
Expand Down Expand Up @@ -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 };
}

/**
Expand Down Expand Up @@ -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.
Expand Down
6 changes: 4 additions & 2 deletions server/index-tools.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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",
{
Expand Down
120 changes: 119 additions & 1 deletion test/apply-edit-dispatcher.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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 });
Expand Down Expand Up @@ -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"),
`<img src="/images/hero.jpg" alt="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 });
Expand Down Expand Up @@ -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<BaseLayout title="Home">\n <h1>Welcome</h1>\n</BaseLayout>\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"),
Expand Down
Loading