diff --git a/.claude/skills/verify-berthd/features/README.md b/.claude/skills/verify-berthd/features/README.md index 4db31eba..6feb7505 100644 --- a/.claude/skills/verify-berthd/features/README.md +++ b/.claude/skills/verify-berthd/features/README.md @@ -18,7 +18,7 @@ The maintained list of what a person can do in the app, and how to prove each on | 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 | -| Chat and board: no question card or answers but the options the agent's screen shows; the mode menu while turn check waits | [chat-ask-card.md](chat-ask-card.md) | `chat-ask-card` (a real box, `--agent`); `app/e2e/ask-card.spec.ts` on a stand-in box | +| Chat and board: no question card or answers but the options the agent's screen shows; the mode menu while turn check waits; a structured agent's form while its helpers report back, and a reply box that answers a card with nothing to pick | [chat-ask-card.md](chat-ask-card.md) | `chat-ask-card`, `chat-structured-ask-live` (a real box, `--agent`); `app/e2e/ask-card.spec.ts` on a stand-in box | | Turn check's "working": an hourglass with why in the sidebar, the tab and the rail's card, not a live turn's spinner; a handoff reads as done | [turn-check-indicator.md](turn-check-indicator.md) | `turn-check-indicator`; `app/e2e/sidebar-turn-check.spec.ts` asserts it, `TestTurnCheckLive` asks the real provider | ## Proof and skip reporting diff --git a/.claude/skills/verify-berthd/features/chat-ask-card.md b/.claude/skills/verify-berthd/features/chat-ask-card.md index 7b82f436..eeef970c 100644 --- a/.claude/skills/verify-berthd/features/chat-ask-card.md +++ b/.claude/skills/verify-berthd/features/chat-ask-card.md @@ -10,12 +10,15 @@ When an agent waits for you, its chat shows what to answer. A permission prompt - `ask-none`: no card for a wait with no options on screen: turn check's wait, a screen at the agent's prompt (whatever list its last message has), Remote Control's menu, the footer's `/rc`. - `mode-at-soft-wait`: the mode menu switches modes during turn check's wait, and stays blocked at a permission prompt. - `board-answers`: the Agent Dashboard's card offers a menu's options as answer chips, and never a numbered list from the agent's last message (`views/dashboard/agent-card.tsx`, `use-screen-tail.ts`). +- `structured-form`: a structured agent's (`driver: "claude"`, the chat's header says "stdio") AskUserQuestion draws as its form even when helpers it left in the background report back after it, and even when their lines put the question megabytes back in its log and the chat is read afresh (a box restart, a chat nobody had open). The box's first read of a log reaches further back when its last 4 MB hold next to none of the conversation (`internal/transcript/transcript.go` `read`); a helper's word never answers the question (`peer.go` `addMessage`, `lib/questions.ts` `openQuestion`). +- `structured-reply`: a structured agent's question or permission card with nothing to pick says "Reply below to answer it.", and the reply box takes words: the box says no to the ask and gives Claude the words (`claudedriver.go` `Send`). A structured agent has no screen of its own to answer at, so only something to pick holds the reply box (`lib/questions.ts` `atMenu`). An agent in its terminal keeps the old hold: its own screen opens instead. ## How to get to it (user POV) - Open a worktree's agent and switch the pane to **Conversation**. - Type `/rc` in the reply box; pick a mode from the mode chip under it (**Permission mode: …**). - Ask the agent something that makes it end its turn on a question in words; ask it to run a command in **Ask before edits**; ask it to use AskUserQuestion. +- A structured agent (an agent the app started, with structured agents on): ask it to leave helpers working in the background and ask you something while they do. ## Driving it with drive.mjs @@ -32,6 +35,23 @@ node .claude/skills/verify-berthd/scripts/drive.mjs chat-ask-card --port $PORT - ``` It drives `/rc`, a switch to bypass, a turn that ends asking which of two numbered items to fix (it waits for `soft_wait: true` on the session), then `/rc` and a switch to **Ask before edits** during that wait, asserting no "Needs your answer" and no Yes button each time. Then a `touch` that asks permission (the card, then Allow) and AskUserQuestion (the form, then Esc). Second view: the box's own API through the agent (`call`): `sessions` (`agent_state`, `soft_wait`, `ask.tool`), `sessions/ask/screen` and `/controls`. +- `structured-form` and `structured-reply` on a real box: `scenarios/chat-structured-ask-live.mjs`, on a dev box whose agent runs structured. A phase asks (four helpers read ~6 MB in the background while the question waits, about two minutes), the box starts again so the chat reads the log afresh, and a phase looks, expecting `form`, `reply` or (for a before on an older build) `locked`. In bash, for its arrays: + +```bash +VERIFY_DRIVER=claude .claude/skills/verify-berthd/scripts/dev-box.sh up +eval "$(.claude/skills/verify-berthd/scripts/dev-box.sh env)" +S=chat-structured-ask-live; A=(--port $PORT --agent http://127.0.0.1:9378) +VERIFY_PHASE=ask node .claude/skills/verify-berthd/scripts/drive.mjs $S "${A[@]}" +.claude/skills/verify-berthd/scripts/dev-box.sh restart # this checkout's berthd: the form +VERIFY_PHASE=look VERIFY_EXPECT=form node .claude/skills/verify-berthd/scripts/drive.mjs $S "${A[@]}" +# Nothing to pick: an older berthd (a main checkout's bin/berthd) reads no form; this app takes a reply. +VERIFY_PHASE=ask node .claude/skills/verify-berthd/scripts/drive.mjs $S "${A[@]}" +.claude/skills/verify-berthd/scripts/dev-box.sh restart ../main/bin/berthd +VERIFY_PHASE=look VERIFY_EXPECT=reply node .claude/skills/verify-berthd/scripts/drive.mjs $S "${A[@]}" +.claude/skills/verify-berthd/scripts/dev-box.sh down +``` + + Second view: the session's own log (`events.jsonl` under the dev box's `box-home`), read by the scenario: the question's `control_request`, the helpers' `task_notification`s after it, and the `control_response` that carried the answers or the reply. ## Gotchas @@ -40,3 +60,5 @@ node .claude/skills/verify-berthd/scripts/drive.mjs chat-ask-card --port $PORT - - Esc on AskUserQuestion declines it, but Claude Code sends no hook for that, so the box keeps the wait (and the reply box stays blocked) until the next prompt. Keep that check last. - Turn check scores the turn: a scripted "reply word for word" prompt scored under its 0.65 bar. Ask for a real decision. - Claude Code 2.1's footer calls the default mode "manual mode". +- Read keeps about 600 lines of a file, ~63 KB of log per call, and one helper told to read 40 files stopped after 24: the scenario splits 100 files over four helpers. +- `dev-box.sh restart` restarts berthd only; the agents in its tmux keep running, so a question stays waiting across it. diff --git a/.claude/skills/verify-berthd/features/chat-questions.md b/.claude/skills/verify-berthd/features/chat-questions.md index 1bf8efda..4d7fba5f 100644 --- a/.claude/skills/verify-berthd/features/chat-questions.md +++ b/.claude/skills/verify-berthd/features/chat-questions.md @@ -36,5 +36,5 @@ The preview must be opened as `http://localhost:<1420-1439>`: the agent's API on ## 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. +- 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. A structured agent's questions are covered by `chat-structured-ask-live` (`VERIFY_DRIVER=claude .claude/skills/verify-berthd/scripts/dev-box.sh up`, [chat-ask-card.md](chat-ask-card.md)). - `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 index 7725bddf..2a10f28a 100755 --- a/.claude/skills/verify-berthd/scripts/dev-box.sh +++ b/.claude/skills/verify-berthd/scripts/dev-box.sh @@ -4,8 +4,12 @@ # 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. +# Code in its terminal there, session "ask" (VERIFY_DRIVER=claude: Claude +# structured instead, as the app starts agents). `env` prints what +# drive.mjs and tmux need; `restart [berthd]` starts the box again, every +# conversation then read afresh, with another build if given (a main +# checkout's bin/berthd, for a before); `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 @@ -19,6 +23,18 @@ lap_env=(BERTH_HOME="$dir/lap-home" BERTH_USER_DIR="$dir/lap-user" BERTH_UI_ADDR box() { env "${box_env[@]}" "$root/bin/berthd" "$@"; } lap() { env "${lap_env[@]}" "$root/bin/berth" "$@"; } tm() { env TMUX_TMPDIR="$dir/tmux" tmux -L berth "$@"; } +serve() { + env "${box_env[@]}" nohup "$1" serve --listen 127.0.0.1:7544 >"$dir/berthd.log" 2>&1 & + local pid=$! + echo $pid >"$dir/berthd.pid" + for _ in $(seq 120); do + grep -q "berthd serving" "$dir/berthd.log" && return + kill -0 $pid 2>/dev/null || { echo "berthd exited before it served: $dir/berthd.log" >&2; exit 1; } + sleep 0.5 + done + echo "berthd isn't serving after a minute: $dir/berthd.log" >&2 + exit 1 +} case ${1:-} in up) @@ -27,9 +43,7 @@ up) 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 + serve "$root/bin/berthd" 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 @@ -45,12 +59,31 @@ up) 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 + if [[ ${VERIFY_DRIVER:-} == claude ]]; then + # Its chat is its stream: no screen of its own, no folder to trust. + curl -sf -X POST -H "Authorization: Bearer $token" -H 'content-type: application/json' \ + http://127.0.0.1:9378/v1/boxes/devq/api/sessions -d '{"name":"ask","location":"acme-shop","agent":"claude","driver":"claude","mode":"bypassPermissions"}' >/dev/null + else + 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 + fi echo "up: box devq (127.0.0.1:7544), agent http://127.0.0.1:9378, worktree devq/acme-shop, session ask" ;; +restart) + # berthd only: tmux, and the agents in it, keep running. + bin=${2:-$root/bin/berthd} + [[ -x $bin ]] || { echo "no berthd at $bin" >&2; exit 1; } + # Until it has exited, not just let go of the port: its cleanup on the + # way out would take the new one's socket. + pid=$(cat "$dir/berthd.pid") + kill "$pid" + for _ in $(seq 150); do kill -0 "$pid" 2>/dev/null || break; sleep 0.2; done + if kill -0 "$pid" 2>/dev/null; then echo "berthd ($pid) is still running after 30 s" >&2; exit 1; fi + serve "$bin" + echo "restarted: $bin" + ;; 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" @@ -66,7 +99,7 @@ down) echo "down: $dir removed" ;; *) - echo "usage: dev-box.sh up|env|down" >&2 + echo "usage: dev-box.sh up|env|restart [berthd]|down" >&2 exit 2 ;; esac diff --git a/.claude/skills/verify-berthd/scripts/scenarios/chat-structured-ask-live.mjs b/.claude/skills/verify-berthd/scripts/scenarios/chat-structured-ask-live.mjs new file mode 100644 index 00000000..da2cd911 --- /dev/null +++ b/.claude/skills/verify-berthd/scripts/scenarios/chat-structured-ask-live.mjs @@ -0,0 +1,203 @@ +// chat-structured-ask-live: a real Claude Code, structured on a dev box +// (VERIFY_DRIVER=claude ../dev-box.sh up), leaves four helpers reading in +// the background and asks two questions (AskUserQuestion) while they read. +// Their lines pile up in the session's log after the question, megabytes +// of them, and they report back. Then the box starts again (../dev-box.sh +// restart), so the chat's conversation is read afresh, as after a restart +// or a chat nobody had open for a while. See ../../features/chat-ask-card.md. +// +// VERIFY_PHASE=ask sends the prompt from the chat and waits until the +// question waits, the helpers are back, and the question +// is more than the box's first read (4 MB) from the end +// (about two minutes). +// VERIFY_PHASE=look opens the chat afresh and checks what it shows, by +// VERIFY_EXPECT: +// form the question's form, with its options: picks them and submits, +// and Claude's record gets the answers. +// reply a card with nothing to pick that says to reply below, and a +// reply box that takes the words: sends them, and Claude's +// record gets them as the answer. +// locked (the bug, for a before) the bare card and a reply box that +// takes nothing. +import { existsSync, globSync, mkdirSync, readFileSync, statSync, writeFileSync } from "node:fs"; +import { join } from "node:path"; + +export const live = true; +export const params = { view: "conversation", sidebar: "list" }; + +const dir = process.env.VERIFY_BOX_DIR ?? "/tmp/verify-berthd-box"; +// Read keeps ~600 lines of a file: ~63 KB of log per read. Four helpers +// read 25 files each, ~6 MB. +const HELPERS = 4; +const EACH = 25; +const FILES = HELPERS * EACH; +const QUESTIONS = { + questions: [ + { header: "Release", question: "Which release do the acme notes cover?", multiSelect: false, options: [{ label: "2.4", description: "The one out now." }, { label: "2.5", description: "The one going out Monday." }] }, + { header: "Readers", question: "Who reads them?", multiSelect: false, options: [{ label: "Customers", description: "Plain words, no internals." }, { label: "The team", description: "Internals welcome." }] }, + ], +}; +const REPLY = "Cover 2.5, for customers"; + +// The session's log on the dev box: berthd agent-host's events.jsonl. +let path; +function log() { + if (path) return path; + const all = globSync(join(dir, "box-home", "**", "conversations", "*", "events.jsonl")); + if (all.length !== 1) throw new Error(`expected one conversation log under ${dir}, found ${all.length}`); + return (path = all[0]); +} +// Its entries, each with the byte offset its line starts at. The host +// appends as it goes: the last line, unless the file ends with one, isn't +// whole yet. +function entries() { + const out = []; + let off = 0; + const lines = readFileSync(log(), "utf8").split("\n"); + lines.pop(); + for (const l of lines) { + if (l) out.push({ off, ...JSON.parse(l) }); + off += Buffer.byteLength(l) + 1; + } + return out; +} +// The answers sent to Claude (control_response) after the first `from`, +// as text. +const answers = () => entries().filter((e) => e.dir === "in" && e.data?.type === "control_response"); +const answeredSince = (from) => answers().slice(from).map((e) => JSON.stringify(e.data)).join("\n"); +const name = (i) => `notes/part${String(i).padStart(3, "0")}.txt`; + +// Synthetic files for the helpers to read. +function seedFiles() { + mkdirSync(join(dir, "acme-shop", "notes"), { recursive: true }); + const line = "acme checkout keeps the basket when a payment retries, and the badge counts what is in it. "; + for (let i = 1; i <= FILES; i++) { + const p = join(dir, "acme-shop", name(i)); + if (!existsSync(p)) writeFileSync(p, Array.from({ length: 600 }, (_, n) => `${i}.${n} ${line}`).join("\n")); + } +} + +// Where the last question's line starts, as bytes from the log's end, and +// how many helpers are back since the prompt (its byte offset, `from`): one +// can be back before the question is. +function sinceAsked(from) { + let at = -1; + let back = 0; + for (const e of entries()) { + if (e.data?.type === "control_request" && e.data.request?.tool_name === "AskUserQuestion") at = e.off; + else if (e.off >= from && e.data?.type === "system" && e.data.subtype === "task_notification") back++; + } + return { behind: at < 0 ? 0 : statSync(log()).size - at, back }; +} + +async function openChat(page, expect) { + const worktree = process.env.VERIFY_WORKTREE; + if (!worktree) throw new Error("set VERIFY_WORKTREE to the worktree row's box/name"); + await page.locator(`[data-testid=worktree-row][data-worktree="${worktree}"]`).click({ position: { x: 32, y: 12 } }); + const pane = page.locator("[data-testid=pane]:visible").first(); + await expect(pane.getByTestId("chat")).toBeVisible({ timeout: 30_000 }); + return pane; +} + +export async function run({ page, shot, step, expect, call }) { + const phase = process.env.VERIFY_PHASE; + if (phase === "ask") { + seedFiles(); + step("open the structured agent's chat"); + const pane = await openChat(page, expect); + 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 for ${HELPERS} helpers reading in the background, and two questions while they read`); + const tasks = Array.from({ length: HELPERS }, (_, h) => `helper ${h + 1}: "Read each of the files ${name(h * EACH + 1)} to ${name((h + 1) * EACH)} with the Read tool, one call per file, in order, then reply with the single word done."`); + await reply.fill( + `First launch ${HELPERS} helpers in one message with the Agent tool, each with run_in_background true and subagent_type general-purpose, with these tasks: ${tasks.join("; ")}. ` + + `Right after launching them, without waiting for them, call the AskUserQuestion tool once with exactly this input JSON: ${JSON.stringify(QUESTIONS)}. ` + + "After I answer, repeat my answers back in one line and stop. Do nothing else.", + ); + // A new session has no log until its first prompt. + let from = 0; + try { + from = statSync(log()).size; + } catch {} + await reply.press("Enter"); + step("the question waits"); + await expect.poll(async () => (await call("/v1/boxes/devq/api/sessions")).find((s) => s.name === "ask")?.ask?.tool ?? "", { timeout: 240_000 }).toBe("AskUserQuestion"); + await shot("1-asked-live"); + step("the helpers read on, then report back, the question still waiting"); + let behind = 0; + await expect + .poll( + () => { + const now = sinceAsked(from); + behind = now.behind; + return now.back; + }, + { timeout: 900_000, intervals: [5_000] }, + ) + .toBeGreaterThanOrEqual(HELPERS); + step(`the question is ${(behind / 1048576).toFixed(1)} MB from the log's end`); + if (behind <= 4 << 20) throw new Error(`the helpers wrote too little after the question (${behind} bytes): raise EACH`); + await shot("2-helpers-back-live"); + return; + } + + if (phase !== "look") throw new Error("set VERIFY_PHASE to ask or look"); + const expectNow = process.env.VERIFY_EXPECT; + step(`open the chat afresh, expecting ${expectNow}`); + const pane = await openChat(page, expect); + const chat = pane.getByTestId("chat"); + const box = pane.getByTestId("composer").getByRole("textbox", { name: "Reply" }); + const send = pane.getByTestId("composer").getByRole("button", { name: "Send" }); + + if (expectNow === "form") { + const form = chat.getByTestId("question-form"); + await expect(form).toContainText("Claude asks you 2 questions", { timeout: 30_000 }); + await expect(chat.locator("[data-testid=chat-item][data-kind=ask]")).toHaveCount(0); + await shot("3-form"); + step("pick 2.5 and Customers, and submit"); + const next = form.getByRole("button", { name: /^(Next|Review|Submit answers)$/ }).last(); + const before = answers().length; + await form.getByRole("radio", { name: /^2\.5/ }).click(); + await next.click(); + await form.getByRole("radio", { name: /^Customers/ }).click(); + await next.click(); + await expect(next).toHaveText(/Submit answers/); + await next.click(); + step("Claude's record has the answers, and it goes on"); + await expect.poll(() => answeredSince(before), { timeout: 60_000, intervals: [2_000] }).toMatch(/2\.5[\s\S]*Customers/); + await expect(chat.locator("[data-testid=chat-item][data-kind=question]").last()).toContainText("Answered 2 questions", { timeout: 60_000 }); + // Answered in the record before the box drops the wait: no bare card + // meanwhile. + await expect(chat.locator("[data-testid=chat-item][data-kind=ask]")).toHaveCount(0); + await shot("4-answered"); + return; + } + + const card = chat.locator("[data-testid=chat-item][data-kind=ask]"); + await expect(card).toContainText("Claude asks you", { timeout: 30_000 }); + await expect(chat.getByTestId("question-form")).toHaveCount(0); + if (expectNow === "locked") { + await expect(card).toContainText("answer it in the terminal"); + await expect(box).toHaveAttribute("placeholder", "Pick an answer above first"); + await box.fill(REPLY); + await expect(send).toBeDisabled(); + await shot("3-locked"); + return; + } + if (expectNow !== "reply") throw new Error("set VERIFY_EXPECT to form, reply or locked"); + await expect(card).toContainText("Reply below to answer it."); + await box.fill(REPLY); + await expect(send).toBeEnabled(); + await shot("3-reply-below"); + step("send the reply"); + const before = answers().length; + await send.click(); + step("Claude's record has the words as the answer, and it goes on"); + await expect.poll(() => answeredSince(before), { timeout: 60_000, intervals: [2_000] }).toContain(REPLY); + await expect(card).toHaveCount(0, { timeout: 60_000 }); + await shot("4-replied"); +} diff --git a/app/e2e/ask-card.spec.ts b/app/e2e/ask-card.spec.ts index c24138fc..929571fc 100644 --- a/app/e2e/ask-card.spec.ts +++ b/app/e2e/ask-card.spec.ts @@ -145,3 +145,96 @@ test("the board still offers a menu's options as answers", async ({ app }) => { await expect(card.getByRole("button", { name: /Overwrite it/ })).toBeVisible(); await expect(card.getByRole("button", { name: /Keep mine/ })).toBeVisible(); }); + +// A structured session (driver "claude") has no screen of its own: its +// chat is the only way to answer it. +const STRUCTURED = { driver: "claude", ask: { tool: "AskUserQuestion" } }; +const helperBack = (id: string, name: string) => ({ kind: "ping", id, msg: { from: { id: `a-${id}`, name, kind: "helper", helper: `toolu_${id}` }, status: "done", summary: `Agent "${name}" finished` }, off: 0 }); +const asking = (items: Record[]) => () => ({ + body: { source: "claude", items: items.map((it, i) => ({ ...it, off: 10 + i * 100 })), next: items.length, crew: [], gen: "1.0", start: 10, file: "abc" }, +}); + +test("a structured agent's questions draw as their form while its helpers report back", async ({ app }) => { + agent.capabilities = ["transcript", "turns", "answer"]; + agent.session = { agent_state: "waiting", turn: `${SESSION}#4`, ...STRUCTURED }; + agent.transcript = asking([ + { kind: "user", id: "u1", text: "Draft the acme release notes" }, + helperBack("tally", "Tally the changes"), + { + kind: "question", + id: "q1", + tool: "toolu_ask", + questions: [ + { question: "Which release do these notes cover?", header: "Release", options: [{ label: "2.4" }, { label: "2.5" }] }, + { question: "Who reads them?", header: "Readers", options: [{ label: "Customers" }, { label: "The team" }] }, + ], + }, + helperBack("shots", "Take the screenshots"), + ]); + await app.open({ agent, params: { view: "conversation" } }); + await app.openWorktree(`${BOX}/fix`); + + const form = app.chat.getByTestId("question-form"); + await expect(form).toContainText("Claude asks you 2 questions"); + await expect(app.chat.locator("[data-testid=chat-item][data-kind=ask]")).toHaveCount(0); + await expect(app.composer.getByRole("textbox", { name: "Reply" })).toHaveAttribute("placeholder", "Answer Claude's questions above"); + if (process.env.BERTH_SHOTS) await app.page.screenshot({ path: `${process.env.BERTH_SHOTS}/structured-form-after-helper.png` }); + const next = form.getByRole("button", { name: /^(Next|Review|Submit answers)$/ }).last(); + await form.getByRole("radio", { name: /2\.5/ }).click(); + await next.click(); + await form.getByRole("radio", { name: /Customers/ }).click(); + await next.click(); + await expect(next).toHaveText(/Submit answers/); + await next.click(); + await expect.poll(() => agent.answers).toEqual([{ tool: "toolu_ask", answers: [{ picks: ["2.5"], other: "" }, { picks: ["Customers"], other: "" }] }]); +}); + +test("a structured agent's ask with nothing to pick is answered from the reply box", async ({ app }) => { + agent.session = { agent_state: "waiting", turn: `${SESSION}#4`, ...STRUCTURED }; + // Its record has no form for it (an older box, or one it can't read). + agent.transcript = asking([ + { kind: "user", id: "u1", text: "Draft the acme release notes" }, + { kind: "text", id: "t1", text: "I'll ask which release first." }, + ]); + await app.open({ agent, params: { view: "conversation" } }); + await app.openWorktree(`${BOX}/fix`); + + const card = app.chat.locator("[data-testid=chat-item][data-kind=ask]"); + await expect(card).toContainText("Claude asks you"); + await expect(card).toContainText("Reply below to answer it."); + await expect(card).not.toContainText("terminal"); + const box = app.composer.getByRole("textbox", { name: "Reply" }); + await expect(box).not.toHaveAttribute("placeholder", "Pick an answer above first"); + await box.fill("Cover 2.5, for customers"); + const send = app.composer.getByRole("button", { name: "Send" }); + await expect(send).toBeEnabled(); + if (process.env.BERTH_SHOTS) await app.page.screenshot({ path: `${process.env.BERTH_SHOTS}/structured-no-options-reply.png` }); + await send.click(); + await expect.poll(() => agent.sends).toContainEqual(expect.objectContaining({ text: "Cover 2.5, for customers", force: true })); +}); + +test("a structured agent's next question, after one answered, keeps its card while its form can't be read", async ({ app }) => { + agent.session = { agent_state: "waiting", turn: `${SESSION}#4`, ...STRUCTURED }; + agent.transcript = asking([ + { kind: "user", id: "u1", text: "Draft the acme release notes" }, + { kind: "question", id: "q1", tool: "toolu_ask1", questions: [{ question: "Which release?", header: "Release", options: [{ label: "2.4" }, { label: "2.5" }] }], answers: ["2.5"], done: true }, + { kind: "text", id: "t1", text: "One more thing before I write them." }, + ]); + await app.open({ agent, params: { view: "conversation" } }); + await app.openWorktree(`${BOX}/fix`); + await expect(app.chat.locator("[data-testid=chat-item][data-kind=question]")).toContainText("Which release?→2.5"); + const card = app.chat.locator("[data-testid=chat-item][data-kind=ask]"); + await expect(card).toContainText("Reply below to answer it."); +}); + +test("a permission in its terminal with no options read still holds the reply box", async ({ app }) => { + agent.session = { agent_state: "waiting", turn: `${SESSION}#4`, ask: { tool: "Bash", input: "make deploy" } }; + agent.transcript = asking([{ kind: "user", id: "u1", text: "Deploy acme" }]); + agent.screen = () => ""; + await app.open({ agent, params: { view: "conversation" } }); + await app.openWorktree(`${BOX}/fix`); + await expect(app.chat.locator("[data-testid=chat-item][data-kind=ask]")).toContainText("make deploy"); + await expect.poll(() => agent.calls.filter((c) => c.includes(`/sessions/${SESSION}/screen`)).length).toBeGreaterThan(0); + await app.composer.getByRole("textbox", { name: "Reply" }).fill("no, stage it first"); + await expect(app.composer.getByRole("button", { name: "Send" })).toBeDisabled(); +}); diff --git a/app/e2e/fake-agent.ts b/app/e2e/fake-agent.ts index 52199913..e8c8e4e4 100644 --- a/app/e2e/fake-agent.ts +++ b/app/e2e/fake-agent.ts @@ -50,6 +50,9 @@ export interface FakeAgent { // dropped once the box had it). sends: Record[]; send: (body: Record) => Answer | "drop"; + // The bodies of POST sessions/fix-claude/answer (a form of questions + // filled in, boxes with "answer"), in order. + answers: Record[]; // Ends every event stream, as an agent restarting does. dropStreams(): void; // Sends an event to the app's stream. @@ -74,6 +77,7 @@ export async function fakeAgent(): Promise { online: {}, sends: [], send: () => ({ body: { sent: true, turn: `${SESSION}#2`, seq: 9, at: now() } }), + answers: [], dropStreams() { for (const s of streams) s.destroy(); streams.clear(); @@ -130,6 +134,11 @@ export async function fakeAgent(): Promise { res.writeHead(status, { "Content-Type": "application/json" }); res.end(JSON.stringify(body)); }; + const json = async (req: IncomingMessage) => { + let raw = ""; + for await (const c of req) raw += c; + return JSON.parse(raw || "{}") as Record; + }; const handle = async (req: IncomingMessage, res: ServerResponse) => { const u = new URL(req.url ?? "/", "http://fake"); res.setHeader("Access-Control-Allow-Origin", req.headers.origin ?? "*"); @@ -154,15 +163,17 @@ export async function fakeAgent(): Promise { if (req.method === "POST" && p === "/v1/refresh") return send(res, 200, status()); if (agent.away && p.startsWith(`/v1/boxes/${BOX}/api/`)) return send(res, 503, { error: `${BOX} is offline; Shipyard is reconnecting`, code: "box_unreachable" }); if (req.method === "POST" && p === `/v1/boxes/${BOX}/api/sessions/${SESSION}/send`) { - let raw = ""; - for await (const c of req) raw += c; - const body = JSON.parse(raw || "{}") as Record; + const body = await json(req); agent.sends.push(body); const a = agent.send(body); if (a === "drop") return void res.destroy(); if (a.delay) await new Promise((r) => setTimeout(r, a.delay)); return send(res, a.status ?? 200, a.body); } + if (req.method === "POST" && p === `/v1/boxes/${BOX}/api/sessions/${SESSION}/answer`) { + agent.answers.push(await json(req)); + return send(res, 200, { answered: [] }); + } if (req.method === "GET" && (p === "/v1/themes" || p === "/v1/templates" || p === "/v1/plugins")) return send(res, 200, []); const m = /^\/v1\/boxes\/([^/]+)\/api\/(.*)$/.exec(p); if (m && m[1] === BOX && req.method === "GET") { diff --git a/app/src/components/conversation/conversation-pane.tsx b/app/src/components/conversation/conversation-pane.tsx index 960267e7..085684d0 100644 --- a/app/src/components/conversation/conversation-pane.tsx +++ b/app/src/components/conversation/conversation-pane.tsx @@ -43,7 +43,7 @@ import { useNotifications } from "@/lib/notifications"; import { updateBoxes } from "@/lib/outdated"; import { addComment, type LineComment, pending, removeComment, sendComments, useComments } from "@/lib/review-comments"; import { githubRepo } from "@/lib/repo-refs"; -import { answerQuestions, type QuestionAnswer } from "@/lib/questions"; +import { answerQuestions, atMenu, lastQuestion, type QuestionAnswer } from "@/lib/questions"; import { permissionChoices } from "@/lib/screen"; import { NONE, useStore } from "@/lib/store"; import { isStructured } from "@/lib/structured"; @@ -98,14 +98,20 @@ export function ConversationPane({ box, session, agent: remembered, visible, onS // from its record: the chat draws it whole and the box fills it in. stuck // is why the box couldn't, for this wait: its own screen opens instead. const canAnswer = useStore((st) => !!st.boxes[box]?.info?.capabilities?.includes("answer")); - const openQ = useMemo(() => { - for (let i = items.length - 1; i >= 0; i--) { - const it = items[i]; - if (it.kind === "question") return it.done ? undefined : it; - if (it.kind === "user") return undefined; - } - }, [items]); + // The form it waits on: the last one it asked, unless answered. + const lastQ = useMemo(() => lastQuestion(items), [items]); + const openQ = lastQ?.done ? undefined : lastQ; const formAsk = state === "waiting" && !!openQ; + // The form this wait showed, by its call: answered in the record, the + // box drops the wait a moment later, and no card stands in for it + // meanwhile. A wait the chat never showed a form for keeps its card. The + // pane can change session, so the form is this session's. + const [shownForm, setShownForm] = useState<{ key: string; at?: string; tool: string }>(); + const openTool = formAsk ? openQ?.tool : undefined; + useEffect(() => { + if (openTool) setShownForm({ key, at: s?.state_since, tool: openTool }); + }, [key, openTool, s?.state_since]); + const formDone = !!lastQ?.done && shownForm?.key === key && shownForm.at === s?.state_since && shownForm.tool === lastQ.tool; // Prompts sent from here that the transcript doesn't show yet. // Each remembers the prompts like it the transcript already held, by ID: // the transcript is a sliding window, so a position in it doesn't last. @@ -141,6 +147,9 @@ export function ConversationPane({ box, session, agent: remembered, visible, onS const [stuck, setStuck] = useState<{ at?: string; why: string }>(); const stuckNow = stuck && stuck.at === s?.state_since ? stuck.why : undefined; const answerable = formAsk && canAnswer && agent === "claude" && !stuckNow; + // A structured session's screen is its host's, never the agent's own: + // there is nothing on it to answer, and it never shows a prompt. + const structured = isStructured(s?.driver); // Comments on the diff are kept per worktree, as Review keys them. const wt = s ? worktreeOf(locations, s) : undefined; const reviewKey = wt ? `${box}|${wt.worktree.path}` : undefined; @@ -211,11 +220,11 @@ export function ConversationPane({ box, session, agent: remembered, visible, onS // A form of questions draws itself (its question item); one the chat // can't read from the record (an older box) only its screen answers. const form = formAsk || !!ask?.form; - if (state === "waiting" && s?.ask?.tool && !form) { + if (state === "waiting" && s?.ask?.tool && !form && !formDone) { // The agent's hooks said what it asks: show that, with its screen's // options matched to Allow, Always allow and Deny. const choices = ask ? (permissionChoices(ask.choices) ?? ask.choices) : []; - out.push({ kind: "ask", id: "live:ask", tool: s.ask.tool, detail: s.ask.input ?? "", why: s.ask.why, structured: true, choices, reading: !ask, decided }); + out.push({ kind: "ask", id: "live:ask", tool: s.ask.tool, detail: s.ask.input ?? "", why: s.ask.why, structured: true, choices, reading: !ask, decided, reply: structured }); } else if (state === "waiting" && ask && !form && ask.choices.length) { // A question is answered by its options: with none read, there is // nothing to pick (never a made-up Yes / No), and its own screen @@ -223,7 +232,7 @@ export function ConversationPane({ box, session, agent: remembered, visible, onS out.push({ kind: "ask", id: "live:ask", tool: "Question", detail: s?.ask?.message || ask.detail, choices: ask.choices, decided }); } return out; - }, [mock, items, state, s?.state_since, s?.ask, s?.created, startedWith, ask, answered, sent, prompted, onScreen, last, formAsk, drafts.draft, drafts.status, drafts.supported]); + }, [mock, items, state, s?.state_since, s?.ask, s?.created, startedWith, ask, answered, sent, prompted, onScreen, last, formAsk, formDone, structured, drafts.draft, drafts.status, drafts.supported]); const edits = useMemo(() => { if (!client || (!canDiff && !mock)) return undefined; @@ -271,9 +280,6 @@ export function ConversationPane({ box, session, agent: remembered, visible, onS }, [answered, s?.state_since, state, forWords]); const staleNow = !!stale && stale === s?.state_since; const recognised = state === "waiting" && !staleNow && (answerable || (!formAsk && !ask?.form && (ask ? !!ask.choices.length : !!s?.ask?.tool))); - // A structured session's screen is its host's, never the agent's own: - // there is nothing on it to answer, and it never shows a prompt. - const structured = isStructured(s?.driver); const live = useLiveScreen({ box, session, agent, enabled: visible && !mock && !away && !!s && state !== "exited" && !recognised && !structured, running: state === "running", nudge }); // Claude Code and Codex write their conversation once they start: until @@ -501,7 +507,17 @@ export function ConversationPane({ box, session, agent: remembered, visible, onS // An option that asks for words ("Tell Claude what to change" on a plan) // leaves the agent at a text field once picked: the reply box types them. const wantsWords = forWords; - const atMenu = !wantsWords && (formAsk || !!ask?.form || !!ask?.choices.length || !!s?.ask?.tool || (open?.kind === "ask" && !open.decided && (!!open.choices?.length || !!open.structured))); + const menu = atMenu({ + structured, + form: formAsk, + answerable, + screenForm: !!ask?.form, + choices: ask?.choices.length ?? 0, + reading: !mock && !ask, + asked: !!s?.ask?.tool, + card: open?.kind === "ask" && !open.decided ? { choices: open.choices?.length ?? 0, hooks: !!open.structured } : undefined, + wantsWords, + }); latest.current = { reply, fail, again, showTerminal: () => onShowTerminal?.(), hideLive: live.hide, sendComments: () => sendComments(box, session, reviewKey!) }; if (ended && !shown.length) { @@ -598,7 +614,7 @@ export function ConversationPane({ box, session, agent: remembered, visible, onS live={live.show} terminal={!!onShowTerminal} mode={state === "running" ? "queue" : state === "waiting" ? "answer" : "send"} - blocked={(asking && atMenu) || live.show} + blocked={(asking && menu) || live.show} hint={live.show ? (liveWhy ?? `${who} is showing its own screen: answer it above`) : wantsWords ? `Tell ${who} what to change, then press Enter` : answerable ? `Answer ${who}'s ${openQ?.questions.length === 1 ? "question" : "questions"} above` : undefined} actions={actions} /> diff --git a/app/src/components/conversation/conversation-view.tsx b/app/src/components/conversation/conversation-view.tsx index 564f9fc9..04e0b6e2 100644 --- a/app/src/components/conversation/conversation-view.tsx +++ b/app/src/components/conversation/conversation-view.tsx @@ -642,6 +642,8 @@ function Permission({ it, onAnswer, who }: { it: Extract Reading its options… + ) : it.reply ? ( + "Reply below to answer it." ) : ( "Its options aren't on its screen: answer it in the terminal." )} diff --git a/app/src/lib/questions.test.ts b/app/src/lib/questions.test.ts index f4d99e3f..9f80a062 100644 --- a/app/src/lib/questions.test.ts +++ b/app/src/lib/questions.test.ts @@ -2,7 +2,8 @@ import assert from "node:assert/strict"; import { test } from "node:test"; -import { answersText, type Question } from "./questions.ts"; +import type { TranscriptItem } from "./transcript.ts"; +import { answersText, atMenu, lastQuestion, type Question, type Waiting } from "./questions.ts"; const qs: Question[] = [ { question: "How should acme roll out the new checkout?\n1. The old flow stays behind a flag.", header: "Rollout", options: [{ label: "Flag first" }, { label: "All at once" }] }, @@ -18,3 +19,59 @@ test("copied answers are one line each, under the question's name", () => { test("a question without a header is named by its first line", () => { assert.equal(answersText([{ ...qs[0], header: undefined }], [{ picks: ["Flag first"] }]), "How should acme roll out the new checkout?: Flag first"); }); + +const asked: TranscriptItem = { kind: "question", id: "q", tool: "toolu_q", questions: qs }; +const prompt: TranscriptItem = { kind: "user", id: "u", text: "Draft the acme release notes" }; +const back: TranscriptItem = { kind: "ping", id: "p", msg: { from: { id: "a1", name: "Tally the changes", kind: "helper" }, status: "done" } }; +const handedBack: TranscriptItem = { kind: "agent-message", id: "m", msg: { from: { id: "a2", name: "Take the screenshots", kind: "helper" }, intent: "report", body: "Six screenshots." } }; + +// The form the agent waits on, as the chat reads it. +const openQuestion = (items: TranscriptItem[]) => { + const q = lastQuestion(items); + return q?.done ? undefined : q; +}; + +test("a question stays open while the agent's helpers report back, until the person speaks", () => { + assert.equal(openQuestion([prompt, asked]), asked); + // One helper back after it, then another's hand-back. + assert.equal(openQuestion([prompt, asked, back]), asked); + assert.equal(openQuestion([prompt, asked, back, handedBack]), asked); + // A helper back before it changes nothing. + assert.equal(openQuestion([prompt, back, asked]), asked); + // The person answered in words, or the form was answered. + assert.equal(openQuestion([prompt, asked, back, { ...prompt, id: "u2" }]), undefined); + assert.equal(openQuestion([prompt, { ...asked, done: true }, back]), undefined); + // Asked before the person's last prompt: answered by it. + assert.equal(openQuestion([asked, prompt]), undefined); + assert.equal(openQuestion([]), undefined); + // Answered, it is still the last one asked: the box's wait for it ends a + // moment later. + const answered = { ...asked, done: true }; + assert.equal(lastQuestion([prompt, answered, back]), answered); + assert.equal(lastQuestion([answered, prompt]), undefined); +}); + +const waiting: Waiting = { structured: false, form: false, answerable: false, screenForm: false, choices: 0, reading: false, asked: false, wantsWords: false }; + +test("a structured agent's ask holds the reply box only while there is something to pick", () => { + const s = { ...waiting, structured: true, asked: true }; + // Nothing to pick (a question whose form isn't read, a permission whose + // options aren't): the reply answers it. + assert.equal(atMenu({ ...s, card: { choices: 0, hooks: true } }), false); + assert.equal(atMenu({ ...s, form: true, card: { choices: 0, hooks: true } }), false); + // A form the box fills in, options to pick, or options still being read. + assert.equal(atMenu({ ...s, form: true, answerable: true }), true); + assert.equal(atMenu({ ...s, choices: 3, card: { choices: 3, hooks: true } }), true); + assert.equal(atMenu({ ...s, reading: true, card: { choices: 0, hooks: true } }), true); + // An option that wants words. + assert.equal(atMenu({ ...s, choices: 3, wantsWords: true }), false); +}); + +test("an agent in its terminal keeps its ask's hold on the reply box: its own screen answers it", () => { + assert.equal(atMenu({ ...waiting, asked: true, card: { choices: 0, hooks: true } }), true); + assert.equal(atMenu({ ...waiting, form: true }), true); + assert.equal(atMenu({ ...waiting, screenForm: true }), true); + assert.equal(atMenu({ ...waiting, choices: 2, card: { choices: 2, hooks: false } }), true); + assert.equal(atMenu(waiting), false); + assert.equal(atMenu({ ...waiting, asked: true, wantsWords: true }), false); +}); diff --git a/app/src/lib/questions.ts b/app/src/lib/questions.ts index b3c01d67..753342ea 100644 --- a/app/src/lib/questions.ts +++ b/app/src/lib/questions.ts @@ -1,4 +1,5 @@ import type { Client } from "@/lib/api"; +import type { TranscriptItem } from "@/lib/transcript"; // Questions an agent asks with a form of its own: Claude Code's // AskUserQuestion (one to four questions, each with its options, some @@ -52,3 +53,47 @@ export function complete(q: Question, a: QuestionAnswer | undefined): boolean { export function answersText(questions: Question[], answers: (QuestionAnswer | undefined)[]): string { return questions.map((q, i) => `${q.header || q.question.split("\n")[0]}: ${shownAnswer(q, answers[i]) || "—"}`).join("\n"); } + +// lastQuestion is the form of questions the agent asked last, unless the +// person has spoken since. Another agent's word after it (a helper back +// from the background, its hand-back) is no answer. +export function lastQuestion(items: TranscriptItem[]): Extract | undefined { + for (let i = items.length - 1; i >= 0; i--) { + const it = items[i]; + if (it.kind === "question") return it; + if (it.kind === "user") return undefined; + } +} + +// What a waiting agent offers the chat to answer it with. +export interface Waiting { + // Runs structured (lib/structured): it has no screen of its own. + structured: boolean; + // A form of questions from its record, and whether the box fills it in. + form: boolean; + answerable: boolean; + // Its screen shows a form no one number answers. + screenForm: boolean; + // The options read (its screen's, or its ask's), and whether they are + // still being read. + choices: number; + reading: boolean; + // Its hooks or its driver said what it asks. + asked: boolean; + // The question card drawn, not yet answered: its options, and whether + // its hooks described it. + card?: { choices: number; hooks: boolean }; + // An option picked that asks for words, which the reply box types. + wantsWords: boolean; +} + +// atMenu says the reply box waits for a pick above: Enter there would pick +// for the agent. A structured agent has no screen to answer at, so only +// something to pick here holds it; with nothing, the reply is the answer +// (its box says no to the ask and gives the agent the words). +export function atMenu(w: Waiting): boolean { + if (w.wantsWords) return false; + const card = w.card?.choices ?? 0; + if (w.structured) return w.answerable || w.reading || w.choices > 0 || card > 0; + return w.form || w.screenForm || w.choices > 0 || w.asked || card > 0 || !!w.card?.hooks; +} diff --git a/app/src/lib/transcript.ts b/app/src/lib/transcript.ts index 87e535d0..4776c94b 100644 --- a/app/src/lib/transcript.ts +++ b/app/src/lib/transcript.ts @@ -21,7 +21,9 @@ export type TranscriptItem = // command). decided is what was answered. // A permission the agent's hooks described (structured) names the tool, // what it would run or touch (detail) and why, rather than a screen line. - | { kind: "ask"; id: string; tool: string; detail: string; choices?: { key: string; label: string; title?: string }[]; decided?: string; why?: string; structured?: boolean; reading?: boolean } + // reply: with no options to pick, a reply answers it (a structured + // agent, which has no screen of its own). + | { kind: "ask"; id: string; tool: string; detail: string; choices?: { key: string; label: string; title?: string }[]; decided?: string; why?: string; structured?: boolean; reading?: boolean; reply?: boolean } // The agent at work: the step it is running (timed from the call), or // its own word for what it does (label, from its status line) with what // else that line says (meta: tokens); since is when it began. diff --git a/docs/changelog.mdx b/docs/changelog.mdx index 1402e0c6..93807252 100644 --- a/docs/changelog.mdx +++ b/docs/changelog.mdx @@ -13,6 +13,14 @@ again. - **The app has a new icon:** a box in its tray with the amber lamp lit on top. A dev build wears the same box on orange. +- **A structured agent's questions can always be answered from the chat.** + An agent that asked you something while helpers it left in the + background were still working showed a bare "Claude asks you" card with + no options, and a reply box that took nothing. Now the chat shows the + agent's form with its options, however much its helpers wrote after + the question, and a helper reporting back no longer counts as your + answer. A card with nothing to pick says **Reply below to answer it**, + and what you type there is the answer. - **An agent waiting on its own work no longer looks like a live turn.** When turn check reads a finished turn as work still running (a build, a deploy, a task it waits on), the sidebar and its tab show an hourglass diff --git a/internal/transcript/history.go b/internal/transcript/history.go index c6ab15d4..93110990 100644 --- a/internal/transcript/history.go +++ b/internal/transcript/history.go @@ -65,7 +65,8 @@ func Before(source, path, dir string, before int64, limit int) (Result, error) { start := max(0, before-span) // One more than asked: read from the middle of a file, the first // item may be the tail of a group begun before it. - c := &conv{source: source, dir: dir, side: isHelper(path), paged: true, limit: limit + 1, p: parserFor(source), byTool: map[string]int{}, crewByID: map[string]int{}} + c := newConv(source, dir, isHelper(path)) + c.paged, c.limit = true, limit+1 if err := c.scan(f, start, before); err != nil { return Result{}, err } diff --git a/internal/transcript/host_test.go b/internal/transcript/host_test.go index 2d68ee1d..870634f4 100644 --- a/internal/transcript/host_test.go +++ b/internal/transcript/host_test.go @@ -158,3 +158,84 @@ func TestAnEditInAStructuredSessionsLogKeepsItsLineNumbers(t *testing.T) { t.Fatalf("detail = %+v, %v", d, err) } } + +// An agent that asks its questions while a helper it left in the +// background works on: the helper's lines go on in the log, megabytes of +// them, and its notifications come back, but the question stays the +// conversation's last word and stays open, read live or afresh. +func TestAQuestionStaysOpenWhileItsHelpersWorkOn(t *testing.T) { + path := filepath.Join(t.TempDir(), "events.jsonl") + seq := 0 + out := func(data m) m { + seq++ + return m{"seq": seq, "at": 1792200000000 + int64(seq)*1000, "dir": "out", "data": data} + } + helperLine := func(parent string, data m) m { data["parent_tool_use_id"] = parent; return out(data) } + launched := func(call, agentID string) m { + return out(user([]m{{"type": "tool_result", "tool_use_id": call, "content": "Async agent launched successfully. agentId: " + agentID}})) + } + back := func(call string) m { + return out(m{"type": "system", "subtype": "task_notification", "tool_use_id": call, "status": "completed"}) + } + write(t, path, + out(user("Draft the acme release notes")), + out(assistant(tool("toolu_tally", "Agent", m{"description": "Tally the changes", "run_in_background": true}))), + launched("toolu_tally", "a1b2c3d4e5f6a7b8c"), + out(assistant(tool("toolu_shots", "Agent", m{"description": "Take the screenshots", "run_in_background": true}))), + launched("toolu_shots", "b1c2d3e4f5a6b7c8d"), + // One helper is back before the question; the other after it. + back("toolu_tally"), + out(assistant(tool("toolu_ask", "AskUserQuestion", m{"questions": threeQuestions[:2]}))), + out(m{"type": "control_request", "request_id": "r1", "request": m{"subtype": "can_use_tool", "tool_name": "AskUserQuestion", "input": m{"questions": threeQuestions[:2]}}}), + ) + live := NewReader() + asked, err := live.Read(SourceClaudeHost, path, "/home/dev/acme", 0) + if err != nil { + t.Fatal(err) + } + pad := strings.Repeat("y", 16<<10) + for i := 0; ; i++ { + n := itoa(i) + write(t, path, + out(m{"type": "stream_event", "event": m{"type": "content_block_delta"}}), + helperLine("toolu_shots", assistant(tool("toolu_r"+n, "Read", m{"file_path": "/home/dev/acme/shot" + n + ".png"}))), + helperLine("toolu_shots", user([]m{{"type": "tool_result", "tool_use_id": "toolu_r" + n, "content": pad}})), + ) + if st, _ := os.Stat(path); st.Size() > maxStart+(1<<20) { + break + } + } + write(t, path, back("toolu_shots")) + + open := func(how string, items []Item) { + t.Helper() + if len(items) == 0 { + t.Fatalf("%s: nothing to show", how) + } + q := items[len(items)-1] + if q.Kind != "question" || q.Done || q.Tool != "toolu_ask" || len(q.Questions) != 2 || len(q.Questions[1].Options) != 4 { + t.Fatalf("%s: last item %+v, not the open question", how, q) + } + } + more, err := live.Read(SourceClaudeHost, path, "/home/dev/acme", asked.Next) + if err != nil { + t.Fatal(err) + } + // The live read sends the open question again, still open and last. + open("followed live", more.Items) + for _, c := range more.Crew { + if c.State != "finished" { + t.Fatalf("a helper not back: %+v", more.Crew) + } + } + afresh, err := NewReader().Read(SourceClaudeHost, path, "/home/dev/acme", 0) + if err != nil { + t.Fatal(err) + } + // Its last 4 MB are the helper's: the window reaches back to the + // conversation, its prompt included. + if afresh.Items[0].Kind != "user" { + t.Fatalf("read afresh, the first item is %+v", afresh.Items[0]) + } + open("read afresh", afresh.Items) +} diff --git a/internal/transcript/peer.go b/internal/transcript/peer.go index 50d17962..73f9978a 100644 --- a/internal/transcript/peer.go +++ b/internal/transcript/peer.go @@ -459,8 +459,10 @@ func (c *conv) addMessage(kind string, m *Message) { } break } + // Another agent's word doesn't answer a question the agent asked the + // person, which stays open until they do (or a prompt of theirs + // settles it). c.closeArtifacts() - c.closeQuestions() c.add(Item{Kind: kind, ID: c.id(), Msg: m}) } diff --git a/internal/transcript/peer_test.go b/internal/transcript/peer_test.go index f0335a7d..1ad54f0e 100644 --- a/internal/transcript/peer_test.go +++ b/internal/transcript/peer_test.go @@ -357,3 +357,27 @@ func TestCodexLeavesItsTurnsAlone(t *testing.T) { t.Fatalf("kinds = %s", got) } } + +// A helper's notification, or its hand-back, while the agent waits on its +// questions isn't the person's answer: the questions stay open until the +// person replies. +func TestAHelpersWordDoesntAnswerTheAgentsQuestion(t *testing.T) { + lines := append(launch("toolu_L", "Map ledger writes", "a3f9c1e7b2d4a6c80"), + assistant(tool("q1", "AskUserQuestion", m{"questions": threeQuestions})), + meta(taskNotification("a3f9c1e7b2d4a6c80", "toolu_L", "completed", `Agent "Map ledger writes" finished`, ""), m{"kind": "task-notification"}), + meta(handback("a3f9c1e7b2d4a6c80", "Ledger writes go through acme/ledger.go only."), m{"kind": "peer", "from": "a3f9c1e7b2d4a6c80", "handback": true}), + ) + res := readAll(t, lines...) + q := only(t, res.Items, "question") + if len(q) != 1 || q[0].Done { + t.Fatalf("after the helper's word: %+v", q) + } + if len(only(t, res.Items, "agent-message")) != 1 { + t.Fatalf("the hand-back isn't shown: %s", kinds(res.Items)) + } + res = readAll(t, append(lines, user("Skip the questions, just write it"))...) + // It closes them unanswered: the prompt is no answer to them. + if q := only(t, res.Items, "question"); len(q) != 1 || !q[0].Done || !q[0].Error || len(q[0].Answers) != 0 { + t.Fatalf("after the person's prompt: %+v", q) + } +} diff --git a/internal/transcript/transcript.go b/internal/transcript/transcript.go index 30f2f900..9e09cd27 100644 --- a/internal/transcript/transcript.go +++ b/internal/transcript/transcript.go @@ -157,6 +157,9 @@ const ( // maxStart is how much of a long file is read when it is first opened: // its end, where the recent conversation is. maxStart = 4 << 20 + // minWindow is the fewest items a window read afresh holds, when the + // file has more: fewer, and it reaches back further (read). + minWindow = 20 // maxText is the most of one reply kept: a long answer, its tables and // code included, reads whole. maxText = 32 << 10 @@ -405,12 +408,40 @@ func (c *conv) read(path string) error { return nil } if st.Size() < c.offset { // replaced or truncated: start again - *c = conv{source: c.source, dir: c.dir, p: c.p, side: c.side, byTool: map[string]int{}, crewByID: map[string]int{}, gen: gens.Add(1)} - } - if c.offset == 0 && st.Size() > maxStart { - c.offset = st.Size() - maxStart - c.truncated = true + c.restart(gens.Add(1)) + } + if c.offset > 0 { + return c.follow(f) + } + // Opened afresh, a long file is read from near its end. A window that + // holds next to none of the conversation reaches further back, as a + // page of older items does: a structured session's log keeps a helper's + // lines beside it, and one left working can write megabytes while the + // agent waits on its question, which is then the window's to show. + for span := int64(maxStart); ; span *= 4 { + c.offset = max(0, st.Size()-span) + c.truncated = c.offset > 0 + if err := c.follow(f); err != nil || !c.truncated || len(c.items) >= minWindow || span >= maxSpan { + return err + } + c.restart(c.gen) } +} + +// newConv is a reading of a file of source's, from nothing. +func newConv(source, dir string, side bool) *conv { + return &conv{source: source, dir: dir, side: side, p: parserFor(source), byTool: map[string]int{}, crewByID: map[string]int{}} +} + +// restart forgets what was read, to read the file again as reading gen. +func (c *conv) restart(gen int64) { + used := c.used + *c = *newConv(c.source, c.dir, c.side) + c.gen, c.used = gen, used +} + +// follow reads the file on from c.offset to its end. +func (c *conv) follow(f *os.File) error { if _, err := f.Seek(c.offset, io.SeekStart); err != nil { return err } @@ -484,8 +515,8 @@ func (r *Reader) Follow(source, path, dir string, since int, gen string) (Result if len(r.convs) >= maxOpen { r.evictOldest() } - c = &conv{source: source, dir: dir, side: isHelper(path), byTool: map[string]int{}, crewByID: map[string]int{}, gen: gens.Add(1)} - c.p = parserFor(source) + c = newConv(source, dir, isHelper(path)) + c.gen = gens.Add(1) r.convs[path] = c } c.used = now diff --git a/internal/transcript/turn.go b/internal/transcript/turn.go index c301c8a9..7e72ef0c 100644 --- a/internal/transcript/turn.go +++ b/internal/transcript/turn.go @@ -238,7 +238,8 @@ func codexTurn(path, dir string, off int64) (Turn, error) { if err != nil { return Turn{}, err } - c := &conv{source: "codex", dir: dir, paged: true, limit: keep, p: parserFor("codex"), byTool: map[string]int{}, crewByID: map[string]int{}} + c := newConv("codex", dir, false) + c.paged, c.limit = true, keep if err := c.scan(f, off, st.Size()); err != nil { return Turn{}, err }