Skip to content

service-messaging: a late HTTP ack from a REAPED claim overwrites the live re-claim on sys_http_delivery — IHttpOutbox.ack(id, …) carries no claim credential (the notification outbox fixed this shape in #11859) #17634

Closed

Description

@hotlong

现象

sys_http_delivery 上,一个已被回收(reap)的认领的迟到 ack,会覆盖另一个节点正在进行的重新认领。

来源:#17623 开发席位在 PR #17632 期间的一次性探针(针对构建后 dist 的 MemoryHttpOutbox):

  1. 节点 A 领取一行,开始发送;A 的认领超过 claimTtlMs,被 reap 回 pending
  2. 节点 B 重新领取这一行:in_flight,claimedBy = node-B,正在发送
  3. A 的发送结束,迟到的 ack 落下 ⇒ 该行变成 dead、claimedBy = null、attempts = 1——而 B 还在发送

原因(读码,origin/main)

packages/services/service-messaging/src/sql-http-outbox.ts:

async ack(id: string, result: HttpAckResult): Promise<void> {
    … where: { id } …          // 只按 id 写,不校验认领者
  • IHttpOutbox.ack(id, result) 的签名不携带认领凭据
  • SqlHttpOutbox.ack 的写条件只有 { id }(经 dispatcherAckOptions)
  • MemoryHttpOutbox.ack 无条件写

通知侧早已修过同一形状:sql-outbox.ts 的 ack(claimed: ClaimedDeliveryRecord, result) 以 (claimedBy, claimedAt) 作为认领凭据做 compare-and-set,读写两半都校验归属(#11859),并对未认领行的 ack 做前置拒绝(#11453)。HTTP outbox 从未获得这层保护。

后果

  • 一次正在进行、可能成功的投递被标成 dead;或者两次投递的结果互相覆盖,最终状态与实际发送结果不一致
  • webhook 可能重复发送,或该送达的被错误记为失败
  • 触发条件是「一次发送耗时超过 claimTtlMs,同时另一个调度器实例在轮询同一张表」。多节点部署天然满足后半条;单容器内若同一环境的旧 kernel(stale / draining)与新 kernel 同时运行各自的调度器,推测也可能满足——这一点未实测

既有缺陷:PR #17632 既未扩大也未修复它。

验收

  1. IHttpOutbox.ack 携带认领凭据,形状与通知侧 ClaimedDeliveryRecord 对齐(新增为兼容形式,不破坏既有调用方——参照 check-adr-0087-registration 对契约成员的要求)
  2. SqlHttpOutbox.ack:写条件含 id + claimed_by + claimed_at + status = 'in_flight';不匹配时不写,并以 warn 记录「认领已不再持有」(与通知侧 ack refused, claim no longer held 同口径)
  3. MemoryHttpOutbox 同样校验
  4. 测试复现上面的三步序列:迟到 ack 不改变 B 的认领;B 的 ack 正常生效

Activity

  1. added
    bugSomething isn't working
    domain:servicesRetired 2026-10-22, alias of domain:server (services surface); in-flight cards keep it until landed
    on Sep 11, 2026
  2. hotlong commented on Sep 11, 2026

    @hotlong
    ContributorAuthor

    Claim: maintainer direct dispatch (cloud ops session, incident follow-up)
    Session: c5c0ce54-bb9c-478c-9e5b-cf44b80d4569
    Time: 2026-09-11T06:40Z
    Branch: claude/issue-17634-http-ack-claim-credential
    Worktree: ~/Documents/GitHub/objectstack-issue-17634
    Domain: domain:services
    File surface: packages/services/service-messaging/src/http-outbox.ts (ack contract), sql-http-outbox.ts, memory-http-outbox.ts, http-dispatcher.ts (ack call site) + tests
    Serial constraint cleared: PR #17632 (same files) merged at a9096af48.
    Clause-②: no — HTTP outbox ack ownership check inside service-messaging; no packages/spec/src/** surface expected (path limb re-checked on the actual diff at review)

  3. self-assigned this
    on Sep 11, 2026
  4. hotlong commented on Sep 11, 2026

    @hotlong
    ContributorAuthor

    os-dev-report
    {
    "issue": 17634,
    "status": "done",
    "branch": "claude/issue-17634-http-ack-claim-credential",
    "pr": "#17641",
    "premise_still_valid": true,
    "summary": "Premise reproduced on origin/main 8f751cd (contains a9096af) for both stores before any fix. IHttpOutbox.ack gains an optional third parameter, claimed?: HttpClaimCredential ({claimedBy: string, claimedAt: number}, the pair ClaimedDeliveryRecord guarantees). Handed it, SqlHttpOutbox.ack and MemoryHttpOutbox.ack refuse unless the row is still in_flight under that pair: read-half refusal, then the same test re-stated in the write via dispatcherAckCasOptions (where id + status in_flight + claimed_by + claimed_at, updateMany), then a (status, attempts) read-back; a refusal writes nothing and throws the new HttpAckError (existing code DELIVERY_NOT_ELIGIBLE). HttpDispatcher.ackAttempt always passes the claimed row, absorbs only that code and warns 'http-dispatcher: ack refused, claim no longer held'. Compatible form chosen over a new optional ackClaimed() method (a subclass overriding a built-in store's ack() would silently stop seeing dispatcher acks; TickRecordingOutbox in this repo is one) and over a record-typed first parameter (not assignable from existing (id: string) implementations, or an object handed to them at run time); claim() keeps declaring HttpDelivery[] because narrowing it breaks subclass overrides (TS2416). Changeset minor, not declared breaking. No packages/spec/src/** in the diff and no spec accept/reject change (Clause-2 stays no). Tenant-audit census regenerated 222 to 223 plus 8 hand-written prose figures; webhooks.mdx section 12 gains a failure-mode row. Deviation noted: the PR body's first line is 'Closes #17634' as this dispatch specified, where os-dev.md prescribes 'Fixes #17634' (same closing effect). Assignee untouched; PR labels were auto-applied by the labeler (documentation, size/l, tests, tooling), none added by this seat.",
    "tests": "BASE RED: repro test committed alone as f945fa8 (non-test diff vs 8f751cd empty); vitest run src/http-outbox-ack-claim-ownership.integration.test.ts: 2 failed | 2 passed (4), 'AssertionError: expected dead:-:-:1:410 to be in_flight:node-b:1060001:0:-' on MemoryHttpOutbox and on SqlHttpOutbox (real ObjectQL + SqlDriver better-sqlite3), negative controls green. FIX (final head b373361, origin/main merged at 2407046): new file 16/16 (dispatcher legs: card replay, no-reap control, two-parameter legacy outbox; store legs: card replay, control, same-node re-claim, reaped-not-reclaimed retry ack, incomplete credential; refusals assert name HttpAckError + code DELIVERY_NOT_ELIGIBLE); delivery-update-tenant-audit pins the HTTP ack on updateMany with where.id scalar + status in_flight + claimed_by n1, bypassTenantAudit true, zero by-id updates, updateMany positive control. pnpm --filter @objectstack/service-messaging build exit 0 (check-dts-emitted 2/2), typecheck exit 0 (tsc --listFiles includes the 3 touched test files), test 40 files / 423 passed. ABLATION A (on committed fix 06215b0, trap-restored): dispatcher stops passing the credential (removed text 1->0, injected 0->1) -> 2 failed | 14 passed (16), both card-replay dispatcher legs red with the base assertion; restore proven by HEAD blob f8e5db21f == hash-object after, git diff HEAD on the path empty. GATES on b373361: dispatch-gates --commands 94, all run with exit codes; --ran: '94 derived famil(ies) accounted for — 93 run, 1 NOT-MEASURED (1 DERIVED from a recorded exit 3)'. 92 exit 0 incl. check-tenant-audit-census OK 223 sites / 23 prose figures, check-adr-0087-registration 'this PR adds no declared-breaking changeset', check:doc-authoring (first sweep caught tracker ids in two HttpAckError strings; fixed in b373361), check:dispatcher-error-vocabulary, check:engine-double-contract, check:nul-bytes, check:cross-package-test-inputs, check:published-files, spec check:docs, check:skill-examples and check:i18n (green after the named 58-task prerequisite closure), check:type-check-debt ('re-measure: OK … none above its recorded number'). NOT MEASURED: check:dual-build-cjs-loads, exit 3 PREREQUISITE NOT MET (needs dist of every package, a workspace-wide build this seat does not run). RED NOT FROM THIS DIFF: check:merge-driver exit 1 (self-test case 'a gate whose RUNNER is not installed refuses the same way' + diagnosis leg); control leg in a detached origin/main 76c9fab worktree on this host exits 1 with the identical cases; worktree removed. os-verify-lock ran in declared UNLOCKED mode (no flock on host). CI on the PR not waited on.",
    "mcp_calls": "0 — GitHub reads and writes went through the gh CLI",
    "open_questions": [],
    "out_of_scope_findings": [
    "reported for PM to file (not filed by this seat): pnpm check:merge-driver self-test exits 1 on this macOS host at origin/main 76c9fab itself — failing case 'a gate whose RUNNER is not installed refuses the same way' and its 'naming the command the shell could not find' leg (node scripts/git-env.mjs --self-test && node scripts/git-merge-regen.mjs --self-test && node scripts/check-regen-pending.mjs --self-test); repro: that command in a detached origin/main worktree",
    "noted, not filed: packages/spec/src/api/error-code-ledger.zod.ts prose under DELIVERY_NOT_ELIGIBLE lists its throw surfaces (IHttpOutbox.redeliver, INotificationOutbox.ack) and does not yet name IHttpOutbox.ack with a credential (HttpAckError); left unedited to keep spec out of the diff, no gate reads it; picked up by: none scheduled",
    "noted, not filed: the two-argument IHttpOutbox.ack arity stays unchecked by design for compatibility; retiring it and making claimed required is next-major work; picked up by: none scheduled",
    "noted, not filed: the (status, attempts) read-back that reports a missed conditional write is blind to one interleaving (reaped, re-claimed and acked by another node to the same status and attempt count between read and write) — identical to SqlNotificationOutbox.ack; not reproduced; picked up by: none",
    "noted, not filed: HttpDispatcher still calls onAttempt after a refused ack (unchanged; no production subscriber in this repo)",
    "noted, not filed: delivery-update-tenant-audit.integration.test.ts header still describes the redeliver assertions as a spy on SqlDriver.update although they moved to updateMany in #11009 (pre-existing prose drift)"
    ]
    }


    Generated by Claude Code

  5. added and removed
    pm:dispatchedDispatched to a dev agent by /pm-dispatch
    on Sep 11, 2026
  6. hotlong commented on Sep 11, 2026

    @hotlong
    ContributorAuthor

    PM 收尾(Session c5c0ce54):已落地

    范围外:check:merge-driver 在 macOS 上自测失败,已由 #16717 跟踪。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

bugSomething isn't workingdomain:servicesRetired 2026-10-22, alias of domain:server (services surface); in-flight cards keep it until landedpm:donepriority:p2Medium: important, M3

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions