Skip to content

fix(review): fail loudly on empty harness output #10

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
khaliqgant merged 1 commit into from
May 28, 2026
Merged

fix(review): fail loudly on empty harness output #10

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 36 additions & 0 deletions review/agent.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,12 +67,20 @@ async function reviewAndFix(ctx: WorkforceCtx, pr: Pr): Promise<void> {
].join('\n')
});

const exitCode = (run as { exitCode?: unknown }).exitCode;
if (typeof exitCode === 'number' && exitCode !== 0) {
await failReviewRun(ctx, pr, `The review harness exited with code ${exitCode}.`);
}

// The harness only writes a review when we explicitly post it. Strip the
// READY sentinel (it's the slack/ready signal, not a review-body line) and
// post whatever's left as a PR comment via ctx.github.comment.
const raw = (run.output ?? '').trimEnd();
const ready = lastLine(raw) === 'READY';
const body = ready ? stripLastLine(raw).trimEnd() : raw;
if (!body) {
await failReviewRun(ctx, pr, 'The review harness produced no review output.');
}
if (body && ctx.github?.comment) {
await ctx.github.comment(
{ owner: pr.owner, repo: pr.repo, number: pr.number },
Expand All @@ -92,6 +100,34 @@ async function reviewAndFix(ctx: WorkforceCtx, pr: Pr): Promise<void> {
}
}

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(

@cubic-dev-ai cubic-dev-ai Bot May 28, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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>

{ 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}`,
);
}
Comment on lines +115 to +127

@coderabbitai coderabbitai Bot May 28, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 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.ts

Repository: 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.ts

Repository: 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.

throw new Error(message);
}

async function mergePr(ctx: WorkforceCtx, pr: Pr): Promise<void> {
if (!ctx.github) return;
const github = ctx.github as GithubMergeClient;
Expand Down