fix(acp): deliver Pi session prompts through the fork - #7508
Conversation
Signed-off-by: Salman Mohammed <smohammed@squareup.com>
🔐 Codex Security Review
|
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
salman1993
left a comment
There was a problem hiding this comment.
🤖 No actionable code findings from static inspection of the complete diff at 3ae55097293385f1960ef825d2219e4c5d434100. The capability-gated fallback, replacement transport, skill forwarding, and transcript projection match the stated intent and the pinned fork implementation.
Validation is not yet a clean merge gate: an independent cargo test -p buzz-acp run at this exact head had 926 pass / 1 fail / 1 ignored (acp::tests::keepalive_resets_idle_past_deadline); the failed test passed when rerun alone, so this looks timing-sensitive rather than caused by the patch, but the full package run was not green. The focused system-prompt lane passed 15 tests. The real Pi fork/session-switch path was not independently exercised because no BUZZ_TEST_PI_ACP adapter path was available. CI was still running when reviewed.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3ae5509729
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Signed-off-by: Salman Mohammed <smohammed@squareup.com>
Signed-off-by: Salman Mohammed <smohammed@squareup.com>
Signed-off-by: Salman Mohammed <smohammed@squareup.com>
Summary
Pi agents now receive Buzz's complete session instructions in their native system prompt. The prior launcher supplied only the base prompt and left persona, team instructions, and memory in the first user message.
--skill <workspace>/.agents/skillsafter the adapter's--separator on every launch._meta.systemPromptwhen the adapter advertises persistent replacement support. Keep_meta.sessionTitleand show the replacement prompt in the transcript. Older adapters retain legacy user-message framing; Claude keeps its append transport.Install the fork and restart managed Pi agents to use the new transport. Start fresh sessions to avoid retaining old user-framed instructions. Install the adapter with
npm install -g --install-links=true git+https://github.com/salman1993/pi-acp.git#main(Node.js 22 or newer). Run the same command again to update it.Related issue
Related to #7329. Replaces the prompt workaround in #7335, using the extensions in salman1993/pi-acp#1, salman1993/pi-acp#2, and salman1993/pi-acp#3. No duplicate implementation PR found.
Testing
BUZZ_TEST_PI_ACP=/absolute/pi-acp/dist/index.js cargo test -p buzz-acp real_pi_preserves -- --ignoredpassed on macOS with fork commitb893ff9and Pi 0.84.4. It exercises Buzz's production session composer and inspects Pi's effective prompt after session switching and adapter restart. All six instruction sections and the extra skill appear exactly once. The fixture uses isolated settings and synthetic history, with no model calls.Built the fork, packed it, and installed the package in an isolated prefix to verify the executable is included.
Update Sep 09, 10:02: Preserve vendor CLI availability checks and missing-component setup guidance. After salman1993/pi-acp#4, the install hint and testing guide use the direct npm install command; a local source build is no longer required.
Generated with Codex