Repository navigation
Commit 7ebbc01
fix(service-storage): answer an upload's organization change with 409 RESOURCE_CONFLICT, not an outage 500 (#22216)
Fixes #22175
Clause-②: no
The commit door and the chunked-completion door of
`@objectstack/service-storage` now answer an uploader whose active
organization changed after starting an upload with `409
RESOURCE_CONFLICT`, in the ADR-0112 envelope, with a message that names
the organization change. They used to answer `500 INTERNAL` with 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)
- "answer with a worded 4xx that names the organization change, in the
ADR-0112 envelope"
- "the operator log names the cause"
- "⛔ No change to which organization an upload belongs to: #22046's
ruling keeps the tenant scope as it is."
- "`Clause-②: no`. Patch changeset for the storage service."
- From the card: "⛔ No catch-all that hides a real engine failure."
## 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 under
`context`: can that write reach `row`? If it cannot, the function
returns the organization the row was started in. Otherwise it returns
`null`.
- It returns `null` for the engine-absent stand-in, which does not scope
its writes. A module-private `WeakSet` filled in the constructor records
which stores are engine-backed.
- It returns `null` when no organization is threaded. It reuses
`writeOptionsFor`, the same rule that decides whether the write carries
a `tenantId`.
- It returns `null` for a row stamped with no organization, because the
driver keeps org-less rows in reach.
- Otherwise, a row is in reach exactly when its organization is the
acting one.
- It composes no predicate. The statement's tenant term stays the
driver's.
- **`storage-routes.ts`**: a new gate,
`requireStartingOrganization(writeContext, rows, …)`. It runs right
after the ownership rule from #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 one `warn`
line naming the door, the upload id, the starting organization and the
active one. It then answers `409` / `RESOURCE_CONFLICT` with a constant
message that names no organization.
- **Commit door** (`POST …/upload/complete`): it asks about the
`sys_file` row. The write context is now bound once and passed both to
the question and to `updateFile`, so the two cannot differ.
- **Chunked-completion door** (`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**: `.changeset/22175-upload-org-change-4xx.md`, a `patch`
for `@objectstack/service-storage`.
**Code choice.** `RESOURCE_CONFLICT` is the standard-catalog member that
HTTP 409 derives, so no ledger entry and no `packages/spec` change 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_DENIED` would 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`: `bootStack` with
`OrganizationsPlugin` and `StorageServicePlugin`, posture `isolated`,
and the uploader switched through the product's own `set-active` door.
| door | before (`1e5d322c`) | after (`af2811c51c`) |
|---|---|---|
| commit, organization changed | `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`) |
| chunked completion, organization changed | `500 INTERNAL`, the same
text for `sys_upload_session` | `409 RESOURCE_CONFLICT`; session and
file rows unchanged; switching back completes it (`200`) |
| commit / chunked completion, same organization | `200` | `200`
(control) |
| progress, organization changed | `200` | `200` (unchanged) |
H1, measured: the scoped miss is a **throw**, not zero rows. The SQL
driver's scoped `UPDATE … WHERE id` touches no row and its scoped
read-back returns `null`. The engine then raises "Record ID not found in
OBJECT", and `engineOp` wraps that as `StorageMetadataStoreError`.
## 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
(`registerStorageRoutes` and `StorageMetadataStore` are both public),
and that reads as `Clause-②: no (narrowing)` against the ruled
`Clause-②: no`. A public getter on `StorageMetadataStore` would 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.
- At both doors: `409` with the code and first sentence, rows unchanged,
and the warn line naming the upload and both organizations.
- Same-organization control (`200`).
- Same-organization **real engine failure** still answers `500 INTERNAL`
with "Restore the data engine", at both doors.
- No active organization (`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.
- The ownership rule runs first: a non-uploader in another organization
still gets `403 PERMISSION_DENIED`, which says nothing about
organizations.
- The progress door is unchanged, and the stand-in is unchanged (`200`
from either organization).
- `metadata-store.test.ts`: 6 cases on `organizationOutOfWriteReach`.
- `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.mjs` in wrap mode,
run under the verify lock. It replaced the question with `const started
= null` (marker `ABLATION-22175`).
- On-disk evidence: anchor count went 1 to 0, marker 0 to 1, blob
`fd950d45ec2a` to `eba43b779bd2`.
- Both suites resolve the subject from `src`: the organizations
package's vitest alias maps `@objectstack/service-storage` to
`src/index.ts`, and the unit tests import relatively. So no `dist` leg
applied.
- Wall: the 2 negative pins turned red. Each received `500 INTERNAL`
with the outage text, the original defect reproduced. The other 4 stayed
green.
- Unit: 5 cases turned red, the conformance case among them. The fake
engine does not scope, so the doors answered `200`, or `410` on the
expiry case.
- Restore: proven by the tool (blob equals HEAD `fd950d45ec2a`, `git
diff HEAD` empty) and re-read afterwards (anchor 1, marker 0).
## Gates (at `af2811c51c`)
- **Derivation.** Run from this worktree with `node
scripts/pm/dispatch-gates.mjs --commands --repo
objectstack-ai/objectstack`, after `git fetch origin main` (origin/main
`13aea18959`, merge base `1e5d322c1`). It derived 68 commands. That is
the seat's 66 plus `check:dispatcher-error-vocabulary` and
`check-dev-prereqs.mjs --self-test`.
- **Reconciliation.** `--ran` was 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)".
- **Exit 0: 67 commands.** One of them, `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)".
- **NOT MEASURED: `check:dual-build-cjs-loads`.** Its prerequisite is a
whole-workspace build (33 packages have no `dist/`). In its place I ran
a narrow probe on the only package whose source this diff touches: the
rebuilt `service-storage` loads through
`require('./packages/services/service-storage/dist/index.cjs')` (exit
0). The new function is absent from `dist/index.d.ts`. CI runs the full
gate.
- **Error-envelope and error-code families.**
- From the derivation: `check:route-envelope` passes, and
`check:dispatcher-error-vocabulary` answered "OK — 54 unregistered
code-stamping site(s), all classified".
- From the roster families the derivation lists but cannot place:
`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).
- Also run beyond the derivation, all exit 0: `check:authz-resolver`,
`check:filter-alias-parity`, `check-changeset-fixed.mjs`,
`check:tenant-chokepoint` and `check:durability-log-level`.
- **Package suites at `af2811c51c`.** `@objectstack/service-storage`: 43
files, 704 tests pass. `@objectstack/organizations`: 11 files, 147 tests
pass. `typecheck` exits 0 for both, with `check:test-typecheck: OK` for
each.
- **Lint, narrowed.** I ran `eslint --no-inline-config --format json`
over the 6 touched TypeScript files: files=6, errors=0, warnings=0.
Population: there was no "file ignored" warning, so all 6 were linted
under `eslint.config.mjs`. Invariance: that config never enables
type-aware linting (no `parserOptions.project`, no typed rules), so this
diff cannot change the verdict on an untouched file. The full `pnpm
lint` is CI's.
## Acceptance notes
- **Same family, outside this card's two doors.** These were measured on
the same wall with a throwaway probe that was deleted after the run:
- the chunk door (`PUT …/upload/chunked/:uploadId/chunk/:chunkIndex`),
after the uploader switches organization, answers `500 INTERNAL` with
the `sys_upload_session` outage text;
- the progress door, for a session past its `expires_at`, answers the
same `500`, because its expiry stamp is a scoped write. From the
starting organization it answers `200` with `expired`.
- The store question this PR adds would answer both of these scoped
writes. They are reported to the seat as one finding and are not fixed
here, because the dispatch fenced the two doors.
- The dispatch said the card had 3 comments. The REST read shows 2, the
triage and the claim. The newest `Claim:` names this branch.
---
_Generated by [Claude
Code](https://claude.ai/code/session_01WkL6Eijt432S1Y7ekb6ovQ)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent c52bfb4 commit 7ebbc01
7 files changed
Lines changed: 738 additions & 3 deletions
File tree
- .changeset
- packages
- plugins/organizations/src
- services/service-storage/src
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
Lines changed: 254 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
| 144 | + | |
| 145 | + | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
| 149 | + | |
| 150 | + | |
| 151 | + | |
| 152 | + | |
| 153 | + | |
| 154 | + | |
| 155 | + | |
| 156 | + | |
| 157 | + | |
| 158 | + | |
| 159 | + | |
| 160 | + | |
| 161 | + | |
| 162 | + | |
| 163 | + | |
| 164 | + | |
| 165 | + | |
| 166 | + | |
| 167 | + | |
| 168 | + | |
| 169 | + | |
| 170 | + | |
| 171 | + | |
| 172 | + | |
| 173 | + | |
| 174 | + | |
| 175 | + | |
| 176 | + | |
| 177 | + | |
| 178 | + | |
| 179 | + | |
| 180 | + | |
| 181 | + | |
| 182 | + | |
| 183 | + | |
| 184 | + | |
| 185 | + | |
| 186 | + | |
| 187 | + | |
| 188 | + | |
| 189 | + | |
| 190 | + | |
| 191 | + | |
| 192 | + | |
| 193 | + | |
| 194 | + | |
| 195 | + | |
| 196 | + | |
| 197 | + | |
| 198 | + | |
| 199 | + | |
| 200 | + | |
| 201 | + | |
| 202 | + | |
| 203 | + | |
| 204 | + | |
| 205 | + | |
| 206 | + | |
| 207 | + | |
| 208 | + | |
| 209 | + | |
| 210 | + | |
| 211 | + | |
| 212 | + | |
| 213 | + | |
| 214 | + | |
| 215 | + | |
| 216 | + | |
| 217 | + | |
| 218 | + | |
| 219 | + | |
| 220 | + | |
| 221 | + | |
| 222 | + | |
| 223 | + | |
| 224 | + | |
| 225 | + | |
| 226 | + | |
| 227 | + | |
| 228 | + | |
| 229 | + | |
| 230 | + | |
| 231 | + | |
| 232 | + | |
| 233 | + | |
| 234 | + | |
| 235 | + | |
| 236 | + | |
| 237 | + | |
| 238 | + | |
| 239 | + | |
| 240 | + | |
| 241 | + | |
| 242 | + | |
| 243 | + | |
| 244 | + | |
| 245 | + | |
| 246 | + | |
| 247 | + | |
| 248 | + | |
| 249 | + | |
| 250 | + | |
| 251 | + | |
| 252 | + | |
| 253 | + | |
| 254 | + | |
Lines changed: 25 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
222 | 222 | | |
223 | 223 | | |
224 | 224 | | |
| 225 | + | |
| 226 | + | |
| 227 | + | |
| 228 | + | |
| 229 | + | |
| 230 | + | |
| 231 | + | |
| 232 | + | |
| 233 | + | |
| 234 | + | |
| 235 | + | |
| 236 | + | |
| 237 | + | |
| 238 | + | |
| 239 | + | |
| 240 | + | |
| 241 | + | |
| 242 | + | |
| 243 | + | |
| 244 | + | |
| 245 | + | |
| 246 | + | |
| 247 | + | |
| 248 | + | |
| 249 | + | |
225 | 250 | | |
226 | 251 | | |
227 | 252 | | |
| |||
Lines changed: 45 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
22 | 22 | | |
23 | 23 | | |
24 | 24 | | |
| 25 | + | |
25 | 26 | | |
26 | 27 | | |
27 | 28 | | |
| |||
359 | 360 | | |
360 | 361 | | |
361 | 362 | | |
| 363 | + | |
| 364 | + | |
| 365 | + | |
| 366 | + | |
| 367 | + | |
| 368 | + | |
| 369 | + | |
| 370 | + | |
| 371 | + | |
| 372 | + | |
| 373 | + | |
| 374 | + | |
| 375 | + | |
| 376 | + | |
| 377 | + | |
| 378 | + | |
| 379 | + | |
| 380 | + | |
| 381 | + | |
| 382 | + | |
| 383 | + | |
| 384 | + | |
| 385 | + | |
| 386 | + | |
| 387 | + | |
| 388 | + | |
| 389 | + | |
| 390 | + | |
| 391 | + | |
| 392 | + | |
| 393 | + | |
| 394 | + | |
| 395 | + | |
| 396 | + | |
| 397 | + | |
| 398 | + | |
| 399 | + | |
| 400 | + | |
| 401 | + | |
| 402 | + | |
| 403 | + | |
| 404 | + | |
| 405 | + | |
| 406 | + | |
0 commit comments