fix(review-pr): stop posting a false LGTM when the agent exits 0 without a review

review-pr/action.yml

@@ -975,27 +975,30 @@ runs:
           fi
         else
           STATUS="✅ **Review completed**"
-          # Defense-in-depth: if the log exists but no review was posted, post a fallback LGTM comment.
-          # This guards against the agent exiting 0 without calling gh api (e.g., zero-findings early exit).
+          # Defense-in-depth: if the log exists but no review was posted, post a neutral
+          # incomplete-run notice. Exit 0 without a posted review can be a degenerate agent
+          # run (e.g. a drafter loop), so the fallback must not read as a clean review and
+          # must not match the incremental-review completion markers.
           if [ -n "$VERBOSE_LOG_FILE" ] && [ -f "$VERBOSE_LOG_FILE" ] && ! grep -qE 'pullrequestreview-[0-9]+' "$VERBOSE_LOG_FILE" 2>/dev/null; then
-            # Dedup guard: skip posting if an identical fallback PR review already exists.
-            # Also check the old issue-comment endpoint for a one-time migration guard
-            # (prior runs posted to /issues/comments; this prevents a duplicate on the first
-            # run of this new code against PRs that already received an LGTM issue comment).
-            EXISTING=$(gh api "repos/$REPOSITORY/pulls/$PR_NUMBER/reviews" \
+            # Dedup guard: skip posting if a fallback PR review already exists. Matches the
+            # current notice plus the legacy LGTM fallback so PRs that already received
+            # either kind never get a second one. Paginated to cover PRs with >30 reviews.
+            # The old issue-comment endpoint is a one-time migration guard (prior runs
+            # posted the LGTM fallback to /issues/comments).
+            EXISTING=$(gh api --paginate "repos/$REPOSITORY/pulls/$PR_NUMBER/reviews" \
+              --jq '[.[] | select((.body // "") | (startswith("⚠️ **Review did not complete**") or startswith("🟢 **No issues found**")))] | length' \
+              2>/dev/null | jq -s 'add // 0' || true)
+            EXISTING_OLD=$(gh api --paginate "repos/$REPOSITORY/issues/$PR_NUMBER/comments" \
               --jq '[.[] | select((.body // "") | startswith("🟢 **No issues found**"))] | length' \
-              2>/dev/null || echo "0")
-            EXISTING_OLD=$(gh api "repos/$REPOSITORY/issues/$PR_NUMBER/comments" \
-              --jq '[.[] | select((.body // "") | startswith("🟢 **No issues found**"))] | length' \
-              2>/dev/null || echo "0")
+              2>/dev/null | jq -s 'add // 0' || true)
             if [ "${EXISTING:-0}" -gt 0 ] || [ "${EXISTING_OLD:-0}" -gt 0 ]; then
-              echo "ℹ️  Fallback LGTM review already exists — skipping duplicate post"
+              echo "ℹ️  Fallback review already exists — skipping duplicate post"
             else
-              echo "::warning::Agent exited 0 but no review was posted — posting fallback LGTM review"
-              jq -n --arg body "🟢 **No issues found** — LGTM! [View logs]($RUN_URL)." --arg event "COMMENT" \
+              echo "::warning::Agent exited 0 but no review was posted — posting incomplete-review notice"
+              jq -n --arg body "⚠️ **Review did not complete** — the agent finished without posting a review. Re-request a review from \`docker-agent\` to retry. [View logs]($RUN_URL)." --arg event "COMMENT" \
                 '{body: $body, event: $event, comments: []}' \
               | gh api "repos/$REPOSITORY/pulls/$PR_NUMBER/reviews" --input - 2>&1 || \
-                echo "::warning::Failed to post fallback LGTM review to PR"
+                echo "::warning::Failed to post incomplete-review notice to PR"
             fi
           fi
         fi

src/incremental-review/__tests__/incremental-review.test.ts

@@ -53,8 +53,13 @@ describe('findLastReviewedSha', () => {
     expect(findLastReviewedSha(reviews)).toBe(SHA_B);
   });
 
-  it('accepts the LGTM fallback body as a completed review', () => {
-    expect(findLastReviewedSha([review({ body: '🟢 **No issues found** — LGTM!' })])).toBe(SHA_A);
+  it('ignores the incomplete-run fallback review (agent exited 0 without posting)', () => {
+    const body = '⚠️ **Review did not complete** — the agent finished without posting a review.';
+    expect(findLastReviewedSha([review({ body })])).toBeNull();
+  });
+
+  it('ignores the legacy LGTM fallback review (never a completed run)', () => {
+    expect(findLastReviewedSha([review({ body: '🟢 **No issues found** — LGTM!' })])).toBeNull();
   });
 
   it('accepts the GitHub App bot login variant', () => {

src/incremental-review/incremental-review.ts

@@ -13,9 +13,9 @@
  * SHA at posting time. This survives across workflow runs, requires no extra
  * writes, and cannot be edited away like a marker embedded in a comment body.
  * Only reviews that represent a *completed* run count — an assessment body
- * ("### Assessment:") or the zero-findings LGTM fallback. Timeout and failure
- * fallback reviews do NOT mark commits as reviewed, so the next run re-covers
- * them.
+ * ("### Assessment:", posted even for zero findings). Timeout, failure, and
+ * incomplete-run fallback reviews (including the retired zero-findings LGTM
+ * fallback) do NOT mark commits as reviewed, so the next run re-covers them.
  *
  * ## Fallbacks to a full review (see planIncrementalReview)
  *
@@ -64,10 +64,12 @@ export interface IncrementalPlan {
 // looser (e.g. a body that starts with "-") must never get through.
 const SHA40 = /^[0-9a-f]{40}$/i;
 
-// Bodies that mark a review run as completed. The timeout ("⏱️ **PR Review
-// Timed Out**") and failure ("❌ **PR Review Failed**") fallbacks match
-// neither, so unreviewed commits stay unreviewed.
-const COMPLETED_BODY_MARKERS = ['### Assessment:', '🟢 **No issues found**'];
+// Bodies that mark a review run as completed. Only the agent's own assessment
+// body counts: the timeout ("⏱️ **PR Review Timed Out**"), failure ("❌ **PR
+// Review Failed**"), and incomplete-run ("⚠️ **Review did not complete**")
+// fallbacks match nothing here, and neither does the retired "🟢 **No issues
+// found**" LGTM fallback, so commits covered only by a fallback stay unreviewed.
+const COMPLETED_BODY_MARKERS = ['### Assessment:'];
 
 // GitHub presents the bot identity as "docker-agent" when posting with a
 // machine user token, or "docker-agent[bot]" through a GitHub App installation

src/rate-limit/index.ts

@@ -16,7 +16,7 @@
  * Counting is per LLM run, so each run contributes exactly one unit:
  *   - Reviews are posted via the Reviews API (POST /pulls/{n}/reviews) with no
  *     inline marker — a findings review, a zero-finding APPROVE, and the
- *     timeout/error/LGTM fallbacks all land there. They are counted from
+ *     timeout/error/incomplete-run fallbacks all land there. They are counted from
  *     `pulls.listReviews` by bot author (a real review run always carries an
  *     assessment/status body); the inline finding comments such a review carries
  *     are deliberately not counted, since that would be N units per single run.
@@ -102,7 +102,7 @@ function isAgentReplyComment(c: CommentLike, botLogin: string, windowStartMs: nu
 function isAgentReview(r: ReviewLike, botLogin: string, windowStartMs: number): boolean {
   if (!matchesBotLogin(r.user?.login, botLogin)) return false;
   // A real review run always carries an assessment/status body ("### Assessment:
-  // …", or a timeout/error/LGTM fallback). Standalone inline comments and replies
+  // …", or a timeout/error/incomplete-run fallback). Standalone inline comments and replies
   // surface in this endpoint as empty-body review entries; skipping them keeps
   // each review run counted exactly once and avoids double-counting an inline
   // reply (already counted via its reply marker on the comment endpoints).