Feat(code review) keep and map error codes

apps/web/src/app/api/internal/code-review-status/[reviewId]/route.test.ts

@@ -1308,6 +1308,86 @@ describe('POST /api/internal/code-review-status/[reviewId]', () => {
         expect.objectContaining({ terminalReason: undefined })
       );
     });
+
+    it('derives the terminal reason from the structured failure payload', async () => {
+      mockGetCodeReviewById.mockResolvedValue(makeReview());
+
+      await POST(
+        makeRequest({
+          cloudAgentSessionId: 'agent_1',
+          status: 'failed',
+          errorMessage: 'Agent wrapper failed while processing the message',
+          failure: { stage: 'agent_activity', code: 'wrapper_error_after_activity' },
+        }),
+        makeParams(REVIEW_ID)
+      );
+
+      expect(mockUpdateCodeReviewAttemptForCallback).toHaveBeenCalledWith(
+        expect.objectContaining({ status: 'failed', terminalReason: 'wrapper_failed' })
+      );
+    });
+
+    it('keeps rate limiting distinct from generic assistant failures', async () => {
+      mockGetCodeReviewById.mockResolvedValue(makeReview());
+
+      await POST(
+        makeRequest({
+          cloudAgentSessionId: 'agent_1',
+          status: 'failed',
+          errorMessage: 'Assistant request was rate limited',
+          failure: { code: 'assistant_error', message: 'Assistant request was rate limited' },
+        }),
+        makeParams(REVIEW_ID)
+      );
+
+      expect(mockUpdateCodeReviewAttemptForCallback).toHaveBeenCalledWith(
+        expect.objectContaining({ status: 'failed', terminalReason: 'assistant_rate_limited' })
+      );
+    });
+
+    // model_missing arrives with the generic message 'No model was selected',
+    // which the message-based model-not-found check cannot match. Without the
+    // structured path re-applying normalization the row would stay 'failed' and
+    // the customer would get generic copy instead of the actionable message.
+    it('normalizes status to cancelled for a structured model_missing failure', async () => {
+      mockGetCodeReviewById.mockResolvedValue(makeReview());
+
+      await POST(
+        makeRequest({
+          cloudAgentSessionId: 'agent_1',
+          status: 'failed',
+          errorMessage: 'No model was selected',
+          failure: { stage: 'pre_dispatch', code: 'model_missing' },
+        }),
+        makeParams(REVIEW_ID)
+      );
+
+      expect(mockUpdateCodeReviewAttemptForCallback).toHaveBeenCalledWith(
+        expect.objectContaining({ status: 'cancelled', terminalReason: 'model_not_found' })
+      );
+    });
+
+    // errorMessage is deliberately omitted. The real payment_required safe
+    // message contains 'insufficient credits', which the earlier text-based
+    // billing heuristic matches, so including it would set terminalReason before
+    // the structured branch runs and the assertion would pass even if the
+    // payment_required mapping were deleted.
+    it('normalizes status to failed for a structured payment_required failure', async () => {
+      mockGetCodeReviewById.mockResolvedValue(makeReview());
+
+      await POST(
+        makeRequest({
+          cloudAgentSessionId: 'agent_1',
+          status: 'interrupted',
+          failure: { code: 'payment_required' },
+        }),
+        makeParams(REVIEW_ID)
+      );
+
+      expect(mockUpdateCodeReviewAttemptForCallback).toHaveBeenCalledWith(
+        expect.objectContaining({ status: 'failed', terminalReason: 'billing' })
+      );
+    });
   });
 
   describe('attempt tracking and infra retry', () => {

apps/web/src/app/api/internal/code-review-status/[reviewId]/route.ts

@@ -61,6 +61,7 @@ import { CALLBACK_TOKEN_SECRET } from '@/lib/config.server';
 import { verifyCallbackToken } from '@kilocode/worker-utils/callback-token';
 import { PLATFORM } from '@/lib/integrations/core/constants';
 import { appendPreviousReviewSummaryHistory } from '@/lib/code-reviews/summary/history';
+import { terminalReasonFromCloudAgentFailure } from '@/lib/code-reviews/terminal-reason-from-failure';
 import {
   appendReviewSummaryFooter,
   buildReviewSummaryFooter,
@@ -369,6 +370,33 @@ function normalizePayload(raw: StatusUpdatePayload): {
     terminalReason = 'model_not_found';
   }
 
+  // Consume the structured failure cloud-agent-next already sends. This runs
+  // after the specific inferences above so they keep priority, and before the
+  // generic 'interrupted' fallback so an interrupt with a known cause is still
+  // attributed to that cause.
+  if (!terminalReason) {
+    const structuredReason = terminalReasonFromCloudAgentFailure(failure, raw.errorMessage);
+    if (structuredReason) {
+      terminalReason = structuredReason;
+      // The billing and model-not-found blocks above own status normalization
+      // for those reasons, but both key off the error text. A structured-only
+      // payload carries a generic message ('No model was selected' for
+      // model_missing), so those checks miss and the status would stay
+      // unnormalized while terminal_reason is correct — leaving the customer
+      // with generic failure copy from mapStatusToCheckRun instead of the
+      // actionable message. Apply the same normalization here.
+      if (structuredReason === 'billing' && status === 'cancelled') {
+        status = 'failed'; // billing is not a user cancellation
+      }
+      if (
+        structuredReason === 'model_not_found' &&
+        (raw.status === 'failed' || raw.status === 'interrupted')
+      ) {
+        status = 'cancelled';
+      }
+    }
+  }
+
   if (!terminalReason && raw.status === 'interrupted') {
     terminalReason = 'interrupted';
   }

apps/web/src/lib/code-reviews/terminal-reason-from-failure.test.ts

@@ -0,0 +1,131 @@
+import {
+  CLOUD_AGENT_FAILURE_CODES,
+  WORKSPACE_FAILURE_SUBTYPES,
+} from '@kilocode/worker-utils/cloud-agent-failure';
+import { CLOUD_AGENT_TERMINAL_REASONS } from '@kilocode/worker-utils/cloud-agent-next-client';
+import { CODE_REVIEW_TERMINAL_REASONS } from '@kilocode/db/schema-types';
+import { terminalReasonFromCloudAgentFailure } from './terminal-reason-from-failure';
+
+describe('terminal reason list parity', () => {
+  // These two lists live in separate packages that cannot import each other, so
+  // drift is only caught here. A mismatch makes the orchestrator send a reason
+  // the callback allowlist rejects, or makes the worker's zod enum silently
+  // coerce a valid reason to undefined via its `.catch(undefined)`.
+  it('keeps the db and worker-utils terminal reason lists identical', () => {
+    expect([...CLOUD_AGENT_TERMINAL_REASONS].sort()).toEqual(
+      [...CODE_REVIEW_TERMINAL_REASONS].sort()
+    );
+  });
+});
+
+describe('terminalReasonFromCloudAgentFailure', () => {
+  it('returns undefined without a structured failure', () => {
+    expect(terminalReasonFromCloudAgentFailure(undefined)).toBeUndefined();
+    expect(terminalReasonFromCloudAgentFailure({})).toBeUndefined();
+  });
+
+  it('prefers the workspace subtype over the generic code', () => {
+    expect(
+      terminalReasonFromCloudAgentFailure({
+        code: 'workspace_setup_failed',
+        subtype: 'sandbox_storage_full',
+      })
+    ).toBe('workspace_capacity');
+
+    expect(
+      terminalReasonFromCloudAgentFailure({
+        code: 'workspace_setup_failed',
+        subtype: 'git_authentication_failed',
+      })
+    ).toBe('repository_auth_failed');
+  });
+
+  it('falls back to the generic workspace reason without a subtype', () => {
+    expect(terminalReasonFromCloudAgentFailure({ code: 'workspace_setup_failed' })).toBe(
+      'workspace_setup_failed'
+    );
+  });
+
+  it('maps the codes behind the largest uncategorized buckets', () => {
+    expect(terminalReasonFromCloudAgentFailure({ code: 'assistant_error' })).toBe(
+      'assistant_failed'
+    );
+    expect(terminalReasonFromCloudAgentFailure({ code: 'wrapper_error_after_activity' })).toBe(
+      'wrapper_failed'
+    );
+  });
+
+  it('splits assistant failures by their safe message', () => {
+    expect(
+      terminalReasonFromCloudAgentFailure({
+        code: 'assistant_error',
+        message: 'Assistant request was rate limited',
+      })
+    ).toBe('assistant_rate_limited');
+
+    expect(
+      terminalReasonFromCloudAgentFailure({
+        code: 'assistant_error',
+        message: 'Assistant service is unavailable',
+      })
+    ).toBe('assistant_unavailable');
+  });
+
+  it('reads the assistant message from the callback errorMessage when absent on the failure', () => {
+    expect(
+      terminalReasonFromCloudAgentFailure(
+        { code: 'assistant_error' },
+        'Assistant request was rate limited'
+      )
+    ).toBe('assistant_rate_limited');
+  });
+
+  it('degrades to the generic assistant reason on an unrecognized message', () => {
+    expect(
+      terminalReasonFromCloudAgentFailure({
+        code: 'assistant_error',
+        message: 'Assistant request failed in some new way',
+      })
+    ).toBe('assistant_failed');
+  });
+
+  it('leaves unclassified failures to the callers message-based inference', () => {
+    expect(terminalReasonFromCloudAgentFailure({ code: 'unclassified' })).toBeUndefined();
+  });
+
+  it.each(['constructor', '__proto__', 'toString', 'valueOf'])(
+    'does not resolve inherited object members for message %p',
+    message => {
+      // A plain object lookup would return a truthy Object.prototype member and
+      // write a function reference into the terminal_reason column.
+      expect(terminalReasonFromCloudAgentFailure({ code: 'assistant_error', message })).toBe(
+        'assistant_failed'
+      );
+    }
+  );
+
+  it('resolves every failure code to a valid terminal reason', () => {
+    const valid = new Set<string>(CODE_REVIEW_TERMINAL_REASONS);
+    const resolved = CLOUD_AGENT_FAILURE_CODES.map(code => [
+      code,
+      terminalReasonFromCloudAgentFailure({ code }),
+    ]);
+
+    // Reported as a table so a future code addition names itself in the failure.
+    expect(
+      resolved.filter(([code, reason]) =>
+        code === 'unclassified' ? reason !== undefined : !valid.has(reason as string)
+      )
+    ).toEqual([]);
+  });
+
+  it('resolves every workspace subtype to a valid terminal reason', () => {
+    const valid = new Set<string>(CODE_REVIEW_TERMINAL_REASONS);
+    const resolved = WORKSPACE_FAILURE_SUBTYPES.map(subtype => [
+      subtype,
+      terminalReasonFromCloudAgentFailure({ code: 'workspace_setup_failed', subtype }),
+    ]);
+
+    expect(resolved.filter(([, reason]) => !valid.has(reason as string))).toEqual([]);
+  });
+});

apps/web/src/lib/code-reviews/terminal-reason-from-failure.ts

@@ -0,0 +1,127 @@
+/**
+ * Maps the structured failure cloud-agent-next already sends on the code review
+ * status callback onto a `CodeReviewTerminalReason`.
+ *
+ * cloud-agent-next classifies every terminal failure into a
+ * `CloudAgentSafeFailure` ({ stage, code, subtype, attempts, message }) and puts
+ * it on the callback payload. Historically the receiver read `failure.code` for
+ * exactly one case (workspace_setup_failed + sandbox_storage_full) and ignored
+ * the rest, so ~26% of failures were stored with a NULL terminal_reason and a
+ * human-readable sentence in error_message. The admin error analysis then tried
+ * to recover the category by pattern-matching that sentence, and everything it
+ * missed collapsed into "Other".
+ *
+ * This module consumes the structured value instead, so classification no longer
+ * round-trips through English.
+ */
+
+import type {
+  CloudAgentFailureCode,
+  CloudAgentSafeFailure,
+  WorkspaceFailureSubtype,
+} from '@kilocode/worker-utils/cloud-agent-failure';
+import type { CodeReviewTerminalReason } from '@kilocode/db/schema-types';
+
+/**
+ * Workspace failures carry a subtype that is strictly more specific than the
+ * `workspace_setup_failed` code, so it is consulted first.
+ */
+const WORKSPACE_SUBTYPE_REASONS = {
+  sandbox_storage_full: 'workspace_capacity',
+  git_clone_timeout: 'repository_clone_failed',
+  git_checkout_timeout: 'repository_clone_failed',
+  git_network_failed: 'repository_clone_failed',
+  git_authentication_failed: 'repository_auth_failed',
+  git_pack_corrupt: 'repository_checkout_failed',
+  git_checkout_conflict: 'repository_checkout_failed',
+  git_branch_missing: 'repository_checkout_failed',
+  kilo_import_timeout: 'session_import_failed',
+  kilo_import_failed: 'session_import_failed',
+  setup_command_timeout: 'setup_command_failed',
+  setup_command_failed: 'setup_command_failed',
+  workspace_setup_unknown: 'workspace_setup_failed',
+} as const satisfies Record<WorkspaceFailureSubtype, CodeReviewTerminalReason>;
+
+/**
+ * Every `CloudAgentFailureCode` maps to a reason. `satisfies Record<...>` makes
+ * this exhaustive: adding a code upstream without deciding its category here is
+ * a type error rather than a silent slide into "Other".
+ */
+const FAILURE_CODE_REASONS = {
+  sandbox_connect_failed: 'sandbox_connection',
+  workspace_setup_failed: 'workspace_setup_failed',
+  kilo_server_failed: 'runtime_startup_failed',
+  wrapper_start_failed: 'runtime_startup_failed',
+  invalid_delivery_request: 'delivery_failed',
+  session_metadata_missing: 'delivery_failed',
+  delivery_failure_unknown: 'delivery_failed',
+  model_missing: 'model_not_found',
+  wrapper_disconnected: 'wrapper_failed',
+  wrapper_no_output: 'wrapper_failed',
+  wrapper_ping_timeout: 'wrapper_failed',
+  wrapper_error_before_activity: 'wrapper_failed',
+  wrapper_error_after_activity: 'wrapper_failed',
+  assistant_error: 'assistant_failed',
+  missing_assistant_reply: 'assistant_no_reply',
+  payment_required: 'billing',
+  user_interrupt: 'user_cancelled',
+  container_shutdown: 'container_shutdown',
+  system_interrupt: 'interrupted',
+  unclassified: 'unknown',
+} as const satisfies Record<CloudAgentFailureCode, CodeReviewTerminalReason>;
+
+/**
+ * `assistant_error` is a single code covering every provider-side failure, so on
+ * its own it would collapse rate limiting (by far the largest bucket) into one
+ * undifferentiated category. The finer reason lives only in the safe message.
+ *
+ * These are exact, whole-string matches against the constants produced by
+ * `classifyAssistantFailure` in
+ * services/cloud-agent-next/src/session/safe-failure-projection.ts — not
+ * substring heuristics. If a message there is reworded this map stops matching
+ * and callers fall back to the generic `assistant_failed`, which is no worse
+ * than the current behaviour.
+ *
+ * KEEP IN SYNC with classifyAssistantFailure's safeMessage values.
+ *
+ * A Map rather than an object literal: the key is caller-supplied free text (the
+ * schema bounds only its length), and an object lookup for 'constructor' or
+ * '__proto__' would resolve an inherited Object.prototype member. That value is
+ * truthy, so it would defeat the fallback and write a function reference into
+ * the terminal_reason column. Map keys have no prototype chain.
+ */
+const ASSISTANT_MESSAGE_REASONS = new Map<string, CodeReviewTerminalReason>([
+  ['Assistant request was rate limited', 'assistant_rate_limited'],
+  ['Assistant service is unavailable', 'assistant_unavailable'],
+  ['Assistant request timed out', 'assistant_timeout'],
+  ['Assistant request was not authorized', 'assistant_unauthorized'],
+  ['Assistant request was invalid', 'assistant_invalid_request'],
+]);
+
+/**
+ * Resolve a terminal reason from the structured callback failure.
+ *
+ * Returns undefined when there is no structured failure, or when the code is
+ * `unclassified` — in that case the caller's existing message-based inference
+ * still gets a chance rather than being pre-empted by a useless 'unknown'.
+ */
+export function terminalReasonFromCloudAgentFailure(
+  failure: CloudAgentSafeFailure | undefined,
+  errorMessage?: string
+): CodeReviewTerminalReason | undefined {
+  if (!failure?.code) return undefined;
+
+  if (failure.code === 'workspace_setup_failed' && failure.subtype) {
+    return WORKSPACE_SUBTYPE_REASONS[failure.subtype];
+  }
+
+  if (failure.code === 'assistant_error') {
+    // The projection puts the safe message on the failure; fall back to the
+    // callback's errorMessage for payloads that only carry the flattened text.
+    const message = (failure.message ?? errorMessage)?.trim();
+    return (message ? ASSISTANT_MESSAGE_REASONS.get(message) : undefined) ?? 'assistant_failed';
+  }
+
+  const reason = FAILURE_CODE_REASONS[failure.code];
+  return reason === 'unknown' ? undefined : reason;
+}