Repository navigation
Description
Found while implementing #10740 (the update-op tenant classification on the delivery outboxes). Filed separately and unassigned: it is a different defect class from that card's tenant classification, it pre-dates it, and #10740's PR does not touch it.
The behaviour, measured
ObjectQL.update resolves { where: { id: <scalar>, ...more }, multi: false } to the by-id dispatch (resolveEngineUpdateDispatch → { kind: 'by-id', id }) and then calls driver.update(object, id, data, options). SqlDriver.update builds its statement as:
const builder = this.getBuilder(object, options).where('id', id);
this.applyTenantScope(builder, object, options);It never applies options.where. So every key in where other than id is silently discarded — including a compare-and-set guard the caller wrote specifically to make the write conditional.
Measured on better-sqlite3 through a real ObjectQL + SqlDriver, on sys_http_delivery:
// row p1 exists with status='pending', attempts=7
await engine.update(
SYS_HTTP_DELIVERY,
{ attempts: 0 },
{ where: { id: 'p1', status: { $in: ['success', 'failed', 'dead'] } }, multi: false },
);
// observed: attempts === 0The predicate demanded a terminal status. The row was pending. The write landed anyway.
Why it matters concretely
SqlHttpOutbox.redeliver (packages/services/service-messaging/src/sql-http-outbox.ts) writes exactly that predicate:
{ where: { id, status: { $in: ['success', 'failed', 'dead'] } }, multi: false }It is there as the atomic half of a check-then-act: redeliver reads the row, refuses a non-terminal one via assertRedeliverAllowed, and then re-states the terminal requirement in the write so a row that changed underneath it is not reset. That second check does nothing.
The window is real rather than theoretical: the HttpDispatcher tick claims pending rows into in_flight continuously and independently of any request. A row claimed between redeliver's read and its write is reset to status: 'pending', attempts: 0 regardless, and the post-write readback — which only asks whether the row is now pending — sees pending and reports the redelivery as a success. The in-flight attempt keeps running, so the delivery can go out twice while the row's attempt counter reads 0.
The generic form is the more important half: any caller in this repo who writes a conditional predicate alongside a scalar where.id gets a guard that silently evaluates to nothing, with no diagnostic. It reads exactly like a working compare-and-set.
What a fix has to decide
Not obvious, which is why this is a card and not an inline fix:
- Apply the remaining
wherekeys on the by-id path inSqlDriver.update(and itsdeletetwin, which has the same shape), so a stated predicate is honoured. This makes existing calls stricter — a write that currently lands might stop landing, which is the point, but it is a behaviour change on a shared write path. - Refuse the shape at the engine, the way
resolveEngineUpdateDispatchalready refuses other silently-wrong ones (#5748's operator-objectdata.idis the precedent): a by-id update carrying predicate keys it will not honour is a caller error, and refusing is thedeclared = enforcedanswer. - Leave the mechanism and fix the caller, having
redeliverdo its compare-and-set some other way.
(1) and (2) are contract changes on ObjectQL.update; (3) leaves the trap armed for the next caller. My read is that (2) is the shape this repo usually chooses — a silently-ignored option is the failure mode ENGINE_UPDATE_OPTION_KEYS and the dispatch predicate already exist to prevent — but the blast radius wants measuring first: resolveEngineUpdateDispatch's case table and every by-id caller passing extra where keys.
Refs
packages/metadata-core/src/engine-update-dispatch.ts—resolveEngineUpdateDispatch/scalarUpdateIdpackages/drivers/driver-sql/src/sql-driver.ts—SqlDriver.update(the builder that ignoresoptions.where)packages/services/service-messaging/src/sql-http-outbox.ts—redeliver, the measured caller- ObjectQL.update 的
data.id不做标量测试 —— 载荷里的算子对象被当成主键绑定,且盖过显式options.multi: true#5748,update的 **by-id** 路径同样把非标量data.id交给驱动写主键列(#6262 的孪生形状,where.id胜出时) #6435 — prior corrections to the same dispatch predicate, both about a shape it read wrongly rather than one it dropped - tenant-audit, the
updatehalf:ackis a dispatcher sweep butredeliveris request-reachable — two sites on one object with OPPOSITE classifications #10740 — where this was found; that card's scope is the tenant classification only
Duplicate search run before filing: nothing open. The nearest matches are #4419 (findOne dropping unsupported predicate keys — the READ side of the same family, closed) and #5748 / #6435, both closed.
Activity
huangyiirene commentedon Aug 22, 2026
Triage: lands in
packages/metadata-core(dispatch predicate) +packages/drivers/driver-sql+ one caller inservice-messaging⇒domain:engine, typeBug, queued (pm:queue).Not routed to the decision inbox, per the standing meta-criterion: a silently-dropped caller declaration defaults into the existing loud-refusal family — this repo has already ruled that shape twice on this exact dispatch predicate (#5748's operator-object
data.id, #6435), andENGINE_UPDATE_OPTION_KEYSexists precisely to make ignored options loud. The inherited reason transfers cleanly: a predicate the engine will not honour must be refused at authoring time, not evaluated to nothing. So the default route is the card's own option 2 (+3):by-iddispatch carrying predicate keys beyondidbecomes a loud error (same style as the ObjectQL.update 的data.id不做标量测试 —— 载荷里的算子对象被当成主键绑定,且盖过显式options.multi: true#5748 refusal), inupdateand thedeletetwin.redeliveroff the trap: its compare-and-set requirement is real and must survive — re-express the terminal-status guard on a dispatch path that honours the full predicate (mechanism assumption to verify, not a ruling: the predicate/multi path reaches the driver with the wholewhere). Acceptance evidence: a concurrent-claim test — row flips toin_flightbetween read and write ⇒ the reset does NOT land and redeliver reports refusal, not success.update/deletecaller passing extrawherekeys. If any caller's guard cannot be expressed on the predicate path, stop and report the fork — do not widen the by-id path to honour predicates (that is a capability addition needing its own ruling) and do not quietly leave that caller broken.Sizing note for the engine seat: M (shared write path + caller migration + refusal pins). Hot-surface note:
sql-driver.tsalso carries queued #10997 and blocked #10998 — fold-or-serial is the seat's five-gate call at claim time.Generated by Claude Code
os-zhuang commentedon Aug 22, 2026
Claim: PM loop round 3⚠️ #10729 is deliberately NOT dispatched alongside this card: it lands in
Session:
session_01RfyXxZ2WPjcjhuXpiQQc3yBranch:
claude/issue-11009-refuse-unhonoured-by-id-predicateWorktree:
objectstack-issue-11009Domain:
domain:engineFile surface:
packages/metadata-core/src/engine-update-dispatch.ts(resolveEngineUpdateDispatch/assertEngineUpdateDispatch,:225) ·packages/objectql/src/engine.ts(ENGINE_UPDATE_OPTION_KEYS:382,rejectUnknownEngineOptions:9256— see the correction below) ·packages/drivers/driver-sql/src/sql-driver.ts(updateat:5815and itsdeletetwin) ·packages/services/service-messaging/src/sql-http-outbox.ts(redeliver) + refusal pins and a concurrent-claim test (stop on breach; explain in the report)Container & model:
M,mode:subagent,model: claude-fable-5Clause-②: yes — see below. This is the declaration limb, and it is the reason for the tier.
Serial constraints cleared:
driver-sqlwas held by PR #11124 (#10676+#10998), now MERGED (95437e7d2) — that is what releases this card. Same-file neighbours onsql-driver.tsNOT dispatched: #11101, #11067 — five-gate ① fails, serial. In-flight sibling #11021 ismetadata-protocol, package-disjoint.packages/objectql/src/engine.ts:4503, and the surface correction below puts this card in that same file. Lane in-flight at claim: 1.⛔ Clause ② — declared
yes, and the tier follows from itThis card's adjudicated route is "a by-id dispatch carrying predicate keys beyond
idbecomes a loud error". That changes contract accept/reject behaviour: calls that succeed today will throw. It narrows rather than widens, and it is adeclared = enforcedrestoration — which is why the adjudication correctly stayed out of the decision inbox — but the tier mandate is written on content, not on who adjudicated:dispatch-gates --tiersays it in as many words, "Clause ② is NOT reachable from paths … judged from the card CONTENT. This line is a FLOOR, never a clearance."Dispatched at
CONTRACT_REVIEW_TIER, read from its single source (scripts/pm/dispatch-gates.mjs:2722) rather than recalled, so the enqueue gate is satisfied at dispatch time rather than by attachingneeds:contract-reviewafterwards.⛔ Surface correction to triage's landing list — verified, not assumed
Triage routed this to
metadata-core+driver-sql+service-messagingand citedENGINE_UPDATE_OPTION_KEYSas the loud-refusal precedent. That constant is not inmetadata-core. Measured onorigin/main@95437e7d2:ENGINE_UPDATE_OPTION_KEYSpackages/objectql/src/engine.ts:382, consumed byrejectUnknownEngineOptionsat:9256resolveEngineUpdateDispatch/assertEngineUpdateDispatchpackages/metadata-core/src/engine-update-dispatch.ts(:225)So the refusal machinery is split across two packages, and a fix written in the style of the cited precedent reaches
objectql— which triage's list did not name. Recorded because it changes both the file surface and what this card can be dispatched alongside.(I checked this only because a grep for
ENGINE_UPDATE_OPTION_KEYSinmetadata-corereturned zero, and a zero-hit is not a reading until a known-present neighbour confirms the query is live.resolveEngineUpdateDispatchwas the control.)Premise re-verified on merged
origin/main@95437e7d2SqlDriver.update(object, id, data, options)at:5815. The card's central claim — that it builds.where('id', id)+ tenant scope and never appliesoptions.where— is the dev's to re-derive at that line.Carried into the dispatch order
Triage's route (option 2 + 3) with its blast-radius-first clause and its stop-condition: if any by-id caller's guard cannot be expressed on the predicate path, ⛔ do not widen the by-id path to honour predicates (a capability addition needing its own ruling) and ⛔ do not quietly leave that caller broken — stop and report the fork.
Generated by Claude Code
os-zhuang commentedon Aug 22, 2026
Claim surface amendment (same session
session_01RfyXxZ2WPjcjhuXpiQQc3y, branchclaude/issue-11009-refuse-unhonoured-by-id-predicate) — files added to the claimed surface beyond the dispatch order's list:packages/metadata-core/src/engine-dispatch-unhonoured-predicate.ts— NEW: the refusal composer both write dispatches share.packages/services/service-messaging/src/memory-http-outbox.ts— bounded inline fix under the four-condition exemption: same defect class (redeliver's terminal-status guard inert against a mid-guard claim), mechanical form pinned by the SQL store's refusal (DELIVERY_NOT_ELIGIBLE, "state changed during redeliver"), file held by no other claim, same gate family. Without it the twoIHttpOutbox.redeliverimplementations would disagree on the exact race A compare-and-setwhereon a by-idupdateis silently inert — the extra predicate keys never reach the driver, andSqlHttpOutbox.redeliver's status guard is one of them #11009's acceptance test pins.packages/services/service-messaging/src/outbox-dispatcher-scope.ts— docstring only:multino longer discriminatesredeliverfrom the sweep helper, so the prose that leaned on it is re-anchored on the tenant classification.packages/services/service-messaging/src/delivery-update-tenant-audit.integration.test.ts— tenant-audit, theupdatehalf:ackis a dispatcher sweep butredeliveris request-reachable — two sites on one object with OPPOSITE classifications #10740's redeliver legs migrate to theupdateManyspy/audit op (redeliver now rides the predicate path).packages/services/service-messaging/src/redeliver-concurrent-claim.integration.test.ts— NEW: the concurrent-claim acceptance evidence.packages/services/service-queue/src/db-queue-adapter.test.ts— its case-driven fake-conformance matcher hardcoded the one classic reject message; it now quotes the predicate's own per-case message.packages/objectql/src/engine-delete-dispatch.test.ts/engine-update-dispatch.test.ts/index.ts,packages/metadata-core/src/index.ts— pins and export wiring for the above.Not touched:
packages/drivers/driver-sql/src/sql-driver.ts— the adjudicated route refuses at the engine and moves the caller; the driver needed no edit, so the serial neighbours #11101/#11067 see no overlap from this card.Generated by Claude Code
Generated by Claude Code
fix(objectql,metadata-core,service-messaging): refuse a by-id write w…
os-zhuang commentedon Aug 22, 2026
{ "issue": 11009, "status": "done", "branch": "claude/issue-11009-refuse-unhonoured-by-id-predicate", "pr": "https://github.com/objectstack-ai/objectstack/pull/11147", "premise_still_valid": true, "summary": "Reproduced both measured claims on origin/main @ 95437e7d2 (better-sqlite3, real ObjectQL + SqlDriver): the by-id CAS shape landed (attempts 7 to 0 on a pending row) and redeliver over a mid-window in_flight claim reset the row and reported success. Implemented triage option 2+3 in the SHARED dispatch predicates (metadata-core) so every pinned double inherits it: a by-id where carrying keys beyond id is refused loudly (one source-blind message composer for both verbs); the same shape WITH multi:true and a where-sourced id routes to the predicate path, which honours the full where; a pure-id where stays by-id even under multi:true (LifecycleService reap idiom, now pinned in both case tables). ObjectQL.update/delete key their predicate branch on the ladder's verdict and throw the dispatch's own message. redeliver rides the predicate path; MemoryHttpOutbox re-checks terminal status after its guard (bounded inline fix, declared in the claim amendment) so both IHttpOutbox impls refuse the race identically. Zone-2 corrections confirmed: ENGINE_UPDATE_OPTION_KEYS is in objectql engine.ts:382 (not metadata-core); SqlDriver.update at :5815 never reads options.where.", "blast_radius_census": { "method": "scripted balanced-paren scan of every .update(/.delete( call carrying a where: bag over 4863 files, plus a data.id-sourced second sweep and a 'multi: false' grep; hits hand-classified", "by_id_where_extra_no_multi_production": ["SqlHttpOutbox.redeliver (the measured victim; migrated to the predicate path in this PR) — no other production caller"], "by_id_where_extra_with_multi": "zero callers repo-wide; the rerouted cell ships unoccupied", "data_id_plus_where_bag": "zero production callers", "pure_id_where_plus_multi_unchanged": ["objectql/src/lifecycle/lifecycle-service.ts:1565 (guarded reap — relies on by-id cascade; verdict deliberately preserved)", "plugin-sharing bu-tree-recompute tests", "objectql engine-data-events.test.ts (pins the per-record event for this exact shape)"], "every_censused_guard_expressible_on_predicate_path": true, "stop_condition_reached": false, "by_id_driver_path_widened": false }, "concurrent_claim_test": { "file": "packages/services/service-messaging/src/redeliver-concurrent-claim.integration.test.ts", "before_fix_measured": "reset landed over the in_flight claim (row pending, attempts 0, claim cleared) and redeliver resolved success — asserted as-is on the unfixed tree and passing", "after_fix": "reset does NOT land (row stays in_flight, attempts 4, claimed_by racer) and redeliver rejects HttpRedeliverError DELIVERY_NOT_ELIGIBLE; still-works leg pins the unraced reset; MemoryHttpOutbox leg pins the same contract" }, "delete_twin_symmetric": true, "delete_twin_evidence": "same shared composer (engineByIdUnhonouredPredicateMessage) imported by both dispatch modules; ENGINE_DELETE_DISPATCH_CASES gained the same reject/multi/pure-id-multi cases; delete-side pins in engine-delete-dispatch.test.ts drive the REAL engine (deleteMany observed with the full where)", "tests": "All at head 2e3d05049 via os-verify-lock, VERDICT command-exit 0 each: suites metadata-core 165/165, objectql 4046/4046 (228 files), driver-sql 1795 passed, metadata-protocol 1853 passed (13 pinned fakes), service-messaging 259/259 re-run at head, service-automation 998/998, plugin-sharing 624/624, plugin-webhooks 128/128, service-queue 56/56; typecheck green on metadata-core, objectql, service-messaging, service-queue (script names echoed). Gates all green at their own verdict lines: changeset-gate-self-tests, durability-log-level, objectui-changeset, slot-lookup, stack-collection-maps, test-source-alias, type-source-resolution, adr-0087-registration, changeset-no-major, ci-filter-parity, empty-changeset, engine-split-ratio, plugin-teardown-shape, docs-audit affected-docs, query-options-erasure, engine-double-contract, where-matcher, nul-bytes, driver-conformance, i18n ('OK, 9 packages, all bundles in sync' after building the CLI it requires), type-check-coverage ('OK — 65/78'), type-check-debt --re-measure ('OK — 33 ledger entries, none above recorded') after the full workspace closure build. Gate list re-derived with dispatch-gates.mjs (no paths; 16 files vs merge base 95437e7d2); it added changeset-gate-self-tests, objectui-changeset, stack-collection-maps, engine-split-ratio, durability-log-level, query-options-erasure, where-matcher, type-check family and adr-0087/changeset families beyond the prompt's list, all run.", "open_questions": [], "out_of_scope_findings": [ "filed as #11142: a scalar data.id beside a DIFFERENT scalar where.id still silently drops the where.id predicate — refusing it reverses the pinned #5748 expectId verdict, a maintainer decision" ], "deviations": [ "prompt-derived gate list vs reality: check:driver-conformance was prompt-named but sql-driver.ts needed no edit (refusal lands at the engine per the adjudicated route); gate run anyway, green", "bounded inline fix under the four-condition exemption: MemoryHttpOutbox.redeliver terminal re-check — declared same-round in the claim-surface amendment comment and named in the PR body", "service-queue/db-queue-adapter.test.ts per-case reject matcher updated to quote the predicate's own message (it hardcoded the one classic constant; the fake itself was already correct through its pin)", "#10740 tenant-audit pins migrated to the updateMany op (redeliver's op moved with the dispatch path); tenant-classification contract unchanged and re-asserted with a new positive control for that op" ] }Generated by Claude Code
os-zhuang commentedon Aug 22, 2026
ACCEPT — PR #11147, 31/31 checks green, enqueued
Verified independently rather than read off the report:
mergeable_state: clean, every check run read by name (31 total;Console Pin GateandBuild Docsskipped, not red — this diff touches neither path),Lint & Repo Gatesgreen includingcheck:engine-double-contract. Flipped to ready, auto-merge enabled.The triage order's three clauses were all discharged: blast-radius measured first,
redelivermoved off the trap, and the stop-condition — "if any caller's guard cannot be expressed on the predicate path, stop and report the fork" — was never reached, because every censused caller's guard is expressible there. The by-id driver path was not widened. That was the one outcome this dispatch most needed to avoid, and it was avoided for the right reason rather than by luck.Triage's route said a by-id dispatch carrying predicate keys beyond
idbecomes a loud error. The implementation makes that true for three of four cells, but routes the fourth —where: { id, …extra }with a declaredmulti: true— silently to the predicate path instead of refusing it. That is a behaviour change shipped without a diagnostic: a caller who wrote that shape previously got an unconditional single-row write and now gets a conditional multi-row one.Accepted, for three reasons in descending weight:
multi: trueis the caller explicitly declaring predicate semantics; compiling their wholewhereis what they asked for. Refusing it would refuse a correctly-spelled compare-and-set.multi: truehit sharing a line withidacross non-testpackages/**is a comment, a dispatch case-table row, or an$inid set; none is a scalar-id-plus-extra-keys call).⛔ Had the census found even one occupant, this cell would have gone back for a refusal or a ruling. It is the emptiness that makes silence acceptable here, not the reasoning alone.
⛔ My scoping error, not the dev's
I declared the file surface as four packages including
packages/drivers/driver-sql/src/sql-driver.ts. The landed PR touches 16 files andsql-driver.tsis not among them — the fix lives in the shared dispatch predicates, exactly where it should, and the driver never needed changing.Consequence: I serialized #11101 and #11067 against a file that was never at risk for the whole life of this dispatch. Over-declaring a surface is not the safe direction of error I had been treating it as — it costs throughput just as under-declaring costs safety, and it does so invisibly. Correction to this seat's practice: a declared surface is a prediction, and when a dev's actual surface lands narrower, the seat releases the serial hold instead of leaving it standing to the merge.
Residual, correctly escalated rather than decided
#11142 — a scalar
data.idbeside a different scalarwhere.idstill silently drops thewhere.id. The dev declined to fix it because doing so reverses a pinned #5748 verdict (theexpectIdcase), and that distinction is right: refusingdata.id+ non-id keys does not touch #5748's precedence rule, while refusingdata.id+where.idwould overturn it. Labelledfindingand left for triage — ⛔ note for the grading round that this is a decision-inbox shape, not an ordinary observation: it is the last surviving cell of the exact silent-predicate-loss class this PR just closed, so grading it as "no user impact today" would be the wrong read.Excellent work — particularly measuring both defect behaviours as passing assertions on the unfixed tree before any edit.
Generated by Claude Code
fix(objectql,metadata-core): refuse a by-id update whose scalar where…
fix(objectql,metadata-core,service-messaging): refuse a by-id write w…
fix(objectql,metadata-core): refuse a by-id update whose scalar where…
fix(objectql,metadata-core): refuse a by-id update whose non-scalar w…
docs(releases): name 17.2.0 as the current v17 series
fix(service-messaging): enforce ack()'s claimed-row precondition in b…
docs(releases): name 17.2.0 as the current v17 series (objectstack-ai…
fix(cli)!: report the named export the config default export already …
fix(spec)!: refuse an array in the equality slot at the shared compar…
test(metadata-core,driver-mongodb): shipped case labels and mongod ha…