fix(service-messaging): an HTTP ack binds the claim credential, so a reaped claim's late ack cannot overwrite the live re-claim - #17641
Conversation
…e re-claim Claude-Session: https://claude.ai/code/session_c5c0ce54-bb9c-478c-9e5b-cf44b80d4569 Co-authored-by: Claude <noreply@anthropic.com>
…reaped claim's late ack cannot overwrite the live re-claim Claude-Session: https://claude.ai/code/session_c5c0ce54-bb9c-478c-9e5b-cf44b80d4569 Co-authored-by: Claude <noreply@anthropic.com>
…ck write (223 sites) Claude-Session: https://claude.ai/code/session_c5c0ce54-bb9c-478c-9e5b-cf44b80d4569 Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_c5c0ce54-bb9c-478c-9e5b-cf44b80d4569 Co-authored-by: Claude <noreply@anthropic.com>
📓 Docs Drift CheckThis PR changes 1 package(s): 4 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
⛔ 1 release-owned page(s) also name something this change touched. These are read-only:
What this run could not see
Coarse fallback — 5 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 a61412bea9f99e1d336588a55287492a122cd965 && git checkout a61412bea9f99e1d336588a55287492a122cd965
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin bc2bf01c8a30f0caed922b33d21ab2dac2bba274 b373361f64acc867de530d39b762d8772a7e5c1e && git checkout -B drift-repro bc2bf01c8a30f0caed922b33d21ab2dac2bba274 && git merge --no-ff b373361f64acc867de530d39b762d8772a7e5c1e
node scripts/docs-audit/affected-docs.mjs --json bc2bf01c8a30f0caed922b33d21ab2dac2bba274
|
…strings Claude-Session: https://claude.ai/code/session_c5c0ce54-bb9c-478c-9e5b-cf44b80d4569 Co-authored-by: Claude <noreply@anthropic.com>
|
PM 复核(Session c5c0ce54):通过,等 CI 复核对象:head 逐项核对:
范围外,不阻塞:
下一步:CI 全部跑完且无失败,就转 ready 并入合并队列(以 timeline 出现 |
Closes #17634
A late ack from a claim the visibility-timeout reap had taken back overwrote the live re-claim on
sys_http_delivery:IHttpOutbox.ack(id, result)carried no claim credential, and both stores wrote by id. This PR gives the HTTP outbox the protection the notification outbox has had since #11859 — no new design: the (claimedBy,claimedAt) credential round-tripped fromclaim(), a compare-and-set that re-states ownership in the write, a refusal that writes nothing, and the dispatcher'sack refused, claim no longer heldwarn.Baseline red — the defect reproduces on
origin/mainBase
8f751cdc5, which contains #17632's mergea9096af48(git merge-base --is-ancestor a9096af48 HEAD→ exit 0). The reproduction test was committed alone asf945fa864—git diff --stat 8f751cdc5 f945fa864 -- . ':!**/*.test.ts'prints nothing — and run there:The sequence is the card's, driven through two real
HttpDispatchers sharing one store — injected clock, gated fetches, no hand-poked rows. Node A claims atT0; A's POST moves the clock pastclaimTtlMsand runs node B's tick, which reaps and re-claims the row and starts POSTing (held open); A's POST answers 410 and A acks. On base the row readdead, unclaimed, 1 attempt, response 410 — while B was still sending. The two green legs are the negative controls (the same dispatchers without the reap: A's ack lands). Reproduced onMemoryHttpOutbox, and onSqlHttpOutboxover a realObjectQL+SqlDriver(better-sqlite3:memory:), the harnesshttp-dispatcher-idle-cost.integration.test.tsuses.The compatible form — design and reasoning
IHttpOutbox.ackgains an optional third parameter:HttpClaimCredentialis{ claimedBy: string; claimedAt: number }— exactly the pair the notification outbox'sClaimedDeliveryRecordguarantees, with the same meaning: the pair identifies one CLAIM, not one node, and ownership is proven by handing back whatclaim()returned (the dispatcher passes the claimed row itself). Three forms were weighed against the three kinds of existing code:IHttpOutboximplementationack()ackClaimed?) + deprecatedack— thereap?pattern from #17632ack(id: string, …)implementation; a union compiles but hands that implementation an object at run timeThe subclass column is not hypothetical:
TickRecordingOutboxinhttp-dispatcher-idle-backoff.test.tsoverridesMemoryHttpOutbox.ack()to record retry schedules. Under the new-method form its retry legs would have lost their readings; under the chosen form it keeps working unchanged, and this PR makes it forward the credential (one line) so its dispatcher acks keep the check.For the same reason
claim()keeps declaringHttpDelivery[]instead of a narrowed claimed-record type: narrowing a built-in store's declared return type stops a subclass override that declares the old one from compiling (TS2416 — that test double overridesclaim()too).SqlHttpOutbox.claim()does now stamp the credential explicitly on its results, in the values its claiming UPDATE wrote, asSqlNotificationOutbox.claim()does.The price, stated: omitting
claimedstill compiles, so the checked path is not type-enforced for a caller. The one production caller isHttpDispatcher, pinned by the dispatcher legs and by ablation A below. The two-argument arity is documented as deprecated (it checks no ownership); retiring it, and making the parameter required, is next-major work.Changeset:
minor, not declared breaking — nothing an implementer or caller wrote has to change.Acceptance, item by item
1.
IHttpOutbox.ackcarries the claim credential, aligned withClaimedDeliveryRecord, in a compatible form. As above. New exports:HttpClaimCredential,HttpAckError.2.
SqlHttpOutbox.ackwrites only onid+claimed_by+claimed_at+status = 'in_flight'; no match ⇒ nothing written, logged atwarnin the notification side's words. Handed the credential,SqlHttpOutbox.acktakesSqlNotificationOutbox.ack's shape: two deterministic refusals read before any write (row notin_flight; row claimed under another credential); the same two tests re-stated in the conditional write throughdispatcherAckCasOptions(id, 'in_flight', claimedBy, claimedAt), i.e.where: { id, status: 'in_flight', claimed_by, claimed_at }on the predicate path (updateMany— the by-id path discards every predicate but the id, #11009); and a(status, attempts)read-back that reports a write which matched nothing. A refusal throwsHttpAckErrorwithDELIVERY_NOT_ELIGIBLE— the code this package already raises for a delivery row in the wrong state, so no new code — and writes nothing.HttpDispatcherabsorbs exactly that code and logshttp-dispatcher: ack refused, claim no longer held(the notification dispatcher's line isnotification-dispatcher: ack refused, claim no longer held), then carries on with the rest of its batch.delivery-update-tenant-audit.integration.test.tsnow reads the HTTP ack on itsupdateManyspy and requires, for both organizations' rows, a scalarwhere.id+status: 'in_flight'+claimed_by: 'n1',bypassTenantAudit: true, notenantId, zero by-idupdatecalls on the object and no audit line — then theupdateManypositive control fires.3.
MemoryHttpOutboxchecks the same. The same two tests (single-threaded, so each is atomic with the mutation), the same error, and the same messages — one helper both stores call.4.
HttpDispatcherpasses the credential its own claim returned.ackAttempt()hands the claimed row back as the credential. Pinned by the dispatcher legs; see ablation A.5. Tests, both stores.
src/http-outbox-ack-claim-ownership.integration.test.ts,describe.eachoverMemoryHttpOutboxandSqlHttpOutbox, 16 tests:in_flight:node-b:1060001:0with no response code, with exactly one warn namingnode-aand the row; B's ack then recordssuccess, 1 attempt, B's 200); the negative control (no reap: A's ack landsdead, 410, no warn); and an outbox whoseackreads(id, result)only still gets its attempt recorded.claim()instants; its negative control; the same node's own re-claim refusing its stale ack (claimedAtis the discriminator); a claim reaped and not re-claimed refusing a retry-shaped ack, whose post-state would also readpending, so onlyattemptscould expose a write that landed; and a credential missing a member refused before anything is written. Refusals assert the error identity (name: 'HttpAckError',code: 'DELIVERY_NOT_ELIGIBLE'), never a bare throw.Ablation A — one-off, trap-restored
On the committed fix (
06215b074) the dispatcher stopped passing the credential:outbox.ack(row.id, result, row as HttpClaimCredential)becameoutbox.ack(row.id, result)(on disk: removed text 1 → 0, injected text 0 → 1). The tests import the source by relative path, so no dist rebuild is in the resolution path.Restored with
git checkout HEAD -- PATH: blobf8e5db21fbefore and after, andgit diff HEADon the path is empty. The store legs stayed green under the mutation — they pin the stores, the dispatcher legs pin the dispatcher.Docs and census
content/docs/automation/webhooks.mdx§12 gains a failure-mode row: a send that outlasts the claim TTL while another dispatcher re-claims the row.sql-http-outbox.ts(4 → 5updatesites), so the tenant-audit census moves 222 → 223.node scripts/tenant-audit-census.mjs --writeregeneratedcontent/docs/permissions/tenant-audit-census.mdxanddocs/audits/2026-08-tenant-audit-write-call-sites.counts.md, and the eight hand-written prose figures the gate named moved with it (unreadable 66 → 67, the over-claim 83 → 84, undecidable 73 → 74, elevation-undecidable 100 → 101, population 222 → 223).Contract
No
packages/spec/src/**in the diff, and nothing inpackages/specchanges what it accepts or rejects. The contract change is onservice-messaging's own runtime interfaceIHttpOutbox: one optional parameter, plus two exports.Verification — head
b373361f6origin/mainwas merged in at240704622(8 incoming commits, none touching this diff's paths, lockfile unchanged); every reading below is on the final head.pnpm --filter @objectstack/service-messaging build→ exit 0,check-dts-emitted: 2/2pnpm --filter @objectstack/service-messaging typecheck→ exit 0;tsc --listFilesincludes all three touched test filespnpm --filter @objectstack/service-messaging test→ 40 files, 423 tests passed (the 16 new ones included)pnpm turbo run build --filter='@objectstack/service-messaging^...' --concurrency=2→ 15/15scripts/pm/os-verify-lock.shran in declared UNLOCKED mode on this host (no usableflock), so nothing was serialized.node scripts/pm/dispatch-gates.mjs --commands→ 94 commands, every one run onb373361f6with its exit code captured before any pipe;node scripts/pm/dispatch-gates.mjs --ran→94 derived famil(ies) accounted for — 93 run, 1 NOT-MEASURED (1 DERIVED from a recorded exit 3). 92 exited 0, among them:check-tenant-audit-census: OK -- 223 write call sites certified (149 decidable; 9 tenancy-enabled sites PROVABLY carry no tenant context, 32 more unreadable), 23 prose figures held to the census.check-adr-0087-registration: this PR adds no declared-breaking changeset (1 non-breaking changeset(s) seen).check-doc-authoring→sibling-package prose ids hold the baseline … no growth. On the first sweep it caught the tracker id this PR had put into twoHttpAckErrormessage strings;b373361f6takes it out of the strings (the JSDoc keeps it).check-dispatcher-error-vocabulary: OK,check-engine-double-contract: OK,check:nul-bytes,check:cross-package-test-inputs,check:published-files,check:docs(222 generated files in sync),check-changeset-no-major,check-empty-changeset.check:skill-examplesandcheck:i18nfirst refused on unbuilt prerequisites (packages/client-react/dist; the workspace CLI and its extract closure). After the targeted closure the gates name —pnpm exec turbo run build --concurrency=2over the CLI, its 9 extract packages and@objectstack/client-react..., 58 tasks — both ran green:258 prose examples type-check across 3 surface(s)andcheck-i18n-bundles: OK (9 package(s) — all bundles in sync, no undeclared authoring keys). That build also letcheck:type-check-debtmeasure:--re-measure: OK — 5 ledger entr(ies) re-measured …, none above its recorded number.check:dual-build-cjs-loads— exit 3, PREREQUISITE NOT MET: it reads thedist/of every package (hono, account, setup, studio, client, cloud-connection and 46 more), a workspace-wide build this seat does not run. CI measures it.check:merge-driver— exit 1: its self-test fails the case "a gate whose RUNNER is not installed refuses the same way" and that case's diagnosis leg. Control leg: the same command in a detached worktree atorigin/main76c9fab30, on this host, exits 1 with the identical failing cases. This diff touches none ofscripts/git-env.mjs,scripts/git-merge-regen.mjs,scripts/check-regen-pending.mjsor.gitattributes. Left for CI; reported to the dispatching PM.Acceptance notes
pnpm check:merge-driverself-test exits 1 on this host atorigin/main76c9fab30itself — the case "a gate whose RUNNER is not installed refuses the same way" and its diagnosis leg. Not touched by this diff.packages/spec/src/api/error-code-ledger.zod.ts— the prose underDELIVERY_NOT_ELIGIBLEnames its throw surfaces per surface (IHttpOutbox.redeliver,INotificationOutbox.ack);IHttpOutbox.ackhanded a credential (HttpAckError) is now a third. Left unedited to keeppackages/specout of this diff; the code is already registered under@objectstack/service-messaging, so no gate reads that prose. Picked up by: the next PR that edits that ledger entry; none scheduled.IHttpOutbox.ackarity stays unchecked by design, for compatibility. Retiring it and makingclaimedrequired is next-major work. Picked up by: none scheduled.(status, attempts)read-back that reports a missed conditional write is blind to one interleaving — the row reaped, re-claimed and acked by another node to the same status with the same attempt count, all between this ack's read and its write.SqlNotificationOutbox.ackhas the identical detector;IDataEngine.updatedeclares its return asany, so the row is the only answer available. Not reproduced. Picked up by: none.HttpDispatcherstill callsonAttemptafter a refused ack (the attempt did go on the wire) — unchanged from before, andonAttempthas no production subscriber in this repo.delivery-update-tenant-audit.integration.test.tsstill describes theredeliverassertions as a spy onSqlDriver.update; they moved toupdateManyin 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. Pre-existing prose drift.Generated by Claude Code