feat(docker): resilient agent image pre-pull with retry + fallback

electron/ipc/docker-pull.test.ts

@@ -0,0 +1,100 @@
+/**
+ * Unit tests for the Docker image pre-pull resilience orchestrator.
+ * Pure logic — Docker is fully faked, no network/subprocess/timers.
+ */
+
+import { describe, expect, it, vi } from 'vitest';
+import { ensureDockerImageAvailable } from './docker-pull.js';
+
+const IMAGE = 'thunderockforge/forge-agent:latest';
+
+/** Build a deps object with sensible fakes; override per test. */
+function makeDeps(over: { present?: boolean[]; pullCodes?: number[]; signal?: AbortSignal }) {
+  const present = [...(over.present ?? [])];
+  const pullCodes = [...(over.pullCodes ?? [])];
+  const status: string[] = [];
+  const pull = vi.fn(async () => (pullCodes.length ? (pullCodes.shift() as number) : -1));
+  const delay = vi.fn(async () => {});
+  const imagePresent = vi.fn(async () => (present.length ? (present.shift() as boolean) : false));
+  return {
+    deps: {
+      imagePresent,
+      pull,
+      delay,
+      onStatus: (l: string) => status.push(l),
+      signal: over.signal ?? new AbortController().signal,
+    },
+    status,
+    pull,
+    delay,
+    imagePresent,
+  };
+}
+
+describe('ensureDockerImageAvailable', () => {
+  it('skips the pull entirely when the image is already cached locally', async () => {
+    const { deps, pull } = makeDeps({ present: [true] });
+    const res = await ensureDockerImageAvailable(IMAGE, deps);
+    expect(res).toEqual({ ok: true, usedLocal: true });
+    expect(pull).not.toHaveBeenCalled();
+  });
+
+  it('pulls once and succeeds when the image is missing', async () => {
+    const { deps, pull } = makeDeps({ present: [false], pullCodes: [0] });
+    const res = await ensureDockerImageAvailable(IMAGE, deps);
+    expect(res).toEqual({ ok: true, usedLocal: false });
+    expect(pull).toHaveBeenCalledTimes(1);
+  });
+
+  it('retries with backoff and succeeds on a later attempt', async () => {
+    const { deps, pull, delay, status } = makeDeps({
+      present: [false],
+      pullCodes: [1, 0], // fail, then succeed
+    });
+    const res = await ensureDockerImageAvailable(IMAGE, deps, { maxAttempts: 3 });
+    expect(res).toEqual({ ok: true, usedLocal: false });
+    expect(pull).toHaveBeenCalledTimes(2);
+    expect(delay).toHaveBeenCalledTimes(1);
+    expect(status.some((s) => /retry/i.test(s))).toBe(true);
+  });
+
+  it('gives up after maxAttempts when pulls keep failing and nothing is cached', async () => {
+    const { deps, pull } = makeDeps({
+      present: [false, false], // initial check + final fallback check
+      pullCodes: [1, 1, 1],
+    });
+    const res = await ensureDockerImageAvailable(IMAGE, deps, { maxAttempts: 3 });
+    expect(res).toEqual({ ok: false, reason: 'pull-failed' });
+    expect(pull).toHaveBeenCalledTimes(3);
+  });
+
+  it('falls back to a locally cached copy when pulls fail but the image is present', async () => {
+    const { deps } = makeDeps({
+      present: [false, true], // missing up front, but present on the final fallback check
+      pullCodes: [1, 1, 1],
+    });
+    const res = await ensureDockerImageAvailable(IMAGE, deps, { maxAttempts: 3 });
+    expect(res).toEqual({ ok: true, usedLocal: true });
+  });
+
+  it('returns cancelled without pulling when aborted before start', async () => {
+    const ac = new AbortController();
+    ac.abort();
+    const { deps, pull } = makeDeps({ present: [false], signal: ac.signal });
+    const res = await ensureDockerImageAvailable(IMAGE, deps);
+    expect(res).toEqual({ ok: false, reason: 'cancelled' });
+    expect(pull).not.toHaveBeenCalled();
+  });
+
+  it('returns cancelled when aborted during a pull', async () => {
+    const ac = new AbortController();
+    const { deps } = makeDeps({ present: [false], pullCodes: [-1], signal: ac.signal });
+    // Abort as soon as the pull is attempted.
+    deps.pull = vi.fn(async () => {
+      ac.abort();
+      return -1;
+    });
+    const res = await ensureDockerImageAvailable(IMAGE, deps, { maxAttempts: 3 });
+    expect(res).toEqual({ ok: false, reason: 'cancelled' });
+  });
+});

electron/ipc/docker-pull.ts

@@ -0,0 +1,135 @@
+import { execFile, execFileSync, spawn as cpSpawn } from 'child_process';
+
+/** Project images are built locally (forge-project:<hash>), never pulled from a registry. */
+export const PROJECT_IMAGE_PREFIX = 'forge-project:';
+
+interface EnsureImageDeps {
+  /** Resolve true if an image with this tag is already present locally. */
+  imagePresent: (image: string) => Promise<boolean>;
+  /** Pull the image; resolve with the process exit code (0 = success). */
+  pull: (image: string, signal: AbortSignal) => Promise<number>;
+  /** Abortable sleep. */
+  delay: (ms: number, signal: AbortSignal) => Promise<void>;
+  /** Emit a human-friendly status line to the terminal. */
+  onStatus: (line: string) => void;
+  signal: AbortSignal;
+}
+
+interface EnsureImageOptions {
+  maxAttempts?: number;
+  /** Backoff before retry N (index 0 = wait before 2nd attempt). Last value reused. */
+  backoffMs?: number[];
+}
+
+export type EnsureImageResult =
+  | { ok: true; usedLocal: boolean }
+  | { ok: false; reason: 'cancelled' | 'pull-failed' };
+
+/**
+ * Ensure a registry image is available locally before `docker run`.
+ *
+ * Fast-paths when the image is already cached (no network). Otherwise pulls with
+ * bounded retries + backoff so a transient Docker Hub blip doesn't hard-fail the
+ * task, and falls back to any locally cached copy before giving up.
+ */
+export async function ensureDockerImageAvailable(
+  image: string,
+  deps: EnsureImageDeps,
+  opts: EnsureImageOptions = {},
+): Promise<EnsureImageResult> {
+  const maxAttempts = opts.maxAttempts ?? 3;
+  const backoff = opts.backoffMs ?? [2000, 4000];
+
+  if (deps.signal.aborted) return { ok: false, reason: 'cancelled' };
+
+  // Already cached — `docker run` will use it, no network round-trip needed.
+  if (await deps.imagePresent(image)) return { ok: true, usedLocal: true };
+
+  for (let attempt = 1; attempt <= maxAttempts; attempt++) {
+    if (deps.signal.aborted) return { ok: false, reason: 'cancelled' };
+    deps.onStatus(
+      attempt === 1
+        ? `Pulling ${image} … (first run can take a few minutes)`
+        : `Retrying pull (attempt ${attempt}/${maxAttempts}) …`,
+    );
+
+    const code = await deps.pull(image, deps.signal).catch(() => -1);
+    if (deps.signal.aborted) return { ok: false, reason: 'cancelled' };
+    if (code === 0) return { ok: true, usedLocal: false };
+
+    if (attempt < maxAttempts) {
+      const wait = backoff[Math.min(attempt - 1, backoff.length - 1)];
+      deps.onStatus(`Pull failed — retrying in ${Math.round(wait / 1000)}s …`);
+      await deps.delay(wait, deps.signal);
+    }
+  }
+
+  // Retries exhausted — use any locally cached copy rather than fail outright
+  // (e.g. a concurrent pull landed it, or an older image is good enough).
+  if (await deps.imagePresent(image)) return { ok: true, usedLocal: true };
+
+  return { ok: false, reason: 'pull-failed' };
+}
+
+/**
+ * Synchronous existence check, used on the spawn fast-path so a cached image
+ * still launches without deferring to an async tick. Bounded timeout; treats
+ * any failure (incl. a hung daemon) as "not present" so we fall back to a pull.
+ */
+export function dockerImagePresentSync(image: string): boolean {
+  try {
+    const out = execFileSync(
+      'docker',
+      ['image', 'ls', '--filter', `reference=${image}`, '--format', '{{.ID}}'],
+      { encoding: 'utf8', timeout: 4000, stdio: ['ignore', 'pipe', 'ignore'] },
+    );
+    return !!out.trim();
+  } catch {
+    return false;
+  }
+}
+
+/** True if an image with this tag exists locally (existence only — no staleness check). */
+export function dockerImagePresentByTag(image: string): Promise<boolean> {
+  return new Promise((resolve) => {
+    // `docker image ls --filter reference=` works around the containerd store
+    // breaking tag-based `docker image inspect`.
+    execFile(
+      'docker',
+      ['image', 'ls', '--filter', `reference=${image}`, '--format', '{{.ID}}'],
+      { encoding: 'utf8', timeout: 5000 },
+      (err, stdout) => resolve(!err && !!String(stdout).trim()),
+    );
+  });
+}
+
+/** Stream `docker pull <image>` output to `onData`; resolve with the exit code (-1 on spawn error/abort). */
+export function pullDockerImage(
+  image: string,
+  onData: (text: string) => void,
+  signal: AbortSignal,
+): Promise<number> {
+  return new Promise((resolve) => {
+    const child = cpSpawn('docker', ['pull', image], { signal });
+    child.stdout?.on('data', (d: Buffer) => onData(d.toString('utf8')));
+    child.stderr?.on('data', (d: Buffer) => onData(d.toString('utf8')));
+    child.on('error', () => resolve(-1)); // includes AbortError when signal fires
+    child.on('close', (code) => resolve(code ?? -1));
+  });
+}
+
+/** Promise that resolves after `ms`, or immediately if the signal aborts. */
+export function delay(ms: number, signal: AbortSignal): Promise<void> {
+  return new Promise((resolve) => {
+    if (signal.aborted) return resolve();
+    const onAbort = () => {
+      clearTimeout(timer);
+      resolve();
+    };
+    const timer = setTimeout(() => {
+      signal.removeEventListener('abort', onAbort);
+      resolve();
+    }, ms);
+    signal.addEventListener('abort', onAbort, { once: true });
+  });
+}

electron/ipc/pty.test.ts

@@ -10,6 +10,12 @@ const { mockExecFileSync, mockExecFile, mockChildProcessSpawn, mockPtySpawn, moc
       if (command === 'which' && args?.[0] === 'nonexistent-binary-xyz') {
         throw new Error('not found');
       }
+      // Docker image-presence fast-path: report the image as cached locally so
+      // spawn stays synchronous (no pull) by default. Tests exercising the pull
+      // path tag their image with "needs-pull" to force a cache miss.
+      if (command === 'docker' && args?.[0] === 'image' && args?.[1] === 'ls') {
+        return args?.[3]?.includes('needs-pull') ? '' : 'abc123def456\n';
+      }
       return '';
     });
 
@@ -1227,3 +1233,92 @@ describe('buildDockerCredentialMounts — read-only auth dir', () => {
     expect(warnMessages.some((m) => /\[docker-auth\].*Could not/.test(m))).toBe(true);
   });
 });
+
+describe('spawnAgent docker mode — image pull resilience', () => {
+  const flush = () => new Promise<void>((resolve) => setImmediate(resolve));
+
+  // A docker pull child whose stdout/stderr/close handlers we can drive.
+  function fakePullChild(closeHandlers: ((code: number) => void)[]) {
+    return {
+      stdout: { on: vi.fn() },
+      stderr: { on: vi.fn() },
+      on: vi.fn((event: string, cb: (code: number) => void) => {
+        if (event === 'close') closeHandlers.push(cb);
+      }),
+    };
+  }
+
+  it('pulls a missing image first, then launches the container', async () => {
+    const image = 'registry.test/needs-pull-ok:latest';
+    // Async presence check (and fallback) report the image absent.
+    mockExecFile.mockImplementation((_cmd: string, _args: string[], opts: unknown, cb: unknown) => {
+      const done = (typeof opts === 'function' ? opts : cb) as (e: unknown, out: string) => void;
+      done?.(null, '');
+    });
+    const closeHandlers: ((code: number) => void)[] = [];
+    mockChildProcessSpawn.mockImplementation(() => fakePullChild(closeHandlers));
+
+    spawnAgent(createMockWindow(), buildSpawnArgs({ dockerImage: image, agentId: nextAgentId() }));
+
+    await flush();
+    // A pull was started and the container has NOT launched yet.
+    expect(mockChildProcessSpawn).toHaveBeenCalledWith(
+      'docker',
+      ['pull', image],
+      expect.anything(),
+    );
+    expect(mockPtySpawn).not.toHaveBeenCalled();
+
+    closeHandlers[0]?.(0); // pull succeeds
+    await flush();
+    expect(mockPtySpawn).toHaveBeenCalled(); // container launched after the pull
+  });
+
+  it('reports a friendly error (not a raw daemon dump) when the pull keeps failing', async () => {
+    vi.useFakeTimers();
+    try {
+      const image = 'registry.test/needs-pull-fail:latest';
+      mockExecFile.mockImplementation(
+        (_cmd: string, _args: string[], opts: unknown, cb: unknown) => {
+          const done = (typeof opts === 'function' ? opts : cb) as (
+            e: unknown,
+            out: string,
+          ) => void;
+          done?.(null, ''); // never present
+        },
+      );
+      const closeHandlers: ((code: number) => void)[] = [];
+      mockChildProcessSpawn.mockImplementation(() => fakePullChild(closeHandlers));
+
+      const win = createMockWindow();
+      spawnAgent(
+        win,
+        buildSpawnArgs({
+          dockerImage: image,
+          agentId: nextAgentId(),
+          onOutput: { __CHANNEL_ID__: 'ch-pull-fail' },
+        }),
+      );
+
+      // Drive three failing pull attempts through their backoff windows.
+      for (let i = 0; i < 3; i += 1) {
+        await vi.advanceTimersByTimeAsync(0);
+        expect(closeHandlers.length).toBe(i + 1);
+        closeHandlers[i](1);
+        await vi.advanceTimersByTimeAsync(5000);
+      }
+      await vi.advanceTimersByTimeAsync(0);
+
+      const calls = vi.mocked(win.webContents.send).mock.calls as Array<
+        [string, { type?: string; data?: { exit_code?: number } }]
+      >;
+      // Never launched a container, and surfaced a clean Exit instead of hanging.
+      expect(mockPtySpawn).not.toHaveBeenCalled();
+      const exit = calls.find(([, msg]) => msg?.type === 'Exit');
+      expect(exit).toBeTruthy();
+      expect(exit?.[1].data?.exit_code).toBe(1);
+    } finally {
+      vi.useRealTimers();
+    }
+  });
+});