Skip to content

fix(plugin-webhooks): re-arm an encrypted-secret webhook when the CryptoProvider registers (#8022) - #8043

Merged
huangyiirene merged 3 commits into
mainfrom
claude/issue-8022-webhook-cache-crypto-ordering
Aug 12, 2026
Merged

fix(plugin-webhooks): re-arm an encrypted-secret webhook when the CryptoProvider registers (#8022)#8043
huangyiirene merged 3 commits into
mainfrom
claude/issue-8022-webhook-cache-crypto-ordering

Conversation

@huangyiirene

Copy link
Copy Markdown
Collaborator

Fixes#8022

What was wrong

For ~60s after every restart, a webhook whose signing_secret is encrypted — the population #7799 created — was not subscribed. A record change in that window produced no delivery and no sys_http_delivery row at all: no dead letter, no retry, no durable trace, while GET /api/v1/data/sys_webhook/ kept reading active: true. It self-healed at the next periodic refresh, so it was invisible to anyone not watching the window.

The fail-closed behaviour is unchanged

Dropping a subscription whose stored key cannot be recovered — rather than delivering it unsigned — is #7799's whole point and still holds. No unsigned delivery, no cleartext fallback, no weakened refusal. What was wrong is that a fail-closed drop outlived its own cause.

Root cause: an ordering, not a race

The filer read it off two log lines ~1.3s apart. It is stronger than that — it is deterministic. Plugins run inside kernel:ready, which runtime.start() completes (packages/core/src/kernel.ts Phase 3); the composition root calls engine.setCryptoProvider(...) only afterruntime.start() returns, unconditionally and on every host (packages/cli/src/commands/serve.ts, packages/verify/src/harness.ts). So AutoEnqueuer's first cache build always precedes the capability it needs, drops every secret-bearing row on what it could see, and nothing re-reads until the periodic refresh up to 60s later.

The shape landed

ObjectQL.onCryptoProviderChange(listener) reports the registration, fired after the provider is in place. AutoEnqueuer subscribes before its first build — the registration can land at any moment from start() onward, including mid-refresh — and rebuilds the cache when it fires.

Event-driven, not a shorter poll. Chosen over the alternatives because it fixes every host rather than one: moving the wiring earlier in serve.ts leaves apps/cloud, packages/verify, and every embedder with the same window, and a retry ladder only shrinks the hole. The engine is the sole party that knows the moment the provider arrives.

The re-arm deliberately does not join an in-flight refresh(): the build most likely running at that instant is the pre-registration one, so joining it would report success having re-armed nothing.

The channel is feature-detected, exactly as resolveSecretField already was — this plugin takes no dependency on @objectstack/objectql. An engine without it keeps today's behaviour with the periodic refresh as backstop.

The drop is no longer quiet

A subscription dropped for an unresolvable key now reports at error with the consequence and the fix in the message, carrying an ADR-0112 code/status pair (INTERNAL_ERROR/500) in its metadata — the same pair the seeder's refusal for the same cause already carried. Per AGENTS.md "Degradation log levels" it is said once per outage per webhook rather than every refresh cycle; a webhook that recovers and breaks again is loud again.

Half (b) — durability — is NOT closed here

A drop that leaves a sys_http_delivery row would need IHttpOutbox to admit a row that is durable but never sendable, and today it cannot: enqueue() only mints pending, and redeliver() resets any terminal row back to pending for a byte-for-byte replay. A row parked for a missing key has no signature, so the existing admin redeliver endpoint would send it unsigned — re-opening #7799 through a new door. That wants a design decision (a new column on the sys_http_delivery platform object, plus a redeliver refusal), not a rider on this fix. Recommending its own card; reasoning in full in the issue report.

Verification


Generated by Claude Code

…ptoProvider registers (#8022)
A webhook whose signing secret is encrypted was unsubscribed for ~60s after
every restart: a record change in that window produced no delivery and no
`sys_http_delivery` row, while the row still read `active: true`.
Not a race — an ordering. Plugins run inside `kernel:ready`, which
`runtime.start()` completes, and every host wires `setCryptoProvider` only after
`runtime.start()` returns. So `AutoEnqueuer`'s first cache build always preceded
the capability it needs, correctly dropped every secret-bearing subscription
(#7799 fail-closed), and nothing re-read until the periodic refresh.
`ObjectQL.onCryptoProviderChange(listener)` reports the registration; the
auto-enqueuer subscribes before its first build and rebuilds when it fires,
without joining the in-flight pre-registration refresh. Feature-detected, as
`resolveSecretField` already was.
The fail-closed drop itself is unchanged. It now reports at `error` with the
consequence and the fix, carries the ADR-0112 `INTERNAL_ERROR`/500 pair, and is
said once per outage rather than every refresh.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FuYKU5d8xPfHSZyASNsyia
… main too (#8022)
Every harness that can inject the CryptoProvider registration while the first
cache build is in flight also moves the secret resolution to after it, so the
naive `() => this.refresh()` passes the test as well. A case that cannot
separate the two revisions is a false green, so it is removed rather than kept
as coverage it does not provide. The guard in `rearmAfterCryptoRegistered`
stays, with the reasoning stated where it lives.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FuYKU5d8xPfHSZyASNsyia
@vercel

vercelBot commented Aug 12, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
ProjectDeploymentActionsUpdated (UTC)
objectstackIgnoredIgnoredAug 12, 2026 1:28pm

Request Review

@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 2 package(s): @objectstack/objectql, @objectstack/plugin-webhooks.

16 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:

  • content/docs/automation/webhooks.mdx(via packages/plugins/plugin-webhooks)
  • content/docs/concepts/metadata-lifecycle.mdx(via @objectstack/objectql)
  • content/docs/data-modeling/formulas.mdx(via packages/objectql)
  • content/docs/deployment/migration-from-objectql.mdx(via @objectstack/objectql)
  • content/docs/deployment/vercel.mdx(via @objectstack/objectql)
  • content/docs/kernel/contracts/data-engine.mdx(via @objectstack/objectql)
  • content/docs/kernel/runtime-services/examples.mdx(via packages/objectql)
  • content/docs/kernel/services-checklist.mdx(via @objectstack/objectql)
  • content/docs/kernel/services.mdx(via @objectstack/objectql)
  • content/docs/permissions/authentication.mdx(via @objectstack/objectql)
  • content/docs/permissions/system-context.mdx(via packages/objectql)
  • content/docs/plugins/index.mdx(via @objectstack/objectql)
  • content/docs/plugins/packages.mdx(via @objectstack/objectql, @objectstack/plugin-webhooks)
  • content/docs/protocol/kernel/index.mdx(via @objectstack/objectql)
  • content/docs/protocol/objectql/query-syntax.mdx(via packages/objectql)
  • content/docs/protocol/objectql/state-machine.mdx(via @objectstack/objectql)

1 release-owned page(s) also reference the affected code. These are read-only:

  • content/docs/releases/implementation-status.mdx(via @objectstack/objectql, @objectstack/plugin-webhooks)

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.

Advisory only. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs origin/main → pass the list as args.docs.

@github-actionsgithub-actionsBot added documentation Improvements or additions to documentation tests tooling labels Aug 12, 2026
…#8022)
The set that makes the fail-closed drop report once per outage instead of once
per refresh kept every id it ever saw. Two consequences: it grows for the life
of the process, and a webhook deactivated while broken and later reactivated
still broken has its first report suppressed as a repeat. Each refresh now
forgets ids the read no longer returns.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FuYKU5d8xPfHSZyASNsyia
@huangyiirene
huangyiirene marked this pull request as ready for review August 12, 2026 14:19
@huangyiirene
huangyiirene added this pull request to the merge queueAug 12, 2026
Merged via the queue into main with commit f46e987Aug 12, 2026
26 checks passed
@huangyiirene
huangyiirene deleted the claude/issue-8022-webhook-cache-crypto-ordering branch August 12, 2026 14:37
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/mteststooling

Projects

None yet

2 participants

@huangyiirene@claude