# PR #23579 proof matrix (audit pass 1)

before = `main` 8d3b061121, after = `fix/issue-18810` e3c21846c2. Paths are relative to `apps/server/src` unless noted.

## Shared setup for a real server

**Start with isolated state.** Run `node apps/server/src/bin.ts --base-dir <tmp> --port <p> --no-browser [--auto-bootstrap-project-from-cwd]` from a scratch git repo (`cli/config.ts:33-62`).
- There is no `--home-dir` flag on `bin.ts`; it is `--base-dir`.
- `--no-seed` is a `vp run dev` / dev-runner option, not a `bin.ts` flag. An empty `--base-dir` starts empty.
- `--auto-bootstrap-project-from-cwd` creates the project without the UI.
- To restart, SIGKILL the PID you captured at spawn, then start the same command again.

**Create the task without the UI.** Three options:
1. WS RPC `scheduledTasks.upsert` (`packages/contracts/src/rpc.ts:515`). Needs a paired session.
2. MCP `POST /mcp` (`mcp/McpHttpServer.ts:1063`). Needs a `Bearer <session token>`. Use the tool `schedule_task` with `schedule:{type:"webhook", dedupe?:{...}}`. Its `clientRequestId` becomes `commandId`, so the id is `scheduled-task:<commandId>` (`ScheduledTaskService.ts:1068-1074`).
3. Seed a row in `scheduled_tasks` while the server is stopped (`persistence/Migrations/OrchestrationV2/ScheduledTasks.ts`, plus `webhook_token`, `webhook_secret`, `last_run_webhook_delivery_id`). The PR author did this.

**Deliver.** `curl -X POST http://127.0.0.1:<p>/api/hooks/<taskId>/<webhook_token> -H 'content-type: application/json' -d '{"id":"evt-1"}'`. A 202 has outcome header `x-t3-hook-outcome` (`scheduledTasks/webhookRoute.ts:21`).
- **Dedupe templates are body, header or query placeholders.** Examples: `replayKey:"{{body.id}}"` or `"{{headers.x-delivery}}"`, `coalesceKey:"{{body.repo}}"`, `coalesceWindowSeconds` from 10 to 3600, default 120 (`packages/contracts/src/scheduledTask.ts:85-103`).
- **A key is not applied if it renders empty or has a missing field** (`webhookDedupe.ts:9-15`). So the request must carry the field.

**How to hold the per-task permit so later deliveries queue.**
- A *webhook* run holds the permit for the whole `threadLaunch.launch` call.
- A sync setup script blocks inside launch (`orchestration-v2/ThreadLaunchService.ts:606-631`, the `!setup.async` branch). Set this up as follows:
  - Give the project a script with `runOnWorktreeCreate:true`, `async:false`, and command `sleep 120`.
  - Give the task `workspaceStrategy` worktree and `threadId` null.
- The first delivery D1 then holds the permit for about 120 s, and every later delivery waits.
- **Do not use a scheduled or `runNow` run to hold it.** That run takes `activeRuns`, not the permit, so a webhook arriving then fails immediately with "Schedule task is already running." (`ScheduledTaskService.ts:740-743`). It does not queue.

**DB observations.** Use `sqlite3 <base-dir>/userdata/statev2.sqlite`, read-only, with the server stopped or in WAL-safe mode.
- `scheduled_task_webhook_deliveries` (`persistence/Migrations/057_ScheduledTaskWebhooks.ts:12-26`): `delivery_id, task_id, received_at, outcome, error, rendered_prompt, missing_fields_json, …`. Outcomes: `accepted`, `duplicate`, `coalesced`, `dispatch_failed` (`markDeliveryFailed`), …
- `scheduled_task_webhook_pending` (`persistence/Migrations/067_WebhookDedupe.ts:16-28`): `delivery_id PK, task_id FK→scheduled_tasks ON DELETE CASCADE, coalesce_key, due_at (ms), state pending|dispatching|running|completed|failed, task_json, request_json, delivery_ids_json`. After this PR, an immediate row has `coalesce_key = ''`.
- `scheduled_task_webhook_replay_keys`: `task_id, replay_key (sha256), delivery_id, expires_at`.
- `scheduled_tasks`: `last_run_status, last_run_error, run_count, last_run_webhook_delivery_id, created_at`. Startup recovery sets `'Run was interrupted by a server restart.'` (`ScheduledTaskService.ts:981-1030`).
- Thread side: a launch uses the deterministic command id `scheduled-task:<taskId>:webhook:<deliveryId>` and message id `scheduled-task-message:<…>` (`ScheduledTaskService.ts:815-820`). To check for one launch per delivery, look for exactly one thread or message with that id in the orchestration projections, or for one `ScheduledTaskService.runWebhookDelivery` span per delivery.

## Matrix

| Row | (a) Real-server setup | (b) Distinguishing DB state | (c) before → after | (d) Reachable? |
|---|---|---|---|---|
| **R1** one immediate delivery, SIGKILL before its run starts | Task with no dedupe or with replayKey only (no coalesceKey). Hold the permit with D1 (sync `sleep 120` setup script). POST D2 and get 202. SIGKILL during the sleep. Restart without the slow script, or wait about 1 tick. | At the kill, before: D2 has a deliveries row `accepted` and **no** pending row. After: pending `D2 state='dispatching', coalesce_key=''`. After restart, before: D2 stays `accepted` forever and no thread has command id `…:webhook:D2`. After restart, after: the `webhook-coalescing` tick drains D2, which yields a thread with that command id. The pending row is deleted once the run finishes. D2 stays `accepted`, or becomes `dispatch_failed` with a reason if the launch fails. | **Before: lost. After: runs once.** | **Yes**, with the slow sync setup script. Without a hold, the gap between the 202 and `markRunning` is milliseconds, so the kill is a race. The PR author's "two concurrent POSTs then kill" relies on that race. |
| **R2** two concurrent immediate deliveries behind a running task, SIGKILL, restart | Same hold. POST D2 and D3 concurrently (`curl … & curl …`). SIGKILL. Restart. | Before: neither D2 nor D3 has a pending row, both stay `accepted`, and nothing dispatches (this matches the PR body's repro). After: two rows `dispatching`. After restart they drain **in order** (`ORDER BY due_at, rowid`) and run one after the other under one permit. You should see two threads (or two queued sends) with distinct command ids. | **Before: both lost. After: both run, serialized.** | **Yes.** D1's pending row is `running` at the kill, and the drain re-runs it first with the same command id (see C1). |
| **R3** coalesced delivery across a restart (unchanged path) | `dedupe:{coalesceKey:"{{body.repo}}", coalesceWindowSeconds:30}`. POST `{"repo":"a"}` twice: the first is `accepted`, the second `coalesced`. SIGKILL within 30 s. Restart. | On both sides: one pending row `state='pending', coalesce_key=<sha>` whose `delivery_ids_json` holds both ids. After `due_at` it becomes `dispatching` and then runs once with the latest `request_json`. One extra difference: on the after branch the list/get queries report `pending_window` only when `coalesce_key <> ''` (`ScheduledTaskService.ts:1347,1368`). This row is unaffected. | **Same before and after: survives and runs once.** It is a regression guard for the shared `drainPendingWebhooks` refactor. | **Yes.** Before the window closes, it is easy to compare the deliveries-list RPC `pendingWindow` flag. |
| **R4** duplicate replay after a restart, same replay key | `dedupe:{replayKey:"{{body.id}}"}`, no coalesceKey. With the permit held, POST `{"id":"X"}` (D2, 202 `accepted`). SIGKILL. Restart. POST `{"id":"X"}` again. | The `replay_keys` row for sha(X) is committed with D2 on both sides. The replay is logged `duplicate`. Before: D2 never ran, so the delivery is **lost** and the replay comes back as `duplicate`, not a run (this is the bug). After: D2 runs exactly once from the pending row. The replay is `duplicate`, and only one thread or message has the command id `…:webhook:D2`. | **Before: dup and zero runs. After: dup and exactly one run.** | **Yes.** Caveat: if D2's re-run **fails** (for example a missing project), `releaseFailedDelivery` deletes the replay key (`ScheduledTaskService.ts:1560-1580`). A replay that arrives *after* that failure is then accepted and runs, which is intended. To prove "never twice", use a project that launches successfully, or replay before the drain finishes. The replay must use the same body field; a missing field gives an empty key and no dedupe. |
| **R5** task replaced between accept and restart | Delete the task (MCP `delete_scheduled_task` or RPC `scheduledTasks.delete`). Recreate it with the **same id** by upserting with the same `commandId`/`clientRequestId` (MCP `schedule_task`). That gives a new `created_at`. | **Across a restart, the permit check cannot apply.** `webhookPermits` is in memory and empty after a restart. The delete also cascades the old `scheduled_task_webhook_pending` rows away (FK + `PRAGMA foreign_keys=ON`, `persistence/Sqlite.ts:19`), so no old row survives to be drained. If a row did survive, `runTask` would skip it as "replaced" by its `createdAt` (`ScheduledTaskService.ts:783-794`). **The guarded case is in-process only.** An old drain fiber still waits on, or loops under, the old permit while delete (`forgetWebhookTask` drops the permit, `:1238`) and recreate produce a new permit and a new D'. Without the check, the old fiber would pick up D' under the wrong permit. With the check (`ScheduledTaskService.ts:1653`), it returns. | Before: n/a (main had no shared drain). After the PR without the check: wrong-permit concurrent run. With the check: D' runs once under its own permit. | **Effectively unit-test-only.** It needs delete and recreate with the same id while an old drain fiber is parked. A real server can get there in principle: hold the permit with a sleep script, queue D2, delete, recreate with the same clientRequestId, then POST D'. But observing the difference requires two runs overlapping, which is timing-dependent. Not a restart scenario. Task **update** (upsert that keeps `createdAt`) keeps the permit and is harmless. |

## Concerns

- **C1: the `running` row is re-run after a crash.** After a SIGKILL, D1's pending row is `state='running'`. The drain selects `running` rows and calls `runTask` again with the same command id (`ScheduledTaskService.ts:1660`, `:815-820`).
  - Whether `threadLaunch.launch` is idempotent when replayed for a thread left half-prepared mid-setup-script is a ThreadLaunchService property this PR does not test. On main the same delivery was simply abandoned.
  - Worth one runtime check in R2: D1 should end as one thread, not as an error or a second thread.
- **C2: drain fibers stack up under the permit.** Every accepted immediate delivery forks a new `drainPendingWebhooks` (`ScheduledTaskService.ts:2082`), and the tick forks another, guarded only by `pendingInFlight`. They queue on one permit, so the count is bounded by `WEBHOOK_MAX_QUEUED_PER_TASK`. It is correct, because every fiber re-selects the head row, but redundant. The extra fibers no-op once the rows are gone.
- **C3: the queue cap now counts persisted rows only.** The capacity check counts `pending|dispatching|running` (`ScheduledTaskService.ts:1944-1952`), and an immediate row is inserted inside the same `admit` transaction (`webhookDedupe.ts:104-108`, `ScheduledTaskService.ts:1954`). Concurrent POSTs are therefore serialized by SQLite, which is fine. A crash still leaves `running` rows counted until they drain.
- **C4 (resolved, no issue): the narrowed `remove` is safe.** `remove` now only deletes `state='completed'` (`ScheduledTaskService.ts:1665`). `runTask` sets the row to `completed` or `failed` in the completion transaction (`ScheduledTaskService.ts:900-911`), and a `failed` row is deleted by `releaseFailedDelivery` (`:1576-1579`). If the task is deleted mid-run, the FK cascade removes the row. No spin.
- **C5: the PR body's runtime transcript does not prove a launch.** It used a missing project, so both deliveries ended `dispatch_failed`. That proves the drain happened, not that a run launched. A successful launch, ideally R4 with a real project, would close C1 and C4.