fix(review): fail loudly on empty harness output#10
Conversation
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
📝 WalkthroughWalkthroughThe PR enhances error handling in the review process by adding exit code validation to ChangesReview Error Handling and Failure Reporting
🎯 2 (Simple) | ⏱️ ~8 minutes
🚥 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 docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@review/agent.ts`:
- Around line 115-127: In failReviewRun, isolate GitHub and Slack notification
failures by wrapping the ctx.github.comment(...) call and the
ctx.slack.post(...) call in separate try/catch blocks so one exception doesn't
prevent the other from running; catch and log each error (use the existing
logger available in the function, e.g. ctx.logger.error or processLogger.error)
and continue, ensuring you still compute channel via input(ctx, 'SLACK_CHANNEL')
and use the same message and reason variables when sending notifications.
🪄 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: CHILL
Plan: Pro Plus
Run ID: dbe0af55-d31d-4455-9fc3-0e82490de9dd
📒 Files selected for processing (1)
review/agent.ts
| if (ctx.github?.comment) { | ||
| await ctx.github.comment( | ||
| { owner: pr.owner, repo: pr.repo, number: pr.number }, | ||
| message, | ||
| ); | ||
| } | ||
| const channel = input(ctx, 'SLACK_CHANNEL'); | ||
| if (channel && ctx.slack) { | ||
| await ctx.slack.post( | ||
| channel, | ||
| `:warning: pr-reviewer failed for PR #${pr.number} in *${pr.owner}/${pr.repo}*: ${reason}`, | ||
| ); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the relevant function and surrounding code with line numbers
nl -ba review/agent.ts | sed -n '80,170p'
# Locate failReviewRun definition and any callers/handling
rg -n "failReviewRun" review/agent.ts
rg -n "ctx\.github\.comment|ctx\.slack\.post" review/agent.ts
# Check if there's any higher-level try/catch in the file that could already isolate these failures
rg -n "try\s*[{]|catch\s*\(" review/agent.tsRepository: AgentWorkforce/agents
Length of output: 108
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== review/agent.ts lines 80-170 (raw) ==="
sed -n '80,170p' review/agent.ts
echo
echo "=== failReviewRun occurrences ==="
rg -n "failReviewRun" review/agent.ts
echo
echo "=== ctx.github.comment / ctx.slack.post occurrences ==="
rg -n "ctx\.github\.comment|ctx\.slack\.post" review/agent.ts
echo
echo "=== try/catch occurrences in review/agent.ts ==="
rg -n "try\s*[{]|catch\s*\(" review/agent.tsRepository: AgentWorkforce/agents
Length of output: 4405
Isolate GitHub/Slack notification failures in failReviewRun so one error doesn’t block the other.
failReviewRun calls await ctx.github.comment(...) and then await ctx.slack.post(...) without try/catch; if either throws, the other notification won’t be sent (there’s no surrounding error handling in this file).
Suggested fix
async function failReviewRun(ctx: WorkforceCtx, pr: Pr, reason: string): Promise<never> {
const message = [
`pr-reviewer could not complete review for #${pr.number} in ${pr.owner}/${pr.repo}.`,
reason,
'No review was posted; this needs operator attention.',
].join('\n');
ctx.log?.('error', 'pr-reviewer harness failed', {
owner: pr.owner,
repo: pr.repo,
number: pr.number,
reason,
});
if (ctx.github?.comment) {
- await ctx.github.comment(
- { owner: pr.owner, repo: pr.repo, number: pr.number },
- message,
- );
+ try {
+ await ctx.github.comment(
+ { owner: pr.owner, repo: pr.repo, number: pr.number },
+ message,
+ );
+ } catch (error) {
+ ctx.log?.('warn', 'pr-reviewer.fail-comment-post-failed', {
+ owner: pr.owner,
+ repo: pr.repo,
+ number: pr.number,
+ error: String(error),
+ });
+ }
}
const channel = input(ctx, 'SLACK_CHANNEL');
if (channel && ctx.slack) {
- await ctx.slack.post(
- channel,
- `:warning: pr-reviewer failed for PR #${pr.number} in *${pr.owner}/${pr.repo}*: ${reason}`,
- );
+ try {
+ await ctx.slack.post(
+ channel,
+ `:warning: pr-reviewer failed for PR #${pr.number} in *${pr.owner}/${pr.repo}*: ${reason}`,
+ );
+ } catch (error) {
+ ctx.log?.('warn', 'pr-reviewer.fail-slack-post-failed', {
+ owner: pr.owner,
+ repo: pr.repo,
+ number: pr.number,
+ channel,
+ error: String(error),
+ });
+ }
}
throw new Error(message);
}🤖 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 `@review/agent.ts` around lines 115 - 127, In failReviewRun, isolate GitHub and
Slack notification failures by wrapping the ctx.github.comment(...) call and the
ctx.slack.post(...) call in separate try/catch blocks so one exception doesn't
prevent the other from running; catch and log each error (use the existing
logger available in the function, e.g. ctx.logger.error or processLogger.error)
and continue, ensuring you still compute channel via input(ctx, 'SLACK_CHANNEL')
and use the same message and reason variables when sending notifications.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 1 file
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="review/agent.ts">
<violation number="1" location="review/agent.ts:116">
P2: GitHub failure comment can abort whole failure path. Catch this call so Slack alert still runs.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| reason, | ||
| }); | ||
| if (ctx.github?.comment) { | ||
| await ctx.github.comment( |
There was a problem hiding this comment.
P2: GitHub failure comment can abort whole failure path. Catch this call so Slack alert still runs.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At review/agent.ts, line 116:
<comment>GitHub failure comment can abort whole failure path. Catch this call so Slack alert still runs.</comment>
<file context>
@@ -92,6 +100,34 @@ async function reviewAndFix(ctx: WorkforceCtx, pr: Pr): Promise<void> {
+ reason,
+ });
+ if (ctx.github?.comment) {
+ await ctx.github.comment(
+ { owner: pr.owner, repo: pr.repo, number: pr.number },
+ message,
</file context>
1 participant
Summary
Tests
Summary by cubic
Fail fast when the review harness exits non-zero or produces no review, and post visible failure notices to GitHub and Slack so bad runs don’t look successful.
exitCodeand empty review output and treat them as hard failures.failReviewRun()to log the error, comment on the PR, notify Slack viaSLACK_CHANNEL, and throw to stop the run.Written for commit 2bbdd83. Summary will update on new commits.
Review in cubic