Skip to content

Commit 3f70ae9

Browse files
ssfdustforge-code-agentDeepSeek
committed
chore: extract ACP stdio transport pre-patch
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>
1 parent 4ee5685 commit 3f70ae9

7 files changed

Lines changed: 33 additions & 11 deletions

File tree

crates/forge_app/src/orch.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -262,6 +262,9 @@ impl<S: AgentService + EnvironmentInfra<Config = forge_config::ForgeConfig>> Orc
262262

263263
// Retrieve the number of requests allowed per tick.
264264
let max_requests_per_turn = self.agent.max_requests_per_turn;
265+
// TODO(acp): when running in ACP stdio mode, chain `.silent(true)`
266+
// so that shell tool output is suppressed on stdout (see
267+
// ToolCallContext and tool_executor).
265268
let tool_context =
266269
ToolCallContext::new(self.conversation.metrics.clone()).sender(self.sender.clone());
267270

crates/forge_app/src/tool_executor.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -277,7 +277,7 @@ impl<
277277
input.command.clone(),
278278
PathBuf::from(normalized_cwd),
279279
input.keep_ansi,
280-
false,
280+
context.silent,
281281
input.env.clone(),
282282
input.description.clone(),
283283
)

crates/forge_domain/src/tools/call/context.rs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,17 @@ use crate::{ArcSender, ChatResponse, Metrics, TitleFormat, Todo, TodoItem};
99
pub struct ToolCallContext {
1010
sender: Option<ArcSender>,
1111
metrics: Arc<Mutex<Metrics>>,
12+
/// When set to true, shell tool output is streamed to io::sink() instead
13+
/// of io::stdout() to avoid contaminating the ACP JSON-RPC transport
14+
/// channel. The field is intentionally NOT in ForgeConfig — it is a
15+
/// per-invocation runtime behaviour flag, not persisted state.
16+
pub silent: bool,
1217
}
1318

1419
impl ToolCallContext {
1520
/// Creates a new ToolCallContext with default values
1621
pub fn new(metrics: Metrics) -> Self {
17-
Self { sender: None, metrics: Arc::new(Mutex::new(metrics)) }
22+
Self { sender: None, metrics: Arc::new(Mutex::new(metrics)), silent: false }
1823
}
1924

2025
/// Send a message through the sender if available

crates/forge_infra/src/executor.rs

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,13 @@ impl ForgeCommandExecutorService {
108108
let mut stdout_pipe = child.stdout.take();
109109
let mut stderr_pipe = child.stderr.take();
110110

111-
// Stream the output of the command to stdout and stderr concurrently
111+
// Stream the output of the command to stdout and stderr concurrently.
112+
//
113+
// TODO(acp): when `silent` is true (ACP stdio mode), output is
114+
// streamed to io::sink() so that raw command output does not
115+
// contaminate the ACP JSON-RPC transport on stdout. The captured
116+
// output is still returned via CommandOutput for the ACP
117+
// notification channel.
112118
let (status, stdout_buffer, stderr_buffer) = if silent {
113119
tokio::try_join!(
114120
child.wait(),

crates/forge_main/src/cli.rs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,15 @@ impl Cli {
7575
pub fn is_interactive(&self) -> bool {
7676
self.prompt.is_none() && self.piped_input.is_none() && self.subcommands.is_none()
7777
}
78+
79+
/// Checks if the subcommand owns stdin and should skip the piped-input
80+
/// pre-read.
81+
///
82+
/// Commands like `forge select` interactively read from stdin and would
83+
/// hang if the startup pipeline consumed stdin first.
84+
pub fn uses_stdin(&self) -> bool {
85+
matches!(&self.subcommands, Some(TopLevelCommand::Select(_)))
86+
}
7887
}
7988

8089
#[derive(Subcommand, Debug, Clone)]

crates/forge_main/src/main.rs

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ use clap::Parser;
77
use forge_api::ForgeAPI;
88
use forge_config::ForgeConfig;
99
use forge_domain::TitleFormat;
10-
use forge_main::{Cli, Sandbox, TitleDisplayExt, TopLevelCommand, UI, tracker};
10+
use forge_main::{Cli, Sandbox, TitleDisplayExt, UI, tracker};
1111

1212
/// Enables ENABLE_VIRTUAL_TERMINAL_PROCESSING on the stdout console handle.
1313
///
@@ -82,18 +82,17 @@ async fn run() -> Result<()> {
8282
"Unexpected error occurred".to_string()
8383
};
8484

85-
println!("{}", TitleFormat::error(message.to_string()).display());
85+
eprintln!("{}", TitleFormat::error(message.to_string()).display());
8686
tracker::error_blocking(message);
8787
std::process::exit(1);
8888
}));
8989

9090
// Initialize and run the UI
9191
let mut cli = Cli::parse();
9292

93-
// Check if there's piped input, but skip for `forge select` since that
94-
// command uses stdin for its item list.
95-
let is_select = matches!(cli.subcommands, Some(TopLevelCommand::Select(_)));
96-
if !is_select && !std::io::stdin().is_terminal() {
93+
// Check if there's piped input, but skip for commands that use stdin
94+
// for their own protocol (e.g. `forge select`).
95+
if !cli.uses_stdin() && !std::io::stdin().is_terminal() {
9796
let mut stdin_content = String::new();
9897
std::io::stdin().read_to_string(&mut stdin_content)?;
9998
let trimmed_content = stdin_content.trim();

crates/forge_main/src/sandbox.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ impl<'a> Sandbox<'a> {
6969
.context("Failed to check if target directory is a git worktree")?;
7070

7171
if worktree_check.status.success() {
72-
println!(
72+
eprintln!(
7373
"{}",
7474
TitleFormat::info("Worktree [Reused]")
7575
.sub_title(worktree_path.display().to_string())
@@ -125,7 +125,7 @@ impl<'a> Sandbox<'a> {
125125
bail!("Failed to create git worktree: {stderr}");
126126
}
127127

128-
println!(
128+
eprintln!(
129129
"{}",
130130
TitleFormat::info("Worktree [Created]")
131131
.sub_title(worktree_path.display().to_string())

0 commit comments

Comments
 (0)