feat(attachments): per-project attachment library

[pat-lewczuk]

Tracking plan: .ai/runs/2026-09-04-attachment-library.md
Status: complete

🎯 Goal

Keep every document a user attaches to a task in one per-project place — .ai/cezar/attachments/, under the file's own name — so a later task can be pointed at spec.md by the name the user knows it by, instead of at runs/<some-other-run>-images/pasted-3.md.

This picks up the one idea from community PR #929 by @Damian-Szczepanski that did not land when #951 shipped non-image attachments. That PR was approved and then withdrawn by its author ("the change already lives in our own build and we are no longer maintaining the branch it was proposed from") — the idea is worth carrying forward, so it is implemented here against current main. Credit for the design, and for the .gitignore diagnosis below, is theirs.

What Changed

• packages/contract/src/runs.ts — the filename reaches the server. attachmentInputSchema gains an additive optional name. Until now an attachment's on-disk name was derived from its media type alone (pasted-<n>.<ext>) and the user's filename never left the browser — correct for the run folder, but it leaves nothing to file a library under. New sanitizeAttachmentName is the one place a client-supplied string may influence a path: basename on both separator conventions, control characters and Windows-refused characters removed, no leading dots, stem bounded to 100, and the extension pinned to the media type the schema already validated. That last part is the one that matters — without it a text/plain upload named install.sh would land in the user's project looking executable, having passed an allowlist that believed it was screening for exactly that. It becomes install.sh.txt.
• packages/web/src/components/composer/ — the composer sends it. Submit now goes through one toAttachmentInput helper instead of an inline destructure, so a single place decides what leaves the browser: the render-only preview (a second full copy of the bytes) never does, and the filename does — for files only.
• packages/cezar/src/workflows/run.ts — the library. copyToAttachmentLibrary files a copy under the user's name. Name clashes are resolved against content first: a library spanning every task in a repo collects a great many notes.md, and the same brief attached to six tasks should leave one file, not six. Only same-name-different-bytes takes a -2/-3 suffix. Best-effort throughout — the run folder already holds the file the agent was promised, so a full disk costs the library entry and nothing else. The run folder itself is unchanged: still pasted-<n>.<ext>, which isImageAttachmentName, the orphan sweep, the restart re-read and the per-stack cap all depend on.
• The agent is told about it as a directory, not per file. The per-run paths in pastedAttachmentsText already cover the current message; what the library answers is "the brief I attached last week", which no per-attachment handle survives to reach — readPersistedAttachments reconstructs an attachment from its URL alone at dequeue, so a libraryPath field would silently vanish on every restart. Naming the folder needs no per-attachment state and is the better answer for what the library is for.
• packages/cezar/src/index.ts — the .gitignore P0. ensureDataGitignore names every entry cezar writes under .ai/cezar/ one by one, because workflows/ and skills/ beside them are meant to be committable. The cost of that design is that an entry nobody adds is covered by nothing — and here the entries are files the user uploaded, so a missing line is one git add -A from an internal PDF in a public repository. attachments/ is now on the list, and data-gitignore.test.ts guards the rule rather than this one entry: a static cross-check of every literal join(dataDir, '…') in the service against the list, with the committable exceptions named, so the next state directory cannot repeat it.
• Docs. BACKWARD_COMPATIBILITY.md §2 (the additive name) and §3 (attachments/, and why no retention sweep touches it — it holds user content, not run data). AGENTS.md now says what makes the ignore allowlist sharp and why the mistake is invisible from inside this repo.

Decision: files only, not images (a deliberate narrowing of #929)

#929 copied every upload. This files documents only, for one product reason and one structural one:

• An image attachment is overwhelmingly a clipboard paste, which has no filename at all — the composer already falls back to the literal string 'pasted image'. A shared folder of pasted-3.png from forty runs is clutter, not a library. A file is always picked or dragged, always has a real name, and that name is the only handle the agent ever gets on it.
• Carrying a name on the image path would mean widening ContentBlock, which is the runner protocol (AGENT_PROTOCOL.md). An extra key there survives contentBlocksOf and reaches vendor APIs that reject unknown fields. FileBlock is cezar's own type and is converted to a path before anything is handed to a session, so the name costs nothing and risks nothing there.

Named image uploads (a dragged diagram.png) can join later behind an explicit paste-vs-pick distinction.

🧪 Tests

Gate, all from this branch:

• npm run typecheck ✅
• npm test — 6275 passed, 6 failed / 330 files. The 6 failures are pre-existing and environmental: every one asserts "not a git repo"/"outside a repo", and TMPDIR resolves inside a git checkout on this machine, so they see this repo's real remote. Verified by running the same six files on a clean origin/main worktree in the same environment — the identical 6 cases fail there too (git-worktree, automations-api, git-changes, git, health-forge, projects-api). Not a subset, the same set. None of them touches attachments.
• npm run test:unit ✅ 36/36 · npm run build ✅ incl. check:pack (478 files) · npm run test:package ✅ 16/16

New coverage:

• sanitizeAttachmentName — traversal (../../etc/shadow), both separator conventions, .. and dot-only names, dotfiles, control characters, Windows-refused characters, the extension/media-type mismatch that is the security case, the .log/.markdown spellings that must be kept, the length bound, and every input that must answer null.
• copyToAttachmentLibrary — first write creates the library; the same document re-attached reuses its copy; same name + different bytes keeps both (and the second document still dedupes onto its own copy afterwards); an extensionless name suffixes without inventing a dot; an unwritable library answers null instead of throwing.
• toPastedContent — sanitizes at the wire boundary, drops an unsalvageable name entirely, and never puts a name on an image block.
• pastedAttachmentsText — names the library; is byte-identical to the pre-feat(composer): non-image file attachments, restored on current main #929 note when there is no library to name (the Implement: accept PDF (plus TXT/MD) attachments in the composer, not just images #950 wording is load-bearing for backends that only see text).
• End to end through the real engine — a named .md is filed as alpha-brief.md before the run is even dequeued, both run-folder paths still reach the agent, an unnamed PDF is persisted but not filed; and neither a user image nor the agent's own tool screenshot ever enters the library.
• data-gitignore.test.ts — the two ignore-list guards, plus a floor on each regex so one that silently stopped matching cannot pass. Both go red with the attachments/ line removed (verified by reverting it).

Manual QA of the P0, in a separate consumer repo (it is unobservable inside this checkout): after cezar init on a repo that already had a .ai/cezar/.gitignore, the file self-healed — attachments/ appended, the user's own entries untouched — and a file dropped in the library gave git status --porcelain empty output with git check-ignore -v naming .ai/cezar/.gitignore:4:attachments/.

💥 Breaking Changes

None. The contract change is an additive optional key — a client that omits it behaves exactly as before, and an older cezar ignores it as unknown. The run folder's naming, the images wire key, the attachment allowlist and the composer's accept filter are all untouched. .ai/cezar/attachments/ is purely additive and entirely optional: nothing reads it, deleting it loses nothing a run depends on, and a project where it cannot be created behaves exactly as it does today.

Follow-ups (deliberately not in this PR)

• Retention/GC for the library. It grows without bound. It holds files the user handed cezar, so an automatic sweep is a product decision, not a cleanup detail — no policy is shipped here.
• Named image uploads, once the composer can distinguish a dragged diagram.png from a clipboard paste.
• Widening the accepted media types (feat(composer): non-image file attachments, restored on current main #929 took any file type). Main's curated allowlist with per-file rejection toasts is deliberate; that is a separate maintainer decision and is explicitly out of scope here.

📋 Progress

See the Progress section in the tracking plan — all steps complete.

[github-actions]

📦 npm preview published — 0.10.1-pr957.1508.2

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

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

Also tagged: npm install -g cezar-cli@pr-957 (moving tag for this PR).
Packages: cezar-cli@0.10.1-pr957.1508.2 → @open-mercato/cezar@0.10.1-pr957.1508.2 → @open-mercato/cezar-api-client@0.10.1-pr957.1508.2 (provenance attested).

[pat-lewczuk]

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

• 🔵 review — the pipeline entry state. The branch is complete, the gate is green and the PR is ready; it now needs the authoritative code-review pass before it can move on.
• ✨ feature — this adds a capability that did not exist (a per-project attachment library) rather than correcting broken behaviour. It does carry one genuine fix — the .gitignore entry that keeps user uploads out of a consumer repo's git status — but that entry only matters because this PR introduces the directory it names, so the change reads as a feature with its own guard rather than as a bug fix.
• 🧪 needs-qa — user-visible behaviour changes in a way unit tests cannot fully speak for: files a user attaches now appear in a new folder inside their project. The one failure mode that matters is invisible from inside this repository (cezar's own root .gitignore covers .ai/cezar/ wholesale), so a human should confirm the P0 against a separate consumer repo. I verified it that way myself and recorded the evidence in the summary comment; qa-approved is still someone else's to give.
• 🟠 priority-medium — nothing is broken today, so this is not release-blocking. It is above low because the feature closes a real gap in a capability that shipped one release ago (feat(composer): accept PDF, TXT and MD attachments, not just images #951): a file the user attaches is currently reachable only from the run that carried it, and only under a synthetic name.
• 🟡 risk-medium — deliberately not risk-low and deliberately not risk-high. The blast radius is small (additive optional wire key, one new best-effort writer, one ignore entry; the run folder, the media-type allowlist and the composer's accept filter are untouched) and every new path is failure-isolated. But the feature's whole job is writing a file into the user's project under a name an untrusted client supplied, which is exactly the shape that deserves a deep read of sanitizeAttachmentName rather than a skim.

[pat-lewczuk]

🤖 om-auto-create-pr completed. Status: complete — every plan step is checked, the gate is green, and the PR is out of draft.

🎯 What this is

The per-project attachment library from community PR #929 by @Damian-Szczepanski, rebuilt against current main. #929 was approved, then withdrawn by its author because they no longer maintain the branch it was proposed from; its core (non-image attachments) had already shipped independently as #951, but the library had not. This is that missing half, plus the .gitignore fix that #929's own review round uncovered.

A user attaching alpha-brief.md to a task today gets it persisted as runs/<runId>-images/pasted-1.md, reachable only from that run and under a name that says nothing. After this PR the same file is also kept at .ai/cezar/attachments/alpha-brief.md, and the agent is told the folder exists — so "use the brief I attached last week" becomes answerable.

🔍 Design decisions worth a reviewer's attention

The filename had to reach the server first, and that is the risky part. attachmentExtension derives the on-disk name from the media type alone; the composer's PendingAttachment.name carried a comment saying it is never sent. So the wire shape gains an additive optional name — and with it, sanitizeAttachmentName, which is the one place a client-supplied string may influence a path. The case that matters is not traversal (basenaming handles that) but extension pinning: a text/plain upload named install.sh must not land in the user's project looking executable, having passed a media-type allowlist that believed it was screening for exactly that. It becomes install.sh.txt.

Files only, not images — a deliberate narrowing of #929. A pasted screenshot has no filename to file it under (the chip falls back to a literal 'pasted image'), so a shared folder of pasted-3.png from forty runs would be clutter. Structurally it is also the safe choice: carrying a name on the image path means widening ContentBlock, which is the runner protocol, and an extra key there survives contentBlocksOf and reaches vendor APIs that reject unknown fields. A test pins that an image never gets a name even when a client sends one.

The agent is pointed at a directory, not per-file paths. The first draft returned a libraryPath per attachment and listed them. That is wrong on this codebase: readPersistedAttachments reconstructs an attachment from its URL alone at dequeue and restart, so the original name is already gone by then and the library line would have silently disappeared from every restarted run — the "find every construction site of a shared object" failure AGENTS.md names. Naming the folder needs no per-attachment state and is a better fit for what the library is for anyway.

Dedupe by content, not by name. A library spanning every task in a repository collects a great many notes.md. The same brief attached to six tasks leaves one file; only same-name-different-bytes takes a -2 suffix. The write uses an exclusive-create flag so two runs persisting at once cannot overwrite each other between the check and the write.

🧪 Verification

Full gate on this branch: npm run typecheck ✅ · npm run test:unit ✅ 36/36 · npm run build ✅ incl. check:pack (478 files) · npm run test:package ✅ 16/16.

npm test — 6277 passed, 6 failed / 330 files. The six are pre-existing and environmental, not this branch's. Every one of them asserts "not a git repo" / "outside a repo", and TMPDIR resolves inside a git checkout on this machine, so they see this repository's real remote. I checked out a clean origin/main into a separate worktree and ran the same six files in the same environment: the identical six cases fail there too (git-worktree, automations-api, git-changes, git, health-forge, projects-api) — not a subset, the same set. None of them touches attachments.

New coverage is listed in the PR body. Two things about it are worth calling out rather than counting:

• The .gitignore guard test goes red with the attachments/ line removed — I reverted it and confirmed, per AGENTS.md's "prove the regression test fails without the fix".
• Its scope is stated honestly in the file: it catches the literal join(dataDir, '…') shape, which is how every data path here is written, and not a module that hides its filename behind a constant. Widening it to chase constants across modules would have traded a guard that is obviously right for one that is merely clever.

📸 Manual QA of the P0 — done in a consumer repo, which is the only place it is visible

This is the part that cannot be tested from inside this repository: cezar's own root .gitignore covers .ai/cezar/ wholesale, which is precisely why the original bug was invisible. On a separate git repo, after cezar init on a checkout that already had a .ai/cezar/.gitignore:

• the file self-healed — attachments/ appended, the user's own hand-written entry left alone;
• a file placed in the library left git status --porcelain empty;
• git check-ignore -v named the rule: .ai/cezar/.gitignore:4:attachments/ → .ai/cezar/attachments/internal.pdf.

needs-qa is still on the PR: I ran this myself, but the sign-off is someone else's to give.

⚠️ What a reviewer should push on

• sanitizeAttachmentName deserves a real read, not a skim. It is the entire security surface of the feature. Self-review after the first green gate found two more holes in it — a byte bound (filesystems bound entries in bytes; 100 emoji are 400) and trailing dots/spaces, which Windows refuses — and both are landed with tests. There may be a third.
• The library grows without bound. No retention policy ships here, deliberately: these are files the user handed cezar, so an automatic sweep is a product decision rather than a cleanup detail. Worth an opinion.
• toAttachmentInput is now the only strip point between composer state and the request body. If a second composer surface ever bypasses it, preview — a full second copy of the bytes — goes on the wire. A test pins the current shape.

💥 Breaking changes

None. The contract change is an additive optional key: a client that omits it behaves exactly as before, an older cezar ignores it as unknown. The run folder's naming, the images wire key, the media-type allowlist and the composer's accept filter are untouched, and .ai/cezar/attachments/ is optional in the strongest sense — nothing reads it, and deleting it loses nothing a run depends on.

[pat-lewczuk]

🤖 om-auto-create-pr — CI result

Green. Run 33911312496: Unit, build, E2E, and package ✅ (4m23s), Publish npm snapshot ✅, CLA ✅.

Worth recording, because it settles the one open question from the summary comment: the six npm test failures I reported locally pass on CI. That is the confirmation that they are environmental rather than this branch's — on this machine TMPDIR resolves inside a git checkout, so the cases asserting "not a git repo" see a real remote. Both lines of evidence now agree: identical failures on a clean origin/main worktree here, and no failures at all on CI.

[pat-lewczuk]

🤖 om-auto-review-pr started by @pat-lewczuk at 2026-09-04T20:14:11Z. Other auto-skills will skip this PR until the lock is released.

[pat-lewczuk]

Verdict: ❌ changes requested. GitHub refuses an APPROVE/REQUEST_CHANGES state on one's own pull request, so this review is submitted as a review comment; the verdict below is authoritative and the changes-requested pipeline label reflects it.

🔍 Code Review: feat(attachments): per-project attachment library

🎯 Summary

This PR carries forward the one idea from community PR #929 that did not land with #951: a per-project .ai/cezar/attachments/ library that keeps every named document a user attaches to any task under the name they know it by, so a later task can be pointed at spec.md rather than at runs/<some-other-run>-images/pasted-3.md. It reaches that in four moves — an additive optional name on attachmentInputSchema, a sanitizeAttachmentName helper that pins the extension to the already-validated media type, a best-effort content-deduplicating writer (copyToAttachmentLibrary), and a .gitignore entry with a static test guarding the rule rather than the one entry.

Reviewed scope: the contract (packages/contract/src/runs.ts), the composer wire boundary (packages/web/src/components/composer/), the engine (packages/cezar/src/workflows/run.ts), the CLI ignore list (packages/cezar/src/index.ts), and the three test files plus the two docs.

A lot of this is genuinely well done and worth saying out loud. Pinning the extension to the validated mediaType is the right call and is the single decision that makes "write a file under a name an untrusted client gave us" safe rather than merely careful — install.sh becoming install.sh.txt closes the exact hole a media-type allowlist would otherwise be lying about. Keeping the run folder's pasted-<n>.<ext> naming untouched preserves every reader that depends on it (isImageAttachmentName, the orphan sweep, the restart re-read, the per-stack cap). Refusing to widen ContentBlock — the runner protocol — and instead putting name on cezar's own FileBlock is the correct place to absorb the change. And data-gitignore.test.ts is the best thing in the diff: it converts an invisible-from-inside-this-repo failure mode into a static cross-check that the next state directory cannot repeat.

The one thing that stops this from being an approve is that the feature's headline promise does not actually reach the agent on the default runner: the note tells the agent to look in a directory that is never added to the directories the agent is allowed to reach.

Verdict

❌ request changes — one major: the library directory is announced to the agent in the prompt but is never passed to --add-dir, so under --permission-mode dontAsk the agent's Read/Glob/Grep calls against it are denied, and the "point a later task at spec.md by name" payoff only works via an unmentioned Bash cat fallback. The three minors (an unnamed file still reaching the library as pasted.md, a lone surrogate escaping the stem truncation, and the exported writer trusting its caller for path safety) do not block on their own. Nothing here breaks a protected surface.

🧪 Validation Gate

Command	Status	Notes
npm run typecheck	✅ PASS	All four workspace projects (api-client, server, web) clean.
npm test	❌ FAIL (environmental — see below)	8 failed / 6275 passed across 330 files: git-worktree, git, git-changes, health-forge, projects-api, automations-api, automations-gate, route-parity.
npm run test:unit	✅ PASS	36/36.
npm run build	✅ PASS	Web bundle built, check:pack ok — 478 files, 85 under web/dist.
npm run test:package	✅ PASS	16/16.

On the npm test failures: I am reporting them because the gate is the gate, but I do not believe they are this branch's, and I am not raising them as a blocker. Every failing case is in a file this PR does not touch, and each asserts one of three environmental things — "not a git repository" / "outside a repo" (git, git-worktree, git-changes, health-forge, projects-api, automations-api, where TMPDIR resolving inside a git checkout on this machine makes the fixture see this repo's real remote and branch, e.g. expected 'on branch cez/c3529d71' to contain 'not a git repository'), an environment-flag-dependent scheduler assertion (automations-gate), or a 5 s testTimeout overrun under parallel load (route-parity). Two independent lines of evidence agree: the count is not even stable between runs on this machine (7 in one pass, 8 in the next, with route-parity and automations-gate moving), and CI is green on this exact head — run 33911312496 has Unit, build, E2E, and package ✅, Publish npm snapshot ✅ and CLA ✅ for 3f4eed9f. None of the eight touches attachments.

Findings

⚠️ Major

packages/cezar/src/workflows/run.ts:390 — the attachment library is never added to the directories the agent may reach, so the note points it at a path its file tools are denied.

agentDirectories(runsDir, env) returns [runsDir, TMPDIR] and is the only source of --add-dir (packages/cezar/src/core/claude-cli-runner.ts:378), applied at both spawn sites (run.ts:2570, run.ts:3181). A default run's cwd is its git worktree at .ai/cezar/worktrees/<runId> (packages/cezar/src/git-worktree.ts:83), so .ai/cezar/attachments/ is neither under the working directory nor on the granted list — it is a sibling of runs/, which is granted precisely so the per-run paths in this same note are readable. Headless runs use --permission-mode dontAsk (claude-cli-runner.ts:355), so nothing prompts: the call simply fails.

Concrete failure: a user attaches alpha-brief.md to task 1; in task 2 they attach a different file and write "use the brief I gave you last week". pastedAttachmentsText (run.ts:545) appends "Files attached anywhere in this project are also kept under their original names in /repo/.ai/cezar/attachments — look there…", the agent does exactly that with Glob/Read, and both are refused because the directory was never granted. The agent can still fall back to Bash cat, since the zero-config DEFAULT_ALLOWED_TOOLS includes unrestricted Bash — which is why this is a major rather than a blocker — but that fallback is undocumented, is not what the note steers the model toward, and is not available on Codex under CEZ_CODEX_NETWORK=0, where the sandbox is workspace-write rooted at the worktree (packages/cezar/src/core/codex-app-server-runner.ts:350). agentDirectories' own docstring is the argument against shipping it this way: handing an agent a directory its file tools are not allowed to use "would trade one silent failure for another".

Fix: widen agentDirectories to take the library directory alongside runsDir and include it in the returned list — agentDirectories(join(this.dataDir, 'runs'), attachmentLibraryDir(this.dataDir), profile.env) at both call sites — with a test asserting the library path appears in additionalDirectories for a run whose message names it. Reading the library is enough; it does not need to be writable by the agent.

🔹 Minor

packages/web/src/components/composer/composer-attachments.ts:72 — the composer's pasted.<ext> display fallback now goes on the wire, so an unnamed file is filed in the library, as exactly the numbered clutter the feature exists to remove.

fileToPendingAttachment sets name: file.name || (isImage ? 'pasted image' : \pasted.${attachmentExtension(mediaType)}`), and toAttachmentInput (composer-attachments.ts:85) sends namefor every non-image, the fallback included. A file that arrived with no name of its own therefore reaches the server aspasted.md, passes sanitizeAttachmentNameunchanged, and is filed aspasted.md— and the next one aspasted-2.md, then pasted-3.md. That contradicts fileInAttachmentLibrary's own contract (run.ts:3551): "A file that arrived without a usable name is left out — the library exists to be browsable by name, and pasted-3.mdis exactly what it is an answer to." The e2e test that asserts an unnamed PDF is not filed constructs theFileBlock` directly, so it never exercises this path and the gap passes the suite.

Fix: send the name only when the file actually had one — ...(isImage || !file.name ? {} : { name }), by carrying the real name separately from the chip's display fallback (for example an originalName?: string on PendingAttachment that toAttachmentInput reads), plus a toAttachmentInput case for a nameless non-image file.

packages/contract/src/runs.ts:739 — the character-based stem slice runs before the byte truncation and can leave a lone surrogate, which is the one thing truncateToBytes documents that it prevents.

truncateToBytes(rawStem.slice(0, MAX_ATTACHMENT_NAME_STEM), …) applies String.prototype.slice, which counts UTF-16 code units, so it can cut an astral character in half before truncateToBytes ever sees it; the for…of guarantee in that helper's docstring ("a surrogate pair is never cut in half into a lone surrogate") then preserves the half rather than preventing it. Verified against this branch: sanitizeAttachmentName('a'.repeat(97) + '😀😀' + '.md', 'text/markdown') returns a stem whose last code point is an unpaired high surrogate (the first 99 units are only 104 UTF-8 bytes, well under the 180-byte budget, so nothing downstream trims it). Node writes that to the filesystem as U+FFFD, so the cost is a library entry ending in � rather than a crash or a traversal — but the existing emoji test passes only because '😀'.repeat(100) happens to cut on an even boundary.

Fix: slice on code points before the byte pass — [...rawStem].slice(0, MAX_ATTACHMENT_NAME_STEM).join('') — or drop the character bound entirely and let truncateToBytes be the single bound. Either way, add the odd-boundary case ('a'.repeat(97) + '😀😀') to the existing byte-bound test, since the current input cannot reach the bug.

packages/cezar/src/workflows/run.ts:457 — copyToAttachmentLibrary is exported and takes name as a bare string, so path safety lives entirely in a caller two modules away.

Nothing is wrong today: toPastedContent (run.ts:510) is the only producer of FileBlock.name and it sanitizes there, and all four attachment-carrying routes go through it (server.ts:3546, :3691, :3767, :3824). But FileBlock.name is typed as a plain optional string and copyToAttachmentLibrary is a public export of the module, so a fifth route or a future caller that builds a FileBlock directly would hand an unsanitized client string straight into join(dir, candidate) with nothing in between — and the failure would be a path traversal, not a type error.

Fix: make the boundary local to the function that does the writing. A cheap version is a guard at the top of copyToAttachmentLibrary that refuses a name containing a separator or a leading dot (if (name !== basename(name) || name.startsWith('.')) return null;); a stronger one is a branded SanitizedAttachmentName type returned by sanitizeAttachmentName and required by FileBlock.name, which makes the invariant checkable by tsc instead of by convention.

💅 Nit

packages/cezar/src/workflows/run.ts:545 — the note's wording promises more than the writer delivers. "Files attached anywhere in this project are also kept under their original names" is not true of images or of files that arrived without a usable name, both of which are deliberately excluded. An agent reading it literally will hunt for last week's pasted screenshot and find nothing. "Documents (PDF, TXT, MD) attached anywhere in this project…" would describe what actually lands there.

packages/cezar/src/data-gitignore.test.ts:41 — wantedEntries() matches '([^']+)' across the whole wanted block, comments included. The comments there have no apostrophes today, so the guard is correct as written; but a future comment containing "the user's git status" would pair quotes across prose and inject phantom entries, silently making the cross-check more permissive — the one direction this test must not fail in. Stripping // lines from the captured block before the match keeps it honest for the cost of one replace.

packages/contract/src/runs.ts:719 — Windows reserved device names survive sanitization. sanitizeAttachmentName('CON.txt', 'text/plain') returns CON.txt, which on Windows names the console device regardless of extension, as do NUL, PRN, AUX, COM1–COM9 and LPT1–LPT9. The helper handles every other Windows filename rule explicitly — refused characters, trailing dots and spaces — so this is the one gap in an otherwise complete set. The write is best-effort, so the cost is a missing or bizarre library entry rather than a failure, and a user would have to attach a file literally named CON.txt; suffixing a match (CON-.txt) closes it in one line if you think it is worth the line.

💥 Breaking Changes

• No exported/public symbol removed or renamed without a deprecation path — sanitizeAttachmentName, attachmentLibraryDir and copyToAttachmentLibrary are new exports; nothing was removed.
• No function signature changed in a breaking way — pastedAttachmentsText, pastedAttachmentsNote and toPastedContent each gained an optional parameter or an optional property, so every existing call site still compiles and behaves identically.
• No required type field removed or narrowed — FileBlock.name and AttachmentInput.name are both optional additions.
• No HTTP route URL removed or renamed; no method changed — the route table is untouched.
• No field removed or retyped in an existing response shape — the change is request-side only.
• No event or message name renamed or removed — ContentBlock is deliberately not widened, which is the point of the image-branch comment at run.ts:515.
• No CLI command or flag renamed or removed — ensureDataGitignore only appends an entry, and appends it to existing files too, so old installs self-heal.
• No database table or column renamed or removed — not applicable.
• No config key renamed and no default changed silently — .ai/cezar/attachments/ is new state, documented in BACKWARD_COMPATIBILITY.md §3.
• Where a contract had to change: old surface kept working — a client that omits name behaves exactly as before, and an older cezar drops it as an unknown key since attachmentInputSchema is not strict.

BACKWARD_COMPATIBILITY.md was updated in the same diff for both surfaces (§2 for the additive name, §3 for attachments/ and the sharpened .gitignore rule), which is what the policy asks for. No protected surface is violated.

🧪 Test Coverage

Coverage on the new code is strong, and specific rather than ceremonial. sanitizeAttachmentName is exercised on the cases that matter: traversal on both separator conventions, .. and dot-only names, dotfiles, control characters, the Windows-refused set, trailing dots and spaces, the length bound in characters and in bytes, the .log/.markdown spellings that must survive, every input that must answer null, and — the important one — the extension/media-type mismatch (install.sh → install.sh.txt, payload.exe → payload.exe.pdf). copyToAttachmentLibrary covers first write, same-document reuse, same-name-different-bytes suffixing including the second document deduplicating onto its own copy afterwards, an extensionless name, and an unwritable library answering null instead of throwing. toPastedContent is pinned at the wire boundary in both directions — it sanitizes a file's name, drops an unsalvageable one entirely, and never puts a name on an image block. The end-to-end test through the real engine proves a named .md is filed before the run is dequeued, that both run-folder paths still reach the agent, and that neither a user image nor an agent tool screenshot ever enters the library. pastedAttachmentsText is pinned byte-identical to the pre-#929 wording when there is no library, which protects the #950 text for backends that only see text. data-gitignore.test.ts guards the ignore rule statically with a floor on each regex so one that stopped matching cannot pass silently.

The gaps line up with the findings above, and each needs a test rather than only a fix:

1. The agent's granted directories. Nothing asserts that the directory the note names is one the agent may read. Add a case alongside the existing spawn-spec tests that starts a run with a named file attachment and asserts attachmentLibraryDir(dataDir) appears in the spawn's additionalDirectories — that is the assertion that would have caught the major.
2. The composer's nameless-file path. The e2e "unnamed PDF is not filed" case builds the FileBlock by hand, so it does not cover what the composer actually sends. Add a toAttachmentInput case for a non-image File with an empty name asserting no name goes on the wire, which is where the fallback leaks.
3. The odd-boundary surrogate. The byte-bound test uses '😀'.repeat(100), which cuts cleanly at unit 100 and so cannot fail. Add 'a'.repeat(97) + '😀😀' and assert no code point of the result falls in U+D800–U+DFFF.
4. Concurrent filing across processes. copyToAttachmentLibrary's exclusive-create plus content compare is safe within one process because the writes are synchronous, but two cezar processes on the same repo can have the second read a partially written file, miss the dedupe and write a redundant -2 copy. Not worth fixing given the best-effort contract, but worth a comment saying the guarantee is single-process.

Unbounded growth of the library is called out as a deliberate follow-up in the PR description, along with named image uploads and widening the accepted media types. I agree those belong outside this PR; retention in particular is a product decision about user-supplied files rather than a cleanup detail.

[pat-lewczuk]

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

• 🚀 merge-queue — the re-review of 15a1db9a approves. origin/main is merged in and the head is MERGEABLE, the one major this pass found (a restored draft dropping the attachment's filename, at the seam with Implement: in-task drafts survive leaving the task — restore unsent composer text and attachments #939) is fixed and covered by two regression tests, and the local gate passes apart from the six nested-worktree environment failures in files this PR does not touch. Not blocked: nothing is failing — what remains are the two human gates below, and neither is a defect in the change.
• 🧪 needs-qa — retained. User-facing behavior changed (what a file attachment is called, and what survives leaving a task), skip-qa is absent and qaGate is on, so the merge stays gated until a QA reviewer adds qa-approved. This run attaches no QA labels of its own.
• ✨ feature — a capability that did not exist: a per-project library of the files a user attaches, filed under their own names.
• 🟡 priority-medium — an ordinary feature. Nothing is broken without it and no release waits on it.
• 🟡 risk-medium — it touches the attachment wire contract and the agent's granted directories, which is more than a leaf change, but every surface is additive and covered by tests.
• 🔄 in-progress — retained, not a stale claim: the om-auto-fix-pr chain that invoked this review still holds the lock and will release it when it reports.

[pat-lewczuk]

🤖 om-auto-fix-pr — merge-ready summary

Driven against a main that had moved again. Head is now 15a1db9a.

Base merged (80cd887d) — four conflicts, the substantive one being that main extracted ensureDataGitignore into its own module; this branch adopts that structure and folds attachments/ + dispatch/ into the exported entry list. A plain merge would also have kept this branch's earlier accidental deletion of main's dispatch?: DispatchIntent doc bullet; it is restored. Details in the re-review.

One major found and fixed (15a1db9a) — where this feature meets in-task drafts (#939), a restored draft attachment lost the filename: attach alpha-brief.pdf, leave the task, come back, send, and the library filed pasted.pdf while the chip still read alpha-brief.pdf. Two regression tests; the named-file one was confirmed to fail without the fix.

UI verified in a real browser against a production build of this head — screenshots, including the leave-and-return round trip now filing beta-brief.md. Evidence-only: no QA labels were set.

Deferred, not dropped: the three optional findings from the earlier re-review are now tracked as #987, #988 and #989 (with #959 and #960 from the first pass).

CI green on this head — Unit, build, E2E, and package ✅, Publish npm snapshot ✅, CLA ✅. One earlier red was a confirmed flake in test-env-launcher.test.ts (failed once, passed on a plain re-run of the same commit, nothing changed to make it pass); it is unrelated to this diff and recorded on #898. Details in the CI result comment.

Merge-ready as far as automation goes. Next: om-approve-merge-pr 957 — which will still refuse until the needs-qa sign-off lands, and GitHub will still require an approving review from a second account.

[pat-lewczuk]

🤖 om-auto-fix-pr — CI result

Green on 15a1db9a — Unit, build, E2E, and package ✅, Publish npm snapshot ✅, CLA ✅. Branch protection is not readable on main (404 Branch not protected), so every reported check was treated as required.

The earlier red was a confirmed flake, not a fix: test-env-launcher.test.ts → "survives its caller and stops by descriptor PID (setsid)" failed once with The input did not match /TEST_ENV_REUSED=1/, then passed on a plain re-run of the same commit. Nothing was changed to make it pass. It is unrelated to this diff — neither the test nor the script it exercises is in the change — and the evidence is recorded on #898, which already owns that try_reuse() surface.

The verdict is unchanged: the PR keeps merge-queue. What still holds the merge is human — an approving review from a second account (GitHub refuses self-approval) and the needs-qa sign-off. ci-monitoring is removed; this agent owes no further follow-up.

[pat-lewczuk]

🤖 om-auto-fix-pr taking over the chain lock — driving this PR to merge-ready after the latest main advance. Started: 2026-09-14T20:42:38Z.

[pat-lewczuk]

🤖 om-auto-review-pr taking over the chain lock — re-review of the merged head 80cd887d. Started: 2026-09-14T20:46:22Z.

[pat-lewczuk]

Verdict: ✅ approve (re-review of 15a1db9a). GitHub refuses an APPROVE/REQUEST_CHANGES state on one's own pull request, so this is submitted as a review comment; the verdict below is authoritative and the merge-queue label reflects it.

🔍 Code Review (re-review): feat(attachments): per-project attachment library

The feature itself is byte-identical to the approved re-review of 3cce047a — contract/src/runs.ts, workflows/run.ts and composer-attachments.ts have not changed since. What is new is the integration with a main that moved underneath it: two merges (80cd887d, 445f564d) and the dispatch/ ignore entry (e66d71c5). So this pass reviewed the merge seam, which is where the one finding came from.

💥 One major, found and fixed in this pass

A restored draft dropped the filename the whole feature is about — packages/web/src/routes/task-thread/thread-draft.ts:361, fixed in 15a1db9a.

In-task drafts (#939) landed on main while this branch was open, and the two features disagree about names. The draft store keeps one name per blob, the chip's label. This PR gives the composer two: that label, and originalName — the name the upload actually carried, which is the only one toAttachmentInput puts on the wire and the only one the library files a copy under.

Restoring the label alone lost the distinction, and lost it silently:

1. attach alpha-brief.pdf to a task's composer;
2. leave the task and come back — the draft restores, which is the entire point of #939;
3. send.

The chip still read alpha-brief.pdf, but the message carried no name, so the library filed another pasted.pdf — the numbered clutter this PR exists to replace. Nothing was lost from the run folder; what was lost is the feature's payoff, on an ordinary path, without a symptom a user could report accurately.

The restore now asks again whether the stored label is a real filename or the generated pasted.<ext> fallback, comparing against fallbackAttachmentName — the one function that produces it, now exported and used by both sides so they cannot drift. Two regression tests pin it (a named file keeps its name on the wire; a restored paste still sends none), and the named-file one was confirmed to fail without the fix.

🔁 Merge resolution

Four conflicts, resolved in 80cd887d:

• data-gitignore — main extracted ensureDataGitignore out of index.ts into its own exported module. This branch adopts that structure rather than keeping its copy, and folds attachments/ and dispatch/ into DATA_GITIGNORE_ENTRIES.
• data-gitignore.test.ts — main's behavioural tests against the exported function are kept, and this branch's static coverage scan is ported on top of them, reading the entry list from the constant instead of regex-scraping index.ts. That scan is what forced dispatch/ into the list: main writes .ai/cezar/dispatch/ and never ignored it, so the guard this PR adds caught a gap that predates it.
• composer.tsx — both sides added an import; both kept.
• BACKWARD_COMPATIBILITY.md — both route bullets kept, and main's dispatch?: DispatchIntent bullet restored: this branch's earlier merge (445f564d) had dropped it, and a plain merge would have kept the deletion silently.

Findings

No blockers. The major above is fixed. The three optional items from the previous re-review are unchanged and still optional — none should hold the merge; each is now filed as a follow-up rather than repeated here.

🧪 Validation gate

Run in an isolated worktree at 15a1db9a, dependencies installed from package-lock.json.

Command	Result
npm run typecheck	✅ pass
npm test	⚠️ 6856/6862 — 6 environment failures, below
npm run test:unit	✅ pass
npm run build	✅ pass
npm run test:package	✅ pass

The six npm test failures are not this PR's — the same six as every previous pass (git-worktree, automations-api, git-changes, git, health-forge, projects-api), each asserting "not a git repository" for a temp directory and each instead resolving /home/cezar/cezar on branch cez/c3529d71, the enclosing checkout, because this worktree is nested inside the repository. None of those files or the modules they exercise are in this diff, and CI is green on them.

🚦 Merge signals

• Conflicts: none — origin/main is merged in and the head is MERGEABLE.
• CI: Unit, build, E2E, and package is IN_PROGRESS on 15a1db9a at the time of this review (checks). Required CI and QA approval still gate merge. A follow-up will report the CI result.
• QA: needs-qa without skip-qa and qaGate on, so the merge stays gated until a reviewer adds qa-approved. Code-review approval is not QA approval. The draft round trip above is worth a click in manual QA, since it is the path that was broken.

[pat-lewczuk]

🤖 om-auto-review-pr completed: APPROVED. Lock retained — chain continues.

Re-review of 15a1db9a — the review. The feature code is unchanged since the last approval; this pass reviewed the merge seam with the new main, and found one major there: in-task drafts (#939) store only the chip's label, so a restored file attachment sent no filename and the library filed it as pasted.<ext> while the chip still showed the real name. Fixed in 15a1db9a with two regression tests (the named-file one verified to fail without the fix). The three optional items from the previous re-review are unchanged and are being filed as follow-ups.

Next in the chain: follow-ups, then CI.

[pat-lewczuk]

📸 om-auto-qa-pr — UI verification (evidence only)

Verdict: ✅ PASS on 15a1db9a, driven through a real browser against a production build of this PR's head (CEZ_DRY_RUN=1, so the agent CLIs are mocked). No labels were changed — a QA reviewer still owns the sign-off.

The run targeted what the diff actually changes: what a file attachment is called, and — since 15a1db9a — what survives leaving a task and coming back.

#	Step	Expected	Observed
1	New-task composer: attach alpha-brief.md	chip shows the real filename	chip alpha-brief.md	✅
2	Start the task with it attached	library files it under its own name; run folder keeps legacy numbering	.ai/cezar/attachments/alpha-brief.md; run folder pasted-1.md	✅
3	Thread composer: attach beta-brief.md + text, do not send	chip and text held as a draft	both present	✅
4	Leave to the task list, then return	draft restores; nothing lingers while away	absent while away, chip + text back on return	✅
5	Send the restored draft	library gets beta-brief.md, not pasted.md	alpha-brief.md, beta-brief.md	✅
6	Re-attach the same file and send again	one copy, not a duplicate	library unchanged	✅
7	Narrow viewport (390×844)	composer usable	renders and takes focus	✅
8	A restored nameless paste must send no filename	no pasted.md in the library	⚠️ not exercised — a clipboard paste is not drivable through the provider; covered by the unit test in thread-draft.test.tsx	⚠️

Steps 3–5 are the path 15a1db9a fixed. Before it, step 5 filed pasted.md while the chip in step 4 still read beta-brief.md — the chip telling the truth while the wire did not.

Coverage limits. CEZ_DRY_RUN=1 mocks the agent CLIs, so the agent's own read of .ai/cezar/attachments/ was not exercised end to end here — the manual QA instructions keep that as P1, along with the hostile-filename P0s, which stay a human job. Chrome needed --no-sandbox and the provider's user-local dependency path on this host (no root available); that is an environment quirk, not a finding.

01 new task named chip

02 task created library filed

03 draft attached before leaving

04 draft restored after return

05 restored draft sent

06 narrow viewport

[pat-lewczuk]

🧪 Follow-up UI test — ready to implement

This change ships no browser-level test for the path that broke. packages/web/e2e/thread-drafts.e2e.ts (#939) already drives the draft round trip for text; the attachment half of it is untested, and the attachment half is where the filename is lost.

Where: extend packages/web/e2e/thread-drafts.e2e.ts — the fixture, the runner and the two-task navigation it needs already exist there.

Scenario: in task A's thread composer, upload a fixture file with a distinctive name (alpha-brief.md) and type a line of text → open task B → return to task A → assert the chip still reads alpha-brief.md and the text is back → send → assert the message's attachment carries name: "alpha-brief.md" (the API response or .ai/cezar/attachments/ in the fixture repo, whichever the suite already reaches for).

The assertion that matters: the wire name, not the chip. The chip was right the whole time this was broken — it read alpha-brief.md while the message shipped no name at all — so a test that only checks the visible label passes on the bug.

Negative case: a restored attachment whose stored label is the generated pasted.md fallback must send no name. Covered at unit level in thread-draft.test.tsx; worth mirroring here only if the suite makes it cheap.

Evidence for the manual pass this replaces is in the QA comment above. Filed as evidence only — no tracking issue opened.

[pat-lewczuk]

🤖 om-auto-qa-pr completed: PASS. Lock retained — chain continues.

UI verified in a real browser against a production build of 15a1db9a; evidence with screenshots. Evidence-only mode: no QA labels were set, so needs-qa stands and a QA reviewer still owns the sign-off.

[pat-lewczuk]

🤖 om-auto-fix-pr completed: MERGE-READY. Lock released.

Base merged (80cd887d), one major found and fixed at the seam with in-task drafts (15a1db9a), UI verified in a real browser with screenshots, all three checks green. Three optional findings deferred as #987, #988, #989; the one red CI run was a confirmed flake, recorded on #898.

Remaining gates are both human and neither is a defect in the change: an approving review from a second account (the repo's ruleset requires one and GitHub refuses self-approval), and the needs-qa sign-off. Then: om-approve-merge-pr 957.