Skip to content

feat(cli): stream output on CLI command start#444

Merged
jdx merged 4 commits into
jdx:mainfrom
gaojunran:feat-stream-output
May 18, 2026
Merged

feat(cli): stream output on CLI command start#444
jdx merged 4 commits into
jdx:mainfrom
gaojunran:feat-stream-output

Conversation

@gaojunran

Copy link
Copy Markdown
Contributor

No description provided.

@socket-security

socket-security Bot commented May 18, 2026

Copy link
Copy Markdown

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff Package Supply Chain
Security
Vulnerability Quality Maintenance License
Updatedcargo/​nix@​0.31.2 ⏵ 0.31.3801009310070
Updatedcargo/​rmcp@​1.5.0 ⏵ 1.7.089 +1100100100100

View full report

@greptile-apps

greptile-apps Bot commented May 18, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR replaces the post-start batch log display with real-time log streaming during daemon startup, backed by a new clx-based progress display. Errors/warnings that were previously swallowed while the progress spinner was active are now surfaced correctly via clx::progress::pause()/resume().

  • stream_startup_logs spawns a background poll task that calls job.println() for each new log line; the task is awaited before returning SpawnTaskResult, eliminating the previous race with update_job_with_result.
  • IpcClient::run now returns structured error_message fields instead of logging inline, letting the caller decide how to surface failure details in progress jobs or quiet mode.
  • All TUI/web/proxy callers correctly set quiet: true so no stray progress jobs are created outside the CLI layer.

Confidence Score: 5/5

Safe to merge — the streaming concurrency is well-structured and all previously-flagged cleanup/exit-code issues are addressed.

The core streaming loop correctly awaits log tasks before touching job state, the logger now properly interleaves error output with the progress display, and the deferred-update pattern keeps start/restart/run consistent. The two remaining findings are a silent drop of multi-line continuation log lines in the polling phase and a stale code comment — neither affects correctness of the daemon lifecycle or exit codes.

src/cli/logs.rs — the stream_startup_logs polling loop and create_ready_check_job are the newest and most complex additions; multi-line log handling deserves a second look.

Important Files Changed

Filename Overview
src/cli/logs.rs Major rework: replaces batch post-start log display with a real-time streaming approach; multi-line log continuation handling is inconsistent between the initial batch read and the polling loop.
src/ipc/batch.rs Significant refactor to carry progress jobs and deferred updates; streaming tasks are now correctly awaited before returning SpawnTaskResult.
src/cli/run.rs Properly handles both Ok and Err paths, stops/clears progress display, and exits non-zero on failure.
src/cli/start.rs Applies deferred job updates and stops progress display after all tasks complete.
src/cli/restart.rs Now applies deferred job updates and calls clx::progress::stop()/clear_jobs().
src/ipc/client.rs Converts inline error logging to structured error_message fields in RunResult.
src/logger.rs Wraps terminal output with clx::progress::pause()/resume() to prevent interleaving.
src/ui/style.rs Replaced local style helpers with pub use clx::style::*.
src/tui/mod.rs All TUI start paths now pass quiet: true.

Reviews (13): Last reviewed commit: "fix(logs): await streaming task before t..." | Re-trigger Greptile

Comment thread src/cli/run.rs Outdated
Comment thread src/cli/run.rs Outdated
Comment thread src/cli/logs.rs
Comment thread src/cli/run.rs Outdated
- Add startup log streaming with real-time output display
- Add progress job updates for ready check status
- Add startup_log_timestamps setting for optional timestamps
- Remove local clx patch, use published v2.1.0 from crates.io
- Fix read_from_offset to preserve partial lines at EOF
- Fix restart.rs to apply pending job updates and clear jobs
- Fix run.rs duplicate error logging and use update_job_with_result
- Run render to regenerate CLI docs and schema
@gaojunran gaojunran force-pushed the feat-stream-output branch from 4d3941d to b24e902 Compare May 18, 2026 09:19
@gaojunran

Copy link
Copy Markdown
Contributor Author

@greptileai

Comment thread src/ipc/batch.rs
Comment thread src/logger.rs Outdated
junrangao added 2 commits May 18, 2026 19:05
…sage in non-quiet mode

- Remove active_jobs() > 0 guard that silently dropped all error! and
  warn! terminal output during progress display. The guard prevented
  critical messages like "Dependency failed, aborting remaining starts"
  from reaching the user.

- Include run_result.error_message in the progress job body for failed
  daemons in non-quiet mode, so users see the specific failure reason
  (e.g. port conflict details) instead of just "[daemon] failed (exit code 1)".
@gaojunran

Copy link
Copy Markdown
Contributor Author

@greptileai

Comment thread src/ipc/batch.rs Outdated
@gaojunran

Copy link
Copy Markdown
Contributor Author

@greptileai

Comment thread src/ipc/batch.rs Outdated
@gaojunran gaojunran force-pushed the feat-stream-output branch from 60630ae to 0ba6a15 Compare May 18, 2026 12:17
Comment thread src/cli/logs.rs
@gaojunran gaojunran force-pushed the feat-stream-output branch from 0ba6a15 to 39ddfa9 Compare May 18, 2026 12:32
Comment thread src/cli/run.rs Outdated
@gaojunran gaojunran force-pushed the feat-stream-output branch from 39ddfa9 to 7632e17 Compare May 18, 2026 12:40
@gaojunran

Copy link
Copy Markdown
Contributor Author

@greptileai

Comment thread src/ipc/batch.rs Outdated
@gaojunran gaojunran force-pushed the feat-stream-output branch from 7632e17 to 47a98d1 Compare May 18, 2026 13:00
@gaojunran

Copy link
Copy Markdown
Contributor Author

@greptileai

@gaojunran gaojunran marked this pull request as ready for review May 18, 2026 13:11
@jdx jdx merged commit ad18961 into jdx:main May 18, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants