Repository navigation
fix(core): skip invalid TOML policy rules - #29431
andreivince wants to merge 3 commits into
Conversation
Summary of ChangesHello, 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 improves the reliability of the TOML policy engine by introducing a filtering mechanism for invalid rule configurations. Previously, certain malformed rules could cause startup failures or unexpected behavior; this change ensures that invalid rules are identified and ignored while allowing valid policies to remain active. The update includes comprehensive test suites to validate these scenarios and updates documentation to reflect the new behavior. Highlights
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 the 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 counterproductive. 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. Footnotes
|
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
|
📊 PR Size: size/L
|
![gemini-code-assist[bot]](/web-api/iframe-proxy?url=https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request modifies the TOML policy loader in packages/core to skip individual invalid rules and safety checkers (such as those with empty tool names or invalid shell-command syntax) rather than failing the entire loading process. Valid sibling rules within the same file are now preserved and successfully loaded. The PR also updates the policy engine documentation to explain this behavior and adds comprehensive unit and integration tests to verify the new logic. As there are no review comments, I have no additional feedback to provide.
|
Hi there! Thank you for your interest in contributing to Gemini CLI. To ensure we maintain high code quality and focus on our prioritized roadmap, we only guarantee review and consideration of pull requests for issues that are explicitly labeled as 'help wanted'. This PR will be closed in 7 days if it remains without that designation. We encourage you to find and contribute to existing 'help wanted' issues in our backlog! Thank you for your understanding. |
|
this one's for #29050 — invalid TOML policy rules were erroring out instead of being skipped. fix + tests are in. asked about the help wanted label on the issue a few days back, no word yet. |
|
This pull request is being closed as it has been open for 14 days without a 'help wanted' designation. We encourage you to find and contribute to existing 'help wanted' issues in our backlog! Thank you for your understanding. |
1 participant
Summary
Skip TOML policy rules that already produced validation errors. An empty tool name currently reaches
PolicyEngineand crashes startup, while conflicting shell-command fields are reported as invalid but still enforced.Details
Track invalid rule indices before transformation and array expansion, including empty names in safety checker rules. Valid siblings and rules in other files continue to load, with original diagnostic indices preserved. Warning-only rules retain their existing behavior.
Cover each existing shell-syntax error, empty scalar and array names, checker rules, file isolation, and actual policy decisions. The same matrix checks diagnostics and skipped-rule behavior, replacing four repeated diagnostic-only cases. Add two recorded-response CLI tests and document the behavior, including a valid schema example.
Related Issues
Fixes #29050
How to Validate
From the repository root with Node.js 20.19.6:
policy-headless.test.tsanduser-policy.test.tsCLI tests passed on both platforms with recorded model responses. The two new cases failed against the unchanged implementation and passed five consecutive runs with retries disabled after the fix.The consolidated revision was revalidated on macOS; Linux checks cover the earlier revision. Windows and the Docker, Podman, and Seatbelt CLI sandbox modes were not tested. Integration tests used recorded responses rather than live model credentials.
Pre-Merge Checklist