Fix Pi custom message turns - #1682
Closed
ryanbbrown wants to merge 1 commit into
Closed
ryanbbrown wants to merge 1 commit into
ryanbbrown wants to merge 1 commit into
Conversation
ryanbbrown
force-pushed
the
bb/fix-unreliable-pi-process-wake-thr_ckq2fgshmf
branch
from
August 17, 2026 23:55
930fb12 to
53ae256
Compare
| }); | ||
| const fallbackTurnId = resolvePiActiveTurnId(context); | ||
| return translated.length > 0 | ||
| return translated.length > 0 || isProcessNotification |
Contributor
There was a problem hiding this comment.
🚨 slopcop/review — [P1] This event-stream change needs a daemon protocol increase.
This condition changes daemon output from provider/unhandled to new turn and item events. Older daemons will keep the old behavior.
The repository rules require a HOST_DAEMON_PROTOCOL_VERSION increase for this semantic wire change. Increase the version and update its contract test after the rebase.
| if (startsProcessTurn) { | ||
| state.processNotificationTurnId = turnId; | ||
| } | ||
| events.push({ |
Contributor
There was a problem hiding this comment.
🚨 slopcop/review — [P1] The process input remains invisible.
The translator emits a userMessage item, but the projector does not handle provider userMessage items. It only creates visible user rows from client/turn/requested.
The production timeline therefore drops this content. The new test checks only translation output, so it cannot detect this failure.
Add a supported projection path. Add a timeline test that proves the process text appears once.
| } | ||
|
|
||
| const events: ThreadEvent[] = []; | ||
| const startsProcessTurn = |
Contributor
There was a problem hiding this comment.
🚨 slopcop/review — [P1] The warning marker uses the wrong Pi event order.
Pi 0.84 emits agent_start before each prompt message_start. The first event opens the turn before this check runs.
Thus, startsProcessTurn stays false, and processNotificationTurnId stays undefined. The empty-response warning cannot run in the real event order.
Use the upstream order in the tests: https://github.com/earendil-works/pi/blob/a5f43bf8aff3c55752432655f7334e3dafd1e256/packages/agent/src/agent-loop.ts#L95-L114
Contributor
There was a problem hiding this comment.
🚨 SLOP COP 🚨 · review
Plain English summary: This PR tries to show an automatic Pi process notice as the input for one normal turn. It also completes that turn and warns when Pi returns no text.
I found three issues that block the merge:
- P1 — The process input remains invisible. The translator emits a provider
userMessage, but the final projector does not create a visible row from it. The current test checks only the raw event. - P1 — The warning logic uses the wrong event order. Pi 0.84 emits
agent_startbeforemessage_start. The turn is already open, so the process-turn marker never activates. - P1 — The daemon protocol version stays unchanged. This PR changes events that the daemon sends to the server. The repository rules require a protocol increase and contract-test update.
Please rebase first. GitHub reports a conflict because the stacked #1663 change already reached main. The rebase should remove that duplicate commit.
Security, performance, and architecture checks found no separate defect. The codebase has no duplicate process-notification translator, so a broad refactor has no clear value.
The package, app, server, integration, Linux smoke, and macOS smoke tests passed. The Ubuntu Checks job failed on an unrelated plugin SDK npm version guard.
I did not run code from this fork. A browser alone cannot create this Pi provider event. Add a projection test, then repeat the live Pi process test with the real event order.
I posted this review as COMMENTED, as required. I did not approve the PR or request changes through GitHub.
ryanbbrown
force-pushed
the
bb/fix-unreliable-pi-process-wake-thr_ckq2fgshmf
branch
from
August 19, 2026 21:15
53ae256 to
ba86544
Compare
SawyerHood
added a commit
that referenced
this pull request
Aug 21, 2026
## What was wrong
A Pi extension can inject a custom message and trigger a turn on its own
(`pi.sendMessage(..., { triggerTurn: true })`; this is how
`@aliou/pi-processes` wakes a thread when a background command
finishes). Pi emits `agent_start`, then `message_start`/`message_end`
with `role: "custom"` for that message. bb's Pi translator had no case
for custom-role boundaries and its visibility metadata rated the role
`unknown`, so both envelopes surfaced as `provider/unhandled`
("Unhandled Pi event" rows in dev builds or with the setting on), and
the message itself was never recorded. The extension-triggered turn
therefore showed an assistant answer with no input in front of it, in
the app and in `bb thread log`. bb also had no grammar for
provider-originated input at all: the narrow-grammar `thread/delta` has
no delta for it, and nothing in `@bb/thread-view` projected a
`userMessage` item.
The second half of the issue (Pi's `agent_end.messages` carrying string
content, which stranded the turn as "Working...") already landed in
#1663.
Issue: #1681. Report: https://get-bb.github.io/reports/issues/1681.html
PR #1682 attacks the same gap but was written against
`event-translation.ts`, which #1834 replaced with
`delta-translation.ts`; it no longer applies to main. This PR implements
the equivalent on the narrow-grammar path and keeps the generic `role:
"custom"` handling the report asked for.
## What changed
- `packages/provider-bridge-protocol/src/thread-delta.ts`: new
`input.provider` delta (`text`, optional `parentRef`) for input the
provider injected without a bb client request. Additive, so no bridge
protocol version change; the G3 grammar guardrail snapshot
(`provider-bridge-grammar.v2.snapshot.json`) gains the new kind.
- `packages/provider-bridge-protocol/src/assembler/delta-assembler.ts`
(moved there from `@bb/agent-runtime` on main): `input.provider` records
an `item/completed` `userMessage` item (assembler-minted id) in the open
turn; with no turn open it is dropped, because Pi appends idle
`attention: context` notes to its own context without running the agent
and there is no bb turn to attach them to.
- `packages/agent-runtime/src/pi/delta-translation.ts`: parses
`message_start`/`message_end` for `role: "custom"` (any `customType`,
string or block-array content). A displayed `message_start` becomes
`input.provider`; `message_end`, hidden messages, and empty text
translate to nothing.
- `packages/agent-runtime/src/pi/visibility.ts`: custom-role boundaries
are `noise`, so the silent cases never reach the unhandled fallback.
- `packages/thread-view/src/user-message-parsing.ts`,
`build-event-projection.ts`: project the `userMessage` item as a
system-initiated accepted steer of its turn. It renders as the existing
"System Message" row in the app (inside the turn's "Worked for" group in
summary mode) and as a `User` row in `bb thread log`. It is a `steer`,
not a `message`, on purpose: the server pages the timeline on `message`
rows backed by stored `client/turn/requested` events
(`timelineSegmentAnchorConditions` in `@bb/db` vs
`isTimelineSegmentAnchorRow` in `timeline-pagination.ts`). With a
`message` row here the latest page silently dropped every earlier turn
and reported `hasOlderRows: false`; I hit this live before switching.
- `HOST_DAEMON_PROTOCOL_VERSION` 151 -> 152 (146 -> 147, then 150 ->
151, before two rebases onto a moving main): the daemon now sends a
`userMessage` item it never emitted before. The shape already existed in
the shared schema, so the bump is for the semantic change and to roll
the fix to enrolled daemons.
- CI guard: this PR no longer carries an `@get-bb/plugin-sdk` version
change. `thread-delta.ts` and the assembler are bundled into the SDK's
published provider-bridge entry points, so the npm version guard
(`check-npm-version-guard.mjs`) needs an unpublished version; `main` has
since moved the SDK to `0.4.13`, which npm has not published (npm latest
is `0.4.12`), so this PR adopts `main`'s version and the guard passes
with no further bump.
Not done, deliberately: the issue's "empty assistant output produces an
explicit warning". Nothing in bb promises that today and #1682's version
never fired on real Pi (it assumed `message_start` before
`agent_start`).
## How you verified
New tests, all fail on `origin/main` source and pass with the fix:
- `packages/agent-runtime/src/pi/delta-translation.test.ts`
- "records a displayed Pi custom message as the input of the turn it
triggered" (real order: `agent_start` -> `message_start` ->
`message_end`). On main: `AssertionError: expected [ Array(1) ] to
deeply equal [ { type: 'item/completed', ... } ]` with a
`provider/unhandled rawType: "sdk/message_start"` received.
- "joins the text blocks of an array-content Pi custom message"
- "drops hidden and idle Pi custom messages without surfacing them as
unhandled". On main: `expected [ Array(1) ] to deeply equal []`.
- `packages/thread-view/test/timeline-cli-rendering.snapshots.test.ts`
"shows provider-injected input as a system-initiated steer of its turn".
On main: `expected [ { initiator: 'user', ... } ] to deeply equal [ {
initiator: 'user', ... }, ...(1) ]` (no provider row projected).
- `apps/server/test/services/threads/timeline-provider-input.test.ts`:
latest page keeps the user's first turn, `returnedSegmentCount: 1`,
`hasOlderRows: false`, provider input nested in turn 2. Fails if the row
is projected as a `message` (first turn dropped: `expected [ ...(2) ] to
deeply equal [ 'user:Reply only with ok.', ...(3) ]`).
Commands, run from the committed tree (`git status --porcelain` empty):
- `pnpm exec turbo run typecheck --filter=@bb/provider-bridge-protocol
--filter=@bb/agent-runtime --filter=@bb/thread-view
--filter=@bb/host-daemon-contract --filter=@bb/host-daemon
--filter=@bb/server --filter=@bb/cli --filter=@bb/app
--filter=bb-plugin-provider-acp --filter=bb-plugin-provider-codex
--filter=bb-plugin-provider-claude-code` -> `Tasks: 15 successful, 15
total`
- `pnpm exec turbo run test --filter=@bb/provider-bridge-protocol
--filter=@bb/agent-runtime --filter=@bb/thread-view
--filter=@bb/host-daemon-contract --filter=bb-plugin-provider-acp
--filter=bb-plugin-provider-codex
--filter=bb-plugin-provider-claude-code` -> `Tasks: 11 successful, 11
total` (agent-runtime 31 files, thread-view 21 files, protocol 10 files)
- `pnpm exec turbo run test --filter=@bb/server` -> 195/196 files pass;
the one failure is `internal-skill-trees.test.ts` expecting file mode
0644 on a umask 0002 machine (pre-existing local-only failure, passes in
CI). `timeline-provider-input.test.ts` passes.
Manual, on my own dev instance with the report's 30-line stand-in
extension (same message shape as pi-processes 0.10.9,
`PI_CODING_AGENT_DIR` pointing at a trust-listed copy of the Pi agent
dir), real Pi session, prompt "Reply only with ok.":
```
16 turn/started turn da6731fd37-t2
17 item/completed turn da6731fd37-t2 userMessage [{"type":"text","text":"<process_event type=\"lifecycle\" kind=\"success\" process_id=\"proc_551c\" name=\"sleep-done\">Process completed ..."}]
18 item/started turn da6731fd37-t2 agentMessage
...
21 item/completed turn da6731fd37-t2 agentMessage "ok"
23 turn/completed turn da6731fd37-t2 status=completed
thread status: idle
```
No `provider/unhandled` events (main produced two for
`sdk/message_start`/`sdk/message_end`). `bb thread log` shows the
process event as a `User` row before the `ok`. The app shows the first
turn, then "Worked for 1s" containing a "System Message" row with the
process event, then `ok`.
Fixes #1681
> AGENT GENERATED: by Claude Opus 5
## Independent verification
Verified by a second agent on a fresh checkout (`git fetch origin
bb/fix-1681-pi-notification-wake && git checkout -b verify-1681-r1
FETCH_HEAD`, head `ce123f38e`; `origin/main` is an ancestor, and main is
still at protocol 146 so the 147 bump does not collide).
Fail-before / pass-after (checked out the `origin/main` versions of the
6 non-test source files, ran the new tests, then restored):
- `packages/agent-runtime` `vitest run src/pi/delta-translation.test.ts
-t "custom message"`: 3 failed on main. First assertion:
`AssertionError: expected [ Array(1) ] to deeply equal [ { type:
'item/completed', …(4) } ]`, received a `provider/unhandled` whose
`rawEvent.params.message.message.role` is `"custom"`. Third: `expected [
Array(1) ] to deeply equal []`. All 3 pass on the PR tree.
- `packages/thread-view` `vitest run
test/timeline-cli-rendering.snapshots.test.ts -t "provider-injected"`:
fails on main with `expected [ { initiator: 'user', …(2) } ] to deeply
equal [ { initiator: 'user', …(2) }, …(1) ]`; passes on the PR tree.
- `apps/server` `vitest run
test/services/threads/timeline-provider-input.test.ts`: fails on main
(`- "user:<process_event …>"` missing from the page); passes on the PR
tree. Also re-checked the guard: patching `parseProviderUserMessage` to
`kind: "message"` makes it fail with the first turn dropped (`expected [
…(2) ] to deeply equal [ 'user:Reply only with ok.', …(3) ]`).
Turbo, from the committed tree:
- `turbo run typecheck` for provider-bridge-protocol, agent-runtime,
thread-view, host-daemon-contract, host-daemon, server, cli, app,
provider-acp, provider-codex, provider-claude-code: `Tasks: 15
successful, 15 total`.
- `turbo run test --force` for provider-bridge-protocol (10 files),
agent-runtime (31), thread-view (21), host-daemon-contract (3),
provider-acp (14), provider-codex (16), provider-claude-code (18):
`Tasks: 11 successful, 11 total`.
- `turbo run test --filter=@bb/server --force`: 195/196 files; the one
failure is `internal-skill-trees.test.ts` (file mode 436 vs 420, local
umask 0002; passes in CI).
Repro on the fixed branch: own dev instance with `PI_CODING_AGENT_DIR`
pointing at a trust-listed copy of the Pi agent dir and the report's
30-line stand-in extension, real Pi session (`thread spawn --provider pi
--permission-mode full --prompt "Reply only with ok."`, thread
`thr_byejnv6paz`). Events: `16 turn/started t2`, `17 item/completed
userMessage <process_event …>`, reasoning, `agentMessage "ok"`, `27
turn/completed status=completed`, thread `status=idle`, zero
`provider/unhandled` (main produced two, for `sdk/message_start` and
`sdk/message_end`). The app shows the first turn intact, then "Worked
for 3s" which expands to a "System Message" row with the process event,
then `ok`. `bb thread log --format verbose` shows the nested `User` row
with the process event and `steer`.
CI at verification time: all ubuntu checks green (Checks, Package Smoke,
Tests app-1/2/3, integration, packages, server); macOS Package Smoke
pending.
Residual risks / notes for the reviewer:
- In the default views the provider input is hidden until expanded: the
app folds it into the collapsed "Worked for" group (existing policy for
system-initiated steers) and `bb thread log` in its default `minimal`
format prints an empty `── Worked for (3s)` header with no input row;
only `--format verbose` shows it. The PR body's "`bb thread log` shows
the process event as a `User` row" holds for verbose only. Making
provider input ungrouped without turning it into a pagination anchor
needs a product decision (a distinct initiator, or teaching the DB
anchor query about `userMessage` items).
- Idle `attention: context` notes (no open turn) are dropped, not
persisted; image blocks in custom messages are ignored.
- Linked PR #1682 is `mergeable=CONFLICTING` and edits
`event-translation.ts`, which #1834 deleted; it cannot land on main.
> AGENT GENERATED: by Claude Opus 5
## Stack
Layer 1/2 of GitHub stack #2217 (`gh stack`), lands first. Base `main`,
`HOST_DAEMON_PROTOCOL_VERSION` 151. Rebased onto main at `75d6fc4d4`
(protocol 150): the only conflicts were `protocol.ts` and
`contract.test.ts`; the rebase also regenerated the bridge grammar
snapshot for the new `input.provider` delta kind. Re-verified on the new
base: the three new test files fail with the six non-test source files
checked out from `origin/main` (`expected [ Array(1) ] to deeply equal [
{ type: 'item/completed', …(4) } ]`, `expected [ 'user:Reply only with
ok.', …(2) ] to deeply equal [ 'user:Reply only with ok.', …(3) ]`) and
pass on this head; `turbo run typecheck test` for host-daemon-contract,
host-daemon, agent-runtime, provider-bridge-protocol, thread-view and
server is green except the known local-only umask `internal-skill-trees`
assertion. #2142 (layer 2/2, protocol 152) is stacked on this branch.
> AGENT GENERATED: by Claude Opus 5
## Independent verification (guards)
Re-verified head `bdf47388f` (rebased onto `origin/main` `27d1017fe`,
`@get-bb/plugin-sdk` 0.4.12) against the previously verified head
`3158939df` on a fresh checkout (`verify-2154-g`).
- `git range-diff 75d6fc4..3158939 origin/main..bdf4738`: the
single commit differs only in
`packages/domain/src/plugin-sdk-version.ts` (`0.4.11` -> `0.4.12`) and
`packages/plugin-sdk/package.json` (`0.4.11` -> `0.4.12`). Every other
hunk is identical. `origin/main` and npm (`npm view @get-bb/plugin-sdk
version`) are both still at 0.4.11 and main is still at protocol 150, so
neither bump collides.
- Guards on this head: `node
packages/plugin-sdk/scripts/check-npm-version-guard.mjs` -> `PASS —
@get-bb/plugin-sdk@0.4.12 is not on npm yet`; `node
scripts/check-provider-literal-ratchet.mjs` -> `OK: 148 references
across 40 core files`.
- Fail-before / pass-after, re-run once on this head (checked out the
`origin/main` copies of the 9 existing non-test source files and deleted
the new `delta-translation.ts`, rebuilt, ran, restored; `git status
--porcelain` empty afterwards):
- `agent-runtime` `delta-translation.test.ts`: cannot load on main
(`Cannot find module './delta-translation.js'`); passes on the PR tree.
- `thread-view` `timeline-cli-rendering.snapshots.test.ts`:
`AssertionError: expected [ { initiator: 'user', …(2) } ] to deeply
equal [ { initiator: 'user', …(2) }, …(1) ]` on main; 50/50 pass on the
PR tree.
- `server` `timeline-provider-input.test.ts`: `AssertionError: expected
[ 'user:Reply only with ok.', …(2) ] to deeply equal [ 'user:Reply only
with ok.', …(3) ]` on main; passes on the PR tree.
- `host-daemon-contract` `contract.test.ts`: `expected 150 to be 151` on
main; 52/52 pass on the PR tree.
- `turbo run typecheck test --force` for agent-runtime, thread-view,
host-daemon-contract, provider-bridge-protocol, domain,
@get-bb/plugin-sdk: `Tasks: 17 successful, 17 total` (442 + 391 + 52 +
218 + 150 + 127 tests). `turbo run typecheck --filter=@bb/server`:
`Tasks: 4 successful, 4 total`.
- CI for `bdf47388f` (run 32508210319): Checks, Package Smoke (ubuntu +
macOS), Tests app-1/2/3, integration, packages, server all pass;
`mergeable=MERGEABLE`, `mergeStateStatus=CLEAN` against `main`.
- The live Pi repro was not re-run in this pass: the diff against the
previously verified head (where it was run with a real Pi extension
turn) is the two version strings above, which do not reach the runtime
path.
## Rebase (2026-08-21, second)
Rebased onto `main` at `d41d1abee`. Two collisions, both from `main`
moving under the PR:
- `main` took protocol **151** (#2242, the auto/steer turn-target
re-resolution), so this PR's change is renumbered **151 -> 152**. Its
comment block now sits above main's 151 block, and the lockstep
assertion in `contract.test.ts` moves to `toBe(152)`. No other file
hardcodes the constant; every other consumer reads it symbolically.
- `main` moved the SDK to `0.4.13`, so `plugin-sdk-version.ts` and
`plugin-sdk/package.json` resolve to main's values and drop out of this
PR's diff.
Re-verified on the new base: `check-npm-version-guard.mjs` -> `PASS -
@get-bb/plugin-sdk@0.4.13 is not on npm yet`. `turbo run typecheck` for
server, agent-runtime, host-daemon-contract, provider-bridge-protocol,
thread-view: `Tasks: 8 successful, 8 total`. `turbo run test` for the
same set: host-daemon-contract 3/3 files, provider-bridge-protocol
16/16, thread-view 23/23, agent-runtime 31/31, server 200 passed / 1
skipped with the single known local-only failure `internal-skill-trees`
(`mode 420` vs `436`, i.e. 0644 vs 0664 under this machine's umask
0002); it passes in CI and this PR does not touch skill trees.
**#2142 (layer 2/2) is renumbered to protocol 153 and rebased on this
head.**
> AGENT GENERATED: by Claude Opus 5
Co-authored-by: Claude <noreply@anthropic.com>
Contributor
Author
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
1 participant
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.
Fixes #1681
What was wrong
Pi extensions can inject visible custom messages and trigger a provider turn. bb did not recognize their
message_startandmessage_endboundaries, so it stored them asprovider/unhandledand omitted the provider-originated input from the projected timeline. A process-triggered run could then complete with no assistant text and no explanation.What changed
role: "custom"message boundaries generically, independent ofcustomTypeagent_startthenmessage_startorder and warn when a provider-triggered turn returns no assistant textmessage_end, hidden custom messages, idlecontext, and allignorenotifications from creating duplicate turns or inputundefinedfields do not replace otherwise valid raw data with a serialization-error stubHOST_DAEMON_PROTOCOL_VERSIONfrom 135 to 136 for the changed daemon event semanticsThe string-content
agent_endschema fix previously stacked in this branch is not included; it already merged in #1663.Validation
pnpm exec turbo run test --filter=@bb/agent-runtime --filter=@bb/thread-view --filter=@bb/provider-bridge-protocol --filter=@bb/host-daemon-contract --forcepnpm exec turbo run typecheck --filter=@bb/agent-runtime --filter=@bb/thread-view --filter=@bb/provider-bridge-protocol --filter=@bb/host-daemon-contractgit diff --checkA live Pi extension session still needs final verification for a normal process response and an empty assistant response.