diff --git a/apps/review-desktop/scripts/e2e/journeys/remote-ask.mjs b/apps/review-desktop/scripts/e2e/journeys/remote-ask.mjs new file mode 100644 index 000000000..5e389d20d --- /dev/null +++ b/apps/review-desktop/scripts/e2e/journeys/remote-ask.mjs @@ -0,0 +1,406 @@ +/** Ask on a review whose checkout is on another machine: a container with a fake OpenCode, reached over SSH and added in Settings as a user would. */ +import assert from "node:assert/strict"; +import { execFile } from "node:child_process"; +import { promisify } from "node:util"; + +import { openHome, openSettings } from "../harness.mjs"; +import { + alias, + closeDesktop, + createRemoteReview, + masterPid, + onRemote, + prepared, + recordRequests, + remote, + remoteToken, + runDir, +} from "./remote-host.mjs"; +import { removeHost } from "./remote-install.mjs"; + +const exec = promisify(execFile); + +const title = "Remote ask"; + +const question = "what does f.ts do"; + +export const name = "remote-ask"; + +// Phase 2: the container image downloads Ubuntu packages and Node. +export const phase = 2; + +export const options = { + settings: { "review.experimental.remoteHosts.enabled": true }, + env: { DEV_FAST_REVIEW_SSH_CONFIG: `${runDir}/ssh_config` }, +}; + +export async function run(ctx) { + // A packaged build ignores DEV_FAST_REVIEW_SSH_CONFIG, so its ssh would read the user's configuration. + if (ctx.report.mode === "packaged") + throw new Error("skip: remote-ask runs in development mode only"); + + if (prepared === undefined) + try { + await exec("docker", ["info", "--format", "{{.ServerVersion}}"]); + } catch (error) { + throw new Error( + `skip: remote-ask needs Docker for its SSH server (${error.message.split("\n")[0]})`, + ); + } + + try { + await journey(ctx); + } finally { + if (prepared === undefined) + await remote("down", "--all").catch((error) => + console.error(`[remote-ask] down --all: ${error.message}`), + ); + else await closeDesktop(ctx); + } +} + +/** Pauses the container, or stops a prepared host's server and returns its resume. */ +async function freeze() { + if (prepared === undefined) { + await remote("pause", "a"); + + return undefined; + } + + const { serverPid } = JSON.parse( + await onRemote("whiteboard server status --json"), + ); + + await onRemote(`kill -STOP ${serverPid}`); + + return () => onRemote(`kill -CONT ${serverPid}`); +} + +async function journey(ctx) { + const { until } = ctx; + const timings = {}; + + const hostState = async () => + (await ctx.apiOk("/remote-hosts")).find((host) => host.alias === alias) + ?.state; + + const waitState = (state, label, timeout = 60000) => + until( + async () => (await hostState()) === state, + `${alias} ${label}`, + timeout, + ); + + const canvas = () => + ctx.page.locator(".review-canvas-root [data-review-api]"); + + // Docked in the side panel or floating, by the layout. + const panel = () => + ctx.page.locator( + '[role=complementary][aria-label="Ask"], [role=dialog][aria-label="Ask"]', + ); + + const composer = () => panel().getByRole("combobox", { name: "Question" }); + + const homeRow = () => + ctx.page + .locator("main.review-home [data-home-row]") + .filter({ hasText: title }); + + async function openReview() { + await openHome(ctx); + await homeRow().waitFor({ timeout: 60000 }); + await homeRow().getByTitle(title, { exact: true }).click(); + await canvas().getByRole("heading", { name: title }).waitFor({ + timeout: 60000, + }); + } + + /** Selects the document's sentence and opens Ask on it with ⌘L. */ + async function askAboutSentence() { + // The workbench's usage-data notice sits over the composer's Ask button. + for (const clear of await ctx.page + .locator(".notifications-toasts .codicon-notifications-clear") + .all()) + await clear.click().catch(() => {}); + + const sentence = canvas().getByText("One now returns"); + + await sentence.waitFor(); + await sentence.evaluate((element) => { + const range = document.createRange(); + + range.selectNodeContents(element); + document.getSelection().removeAllRanges(); + document.getSelection().addRange(range); + }); + await ctx.page + .getByRole("button", { name: "Ask OpenCode", exact: true }) + .waitFor({ timeout: 30000 }); + await ctx.page.keyboard.press("Meta+l"); + await composer().waitFor({ timeout: 30000 }); + } + + /** Resolves with the ms until the panel shows the answer. */ + async function send(text) { + await composer().fill(text); + + const sent = Date.now(); + + await panel().getByRole("button", { name: "Ask", exact: true }).click(); + await panel().getByText(`You asked: ${text}`).waitFor({ timeout: 20000 }); + + return Date.now() - sent; + } + + const savedConversations = () => + canvas().getByRole("button", { name: "Saved conversations" }); + + async function historyLists(label) { + await until( + async () => { + await savedConversations().first().click(); + + return ( + (await panel() + .getByRole("list") + .filter({ hasText: question }) + .count()) > 0 + ); + }, + label, + 30000, + ); + } + + // 1. A container with sshd, this checkout's package and the fake OpenCode, or a prepared host, holding a review. + if (prepared === undefined) { + await remote("up", "a", "--fake-agent"); + await remote("install", "a"); + } + + const [reviewId] = ( + await onRemote(`bash -s -- '${title}'`, createRemoteReview) + ).split("\n"); + + assert.match(reviewId, /^[0-9a-f-]{36}$/, "the remote review's id"); + + const remoteTokens = new Set([await remoteToken()]); + + ctx.check(`1. ${alias}, with a fake OpenCode, holds a review`); + + // 2. Settings, as a user adds a host. + const section = (await openSettings(ctx)).getByRole("region", { + name: "Remote hosts", + }); + + await section.getByLabel("SSH alias").fill(alias); + + const added = Date.now(); + + await section.getByRole("button", { name: "Add", exact: true }).click(); + await waitState("online", "online"); + timings.addToOnline = Date.now() - added; + ctx.check( + `2. Settings added ${alias}; online ${timings.addToOnline} ms after Add`, + ); + + // 3. The remote review, a sentence selected, ⌘L. + const requests = await recordRequests(ctx.page, /\/ask\/watch(\?|$)/); + const recorders = [{ requests, origin: new URL(ctx.discovery.url).origin }]; + + await openReview(); + await askAboutSentence(); + ctx.check("3. ⌘L on a selected sentence of the remote review opened Ask"); + + // 4. The agents are the remote's: with OpenCode gone, the panel names the host; back, it answers again. + await onRemote("mv ~/.opencode ~/.opencode.off"); + await until( + async () => { + await ctx.page.evaluate(() => window.dispatchEvent(new Event("focus"))); + + return ( + (await panel().getByText(`Not installed on ${alias}`).count()) >= 3 + ); + }, + `the panel to read Not installed on ${alias}`, + 30000, + ); + + for (const agent of ["Claude Code", "Codex", "OpenCode"]) + assert.match( + await panel() + .getByRole("listitem") + .filter({ hasText: agent }) + .innerText(), + new RegExp(`Not installed on ${alias}`), + ); + + await onRemote("mv ~/.opencode.off ~/.opencode"); + await until( + async () => { + await ctx.page.evaluate(() => window.dispatchEvent(new Event("focus"))); + + return (await composer().count()) > 0; + }, + "the composer to come back with OpenCode", + 30000, + ); + await panel().getByText("OpenCode", { exact: true }).waitFor(); + + for (const name of ["OpenCode", "Choose an agent"]) + assert.equal( + await panel().getByRole("button", { name, exact: true }).count(), + 0, + ); + ctx.check( + `4. without the remote's OpenCode, Claude Code, Codex and OpenCode read Not installed on ${alias}; with it back, the header names OpenCode alone, without a menu`, + ); + + // 5. The question goes to the remote's agent; its answer streams back through the gateway. + timings.firstAnswer = await send(question); + ctx.check( + `5. OpenCode on ${alias} answered "You asked: …" ${timings.firstAnswer} ms after Ask`, + ); + + // 6. A review on another machine has no Source window: the answer's f.ts is text that says so, and a click asks nothing. + const reference = panel().getByTitle( + "Source windows are not available for a review on another machine.", + ); + + await reference.waitFor(); + assert.equal(await reference.innerText(), "f.ts"); + assert.equal( + await reference.evaluate( + (element) => + element.closest("a, button, [role=link], [role=button]") !== null, + ), + false, + "the f.ts reference is a link or a button", + ); + assert.equal(await panel().getByRole("link", { name: "f.ts" }).count(), 0); + + // Step 11 audits the whole run for the requests a click could send. + await reference.click(); + ctx.check( + "6. the answer's f.ts is text titled Source windows are not available…, not a link or button", + ); + + // 7. Ask spoke only to the local server, and no remote token reached the page. + const asks = audit(recorders, remoteTokens); + + assert.ok( + asks.some((r) => r.received?.toString().includes("You asked:")), + "no watch stream carried the answer", + ); + ctx.check( + `7. ${asks.length} Ask requests, all to the local server, none with the remote token; the answer came on its watch stream`, + ); + + // 8. The ssh master dies and comes back: the thread is still saved. + const master = await masterPid(); + + assert.ok(master, "no ssh master for the host"); + process.kill(master, "SIGKILL"); + await until( + async () => + (await hostState()) === "online" && (await masterPid()) !== master, + `${alias} online on a new master`, + 60000, + ); + await panel().getByRole("button", { name: "Close Ask" }).click(); + await historyLists("the thread in Ask history after the reattach"); + ctx.check("8. after a new ssh master, Ask history lists the thread"); + + // 9. A Desktop restart: the thread is still listed. + await ctx.quitAndRelaunchDesktop(); + recorders.push({ + requests: await recordRequests(ctx.page, /^$/), + origin: new URL(ctx.discovery.url).origin, + }); + await waitState("online", "online after the relaunch"); + remoteTokens.add(await remoteToken()); + await openReview(); + await historyLists("the thread in Ask history after the relaunch"); + await panel().getByRole("button", { name: "Close Ask" }).click(); + ctx.check("9. after a Desktop restart, Ask history lists the thread"); + + // 10. The host goes away with a conversation open: the panel says so, and the host reads offline. + await askAboutSentence(); + await send("and then"); + const thaw = await freeze(); + + const paused = Date.now(); + + await until( + async () => + (await panel() + .getByRole("alert") + .filter({ hasText: "lost its connection" }) + .count()) > 0, + "the panel's lost-connection alert", + 20000, + ); + timings.streamEnded = Date.now() - paused; + await waitState("offline", "offline after the pause", 20000); + timings.offline = Date.now() - paused; + assert.ok( + timings.streamEnded <= 15000, + `alert after ${timings.streamEnded} ms`, + ); + assert.ok(timings.offline <= 15000, `offline after ${timings.offline} ms`); + await thaw?.(); + ctx.check( + `10. ${prepared === undefined ? "docker pause" : "server SIGSTOP"}: the panel said it lost OpenCode after ${timings.streamEnded} ms; ${alias} offline after ${timings.offline} ms`, + ); + + // 11. Removing the host takes the review out of Home. + await removeHost(ctx, alias); + await openHome(ctx); + await until( + async () => (await homeRow().count()) === 0, + "the remote review to leave Home", + ); + const allAsks = audit(recorders, remoteTokens); + + assert.deepEqual( + recorders.flatMap(({ requests: recorded }) => + [...recorded.values()].flatMap((r) => + /\/(navigator|files)(\?|$)/.test(r.url ?? "") ? [r.url] : [], + ), + ), + [], + "navigator or files requests", + ); + ctx.check( + `11. removing the host took the review out of Home; across both launches ${allAsks.length} Ask requests, all to the local server, none with a remote token, and no navigator or files request`, + ); +} + +/** Asserts each launch's Ask requests stayed local and none carried a remote token. */ +function audit(recorders, remoteTokens) { + const asks = []; + + for (const { requests, origin } of recorders) + for (const r of requests.values()) { + if (!r.url) continue; + assert.ok( + ![r.url, ...Object.values(r.headers ?? {})].some((value) => + [...remoteTokens].some((token) => String(value).includes(token)), + ), + "a request carried a remote token", + ); + + if (!/\/ask(\/|\?|$)/.test(new URL(r.url).pathname)) continue; + assert.equal( + new URL(r.url).origin, + origin, + "an Ask request past the local server", + ); + asks.push(r); + } + + assert.ok(asks.length > 0, "no Ask request was recorded"); + + return asks; +} diff --git a/apps/review-desktop/scripts/e2e/journeys/remote-host.mjs b/apps/review-desktop/scripts/e2e/journeys/remote-host.mjs index dae1f47ce..21077540a 100644 --- a/apps/review-desktop/scripts/e2e/journeys/remote-host.mjs +++ b/apps/review-desktop/scripts/e2e/journeys/remote-host.mjs @@ -12,11 +12,11 @@ const remoteScript = path.join(import.meta.dirname, "../remote/remote.mjs"); const runId = process.env.WB_TEST_RUN ?? `e2e${Date.now().toString(36)}`; -const runDir = `/tmp/wbt.${runId}`; +export const runDir = `/tmp/wbt.${runId}`; -const prepared = process.env.REVIEW_E2E_REMOTE_HOST; +export const prepared = process.env.REVIEW_E2E_REMOTE_HOST; -const alias = `wb-test-${prepared ?? "a"}`; +export const alias = `wb-test-${prepared ?? "a"}`; const title = "Remote order"; @@ -32,7 +32,7 @@ export const options = { env: { DEV_FAST_REVIEW_SSH_CONFIG: `${runDir}/ssh_config` }, }; -const createRemoteReview = String.raw` +export const createRemoteReview = String.raw` set -e field() { node -pe "JSON.parse(require('fs').readFileSync(0, 'utf8')).$1"; } rm -rf ~/wbrepo @@ -61,7 +61,7 @@ export async function listedAs(ctx, title) { return review && `${review.host}: ${review.repositoryGroup?.label}`; } -async function remote(...args) { +export async function remote(...args) { return ( await exec(process.execPath, [remoteScript, ...args], { env: { ...process.env, WB_TEST_RUN: runId }, @@ -70,7 +70,7 @@ async function remote(...args) { ).stdout.trim(); } -function onRemote(command, input = "") { +export function onRemote(command, input = "") { return new Promise((resolve, reject) => { const child = spawn( "ssh", @@ -96,7 +96,7 @@ function onRemote(command, input = "") { const remoteApi = (tool, input) => onRemote(`whiteboard api ${tool} -`, JSON.stringify(input)); -async function remoteToken() { +export async function remoteToken() { const out = (await onRemote("whiteboard remote attach --json")).split("\n"); let token; @@ -115,7 +115,7 @@ async function remoteToken() { return token; } -async function recordRequests(page, streamed) { +export async function recordRequests(page, streamed) { const cdp = await page.context().newCDPSession(page); const requests = new Map(); const entry = (id) => requests.get(id) ?? requests.set(id, { id }).get(id); @@ -191,7 +191,7 @@ export async function run(ctx) { } } -async function closeDesktop(ctx) { +export async function closeDesktop(ctx) { const session = await ctx.browser.newBrowserCDPSession().catch(() => null); await Promise.race([ @@ -672,7 +672,7 @@ async function desktopSshProcesses() { const desktopSsh = async () => (await desktopSshProcesses()).map(([pid]) => pid); -async function masterPid() { +export async function masterPid() { return (await desktopSshProcesses()).find( ([, args]) => args.includes(" -M -N ") && args.endsWith(`-- ${alias}`), )?.[0]; diff --git a/apps/review-desktop/scripts/e2e/journeys/remote-install.mjs b/apps/review-desktop/scripts/e2e/journeys/remote-install.mjs index d9c9000bc..52d339074 100644 --- a/apps/review-desktop/scripts/e2e/journeys/remote-install.mjs +++ b/apps/review-desktop/scripts/e2e/journeys/remote-install.mjs @@ -323,7 +323,7 @@ async function addHost(ctx, host) { return section; } -async function removeHost(ctx, host, { uninstall = false } = {}) { +export async function removeHost(ctx, host, { uninstall = false } = {}) { const section = await hostsSection(ctx); await section.getByRole("button", { name: `Remove ${host}` }).click(); diff --git a/apps/review-desktop/scripts/e2e/remote/aws-hosts.mjs b/apps/review-desktop/scripts/e2e/remote/aws-hosts.mjs index 573a9e6ce..2d546a83c 100644 --- a/apps/review-desktop/scripts/e2e/remote/aws-hosts.mjs +++ b/apps/review-desktop/scripts/e2e/remote/aws-hosts.mjs @@ -1,6 +1,6 @@ /** Hosts on AWS: `aws-up`, and removing a run's instances, key pair and security groups. */ import { addOnce, aws, run, sleep } from "./exec.mjs"; -import { waitForSsh } from "./ssh.mjs"; +import { installFakeAgent, waitForSsh } from "./ssh.mjs"; const maxInstances = 2; @@ -221,6 +221,9 @@ export async function awsUp(runState, name, options) { 60, "test -f /run/systemd/shutdown/scheduled", ); + + if (options["fake-agent"]) await installFakeAgent(runState, host); + console.log(alias); } diff --git a/apps/review-desktop/scripts/e2e/remote/docker-hosts.mjs b/apps/review-desktop/scripts/e2e/remote/docker-hosts.mjs index d4737ddcf..d74934e18 100644 --- a/apps/review-desktop/scripts/e2e/remote/docker-hosts.mjs +++ b/apps/review-desktop/scripts/e2e/remote/docker-hosts.mjs @@ -8,7 +8,7 @@ import { remoteDevInstall } from "../../remote-dev-install.mjs"; import { addOnce, docker, exec, run, sleep } from "./exec.mjs"; import { hostOf } from "./run-state.mjs"; -import { waitForSsh } from "./ssh.mjs"; +import { installFakeAgent, waitForSsh } from "./ssh.mjs"; const imageDir = path.join(import.meta.dirname, "image"); @@ -122,6 +122,9 @@ export async function up(runState, name, options) { if (jump && port) throw new Error("--port and --jump cannot be combined"); + if (options["fake-agent"] && auth !== "key") + throw new Error("--fake-agent needs --auth key"); + const toolchain = options.toolchain ?? "none"; if (toolchain !== "none" && !TOOLCHAIN_IMAGES[toolchain]) @@ -286,6 +289,8 @@ export async function up(runState, name, options) { : [], ); + if (options["fake-agent"]) await installFakeAgent(runState, host); + console.log(alias); } diff --git a/apps/review-desktop/scripts/e2e/remote/fake-agent/acp-agent.mjs b/apps/review-desktop/scripts/e2e/remote/fake-agent/acp-agent.mjs new file mode 100644 index 000000000..3665aadb5 --- /dev/null +++ b/apps/review-desktop/scripts/e2e/remote/fake-agent/acp-agent.mjs @@ -0,0 +1,83 @@ +import { createInterface } from "node:readline"; + +const send = (message) => process.stdout.write(`${JSON.stringify(message)}\n`); + +const reply = (id, result) => send({ jsonrpc: "2.0", id, result }); + +const notify = (method, params) => send({ jsonrpc: "2.0", method, params }); + +const cwds = new Map(); + +let toolCalls = 0; + +createInterface({ input: process.stdin }).on("line", (line) => { + if (!line.trim()) return; + + const { id, method, params } = JSON.parse(line); + + switch (method) { + case "initialize": + return reply(id, { + protocolVersion: 1, + agentCapabilities: { + loadSession: false, + promptCapabilities: { image: false }, + }, + authMethods: [], + }); + case "session/new": { + const sessionId = `wb-test-${cwds.size + 1}`; + + cwds.set(sessionId, params.cwd); + + return reply(id, { + sessionId, + modes: { + currentModeId: "build", + availableModes: [ + { id: "build", name: "Build" }, + { id: "plan", name: "Plan" }, + ], + }, + }); + } + + case "session/set_mode": + return reply(id, {}); + case "session/prompt": { + const { sessionId, prompt } = params; + const question = prompt.findLast((block) => block.type === "text")?.text; + + notify("session/update", { + sessionId, + update: { + sessionUpdate: "agent_message_chunk", + content: { type: "text", text: `You asked: ${question}. See \`f.ts\`.` }, + }, + }); + notify("session/update", { + sessionId, + update: { + sessionUpdate: "tool_call", + toolCallId: `t${++toolCalls}`, + title: "Read f.ts", + kind: "read", + status: "completed", + locations: [{ path: `${cwds.get(sessionId)}/f.ts`, line: 1 }], + }, + }); + + return reply(id, { stopReason: "end_turn" }); + } + + case "session/cancel": + return; + default: + if (id !== undefined) + send({ + jsonrpc: "2.0", + id, + error: { code: -32601, message: `unknown method ${method}` }, + }); + } +}); diff --git a/apps/review-desktop/scripts/e2e/remote/fake-agent/opencode b/apps/review-desktop/scripts/e2e/remote/fake-agent/opencode new file mode 100755 index 000000000..9ea3b4c7a --- /dev/null +++ b/apps/review-desktop/scripts/e2e/remote/fake-agent/opencode @@ -0,0 +1,10 @@ +#!/bin/sh +# wb-test: stands in for OpenCode. `opencode acp` speaks ACP; anything else prints a version. +case "$1" in + acp) + node=$(command -v node) + [ -n "$node" ] || for candidate in /opt/whiteboard/whiteboard-remote/bin/node "$HOME"/.dev/whiteboard-remote/versions/*/bin/node; do [ -x "$candidate" ] && node=$candidate; done + [ -x "$node" ] || { echo "opencode (wb-test): no node on PATH or in a Whiteboard install" >&2; exit 127; } + exec "$node" "${0%/*}/acp-agent.mjs" ;; + *) echo "opencode 0.0.0-wb-test" ;; +esac diff --git a/apps/review-desktop/scripts/e2e/remote/remote.mjs b/apps/review-desktop/scripts/e2e/remote/remote.mjs index 583c3bba5..100cdc02a 100644 --- a/apps/review-desktop/scripts/e2e/remote/remote.mjs +++ b/apps/review-desktop/scripts/e2e/remote/remote.mjs @@ -15,13 +15,13 @@ const usage = `usage: remote.mjs up [--platform linux/amd64|linux/arm64] [--image ] [--node 24|20|none] [--auth key|password] [--banner] [--shell bash|fish] [--jump ] [--sealed] [--delay-ms ] [--no-forwarding] [--port ] - [--toolchain rust|swift|dotnet] + [--toolchain rust|swift|dotnet] [--fake-agent] install [--version ] [--commit ] pack --out [--version ] [--arch x64|arm64] ssh -- forward pause | resume | logs - aws-up [--arch x64|arm64] [--sealed] + aws-up [--arch x64|arm64] [--sealed] [--fake-agent] down | down --all | down --every-run verify-clean @@ -49,6 +49,7 @@ async function main(argv) { "no-forwarding": { type: "boolean" }, port: { type: "string" }, toolchain: { type: "string" }, + "fake-agent": { type: "boolean" }, version: { type: "string" }, commit: { type: "string" }, out: { type: "string" }, diff --git a/apps/review-desktop/scripts/e2e/remote/ssh.mjs b/apps/review-desktop/scripts/e2e/remote/ssh.mjs index d28c5e956..67570fafe 100644 --- a/apps/review-desktop/scripts/e2e/remote/ssh.mjs +++ b/apps/review-desktop/scripts/e2e/remote/ssh.mjs @@ -145,3 +145,21 @@ export async function stopTunnels(runState, name) { state.forwards = state.forwards.filter((f) => name && f.host !== name); await runState.save(); } + +/** Puts the fake OpenCode in ~/.opencode/bin, where Ask finds it. */ +export async function installFakeAgent(runState, host) { + const dir = path.join(import.meta.dirname, "fake-agent"); + + await run("ssh", [...sshArgs(runState, host), "mkdir -p ~/.opencode/bin"]); + await run("scp", [ + "-F", + `${runState.dir}/ssh_config`, + path.join(dir, "opencode"), + path.join(dir, "acp-agent.mjs"), + `${host.alias}:.opencode/bin/`, + ]); + await run("ssh", [ + ...sshArgs(runState, host), + "chmod 755 ~/.opencode/bin/opencode ~/.opencode/bin/acp-agent.mjs", + ]); +} diff --git a/docs/remote-hosts.md b/docs/remote-hosts.md index 11b387ed3..833715da9 100644 --- a/docs/remote-hosts.md +++ b/docs/remote-hosts.md @@ -117,6 +117,14 @@ Once a host is `online`, Settings lists its unconnected agents: Claude Code, Codex, OpenCode and Pi. **Connect** runs the agent's own install commands on the host. +**Ask.** Ask works on a review that lives on a host. The agent runs on the +host, in the review's checkout, with the agents and sign-ins already there +(`claude auth login`, `codex login`, `opencode auth login`, +`cursor-agent login`, Pi's `/login`). Threads are kept on the host, so every +laptop that connects to it sees the same history for its reviews. A file that +an answer names shows as text, because Source windows are not available for a +remote review. + ## Remove Whiteboard from a host In Settings, click **Remove** on the host, check **Also remove Whiteboard from diff --git a/packages/review/app/src/ask-files.test.tsx b/packages/review/app/src/ask-files.test.tsx index bf8b48495..052da414b 100644 --- a/packages/review/app/src/ask-files.test.tsx +++ b/packages/review/app/src/ask-files.test.tsx @@ -122,3 +122,63 @@ it("links the files an answer names to the Source window, once the checkout has await act(async () => root.unmount()); }); + +it("shows the files an answer names as text where Source windows are unavailable", async () => { + vi.stubGlobal("IS_REACT_ACT_ENVIRONMENT", true); + const post = vi.fn(async () => ({ ok: true })); + const session = testReviewSession({}, { post }); + const fetch = vi.spyOn(session, "fetch"); + + session.review!.available = { + sourceWindows: false, + languageFeatures: false, + }; + + const entries: AgentEntry[] = [ + { kind: "agent", id: "answer", text: "See `src/f.ts:12`." }, + ]; + + const thread: AskThreadState = { + id: "thread", + agent: "codex", + agentName: "Codex", + status: "idle", + readOnly: true, + bypass: false, + head: "7fd03b8e2", + cwd: "/checkouts/whiteboard", + selection: { title: "remote review" }, + entries, + }; + + const container = document.createElement("div"); + document.body.append(container); + const root = createRoot(container); + + await act(async () => + root.render( + + + null} + /> + + , + ), + ); + + const code = [...container.querySelectorAll("code")].find( + (element) => element.textContent === "src/f.ts:12", + )!; + + expect(code).toBeDefined(); + expect(container.querySelector('[role="link"], a, button')).toBeNull(); + + await act(async () => code.click()); + expect(fetch).not.toHaveBeenCalled(); + expect(post).not.toHaveBeenCalled(); + + await act(async () => root.unmount()); +}); diff --git a/packages/review/app/src/ask-files.tsx b/packages/review/app/src/ask-files.tsx index d5c5dac74..6794c626a 100644 --- a/packages/review/app/src/ask-files.tsx +++ b/packages/review/app/src/ask-files.tsx @@ -141,16 +141,19 @@ function AskFileLink({ children: ReactNode; }): ReactNode { const files = useContext(AskFilesContext); + const canOpen = useReviewSession().review?.available?.sourceWindows !== false; const { path, line } = fileRef; - useEffect(() => files?.request(path), [files, path]); + useEffect(() => { + if (canOpen) files?.request(path); + }, [canOpen, files, path]); const file = files?.lookup(path); const tooltip = useTooltip( `Open ${file}${line ? `:${line}` : ""}`, ); - if (!files || !file) return fallback; + if (!canOpen || !files || !file) return fallback; const open = () => files.open(file, line); // A link, not a button: a button is one box, so a long path would jump to diff --git a/packages/review/app/src/ask-setup.tsx b/packages/review/app/src/ask-setup.tsx index 04dfee72e..a3e2a170f 100644 --- a/packages/review/app/src/ask-setup.tsx +++ b/packages/review/app/src/ask-setup.tsx @@ -24,6 +24,7 @@ export function AskSetup({ selection: AgentSelection; }): ReactElement { const session = useReviewSession(); + const host = session.review?.host; const notify = useNotify(); const copy = async () => { @@ -47,7 +48,11 @@ export function AskSetup({
    @@ -61,7 +66,9 @@ export function AskSetup({ {candidate.name} - Not installed + + {host ? `Not installed on ${host}` : "Not installed"} + ))} @@ -93,6 +100,8 @@ export function AskSignIn({ command: string; onRetry: () => void; }): ReactElement { + const host = useReviewSession().review?.host; + return (

    {agentName} is signed out. Sign in again in a terminal, then try again; - the question is still here. + the question is still here.{host ? ` Run it on ${host}.` : null}

    $ {command} diff --git a/packages/review/src/ask/agents.ts b/packages/review/src/ask/agents.ts index acc83aaea..109fc1d3e 100644 --- a/packages/review/src/ask/agents.ts +++ b/packages/review/src/ask/agents.ts @@ -206,6 +206,7 @@ export type AskAgentLauncher = ( bypass?: boolean; /** The MCP servers its sessions are given. */ mcpServers?: McpServer[]; + env?: NodeJS.ProcessEnv; }, ) => Promise; @@ -286,15 +287,15 @@ const STDERR_LIMIT = 8_000; export const launchAskAgent: AskAgentLauncher = async ( agent, cwd, - { bypass = false, mcpServers = [] } = {}, + { bypass = false, mcpServers = [], env: base = process.env } = {}, ) => { const spec = askAgents[agent]; - const executable = await findAgent(spec); + const executable = await findAgent(spec, base); if (!executable) throw new Error(`${spec.name} is not installed.`); const env: NodeJS.ProcessEnv = { - ...process.env, + ...base, ...((bypass && spec.bypass?.env) || spec.env), }; diff --git a/packages/review/src/ask/threads.ts b/packages/review/src/ask/threads.ts index 2bd13f02c..b529f6166 100644 --- a/packages/review/src/ask/threads.ts +++ b/packages/review/src/ask/threads.ts @@ -1,5 +1,5 @@ import { PROTOCOL_VERSION, client, methods } from "@agentclientprotocol/sdk"; -import { errorMessage } from "@dev.fast/trace-core"; +import { errorMessage, shellQuote } from "@dev.fast/trace-core"; import { type AskAgentLauncher, askAgentTakesMcp, @@ -146,6 +146,52 @@ export interface AskTools { takesMcp?: (agent: AskAgentId) => Promise; } +/** Whiteboard's CLI as the tools, with `selector` in its env so it reaches + * this server and no other. */ +export function cliAskTools( + cliPath: () => string | undefined, + selector: { name: string; value: string }, +): AskTools { + const env = () => [ + selector, + ...(process.versions.electron + ? [{ name: "ELECTRON_RUN_AS_NODE", value: "1" }] + : []), + ...(process.env.DEV_REVIEW_HOME + ? [{ name: "DEV_REVIEW_HOME", value: process.env.DEV_REVIEW_HOME }] + : []), + ]; + + return { + mcpServers: () => { + const cli = cliPath(); + + return cli + ? [ + { + name: "whiteboard", + command: process.execPath, + args: [cli, "mcp"], + env: env(), + }, + ] + : []; + }, + cli: () => { + const cli = cliPath(); + + return ( + cli && + [ + ...env().map(({ name, value }) => `${name}=${shellQuote(value)}`), + shellQuote(process.execPath), + shellQuote(cli), + ].join(" ") + ); + }, + }; +} + /** How an agent reaches Whiteboard's tools, which its first prompt says. */ export type AskToolsReach = | { kind: "mcp" } diff --git a/packages/review/src/server/desktop-server.ts b/packages/review/src/server/desktop-server.ts index 18a3ef25a..d3de0471d 100644 --- a/packages/review/src/server/desktop-server.ts +++ b/packages/review/src/server/desktop-server.ts @@ -20,8 +20,8 @@ import { parseReviewCliInstallApplyRequest, reviewDiffrSummarizerInputSchema, } from "@dev.fast/review-protocol"; -import { shellQuote, writePrivateJsonAtomic } from "@dev.fast/trace-core"; -import type { AskTools } from "@review/ask/threads.js"; +import { writePrivateJsonAtomic } from "@dev.fast/trace-core"; +import { cliAskTools } from "@review/ask/threads.js"; import { applyCliInstall, declineCliInstall, @@ -149,40 +149,10 @@ export function createGlobalReviewServer( const relay = input.relay ?? new GlobalReviewDesktopVerbRelay({ maxClients: 1 }); - // This Desktop's own CLI, pinned to this instance so another running - // Whiteboard never answers it. - const askCliEnv = () => [ - { name: REVIEW_INSTANCE_ENV, value: identity.key }, - ...(process.versions.electron - ? [{ name: "ELECTRON_RUN_AS_NODE", value: "1" }] - : []), - ...(process.env.DEV_REVIEW_HOME - ? [{ name: "DEV_REVIEW_HOME", value: process.env.DEV_REVIEW_HOME }] - : []), - ]; - - // Ask sessions get its MCP server, `whiteboard mcp`; an agent whose model - // would not get it uses `whiteboard api` from its shell instead. - const askTools: AskTools = { - mcpServers: () => - discovery.cliPath - ? [ - { - name: "whiteboard", - command: process.execPath, - args: [discovery.cliPath, "mcp"], - env: askCliEnv(), - }, - ] - : [], - cli: () => - discovery.cliPath && - [ - ...askCliEnv().map(({ name, value }) => `${name}=${shellQuote(value)}`), - shellQuote(process.execPath), - shellQuote(discovery.cliPath), - ].join(" "), - }; + const askTools = cliAskTools(() => discovery.cliPath, { + name: REVIEW_INSTANCE_ENV, + value: identity.key, + }); const reviewStore = input.reviewStore; diff --git a/packages/review/src/server/headless-host.test.ts b/packages/review/src/server/headless-host.test.ts index be0a3ca42..120492751 100644 --- a/packages/review/src/server/headless-host.test.ts +++ b/packages/review/src/server/headless-host.test.ts @@ -17,6 +17,11 @@ import path from "node:path"; import { DatabaseSync } from "node:sqlite"; import { PassThrough } from "node:stream"; +import { + type ClientConnection, + agent, + methods, +} from "@agentclientprotocol/sdk"; import * as diffr from "@dev.fast/diffr"; import { type JsonValue, @@ -26,6 +31,7 @@ import { STRUCTURAL_DIFF_WIRE_VERSION, parseReviewDesktopVerbFrame, } from "@dev.fast/review-protocol"; +import type { AskAgentLauncher } from "@review/ask/agents.js"; import { runReviewCli } from "@review/cli-runner.js"; import { connectReviewApi, @@ -65,6 +71,7 @@ beforeEach(async () => { root = await mkdtemp(path.join(tmpdir(), "review-headless-")); vi.stubEnv("DEV_REVIEW_HOME", root); vi.stubEnv("DEV_FAST_REVIEW_TELEMETRY_DISABLED", "1"); + vi.stubEnv("SHELL", ""); }); afterEach(async () => { @@ -76,6 +83,7 @@ afterEach(async () => { async function start( stateDir = path.join(root, "server"), softwareMapEnabled = false, + launchAskAgent?: AskAgentLauncher, ) { const controller = new AbortController(); const ready = Promise.withResolvers(); @@ -85,6 +93,7 @@ async function start( softwareMapEnabled, signal: controller.signal, onReady: ready.resolve, + launchAskAgent, }); const stop = async () => { @@ -1305,3 +1314,130 @@ it("leaves a review's checkouts to a live lease owner and takes over a dead owne ); expect(worktrees()).toBe(2); }); + +async function askCall( + server: Awaited>, + route: string, + body?: JsonValue, +) { + return fetch(`${server.discovery.url}/reviews-api/${route}`, { + headers: { + "x-review-token": server.discovery.token, + "content-type": "application/json", + }, + ...(body !== undefined && { method: "POST", body: JSON.stringify(body) }), + }); +} + +async function executable(file: string, body: string) { + await mkdir(path.dirname(file), { recursive: true }); + await writeFile(file, `#!/bin/sh\n${body}\n`, { mode: 0o755 }); +} + +async function askAgentsAndEnv() { + const launched = Promise.withResolvers(); + + const server = await start( + undefined, + false, + async (_agent, _cwd, options) => { + launched.resolve(options?.env); + throw new Error("No agent here."); + }, + ); + + const { worktree } = await reviewsOfBothKinds(server.client); + + const { agents } = await ( + await askCall(server, `${worktree}/ask/agents`) + ).json(); + + await askCall(server, `${worktree}/ask`, { + agent: "claude", + question: { text: "Why?" }, + selection: { target: { kind: "text", quote: "value" }, title: "value" }, + }); + + return { + claude: agents.find((entry: { id: string }) => entry.id === "claude"), + env: await launched.promise, + }; +} + +it("finds and launches Ask agents on the login shell's PATH, with its own node last", async () => { + const login = path.join(root, "login-bin"); + await executable(path.join(login, "claude"), "exit 0"); + await executable( + path.join(root, "shell"), + `echo Welcome\nprintf '\\nWHITEBOARD-PATH=%s\\n' '${login}'\necho Bye`, + ); + vi.stubEnv("SHELL", path.join(root, "shell")); + vi.stubEnv("PATH", "/usr/bin:/bin"); + + const { claude, env } = await askAgentsAndEnv(); + + expect(claude).toMatchObject({ available: true }); + expect(env?.PATH).toBe( + [login, "/usr/bin:/bin", path.dirname(process.execPath)].join( + path.delimiter, + ), + ); +}); + +it("opens an Ask thread in the review's checkout and closes it on stop", async () => { + let launches = 0, + stopped = 0; + + const fake = agent({ name: "fake" }) + .onRequest(methods.agent.initialize, () => ({ + protocolVersion: 1, + agentCapabilities: {}, + authMethods: [], + })) + .onRequest(methods.agent.session.new, () => ({ sessionId: "session" })) + .onRequest(methods.agent.session.prompt, () => ({ + stopReason: "end_turn" as const, + })); + + const server = await start(undefined, false, async () => { + let connection: ClientConnection | undefined; + launches++; + + return { + connect: (client) => (connection = client.connect(fake)), + diagnostics: () => "", + stop: () => { + stopped++; + connection?.close(); + }, + }; + }); + + const { root: checkout, worktree } = await reviewsOfBothKinds(server.client); + + const opened = await askCall(server, `${worktree}/ask`, { + agent: "claude", + question: { text: "Why?" }, + selection: { target: { kind: "text", quote: "value" }, title: "value" }, + }); + + expect(opened.status).toBe(200); + + const { threadId } = await opened.json(); + + const reader = ( + await askCall(server, `${worktree}/ask/watch`, { threads: [threadId] }) + ) + .body!.pipeThrough(new TextDecoderStream()) + .getReader(); + + const { value = "" } = await reader.read(); + expect(JSON.parse(value.split("\n")[0]!).update.snapshot.cwd).toBe(checkout); + + await vi.waitFor(() => expect(launches).toBe(1)); + expect(stopped).toBe(0); + await server.stop(); + expect(stopped).toBe(1); + + while (!(await reader.read()).done); +}); diff --git a/packages/review/src/server/headless-host.ts b/packages/review/src/server/headless-host.ts index 7d589aa1e..a33c5c0a4 100644 --- a/packages/review/src/server/headless-host.ts +++ b/packages/review/src/server/headless-host.ts @@ -1,12 +1,18 @@ +import { spawn } from "node:child_process"; import { randomBytes, randomUUID } from "node:crypto"; import { once } from "node:events"; +import { existsSync } from "node:fs"; import { mkdir, realpath, rm } from "node:fs/promises"; import { createServer } from "node:http"; +import path from "node:path"; import { setTimeout as delay } from "node:timers/promises"; import { isObjectValue } from "@dev.fast/json"; import { withFileLock, writePrivateJsonAtomic } from "@dev.fast/trace-core"; +import type { AskAgentLauncher } from "@review/ask/agents.js"; +import { cliAskTools } from "@review/ask/threads.js"; import { desktopUsesHome } from "@review/desktop-discovery.js"; +import { findReviewPackageRoot } from "@review/package-paths.js"; import { openReviewProfile } from "@review/review-api/profile.js"; import { DESKTOP_TAKEOVER_SIGNAL, @@ -40,6 +46,7 @@ interface HeadlessServerInput { /** The CLI's instance, already on the `headless` surface. */ telemetry?: Pick; onReady(discovery: ReviewServerDiscovery): void; + launchAskAgent?: AskAgentLauncher; } export class HeadlessServerBusyError extends Error {} @@ -114,6 +121,60 @@ export async function takeOverServerHome( } } +const LOGIN_PATH_TIMEOUT_MS = 4_000; + +/** Started over ssh, this server has sshd's PATH; agents are on the login + * shell's. */ +async function agentEnv(): Promise { + const login = process.env.SHELL && (await loginPath(process.env.SHELL)); + + return { + ...process.env, + PATH: [login, process.env.PATH, path.dirname(process.execPath)] + .filter(Boolean) + .join(path.delimiter), + }; +} + +function loginPath(shell: string) { + return new Promise((resolve) => { + const child = spawn( + shell, + ["-lic", 'printf "\\nWHITEBOARD-PATH=%s\\n" "$PATH"'], + { detached: true, stdio: ["ignore", "pipe", "ignore"] }, + ); + + let output = ""; + + child.stdout.setEncoding("utf8"); + child.stdout.on("data", (chunk: string) => (output += chunk)); + + const timer = setTimeout(() => { + try { + process.kill(-child.pid!, "SIGKILL"); + } catch {} + + resolve(undefined); + }, LOGIN_PATH_TIMEOUT_MS); + + child.once("error", () => { + clearTimeout(timer); + resolve(undefined); + }); + child.once("close", () => { + clearTimeout(timer); + + const found = output + .replaceAll("\r", "") + .split("\n") + .findLast((line) => line.startsWith("WHITEBOARD-PATH=")) + ?.slice("WHITEBOARD-PATH=".length); + + resolve(found?.startsWith("/") ? found : undefined); + }); + }); +} + async function serve(input: HeadlessServerInput) { if (input.signal.aborted) return; @@ -139,7 +200,13 @@ async function serve(input: HeadlessServerInput) { const relay = new GlobalReviewDesktopVerbRelay(); - const { app, api } = createWhiteboardCore({ + const cliPath = path.join( + findReviewPackageRoot(import.meta.url), + "dist", + "cli.js", + ); + + const { app, api, close } = createWhiteboardCore({ profile: local, relay, token: discovery.token, @@ -148,6 +215,14 @@ async function serve(input: HeadlessServerInput) { // The scratchpad is the laptop's alone, even with a Desktop attached. scratchpad: () => false, status: () => ({ key: "headless", home: input.stateDir }), + ask: { + tools: cliAskTools(() => (existsSync(cliPath) ? cliPath : undefined), { + name: "DEV_REVIEW_SERVER_DIR", + value: input.stateDir, + }), + launch: input.launchAskAgent, + env: agentEnv(), + }, }); app.route("/reviews-api", api); @@ -177,6 +252,8 @@ async function serve(input: HeadlessServerInput) { input.signal.addEventListener("abort", () => resolve(), { once: true }); }); } finally { + close(); + // Watch streams may live forever. Drain ordinary requests, then bound shutdown. const forceClose = setTimeout(() => server.closeAllConnections(), 5_000); forceClose.unref(); diff --git a/packages/review/src/server/headless-sharing.test.ts b/packages/review/src/server/headless-sharing.test.ts index 010ab7816..525f6b4c1 100644 --- a/packages/review/src/server/headless-sharing.test.ts +++ b/packages/review/src/server/headless-sharing.test.ts @@ -37,6 +37,7 @@ it("commits and uploads through a real headless server and CLI without Desktop, vi.stubEnv("DEV_REVIEW_SHARE_ORIGIN", "https://sharing.test"); vi.stubEnv("DEV_REVIEW_HOME", stateDir); vi.stubEnv("DEV_FAST_REVIEW_TELEMETRY_DISABLED", "1"); + vi.stubEnv("SHELL", ""); const realFetch = globalThis.fetch; const requests: { url: string; headers: Headers; body?: string }[] = []; const blobs = new Map(); diff --git a/packages/review/src/server/review-gateway.ts b/packages/review/src/server/review-gateway.ts index 0791cde59..5b1c779dd 100644 --- a/packages/review/src/server/review-gateway.ts +++ b/packages/review/src/server/review-gateway.ts @@ -51,7 +51,7 @@ const LOCAL_ROUTES = new Set([ interface RemoteRoute { method: string; path: string; - /** May wait on preparing a checkout. */ + /** May wait on preparing a checkout or launching an agent. */ slow?: true; } @@ -69,6 +69,23 @@ const REMOTE_ROUTES: readonly RemoteRoute[] = [ { method: "GET", path: "maps/:name" }, { method: "POST", path: "copy-context" }, { method: "GET", path: "language-context", slow: true }, + { method: "GET", path: "ask/agents" }, + { method: "GET", path: "ask/agents/:agent/offer", slow: true }, + { method: "GET", path: "ask/mentions", slow: true }, + { method: "GET", path: "ask/threads" }, + { method: "POST", path: "ask/watch" }, + { method: "GET", path: "ask/:thread" }, + { method: "POST", path: "ask", slow: true }, + { + method: "POST", + path: "ask/:thread/:action{open|choice|permissions}", + slow: true, + }, + { + method: "POST", + path: "ask/:thread/:action{prompt|permission|files|retry|cancel|close}", + }, + { method: "DELETE", path: "ask/:thread" }, ]; const SLOW_ROUTE_TIMEOUT_MS = 120_000; diff --git a/packages/review/src/server/review-server-core.test.ts b/packages/review/src/server/review-server-core.test.ts index b020e9343..197d72550 100644 --- a/packages/review/src/server/review-server-core.test.ts +++ b/packages/review/src/server/review-server-core.test.ts @@ -26,6 +26,7 @@ beforeEach(async () => { root = await mkdtemp(path.join(tmpdir(), "review-server-core-")); vi.stubEnv("DEV_REVIEW_HOME", root); vi.stubEnv("DEV_FAST_REVIEW_TELEMETRY_DISABLED", "1"); + vi.stubEnv("SHELL", ""); }); afterEach(async () => { diff --git a/packages/review/src/server/review-server-core.ts b/packages/review/src/server/review-server-core.ts index 0df82a7b7..f910bfdaf 100644 --- a/packages/review/src/server/review-server-core.ts +++ b/packages/review/src/server/review-server-core.ts @@ -4,7 +4,11 @@ import type { ReviewServerHealthWithToken, } from "@dev.fast/review-protocol"; import { traceMachineEnabled } from "@dev.fast/trace-core"; -import { detectAskAgents, launchAskAgent } from "@review/ask/agents.js"; +import { + type AskAgentLauncher, + detectAskAgents, + launchAskAgent, +} from "@review/ask/agents.js"; import { AskThreads, type AskTools } from "@review/ask/threads.js"; import { readBuildCommit, @@ -128,7 +132,13 @@ export interface WhiteboardCoreInput { scratchpad: () => boolean; status: () => JsonObject; hooks?: ReviewApiHooks; - ask?: { tools: AskTools }; + ask?: { + tools: AskTools; + /** Tests only: an in-process agent. */ + launch?: AskAgentLauncher; + /** Where agents are found, and the environment they run in. */ + env?: Promise; + }; } export function createWhiteboardCore(input: WhiteboardCoreInput) { @@ -143,8 +153,17 @@ export function createWhiteboardCore(input: WhiteboardCoreInput) { const callbacks = relayReviewCallbacks(input.relay, input.softwareMapEnabled); + const { ask } = input; + + const launch = ask?.launch ?? launchAskAgent; + const askThreads = - input.ask && new AskThreads(launchAskAgent, input.ask.tools); + ask && + new AskThreads( + async (agent, cwd, options) => + launch(agent, cwd, { ...options, env: await ask.env }), + ask.tools, + ); const closeDeletedThreads = () => askThreads?.closeDeletedReviews((id) => @@ -171,7 +190,10 @@ export function createWhiteboardCore(input: WhiteboardCoreInput) { () => traceMachineEnabled(), input.status, input.hooks, - askThreads && { threads: askThreads, agents: () => detectAskAgents() }, + askThreads && { + threads: askThreads, + agents: async () => detectAskAgents(await ask?.env), + }, ); // A shared store mounts the publisher with the rest of sharing.