feat: implement issue #492 — [Fleet Monitor] petry-projects/.github-private — issue-triage-runner.yml - #493
Conversation
…rivate — issue-triage-runner.yml
|
Warning Review limit reached
More reviews will be available in 53 minutes and 39 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds path filters to .github/workflows/test-aw.yml and a new test-issue-triage job, and introduces tests/aw/issue-triage/test_runner_yml.sh, a bash structural-regression script that validates .github/workflows/issue-triage-runner.yml across seven YAML/assertion checks. ChangesIssue-Triage Workflow Testing Infrastructure
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Dev-Lead — waiting on PR blockers (intent: review-changes)PR: #493 |
|
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. |
![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 bash script, test_runner_yml.sh, which performs structural regression tests on the GitHub Actions workflow .github/workflows/issue-triage-runner.yml using Python's yaml module. The review feedback focuses on improving the robustness and debuggability of the test script. Key recommendations include adding pre-flight checks for python3 and PyYAML dependencies, removing stderr redirections (2>/dev/null) to prevent silencing Python or YAML parsing errors, and refining newline handling in error reporting by using $'\n' and printf '%s'.
Dev-Lead — fix-bot-comment (no-changes)Agent reasoning |
There was a problem hiding this comment.
Pull request overview
Adds automated regression coverage around the issue-triage-runner.yml workflow to prevent a repeat of issue #492 (workflow failing during “Set up job” due to an unreachable external action), and wires those checks into the existing AW test workflow.
Changes:
- Added a new structural test script that validates key invariants of
.github/workflows/issue-triage-runner.yml(permissions model, two-job pattern, and avoiding external setup actions). - Updated
.github/workflows/test-aw.ymlto run the new issue-triage unit + structural tests and to trigger when the issue-triage workflow files change.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
tests/aw/issue-triage/test_runner_yml.sh |
New bash-based structural regression tests for issue-triage-runner.yml. |
.github/workflows/test-aw.yml |
Runs the new issue-triage test suite in CI and expands path triggers to include issue-triage files. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/aw/issue-triage/test_runner_yml.sh`:
- Line 25: Replace the POSIX test bracket in the conditional that checks the
RUNNER file existence by using Bash's safer double-bracket form: change the line
using if [ -f "$RUNNER" ]; then to use [[ -f "$RUNNER" ]] (retain the RUNNER
variable and quoting) so the script uses the bash builtin conditional instead of
[ ].
- Around line 19-20: REPO_ROOT and RUNNER are defined as constants but not
marked readonly; update the assignments for REPO_ROOT and RUNNER (the variables
named REPO_ROOT and RUNNER in this script) to be readonly so they cannot be
modified at runtime—use either the readonly keyword at definition or assign
first and then run readonly VAR (e.g., set them as readonly immediately after
assignment) to follow the coding guideline.
- Around line 37-50: The Python check in the heredoc is too permissive: replace
the current conditional that allows None (the line checking wf['permissions']
not in ({}, None)) with a strict equality check that requires wf['permissions']
== {} so the test only passes when top-level permissions are exactly an empty
dict; update the inline script in the runner check (the heredoc block executed
by python3) to exit nonzero unless 'permissions' exists and equals {}.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 635bc5a1-592b-4dd8-bf96-83e529c3ea0a
📒 Files selected for processing (2)
.github/workflows/test-aw.ymltests/aw/issue-triage/test_runner_yml.sh
Dev-Lead — fix-reviews (applied)Changes committed and pushed. |
Dev-Lead — waiting on PR blockers (intent: fix-reviews)PR: #493 |
Dev-Lead — fix-reviews (no-changes)Agent reasoning |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/aw/issue-triage/test_runner_yml.sh (1)
138-147:⚠️ Potential issue | 🟠 Major | ⚡ Quick winScope Test 6 to the actual triage invocation step, not any step env.
This assertion currently succeeds if
CLAUDE_CODE_OAUTH_TOKENappears on anytriagestep, so it can miss a brokenRun issue-triage (staged)step and report a false pass.Suggested patch
triage_steps = wf.get('jobs', {}).get('triage', {}).get('steps', []) -for step in triage_steps: - env = step.get('env') or {} - if 'CLAUDE_CODE_OAUTH_TOKEN' in env: - sys.exit(0) -sys.exit(1) +target = None +for step in triage_steps: + if (step.get('id') == 'run' + or (step.get('name') or '').strip().lower() == 'run issue-triage (staged)'): + target = step + break + +if not target: + print("triage invocation step not found", file=sys.stderr) + sys.exit(1) + +env = target.get('env') or {} +if 'CLAUDE_CODE_OAUTH_TOKEN' not in env: + print("CLAUDE_CODE_OAUTH_TOKEN missing on triage invocation step", file=sys.stderr) + sys.exit(1)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/aw/issue-triage/test_runner_yml.sh` around lines 138 - 147, The current check scans every triage step's env and passes if any step contains CLAUDE_CODE_OAUTH_TOKEN; change it to locate the specific triage invocation step (e.g., inspect triage_steps and find the step where step.get('name') == "Run issue-triage (staged)" or where the run command matches the runner invocation), then only check that step's env for 'CLAUDE_CODE_OAUTH_TOKEN' and exit 0 if present (otherwise exit 1); update the loop that builds triage_steps and the conditional that inspects step.get('env') so it targets that named invocation instead of any triage step.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@tests/aw/issue-triage/test_runner_yml.sh`:
- Around line 138-147: The current check scans every triage step's env and
passes if any step contains CLAUDE_CODE_OAUTH_TOKEN; change it to locate the
specific triage invocation step (e.g., inspect triage_steps and find the step
where step.get('name') == "Run issue-triage (staged)" or where the run command
matches the runner invocation), then only check that step's env for
'CLAUDE_CODE_OAUTH_TOKEN' and exit 0 if present (otherwise exit 1); update the
loop that builds triage_steps and the conditional that inspects step.get('env')
so it targets that named invocation instead of any triage step.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 5d173a7a-a147-4f33-af82-9ef263673ad6
📒 Files selected for processing (1)
tests/aw/issue-triage/test_runner_yml.sh
Dev-Lead — waiting on PR blockers (intent: review-changes)PR: #493 |
|
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. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3698dcc1f1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Dev-Lead — fix-reviews (applied)Changes committed and pushed. |
Dev-Lead — waiting on PR blockers (intent: review-changes)PR: #493 |
|
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 — 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: LOW
Reviewed commit: f29bbd0ae5f863e3d2afffb37932e4f7725058b6
Review mode: triage-approved (single reviewer)
Summary
Tests-only PR adding structural regression guards for .github/workflows/issue-triage-runner.yml plus CI wiring in test-aw.yml. Directly addresses the failure mode from #492 (workflow failing at "Set up job" because the anthropics/claude-code-action/setup@<SHA> action became unreachable) by asserting the install step uses run: instead of an external uses:, alongside permissions, two-job pattern, and token-forwarding invariants. No production code touched.
Linked issue analysis
Closes #492 (Fleet Monitor: issue-triage-runner.yml 100% failure rate). The PR substantively addresses it via Test 5, which explicitly fails if the Install claude CLI step is changed back to an external uses: step — the exact regression that caused 6/6 runs to fail at Set-up-job in ~5s. The other six tests lock in the surrounding invariants (top-level permissions: {}, triage issues:read / apply issues:write, token forwarded to the id: run step, two-job pattern with skip gate) so adjacent regressions are also caught. Verified the current issue-triage-runner.yml satisfies all seven assertions, and the "Test issue-triage" CI job is green.
Findings
No issues found.
- Test script uses
set -euo pipefail,[[ ]]brackets,readonlyconstants — earlier CodeRabbit nits resolved in current HEAD. - Test 6 now correctly targets the step with
id: runrather than scanning all triage steps (resolves the 17:35 CodeRabbit comment on commit 3698dcc). - Test 2 enforces strict
permissions == {}equality (no None fallback). actions/checkoutis pinned by commit SHA with a version comment.- ShellCheck, bats, gh-aw-compile, CodeQL, SonarCloud, AgentShield, gitleaks all SUCCESS. The one CANCELLED check is an earlier review run superseded by a later SUCCESS — not a real failure.
CI status
All checks passing. The single CANCELLED entry is a superseded "PR Review Agent / review" run; a later run of the same workflow completed SUCCESS. SonarCloud Quality Gate Passed, CodeQL clean, no dependency advisories.
Reviewed automatically by the PR-review agent (single-reviewer mode: opus 4.7). Reply if you need a human review.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f29bbd0ae5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| install_steps = [ | ||
| s for s in triage_steps | ||
| if 'claude' in (s.get('name') or '').lower() | ||
| and 'install' in (s.get('name') or '').lower() | ||
| ] |
There was a problem hiding this comment.
When a future edit leaves this Install claude CLI run step in place but reintroduces uses: anthropics/claude-code-action/setup@... under another step name, GitHub still resolves that uses action during job setup and the issue-triage run can fail before any shell step runs, while this regression test stays green because it only inspects steps whose name contains both claude and install. Since the test is meant to guard issue #492, scan all triage steps for the forbidden external setup action rather than only the install-named step.
Useful? React with 👍 / 👎.
| env = invocation.get('env') or {} | ||
| if 'CLAUDE_CODE_OAUTH_TOKEN' not in env: | ||
| print("CLAUDE_CODE_OAUTH_TOKEN not found in env of step id: run", file=sys.stderr) | ||
| sys.exit(1) |
There was a problem hiding this comment.
If the invocation step keeps an env key named CLAUDE_CODE_OAUTH_TOKEN but points it at the wrong expression or an empty value, this guard still passes even though scripts/aw.sh run issue-triage will fail when Claude is invoked without the OAuth secret. Since this test is specifically protecting credential forwarding to the real invocation step, also assert that the value is ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }}.
Useful? React with 👍 / 👎.
| triage = wf.get('jobs', {}).get('triage', {}) | ||
| perms = triage.get('permissions', {}) or {} | ||
| if perms.get('issues') != 'read': | ||
| sys.exit(1) |
There was a problem hiding this comment.
With the runner's top-level permissions: {}, the triage job also needs contents: read for its checkout step before it can run scripts/aw.sh; if a future edit drops that permission while leaving issues: read, this test still passes but the private-repo workflow can fail before the triage command runs. Extend the permission assertion here to cover the required checkout permission as well as issue access.
Useful? React with 👍 / 👎.
| apply = wf.get('jobs', {}).get('apply', {}) | ||
| needs = apply.get('needs', []) | ||
| if isinstance(needs, str): | ||
| needs = [needs] | ||
| if 'triage' not in needs: | ||
| print("apply job does not declare 'needs: triage'", file=sys.stderr) | ||
| sys.exit(1) | ||
| if_cond = str(apply.get('if', '')) | ||
| if 'needs.triage.outputs.skip' not in if_cond: |
There was a problem hiding this comment.
If a future edit removes or miswires jobs.triage.outputs.skip/result, this check still passes because it only inspects the apply job's needs and if strings; at runtime needs.triage.outputs.skip/result then resolve empty, so the apply job can run with an empty TRIAGE_RESULT and fail while the structural guard stays green. Add assertions that the triage outputs are mapped to steps.run.outputs.skip and steps.run.outputs.result.
Useful? React with 👍 / 👎.
…rivate — issue-triage-runner.yml (#493) * feat: implement issue #492 — [Fleet Monitor] petry-projects/.github-private — issue-triage-runner.yml * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] --------- Co-authored-by: donpetry-bot <281750570+donpetry-bot@users.noreply.github.com>
…rivate — issue-triage-runner.yml (#493) * feat: implement issue #492 — [Fleet Monitor] petry-projects/.github-private — issue-triage-runner.yml * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] --------- Co-authored-by: donpetry-bot <281750570+donpetry-bot@users.noreply.github.com>
Closes #492
Implemented by dev-lead agent. Please review.
Summary by CodeRabbit