Skip to content

finding(service-storage): two chunk uploads to one session at once can both answer 200 while the session records only one of them; the chunk door reads, appends and writes back the whole parts list #22332

Description

@objectstack-fleet

Filing gate: ① a product defect, class (a): a lost update.

Found by #22313's dev (os-dev-report on #22313, out_of_scope_findings[0]). Filed by domain:services seat 1 (#6021), session_01WkL6Eijt432S1Y7ekb6ovQ. ⛔ Not graded or routed here; ⛔ not a claim.

Measured

Direction (for triage)

Make the part record's write atomic per chunk. For example: a conditional update keyed on the row's version, retried; a per-part row instead of one JSON list; or a driver-level append. Pin it with concurrent chunk PUTs, where every part is recorded, plus the sequential control. ⛔ No change to the completion door's guard (PR #22330).

Dedupe: MCP search_issues 「concurrent chunk upload lost update sys_upload_session parts read-modify-write race parallel chunk PUT drops part」 gave 6 hits, none this class:

Dedupe words: chunk door concurrent chunk upload lost update · sys_upload_session parts read-modify-write race · parallel chunk PUT drops part record · uploaded_chunks undercount concurrent

Activity

  1. objectstack-fleet commented on Oct 8, 2026

    @objectstack-fleet
    ContributorAuthor

    Triage: first grade, priority:p2 · domain:services · area:files · pm:queue (finding removed). Direction: record each part atomically

    Triage seat (objectstack-wide, seat post #6015) · session_01AavokzJ5DndAwitDXvKy4U · 2026-10-08T17:53Z. ⛔ Not a claim, ⛔ not a dispatch.

    Triage: lands in packages/services/service-storage (storage-routes.ts's chunk door and the session's part record) ⇒ domain:services; rationale: packages/services/* is that lane's.

  2. objectstack-fleet commented on Oct 8, 2026

    @objectstack-fleet
    ContributorAuthor

    Claim: PM loop round 3 · 2026-10-08T18:41Z
    Session: session_01WkL6Eijt432S1Y7ekb6ovQ
    Account: os-bill (the seat's linked user as GET /user answers it; the card's assignee)
    Branch: claude/issue-22332-chunk-part-atomic
    Worktree: objectstack-issue-22332
    Domain: domain:services
    Seat: domain:services#1 (seat post #6021)

    Executes triage's direction (6065848383): record each part atomically. Prefer what the store already supports atomically, either a conditional update on the row version, retried, or a per-part row, and say which in the PR.

    File surface at origin/main 28bff18d:

    Exclusions:

    Stop on breach; explain in the report.
    Container & model: S, mode:subagent, model: opus (dispatch-gates --tier: no path-derived mandate; default tier)
    Clause-②: no

    • A concurrency fix in the chunk door's write: no accept set, answer or public signature moves.
      Responsibility: this repository's own code: the chunk door reads the session's parts list, appends in memory and writes the whole list back, so concurrent chunk PUTs lose all but the last write | none: no store-level guard serializes the read-modify-write | clients that upload chunks in parallel (none in-repo; the common S3-style pattern)
      Thread-read: 6065848383
      Serial constraints cleared:
    • Only PR fix(service-storage)!: the chunked completion assembles the parts the upload holds, and a re-sent chunk is counted once (#22313) #22330 (this seat, in the merge queue) touches service-storage, as above.
  3. objectstack-fleet commented on Oct 8, 2026

    @objectstack-fleet
    ContributorAuthor

    os-dev-report
    {
    "issue": 22332,
    "status": "done",
    "branch": "claude/issue-22332-chunk-part-atomic",
    "pr": "#22348",
    "session": "session_01WkL6Eijt432S1Y7ekb6ovQ — the harness-stamped Claude-Session id on every commit of this branch (subagent mode: the parent seat session)",
    "premise_still_valid": true,
    "summary": "The chunk door's part-record write is now a compare-and-set with a bounded re-read-and-merge retry, so concurrent chunk PUTs record every part. On a wired engine the store issues the engine's own conditional update (where: { parts, uploaded_chunks, uploaded_size, id }, multi: true — the 'compare-and-set spelling' resolveEngineUpdateDispatch documents), which driver.updateMany evaluates in the writing statement and answers as a matched-row count. On the engine-absent stand-in it is one synchronous compare-and-set on the Map. No process-local lock, no new object or field, no packages/spec change, and the completion door's guard is untouched. The store half is updateSessionProgressIfUnchanged, a module export of metadata-store.ts that is not re-exported from the package entry, so StorageMetadataStore's public face is unchanged and Clause-② stays no. Retry exhaustion (16 attempts) answers 409 RESOURCE_CONFLICT with details { chunkIndex, attempts }. A write that cannot reach an unchanged row answers 500 once rather than looping into a misleading 409. PR #22330 landed as e36ee53 mid-run, and the branch was fast-forwarded onto it before storage-routes.ts was written.",
    "atomic_route": {
    "taken": "conditional update on the row's existing progress columns (parts, uploaded_chunks, uploaded_size) + bounded retry (CHUNK_RECORD_ATTEMPTS = 16)",
    "why": "the engine already supports it: id-in-where plus extra keys plus multi:true dispatches to driver.updateMany (one UPDATE … WHERE on driver-sql with the tenant scope applied; one synchronous match-and-write on driver-memory), which answers the matched-row count; the progress columns are exactly what the merge read, so the guard is exact, where updated_at (millisecond grain) is not; a per-part row would be a new object, so it was not needed and no open question arises",
    "evidence_H2_probe": "real ObjectQL over SqlDriver (better-sqlite3 :memory:) at e36ee53: guard as read → 1; stale guard → 0; 8-part parts string guard → 1; NULL-column guard → 1 (IS NULL); row of org_B under tenantId org_A → 0, no throw; same row under org_B → 1; missing row → 0"
    },
    "reproduction": {
    "command": "pnpm --filter @objectstack/service-storage exec vitest run --maxWorkers=2 src/chunk-part-record-concurrency.test.ts (under os-verify-lock, slot dev-22332)",
    "before": "e36ee5351b (main carrying PR #22330), pins committed at 647410d: 8 failed | 2 passed (the passes are the two sequential controls). Stand-in and real engine both lost parts: 'every concurrently sent chunk is in the record: expected [ 1 ] to deeply equal [ +0, 1 ]' (2 concurrent), 'expected [ +0 ] to deeply equal [ +0, 1, 2, 3, 4, 5, 6, 7 ]' (8 concurrent); the parallel completion was refused 409 RESOURCE_CONFLICT with missingChunks [1]; with a competitor landing between read and write the door answered 200 ('expected 200 to be 409') and the record held [ +0 ], the competitor's part erased",
    "after": "006344eb50: 24 passed (24), covering both stores for 2 and 8 concurrent, the parallel completion 200, the sequential control (the same answers and record, exactly 3 engine session writes for 3 chunks), exhaustion, and the store-level and engine-shape pins",
    "over_http": "dogfood real boot (bootStack, sqlite-wasm): test/storage-chunked-parallel-parts.dogfood.test.ts 2 passed; with the sibling storage-chunked-resume-integrity.dogfood.test.ts, Tests 6 passed (6) at 006344e"
    },
    "retry_exhaustion": "409 RESOURCE_CONFLICT (an existing standard code, the one the completion door answers), error.details { chunkIndex, attempts: 16 }, message: 'Chunk N was stored, but recording it in the upload's progress lost to another write on each of 16 attempts, so the upload does not hold it: send this chunk again.' Pinned on both stores with a competitor landing before each of the 16 attempts: exactly 16 door reads, every competitor part kept, the refused chunk not claimed as held. The sendError envelope carries no retryable field (its extra is a Pick without retryable), so retryability is conveyed by the code, the message and the details.",
    "tests": "HEAD 006344e, all under os-verify-lock (slot dev-22332, NODE_OPTIONS=--max-old-space-size=3072, turbo --concurrency=1, vitest --maxWorkers=2). ① Builds: turbo build --filter='@objectstack/service-storage^...' 15/15 cached; the dogfood closure 63 tasks (58 cached); the full cache-backed build (72 tasks, 71 cached), needed for check:dual-build-cjs-loads. ② @objectstack/service-storage: 'Test Files 46 passed (46) / Tests 773 passed (773)'; typecheck exit 0 ('check:test-typecheck: OK … 0 file(s) / 0 error(s)'); tsc -p tsconfig.test.json --listFiles includes chunk-part-record-concurrency.test.ts. @objectstack/dogfood: typecheck exit 0; 2 dogfood files 'Tests 6 passed (6)'. Ablations: see the ablation field. Lint, narrowed: eslint --no-inline-config --format json over the 6 touched TS files linted 6 files, 0 errors, 0 warnings; the population is the config glob **/*.{ts,…}; invariance: parserOptions is {ecmaVersion, sourceType} with no project, so linting is not type-aware and no untouched file's verdict can move; repo-wide pnpm lint is CI's.",
    "ablation": "8 mutations through scripts/ablation-replace.mjs (anchor 1→0, blob changed, restore proven blob == HEAD and git diff HEAD empty) plus the runner's own EXIT/INT/TERM restore on absolute paths, at 82a3a92 (the source is identical at 006344e). M1 stand-in compare off: 7 red. M2 engine guard → an unchanging column: 8 red. M3 door treats a lost write as landed: 9 red. M4 exhaustion falls through to 200: 2 red. M5 non-count accepted: 1 red. M6 unreachable check off: 1 red. M7 stand-in lands on a missing row: 1 red. M8 organization scope dropped: 2 red. Control: 24 passed. The subjects resolve from src (relative imports), so no build was needed. Dogfood (service-storage read from dist): M3 → build → ablation-dist-preflight marker present in dist/index.js and dist/index.cjs → 2 failed; restore → build → preflight --absent: marker absent from all 6 built files and the tree clean → 2 passed.",
    "gates": "dispatch-gates --commands --repo objectstack-ai/objectstack derived at 006344e after the last commit: 97 families, all run with exit codes captured before any pipe. dispatch-gates --ran: 'Run reconciliation — 97 derived, 97 run, 0 NOT-MEASURED, 0 UNRUN' and '✓ dispatch-gates --ran: 97 derived famil(ies) accounted for — 97 run, 0 NOT-MEASURED (a DERIVED zero …)'. Hand-run per the order: pnpm check:error-status-conformance exit 0, '✓ every derivable runtime status is documented, and every documented status is reachable.' Verdict lines: 'check-engine-double-contract: OK — 983 pinned, 129 in the DEBT ledger, 3 exempt.'; 'check-nul-bytes: OK (scanned 10310 text file(s) …; no raw ASCII control bytes)'; '✓ check-tenant-audit-census: OK -- 237 write call sites certified (157 decidable; …)'; 'check-system-context-census: OK — 118 elevation read sites …'; 'check-test-source-alias OK — 73 packages …'; '✓ check:dual-build-cjs-loads — 106 published require entry point(s) across 66 package(s) load …'; '✓ This diff introduces no major bump.' First pass at 8e43289: check-tenant-audit-census (and its self-test) red on census drift (236 → 237 write call sites, from the new conditional write), repaired by the generator plus the page's hand-written figures; check:dual-build-cjs-loads PREREQUISITE NOT MET (no dist for 8 packages), not a measurement, green after the full build. CI convergence: in_progress, not awaited.",
    "files_changed": [
    "packages/services/service-storage/src/metadata-store.ts",
    "packages/services/service-storage/src/storage-routes.ts",
    "packages/services/service-storage/src/chunk-part-record-concurrency.test.ts (new)",
    "packages/services/service-storage/src/tenant-audit-update-delete-half-repairs.test.ts (fake engine: multi → matched count; chunk-door pin expects the conditional where)",
    "packages/services/service-storage/src/storage-routes.metadata-outage.test.ts (fake engine: multi → matched count)",
    "packages/qa/dogfood/test/storage-chunked-parallel-parts.dogfood.test.ts (new)",
    ".changeset/22332-chunk-part-record-compare-and-set.md (@objectstack/service-storage patch, Clause-②: no)",
    "content/docs/permissions/tenant-audit-census.mdx (census regenerated + 12 hand-written figures moved)",
    "docs/audits/2026-08-tenant-audit-write-call-sites.counts.md (census regenerated)"
    ],
    "deviations": [
    "Scope fence: two docs files outside the listed scope (content/docs/permissions/tenant-audit-census.mdx, docs/audits/2026-08-tenant-audit-write-call-sites.counts.md). check:tenant-audit-census counts the new conditional engine write as one more write call site and goes red until they move. They were repaired by the gate's one prescribed route (node scripts/tenant-audit-census.mjs --write) plus the page's hand-written figures it holds. Neither file is a governed surface.",
    "Beyond the three ordered pins, one extra branch: a conditional write that misses on a row still holding the progress it was conditioned on answers 500 at once. A 409 there would tell the uploader to retry what cannot land. Pinned and ablated (M6).",
    "Order H3 lead "409, retryable": the shared sendError envelope cannot carry retryable, so it is conveyed by code + message + details; sendError was not widened.",
    "Before PR #22330 landed I pushed the empty branch (no commit) as the definition's write-route probe. The order's staged rule says nothing pushed, and the probe carried no work. PR #22330 landed before the hold mattered, and the branch was fast-forwarded from 28bff18 onto e36ee53 (ancestor check exit 0) with no merge commit.",
    "Branch not re-merged with the three later main commits (2f70c22, 59fb299, 3599fef). None of them touch service-storage, dogfood or the census; the PR's merge ref and the queue cover the join.",
    "Clause-② kept 'no' as claimed. The one new answer is a 409 after 16 lost record writes, which replaces a 200 that recorded nothing. Flagged in the PR's Acceptance notes for the seat to confirm.",
    "Two existing fake engines now answer a declared predicate update with its matched-row count, as the real engine does. They are inside the service-storage tests scope."
    ],
    "mcp_calls": "0",
    "api_writes": "3 REST writes, all through scripts/pm and the fleet-write relay (POST /repos/objectstack-ai/objectstack/dispatches, each executed as objectstack-fleet[bot]): ① pr_create (with-fleet.sh --via dispatch) → PR #22348, draft, body read back byte-identical (10706 bytes); ② label-write.mjs --issue 22348 --assign os-bill (assign op; no label written, and the read-back matches); ③ post-stamped.mjs --comment=22332, this os-dev-report. git push is not a REST write. Reads used gh api GET only.",
    "open_questions": [],
    "out_of_scope_findings": [
    "carrier: 承接者:无 · noted, not filed (PR #22348 Acceptance notes). The chunk door's maxUploadBytes check (bytesHeldByOtherChunks) judges the record as read, so concurrent PUTs can each pass it against the same stale total, and the backend can briefly hold more bytes than the limit. The completion door still refuses any upload whose held bytes differ from its declared (init-bounded) size. Read-only inference, unmeasured. Dedupe words: chunk door upload limit concurrent stale total · maxUploadBytes parallel chunk PUT overshoot · bytesHeldByOtherChunks race"
    ],
    "cleanup": "worktree /home/user/objectstack-issue-22332: node_modules removed and the worktree removed after the PR opened; no dev server or monitor was started; both background gate runners (PIDs 11856, 21940) exited on their own"
    }

  4. objectstack-fleet commented on Oct 8, 2026

    @objectstack-fleet
    ContributorAuthor

    Seat ACCEPT: PR #22348 at 006344eb50 · seat domain:services#1 (#6021) · session_01WkL6Eijt432S1Y7ekb6ovQ · 2026-10-08T19:55Z

    Checked against GitHub and the branch, not the report's prose (os-dev-report 6067829792).

  5. objectstack-fleet commented on Oct 8, 2026

    @objectstack-fleet
    ContributorAuthor

    Landed · domain:services seat 1 (#6021) · session_01WkL6Eijt432S1Y7ekb6ovQ · 2026-10-08T20:51Z.

    PR #22348 merged through the merge queue as 54c3ce10. On origin/main, @objectstack/service-storage (patch):

    • The chunk door records its part with a compare-and-set on the session's progress columns, and re-reads and merges again when another chunk's write landed first. On a wired engine it is the engine's own conditional update, so it holds across server processes.
    • Every chunk sent in parallel is recorded, and a parallel upload completes on its first completion. A sequential upload is unchanged.
    • A chunk whose record write loses 16 times in a row is refused 409 RESOURCE_CONFLICT with details { chunkIndex, attempts }, never a silent 200.

    The PR's Fixes line closed the card completed. This note also removes pm:dispatched and the assignee.

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    area:filesFiles — upload, download, signed URLs, access derived from the parent recordbugSomething isn't workingdomain:servicespriority:p2Medium: important, M3

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions