fix(vinpin): warm-path Fiat silent not_found (wrong-window raise + silent null)
When the warm daemon is up and Rpartstore is DOWN, a partial-warm session (Fiat opened, Rpartstore launch-error, Dialogys opened last → foreground) made a Fiat warm decode fail silently: the Fiat foreground regex shared the `Поиск` token with the Dialogys "ПОИСК" button, so raiseWarmWindow reported success without raising Fiat and the VIN was typed into Dialogys → garbage; warmDecode's Fiat branch then did a bare `return null` (no log, no fallback). - FIX 1: drop the ambiguous `Поиск` from VINPIN_WINDOW_FOREGROUND.fiat; keep Fiat-only chrome (Dealer/ePER) + VIN-panel model tokens. - FIX 2: warmDecode Fiat unusable-parse no longer returns a silent null — OCR the frame; on-panel + genuine not-found → null (real miss), otherwise warn (cold-path parity) and fall back to the proven cold decodeFiatLocked. - FIX 3: ensureWarmWindow panel-verifies a raised Fiat window (catalogueReady); if up but off the VIN panel, re-navigate via establishSession (bounded/never-throw). - FIX 4: _warmUp records per-window availability (warmWindows) so a Fiat VIN routes straight to cold when no Fiat window opened; and dismisses a leftover Rpartstore launch-error modal before opening Dialogys so it can't dirty the desktop / drive the wrong-window state. Adds ocrFrame() test seam + 4 unit tests. tsc clean; 114 vinpin tests green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -1,6 +1,6 @@
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import { VinpinDriverService, VinpinSessionDroppedError } from "./vinpin-driver.service";
|
||||
import { VINPIN_COORDS, VINPIN_OCR } from "./vinpin.constants";
|
||||
import { VINPIN_COORDS, VINPIN_OCR, VINPIN_WINDOW_FOREGROUND } from "./vinpin.constants";
|
||||
|
||||
/**
|
||||
* Livelock-breaker unit tests for the Vinpin decode driver. These exercise the
|
||||
@@ -585,3 +585,123 @@ describe("VinpinDriverService — Rpartstore spinner guard", () => {
|
||||
expect(any.sessionPoisoned).toBe(true); // capped path poisons for a clean cold restart
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* Warm-path Fiat fixes: the disambiguated foreground regex (Defect B) and the
|
||||
* warmDecode Fiat branch that must never fail silently (Defect A) — genuine
|
||||
* not-found → null, wrong/garbled window → cold fallback, no Fiat warm window →
|
||||
* cold fallback.
|
||||
*/
|
||||
describe("VinpinDriverService — warm Fiat not-found / wrong-window hardening", () => {
|
||||
const savedEnv = { ...process.env };
|
||||
const VALID_FIAT_VIN = "NM435600006H43436"; // 17 alphanumerics → fiat warm window
|
||||
const FAR_DEADLINE = () => Date.now() + 5 * 60_000;
|
||||
|
||||
beforeEach(() => {
|
||||
process.env.VINPIN_ENABLED = "true";
|
||||
process.env.VINPIN_USER = "user";
|
||||
process.env.VINPIN_PASS = "pass";
|
||||
});
|
||||
afterEach(() => {
|
||||
vi.restoreAllMocks();
|
||||
process.env = { ...savedEnv };
|
||||
});
|
||||
|
||||
function fakePage() {
|
||||
return { isClosed: () => false, keyboard: { press: vi.fn(async () => undefined) } };
|
||||
}
|
||||
function callWarmDecode(driver: VinpinDriverService, vin: string, deadline: number) {
|
||||
const any = driver as unknown as Record<string, unknown>;
|
||||
return (any.warmDecode as (v: string, c: unknown, d: number) => Promise<unknown>).call(
|
||||
driver,
|
||||
vin,
|
||||
{},
|
||||
deadline,
|
||||
);
|
||||
}
|
||||
|
||||
it("FIX 1: VINPIN_WINDOW_FOREGROUND.fiat does NOT match a Dialogys 'ПОИСК' string but DOES match Fiat 'Dealer'/'ePER'/'TIPO-EGEA'", () => {
|
||||
const fg = VINPIN_WINDOW_FOREGROUND.fiat;
|
||||
// The shared-token bug: the Dialogys ПОИСК search button must NOT read as the
|
||||
// Fiat foreground (this false-positive typed the VIN into Dialogys — Defect B).
|
||||
expect(fg.test("ПОИСК Dialogys ИЗМЕНИТЬ")).toBe(false);
|
||||
expect(fg.test("ПОИСК")).toBe(false);
|
||||
// Fiat-window-only chrome + VIN-panel model tokens still match.
|
||||
expect(fg.test("Fiat Dealer")).toBe(true);
|
||||
expect(fg.test("ePER window")).toBe(true);
|
||||
expect(fg.test("6J - TIPO - EGEA (2015-2021)")).toBe(true);
|
||||
expect(fg.test("Spare Parts Catalogue")).toBe(true);
|
||||
// Sanity: the Dialogys foreground regex STILL recognises ПОИСК (its own detection
|
||||
// is unaffected — only the Fiat regex dropped the shared token).
|
||||
expect(VINPIN_WINDOW_FOREGROUND.dialogys.test("ПОИСК Dialogys ИЗМЕНИТЬ")).toBe(true);
|
||||
});
|
||||
|
||||
it("FIX 2: Fiat unusable-parse NOT-on-panel (wrong/garbled window) → falls back to decodeFiatLocked, not a silent null", async () => {
|
||||
const driver = new VinpinDriverService();
|
||||
const any = driver as unknown as Record<string, unknown>;
|
||||
any.warm = true;
|
||||
any.page = fakePage();
|
||||
any.warmWindows = { fiat: true }; // has a Fiat window → not the FIX-4 route
|
||||
vi.spyOn(any as never, "assertSessionAlive").mockResolvedValue(undefined as never);
|
||||
vi.spyOn(any as never, "ensureWarmWindow").mockResolvedValue(true as never);
|
||||
vi.spyOn(any as never, "runVinFlow").mockResolvedValue(null as never); // no usable parse
|
||||
// Frame shows the DIALOGYS window (Defect B), not the Fiat VIN panel.
|
||||
vi.spyOn(any as never, "ocrFrame").mockResolvedValue("ПОИСК Dialogys ИЗМЕНИТЬ" as never);
|
||||
const close = vi.spyOn(any as never, "close").mockResolvedValue(undefined as never);
|
||||
const cold = vi
|
||||
.spyOn(any as never, "decodeFiatLocked")
|
||||
.mockResolvedValue({ brand: "Fiat", model: "TIPO" } as never);
|
||||
|
||||
const result = await callWarmDecode(driver, VALID_FIAT_VIN, FAR_DEADLINE());
|
||||
|
||||
expect(cold).toHaveBeenCalledTimes(1); // cold fallback, NOT a silent null
|
||||
expect(result).toMatchObject({ brand: "Fiat", model: "TIPO" });
|
||||
expect(any.warm).toBe(false); // warm dropped (untrusted Fiat window)
|
||||
expect(close).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("FIX 2: Fiat genuine not-found (on the VIN panel + not-found string) → returns null, no cold fallback", async () => {
|
||||
const driver = new VinpinDriverService();
|
||||
const any = driver as unknown as Record<string, unknown>;
|
||||
any.warm = true;
|
||||
any.page = fakePage();
|
||||
any.warmWindows = { fiat: true };
|
||||
vi.spyOn(any as never, "assertSessionAlive").mockResolvedValue(undefined as never);
|
||||
vi.spyOn(any as never, "ensureWarmWindow").mockResolvedValue(true as never);
|
||||
vi.spyOn(any as never, "runVinFlow").mockResolvedValue(null as never);
|
||||
// Positively on the Fiat Spare-Parts catalogue AND a genuine "не найден" miss.
|
||||
vi.spyOn(any as never, "ocrFrame").mockResolvedValue(
|
||||
"Spare Parts Catalogue TIPO не найден" as never,
|
||||
);
|
||||
const cold = vi.spyOn(any as never, "decodeFiatLocked");
|
||||
|
||||
const result = await callWarmDecode(driver, VALID_FIAT_VIN, FAR_DEADLINE());
|
||||
|
||||
expect(result).toBeNull(); // a real miss is correctly null
|
||||
expect(cold).not.toHaveBeenCalled(); // NOT re-driven cold
|
||||
});
|
||||
|
||||
it("FIX 4: routes a Fiat VIN straight to the cold path when warmWindows.fiat is false (no Fiat warm window)", async () => {
|
||||
const driver = new VinpinDriverService();
|
||||
const any = driver as unknown as Record<string, unknown>;
|
||||
any.warm = true;
|
||||
any.page = fakePage();
|
||||
any.warmWindows = { fiat: false, dialogys: true }; // Renault-only partial warm
|
||||
vi.spyOn(any as never, "assertSessionAlive").mockResolvedValue(undefined as never);
|
||||
const ensure = vi.spyOn(any as never, "ensureWarmWindow");
|
||||
const runVinFlow = vi.spyOn(any as never, "runVinFlow");
|
||||
const close = vi.spyOn(any as never, "close").mockResolvedValue(undefined as never);
|
||||
const cold = vi
|
||||
.spyOn(any as never, "decodeFiatLocked")
|
||||
.mockResolvedValue({ brand: "Fiat", model: "EGEA" } as never);
|
||||
|
||||
const result = await callWarmDecode(driver, VALID_FIAT_VIN, FAR_DEADLINE());
|
||||
|
||||
expect(cold).toHaveBeenCalledTimes(1); // straight to cold — never touched a warm window
|
||||
expect(ensure).not.toHaveBeenCalled();
|
||||
expect(runVinFlow).not.toHaveBeenCalled();
|
||||
expect(result).toMatchObject({ brand: "Fiat", model: "EGEA" });
|
||||
expect(any.warm).toBe(false);
|
||||
expect(close).toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -174,6 +174,11 @@ export class VinpinDriverService implements OnModuleDestroy {
|
||||
/** brand-window → bound taskbar button coordinate, resolved by OCR at warmUp
|
||||
* (order depends on the open sequence, so it's read, not hardcoded). */
|
||||
private taskbarCoords: Partial<Record<WarmWindow, { x: number; y: number }>> = {};
|
||||
/** Which catalog windows actually came up in the CURRENT warm session, recorded
|
||||
* by _warmUp. A partial warm (e.g. Rpartstore DOWN → only Dialogys) can leave
|
||||
* `warm=true` with NO Fiat window; warmDecode consults this to route a Fiat VIN
|
||||
* straight to the cold path instead of decoding on the wrong (non-Fiat) window. */
|
||||
private warmWindows: Partial<Record<WarmWindow, boolean>> = {};
|
||||
/** Set while ANY exclusive seat operation runs (decode / warmUp / teardown /
|
||||
* keepalive). The keepalive loop reads this to SKIP a nudge during a decode —
|
||||
* it never takes a lock that could block a decode. */
|
||||
@@ -186,6 +191,7 @@ export class VinpinDriverService implements OnModuleDestroy {
|
||||
await this.close();
|
||||
this.warm = false;
|
||||
this.taskbarCoords = {};
|
||||
this.warmWindows = {};
|
||||
await terminateOcr();
|
||||
}
|
||||
|
||||
@@ -371,6 +377,7 @@ export class VinpinDriverService implements OnModuleDestroy {
|
||||
await this.runExclusive(async () => {
|
||||
this.warm = false;
|
||||
this.taskbarCoords = {};
|
||||
this.warmWindows = {};
|
||||
// Clean the RDS SEAT (close catalog windows + log out) BEFORE dropping the
|
||||
// browser, so the next warm-up starts from a fresh login/grid rather than
|
||||
// resuming into the last-open dirty catalog desktop.
|
||||
@@ -479,6 +486,7 @@ export class VinpinDriverService implements OnModuleDestroy {
|
||||
if (!cfg.enabled || !cfg.warmDaemon || !cfg.user || !cfg.pass) return false;
|
||||
this.warm = false;
|
||||
this.taskbarCoords = {};
|
||||
this.warmWindows = {};
|
||||
// Fresh relogin budget for this establish (one close+relaunch escalation, no more).
|
||||
this.reloginUsedThisDecode = false;
|
||||
try {
|
||||
@@ -497,10 +505,18 @@ export class VinpinDriverService implements OnModuleDestroy {
|
||||
// open (ensureRpartstore opens from the grid and does NOT close ePER).
|
||||
let okR = false;
|
||||
if (await this.raiseGridViaTaskbar(page)) okR = await this.ensureRpartstore(page);
|
||||
// If Rpartstore did NOT come up (e.g. it's DOWN and left its hard launch-error
|
||||
// modal on screen), clear any leftover blocking modal BEFORE opening Dialogys.
|
||||
// Otherwise the launch-error dialog dirties the desktop and can drive the
|
||||
// wrong-window foreground state that this fix set out to close (Defect B).
|
||||
if (!okR) await this.dismissBlockingModal(page);
|
||||
// Window 3: Renault Dialogys — background Rpartstore via the grid, then open
|
||||
// (submenu-only opener, no window-closing returnToBrandGrid).
|
||||
let okD = false;
|
||||
if (await this.raiseGridViaTaskbar(page)) okD = await this.openDialogysSubmenu(page);
|
||||
// Record per-window availability so warmDecode can route a VIN to the cold path
|
||||
// when its warm window never opened (closes the "warm=true, no Fiat window" hole).
|
||||
this.warmWindows = { fiat: fiatUp, rpartstore: okR, dialogys: okD };
|
||||
// Bind taskbar buttons by OCR'd left→right order (raiseWarmWindow re-verifies
|
||||
// and re-binds by probing, so this is only a fast-path hint).
|
||||
await this.bindTaskbarCoords(page);
|
||||
@@ -598,6 +614,17 @@ export class VinpinDriverService implements OnModuleDestroy {
|
||||
if (!page || page.isClosed()) throw new VinpinSessionDroppedError("warm page gone");
|
||||
await this.assertSessionAlive(page);
|
||||
const win = selectVinpinWarmWindow(vin); // "fiat" | "rpartstore"
|
||||
// Partial-warm guard: this warm session may have come up WITHOUT a Fiat window
|
||||
// (e.g. Rpartstore down → only Dialogys). Never decode a Fiat VIN on a non-Fiat
|
||||
// window — route it straight to the proven cold Fiat path (with its own retries).
|
||||
if (win === "fiat" && !this.warmWindows?.fiat) {
|
||||
this.logger.warn(
|
||||
`warm ${vin}: no Fiat window in this warm session — routing to the cold Fiat path`,
|
||||
);
|
||||
this.warm = false;
|
||||
await this.close();
|
||||
return this.decodeFiatLocked(vin, cfg, deadline);
|
||||
}
|
||||
if (win === "fiat") {
|
||||
if (!(await this.ensureWarmWindow(page, "fiat", cfg))) {
|
||||
throw new VinpinSessionDroppedError("could not focus the Fiat ePER window");
|
||||
@@ -612,7 +639,26 @@ export class VinpinDriverService implements OnModuleDestroy {
|
||||
);
|
||||
return this.toResult(parsed, text);
|
||||
}
|
||||
return null;
|
||||
// No usable parse. Distinguish a GENUINE not-found (correctly null) from a
|
||||
// decode that ran on the wrong/garbled window (Defect B). OCR the frame: only
|
||||
// when we're positively on the Fiat VIN panel AND it shows a real not-found is
|
||||
// this a true miss → null. Otherwise NEVER fail silently (cold-path parity):
|
||||
// warn and fall back to the proven cold Fiat path, which re-establishes and
|
||||
// retries. Dropping warm + close() here is intentional — a Fiat window that
|
||||
// just produced garbage isn't trusted; the daemon re-warms next cycle.
|
||||
const frame = await this.ocrFrame(page);
|
||||
const onFiatPanel = VINPIN_OCR.catalogueReady.test(frame);
|
||||
const genuineNotFound = VINPIN_OCR.notFound.test(frame);
|
||||
if (onFiatPanel && genuineNotFound) {
|
||||
this.logger.log(`warm ${vin}: genuine not-found on the Fiat VIN panel → null`);
|
||||
return null;
|
||||
}
|
||||
this.logger.warn(
|
||||
`warm ${vin}: Fiat decode produced no usable parse and the frame is not a confirmed Fiat-panel not-found (onPanel=${onFiatPanel}, notFound=${genuineNotFound}) — falling back to the cold Fiat path`,
|
||||
);
|
||||
this.warm = false;
|
||||
await this.close();
|
||||
return this.decodeFiatLocked(vin, cfg, deadline);
|
||||
}
|
||||
return this.warmRenaultDecode(page, vin, cfg, deadline);
|
||||
});
|
||||
@@ -687,7 +733,7 @@ export class VinpinDriverService implements OnModuleDestroy {
|
||||
* window can't be focused nor reopened (→ caller treats it as a session drop).
|
||||
*/
|
||||
private async ensureWarmWindow(page: Page, key: WarmWindow, cfg: VinpinConfig): Promise<boolean> {
|
||||
if (await this.raiseWarmWindow(page, key)) return true;
|
||||
if (await this.raiseWarmWindow(page, key)) return this.ensureFiatPanelIfNeeded(page, key, cfg);
|
||||
this.logger.warn(`warm ${key} window not focusable — reopening from the grid`);
|
||||
if (!(await this.raiseGridViaTaskbar(page))) return false;
|
||||
let reopened = false;
|
||||
@@ -707,7 +753,44 @@ export class VinpinDriverService implements OnModuleDestroy {
|
||||
}
|
||||
if (!reopened) return false;
|
||||
await this.bindTaskbarCoords(page);
|
||||
return this.raiseWarmWindow(page, key);
|
||||
if (!(await this.raiseWarmWindow(page, key))) return false;
|
||||
return this.ensureFiatPanelIfNeeded(page, key, cfg);
|
||||
}
|
||||
|
||||
/**
|
||||
* After a chrome-verified raise of the FIAT window, make sure it's actually parked
|
||||
* on the VIN-identification panel (the Spare Parts Catalogue). A partial-warm
|
||||
* resume can leave the ePER window UP but off the VIN panel (Defect B), which would
|
||||
* type the VIN into nothing. If it's up but not on the panel, re-navigate via the
|
||||
* proven `establishSession` (opens the catalogue) and re-confirm. Bounded and
|
||||
* never-throw: non-fiat keys pass straight through, and any hiccup degrades to the
|
||||
* runVinFlow / FIX-2 backstop rather than throwing.
|
||||
*/
|
||||
private async ensureFiatPanelIfNeeded(
|
||||
page: Page,
|
||||
key: WarmWindow,
|
||||
cfg: VinpinConfig,
|
||||
): Promise<boolean> {
|
||||
if (key !== "fiat") return true;
|
||||
try {
|
||||
if (VINPIN_OCR.catalogueReady.test(await this.ocrFrame(page))) return true;
|
||||
this.logger.warn(
|
||||
"warm Fiat window is up but not on the VIN panel — re-navigating via establishSession",
|
||||
);
|
||||
await this.establishSession(cfg);
|
||||
this.establishedFlow = null;
|
||||
return VINPIN_OCR.catalogueReady.test(await this.ocrFrame(page));
|
||||
} catch (err) {
|
||||
this.logger.warn(`ensureFiatPanelIfNeeded best-effort failure: ${(err as Error).message}`);
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
/** Full-frame OCR read, never-throw (empty string on any failure — `ocrRegion`
|
||||
* already swallows its own errors). A thin wrapper so the warm-path panel /
|
||||
* not-found checks can be unit-tested by spying on it. */
|
||||
private async ocrFrame(page: Page): Promise<string> {
|
||||
return ocrRegion(page).catch(() => "");
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -248,7 +248,12 @@ export type VinpinWindowKey = keyof typeof VINPIN_WINDOW_LABELS;
|
||||
* on the right window before running a decode on it.
|
||||
*/
|
||||
export const VINPIN_WINDOW_FOREGROUND: Record<"fiat" | "rpartstore" | "dialogys", RegExp> = {
|
||||
fiat: /Spare\s*Parts|GRANDE\s*PANDA|TIPO|EGEA|DOBLO|Dealer|ePER|Поиск/i,
|
||||
// ⚠️ Fiat-window-ONLY tokens. The old `Поиск` token was SHARED with the Dialogys
|
||||
// "ПОИСК" search button (`/Поиск/i.test("ПОИСК") === true`), so in a partial-warm
|
||||
// session where Dialogys ends up foreground the Fiat raise falsely reported success
|
||||
// and the VIN was typed into Dialogys → silent garbage. `Dealer`/`ePER` are Fiat
|
||||
// ePER chrome (never on Dialogys/Rpartstore); the rest are VIN-panel model tokens.
|
||||
fiat: /Spare\s*Parts\s*Catalogue|GRANDE\s*PANDA|TIPO|EGEA|DOBLO|Dealer|ePER/i,
|
||||
rpartstore: /RPartStore|Rpartstore|Güncel\s*ara|Grup\s*sipariş|Şasi|Sasi/i,
|
||||
dialogys: /ПОИСК|ПОИC|Dialogys|ИЗМЕНИТЬ/i,
|
||||
};
|
||||
|
||||
Reference in New Issue
Block a user