Files
Triple-C/app/src/components/terminal/UrlToast.test.tsx
T
shadowdaoandClaude Opus 5 60188610ee fix: do not let an in-flight open blank a newer prompt, or promise a bridge that is off
Two findings from review of this branch.

Awaiting the open instead of dismissing up front bought a window: on Linux
it is at least OPENER_GRACE, doubled when xdg-open fails and gio is tried.
If the container relays a second URL inside that window, the first open's
resolution blanked the second prompt -- losing a link that exists only in
the container's transcript, which is the failure "dismiss on success only"
was made to prevent. The slot already carried a `seq` for exactly this
reason; dismissal is now conditional on it.

`urlPromptRef` is written eagerly by the two functions that change the slot
rather than synced by an effect. That is load-bearing: an effect-synced
mirror lags state by a commit, and a promise microtask can resolve between
`setUrlPrompt` and React flushing passive effects -- so it answers "did a
newer prompt land?" wrong in precisely the window the guard exists for.
Dropping the functional updater also fixes `promptSeqRef.current += 1`
being mutated inside a state updater React is free to invoke twice.

The guard is a sibling function rather than an optional argument on
`dismissUrlPrompt`, because that function is passed by reference as
UrlToast's `onDismiss` and React would hand it a MouseEvent as its first
argument -- the seq check would fail and the close button would silently
stop working, with the types still assignable.

Separately, the sign-in hint was binary on which button leads, but "host
leads" covers both a live bridge and a fallback where nothing is set up to
catch the callback at all. In the second case the toast promised the bridge
would carry it and the login hung to its timeout. The target is now
three-state, the hint tells the truth in the fallback case and names the
control that fixes it, and the hook starts at `host-fallback` rather than
assuming a bridge it has not confirmed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-17 11:12:22 -07:00

319 lines
11 KiB
TypeScript

import { describe, it, expect, vi } from "vitest";
import { fireEvent, render, screen } from "@testing-library/react";
import UrlToast, { URL_TOAST_PRIMARY_SELECTOR } from "./UrlToast";
/**
* The toast is the *only* thing standing between a container-chosen URL and
* the host's browser, so what it shows has to be what will be opened — and the
* part that decides that is the origin.
*/
describe("UrlToast", () => {
const noop = () => {};
it("shows the origin separately from the truncatable remainder", () => {
render(
<UrlToast
url="https://github.com/login/device?code=ABCD-EFGH"
onOpen={noop}
onDismiss={noop}
/>,
);
expect(screen.getByTestId("url-toast-origin")).toHaveTextContent(
"https://github.com",
);
expect(screen.getByTestId("url-toast-rest")).toHaveTextContent(
"/login/device?code=ABCD-EFGH",
);
});
it("keeps the origin intact when the path is long enough to push it out", () => {
const url = `https://evil.tld/${"padding/".repeat(200)}end`;
render(<UrlToast url={url} onOpen={noop} onDismiss={noop} />);
// The registrable domain must be present in its own element, whole. A
// single ellipsised line would render this and show only the padding.
expect(screen.getByTestId("url-toast-origin")).toHaveTextContent(
"https://evil.tld",
);
});
it("exposes the whole URL as a tooltip", () => {
const url = "https://example.com/a/b?c=d";
render(<UrlToast url={url} onOpen={noop} onDismiss={noop} />);
expect(screen.getByTestId("url-toast-url")).toHaveAttribute("title", url);
});
it("announces itself, so a replacement prompt is not silent", () => {
render(
<UrlToast url="https://example.com/" onOpen={noop} onDismiss={noop} />,
);
expect(screen.getByRole("status")).toBeInTheDocument();
});
it("opens only via the button, never on its own", () => {
const onOpen = vi.fn();
render(
<UrlToast url="https://example.com/" onOpen={onOpen} onDismiss={noop} />,
);
expect(onOpen).not.toHaveBeenCalled();
screen.getByRole("button", { name: "Open" }).click();
expect(onOpen).toHaveBeenCalledTimes(1);
});
describe("keyboard", () => {
// This toast is the only route to completing a sign-in started in a
// terminal, and xterm's helper textarea swallows Tab — so without these it
// is unreachable for a keyboard-only user.
const SIGN_IN =
"https://claude.ai/oauth/authorize?code=true&client_id=abc&response_type=code";
it("does not take focus from the live terminal when it appears", () => {
// Deliberate. The user may be mid-command, and the default action opens a
// URL the *container* chose — a focused button is one stray Enter away
// from doing it. The shortcut hint below is what makes that affordable.
render(
<UrlToast url="https://example.com/" onOpen={noop} onDismiss={noop} />,
);
expect(document.activeElement).toBe(document.body);
});
it("says how to reach it, since nothing announces a shortcut by itself", () => {
render(
<UrlToast url="https://example.com/" onOpen={noop} onDismiss={noop} />,
);
expect(screen.getByTestId("url-toast-shortcut")).toHaveTextContent(
"Ctrl+Shift+O",
);
});
it("marks the default action so the shortcut has somewhere to land", () => {
// Which button that is depends on the URL, so the marker moves with the
// decision rather than the owner having to repeat it.
const { rerender } = render(
<UrlToast
url="https://github.com/login/device?code=ABCD"
onOpen={noop}
onOpenInContainer={noop}
onDismiss={noop}
/>,
);
expect(
document.querySelector(URL_TOAST_PRIMARY_SELECTOR),
).toHaveTextContent("Open");
rerender(
<UrlToast
url={SIGN_IN}
onOpen={noop}
onOpenInContainer={noop}
signInDefault="container"
onDismiss={noop}
/>,
);
expect(
document.querySelector(URL_TOAST_PRIMARY_SELECTOR),
).toHaveTextContent("In container");
});
it("dismisses on Escape from anywhere inside it", () => {
const onDismiss = vi.fn();
render(
<UrlToast
url="https://example.com/"
onOpen={noop}
onDismiss={onDismiss}
/>,
);
fireEvent.keyDown(screen.getByRole("button", { name: "Open" }), {
key: "Escape",
});
expect(onDismiss).toHaveBeenCalledTimes(1);
});
it("does not answer Escape pressed outside it", () => {
// Escape belongs to whatever is running in the terminal — vim, above all.
// A document-level binding would break it for everyone who never looked
// at this toast.
const onDismiss = vi.fn();
render(
<UrlToast
url="https://example.com/"
onOpen={noop}
onDismiss={onDismiss}
/>,
);
fireEvent.keyDown(document.body, { key: "Escape" });
expect(onDismiss).not.toHaveBeenCalled();
});
it("gives every action a real button, so Tab reaches all three", () => {
render(
<UrlToast
url={SIGN_IN}
onOpen={noop}
onOpenInContainer={noop}
signInDefault="container"
onDismiss={noop}
/>,
);
const names = screen
.getAllByRole("button")
.map((b) => b.getAttribute("aria-label") ?? b.textContent);
expect(names).toEqual(["In container", "Open", "Dismiss"]);
// Nothing is taken out of the tab order.
for (const b of screen.getAllByRole("button")) {
expect(b).not.toHaveAttribute("tabindex", "-1");
}
});
});
describe("Anthropic sign-in links", () => {
// The callback listener a `claude login` is waiting on is *inside* the
// container, so a sign-in is the one case where the host browser may be the
// wrong lead. Whether it actually is depends on the project — a live auth
// bridge carries the callback back, and the container-side alternative is
// not installed on a fresh project — so the owner decides and passes
// `signInDefault`. This component only renders the decision.
const SIGN_IN =
"https://claude.ai/oauth/authorize?code=true&client_id=abc&response_type=code";
function actions() {
return screen
.getAllByRole("button")
.map((b) => b.textContent)
.filter((t) => t === "Open" || t === "In container");
}
it("puts the container browser first when the caller asks for it", () => {
render(
<UrlToast
url={SIGN_IN}
onOpen={noop}
onOpenInContainer={noop}
signInDefault="container"
onDismiss={noop}
/>,
);
expect(actions()).toEqual(["In container", "Open"]);
expect(screen.getByTestId("url-toast-signin-hint")).toHaveTextContent(
/callback listener is inside the container/i,
);
});
it("leads with the host, and promises the bridge, when the bridge is live", () => {
// The pair is unchanged; only the order and which one is filled.
render(
<UrlToast
url={SIGN_IN}
onOpen={noop}
onOpenInContainer={noop}
signInDefault="host-bridged"
onDismiss={noop}
/>,
);
expect(actions()).toEqual(["Open", "In container"]);
expect(
document.querySelector(URL_TOAST_PRIMARY_SELECTOR),
).toHaveTextContent("Open");
// Still recognised as a sign-in, so the explanation stays — and here the
// explanation is true, which is the only state in which it may be given.
expect(screen.getByTestId("url-toast-signin-hint")).toHaveTextContent(
/the auth bridge is what carries the callback/i,
);
});
it("says the callback has nothing carrying it when the host is the last resort", () => {
// `host-fallback`: bridge off or unknown *and* no browser in the
// container. The old two-state hint said the auth bridge would carry the
// callback here too, which is a false promise — the user opens the link
// in their own browser and `claude login` hangs to its timeout with
// nothing on screen explaining why.
render(
<UrlToast
url={SIGN_IN}
onOpen={noop}
onOpenInContainer={noop}
signInDefault="host-fallback"
onDismiss={noop}
/>,
);
// Which button leads does not change — only what the hint claims.
expect(actions()).toEqual(["Open", "In container"]);
expect(
document.querySelector(URL_TOAST_PRIMARY_SELECTOR),
).toHaveTextContent("Open");
const hint = screen.getByTestId("url-toast-signin-hint");
expect(hint).toHaveTextContent(/nothing is set up to reach it/i);
// And it points at the two things that would fix it, since a warning
// with no next step is only a nicer way to fail.
expect(hint).toHaveTextContent(/Auth bridge/);
expect(hint).toHaveTextContent(/install browser support/i);
expect(hint).not.toHaveTextContent(/the auth bridge is what carries the callback/i);
});
it("defaults to the least-bad reading when the caller passes nothing", () => {
// A caller that says nothing has not told us a bridge is live, so the
// hint must not invent one. The host still leads: it is the answer more
// likely to work, and the one that reports its own failure.
render(
<UrlToast
url={SIGN_IN}
onOpen={noop}
onOpenInContainer={noop}
onDismiss={noop}
/>,
);
expect(actions()).toEqual(["Open", "In container"]);
expect(screen.getByTestId("url-toast-signin-hint")).toHaveTextContent(
/nothing is set up to reach it/i,
);
});
it("keeps the host browser available as a fallback", () => {
const onOpen = vi.fn();
render(
<UrlToast
url={SIGN_IN}
onOpen={onOpen}
onOpenInContainer={noop}
signInDefault="container"
onDismiss={noop}
/>,
);
screen.getByRole("button", { name: "Open" }).click();
expect(onOpen).toHaveBeenCalledTimes(1);
});
it("leaves an ordinary URL alone", () => {
// A `gh auth login` device code, a docs page, a preview build — the host
// browser is the right answer for all of them and stays the default,
// whatever the project's sign-in preference happens to be.
render(
<UrlToast
url="https://github.com/login/device?code=ABCD-EFGH"
onOpen={noop}
onOpenInContainer={noop}
signInDefault="container"
onDismiss={noop}
/>,
);
expect(actions()).toEqual(["Open", "In container"]);
expect(screen.queryByTestId("url-toast-signin-hint")).not.toBeInTheDocument();
});
it("is not fooled by a lookalike host", () => {
// `isAnthropicSignInUrl` uses the same allowlist the sign-in flow does,
// so a URL that merely says "claude.ai" somewhere is not one.
render(
<UrlToast
url="https://claude.ai.evil.tld/oauth/authorize?x=1"
onOpen={noop}
onOpenInContainer={noop}
signInDefault="container"
onDismiss={noop}
/>,
);
expect(actions()).toEqual(["Open", "In container"]);
});
});
});