Merge dev into main: structured review session tags and no retry on usage limits

action.yml

@@ -132,7 +132,7 @@ inputs:
     required: false
     default: ""
   path_to_droid_executable:
-    description: "Optional path to a custom Droid CLI executable. If provided, skips automatic installation and uses this executable instead. WARNING: Using an older version may cause problems if the action begins taking advantage of new Droid Exec features. This input is typically not needed unless you're debugging something specific or have unique needs in your environment."
+    description: "Optional path to a custom Droid CLI executable. If provided, skips automatic installation and uses this executable instead. WARNING: Using an older version may cause problems if the action begins taking advantage of new Droid Exec features. Review runs pass a JSON `--tag` (supported since the June 2026 CLI); an older CLI stores the JSON as a literal tag name and the session is not classified as a code review. This input is typically not needed unless you're debugging something specific or have unique needs in your environment."
     required: false
     default: ""
   path_to_bun_executable:

base-action/src/run-droid.ts

@@ -10,6 +10,10 @@ import {
   isModelPolicyError,
   stripModelArgs,
 } from "./utils/model-policy-error";
+import {
+  describeUsageLimitError,
+  isUsageLimitError,
+} from "./utils/usage-limit-error";
 
 const execAsync = promisify(exec);
 
@@ -160,6 +164,18 @@ export class MaxTurnsExceededError extends Error {
   }
 }
 
+/**
+ * The organization's usage limit (or a BYOK provider's balance) is exhausted.
+ * Limits reset on a 5-hour/weekly/monthly schedule, so re-running within
+ * seconds cannot succeed; each retry only opens another failed session.
+ */
+export class UsageLimitError extends Error {
+  constructor(detail: string) {
+    super(`Droid could not run because a usage limit was reached: ${detail}`);
+    this.name = "UsageLimitError";
+  }
+}
+
 export function prepareRunConfig(
   promptPath: string,
   options: DroidOptions,
@@ -339,14 +355,21 @@ export async function runDroid(promptPath: string, options: DroidOptions) {
   // a bounded tail of stderr as an error-message fallback.
   const STDERR_TAIL_LIMIT = 1500;
   let stderrTail = "";
+  // In stream-json mode a fatal agent-loop error (402 usage limit, 403 model
+  // policy, provider outage) is reported as
+  // `{type:"error",source:"agent_loop",message}` before the process exits
+  // non-zero; there is no result event carrying it.
+  let lastAgentLoopError: string | null = null;
   // Indirection defeats TS control-flow narrowing: the variables are only
   // assigned inside stream handler closures, so direct reads after the
   // retry loop would otherwise be narrowed to their initial values.
   const getLastResultEvent = (): ResultEvent | null => lastResultEvent;
   const getStderrTail = (): string => stderrTail;
+  const getLastAgentLoopError = (): string | null => lastAgentLoopError;
 
   const runDroidOnce = (): Promise<number> => {
     stderrTail = "";
+    lastAgentLoopError = null;
     const turns = createTurnCounter();
     let turnLimitHit = false;
     const droidProcess = spawn(droidExecutable, currentDroidArgs, {
@@ -417,6 +440,16 @@ export async function runDroid(promptPath: string, options: DroidOptions) {
               typeof parsed.result === "string" ? parsed.result : undefined,
           };
         }
+        if (
+          typeof parsed === "object" &&
+          parsed !== null &&
+          parsed.type === "error" &&
+          parsed.source === "agent_loop" &&
+          typeof parsed.message === "string" &&
+          parsed.message.trim()
+        ) {
+          lastAgentLoopError = parsed.message;
+        }
         if (
           maxTurns !== null &&
           !turnLimitHit &&
@@ -478,6 +511,8 @@ export async function runDroid(promptPath: string, options: DroidOptions) {
 
   let turnCapError: MaxTurnsExceededError | null = null;
   const getTurnCapError = (): MaxTurnsExceededError | null => turnCapError;
+  let usageLimitError: UsageLimitError | null = null;
+  const getUsageLimitError = (): UsageLimitError | null => usageLimitError;
 
   try {
     await retryWithBackoff(
@@ -493,14 +528,22 @@ export async function runDroid(promptPath: string, options: DroidOptions) {
         }
         if (lastExitCode !== 0) {
           console.log(`Droid Exec exited with code ${lastExitCode}`);
+          const agentLoopError = getLastAgentLoopError();
+          if (isUsageLimitError(agentLoopError)) {
+            usageLimitError = new UsageLimitError(
+              describeUsageLimitError(agentLoopError!),
+            );
+            throw usageLimitError;
+          }
           // If the failure was caused by the requested model being rejected
           // (blocked by the org's model policy, or not a recognized model
           // id), retry without --model so droid exec falls back to the
           // organization's default model.
           const resultEvent = getLastResultEvent();
           const policyBlocked =
-            resultEvent?.is_error === true &&
-            isModelPolicyError(resultEvent.result);
+            (resultEvent?.is_error === true &&
+              isModelPolicyError(resultEvent.result)) ||
+            isModelPolicyError(agentLoopError);
           const invalidModel = isInvalidModelError(getStderrTail());
           if (
             !modelArgsStripped &&
@@ -533,32 +576,43 @@ export async function runDroid(promptPath: string, options: DroidOptions) {
         initialDelayMs: 5000,
         maxDelayMs: 20000,
         // A runaway agent is not a transient failure; re-running it would
-        // only repeat the loop.
-        shouldRetry: (error) => !(error instanceof MaxTurnsExceededError),
+        // only repeat the loop. A usage limit will not reset within the
+        // backoff window either.
+        shouldRetry: (error) =>
+          !(
+            error instanceof MaxTurnsExceededError ||
+            error instanceof UsageLimitError
+          ),
       },
     );
     core.setOutput("conclusion", "success");
     return;
   } catch (_) {
     const capError = getTurnCapError();
-    if (!capError) {
+    const limitError = getUsageLimitError();
+    if (!capError && !limitError) {
       // All retry attempts exhausted
       console.error(
         `Droid Exec failed after 3 total attempts (exit code: ${lastExitCode})`,
       );
     }
     const finalResultEvent = getLastResultEvent();
+    const finalAgentLoopError = getLastAgentLoopError()?.trim();
     let finalStderrTail = getStderrTail().trim();
     if (isInvalidModelError(finalStderrTail)) {
       finalStderrTail = condenseInvalidModelError(finalStderrTail);
     }
     const rawErrorMessage = capError
       ? capError.message
-      : finalResultEvent?.is_error && finalResultEvent.result?.trim()
-        ? finalResultEvent.result.trim()
-        : finalStderrTail
-          ? `Droid Exec exited with code ${lastExitCode}:\n${finalStderrTail}`
-          : `Droid Exec exited with code ${lastExitCode}`;
+      : limitError
+        ? limitError.message
+        : finalResultEvent?.is_error && finalResultEvent.result?.trim()
+          ? finalResultEvent.result.trim()
+          : finalAgentLoopError
+            ? finalAgentLoopError
+            : finalStderrTail
+              ? `Droid Exec exited with code ${lastExitCode}:\n${finalStderrTail}`
+              : `Droid Exec exited with code ${lastExitCode}`;
     const errorMessage =
       rawErrorMessage.length > 2000
         ? `${rawErrorMessage.slice(0, 2000)}…`

base-action/src/utils/usage-limit-error.ts

@@ -0,0 +1,57 @@
+/**
+ * Matches the 402 Payment Required failures droid exec surfaces when the
+ * organization has hit a usage limit (5-hour / weekly / monthly standard or
+ * Droid Core limit, exhausted Extra Usage, credit limit) or, on BYOK models,
+ * when the upstream provider reports an empty balance.
+ *
+ * In stream-json mode the CLI reports the raw error as
+ * `{"type":"error","source":"agent_loop","message":"402 {...}"}` where the
+ * JSON body carries a human-readable `detail`. The copy-based patterns are a
+ * fallback for wrappers that drop the status prefix.
+ */
+const USAGE_LIMIT_COPY_PATTERNS = [
+  /reached your [^\n"]*usage limit/i,
+  /credit limit reached/i,
+  /extra usage balance is empty/i,
+  /model budget is exhausted/i,
+];
+
+export function isUsageLimitError(text: string | undefined | null): boolean {
+  if (!text) {
+    return false;
+  }
+  if (/^(?:Error:\s*)?402\b/.test(text.trim())) {
+    return true;
+  }
+  if (/"status"\s*:\s*402\b/.test(text)) {
+    return true;
+  }
+  return USAGE_LIMIT_COPY_PATTERNS.some((pattern) => pattern.test(text));
+}
+
+/**
+ * Pull the human-readable `detail` out of a `402 {"detail":"...",...}` error
+ * message so the PR comment shows "You've reached your weekly ... usage
+ * limit" instead of a JSON blob. Falls back to the raw text.
+ */
+export function describeUsageLimitError(text: string): string {
+  const jsonStart = text.indexOf("{");
+  if (jsonStart !== -1) {
+    try {
+      const body: unknown = JSON.parse(text.slice(jsonStart));
+      if (
+        typeof body === "object" &&
+        body !== null &&
+        typeof (body as { detail?: unknown }).detail === "string"
+      ) {
+        const detail = (body as { detail: string }).detail.trim();
+        if (detail) {
+          return detail;
+        }
+      }
+    } catch {
+      // Not a JSON body; fall through to the raw text.
+    }
+  }
+  return text.trim();
+}

base-action/test/run-droid-mcp.test.ts

@@ -1,6 +1,7 @@
 #!/usr/bin/env bun
 
 import {
+  afterAll,
   afterEach,
   beforeAll,
   beforeEach,
@@ -9,12 +10,18 @@ import {
   mock,
   test,
 } from "bun:test";
+import * as childProcess from "child_process";
 import { EventEmitter } from "node:events";
 import { mkdtemp, rm, writeFile } from "node:fs/promises";
 import { tmpdir } from "node:os";
 import path from "node:path";
 import type { DroidOptions } from "../src/run-droid";
 
+// mock.module below rewrites the live `child_process` bindings for the rest
+// of the bun process, including this namespace import, so copy the real
+// implementation out first and hand it back once this file is done.
+const realChildProcess = { ...childProcess };
+
 const originalRunnerTemp = process.env.RUNNER_TEMP;
 
 if (!process.env.RUNNER_TEMP) {
@@ -109,6 +116,10 @@ beforeAll(async () => {
   runDroid = module.runDroid;
 });
 
+afterAll(() => {
+  mock.module("child_process", () => realChildProcess);
+});
+
 async function createPromptFile(): Promise<string> {
   const tmpDir = await mkdtemp(path.join(tmpdir(), "run-droid-test-"));
   const promptPath = path.join(tmpDir, "prompt.txt");

base-action/test/run-droid-usage-limit.test.ts

@@ -0,0 +1,91 @@
+import { afterEach, beforeEach, describe, expect, spyOn, test } from "bun:test";
+import * as core from "@actions/core";
+import { chmod, mkdtemp, readFile, rm, writeFile } from "fs/promises";
+import { tmpdir } from "os";
+import { join } from "path";
+import { runDroid } from "../src/run-droid";
+
+/**
+ * Stand-in for `droid exec --output-format stream-json` on an organization
+ * whose weekly usage limit is exhausted: the agent loop reports the raw 402,
+ * the CLI reports the (empty) final text, and the process exits 1. Records
+ * every invocation so the test can prove it ran exactly once.
+ */
+const USAGE_LIMITED_DROID = `#!/usr/bin/env bash
+echo run >> "$DROID_FAKE_INVOCATIONS"
+echo '{"type":"system","subtype":"init","cwd":"/","session_id":"s","tools":[],"model":"m","reasoning_effort":"medium"}'
+echo '{"type":"message","role":"user","id":"u1","text":"review this PR","timestamp":1,"session_id":"s"}'
+echo '{"type":"error","source":"agent_loop","message":"402 {\\"detail\\":\\"You'"'"'ve reached your weekly Droid Core usage limit (resets in 5 days).\\\\nReload Extra Usage credits or wait for your limits to reset.\\",\\"status\\":402,\\"displayToUser\\":true}","timestamp":2,"session_id":"s"}'
+echo '{"type":"error","source":"cli","message":"Exec failed","timestamp":3,"session_id":"s"}'
+exit 1
+`;
+
+describe("runDroid usage limit", () => {
+  let dir: string;
+  let fakeDroid: string;
+  let promptPath: string;
+  let invocationsPath: string;
+  let outputs: Record<string, string>;
+  let setOutputSpy: ReturnType<typeof spyOn>;
+  let exitSpy: ReturnType<typeof spyOn>;
+  const originalInvocations = process.env.DROID_FAKE_INVOCATIONS;
+
+  beforeEach(async () => {
+    dir = await mkdtemp(join(tmpdir(), "droid-usage-limit-"));
+    fakeDroid = join(dir, "droid");
+    promptPath = join(dir, "prompt.txt");
+    invocationsPath = join(dir, "invocations.log");
+    await writeFile(fakeDroid, USAGE_LIMITED_DROID);
+    await chmod(fakeDroid, 0o755);
+    await writeFile(promptPath, "review this PR");
+    process.env.DROID_FAKE_INVOCATIONS = invocationsPath;
+    outputs = {};
+    setOutputSpy = spyOn(core, "setOutput").mockImplementation(
+      (name: string, value: unknown) => {
+        outputs[name] = String(value);
+      },
+    );
+  });
+
+  afterEach(async () => {
+    setOutputSpy.mockRestore();
+    exitSpy?.mockRestore();
+    if (originalInvocations === undefined) {
+      delete process.env.DROID_FAKE_INVOCATIONS;
+    } else {
+      process.env.DROID_FAKE_INVOCATIONS = originalInvocations;
+    }
+    await rm(dir, { recursive: true, force: true });
+  });
+
+  test("fails once with the limit detail instead of retrying", async () => {
+    let exitCode: number | undefined;
+    exitSpy = spyOn(process, "exit").mockImplementation(((code?: number) => {
+      exitCode = code;
+      throw new Error(`process.exit(${code})`);
+    }) as never);
+
+    const started = Date.now();
+    await expect(
+      runDroid(promptPath, {
+        pathToDroidExecutable: fakeDroid,
+        showFullOutput: "false",
+      }),
+    ).rejects.toThrow("process.exit(1)");
+
+    expect(exitCode).toBe(1);
+    expect(outputs.conclusion).toBe("failure");
+    expect(outputs.error_message).toContain("usage limit was reached");
+    expect(outputs.error_message).toContain(
+      "You've reached your weekly Droid Core usage limit (resets in 5 days).",
+    );
+    // The raw JSON body must not leak into the PR comment.
+    expect(outputs.error_message).not.toContain('"status"');
+
+    const invocations = await readFile(invocationsPath, "utf8");
+    expect(invocations.trim().split("\n")).toHaveLength(1);
+    // A retry would have waited for the 5s backoff before re-spawning; the
+    // usage-limit guard must short-circuit that.
+    expect(Date.now() - started).toBeLessThan(4000);
+  }, 15000);
+});