mirror of
https://github.com/software-mansion/argent.git
synced 2026-09-14 19:27:14 +08:00
Ignacy Łątka
1e8e5fa309
Three independent reliability fixes for **Argent Lens** (the `propose_variant` / `await_user_selection` variant-review flow and its preview window). They touch disjoint files and were validated separately; combined here into one PR. --- ### 1. Preview window 404s on dot-path installs (nvm/fnm/volta/asdf) The Lens window showed a raw `NotFoundError` instead of the variant UI whenever argent was installed under a path containing a dot-segment (most commonly nvm, `~/.nvm/...`). `GET /preview/` and `/preview/theme.css` returned 404 while JSON routes worked. Cause: the express 4 → 5 bump (#354, shipped in v0.12.1). Express 5's `send@1` defaults to `dotfiles: "ignore"`, which 404s any file path containing a dot-segment, and the preview routes pass the absolute install path to `res.sendFile`. Proven in isolation: express 5.2.1 `sendFile` of the file → 404; express 4.22.1 → 200; `sendFile(basename, { root: dir })` → 200. Fix: serve the preview UI via `sendFile(basename, { root: dir })` (a `serveUiFile` helper) so the request path carries no dot-segment. Regression test serves the UI from a temp dir whose name starts with `.`. > Note: the underlying `send@1` dot-path behaviour is a property of the express-5 migration, not of Lens — these two routes are simply the only `sendFile` calls in the tool-server today. ### 2. Variant fails to anchor right after navigation A `propose_variant` made immediately after navigating to a screen (exactly the navigate → screenshot → propose workflow) often failed to anchor: the card floated/hid ("didn't register"), or latched onto a same-text distractor (e.g. a bottom-bar tab instead of a header). Cause: `captureElementFrame` describes the device **once** at propose time, but iOS' accessibility tree is empty for ~1.25 s after a screen appears and then fills incrementally (nav chrome before screen content) — measured on a real device. Fix: `captureElementFrame` now retries across the warm-up window and **holds out for an exact label/identifier hit** (the intended element), falling back to a substring match only after the budget. `findElementMatch` / `matchFrameInTree` rank exact matches above substring ones. Adds `match-element-frame-cold-describe.test.ts`; two existing tests updated for the new retry signature. ### 3. Off-screen choices were unreachable In the preview window, the off-screen reveal prompt (the "Show them" card and the "N hidden — show" pill) was hidden unless **every on-screen choice was already staged** (`onScreenStaged`). So a proposal whose element was off-screen showed a hidden-choice count with no affordance to surface it. Fix: show the prompt whenever any element is off-screen (`offScreenCount > 0 && !revealOffscreen && !submitting`), dropping the `onScreenStaged` gate. --- Supersedes #390, #392, #393 (same commits, combined).
55 lines
2.3 KiB
TypeScript
55 lines
2.3 KiB
TypeScript
/**
|
|
* Regression test: `captureElementFrame` must NOT dispatch the Android describe
|
|
* adapter for a Chromium (CDP) device. Without the chromium guard, a
|
|
* `chromium-cdp-<port>` udid falls into `describeAndroid`, which shells
|
|
* `adb -s chromium-cdp-<port> ...` against a serial that does not exist —
|
|
* pointless process spawn + misleading adb error (swallowed by the outer
|
|
* try/catch, so the only observable symptom is the wasted adb call).
|
|
*
|
|
* The guard short-circuits to `null` before any describe dispatch, so the test
|
|
* asserts on the *side effect* (no adapter invoked) in addition to the null
|
|
* result, since the null is what the catch would have returned anyway.
|
|
*/
|
|
|
|
import { describe, it, expect, vi, beforeEach } from "vitest";
|
|
|
|
const describeIosMock = vi.fn();
|
|
const describeAndroidMock = vi.fn();
|
|
|
|
vi.mock("../src/tools/describe/platforms/ios", () => ({
|
|
describeIos: (...args: unknown[]) => describeIosMock(...args),
|
|
}));
|
|
vi.mock("../src/tools/describe/platforms/android", () => ({
|
|
describeAndroid: (...args: unknown[]) => describeAndroidMock(...args),
|
|
}));
|
|
|
|
import { captureElementFrame } from "../src/utils/match-element-frame";
|
|
import type { Registry } from "@argent/registry";
|
|
|
|
const fakeRegistry = {} as Registry;
|
|
const match = { by: "text", value: "Submit" } as const;
|
|
|
|
beforeEach(() => {
|
|
describeIosMock.mockReset();
|
|
describeAndroidMock.mockReset();
|
|
});
|
|
|
|
describe("captureElementFrame — chromium guard", () => {
|
|
it("returns null for a chromium device WITHOUT dispatching any describe adapter", async () => {
|
|
const out = await captureElementFrame(fakeRegistry, "chromium-cdp-9222", match);
|
|
expect(out).toBeNull();
|
|
expect(describeAndroidMock).not.toHaveBeenCalled();
|
|
expect(describeIosMock).not.toHaveBeenCalled();
|
|
});
|
|
|
|
it("still dispatches the Android adapter for a genuine Android serial", async () => {
|
|
describeAndroidMock.mockResolvedValue({ tree: null });
|
|
// attempts: 1 — this asserts the adapter is dispatched (vs the chromium
|
|
// guard); the warm-up retry would otherwise re-describe across the budget.
|
|
const out = await captureElementFrame(fakeRegistry, "emulator-5554", match, { attempts: 1 });
|
|
expect(out).toBeNull(); // null tree → no match, but the adapter WAS consulted
|
|
expect(describeAndroidMock).toHaveBeenCalledTimes(1);
|
|
expect(describeIosMock).not.toHaveBeenCalled();
|
|
});
|
|
});
|