diff --git a/README.md b/README.md index 402668e..3455537 100644 --- a/README.md +++ b/README.md @@ -147,10 +147,13 @@ and open it. than something the reader has to infer. - **Map**: systems, containers, components and code, with what the change added, removed or touched, linked to files and code. -- **Threads**: _Ask now_ sends a question to the agent immediately and the - answer lands in the same thread. _Add to review_ holds a comment until you - submit with _Approve_ or _Request changes_. _Close_ ends the review - without approving it. +- **Threads**: _Send to the agent_ delivers a question at once and the answer + lands in the same thread. _Add to the review_ holds a comment until you + submit with _Approve_ or _Request changes_. _Close_ ends the review without + approving it. Each thread says where it stands - held, queued, delivered or + answered - and the panel says whether an agent is listening at all. Nothing + claims a reader is there when none is: a question asked with no agent + attached is queued, not lost, and reaches the agent the next time it checks. - **Revisions**: every publish is sealed; switch back to earlier ones. - **Theme**: the agent reads the project's design tokens and fonts and publishes them with the review, so each review looks like the code it diff --git a/scripts/demo/record-browser.mjs b/scripts/demo/record-browser.mjs index 6ae9e95..b6806d6 100644 --- a/scripts/demo/record-browser.mjs +++ b/scripts/demo/record-browser.mjs @@ -190,7 +190,8 @@ await type( await sleep(500); await sleep(400); await shot("files"); -await click(".comment-popover button.ok"); +// `.ok` is now "Send to the agent"; this comment is meant to be held for the review. +await clickByText(".comment-popover button", "Add to the review"); await sleep(1600); // 3. Map: what the change added. diff --git a/skills/thurview/SKILL.md b/skills/thurview/SKILL.md index 667bf0a..392a629 100644 --- a/skills/thurview/SKILL.md +++ b/skills/thurview/SKILL.md @@ -249,7 +249,9 @@ Tell the user, in a few lines and nothing more: - which theme source you used: the user's request, the project's design system (name the files), or the default skin - when the review has no map, why not, in one clause -- that you are now waiting for their questions and their decision +- that you are now waiting for their questions and their decision, and that + a question asked after you stop waiting is queued rather than lost - the + page tells them which of the two it is The page explains its own controls; do not describe them. @@ -265,9 +267,16 @@ nothing: keep `--timeout` under that limit and run `wait` again on `timeout`. When the tool can run a command in the background and wake you when it exits, run `wait` that way, so the user has the terminal back while they read. +While `wait` runs the reader's page says an agent is listening, and says the +opposite within seconds of it returning. Do not loop it to look present: a +question asked with nobody waiting is queued, not lost, and `thurview` +reports it as `needsAgent` the next time you run any command in the +worktree. + `wait.reason` says what happened, with the threads that need you: -- `question`: an "Ask now" thread. Answer each thread in `threads` with +- `question`: a thread the reader sent to you. Answer each thread in + `threads` with `thurview threads reply --review --body ""`. Do not change the document for a question. Wait again. - `awaiting-agent-updates`: the reader submitted with "Request changes". diff --git a/skills/thurview/references/lifecycle.md b/skills/thurview/references/lifecycle.md index 841b396..f4ff1a3 100644 --- a/skills/thurview/references/lifecycle.md +++ b/skills/thurview/references/lifecycle.md @@ -27,7 +27,7 @@ warns when the branch moved past them. | `accepted` | Terminal. Cannot be republished. | | `closed` | Terminal. Ended without approval. Cannot be republished. | -"Ask now" does not change the status. "Submit review" with "Request changes" +Asking the agent a question does not change the status. "Submit review" with "Request changes" sets `awaiting-agent-updates`; with "Approve" sets `accepted`; with "Close" sets `closed`. @@ -36,21 +36,62 @@ Dismissal is separate: the reader removes the review from the active list and ## Threads -Two kinds, chosen by the reader when creating one: +Two kinds, chosen by the reader when creating one. In the browser these are +the two buttons on the comment box, one click each: -- `ask` mode (a question): delivered at once. `wait` returns `question`. - Answer with `threads reply`. It stays open until the reader resolves it; - open questions never block a republish. -- `review` mode (a comment): held as pending until the reader submits. Then - `wait` returns `awaiting-agent-updates` with the submitted threads. +- `ask` mode (a question, "Send to the agent"): submitted on creation and + delivered at once. `wait` returns `question`. Answer with `threads reply`. + Open questions never block a republish. +- `review` mode (a comment, "Add to the review"): held as pending until the + reader submits. Then `wait` returns `awaiting-agent-updates` with the + submitted threads. Targets: a document block (with an optional quoted selection), a file line on the base or head side, a map node, or the whole review. +### Status, `submitted` and `needsAgent` + +Two flags and one derived predicate decide whether a thread reaches you. +`needsAgent` is the whole queue: `wait` reports it, `threads list --open` +counts it, and a thread outside it will not be delivered to anyone. + +```text +needsAgent = status is open AND submitted AND the last message is the reader's +``` + +| Transition | status | submitted | +| ------------------------------ | -------------------- | ------------- | +| reader creates an `ask` thread | `open` | `true` | +| reader creates a `review` one | `open` | `false` | +| reader submits the review | unchanged | `true` (all) | +| **reader writes in a thread** | **forced to `open`** | `true` if ask | +| agent replies | unchanged | unchanged | +| `threads resolve` / Resolve | `resolved` | unchanged | +| Reopen | `open` | unchanged | + +A message from the reader always reopens the thread. It has to: a reply that +left the thread resolved would sit at `needsAgent: false`, invisible to +`wait` and to `threads list --open`, and the reader would be writing to +nobody while the page still offered them a Reply button. Publishing a new +revision never touches a thread's status. + `publish` after the first revision requires zero open submitted comment threads. Resolve a thread only when its requested change is present. Do not rewrite or merge threads. +### Presence: what the reader is told + +While `thurview wait` runs it writes a heartbeat to +`${THURVIEW_HOME:-~/.thurview}/agents/.json`, and the browser reads +it back as one of two sentences: an agent is listening now, or nothing is +listening and what you send is queued until one checks in. Nothing else +writes it, so presence is never inferred and never faked. A heartbeat older +than 15 seconds is a dead `wait`, not an agent. + +That is why a question asked while you are away is not lost and does not need +you to sit in `wait`: it is queued, `thurview` reports it as `needsAgent` the +next time you run any command in the worktree, and you answer it then. + ```sh thurview threads list --review [--open] thurview threads get --review @@ -64,6 +105,7 @@ thurview threads resolve --review ${THURVIEW_HOME:-~/.thurview}/ ├── THURVIEW.md user guidance (optional) ├── server.json running server, if any +├── agents/.json heartbeat of a running `wait`, removed when it ends └── reviews// ├── review.md you edit ├── data.yaml you edit @@ -90,3 +132,8 @@ again. `thurview threads get ` truncates bodies over 1500 characters; pass `--full` when the hint says so. + +While `wait` runs, the reader's page says an agent is listening; when it +returns, the page says the opposite within seconds. Do not leave `wait` +running to look present when you are not going to answer, and do not loop it +to keep a queue drained: the queue survives you, and the reader is told so. diff --git a/src/cli.ts b/src/cli.ts index 1ed4d92..b097337 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -53,6 +53,7 @@ import type { InterfaceDelta } from "./interfaces.js"; import { parseTheme, compileTheme, type CompiledTheme } from "./theme.js"; import { registerTheme } from "./highlight.js"; import { replyThread, setThreadStatus, needsAgent } from "./threads.js"; +import { attach } from "./presence.js"; import { startServer } from "./server/server.js"; import { parseFlags, helpFor, str, bool, type FlagSpec } from "./flags.js"; import { VERSION } from "./version.js"; @@ -1140,6 +1141,9 @@ const commands: Record Promise> = { ]); const deadline = Date.now() + seconds * 1000; const id = short(review.id); + // While this loop runs the reader is told an agent is listening, and told + // the opposite the moment it stops. + const listening = attach(review.id); const rows = (ts: Thread[]) => ts.map((t) => ({ id: t.id, @@ -1147,71 +1151,75 @@ const commands: Record Promise> = { target: targetLabel(t.target), last: lastMessage(t), })); - while (Date.now() < deadline) { - const r = await readReview(review.id); - if (!r) - return { - wait: { reason: "review-deleted", id }, - help: ["Stop the loop; the review no longer exists"], - }; - const t = await readThreads(review.id); - const last = t.decisions[t.decisions.length - 1]; - if (r.status === "awaiting-agent-updates") { - const need = t.threads.filter(needsAgent); - return { - wait: { - reason: "awaiting-agent-updates", - id, - status: r.status, - decision: last - ? `${last.decision}${last.body ? `: ${truncate(last.body, 300)}` : ""}` - : "", - }, - count: need.length, - threads: rows(need), - help: [ - `Run \`thurview threads get --review ${id}\` for the full thread`, - `Run \`thurview threads resolve --review ${id}\` after addressing each`, - `Run \`thurview publish --review ${id}\` when every open comment is resolved`, - ], - }; + try { + while (Date.now() < deadline) { + const r = await readReview(review.id); + if (!r) + return { + wait: { reason: "review-deleted", id }, + help: ["Stop the loop; the review no longer exists"], + }; + const t = await readThreads(review.id); + const last = t.decisions[t.decisions.length - 1]; + if (r.status === "awaiting-agent-updates") { + const need = t.threads.filter(needsAgent); + return { + wait: { + reason: "awaiting-agent-updates", + id, + status: r.status, + decision: last + ? `${last.decision}${last.body ? `: ${truncate(last.body, 300)}` : ""}` + : "", + }, + count: need.length, + threads: rows(need), + help: [ + `Run \`thurview threads get --review ${id}\` for the full thread`, + `Run \`thurview threads resolve --review ${id}\` after addressing each`, + `Run \`thurview publish --review ${id}\` when every open comment is resolved`, + ], + }; + } + if (r.status === "accepted" || r.status === "closed") + return { + wait: { + reason: r.status, + id, + status: r.status, + decision: last + ? `${last.decision}${last.body ? `: ${truncate(last.body, 300)}` : ""}` + : "", + }, + help: ["The review is complete; report it and stop the loop"], + }; + if (r.dismissed) + return { + wait: { reason: "review-dismissed", id, status: r.status }, + help: ["Stop the loop; the reader dismissed the review"], + }; + const asks = t.threads.filter((x) => needsAgent(x) && x.mode === "ask"); + if (asks.length) + return { + wait: { reason: "question", id, status: r.status }, + count: asks.length, + threads: rows(asks), + help: [ + `Run \`thurview threads reply --review ${id} --body ""\``, + `Run \`thurview wait --review ${id}\` again afterwards`, + ], + }; + await new Promise((res) => setTimeout(res, 700)); } - if (r.status === "accepted" || r.status === "closed") - return { - wait: { - reason: r.status, - id, - status: r.status, - decision: last - ? `${last.decision}${last.body ? `: ${truncate(last.body, 300)}` : ""}` - : "", - }, - help: ["The review is complete; report it and stop the loop"], - }; - if (r.dismissed) - return { - wait: { reason: "review-dismissed", id, status: r.status }, - help: ["Stop the loop; the reader dismissed the review"], - }; - const asks = t.threads.filter((x) => needsAgent(x) && x.mode === "ask"); - if (asks.length) - return { - wait: { reason: "question", id, status: r.status }, - count: asks.length, - threads: rows(asks), - help: [ - `Run \`thurview threads reply --review ${id} --body ""\``, - `Run \`thurview wait --review ${id}\` again afterwards`, - ], - }; - await new Promise((res) => setTimeout(res, 700)); + return { + wait: { reason: "timeout", id, status: review.status, seconds }, + help: [ + `Run \`thurview wait --review ${id}\` again, or report that the reader has not responded`, + ], + }; + } finally { + await listening.stop(); } - return { - wait: { reason: "timeout", id, status: review.status, seconds }, - help: [ - `Run \`thurview wait --review ${id}\` again, or report that the reader has not responded`, - ], - }; }, async graph(args) { diff --git a/src/presence.ts b/src/presence.ts new file mode 100644 index 0000000..c6e0876 --- /dev/null +++ b/src/presence.ts @@ -0,0 +1,47 @@ +import { rm } from "node:fs/promises"; +import { agentFile, readJson, writeJson, now } from "./store.js"; + +/** + * Whether an agent is listening to a review right now, so the reader is told + * the truth about where their question went: an agent in `thurview wait` reads + * it within a second; with nobody attached it is queued until one next checks. + * Presence is never inferred - only a live `wait` writes it. + */ +export interface Presence { + attached: boolean; + /** when the attached agent last checked in, null when none ever has */ + lastSeen: string | null; +} + +/** A heartbeat older than this is a dead `wait`, not a listening agent. */ +const TTL_MS = 15_000; +const BEAT_MS = 3_000; + +export const NOBODY: Presence = { attached: false, lastSeen: null }; + +export async function presenceOf(reviewId: string): Promise { + const rec = await readJson<{ at: string; pid: number }>(agentFile(reviewId)); + if (!rec) return NOBODY; + const at = Date.parse(rec.at); + if (!Number.isFinite(at)) return NOBODY; + return { attached: Date.now() - at < TTL_MS, lastSeen: rec.at }; +} + +/** Announce that this process is waiting on the review until `stop()` is called. */ +export function attach(reviewId: string): { stop: () => Promise } { + let stopped = false; + const beat = () => + writeJson(agentFile(reviewId), { at: now(), pid: process.pid }).catch(() => {}); + void beat(); + const timer = setInterval(() => { + if (!stopped) void beat(); + }, BEAT_MS); + timer.unref(); + return { + stop: async () => { + stopped = true; + clearInterval(timer); + await rm(agentFile(reviewId), { force: true }); + }, + }; +} diff --git a/src/server/server.ts b/src/server/server.ts index e8bc13c..40939d9 100644 --- a/src/server/server.ts +++ b/src/server/server.ts @@ -30,6 +30,7 @@ import { submitReview, deleteThread, } from "../threads.js"; +import { presenceOf } from "../presence.js"; const UI_DIR = join(dirname(fileURLToPath(import.meta.url)), "..", "ui"); @@ -207,12 +208,14 @@ export async function startServer( ...data, threads: threads.threads, decisions: threads.decisions, + agent: await presenceOf(id), }; } if (sub === "events") { throw new HttpError(500, "handled elsewhere"); } + if (sub === "presence") return (await presenceOf(id)) as unknown as Json; if (sub === "revisions") { const out = []; for (let n = 1; n <= review.revision; n++) diff --git a/src/store.ts b/src/store.ts index 10cce4a..890c7b1 100644 --- a/src/store.ts +++ b/src/store.ts @@ -68,6 +68,12 @@ export interface Thread { kind: "question" | "comment"; /** ask: delivered to the agent at once. review: held until the reviewer submits. */ mode: "ask" | "review"; + /** + * `open` means someone still owes something here. A message from the reviewer + * forces it back to `open`, because `needsAgent` - the queue `wait` and + * `threads list --open` read - is false for a resolved thread, and a reply + * nobody is assigned to reaches nobody. See references/lifecycle.md. + */ status: "open" | "resolved"; submitted: boolean; target: ThreadTarget; @@ -109,6 +115,16 @@ export function serverStateFile(): string { return join(home(), "server.json"); } +/** + * Heartbeat of an agent draining this review's threads. It lives outside + * `reviewDir` on purpose: the server watches that directory to push changes to + * the browser, and a file rewritten every few seconds would reload the page + * under the reader's hands. + */ +export function agentFile(id: string): string { + return join(home(), "agents", `${id}.json`); +} + export function now(): string { return new Date().toISOString(); } @@ -164,6 +180,7 @@ export async function listReviews(): Promise { export async function deleteReview(id: string): Promise { await rm(reviewDir(id), { recursive: true, force: true }); + await rm(agentFile(id), { force: true }); } export async function readThreads(id: string): Promise { diff --git a/src/thread-state.ts b/src/thread-state.ts new file mode 100644 index 0000000..b7c5f79 --- /dev/null +++ b/src/thread-state.ts @@ -0,0 +1,29 @@ +import type { Thread } from "./store.js"; +import type { Presence } from "./presence.js"; + +/** Threads the agent must act on: open, submitted, and the last message is from the reviewer. */ +export function needsAgent(th: Thread): boolean { + if (th.status !== "open" || !th.submitted) return false; + const last = th.messages[th.messages.length - 1]; + return !!last && last.role === "reviewer"; +} + +/** + * Where the reader's last message in a thread stands. This is the one rule the + * CLI and the browser both answer from, so the receipt the reader reads and the + * queue the agent drains cannot disagree. + * + * - `held`: written but not submitted; no agent can see it yet. + * - `queued`: submitted, unanswered, nobody listening. It waits for an agent. + * - `listening`: submitted, unanswered, an agent is in `thurview wait` now. + * - `answered`: the agent spoke last. + * - `closed`: resolved; nobody owes anything. + */ +export type Delivery = "held" | "queued" | "listening" | "answered" | "closed"; + +export function deliveryOf(th: Thread, agent: Presence): Delivery { + if (th.status === "resolved") return "closed"; + if (!th.submitted) return "held"; + if (!needsAgent(th)) return "answered"; + return agent.attached ? "listening" : "queued"; +} diff --git a/src/threads.ts b/src/threads.ts index 37893ce..d004f8c 100644 --- a/src/threads.ts +++ b/src/threads.ts @@ -9,6 +9,7 @@ import { type ThreadTarget, type ThreadsFile, } from "./store.js"; +export { needsAgent } from "./thread-state.js"; export async function createThread( reviewId: string, @@ -51,7 +52,14 @@ export async function replyThread( if (!thread) throw new Error(`thread ${threadId} not found`); thread.messages.push({ role, body, at: now() }); thread.updatedAt = now(); - if (role === "reviewer" && thread.mode === "ask") thread.submitted = true; + // A message from the reader is a message that wants an answer. Leaving the + // thread resolved would drop it: `needsAgent` is false there, so `wait` never + // reports it and `threads list --open` never shows it. The reader would be + // writing to nobody, with a Reply button that says otherwise. + if (role === "reviewer") { + thread.status = "open"; + if (thread.mode === "ask") thread.submitted = true; + } await writeThreads(reviewId, t); return thread; } @@ -103,13 +111,6 @@ export async function submitReview( return t; } -/** Threads the agent must act on: open, submitted, and the last message is from the reviewer. */ -export function needsAgent(th: Thread): boolean { - if (th.status !== "open" || !th.submitted) return false; - const last = th.messages[th.messages.length - 1]; - return !!last && last.role === "reviewer"; -} - export function threadSummary(th: Thread) { return { id: th.id, diff --git a/src/ui/api.ts b/src/ui/api.ts index cbfdc5f..ccb02d4 100644 --- a/src/ui/api.ts +++ b/src/ui/api.ts @@ -4,6 +4,7 @@ import type { Coverage } from "../coverage.js"; import type { FileDiff } from "../diff.js"; import type { ChangedFile, Commit } from "../git.js"; import type { SymbolDef } from "../symbols.js"; +import type { Presence } from "../presence.js"; export interface Payload { review: ReviewState; @@ -17,6 +18,8 @@ export interface Payload { theme: { name: string; source?: string; css: string } | null; threads: Thread[]; decisions: Decision[]; + /** whether an agent is listening to this document right now */ + agent: Presence; } export interface FileLines { @@ -51,6 +54,7 @@ export const api = { revisions: (id: string) => j<{ revision: number; at: string; title: string }[]>(`/api/reviews/${id}/revisions`), commits: (id: string) => j(`/api/reviews/${id}/commits`), + presence: (id: string) => j(`/api/reviews/${id}/presence`), diff: (id: string, path: string) => j(`/api/reviews/${id}/diff?path=${encodeURIComponent(path)}`), file: (id: string, path: string, graph: "head" | "base", from?: number, to?: number) => diff --git a/src/ui/app.css b/src/ui/app.css index 53ef9f9..da433ac 100644 --- a/src/ui/app.css +++ b/src/ui/app.css @@ -539,6 +539,24 @@ kbd { .thread-pin.resolved { opacity: 0.55; } +/* Where the reader's last message stands. `queued` is the one that matters: + nobody is listening, and the page says so instead of implying delivery. */ +.receipt { + font-size: 12px; + line-height: 1.45; + color: var(--muted); + margin: 4px 0 2px; + padding-left: 8px; + border-left: 2px solid var(--line); +} +.receipt.listening { + color: var(--ok); + border-left-color: var(--ok); +} +.receipt.queued { + color: var(--warn); + border-left-color: var(--warn); +} .comment-popover { border-radius: var(--radius); position: absolute; diff --git a/src/ui/app.ts b/src/ui/app.ts index e0caaa2..3749c26 100644 --- a/src/ui/app.ts +++ b/src/ui/app.ts @@ -347,6 +347,18 @@ function renderBanner(): void { } } +function pollPresence(): void { + setInterval(async () => { + const cur = state.data; + if (!cur) return; + const agent = await api.presence(state.id).catch(() => cur.agent); + if (agent.attached !== cur.agent.attached) { + state.data = { ...cur, agent }; + emit("threads"); + } + }, 5000); +} + function connectEvents(): void { const es = new EventSource(`/api/reviews/${state.id}/events`); es.onmessage = async (ev) => { @@ -419,6 +431,7 @@ async function reviewPage(id: string): Promise { window.matchMedia(NARROW).addEventListener("change", () => emit("view")); emit("data"); connectEvents(); + pollPresence(); } const m = /^\/review\/([^/]+)/.exec(location.pathname); diff --git a/src/ui/threads.ts b/src/ui/threads.ts index e29359c..d3a82cb 100644 --- a/src/ui/threads.ts +++ b/src/ui/threads.ts @@ -2,38 +2,54 @@ import { api } from "./api.js"; import { h, closePopover, dialog, timeAgo } from "./dom.js"; import { state, emit, describeTarget, navigate, readOnly, kind } from "./state.js"; import type { Thread, ThreadTarget } from "../store.js"; +import type { Presence } from "../presence.js"; +import { deliveryOf, type Delivery } from "../thread-state.js"; export async function reload(): Promise { state.data = await api.review(state.id, state.viewingRevision ?? undefined); emit("threads"); } +// Spelled out rather than imported from presence.js, which reads the store and +// belongs to the server side of the bundle. +const NOBODY: Presence = { attached: false, lastSeen: null }; + +function agent(): Presence { + return state.data?.agent ?? NOBODY; +} + +/** + * What each thread state means to the reader, in the reader's terms. Silence is + * the failure this wording exists to prevent: a question nobody is listening to + * says so, before it is asked and after it is sent. + */ +const RECEIPT: Record = { + held: "Held. It reaches the agent when you submit.", + queued: + "Queued. Nobody is listening right now — it is delivered the next time an agent checks this document.", + listening: "Delivered. An agent is listening and answers here.", + answered: "The agent answered. Reply, or resolve it.", + closed: "Resolved. Writing here reopens it.", +}; + +/** The one line that says whether asking now would reach anybody. */ +function listeningLine(): string { + return agent().attached + ? "An agent is listening to this document now." + : "No agent is listening right now. Anything you send is queued until one checks in."; +} + /** Popover to start a thread on a target. */ export function commentPopover(target: ThreadTarget, quote?: string): HTMLElement { const ta = h("textarea", { placeholder: "Comment, or a question for the agent…" }); - let mode: "review" | "ask" = "review"; - const modeBtns = h("div", { class: "row" }); - const b1 = h("button", { class: "small primary", onclick: () => set("review") }, "Add to review"); - const b2 = h("button", { class: "small", onclick: () => set("ask") }, "Ask now"); - const set = (m: "review" | "ask") => { - mode = m; - b1.className = `small ${m === "review" ? "primary" : ""}`; - b2.className = `small ${m === "ask" ? "primary" : ""}`; - hint.textContent = - m === "review" - ? "Held until you submit the review." - : "Sent to the agent at once; it answers in this thread."; - }; - const hint = h( - "span", - { class: "muted", style: { fontSize: "12px" } }, - "Held until you submit the review.", - ); - modeBtns.append(b1, b2, hint); - const submit = async () => { + const held = kind() === "explainer" ? "Add to my notes" : "Add to the review"; + // One click, one consequence. The old control asked the reader to set a mode + // and then press "Save", and a question saved in the wrong mode reached + // nobody; each button here both chooses and commits, and says which it is. + const submit = async (mode: "ask" | "review") => { const body = ta.value.trim(); if (!body) return; - await api.createThread(state.id, { + const th = await api.createThread(state.id, { kind: mode === "ask" ? "question" : "comment", mode, target: quote ? ({ ...target, quote } as ThreadTarget) : target, @@ -41,9 +57,12 @@ export function commentPopover(target: ThreadTarget, quote?: string): HTMLElemen }); closePopover(); await reload(); + // The receipt: the thread panel opens on what was just written, and its + // card carries the delivery line for the state the server actually recorded. + focusThread(th.id); }; ta.addEventListener("keydown", (e) => { - if ((e.metaKey || e.ctrlKey) && e.key === "Enter") void submit(); + if ((e.metaKey || e.ctrlKey) && e.key === "Enter") void submit("ask"); }); const el = h( "div", @@ -55,12 +74,28 @@ export function commentPopover(target: ThreadTarget, quote?: string): HTMLElemen describeTarget(target), ), ta, - modeBtns, h( "div", - { class: "row", style: { justifyContent: "flex-end" } }, + { class: "row" }, + h( + "button", + { class: "small ok", onclick: () => void submit("ask") }, + "Send to the agent ", + h("kbd", null, "⌘↵"), + ), + h("button", { class: "small", onclick: () => void submit("review") }, held), + h("span", { style: { flex: "1" } }), h("button", { class: "small ghost", onclick: () => closePopover() }, "Cancel"), - h("button", { class: "small ok", onclick: submit }, "Save ", h("kbd", null, "⌘↵")), + ), + h( + "div", + { class: "muted", style: { fontSize: "12px", marginTop: "6px" } }, + h("div", null, `Send to the agent — ${listeningLine()}`), + h( + "div", + null, + `${held} — held until you ${kind() === "explainer" ? "send the document back or finish" : "submit the review"}.`, + ), ), ); setTimeout(() => ta.focus()); @@ -164,6 +199,17 @@ export function renderThreadsPanel(container: HTMLElement): void { const body = h("div", { class: "side-body" }); const draw = () => { body.innerHTML = ""; + // Who, if anyone, is on the other end. Stated once here so the reader knows + // before they write, not only after they have waited. A finished review or + // an older revision takes no new writing, so it says nothing. + if (!readOnly()) + body.appendChild( + h( + "div", + { class: `receipt ${agent().attached ? "listening" : "queued"}` }, + listeningLine(), + ), + ); if (pending.length && !readOnly()) { body.appendChild( h( @@ -198,6 +244,7 @@ export function renderThreadsPanel(container: HTMLElement): void { } function threadCard(t: Thread): HTMLElement { + const delivery = deliveryOf(t, agent()); const ta = h("textarea", { placeholder: "Reply…", rows: 2 }); const send = async () => { const v = ta.value.trim(); @@ -224,6 +271,7 @@ function threadCard(t: Thread): HTMLElement { h("span", { class: "target", onclick: () => goToTarget(t) }, describeTarget(t.target)), h("span", { class: `badge ${t.status === "resolved" ? "ok" : ""}` }, t.status), ), + h("div", { class: `receipt ${delivery}` }, RECEIPT[delivery]), quote ? h("div", { class: "quote" }, `“${quote.length > 160 ? quote.slice(0, 160) + "…" : quote}”`) : null, @@ -263,12 +311,19 @@ function threadCard(t: Thread): HTMLElement { "button", { class: "small", + // An unanswered question is not resolved, it is abandoned. + // Saying so is what stops a reader closing their own + // question and then waiting for an answer to it. + title: + delivery === "answered" + ? "Nothing more is owed here" + : "Stop waiting for an answer; no agent will pick this up", onclick: async () => { await api.resolve(state.id, t.id); await reload(); }, }, - "Resolve", + delivery === "answered" || delivery === "held" ? "Resolve" : "Withdraw", ) : h( "button", diff --git a/test/e2e.test.ts b/test/e2e.test.ts index 1d14eab..f16dd21 100644 --- a/test/e2e.test.ts +++ b/test/e2e.test.ts @@ -1,4 +1,4 @@ -import { describe, it, expect, beforeAll, afterAll } from "vitest"; +import { describe, it, expect, beforeAll, afterAll, beforeEach, afterEach } from "vitest"; import { execFile, spawn } from "node:child_process"; import { decode } from "@toon-format/toon"; import { promisify } from "node:util"; @@ -739,4 +739,91 @@ check expect.arrayContaining(["--review ", "--body "]), ); }); + + describe("a question the reader asks reaches an agent", () => { + let qid = ""; + beforeEach(async () => { + qid = (await cli(["scaffold"]))["review"].uuid as string; + }); + afterEach(async () => { + if (qid) await cli(["delete", "--review", qid]); + }); + + const ask = (body: string) => + post(`/api/reviews/${qid}/threads`, { + kind: "question", + mode: "ask", + target: { type: "document", blockId: "b1" }, + body, + }) as Promise<{ id: string }>; + + it("leaves a submitted Ask-now question open and needing the agent", async () => { + const th = await ask("How is the memory ceiling defined?"); + const got = await cli(["threads", "get", th.id, "--review", qid]); + expect(got["thread"].status).toBe("open"); + expect(got["thread"].needsAgent).toBe(true); + const open = await cli(["threads", "list", "--review", qid, "--open"]); + expect(open["threads"].map((t: Out) => t["id"])).toContain(th.id); + }); + + // The reader's own evidence: they resolved the thread, then wrote again. + // A message nobody is assigned to is a message that reaches nobody. + it("reopens a resolved thread when the reader writes in it again", async () => { + const th = await ask("How is the memory ceiling defined?"); + await post(`/api/reviews/${qid}/threads/${th.id}/resolve`, {}); + await post(`/api/reviews/${qid}/threads/${th.id}/reply`, { body: "Hey" }); + const got = await cli(["threads", "get", th.id, "--review", qid]); + expect(got["messages"].map((m: Out) => m["role"])).toEqual(["reviewer", "reviewer"]); + expect(got["thread"].status).toBe("open"); + expect(got["thread"].needsAgent).toBe(true); + const open = await cli(["threads", "list", "--review", qid, "--open"]); + expect(open["threads"].map((t: Out) => t["id"])).toContain(th.id); + }); + + it("keeps an answered question resolvable by the reader", async () => { + const th = await ask("How is the memory ceiling defined?"); + await cli(["threads", "reply", th.id, "--review", qid, "--body", "It is a heap cap."]); + await post(`/api/reviews/${qid}/threads/${th.id}/resolve`, {}); + const got = await cli(["threads", "get", th.id, "--review", qid]); + expect(got["thread"].status).toBe("resolved"); + expect(got["thread"].needsAgent).toBe(false); + }); + + it("reports whether an agent is listening, and never claims one that is not", async () => { + const idle = await api<{ agent: { attached: boolean; lastSeen: string | null } }>( + `/api/reviews/${qid}`, + ); + expect(idle.agent).toEqual({ attached: false, lastSeen: null }); + const waiting = cli(["wait", "--review", qid, "--timeout", "4"]); + let seen = { attached: false }; + for (let i = 0; i < 40 && !seen.attached; i++) { + await new Promise((r) => setTimeout(r, 100)); + seen = (await api<{ agent: { attached: boolean } }>(`/api/reviews/${qid}`)).agent; + } + expect(seen.attached).toBe(true); + expect((await waiting)["wait"].reason).toBe("timeout"); + const after = await api<{ agent: { attached: boolean } }>(`/api/reviews/${qid}`); + expect(after.agent.attached).toBe(false); + }, 20_000); + + // An open tab polls this endpoint instead of relying on the reviewDir SSE + // watch, which the heartbeat deliberately never fires. It must answer with + // the live fact, not a value cached from the last full payload fetch. + it("answers a standalone presence check without a full payload fetch", async () => { + const idle = await api<{ attached: boolean; lastSeen: string | null }>( + `/api/reviews/${qid}/presence`, + ); + expect(idle).toEqual({ attached: false, lastSeen: null }); + const waiting = cli(["wait", "--review", qid, "--timeout", "4"]); + let seen = { attached: false }; + for (let i = 0; i < 40 && !seen.attached; i++) { + await new Promise((r) => setTimeout(r, 100)); + seen = await api<{ attached: boolean }>(`/api/reviews/${qid}/presence`); + } + expect(seen.attached).toBe(true); + await waiting; + const after = await api<{ attached: boolean }>(`/api/reviews/${qid}/presence`); + expect(after.attached).toBe(false); + }, 20_000); + }); });