🎨 Palette: Interactive 'Run Now' after Dry Run#411
Conversation
- Adds a prompt to press Enter to execute live sync immediately after dry run summary. - Uses `os.execv` to restart the process cleanly. - Preserves all command-line arguments (filtering out --dry-run) for forward compatibility. - Preserves environment variables, including interactive credentials (TOKEN). - Only activates in interactive TTY sessions (safe for CI). Co-authored-by: abhimehro <84992105+abhimehro@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
😎 Merged manually by @abhimehro - details. |
|
👋 Development Partner is reviewing this PR. Will provide feedback shortly. |
Summary of ChangesHello @abhimehro, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the user experience for the CLI tool by introducing an interactive "Run Now" option after a dry run. This change streamlines the workflow by eliminating the need for users to manually copy and paste commands to proceed with a live sync, while intelligently preserving all original arguments and securely handling API tokens. The feature is designed to be user-friendly and safe for both interactive sessions and automated environments. Highlights
Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| **Learning:** Using Unicode box drawing characters enhances the CLI experience, but a robust ASCII fallback is crucial for CI environments and piped outputs. | ||
| **Action:** Always implement a fallback mechanism (like checking `sys.stderr.isatty()`) when using rich text or Unicode symbols. | ||
|
|
||
| ## 2025-02-28 - [Interactive Restart] |
Check notice
Code scanning / Remark-lint (reported by Codacy)
Warn when references to undefined definitions are found. Note
| **Learning:** Using Unicode box drawing characters enhances the CLI experience, but a robust ASCII fallback is crucial for CI environments and piped outputs. | ||
| **Action:** Always implement a fallback mechanism (like checking `sys.stderr.isatty()`) when using rich text or Unicode symbols. | ||
|
|
||
| ## 2025-02-28 - [Interactive Restart] |
Check notice
Code scanning / Remark-lint (reported by Codacy)
Warn when shortcut reference links are used. Note
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a great usability improvement by adding an interactive 'Run Now' prompt after a successful dry run. The implementation is well-thought-out, using os.execv for a clean restart and securely passing credentials via environment variables. The logic to preserve arguments is also mostly robust. I've found one potential bug in the argument reconstruction that could cause issues when arguments are passed with an equals sign, and I've provided a suggestion to fix it. Otherwise, the changes look solid.
main.py
Outdated
| if "--profiles" not in sys.argv and profile_ids: | ||
| new_argv.extend(["--profiles", ",".join(profile_ids)]) |
There was a problem hiding this comment.
The check "--profiles" not in sys.argv is not robust enough. It fails to detect the --profiles argument when it's passed with an equals sign, like --profiles=p1,p2. This can lead to the --profiles argument being duplicated in the restarted command, potentially causing it to fail or behave unexpectedly.
A more robust check would be to see if any argument starts with "--profiles".
| if "--profiles" not in sys.argv and profile_ids: | |
| new_argv.extend(["--profiles", ",".join(profile_ids)]) | |
| if not any(arg.startswith("--profiles") for arg in sys.argv) and profile_ids: | |
| new_argv.extend(["--profiles", ",".join(profile_ids)]) |
- Adds a prompt to press Enter to execute live sync immediately after dry run summary. - Uses `os.execv` to restart the process cleanly. - Preserves all command-line arguments (filtering out --dry-run) for forward compatibility. - Preserves environment variables, including interactive credentials (TOKEN). - Only activates in interactive TTY sessions (safe for CI). - Refactored `main()` to extract `prompt_for_interactive_restart` logic to fix CI complexity warning. - Suppressed Bandit B606 warning for `os.execv` call as it is intended behavior. Co-authored-by: abhimehro <84992105+abhimehro@users.noreply.github.com>
|
👋 Development Partner is reviewing this PR. Will provide feedback shortly. |
There was a problem hiding this comment.
Pull request overview
This PR adds an interactive "Run Now" feature that prompts users to immediately restart the tool in live mode after a successful dry run. When running in an interactive terminal (TTY), users can simply press Enter to automatically re-execute the command without --dry-run, eliminating the need to manually copy and paste the suggested command.
Changes:
- Added interactive prompt after successful dry runs that detects TTY environments and offers instant restart via
os.execv - Implemented argument preservation logic that filters out
--dry-runwhile maintaining other user-provided flags - Added automatic profile injection when profiles were provided interactively rather than via command-line arguments
- Documented the learning about using
sys.argvfiltering instead of manual argument reconstruction
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| main.py | Added interactive restart feature with TTY detection, input prompt, argument filtering, and process re-execution using os.execv |
| .Jules/palette.md | Documented the learning about using sys.argv filtering for forward-compatible argument handling during process restarts |
| "🎨 Beautifully done!", | ||
| "💎 Smooth operation!", |
There was a problem hiding this comment.
Setting TOKEN in os.environ makes the API token accessible to any subprocess spawned by the relaunched process and persists in the environment beyond the intended scope. This creates a security risk as the token could be leaked through process listings, error messages, or logs that dump environment variables. Consider passing the token through a more secure mechanism, such as stdin, a temporary file with restricted permissions, or a command-line argument that's immediately consumed and cleared.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
👋 Development Partner is reviewing this PR. Will provide feedback shortly. |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
👋 Development Partner is reviewing this PR. Will provide feedback shortly. |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
👋 Development Partner is reviewing this PR. Will provide feedback shortly. |
1 similar comment
|
👋 Development Partner is reviewing this PR. Will provide feedback shortly. |
- Verified main.py syntax locally (passed pytest and import checks). - Re-submitting to resolve potential CI cache/merge artifact issues. - The interactive restart logic was refactored to `prompt_for_interactive_restart` to reduce complexity. - B606 warning suppressed as `os.execv` is intentional. Co-authored-by: abhimehro <84992105+abhimehro@users.noreply.github.com>
|
👋 Development Partner is reviewing this PR. Will provide feedback shortly. |
abhimehro
deleted the
palette/interactive-dry-run-17891995426056761148
branch
🎨 Palette: Interactive 'Run Now' after Dry Run
💡 What:
Added a small but delightful interaction: after a successful dry run, the tool now asks "🚀 Ready to launch? Press [Enter] to run now...".
🎯 Why:
Previously, users had to copy-paste the suggested command manually to proceed with the live sync. This friction is now removed for interactive sessions.
✨ UX Details:
♿ Accessibility:
PR created automatically by Jules for task 17891995426056761148 started by @abhimehro