Repository navigation
fix(service-storage): answer an upload's organization change with 409 RESOURCE_CONFLICT, not an outage 500 - #22216
Conversation
…an organization change with 409, not an outage 500 An uploader whose active organization changed after starting an upload names a row the doors' scoped by-id write cannot reach. The engine's miss came back as 500 INTERNAL carrying the store's "Restore the data engine" text. Both doors now ask the store (organizationOutOfWriteReach) before any write and answer 409 RESOURCE_CONFLICT naming the change, with one operator warning naming the cause. The engine-absent stand-in does not scope and keeps its answers. Claude-Session: https://claude.ai/code/session_01WkL6Eijt432S1Y7ekb6ovQ Co-authored-by: Claude <noreply@anthropic.com>
…dapter so a same-organization completion assembles Claude-Session: https://claude.ai/code/session_01WkL6Eijt432S1Y7ekb6ovQ Co-authored-by: Claude <noreply@anthropic.com>
📓 Docs Drift CheckThis PR changes 1 package(s): 2 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
⛔ 2 release-owned page(s) also name something this change touched. These are read-only:
What this run could not see
Coarse fallback — 8 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin bcd3417b3c44a821688f56636faf8d4ca560fae0 && git checkout bcd3417b3c44a821688f56636faf8d4ca560fae0
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 7d7943dd0dfba6c98afa2200a08983ec85f9849a af2811c51c814a5cca6ce315b3ff8fe0ca01c88e && git checkout -B drift-repro 7d7943dd0dfba6c98afa2200a08983ec85f9849a && git merge --no-ff af2811c51c814a5cca6ce315b3ff8fe0ca01c88e
node scripts/docs-audit/affected-docs.mjs --json 7d7943dd0dfba6c98afa2200a08983ec85f9849a
|
… and the progress door's expiry stamp with 409 RESOURCE_CONFLICT, not an outage 500 (objectstack-ai#22251) Fixes objectstack-ai#22218 Clause-②: no The chunk door and the progress door of `@objectstack/service-storage` now answer an uploader whose active organization changed after starting an upload with the same `409 RESOURCE_CONFLICT` that PR objectstack-ai#22216 gives at the commit and chunked-completion doors. They used to answer `500 INTERNAL` with the store's data-engine outage text. The progress door asks only when the session's expiry stamp is due. Which organization an upload belongs to does not change, and the tenant scope of every write is unchanged. ## Rulings carried (triage `6054616896`, verbatim) - "Direction: the same 409 that PR objectstack-ai#22216 gives, asked before the write" - "⛔ No change to which organization owns an upload (objectstack-ai#22046's ruling). ⛔ No catch-all that would hide a real engine failure." ## What changed Only `packages/services/service-storage/src/storage-routes.ts` changed in source. `metadata-store.ts` is untouched: PR objectstack-ai#22216's `organizationOutOfWriteReach` answers both writes as it stands. - **Chunk door** (`PUT …/upload/chunked/:uploadId/chunk/:chunkIndex`): it calls `requireStartingOrganization(writeContext, [session], 'chunk', …)` right after the resume-token check. That is before the expiry stamp, before the chunk goes to the storage backend, and before the progress write. The door writes only the session row, so only that row is asked about. - The resume token stays first. It is this door's proof that the caller may act on the upload, and the door has no uploader rule (objectstack-ai#22046's ruling covers the commit, chunked-completion and progress doors). A caller without the token still gets `403 INVALID_RESUME_TOKEN` and learns nothing about organizations. - Before this change, a refused chunk was sent to the backend first. Its bytes landed, and only the session write after it failed. - **Progress door** (`GET …/upload/chunked/:uploadId/progress`): the expiry decision is split out of `expireIfPastDeadline` as `expiryStampDue(session)`, and the write as `stampExpired(session, context)`. `expireIfPastDeadline` is now the composition of the two, so its behaviour at the chunk and chunked-completion doors is unchanged. The progress door computes `due` once. When the stamp is due, it calls `requireStartingOrganization(writeContext, [stored], 'progress', …)` and then stamps. The call comes after the uploader rule, as at the other doors. - `requireStartingOrganization`, the `409` constants and the warn line are reused unchanged. The warn names the door (`chunk:` / `progress:`), the upload id, the starting organization and the active one. No new error code and no `packages/spec` change. - **Changeset:** `.changeset/22218-chunk-progress-org-change-4xx.md`, a `patch` for `@objectstack/service-storage`. ### Why the progress door asks only when the stamp is due (H3's decision) - The question is about the reach of a write. A progress read of a live session, or of one already `expired`, writes nothing, so there is no reach to miss. It answers `200` today from any organization, and the card does not ask to change that. - Asking on every call would turn that `200` into `409`. That narrows a published accept set (`Clause-②: no (narrowing)`), against the ruled `Clause-②: no`. It would also contradict PR objectstack-ai#22216's own wall pin, "UNCHANGED — the progress door still answers the uploader after the switch". - `due` is computed once and drives both the question and the stamp. The question and the write therefore cannot disagree about a deadline that passes between two clock reads. - This choice is pinned. Ablation leg C (below) removes the `due &&` guard, and the pins go red. ## Before and after, on the booted organization wall Measured in `packages/plugins/organizations` with `bootStack`, `OrganizationsPlugin`, `StorageServicePlugin` and posture `isolated`. The uploader switches organization through the product's own `set-active` door. To put a session past its deadline, `expires_at` is moved into the past as SYSTEM, in the session's own organization. | door | before (base `f4bed58341`) | after (`c26b15c37c`) | |---|---|---| | chunk, organization changed | `500 INTERNAL`, "StorageMetadataStore: sys_upload_session update failed against the data engine — the persisted upload progress is stale … Restore the data engine …", cause "Record … not found in sys_upload_session" | `409 RESOURCE_CONFLICT`, "This upload was started in a different organization than your active one. …"; session row unchanged; switching back lands the chunk (`200`, `uploaded_chunks` 1) | | progress past `expires_at`, organization changed | `500 INTERNAL`, the same text | `409 RESOURCE_CONFLICT`; the session is not stamped; switching back answers `200` `expired` and stamps it | | chunk, same organization | `200` | `200` (control) | | progress past `expires_at`, same organization | `200` `expired` | `200` `expired` (control) | | progress of a live session, organization changed | `200` | `200` (unchanged) | H1 and H2 were measured as stated. Both are throws from the scoped `UPDATE`, wrapped by `engineOp` as `StorageMetadataStoreError`. The "before" column comes from the new wall file run against the base's `storage-routes.ts`. That file was swapped in on disk inside a trap-armed script and restored to the HEAD blob afterwards. Result: "Tests 2 failed | 8 passed (10)", and the failing two are exactly the two negative pins. ## Tests - `storage-routes.metadata-outage.test.ts`: two new blocks inside the existing objectstack-ai#22175 block, plus one stand-in case. All use that file's existing fake engine; no new double was pinned. - **Chunk door (6 cases):** - From another organization: `409` with the code and first sentence. The backend `uploadChunk` is never called, the session row is unchanged, and the warn names `chunk:`, the upload and both organizations. - Asked before the expiry write: from another organization the session is not stamped, while the same organization gets `410` and the stamp. - Same-organization control: `200`. - A same-organization **real engine failure** still answers `500 INTERNAL` with "Restore the data engine". - An org-less session stays in reach (`200`). - The resume token runs first: `403 INVALID_RESUME_TOKEN`, with no mention of organizations. - **Progress door past its deadline (6 cases):** - From another organization: `409`, not stamped, and the warn names `progress:`. - Same-organization control: `200` `expired`, stamped. - A same-organization real engine failure on the stamp still answers `500` with "Restore the data engine". - Asked only when due: an already `expired` session answers `200` from the other organization. - An org-less session is stamped from any organization (`200`). - The ownership rule runs first: `403 PERMISSION_DENIED`. - **Stand-in:** the engine-absent store answers both doors from any organization, as before. - `storage-upload-organization-change.wall.test.ts`: 4 new cases. For both doors: the negative pin with switch-back, and a same-organization control. ## Ablation (from committed `c26b15c37c`) Three legs, each through `scripts/ablation-replace.mjs` in WRAP mode under the verify lock. Each anchor hit x1 and went to x0, and each restore was proven by the tool ("blob == HEAD (5fe0c1a) and `git diff HEAD` is empty"). Afterwards: anchors 1/1, markers 0, status clean. Both suites resolve the subject from `src`: the organizations vitest config aliases `@objectstack/service-storage` to `src/index.ts`, and the unit file imports `./storage-routes.js`. So no `dist` leg applies. | leg | mutation | wall | unit | |---|---|---|---| | A | the chunk door's question removed (blob → `091b433f37f1`) | 1 red: the chunk pin, which received `500 INTERNAL` with the outage text | 3 red: `409`, expiry-first, token-then-409 | | B | the progress door's question removed (blob → `f621826ca74d`) | 1 red: the progress pin, `500 INTERNAL` with the outage text | 1 red | | C | the `due &&` guard removed, so the question is asked on every progress call (blob → `78b594f67293`) | 1 red: objectstack-ai#22216's "UNCHANGED — the progress door still answers…" pin, which received `409` | 2 red: "asked only when the stamp is due" and objectstack-ai#22216's progress-read pin | ## Gates (at `c26b15c37c`) - **Derivation:** `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack`, run from this worktree after `git fetch origin main` (origin/main and the merge base are both `f4bed58341`). It derived 68 commands: the seat's 66 plus `check:dispatcher-error-vocabulary` and `check-dev-prereqs.mjs --self-test`. - **Reconciliation:** "✓ dispatch-gates --ran: 68 derived famil(ies) accounted for — 68 run, 0 NOT-MEASURED (a DERIVED zero — all 68 recorded an exit code and none of them is 3)". - **All 68 exit 0.** Two of them first answered PREREQUISITE NOT MET (exit 3), `check:dual-build-cjs-loads` and `check:i18n`. After a whole-workspace build (`turbo run build --filter='!@objectstack/docs' --concurrency=1`, 72 tasks, VERDICT command-exit 0), both exit 0: - "✓ check:dual-build-cjs-loads — 106 published require entry point(s) across 66 package(s) load; …" - "check-i18n-bundles: OK (9 package(s) — all bundles in sync, no undeclared authoring keys)" - **Error-envelope families named by the derivation:** - `check:route-envelope`: exit 0. - `check:dispatcher-error-vocabulary`: "OK — 54 unregistered code-stamping site(s), all classified". - **Also run beyond the derivation, all exit 0:** - `check:error-code-casing` ("no unlisted lowercase error codes in 7802 scanned file(s)") - `check:error-status-conformance` ("every derivable runtime status is documented, and every documented status is reachable") - `@objectstack/spec check:error-code-provenance` - `check:durability-log-level` - `check:tenant-chokepoint` - **Other gates:** `check:engine-double-contract` "OK — 982 pinned, 129 in the DEBT ledger, 3 exempt". `check:nul-bytes` "OK (… no raw ASCII control bytes)". - **Package suites at `c26b15c37c`:** - `@objectstack/service-storage`: 43 files, 717 tests pass (704 before, plus 13 new). - `@objectstack/organizations`: 11 files, 151 tests pass (147, plus 4). - `typecheck` exits 0 for both, with `check:test-typecheck: OK`. - `tsc -p tsconfig.test.json --listFilesOnly` lists both edited test files. - **Lint, narrowed:** `eslint --no-inline-config --format json` over the 3 touched TypeScript files gave files=3, errors=0, warnings=0. - Population: no "file ignored" warning, and `--print-config` resolves rules for each file under `eslint.config.mjs`. - Invariance: that config never enables type-aware linting (`parserOptions.project` is null), so this diff cannot move the verdict on an untouched file. - The full `pnpm lint` is CI's. ## Acceptance notes - **The seed helper's org-less arm was vacuous; fixed here, because the new pins depend on it.** The helper in `storage-routes.metadata-outage.test.ts` took `sessionOrganization: string | undefined = 'org_a'`, and `seed(store, undefined)` takes the default. So PR objectstack-ai#22216's case "a session row with no organization still answers 409 when its FILE row is out of reach" seeded the session in `org_a`. It passed for that reason, not for the reason its title gives. - Measured here: the two new org-less cases first answered `409`, not `200`, in the first run at `3be6d508d4`. - The helper now takes `null` for "no organization". Each org-less case, the objectstack-ai#22175 one included, asserts its premise (`organization_id` undefined), and all three pass. - **Release text:** PR objectstack-ai#22216's pending changeset (`.changeset/22175-upload-org-change-4xx.md`) lists "the progress door" under **Unchanged**. That was true of that change. This PR's changeset states the progress door's new answer for a session past its deadline, so the two entries will sit side by side in one release's CHANGELOG. Neither changeset is edited here. --- _Generated by [Claude Code](https://claude.ai/code/session_01WkL6Eijt432S1Y7ekb6ovQ)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
Fixes #22175
Clause-②: no
The commit door and the chunked-completion door of
@objectstack/service-storagenow answer an uploader whose active organization changed after starting an upload with409 RESOURCE_CONFLICT, in the ADR-0112 envelope, with a message that names the organization change. They used to answer500 INTERNALwith the store's data-engine outage text. The operator log gets one warning that names the cause. Which organization an upload belongs to does not change, and the tenant scope of every write is unchanged.Rulings carried (triage
6052856225, verbatim)Clause-②: no. Patch changeset for the storage service."What changed
metadata-store.ts: a new module-level function,organizationOutOfWriteReach(store, row, context). It is not re-exported from the package entry, and the public face is unchanged. It answers one question about the by-id write the store would issue undercontext: can that write reachrow? If it cannot, the function returns the organization the row was started in. Otherwise it returnsnull.nullfor the engine-absent stand-in, which does not scope its writes. A module-privateWeakSetfilled in the constructor records which stores are engine-backed.nullwhen no organization is threaded. It reuseswriteOptionsFor, the same rule that decides whether the write carries atenantId.nullfor a row stamped with no organization, because the driver keeps org-less rows in reach.storage-routes.ts: a new gate,requireStartingOrganization(writeContext, rows, …). It runs right after the ownership rule from fix(service-storage)!: the upload commit, chunked-completion and progress doors act only for the uploader #22170 and before any write, so only the uploader ever reaches it. It asks the store about every row the door is about to write. When a row is out of reach, the gate logs onewarnline naming the door, the upload id, the starting organization and the active one. It then answers409/RESOURCE_CONFLICTwith a constant message that names no organization.POST …/upload/complete): it asks about thesys_filerow. The write context is now bound once and passed both to the question and toupdateFile, so the two cannot differ.POST …/upload/chunked/:uploadId/complete): it asks about the upload session and its file, and it asks before the expiry stamp, which is the first scoped write. The file row that the ownership rule reads is now bound to a variable rather than read inline..changeset/22175-upload-org-change-4xx.md, apatchfor@objectstack/service-storage.Code choice.
RESOURCE_CONFLICTis the standard-catalog member that HTTP 409 derives, so no ledger entry and nopackages/specchange were needed. The request conflicts with where the upload stands, and the caller can resolve it: the same call succeeds after switching back, and the wall pin shows this.PERMISSION_DENIEDwould merge this answer with the ownership rule's one refusal, and this caller is the uploader.PRECONDITION_REQUIRED(428) means the request is missing a precondition, and nothing is missing here.Before and after, on the booted organization wall
Measured with
packages/plugins/organizations:bootStackwithOrganizationsPluginandStorageServicePlugin, postureisolated, and the uploader switched through the product's ownset-activedoor.1e5d322c)af2811c51c)500 INTERNAL, "StorageMetadataStore: sys_file update failed against the data engine … Restore the data engine …", cause "Record … not found in sys_file"409 RESOURCE_CONFLICT, "This upload was started in a different organization than your active one. …"; row unchanged; switching back commits it (200)500 INTERNAL, the same text forsys_upload_session409 RESOURCE_CONFLICT; session and file rows unchanged; switching back completes it (200)200200(control)200200(unchanged)H1, measured: the scoped miss is a throw, not zero rows. The SQL driver's scoped
UPDATE … WHERE idtouches no row and its scoped read-back returnsnull. The engine then raises "Record ID not found in OBJECT", andengineOpwraps that asStorageMetadataStoreError.Why the question lives in the store, not as a bare comparison at the door
A comparison written only at the door would also refuse on the engine-absent stand-in. The stand-in records the organization on insert but does not scope its writes, and it finishes this upload today (
200). Refusing there would narrow an accept set that is published (registerStorageRoutesandStorageMetadataStoreare both public), and that reads asClause-②: no (narrowing)against the ruledClause-②: no. A public getter onStorageMetadataStorewould widen the public face instead. The module-level function changes neither, and it keeps the stand-in's answers unchanged, which is H4.Tests
storage-routes.metadata-outage.test.ts: a new block,an organization change is not an outage, with 14 cases on that file's existing fake engine. The double was reused and no new double was pinned.409with the code and first sentence, rows unchanged, and the warn line naming the upload and both organizations.200).500 INTERNALwith "Restore the data engine", at both doors.200), an org-less row (200), an org-less session whose file is out of reach (409), and the question asked before the expiry write.403 PERMISSION_DENIED, which says nothing about organizations.200from either organization).metadata-store.test.ts: 6 cases onorganizationOutOfWriteReach.error-envelope.conformance.test.ts: one case for the new code, parsed against the declared envelope.packages/plugins/organizations/src/storage-upload-organization-change.wall.test.ts(new): the premise, both doors' negative pins with switch-back, both same-organization controls, and the progress door unchanged.Ablation (from committed
af2811c51c)The mutation went through
scripts/ablation-replace.mjsin wrap mode, run under the verify lock. It replaced the question withconst started = null(markerABLATION-22175).fd950d45ec2atoeba43b779bd2.src: the organizations package's vitest alias maps@objectstack/service-storagetosrc/index.ts, and the unit tests import relatively. So nodistleg applied.500 INTERNALwith the outage text, the original defect reproduced. The other 4 stayed green.200, or410on the expiry case.fd950d45ec2a,git diff HEADempty) and re-read afterwards (anchor 1, marker 0).Gates (at
af2811c51c)node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack, aftergit fetch origin main(origin/main13aea18959, merge base1e5d322c1). It derived 68 commands. That is the seat's 66 pluscheck:dispatcher-error-vocabularyandcheck-dev-prereqs.mjs --self-test.--ranwas run with every exit code recorded. It printed: "68 derived famil(ies) accounted for — 67 run, 1 NOT-MEASURED (1 DERIVED from a recorded exit 3)".check:i18n, first answered PREREQUISITE NOT MET (exit 3). After its stated build closure, it printed: "check-i18n-bundles: OK (9 package(s) — all bundles in sync, no undeclared authoring keys)".check:dual-build-cjs-loads. Its prerequisite is a whole-workspace build (33 packages have nodist/). In its place I ran a narrow probe on the only package whose source this diff touches: the rebuiltservice-storageloads throughrequire('./packages/services/service-storage/dist/index.cjs')(exit 0). The new function is absent fromdist/index.d.ts. CI runs the full gate.check:route-envelopepasses, andcheck:dispatcher-error-vocabularyanswered "OK — 54 unregistered code-stamping site(s), all classified".check:error-code-casing("no unlisted lowercase error codes in 7791 scanned file(s)"),check:error-status-conformance("every derivable runtime status is documented, and every documented status is reachable") and@objectstack/spec check:error-code-provenance(OK).check:authz-resolver,check:filter-alias-parity,check-changeset-fixed.mjs,check:tenant-chokepointandcheck:durability-log-level.af2811c51c.@objectstack/service-storage: 43 files, 704 tests pass.@objectstack/organizations: 11 files, 147 tests pass.typecheckexits 0 for both, withcheck:test-typecheck: OKfor each.eslint --no-inline-config --format jsonover the 6 touched TypeScript files: files=6, errors=0, warnings=0. Population: there was no "file ignored" warning, so all 6 were linted undereslint.config.mjs. Invariance: that config never enables type-aware linting (noparserOptions.project, no typed rules), so this diff cannot change the verdict on an untouched file. The fullpnpm lintis CI's.Acceptance notes
PUT …/upload/chunked/:uploadId/chunk/:chunkIndex), after the uploader switches organization, answers500 INTERNALwith thesys_upload_sessionoutage text;expires_at, answers the same500, because its expiry stamp is a scoped write. From the starting organization it answers200withexpired.Claim:names this branch.Generated by Claude Code