Repository navigation
Match Tool Names and the all Keyword by Case in the Windows Agents Loaders - #3130
Merged
Merged
Conversation
…aders bootstrap.ps1 -Agents ALL took the all keyword because -contains ignores case, while bootstrap.sh --agents ALL passed ALL through and install-agents.sh refused it as an unknown tool. The PowerShell tool-name comparisons are now case-sensitive, so both loaders treat only lowercase all as the keyword and refuse any other case as an unknown tool. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Owner
Author
|
Local strict review (frontier tier) recorded against develop at content digest b4dd060f1cab. It raised 3 findings: 1 pre-existing, filed as #3129; 2 test-strength points, fixed in the second commit. |
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configuration
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
A Windows-native check remains owed, so final human review is recommended.
0 open findings
What changed in this PR
Aligns Windows agent selection with Linux by making all and tool-name matching case-sensitive.
Changes:
- Uses case-sensitive PowerShell comparisons.
- Adds regression tests for mixed-case keywords and tool names.
| File | Summary |
|---|---|
tests/test_install_agents.py |
Tests rejection of incorrectly cased tool names. |
tests/test_bootstrap.py |
Tests mixed-case keyword handoff behavior. |
host-setup/windows/install-agents.ps1 |
Enforces exact tool-name matching. |
host-setup/bootstrap.ps1 |
Matches only lowercase all. |
🧠 Review effort: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Owner
Author
|
Triage of the reviewer's closer-look headline: it names a caution rather than a defect, the Windows-native verification of the PowerShell comparisons, which the PR body already lists as a paste-ready block owed by the maintainer. The PowerShell tests ran under Linux pwsh, including the handoff test, which fails with the change reverted. No code change is owed. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #3130 +/- ##
==========================================
Coverage ? 63.94%
==========================================
Files ? 17
Lines ? 9151
Branches ? 0
==========================================
Hits ? 5852
Misses ? 3299
Partials ? 0
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This was referenced Oct 16, 2026
ptr727
added a commit
that referenced
this pull request
Oct 16, 2026
… Fixes to Main (#3146) - #3135 makes `gh-write-guard` read a gh write's subcommand and target the way gh parses them: the subcommand is found past flags before the command and before the subcommand, an `R` inside a short-option cluster is read as a target, `GH_REPO` is read as a target, a run-time repository owner on a write fails closed, and a run-time word where `pr` or `merge` goes fails closed beside `--admin` or where word splitting may split it - #3139 releases the bootstrap lock when a `bootstrap.ps1` run ends rather than when its process does, so a second run typed at the same pwsh prompt is no longer refused the lock - #3130 matches tool names and the `all` keyword by case in the Windows agents loaders, as the Linux loaders already do - #3128 reads CodeRabbit's rate limit on a reply to an explicit review command in `pr_review.py` - #3126 reconciles VSCode-Server-DotNetCore in the hub after its resync - #3124 logs the Docker task's build-base leg in to Docker Hub - #3122 names the carried skill and the skipped review correctly - #3120 states the subprocess encoding rule in `CODESTYLE.md`, where carriers read Python style - #3118 and #3111 move the release-guard tests' workflow-command checks onto `runner_commands`, and test the alternatives wiring at the `audit_repo` and `fidelity_pass` call sites - #3116 stops the doc gates' printed names forging workflow commands - #3113 adds a catalog snippet for the release-with-smoke caller - #3109 moves the `workflow_reuse` fetch onto `audit.contents_text` - #3105 accepts `.codecov.yml` as an alternative to `codecov.yml` - #3103 adds a pip ecosystem to the carried Dependabot snippet - #3100 prefers the venv's ruff and pyright for a declared pip directory - #3098 tracks composite action pins in Dependabot - #3096 reads an uppercase Markdown suffix as Markdown at every prose gate branch After this merges, the host-safety installer payload changes, so the maintainer runs the installer after promotion. Closes #1416 Closes #1425 Closes #1578 Closes #1874 Closes #1892 Closes #1899 Closes #2804 Closes #2850 Closes #2852 Closes #2854 Closes #2996 Closes #3065 Closes #3067 Closes #3069 Closes #3079 Closes #3080 Closes #3089 Closes #3093 Closes #3101 Closes #3106 Closes #3107 Closes #3137 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added a pull-request smoke build that runs when relevant release files change and reports the combined validation and build status. * Repository audits now recognize `.codecov.yml` as an alternative to `codecov.yml`. * **Improvements** * Python linting and type checks use tools from declared project environments when available, with fallbacks for missing tools. * Workflow and lint reports safely display unusual filenames and command-like text. * GitHub write-target checks now account for additional command forms and fail closed when targets cannot be verified. * Rate-limit detection now recognizes another CodeRabbit response format. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
bootstrap.ps1 -Agents ALLtook theallkeyword because PowerShell's-containsignores case, whilebootstrap.sh --agents ALLpassedALLthrough andinstall-agents.shrefused it as an unknown tool. The Windows side now matches Linux: the keyword check inbootstrap.ps1uses-ccontains, and the tool-name comparisons ininstall-agents.ps1(Get-Tool,Resolve-Selection) use-ceq,-cnotcontainsand-ccontains, so only lowercaseallis the keyword and a tool name in another case is refused.Behavior of
ALLon Windows after the changebootstrap.ps1 -Agents ALLhands-Install ALLtoinstall-agents.ps1, which refuses it withUnknown tool "ALL", the same refusal Linux gives.-Agents Herdris refused the same way instead of being accepted.Tests
tests/test_bootstrap.py:ALLandAllhand off as tool names on both the bash and the PowerShell loaders.tests/test_install_agents.py:Herdris refused as unknown on both sides.-Report Herdrrefusal fail, and the change restores them.tests/test_bootstrap.pyandtests/test_install_agents.pypass (171 tests, 5 skipped Windows-only), with PowerShell tests run under Linux pwsh, not Windows. The local review pass ran on the first commit, and the second commit only tightened the new tests to compare the whole step list, per its findings.Filed from the review: #3129 (a dash-prefixed
-Agentsname binds-Allon both platforms, pre-existing).Windows-native check still owed
Not run on Windows. For the maintainer, from a hub checkout in pwsh:
Closes #3065
🤖 Generated with Claude Code