fix: Keep shutdown cleanup diagnostic-only

Record embedded housekeeping cleanup errors as shutdown diagnostics instead
of failed shutdown steps. Keep diagnostic-only cleanup telemetry at info
severity so best-effort cleanup does not look like an operational failure.

Co-Authored-By: OpenAI Codex <noreply@openai.com>

Author: cameroncooke

src/server/__tests__/mcp-shutdown.test.ts

@@ -113,7 +113,7 @@ describe('runMcpShutdown', () => {
     expect(mocks.stopAllTrackedProcesses).toHaveBeenCalledTimes(1);
   });
 
-  it('marks workspace filesystem cleanup as failed when embedded errors are reported', async () => {
+  it('records workspace filesystem cleanup diagnostics without failing the step', async () => {
     mocks.cleanupOwnedWorkspaceFilesystemArtifacts.mockResolvedValueOnce({
       workspaceKey: 'workspace-a',
       trigger: 'shutdown',
@@ -135,10 +135,12 @@ describe('runMcpShutdown', () => {
     const filesystemStep = result.steps.find(
       (step) => step.name === 'workspace-filesystem.cleanup-owned',
     );
-    expect(filesystemStep?.status).toBe('failed');
+    expect(filesystemStep?.status).toBe('completed');
+    expect(filesystemStep?.diagnosticCount).toBe(1);
+    expect(filesystemStep?.diagnostics).toEqual(['could not delete stale oslog file']);
   });
 
-  it('marks video capture cleanup as failed when embedded errors are reported', async () => {
+  it('records video capture cleanup diagnostics without failing the step', async () => {
     mocks.stopAllVideoCaptureSessions.mockResolvedValueOnce({
       stoppedSessionCount: 0,
       errorCount: 1,
@@ -152,10 +154,12 @@ describe('runMcpShutdown', () => {
     });
 
     const videoStep = result.steps.find((step) => step.name === 'video-capture.stop-all');
-    expect(videoStep?.status).toBe('failed');
+    expect(videoStep?.status).toBe('completed');
+    expect(videoStep?.diagnosticCount).toBe(1);
+    expect(videoStep?.diagnostics).toEqual(['failed to stop recorder']);
   });
 
-  it('marks video capture cleanup as failed when errorCount is zero but errors are reported', async () => {
+  it('records video capture diagnostics when errorCount is zero but errors are reported', async () => {
     mocks.stopAllVideoCaptureSessions.mockResolvedValueOnce({
       stoppedSessionCount: 0,
       errorCount: 0,
@@ -169,10 +173,12 @@ describe('runMcpShutdown', () => {
     });
 
     const videoStep = result.steps.find((step) => step.name === 'video-capture.stop-all');
-    expect(videoStep?.status).toBe('failed');
+    expect(videoStep?.status).toBe('completed');
+    expect(videoStep?.diagnosticCount).toBe(1);
+    expect(videoStep?.diagnostics).toEqual(['failed to stop recorder']);
   });
 
-  it('marks swift tracked process cleanup as failed when embedded errors are reported', async () => {
+  it('records swift tracked process cleanup diagnostics without failing the step', async () => {
     mocks.stopAllTrackedProcesses.mockResolvedValueOnce({
       stoppedProcessCount: 0,
       errorCount: 1,
@@ -186,10 +192,12 @@ describe('runMcpShutdown', () => {
     });
 
     const swiftStep = result.steps.find((step) => step.name === 'swift-processes.stop-all');
-    expect(swiftStep?.status).toBe('failed');
+    expect(swiftStep?.status).toBe('completed');
+    expect(swiftStep?.diagnosticCount).toBe(1);
+    expect(swiftStep?.diagnostics).toEqual(['failed to terminate swift process']);
   });
 
-  it('marks swift tracked process cleanup as failed when errorCount is zero but errors are reported', async () => {
+  it('records swift tracked process diagnostics when errorCount is zero but errors are reported', async () => {
     mocks.stopAllTrackedProcesses.mockResolvedValueOnce({
       stoppedProcessCount: 0,
       errorCount: 0,
@@ -203,7 +211,9 @@ describe('runMcpShutdown', () => {
     });
 
     const swiftStep = result.steps.find((step) => step.name === 'swift-processes.stop-all');
-    expect(swiftStep?.status).toBe('failed');
+    expect(swiftStep?.status).toBe('completed');
+    expect(swiftStep?.diagnosticCount).toBe(1);
+    expect(swiftStep?.diagnostics).toEqual(['failed to terminate swift process']);
   });
 
   it('adds outer timeout headroom for one-item bulk cleanup', async () => {

src/server/mcp-shutdown.ts

@@ -30,6 +30,8 @@ export interface ShutdownStepResult {
   status: ShutdownStepStatus;
   durationMs: number;
   error?: string;
+  diagnosticCount?: number;
+  diagnostics?: string[];
 }
 
 interface ShutdownStepOutcome<T> {
@@ -112,12 +114,22 @@ function buildExitCode(reason: McpShutdownReason): number {
   return FAILURE_REASONS.has(reason) ? 1 : 0;
 }
 
-function throwIfErrors(name: string, errors: string[], errorCount?: number): void {
-  const effectiveCount = Math.max(errorCount ?? 0, errors.length);
-  if (effectiveCount > 0) {
-    const detail = errors.length > 0 ? errors.join('; ') : 'no error details provided';
-    throw new Error(`${name} reported ${effectiveCount} error(s): ${detail}`);
+function getCleanupDiagnostics(value: unknown): { count: number; messages: string[] } | null {
+  if (value === null || typeof value !== 'object') {
+    return null;
   }
+
+  const result = value as { errorCount?: unknown; errors?: unknown };
+  const messages = Array.isArray(result.errors)
+    ? result.errors.filter((error): error is string => typeof error === 'string')
+    : [];
+  const explicitCount =
+    typeof result.errorCount === 'number' && Number.isFinite(result.errorCount)
+      ? result.errorCount
+      : 0;
+  const count = Math.max(explicitCount, messages.length);
+
+  return count > 0 ? { count, messages } : null;
 }
 
 function workspaceFilesystemCleanupTimeoutForOwnedSessions(ownedSessionCount: number): number {
@@ -162,6 +174,15 @@ export async function runMcpShutdown(input: {
     if (outcome.error) {
       step.error = outcome.error;
     }
+    if (outcome.status === 'completed') {
+      const diagnostics = getCleanupDiagnostics(outcome.value);
+      if (diagnostics) {
+        step.diagnosticCount = diagnostics.count;
+        if (diagnostics.messages.length > 0) {
+          step.diagnostics = diagnostics.messages;
+        }
+      }
+    }
     steps.push(step);
   };
 
@@ -210,29 +231,23 @@ export async function runMcpShutdown(input: {
       name: 'workspace-filesystem.cleanup-owned',
       timeoutMs: workspaceFilesystemCleanupTimeoutMs,
       operation: async (): Promise<unknown> => {
-        const result = await cleanupOwnedWorkspaceFilesystemArtifacts({
+        return cleanupOwnedWorkspaceFilesystemArtifacts({
           timeoutMs: STEP_TIMEOUT_MS,
         });
-        throwIfErrors('workspace-filesystem.cleanup-owned', result.errors);
-        return result;
       },
     },
     {
       name: 'video-capture.stop-all',
       timeoutMs: bulkStepTimeoutMs(input.snapshot.videoCaptureSessionCount),
       operation: async (): Promise<unknown> => {
-        const result = await stopAllVideoCaptureSessions(STEP_TIMEOUT_MS);
-        throwIfErrors('video-capture.stop-all', result.errors, result.errorCount);
-        return result;
+        return stopAllVideoCaptureSessions(STEP_TIMEOUT_MS);
       },
     },
     {
       name: 'swift-processes.stop-all',
       timeoutMs: bulkStepTimeoutMs(input.snapshot.swiftPackageProcessCount),
       operation: async (): Promise<unknown> => {
-        const result = await stopAllTrackedProcesses(STEP_TIMEOUT_MS);
-        throwIfErrors('swift-processes.stop-all', result.errors, result.errorCount);
-        return result;
+        return stopAllTrackedProcesses(STEP_TIMEOUT_MS);
       },
     },
   ];
@@ -243,17 +258,22 @@ export async function runMcpShutdown(input: {
   }
 
   const triggerError = input.error === undefined ? undefined : toErrorMessage(input.error);
-  const cleanupFailureCount = steps.filter(
+  const shutdownStepFailureCount = steps.filter(
     (step) => step.status === 'failed' || step.status === 'timed_out',
   ).length;
+  const cleanupDiagnosticCount = steps.reduce(
+    (total, step) => total + (step.diagnosticCount ?? 0),
+    0,
+  );
 
   captureMcpShutdownSummary({
     reason: input.reason,
     phase: input.snapshot.phase,
     exitCode,
     transportDisconnected,
     triggerError,
-    cleanupFailureCount,
+    shutdownStepFailureCount,
+    cleanupDiagnosticCount,
     shutdownDurationMs: Date.now() - shutdownStartedAt,
     snapshot: input.snapshot as unknown as Record<string, unknown>,
     steps: steps as unknown as Array<Record<string, unknown>>,

src/utils/sentry.ts

@@ -376,7 +376,8 @@ export interface McpShutdownSummaryEvent {
   exitCode: number;
   transportDisconnected: boolean;
   triggerError?: string;
-  cleanupFailureCount: number;
+  shutdownStepFailureCount: number;
+  cleanupDiagnosticCount?: number;
   shutdownDurationMs: number;
   snapshot: Record<string, unknown>;
   steps: Array<Record<string, unknown>>;
@@ -415,7 +416,7 @@ export function captureMcpShutdownSummary(summary: McpShutdownSummaryEvent): voi
     let level: 'error' | 'warning' | 'info';
     if (isCrashReason) {
       level = 'error';
-    } else if (summary.cleanupFailureCount > 0 || localAnomalyCount > 0) {
+    } else if (summary.shutdownStepFailureCount > 0 || localAnomalyCount > 0) {
       level = 'warning';
     } else {
       level = 'info';
@@ -433,7 +434,8 @@ export function captureMcpShutdownSummary(summary: McpShutdownSummaryEvent): voi
         exitCode: summary.exitCode,
         transportDisconnected: summary.transportDisconnected,
         triggerError: summary.triggerError,
-        cleanupFailureCount: summary.cleanupFailureCount,
+        shutdownStepFailureCount: summary.shutdownStepFailureCount,
+        cleanupDiagnosticCount: summary.cleanupDiagnosticCount,
         shutdownDurationMs: summary.shutdownDurationMs,
         snapshot: summary.snapshot,
         steps: summary.steps,