Skip to content

feat(composer): accept PDF, TXT and MD attachments, not just images - #951

Merged
pat-lewczuk merged 11 commits into
mainfrom
feat/pdf-text-attachments
Sep 3, 2026
Merged

pat-lewczuk merged 11 commits into
mainfrom
feat/pdf-text-attachments

Conversation

@pat-lewczuk

@pat-lewczuk pat-lewczuk commented Sep 3, 2026

Copy link
Copy Markdown
Collaborator

Closes #950
Tracking plan: .ai/runs/2026-09-03-pdf-text-attachments.md
Status: complete

🎯 Goal

  • The composer takes a .pdf, .txt or .md the same way it already takes a screenshot — paperclip, ⌘V, drag-drop — and the agent is handed the path of the file on disk rather than its bytes. Before this, the paperclip greyed a .md out in the OS dialog, paste and drop discarded it without a word, and the wire would have refused it anyway (mediaType: /^image\//).

What Changed

  • packages/contract/src/runs.ts — the wire widened in place, not renamed. The images key on POST /runs, POST /runs/:id/messages, PATCH /runs/:id/queued-messages/:msgId and POST /runs/:id/continue keeps its name and its {mediaType, data} shape; only the element's media-type rule widens, from image/* to image/* plus application/pdf, text/plain, text/markdown, text/x-markdown. Images were not narrowed — an image/svg+xml a client has always been allowed to paste is still allowed. Two helpers ship with it and are the single source of truth for wire, engine and cockpit alike: attachmentExtension (media type → on-disk extension) and isImageAttachmentName (the one predicate every reader branches on). imageInputSchema / ImageInput remain as deprecated aliases.
  • packages/cezar/src/server/server.ts — one mapping, four routes, safe serving. Every attachment-carrying route maps through the same toPastedContent, so an image becomes a viewable block and a file becomes a file block the engine turns into a path; none of them can invent a different mapping. GET /runs/:id/images/:file answers an image byte-identically (headers included), while a non-image now leaves with its true content type plus X-Content-Type-Options: nosniff and Content-Disposition: attachment — these are user-supplied bytes coming back from the cockpit's own origin, so they must never render as an active document there. .pdfapplication/pdf, .md/.txttext/plain, anything else keeps the pre-existing application/octet-stream.
  • packages/cezar/src/workflows/run.ts — persistence, the re-read, and the paths note. persistImage becomes persistAttachment and takes its extension from the shared contract map, so a pasted-3.md lands in the same numbering space as pasted-3.png and cannot collide with it. One helper (persistPastedAttachments) now serves all five entry points — task start, live delivery, the queued stack, an edit, and Continue — and contentBlocksOf guarantees a file block can never reach a backend. The restart/dequeue re-read (readPersistedAttachments) branches on the persisted name: an image comes back as a base64 block, a file as a path only, because re-encoding a .pdf into image data composes a message no backend can accept.
  • Two real gaps closed on the way, both proven red before the fix. (1) The on-disk paths note was nested inside if (images?.length), so an attachment-only task would have handed the agent a brief about a file it was never told the location of. (2) The note listed the initial prompt's attachments only, so a file stacked onto a still-queued run reached the agent as nothing at all — a stacked screenshot could at least fall back on its inline block; a stacked .md has no fallback.
  • packages/web — the cockpit. composer-images.ts is now composer-attachments.ts (and ImageInput reads AttachmentInput at every call site) because neither is images-only any more. The paperclip accepts both the MIME and the extension spelling of every type — accept="image/*,text/plain" alone still greys a .md out on Windows — paste filters on kind: 'file' rather than an image/ type test, and anything cezar will not take is now refused out loud, naming the file. The browser's file.type is not treated as a good enough reason to refuse: when it is empty or unrecognised the extension decides, because a user cannot fix their machine's MIME database. An attachment with nothing to preview renders as a named chip — in the composer row, and on the thread bubble, where the persisted name decides which of the two a URL in the shared list becomes.
  • Docs. BACKWARD_COMPATIBILITY.md §2 records the widened request surface, the two widened refusal strings and the new response headers; §3 records the honest cost of keeping one on-disk list. The agent-facing system-prompt note said "the inline image is for viewing only", which reads as though every attachment has an inline copy — a non-image one does not, and it now says so. README.md names the formats a user can attach.

🧪 Tests

  • npm run typecheck ✓ · npm test6252 passed / 329 files ✓ · npm run test:unit — 36 ✓ · npm run build (incl. check:pack) ✓ · npm run test:package — 16 ✓.
  • Every command was run with TMPDIR=/tmp. This machine's TMPDIR points inside the checkout, which makes six pre-existing "outside a git repository" cases (git, git-changes, git-worktree, health-forge, projects-api, plus one automations preview-timing case) fail on main too; with the tmpdir outside the repo the whole suite is green.
  • New coverage, and what it locks in:
    • Engine e2e through the real dry-run backend — a .md and a .pdf attached at task start land as pasted-1.md / pasted-2.pdf, their paths reach the opening prompt, their bytes reach neither the prompt nor the NDJSON event log, and imageCount is 0. A second case covers a file stacked onto a queued run.
    • Both gap fixes proven red first (git-stash-style revert, test fails, fix restored) — the ungated note and the stacked-attachment paths.
    • Restart re-read — a .pdf sitting in the same list as a screenshot comes back as a path, never as an image block; a file whose bytes are gone is noted and dropped rather than named as a dead path.
    • Route contract (attachments-api.test.ts, new) — a PDF/TXT/MD is accepted and reaches the engine as a file block, application/zip and text/html are 400s, image/svg+xml still passes, an attachment-only message is accepted, and each serving-header case is pinned including the unchanged image response.
    • Composer intake — the format matrix, the typeless-.md extension fallback, the named refusal, the caps, and the chip-not-thumbnail rendering through the real component; thread bubbles — an image renders inline and a .pdf/.md/.txt renders as a download chip.

💥 Breaking Changes

  • None on the wire or on disk. The widening is additive (a client that sends what it always sent sees no change), the on-disk shape is untouched (one array of URL strings per message, one pasted-<n>/screenshot-<n> numbering space), and a record written here parses on an older cezar.
  • Two refusal strings widened (too many queued imagestoo many queued attachments, … at least one image… at least one attachment); statuses and the {error} shape are unchanged.
  • Documented degradation, not avoided: an older cezar reading a newer record renders a broken <img> for a .pdf entry and its engine would re-encode the file as image/png. Keeping one list was preferred to a second one that every reader — the orphan sweep, the dequeue re-read, the per-stack cap, the bubble — would have had to learn about and that an older cezar would have dropped entirely. BACKWARD_COMPATIBILITY.md §3 says so.

🔗 Related

📋 Progress

See the Progress section in the tracking plan.

@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown

📦 npm preview published — 0.10.0-pr951.1402

Try this PR build (exact pinned version — copy-paste as-is):

npx cezar-cli@0.10.0-pr951.1402                                # cockpit at http://localhost:4321
npx cezar-cli@0.10.0-pr951.1402 run "…"                        # headless run
npx cezar-cli@0.10.0-pr951.1402 server-deploy --platform <id>  # roll a server to this exact build

Also tagged: npm install -g cezar-cli@pr-951 (moving tag for this PR).
Packages: cezar-cli@0.10.0-pr951.1402@open-mercato/cezar@0.10.0-pr951.1402@open-mercato/cezar-api-client@0.10.0-pr951.1402 (provenance attested).

The `images` key of POST /runs, /runs/:id/messages, the queued-message PATCH and
/runs/:id/continue is the wire's one attachment list, and it accepted image/* only.
Widen the element schema in place — every image type still passes, and PDF/TXT/MD
join them — rather than growing a second list the on-disk shape would have to mirror.

The extension an attachment lands under is derived from its media type alone
(`attachmentExtension`), so a user's filename never reaches a path, and
`isImageAttachmentName` is the one predicate every reader branches on.

Refs #950
A PDF or a .md has nothing for a model to look at, so it never rides inline: the
engine writes it into the run's attachment folder, in the same `pasted-<n>`
numbering space as the screenshots, and the agent gets its absolute path — the form
its file tools want, the only one that survives the codex/opencode backends, and the
one that keeps a multi-megabyte document out of the prompt bounds.

Two gaps closed on the way, both proven red first:

- the path note was gated on an image block existing, so an attachment-only task
  would have handed the agent a brief about a file it was never told the path of;
- the note listed the initial prompt's attachments only, so a file stacked onto a
  queued run reached the agent as nothing at all.

The restart re-read now branches on the persisted name: an image comes back as a
block, a file as a path, because re-encoding a .pdf as base64 image data composes a
message no backend can accept.

Refs #950
All four attachment-carrying routes map through one shared helper, so an image
becomes a viewable block and a file becomes a file block the engine turns into a
path — none of them can invent a different mapping.

The serving route is where user-supplied bytes come back from the cockpit's own
origin, so a non-image now leaves under `nosniff` and an attachment disposition,
and a .pdf/.txt/.md gets its true content type instead of octet-stream. Image
responses are unchanged, headers included.

Refs #950
The paperclip greyed a .md out in the OS dialog, paste and drop filtered on
`image/*`, and everything else was discarded without a word — which reads as a
broken composer rather than as an unsupported file. All three now take the same
files the wire does, and anything cezar will not take is refused out loud, naming it.

Two details that matter more than they look:

- the browser's `file.type` is not a good enough reason to refuse. Windows reports
  '' for a .md on plenty of setups, so the extension decides when the type is empty
  or unknown — a user cannot fix their machine's MIME database.
- an attachment with nothing to preview renders as a named chip, in the composer row
  and on the thread bubble, where the persisted NAME decides which of the two a URL
  in the shared list becomes.

`composer-images` is now `composer-attachments`, and `ImageInput` reads
`AttachmentInput` at every call site, because neither is images-only any more.

Refs #950
…t files

BACKWARD_COMPATIBILITY §2 gets what changed on the wire (an allowlist widened in
place, two refusal strings, the serving route's new headers for non-images) and §3
gets the honest cost of keeping ONE on-disk list: an older cezar renders a broken
img for a .pdf entry and re-encodes it as image/png. That was preferred to a second
list every reader would have had to learn about.

The agent-facing contract in the system prompt said 'the inline image is for viewing
only', which reads as though every attachment has an inline copy. A non-image one
does not — it exists only as the file at its path, and now says so.

Refs #950
@pat-lewczuk pat-lewczuk added review Ready for code review needs-qa Requires manual QA before merge feature New capability priority-medium Ordinary bug or feature risk-high Wide blast radius, review deeply frontend GUI / React / Next.js labels Sep 3, 2026
@pat-lewczuk

Copy link
Copy Markdown
Collaborator Author

🤖 om-auto-create-pr — 🏷️ label rationale

  • 🔍 review — the pipeline state this PR opens in: the work is complete and the gate is green, so what it needs next is a reviewer, not more implementation.
  • feature — net-new user-facing capability (the composer accepting PDF/TXT/MD), not a regression of behaviour that once worked. The two silent-drop bugs fixed along the way (the path note gated on an image block, and stacked attachments never listed) are on the path this feature opens, so they travel with it rather than as a separate bug PR.
  • 🎨 frontend — the cockpit is a real part of the change: the composer's intake, picker, paste/drop and attachment row, plus the thread bubble's chip rendering. It is not only a backend widening.
  • 🔹 priority-medium — per SDLC.md, an ordinary net-new feature reads as medium: nothing published is broken, no data is at risk, and the existing screenshot path keeps working untouched. The gap it closes is everyday friction rather than an outage.
  • ⚠️ risk-highSDLC.md names the HTTP API surface and the .ai/cezar/ state formats as high-risk, and this touches both: four request schemas widen, GET /runs/:id/images/:file changes what it serves and with which headers for non-images, and a new kind of entry joins the same on-disk attachment list an older cezar will read back. The sibling PR feat(composer): attach text files (.md, .txt) as well as images #948 carries risk-high for the same reasons.
  • 🧪 needs-qa — user-facing UI behaviour changed in ways a test suite cannot fully stand in for: an OS file dialog's accept filtering, a real ⌘V of a .md, a drag-drop, and how the chip and the download link look in both themes. A CEZ_DRY_RUN=1 cockpit session exercises all of it. Deliberately not skip-qa, which is for docs-only and similarly non-user-facing work.

No blocked, do-not-merge or changes-requested: nothing is holding this back at the time of writing.

@pat-lewczuk pat-lewczuk added the in-progress Cezar agent is actively working this issue label Sep 3, 2026
@pat-lewczuk pat-lewczuk self-assigned this Sep 3, 2026
@pat-lewczuk

Copy link
Copy Markdown
Collaborator Author

🤖 om-auto-review-pr started by @pat-lewczuk at 2026-09-03T07:19:49Z. Other auto-skills will skip this PR until the lock is released.

… runner seam's shape

Review follow-ups on this PR, all minor:

- the refusal toast promised 'images, PDF, TXT and MD only' while the intake also
  takes a typeless .log as text/plain — the message now states the rule the code
  actually applies rather than a narrower one the user would be surprised by;
- a task carrying only file attachments handed the runner seam `[]` where it has
  always handed `undefined`. Both runners spread with `?? []` so nothing behaved
  differently, but the seam keeps its historical shape;
- `PersistedImages` reads `PersistedAttachments` (it returns paths for files too),
  and the comment left where server.ts's local image schema used to live now says
  what replaced it.

Refs #950
@pat-lewczuk

Copy link
Copy Markdown
Collaborator Author

🔍 Code Review: feat(composer): accept PDF, TXT and MD attachments, not just images

🎯 Summary

This PR lets the composer take a .pdf, .txt or .md on the same road a pasted screenshot already travels, and the shape of that road is the reason the change stays small. Rather than adding a second attachment list, it widens the element schema behind the existing images key — every image/* type still passes and four new ones join them — so the on-disk contract (taskImages, queuedMessages[].images, one pasted-<n> numbering space) does not move at all. The one genuinely new idea is the file block: a private engine type (packages/cezar/src/workflows/run.ts:436) that deliberately is not a ContentBlock variant, because ContentBlock is the runner protocol and a PDF has nothing a model can look at. contentBlocksOf drops those blocks before anything reaches a session, which makes "a backend never sees a file block" a property of the type flow rather than of a reviewer's vigilance.

I reviewed all 25 changed files at head e3f7efd6 in an isolated worktree — the contract, the four attachment-carrying routes and the serving route, the run engine, the cockpit composer and thread, and the docs. Two things stand out as better than the brief required. First, the PR found and fixed two pre-existing silent-drop bugs that this feature would have walked straight into: the on-disk paths note was nested inside if (images?.length) (run.ts:2907), so an attachment-only task would have handed the agent a brief about a file it was never told the location of, and the note listed only the initial prompt's attachments, so a file stacked onto a queued run reached the agent as nothing at all. Both were proven red before the fix, which is the discipline AGENTS.md asks for and the reason I trust them. Second, the security question that a "serve user files from the cockpit's own origin" change always raises is answered in the code rather than in the description: nosniff plus an attachment disposition on every non-image, a Content-Disposition filename that degrades exotic characters to underscores rather than to a quote that could split the header, and — the decision I like most — no user filename reaches the wire or a path at all, since the extension is derived from the media type (attachmentExtension). That removes the whole sanitization-and-traversal class of bug instead of defending against it.

Three minor findings came out of the pass; all three were fixed in-review (fa758d07) since this is an autofix-eligible run. No blockers, no majors.

Verdict

APPROVE — with the procedural caveat that GitHub rejects self-approval, so no formal approving review could be submitted on this PR; see the note under Findings.

🧪 Validation Gate

Command Result
npm run typecheck ✅ pass (contract, api-client, server, web)
npm test ✅ pass — 6252 passed / 329 files
npm run test:unit ✅ pass — 36
npm run build (incl. check:pack) ✅ pass — check:pack ok — 478 files, 85 under web/dist
npm run test:package ✅ pass — 16

Every command was run with TMPDIR=/tmp. On this machine TMPDIR points inside the checkout, which makes six "outside a git repository" cases (git, git-changes, git-worktree, health-forge, projects-api, plus one automations preview-timing case) fail on main too — an environment artifact, not a branch regression. With the tmpdir outside the repository the suite is green end to end.

Findings

Minor

  1. packages/web/src/components/composer/composer-attachments.ts:95 — the refusal toast promised a narrower rule than the code applies. The message read "images, PDF, TXT and MD only", but the intake resolves a typeless .log to text/plain and the wire allowlist takes text/plain whatever the extension — so a user could be told a file was unsupported by a rule that is not the one being enforced, or (worse) see a .log accepted right after reading that only TXT and MD are. Fixed by stating the actual rule: "images, PDF and plain-text files such as TXT or MD". Worth being explicit about the consequence in the PR description too: allowlisting media types means any file the browser calls text/plain is accepted, which is slightly wider than "TXT and MD" and is the right trade — refusing a .csv the browser typed as text would be arbitrary.

  2. packages/cezar/src/workflows/run.ts:2734startImages became [] where the seam has always been handed undefined. contentBlocksOf returns an array, so a task carrying only file attachments (or none) now passes an empty array into AgentSessionSpec.images. I checked both consumers — claude-cli-runner.ts:190 and pi-runner.ts:134 both spread with ?? [] — so nothing behaved differently; but [] is truthy where undefined is not, and a future runner written against the documented optional field would be the one to find out. Fixed by keeping the field absent when there is nothing to view.

Nit

  1. packages/cezar/src/workflows/run.ts:511 and packages/cezar/src/server/server.ts:778 — two names left behind by the widening. The PersistedImages interface now also carries paths for files, and the comment where server.ts's local imageInputSchema used to live described a schema no longer declared there. Renamed to PersistedAttachments and reworded to name the contract schema that replaced it.

Considered and deliberately not changed

  • Pasting an unsupported file now consumes the paste. composer.tsx:293 filters on item.kind === 'file' instead of an image/ type test, so a pasted .zip reaches screenFiles, is refused with a named toast, and the handler calls preventDefault(). Before, that paste fell through silently. A clipboard file item has no text to insert, so nothing is lost in practice, and the loud refusal is the point of the change — the silent drop is the defect being fixed.
  • The older-cezar degradation is documented, not avoided. A .pdf entry in a shared list renders as a broken <img> on an older cockpit, and an older engine would re-encode it as image/png. BACKWARD_COMPATIBILITY.md §3 says so plainly and argues the alternative — a second on-disk list that every reader (orphan sweep, dequeue re-read, per-stack cap, bubble) would have to learn and an older cezar would drop entirely — is the larger break. I agree with the call; what makes it safe to live with is isImageAttachmentName, the single predicate every reader now branches on.

Procedural note (not a code finding)

GitHub does not accept a review from the PR's own author, so this report is posted as a comment rather than as an approving review, and the review pipeline label stays — the PR still needs a second pair of eyes to convert this verdict into a formal approval. Per the tracker descriptor, that limitation is surfaced rather than worked around.

💥 Breaking Changes

  • Exported APIs — additive only. attachmentInputSchema / AttachmentInput, isImageMediaType, isAttachmentMediaType, attachmentExtension, isImageAttachmentName are new; imageInputSchema and ImageInput are kept as deprecated aliases, so nothing importing them breaks.
  • HTTP request shapes — the images key keeps its name and {mediaType, data} element shape on all four routes; only the media-type rule widens. A client that sends what it always sent is unaffected, and image/svg+xml (an exotic type the old regex allowed) still passes — verified by a dedicated case in attachments-api.test.ts.
  • HTTP response shapesGET /runs/:id/images/:file keeps its path; an image response is byte-identical, headers included (pinned by a test). Non-image responses are the additive case.
  • Persisted state — no new field, no retyped field, no rename. taskImages and queuedMessages[].images stay arrays of URL strings; the file-name space is shared, so nothing is orphaned.
  • Event names / CLI flags / config formats / DB schema — untouched. No new runtime dependency (CODE_REVIEW.md's server dependency budget is unchanged).
  • Two user-facing error strings widenedtoo many queued imagestoo many queued attachments, … at least one image… at least one attachment. Statuses and the {error} shape are unchanged, and BACKWARD_COMPATIBILITY.md §2 records both. Not a protected surface, but called out because a script grepping the old wording would notice.

BACKWARD_COMPATIBILITY.md §2 and §3 were both updated in this PR, and I checked every touched surface against them: no protected surface is broken and no deprecation path is owed.

🧪 Test Coverage

Coverage is proportionate to the risk in every direction the change moves, and the two tests that matter most are the ones asserting a negative:

  • Engine e2e through the real dry-run backend (pasted-attachments.test.ts) — a .md and a .pdf attached at task start land as pasted-1.md / pasted-2.pdf, their paths reach the opening prompt, and their bytes reach neither the prompt nor the NDJSON log (asserted on the decoded content and the base64), with imageCount at 0. This is the test that proves the design claim rather than the implementation detail.
  • Both silent-drop fixes proven red first — the ungated note and the stacked-attachment paths each fail without their fix, which is what makes them regression tests rather than green-either-way assertions.
  • Restart re-read (run.test.ts) — a .pdf beside a screenshot in one list comes back as a path and never as a re-encoded image block; a file whose bytes are gone is noted and dropped rather than named as a dead path.
  • Route contract (attachments-api.test.ts, new) — the allowlist in both directions (application/zip and text/html are 400s, image/svg+xml still passes), an attachment-only message accepted, file blocks reaching the engine, and every serving-header case including the unchanged image response.
  • Cockpit — the intake matrix, the typeless-.md extension fallback, the named refusal, the caps, chip-not-thumbnail rendering through the real component, and the thread bubble choosing an inline image or a download chip by the persisted name.

Gap worth naming, and it is the QA layer's rather than this PR's: nothing here exercises a real browser, so the OS file dialog's accept filtering, an actual ⌘V of a .md, and how the chip and download link look in both themes remain unverified by machine. That is exactly what the needs-qa label on this PR is for.

@pat-lewczuk

Copy link
Copy Markdown
Collaborator Author

🤖 om-auto-review-pr — 🧪 manual QA instructions

This PR carries needs-qa because the parts a test suite cannot stand in for are exactly the parts that changed: an OS file dialog's own filtering, a real clipboard paste, a drag-drop, and how a chip and a download link look in both themes. Run the cockpit with CEZ_DRY_RUN=1 (no agent login needed) and work down this list — P0 must pass for the PR to merge.

Setup: npm run build && CEZ_DRY_RUN=1 npx cezar (or npm run dev), then open the cockpit and start from New task. Have four files ready: a screenshot, a .pdf, a .txt and a .md.

P0 — the feature, and the paths that used to fail silently

  1. Paperclip takes all four. Click the paperclip in the New-task composer. In the OS dialog, confirm the .pdf, .txt and .md are selectable (not greyed out) — this is the failure that made the feature unreachable before, and it is OS-specific, so please note which OS you tested on. Pick the .md: it must appear as a named chip showing the real filename, not as a thumbnail or a broken image.
  2. Drag-drop and paste. Drag the .pdf onto the composer; copy the .txt in your file manager and ⌘V/Ctrl+V into the composer. Both must be accepted the same way. Then try a file cezar does not take (a .zip): it must be refused with a toast naming the file — the old behaviour was to drop it with no message at all, so "nothing happened" is a failure here.
  3. The agent actually gets the file. Start the task with the .md attached and a prompt like "tell me what the attached file says". In the run thread, the task bubble must show the attachment; the agent's opening prompt must contain the absolute path (…/.ai/cezar/runs/<id>-images/pasted-1.md) and not the file's text. Confirm the file really exists at that path on disk.
  4. Attachment-only message. Send a follow-up into a live session with only a file and no text. It must be accepted (not rejected as an empty message) and the path must reach the session.
  5. Stacked onto a queued run. With another task occupying the slot, start a task, then add a message carrying a .pdf while it is still queued. When it dequeues, the opening prompt must name that file's path too — this is the second path that used to hand the agent nothing.
  6. Mixed message. One message with a screenshot and a .pdf: the image renders inline in the bubble, the PDF renders as a download chip. Click the chip — it must download, never render in the browser tab.

P1 — the edges around it

  1. Continue. Finish a task, then use the follow-up composer on the closed run to Continue with a .txt attached. The reopened session must receive the path.
  2. Caps. Attach five files — the fifth is refused with "max 4 attachments per message". Attach something over 5 MB — refused with the size message, naming the file.
  3. Restart survival. Attach a .pdf to a task, leave it queued, restart cezar, then let it run. The attachment must survive and its path must still reach the agent (nothing should be re-encoded or lost).
  4. Screenshots did not regress. Paste a screenshot into a live session exactly as before: inline thumbnail in the bubble, click-to-zoom still works, and the agent still gets both the image and its path.

P2 — presentation

  1. Both themes. Check the composer chip and the thread download chip in light and dark mode: the text truncates rather than overflowing on a long filename, and the hover/focus "remove" affordance on the composer chip is visible in both.
  2. Keyboard. Tab to the composer chip and press Enter/Space — it must remove that attachment; Tab to a thread download chip — it must be focusable and activatable.
  3. A typeless file. If you are on Windows (or can simulate it), attach a .md from a machine whose browser reports no MIME type for it. It must still be accepted, on its extension.

Attach a screenshot of at least items 1, 6 and 11 to this PR, then apply qa-approved. If you are signing off under the self-QA exception, add qa-self-verified alongside it and say what you exercised.

@pat-lewczuk pat-lewczuk added ci-monitoring and removed in-progress Cezar agent is actively working this issue labels Sep 3, 2026
@pat-lewczuk pat-lewczuk removed their assignment Sep 3, 2026
@pat-lewczuk

Copy link
Copy Markdown
Collaborator Author

🤖 om-auto-review-pr completed: APPROVE (no blockers, no majors). Lock released — swapped for ci-monitoring while the CI follow-up runs.

Three minor findings were fixed in-review on this autofix-eligible run (fa758d07): the refusal toast now states the rule the intake actually applies, a task carrying only file attachments hands the runner seam undefined rather than [] as it always has, and two names left behind by the widening (PersistedImages, the comment where server.ts's local image schema used to live) were corrected. The full validation gate is green after those fixes — typecheck, 6252 vitest cases across 329 files, 36 node:test cases, build including check:pack, and the packaged CLI E2E.

The pipeline label stays review rather than moving to merge-queue, and deliberately so: GitHub refuses a review from a PR's own author, so the verdict above could not be submitted as a formal approval. This PR still needs a second pair of eyes to convert it. needs-qa also stays — the manual QA instructions are in the comment above.

@pat-lewczuk
pat-lewczuk marked this pull request as ready for review September 3, 2026 07:26
@pat-lewczuk

Copy link
Copy Markdown
Collaborator Author

🤖 om-auto-create-pr — run summary

Engine: om-auto-create-pr (steps: 12, --loop: no)

🎯 What this run was asked to do

Make the composer accept file uploads in PDF, TXT and MD format, not just images. That brief arrived as issue #950, which I had filed earlier from the same request; this run implemented it end to end on feat/pdf-text-attachments off main, in an isolated worktree, and never touched the user's checkout.

📋 How it was done

The execution plan (.ai/runs/2026-09-03-pdf-text-attachments.md, committed as the branch's first commit so the run is resumable) broke the work into five phases and twelve steps, every one of them now checked with its commit SHA:

  1. Wire contract and routes — the element schema behind the existing images key widened in place (653f3518, d5d7abc0), plus the safe serving headers.
  2. Run engine — file blocks, persistence, the restart re-read, and the on-disk paths note (c7f6a206).
  3. Cockpit intake and 4. rendering — picker, paste, drop, chips (cbe43a2e).
  4. Docs and gateBACKWARD_COMPATIBILITY.md, README.md, the agent-facing prompt note (d085fd03).

Then the review pass (om-auto-review-pr 951 --autofix) added fa758d07.

🧩 The decisions worth knowing about

  • One list, not two. The wire keeps the images key and the disk keeps one attachment list with one pasted-<n> numbering space; only the media-type rule widened. That is what let the orphan sweep, the dequeue re-read, the per-stack cap and the thread bubble all keep working without a second code path — and it is why isImageAttachmentName (branch on the persisted name, never on the list) is now the single predicate every reader uses.
  • A file is never inlined. It has nothing for a model to look at, so it is written to the run's attachment folder and the agent is handed its path — the form its file tools want, the only form that survives the codex/opencode backends, and the one that keeps a multi-megabyte PDF out of the prompt bounds.
  • No user filename ever reaches a path. The on-disk extension is derived from the media type alone, which removes the sanitization/traversal question rather than defending against it.
  • Serving is binary-safe. A non-image leaves the cockpit's own origin with nosniff and an attachment disposition; images answer byte-identically to before.

🐛 Two silent-drop bugs found and fixed on the way

Both pre-existed this feature and would have made it half-work; both were proven red before the fix, per AGENTS.md:

  • the agent-facing paths note was nested inside if (images?.length), so an attachment-only task would have handed the agent a brief about a file it was never told the location of;
  • the note listed only the initial prompt's attachments, so a file stacked onto a still-queued run reached the agent as nothing at all.

🧪 Verification

The full configured gate, run at the reviewed head with TMPDIR=/tmp:

Command Result
npm run typecheck
npm test ✅ 6252 passed / 329 files
npm run test:unit ✅ 36
npm run build (incl. check:pack) ✅ 478 files packed
npm run test:package ✅ 16

TMPDIR matters here and is worth recording for whoever runs this next: on this machine it points inside the checkout, which makes six "outside a git repository" cases fail on main too. They are an environment artifact, not a branch regression.

New coverage is listed in the review comment above; the one that carries the design claim is the engine e2e through the real dry-run backend asserting a .md's path reaches the prompt and its bytes reach neither the prompt nor the event log.

Still pending: the GitHub Actions run for the latest head, and manual QA. Neither was waited on before reporting — the CI follow-up is bounded and its result will be posted as its own comment.

🔍 Review

om-auto-review-pr 951 --autofix ran as the single authoritative pass: approve, no blockers and no majors, with three minor findings fixed in-review. GitHub refuses a review from a PR's own author, so that verdict is posted as a comment and the review label stays — this PR still needs a second pair of eyes to convert it into a formal approval.

🏷️ Labels

review · feature · frontend · priority-medium · risk-high · needs-qa (rationale in its own comment), plus ci-monitoring while the CI follow-up runs.

🔗 Relationship to the in-flight PRs

This supersedes the TXT/MD half of #948 and the "any file type, no allowlist" approach of #929, and adds the PDF support neither has. It is an independent implementation — no code was taken from either branch — and the three Major findings raised on #948 (a supported file refused over its browser-reported MIME type, text/html inside the allowlist, and user-controlled bytes served without nosniff/disposition) are addressed here by construction.

@pat-lewczuk

Copy link
Copy Markdown
Collaborator Author

🤖 om-auto-review-pr — CI result

Green at head 2fd53b46, within the follow-up budget:

Check Result
Unit, build, E2E, and package ✅ pass (4m14s)
Publish npm snapshot ✅ pass (33s)
license/cla ✅ pass

This matches the local gate from the review pass, which is the outcome I expected but not one worth assuming — CI runs with a tmpdir outside the checkout, so it exercises the six "outside a git repository" cases that fail on this machine (and on main) purely because of where TMPDIR points.

Worth recording for anyone reading the run history rather than the result: the earlier red run on c6767979 was a stale assertion in continue-run.test.ts, which still expected text/plain to be refused after the allowlist had widened. It was fixed in d085fd03, and every completed run since has been green.

ci-monitoring removed. The PR is now blocked only on the two things this agent cannot supply itself: a formal approving review from an account other than the author (GitHub refuses self-approval), and the manual QA that clears needs-qa — instructions for it are in the P0/P1/P2 comment above.

@pat-lewczuk
pat-lewczuk merged commit ff9c44e into main Sep 3, 2026
3 checks passed
@pat-lewczuk
pat-lewczuk deleted the feat/pdf-text-attachments branch September 3, 2026 11:51
pat-lewczuk added a commit that referenced this pull request Sep 4, 2026
Stamp the release set (contract, api-client, cezar, web, alias-cezar) and the
lockfile 0.10.0 -> 0.10.1, with the intra-release ranges rewritten to ^0.10.1.
A patch bump: every change since 0.10.0 is a feature or a fix, no breaking
surface.

Turn `# Unreleased` into `# 0.10.1 (2026-09-04)` and make it a real release
entry:

- keep the five essay-length entries already drafted (pin tasks, agent-account
  Continue, the CEZ:ASK brace repair, the merge-conflict chip, and the three
  reference-chip fixes), fixing their PR citations along the way (the pin
  feature cited its tracking issue, #935, instead of the PR that shipped it,
  #938; the CEZ:ASK and merge-conflict entries had no citation at all)
- add the ~14 PRs that merged since with no changelog note at all: PDF/TXT/MD
  composer attachments (#951), cross-repo reference-chip safety (#946), a
  stale review request or an "Update branch" merge clearing a rejection
  (#909), cross-project task 404s (#905), the Changes tab's independent
  scroll (#918), a clearable composer skill pill (#919), a resumed session
  keeping its step's tools (#928), Alt quick replies eating Polish
  characters (#943), host-local Claude model discovery (#841), GitHub search
  across every state (#732), two mobile run-header/history fixes (#764,
  #873), and a fresh task's /skill expansion (#947)
- add a CI/CD & Infrastructure section for the GITHUB_RUN_ATTEMPT e2e fix
  (#911)
- add a Highlights paragraph and credit the seven contributors

No code changes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature New capability frontend GUI / React / Next.js needs-qa Requires manual QA before merge priority-medium Ordinary bug or feature review Ready for code review risk-high Wide blast radius, review deeply

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Implement: accept PDF (plus TXT/MD) attachments in the composer, not just images

1 participant