[local-test-e2e-cover-the-local-auto-review-loo] test(e2e): cover the local --auto-review loop and iteration cap

packages/e2e/README.md

@@ -86,6 +86,34 @@ and secret `E2E_GITHUB_TOKEN` before running it. Do not expose these
 credentials to untrusted pull-request code. Build the CLI in that job before
 running the suite so the artifact matches the checkout.
 
+## Local auto-review loop
+
+`tests/local/cli-auto-review.test.ts` runs the built CLI's `--auto-review` loop
+against a local bare Git remote and the deterministic fixture agent, with no
+hosted PR, account, or network access. The fixture agent gains two prompt-driven
+modes: `auto-review-feedback` (the bundled `review.md` prompt) prints only a
+JSON verdict, and `auto-review-fix` (the bundled `fix.md` prompt) edits
+`src/greeting.ts` without committing or pushing. The reviewer reports one
+`medium` finding on iteration 1 and approves from iteration 2, so the loop
+converges; `E2E_AUTO_REVIEW_STUCK=1` keeps every iteration unapproved to prove
+`--auto-review-iterations` caps the review/fix cycles. Both modes are recorded
+in `E2E_AGENT_INVOCATIONS` so the test asserts the exact
+implement → review → fix → review sequence.
+
+The scenarios assert the pushed `feature/<task-key>` branch, the exact
+`src/greeting.ts` diff, the two commits ahead of base (the implementation plus
+`fix: address PR review feedback`), and the
+`DEVINTERN_OUTPUT_DIR/<task-key>/auto-review-summary.json` artifact
+(`iterations`, `success`, and `history`). Per-iteration verdicts are saved under
+`iteration-<n>/feedback.json`. A non-approved loop still exits 0 and delivers
+the branch; the local remote has no supported PR host, so `--create-pr` pushes
+and exits without a PR URL.
+
+```sh
+bun run --filter @getdevintern/code build
+bun run --filter @devintern/e2e test
+```
+
 ## Worker `worker add` option matrix
 
 The default local suite (`tests/local/worker-add-options.test.ts`) drives the built

packages/e2e/docs/coverage-matrix.md

@@ -68,7 +68,7 @@ independently of this repository.
 | GitHub: CI failure repair and retry budget                                                 | `gap`     | —                                     | Worker parity with the GitLab CI scenarios.                                             |
 | GitHub: automatic merge-conflict resolution and scheduled mode                             | `gap`     | —                                     | Worker parity with the GitLab conflict scenario.                                        |
 | GitHub: `@mention` handling                                                                | `gap`     | —                                     | —                                                                                       |
-| GitHub: `--auto-review` loop and iteration cap                                             | `gap`     | —                                     | Both providers; fake reviewer agent.                                                    |
+| GitHub: `--auto-review` loop and iteration cap                                             | `default` | `tests/local/cli-auto-review.test.ts` | Provider parity: real PR review feedback on GitHub/GitLab remains live/opt-in.          |
 | GitLab Self-Managed: MR, default branch, invalid API credentials                           | `live`    | `tests/code/gitlab.test.ts`           | —                                                                                       |
 | GitLab Self-Managed: collision, rejected push, address-review                              | `live`    | `tests/code/gitlab-lifecycle.test.ts` | Standalone `address-review` CLI options.                                                |
 | GitLab Self-Managed worker: injected task, polling, CI repair/escalation, review, conflict | `live`    | `tests/code/gitlab-worker.test.ts`    | Automations, estimations, quiet hours, failover, orphan recovery, service, auto-update. |
@@ -124,7 +124,7 @@ Run: `bun run --filter @devintern/e2e test` (build the CLI first); the sandbox s
 | `--force`                                                                | `gap`              | —                                                                                                                             | Incomplete task rerun gate.                                            |
 | `--no-git`, `--no-auto-commit`                                           | `gap`              | —                                                                                                                             | Branch/commit omissions.                                               |
 | `--estimate`                                                             | `gap`              | —                                                                                                                             | Per-tracker story-point write/comment.                                 |
-| `--auto-review`, `--auto-review-iterations`                              | `gap`              | —                                                                                                                             | Review/fix loop and cap.                                               |
+| `--auto-review`, `--auto-review-iterations`                              | `default`          | `tests/local/cli-auto-review.test.ts` (`bun run --filter @devintern/e2e test`)                                                | Provider parity: live PR review feedback remains opt-in.               |
 | `--env-file`                                                             | `gap`              | —                                                                                                                             | Alternate credential source and precedence.                            |
 | `--max-turns`, `--hook-retries`                                          | `partial`          | helper passes `--hook-retries`                                                                                                | Failure/retry path.                                                    |
 | `--sandbox <provider>`                                                   | `partial`          | `tests/local/sandbox.test.ts` (per-provider fail-closed, removed srt/smolvm migration, native/goose rejection, unknown value) | Real run per provider remains optional.                                |
@@ -190,7 +190,9 @@ and each task opens its own PR.
 4. Trello lifecycle against a dedicated board (Layer 2), when that board and
    token exist.
 5. GitHub worker parity: review polling, CI repair, conflict resolution, mention.
-6. `--auto-review` loop and iteration cap on GitHub and GitLab.
+6. ~~`--auto-review` loop and iteration cap on GitHub and GitLab~~ (the local
+   loop/cap is implemented by `tests/local/cli-auto-review.test.ts`; provider
+   parity with real PR review feedback remains live/opt-in).
 7. `worker add` full option matrix (`--body-file`, idempotency, resume, wait,
    wait-timeout, workspace, validation and deduplication).
 8. `worker scaffold`, `add-repo`, `run-now`, and `connect status` in temp homes.

packages/e2e/helpers/fake-agent.ts

@@ -5,7 +5,17 @@ import { appendFileSync, readFileSync, writeFileSync } from "node:fs";
 // cannot: the same daemon runs implementation, review, CI-repair and conflict
 // jobs, so those tests identify the prompt by a seeded marker instead.
 const agentInput = process.argv.join(" ");
+// The local auto-review loop sends the bundled review and fix prompts from the
+// same one-shot CLI run that implemented the task. Detect those turns before
+// the explicit E2E_AGENT_MODE so a single run can exercise
+// implement → review → fix → review without the fixture pinning one mode.
+const promptMode = agentInput.includes("You are reviewing a pull request")
+  ? "auto-review-feedback"
+  : agentInput.includes("## Feedback to Address")
+    ? "auto-review-fix"
+    : undefined;
 const mode =
+  promptMode ??
   process.env.E2E_AGENT_MODE ??
   (agentInput.includes("# Resolve Merge Conflicts")
     ? "conflict"
@@ -73,6 +83,60 @@ if (
 if (missingMarkers.length > 0) {
   throw new Error(`Seeded ${missingMarkers.join(" and ")} marker(s) did not reach the agent input`);
 }
+// Auto-review reviewer turn: print only the JSON verdict the loop parses. The
+// first iteration reports one blocking medium finding; a later iteration
+// approves so the loop converges. E2E_AUTO_REVIEW_STUCK keeps every iteration
+// unapproved to prove the iteration cap.
+if (mode === "auto-review-feedback") {
+  const iteration = Number(agentInput.match(/\*\*Review Iteration\*\*:\s*(\d+)/)?.[1] ?? "1");
+  const stuck = process.env.E2E_AUTO_REVIEW_STUCK === "1";
+  if (!stuck && iteration > 1) {
+    console.log(
+      JSON.stringify({
+        summary: "The review finding was addressed and no blocking issues remain.",
+        approved: true,
+        items: [],
+      }),
+    );
+  } else {
+    console.log(
+      JSON.stringify({
+        summary: "The greeting change works, but the exported value is not asserted.",
+        approved: false,
+        items: [
+          {
+            priority: "medium",
+            confidence: "high",
+            category: "code-quality",
+            file: "src/greeting.ts",
+            line: "1",
+            issue:
+              "The fixture test only checks a Hello prefix, so it cannot catch a wrong greeting value.",
+            evidence:
+              "tests/greeting.test.ts:5 asserts only a Hello prefix, not the exact exported value",
+            suggestion: "Assert the exact exported greeting value in tests/greeting.test.ts.",
+          },
+        ],
+      }),
+    );
+  }
+  process.exit(0);
+}
+// Auto-review fixer turn: apply the suggested edit and leave the commit to the
+// production loop. Accept an already-fixed file so a later fix turn is a no-op.
+if (mode === "auto-review-fix") {
+  const fixedPath = "src/greeting.ts";
+  const fixed = 'export const greeting = "Hello, reviewer!";\n';
+  const current = readFileSync(fixedPath, "utf8");
+  if (current !== fixed) {
+    if (current !== 'export const greeting = "Hello, world!";\n') {
+      throw new Error("Unexpected fixture contents; refusing to edit this workspace");
+    }
+    writeFileSync(fixedPath, fixed);
+  }
+  console.log("Applied the review suggestion: src/greeting.ts now exports Hello, reviewer!");
+  process.exit(0);
+}
 const path = "src/greeting.ts";
 if (mode === "no-change") {
   console.log(

packages/e2e/helpers/fixture-workspace.ts

@@ -171,6 +171,8 @@ export class FixtureWorkspace {
       skipComments?: boolean;
       /** Labels the CLI must apply to the created PR (`PR_LABELS`). */
       prLabels?: string[];
+      /** Additional CLI arguments appended after the fixture's defaults. */
+      extraArgs?: string[];
     } = {},
   ) {
     return command(
@@ -189,6 +191,7 @@ export class FixtureWorkspace {
         "--hook-retries",
         "0",
         "--no-update",
+        ...(options.extraArgs ?? []),
       ],
       this.cwd,
       {

packages/e2e/tests/local/cli-auto-review.test.ts

@@ -0,0 +1,162 @@
+import { expect, test } from "bun:test";
+import { existsSync, readFileSync } from "node:fs";
+import { join } from "node:path";
+import { cliPath, FixtureWorkspace } from "../../helpers/fixture-workspace";
+import { checkedCommand } from "../../helpers/process";
+
+/**
+ * Seed a local bare Git remote so the run exercises the production git path
+ * without a hosted code host. `--create-pr` still pushes and exits 0 because
+ * no supported platform is detected.
+ *
+ * @param run - Fixture workspace whose isolated environment is reused
+ */
+async function prepareLocalRemote(run: FixtureWorkspace): Promise<void> {
+  const remote = join(run.root, "remote.git");
+  await checkedCommand(["git", "init", "--bare", remote], run.root, run.env);
+  await run.prepare(remote, "test-token", {});
+}
+
+/**
+ * Path of the auto-review summary the CLI writes under the output directory.
+ *
+ * @param run - Fixture workspace that ran the task
+ */
+function autoReviewDir(run: FixtureWorkspace): string {
+  return join(run.env.DEVINTERN_OUTPUT_DIR, run.taskKey.toLowerCase());
+}
+
+/**
+ * Read and parse a JSON artifact, failing with its path when it is absent.
+ *
+ * @param path - Artifact path inside the fixture's temporary output directory
+ */
+function readJson(path: string): Record<string, unknown> {
+  if (!existsSync(path)) throw new Error(`Expected artifact was not written: ${path}`);
+  return JSON.parse(readFileSync(path, "utf8")) as Record<string, unknown>;
+}
+
+/** The exact base-to-head diff the two auto-review commits must produce. */
+const EXPECTED_GREETING_DIFF = [
+  "diff --git a/src/greeting.ts b/src/greeting.ts",
+  "--- a/src/greeting.ts",
+  "+++ b/src/greeting.ts",
+  "@@ -1 +1 @@",
+  '-export const greeting = "Hello";',
+  '+export const greeting = "Hello, reviewer!";',
+].join("\n");
+
+test.skipIf(!existsSync(cliPath))(
+  "the local --auto-review loop fixes a finding, approves, and pushes both commits",
+  async () => {
+    const run = new FixtureWorkspace();
+    try {
+      await prepareLocalRemote(run);
+      const result = await run.runTask({
+        extraArgs: ["--auto-review", "--auto-review-iterations", "2"],
+      });
+
+      expect(result.exitCode).toBe(0);
+      // The local bare remote has no supported PR host: the documented behavior
+      // is still to push the branch and exit successfully, without a PR URL.
+      expect(await run.remoteSha(run.head)).toBe(await run.git("rev-parse", "HEAD"));
+      expect(await run.git("branch", "--show-current")).toBe(run.head);
+
+      // The loop's fix commit lands on the same task branch: implementation
+      // plus exactly one review-fix commit.
+      expect(await run.git("rev-list", "--count", `${run.base}..HEAD`)).toBe("2");
+      expect(await run.git("log", "-1", "--format=%s")).toBe("fix: address PR review feedback");
+      expect(await run.git("diff", "--name-only", `${run.base}...HEAD`)).toBe("src/greeting.ts");
+      expect(await run.git("show", "HEAD:src/greeting.ts")).toBe(
+        'export const greeting = "Hello, reviewer!";',
+      );
+      const diff = await run.git("diff", `${run.base}...HEAD`, "--", "src/greeting.ts");
+      expect(
+        diff
+          .split("\n")
+          .filter((line) => !line.startsWith("index "))
+          .join("\n"),
+      ).toBe(EXPECTED_GREETING_DIFF);
+
+      const outputDir = autoReviewDir(run);
+      const summary = readJson(join(outputDir, "auto-review-summary.json"));
+      expect(summary.iterations).toBe(2);
+      expect(summary.success).toBe(true);
+      expect(summary.history).toHaveLength(2);
+
+      const firstFeedback = readJson(join(outputDir, "iteration-1", "feedback.json")) as {
+        items: Array<Record<string, unknown>>;
+      };
+      expect(firstFeedback.items).toHaveLength(1);
+      expect(firstFeedback.items[0]).toMatchObject({
+        priority: "medium",
+        category: "code-quality",
+      });
+      expect(firstFeedback.items[0].issue).toBeTruthy();
+      expect(firstFeedback.items[0].suggestion).toBeTruthy();
+      expect(firstFeedback.items[0].evidence).toBeTruthy();
+
+      // One implementation turn, then review → fix → review: the second review
+      // approves and ends the loop before the cap.
+      expect(run.agentInvocations().map((invocation) => invocation.mode)).toEqual([
+        "implement",
+        "auto-review-feedback",
+        "auto-review-fix",
+        "auto-review-feedback",
+      ]);
+
+      expect(result.stdout).toContain("Auto-review loop finished after 2 iteration(s)");
+      // The loop stops on "no important issues to address" before its
+      // `approved` check, so the converged success is this line plus the
+      // success status, not the unreachable `PR approved` branch.
+      expect(result.stdout).toContain("No important issues to address. Review loop complete!");
+      expect(result.stdout).toContain("Status: ✅ Success");
+      expect(run.runRecords().map((record) => record.status)).toEqual(["succeeded"]);
+      expect(await run.git("status", "--porcelain")).toBe("");
+    } finally {
+      run.dispose();
+    }
+  },
+  60_000,
+);
+
+test.skipIf(!existsSync(cliPath))(
+  "the --auto-review-iterations cap stops a non-converging review after one cycle",
+  async () => {
+    const run = new FixtureWorkspace();
+    try {
+      await prepareLocalRemote(run);
+      run.env.E2E_AUTO_REVIEW_STUCK = "1";
+      const result = await run.runTask({
+        extraArgs: ["--auto-review", "--auto-review-iterations", "1"],
+      });
+
+      // A non-approved review is reported, not fatal: the branch is still
+      // delivered with the fix commit.
+      expect(result.exitCode).toBe(0);
+      expect(run.agentInvocations().map((invocation) => invocation.mode)).toEqual([
+        "implement",
+        "auto-review-feedback",
+        "auto-review-fix",
+      ]);
+      expect(await run.remoteSha(run.head)).toBe(await run.git("rev-parse", "HEAD"));
+      expect(await run.git("rev-list", "--count", `${run.base}..HEAD`)).toBe("2");
+      expect(await run.git("show", "HEAD:src/greeting.ts")).toBe(
+        'export const greeting = "Hello, reviewer!";',
+      );
+
+      const summary = readJson(join(autoReviewDir(run), "auto-review-summary.json"));
+      expect(summary.iterations).toBe(1);
+      expect(summary.success).toBe(false);
+      expect(summary.history).toHaveLength(1);
+
+      expect(result.stdout).toContain("Auto-review loop finished after 1 iteration(s)");
+      expect(result.stdout).toContain("Incomplete");
+      expect(run.runRecords().map((record) => record.status)).toEqual(["succeeded"]);
+      expect(await run.git("status", "--porcelain")).toBe("");
+    } finally {
+      run.dispose();
+    }
+  },
+  60_000,
+);