chore: extract ACP stdio transport pre-patch#3310
Open
ssfdust wants to merge 1 commit intotailcallhq:mainfrom
Open
chore: extract ACP stdio transport pre-patch#3310ssfdust wants to merge 1 commit intotailcallhq:mainfrom
ssfdust wants to merge 1 commit intotailcallhq:mainfrom
Conversation
|
|
Extracts independently-mergable fixes from the ongoing ACP work (tailcallhq#2858) that can land on main immediately. Three classes of fix: 1. stdin pre-read encapsulation (cli.rs, main.rs) Add Cli::uses_stdin() method that centralizes the decision of which subcommands own stdin. Subcommands opt in via uses_stdin() rather than having the startup pipeline guess from a hardcoded exclusion list. This makes it easy to add new stdin-consuming subcommands (e.g. an ACP stdio server) without touching the startup logic. 2. Tool output leaking to stdout (context.rs, tool_executor.rs) Shell tool execution streamed raw output to io::stdout(), corrupting the ACP JSON-RPC stream. Add silent: bool field on ToolCallContext (the correct home for per-invocation runtime flags). Default false; ACP sets .silent(true) to route output to io::sink() instead. 3. Stderr vs stdout discipline (main.rs, sandbox.rs) Panic hook and worktree status messages used println! which writes to stdout. Changed to eprintln! (stderr) to avoid contaminating structured transport channels. Signed-off-by: ssfdust <ssfdust@gmail.com> Co-authored-by: ForgeCode <noreply@forgecode.dev> Co-authored-by: DeepSeek <deepseek-ai@deepseek.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.
ACP stdio transport pre-patch
Extracts independently-mergable fixes from the ongoing ACP work in #2858.
Background
When spawning
forge machine stdioas an ACP subprocess, stdin is a pipethat remains open for bidirectional JSON-RPC communication. This patch
resolves three classes of bug discovered during ACP integration testing.
Changes
stdin pre-read encapsulation (
cli.rs,main.rs)Added
Cli::uses_stdin()method that centralizes the decision ofwhich subcommands own stdin. Subcommands opt in via
uses_stdin()rather than having the startup pipeline guess from a hardcoded
exclusion list. This makes it easy to add new stdin-consuming
subcommands (e.g. an ACP stdio server) without touching the
startup logic.
tool output leaking to stdout (
context.rs,tool_executor.rs)Shell tool execution always streamed raw output to
io::stdout(),corrupting the ACP JSON-RPC stream. Added a
silent: boolfieldon
ToolCallContext(the correct home for per-invocation runtimeflags). Default
false, ACP sets.silent(true)— output thengoes to
io::sink()instead.stderr vs stdout discipline (
main.rs,sandbox.rs)Panic hook and worktree status messages used
println!, whichwrites to stdout and contaminates structured transport. Changed
to
eprintln!(stderr).