fix(cli): gate remaining dashboard URL prints behind is_verbose() for --silent compliance#211
Merged
Merged
Conversation
Contributor
Author
|
@microsoft-github-policy-service agree |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #211 +/- ##
=======================================
Coverage ? 88.15%
=======================================
Files ? 60
Lines ? 9658
Branches ? 0
=======================================
Hits ? 8514
Misses ? 1144
Partials ? 0 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
jrob5756
added a commit
that referenced
this pull request
May 21, 2026
- feat(script): script agents output schemas (#206, #118) - feat(validate): warn on undeclared agent.output refs and field-level mismatches in explicit mode (#208) - feat(copilot): attribute verbose logs to agents in parallel/for-each runs (#207) - fix(resume): replay original event log into dashboard on --web (#167, #205) - fix(windows): make --web-bg startup crashes diagnosable (#116, #204) - fix(engine,web): resolve max-iterations gate from dashboard in --web-bg (#202) - fix(bg): detach --web-bg child from Windows job to prevent kill-on-close (#200) - fix(bg): stop passing redundant --silent to bg child (#199, #196) - fix(cli): suppress web-bg dashboard output in silent mode (#203, #211) - fix(config): auto-fetch sibling sub-workflow from registry cache during validation (#197) - fix(registry): mirror repo layout in cache so cross-workflow refs resolve (#194) - fix(copilot): tolerate SDK metadata parsing errors when listing models (#193) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
jrob5756
added a commit
that referenced
this pull request
May 21, 2026
…209) (#223) * fix(cli): make _verbose_console silent-aware and gate replay prints (#209) PR #211 (follow-up to #203) gated three foreground dashboard URL prints behind `is_verbose()` but left the remaining stderr leaks in place, violating the `--silent` contract ("No progress output. Only JSON result on stdout."). Specifically: * `app.py:_run_replay` still printed 'Press Ctrl+C to exit' and 'Replay stopped' unconditionally. * `run.py` had ~12 `_verbose_console.print` call sites that bypassed `is_verbose()` — warnings (dashboard failed to start, log-file open failure, workflow-hash mismatch), 'Press Esc to interrupt', 'Event log written to...', 'Log written to...', and `_print_resume_instructions` (printed on failure). Fix at the source: subclass `Console` for `_verbose_console` so every `.print(...)` no-ops when `is_verbose()` is False. This aligns the implementation with the long-misleading name, removes the per-call-site audit burden, and is a no-op for the already-gated helpers. The app-wide `console` in app.py is intentionally NOT made silent-aware because it carries real error messages; the two remaining replay prints are gated per-call instead. Acceptance criterion verified: `conductor --silent replay <log>` produces 0 bytes on stderr (was leaking 'Press Ctrl+C to exit'). Regression tests: * `TestSilentAwareConsole` (3 tests) — verifies the subclass mechanism and that the module-level instance uses it. * `TestReplaySilentCompliance` (2 tests) — verifies the replay command produces no stderr under `--silent` and still prints when verbose. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(cli): address PR #223 review findings (#209) Comprehensive PR review surfaced one critical coverage gap and a few defensive improvements. All addressed in this commit: * **Critical (pr-test-analyzer)**: The `KeyboardInterrupt` branch at `app.py:1007-1009` (`Replay stopped`) had zero behavioral test coverage — the previous `CancelledError`-via-`asyncio.Event` path never reaches the outer `except KeyboardInterrupt`. Added `test_silent_replay_suppresses_keyboardinterrupt_message` and its verbose counterpart that drive that branch via `patch("asyncio.run", side_effect=KeyboardInterrupt())`. This is also more robust than the inner-path mocking: a future refactor of `_run_replay`'s wait primitive cannot silently bypass it. * **Important (type-design-analyzer)**: `_SilentAwareConsole.__init__` now locks `stderr=True`. A future caller constructing an instance with `stderr=False` would have silently routed gated output to stdout and corrupted the `--silent` JSON contract. * **Important (comment-analyzer)**: Tests now assert on `result.stderr` (not the combined `result.output`), matching the test names and catching a hypothetical regression that moved the prints to stdout. * **Suggestion (pr-test-analyzer)**: Added `test_quiet_replay_prints_dashboard_messages` to lock in the contract at `app.py:204` that `--quiet` (MINIMAL) keeps `verbose_mode=True` — MINIMAL means "limited progress", not zero. * **Suggestion (multiple agents)**: Class-level comment on `_SilentAwareConsole` warns that only `.print()` is gated; `.rule()`/`.log()`/`.status()`/`.print_json()` would silently bypass `--silent`. All current call sites use only `.print`. Verified: `conductor --silent replay <log>` still produces 0 bytes on stderr; `conductor --quiet replay <log>` still prints the dashboard URL and Ctrl+C hint as before. 400 `tests/test_cli` tests pass (was 397; +3 net new). Lint/format/typecheck clean. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.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
3 participants
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.
Description
Follow-up to PR #203 (merged, by @sjh9714). PR #203 fixed the web-bg startup dashboard URL leak in
src/conductor/cli/app.py, but the--silentflag is supposed to suppress ALL dashboard URL output, not just the startup print.Remaining fixes
This PR gates the remaining dashboard URL prints behind
is_verbose():src/conductor/cli/run.py—run_workflow_asyncfunction (line 1174): The--silentflag should suppress this dashboard URL print during foreground --web runs, not just web-bg mode (fix(cli): suppress web-bg dashboard output in silent mode #203's scope).src/conductor/cli/run.py—resumefunction (line 1832): Same issue in the resume code path — dashboard URL printed regardless of --silent.src/conductor/cli/app.py—_run_replayfunction (line 994): Replay dashboard URL leaked even in --silent mode.src/conductor/cli/run.py— "Workflow complete. Dashboard still running at..." shutdown messages (lines 1321, 1884): Both therunandresumefunctions printed the dashboard URL during shutdown regardless of --silent mode.Testing
Existing tests from #203 should continue to pass. The fix follows the same pattern as #203: lazy-import
is_verbose()fromapp.pyand gate the print call.Co-authored-by: sjh9714 sjh9714@users.noreply.github.com