Skip to content

Advisor rejects one bounded same-turn terminal submit repair #9630

Description

@jyaunches

Investigation Summary

  • Main commit 0ac5a7cd0565ca8aac32fab1a22603da459a4158 retains the terminal-submit controller introduced by refactor(advisor): use two-turn atomic review #9590.
  • Runs 32260252038, 32260405182, and 32260813376 each recorded two submit_review starts, one failed completion, and one successful completion in one turn.
  • The successful bounded repair was rejected because terminalSubmitRepaired remains false unless the controller starts a separate repair turn.
  • Run 32261884064 recorded three starts, two failed completions, and one success. That sequence exceeds the bounded repair and must remain rejected.
  • Open issue, PR, comment, and changed-path searches found no active owner for this root.

Description

The challenge-and-record turn permits one repair after submit_review rejects a draft without changing canonical state. The SDK can complete that repair in the same turn by returning one failed completion and then one successful completion.

The session controller currently evaluates that valid sequence as an unrepaired submit. It expects one attempt with no failures and rejects the completed review. The controller should accept exactly one failed attempt followed by one successful attempt in the same turn. It must continue to reject every broader or incomplete sequence.

Reproduction Steps

  1. Run a challenge-and-record Advisor turn with submit_review as its terminal submit tool.
  2. Let the first submit_review validation fail without changing canonical state.
  3. Let the SDK call submit_review once more in the same turn and complete successfully.
  4. Evaluate the turn flow with the current session controller.

The controller reports that the turn must make exactly one submit attempt, despite the promised one bounded repair.

Expected Behavior

  • Accept exactly two settled attempts with one failed completion followed by one successful completion.
  • Reject three attempts with two failed completions and one successful completion.
  • Reject all-failed, unsettled, malformed, extra-attempt, and post-success activity.
  • Finalize pending review state only after the complete terminal flow passes validation.

Detection Gap

Existing tests require the preparatory terminal-submit path to reject a same-turn failed-then-successful sequence. They do not distinguish the #9590 challenge-and-record submit_review contract, where the same nonmutating validation tool owns the bounded repair. The regression test must cover the complete session and protocol boundary.

Environment

  • Repository: NVIDIA/NemoClaw
  • Commit under investigation: 0ac5a7cd0565ca8aac32fab1a22603da459a4158
  • Runtime boundary: GitHub Actions PR Review Advisor sessions

Debug Output

  • Run 32260252038: 2 starts, 1 failed completion, 1 successful completion
  • Run 32260405182: 2 starts, 1 failed completion, 1 successful completion
  • Run 32260813376: 2 starts, 1 failed completion, 1 successful completion
  • Run 32261884064: 3 starts, 2 failed completions, 1 successful completion

Checklist

  • I confirmed this bug is reproducible
  • I searched existing issues and this is not a duplicate

Activity

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

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