fix(admin): abort hung webhook drain fetches

[SebTardif]

What Problem This Solves

Fixes an issue where operators who save a webhook URL that never responds would stall the admin Worker after an otherwise successful mutation. Every successful POST /admin/* schedules waitUntil(drainWebhooks), which POSTs new change-log entries to each enabled subscriber. That fetch had no AbortSignal, so one hung peer kept the isolate busy until the Worker waitUntil budget expired.

Why This Change Was Made

Each subscriber POST now uses AbortSignal.timeout (2 seconds by default). Delivery stays notify-only: a timeout records last_status=timeout and still advances the cursor, same as HTTP and unreachable failures. A hung URL cannot pin waitUntil or block later events.

User Impact

A stalled webhook no longer pins the post-mutation drain. The hung hook shows timeout on the webhook list. Later change-log entries still move.

Evidence

Live Node v26.7.0 against a TCP server that accepts the connection and never writes an HTTP response:

hanging subscriber http://127.0.0.1:55398/hook
error.name TimeoutError 204ms
last_status timeout
cursor after timeout 11 (advanced, notify-only)

drainWebhooks in the Cloudflare Workers isolate then recorded last_status=timeout and the next drain had nothing left to send to that URL, so the cursor advanced.

Real behavior proof

• Behavior or issue addressed: Hung webhook drain fetch after POST /admin/* blocked waitUntil. Timeouts keep the shipped notify-only cursor contract.

• Real environment tested: macOS Darwin 25.6.0 arm64, Node v26.7.0, worktree /tmp/oc-impl-krillswitch-webhook on fix/webhook-drain-timeout. Cloudflare Workers isolate via drainWebhooks (the same function waitUntil calls).

• Exact steps or command run after this patch: Started a TCP listener that never writes HTTP and called fetch with AbortSignal.timeout(200). Then ran drainWebhooks in the Worker isolate against a subscriber that waits for that abort, and read last_status plus a second drain.

• Evidence after fix: terminal output from the patched worktree:

  hanging subscriber http://127.0.0.1:55398/hook
  error.name TimeoutError 204ms
  last_status timeout
  cursor after timeout 11 (advanced, notify-only)

• Observed result after fix: The signaled POST returned TimeoutError at about 200ms. The Worker drain recorded timeout and advanced the cursor, so later events are not stuck behind the hung URL.

• What was not tested: A live production isolate on switch.openclaw.ai against a public webhook host.

Origin

The unbounded fetch landed in feeefc40 on 2026-07-29 as part of #46. The React SDK already documents AbortSignal.timeout in docs/react-sdk.md (#43).

[clawsweeper]

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

[clawsweeper]

Codex review: blocked before merge. Reviewed September 3, 2026, 9:39 AM ET / 13:39 UTC.

ClawSweeper review

What this changes

Bounds each background admin-webhook POST to two seconds, records timeouts, advances the notify-only cursor, and adds a hanging-subscriber regression test.

Merge readiness

⛔ Blocked before merge - 6 items remain

Keep open: the hang fix has credible real-behavior proof, but the newly fixed two-second cutoff irreversibly skips slow subscriber deliveries and needs an explicit maintainer delivery-policy decision before merge.

Priority: P1
Reviewed head: e12a356298dbe75e3b4fa4abbdebc3da63091f43
Owner decision: Required. See Decision needed.

Review scores

Measure	Result	What it means
Overall readiness	🦐 gold shrimp (3/6)	The patch is focused and has strong hang-path evidence, but its irreversible deadline requires a maintainer-approved delivery contract.
Proof confidence	🦞 diamond lobster (5/6)	Sufficient (terminal): The changed production drain passes an abort signal to its subscriber POST; the supplied terminal trace exercises a real hung TCP response and reports the Worker-isolate drain recording timeout and advancing the cursor after the fix.
Patch quality	🦐 gold shrimp (3/6)	2 actionable review findings remain.

Verification

Check	Result	Evidence
Real behavior	Verified	Sufficient (terminal): The changed production drain passes an abort signal to its subscriber POST; the supplied terminal trace exercises a real hung TCP response and reports the Worker-isolate drain recording timeout and advancing the cursor after the fix.
Evidence reviewed	6 items	Introduced cutoff behavior: The PR-introduced POST supplies a fixed two-second abort signal; its timeout handler then advances the cursor and stops the current batch. Established delivery contract: The shipped schema describes webhooks as an outbox where every change-log entry is POSTed and failures advance a notify-only cursor; it does not define a response-time SLA. Original feature provenance: The original webhook feature commit explicitly chose cursor advancement on failure so dead URLs cannot wedge the queue, establishing the existing at-most-once direction but not a two-second cutoff.
Findings	2 actionable findings	[P1] Obtain approval for the two-second delivery cutoff [P3] Document the new timeout behavior in Unreleased
Security	None	None.

How this fits together

The admin API appends successful mutations to an audit change log, then schedules a background webhook drain through the Worker execution context. The drain POSTs each entry to enabled operator-configured subscriber URLs and stores a cursor and delivery status.

flowchart LR
  A[Admin mutation] --> B[Change log]
  B --> C[Background webhook drain]
  C --> D[Subscriber POST]
  D --> E{Response before cutoff?}
  E -->|Yes| F[Advance cursor and status]
  E -->|No| G[Timeout status and cursor advance]
  F --> H[Webhook list]
  G --> H[Webhook list]

Loading

Decision needed

Question	Recommendation
Should admin webhooks adopt a documented two-second at-most-once delivery cutoff that permanently advances the cursor after timeout?	Approve bounded at-most-once delivery: Adopt a documented timeout SLA, retain cursor advancement on timeout, and describe the behavior in the Unreleased changelog.

Why: The existing notify-only contract resolves retries after failure but does not establish an acceptable subscriber response deadline; choosing one changes what existing webhook consumers can receive.

Before merge

• Obtain approval for the two-second delivery cutoff (P1) - The introduced abort at this line makes a normally slow subscriber fail after two seconds; the timeout path then advances its cursor, so that batch cannot be retried. The existing notify-only contract describes failure handling but does not define a two-second response SLA, while the changelog promises each entry is sent. This is the unchanged blocker from the previous review cycle.
• Document the new timeout behavior in Unreleased (P3) - This user-visible webhook-delivery change leaves CHANGELOG.md's Unreleased section empty, contrary to the repository's documented change discipline.
• Resolve merge risk (P1) - A subscriber that receives the request but takes longer than two seconds to reply is now treated as timed out and its cursor is advanced, so that change-log entry cannot be retried.
• Resolve merge risk (P2) - The empty Unreleased changelog leaves operators without release-visible notice of the new delivery cutoff and timeout status.
• Complete next step (P2) - Obtain product-owner approval for the two-second at-most-once delivery cutoff and add the required Unreleased changelog entry before merge.
• Resolve maintainer decision - Resolve the maintainer decision shown above before merge.

Findings

• [P1] Obtain approval for the two-second delivery cutoff — apps/api/src/admin/webhooks.ts:181-191
• [P3] Document the new timeout behavior in Unreleased — apps/api/src/admin/webhooks.ts:7-8

Agent review details

Security

None.

Review metrics

Metric	Value	Why it matters
Patch size	2 files; production +29/-7, tests +76	A small production change is paired with focused hanging-subscriber coverage, while the delivery-policy change remains the material merge question.

Merge-risk options

Maintainer options:

1. Approve and document bounded at-most-once delivery (recommended)
   Accept the two-second cutoff only after a product owner confirms the SLA and the branch records its operator-visible delivery consequences.
2. Pause for a different delivery contract
   Keep this PR unmerged if slow subscribers must remain eligible for delivery beyond two seconds, because timeout retry semantics require a separate duplicate-delivery decision.

Technical review

Best possible solution:

Adopt and document a maintainer-approved bounded at-most-once webhook-delivery SLA, including the chosen deadline and timeout outcome, then record the user-visible change in Unreleased.

Do we have a high-confidence way to reproduce the issue?

Yes. Current main visibly performs the subscriber POST without an abort signal, and the supplied real TCP-hang trace shows the unbounded fetch remains pending while the patched path returns on timeout.

Is this the best way to solve the issue?

Unclear. Bounding a hung background fetch is narrow and maintainable, but the fixed two-second irreversible cutoff is a delivery-policy choice that needs explicit approval before it is the best solution.

Full review comments:

• [P1] Obtain approval for the two-second delivery cutoff — apps/api/src/admin/webhooks.ts:181-191
  The introduced abort at this line makes a normally slow subscriber fail after two seconds; the timeout path then advances its cursor, so that batch cannot be retried. The existing notify-only contract describes failure handling but does not define a two-second response SLA, while the changelog promises each entry is sent. This is the unchanged blocker from the previous review cycle.
  Confidence: 0.95
• [P3] Document the new timeout behavior in Unreleased — apps/api/src/admin/webhooks.ts:7-8
  This user-visible webhook-delivery change leaves CHANGELOG.md's Unreleased section empty, contrary to the repository's documented change discipline.
  Confidence: 0.98

Overall correctness: patch is incorrect
Overall confidence: 0.93

AGENTS.md: found, but no applicable review policy affected this item.

Codex review notes: model internal, reasoning high; reviewed against 59f724c8d5aa.

Labels

Label justifications:

• P1: The patch changes whether existing webhook subscribers can receive audit events when responses exceed a new fixed deadline.
• merge-risk: 🚨 compatibility: Existing webhook integrations gain an undocumented two-second response SLA that can make previously tolerated slow responses terminal.
• merge-risk: 🚨 message-delivery: A timeout now advances the per-webhook cursor, preventing a later drain from resending that entry.
• rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦞 diamond lobster and patch quality is 🦐 gold shrimp.
• status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Sufficient (terminal): The changed production drain passes an abort signal to its subscriber POST; the supplied terminal trace exercises a real hung TCP response and reports the Worker-isolate drain recording timeout and advancing the cursor after the fix.
• proof: sufficient: Contributor real behavior proof is sufficient. The changed production drain passes an abort signal to its subscriber POST; the supplied terminal trace exercises a real hung TCP response and reports the Worker-isolate drain recording timeout and advancing the cursor after the fix.

Evidence

What I checked:

• Introduced cutoff behavior: The PR-introduced POST supplies a fixed two-second abort signal; its timeout handler then advances the cursor and stops the current batch. (apps/api/src/admin/webhooks.ts:181, e12a356298db)
• Established delivery contract: The shipped schema describes webhooks as an outbox where every change-log entry is POSTed and failures advance a notify-only cursor; it does not define a response-time SLA. (apps/api/src/db/schema.ts:201, 59f724c8d5aa)
• Original feature provenance: The original webhook feature commit explicitly chose cursor advancement on failure so dead URLs cannot wedge the queue, establishing the existing at-most-once direction but not a two-second cutoff. (apps/api/src/admin/webhooks.ts:117, feeefc406d6b)
• Current-main check: Current main still calls the subscriber fetch without an abort signal, so it has not already adopted this proposed timeout behavior. (apps/api/src/admin/webhooks.ts:171, 59f724c8d5aa)
• Repository change discipline: The target repository requires user-visible changes to be recorded in CHANGELOG.md; its Unreleased section is empty while this branch changes observable webhook delivery behavior. (docs/development.md:52, e12a356298db)
• Real behavior proof: The PR body records a real TCP listener that accepted a connection without replying, observed TimeoutError at about 204 ms, and reports the Worker drain recorded timeout and advanced its cursor. (e12a356298db)

Likely related people:

• Vincent Koc: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

• Obtain explicit approval for the bounded at-most-once webhook-delivery policy.
• Add an Unreleased changelog entry describing the timeout and cursor outcome.

Rating scale

Score	Internal tier	Crab rank	Meaning
6/6	S	🦀 challenger crab	Exceptional readiness
5/6	A	🦞 diamond lobster	Very strong readiness
4/6	B	🐚 platinum hermit	Good normal PR; ordinary maintainer review
3/6	C	🦐 gold shrimp	Useful, but confidence is limited
2/6	D	🦪 silver shellfish	Proof or implementation needs work
1/6	F	🧂 unranked krab	Not merge-ready
N/A	NA	🌊 off-meta tidepool	Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

• ClawSweeper keeps one durable marker-backed review comment per issue or PR.
• Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
• A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
• PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
• Maintainers can also comment @clawsweeper review to request a fresh review only.
• Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
• Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
• Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

History

Review history (26 earlier review cycles; latest 8 shown)

• reviewed 2026-08-26T05:25:54.924Z sha e12a356 :: found issues before merge. :: [P1] Preserve existing slow subscriber deliveries
• reviewed 2026-08-26T10:26:56.429Z sha e12a356 :: found issues before merge. :: [P1] Preserve slow webhook deliveries
• reviewed 2026-08-28T23:46:57.653Z sha e12a356 :: found issues before merge. :: [P1] Preserve slow webhook deliveries
• reviewed 2026-08-30T05:48:40.480Z sha e12a356 :: found issues before merge. :: [P1] Preserve slow webhook deliveries across the cutoff
• reviewed 2026-08-30T13:49:15.700Z sha e12a356 :: found issues before merge. :: [P1] Preserve deliveries that exceed the new cutoff
• reviewed 2026-09-01T05:57:32.492Z sha e12a356 :: found issues before merge. :: [P1] Preserve deliveries that exceed the new cutoff
• reviewed 2026-09-01T20:40:24.929Z sha e12a356 :: found issues before merge. :: [P1] Obtain approval for the two-second delivery cutoff | [P3] Add the required Unreleased changelog entry
• reviewed 2026-09-03T07:57:00.477Z sha e12a356 :: blocked before merge. :: [P1] Preserve slow subscriber deliveries | [P3] Add the required Unreleased changelog entry

[SebTardif]

@clawsweeper re-review

[clawsweeper]

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event exact_review_queue).
Result: when the review finishes, ClawSweeper will create the durable review comment if needed or update the existing comment in place.

[SebTardif]

@clawsweeper re-review

[clawsweeper]

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event exact_review_queue).
Result: when the review finishes, ClawSweeper will create the durable review comment if needed or update the existing comment in place.

[SebTardif]

@clawsweeper

[P1] Preserve slow webhook deliveries across the new cutoff
A subscriber that responds just after two seconds now reaches the existing cursor-advance path and permanently misses the batch.

Pushed 63f6c5e. Timeout still aborts the hung POST. The cursor stays put so the next drain retries that batch. HTTP and unreachable outcomes still advance (notify-only).

@clawsweeper re-review

[SebTardif]

@clawsweeper

[P1] Get approval for retry-on-timeout delivery semantics
The new timeout path leaves the cursor unchanged, but the released webhook contract says failed posts are notify-only and never retried. A timeout can follow a completed remote side effect, so the next drain may duplicate entries; either preserve cursor advancement or obtain explicit approval.

Pushed e12a356. Contract chosen: notify-only, matching the released drain docs. Timeout now advances the cursor like HTTP and unreachable, then stops the batch.

@clawsweeper re-review