Skip to content

Separate Pi retry policy from lifecycle process coverage - #2353

Merged
ymichael merged 2 commits into
mainfrom
bb/ci-flake-fix-provider-pi-lifecycle-no-thread-sta-thr_uvszfadx9d
Aug 24, 2026
Merged

Separate Pi retry policy from lifecycle process coverage#2353
ymichael merged 2 commits into
mainfrom
bb/ci-flake-fix-provider-pi-lifecycle-no-thread-sta-thr_uvszfadx9d

Conversation

@ymichael

@ymichael ymichael commented Aug 24, 2026

Copy link
Copy Markdown
Collaborator

What was wrong

The Pi lifecycle test coupled two separate contracts. To prove that a failed construction leaves no child, it requested a permanently mismatched model. Production correctly treats an initial mismatch as a transient auth race and retries eight times, so this single lifecycle assertion launched nine real Node children in series and imported the real Pi extension nine times. The test asserted only that more than one child spawned and that all children exited; it did not need nine real processes to prove cleanup. Under packages-shard contention, those avoidable serial launches crossed the harness's unchanged 60-second response deadline and produced no response to thread/start in the failed packages job on PR #2349.

What changed

The retry policy and real-process lifecycle coverage are now separate:

  • runPiTransientAuthConstruction contains the unchanged production policy and accepts injected construction, discard, closed-state, and wait operations. A zero-process test deterministically proves one initial attempt plus eight retries, eight failed-attempt discards, and propagation of the ninth error.
  • The existing real-process transient-mismatch test still proves that a mismatched child is killed, a replacement child succeeds, and no construction-time session error leaks.
  • The failed-construction lifecycle test now uses a fake-Pi fault that exits before its first response. It launches one real child and still verifies the bridge error, exact spawn count, observed process exit, and no child left behind.
  • The scratch-file lifecycle case uses the same one-process construction failure and verifies its tool/prompt files disappear after the child exits.

The earlier timeout-only changes were reverted: the shared request deadline remains 60 seconds, and the existing lifecycle wrappers remain 90 and 60 seconds. Production retry count, delay, child detachment, and error behavior are unchanged. This does not alter the host-daemon wire protocol, so HOST_DAEMON_PROTOCOL_VERSION is unchanged.

How you verified

Before edits, the parent and worker independently confirmed that HEAD, origin/main, and git merge-base HEAD origin/main all equaled 21cb6b68b8fdffe2cd576f11c8af500f98d22c11, with a clean worktree. Exact GitHub issue/PR and bb thread searches found no overlapping lifecycle fix; PRs #2337 and #2342 cover different provider-pi signatures.

The original coupled test reproduced the exact CI error under 256 CPU competitors at 61.288 seconds: Error: no response to thread/start. With the structural split and the original clocks restored, the identical stress command passed with 150 milliseconds of lifecycle test time (7.73 seconds total including imports). The deterministic nine-attempt policy test completes in 1–2 milliseconds.

  • Focused retry-policy and failed-construction coverage — 3 passed, 6 skipped
  • pnpm exec turbo run test --filter=bb-plugin-provider-pi --force — 18 files, 105 tests passed
  • pnpm exec turbo run typecheck --filter=bb-plugin-provider-pi --force — 4 Turbo tasks passed
  • pnpm exec turbo run build --filter=@bb/server --force — 4 Turbo tasks passed
  • pnpm exec oxfmt plugins/provider-pi/src/bridge/rpc-session.retry.test.ts --check — passed
  • git diff origin/main --check — passed

AGENT GENERATED: by GPT-5.6-Sol

@ymichael ymichael changed the title Stabilize Pi lifecycle retry test budgets Separate Pi retry policy from lifecycle process coverage Aug 24, 2026
SawyerHood added a commit that referenced this pull request Aug 24, 2026
## What was wrong

The parity self-test `pi/compaction reproduces its recording` fails on
`main` (CI for #2346 and #2349, job `Tests (packages, ubuntu-latest,
Node 22.x)`). The recording contains a steer. After the steer, pi writes
the `prompt` response and then the `tool_execution_end` for `sleep 2`.
One pipe read can carry both lines. The pi bridge handled a chunk in one
synchronous loop: the event's delivery (`deliverInOrder`) runs one
microtask hop after its line, but the steer's `input.accepted` runs
several `await` hops after the response. So `item.close` went out before
`input.accepted`, and the bridge's output order depended on pipe
chunking. The parity replay child keeps a 50 ms gap after a response,
but a starved CI runner can stall the bridge past that gap. The diff in
CI shows this exactly: `turn/input/accepted` moved, and the `sleep 2`
row spans seq 9–11 instead of 9–12.

The other red test on `main` (`bridge.lifecycle.test.ts > a failed
construction leaves no child`, #2346 only) is the pi lifecycle budget
flake that #2353 addresses.

## What changed

- `plugins/provider-pi/src/bridge/rpc-child.ts`: `PiRpcChild` queues
stdout lines and handles one line per event-loop turn (`setImmediate`).
A request's continuation finishes before the next line, which is the
order a line-at-a-time read gives.
- `plugins/provider-pi/src/bridge/fake-pi-rpc.mjs`: new knob
`FAKE_PI_BATCH_STEER_REPLY=1` writes a steer's `prompt` response and the
resumed run's first event in one stdout write.
- `plugins/provider-pi/src/bridge/bridge.round2.test.ts`: regression
test that pins the ack before the same-chunk event.

No wire change. `HOST_DAEMON_PROTOCOL_VERSION` stays the same: only the
bridge's ordering became deterministic.

## How you verified

- The new round2 test fails before the change (ack at index 5, event at
index 4) and passes after it.
- Deterministic repro of the parity failure: a replay child patched to
write the post-response line in the same write failed the
`pi/compaction` cell 3/3 before the change and passed 3/3 after it. The
patch was a local experiment and is not part of this PR.
- `pnpm exec turbo run test typecheck --filter=./plugins/provider-pi
--filter=@bb/provider-parity --force`: pi plugin 105/105, parity 56/56,
typecheck clean.
- Off-pipeline: real `pi` (0.84.3) slice of
`@bb/agent-runtime#test:integration`
(`integration.provider-basic.test.ts -t "pi provider"`): 8/8, including
"steers an active turn".

Fixes the `pi/compaction` parity failure on `main` (no issue filed).

> AGENT GENERATED

Co-authored-by: Claude <noreply@anthropic.com>
@ymichael
ymichael merged commit 8e712e1 into main Aug 24, 2026
24 of 25 checks passed
@ymichael
ymichael deleted the bb/ci-flake-fix-provider-pi-lifecycle-no-thread-sta-thr_uvszfadx9d branch August 24, 2026 23:00
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.

1 participant