Skip to content

feat(wire,client-core): chunked attachment upload and read protocol - #514

Open
Zerlight wants to merge 11 commits into
ruocheng/code-635from
ruocheng/code-636
Open

feat(wire,client-core): chunked attachment upload and read protocol#514
Zerlight wants to merge 11 commits into
ruocheng/code-635from
ruocheng/code-636

Conversation

@Zerlight

@Zerlight Zerlight commented Sep 7, 2026

Copy link
Copy Markdown
Member

Summary

Phase 3 of CODE-627 — Conversation turn graph & immutable attachment store. Linear: https://linear.app/arcbox/issue/CODE-636/featwireclient-core-chunked-attachment-upload-and-read-protocol

Stack: #513this PR (ruocheng/code-636, base ruocheng/code-635) ← #515. Merge bottom-up; this PR's diff is only its own commits.

Additive chunked upload/read frames sized for the tunnel: begin carries the client SHA-256 for a zero-chunk dedupe short-circuit, credit-window flow control accepts pipelined chunks, commit verifies size, coverage, hash, and sniffed MIME before the atomic rename, and attachment.read is reachability-checked against the session's graph. client-core gains upload/read plumbing and a bounded blob cache; the mime sniff moves to the schema so clients validate too; the dev mock implements the frames.

Commits

  • feat(schema): add chunked attachment upload and read wire frames
  • feat(engine,daemon): read blobs by offset and check attachment reachability
  • feat(engine): add the chunked attachment upload and read service
  • test(engine): cover attachment upload reject paths and wire reads
  • feat(client-core): add chunked attachment upload, read, and blob cache
  • feat(workbench): implement chunked attachment frames on the dev mock
  • fix(engine): accept pipelined chunks, verify dedupe hits, and release abandoned stages
  • fix(client-core): terminate the attachment read walk, abort a failed upload, bound the blob cache
  • test(workbench,schema): cover the dev mock's attachment frames and pin the chunk budget
  • refactor(schema): host the attachment mime sniff so clients can validate too
  • fix(workbench): gate dev-mock attachment reads on a session root and sniff its commits

Verification

Every commit passed pnpm check:ci and pnpm test at its own tip; the stack tip (1d942a62; the same tree as the originally gated 73fc5ff2 plus the two review fixes below on 628/629) is at pnpm check:ci 0 errors, pnpm test 3408 passed / 1 skipped. Adversarial reviewers (one per axis, isolated read-only worktrees) reviewed the branch; each P1/P2 was reproduced with a failing test or a probe step before its fix — the round-by-round record is in the Linear issue's comments. Tests pin the chunk budget and cover the reject paths (wrong offset/hash/size, cross-session reads) on the engine and the dev mock.

Checklist

  • pnpm check:ci and pnpm test both pass (no Rust changes)
  • I ran the affected surface and observed the change working — engine exercised through its harness tests and the dev-mock parity suite; the live-daemon run against real providers is recorded per issue in Linear (the claude edit path still needs a paid turn, opencode its binary)
  • Wire: frames added under the still-unshipped wire 80
  • New code and assets are my own work
  • Docs and comments are updated where behavior changed (AGENTS.md and module docs in this branch)

@pullfrog

pullfrog Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Your Claude subscription has hit its usage limit. It resets at 11:30am (UTC). Re-trigger Pullfrog after the reset, or add an ANTHROPIC_API_KEY repo secret — Pullfrog routes around an exhausted subscription automatically when one is present.

Add repo secret → · Model settings → · Setup docs → · Ask in Discord →

Pullfrog  | Rerun failed job ➔View workflow run | via Pullfrog | Using Claude Opus𝕏

@linear-code

linear-code Bot commented Sep 7, 2026

Copy link
Copy Markdown

CODE-636

@greptile-apps

greptile-apps Bot commented Sep 7, 2026

Copy link
Copy Markdown

Greptile Summary

This PR introduces a chunked, content-addressed attachment protocol across the shared wire schema, client core, host engine, daemon persistence, and development mock.

  • Adds begin, chunk, commit, abort, and session-scoped read frames with tunnel-sized payload limits.
  • Implements staged uploads, integrity and MIME checks, deduplication, immutable blob reads, and reachability checks.
  • Adds client-side windowed upload, paginated assembly, hashing, and a bounded blob cache.
  • Extends resource and conversation stores to act as session-scoped attachment roots.
  • Adds engine, schema, client, daemon, and dev-mock integration coverage.

Confidence Score: 0/5

The PR is not safe to merge until cross-session mock authorization, cross-client upload replay, abandoned-stage cleanup, and malformed mock chunk handling are corrected.

The shared upload service can expose or abort another client's active upload through an operation-ID collision, the dev mock can grant attachment access from an arbitrary prompt reference, abandoned begins can exhaust staging resources, and malformed chunks can leave client requests permanently pending.

Files Needing Attention: packages/host/engine/src/attachment/upload-service.ts, packages/client/workbench/src/mock/dev-mock-host.ts, packages/client/core/src/client/attachment-channel.ts

Security Review

Three security-relevant issues remain:

  • The dev mock allows arbitrary prompt references to grant cross-session attachment read access.
  • Globally replayed operation IDs can disclose a live upload capability across connected clients.
  • Unbounded staged uploads can retain file descriptors and staging storage without periodic or disconnect cleanup.

Important Files Changed

Filename Overview
packages/host/engine/src/attachment/upload-service.ts Implements upload staging, deduplication, validation, reads, and replay, but globally scoped replay capabilities and unbounded live-stage lifecycle require attention.
packages/client/workbench/src/mock/dev-mock-host.ts Adds dev-mock attachment parity, but prompt rooting permits cross-session reads and malformed chunks can strand pending requests.
packages/client/core/src/client/attachment-channel.ts Adds windowed uploads and cached paginated reads; later read pages should be checked for identity and size consistency.
packages/foundation/schema/src/wire/attachment.ts Defines bounded additive upload and read frames with protocol capability constants.
apps/daemon/src/attachment-store.ts Adds session-scoped reachability checks through persisted prompt and resource roots.
packages/host/engine/src/attachment/blob-store.ts Adds positional immutable-blob reads and supplies the retained staging handles involved in upload lifecycle management.

Sequence Diagram

sequenceDiagram
    participant Client
    participant Wire as Shared wire transport
    participant Engine as Attachment service
    participant Stage as Staging blob
    participant Store as Attachment store
    participant Blob as Immutable blob store

    Client->>Wire: attachment.upload.begin(hash, size, metadata)
    Wire->>Engine: validated begin
    Engine->>Store: create upload lease
    alt Blob already exists
        Engine-->>Client: "begun(state=exists)"
    else New blob
        Engine->>Stage: open staging file
        Engine-->>Client: "begun(state=ready, chunkBytes)"
        loop Credit-windowed chunks
            Client->>Engine: chunk(uploadId, offset, base64)
            Engine->>Stage: positional write
            Engine-->>Client: cumulative acknowledgement
        end
    end
    Client->>Engine: commit(uploadId)
    Engine->>Engine: verify size, MIME, and SHA-256
    Engine->>Blob: atomically publish immutable bytes
    Engine->>Store: commit attachment and release lease
    Engine-->>Client: attachmentId and blobId

    Client->>Engine: read(sessionId, attachmentId, offset)
    Engine->>Store: verify session reachability
    Engine->>Blob: positional read
    Engine-->>Client: immutable blob page
Loading

Reviews (1): Last reviewed commit: "fix(workbench): gate dev-mock attachment..." | Re-trigger Greptile

Comment thread packages/client/workbench/src/mock/dev-mock-host.ts
Comment thread packages/host/engine/src/attachment/upload-service.ts
Comment thread packages/host/engine/src/attachment/upload-service.ts
Comment thread packages/client/workbench/src/mock/dev-mock-host.ts
Comment thread packages/client/core/src/client/attachment-channel.ts

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Greptile has paused reviews on this repository — it used its 100 free open-source review credits for this billing period. Reviews resume automatically on September 23. To continue before then, an organization admin can keep reviews running past the free credits — those bill as normal usage.

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Greptile has paused reviews on this repository — it used its 100 free open-source review credits for this billing period. Reviews resume automatically on September 23. To continue before then, an organization admin can keep reviews running past the free credits — those bill as normal usage.

@pullfrog

pullfrog Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Your Claude subscription has hit its usage limit. It resets at 11:30am (UTC). Re-trigger Pullfrog after the reset, or add an ANTHROPIC_API_KEY repo secret — Pullfrog routes around an exhausted subscription automatically when one is present.

Add repo secret → · Model settings → · Setup docs → · Ask in Discord →

Pullfrog  | Rerun failed job ➔View workflow run | via Pullfrog | Using Claude Opus𝕏

Additive kinds at wire 80: begin/chunk/commit/abort/read, 256 KiB chunks,
and a feature-detect constant. The payload field is attachmentKind so it
does not collide with the frame discriminator.
…bility

Positional BlobStore.read, session-scoped isReachable (prompt refs or
session resources), and a missing-file stat so a vanished CAS object is
not treated as a dedupe hit.
Begin/chunk/commit/abort with a credit window and SHA-256 dedupe, MIME
sniff at commit, and reachability-gated reads. Typed request errors for
offset, hash, size, and missing ids.
Wrong offset/hash/size, missing blob files, path-shaped ids, and
cross-session reads fail typed. Dedupe exists and resource-rooted
reads go through the engine.
Feature-detects wire 80, pipelines chunks in a two-chunk credit window,
and caches decoded bytes by blobId.
Begin/chunk/commit/abort/read run in memory so showcase and mock E2E
exercise the new protocol instead of silently dropping the kinds.
Copilot AI lite review requested due to automatic review settings September 7, 2026 07:25
@pullfrog

pullfrog Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Your Claude subscription has hit its usage limit. It resets at 11:30am (UTC). Re-trigger Pullfrog after the reset, or add an ANTHROPIC_API_KEY repo secret — Pullfrog routes around an exhausted subscription automatically when one is present.

Add repo secret → · Model settings → · Setup docs → · Ask in Discord →

Pullfrog  | Rerun failed job ➔View workflow run | via Pullfrog | Using Claude Opus𝕏

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@Zerlight

Zerlight commented Sep 7, 2026

Copy link
Copy Markdown
Member Author

Review round on this PR (Greptile, five inline threads), fixes on the top of the stack: fdda262 — a replayed attachment.upload.begin must match its original declaration (hash, size, name, MIME type, kind) or is refused invalid_request, and live uploads are bounded: a cap of 32 reserved synchronously from the check, plus stages idle for 5 minutes reaped at the next begin. 8278289 — the client read walk rejects a page naming another blob or size before copying it, and the dev mock mirrors the replay rule. Declined with reasoning on the threads: the mock's prompt-ref rooting is daemon parity (refs are not session-scoped by design — integrity, not confidentiality, within one account), and the malformed-chunk hang was already fixed in ae6a254 (#516). Gates at 8278289: check:ci 0 errors, 3421 tests. PR body unchanged.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants