fix(FN-7143): address image artifact review feedback
- classify image artifact support as a minor feature changeset - reject non-image, empty, and signature-mismatched dataBase64 payloads - trap keyboard focus in the task image lightbox - assert media streaming with raw image bytes Fusion-Task-Id: FN-7143 Co-authored-by: Fusion (runfusion.ai) <noreply@runfusion.ai>
This commit is contained in:
@@ -1,7 +1,7 @@
|
||||
---
|
||||
"@runfusion/fusion": patch
|
||||
"@runfusion/fusion": minor
|
||||
---
|
||||
|
||||
summary: Allow task image artifacts to be created from agent tools and viewed in task details.
|
||||
category: fix
|
||||
category: feature
|
||||
dev: Adds `dataBase64` support to `fn_artifact_register` and task-detail image preview expansion.
|
||||
|
||||
@@ -128,6 +128,7 @@ export function TaskDocumentsTab({
|
||||
* Task detail image artifacts must be viewable in-place from the task modal. Keep the expand target image-only so document, audio, video, and generic cards retain their current non-lightbox behavior without empty controls.
|
||||
*/
|
||||
const [lightboxArtifact, setLightboxArtifact] = useState<ArtifactWithTask | null>(null);
|
||||
const lightboxDialogRef = useRef<HTMLDivElement>(null);
|
||||
const lightboxCloseRef = useRef<HTMLButtonElement>(null);
|
||||
const lightboxReturnFocusRef = useRef<HTMLElement | null>(null);
|
||||
const { artifacts, loading: artifactsLoading, error: artifactsError } = useArtifacts({ projectId, taskId });
|
||||
@@ -310,6 +311,40 @@ export function TaskDocumentsTab({
|
||||
if (event.key === "Escape") {
|
||||
event.preventDefault();
|
||||
handleCloseLightbox();
|
||||
return;
|
||||
}
|
||||
|
||||
if (event.key !== "Tab") {
|
||||
return;
|
||||
}
|
||||
|
||||
/*
|
||||
* FNXC:ArtifactRegistry 2026-06-29-17:08:
|
||||
* The artifact preview declares an aria-modal dialog, so keyboard focus must stay inside the lightbox until Escape, overlay click, or the close button dismisses it. Cycle Tab/Shift+Tab over current focusable controls instead of letting focus escape into the task-detail modal behind the overlay.
|
||||
*/
|
||||
const dialog = lightboxDialogRef.current;
|
||||
const focusableElements = Array.from(dialog?.querySelectorAll<HTMLElement>(
|
||||
'a[href], button:not([disabled]), textarea:not([disabled]), input:not([disabled]), select:not([disabled]), [tabindex]:not([tabindex="-1"])',
|
||||
) ?? []).filter((element) => !element.hasAttribute("disabled") && element.getAttribute("aria-hidden") !== "true");
|
||||
|
||||
if (!dialog || focusableElements.length === 0) {
|
||||
event.preventDefault();
|
||||
return;
|
||||
}
|
||||
|
||||
const firstElement = focusableElements[0];
|
||||
const lastElement = focusableElements[focusableElements.length - 1];
|
||||
const activeElement = document.activeElement;
|
||||
|
||||
if (event.shiftKey && activeElement === firstElement) {
|
||||
event.preventDefault();
|
||||
lastElement.focus();
|
||||
} else if (!event.shiftKey && activeElement === lastElement) {
|
||||
event.preventDefault();
|
||||
firstElement.focus();
|
||||
} else if (!dialog.contains(activeElement)) {
|
||||
event.preventDefault();
|
||||
firstElement.focus();
|
||||
}
|
||||
};
|
||||
|
||||
@@ -612,6 +647,7 @@ export function TaskDocumentsTab({
|
||||
|
||||
{lightboxArtifact && (
|
||||
<div
|
||||
ref={lightboxDialogRef}
|
||||
className="modal-overlay open documents-artifact-lightbox-overlay"
|
||||
role="dialog"
|
||||
aria-modal="true"
|
||||
|
||||
@@ -252,6 +252,31 @@ describe("TaskDocumentsTab", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("traps keyboard focus inside the image artifact lightbox", async () => {
|
||||
mockUseArtifacts.mockReturnValue({
|
||||
artifacts: mockArtifacts,
|
||||
loading: false,
|
||||
error: null,
|
||||
refresh: vi.fn().mockResolvedValue(undefined),
|
||||
});
|
||||
|
||||
render(<TaskDocumentsTab taskId="KB-001" addToast={addToast} projectId="project-1" />);
|
||||
|
||||
fireEvent.click(await screen.findByRole("button", { name: "Expand image artifact Image artifact" }));
|
||||
const closeButton = screen.getByRole("button", { name: "Close artifact preview" });
|
||||
expect(closeButton).toHaveFocus();
|
||||
|
||||
fireEvent.keyDown(document, { key: "Tab" });
|
||||
expect(closeButton).toHaveFocus();
|
||||
|
||||
fireEvent.keyDown(document, { key: "Tab", shiftKey: true });
|
||||
expect(closeButton).toHaveFocus();
|
||||
|
||||
screen.getAllByRole("button", { name: "Expand" })[0].focus();
|
||||
fireEvent.keyDown(document, { key: "Tab" });
|
||||
expect(closeButton).toHaveFocus();
|
||||
});
|
||||
|
||||
it("surfaces artifact fetch errors", async () => {
|
||||
mockUseArtifacts.mockReturnValue({
|
||||
artifacts: [],
|
||||
|
||||
@@ -3,6 +3,7 @@
|
||||
import { describe, it, expect, beforeEach, afterEach } from "vitest";
|
||||
import express from "express";
|
||||
import { mkdtempSync, rmSync } from "node:fs";
|
||||
import http from "node:http";
|
||||
import { tmpdir } from "node:os";
|
||||
import { join } from "node:path";
|
||||
import { TaskStore, type ArtifactWithTask } from "@fusion/core";
|
||||
@@ -41,7 +42,7 @@ describe("artifacts route integration", () => {
|
||||
title: "Render screenshot",
|
||||
description: "Capture dashboard artifact rendering evidence",
|
||||
});
|
||||
const imageBytes = Buffer.from("PNG-FN-7125-route-integration-image-bytes", "utf8");
|
||||
const imageBytes = Buffer.from("iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mP8/x8AAwMCAO+/p9sAAAAASUVORK5CYII=", "base64");
|
||||
const artifact = await store.registerArtifact({
|
||||
type: "image",
|
||||
title: "Dashboard screenshot",
|
||||
@@ -54,6 +55,36 @@ describe("artifacts route integration", () => {
|
||||
return { task, artifact, imageBytes };
|
||||
}
|
||||
|
||||
async function requestRawBuffer(app: express.Express, path: string) {
|
||||
/*
|
||||
* FNXC:ArtifactRegistry 2026-06-29-17:11:
|
||||
* Media route verification must compare the raw streamed bytes, not a UTF-8 string re-encoding, so binary image corruption fails the integration test.
|
||||
*/
|
||||
const server = http.createServer(app);
|
||||
return await new Promise<{ status: number; headers: http.IncomingHttpHeaders; body: Buffer }>((resolve, reject) => {
|
||||
server.listen(0, "127.0.0.1", () => {
|
||||
const address = server.address();
|
||||
if (!address || typeof address === "string") {
|
||||
reject(new Error("Expected an ephemeral TCP address for raw media request"));
|
||||
return;
|
||||
}
|
||||
|
||||
const req = http.get({ host: "127.0.0.1", port: address.port, path }, (res) => {
|
||||
const chunks: Buffer[] = [];
|
||||
res.on("data", (chunk: Buffer) => chunks.push(chunk));
|
||||
res.on("end", () => {
|
||||
server.close();
|
||||
resolve({ status: res.statusCode ?? 0, headers: res.headers, body: Buffer.concat(chunks) });
|
||||
});
|
||||
});
|
||||
req.on("error", (error) => {
|
||||
server.close();
|
||||
reject(error);
|
||||
});
|
||||
});
|
||||
});
|
||||
}
|
||||
|
||||
it("an image artifact created on a task appears in the artifacts listing with task association", async () => {
|
||||
const { task, artifact } = await createTaskImageArtifact();
|
||||
|
||||
@@ -77,15 +108,15 @@ describe("artifacts route integration", () => {
|
||||
it("the image artifact streams its real bytes with the correct content type", async () => {
|
||||
const { artifact, imageBytes } = await createTaskImageArtifact();
|
||||
|
||||
const res = await REQUEST(app, "GET", `/api/artifacts/${artifact.id}/media`);
|
||||
const res = await requestRawBuffer(app, `/api/artifacts/${artifact.id}/media`);
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.headers["content-type"]).toBe("image/png");
|
||||
expect(Buffer.from(res.body as string, "utf8")).toEqual(imageBytes);
|
||||
expect(res.body).toEqual(imageBytes);
|
||||
});
|
||||
|
||||
it("a global image artifact still streams from the managed global artifacts directory", async () => {
|
||||
const imageBytes = Buffer.from("PNG-FN-7143-global-image-bytes", "utf8");
|
||||
const imageBytes = Buffer.from("iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mP8/x8AAwMCAO+/p9sAAAAASUVORK5CYII=", "base64");
|
||||
const artifact = await store.registerArtifact({
|
||||
type: "image",
|
||||
title: "Global screenshot",
|
||||
@@ -95,11 +126,11 @@ describe("artifacts route integration", () => {
|
||||
authorType: "agent",
|
||||
});
|
||||
|
||||
const res = await REQUEST(app, "GET", `/api/artifacts/${artifact.id}/media`);
|
||||
const res = await requestRawBuffer(app, `/api/artifacts/${artifact.id}/media`);
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.headers["content-type"]).toBe("image/png");
|
||||
expect(Buffer.from(res.body as string, "utf8")).toEqual(imageBytes);
|
||||
expect(res.body).toEqual(imageBytes);
|
||||
});
|
||||
|
||||
it("a URI-only image artifact whose file is missing still returns 404", async () => {
|
||||
|
||||
@@ -15,6 +15,7 @@ vi.mock("@fusion/core", async (importOriginal) => {
|
||||
|
||||
const TASK_ID = "FN-6778";
|
||||
const AUTHOR_ID = "agent-007";
|
||||
const PNG_IMAGE_BYTES = Buffer.from("iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mP8/x8AAwMCAO+/p9sAAAAASUVORK5CYII=", "base64");
|
||||
|
||||
type ArtifactStore = Pick<TaskStore, "registerArtifact" | "getArtifact" | "listArtifacts">;
|
||||
|
||||
@@ -91,7 +92,6 @@ describe("artifact register tool", () => {
|
||||
|
||||
it("decodes base64 image bytes before calling store.registerArtifact", async () => {
|
||||
const { store, registerArtifact } = createMockStore();
|
||||
const imageBytes = Buffer.from("small-png-bytes");
|
||||
registerArtifact.mockResolvedValue(createMockArtifact({
|
||||
id: "art-image",
|
||||
type: "image",
|
||||
@@ -106,7 +106,7 @@ describe("artifact register tool", () => {
|
||||
type: "image",
|
||||
title: "Screenshot",
|
||||
mimeType: "image/png",
|
||||
dataBase64: imageBytes.toString("base64"),
|
||||
dataBase64: PNG_IMAGE_BYTES.toString("base64"),
|
||||
taskId: TASK_ID,
|
||||
});
|
||||
|
||||
@@ -117,12 +117,44 @@ describe("artifact register tool", () => {
|
||||
taskId: TASK_ID,
|
||||
content: undefined,
|
||||
uri: undefined,
|
||||
data: imageBytes,
|
||||
data: PNG_IMAGE_BYTES,
|
||||
}));
|
||||
expect(getText(result)).toContain("Registered artifact");
|
||||
expect(getText(result)).not.toContain("ERROR:");
|
||||
});
|
||||
|
||||
it("rejects empty, non-image, and arbitrary-byte base64 payloads without registering", async () => {
|
||||
const { store, registerArtifact } = createMockStore();
|
||||
const tool = createArtifactRegisterTool(store, AUTHOR_ID);
|
||||
|
||||
const emptyResult = await runTool(tool, "call-empty-image", {
|
||||
type: "image",
|
||||
title: "Empty screenshot",
|
||||
mimeType: "image/png",
|
||||
dataBase64: " ",
|
||||
taskId: TASK_ID,
|
||||
});
|
||||
const documentResult = await runTool(tool, "call-document-base64", {
|
||||
type: "document",
|
||||
title: "Document bytes",
|
||||
mimeType: "text/plain",
|
||||
dataBase64: PNG_IMAGE_BYTES.toString("base64"),
|
||||
taskId: TASK_ID,
|
||||
});
|
||||
const arbitraryBytesResult = await runTool(tool, "call-arbitrary-image", {
|
||||
type: "image",
|
||||
title: "Text pretending to be PNG",
|
||||
mimeType: "image/png",
|
||||
dataBase64: Buffer.from("not-a-real-png").toString("base64"),
|
||||
taskId: TASK_ID,
|
||||
});
|
||||
|
||||
expect(registerArtifact).not.toHaveBeenCalled();
|
||||
expect(getText(emptyResult)).toContain("dataBase64 must decode to non-empty artifact bytes");
|
||||
expect(getText(documentResult)).toContain("dataBase64 is only supported for image artifacts");
|
||||
expect(getText(arbitraryBytesResult)).toContain("dataBase64 must decode to valid image bytes matching mimeType");
|
||||
});
|
||||
|
||||
it("returns an ERROR response without registering malformed base64 image payloads", async () => {
|
||||
const { store, registerArtifact } = createMockStore();
|
||||
|
||||
@@ -147,7 +179,7 @@ describe("artifact register tool", () => {
|
||||
const result = await runTool(tool, "call-register-missing-mime", {
|
||||
type: "image",
|
||||
title: "No MIME screenshot",
|
||||
dataBase64: Buffer.from("bytes").toString("base64"),
|
||||
dataBase64: PNG_IMAGE_BYTES.toString("base64"),
|
||||
taskId: TASK_ID,
|
||||
});
|
||||
|
||||
@@ -412,7 +444,6 @@ describe("chat artifact tools", () => {
|
||||
|
||||
it("registers with explicit task_id, decoded image bytes, and fixed dashboard-chat author", async () => {
|
||||
const { store, registerArtifact } = createMockStore();
|
||||
const imageBytes = Buffer.from("chat-image-bytes");
|
||||
registerArtifact.mockResolvedValue(createMockArtifact({
|
||||
id: "art-chat-image",
|
||||
authorId: "dashboard-chat",
|
||||
@@ -431,7 +462,7 @@ describe("chat artifact tools", () => {
|
||||
type: "image",
|
||||
title: "Chat screenshot",
|
||||
mimeType: "image/png",
|
||||
dataBase64: imageBytes.toString("base64"),
|
||||
dataBase64: PNG_IMAGE_BYTES.toString("base64"),
|
||||
});
|
||||
|
||||
expect(registerArtifact).toHaveBeenCalledWith(expect.objectContaining({
|
||||
@@ -439,7 +470,7 @@ describe("chat artifact tools", () => {
|
||||
authorId: "dashboard-chat",
|
||||
authorType: "agent",
|
||||
title: "Chat screenshot",
|
||||
data: imageBytes,
|
||||
data: PNG_IMAGE_BYTES,
|
||||
}));
|
||||
expect(getText(result)).toContain("Registered artifact");
|
||||
});
|
||||
|
||||
@@ -1481,18 +1481,30 @@ async function registerArtifactForAgent(
|
||||
/**
|
||||
* FNXC:ArtifactRegistry 2026-06-29-00:00:
|
||||
* Agents need a portable way to create task-scoped image artifacts without reading arbitrary local files. `dataBase64` decodes inside the tool and then uses TaskStore's existing binary persistence path so registry rows continue to store only managed artifact URIs.
|
||||
*
|
||||
* FNXC:ArtifactRegistry 2026-06-29-17:05:
|
||||
* `dataBase64` is an image-only payload source. Reject empty, non-image, and signature-mismatched bytes early so agents get actionable tool errors instead of persisting artifacts the dashboard cannot preview.
|
||||
*/
|
||||
function decodeArtifactDataBase64(params: Static<typeof artifactRegisterParams>): Buffer | undefined {
|
||||
const encoded = params.dataBase64?.trim();
|
||||
if (!encoded) {
|
||||
if (params.dataBase64 === undefined) {
|
||||
return undefined;
|
||||
}
|
||||
|
||||
const encoded = params.dataBase64.trim();
|
||||
if (encoded.length === 0) {
|
||||
throw new Error("dataBase64 must decode to non-empty artifact bytes.");
|
||||
}
|
||||
|
||||
if (params.uri || params.content) {
|
||||
throw new Error("dataBase64 cannot be combined with uri or content; provide exactly one artifact payload source.");
|
||||
}
|
||||
|
||||
if (params.type === "image" && !params.mimeType?.startsWith("image/")) {
|
||||
if (params.type !== "image") {
|
||||
throw new Error("dataBase64 is only supported for image artifacts; use uri or content for other types.");
|
||||
}
|
||||
|
||||
const mimeType = params.mimeType?.toLowerCase().split(";", 1)[0];
|
||||
if (!mimeType || !mimeType.startsWith("image/")) {
|
||||
throw new Error("image artifacts registered with dataBase64 require an image/* mimeType such as image/png.");
|
||||
}
|
||||
|
||||
@@ -1506,9 +1518,36 @@ function decodeArtifactDataBase64(params: Static<typeof artifactRegisterParams>)
|
||||
throw new Error("dataBase64 must decode to non-empty artifact bytes.");
|
||||
}
|
||||
|
||||
if (!hasImageSignature(data, mimeType)) {
|
||||
throw new Error("dataBase64 must decode to valid image bytes matching mimeType.");
|
||||
}
|
||||
|
||||
return data;
|
||||
}
|
||||
|
||||
function hasImageSignature(data: Buffer, mimeType: string): boolean {
|
||||
if (mimeType === "image/png") {
|
||||
return data.subarray(0, 8).equals(Buffer.from("89504e470d0a1a0a", "hex"));
|
||||
}
|
||||
|
||||
if (mimeType === "image/jpeg") {
|
||||
return data.length >= 3 && data[0] === 0xff && data[1] === 0xd8 && data[2] === 0xff;
|
||||
}
|
||||
|
||||
if (mimeType === "image/gif") {
|
||||
const header = data.subarray(0, 6).toString("ascii");
|
||||
return header === "GIF87a" || header === "GIF89a";
|
||||
}
|
||||
|
||||
if (mimeType === "image/webp") {
|
||||
return data.length >= 12
|
||||
&& data.subarray(0, 4).toString("ascii") === "RIFF"
|
||||
&& data.subarray(8, 12).toString("ascii") === "WEBP";
|
||||
}
|
||||
|
||||
return false;
|
||||
}
|
||||
|
||||
function notifyArtifactRegistered(messageStore: MessageStore | undefined, artifact: Artifact, authorId: string): void {
|
||||
if (!messageStore) return;
|
||||
|
||||
|
||||
Reference in New Issue
Block a user