Skip to content

fix(cli): generic watch-path error message and sys.argv sync for programmatic main()#25

Merged
mahimairaja merged 2 commits intocursor/tui-watch-path-handling-3694from
copilot/sub-pr-23-again
Mar 22, 2026
Merged

fix(cli): generic watch-path error message and sys.argv sync for programmatic main()#25
mahimairaja merged 2 commits intocursor/tui-watch-path-handling-3694from
copilot/sub-pr-23-again

Conversation

Copy link
Contributor

Copilot AI commented Mar 22, 2026

Two follow-up fixes from code review on the TUI defaults / positional-paths PR.

validate_metrics_watch_path error message was --watch-specific

The error said '--watch' must be a JSONL file path… but the watch path can also be supplied positionally (openrtc tui ./file.jsonl). Reworded to The metrics watch path must be a JSONL file path….

main(argv=...) didn't update sys.argv

When called programmatically, the argv is not None branch passed injected args to cli.main(args=…) but left sys.argv unchanged. The LiveKit handoff (_livekit_sys_argv) inspects sys.argv to detect the subcommand and forward flags like --reload, so programmatic calls like main(["dev", "./agents", "--reload"]) would silently drop those flags. Now sets sys.argv = [previous_argv[0], *injected_args] before cli.main, matching the existing sys.argv-rewrite path for the no-argv branch. Restored in finally as before.

# before: sys.argv unchanged, LiveKit handoff missed --reload
main(["dev", "./agents", "--reload"])

# after: sys.argv mirrors injected args during execution, restored in finally

📱 Kick off Copilot coding agent tasks wherever you are with GitHub Mobile, available on iOS and Android.

…ys.argv for programmatic main(argv=...)

Co-authored-by: mahimairaja <81288263+mahimairaja@users.noreply.github.com>
Agent-Logs-Url: https://github.com/mahimairaja/openrtc-python/sessions/8c419ba3-63a6-4ee9-9afc-eed8d22cd894
Copy link
Owner

@mahimairaja mahimairaja left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@mahimairaja mahimairaja marked this pull request as ready for review March 22, 2026 19:03
Copilot AI review requested due to automatic review settings March 22, 2026 19:03
@mahimairaja mahimairaja merged commit a25b493 into cursor/tui-watch-path-handling-3694 Mar 22, 2026
5 checks passed
Copilot AI changed the title [WIP] Fix TUI defaults and clearer --watch handling fix(cli): generic watch-path error message and sys.argv sync for programmatic main() Mar 22, 2026
Copilot AI requested a review from mahimairaja March 22, 2026 19:04
Copilot stopped work on behalf of mahimairaja due to an error March 22, 2026 19:04
@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR improves OpenRTC CLI/TUI ergonomics by clarifying metrics watch-path validation and ensuring programmatic CLI invocations correctly mirror sys.argv for the LiveKit handoff logic.

Changes:

  • Update validate_metrics_watch_path() error text to avoid implying the path must come specifically from --watch (since it may be positional).
  • When calling cli_app.main(argv=...) programmatically, set sys.argv to the injected/re-written args so LiveKit handoff logic sees the correct tokens (e.g. --reload).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
src/openrtc/tui_app.py Makes the directory-path validation error message more accurate and less flag-specific.
src/openrtc/cli_app.py Ensures programmatic main(argv=...) updates sys.argv with injected args so LiveKit handoff logic observes the same argv as a real CLI invocation.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@codecov
Copy link

codecov bot commented Mar 22, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.15%. Comparing base (23224dc) to head (e43bd81).
⚠️ Report is 3 commits behind head on cursor/tui-watch-path-handling-3694.

Additional details and impacted files

Impacted file tree graph

@@                           Coverage Diff                           @@
##           cursor/tui-watch-path-handling-3694      #25      +/-   ##
=======================================================================
+ Coverage                                90.14%   90.15%   +0.01%     
=======================================================================
  Files                                       13       13              
  Lines                                     1420     1422       +2     
=======================================================================
+ Hits                                      1280     1282       +2     
  Misses                                     140      140              
Files with missing lines Coverage Δ
src/openrtc/cli_app.py 92.85% <100.00%> (+0.17%) ⬆️
src/openrtc/tui_app.py 100.00% <ø> (ø)

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 23224dc...e43bd81. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@mahimairaja mahimairaja deleted the copilot/sub-pr-23-again branch March 23, 2026 17:07
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.

4 participants