Fix HIGH and MEDIUM frontend defects
Files pane
- F16: a drag-out released back inside the app no longer re-imports its own
staged copy over the container original. An in-flight flag (cleared from the
drag plugin's `onEvent` channel, with a watchdog) suppresses the drop and the
"Drop files into …" hint, and an exact staged-path filter is the second line
of defence — the `path|size|modified` cache could otherwise write a
minutes-old snapshot over a file an agent had since rewritten.
- F17: a slow upload/rename no longer yanks the user back to the directory the
operation started in. Every operation captures its target path and re-lists
only if the user is still there; failures go to the toast host either way.
- The grid keeps keyboard focus. Roving tabindex (one tab stop, not one per
row) plus focus restore after navigation, rename commit/cancel and Escape.
- Transient failures now surface in `ToastHost` (z-[60], persistent aria-live)
instead of a `role="alert"` 300 rows down a scroller or behind a modal
overlay. The inline error is kept only for the listing failure.
- `navigate` is sequenced by generation; "Save to host…" sets `busy`.
- Grid a11y: column headers, a text affordance for folder vs file, a live
region that is mounted empty and announces completion, Label-in-Name fixed.
- FileViewerModal: the blob URL is released only once its replacement exists;
the preview is a focusable, named, scrollable region.
Native drop routing
- New `lib/dropTarget.ts`: the hit test now refuses a drop while any
`[aria-modal="true"]` dialog or `[data-blocks-drop]` overlay is up, and
checks z-order where the environment can answer it. Shared by FilesTab and
TerminalView; App's shutdown overlay opts in.
Disk
- A partially failed reclaim says so in words ("… — 2 of 5 failed"), not by hue
alone.
- The scan/reclaim race is closed: every mutation retires an in-flight scan, so
a scan can no longer repaint a pre-reclaim report plus a clickable plan of
objects that are gone. Scan is disabled while working; the status is a live
region; a failed destructive action keeps its dialog open and reports there.
- The "unknown" layer count gets a screen-reader fallback; `--text-disabled`
no longer carries live information.
Terminal / OAuth
- After the toast is dismissed, a truncated heuristic guess can no longer fill
the slot that an exact OSC 8 or relay URL occupied — the detector remembers
every exact URL and drops any candidate that is a strict prefix of one.
- The prompt is reachable by keyboard: Ctrl+Shift+O jumps to the default
action, Escape dismisses, focus returns to the terminal, and auto-dismiss
holds off while focus is inside. It deliberately does not steal focus.
- UrlToast renders through `ui/Button` and `--shadow-overlay`.
Elsewhere
- AuthBridgeRow: a pushed `auth-bridge-changed` status always outranks an older
awaited toggle result.
- The last two ad-hoc byte formatters route through `lib/formatBytes`.
Contract for the backend agent: `upload_file_to_container` refusing to
overwrite must satisfy `isFileExistsError` in `src/lib/uploadErrors.ts` (marker
`FILE_EXISTS`) and accept an `overwrite` argument; the frontend turns that into
an `ui/Modal` Replace/Skip prompt rather than a raw error string.
Tests: 536 -> 627 passing. `npm run build` and `npx tsc --noEmit` green.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GBq2rGum6GX7xXgsas1fDc
This commit is contained in:
@@ -0,0 +1,76 @@
|
||||
import { describe, expect, it, beforeEach, afterEach } from "vitest";
|
||||
import { dropIsBlocked, isDropTarget } from "./dropTarget";
|
||||
|
||||
function pane(rect: Partial<DOMRect>): HTMLElement {
|
||||
const el = document.createElement("div");
|
||||
document.body.appendChild(el);
|
||||
el.getBoundingClientRect = () =>
|
||||
({
|
||||
left: 0,
|
||||
top: 0,
|
||||
right: 100,
|
||||
bottom: 100,
|
||||
width: 100,
|
||||
height: 100,
|
||||
x: 0,
|
||||
y: 0,
|
||||
toJSON: () => ({}),
|
||||
...rect,
|
||||
}) as DOMRect;
|
||||
return el;
|
||||
}
|
||||
|
||||
describe("dropTarget", () => {
|
||||
beforeEach(() => {
|
||||
document.body.innerHTML = "";
|
||||
});
|
||||
afterEach(() => {
|
||||
document.body.innerHTML = "";
|
||||
});
|
||||
|
||||
it("accepts a point inside the pane", () => {
|
||||
expect(isDropTarget(pane({}), { x: 50, y: 50 }, { devicePixelRatio: 1 })).toBe(true);
|
||||
});
|
||||
|
||||
it("rejects a point outside the pane", () => {
|
||||
expect(isDropTarget(pane({}), { x: 400, y: 50 }, { devicePixelRatio: 1 })).toBe(false);
|
||||
});
|
||||
|
||||
it("converts physical pixels to CSS pixels", () => {
|
||||
const el = pane({});
|
||||
expect(isDropTarget(el, { x: 150, y: 150 }, { devicePixelRatio: 2 })).toBe(true);
|
||||
expect(isDropTarget(el, { x: 150, y: 150 }, { devicePixelRatio: 1 })).toBe(false);
|
||||
});
|
||||
|
||||
it("rejects a hidden pane, which has a zero-size rect", () => {
|
||||
const el = pane({ right: 0, bottom: 0, width: 0, height: 0 });
|
||||
expect(isDropTarget(el, { x: 0, y: 0 }, { devicePixelRatio: 1 })).toBe(false);
|
||||
});
|
||||
|
||||
it("rejects every drop while a modal is open", () => {
|
||||
const el = pane({});
|
||||
const dialog = document.createElement("div");
|
||||
dialog.setAttribute("role", "dialog");
|
||||
dialog.setAttribute("aria-modal", "true");
|
||||
document.body.appendChild(dialog);
|
||||
|
||||
expect(dropIsBlocked()).toBe(true);
|
||||
expect(isDropTarget(el, { x: 50, y: 50 }, { devicePixelRatio: 1 })).toBe(false);
|
||||
|
||||
dialog.remove();
|
||||
expect(dropIsBlocked()).toBe(false);
|
||||
expect(isDropTarget(el, { x: 50, y: 50 }, { devicePixelRatio: 1 })).toBe(true);
|
||||
});
|
||||
|
||||
it("rejects every drop while a blocking overlay is up", () => {
|
||||
const el = pane({});
|
||||
const overlay = document.createElement("div");
|
||||
overlay.setAttribute("data-blocks-drop", "true");
|
||||
document.body.appendChild(overlay);
|
||||
expect(isDropTarget(el, { x: 50, y: 50 }, { devicePixelRatio: 1 })).toBe(false);
|
||||
});
|
||||
|
||||
it("rejects a null pane", () => {
|
||||
expect(isDropTarget(null, { x: 1, y: 1 }, { devicePixelRatio: 1 })).toBe(false);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,81 @@
|
||||
/**
|
||||
* Routing for Tauri's *native* drag-drop event.
|
||||
*
|
||||
* The listener is window-wide — every pane that wants dropped file paths gets
|
||||
* the same event — so each one decides for itself whether the drop was meant
|
||||
* for it. That decision used to be purely geometric: is the payload position
|
||||
* inside my rect? A rect is not what the user sees, though. An open `Modal` is
|
||||
* a `fixed inset-0` portal at `z-50` painted *over* the whole window, and the
|
||||
* pane underneath still had its rect, so releasing a drag onto a dialog
|
||||
* uploaded the file into the directory the dialog was covering. Same for the
|
||||
* shutdown overlay, which is on screen precisely while nothing should be
|
||||
* accepting work at all.
|
||||
*
|
||||
* So the hit test is now: nothing is covering the window, **and** the point is
|
||||
* inside my rect, **and** whatever is actually painted at that point is mine.
|
||||
*/
|
||||
|
||||
export interface DropPoint {
|
||||
x: number;
|
||||
y: number;
|
||||
}
|
||||
|
||||
/**
|
||||
* Anything that swallows a drop wherever it lands.
|
||||
*
|
||||
* `[aria-modal="true"]` is every dialog in the app for free — `ui/Modal` is
|
||||
* the only way one is built, and it sets that attribute. `data-blocks-drop`
|
||||
* is for full-window overlays that are not dialogs (the shutdown overlay).
|
||||
*/
|
||||
const BLOCKING_SELECTOR = '[aria-modal="true"],[data-blocks-drop="true"]';
|
||||
|
||||
/** True while a modal or a blocking overlay is on screen. */
|
||||
export function dropIsBlocked(doc: Document = document): boolean {
|
||||
return doc.querySelector(BLOCKING_SELECTOR) !== null;
|
||||
}
|
||||
|
||||
export interface DropTargetOptions {
|
||||
doc?: Document;
|
||||
/** Override the ratio used to convert physical pixels to CSS pixels. */
|
||||
devicePixelRatio?: number;
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether a native drop at `pos` (physical pixels) belongs to `el`.
|
||||
*
|
||||
* A hidden pane is `display:none` and therefore has a zero-size rect, which is
|
||||
* what stops two panes both claiming the same drop.
|
||||
*/
|
||||
export function isDropTarget(
|
||||
el: HTMLElement | null | undefined,
|
||||
pos: DropPoint,
|
||||
options: DropTargetOptions = {},
|
||||
): boolean {
|
||||
const doc = options.doc ?? el?.ownerDocument ?? document;
|
||||
if (dropIsBlocked(doc)) return false;
|
||||
|
||||
const rect = el?.getBoundingClientRect();
|
||||
if (!el || !rect || rect.width === 0 || rect.height === 0) return false;
|
||||
|
||||
const dpr =
|
||||
options.devicePixelRatio ??
|
||||
(doc.defaultView?.devicePixelRatio || 1);
|
||||
const x = pos.x / dpr;
|
||||
const y = pos.y / dpr;
|
||||
if (x < rect.left || x > rect.right || y < rect.top || y > rect.bottom) {
|
||||
return false;
|
||||
}
|
||||
|
||||
// Z-order, where the environment can answer it. `elementFromPoint` skips
|
||||
// `pointer-events: none`, so the pane's own decorative drop hint does not
|
||||
// count as something covering it. jsdom has no layout and returns null,
|
||||
// which is treated as "no opinion" rather than "not mine".
|
||||
if (typeof doc.elementFromPoint === "function") {
|
||||
const top = doc.elementFromPoint(x, y);
|
||||
if (top && top !== doc.body && top !== doc.documentElement && !el.contains(top)) {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
return true;
|
||||
}
|
||||
@@ -29,6 +29,22 @@ describe("formatBytes", () => {
|
||||
expect(formatBytes(1_610_612_736, { binary: true })).toBe("1.5 GB");
|
||||
});
|
||||
|
||||
it("absorbs the last two ad-hoc formatters, behaviour change and all", () => {
|
||||
// `UpdateDialog.formatSize` and the inline `toFixed(1)` in
|
||||
// `useProjectActions` both divided by 1024 and both stopped at MB. Routing
|
||||
// them here is what finally makes this the *only* byte formatter, and it
|
||||
// changes two things on purpose — pinned so neither reads as a regression
|
||||
// to whoever meets them next.
|
||||
//
|
||||
// KB gains a decimal, matching every other size in the app:
|
||||
expect(formatBytes(512 * 1024, { binary: true })).toBe("512.0 KB");
|
||||
// and the ladder no longer bottoms out at a five-digit megabyte count:
|
||||
expect(formatBytes(2 * 1024 ** 3, { binary: true })).toBe("2.0 GB");
|
||||
// Sub-kilobyte sizes stop rendering as "0 KB", which is what the old
|
||||
// `(bytes / 1024).toFixed(0)` said about every release asset under 512 B.
|
||||
expect(formatBytes(400, { binary: true })).toBe("400 B");
|
||||
});
|
||||
|
||||
it("reproduces the migration convention exactly by default", () => {
|
||||
// `migrationCopy.formatDataSize` is now a call to this, and its output is
|
||||
// asserted in MigrateContainerModal.test.tsx.
|
||||
|
||||
@@ -4,11 +4,16 @@
|
||||
* The app had four of them — `projects/home/format.ts`,
|
||||
* `projects/migrationCopy.ts`, `settings/UpdateDialog.tsx` and an inline
|
||||
* `toFixed(1)` in `useProjectActions.ts` — disagreeing about the divisor, the
|
||||
* unit labels and the precision. The first two now delegate here.
|
||||
* unit labels and the precision. All four now delegate here, and there are no
|
||||
* remaining copies.
|
||||
*
|
||||
* The other two deliberately do not, yet: `UpdateDialog` renders KB at
|
||||
* `toFixed(0)`, so re-pointing it would change what a download size reads as,
|
||||
* and neither is on the Disk panel's path. They are the remaining copies.
|
||||
* The last two were held back because re-pointing them changes what they
|
||||
* render, and that turned out to be the argument for doing it rather than
|
||||
* against. `UpdateDialog` rendered KB at `toFixed(0)` (`512 KB` is now
|
||||
* `512.0 KB`, consistent with every other size in the app) and both stopped
|
||||
* the ladder at MB, so a 2 GB asset or backup read as a five-digit number of
|
||||
* megabytes. Both are `{ binary: true }`: they describe files, and a host file
|
||||
* browser shows the ÷1024 figure for the same bytes.
|
||||
*
|
||||
* ## Why the default is base 1000
|
||||
*
|
||||
|
||||
@@ -75,8 +75,21 @@ export const downloadContainerFile = (projectId: string, containerPath: string,
|
||||
invoke<void>("download_container_file", { projectId, containerPath, hostPath });
|
||||
export const downloadContainerBackup = (projectId: string, hostPath: string, containerPath?: string) =>
|
||||
invoke<number>("download_container_backup", { projectId, hostPath, containerPath });
|
||||
export const uploadFileToContainer = (projectId: string, hostPath: string, containerDir: string) =>
|
||||
invoke<void>("upload_file_to_container", { projectId, hostPath, containerDir });
|
||||
/**
|
||||
* Copy a host file into a container directory.
|
||||
*
|
||||
* `overwrite` is opt-in because a drop is aimed with a mouse: the backend
|
||||
* refuses by default when the name is already taken (see `lib/uploadErrors.ts`
|
||||
* for the marker that refusal carries), and the caller re-runs with `true`
|
||||
* only once the user has said "Replace" to that specific file. Leaving it off
|
||||
* is the safe default every existing caller gets.
|
||||
*/
|
||||
export const uploadFileToContainer = (
|
||||
projectId: string,
|
||||
hostPath: string,
|
||||
containerDir: string,
|
||||
overwrite?: boolean,
|
||||
) => invoke<void>("upload_file_to_container", { projectId, hostPath, containerDir, overwrite });
|
||||
export const readContainerFile = (projectId: string, path: string, maxBytes?: number) =>
|
||||
invoke<FileContents>("read_container_file", { projectId, path, maxBytes });
|
||||
/** `toPath` is the new *name*, not a destination — renames never move. */
|
||||
|
||||
@@ -0,0 +1,81 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import {
|
||||
FILE_EXISTS_MARKER,
|
||||
fileExistsPath,
|
||||
isFileExistsError,
|
||||
} from "./uploadErrors";
|
||||
|
||||
/**
|
||||
* The shapes here are the point of the module.
|
||||
*
|
||||
* A Tauri command error crosses the IPC boundary as whatever `serde` made of
|
||||
* it, and the Rust side is free to change from `Err(String)` to a serialised
|
||||
* error enum without anyone thinking of this file. Every one of these has to
|
||||
* keep meaning "that name is taken", or an upload that could have been
|
||||
* retried with `overwrite: true` degrades into a raw string in a toast.
|
||||
*/
|
||||
describe("isFileExistsError", () => {
|
||||
it("recognises the agreed prose form", () => {
|
||||
expect(isFileExistsError("FILE_EXISTS: /workspace/notes.txt already exists")).toBe(true);
|
||||
});
|
||||
|
||||
it("recognises a bare marker", () => {
|
||||
expect(isFileExistsError(FILE_EXISTS_MARKER)).toBe(true);
|
||||
});
|
||||
|
||||
it("recognises a serialised error enum, whatever case it is written in", () => {
|
||||
expect(isFileExistsError({ kind: "FileExists", path: "/workspace/a.txt" })).toBe(true);
|
||||
expect(isFileExistsError({ code: "file-exists" })).toBe(true);
|
||||
expect(isFileExistsError({ type: "file_exists" })).toBe(true);
|
||||
});
|
||||
|
||||
it("recognises it inside a message field", () => {
|
||||
expect(isFileExistsError({ message: "upload refused: FILE_EXISTS" })).toBe(true);
|
||||
expect(isFileExistsError(new Error("FILE_EXISTS: /workspace/a.txt"))).toBe(true);
|
||||
});
|
||||
|
||||
it("looks one level into a wrapped error", () => {
|
||||
expect(isFileExistsError({ error: { kind: "FileExists" } })).toBe(true);
|
||||
});
|
||||
|
||||
it("says no to every other failure, which must not raise an overwrite prompt", () => {
|
||||
expect(isFileExistsError("File too large to upload (900 MB; limit 256 MB)")).toBe(false);
|
||||
expect(isFileExistsError("cp: cannot create regular file: Permission denied")).toBe(false);
|
||||
expect(isFileExistsError({ kind: "NotRunning" })).toBe(false);
|
||||
expect(isFileExistsError(null)).toBe(false);
|
||||
expect(isFileExistsError(undefined)).toBe(false);
|
||||
expect(isFileExistsError(42)).toBe(false);
|
||||
expect(isFileExistsError({})).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe("fileExistsPath", () => {
|
||||
it("reads the path out of the agreed prose form", () => {
|
||||
expect(fileExistsPath("FILE_EXISTS: /workspace/notes.txt already exists")).toBe(
|
||||
"/workspace/notes.txt",
|
||||
);
|
||||
});
|
||||
|
||||
it("prefers a structured field", () => {
|
||||
expect(fileExistsPath({ kind: "FileExists", path: "/workspace/a.txt" })).toBe(
|
||||
"/workspace/a.txt",
|
||||
);
|
||||
expect(fileExistsPath({ kind: "FileExists", container_path: "/workspace/b.txt" })).toBe(
|
||||
"/workspace/b.txt",
|
||||
);
|
||||
});
|
||||
|
||||
it("finds one in a wrapped error", () => {
|
||||
expect(fileExistsPath({ error: { kind: "FileExists", path: "/workspace/c.txt" } })).toBe(
|
||||
"/workspace/c.txt",
|
||||
);
|
||||
});
|
||||
|
||||
it("returns null rather than guessing", () => {
|
||||
// The caller falls back to the host path it was uploading, which is always
|
||||
// known — so "no path" is a perfectly good answer.
|
||||
expect(fileExistsPath("FILE_EXISTS")).toBeNull();
|
||||
expect(fileExistsPath({ kind: "FileExists" })).toBeNull();
|
||||
expect(fileExistsPath(null)).toBeNull();
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,116 @@
|
||||
/**
|
||||
* The one place the frontend agrees with Rust about "that name is taken".
|
||||
*
|
||||
* `upload_file_to_container` used to clobber whatever was already at the
|
||||
* destination, which is the wrong default for a drop: a drag is aimed with a
|
||||
* mouse, and the file it lands on is frequently not the file the user meant to
|
||||
* replace. So the backend refuses by default and the frontend asks — but only
|
||||
* if it can tell *this* refusal apart from "permission denied" or "no space
|
||||
* left", because an overwrite prompt raised over an unrelated failure would
|
||||
* offer a button that cannot possibly work.
|
||||
*
|
||||
* **This module is the contract point, and the Rust half has to hold up its
|
||||
* end**: `upload_file_to_container` must put `FILE_EXISTS_MARKER` in the error
|
||||
* it returns when the destination already exists, ideally in the agreed shape
|
||||
*
|
||||
* FILE_EXISTS: /workspace/notes.txt already exists
|
||||
*
|
||||
* and must accept an `overwrite: bool` argument that skips the check. Nothing
|
||||
* here parses a human sentence — the marker is the whole agreement, and the
|
||||
* path is a bonus that is only used to name the file in the prompt.
|
||||
*
|
||||
* The predicate is deliberately tolerant about the *shape* of the error rather
|
||||
* than its wording, because a Tauri command error crosses the IPC boundary as
|
||||
* whatever `serde` made of it: a bare string from `Err(String)`, an object from
|
||||
* a `#[derive(Serialize)]` error enum, or an `Error` if a JS layer wrapped it
|
||||
* on the way through. All three are the same refusal, and the UI must not
|
||||
* behave differently depending on which one a future refactor produces.
|
||||
*/
|
||||
|
||||
/** Marker the backend puts in the error for "a file with this name is already there". */
|
||||
export const FILE_EXISTS_MARKER = "FILE_EXISTS";
|
||||
|
||||
/**
|
||||
* Structured error shapes carry the marker in a discriminant rather than in
|
||||
* prose. These are the field names a serialised Rust error realistically uses;
|
||||
* matching is case-insensitive and ignores `_`/`-` so `FileExists`,
|
||||
* `file_exists` and `FILE-EXISTS` all read as the same variant.
|
||||
*/
|
||||
const KIND_FIELDS = ["kind", "code", "type", "error", "reason"] as const;
|
||||
const MESSAGE_FIELDS = ["message", "msg", "detail", "description"] as const;
|
||||
const PATH_FIELDS = ["path", "container_path", "containerPath", "target", "file"] as const;
|
||||
|
||||
/** `FileExists` / `file-exists` / `FILE_EXISTS` all normalise to `fileexists`. */
|
||||
function normaliseKind(value: string): string {
|
||||
return value.toLowerCase().replace(/[\s_-]/g, "");
|
||||
}
|
||||
|
||||
const KIND_NEEDLE = normaliseKind(FILE_EXISTS_MARKER);
|
||||
|
||||
function asRecord(e: unknown): Record<string, unknown> | null {
|
||||
return typeof e === "object" && e !== null ? (e as Record<string, unknown>) : null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Every string an error carries, flattened: the error itself if it is one, its
|
||||
* message-ish fields, and its kind-ish fields. Nesting is followed one level
|
||||
* because a wrapped error (`{ error: { kind: … } }`) is the same refusal.
|
||||
*/
|
||||
function stringsIn(e: unknown, depth = 0): string[] {
|
||||
if (typeof e === "string") return [e];
|
||||
if (e instanceof Error) return [e.message, e.name];
|
||||
const record = asRecord(e);
|
||||
if (!record || depth > 1) return [];
|
||||
const out: string[] = [];
|
||||
for (const field of [...KIND_FIELDS, ...MESSAGE_FIELDS]) {
|
||||
const value = record[field];
|
||||
if (typeof value === "string") out.push(value);
|
||||
else if (value !== undefined) out.push(...stringsIn(value, depth + 1));
|
||||
}
|
||||
return out;
|
||||
}
|
||||
|
||||
/**
|
||||
* True when the backend refused an upload because the destination is taken.
|
||||
*
|
||||
* Accepts a bare string, an `Error`, or an object with a `kind`/`code`
|
||||
* discriminant or a `message` — see the module comment for why all three have
|
||||
* to work.
|
||||
*/
|
||||
export function isFileExistsError(e: unknown): boolean {
|
||||
return stringsIn(e).some((s) => normaliseKind(s).includes(KIND_NEEDLE));
|
||||
}
|
||||
|
||||
/**
|
||||
* The container path the conflict is about, when the error carries one — used
|
||||
* only to name the file in the prompt, so `null` is a perfectly good answer
|
||||
* and the caller falls back to the host path it was uploading.
|
||||
*/
|
||||
export function fileExistsPath(e: unknown): string | null {
|
||||
const record = asRecord(e);
|
||||
if (record) {
|
||||
for (const field of PATH_FIELDS) {
|
||||
const value = record[field];
|
||||
if (typeof value === "string" && value.length > 0) return value;
|
||||
}
|
||||
// One level down, for `{ error: { path } }`.
|
||||
for (const field of KIND_FIELDS) {
|
||||
const nested = fileExistsPath(record[field]);
|
||||
if (nested) return nested;
|
||||
}
|
||||
}
|
||||
for (const s of stringsIn(e)) {
|
||||
// The agreed prose form: `FILE_EXISTS: <path>` — everything up to the
|
||||
// first space after the marker.
|
||||
const match = new RegExp(`${FILE_EXISTS_MARKER}\\s*[:=]\\s*(\\S+)`).exec(s);
|
||||
if (match) return match[1];
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
/**
|
||||
* What the user answered to one conflict. The blanket answers exist because a
|
||||
* ten-file drop onto a populated directory is ten prompts otherwise, which is
|
||||
* the kind of dialog people dismiss without reading.
|
||||
*/
|
||||
export type OverwriteChoice = "replace" | "skip" | "replace-all" | "skip-all";
|
||||
@@ -225,6 +225,55 @@ describe("UrlDetector — OSC 8", () => {
|
||||
expect(seen).toEqual([[url, "heuristic"]]);
|
||||
});
|
||||
|
||||
it("never hands back a truncated guess at a link it has already seen exactly", () => {
|
||||
// The defect: the prompt slot is emptied (dismissed, or auto-dismissed
|
||||
// after 30 s), the OSC 8 target is deduped for the session and cannot come
|
||||
// back, and the next repaint — sliced at a different offset, so a *new*
|
||||
// string — reassembles into a prefix of the real link that fills the empty
|
||||
// slot. It parses, it points at claude.ai, and it authorises nothing.
|
||||
//
|
||||
// Nothing here knows the slot was emptied, and that is the point: the rule
|
||||
// holds however many times it is.
|
||||
const seen: [string, UrlSource][] = [];
|
||||
const d = new UrlDetector((u, s) => seen.push([u, s]), () => COLS);
|
||||
|
||||
feed(d, "Open this link to sign in:\r\n" + slicedHyperlink(SIGN_IN_URL) + "\r\ndone\r\n");
|
||||
expect(seen).toEqual([[SIGN_IN_URL, "osc8"]]);
|
||||
|
||||
// …the user dismisses the toast; the TUI repaints the same link as plain
|
||||
// text, cut short by the frame it was painted into.
|
||||
feed(d, SIGN_IN_URL.slice(0, 150) + "\r\nWaiting for the browser…\r\n");
|
||||
|
||||
expect(seen).toHaveLength(1);
|
||||
expect(seen.map(([u]) => u)).not.toContain(SIGN_IN_URL.slice(0, 150));
|
||||
});
|
||||
|
||||
it("still offers a genuinely different link after an exact one", () => {
|
||||
// The suppression is a prefix rule, not "one prompt per session".
|
||||
const seen: string[] = [];
|
||||
const d = new UrlDetector((u) => seen.push(u), () => COLS);
|
||||
const other = "https://github.com/login/device?code=" + "x".repeat(90);
|
||||
|
||||
feed(d, slicedHyperlink(SIGN_IN_URL) + "\r\n");
|
||||
feed(d, other + "\r\nnext\r\n");
|
||||
|
||||
expect(seen).toEqual([SIGN_IN_URL, other]);
|
||||
});
|
||||
|
||||
it("suppresses a guess at a URL the consumer reported from the relay", () => {
|
||||
// The OSC 7777 relay hands `TerminalView` a base64-encoded — therefore
|
||||
// exact — URL that this detector never sees. `noteExactUrl` is how it gets
|
||||
// told, so a dismissed relay prompt cannot be replaced by a scrape of the
|
||||
// same link either.
|
||||
const seen: string[] = [];
|
||||
const d = new UrlDetector((u) => seen.push(u), () => COLS);
|
||||
d.noteExactUrl(SIGN_IN_URL);
|
||||
|
||||
feed(d, SIGN_IN_URL.slice(0, 150) + "\r\nnext\r\n");
|
||||
|
||||
expect(seen).toEqual([]);
|
||||
});
|
||||
|
||||
it("ignores a short hyperlink", () => {
|
||||
// `ls --hyperlink` decorates every filename; none of that is a prompt.
|
||||
const seen: string[] = [];
|
||||
|
||||
@@ -45,8 +45,27 @@
|
||||
*
|
||||
* So each emitted candidate is tagged with where it came from, and the consumer
|
||||
* refuses to let a `heuristic` candidate displace an `osc8` one.
|
||||
*
|
||||
* ## …and the exact copy keeps winning after the prompt is gone
|
||||
*
|
||||
* The consumer's precedence rule only compares a new candidate against what is
|
||||
* *currently* in the prompt slot. Empty the slot — the user dismisses the
|
||||
* toast, or its 30 s auto-dismiss fires — and it has nothing to compare
|
||||
* against, so the next truncated guess walks straight in. Meanwhile the OSC 8
|
||||
* target is deduped for the life of the session and cannot come back to
|
||||
* displace it. The user is then holding a URL that parses, points at
|
||||
* claude.ai, and authorises nothing, which is the exact bug the OSC 8 branch
|
||||
* was added to kill.
|
||||
*
|
||||
* That is fixed *here* rather than in the consumer, because this is the side
|
||||
* that knows both halves: {@link UrlDetector} remembers every exact URL it has
|
||||
* seen and refuses to emit a heuristic candidate that is a strict prefix of
|
||||
* one — see `truncatesKnownExact`. The rule then holds however often the slot
|
||||
* is emptied, and needs no cooperation from whoever owns it.
|
||||
*/
|
||||
|
||||
import { extendsUrl } from "./urlRelay";
|
||||
|
||||
const ANSI_RE =
|
||||
/\x1b(?:\[[0-9;?]*[A-Za-z]|\][^\x07\x1b]*(?:\x07|\x1b\\)?|[()#][A-Za-z0-9]|.)/g;
|
||||
|
||||
@@ -196,6 +215,21 @@ export class UrlDetector {
|
||||
/** OSC 8 targets already offered, so a hyperlink repainted every frame does
|
||||
* not re-prompt. Bounded by {@link MAX_REMEMBERED_LINKS}. */
|
||||
private emittedLinks = new Set<string>();
|
||||
/**
|
||||
* Every *exact* URL this session has seen — OSC 8 parameters, plus whatever
|
||||
* the consumer reports through {@link noteExactUrl} (the OSC 7777 relay).
|
||||
*
|
||||
* Kept separately from `emittedLinks` because the two answer different
|
||||
* questions: that one is "have I already prompted for this?", this one is "do
|
||||
* I know the full text of a link some guess might be a prefix of?". The
|
||||
* second answer must survive the prompt being dismissed; the whole defect is
|
||||
* that a truncated guess fills the slot the moment it is empty.
|
||||
*
|
||||
* Bounded the same way, and cleared wholesale rather than evicted one by one:
|
||||
* a program printing a fresh hyperlink every frame is not a program whose
|
||||
* older links are still on screen to be mis-scraped.
|
||||
*/
|
||||
private exactUrls = new Set<string>();
|
||||
|
||||
constructor(callback: UrlCallback, columns: ColumnsGetter) {
|
||||
this.callback = callback;
|
||||
@@ -285,7 +319,7 @@ export class UrlDetector {
|
||||
|
||||
// 6. URL is clearly complete (more content follows) — dedup + emit
|
||||
this.pendingUrl = null;
|
||||
if (url !== this.lastEmitted) {
|
||||
if (url !== this.lastEmitted && !this.truncatesKnownExact(url)) {
|
||||
this.lastEmitted = url;
|
||||
this.callback(url, "heuristic");
|
||||
}
|
||||
@@ -304,10 +338,23 @@ export class UrlDetector {
|
||||
* `lastEmitted` is moved along with them so an identical string arriving on
|
||||
* the heuristic path a moment later is recognised as the same candidate
|
||||
* rather than fired a second time.
|
||||
*
|
||||
* Every target is remembered as exact whether or not it is offered — a
|
||||
* hyperlink repainted a second time is the same known link, and the dedup
|
||||
* that stops it re-prompting must not also stop it counting as something a
|
||||
* later guess can be a truncation of.
|
||||
*
|
||||
* The alternative fix considered here was to make this dedup *releasable*,
|
||||
* so the consumer could hand the exact URL back and have it re-offered once
|
||||
* the prompt slot emptied. Rejected: it re-offers on the very next repaint,
|
||||
* so dismissing the toast would put it straight back on screen — and it
|
||||
* still would not establish the invariant, because a truncated guess and the
|
||||
* released exact URL would simply race for the empty slot.
|
||||
*/
|
||||
private scanLinks(): void {
|
||||
for (const uri of osc8Targets(this.buffer)) {
|
||||
if (uri.length < MIN_URL_LENGTH) continue;
|
||||
this.rememberExact(uri);
|
||||
if (this.emittedLinks.has(uri)) continue;
|
||||
if (this.emittedLinks.size >= MAX_REMEMBERED_LINKS) {
|
||||
this.emittedLinks.clear();
|
||||
@@ -319,13 +366,56 @@ export class UrlDetector {
|
||||
}
|
||||
|
||||
private emitPending(): void {
|
||||
if (this.pendingUrl && this.pendingUrl !== this.lastEmitted) {
|
||||
if (
|
||||
this.pendingUrl &&
|
||||
this.pendingUrl !== this.lastEmitted &&
|
||||
!this.truncatesKnownExact(this.pendingUrl)
|
||||
) {
|
||||
this.lastEmitted = this.pendingUrl;
|
||||
this.callback(this.pendingUrl, "heuristic");
|
||||
}
|
||||
this.pendingUrl = null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether `url` is a strict prefix of an exact URL already seen — i.e. a
|
||||
* truncated guess at a link whose full text is known.
|
||||
*
|
||||
* {@link extendsUrl} is the predicate, used in the direction that asks "does
|
||||
* the link I already have *extend* this guess?". It is the same rule the
|
||||
* prompt slot uses to let a candidate grow into its complete form, which is
|
||||
* the point: the two must agree about what "the same link, only shorter"
|
||||
* means, so there is one implementation of it.
|
||||
*
|
||||
* Deliberately *not* symmetric. A candidate that is longer than a known exact
|
||||
* URL and starts with it is a different problem (text glued onto the end by a
|
||||
* wrap that was not a wrap), and it is still shown in full and confirmed by
|
||||
* the user before anything opens.
|
||||
*/
|
||||
private truncatesKnownExact(url: string): boolean {
|
||||
for (const exact of this.exactUrls) {
|
||||
if (extendsUrl(exact, url)) return true;
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
/**
|
||||
* Record a URL that arrived somewhere exact, outside this detector.
|
||||
*
|
||||
* The OSC 7777 relay hands `TerminalView` a base64-encoded URL — exact by
|
||||
* construction, and never seen here. Without this the suppression rule above
|
||||
* would cover hyperlinks and miss the relay, and a dismissed relay prompt
|
||||
* could still be replaced by a truncated scrape of the same link.
|
||||
*/
|
||||
noteExactUrl(url: string): void {
|
||||
this.rememberExact(url);
|
||||
}
|
||||
|
||||
private rememberExact(url: string): void {
|
||||
if (this.exactUrls.size >= MAX_REMEMBERED_LINKS) this.exactUrls.clear();
|
||||
this.exactUrls.add(url);
|
||||
}
|
||||
|
||||
dispose(): void {
|
||||
if (this.timer !== null) {
|
||||
clearTimeout(this.timer);
|
||||
|
||||
Reference in New Issue
Block a user