fix: replace exit statements with returns in automated-pr-reviewer skill#680
fix: replace exit statements with returns in automated-pr-reviewer skill#680badMade wants to merge 8 commits into
Conversation
The `exit` statements in the `automated-pr-reviewer` bash script would prematurely terminate interactive bash sessions when the script was pasted into an agent's terminal or sourced. This commit wraps the bash logic in a `main()` function and changes the top-level `exit 1` to `return 1 2>/dev/null || true` and `exit 0` to `return 0 2>/dev/null || true`. It also updates the autogenerated documentation pages. Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@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. |
![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 several new optional and bundled skills (such as hyperliquid, stocks, pr-review, and rest-graphql-debug) to the catalog, updates documentation across various skills to improve shell safety and execution paths, and refactors the automated PR reviewer script to wrap its logic in a main() function. Feedback on the changes focuses on the refactored bash script, pointing out that the expressions return 1 2>/dev/null || true and return 0 2>/dev/null || true contain redundant redirection and fallback operators since return exits the function immediately. Additionally, the reviewer noted that returning a non-zero status from main will still trigger an exit under set -e unless handled at the call site.
Auto-merge: review received — action requiredReview activity detected (4 inline comment(s) by non-author(s); 1 comment(s)/review(s) from review bot(s): gemini-code-assist[bot]). To confirm you have reviewed and accepted the feedback, please add the |
There was a problem hiding this comment.
Pull request overview
This PR updates the automated-pr-reviewer skill’s embedded bash workflow to avoid terminating the caller’s shell by wrapping logic in main() and replacing exit with return, and then regenerates/updates the website skill documentation and catalogs (including several additional skill doc pages).
Changes:
- Refactors the
skills/github/automated-pr-reviewer/SKILL.mdbash snippet to use amain()function andreturninstead ofexit. - Regenerates/updates multiple skill docs pages (bundled + optional), and adds new generated pages for several skills.
- Updates skills catalogs to include new/previously-missing entries and sections.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
skills/github/automated-pr-reviewer/SKILL.md |
Refactors the embedded bash workflow to use main() + return instead of exit. |
website/docs/user-guide/skills/bundled/github/github-automated-pr-reviewer.md |
Regenerated skill docs reflecting the updated workflow script. |
website/docs/user-guide/skills/optional/productivity/productivity-memento-flashcards.md |
Updates docs to prefer the JSON stdin interface and adds shell-safety guidance. |
website/docs/user-guide/skills/optional/mlops/mlops-hermes-atropos-environments.md |
Fixes related-skill links to point at optional-skill doc paths. |
website/docs/user-guide/skills/optional/finance/finance-dcf-model.md |
Tightens recalc instructions to resolve excel-author path via skill_view. |
website/docs/user-guide/skills/bundled/software-development/software-development-requesting-code-review.md |
Adjusts commit guidance to avoid git add -A post-verification. |
website/docs/user-guide/skills/bundled/research/research-research-paper-writing.md |
Clarifies safe usage constraints for execute_code. |
website/docs/user-guide/skills/bundled/red-teaming/red-teaming-godmode.md |
Updates guidance to “report-only” mode and disables persistence language. |
website/docs/user-guide/skills/bundled/devops/devops-kanban-worker.md |
Adds “review-required” handoff pattern using kanban_block + metadata comment. |
website/docs/user-guide/skills/bundled/pr-review/pr-review-pr-review.md |
Adds generated docs page for the pr-review bundled skill. |
website/docs/user-guide/skills/optional/software-development/software-development-rest-graphql-debug.md |
Adds generated docs page for the optional rest-graphql-debug skill. |
website/docs/user-guide/skills/optional/finance/finance-stocks.md |
Adds generated docs page for the optional stocks skill. |
website/docs/user-guide/skills/optional/blockchain/blockchain-hyperliquid.md |
Adds generated docs page for the optional hyperliquid skill. |
website/docs/reference/skills-catalog.md |
Updates bundled skills catalog (adds entries/sections). |
website/docs/reference/optional-skills-catalog.md |
Updates optional skills catalog (adds entries/sections). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| | Skill | Description | | ||
| |-------|-------------| | ||
| | [**base**](/docs/user-guide/skills/optional/blockchain/blockchain-base) | Query Base (Ethereum L2) blockchain data with USD pricing — wallet balances, token info, transaction details, gas analysis, contract inspection, whale detection, and live network stats. Uses Base RPC + CoinGecko. No API key required. | | ||
| | [**hyperliquid**](/docs/user-guide/skills/optional/blockchain/blockchain-hyperliquid) | Hyperliquid market data, account history, trade review. | | ||
| | [**solana**](/docs/user-guide/skills/optional/blockchain/blockchain-solana) | Query Solana blockchain data with USD pricing — wallet balances, token portfolios with values, transaction details, NFTs, whale detection, and live network stats. Uses Solana RPC + CoinGecko. No API key required. | | ||
|
|
There was a problem hiding this comment.
@gemini-code-assist, is this suggestion still valid? If so, what are the steps and line numbers to make this fix?
There was a problem hiding this comment.
The suggestion provided by Copilot is valid. The pull request includes a broad set of documentation updates across multiple skill pages, which makes the review process less focused than the PR title and description suggest. To resolve this, you should either update the PR description to explicitly list these additional changes (e.g., adding hyperliquid, stocks, rest-graphql-debug, and pr-review documentation) or move the documentation regeneration to a separate, dedicated pull request. If you choose to keep the current scope, simply update the PR description body to reflect the broader changes. No specific code changes are required to address this comment, as it is a process-related suggestion.
There was a problem hiding this comment.
@gemini-code-assist, what are the steps and line numbers to make this fix?
There was a problem hiding this comment.
To address the feedback regarding the PR scope, you should update the PR description to clearly outline all changes included in this pull request. Since this PR contains documentation updates for multiple skills (hyperliquid, stocks, rest-graphql-debug, pr-review) in addition to the automated-pr-reviewer script fix, listing these clearly will help reviewers understand the full scope. You can edit the PR description body directly in the GitHub UI to include a section like 'Additional Documentation Updates' that lists these files. No code changes are required to address this process-related feedback.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
…-pr-reviewer.md Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
…-pr-reviewer.md Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
|
@copilot resolve the merge conflicts in this pull request |
Auto-merge: checks failingThe following checks did not pass:
Please fix the failing checks before this PR can be merged. |
The `exit` statements in the `automated-pr-reviewer` bash script would prematurely terminate interactive bash sessions when the script was pasted into an agent's terminal or sourced. This commit wraps the bash logic in a `main()` function and changes the top-level `exit 1` to `return 1 2>/dev/null || true` and `exit 0` to `return 0 2>/dev/null || true`. It also updates the autogenerated documentation pages. Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
What
Wrapped the bash script logic in
skills/github/automated-pr-reviewer/SKILL.mdinside amain()function and replacedexit 1/exit 0withreturnstatements (return 1 2>/dev/null || true). It also regenerates corresponding documentation.Why
When the script is pasted into an agent's terminal or sourced within an interactive bash session, early exit conditions (like missing dependencies or no PRs to review) would trigger an
exitand kill the entire active shell session. Wrapping it in a function and returning allows it to exit gracefully without terminating the session.Verification
exitstatements are gone.generate-skill-docs.pyto regenerate the skill documentation and synced.tests/test_plugin_skills.pysuccessfully.Result
Agents can now safely run the script in an interactive session without crashing it when no PRs are found to review.
PR created automatically by Jules for task 10103983659694188912 started by @badMade