From 22d142c70ddbf7aa14819cc5a6d5bde23a270619 Mon Sep 17 00:00:00 2001 From: Josh Knapp Date: Sun, 23 Aug 2026 08:31:39 -0700 Subject: [PATCH] Shift+Enter newline, OAuth URL truncation, and the auth bridge toggle MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three fixes that all land on the same journey: sign in, paste a prompt, and have the terminal behave the way every other Claude Code host does. Shift+Enter inserts a newline ----------------------------- xterm.js does not consult `shiftKey` for Enter (`Keyboard.ts`, case 13), so Shift+Enter was byte-identical to Enter and submitted the prompt. Both terminals now send `\x1b\r` (ESC+CR) instead, which Claude Code parses as return+meta — the same bytes its own `/terminal-setup` writes into the VS Code, Cursor, Alacritty and Zed keymaps, so this is in-band rather than a guess. Not `\n`: Claude Code accepts it, but a shell would run the line, so the two session types would diverge. Bound in Claude sessions only for that reason. `entrypoint.sh` sets `shiftEnterKeyBindingInstalled` in `~/.claude.json` so the CLI stops printing its "run /terminal-setup" tip. Purely cosmetic — the decoding is unconditional either way. Alt+Enter has always done the same thing (xterm ESC-prefixes on altKey) and was simply never documented. It is now, along with the rest. OAuth login URL truncation -------------------------- Two producers wrote one toast slot, last-writer-wins. The OSC 7777 relay delivers the URL base64-encoded and therefore exact; ~300 ms later the screen-scraper's debounce fired and overwrote it with a truncated guess at the same link — a URL that parses, points at the right host, and authorises nothing. The user is the one who has to notice. Why the scraper truncated: `ANSI_RE` strips OSC sequences wholesale, including the OSC 8 hyperlink whose parameter carries the complete URL. Claude Code slices the *visible* text of that hyperlink to the terminal width while every emission carries the whole URL in its parameter. The backend already knew this (`commands/auth_token_commands.rs`); the frontend did not. - `urlDetector` now reads OSC 8 targets out of the raw buffer before stripping, filtered by a port of `usable_sign_in_link`, and tags every candidate with its provenance. - The prompt slot gained `supersedes`: better provenance always wins, worse never does, and between equals only a candidate that *extends* what is showing may replace it. That last rule is `extendsUrl`, factored out of `pickSignInUrl` rather than copied — same rule, same reason, one implementation. - `flatten` splits on a bare `\r` as well as on `\r?\n`, so a `\r`-repainted TUI frame no longer inflates a line past the width and suppresses a join that should have happened; and the width is now sampled at `feed()` rather than read at `scan()`, so a resize inside the 300 ms debounce cannot reassemble 80-column text against a 120-column rule. Also corrects the comment claiming `acquire_claude_token` enables the auth bridge. It deliberately does not, and the module comment in `auth_token_commands.rs` explains at length why not. The auth bridge toggle ---------------------- `setAuthBridgeEnabled` and `getAuthBridgeStatus` had zero call sites: the Rust was complete, the IPC wrapper shipped, and there was nowhere to click — so the docs told users to "enable the Auth Bridge" for a switch that did not exist. `AuthBridgeRow` is that switch, in Config → Runtime. It deliberately does not go through the tab's stopped-only save: the dedicated command exists so the bridge can be flipped while a login is hanging in a running container, which is the only moment anyone reaches for it. It also subscribes to `auth-bridge-changed`, which the poller has been emitting to nobody — so a host port the bridge could not take was a completely silent failure, indistinguishable from a login that hung. `tunnel.rs` promotes the best-effort `::1` bind failure from debug to a warning recorded on the port. Half-bound is the failure mode that looks like success: the status says bridged, and a client that resolves `localhost` to `::1` without falling back is still refused. Finally, for a recognised Anthropic sign-in URL the toast now leads with "In container" and demotes the host "Open". The callback listener is inside the container, so the container-side browser closes the loop with no host round trip and no auth bridge; the host button stays as the fallback. Ordinary URLs are unchanged. Tests: 402 frontend (was 359), 285 Rust (unchanged). Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01GBq2rGum6GX7xXgsas1fDc --- HOW-TO-USE.md | 44 +++- README.md | 17 +- app/src-tauri/src/auth_bridge/mod.rs | 5 + app/src-tauri/src/auth_bridge/tunnel.rs | 49 +++- app/src-tauri/src/web_terminal/terminal.html | 33 ++- app/src/components/layout/StatusBar.tsx | 13 + .../home/config/AuthBridgeRow.test.tsx | 178 ++++++++++++++ .../projects/home/config/AuthBridgeRow.tsx | 198 ++++++++++++++++ .../home/config/RuntimeSection.test.tsx | 39 ++- .../projects/home/config/RuntimeSection.tsx | 7 + .../components/terminal/TerminalView.test.tsx | 224 ++++++++++++++++++ app/src/components/terminal/TerminalView.tsx | 148 ++++++++++-- app/src/components/terminal/UrlToast.test.tsx | 75 ++++++ app/src/components/terminal/UrlToast.tsx | 161 +++++++++---- app/src/hooks/useClaudeAuth.ts | 7 +- app/src/lib/tauri-commands.ts | 9 +- app/src/lib/types.ts | 5 + app/src/lib/urlDetector.test.ts | 194 ++++++++++++++- app/src/lib/urlDetector.ts | 162 ++++++++++++- app/src/lib/urlRelay.ts | 36 +++ container/entrypoint.sh | 23 ++ 21 files changed, 1529 insertions(+), 98 deletions(-) create mode 100644 app/src/components/projects/home/config/AuthBridgeRow.test.tsx create mode 100644 app/src/components/projects/home/config/AuthBridgeRow.tsx create mode 100644 app/src/components/terminal/TerminalView.test.tsx diff --git a/HOW-TO-USE.md b/HOW-TO-USE.md index 643b022..df4a4e0 100644 --- a/HOW-TO-USE.md +++ b/HOW-TO-USE.md @@ -128,8 +128,11 @@ Anthropic-backend project uses that token without its own login. See 2. Claude prints an OAuth URL. Triple-C detects long URLs and shows a clickable toast at the top of the terminal — click **Open** to open it in your browser. 3. Complete the login in your browser. The token is saved and persists across container stops, starts and recreations. A **Reset** deletes it — see below. -> If the login hangs after the browser step, the callback could not reach the container. Enable the -> [Auth Bridge](#browser-logins-inside-the-container-auth-bridge) for that project. +> If the login hangs after the browser step, the callback could not reach the container. Either +> click **In container** on the toast instead of **Open** — the callback then never has to leave the +> container at all — or turn on the +> [Auth Bridge](#browser-logins-inside-the-container-auth-bridge) in the project's +> **Config → Runtime** section. **AWS Bedrock:** @@ -789,6 +792,19 @@ web server they started on `localhost`. `claude login`, `aws sso login` and Conc The **Auth Bridge** fixes this. It is **opt-in per project** and **off by default**. +### Where the switch is + +Project Home → **Config** → **Runtime** → **Auth bridge**. + +Unlike the rest of that tab, it is **not** greyed out while the container is running — it is a +host-side feature that recreates nothing, and the moment you want it is usually the moment a login +is already hanging in a running container. Switch it on, then retry the login. + +Beside the switch is its live state: **Off**, **Watching** (on, nothing to bridge yet — normal, +there is only something to bridge while a login is waiting), **Bridging *n* ports**, **IPv4 only**, +or **Port conflict** with the port and the reason. A conflict means the host port was already taken +and the callback will not arrive; free the port, or use **In container** instead. + ### What it does - Every couple of seconds it looks inside the container for programs listening on the container's @@ -1293,9 +1309,19 @@ triple-c-scheduler add --name "test" --schedule "0 */6 * * *" --prompt "Run test | **Ctrl+Shift+V** | Paste | | **Ctrl+V** | Paste an image from the clipboard into the container | | **Ctrl+Shift+M** | Toggle speech-to-text recording (when enabled) | +| **Shift+Enter** | Insert a newline in Claude Code's prompt instead of submitting it | +| **Alt+Enter** | The same thing, and it has always worked — it was simply never written down | Everything else goes straight through to the program running in the container. +> **Shift+Enter** sends `ESC` + `CR`, the same bytes Claude Code's own `/terminal-setup` installs +> for VS Code, Cursor, Alacritty and Zed — so there is nothing to run and no tip to follow. It is +> bound in **Claude** tabs only: in a **bash** tab that sequence means nothing to readline, and +> Shift+Enter there submits the line as it always has. +> +> In the [Web Terminal](#web-terminal-remote-access) the same chord works, and there is an **↵+** +> key beside **Enter** on the mobile key row for devices with no Shift. + --- ## What's Inside the Container @@ -1402,8 +1428,18 @@ your machine (anything that isn't `http`/`https`). You opened the URL, signed in successfully, and the CLI in the terminal is still waiting. The callback from your browser is landing on your host's `localhost` while the CLI is listening on the -*container's*. Enable the -[Auth Bridge](#browser-logins-inside-the-container-auth-bridge) for that project and try again. +*container's*. + +Two ways out, in order of least effort: + +1. Dismiss and re-trigger the login, then click **In container** on the toast rather than **Open**. + The page opens in a browser *inside* the container, so the callback never has to cross to the + host. This needs no auth bridge — only a running container with Playwright installed (Project + Home → **Browser**). For a recognised Anthropic sign-in link this is already the default button. +2. Turn on the [Auth Bridge](#browser-logins-inside-the-container-auth-bridge) — Project Home → + **Config** → **Runtime** → **Auth bridge** — and try again. It can be switched on while the + container is running. Check the indicator beside it: **Port conflict** means the host port was + already taken and the callback still will not arrive. For Claude specifically, the simpler answer is usually [Shared Claude Authentication](#shared-claude-authentication), which finishes on an Anthropic-hosted diff --git a/README.md b/README.md index 61359a2..4845abf 100644 --- a/README.md +++ b/README.md @@ -76,8 +76,21 @@ Implemented in `hooks/useKeyboardShortcuts.ts` (document-level, capture phase): `Ctrl+W` is deliberately **not** bound: it is readline's `kill-word`, used constantly in the terminal this app is built around. Plain `Ctrl+←/→` is readline's word-wise cursor motion, which is -why moving a tab takes Shift as well. Terminal-scoped keys (`Ctrl+Shift+C`, `Ctrl+Shift+Alt+C`, -`Ctrl+Shift+M`) are handled in `TerminalView.tsx`. +why moving a tab takes Shift as well. + +Terminal-scoped keys are handled in `TerminalView.tsx`: + +| Shortcut | Action | +|---|---| +| `Ctrl+Shift+C` / `Ctrl+Shift+Alt+C` | Copy the selection, trimmed / exactly as-is | +| `Ctrl+Shift+M` | Toggle speech-to-text recording | +| `Shift+Enter` | Insert a newline in Claude Code's prompt instead of submitting | +| `Alt+Enter` | The same thing — xterm.js already ESC-prefixes on Alt, so this has always worked | + +`Shift+Enter` sends `ESC` + `CR`, which is what Claude Code's own `/terminal-setup` installs for +VS Code, Cursor, Alacritty and Zed. It is bound in Claude sessions only: in a bash tab those bytes +are unbound in readline. The web terminal does the same, and adds an `↵+` key beside Enter for +devices with no Shift. ### Project Home diff --git a/app/src-tauri/src/auth_bridge/mod.rs b/app/src-tauri/src/auth_bridge/mod.rs index 3f89ba3..bc2b7e0 100644 --- a/app/src-tauri/src/auth_bridge/mod.rs +++ b/app/src-tauri/src/auth_bridge/mod.rs @@ -82,6 +82,10 @@ pub struct BridgedPort { pub family: PortFamily, /// RFC 3339 timestamp of when the host listener was bound. pub bridged_at: String, + /// Set when only the IPv4 half of the host listener could be bound. The + /// port still works, but not for a client that insists on `::1` — see + /// [`tunnel::PortForward::ipv6_warning`]. + pub ipv6_warning: Option, } /// A loopback listener that was discovered but could not be bridged. @@ -132,6 +136,7 @@ impl BridgeState { port: f.port, family: f.family, bridged_at: f.bridged_at.clone(), + ipv6_warning: f.ipv6_warning.clone(), }) .collect(), conflicts: self diff --git a/app/src-tauri/src/auth_bridge/tunnel.rs b/app/src-tauri/src/auth_bridge/tunnel.rs index 0ee6ecf..3882597 100644 --- a/app/src-tauri/src/auth_bridge/tunnel.rs +++ b/app/src-tauri/src/auth_bridge/tunnel.rs @@ -52,6 +52,15 @@ pub struct PortForward { pub port: u16, pub family: PortFamily, pub bridged_at: String, + /// Why `[::1]` could not be taken alongside `127.0.0.1`, if it could not. + /// + /// A half-bound forward is the one failure mode that looks like a success: + /// the status says the port is bridged, and a browser that resolves + /// `localhost` to `::1` and does not fall back still gets a refused + /// connection. It is not a conflict — the IPv4 half really is carrying + /// traffic — so it rides along with the port it belongs to and the UI says + /// so, rather than being logged at debug where nobody sees it. + pub ipv6_warning: Option, task: JoinHandle<()>, } @@ -86,18 +95,33 @@ impl PortForward { // first, so a v4-only host listener would miss those callbacks. This is // best-effort: if ::1 is unavailable (no IPv6, or that half is taken) // the v4 listener alone still works, so it is not treated as a conflict. - let v6 = match TcpListener::bind(SocketAddr::from((Ipv6Addr::LOCALHOST, port))).await { - Ok(l) => Some(l), - Err(e) => { - log::debug!( - "Auth bridge: bound 127.0.0.1:{} but not [::1]:{} ({}) — continuing with IPv4 only", - port, - port, - e - ); - None - } - }; + let (v6, ipv6_warning) = + match TcpListener::bind(SocketAddr::from((Ipv6Addr::LOCALHOST, port))).await { + Ok(l) => (Some(l), None), + Err(e) => { + // Warn, not debug. Best-effort is about whether to *fail*, + // not about whether to say anything: on a host where + // `localhost` resolves to `::1` and the client does not + // fall back to IPv4, the callback is refused while the + // bridge reports itself healthy — a silent failure with no + // thread back to this line. + log::warn!( + "Auth bridge: bound 127.0.0.1:{} but not [::1]:{} ({}) — continuing with IPv4 only; \ + a client that resolves localhost to ::1 without falling back will not reach it", + port, + port, + e + ); + ( + None, + Some(format!( + "IPv4 only — [::1]:{} could not be bound ({}). A browser that resolves \ + localhost to ::1 without falling back will not reach this port.", + port, e + )), + ) + } + }; let target = family.socat_target(port); let task = tokio::spawn(accept_loop(container_id, port, target, v4, v6)); @@ -106,6 +130,7 @@ impl PortForward { port, family, bridged_at: chrono::Utc::now().to_rfc3339(), + ipv6_warning, task, }) } diff --git a/app/src-tauri/src/web_terminal/terminal.html b/app/src-tauri/src/web_terminal/terminal.html index cd06179..1ca555d 100644 --- a/app/src-tauri/src/web_terminal/terminal.html +++ b/app/src-tauri/src/web_terminal/terminal.html @@ -334,6 +334,9 @@ autocomplete="off" autocorrect="off" autocapitalize="off" spellcheck="false" enterkeyhint="send" inputmode="text"> + + @@ -360,6 +363,7 @@ const emptyState = document.getElementById('emptyState'); const mobileInput = document.getElementById('mobileInput'); const btnEnter = document.getElementById('btnEnter'); + const btnNewline = document.getElementById('btnNewline'); const btnTab = document.getElementById('btnTab'); const btnCtrlC = document.getElementById('btnCtrlC'); const scrollBottomBtn = document.getElementById('scrollBottomBtn'); @@ -647,6 +651,28 @@ }); }); + // Shift+Enter inserts a newline in Claude Code's prompt instead of + // submitting it. xterm.js does not consult `shiftKey` for Enter, so + // without this Shift+Enter is byte-identical to Enter. + // + // `\x1b\r` — ESC then CR — is what Claude Code parses as `return` with + // meta, and it is the same sequence its own `/terminal-setup` installs for + // VS Code, Cursor, Alacritty and Zed. Do not "simplify" it to `\n`: that + // also works in Claude Code, but a shell would *run* the line, so the two + // session types would diverge. Claude sessions only, for that reason — + // `bash -l` has no readline binding for `\e\r`. + term.attachCustomKeyEventHandler(e => { + if ( + e.type === 'keydown' && e.key === 'Enter' && e.shiftKey && + !e.ctrlKey && !e.altKey && !e.metaKey && !e.isComposing && + sessionType === 'claude' + ) { + sendTerminalInput('\x1b\r'); + return false; // xterm must not also send a bare CR, which submits + } + return true; + }); + // Track scroll position for scroll-to-bottom button term.onScroll(() => updateScrollButton()); @@ -799,7 +825,11 @@ sendTerminalInput(val); mobileInput.value = ''; } - sendTerminalInput('\r'); + // Shift+Enter is a newline, not a submit — same bytes, and the same + // reasoning, as the terminal's own key handler above. A hardware + // keyboard on a tablet is the only way to reach this; the phone case is + // the dedicated newline button beside Enter. + sendTerminalInput(e.shiftKey ? '\x1b\r' : '\r'); } else if (e.key === 'Tab') { e.preventDefault(); sendTerminalInput('\t'); @@ -807,6 +837,7 @@ }); btnEnter.onclick = () => { sendTerminalInput('\r'); mobileInput.focus(); }; + btnNewline.onclick = () => { sendTerminalInput('\x1b\r'); mobileInput.focus(); }; btnTab.onclick = () => { sendTerminalInput('\t'); mobileInput.focus(); }; btnCtrlC.onclick = () => { sendTerminalInput('\x03'); mobileInput.focus(); }; diff --git a/app/src/components/layout/StatusBar.tsx b/app/src/components/layout/StatusBar.tsx index d52cca8..4b10d86 100644 --- a/app/src/components/layout/StatusBar.tsx +++ b/app/src/components/layout/StatusBar.tsx @@ -23,6 +23,11 @@ export default function StatusBar({ stt }: Props) { })) ); const running = projects.filter((p) => p.status === "running").length; + // Only in a Claude tab: the chord is bound there and nowhere else, and a hint + // for a key that does nothing is worse than no hint. + const inClaudeSession = sessions.some( + (s) => s.id === activeSessionId && s.sessionType === "claude", + ); return (
@@ -45,6 +50,14 @@ export default function StatusBar({ stt }: Props) { )} + {!terminalHasSelection && inClaudeSession && ( + <> + | + + Shift+Enter: newline + + + )} {/* Right-aligned controls: Jump to Current + STT mic */}
{activeSessionId && !terminalAtBottom && ( diff --git a/app/src/components/projects/home/config/AuthBridgeRow.test.tsx b/app/src/components/projects/home/config/AuthBridgeRow.test.tsx new file mode 100644 index 0000000..12e0199 --- /dev/null +++ b/app/src/components/projects/home/config/AuthBridgeRow.test.tsx @@ -0,0 +1,178 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { render, screen, fireEvent, waitFor } from "@testing-library/react"; +import AuthBridgeRow, { bridgeIndicator } from "./AuthBridgeRow"; +import type { AuthBridgeStatus, Project } from "../../../../lib/types"; + +/** + * The bridge shipped with a working backend, a typed IPC wrapper, and no way to + * reach either: `setAuthBridgeEnabled` had zero call sites, and the + * `auth-bridge-changed` event had no listener — so a host port the bridge could + * not take was a silent failure that presented as a login that simply hung. + * These tests hold both halves down. + */ + +const getAuthBridgeStatus = vi.fn<() => Promise>(); +const setAuthBridgeEnabled = vi.fn<(id: string, on: boolean) => Promise>(); + +vi.mock("../../../../lib/tauri-commands", () => ({ + getAuthBridgeStatus: () => getAuthBridgeStatus(), + setAuthBridgeEnabled: (id: string, on: boolean) => setAuthBridgeEnabled(id, on), +})); + +/** Captured so a test can push an `auth-bridge-changed` payload by hand. */ +let emit: ((payload: unknown) => void) | null = null; + +vi.mock("@tauri-apps/api/event", () => ({ + listen: vi.fn(async (_name: string, handler: (e: { payload: unknown }) => void) => { + emit = (payload) => handler({ payload }); + return () => { + emit = null; + }; + }), +})); + +const OFF: AuthBridgeStatus = { enabled: false, active_ports: [], conflicts: [] }; + +const project = { + id: "p1", + name: "api", + status: "running", + auth_bridge_enabled: false, +} as unknown as Project; + +beforeEach(() => { + vi.clearAllMocks(); + getAuthBridgeStatus.mockResolvedValue(OFF); + setAuthBridgeEnabled.mockResolvedValue({ ...OFF, enabled: true }); +}); + +describe("AuthBridgeRow", () => { + it("turns the bridge on through its own command, not the project save", async () => { + // The dedicated command exists so this can be flipped while the container + // runs — which is exactly when a user discovers they need it. Routing it + // through the Config tab's stopped-only save would make it unreachable at + // the only moment it matters. + render(); + await waitFor(() => expect(getAuthBridgeStatus).toHaveBeenCalled()); + + fireEvent.click(screen.getByRole("switch", { name: "Auth bridge" })); + + await waitFor(() => + expect(setAuthBridgeEnabled).toHaveBeenCalledWith("p1", true), + ); + }); + + it("stays usable while the container is running", async () => { + render(); + await waitFor(() => expect(getAuthBridgeStatus).toHaveBeenCalled()); + expect(screen.getByRole("switch", { name: "Auth bridge" })).not.toBeDisabled(); + }); + + it("reports a port conflict the poller emitted", async () => { + getAuthBridgeStatus.mockResolvedValue({ ...OFF, enabled: true }); + render(); + await waitFor(() => expect(emit).not.toBeNull()); + + emit!({ + project_id: "p1", + status: { + enabled: true, + active_ports: [], + conflicts: [ + { port: 54545, reason: "Host port 54545 is already in use (…); not bridged." }, + ], + }, + }); + + expect(await screen.findByText(/Port 54545/)).toBeInTheDocument(); + expect(screen.getByText("Port conflict")).toBeInTheDocument(); + }); + + it("ignores an event for a different project", async () => { + getAuthBridgeStatus.mockResolvedValue({ ...OFF, enabled: true }); + render(); + await waitFor(() => expect(emit).not.toBeNull()); + + emit!({ + project_id: "other", + status: { enabled: true, active_ports: [], conflicts: [{ port: 1, reason: "nope" }] }, + }); + + expect(screen.queryByText(/Port 1:/)).not.toBeInTheDocument(); + }); + + it("puts the switch back if the command rejects", async () => { + setAuthBridgeEnabled.mockRejectedValue("Project p1 not found"); + render(); + await waitFor(() => expect(getAuthBridgeStatus).toHaveBeenCalled()); + + fireEvent.click(screen.getByRole("switch", { name: "Auth bridge" })); + + expect(await screen.findByText(/not found/)).toBeInTheDocument(); + expect(screen.getByRole("switch", { name: "Auth bridge" })).not.toBeChecked(); + }); +}); + +describe("bridgeIndicator", () => { + // Every branch is a glyph plus a word — status is never colour alone. + it("says nothing is on when it is off", () => { + expect(bridgeIndicator(OFF, true)).toEqual({ tone: "off", label: "Off" }); + }); + + it("puts a conflict ahead of everything else", () => { + expect( + bridgeIndicator( + { + enabled: true, + active_ports: [ + { port: 1, family: "v4", bridged_at: "", ipv6_warning: null }, + ], + conflicts: [{ port: 2, reason: "taken" }], + }, + true, + ).tone, + ).toBe("error"); + }); + + it("flags a port that only took the IPv4 half", () => { + // Node resolves `localhost` to IPv6 first on Linux, so a v4-only listener + // is a callback that never arrives in front of a bridge reporting healthy. + expect( + bridgeIndicator( + { + enabled: true, + active_ports: [ + { port: 1, family: "v6", bridged_at: "", ipv6_warning: "no ::1" }, + ], + conflicts: [], + }, + true, + ).label, + ).toBe("IPv4 only"); + }); + + it("counts the ports it is holding", () => { + expect( + bridgeIndicator( + { + enabled: true, + active_ports: [ + { port: 1, family: "v4", bridged_at: "", ipv6_warning: null }, + { port: 2, family: "v4", bridged_at: "", ipv6_warning: null }, + ], + conflicts: [], + }, + true, + ).label, + ).toBe("Bridging 2 ports"); + }); + + it("says it is waiting when the container is not running", () => { + // Enabled and holding nothing is normal; enabled with no container is a + // different thing, and saying so stops it reading as a failure. + expect(bridgeIndicator({ ...OFF, enabled: true }, false).label).toBe( + "Waiting for the container", + ); + expect(bridgeIndicator({ ...OFF, enabled: true }, true).label).toBe("Watching"); + }); +}); diff --git a/app/src/components/projects/home/config/AuthBridgeRow.tsx b/app/src/components/projects/home/config/AuthBridgeRow.tsx new file mode 100644 index 0000000..ea8bc92 --- /dev/null +++ b/app/src/components/projects/home/config/AuthBridgeRow.tsx @@ -0,0 +1,198 @@ +import { useCallback, useEffect, useState } from "react"; +import { listen } from "@tauri-apps/api/event"; +import { + getAuthBridgeStatus, + setAuthBridgeEnabled, +} from "../../../../lib/tauri-commands"; +import type { + AuthBridgeChangedEvent, + AuthBridgeStatus, + Project, +} from "../../../../lib/types"; +import { SwitchRow } from "../../../ui/Field"; +import StatusIndicator, { type StatusTone } from "../../../ui/StatusIndicator"; +import Toggle from "../../../ui/Toggle"; + +/** Emitted by `auth_bridge/mod.rs` whenever the port or conflict set changes. */ +const AUTH_BRIDGE_EVENT = "auth-bridge-changed"; + +const LABEL = "Auth bridge"; + +/** + * What the indicator beside the switch says. + * + * Split out so the interesting part — that a conflict is a *visible* failure — + * can be tested without a container. Every branch pairs a glyph with a word; + * none of them are distinguished by colour alone. + */ +export function bridgeIndicator( + status: AuthBridgeStatus | null, + containerRunning: boolean, +): { tone: StatusTone; label: string } { + if (!status) return { tone: "unknown", label: "Checking" }; + if (!status.enabled) return { tone: "off", label: "Off" }; + // A conflict means a login is in progress and its port could not be taken — + // the one state where doing nothing is the wrong answer, and until now the + // one state nothing in the app reported at all. + if (status.conflicts.length > 0) { + return { tone: "error", label: "Port conflict" }; + } + if (status.active_ports.some((p) => p.ipv6_warning)) { + return { tone: "busy", label: "IPv4 only" }; + } + if (status.active_ports.length > 0) { + const n = status.active_ports.length; + return { tone: "running", label: `Bridging ${n} port${n === 1 ? "" : "s"}` }; + } + // Enabled but holding nothing. Normal: there is only something to bridge + // while a login is actually waiting for a callback. + if (!containerRunning) { + return { tone: "stopped", label: "Waiting for the container" }; + } + return { tone: "ok", label: "Watching" }; +} + +/** + * The switch for `auth_bridge_enabled`, and the only place it can be changed. + * + * Two things here are deliberate and easy to undo by accident: + * + * - **It does not go through the Config tab's `save`.** That path is gated on + * a stopped container, because almost everything else in the tab is baked + * into the container at creation. This is not: the bridge is entirely + * host-side, and `set_auth_bridge_enabled` exists precisely so it can be + * flipped *while a login is hanging*, which is when the user finds out they + * need it. Routing it through the generic save would make it unreachable at + * the only moment it matters. + * - **It subscribes to `auth-bridge-changed`.** The poller already emits the + * bridged-port and conflict sets on every change and, before this, nothing + * listened — so a host port the bridge could not take was a completely + * silent failure, indistinguishable from a login that simply hung. + */ +export default function AuthBridgeRow({ project }: { project: Project }) { + const projectId = project.id; + const containerRunning = project.status === "running"; + + const [status, setStatus] = useState(null); + const [busy, setBusy] = useState(false); + const [error, setError] = useState(null); + + useEffect(() => { + let cancelled = false; + setStatus(null); + setError(null); + getAuthBridgeStatus(projectId) + .then((s) => { + if (!cancelled) setStatus(s); + }) + .catch((e) => { + if (!cancelled) setError(String(e)); + }); + return () => { + cancelled = true; + }; + }, [projectId]); + + useEffect(() => { + let cancelled = false; + let unlisten: (() => void) | undefined; + listen(AUTH_BRIDGE_EVENT, (event) => { + if (event.payload.project_id !== projectId) return; + setStatus(event.payload.status); + }) + .then((un) => { + if (cancelled) un(); + else unlisten = un; + }) + .catch((e) => console.error("Auth bridge event subscription failed:", e)); + return () => { + cancelled = true; + unlisten?.(); + }; + }, [projectId]); + + const toggle = useCallback( + async (next: boolean) => { + setBusy(true); + setError(null); + // Optimistic, so the switch responds even though enabling has to await a + // container probe. The command's return value replaces it either way. + setStatus((s) => (s ? { ...s, enabled: next } : s)); + try { + setStatus(await setAuthBridgeEnabled(projectId, next)); + } catch (e) { + setStatus((s) => (s ? { ...s, enabled: !next } : s)); + setError(String(e)); + } finally { + setBusy(false); + } + }, + [projectId], + ); + + // Fall back to the persisted flag until the first status arrives, so the + // switch never renders in the wrong position. + const enabled = status?.enabled ?? project.auth_bridge_enabled; + const indicator = bridgeIndicator(status, containerRunning); + + return ( + + Mirrors a port a program inside the container is listening on onto the + host's 127.0.0.1, so a browser OAuth callback can reach + the listener waiting inside the container —{" "} + claude login, aws sso login and{" "} + gh auth login all work this way, and without it the + browser calls back into nothing and the login hangs. Host-side only: + it never recreates the container, and it can be switched on while one + is running. A bridged port is unauthenticated and reachable by any + local process for as long as the in-container listener exists, so + leave it off unless you need it. + + + {status?.active_ports.map((p) => ( + + 127.0.0.1:{p.port} + {p.ipv6_warning ? " (IPv4 only)" : ""} + + ))} + + {status?.conflicts.map((c) => ( + + Port {c.port}: {c.reason} + + ))} + {status?.active_ports + .filter((p) => p.ipv6_warning) + .map((p) => ( + + Port {p.port}: {p.ipv6_warning} + + ))} + {error && ( + {error} + )} + + } + control={ + + } + /> + ); +} diff --git a/app/src/components/projects/home/config/RuntimeSection.test.tsx b/app/src/components/projects/home/config/RuntimeSection.test.tsx index 8d3ab45..e8b7201 100644 --- a/app/src/components/projects/home/config/RuntimeSection.test.tsx +++ b/app/src/components/projects/home/config/RuntimeSection.test.tsx @@ -1,7 +1,21 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; -import { render, screen, fireEvent } from "@testing-library/react"; +import { render, screen, fireEvent, waitFor } from "@testing-library/react"; import RuntimeSection from "./RuntimeSection"; -import type { Project } from "../../../../lib/types"; +import type { AuthBridgeStatus, Project } from "../../../../lib/types"; + +// The auth-bridge row owns its own IPC — see `AuthBridgeRow.tsx` for why it +// does not go through `save`. +const OFF_BRIDGE: AuthBridgeStatus = { enabled: false, active_ports: [], conflicts: [] }; +const setAuthBridgeEnabled = vi.fn(async () => ({ ...OFF_BRIDGE, enabled: true })); + +vi.mock("../../../../lib/tauri-commands", () => ({ + getAuthBridgeStatus: vi.fn(async () => OFF_BRIDGE), + setAuthBridgeEnabled: (id: string, on: boolean) => setAuthBridgeEnabled(id, on), +})); + +vi.mock("@tauri-apps/api/event", () => ({ + listen: vi.fn(async () => () => {}), +})); const baseProject: Project = { id: "p1", @@ -92,3 +106,24 @@ describe("RuntimeSection — VPN support toggle", () => { ).toBeInTheDocument(); }); }); + +describe("RuntimeSection — auth bridge toggle", () => { + beforeEach(() => vi.clearAllMocks()); + + it("is reachable while the container is running", async () => { + // The rest of the tab is gated on a stopped container because those + // settings are baked in at creation. This one is host-side and has its own + // command, and the moment a user needs it is the moment a login is hanging + // in a *running* container — so the tab's `disabled` must not reach it. + renderSection({ status: "running" }, true); + + const toggle = screen.getByRole("switch", { name: "Auth bridge" }); + await waitFor(() => expect(toggle).not.toBeDisabled()); + + fireEvent.click(toggle); + await waitFor(() => expect(setAuthBridgeEnabled).toHaveBeenCalledWith("p1", true)); + // And never through the generic project save, which would drop it on the + // floor while the container runs. + expect(save).not.toHaveBeenCalled(); + }); +}); diff --git a/app/src/components/projects/home/config/RuntimeSection.tsx b/app/src/components/projects/home/config/RuntimeSection.tsx index f971c56..d9c80e1 100644 --- a/app/src/components/projects/home/config/RuntimeSection.tsx +++ b/app/src/components/projects/home/config/RuntimeSection.tsx @@ -4,6 +4,7 @@ import { ConfigGroup, SwitchRow } from "../../../ui/Field"; import PermissionModeControl, { permissionModePatch } from "../../PermissionModeControl"; import ClaudeInstructionsEditor from "../../ClaudeInstructionsEditor"; import ClaudeCodeSettingsEditor from "../../ClaudeCodeSettingsEditor"; +import AuthBridgeRow from "./AuthBridgeRow"; interface Props { project: Project; @@ -70,6 +71,12 @@ export default function RuntimeSection({ } /> + {/* Not gated on `disabled`: the bridge is host-side and has its own + command, so it can be switched on while a login is hanging — which + is the only moment anyone reaches for it. It owns its state rather + than going through `save`. */} + + {}); + +vi.mock("../../lib/tauri-commands", () => ({ + terminalInput: (sessionId: string, bytes: number[]) => + terminalInput(sessionId, bytes), + terminalResize: vi.fn(async () => {}), + pasteImageToTerminal: vi.fn(async () => ""), + openTerminalSession: vi.fn(async () => {}), + closeTerminalSession: vi.fn(async () => {}), + updateProject: vi.fn(async () => ({})), + awsSsoRefresh: vi.fn(async () => {}), + openPageInContainerBrowser: vi.fn(async () => ({ error: null })), + uploadHostFileToTerminal: vi.fn(async () => ""), +})); + +vi.mock("@tauri-apps/api/event", () => ({ + listen: vi.fn(async () => () => {}), +})); + +vi.mock("@tauri-apps/plugin-opener", () => ({ + openUrl: vi.fn(async () => {}), +})); + +vi.mock("@tauri-apps/api/webview", () => ({ + getCurrentWebview: () => ({ onDragDropEvent: vi.fn(async () => () => {}) }), +})); + +/** jsdom has no ResizeObserver, and the mount effect installs one. */ +class NoopResizeObserver { + observe() {} + unobserve() {} + disconnect() {} +} + +/** What `sendInput` put on the wire, decoded back to a string. */ +function sent(): string[] { + return terminalInput.mock.calls.map((call) => + new TextDecoder().decode(new Uint8Array((call as unknown as [string, number[]])[1])), + ); +} + +function mountSession(sessionType: "claude" | "bash") { + useAppState.setState({ + sessions: [ + { + id: "s1", + projectId: "p1", + projectName: "api", + sessionType, + sessionName: null, + }, + ], + }); + return render(); +} + +/** The hidden textarea xterm binds its keyboard handling to. */ +function helperTextarea(container: HTMLElement): HTMLTextAreaElement { + const el = container.querySelector( + "textarea.xterm-helper-textarea", + ); + if (!el) throw new Error("xterm helper textarea not found"); + return el; +} + +beforeEach(() => { + vi.stubGlobal("ResizeObserver", NoopResizeObserver); + // xterm's renderer asks the window for its device pixel ratio on open. + vi.stubGlobal( + "matchMedia", + (query: string) => ({ + matches: false, + media: query, + addEventListener() {}, + removeEventListener() {}, + addListener() {}, + removeListener() {}, + onchange: null, + dispatchEvent: () => false, + }), + ); + terminalInput.mockClear(); + useAppState.setState({ sessions: [] }); +}); + +afterEach(() => { + cleanup(); + vi.unstubAllGlobals(); +}); + +describe("TerminalView — Shift+Enter", () => { + it("sends ESC+CR and nothing else in a Claude session", () => { + const { container } = mountSession("claude"); + + fireEvent.keyDown(helperTextarea(container), { + key: "Enter", + keyCode: 13, + shiftKey: true, + }); + + // The bytes `/terminal-setup` installs for every other editor. + expect(sent()).toEqual(["\x1b\r"]); + // And specifically not the bare CR that would have submitted the prompt. + expect(sent()).not.toContain("\r"); + }); + + it("leaves a plain Enter alone", () => { + const { container } = mountSession("claude"); + + fireEvent.keyDown(helperTextarea(container), { key: "Enter", keyCode: 13 }); + + expect(sent()).toEqual(["\r"]); + }); + + it("does not bind it in a bash session", () => { + // `bash -l` runs readline, which has no binding for `\e\r`: it would answer + // with a bell and swallow the Enter the user actually pressed. + const { container } = mountSession("bash"); + + fireEvent.keyDown(helperTextarea(container), { + key: "Enter", + keyCode: 13, + shiftKey: true, + }); + + expect(sent()).toEqual(["\r"]); + }); + + it("leaves a modified Shift+Enter to xterm", () => { + // Adding Ctrl is not the chord this binds; whatever xterm does with it is + // xterm's business. + const { container } = mountSession("claude"); + + fireEvent.keyDown(helperTextarea(container), { + key: "Enter", + keyCode: 13, + shiftKey: true, + ctrlKey: true, + }); + + expect(sent()).not.toContain("\x1b\r"); + }); + + it("Alt+Enter already produced ESC+CR without any handler", () => { + // Pinned because it is the reason Shift+Enter was the only gap: xterm + // ESC-prefixes on `altKey` by itself, so Alt+Enter has always inserted a + // newline in Claude Code. It was simply undocumented. + const { container } = mountSession("bash"); // no custom branch involved + + fireEvent.keyDown(helperTextarea(container), { + key: "Enter", + keyCode: 13, + altKey: true, + }); + + expect(sent()).toEqual(["\x1b\r"]); + }); +}); + +describe("supersedes — who owns the prompt slot", () => { + const relay = (url: string) => ({ url, source: "relay" as const }); + const osc8 = (url: string) => ({ url, source: "osc8" as const }); + const guess = (url: string) => ({ url, source: "heuristic" as const }); + + const COMPLETE = + "https://claude.ai/oauth/authorize?code=true&client_id=abc123&response_type=code&redirect_uri=https%3A%2F%2Fconsole.anthropic.com%2Foauth%2Fcode%2Fcallback&scope=user%3Ainference"; + // What the screen-scraper reconstructs from the visible text: parses, points + // at the right host, authorises nothing. + const TRUNCATED = COMPLETE.slice(0, 80); + + it("fills an empty slot from anywhere", () => { + expect(supersedes(guess(TRUNCATED), null)).toBe(true); + }); + + it("refuses to let a truncated guess replace the exact copy", () => { + // The whole bug: the relay lands first with the complete URL, and 300 ms + // later the detector's debounce fires with a prefix of it. + expect(supersedes(guess(TRUNCATED), relay(COMPLETE))).toBe(false); + expect(supersedes(guess(TRUNCATED), osc8(COMPLETE))).toBe(false); + }); + + it("lets a better source take over from a worse one", () => { + expect(supersedes(osc8(COMPLETE), guess(TRUNCATED))).toBe(true); + expect(supersedes(relay(COMPLETE), guess(TRUNCATED))).toBe(true); + }); + + it("lets a scraped candidate grow into the complete link", () => { + // A repaint can land the truncated copy first. Extending it is safe: a + // longer string with the same prefix has the same origin. + expect(supersedes(guess(COMPLETE), guess(TRUNCATED))).toBe(true); + }); + + it("does not let an unrelated scrape displace what is on screen", () => { + // Longest-wins without the prefix test hands the choice to whoever pads + // their URL the most. + expect( + supersedes(guess("https://evil.tld/" + "a".repeat(400)), guess(COMPLETE)), + ).toBe(false); + }); + + it("lets a second explicit relay request through", () => { + // Each OSC 7777 is a fresh deliberate ask, not another view of the last + // one — a second `gh auth login` must be able to replace the first. + expect( + supersedes(relay("https://github.com/login/device"), relay(COMPLETE)), + ).toBe(true); + }); +}); diff --git a/app/src/components/terminal/TerminalView.tsx b/app/src/components/terminal/TerminalView.tsx index a151b76..0ffc40d 100644 --- a/app/src/components/terminal/TerminalView.tsx +++ b/app/src/components/terminal/TerminalView.tsx @@ -13,10 +13,11 @@ import { uploadHostFileToTerminal, } from "../../lib/tauri-commands"; import { getCurrentWebview } from "@tauri-apps/api/webview"; -import { UrlDetector } from "../../lib/urlDetector"; +import { UrlDetector, type UrlSource } from "../../lib/urlDetector"; import { RelayRateLimiter, URL_RELAY_OSC, + extendsUrl, parseUrlRelayOsc, sanitizeRelayUrl, } from "../../lib/urlRelay"; @@ -29,6 +30,58 @@ interface Props { active: boolean; } +/** + * Where a prompted URL came from. + * + * `relay` is the container asking explicitly, over OSC 7777, with the URL + * base64-encoded — exact by construction. `osc8` is lifted verbatim out of a + * hyperlink parameter — also exact, but nobody asked for it. `heuristic` was + * reassembled from painted text and is the only one that can be a *truncated + * guess* at the link it is showing. + */ +export type PromptSource = "relay" | UrlSource; + +/** Higher wins. Provenance, not recency. */ +const SOURCE_RANK: Record = { + heuristic: 0, + osc8: 1, + relay: 2, +}; + +/** + * Whether `next` may take over the prompt slot from `current`. + * + * The bug this exists for: `claude login` relays its OAuth URL over OSC 7777, + * base64-encoded and therefore complete; the screen-scraper's 300 ms debounce + * then fires, finds the same link cut into terminal-width pieces, and — under + * the old last-writer-wins slot — replaced the good URL with a truncated one + * that still parses, still points at the right host, and cannot authorise + * anything. The user is the one who has to notice. + * + * Two rules, in order: + * + * - Better provenance always wins, worse provenance never does. A scraped + * guess cannot displace an exact copy. + * - Between equals, only an *extension* of what is showing may replace it. + * That is {@link extendsUrl}, the same rule and the same reasoning as + * `pickSignInUrl` in `hooks/useClaudeAuth.ts`: a repaint can land a + * truncated copy before the complete one, and a longer string sharing a + * prefix cannot move the origin. The relay is exempt because each OSC 7777 + * is a fresh deliberate request rather than another view of the last one — + * a second `gh auth login` must be able to replace the first. + */ +export function supersedes( + next: { url: string; source: PromptSource }, + current: { url: string; source: PromptSource } | null, +): boolean { + if (!current) return true; + if (SOURCE_RANK[next.source] !== SOURCE_RANK[current.source]) { + return SOURCE_RANK[next.source] > SOURCE_RANK[current.source]; + } + if (next.source === "relay") return true; + return extendsUrl(next.url, current.url); +} + export default function TerminalView({ sessionId, active }: Props) { const containerRef = useRef(null); const terminalContainerRef = useRef(null); @@ -47,13 +100,24 @@ export default function TerminalView({ sessionId, active }: Props) { (s) => s.sessions.find((sess) => sess.id === sessionId)?.projectId ); - // One toast slot, two producers: the heuristic long-URL detector and the - // container's explicit "open this in the host browser" relay (OSC 7777). - // Sharing the slot keeps them from stacking on top of each other. + // Which program is on the other end of the PTY. Read through a ref because + // the key handler is registered once, in the mount effect keyed on + // `sessionId`, and a value captured there would go stale if the session + // record arrived after the first render. + const sessionType = useAppState( + (s) => s.sessions.find((sess) => sess.id === sessionId)?.sessionType + ); + const sessionTypeRef = useRef(sessionType); + sessionTypeRef.current = sessionType; + + // One toast slot, three producers: the container's explicit "open this in the + // host browser" relay (OSC 7777), OSC 8 hyperlink targets, and the heuristic + // long-URL detector. Sharing the slot keeps them from stacking on top of each + // other. // - // Both producers read the container's PTY output, so both are untrusted, and - // both must go through `sanitizeRelayUrl` before anything is stored here — - // see `promptUrl` below, which is the only writer. + // All three read the container's PTY output, so all three are untrusted, and + // all three must go through `sanitizeRelayUrl` before anything is stored here + // — see `promptUrl` below, which is the only writer. // // `seq` exists because the slot is shared and long-lived: a second prompt // replacing a first would otherwise mutate the toast in place, swapping the @@ -62,6 +126,7 @@ export default function TerminalView({ sessionId, active }: Props) { const [urlPrompt, setUrlPrompt] = useState<{ url: string; label: string; + source: PromptSource; seq: number; } | null>(null); const promptSeqRef = useRef(0); @@ -72,16 +137,27 @@ export default function TerminalView({ sessionId, active }: Props) { * found: the OSC relay branch has already been through `parseUrlRelayOsc`, * but the heuristic detector branch has been through nothing at all, and a * raw regex match is exactly the input `sanitizeRelayUrl` exists to refuse. + * + * Last-writer-wins is what this used to be, and it lost the OAuth URL every + * time: the relay delivers the link base64-encoded and therefore exact, and + * ~300 ms later the screen-scraper's debounce fired and overwrote it with a + * truncated guess at the same link. `supersedes` is the fix — see there. */ - const promptUrl = useCallback((raw: string, label: string) => { - const url = sanitizeRelayUrl(raw); - if (!url) { - console.warn("Refusing to prompt for a URL that failed validation"); - return; - } - promptSeqRef.current += 1; - setUrlPrompt({ url, label, seq: promptSeqRef.current }); - }, []); + const promptUrl = useCallback( + (raw: string, label: string, source: PromptSource) => { + const url = sanitizeRelayUrl(raw); + if (!url) { + console.warn("Refusing to prompt for a URL that failed validation"); + return; + } + setUrlPrompt((current) => { + if (!supersedes({ url, source }, current)) return current; + promptSeqRef.current += 1; + return { url, label, source, seq: promptSeqRef.current }; + }); + }, + [], + ); const [imagePasteMsg, setImagePasteMsg] = useState(null); const [isAtBottom, setIsAtBottom] = useState(true); const [isAutoFollow, setIsAutoFollow] = useState(true); @@ -234,6 +310,34 @@ export default function TerminalView({ sessionId, active }: Props) { useAppState.getState().sttToggle(); return false; } + // Shift+Enter inserts a newline in Claude Code's prompt instead of + // submitting it. xterm.js does not consult `shiftKey` for Enter + // (`Keyboard.ts`, `case 13`), so without this branch Shift+Enter is + // byte-identical to Enter and submits. + // + // `\x1b\r` — ESC then CR — is what Claude Code parses as `return` with + // meta, and it is exactly what its own `/terminal-setup` writes into the + // VS Code, Cursor, Alacritty and Zed keymaps. These are the in-band + // bytes, not a guess, which is why this must NOT be "simplified" to + // `\n`: Claude Code accepts `\n` too, but a shell would *run* the line, + // so the two session types would quietly diverge. + // + // Scoped to Claude sessions for the same reason. A bash tab runs + // `bash -l`, where readline has no binding for `\e\r` and answers with a + // bell — harmless, but there is nothing to gain from sending it. + if ( + event.type === "keydown" && + event.key === "Enter" && + event.shiftKey && + !event.ctrlKey && + !event.altKey && + !event.metaKey && + !event.isComposing && + sessionTypeRef.current === "claude" + ) { + sendInput(sessionId, "\x1b\r"); + return false; // xterm must not also send a bare CR, which submits + } return true; }); @@ -287,7 +391,7 @@ export default function TerminalView({ sessionId, active }: Props) { console.warn("URL relay: rate-limited", url); return true; } - promptUrl(url, "Container asked to open a URL"); + promptUrl(url, "Container asked to open a URL", "relay"); return true; }); @@ -374,11 +478,17 @@ export default function TerminalView({ sessionId, active }: Props) { // Handle backend output -> terminal let aborted = false; - // The width is read per scan, not captured: only a break the terminal + // The detector samples this getter on every `feed`, so what it reassembles + // with is the width the bytes were *printed* at — only a break the terminal // itself inserted may be deleted, and where that is moves with every // resize. const detector = new UrlDetector( - (url) => promptUrl(url, "Long URL detected"), + (url, source) => + promptUrl( + url, + source === "osc8" ? "Link detected" : "Long URL detected", + source, + ), () => termRef.current?.cols ?? 0, ); detectorRef.current = detector; diff --git a/app/src/components/terminal/UrlToast.test.tsx b/app/src/components/terminal/UrlToast.test.tsx index 0660396..a991ef6 100644 --- a/app/src/components/terminal/UrlToast.test.tsx +++ b/app/src/components/terminal/UrlToast.test.tsx @@ -58,4 +58,79 @@ describe("UrlToast", () => { screen.getByRole("button", { name: "Open" }).click(); expect(onOpen).toHaveBeenCalledTimes(1); }); + + describe("Anthropic sign-in links", () => { + // The callback listener a `claude login` is waiting on is *inside* the + // container. Sending the user to their host browser completes the sign-in + // and then posts the result where nothing is listening, and the terminal + // hangs to its timeout — so for these, and only these, the container-side + // browser leads. + 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", () => { + render( + , + ); + expect(actions()).toEqual(["In container", "Open"]); + expect(screen.getByTestId("url-toast-signin-hint")).toHaveTextContent( + /callback listener is inside the container/i, + ); + }); + + it("keeps the host browser available as a fallback", () => { + const onOpen = vi.fn(); + render( + , + ); + 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. + render( + , + ); + 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( + , + ); + expect(actions()).toEqual(["Open", "In container"]); + }); + }); }); diff --git a/app/src/components/terminal/UrlToast.tsx b/app/src/components/terminal/UrlToast.tsx index 6874d31..0c17737 100644 --- a/app/src/components/terminal/UrlToast.tsx +++ b/app/src/components/terminal/UrlToast.tsx @@ -1,4 +1,5 @@ -import { urlOrigin } from "../../lib/urlRelay"; +import type { CSSProperties, MouseEvent } from "react"; +import { isAnthropicSignInUrl, urlOrigin } from "../../lib/urlRelay"; interface Props { /** Already validated by `sanitizeRelayUrl` — this component never opens it. */ @@ -28,6 +29,18 @@ interface Props { * is shared and long-lived, so without one React mutates the node in place: the * text swaps with no animation, and a user reading URL A can click Open on URL * B that arrived a second later. + * + * ## Anthropic sign-in links default to the container's browser + * + * For an ordinary URL the host browser is the right answer and stays the + * default. For a sign-in it is the *wrong* one: the callback listener the CLI + * is waiting on is inside the container, so a host browser completes the sign-in + * and then posts the result somewhere nothing is listening, and the terminal + * hangs until it times out. Making the host button primary there was quietly + * steering every user into that. The container-side browser closes the loop + * with no host round trip and no auth bridge, so it leads — and the host button + * stays, because a user who has the auth bridge on, or who wants their existing + * browser session, still needs it. */ export default function UrlToast({ url, @@ -38,6 +51,81 @@ export default function UrlToast({ }: Props) { const origin = urlOrigin(url); const rest = origin && url.startsWith(origin) ? url.slice(origin.length) : url; + // Only when there is somewhere to send it: without `onOpenInContainer` the + // host button is the only action there is, so it stays primary. + const signIn = !!onOpenInContainer && isAnthropicSignInUrl(url); + + // Filled uses `--accent-emphasis`, never `--accent` — the latter is the + // foreground/link accent and fails WCAG AA behind white text. + const primaryStyle: CSSProperties = { + padding: "4px 12px", + fontSize: 12, + fontWeight: 600, + color: "#fff", + background: "var(--accent-emphasis)", + border: "1px solid transparent", + borderRadius: 4, + cursor: "pointer", + whiteSpace: "nowrap", + flexShrink: 0, + }; + const secondaryStyle: CSSProperties = { + padding: "4px 10px", + fontSize: 12, + fontWeight: 600, + color: "var(--text-primary)", + background: "transparent", + border: "1px solid var(--border-color)", + borderRadius: 4, + cursor: "pointer", + whiteSpace: "nowrap", + flexShrink: 0, + }; + + /** Hover feedback for whichever button is currently the filled one. */ + const hover = (primary: boolean) => + primary + ? { + onMouseEnter: (e: MouseEvent) => + (e.currentTarget.style.background = "var(--accent-emphasis-hover)"), + onMouseLeave: (e: MouseEvent) => + (e.currentTarget.style.background = "var(--accent-emphasis)"), + } + : { + onMouseEnter: (e: MouseEvent) => + (e.currentTarget.style.background = "var(--bg-tertiary)"), + onMouseLeave: (e: MouseEvent) => + (e.currentTarget.style.background = "transparent"), + }; + + const hostButton = ( + + ); + + const containerButton = onOpenInContainer && ( + // A sign-in completed in the *container's* browser lands its callback on + // the container's own loopback, which is where the tool waiting for it is + // listening — no host round trip, no auth bridge. + + ); return (
+ {signIn && ( +
+ Sign-in link — the callback listener is inside the container. + Opening it there closes the loop; the host browser needs the auth + bridge. +
+ )}
- - - {onOpenInContainer && ( - // A sign-in completed in the *container's* browser lands its callback - // on the container's own loopback, which is where the tool waiting for - // it is listening — no host round trip, no auth bridge. - + {signIn ? ( + <> + {containerButton} + {hostButton} + + ) : ( + <> + {hostButton} + {containerButton} + )}