Repository navigation
feat: implement issue #1025 — petry-projects/.github has no duplicate-decl-gate — the #1485 corruption class just recurred unguarded in PR #1024 and left apply-repo-settings.sh unparseable - #1033
Conversation
…-decl-gate — the #1485 corruption class just recurred unguarded in PR #1024 and left apply-repo-settings.sh unparseable
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
🤖 CodeAnt AI — Review Status
|
|
ⓘ Qodo reviews are paused because your trial has ended. Ask your workspace admin to add credits to resume reviews. Manage billing |
Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
|
Warning Review limit reachedNext included review available in 46 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Team Run ID: 📒 Files selected for processing (6)
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 |
codeant-ai
Bot
added
the
size:XL
![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 a new CI gate script check-duplicate-decls.sh to detect duplicate top-level declarations in shell, markdown, and JSON files, alongside its corresponding GitHub ruleset configuration and BATS tests. The feedback recommends declaring loop variables as local in Bash functions to prevent global scope leakage, stripping carriage returns (\r) before passing text to awk to avoid issues with CRLF line endings, and using mktemp under $BATS_TEST_TMPDIR in BATS tests to ensure proper test isolation and cleanup.
![codeant-ai[bot]](https://avatars.githubusercontent.com/in/646884?s=60&v=4){kind=small}
Dev-Lead — review-changes (applied)Changes committed and pushed. |
Dev-Lead — fix-reviews (applied)Changes committed and pushed. |
Maintainer verification at head
|
| # | claim | verified at 47778d92 |
|---|---|---|
| 1 | CRLF strip before awk |
✅ tr -d '\r' < at lines 66 and 115 — both extractors |
| 2 | $BATS_TEST_TMPDIR in test setup |
✅ WORKDIR="$(mktemp -d "$BATS_TEST_TMPDIR/scan.XXXXXX")" |
| 3 | heredoc tracking | ✅ inheredoc state present |
| 4 | fence length tracking | ✅ closing fence must match char and length |
| 5 | HTML-comment tracking | ✅ inhtml state present |
| 6 | find -print0 |
✅ -print0 + 3× read -r -d '' + 3× sort -z |
| 7 | NaN/Infinity rejection |
✅ _NAN_RE + explicit ValueError |
| 8 | loop vars localised | ✅ (resolved earlier, outdated) |
Also: bash -n clean, and the gate declares no function twice — it does not trip its own check.
Behavioural validation I ran directly:
- Against
.githubmain→no duplicate top-level declarations, exit 0. It does not false-positive on main's legitimate repeated headings (### Agentic Directives×3,#### Adopting in a new repo×2). Merging this will not turnmainred. - Against feat: implement issue #1023 — canary autocut: scope bump signals to watched-path commits, paginate the range, and persist failed promotions #1024's corrupted tree → correctly blocks, reporting
ensure_required_labels (declared 2 times).
This is good to land from my side.
Two gaps worth recording — not blockers, and I am not opening threads for them
The prior handoff recorded that this gate, run against #1024, blocked "both the shell duplicate and the AGENTS.md heading duplication." Re-running it against #1024's actual tree, only the shell duplicate is caught. Correcting that for the record so the gap is not assumed closed:
-
Duplicated
####headings are not detected. feat: implement issue #1023 — canary autocut: scope bump signals to watched-path commits, paginate the range, and persist failed promotions #1024'sAGENTS.mdcarries#### Pull Request Limits (automation open-PR cap)at both 599 and 1091, and the gate reports nothing. It appears to consider only top-level headings, so a duplicated deep section passes. -
Duplicate YAML keys inside fenced blocks in Markdown are not detected — and this is the one that matters most. feat: implement issue #1023 — canary autocut: scope bump signals to watched-path commits, paginate the range, and persist failed promotions #1024's
standards/ci-standards.mdemits theclaude-issue:job key twice (926 and 961), both inside the```yamlfence opened at line 865. Fenced blocks are deliberately skipped (correctly, for prose), so this can never be caught. Butci-standards.mdis the template adopting repos copy their workflows from, and a duplicate YAML mapping key makes the resulting workflow unparseable — so this is the highest-blast-radius variant of the #1485 class and it is currently unguarded.
Neither weakens the case for merging this — it catches the shell class cleanly and that is the recurring one. Filing them as follow-up rather than expanding scope here.
Related: #1025, #1040, #1039, petry-projects/.github-private#1609.
Dev-Lead — fix-bot-comment (no-changes)Agent reasoning |
donpetry-bot
left a comment
donpetry-bot
left a comment
There was a problem hiding this comment.
Automated review — APPROVED ✓
Risk: MEDIUM
Reviewed commit: 47778d920e690163741e08626e242df9139d5327
Review mode: triage-approved (single reviewer)
Summary
Adds the duplicate-decl-gate CI backstop (ported from .github-private #1520) to petry-projects/.github: a pure detection script (scripts/check-duplicate-decls.sh) covering duplicate top-level shell functions, duplicate H1/H2 markdown headings, and duplicate/invalid JSON keys; a thin required-check workflow; a bats test workflow; a targeted branch ruleset; and README docs. Triage assessment confirmed — the change is protective infrastructure with tight permissions and no security concerns.
Linked issue analysis
Closes #1025, which documented that the #1485 corruption class recurred unguarded in PR #1024 (duplicated functions, doubled AGENTS.md, unparseable apply-repo-settings.sh) because this repo lacked the duplicate-decl-gate that guards .github-private. The PR substantively addresses it: gate workflow runs on every PR (no path filter, deliberately, so the required check always reports), detection extends the private gate with markdown/JSON coverage matching the exact PR #1024 signature, and the required-check ruleset (standards/rulesets/duplicate-decl-gate.json) is targeted to the meta-repos only with the standard bypass actors (OrganizationAdmin + Integration 3167543, byte-identical to code-quality.json).
Findings
No blocking findings.
Verified during review:
- actions/checkout pin 3d3c42e5aac5ba805825da76410c181273ba90b1 confirmed via GitHub API as the v7.0.1 tag commit (not guessed).
- Both new workflows use top-level
permissions: {}with job-levelcontents: readonly; the gate workflow setspersist-credentials: false. No untrusted input reaches any run step. - Ruleset bypass actors match the org standard in code-quality.json exactly.
- All 9 review threads (gemini-code-assist, codeant-ai) are resolved; maintainer independently verified all eight fixes present at head 47778d9 via the contents API.
- Secret scanning MCP tool unavailable in this run; gitleaks CI check passed.
Minor (non-blocking):
- duplicate-decl-gate-tests.yml checkout omits
persist-credentials: false(the gate workflow sets it); blast radius is limited by contents:read. - Branch is BEHIND main; strict status checks will require an update before merge — merge-time mechanic, not a review issue.
CI status
All required checks green: SonarCloud, CodeQL, agent-shield, dependency-audit, plus the new duplicate-decl-gate and its "Lint and bats" test job all SUCCESS. Secret scan (gitleaks) SUCCESS. Two non-required Dev-Lead agent jobs (dispatch, ci-relay) show CANCELLED — superseded agent-automation runs, not failures.
Reviewed automatically by the PR-review agent (single-reviewer mode: fable 5). Reply if you need a human review.
|
Fixed in scripts/check-duplicate-decls.sh: declared IFS as local in the _record function to prevent global scope leakage when modifying IFS in the while loop (line 165). Verified the other recommendations are already implemented: carriage returns stripped before awk (lines 66, 115) and mktemp using $BATS_TEST_TMPDIR in tests (line 11). |
Dev-Lead — fix-bot-comment (applied)Changes committed and pushed. |
|
Dev-Lead — review-changes (no-changes)No changes were needed for this PR. |
donpetry-bot
left a comment
donpetry-bot
left a comment
There was a problem hiding this comment.
Automated review — APPROVED ✓
Risk: MEDIUM
Reviewed commit: ecdcd7e3e63d9b0c5d5d3f976581105ee3117a4d
Review mode: triage-approved (single reviewer)
Summary
Re-review at head ecdcd7e after the prior single-reviewer approval at 47778d9. The only delta since that approval is a merge of main into the branch, bringing in the unrelated automated docs commit #1034 (README.md, profile/README.md) — none of the PR's own six files changed. The PR adds the duplicate-decl-gate CI backstop (ported from .github-private #1520): a pure detection script for duplicate top-level shell functions, duplicate H1/H2 markdown headings, and duplicate/invalid JSON keys; a thin required-check workflow; a bats test workflow (19 tests); a targeted branch ruleset; and README docs. All prior findings remain resolved; the merge clears the previously noted BEHIND-main state (mergeStateStatus is now CLEAN).
Linked issue analysis
Closes #1025, which documented that the #1485 corruption class recurred unguarded in PR #1024 because this repo lacked the duplicate-decl-gate that guards .github-private. Substantively addressed: the gate runs on every PR with no path filter (so the required check always reports), detection covers the exact PR #1024 signature (duplicated shell functions, doubled markdown headings, unparseable JSON), and the required-check ruleset is targeted to the meta-repos only with the standard bypass actors (OrganizationAdmin + Integration 3167543).
Findings
No blocking findings.
Verified in this pass:
- Independently re-verified via the GitHub API that actions/checkout pin 3d3c42e5aac5ba805825da76410c181273ba90b1 is the v7.0.1 tag commit.
- Compare 47778d9...ecdcd7e shows only the main-merge (README.md +2, profile/README.md +5/-4 from merged #1034); the PR's own changeset is byte-identical to the previously approved state.
- All 9 review threads (gemini-code-assist, codeant-ai) are resolved, including the local-IFS fix in _record.
- Both workflows use top-level permissions: {} with job-level contents: read only; the gate checkout sets persist-credentials: false; no untrusted input reaches any run step.
- Secret scanning MCP tool unavailable in this run; gitleaks CI check passed.
Minor (non-blocking, carried from prior review):
- duplicate-decl-gate-tests.yml checkout omits persist-credentials: false; blast radius limited by contents: read.
CI status
All checks green at head ecdcd7e: SonarCloud, CodeQL, agent-shield, Agent Security Scan, ShellCheck, dependency-audit, Secret scan (gitleaks), plus the new duplicate-decl-gate and both Lint-and-bats jobs — all SUCCESS. mergeStateStatus CLEAN, reviewDecision APPROVED.
Reviewed automatically by the PR-review agent (single-reviewer mode: fable 5). Reply if you need a human review.
Resolves the conflict in tests/canary_rollout.bats by keeping BOTH test blocks: #1046's `_is_release_tag_suffix` suite (landed on main via #1051) and this PR's `_is_evicted_run` suite. Both branches appended to the same region of the file; neither change touches the other's code. Hand-resolved because dev-lead's rebase dispatch was itself cancelled by a concurrency eviction (run 33462968616) — the exact defect this PR fixes. Verified on the merged tree: - _is_evicted_run declared 1x, _is_release_tag_suffix declared 1x - 9 + 3 tests present, no duplicate @test names, no conflict markers - duplicate-decl-gate (live on main since #1033) passes - bash -n clean; no repeated statement blocks
2 participants
User description
Closes #1025
Implemented by dev-lead agent. Please review.
CodeAnt-AI Description
Block merges that contain duplicate declarations or invalid configuration files
What Changed
mainnow reject duplicate top-level shell functions, repeated H1/H2 Markdown headings, duplicate JSON keys, and invalid or non-standard JSON.Impact
✅ Fewer corrupted merges✅ Duplicate configuration caught before deployment✅ Clearer validation failures💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.