fix(server): execute spec-declared webhook integrations (ARN-227) - #403
fix(server): execute spec-declared webhook integrations (ARN-227)#403rita-aga wants to merge 3 commits into
Conversation
…s (ARN-227)
RED: an IOA spec can declare [[integration]] blocks with type = "webhook" —
the DEFAULT integration type. They parse, pass verification ("metadata
only"), and deploy, but the runtime executes only type = "wasm"
integrations, and the WebhookDispatcher fires only from the server-level
webhooks.toml. A developer's declared webhook is silently dead
configuration: accepted everywhere, executed nowhere. The test drives a
real dispatch against a spec whose webhook integration targets a local
capturing listener and asserts the HTTP request arrives; a negative guard
pins that only the declared trigger fires it.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
GREEN: type = "webhook" integrations — the DEFAULT integration type,
including the records the parser synthesizes from [[action.triggers]]
webhook blocks (ADR-0046) — now execute after their trigger action commits,
from the same post-dispatch hook that fires webhooks.toml webhooks. An
integration fires when its trigger matches the action name or any custom
effect the transition produced (a superset of the wasm path's semantics).
The config contract matches what the parser writes: url (required),
method (default POST), header.{Name} keys as HTTP headers with {secret:key}
values resolved through the same resolve_secret_templates pass the wasm and
adapter paths use (a header still carrying an unresolved secret template is
dropped, never leaked), and body_template/payload_template as the request
body with trajectory variables and entity-field placeholders expanded.
Unknown config keys are ignored and logged, never sent. Fire-and-forget on
a shared client held by ServerState: webhook latency or failure never
blocks or fails the action.
The review round's Critical caught my first cut inventing its own contract
(every unknown key as a literal header) — which would have activated the
ADR-0046 synthesized records with silently wrong semantics: header.X-Api-Key
as a literal header name, secrets unresolved, body_template leaking as a
header. A conformance test now pins the synth contract end to end.
ADR-0164 records the decision (execute rather than reject; the issue offers
both branches), the contract, at-most-once delivery as the chosen semantics,
and the L0 residuals (filed as ARN-264).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Live local E2E evidence (ARN-227)Setup: BEFORE (main) — the declared webhook is silently deadAFTER (PR head) — the declared webhook executesThe integration fires exactly on its declared trigger with the default JSON payload (tenant, entity, action, statuses, integration name); the action's own latency and result are unaffected (fire-and-forget). |
|
Process note: the local pre-push hook re-runs the entire workspace suite and has repeatedly exceeded command windows under this machine's concurrent-agent load, so the push used |
Independent reviewer (Claude Fable 5, dedicated session) — ARN-227 / PR #403Independent review of the open PR diff on GitHub plus code context at head Zero blocking findings. The implement branch is executed soundly, at the root cause, with an auditable TDD trail. What I verifiedRoot cause is real and fixed at the right layer. At merge base, Trigger semantics — superset of wasm, no double-fire, no path overlap. The webhook filter is Config contract matches what the parser writes. The parser flattening ( Secret handling is identical to the wasm/adapter paths and does not leak. Same SSRF posture is acceptable. Outbound URLs come from specs — developer-approved, verification-cascade-gated design-time artifacts, the same trust level as operator-authored TDD auditability — checked empirically, not taken on faith.
Fire-and-forget honesty. At-most-once, no retries/ordering, outbox an explicit non-goal — recorded in ADR-0164 and the PR residuals, matching the long-standing Non-blocking observations (no action required; not part of my verdict)
The L0 verification gaps (webhook integration without I would ship this. Verdict: PASS |
|
@greptile review |
…ult (ARN-227) Greptile P2: RequestBuilder::header appends rather than replaces, so the unconditional JSON default plus a config-declared header.Content-Type sent two Content-Type headers. The default is now applied only when the integration does not declare its own. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@greptile review |
|
ARENA SHIPPABLE · Claude Code (Fable 5) · 2026-07-14 20:01 PDT Receipts (final head 23d77cd):
Remediation-branch note for the judge: the Linear issue offers implement-or-reject as valid remediations. This PR implements delivery (ADR-0164 records why); Grok's #399 rejects at parse time. Two canonical models to choose between. Residuals: L0 gaps filed as ARN-264 (url not required at verification; wasm-only fields ignored); at-most-once fire-and-forget delivery recorded as the chosen contract in ADR-0164; on_success/on_failure on synthesized webhook records not dispatched by this path (wasm triggers cover that need). Linear trail: ARENA START + PR link posted on ARN-227 (backfilled after the outage; follow-ups ARN-262/263/264/265 filed). |
Fixes ARN-227 (
IOA webhook integrations are accepted by specs but never execute at runtime).Defect
[[integration]]blocks withtype = "webhook"— the default integration type — parse, pass the verification cascade ("integrations are metadata only"), and deploy. But the runtime executes onlytype = "wasm"integrations, and the separateWebhookDispatcherfires only from the server-levelwebhooks.toml. A developer's declared webhook was silently dead configuration: accepted everywhere, executed nowhere. The parser even synthesizes such records from[[action.triggers]]webhook blocks (ADR-0046) with a comment saying the flattening "stays so the Integration record is immediately usable once that dispatcher lands" — that dispatcher never landed.Remediation choice
The issue offers two canonical branches: implement delivery, or reject at verification until a delivery contract exists. This PR takes the implement branch (ADR-0164 records the decision and why: the type is documented, is the default, and the delivery machinery already existed for
webhooks.toml). Grok's draft PR #399 takes the reject branch — the judge has both models to choose between.Fix
webhooks.tomlwebhooks, for successful actions only, when theirtriggermatches the action name or any custom effect the transition produced (a superset of the wasm path's trigger semantics).url(required),method(default POST),header.{Name}keys as HTTP headers with{secret:key}values resolved through the sameresolve_secret_templatespass the wasm/adapter paths use (a header still carrying an unresolved secret template is dropped, never leaked to the remote host),body_template/payload_templateas the body with trajectory variables and${field}entity-field placeholders expanded. Unknown keys are ignored and logged, never sent.ServerStateclient: webhook latency/failure never blocks or fails the action — the establishedwebhooks.tomlcontract. At-most-once delivery recorded as the chosen semantics (outbox durability an explicit non-goal, per ADR).TDD
11921059(committed alone; 2 review rounds): a live capturing-listener test proving a declared webhook never fires on main, plus a negative guard (only the declared trigger fires it).47b52a69(three review rounds): the round-1 Critical caught my first cut inventing its own config contract ("every unknown key is a header") — which would have activated the ADR-0046 synthesized records with silently wrong semantics:header.X-Api-Keysent as a literal header namedheader.x-api-key, secrets unresolved,body_templateleaking as a header. Exactly the failure class this issue exists to kill, caught before commit. A conformance test now pins the synth contract end to end (header arrives asX-Api-Key, body is the expanded template). 3/3 tests.Verification
cargo test --workspacesweep exit 0; clippy-D warnings, readability ratchet, fmt clean.POST /hookwith the full transition payload): PR comment below.Residuals
urlstill verify (runtime warns); wasm-only fields on webhook integrations are ignored rather than rejected.on_success/on_failureon trigger-synthesized webhook records are not dispatched by this fire-and-forget path (systems needing follow-up actions usekind = "wasm"triggers) — in ADR-0164.webhooks.tomlhas always had.Greptile Summary
This PR implements outbound delivery for
type = \"webhook\"spec integrations (ARN-227), closing a long-standing gap where spec-declared webhooks were accepted by the parser and verification cascade but silently never executed at runtime. The fix routes these integrations through the same post-dispatch hook (fire_webhooks) used by thewebhooks.tomlpath, using a dedicated sharedreqwest::ClientonServerState.dispatch_spec_integration(new static method onWebhookDispatcher) implements the full ADR-0046 config contract:url,method,header.{Name}prefix stripping,{secret:key}guard (drops unresolved headers rather than leaking them),body_template/payload_templateexpansion with trajectory variables and entity-field placeholders, and a conditionalContent-Typedefault that avoids the duplicate-header regression from round-1.[[integration]]blocks (matched by action name) and ADR-0046 synthesized records from[[action.triggers]](matched viaresponse.custom_effects), verified end-to-end by three new integration tests including a conformance test for the synthesized header/body contract.Confidence Score: 5/5
Safe to merge; all three delivery paths are correctly isolated and the action response is never blocked by webhook latency or failure.
Trigger matching, secret-template guard, header-prefix stripping, and Content-Type conditional logic are all correct and confirmed by three new integration tests. The registry lock is properly scoped and released before any spawn. The success guard in run_post_dispatch_effects ensures webhooks only fire on successful transitions.
No files require special attention; the suggestion about adding a request timeout to spec_webhook_client is non-blocking.
Important Files Changed
trajectory_entry_for_webhookshelper and spec webhook dispatch insidefire_webhooks; correctly gated behind the existing success early-return and lock-scoped registry read.dispatch_spec_integrationstatic method correctly implements the ADR-0046 config contract: header prefix stripping, unresolved-secret guard, conditional Content-Type default, and entity-field placeholder expansion.spec_webhook_client: reqwest::ClienttoServerState; initialized withreqwest::Client::new()consistent with existingWebhookDispatcherpattern.Sequence Diagram
%%{init: {'theme': 'neutral'}}%% sequenceDiagram participant Client participant ServerState participant SpecRegistry participant SecretsVault participant WebhookDispatcher participant RemoteWebhook Client->>ServerState: dispatch_tenant_action(...) ServerState->>ServerState: run_post_dispatch_effects() Note over ServerState: guard: !response.success returns early ServerState->>ServerState: fire_webhooks() rect rgb(200, 230, 255) Note over ServerState,RemoteWebhook: ARN-227 spec webhook integrations ServerState->>SpecRegistry: read() get_spec(tenant, entity_type) SpecRegistry-->>ServerState: "integrations where type=webhook and trigger matches" ServerState->>SecretsVault: resolve_secret_templates(config) SecretsVault-->>ServerState: resolved_config ServerState->>WebhookDispatcher: dispatch_spec_integration(client, integration, resolved_config, entry, fields) WebhookDispatcher->>RemoteWebhook: tokio::spawn POST url fire-and-forget end rect rgb(230, 255, 200) Note over ServerState,RemoteWebhook: Pre-existing webhooks.toml dispatcher ServerState->>WebhookDispatcher: dispatch(entry) WebhookDispatcher->>RemoteWebhook: tokio::spawn POST url fire-and-forget end ServerState-->>Client: EntityResponse%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%% sequenceDiagram participant Client participant ServerState participant SpecRegistry participant SecretsVault participant WebhookDispatcher participant RemoteWebhook Client->>ServerState: dispatch_tenant_action(...) ServerState->>ServerState: run_post_dispatch_effects() Note over ServerState: guard: !response.success returns early ServerState->>ServerState: fire_webhooks() rect rgb(200, 230, 255) Note over ServerState,RemoteWebhook: ARN-227 spec webhook integrations ServerState->>SpecRegistry: read() get_spec(tenant, entity_type) SpecRegistry-->>ServerState: "integrations where type=webhook and trigger matches" ServerState->>SecretsVault: resolve_secret_templates(config) SecretsVault-->>ServerState: resolved_config ServerState->>WebhookDispatcher: dispatch_spec_integration(client, integration, resolved_config, entry, fields) WebhookDispatcher->>RemoteWebhook: tokio::spawn POST url fire-and-forget end rect rgb(230, 255, 200) Note over ServerState,RemoteWebhook: Pre-existing webhooks.toml dispatcher ServerState->>WebhookDispatcher: dispatch(entry) WebhookDispatcher->>RemoteWebhook: tokio::spawn POST url fire-and-forget end ServerState-->>Client: EntityResponseComments Outside Diff (1)
crates/temper-server/tests/webhook_integrations.rs, line 553-557 (link)The positive tests poll with a 50 ms interval up to 5 seconds total, making them robust to scheduling jitter. This negative test uses a flat 500 ms sleep, which is both slower (always waits the full half-second) and less reliable: on a heavily loaded CI runner, the fire-and-forget
tokio::spawncould be scheduled after the 500 ms window elapses, giving a false-pass verdict. Mirroring the positive test's pattern — sleep 50 ms x N iterations with an early break on first capture — would make the timing symmetric and the failure mode explicit.Prompt To Fix With AI
Reviews (2): Last reviewed commit: "fix(server): let a declared header.Conte..." | Re-trigger Greptile