Conversation
Fix 5 review issues identified after initial Epic 1 implementation:
1. Critical: log_file parameter in run_workflow_async was never used.
- Added init_file_logging() and close_file_logging() functions
- Wired try/finally lifecycle in run_workflow_async()
- Log file path printed to stderr on completion
2. High: generate_log_path() didn't create parent directory.
- Added mkdir(parents=True, exist_ok=True) before returning path
3. Medium: no tests verified file output.
- Added 8 TestFileLogging integration tests covering file creation,
plain-text output, untruncated content, auto/explicit paths,
CI pattern (--silent --log-file), and OSError handling
4. Low: mutual exclusion test assertion too broad.
- Tightened to assert specifically on 'mutually exclusive' substring
5. copilot.py _log_event_verbose updated for dual-write.
- Added _print() helper that writes to both stderr Console and
_file_console when file logging is active
All 1099 tests pass (42 in test_logging.py, 1057 across the rest).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Update verbose_log_section() to gate console output on is_full() so MINIMAL mode (--quiet) skips sections entirely instead of truncating. Remove the 500-char truncation logic and truncate parameter since FULL is now the default and MINIMAL skips sections. Verified that verbose_log(), verbose_log_agent_start/complete(), verbose_log_route(), verbose_log_timing() already work correctly for all three verbosity levels. Confirmed _log_event_verbose() in copilot.py already gates tool args/results/reasoning on full_mode. Added 10 new tests for FULL/MINIMAL/SILENT behavior and updated 3 existing section tests to match the new semantics. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add 22 new tests across 3 test classes for file logging functionality: - TestFileLoggingDualWrite: 15 tests verifying all verbose_log_* functions and display_usage_summary write to file console independently of console verbosity (including silent mode) - TestFileLoggingStderrNotification: 3 tests for stderr log path notification, file handle cleanup on error, and graceful handling when init fails - TestFileLoggingErrorHandling: 4 tests for permission denied, graceful error recovery in run_workflow_async, idempotent close, and ANSI-free file output All acceptance criteria for Epic 3 are now verified by tests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Fix docs/cli-reference.md line 78: add explicit 'auto' value after --log-file to prevent Typer from consuming --skip-gates as the log file path. The --log-file option is str|None and requires an explicit value; without 'auto', the next token is consumed silently. All other --log-file occurrences in docs already supply an explicit path or are in design documents (not runnable examples). Consistent with in-source examples in app.py lines 267–269. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… docs - Replace 'Check verbose output (-V) for details.' with 'Enable --log-file to capture full debug output.' in idle recovery error message, since -V was removed in the logging redesign - Update --log-file table entry from '[PATH]' to '<auto|PATH>' to accurately reflect that a value is required and what the valid values are Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replace removed -V/--verbose flag with --quiet/-q, --silent/-s, and --log-file/-l options to match cli-reference.md. 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
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.
Logging Redesign
Replaces the current
--verbose/-Vflag with a two-dimensional logging model.Console Output
--quiet/-q) — agent start/complete, routing, timing only--silent/-s) — no progress output, only final JSON on stdoutFile Output
--log-file/-l) — writes to temp directory--log-file PATH) — writes to specified pathFile output is always full/untruncated regardless of console level.
Changes
--verbose/-Vremoved entirely--quiet/-qand--silent/-sflags added--log-file/-lflag added with optional pathSee docs/projects/planned-features.md and docs/projects/planned-features-logging-redesign.plan.md for full specification.