feat(kap-server): support server-local path attachments - #3247
Conversation
Web and desktop clients can now attach files, images, and videos to a prompt by server-local absolute path instead of uploading a copy. The daemon validates the path (absolute, realpath-resolved, non-sensitive, local runtime only) and references the original file in place, so the agent reads the original path; the upload flow is unchanged. Submitted file attachments are also recorded on the prompt origin and projected as typed transcript attachments, so web clients render attachment chips for plain files without parsing the model-facing notice text.
🦋 Changeset detectedLatest commit: 0eaa85d The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
commit: |
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Forward file metadata from skill activations
When the new skill-activation path receives a file attachment, resolvePromptMediaFiles returns its typed metadata in preparedMedia.attachments, but this route forwards only the converted text notice. Since turnPromptAttachments can recover file attachments only from prompt origin metadata, these files produce no attachment entity or chip in either the live projection or cold rebuild; pass the metadata through the skill activation prompt origin as the normal prompt route does.
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if (part.type === 'file') { | ||
| if (part.path !== undefined) { | ||
| const sourcePath = part.path; | ||
| const { info } = await statAttachmentFile(sourcePath); |
There was a problem hiding this comment.
If a symlink is retargeted after assertPromptPathRefs runs but before media preparation, this second realpath can resolve to a sensitive file without another isSensitiveFile check; image/video branches then read that file directly, while the file branch publishes the mutable symlink path for a later Read. Validate the same resolvedPath that is subsequently read and retain that canonical path in file notices and metadata.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0eaa85d40f
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| ownedFileIds.add(saved.id); | ||
| content.push({ | ||
| type: 'image', | ||
| source: { kind: 'url', url: buildDaemonFileUrl(saved.id) }, |
There was a problem hiding this comment.
When a client is already subscribed to the live transcript and submits a path-sourced image, this generated image_url omits id (and the video branch does the same), so turnPromptAttachments rejects it because it requires the URL's file ID to equal the explicit id. Consequently the live projector emits no attachment for the turn, although a later cold rebuild recognizes the daemon URL and restores it; include id: saved.id on both generated media references so live and cold transcripts agree. Transcript subscribers cannot recover this from the legacy event because projected session events are suppressed for them.
AGENTS.md reference: packages/kap-server/AGENTS.md:L29-L29
Useful? React with 👍 / 👎.
| .catch((error: unknown) => { | ||
| if (isFsError(error)) throw fileNotFoundError(sourcePath); |
There was a problem hiding this comment.
When copying a path-sourced video into the daemon store fails because the destination is full, read-only, or otherwise unavailable, the catch covers the entire store.save(...) operation and isFsError converts errors such as ENOSPC or EACCES into FILE_NOT_FOUND. The source can still exist, so clients receive a misleading 404 and cannot distinguish a server storage failure; only map errors raised while reading the source path and let destination/store failures propagate as server errors.
Useful? React with 👍 / 👎.
2 participants
Related Issue
None — internal feature (problem described below).
Problem
In web/desktop sessions, attached files go through the browser File API, which only exposes name + bytes. The daemon therefore materializes every upload into the session attachments dir and the agent only ever sees that copy path — never the file's original location. Consequences: agents asked to edit an attached file edit the throwaway copy, large files are stored twice, and the model-facing notice text (
Attached file "..." ... — open it with the Read tool) doubles as the UI's attachment record, which web clients must regex-parse back into chips.What changed
Attach by server-local path (zero-copy). The prompt API now accepts file parts with a server-local absolute
path(exactly one offile_id/path), and image/video sources withkind: 'path'. Validation: absolute, realpath-resolved (symlink-safe), not on the Read tool's sensitive-file list, and local runtime only (checked against the target agent's runtime binding before any fs probing). Path-sourced files keep their original path in the byte-identical model-facing notice — no copying. Path images reuse the existing compression pipeline (caption names the original path, originals are not re-persisted) and are capped by the existingMAX_IMAGE_DECODE_BYTES; path videos stream into the daemon store like uploads.Typed attachment metadata on the prompt origin.
UserPromptOrigingainsattachments({name, mediaType, size, path}), populated at submit for every resolved file part, merged on steer, and surfaced onturn.startedas a newkind: 'file'prompt-attachment variant. Live (coreEventMap) and cold (groupTurns) transcript projections emit attachment entities withmediaType/name/sizeand nosource—source.kindis a closed union parsed by shipped clients, so no new enum values are introduced; clients keep recovering the path from the notice text they already parse.packages/protocolmirrors the new wire shapes (schema + XOR constraint); the submit route's OpenAPI errors declareFILE_NOT_FOUND.Review-hardening included (external model review): realpath before sensitive-check, guard ordered before fs stat (no existence probing via error codes), resolve-stage fs errors mapped to
file.not_found.Deferred (pre-existing
#3088-chain gaps, not regressions): steered/hook-blocked prompts don't re-project live attachments; undo doesn't retract live attachment entities.Checklist
turn.startedattachments, steer merge), transcript (cold-rebuild fold), protocol (schema XOR).gen-changesetsskill, or this PR needs no changeset.gen-docsskill, or this PR needs no doc update.