Repository navigation
ci: fail AI triage loudly when the session bails before posting (#338) - #340
Conversation
A Claude Code CLI update made Task sub-agents background-by-default, so the triage session fanned out its search agents, said it would wait for them, and ended its turn — which terminates the headless session with no comment posted. The result record is a clean success, so the is_error watchdog stayed green (a new variant of the #317 silent-no-op shape). - Command files + workflow prompt: Task calls must pass run_in_background: false, and the session must never end its turn while a sub-agent is pending. - Workflow prompt now requires a machine-checkable final message (`TRIAGE-RESULT: <command> commented|refreshed|skipped (<reason>)`); the watchdog jq additionally requires the final result text to be nothing but such sentinel lines, so any future mid-task bailout fails the job instead of showing green. - Search agent rules: one prefix-allowlisted gh command per Bash call (loops/echo prefixes were being permission-denied) and a 6-search cap per agent (bursts were 403ing on the shared API rate limit). Fixes #338 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01K1LrTrVRbtdh8paN7f67CB
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
WalkthroughThe triage command specifications now require synchronous agents and constrained GitHub CLI searches. The AI triage workflow requires exact ChangesTriage reliability controls
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Claude
participant ExecutionFile
participant Watchdog
Claude->>ExecutionFile: Write final triage result
ExecutionFile->>Watchdog: Provide result record
Watchdog->>Watchdog: Validate error status and sentinel count
Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
Preview DeploymentPreview URL: https://ca5fa99a.bestax.pages.dev |
|
@claude please do a deep adversarial review of this PR's diff. Focus on:
Please report findings as review comments or a summary comment — do not push fixes. Generated by Claude Code |
|
Claude finished @allxsmith's task in 7m 10s —— View job Deep adversarial review complete
I pulled the actual execution logs for the two cited failures (runs Confirmed: the core mechanism does close the exact #338 shape. Both real bailouts have
Neither starts with That said, I found gaps that mean a session can still bail (semi-)silently: 1. The watchdog can't detect a dropped second command on PR runs ( 2. The sentinel is self-attested, not cross-checked ( 3. "No shell loops, echo prefixes, or 4. "Never end your turn while pending" has no actionable mechanism ( 5. Minor: zero tolerance for chatter risks false failures No contradictions found between the new synchronous-agent/rate-limit rules and the existing pre-check/idempotency sections of the three command files — the "silent exit still gets a sentinel" clarification ( |
There was a problem hiding this comment.
Deep review — 0 finding(s)
| # | Severity | Area | Finding | Location |
|---|---|---|---|---|
| — | — | — | No blocking defects found. | — |
Overall: This is a focused CI/prompt hardening change (three .claude/commands/*.md triage prompts + the ai-triage.yml watchdog) with no component or public-API surface, so no story/docs/skills updates are owed. I exercised the new jq gate against clean-sentinel, trailing-chatter, is_error:true, null/empty result, missing-result-record, and array-vs-NDJSON inputs — it is correctly fail-closed (only a last result record with is_error==false whose text is nothing but TRIAGE-RESULT: lines passes; everything else, including the #338 orphaned-agent bailout message, fails the job). The one intentional strictness is that a leading-whitespace-indented sentinel line would be rejected, which matches the "and nothing else" contract in the prompt. The human's main focus should just be confirming the sentinel/idempotency contract is one the Sonnet triage session can reliably satisfy on the silent-skip and multi-command PR paths, since a compliant-but-empty final message is the only thing standing between green and a false CI failure.
🏄 Chill little config swell, dude — no gnarly bugs lurking under the surface, the watchdog holds its line and the sentinel logic paddles out clean. Good to send it.
…agent rules Deep-review follow-up on #340: the watchdog accepted any nonzero number of TRIAGE-RESULT lines, so a PR run that completed triage-find-issues but bailed during triage-find-duplicate-prs passed on one sentinel. The gate step's is_pr output now feeds the check, which requires exactly one sentinel for issues and two for PRs. The search-agent command rules also now name pipes and command substitution alongside loops/chains — the same prefix allowlist denies those too. Refs #338 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01K1LrTrVRbtdh8paN7f67CB
|
Response to the deep adversarial review's five findings (addressed in d2ffb51): Finding 1 (dropped second command on PR runs) — fixed. The gate step's Finding 3 (pipes/substitution omitted from the Bash rule) — fixed. All three command files now name pipes and command substitution alongside loops, Finding 2 (sentinel is self-attested) — accepted as a limitation, no change. Cross-checking the claimed outcome would mean probing the item for marker comments with carve-outs for every legitimate silent exit (closed item, too-vague, no credible match) — that probing design was considered and rejected for exactly that complexity. Format+count validation catches the observed failure class (mid-task bailout); a session that fabricates a plausible sentinel without doing the work is a lying-agent problem no jq can solve. Finding 4 ("never end your turn" lacks an enforcement mechanism) — accepted as belt-and-braces, no change. Correct that the instruction is aspirational if the runtime ignores Finding 5 (zero-chatter rule risks false positives) — accepted deliberately, no change. A noisy false failure is strictly better than the silent false success it replaces: it's visible, re-runnable via the label, and diagnosable from the full session output ( Generated by Claude Code |
Preview DeploymentPreview URL: https://79c62ccb.bestax.pages.dev |
…ot alone (#342) The first live run after #340 (29794090279) false-failed: the session correctly skipped closed issue #330 and emitted its sentinel, but prefixed one explanatory line, which the nothing-but-sentinels check rejected. The watchdog now requires the TRIAGE-RESULT lines to be the LAST non-empty lines of the final message, exactly one per expected command and none elsewhere. Bailouts never emit sentinels and still fail; a bailout after a sentinel fails; over- and under-counts fail. Refs #338 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01K1LrTrVRbtdh8paN7f67CB
|
🎉 This PR is included in version 3.5.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
The deep-review label prompt validated diffs in isolation (0 findings on PR #340) while a prompted @claude session on the same diff found 5, including a real residual bug. Close the gap in what the reviewer is told to do, not the model: - Evidence phase: chase PR claims into linked issues and run/job logs, then verify the fix empirically — reading the diff is not verification. - Residual-risk hunt: enumerate how the addressed failure class could still occur; refute with evidence or post as a finding. A Residual risk section is now required in the summary review. - Advisory tier: 🔵 Advisory for real limitations/trade-offs worth putting on the record. Summary-table only — never inline, so advisory notes cannot become fixer work items and spin the AI loop. - PR-type-aware lens: keep the component checklist for bulma-ui/docs diffs; add an infra lens (guard bypasses, shell+jq edge cases, untrusted input, silent failures, idempotency) for .github/.claude/ scripts diffs. - Optional focus steer: a triage+ user may pre-post a PR comment starting with deep-review: — a new gate step verifies each candidate author's live role (newest steer per distinct author, newest-first, max 5 role checks, so an outsider can neither inject text nor displace a maintainer's steer), and injects it as a FOCUS block via a random-delimiter heredoc output. Fail-soft by design: API failures degrade to an unfocused review, never a red run. Unchanged: opus model, 120 turns, dedupe marker, one-review invariant, allowed_bots, show_full_output debug-only, the is_error gate. Docs: mention the steer in the AI development guide's label table and CLAUDE.md's deep-review sentence. Fixes #341
The deep-review label prompt validated diffs in isolation (0 findings on PR #340) while a prompted @claude session on the same diff found 5, including a real residual bug. Close the gap in what the reviewer is told to do, not the model: - Evidence phase: chase PR claims into linked issues and run/job logs, then verify the fix empirically — reading the diff is not verification. - Residual-risk hunt: enumerate how the addressed failure class could still occur; refute with evidence or post as a finding. A Residual risk section is now required in the summary review. - Advisory tier: 🔵 Advisory for real limitations/trade-offs worth putting on the record. Summary-table only — never inline, so advisory notes cannot become fixer work items and spin the AI loop. - PR-type-aware lens: keep the component checklist for bulma-ui/docs diffs; add an infra lens (guard bypasses, shell+jq edge cases, untrusted input, silent failures, idempotency) for .github/.claude/ scripts diffs. - Optional focus steer: a triage+ user may pre-post a PR comment starting with deep-review: — a new gate step verifies each candidate author's live role (newest steer per distinct author, newest-first, max 5 role checks, so an outsider can neither inject text nor displace a maintainer's steer), and injects it as a FOCUS block via a random-delimiter heredoc output. Fail-soft by design: API failures degrade to an unfocused review, never a red run. Unchanged: opus model, 120 turns, dedupe marker, one-review invariant, allowed_bots, show_full_output debug-only, the is_error gate. Docs: mention the steer in the AI development guide's label table and CLAUDE.md's deep-review sentence. Fixes #341
|
🎉 This PR is included in version 5.8.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
The deep-review label prompt validated diffs in isolation (0 findings on PR #340) while a prompted @claude session on the same diff found 5, including a real residual bug. Close the gap in what the reviewer is told to do, not the model: - Evidence phase: chase PR claims into linked issues and run/job logs, then verify the fix empirically — reading the diff is not verification. - Residual-risk hunt: enumerate how the addressed failure class could still occur; refute with evidence or post as a finding. A Residual risk section is now required in the summary review. - Advisory tier: 🔵 Advisory for real limitations/trade-offs worth putting on the record. Summary-table only — never inline, so advisory notes cannot become fixer work items and spin the AI loop. - PR-type-aware lens: keep the component checklist for bulma-ui/docs diffs; add an infra lens (guard bypasses, shell+jq edge cases, untrusted input, silent failures, idempotency) for .github/.claude/ scripts diffs. - Optional focus steer: a triage+ user may pre-post a PR comment starting with deep-review: — a new gate step verifies each candidate author's live role (newest steer per distinct author, newest-first, max 5 role checks, so an outsider can neither inject text nor displace a maintainer's steer), and injects it as a FOCUS block via a random-delimiter heredoc output. Fail-soft by design: API failures degrade to an unfocused review, never a red run. Unchanged: opus model, 120 turns, dedupe marker, one-review invariant, allowed_bots, show_full_output debug-only, the is_error gate. Docs: mention the steer in the AI development guide's label table and CLAUDE.md's deep-review sentence. Fixes #341
|
🎉 This PR is included in version 2.0.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 1.0.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Pull Request
Description
Fixes the silent no-op in AI auto-triage: a Claude Code CLI update made Task sub-agents background-by-default, so the triage session fanned out its search agents, said it would "wait for them to finish", and ended its turn — which terminates the headless session with no comment posted. The result record is a clean success (
is_error: false, stop_reason: end_turn), so the existing watchdog stayed green (a new variant of the #317 silent-no-op shape; evidence in #338 from runs 29686174473 / issue #330 and 29625394616 / issue #327).Changes:
.github/workflows/ai-triage.yml— the session prompt now requires every Task call to passrun_in_background: falseand forbids ending the turn while any sub-agent is pending; it also requires a machine-checkable final message of onlyTRIAGE-RESULT: <command> commented|refreshed|skipped (<reason>)lines. The "Fail on silent session error" watchdog jq additionally requires that sentinel shape in the final result text, so any mid-task bailout — this one or a future runtime behavior shift — fails the job loudly instead of showing green. Header comment documents the ci: AI triage silently no-ops — headless session ends turn while background Task sub-agents still running #338 failure shape next to the existing [Bug] ai-triage session completes without posting its comment (3 permission denials, 11 turns — fan-out never runs) #317 note..claude/commands/triage-dedupe.md,triage-find-issues.md,triage-find-duplicate-prs.md— same synchronous fan-out rule, plus search hardening from the same logs: one prefix-allowlistedghcommand per Bash call (shell loops/echoprefixes were being permission-denied) and a 6-search cap per agent (bursts were 403ing on the shared API rate limit).bulma-ui (
@allxsmith/bestax-bulma)create-bestax (
create-bestax)docs (
@allxsmith/bestax-docs)Other (please specify): CI workflow (
.github/workflows/ai-triage.yml) + triage command files (.claude/commands/)Related Issue(s)
Fixes #338
Related to #317, #330, #327
Type of Change
Checklist
is_errorshape, missing/empty/malformed records, single-line / multi-line / JSON-array sentinel forms)prettier --check,pnpm check:conformance, YAML parse)CLAUDE.mdfiles are updated — n/a (behavior described in CLAUDE.md is unchanged)Screenshots / Demos
n/a — see the quoted session output in #338.
Additional Context
ci:type, no package scope.issues:events run the workflow frommain, and the checkout step pulls main's.claude/commands/, so the full fix only takes effect after merge. Pre-merge, applying theai-triagelabel to this PR exercises the PR's copy of the workflow (prompt + watchdog) but still with main's command files. Post-merge test: applyai-triageto an open issue (label runs are budget-exempt) and confirm a<!-- ai-triage:dedupe -->comment appears — or a loud red run if not..github/**, which the loop refuses).🤖 Generated with Claude Code
https://claude.ai/code/session_01K1LrTrVRbtdh8paN7f67CB
Generated by Claude Code
Summary by CodeRabbit