test(runtime): pin ActionEngineFacade.delete's partial-failure shape against a mid-array datastore rejection - #17925
Conversation
…against a datastore-side rejection `ActionEngineFacade.delete`'s member doc declares that a failure part-way through the array form leaves the ids before it deleted, the ids after it untouched, and delivers the rejection that stopped it. The existing `action-engine-facade-nullish-id.test.ts` reaches the sequential await loop, but every rejection it pins is the dispatch predicate's malformed-argument refusal. The causes the sentence exists for are the opposite kind: a well-formed by-id delete the datastore refuses. Adds a pin driving `buildActionEngineFacade` against an engine double whose `delete` accepts dispatch (it calls the producer's own `assertEngineDeleteDispatch`) and then refuses a well-formed mid-array id for a datastore reason. The double records every ATTEMPT as well as every deletion, so "the ids after it are untouched" is read off the calls the arm actually made. The rejection is pinned by object identity, plus the code/status envelope it carries. No behaviour change: tests and the regenerated pinned ledger only. Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TSf4DV7ziu4V5j73e46b7c
📓 Docs Drift CheckNothing in this diff resolved to a documentable surface (no symbol, route or SDK anchor derived from 0 changed package(s)), so this run has no opinion about the docs. What this run could not see
Coarse fallback — 0 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): |
Contract review — PR #17925 (card #17619)Head reviewed:
① ⛔ FIRST: the mechanism in triage's ruling AND in this seat's order is FALSE. The dev measured it; this seat confirms it at source.Triage's ruling
⇒ The existing pin already enters the sequential await loop and its rejection already comes from ② The residue is still real — for a NARROWER reason, and the narrower one is betterThe only rejection cause pinned anywhere was ③ ⛔ Acceptance item 2's LITERAL ablation is unachievable, and that is measured rather than arguedItem 2 names 「把循环包进事务(或
⭐ And a literal ④ The pin is STRONGER than the acceptance asked for — verified at source
⑤ Red lines, measured on the delivered head
⭐ ⑥ Changeset — none, via
|
Fixes #17619
Tests only. No behaviour change, no contract-text change, nothing touched in
ActionEngineFacade.deleteitself.What this adds
One new file,
packages/runtime/src/action-engine-facade-delete-partial-failure.test.ts, pinning the clause the card called "the only one whose violation is silent to the caller" — the declared partial-failure shape ofActionEngineFacade.delete's array form:Plus the mechanical ledger row the gate asked for (see Gates below).
The pin's exact assertions
Both cases drive
buildActionEngineFacadeagainst an engine double whosedeleteopens with the producer's ownassertEngineDeleteDispatch(options)— so dispatch ACCEPTS every id in the array — and then refuses a well-formed mid-array id for a datastore reason (DatastoreRefusal, carrying an ADR-0112code/statusenvelope). The double records every ATTEMPT as well as every deletion.Case 1 —
delete('crm_case', ['case_1', 'case_2', 'case_3']),case_2refused:expect(err).toBe(refusal)expect(err.code).toBe('PERMISSION_DENIED')·expect(err.status).toBe(403)expect(ids(ql.deleted)).toEqual(['case_1'])expect(ids(ql.attempted)).toEqual(['case_1', 'case_2'])Case 2 —
case_2andcase_3both refused:expect(err).toBe(first),expect(err).not.toBe(second), same two ledgers. That is "the rejection a caller sees is the one that stopped it" read as FIRST, not merely "some rejection".Assertion 2 is the one the sibling file structurally cannot make: its double records only successful deletions, so "untouched" can only be inferred from an absence. Here it is read off the calls the arm actually made.
Premise correction — read this before the acceptance
The dispatch order and triage ruling
5650883389both state that the sibling pin's rejection comes from "#17620's nullish pre-guard, which refuses the element beforeql.deleteis called at all". Measured atorigin/main5741ff10c, that is false, and the card is still right.buildActionEngineFacade's delete arm at head carries no pre-guard at all:#17620REMOVED theif (id != null)that used to open that loop — the file's own comment says so ataction-execution.ts:1473, andaction-engine-facade-nullish-id.test.ts's docblock says it again: "Every id now reachesql.delete(object, { where: { id }, context })as written, and the engine's own dispatch predicate answers that call". So the sibling pin ALREADY enters the sequential await loop and its rejection ALREADY comes fromql.delete.The residue survives for a narrower reason, and it is the reason that decides which ablation proves anything: the only rejection CAUSE pinned anywhere is
ENGINE_DELETE_REJECT_MESSAGE, a malformed-argument refusal raised before the driver is asked to do anything. An arm that caught rejections, re-threw the dispatch refusal and swallowed the rest would keep every sibling case green while continuing past a permission denial to delete rows the caller was told were untouched. That is the shape this file pins against, and it is the realistic refactor — discriminating on the error's type is a thing this repo's own rules elsewhere ask authors to do.Consequence for acceptance item 2: the ablation it names LITERALLY (a plain
try/catchthat continues) does NOT produce the stated contrast, because it reds the sibling file too. Measured, both ways, below. The kind-discriminating variant does produce it exactly.Ablation — two legs, both executed, mutation proved on disk, restore proved by state
Driver: a script with
trap restore EXIT INT TERM, absolute paths, restore spelledgit checkout HEAD -- PATH(never the bare form, which would restore the mutation out of the index). Both suites run in ONE vitest invocation per leg, so "new pin red" and "sibling green" are readings of the same on-disk state. The fix was committed FIRST (7ec6d1eb2), so every restore has a real commit to restore from.Subject resolution: the pin imports its subject as
./action-execution.js— a RELATIVE, same-package import, so vitest readssrc/, notdist/. Nodistpreflight applies; the mutation reaching the subject is proved directly by the verdict flipping.Leg A — error-KIND-discriminating swallow (rethrow the dispatch refusal, continue past everything else)
⭐ This is the card's proof: the new pin goes red, all six sibling cases stay green, in the same run, on the same bytes. The new coverage is additive, not a restatement.
Leg B — blanket
try/catch-continue (acceptance item 2's literal wording)Reported as measured rather than omitted: under the literal wording BOTH files red, so this leg cannot demonstrate that the new pin adds coverage. It is the evidence for the premise correction above, and the reason leg A is the ablation of record.
An actual
ql.transaction(...)wrapper is not expressible against these doubles at all — neither declares atransactionmember, so the arm would die of a TypeError in every case in both files, which is a broken harness and not a contrast.Verification
pnpm --filter @objectstack/runtime test(fulllocalproject)pnpm --filter @objectstack/runtime typecheckcheck:test-typecheck: OK· 27 files / 191 errors / 69 pinned signatures, ledger unmovedpnpm lint(eslint . --no-inline-config, WHOLE repo, no narrowing)turbo run buildover all packagesEvery heavy run went through
scripts/pm/os-verify-lock.sh(slotissue-17619-dev); the queue was empty throughout, so no wait was contended with the sibling dev on #12271.Gates
node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstackderived 59 commands from this diff. All 59 were run with$?captured BEFORE any pipe. Reconciliation with--ran:57 exited 0 on the first pass. Two exited 3 — PREREQUISITE NOT MET, which is neither a pass nor a finding:
pnpm check:dual-build-cjs-loads— "PREREQUISITE NOT MET — this gate reads built output, and some package has no dist/. … Runpnpm buildfirst. This is NOT a pass: nothing was measured."pnpm check:type-check-debt— "--re-measurecannot run: 2 workspace dependenc(ies) of the ledgered packages have no built type entry point on disk -- @objectstack/hono, @objectstack/runtime."Both name the same missing thing: a built workspace. After
pnpm exec turbo run build --concurrency=2 --filter='./packages/*' --filter='./packages/*/*'(72/72 successful), both were re-run and both exited 0 —check-type-check-coverage --re-measure: OK — 5 ledger entr(ies) re-measured in 83.0s, 55 raw tsc error(s) total, none above its recorded number.Both readings are reported here separately, as asked.pnpm check:engine-double-contractis the one gate the diff actively moved. Before the ledger row it said, verbatim:Done exactly that:
796 (file, verb) row(s), 1 added or grown, 0 lost— one row, no losses, no other file moved.Changeset
skip-changeset, on the gate's own route 2.Check Changeset's failure message (.github/workflows/pr-automation.yml) offers two routes and marks this one PREFERRED, verbatim:Measured rather than assumed, against
@objectstack/runtime's declaredfiles(dist,README.md,CHANGELOG.md) on a freshly built tree:packages/runtime/distDatastoreRefusal(new, test-only)buildActionEngineFacade(positive control)And
scripts/engine-double-contract.pinned.jsonappears in no package's shipped output at all. Nothing published moves, so there is no package to name.Acceptance notes
5650883389and this order both attribute the sibling pin's rejection to a facade-level nullish pre-guard that does not exist at head. The ruling's INTENT holds and is delivered; only its stated mechanism is wrong, and the correction is recorded above rather than as a card — the seat that wrote the ruling is the one carrier that will read it, and no file or PR other than this one is affected. Successor: none.try/catchthat continues) cannot produce the contrast it asks for, for the same reason. Measured in leg B. Same carrier, same reason not to file.action-engine-facade-nullish-id.test.ts,ActionEngineFacade.delete's behaviour and contract text,packages/spec/**, andupdate/find.Draft, and left that way: flipping to ready and arming auto-merge are the dispatching seat's acts, after the contract review of record.
Generated by Claude Code