Skip to content

fix(review): preserve numbering and diagnostics after failed attempts - #655

Open
cookerpapa wants to merge 1 commit into
HenryLach:mainfrom
cookerpapa:fix/reviewer-attempts
Open

fix(review): preserve numbering and diagnostics after failed attempts#655
cookerpapa wants to merge 1 commit into
HenryLach:mainfrom
cookerpapa:fix/reviewer-attempts

Conversation

@cookerpapa

Copy link
Copy Markdown

Summary

An inline reviewer can exit without a review file while consuming an R00N number and advancing the review round. Its stderr and exit details are also lost. This makes retries difficult to diagnose and inflates the reported round.

Type of Change

  • fix (bug fix)

Changes

  • Run inline reviewers through the existing agent host to retain events, exit summaries, timeout handling, and stderr tails. Each attempt has unique diagnostic files, so retries preserve earlier failures.
  • Log missing/empty-output failures in STATUS.md and retain the review number for the next attempt. Remove empty artifacts so they cannot supersede an earlier gate review.
  • Advance rounds only for APPROVE/REVISE/RETHINK, using the same transition for live execution and resume. Preserve the explicit treatUnavailableAsNonApprove streak option.
  • Preserve configuration, dashboard telemetry, and existing fail-closed handling of nonempty reviews with unclear verdicts. Such artifacts still retain their allocated number.

Validation

  • npm run typecheck
  • npm run lint
  • npm run format:check
  • 65 focused tests pass, including 10 new subprocess tests and a resume regression. Seven subprocess regressions were first reproduced on unchanged main.
  • Full native Node test suite: 4,107 pass, 6 fail. The same six failures reproduce on unchanged main (293d9a8): execution-path-resolution, repo-mode template lookup in project-config-loader, and four Windows absolute-path cases in workspace-config.integration.

The full suite used the documented Node runner with --experimental-strip-types, --experimental-test-module-mocks and tests/loader.mjs.

Documentation

  • Review-loop behavior and STATUS review-counter reference updated.

Checklist

  • Changes are scoped and focused.
  • No secrets or private data introduced.

Related Issues

Related to #653. This addresses missing/empty-output attempts; nonempty malformed review artifacts retain the existing numbering behavior.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant