fix(advisor): accept same-turn submit repair

test/advisor-session-context-tools.test.ts

@@ -12,6 +12,7 @@ import {
   resolveAdvisorTurnTools,
 } from "../tools/advisors/session.mts";
 import {
+  hasCompletedTerminalSubmitRepair,
   repairableTerminalSubmitToolName,
   terminalSubmitRepairErrors,
 } from "../tools/advisors/turn-protocol.mts";
@@ -34,6 +35,24 @@ const atomicMutationTools = {
   requireAssistantText: false,
   atomicTerminalToolName: ledgerToolName,
 };
+
+function terminalSubmitRepairContract(repairToolNames = ["repair_draft"]) {
+  const turn: AdvisorPromptTurn = {
+    name: "prepare",
+    prompt: "prepare",
+    terminalSubmitToolName: ledgerToolName,
+    terminalSubmitRepairPrompt: "repair",
+  };
+  return {
+    turn,
+    tools: {
+      ...atomicMutationTools,
+      atomicTerminalToolName: undefined,
+      terminalSubmitToolName: ledgerToolName,
+      terminalSubmitRepairToolNames: repairToolNames,
+    },
+  };
+}
 const analysisEvent: AdvisorTurnFlowEvent = { type: "text", text: "analysis" };
 const ledgerStart: AdvisorTurnFlowEvent = { type: "tool_start", toolName: ledgerToolName };
 const ledgerSuccess: AdvisorTurnFlowEvent = {
@@ -235,37 +254,15 @@ describe("advisor session context tool flow", () => {
   ] as const)(
     "does not repair terminal submit after %s",
     (_case, events, turnError, successful) => {
-      const turn: AdvisorPromptTurn = {
-        name: "prepare",
-        prompt: "prepare",
-        terminalSubmitToolName: ledgerToolName,
-        terminalSubmitRepairPrompt: "repair",
-      };
-      const tools = {
-        ...atomicMutationTools,
-        atomicTerminalToolName: undefined,
-        terminalSubmitToolName: ledgerToolName,
-        terminalSubmitRepairToolNames: [],
-      };
+      const { turn, tools } = terminalSubmitRepairContract([]);
       expect(
         repairableTerminalSubmitToolName(turn, [...events], tools, successful, turnError),
       ).toBe(undefined);
     },
   );
 
   it("repairs terminal submit after exactly one settled failed submit", () => {
-    const turn: AdvisorPromptTurn = {
-      name: "prepare",
-      prompt: "prepare",
-      terminalSubmitToolName: ledgerToolName,
-      terminalSubmitRepairPrompt: "repair",
-    };
-    const tools = {
-      ...atomicMutationTools,
-      atomicTerminalToolName: undefined,
-      terminalSubmitToolName: ledgerToolName,
-      terminalSubmitRepairToolNames: ["repair_draft"],
-    };
+    const { turn, tools } = terminalSubmitRepairContract();
     expect(
       repairableTerminalSubmitToolName(
         turn,
@@ -286,6 +283,58 @@ describe("advisor session context tool flow", () => {
     ).toBeUndefined();
   });
 
+  it("accepts one settled same-turn terminal submit repair (#9630)", () => {
+    const { turn, tools } = terminalSubmitRepairContract();
+    const events = [ledgerStart, ledgerFailure, ledgerStart, ledgerSuccess];
+
+    expect(hasCompletedTerminalSubmitRepair(turn, events, tools, undefined)).toBe(true);
+    expect(advisorTurnFlowErrors("prepare", events, tools, true)).toEqual([]);
+  });
+
+  it("requires configured repair and no provider error for same-turn repair (#9630)", () => {
+    const { turn, tools } = terminalSubmitRepairContract();
+    const events = [ledgerStart, ledgerFailure, ledgerStart, ledgerSuccess];
+    const withoutConfiguredRepair = hasCompletedTerminalSubmitRepair(
+      { ...turn, terminalSubmitRepairPrompt: undefined },
+      events,
+      tools,
+      undefined,
+    );
+    const withProviderFailure = hasCompletedTerminalSubmitRepair(
+      turn,
+      events,
+      tools,
+      "provider failed",
+    );
+
+    expect(withoutConfiguredRepair).toBe(false);
+    expect(advisorTurnFlowErrors("prepare", events, tools, withoutConfiguredRepair)).not.toEqual(
+      [],
+    );
+    expect(withProviderFailure).toBe(false);
+    expect(advisorTurnFlowErrors("prepare", events, tools, withProviderFailure)).not.toEqual([]);
+  });
+
+  it.each([
+    ["all attempts fail", [ledgerStart, ledgerFailure, ledgerStart, ledgerFailure]],
+    ["the second attempt is unsettled", [ledgerStart, ledgerFailure, ledgerStart]],
+    ["attempt events overlap", [ledgerStart, ledgerStart, ledgerFailure, ledgerSuccess]],
+    [
+      "a third attempt succeeds",
+      [ledgerStart, ledgerFailure, ledgerStart, ledgerFailure, ledgerStart, ledgerSuccess],
+    ],
+    [
+      "activity follows success",
+      [ledgerStart, ledgerFailure, ledgerStart, ledgerSuccess, analysisEvent],
+    ],
+  ])("does not accept same-turn terminal submit repair when %s (#9630)", (_case, events) => {
+    const { turn, tools } = terminalSubmitRepairContract();
+
+    const repaired = hasCompletedTerminalSubmitRepair(turn, events, tools, undefined);
+    expect(repaired).toBe(false);
+    expect(advisorTurnFlowErrors("prepare", events, tools, repaired)).not.toEqual([]);
+  });
+
   it("rejects prose, unconfigured tools, and multiple submits during terminal-submit repair", () => {
     const successfulSubmit = [ledgerStart, ledgerSuccess];
     expect(
@@ -329,10 +378,6 @@ describe("advisor session context tool flow", () => {
 
   it.each([
     ["duplicate success", [ledgerStart, ledgerSuccess, ledgerStart, ledgerSuccess]],
-    [
-      "failed then successful initial attempts",
-      [ledgerStart, ledgerFailure, ledgerStart, ledgerSuccess],
-    ],
     [
       "failed twice then successful initial attempts",
       [ledgerStart, ledgerFailure, ledgerStart, ledgerFailure, ledgerStart, ledgerSuccess],

test/advisor-session-runner.test.ts

@@ -395,10 +395,8 @@ describe("advisor session runner", () => {
     expect(sdk.state.prompts).toHaveLength(2);
   });
 
-  it.each([
-    ["two failed initial attempts", ["fail-twice", "success"]],
-    ["a failed then successful initial attempt", ["fail-then-success"]],
-  ] as const)("rejects %s without terminal-submit repair", async (_case, responses) => {
+  it("rejects two failed initial attempts without terminal-submit repair", async () => {
+    const responses = ["fail-twice", "success"] as const;
     sdk.state.terminalResponses = [...responses];
     const result = await run([submitTurn("prepare-and-submit")]);
 
@@ -407,6 +405,16 @@ describe("advisor session runner", () => {
     expect(sdk.state.prompts).toHaveLength(1);
   });
 
+  it("accepts one failed submit followed by one same-turn success (#9630)", async () => {
+    sdk.state.terminalResponses = ["fail-then-success"];
+    const result = await run([submitTurn("prepare-and-submit")]);
+
+    expect(result.fatalError).toBeUndefined();
+    expect(result.turnErrors).toEqual([]);
+    expect(result.raw).not.toContain("terminal_submit_repair_start");
+    expect(sdk.state.prompts).toHaveLength(1);
+  });
+
   it("allows one failed initial submit followed by one repair success", async () => {
     sdk.state.terminalResponses = ["fail-once", "success"];
     const result = await run([submitTurn("prepare-and-submit")]);

tools/advisors/session.mts

@@ -30,6 +30,7 @@ import {
   advisorTurnFlowErrors,
   atomicTerminalRepairErrors,
   atomicTerminalRepairPrompt,
+  hasCompletedTerminalSubmitRepair,
   missingRequiredAdvisorToolNames,
   normalizedToolNames,
   promptWithRequiredContextTools,
@@ -604,7 +605,12 @@ export async function runReadOnlyAdvisor(
               `[${options.logPrefix}] atomic_terminal_repair_end ${turn.name} ${repairToolName} ok\n`,
             );
           }
-          let terminalSubmitRepaired = false;
+          let terminalSubmitRepaired = hasCompletedTerminalSubmitRepair(
+            turn,
+            currentTurnFlow,
+            tools,
+            currentTurnError,
+          );
           const submitRepairToolName = repairableTerminalSubmitToolName(
             turn,
             currentTurnFlow,

tools/advisors/turn-protocol.mts

@@ -49,10 +49,11 @@ export type AdvisorPromptTurn = {
   atomicTerminalRepairPrompt?: string;
   /**
    * Terminal submit tool that may follow context, reads, prose, and other active draft tools.
-   * The initial turn permits exactly one submit attempt, and nothing may follow a success.
+   * The turn permits one success, or one failed attempt followed by one success when repair is enabled.
+   * Nothing may follow a success.
    */
   terminalSubmitToolName?: string;
-  /** Opt into one repair continuation after a settled failed terminal submit. */
+  /** Opt into one repair in the same SDK turn or one continuation after a settled failure. */
   terminalSubmitRepairPrompt?: string;
   /** Tools available during the terminal-submit repair continuation. */
   terminalSubmitRepairToolNames?: string[];
@@ -180,6 +181,37 @@ function terminalToolEventCounts(events: AdvisorTurnFlowEvent[], toolName: strin
   };
 }
 
+function terminalSubmitAttemptSequence(events: AdvisorTurnFlowEvent[], toolName: string) {
+  const outcomes: Array<"failed" | "successful"> = [];
+  let active = false;
+  let malformed = false;
+  for (const event of events) {
+    if (event.type === "tool_start" && event.toolName === toolName) {
+      if (active) malformed = true;
+      active = true;
+      continue;
+    }
+    if (event.type === "tool_end" && event.toolName === toolName) {
+      if (!active) malformed = true;
+      active = false;
+      outcomes.push(event.isError ? "failed" : "successful");
+      continue;
+    }
+    if (active) malformed = true;
+  }
+  return { outcomes, malformed, unsettled: active };
+}
+
+function hasActivityAfterSuccessfulTerminalSubmit(
+  events: AdvisorTurnFlowEvent[],
+  toolName: string,
+): boolean {
+  const successIndex = events.findIndex(
+    (event) => event.type === "tool_end" && event.toolName === toolName && !event.isError,
+  );
+  return successIndex >= 0 && events.slice(successIndex + 1).length > 0;
+}
+
 function unexpectedAtomicToolEvent(events: AdvisorTurnFlowEvent[], toolName: string) {
   return events.find((event) =>
     event.type === "text" ? Boolean(event.text.trim()) : event.toolName !== toolName,
@@ -202,6 +234,10 @@ function terminalSubmitToolErrors(
         `(observed ${counts.starts} starts and ${counts.completions} completions)`,
     );
   }
+  const sequence = terminalSubmitAttemptSequence(events, toolName);
+  if (sequence.malformed) {
+    errors.push(`${turnName} emitted a malformed ${toolName} submit attempt sequence`);
+  }
   if (
     counts.starts !== expectedAttempts ||
     counts.completions !== expectedAttempts ||
@@ -214,10 +250,13 @@ function terminalSubmitToolErrors(
         `(observed ${counts.starts} starts, ${counts.successfulCompletions} successful, and ${counts.failedCompletions} failed completions)`,
     );
   }
-  const successIndex = events.findIndex(
-    (event) => event.type === "tool_end" && event.toolName === toolName && !event.isError,
-  );
-  if (successIndex >= 0 && events.slice(successIndex + 1).length > 0) {
+  const expectedOutcomes = repaired ? ["failed", "successful"] : ["successful"];
+  if (sequence.outcomes.join(",") !== expectedOutcomes.join(",")) {
+    errors.push(
+      `${turnName} must complete ${toolName} attempts in this order: ${expectedOutcomes.join(", ")}`,
+    );
+  }
+  if (hasActivityAfterSuccessfulTerminalSubmit(events, toolName)) {
     errors.push(`${turnName} emitted activity after successful ${toolName}`);
   }
   return errors;
@@ -336,6 +375,24 @@ export function repairableTerminalSubmitToolName(
   return toolName;
 }
 
+export function hasCompletedTerminalSubmitRepair(
+  turn: AdvisorPromptTurn,
+  events: AdvisorTurnFlowEvent[],
+  tools: AdvisorTurnTools,
+  turnError: string | undefined,
+): boolean {
+  if (!turn.terminalSubmitRepairPrompt?.trim() || turnError) return false;
+  const toolName = tools.terminalSubmitToolName;
+  if (!toolName) return false;
+  const sequence = terminalSubmitAttemptSequence(events, toolName);
+  return (
+    !sequence.malformed &&
+    !sequence.unsettled &&
+    sequence.outcomes.join(",") === "failed,successful" &&
+    !hasActivityAfterSuccessfulTerminalSubmit(events, toolName)
+  );
+}
+
 export function terminalSubmitRepairPrompt(turn: AdvisorPromptTurn, toolName: string): string {
   return `${turn.terminalSubmitRepairPrompt?.trim()}\n\nComplete the repair with exactly one successful \`${toolName}\` call. Emit no prose before or after the tool calls.`;
 }

tools/pr-review-advisor/README.md

@@ -38,7 +38,13 @@ It intentionally does not report GitHub mergeability, branch protection, CI stat
 4. Opens one Pi session per model lane and performs exactly two normal turns.
 5. The `investigate` turn has repo-confined `read`, `grep`, `find`, and `ls` tools, deterministic PR context tools, and trusted terminology tracing. It examines scope, architecture and simplicity, terminology, correctness, acceptance, source-of-truth behavior, all security categories, tests, CI and operations, E2E coverage, prior findings, positives, and limitations in one coherent pass.
 6. The `challenge-and-record` turn keeps repository reads and adds `record_findings`, `record_review_receipt`, `recommend_e2e`, and `submit_review`. The first three replace complete in-memory draft sections. They do not update canonical state.
-7. `submit_review` validates the complete draft, deterministic E2E floors and allowlists, terminology trace bindings, finding references, and the public result schema. A successful call validates and assembles pending state, then ends the turn. The session runner atomically commits that state only after accepting the complete terminal flow. Failed validation does not mutate canonical state. A settled invalid call permits one bounded repair; omission, provider failure, unsettled calls, or activity after success fail closed and discard pending state.
+7. `submit_review` validates the complete draft, deterministic E2E floors and allowlists, terminology trace bindings, finding references, and the public result schema.
+   A successful call validates and assembles pending state, then ends the turn.
+   The session runner atomically commits that state only after accepting the complete terminal flow.
+   Failed validation does not mutate canonical state.
+   The `challenge-and-record` turn accepts one failed call followed by one successful call in one SDK response.
+   If the first response settles after the failed call, the controller can request one tool-only continuation.
+   Omission, provider failure, unsettled calls, extra attempts, or activity after success fail closed and discard pending state.
 8. Trusted code writes the session transcript, finding, terminology, result, summary, and detailed-review artifacts. The trusted publisher posts only validated artifacts for the same pull request commit.
 9. The primary GPT-5.6 Terra lane publishes the sticky comment. The Nemotron Ultra lane remains an artifact-only evaluation lane.
     The evaluation lane does not publish another review.