From 8fa2ad0ecdaa986bff003331d691a96435b8cdc8 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Fri, 10 Jul 2026 23:33:32 -0700 Subject: [PATCH] Address PR review feedback (#1996) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 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 --- docs/agents.md | 2 +- docs/storage.md | 2 +- packages/core/src/agent-prompts.ts | 11 ++ .../__tests__/DocumentsView.test.tsx | 41 ++++++ .../artifacts-route-integration.test.ts | 61 ++++----- .../routes/register-task-workflow-routes.ts | 6 +- packages/dashboard/src/test-request.ts | 9 +- .../__tests__/agent-artifact-tools.test.ts | 119 +++++++++++++++++- packages/engine/src/agent-heartbeat.ts | 17 ++- packages/engine/src/agent-tools.ts | 67 +++++++++- 10 files changed, 285 insertions(+), 50 deletions(-) diff --git a/docs/agents.md b/docs/agents.md index 8be00acfba..d21a2ce852 100644 --- a/docs/agents.md +++ b/docs/agents.md @@ -44,7 +44,7 @@ fn chat [message…] [--once] [--non-interactive] [--poll-ms ] 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. diff --git a/docs/storage.md b/docs/storage.md index 0279f73195..8a245f5ca4 100644 --- a/docs/storage.md +++ b/docs/storage.md @@ -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 `