Skip to content

fix(server): fast session-webhook ack (Toolyard callbacks no longer time out) - #318

Merged
bk-agent-01 merged 3 commits into
bkmainfrom
tushar/webhook-ingest-fast-ack
Oct 16, 2026
Merged

bk-agent-01 merged 3 commits into
bkmainfrom
tushar/webhook-ingest-fast-ack

Conversation

@bk-agent-01

@bk-agent-01 bk-agent-01 commented Oct 15, 2026 •

Copy link
Copy Markdown
Collaborator

Why

B1: session webhooks time out. Toolyard sends a signed decision callback to POST /api/session-webhooks/:webhookId. Since 2026-10-04, 536 of 542 callback attempts failed with network_error. The T3 trace shows the request took 19 963 ms (http.server POST, route /api/session-webhooks/:webhookId, user agent Go-http-client/1.1, 2026-10-15 14:49Z and 14:53Z). Toolyard's sender stops after 20 s (internal/callbacks/service.go:531). The client then disconnects, T3 interrupts the request, and the transaction rolls back. So session_webhook_events stays empty and no decision reaches the session.

Root cause, from the trace spans: sessionWebhook.receive runs the first SELECT, two ServerSecretStore.get calls, then sql.transaction. Inside the transaction, the two SELECTs finish in under 1 ms, and then nothing happens until the interrupt at about 19.9 s.

  • The next call in the transaction was integration.callbackBinding(...) (SessionWebhookService.ts, old lines 434-436).
  • callbackBinding calls core.assertConnectionOwner → verifyUser. That function reads the user with Effect.runPromise(users.get(...)) (toolyard/ToolyardIntegration.ts:176). This starts a new root fiber that does not share the transaction's connection.
  • The SQLite client has one connection behind a one-permit semaphore (packages/shared/src/nodeSqliteClient.ts:262-272). The open transaction holds that permit. The new fiber waits for the permit, and the transaction waits for the new fiber. This is a deadlock. It ends only when the client disconnects.
  • During the wait, every other SQL statement on the server also waits for the same permit.

Scope

This PR first also had "B2", a personal-token route to /mcp/upstream/toolyard for a host relay timer. That timer was cancelled on 2026-10-15, so commit 962e5f40 removes B2: the PR now changes only SessionWebhookService.ts and its test.

What changed

B1 (apps/server/src/session-webhooks/SessionWebhookService.ts)

  • The admission transaction now runs SQL only. It no longer calls the integration. It re-reads the webhook row and checks that the status is active and the trust_binding is the same as the row that the handler verified before the transaction.
  • The signature check (Standard Webhooks), the payload parse, the event-id idempotency, the content-conflict check (409) and the rate limit (429) are unchanged.
  • The handler stores the event as queued and returns 202. The existing dispatch loop (drain, every 2 s) sends it into the thread. That loop already re-checks the integration binding before dispatch (line ~570), and retireReplacedReceiver ends queued events of a replaced receiver.
  • The pull path calls the same receive, so this also fixes pulled callbacks. Before the fix, the 2 s pull timeout cut them off.
  • Test (SessionWebhookService.test.ts): a new harness flag makes the owner check read the database on a fresh fiber, like production. The new it.live test receives a callback, then a duplicate, within a 2 s bound, and then drains it. With the old code this test times out.

Risk and undo

  • B1: the transaction no longer re-reads the live Toolyard binding. If someone replaces the binding in the small time between the check and the insert, the event is stored. The dispatch loop then makes it terminal (integration-disabled-or-replaced) and does not deliver it. No path that delivered events before now skips a check.
  • Undo: revert this PR. It does not touch the database schema.

How I tested

  • CI on the current head 962e5f4009a6b1843dd340e87e766928055f6c11 (B2 removed), CI run 38769006567 and Validate and deploy bkt3 38769006468: Lint, validate, Typecheck, Build, Test, Test Web, Test Server 1-6, Rust, Release Smoke, markers, Mobile Native Changes and Native fingerprint diff pass. Transfer report artifact stays queued (pre-existing, below).

  • Nothing ran on the host (dev-server-1 runs no tests, typechecks or builds). CI ran the checks on the earlier head e87c1a7e14144a6d390d560e5eb995c7493a8b7a (B1 and B2): CI run 38763240414 and the "Validate and deploy bkt3" run of the same head.

  • Result per job on that head:

    Job Result
    Lint pass
    validate (formatting and lint) pass
    Typecheck pass
    Build pass
    Test pass
    Test Web pass
    Test Server 1, 2, 3, 4, 5, 6 pass (each)
    Rust pass
    Release Smoke pass
    markers (fork markers) pass
    Mobile Native Changes, Native fingerprint diff pass
    Mobile Native Static Analysis, EAS Preview, Deploy web preview skipped (no native or web change)
    Transfer report artifact queued, does not start (pre-existing, see below)
    Check no result, because it waits for Transfer report artifact
  • SessionWebhookService.test.ts: 45 tests pass. This includes the new test "acknowledges a push callback when the owner check reads the database".

  • On the first head 6d1f3e1a, four jobs failed: Lint, validate, Check and Test Server 2. This PR caused all four.

    • Lint, validate and Check failed on one lint error. The new test called Effect.runPromise, which t3code/no-manual-effect-runtime-in-tests refuses in tests.
    • Test Server 2 failed in the new it.live test. The test called h.event() two times. The live clock gave the second payload a new timestamp, so the second delivery was a 409 content conflict and not a duplicate.
    • Commit e87c1a7e fixes both. The test now sends one payload two times. The fresh-fiber read stays, because it copies the production owner check, behind an oxlint suppression that gives the reason.
  • Pre-existing, not from this PR: "Transfer report artifact" stays queued on the Blacksmith runner. The same job is also queued on other PRs, since 11:47Z and 13:48Z today (run 38729075691, run 38744996435). The aggregate Check needs that job, so Check does not report.

  • bkmain has no CI run to compare with. CI runs only on pull requests and on pushes to main. The latest bkmain "Validate and deploy bkt3" run passed (run 38561344711).

  • After deploy, the proof for B1 is one row in Toolyard callback_attempts with a 2xx status for a T3 session webhook, and the matching row in T3 session_webhook_events.

Merge blockers

  • The CI job "Transfer report artifact" stays queued on Blacksmith, so the aggregate Check does not report. This is a known problem that this PR did not cause.
  • This PR needs Tushar's GitHub review. The Toolyard GitHub App gets HTTP 403 on iamtushar324/* repositories, so the Toolyard approval route cannot submit it.

🤖 Generated with Claude Code

Session webhook receive: the admission transaction called the Toolyard
integration's owner check, which reads environment users through
Effect.runPromise on a fresh fiber. The SQLite client has one connection
behind a one-permit semaphore held by the open transaction, so that read
waited for the transaction that waited for it. Every push callback hung
until Toolyard's 20 s client timeout, the request was interrupted and the
insert rolled back. The transaction now runs SQL only; the binding is
checked before it and again by the dispatch loop.

Upstream MCP proxy: a personal T3 token (t3usr_) may call
/mcp/upstream/toolyard as the token's own user, with that user's managed
Toolyard credential, when the server and user external MCP switches are on.
No x-t3-session-id is sent for these calls. Provider sessions are unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:L labels Oct 15, 2026
…he live clock

- Reuse one payload for the duplicate delivery. Under it.live a second
  h.event() carries a new timestamp, so the retry was a 409 content
  conflict instead of a duplicate.
- Keep the fresh-fiber Effect.runPromise read, which replicates the
  production owner check, behind a reasoned oxlint suppression.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@bk-agent-01
bk-agent-01 marked this pull request as ready for review October 15, 2026 15:56
@bk-agent-01

Copy link
Copy Markdown
Collaborator Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

To use Codex here, create a Codex account and connect to github.

The host relay timer that needed a personal t3usr_ token on
/mcp/upstream/toolyard was cancelled (2026-10-15), so the new access
path has no user. Keep only the fast session-webhook ack.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@bk-agent-01 bk-agent-01 changed the title fix(server): fast session-webhook ack and personal-token toolyard proxy fix(server): fast session-webhook ack (Toolyard callbacks no longer time out) Oct 15, 2026
@github-actions github-actions Bot added size:S and removed size:L labels Oct 15, 2026
@bk-agent-01
bk-agent-01 merged commit d095635 into bkmain Oct 16, 2026
26 of 29 checks passed
@bk-agent-01

Copy link
Copy Markdown
Collaborator Author

Merged at head 962e5f40 with the approval of @tusharbhardwaj-bk on that head. The job "Transfer report artifact" was still queued: it runs on blacksmith-2vcpu-ubuntu-2404, which takes no jobs in this repository (the job also never ran on #309, #312, #314, #315 and #321). Tushar accepted the merge without that job in the session chat on 2026-10-16. Every other check on the head passed.

@bk-agent-01

Copy link
Copy Markdown
Collaborator Author

Production check (2026-10-31)

Read-only count of callback_attempts on the toolyard instance that sends callbacks to T3 session webhooks:

Period delivered (HTTP 202) network_error receiver_unavailable
2026-10-05 to 2026-10-16 12:40 UTC (before this merge) 2 813 0
2026-10-16 12:40 UTC to now (after this merge) 158 1 1

The network errors stop just after this merge (the last one is at 12:48 UTC, before the deploy finished). The newest delivery is on 2026-10-29 16:58 UTC.

Sent by: BK-T3-Code session (toolyard audit).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:S vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants