diff --git a/CLAUDE.md b/CLAUDE.md index f8c3033..9dae98a 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -104,6 +104,32 @@ docker exec stdout → tokio task → emit("terminal-output-{sessionId}") → li OAuth listener, wrong for remote control of a browser. Host ports are confined to `47820..=47827` because CSP `frame-src` cannot express a port range and must enumerate them; a unit test asserts the Rust range matches `tauri.conf.json`. Opt-in per project. + - **Detection has to look past `node_modules`.** `claude mcp add … npx @playwright/mcp@latest` + installs into `~/.npm/_npx//node_modules`, not any `node_modules`, so `detect.rs` + globs that cache as well as `/workspace`, `$HOME/node_modules` and `npm root -g`. It also + hops from a wrapper `playwright` to its **nested** `playwright-core`: verified that npm does + not hoist for global installs, and the wrapper ships no `types/types.d.ts`, so reading the + wrapper alone reports a current build as "predates `browser.bind()`". + - **`@playwright/mcp` can never satisfy this pane.** It bundles a `playwright-core` that binds, + but never `@playwright/cli`, which is the viewer. Never offer it as a setup route — only as + what binds sessions automatically once Playwright is present. + - **`install.rs` installs into `/workspace`, as `claude`, with `--no-save`.** `/workspace` is + *not* a bind mount — project directories are mounted at `/workspace/{mount_name}` — so this + touches nothing of the user's, needs no sudo (npm's prefix is `/usr`, which is root-owned), + and is on the module resolution path for scripts in the project. Browsers go to + `~/.cache/ms-playwright` as `claude`, i.e. the home volume. + - **Current base images ship Chromium's shared libraries; older ones do not** — and a project + keeps the base image it was first built from until it is migrated, so "older" is the normal + case. Without them `playwright install chromium` downloads a browser that cannot launch, which + is why installing Chrome via apt looks like a fix. `install.rs` asks + `install-deps --dry-run` first and skips the apt step when the answer is "all present", + *saying so* in the progress stream. Do not decide this by probing for library names: the + dry-run simulates the same `apt-get install` the fix would run, so check and fix cannot + disagree about what the dependency set is. Note that `--dry-run` exits **0** both when + everything is installed and when Playwright has no list for the platform — match on its + output, not its exit code. Either way the action ends by *actually launching* the browser to + verify. `@playwright/mcp` wants the `chrome` **channel** specifically, so both browsers are + offered. - **`docker/`** — Docker API layer using bollard: - `client.rs` — Singleton Docker connection via `OnceLock` - `container.rs` — Container lifecycle (create, start, stop, remove, inspect) @@ -134,7 +160,27 @@ docker exec stdout → tokio task → emit("terminal-output-{sessionId}") → li ### Container (`container/`) -- **`Dockerfile`** — Ubuntu 24.04 base with Claude Code, Node.js 22, Python 3.12, Rust, Docker CLI, git, gh, AWS CLI v2, ripgrep, pnpm, uv, ruff pre-installed +- **`Dockerfile`** — Ubuntu 24.04 base with Claude Code, Node.js 22, Python 3.12, Rust, Docker CLI, git, gh, AWS CLI v2, ripgrep, pnpm, uv, ruff pre-installed, plus the shared + libraries a browser links against (see below) +- **Browser runtime libraries are baked in; browser *binaries* are not.** A layer runs + `npx --yes playwright@latest install-deps chromium` as root, so Playwright names its own + dependencies and the list cannot rot against Ubuntu 24.04's `t64` renames or a new Chromium + dependency. Measured: +99 packages, +334 MiB unpacked / +119 MiB compressed, on both arches. Do + not replace it with a hand-written apt list without pinning the Playwright version you derived + it from — a `chromium`-only list saves ~94 MiB (Playwright's `tools` group: xvfb and the CJK + fonts) and nothing more, because `libgbm1` → `mesa-libgallium` → `libllvm20` is ~213 MiB that + no trimming removes. + - The `install-deps --dry-run` call after it is a **build-time assertion, not decoration**: on a + platform Playwright's table does not cover, `install-deps` prints a warning and returns having + installed nothing **with exit status 0**. Without the assertion that ships a broken image + behind a clean build log. + - Baking the libraries but not the browsers is the whole point of the split. Browsers live in + `~/.cache/ms-playwright` (home volume) and already survive recreation *and* migration; a + runtime `apt-get install` of the libraries lands in the writable layer, is re-paid after every + Reset, and is **lost on base-image migration**, which replays apt from a manifest. The runtime + approach converges on the worst state: a 400 MB browser present with its libraries gone. + - The layer sits immediately after Node (npx is its only prerequisite) and well above the shim + `COPY`s, so editing a shim does not re-run a multi-hundred-megabyte apt install. - **`entrypoint.sh`** — UID/GID remapping to match host user, SSH key setup, git config, docker socket permissions, Claude Code settings.json injection, then `sleep infinity` - **`triple-c-scheduler`** — Bash-based scheduled task system for recurring Claude Code invocations diff --git a/HOW-TO-USE.md b/HOW-TO-USE.md index 9ca7464..09c1979 100644 --- a/HOW-TO-USE.md +++ b/HOW-TO-USE.md @@ -509,6 +509,10 @@ This lives in the sidebar under **Settings → Claude Authentication**. code to copy — this flow finishes on an Anthropic-hosted page, not a local callback. 4. Paste the code back into Triple-C. The token is captured and written straight to the keychain. +The code is long and easy to truncate. If Anthropic refuses it, the dialog says so and lets you +paste another one without restarting the sign-in — the CLI is still waiting. After a few refusals +the flow gives up and reports it rather than sitting there. + Only one sign-in can run at a time, and the whole flow times out after 15 minutes. A long-lived token requires a Claude subscription; without one, `setup-token` finishes without printing a token and nothing is stored. @@ -1157,6 +1161,12 @@ The sandbox container (Ubuntu 24.04) comes pre-installed with: The container also includes **clipboard shims** (`xclip`, `xsel`, `pbcopy`) that forward copy operations to the host via OSC 52, a **browser shim** (`triple-c-open`, installed as `xdg-open`, `sensible-browser`, `www-browser`, `x-www-browser` and `$BROWSER`) that relays URLs to your host browser — see [Opening URLs in Your Browser](#opening-urls-in-your-browser-url-relay) — and an **audio shim** (`rec`, `arecord`) for future voice mode support. +It also ships the **system libraries a browser needs to run** (`libnss3`, `libgbm1`, `libatk*`, `libasound2t64`, `libcups2t64`, `libpango`, `libdrm2`, fonts, and the rest of the set Playwright asks for). So `npx playwright install chromium` gives you a browser that actually starts. Before these were baked in, that download succeeded and the browser then died with *"Host system is missing dependencies: libnss3.so"*, which is why `sudo apt install google-chrome-stable` looked like the cure — apt was quietly installing the same libraries as Chrome's own dependencies. + +The **browsers themselves are not pre-installed** — they are hundreds of megabytes and tied to the Playwright version you use. Install one with the Browser tab's setup buttons, or `npx playwright install chromium` in a terminal. They land in `~/.cache/ms-playwright`, which is on the home volume, so a browser survives container recreation and base-image migration and is only lost on a project **Reset**. + +If your project's container was created from an older base image, it won't have the libraries — the Browser tab's install action detects that and installs them for you first, and says so while it does. That install lives in the container's writable layer, so it is undone by a **Reset** and by a base-image migration; migrating the project onto the current base image is what picks the libraries up for good. + You can install additional tools at runtime with `sudo apt install`, `pip install`, `npm install -g`, etc. Installed packages persist across container stops (but not across resets). --- diff --git a/README.md b/README.md index 5208d32..552e044 100644 --- a/README.md +++ b/README.md @@ -386,4 +386,24 @@ Users can override this in Settings via the global `docker_socket_path` option. **Shims**: `xclip`/`xsel`/`pbcopy` (OSC 52 clipboard forwarding), `xdg-open`/`sensible-browser`/`www-browser`/`x-www-browser`/`$BROWSER` (OSC 7777 URL relay to the host browser), `rec`/`arecord` (audio FIFO for voice mode) +**Browser runtime libraries**: the shared libraries Chromium links against (`libnss3`, `libgbm1`, +`libatk*`, `libasound2t64`, `libcups2t64`, `libpango`, `libdrm2`, … plus fonts) are baked in, via +`npx playwright install-deps chromium` at build time. Without them `playwright install chromium` +downloads a browser that then dies at launch with *"Host system is missing dependencies: +libnss3.so"* — which is why installing `google-chrome-stable` used to look like the fix (apt was +pulling the libraries in as *its* dependencies). Measured cost of the layer: +99 packages, +**+334 MiB unpacked / +119 MiB compressed** (2950 → 3284 MiB unpacked, 759 → 878 MiB compressed). +Two thirds of that is not avoidable by trimming — `libgbm1`, which Chromium needs, depends on +`mesa-libgallium`, which depends on `libllvm20`. The list is taken from Playwright rather than +hand-written so it cannot rot against Ubuntu 24.04's `t64` renames or a future Chromium dependency, +and the `install-deps --dry-run` that follows it is a build-time assertion: on a platform +Playwright has no list for, `install-deps` installs nothing and still exits 0. + +**Browser binaries are deliberately not baked.** They are large, they are version-coupled to +whatever Playwright the user installs, and they already persist: `~/.cache/ms-playwright` is inside +the home volume, so a downloaded browser survives container recreation *and* base-image migration. +The libraries are the opposite — a runtime `apt-get install` lands in the container's writable +layer, is re-paid after every Reset, and is lost on migration (which replays apt from a manifest +against the new base). Baking one and not the other puts each half where it already persists. + **Default user**: `claude` (UID/GID 1000, remapped by entrypoint to match host) diff --git a/TECHNICAL.md b/TECHNICAL.md index 58d657c..d33a14d 100644 --- a/TECHNICAL.md +++ b/TECHNICAL.md @@ -245,9 +245,31 @@ minutes. - **Storage** — the OS keychain, under a dedicated service name; the token is never returned to the frontend, never written to a log, and no command accepts or returns it. +- **The sign-in URL comes from the OSC 8 parameter, not the screen.** The CLI emits the URL as a + hyperlink and slices the *visible* text of it to the terminal width — measured against 2.1.226, a + 346-character URL arrives at 80 columns as five separate hyperlink emissions, each carrying the + whole URL in its parameter and 80 characters of it on screen. Scraping the visible text yields a + URL that parses, points at `claude.com`, and cannot authorise anything, so the ANSI stripper + surfaces the hyperlink target and `claude-token-link` carries it to the UI. The frontend applies + the `ANTHROPIC_SIGN_IN_HOSTS` allowlist to it before display and again before `openUrl` — an OSC 8 + parameter is container output that is never rendered, which makes it the *easier* place to hide a + hostile host, not a trusted one. `stty cols 400` (up from 200, which the URL still overflowed) + removes wrapping as a variable elsewhere, but it is not the fix: that line fails silently. +- **A rejected code is recoverable, not a hang.** On a bad paste the CLI prints + `OAuth error: Invalid code…` / `Press Enter to retry.` and blocks on stdin rather than exiting. + The streamed output is scanned for that, `claude-token-code-rejected` reopens the input with an + explanation, and the Enter is sent so the next code has a prompt to land in — bounded by + `MAX_CODE_ATTEMPTS`, after which the flow reports a failure. Without this the exec sat until the + 15-minute timeout with the UI still saying "Finishing sign-in". - **Redaction** — streamed output is stripped of ANSI sequences and passed through a stateful redactor that masks anything matching `sk-ant-` with a plausible body, withholding any tail that - could still grow into a secret across a chunk boundary. + could still grow into a secret across a chunk boundary. A credential split across a hard line + wrap is reassembled by both the parser and the redactor from the same `scan_credential_body`, so + the two cannot disagree about where a credential ends — previously a wrapped token was rejected + as too short *and* its second line, which carries no `sk-ant-` marker, was printed to the UI in + clear. A run is only joined across a break that sits at a plausible terminal margin and is not + already long enough to be a whole credential; otherwise a repainting TUI would weld one frame's + token onto the next frame's first word. - **Injection** — `CLAUDE_CODE_OAUTH_TOKEN` is set only when the backend is Anthropic, the project has not opted out (`use_shared_auth_token`, default `true`), and a non-blank token is stored. When those conditions do not hold, the variable is explicitly set to empty rather than omitted, so a diff --git a/app/src-tauri/src/browser_view/commands.rs b/app/src-tauri/src/browser_view/commands.rs index b9f5efb..293b0f8 100644 --- a/app/src-tauri/src/browser_view/commands.rs +++ b/app/src-tauri/src/browser_view/commands.rs @@ -4,6 +4,7 @@ use tauri::{AppHandle, State}; +use crate::browser_view::install::{self, BrowserSetupOutcome}; use crate::browser_view::{manager, BrowserViewStatus}; use crate::AppState; @@ -27,20 +28,7 @@ pub async fn set_browser_view_enabled( return Ok(manager().status(&project_id).await); } - let project = state - .projects_store - .get(&project_id) - .ok_or_else(|| format!("Project {} not found", project_id))?; - - let Some(container_id) = project.container_id.clone() else { - return Err("Start the container before opening the browser view.".to_string()); - }; - if !crate::docker::container::is_container_running(&container_id) - .await - .unwrap_or(false) - { - return Err("Start the container before opening the browser view.".to_string()); - } + let container_id = running_container(&state, &project_id, "opening the browser view").await?; manager() .start( @@ -61,18 +49,84 @@ pub async fn get_browser_view_status(project_id: String) -> Result, ) -> Result { - let project = state - .projects_store - .get(&project_id) - .ok_or_else(|| format!("Project {} not found", project_id))?; - let container_id = project - .container_id - .ok_or_else(|| "Start the container to check for Playwright.".to_string())?; + let container_id = running_container(&state, &project_id, "checking for Playwright").await?; crate::browser_view::detect::detect(&container_id).await } + +/// Install `playwright` and `@playwright/cli` into the container. +/// +/// **This mutates the container**, so it is a command of its own and is only +/// ever reached by the user pressing the button — nothing here runs on tab +/// open. Progress streams on `container-progress`; the outcome carries a fresh +/// probe so the pane updates itself. +/// +/// Browsers are *not* fetched here. They are hundreds of megabytes and get +/// their own action, with the size stated before the click. +#[tauri::command] +pub async fn install_browser_view_support( + project_id: String, + app_handle: AppHandle, + state: State<'_, AppState>, +) -> Result { + let container_id = running_container(&state, &project_id, "installing Playwright").await?; + install::install_packages(&app_handle, &project_id, &container_id).await +} + +/// Install a browser — `chromium` (Playwright's own build, for scripts that +/// call `chromium.launch()`) or `chrome` (the Google Chrome channel that +/// `@playwright/mcp` asks for) — along with the system libraries it needs, and +/// verify that it actually starts. +/// +/// Also a mutation, also user-initiated only. +#[tauri::command] +pub async fn install_browser_view_browser( + project_id: String, + browser: String, + app_handle: AppHandle, + state: State<'_, AppState>, +) -> Result { + let target = install::BrowserTarget::parse(&browser)?; + let container_id = running_container(&state, &project_id, "installing a browser").await?; + install::install_browser(&app_handle, &project_id, &container_id, target).await +} + +/// The project's container, or a sentence saying why there isn't one. +/// +/// Every command here needs a *running* container, and every one of them used +/// to be able to fail somewhere further in with a Docker error instead. The +/// `action` is folded into the message so "start the container first" arrives +/// attached to what the user was trying to do. +async fn running_container( + state: &State<'_, AppState>, + project_id: &str, + action: &str, +) -> Result { + let project = state + .projects_store + .get(project_id) + .ok_or_else(|| format!("Project {} not found", project_id))?; + + let Some(container_id) = project.container_id.clone() else { + return Err(format!( + "This project has no container yet. Start it before {}.", + action + )); + }; + if !crate::docker::container::is_container_running(&container_id) + .await + .unwrap_or(false) + { + return Err(format!( + "The container for “{}” isn't running. Start it before {}.", + project.name, action + )); + } + Ok(container_id) +} diff --git a/app/src-tauri/src/browser_view/detect.rs b/app/src-tauri/src/browser_view/detect.rs index 154f345..280343d 100644 --- a/app/src-tauri/src/browser_view/detect.rs +++ b/app/src-tauri/src/browser_view/detect.rs @@ -17,6 +17,22 @@ //! Discovery of published browsers is local-filesystem based (a cache directory //! plus a unix-socket singleton in the temp dir), which is exactly why the viewer //! has to run *in the container* next to the browsers rather than on the host. +//! +//! ## Where a Playwright can legitimately be +//! +//! `node_modules` is not the only answer, and assuming it was is what made this +//! probe lie. `claude mcp add … npx @playwright/mcp@latest` — the way most +//! people end up with Playwright in the container — installs nothing into any +//! `node_modules`: npx unpacks the tree into `~/.npm/_npx//node_modules` +//! and runs it from there. So that cache is searched too, every entry of it, +//! and [`PlaywrightDetection::searched`] echoes back every root actually +//! consulted so a "not found" is checkable rather than merely asserted. +//! +//! Note what that npx route can and cannot do: `@playwright/mcp` bundles a +//! `playwright-core` new enough to `bind()`, so it can satisfy points 1 and 2 — +//! but it never ships `@playwright/cli`, so it can never satisfy point 3 on its +//! own. Any message that offers it as a way to *set up* this pane is sending +//! the user down a dead end; see [`PlaywrightDetection::blocker`]. use serde::{Deserialize, Serialize}; @@ -39,6 +55,14 @@ pub struct PlaywrightDetection { /// Absolute path of the resolved package manifest, for the diagnostics line. #[serde(default)] pub playwright_path: Option, + /// Absolute path of the resolved Playwright's own CLI entry (`cli.js`). + /// + /// Both `playwright` and `playwright-core` declare one, and it is the thing + /// that installs browsers and their system libraries. Driving *that* file + /// with `node` — rather than whatever `playwright` happens to be on `PATH` — + /// is what keeps the browser install pinned to the copy this pane found. + #[serde(default)] + pub playwright_cli: Option, /// Whether the resolved build's type definitions declare `Browser.bind()`. #[serde(default)] pub has_bind: bool, @@ -50,6 +74,25 @@ pub struct PlaywrightDetection { /// we can signal. #[serde(default)] pub cli_entry: Option, + /// Browser bundles present in the Playwright browser cache + /// (`~/.cache/ms-playwright`), e.g. `chromium-1200`. `ffmpeg-*` is excluded + /// — it is not a browser and its presence must not read as one. + /// + /// Not part of [`PlaywrightDetection::is_usable`]: the viewer serves + /// whatever has been published to it, and a browser could in principle be + /// remote. It is here because "installed but no browser to drive" is a real + /// state the pane has to be able to say out loud. + #[serde(default)] + pub browsers: Vec, + /// Path to Google Chrome, if the `chrome` *channel* is installed. + /// + /// Separate from [`Self::browsers`] because it is not in Playwright's cache + /// at all — the channel is an apt package. It is tracked because + /// `@playwright/mcp` asks for `channel: 'chrome'` specifically, so a + /// container with the bundled Chromium and no Chrome is set up for the + /// user's own scripts and not for the MCP plugin. + #[serde(default)] + pub chrome_channel: Option, /// Where the probe looked, echoed back for the "not found" message. #[serde(default)] pub searched: Vec, @@ -63,6 +106,13 @@ impl PlaywrightDetection { /// A specific, actionable explanation of what is missing. `None` when the /// container is ready. + /// + /// Every branch names the *package* that is missing and points at this + /// pane's install action, because assembling npm commands by hand is the + /// thing that went wrong for real users. `@playwright/mcp` is named only in + /// the role it actually plays — it binds sessions automatically once + /// Playwright is present — and never as a route through setup, because it + /// does not ship `@playwright/cli` and so can never make the viewer work. pub fn blocker(&self) -> Option { if self.node_version.is_none() { return Some( @@ -72,41 +122,66 @@ impl PlaywrightDetection { } if self.playwright_version.is_none() { return Some(format!( - "Playwright isn't installed in this container. Install it with \ - `npm i -D playwright` (or `npm i -g playwright`), then have Claude call \ - `await browser.bind('claude')` after launching a browser — or use \ - `@playwright/mcp`, which binds automatically. Looked in: {}.", - if self.searched.is_empty() { - "the container's default module paths".to_string() - } else { - self.searched.join(", ") - } + "Playwright isn't installed in this container. Two packages are needed: \ + `playwright` (for the `browser.bind()` live-dashboard API) and \ + `@playwright/cli` (the viewer UI this pane embeds). Use “Set up Playwright” \ + below to install both into the container. Installing `@playwright/mcp` on \ + its own is not enough — it binds sessions for you once Playwright is there, \ + but it never provides the viewer. Looked in: {}.", + self.searched_text() )); } if !self.has_bind { return Some(format!( - "Playwright {} is installed, but it predates the live-dashboard API \ - (`browser.bind()`). Upgrade with `npm i -D playwright@latest` and restart \ - the browser Claude is driving.", - self.playwright_version.as_deref().unwrap_or("?") + "Playwright {} is installed{}, but it predates the live-dashboard API \ + (`browser.bind()`). Use “Set up Playwright” below to upgrade to the latest \ + `playwright`, then restart the browser Claude is driving.", + self.playwright_version.as_deref().unwrap_or("?"), + match self.playwright_path.as_deref() { + Some(p) => format!(" at {}", p), + None => String::new(), + } )); } if self.cli_entry.is_none() { - return Some( - "Playwright is installed, but the viewer UI package isn't. Install it with \ - `npm i -D @playwright/cli`, then reopen this tab." - .to_string(), - ); + return Some(format!( + "Playwright {} is installed, but `@playwright/cli` — the package that serves \ + the viewer UI — isn't, and nothing else provides it (`@playwright/mcp` does \ + not). Use “Set up Playwright” below to install it. Looked in: {}.", + self.playwright_version.as_deref().unwrap_or("?"), + self.searched_text() + )); } None } + + /// Whether Playwright is present but has no browser at all to drive — + /// neither a downloaded bundle nor the Chrome channel. Advisory: the viewer + /// still runs, it just has nothing to show until a browser is bound. + pub fn needs_browser(&self) -> bool { + self.playwright_version.is_some() + && self.browsers.is_empty() + && self.chrome_channel.is_none() + } + + /// The searched roots as prose, so a message never trails off into "Looked + /// in: ." when the probe couldn't build a root list at all. + fn searched_text(&self) -> String { + if self.searched.is_empty() { + "the container's default module paths".to_string() + } else { + self.searched.join(", ") + } + } } /// One `node -e` probe, run as `claude` inside the container. /// /// No shell quoting is involved: the script is a single `argv` element. The -/// script finds the global `node_modules` root itself, so a Playwright installed -/// with `npm i -g` is found as readily as one in `/workspace/node_modules`. +/// script finds the global `node_modules` root and the npx cache itself, so a +/// Playwright installed with `npm i -g`, or merely *run* once through +/// `npx @playwright/mcp`, is found as readily as one in +/// `/workspace/node_modules`. pub async fn detect(container_id: &str) -> Result { let output = exec_oneshot( container_id, @@ -151,15 +226,44 @@ pub(crate) fn parse_probe_output(output: &str) -> Result` is a separate tree — `@playwright/mcp` and any other + // npx-run package each get their own — so all of them are searched, in a + // stable order, and all of them are reported in `searched`. + r#"const npx=[];if(cache){try{for(const d of fs.readdirSync(path.join(cache,"_npx")).sort()){"#, + r#"const p=path.join(cache,"_npx",d,"node_modules");"#, + r#"try{if(fs.statSync(p).isDirectory())npx.push(p);}catch(e){}}}catch(e){}}"#, + r#"const roots=[...new Set(["/workspace",process.cwd(),home?path.join(home,"node_modules"):null,g,...npx].filter(Boolean))];"#, r#"out.searched=roots;"#, - r#"const res=(s)=>{for(const r of roots){try{return require.resolve(s,{paths:[r]});}catch(e){}}return null;};"#, - r#"const core=res("playwright-core/package.json")||res("playwright/package.json");"#, - r#"if(core){try{out.playwright_path=core;out.playwright_version=JSON.parse(fs.readFileSync(core,"utf8")).version;}catch(e){}"#, + r#"const at=(s,r)=>{try{return require.resolve(s,{paths:[r]});}catch(e){return null;}};"#, + r#"const res=(s)=>{for(const r of roots){const p=at(s,r);if(p)return p;}return null;};"#, + // One `bin` reader for both packages: `bin` is a string for some manifests + // and an object for others, and getting that wrong on either one loses the + // entry point silently. + r#"const bin=(m,j)=>{const b=typeof j.bin==="string"?{[j.name]:j.bin}:(j.bin||{});"#, + r#"const k=Object.keys(b)[0];return k?path.resolve(path.dirname(m),b[k]):null;};"#, + // `playwright-core` is what carries the typings and the browser registry, but + // it is frequently *nested*: verified against a real `npm i -g playwright + // @playwright/cli`, npm does not hoist for global installs, so the global + // root holds `playwright/` and `@playwright/cli/` and no top-level + // `playwright-core/`. Resolving only the outer `playwright` would then read + // a package that ships no `types/types.d.ts` at all and report a perfectly + // current build as "predates browser.bind()". So: hop from the wrapper to + // its own `playwright-core`, and only fall back to the wrapper's manifest. + r#"let core=res("playwright-core/package.json");"#, + r#"if(!core){const pw=res("playwright/package.json");"#, + r#"if(pw)core=at("playwright-core/package.json",path.dirname(pw))||pw;}"#, + r#"if(core){try{out.playwright_path=core;const j=JSON.parse(fs.readFileSync(core,"utf8"));"#, + r#"out.playwright_version=j.version;out.playwright_cli=bin(core,j);}catch(e){}"#, // `bind`/`unbind` are checked against the shipped type definitions rather // than by loading the module: it is a static read, needs no browser, and // cannot be tripped up by a package that fails to import. @@ -167,8 +271,16 @@ const PROBE: &str = concat!( r#"out.has_bind=/\bunbind\s*\(\s*\)/.test(t)&&/\bbind\s*\(/.test(t);}catch(e){}}"#, r#"const cli=res("@playwright/cli/package.json");"#, r#"if(cli){try{const j=JSON.parse(fs.readFileSync(cli,"utf8"));out.cli_version=j.version;"#, - r#"const b=typeof j.bin==="string"?{[j.name]:j.bin}:(j.bin||{});const k=Object.keys(b)[0];"#, - r#"if(k)out.cli_entry=path.resolve(path.dirname(cli),b[k]);}catch(e){}}"#, + r#"out.cli_entry=bin(cli,j);}catch(e){}}"#, + // Browser bundles. `ffmpeg-*` lives in the same directory and is filtered + // out: it is not something that can be driven, and counting it would let + // the pane claim a browser is present when none is. + r#"try{const bd=process.env.PLAYWRIGHT_BROWSERS_PATH||(home?path.join(home,".cache","ms-playwright"):null);"#, + r#"if(bd)out.browsers=fs.readdirSync(bd).filter((n)=>/^(chromium|firefox|webkit)/.test(n)).sort();}catch(e){}"#, + // The Chrome *channel* is an apt package, not a Playwright download, so it + // is looked for where apt puts it. + r#"try{for(const p of ["/usr/bin/google-chrome-stable","/usr/bin/google-chrome","/opt/google/chrome/chrome"]){"#, + r#"if(fs.existsSync(p)){out.chrome_channel=p;break;}}}catch(e){}"#, r#"process.stdout.write("\n__TRIPLE_C_BROWSER_VIEW__"+JSON.stringify(out)+"\n");"#, ); @@ -201,27 +313,158 @@ mod tests { } #[test] - fn a_missing_playwright_is_reported_with_where_we_looked() { + fn a_missing_playwright_names_both_packages_and_where_we_looked() { let d = parse_probe_output(&payload( - r#"{"node_version":"22.11.0","searched":["/workspace","/usr/lib/node_modules"]}"#, + r#"{"node_version":"22.11.0","searched":["/workspace","/usr/lib/node_modules","/home/claude/.npm/_npx/a1/node_modules"]}"#, )) .unwrap(); assert!(!d.is_usable()); let msg = d.blocker().unwrap(); - assert!(msg.contains("npm i -D playwright"), "{}", msg); + // The two packages that actually have to be there, by name. + assert!(msg.contains("`playwright`"), "{}", msg); + assert!(msg.contains("`@playwright/cli`"), "{}", msg); assert!(msg.contains("browser.bind"), "{}", msg); + // Every root consulted, including the npx cache, so the claim is checkable. assert!(msg.contains("/usr/lib/node_modules"), "{}", msg); + assert!(msg.contains("/home/claude/.npm/_npx/a1/node_modules"), "{}", msg); + } + + #[test] + fn no_message_offers_playwright_mcp_as_a_way_through_setup() { + // It bundles a playwright-core new enough to bind, but never ships the + // viewer — so proposing it as an install route is a dead end, which is + // exactly what a user hit. It may only be named for what it does do. + for json in [ + r#"{"node_version":"22.11.0","searched":["/workspace"]}"#, + r#"{"node_version":"22.11.0","playwright_version":"1.44.0","has_bind":false}"#, + r#"{"node_version":"22.11.0","playwright_version":"1.62.1","has_bind":true}"#, + ] { + let msg = parse_probe_output(&payload(json)).unwrap().blocker().unwrap(); + let offers_install = msg.contains("install `@playwright/mcp`") + || msg.contains("or use `@playwright/mcp`") + || msg.contains("npm i -D @playwright/mcp") + || msg.contains("npm i -g @playwright/mcp"); + assert!(!offers_install, "{}", msg); + // And every message points at the one action that does work. + assert!(msg.contains("Set up Playwright"), "{}", msg); + } } #[test] fn a_playwright_without_bind_asks_for_an_upgrade() { let d = parse_probe_output(&payload( - r#"{"node_version":"22.11.0","playwright_version":"1.44.0","has_bind":false,"cli_entry":"/x/cli.js"}"#, + r#"{"node_version":"22.11.0","playwright_version":"1.44.0","playwright_path":"/workspace/node_modules/playwright/package.json","has_bind":false,"cli_entry":"/x/cli.js"}"#, )) .unwrap(); let msg = d.blocker().unwrap(); assert!(msg.contains("1.44.0"), "{}", msg); - assert!(msg.contains("playwright@latest"), "{}", msg); + assert!(msg.contains("/workspace/node_modules/playwright"), "{}", msg); + assert!(msg.contains("Set up Playwright"), "{}", msg); + } + + #[test] + fn an_npx_cached_playwright_counts_as_installed() { + // What `claude mcp add … npx @playwright/mcp@latest` leaves behind: a + // real playwright-core, in no `node_modules` the old probe looked at. + // It satisfies bind — and nothing else, because npx never brings the + // viewer with it. + let d = parse_probe_output(&payload( + concat!( + r#"{"node_version":"22.11.0","playwright_version":"1.62.1","#, + r#""playwright_path":"/home/claude/.npm/_npx/9f/node_modules/playwright-core/package.json","#, + r#""playwright_cli":"/home/claude/.npm/_npx/9f/node_modules/playwright-core/cli.js","#, + r#""has_bind":true,"#, + r#""searched":["/workspace","/usr/lib/node_modules","/home/claude/.npm/_npx/9f/node_modules"]}"#, + ), + )) + .unwrap(); + assert_eq!(d.playwright_version.as_deref(), Some("1.62.1")); + assert!(d.has_bind); + assert_eq!( + d.playwright_cli.as_deref(), + Some("/home/claude/.npm/_npx/9f/node_modules/playwright-core/cli.js") + ); + // Still not usable, and the message says why: the viewer is missing. + assert!(!d.is_usable()); + let msg = d.blocker().unwrap(); + assert!(msg.contains("@playwright/cli"), "{}", msg); + } + + #[test] + fn the_probe_searches_the_npx_cache_as_well_as_the_module_roots() { + // The roots are built inside the probe, so this is the only place the + // set can be asserted without a container. Each fragment is load-bearing: + // dropping any one of them is how an install becomes invisible. + assert!(PROBE.contains(r#""/workspace""#), "{}", PROBE); + assert!(PROBE.contains("process.cwd()"), "{}", PROBE); + assert!(PROBE.contains(r#"path.join(home,"node_modules")"#), "{}", PROBE); + assert!(PROBE.contains("npm root -g"), "{}", PROBE); + assert!(PROBE.contains(r#"path.join(cache,"_npx")"#), "{}", PROBE); + assert!(PROBE.contains("npm_config_cache"), "{}", PROBE); + // Every one of them, not just the first hit, and all of them reported. + assert!(PROBE.contains("...npx"), "{}", PROBE); + assert!(PROBE.contains("out.searched=roots"), "{}", PROBE); + } + + #[test] + fn a_partial_tree_still_answers_rather_than_failing() { + // Playwright resolved, but its manifest unreadable and no viewer: the + // probe's guards must still produce a parseable payload carrying what + // it did learn, because that is what the message is built from. + let d = parse_probe_output(&payload( + r#"{"node_version":"22.11.0","has_bind":false,"searched":["/workspace"],"browsers":["chromium-1200"]}"#, + )) + .unwrap(); + assert_eq!(d.node_version.as_deref(), Some("22.11.0")); + assert_eq!(d.browsers, vec!["chromium-1200".to_string()]); + assert!(d.blocker().is_some()); + } + + #[test] + fn a_playwright_with_no_browser_bundle_is_flagged_without_blocking() { + let d = parse_probe_output(&payload( + concat!( + r#"{"node_version":"22.11.0","playwright_version":"1.62.1","has_bind":true,"#, + r#""cli_version":"0.1.18","cli_entry":"/g/cli.js","browsers":[]}"#, + ), + )) + .unwrap(); + // Serving the viewer is possible; there is just nothing to drive yet. + assert!(d.is_usable()); + assert_eq!(d.blocker(), None); + assert!(d.needs_browser()); + + let with_browser = parse_probe_output(&payload( + concat!( + r#"{"node_version":"22.11.0","playwright_version":"1.62.1","has_bind":true,"#, + r#""cli_version":"0.1.18","cli_entry":"/g/cli.js","browsers":["chromium-1200"]}"#, + ), + )) + .unwrap(); + assert!(!with_browser.needs_browser()); + + // The Chrome channel counts too — it is an apt package rather than a + // Playwright download, so it never appears in `browsers`, and + // `@playwright/mcp` is the caller that asks for it. + let chrome_only = parse_probe_output(&payload( + concat!( + r#"{"node_version":"22.11.0","playwright_version":"1.62.1","has_bind":true,"#, + r#""cli_version":"0.1.18","cli_entry":"/g/cli.js","browsers":[],"#, + r#""chrome_channel":"/usr/bin/google-chrome-stable"}"#, + ), + )) + .unwrap(); + assert!(!chrome_only.needs_browser()); + assert_eq!( + chrome_only.chrome_channel.as_deref(), + Some("/usr/bin/google-chrome-stable") + ); + } + + #[test] + fn the_probe_looks_for_the_chrome_channel_where_apt_puts_it() { + assert!(PROBE.contains("google-chrome-stable"), "{}", PROBE); + assert!(PROBE.contains("/opt/google/chrome/chrome"), "{}", PROBE); } #[test] @@ -252,6 +495,19 @@ mod tests { assert!(err.contains("no output"), "{}", err); } + #[test] + fn the_probe_reads_bind_from_the_nested_core_of_a_wrapper_install() { + // `npm i -g playwright` leaves `playwright-core` under + // `playwright/node_modules`, and the wrapper ships no + // `types/types.d.ts` — so without this hop a current build reports + // `has_bind: false`. Verified against a real global install. + assert!( + PROBE.contains(r#"at("playwright-core/package.json",path.dirname(pw))"#), + "{}", + PROBE + ); + } + #[test] fn the_probe_is_a_single_argv_element_with_no_quoting_hazards() { // It is passed straight to `node -e`; a stray single quote would only diff --git a/app/src-tauri/src/browser_view/install.rs b/app/src-tauri/src/browser_view/install.rs new file mode 100644 index 0000000..27b8de9 --- /dev/null +++ b/app/src-tauri/src/browser_view/install.rs @@ -0,0 +1,1047 @@ +//! One-action setup for the browser view. +//! +//! The pane used to answer "Playwright isn't installed" with a sentence of npm +//! commands and leave the user to it. That went badly, and every part of how it +//! went badly is a constraint on this module: +//! +//! * `claude mcp add … npx @playwright/mcp@latest` looks like installing +//! Playwright and is not — see [`super::detect`] — and it can never satisfy +//! this pane on its own, because the viewer lives in `@playwright/cli`. +//! * `sudo npm i -g playwright` is what people try next. It works, but the +//! image leaves npm's prefix at `/usr`, so the unprivileged form fails with +//! `EACCES` first. +//! * `playwright install chromium` downloads a browser that then cannot start, +//! because the image shipped **none** of Chromium's shared libraries +//! (`libnss3`, `libgbm1`, `libatk*`, `libasound2`, `libcups2`, …). The +//! download succeeds, the launch fails, and the error reads like a Playwright +//! bug. Current base images bake those libraries in (see `container/ +//! Dockerfile`), so this step is now usually a no-op — but a project stays on +//! the base image it was first built from until someone migrates it, so the +//! old case is the *normal* case and has to keep working. Hence: check, then +//! install only if needed, and say which happened. +//! * Getting from there to a working pane took a long tail of further commands. +//! +//! ## Where the packages go, and why it is `/workspace` +//! +//! `playwright` + `@playwright/cli`, installed **locally into `/workspace`** as +//! `claude`, with `--no-save`. +//! +//! `/workspace` is *not* the user's repository. Project directories are +//! bind-mounted one level down, at `/workspace/{mount_name}` (see +//! `container_config`), so `/workspace` itself is ordinary container storage. +//! That single fact settles the choice: +//! +//! * **Nothing of the user's is mutated.** `/workspace/node_modules` is not +//! inside any bind mount, so no host file, no `package.json` and no lockfile +//! of theirs is touched. `--no-save` is belt and braces for the case where +//! someone has put a `package.json` at `/workspace` themselves — verified +//! that an install into a directory without one writes `node_modules` and +//! nothing else. +//! * **No sudo.** `/usr/lib/node_modules` is root-owned; `/workspace` is the +//! `claude` user's own working directory. A setup action that needs no +//! privilege escalation is a setup action with one fewer way to fail. +//! * **Node can actually find it.** A global install is *not* on the module +//! resolution path — `require('playwright')` from a script in +//! `/workspace/my-project` does not see `/usr/lib/node_modules`, but it does +//! walk up to `/workspace/node_modules`. Since the whole point is for Claude +//! to drive a browser from a script in the project, this is the difference +//! between setup that works and setup that merely reports as complete. +//! * **It hoists.** A local install puts `playwright-core` at the top of the +//! tree where the probe finds it directly; `npm i -g` leaves it nested (see +//! the note in [`super::detect`]). +//! * **It persists.** `/workspace` outside the bind mounts rides the project's +//! snapshot image across container recreation, and migration copies the +//! non-bind-mounted parts of `/workspace` forward. +//! +//! Browsers are the exception and are downloaded as `claude` into +//! `~/.cache/ms-playwright`, inside the home volume — so they survive +//! recreation *and* base-image migration, and are lost only on a project Reset. +//! That is worth saying in the UI: it is the difference between a +//! several-hundred-megabyte download once and one every time. + +use std::collections::VecDeque; +use std::time::Duration; + +use futures_util::StreamExt; +use serde::Serialize; +use tauri::AppHandle; + +use crate::commands::project_commands::emit_progress; +use crate::docker::exec::{ + create_attached_exec_as, exec_oneshot_as, wait_for_exec_exit, AttachedExec, +}; + +use super::detect::{self, PlaywrightDetection}; + +/// The two packages the pane genuinely needs, pinned to `@latest` because +/// `browser.bind()` is recent and the viewer tracks it. +/// +/// This is the *minimum* set. A user who followed the old guidance ended up +/// with a global install as well as these; only these are required. Note what +/// is not here: `@playwright/mcp` is Claude's MCP configuration to make, not +/// this pane's, and it contributes nothing to serving a viewer. +pub const PACKAGES: [&str; 2] = ["playwright@latest", "@playwright/cli@latest"]; + +/// Where the packages are installed. Container storage, not a bind mount — see +/// the module docs. +pub const INSTALL_DIR: &str = "/workspace"; + +/// npm reaching the registry and unpacking two packages. Minutes, not hours. +const NPM_TIMEOUT: Duration = Duration::from_secs(10 * 60); +/// `apt-get update` plus a dozen library packages, or Google's apt repository. +const DEPS_TIMEOUT: Duration = Duration::from_secs(20 * 60); +/// `apt-get install -s` over ~100 already-installed packages. Local work; a +/// container that cannot answer this in two minutes gets the libraries +/// installed rather than a hang. +const DEPS_CHECK_TIMEOUT: Duration = Duration::from_secs(2 * 60); +/// The browser download itself, on a bad connection. +const BROWSER_TIMEOUT: Duration = Duration::from_secs(45 * 60); +/// Starting a headless browser, and one page load. +const VERIFY_TIMEOUT: Duration = Duration::from_secs(120); + +/// Lines of command output kept for the result. Enough to carry npm's actual +/// error, bounded so a chatty download can't grow without limit. +const LOG_LINES: usize = 200; + +/// Longest run of bytes without a line break still treated as one progress +/// line. npm's progress output is `\r`-driven and can go a long way. +const MAX_PARTIAL: usize = 4 * 1024; + +/// Marks the verdict in the launch check's output, for the same reason +/// [`super::detect`] marks its payload: the stream also carries whatever +/// Chromium felt like writing to stderr. +const LAUNCH_MARKER: &str = "__TRIPLE_C_BROWSER_LAUNCH__"; + +/// URL the launch check navigates to once a browser is up. The registry is the +/// one host we know the container just reached, during the npm step — so a +/// failure here is informative rather than ambient. +const REACHABILITY_URL: &str = "https://registry.npmjs.org/-/ping"; + +/// Which browser to install. Both are legitimate; they serve different callers. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum BrowserTarget { + /// Playwright's own build. What `chromium.launch()` uses with no `channel`, + /// i.e. the default for scripts the user or Claude writes. + Chromium, + /// Google Chrome from Google's apt repository. **`@playwright/mcp` asks for + /// the `chrome` channel specifically**, so anyone driving the browser + /// through the MCP plugin needs this one rather than (or as well as) the + /// bundled build. + Chrome, +} + +impl BrowserTarget { + pub fn parse(s: &str) -> Result { + match s { + "chromium" => Ok(Self::Chromium), + "chrome" => Ok(Self::Chrome), + other => Err(format!( + "Unknown browser '{}'. This pane installs 'chromium' (Playwright's own build) \ + or 'chrome' (the Google Chrome channel that @playwright/mcp asks for).", + other + )), + } + } + + /// The name Playwright's CLI knows it by. + pub fn cli_name(self) -> &'static str { + match self { + Self::Chromium => "chromium", + Self::Chrome => "chrome", + } + } + + /// Shown *before* the click. Size first, because the honest failure mode + /// here is a user who did not know they were starting a large download. + pub fn download_note(self) -> &'static str { + match self { + Self::Chromium => { + "Playwright's Chromium build — a few hundred MB, a few minutes on a normal \ + connection. The system libraries it needs are already in current base images; \ + on an older container they are installed first" + } + Self::Chrome => { + "Google Chrome from Google's apt repository, with its dependencies — roughly \ + 150 MB" + } + } + } + + /// Who needs it, in the user's terms. + pub fn needed_for(self) -> &'static str { + match self { + Self::Chromium => "Playwright scripts that call `chromium.launch()` with no channel", + Self::Chrome => "`@playwright/mcp`, which asks for the `chrome` channel", + } + } + + /// The `channel` a launch check must pass. `None` means the bundled build. + fn channel(self) -> Option<&'static str> { + match self { + Self::Chromium => None, + Self::Chrome => Some("chrome"), + } + } +} + +/// What a setup step did, handed back to the pane so it can update itself +/// without the user reopening the tab. +#[derive(Debug, Clone, Serialize)] +pub struct BrowserSetupOutcome { + /// A fresh probe, run after the step. This is what makes the pane + /// self-updating. + pub detection: PlaywrightDetection, + /// Tail of the actual command output — always populated, success or not. + pub log: String, + /// Verdict of a real headless launch. `None` when the step didn't try one + /// (the npm step doesn't), `Some(false)` when a browser is installed and + /// still would not start. + pub browser_launched: Option, + /// A step that failed, or succeeded suspiciously, without failing the whole + /// action — the user is told rather than left to find out at launch time. + pub warning: Option, +} + +/// Install `playwright` and `@playwright/cli` into `/workspace`. Deliberately +/// does *not* fetch browsers: that is the second step, and its size is stated +/// before it is offered. +pub async fn install_packages( + app: &AppHandle, + project_id: &str, + container_id: &str, +) -> Result { + emit_progress( + app, + project_id, + &format!( + "Installing playwright and @playwright/cli into {}/node_modules…", + INSTALL_DIR + ), + ); + + // `env VAR=… cmd` rather than an exec env: it keeps the one exec path in + // `docker/exec.rs` untouched, and `env` is a real binary so no shell is + // involved. The guard matters because these are `@latest`: current + // Playwright has no postinstall (verified — `playwright@1.62.1` declares no + // `scripts` at all), but if a future release brings the browser download + // back, this step must stay small and the download must stay the step the + // user explicitly asked for. + let mut cmd = vec![ + "env".to_string(), + "PLAYWRIGHT_SKIP_BROWSER_DOWNLOAD=1".to_string(), + "npm".to_string(), + "install".to_string(), + // Leaves any package.json and lockfile at /workspace untouched. + "--no-save".to_string(), + "--no-fund".to_string(), + "--no-audit".to_string(), + ]; + cmd.extend(PACKAGES.iter().map(|p| p.to_string())); + + let step = run_step( + app, + project_id, + container_id, + "claude", + INSTALL_DIR, + cmd, + NPM_TIMEOUT, + ) + .await?; + if step.exit_code != 0 { + return Err(format!( + "npm couldn't install Playwright in this container (exit {}).\n\nnpm said:\n{}", + step.exit_code, + step.log_or("it produced no output at all") + )); + } + + emit_progress(app, project_id, "Re-checking what the container has…"); + let detection = detect::detect(container_id).await?; + + // A probe that still finds something missing after a successful npm run is + // the interesting case, so it is surfaced rather than swallowed. And a + // container with the packages but no browser is *not* finished setup — + // saying so here is what stops someone walking away from a pane that will + // never show them anything. + let mut warning = detection.blocker(); + if detection.needs_browser() { + warning = merge( + warning, + "Playwright is installed, but this container has no browser to drive yet. Install \ + one below — that is the large download, and it is a separate step on purpose." + .to_string(), + ); + } + + Ok(BrowserSetupOutcome { + warning, + detection, + log: step.log, + browser_launched: None, + }) +} + +/// Install a browser: its system libraries first, then the browser, then prove +/// one actually starts. +/// +/// The order is the whole point. `playwright install chromium` on this image +/// downloads a browser that cannot launch, because the libraries it links +/// against are absent — which is why installing Chrome through apt looked like +/// the fix: apt pulls those libraries in as dependencies. +pub async fn install_browser( + app: &AppHandle, + project_id: &str, + container_id: &str, + target: BrowserTarget, +) -> Result { + let before = detect::detect(container_id).await?; + let Some(cli) = before.playwright_cli.clone() else { + return Err( + "Playwright isn't installed in this container yet, so there is nothing to install a \ + browser for. Run “Set up Playwright” first." + .to_string(), + ); + }; + + let mut log = String::new(); + let mut warning: Option = None; + + // Step 1 — system libraries. Slow, previously invisible, and the actual + // cause of the "Chromium downloads and then dies" reports, so it gets its + // own progress line rather than being folded into the download. + // + // Current base images bake these in, so on an up-to-date container there is + // nothing to do here. That is *not* a reason to drop the step: a project + // keeps the base image it was first built from until it is migrated, so + // containers without the libraries are the common case for a long while + // yet. So: ask first, skip loudly, install only when the answer is no. + // + // The question is put to Playwright rather than answered by probing for + // library names ourselves. `install-deps --dry-run` simulates the very + // `apt-get install` that `install-deps` would run and exits non-zero if + // anything is missing, which means the check and the fix can never disagree + // about what "the libraries" means — including after a Playwright release + // adds one. + // + // Installing runs `install-deps` directly *as root*: that is what `playwright + // install --with-deps` does internally, minus Playwright's own privilege + // escalation (read from the shipped source, it shells out to + // `sudo -- sh -c "apt-get update && apt-get install …"` when it is not root, + // which would work here — `claude` has passwordless sudo — but puts an extra + // failure mode between the user and the answer). + // + // For the Chrome channel apt installs `google-chrome-stable`, whose own + // dependencies cover the same libraries — but going through the same check + // first makes the two paths behave identically. + emit_progress( + app, + project_id, + "Step 1/3 — checking whether this container already has the browser system libraries…", + ); + let state = check_libraries(container_id, &cli, target).await; + match &state { + LibraryState::Present => { + emit_progress( + app, + project_id, + "Step 1/3 — already there: this image ships the browser system libraries. \ + Skipping the apt install.", + ); + push_section(&mut log, "Browser system libraries: already installed, apt skipped."); + } + LibraryState::Missing(_) | LibraryState::Unknown(_) => { + emit_progress( + app, + project_id, + &format!( + "Step 1/3 — {} Installing browser system libraries with apt (needs root; a \ + minute or two)…", + state.detail() + ), + ); + let deps = run_step( + app, + project_id, + container_id, + "root", + "/tmp", + vec![ + "node".to_string(), + cli.clone(), + "install-deps".to_string(), + target.cli_name().to_string(), + ], + DEPS_TIMEOUT, + ) + .await?; + push_section(&mut log, &deps.log); + if deps.exit_code != 0 { + // Not fatal on its own — some of the libraries may already be + // present — but it must never pass silently, because the failure + // it causes surfaces much later and looks like something else. + warning = Some(format!( + "Installing the browser's system libraries failed (exit {}). The browser may \ + install and then refuse to start. apt said:\n{}", + deps.exit_code, + deps.log_or("nothing") + )); + } + } + } + + // Step 2 — the download the user was warned about. Chromium is fetched as + // `claude` so the bundle lands in the home volume's `~/.cache/ms-playwright` + // where the viewer looks for it; the Chrome channel is an apt install and + // has to be root. + emit_progress( + app, + project_id, + &format!( + "Step 2/3 — installing {}, needed for {} — {}…", + target.cli_name(), + target.needed_for(), + target.download_note() + ), + ); + let (user, workdir) = match target { + BrowserTarget::Chromium => ("claude", INSTALL_DIR), + BrowserTarget::Chrome => ("root", "/tmp"), + }; + let dl = run_step( + app, + project_id, + container_id, + user, + workdir, + vec![ + "node".to_string(), + cli, + "install".to_string(), + target.cli_name().to_string(), + ], + BROWSER_TIMEOUT, + ) + .await?; + push_section(&mut log, &dl.log); + if dl.exit_code != 0 { + return Err(format!( + "{} didn't install (exit {}).\n\nPlaywright said:\n{}", + target.cli_name(), + dl.exit_code, + dl.log_or("nothing") + )); + } + + // Step 3 — "installed" is not "works". The absence of this check is what + // turned a missing library into a pile of confusing errors. + emit_progress( + app, + project_id, + &format!("Step 3/3 — checking that {} actually launches…", target.cli_name()), + ); + let verdict = verify_launch(container_id, &before, target).await; + push_section(&mut log, &verdict.detail); + if !verdict.ok { + warning = merge( + warning, + format!( + "{} is installed but would not start: {}", + target.cli_name(), + verdict.detail.trim() + ), + ); + } + if let Some(cert) = verdict.cert_error { + // Distinct on purpose. A TLS-intercepting proxy is a container-wide + // trust-store gap — it breaks npm, git, curl and Claude Code the same + // way — and telling someone behind one that Playwright is broken sends + // them to fix the wrong thing. This pane does not install CAs. + warning = merge( + warning, + format!( + "The browser starts, but HTTPS pages fail certificate validation ({}). That is \ + this container not trusting your network's certificate authority — a TLS-\ + intercepting proxy — and it affects everything in the container, not just the \ + browser. Installing the CA into the container's trust store is the fix; this \ + pane doesn't do that.", + cert.trim() + ), + ); + } + + emit_progress(app, project_id, "Re-checking what the container has…"); + let detection = detect::detect(container_id).await?; + + Ok(BrowserSetupOutcome { + detection, + log: log.trim().to_string(), + browser_launched: Some(verdict.ok), + warning, + }) +} + +/// Phrases `install-deps --dry-run` prints. Matching on Playwright's own words +/// is load-bearing: the exit code alone cannot tell "everything is installed" +/// (0) apart from "this platform isn't in my table, so I did nothing" (also 0). +const DEPS_OK_MARKER: &str = "All system dependencies are installed"; +const DEPS_MISSING_MARKER: &str = "Missing system dependencies"; +const DEPS_UNKNOWN_PLATFORM_MARKER: &str = "Cannot install dependencies for"; + +/// Whether this container already has the libraries a browser links against. +#[derive(Debug, Clone, PartialEq, Eq)] +enum LibraryState { + /// Playwright confirms every package it would install is present. Current + /// base images bake them, so this is the answer on an up-to-date container. + Present, + /// Playwright named packages that are absent. + Missing(String), + /// The check could not answer. Always installs — an unnecessary apt run + /// costs a minute, a skipped one costs a browser that will not start. + Unknown(String), +} + +impl LibraryState { + /// What the user sees on the progress line, and why. + fn detail(&self) -> &str { + match self { + Self::Present => "", + Self::Missing(d) | Self::Unknown(d) => d, + } + } +} + +/// Ask Playwright whether the libraries are already installed. +/// +/// `--dry-run` simulates the same `apt-get install` that `install-deps` would +/// perform and exits non-zero if any package is missing, so the check can never +/// disagree with the fix about what the dependency set is — including after a +/// Playwright release changes it. +/// +/// Note that the simulation works on an image whose `/var/lib/apt/lists` has +/// been cleaned (every base image's has): apt knows installed packages from +/// dpkg's status file. A *missing* package on such an image is simply not in +/// any index, so apt fails, Playwright reports the failure, and this returns +/// [`LibraryState::Unknown`] — which installs, which is the right answer. +async fn check_libraries(container_id: &str, cli: &str, target: BrowserTarget) -> LibraryState { + let run = exec_oneshot_as( + container_id, + "root", + vec![ + "node".to_string(), + cli.to_string(), + "install-deps".to_string(), + "--dry-run".to_string(), + target.cli_name().to_string(), + ], + vec![], + ); + match tokio::time::timeout(DEPS_CHECK_TIMEOUT, run).await { + Ok(Ok((output, code))) => classify_library_check(&output, code), + Ok(Err(e)) => LibraryState::Unknown(format!( + "Couldn't ask Playwright whether they're already there ({}), so installing them to be \ + sure.", + e + )), + Err(_) => LibraryState::Unknown( + "The check for them didn't finish in time, so installing them to be sure.".to_string(), + ), + } +} + +/// Read the verdict out of `install-deps --dry-run`'s output. +fn classify_library_check(output: &str, exit_code: i64) -> LibraryState { + // Checked before the success marker, not after: this branch also exits 0. + if output.contains(DEPS_UNKNOWN_PLATFORM_MARKER) { + return LibraryState::Unknown( + "Playwright doesn't have a dependency list for this container's platform, so it \ + can't say — installing them to be sure." + .to_string(), + ); + } + if exit_code == 0 && output.contains(DEPS_OK_MARKER) { + return LibraryState::Present; + } + if let Some(idx) = output.find(DEPS_MISSING_MARKER) { + let summary = output[idx..] + .lines() + .next() + .unwrap_or(DEPS_MISSING_MARKER) + .trim(); + return LibraryState::Missing(format!("Playwright reports {}", summary.to_lowercase())); + } + LibraryState::Unknown(format!( + "Couldn't tell whether they're already there (the check exited {}), so installing them to \ + be sure.", + exit_code + )) +} + +/// The result of actually starting a browser. +#[derive(Debug, Clone)] +struct LaunchVerdict { + ok: bool, + detail: String, + /// Set when a page load failed specifically on certificate trust. + cert_error: Option, +} + +/// Launch the browser headless, load one page, and close it. Seconds, and it is +/// the only thing that distinguishes "downloaded" from "usable". +async fn verify_launch( + container_id: &str, + detection: &PlaywrightDetection, + target: BrowserTarget, +) -> LaunchVerdict { + // The module directory of whatever Playwright the probe resolved, passed in + // the environment rather than interpolated into the script, so no path can + // ever be read as JavaScript. + let Some(manifest) = detection.playwright_path.as_deref() else { + return LaunchVerdict { + ok: false, + detail: "Playwright could not be located to test with.".to_string(), + cert_error: None, + }; + }; + let dir = manifest.trim_end_matches("package.json").trim_end_matches('/'); + + let run = exec_oneshot_as( + container_id, + "claude", + vec![ + "node".to_string(), + "-e".to_string(), + LAUNCH_PROBE.to_string(), + ], + vec![ + format!("TRIPLE_C_PW_DIR={}", dir), + format!("TRIPLE_C_PW_CHANNEL={}", target.channel().unwrap_or("")), + format!("TRIPLE_C_PW_URL={}", REACHABILITY_URL), + ], + ); + match tokio::time::timeout(VERIFY_TIMEOUT, run).await { + Ok(Ok((output, _))) => parse_launch_output(&output), + Ok(Err(e)) => LaunchVerdict { + ok: false, + detail: e, + cert_error: None, + }, + Err(_) => LaunchVerdict { + ok: false, + detail: "the launch check didn't finish in time — treat the browser as unproven" + .to_string(), + cert_error: None, + }, + } +} + +/// Pull the verdict out of the launch check's combined output. +fn parse_launch_output(output: &str) -> LaunchVerdict { + let Some(idx) = output.find(LAUNCH_MARKER) else { + let trimmed = output.trim(); + return LaunchVerdict { + ok: false, + detail: if trimmed.is_empty() { + "the launch check produced no output".to_string() + } else { + trimmed.lines().next_back().unwrap_or(trimmed).trim().to_string() + }, + cert_error: None, + }; + }; + let line = output[idx + LAUNCH_MARKER.len()..] + .lines() + .next() + .unwrap_or("") + .trim(); + let value: serde_json::Value = match serde_json::from_str(line) { + Ok(v) => v, + Err(e) => { + return LaunchVerdict { + ok: false, + detail: format!("unreadable launch check result: {}", e), + cert_error: None, + } + } + }; + + let ok = value.get("ok").and_then(|b| b.as_bool()).unwrap_or(false); + let detail = value + .get("detail") + .and_then(|d| d.as_str()) + .unwrap_or("") + .to_string(); + let nav = value.get("nav"); + let cert_error = nav + .filter(|n| n.get("cert").and_then(|c| c.as_bool()).unwrap_or(false)) + .and_then(|n| n.get("detail").and_then(|d| d.as_str())) + .map(|s| s.to_string()); + + let detail = if !ok { + detail + } else { + match nav.and_then(|n| n.get("ok").and_then(|o| o.as_bool())) { + // A launch that works and a page that loads: setup is genuinely done. + Some(true) => format!("Browser launched ({}) and loaded a page.", detail), + // Launch fine, page not. Cert failures are reported separately; any + // other cause is likely just an offline container, so it is stated + // without being turned into an alarm. + Some(false) => format!( + "Browser launched ({}), but couldn't load a test page: {}", + detail, + nav.and_then(|n| n.get("detail").and_then(|d| d.as_str())) + .unwrap_or("no detail") + ), + _ => format!("Browser launched ({}).", detail), + } + }; + + LaunchVerdict { + ok, + detail, + cert_error, + } +} + +/// The launch check. One `argv` element, no newlines, same contract as the +/// detection probe. +/// +/// Playwright leaves the Chromium sandbox disabled by default, which is what +/// makes this work in a container at all. The timeout exists so a browser that +/// hangs on a missing library still returns a verdict rather than sitting there +/// until the exec is torn down. The navigation is best-effort and never decides +/// `ok` — it exists to tell a TLS-intercepted network apart from a broken +/// install. +const LAUNCH_PROBE: &str = concat!( + r#"const d=process.env.TRIPLE_C_PW_DIR,ch=process.env.TRIPLE_C_PW_CHANNEL||undefined,u=process.env.TRIPLE_C_PW_URL;"#, + r#"let done=false;const say=(ok,detail,nav)=>{if(done)return;done=true;"#, + r#"process.stdout.write("\n__TRIPLE_C_BROWSER_LAUNCH__"+JSON.stringify({ok,detail,nav:nav||null})+"\n");};"#, + r#"const one=(e)=>String((e&&e.message)||e).split("\n").slice(0,8).join(" | ");"#, + r#"const t=setTimeout(()=>{say(false,"the browser did not finish starting within 90s");process.exit(0);},90000);"#, + r#"(async()=>{let b=null;try{const {chromium}=require(d);b=await chromium.launch(ch?{channel:ch}:{});"#, + r#"let v="";try{v=b.version();}catch(e){}"#, + r#"let nav={ok:true,cert:false,detail:""};"#, + r#"try{const p=await b.newPage();await p.goto(u,{timeout:20000});}"#, + // A certificate failure is classified here, next to the message, because + // Chromium's wording is the only place the distinction exists. + r#"catch(e){const m=one(e);nav={ok:false,cert:/ERR_CERT|CERT_AUTHORITY|ERR_SSL|SSL_ERROR|self.signed/i.test(m),detail:m};}"#, + r#"await b.close();clearTimeout(t);say(true,v,nav);}"#, + r#"catch(e){clearTimeout(t);try{if(b)await b.close();}catch(e2){}say(false,one(e));}"#, + r#"process.exit(0);})();"#, +); + +/// One command's result: its exit code and the tail of what it printed. +struct StepResult { + exit_code: i64, + log: String, +} + +impl StepResult { + fn log_or<'a>(&'a self, fallback: &'a str) -> &'a str { + if self.log.trim().is_empty() { + fallback + } else { + &self.log + } + } +} + +/// Run one command in the container, streaming every line it prints to the pane +/// as `container-progress` and keeping the tail for the result. +/// +/// Uses `create_attached_exec_as` — the single attached-exec path — rather than +/// `exec_oneshot`, because these commands run for minutes and the point is that +/// the user can watch them. +async fn run_step( + app: &AppHandle, + project_id: &str, + container_id: &str, + user: &str, + workdir: &str, + cmd: Vec, + limit: Duration, +) -> Result { + let AttachedExec { + exec_id, + mut output, + input, + } = create_attached_exec_as(container_id, cmd, false, user, workdir).await?; + + // Nothing is ever written to this exec. Closing stdin means anything that + // would prompt sees EOF and gives up, instead of waiting for a person who + // isn't there. + drop(input); + + let mut tail: VecDeque = VecDeque::with_capacity(LOG_LINES); + let mut partial = String::new(); + + let pump = async { + while let Some(msg) = output.next().await { + let data = msg.map_err(|e| format!("Lost the container's output: {}", e))?; + let chunk = String::from_utf8_lossy(&data.into_bytes()).into_owned(); + // npm and Playwright both redraw with `\r`; treating that as a line + // break is what turns a progress bar into progress. + for piece in chunk.split_inclusive(|c| c == '\n' || c == '\r') { + partial.push_str(piece); + if piece.ends_with('\n') || piece.ends_with('\r') || partial.len() >= MAX_PARTIAL { + let line = std::mem::take(&mut partial); + record(&mut tail, line.trim(), app, project_id); + } + } + } + Ok::<(), String>(()) + }; + + let outcome = tokio::time::timeout(limit, pump).await; + if !partial.trim().is_empty() { + let line = std::mem::take(&mut partial); + record(&mut tail, line.trim(), app, project_id); + } + let log = tail.iter().cloned().collect::>().join("\n"); + + match outcome { + // The captured tail is deliberately part of the timeout message: a step + // that ran for 45 minutes and stopped has its reason in its last lines. + Err(_) => Err(format!( + "The command didn't finish within {} minutes and was abandoned.\n\nLast output:\n{}", + limit.as_secs() / 60, + if log.trim().is_empty() { "none" } else { &log } + )), + Ok(Err(e)) => Err(e), + Ok(Ok(())) => Ok(StepResult { + exit_code: wait_for_exec_exit(&exec_id).await.unwrap_or(0), + log, + }), + } +} + +/// Keep a line for the result and show it in the pane. +fn record(tail: &mut VecDeque, line: &str, app: &AppHandle, project_id: &str) { + if line.is_empty() { + return; + } + if tail.len() == LOG_LINES { + tail.pop_front(); + } + tail.push_back(line.to_string()); + emit_progress(app, project_id, &truncate(line, 160)); +} + +/// Append a further command's output to the accumulated log. +fn push_section(log: &mut String, section: &str) { + if section.trim().is_empty() { + return; + } + if !log.is_empty() { + log.push_str("\n\n"); + } + log.push_str(section.trim()); +} + +/// Combine warnings so a second one never silently replaces the first. +fn merge(existing: Option, next: String) -> Option { + Some(match existing { + Some(prev) => format!("{}\n\n{}", prev, next), + None => next, + }) +} + +/// Progress is a single line in the UI, so an over-long one is cut here rather +/// than allowed to reflow the layout. Cuts on a char boundary. +fn truncate(line: &str, max: usize) -> String { + if line.chars().count() <= max { + return line.to_string(); + } + let mut s: String = line.chars().take(max.saturating_sub(1)).collect(); + s.push('…'); + s +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn the_package_set_is_the_minimum_that_satisfies_the_probe() { + // The viewer package is not optional, and `@playwright/mcp` is not a + // member: it can bind sessions, it can never serve the UI. + assert!(PACKAGES.iter().any(|p| p.starts_with("playwright@"))); + assert!(PACKAGES.iter().any(|p| p.starts_with("@playwright/cli@"))); + assert!(!PACKAGES.iter().any(|p| p.contains("@playwright/mcp"))); + assert_eq!(PACKAGES.len(), 2); + } + + #[test] + fn packages_are_installed_where_no_sudo_and_no_bind_mount_are_involved() { + // Bind mounts live at /workspace/; /workspace itself does + // not belong to the user's repository, and does belong to `claude`. + assert_eq!(INSTALL_DIR, "/workspace"); + } + + #[test] + fn both_browser_targets_are_offered_and_named_for_who_needs_them() { + assert_eq!(BrowserTarget::parse("chromium").unwrap(), BrowserTarget::Chromium); + assert_eq!(BrowserTarget::parse("chrome").unwrap(), BrowserTarget::Chrome); + // `@playwright/mcp` asks for the chrome channel specifically, so the UI + // must be able to say so. + assert!(BrowserTarget::Chrome.needed_for().contains("@playwright/mcp")); + assert_eq!(BrowserTarget::Chrome.channel(), Some("chrome")); + assert_eq!(BrowserTarget::Chromium.channel(), None); + // And a size, before the click, for both. + for t in [BrowserTarget::Chromium, BrowserTarget::Chrome] { + assert!(t.download_note().to_lowercase().contains("mb"), "{:?}", t); + } + } + + #[test] + fn an_unknown_browser_is_refused_with_the_two_that_work() { + let err = BrowserTarget::parse("firefox").unwrap_err(); + assert!(err.contains("chromium"), "{}", err); + assert!(err.contains("chrome"), "{}", err); + } + + #[test] + fn baked_in_libraries_are_detected_and_the_apt_step_is_skipped() { + // What a container built from a current base image says. The whole + // point of baking them: this must not re-run apt. + assert_eq!( + classify_library_check("All system dependencies are installed.\n", 0), + LibraryState::Present + ); + } + + #[test] + fn an_older_image_without_them_is_detected_and_named() { + let v = classify_library_check( + "Missing system dependencies (12):\n libnss3\n libgbm1\n", + 1, + ); + match v { + LibraryState::Missing(d) => assert!(d.contains("(12)"), "{}", d), + other => panic!("expected Missing, got {:?}", other), + } + } + + #[test] + fn an_unrecognised_platform_installs_rather_than_reporting_success() { + // Playwright prints this and exits **0** having installed nothing, so a + // check that trusted the exit code would skip the apt step on exactly + // the container that needs it. + let v = classify_library_check( + "Cannot install dependencies for ubuntu24.04-riscv64 with Playwright 1.62.1!\n", + 0, + ); + assert!(matches!(v, LibraryState::Unknown(_)), "{:?}", v); + } + + #[test] + fn a_check_that_could_not_run_installs_rather_than_guessing() { + // e.g. apt cannot resolve a package because the image's package index + // was cleaned and the package is genuinely absent. + let v = classify_library_check("E: Unable to locate package libgbm1\n", 100); + match v { + LibraryState::Unknown(d) => assert!(d.contains("100"), "{}", d), + other => panic!("expected Unknown, got {:?}", other), + } + // Present contributes nothing to the progress line; the other two must + // explain themselves, because the reason is shown to the user. + assert_eq!(LibraryState::Present.detail(), ""); + assert!(!LibraryState::Unknown("why".into()).detail().is_empty()); + } + + #[test] + fn a_successful_launch_and_page_load_is_reported_as_working() { + let v = parse_launch_output(concat!( + "some chromium noise\n__TRIPLE_C_BROWSER_LAUNCH__", + r#"{"ok":true,"detail":"140.0.1","nav":{"ok":true,"cert":false,"detail":""}}"#, + "\n", + )); + assert!(v.ok); + assert!(v.cert_error.is_none()); + assert!(v.detail.contains("140.0.1"), "{}", v.detail); + assert!(v.detail.contains("loaded a page"), "{}", v.detail); + } + + #[test] + fn a_missing_shared_library_is_reported_verbatim() { + // The exact failure the base image produces. It must reach the user as + // itself, not as "installation failed". + let v = parse_launch_output(concat!( + "__TRIPLE_C_BROWSER_LAUNCH__", + r#"{"ok":false,"detail":"Host system is missing dependencies: libnss3.so"}"#, + "\n", + )); + assert!(!v.ok); + assert!(v.detail.contains("libnss3.so"), "{}", v.detail); + } + + #[test] + fn a_tls_intercepting_proxy_is_distinguished_from_a_broken_install() { + // The browser is fine; the container doesn't trust the network's CA. + // Reporting this as a launch failure sends the user to fix Playwright. + let v = parse_launch_output(concat!( + "__TRIPLE_C_BROWSER_LAUNCH__", + r#"{"ok":true,"detail":"140.0.1","nav":{"ok":false,"cert":true,"#, + r#""detail":"page.goto: net::ERR_CERT_AUTHORITY_INVALID at https://registry.npmjs.org/"}}"#, + "\n", + )); + assert!(v.ok, "the browser did launch"); + assert_eq!( + v.cert_error.as_deref().map(|s| s.contains("ERR_CERT_AUTHORITY_INVALID")), + Some(true) + ); + } + + #[test] + fn an_offline_container_is_not_reported_as_a_certificate_problem() { + let v = parse_launch_output(concat!( + "__TRIPLE_C_BROWSER_LAUNCH__", + r#"{"ok":true,"detail":"140.0.1","nav":{"ok":false,"cert":false,"#, + r#""detail":"net::ERR_NAME_NOT_RESOLVED"}}"#, + "\n", + )); + assert!(v.ok); + assert!(v.cert_error.is_none()); + assert!(v.detail.contains("ERR_NAME_NOT_RESOLVED"), "{}", v.detail); + } + + #[test] + fn an_unmarked_stream_surfaces_the_containers_own_error() { + let v = parse_launch_output("node: command not found\n"); + assert!(!v.ok); + assert!(v.detail.contains("command not found"), "{}", v.detail); + } + + #[test] + fn an_empty_stream_is_explained_rather_than_parsed() { + let v = parse_launch_output(" \n"); + assert!(!v.ok); + assert!(v.detail.contains("no output"), "{}", v.detail); + } + + #[test] + fn the_launch_probe_is_a_single_argv_element() { + assert!(!LAUNCH_PROBE.contains('\n')); + assert!(LAUNCH_PROBE.contains(LAUNCH_MARKER)); + // Paths and channels are read from the environment, never interpolated. + assert!(LAUNCH_PROBE.contains("process.env.TRIPLE_C_PW_DIR")); + assert!(LAUNCH_PROBE.contains("process.env.TRIPLE_C_PW_CHANNEL")); + assert!(LAUNCH_PROBE.contains("ERR_CERT")); + } + + #[test] + fn warnings_accumulate_rather_than_overwrite() { + let w = merge(Some("first".to_string()), "second".to_string()).unwrap(); + assert!(w.contains("first") && w.contains("second"), "{}", w); + assert_eq!(merge(None, "only".to_string()).as_deref(), Some("only")); + } + + #[test] + fn long_progress_lines_are_cut_on_a_char_boundary() { + let line = "é".repeat(400); + let cut = truncate(&line, 160); + assert_eq!(cut.chars().count(), 160); + assert!(cut.ends_with('…')); + assert_eq!(truncate("short", 160), "short"); + } +} diff --git a/app/src-tauri/src/browser_view/mod.rs b/app/src-tauri/src/browser_view/mod.rs index b77c618..cde1369 100644 --- a/app/src-tauri/src/browser_view/mod.rs +++ b/app/src-tauri/src/browser_view/mod.rs @@ -63,6 +63,7 @@ pub mod commands; pub mod detect; +pub mod install; pub mod proxy; use std::collections::HashMap; diff --git a/app/src-tauri/src/commands/auth_token_commands.rs b/app/src-tauri/src/commands/auth_token_commands.rs index 337162f..3ecc730 100644 --- a/app/src-tauri/src/commands/auth_token_commands.rs +++ b/app/src-tauri/src/commands/auth_token_commands.rs @@ -23,6 +23,28 @@ //! * The flow needs a way to deliver the pasted code, hence //! [`submit_claude_token_code`] and the stdin channel below. Without it the //! command would simply sit at the prompt until it timed out. +//! +//! And the code can be *refused*. On a bad paste the CLI prints +//! `OAuth error: Invalid code…` / `Press Enter to retry.` and then blocks on +//! stdin waiting for that Enter — it does **not** exit. Nothing recognised +//! that, so a rejected code used to wedge the flow until [`SETUP_TIMEOUT`] +//! with the UI still claiming it was finishing. [`detect_code_rejection`] +//! spots it, [`CODE_REJECTED_EVENT`] tells the user, and the Enter is sent +//! for them so their next code has a prompt to land in — bounded by +//! [`MAX_CODE_ATTEMPTS`], because a retry loop nobody can win is just the +//! same hang with more steps. +//! +//! * The URL is emitted as an **OSC 8 hyperlink**, and the visible text of +//! that hyperlink is sliced by the CLI into terminal-width pieces on +//! separate lines. Measured against 2.1.226: the URL is 346 characters, and +//! at 80 columns it arrives as five separate hyperlink emissions, each +//! carrying the *complete* URL in its OSC 8 parameter and 80 characters of +//! it as visible text. Scraping the visible text therefore yields a +//! truncated URL that still parses, still points at claude.com, and still +//! fails to authorise — the worst possible shape of wrong. The parameter is +//! contiguous and authoritative, so [`AnsiStripper`] surfaces it and +//! [`LINK_EVENT`] carries it to the UI, which allowlists it before it is +//! shown or opened. //! * [`crate::auth_bridge`] is **not involved**. There is no container-local //! listener to reach, so there is nothing for it to bridge. //! @@ -67,6 +89,19 @@ const PROGRESS_EVENT: &str = "claude-token-progress"; /// URL to visit. Payload `{ project_id, chunk }`. const OUTPUT_EVENT: &str = "claude-token-output"; +/// A sign-in URL lifted from an OSC 8 hyperlink parameter, which is the only +/// place the *whole* URL appears — see the module docs. Payload +/// `{ project_id, url }`. +/// +/// This is a candidate, not a verdict: the payload is container output, so the +/// frontend re-parses it and applies the `ANTHROPIC_SIGN_IN_HOSTS` allowlist +/// before showing it and again before handing it to the OS opener. +const LINK_EVENT: &str = "claude-token-link"; + +/// The CLI refused the submitted code and is waiting to be handed another. +/// Payload `{ project_id, message, attempts_remaining }`. +const CODE_REJECTED_EVENT: &str = "claude-token-code-rejected"; + /// How long to wait for the whole flow. Generous: the user has to switch to a /// browser, sign in, and approve. Bounded so a wedged exec can't leak a task. const SETUP_TIMEOUT: Duration = Duration::from_secs(15 * 60); @@ -118,6 +153,114 @@ fn is_token_byte(b: u8) -> bool { b.is_ascii_alphanumeric() || b == b'-' || b == b'_' } +/// How far into a line a break has to sit before it can be believed to be the +/// terminal's right margin rather than the end of a line of prose. +/// +/// This is what makes reassembling a wrapped credential safe. Without it, +/// "join a credential-shaped run to whatever starts the next line" would happily +/// weld `sk-ant-oat01-…` in a usage example onto the first word below it and +/// manufacture a token-length string out of two unrelated things — silently +/// storing a credential that cannot work, which is the exact failure +/// [`MIN_TOKEN_BODY`] exists to prevent. A hard wrap, by contrast, always breaks +/// at the pty's width, and no pty this code can be handed is narrower than 40 +/// columns (Docker's default is 80; [`SETUP_TOKEN_SCRIPT`] asks for 400). +const MIN_WRAP_COLUMN: usize = 40; + +/// How many hard wraps one credential may be reassembled across. A ~103 +/// character token needs one at 80 columns and two at 40; three is slack, and a +/// bound at all stops a pathological input walking the whole transcript. +const MAX_CREDENTIAL_WRAPS: usize = 3; + +/// A credential-shaped run of characters, possibly spanning hard wraps. +struct CredentialRun { + /// One past the last byte of the run, embedded line breaks included. + end: usize, + /// How many credential characters it holds, line breaks excluded. + body_len: usize, + /// The run ran into the end of the buffer, so more input could extend it. + open: bool, +} + +/// Walk a credential body forwards from `body_start`, stepping over the hard +/// line breaks a pty inserts when the value is wider than the terminal. +/// +/// Wrapping is not hypothetical and it is not harmless. `stty cols` in +/// [`SETUP_TOKEN_SCRIPT`] fails *silently* (`2>/dev/null || true`), and an +/// 80-column fallback splits the ~103 character token across two lines. Before +/// this, that produced two bad outcomes at once: the parser saw only a +/// too-short fragment and the whole sign-in failed for no visible reason, while +/// [`redact_complete`] masked the first line — which carries the `sk-ant-` +/// marker — and printed the *second* line, the tail of a live credential, +/// straight to the UI. Both halves are handled here so the two can never +/// disagree about where a credential ends. +/// +/// The column test is measured from the last line break *in the buffer given*. +/// For [`SecretRedactor`], text earlier on the same line may already have been +/// emitted and drained, so the measured column can be shorter than the true one +/// — which can only make the scan *refuse* a join it would otherwise make, +/// never invent one. In practice it does not bite: the redactor withholds from +/// the marker onwards, so the whole credential and any wrap inside it are +/// together in the buffer by the time this runs. +fn scan_credential_body(bytes: &[u8], body_start: usize) -> CredentialRun { + let mut end = body_start; + let mut body_len = 0usize; + let mut wraps = 0usize; + + loop { + while end < bytes.len() && is_token_byte(bytes[end]) { + end += 1; + body_len += 1; + } + + // Ran out of input mid-run: the rest may be in the next chunk. + if end >= bytes.len() { + return CredentialRun { end, body_len, open: true }; + } + if bytes[end] != b'\n' || wraps >= MAX_CREDENTIAL_WRAPS { + return CredentialRun { end, body_len, open: false }; + } + + // A run already long enough to *be* a credential does not need + // continuing, and continuing it is how the reassembly turns into a + // fabrication machine: a repainting TUI prints `Your token: \n` + // over and over, so the character after the break is very often another + // token character belonging to the next frame entirely. Stopping here + // means the only runs ever joined are the ones too short to stand alone + // — which is exactly what a wrap produces. + // + // The cost is a token wrapped at a width between ~93 and ~102 columns, + // where the first line would already clear this bar. No pty in this flow + // is that size (Docker gives 80, [`SETUP_TOKEN_SCRIPT`] asks for 400), + // and the outcome there is the pre-existing loud failure, not a wrong + // credential. + if body_len >= MIN_TOKEN_BODY { + return CredentialRun { end, body_len, open: false }; + } + + let line_start = bytes[..end] + .iter() + .rposition(|b| *b == b'\n') + .map_or(0, |i| i + 1); + if end - line_start < MIN_WRAP_COLUMN { + return CredentialRun { end, body_len, open: false }; + } + + // The break is at a plausible margin, so whether this is a wrap turns on + // what follows it. A continuation resumes in column 0 with more + // credential characters; anything else — a blank line, an indent, prose + // — ends the run. + if end + 1 >= bytes.len() { + return CredentialRun { end, body_len, open: true }; + } + if !is_token_byte(bytes[end + 1]) { + return CredentialRun { end, body_len, open: false }; + } + + wraps += 1; + end += 1; + } +} + // ───────────────────────────────────────────────────────────────────────────── // Token extraction // ───────────────────────────────────────────────────────────────────────────── @@ -130,6 +273,12 @@ fn is_token_byte(b: u8) -> bool { /// * the prefix must not be glued to the tail of a longer word; /// * at least [`MIN_TOKEN_BODY`] token characters must follow it. /// +/// Those characters may be split across hard line breaks — see +/// [`scan_credential_body`] — and the breaks are removed from the value. The +/// length floor is applied to the *reassembled* body, so a fragment is still +/// never accepted on its own; reassembly only ever turns a failure into the +/// whole credential, never a fragment into a plausible one. +/// /// The **last** match wins. The command narrates before it succeeds, and a TUI /// may repaint the same frame repeatedly, so earlier matches are either prose /// or superseded repaints of the same value. @@ -147,16 +296,15 @@ pub fn parse_setup_token(output: &str) -> Option { continue; } - let body_start = start + TOKEN_PREFIX.len(); - let mut end = body_start; - while end < bytes.len() && is_token_byte(bytes[end]) { - end += 1; - } - if end - body_start < MIN_TOKEN_BODY { + let run = scan_credential_body(bytes, start + TOKEN_PREFIX.len()); + if run.body_len < MIN_TOKEN_BODY { continue; } - found = Some(output[start..end].to_string()); + // `run.end` lands on a non-token byte or the end of the buffer, and + // every non-token byte is either ASCII or a UTF-8 lead byte, so both + // ends are char boundaries. + found = Some(output[start..run.end].replace('\n', "")); } found @@ -167,6 +315,12 @@ pub fn parse_setup_token(output: &str) -> Option { // ───────────────────────────────────────────────────────────────────────────── /// Mask every *complete* credential in `text`. +/// +/// A credential wrapped across lines is masked as one span, line breaks +/// included, so the placeholder replaces the whole thing rather than leaving the +/// tail visible on the next line. That welds the two display lines together; +/// losing a line break in the transcript is a fair price for not printing half a +/// live credential to the UI. fn redact_complete(text: &str) -> String { let bytes = text.as_bytes(); let mut out = String::with_capacity(text.len()); @@ -180,19 +334,15 @@ fn redact_complete(text: &str) -> String { if start > 0 && is_token_byte(bytes[start - 1]) { continue; } - let body_start = start + SECRET_MARKER.len(); - let mut end = body_start; - while end < bytes.len() && is_token_byte(bytes[end]) { - end += 1; - } - if end - body_start < MIN_SECRET_BODY { + let run = scan_credential_body(bytes, start + SECRET_MARKER.len()); + if run.body_len < MIN_SECRET_BODY { continue; } out.push_str(&text[copied..start]); out.push_str(SECRET_PLACEHOLDER); - copied = end; - cursor = end; + copied = run.end; + cursor = run.end; } out.push_str(&text[copied..]); @@ -205,15 +355,14 @@ fn redact_complete(text: &str) -> String { fn holdback_index(text: &str) -> usize { let bytes = text.as_bytes(); - // A credential already under way: the last marker with nothing but token - // characters after it. If the *last* marker fails that test, no earlier one - // can pass it either — the disqualifying character lies after them all. + // A credential already under way: the last marker whose body runs to the end + // of what we have, so the next chunk could extend it. If the *last* marker + // fails that test, no earlier one can pass it either — the disqualifying + // character lies after them all. A body that stops at a hard wrap counts as + // still open, because the continuation is what the next chunk will bring. if let Some(start) = text.rfind(SECRET_MARKER) { let clean_start = start == 0 || !is_token_byte(bytes[start - 1]); - let body_all_token = bytes[start + SECRET_MARKER.len()..] - .iter() - .all(|b| is_token_byte(*b)); - if clean_start && body_all_token { + if clean_start && scan_credential_body(bytes, start + SECRET_MARKER.len()).open { return start; } } @@ -288,18 +437,59 @@ fn utf8_len(b: u8) -> usize { /// a separator can never fabricate or destroy a match. const CURSOR_MOVE_FINALS: &[u8] = b"ABCDEFGHd"; +/// Escape intermediates that introduce a **three**-byte sequence: `ESC`, the +/// intermediate, then one final byte. +/// +/// Claude Code prefixes every repaint frame with `ESC ( B` (designate ASCII as +/// G0). Treating that as a two-byte escape — which is what "anything else is two +/// bytes" did — consumed `ESC (` and emitted the `B` as ordinary text. Mostly +/// that was a stray letter in the transcript; landing immediately before a +/// token it would have glued `B` onto `sk-ant-oat01-…` and made +/// [`parse_setup_token`] reject a perfectly good credential. +const ESCAPE_INTERMEDIATES: &[u8] = b"()*+-./#%"; + +/// Largest OSC 8 target this will carry. Real authorize URLs are ~350 +/// characters (measured: 346 against 2.1.226); anything past a few kilobytes is +/// not a link, and the frontend's own relay cap is 8192. +const MAX_LINK_LENGTH: usize = 8192; + +/// Cap on undrained OSC 8 targets, so a container printing hyperlinks in a loop +/// cannot grow [`AnsiStripper`] without bound. The caller drains after every +/// chunk, so reaching this means something pathological is happening. +const MAX_PENDING_LINKS: usize = 32; + +/// Pull the link target out of an OSC 8 payload — everything between `ESC ]` +/// and the terminator. +/// +/// Shape: `8;;`, e.g. `8;id=1umaq0e;https://claude.com/…`. The +/// closing half of a hyperlink is `8;;` and so yields `None`, as does any other +/// OSC (window title, the URL relay's own OSC 7777, …). +fn osc8_target(payload: &[u8]) -> Option { + let payload = std::str::from_utf8(payload).ok()?; + let rest = payload.strip_prefix("8;")?; + let uri = &rest[rest.find(';')? + 1..]; + if uri.is_empty() || uri.len() > MAX_LINK_LENGTH { + return None; + } + Some(uri.to_string()) +} + /// Strip terminal control sequences from the front of `bytes`, stopping at the -/// first incomplete sequence or truncated character. Returns the clean text and -/// how many bytes were consumed. -fn strip_ansi_prefix(bytes: &[u8]) -> (String, usize) { +/// first incomplete sequence or truncated character. Returns the clean text, any +/// OSC 8 link targets found, and how many bytes were consumed. +fn strip_ansi_prefix(bytes: &[u8]) -> (String, Vec, usize) { let mut out = String::with_capacity(bytes.len()); + let mut links: Vec = Vec::new(); let mut i = 0usize; - while i < bytes.len() { + let consumed = 'scan: loop { + if i >= bytes.len() { + break 'scan i; + } match bytes[i] { 0x1b => { if i + 1 >= bytes.len() { - return (out, i); + break 'scan i; } match bytes[i + 1] { // CSI: parameter/intermediate bytes, then a final 0x40..=0x7e. @@ -309,7 +499,7 @@ fn strip_ansi_prefix(bytes: &[u8]) -> (String, usize) { j += 1; } if j >= bytes.len() { - return (out, i); + break 'scan i; } if CURSOR_MOVE_FINALS.contains(&bytes[j]) { out.push(' '); @@ -318,29 +508,46 @@ fn strip_ansi_prefix(bytes: &[u8]) -> (String, usize) { } // OSC: runs until BEL or ST (ESC \). b']' => { - let mut j = i + 2; + let payload_start = i + 2; + let mut j = payload_start; + let payload_end; loop { if j >= bytes.len() { - return (out, i); + break 'scan i; } if bytes[j] == 0x07 { + payload_end = j; j += 1; break; } if bytes[j] == 0x1b { if j + 1 >= bytes.len() { - return (out, i); + break 'scan i; } if bytes[j + 1] == b'\\' { + payload_end = j; j += 2; break; } } j += 1; } + // The one part of an OSC worth keeping: the hyperlink + // target, which is the only contiguous copy of the + // sign-in URL the CLI emits. + if let Some(uri) = osc8_target(&bytes[payload_start..payload_end]) { + links.push(uri); + } i = j; } - // Two-byte escapes (charset selection, keypad mode, …). + // Three-byte escapes: charset designation and friends. + b if ESCAPE_INTERMEDIATES.contains(&b) => { + if i + 2 >= bytes.len() { + break 'scan i; + } + i += 3; + } + // Two-byte escapes (keypad mode, index, …). _ => i += 2, } } @@ -355,7 +562,7 @@ fn strip_ansi_prefix(bytes: &[u8]) -> (String, usize) { j += 1; } if j >= bytes.len() { - return (out, i); + break 'scan i; } if bytes[j] != b'\n' { out.push('\n'); @@ -374,7 +581,7 @@ fn strip_ansi_prefix(bytes: &[u8]) -> (String, usize) { b => { let len = utf8_len(b); if i + len > bytes.len() { - return (out, i); + break 'scan i; } if let Ok(s) = std::str::from_utf8(&bytes[i..i + len]) { out.push_str(s); @@ -382,9 +589,9 @@ fn strip_ansi_prefix(bytes: &[u8]) -> (String, usize) { i += len; } } - } + }; - (out, i) + (out, links, consumed) } /// Cap on the bytes [`AnsiStripper`] will hold waiting for a control sequence @@ -406,12 +613,17 @@ const MAX_ANSI_CARRY: usize = 64 * 1024; #[derive(Default)] struct AnsiStripper { carry: Vec, + /// OSC 8 link targets seen since the last [`AnsiStripper::take_links`]. + /// Kept out of the return value so every existing caller and test of + /// `push` keeps reading as "bytes in, visible text out". + links: Vec, } impl AnsiStripper { fn push(&mut self, chunk: &[u8]) -> String { self.carry.extend_from_slice(chunk); - let (mut out, consumed) = strip_ansi_prefix(&self.carry); + let (mut out, links, consumed) = strip_ansi_prefix(&self.carry); + self.record_links(links); self.carry.drain(..consumed); // Past the cap the leading sequence is not going to terminate. Drop @@ -429,13 +641,116 @@ impl AnsiStripper { } while self.carry.len() > MAX_ANSI_CARRY { self.carry.drain(..1); - let (more, consumed) = strip_ansi_prefix(&self.carry); + let (more, links, consumed) = strip_ansi_prefix(&self.carry); + self.record_links(links); self.carry.drain(..consumed); out.push_str(&more); } out } + + fn record_links(&mut self, links: Vec) { + for link in links { + if self.links.len() >= MAX_PENDING_LINKS { + break; + } + self.links.push(link); + } + } + + /// Hand over the hyperlink targets seen so far and forget them. + fn take_links(&mut self) -> Vec { + std::mem::take(&mut self.links) + } +} + +/// Whether an OSC 8 target is worth forwarding to the UI as a sign-in candidate. +/// +/// Deliberately shallow. The frontend re-parses it, applies the +/// `ANTHROPIC_SIGN_IN_HOSTS` allowlist before it is displayed, and applies it +/// again at the sink before `openUrl` — that is where the security decision +/// lives, and duplicating a host allowlist here would be a second place for it +/// to go stale. All this does is keep obvious junk off the wire. +fn usable_sign_in_link(uri: &str) -> bool { + if !uri.starts_with("https://") && !uri.starts_with("http://") { + return false; + } + if uri.len() > MAX_LINK_LENGTH { + return false; + } + // Printable ASCII only. Control characters and whitespace are exactly how a + // URL is smuggled past a display, and `new URL()` on the other side strips + // some of them silently; a real authorize URL is percent-encoded anyway. + if !uri.bytes().all(|b| (0x21..=0x7e).contains(&b)) { + return false; + } + // This path bypasses [`SecretRedactor`] entirely, so nothing + // credential-shaped is allowed to ride it. + if uri.contains(SECRET_MARKER) { + return false; + } + true +} + +// ───────────────────────────────────────────────────────────────────────────── +// Rejected codes +// ───────────────────────────────────────────────────────────────────────────── + +/// How many codes may be refused before the flow gives up. +/// +/// The CLI will retry forever, which is the wrong bound for a dialog: the same +/// truncated clipboard pasted a fourth time will fail a fourth time, and a flow +/// that never resolves is indistinguishable from the hang this replaced. +const MAX_CODE_ATTEMPTS: usize = 3; + +/// How much recent output to keep for [`detect_code_rejection`]. The message +/// lands within a few hundred characters of the paste; anything older belongs to +/// a previous attempt. +const REJECTION_SCAN_WINDOW: usize = 4096; + +/// Phrases `claude setup-token` prints when it refuses a pasted code. +/// +/// Measured against 2.1.226 under a pty. The CLI writes +/// +/// ```text +/// OAuth error: Invalid code. Please make sure the full code was copied +/// Press Enter to retry. +/// ``` +/// +/// on two lines placed with cursor motion rather than newlines, then blocks on +/// stdin. Either phrase is enough — matching both would make a change to one of +/// them silently restore the hang. +const CODE_REJECTED_MARKERS: &[&str] = &["invalid code", "press enter to retry"]; + +/// Append `chunk` to `buf`, keeping no more than `cap` bytes of the tail. +fn push_capped_tail(buf: &mut String, chunk: &str, cap: usize) { + buf.push_str(chunk); + if buf.len() <= cap { + return; + } + let cut = buf.len() - cap; + let cut = (cut..buf.len()) + .find(|i| buf.is_char_boundary(*i)) + .unwrap_or(buf.len()); + buf.drain(..cut); +} + +/// Whether `text` shows the CLI has refused a code and parked on stdin. +/// +/// Whitespace is collapsed before matching because [`strip_ansi_prefix`] turns +/// the cursor moves that lay this message out into spaces and line breaks, so +/// the phrase arrives with runs of blanks inside it that are not in the source +/// string. +fn detect_code_rejection(text: &str) -> bool { + let normalized: String = text + .to_ascii_lowercase() + .split_whitespace() + .collect::>() + .join(" "); + CODE_REJECTED_MARKERS + .iter() + .any(|marker| normalized.contains(marker)) } // ───────────────────────────────────────────────────────────────────────────── @@ -481,17 +796,43 @@ fn emit_output(app: &AppHandle, project_id: &str, chunk: &str) { ); } +fn emit_link(app: &AppHandle, project_id: &str, url: &str) { + let _ = app.emit( + LINK_EVENT, + serde_json::json!({ "project_id": project_id, "url": url }), + ); +} + +fn emit_code_rejected(app: &AppHandle, project_id: &str, message: &str, remaining: usize) { + let _ = app.emit( + CODE_REJECTED_EVENT, + serde_json::json!({ + "project_id": project_id, + "message": message, + "attempts_remaining": remaining, + }), + ); +} + /// Shell run inside the container. /// /// * `stty` widens the pty before Claude Code starts, so its layout engine does /// not wrap the token or the sign-in URL across lines. Docker's default exec /// pty is 80 columns; both are longer than that. Setting it here rather than /// via a post-start resize avoids racing the process's startup. +/// +/// 400 columns, not 200: the sign-in URL is 346 characters (measured against +/// 2.1.226), so at 200 it wrapped anyway. Widening is *not* the fix for that +/// — this line is `|| true` and fails silently, which is precisely how the +/// truncated-URL bug survived — but it removes wrapping as a variable +/// everywhere else in the flow. The two things that must survive a wrap +/// regardless are handled directly: the URL comes from the OSC 8 parameter, +/// and [`scan_credential_body`] reassembles a split token. /// * The `unset` line strips inherited auth so `setup-token` runs against a /// clean claude.ai login instead of warning about, or deferring to, whatever /// credential the container is already configured with — including a shared /// token from a previous run, which is likely the very thing being replaced. -const SETUP_TOKEN_SCRIPT: &str = r#"stty cols 200 rows 50 2>/dev/null || true +const SETUP_TOKEN_SCRIPT: &str = r#"stty cols 400 rows 50 2>/dev/null || true unset CLAUDE_CODE_OAUTH_TOKEN ANTHROPIC_API_KEY ANTHROPIC_AUTH_TOKEN ANTHROPIC_BASE_URL \ ANTHROPIC_MODEL CLAUDE_CODE_USE_BEDROCK AWS_BEARER_TOKEN_BEDROCK exec claude setup-token"#; @@ -528,6 +869,17 @@ async fn run_setup_token( let mut transcript = String::new(); let deadline = tokio::time::Instant::now() + SETUP_TIMEOUT; + // The last hyperlink forwarded, so the five consecutive emissions the CLI + // makes for one wrapped URL become one event. Only the immediately previous + // one is remembered — the CLI reprints the same URL after a retry, and a + // genuinely different one would be news. + let mut last_link: Option = None; + // Recent visible output, scanned for a rejection message. Only filled while + // a code is outstanding, so a repaint of an old message cannot re-fire. + let mut recent = String::new(); + let mut awaiting_code_result = false; + let mut rejected_codes = 0usize; + loop { // Writing stdin and reading stdout are driven from the same loop: with // a hijacked exec both halves ride one socket, and `input` must stay @@ -550,6 +902,10 @@ async fn run_setup_token( )); } let _ = input.flush().await; + // Arm the rejection detector. Anything the CLI says from here + // on is a verdict on *this* code. + awaiting_code_result = true; + recent.clear(); continue; } next = tokio::time::timeout_at(deadline, output.next()) => match next { @@ -576,10 +932,67 @@ async fn run_setup_token( }; let visible = stripper.push(&frame.into_bytes()); + + // Before the emptiness check: a chunk can be nothing but hyperlink + // wrappers, which is exactly the chunk carrying the sign-in URL. + for link in stripper.take_links() { + if !usable_sign_in_link(&link) || last_link.as_deref() == Some(link.as_str()) { + continue; + } + emit_link(app, project_id, &link); + last_link = Some(link); + } + if visible.is_empty() { continue; } + // A refused code parks the CLI on stdin instead of ending it, so this is + // the only thing between the user and a 15-minute wait. + if awaiting_code_result { + push_capped_tail(&mut recent, &visible, REJECTION_SCAN_WINDOW); + if detect_code_rejection(&recent) { + awaiting_code_result = false; + recent.clear(); + rejected_codes += 1; + + if rejected_codes >= MAX_CODE_ATTEMPTS { + return Err(format!( + "`claude setup-token` rejected the code {} times, so the sign-in was \ + abandoned. No token was stored. The code is long and easy to \ + truncate — copy all of it from the Anthropic page, then start \ + authentication again.", + rejected_codes + )); + } + + // The CLI is parked on "Press Enter to retry" and will not draw + // its paste prompt again until it gets that Enter. Send it, so + // the user's next code has somewhere to land. Bounded above, and + // announced below — this is a retry the user is told about, not + // a loop hidden from them. + if let Err(e) = input.write_all(b"\r").await { + return Err(format!( + "`claude setup-token` rejected the code and could not be asked to \ + retry: {}. No token was stored.", + e + )); + } + let _ = input.flush().await; + + let remaining = MAX_CODE_ATTEMPTS - rejected_codes; + let message = format!( + "That code was rejected — `claude setup-token` reports the full code was \ + not copied. Copy it again from the Anthropic page and submit it; {} \ + attempt{} left.", + remaining, + if remaining == 1 { "" } else { "s" } + ); + emit_code_rejected(app, project_id, &message, remaining); + emit_progress(app, project_id, &message); + } + } + transcript.push_str(&visible); if transcript.len() > MAX_TRANSCRIPT { // Keep the tail: that is where the token lands. @@ -601,20 +1014,40 @@ async fn run_setup_token( emit_output(app, project_id, &tail); } - let exit_code = wait_for_exec_exit(&exec_id).await.unwrap_or(0); - if exit_code != 0 { - return Err(format!( - "`claude setup-token` exited with status {}. No token was stored — \ - see the command output above for what went wrong.", - exit_code - )); + // `None` means the exit code could not be determined, not that it was zero. + // Falling through to the token parse is right either way — a run that + // printed a token succeeded whatever Docker says about it — but it is worth + // saying so rather than silently calling it a clean exit. + match wait_for_exec_exit(&exec_id).await { + Some(0) => {} + Some(code) => { + return Err(format!( + "`claude setup-token` exited with status {}. No token was stored — \ + see the command output above for what went wrong.", + code + )) + } + None => log::warn!( + "Could not read the exit status of `claude setup-token`; judging the run \ + by whether it printed a token" + ), } parse_setup_token(&transcript).ok_or_else(|| { - "`claude setup-token` finished but printed no recognisable token. \ - Nothing was stored. This usually means the login was cancelled, or the \ - account has no Claude subscription (long-lived tokens require one)." - .to_string() + if rejected_codes > 0 { + format!( + "`claude setup-token` ended without a token after rejecting {} code{}. \ + Nothing was stored. Copy the whole code from the Anthropic page — it is \ + long and easy to truncate — then start authentication again.", + rejected_codes, + if rejected_codes == 1 { "" } else { "s" } + ) + } else { + "`claude setup-token` finished but printed no recognisable token. \ + Nothing was stored. This usually means the login was cancelled, or the \ + account has no Claude subscription (long-lived tokens require one)." + .to_string() + } }) } @@ -1012,11 +1445,11 @@ mod tests { visible.push_str(&s.push(b"1mb")); assert_eq!(visible, "ab"); } - // ── Truncated credentials ──────────────────────────────────────────── - // `stty cols 200` runs before Claude Code starts precisely so the ~103 - // character token never wraps. If that fails, the pty falls back to 80 - // columns and the token arrives split across two lines. The old floor of - // 32 body characters believed the first half. + // ── Truncated and wrapped credentials ──────────────────────────────── + // `stty cols 400` runs before Claude Code starts precisely so the ~103 + // character token never wraps. But that line is `|| true` and fails + // silently, and an 80-column fallback splits the value across two lines. + // A fragment must never be accepted; the whole value, reassembled, must be. #[test] fn a_token_shorter_than_a_real_one_is_rejected() { @@ -1028,18 +1461,107 @@ mod tests { } #[test] - fn a_line_wrapped_token_yields_nothing_rather_than_half_a_credential() { + fn a_truncated_token_with_no_continuation_is_still_rejected() { + let tok = token('N'); + // The first half of an 80-column wrap, and nothing after it. + let half = format!("Your token: {}\n", &tok[..68]); + assert_eq!( + parse_setup_token(&half), + None, + "half a credential must fail loudly, not be stored truncated" + ); + } + + #[test] + fn a_line_wrapped_token_is_reassembled_whole() { let tok = token('N'); // Column 12 is where `Your token: ` ends, so an 80-column pty breaks // the value 68 characters in. let wrapped = format!("Your token: {}\n{}\n", &tok[..68], &tok[68..]); assert_eq!( parse_setup_token(&wrapped), - None, - "a wrapped token must fail loudly, not be stored truncated" + Some(tok), + "the halves of a wrapped token belong to one credential" ); } + #[test] + fn a_token_wrapped_twice_is_reassembled_whole() { + let tok = token('N'); + // A 40-column pty needs two breaks for a 103-character value. + let wrapped = format!("{}\n{}\n{}\n", &tok[..40], &tok[40..80], &tok[80..]); + assert_eq!(parse_setup_token(&wrapped), Some(tok)); + } + + /// The guard that keeps reassembly from becoming a fabrication machine: a + /// break early in a line is prose, not the terminal's right margin, so the + /// two sides are two different things and must not be welded together. + #[test] + fn a_break_before_the_margin_does_not_join_two_lines() { + let tail = "N".repeat(70); + let text = format!("{}ABC\n{}\n", TOKEN_PREFIX, tail); + assert_eq!( + parse_setup_token(&text), + None, + "a short first line is prose; joining it would invent a credential" + ); + } + + /// A repainting TUI prints the same line again and again, so the character + /// after a line break is very often the start of the next frame. Joining + /// there would hand back `Your` instead of ``. + #[test] + fn a_repaint_after_a_complete_token_is_not_joined_onto_it() { + let tok = token('P'); + let output = format!("Your token: {}\n", tok).repeat(3); + assert_eq!(parse_setup_token(&output), Some(tok)); + } + + /// The wrap has to produce a *whole* credential to be believed. Two short + /// pieces that still fall short of the floor are not one. + #[test] + fn joining_a_wrap_does_not_lower_the_length_floor() { + let body_a = "N".repeat(45); + let body_b = "N".repeat(20); + let text = format!("{}{}\n{}\n", TOKEN_PREFIX, body_a, body_b); + assert_eq!(parse_setup_token(&text), None); + } + + /// The security half of the same bug. Before the parser could reassemble a + /// wrapped token, the redactor could not either: it masked the first line, + /// which carries the `sk-ant-` marker, and printed the second — the tail of + /// a live credential — to the UI in clear. + #[test] + fn redaction_masks_both_halves_of_a_wrapped_token() { + let tok = token('R'); + let mut r = SecretRedactor::default(); + let mut seen = r.push(&format!("Your token: {}\n{}\n", &tok[..68], &tok[68..])); + seen.push_str(&r.flush()); + assert!(!seen.contains(TOKEN_PREFIX), "leaked: {}", seen); + assert!( + !seen.contains(&tok[68..]), + "the tail of the credential reached the UI: {}", + seen + ); + assert!(seen.contains(SECRET_PLACEHOLDER)); + } + + /// …and the same when the wrap lands on a chunk boundary, which is the way + /// it actually arrives off the socket. + #[test] + fn redaction_masks_a_wrapped_token_split_across_chunks() { + let tok = token('S'); + let mut r = SecretRedactor::default(); + let mut seen = r.push(&format!("Your token: {}", &tok[..68])); + seen.push_str(&r.push("\n")); + seen.push_str(&r.push(&tok[68..])); + seen.push_str(&r.push("\ndone\n")); + seen.push_str(&r.flush()); + assert!(!seen.contains(TOKEN_PREFIX), "leaked: {}", seen); + assert!(!seen.contains(&tok[68..]), "leaked tail: {}", seen); + assert!(seen.ends_with("\ndone\n")); + } + #[test] fn a_real_length_token_is_still_accepted() { // Guards the floor from being raised past what Anthropic actually mints. @@ -1049,6 +1571,165 @@ mod tests { assert_eq!(parse_setup_token(&format!("{}\n", tok)), Some(tok)); } + // ── OSC 8 sign-in link ─────────────────────────────────────────────── + // The URL only exists in one piece inside the hyperlink parameter. Its + // visible text is chopped into terminal-width slices, each of them a + // separate, complete hyperlink emission — measured against 2.1.226 at 80 + // columns, where a 346-character URL arrives as five of them. + + /// The real sign-in URL's shape and length, from the pty capture. + fn sign_in_url() -> String { + let url = format!( + "https://claude.com/cai/oauth/authorize?code=true&client_id=9d1c250a-e61b-44d9-88ed-\ + 5944d1962f5e&response_type=code&redirect_uri=https%3A%2F%2Fplatform.claude.com%2F\ + oauth%2Fcode%2Fcallback&scope=user%3Ainference&code_challenge={}&\ + code_challenge_method=S256&state=su-{}", + "R".repeat(43), + "x".repeat(40) + ); + assert!(url.len() > 300, "the point of this fixture is its length"); + url + } + + /// One hyperlink emission: the whole URL in the parameter, `visible` on + /// screen, then the empty `8;;` that closes it. + fn osc8(url: &str, visible: &str) -> String { + format!( + "\x1b]8;id=1umaq0e;{}\x07\x1b[38;2;153;153;153m{}\x1b[39m\x1b]8;;\x07", + url, visible + ) + } + + #[test] + fn the_full_url_survives_a_display_text_wrapped_mid_url() { + let url = sign_in_url(); + let mut frame = String::new(); + for slice in url.as_bytes().chunks(80) { + frame.push_str(&osc8(&url, std::str::from_utf8(slice).unwrap())); + frame.push_str("\r\r\n"); + } + + let mut s = AnsiStripper::default(); + let visible = s.push(frame.as_bytes()); + let links = s.take_links(); + + // The visible text is exactly the broken form the old scraper saw. + assert!(visible.contains(&format!("{}\n", &url[..80]))); + assert!(!visible.contains(&url)); + + assert!(links.iter().all(|l| *l == url), "links: {:?}", links); + assert_eq!(links.len(), 5, "one emission per display slice"); + assert!(usable_sign_in_link(&links[0])); + } + + #[test] + fn the_closing_half_of_a_hyperlink_is_not_a_link() { + let mut s = AnsiStripper::default(); + s.push(b"\x1b]8;;\x07"); + assert!(s.take_links().is_empty()); + } + + #[test] + fn a_non_hyperlink_osc_is_not_a_link() { + let mut s = AnsiStripper::default(); + // Window title, and the app's own URL relay sequence. + s.push(b"\x1b]0;a title\x07\x1b]7777;open;aHR0cHM6Ly9ldmlsLnRsZA==\x07"); + assert!(s.take_links().is_empty()); + } + + #[test] + fn a_hyperlink_split_across_chunks_still_yields_its_target() { + let url = sign_in_url(); + let frame = osc8(&url, &url[..80]); + let (head, tail) = frame.as_bytes().split_at(60); + + let mut s = AnsiStripper::default(); + s.push(head); + assert!(s.take_links().is_empty(), "incomplete: nothing to report yet"); + s.push(tail); + assert_eq!(s.take_links().first().map(String::as_str), Some(url.as_str())); + } + + #[test] + fn only_plausible_http_targets_reach_the_frontend() { + assert!(usable_sign_in_link("https://claude.ai/oauth/authorize?code=true")); + assert!(usable_sign_in_link("http://127.0.0.1:8123/callback")); + // The host allowlist lives on the frontend; these are the shallow + // checks that keep junk off the wire. + assert!(!usable_sign_in_link("file:///etc/passwd")); + assert!(!usable_sign_in_link("javascript:alert(1)")); + assert!(!usable_sign_in_link("https://claude.ai/a b")); + assert!(!usable_sign_in_link("https://claude.ai/\u{7f}")); + assert!(!usable_sign_in_link(&format!( + "https://claude.ai/?t={}", + "A".repeat(MAX_LINK_LENGTH) + ))); + // Nothing credential-shaped may ride the one path that skips redaction. + assert!(!usable_sign_in_link(&format!( + "https://claude.ai/?t={}", + token('T') + ))); + } + + /// Claude Code prefixes every repaint frame with `ESC ( B`. Treating that + /// as a two-byte escape emitted the `B` as text — and a stray `B` glued to + /// the front of a token makes [`parse_setup_token`] refuse it. + #[test] + fn a_charset_designation_does_not_leave_a_letter_behind() { + let tok = token('U'); + let mut s = AnsiStripper::default(); + let visible = s.push(format!("\x1b(B\x0f{}\r\n", tok).as_bytes()); + assert_eq!(visible, format!("{}\n", tok)); + assert_eq!(parse_setup_token(&visible), Some(tok)); + } + + // ── Rejected codes ─────────────────────────────────────────────────── + + /// The exact bytes 2.1.226 emits after a bad paste, from the pty capture. + const REJECTION_FRAME: &[u8] = + b"\x1b(B\x0f\x1b[2K\x1b[1A\x1b[2K\x1b[G\x1b[1A\r\x1b[1C\x1b[4A\x1b[38;2;255;107;128m\ + OAuth error: Invalid code. Please make sure the full code was copied\ + \r\x1b[2B\x1b[39m\x1b[K\r\x1b[1B \x1b[38;2;177;185;249mPress \x1b[1mEnter\x1b[22m \ + to retry.\x1b[39m\x1b[K\r\x1b[5A"; + + #[test] + fn the_real_rejection_frame_is_recognised_after_ansi_stripping() { + let mut s = AnsiStripper::default(); + let visible = s.push(REJECTION_FRAME); + assert!( + detect_code_rejection(&visible), + "a rejected code must be seen, not waited out: {:?}", + visible + ); + } + + #[test] + fn cursor_motion_inside_the_message_does_not_hide_it() { + // The layout engine can place these words with column jumps, which + // become runs of spaces. Matching must survive that. + let mut s = AnsiStripper::default(); + let visible = s.push(b"\x1b[2GPress\x1b[9GEnter\x1b[16Gto\x1b[20Gretry."); + assert!(detect_code_rejection(&visible)); + } + + #[test] + fn ordinary_progress_output_is_not_mistaken_for_a_rejection() { + assert!(!detect_code_rejection( + "Browser didn't open? Use the url below to sign in (c to copy)" + )); + assert!(!detect_code_rejection("Paste code here if prompted >")); + assert!(!detect_code_rejection("Login successful! Your token:")); + assert!(!detect_code_rejection("")); + } + + /// The retry budget has to be finite: the CLI itself will loop forever, and + /// a flow that never resolves is the hang this replaced wearing a hat. + #[test] + fn the_retry_budget_is_bounded_and_leaves_room_for_a_retry() { + assert!(MAX_CODE_ATTEMPTS >= 2, "one attempt is not a retry"); + assert!(MAX_CODE_ATTEMPTS <= 5, "the budget must actually run out"); + } + // ── Bounded buffering ──────────────────────────────────────────────── #[test] @@ -1081,3 +1762,4 @@ mod tests { assert_eq!(parse_setup_token(&seen), Some(tok)); } } + diff --git a/app/src-tauri/src/lib.rs b/app/src-tauri/src/lib.rs index 9998f80..d442ede 100644 --- a/app/src-tauri/src/lib.rs +++ b/app/src-tauri/src/lib.rs @@ -426,6 +426,8 @@ pub fn run() { browser_view::commands::set_browser_view_enabled, browser_view::commands::get_browser_view_status, browser_view::commands::check_browser_view_support, + browser_view::commands::install_browser_view_support, + browser_view::commands::install_browser_view_browser, // Shared Claude Code auth token commands::auth_token_commands::acquire_claude_token, commands::auth_token_commands::submit_claude_token_code, diff --git a/app/src/components/projects/home/BrowserTab.test.tsx b/app/src/components/projects/home/BrowserTab.test.tsx index 0cd7221..6f63510 100644 --- a/app/src/components/projects/home/BrowserTab.test.tsx +++ b/app/src/components/projects/home/BrowserTab.test.tsx @@ -1,23 +1,41 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; import { act, fireEvent, render, screen, waitFor } from "@testing-library/react"; import BrowserTab from "./BrowserTab"; -import type { BrowserViewStatus, Project } from "../../../lib/types"; +import type { + BrowserSetupOutcome, + BrowserViewStatus, + PlaywrightDetection, + Project, +} from "../../../lib/types"; const getBrowserViewStatus = vi.fn<() => Promise>(); const setBrowserViewEnabled = vi.fn<() => Promise>(); +const checkBrowserViewSupport = vi.fn<() => Promise>(); +const installBrowserViewSupport = vi.fn<() => Promise>(); +const installBrowserViewBrowser = vi.fn<(id: string, b: string) => Promise>(); const pushToast = vi.fn(); +const setContainerProgress = vi.fn(); vi.mock("../../../lib/tauri-commands", () => ({ getBrowserViewStatus: () => getBrowserViewStatus(), setBrowserViewEnabled: () => setBrowserViewEnabled(), + checkBrowserViewSupport: () => checkBrowserViewSupport(), + installBrowserViewSupport: () => installBrowserViewSupport(), + installBrowserViewBrowser: (id: string, b: string) => installBrowserViewBrowser(id, b), })); vi.mock("@tauri-apps/api/event", () => ({ listen: vi.fn(async () => () => {}), })); +const storeState = { + pushToast, + setContainerProgress, + containerProgress: {} as Record, +}; + vi.mock("../../../store/appState", () => ({ - useAppState: (selector: (s: unknown) => unknown) => selector({ pushToast }), + useAppState: (selector: (s: unknown) => unknown) => selector(storeState), })); const OFF: BrowserViewStatus = { @@ -31,6 +49,33 @@ const OFF: BrowserViewStatus = { message: null, }; +const NOTHING: PlaywrightDetection = { + node_version: "22.11.0", + playwright_version: null, + playwright_path: null, + playwright_cli: null, + has_bind: false, + cli_version: null, + cli_entry: null, + browsers: [], + chrome_channel: null, + searched: [ + "/workspace", + "/usr/lib/node_modules", + "/home/claude/.npm/_npx/9f3a/node_modules", + ], +}; + +const READY: PlaywrightDetection = { + ...NOTHING, + playwright_version: "1.62.1", + playwright_path: "/workspace/node_modules/playwright-core/package.json", + playwright_cli: "/workspace/node_modules/playwright-core/cli.js", + has_bind: true, + cli_version: "0.1.18", + cli_entry: "/workspace/node_modules/@playwright/cli/playwright-cli.js", +}; + const project: Project = { id: "p1", name: "api-server", @@ -63,7 +108,9 @@ const project: Project = { beforeEach(() => { vi.clearAllMocks(); + storeState.containerProgress = {}; getBrowserViewStatus.mockResolvedValue(OFF); + checkBrowserViewSupport.mockResolvedValue(READY); }); describe("BrowserTab", () => { @@ -72,17 +119,24 @@ describe("BrowserTab", () => { expect(await screen.findByText(/container isn’t running/i)).toBeInTheDocument(); expect(screen.queryByRole("button", { name: /start browser view/i })).toBeNull(); expect(getBrowserViewStatus).not.toHaveBeenCalled(); + expect(checkBrowserViewSupport).not.toHaveBeenCalled(); }); - it("starts off, and never starts a view without being asked", async () => { + it("starts off, and never starts a view or installs anything without being asked", async () => { + checkBrowserViewSupport.mockResolvedValue({ ...READY, browsers: ["chromium-1200"] }); render(); await waitFor(() => expect(getBrowserViewStatus).toHaveBeenCalled()); expect(screen.getByText("Off")).toBeInTheDocument(); expect(screen.queryByTitle(/browser view for/i)).toBeNull(); expect(setBrowserViewEnabled).not.toHaveBeenCalled(); + // Probing is read-only and expected; installing is a mutation and is not. + await waitFor(() => expect(checkBrowserViewSupport).toHaveBeenCalled()); + expect(installBrowserViewSupport).not.toHaveBeenCalled(); + expect(installBrowserViewBrowser).not.toHaveBeenCalled(); }); it("shows the live pane, pointed at loopback with a token, once started", async () => { + checkBrowserViewSupport.mockResolvedValue({ ...READY, browsers: ["chromium-1200"] }); setBrowserViewEnabled.mockResolvedValue({ ...OFF, enabled: true, @@ -109,27 +163,121 @@ describe("BrowserTab", () => { expect(screen.getByRole("button", { name: "Stop" })).toBeInTheDocument(); }); + it("offers setup before the user hits a wall, naming what is missing", async () => { + checkBrowserViewSupport.mockResolvedValue(NOTHING); + + render(); + + // No Start attempt was needed to learn this. + expect(await screen.findByRole("button", { name: /set up playwright/i })).toBeInTheDocument(); + expect(screen.getByText(/Missing: playwright, @playwright\/cli/)).toBeInTheDocument(); + // The npx cache is shown among the searched roots — that is where an + // MCP-installed Playwright actually lives. + expect(screen.getByText(/_npx\/9f3a\/node_modules/)).toBeInTheDocument(); + // A browser can't be installed before Playwright is. + expect(screen.getByRole("button", { name: /install chromium/i })).toBeDisabled(); + }); + + it("installs Playwright on request and updates itself from the fresh probe", async () => { + checkBrowserViewSupport.mockResolvedValue(NOTHING); + installBrowserViewSupport.mockResolvedValue({ + detection: READY, + log: "added 5 packages in 3s", + browser_launched: null, + warning: "Playwright is installed, but this container has no browser to drive yet.", + }); + + render(); + const button = await screen.findByRole("button", { name: /set up playwright/i }); + await act(async () => { + fireEvent.click(button); + }); + + await waitFor(() => expect(installBrowserViewSupport).toHaveBeenCalled()); + // The pane re-rendered from the returned probe — no reopening the tab. + expect(await screen.findByText("1.62.1")).toBeInTheDocument(); + // Stated in the warning box, and again in the pane's own summary line. + expect(screen.getAllByText(/no browser to drive yet/).length).toBeGreaterThan(0); + // And the browser buttons are now live. + expect(screen.getByRole("button", { name: /install chromium/i })).toBeEnabled(); + expect(screen.getByRole("button", { name: /install chrome channel/i })).toBeEnabled(); + // The progress line is always cleared, whatever happened. + expect(setContainerProgress).toHaveBeenCalledWith("p1", null); + }); + + it("says which browser is for which caller, and states the size first", async () => { + checkBrowserViewSupport.mockResolvedValue(READY); + render(); + + expect(await screen.findByText(/several hundred mb/i)).toBeInTheDocument(); + // The copy is broken across a element, so match the container. + expect( + screen.getByText((_, el) => + (el?.textContent ?? "").includes("@playwright/mcp") && + (el?.textContent ?? "").includes("asks for") && + el?.tagName.toLowerCase() === "li", + ), + ).toBeInTheDocument(); + expect(screen.getByText(/roughly 150 mb/i)).toBeInTheDocument(); + }); + + it("installs the chrome channel when that is the one asked for", async () => { + checkBrowserViewSupport.mockResolvedValue(READY); + installBrowserViewBrowser.mockResolvedValue({ + detection: { ...READY, chrome_channel: "/usr/bin/google-chrome-stable" }, + log: "Installing google-chrome-stable", + browser_launched: true, + warning: null, + }); + + render(); + const button = await screen.findByRole("button", { name: /install chrome channel/i }); + await act(async () => { + fireEvent.click(button); + }); + + await waitFor(() => + expect(installBrowserViewBrowser).toHaveBeenCalledWith("p1", "chrome"), + ); + // Shown as the step's "done" line and again in the diagnostics table. + await waitFor(() => + expect(screen.getAllByText(/google-chrome-stable/).length).toBeGreaterThan(0), + ); + }); + + it("reports an install failure with the real command output", async () => { + checkBrowserViewSupport.mockResolvedValue(NOTHING); + installBrowserViewSupport.mockRejectedValue( + "npm couldn't install Playwright in this container (exit 1).\n\nnpm said:\nEACCES: permission denied", + ); + + render(); + const button = await screen.findByRole("button", { name: /set up playwright/i }); + await act(async () => { + fireEvent.click(button); + }); + + expect(await screen.findByText(/EACCES: permission denied/)).toBeInTheDocument(); + expect(pushToast).toHaveBeenCalledWith( + expect.objectContaining({ kind: "error" }), + ); + expect(setContainerProgress).toHaveBeenCalledWith("p1", null); + }); + it("explains precisely what is missing instead of spinning", async () => { + checkBrowserViewSupport.mockRejectedValue("container busy"); getBrowserViewStatus.mockResolvedValue({ ...OFF, enabled: true, state: "unavailable", message: - "Playwright isn't installed in this container. Install it with `npm i -D playwright`.", - detection: { - node_version: "22.11.0", - playwright_version: null, - playwright_path: null, - has_bind: false, - cli_version: null, - cli_entry: null, - searched: ["/workspace", "/usr/lib/node_modules"], - }, + "Playwright isn't installed in this container. Two packages are needed: `playwright` and `@playwright/cli`.", + detection: NOTHING, }); render(); - expect(await screen.findByText(/npm i -D playwright/)).toBeInTheDocument(); + expect(await screen.findByText(/Two packages are needed/)).toBeInTheDocument(); expect(screen.getByText("Unavailable")).toBeInTheDocument(); // The probe's findings are shown, so the user can see why. expect(screen.getByText("22.11.0")).toBeInTheDocument(); @@ -139,6 +287,7 @@ describe("BrowserTab", () => { }); it("surfaces a start failure rather than leaving the pane blank", async () => { + checkBrowserViewSupport.mockResolvedValue({ ...READY, browsers: ["chromium-1200"] }); setBrowserViewEnabled.mockRejectedValue("container went away"); render(); @@ -155,6 +304,7 @@ describe("BrowserTab", () => { }); it("stops the view when asked", async () => { + checkBrowserViewSupport.mockResolvedValue({ ...READY, browsers: ["chromium-1200"] }); getBrowserViewStatus.mockResolvedValue({ ...OFF, enabled: true, diff --git a/app/src/components/projects/home/BrowserTab.tsx b/app/src/components/projects/home/BrowserTab.tsx index 8e1fde0..82221f0 100644 --- a/app/src/components/projects/home/BrowserTab.tsx +++ b/app/src/components/projects/home/BrowserTab.tsx @@ -1,15 +1,22 @@ import { useCallback, useEffect, useRef, useState } from "react"; import { listen } from "@tauri-apps/api/event"; import type { + BrowserInstallTarget, + BrowserSetupOutcome, BrowserViewChangedEvent, BrowserViewStatus, + PlaywrightDetection, Project, } from "../../../lib/types"; import { + checkBrowserViewSupport, getBrowserViewStatus, + installBrowserViewBrowser, + installBrowserViewSupport, setBrowserViewEnabled, } from "../../../lib/tauri-commands"; import { useAppState } from "../../../store/appState"; +import AccordionSection from "../../ui/AccordionSection"; import Button from "../../ui/Button"; import StatusIndicator from "../../ui/StatusIndicator"; @@ -29,6 +36,9 @@ const OFF: BrowserViewStatus = { message: null, }; +/** Which install is in flight. `null` means none — nothing installs itself. */ +type SetupJob = null | "packages" | BrowserInstallTarget; + /** * Watch — and take over — the browser Claude is driving with Playwright inside * the container. @@ -38,6 +48,12 @@ const OFF: BrowserViewStatus = { * loopback. Nothing starts until the user asks: this is remote control of a * browser in a privileged sandbox, so it is off by default and opted into per * project, exactly like the auth bridge. + * + * The same rule, harder, applies to setup. Opening this tab *probes* the + * container (one `node -e`, read-only) so the pane can say what is missing + * before the user asks for a view — but it never installs anything. Installing + * packages and downloading a browser are container mutations measured in + * hundreds of megabytes; both are separate, labelled, user-pressed buttons. */ export default function BrowserTab({ project, active }: Props) { const [status, setStatus] = useState(OFF); @@ -45,7 +61,14 @@ export default function BrowserTab({ project, active }: Props) { const [error, setError] = useState(null); /** Bumped to force the iframe to reload without changing its src. */ const [reloadKey, setReloadKey] = useState(0); + /** Last read-only probe of the container, for the setup panel. */ + const [detection, setDetection] = useState(null); + const [job, setJob] = useState(null); + const [outcome, setOutcome] = useState(null); + const [setupError, setSetupError] = useState(null); const pushToast = useAppState((s) => s.pushToast); + const setContainerProgress = useAppState((s) => s.setContainerProgress); + const progress = useAppState((s) => s.containerProgress[project.id]); const running = project.status === "running"; // The backend is the source of truth: it emits whenever a view starts or is @@ -77,6 +100,11 @@ export default function BrowserTab({ project, active }: Props) { getBrowserViewStatus(projectId) .then((s) => mounted.current && setStatus(s)) .catch(() => {}); + // Read-only. This is what lets the pane offer setup before the user hits a + // wall, and it is why a "not installed" answer is never stale. + checkBrowserViewSupport(projectId) + .then((d) => mounted.current && setDetection(d)) + .catch(() => {}); }, [active, projectId, running]); const toggle = useCallback( @@ -101,8 +129,52 @@ export default function BrowserTab({ project, active }: Props) { [projectId, pushToast], ); - // A stopped container can't be hosting a browser, so say that plainly rather - // than offering a control that would only fail. + /** Run one install. Every path clears the progress line it started. */ + const install = useCallback( + async (which: Exclude) => { + setJob(which); + setSetupError(null); + setOutcome(null); + try { + const result = + which === "packages" + ? await installBrowserViewSupport(projectId) + : await installBrowserViewBrowser(projectId, which); + if (!mounted.current) return; + // The command re-probes, so the pane updates itself — no reopening the + // tab, no second button to press. + setDetection(result.detection); + setOutcome(result); + if (result.warning) { + // Not an error — the step did what it said — but the caveat is the + // part that decides whether the browser will actually work. + pushToast({ + kind: "info", + message: "Setup finished, with something to know", + detail: result.warning, + }); + } else { + pushToast({ + kind: "success", + message: + which === "packages" ? "Playwright installed" : `${which} installed and verified`, + }); + } + } catch (e) { + const detail = String(e); + if (mounted.current) setSetupError(detail); + pushToast({ kind: "error", message: "Setup failed", detail }); + } finally { + setContainerProgress(projectId, null); + if (mounted.current) setJob(null); + } + }, + [projectId, pushToast, setContainerProgress], + ); + + // A stopped container can't be hosting a browser — and can't be installed + // into either, so say that plainly rather than offering controls that would + // only fail. if (!running) { return ( @@ -113,6 +185,16 @@ export default function BrowserTab({ project, active }: Props) { } const live = status.state === "running" && status.url; + // Prefer the probe: it is the fresher of the two, and it is the one that + // reflects an install that just finished. + const probed = detection ?? status.detection; + const ready = isUsable(probed); + // Mirrors Rust `PlaywrightDetection::needs_browser`: the Chrome channel is an + // apt package, so it never shows up in `browsers`, and a container that has + // it is not missing a browser. + const needsBrowser = + probed !== null && probed.browsers.length === 0 && probed.chrome_channel === null; + const needsSetup = probed !== null && (!ready || needsBrowser); return (
@@ -151,7 +233,7 @@ export default function BrowserTab({ project, active }: Props) { + } + /> + + + Both check the system libraries a browser links against first. Current base + images ship them, so that step is normally skipped; a container built from an + older image gets them installed with apt, which is the difference between a + browser that downloads successfully and one that also starts. Both end by + actually launching the browser to prove it works. Browsers land in{" "} + ~/.cache/ms-playwright, which is on the home volume, so they + survive container recreation and are only lost on a project Reset. + + } + done={browsers.length > 0 || chrome !== null} + doneLabel={[ + browsers.length > 0 ? browsers.join(", ") : null, + chrome ? `Chrome channel (${chrome})` : null, + ] + .filter(Boolean) + .join(" · ")} + action={ +
+ + +
+ } + > +
    +
  • + Chromium — Playwright’s + own build, used by chromium.launch() with no channel. Several + hundred MB. +
    • +
  • + Chrome channel — Google + Chrome from apt, which is what @playwright/mcp asks for. Install + this one if Claude drives the browser through the MCP plugin. Roughly 150 MB. +
    • +
+ + + {busy && ( +

+ {progress ?? "Working…"} +

+ )} + + {error && ( +
+

That didn’t work.

+ 
+            {error}
+
+
+ )} + + {outcome?.warning && ( +
+

Worth knowing

+

+ {outcome.warning} +

+
+ )} + + {outcome?.log && ( + + 
+            {outcome.log}
+
+ + )} + + {detection && ( +
+ + + + + + 0 ? browsers.join(", ") : null} + /> + + {detection.searched.length > 0 && ( + )}
)} @@ -216,6 +502,44 @@ function Unavailable({ status }: { status: BrowserViewStatus }) { ); } +/** One numbered setup step: what it does, whether it is done, and its button. */ +function Step({ + title, + detail, + done, + doneLabel, + action, + children, +}: { + title: string; + detail: React.ReactNode; + done: boolean; + doneLabel?: string; + action: React.ReactNode; + children?: React.ReactNode; +}) { + return ( +
+
+
+
+

{title}

+ +
+

{detail}

+ {done && doneLabel && ( +

+ {doneLabel} +

+ )} + {children} +
+
{action}
+
+
+ ); +} + function Detail({ label, value }: { label: string; value: string | null }) { return ( <> diff --git a/app/src/components/settings/ClaudeAuthModal.test.tsx b/app/src/components/settings/ClaudeAuthModal.test.tsx index 7487c1a..937cb1f 100644 --- a/app/src/components/settings/ClaudeAuthModal.test.tsx +++ b/app/src/components/settings/ClaudeAuthModal.test.tsx @@ -31,6 +31,22 @@ vi.mock("@tauri-apps/api/event", () => ({ }), })); +/** Every event the hook subscribes to, so the unmount test counts the right + * number of teardowns instead of a magic number that drifts. */ +const EVENT_NAMES = [ + "claude-token-progress", + "claude-token-output", + "claude-token-link", + "claude-token-code-rejected", +]; + +/** The sign-in URL at its real length (346 characters, measured against + * Claude Code 2.1.226) and the 80-column slice of it that is all the visible + * transcript ever contains. */ +const FULL_URL = + "https://claude.com/cai/oauth/authorize?code=true&client_id=9d1c250a-e61b-44d9-88ed-5944d1962f5e&response_type=code&redirect_uri=https%3A%2F%2Fplatform.claude.com%2Foauth%2Fcode%2Fcallback&scope=user%3Ainference&code_challenge=RUX5MlWvwld1dmpvF_aPIJQWMBmffuJt4dOdL13zWAg&code_challenge_method=S256&state=su-x9PgZzvkBd3-um6G1llLNDgxptyO6HERvvCSrTbg"; +const TRUNCATED_URL = FULL_URL.slice(0, 80); + function emitOutput(chunk: string, projectId = "p1") { act(() => { handlers.get("claude-token-output")?.({ @@ -39,6 +55,26 @@ function emitOutput(chunk: string, projectId = "p1") { }); } +function emitLink(url: string, projectId = "p1") { + act(() => { + handlers.get("claude-token-link")?.({ + payload: { project_id: projectId, url }, + }); + }); +} + +function emitCodeRejected(message: string, attemptsRemaining: number) { + act(() => { + handlers.get("claude-token-code-rejected")?.({ + payload: { + project_id: "p1", + message, + attempts_remaining: attemptsRemaining, + }, + }); + }); +} + function renderModal( overrides: { onClose?: () => void; onAuthenticated?: () => void } = {}, ) { @@ -200,6 +236,93 @@ describe("ClaudeAuthModal", () => { const { unmount } = renderModal(); await flowStarted(); unmount(); - await waitFor(() => expect(unlisten).toHaveBeenCalledTimes(2)); + await waitFor(() => + expect(unlisten).toHaveBeenCalledTimes(EVENT_NAMES.length), + ); + }); + + // ── The hyperlink target, not the wrapped display text ──────────────── + // + // `claude setup-token` slices the *visible* text of its OSC 8 hyperlink to + // the terminal width, so the transcript holds five 80-character pieces of a + // 346-character URL. The backend lifts the whole thing out of the hyperlink + // parameter and sends it on `claude-token-link`. + + it("prefers the hyperlink target over the wrapped copy in the transcript", async () => { + renderModal(); + await flowStarted(); + + // What the transcript holds: the first slice only. + emitOutput(`Browser didn't open? Use the url below to sign in\n${TRUNCATED_URL}\n`); + // What the hyperlink parameter holds: all of it. + emitLink(FULL_URL); + + const link = await screen.findByRole("link", { name: FULL_URL }); + fireEvent.click(link); + await waitFor(() => expect(openUrl).toHaveBeenCalledWith(FULL_URL)); + expect(openUrl).not.toHaveBeenCalledWith(TRUNCATED_URL); + }); + + it("refuses a hyperlink target that is not an Anthropic sign-in address", async () => { + renderModal(); + await flowStarted(); + + emitLink("https://evil.tld/cai/oauth/authorize?code=true"); + + expect(screen.queryByRole("link")).not.toBeInTheDocument(); + expect(openUrl).not.toHaveBeenCalled(); + }); + + it("ignores a hyperlink belonging to a different project", async () => { + renderModal(); + await flowStarted(); + + emitLink(FULL_URL, "p2"); + expect(screen.queryByRole("link")).not.toBeInTheDocument(); + }); + + // ── A refused code is recoverable, not a hang ───────────────────────── + + it("reports a rejected code and lets another one be submitted", async () => { + renderModal(); + await flowStarted(); + + const input = screen.getByLabelText("Authentication code"); + fireEvent.change(input, { target: { value: "truncated" } }); + fireEvent.click(screen.getByRole("button", { name: "Submit code" })); + await waitFor(() => + expect(submitClaudeTokenCode).toHaveBeenCalledWith("truncated"), + ); + // Before the rejection arrives the UI claims the sign-in is completing. + expect(screen.getByText("Finishing sign-in")).toBeInTheDocument(); + + emitCodeRejected( + "That code was rejected — `claude setup-token` reports the full code was not copied. Copy it again from the Anthropic page and submit it; 2 attempts left.", + 2, + ); + + // Reported, not waited out — and the flow is still live. + await screen.findByText(/That code was rejected/); + expect(screen.getByText("Code rejected — try again")).toBeInTheDocument(); + expect(screen.queryByText("Finishing sign-in")).not.toBeInTheDocument(); + expect(screen.queryByTestId("claude-auth-error")).not.toBeInTheDocument(); + + // A second code goes through without restarting the whole flow. + fireEvent.change(input, { target: { value: "the-whole-code" } }); + fireEvent.click(screen.getByRole("button", { name: "Submit code" })); + await waitFor(() => + expect(submitClaudeTokenCode).toHaveBeenLastCalledWith("the-whole-code"), + ); + expect(acquireClaudeToken).toHaveBeenCalledTimes(1); + }); + + it("ends with a reported failure when the retries run out", async () => { + acquireClaudeToken.mockRejectedValue( + "`claude setup-token` rejected the code 3 times, so the sign-in was abandoned. No token was stored.", + ); + renderModal(); + + const banner = await screen.findByTestId("claude-auth-error"); + expect(banner).toHaveTextContent(/rejected the code 3 times/); }); }); diff --git a/app/src/components/settings/ClaudeAuthModal.tsx b/app/src/components/settings/ClaudeAuthModal.tsx index 2845eb6..c58296b 100644 --- a/app/src/components/settings/ClaudeAuthModal.tsx +++ b/app/src/components/settings/ClaudeAuthModal.tsx @@ -27,6 +27,9 @@ interface Props { const PHASE_STATUS: Record = { waiting: { tone: "busy", label: "Waiting for sign-in" }, finishing: { tone: "busy", label: "Finishing sign-in" }, + // The CLI refused a code and is back at its prompt. Distinct from "failed": + // the flow is still live and another code will be accepted. + rejected: { tone: "error", label: "Code rejected — try again" }, succeeded: { tone: "ok", label: "Token stored" }, failed: { tone: "error", label: "Authentication failed" }, }; @@ -88,7 +91,9 @@ export default function ClaudeAuthModal({ ? PHASE_STATUS.failed : flow.codeSubmitted ? PHASE_STATUS.finishing - : PHASE_STATUS.waiting; + : flow.codeRejections > 0 + ? PHASE_STATUS.rejected + : PHASE_STATUS.waiting; // Split for display only. `flow.signInUrl` has already passed the host // allowlist; this decides which half of it an ellipsis is allowed to eat. diff --git a/app/src/hooks/useClaudeAuth.test.ts b/app/src/hooks/useClaudeAuth.test.ts index d9e9894..5347fd2 100644 --- a/app/src/hooks/useClaudeAuth.test.ts +++ b/app/src/hooks/useClaudeAuth.test.ts @@ -1,5 +1,9 @@ import { describe, it, expect } from "vitest"; -import { authErrorMessage, extractSignInUrl } from "./useClaudeAuth"; +import { + authErrorMessage, + extractSignInUrl, + pickSignInUrl, +} from "./useClaudeAuth"; describe("extractSignInUrl", () => { it("finds the authorize URL in realistic setup-token output", () => { @@ -79,6 +83,64 @@ describe("extractSignInUrl", () => { const second = "https://platform.claude.com/oauth/authorize?code=true&more=1"; expect(extractSignInUrl(`${first}\n${second}\n`)).toBe(first); }); + + // ── Why the scraper is only the fallback ───────────────────────────────── + // `claude setup-token` emits the URL as an OSC 8 hyperlink and slices the + // *visible* text of it to the terminal width, so the transcript holds five + // 80-character pieces of a 346-character URL. Each piece is a valid, + // Anthropic-hosted, oauth-looking URL — and none of them authorises + // anything. + + it("cannot recover a URL the CLI sliced across lines, which is why the hyperlink wins", () => { + const slices = [ + FULL_URL.slice(0, 80), + FULL_URL.slice(80, 160), + FULL_URL.slice(160, 240), + FULL_URL.slice(240, 320), + FULL_URL.slice(320), + ]; + const scraped = extractSignInUrl(slices.join("\n")); + + // Documenting the limit, not endorsing it: the pieces share no prefix, so + // the "extends the current pick" rule cannot join them, and guessing at + // line joins on an untrusted stream is not on the table. + expect(scraped).toBe(slices[0]); + expect(scraped).not.toBe(FULL_URL); + + // The hyperlink parameter carries the whole thing, and that is what the + // hook prefers. + expect(pickSignInUrl([FULL_URL])).toBe(FULL_URL); + }); +}); + +/** The real sign-in URL, at its measured length (346 characters, Claude Code + * 2.1.226). */ +const FULL_URL = + "https://claude.com/cai/oauth/authorize?code=true&client_id=9d1c250a-e61b-44d9-88ed-5944d1962f5e&response_type=code&redirect_uri=https%3A%2F%2Fplatform.claude.com%2Foauth%2Fcode%2Fcallback&scope=user%3Ainference&code_challenge=RUX5MlWvwld1dmpvF_aPIJQWMBmffuJt4dOdL13zWAg&code_challenge_method=S256&state=su-x9PgZzvkBd3-um6G1llLNDgxptyO6HERvvCSrTbg"; + +describe("pickSignInUrl", () => { + it("keeps a 346-character authorize URL intact", () => { + expect(FULL_URL).toHaveLength(346); + expect(pickSignInUrl([FULL_URL])).toBe(FULL_URL); + }); + + it("applies the same host allowlist to a hyperlink target", () => { + // An OSC 8 parameter is container output like anything else, and it is + // never displayed — so it is the *easier* place to hide a hostile host. + expect(pickSignInUrl(["https://evil.tld/cai/oauth/authorize"])).toBeNull(); + expect( + pickSignInUrl(["https://claude.ai@evil.tld/oauth/authorize"]), + ).toBeNull(); + expect(pickSignInUrl(["javascript:alert(1)"])).toBeNull(); + expect(pickSignInUrl([])).toBeNull(); + }); + + it("does not let a later hyperlink displace the one already shown", () => { + const real = `${FULL_URL}`; + const spoof = "https://claude.com.evil.tld/cai/oauth/authorize?code=true"; + expect(pickSignInUrl([real, spoof])).toBe(real); + expect(pickSignInUrl([spoof, real])).toBe(real); + }); }); describe("authErrorMessage", () => { diff --git a/app/src/hooks/useClaudeAuth.ts b/app/src/hooks/useClaudeAuth.ts index c42d70f..8fcb7a2 100644 --- a/app/src/hooks/useClaudeAuth.ts +++ b/app/src/hooks/useClaudeAuth.ts @@ -3,6 +3,8 @@ import { listen, type UnlistenFn } from "@tauri-apps/api/event"; import * as commands from "../lib/tauri-commands"; import { ANTHROPIC_SIGN_IN_HOSTS, sanitizeRelayUrl } from "../lib/urlRelay"; import type { + ClaudeTokenCodeRejectedEvent, + ClaudeTokenLinkEvent, ClaudeTokenOutputEvent, ClaudeTokenProgressEvent, } from "../lib/types"; @@ -19,10 +21,17 @@ import type { /** Emitted by `auth_token_commands.rs`; payload shapes live in `lib/types.ts`. */ const PROGRESS_EVENT = "claude-token-progress"; const OUTPUT_EVENT = "claude-token-output"; +const LINK_EVENT = "claude-token-link"; +const CODE_REJECTED_EVENT = "claude-token-code-rejected"; /** Bound on the retained transcript. The tail is the interesting part. */ const MAX_OUTPUT = 64 * 1024; +/** Bound on retained sign-in candidates. The backend already deduplicates + * consecutive repeats; this stops a container that prints a fresh hyperlink + * every frame from growing state without limit. */ +const MAX_LINKS = 16; + /** * Tauri rejects an `invoke` with the Rust `Err(String)` itself, and this * backend writes its errors as complete, actionable sentences ("The container @@ -38,13 +47,13 @@ export function authErrorMessage(e: unknown, fallback: string): string { } /** - * Pick the sign-in URL out of `claude setup-token`'s transcript. + * Choose one sign-in URL from a list of candidates. * - * **The transcript is container output, so every candidate here is - * attacker-controlled if the sandboxed agent misbehaves.** It is then rendered - * under a heading that says "Sign in with Anthropic" and handed to the host - * browser, which makes this the highest-value URL in the app to spoof: a user - * who follows it types their real Anthropic credentials into whatever it + * **Every candidate is container output, so all of them are + * attacker-controlled if the sandboxed agent misbehaves.** The winner is + * rendered under a heading that says "Sign in with Anthropic" and handed to the + * host browser, which makes this the highest-value URL in the app to spoof: a + * user who follows it types their real Anthropic credentials into whatever it * resolves to. Three rules follow, and none of them are optional: * * - Every candidate goes through the shared {@link sanitizeRelayUrl}, with a @@ -60,14 +69,8 @@ export function authErrorMessage(e: unknown, fallback: string): string { * the complete one — and it cannot swap the origin, because a longer string * with the same prefix has the same host. */ -export function extractSignInUrl(text: string): string | null { - // eslint-disable-next-line no-control-regex - const matches = text.match(/https?:\/\/[^\s"'`<>\x00-\x20\x7f]+/g); - if (!matches) return null; - - const cleaned = matches - // Trailing punctuation belongs to the prose, not the URL. - .map((url) => url.replace(/[.,;:!?)\]}>'"]+$/, "")) +export function pickSignInUrl(candidates: readonly string[]): string | null { + const cleaned = candidates .map((url) => sanitizeRelayUrl(url, { allowHosts: ANTHROPIC_SIGN_IN_HOSTS })) .filter((url): url is string => url !== null); @@ -81,6 +84,33 @@ export function extractSignInUrl(text: string): string | null { return best; } +/** + * Scrape a sign-in URL out of `claude setup-token`'s visible transcript. + * + * **This is the fallback, not the primary route.** The CLI emits the URL as an + * OSC 8 hyperlink and slices the *visible* text of that hyperlink to the + * terminal width — measured at 80 columns, a 346-character URL arrives as five + * 80-character pieces on five lines. Nothing scraping the visible text can put + * those back together: the pieces share no prefix, so the "extends the current + * pick" rule cannot join them, and joining adjacent lines by guesswork on an + * untrusted stream is exactly the sort of thing the rules above exist to + * forbid. What comes out is the first 80 characters — a URL that parses, that + * points at claude.com, and that cannot authorise anything. + * + * So the backend lifts the whole URL out of the hyperlink parameter and sends + * it on `claude-token-link`, and {@link useClaudeTokenAcquisition} prefers that. + * This remains for CLI versions that print a bare URL with no hyperlink at all, + * where a URL narrow enough not to wrap is recovered correctly. + */ +export function extractSignInUrl(text: string): string | null { + // eslint-disable-next-line no-control-regex + const matches = text.match(/https?:\/\/[^\s"'`<>\x00-\x20\x7f]+/g); + if (!matches) return null; + + // Trailing punctuation belongs to the prose, not the URL. + return pickSignInUrl(matches.map((url) => url.replace(/[.,;:!?)\]}>'"]+$/, ""))); +} + // ───────────────────────────────────────────────────────────────────────────── // Token presence // ───────────────────────────────────────────────────────────────────────────── @@ -133,6 +163,12 @@ export interface ClaudeTokenAcquisition { submitting: boolean; codeSubmitted: boolean; submitError: string | null; + /** + * How many codes `claude setup-token` has refused. Non-zero means the CLI is + * still alive and waiting for another one — a recoverable state, not the end + * of the flow. + */ + codeRejections: number; submitCode: (code: string) => Promise; } @@ -154,6 +190,13 @@ export function useClaudeTokenAcquisition( const [submitting, setSubmitting] = useState(false); const [codeSubmitted, setCodeSubmitted] = useState(false); const [submitError, setSubmitError] = useState(null); + const [codeRejections, setCodeRejections] = useState(0); + // Candidates from `claude-token-link`, in arrival order. Kept as a list + // rather than a single value so `pickSignInUrl` applies the same first-wins + // rule here as it does to the scraped transcript — the CLI reprints the same + // hyperlink after every retry, and a *different* one arriving later must not + // be able to displace the one the user was already shown. + const [links, setLinks] = useState([]); // Held in a ref so a fresh callback identity cannot restart the flow. const succeededRef = useRef(onSucceeded); @@ -193,6 +236,26 @@ export function useClaudeTokenAcquisition( : next; }); }); + await register(LINK_EVENT, (payload) => { + if (payload.project_id !== projectId) return; + setLinks((prev) => + prev.includes(payload.url) || prev.length >= MAX_LINKS + ? prev + : [...prev, payload.url], + ); + }); + await register( + CODE_REJECTED_EVENT, + (payload) => { + if (payload.project_id !== projectId) return; + // The CLI is alive and back at its prompt, so this is a correction + // the user can act on — not a failure. Re-open the input and say + // why, rather than leaving "Finishing sign-in" on screen forever. + setCodeRejections((n) => n + 1); + setCodeSubmitted(false); + setSubmitError(payload.message); + }, + ); } catch (e) { if (cancelled) return; setPhase("failed"); @@ -261,7 +324,13 @@ export function useClaudeTokenAcquisition( } }, []); - const signInUrl = useMemo(() => extractSignInUrl(output), [output]); + // The hyperlink parameter wins whenever there is one: it is the only place + // the CLI emits the URL contiguously. Scraping the visible text is the + // fallback for versions that print a bare URL — see `extractSignInUrl`. + const signInUrl = useMemo( + () => pickSignInUrl(links) ?? extractSignInUrl(output), + [links, output], + ); return { phase, @@ -272,6 +341,7 @@ export function useClaudeTokenAcquisition( submitting, codeSubmitted, submitError, + codeRejections, submitCode, }; } diff --git a/app/src/lib/tauri-commands.ts b/app/src/lib/tauri-commands.ts index f90ac0a..d02013a 100644 --- a/app/src/lib/tauri-commands.ts +++ b/app/src/lib/tauri-commands.ts @@ -1,5 +1,5 @@ import { invoke } from "@tauri-apps/api/core"; -import type { Project, ProjectPath, ContainerInfo, SiblingContainer, AppSettings, UpdateInfo, ImageUpdateInfo, FileEntry, WebTerminalInfo, SttStatus, GatewayStatus, InstallOptions, ClaudeSession, ContainerCapabilities, ScheduledTask, ScheduledTaskInput, SchedulerNotification, AuthBridgeStatus, BrowserViewStatus, PlaywrightDetection, ContainerStaleness, MigrationOptions, MigrationReport, MigrationState, ClearTokenOutcome, CaCertInfo } from "./types"; +import type { Project, ProjectPath, ContainerInfo, SiblingContainer, AppSettings, UpdateInfo, ImageUpdateInfo, FileEntry, WebTerminalInfo, SttStatus, GatewayStatus, InstallOptions, ClaudeSession, ContainerCapabilities, ScheduledTask, ScheduledTaskInput, SchedulerNotification, AuthBridgeStatus, BrowserViewStatus, PlaywrightDetection, BrowserSetupOutcome, BrowserInstallTarget, ContainerStaleness, MigrationOptions, MigrationReport, MigrationState, ClearTokenOutcome, CaCertInfo } from "./types"; // Docker export const checkDocker = () => invoke("check_docker"); @@ -182,6 +182,24 @@ export const getBrowserViewStatus = (projectId: string) => /** Probe for Playwright without starting anything — used to re-check after installing it. */ export const checkBrowserViewSupport = (projectId: string) => invoke("check_browser_view_support", { projectId }); +/** + * Install `playwright` + `@playwright/cli` into the container's `/workspace`. + * + * A container mutation, so it only ever runs from an explicit click. Progress + * streams on the existing `container-progress` event; the result carries a + * fresh probe. Browsers are a separate action — see below. + */ +export const installBrowserViewSupport = (projectId: string) => + invoke("install_browser_view_support", { projectId }); +/** + * Install a browser and the apt libraries it needs, then verify it launches. + * `chromium` is Playwright's own build; `chrome` is the channel + * `@playwright/mcp` asks for. Hundreds of MB — never call this implicitly. + */ +export const installBrowserViewBrowser = ( + projectId: string, + browser: BrowserInstallTarget, +) => invoke("install_browser_view_browser", { projectId, browser }); // Shared Claude Code auth token — one `claude setup-token` run authenticates // every Anthropic-backend project. The token itself is never exposed here: it diff --git a/app/src/lib/types.ts b/app/src/lib/types.ts index 7aff4a9..524939c 100644 --- a/app/src/lib/types.ts +++ b/app/src/lib/types.ts @@ -458,14 +458,40 @@ export interface PlaywrightDetection { node_version: string | null; playwright_version: string | null; playwright_path: string | null; + /** Playwright's own `cli.js`, which installs browsers and their apt libraries. */ + playwright_cli: string | null; /** Whether the resolved Playwright declares the `browser.bind()` live-dashboard API. */ has_bind: boolean; cli_version: string | null; cli_entry: string | null; - /** Module roots the probe searched, echoed back for the "not found" message. */ + /** Browser bundles in `~/.cache/ms-playwright`, e.g. `chromium-1200`. Never `ffmpeg-*`. */ + browsers: string[]; + /** Path to Google Chrome when the `chrome` channel — what `@playwright/mcp` + * asks for — is installed. It is an apt package, so it is never in `browsers`. */ + chrome_channel: string | null; + /** Module roots the probe searched, echoed back for the "not found" message. + * Includes the npx cache (`~/.npm/_npx/*​/node_modules`), which is where a + * Playwright installed through Claude Code's MCP setup actually lives. */ searched: string[]; } +/** Result of an install action. Mirrors Rust `BrowserSetupOutcome`. */ +export interface BrowserSetupOutcome { + /** Fresh probe taken after the install, so the pane can update itself. */ + detection: PlaywrightDetection; + /** Tail of the real npm/apt/Playwright output — shown instead of a generic message. */ + log: string; + /** Whether a browser was actually started and closed. `null` when the step + * didn't try (the package step doesn't). */ + browser_launched: boolean | null; + /** Something that didn't fail the action but the user still needs to know. */ + warning: string | null; +} + +/** Browsers the pane can install. `chromium` is Playwright's own build; + * `chrome` is the Google Chrome channel `@playwright/mcp` asks for. */ +export type BrowserInstallTarget = "chromium" | "chrome"; + /** Mirrors Rust `BrowserViewState` (serde snake_case). */ export type BrowserViewState = "off" | "running" | "unavailable"; @@ -526,6 +552,26 @@ export interface ClaudeTokenOutputEvent { chunk: string; } +/** Payload of the `claude-token-link`: a sign-in URL taken from an OSC 8 + * hyperlink parameter, which is the only place the CLI emits it whole — the + * visible text is sliced to the terminal width. **Untrusted**: it is container + * output, so it goes through `sanitizeRelayUrl` with the + * `ANTHROPIC_SIGN_IN_HOSTS` allowlist before it is shown or opened. */ +export interface ClaudeTokenLinkEvent { + project_id: string; + url: string; +} + +/** Payload of `claude-token-code-rejected`: `claude setup-token` refused the + * submitted code and is parked waiting for another one. The flow is still + * alive, so this is recoverable — `attempts_remaining` is how many more codes + * the backend will pass on before giving up. */ +export interface ClaudeTokenCodeRejectedEvent { + project_id: string; + message: string; + attempts_remaining: number; +} + // ── Container base-image migration ─────────────────────────────────────────── // // A project's container is created from its own `triple-c-snapshot-:latest` diff --git a/container/Dockerfile b/container/Dockerfile index 4dfcdf4..9a52eb7 100644 --- a/container/Dockerfile +++ b/container/Dockerfile @@ -85,6 +85,95 @@ RUN curl -fsSL https://deb.nodesource.com/gpgkey/nodesource-repo.gpg.key \ && rm -rf /var/lib/apt/lists/* \ && npm install -g pnpm +# ── Browser runtime libraries (Chromium / Google Chrome) ──────────────────── +# Chromium links against a set of shared libraries Ubuntu's base image does not +# ship — libnss3, libgbm1, libatk*, libasound2t64, libcups2t64, libpango, +# libdrm2 and friends. Without them `playwright install chromium` downloads a +# browser that then dies at launch with "Host system is missing dependencies: +# libnss3.so", which reads like a Playwright bug and is not one. Installing +# google-chrome-stable used to look like the fix only because apt pulled these +# in as *its* dependencies. +# +# ## Why baked, and why only the libraries +# +# A runtime `apt-get install` lands in the container's writable layer: it is +# re-paid after every project Reset, and it is *lost* on base-image migration, +# which replays apt from a manifest against the new base. The browsers +# themselves live in ~/.cache/ms-playwright, inside the home volume, and survive +# both — so the runtime approach converges on the worst state, a 400 MB browser +# present with its libraries gone. Baking the libraries and leaving the browsers +# out puts each half where it already persists. +# +# Browser binaries are deliberately NOT baked: they are large, they are +# version-coupled to whatever Playwright the user installs, and the home volume +# already keeps them. +# +# ## Why `install-deps` rather than a hand-written apt list +# +# Playwright names its own dependencies, so the list cannot silently rot. That +# matters more than usual on Ubuntu 24.04, whose 64-bit-time_t transition +# renamed a swathe of these packages (libasound2 → libasound2t64, libatk1.0-0 → +# libatk1.0-0t64, libglib2.0-0 → libglib2.0-0t64, …); a hardcoded list drifts +# into "E: Unable to locate package" build failures, and a list that predates a +# new Chromium dependency drifts into exactly the launch failure this layer +# exists to prevent. +# +# Verified on a real `--platform linux/arm64` build of this file, not assumed: +# it resolves and installs there too (99 packages on both arches), and the +# --dry-run assertion below passes. Worth checking rather than assuming: +# Playwright looks its dependency list up under `-`, so +# arm64 is a separate lookup that could have missed. +# +# ## What it costs +# +# Measured with this layer applied on top of an otherwise identical image +# (linux/amd64, playwright 1.62.1): **+99 packages, +334 MiB unpacked, +119 MiB +# compressed** — the image goes 2950 → 3284 MiB unpacked, 759 → 878 MiB +# compressed. (`docker history` calls the layer 361 MB, i.e. 344 MiB; the +# difference is tar metadata `du` doesn't count.) +# +# Where it goes, by dpkg Installed-Size: +# ~213 MiB libllvm20 + mesa-libgallium + libicu74. Not optional and not +# avoidable by trimming the list: libgbm1, which Chromium genuinely +# needs, Depends on mesa-libgallium, which Depends on libllvm20. +# ~94 MiB Playwright's `tools` group — xvfb and the CJK/emoji fonts. Kept: +# the base image ships no fonts at all, so without them every page +# this feature exists to display renders as tofu, and xvfb is what +# lets a *headed* browser run in here. +# the rest Chromium's own library closure. +# +# An explicit apt list of just `chromium`'s dependencies measures 247 MiB +# installed against install-deps' 341 MiB, so hand-maintaining one would save +# ~94 MiB. Not worth owning the drift; if you disagree, derive the list from +# `install-deps --dry-run chromium` and pin the Playwright version you took it +# from in a comment here. +# +# The retry loop is for the same transient mirror-sync failures the other apt +# layers guard against; install-deps runs its own un-retried `apt-get update` +# internally. `npx --yes` is what makes it non-interactive, and the version it +# resolved is printed so a build log says which Playwright named this set. +# +# Placed immediately after Node (npx is its only prerequisite) and well above +# the shim COPYs, so editing a shim at the bottom of this file does not re-run a +# multi-hundred-megabyte apt install. +# +# `--dry-run` afterwards is the build-time assertion, and it is not decoration: +# on a platform Playwright's table does not cover, `install-deps` prints a +# warning and returns having installed **nothing, with exit status 0**. Without +# this check that failure mode would ship an image whose build log looked clean. +# `--dry-run` exits non-zero if any required package is still missing. +RUN npx --yes playwright@latest --version \ + && ok=0 \ + && for i in 1 2 3 4 5; do \ + if npx --yes playwright@latest install-deps chromium; then ok=1; break; fi; \ + echo "install-deps failed (attempt $i), retrying in 10s..."; \ + rm -rf /var/lib/apt/lists/*; \ + sleep 10; \ + done \ + && [ "$ok" = 1 ] \ + && npx --yes playwright@latest install-deps --dry-run chromium \ + && rm -rf /var/lib/apt/lists/* /root/.npm + # ── Python 3 + pip + uv + ruff ────────────────────────────────────────────── RUN for i in 1 2 3 4 5; do \ apt-get -o Acquire::Retries=3 update && break; \