Give the dispatch hook a stable answer for who sent a message - #3816
Merged
ymichael merged 9 commits intoSep 17, 2026
Merged
Conversation
A `message.dispatch` handler could not tell a message a user typed from one thread sending another. The context's only provenance fields were `startedOnBehalfOf`, which describes why a THREAD was started and is null on every send, steer, drain and retry, and `queuedMessage`, which carries the author but exists only on a re-attempt. So the sender was visible on the second attempt of a message and invisible on the first. The value was already computed twice per request — once in `thread-send` for the turn event and permission escalation, once in `thread-provisioning` for a thread start — and thrown away before the checkpoint ran. Hoist that rule into `resolveDispatchAuthor`, next to the escalation rule it feeds, and read it at the checkpoint from the sender the attempt has already resolved. Both existing derivations now call it, so the checkpoint cannot drift from the pair the dispatched turn is recorded with. The context gains `initiator` and `senderThreadId`, matching the names and enum `ThreadQueuedMessage` already uses; `startedOnBehalfOf` is unchanged and now documents what it does not answer. Correct on every path with no per-path branching: a drain already puts the row's sender into the payload, a retry carries `retryOf`, and a first dispatch carries `startedOnBehalfOf`. Fixes #3781 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two sites still worked out "did a user send this?" by hand: the thread-create telemetry gate, which read `startedOnBehalfOf?.initiator ?? "user"`, and the message edit, which read `senderThreadId === null ? "user" : "agent"`. Both answer the question `resolveDispatchAuthor` was extracted to answer, and both predate it. Neither changes behaviour: `startedOnBehalfOf.initiator` is `agent | system` by schema, so the create gate agrees with the rule on every input, and the edit is branches two and four of it verbatim. The point is that a fourth copy of a rule is a fourth thing to forget when the rule moves. `toThreadQueuedMessage` keeps its own derivation. That one reads a stored row — a system notice makes it `system` — which is not a question this rule's arguments can express. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A `message.dispatch` handler is promised the same answer every time one message is decided, however many attempts that takes. Three context fields broke that promise in the opposite direction to the sender bug: `origin`, `originPluginId` and `startedOnBehalfOf` came from the request, so the first attempt saw values and a drained re-attempt saw null. A policy keyed on them was right on an idle host and quietly wrong under load, which is when the queue exists. The author was not safe either. Its fix carried the sender on the row, and a message sent to a thread has one — but a thread-start does not. Its author comes from `startedOnBehalfOf`, which nothing persisted, so a spawned thread's first message reported `agent` inline and `user` once drained. The shipped drain test only covered the send path. Four columns on `queued_thread_messages`, written in the same insert as the wait and read back by the drain: `origin`, `origin_plugin_id`, `requested_by_initiator` and `requested_by_thread_id`. The requester is its own pair rather than reusing `sender_thread_id`, which drives the agent message prefix and would have rewritten the drained message's text; and it is not a bare `initiator` column, which would contradict the one `toThreadQueuedMessage` derives on the same row for three row kinds out of five. Half a pair is a write-side bug and fails loudly. `ThreadCreateOrigin` and `StartedOnBehalfOf` move to `@bb/domain`, which is where the values a column stores already live: the alternative was typing two enum columns as `string`. `startedOnBehalfOf` leaves `MessageDispatchHookContext` in the same change. Every bit it carried is now in `initiator`, `senderThreadId` and a stable `origin`, and leaving it would leave a field whose name answers why a THREAD was started next to fields that answer who sent this message. Core still sets it on the context object for handlers built against an older SDK; the audit entry says to drop that shim before stabilizing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The drain claims several queued rows and sends them as one turn, but the hook was handed `claimed[0]`: a plugin deciding whether a send may proceed saw one message and decided for all of them. A hold a plugin had placed on the second row was invisible, so the row it was holding went out with the group — for `drafts`, that means text the user never sent reaching the provider. `queuedMessage` becomes `queuedMessages`: every claimed row, in dispatch order, each with its own content and author. Core still emits the first row under the old name so a handler built against an older SDK keeps working; the audit entry says to drop that shim before stabilizing. `drafts` now asks whether any row in the group is one of its holds, which is the shape every hook needs, not a `drafts` special case. `initiator` and `senderThreadId` summarize the group, and a group need not agree: both report `mixed` when its rows differ, rather than presenting the first row's author as if it described all of them. Individual rows never report `mixed`, so a handler that cares reads `queuedMessages`. The row's own author now comes from the same rule as everything else, which it had to for the summary to mean anything: a queued thread-start reports the thread that asked for it instead of reading as one the user typed. A retry reports `system`, which is what the retried turn has been recorded as since long before this branch; the queue row was the only place still calling it a user message. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ymichael
force-pushed
the
bb/explain-startedonbehalfof-dispatch-hook-thr_wfbvn5e5ri
branch
from
September 17, 2026 21:42
4987eb3 to
0cf5b4c
Compare
`threadQueuedMessageSchema` gained `origin` and `originPluginId` as required nullable fields. `makeThreadQueuedMessage` was updated; three objects built by hand were not, so they failed to parse or to typecheck. The domain schema test gets them on both objects, including the one asserted to throw: without them it would have thrown for the wrong reason and stopped proving what its name claims. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The row reports `initiator: "system"`, which the drawer turned into "From System". That reads as an author, and a retry has none: it re-sends a turn someone else already wrote, and the wait line beside it already says what re-sent it and why — "Rate limited · retrying at 2:58 PM · attempt 2". Suppressed in the drawer rather than in `toThreadQueuedMessage`, because the `system` the row reports is correct and a `message.dispatch` handler needs it. Only the label is wrong, so only the label changes. A system notice keeps "From System": there BB did write the text. No height change — `queuedMessageHasWaitLine` is already true for a retry, so the second line was reserved either way. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ymichael
deleted the
bb/explain-startedonbehalfof-dispatch-hook-thr_wfbvn5e5ri
branch
September 17, 2026 22:32
albrand
added a commit
to albrand/bb
that referenced
this pull request
Sep 18, 2026
Resolved 12 conflicts with fork behavior kept and upstream API shape taken: dispatch-attempt keeps the fork file plus upstream get-bb#3816/get-bb#3878 deltas; openThreadInSplit keeps fork side/SplitOpenResult and adds upstream state; plugin tools keep fork waitsForUserInput on upstream PluginRowPresentation; guide chapters union spend + json; contract optional-group limit 46. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Human comments
What was wrong
A
message.dispatchhandler could not tell a message a user typed from one thread sending another, and the provenance it did get changed between attempts of the same dispatch. The context's only sender-ish field wasstartedOnBehalfOf, which answers why a THREAD was started and is null on every send, steer, drain and retry; the author was otherwise only reachable throughqueuedMessage, which exists only on a re-attempt. So the sender was invisible on a message's first attempt and visible on its second. In the other direction,origin,originPluginIdandstartedOnBehalfOfwere read from the request, so a drained re-attempt saw null where the first attempt saw values — a policy keyed on them was right on an idle host and quietly wrong under load, which is when the queue exists. Fixes #3781.What changed
resolveDispatchAuthor(apps/server/src/services/threads/thread-runtime-config.ts) is the single derivation of(initiator, senderThreadId): retry →system, sender thread →agent,startedOnBehalfOf→ its initiator, elseuser. The value was already computed inline inthread-send(turn event + permission escalation) andthread-provisioning(thread start) and thrown away before the hook ran; both now call it, as do the thread-create telemetry gate andthread-edit-message, which each had their own copy. No behaviour change at those four sites — they agree with the rule on every input.queued_thread_messages—origin,origin_plugin_id,requested_by_initiator,requested_by_thread_id— written in the same insert as the wait and read back by the drain (migration0126_overconfident_vin_gonzales.sql). The requester is its own pair rather than a reuse ofsender_thread_id: that column drives the[bb message from thread:…]prefix and would have rewritten the drained message's text. Half a pair throws rather than degrading to null.MessageDispatchHookContextgainsinitiator: ThreadTurnInitiatorandsenderThreadId: string | null;origin/originPluginIdkeep their types but are now stable across re-attempts.startedOnBehalfOfleaves the type — every bit it carried is now in those three fields, and its name answers a different question. Core still sets it on the emitted object for handlers built against an older SDK, via a named shim indispatch-hooks.ts;docs/api_to_audit.mdrecords that the shim must go before stabilization.ThreadCreateOriginandStartedOnBehalfOfmove from@bb/server-contractto@bb/domain, becausepackages/db/src/schema.tsneeds the enum values — the alternative was typing two enum columns asstring.backend-events.md(Plugin Guide reference),plugin-api-map/src/surfaces.tsbullets, and themakeMessageDispatchHookContextfixture updated. No wire change:HOST_DAEMON_PROTOCOL_VERSIONis untouched, as nothing crosses the server/daemon boundary.How you verified
Eight new tests in
apps/server/test/threads/dispatch-hooks.test.ts: the four author paths (user, agent, retry, and the sender named on a re-attempt), the drained-send and drained-thread-start author-stability cases, drainedoriginstability plus a follow-up send that must stay origin-less, and the older-SDKstartedOnBehalfOfshim. The two drain tests fail before this change — thread-start reportsagentinline anduseronce drained, which the shipped drain test missed because it only covered the send path.pnpm exec turbo run typecheck lint --filter=@bb/db --filter=@bb/domain --filter=@get-bb/plugin-sdk --filter=@bb/server --filter=@bb/plugin-api-map --filter=@bb/server-contract— 11/11 passpnpm exec turbo run test --filter=@bb/db --filter=@bb/domain --filter=@get-bb/plugin-sdk --filter=@bb/plugin-api-map --filter=@bb/server-contract— pass, includingmigrate.test.ts(57 tests) with the new rewind coverage for the four columnspnpm exec turbo run test --filter=@bb/server— 2920 pass. Two failures under the full parallel run, both green in isolation:test/services/machines/lifecycle-recovery.test.tstimed out at 5s under load (unrelated area), anddispatch-hooks.test.tsneeded the fork source to announce a provider session after rebasing onto Keep resumed threads in their own provider sessions #3512, now seeded the way the passing fork tests do.node .github/workflows/check-plugin-sdk-version.mjs— passes with the 0.4.100 bumpFixes #3781