Skip to content

Hold messages for threads that await user interaction - #2129

Merged
SawyerHood merged 4 commits into
mainfrom
bb/fix-1650-blocked-thread-messages
Aug 25, 2026
Merged

Hold messages for threads that await user interaction#2129
SawyerHood merged 4 commits into
mainfrom
bb/fix-1650-blocked-thread-messages

Conversation

@SawyerHood

@SawyerHood SawyerHood commented Aug 21, 2026

Copy link
Copy Markdown
Collaborator

What was wrong

A thread parked on a pending user interaction (an AskUserQuestion, a command approval, a plugin input request) cannot take a prompt, and the server turned that into a drop with no recipient-side trace. Two guards keyed on hasPendingThreadInteraction did it: POST /threads/:id/send (every bb thread tell mode, including --mode queue since #112) threw 409 awaiting_user_interaction before anything was persisted, and queueParentSystemMessage returned false silently for child-completed/failed notices. A blocked orchestrator therefore never heard that its workers reported or finished, and only the sender saw the 409. Issue: #1650. Report: https://get-bb.github.io/reports/issues/1650.html

What changed

  • packages/db: new deferred_thread_messages table (migration 0108_deferred_thread_messages, snapshot regenerated with drizzle-kit) with a data module; migrate.test.ts rewinds drop it.
  • apps/server/src/services/threads/deferred-thread-messages.ts (new): the zod payload schema (send rows carry the original SendMessageRequest; parent-system rows carry input + taxonomy) and deferThreadMessage.
  • apps/server/src/services/threads/thread-send-request.ts (new): acceptThreadSendRequest is now the single send policy used by the route and by the flush: queue (queue-if-active on an active thread, compaction), else defer when the thread awaits interaction and mode !== "start", else send. Sender thread and attachment references are validated before a message is held, so a bad request still fails fast. flushDeferredThreadMessages delivers rows in arrival order in the mode the sender asked for (a steer-if-active tell steers into the resumed turn; a parent notice goes through queueParentSystemMessage), deletes a row only after delivery, and keeps the rest on failure. Flushes are serialized per thread through a new lifecycleDedupers.deferredThreadMessageFlush.
  • The sweep visits only threads that can act on their rows now, the way listIdleThreadsWithQueuedMessages does (A thread whose environment no longer exists still reports idle, and the message queue accepts sends into it that can never be delivered #1789). listThreadIdsWithDeliverableDeferredThreadMessages selects visible threads in status idle or active with a live environment. A thread in error, starting or stopping is deliberately absent: it refuses every send, and its state only changes when a user retries it, a start lands, or a stop finishes, so its rows wait and the tick after the change delivers them. Driving it on every tick would re-run the send pipeline and log a failure every ten seconds for as long as the thread sat there.
  • listThreadIdsWithUndeliverableDeferredThreadMessages selects the threads whose rows can never deliver — archived, deleted, or an environment that is gone and so is never reprovisioned — and the flush drops those rows once with a warn naming the reason, instead of leaving them until somebody archives the thread. The flush applies the same gone-environment rule, so the settle path and the sweep agree.
  • A held row whose request can no longer be honored at flush time (its sender thread was deleted, its attachment is gone: an ApiError 400/404 from the send pipeline) is dropped with a warn log and the flush continues, so it cannot head-of-line block the thread's later held messages. Plugin mentions (422), absent hosts (502) and host timeouts still keep the row and retry on the next settle or sweep.
  • PendingInteractionLifecycle.setThreadInteractionSettledListener: one settle hook (resolving/resolved/interrupted) that schedules a flush; createApp wires it. A deferred-thread-message-flush periodic sweep re-drives rows a restart or a stopping thread left behind.
  • queueParentSystemMessage defers instead of returning false when the parent is blocked.
  • createQueuedMessageForThread and queuedMessagePayloadFromSendRequest moved from the route file to services/threads/queued-messages.ts unchanged, so the service can reuse them.
  • Contract/SDK/CLI: /send now returns { ok: true, delivery: "sent" | "queued" | "deferred" } (additive; start keeps its 409). ThreadSendResult follows, so @get-bb/plugin-sdk bumps to 0.4.17; the provider-retry plugin's test mocks return delivery: "sent" to match. bb thread tell prints "awaiting user interaction; message held and delivers once the interaction settles" and --json carries delivery; an older server that only reports ok keeps the old wording.
  • The web app reads delivery instead of discarding the response. A deferred send restores the thread record rather than showing a working indicator for a turn that never started, keeps the optimistic message row instead of letting a refetch drop it, and tells the user the message is held and must not be sent again. Every composer the app renders is already swapped out for the pending interaction, so this closes the window where a send and an interaction register at the same moment.
  • Guide (bb-guide-threads.md), the bb-cli skill, and the manual runbook tell agents and testers the message is held, not rejected, and that a held message waits for a thread that failed while it was held and delivers when the thread is retried.
  • No host daemon wire change, so no HOST_DAEMON_PROTOCOL_VERSION bump.
  • Not in this PR: held rows have no list or cancel route in the SDK, CLI, or UI, unlike queued messages. Worth a follow-up issue.

How you verified

  • apps/server/test/threads/deferred-thread-messages.test.ts (9 tests, real SQLite through the harness): a worker tell to a blocked active thread returns delivery: "deferred" and steers in after the answer is delivered through the real interactive.resolve command path; mode=start keeps its 409; a missing sender thread is rejected before anything is held; a child-completed notice to a blocked parent is held and flushes with its taxonomy; three held messages deliver in arrival order and a sweep while still blocked delivers nothing; a row survives a stopping thread and the sweep delivers it once idle; a held tell whose sender is deleted before the flush is dropped and the user's message behind it still delivers; a thread that errors while a message is held logs at most one delivery failure and none across five further sweep ticks, then delivers the row as a steer after run.preparing + run.started; a thread whose environment is gone has its rows dropped once with a single warn. The last two fail on the previous revision with 6 warns instead of 1, and with the row never dropped.
  • App: thread-runtime-cache-owner.test.ts asserts a deferred delivery leaves the thread record idle, keeps the optimistic row, and does not invalidate the timeline (it fails on the previous revision, which marked the timeline stale so the refetch dropped the row); thread-runtime-mutations.test.tsx asserts the mutation returns the server's delivery.
  • Updated the Fix pending interaction queue send guard #112 regression (public-thread-interactions.test.ts): auto on a blocked thread is deferred, queue-if-active on a blocked active thread queues, start and queued-message send still 409. Updated { ok: true } assertions in public-thread-data, internal-events-tool-calls, plugin-sdk, the CLI thread tell tests (plus new held/legacy-server cases), the SDK test, the app mutation mock, and the provider-retry plugin send mocks.
  • pnpm exec turbo run build typecheck lint over the whole workspace: 92/92 tasks pass. node scripts/check-provider-literal-ratchet.mjs --base origin/main, node .github/workflows/check-plugin-sdk-version.mjs, and node packages/plugin-sdk/scripts/check-npm-version-guard.mjs all pass. pnpm exec turbo run test --force for @bb/server (211 files / 2016 tests), @bb/app (431 / 3355), @bb/cli (50 / 477), @bb/db (28 / 409), @bb/client-core (20 / 239), @bb/sdk (6 / 96), @bb/server-contract (7 / 60), @bb/templates (6 / 41) and bb-plugin-provider-retry (2 / 26): all pass.
  • Live on a dev instance (claude-code claude-haiku-4-5 with the native AskUserQuestion, and codex with the ask-user-question plugin's interaction). Held messages: bb thread tell printed the held line, --json returned delivery: "deferred", curl with mode=start returned 409 awaiting_user_interaction, and answering the question delivered every held row with a "Delivered deferred thread message" line and no failures.
  • Live on the sweep fix: a thread parked on a question with one held tell, whose provider process was then killed, sat in error with the row held. Before the fix the server logged the same 409 delivery failure every ten seconds — 9 warns in 90 seconds, the row never moved. After the fix, on the same database and the same thread, 0 warns in 90 seconds and the row still held; a mode=start retry returned delivery: "sent" and the held row delivered within ten seconds. Archiving a thread with a held row dropped it once with reason: "thread_archived" and did not repeat.
  • Live on the web app: a Claude Code thread parked on a native AskUserQuestion and a Codex thread parked on the plugin interaction both replace the whole composer with the interaction panel, so there is no composer to type a message into while a thread is blocked. The panel's "Cancel" resolves the interaction rather than dismissing it. The app's delivery handling is therefore defence in depth for the send/ask race, not a reproduced user-facing bug.

Independent verification

Rebased onto main (7655bad4c) on 2026-08-24: migration renumbered to 0108, SDK bump moved to 0.4.17, buildExecutionOptions call ported to the moved queued-messages.ts. The figures below describe the pre-rebase head 369b625d9.

Verified round 2 (head 369b625d9) in a separate worktree by an independent agent.

Commands run

  • git fetch origin main && git fetch origin bb/fix-1650-blocked-thread-messages && git checkout -b verify-1650-r2 FETCH_HEAD; git merge --no-commit origin/main (main at 2ff85986e, two commits past the PR base): clean merge, no conflicts.
  • pnpm install --frozen-lockfile --prefer-offline && pnpm exec turbo run build.
  • cd packages/db && pnpm exec drizzle-kit generate: "No schema changes, nothing to migrate", so 0105_snapshot.json matches schema.ts (not hand-edited).
  • pnpm exec turbo run typecheck --continue (whole workspace): 74/74 pass.
  • pnpm exec turbo run typecheck test --continue --force --filter=@bb/server --filter=@bb/cli --filter=@bb/db --filter=@bb/sdk --filter=@bb/server-contract --filter=bb-plugin-provider-retry --filter=@bb/templates: server 195/196 test files pass (the one failure is internal-skill-trees.test.ts, the known local umask 0664-vs-0644 difference; it passes in CI), cli 48/48, db 29/29, sdk 6/6, server-contract 7/7, provider-retry 2/2, templates 6/6.
  • eslint and prettier --check on the changed source/test files: clean.

Fail-before / pass-after

  • git checkout origin/main -- apps/server/src/routes/threads/actions.ts apps/server/src/services/threads/parent-system-messages.ts apps/server/src/server.ts apps/server/src/services/interactions/pending-interactions.ts (new modules and db layer left in place so the tests compile), then pnpm exec vitest run test/threads/deferred-thread-messages.test.ts test/public/public-thread-interactions.test.ts in apps/server: 7 failed / 25 passed. Failing assertions: AssertionError: expected 409 to be 200 (x5, the held sends), expected 409 to be 400 (sender validation before the guard), expected false to be true (queueParentSystemMessage still returned false for a blocked parent).
  • git checkout HEAD -- <same files> and rerun: 32/32 pass.

Repro on the fixed branch (own dev instance, ports 17012/25012/33012, claude-code haiku, native AskUserQuestion)

  • Orchestrator thr_jfcrwfpak2 parked on pint_w3eur8n8j4 ("Proceed?"). While blocked: 11 steer-if-active sends (bb thread tell, raw curl, SDK), one --mode auto, two --mode queue, and one mode=start. Results: every steer/auto returned {"ok":true,"delivery":"deferred"} and the CLI printed "awaiting user interaction; message held and delivers once the interaction settles" (--json carries delivery); --mode queue returned delivery: "queued" and showed in bb thread queue list; mode=start returned HTTP 409 awaiting_user_interaction.
  • Spawned a real child (thr_a2nxkwgvwq, parent = orchestrator, "Reply only with ok."). After it went idle a parent-system row (child-completed) appeared in deferred_thread_messages (14 rows total).
  • bb thread interactions answer ...: within ~14 s the table was empty, the dev log had 14 "Delivered deferred thread message" lines and no "failed"/"Dropped" lines, and client/turn/requested rows seq 21-38 carried all 12 tells in arrival order (target.kind=steer/auto) plus the initiator=system, systemMessageKind=child-completed notice; the two queued tells drained as new-turn (seq 63, 83) once the thread went idle. The model's follow-up reply: "Thread thr_a2nxkwgvwq completed successfully. All worker reports received (A, C, D, E, F, G tasks done, plus probes and debug reports)." On main the same steps lose every one of these (report section 4a).

CI: all checks green on 369b625d9 (Checks, Package Smoke x2, Tests app-1/2/3, integration, packages, server, version checks).

Residual risks / notes (none blocking)

  • qa/manual-runbook.md says "--mode start is still rejected"; bb thread tell only exposes steer/queue/auto, so that sentence describes the raw API mode: "start", not a CLI flag. Doc nit.
  • Version skew: an older CLI (for example the daemon-bundled bb from a prior release, reached via BB_CLI re-exec) against this server prints "Thread X steered" for a message that was actually held, because it ignores delivery. CLIs ship with the server, so this only matters mid-upgrade.
  • A held row that fails at flush with a non-terminal error (409 environment unavailable, 422 plugin mention, 502 host away) retries on every sweep with a warn log and holds later rows for that thread, matching the queued-message sweep's behavior; 400/404 rows are dropped with a warn.
  • If an interaction is registered in the microseconds between the flush's pending check and the send, the row is re-deferred at the tail of the thread's held list, so order relative to later rows can invert in that narrow race.

AGENT GENERATED: by Claude Opus 5

Fixes #1650

@SawyerHood
SawyerHood marked this pull request as ready for review August 21, 2026 03:15
@SawyerHood
SawyerHood force-pushed the bb/fix-1650-blocked-thread-messages branch from 369b625 to 5c8bbca Compare August 24, 2026 23:44
SawyerHood and others added 4 commits August 24, 2026 18:40
A thread blocked on an AskUserQuestion, a command approval, or a plugin
input request cannot take a prompt. The /send route refused every mode
with 409 and persisted nothing, and queueParentSystemMessage returned
false silently, so bb thread tell reports and child-completed notices
addressed to a blocked orchestrator vanished with no trace on the
recipient side (#1650).

Hold them in a new deferred_thread_messages table instead. Sends (every
mode but start) return { ok: true, delivery: "deferred" }, parent
system messages are stored with their taxonomy, and a settle hook on the
pending-interaction lifecycle plus a periodic sweep deliver them in
arrival order and in the requested mode once the thread unblocks. The
send policy moves into acceptThreadSendRequest so the route and the
flush share one decision; createQueuedMessageForThread moves next to the
rest of the queue service. The CLI prints the held outcome, and the
guide and bb-cli skill tell agents not to resend.

Co-Authored-By: Claude <noreply@anthropic.com>
The provider-retry plugin's send mocks still returned { ok: true }, which
no longer satisfies the widened SendMessageResponse and failed the
workspace typecheck.

A deferred row whose request can no longer be honored (its sender thread
was deleted, its attachment is gone) used to be retried on every sweep
and, because flushes stop at the first error to keep arrival order,
blocked every later held message for that thread. A 400/404 from the
send pipeline is now terminal for that row: it is deleted with a warn
log and the flush continues. Stopping threads (409), plugin mentions
(422), absent hosts (502) and timeouts still retry.

Also update the manual runbook line that still described tells as
rejected while a thread awaits user interaction.

Co-Authored-By: Claude <noreply@anthropic.com>
runDeferredThreadMessageSweep drove every thread that held a row, and the
flush treated every non-400/404 error as transient. A thread that failed
while a message was held (a provider exit or a host-daemon restart while
an AskUserQuestion is open interrupts the interaction and applies
run.failed) sits in `error` until somebody retries it, and a steer
refuses with 409 `thread_not_writable` there. The sweep ticks every ten
seconds, so one held tell produced one full send-pipeline call and one
warn per tick, forever. That is the pattern #1789 removed from the
queued-message sweep, whose selection this one claimed to match but did
not: listIdleThreadsWithQueuedMessages filters to idle threads with a
live environment.

The sweep now visits only threads that can act on their rows. A thread in
`error`, `starting` or `stopping` is left alone: its rows are not lost,
they wait for the status change (a user retry, a start that lands, a stop
that finishes) and the tick after it delivers them. Threads whose rows
can never deliver — archived, deleted, or an environment that is gone and
so is never reprovisioned — are listed separately and their rows dropped
once with a warn naming the reason, instead of living until somebody
archives the thread. The flush applies the same gone-environment rule, so
the settle path and the sweep agree.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The send route answers `{ ok: true, delivery }`, but the app's mutation
discarded the response, so a `deferred` send — the server holding the
message until the thread's open question or approval settles — was
treated as an accepted turn: the thread was flipped to active with a
working indicator for a turn that never started, and, with realtime
down, the timeline was invalidated and the refetch dropped the optimistic
message row, so the message looked lost.

The mutation now returns the response (an older server that answers a
bare `{ ok: true }` reads as `sent`), and a `deferred` delivery restores
the thread record, keeps the optimistic row — it is the only place the
held message is visible until the real event replaces it — and leaves the
timeline queries alone. The composer says the message is held and must
not be sent again.

Note on reachability: every composer the app renders is already swapped
out for the pending interaction, verified live against a Claude Code
thread parked on a native AskUserQuestion and a Codex thread parked on
the ask-user-question plugin's interaction. This closes the window where
a send and an interaction register at the same moment, and stops the app
ignoring a field of the contract it consumes.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@SawyerHood
SawyerHood force-pushed the bb/fix-1650-blocked-thread-messages branch from 5c8bbca to 4645b23 Compare August 25, 2026 01:43
@SawyerHood
SawyerHood merged commit fe58ab4 into main Aug 25, 2026
15 checks passed
@SawyerHood
SawyerHood deleted the bb/fix-1650-blocked-thread-messages branch August 25, 2026 02:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Messages to a thread blocked on AskUserQuestion are dropped, and only the sender is told

1 participant