diff --git a/.claude/skills/verify-berthd/SKILL.md b/.claude/skills/verify-berthd/SKILL.md index becdc70b..62013f04 100644 --- a/.claude/skills/verify-berthd/SKILL.md +++ b/.claude/skills/verify-berthd/SKILL.md @@ -5,7 +5,7 @@ description: Prove a change to the Shipyard app works the way a person sees it - # Verify the Shipyard app -The app (`app/`) is what people touch. Its production build runs in a browser on the mock fixtures (`?mock=1`, `src/lib/mock*.ts`, synthetic "acme" data), with no laptop agent and no box. That is the surface this skill drives. The Go daemons (`berthd`, `berth`) are verified by `go test -race ./...`; a box-side change also needs a run on a real box, which this skill doesn't cover. +The app (`app/`) is what people touch. Its production build runs in a browser on the mock fixtures (`?mock=1`, `src/lib/mock*.ts`, synthetic "acme" data), with no laptop agent and no box. That is the surface this skill drives. The Go daemons (`berthd`, `berth`) are verified by `go test -race ./...`; a box-side change also needs a run on a real box: `scripts/dev-box.sh up` starts a box and an agent of this checkout beside whatever Shipyard already runs on the machine, and `drive.mjs --agent` drives the same build against it (see [features/chat-questions.md](features/chat-questions.md)). The e2e suite (`app/e2e`, see `.claude/skills/e2e-tests`) asserts. This skill **shows**: it drives the same build through a scenario and keeps the evidence for a PR or a person. @@ -37,7 +37,7 @@ Read only. It checks the port answers with the app, that the PID in `/tmp/verify node .claude/skills/verify-berthd/scripts/drive.mjs --port $PORT [--out DIR] ``` -`` is a file in `scripts/scenarios/` (by name, without `.mjs`) or a path. The driver: +`` is a file in `scripts/scenarios/` (by name, without `.mjs`) or a path. With `--agent http://127.0.0.1:9378` and `VERIFY_BERTH_TOKEN` in the environment it opens the app on that laptop agent instead of the mock (a scenario that needs it exports `live = true`). The driver: - opens `http://localhost:$PORT/?mock=1` at 1440×900 in dark mode, and waits until the sidebar's `nav-home` is there; - refuses every request to the laptop agent's ports (1377-1379), as the e2e fixtures do; diff --git a/.claude/skills/verify-berthd/features/README.md b/.claude/skills/verify-berthd/features/README.md index a10258b9..db270c15 100644 --- a/.claude/skills/verify-berthd/features/README.md +++ b/.claude/skills/verify-berthd/features/README.md @@ -15,8 +15,9 @@ The maintained list of what a person can do in the app, and how to prove each on | Sidebar places: Pinned, More, Hidden, counts on rows (Home's waiting, Review's, a plugin's) | [sidebar-nav.md](sidebar-nav.md) | `plugin-nav-badges` | | Plugins: built-in and your own, what they add (screens, sidebar items, widgets, panels) | [plugins.md](plugins.md) | `plugin-nav-badges` (sidebar items) | | Home's widget grid | [home-widgets.md](home-widgets.md) | none yet; `app/e2e/home-widgets.spec.ts` asserts it | +| Questions in chat: an agent's form of questions as a card, answered on a real box; kept with Retry and Copy when it can't be | [chat-questions.md](chat-questions.md) | `chat-questions-live` (a real box, `--agent`); `app/e2e/chat.spec.ts` on the mock | ## Proof and skip reporting - Every proof names the feature file and the scenario it ran, and attaches the screenshots from the evidence folder. -- Something the mock can't show (a real box, the Mac app's window and updater, a plugin's helper on a box) is reported as not verified here, with what would verify it. +- Something the mock can't show (the Mac app's window and updater, a plugin's helper on a box) is reported as not verified here, with what would verify it. A real box and agent of this checkout run on the same machine with `../scripts/dev-box.sh` (see [chat-questions.md](chat-questions.md)). diff --git a/.claude/skills/verify-berthd/features/chat-questions.md b/.claude/skills/verify-berthd/features/chat-questions.md new file mode 100644 index 00000000..1bf8efda --- /dev/null +++ b/.claude/skills/verify-berthd/features/chat-questions.md @@ -0,0 +1,40 @@ +# Questions in chat + +When an agent asks with a form of its own (Claude Code's AskUserQuestion: up to four questions, each with options, some taking several picks, each with room for the person's own words), the chat draws it as a card: a step per question, then a review with **Submit answers**. The box fills in the agent's own form with keys, checking its screen after every key (`internal/box/answer.go`, POST `/v1/sessions/{name}/answer`). The card is `app/src/components/conversation/question-form.tsx`. + +## Sub-features + +- `questions-card`: the card's steps, the person's own words, the review, Submit; once answered it folds to what was answered (`app/e2e/chat.spec.ts`, mock). +- `questions-deliver`: on a real box, long own words (wrapped by Claude Code), questions with lists in them, on an agent nobody has open (its pane at tmux's 80x24): every answer arrives word for word and the agent goes on. The box makes the pane tall enough while it answers and puts it back. +- `questions-kept`: when the box can't fill the form in, the card keeps every answer and says why, with **Retry** and **Copy answers** (each answer on a line under its question's name), and the agent's own screen opens under it to finish there. A retry that goes closes both. +- `questions-log`: every answer that doesn't go is in the box's log with why, and the screen it stopped at. + +## How to get to it (user POV) + +- Open an agent in chat and have it ask (or ask it to call AskUserQuestion). The card is in the conversation, and the reply box points at it. + +## Driving it with drive.mjs + +- Mock: `app/e2e/chat.spec.ts` covers the card and, with `?scene=answer-stuck`, a first submit that fails then a Retry. +- Real box: `scenarios/chat-questions-live.mjs`, on a dev box of this checkout (`../scripts/dev-box.sh`): + +```sh +.claude/skills/verify-berthd/scripts/dev-box.sh up +eval "$(.claude/skills/verify-berthd/scripts/dev-box.sh env)" # the token stays in env +# An agent nobody has open sits at 80x24: +tmux -L berth resize-window -t ask -x 80 -y 24 && tmux -L berth set-option -w -u -t ask window-size +node .claude/skills/verify-berthd/scripts/drive.mjs chat-questions-live --port $PORT --agent http://127.0.0.1:9378 +# The failure path: Claude's own-words row holds something else when the box gets there. +VERIFY_SURPRISE="tmux -L berth send-keys -t ask Down Down Down && sleep 0.6 && tmux -L berth send-keys -t ask -l 'typed in the terminal' && sleep 0.8" \ +VERIFY_UNSURPRISE="tmux -L berth send-keys -t ask -N 21 BSpace && sleep 0.8" \ +node .claude/skills/verify-berthd/scripts/drive.mjs chat-questions-live --port $PORT --agent http://127.0.0.1:9378 +.claude/skills/verify-berthd/scripts/dev-box.sh down +``` + +The preview must be opened as `http://localhost:<1420-1439>`: the agent's API only takes those origins. Second view: Claude's own record (`~/.claude/projects/*acme-shop*/*.jsonl`) has the tool result with every answer, and the dev box's log (`/tmp/verify-berthd-box/berthd.log`) has `answer ask: answered 3 questions`, or why not with the screen. + +## Gotchas + +- The scenario sends its prompt from the chat, so the session must be at its prompt: one left at a form from a failed run needs Esc first. +- A box started from the app starts agents structured (Claude through its SDK, which answers through a hook, not keys); `dev-box.sh` sets the box's defaults so they start in their terminals, the path this covers. +- `dev-box.sh up` refuses if the dev box rewrote the agent hooks in your home (it does that only when they are older than its own); put them back from your backup before going on. diff --git a/.claude/skills/verify-berthd/scripts/dev-box.sh b/.claude/skills/verify-berthd/scripts/dev-box.sh new file mode 100755 index 00000000..7725bddf --- /dev/null +++ b/.claude/skills/verify-berthd/scripts/dev-box.sh @@ -0,0 +1,72 @@ +#!/usr/bin/env bash +# dev-box.sh up|env|down: a box and a laptop agent of this checkout's +# build, on this machine, apart from the Shipyard already running here: +# their own state, tmux server and ports. `up` builds bin/, starts berthd +# on 127.0.0.1:7544 and the agent on 9377/9378 (never 1377-1379), pairs +# them as box "devq", adds a synthetic repo "acme-shop" and starts Claude +# Code in its terminal there, session "ask". `env` prints what drive.mjs +# and tmux need; `down` stops it all and deletes the state. +# +# HOME stays real: Claude Code needs its sign-in. The box refreshes +# Claude's hooks in ~ only when they are out of date; `up` stops if it +# did, so a dev box never changes the real one's hooks unnoticed. +set -euo pipefail +root=$(git -C "$(dirname "$0")" rev-parse --show-toplevel) +# Short: a unix socket's path can't be longer than 108 bytes. +dir=${VERIFY_BOX_DIR:-/tmp/verify-berthd-box} +box_env=(BERTH_HOME="$dir/box-home" BERTH_USER_DIR="$dir/box-user" TMUX_TMPDIR="$dir/tmux") +lap_env=(BERTH_HOME="$dir/lap-home" BERTH_USER_DIR="$dir/lap-user" BERTH_UI_ADDR=127.0.0.1:9378 BERTH_PROXY_ADDR=127.0.0.1:9377) +box() { env "${box_env[@]}" "$root/bin/berthd" "$@"; } +lap() { env "${lap_env[@]}" "$root/bin/berth" "$@"; } +tm() { env TMUX_TMPDIR="$dir/tmux" tmux -L berth "$@"; } + +case ${1:-} in +up) + for p in 7544 9377 9378; do + if ss -ltn | grep -q ":$p "; then echo "port $p is taken: is a dev box up already? ($0 down)" >&2; exit 1; fi + done + mkdir -p "$dir"/{box-home,box-user,lap-home,lap-user,tmux} && chmod 700 "$dir/tmux" + make -C "$root" build >/dev/null + env "${box_env[@]}" nohup "$root/bin/berthd" serve --listen 127.0.0.1:7544 >"$dir/berthd.log" 2>&1 & + echo $! >"$dir/berthd.pid" + until grep -q "berthd serving" "$dir/berthd.log"; do sleep 0.5; done + if grep -q "updated berth's hooks" "$dir/berthd.log"; then + echo "the dev box rewrote agent hooks in $HOME (berthd.log): put them back before going on" >&2 + exit 1 + fi + lap pair "$(box pair --address 127.0.0.1:7544 --json | jq -r .link)" --name devq >/dev/null + token=$(lap ui-token | jq -r .token) + # Agents in their terminals, as the bug reports' sessions run. + curl -sf -X PUT -H "Authorization: Bearer $token" -H 'content-type: application/json' \ + http://127.0.0.1:9378/v1/boxes/devq/api/agents/defaults -d '{"structured":false,"mode":"bypassPermissions"}' >/dev/null + repo=$dir/acme-shop + if [[ ! -d $repo/.git ]]; then + git init -q -b main "$repo" && echo "# acme shop" >"$repo/README.md" + git -C "$repo" add . && git -C "$repo" -c user.email=dev@acme.test -c user.name=acme commit -qm init + fi + box location add acme-shop "$repo" >/dev/null + box session new acme-shop --name ask -- claude --dangerously-skip-permissions >/dev/null + # A new folder asks to be trusted first. + sleep 8 + tm send-keys -t ask Up && sleep 0.5 && tm send-keys -t ask Enter + echo "up: box devq (127.0.0.1:7544), agent http://127.0.0.1:9378, worktree devq/acme-shop, session ask" + ;; +env) + # VERIFY_BERTH_TOKEN is a secret: export it, don't print it anywhere else. + echo "export VERIFY_BERTH_TOKEN=$(lap ui-token | jq -r .token) VERIFY_WORKTREE=devq/acme-shop TMUX_TMPDIR=$dir/tmux" + ;; +down) + [[ -f $dir/berthd.pid ]] && kill "$(cat "$dir/berthd.pid")" 2>/dev/null || true + tm kill-server 2>/dev/null || true + # The agent the CLI started: the one on 9378 with this state, no other. + for pid in $(ss -ltnpH "sport = :9378" 2>/dev/null | grep -o 'pid=[0-9]*' | cut -d= -f2 | sort -u); do + tr '\0' '\n' <"/proc/$pid/environ" | grep -qx "BERTH_HOME=$dir/lap-home" && kill "$pid" + done + rm -rf "$dir" + echo "down: $dir removed" + ;; +*) + echo "usage: dev-box.sh up|env|down" >&2 + exit 2 + ;; +esac diff --git a/.claude/skills/verify-berthd/scripts/drive.mjs b/.claude/skills/verify-berthd/scripts/drive.mjs index 7cd83f83..33bd11b4 100755 --- a/.claude/skills/verify-berthd/scripts/drive.mjs +++ b/.claude/skills/verify-berthd/scripts/drive.mjs @@ -1,7 +1,9 @@ #!/usr/bin/env node -// drive.mjs --port N [--out DIR]: drive the app's preview build -// on its mock fixtures through a scenario (scenarios/*.mjs), with the -// scenario's test plugins loaded as built-ins, and keep the evidence. +// drive.mjs --port N [--out DIR] [--agent URL]: drive the +// app's preview build on its mock fixtures through a scenario +// (scenarios/*.mjs), with the scenario's test plugins loaded as built-ins, +// and keep the evidence. --agent drives a real laptop agent instead (a dev +// one, never 1377-1379), with its UI token in VERIFY_BERTH_TOKEN. // See ../SKILL.md. import { mkdirSync, writeFileSync } from "node:fs"; import { createRequire } from "node:module"; @@ -20,12 +22,18 @@ const flag = (name) => { }; const name = args.find((a, i) => !a.startsWith("--") && !args[i - 1]?.startsWith("--")); const port = flag("--port"); -if (!name || !port) { - console.error("usage: drive.mjs --port N [--out DIR]"); +const agent = flag("--agent"); +const token = process.env.VERIFY_BERTH_TOKEN; +if (!name || !port || (agent && !token)) { + console.error("usage: drive.mjs --port N [--out DIR] [--agent URL, with VERIFY_BERTH_TOKEN set]"); process.exit(2); } const file = name.endsWith(".mjs") || name.includes("/") ? resolve(name) : join(here, "scenarios", `${name}.mjs`); const scenario = await import(pathToFileURL(isAbsolute(file) ? file : resolve(file)).href); +if (scenario.live && !agent) { + console.error(`${name} drives a real box: give it --agent (see ../features/chat-questions.md)`); + process.exit(2); +} const stamp = new Date().toISOString().replace(/[:.]/g, "-"); const out = resolve(flag("--out") ?? `/tmp/verify-berthd/${name.replace(/.*\//, "").replace(/\.mjs$/, "")}-${stamp}`); mkdirSync(out, { recursive: true }); @@ -34,6 +42,11 @@ const log = []; const steps = []; const failures = []; const allow = scenario.allowErrors ?? []; +// The live agent's token is in the page's address: a browser build takes +// it only from ?token= (lib/api.ts endpoint, as e2e/live.mjs does). It is +// a dev agent's on loopback (dev-box.sh), and nothing the driver writes +// keeps it. +const redact = (text) => (token ? String(text).replaceAll(token, "") : String(text)); const step = (text) => { steps.push(text); console.log(`- ${text}`); @@ -41,16 +54,16 @@ const step = (text) => { const browser = await chromium.launch(); const context = await browser.newContext({ viewport: { width: 1440, height: 900 }, colorScheme: "dark" }); -// The laptop agent's ports: a mock run must never reach them. +// The laptop agent's ports: a run must never reach this machine's own. await context.route(/^https?:\/\/(?:[^/]*\.)?(?:localhost|127\.0\.0\.1|\[::1\]):(?:1377|1378|1379)(?:\/|$)/, (r) => r.abort()); const page = await context.newPage(); page.on("console", (m) => { - log.push(`[${m.type()}] ${m.text()}`); - if (m.type() === "error" && !allow.some((re) => re.test(m.text()))) failures.push(`console error: ${m.text()}`); + log.push(redact(`[${m.type()}] ${m.text()}`)); + if (m.type() === "error" && !allow.some((re) => re.test(m.text()))) failures.push(redact(`console error: ${m.text()}`)); }); page.on("pageerror", (e) => { - log.push(`[pageerror] ${e.message}`); - failures.push(`page error: ${e.message}`); + log.push(redact(`[pageerror] ${e.message}`)); + failures.push(redact(`page error: ${e.message}`)); }); const plugins = scenario.plugins ?? []; @@ -65,19 +78,22 @@ if (plugins.length) { const shot = async (label) => { await page.screenshot({ path: join(out, `${label}.png`) }); - writeFileSync(join(out, `${label}.aria.txt`), await page.locator("body").ariaSnapshot()); + writeFileSync(join(out, `${label}.aria.txt`), redact(await page.locator("body").ariaSnapshot())); console.log(` saved ${label}.png`); }; let ok = true; try { - step(`open http://localhost:${port}/?mock=1`); - await page.goto(`http://localhost:${port}/?mock=1`); + const params = new URLSearchParams(agent ? { token, agent } : { mock: "1" }); + for (const [k, v] of Object.entries(scenario.params ?? {})) params.set(k, v); + // The token is in the page's address, as the app takes it; never in the log. + step(agent ? `open http://localhost:${port}/ on the agent at ${agent}` : `open http://localhost:${port}/?${params}`); + await page.goto(`http://localhost:${port}/?${params}`); await expect(page.getByTestId("nav-home")).toBeVisible({ timeout: 20_000 }); await scenario.run({ page, shot, step, expect }); } catch (err) { ok = false; - failures.push(`scenario failed: ${err.message?.split("\n").slice(0, 6).join("\n")}`); + failures.push(redact(`scenario failed: ${err.message?.split("\n").slice(0, 6).join("\n")}`)); await page.screenshot({ path: join(out, "failed.png") }).catch(() => {}); } await browser.close(); diff --git a/.claude/skills/verify-berthd/scripts/scenarios/chat-questions-live.mjs b/.claude/skills/verify-berthd/scripts/scenarios/chat-questions-live.mjs new file mode 100644 index 00000000..98191c73 --- /dev/null +++ b/.claude/skills/verify-berthd/scripts/scenarios/chat-questions-live.mjs @@ -0,0 +1,127 @@ +// chat-questions-live: a real Claude Code, in its terminal on a dev box, +// asks three questions (AskUserQuestion) with lists in them; the chat's +// card answers them with long words of the person's own on two and an +// option on one, and the box fills in Claude's own form. With +// VERIFY_SURPRISE (a shell command that puts Claude's form in a state the +// box doesn't expect), the first submit fails: the card keeps every +// answer, Copy answers copies them, VERIFY_UNSURPRISE puts the form back, +// and Retry delivers them. See ../../features/chat-questions.md. +import { execSync } from "node:child_process"; + +export const live = true; +// The forced failure's 409, as the browser logs it. +export const allowErrors = process.env.VERIFY_SURPRISE ? [/status of 409/] : []; +export const params = { view: "conversation", sidebar: "list" }; + +const QUESTIONS = { + questions: [ + { + header: "Rollout", + question: "How should acme roll out the new checkout?\n1. The old flow stays behind a flag for a week.\n2. Support wants a heads-up before any change customers can see, and release notes go out on Mondays.\n3. Mobile ships on its own schedule.", + multiSelect: false, + options: [ + { label: "Flag first (Recommended)", description: "Ship behind the flag, turn it on for staff, then everyone." }, + { label: "All at once", description: "Turn it on for everyone on release day." }, + { label: "Wait", description: "Hold until mobile is ready too." }, + ], + }, + { + header: "Owner", + question: "Who owns the rollout?\n1. Alex wrote most of the change.\n2. Sam is on call that week.", + multiSelect: false, + options: [ + { label: "Alex", description: "The author." }, + { label: "Sam", description: "The on-call engineer." }, + ], + }, + { + header: "Notes", + question: "Anything else the team should know before starting?", + multiSelect: false, + options: [ + { label: "Nothing", description: "No further notes." }, + { label: "Later", description: "More notes will follow." }, + ], + }, + ], +}; + +const WORDS_1 = "Flag first, but keep the flag on for staff only until the support team has had a full Monday with the new flow. If the error rate on payment confirmation goes above half a percent at any point, turn the flag off and tell the channel before anything else."; +const WORDS_3 = "The staging database was restored from last week's backup, so some fixtures may be stale. Release notes are drafted in the shared folder; please add a line about the flag and who to ask."; + +export async function run({ page, shot, step, expect }) { + const worktree = process.env.VERIFY_WORKTREE; + if (!worktree) throw new Error("set VERIFY_WORKTREE to the worktree row's box/name"); + step(`open ${worktree}'s agent in chat`); + const row = page.locator(`[data-testid=worktree-row][data-worktree="${worktree}"]`); + await row.click({ position: { x: 32, y: 12 } }); + const pane = page.locator("[data-testid=pane]:visible").first(); + const chat = pane.getByTestId("chat"); + // A chat with nothing in it yet asks for a first task instead. + let reply = pane.getByRole("textbox", { name: "What should Claude Code do?", exact: true }); + try { + await expect(reply).toBeEditable({ timeout: 15_000 }); + } catch { + reply = pane.getByTestId("composer").getByRole("textbox", { name: "Reply" }); + } + + step("ask Claude for three questions from the reply box"); + await reply.fill(`Call the AskUserQuestion tool once with exactly this input JSON, copying every question, header and option verbatim (keep the newlines): ${JSON.stringify(QUESTIONS)}. After I answer, repeat each answer back verbatim, one per line, and stop.`); + await reply.press("Enter"); + const form = chat.getByTestId("question-form"); + await expect(form).toBeVisible({ timeout: 180_000 }); + await expect(form).toContainText("Claude asks you 3 questions"); + const next = form.getByRole("button", { name: /^(Next|Review|Submit answers)$/ }).last(); + + step("answer: long words on 1 and 3, Sam on 2"); + await form.getByRole("radio", { name: "Something else" }).click(); + await form.getByRole("textbox", { name: "Your own answer" }).fill(WORDS_1); + await shot("1-card"); + await next.click(); + await form.getByRole("radio", { name: /^Sam/ }).click(); + await next.click(); + await form.getByRole("radio", { name: "Something else" }).click(); + await form.getByRole("textbox", { name: "Your own answer" }).fill(WORDS_3); + await next.click(); + await expect(form).toContainText("Review your answers"); + await shot("2-review"); + + if (process.env.VERIFY_SURPRISE) { + step("put Claude's form in a state the box doesn't expect (VERIFY_SURPRISE)"); + execSync(process.env.VERIFY_SURPRISE, { stdio: "inherit" }); + } + step("submit"); + await next.click(); + + if (process.env.VERIFY_SURPRISE) { + const failed = form.getByTestId("question-form-failed"); + await expect(failed).toBeVisible({ timeout: 60_000 }); + await expect(failed).toContainText("Your answers didn't reach Claude"); + await expect(form).toContainText(WORDS_1); + await expect(form).toContainText(WORDS_3); + await shot("3-failed-kept"); + step("Claude's own screen opens under the form, to finish there"); + const own = pane.getByRole("region", { name: "Claude Code's own screen" }); + await expect(own).toBeVisible({ timeout: 20_000 }); + await own.scrollIntoViewIfNeeded(); + await shot("3b-own-screen-below"); + step("Copy answers"); + await page.context().grantPermissions(["clipboard-read", "clipboard-write"]); + await failed.getByRole("button", { name: "Copy answers" }).click(); + await expect.poll(() => page.evaluate(() => navigator.clipboard.readText())).toBe(`Rollout: ${WORDS_1}\nOwner: Sam\nNotes: ${WORDS_3}`); + step("put the form back (VERIFY_UNSURPRISE), then Retry"); + if (process.env.VERIFY_UNSURPRISE) execSync(process.env.VERIFY_UNSURPRISE, { stdio: "inherit" }); + await failed.getByRole("button", { name: "Retry" }).click(); + } + + step("Claude has the answers, word for word, and goes on"); + await expect(form).toHaveCount(0, { timeout: 60_000 }); + const answered = chat.locator("[data-testid=chat-item][data-kind=question]").last(); + await expect(answered).toContainText("Answered 3 questions", { timeout: 60_000 }); + await expect(answered).toContainText(WORDS_1); + await expect(answered).toContainText(WORDS_3); + await expect(answered).toContainText("Sam"); + await shot("4-answered"); + await expect(chat.locator("[data-testid=chat-item][data-kind=text]").last()).toContainText("who to ask", { timeout: 120_000 }); + await shot("5-claude-goes-on"); +} diff --git a/app/e2e/chat.spec.ts b/app/e2e/chat.spec.ts index bd4bc6e8..86195d40 100644 --- a/app/e2e/chat.spec.ts +++ b/app/e2e/chat.spec.ts @@ -186,6 +186,45 @@ test("a form of questions steps through to a review and submits", async ({ app } await expect(app.chat.getByText("I'll plan it for Next minor")).toBeVisible(); }); +test("answers the box couldn't deliver stay in the form, to retry or copy", async ({ app }) => { + mockOnly("answers a question on a box"); + await app.open({ params: { view: "conversation", scene: "answer-stuck" } }); + await app.openWorktree("gpu/shop"); + const form = app.chat.getByTestId("question-form"); + const next = form.getByRole("button", { name: /^(Next|Review|Submit answers)$/ }).last(); + const words = "Ship it Thursday only if the payment provider's sandbox is green by Wednesday noon; otherwise hold it for QA and tell support before anything else."; + await form.getByRole("radio", { name: "Something else" }).click(); + await form.getByRole("textbox", { name: "Your own answer" }).fill(words); + await next.click(); + await form.getByRole("checkbox", { name: /Unit tests/ }).click(); + await next.click(); + await form.getByRole("radio", { name: /bailey/ }).click(); + await next.click(); + await expect(next).toHaveText(/Submit answers/); + await next.click(); + + // It didn't take: the form stays, with every answer and why. + const failed = form.getByTestId("question-form-failed"); + await expect(failed).toBeVisible(); + await expect(failed).toContainText("Your answers didn't reach Claude"); + await expect(failed).toContainText("Claude's screen doesn't show its questions"); + await expect(form).toContainText("Review your answers"); + await expect(form).toContainText(words); + await expect(form).toContainText("Unit tests"); + + // Copy answers: each under its question's name, ready to paste. + await app.context.grantPermissions(["clipboard-read", "clipboard-write"]); + await failed.getByRole("button", { name: "Copy answers" }).click(); + await expect.poll(() => app.page.evaluate(() => navigator.clipboard.readText())).toBe(`Release: ${words}\nChecks: Unit tests\nReviewer: bailey`); + + // Retry goes, and the form becomes what was answered. + await failed.getByRole("button", { name: "Retry" }).click(); + await expect(form).toHaveCount(0); + const answered = app.chat.locator("[data-testid=chat-item][data-kind=question]"); + await expect(answered).toContainText("Answered 3 questions"); + await expect(answered).toContainText(words); +}); + test("the artifacts chip lists the pages an agent published", async ({ app }) => { mockOnly(); await app.openWorktree("devl/search-perf"); diff --git a/app/src/components/conversation/conversation-pane.tsx b/app/src/components/conversation/conversation-pane.tsx index ed289ada..b3775625 100644 --- a/app/src/components/conversation/conversation-pane.tsx +++ b/app/src/components/conversation/conversation-pane.tsx @@ -388,11 +388,13 @@ export function ConversationPane({ box, session, agent: remembered, visible, onS }; // The whole form's answers, filled in on the box; when they don't take, - // the agent's own screen opens to finish there, saying why. + // the agent's own screen opens under the form, which keeps them, saying + // why. A retry that takes closes it. const submitQuestions = async (tool: string, answers: QuestionAnswer[]) => { - if (!client) throw new Error("Not connected"); try { + if (!client) throw new Error("Not connected"); await answerQuestions(client, box, session, { tool, answers }); + setStuck(undefined); } catch (err) { setStuck({ at: s?.state_since, why: errorMessage(err) }); setNudge((n) => n + 1); diff --git a/app/src/components/conversation/question-form.tsx b/app/src/components/conversation/question-form.tsx index d1e70bf3..cbd80135 100644 --- a/app/src/components/conversation/question-form.tsx +++ b/app/src/components/conversation/question-form.tsx @@ -1,13 +1,16 @@ -import { ArrowLeftIcon, ArrowRightIcon, CheckIcon, ListChecksIcon, PencilIcon, SendHorizontalIcon, SquareTerminalIcon, XIcon } from "lucide-react"; +import { ArrowLeftIcon, ArrowRightIcon, CheckIcon, CopyIcon, ListChecksIcon, PencilIcon, RotateCwIcon, SendHorizontalIcon, SquareTerminalIcon, TriangleAlertIcon, XIcon } from "lucide-react"; import { createContext, type ReactNode, useContext, useEffect, useRef, useState } from "react"; +import { ErrorText } from "@/components/error-note"; +import { Alert, AlertAction, AlertDescription, AlertTitle } from "@/components/ui/alert"; import { Button } from "@/components/ui/button"; import { Card } from "@/components/ui/card"; import { Checkbox } from "@/components/ui/checkbox"; import { Input } from "@/components/ui/input"; import { Radio, RadioGroup } from "@/components/ui/radio-group"; import { Spinner } from "@/components/ui/spinner"; -import { complete, type Question, type QuestionAnswer, shownAnswer } from "@/lib/questions"; +import { copyText } from "@/lib/clipboard"; +import { answersText, complete, type Question, type QuestionAnswer, shownAnswer } from "@/lib/questions"; import type { TranscriptItem } from "@/lib/transcript"; import { cn } from "@/lib/utils"; @@ -23,8 +26,8 @@ export interface QuestionsActions { live?: string; // The chat can fill in the form: Claude Code, on a box that answers. canAnswer: boolean; - // Why it couldn't, when it tried: the person finishes in the agent's - // own screen, open below. + // Why it couldn't, when it tried: the form keeps every answer, to try + // again or copy, and the agent's own screen opens below to finish in. stuck?: string; submit(tool: string, answers: QuestionAnswer[]): Promise; } @@ -38,13 +41,14 @@ export function QuestionCard({ it, who }: { it: Item; who: string }) { const [sent, setSent] = useState(); if (it.done) return ; const live = !!ctx && !!it.tool && ctx.live === it.tool; - if (sent && !ctx?.stuck) return ; + if (sent) return ; if (!live) return ; - if (!ctx.canAnswer || ctx.stuck) return ; + if (!ctx.canAnswer) return ; return (
{ await ctx.submit(it.tool ?? "", answers); setSent(answers); @@ -139,7 +143,7 @@ function Asked({ it }: { it: Item }) { // The questions, when only the agent's own screen can answer them: its // terminal opens under the conversation. -function InTerminal({ it, who, reason }: { it: Item; who: string; reason?: string }) { +function InTerminal({ it, who }: { it: Item; who: string }) { return (
@@ -166,7 +170,7 @@ function InTerminal({ it, who, reason }: { it: Item; who: string; reason?: strin
- {reason ? `${reason}.` : `Answer in ${who}'s screen below.`} + Answer in {who}'s screen below.
); @@ -178,7 +182,7 @@ type Draft = { picks: string[]; other: string; otherOn: boolean }; const toAnswer = (d: Draft): QuestionAnswer => ({ picks: d.picks, other: d.otherOn ? d.other.replace(/\s+/g, " ").trim() : "" }); -function Form({ it, who, onSubmit }: { it: Item; who: string; onSubmit(answers: QuestionAnswer[]): Promise }) { +function Form({ it, who, stuck, onSubmit }: { it: Item; who: string; stuck?: string; onSubmit(answers: QuestionAnswer[]): Promise }) { const qs = it.questions; const [drafts, setDrafts] = useState(() => qs.map(() => ({ picks: [], other: "", otherOn: false }))); // One question with one pick goes as soon as it is picked, as the @@ -202,19 +206,19 @@ function Form({ it, who, onSubmit }: { it: Item; who: string; onSubmit(answers: body.current?.querySelector('[role=radio][tabindex="0"], [role=radio], [role=checkbox], textarea, input:not([type=hidden]):not([aria-hidden=true]), button:not([disabled])')?.focus({ preventScroll: true }); }, [step]); const [busy, setBusy] = useState(false); - const [error, setError] = useState(); const answers = drafts.map(toAnswer); + // The box couldn't fill in the agent's form: nothing here is lost. + const failed = busy ? undefined : stuck; const done = qs.map((q, i) => complete(q, answers[i])); const review = reviewed && step === qs.length; const q = qs[Math.min(step, qs.length - 1)]; const set = (i: number, f: (d: Draft) => Draft) => setDrafts((ds) => ds.map((d, j) => (j === i ? f(d) : d))); const submit = async () => { setBusy(true); - setError(undefined); try { await onSubmit(answers); - } catch (err) { - setError(err instanceof Error ? err.message : String(err)); + } catch { + // Why is the pane's (stuck), for this wait. setBusy(false); } }; @@ -269,6 +273,28 @@ function Form({ it, who, onSubmit }: { it: Item; who: string; onSubmit(answers: set(step, f)} onEnter={next} /> )} + {failed && ( +
+ + + Your answers didn't reach {who} + + + They're kept here: try again, or copy them and finish in {who}'s screen below. + + + + + + +
+ )}
{steps > 1 && ( )} - - {error ? error : busy ? `Answering in ${who}'s form…` : review ? "Check your answers, then submit" : `${who} waits for your answer`} - + {busy ? `Answering in ${who}'s form…` : failed ? "Not sent yet" : review ? "Check your answers, then submit" : `${who} waits for your answer`}