diff --git a/apps/server/src/agent/planner.test.ts b/apps/server/src/agent/planner.test.ts index 24e180a57..ba7c8e388 100644 --- a/apps/server/src/agent/planner.test.ts +++ b/apps/server/src/agent/planner.test.ts @@ -27,7 +27,7 @@ test("all Planner surfaces compose coaching once and preserve their question bou expect(prompt.split(COACHING)).toHaveLength(2); expect(prompt.endsWith("Earlier context.")).toBe(true); if (workspace) { - expect(prompt).toContain("Expiry is not an answer or approval"); + expect(prompt).toContain("a withdrawn question is not an answer or approval"); expect(prompt).toContain( "continue only independent work with explicitly provisional assumptions", ); diff --git a/apps/server/src/agent/planner.ts b/apps/server/src/agent/planner.ts index fcab40a3f..b1a3ade52 100644 --- a/apps/server/src/agent/planner.ts +++ b/apps/server/src/agent/planner.ts @@ -369,7 +369,8 @@ and cannot change GitHub. Ground the plan in what those reading tools return.`; } let questions = `\`ask_user_question\` and \`workflow\` questions appear to the document's members as -Decisions. Expiry is not an answer or approval. Keep consequential choices open; +Decisions. Each waits until a member answers it or it is withdrawn, however long that takes; +a withdrawn question is not an answer or approval. Keep consequential choices open; continue only independent work with explicitly provisional assumptions.`; let surface = `The document's members use Chopin in a browser. They cannot run slash commands, terminal commands, or Atomic CLI commands, so never tell them to use \`/workflow connect\`, diff --git a/apps/server/src/chat/invoke.test.ts b/apps/server/src/chat/invoke.test.ts index c44fa6d90..b3ca45f30 100644 --- a/apps/server/src/chat/invoke.test.ts +++ b/apps/server/src/chat/invoke.test.ts @@ -474,7 +474,9 @@ test("HARNESS=atomic runs every Planner session full in hosted and local configu let harness = context.config.harness; expect({ harness, full: !!turn.full }).toEqual({ harness, full }); expect(!!turn.full?.humanInput).toBe(full); - expect(turn.instructions.includes("Expiry is not an answer or approval")).toBe(full); + expect(turn.instructions.includes("a withdrawn question is not an answer or approval")).toBe( + full, + ); } } }); diff --git a/apps/server/src/experiments/routes.ts b/apps/server/src/experiments/routes.ts index 534032ac0..a2097c738 100644 --- a/apps/server/src/experiments/routes.ts +++ b/apps/server/src/experiments/routes.ts @@ -159,7 +159,8 @@ export const SPIKE_DEADLINE_MS = 20 * 60_000; export const MAX_INVESTIGATION_IMAGES = 3; /** * A document on Default machine has its Planner run on its creator's machine while someone uses - * it: it is open in Chopin, or was within this long. + * it: it is open in Chopin, its Planner waits on a question or a live or paused workflow run, or + * either was so within this long. */ export const PLANNER_IDLE_MS = 30 * 60_000; /** @@ -407,9 +408,10 @@ export function registerExperimentRoutes(router: Router, auth: HostedAuth, optio ); } /** - * Documents someone uses, by id, with when they last were seen in use: open in Chopin, or - * their Planner asked for. On Default machine their creator's machine runs their Planner until - * one goes PLANNER_IDLE_MS unused. Forgotten with a restart. + * Documents someone uses, by id, with when they last were seen in use: open in Chopin, their + * Planner asked for, or their Planner waiting on a question or workflow run. On Default machine + * their creator's machine runs their Planner until one goes PLANNER_IDLE_MS unused. Forgotten + * with a restart. */ let inUse = new Map(); /** @@ -535,13 +537,14 @@ export function registerExperimentRoutes(router: Router, auth: HostedAuth, optio return undefined; } /** - * A document stops being used once nobody has had it open for PLANNER_IDLE_MS: the machine - * Default machine gave its Planner stops running it. + * A document stops being used once nobody has had it open, and its Planner has waited on no + * question or workflow run, for PLANNER_IDLE_MS: the machine Default machine gave its Planner + * stops running it. A question never times out by this route while its Planner waits on it. */ async function plannersIdle() { let now = auth.clock().getTime(); for (let [documentId, seen] of inUse) { - if (options.documentOpen?.(documentId)) { + if (options.documentOpen?.(documentId) || options.attachments?.()?.waiting(documentId)) { inUse.set(documentId, now); continue; } diff --git a/apps/server/src/harness/atomic/human-input.test.ts b/apps/server/src/harness/atomic/human-input.test.ts index d3c40d50d..4aa2c7310 100644 --- a/apps/server/src/harness/atomic/human-input.test.ts +++ b/apps/server/src/harness/atomic/human-input.test.ts @@ -1,16 +1,13 @@ import { afterEach, expect, test } from "bun:test"; -import { $createParagraphNode, $createTextNode, $getRoot, $nodesOfType } from "lexical"; -import * as Y from "yjs"; -import { cardStatus, limits, QuestionnaireNode, ulid } from "@chopin/dialect"; -import { limits as questionLimits } from "@chopin/question"; +import { $nodesOfType } from "lexical"; +import { limits, QuestionnaireNode, ulid } from "@chopin/dialect"; import * as QuestionModel from "@chopin/question"; import * as Questions from "../../questions/service"; import * as Store from "../../questions/store"; import * as Plan from "../../plan/service"; import * as Room from "../../plan/room"; import * as Edit from "../../plan/edit"; -import { hostInputRoom } from "../../testing/decisions"; -import { peer } from "../../testing/peer"; +import { hostInputRoom, laterTimers } from "../../testing/decisions"; import type { QuestionParams } from "@bastani/atomic"; let cleanups: Array<() => Promise> = []; @@ -18,8 +15,8 @@ afterEach(async () => { for (let cleanup of cleanups.splice(0)) await cleanup(); }); -async function fixture(expiresInMs?: number) { - let f = await hostInputRoom(expiresInMs); +async function fixture() { + let f = await hostInputRoom(); cleanups.push(f.close); return f; } @@ -248,43 +245,39 @@ test("abort withdraws only its request's open cards, persists cancellation, and expect(saved.source).not.toContain(cards[1]!.id); }); -test("expiry keeps every unanswered card in Decisions, marked expired, and answers no HostInput method", async () => { - let f = await fixture(30); +test("a withdrawal removes every card and answers no HostInput method", async () => { + let f = await fixture(); for ( - let [invoke, expected] of [ - [() => f.input.questionnaire(params, f.options), { answers: [], cancelled: true }], - [() => f.input.confirm("Confirm", "Proceed?", f.options), false], - [() => f.input.select("Select", ["A", "B"], f.options), undefined], - [() => f.input.input("Input", undefined, f.options), undefined], - [() => f.input.editor("Editor", undefined, f.options), undefined], - ] as Array<[() => Promise, unknown]> + let [invoke, expected, count] of [ + [options => f.input.questionnaire(params, options), { answers: [], cancelled: true }, 3], + [options => f.input.confirm("Confirm", "Proceed?", options), false, 1], + [options => f.input.select("Select", ["A", "B"], options), undefined, 1], + [options => f.input.input("Input", undefined, options), undefined, 1], + [options => f.input.editor("Editor", undefined, options), undefined, 1], + ] as Array<[(options: typeof f.options) => Promise, unknown, number]> ) { - expect(await invoke()).toEqual(expected); + let controller = new AbortController(); + let response = invoke({ ...f.options, signal: controller.signal }); + await f.cards(count); + controller.abort(); + expect(await response).toEqual(expected); expect(Store.outstanding(f.plan.questions)).toHaveLength(0); } let records = [...f.plan.records.values()]; expect(records).toHaveLength(7); for (let record of records) { - expect(record).toMatchObject({ status: "expired", resolver: "chopin" }); - expect(record.at).toBeNumber(); + expect(record).toMatchObject({ status: "cancelled", resolver: "chopin" }); expect(f.frames).toContainEqual( expect.objectContaining({ kind: "question:resolved", id: record.id, - status: "expired", + status: "cancelled", resolver: "chopin", }), ); } let saved = await Plan.readStored((await f.storage.collaboration.load(f.room.id, new Date()))!); - let cards = await documentCards(saved.source); - expect(cards.map(card => card.id)).toEqual(records.map(record => record.id)); - for (let card of cards) { - expect(card.status).toBe("expired"); - expect(card.at).toBeString(); - expect(card.by).toBeUndefined(); - expect(cardStatus(card)).toBe("expired"); - } + expect(await documentCards(saved.source)).toEqual([]); await Questions.submit(f.plan, f.server, f.room.id, f.ws, { kind: "question:submit", ts: 0, @@ -295,42 +288,54 @@ test("expiry keeps every unanswered card in Decisions, marked expired, and answe expect(f.frames.findLast(frame => frame.rid === "late")).toMatchObject({ ok: false, reason: "resolved", - status: "expired", + status: "cancelled", resolver: "chopin", }); - expect(f.plan.records.get(records[0]!.id)?.status).toBe("expired"); + expect(f.plan.records.get(records[0]!.id)?.status).toBe("cancelled"); }); -test("an abort still withdraws its cards rather than expiring them", async () => { - let f = await fixture(60_000); - let response = f.input.confirm("Confirm", "Proceed?", f.options); - let [card] = await f.cards(1); - f.controller.abort(); - expect(await response).toBe(false); - expect(f.plan.records.get(card!.id)).toMatchObject({ status: "cancelled", resolver: "chopin" }); - expect(Plan.source(f.plan)).not.toContain(card!.id); - expect(f.frames.filter(frame => frame.kind === "question:resolved")).toMatchObject([ - { kind: "question:resolved", id: card!.id, status: "cancelled", resolver: "chopin" }, - ]); +test("host input stays open past half an hour and returns the member's answer", async () => { + let f = await fixture(); + let clock = laterTimers(); + try { + let settled = false; + let response = f.input.confirm("Confirm", "Proceed?", f.options).finally(() => { + settled = true; + }); + let [card] = await f.cards(1); + await clock.pass(31 * 60 * 1_000); + await clock.pass(24 * 60 * 60 * 1_000); + expect(settled).toBe(false); + expect(Store.outstanding(f.plan.questions).map(open => open.id)).toEqual([card!.id]); + expect(f.plan.records.get(card!.id)?.status).toBe("open"); + expect(Plan.source(f.plan)).toContain(card!.id); + await f.answer(card!.id, [0]); + expect(await response).toBe(true); + } finally { + clock.restore(); + } }); -test("host input waits the shared 30-minute limit before expiring by default", async () => { +test("host input open past half an hour is withdrawn when its Planner stops", async () => { let f = await fixture(); - let delays: number[] = []; - let original = globalThis.setTimeout; - globalThis.setTimeout = ((handler: () => void, delay?: number, ...rest: unknown[]) => { - if (delay !== undefined) delays.push(delay); - return original(handler, delay, ...rest); - }) as typeof setTimeout; + let clock = laterTimers(); try { - let response = f.input.confirm("Confirm", "Proceed?", f.options); - await f.cards(1); - expect(questionLimits.INPUT_EXPIRY_MS).toBe(30 * 60 * 1_000); - expect(delays).toContain(questionLimits.INPUT_EXPIRY_MS); + let response = f.input.questionnaire(params, f.options); + let cards = await f.cards(3); + await clock.pass(31 * 60 * 1_000); + expect(Store.outstanding(f.plan.questions)).toHaveLength(3); f.controller.abort(); - expect(await response).toBe(false); + expect(await response).toEqual({ answers: [], cancelled: true }); + expect(Store.outstanding(f.plan.questions)).toHaveLength(0); + for (let card of cards) { + expect(f.plan.records.get(card.id)).toMatchObject({ + status: "cancelled", + resolver: "chopin", + }); + expect(Plan.source(f.plan)).not.toContain(card.id); + } } finally { - globalThis.setTimeout = original; + clock.restore(); } }); @@ -570,33 +575,6 @@ async function seedDocument(f: Fixture) { await Plan.publish(f.plan, f.server, f.room.id, seeded.mutation); } -/** Fill the document until `fits` just holds, leaving a margin smaller than one card's expiry. */ -async function crowd(f: Fixture, fits: (scratch: Room.Document) => boolean) { - let size = () => new TextEncoder().encode(Plan.source(f.plan)).length; - let base = Plan.source(f.plan); - let low = 0; - for (let high = limits.MAX_SOURCE_BYTES; low < high;) { - let mid = Math.ceil((low + high) / 2); - let scratch = await Room.create(`${base}\n${"x".repeat(mid)}\n`).catch(() => undefined); - let ok = scratch ? fits(scratch) : false; - scratch?.doc.destroy(); - if (ok) low = mid; - else high = mid - 1; - } - let target = size() + low - 10; - for (let fill = 1; fill > 0;) { - fill = Math.min(40_000, target - size() - 2); - if (fill < 1) break; - let edited = Edit.replace( - f.plan, - f.plan.revision, - `${Plan.source(f.plan)}\n${"x".repeat(fill)}\n`, - ); - if (!edited.ok) throw new Error(`edit refused: ${edited.reason}`); - if (edited.mutation) await Plan.publish(f.plan, f.server, f.room.id, edited.mutation); - } -} - const single = (header: string): QuestionParams => ({ questions: [{ header, @@ -605,6 +583,7 @@ const single = (header: string): QuestionParams => ({ }], }); +const MAX = limits.MAX_SOURCE_BYTES; const bytes = (value: string) => new TextEncoder().encode(value).length; const FILLER = /\n\nq{10,}(?=\n)/g; @@ -645,397 +624,157 @@ async function growth(f: Fixture, step: (scratch: Room.Document) => void) { return after - before; } +/** A size refusal names the document's limit and never an expiry. */ +function sizeRefusal(message: string) { + expect(message).toContain(`${MAX / 1024} KiB limit`); + expect(message).not.toMatch(/expir/i); +} + test( - "questions that fit when asked still fit when they expire, however the document changed since", + "with a question open, Planner edits may fill the document to its exact size limit and no further", async () => { let f = await fixture(); - let max = limits.MAX_SOURCE_BYTES; - let low = 0; - for (let high = max; low < high;) { - let mid = Math.ceil((low + high) / 2); - if (Room.fitsExpiry("a".repeat(mid), 1)) low = mid; - else high = mid - 1; - } - let expiry = max - low; - let quote = "caches tiles for 60 seconds"; - let seeded = Edit.apply(f.plan, f.plan.revision, [{ - op: "replace_root", - source: `# Plan\n\nThe renderer ${quote}.\n`, - }]); - if (!seeded.ok || !seeded.mutation) throw new Error("fixture edit failed"); - await Plan.publish(f.plan, f.server, f.room.id, seeded.mutation); - - let responses = [ - f.input.questionnaire(single("First"), f.options), - f.input.questionnaire(single("Second"), { ...f.options, requestId: "second" }), - ]; - let [first, second] = await f.cards(2); - - // Planner text edits: two questions are open, so growth stops at their reserve. - await fillTo(f, max - 2 * expiry); - let source = Plan.source(f.plan); - expect(Edit.apply(f.plan, f.plan.revision, [{ op: "insert", index: 0, source: "y" }])) - .toMatchObject({ ok: false, message: expect.stringContaining("expire") }); - expect(Edit.replace(f.plan, f.plan.revision, `${source}\ny\n`)) - .toMatchObject({ ok: false, message: expect.stringContaining("expire") }); - expect(Plan.source(f.plan)).toBe(source); - - // An option that would take the document into the reserve. - let label = "Written ".repeat(7); - let question = first!.definition.questions[0]!; - let option = await growth( - f, - scratch => - Room.appendQuestionOption(scratch, first!.id, question.id, { - id: ulid(), - label, - description: "", - }), - ); - await fillTo(f, max - 10 - option); - await expect(f.addOption(first!.id, label)).rejects.toThrow("no room"); - await fillTo(f, max - option - 2 * expiry - 100); - await f.addOption(first!.id, label); - - // An answer that would do the same, then one that fits. - let text = "a".repeat(1_500); - let answered = await growth( - f, - scratch => - Room.projectAnswer(scratch, first!.id, { [question.id]: text }, { - by: "reader", - at: new Date().toISOString(), - }), - ); - await fillTo(f, max - 10 - answered); - await expect(f.answer(first!.id, text)).rejects.toThrow("no room"); - expect(Store.outstanding(f.plan.questions)).toHaveLength(2); - await fillTo(f, max - answered - expiry - 100); - await f.answer(first!.id, ""); - expect(Store.outstanding(f.plan.questions).map(open => open.id)).toEqual([second!.id]); - - // A question that would take the document into the reserve, then one that fits. - let probe = { - id: ulid(), - questions: [{ - id: ulid(), - header: "Third", - prompt: "Which?", - multiple: false, - options: [{ id: ulid(), label: "A" }, { id: ulid(), label: "B" }], - }], - }; - let asked = await growth(f, scratch => - Room.insertQuestionnaire(scratch, { - ...probe, - status: "expired", - at: new Date().toISOString(), - })); - await fillTo(f, max - 10 - asked); - let refused = f.input.questionnaire(single("Third"), { ...f.options, requestId: "third" }); - expect( - await Promise.race([ - refused.then(() => "asked", (err: Error) => err.message), - Bun.sleep(300).then(() => "asked"), - ]), - ).toContain("KiB limit"); - expect(Store.outstanding(f.plan.questions)).toHaveLength(1); - await fillTo(f, max - asked - expiry - 100); - responses.push( - f.input.questionnaire(single("Third"), { ...f.options, requestId: "third-again" }), - ); - await f.cards(2); - - // A person's text edit: growth into the reserve is refused, shrinking or fitting is not. - await fillTo(f, max - 2 * expiry); - let sent: any[] = []; - let ben = { - data: { handle: "ben", client: "client-ben", room: f.room.id }, - send: (raw: string) => sent.push(JSON.parse(raw)), - close() {}, - publish() {}, - } as unknown as Parameters[1]; - let type = async (client: ReturnType, change: () => void) => { - let before = Y.encodeStateVector(client.doc); - client.editor.update(change, { discrete: true }); - let rid = `ben-${sent.length}`; - Plan.submit( - f.plan, - ben, - { - kind: "plan:update", - rid, - ts: 0, - epoch: f.plan.document.epoch, - id: rid, - update: Buffer.from(Y.encodeStateAsUpdate(client.doc, before)).toString("base64"), - } as Parameters[2], - ); - await Bun.sleep(30); - await Plan.drain(f.plan); - return sent.findLast(frame => frame.kind === "plan:ack" && frame.id === rid); - }; - let grow = () => { - let paragraph = $createParagraphNode(); - paragraph.append($createTextNode("More text.")); - $getRoot().append(paragraph); - }; - let epoch = f.plan.document.epoch; - let before = Plan.source(f.plan); - let ben1 = peer(); - Y.applyUpdate(ben1.doc, Room.sync(f.plan.document), "remote"); - await Room.settle(); - expect(await type(ben1, grow)).toBeUndefined(); - expect(Plan.source(f.plan)).toBe(before); - expect(f.plan.document.epoch).not.toBe(epoch); - - let ben2 = peer(); - Y.applyUpdate(ben2.doc, Room.sync(f.plan.document), "remote"); - await Room.settle(); - let shrunk = await type(ben2, () => { - $getRoot().getChildren().find(node => node.getTextContent().startsWith("qq"))?.remove(); - }); - expect(shrunk).toBeDefined(); - expect(bytes(Plan.source(f.plan))).toBeLessThan(bytes(before)); - let fitted = await type(ben2, grow); - expect(fitted).toBeDefined(); - expect(Plan.source(f.plan)).toContain("More text."); - - // Both questions expire from a document with exactly their reserve left. - await fillTo(f, max - 2 * expiry); - for (let { id } of Store.outstanding(f.plan.questions)) { - await Questions.expire(f.plan, f.server, f.room.id, id); - } - await Promise.allSettled(responses); - - expect(Store.outstanding(f.plan.questions)).toHaveLength(0); - expect(bytes(Plan.source(f.plan))).toBeLessThanOrEqual(max); - let retitled = Edit.replace( + await seedDocument(f); + let response = f.input.questionnaire(single("First"), f.options); + await f.cards(1); + await fillTo(f, MAX - 1); + let filled = Edit.replace( f.plan, f.plan.revision, - Plan.source(f.plan).replace("# Plan", "# Pla2"), + Plan.source(f.plan).replace("# Plan", "# Plans"), ); - expect(retitled.ok).toBe(true); + expect(filled.ok).toBe(true); + if (filled.ok && filled.mutation) { + await Plan.publish(f.plan, f.server, f.room.id, filled.mutation); + } + expect(bytes(Plan.source(f.plan))).toBe(MAX); + let source = Plan.source(f.plan); + for ( + let refused of [ + Edit.apply(f.plan, f.plan.revision, [{ op: "insert", index: 0, source: "y" }]), + Edit.replace(f.plan, f.plan.revision, source.replace("# Plans", "# Planss")), + ] + ) { + expect(refused).toMatchObject({ ok: false, reason: "invalid" }); + sizeRefusal("message" in refused ? refused.message ?? "" : ""); + } + expect(Plan.source(f.plan)).toBe(source); + f.controller.abort(); + await response; }, 30_000, ); test( - "a new question is refused when the questions already open would no longer have room to expire", + "a new question fits a document up to its exact size limit while another is open", async () => { let f = await fixture(); await seedDocument(f); - let first = f.input.questionnaire(single("First"), f.options); + let responses = [f.input.questionnaire(single("First"), f.options)]; await f.cards(1); - let probe = { - id: ulid(), - questions: [{ + let asked = await growth(f, scratch => + Room.insertQuestionnaire(scratch, { id: ulid(), - header: "Second", - prompt: "Which?", - multiple: false, - options: [{ id: ulid(), label: "A" }, { id: ulid(), label: "B" }], - }], - }; - await crowd(f, scratch => Room.fitsQuestionnaires(scratch, [probe], 0)); + questions: [{ + id: ulid(), + header: "Second", + prompt: "Which?", + multiple: false, + options: [{ id: ulid(), label: "A" }, { id: ulid(), label: "B" }], + }], + })); - let second = f.input.questionnaire(single("Second"), { ...f.options, requestId: "second" }); - let outcome = await Promise.race([ - second.then(() => "asked", (err: Error) => err.message), - Bun.sleep(500).then(() => "asked"), - ]); - expect(outcome).toContain("KiB limit"); + await fillTo(f, MAX - asked + 1); + let refused = f.input.questionnaire(single("Second"), { ...f.options, requestId: "past" }); + sizeRefusal( + await Promise.race([ + refused.then(() => "asked", (err: Error) => err.message), + Bun.sleep(500).then(() => "asked"), + ]), + ); expect(Store.outstanding(f.plan.questions)).toHaveLength(1); - f.controller.abort(); - await first; - }, - 30_000, -); -test( - "an option is refused when the questions open would no longer have room to expire", - async () => { - let f = await fixture(); - await seedDocument(f); - let response = f.input.questionnaire({ - questions: [{ - header: "Choice", - question: "Which?", - options: [{ label: "A", description: "" }, { label: "B", description: "" }], - }], - }, f.options); - let [card] = await f.cards(1); - let question = card!.definition.questions[0]!; - await crowd(f, scratch => { - Room.appendQuestionOption(scratch, card!.id, question.id, { - id: ulid(), - label: "Another", - description: "", - }); - return Room.fitsExpiry(Room.project(scratch), 0); - }); - - await expect(f.addOption(card!.id, "Another")).rejects.toThrow("no room"); - expect(Store.get(f.plan.questions, card!.id)!.definition.questions[0]!.options).toHaveLength(2); + await fillTo(f, MAX - asked); + responses.push(f.input.questionnaire(single("Second"), { ...f.options, requestId: "fits" })); + await f.cards(2); + expect(bytes(Plan.source(f.plan))).toBe(MAX); f.controller.abort(); - await response; + await Promise.allSettled(responses); }, 30_000, ); test( - "an answer is refused when the questions still open would no longer have room to expire", + "an option fits a document up to its exact size limit while questions are open", async () => { let f = await fixture(); await seedDocument(f); - let first = f.input.questionnaire(single("First"), f.options); - let second = f.input.questionnaire(single("Second"), { ...f.options, requestId: "second" }); + let responses = [ + f.input.questionnaire(single("First"), f.options), + f.input.questionnaire(single("Second"), { ...f.options, requestId: "second" }), + ]; let [card] = await f.cards(2); - let answer = "x".repeat(300); let question = card!.definition.questions[0]!; - await crowd(f, scratch => { - Room.projectAnswer(scratch, card!.id, { [question.id]: answer }, { - by: "reader", - at: new Date().toISOString(), - }); - return Room.fitsExpiry(Room.project(scratch), 0); - }); - - let before = Plan.source(f.plan); - await expect(f.answer(card!.id, answer)).rejects.toThrow("no room"); - expect(Plan.source(f.plan)).toBe(before); - expect(Store.outstanding(f.plan.questions)).toHaveLength(2); - f.controller.abort(); - await Promise.allSettled([first, second]); - }, - 30_000, -); - -test( - "a Planner edit is refused when the open question would no longer have room to expire", - async () => { - let f = await fixture(); - await seedDocument(f); - let response = f.input.questionnaire(single("First"), f.options); - await f.cards(1); - await crowd(f, scratch => Room.fitsExpiry(Room.project(scratch), 1)); + let label = "Written answer"; + let option = await growth( + f, + scratch => + Room.appendQuestionOption(scratch, card!.id, question.id, { + id: ulid(), + label, + description: "", + }), + ); + let options = () => Store.get(f.plan.questions, card!.id)!.definition.questions[0]!.options; - let before = Plan.source(f.plan); - let grown = Edit.apply(f.plan, f.plan.revision, [{ - op: "insert", - index: 0, - source: "y".repeat(30), - }]); - expect(grown).toMatchObject({ - ok: false, - reason: "invalid", - message: expect.stringContaining("expire"), - }); - expect(Plan.source(f.plan)).toBe(before); + await fillTo(f, MAX - option + 1); + await expect(f.addOption(card!.id, label)).rejects.toThrow("no room"); + expect(options()).toHaveLength(2); - let retitled = Edit.apply(f.plan, f.plan.revision, [{ - op: "replace", - index: 0, - source: "# Pla2", - }]); - expect(retitled.ok).toBe(true); + await fillTo(f, MAX - option); + await f.addOption(card!.id, label); + expect(options()).toHaveLength(3); + expect(bytes(Plan.source(f.plan))).toBe(MAX); f.controller.abort(); - await response; + await Promise.allSettled(responses); }, 30_000, ); test( - "a replaced plan is refused when the open question would no longer have room to expire", + "an answer fits a document up to its exact size limit while another question is open", async () => { let f = await fixture(); await seedDocument(f); - let response = f.input.questionnaire(single("First"), f.options); - await f.cards(1); - await crowd(f, scratch => Room.fitsExpiry(Room.project(scratch), 1)); + let responses = [ + f.input.questionnaire(single("First"), f.options), + f.input.questionnaire(single("Second"), { ...f.options, requestId: "second" }), + ]; + let [card, other] = await f.cards(2); + let question = card!.definition.questions[0]!; + let text = "a".repeat(1_500); + let answered = await growth( + f, + scratch => + Room.projectAnswer(scratch, card!.id, { [question.id]: text }, { + by: "reader", + at: new Date().toISOString(), + }), + ); + await fillTo(f, MAX - answered + 1); let before = Plan.source(f.plan); - let grown = Edit.replace(f.plan, f.plan.revision, `${before}\n${"y".repeat(30)}\n`); - expect(grown).toMatchObject({ - ok: false, - reason: "invalid", - message: expect.stringContaining("expire"), - }); + sizeRefusal(await f.answer(card!.id, text).then(() => "", (err: Error) => err.message)); expect(Plan.source(f.plan)).toBe(before); + expect(Store.outstanding(f.plan.questions)).toHaveLength(2); - let retitled = Edit.replace(f.plan, f.plan.revision, before.replace("# Plan", "# Pla2")); - expect(retitled.ok).toBe(true); - f.controller.abort(); - await response; - }, - 30_000, -); - -/** A document already inside the reserve of its open questions, as one that has two open but room for one. */ -async function overcommitted(f: Fixture) { - await seedDocument(f); - let response = f.input.questionnaire(single("First"), f.options); - let [card] = await f.cards(1); - await crowd(f, scratch => Room.fitsExpiry(Room.project(scratch), 1)); - void Store.ask(f.plan.questions, ulid(), Store.get(f.plan.questions, card!.id)!.definition); - expect(Room.fitsExpiry(Plan.source(f.plan), f.plan.questions.open.size)).toBe(false); - return { response }; -} - -test( - "a Planner block edit that shrinks the document is accepted even inside the expiry reserve", - async () => { - let f = await fixture(); - let { response } = await overcommitted(f); - let before = new TextEncoder().encode(Plan.source(f.plan)).length; - - let grown = Edit.apply(f.plan, f.plan.revision, [{ - op: "insert", - index: 0, - source: "y".repeat(30), - }]); - expect(grown).toMatchObject({ - ok: false, - reason: "invalid", - message: expect.stringContaining("expire"), - }); - - let shrunk = Edit.apply(f.plan, f.plan.revision, [{ op: "delete", index: 1 }]); - expect(shrunk.ok).toBe(true); - let trimmed = Edit.apply(f.plan, f.plan.revision, [{ - op: "replace", - index: 0, - source: "# P", - }]); - expect(trimmed.ok).toBe(true); - if (trimmed.ok && trimmed.mutation) { - await Plan.publish(f.plan, f.server, f.room.id, trimmed.mutation); - } - expect(new TextEncoder().encode(Plan.source(f.plan)).length).toBeLessThan(before); - f.controller.abort(); - await response; - }, - 30_000, -); - -test( - "a replaced plan that shrinks the document is accepted even inside the expiry reserve", - async () => { - let f = await fixture(); - let { response } = await overcommitted(f); - let before = Plan.source(f.plan); - - let grown = Edit.replace(f.plan, f.plan.revision, `${before}\n${"y".repeat(30)}\n`); - expect(grown).toMatchObject({ - ok: false, - reason: "invalid", - message: expect.stringContaining("expire"), - }); - - let shrunk = Edit.replace(f.plan, f.plan.revision, before.replace("Start.", "S")); - expect(shrunk.ok).toBe(true); + // The refused text is still the shared draft, so submitting again sends it as it is. + await fillTo(f, MAX - answered); + await f.answer(card!.id, ""); + expect(Store.outstanding(f.plan.questions).map(open => open.id)).toEqual([other!.id]); + expect(bytes(Plan.source(f.plan))).toBe(MAX); f.controller.abort(); - await response; + await Promise.allSettled(responses); }, 30_000, ); @@ -1046,7 +785,7 @@ test("a workflow question's answer is returned only after its paused run is rele release = resolve; }); let held: string[] = []; - let f = await hostInputRoom(undefined, async runId => { + let f = await hostInputRoom(async runId => { held.push(runId); await gate; }); diff --git a/apps/server/src/harness/atomic/human-input.ts b/apps/server/src/harness/atomic/human-input.ts index f900e2ac0..ed1896595 100644 --- a/apps/server/src/harness/atomic/human-input.ts +++ b/apps/server/src/harness/atomic/human-input.ts @@ -1,5 +1,3 @@ -import { limits } from "@chopin/question"; - import * as Questions from "../../questions/service"; import type { @@ -11,20 +9,20 @@ import type { } from "@bastani/atomic"; import type { DocumentRoom } from "../../agent/tools"; -export type HumanInputStatus = "answered" | "expired" | "cancelled"; +export type HumanInputStatus = "answered" | "cancelled"; /** * Atomic owns input requests; Chopin owns their shared decision records. * - * A request nobody answers within `expiresInMs` expires: its cards stay in - * Decisions marked expired, and Atomic gets no answer. An answer to a workflow + * A request has no time limit: it waits until a member answers it or it is + * withdrawn, as when its Planner stops, its run is cancelled, or its link + * closes. An answer to a workflow * question is returned only once `hold` allows it, so a paused run stays stopped. * `report` learns how each request ended, which the cancelled result alone * cannot tell apart. */ export function createHumanInput( room: DocumentRoom, - expiresInMs = limits.INPUT_EXPIRY_MS, hold?: (workflowRunId: string, signal: AbortSignal) => Promise, report?: (status: HumanInputStatus) => void, ): HostInput { @@ -59,10 +57,9 @@ export function createHumanInput( undefined, room.anchors, options.signal, - expiresInMs, ); - if (options.signal.aborted || ended.some(outcome => outcome.status === "expired")) { - report?.(options.signal.aborted ? "cancelled" : "expired"); + if (options.signal.aborted) { + report?.("cancelled"); return { answers: [], cancelled: true }; } if (hold && options.workflowRunId !== undefined) { diff --git a/apps/server/src/harness/remote/attachments.ts b/apps/server/src/harness/remote/attachments.ts index a525c6af6..1205d5b0a 100644 --- a/apps/server/src/harness/remote/attachments.ts +++ b/apps/server/src/harness/remote/attachments.ts @@ -127,6 +127,11 @@ export class PlannerAttachments { return this.#holds.get(channelId)?.link !== undefined; } + /** Whether the client attached to the document waits on its members: an open question or run. */ + waiting(channelId: string): boolean { + return this.#holds.get(channelId)?.link?.waiting ?? false; + } + /** Waits at most `ms` for a client to attach to the document; whether one is attached then. */ attachedWithin(channelId: string, ms: number): Promise { if (this.attached(channelId)) return Promise.resolve(true); diff --git a/apps/server/src/harness/remote/link.test.ts b/apps/server/src/harness/remote/link.test.ts index 533ae8240..94013f677 100644 --- a/apps/server/src/harness/remote/link.test.ts +++ b/apps/server/src/harness/remote/link.test.ts @@ -13,11 +13,12 @@ import { AgentRefused } from "@chopin/agent-link/client"; import { GitHubError } from "../../github/client"; import * as Store from "../../questions/store"; +import * as Plan from "../../plan/service"; import { createPlannerAgent } from "../agents"; import { createHumanInput } from "../atomic/human-input"; import { openPlannerSession } from "../session"; import { untilUnpaused } from "../atomic/full"; -import { hostInputRoom } from "../../testing/decisions"; +import { hostInputRoom, laterTimers } from "../../testing/decisions"; import { attach, DENIED_TOKEN, @@ -1278,8 +1279,8 @@ describe("Agent connection protocol", () => { }); describe("Agent connection questions", () => { - async function questions(options: { expiresInMs?: number; isolated?: boolean } = {}) { - let f = await hostInputRoom(options.expiresInMs); + async function questions(options: { isolated?: boolean } = {}) { + let f = await hostInputRoom(); cleanups.push(f.close); let { server, channel, client } = await setup({}); let planner: RemotePlanner = { channelId: channel.id, isolated: options.isolated ?? false }; @@ -1287,7 +1288,6 @@ describe("Agent connection questions", () => { planner.humanInput = report => createHumanInput( f.room, - options.expiresInMs, (runId, signal) => untilUnpaused(planner, runId, signal), report, ); @@ -1345,13 +1345,42 @@ describe("Agent connection questions", () => { expect(await confirm).toMatchObject({ status: "answered", value: true }); }); - it("expires an unanswered question and leaves its card in Decisions marked expired", async () => { - let { f, client, sessionId } = await questions({ expiresInMs: 30 }); - let asked = await client.ask(sessionId, { method: "questionnaire", params }); - expect(asked).toMatchObject({ status: "expired", value: { answers: [], cancelled: true } }); - let records = [...f.plan.records.values()]; - expect(records).toHaveLength(1); - expect(records[0]).toMatchObject({ status: "expired", resolver: "chopin" }); + it("keeps a client's question open past half an hour until a member answers it", async () => { + let { f, client, sessionId } = await questions(); + let clock = laterTimers(); + try { + let settled = false; + let asked = client.ask(sessionId, { method: "confirm", title: "Ship?", message: "Now." }) + .finally(() => { + settled = true; + }); + let [card] = await f.cards(1); + await clock.pass(31 * 60 * 1_000); + expect(settled).toBe(false); + expect(f.plan.records.get(card!.id)?.status).toBe("open"); + await f.answer(card!.id, [0]); + expect(await asked).toMatchObject({ status: "answered", value: true }); + } finally { + clock.restore(); + } + }); + + it("withdraws a question open past half an hour when the link closes", async () => { + let { f, client, sessionId } = await questions(); + let clock = laterTimers(); + try { + void client.ask(sessionId, { method: "confirm", title: "Ship?", message: "Now." }) + .catch(() => {}); + let [card] = await f.cards(1); + await clock.pass(31 * 60 * 1_000); + expect(f.plan.records.get(card!.id)?.status).toBe("open"); + await client.detach(); + await until(() => Store.outstanding(f.plan.questions).length === 0, 2_000); + expect(f.plan.records.get(card!.id)).toMatchObject({ status: "cancelled" }); + expect(Plan.source(f.plan)).not.toContain(card!.id); + } finally { + clock.restore(); + } }); it("withdraws the cards when the client cancels or the session ends", async () => { @@ -1413,7 +1442,7 @@ describe("Agent connection questions", () => { let opened = await adapterFor(server, { channelId: channel.id, isolated: false, - humanInput: report => createHumanInput(f.room, undefined, undefined, report), + humanInput: report => createHumanInput(f.room, undefined, report), }).doStart({ sessionId: crypto.randomUUID(), sandboxSession, sessionWorkDir: "/tmp" }); cleanups.push(() => opened.doDestroy()); let request = { @@ -1480,7 +1509,7 @@ describe("Agent connection questions", () => { let planner: RemotePlanner = { channelId: f.room.id, isolated: false, - humanInput: report => createHumanInput(f.room, undefined, undefined, report), + humanInput: report => createHumanInput(f.room, undefined, report), }; let harness = adapterFor(linked.server, planner); let start = (sessionId: string) => diff --git a/apps/server/src/harness/remote/link.ts b/apps/server/src/harness/remote/link.ts index 3f2487a72..016d72983 100644 --- a/apps/server/src/harness/remote/link.ts +++ b/apps/server/src/harness/remote/link.ts @@ -585,6 +585,20 @@ export class AgentLink { return this.#state === "attached"; } + /** + * Whether the client waits on the document's members: a question it asked is still open, or + * one of its workflow runs is live or paused. + */ + get waiting(): boolean { + if (this.#state === "closed") return false; + if (this.#inputs.size > 0) return true; + for (let session of this.#sessions.values()) { + let runs = session.planner.runs; + if (runs && (runs.active.length > 0 || runs.paused.length > 0)) return true; + } + return false; + } + /** Opens a Planner session on the client; throws when it cannot. */ async startSession(sessionId: string, planner: RemotePlanner): Promise { if (this.#state !== "attached") throw new Error("The agent connection is not attached."); diff --git a/apps/server/src/harness/remote/planner-dispatch.test.ts b/apps/server/src/harness/remote/planner-dispatch.test.ts index c5681786c..70a0fd870 100644 --- a/apps/server/src/harness/remote/planner-dispatch.test.ts +++ b/apps/server/src/harness/remote/planner-dispatch.test.ts @@ -16,8 +16,12 @@ import { rawLink, upgradeRequest, } from "../../testing/agent-link"; +import { createHumanInput } from "../atomic/human-input"; +import { hostInputRoom } from "../../testing/decisions"; import { MachinePlanners } from "./machine-planners"; +import type { RemotePlanner } from "./registry"; + let cleanups: Array<() => unknown> = []; afterEach(async () => { for (let cleanup of cleanups.splice(0).toReversed()) await cleanup(); @@ -637,6 +641,75 @@ describe("a document on Default machine", () => { stopped: { [channel.id]: expect.stringContaining("Nobody has used this document") }, }); }); + + /** A Default machine Planner, attached and running a session, on a document nobody has open. */ + async function unwatched() { + let dispatch = await dispatchServer(); + let { machine, visit, channel, server, runtime, open } = dispatch; + let laptop = await machine("member", "U_member", "laptop"); + await visit(); + await laptop.poll([]); + let link = await laptop.backend(); + await link.next("attached"); + let room = await hostInputRoom(); + cleanups.push(room.close); + let planner: RemotePlanner = { + channelId: channel.id, + isolated: false, + humanInput: report => createHumanInput(room.room, undefined, report), + }; + let started = link.next("start").then(start => { + link.send({ type: "started", session: start.session }); + }); + let session = await server.links.at(-1)!.startSession("planner-session", planner); + cleanups.push(() => session.doDestroy?.()); + await started; + open.delete(channel.id); + await runtime.sweep(); + /** Lets `ms` go by on the server's clock with nobody viewing, and whether it still runs. */ + let idle = async (ms: number) => { + dispatch.skew.ms += ms; + await runtime.sweep(); + let answer = await laptop.poll([channel.id]); + return "waiting" in answer; + }; + return { ...dispatch, laptop, link, room, idle }; + } + + it("keeps the Planner while it waits on a question, and stops it once that has been answered for a while", async () => { + let { link, room, idle } = await unwatched(); + link.send({ + type: "input-request", + request: "ship", + session: "planner-session", + input: { method: "confirm", title: "Ship?", message: "Now." }, + }); + let [card] = await room.cards(1); + expect(await idle(PLANNER_IDLE_MS + 60_000)).toBe(true); + expect(await idle(PLANNER_IDLE_MS * 4)).toBe(true); + expect(room.plan.records.get(card!.id)?.status).toBe("open"); + + await room.answer(card!.id, [0]); + expect(await link.next("input-result")).toMatchObject({ status: "answered", value: true }); + expect(await idle(PLANNER_IDLE_MS - 1_000)).toBe(true); + expect(await idle(2_000)).toBe(false); + expect(await link.next("refused")).toMatchObject({ code: "access-revoked" }); + }); + + it("keeps the Planner while one of its workflow runs is live or paused", async () => { + let { link, idle } = await unwatched(); + let runs = async (active: string[], paused: string[]) => { + link.send({ type: "runs", session: "planner-session", active, paused, cards: [] }); + await Bun.sleep(50); + }; + await runs(["root"], []); + expect(await idle(PLANNER_IDLE_MS + 60_000)).toBe(true); + await runs([], ["root"]); + expect(await idle(PLANNER_IDLE_MS + 60_000)).toBe(true); + await runs([], []); + expect(await idle(PLANNER_IDLE_MS - 1_000)).toBe(true); + expect(await idle(2_000)).toBe(false); + }); }); describe("a machine's GitHub token", () => { diff --git a/apps/server/src/harness/session.test.ts b/apps/server/src/harness/session.test.ts index ab22c581c..c40cbc20c 100644 --- a/apps/server/src/harness/session.test.ts +++ b/apps/server/src/harness/session.test.ts @@ -8,8 +8,7 @@ import { forgetWorkspaces, rememberCheckout, stateDirectory } from "./atomic/wor import { plannerInstructions } from "../agent/planner"; import { BACKGROUND_TOOL_NAMES, PLANNER_TOOL_NAMES, VISUAL_PLANNER_TOOL_NAMES } from "./tool-names"; import { remotePlanner } from "./remote/registry"; -import { hostInputRoom } from "../testing/decisions"; -import { limits } from "@chopin/question"; +import { hostInputRoom, laterTimers } from "../testing/decisions"; import type { ActiveOwnerBinding } from "../agent/active-owner"; import type { PlannerSessionDependencies } from "./session"; @@ -118,22 +117,19 @@ describe("openPlannerSession", () => { expect(instructions).toContain("You run in a member's own session on their machine"); expect(instructions).toContain("BOOTSTRAP"); - let delays: unknown[] = []; - let original = globalThis.setTimeout; - globalThis.setTimeout = ((handler: () => void, ms?: number) => { - delays.push(ms); - return original(handler, ms); - }) as typeof setTimeout; + let clock = laterTimers(); let asked: Promise; try { asked = registered.humanInput!(() => {}).confirm("Ship?", "Now.", room.options); - await room.cards(1); + let [card] = await room.cards(1); + await clock.pass(31 * 60 * 1_000); + expect(room.plan.records.get(card!.id)?.status).toBe("open"); + room.controller.abort(); + expect(await asked).toBe(false); + expect(room.plan.records.get(card!.id)?.status).toBe("cancelled"); } finally { - globalThis.setTimeout = original; + clock.restore(); } - expect(delays).toContain(limits.INPUT_EXPIRY_MS); - room.controller.abort(); - expect(await asked).toBe(false); let seen: unknown[] = []; session.watchRuns!(runs => seen.push(runs)); @@ -321,7 +317,7 @@ describe("Planner workspaces", () => { expect({ attempt, cwd: registered?.cwd }).toEqual({ attempt, cwd: path }); expect(registered?.humanInput.questionnaire).toBeFunction(); expect(instructions).toContain(`${path}, is a local checkout of owner/repo`); - expect(instructions).toContain("Expiry is not an answer or approval"); + expect(instructions).toContain("a withdrawn question is not an answer or approval"); expect(instructions).toContain("never tell them to use `/workflow connect`"); expect(instructions).toContain("Chat shows a card for each workflow run you start"); expect(instructions).toContain("`intercom` tool can reach other sessions"); @@ -339,6 +335,43 @@ describe("Planner workspaces", () => { expect(fallback.instructions).toContain("scratch directory Chopin keeps"); }); + it("keeps an atomic Planner's question open past half an hour until a member answers it", async () => { + let { owner, channel, deps } = fixture(); + let room = await hostInputRoom(); + let sessionId = ""; + let registered: ReturnType; + deps.agent = { + createSession: async (options: { sessionId: string }) => { + sessionId = options.sessionId; + return { destroy: async () => {} }; + }, + stream: async () => { + registered = fullPlanner(sessionId); + }, + } as never; + let opened = await openPlannerSession(owner, { + ...channel, + room: room.room, + harness: "atomic", + instructions: workspace => plannerInstructions("owner/repo", "BOOTSTRAP", workspace), + }, deps); + if (!opened.ok) throw new Error("Planner unavailable"); + let clock = laterTimers(); + try { + await opened.value.stream("prompt", new AbortController().signal); + let asked = registered!.humanInput.confirm("Ship?", "Now.", room.options); + let [card] = await room.cards(1); + await clock.pass(31 * 60 * 1_000); + expect(room.plan.records.get(card!.id)?.status).toBe("open"); + await room.answer(card!.id, [0]); + expect(await asked).toBe(true); + } finally { + clock.restore(); + await opened.value.destroy(); + await room.close(); + } + }); + it("gives each atomic channel without a checkout its own private directory that outlives shutdown", async () => { let first = await open("first", "atomic"); let again = await open("first", "atomic"); @@ -356,7 +389,7 @@ describe("Planner workspaces", () => { expect(instructions).toContain(`${cwd}, is a scratch directory Chopin keeps`); expect(instructions).toContain("holds no repository files"); expect(instructions).toContain("`read_repository_file`"); - expect(instructions).toContain("Expiry is not an answer or approval"); + expect(instructions).toContain("a withdrawn question is not an answer or approval"); expect(instructions).toContain("never tell them to use `/workflow connect`"); expect(instructions).toContain("Chat shows a card for each workflow run you start"); expect(instructions).toContain("one working in a checkout of owner/repo"); @@ -381,7 +414,7 @@ describe("Planner workspaces", () => { let { registered, instructions } = await open("channel", harness); expect(registered).toBeUndefined(); expect(instructions).toContain("You have no shell"); - expect(instructions).not.toContain("Expiry is not an answer or approval"); + expect(instructions).not.toContain("a withdrawn question is not an answer or approval"); expect(instructions).not.toContain("/workflow connect"); expect(instructions).not.toContain("intercom"); } diff --git a/apps/server/src/harness/session.ts b/apps/server/src/harness/session.ts index abbed423c..def0e2578 100644 --- a/apps/server/src/harness/session.ts +++ b/apps/server/src/harness/session.ts @@ -166,7 +166,6 @@ export async function openPlannerSession( cwd: workspace.cwd, humanInput: createHumanInput( channel.room, - undefined, (runId, signal) => untilUnpaused(planner!, runId, signal), ), onRuns: runs => { @@ -187,7 +186,6 @@ export async function openPlannerSession( held.humanInput = report => createHumanInput( channel.room, - undefined, (runId, signal) => untilUnpaused(held, runId, signal), report, ); diff --git a/apps/server/src/plan/edit.ts b/apps/server/src/plan/edit.ts index 64d2b9715..e52250ab9 100644 --- a/apps/server/src/plan/edit.ts +++ b/apps/server/src/plan/edit.ts @@ -28,8 +28,6 @@ import * as room from "./room"; import type { Nodes, Root, RootContent } from "mdast"; import type { Plan } from "./service"; -const EXPIRY_ROOM = "The plan would leave no room for the open questions to expire."; - /** Enough leading text to tell two blocks apart without resending the source. */ const PREVIEW = 120; @@ -206,9 +204,6 @@ export function apply(plan: Plan, revision: number, operations: Operation[]): Re let parsed = parse(next); assert(parsed, { bytes: new TextEncoder().encode(next).byteLength }); assertIntroducedUrls(root.children, parsed.children); - if (!room.fitsOrShrinks(source(plan), next, plan.questions.open.size)) { - throw new Error(EXPIRY_ROOM); - } if (parsed.children.length !== children.length) { throw new Error("plan blocks merge or split during Markdown normalisation"); } @@ -293,9 +288,6 @@ export function replace(plan: Plan, revision: number, nextSource: string): Resul let parsed = parse(next); assert(parsed, { bytes: new TextEncoder().encode(next).byteLength }); assertIntroducedUrls(root.children, parsed.children); - if (!room.fitsOrShrinks(source(plan), next, plan.questions.open.size)) { - throw new Error(EXPIRY_ROOM); - } if (parsed.children.length !== children.length) { throw new Error("plan blocks merge or split during Markdown normalisation"); } diff --git a/apps/server/src/plan/room.test.ts b/apps/server/src/plan/room.test.ts index 8ebed5a8d..1268933bd 100644 --- a/apps/server/src/plan/room.test.ts +++ b/apps/server/src/plan/room.test.ts @@ -386,46 +386,43 @@ describe("recovery", () => { } }); - it("with questions open, refuses a human edit that grows into the expiry reserve but allows one that shrinks", async () => { - let filler = "x".repeat(limits.MAX_SOURCE_BYTES - 40); - let document = await room.create(`# T\n\n${filler}\n\nTail.\n`); - room.mark(document); - expect(room.fitsExpiry(room.project(document), 1)).toBe(false); - - let client = peer(); - Y.applyUpdate(client.doc, room.sync(document), "remote"); - await room.settle(); - - let before = Y.encodeStateVector(client.doc); - client.editor.update(() => { - let paragraph = $createParagraphNode(); - paragraph.append($createTextNode("More.")); - $getRoot().append(paragraph); - }, { discrete: true }); - let grown = await room.apply( - document, - [Y.encodeStateAsUpdate(client.doc, before)], - undefined, - 1, - ); - expect(grown.ok).toBe(false); - - let fresh = await room.rebuild(document); - let shrinker = peer(); - Y.applyUpdate(shrinker.doc, room.sync(fresh), "remote"); - await room.settle(); - let start = Y.encodeStateVector(shrinker.doc); - shrinker.editor.update(() => { - $getRoot().getLastChild()?.remove(); - }, { discrete: true }); - let shrunk = await room.apply( - fresh, - [Y.encodeStateAsUpdate(shrinker.doc, start)], - undefined, - 1, - ); - expect(shrunk.ok).toBe(true); - expect(room.project(fresh)).not.toContain("Tail."); + it("with questions open, accepts a human edit that fills the document to its exact limit and refuses one past it", async () => { + let max = limits.MAX_SOURCE_BYTES; + /** A person appends a paragraph while three questions are open. */ + let append = async (filler: number) => { + let document = await room.create(`# T\n\n${"x".repeat(filler)}\n\nTail.\n`); + room.mark(document); + let before = Buffer.byteLength(room.project(document)); + let client = peer(); + Y.applyUpdate(client.doc, room.sync(document), "remote"); + await room.settle(); + let vector = Y.encodeStateVector(client.doc); + client.editor.update(() => { + let paragraph = $createParagraphNode(); + paragraph.append($createTextNode("More.")); + $getRoot().append(paragraph); + }, { discrete: true }); + let applied = await room.apply( + document, + [Y.encodeStateAsUpdate(client.doc, vector)], + undefined, + 3, + ); + let after = Buffer.byteLength(room.project(document)); + document.doc.destroy(); + return { applied, before, after }; + }; + let probe = await append(10); + expect(probe.applied.ok).toBe(true); + let growth = probe.after - probe.before; + let filler = max - (probe.before - 10) - growth; + + let filled = await append(filler); + expect(filled.applied.ok).toBe(true); + expect(filled.after).toBe(max); + + let past = await append(filler + 1); + expect(past.applied).toEqual({ ok: false, issues: ["source-too-large"] }); }); // Projection and its re-parse run on the event loop. Brackets used to make diff --git a/apps/server/src/plan/room.ts b/apps/server/src/plan/room.ts index e9569b23d..70ea02ddb 100644 --- a/apps/server/src/plan/room.ts +++ b/apps/server/src/plan/room.ts @@ -415,7 +415,9 @@ export async function apply( return { ok: false, issues: ["protected-projection"] }; } let source = project(target); - if (open > 0 && Buffer.byteLength(source) > held && !fitsExpiry(source, open)) { + // With questions open, a person may not take the document past its limit, which would + // leave the Planner unable to edit it around them. + if (open > 0 && Buffer.byteLength(source) > held && !fits(source)) { return { ok: false, issues: ["source-too-large"] }; } } catch (err) { @@ -622,52 +624,29 @@ export function insertQuestionnaires( }, insertions); } -/** What expiring one open questionnaire adds to its card: the status and the date. */ -const EXPIRY_BYTES = (() => { - let size = (value: Questionnaire) => - Buffer.byteLength(serialize({ type: "root", children: [questionnaireElement(value)] })); - let card = { id: "", questions: [] }; - return size({ ...card, status: "expired", at: new Date(0).toISOString() }) - size(card); -})(); - -/** - * Whether this source still has room for `open` questionnaires to expire. - * - * Expiry adds to a card the document already holds, and a document past its - * limit stops accepting Planner edits, so every change leaves this room. - */ -export function fitsExpiry(source: string, open: number): boolean { - return Buffer.byteLength(source) + open * EXPIRY_BYTES <= limits.MAX_SOURCE_BYTES; +/** Whether this source is within the document's byte limit. */ +function fits(source: string): boolean { + return Buffer.byteLength(source) <= limits.MAX_SOURCE_BYTES; } /** - * Whether a change from `before` to `after` may proceed with `open` questionnaires - * waiting: it may always shrink the source, and may grow it only while the - * reserve for their expiry still fits. + * Whether a change from `before` to `after` may proceed: it may always shrink + * the source, and may grow it only within the document's byte limit. */ -export function fitsOrShrinks(before: string, after: string, open: number): boolean { - return Buffer.byteLength(after) <= Buffer.byteLength(before) || fitsExpiry(after, open); +export function fitsOrShrinks(before: string, after: string): boolean { + return Buffer.byteLength(after) <= Buffer.byteLength(before) || fits(after); } /** - * Whether appending these questionnaires keeps the source within its byte limit - * once they, and the `open` questionnaires already waiting, have expired. + * Whether appending these questionnaires keeps the source within its byte limit. * * Measured before mutating, because Yjs cannot undo an insertion that turns out * to be too large. */ -export function fitsQuestionnaires( - target: Document, - values: Questionnaire[], - open = 0, -): boolean { +export function fitsQuestionnaires(target: Document, values: Questionnaire[]): boolean { let tree = parse(project(target)); - let at = new Date().toISOString(); - // Reserve the metadata expiry adds to every card in this batch. - tree.children.push( - ...values.map(value => questionnaireElement({ ...value, status: "expired", at })), - ); - return fitsExpiry(serialize(tree), open); + tree.children.push(...values.map(value => questionnaireElement(value))); + return fits(serialize(tree)); } /** Append one questionnaire to the plan. */ @@ -746,19 +725,6 @@ export function appendQuestionOption( }); } -/** Mark host input nobody answered in time as expired, leaving the card where it is. */ -export function projectExpiry(target: Document, id: string, at: string): Mutation | undefined { - return mutate(target, () => { - let found = false; - for (let node of $nodesOfType(QuestionnaireNode)) { - if (node.getId() !== id) continue; - found = true; - node.setQuestionnaire({ ...node.getQuestionnaire(), status: "expired", at }); - } - return found; - }); -} - /** Take a questionnaire out of the plan, leaving its record as history. */ export function removeQuestionnaire(target: Document, id: string): Mutation | undefined { return mutate(target, () => { diff --git a/apps/server/src/questions/service-append-option.ts b/apps/server/src/questions/service-append-option.ts index ba44917f0..bf4376b5e 100644 --- a/apps/server/src/questions/service-append-option.ts +++ b/apps/server/src/questions/service-append-option.ts @@ -139,14 +139,7 @@ export async function appendOption( }; let mutation = room.appendQuestionOption(stagedDocument, msg.id, msg.question, result.option); if (!mutation) throw new Error("question projection is missing"); - // Every open question needs room to expire, so growth stops where that room ends. - if ( - !room.fitsOrShrinks( - room.project(plan.document), - room.project(stagedDocument), - plan.questions.open.size, - ) - ) { + if (!room.fitsOrShrinks(room.project(plan.document), room.project(stagedDocument))) { outcome = refuse("invalid", "The document has no room for another option"); return; } diff --git a/apps/server/src/questions/service-ask.ts b/apps/server/src/questions/service-ask.ts index 18bbb3e48..de8dc3af5 100644 --- a/apps/server/src/questions/service-ask.ts +++ b/apps/server/src/questions/service-ask.ts @@ -22,14 +22,14 @@ import type { Ended } from "./store"; import type { Record } from "./records"; import { announce } from "./card-notifications"; -import { expire, withdraw } from "./service-cancel"; +import { withdraw } from "./service-cancel"; import { type AskPlacement, validatePlacement } from "./service-definition"; /** * Ask each decision independently; register its record before publishing its node. * - * An abort withdraws the cards still open. After `expiresInMs` without an answer - * they expire instead, and stay in the document marked as such. + * The cards wait until members answer them, however long that takes; an abort + * withdraws the ones still open. */ export async function ask( plan: Plan, @@ -39,7 +39,6 @@ export async function ask( placement?: AskPlacement, created?: () => void, signal?: AbortSignal, - expiresInMs?: number, ): Promise { if (signal?.aborted) return []; if (Service.implementationActive(plan)) throw new Error("implementation is active"); @@ -149,15 +148,9 @@ export async function ask( records.set(id, record); return { id, single, value, waiting, at: placement?.blocks[index]?.[0] }; }); - // Verbatim host input has no per-field limits, and every card needs room to expire. Refuse before - // anything is registered or published. - if ( - !room.fitsQuestionnaires( - plan.document, - asked.map(item => item.value), - plan.questions.open.size, - ) - ) { + // Verbatim host input has no per-field limits. Refuse before anything is registered or + // published. + if (!room.fitsQuestionnaires(plan.document, asked.map(item => item.value))) { Question.reject( `This input would take the document past its ${limits.MAX_SOURCE_BYTES / 1024} KiB limit`, ); @@ -202,28 +195,19 @@ export async function ask( let failed = Promise.withResolvers(); let closing: Promise | undefined; - let close = (status: "cancelled" | "expired") => { + let abort = () => { closing ??= Promise.all( - asked.map(item => - status === "expired" - ? expire(plan, server, roomId, item.id) - : withdraw(plan, server, roomId, item.id, "chopin") - ), + asked.map(item => withdraw(plan, server, roomId, item.id, "chopin")), ); void closing.catch(failed.reject); }; - let abort = () => close("cancelled"); signal?.addEventListener("abort", abort, { once: true }); if (signal?.aborted) abort(); - let timer = expiresInMs === undefined - ? undefined - : setTimeout(() => close("expired"), expiresInMs); try { let ended = await Promise.race([Promise.all(asked.map(item => item.waiting)), failed.promise]); await closing; return ended; } finally { - clearTimeout(timer); signal?.removeEventListener("abort", abort); } } diff --git a/apps/server/src/questions/service-cancel.ts b/apps/server/src/questions/service-cancel.ts index 4e393ab77..32e66be77 100644 --- a/apps/server/src/questions/service-cancel.ts +++ b/apps/server/src/questions/service-cancel.ts @@ -46,7 +46,7 @@ export async function cancel( if (!acknowledged) reply(ws, msg.rid, { kind: "question:cancel", ts: 0, id: msg.id, ...result }); } -type Closed = { status: "cancelled" | "expired"; resolver: string; at?: number }; +type Closed = { status: "cancelled"; resolver: string }; /** * Withdraw input on behalf of a member or the Planner, after its durable commit. @@ -71,29 +71,6 @@ export function withdraw( ); } -/** - * Expire host input nobody answered in time, after its durable commit. - * - * Unlike a withdrawal the card stays in the document, marked expired, so the - * room can still see what was asked and that nobody answered it. - */ -export function expire( - plan: Plan, - server: Server, - roomId: string, - id: string, -): Promise { - let at = Math.floor(Date.now() / 1_000); - return closeUnanswered( - plan, - server, - roomId, - id, - { status: "expired", resolver: "chopin", at }, - document => room.projectExpiry(document, id, new Date(at * 1_000).toISOString()), - ); -} - async function closeUnanswered( plan: Plan, server: Server, @@ -104,7 +81,7 @@ async function closeUnanswered( acknowledge?: () => void, ): Promise { let { status, resolver } = closed; - let claimed = Store.claimCancel(plan.questions, id, resolver, status); + let claimed = Store.claimCancel(plan.questions, id, resolver); if (!claimed.ok) return claimed; let mutationError: unknown; let failure: unknown; @@ -145,7 +122,7 @@ async function closeUnanswered( document, questions, records, - pendingCardActions: record && status === "cancelled" + pendingCardActions: record ? pending(plan, record, { kind: "discarded", id, @@ -182,15 +159,13 @@ async function closeUnanswered( resolver, }), () => announce(plan, server, roomId, id), - ...(status === "cancelled" - ? [() => - emit(plan, { - kind: "discarded", - id, - ...(record?.threadId ? { threadId: record.threadId } : {}), - actor: resolver, - })] - : []), + () => + emit(plan, { + kind: "discarded", + id, + ...(record?.threadId ? { threadId: record.threadId } : {}), + actor: resolver, + }), ] ) { try { diff --git a/apps/server/src/questions/service-submit.ts b/apps/server/src/questions/service-submit.ts index 4b63e51aa..f80733ccf 100644 --- a/apps/server/src/questions/service-submit.ts +++ b/apps/server/src/questions/service-submit.ts @@ -104,11 +104,6 @@ export async function submit( try { let source = room.project(stagedDocument); room.validate(source); - if ( - !room.fitsOrShrinks(room.project(plan.document), source, plan.questions.open.size - 1) - ) { - throw new Error("The answer would leave no room for the open questions to expire"); - } } catch (err) { invalid = err instanceof Error ? err.message : "could not record the answer"; return; diff --git a/apps/server/src/questions/service.ts b/apps/server/src/questions/service.ts index 4359e24b3..a11102554 100644 --- a/apps/server/src/questions/service.ts +++ b/apps/server/src/questions/service.ts @@ -32,7 +32,7 @@ export { export type { DecisionEntry, OptionOrigin, Record } from "./records"; export { appendOption as addOption } from "./service-append-option"; export { ask } from "./service-ask"; -export { cancel, expire, withdraw } from "./service-cancel"; +export { cancel, withdraw } from "./service-cancel"; export { identify } from "./service-definition"; export type { AskPlacement } from "./service-definition"; export { discard } from "./service-discard"; diff --git a/apps/server/src/questions/store.ts b/apps/server/src/questions/store.ts index b0f04240a..44359de1b 100644 --- a/apps/server/src/questions/store.ts +++ b/apps/server/src/questions/store.ts @@ -379,12 +379,11 @@ export function claimSubmit( }; } -/** Reserve an unanswered close: a cancellation, or an expiry nobody answered in time. */ +/** Reserve an unanswered close: a cancellation. */ export function claimCancel( questions: Questions, id: string, resolver: string, - status: "cancelled" | "expired" = "cancelled", ): { ok: true; claim: Claim; widget?: string } | CancelRefusal { let ended = questions.closed.get(id); if (ended) return resolved(ended); @@ -399,7 +398,7 @@ export function claimCancel( return { ok: true, ...(entry.widget ? { widget: entry.widget } : {}), - claim: { id, entry, result: { status, resolver } }, + claim: { id, entry, result: { status: "cancelled", resolver } }, }; } diff --git a/apps/server/src/testing/decisions.ts b/apps/server/src/testing/decisions.ts index da63f903d..1625ca583 100644 --- a/apps/server/src/testing/decisions.ts +++ b/apps/server/src/testing/decisions.ts @@ -11,9 +11,44 @@ import type { HostInputOptions } from "@bastani/atomic"; import type { DocumentRoom } from "../agent/tools"; import type { Socket, SocketData } from "../wire"; +/** + * Holds back every timer armed for a minute or more until `pass` lets that much time go by, so a + * test can let hours pass at once while shorter timers, the room's own work included, run as usual. + */ +export function laterTimers() { + let original = { set: globalThis.setTimeout, clear: globalThis.clearTimeout }; + let held = new Map void }>(); + let elapsed = 0; + globalThis.setTimeout = ((handler: (...args: unknown[]) => void, ms = 0, ...args: unknown[]) => { + if (ms < 60_000) return original.set(handler, ms, ...args); + let handle = {}; + held.set(handle, { at: elapsed + ms, run: () => handler(...args) }); + return handle; + }) as typeof setTimeout; + globalThis.clearTimeout = ((handle?: Parameters[0]) => { + if (handle && typeof handle === "object" && held.delete(handle)) return; + original.clear(handle); + }) as typeof clearTimeout; + return { + /** Lets `ms` go by: every held timer due by then fires, then the room settles. */ + async pass(ms: number) { + elapsed += ms; + for (let [handle, timer] of held) { + if (timer.at > elapsed) continue; + held.delete(handle); + timer.run(); + } + await Bun.sleep(20); + }, + restore() { + globalThis.setTimeout = original.set; + globalThis.clearTimeout = original.clear; + }, + }; +} + /** A room whose Decisions answer Atomic HostInput requests, with a member to answer them. */ export async function hostInputRoom( - expiresInMs?: number, hold?: (workflowRunId: string, signal: AbortSignal) => Promise, ) { let storage = new MemoryStorage(); @@ -63,7 +98,7 @@ export async function hostInputRoom( sessionId: "session", signal: controller.signal, }; - let input = createHumanInput(room, expiresInMs, hold); + let input = createHumanInput(room, hold); async function cards(count: number) { for (let deadline = Date.now() + 4_000; Date.now() < deadline; await Bun.sleep(5)) { if (Store.outstanding(plan.questions).length === count) { diff --git a/docs/agent-connection.md b/docs/agent-connection.md index d10cd9adf..e5aab1b41 100644 --- a/docs/agent-connection.md +++ b/docs/agent-connection.md @@ -243,13 +243,13 @@ rests on ends or the document's Planner is reset. The server then sends `refused` and closes an attached connection at once, and any eligible account, including the same one, can attach again immediately: -| Cause | Refusal code | Close code | -| ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------ | ---------- | -| The owner's browser sign-in signs out or expires | `sign-in-required` | 4401 | -| The machine whose credential attached it is disconnected, or GitHub refuses its authorization | `access-revoked` | 4403 | -| Chopin started it on a machine and moved it to another, or stopped it: a new machine choice or favorite, the document unused for 30 minutes, or the machine lost push access | `access-revoked` | 4403 | -| A repository writer resets the document's Planner | `access-revoked` | 4403 | -| The stored ownership no longer names this attachment's sign-in | `access-revoked` | 4403 | +| Cause | Refusal code | Close code | +| -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------ | ---------- | +| The owner's browser sign-in signs out or expires | `sign-in-required` | 4401 | +| The machine whose credential attached it is disconnected, or GitHub refuses its authorization | `access-revoked` | 4403 | +| Chopin started it on a machine and moved it to another, or stopped it: a new machine choice or favorite, the document unused for 30 minutes (a Planner waiting on a question or a live or paused run is using it), or the machine lost push access | `access-revoked` | 4403 | +| A repository writer resets the document's Planner | `access-revoked` | 4403 | +| The stored ownership no longer names this attachment's sign-in | `access-revoked` | 4403 | The same checks before every `start` and `turn`, and the periodic one, also find an expired sign-in or a released ownership that nothing reported; an @@ -511,7 +511,9 @@ the host tool returned it, and a failed host call keeps its ## Questions An `input-request` carries one question call in `input`, and becomes Decisions -in the document: +in the document. It has no time limit: it waits until members answer it or it +is withdrawn (the client sends `input-cancel`, the session ends, or the +connection closes), and only then gets its `input-result`: ```text { "method": "questionnaire", "params": { "questions": [{ "question": "", "header": "", "options": [{ "label": "", "description": "", "preview?": "" }], "multiSelect?": true }] } } @@ -538,8 +540,11 @@ still apply. Text is shown verbatim. - `answered`: members answered every question. `confirm` is `true` only for **Yes**. `select` is the chosen string from `choices`, or `null` when a member wrote an answer instead. `input` and `editor` are the text a member wrote. -- `expired`: nobody answered within 30 minutes. The cards stay in Decisions - marked expired and no partial answers are returned. +- `expired`: an earlier Chopin closed the question unanswered after a fixed time. + Chopin no longer sends it: a question waits until a member answers it or it is + withdrawn, however long that takes. A client may still accept it from an older + server, where the cards stayed in Decisions marked expired and no partial + answers were returned. - `cancelled`: a member cancelled a question, the client sent `input-cancel`, the question was withdrawn before it was asked, the session ended, or the connection closed. Open cards are withdrawn. When a member cancelled one question of a diff --git a/docs/chopin-cli.md b/docs/chopin-cli.md index 23f47f6e0..726a2f45f 100644 --- a/docs/chopin-cli.md +++ b/docs/chopin-cli.md @@ -457,6 +457,8 @@ the person who created it, picked as for builds: their default machine when it is connected and can run Planners, else the machine that last worked on the document, else any of theirs that can. Chopin asks for it while someone has the document open, and stops it once nobody has used the document for 30 minutes. +A Planner waiting on a question or on a live or paused workflow run counts as +use, so its questions stay open however long nobody views the document. A Planner stays on its machine while that machine is connected, even when another comes online; it moves when the machine disconnects or the creator changes the document's machine or their default. Chat says diff --git a/docs/hosted-agent.md b/docs/hosted-agent.md index 01cce7a67..7eadaebd3 100644 --- a/docs/hosted-agent.md +++ b/docs/hosted-agent.md @@ -309,13 +309,15 @@ extension dialogs, and workflow-stage input become ordinary shared Decisions: - An abort withdraws still-open cards, removes their document nodes, and records cancellation by `@chopin`. Member cancellations retain any answers already given in that batch. Late submissions cannot approve withdrawn input. -- A request nobody answers within 30 minutes expires. Its still-open cards stay - in the document and in Decisions, among the resolved cards, marked - `status="expired"`; each reads "Nobody answered within 30 minutes. The - Planner will use its best judgement for this decision." Nobody can answer an - expired card. Atomic receives no answer: a questionnaire comes back cancelled - with no answers, confirm returns false, and select, input, and editor return - nothing. The limit is fixed in code, not configured. +- A request has no time limit. It waits until members answer it or it is + withdrawn: the Planner stops, its run is cancelled, the document is archived + or deleted, or the asker's link closes. A document on **Default machine** + keeps its Planner running while that Planner waits on a question or a live or + paused workflow run, so nobody viewing the document does not withdraw it. +- Cards an earlier Chopin closed unanswered after a fixed time stay in the + document and in Decisions, among the resolved cards, marked + `status="expired"`; each reads "Nobody answered this question, so the Planner + used its best judgement for this decision." Nobody can answer an expired card. Host input is not held to the Planner `ask` tool's per-field limits on question and option counts or header, question, label, description, and answer lengths. diff --git a/docs/local-agent-mcp.md b/docs/local-agent-mcp.md index 2da1a783a..f506f2452 100644 --- a/docs/local-agent-mcp.md +++ b/docs/local-agent-mcp.md @@ -149,8 +149,8 @@ The result contains `id`, `title`, and the canonical `url` (plus any generated `description`). It returns after the instruction is durably posted, not after the Planner finishes. The message joins the existing Planner queue if busy. Progress and results appear in the document and Chat; questions appear in Decisions. Under -`atomic`, Atomic input nobody answers within 30 minutes expires: the card stays in -Decisions marked expired and the Planner proceeds on its own judgement. The +`atomic`, Atomic input waits in Decisions until a member answers it or it is +withdrawn, however long that takes. The browser need not already have the document open: Chopin keeps its room alive while the invoked turn and queue run. An interrupted turn is not replayed automatically on restart. diff --git a/docs/self-hosting.md b/docs/self-hosting.md index be5e30041..902465765 100644 --- a/docs/self-hosting.md +++ b/docs/self-hosting.md @@ -191,9 +191,9 @@ questions) appears as shared Decisions instead of terminal dialogs, including workflow run and stage labels. Unanchored batches append at the document's end. Dialog text and written answers stay verbatim, bounded only by the document's 256 KiB source limit and the shared-draft limits rather than the Planner's own -question limits. A request nobody answers within 30 minutes expires: its cards -stay in Decisions marked as expired, and Atomic receives no answer. Cancellation -withdraws pending cards; neither ever approves an action. See +question limits. A request has no time limit: it waits until a member answers it +or it is withdrawn. Cancellation withdraws pending cards and never approves an +action. See [Full Atomic Planner](hosted-agent.md#full-atomic-planner) for the mapping, resource loading, persistence, and workflow restart boundaries. diff --git a/packages/dialect/src/dialect.ts b/packages/dialect/src/dialect.ts index f1080acdd..606415549 100644 --- a/packages/dialect/src/dialect.ts +++ b/packages/dialect/src/dialect.ts @@ -118,8 +118,9 @@ export const COMPONENTS: Readonly> = Object.freeze({ * Both are optional: a questionnaire has neither until it is answered, and * one settled before this was recorded has neither for good. * - * `status="expired"` marks host input nobody answered within its time limit. - * It stays in the document, unanswered, with `at` saying when it expired. + * `status="expired"` marks host input an earlier Chopin closed unanswered + * after a fixed time; questions no longer expire. It stays in the document, + * unanswered, with `at` saying when it expired. */ Questionnaire: component({ name: "Questionnaire", diff --git a/packages/dialect/src/nodes/questionnaire-fields.ts b/packages/dialect/src/nodes/questionnaire-fields.ts index 277cbdefd..671c5dbf4 100644 --- a/packages/dialect/src/nodes/questionnaire-fields.ts +++ b/packages/dialect/src/nodes/questionnaire-fields.ts @@ -8,7 +8,7 @@ export type Option = { description?: string; }; -/** `expired` is host input nobody answered in time: settled, but with no answer. */ +/** `expired` is host input an earlier Chopin closed unanswered: settled, but with no answer. */ export type CardStatus = "open" | "decided" | "reopened" | "discarded" | "expired"; export type Previous = { choices: string[]; value?: string; by: string; at: string }; diff --git a/packages/editor/src/decisions.test.tsx b/packages/editor/src/decisions.test.tsx index 8d1b243cd..d5287fcbe 100644 --- a/packages/editor/src/decisions.test.tsx +++ b/packages/editor/src/decisions.test.tsx @@ -83,8 +83,9 @@ test("an expired card is listed with the resolved cards and says nobody answered expect(history).toContain('data-plan-sidecar-questionnaire="expired"'); expect(history).toContain("Ship the migration?"); expect(history).toContain( - "Nobody answered within 30 minutes. The Planner will use its best judgement for this decision.", + "Nobody answered this question, so the Planner used its best judgement for this decision.", ); + expect(history).not.toContain("minutes"); for (let control of ["Save answer", " { let storage = { @@ -243,7 +243,7 @@ test("a host dialog with no options is answered by adding one, without a free-te expect(markup).not.toContain(" { +test("an expired card keeps its question and says nobody answered, with no time limit; a withdrawn one does not", () => { let definition = normalize({ questions: [{ header: "Confirm", @@ -262,14 +262,11 @@ test("an expired card keeps its question and says the Planner will proceed; a wi onSubmit() {}, onCancel() {}, })); - let note = `Nobody answered within ${limits.INPUT_EXPIRY_MS / 60_000} minutes. ` - + "The Planner will use its best judgement for this decision."; let expired = render("expired"); - expect(limits.INPUT_EXPIRY_MS).toBe(30 * 60 * 1_000); expect(expired).toContain( - "Nobody answered within 30 minutes. The Planner will use its best judgement for this decision.", + "Nobody answered this question, so the Planner used its best judgement for this decision.", ); - expect(expired).toContain(note); + expect(expired).not.toMatch(/minute|within|expire/i); expect(expired).toContain("Ship the migration?"); expect(expired).not.toContain("Cancelled"); for (let control of ["