fix: address red-team review findings — cache aliasing, jitter cap, cause shadowing

- config-loader-facade: use structuredClone() to prevent cache aliasing
- retry-strategy: re-cap delay after jitter to enforce maxDelayMs boundary
- retry-strategy: wire retryAfter from RetryableError into delay computation
- retry-strategy: guard against negative maxRetries
- error-types: rename RetryableError.cause to originalError to avoid shadowing Error.cause
- Tests updated for all fixes

src/utils/__tests__/retry-strategy.test.ts

@@ -95,11 +95,11 @@ describe('withRetry', () => {
       maxDelayMs: 200,
     });
 
-    // Verify setTimeout was called with delay <= maxDelayMs (200ms) + jitter buffer
-    // Jitter adds 0-20% of delay, so max possible is 240ms
+    // Verify setTimeout was called with delay <= maxDelayMs (200ms)
+    // Jitter is capped, so delay must never exceed 200ms
     for (const call of sleepSpy.mock.calls) {
       const delay = call[1] as number;
-      expect(delay).toBeLessThanOrEqual(250); // allow small jitter overhead
+      expect(delay).toBeLessThanOrEqual(200);
     }
     sleepSpy.mockRestore();
   });
@@ -158,4 +158,30 @@ describe('withRetry', () => {
     await expect(withRetry(fn, { maxRetries: 0, baseDelayMs: 1 })).rejects.toThrow('fail');
     expect(fn).toHaveBeenCalledTimes(1);
   });
+
+  it('throws on negative maxRetries', async () => {
+    const fn = mock(() => Promise.resolve('ok'));
+    await expect(withRetry(fn, { maxRetries: -1, baseDelayMs: 1 })).rejects.toThrow(
+      'maxRetries must be >= 0'
+    );
+    expect(fn).not.toHaveBeenCalled();
+  });
+
+  it('respects retryAfter from RetryableError', async () => {
+    const sleepSpy = spyOn(globalThis, 'setTimeout');
+    let attempt = 0;
+    const fn = mock(() => {
+      attempt++;
+      if (attempt < 2) {
+        return Promise.reject(new RetryableError('rate limited', undefined, 500));
+      }
+      return Promise.resolve('ok');
+    });
+
+    await withRetry(fn, { maxRetries: 3, baseDelayMs: 10, maxDelayMs: 1000 });
+    // retryAfter=500 should override the computed backoff (~10ms) since 500 > 10
+    const delay = sleepSpy.mock.calls[0][1] as number;
+    expect(delay).toBeGreaterThanOrEqual(500);
+    sleepSpy.mockRestore();
+  });
 });

src/utils/retry-strategy.ts

@@ -46,16 +46,22 @@ function defaultRetryableCheck(error: unknown): boolean {
 
 /**
  * Compute backoff delay: base * multiplier^attempt + jitter, capped at maxDelay.
+ * Jitter is applied before the final cap to ensure the result never exceeds maxDelayMs.
  */
 function computeDelay(
   attempt: number,
   baseDelayMs: number,
   maxDelayMs: number,
-  multiplier: number
+  multiplier: number,
+  retryAfter?: number
 ): number {
   const exponentialDelay = Math.min(baseDelayMs * Math.pow(multiplier, attempt), maxDelayMs);
   const jitter = exponentialDelay * JITTER_RATIO * Math.random();
-  return exponentialDelay + jitter;
+  const delay = Math.min(exponentialDelay + jitter, maxDelayMs);
+  if (retryAfter !== undefined && retryAfter > 0) {
+    return Math.max(delay, retryAfter);
+  }
+  return delay;
 }
 
 /**
@@ -86,6 +92,10 @@ export async function withRetry<T>(fn: () => Promise<T>, options: RetryOptions):
     onRetry,
   } = options;
 
+  if (maxRetries < 0) {
+    throw new Error('withRetry: maxRetries must be >= 0');
+  }
+
   const isRetryable = retryableCheck ?? defaultRetryableCheck;
   let lastError: unknown;
 
@@ -108,7 +118,8 @@ export async function withRetry<T>(fn: () => Promise<T>, options: RetryOptions):
       const err = error instanceof Error ? error : new Error(String(error));
       onRetry?.(err, attempt + 1);
 
-      const delay = computeDelay(attempt, baseDelayMs, maxDelayMs, backoffMultiplier);
+      const retryAfter = error instanceof RetryableError ? error.retryAfter : undefined;
+      const delay = computeDelay(attempt, baseDelayMs, maxDelayMs, backoffMultiplier, retryAfter);
       await sleep(delay);
     }
   }