Repository navigation
Keep manager responses visible across child lifecycle steers - #2071
Closed
ryanbbrown wants to merge 1 commit into
Closed
ryanbbrown wants to merge 1 commit into
ryanbbrown wants to merge 1 commit into
Conversation
SawyerHood
added a commit
that referenced
this pull request
Aug 21, 2026
## What was wrong When a Claude Code Stop hook blocks a stop, Claude Code injects the hook's reason as a synthetic user message and the model answers again. That synthetic message never becomes a bb thread event (`provider-claude-code` only forwards `tool_result` user messages), so one bb turn ends with two `agentMessage` items back to back: the real answer, then a one-line reply to the hook. Completed-turn grouping in `@bb/thread-view` keeps exactly one terminal message per segment (`findLastTerminalTimelineMessage`) and slices everything before it into the collapsed `Worked for` summary. The real answer, including any question for the user, disappears behind a row that reads like a duration stat, in the app and in `bb thread log`. Issue: #1355. Report: https://get-bb.github.io/reports/issues/1355.html (reproduced live on a fresh instance from 16ceb3a; the code at the root cause is unchanged on current main). ## What changed `packages/thread-view/src/completed-turn-grouping.ts`: an assistant text that is directly followed by more assistant text, with no work in between, is a complete response, not narration about upcoming tool activity. `groupCompletedTurnSummaryMessages` now promotes such messages to `ungrouped-message` items (the terminal message counts as the follower of the last summary message). The existing collapse is unchanged for text followed by tool activity, so ordinary `text -> tool -> text` turns still render as `user -> Worked for -> last answer`. Behavior for the issue's shape (`answer, hook reply`): both assistant rows are visible at rest and no summary row is emitted. For `narration, command, answer, hook reply`: `Worked for` holds the narration and the command, then the answer and the hook reply are both visible. Deviation from the report's primary proposal: the report suggests surfacing every top-level assistant text. That changes every agentic turn (interim narration such as "Let me check X" would leave the summary), which #320 deliberately collapses, and #1508's review measured a 1,500-event page producing ~1,500 rows (past the 200-row completed-timeline cache limit). This change is scoped to the shape that loses the answer. Remaining gap: a hook that makes the model call a tool before answering (`answer, [hook], tool, reply`) still folds the answer, because nothing in the event stream marks the boundary; fixing that needs `provider-claude-code` to surface the synthetic hook message through the bridge protocol, which is a contract change and a separate PR. No wire change. The app and `bb thread log` share this row builder, so both pick it up. ## How you verified New tests, which fail on origin/main and pass with this change: - `packages/thread-view/test/completed-turn-grouping.test.ts`: `keeps an assistant response visible when more assistant text follows it directly`, `folds narration that precedes work and keeps the response that precedes the terminal text`, `keeps every response in a run of adjacent assistant texts`. Two existing tests that encoded the old behavior with two adjacent assistant texts now use a command message as the folded work. - `packages/thread-view/test/completed-turn-summary-rendering.test.ts`: `keeps an assistant answer visible when the provider re-queries and the model answers again`, `folds narration before work but keeps the answer that precedes a hook reply`. With `completed-turn-grouping.ts` checked out from origin/main the rendering test fails with: ``` - Expected + Received [ "conversation:user", - "conversation:assistant", + "turn:4-4", "conversation:assistant", ] ``` Commands: - `pnpm exec turbo run typecheck --filter=@bb/thread-view --filter=@bb/server --filter=@bb/app --filter=@bb/cli --filter=@bb/client-core`: 8/8 pass. - `pnpm exec turbo run test --filter=@bb/thread-view --filter=@bb/server --filter=@bb/cli --filter=@bb/client-core`: thread-view 21/21 files, cli 48/48, client-core 20/20, server 194/195 (the one failure is `internal-skill-trees.test.ts` mode 0644 vs 0664, a pre-existing local umask difference unrelated to this change). - `apps/app` `vitest run src/components/thread/timeline`: 28/28 files. Manual: on a dev instance built from this branch, a `claude-code` thread in a scratch repo with a blocking Stop hook (the report's 4a repro) produced events `agentMessage, agentMessage, turn/completed`. `bb thread log` now prints the three-bullet answer with its closing question as an `Assistant` row, followed by the hook reply as a second `Assistant` row, with no `Worked for` row; the app shows the same two rows at rest. Fixes #1355 > AGENT GENERATED: by Claude Opus 5 ## Independent verification Verified by a second agent on a fresh checkout of this branch (`40224ee60`, origin/main is an ancestor; no conflicts) in its own worktree and dev instance. Commands: - `pnpm install --frozen-lockfile --prefer-offline && pnpm exec turbo run build` - Fail-before: `git checkout origin/main -- packages/thread-view/src/completed-turn-grouping.ts`, then `pnpm exec vitest run test/completed-turn-grouping.test.ts test/completed-turn-summary-rendering.test.ts` in `packages/thread-view`: 5 failed / 19 passed. The five new tests are exactly the ones that fail. Rendering test assertion: ``` - Expected + Received [ "conversation:user", - "conversation:assistant", + "turn:4-4", "conversation:assistant", ] ``` Grouping test: `expected [ { kind: 'summary', …(5) } ] to deeply equal [ { kind: 'ungrouped-message', …(1) } ]`. - Pass-after: restored the PR file, same command: 24/24 pass. - `pnpm exec turbo run typecheck --filter=@bb/thread-view --filter=@bb/server --filter=@bb/cli --filter=@bb/client-core --filter=@bb/app --filter=@bb/mobile`: 9/9 successful. - `pnpm exec turbo run test --filter=@bb/thread-view --filter=@bb/server --filter=@bb/cli --filter=@bb/client-core`: thread-view 21/21 files (385 tests), cli 48/48 (453), client-core 20/20 (238). See the server result note below. - Ad-hoc probes through `renderTimelineFixture` (not committed): `reasoning, text, reasoning, text` (extended thinking between the answer and the hook reply) still renders both assistant rows at rest; `text, tool, text` still collapses to `user -> Worked for -> assistant` (the deliberate #320 behavior); a steer between two answers keeps the existing per-segment preservation; `text, text, text, tool, text` shows the first two, folds the third with the tool. Repro on the fixed branch (report section 4a, real `claude-code` turn with a blocking Stop hook on a scratch repo, own dev instance): persisted events were `item/completed agentMessage` (seq 22), `item/completed agentMessage` (seq 29), `turn/completed` (seq 32) with no hook event between, the same shape as the report. `bb thread log` (default minimal format) printed `User`, `Provisioned thread`, then the full three-bullet answer with its closing question as an `Assistant` row, then the hook reply as a second `Assistant` row, with no `Worked for` row. `GET /api/v1/threads/:id/timeline` (what the app renders) returned `conversation:user, system:operation, conversation:assistant, conversation:assistant` and no `turn` row. The bug no longer reproduces. The provider still drops the synthetic hook message (`plugins/provider-claude-code/src/delta-translation.ts` `translateUserMessage` returns `[]` when there are no tool results), so the fix is entirely in the shared row builder, as described. CI: all required checks green on the PR head (Checks, Tests server/packages/integration/app-1..3, Package Smoke ubuntu+macos, version check). Residual risks (agreed with the PR body): `answer, [hook], tool, reply` still folds the answer because no event marks the hook boundary; that needs `provider-claude-code` to surface the synthetic user message (wire change, separate PR). Any provider that emits two adjacent assistant items for one response now shows both rows. `answer, answer2, error` shows `answer` at rest but still folds `answer2` before the error row (pre-existing behavior for error terminals, not made worse). Textual merge overlap with #2071 in the same loop of `completed-turn-grouping.ts`. > AGENT GENERATED: by Claude Opus 5 Co-authored-by: Claude <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.
What was wrong
A child can send a detailed report while its parent manager turn is active. The parent can then show a complete assistant response before BB accepts the child's normal lifecycle notification as a system auto-steer in the same turn. When the turn completes, that visible response can disappear.
The failure has two connected causes:
Worked forsummary.message_start. If Pi emitted two assistant messages in one steered turn with no tool boundary, both could use one canonical item ID. The later completion could replace the earlier text, and resolved-delta pruning could then remove the only canonical copy of the first response.The first cause affects completed timelines from all providers. The second affects Pi and makes this a high-severity canonical-data-loss bug, not only a display bug. Histories already pruned before this fix are not repaired; a native provider transcript can still contain text that BB's canonical event history no longer has.
Minimal reproduced order:
child-completedas a system auto-steer in that turn.Before this change, the first response became collapsed or was lost from canonical Pi history. It must remain a top-level row with its own item ID.
What changed
child-needs-attention,child-completed,child-failed,child-interrupted, andchild-outcome-batchsystem steers as visible exchange boundaries.message_start. The shared assembler then gives later assistant output a new canonical item ID. Retry and empty-abort boundaries retain their existing safe behavior.HOST_DAEMON_PROTOCOL_VERSIONfrom 143 to 144 because daemon-emitted canonical events can now use separate Pi assistant item IDs.This does not weaken pruning and does not add a compatibility fallback. It is not retroactive: it prevents new loss but cannot reconstruct canonical text that an older daemon already merged and pruned.
How you verified
Tests added for this change fail on the old behavior and pass with the fix.
pnpm exec turbo run build typecheck lint --output-logs=new-only: 88/88 tasks passed afterpnpm install --frozen-lockfilerestored missing managed-worktree dependencies; the lockfile did not change.pnpm exec turbo run test --output-logs=new-only: 68/68 tasks passed. The app result was 402 files, 3,096 passed tests, and 3 skipped tests.PromptBoxInternal.test.tsxtimeout. The full file then passed three forced runs at 101/101 tests each, and the full repository rerun passed.bb-apptarball smoke passed. The tarball smoke ran 12/12 tasks.git diff --checkpassed. Every changed file passes Prettier.pnpm run format:checkstill stops on the pre-existing unquoted-colon parse errors in.github/ISSUE_TEMPLATE/bug.yml:71and.github/ISSUE_TEMPLATE/feature.yml:42; neither file is changed by this PR.Disposable live QA used a separate SQLite data directory and ports 63894-63896. It used production Pi translation and assembly with synthetic Pi 0.84 events, real persistence and pruning, the public timeline route, and the React app. No provider API was called. The exact sequence remained top-level before completion, after completion, after full browser/timeline refetch, after pruning two resolved deltas, and after a full disposable server/app restart. The assistant row IDs remained distinct:
...|turn:qa-pi-t1|parent:root|item:qa-pi-i1...|turn:qa-pi-t1|parent:root|item:qa-pi-i2The accepted unlabeled reconnect control remained folded. Browser page errors were empty. QA did not exercise the process-local two-second notification batch or live WebSocket ingestion because fixture insertion does not publish realtime hub events; automated tests cover the accepted lifecycle event producer shape.
Related work
Related but not duplicate: #1355, #1656, #1657, #1981, #897, #1224, and #1834.