fix(service-storage): chunks sent in parallel to one chunked upload each record their part (#22332)

.changeset/22332-chunk-part-record-compare-and-set.md

@@ -0,0 +1,13 @@
+---
+'@objectstack/service-storage': patch
+---
+
+fix(service-storage): chunks sent in parallel to one chunked upload each record their part
+
+Clause-②: no
+
+`PUT /storage/upload/chunked/:uploadId/chunk/:chunkIndex` merges its chunk into the upload session's record of the chunks it holds (`parts`, `uploaded_chunks`, `uploaded_size` on `sys_upload_session`). It read that record, merged in memory and wrote the whole record back, so two chunk PUTs to one upload at the same time both answered `200` while the record kept only one of them: `GET …/progress` undercounted, and the completion was refused `409 RESOURCE_CONFLICT` naming the chunk the record had lost until the client sent it again.
+
+The record is now written with a compare-and-set: the write lands only while the row still holds the progress the chunk door read, and when another chunk's write landed first the door reads the record again and merges again. On a wired data engine this is the engine's own conditional update, evaluated in the same statement that writes, 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 to another write on 16 attempts in a row is refused `409 RESOURCE_CONFLICT`, with `error.details` `{ chunkIndex, attempts }`: its bytes are stored, but the upload does not hold it. Send that chunk again.

content/docs/permissions/tenant-audit-census.mdx

@@ -83,7 +83,7 @@ what moved this page's population from 225 to 227; nothing about the two sites
 changed, only whether this instrument could see them.
 
 **The expensive failure direction is a keyword.** Sites whose receiver the author
-typed `any` have no type to read, and there are 50 of them — just over a fifth
+typed `any` have no type to read, and there are 51 of them — just over a fifth
 of the population, concentrated in exactly the seed and bootstrap paths this
 control exists for. Scoring an unreadable receiver as "not an engine" would have
 dropped every one of them silently, with a clean exit and a smaller number that
@@ -122,7 +122,7 @@ are reported as `undecidable` rather than assumed either way.
 
 The same holds twice over for the context. An options argument spelled as a
 literal can be read; one spelled `options`, `{ ...opts }`, or handed through a
-forwarding shim cannot, and **52 of the 236 sites are spelled that way**. A
+forwarding shim cannot, and **53 of the 237 sites are spelled that way**. A
 context resolved from an inline literal or a local `const` can be tested for
 `isSystem`; one arriving from a helper call cannot.
 
@@ -150,8 +150,8 @@ now **0**: nothing on this surface threads a context that provably lacks the fla
 
 **"No tenant context" counted sites it had not read.** An options argument the
 walker could not parse was folded into the same bucket as one it had read and
-found empty. That published **60 sites "carrying no tenant context at all"**
-when 8 said so and 52 were simply unread — an over-claim in the *alarming*
+found empty. That published **61 sites "carrying no tenant context at all"**
+when 8 said so and 53 were simply unread — an over-claim in the *alarming*
 direction, on the very figure this page tells other cards to cite. `carries` is
 now three-valued, and an unreadable argument can never contribute to the
 provable count.
@@ -187,10 +187,10 @@ reproduce them. Where it disagrees, it disagrees on the page:
 
 | carried figure | where it survives | this census |
 | :--- | :--- | ---: |
-| 175 write call sites | quoted in the merged changeset | **236** |
-| 24 carrying no tenant context | quoted in the merged changeset | **2** provable and tenancy-enabled; **31** more whose options argument is unreadable |
-| 127 of 175 statically decidable, 48 runtime-parameter-name sites | restated on the `isSystem`-scoping card | **156 of 236** decidable, **80** undecidable |
-| 135 (77%) silenced by the `isSystem` guard before the posture gate | the lost issue body — **no surviving corroboration** | **not reproduced**: 125 decidably elevated, 0 decidably not, 103 undecidable |
+| 175 write call sites | quoted in the merged changeset | **237** |
+| 24 carrying no tenant context | quoted in the merged changeset | **2** provable and tenancy-enabled; **32** more whose options argument is unreadable |
+| 127 of 175 statically decidable, 48 runtime-parameter-name sites | restated on the `isSystem`-scoping card | **157 of 237** decidable, **80** undecidable |
+| 135 (77%) silenced by the `isSystem` guard before the posture gate | the lost issue body — **no surviving corroboration** | **not reproduced**: 125 decidably elevated, 0 decidably not, 104 undecidable |
 | 141 and 132, two independent re-derivations | the card that filed this work | — |
 
 **The differences are not reconciled, and deliberately so.** The old census's
@@ -200,21 +200,21 @@ be stated is what this instrument counts, which is written above and re-runnable
 at any commit.
 
 Two structural facts do plausibly widen this reading against any hand or regex
-one, and both are counted in the generated tables below: the 50 sites reached
+one, and both are counted in the generated tables below: the 51 sites reached
 through an erased (`any`) receiver, and the 50 that name their object through a
 `const` rather than inline. An instrument that read either the way a person does
 would report a smaller number and would not say so.
 
 The fourth row is the one worth flagging to anyone citing it. **The 135 / 77%
 figure has no surviving corroboration anywhere in the tree.** This census reads
-125 of 236 (53%) as decidably elevated, with 103 more whose elevation is a
+125 of 237 (53%) as decidably elevated, with 104 more whose elevation is a
 run-time fact — so the claim is neither confirmed nor refuted, and the honest
 answer is that a static reading cannot settle it.
 
-⇒ **Cite `2 / 236`, and say what it is**: the sites whose options argument was
+⇒ **Cite `2 / 237`, and say what it is**: the sites whose options argument was
 READ and holds no tenant context, against a decidably tenancy-enabled object.
 That is the control's provable yield surface. ⛔ Do not cite it as "the sites
-without tenant context" — **31 further sites** have an options argument this
+without tenant context" — **32 further sites** have an options argument this
 cannot read, and they are neither in nor out.
 
 {/* BEGIN GENERATED: tenant-audit-census (scripts/tenant-audit-census.mjs) — DO NOT EDIT */}
@@ -223,28 +223,28 @@ cannot read, and they are neither in nor out.
 
 | what | count |
 | :--- | ---: |
-| write call sites on the application surface | **236** |
-| …whose object name is statically decidable | 156 |
+| write call sites on the application surface | **237** |
+| …whose object name is statically decidable | 157 |
 | …whose object name is chosen at run time | 80 |
-| …against an object with tenancy ENABLED | 155 |
+| …against an object with tenancy ENABLED | 156 |
 | …against an object that declares tenancy off | 1 |
 | threading a tenant context | 176 |
 | PROVABLY carrying none (options read, no context key) | **8** |
 | …of those, against a decidably tenancy-enabled object | **2** |
-| options argument UNREADABLE — may or may not carry one | 52 |
-| …of those, against a decidably tenancy-enabled object | 31 |
+| options argument UNREADABLE — may or may not carry one | 53 |
+| …of those, against a decidably tenancy-enabled object | 32 |
 | threading a decidably ELEVATED (`isSystem`) context | 125 |
 | threading a context that is decidably NOT elevated | 0 |
-| threading a context whose elevation is a run-time fact | 103 |
+| threading a context whose elevation is a run-time fact | 104 |
 
 | how the instrument reached the site | count |
 | :--- | ---: |
 | receiver carried a readable engine type | 186 |
-| receiver erased, placed by the object NAME | 30 |
+| receiver erased, placed by the object NAME | 31 |
 | receiver erased, placed by an `object: string` PARAMETER | 15 |
 | receiver erased, placed by an `UNTYPED_RECEIVERS` row | 5 |
 
-| object name spelled inline | 106 |
+| object name spelled inline | 107 |
 | object name spelled through a `const` | 50 |
 | object name is an `object: string` parameter | 19 |
 | object name is some other run-time expression | 61 |
@@ -297,13 +297,13 @@ holds still. They are required to be HERE and to say WHEN they were true;
 their values are not compared. The reasoning, and the measurement behind it,
 are in `scripts/check-tenant-audit-census.mjs`.
 
-Measured on 2026-10-08 at `4b40ca31f`.
+Measured on 2026-10-08 at `8e432893f`.
 
 | corpus scale (not enforced) | count |
 | :--- | ---: |
-| tracked non-test sources scanned | 618 |
+| tracked non-test sources scanned | 621 |
 | engine-shaped types recognised | 70 |
 | declared objects in the registry | 117 |
-| same-named calls subtracted as non-engine | 161 |
+| same-named calls subtracted as non-engine | 162 |
 
 {/* END GENERATED: tenant-audit-census */}

docs/audits/2026-08-tenant-audit-write-call-sites.counts.md

@@ -33,19 +33,19 @@ silent, and `node scripts/tenant-audit-census.mjs --write` is the resolution.
 
 | Measure | Value |
 |---|---:|
-| Write call sites | 236 |
-| Object name statically decidable | 156 |
+| Write call sites | 237 |
+| Object name statically decidable | 157 |
 | Object name chosen at run time | 80 |
-| Against a tenancy-enabled object | 155 |
+| Against a tenancy-enabled object | 156 |
 | Against an object declaring tenancy off | 1 |
 | Threading a tenant context | 176 |
 | Provably carrying none | 8 |
 | …and decidably tenancy-enabled | 2 |
-| Options argument unreadable | 52 |
-| …and decidably tenancy-enabled | 31 |
+| Options argument unreadable | 53 |
+| …and decidably tenancy-enabled | 32 |
 | Threading a decidably elevated context | 125 |
 | Threading a decidably non-elevated context | 0 |
-| Threading a context of undecidable elevation | 103 |
+| Threading a context of undecidable elevation | 104 |
 
 ## Subtractions the census could NOT defend — enforced
 
@@ -90,14 +90,14 @@ holds still. They are required to be HERE and to say WHEN they were true;
 their values are not compared. The reasoning, and the measurement behind it,
 are in `scripts/check-tenant-audit-census.mjs`.
 
-Measured on 2026-10-08 at `4b40ca31f`.
+Measured on 2026-10-08 at `8e432893f`.
 
 | corpus scale (not enforced) | count |
 | :--- | ---: |
-| tracked non-test sources scanned | 618 |
+| tracked non-test sources scanned | 621 |
 | engine-shaped types recognised | 70 |
 | declared objects in the registry | 117 |
-| same-named calls subtracted as non-engine | 161 |
+| same-named calls subtracted as non-engine | 162 |
 
 ## Every site
 
@@ -255,4 +255,4 @@ Measured on 2026-10-08 at `4b40ca31f`.
 | `packages/services/service-storage/src/metadata-store.ts` | `update` | `sys_file` | enabled | options unreadable | 1 |
 | `packages/services/service-storage/src/metadata-store.ts` | `delete` | `sys_upload_session` | enabled | options unreadable | 1 |
 | `packages/services/service-storage/src/metadata-store.ts` | `insert` | `sys_upload_session` | enabled | options unreadable | 1 |
-| `packages/services/service-storage/src/metadata-store.ts` | `update` | `sys_upload_session` | enabled | options unreadable | 1 |
+| `packages/services/service-storage/src/metadata-store.ts` | `update` | `sys_upload_session` | enabled | options unreadable | 2 |

packages/qa/dogfood/test/storage-chunked-parallel-parts.dogfood.test.ts

@@ -0,0 +1,117 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+//
+// [#22332] Chunk PUTs sent in parallel to one upload each record their part —
+// over the HTTP doors of a REAL boot.
+//
+// The package suite (`service-storage/src/chunk-part-record-concurrency.test.ts`)
+// pins the rule at the route handlers, over the engine-absent stand-in and a
+// real ObjectQL on SqlDriver. This file sends the chunks the way a parallel
+// (S3-style) uploader does — every PUT in flight at once — through the booted
+// stack's own engine and driver, then reads the progress back and completes the
+// upload listing every part. Before the fix both PUTs answered `200`, the
+// progress counted one chunk, and the completion was refused `409` naming the
+// chunk the record had lost.
+//
+// Not eligible for the shared showcase project: it boots its own storage root.
+
+import { describe, it, expect, beforeAll, afterAll } from 'vitest';
+import { mkdtempSync } from 'node:fs';
+import { join } from 'node:path';
+import { tmpdir } from 'node:os';
+import { bootStack, type VerifyStack } from '@objectstack/verify';
+import { StorageServicePlugin } from '@objectstack/service-storage';
+import { attachmentsFixtureStack, attachmentsFixtureSecurity } from './fixtures/attachments-fixture.js';
+
+const MIB = 1024 * 1024;
+/** The chunk size the init door floors every upload at. */
+const MIN_CHUNK = 5 * MIB;
+
+describe('[#22332] parallel chunk PUTs each record their part, over a real boot', () => {
+  let stack: VerifyStack;
+  let token: string;
+
+  const json = { 'Content-Type': 'application/json' };
+  const auth = () => ({ Authorization: `Bearer ${token}` });
+
+  const init = async (totalSize: number) => {
+    const res = await stack.api('/storage/upload/chunked', {
+      method: 'POST',
+      headers: { ...json, ...auth() },
+      body: JSON.stringify({ filename: 'parallel.bin', mimeType: 'application/octet-stream', totalSize }),
+    });
+    expect(res.status, await res.clone().text()).toBe(200);
+    return ((await res.json()) as any).data as { uploadId: string; resumeToken: string; fileId: string; totalChunks: number };
+  };
+
+  const putChunk = async (uploadId: string, resumeToken: string, chunkIndex: number, bytes: Uint8Array<ArrayBuffer>) => {
+    const res = await stack.api(`/storage/upload/chunked/${uploadId}/chunk/${chunkIndex}`, {
+      method: 'PUT',
+      headers: { ...auth(), 'Content-Type': 'application/octet-stream', 'x-resume-token': resumeToken },
+      body: bytes,
+    });
+    expect(res.status, await res.clone().text()).toBe(200);
+    return ((await res.json()) as any).data.eTag as string;
+  };
+
+  const progress = async (uploadId: string) => {
+    const res = await stack.api(`/storage/upload/chunked/${uploadId}/progress`, { headers: auth() });
+    expect(res.status, await res.clone().text()).toBe(200);
+    return ((await res.json()) as any).data as Record<string, unknown>;
+  };
+
+  const fill = (byte: number, n: number) => new Uint8Array(n).fill(byte);
+
+  beforeAll(async () => {
+    const rootDir = mkdtempSync(join(tmpdir(), 'chunked-parallel-dogfood-'));
+    stack = await bootStack(attachmentsFixtureStack as never, {
+      security: attachmentsFixtureSecurity(),
+      // `bindToSettings:false` keeps the constructor rootDir; the size limit is
+      // not what this file is about.
+      extraPlugins: [new StorageServicePlugin({ adapter: 'local', local: { rootDir }, bindToSettings: false })],
+    });
+    token = await stack.signIn();
+  }, 120_000);
+
+  afterAll(async () => {
+    await stack?.stop();
+  });
+
+  it('two chunks sent at once are both recorded, and the upload completes with the whole file', async () => {
+    const chunk0 = fill(0x61, MIN_CHUNK);
+    const chunk1 = fill(0x62, 10);
+    const { uploadId, resumeToken, fileId } = await init(MIN_CHUNK + 10);
+
+    const [eTag0, eTag1] = await Promise.all([
+      putChunk(uploadId, resumeToken, 0, chunk0),
+      putChunk(uploadId, resumeToken, 1, chunk1),
+    ]);
+
+    expect(await progress(uploadId)).toMatchObject({ uploadedChunks: 2, uploadedSize: MIN_CHUNK + 10 });
+    const res = await stack.api(`/storage/upload/chunked/${uploadId}/complete`, {
+      method: 'POST',
+      headers: { ...json, ...auth() },
+      body: JSON.stringify({
+        uploadId,
+        parts: [
+          { chunkIndex: 0, eTag: eTag0 },
+          { chunkIndex: 1, eTag: eTag1 },
+        ],
+      }),
+    });
+    expect(res.status, await res.clone().text()).toBe(200);
+    expect(((await res.json()) as any).data).toMatchObject({ fileId, size: MIN_CHUNK + 10 });
+  });
+
+  it('eight chunks sent at once are all recorded', async () => {
+    const { uploadId, resumeToken, totalChunks } = await init(8 * MIN_CHUNK);
+    expect(totalChunks).toBe(8);
+    // Small, distinct sizes: the chunk door records what it is sent, and a lost
+    // part shows in both counts.
+    const chunks = Array.from({ length: 8 }, (_, i) => fill(0x61 + i, 10 + i));
+
+    await Promise.all(chunks.map((bytes, i) => putChunk(uploadId, resumeToken, i, bytes)));
+
+    const total = chunks.reduce((sum, c) => sum + c.length, 0);
+    expect(await progress(uploadId)).toMatchObject({ uploadedChunks: 8, uploadedSize: total });
+  });
+});