Treat all uploaded files as normal workspace files

[deregtd]

User scenario

A user uploads a CSV, image, PDF, archive, or other file and asks an agent to work with it. The file should have a stable workspace path, be usable through ordinary file APIs, and eventually materialize into a container alongside repository files. Downloading the original upload must still return the original bytes after the workspace copy is edited.

Previously attachments lived in a separate upload store and model-input path rather than the workspace's Git-backed file model.

What changes

Every new upload becomes a file in one shared chat-private SCRATCH worktree, regardless of MIME type or the selected model's input support. Agent-created plans and other scratch files can share that worktree.

• Valid text keeps ordinary text OTs. Other files use immutable Git-blob references and whole-file binary replacement. All payloads are ordinary zlib-compressed Git blobs in the existing workspace gitObjects store, regardless of their origin; there is no separate chunk store or binary-diff format. Byte APIs transfer raw bytes, not base64 payloads embedded in OTs.
• Upload projection and the user message commit atomically. Names avoid existing files/directories, preserve Unicode, and respect Linux filename-byte limits.
• Original downloads/replay remain immutable. Images are not resized or transcoded. Legacy inline attachments migrate lazily, one file at a time, with reset-safe progress and original downloads preserved.
• The model receives only compatible inline content; other files are identified by workspace path. The code browser retains binary paths without treating them as editable text.
• Ordinary byte-aware worktree APIs and Git pack export carry these files without a separate container attachment channel.

Attachments are limited to 4 MiB per file, 5 MiB per message, and five files. Original images must now fit the 4 MiB file limit: the old path that accepted source images up to 25 MiB and resized them is intentionally removed. Raw Git-blob ingress is bounded to 8,000,000 bytes; general large-file support is deferred.

Discarded uploads can leave unreferenced blobs in the shared Git store. Retention/quota is explicitly a follow-up: deleting a blob on chip removal is unsafe because another message or worktree may reference the same OID.

Boundary and dependencies

Targets main independently. This branch includes the shared whole-file binary foundation reused from #759; it does not require merging #759 first. Binary diffs are not included. Container replica mutations remain in the replica/binary follow-ups, not this upload-ingress PR.

No container engine, lifecycle, replica lease, network policy, or provider-bootstrap implementation. Backend/shared and frontend changes are separate commits for review.

Verification

• Full lint, generated-config checks, and uncached workspace build/type checks passed on the latest pushed revision.
• Backend: 1,466 tests passed, excluding the previously reproduced failures in unchanged main's ai-models suite. RPC integration: 193 passed, with one expected failure.
• Earlier frontend verification covered 1,158 tests, including an isolated rerun of 14 integration tests with the reused dependency symlink accommodated.
• Coverage includes initial/subsequent uploads, maximum-size text, malformed and multibyte filenames, immutable originals, byte-exact Git pack transfer, model delivery, and real DO eviction.

Extracted from #729 and since expanded to all uploaded files, not just text. No live deployment or feature enablement.

[github-actions]

Preview: pr770-ddr-split-att-0a5c6611

https://pr770-ddr-split-att-0a5c6611-router.cloudflare-os-previews.workers.dev

Dashboard · deleted when this PR closes

[ask-bonk]

Review: 2 findings.

Posted two actionable inline comments. Verified both with code inspection and focused Node checks.

The backend test suite was not run because dependencies are not installed.

github run

[ask-bonk]

Review: 1 finding.

Posted one actionable inline comment. Both earlier findings are fixed.

Focused byte-storage checks passed for 4 MiB payloads, cross-chunk reads, scoped grants, resumed uploads, and empty files. The backend suite was not run because dependencies are not installed.

github run

[kentonv]

I like the idea of putting attachments in a worktree.

I wonder if env.ATTACHMENTS is actually what we want to call it. Some other ideas:

• The idea of a general "scratch space" worktree has come up a few times. It could also be used for the agent to write plan files, for example. Should we have just one tree for multiple purposes here?
• Or going the other direction, why have a single worktree? Maybe each time attachments are ulpoaded, we should put them in their own new worktree? This is more consistent with how capsules work (gatekeepers introduced in chat messages). Worktrees are super-cheap, there's actually not much gained by reusing a worktree, and then you can get rid of the collision suffix logic.

[deregtd]

I like the idea of putting attachments in a worktree.

I wonder if env.ATTACHMENTS is actually what we want to call it. Some other ideas:

• The idea of a general "scratch space" worktree has come up a few times. It could also be used for the agent to write plan files, for example. Should we have just one tree for multiple purposes here?
• Or going the other direction, why have a single worktree? Maybe each time attachments are ulpoaded, we should put them in their own new worktree? This is more consistent with how capsules work (gatekeepers introduced in chat messages). Worktrees are super-cheap, there's actually not much gained by reusing a worktree, and then you can get rid of the collision suffix logic.

So longer-term my SWAG is that we're going to want to be caching per-worktree, not per-container, for network volume attach. So having these "volumes" at a useful fidelity to be individual units attached to multiple containers is kinda where I'm roughly aiming. Having one growing "attachment worktree" gets closer to that being a useful level of fidelity without making too many of them to attach to a container longer term. So I landed here for now. Definitely open to pushback on that, but basically I'm just pulling out my divining rod and thinking that in the future, worktrees won't be quite as free as they are today, for the sake of faster launch times.

[ask-bonk]

Review: 2 findings.

Posted two actionable inline comments, verified with focused Node reproductions and provider documentation. The three earlier findings are addressed.

The full test suites were not run because dependencies are not installed.

github run

[ask-bonk]

LGTM!

github run

[Maximo-Guk]

🤖 Some more GPT comments:

1. P2 Large text uploads lose text-tool access in an existing scratch worktree
   overseer.ts:5298–5302 (packages/workshop-backend/src/overseer.ts#L5298-L5302)
   When valid UTF-8 text exceeds the OT budget, this stores it as a BytesRef. Text readers then treat that reference as binary rather than decoding its contents. Reproduced with a 4 MiB text file uploaded after a small initial attachment: readFile(..., lineCount: 1) fails with “Binary content requires byte operations.” The identical file is readable when uploaded first. Chunk-backed text should preserve text-read semantics, including windowed reads and grep.
2. P2 Replay rejects binary content before applying its pending text replacement
   agent.ts:2303–2309 (packages/workshop-backend/src/agent.ts#L2303-L2309)
   If an agent calls writeFile to replace a byte-backed attachment with text, then calls readFile in the same step, both calls succeed live. On the next turn, replay calls requireTextContent on the old binary value before applying pendingReplayEdits, so the previously successful read becomes an error and its content disappears from the model’s history. Apply pending whole-file replacements before requiring a text value.

[deregtd]

Independently reproduced both issues in the report above at c46ad806c4274a4be6313c7c899c19f08cd0712f with two passing regression probes in the actual Workers runtime (real Overseer DO and agent loop; faux model, no live provider):

• A 4 MiB valid UTF-8 file with a short first line is readable through readFile({lineCount: 1}) when uploaded first. Uploading the identical bytes after a small initial attachment makes that call fail with Binary content requires byte operations.
• Replacing a byte-backed attachment with writeFile and reading it in the same step returns the replacement text live. After the next user turn, replay changes that successful read into the binary-content error.

These remain functional blockers despite green CI. Please retain text-tool access for the large-text fallback and apply pending whole-file replacements before replay requires text. The probes cover those paths directly; they do not exercise a live model/provider.

[deregtd]

Re-reviewed e175ced5488c395d450b7669a350e9db9077c326 against 7b0ac75ebcfa53292d56245f288a27f11831aa6d. The rename race/wording changes and documentation updates are covered.

I independently reproduced the existing many-image finding through the actual Overseer and runAgent in workerd: five turns uploading 5/5/5/5/1 valid 4000×1000 PNGs produce model inputs with 5/10/15/20/21 images, all retaining the original bytes. The fifth input therefore exceeds Anthropic’s documented dimensions for requests with more than 20 images. The guard needs to account for the complete replayed input, or use a compatible projection/fallback while preserving workspace originals. This probe uses a faux model; it does not claim a live provider request.

Validation: 126 focused Workers tests, 20 frontend tests, the independent replay probe, and backend/frontend type checks passed. No duplicate inline finding added; this existing issue still blocks readiness.

[deregtd]

Review follow-up:

• Rebased onto main at 9ae2f05. Conflict resolutions preserve main's new transform-validation diagnostics and this PR's byte validation; the range diff shows no unrelated changes. Published to ddr/split-attachment-worktree with an explicit force-with-lease; the final fixes are in 940716e.
• Answered and resolved the five older inline threads: lazy legacy migration exists; chunk sizing and per-chunk sync are obsolete because the entire chunk/grant store is gone; committed-byte resolution now lives in WorkspaceGitCache; attachment reservations use cached current chat content.
• Kenton's top-level storage feedback is implemented: one ordinary compressed Git object store for all origins, no chunking or grants. Generic large-object support is deferred. One shared SCRATCH worktree per chat is the selected layout.
• Maximo's top-level text-access/replay findings are already covered: blob-backed UTF-8 supports windowed text reads and grep, and replay applies pending whole-file text replacements before rejecting old binary content. Worker-level initial/follow-up tests cover both cases.
• The latest migration warning is valid and fixed in 940716e: bounded per-file retries, continued progress past a broken attachment, no permanent skip flag, explicit unavailable-content replay markers, and retry after a source is repaired. Tests include real DO eviction and a lost acknowledgement after durable publication, with no duplicate projection.
• The many-image replay limit is fixed in 940716e and its inline thread is resolved: candidate images/documents are counted across the entire replay, and requests with more than 20 candidates retain only images within 2000 px per edge as inline inputs. Original attachment bytes remain unchanged.
• The second #pinBase() is a post-await liveness/revocation check; added that explanation. The before/after overlay checks are intentionally kept local rather than introducing another helper for a few lines.
• Retention/quota stays a follow-up, as explicitly agreed. The optional AGENTS pointer is omitted to keep global instructions small; docs/attachment-worktrees.md remains the detailed design reference. A model-eval harness extension for seeding attachments and verifying worktrees/downloads is also deferred; the current deterministic real-Worker tests exercise these paths, while the existing generic gadget evals do not.

Validation: full uncached workspace build, lint, script types, generated-config and diff checks passed; backend 1,503 tests passed excluding only the previously reproduced unchanged ai-models baseline failures, followed by an 81-test attachment rerun including the additional lost-ACK case; 68 focused frontend tests and 34 real WebSocket RPC lifecycle/worktree/authorization/recovery tests passed. The backend's separate integration suite also passed (3 tests, 4 pre-existing skips).

[ask-bonk]

LGTM!

github run

[deregtd]

Re-reviewed 940716e1 against base 9ae2f05f, including the real diff after the rebase. The image-limit finding is fixed: an independent real-Worker probe using valid 4000×1000 PNGs produces image counts 5/10/15/20/0 across successive turns, while all 21 original downloads remain byte-exact. The migration tests also cover continued progress with an unavailable legacy payload, retry after repair/eviction, and a lost acknowledgement after durable publication.

146 focused Workers tests, 20 frontend tests, and backend/frontend type checks passed locally with codegen prerequisites. No additional blocking finding from this review. The remaining documentation wording and retention/quota suggestions are nonblocking follow-ups; required Build and test CI is still running. No live model/provider call was made.

[ndisidore]

Clever! I could see a future here were files over the 8MB limit still get a blob OID but they get chunked rows in R2 keyed by OID

[kentonv]

🤖 Written by an AI agent:

[low · bug] Leftover "attachments worktree" naming for what is now the shared SCRATCH worktree

packages/workshop-backend/src/overseer.ts:5361

Requested by: 🤖 AI summary agent (noticed while summarizing the PR)

The chat's upload worktree was renamed from env.ATTACHMENTS (earlier iteration, #729) to the shared env.SCRATCH worktree. It now also holds plans and other agent working files. Code, comments, docs and tests still describe it as the "attachments worktree", which misleads readers about its scope. Rename consistently (e.g. "scratch worktree"):

Persisted storage field (decide before merge; main has never had it, so no data migration is needed):

• AiChatAgentContext.attachments: {worktreeId, bindingName} in storage-schema/overseer-storage.ts (~line 591). Consider e.g. scratch. It is read in overseer.ts (#attachmentsWorktree, #installAttachmentsWorktree, chatScopeNames ~7469, the seed ~7635, the seeded-bindings listing ~7891) and in agent.ts ~2200 (agentContext.attachments.bindingName).

Private code identifiers in overseer.ts:

• type PreparedAttachmentsWorktree (~246)
• #attachmentsWorktree (~5361), #prepareAttachmentsWorktree (~5378), #installAttachmentsWorktree (~5388)
• Locals attachmentsWorktree / preparedAttachmentsWorktree in newChat (~6444) and sendChatMessage (~6527–6565), and the attachmentsWorktree parameter of #commitPreparedChatMessage (~6358)

User-visible / comment text:

• Error "Attachment worktree changed while preparing the message; retry." (overseer.ts ~5421)
• Doc comment on workspacePath, "Path in the chat's attachments worktree…" (overseer-storage.ts ~850)

Docs / PR description:

• docs/attachment-worktrees.md line 10, "Existing attachment worktrees keep their binding names.", and the PR description's "existing attachment worktree binding names remain valid". On main no such worktrees ever existed, so this compatibility claim refers only to the unmerged earlier iteration. Drop it, unless that iteration was deployed somewhere with real data. The doc's filename/title ("Attachments in worktrees") could arguably follow too.

Tests (__tests__/attachment-worktrees.test.ts):

• Suite title "attachment worktree ingress" (line 43) and the assertion message "Expected attachments worktree" (line 301).

Not affected: MAX_CHAT_ATTACHMENTS_PER_MESSAGE / MAX_COMPOSER_ATTACHMENTS are per-message attachment counts, which is correct usage.

[deregtd]

Addressed the naming cleanup from Kenton’s request in ad8bec3 (committed and pushed):

• Renamed AiChatAgentContext.attachments to scratch, together with the preparation/install helpers, local variables, seeding/catalog paths, and replay lookup.
• Updated worktree errors, storage comments, and test terminology. Attachment-specific message counts, payload metadata, and legacy-attachment migration names remain attachment-specific.
• Renamed the design document to docs/chat-scratch-worktree.md and updated all repository links. Removed the earlier prototype’s binding-name compatibility claim from both the document and this PR’s description. No compatibility alias/migration for that unmerged schema is added.

This is naming-only: one shared chat-private SCRATCH worktree still holds uploads, plans, and other working files. Validation: all 1,504 backend tests passed (excluding only the previously established unrelated ai-models baseline suite), including real DO eviction/retry coverage; backend/browser type checks, the full pnpm lint (including the uncached 84-task workspace build), and diff checks passed.

[ask-bonk]

LGTM!

github run

[github-actions]

Eval results

Verdict: ⚪ Unchanged. No task moved beyond what 10 runs can tell apart from noise.

Task	Score	Δ score	Fisher test	Cache hits	Avg min	Avg steps
change-calendar	100%	0 pp	p = 1.00	95% 0 pp	4.1 → 3.8	30.0 → 27.5
chess	80% → 100%	+20 pp	p = 0.47	98% 0 pp	8.8 → 9.7	53.3 → 69.3
incident-desk	100%	0 pp	p = 1.00	96% 0 pp	4.3	31.1 → 31.3
worker-logs	80% → 90%	+10 pp	p = 1.00	95% 0 pp	3.7 → 4.1	22.4 → 27.2

Failed checks

Task	Check	Failed
chess	t1 agrees-with-the-oracle-through-random-games	2/10 → 0/10
chess	t1 starts-from-the-standard-position	1/10 → 0/10
chess	t1 agrees-with-the-oracle-on-the-hard-positions	1/10 → 0/10
chess	t1 agrees-with-the-oracle-on-perft-positions	1/10 → 0/10
chess	t1 the-game-is-shared-across-connections	1/10 → 0/10
worker-logs	t1 ingests-resets-and-summarises-per-worker	1/10 → 0/10
worker-logs	t1 hourly-buckets-cover-every-hour-including-empty-ones	1/10 → 0/10
worker-logs	t1 ranges-are-half-open-and-filter-by-worker	1/10 → 0/10
worker-logs	t2 breaks-down-by-colo-and-ranks-slow-routes	1/9 → 1/10
worker-logs	t2 existing-data-and-summary-survive	0/9 → 1/10
worker-logs	t2 filters-combine-across-colo-route-and-worker	0/9 → 1/10
worker-logs	t2 reset-and-ingest-still-work	0/9 → 1/10

Run · trajectories and raw results

[ask-bonk]

🔬 Eval runs review

Performance

Across 10 runs per task, change-calendar and incident-desk stayed at 10/10, chess rose from 8/10 to 10/10, and worker-logs from 8/10 to 9/10; none moved beyond pass-rate noise. Mean cost/run was effectively flat for calendar ($0.0242 → $0.0243) and higher for chess ($0.0506 → $0.0615), incident-desk ($0.0239 → $0.0257), and worker-logs ($0.0184 → $0.0218), within the observed run-to-run spread; chess consumed the most steps, 53.3 → 69.3, partly from completing turns that main never reached and repeated PGN-edit repairs. Cache hits remained around 95–98%, with only calendar’s decrease statistically significant and no new cache-miss step evident there.

⚪ VERDICT: NO REGRESSION FROM THIS PR

The candidate’s lone failure repeats main’s unsorted-percentile model error, while the upload/SCRATCH changes do not explain the cost variation or introduce observable cache invalidation in these attachment-free tasks.

Triage

Failure modes

• Percentiles calculated from unsorted durations · worker-logs 1/10 · model error · this PR: no — trial 5, turn 2’s writeFile(server.js) removed the duration ordering used by summary and indexed unsorted arrays in both summary and slowestRoutes. This explains all four failed checks: requests were preserved, but auth-edge’s p95 became 44 ms instead of 3000 ms. Main trial 5 already made the same mistake in slowestRoutes.

Tool errors

• editFile: No matching text was found in the file. · change-calendar 2 → 6, chess 3 → 14, incident-desk 0 → 1, worker-logs 0 → 3 · model error — whitespace and regex-escaping mismatches caused retries. Chess trial 9, turn 2 repeatedly submitted a misplaced space, even after grep displayed the correct text.
• editFile: Validation failed · change-calendar 1 → 2, chess 3 → 3, incident-desk 3 → 2, worker-logs 2 → 4 · model error — calls omitted required arguments or misspelled replacement as .replacement. Calendar trial 7, turn 1 and chess trial 9, turn 3 recovered by correcting those arguments.
• editFile: Multiple matches were found. The text to match must be unique. · change-calendar 2 → 0, chess 3 → 2, incident-desk 2 → 4, worker-logs 1 → 0 · model error — agents quoted repeated fragments rather than unique context. Main calendar trial 9 targeted await refresh();; candidate chess trial 7 targeted duplicated hint text, then used grep and larger spans successfully.
• readFile: File does not exist. · incident-desk 2 → 2 · model error — main trial 8 and candidate trial 6, turn 1 both tried reading server.js and client.js immediately after creating an empty gadget, before writing either file.
• executeCode: Timed out waiting for logs from code execution. · chess 0 → 2 · model error — candidate trial 10, turn 2’s subscription probes stalled; a reduced probe exposed subscribe() still calling the removed this.readState(). The agent patched that reference and subsequent probes succeeded.

Prompt cache

• change-calendar · cache hits 95.47% → 94.88% · breaks 0.00947% → 0.00947% · this PR: no — hit p = 0.043, break p = 1. No new miss stands out: the first request reads 7,220 → 7,244 cached tokens, consistent with the longer readFile description, and both trajectories retain cache reads across subsequent turns and gadget creation. With 30.0 → 27.5 steps and different newly written content, the lower hit share does not establish worse caching.

What to do

• Optional follow-up: add packages/workshop-evals/evals/attachment-worktree.eval.ts covering uploaded-file seeding, SCRATCH reads/edits, and immutable original downloads on a later turn; these four tasks do not exercise the feature being changed.
• Optional follow-up in packages/workshop-backend/src/agent.ts: add to EDIT_FILE_TOOL_DESCRIPTION that a no-match error should trigger grep or a targeted read before retrying, using the exact current text and unique surrounding context. The observed retry problem is not introduced by this PR.

github run