Skip to content

fix(service-storage): refuse an out-of-range chunkIndex and a non-byte-count presigned size at the upload doors - #27537

Merged
objectstack-fleet[bot] merged 6 commits into
mainfrom
claude/issue-27412-storage-route-input
Oct 29, 2026
Merged

objectstack-fleet[bot] merged 6 commits into
mainfrom
claude/issue-27412-storage-route-input

Conversation

@objectstack-fleet

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

Copy link
Copy Markdown
Contributor

domain:devx#1 · sunhaolin · session_017xRBcScm27Q3R5mkYLPWdB

Fixes #27412
Fixes #27413

Clause-②: no (narrowing)
Metadata-protocol: none

Narrowed: a negative size, which the spec number (GetPresignedUrlRequestSchema.size, a bare z.number()) admits, is now refused 400 at the presigned door.

维护者速读

  • 改了什么:存储上传的两个入口在门口拒收坏参数:分片上传的 chunkIndex 必须是 [0, totalChunks) 内的整数,预签名上传的 size 必须是非负有限数字;其余一律 400 INVALID_REQUEST,不落任何数据。
  • 为什么改:之前 chunkIndex=-1 返回 500,越界分片被存下并计入进度,之后 complete 永远 409 且无法修复;size="abc" 返回 500,size=-5 会写入一条负大小的 sys_file 记录。
  • 风险与回滚:收窄了这两个入口接受的请求(例如数字字符串 "100" 作为 size 现在被拒);SDK 与 Console 上传客户端都发送合法值,不受影响。回滚即 revert 本 PR。
  • 先看:packages/services/service-storage/src/storage-routes.ts(分片门的范围判断位置:在 resume token 校验之后、读取字节之前),chunked-upload-integrity.test.ts,storage-limits.test.ts(夹具改为声明两个分片)。
  • 两分钟验证:pnpm --filter @objectstack/service-storage exec vitest run --maxWorkers=2 src/chunked-upload-integrity.test.ts src/storage-routes.test.ts ,预期 2 个文件全部通过。
  • 需要你做的:无,席位落地(落地前席位做一次 contract-review 档复核)。

What changes

Two upload doors in @objectstack/service-storage now refuse malformed request input at the door with 400 INVALID_REQUEST, the code these doors already answer for a request they cannot take.

Chunk door PUT /api/v1/storage/upload/chunked/UPLOAD_ID/chunk/CHUNK_INDEX (#27412)

  • The index is read strictly: decimal digits only, a safe integer. parseInt read 0.5 as 0, and 1e0 as 1.
  • It is then judged against the session's total_chunks: it must be in [0, totalChunks). The check runs after the resume-token, organization and expiry checks (a caller without the token still gets 403 INVALID_RESUME_TOKEN and learns nothing of the range) and before a byte is read, stored or counted.
  • The refusal names the range (chunkIndex must be 0: this upload declared 1 chunk, indexed from zero. / chunkIndex must be a whole number from 0 to N: ...) and carries details: { chunkIndex, totalChunks }.

Presigned door POST /api/v1/storage/upload/presigned (#27413)

  • size must be a finite, non-negative JSON number (GetPresignedUrlRequestSchema.size is z.number()). It is judged after the required-field check and before the scope gate, the size limit and the pending sys_file insert.
  • The refusal message starts size must be the file size in bytes.

Exactly what is now refused (the narrowing the contract review reads)

  • Chunk door: any chunkIndex path segment that is not all decimal digits (-1, 0.5, 1e0, 0x0, abc), and any whole number at or past the session's total_chunks. Before: -1 answered 500 INTERNAL; 0.5 was stored as chunk 0; an out-of-range whole number answered 200, was stored and counted, and then blocked complete with 409 for good.
  • Presigned door: a size that is a string (numeric strings such as "100" included), NaN, an infinity, a negative number, or a boolean. Before: "abc" answered 500 INTERNAL (the sys_file insert's number validation); -5 answered 200 and stored a pending row with size: -5. A numeric string's previous answer depended on the engine's number-field handling at insert and was not measured here.
  • Unchanged: chunk indexes 0 .. totalChunks - 1; sizes 0 and any positive number. The SDK (storage.upload, resumeUpload) and the Console UploadProvider send File.size and indexes in range.

The completion door's own unexpectedChunks refusal stays: a session recorded before this change can still hold a stray chunk.

Tests

  • chunked-upload-integrity.test.ts: new block for storage: the chunk door accepts chunk indexes outside the declared range and answers 500 on a negative index, leaving a session that can never complete #27412. Seven refused indexes (-1, 1 = totalChunks, 7, 0.5, 1e0, 0x0, abc), each asserting status 400, error.code INVALID_REQUEST, the range message, the details, that the backend uploadChunk was never called, that the session's parts and progress are unchanged, and that a following in-range upload completes 200 with the right bytes. Controls: the multi-chunk range message, the last declared index accepted, and the 403 ordering before the index is judged. The existing stray-chunk completion case now plants its stray part on the session row (the shape a pre-fix session carries), since the chunk door no longer stores one.
  • storage-routes.test.ts: new presign cases for storage: the presigned-upload door answers 500 INTERNAL for a non-numeric size and accepts a negative size #27413. Eight refused sizes, each asserting status 400, error.code INVALID_REQUEST, the message naming size, and that store.createFile was never called. Controls: 0 and 1024 answer 200 and store a pending row with that size.
  • storage-limits.test.ts: two upload-limit cases started a 1 MiB upload (one declared chunk under the 5 MiB chunk floor) and then sent chunk index 1. They now widen the session row to two declared chunks first; what they pin, bytes received across chunks against the limit, is unchanged.

Acceptance notes

Verification

Package tests and typecheck at dbfaf81 (before the merge of origin/main; the merged commit touches no service-storage file)
pnpm --filter @objectstack/service-storage test && pnpm --filter @objectstack/service-storage typecheck
 Test Files  70 passed (70)
      Tests  1278 passed (1278)
check:test-typecheck: OK, 0 file(s) / 0 error(s)
os-verify-lock: VERDICT command-exit 0

The first run, before the storage-limits.test.ts fixture update, failed the two limit cases named above (Tests 2 failed | 1276 passed).

Ablation, one per guard (source mutation through scripts/ablation-replace.mjs; the suites import the routes from src, so no dist build is involved)
range check removed  (anchor x1 -> x0, blob 6c89ee52a759 -> 14e5b17d7623)
  chunked-upload-integrity.test.ts: Tests 3 failed | 20 passed  (index 1, index 7, multi-chunk range)
  restored: blob == HEAD (6c89ee52a759), git diff HEAD empty
size check removed   (anchor x1 -> x0, blob 6c89ee52a759 -> ed2276f74506)
  storage-routes.test.ts: Tests 8 failed | 52 passed  (all eight refused sizes)
  restored: blob == HEAD (6c89ee52a759), git diff HEAD empty

With the range check removed, -1, 0.5, 1e0, 0x0 and abc stay refused by the strict parse, as expected.

Derived gate families (dispatch-gates --commands at 3a10b29)
dispatch-gates --ran: 64 derived famil(ies) accounted for: 61 run, 3 NOT-MEASURED (3 DERIVED from a recorded exit 3)
61 x exit 0
exit 3 PREREQUISITE NOT MET: pnpm check:dual-build-cjs-loads (workspace dists not built), pnpm check:i18n (CLI not built),
       node scripts/check-plugin-teardown-shape.mjs --self-test (shallow clone, pinned fixture commit unreachable)
check-nul-bytes: OK

Those three are NOT MEASURED locally and are left to CI.


Generated by Claude Code

… the chunk door

The chunk door read its index with parseInt and checked only isNaN: 0.5
read as 0, -1 reached the backend as part number 0 and answered 500, and
an index at or past totalChunks was stored, counted in progress, and then
blocked the completion door for good. The index is now read strictly
(decimal digits only) and judged against the session's total_chunks
after the resume-token check; anything else answers 400 INVALID_REQUEST
naming the range, with nothing stored or counted.

Claude-Session: https://claude.ai/code/session_017xRBcScm27Q3R5mkYLPWdB
Co-authored-by: Claude <noreply@anthropic.com>
…te count

The presigned door refused only a missing size, then judged Number(size)
against the upload limit: a non-numeric size reached the sys_file insert
and failed there as a 500, and a negative size stored a pending row. The
size must now be a finite, non-negative JSON number; anything else
answers 400 INVALID_REQUEST naming size, before any row is written.

Claude-Session: https://claude.ai/code/session_017xRBcScm27Q3R5mkYLPWdB
Co-authored-by: Claude <noreply@anthropic.com>
The upload-limit cases start a 1 MiB upload, which the init door floors
to ONE declared chunk, then send chunk index 1. The chunk door now
refuses an index past the declared count, so those cases widen the
session row to two declared chunks; what they pin, the bytes received
across chunks against the limit, is unchanged.

Claude-Session: https://claude.ai/code/session_017xRBcScm27Q3R5mkYLPWdB
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 29, 2026
@github-actions

github-actions Bot commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

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

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

  • content/docs/api/error-catalog.mdx (via INVALID_REQUEST (literal, a string literal in registerStorageFamily))
  • content/docs/api/metadata-api.mdx (via INVALID_REQUEST (literal, a string literal in registerStorageFamily))
  • content/docs/automation/webhooks.mdx (via INVALID_REQUEST (literal, a string literal in registerStorageFamily))
  • content/docs/data-modeling/import-mappings.mdx (via INVALID_REQUEST (literal, a string literal in registerStorageFamily))
  • content/docs/permissions/authentication.mdx (via INVALID_REQUEST (literal, a string literal in registerStorageFamily))
  • content/docs/permissions/sso.mdx (via INVALID_REQUEST (literal, a string literal in registerStorageFamily))
  • content/docs/ui/forms.mdx (via INVALID_REQUEST (literal, a string literal in registerStorageFamily))

⛔ 5 release-owned page(s) also name something this change touched. These are read-only:

  • content/docs/releases/v17/17-0.mdx (via INVALID_REQUEST (literal, a string literal in registerStorageFamily))
  • content/docs/releases/v17/17-6.mdx (via INVALID_REQUEST (literal, a string literal in registerStorageFamily))
  • content/docs/releases/v18/18-0.mdx (via INVALID_REQUEST (literal, a string literal in registerStorageFamily))
  • content/docs/releases/v18/18-1.mdx (via INVALID_REQUEST (literal, a string literal in registerStorageFamily))
  • content/docs/releases/v18/18-2.mdx (via INVALID_REQUEST (literal, a string literal in registerStorageFamily))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see
  • the SDK route bridge reached 58 of 210 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 — 10 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 aef631b80b3175f2f43287a6e0185c828c356ac8 → packageMentionDocs.

Which tree this was computed on

This run read content/docs from a200cfbda34f98cd236de0d10e5d937bd570f9c6 — the merge of head 41272f9f5693500d15a712731557f41502a8d982 into base aef631b80b3175f2f43287a6e0185c828c356ac8, 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 a200cfbda34f98cd236de0d10e5d937bd570f9c6 && git checkout a200cfbda34f98cd236de0d10e5d937bd570f9c6
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin aef631b80b3175f2f43287a6e0185c828c356ac8 41272f9f5693500d15a712731557f41502a8d982 && git checkout -B drift-repro aef631b80b3175f2f43287a6e0185c828c356ac8 && git merge --no-ff 41272f9f5693500d15a712731557f41502a8d982

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

⚠️ 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 aef631b80b3175f2f43287a6e0185c828c356ac8 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

…② line

GetPresignedUrlRequestSchema.size is a bare z.number(), so a negative
size is inside the published accept set and the presigned door now
refuses it: the declaration reads `no (narrowing)` and names it.

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

Copy link
Copy Markdown
Contributor Author

domain:devx#1 · sunhaolin · session_017xRBcScm27Q3R5mkYLPWdB

席位核对

Verdict: REWORK (one item) at head 3a10b29c, reviewed 2026-10-29T11:35Z. Patch round to the same dev, on the same claim and branch.

  1. The declaration is missing. GetPresignedUrlRequestSchema.size (packages/spec/src/api/storage.zod.ts) is a bare z.number(), so a negative size is inside the published request accept set, and this PR refuses it. The PR body and changeset must therefore read Clause-②: no (narrowing), with one sentence naming what is narrowed. The refusals of non-numbers and the chunk-index range checks restore the route's own declared contract and need no declaration.

What is already checked and holds:

  • The diff: the strict index parse, the range check placed after the resume-token check, and the presign size guard placed before any sys_file write.
  • Every refusal case asserts status and error.code, each with a control.
  • total_chunks is a required session field written at init, and the completion door already reads it the same way.
  • Files: 5, 0 governed, +195/-5.

After the push, this PR owes a contract-tier review, recorded under ## Contract review, before the queue.


Generated by Claude Code

A `(narrowing)` arm declares a breaking change, and
check-adr-0087-registration requires its disposition in writing: no
migration prescription, since only a REST request value is refused.

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

Copy link
Copy Markdown
Contributor Author

domain:devx#1 · sunhaolin · session_017xRBcScm27Q3R5mkYLPWdB

Contract review

Served-tier: CONTRACT_REVIEW_TIER
Head-sha: 41272f9f5693500d15a712731557f41502a8d982
Local-runs: none
Scope: packages/services/service-storage/src/storage-routes.ts, packages/services/service-storage/src/storage-routes.test.ts, packages/services/service-storage/src/chunked-upload-integrity.test.ts, packages/services/service-storage/src/storage-limits.test.ts + packages/spec/src/api/storage.zod.ts (GetPresignedUrlRequestSchema, InitiateChunkedUploadRequestSchema, UploadChunkRequestSchema), packages/client/src/index.ts (storage.upload, storage.uploadPart, storage.resumeUpload), ui/packages/providers/src/UploadProvider.tsx (the presigned adapter), packages/services/service-storage/src/metadata-store.ts (UploadSessionRecord.total_chunks), packages/services/service-storage/src/objects/system-upload-session.object.ts (total_chunks), packages/types/src/response-envelope.ts (sendError). Face: narrowing, so the floor is every path of the diff outside the prose faces; the changeset is read under ② and is not reviewed under ④.

Inputs read: cards #27412 and #27413 with every comment (triage, claims, both os-dev-report comments, the seat's REWORK note); PR #27537 body, file list and the diff of its five files against the merge base 3604371f; the 52 check-runs on 41272f9f: 36 success, 16 skipped (UI, docs and opt-in lanes the paths do not select), 0 failed, 0 pending at the time of this record. Check Changeset, every Lint lane (the home of check-adr-0087-registration and check-changeset-no-major), Test Core 1/8..8/8, the type-check lanes, Governed Surface Queue Guard and Dogfood Regression Gate are among the 36.

① Derived judgments

  • Presigned door accept set for size. Before: anything but null/undefined. Now: a JSON number that is finite and at least 0. Judged against the published GetPresignedUrlRequestSchema.size (z.number()): a string (numeric strings included) and a boolean were never inside the published set, so refusing them restores the contract and is not a narrowing: right. NaN is refused by z.number() itself, and no JSON wire body carries NaN or an infinity, so neither moves the published set over the wire: right. A negative number (-5, -0.5) IS inside the published set and is now refused: this is the one narrowing, and the PR body and changeset declare exactly it. Right. A fractional size stays accepted, which is what the spec declares today: consistent.
  • Chunk door accept set for chunkIndex. Before: anything parseInt could read. Now: decimal digits, a safe integer, below the session's total_chunks. Judged against UploadChunkRequestSchema.chunkIndex (z.number().int().min(0)): -1, 0.5, 1e0, 0x0, abc were never inside the published set, so the strict parse restores the contract: right. A whole number at or past total_chunks is inside the static schema (it has no upper bound the spec can state per session) but outside the protocol's declared capability: the init response declares totalChunks as the expected number of chunks, and the completion door has always refused a session holding a chunk beyond it, so no completable upload ever held one. Refusing it at the door where the harm is done is the right place, and it turns the completion door's refusal of such a session from a permanent 409 into a never-stored chunk. The (narrowing) arm the PR already carries covers the consumer-facing truth of this change whichever way a reader classifies it.
  • Public surface. No route, export, type or error code is added, removed or renamed. INVALID_REQUEST is the code both doors already answer for a request they cannot take (the required-field refusals a few lines above each new check), and details is a declared field of the sendError envelope. The response shapes are unchanged. Right.
  • Refusal ordering. Presigned: after the required-field check, before the scope gate, the limit and the sys_file insert. Chunk: after 404, the 403 resume-token check, the organization-reach check and the 410 expiry check, before a byte is read, stored or counted. This keeps the standing rule that a caller who cannot prove ownership learns nothing about the session, and the control test pins the 403-before-range order. Right.
  • Is narrowing below the published spec the right shape here, or should size be tightened in the spec? The door narrowing is the right fix for these two cards: the route destructures req.body and never parses it with GetPresignedUrlRequestSchema, so a spec-only .int().min(0) would change nothing at runtime, and the 500 and the negative sys_file row would stay. Tightening the spec in THIS PR would be the wrong lane: packages/spec was outside the dispatched file surface, a spec edit owes regenerated api-surface and JSON-schema artifacts, and it would flip the review to the schema face in a spec-lane card. Should-fix, as a spec-lane follow-up card (a recommendation, not filed here): GetPresignedUrlRequestSchema.size to .int().min(0) with the presigned door moved to Number.isInteger in the same PR, so declared equals enforced in both directions; and in the same card, the init door, which InitiateChunkedUploadRequestSchema declares as totalSize: int().min(1) and chunkSize: int().min(5242880) while POST /upload/chunked checks totalSize only for truthiness (a negative totalSize mints a session with total_chunks of -0 that no chunk can ever enter, and a numeric-string totalSize is stored as sent). That is a pre-existing hole in the same defect class as storage: the presigned-upload door answers 500 INTERNAL for a non-numeric size and accepts a negative size #27413; this PR does not touch it and does not worsen it (such a session could never complete before either).

② Semver level

  • Changeset .changeset/27412-storage-route-input.md: '@objectstack/service-storage': patch. The diff moves only that package; packages/spec, packages/client and ui/ are untouched, so one bump is right.
  • Declarations: Clause-②: no (narrowing) and Metadata-protocol: none on both the PR body and the changeset, identical. The Metadata-protocol kind is right: nothing an author writes in metadata changes. Per the Post-Task Checklist, a non-protocol change that narrows ships patch and keeps its ADR-0087 disposition: the changeset carries exactly one marker, adr-0087: not-required (no-migration-prescription), whose reason matches the diff (no key, export, type or error code removed or renamed; nothing for objectstack migrate meta to rewrite; no tombstone owed). The Narrowed: sentence names the negative size against the spec's bare z.number(), which is the narrowing ① confirms.
  • Should-fix (prose, changeset): the Narrowed: sentence could also name the whole-number chunkIndex at or past total_chunks, since UploadChunkRequestSchema.chunkIndex is int().min(0) with no upper bound; the body's "Exactly what is now refused" section already states it, so the disclosure exists and only the one-line summary is partial.
  • The level gates on the head (Check Changeset, the Lint lanes) are green, which agrees with this reading.

③ Boundary flags

Dev report 1 (comment 6495538727), deviations:

  1. storage-limits.test.ts edited: inside the dispatch's "or a sibling test". The edit widens the session row to two declared chunks through store.updateSession, the real store write, and leaves the pinned claim (bytes across chunks against the limit) unchanged. Answered, holds.
  2. Three commits, not two (now five after the patch round): history shape only; the branch is the identity and the queue merges the PR. Refusing a force-push was right. Answered.
  3. Model-free commit trailers: the AGENTS.md form. Answered.
  4. Worktree name differs between the two claims: cosmetic; one branch, one PR. Answered.
  5. Package suite not rerun after the merge of origin/main at 3a10b29c: superseded by Test Core 1/8..8/8 green on 41272f9f. Answered.
  6. os-dev-report marker on the second line: post-stamped requires the signature first. Answered.

Dev report 2 (comment 6496406943), deviations:
7. An extra commit for the ADR-0087 marker: the gate requires it for the (narrowing) arm. Right.
8. PR body edit refused by the relay: the live body read for this record carries Clause-②: no (narrowing) and the Narrowed: sentence, so the owed edit landed after that report. Should-fix (prose, PR body): the Acceptance note "three commits, not two" is stale (five); no gate reads it.

open_questions: empty in both reports. out_of_scope_findings (report 1): the spec size being z.number(): answered in ① with a follow-up recommendation. The dogfood battery the arm owes was run at 41272f9f per report 2, and Dogfood Regression Gate is green on the head. The seat's REWORK note (comment 6495583454) asked for one item, the declaration; it is present on both carriers. No flag escalates.

④ Design judgment

packages/services/service-storage/src/storage-routes.ts

  • isDeclaredByteCount: typeof === 'number', Number.isFinite, at least 0. Holds against the sys_file size number field and requireWithinUploadLimit(size: number, ...): the former Number(size) coercion is gone, so a numeric string can no longer pass the limit check as a number and reach the insert as a string. -0 passes (-0 >= 0) and is stored as -0, which serializes as 0: harmless, noted. Tried: "" is not == null, so it reaches this guard and is refused with the size message, which the test pins. Holds.
  • parseChunkIndex: /^\d+$/ (ASCII digits without the u flag) then Number.isSafeInteger. "01" reads as 1, a legal integer; an empty segment is caught by the !chunkIndexStr check above; an index past MAX_SAFE_INTEGER reads null and is refused. Holds.
  • The range check reads Number(session.total_chunks). total_chunks is required: true on sys_upload_session, typed number on UploadSessionRecord, and written unconditionally by the init door; the completion door already reads it with the same Number(...) (the missingChunks/unexpectedChunks judgment), so the two doors agree about one session, and a store that hands numerics back as strings is covered. Holds.
  • Placement after expireIfPastDeadline: that helper writes the row only when the deadline has passed (expiryStampDue(session) ? stampExpired(...) : session), so a refused in-date chunk leaves the row untouched; the test pins parts and the progress unchanged. The check is a pure read before the conditional progress write, so the retry loop that door uses for progress is unaffected. Holds.
  • Refusal envelope: sendError(res, 400, 'INVALID_REQUEST', message, { details: { chunkIndex: raw, totalChunks } }), with details a declared extra field. Echoing the raw segment (a string) is right for abc. The message picks must be 0 for one chunk and from 0 to N-1 otherwise; the N=2 case is pinned. Holds.
  • Tried to break it: total_chunks of 0 or -0 (reachable only through the init door's unvalidated totalSize, see ①): every index is refused with a truthful "declared 0 chunks" where before the chunk was stored and completion answered 409 for good; not a regression. A session without total_chunks cannot exist under the required field. A legitimate resume (resumeUpload iterates uploadedChunks .. totalChunks-1 from the progress door) stays inside the range. Nothing breaks.

packages/services/service-storage/src/storage-routes.test.ts

  • Eight refused sizes and two controls through the mounted handler, each asserting status, error.code, the message prefix and that store.createFile was never called; the controls read the stored row's size and status. NaN and Infinity are in-process-only inputs (JSON cannot carry them) and pin the predicate, not the wire. Holds against the scenario the card accepts ("abc", -5, NaN, "" each 400, no row; 0 and a positive size unchanged).

packages/services/service-storage/src/chunked-upload-integrity.test.ts

  • Each of the seven refused indexes asserts status, code, message, details, no backend uploadChunk call, unchanged parts and progress, and then completes the same upload: the strongest pin of the card's "session that can never complete". The multi-chunk range message, the last declared index accepted, and 403-before-range are controls. The pre-existing stray-chunk completion case now plants the part on the session row, the shape a pre-fix session carries, so the completion door's unexpectedChunks branch stays reachable and covered instead of going dead. The reported ablation (range check removed: 1, 7 and the multi-chunk case fail; the five non-digit cases stay refused by the strict parse) is consistent with what the suite can distinguish. Holds.

packages/services/service-storage/src/storage-limits.test.ts

  • declareTwoChunks writes total_chunks: 2 through store.updateSession, the same store the chunk door reads, so the fixture is honest and the two limit pins (413 on the chunk that crosses the limit; a retried index counted once) are unchanged in what they claim. Holds.

Findings: must-fix none. Should-fix: the spec-lane follow-up card in ① (size .int().min(0) with the door on Number.isInteger, plus the init door's totalSize/chunkSize enforcement); the Narrowed: sentence naming the out-of-range chunkIndex too (②); the stale "three commits" acceptance note (③).

Implemented-by: claude/issue-27412-storage-route-input
Reviewed-by: session_017xRBcScm27Q3R5mkYLPWdB

VERDICT: PASS


Generated by Claude Code

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

domain:devx#1 · sunhaolin · session_017xRBcScm27Q3R5mkYLPWdB

席位核对

Verdict: ACCEPT at head 41272f9f5693500d15a712731557f41502a8d982, reviewed 2026-10-29T12:29Z. This supersedes the REWORK at 3a10b29c (comment 6495583454); its one item is delivered.

check reading
REWORK item Clause-②: no (narrowing) is on the changeset (commit 1ee2b1e9, with the Narrowed: sentence) and on the PR body. The body was edited through the relay by the seat, because the relay's issue_patch requires the seat signature as the first line. The read-back is identical, and diff against the previous body shows only the two intended lines. The Fixes #27412 / Fixes #27413 lines now follow the signature line, and the Clause-② reader matches a line start anywhere.
ADR-0087 marker adr-0087: not-required (no-migration-prescription) (commit 41272f9f), required by the narrowing arm; check-adr-0087-registration passes.
Contract review PASS at CONTRACT_REVIEW_TIER on this head (comment 6496658225), by an isolated reviewer, no must-fix.
Should-fixes (1) Spec tightening plus init-door enforcement: filed as #27549 for triage. (2) The Narrowed: sentence could also name the out-of-range chunkIndex, and (3) the "three commits" acceptance note is stale. Both are prose that no gate reads, and no further code push is coming to carry them, so they stay as is. The PR body already states the chunk-index refusal in full.
Dogfood battery Owed by the narrowing arm. Run by the dev at 41272f9f: 2806 passed, 20 skipped. Dogfood Regression Gate is green on the head.
CI on head 52 check-runs, 36 success, 16 skipped, 0 failed.

Landing: ready to land, held. The landing step (relay pr_ready + automerge_enable) is the command this session's permission classifier refused on #27424 (comment 6494048067), so it is not run here. Needed from the maintainer: allow that command for this seat, or mark this PR ready and enable auto-merge by hand.


Generated by Claude Code

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

2 participants