fix(vinpin): break the single-seat decode livelock (attempts:1 + budget + poison)

A bad/unresolvable VIN could leave a catalog window open on the shared Vinpin
seat; the next decode's ensureBrandGrid found it "resumed open" and looped
(close→brand-grid-not-confirmed→ePER-open→browser-disconnect→relaunch) forever.
BullMQ attempts:2 + 30s backoff auto-re-fed every failure straight back into the
stuck seat, starving all real decodes for minutes.

- queue: attempts:1, drop the 30s exponential backoff (extract VINPIN_DECODE_JOB_
  OPTIONS). The driver already runs its own bounded internal retries; a BullMQ
  retry on top is what compounded the livelock. Null decode still persists as
  not_found; a hard infra throw stays user-retriable (failed).
- driver: hard per-decode wall-clock budget (VINPIN_DECODE_BUDGET_MS, 150s) via
  withDeadline() racing each attempt; on abort → close() + poison seat + return
  null (no retry into the stuck state).
- driver: sessionPoisoned flag — set at nav-loop exhaustion (ensureBrandGrid /
  establishSession), budget abort, and failed post-decode cleanup; the NEXT
  decode forces a full cold re-establish instead of reconnecting to the resumed
  desktop. Cleared on any confirmed-clean grid/catalogue.
- driver: finally-cleanup after every decode — on failure/not-found return the
  seat to a clean brand grid; if that can't reach the grid, poison + tear down.
- driver: basic VIN sanity (17 alphanumerics) before touching the seat.

Healthy Fiat/Renault happy paths are byte-identical when nothing is stuck.
Cannot be live-tested (seat is on prod) — needs prod validation.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

apps/api/src/integrations/vinpin/vinpin-driver.service.spec.ts

@@ -0,0 +1,109 @@
+import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
+import { VinpinDriverService } from "./vinpin-driver.service";
+
+/**
+ * Livelock-breaker unit tests for the Vinpin decode driver. These exercise the
+ * failure/abort machinery (wall-clock budget, sessionPoisoned cold re-establish,
+ * VIN sanity gate) in isolation by stubbing the browser-driving privates — the
+ * shared Vinpin seat lives on prod and can't be driven from a test.
+ */
+describe("VinpinDriverService — livelock breakers", () => {
+  const savedEnv = { ...process.env };
+  const VALID_FIAT_VIN = "NM435600006H43436"; // 17 alphanumerics → fiat flow
+
+  beforeEach(() => {
+    // afterEach restores the full env snapshot, so VINPIN_DECODE_BUDGET_MS set by
+    // the budget test never leaks into the others.
+    process.env.VINPIN_ENABLED = "true";
+    process.env.VINPIN_USER = "user";
+    process.env.VINPIN_PASS = "pass";
+  });
+
+  afterEach(() => {
+    vi.restoreAllMocks();
+    process.env = { ...savedEnv };
+  });
+
+  it("aborts a single decode that blows past the wall-clock budget, tears down and poisons the seat", async () => {
+    process.env.VINPIN_DECODE_BUDGET_MS = "50";
+    const driver = new VinpinDriverService();
+    const anyDriver = driver as unknown as Record<string, unknown>;
+
+    const closeSpy = vi.spyOn(anyDriver as never, "close").mockResolvedValue(undefined as never);
+    // ensureReady succeeds cheaply, then runVinFlow hangs forever → the budget
+    // timer must win the race.
+    vi.spyOn(anyDriver as never, "ensureReady").mockImplementation((async () => {
+      anyDriver.page = { isClosed: () => false };
+    }) as never);
+    vi.spyOn(anyDriver as never, "runVinFlow").mockReturnValue(
+      new Promise(() => {}) as never, // never resolves
+    );
+
+    const started = Date.now();
+    const result = await driver.decode(VALID_FIAT_VIN);
+    const elapsed = Date.now() - started;
+
+    expect(result).toBeNull();
+    expect(closeSpy).toHaveBeenCalled(); // torn down on abort
+    expect(anyDriver.sessionPoisoned).toBe(true); // seat marked poisoned
+    // Aborted near the budget, not after minutes / all 3 internal attempts.
+    expect(elapsed).toBeLessThan(2_000);
+  });
+
+  it("forces a full cold re-establish (close) when the seat was poisoned by a prior decode", async () => {
+    const driver = new VinpinDriverService();
+    const anyDriver = driver as unknown as Record<string, unknown>;
+    anyDriver.sessionPoisoned = true;
+
+    const closeSpy = vi.spyOn(anyDriver as never, "close").mockResolvedValue(undefined as never);
+    const fiatSpy = vi
+      .spyOn(anyDriver as never, "decodeFiatLocked")
+      .mockResolvedValue({ brand: "Fiat", model: "EGEA" } as never);
+
+    const result = await driver.decode(VALID_FIAT_VIN);
+
+    expect(closeSpy).toHaveBeenCalledTimes(1); // the poison-forced teardown
+    expect(anyDriver.sessionPoisoned).toBe(false); // flag consumed
+    expect(fiatSpy).toHaveBeenCalledTimes(1);
+    expect(result).toEqual({ brand: "Fiat", model: "EGEA" });
+  });
+
+  it("does NOT tear down a healthy warm session when the seat is not poisoned", async () => {
+    const driver = new VinpinDriverService();
+    const anyDriver = driver as unknown as Record<string, unknown>;
+    anyDriver.sessionPoisoned = false;
+
+    const closeSpy = vi.spyOn(anyDriver as never, "close").mockResolvedValue(undefined as never);
+    vi.spyOn(anyDriver as never, "decodeFiatLocked").mockResolvedValue({
+      brand: "Fiat",
+      model: "EGEA",
+    } as never);
+
+    const result = await driver.decode(VALID_FIAT_VIN);
+
+    expect(closeSpy).not.toHaveBeenCalled(); // healthy path untouched
+    expect(result).toEqual({ brand: "Fiat", model: "EGEA" });
+  });
+
+  it("skips obviously-junk input before it ever touches the shared seat", async () => {
+    const driver = new VinpinDriverService();
+    const anyDriver = driver as unknown as Record<string, unknown>;
+    const fiatSpy = vi.spyOn(anyDriver as never, "decodeFiatLocked");
+
+    expect(await driver.decode("SHORT")).toBeNull();
+    expect(await driver.decode("VF1RFE0065363319")).toBeNull(); // 16 chars
+    expect(await driver.decode("NM4356 0006H43436")).toBeNull(); // space (non-alnum)
+    expect(fiatSpy).not.toHaveBeenCalled();
+  });
+
+  it("still runs a well-formed VIN through the decode loop (sanity gate is not over-filtering)", async () => {
+    const driver = new VinpinDriverService();
+    const anyDriver = driver as unknown as Record<string, unknown>;
+    const fiatSpy = vi
+      .spyOn(anyDriver as never, "decodeFiatLocked")
+      .mockResolvedValue(null as never);
+
+    expect(await driver.decode(VALID_FIAT_VIN)).toBeNull();
+    expect(fiatSpy).toHaveBeenCalledTimes(1);
+  });
+});

apps/api/src/integrations/vinpin/vinpin-driver.service.ts

@@ -31,6 +31,7 @@ import { Injectable, Logger, type OnModuleDestroy } from "@nestjs/common";
 import type { Browser, BrowserContext, Page } from "playwright";
 import {
   VINPIN_COORDS,
+  VINPIN_DECODE_BUDGET_MS,
   VINPIN_FIELD_CLEAR_BACKSPACES,
   VINPIN_MODAL_REGION,
   VINPIN_OCR,
@@ -80,6 +81,8 @@ interface VinpinConfig {
   pass: string | undefined;
   maxAttempts: number;
   headful: boolean;
+  /** Hard wall-clock cap (ms) for a single decode(vin), across all attempts. */
+  budgetMs: number;
 }
 
 function readConfig(): VinpinConfig {
@@ -90,9 +93,17 @@ function readConfig(): VinpinConfig {
     pass: process.env.VINPIN_PASS || undefined,
     maxAttempts: Number(process.env.VINPIN_DECODE_MAX_ATTEMPTS) || 3,
     headful: process.env.VINPIN_HEADFUL === "true",
+    budgetMs: Number(process.env.VINPIN_DECODE_BUDGET_MS) || VINPIN_DECODE_BUDGET_MS,
   };
 }
 
+/**
+ * Thrown when a single decode blows past its wall-clock budget. Distinct type so
+ * the attempt loop can tell a budget abort (tear down, poison the seat, give up
+ * immediately) apart from an ordinary transient failure (which retries).
+ */
+class VinpinBudgetError extends Error {}
+
 @Injectable()
 export class VinpinDriverService implements OnModuleDestroy {
   private readonly logger = new Logger(VinpinDriverService.name);
@@ -108,6 +119,13 @@ export class VinpinDriverService implements OnModuleDestroy {
    *  Renault flows land on different catalog windows, so a brand switch forces a
    *  re-establish. null = only logged in to the VinPower brand grid. */
   private establishedFlow: "fiat" | "renault" | null = null;
+  /** Set when a decode left the shared seat in a state we could NOT clean up
+   *  (a resumed catalog window we couldn't close, a nav livelock, a budget abort,
+   *  or a failed post-decode cleanup). The NEXT decode sees this and forces a full
+   *  cold re-establish (close() → fresh launch + establishSession) instead of
+   *  reusing/reconnecting to the poisoned warm session. Survives close() on
+   *  purpose (close() resets the session flags, not the poison marker). */
+  private sessionPoisoned = false;
 
   // Single Vinpin seat → serialize all decodes through one promise chain.
   private lock: Promise<unknown> = Promise.resolve();
@@ -131,8 +149,17 @@ export class VinpinDriverService implements OnModuleDestroy {
       this.logger.warn("decode skipped — VINPIN_USER/VINPIN_PASS not set");
       return null;
     }
+    // Basic sanity: a VIN is exactly 17 alphanumerics. Reject obvious junk before
+    // it ever touches the shared seat (kept deliberately loose — length + charset
+    // only — so no real VIN is ever filtered; typos of the right shape still run).
+    if (!/^[A-Za-z0-9]{17}$/.test(vin ?? "")) {
+      this.logger.warn(`decode skipped — not a 17-char alphanumeric VIN: "${vin}"`);
+      return null;
+    }
+    // Hard wall-clock deadline for THIS decode, spanning every internal attempt.
+    const deadline = Date.now() + cfg.budgetMs;
     // Serialize behind the single-seat lock.
-    const run = this.lock.then(() => this.decodeLocked(vin, cfg));
+    const run = this.lock.then(() => this.decodeLocked(vin, cfg, deadline));
     // Keep the chain alive even if this run rejects (it shouldn't — defensive).
     this.lock = run.catch(() => undefined);
     return run;
@@ -170,24 +197,114 @@ export class VinpinDriverService implements OnModuleDestroy {
     });
   }
 
-  private async decodeLocked(vin: string, cfg: VinpinConfig): Promise<VinpinDecodeResult | null> {
+  private async decodeLocked(
+    vin: string,
+    cfg: VinpinConfig,
+    deadline: number,
+  ): Promise<VinpinDecodeResult | null> {
+    // A prior decode left the seat unrecoverable → never reuse/reconnect to it;
+    // tear it down so this decode cold re-establishes from a fresh browser.
+    if (this.sessionPoisoned) {
+      this.logger.warn("seat poisoned by a prior decode — forcing a full cold re-establish");
+      this.sessionPoisoned = false;
+      await this.close();
+    }
+
     const flow = selectVinpinBrandFlow(vin);
-    if (flow === "renault") return this.decodeRenaultLocked(vin, cfg);
-    return this.decodeFiatLocked(vin, cfg);
+    let result: VinpinDecodeResult | null = null;
+    try {
+      result =
+        flow === "renault"
+          ? await this.decodeRenaultLocked(vin, cfg, deadline)
+          : await this.decodeFiatLocked(vin, cfg, deadline);
+      return result;
+    } finally {
+      // Clean up after EVERY decode. On success the warm session is already clean
+      // (runVinFlow clears its own modal) so this is a cheap no-op that leaves the
+      // happy path untouched; on a failure/not-found it makes sure we didn't leave
+      // a catalog window open to poison the next decode. If cleanup itself can't
+      // reach a clean grid, mark the seat poisoned → next decode cold-restarts.
+      if (!result) {
+        await this.cleanupAfterFailure(cfg).catch((err) => {
+          this.logger.warn(`post-decode cleanup failed for ${vin}: ${(err as Error).message}`);
+          this.sessionPoisoned = true;
+        });
+      }
+    }
   }
 
-  /** Existing Fiat ePER decode loop — behaviour byte-identical to before. */
+  /**
+   * Race an in-flight page operation against the decode's wall-clock deadline. If
+   * the deadline passes first, reject with a `VinpinBudgetError` so the caller can
+   * abort cleanly — the orphaned work promise is defused (`close()` will reject its
+   * pending page ops) so it never surfaces as an unhandled rejection.
+   */
+  private async withDeadline<T>(deadline: number, label: string, fn: () => Promise<T>): Promise<T> {
+    const remaining = deadline - Date.now();
+    if (remaining <= 0) throw new VinpinBudgetError(label);
+    let timer: ReturnType<typeof setTimeout> | undefined;
+    const timeout = new Promise<never>((_, reject) => {
+      timer = setTimeout(() => reject(new VinpinBudgetError(label)), remaining);
+    });
+    const work = fn();
+    work.catch(() => {}); // defuse: if the timeout wins, close() rejects these ops
+    try {
+      return await Promise.race([work, timeout]);
+    } finally {
+      if (timer) clearTimeout(timer);
+    }
+  }
+
+  /**
+   * Return the shared seat to a clean state after a failed/not-found decode so it
+   * can't poison the next one. If the browser is already gone (the Fiat null path
+   * close()s as it retries) this is a no-op and the next decode cold-establishes.
+   * If a live session can't be brought back to the brand grid, poison the seat and
+   * tear it down rather than leaving a stray catalog window open.
+   */
+  private async cleanupAfterFailure(cfg: VinpinConfig): Promise<void> {
+    if (!this.browser?.isConnected() || !this.context || !this.page || this.page.isClosed()) {
+      return; // nothing live to clean — next decode starts cold
+    }
+    if (!this.authed) return; // never got past login → cold path is simpler
+    const page = this.page;
+    await this.returnToBrandGrid(page, cfg);
+    if (!VINPIN_OCR.brandGrid.test(await ocrRegion(page))) {
+      // Couldn't reach a clean grid → the seat is poisoned; tear it down so the
+      // next decode does a full cold re-establish instead of reusing this state.
+      this.sessionPoisoned = true;
+      await this.close();
+      return;
+    }
+    // Back on a clean grid → forget the (now-closed) catalog so the next decode
+    // opens fresh from the grid rather than assuming a warm catalog window.
+    this.loggedIn = false;
+    this.establishedFlow = null;
+  }
+
+  /**
+   * Fiat ePER decode loop. Happy path is byte-identical to before; the only added
+   * behaviour is the wall-clock breaker: each attempt runs under the shared
+   * `deadline`, and a budget abort tears down + poisons the seat + gives up
+   * immediately (rather than retrying into the same stuck state).
+   */
   private async decodeFiatLocked(
     vin: string,
     cfg: VinpinConfig,
+    deadline: number,
   ): Promise<VinpinDecodeResult | null> {
     for (let attempt = 1; attempt <= cfg.maxAttempts; attempt++) {
+      if (Date.now() >= deadline) {
+        this.logger.warn(`decode budget exhausted before attempt ${attempt} for ${vin} — aborting`);
+        break;
+      }
       try {
-        await this.ensureReady(cfg);
-        const page = this.page;
-        if (!page) throw new Error("Vinpin page not initialized");
-
-        const text = await this.runVinFlow(page, vin);
+        const text = await this.withDeadline(deadline, `fiat-decode ${vin}`, async () => {
+          await this.ensureReady(cfg);
+          const page = this.page;
+          if (!page) throw new Error("Vinpin page not initialized");
+          return this.runVinFlow(page, vin);
+        });
         const parsed = parseVinpinModal(text);
 
         if (isUsableParse(parsed)) {
@@ -203,6 +320,14 @@ export class VinpinDriverService implements OnModuleDestroy {
         // Empty/garbled — re-establish the session on the next attempt.
         await this.close();
       } catch (err) {
+        if (err instanceof VinpinBudgetError) {
+          this.logger.warn(
+            `decode budget (${cfg.budgetMs}ms) exceeded for ${vin} — tearing down + poisoning seat`,
+          );
+          this.sessionPoisoned = true;
+          await this.close();
+          return null;
+        }
         const e = err as Error;
         this.logger.warn(`attempt ${attempt}/${cfg.maxAttempts} failed for ${vin}: ${e.message}`);
         // Hard failure → tear the browser down so the next attempt re-establishes.
@@ -222,18 +347,26 @@ export class VinpinDriverService implements OnModuleDestroy {
   private async decodeRenaultLocked(
     vin: string,
     cfg: VinpinConfig,
+    deadline: number,
   ): Promise<VinpinDecodeResult | null> {
     for (let attempt = 1; attempt <= cfg.maxAttempts; attempt++) {
+      if (Date.now() >= deadline) {
+        this.logger.warn(
+          `decode budget exhausted before Renault attempt ${attempt} for ${vin} — aborting`,
+        );
+        break;
+      }
       try {
-        await this.ensureAuthenticated(cfg);
-        const page = this.page;
-        if (!page) throw new Error("Vinpin page not initialized");
-        // Leaving any warm Fiat catalogue behind (the Renault flow drives its own
-        // Rpartstore/Dialogys windows).
-        this.loggedIn = false;
-        this.establishedFlow = "renault";
-
-        const outcome = await this.runRenaultFlow(page, vin);
+        const outcome = await this.withDeadline(deadline, `renault-decode ${vin}`, async () => {
+          await this.ensureAuthenticated(cfg);
+          const page = this.page;
+          if (!page) throw new Error("Vinpin page not initialized");
+          // Leaving any warm Fiat catalogue behind (the Renault flow drives its own
+          // Rpartstore/Dialogys windows).
+          this.loggedIn = false;
+          this.establishedFlow = "renault";
+          return this.runRenaultFlow(page, vin);
+        });
         if (outcome.status === "found") {
           const p = outcome.parsed;
           this.logger.log(
@@ -251,6 +384,14 @@ export class VinpinDriverService implements OnModuleDestroy {
         );
         await this.close();
       } catch (err) {
+        if (err instanceof VinpinBudgetError) {
+          this.logger.warn(
+            `decode budget (${cfg.budgetMs}ms) exceeded for Renault ${vin} — tearing down + poisoning seat`,
+          );
+          this.sessionPoisoned = true;
+          await this.close();
+          return null;
+        }
         const e = err as Error;
         this.logger.warn(
           `attempt ${attempt}/${cfg.maxAttempts} Renault decode failed for ${vin}: ${e.message}`,
@@ -479,7 +620,12 @@ export class VinpinDriverService implements OnModuleDestroy {
       opened = matched;
       if (!opened) this.logger.warn(`Fiat ePER open attempt ${i + 1}/4 — not yet (retrying)`);
     }
-    if (!opened) throw new Error("Fiat ePER did not open after retries");
+    if (!opened) {
+      // Bounded tries exhausted → do NOT keep relaunching into the same resumed
+      // desktop. Poison the seat so the next decode cold re-establishes fresh.
+      this.sessionPoisoned = true;
+      throw new Error("Fiat ePER did not open after retries");
+    }
 
     // c. Dismiss the spurious Russian-language dialog (click its OK button).
     await page.mouse.click(VINPIN_COORDS.langDialogDismiss.x, VINPIN_COORDS.langDialogDismiss.y);
@@ -497,10 +643,16 @@ export class VinpinDriverService implements OnModuleDestroy {
       await page.mouse.click(VINPIN_COORDS.sparePartsTile.x, VINPIN_COORDS.sparePartsTile.y);
       await page.waitForTimeout(VINPIN_WAITS.afterSearchOpen);
     }
-    if (!onCatalogue) throw new Error("could not reach Spare Parts Catalogue (VIN panel)");
+    if (!onCatalogue) {
+      // Bounded tries exhausted → poison the seat so the next decode cold-restarts
+      // instead of looping back into this resumed-catalogue state.
+      this.sessionPoisoned = true;
+      throw new Error("could not reach Spare Parts Catalogue (VIN panel)");
+    }
 
     this.loggedIn = true;
     this.establishedFlow = "fiat";
+    this.sessionPoisoned = false; // reached a confirmed-clean state
     this.logger.log("Vinpin session established (Fiat ePER Spare Parts catalogue)");
   }
 
@@ -519,6 +671,7 @@ export class VinpinDriverService implements OnModuleDestroy {
       const t = await ocrRegion(page);
       if (VINPIN_OCR.brandGrid.test(t)) {
         if (i > 0) this.logger.log("VinPower brand grid up");
+        this.sessionPoisoned = false; // reached a confirmed-clean grid
         return;
       }
       // If VinPower's own login dialog is up, satisfy it (never re-click the app
@@ -560,6 +713,10 @@ export class VinpinDriverService implements OnModuleDestroy {
       await this.dismissDisconnected(page);
       await page.waitForTimeout(VINPIN_WAITS.afterVinpinLaunch);
     }
+    // Bounded tries exhausted without confirming the grid — most often a resumed
+    // catalog window we couldn't close (the seat-livelock trigger). Poison the seat
+    // so the next decode cold re-establishes rather than reconnecting to this state.
+    this.sessionPoisoned = true;
     this.logger.warn("VinPower brand grid not confirmed — letting the ePER-open loop try anyway");
   }
 

apps/api/src/integrations/vinpin/vinpin.constants.ts

@@ -183,6 +183,17 @@ export const VINPIN_WAITS = {
   afterDialogysSubmit: 8_000,
 } as const;
 
+/**
+ * Hard wall-clock budget (ms) for a SINGLE decode(vin) call, spanning all of its
+ * internal attempts. A single Vinpin seat is shared by every decode, so one VIN
+ * that gets stuck in a re-establish/relaunch livelock must never grind for
+ * minutes and starve real decodes. When the budget is exceeded the driver aborts,
+ * tears the browser down, poisons the seat (forcing the next decode to cold
+ * re-establish) and returns null. Kept well above the healthy p90 (a warm decode
+ * is seconds; a full cold re-establish is ~60-80s) so it only ever fires on a
+ * genuine stall. Override with VINPIN_DECODE_BUDGET_MS. */
+export const VINPIN_DECODE_BUDGET_MS = 150_000;
+
 /** Per-key delay (ms) when typing a VIN into a focused canvas field. A focused
  *  field keeps up at ~20ms; the old 45-50ms was conservative padding (17-char VIN
  *  ≈ 340ms vs ≈ 850ms). */