Skip to content

fix(plugin-webhooks): the redeliver veto refuses a delivery whose subscription is inactive - #22836

Merged
objectstack-fleet[bot] merged 1 commit into
mainfrom
claude/issue-22804-redeliver-inactive-veto
Oct 11, 2026
Merged

objectstack-fleet[bot] merged 1 commit into
mainfrom
claude/issue-22804-redeliver-inactive-veto

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

Fixes #22804
Clause-②: no

What changes

createWebhookRedeliverGuard (packages/plugins/plugin-webhooks/src/redeliver-guard.ts), the redeliver veto, now refuses a delivery whose sys_webhook subscription is inactive. This follows triage's reading on the card: an explicit replay is "the dispatcher" for active. The refusal works like the gone case. The door answers 409 DELIVERY_NOT_ELIGIBLE and leaves the row untouched. The reason names the webhook, says it is inactive, and names the remedy:

webhook 'NAME' is inactive, and an inactive webhook is skipped by the dispatcher, so this delivery is refused rather than sent to an endpoint that was switched off. Fix: re-activate the webhook, then redeliver; new events are delivered again from the moment it is active.

On the wire, the outbox puts its own prefix before the reason: Delivery row 'ID' cannot be redelivered: REASON.

  • Order. The new check runs after "gone" and before the secret checks. So an inactive webhook whose key is also unrecoverable is refused as inactive, the operator's first remedy. The gone and unrecoverable-secret refusals keep their text and their order. Ruling 5271033283 on [security] a dropped webhook subscription leaves no durable record — and the naive fix hands an operator a redeliver button that sends UNSIGNED #8069 says no operator action may send an unsigned delivery. This change only adds a refusal and weakens nothing.
  • The active field description stays as it is. "Inactive webhooks are skipped by the dispatcher" is now true of both paths.
  • The guard's docblock lists the refusal cases as 1 gone, 2 inactive, 3 unrecoverable secret, 4 lookup failed (refused by the caller). The SYSTEM_CTX docblock now says what the read is for: existence, the active switch, name and secret posture.

Measured (the dispatch's mechanism assumptions)

  1. The column. Its name is active: Field.boolean, required: true, defaultValue: true (sys-webhook.object.ts:175–181). The guard reads with engine.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/MySQL 0/1 to booleans in formatOutput and 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 and required are why the engine never writes such a row (it fills true on 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 is subscription.active !== true.
  2. The door. Measured on both faces: the webhooks slot's handleRedeliver and the self-hosted mount. The guard's string reaches the door as 409 DELIVERY_NOT_ELIGIBLE with the string as the message, through assertRedeliverAllowed (service-messaging/src/http-outbox.ts). The member and the mount answer byte-equal (status, headers, body), as the parity pin asserts.
  3. The spec docblock. The IWebhookService.handleRedeliver text, "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/spec is untouched, as the claim directs; the report names the sentence for the seat to route.
  4. The order and existing pins. No existing pin depends on the order. One existing fixture changed. The webhook-system-context.pin.test.ts row had no active key, and the new rule refuses that. The fixture gains active: true, as every engine-written row carries it. The pin still asserts what it is for, the read's isSystem context.

Pins

  • Guard (webhook-drop-durable-record.test.ts, new describe):
    • inactive is refused, naming the webhook, inactive and the remedy;
    • null and absent active are refused;
    • inactive is checked before the secret: the resolver is never called;
    • control: active, signed and unsigned, is allowed;
    • control: gone keeps its own refusal.
  • Door (webhook-redeliver-member.test.ts):
    • inactive answers 409 DELIVERY_NOT_ELIGIBLE on both faces, with the reason, and the row stays dead;
    • CONTROL: active and signed is replayed by both faces (200, pending);
    • CONTROL: gone is unchanged;
    • CONTROL: the same delivery is replayed once re-activated;
    • the inactive case is also a row in the existing every-refusal table and in the member/mount byte-parity table.
  • Refusal pins assert the envelope's code and the HTTP status. On the message they assert the named subject (webhook 'paused_hook'), the state word inactive, and the remedy fragment re-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), through scripts/ablation-replace.mjs, wrapped with a trap restore. The anchor if (subscription.active !== true) { became if (false) {.

  • On disk: anchor 1 to 0, replacement 0 to 1, blob 4a8096099b21 to 684c106c56e3.
  • Result: 2 of 3 files and 7 of 35 tests red. These are the five pins that assert a refusal (the three guard refusal pins, the door's inactive pin and the re-activated control's first leg) plus the two tables that now carry the inactive row.
  • The controls stayed green: active and signed replayed, gone unchanged, and the guard's active and gone controls.
  • Restore was proven: blob equal to the HEAD blob, git diff HEAD empty, and the tree clean.
  • The tests import the subject by relative source path (./redeliver-guard.js, ./webhook-outbox-plugin.js), not through a package exports. So no dist/ is in the loop, and the dist preflight does not apply.

Verification

All runs below are at 4c3ef9cef. os-verify-lock timings 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-emitted 4/4.
  • pnpm --filter @objectstack/plugin-webhooks typecheck: tsc --noEmit and the scripts program clean, and check:test-typecheck: OK, 0 errors.
  • pnpm --filter @objectstack/plugin-webhooks test: 17 files, 191 tests passed.
  • Gates: I re-derived the gate list on this change: node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack gives 65 commands, the dispatch's 54 plus 11 that the changeset brings. All 65 exit 0, and --ran reconciles them: "65 derived, 65 run, 0 NOT-MEASURED, 0 UNRUN", a derived zero, because every line records its exit code.
    • Three first runs answered exit 3, PREREQUISITE NOT MET, and none of them counted as a measurement. check-plugin-teardown-shape.mjs --self-test needed its pinned fixture commit in this shallow clone, fetched at depth 1. check:i18n and check:dual-build-cjs-loads needed 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.
    • Every gate that reads dist/ was re-run on the fully built tree: check:dts-closure (73 packages, 172/172), check:published-files, check:lean-entry-closure and check:sourcemap-no-sources-content.
    • ESLint, narrowed to the 4 changed .ts files: 0 errors and 0 warnings over 4 files, a count read from --format json. The config ignores none of the four (ESLint.isPathIgnored is false for each), and it enables no type-aware linting (no parserOptions.project), so this diff cannot move any untouched file's verdict. The repo-wide pnpm lint is CI's.
    • The branch was not merged with origin/main. The 3 commits since base (efcbac73c, trigger-api, mcp and the objectui pin) touch none of these files, and git merge-tree against efcbac73c is 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

  • IWebhookService header 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.mdx lists 409 DELIVERY_NOT_ELIGIBLE without naming the veto's cases, and its active row 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

…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
@github-actions github-actions Bot added size/m documentation Improvements or additions to documentation tests tooling labels Oct 11, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

1 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
  • 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 — 4 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 efcbac73cc58510278b970949ac3b65cb9712815 → packageMentionDocs.

Which tree this was computed on

This run read content/docs from 499d445f9939c205ed1ecf51880c50fde0c53d0d — the merge of head 4c3ef9cef48085a1baaa97a15fbe50c8e70c5b15 into base efcbac73cc58510278b970949ac3b65cb9712815, 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 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

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

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

Contract review

Served-tier: CONTRACT_REVIEW_TIER
Head-sha: 4c3ef9cef48085a1baaa97a15fbe50c8e70c5b15
Local-runs: none

Inputs: card #22804 (body and its four comments: triage 6107327152, claim 6108491444, os-dev-report 6109045625, ACCEPT 6109072386), PR #22836 (body, the 5-file list, the net diff against main at merge base b7cd1af9d), and the check-runs on the head. Read-only: the diff and the tree at the head and at origin/main were read with git show / git grep; nothing was built, run or re-run. The card is labelled security, so this record names classes and positions only.

① Derived judgments

  1. Accept set of the redeliver door narrows — right. POST /api/v1/webhooks/redeliver and the webhooks slot's handleRedeliver now answer 409 DELIVERY_NOT_ELIGIBLE for a finished delivery whose sys_webhook row has active !== true, where before the veto let it through; the row is left as it was. This is the triage reading of record on the card (an explicit replay is "the dispatcher" for active), it adds a refusal to an existing ledger code, and it sends nothing new: ruling 5271033283's non-negotiable half (no operator action sends an unsigned delivery) is untouched. Position: redeliver-guard.ts:127 at the head; door pins on both faces in webhook-redeliver-member.test.ts (the new describe, plus a row in the every-refusal table and the member/mount byte-parity table).
  2. A null or absent active is refused — right, and it is the consistent reading, not merely the fail-closed one. The predicate is active !== true, which is exactly what automatic dispatch uses: auto-enqueuer.ts:393 loads { where: { active: true } }, so such a row is never dispatched on any driver, and a replay that sent it would make the field's own sentence false for that row. Such a row is also unreachable through the engine: the field is required: true, defaultValue: true (sys-webhook.object.ts:175–181), the engine's insert resolves defaultValue (objectql engine.ts, the [#4633] note), and the declared-webhook bootstrap always writes a boolean (bootstrap-declared-webhooks.ts:349, active: wh.isActive !== false). The guard's read has no projection ({ where: { id } } only), so the column is present on every row it sees. Noted, not a conflict: core's isRowActive (packages/core/src/security/row-active.ts) reads an absent active as ACTIVE for the RBAC catalogues; its reason (a mass revocation on deployed rows) does not transfer here, because the dispatcher already skips these rows and the refusal names the remedy.
  3. Check order (gone → inactive → secret) — right. The gone and unrecoverable-secret refusals keep their text and their order; the inactive check sits between them, so the secret resolver is never called for an inactive row (pinned: resolver call count 0). The only observable change inside the already-refused set is the reason text for a row that is both inactive and key-less, which now names the first remedy; status and code are unchanged, so no accept set moves there.
  4. Public TypeScript surface unchanged — right. createWebhookRedeliverGuard is not re-exported from @objectstack/plugin-webhooks's . or ./schema entry (src/index.ts names it nowhere; the package exports map has only those two subpaths), and the function's signature and return type are unchanged. The changeset's "No code, status, field or export changes" is accurate: the published change is behavioural, at the door.
  5. The refusal string — right. A runtime string, it carries no tracker number (the [#22804] markers are in comments and docblocks only), names the webhook, the state and the remedy, and reaches the wire as the 409 message through the outbox's existing assertRedeliverAllowed seam, which the diff does not touch. The pins assert code, status, subject, the state word and the remedy fragment rather than the whole sentence; that matches the card's statement of what the reason owes.
  6. active field description untouched — right. "Inactive webhooks are skipped by the dispatcher" is now true of both paths, which is what triage directed. content/docs/automation/webhooks.mdx:105 and :562–:568 state the field and the door's codes without enumerating the veto's cases; nothing there reads false after this diff (prose surface: the ACCEPT's sentence-level check stands; no finding against it here).
  7. Existing pin re-aimed only where the new rule required it — right. webhook-system-context.pin.test.ts gains active: true on its one fixture row; the pin still asserts what it is for (findOne under isSystem). The subscriptionRow helper in webhook-drop-durable-record.test.ts already defaulted active: true at the head, so no #8069 pin changed meaning silently.
  8. Driver presentation of active (the ACCEPT's one question for this tier) — answered: every driver hands this check a JS boolean, or null. At origin/main, the SQL driver's formatOutput runs a [#11782] pass that presents every registered scalar boolean / toggle column (booleanFields[object], filled at schema sync) as Boolean(v) on SQLite and MySQL, leaving null/undefined alone; findOne returns through formatOutput; Postgres stores a real boolean that node-pg parses; the memory driver round-trips the engine-written value; MongoDB is native. A null therefore reaches the guard as null and is refused as inactive, which item 2 shows is the dispatcher's own answer for that row. Residual class, named and not a defect: the coercion is keyed to the schema-sync registry, so a table whose boolean column were missing from it would fail CLOSED here (a false "inactive" refusal of an active webhook on SQLite/MySQL, visible to the operator at the door), never open — a refusal class, not an egress class.
  9. Check-runs on the head — all completed; latest per name is success or skipped; none failed or was cancelled. The two still running at brief time, Test Core (3/6) and the aggregate Test Core, both completed success. Check Changeset, Governed Surface Queue Guard, Lint & Repo Gates, all four Type Check jobs and the six Test Core shards are success. No governed surface is in the file list; head repo equals base repo.

② Semver level

  • .changeset/22804-redeliver-inactive-veto.md: @objectstack/plugin-webhooks patch — right. The card is a bug card (class (b): implementation contradicting declared text), and a bug fix in a released package takes patch. The body carries FROM → TO, the operator's one-line fix, and the statement that the gone/secret refusals are unchanged; each sentence matches the diff. Check Changeset is success.
  • Clause-②: no, no direction arm — right, on the PR body line 2 and in the changeset body. The question the line answers is widening of a published accept set or public surface; this diff widens neither (no new error code — DELIVERY_NOT_ELIGIBLE is already in the ledger — no new export, no new accepted input). It narrows a measured accept set back to declared contract text, and the rule for that shape is no, in lane, owing one CONTRACT_REVIEW_TIER review before the queue — this record. The text it is pulled back to: sys-webhook.object.ts:179 ("Inactive webhooks are skipped by the dispatcher") under triage's reading that an explicit replay is the dispatcher. The (narrowing) arm is for a BREAKING narrowing of a published contract; the published door text, IWebhookService.handleRedeliver (packages/spec/src/contracts/webhook-service.ts:56–58), already names "the producer's veto refused it" as a 409 DELIVERY_NOT_ELIGIBLE cause without closing the veto's case set, so no published contract is contradicted by the third case. The one sentence that could be read the other way is the same file's header rationale (:22–23, "whose subscription or signing secret is gone"); it is why-prose, not the member's contract, and it stays true for the two cases it names — so it does not move the level. Triage's Clause-②: yes suggestion was a conflation of "narrows" with "widens" and the claim was right to decline it.

③ Boundary flags

Dev deviations (five, from os-dev-report 6109045625), each answered:

  1. Null/absent active refused rather than defaulted true — accepted; see ① items 2 and 8. The decisive evidence is the dispatcher's own predicate, which the dispatch's "declared default" reading would have contradicted for that row.
  2. Branch not merged with origin/main before the PR — no bearing; the three intervening commits touch none of the five files, and every check on the head ran on CI's merge ref and is green.
  3. Shallow fetch of a pinned fixture commit into the shared object store — process note, no contract bearing.
  4. Commit/PR attribution trailer form — outside this record's three sections; AGENTS.md governs that spelling, and it changes nothing published.
  5. Door pins assert subject, state word and remedy fragment, not the full sentence — accepted; the card and triage name those three facts as the reason's contract, and the status and code are pinned in full.

open_questions: none declared. Out-of-scope findings (two), answered:

  • packages/spec/src/contracts/webhook-service.ts:22–23 now names two of the veto's three cases — incomplete, not false; not a blocker for this head. The member's own contract text covers the new case. The ACCEPT's carrier ("the next PR that edits that file") is not a carrier; escalated to the owning seat: name one — a one-line packages/spec docblock follow-up in the spec lane, which, touching a published contract surface, takes its own record. ⛔ Not folded into this PR: the claim excluded packages/spec and the dev correctly left it untouched.
  • webhook-outbox-plugin.ts:415–418 (installRedeliverGuard docblock, "whose signing configuration is no longer available") and the :438 log string — verified present at origin/main; still true, narrower than the veto now is; acceptance-note class, nothing owed.

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: claude/issue-22804-redeliver-inactive-veto
Reviewed-by: session_013LbZ9MhPp1iriZEtgJqioA

VERDICT: PASS

@objectstack-fleet
objectstack-fleet Bot marked this pull request as ready for review October 11, 2026 12:56
@objectstack-fleet
objectstack-fleet Bot enabled auto-merge October 11, 2026 12:56
@objectstack-fleet
objectstack-fleet Bot added this pull request to the merge queue Oct 11, 2026
Merged via the queue into main with commit bb0db13 Oct 11, 2026
43 checks passed
@objectstack-fleet
objectstack-fleet Bot deleted the claude/issue-22804-redeliver-inactive-veto branch October 11, 2026 13:15
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/m tests tooling

Projects

None yet

2 participants