Address PR review feedback (#1996)
- contain fn_artifact_register path payloads: realpath-canonicalized containment before stat/read — relative paths require and must stay inside baseDir, absolute paths allowed only under baseDir or the OS temp dir (deliberate allowance for browser/screenshot tooling); the process.cwd() fallback is gone, symlink escapes rejected - bind task-scoped heartbeat artifact registration to the acquired worktree (baseDir: sessionCwd rebind after acquisition); no-task heartbeat prompt now says to pass absolute temp-dir paths - enforce exactly-one payload source (content/uri/dataBase64/path); content+uri combos are now rejected to match the documented contract - add FNXC rationale comments at both visual-artifact instruction sites in the planning prompts (sync contract with the executor prompt) - media route: statSync -> await stat from node:fs/promises - range tests ride the in-memory MockSocket harness (TestResponse gains binary-safe bodyBuffer; real-TCP helper deleted) and assert the full 206 Content-Range/Content-Length contract for every range form - add PdfViewer coverage (iframe src/title) in DocumentsView tests Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
gsxdsm
@@ -44,7 +44,7 @@ fn chat <agent-id> [message…] [--once] [--non-interactive] [--poll-ms <n>]
|
||||
|
||||
Artifact tools operate on the shared artifact registry, so artifacts are visible across agents and tasks when the caller has the artifact ID or can discover it through filters.
|
||||
|
||||
- `fn_artifact_register` registers a `document`, `image`, `video`, `audio`, or `other` artifact with `title`, optional `description`, optional `mimeType`, and exactly one payload source: inline text `content`, a local file `path` (preferred for media the agent saved to disk — screenshots, wireframes, mockups, screen recordings, PDF exports; the file is copied into managed artifact storage with MIME inference and image/video/PDF signature validation), base64 `dataBase64` image bytes, or a `uri` reference for media stored elsewhere. HTML mockups register as `type="document"` + `mimeType="text/html"` (inline `content` or `path`) and render as live sandboxed previews in the Artifacts view; PDFs (`mimeType="application/pdf"`, `path`) open in an embedded viewer; videos stream with range-request seeking. Executor sessions resolve relative `path` values against the task worktree and default `taskId` to the executing task; task-scoped heartbeat sessions default `taskId` to the assigned task; dashboard chat uses the `dashboard-chat` author and requires `task_id` because chat has no ambient task.
|
||||
- `fn_artifact_register` registers a `document`, `image`, `video`, `audio`, or `other` artifact with `title`, optional `description`, optional `mimeType`, and exactly one payload source: inline text `content`, a local file `path` (preferred for media the agent saved to disk — screenshots, wireframes, mockups, screen recordings, PDF exports; the file is copied into managed artifact storage with MIME inference and image/video/PDF signature validation), base64 `dataBase64` image bytes, or a `uri` reference for media stored elsewhere. HTML mockups register as `type="document"` + `mimeType="text/html"` (inline `content` or `path`) and render as live sandboxed previews in the Artifacts view; PDFs (`mimeType="application/pdf"`, `path`) open in an embedded viewer; videos stream with range-request seeking. Executor sessions resolve relative `path` values against the task worktree and default `taskId` to the executing task; task-scoped heartbeat sessions resolve relative `path` values against the acquired heartbeat worktree and default `taskId` to the assigned task; dashboard chat uses the `dashboard-chat` author and requires `task_id` because chat has no ambient task. `path` values are containment-checked before any file read: the realpath-canonicalized file must stay inside the session's workspace directory (`baseDir`) or the OS temp directory (where browser/screenshot tooling writes captures); relative paths are rejected outright in lanes without a workspace directory (dashboard chat, no-task heartbeats), which are bounded to tmpdir-only absolute paths.
|
||||
- `fn_artifact_list` lists artifacts across agents and tasks with optional `type`, `authorId`, `taskId`, `search`, `limit`, and `offset` filters. Dashboard chat's scoped variant requires `task_id` and otherwise supports `type`, `authorId`, `search`, `limit`, and `offset` for that task.
|
||||
- `fn_artifact_view` fetches one artifact by `id`, returning registry metadata plus inline `content` when present or the stored `uri`/path reference for media artifacts.
|
||||
- Successful registration emits a best-effort `system` → `user` inbox notification to `DASHBOARD_USER_ID` with `artifactId`, `artifactType`, `title`, `authorId`, and optional `taskId` metadata. Notification delivery failures are logged and must never fail or roll back the artifact registration.
|
||||
|
||||
@@ -80,7 +80,7 @@
|
||||
- Inline text/document artifacts may store `content` directly in SQLite and therefore have no media file. The dashboard media route streams `GET /api/artifacts/:id/media` from disk when `uri` is present, accepting task-scoped artifact URIs under `artifacts/` and bridged image-attachment URIs under `attachments/`, or returns inline `content` with the persisted MIME type when no `uri` exists.
|
||||
- `getArtifact(id)` returns metadata by ID, `getArtifacts(taskId)` returns active-task artifacts newest-first, and `listArtifacts(...)` is the cross-agent query path with type/author/task/search filters and pagination. List reads hide artifacts whose parent task is soft-deleted while preserving task-less artifacts.
|
||||
- `updateArtifact(id, { title?, description?, content? })` powers the dashboard Artifacts view's in-place doc editing (`GET`/`PATCH /api/artifacts/:id`). Content edits are only allowed on inline-content rows (no `uri`); binary-backed rows accept metadata edits only, archived-task artifacts stay read-only, and successful updates emit `artifact:updated` for live gallery refresh.
|
||||
- `fn_artifact_register` accepts a local file `path` (in addition to inline `content`/`dataBase64`): the tool reads the file (50 MB cap), infers the MIME type from the extension when omitted, signature-validates image payloads (PNG/JPEG/GIF/WebP magic bytes, SVG text sniff), video payloads (mp4/mov `ftyp` box, WebM EBML header), and PDF payloads (`%PDF-` prefix), and persists the bytes through `registerArtifact()`'s managed storage path so the registry row keeps a servable URI after worktrees are cleaned up. Executor-lane registrations resolve relative paths against the task worktree and default `taskId` to the executing task. HTML mockups register as `type="document"` + `mimeType="text/html"` (via `content` or `path`) and render as live sandboxed previews in the Artifacts view.
|
||||
- `fn_artifact_register` accepts a local file `path` (in addition to inline `content`/`dataBase64`): the tool reads the file (50 MB cap), infers the MIME type from the extension when omitted, signature-validates image payloads (PNG/JPEG/GIF/WebP magic bytes, SVG text sniff), video payloads (mp4/mov `ftyp` box, WebM EBML header), and PDF payloads (`%PDF-` prefix), and persists the bytes through `registerArtifact()`'s managed storage path so the registry row keeps a servable URI after worktrees are cleaned up. Executor-lane registrations resolve relative paths against the task worktree and default `taskId` to the executing task. Every `path` is containment-checked before stat/read: the realpath-canonicalized file (symlinks and `../` segments resolved) must remain inside the session's `baseDir` or the OS temp directory; relative paths require a configured `baseDir`, and lanes without one (dashboard chat, no-task heartbeats) accept only absolute paths under the OS temp directory. HTML mockups register as `type="document"` + `mimeType="text/html"` (via `content` or `path`) and render as live sandboxed previews in the Artifacts view.
|
||||
- `GET /api/artifacts/:id/media` serves HTTP byte ranges (`Accept-Ranges: bytes`, 206 + `Content-Range` for single ranges, 416 for unsatisfiable ranges) so `<video>`/`<audio>` seeking works and Safari plays media at all.
|
||||
- Task-linked artifact registration requires an active, non-archived task. Archived tasks are read-only for artifact writes; soft-deleted or missing tasks are rejected.
|
||||
- Retention follows the existing task lifecycle rather than a separate artifact policy: soft-deleted parent tasks keep artifact rows/files for forensics but normal live-reader APIs hide them; hard deletion from the active `tasks` table cascades artifact metadata through the `taskId` foreign key, and archive cleanup removes the task directory that contains task-scoped artifact binaries. Task-less artifacts live under `<rootDir>/.fusion/artifacts/` and are not tied to task archival cleanup.
|
||||
|
||||
@@ -422,6 +422,12 @@ For bug-class/bug-fix tasks, add and fill in the exact \`## Symptom Verification
|
||||
|
||||
### Step {N}: Documentation & Delivery
|
||||
|
||||
<!--
|
||||
FNXC:ArtifactRegistry 2026-07-11-10:20:
|
||||
Agents historically never registered visual/media deliverables because nothing in the planning/spec layer required it — executors saved screenshots or mockups to disk and moved on, so the dashboard Artifacts gallery stayed empty.
|
||||
This checkbox is the planning-side half of the artifact-pipeline contract; the executor-side half lives in the "Artifact Registry" prompt section in packages/engine/src/executor.ts.
|
||||
The two must stay in sync on the supported types (images, videos, HTML mockups rendered as live previews, PDFs) and the fn_artifact_register(type=..., path=...) recipes.
|
||||
-->
|
||||
- [ ] Update relevant documentation
|
||||
- [ ] Save documentation deliverables as task documents via \`fn_task_document_write\` (key="docs", content=...)
|
||||
- [ ] For UI-visible changes or design deliverables: register screenshots/wireframes/mockups as image artifacts via \`fn_artifact_register(type="image", title=..., path="<saved file>")\`; screen recordings as \`type="video"\` with \`path\`; interactive HTML mockups as \`type="document"\` with \`mimeType="text/html"\` (rendered as live previews); PDF exports as \`type="document"\` with \`mimeType="application/pdf"\` and \`path\`
|
||||
@@ -579,6 +585,11 @@ For source-free forensic or spec-compliance tasks whose only deliverables are gi
|
||||
- Steps should express OUTCOMES, not micro-instructions (2-5 checkboxes per step)
|
||||
- Always include a testing step and a documentation step
|
||||
- For tasks whose primary deliverable is documentation (updating docs, writing README, API references), include an explicit step or checkbox instructing the executor to save the final documentation content via \`fn_task_document_write\`
|
||||
<!--
|
||||
FNXC:ArtifactRegistry 2026-07-11-10:22:
|
||||
Spec-authoring counterpart of the Documentation & Delivery artifact-registration checkbox above: specs for visual/media tasks must instruct the executor explicitly, because agents historically skipped registration when the planning layer never demanded it.
|
||||
This bullet is the planning-side half of the artifact-pipeline contract; keep it in sync with the executor-side "Artifact Registry" prompt section in packages/engine/src/executor.ts on supported types (images, videos, HTML mockups rendered as live previews, PDFs) and the fn_artifact_register(type=..., path=...) recipes.
|
||||
-->
|
||||
- For tasks with a visible UI surface or whose deliverable is visual/media (wireframes, mockups, designs, diagrams, screenshots, screen recordings, HTML prototypes, PDF exports), include an explicit step or checkbox instructing the executor to save each deliverable to disk and register it via \`fn_artifact_register\` (images via \`type="image", path=...\`; recordings via \`type="video", path=...\`; HTML mockups via \`type="document", mimeType="text/html"\` for live gallery previews; PDFs via \`type="document", mimeType="application/pdf", path=...\`) so it appears in the dashboard Artifacts gallery
|
||||
- Include a "Do NOT" section with project-appropriate guardrails
|
||||
- Size assessment: S (<{{triageSizeSmallMaxHours}}h), M ({{triageSizeSmallMaxHours}}-{{triageSizeMediumMaxHours}}h), L ({{triageSizeMediumMaxHours}}-{{triageSizeLargeMaxHours}}h). Split if XL ({{triageSizeLargeMaxHours}}h+)
|
||||
|
||||
@@ -675,6 +675,47 @@ describe("DocumentsView", () => {
|
||||
expect(within(dialog).getByText("<h1>Login mock</h1>")).toBeInTheDocument();
|
||||
});
|
||||
|
||||
/*
|
||||
FNXC:ArtifactsGallery 2026-07-10-12:30:
|
||||
PDF artifacts (application/pdf documents) must open a dedicated embedded PDF viewer that
|
||||
points its iframe at the artifact media route, not the inline doc viewer.
|
||||
*/
|
||||
it("renders PDF doc artifacts in the embedded PDF viewer iframe", async () => {
|
||||
const pdfArtifact: ArtifactWithTask = {
|
||||
id: "artifact-pdf",
|
||||
type: "document",
|
||||
title: "Spec export",
|
||||
mimeType: "application/pdf",
|
||||
uri: "artifacts/spec.pdf",
|
||||
authorId: "doc-agent",
|
||||
authorType: "agent",
|
||||
createdAt: "2026-04-19T09:45:00.000Z",
|
||||
updatedAt: "2026-04-19T09:45:00.000Z",
|
||||
};
|
||||
mockUseArtifacts.mockReturnValue({
|
||||
artifacts: [...mockArtifacts, pdfArtifact],
|
||||
loading: false,
|
||||
error: null,
|
||||
refresh: vi.fn().mockResolvedValue(undefined),
|
||||
});
|
||||
|
||||
render(<DocumentsView addToast={addToast} onOpenDetail={onOpenDetail} />);
|
||||
fireEvent.click(screen.getByRole("tab", { name: /show artifacts/i }));
|
||||
|
||||
const pdfCard = screen.getByRole("article", { name: "Artifact Spec export" });
|
||||
fireEvent.click(within(pdfCard).getByRole("button", { name: "Open Spec export" }));
|
||||
await screen.findByRole("dialog", { name: "PDF artifact viewer" });
|
||||
|
||||
// The FloatingWindow portals to document.body, so query the document rather than the render container.
|
||||
const iframe = document.querySelector(".artifacts-gallery-viewer-pdf");
|
||||
expect(iframe).toBeInTheDocument();
|
||||
expect(iframe).toHaveAttribute("src", "/api/artifacts/artifact-pdf/media");
|
||||
expect(iframe).toHaveAttribute("title", "Spec export");
|
||||
|
||||
fireEvent.keyDown(document, { key: "Escape" });
|
||||
expect(screen.queryByRole("dialog", { name: "PDF artifact viewer" })).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("clicking project file shows content", async () => {
|
||||
render(<DocumentsView addToast={addToast} onOpenDetail={onOpenDetail} />);
|
||||
// Landing tab is now Artifacts; these tests exercise the Project Files tab explicitly.
|
||||
|
||||
@@ -71,32 +71,6 @@ describe("artifacts route integration", () => {
|
||||
return { task, artifact, imageBytes };
|
||||
}
|
||||
|
||||
async function requestRawBufferWithHeaders(app: express.Express, path: string, headers: Record<string, string>) {
|
||||
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, headers }, (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);
|
||||
});
|
||||
});
|
||||
});
|
||||
}
|
||||
|
||||
async function requestRawBuffer(app: express.Express, path: string) {
|
||||
/*
|
||||
* FNXC:ArtifactRegistry 2026-06-29-17:11:
|
||||
@@ -602,6 +576,11 @@ describe("artifacts route integration", () => {
|
||||
* Video playback contract: the media route must serve HTTP byte ranges (206 + Content-Range +
|
||||
* Accept-Ranges) because <video> seeking issues Range requests and Safari refuses to play from
|
||||
* servers that ignore them. Unsatisfiable ranges answer 416.
|
||||
*
|
||||
* FNXC:ArtifactRegistry 2026-07-10-12:30:
|
||||
* Range assertions ride the in-memory test-request harness (TestResponse.bodyBuffer for exact
|
||||
* byte comparison) per the no-real-network testing rule; no real TCP server is started.
|
||||
* Every 206 variant asserts the full contract: status + Content-Range + Content-Length + bytes.
|
||||
*/
|
||||
it("serves byte-range requests for video artifact media", async () => {
|
||||
const task = await store.createTask({ title: "Demo recording", description: "range test" });
|
||||
@@ -615,27 +594,33 @@ describe("artifacts route integration", () => {
|
||||
authorType: "agent",
|
||||
taskId: task.id,
|
||||
});
|
||||
const mediaPath = `/api/artifacts/${artifact.id}/media`;
|
||||
|
||||
const full = await requestRawBuffer(app, `/api/artifacts/${artifact.id}/media`);
|
||||
const full = await REQUEST(app, "GET", mediaPath);
|
||||
expect(full.status).toBe(200);
|
||||
expect(full.headers["accept-ranges"]).toBe("bytes");
|
||||
expect(full.headers["content-length"]).toBe(String(videoBytes.length));
|
||||
expect(full.body).toEqual(videoBytes);
|
||||
expect(String(full.headers["content-length"])).toBe(String(videoBytes.length));
|
||||
expect(full.bodyBuffer).toEqual(videoBytes);
|
||||
|
||||
const ranged = await requestRawBufferWithHeaders(app, `/api/artifacts/${artifact.id}/media`, { Range: "bytes=4-11" });
|
||||
const ranged = await REQUEST(app, "GET", mediaPath, undefined, { Range: "bytes=4-11" });
|
||||
expect(ranged.status).toBe(206);
|
||||
expect(ranged.headers["content-range"]).toBe(`bytes 4-11/${videoBytes.length}`);
|
||||
expect(ranged.body).toEqual(videoBytes.subarray(4, 12));
|
||||
expect(String(ranged.headers["content-length"])).toBe("8");
|
||||
expect(ranged.bodyBuffer).toEqual(videoBytes.subarray(4, 12));
|
||||
|
||||
const suffix = await requestRawBufferWithHeaders(app, `/api/artifacts/${artifact.id}/media`, { Range: "bytes=-5" });
|
||||
const suffix = await REQUEST(app, "GET", mediaPath, undefined, { Range: "bytes=-5" });
|
||||
expect(suffix.status).toBe(206);
|
||||
expect(suffix.body).toEqual(videoBytes.subarray(videoBytes.length - 5));
|
||||
expect(suffix.headers["content-range"]).toBe(`bytes ${videoBytes.length - 5}-${videoBytes.length - 1}/${videoBytes.length}`);
|
||||
expect(String(suffix.headers["content-length"])).toBe("5");
|
||||
expect(suffix.bodyBuffer).toEqual(videoBytes.subarray(videoBytes.length - 5));
|
||||
|
||||
const openEnded = await requestRawBufferWithHeaders(app, `/api/artifacts/${artifact.id}/media`, { Range: `bytes=10-` });
|
||||
const openEnded = await REQUEST(app, "GET", mediaPath, undefined, { Range: `bytes=10-` });
|
||||
expect(openEnded.status).toBe(206);
|
||||
expect(openEnded.body).toEqual(videoBytes.subarray(10));
|
||||
expect(openEnded.headers["content-range"]).toBe(`bytes 10-${videoBytes.length - 1}/${videoBytes.length}`);
|
||||
expect(String(openEnded.headers["content-length"])).toBe(String(videoBytes.length - 10));
|
||||
expect(openEnded.bodyBuffer).toEqual(videoBytes.subarray(10));
|
||||
|
||||
const unsatisfiable = await requestRawBufferWithHeaders(app, `/api/artifacts/${artifact.id}/media`, { Range: `bytes=${videoBytes.length + 5}-` });
|
||||
const unsatisfiable = await REQUEST(app, "GET", mediaPath, undefined, { Range: `bytes=${videoBytes.length + 5}-` });
|
||||
expect(unsatisfiable.status).toBe(416);
|
||||
expect(unsatisfiable.headers["content-range"]).toBe(`bytes */${videoBytes.length}`);
|
||||
});
|
||||
@@ -655,9 +640,9 @@ describe("artifacts route integration", () => {
|
||||
authorType: "system",
|
||||
});
|
||||
|
||||
const media = await requestRawBuffer(app, `/api/artifacts/${bridged!.id}/media`);
|
||||
const media = await REQUEST(app, "GET", `/api/artifacts/${bridged!.id}/media`);
|
||||
expect(media.status).toBe(200);
|
||||
expect(media.headers["content-type"]).toBe("video/mp4");
|
||||
expect(media.body).toEqual(videoBytes);
|
||||
expect(media.bodyBuffer).toEqual(videoBytes);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import { createReadStream, statSync } from "node:fs";
|
||||
import { createReadStream } from "node:fs";
|
||||
import { stat } from "node:fs/promises";
|
||||
import { join, resolve, sep } from "node:path";
|
||||
import type {
|
||||
TaskStore,
|
||||
@@ -3848,7 +3849,8 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork
|
||||
*/
|
||||
let fileSize: number;
|
||||
try {
|
||||
fileSize = statSync(mediaPath).size;
|
||||
// FNXC:ArtifactRegistry 2026-07-10-00:00: use async stat so media/range requests never block the event loop.
|
||||
fileSize = (await stat(mediaPath)).size;
|
||||
} catch {
|
||||
throw notFound("Artifact media not found");
|
||||
}
|
||||
|
||||
@@ -5,6 +5,11 @@ import { PassThrough } from "node:stream";
|
||||
type TestResponse = {
|
||||
status: number;
|
||||
body: unknown;
|
||||
/*
|
||||
FNXC:TestHarness 2026-07-10-12:30:
|
||||
Raw response bytes concatenated before any UTF-8 decoding. Binary/media route tests (e.g. artifact byte-range serving) must compare exact bytes, and per the no-real-network testing rule they ride this in-memory harness instead of spinning up a real TCP server.
|
||||
*/
|
||||
bodyBuffer: Buffer;
|
||||
headers: http.OutgoingHttpHeaders;
|
||||
};
|
||||
|
||||
@@ -83,7 +88,8 @@ export async function request(
|
||||
|
||||
const response = new Promise<TestResponse>((resolve, reject) => {
|
||||
res.on("finish", () => {
|
||||
const rawBody = Buffer.concat(chunks).toString("utf8");
|
||||
const bodyBuffer = Buffer.concat(chunks);
|
||||
const rawBody = bodyBuffer.toString("utf8");
|
||||
const contentType = res.getHeader("content-type");
|
||||
const shouldParseJson = typeof contentType === "string" && contentType.includes("application/json");
|
||||
|
||||
@@ -91,6 +97,7 @@ export async function request(
|
||||
resolve({
|
||||
status: res.statusCode,
|
||||
body: shouldParseJson && rawBody.length > 0 ? JSON.parse(rawBody) : rawBody,
|
||||
bodyBuffer,
|
||||
headers: res.getHeaders(),
|
||||
});
|
||||
} catch (error) {
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs";
|
||||
import { mkdirSync, mkdtempSync, rmSync, symlinkSync, writeFileSync } from "node:fs";
|
||||
import { tmpdir } from "node:os";
|
||||
import { join } from "node:path";
|
||||
import type { Artifact, ArtifactType, ArtifactWithTask, MessageStore, TaskStore } from "@fusion/core";
|
||||
@@ -440,6 +440,123 @@ describe("artifact register tool path payloads", () => {
|
||||
expect(getText(result)).toContain("path cannot be combined with uri, content, or dataBase64");
|
||||
});
|
||||
|
||||
it("rejects combining content with uri", async () => {
|
||||
const { store, registerArtifact } = createMockStore();
|
||||
const tool = createArtifactRegisterTool(store, AUTHOR_ID, undefined, { baseDir });
|
||||
|
||||
const result = await runTool(tool, "call-content-uri-conflict", {
|
||||
type: "document",
|
||||
title: "Conflicting payloads",
|
||||
content: "# inline",
|
||||
uri: "https://example.com/doc.md",
|
||||
});
|
||||
|
||||
expect(registerArtifact).not.toHaveBeenCalled();
|
||||
expect(getText(result)).toContain("provide exactly one artifact payload source: content, uri, dataBase64, or path");
|
||||
});
|
||||
|
||||
/*
|
||||
FNXC:ArtifactRegistry 2026-07-11-10:05:
|
||||
Containment coverage: `path` must never read files outside the session workspace directory or
|
||||
the OS temp directory (realpath-canonicalized, so `../` and symlink escapes are caught), and
|
||||
no-baseDir lanes must reject relative paths instead of resolving against process.cwd().
|
||||
macOS note: tmpdir() is /var/folders/... which realpaths to /private/var/...; these tests rely
|
||||
on the implementation comparing canonical roots.
|
||||
*/
|
||||
it("rejects a relative path that escapes the baseDir via ../ segments", async () => {
|
||||
const { store, registerArtifact } = createMockStore();
|
||||
const outsideDir = mkdtempSync(join(tmpdir(), "agent-artifact-outside-"));
|
||||
try {
|
||||
writeFileSync(join(outsideDir, "escape.png"), PNG_IMAGE_BYTES);
|
||||
const tool = createArtifactRegisterTool(store, AUTHOR_ID, undefined, { baseDir, defaultTaskId: TASK_ID });
|
||||
|
||||
const result = await runTool(tool, "call-path-escape", {
|
||||
type: "image",
|
||||
title: "Escaped screenshot",
|
||||
path: join("..", outsideDir.split("/").pop()!, "escape.png"),
|
||||
});
|
||||
|
||||
expect(registerArtifact).not.toHaveBeenCalled();
|
||||
expect(getText(result)).toContain("escapes the session workspace directory");
|
||||
} finally {
|
||||
rmSync(outsideDir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it("rejects a symlink inside the baseDir that targets a file outside the allowed roots", async () => {
|
||||
const { store, registerArtifact } = createMockStore();
|
||||
const tool = createArtifactRegisterTool(store, AUTHOR_ID, undefined, { baseDir, defaultTaskId: TASK_ID });
|
||||
const outsideTarget = join(process.cwd(), "package.json");
|
||||
symlinkSync(outsideTarget, join(baseDir, "sneaky.json"));
|
||||
|
||||
const result = await runTool(tool, "call-symlink-escape", {
|
||||
type: "document",
|
||||
title: "Sneaky symlink",
|
||||
path: "sneaky.json",
|
||||
});
|
||||
|
||||
expect(registerArtifact).not.toHaveBeenCalled();
|
||||
expect(getText(result)).toContain("escapes the session workspace directory");
|
||||
});
|
||||
|
||||
it("rejects an absolute path outside both baseDir and the OS temp directory, naming the allowed roots", async () => {
|
||||
const { store, registerArtifact } = createMockStore();
|
||||
const tool = createArtifactRegisterTool(store, AUTHOR_ID, undefined, { baseDir, defaultTaskId: TASK_ID });
|
||||
// package.json of the engine package: exists, but lives outside tmpdir and outside baseDir.
|
||||
const outsideAbsolute = join(process.cwd(), "package.json");
|
||||
|
||||
const result = await runTool(tool, "call-absolute-outside", {
|
||||
type: "document",
|
||||
title: "Server file grab",
|
||||
path: outsideAbsolute,
|
||||
});
|
||||
|
||||
expect(registerArtifact).not.toHaveBeenCalled();
|
||||
expect(getText(result)).toContain("outside the allowed roots");
|
||||
expect(getText(result)).toContain("OS temp directory");
|
||||
});
|
||||
|
||||
it("rejects a relative path when no baseDir is configured instead of resolving against process.cwd()", async () => {
|
||||
const { store, registerArtifact } = createMockStore();
|
||||
const tool = createArtifactRegisterTool(store, AUTHOR_ID);
|
||||
|
||||
const result = await runTool(tool, "call-relative-no-basedir", {
|
||||
type: "image",
|
||||
title: "CWD-relative screenshot",
|
||||
path: "package.json",
|
||||
});
|
||||
|
||||
expect(registerArtifact).not.toHaveBeenCalled();
|
||||
expect(getText(result)).toContain("relative path requires a workspace directory");
|
||||
});
|
||||
|
||||
it("accepts an absolute path under the OS temp directory when no baseDir is configured", async () => {
|
||||
const { store, registerArtifact } = createMockStore();
|
||||
registerArtifact.mockResolvedValue(createMockArtifact({ id: "art-tmp", type: "image", mimeType: "image/png", content: undefined }));
|
||||
const captureDir = mkdtempSync(join(tmpdir(), "agent-artifact-capture-"));
|
||||
try {
|
||||
const capturePath = join(captureDir, "capture.png");
|
||||
writeFileSync(capturePath, PNG_IMAGE_BYTES);
|
||||
const tool = createArtifactRegisterTool(store, AUTHOR_ID);
|
||||
|
||||
const result = await runTool(tool, "call-absolute-tmpdir", {
|
||||
type: "image",
|
||||
title: "Temp-dir capture",
|
||||
path: capturePath,
|
||||
taskId: TASK_ID,
|
||||
});
|
||||
|
||||
expect(getText(result)).toContain("Registered artifact");
|
||||
expect(registerArtifact).toHaveBeenCalledWith(expect.objectContaining({
|
||||
type: "image",
|
||||
mimeType: "image/png",
|
||||
data: PNG_IMAGE_BYTES,
|
||||
}));
|
||||
} finally {
|
||||
rmSync(captureDir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it("registers video media from path with extension-inferred mimeType and container signature validation", async () => {
|
||||
const { store, registerArtifact } = createMockStore();
|
||||
registerArtifact.mockResolvedValue(createMockArtifact({ id: "art-video", type: "video", mimeType: "video/mp4", content: undefined, uri: "artifacts/demo.mp4" }));
|
||||
|
||||
@@ -544,7 +544,7 @@ You have coding-capable workspace tools (read/write/edit/bash within worktree bo
|
||||
- fn_list_agents and fn_delegate_task
|
||||
- fn_get_agent_config and fn_update_agent_config (for direct reports only)
|
||||
- fn_agent_create and fn_agent_delete (for direct reports only)
|
||||
- fn_artifact_register, fn_artifact_list, and fn_artifact_view (register visual/media outputs so they appear in the dashboard Artifacts gallery: screenshots/wireframes/mockups/diagrams as type="image" via \`path\`; screen recordings as type="video" via \`path\`; HTML mockups as type="document" with mimeType="text/html" — rendered as live previews; PDFs as type="document" with mimeType="application/pdf" via \`path\`)
|
||||
- fn_artifact_register, fn_artifact_list, and fn_artifact_view (register visual/media outputs so they appear in the dashboard Artifacts gallery: screenshots/wireframes/mockups/diagrams as type="image" via \`path\`; screen recordings as type="video" via \`path\`; HTML mockups as type="document" with mimeType="text/html" — rendered as live previews; PDFs as type="document" with mimeType="application/pdf" via \`path\`. No-task runs have no session workspace directory, so save files under the OS temp directory and pass an absolute \`path\` — relative paths are rejected in this mode)
|
||||
- fn_read_evaluations and fn_update_identity (available in no-task runs)
|
||||
- fn_reflect_on_performance when reflection is enabled for this run
|
||||
- fn_workflow_list, fn_workflow_get, fn_workflow_create, fn_workflow_update, fn_workflow_delete, fn_workflow_settings, and fn_trait_list for workflow discovery/authoring
|
||||
@@ -2790,6 +2790,21 @@ export class HeartbeatMonitor {
|
||||
}
|
||||
}
|
||||
|
||||
/*
|
||||
FNXC:ArtifactRegistry 2026-07-11-09:55:
|
||||
Task-scoped heartbeat tools are built before the worktree is acquired, so the initial
|
||||
fn_artifact_register binding has no baseDir and would reject relative artifact paths.
|
||||
Once the acquired worktree cwd is known, rebind the tool with `baseDir: sessionCwd` so
|
||||
relative `path` payloads resolve inside the heartbeat worktree (never process.cwd())
|
||||
and absolute paths are contained to that worktree or the OS temp directory.
|
||||
*/
|
||||
if (!isNoTaskRun) {
|
||||
const registerToolIndex = heartbeatTools.findIndex((tool) => tool.name === "fn_artifact_register");
|
||||
if (registerToolIndex >= 0) {
|
||||
heartbeatTools[registerToolIndex] = createArtifactRegisterTool(taskStore, agentId, this.messageStore, { baseDir: sessionCwd, defaultTaskId: taskId! });
|
||||
}
|
||||
}
|
||||
|
||||
const heartbeatSessionModels = resolveHeartbeatSessionModels(heartbeatModelSettings, agent.runtimeConfig);
|
||||
/*
|
||||
* FNXC:McpConfig 2026-06-26-00:00:
|
||||
|
||||
@@ -7,10 +7,11 @@
|
||||
* The parameter schemas are canonical here — executor.ts imports and reuses them.
|
||||
*/
|
||||
|
||||
import { appendFile, mkdir, readFile, readdir, stat, writeFile } from "node:fs/promises";
|
||||
import { appendFile, mkdir, readFile, readdir, realpath, stat, writeFile } from "node:fs/promises";
|
||||
import { existsSync } from "node:fs";
|
||||
import { createHash } from "node:crypto";
|
||||
import { extname, isAbsolute, join, relative, resolve } from "node:path";
|
||||
import { tmpdir } from "node:os";
|
||||
import { extname, isAbsolute, join, relative, resolve, sep } from "node:path";
|
||||
import * as fusionCore from "@fusion/core";
|
||||
import type { AgentState, AgentCapability, AgentUpdateInput, Artifact, ArtifactCreateInput, ArtifactWithTask, TaskDocument, TaskDocumentCreateInput, TaskStore, RunMutationContext, MessageStore, Message, SourceType, Settings, ResearchRun, ResearchRunStatus, TaskCreateInput, ReflectionStore, ApprovalRequestStore, ProjectSettings, ChatStore, WorkflowSettingDefinition, GoalStatus } from "@fusion/core";
|
||||
import { listTraits, isBuiltinWorkflowId, AgentStore, validateColumnAgentBindings, ColumnAgentBindingError, stripApprovalBypassFlags, WorkflowSettingRejectionError, resolveEffectiveSettingsById, resolveWorkflowIrById, findOrphanedSettingValues, BUILTIN_WORKFLOW_SETTINGS, MAX_TASK_LIST_TEXT_CHARS, formatCurrentTaskLine, normalizeWorkflowIcon } from "@fusion/core";
|
||||
@@ -1629,6 +1630,16 @@ async function registerArtifactForAgent(
|
||||
options?: ArtifactRegisterToolOptions,
|
||||
) {
|
||||
try {
|
||||
/*
|
||||
FNXC:ArtifactRegistry 2026-07-11-09:40:
|
||||
docs/agents.md promises "exactly one payload source" for fn_artifact_register. The path and
|
||||
dataBase64 readers already reject their own mixed combos with specific messages; this guard
|
||||
closes the remaining content+uri gap so both fields are never persisted on one artifact row.
|
||||
Zero payload sources stays allowed (metadata-only registrations are unchanged).
|
||||
*/
|
||||
if (params.content !== undefined && params.uri !== undefined) {
|
||||
throw new Error("content cannot be combined with uri; provide exactly one artifact payload source: content, uri, dataBase64, or path.");
|
||||
}
|
||||
const filePayload = await readArtifactFileFromPath(params, options?.baseDir);
|
||||
const data = filePayload ? filePayload.data : decodeArtifactDataBase64(params);
|
||||
const input: ArtifactCreateInput = {
|
||||
@@ -1716,11 +1727,57 @@ async function readArtifactFileFromPath(
|
||||
throw new Error("path cannot be combined with uri, content, or dataBase64; provide exactly one artifact payload source.");
|
||||
}
|
||||
|
||||
const resolvedPath = isAbsolute(rawPath) ? rawPath : resolve(baseDir ?? process.cwd(), rawPath);
|
||||
/*
|
||||
FNXC:ArtifactRegistry 2026-07-11-09:45:
|
||||
`path` reads server-side files, so it must be contained: an injected tool call must not be able to
|
||||
copy arbitrary readable server files (e.g. secrets, /etc files) into managed artifact storage.
|
||||
Containment rule (checked BEFORE stat/readFile, on realpath-canonicalized paths so symlinks and
|
||||
`../` segments cannot escape; macOS tmpdir /var/folders/... canonicalizes to /private/var/...):
|
||||
- Relative paths REQUIRE a configured session `baseDir` (executor/heartbeat worktree) and must
|
||||
canonicalize to inside it; without a baseDir they are rejected instead of silently resolving
|
||||
against process.cwd() (the server process directory).
|
||||
- Absolute paths are allowed only inside the canonical `baseDir` or the canonical OS temp
|
||||
directory. The tmpdir allowance is deliberate: browser/screenshot/recording tooling writes
|
||||
captures under os.tmpdir(), and agents must be able to register those from every lane.
|
||||
- Lanes without a baseDir (dashboard chat, no-baseDir heartbeats) are therefore bounded to
|
||||
tmpdir-only absolute paths.
|
||||
*/
|
||||
const isRelative = !isAbsolute(rawPath);
|
||||
if (isRelative && !baseDir) {
|
||||
throw new Error("relative path requires a workspace directory for this session; pass an absolute path under the OS temp directory instead.");
|
||||
}
|
||||
const resolvedPath = isRelative ? resolve(baseDir!, rawPath) : rawPath;
|
||||
|
||||
let canonicalPath: string;
|
||||
try {
|
||||
canonicalPath = await realpath(resolvedPath);
|
||||
} catch {
|
||||
throw new Error(`path ${resolvedPath} does not exist or is not readable.`);
|
||||
}
|
||||
|
||||
let canonicalBaseDir: string | undefined;
|
||||
if (baseDir) {
|
||||
try {
|
||||
canonicalBaseDir = await realpath(baseDir);
|
||||
} catch {
|
||||
canonicalBaseDir = undefined;
|
||||
}
|
||||
}
|
||||
const canonicalTmpDir = await realpath(tmpdir());
|
||||
const isInside = (child: string, root: string): boolean => child === root || child.startsWith(root.endsWith(sep) ? root : root + sep);
|
||||
|
||||
if (isRelative) {
|
||||
if (!canonicalBaseDir || !isInside(canonicalPath, canonicalBaseDir)) {
|
||||
throw new Error(`path ${rawPath} escapes the session workspace directory ${baseDir}; relative artifact paths must stay inside it.`);
|
||||
}
|
||||
} else if (!(canonicalBaseDir && isInside(canonicalPath, canonicalBaseDir)) && !isInside(canonicalPath, canonicalTmpDir)) {
|
||||
const allowedRoots = [canonicalBaseDir, canonicalTmpDir].filter(Boolean).join(", ");
|
||||
throw new Error(`path ${resolvedPath} is outside the allowed roots (${allowedRoots}); artifact files must live under the session workspace directory or the OS temp directory.`);
|
||||
}
|
||||
|
||||
let fileStat;
|
||||
try {
|
||||
fileStat = await stat(resolvedPath);
|
||||
fileStat = await stat(canonicalPath);
|
||||
} catch {
|
||||
throw new Error(`path ${resolvedPath} does not exist or is not readable.`);
|
||||
}
|
||||
@@ -1743,7 +1800,7 @@ async function readArtifactFileFromPath(
|
||||
throw new Error(`Could not infer a MIME type from ${resolvedPath}; pass mimeType explicitly.`);
|
||||
}
|
||||
|
||||
const data = await readFile(resolvedPath);
|
||||
const data = await readFile(canonicalPath);
|
||||
|
||||
if (params.type === "image") {
|
||||
if (!mimeType.startsWith("image/")) {
|
||||
|
||||
Reference in New Issue
Block a user