Make subprocess stderr routing an explicit policy (#340)#372
Conversation
Diagnostic JSON mode was threaded into process execution as a bare boolean that silently changed child stderr forwarding, leaking a reporting decision into subprocess transport handling. Replace the flag with a named `StderrMode` policy (`Forward` / `Suppress`). The runner chooses the policy from CLI state at the orchestration boundary (`ninja_process_options`); the process layer only consumes it. Under `Suppress`, child stderr is still drained but discarded, keeping stderr machine-readable for JSON diagnostics exactly as before.
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Reviewer's GuideReplace the boolean stderr suppression flag with an explicit StderrMode policy and thread it through the runner and process layers so stderr routing is decided at the runner boundary while preserving existing JSON-diagnostics behavior. Sequence diagram for stderr_mode selection and routingsequenceDiagram
actor User
participant Cli
participant Runner
participant Process
User->>Cli: set resolved_diag_json()
Runner->>Runner: ninja_process_options(cli)
Runner->>Runner: set stderr_mode (Forward or Suppress)
Runner->>Process: run_ninja_build_internal(request.options.stderr_mode)
Process->>Process: run_command_and_stream(cmd, status_observer, stderr_mode)
Process->>Process: spawn_and_stream_output(child, status_observer, stderr_mode)
alt [stderr_mode.is_suppressed()]
Process->>Process: forward_child_output(stderr, io::sink(), stderr)
else [not suppressed]
Process->>Process: forward_child_output(stderr, io::stderr(), stderr)
end
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Summary
Closes #340
Replaces the
suppress_stderrboolean with the named policy type the issue proposes:ninja_process_options(the only place that consultsCli::resolved_diag_json(); the process layer stopped calling it in Decouple runner process execution from Cli (#339) #371).NinjaProcessOptions,run_command_and_stream, andspawn_and_stream_outputconsume the policy.Suppress, child stderr is still drained but discarded, so JSON diagnostics keep stderr machine-readable exactly as before.Stacked on #371 (designed together per #347); retargets to
mainautomatically once #371 merges.Validation
make check-fmt/make lint/make test— pass (37 suites; JSON-diagnostics BDD scenarios unchanged and green)🤖 Generated with Claude Code
Summary by Sourcery
Introduce an explicit stderr routing policy for subprocesses and wire it through the runner and process layers.
Enhancements:
StderrModeenum to make subprocess stderr handling explicit and self-describing.NinjaProcessOptionsand subprocess execution helpers so JSON diagnostics can suppress child stderr while still draining it.