Route configure.sh's ruleset_id() Through jqr (#1253) - #1883
Conversation
`ruleset_id()`'s id extraction used a plain `jq -r`, so a native Windows jq's trailing CR reached the `rulesets/<id>` API URL and broke the live-state fetch `check_ruleset()` and `apply_ruleset()` depend on. Route it through the existing `jqr()` helper, matching every other CRLF-exposed site in this file. An adversarial local-review pass over this one-line diff found no introduced defects, and three pre-existing plain-`jq` sites of the same class elsewhere in the file, filed separately as #1880, #1881, #1882 rather than expanding this fix's scope. Closes on promotion: #1253 Co-Authored-By: Claude Sonnet 5 <[email protected]>
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Repository: ptr727/ProjectTemplate/.coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 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 |
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The change is a minimal, consistent use of the existing jqr() normalization helper and does not alter behavior on CR-free hosts.
Review effort: Lite
Findings: None
What changed in this PR
This pull request updates repo-config/configure.sh to normalize the ruleset ID extraction in ruleset_id() by routing it through the existing jqr() helper, preventing a trailing carriage return from contaminating the ID on native Windows jq output and breaking subsequent gh api calls.
Changes:
- Switched
ruleset_id()'s ruleset ID lookup fromjq -r ...tojqr ...so any trailing\ris stripped before the ID is used in API paths.
| File | Description |
|---|---|
| repo-config/configure.sh | Uses jqr() for ruleset ID extraction to avoid CR-suffixed IDs breaking gh api requests on Windows. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…and Overnight Fixes to Main (#1850) Promotes `develop` to `main`. ## Carried - #1846: exits `repo-config/configure.sh` early, writing nothing, for a repository whose registry `status` is `archived`. - #1848: fails the validator's C# unit test step when the run wrote no non-empty Cobertura report, clearing `./coverage` first. It declines #1134's `ref` input with evidence, since a bare checkout already validates `github.sha`, and D1.2 now says so. #1134's third gap moved to #1800. - #1851: asserts that the two planted registration defects are themselves reported. - #1854, #1858: correct #1846's test docstrings, which claimed the script makes no `gh` call before the archived check in cases where it does. Raised by Copilot on this pull request. - #1856: passes `--repo` on documented handoff commands and guards handoff reads against a full page, per #1847. - #1859, #1863: fall back to the default search path when `PATH` is unset in #1848's and #1846's test harnesses. Raised by Copilot on this pull request. - #1861: scopes `VerifyReferenceAotCompatibility` to an AOT publish in `dotnet-codestyle`, per #1857. - #1867: establishes the `PATH` order `tool_shadow_path` names, per #1644. - #1870: makes the installer's dirty-checkout tests independent of the real checkout's state, per #1641. - #1873: pins and decodes git's quoting in `repo_gate.py`'s `ls-files` read, per #1580 and #1872. - #1878: folds typographic punctuation in `pr_review.py reply --match`, per #1299. - #1883: routes `configure.sh`'s `ruleset_id()` through `jqr`, per #1253. - #1885: states the pin comment as the release tag and defines `$/`, per #1805. - #1888: distinguishes `./` from `$/` resolution in the pin rule's prose, per #1886. - #1893: drops the issue reference from `repo-config/README.md`'s archived-exemption note, which Copilot flagged on six rounds of this pull request. - #1895: describes IL3058 in `dotnet-codestyle` as a referenced assembly lacking `IsAotCompatible` metadata set to `true`, and drops the unversioned package examples. Raised by CodeRabbit on this pull request. Callers that pin a hub release get the new C# check on their next pin bump. A test project that runs `dotnet test --coverage` without writing a report now fails its step rather than passing silently. Closes #1134 Closes #1847 Closes #1857 Closes #1644 Closes #1641 Closes #1580 Closes #1872 Closes #1299 Closes #1253 Closes #1805 Closes #1886 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Backlog counts and rankings now consistently exclude handoff issues, including those also marked blocked. * Repository configuration commands now exit without writing when a repository is archived. * Review-thread matching handles typographic punctuation, and no-match responses report the unresolved-thread count. * Tool setup handles PATH entries more precisely, and repository checks report unusual file paths without crashing. * **Reliability** * Validation now fails when C# or Python tests produce no coverage report. * Agent setup can use an explicit dirty-checkout override. * Workflow and repository guidance clarifies reference resolution, release-tag pinning, and AOT configuration. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Summary
Next in the #1123/#1234/#1246/#1247/#1253/#1254 chain.
repo-config/configure.sh'sruleset_id()still read a ruleset id with a plainjq -rrather than thejqr()helper #1229 introduced, and it sits directly upstream ofcheck_ruleset(), the function #1246/#1254 already fixed.The defect
ids="$(jq -r --arg n "$1" '.[] | select(.name==$n) | .id' <<<"$out")"(
repo-config/configure.sh,ruleset_id(), around line 150.) On a native Windows jq, the returned id carries a trailing\r. The[ -z "$id" ]guard passes (the string is non-empty), and the CR-suffixed id is then interpolated straight into the API URL:gh api "repos/$repo/rulesets/$id". That request targetsrulesets/<id>\r, which fails outright, socheck_ruleset()reports "could not read live state" (orapply_ruleset()aborts) before the comparisons #1246/#1254 fixed are ever reached.Fix
Route the extraction through the existing
jqr()helper (jqr() { jq -r "$@" | sed $'s/\r$//'; }), matching every other CRLF-exposed site in this file. No behavior change on a CR-freejq(Linux/macOS).The issue's secondary site, the bypass-list extraction in
check_ruleset(), already reads throughjqr()ondevelop(from #1254), confirmed unchanged rather than edited blind.Verification
shellcheckandshfmt -dboth clean onrepo-config/configure.sh.python3 scripts/prose_lint.py .,scripts/repo_gate.py,spec/validate.py, andpython3 -m unittest discover -s scripts/tests(1696 tests) all pass.jq -rto append a trailing CR (the documented native-Windows behavior), then shows the unfixed extraction yields123\r(mismatches the expected id) while thejqr()-routed extraction yields123(matches), and that a real (CR-free)jqis unaffected by the fix.jqCRLF-exposure sites elsewhere in the same file, filed separately as configure.sh's ruleset_id() per_page=100 Guard Reads a Plain jq, Breaking the Integer Test on Windows #1880, configure.sh's workflowModel Registry Read Is a Plain jq -r, Aborting on Windows With No Model Passed #1881, configure.sh's check_environments() Percent-Encoded Name Is a Plain jq -rn, Breaking the Branch-Policies URL on Windows #1882 rather than expanding this fix's scope.This defect and its fix are Windows-only, and I cannot reproduce or verify the actual Windows-jq CRLF behavior on this (Linux) host. The verification above proves the CR-stripping mechanism is correct and unchanged on Linux/macOS; it does not and cannot prove the fix resolves the issue on a real Windows host. That remains unverified here, same as #1123/#1229/#1234/#1246/#1247/#1254 before it.
Closes on promotion: #1253
🤖 Generated with Claude Code