fix: fail when all providers fail during review

Author: 777genius

__tests__/unit/core/orchestrator.health.test.ts

@@ -198,4 +198,67 @@ describe('ReviewOrchestrator health check guard rails', () => {
     expect(review).toBeTruthy();
     expect(execute).toHaveBeenCalledWith([provider], expect.any(String), expect.any(Number));
   });
+
+  it('fails when every LLM provider fails and provider failure is configured as blocking', async () => {
+    const previousFailOnNoHealthy = process.env.FAIL_ON_NO_HEALTHY_PROVIDERS;
+    process.env.FAIL_ON_NO_HEALTHY_PROVIDERS = 'true';
+
+    const provider = {
+      name: 'codex/gpt-5.5',
+      review: jest.fn(),
+      healthCheck: jest.fn(),
+    } as unknown as Provider;
+    const execute = jest.fn().mockResolvedValue([
+      {
+        name: 'codex/gpt-5.5',
+        status: 'error',
+        error: new Error('Codex CLI failed: OPENAI_API_KEY=sk-1234567890abcdef refresh_token=secret-value'),
+        durationSeconds: 0,
+      } as ProviderResult,
+    ]);
+
+    try {
+      const orchestrator = makeOrchestrator({
+        config: {
+          ...DEFAULT_CONFIG,
+          dryRun: true,
+          enableCaching: false,
+          analyticsEnabled: false,
+          graphEnabled: false,
+          providers: ['codex/gpt-5.5'],
+          fallbackProviders: [],
+          providerLimit: 1,
+        },
+        providerRegistry: {
+          createProviders: jest.fn().mockResolvedValue([provider]),
+          discoverAdditionalFreeProviders: jest.fn().mockResolvedValue([]),
+        } as any,
+        llmExecutor: {
+          filterHealthyProviders: jest.fn().mockResolvedValue({
+            healthy: [provider],
+            healthCheckResults: [],
+          }),
+          execute,
+        } as any,
+      });
+
+      const pr = makePR([{ filename: 'a.ts', status: 'modified', additions: 1, deletions: 0, changes: 1 }]);
+
+      let thrown: Error | undefined;
+      try {
+        await orchestrator.executeReview(pr);
+      } catch (error) {
+        thrown = error as Error;
+      }
+
+      expect(thrown?.message).toMatch(/All LLM providers failed during review/);
+      expect(thrown?.message).not.toMatch(/sk-1234567890abcdef|secret-value/);
+    } finally {
+      if (previousFailOnNoHealthy === undefined) {
+        delete process.env.FAIL_ON_NO_HEALTHY_PROVIDERS;
+      } else {
+        process.env.FAIL_ON_NO_HEALTHY_PROVIDERS = previousFailOnNoHealthy;
+      }
+    }
+  });
 });

dist/index.js

@@ -26615,8 +26615,17 @@ var ReviewOrchestrator = class {
           if (batchFailures > 0) {
             if (batchSuccesses === 0) {
               const failedNames = mergedResults.filter((r) => r.status !== "success").map((r) => r.name).join(", ");
-              logger.error(`All LLM batches failed (${batchFailures}/${batches.length}): ${failedNames}. Continuing with static analysis only.`);
+              const providerFailureSummary = this.formatProviderFailureSummary(mergedResults);
+              const failOnProviderFailure = process.env.FAIL_ON_NO_HEALTHY_PROVIDERS === "true";
+              logger.error(
+                `All LLM batches failed (${batchFailures}/${batches.length}): ${failedNames}. ` + (failOnProviderFailure ? "Failing because FAIL_ON_NO_HEALTHY_PROVIDERS=true." : "Continuing with static analysis only.")
+              );
               await progressTracker?.updateProgress("llm", "failed", `All batches failed: ${failedNames}`);
+              if (failOnProviderFailure) {
+                throw new Error(
+                  `All LLM providers failed during review; failing because FAIL_ON_NO_HEALTHY_PROVIDERS=true. ${providerFailureSummary}`
+                );
+              }
             } else {
               logger.warn(`Partial batch failure: ${batchFailures} failed, ${batchSuccesses} succeeded. Using successful results.`);
               await progressTracker?.updateProgress("llm", "completed", `Batches: ${batchSuccesses}/${batches.length} succeeded`);
@@ -26869,6 +26878,20 @@ var ReviewOrchestrator = class {
       logger.warn("Failed to update PR description summary", error2);
     }
   }
+  formatProviderFailureSummary(results) {
+    const failures = results.filter((result) => result.status !== "success").map((result) => {
+      const reason = result.error?.message || result.status;
+      return `${result.name}: ${this.redactProviderFailureReason(reason)}`;
+    });
+    if (failures.length === 0) {
+      return "No provider error details were reported.";
+    }
+    const summary = failures.join("; ");
+    return summary.length > 1e3 ? `${summary.slice(0, 1e3)}...` : summary;
+  }
+  redactProviderFailureReason(reason) {
+    return reason.replace(/sk-[A-Za-z0-9_-]{16,}/g, "sk-***").replace(/gh[pousr]_[A-Za-z0-9_]{16,}/g, "gh*-***").replace(/github_pat_[A-Za-z0-9_]+/g, "github_pat_***").replace(/(refresh_token["'\s:=]+)[^"',\s}]+/gi, "$1***").replace(/(authorization:\s*bearer\s+)[^\s]+/gi, "$1***").replace(/(OPENAI_API_KEY["'\s:=]+)[^"',\s}]+/gi, "$1***").replace(/(OPENROUTER_API_KEY["'\s:=]+)[^"',\s}]+/gi, "$1***");
+  }
   /**
    * Run all static analysis operations in parallel
    */

dist/index.js.map

@@ -557,8 +557,20 @@ export class ReviewOrchestrator {
           if (batchFailures > 0) {
             if (batchSuccesses === 0) {
               const failedNames = mergedResults.filter(r => r.status !== 'success').map(r => r.name).join(', ');
-              logger.error(`All LLM batches failed (${batchFailures}/${batches.length}): ${failedNames}. Continuing with static analysis only.`);
+              const providerFailureSummary = this.formatProviderFailureSummary(mergedResults);
+              const failOnProviderFailure = process.env.FAIL_ON_NO_HEALTHY_PROVIDERS === 'true';
+              logger.error(
+                `All LLM batches failed (${batchFailures}/${batches.length}): ${failedNames}. ` +
+                (failOnProviderFailure
+                  ? 'Failing because FAIL_ON_NO_HEALTHY_PROVIDERS=true.'
+                  : 'Continuing with static analysis only.')
+              );
               await progressTracker?.updateProgress('llm', 'failed', `All batches failed: ${failedNames}`);
+              if (failOnProviderFailure) {
+                throw new Error(
+                  `All LLM providers failed during review; failing because FAIL_ON_NO_HEALTHY_PROVIDERS=true. ${providerFailureSummary}`
+                );
+              }
             } else {
               logger.warn(`Partial batch failure: ${batchFailures} failed, ${batchSuccesses} succeeded. Using successful results.`);
               await progressTracker?.updateProgress('llm', 'completed', `Batches: ${batchSuccesses}/${batches.length} succeeded`);
@@ -889,6 +901,33 @@ export class ReviewOrchestrator {
     }
   }
 
+  private formatProviderFailureSummary(results: ProviderResult[]): string {
+    const failures = results
+      .filter(result => result.status !== 'success')
+      .map(result => {
+        const reason = result.error?.message || result.status;
+        return `${result.name}: ${this.redactProviderFailureReason(reason)}`;
+      });
+
+    if (failures.length === 0) {
+      return 'No provider error details were reported.';
+    }
+
+    const summary = failures.join('; ');
+    return summary.length > 1000 ? `${summary.slice(0, 1000)}...` : summary;
+  }
+
+  private redactProviderFailureReason(reason: string): string {
+    return reason
+      .replace(/sk-[A-Za-z0-9_-]{16,}/g, 'sk-***')
+      .replace(/gh[pousr]_[A-Za-z0-9_]{16,}/g, 'gh*-***')
+      .replace(/github_pat_[A-Za-z0-9_]+/g, 'github_pat_***')
+      .replace(/(refresh_token["'\s:=]+)[^"',\s}]+/gi, '$1***')
+      .replace(/(authorization:\s*bearer\s+)[^\s]+/gi, '$1***')
+      .replace(/(OPENAI_API_KEY["'\s:=]+)[^"',\s}]+/gi, '$1***')
+      .replace(/(OPENROUTER_API_KEY["'\s:=]+)[^"',\s}]+/gi, '$1***');
+  }
+
   /**
    * Run all static analysis operations in parallel
    */