fix(bg): stop passing redundant --silent to bg child (#196)#199
Merged
Conversation
jrob5756
force-pushed
the
fix/196-bg-runner-redundant-silent
branch
from
8e0638f to
02e4870
Compare
`bg_runner.launch_background()` and `launch_background_resume()` already redirect the child's `stdout`/`stderr`/`stdin` to `subprocess.DEVNULL`, so silence is enforced at the OS level. Passing `--silent` in addition to that adds nothing visible to the user — but it does flip `verbose_mode=False` in the child, which gates more than just console prints: * `copilot.py` skips `_log_event_verbose()` for SDK events * `_log_parse_recovery()` and `_log_recovery_attempt()` are no-ops `_log_event_verbose()` writes to both stderr and `_file_console`, so when `--silent` is set those events are dropped from the log file too. That means `conductor run --web-bg --log-file auto` produces a log file with no provider-side trace — exactly when you most want one. Drop `--silent` from both synthesized commands. Console output still goes to DEVNULL via the Popen kwargs, and `--log-file` now captures a real trace of the child's behavior. Side benefit: the synthesized command is now reproducible by hand without learning that `--silent` was being injected behind the scenes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
jrob5756
force-pushed
the
fix/196-bg-runner-redundant-silent
branch
from
02e4870 to
4728779
Compare
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
1 participant
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.
Fixes #196.
Problem
bg_runner.launch_background()andlaunch_background_resume()already redirect the child'sstdout/stderr/stdintosubprocess.DEVNULL, so silence is enforced at the OS level. Passing--silentin addition to that adds nothing visible to the user — but it does flipverbose_mode=Falsein the child, and that gates real behavior, not just console prints:copilot.py:867-868—_log_event_verbose()is skipped for SDK eventscopilot.py:701-706—_log_parse_recovery()is skippedcopilot.py:1619-1620—_log_recovery_attempt()is skippedCritically,
_log_event_verbose()writes to both a stderr Console and_file_console. So when--silentis set, those events are dropped from the log file too. That meansconductor run --web-bg --log-file autotoday produces a log file with no provider-side trace — exactly when you most want one for debugging a detached process.By contrast, the
verbose_log_*helpers incli/run.pywrite to the file unconditionally, so the engine-level trace already survives--silent. Only the provider-side SDK events are the casualty, which makes the impact specifically about debugging stuck/looping agent sessions in bg mode.Fix
Drop
--silentfrom both synthesized commands. Console output still goes to DEVNULL via the Popen kwargs (no visible change for users). The startup update hint is also gated onconsole.is_terminal, which is False when stderr=DEVNULL, so that doesn't reappear either.Side benefit: the synthesized command is now reproducible by hand — useful when diagnosing related issues like #195.
Changes
src/conductor/cli/bg_runner.py: remove--silentfrom both `cmd` lists; replace the inline comment with a longer block explaining why it's absent and pointing at this issue.tests/test_cli/test_resume_command.py: flip the existing `--silent` assertion in `test_builds_resume_subcommand_with_workflow` from "in cmd" to "not in cmd" so the contract is locked in.tests/test_cli/test_web_flags.py: add `TestLaunchBackgroundSilentFlag::test_launch_background_does_not_pass_silent` — the run-side parity test that didn't exist before.Validation
Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com