Deleted thread keeps its pull-request link, making the PR permanently unopenable #12336

Activity

1. juliusmarminge commented on Sep 18, 2026

   

   juliusmarminge

   on Sep 18, 2026

   Member

   More actions

   Triage

   Verdict: confirmed bug, still present on current main (not a duplicate, not fully fixed).
   Labels: bug, accepted, via-triage

   The report matches the code. A deleted thread can keep (or regain) the projection_thread_pull_requests row for that PR, and a later open/link against the same thread id refreshes linked_at instead of creating a replacement thread.

   What already exists

   thread.deleted already drops the thread's PR-link rows, with a projector test for it. That landed in #10839 and is in 0.0.42, the version on the report:

           case "thread.deleted": {
             // ...
             // A tombstoned thread must not show up as linked to a pull request.
             yield* projectionThreadPullRequestRepository.deleteByThreadId({
               threadId: event.payload.threadId,
             });

   The public linked-threads query also skips deleted threads (t.deleted_at IS NULL in apps/server/src/pullRequest/linkedThreads.ts). The HTTP shell snapshot filters them out, and the WS shell stream emits thread-removed on thread.deleted.

   So “delete never clears the link table” is not the whole story on 0.0.42. The timestamps in the report are the rest of it: deleted_at = 15:20:06Z, then linked_at refreshed at 23:49:44Z after a successful preparePullRequestThread git path. That refresh is an upsert, not a leftover row that delete failed to touch.

   Remaining hole

   Most thread commands still accept a soft-deleted id. requireThread only checks that the row exists. thread.pull-request.link uses that helper and does not check deletedAt. The projector then upserts the link and overwrites linked_at. thread.meta.update can take the same path via the legacy single-link rewrite. The in-memory projector sets deletedAt on thread.deleted but does not clear pullRequests, so the decider still sees the old link.

   listByPullRequest also has no deleted_at filter. Anything that resolves “which thread owns this PR?” from that table (or from the command read model, which includes deleted threads and their remaining links) can land on the tombstone.

   That matches this client: iframe + /api/orchestration/shell + /api/orchestration/dispatch. Official checkout creates a new draft id (openOrReuseProjectDraftThread) and should still be able to spawn a second thread — there is no unique constraint on (host, repository, number). The “permanently unopenable” failure is for a client that treats the existing link / cached thread id as the resolution target. The server will happily re-link that tombstone.

   OrchestrationThreadShell has settledAt / snoozedUntil / archivedAt and no deletedAt. The list snapshot already omits deleted threads, and per-id shell/detail reads use deleted_at IS NULL. The API gap is real for a client that caches a thread id or mixes shell with the command snapshot (GET /api/orchestration serves getCommandReadModel, which does include deleted threads). Dispatch then still accepts commands on that id.

   Suggested fix

   1. Keep the existing deleteByThreadId on thread.deleted (already there). Also clear pullRequests in the in-memory projector.
   2. Reject thread.pull-request.link / thread.pull-request-link.sync / legacy thread.meta.update link writes when deletedAt !== null. Skip deleted threads in every PR→thread lookup (listByPullRequest consumers and command-read-model resolution), not only listLinkedPullRequestThreads.
   3. Optional but useful for embedders: put deletedAt on OrchestrationThreadShell, or document that shell absence plus thread-removed is the only deletion signal. Filtering the list is not enough if dispatch still mutates the tombstone.

   The SQL workaround in the report is the right recovery: delete the orphaned projection_thread_pull_requests row, then restart.

   Related

   No existing issue or later commit covers the re-link-after-delete hole. #10839 only added the delete-time cleanup. Related but distinct: #9085 (worktree cleanup on archived delete), open #8796 (provider_session_runtime binding), closed #7653 (merged PR after remote branch delete).
2. juliusmarminge

   added

   bugSomething is broken or behaving incorrectly.Something is broken or behaving incorrectly.

   acceptedfeature request acceptedfeature request accepted

   via-triageFiled through npx t3 triageFiled through npx t3 triage

   on Sep 18, 2026
3. chrisjfunk commented on Sep 18, 2026

   

   chrisjfunk

   on Sep 18, 2026

   Author

   More actions

   Follow-up after working around this on the affected machine. Two things that change the picture.

   Deleting the orphaned row does not hold. I deleted it directly in state.sqlite and confirmed zero rows. The next open attempt recreated it, same deleted thread, three minutes later:

   deleted by hand   18:58Z          (verified: 0 rows)
   recreated         00:01:33.586Z   thread d261fce4-…, deleted_at 15:20:06.731Z
   

   Nothing rebuilt it from an event log — the client re-sent that thread id, and the server re-linked the pull request to a thread it knows is deleted. So the link is not merely stale state left behind by deletion; it is writable against a deleted thread at any time. A check at link time, rejecting a thread_id whose deleted_at is set, would close this independently of whatever cleanup deletion does.

   The client could not have known better, and this is the part worth fixing first. The thread id came from the client's own item-to-thread mapping. Before re-sending it, the client verified the thread was live the only way the API allows — it looked for it in /api/orchestration/shell — and found it, because the shell lists deleted threads and the thread shape has no field that distinguishes one. The client then did exactly what it should with a thread the server says exists.

   That is the whole loop, and it is closed:

   1. Client sends thread id from its mapping.
   2. Server re-links the PR to that deleted thread and refreshes linked_at.
   3. Client validates against the shell; the deleted thread is present, so validation passes.
   4. No new thread is ever created. Repeat forever.

   The pull request became openable only after clearing both ends: the projection row, and the client's mapping from a browser console. A new live thread was created immediately after, and now coexists with the stale row:

   8a4427d4-…   live                      linked 00:05:06Z
   d261fce4-…   deleted 15:20:06Z         linked 00:01:33Z   ← still present
   

   Two rows for the same pull request, one pointing at a deleted thread. Whatever the resolution order is, that is worth rejecting at write time.

   Ranked by what would have prevented the dead end:

   1. Reject a link write whose target thread is deleted. Breaks the loop at step 2 even with a client sending a stale id.
   2. Exclude deleted threads from /api/orchestration/shell, or give the thread shape a deletedAt. Breaks it at step 3 and lets clients self-heal — without this, no client-side liveness check is possible at all.
   3. Clear a thread's projection_thread_pull_requests rows when it is deleted. Prevents the orphan existing, though on its own it does not stop step 2 from recreating one.

   We shipped a client-side workaround — forget the mapping and reopen once when a thread we were sent to does not materialise — but it only helps where the shell omits the thread. While the shell reports deleted threads as live, no client can detect this case.
4. juliusmarminge commented on Oct 2, 2026

   

   juliusmarminge

   on Oct 2, 2026

   Member

   More actions

   Thanks for taking the time to report this and provide the details. We revisited it during the orchestrator V2 cleanup.

   V2 dispatch now rejects metadata and pull-request link/sync commands when thread.deletedAt is non-null. This directly breaks the reporter-confirmed re-link-after-delete loop even if an external client resends a cached tombstone ID.

   I’m closing this based on the current source and the evidence in this thread.

   Source reviewed.

   Verified server write guard from source. External embedders must refresh stale mappings; historical orphan rows were not inspected.

   If you still hit this on a current build, please reply with the app/server versions and the steps that reproduce it. We can reopen this if the original problem is still there.
5. juliusmarminge

   closed this as completedon Oct 2, 2026