Skip to content

reviewer: posted reviews can still fail when completion marker parsing or marker verification misses them #225

@mrcfps

Description

@mrcfps

Summary

Some reviewer runs successfully post a GitHub review with a Looper marker, but the run is still marked failed because the agent did not emit a parsed __LOOPER_RESULT__ marker or because publish-time marker verification does not find the already-posted review.

This creates false failed reviewer loops and can trigger duplicate follow-up attempts.

Evidence

Posted review but run failed as missing completion marker

Loop 133 (powerformer/looper#125) shows the agent posted a GitHub review successfully:

{"id":4202572321,...,"state":"COMMENTED","commit_id":"fdd4dd124989cae611642ba392bfe462694f01b6"}

The review body includes a Looper marker:

<!-- looper:review id=reviewer:333e9f1e-75cb-466d-9224-13432ab972fe:fdd4dd124989cae611642ba392bfe462694f01b6 head=fdd4dd124989cae611642ba392bfe462694f01b6 outcome=actionable -->

But the loop ended as:

last_error_kind = non_retryable
last_error = Reviewer agent did not report a valid completion marker after publishing review

Publish step failed despite existing reviews

Loops 325 and 333 failed in step=publish with:

Reviewer agent completed but no matching GitHub review marker was found; retrying marker verification before rerunning review

Then their queue items were cancelled with loop_terminated, even though the relevant PR already had reviews/approvals in the review list.

Root cause

The runner has two separate success signals:

  1. Agent stdout marker (__LOOPER_RESULT__) parsed by the executor.
  2. GitHub review marker verification (<!-- looper:review ... -->).

When (1) is missing, the runner tries to recover by verifying (2), but that verification can miss valid posted reviews due to marker shape, idempotency key mismatch, event type, author, or timing. The fallback then treats the run as failed even though the external side effect already happened.

Relevant code:

  • internal/agent/executor.go:499-502 — failed/missing parse marker handling
  • internal/reviewer/runner.go:2083-2118 — missing parse marker recovery and final FailureNonRetryable
  • internal/reviewer/runner.go:2144+ — publish step verification of pending review markers

Expected behavior

If a Looper-marked GitHub review exists for the expected head, the reviewer run should converge to success or skipped/idempotent completion even when the agent fails to emit __LOOPER_RESULT__.

It should not leave a failed loop after an external review was already posted.

Proposed fix

  • Make marker verification tolerant of legacy/current marker id shapes where safe.
  • Search recent reviews/comments by expected head and marker prefix before failing missing completion marker cases.
  • Treat found existing marker as PendingReview and advance to publish/idempotent completion.
  • If verification is inconclusive, prefer retryable_after_resume over non_retryable so the runner can re-check GitHub state before rerunning the full review.
  • Add tests where the agent posts a valid marked review but omits __LOOPER_RESULT__.

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions