Repository navigation
fix(plugin-webhooks): the redeliver veto refuses a delivery whose subscription is inactive - #22836
Conversation
…scription is inactive The `active` field declares that an inactive webhook is skipped by the dispatcher, and an operator's explicit replay is the dispatcher too. `createWebhookRedeliverGuard` now refuses unless `active === true`, the dispatcher's own predicate, with a reason that names the webhook, its state and the remedy (re-activate, then redeliver). It runs after the gone check and before the secret checks, which are unchanged. Pins: the guard (inactive, null/absent, order before the secret, active and gone controls) and the door on both faces (inactive 409, active and signed replayed, gone unchanged, re-activated replayed). Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013LbZ9MhPp1iriZEtgJqioA
📓 Docs Drift Check1 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to list — not a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run. What this run could not see
Coarse fallback — 4 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 499d445f9939c205ed1ecf51880c50fde0c53d0d && git checkout 499d445f9939c205ed1ecf51880c50fde0c53d0d
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin efcbac73cc58510278b970949ac3b65cb9712815 4c3ef9cef48085a1baaa97a15fbe50c8e70c5b15 && git checkout -B drift-repro efcbac73cc58510278b970949ac3b65cb9712815 && git merge --no-ff 4c3ef9cef48085a1baaa97a15fbe50c8e70c5b15
node scripts/docs-audit/affected-docs.mjs --json efcbac73cc58510278b970949ac3b65cb9712815 |
Contract reviewServed-tier: Inputs: card #22804 (body and its four comments: triage ① Derived judgments
② Semver level
③ Boundary flagsDev deviations (five, from
Nothing here goes to the maintainer's decision box: triage took the product reading on the card and the diff implements exactly that reading, tightening the door and loosening nothing. Implemented-by: VERDICT: PASS |
Fixes #22804
Clause-②: no
What changes
createWebhookRedeliverGuard(packages/plugins/plugin-webhooks/src/redeliver-guard.ts), the redeliver veto, now refuses a delivery whosesys_webhooksubscription is inactive. This follows triage's reading on the card: an explicit replay is "the dispatcher" foractive. The refusal works like the gone case. The door answers409 DELIVERY_NOT_ELIGIBLEand leaves the row untouched. The reason names the webhook, says it is inactive, and names the remedy:On the wire, the outbox puts its own prefix before the reason:
Delivery row 'ID' cannot be redelivered: REASON.5271033283on [security] a dropped webhook subscription leaves no durable record — and the naive fix hands an operator aredeliverbutton that sends UNSIGNED #8069 says no operator action may send an unsigned delivery. This change only adds a refusal and weakens nothing.activefield description stays as it is. "Inactive webhooks are skipped by the dispatcher" is now true of both paths.SYSTEM_CTXdocblock now says what the read is for: existence, theactiveswitch, name and secret posture.Measured (the dispatch's mechanism assumptions)
active:Field.boolean,required: true,defaultValue: true(sys-webhook.object.ts:175–181). The guard reads withengine.findOne(subscriptionsObject, { where: { id } }, { context: { isSystem: true } }), with no projection, so the column is in the row. Booleans come back as JS booleans on every driver: the SQL driver converts SQLite/MySQL0/1to booleans informatOutputand leaves null as null, and Postgres is native.Null or absent
active: refused. I decided this from the dispatcher's predicate, not from the declared default. The dispatch suggested the declared default, so this is a deviation, stated here. The automatic dispatcher loads{ where: { active: true } }(auto-enqueuer.ts:393) and so skips a null or absent row on every driver. If the guard allowed that row, the field's sentence would be false again for that row. The declared default andrequiredare why the engine never writes such a row (it fillstrueon insert), so the two readings differ only on rows written outside the engine. There, refusing is the fail-closed side and matches what Setup shows. The test issubscription.active !== true.webhooksslot'shandleRedeliverand the self-hosted mount. The guard's string reaches the door as409DELIVERY_NOT_ELIGIBLEwith the string as the message, throughassertRedeliverAllowed(service-messaging/src/http-outbox.ts). The member and the mount answer byte-equal (status, headers, body), as the parity pin asserts.IWebhookService.handleRedelivertext, "DELIVERY_NOT_ELIGIBLE(the row is not finished, or the producer's veto refused it)", covers the third case: verified, no change. One sentence in the same file's header now under-describes the veto: "the messaging service's veto over replaying a webhook row whose subscription or signing secret is gone". It still holds for the two cases it names, but it lists two of the three.packages/specis untouched, as the claim directs; the report names the sentence for the seat to route.webhook-system-context.pin.test.tsrow had noactivekey, and the new rule refuses that. The fixture gainsactive: true, as every engine-written row carries it. The pin still asserts what it is for, the read'sisSystemcontext.Pins
webhook-drop-durable-record.test.ts, new describe):inactiveand the remedy;activeare refused;webhook-redeliver-member.test.ts):409DELIVERY_NOT_ELIGIBLEon both faces, with the reason, and the row staysdead;200,pending);codeand the HTTPstatus. On the message they assert the named subject (webhook 'paused_hook'), the state wordinactive, and the remedy fragmentre-activate the webhook, then redeliver. The card names those as the reason's contract, so the whole sentence is not pinned.Ablation (every negative pin)
The ablation was done on the committed fix (
4c3ef9cef), throughscripts/ablation-replace.mjs, wrapped with atraprestore. The anchorif (subscription.active !== true) {becameif (false) {.4a8096099b21to684c106c56e3.git diff HEADempty, and the tree clean../redeliver-guard.js,./webhook-outbox-plugin.js), not through a packageexports. So nodist/is in the loop, and the dist preflight does not apply.Verification
All runs below are at
4c3ef9cef.os-verify-locktimings are shared-box figures.pnpm --filter '@objectstack/plugin-webhooks^...' build(the dependency closure): VERDICT command-exit 0.pnpm --filter @objectstack/plugin-webhooks build:check-dts-emitted4/4.pnpm --filter @objectstack/plugin-webhooks typecheck:tsc --noEmitand the scripts program clean, andcheck:test-typecheck: OK, 0 errors.pnpm --filter @objectstack/plugin-webhooks test: 17 files, 191 tests passed.node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstackgives 65 commands, the dispatch's 54 plus 11 that the changeset brings. All 65 exit 0, and--ranreconciles them: "65 derived, 65 run, 0 NOT-MEASURED, 0 UNRUN", a derived zero, because every line records its exit code.check-plugin-teardown-shape.mjs --self-testneeded its pinned fixture commit in this shallow clone, fetched at depth 1.check:i18nandcheck:dual-build-cjs-loadsneeded built output, so I built the whole workspace (turbo run build --filter='!@objectstack/docs', 73/73 tasks). After those steps, all three re-ran at exit 0.dist/was re-run on the fully built tree:check:dts-closure(73 packages, 172/172),check:published-files,check:lean-entry-closureandcheck:sourcemap-no-sources-content..tsfiles: 0 errors and 0 warnings over 4 files, a count read from--format json. The config ignores none of the four (ESLint.isPathIgnoredis false for each), and it enables no type-aware linting (noparserOptions.project), so this diff cannot move any untouched file's verdict. The repo-widepnpm lintis CI's.origin/main. The 3 commits since base (efcbac73c, trigger-api, mcp and the objectui pin) touch none of these files, andgit merge-treeagainstefcbac73cis clean. CI's merge ref and the queue test the joint tree.The public surface is unchanged in bytes: no export, type, code or status moves. So the import sites owe no tests of their own.
Acceptance notes
IWebhookServiceheader sentence (packages/spec/src/contracts/webhook-service.ts): see measured item 3. Left for the seat, as the claim directs.WebhookOutboxPlugin.installRedeliverGuard's docblock still describes the veto by its signing half ("refuses a webhook row whose signing configuration is no longer available"). It is still true, just narrower than the veto now is. That file is outside the claim's file surface, so it is left alone.content/docs/automation/webhooks.mdxlists409 DELIVERY_NOT_ELIGIBLEwithout naming the veto's cases, and itsactiverow now reads true of both paths. No sentence there is false.Disclosure: the card is labelled
security. This body describes behaviour only, with no reproduction recipe.Generated by Claude Code