Skip to content

Commit 3d231ba

Browse files
fix(runtime)!: the dispatcher's DELETE /data/:object/:id honours ?expectedVersion and If-Match, as RestServer's twin does (#23460)
Fixes #23265 Clause-②: no (narrowing) The runtime dispatcher's `DELETE /data/:object/:id` now reads the optimistic-concurrency token the way `RestServer`'s twin reads it and forwards it to `deleteData`. A stale delete through the dispatcher is refused instead of applied. Dispatched by the `domain:cli` seat 2 PM (claim `6164842979`, branch `claude/issue-23265-dispatcher-delete-version`), session `session_01Q9PZ5Pzqcus4woVvTZwWNU`. ## What was wrong `RestServer` reads `?expectedVersion`, else `If-Match`, behind `refuseRepeatedQueryParams(req, res, ['expectedVersion'])`, and hands the token to `deleteData`. The protocol then refuses a delete whose record has moved (`409 CONCURRENT_UPDATE`). The dispatcher dropped the token twice: 1. `domains/data.ts`, the `DELETE` branch, read neither the query name nor the header. 2. `action-execution.ts`, `callData`'s delete arm, forwarded only `{ object, id }` to `deleteData`. So even a domain that read the token could not have passed it on. Through the `createHonoApp` catch-all, a stale delete deleted the row and answered `200`, the same answer as a delete with no token. ## What changes - **`domains/data.ts`, the `DELETE` branch.** - Reads `?expectedVersion`, else `If-Match`, in REST's order. - Refuses a repeated `?expectedVersion` first, before the service is asked, with `400 VALIDATION_ERROR` and `repeatedQueryParamMessage('expectedVersion', n)`. That is the idiom `i18n.ts`, `security.ts` and `packages.ts` already use. - Unwraps a one-element array. An empty array counts as no token. - Forwards a non-empty token as `String(token)`, which is REST's own test. - Reads the header through a local `requestIfMatch`, which covers both shapes a host hands `dispatch()`: the Fetch `Headers` that the catch-all passes as `c.req.raw`, and a Node header record. This mirrors `metaSaveRequestOptions`'s module-private `requestHeader`. - **`action-execution.ts`, `callData`'s delete arm.** - Forwards `expectedVersion` to `deleteData`, only when a caller sent one. - The ObjectQL fallback, used on a host whose protocol has no `deleteData`, cannot judge a version. A delete carrying a token is refused there with `501` before anything is read or deleted. This is the shape #22141 gave the `/meta` save's fallback writer. - **`.changeset/23265-dispatcher-delete-expected-version.md`.** `@objectstack/runtime` `minor`, `fix(runtime)!:`, with FROM → TO, `Clause-②: no (narrowing)`, one ADR-0087 marker (`not-required (no-migration-prescription)`) and the launch-window BREAKING line. No `@objectstack/rest` export is added, no `packages/spec` change, no other domain or branch. ## ⚠ File surface: one file beyond the claim The claim names the `DELETE` branch of `data.ts`. The fix also edits `packages/runtime/src/action-execution.ts` (`callData`'s delete arm, +25/-1). The card's acceptance is "forwards the version to `deleteData`", and `callData` is the only path from this branch to `deleteData`. Without this edit the acceptance cannot be met: ablation B below shows the stale pin red with `data.ts` fixed and only the `callData` forward removed. It was taken under the role file's bounded in-place-fix rule: 1. **Same defect.** It is the second drop of this card's token. 2. **Mechanical.** A conditional spread in REST's own shape, plus the #22141 `501` fallback refusal. 3. **No other holder.** None of the 15 open PRs' file lists names the file (read 2026-10-14 at about 09:20Z). 4. **Same package and gate family.** Every other `callData('delete', …)` caller (`domains/mcp.ts`, `endpoint-executor.ts`) passes `{ object, id }`, so its `deleteData` call is unchanged. ## Pins (`packages/runtime/src/domains/data-delete-expected-version.test.ts`) **The rig.** Each door gets a real store: better-sqlite3 `:memory:`, the real `ObjectQL` and a real `ObjectStackProtocolImplementation`. The same cases run against `RestServer`'s registered `DELETE` handler and against the real `HttpDispatcher.dispatch()`. The dispatcher is fed what the `createHonoApp` catch-all feeds it, including a real Fetch `Request`. After every request the test reads the store, not the door's answer. **Cases on each door:** - no token deletes, and `deleteData` gets no token; - the current `?expectedVersion` deletes; - a stale `?expectedVersion` is refused `409 CONCURRENT_UPDATE` and the row is kept; - `If-Match` behaves like `?expectedVersion`; - `?expectedVersion` is judged before `If-Match`; - a repeated `?expectedVersion` is refused `400 VALIDATION_ERROR` with the multiplicity sentence, before `deleteData`; - a one-element array is the one token it encodes. **Side by side:** - the stale refusal: same status and code at both doors; - the repeat refusal: same status, code and sentence; - a Node-style plain header record carries `If-Match` the way the Fetch `Headers` does. **Fallback:** - `?expectedVersion` or `If-Match` on a protocol with no `deleteData` gives `501 NOT_IMPLEMENTED`, and the engine is neither read nor written; - the control: an unguarded delete still reaches the fallback. How the conflict is read: the stale refusal is not returned by `dispatch()`. `/data` has no local catch, so it throws the protocol's `ConcurrentUpdateError`. The pin reads it through `resolveThrownHttpError` (`@objectstack/types`), the one rule the dispatcher's exits answer a throw with, and sets it beside what `RestServer` answered through `mapDataError`. **Before / after.** - **Before** (unfixed source at `823da2f21c`, with this test file): `Tests 11 failed | 9 passed (20)`. Every `RestServer` case was green. Every catch-all case except the control was red: the stale delete answered `200` and was applied, the repeat was ignored, and `deleteData` received no token. - **After** (`6059514619`): `Tests 20 passed (20)`. **Ablations.** Each ran through `node scripts/ablation-replace.mjs` in WRAP mode, with a restore trap. Each anchor hit once and `mutation landed` was printed. Each restore was proven by the blob hash equalling HEAD's and by an empty `git diff HEAD`. The expected direction was red each time, and each went red. - **A**, `data.ts` forward deleted: `9 failed | 11 passed`. These went red: matching, stale, `If-Match`, precedence and one-element on the catch-all; the side-by-side conflict; the header-record pin; and both `501` pins, since no token reaches `callData` without the forward. Every `RestServer` case, the control and both repeat refusals stayed green, because the repeat refusal sits ahead of the forward. - **B**, `callData` forward deleted: `7 failed | 13 passed`. These went red: matching, stale, `If-Match`, precedence and one-element on the catch-all; the side-by-side conflict; and the header-record pin. The `501` pins stayed green, because the fallback refusal is its own statement. - **C**, fallback refusal disabled (`if (false)`): `2 failed | 18 passed`. Both `501` pins went red. ## Mechanism readings (the dispatch's hypotheses) - **(a) Confirmed: no exported reading.** REST spells it inline. `readSingleQueryValue` exists in `query-multiplicity.ts` but is not on the `@objectstack/rest` entry, so `repeatedQueryParamMessage` is used and no export is added. - One correction: `d45d0e938a` used `repeatedQueryParamMessage` in `i18n.ts`, `security.ts` and `packages.ts`. In `data.ts` it used `assertFilterParamSuppliedOnce`, the filter-slot rule. - **(b) Confirmed.** The `/meta` save reads `_context.request?.headers` in both shapes. Its reader is module-private in `meta-save-request.ts`, so `data.ts` carries a ten-line mirror rather than a new export. - **(c) Measured.** - **Through `dispatch()`:** a stale delete throws the protocol's `ConcurrentUpdateError` (`status 409`, `code CONCURRENT_UPDATE`). `resolveThrownHttpError` reads `409 / CONCURRENT_UPDATE`, which is REST's status and code. - **Through the real `createHonoApp`** (scratch run: adapter source over the runtime `dist` built at `caeac16970`, not committed): a stale delete through `?expectedVersion` or through `If-Match` answers `500 INTERNAL_ERROR` with the conflict sentence, and the row is **kept**. The current version answers `200` and the row is gone. - **Why the 500:** the catch-all renders a thrown error from `statusCode` alone. That is outside this branch, and #23365 remains open for it. It is not edited here. - **(d) Pinned:** a repeated `?expectedVersion` handed to `dispatch()` as an array gives `400 VALIDATION_ERROR` and REST's sentence, identical to `RestServer`'s answer. ## Gates (final head `6f793007c5`) This head merges `origin/main` at `05ab9ef895`. The incoming commits touched `packages/spec` (the ui page and action schemas), one `metadata-protocol` test and one `lint` test, and nothing on this path. After the merge: - the runtime closure was rebuilt (`turbo run build --filter="@objectstack/runtime..."`, 31 tasks); - `pnpm --filter @objectstack/spec check:generated` reported all 14 artifacts up to date. - `pnpm --filter @objectstack/runtime run typecheck`: `TC_EXIT=0`. That covers `tsc --noEmit` and `check:test-typecheck`: 27 files, 190 errors, 68 pinned signatures held. The new test is compiled by `tsconfig.test.json`'s program (`--listFilesOnly`: 1 hit), and the main `tsconfig.json` excludes it (0 hits). - `vitest run --project local --maxWorkers=2`: `Test Files 374 passed (374)`, `Tests 5475 passed | 19 skipped (5494)`. - `vitest run --project repo --maxWorkers=2`: `Test Files 3 passed (3)`, `Tests 808 passed (808)`. - `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack`: 65 commands, each run with its exit code captured before any pipe. All 65 exited 0. `--ran`: `65 derived, 65 run, 0 NOT-MEASURED, 0 UNRUN`. - `pnpm lint` (this lane's addition): `LINT_EXIT=0`. Earlier head `caeac16970`: the union went red on `check:query-options-erasure`. The test surface grew 236 to 238, from two `as any` casts on the new test's store probes. `3813914f60` typed them and the ratchet held at 236. At the same head, `check:dual-build-cjs-loads` answered `PREREQUISITE NOT MET`; at `6f793007c5` it ran and passed. Since the merge, `origin/main` has moved again, at least 3 commits (to `2d2e3ce431`). One of them touches `metadata-protocol/src/protocol.ts` (an object-level sharing key retired), outside `deleteData`. CI's merge ref measures that joint state. ## Acceptance notes - **The dispatcher's `PATCH /data/:object/:id` has the same drop, and it is not fixed here.** The dispatch rules out other branches. Measured on the real `createHonoApp` in the same scratch run: - a `PATCH` with a stale `If-Match` answered `200` and **wrote** the row; - a body `expectedVersion` answered `500` with `Unknown field 'expectedVersion' on object 'task'`; - `RestServer` judges both as the OCC token, refusing a stale one `409`, and strips it from the patch. - This is reported to the seat as a finding; this PR does not file it. - **`RestServer`'s 409 body carries `currentVersion` / `currentRecord`.** The dispatcher's thrown-error exits (`errorFromThrown`, `dispatcher-plugin`'s `errorResponseBase`, both via `resolveThrownHttpError`) carry neither. On `createHonoApp` the question is moot until #23365 lands. Carrier: #23365. --- _Generated by [Claude Code](https://claude.ai/code/session_01Q9PZ5Pzqcus4woVvTZwWNU)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 9063727 commit 3d231ba

4 files changed

Lines changed: 443 additions & 3 deletions

File tree

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
---
2+
'@objectstack/runtime': minor
3+
---
4+
5+
fix(runtime)!: the runtime dispatcher's `DELETE /data/:object/:id` honours `?expectedVersion` and `If-Match` as its `RestServer` twin does, so a delete carrying a stale version is refused instead of applied
6+
7+
`RestServer`'s `DELETE /data/:object/:id` reads an optimistic-concurrency token from `?expectedVersion`, else from the `If-Match` header, refuses the query name supplied more than once, and hands the token to the protocol's `deleteData`, which refuses a delete whose record has moved past it. The runtime dispatcher's `/data` domain read neither, and its shared `callData` delete arm forwarded only the object and the id. So on a host whose `/data` routes the dispatcher serves (a `createHonoApp` host), a delete carrying a stale token deleted the record and answered `200`, the same answer a delete carrying no token gets. The domain now reads the token the way `RestServer` does and `callData` forwards it to `deleteData`.
8+
9+
Clause-②: no (narrowing)
10+
11+
<!-- adr-0087: not-required (no-migration-prescription) No metadata moves: no spec key, authorable spelling, object definition or stored shape is added, removed, renamed or re-shaped, and no exported declaration is added, removed or narrowed, so there is nothing for `objectstack migrate meta` to rewrite. What narrows is the accept set of one runtime dispatcher route: a delete whose version token is stale, repeated, or sent to a host that cannot judge it is now refused where it was applied, and the remedy is in the request a caller sends, never a rewrite of a package source or a stored row. The other categories are closed on facts: the package publishes (not unpublished); no ADR-0087 id is named or touched (not registered or already-registered); and nothing exported is removed or narrowed (not runtime-interface-only, type-surface-only or runtime-export-retired). -->
12+
13+
**BREAKING** (an accept-set narrowing), shipped as `minor` under the launch-window convention for breaking changes.
14+
15+
What changes for you, wherever the runtime dispatcher serves `DELETE /data/:object/:id`. Where `@objectstack/rest` is mounted it serves this route itself and already answered each of these, so the standalone server's answers do not move.
16+
17+
- FROM a delete with a stale `?expectedVersion` or `If-Match` that deleted the record and answered `200`, TO a refusal with the record kept: the protocol's `409 CONCURRENT_UPDATE`, thrown out of `dispatch()` for the host's exit to render, as every other refusal `deleteData` throws is. A `@objectstack/hono` `createHonoApp` host still renders that refusal as `500` (the record is kept): its catch-all reads only an error's `statusCode`, and rendering the declared `409` is a separate fix.
18+
- FROM a repeated `?expectedVersion` that was ignored (the record deleted, `200`), TO `400 VALIDATION_ERROR` in the dispatcher's envelope with the sentence `RestServer` answers ("The "expectedVersion" query parameter was supplied N times…"), before the protocol is asked. A one-element array is read as the one token it encodes. On a host whose adapter hands the dispatcher only the last value of a repeated query name (the `@objectstack/hono` `createHonoApp` catch-all, until that adapter keeps repeats), the last `?expectedVersion` is judged as the token.
19+
- FROM a delete carrying a token on a host whose protocol has no `deleteData` (the ObjectQL fallback), which deleted unguarded, TO `501 NOT_IMPLEMENTED`, before anything is read or deleted. A delete with no token reaches the fallback as before.
20+
21+
To delete over a version you read, send that version (the record's `updated_at`) once, as `?expectedVersion` or `If-Match`; `?expectedVersion` is judged when both are sent. A delete with no token is unchanged. The `@objectstack/client` SDK's `data.delete(object, id, { ifMatch })` sends `If-Match`, so an SDK caller that passes `ifMatch` now gets, through the dispatcher, the guard it asked for.

‎packages/runtime/src/action-execution.ts‎

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -395,8 +395,32 @@ export async function callData(deps: ActionExecutionDeps,
395395
}
396396

397397
if (action === 'delete') {
398+
// [#23265] `expectedVersion` rides through to `deleteData`, whose
399+
// version check refuses a stale delete. This arm forwarded only
400+
// `{ object, id }`, so a token the `/data` domain read was dropped here
401+
// a second time. Present only when a caller sent one: every other
402+
// caller hands `{ object, id }` and reaches `deleteData` as before.
403+
const expectedVersion = params?.expectedVersion;
398404
if (protocol && typeof protocol.deleteData === 'function') {
399-
return await protocol.deleteData({ object: params.object, id: params.id, ...(scopeId ? { environmentId: scopeId } : {}), context: executionContext });
405+
return await protocol.deleteData({
406+
object: params.object,
407+
id: params.id,
408+
...(expectedVersion !== undefined ? { expectedVersion } : {}),
409+
...(scopeId ? { environmentId: scopeId } : {}),
410+
context: executionContext,
411+
});
412+
}
413+
// [#23265] The fallback below deletes by id and nothing else, so it
414+
// cannot judge a version. A caller who sent one is refused `501`
415+
// before anything is read, never deleted unguarded: that is the very
416+
// drop the arm above just closed. The `/meta` save refuses a
417+
// precondition its fallback writer cannot honour the same way.
418+
if (expectedVersion !== undefined) {
419+
throw {
420+
statusCode: 501,
421+
message: 'This host has no protocol deleteData, so it cannot honour ?expectedVersion or If-Match on a '
422+
+ 'delete. Send the delete without them, or delete through a host whose protocol implements deleteData.',
423+
};
400424
}
401425
if (ql && typeof ql.delete === 'function') {
402426
// [#5138] There was NO existence check here: the delete ran and the

0 commit comments

Comments
 (0)