Refuse queued messages for threads whose environment is gone

[SawyerHood]

What was wrong

POST /threads/:id/queued-messages (createQueuedMessageForThread in apps/server/src/routes/threads/actions.ts) only checked archived/stopping/deleted. It never applied the gone-environment rule (goneThreadEnvironmentDetails) that the direct send path applies through requireThreadCommandEnvironment. A thread whose managed worktree was destroyed (archive → grace window → destroy → unarchive) kept status: idle, answered 201 with a queued-message id, and the message could never drain: the auto-send hit the same 409 thread_environment_unavailable internally and the 10 s sweep retried forever. The CLI also labelled the destroyed worktree "Provisioning".

Issue: #1789. Report: https://get-bb.github.io/reports/issues/1789.html

What changed

• apps/server/src/routes/threads/actions.ts: createQueuedMessageForThread admits the message inside the same BEGIN IMMEDIATE transaction as the insert, on the freshly loaded thread row (admitQueuedMessage): writable check, environment check, and one provider-thread-id read. A destroying/destroyed environment returns the same 409 thread_environment_unavailable as the send path. A thread with environmentId === null that already has a provider thread id (the row was pruned after destroy) returns 409 with reason never_attached, again the same as send. A thread that has not run yet and has no environment still accepts queued messages, because that is how messages wait for provisioning.
• packages/db/src/data/queued-thread-messages.ts: createQueuedThreadMessageInTransaction for caller-owned transactions; listIdleThreadsWithQueuedMessages joins environments and skips destroying/destroyed, so queued rows that survived archive → destroy → unarchive no longer fail the sweep every 10 s.
• packages/core-ui/src/environment-display.ts: formatEnvironmentDisplay labels a destroying environment "Destroying" and a destroyed one "Destroyed" instead of "Provisioning". This changes bb thread show and app metadata labels.
• No wire shape changed, so HOST_DAEMON_PROTOCOL_VERSION is unchanged. No new routes or CLI flags.

Not done here: the report also suggests a runtime.environmentStatus/canRun field on the thread response. That is an API contract addition and is left as a follow-up.

How you verified

• apps/server/test/public/public-thread-queue-gone-environment.test.ts (from the report, extended): 3 of 5 tests fail before the route fix (expected 201 to be 409); the sweep test (real archiveThread → retire.requested → destroy.started → destroy.completed → POST /unarchive) fails before the query change; all 5 pass after. One test is a control that a never-run, environment-less thread still gets 201.
• packages/core-ui/test/environment-display.test.ts: new case for destroying/destroyed labels.
• pnpm exec turbo run test typecheck --filter=@bb/db --filter=@bb/server --filter=@bb/core-ui --filter=@bb/cli: typecheck clean; db 406/406; core-ui 17/17; cli 452/452; server 1798/1799. The one failure is test/internal/internal-skill-trees.test.ts (file mode 420 vs 436), a umask difference on this machine, unrelated to this change.

Fixes #1789

AGENT GENERATED: by Claude Opus 5

[bb-slop-cop]

🚨 SLOP COP 🚨 · review

I am SlopCop. I am reviewing this pull request for security, code quality, performance, architecture, and end-to-end behavior.

[bb-slop-cop]

🚨 slopcop/review — The formatter does not show the lifecycle state consistently.

A custom name replaces Destroyed later in this function. Also, destroying maps to Destroyed before destruction completes.

Use a separate lifecycle label. Another option is to define and test the intended name priority and state text.

[bb-slop-cop]

🚨 slopcop/review — Existing queue rows can still enter the permanent retry loop.

This check protects only new rows. Queue rows survive archive. Unarchive makes them sweep candidates after destruction.

Each sweep then fails and logs again. Handle existing rows during cleanup, or skip them without repeated retries.

Add an archive, destroy, unarchive, and sweep regression test.

This check also occurs before two waits. An archive can change both records before the later insert uses their old state.

Recheck both records inside the same immediate transaction as the insert.

[bb-slop-cop]

🚨 SLOP COP 🚨 · review

Plain-English summary: This change stops new queue messages when a thread's work area is being removed or has disappeared. It also marks that work area as Destroyed.

I found two correctness blockers.

1. Existing queue rows still survive archive and destruction. Unarchive makes them sweep candidates again, so the permanent failure loop remains.
2. The new lifecycle check occurs before two waits. An archive can change the records before the queue insert uses their old state.

I also found two display gaps. A custom environment name hides Destroyed, and destroying displays as Destroyed before completion.

The null-environment path reads provider history twice. The second read can repeat a long synchronous event scan.

Security review found no separate security issue. The 409 response contains only fixed lifecycle values.

The route helper repeats policy from thread-command-environment.ts. A transaction-based queue admission service would keep one policy and close the race.

Validation passed:

• The server regression test passed all four cases.
• The Core UI test passed all 11 cases.
• Server and Core UI type checks passed.
• The source app browser test showed Environment archived and Destroyed.
• The browser queue request returned 409 with thread_environment_unavailable.
• git diff --check passed.

The independent GPT-5.6 gate confirmed the blockers. I used a comment review, not an approval or request-changes review.

[SawyerHood]

Addressed the SlopCop findings in c4c6df9:

1. Existing queue rows still loop in the sweep — fixed. listIdleThreadsWithQueuedMessages now joins environments and skips destroying/destroyed rows, so a row that survived archive → destroy → unarchive is no longer a sweep candidate. Regression test: drops a thread from the auto-send sweep once its environment is gone drives the real lifecycle (archiveThread, retire.requested, destroy.started, destroy.completed, POST /unarchive) and asserts the thread leaves the candidate list while the row stays visible. It fails without the query change.
2. Check before two awaits — fixed structurally. The environment check no longer runs before the awaits at all. admitQueuedMessage (writable + environment + provider-thread-id, one event read) runs on the freshly loaded thread row inside the same BEGIN IMMEDIATE transaction as the insert (createQueuedThreadMessageInTransaction). Any state the checks see is the state the insert commits against. There is no deterministic injection point in the route to interleave an archive between the awaits, so the admission path is covered by the 409 tests, which now go through it.
3. destroying displayed as Destroyed — fixed: Destroying vs Destroyed, with a test.
4. Custom name hides Destroyed — not changed, on purpose. formatEnvironmentDisplay has always let a custom name win over every generated lifecycle label, including Provisioning; that is its documented contract and the app shows the gone state with a separate banner. Changing name priority is a product decision outside this fix.
5. Double provider-history read — fixed; the id is read once in the transaction and reused for the auto-send decision.

AGENT GENERATED: by Claude Opus 5