Uh oh!
There was an error while loading. Please reload this page.
fix(service-messaging,plugin-webhooks): a dropped webhook subscription leaves a durable record that can never be redelivered unsigned - #8541
Conversation
…nt rows and record dropped webhook subscriptions durably (#8069) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ARidKDYSCD56LaygrvDPnk
…rrect the drop message (#8069) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ARidKDYSCD56LaygrvDPnk
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
📓 Docs Drift CheckThis PR changes 3 package(s): 106 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
⛔ 7 release-owned page(s) also reference the affected code. These are read-only:
|
os-zhuang
commented
Aug 13, 2026
PM review — ⛔ First: the routing premise was false, and that one is mine
My claim comment ( ⭐ The cost was small and one-directional: I ran an in-flight check against ⭐ The discriminator choice is the best decision in this PRRefusing on
That is the difference between a fix and a fix-shaped regression. The obvious guard (no signature ⇒ refuse) would have silently disabled redelivery for every deliberately-unsigned webhook. ⭐ And registering the ⭐ The fail-open found by measurement, not assumed
Ruling compliance
⭐ Step 3's justification is load-bearing and correctly argued: the ⛔ The standing prohibition holds: nothing here produces a Reverse verification — the control is what makes it credibleRED readings are behavioural, not "the new API is absent": ⭐ One test passed in both directions on purpose: the byte-for-byte dead-letter replay. That is the control proving the suite is not uniformly red for a trivial reason and that the refusal is not a blanket. All three vacuity traps are named, including the sharpest one — "asserting 'a row exists' is satisfied by the prohibited naive shape" — so the record tests assert the row is unclaimable, not merely present. ⭐ Updating the two #8022/#7986 guards rather than leaving them green is right: they asserted ⭐ And correcting #8043's message — it claimed the drop ends "with NO delivery and NO Generated by Claude Code |
os-zhuang
commented
Aug 13, 2026
|
… the spec references (#8069) The route table said "Re-queue a previously failed/dead delivery" and enumerated the failure codes as RESOURCE_NOT_FOUND / DELIVERY_NOT_ELIGIBLE. Both are now false for the population this card creates: a dead row with 0 attempts is a parked record, and redelivering it is refused with 409 DELIVERY_NEVER_SENT. content/docs/references/ is regenerated, not hand-edited. Every line of that diff flows from the one ledger entry: 132 lines are the rendered ErrorCode enum's cardinality counter (+265 more -> +266 more), and 2 list the new code. Verified NOT to be inherited drift — check:docs is green on this branch with only the ledger entry reverted and the json-schema tree regenerated. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ARidKDYSCD56LaygrvDPnk
os-zhuang
commented
Aug 13, 2026
Patch round pushed — 1. |
Uh oh!
There was an error while loading. Please reload this page.
Fixes#8069
Implements the maintainer's minimal cut (ruling
5271033283, 2026-08-12), in the ruled order. Cross-domain touch authorised by triage's routing ruling — #8069 (comment) — though see "the routing premise did not survive re-verification" below: it turned out not to be needed.1.
redeliver()refuses fail-closed — the safety property, landed firstassertHttpRedeliverable(inhttp-outbox.ts, called by both outbox implementations) refuses any terminal row withattempts === 0.ack()— the only writer of a terminal status — incrementsattemptsunconditionally, so terminal + 0 attempts is reachable only through the new parking door. Such a row was never sent, so re-sending it is not a replay: it is a first delivery conjured by an operator button. And a parked row carries no signature, because the signature is computed at enqueue from the very secret that could not be resolved — so that first delivery would go out unsigned, reopening #7799 through the auth-onlyPOST /api/v1/webhooks/redeliver. New error codeDELIVERY_NEVER_SENT, registered in the ADR-0112 ledger, 409 on the route.Why
attemptsand not "the row has no signature":signature === undefinedis ambiguous — it also means authored unsigned, a legitimate configuration — so refusing on it would break a working feature.attempts === 0on a terminal row is unambiguous, needs no new column and no new state, and stays fail-closed under the ambiguity that remains.Second layer, for the case the maintainer named specifically (the webhook config was deleted):
redeliver()now also consults a producer-registeredRedeliverGuard.service-messagingdeliberately knows nothing aboutsys_webhook, soplugin-webhooksregisterscreateWebhookRedeliverGuard, which refuses when the subscription row is gone or its stored secret cannot be recovered. Registered onMessagingServicerather than in the route, so every caller is covered — the card's whole point is a delivery going out through a door nobody audited. A guard that throws is a refusal: "could not check" never reads as "allowed".2. The durable record — reusing the existing reason column
A parked subscription now stays cached with
parkedReasonset and no credentials, and its matching events are written throughrecordUndeliverable()asstatus: dead,attempts: 0, cause in the existingerrorcolumn, no signature, no header map.3. No new lifecycle state was needed — the measured answer
The ruling makes step 3 conditional on step 2's column being unable to carry the cause. It can:
sys_http_delivery.erroris an unboundedField.textareawith no format contract, so it carries the cause and the remedy an AGENTS.mderrorowes.status in (failed, dead)and renderserroras a column. An operator finds the backlog with no new vocabulary to learn, no new saved filter, and no migration.attempts, a counter the outbox already maintains as a core invariant — so the reason column carries the cause for humans while the refusal stays structural.HttpDeliveryStatusis therefore unchanged. The decision is recorded on the type and on the object definition, not only here.Enumerating "signing configuration unavailable"
Measured conditions, and where each is covered:
sys_secretrow missing/deletedresolveSecretField, opaque stored valuesys_webhookrow deletedThat last row is a real fail-open I measured rather than assumed.
resolveWebhookSecretreturnsundefinedfor both "authored unsigned" and "the stored value is not a resolvable ref", so a guard built ontry/catchalone would read an unrecoverable key as a legitimately unsigned webhook and allow the replay. Presence is decidable from the masked read even when the value is not, so the guard asks "is a secret stored and did nothing come back?" instead. The enqueue side of that same ambiguity is NOT fixed here — it is a separate producer-side defect, filed as #8542, and out of scope for this card.Regression guards
sys_webhook.definition_json#7799 — nothing is delivered unsigned; a genuine dead-letter still replays byte-for-byte with the same signature (pinned, so "refuse everything" cannot pass).sys_http_deliveryrow, while it still readsactive:true#8022 — parking is not sticky: a refresh that resolves the credential re-arms and later events are signed. Say-once ledger untouched.errorused to end "with NO delivery and NO sys_http_delivery row". The second half is now false, so the message says where the evidence is instead. Anerrorthat misdescribes its consequence sends an operator looking in the wrong place.The routing premise did not survive re-verification
Triage's exception was granted because the card stated the
sys_http_deliveryobject definition lives inpackages/platform-objects(domain:metadata). It does not — it ispackages/services/service-messaging/src/objects/http-delivery.object.ts, this lane's own package, andpackages/platform-objectscontains nosys_http_deliverydefinition at all. Nodomain:metadatafile was touched, so the in-flight-collision risk the exception protocol exists to manage never applied. The routing ruling is linked above as the authority I worked under; the conclusion is that it was not needed.Reverse verification
Prediction written before running, then measured with the tests added and the implementation checked out from
origin/main.Behavioural RED — not merely "the new API is absent":
refuses to redeliver a terminal row that was never attemptedbuilds itsdead/0-attempt row withengine.insertalone and calls the currentredeliver(id):AssertionError: promise resolved "{ id: 'parked_1', …(21) }" instead of rejecting.promise resolved "'46b5d323-…'" instead of rejecting— unfixedenqueue()silently minted a pending row from it, which is the catastrophe.promise resolved … instead of rejecting(the old signature ignores the guard).expected [] to have a length of 1 but got +0— the drop leaves nothing.GREEN after restore, and one test passed in both directions on purpose: the byte-for-byte dead-letter replay, which is the control proving the suite is not uniformly red for a trivial reason and that the refusal is not a blanket.
Vacuity traps closed, named in the test files: (1) asserting the refusal on a
pendingrow would pass on a completely unfixed tree, since non-terminal rows were already refused — every refusal test usesdead; (2) asserting "a row exists" is satisfied by the prohibited naive shape, so the record tests assert the row is unclaimable and unredeliverable; (3) an over-broad refusal would pass everything above, so the replay control exists.Cross-package type reverse verification: renaming
registerRedeliverGuardin the consumer producedTS2551: Property 'registerRedeliverGuardXX' does not exist on type 'MessagingService', proving the consumer reads the rebuilt.d.ts; restored and confirmed byte-identical bygit hash-object.Two existing #8022/#7986 guards were updated rather than left passing: they asserted
outbox.list()is empty — the absence of durability this card removes — and they bound the enqueuer straight toIHttpOutbox.enqueueinstead of the messaging seam. Their fail-closed assertion (callsis empty) is untouched; the durability assertion is now stronger, checking the parked row isdead, unsigned, and header-free.Verification
service-messaging219/219,plugin-webhooks66/66,service-automation955/955, typecheck clean. Downstream consumer sweep —--filter '...@objectstack/service-messaging', the prefix form, i.e. dependents — 24 packages typecheck clean. Gates run locally:check:nul-bytes,check:i18n,check:error-code-casing,check:engine-double-contract,check:durability-log-level,check:test-source-alias,check:type-source-resolution,check:merge-driver,check:spec-parsed-alias,check:query-options-erasure,check:type-check-coverage,check:doc-formula-expressions,check-dev-prereqs, plus the ledger admission suiteerror-code-ledger.test.ts(16/16). Re-derived against the actual changed paths withscripts/pm/dispatch-gates.mjsand re-run after rebasing ontof1da948d8.Docs patch round (PM review, commit
f219c4206)Two statements on
content/docs/automation/webhooks.mdxthat this PR makes false, corrected: thePOST /api/v1/webhooks/redeliverroute-table row now says a delivery that was never attempted cannot be redelivered, and the route's failure-code enumeration — which reads as exhaustive — now carries409 DELIVERY_NEVER_SENTwith the reason (no signature on the row, so re-queuing would be a first delivery going out unsigned).content/docs/references/regenerated withgen:schema && gen:docs, never hand-edited, followed bygen:openapibecausegen:schema's cleanup wipes it. The gate listed 11 files, and all 11 are downstream of this PR's one ledger entry — no inherited drift frommain:check:docsreports228 generated files in syncon this branch with onlyerror-code-ledger.zod.tsreverted (after regenerating the gitignoredjson-schematree — the first attempt returned a stale-artifact prerequisite failure, not a content verdict). The diff bears that out mechanically: across the 10 non-ledger pages every changed line is the renderedErrorCodeenum's cardinality counter (+265 moreto+266 more, 66 each way, plus one+261/+262), with exactly two lines naming the new code.authorable-surface.base.jsonis unchanged and the tree was not in merge state.Gates:
check:docs,check:authorable-surface,check:docs-audit-scope,check:role-word,check:doc-anchors,check:adr-anchors,check:adr-links,check:doc-authoring,check:nul-bytes,check:error-code-casingall pass;spec10461/10461 after the regen.check:docsis a structurally un-derivable gate (#8478) and a sharp instance of it: the trigger and the failing artifact sit in different top-level directories with no textual link, because the relationship is "this file is a generator input" and that fact lives only inside the generator.Generated by Claude Code