Repository navigation
feat: implement issue #351 — SonarCloud: shell script hygiene cleanup (×7) - #352
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Warning Review limit reached
Next review available in: 56 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![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 refactors several CI and validation shell scripts by introducing a central DONE_MARK constant, localizing variables in the error function, and redirecting warning outputs to standard error. The review feedback correctly identifies a large block of duplicate checks in tools/test-repo-settings.sh that should be removed to clean up the script.
There was a problem hiding this comment.
Pull request overview
This PR addresses issue #351 by applying mechanical shell-script hygiene cleanups (per SonarCloud findings) in the repo’s validation/verification scripts, without changing their intended behavior.
Changes:
- Replaced repeated
echo " done."literals with a per-script constant (readonly DONE_MARK=" done."). - Updated
error()helpers to assign positional parameters to local variables before use. - Redirected a “python3/PyYAML unavailable — skipping …” message to
stderrintools/test-repo-settings.sh.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| tools/validate-skills.sh | Introduces DONE_MARK constant and refactors error() to use a local variable before emitting to stderr. |
| tools/test-repo-settings.sh | Introduces DONE_MARK, refactors error() to use a local variable, and redirects the “PyYAML unavailable” diagnostic to stderr. |
| tools/test-ci-secret-scan.sh | Introduces DONE_MARK and refactors error() to use a local variable before emitting to stderr. |
Superseded by automated re-review at
|
Dev-Lead — review-changes (applied)Changes committed and pushed. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
|
|
Note @don-petry I reviewed this PR and no code changes were needed, but it still has blocking checks or reviews (failing or cancelled checks, or changes-requested reviews), so I cannot mark it done yet. I'll re-check automatically. |
Dev-Lead — waiting on PR blockers (intent: review-changes)PR: #352 |
donpetry-bot
left a comment
donpetry-bot
left a comment
There was a problem hiding this comment.
Automated review — APPROVED ✓
Risk: LOW
Reviewed commit: 7b9da4b7128652c906c66d2710ffbc797fddec30
Review mode: triage-approved (single reviewer)
Summary
Triage-approved confirmation review (cycle 1/3). This PR implements issue #351 (SonarCloud shell hygiene ×7) across three test/validation scripts: readonly DONE_MARK constant replacing repeated ' done.' literals (S1192), error() assigning its positional parameter to a local variable (S7679), and the PyYAML-unavailable diagnostic redirected to stderr (S7677). All changes are mechanical and behavior-preserving. Since the prior cascade review (ca005bf), the head now also removes the stale duplicate check block in tools/test-repo-settings.sh (former lines 181-253 duplicating Checks 5/7/8) — the prior review's only MAJOR finding — and the associated Gemini review thread is resolved/outdated. CodeRabbit approved the current head; Copilot reviewed with no comments. All substantive CI checks are green (CI Validate, gitleaks, CodeQL, SonarCloud quality gate: 0 new issues, AgentShield, dependency-audit). The run_secret_scanning MCP tool is not exposed in this environment; gitleaks CI is green and the diff adds no secret-like content.
Linked issue analysis
Closes #351 (SonarCloud: shell script hygiene cleanup ×7). All three rule groups are substantively addressed in the affected files: S7679 local-variable assignment in error() in all three scripts; S1192 DONE_MARK constant in all three scripts; S7677 stderr redirect for the PyYAML-unavailable message in tools/test-repo-settings.sh. SonarCloud quality gate passed with 0 new issues and 0 duplication on new code, consistent with the issue's acceptance criteria (real fixes, no NOSONAR suppressions, behavior unchanged, CI green).
Findings
- RESOLVED: Prior review's MAJOR finding (duplicate check block at tools/test-repo-settings.sh lines 181-253 re-running Checks 5/7/8 with scrambled numbering) is fixed — the block is removed in the current head, and the Gemini review thread on it is resolved and outdated.
- INFO: Two CANCELLED entries in the status rollup (dev-lead / dispatch and dev-lead / ci-relay on run 29378624508) are Dev-Lead Agent orchestration jobs superseded by concurrency — a later dev-lead / dispatch run succeeded, and ci-relay was intentionally skipped via [skip ci-relay] in the head commit. These are not code-validation checks and do not gate approval.
- INFO: run_secret_scanning MCP tool unavailable in this environment; gitleaks CI check is green, so no secret-scanning gap.
CI status
All code-validation checks green at 7b9da4b: CI / Validate SUCCESS, CI / Secret scan (gitleaks) SUCCESS, CodeQL (actions) SUCCESS, SonarCloud quality gate PASSED (0 new issues, 0 security hotspots, 0 duplication on new code), AgentShield SUCCESS, dependency-audit SUCCESS (ecosystem jobs skipped where not applicable), pr-auto-review SUCCESS, CodeRabbit status SUCCESS. Two cancelled Dev-Lead Agent orchestration jobs are concurrency-superseded duplicates, not test failures. mergeable=MERGEABLE; mergeStateStatus=BLOCKED pending this review approval.
Reviewed automatically by the PR-review agent (single-reviewer mode: fable 5). Reply if you need a human review.
Closes #351
Implemented by dev-lead agent. Please review.