feat(cli): add pfterminal-acp, a stable ACP entry point - #78
Open
0xzoz wants to merge 2 commits into
Open
Conversation
ACP clients (Buzz, Zed, and others) speak JSON-RPC over stdio. PFTerminal exposes the Codex app-server protocol instead, so it cannot be registered with an ACP host directly. The maintained codex-acp adapter bridges the two and honours CODEX_PATH to choose which Codex-compatible binary it drives. This adds a thin launcher that resolves the PFTerminal executable, sets CODEX_PATH authoritatively, and hands off to codex-acp, preserving stdio, signals and the adapter's exit code. It is not an ACP implementation. Shipping it as a command rather than documenting "set CODEX_PATH and run codex-acp" matters because ACP hosts key runtime identity off the command. Buzz's built-in Codex runtime is already defined around codex-acp, so sharing that executable risks mistaken runtime identity, stale persona pins and merged usage attribution between two different agents. Details: - resolution order is PFTERMINAL_PATH, then sibling-of-self, then PATH, so a multi-install machine drives the PFTerminal it shipped beside rather than whichever wins on PATH - CODEX_PATH is overridden, never inherited: an inherited value would point the adapter at another agent while the client still believed it was talking to PFTerminal - all diagnostics go to stderr, because after handoff stdout carries ACP frames exclusively and one stray line corrupts the stream - Unix uses exec() so the adapter becomes this process and signals and exit codes need no forwarding logic; Windows spawns and propagates the code - --version reports the launcher plus both resolved paths, which is what makes it useful for diagnosing a broken install Signed-off-by: 0xzoz <zeroexzoz@gmail.com>
This was referenced Aug 5, 2026
Independent review (GPT-5.5) scored the launcher 8/10 with no blocking findings and four majors. All four are real; fixed here. Windows exit codes were not actually propagated. Returning `ExitCode` truncates to a u8, so 256 arrived as success and 3010 as 194. Now exits via `process::exit`, which takes an i32, and `exec` is typed `!` on both platforms so the divergence is explicit. Windows adapter discovery would have failed for the install we document. `npm install -g` produces a `codex-acp.cmd` shim, never a `.exe`, so probing only `.exe` left the adapter undiscoverable on a correctly-installed machine. Candidate names now follow PATHEXT. Resolution accepted any regular file as an executable, so a non-executable file earlier on PATH won the search and then failed at spawn with 126 even when a real executable sat further along. Now checks the executable bit. Resolved paths are canonicalised, so CODEX_PATH cannot be re-resolved against the adapter's cwd — the previous behaviour was weaker than the "authoritative" the docs claimed. Documentation corrected rather than the behaviour, in one case: --version and --help stay on stdout, because that is what every CLI does and redirecting them breaks shell usage. The module docs said "every diagnostic to stderr", which was simply false; they now state the exception and why it cannot affect a session. The help text also claimed all arguments were forwarded unchanged, which ignored those intercepts. Tests: the "single-threaded test process" SAFETY comments were wrong, since Cargo runs tests in parallel threads; env mutation is now serialised behind a lock. Adds an integration suite driving the real binary against a fake adapter, covering the properties that break an ACP session silently rather than loudly — stdout stays byte-clean on launcher failure, the adapter's exit code survives, CODEX_PATH is overridden rather than inherited, and argv reaches the adapter unchanged. Live ACP handshake re-verified after the refactor: initialize, session/new, 80-model catalogue, streamed prompt, end_turn. Signed-off-by: 0xzoz <zeroexzoz@gmail.com>
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
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.
Summary
Adds
pfterminal-acp, a stable entry point that lets PFTerminal be registered as an agent in ACP hosts (Buzz, Zed, and anything else speaking the Agent Client Protocol).ACP clients speak JSON-RPC over stdio. PFTerminal exposes the Codex app-server protocol instead, so it cannot be registered with an ACP host directly. The maintained
codex-acpadapter bridges the two and honoursCODEX_PATHto choose which Codex-compatible binary it drives:This binary is deliberately not an ACP implementation — it is a thin launcher that resolves the PFTerminal executable, sets
CODEX_PATH, and hands off to the adapter.Why a shipped command rather than documentation
The alternative is telling users to register
codex-acpwithCODEX_PATH=pfterminalin the host's env. That works mechanically but is the wrong shape: ACP hosts key runtime identity off the command. Buzz's built-in Codex runtime is already defined aroundcodex-acp, so two different runtimes sharing that executable risks incorrect runtime identity, stale persona pins, and merged usage attribution.Shipping the command also means the host preset can declare
underlying_cli: "pfterminal", so a user who has PFTerminal installed but not the bridge sees adapter missing rather than not installed.Implementation notes
Three decisions worth calling out, each with a failure mode behind it:
PFTERMINAL_PATH→ sibling-of-self →PATH. Sibling first because the two binaries ship together: on a machine with several PFTerminal installs, the adapter should drive the one it was installed beside, not whichever winsPATH.CODEX_PATHis overridden, never inherited. An inherited value would silently point the adapter at a different agent while the client still believed it was talking to PFTerminal.exec()so the adapter becomes the process and signals and exit codes need no forwarding logic that could get them subtly wrong; Windows spawns and propagates the exit code.--versionreports the launcher plus both resolved paths, which is what makes it useful when an install is broken — a bare version number tells you nothing.Testing
cargo build,cargo test(3/3),cargo fmt --check, andcargo clippyall clean; clippy reports no lints on the new file.Live ACP handshake against a real install (PFTerminal 0.1.20,
codex-acp1.1.9), driven through the compiled binary over stdio:initializeprotocolVersion: 1, authMethods[api-key, chat-gpt]session/newgpt-5.5[xhigh]session/promptstopReason: end_turn~/.pfterminal; stock Codex's~/.codexuntouchedNote:
cargo clippy -- -D warningsfails oncodex-http-client(unused variable: trace_bytes). That is pre-existing onmainand unrelated to this change — the flag merely escalates an existing warning in a dependency.Related
Downstream consumer, opened as a draft pending a release that includes this command: block/buzz#4956 adds PFTerminal as a tier-2 preset harness. That preset points at
pfterminal-acp, so it stays draft until this ships.