Translate selected Pi skills to native commands - #3249
Open
bb-slop-cop[bot] wants to merge 1 commit into
Open
Conversation
Contributor
|
The in-place rewrite fixes a skill chip only when it is already at the beginning of the prompt. For example, a mid-message selection becomes:
Pi expands skill commands only when the input starts with #3253 handles the picker selection semantically: it moves exactly one validated skill invocation to the beginning and passes all remaining text as its arguments. It also preserves whitespace and attachments, leaves plain slash text and provider commands unchanged, and has been verified against real Pi with a disabled native skill. I recommend merging #3253 instead and closing this PR as superseded. |
SawyerHood
pushed a commit
that referenced
this pull request
Sep 8, 2026
## Human comments ## What was wrong The Pi bridge discarded structured skill-mention metadata and forwarded only the picker’s `/name` display text. Pi requires `/skill:name` to explicitly invoke native skills, so skills with `disable-model-invocation: true` could not be used through BB. This is tracked in #3246. ## What changed The Pi bridge now recognizes exactly one validated, picker-selected skill mention and converts it into a leading `/skill:<name>` command. All remaining prompt text becomes the skill arguments, regardless of where the chip appeared, while preserving argument whitespace and existing file/image input behavior. Plain slash text, provider commands, malformed mentions, and prompts containing multiple selected skills remain unchanged. The fake Pi harness gained an opt-in prompt dump used to verify that image payloads survive the transformation. This supersedes #3249. That PR rewrites the selected token in place, so a mid-message selection becomes `text /skill:name arguments`; Pi only expands `/skill:name` at the beginning of its input. This change instead moves the semantic invocation to the front and passes the remaining text as arguments. This changes no wire protocol, persisted data, CLI surface, or skill-discovery behavior. ## How you verified - Added Pi bridge regression coverage for: - leading and mid-message skill selections; - invocations with and without arguments; - boundary whitespace; - multiple text chunks, local files, and image payloads; - malformed mention ranges and invalid triggers; - multiple selected skills; - provider commands and unselected slash text. - Confirmed the regression failed before the production change. - Ran a live test with Pi 0.85.0, `openai-codex/gpt-5.6-terra`, and a disposable native skill using `disable-model-invocation: true`. - `pnpm exec turbo run typecheck --filter=bb-plugin-provider-pi` - `pnpm exec turbo run test --filter=bb-plugin-provider-pi` — 23 files and 133 tests passed. - `pnpm exec oxfmt --check plugins/provider-pi/src/bridge/bridge.ts plugins/provider-pi/src/bridge/bridge.skill-command.test.ts` - `git diff --check` Fixes #3246 > AGENT GENERATED
ymichael
added a commit
that referenced
this pull request
Sep 11, 2026
## Human comments
## What was wrong
The local-file bridge test wrapped three deterministic input-to-prompt
assertions in three independent real fake-Pi session lifecycles. Each
case constructed a Pi RPC child, loaded the real extension boundary,
delivered one local-file conversion, and tore the child down. The
packages shard runs Turbo at concurrency 4 while each package starts a
Vitest pool, so CPU/process oversubscription made those avoidable serial
child startups cross Vitest's unchanged 5-second case ceiling. Scheduler
contention is the reproduction trigger; the root cause is paying three
feature-specific subprocess startups for a typed conversion already
exposed as `extractPiPromptInput`.
On the frozen Intel base, the unchanged file passed unloaded in 4.43
seconds of test time (2.197/1.117/1.118 seconds per case). With 32
bounded CPU burners, all three unchanged cases timed out at the existing
5-second ceiling and were reported at 6.927/6.913/8.498 seconds.
## What changed
- Added exact mixed text/local-file and file-only assertions to
`turn-input.test.ts`, directly against the typed conversion called by
both turn and steer handlers.
- Replaced the generic text prompt in the existing real-process
`stop{release}` lifecycle case with a metadata-bearing local-file-only
turn and retained the child shutdown/checkpoint assertions.
- Added a local file to the existing real-process successful steer case
and retained its accepted-input, turn-boundary, framing, and no-error
assertions.
- Deleted the standalone three-session `bridge.local-file.test.ts`.
This adds no subprocess startup: the meaningful `turn/start` and
`turn/steer` protocol checks ride real child sessions already required
for lifecycle coverage. No timeout, polling deadline, retry budget,
production behavior, protocol field, CLI/SDK surface, or host-daemon
wire contract changed, so `HOST_DAEMON_PROTOCOL_VERSION` is unchanged.
A timeout increase was rejected because the work is deterministic and
its repeated process construction is avoidable. A shared
feature-specific fake-Pi session was also rejected because it would
preserve unnecessary startup exposure and duplicate existing lifecycle
sessions. The retained real-process checks continue waiting for
observable turn boundaries and child exit, while the conversion matrix
itself completes synchronously.
Open PR/issue searches found no owner for `bridge.local-file.test.ts` or
the exact timeout. The only open PR touching the Pi bridge is #3249, for
skill-command behavior; it does not touch this test. BB thread searches
likewise found no overlapping local-file flake owner. Merged #3302 is
the structural precedent for the distinct skill-command suite.
## How you verified
Mandatory host gate and every workload ran on enrolled Intel host
`host_nwqfteeqz4` (`Intel(R) Core(TM) i5-1038NG7 CPU @ 2.00GHz`,
`x86_64`). HEAD began at and retains merge-base
`a4f513251851bc357fbc0f28e61c331ea3f9f2e4`; `origin/main` advanced
afterward and this branch intentionally kept the frozen base.
- Unchanged-main focused run: 3 passed; 4.43 seconds of test time.
- Unchanged-main 32-burner reproduction: 3/3 timed out at the unchanged
5-second ceiling; 6.927/6.913/8.498 seconds reported.
- Final identical 32-burner conversion run: 12/12 passed in 50 ms of
test time; the local-file case took 3 ms.
- Final focused conversion plus reused turn/steer protocol checks: 3/3
passed.
- `pnpm exec turbo run test --filter=bb-plugin-provider-pi --force --
--maxWorkers=2`: 26 files and 154 tests passed, one file/test skipped.
- The default-worker full package run demonstrated the systemic
oversubscription separately: 150 tests passed and four unrelated
settings/conformance/lifecycle cases timed out or observed late cleanup;
both changed real-process cases passed under that load at 13.413 and
12.637 seconds. No clock was changed in response.
- `pnpm exec turbo run typecheck --filter=bb-plugin-provider-pi
--force`: 4 Turbo tasks passed.
- `pnpm exec turbo run build --filter=@bb/host-daemon --force`: 5 Turbo
tasks passed.
- `pnpm exec turbo run test --filter=@bb/plugin-build --force --
--maxWorkers=2`: 10 files and 137 tests passed, one skipped, including
provider-Pi server and host artifact builds.
- `pnpm exec oxfmt --check
plugins/provider-pi/src/bridge/bridge.lifecycle.test.ts
plugins/provider-pi/src/bridge/bridge.round2.test.ts
plugins/provider-pi/src/bridge/turn-input.test.ts`
- `git diff --check`
Every test, build, formatting, and synthetic-load command used an
external deadline and a dedicated top-level process group; the load
runner trapped EXIT/INT/TERM/HUP and reaped its workers. Post-run
process and temporary-directory audits found no surviving burner,
Vitest, Turbo, fake-Pi child, or task-specific temporary directory.
> AGENT GENERATED
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
2 participants
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
The Pi bridge discarded structured command-mention metadata and forwarded only the displayed slash text. A selected skill therefore reached Pi without the provider-native prefix required for RPC skill expansion. The bug and boundary behavior were reproduced twice at
origin/main; see the reproduction and root-cause report.What changed
The bridge now rewrites only validated text ranges marked as skill command mentions into Pi's native skill-command form. Ordinary provider commands and slash-like plain text retain their existing form. This stays within the Pi provider and changes no wire protocol, schema, dependency, generated file, or stored data.
The diff changes 149 text lines across two files.
How you verified
Response to: /skill:inspect src, receivedResponse to: /inspect src.pnpm exec turbo run typecheck test --filter=bb-plugin-provider-pi— 124 tests passed and typecheck passed.pnpm exec oxfmt --check plugins/provider-pi/src/bridge/bridge.ts plugins/provider-pi/src/bridge/bridge.skill-command.test.tsgit diff --checkFixes #3246