fix(server): release webhook claims when acceptance fails

[saphid]

A failed webhook acceptance could leave its relay deduplication claim behind. The relay retries the HTTP 500, but the stale claim answers duplicate and the task never starts.

The service now tracks whether this invocation acquired the claim, releases it when acceptance fails before dispatch, and retains it after dispatch. A duplicate request or failed claim insert cannot delete another invocation's claim. Existing rate-limit and queue-full release behavior is preserved.

Credit to @saphid for #18820. Contributor commits are preserved, with current main merged in. An independent implementation pass retained the focused production fix; the maintainer test pass consolidated the two added tests into one recovery flow in the existing suite.

Verification

Reviewed local head: 3f0cc0983d, based on c274a25151.

• PR-added tests: 2 → 1, in the existing webhook service suite; focused total 41 → 40 across service and route suites (33 + 7), all passing.
• Before: the retained regression fails on the pinned main implementation because both retries return duplicate. Removing the ownership guard also fails because a non-owner deletes the accepted claim.
• After: the regression covers a delivery-log failure, concurrent retries yielding one accepted and one duplicate, one launch, and a failed non-owner request preserving the accepted claim. Real migrated in-memory SQLite, no sleeps or polling.
• Server typecheck, targeted lint and formatting pass.
• Round 1 found no behavior defects; Opus requested restoration of the existing claim-lifetime rationale, now restored. Fresh full-diff round 2 by Opus and Sol found no actionable issues. Fresh pushed-head CI is required before merging.

Actual Linux loopback HTTP verification used the real webhook route, signed relay JWT verification, service and migrated in-memory SQLite. The external provider launcher was stubbed and observed through a Queue; no provider turn ran. Safe HTTP proof log.

Request	HTTP / outcome	Observed result
Accepted-log write fails	500 / error	No delivery or launch
Two concurrent retries after removing failure	202 / accepted + duplicate	One delivery and one launch
Non-owner claim insert fails	500 / error	Existing claim retained
Same delivery retried afterward	202 / duplicate	No second launch

The HTTP proof and focused checks ran at 061cac9475. The final follow-up only restores four existing comment lines; executable source and tests are unchanged. Targeted lint and formatting also pass after that comment change. The ephemeral HTTP server was closed after verification. Screenshots do not apply to this server behavior. No deployed relay, actual provider completion, macOS, Windows, Electron shell, iOS or Android execution was verified here.

Cleanup remains best effort if SQLite cannot delete the claim. The pre-existing overlap race (a duplicate receives 202 before the first request later fails acceptance) and restart durability in #18810 remain outside this fix. This is the focused obvious-bug contribution exception; it changes no schema, configuration or product defaults.

Implementation: gpt-6.1-sol; tests: gpt-6-astra; reviews: gpt-6-sol and claude-opus-5-5. Harness: Codex / Claude Code through T3 Code.

[macroscopeapp]

Approvability

Verdict: Approved at 3f0cc09

Macroscope's review found this PR approvable — This is a small, self-contained server bug fix that makes webhook claim cleanup ownership-aware and releases claims when acceptance fails before dispatch. Existing successful paths remain unchanged, and focused tests cover the failure and retry scenarios.

^{You can add or adjust custom eligibility rules. Learn more.}

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Path: .coderabbit.config.ts
• Review profile: CHILL
• Plan: Advanced
• Run ID: 71cba453-e7e1-48c6-b1b9-067833204b43

📥 Commits

Reviewing files that changed from the base of the PR and between bdb5604 and 3f0cc09.

📒 Files selected for processing: 2

• apps/server/src/scheduledTasks/ScheduledTaskService.ts
• apps/server/src/scheduledTasks/ScheduledTaskService.webhook.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 0 remain after this review.

📝 Walkthrough

Relay claim ownership now follows acquisition and dispatch handoff. The service releases acquired claims when errors occur before dispatch starts. Tests cover delivery-log and claim insertion failures, retry, and concurrent requests using the same relay delivery ID.

Changes: Relay delivery claims

Layer / File(s)	Summary
Track claim ownership through dispatch apps/server/src/scheduledTasks/ScheduledTaskService.ts	The service records when an invocation acquires a relay claim. It releases the claim on errors only while that invocation owns it, and retains the claim after background dispatch starts.
Test retry and concurrent delivery handling apps/server/src/scheduledTasks/ScheduledTaskService.webhook.test.ts	The test setup provides a SQL client to test callbacks. Tests verify retry and concurrent request outcomes after delivery-log or relay-claim insertion failures.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~12 minutes

Change: Bug fix

Suggested reviewers: juliusmarminge

Merge Risk: ⚪ Minimal · up to 3f0cc

The claim-release change is ready to merge after normal checks. No new blocking issue was identified.

Security architecture risk: 🔵 Low · up to 3f0cc

The change limits claim deletion to the request that acquired it and preserves deduplication after dispatch. No expanded access or weakened authentication was identified. Recovery remains best-effort when storage writes fail, and restart recovery remains outside this fix.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• observed — The affected state is the server's persistent relay-delivery claim table and scheduled webhook dispatch. Claims are keyed by relay delivery ID and associated with a task. This change does not grant a caller broader task-execution authority.

Security Findings and Attack Paths

• observed — The change closes a cross-request cleanup hazard: a duplicate request never records ownership, so its error handling cannot delete the original request's claim. After fork handoff, parent cleanup no longer has claim-release authority, preserving deduplication for dispatched work.

Trust Boundaries and Controls

• observed — The verifier validates a typed EdDSA JWT with issuer, environment audience, age, and schema checks, then matches environment, delivery ID, received timestamp, and hook ID. Missing or invalid proof follows the existing direct-webhook path without forwarding relay identity; it does not bypass the URL-token check.

Resilience and Maintainability Implications

• inferred — The PR improves recovery from pre-dispatch errors but does not establish durable dispatch recovery or eliminate the existing window in which a concurrent retry observes a claim before acceptance finishes. Cancellation protection is not process-crash protection. These are unchanged lifecycle limitations, not new PR concerns.

Pre-merge checks: 4

Check name	Status	Explanation
Title check		The title is concise, specific, and accurately describes the main fix: releasing webhook claims when acceptance fails.
Description check		The description clearly covers the problem, change, scope exception, verification results, limitations, and agent attribution. It does not use the template headings for Problem, Change, and Scope and …
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Description check

Explanation

The description clearly covers the problem, change, scope exception, verification results, limitations, and agent attribution. It does not use the template headings for Problem, Change, and Scope and approval, but the required information is present and the description is otherwise complete.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

_{Comment @coderabbitai help to get the list of available commands.}

Dismissing prior approval to re-evaluate bdb5604

[juliusmarminge]

Taking over this one as maintainer for batch scheduler-tasks-01. I will preserve the original commits, integrate current main, rebuild the focused claim-release fix, and verify it with fresh tests and independent reviews. This takeover will remain unmerged.

Dismissing prior approval to re-evaluate 3f0cc09