Skip to content

feat: implement issue #418 — Compliance: ruleset-bypass-orgadmin-code-quality - #426

Merged
don-petry merged 4 commits into
mainfrom
dev-lead/issue-418-20260818-2015
Aug 21, 2026
Merged

don-petry merged 4 commits into
mainfrom
dev-lead/issue-418-20260818-2015

Conversation

@don-petry

@don-petry don-petry commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

User description

Relates to #418 (test cleanup only; live ruleset remediation requires GH_TOKEN_ADMIN — tracked in #418)

Implemented by dev-lead agent. Please review.

Summary by CodeRabbit

  • Tests
    • Improved test isolation and reliability by using dedicated temporary directories for test data.
    • Simplified test setup and cleanup handling.

CodeAnt-AI Description

Update ruleset tests to use Bats-managed temporary files

What Changed

  • Ruleset payload tests now write captured request data to Bats’ temporary directory
  • Removed redundant test setup and cleanup for temporary files and tokens
  • Existing checks still verify OrganizationAdmin and integration bypass actors, including preservation of existing actors

Impact

✅ More reliable ruleset test isolation
✅ Fewer test-environment cleanup failures
✅ Bypass actor coverage remains intact

💡 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:

@codeant-ai ask: Your question here

This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.

Example

@codeant-ai ask: Can you suggest a safer alternative to storing this secret?

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:

@codeant-ai: Your feedback here

This helps CodeAnt AI learn and adapt to your team's coding style and standards.

Example

@codeant-ai: Do not flag unused imports.

Retrigger review

Ask CodeAnt AI to review the PR again, by typing:

@codeant-ai: review

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.

@don-petry
don-petry requested a review from a team as a code owner August 18, 2026 20:20
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@codeant-ai

codeant-ai Bot commented Aug 18, 2026

Copy link
Copy Markdown

🤖 CodeAnt AI — Review Status

Status Commit Started (UTC) Finished (UTC)
✅ Incremental review completed df74057 Aug 21, 2026 · 13:30 13:31
✅ Reviewed your PR 888246d Aug 18, 2026 · 20:20 20:22

@codeant-ai

codeant-ai Bot commented Aug 18, 2026

Copy link
Copy Markdown

Thanks for using CodeAnt! 🎉

We're free for open-source projects. if you're enjoying it, help us grow by sharing.

Share on X ·
Reddit ·
LinkedIn

@qodo-code-review

Copy link
Copy Markdown

ⓘ Qodo reviews are paused because your trial has ended. Ask your workspace admin to add credits to resume reviews. Manage billing

@codeant-ai codeant-ai Bot added the size:XS This PR changes 0-9 lines, ignoring generated files label Aug 18, 2026
@coderabbitai

coderabbitai Bot commented Aug 18, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 45bbd666-2434-4977-870a-613f5dde014e

📥 Commits

Reviewing files that changed from the base of the PR and between 888246d and 1b95590.

📒 Files selected for processing (1)
  • .github/scripts/tests/apply-code-quality-ruleset.bats

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The Bats ruleset tests remove shared temporary-directory setup and cleanup. Create and update request payloads now use BATS_TEST_TMPDIR for both writing and assertions.

Changes

Ruleset test temporary storage

Layer / File(s) Summary
Update payload storage and assertions
.github/scripts/tests/apply-code-quality-ruleset.bats
The tests remove shared temporary-directory hooks. Create and update request mocks and assertions use BATS_TEST_TMPDIR.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Merge Risk: ⚪ Minimal · up to 1b955

This localized change updates the OrganizationAdmin bypass configuration while preserving existing bypass actors and adds related test coverage; no actionable merge-blocking risk remains after normal checks and review.

Possibly related PRs

Suggested reviewers: donpetry-bot

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Linked Issues check ⚠️ Warning The PR does not demonstrate that the live code-quality ruleset has the required OrganizationAdmin bypass with bypass_mode: always. Apply the ruleset change successfully through apply-repo-settings and verify the live configuration before closing issue #418.
✅ Passed checks (4 passed)
Check name Status Explanation
Out of Scope Changes check ✅ Passed The Bats temporary-directory refactoring supports the affected tests and is related to validating the ruleset change.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies issue #418 and the intended OrganizationAdmin bypass compliance change for the code-quality ruleset.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch dev-lead/issue-418-20260818-2015

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request attempts to add an "actor_id" of 0 to the OrganizationAdmin bypass actor configuration in the code quality ruleset script and its tests. However, the feedback points out that the GitHub Rulesets API does not accept an "actor_id" for OrganizationAdmin, which would result in a validation error. The reviewer recommends reverting this change in the script, comments, and test assertions, while also suggesting the use of the built-in "$BATS_TEST_TMPDIR" variable in the BATS tests.

Comment thread .github/scripts/apply-code-quality-ruleset.sh Outdated
Comment thread .github/scripts/tests/apply-code-quality-ruleset.bats Outdated
Comment thread .github/scripts/tests/apply-code-quality-ruleset.bats Outdated
Comment thread .github/scripts/apply-code-quality-ruleset.sh Outdated
@don-petry

Copy link
Copy Markdown
Contributor Author

Dev-Lead — waiting on PR blockers (intent: review-changes)

PR: #426
No changes were committed, but the PR still has blocking checks or reviews (failing or cancelled checks, or changes-requested reviews). The retry cron will re-attempt automatically. Next attempt after: 2026-08-18T20:51:59Z

@don-petry

Copy link
Copy Markdown
Contributor Author

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.
Next attempt after: 2026-08-18T20:51:59Z

@don-petry
don-petry enabled auto-merge (squash) August 18, 2026 20:22
@don-petry

Copy link
Copy Markdown
Contributor Author

No description provided.

@don-petry
don-petry disabled auto-merge August 18, 2026 20:41
@don-petry

Copy link
Copy Markdown
Contributor Author

Dev-Lead — review-changes (applied)

Changes committed and pushed.

@don-petry
don-petry enabled auto-merge (squash) August 18, 2026 20:44
@don-petry
don-petry disabled auto-merge August 18, 2026 20:45
@don-petry
don-petry enabled auto-merge (squash) August 18, 2026 20:47
@don-petry
don-petry disabled auto-merge August 18, 2026 20:48
@don-petry

Copy link
Copy Markdown
Contributor Author

Dev-Lead — fix-bot-comment (no-changes)

Agent reasoning
1. ✅ Removed `actor_id: 0` from OrganizationAdmin bypass actor in script (line 46)
2. ✅ Updated comment to remove `actor_id: 0` reference (line 14)
3. ✅ Switched test file to use built-in `$BATS_TEST_TMPDIR` instead of custom `$TEST_TMPDIR`
4. ✅ Updated test assertions to reflect corrected bypass_actors structure without actor_id for OrganizationAdmin
**Current State (HEAD d6dd997):**
- Script correctly defines: `REQUIRED_BYPASS_ORG_ADMIN='{"actor_type":"OrganizationAdmin","bypass_mode":"always"}'` (no actor_id)
- Test file correctly uses `$BATS_TEST_TMPDIR` and asserts the correct payload structure
- All CI checks passing (Analyze, CodeQL, SonarCloud, dependency-audit, etc.)
- No Tier 1 blockers (no `CHANGES_REQUESTED` reviews, no failed checks)
**Conclusion:** The dev-lead commit (d6dd997) already applied all necessary fixes based on the review feedback. The PR is ready and requires no additional changes.

@don-petry
don-petry enabled auto-merge (squash) August 18, 2026 20:49
@donpetry-bot

donpetry-bot commented Aug 18, 2026

Copy link
Copy Markdown
Contributor
Superseded by automated re-review at 1b955908b30cc481403a0160821f712446a892f8 — click to expand prior review.

Review — fix requested (cycle 1/3)

The automated review identified the following issues. Please address each one:

Findings to fix

Automated review — NEEDS HUMAN REVIEW

Risk: LOW
Reviewed commit: d6dd9971eb75f6b039b4dd5c992246abcee361a3
Review mode: triage-approved (single reviewer)

Summary

The net diff is a sound, test-only refactor of apply-code-quality-ruleset.bats: the custom setup()/teardown() (mktemp + GH_TOKEN export) is replaced with the bats built-in BATS_TEST_TMPDIR (auto-created/cleaned), and GH_TOKEN is exported inside each test body, so the tests remain self-contained. The first commit's invalid actor_id: 0 for the OrganizationAdmin bypass actor was correctly reverted after review feedback, leaving apply-code-quality-ruleset.sh unchanged from main. The escalation is not about the code — it is about the PR closing compliance issue #418 without remediating the live ruleset drift it describes.

Linked issue analysis

Issue #418 is an error-severity compliance finding (ruleset-bypass-orgadmin-code-quality): the live code-quality ruleset is missing the required OrganizationAdmin bypass actor. This PR declares Closes #418, but its net change touches only the test file — the apply script on main already declares the correct bypass actor, so the finding reflects live-state drift, not a script defect. Remediation requires the apply-repo-settings workflow to run successfully, and its last 5 runs all failed (including the scheduled self-heal on 2026-08-17 — consistent with the missing GH_TOKEN_ADMIN secret its preflight was added to surface; see the #325#403#420 recurrence chain). Merging as-is closes #418 while the non-compliant live state persists and the self-heal is broken; the weekly audit will simply re-file the finding. Note the issue also carries the dev-lead:needs-human label. Recommended paths: (a) provision GH_TOKEN_ADMIN and run apply-repo-settings successfully, verify the ruleset, then merge; or (b) remove the Closes #418 linkage so the test cleanup can merge on its own merits while #418 stays open to track the operational fix.

Findings

  1. [blocking] Linked issue not substantively addressedCloses #418 on a net test-only diff; live ruleset drift remains and the self-healing workflow (apply-repo-settings.yml) is failing on every trigger. See analysis above.
  2. [resolved] Initial commit added actor_id: 0 to the OrganizationAdmin bypass actor (invalid — the Rulesets API rejects actor_id for this actor type). Flagged by gemini-code-assist; reverted in the second commit. All 4 review threads are resolved.
  3. [good] The BATS_TEST_TMPDIR refactor is correct: GH_TOKEN="mock-token" is exported inside each test that runs the script, and the built-in temp dir removes the manual mktemp/teardown lifecycle.
  4. Secret scan: the run_secret_scanning MCP tool is not available in this environment; the gitleaks CI check passed and the diff contains no credential-like content (only the literal mock-token test fixture, unchanged in nature).

CI status

All required checks green: CodeQL ✓, SonarCloud ✓, agent-shield / AgentShield ✓, dependency-audit / Detect ecosystems ✓, Secret scan (gitleaks) ✓, CodeRabbit ✓, Graphite ✓. The review / review check shows CANCELLED — that is a superseded prior run of this auto-review workflow itself, not a product CI failure. mergeStateStatus: BLOCKED reflects the required review, not failing checks.


Reviewed automatically by the PR-review agent (single-reviewer mode: fable 5). Reply if you need a human review.

Additional tasks

  1. Resolve all unresolved review thread comments from other reviewers
  2. Ensure all CI checks pass after your changes
  3. Rebase on the target branch if behind
  4. Do NOT modify files unrelated to the findings above

The review cascade will automatically re-review after new commits are pushed.

@don-petry
don-petry disabled auto-merge August 18, 2026 22:03
@don-petry

Copy link
Copy Markdown
Contributor Author

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.
Next attempt after: 2026-08-18T22:35:53Z

@don-petry
don-petry enabled auto-merge (squash) August 18, 2026 22:05
@don-petry

Copy link
Copy Markdown
Contributor Author

No description provided.

@don-petry
don-petry disabled auto-merge August 18, 2026 22:42
@donpetry-bot

donpetry-bot commented Aug 18, 2026

Copy link
Copy Markdown
Contributor
Superseded by automated re-review at df74057d5361a318e4ffa4c4bbeed53d11141bff — click to expand prior review.

Review — fix requested (cycle 2/3)

The automated review identified the following issues. Please address each one:

Findings to fix

Automated review — NEEDS HUMAN REVIEW

Risk: LOW
Reviewed commit: 1b955908b30cc481403a0160821f712446a892f8
Review mode: triage-approved (single reviewer)

Summary

Re-review after cycle-1 fix request. The only change since the previously reviewed commit (d6dd997) is a merge of main (bringing in PR #427's one-line pr-auto-review.yml change); this PR's own net diff is unchanged — a sound, test-only refactor of apply-code-quality-ruleset.bats to use the bats built-in BATS_TEST_TMPDIR. The cycle-1 blocking finding was not addressed: the PR still declares Closes #418 while remediating nothing about the live ruleset drift the issue describes.

Linked issue analysis

Issue #418 (error-severity compliance finding ruleset-bypass-orgadmin-code-quality) requires the live code-quality ruleset to carry the OrganizationAdmin bypass actor. Verified live state this cycle: GET repos/petry-projects/markets/rulesets/14805963 returns bypass_actors: null (last updated 2026-07-31 — before the finding was filed), so the drift persists. The apply script on main is already correct; remediation requires a successful apply-repo-settings run, and its last 5 runs all failed (most recent: scheduled self-heal on 2026-08-17), consistent with the missing GH_TOKEN_ADMIN secret. Merging as-is would close #418 with the non-compliant state intact, and the weekly audit would re-file it. The issue also carries the dev-lead:needs-human label. Paths forward remain: (a) provision GH_TOKEN_ADMIN, run apply-repo-settings successfully, verify the ruleset, then merge; or (b) remove the Closes #418 linkage so the test cleanup merges on its own merits while #418 stays open for the operational fix.

Findings

  1. [blocking — carried forward from cycle 1] Linked issue not substantively addressed: Closes #418 on a net test-only diff. Re-verified this cycle that the live ruleset still has no bypass actors and the self-heal workflow is still failing on every trigger.
  2. [no new issues] The delta since the prior review (d6dd997...1b95590) is a clean merge of main only; it introduces no changes of its own to this branch's content.
  3. [good — unchanged] The BATS_TEST_TMPDIR refactor remains correct and self-contained; all 4 review threads on the earlier actor_id: 0 mistake are resolved.
  4. Secret scan: the run_secret_scanning MCP tool is not available in this environment; gitleaks CI passed and the diff contains no credential-like content.

CI status

All required checks green at 1b95590: CodeQL ✓, SonarCloud ✓, agent-shield / AgentShield ✓, dependency-audit / Detect ecosystems ✓, Secret scan (gitleaks) ✓, CodeRabbit ✓. review / review shows CANCELLED — a superseded prior run of this auto-review workflow itself, not a product CI failure. mergeStateStatus: BLOCKED reflects the required review.


Reviewed automatically by the PR-review agent (single-reviewer mode: fable 5). Reply if you need a human review.

Additional tasks

  1. Resolve all unresolved review thread comments from other reviewers
  2. Ensure all CI checks pass after your changes
  3. Rebase on the target branch if behind
  4. Do NOT modify files unrelated to the findings above

The review cascade will automatically re-review after new commits are pushed.

@don-petry

Copy link
Copy Markdown
Contributor Author

Dev-Lead — waiting on PR blockers (intent: review-changes)

PR: #426
No changes were committed, but the PR still has blocking checks or reviews (failing or cancelled checks, or changes-requested reviews). The retry cron will re-attempt automatically. Next attempt after: 2026-08-18T23:21:35Z

@don-petry
don-petry enabled auto-merge (squash) August 18, 2026 22:51
@don-petry

Copy link
Copy Markdown
Contributor Author

No description provided.

@don-petry
don-petry disabled auto-merge August 19, 2026 01:54
@don-petry

Copy link
Copy Markdown
Contributor Author

Dev-Lead — review-changes (no-changes)

No changes were needed for this PR.

@don-petry
don-petry enabled auto-merge (squash) August 19, 2026 01:55
@codeant-ai

codeant-ai Bot commented Aug 21, 2026

Copy link
Copy Markdown

Thanks for using CodeAnt! 🎉

We're free for open-source projects. if you're enjoying it, help us grow by sharing.

Share on X ·
Reddit ·
LinkedIn

@codeant-ai codeant-ai Bot added size:XS This PR changes 0-9 lines, ignoring generated files and removed size:XS This PR changes 0-9 lines, ignoring generated files labels Aug 21, 2026
@sonarqubecloud

Copy link
Copy Markdown

@don-petry
don-petry disabled auto-merge August 21, 2026 13:31
@donpetry-bot

Copy link
Copy Markdown
Contributor

Advisory bots were rate-limited; auto-approval is withheld until they recover. pr-review-sweep will re-review this PR after 2026-08-21T14:32:12Z.

@don-petry

Copy link
Copy Markdown
Contributor Author

Dev-Lead — fix-bot-comment (no-changes)

Agent reasoning
Issues addressed: 0 (all previously resolved)
- Invalid actor_id in OrganizationAdmin config: already removed
- Test assertions updated: already using correct format  
- BATS temp directory: already using $BATS_TEST_TMPDIR
Files changed: None (issues pre-addressed in current HEAD)
Skipped (informational): CodeAnt promotional comment (no findings)
```

@don-petry
don-petry enabled auto-merge (squash) August 21, 2026 13:33

@donpetry-bot donpetry-bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review — APPROVED ✓

Risk: LOW
Reviewed commit: df74057d5361a318e4ffa4c4bbeed53d11141bff
Review mode: triage-approved (single reviewer)

Summary

Test-only refactor of .github/scripts/tests/apply-code-quality-ruleset.bats. Removes the shared setup()/teardown() hooks that used mktemp -d and manual rm -rf, switching all temp payload writes/assertions to Bats' managed BATS_TEST_TMPDIR. The GH_TOKEN="mock-token" export is preserved by moving it into each @test block. Net +4/-... one file, size:XS.

Linked issue analysis

Relates to #418. PR is explicitly scoped as test cleanup only; the live ruleset remediation (adding OrganizationAdmin bypass with bypass_mode: always) requires GH_TOKEN_ADMIN and remains tracked in #418. CodeRabbit's 'Linked Issues' warning that the live config isn't yet proven is expected and acknowledged by the author — it is not in scope for this test-cleanup PR. Existing assertions still verify OrganizationAdmin + Integration bypass actors and preservation of pre-existing actors.

Findings

No blocking findings.

  • No secrets, credentials, auth, or migration changes; the only token is a literal mock (GH_TOKEN="mock-token") used to satisfy the script under test.
  • Verified no stale $TEST_TMPDIR references remain in the head revision; all temp I/O uses $BATS_TEST_TMPDIR (Bats-managed, auto-cleaned per test), which is a strict reliability improvement over shared mktemp -d + manual teardown.
  • Secret-scanning MCP tool (run_secret_scanning) not available in this environment; relied on the gitleaks CI check, which passed.

CI status

All required checks green: bats (.github/scripts) SUCCESS, CodeQL SUCCESS, Secret scan (gitleaks) SUCCESS, SonarCloud SUCCESS, AgentShield SUCCESS. 'review / review' shows CANCELLED (a superseded/re-run review workflow, not a test failure). mergeStateStatus is BLOCKED only pending required human review approval.


Reviewed automatically by the PR-review agent (Claude single reviewer). Reply if you need a human review.

@don-petry
don-petry merged commit 0d32a6a into main Aug 21, 2026
25 of 26 checks passed
@don-petry
don-petry deleted the dev-lead/issue-418-20260818-2015 branch August 21, 2026 14:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:XS This PR changes 0-9 lines, ignoring generated files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants