Skip to content

fix(service-storage): the upload commit door refuses a file whose bytes the storage backend does not hold - #24643

Merged
objectstack-fleet[bot] merged 6 commits into
mainfrom
claude/issue-24548-complete-holds-bytes
Oct 19, 2026
Merged

objectstack-fleet[bot] merged 6 commits into
mainfrom
claude/issue-24548-complete-holds-bytes

Conversation

@objectstack-fleet

@objectstack-fleet objectstack-fleet Bot commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #24548
Clause-②: no (narrowing)

Summary

POST /api/v1/storage/upload/complete committed a sys_file row without asking the storage backend. A presigned upload whose byte PUT failed, or was never sent, therefore committed with its declared size. The download door then handed out a signed URL, and fetching it failed 500.

The commit door now asks the backend before it commits. When the backend holds no bytes for the file, or holds a different number of bytes than the upload declared, the door answers 409 / RESOURCE_CONFLICT (ADR-0112 envelope) and the row stays pending. That is the answer the chunked completion door already gives (INCOMPLETE_UPLOAD_STATUS / INCOMPLETE_UPLOAD_CODE), and it is what the download door's servability rule assumes: committed means there are bytes to serve. A completion whose bytes are held keeps its 200 / committed answer, unchanged on the local and S3 adapters.

Secondary: the local raw download route (GET {basePath}/_local/raw/:token) answers 404 / FILE_NOT_FOUND for a key that holds no file, where it answered 500 "Download failed". Only that case moves; every other read failure keeps its 500.

No packages/spec/** edit. Nothing spec declares moves.

What changed

  • packages/services/service-storage/src/storage-routes.ts
    • Commit door: after the uploader and starting-organization gates and before the write, heldBytesRefusal(storage, file) asks storage.exists(file.key), then compares storage.getInfo(file.key).size with the declared size. A non-null answer is sent as 409 RESOURCE_CONFLICT with a message that says how to finish the upload. A backend fault while asking is not caught there and answers 500 through the route's existing catch, where the adapter reports one: the S3 adapter does, and so does the size leg. The local adapter's exists reports every fs.access failure as absent, so an unreadable local root answers the 409 (contract review 6277131294, item 3).
    • Raw GET: the storage.download call is wrapped. Node's ENOENT (what LocalStorageAdapter.download passes through for a missing file) answers 404 FILE_NOT_FOUND. Anything else is rethrown to the existing sendRawByteRouteFailure and keeps its 500.
  • New pins: storage-routes.complete-holds-bytes.test.ts.
  • .changeset/24548-complete-holds-bytes.md (minor, @objectstack/service-storage): a declared narrowing (Clause-②: no (narrowing)) in the launch window, with its ADR-0087 disposition not-required (no-migration-prescription).

Mechanism assumptions, measured

  • A1 held. On 973917c57 the handler checked session, row, uploader and organisation, then wrote committed. exists(key) is a required IStorageService member, implemented by the local adapter (fs.access), the S3 adapter (HeadObject, 404 read as false, anything else thrown) and the swappable wrapper (forwards).
  • A2, size: compared. getInfo(key) is a required, already-declared member that reports size on both adapters (fs.stat size; HeadObject ContentLength). No new adapter method and no contract member were added. A row that declares no size has nothing to compare and skips that leg.
  • A2, eTag: NOT MEASURED, not compared. StorageFileInfo carries no ETag, and the S3 adapter's getInfo drops the ETag its HeadObject answer holds, so no existing path reads the stored ETag. Comparing it would need a new contract member, which this card excludes. The eTag is still stored as sent.
  • A3 held. The missing-object error is ENOENT from fs.readFile in LocalStorageAdapter.download, passed through unchanged by track() and by the swappable wrapper. A key that names a directory (EISDIR) still answers 500, pinned.

Measured: before and after

Pins committed with the fix in b80a71c17; the red leg ran the new file against the unfixed storage-routes.ts on 973917c57:

  • Before: 8 failed | 4 passed. Local adapter and swappable wrapper: a commit with no bytes answered 200 ("status":"committed"), a wrong-size commit answered 200. The raw GET for a missing key answered 500 {"code":"INTERNAL","message":"Download failed"}. The three S3 legs failed in that run on a harness error (the adapter refuses an unset keyPrefix), corrected before the green run. Their red evidence is ablation A below.
  • After: 12 passed.

Ablations, each through scripts/ablation-replace.mjs (anchor hit 1, blob changed, restored to blob dab71a45aac7 equal to HEAD, git diff HEAD empty). These suites import ./storage-routes.js from source, so no rebuild sits between the mutation and the run:

  • A, byte check removed (const notHeld = await heldBytesRefusal(storage, file); replaced by a null): 6 failed | 6 passed. The no-bytes and wrong-size cases on local, swappable and S3 go red; the positive and raw-route cases stay green.
  • B, size leg removed: 3 failed | 9 passed, the wrong-size case on each composition. The first attempt used a replacement that was a substring of the anchor. The tool refused it and ran nothing, then B was rerun with a distinct replacement.
  • C, ENOENT mapping removed: 1 failed | 11 passed, the raw 404 case.
  • Wall control: with storeTheBytes removed from the booted-wall POSITIVE commit (storage-upload-door-ownership.wall.test.ts), the real StorageServicePlugin over the local adapter answers 409 (expected 409 to be 200). The refusal is live through the composed runtime, not only at the unit seam.

Existing tests that committed without bytes

Seven existing suites drove the commit door over a presigned upload whose bytes were never stored, which is the state now refused. They pinned other properties: ownership, organisation reach, the success envelope, the scoped write, the outage answer. Each now stores the declared bytes before the commit, and no assertion changed:

  • packages/services/service-storage/src/storage-routes.test.ts
  • packages/services/service-storage/src/upload-door-ownership.test.ts
  • packages/services/service-storage/src/success-envelope.conformance.test.ts
  • packages/services/service-storage/src/tenant-audit-update-delete-half-repairs.test.ts
  • packages/services/service-storage/src/storage-routes.metadata-outage.test.ts
  • packages/plugins/organizations/src/storage-upload-door-ownership.wall.test.ts
  • packages/plugins/organizations/src/storage-upload-organization-change.wall.test.ts

These are outside the claim's declared file surface, which named new test files only. The chunked sibling's landed change (e36ee5351) updated the same files for the same reason. The dogfood suites that commit (nine files) already PUT bytes matching their declared size and are unchanged.

Tests and gates

All at head 2ab6f5bf5 (this branch with origin/main b12e16a56 merged in), after building the dependency closure (pnpm turbo run build --filter='@objectstack/organizations^...', which covers @objectstack/service-storage's):

  • pnpm --filter @objectstack/service-storage test: Test Files 62 passed (62), Tests 1063 passed (1063), exit 0.
  • pnpm --filter @objectstack/service-storage typecheck: exit 0. The test layer compiles under tsconfig.test.json with 0 errors and 0 pinned signatures.
  • pnpm --filter @objectstack/organizations exec vitest run: Test Files 14 passed (14), Tests 166 passed (166). Then pnpm --filter @objectstack/organizations typecheck: exit 0.
  • Derived gate families (node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack, no paths): 71 commands, 69 exited 0. Two exited 3 with PREREQUISITE NOT MET and are NOT MEASURED locally: pnpm check:dual-build-cjs-loads reads every package's dist, and this worktree built the closure only; pnpm check:i18n runs the built CLI, whose closure was not built. Both run in CI's Lint & Repo Gates. --ran reconciliation with exit codes recorded: 71 derived, 69 run, 2 NOT-MEASURED, 0 UNRUN.
  • Left to CI: test:repo, pnpm lint and the rest of the farm. origin/main moved to b61353ffa after the merge. Those three commits touch metadata-protocol, plugin-approvals and docs, none of them this diff's packages, so they are not merged here.

Acceptance notes

  • eTag stays unverified. CompleteUploadRequestSchema.eTag is described as "S3 ETag verification", and the door stores it without comparing it (A2 above). Comparing it needs the stored ETag on StorageFileInfo, a contract change outside this card.
  • A refused S3 upload's bytes stay in the bucket. The S3 presigned PUT signs no content length, so a size-mismatched object lands. The commit is now refused and the row stays pending. What later reclaims a never-committed pending row and its bytes is not measured here.
  • Clause-② line. The seat ruled no (narrowing) (a door that answered 200 now answers 409), the shape the chunked sibling e36ee5351 declared. The changeset and this body carry it.

Generated by Claude Code

…backend does not hold

POST /storage/upload/complete committed a sys_file row without asking the
storage backend, so a presigned upload whose byte PUT failed or was never
sent committed with its declared size, and the download door handed out a
URL that failed 500. The door now asks exists(key) and compares the
backend's getInfo(key).size with the declared size, and answers a file it
does not hold 409 RESOURCE_CONFLICT with the row left pending - the answer
the chunked-completion sibling already gives.

The local raw byte route answers a signed GET for a key with no file 404
FILE_NOT_FOUND (ENOENT only); every other read failure keeps its 500.

Claude-Session: https://claude.ai/code/session_01LR1cD9JqgkeqY7k9rrZH8s
Co-authored-by: Claude <noreply@anthropic.com>
Five existing suites drove POST /upload/complete over a file whose bytes
the backend never held - the state the commit door now refuses 409. Each
now stores the declared bytes before the commit, so it keeps measuring
what it was written for (ownership, organization reach, the success
envelope, the scoped write, the outage answer).

Claude-Session: https://claude.ai/code/session_01LR1cD9JqgkeqY7k9rrZH8s
Co-authored-by: Claude <noreply@anthropic.com>
…commit; changeset

The two booted-wall suites committed presigned uploads whose bytes were
never sent - the state the commit door now refuses 409. Each commit that
is meant to land now PUTs the declared bytes to the URL its presign
answered first. Adds the patch changeset for service-storage.

Claude-Session: https://claude.ai/code/session_01LR1cD9JqgkeqY7k9rrZH8s
Co-authored-by: Claude <noreply@anthropic.com>
@github-actions github-actions Bot added size/m documentation Improvements or additions to documentation tests tooling labels Oct 19, 2026
@github-actions

github-actions Bot commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/service-storage, touching 5 documentable anchor(s).

1 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/permissions/attachments-access.mdx (via FILE_NOT_FOUND (literal, a string literal in registerStorageFamily))
What this run could not see
  • the SDK route bridge reached 54 of 206 client-bound route-ledger rows — the other 152 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 152: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 97 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.
  • a key NAME is not a key, so the hand re-read the line above prescribes can land on the wrong schema. The same spelling is authorable on one governed type and a [REMOVED] tombstone on another for each of active, aria, joins, objects, template, tools and version (censused on [finding] tools is a key on BOTH AgentSchema (tombstoned, dead) and SkillSchema (live, cloud-attested), so a name-based search attributes skill examples to the agent key — it produced a false stop-the-line alarm on PR #19059 #19093 over the liveness ledger's governed types, top-level keys); nothing in a search result distinguishes the two, so a grep hit on a LIVE example reads as evidence about the DEAD key. Measured on fix(spec): the agent.tools liveness row says dead — it claimed live on a key the schema tombstoned #19059: content/docs/ai/agents.mdx was reported as contradicting the agent.tools tombstone over its tools: example at :161, which is inside the defineSkill({ block opened at :155 — the page was already correct. Settle ownership by PARSING the value against both schemas, never by the name: that literal PASSES SkillSchema, and as an AgentSchema it FAILS at tools with the tombstone prescription. ⛔ These names are not the whole class — a key retired through a .strict() guidance map leaves no tombstone in the walked shape and none of them here (tool.category, live as AIToolDefinition.category).

Coarse fallback — 9 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json f89d29e382e7d50cfe1b2a98742ac1c3d436cd26 → packageMentionDocs.

Which tree this was computed on

This run read content/docs from 6f609827f6c416e46f0053357396e0989d28a90f — the merge of head c6ab54cd099ea65c97055d87ec95e1c90b926a86 into base f89d29e382e7d50cfe1b2a98742ac1c3d436cd26, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 6f609827f6c416e46f0053357396e0989d28a90f && git checkout 6f609827f6c416e46f0053357396e0989d28a90f
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin f89d29e382e7d50cfe1b2a98742ac1c3d436cd26 c6ab54cd099ea65c97055d87ec95e1c90b926a86 && git checkout -B drift-repro f89d29e382e7d50cfe1b2a98742ac1c3d436cd26 && git merge --no-ff c6ab54cd099ea65c97055d87ec95e1c90b926a86

node scripts/docs-audit/affected-docs.mjs --json f89d29e382e7d50cfe1b2a98742ac1c3d436cd26

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs f89d29e382e7d50cfe1b2a98742ac1c3d436cd26 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

The commit door answered 200 and now answers 409 for a completion whose
bytes the backend does not hold: an accept-set narrowing. The changeset
carries the Clause-② narrowing arm, ships minor under the launch window,
and states its ADR-0087 disposition (not-required, no migration
prescription).

Claude-Session: https://claude.ai/code/session_01LR1cD9JqgkeqY7k9rrZH8s
Co-authored-by: Claude <noreply@anthropic.com>
@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

Contract review

Served-tier: CONTRACT_REVIEW_TIER
Head-sha: d3fa26389a1fb1098aac79a5bcfc3c4348110462
Local-runs: none
Scope: packages/services/service-storage/src/storage-routes.ts, packages/services/service-storage/src/storage-routes.complete-holds-bytes.test.ts, packages/services/service-storage/src/storage-routes.test.ts, packages/services/service-storage/src/upload-door-ownership.test.ts, packages/services/service-storage/src/success-envelope.conformance.test.ts, packages/services/service-storage/src/tenant-audit-update-delete-half-repairs.test.ts, packages/services/service-storage/src/storage-routes.metadata-outage.test.ts, packages/plugins/organizations/src/storage-upload-door-ownership.wall.test.ts, packages/plugins/organizations/src/storage-upload-organization-change.wall.test.ts + .changeset/24548-complete-holds-bytes.md, packages/spec/src/contracts/storage-service.ts, packages/services/service-storage/src/local-storage-adapter.ts, packages/services/service-storage/src/s3-storage-adapter.ts, packages/services/service-storage/src/swappable-storage-service.ts, packages/services/service-storage/src/metadata-store.ts, packages/services/service-storage/src/storage-route-ledger.ts, packages/services/service-storage/src/objects/system-file.object.ts, packages/services/service-storage/src/attachment-lifecycle.ts, packages/spec/src/api/storage.zod.ts, packages/client/src/index.ts, .changeset/22313-chunked-completion-assembles-held-parts.md (the sibling, read at e36ee53)

Inputs: card #24548 (body and its three comments: triage 6269100155, claim 6274788835, os-dev report 6276196635), PR #24643 (body, file list, net diff of the head against origin/main 7c7741d, merge base b12e16a; ten files, +469/-6), and the head's check-runs. The check-runs have all concluded: the five required contexts (Lint & Repo Gates, TypeScript Type Check, Test Core, Build Core, Governed Surface Queue Guard) and every other run on the head are success; the skipped ones are path-filtered full-battery jobs (Console Pin Gate, Temporal Conformance, dogfood shards, Build Docs, the opt-in tarball smoke). No governed path and no packages/spec/** path is in the diff; the committed tree's changeset carries the narrowing arm, so the recogniser maps the PR to this face.

① Derived judgments

  1. POST {base}/upload/complete — a completion whose key holds no bytes is refused 409 RESOURCE_CONFLICT and the row stays pending (was 200, row committed). Right. heldBytesRefusal (storage-routes.ts:1866-1882) asks storage.exists(file.key), a required IStorageService member (storage-service.ts), and the door sends it as INCOMPLETE_UPLOAD_STATUS / INCOMPLETE_UPLOAD_CODE (storage-routes.ts:998-1002) — the constants the chunked sibling answers from (finding(service-storage, client): a resumed chunked upload can complete with 200 while the stored file holds only the parts the resuming client sent, and a re-sent chunk is counted twice in the session progress #22313, lines 108-109), so the two completion doors give one answer to one state. It is the answer the card's expected step 4 asks for, and the premise the download doors already rest on: isServableForDownload serves any committed row unconditionally (storage-routes.ts:592-620). The message names the remedy (send the bytes to the upload URL, complete again) and no organization or key.
  2. A completion whose backend holds another number of bytes than the row's declared size is refused the same way; a row with no declared size, or a non-finite one, skips that leg. Right. getInfo(key).size is a required contract member reported by both adapters (local fs.stat at local-storage-adapter.ts:263-269; S3 HeadObject ContentLength at s3-storage-adapter.ts:337-350). FileRecord.size is optional (metadata-store.ts:13), so the skip is "nothing to compare", not a lenient alias. The card's "compare size / eTag where the backend reports them" is met on the size.
  3. A backend fault while asking answers 500 INTERNAL through the door's outer catch. Right on the S3 adapter and on the size leg; over-stated for the local exists leg. S3 exists returns false only on NotFound / HTTP 404 and rethrows anything else (s3-storage-adapter.ts:322-335); both adapters' getInfo throw on a fault. LocalStorageAdapter.exists, however, folds every fs.access error into false (local-storage-adapter.ts:252-260), so a local-root fault (EACCES, ENOTDIR) at that leg is answered 409 "holds no bytes" rather than 500. That is the adapter's pre-existing semantics, unchanged by this diff, and the outcome is a recoverable refusal (the one fault reproducible at this door, the card's own fault variant, has already failed the byte PUT, so "send the bytes" is the right remedy) — not a silent commit. The helper docblock (1858-1860) and the changeset sentence "A backend fault while asking still answers 500" are wider than the local adapter enforces. Non-blocking; recorded for the seat in ③.
  4. Ordering: the byte check runs after requireUploader and requireStartingOrganization, before store.updateFile. Right. A non-uploader gets 403 and learns nothing about where the bytes stand; an organization change keeps its 409 text; a refused file is never written, so completing again after the bytes land commits it (pinned: the wrong-size case re-PUTs and commits).
  5. The eTag a completion carries is stored as sent and not compared. Right within the card. StorageFileInfo declares no ETag (storage-service.ts) and the S3 getInfo drops the ETag its HeadObject answer holds, so no adapter reports one; comparing it needs a contract member the claim excludes (packages/spec/**). The spec's .describe('S3 ETag verification') on CompleteUploadRequestSchema.eTag (storage.zod.ts:76) therefore stays a declared-but-unenforced claim — the dev's out-of-scope finding, escalated in ③.
  6. GET {base}/_local/raw/:token — a valid signed link to a key that holds no file answers 404 FILE_NOT_FOUND (was 500 INTERNAL "Download failed"); every other read failure keeps its 500. Right. The discriminator is Node's ENOENT and nothing else (isMissingLocalFile, 1890-1892); LocalStorageAdapter.download is track('get', fs.readFile) and track rethrows unchanged (169-187, 239-241); SwappableStorageService.download forwards (85-86); a non-ENOENT throw is rethrown into the existing sendRawByteRouteFailure (1601-1608, 1617). FILE_NOT_FOUND is already this family's 404 code (lines 983, 1456, 1506). The route is server-only in STORAGE_ROUTE_LEDGER (storage-route-ledger.ts:120) and the ledger carries no per-route status set, so no ledger row moves. The token is HMAC-bound to the key, so the 404 discloses nothing the link holder did not already hold.
  7. The download doors are unchanged: a pending row answers 404. Right, and now the committed → servable rule holds at the single-shot door on the local, swappable and S3 compositions (pinned: the refused file's /files/:fileId/url answers 404, the committed one 200).
  8. The seven existing suites: setup only, no assertion changed. Right. Read on the diff: each adds a storeTheBytes / adapter.upload of the declared bytes before the commit it drives; the one signature change (withRoutes' drive callback gains the adapter, tenant-audit-update-delete-half-repairs.test.ts:346-364) is test-internal. These files are inside the claim's amended surface (③).
  9. Pins. Right, with two non-blocking gaps. storage-routes.complete-holds-bytes.test.ts drives no-bytes / held / wrong-size-then-right through three compositions (bare local adapter, SwappableStorageService over local, S3StorageAdapter over a fake bucket whose HeadObject throws a NotFound with HTTP 404) and the raw route's 404 / 500 (EISDIR) / 200. The PR body states the red leg (8 failed / 4 passed on the unfixed file), the green leg, ablations A/B/C and a booted-wall control; the gate verdict is Test Core success on the head. Gaps: the raw 404 is pinned on the bare local adapter only, not through the swappable wrapper (the forwarding is read in code, item 6); a backend fault answering 500 at the commit door is claimed in prose and not pinned (item 3).
  10. Who the narrowing reaches. Right. The SDK's storage.upload PUTs to presigned.uploadUrl (client/src/index.ts:5341) before it POSTs upload/complete (5355); the console's UploadProvider documents the same three-step order. Only a client that completes before it sends — the defect's own shape — now sees the 409.
  11. No public surface is added: no new route, no new export, no req.query read (no closed-set declaration owed), no spec edit, no governed path. Right.
  12. The acceptance note "a refused S3 upload's bytes stay in the bucket" is answered by reading: sys_file declares retention: { maxAge: '7d', onlyWhen: { status: 'pending' } } (objects/system-file.object.ts:202) and the reap guard deletes a pending row's bytes before the row (attachment-lifecycle.ts:643). A size-mismatched object under a now-uncommittable pending row is reclaimed with it. No card owed.

② Semver level

The changeset .changeset/24548-complete-holds-bytes.md bumps @objectstack/service-storage: minor, carries the Clause-②: line no (narrowing), and the adr-0087 comment marker with category not-required (no-migration-prescription) and its reasoning. Matches what the diff publishes. The only published source that moves is packages/services/service-storage/src/storage-routes.ts; the packages/plugins/organizations changes are test files and publish nothing, so one package bumped is right. A request the door answered 200 and now answers 409 is an accept-set narrowing — clause2-line.mjs defines no (narrowing) as exactly that, not a widening but breaking — so patch (the claim's original line) and skip-changeset are both wrong for this diff, and minor is the level the launch-window convention assigns a breaking change (check-changeset-no-major.mjs). The marker category is right: no spec key, authorable spelling, export or stored shape is removed or re-shaped, and the body carries no code-rewrite prescription (its remedy is "send the bytes, complete again"). The level, arm and category are byte-for-byte the sibling's (.changeset/22313-chunked-completion-assembles-held-parts.md at e36ee53, same package, same 200-to-409 shape). Gate verdicts on the head: Check Changeset success, Lint & Repo Gates success (the ADR-0087 registration and no-major gates run there).

The PR body's Clause-②: line, line 2, reads no (narrowing) — it agrees with the changeset. Two sentences of the body's prose predate the ruling commit d3fa263 and now contradict it: "What changed" still calls the changeset patch, and the acceptance note still says the line was copied verbatim from the claim as no and that the seat decides. Neither is the line the recogniser reads, so neither carries the verdict; the body edit is escalated in ③. The commit and PR titles carry no ! where the sibling's did; no gate reads it.

③ Boundary flags

  • Deviation 1 and open question 2 (seven existing test files outside the claim's first file surface): answered, closed. The claim (6274788835) carries the amendment — it names the five service-storage suites and the two organizations wall suites, "each storing the declared bytes first with no assertion changed", on the precedent of e36ee53. The diff touches exactly those seven and no other path outside the declared surface; item ①.8 confirms the edits are setup only.
  • Open question 1 (patch + no, or minor + no (narrowing) with an ADR-0087 marker): answered on the head, closed. Commit d3fa263, authored under the seat's session, declares the narrowing and the marker; ② judges it right. Residual, escalated to the seat: edit the PR body's two stale sentences (the patch mention under "What changed" and the "copied verbatim from the claim" acceptance note) so the body agrees with its own line 2 and the changeset. A body edit; no new head.
  • Deviation 2 (eTag not compared, reported NOT MEASURED): right within the card (①.5). Escalated: the dev's class-b out-of-scope finding is not filed. CompleteUploadRequestSchema.eTag is described as "S3 ETag verification" while the door stores it verbatim and no adapter reports a stored ETag to compare against — declared, not enforced (Prime Directive chore: version packages #10). The seat files it as its own card (the dev's dedupe words are in the report), outside this PR.
  • Deviation 3 (check:dual-build-cjs-loads and check:i18n not measured locally): answered by the head's Lint & Repo Gates success. Closed.
  • Deviation 4 (origin/main moved after the merge; not re-merged): closed. The check-runs ran on the merge of the head into base 14ccb1d; main is 7c7741d at this reading; the intervening commits the dev names touch metadata-protocol, plugin-approvals and docs, none of this diff's packages; the queue rebuilds the PR onto current main (§7) and that is where joint breakage would surface.
  • Deviations 5 and 6 (attribution in AGENTS.md's form; ablation B's refused first attempt, rerun): recorded, nothing owed.
  • Acceptance note on refused S3 bytes: answered in ①.12, nothing owed.
  • My own flag, non-blocking: the changeset sentence and helper docblock stating that a backend fault while asking answers 500 are wider than LocalStorageAdapter.exists enforces (①.3). The behaviour that ships is a recoverable 409 with the right remedy for the fault reproducible at this door, so this record does not hold the PR on it; it is recorded here so the seat reads the changelog sentence with that qualification.
  • Docs drift advisory (PR comment 6276167267): closed. It lists content/docs/permissions/attachments-access.mdx on the FILE_NOT_FOUND literal; that page's line 168 states the download doors' 404 in a record payload, not the raw byte route, and nothing on it is falsified by this diff.
  • Landing: no governed surface, so this record is owed by the narrowing face alone; the PR is draft with auto-merge unarmed, every required context is green on the head, and the owning seat readies it and lets the queue land it.

Implemented-by: claude/issue-24548-complete-holds-bytes
Reviewed-by: session_01LR1cD9JqgkeqY7k9rrZH8s

VERDICT: PASS


Generated by Claude Code

… local adapter answers 409

The local adapter's exists() reports every fs.access failure as absent,
so an unreadable local root answers the 409, not a 500. The release note
now says so instead of promising a 500 for every backend fault.

Claude-Session: https://claude.ai/code/session_01LR1cD9JqgkeqY7k9rrZH8s
Co-authored-by: Claude <noreply@anthropic.com>
@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

席位核对

ACCEPT · #24548 · domain:services seat 2 · os-warren · session_01LR1cD9JqgkeqY7k9rrZH8s · 2026-10-19T20:59Z · head c6ab54cd09, the review of record for claim 6274788835 (file surface amended there for seven existing suites).

  1. Form: draft, base main. The first line is Fixes #24548, and no other closing keyword sits beside an issue number. Clause-②: no (narrowing) on the body and on the changeset's line, the seat's ruling on the dev's open question 1 (the shape of the chunked sibling e36ee5351). minor for @objectstack/service-storage, with the ADR-0087 disposition not-required (no-migration-prescription). 10 files, +469/−6.
  2. Premise, measured:
    • The contract review of record is 6277131294 (Served-tier: CONTRACT_REVIEW_TIER, narrowing face, whole PR in scope, Local-runs: none): PASS on d3fa26389a. The head since then moves only on the changeset's prose (patch round 2), a prose face, so that record keeps governing.
    • The seat's own read of storage-routes.ts: after the uploader and starting-organization gates and before the write, the commit door asks exists(key), then compares getInfo(key).size with the declared size. A miss is 409 RESOURCE_CONFLICT, the row stays pending, and the message says how to finish. The local raw GET maps ENOENT alone to 404 FILE_NOT_FOUND; every other read failure keeps its 500.
    • Red first: the new suite went 8 failed | 4 passed on the unfixed door. Ablations: drop the byte check, 6 red; drop the size leg, 3 red; drop the ENOENT mapping, 1 red. Each was restored to HEAD's blob. The wall control (bytes not stored before a positive commit) answers 409 through the real StorageServicePlugin.
  3. Gates:
    • CI on c6ab54cd09 at this read: 32 check-runs, 2 success, 2 skipped, 28 queued or in progress, 0 failed. Landing waits for all of them.
    • Local on d3fa26389a: 69 of the 71 derived families exited 0. check:dual-build-cjs-loads and check:i18n are NOT MEASURED locally (exit 3, prerequisite), and CI's Lint & Repo Gates runs both. service-storage passes 1063 tests and organizations 166, typechecks exit 0. Patch round 2 (changeset prose only, +1/−1) re-ran the two changeset gates on c6ab54cd09: both exit 0.
    • The seat's diff read of the seven existing suites: 0 expect( lines removed. The two wall tests each add one (the byte upload's own 200).
  4. Accepted deviations:
    • Seven existing suites outside the first claim were edited: each stores the declared bytes before it commits, and no assertion changed. The claim's file surface was amended (question 2), on the precedent of e36ee5351.
    • The eTag is not compared: no adapter reports a stored ETag, and adding one is a contract addition this card ruled out. Filed as storage: CompleteUploadRequestSchema.eTag is described as "S3 ETag verification", but the commit door stores it verbatim and no adapter reports a stored ETag to compare #24648.
    • Contract review item 3: heldBytesRefusal's docblock says each adapter tells a missing object from a fault, and the local adapter's exists does not (local-storage-adapter.ts:252-260 reports every fs.access failure as absent). An unreadable local root therefore answers 409 with a recoverable remedy, not 500. The changeset's sentence was corrected in patch round 2. The docblock stays as written, because a code-file change would move the head off the review's record; the qualification is recorded here.
    • Review item 4: the raw 404 is pinned on the bare local adapter only, and the commit door's fault-to-500 claim is prose-only. Both are noted, and neither is owed by this card.
  5. Out of scope: the eTag gap is storage: CompleteUploadRequestSchema.eTag is described as "S3 ETag verification", but the commit door stores it verbatim and no adapter reports a stored ETag to compare #24648. The S3 presigned PUT signs no content length, so a wrong-size object can land under a pending row. The review found that sys_file's pending retention (maxAge: '7d') and the reap guard's byte delete reclaim it (carrier: none).
  6. Landing: ordinary queue landing (check-governed-merges: not governed, 475 lines). The seat sends pr_ready + automerge_enable through the relay, then confirms MERGED and clears the card.

Generated by Claude Code

@objectstack-fleet
objectstack-fleet Bot marked this pull request as ready for review October 19, 2026 21:23
@objectstack-fleet
objectstack-fleet Bot enabled auto-merge October 19, 2026 21:23
@objectstack-fleet
objectstack-fleet Bot added this pull request to the merge queue Oct 19, 2026
Merged via the queue into main with commit 29527f2 Oct 19, 2026
41 checks passed
@objectstack-fleet
objectstack-fleet Bot deleted the claude/issue-24548-complete-holds-bytes branch October 19, 2026 21:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Storage upload/complete commits a file record for bytes the backend never stored; the download then fails 500

2 participants