diff --git a/apps/connector/src/ending.ts b/apps/connector/src/ending.ts new file mode 100644 index 000000000..6dfd6f208 --- /dev/null +++ b/apps/connector/src/ending.ts @@ -0,0 +1,24 @@ +/** How a build ended, as Chopin says in its answer to the build's last report. */ + +export type BuildEnding = + | { kind: "stopped"; by: string } + | { kind: "blocked"; task?: string; reason: string } + | { kind: "failed"; error: string } + | { kind: "finished"; pullRequests: string[]; tasks: number } + | { kind: "returned"; reason: string } + | { kind: "unfinished" }; + +/** The ending in Chopin's answer, when a server that says one sent it. */ +export function endingOf(answer: unknown): BuildEnding | undefined { + let ending = (answer as { ending?: unknown } | undefined)?.ending as BuildEnding | undefined; + if (!ending || typeof ending !== "object" || typeof ending.kind !== "string") return; + return ending; +} + +/** A blocked build's reason, naming its task when Chopin did. */ +export function blockedText(ending: { task?: string; reason: string }): string { + return `${ending.task ? `“${ending.task}” is blocked: ` : ""}${ending.reason}`.replace( + /[.\s]+$/, + "", + ); +} diff --git a/apps/connector/src/output.ts b/apps/connector/src/output.ts index 403ad30a1..a70c1f313 100644 --- a/apps/connector/src/output.ts +++ b/apps/connector/src/output.ts @@ -2,6 +2,8 @@ * The connector's human-facing terminal lines. Messages go to stdout so terminals don't paint them * as errors; failures go to stderr. ANSI styling only reaches a TTY and honours `NO_COLOR`. */ +import { blockedText, type BuildEnding } from "./ending"; + type Stream = { write(text: string): unknown; isTTY?: boolean }; const CODES = { @@ -147,8 +149,37 @@ export function createOutput( finished() { say(` ${s.green("✓")} Finished`, ""); }, - stopped() { - say(` ${s.dim("✗ Stopped from the connector terminal.")}`, ""); + /** + * A build's agent ended its turn, and Chopin said how the build ended: blocked on a task and + * why, finished with the pull requests it opened, or with tasks left. A server that says + * nothing gets plain Finished. + */ + ended(ending?: BuildEnding) { + if (ending?.kind === "blocked") { + return say(` ${s.dim(`✗ Blocked: ${blockedText(ending)}.`)}`, ""); + } + if (ending?.kind === "unfinished") { + return say( + ` ${s.dim("✗ Ended with tasks left; Build plan in Chopin builds them again.")}`, + "", + ); + } + if (ending?.kind === "returned") { + return say( + ` ${s.dim(`✗ Returned for changes: ${ending.reason.replace(/[.\s]+$/, "")}.`)}`, + "", + ); + } + if (ending?.kind === "finished") { + let tasks = `${ending.tasks} ${ending.tasks === 1 ? "task" : "tasks"}`; + let opened = ending.pullRequests.length ? `, ${ending.pullRequests.join(", ")}` : ""; + return say(` ${s.green("✓")} Finished: ${tasks}${opened}`, ""); + } + say(` ${s.green("✓")} Finished`, ""); + }, + /** Ctrl-C here, or `why`: a person's Stop in Chopin, which names them. */ + stopped(why = "Stopped from the connector terminal.") { + say(` ${s.dim(`✗ ${why}`)}`, ""); }, /** `agent` is the agent's last stderr lines, shown when debugging didn't already show them. */ failed(message: string, agent: string[] = []) { diff --git a/apps/connector/src/run.test.ts b/apps/connector/src/run.test.ts index f805ca426..63ded598b 100644 --- a/apps/connector/src/run.test.ts +++ b/apps/connector/src/run.test.ts @@ -981,6 +981,81 @@ test("a run Chopin has ended stops its agent at once", async () => { }]); }); +/** The run's journal, read before the run's folders are removed. */ +function journalOf(into: { value?: Record }) { + return async ({ state, id }: { state: string; id: string }) => { + into.value = JSON.parse(await readFile(join(state, "runs", `${id}.json`), "utf8")); + }; +} + +test("a build a person stopped in Chopin says who stopped it, as a stop and not a failure", async () => { + let terminal = captured(); + let journal: { value?: Record } = {}; + let { calls } = await run( + {}, + ["--hang"], + name => { + if (name === "renew_implementation_build") { + throw new ConnectorError("build-stopped", "build-stopped: Stopped in Chopin by @ana."); + } + return {}; + }, + undefined, + { output: terminal.output }, + journalOf(journal), + ); + expect(terminal.out()).toContain(" ✗ Stopped in Chopin by @ana.\n"); + expect(terminal.err()).toBe(""); + // Chopin ended the build itself, so there is nothing to report back. + expect(calls.at(-1)?.[0]).toBe("renew_implementation_build"); + expect(journal.value).toMatchObject({ + state: "finished", + outcome: { state: "stopped", reason: "Stopped in Chopin by @ana." }, + }); +}); + +test("a build whose agent blocked a task says it is blocked and why, not Finished", async () => { + let terminal = captured(); + let journal: { value?: Record } = {}; + await run( + { live: true }, + ["--live"], + name => + name === "report_implementation_build" + ? { + accepted: true, + ending: { kind: "blocked", task: "Prove it", reason: "Missing source: the files" }, + } + : {}, + undefined, + { output: terminal.output }, + journalOf(journal), + ); + expect(terminal.out()).toContain( + " ✗ Blocked: “Prove it” is blocked: Missing source: the files.\n", + ); + expect(terminal.out()).not.toContain("Finished"); + expect(journal.value).toMatchObject({ + outcome: { state: "blocked", reason: "“Prove it” is blocked: Missing source: the files" }, + }); +}); + +test("a build that delivered names its pull requests when it finishes", async () => { + let terminal = captured(); + let url = "https://github.com/org/repo/pull/7"; + await run( + { live: true }, + ["--live"], + name => + name === "report_implementation_build" + ? { accepted: true, ending: { kind: "finished", pullRequests: [url], tasks: 1 } } + : {}, + undefined, + { output: terminal.output }, + ); + expect(terminal.out()).toContain(` ✓ Finished: 1 task, ${url}\n`); +}); + test("a build Chopin forgot by restarting stops saying so", async () => { let { calls } = await run({}, ["--hang"], name => { if (name === "renew_implementation_build") { diff --git a/apps/connector/src/run.ts b/apps/connector/src/run.ts index d98d76295..801e15f11 100644 --- a/apps/connector/src/run.ts +++ b/apps/connector/src/run.ts @@ -11,6 +11,7 @@ import { AgentTimedOut, runAgent, timedOut } from "./acp"; import type { AgentInfo } from "./acp"; import { serveLocalBridge } from "./local-bridge"; import { createOutput } from "./output"; +import { blockedText, endingOf } from "./ending"; import type { Output } from "./output"; import { ask, describePermission, permissionMemory } from "./permission"; import type { Sandbox } from "./sandbox"; @@ -86,6 +87,12 @@ export const STOPPED = "Stopped from the connector terminal."; /** A run out of reach says it is still trying at most this often. */ const RETRY_SAID_MS = 10_000; +/** A refusal's message without the code Chopin put in front of it. */ +function refusalMessage(error: unknown): string | undefined { + let message = error instanceof Error ? error.message.replace(/^[a-z][a-z0-9-]*:\s*/, "") : ""; + return message.trim() || undefined; +} + function minutes(ms: number) { let value = Math.round(ms / 60_000); return value >= 1 @@ -142,6 +149,8 @@ export async function runWork( let ended: string | undefined; /** Chopin forgot this connection, so the connector must stop once the run has reported. */ let forgotten = false; + /** A person stopped the build in Chopin, which says who: a stop, not a failure. */ + let stoppedInChopin = false; let now = options.now ?? Date.now; /** When the terminal last said Chopin is out of reach, so retries say so at most every 10 s. */ let saidAt = 0; @@ -211,6 +220,10 @@ export async function runWork( continue; } let code = errorCode(error); + if (code === "build-stopped" && ended === undefined) { + stoppedInChopin = true; + ended = refusalMessage(error) ?? "Stopped in Chopin."; + } // A build claimed before Chopin restarted is one it no longer knows. ended ??= code === "server-restarted" ? "Chopin restarted, so it ended this run." @@ -444,12 +457,19 @@ export async function runWork( agentStop = stop; throw new Error(`Agent stopped: ${stop}`); } - await reports.deliver(implementation ? "report_implementation_build" : "complete_experiment", { - ...identity, - ...(implementation ? { state: "stopped", idempotencyKey: crypto.randomUUID() } : {}), - }); - output.finished(); - outcome = { state: "finished" }; + let answer = await reports.deliver( + implementation ? "report_implementation_build" : "complete_experiment", + { + ...identity, + ...(implementation ? { state: "stopped", idempotencyKey: crypto.randomUUID() } : {}), + }, + ); + // Chopin says how the build ended: blocked on a task, or the pull requests it opened. + let ending = endingOf(answer); + output.ended(ending); + outcome = ending?.kind === "blocked" + ? { state: "blocked", reason: blockedText(ending) } + : { state: "finished" }; } catch (error) { // Ctrl-C reports why before the connector disconnects, so Chopin knows at once. An agent // that cancelled its own turn was interrupted too, unless the connector cancelled it. @@ -469,10 +489,16 @@ export async function runWork( : error instanceof Error ? error.message : "Local work failed"; + // A person's Stop in Chopin ended the build there already: nothing failed, and nothing is + // left to report. + let chopinStop = !stopped && stoppedInChopin && ended === message; outcome = stopped ? { state: "stopped" } + : chopinStop + ? { state: "stopped", reason: message } : { state: "failed", error: redacted(message, secrets()).slice(0, 2000) }; if (stopped) output.stopped(); + else if (chopinStop) output.stopped(message); else { output.failed( redacted(message, secrets()), @@ -481,13 +507,15 @@ export async function runWork( } // After Ctrl-C, or out of reach for the whole grace period, the failure gets one bounded attempt. let send = stopped || reports.expired() ? reports.final : reports.deliver; - await send(implementation ? "report_implementation_build" : "fail_experiment", { - ...identity, - ...(implementation ? { state: "failed", idempotencyKey: crypto.randomUUID() } : {}), - error: stopped || ended || late || !implementation - ? message.slice(0, 2000) - : "Local implementation failed. Inspect the connector terminal.", - }).catch(() => {}); + if (!chopinStop) { + await send(implementation ? "report_implementation_build" : "fail_experiment", { + ...identity, + ...(implementation ? { state: "failed", idempotencyKey: crypto.randomUUID() } : {}), + error: stopped || ended || late || !implementation + ? message.slice(0, 2000) + : "Local implementation failed. Inspect the connector terminal.", + }).catch(() => {}); + } // Retrying cannot reach a server that forgot this connection, so the connector stops too. if (forgotten && !stopped) throw new ConnectorStopped("forgotten", message); } finally { diff --git a/apps/connector/src/workspace.ts b/apps/connector/src/workspace.ts index ddcf8d2a8..a4061ecb4 100644 --- a/apps/connector/src/workspace.ts +++ b/apps/connector/src/workspace.ts @@ -694,7 +694,10 @@ function extraWorktrees(path: string) { /** How a run ended, as its journal records it: its agent finished, failed and why, or was stopped. */ export type RunOutcome = | { state: "finished" } - | { state: "stopped" } + /** `reason` names who stopped it in Chopin; Ctrl-C here has none. */ + | { state: "stopped"; reason?: string } + /** Its agent blocked a task: which one and why. */ + | { state: "blocked"; reason: string } | { state: "failed"; error: string }; /** diff --git a/apps/server/src/agent/scoped-followup-tools.ts b/apps/server/src/agent/scoped-followup-tools.ts index ad6553ad7..e691545ca 100644 --- a/apps/server/src/agent/scoped-followup-tools.ts +++ b/apps/server/src/agent/scoped-followup-tools.ts @@ -1,7 +1,7 @@ import { jsonSchema, tool } from "ai"; import { answer, cardToolSchema } from "./scoped-tool-result"; import { runJobTool } from "./job-scope"; -import { followupIntent, scopeSnapshot } from "../questions/decision-followup"; +import { followupIntent, readScope } from "../questions/decision-followup"; import { type ScopeWrite, writeScope } from "../questions/decision-followup-write"; import type { DocumentRoom } from "./tools"; @@ -27,7 +27,7 @@ export function createFollowupTools() { answer("read_decision_scope", () => room.exclusive(async () => { let job = current(room); - let { indices: _indices, ...snapshot } = scopeSnapshot(room.plan, job.target); + let { indices: _indices, ...snapshot } = readScope(room.plan, job); return snapshot; })), }), diff --git a/apps/server/src/chat/archive-runs.test.ts b/apps/server/src/chat/archive-runs.test.ts new file mode 100644 index 000000000..29a85ce28 --- /dev/null +++ b/apps/server/src/chat/archive-runs.test.ts @@ -0,0 +1,64 @@ +import { expect, test } from "bun:test"; +import * as Chat from "./service"; +import * as Plan from "../plan/service"; +import { hostInputRoom } from "../testing/decisions"; + +import type { Chat as Wire } from "@chopin/protocol"; +import type { Server } from "bun"; +import type { SocketData } from "../wire"; + +let server = { publish() {} } as unknown as Server; + +function card(id: string, status: Wire.Run["status"], waiting = 0): Wire.Run { + return { id, name: id, status, started: 1, updated: 2, stages: [], waiting }; +} + +test("archiving ends every live run card, one waiting on a question included, and saves it", async () => { + // A disconnected Planner's cards come back as it last reported them, live. + let chat = Chat.restore( + [], + [card("plan", "waiting", 1), card("verify", "running"), card("earlier", "finished")], + false, + "down", + ); + expect(chat.runs?.map(run => run.status)).toEqual(["waiting", "running", "finished"]); + let saved: Wire.Run[] | undefined; + await Chat.endRuns({ + chat, + room: "document", + server, + persist: async change => { + saved = change?.().runs; + }, + }); + expect(saved?.map(run => [run.status, run.waiting])).toEqual([ + ["stopped", 0], + ["stopped", 0], + ["finished", 0], + ]); + expect(chat.runs).toEqual(saved); + expect(chat.runs?.some(run => run.disconnected)).toBe(false); +}); + +test("an archived document opens with its runs stopped, even if a Planner hold still claims them", async () => { + let first = await hostInputRoom(); + first.plan.chat.runs = [card("plan", "waiting", 1)]; + await Plan.persist(first.plan, () => first.plan.chat); + await first.close(); + let reopen = { storage: first.storage, channelId: first.channelId, lease: first.lease }; + // A hold the document's archive could not end, as when it came while a deploy handed over. + let backend = { plannerKeepsRuns: () => "down" as const }; + let live = await hostInputRoom(undefined, { ...reopen, backend }); + expect(live.plan.chat.runs?.map(run => [run.status, run.waiting])).toEqual([["waiting", 1]]); + await live.close(); + await first.storage.channels.archive({ id: first.channelId, now: new Date() }); + let archived = await hostInputRoom(undefined, { ...reopen, backend }); + try { + expect(archived.plan.chat.runs?.map(run => [run.status, run.waiting])).toEqual([[ + "stopped", + 0, + ]]); + } finally { + await archived.close(); + } +}); diff --git a/apps/server/src/chat/job-tools.test.ts b/apps/server/src/chat/job-tools.test.ts index da92c0fcd..36f1b06b3 100644 --- a/apps/server/src/chat/job-tools.test.ts +++ b/apps/server/src/chat/job-tools.test.ts @@ -37,7 +37,9 @@ test("two failed own-tool calls end the turn after one possible re-read", async let outcome = await running; expect(outcome.status).toBe("failed"); if (outcome.status === "done") throw new Error("two tool failures completed the job"); - expect(outcome.reason).toContain("failed twice"); + expect(outcome.reason).toBe( + "The Planner's refine_decision failed twice: the document changed while it was being written.", + ); expect(state.aborts()).toBe(1); expect(state.chat.job).toBeUndefined(); }); diff --git a/apps/server/src/chat/job-translate.ts b/apps/server/src/chat/job-translate.ts index 8f1a57993..bbe7940e5 100644 --- a/apps/server/src/chat/job-translate.ts +++ b/apps/server/src/chat/job-translate.ts @@ -50,7 +50,10 @@ export function translateJob(context: Room, part: TextStreamPart): void ) { chat.jobFailures = (chat.jobFailures ?? 0) + 1; if (chat.jobFailures >= 2) { - interrupt(chat.jobFailureCode ?? `The Planner's ${own} failed twice.`); + let why = cause(content); + interrupt( + chat.jobFailureCode ?? `The Planner's ${own} failed twice${why ? `: ${why}` : ""}.`, + ); } } return; @@ -65,3 +68,22 @@ export function translateJob(context: Room, part: TextStreamPart): void return; } } + +/** + * Why a job's own tool refused, in plain words, for the causes Chopin names. Anything else says + * nothing, so a refusal that quotes the tool's arguments never reaches the card. + */ +function cause(content: unknown): string | undefined { + let text = content instanceof Error + ? content.message + : typeof content === "string" + ? content + : ""; + if (/saved answers changed/.test(text)) { + return "another answer to it changed while it was being written"; + } + if (/orphaned|ambiguous/.test(text)) return "the passage it rewrites was removed or changed"; + if (/visuals-required/.test(text)) return "the passage needs a diagram or table it did not add"; + if (/\bstale\b/.test(text)) return "the document changed while it was being written"; + if (/implementation is active/.test(text)) return "a build is running"; +} diff --git a/apps/server/src/chat/service.ts b/apps/server/src/chat/service.ts index aea51f88c..e1d14eb12 100644 --- a/apps/server/src/chat/service.ts +++ b/apps/server/src/chat/service.ts @@ -349,6 +349,16 @@ export type Chat = { jobFailureCode?: "source-shape"; jobCalls?: Map; jobToolNames?: ReadonlySet; + /** What a decision-followup job's turn last read of its scope (see `readScope`). */ + jobScopeRead?: { + job: ConversationPlan.Job; + turn: Wire.Turn | undefined; + revision: number; + scope: string; + answersHash: string; + source: string; + context: string; + }; /** The job taken from the waiting turns while its handoff save is pending. */ handoffJob?: JobTurn; /** The background job whose turn holds the Chat, until the next turn starts or the Chat is idle. */ @@ -2208,6 +2218,24 @@ export async function restoredRuns( state(chat, server, room); } +/** + * The document was archived: every run its Planner held ends with it, whether the Planner was + * attached, disconnected with its work kept, or between two processes of a deploy, so no card is + * left running or waiting on a question nobody can answer now. Saved with the document. + */ +export async function endRuns( + context: Pick, +): Promise { + let { chat, room, server } = context; + if (!chat.runs?.length) return; + let at = now(); + let next = chat.runs.map(run => ENDED_RUN[run.status] ? run : stopped(run, at)); + if (JSON.stringify(next) === JSON.stringify(chat.runs)) return; + await context.persist(() => ({ entries: chat.entries, runs: next })); + chat.runs = next; + state(chat, server, room); +} + /** Run cards restored with the document; no session survives a reload of the room, so live ones were stopped. */ export function restoreRuns(runs: Wire.Run[] | undefined): Wire.Run[] | undefined { return runs?.length ? mergeRuns(runs, undefined, now()) : undefined; diff --git a/apps/server/src/conversation-plan/prose-job.test.ts b/apps/server/src/conversation-plan/prose-job.test.ts index 28467cb35..3dd29f82d 100644 --- a/apps/server/src/conversation-plan/prose-job.test.ts +++ b/apps/server/src/conversation-plan/prose-job.test.ts @@ -172,6 +172,37 @@ test("writeup projects only the current saved generation's unfinished prose job" expect(writeup(plan([job("failed")]), record({ origin: "planner" }))).toBeUndefined(); }); +test("a failed write-up says why in plain words, never naming a tool", () => { + let id = `prose:${CARD}:decided:${CARD}:1`; + let failed = (reason: string) => ({ + conversationPlanJobs: [{ + id, + kind: "prose" as const, + target: CARD, + trigger: `decided:${CARD}:1`, + status: "failed" as const, + attempts: 1, + reason, + at: "2026-10-07T00:00:00.000Z", + }], + conversationPlanPendingEffects: [], + }); + expect(writeup( + failed( + "The Planner's write_decision_prose failed twice: the document changed while it was being written.", + ), + record(), + )).toEqual({ + status: "failed", + job: id, + reason: "Writing it failed twice: the document changed while it was being written.", + }); + expect(writeup(failed("The Planner ended without calling write_decision_prose."), record())) + .toEqual({ status: "failed", job: id, reason: "chopin finished without writing it." }); + expect(writeup(failed("The machine disconnected during the turn."), record())) + .toEqual({ status: "failed", job: id, reason: "The machine disconnected during the turn." }); +}); + test("a conversation decision's write-up is unchanged and keeps its thread", () => { let effects = appendProseEffect([], [], record()); expect(effects).toHaveLength(1); diff --git a/apps/server/src/conversation-plan/prose-job.ts b/apps/server/src/conversation-plan/prose-job.ts index 66635aefc..261ed6154 100644 --- a/apps/server/src/conversation-plan/prose-job.ts +++ b/apps/server/src/conversation-plan/prose-job.ts @@ -9,6 +9,7 @@ import { markKey } from "@chopin/question"; import type { Plan } from "../plan/service"; import type { Record as CardRecord } from "../questions/records"; import { followupIntent } from "../questions/decision-followup"; +import { PLANNER_NAME } from "@chopin/protocol/address"; export function proseIntent(record: CardRecord): JobIntent | undefined { if ( @@ -52,7 +53,7 @@ export function settleIntent(record: CardRecord): JobIntent | undefined { export function writeup( plan: Pick, record: CardRecord, -): { status: "writing" | "failed"; job: string } | undefined { +): { status: "writing" | "failed"; job: string; reason?: string } | undefined { let intent = proseIntent(record) ?? followupIntent(record); if (!intent) return; if ( @@ -61,7 +62,10 @@ export function writeup( ) return; let id = jobId(intent); let job = plan.conversationPlanJobs.find(item => item.id === id); - if (job?.status === "failed") return { status: "failed", job: id }; + if (job?.status === "failed") { + let reason = job.reason && failedBecause(job.reason); + return { status: "failed", job: id, ...(reason ? { reason } : {}) }; + } if (job?.status === "pending" || job?.status === "running") return { status: "writing", job: id }; if (job) return; // Save commits the job effect before the queue accepts it. @@ -72,6 +76,16 @@ export function writeup( return queued ? { status: "writing", job: id } : undefined; } +/** A failed write-up's reason as its card says it: in plain words, naming no tool. */ +function failedBecause(reason: string): string { + let twice = /^The Planner's \w+ failed twice(?:: (.+?))?\.$/.exec(reason); + if (twice) return `Writing it failed twice${twice[1] ? `: ${twice[1]}` : ""}.`; + if (/^The Planner ended without calling \w+\.$/.test(reason)) { + return `${PLANNER_NAME} finished without writing it.`; + } + return reason; +} + /** Save adds this to the same candidate as its answered record and mirror action. */ export function appendProseEffect( pending: readonly Effect[], diff --git a/apps/server/src/main.ts b/apps/server/src/main.ts index 4ab93f5b8..228b0f820 100644 --- a/apps/server/src/main.ts +++ b/apps/server/src/main.ts @@ -2524,6 +2524,14 @@ async function archiveChannelLocked(channelId: string, now: Date) { await stopped; await conversationRuntime.interrupt(active); await Service.drain(active); + // Its runs end with it, whatever its Planner's link is doing: attached, disconnected + // with its work kept, or between two processes of a deploy, which report nothing. + await Chat.endRuns({ + chat: active.chat, + room: channelId, + server, + persist: chat => Service.persist(active, chat), + }); await Service.persist(active); } return storage.channels.archive({ id: channelId, now }); diff --git a/apps/server/src/plan/service.ts b/apps/server/src/plan/service.ts index b33c2e05c..c978f7ee8 100644 --- a/apps/server/src/plan/service.ts +++ b/apps/server/src/plan/service.ts @@ -1605,7 +1605,8 @@ export async function open( sidecar.transcript, sidecar.workflowRuns, sidecar.initiativePaused, - backend.plannerKeepsRuns?.(id), + // An archived document's Planner ended with it: its runs come back stopped. + loaded.channel.archivedAt ? undefined : backend.plannerKeepsRuns?.(id), ), conversationPlan: sidecar.conversationPlan ?? restoreConversationPlan(undefined), conversationPlanRetries: sidecar.conversationPlanRetries ?? [], diff --git a/apps/server/src/questions/decision-followup-concurrency.test.ts b/apps/server/src/questions/decision-followup-concurrency.test.ts index f0129a118..cee85ec45 100644 --- a/apps/server/src/questions/decision-followup-concurrency.test.ts +++ b/apps/server/src/questions/decision-followup-concurrency.test.ts @@ -3,7 +3,7 @@ import * as Edit from "../plan/edit"; import * as Room from "../plan/room"; import * as Service from "../plan/service"; import { fixture, restart, running, scopeRoom } from "./decision-followup.test-fixtures"; -import { scopeSnapshot } from "./decision-followup"; +import { readScope, scopeSnapshot } from "./decision-followup"; import { writeScope } from "./decision-followup-write"; import * as Questions from "./service"; @@ -109,3 +109,52 @@ test.each(["passage", "answer", "section"])( expect(context.plan.records.get(id)!.followUp!.appliedGeneration).toBeUndefined(); }, ); + +test("a write-up lands though the document moved on elsewhere after its scope was read", async () => { + // While a run publishes its plan and other decisions are written up, the document's revision + // moves between the job's read and its write, but its passage and answers stay the same. + let context = await fixture(); + let id = context.ids[0]!; + await context.save(id, "Keep offline support."); + let job = running(context); + let room = scopeRoom(context); + let read = readScope(context.plan, job); + await room.exclusive(async () => { + let before = Room.project(context.plan.document); + let outcome = Edit.apply(context.plan, context.plan.revision, [ + { op: "insert_root", source: "A section the run published meanwhile." }, + ]); + if (!outcome.ok || !outcome.mutation) throw new Error("expected the unrelated edit"); + Questions.rebase(context.plan, before); + await Service.publish(context.plan, context.server, context.plan.id, outcome.mutation); + }); + expect(context.plan.revision).toBeGreaterThan(read.revision); + await expect(writeScope(room, job, context.plan.chat.turn, { + id, + revision: read.revision, + answersHash: read.answersHash, + source: "Offline support is required.", + })).resolves.toMatchObject({ output: { id, replayed: false } }); + expect(Room.project(context.plan.document)).toContain("Offline support is required."); + expect(Room.project(context.plan.document)).toContain("A section the run published meanwhile."); + // Its passage changing after the read still refuses, as does a revision it never read. + let fresh = await fixture(); + await fresh.save(fresh.ids[0]!, "Keep offline support."); + let next = running(fresh); + let seen = readScope(fresh.plan, next); + await scopeRoom(fresh).exclusive(async () => { + let outcome = Edit.apply(fresh.plan, fresh.plan.revision, [{ + op: "replace", + index: 0, + source: "A person rewrote the passage after it was read.", + }]); + if (!outcome.ok || !outcome.mutation) throw new Error("expected the human edit"); + await Service.publish(fresh.plan, fresh.server, fresh.plan.id, outcome.mutation); + }); + await expect(writeScope(scopeRoom(fresh), next, fresh.plan.chat.turn, { + id: fresh.ids[0]!, + revision: seen.revision, + answersHash: seen.answersHash, + source: "An outdated scoped write.", + })).rejects.toThrow(/stale|orphaned/); +}); diff --git a/apps/server/src/questions/decision-followup-write.ts b/apps/server/src/questions/decision-followup-write.ts index a7f194c39..9fb7e8272 100644 --- a/apps/server/src/questions/decision-followup-write.ts +++ b/apps/server/src/questions/decision-followup-write.ts @@ -42,11 +42,21 @@ export async function writeScope( if (record.followUp!.appliedGeneration === decisionGeneration(record)) { return { replay: true as const }; } - if (args.revision !== plan.revision) throw new Error("stale; read_decision_scope again"); let snapshot = scopeSnapshot(plan, args.id); if (snapshot.answersHash !== args.answersHash) { throw new Error("saved answers changed; read_decision_scope again"); } + // The document may have moved on elsewhere since the read, as a run publishing its plan or + // another decision's write-up does: the write still lands if this turn read this passage, + // under these headings, at the revision it names. + let read = plan.chat.jobScopeRead; + if ( + args.revision !== plan.revision && !( + read?.job === job && read.turn === turn && read.revision === args.revision + && read.answersHash === args.answersHash && read.scope === snapshot.scope + && read.source === snapshot.source && read.context === snapshot.context + ) + ) throw new Error("stale; read_decision_scope again"); if (typeof args.source !== "string" || args.source.length > 50_000) { throw new Error("scope source exceeds 50000 characters"); } @@ -64,7 +74,7 @@ export async function writeScope( { op: "replace", index: snapshot.indices[0]!, source: args.source }, ...snapshot.indices.slice(1).map(index => ({ op: "delete" as const, index })), ]; - let prepared = Edit.prepare(plan, args.revision, operations); + let prepared = Edit.prepare(plan, plan.revision, operations); if (!prepared.ok) { throw new Error(prepared.reason === "invalid" ? prepared.message : "stale scope"); } diff --git a/apps/server/src/questions/decision-followup.ts b/apps/server/src/questions/decision-followup.ts index b968ac13c..797add2a9 100644 --- a/apps/server/src/questions/decision-followup.ts +++ b/apps/server/src/questions/decision-followup.ts @@ -8,6 +8,7 @@ import * as Room from "../plan/room"; import type { Plan } from "../plan/service"; import type { Record } from "./records"; import type { Heading } from "mdast"; +import type { ConversationPlan } from "@chopin/protocol"; export function followupIntent(record: Record): JobIntent | undefined { if ( @@ -99,6 +100,25 @@ export function scopeSnapshot(plan: Plan, id: string) { return { id, scope, revision: plan.revision, answersHash, source, context, decisions, indices }; } +/** + * A decision-followup job reads its scope: the snapshot it is given, which its turn keeps (on the + * Chat, with the job and turn it was read for) so a write made after the document moved on + * elsewhere still lands when its own passage, section and answers are the ones it read. + */ +export function readScope(plan: Plan, job: ConversationPlan.Job) { + let snapshot = scopeSnapshot(plan, job.target); + plan.chat.jobScopeRead = { + job, + turn: plan.chat.turn, + revision: snapshot.revision, + scope: snapshot.scope, + answersHash: snapshot.answersHash, + source: snapshot.source, + context: snapshot.context, + }; + return snapshot; +} + export function followupPrompt(id: string): string { return [ "[Background job: decision-followup]", diff --git a/apps/server/src/tasks/build-ending.ts b/apps/server/src/tasks/build-ending.ts new file mode 100644 index 000000000..97b745858 --- /dev/null +++ b/apps/server/src/tasks/build-ending.ts @@ -0,0 +1,84 @@ +/** + * How a first build ended, said the same way wherever it shows: the run record, Chat's end line, + * the Build tab and the machine's terminal. A person stopped it, its agent blocked a task, it + * failed, or it finished and opened its pull requests. + */ + +import { historyFor, progressFor } from "./lifecycle"; + +import type { BuildRequest } from "@chopin/protocol/implementation"; +import type { Graph } from "./graphs"; +import type { LifecycleState } from "./lifecycle"; + +export type BuildEnding = + /** A person pressed Stop in Chopin; `by` is their login. */ + | { kind: "stopped"; by: string } + /** The agent blocked a task and ended its turn; `task` is its title when it is known. */ + | { kind: "blocked"; task?: string; reason: string } + | { kind: "failed"; error: string } + | { kind: "finished"; pullRequests: string[]; tasks: number } + /** The agent or a reviewer returned the run for changes. */ + | { kind: "returned"; reason: string } + /** The agent ended its turn with tasks left and said nothing about them. */ + | { kind: "unfinished" }; + +const UNFINISHED = "The build stopped before it finished."; + +/** What a person's Stop records as the build's error and its run's reason. */ +export function stoppedIn(login: string): string { + return `Stopped in Chopin by @${login}.`; +} + +/** The first task the agent left blocked, with its title. */ +export function blockedTask( + graph: Graph, + graphVersion: number, + tasks: readonly { id: string; state: string; blocker?: string }[], +): { task?: string; reason: string } | undefined { + let blocked = tasks.find(task => task.state === "blocked" && task.blocker?.trim()); + if (!blocked?.blocker) return; + let title = graph.versions.find(version => version.number === graphVersion)?.definition.tasks + .find(task => task.id === blocked.id)?.title; + return { ...(title ? { task: title } : {}), reason: blocked.blocker.trim() }; +} + +/** + * Why a build's run is returned once the build has ended: who stopped it, the task its agent + * blocked, or the build's error. `state` holds the run still claimed. + */ +export function endedReason(state: LifecycleState, build: BuildRequest): string { + if (build.stoppedBy) return stoppedIn(build.stoppedBy); + let run = state.execution; + let tasks = run ? progressFor(state.graph, state.lifecycle, run)?.tasks ?? [] : []; + // Only the agent's own end leaves a blocker standing as the reason; a failure says what failed. + let blocked = build.state === "stopped" && run + ? blockedTask(state.graph, run.graphVersion, tasks) + : undefined; + if (blocked) return `${blocked.task ? `“${blocked.task}” is blocked: ` : ""}${blocked.reason}`; + return build.error ?? UNFINISHED; +} + +/** How `build` ended, once it has; a rebuild or a build still running has no ending here. */ +export function buildEnding(state: LifecycleState, build: BuildRequest): BuildEnding | undefined { + if (build.kind === "rebuild" || !["stopped", "failed"].includes(build.state)) return; + if (build.stoppedBy) return { kind: "stopped", by: build.stoppedBy }; + let ended = historyFor(state.graph, state.lifecycle).findLast(item => item.run.id === build.id); + if (ended && (ended.outcome.kind === "implemented" || ended.outcome.kind === "delivered")) { + let done = ended.progress.tasks.filter(task => task.state === "completed"); + let pullRequests = [ + ...new Set(done.flatMap(task => task.pullRequest ? [task.pullRequest.url] : [])), + ]; + return { kind: "finished", pullRequests, tasks: done.length }; + } + if (ended && build.state === "stopped") { + let blocked = blockedTask(state.graph, ended.run.graphVersion, ended.progress.tasks); + if (blocked) return { kind: "blocked", ...blocked }; + } + if (build.state === "failed") { + return { kind: "failed", error: build.error ?? "The build failed." }; + } + if (ended?.outcome.kind === "revision_requested" && ended.outcome.reason !== UNFINISHED) { + return { kind: "returned", reason: ended.outcome.reason }; + } + return { kind: "unfinished" }; +} diff --git a/apps/server/src/tasks/build-notices.ts b/apps/server/src/tasks/build-notices.ts index 6f1046899..06f541227 100644 --- a/apps/server/src/tasks/build-notices.ts +++ b/apps/server/src/tasks/build-notices.ts @@ -5,11 +5,15 @@ */ import { implementationLifecycle } from "./lifecycle"; +import { blockedTask } from "./build-ending"; import type { Plan } from "../plan/service"; export type BuildNotice = { id: string; text: string }; +/** A person's Ctrl-C at the machine's terminal: a stop, not a failure. */ +const STOPPED_HERE = "Stopped from the connector terminal."; + type State = Pick; /** `#12` linked to its pull request; any other link as itself. */ @@ -95,17 +99,22 @@ export function buildNotices(state: State): BuildNotice[] { ? `Build finished: ${count}, ${links(opened)}.` : `Build finished: ${count}, no code change needed.`, }); + } else if (ended && build.stoppedBy) { + notices.push({ id, text: `Build stopped by @${build.stoppedBy}.` }); } else if (ended && ended.outcome.kind === "revision_requested") { // A blocked task says why better than the release that followed it. - let blocked = ended.progress.tasks.find(task => task.state === "blocked" && task.blocker); - let title = blocked && titles(ended.run.graphVersion).get(blocked.id); + let blocked = build.state === "stopped" + ? blockedTask(graph, ended.run.graphVersion, ended.progress.tasks) + : undefined; let reason = sentence(ended.outcome.reason); notices.push({ id, - text: blocked?.blocker - ? `Build stopped: ${title ? `“${sentence(title)}” is blocked: ` : ""}${ - sentence(blocked.blocker) + text: blocked + ? `Build blocked${blocked.task ? ` on “${sentence(blocked.task)}”` : ""}: ${ + sentence(blocked.reason) }.` + : build.state === "failed" && build.error && build.error !== STOPPED_HERE + ? `Build failed: ${sentence(build.error)}.` : /^the build stopped before it finished$/i.test(reason) ? "Build stopped before it finished." : `Build stopped: ${reason}.`, diff --git a/apps/server/src/tasks/builds.ts b/apps/server/src/tasks/builds.ts index 615022ce2..57be2021d 100644 --- a/apps/server/src/tasks/builds.ts +++ b/apps/server/src/tasks/builds.ts @@ -14,6 +14,7 @@ import { } from "./lifecycle"; import { announceImplementation } from "./notifications"; import { planSourceFor, planSourceSchema } from "./plan-source"; +import { endedReason } from "./build-ending"; import { drain, exclusive, persistExclusive, source } from "../plan/service"; import { broadcast } from "../wire"; import type { Plan } from "../plan/service"; @@ -57,6 +58,8 @@ let buildSchema = z.object({ session: z.string().min(1).max(128).optional(), error: z.string().max(2000).optional(), stoppedAtRevision: z.number().int().nonnegative().optional(), + /** The login of the person who pressed Stop in Chopin. */ + stoppedBy: z.string().min(1).max(128).optional(), activity: z.object({ text: z.string().min(1).max(2000), at: z.string().datetime(), @@ -102,7 +105,7 @@ export function releaseEndedRun( let result = transition(state, { kind: "request_revision", runId: run.id, - reason: build.error ?? "The build stopped before it finished.", + reason: endedReason(state, build), idempotencyKey: `${run.id}:ended`, }); return result.kind === "accepted" ? result.state : state; @@ -634,7 +637,8 @@ export function reportBuild( expiresAt?: number; reportKey?: string; progress?: string; - stoppedByUser?: true; + /** A person pressed Stop in Chopin: their login. */ + stoppedBy?: string; }, ): Promise { return exclusive(plan, async () => { @@ -652,13 +656,13 @@ export function reportBuild( ) { throw new Error("build report is out of order"); } - let { progress: reported, stoppedByUser, ...fields } = report; + let { progress: reported, ...fields } = report; // Whatever a connector sends, the progress line never shows a raw command or a credential. let progress = reported === undefined ? undefined : activityText(reported); let next = buildSchema.parse({ ...current, ...fields, - ...(stoppedByUser ? { stoppedAtRevision: plan.revision } : {}), + ...(report.stoppedBy ? { stoppedAtRevision: plan.revision } : {}), // A build's age is counted from when it finished, not from when it was asked for. ...(current.state !== report.state && (report.state === "stopped" || report.state === "failed") diff --git a/apps/server/src/tasks/connector.ts b/apps/server/src/tasks/connector.ts index 9ed4f9f5d..d095649e3 100644 --- a/apps/server/src/tasks/connector.ts +++ b/apps/server/src/tasks/connector.ts @@ -3,6 +3,7 @@ import { APP_TOKEN_PROTOCOL, RESUME_PROTOCOL, serves } from "../experiments/conn import type { Connection, Connections, DurableRun, Grant } from "../experiments/connections"; import { ExperimentError } from "@chopin/experiment"; import { fail } from "../experiments/service"; +import { buildEnding, stoppedIn } from "./build-ending"; import { LIFECYCLE_TOOLS, lifecycleCall } from "../mcp/lifecycle"; import { exclusive, source } from "../plan/service"; import type { Plan } from "../plan/service"; @@ -467,6 +468,21 @@ export function implementationConnector( } } let build = plan.builds.at(-1); + // A person stopped it in Chopin: its machine hears who did, not that it was refused. + if ( + build && !grant.run && build.id === args.id && build.stoppedBy !== undefined + && build.state === "failed" && runner(build) === connection.owner + && (build.machineId !== undefined + ? build.machineId === connection.machineId + : build.connectionId === connection.id) + && ["renew_implementation_build", "report_implementation_build"].includes(name) + ) { + console.info( + `chopin: told ${caller} that build ${build.id} of document ${plan.id} was stopped ` + + "in Chopin", + ); + fail("build-stopped", stoppedIn(build.stoppedBy)); + } // Chopin restarted since the build's connection opened. Its machine may carry on with a // resumable build for a while; any other is over. let restarted = !!build && !grant.run && build.id === args.id @@ -607,7 +623,13 @@ export function implementationConnector( name === "report_implementation_build" && args.idempotencyKey !== undefined && build.reportKey === args.idempotencyKey ) { - return { accepted: true }; + let ending = args.state !== "running" && plan.graph + ? buildEnding( + { graph: plan.graph, execution: plan.execution, lifecycle: plan.lifecycle }, + build, + ) + : undefined; + return { accepted: true, ...(ending ? { ending } : {}) }; } if (!["starting", "running"].includes(build.state) || build.expiresAt <= Date.now()) { refuse("build-inactive", build); @@ -660,6 +682,14 @@ export function implementationConnector( if (claim.kind !== "started") fail("build-claim-refused"); } await reportBuild(plan, connection.owner, connection.id, build.id, report); + // The machine's terminal and journal say how the build ended, as Chopin does. + let ending = report.state !== "running" && plan.graph + ? buildEnding( + { graph: plan.graph, execution: plan.execution, lifecycle: plan.lifecycle }, + plan.builds.at(-1)!, + ) + : undefined; + if (ending) return { accepted: true, ending }; } else fail("tool-forbidden"); return { accepted: true }; }); diff --git a/apps/server/src/tasks/routes.test.ts b/apps/server/src/tasks/routes.test.ts index 26d6a9101..9d26c17ff 100644 --- a/apps/server/src/tasks/routes.test.ts +++ b/apps/server/src/tasks/routes.test.ts @@ -11,6 +11,7 @@ import { implementationGraphs } from "./plan-graphs"; import { registerImplementationRoutes } from "./routes"; import { queueRebuild, requestBuild } from "./builds"; import { implementationStatus } from "./notifications"; +import { buildNotices } from "./build-notices"; import { LiveSyncCoordinator } from "./live-sync"; import { serves } from "../experiments/connections"; import type { GitHub } from "../github/client"; @@ -424,6 +425,96 @@ test("the build owner can stop from the browser without losing reported pull req await Plan.close(context.plan); }); +test("a person's Stop names who stopped the build to its machine, its run record and Chat", async () => { + let context = await setup(); + try { + let { connection, built } = await running(context); + expect((await context.call(`${context.path}/stop`, { id: built.id })).status).toBe(200); + let snapshot = await (await context.call(context.path)).json(); + expect(snapshot.build).toMatchObject({ state: "failed", stoppedBy: "test" }); + expect(snapshot.lifecycle.history.at(-1).outcome).toEqual({ + kind: "revision_requested", + reason: "Stopped in Chopin by @test.", + }); + // The machine's next heartbeat hears that a person stopped it, not that it was refused. + let refused = await tool(context, connection.token, "renew_implementation_build", { + id: built.id, + documentId: context.plan.id, + }); + expect(refused.isError).toBe(true); + expect(refused.content[0].text).toBe("build-stopped: Stopped in Chopin by @test."); + expect(buildNotices(context.plan).at(-1)?.text).toBe("Build stopped by @test."); + } finally { + await Plan.close(context.plan); + } +}); + +test("a build whose agent blocked a task ends blocked, with its reason, wherever it shows", async () => { + let context = await setup(); + try { + let { connection, built, claim } = await running(context); + await tool(context, claim.runToken, "start_task", { taskId: "first", idempotencyKey: "start" }); + await tool(context, claim.runToken, "block_task", { + taskId: "first", + idempotencyKey: "block", + reason: "Missing source: the formal files are not in the repository", + }); + let ended = await tool(context, connection.token, "report_implementation_build", { + id: built.id, + state: "stopped", + }); + expect(ended).toEqual({ + accepted: true, + ending: { + kind: "blocked", + task: "First", + reason: "Missing source: the formal files are not in the repository", + }, + }); + let snapshot = await (await context.call(context.path)).json(); + expect(snapshot.lifecycle.history.at(-1).outcome).toEqual({ + kind: "revision_requested", + reason: "“First” is blocked: Missing source: the formal files are not in the repository", + }); + expect(buildNotices(context.plan).at(-1)?.text).toBe( + "Build blocked on “First”: Missing source: the formal files are not in the repository.", + ); + } finally { + await Plan.close(context.plan); + } +}); + +test("a build that delivered tells its machine which pull requests it opened", async () => { + let context = await setup(); + try { + let { connection, built, claim } = await running(context); + let url = "https://github.com/owner/repository/pull/7"; + for ( + let [name, args] of [ + ["start_task", { taskId: "first", idempotencyKey: "start" }], + ["report_pr", { taskId: "first", idempotencyKey: "pr", url, state: "open" }], + ["complete_task", { taskId: "first", idempotencyKey: "done", summary: "Launched" }], + ["report_verification", { + idempotencyKey: "verified", + passed: true, + summary: "Reviewed.", + reviewerMethod: "Ran the suite.", + evidence: [{ taskId: "first", evidence: ["Suite passed."] }], + tasksNeedingWork: [], + }], + ] as const + ) expect((await tool(context, claim.runToken, name, args)).isError).toBeUndefined(); + expect( + await tool(context, connection.token, "report_implementation_build", { + id: built.id, + state: "stopped", + }), + ).toEqual({ accepted: true, ending: { kind: "finished", pullRequests: [url], tasks: 1 } }); + } finally { + await Plan.close(context.plan); + } +}); + test("a stale Stop rejects a naturally ended build instead of claiming cancellation", async () => { for (let state of ["failed", "stopped"] as const) { let context = await setup(); @@ -1068,7 +1159,7 @@ test.each([false, true])( ...args, state: "stopped", }), - ).toEqual({ accepted: true }); + ).toEqual({ accepted: true, ending: { kind: "unfinished" } }); expect(context.plan.builds.at(-1)).toMatchObject({ state: "stopped" }); } finally { await Plan.close(context.plan); @@ -1197,7 +1288,7 @@ test("a build carries on across a server restart once its machine comes back", a ...hint, state: "stopped", }), - ).toEqual({ accepted: true }); + ).toEqual({ accepted: true, ending: { kind: "unfinished" } }); expect(context.plan.builds.at(-1)).toMatchObject({ state: "stopped" }); expect(context.plan.builds.at(-1)!.error).toBeUndefined(); // Its run credential ended with the build. @@ -1244,7 +1335,8 @@ test("a build that ended before a restart stays ended", async () => { id: built.id, documentId: context.plan.id, }); - expect(renewed.content[0].text).toMatch(/^build-inactive:/); + // Its machine hears who stopped it, after the restart too. + expect(renewed.content[0].text).toBe("build-stopped: Stopped in Chopin by @test."); let agent = await tool(context, claim.runToken, "start_task", { taskId: "first", idempotencyKey: "start", @@ -1412,6 +1504,7 @@ test("a build report retried after its answer was lost lands once, and an invali await tool(context, connection.token, "report_implementation_build", stopped); expect(await tool(context, connection.token, "report_implementation_build", stopped)).toEqual({ accepted: true, + ending: { kind: "unfinished" }, }); expect(context.plan.builds.at(-1)!.state).toBe("stopped"); // The run's earlier report no longer describes the build, so its key no longer passes. diff --git a/apps/server/src/tasks/routes.ts b/apps/server/src/tasks/routes.ts index 53e929833..3d4767ea2 100644 --- a/apps/server/src/tasks/routes.ts +++ b/apps/server/src/tasks/routes.ts @@ -533,7 +533,7 @@ export function registerImplementationRoutes(router: Router, auth: HostedAuth, o await reportBuild(plan, runner(build), build.connectionId, build.id, { state: "failed", error: "Stopped in Chopin.", - stoppedByUser: true, + stoppedBy: session.user.login, }); options.connections.release(build.connectionId, id); } catch (error) { diff --git a/apps/web/src/build-model.test.ts b/apps/web/src/build-model.test.ts index 53325b82e..bc0cde476 100644 --- a/apps/web/src/build-model.test.ts +++ b/apps/web/src/build-model.test.ts @@ -17,6 +17,7 @@ import { builtDetail, draftRefusalCopy, elapsed, + endedCard, endedReason, firstBuildStep, linkParts, @@ -116,6 +117,72 @@ function run(outcome: History["outcome"], pullRequests = 0): History { }; } +describe("endedCard", () => { + let returned = ( + ended: BuildRequest, + reason: string, + tasks: History["progress"]["tasks"] = [], + ) => + endedCard(snapshot({ + build: ended, + lifecycle: { + execution: { state: "idle" }, + history: [{ + ...run({ kind: "revision_requested", reason }), + progress: { tasks, events: [] }, + }], + }, + })); + + it("says who stopped a build in Chopin, and when", () => { + let at = "2026-10-10T17:54:55.000Z"; + let time = new Date(at).toLocaleTimeString([], { hour: "numeric", minute: "2-digit" }); + expect(returned( + { ...build("failed"), error: "Stopped in Chopin.", stoppedBy: "ana", finishedAt: at }, + "Stopped in Chopin by @ana.", + )).toEqual({ title: "Stopped by @ana", detail: `At ${time}. Build plan builds it again.` }); + }); + + it("says its agent blocked a task, leading with the agent's reason", () => { + expect(returned(build("stopped"), "“First” is blocked: Missing source: the files", [{ + id: "a", + state: "blocked", + blocker: "Missing source: the files", + }])).toEqual({ + title: "Blocked", + detail: "Missing source: the files. Build plan builds it again.", + }); + }); + + it("says a build failed, and why", () => { + expect(returned( + { ...build("failed"), error: "Your local agent disconnected." }, + "Your local agent disconnected.", + )).toEqual({ + title: "Build failed", + detail: "Your local agent disconnected. Build plan builds it again.", + }); + expect(returned( + { ...build("failed"), error: "Stopped from the connector terminal." }, + "Stopped from the connector terminal.", + )).toEqual({ + title: "Stopped from the connector terminal", + detail: "Build plan builds it again.", + }); + }); + + it("keeps Returned for changes for a run its agent or a reviewer returned", () => { + expect(returned(build("stopped"), "Split it")).toEqual({ + title: "Returned for changes", + detail: "Split it. Build plan builds it again.", + }); + expect(returned(build("stopped"), "The build stopped before it finished.")).toEqual({ + title: "Stopped before it finished", + detail: "Build plan builds it again.", + }); + }); +}); + describe("buildPhase", () => { it("loads before the first snapshot", () => { expect(buildPhase(undefined)).toEqual({ kind: "loading" }); diff --git a/apps/web/src/build-model.ts b/apps/web/src/build-model.ts index eab432f36..17f278efc 100644 --- a/apps/web/src/build-model.ts +++ b/apps/web/src/build-model.ts @@ -200,6 +200,49 @@ export function endedReason(build: { error?: string } | undefined): string | und } } +const AGAIN = "Build plan builds it again."; +const UNFINISHED = "The build stopped before it finished."; + +/** `text` as a sentence, ending with one full stop unless it ends a sentence already. */ +function said(text: string): string { + let trimmed = text.replace(/\s+/g, " ").trim(); + return /[.?!…]$/.test(trimmed) ? trimmed : `${trimmed}.`; +} + +/** + * The Build view's card for a first build whose run ended with it: who stopped it in Chopin and + * when, the task its agent blocked and why, why it failed, or that its agent or a reviewer + * returned it for changes. Each says what Chat's end line and the machine's terminal say. + */ +export function endedCard(snapshot: Snapshot): { title: string; detail: string } { + let build = snapshot.build; + let run = latestRun(snapshot); + if (build?.stoppedBy) { + let at = build.finishedAt + ? new Date(build.finishedAt).toLocaleTimeString([], { hour: "numeric", minute: "2-digit" }) + : undefined; + return { title: `Stopped by @${build.stoppedBy}`, detail: at ? `At ${at}. ${AGAIN}` : AGAIN }; + } + let blocked = build?.state === "stopped" + ? run?.progress.tasks.find(task => task.state === "blocked" && task.blocker?.trim()) + : undefined; + // The blocked task's row, open beneath, names it; the card's one line leads with why. + if (blocked?.blocker) { + return { title: "Blocked", detail: `${said(blockerText(blocked.blocker))} ${AGAIN}` }; + } + if (build?.state === "failed" && build.error === "Stopped from the connector terminal.") { + return { title: "Stopped from the connector terminal", detail: AGAIN }; + } + if (build?.state === "failed") { + return { title: "Build failed", detail: build.error ? `${said(build.error)} ${AGAIN}` : AGAIN }; + } + let reason = run?.outcome.kind === "revision_requested" ? run.outcome.reason : undefined; + if (!reason || reason === UNFINISHED) { + return { title: "Stopped before it finished", detail: AGAIN }; + } + return { title: "Returned for changes", detail: `${said(reason)} ${AGAIN}` }; +} + /** * Why a task the build left unfinished stopped: the agent's last report, or where it * stopped when it said nothing. diff --git a/apps/web/src/build-view.tsx b/apps/web/src/build-view.tsx index 235c775b6..09946eb84 100644 --- a/apps/web/src/build-view.tsx +++ b/apps/web/src/build-view.tsx @@ -23,6 +23,7 @@ import { buildProgress, builtCount, builtDetail, + endedCard, linkParts, liveTaskGroups, liveTaskState, @@ -239,8 +240,9 @@ export function BuildView( : "This document can’t be built yet.", }; } - if (phase.kind === "drafting" && phase.draft === "returned") { - return { title: "Returned for changes", detail: "Build plan builds it again." }; + // Who stopped it, the task its agent blocked, or why it failed, as Chat's end line says. + if (phase.kind === "drafting" && phase.draft === "returned" && snapshot) { + return endedCard(snapshot); } // Build plan is the approval: pressing it approves these tasks and starts them. return { diff --git a/docs/agent-connection.md b/docs/agent-connection.md index dac1cc407..b73a89c24 100644 --- a/docs/agent-connection.md +++ b/docs/agent-connection.md @@ -1354,7 +1354,9 @@ not apply to it. same account attaching with a client that does not resume, which includes a browser sign-in attaching over a paired machine's parked work, since that client cannot resume it. All of these withdraw the questions and show the runs as `stopped`, and a - later `resume` finds nothing. A server restart does not end it: see below. A person + later `resume` finds nothing. Archiving stops the document's run cards itself, before it + releases the link, so they end even when the Planner is disconnected or a deploy is handing + over and its link reports nothing; an archived document always opens with its runs stopped. A server restart does not end it: see below. A person with write access can always end a disconnected Planner by resetting it. #### When the server is restarting diff --git a/docs/hosted-agent.md b/docs/hosted-agent.md index 0e72fb686..2496e5e13 100644 --- a/docs/hosted-agent.md +++ b/docs/hosted-agent.md @@ -174,7 +174,11 @@ and does not edit prose; only people approve and start implementation. Saving an anchored Planner decision durably queues a scoped prose update, independently of conversation-analysis flags. It uses the complete saved answers affecting that passage and updates only its explicit target. Applied answer generations commit with the source, so an -interrupted retry cannot write them twice. Lost or ambiguous targets refuse safely. Work waits +interrupted retry cannot write them twice. Lost or ambiguous targets refuse safely. Edits +elsewhere in the document between the job's read of its passage and its write, such as a +workflow run publishing its plan or another decision's update, do not refuse the write: it +lands when its passage, headings and saved answers are still the ones it read. A write-up that +still fails says why on its card in plain words, beside Retry. Work waits for an existing authorized Planner owner; it never claims ownership from a viewer or saver. The same Save also hands the answer to a Planner turn, so the Planner acts on what was decided rather than only rewriting the passage; answers saved close together share one turn. diff --git a/docs/local-experiments.md b/docs/local-experiments.md index 8dffc90b6..8e1446978 100644 --- a/docs/local-experiments.md +++ b/docs/local-experiments.md @@ -246,7 +246,7 @@ back ends as **your local agent disconnected** when the window passes, once someone opens the document or the agent calls Chopin. The window, the agent's `unavailable` answers and the machine taking the build back follow the same rule, so they never disagree. A build that ended before the restart stays ended -(`build-inactive`). +(`build-inactive`, or `build-stopped` with who stopped it when a person pressed Stop). A build claimed by an older connector, or by a server without this change, cannot go on, as Chopin cannot find it again: the connector stops its agent with diff --git a/docs/local-launcher.md b/docs/local-launcher.md index a3c9576dc..625e76de1 100644 --- a/docs/local-launcher.md +++ b/docs/local-launcher.md @@ -105,12 +105,19 @@ If a claimed run stops or fails, it ends with its build: Chopin records a revisi request with the build's error as the reason, releases the graph and opens the document for editing again. Its agent can no longer report on the run, so keeping the lock would only leave the document read-only. Build keeps the run's tasks, -blocker and pull requests and reads **Returned for changes**; the retained checkout -is there to inspect. **Build plan** then asks the Planner to revise the tasks and +blocker and pull requests, and its card says how the build ended, in the same words +as Chat's end line, the run's recorded reason and the connector's terminal: +**Stopped by @login** and when, for a person's Stop; **Blocked**, with the task and +the agent's reason, when its agent blocked a task and ended its turn; **Build +failed** with the error; or **Returned for changes** with the reason, when the agent +or a reviewer returned it. A build that delivered reads **Built**, and the connector +prints **Finished** with its pull requests. The retained checkout is there to +inspect. **Build plan** then asks the Planner to revise the tasks and builds the replacement. While the agent is still running the document stays locked; select **Stop build** in Build to end the run. Stop preserves existing commits and pull requests and rejects further reports from that run. The connector stops the agent on its next heartbeat (normally -within 10 seconds) and stays connected. Ctrl-C in its terminal also stops the work +within 10 seconds), printing **✗ Stopped in Chopin by @login.** rather than a failure, +and stays connected. Ctrl-C in its terminal also stops the work and disconnects the connector. Reconnect with the updated connector to receive activity updates. Stop also cancels automatic rebuilds already waiting behind the run; a later diff --git a/e2e/implementation-launcher.e2e.ts b/e2e/implementation-launcher.e2e.ts index 48163e494..e23406e28 100644 --- a/e2e/implementation-launcher.e2e.ts +++ b/e2e/implementation-launcher.e2e.ts @@ -343,7 +343,8 @@ for (let width of [1280, 390]) { "--command-activity", ]); try { - let page = await enter(person(), { viewport: { width, height: 844 } }); + let login = person(); + let page = await enter(login, { viewport: { width, height: 844 } }); await page.goto(await local.pairing); await page.getByRole("button", { name: "Add machine", exact: true }).click(); await expect(page.getByRole("status").filter({ hasText: "Connected" })).toHaveText( @@ -372,15 +373,27 @@ for (let width of [1280, 390]) { contentType: "image/png", }); await page.keyboard.press("Enter"); - // Stopping ends the run the build claimed: it is returned for changes, and the document opens. - await expect(view).toContainText("Returned for changes"); + // Stopping ends the run the build claimed, and the document opens. The Build tab, Chat, the + // run record and the connector's terminal each say who stopped it, never a failure. + await expect(view.getByText(`Stopped by @${login}`, { exact: true })).toBeVisible(); + await expect(view).not.toContainText("Returned for changes"); await expect(stop).toHaveCount(0); await expect(page.getByRole("textbox", { name: "editable markdown", includeHidden: true })) .toHaveAttribute("contenteditable", "true"); await expect.poll(() => local.output(), { timeout: 20_000 }) - .toMatch(/Chopin ended this run \(build-(?:inactive|forbidden)\)/); + .toContain(`✗ Stopped in Chopin by @${login}.`); + expect(local.output()).not.toContain("Failed"); let snapshot = await (await page.request.get(`/api/channels/${room}/implementation`)).json(); - expect(snapshot.build.error).toBe("Stopped in Chopin."); + expect(snapshot.build).toMatchObject({ error: "Stopped in Chopin.", stoppedBy: login }); + expect(snapshot.lifecycle.history.at(-1).outcome.reason).toBe( + `Stopped in Chopin by @${login}.`, + ); + // A phone shows Build in Chat's place; a desktop shows Chat's end line beside it, which + // draws the handle as a mention. + if (width > 390) { + let chat = page.getByRole("complementary", { name: "Chat", exact: true }); + await expect(chat.getByText(new RegExp(`^Build stopped by @?${login}\\.$`))).toBeVisible(); + } expect(snapshot.localAgent).toBe(true); } finally { await local.close(); @@ -479,9 +492,21 @@ for (let mode of ["block", "complete", "startup-failure", "live"] as const) { await expect(chat).toBeVisible(); await expect(chat.getByRole("region", { name: "Workflow runs" })).toHaveCount(0); } else { - // The run ended with its build, so it was returned for changes and keeps its blocker. - await expect(view.getByText("Returned for changes", { exact: true })).toBeVisible(); - await expect(view.getByText("Build plan builds it again.", { exact: true })).toBeVisible(); + // The run ended with its build, blocked: the card says why, as Chat and the connector's + // terminal do, and the blocked task's row names it. + await expect(view.getByText("Blocked", { exact: true })).toBeVisible(); + await expect(view).not.toContainText("Returned for changes"); + await expect( + view.getByText( + "Choose the next tracer. Build plan builds it again.", + { exact: true }, + ), + ).toBeVisible(); + if (mode === "block") { + await expect.poll(() => local.output(), { timeout: 20_000 }) + .toContain("✗ Blocked: “Connect the local agent” is blocked: Choose the next tracer."); + expect(local.output()).not.toContain("Finished"); + } await expect(blocked).toHaveAttribute("aria-expanded", "true"); await expect(view.getByText(/^Blocked: Choose the next tracer/)).toBeVisible(); // Build replaces the document area, so the open editor is hidden here. diff --git a/packages/editor/src/widgets/questionnaire-document-visibility.test.tsx b/packages/editor/src/widgets/questionnaire-document-visibility.test.tsx index 060d7ca50..2f685fb3b 100644 --- a/packages/editor/src/widgets/questionnaire-document-visibility.test.tsx +++ b/packages/editor/src/widgets/questionnaire-document-visibility.test.tsx @@ -113,6 +113,27 @@ test("a failed write-up offers Retry from the Decisions card", () => { ); }); +test("a failed write-up says why beside its Retry", () => { + let markup = renderToStaticMarkup(createElement(QuestionnaireCard, { + canEdit: true, + connected: true, + meta: { + ...PLANNER, + writeup: { + status: "failed", + job: "decision-followup:w:decided:w:1", + reason: "Writing it failed twice: the document changed while it was being written.", + }, + }, + presentation: "list", + value: DECIDED, + wire: { ask: async () => ({}), send() {}, on: () => () => {} } as never, + })); + expect(markup).toContain( + "Couldn't write this up. Writing it failed twice: the document changed while it was being written.", + ); +}); + test("decided cards a deck still draws are not counted as written up out of sight", () => { let open = { ...DECIDED, diff --git a/packages/editor/src/widgets/writeup-status.tsx b/packages/editor/src/widgets/writeup-status.tsx index 477244f1f..772ab5c27 100644 --- a/packages/editor/src/widgets/writeup-status.tsx +++ b/packages/editor/src/widgets/writeup-status.tsx @@ -42,8 +42,10 @@ export function WriteupStatus( setRetrying(false); }; return ( - - Couldn't write this up + + + {writeup.reason ? `Couldn't write this up. ${writeup.reason}` : "Couldn't write this up"} +