Skip to content

fix(service-messaging): read the outbox claim credential back from the store, so ack() lands on a real column - #23138

Merged
objectstack-fleet[bot] merged 6 commits into
mainfrom
claude/issue-23122-outbox-claim-credential
Oct 13, 2026
Merged

objectstack-fleet[bot] merged 6 commits into
mainfrom
claude/issue-23122-outbox-claim-credential

Conversation

@objectstack-fleet

@objectstack-fleet objectstack-fleet Bot commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #23122
Clause-②: no — a runtime fix to the outbox claim credential; no published accept set or public surface moves.

What changes

SqlNotificationOutbox (sys_notification_delivery) and SqlHttpOutbox (sys_http_delivery) now hand out the claim credential as the store holds it:

  • claim() / claimDigest() return claimedAt = the claimed_at value read back from the row after the claiming UPDATE, not the in-memory now. The value written is unchanged.
  • The read-back that identifies the claimed rows filters on candidate id, holder and status only. claimed_at: now is no longer an equality predicate there.
  • ack() is unchanged in code. Its read half (current.claimed_at !== claimed.claimedAt) now compares two reads of the same cell, and its compare-and-set (dispatcherAckCasOptions) now binds the stored value. Doc comments on IHttpOutbox.claim and dispatcherAckCasOptions say so.
  • ClaimedDeliveryRecord, HttpClaimCredential and every export are unchanged. No schema change.

Credential shape chosen: the value read back from the store, not a claim token. A token needs a new text column on both platform objects, with label translations, and a new member on the published credential types. That is a public-surface move the claim's Clause-②: no rules out. The read-back shape needs none of that. It was measured exact on live PostgreSQL 16 real and numeric columns, and on SQLite and memory (the existing suites).

Measurement: live PostgreSQL 16.15 (extra_float_digits = 1), SqlDriver + node-postgres

Fresh tables (numeric), then the epoch-ms columns altered to real to recreate a table from before the new column mapping. Pre-fix code at bcebd8409:

now written stored text JS read-back claim() rows SQL claimed_at = now ack()
1791810012345 1.7918101e+12 1791810100000 1 matches refused, DELIVERY_NOT_ELIGIBLE; row stays in_flight, attempts 0
1791813456789 1.7918135e+12 1791813500000 1 matches refused (same)
1791857407123 1.7918574e+12 1791857400000 1 matches refused (same)
control: numeric column exact exact 1 matches ok

The same three refusals happen on sys_http_delivery (HttpAckError, DELIVERY_NOT_ELIGIBLE).

  • H1 confirmed. The failing check is ack()'s JS read half. The read-back is the shortest decimal that round-trips to the stored binary32. That is neither now nor Math.fround(now). A round instant such as 1791810000000 happens to read back equal and acks, which is why the loop is per row and not universal.
  • H2 falsified on PostgreSQL 16. The claim's verification find (claimed_at: now) matched: node-postgres sends the parameter untyped, so the server coerces it to real. The reporter's loop therefore takes the ack path, not the claim path. This is consistent with an inbox row being written every cycle. The predicate is still removed, because the equality holds only when the store coerces the comparand to the column's own type.
  • H3 confirmed. The reap compares binary32 with binary32. It does not fire at claimTtlMs + 1 but does fire within one 131 072 ms step after it.

Fixed code (e2af29fd9; git diff --stat e2af29fd9 HEAD over the four source files is empty), same server:

  • On the real column all three instants ack, for both outboxes. The row reaches success with attempts 1, and the credential is the read-back value. The numeric control acks.
  • Stuck row: an ack carrying the pre-fix credential is refused, which reproduces production (in_flight, attempts 0). After the reap, the fixed code re-claims the row and the ack lands. A later claim takes nothing.
  • Lost claim: node B reaps and re-claims. Node A's late ack is refused with DELIVERY_NOT_ELIGIBLE. Node B's ack lands.

Pins: outbox-claim-credential-lossy-column.integration.test.ts (15 tests)

A real engine (ObjectQL + SqlDriver, better-sqlite3) sits behind a double that gives the epoch-ms columns Postgres real semantics:

  • it stores the binary32 rounding;
  • it coerces comparands to the column's type;
  • it reads back the shortest round-tripping decimal.

A fidelity leg pins the three live readings above. Every write is still a real conditional UPDATE. Per outbox, on the real double, on an exact SQLite column (control) and on the memory store (control):

  • one send and one ack reach success, and three later reap windows send nothing;
  • a claim lost to another node still refuses the late ack, and the holder's ack lands;
  • real double only: a row stuck in_flight behind the pre-fix credential is sent exactly once more over five dispatcher-shaped passes (reap, claim, ack, DELIVERY_NOT_ELIGIBLE absorbed), then left alone. No operator act is involved.

Negative-pin ablation (committed first, at dee1e11da)

scripts/ablation-replace.mjs (anchor must hit, blob hash verified) put the pre-fix verification and credential back. Both files then ran under a trap restore.

  • Leg A: sql-outbox.ts. return this.readBackClaimed(ids, opts.nodeId); (x2) was replaced by the old claimed_at: now find plus claimedAt: now. Blob 8821ed0806e6 became ef1e0996acd3. Result: Tests 3 failed | 12 passed (15). Exactly the three SqlNotificationOutbox on a real column legs went red:
    × one send, one ack: the row reaches success, and no later pass sends it again
      AssertionError: promise rejected "NotificationAckError: ..." instead of resolving
      Caused by: NotificationAckError: Delivery row '...' is no longer held by the claim this ack completes
        (it now reads 'in_flight'), so the ownership-checked conditional update matched no row and NOTHING was written
      Serialized Error: { code: 'DELIVERY_NOT_ELIGIBLE' }
    × a claim lost to another node still refuses the late ack, and the holder's ack lands
      (same refusal, on the HOLDER's ack)
    × a row stuck in_flight behind an ack that could never land is sent once more, acked, and left alone
      expected [ ...(5) ] to deeply equal [ Array(1) ]   (the same id sent on all five passes)
    
  • Leg B: sql-http-outbox.ts. claimedAt: now, and the old claimed_at: now predicate went back in, nested. Result: Tests 3 failed | 12 passed (15). The same three legs went red for SqlHttpOutbox on a real column, with HttpAckError and DELIVERY_NOT_ELIGIBLE.
  • Controls. The exact-column and memory legs stayed green in both runs.
  • Restore. sql-outbox.ts is at 8821ed0806e6, the HEAD blob. sql-http-outbox.ts is at ceb424363276, the HEAD blob. git diff HEAD is empty.

Census: every platform claim or lease check comparing a stored epoch-ms number column by equality

Tree bcebd8409 (pre-fix), non-test .ts under packages/, examples/ and apps/:

Pass Search Hits Disposition
1 git grep -E "(claimed|locked|leased|lease|lock|heartbeat|acquired|taken|picked|started)_at['\"]?\s*:\s*(now|claimedAt|claimed\.claimedAt|ts|stamp|t)\b" 9 7 service-messaging (below); service-job/db-job-adapter.ts:547 and service-storage/metadata-store.ts:609 are started_at written on insert (datetime), never compared
2 JS === / !== on a *_at / *At operand 38 lines, 23 after dropping null/undefined/literal/typeof in class: sql-outbox.ts:269, sql-http-outbox.ts:380 (ack read halves); memory-outbox.ts:150, memory-http-outbox.ts:162 (JS numbers, no column, exact: unchanged). Out of class: approval-service.ts:6224 and suspended-run-store.ts:206 (node-id strings), marketplace-install-local-plugin.ts:1761 (ISO string from a JSON ledger file), verify/parent-write-reach.ts:824 (diagnostic updated_at compare, not a claim)
3 multi-line where: { ... } literal scanner: a *_at key bound to a non-literal, non-operator value, 3325 files 4 all service-messaging. Positive control sql-outbox.ts:189 is present. sql-outbox.ts:189, :227 and sql-http-outbox.ts:259 are the claim verifications: fixed (predicate removed, credential read back). outbox-dispatcher-scope.ts:290 is the ack compare-and-set: fixed by construction (it now binds the read-back value; measured matching on PG real)
4 raw SQL _at = ? / _at = $n 1 metadata-protocol/protocol.ts:2532, a doc comment, no code
5 Field.number columns holding time on platform objects 12 the two delivery objects' epoch-ms columns (this fix); sys_automation_run.start_time (no claim or lease reads it); five *_ms durations

No same-class hit outside packages/services/service-messaging. One adjacent shape is out of class: service-storage/metadata-store.ts:732, the upload-session progress compare-and-set. It compares byte counts, not epoch-ms, and its comparand is already a store read-back.

Verification

  • pnpm --filter @objectstack/service-messaging test: 49 files, 564 tests passed, on the main-merged tree e97bbb72e.
  • pnpm --filter @objectstack/service-messaging typecheck: clean. tsc --listFiles includes the new test file.
  • Dependency closure built first (pnpm --filter '@objectstack/service-messaging^...' build), then the package itself for the gates that read dist/.
  • Gates: 66 derived (node scripts/pm/dispatch-gates.mjs --commands, no paths, identical at e97bbb72e and 82c0b1457). First union at e97bbb72e: 63 exit 0; check:query-options-erasure exit 1 (test surface 236 to 240, from the new double's erased options), fixed in 82c0b1457 and re-run exit 0 (236, at the ceiling); check:i18n exit 3 PREREQUISITE NOT MET (CLI closure not built), prerequisite closure since built; check:dual-build-cjs-loads exit 3 PREREQUISITE NOT MET (needs every package's dist/): NOT MEASURED locally, declared to CI, narrowed probe require('./dist/index.cjs') of this package loads (59 exports). The full union re-run at the final head 82c0b1457 was in progress when this PR opened; its verdict is in the os-dev-report comment on the card.

Acceptance notes

  • Concurrent same-node claims. The claim read-back no longer separates two concurrent claim() calls that carry the same nodeId over overlapping candidates. Before, they were told apart only when their now differed by at least 1 ms. The dispatcher never does this: a node's ticks never overlap, and claim / claimDigest split rows on digest_key. On a real column the read-back credential still separates a re-claim from the claim it replaced. The re-claim needs a reap first, the reap needs the stored instant to be older than now minus claimTtlMs on the same column, and the re-claim then stores a later instant.
  • Not measured: PostgreSQL 11 and older. Their default extra_float_digits = 0 prints six significant digits, which do not round-trip binary32. On a real column there, the compare-and-set binding the read-back value would miss. Those versions are past upstream support.
  • Out of this card, per triage and the "new tables only" ruling:
    • existing real columns stay as they are, so the reap keeps its ~131 s granularity;
    • no reap-count dead-letter policy;
    • no clean-up of duplicate sys_inbox_message rows.
  • Reporter's framing. The issue says the stored value "reads back rounded to a multiple of ~131 s". Through node-postgres it reads back as the shortest round-tripping decimal, for example 1791810100000 for a binary32 of 1791810076672. The conclusion is the same.

Final verification (appended by the domain:services seat 2 PM)

The dev writes this body once, so the seat appends the dev's report text here (report 6137555861 on #23122). The final union re-run, which was still in progress when this PR opened, finished at 82c0b1457:

  • 66 of 66 derived gate families exited 0;
  • check:i18n and check:dual-build-cjs-loads were measured after their prerequisite closure was built;
  • dispatch-gates --ran reports 66 run and 0 NOT-MEASURED.

Generated by Claude Code

…e store

claim()/claimDigest() on SqlNotificationOutbox and SqlHttpOutbox now hand
out claimedAt as the claimed_at value the store returned after the claiming
UPDATE, not the in-memory now, and identify the claimed rows by exact
predicates only (candidate id, holder, status). On a Postgres table whose
epoch-ms columns are still `real`, the stored instant is a float32 rounding
of now, so the old credential never equalled the stored value: every ack()
was refused as a lost claim and the row was re-sent after each reap.

Claude-Session: https://claude.ai/code/session_01FzQy26KznSa5j2gnf3AzsE
Co-authored-by: Claude <noreply@anthropic.com>
A real engine behind a double that gives the epoch-ms columns Postgres real
semantics (binary32 store, comparand coercion, shortest-decimal read-back,
pinned to three live PostgreSQL 16.15 readings). Both outboxes: one send and
one ack reach success, a claim lost to another node still refuses the late
ack, and a row stuck in_flight behind an ack that could never land is sent
once more and acked with no operator act. Exact-column and memory stores are
the controls.

Claude-Session: https://claude.ai/code/session_01FzQy26KznSa5j2gnf3AzsE
Co-authored-by: Claude <noreply@anthropic.com>
…n the lossy-column pins

Claude-Session: https://claude.ai/code/session_01FzQy26KznSa5j2gnf3AzsE
Co-authored-by: Claude <noreply@anthropic.com>
…nstead of erasing their options

Claude-Session: https://claude.ai/code/session_01FzQy26KznSa5j2gnf3AzsE
Co-authored-by: Claude <noreply@anthropic.com>
@github-actions github-actions Bot added size/l documentation Improvements or additions to documentation tests tooling labels Oct 13, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/service-messaging, touching 5 documentable anchor(s). ⚠️ 1 changed file(s) yielded no anchor (packages/services/service-messaging/src/outbox-dispatcher-scope.ts), so the pages documenting them are NOT COVERED by this run — this is not a clean bill of health for those files.

1 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/automation/webhooks.mdx (via in_flight (literal, a string literal in claim; a string literal in claimDigest; a string literal in readBackClaimed))

⛔ 1 release-owned page(s) also name something this change touched. These are read-only:

  • content/docs/releases/v17/17-2.mdx (via IHttpOutbox (symbol, a top-level interface))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see
  • 1 changed file(s) yielded no anchor (packages/services/service-messaging/src/outbox-dispatcher-scope.ts) — pages documenting those are invisible to this run
  • 2 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 54 of 206 client-bound route-ledger rows — the other 152 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 152: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 97 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.
  • a key NAME is not a key, so the hand re-read the line above prescribes can land on the wrong schema. The same spelling is authorable on one governed type and a [REMOVED] tombstone on another for each of active, aria, joins, objects, template, tools and version (censused on [finding] tools is a key on BOTH AgentSchema (tombstoned, dead) and SkillSchema (live, cloud-attested), so a name-based search attributes skill examples to the agent key — it produced a false stop-the-line alarm on PR #19059 #19093 over the liveness ledger's governed types, top-level keys); nothing in a search result distinguishes the two, so a grep hit on a LIVE example reads as evidence about the DEAD key. Measured on fix(spec): the agent.tools liveness row says dead — it claimed live on a key the schema tombstoned #19059: content/docs/ai/agents.mdx was reported as contradicting the agent.tools tombstone over its tools: example at :161, which is inside the defineSkill({ block opened at :155 — the page was already correct. Settle ownership by PARSING the value against both schemas, never by the name: that literal PASSES SkillSchema, and as an AgentSchema it FAILS at tools with the tombstone prescription. ⛔ These names are not the whole class — a key retired through a .strict() guidance map leaves no tombstone in the walked shape and none of them here (tool.category, live as AIToolDefinition.category).

Coarse fallback — 5 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 287fd6e5d054b9b1efa80f08f9e5be609edb32c2 → packageMentionDocs.

Which tree this was computed on

This run read content/docs from 3690c30c476dadb01335543e20c776d12339bc61 — the merge of head 82c0b1457f17b73b1cb91fc25180e3c79f1791f2 into base 287fd6e5d054b9b1efa80f08f9e5be609edb32c2, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 3690c30c476dadb01335543e20c776d12339bc61 && git checkout 3690c30c476dadb01335543e20c776d12339bc61
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 287fd6e5d054b9b1efa80f08f9e5be609edb32c2 82c0b1457f17b73b1cb91fc25180e3c79f1791f2 && git checkout -B drift-repro 287fd6e5d054b9b1efa80f08f9e5be609edb32c2 && git merge --no-ff 82c0b1457f17b73b1cb91fc25180e3c79f1791f2

node scripts/docs-audit/affected-docs.mjs --json 287fd6e5d054b9b1efa80f08f9e5be609edb32c2

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 287fd6e5d054b9b1efa80f08f9e5be609edb32c2 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

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

Labels

documentation Improvements or additions to documentation size/l tests tooling

Projects

None yet

2 participants