feat: implement issue #316 — [Fleet Monitor] petry-projects/.github-private — issue-triage-runner.yml - #516
Conversation
…rivate — issue-triage-runner.yml
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
📝 WalkthroughWalkthroughThe issue-triage workflow renames two allowed labels ( ChangesIssue Triage Label Rename
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 updates the issue triage scenarios and test suite to replace the non-existent label needs-triage with needs-human-review and correct good-first-issue to good first issue. It also adds several regression tests (Tests 7-11) to verify these label changes, ensure the model is not instructed to return skip flags, and validate the safe-output handling. The review feedback suggests improving shell script robustness by using printf instead of echo for JSON variables, replacing Python assert statements with sys.exit() to avoid silencing interpreter errors, and safely handling cases where yaml.safe_load returns None.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Dev-Lead — fix-reviews (applied)Changes committed and pushed. |
There was a problem hiding this comment.
Pull request overview
Updates the Fleet Monitor “issue triage” agent workflow spec and its test suite to prevent runtime failures caused by using non-existent repo labels and by instructing the model to emit a skip response that aw.sh treats as prompt injection (issue #316).
Changes:
- Replaced invalid labels (
needs-triage,good-first-issue) with repo-valid labels (needs-human-review,good first issue) in the issue triage workflow spec and scenario documentation. - Removed the prompt instruction to return
{"skip": true}(skip is handled byaw.sh’s pre-Claude guard). - Added regression tests to lock the allowed-label list and ensure the prompt body doesn’t include skip instructions.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
tests/aw/issue-triage/test_aw_run.sh |
Updates safe-output label test and adds regression tests validating allowed-labels and the absence of skip instructions in the prompt. |
tests/aw/issue-triage/scenarios.md |
Updates scenario expectations and the documented allowed-label set to match actual repo labels. |
.github/workflows/issue-triage.md |
Fixes safe-output allowed labels and updates the prompt label taxonomy to repo-valid labels; removes skip-output instruction. |
Dev-Lead — fix-bot-comment (no-changes)Agent reasoning |
Dev-Lead — review-changes (applied)Changes committed and pushed. |
|
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: 5e9cf6d74baf289c13f1805b5d34715bd95744c1
Review mode: triage-approved (single reviewer)
Summary
Fixes issue #316 (DEGRADED issue-triage-runner.yml, 25% failure rate). The triage prompt referenced labels that do not exist in the repo (needs-triage, good-first-issue) and instructed the model to emit {"skip":true}, both of which cause runtime failures. The PR corrects the allowed-label set to the real repo labels (needs-human-review, good first issue), removes the skip instruction (aw.sh's pre-Claude guard already handles label-count skips and rejects model-emitted skip flags as prompt injection), and adds regression tests.
Linked issue analysis
Issue #316 reports issue-triage-runner.yml degraded at 25% failure. Verified against the live repo label set: 'good first issue' and 'needs-human-review' exist; 'needs-triage' and 'good-first-issue' do NOT. The PR swaps the workflow + scenarios to the correct labels and drops the conflicting skip instruction — directly addressing the root cause of the runtime failures.
Findings
No issues found. Changes are limited to the issue-triage AW spec (.github/workflows/issue-triage.md) and its tests. New tests (7-11) lock in the fix: assert the allowed list excludes needs-triage/good-first-issue, includes needs-human-review/good first issue, the prompt body contains no {"skip":true}, and needs-human-review passes safe-output validation end-to-end. Test 9b correctly pairs with Test 9 to prevent a label-drop false pass. No auth/secret/migration/permission changes; no security smells.
CI status
All required checks green (CodeQL, SonarCloud, gitleaks, shellcheck, bats, unit-tests, AW validation, issue-triage runner test, AgentShield, Agent Security Scan). Dependency-audit ecosystem jobs SKIPPED as expected. coderabbitai and donpetry-bot both APPROVED.
Reviewed automatically by the PR-review agent (single-reviewer mode: fable 5). Reply if you need a human review.
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: 3b02b9d1d70c7f5dc03876b6b3ee0ab5502d52a1
Review mode: triage-approved (single reviewer)
Summary
Root-cause fix for the issue-triage-runner.yml degradation (#316). The triage prompt referenced labels that do not exist in this repo — 'needs-triage' and 'good-first-issue' — which made 'gh issue edit --add-label' fail at runtime, and it instructed the model to emit {"skip":true}, which aw.sh rejects as prompt injection. The PR switches to the real repo labels ('needs-human-review', 'good first issue'), removes the skip instruction (aw.sh's pre-Claude guard already handles label-count skipping), and adds regression tests. Verified against the live label set: 'needs-human-review' and 'good first issue' exist; 'needs-triage' and 'good-first-issue' do not.
Linked issue analysis
Closes #316 (Fleet Monitor DEGRADED, 25% failure rate on issue-triage-runner.yml). The failures are consistent with the two defects fixed here (nonexistent labels → add-label failure; model skip flag → injection rejection). The fix is correct and directly targets those causes.
Findings
No blocking findings.
- Label corrections verified against
gh label list: spaced/hyphen forms handled correctly. - New regression tests (7–11, incl. 9b) are well-paired: each negative assertion (label absent) is matched by a positive assertion (correct label present), so dropping a label entirely cannot silently pass.
- One unresolved review thread from copilot-pull-request-reviewer asked Test 9 to also assert the spaced form is present — already addressed by the added Test 9b. Non-blocking (bot comment, concern resolved in code).
CI status
All required checks green (CodeQL, SonarCloud, CodeRabbit, shellcheck, bats, unit-tests, issue-triage runner tests, secret scan, AgentShield, AW validation, etc.). Dependency-audit and dependabot jobs SKIPPED as expected. mergeStateStatus is BLOCKED pending an approving review.
Reviewed automatically by the PR-review agent (single-reviewer mode: fable 5). Reply if you need a human review.
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: bfc8474d48103db4cfaca164f057c8dfba2a9d88
Review mode: triage-approved (single reviewer)
Summary
Corrects the issue-triage workflow to use labels that actually exist in the repo (needs-triage→needs-human-review, good-first-issue→good first issue) and removes the {"skip":true} prompt instruction that aw.sh rejects as prompt injection. Adds regression tests (7-11) guarding all three failure modes. Confirms the triage assessment: LOW-risk, all CI green.
Linked issue analysis
Closes #316, a Fleet Monitor DEGRADED report (25% failure rate on issue-triage-runner.yml). Root cause: the triage prompt allowed labels that don't exist in the repo, so 'gh issue edit --add-label' failed at runtime, plus a {"skip":true} instruction that aw.sh treats as injection. This PR fixes the label names and removes the skip instruction, directly addressing the runtime failures driving the degradation. Substantively addressed.
Findings
No blocking findings.
- Workflow prompt and test-only changes (3 files, +109/-18); no auth, secrets, migrations, or Actions permission/trigger changes.
- New regression tests assert: 'needs-triage' absent + 'needs-human-review' present; 'good-first-issue' absent + 'good first issue' present (Test 9 + 9b pairing prevents the drop-entirely gap); no skip instruction in prompt; and end-to-end safe-output acceptance of 'needs-human-review'.
- One unresolved Copilot thread (Test 9 doesn't assert the correct label is present) is already addressed by Test 9b in the diff; author confirmed and the test is present. Effectively resolved.
CI status
All 28 active checks green (ShellCheck, CodeQL, SonarCloud, Secret scan, Agent Security Scan, issue-triage unit tests, AW validation, etc.); remaining checks SKIPPED by design. mergeStateStatus BLOCKED pending review approval only.
Reviewed automatically by the PR-review agent (single-reviewer mode: fable 5). Reply if you need a human review.
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: d476d16f4e088c6b2e9354f9b2364c5c0514ac4b
Review mode: triage-approved (single reviewer)
Summary
Corrects the issue-triage workflow's allowed-label set to match labels that actually exist in the repo (needs-human-review, good first issue) and removes labels that do not (needs-triage, good-first-issue), plus removes the prompt's {"skip":true} instruction that aw.sh rejected as injection. Adds regression tests (7-11, 9b). Verified against live repo labels and CI.
Linked issue analysis
Closes #316 (Fleet Monitor: issue-triage-runner.yml DEGRADED, 25% failure rate). Root cause was the triage prompt instructing the model to apply labels 'needs-triage' and 'good-first-issue', which do not exist in the repo — 'gh issue edit --add-label' fails at runtime — plus a {"skip":true} instruction that aw.sh's guard rejects as prompt injection. I confirmed via 'gh label list' that the repo has 'needs-human-review' and 'good first issue' but NOT 'needs-triage' or 'good-first-issue', so the relabeling is correct and addresses the failure mode.
Findings
No blocking findings. Changes are confined to a workflow prompt (.github/workflows/issue-triage.md), a scenarios doc, and a test script — no auth/secrets/permissions/CI-security changes. The single unresolved review thread is a copilot bot concern (Test 9 didn't assert presence of the spaced label); it was addressed by Test 9b, which is present in the diff and affirmatively asserts 'good first issue' is in the allowed list. No unanswered human-reviewer questions.
CI status
All required checks green (Lint, shellcheck, ShellCheck, bats, unit-tests, Test issue-triage, CodeQL, SonarCloud/SonarCloud Code Analysis, Secret scan/gitleaks, AgentShield, Agent Security Scan, validate-agent-profiles, gh-aw-compile, etc.). Dependency-audit ecosystem jobs SKIPPED (n/a). SonarCloud Quality Gate passed with 0 new issues.
Reviewed automatically by the PR-review agent (single-reviewer mode: fable 5). Reply if you need a human review.
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: 481aaeb004b3568ffc79aed55c975e7b94d1b7ce
Review mode: triage-approved (single reviewer)
Summary
Fixes the DEGRADED issue-triage-runner.yml (issue #316). Replaces two labels that do not exist in the repo (needs-triage, good-first-issue) with the real labels (needs-human-review, good first issue), and removes the prompt instruction telling the model to emit {"skip": true} (which aw.sh rejects as prompt injection). Adds regression tests 7–11 covering all three root causes. Verified against the live repo label set.
Linked issue analysis
Closes #316. The issue reported a 25% failure rate on the triage runner. The PR addresses the three runtime failure modes directly: (1) gh issue edit --add-label failed on the non-existent needs-triage label; (2) same failure for hyphenated good-first-issue (repo label is good first issue with spaces); (3) the prompt's {"skip": true} branch produced output aw.sh treats as prompt injection. I confirmed against the live label set that needs-human-review and good first issue exist while needs-triage and good-first-issue do not — so the substitution is correct.
Findings
No blocking findings. Changes are confined to a workflow prompt spec, its scenario doc, and its test script — no permissions, triggers, secrets, or scripts-with-side-effects touched. New tests are well-paired (e.g. 9/9b assert both the absence of the bad form and the presence of the good form, preventing a 'drop the label entirely' false pass). Earlier automated-reviewer suggestions (printf vs echo, sys.exit over assert) were addressed in commit 497513a.
CI status
All required checks green. SUCCESS across Lint, shellcheck, bats, unit-tests, the dedicated 'Test issue-triage' job, CodeQL, SonarCloud, gitleaks, AgentShield, and Agent Security Scan; remaining checks SKIPPED (dependency-audit ecosystems, dependabot). coderabbitai and donpetry-bot both APPROVED. mergeStateStatus is BLOCKED pending approval, not a failing check.
Reviewed automatically by the PR-review agent (single-reviewer mode: fable 5). Reply if you need a human review.
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: 2cfa4984ab15a010dd9e2c0bea1942f35cb91b3b
Review mode: triage-approved (single reviewer)
Summary
Fixes the root cause of issue-triage-runner.yml's degraded failure rate (#316): the triage workflow referenced labels that do not exist in this repo (needs-triage, hyphenated good-first-issue) and instructed the model to emit {"skip":true}, which aw.sh rejects as prompt injection. The PR switches to the real labels (needs-human-review, good first issue), removes the skip instruction (the pre-Claude label-count guard in aw.sh already handles skipping), and adds regression tests. Verified against the live repo label set: good first issue and needs-human-review exist; needs-triage and good-first-issue do not.
Linked issue analysis
Closes #316 — a Fleet Monitor report flagging issue-triage-runner.yml as DEGRADED (25% failure rate, 2/8 runs). The two failure modes (invalid label causing gh issue edit --add-label to fail, and the skip-flag being rejected as injection) are both addressed at the source. Substantively resolved.
Findings
.github/workflows/issue-triage.md: allowed-label set and prompt body corrected to use repo-real labels and drop the skip instruction. Correct — confirmed againstgh label list.tests/aw/issue-triage/test_aw_run.sh: adds Tests 7–11 as regression guards (noneeds-triage, hasneeds-human-review, no hyphenatedgood-first-issue, hasgood first issue, no skip instruction, end-to-end safe-output acceptance). Well-paired (presence + absence) so dropping a label can't silently pass.tests/aw/issue-triage/scenarios.md: validation docs updated to match. Consistent.
No security-sensitive surface (no auth/secrets/migrations/Actions privilege changes). No unresolved review threads; CodeRabbit and the review bot approved.
CI status
All required checks green (CodeQL, ShellCheck, gitleaks, Agent Security Scan, SonarCloud, bats, unit-tests, issue-triage runner tests, AW validation, agent-shield). Conditional jobs (dependabot, language-specific audits) correctly SKIPPED. mergeStateStatus BLOCKED pending this review gate.
Reviewed automatically by the PR-review agent (single-reviewer mode: fable 5). Reply if you need a human review.
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: b714bff4ac80ae9d60022dfc7edc5f964f6525fc
Review mode: triage-approved (single reviewer)
Summary
Corrects the issue-triage agentic workflow to use labels that actually exist in the repo (needs-human-review, 'good first issue') instead of the non-existent needs-triage / good-first-issue, and removes the prompt instruction telling Claude to emit {"skip":true}. Both were root causes of the 25% failure rate flagged in #316. Adds six regression tests guarding the label names and the skip instruction. Verified against repo state.
Linked issue analysis
Closes #316 (Fleet Monitor DEGRADED alert: issue-triage-runner.yml at 25% failure). Two confirmed root causes, both fixed:
- Label mismatch — prompt/allowed-set used 'needs-triage' and 'good-first-issue', which do not exist; the repo has 'needs-human-review' and 'good first issue' (verified via gh label list). The wrong names make 'gh issue edit --add-label' fail at runtime.
- Skip instruction — prompt told the model to return {"skip":true} for issues with 2+ labels, but scripts/aw.sh rejects any model-emitted skip flag as prompt injection (aw.sh:312-320), while its own pre-Claude label-count guard (aw.sh:234) already handles skipping. Removing the instruction resolves the false rejection.
Substantively addressed.
Findings
No blocking issues.
- Label corrections verified against live repo labels: 'needs-human-review' and 'good first issue' exist; hyphenated/needs-triage forms do not.
- Skip-instruction removal matches aw.sh behavior (model skip flag rejected as injection; pre-Claude guard owns skip).
- New tests 7/8/9/9b/10/11 are well-scoped regression guards; 9b correctly pairs with 9 so dropping the label entirely cannot silently pass, and 11 exercises the fix end-to-end through safe-output apply.
- Scope is limited to one workflow prompt spec plus its tests; no auth, secrets, migrations, permissions, or trigger changes.
CI status
All required checks green (CodeQL, ShellCheck, gitleaks, Agent Security Scan, AgentShield, SonarCloud quality gate passed with 0 new issues, issue-triage unit tests, AW validation/compile). Routine skips only (dependency-audit ecosystems, dependabot-automerge, ci-relay). Advisory bots: coderabbitai APPROVED, donpetry-bot APPROVED, SonarCloud passed. No pending review requests or unresolved threads. mergeStateStatus BLOCKED pending this approval.
Reviewed automatically by the PR-review agent (single-reviewer mode: fable 5). Reply if you need a human review.
|
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_aw_run.sh (1)
36-48:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUpdate test data to use valid labels from the allowed set.
Test 2 intends to validate the max-labels count constraint (4 labels exceeds max of 3), but line 40 uses
needs-triage, which is no longer in the workflow's allowed set after the rename toneeds-human-review. The safe-output validator checks the allowed-set membership before the count (per the enforcement order in scripts/aw.sh lines 427-434), so this test now fails with "labels not in allowed set" rather than the intended "more than 3 labels" rejection. The test still passes because it only checks for the substring "rejected", but it no longer validates the invariant it claims to test.🔧 Proposed fix
Replace line 40 with four valid labels from the current allowed set:
-printf '{"labels":["bug","needs-triage","question","enhancement"],"comment":"Hello!"}' > "$tmp" +printf '{"labels":["bug","enhancement","question","documentation"],"comment":"Hello!"}' > "$tmp"🤖 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_aw_run.sh` around lines 36 - 48, The test data in Test 2 uses the label "needs-triage" on line 40 which is no longer in the workflow's allowed set after being renamed to "needs-human-review". This causes the validator to reject the input for invalid labels before checking the max-labels count, so the test validates the wrong constraint. Replace the four labels in the printf statement on line 40 with four valid labels from the current allowed set (such as "bug", "enhancement", "documentation", and "needs-human-review") to ensure the test properly validates that 4 labels exceeds the max of 3 labels as intended.
🤖 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_aw_run.sh`:
- Around line 36-48: The test data in Test 2 uses the label "needs-triage" on
line 40 which is no longer in the workflow's allowed set after being renamed to
"needs-human-review". This causes the validator to reject the input for invalid
labels before checking the max-labels count, so the test validates the wrong
constraint. Replace the four labels in the printf statement on line 40 with four
valid labels from the current allowed set (such as "bug", "enhancement",
"documentation", and "needs-human-review") to ensure the test properly validates
that 4 labels exceeds the max of 3 labels as intended.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: b5e47b4c-f8eb-419b-9b9d-adc73b4b36cc
📒 Files selected for processing (3)
.github/workflows/issue-triage.mdtests/aw/issue-triage/scenarios.mdtests/aw/issue-triage/test_aw_run.sh
Dev-Lead — fix-reviews (no-changes)Agent reasoning |
Closes #316
Implemented by dev-lead agent. Please review.
Summary by CodeRabbit
needs-human-reviewreplacesneeds-triage, andgood first issuereplacesgood-first-issue. Enhanced validation tests and added regression test coverage to ensure consistent application of the new label conventions in issue categorization automation.