Skip to content

Apply later-turn provider environment to Pi and ACP - #3299

Merged
ymichael merged 1 commit into
mainfrom
bb/fixer-apply-later-turn-provider-environment-to-p-thr_g4d6wqfyk4
Sep 9, 2026
Merged

Apply later-turn provider environment to Pi and ACP#3299
ymichael merged 1 commit into
mainfrom
bb/fixer-apply-later-turn-provider-environment-to-p-thr_g4d6wqfyk4

Conversation

@ymichael

@ymichael ymichael commented Sep 8, 2026

Copy link
Copy Markdown
Collaborator

Human comments

What was wrong

PR #3035 made plugin-contributed provider environments available on every command, but Pi reconciled only model and reasoning changes while ACP ignored later turn/start options. Their long-lived child processes therefore kept the environment from session creation even after BB accepted and recorded new credentials or endpoints, so a later turn could use the wrong account or fail with stale credentials.

What changed

Pi now compares the effective turn environment with its construction environment and uses its existing rollback-safe rebuild and resume path when values differ. ACP now retains its construction parameters, merges current launch and turn environments, and restarts through its existing resume path; it reports context loss only when fallback changes the provider thread id. Identical environments do not trigger replacement, and legacy empty replay payloads keep their existing no-change behavior.

The rebase preserves current main's deterministic Pi skill-input coverage and drops the now-obsolete timeout-only commit for the deleted process-backed test. No public contract, CLI, documentation, or host-daemon wire behavior changed, so no protocol bump is required.

How you verified

  • Added red-first Pi and ACP regressions that reproduce stale child environments and pass after the fix.
  • Full provider suites passed: Pi 157/157 tests and ACP 312/312 tests.
  • Focused post-format reruns passed: Pi turn options 7/7 and ACP bridge 96/96.
  • pnpm exec turbo run test typecheck --filter=bb-plugin-provider-pi --filter=@bb/provider-bridge-acp --force passed all eight Turbo tasks.
  • bb plugin build plugins/provider-pi and bb plugin build plugins/provider-acp emitted their packages successfully.
  • Targeted formatting for the four normally formatted source/test files and git diff --check origin/main...HEAD passed. The three-line fake-Pi fixture addition follows formatter output; the remainder of that legacy fixture is unchanged.

No linked issue; this is a follow-up to #3035.

AGENT GENERATED

ymichael added a commit that referenced this pull request Sep 9, 2026
## Human comments

## What was wrong

`plugins/provider-pi/src/bridge/bridge.skill-command.test.ts` coupled a
deterministic input-to-Pi-prompt transformation matrix to full bridge
process lifecycle. Per-test setup initialized a new isolated bridge
harness, each case's first prompt started a fresh thread/session and
spawned a fake Pi RPC Node child, and teardown stopped that child; the
final case started two sessions. The 11-case file therefore paid for 12
serial child startups even though the behavior under test is a typed
input conversion.

The packages shard runs 89 package tests with Turbo `--concurrency=4`,
while each package starts its own Vitest worker pool. That nested
CPU/process oversubscription is the systemic trigger: child construction
crossed Vitest's 5 s case ceiling in unrelated [PR #3295 attempt
1](https://github.com/get-bb/bb/actions/runs/34285876225/job/102261224349),
[PR #3295 attempt
2](https://github.com/get-bb/bb/actions/runs/34285876225/job/102263472294),
and [PR #3299 attempt
2](https://github.com/get-bb/bb/actions/runs/34287966835/job/102269943782).
Attempt 1 timed out the no-arguments case; attempts 2 and 3 timed out
the two-turn unchanged-input case.

PR #3295 subsequently merged a file-wide 30 s timeout override for this
test. That masks the repeated-startup cost instead of removing it.
Scheduler contention exposes the flake, but the root cause is the
avoidable process lifecycle around pure transformation assertions.

## What changed

The existing typed input conversion now lives in `turn-input.ts`, and
`bridge.ts` calls it for start and steer input. The exact transformation
matrix and raw schema-boundary rejection cases run directly in
`turn-input.test.ts`. The old process-backed feature test and its newly
merged 30 s override are removed.

Feature behavior remains covered at the narrowest deterministic
boundary: native skill invocation, no-argument invocation,
command-position movement, whitespace, text/file/image composition,
multiple skills, malformed ranges, text mismatch, provider commands, raw
slash text, and invalid protocol inputs. Generic real-process bridge
lifecycle and input delivery remain covered by the existing conformance,
lifecycle, local-file, framing, and session suites. Keeping another
child solely around the skill conversion is redundant: the bridge now
directly calls the tested typed function, and the existing integration
suites cover delivery of its output to Pi.

No timeout, polling deadline, retry budget, wrapper limit, protocol
assertion, or production behavior is increased or weakened. This does
not change host-daemon wire behavior, so `HOST_DAEMON_PROTOCOL_VERSION`
is unchanged. There is no CLI, SDK, configuration, or documentation
surface change.

## How you verified

The mandatory fresh-main gate ran on enrolled Intel host
`host_nwqfteeqz4` at frozen SHA and merge-base
`124cf2e8888a396726f3f00271074b6a622bdb42`. After #3295 merged, the
final commit was rebased onto `bb26b4fd9a054d50e564642416319711bc4db16a`
and deliberately removed its timeout-only workaround.

- Unchanged-main focused baseline: 11 passed in 14.05 s of test time;
ordinary process cases took about 1.1 s and the two-session case took
2.218 s.
- Unchanged-main 32-burner stress: 7 of 11 cases hit the unchanged 5 s
ceiling; the first two were reported at 6.175 s and 6.177 s.
- An intermediate one-shared-child version was still not robust: one
identical 32-burner round passed, but another exceeded the existing
suite-hook ceiling before either bridge assertion ran. That proved the
feature-specific process was unnecessary residual exposure.
- Final identical 32-burner stress with no feature child and no timeout
override: all 11 deterministic/protocol cases passed in 89 ms of test
time.
- Final-tree `pnpm exec turbo run typecheck
--filter=bb-plugin-provider-pi --force` — 4 Turbo tasks passed.
- Final-tree full provider-pi Turbo test reached all 11 affected cases
in 81 ms; a separate existing `bridge.lifecycle.test.ts` local-image
process case timed out at 5.262 s while many unrelated process cases ran
2–3× slower. An earlier unloaded full run with the same production
extraction passed 28 files plus 1 skipped and 156 tests plus 1 skipped.
The separate recurrence is outside this focused fix and is reported as a
limitation.
- `pnpm exec turbo run build --filter=@bb/host-daemon --force` — 5 Turbo
tasks passed. Provider-pi has no package-local build task.
- `pnpm exec turbo run test --filter=@bb/plugin-build --force` — 10
files and 137 tests passed (1 skipped), including bundling and importing
the built-in provider-pi server and host artifacts.
- `pnpm exec oxfmt --check plugins/provider-pi/src/bridge/bridge.ts
plugins/provider-pi/src/bridge/turn-input.ts
plugins/provider-pi/src/bridge/turn-input.test.ts`
- `git diff --check`
- Post-run process and temp-directory audits found no surviving test,
build, load, fake-Pi, watcher, or worktree process and no related test
temp directory.

Every stress/load/test/build/watch command used a tracked process group,
external deadline, and cleanup trap. The 32-burner stress is
intentionally synthetic and models nested worker oversubscription rather
than treating contention as the root cause. Hosted CI on final head
`ecb35451f9407c9de5c6e9cd29cb41854e42d027` passed every required check,
including the packages shard and both package-smoke jobs.

> AGENT GENERATED
@ymichael
ymichael force-pushed the bb/fixer-apply-later-turn-provider-environment-to-p-thr_g4d6wqfyk4 branch from c084b3a to 79db9d1 Compare September 9, 2026 04:06
@ymichael
ymichael merged commit 682f0d2 into main Sep 9, 2026
16 checks passed
@ymichael
ymichael deleted the bb/fixer-apply-later-turn-provider-environment-to-p-thr_g4d6wqfyk4 branch September 9, 2026 04:10
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