Skip to content

fix(mcp): avoid probing JSON-RPC stdin for explicit batch input #293

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
EricLingRui wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(mcp): avoid probing JSON-RPC stdin for explicit batch input #293

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 22 additions & 10 deletions src/officecli/CommandBuilder.Batch.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,21 +172,41 @@ private static Command BuildBatchCommand(Option<bool> jsonOption)
var bestEffort = result.GetValue(batchBestEffortOpt);

string jsonText;
if (inlineCommands != null && inputFile != null)
throw new ArgumentException(
"batch: --commands and --input are mutually exclusive. Pick one source.");
// '--input -' explicitly opts INTO stdin — don't emit the
// "stdin will be ignored" warning in that case, since stdin
// is exactly what will be read.
var inputIsStdinAlias = inputFile != null && inputFile.Name == "-";
// BUG-R7-09 (F-6): previously --commands/--input/stdin were
// silently prioritized in that order — passing two of them at
// once dropped the lower-priority source with no warning, so
// scripts could fail subtly when an agent piped data into a
// command that already had --commands set. Reject the
// combination loudly. (Detect stdin via Console.IsInputRedirected
// to avoid spurious failures from interactive terminals.)
//
// Only probe stdin when all of these are true: an explicit
// non-stdin source will make us ignore stdin, the warning has not
// been disabled, and stdin is redirected. This is a correctness
// boundary for MCP: its stdin is the JSON-RPC transport. Starting
// a background Peek there and abandoning it after the timeout
// leaves a live reader that consumes the next JSON-RPC request.
// OFFICECLI_BATCH_ALLOW_STDIN_REDIRECT is enabled by McpServer, so
// MCP calls with --commands/--input must never touch stdin here.
// IsInputRedirected alone is true for every non-interactive
// invocation (cron, CI, `< /dev/null`, systemd), so the warning
// below fired on effectively all scripted batch runs with only
// one source supplied. Refine: a seekable stdin (regular file or
// /dev/null redirect) with zero length carries no second payload
// — skip the warning. Pipes (CanSeek=false) still warn: someone
// is actively piping data that will be ignored.
bool stdinHasInput = Console.IsInputRedirected;
var hasExplicitNonStdinSource = inlineCommands != null
|| (inputFile != null && !inputIsStdinAlias);
var shouldWarnAboutIgnoredStdin = hasExplicitNonStdinSource
&& Environment.GetEnvironmentVariable("OFFICECLI_BATCH_ALLOW_STDIN_REDIRECT") == null;
bool stdinHasInput = shouldWarnAboutIgnoredStdin && Console.IsInputRedirected;
if (stdinHasInput)
{
// Peek with a short timeout: /dev/null and closed stdin hit
Expand All @@ -207,15 +227,7 @@ private static Command BuildBatchCommand(Option<bool> jsonOption)
}
catch { /* keep IsInputRedirected verdict */ }
}
if (inlineCommands != null && inputFile != null)
throw new ArgumentException(
"batch: --commands and --input are mutually exclusive. Pick one source.");
// '--input -' explicitly opts INTO stdin — don't emit the
// "stdin will be ignored" warning in that case, since stdin
// is exactly what will be read.
var inputIsStdinAlias = inputFile != null && inputFile.Name == "-";
if ((inlineCommands != null || (inputFile != null && !inputIsStdinAlias)) && stdinHasInput
&& Environment.GetEnvironmentVariable("OFFICECLI_BATCH_ALLOW_STDIN_REDIRECT") == null)
if (stdinHasInput)
{
Console.Error.WriteLine(
"Warning: batch is reading from --commands/--input but stdin is also redirected; "
Expand Down