diff --git a/apps/server/src/questions.service.test.ts b/apps/server/src/questions.service.test.ts index 218c9118..1924319e 100644 --- a/apps/server/src/questions.service.test.ts +++ b/apps/server/src/questions.service.test.ts @@ -119,12 +119,20 @@ test("a batched ask creates independently addressed decision records and nodes", { status: "answered", resolver: "Storage", - answers: [{ question: "Where should room state live?", choices: ["MDX on disk"] }], + answers: [{ + question: "Where should room state live?", + choices: ["MDX on disk"], + optionIds: [records[0]!.definition.questions[0]!.options[0]!.id], + }], }, { status: "answered", resolver: "Scope", - answers: [{ question: "What belongs in the first cut?", choices: ["Anchors"] }], + answers: [{ + question: "What belongs in the first cut?", + choices: ["Anchors"], + optionIds: [records[1]!.definition.questions[0]!.options[0]!.id], + }], }, ]); }); @@ -356,7 +364,11 @@ test("an appended option is durable in the record, draft store and plan before a if (!edited.open || !edited.accepted) throw new Error("could not choose the option"); let claimed = Store.claimSubmit(plan.questions, id, edited.revision, "ana"); if (!claimed.ok) throw new Error("could not claim"); - expect(claimed.answers).toEqual([{ question: item.question, choices: ["A third way"] }]); + expect(claimed.answers).toEqual([{ + question: item.question, + choices: ["A third way"], + optionIds: [reply.option.id], + }]); Store.commit(plan.questions, claimed.claim); await asked.waiting; }); diff --git a/apps/server/src/questions/record-fields.ts b/apps/server/src/questions/record-fields.ts new file mode 100644 index 00000000..cf601a7d --- /dev/null +++ b/apps/server/src/questions/record-fields.ts @@ -0,0 +1,31 @@ +export const STATUSES = new Set(["open", "answered", "reopened", "discarded", "cancelled"]); +export const ORIGINS = new Set(["chat", "planner", "human"]); +export const MAX_UNIX_SECONDS = 253_402_300_799; + +export type ObjectValue = { [key: string]: unknown }; + +export function invalid(): never { + throw new Error("hosted channel has an invalid question record"); +} + +export function object(value: unknown): ObjectValue { + if (!value || typeof value !== "object" || Array.isArray(value)) invalid(); + return value as ObjectValue; +} + +export function text(value: unknown, max?: number): string { + if (typeof value !== "string" || !value.trim() || max !== undefined && value.length > max) { + invalid(); + } + return value as string; +} + +export function unix(value: unknown): number { + if ( + typeof value !== "number" || !Number.isSafeInteger(value) + || value < 0 || value > MAX_UNIX_SECONDS + ) invalid(); + return value as number; +} + +export type Known = { questions: Map; options: Map }; diff --git a/apps/server/src/questions/record-provenance.ts b/apps/server/src/questions/record-provenance.ts new file mode 100644 index 00000000..b48d8a0f --- /dev/null +++ b/apps/server/src/questions/record-provenance.ts @@ -0,0 +1,45 @@ +import type { ConversationPlan } from "@chopin/protocol"; + +/** A question mention is provenance only when it is the card thread's exact saved ref. */ +export function matchesQuestionSource( + source: ConversationPlan.SourceRef, + thread: ConversationPlan.Thread | undefined, +): boolean { + return source.role === "question" + && !!thread?.questionSources.some(ref => + ref.role === "question" && ref.messageId === source.messageId + && ref.quote === source.quote && ref.start === source.start && ref.end === source.end + && ref.author.kind === source.author.kind + && (ref.author.kind !== "member" || source.author.kind === "member" + && ref.author.handle === source.author.handle) + ); +} + +function words(value: string): string[] { + return value.normalize("NFKC").toLocaleLowerCase().match(/[\p{L}\p{N}]+/gu) ?? []; +} + +function containsWords(haystack: string[], part: string[]): boolean { + return part.length > 0 + && haystack.some((_, index) => part.every((word, offset) => haystack[index + offset] === word)); +} + +/** Match a named alternative as whole, adjacent words in the saved question quote. */ +export function questionMentionsOption(quote: string, label: string): boolean { + let mention = words(quote); + let option = words(label); + if (["use", "using", "choose"].includes(option[0] ?? "")) option.shift(); + if (option[0] === "host") { + if (option[1] === "on") option.splice(0, 2); + else { + let on = option.indexOf("on", 2); + let topicEnd = quote.indexOf(":"); + if ( + on > 1 && on <= 6 && topicEnd >= 0 + && containsWords(words(quote.slice(0, topicEnd)), option.slice(1, on)) + ) option.splice(0, on + 1); + } + } + if (["a", "an", "the"].includes(option[0] ?? "")) option.shift(); + return containsWords(mention, option); +} diff --git a/apps/server/src/questions/record-types.ts b/apps/server/src/questions/record-types.ts new file mode 100644 index 00000000..b998b0c4 --- /dev/null +++ b/apps/server/src/questions/record-types.ts @@ -0,0 +1,39 @@ +import type { ConversationPlan, Plan as Wired } from "@chopin/protocol"; +import type { Definition } from "@chopin/question"; + +export type OptionOrigin = { + origin: "chat" | "planner" | "human"; + rationale?: string; + by?: string; + source?: ConversationPlan.SourceRef; +}; + +export type DecisionEntry = { + choices: string[]; + /** Text for older answers without durable option IDs, keyed by question. */ + answers?: { [questionId: string]: string }; + owner: string; + at: number; +}; + +export type Record = { + id: string; + definition: Definition; + /** "answered" is the stored status of a decided card. */ + status: "open" | "answered" | "reopened" | "discarded" | "cancelled"; + answers?: { [question: string]: string }; + resolver?: string; + at?: number; + anchors?: Wired.WidgetAnchors; + origin: "planner" | "conversation"; + threadId?: string; + owner?: string; + decidedAt?: number; + choices?: string[]; + history: DecisionEntry[]; + prose?: Wired.Anchor[]; + optionOrigins: { [optionId: string]: OptionOrigin }; + editors: string[]; + /** Durable request keys for shared option appends, bounded by the option limit. */ + appended?: { [key: string]: string }; +}; diff --git a/apps/server/src/questions/record-validation.ts b/apps/server/src/questions/record-validation.ts new file mode 100644 index 00000000..5d569bfa --- /dev/null +++ b/apps/server/src/questions/record-validation.ts @@ -0,0 +1,162 @@ +import type { ConversationPlan, Plan as Wired } from "@chopin/protocol"; +import { limits } from "@chopin/question"; +import { assertSourceShape } from "../conversation-plan/sources"; +import type { DecisionEntry, Record } from "./record-types"; +import { invalid, object, ORIGINS, text, unix } from "./record-fields"; +import type { Known } from "./record-fields"; + +export function options(definition: unknown, pending: boolean): Known { + let source = object(definition); + if ( + !Array.isArray(source.questions) || source.questions.length === 0 + || source.questions.length > limits.MAX_QUESTIONS + ) invalid(); + let ids = new Map(); + let questions = new Map(); + for (let candidate of source.questions) { + let question = object(candidate); + let id = text(question.id); + if (questions.has(id)) invalid(); + if ( + typeof question.multiple !== "boolean" + || !Array.isArray(question.options) + || question.options.length === 0 + && !(pending && source.questions.length === 1 && question.multiple === false) + || question.options.length > limits.MAX_OPTIONS + ) invalid(); + questions.set(id, { multiple: question.multiple as boolean }); + for (let candidate of question.options) { + let option = object(candidate); + let optionId = text(option.id); + if (ids.has(optionId)) invalid(); + ids.set(optionId, id); + } + } + return { questions, options: ids }; +} + +export function choices(value: unknown, known: Known): string[] { + if (!Array.isArray(value)) invalid(); + let found = new Set(); + let counts = new Map(); + for (let id of value) { + if (typeof id !== "string" || !known.options.has(id) || found.has(id)) invalid(); + found.add(id); + let question = known.options.get(id)!; + let count = (counts.get(question) ?? 0) + 1; + if (count > (known.questions.get(question)!.multiple ? limits.MAX_OPTIONS : 1)) invalid(); + counts.set(question, count); + } + return value as string[]; +} + +export function answers(value: unknown, known: Known): { [question: string]: string } { + let entries = object(value); + for (let [id, answer] of Object.entries(entries)) { + if (!known.questions.has(id)) invalid(); + text(answer, limits.MAX_CUSTOM); + } + return value as { [question: string]: string }; +} + +export function editors(value: unknown): string[] { + if (!Array.isArray(value)) invalid(); + let found = new Set(); + for (let item of value) { + let handle = text(item); + if (found.has(handle)) invalid(); + found.add(handle); + } + return value as string[]; +} + +export function history(value: unknown, known: Known): DecisionEntry[] { + if (!Array.isArray(value)) invalid(); + for (let candidate of value) { + let entry = object(candidate); + if ( + Object.keys(entry).some(key => !["choices", "answers", "owner", "at"].includes(key)) + || !Object.hasOwn(entry, "choices") + || !Object.hasOwn(entry, "owner") || !Object.hasOwn(entry, "at") + ) invalid(); + let selected = choices(entry.choices, known); + let legacy = Object.hasOwn(entry, "answers") ? answers(entry.answers, known) : {}; + let chosenQuestions = new Set(selected.map(id => known.options.get(id)!)); + for (let id of Object.keys(legacy)) { + if (chosenQuestions.has(id)) invalid(); + chosenQuestions.add(id); + } + if (chosenQuestions.size !== known.questions.size) invalid(); + text(entry.owner); + unix(entry.at); + } + return value as DecisionEntry[]; +} + +export function optionOrigins( + value: unknown, + known: Set, + conversationThread: boolean, +): Record["optionOrigins"] { + let origins = object(value); + for (let [id, candidate] of Object.entries(origins)) { + if (!known.has(id)) invalid(); + let entry = object(candidate); + if ( + !ORIGINS.has(entry.origin as string) + || Object.keys(entry).some(key => + key !== "origin" && key !== "rationale" && key !== "by" && key !== "source" + ) + ) { + invalid(); + } + if (Object.hasOwn(entry, "rationale")) text(entry.rationale); + if (Object.hasOwn(entry, "by")) text(entry.by); + if (Object.hasOwn(entry, "source")) { + if (entry.origin !== "planner" && entry.origin !== "chat") invalid(); + try { + assertSourceShape(entry.source); + let role = (entry.source as ConversationPlan.SourceRef).role; + if ( + role !== "option" && !(role === "question" && conversationThread + && entry.origin === "planner") + ) { + invalid(); + } + } catch { + invalid(); + } + } + } + return value as Record["optionOrigins"]; +} + +export function prose(value: unknown): Wired.Anchor[] { + if (!Array.isArray(value)) invalid(); + for (let candidate of value) { + let anchor = object(candidate); + if ( + Object.keys(anchor).some(key => + key !== "epoch" && key !== "position" && key !== "digest" && key !== "orphaned" + && key !== "recoverOnNextEdit" + ) + ) invalid(); + text(anchor.epoch); + if ( + typeof anchor.position !== "string" || !anchor.position + || !/^(?:[A-Za-z0-9+/]{4})*(?:[A-Za-z0-9+/]{2}==|[A-Za-z0-9+/]{3}=)?$/.test( + anchor.position, + ) + ) { + invalid(); + } + if (typeof anchor.digest !== "string" || !/^sha256:[0-9a-f]{64}$/.test(anchor.digest)) { + invalid(); + } + if (Object.hasOwn(anchor, "orphaned") && anchor.orphaned !== true) invalid(); + if (Object.hasOwn(anchor, "recoverOnNextEdit")) { + if (anchor.recoverOnNextEdit !== true || anchor.orphaned !== true) invalid(); + } + } + return value as Wired.Anchor[]; +} diff --git a/apps/server/src/questions/records.bounds.test.ts b/apps/server/src/questions/records.bounds.test.ts new file mode 100644 index 00000000..cbbef2b7 --- /dev/null +++ b/apps/server/src/questions/records.bounds.test.ts @@ -0,0 +1,48 @@ +import { expect, test } from "bun:test"; +import { normalizeRecord } from "./records"; + +test("conversation thread IDs accept 200 UTF-16 units and reject 201", () => { + let record = { + id: "card-1", + status: "open", + origin: "conversation", + threadId: "🧪".repeat(100), + definition: { + questions: [{ + id: "question-1", + header: "Auth", + question: "Which system?", + multiple: false, + options: [], + }], + }, + }; + expect(record.threadId).toHaveLength(200); + expect(normalizeRecord(record).threadId).toBe(record.threadId); + expect(() => normalizeRecord({ ...record, threadId: record.threadId + "x" })) + .toThrow("hosted channel has an invalid question record"); +}); + +test("durable option append keys survive record normalization and reject foreign option IDs", () => { + let record = { + id: "card-1", + status: "open", + definition: { + questions: [{ id: "question-1", multiple: false, options: [{ id: "option-1" }] }], + }, + appended: { "key-once-0001": "option-1" }, + }; + expect(normalizeRecord(record).appended).toEqual(record.appended); + for ( + let appended of [ + { bad: "option-1" }, + { "key-once-0001": "foreign-option" }, + { "key-once-0001": "option-1", "key-once-0002": "option-1" }, + [], + null, + ] + ) { + expect(() => normalizeRecord({ ...record, appended })) + .toThrow("hosted channel has an invalid question record"); + } +}); diff --git a/apps/server/src/questions/records.reopen.test.ts b/apps/server/src/questions/records.reopen.test.ts new file mode 100644 index 00000000..e7d821fc --- /dev/null +++ b/apps/server/src/questions/records.reopen.test.ts @@ -0,0 +1,182 @@ +import { expect, test } from "bun:test"; + +import { normalizeRecord, questionMentionsOption } from "./records"; + +test("question provenance requires the option's named terms, including D02 hosting labels", () => { + let question = "hosting for the beta: managed app service or our own server?"; + expect(questionMentionsOption(question, "Use a managed app service")).toBe(true); + expect(questionMentionsOption(question, "Host on our own server")).toBe(true); + expect(questionMentionsOption(question, "Host on a small VPS")).toBe(false); + expect(questionMentionsOption("Should we use GitHub Apps or OAuth?", "Redis")).toBe(false); + expect(questionMentionsOption("Should we use GitHub Apps or OAuth?", "GitHub Apps")).toBe(true); + expect(questionMentionsOption("Should we use GitHub Apps or OAuth?", "OAuth")).toBe(true); + expect(questionMentionsOption("Should we use Auth0?", "Auth")).toBe(false); +}); + +let definition = { + questions: [ + { + id: "q1", + header: "First", + question: "First choice?", + multiple: true, + options: Array.from({ length: 12 }, (_, index) => ({ + id: `a${index}`, + label: `A ${index}`, + description: "", + })), + }, + { + id: "q2", + header: "Second", + question: "Second choice?", + multiple: true, + options: Array.from({ length: 12 }, (_, index) => ({ + id: `b${index}`, + label: `B ${index}`, + description: "", + })), + }, + ], +}; + +let base = { + id: "w", + definition, + status: "reopened", + origin: "planner", + optionOrigins: {}, + editors: [], +}; + +test("an open conversation card can have no quoted options yet", () => { + let pending = { + ...base, + status: "open", + origin: "conversation", + threadId: "thread-a", + definition: { + questions: [{ + id: "q1", + header: "Auth", + question: "Which system?", + multiple: false, + options: [], + }], + }, + }; + expect(normalizeRecord(pending).definition.questions[0]?.options).toEqual([]); + expect(normalizeRecord({ ...pending, status: "discarded" }).status).toBe("discarded"); + expect(() => normalizeRecord({ ...pending, status: "answered" })).toThrow(/invalid/); + expect(() => normalizeRecord({ ...pending, origin: "planner" })).toThrow(/invalid/); +}); + +test("history accepts more than 20 choices across legacy questions", () => { + let all = definition.questions.flatMap(question => question.options.map(option => option.id)); + let record = normalizeRecord({ + ...base, + history: [{ choices: all, owner: "ana", at: 1 }], + }); + expect(record.history[0]?.choices).toEqual(all); +}); + +test("history retains a mixed ID and text answer without assigning an ID to the text", () => { + let record = normalizeRecord({ + ...base, + history: [{ choices: ["a0"], answers: { q2: "A custom answer" }, owner: "ana", at: 1 }], + }); + expect(record.history[0]).toEqual({ + choices: ["a0"], + answers: { q2: "A custom answer" }, + owner: "ana", + at: 1, + }); +}); + +test("history rejects overlap, missing answers, unknown IDs, and malformed text", () => { + let invalid = [ + { choices: ["a0"], answers: { q1: "Duplicate", q2: "Text" }, owner: "ana", at: 1 }, + { choices: ["a0"], owner: "ana", at: 1 }, + { choices: ["unknown"], answers: { q2: "Text" }, owner: "ana", at: 1 }, + { choices: ["a0"], answers: { q2: "" }, owner: "ana", at: 1 }, + { choices: ["a0"], answers: { q2: "x".repeat(4001) }, owner: "ana", at: 1 }, + { choices: ["a0"], answers: { wrong: "Text" }, owner: "ana", at: 1 }, + ]; + for (let entry of invalid) { + expect(() => normalizeRecord({ ...base, history: [entry] })).toThrow(/invalid question record/); + } +}); + +test("current choice bounds are checked per question", () => { + let single = { + ...definition, + questions: definition.questions.map((question, index) => ({ + ...question, + multiple: index !== 0, + })), + }; + expect(() => + normalizeRecord({ + ...base, + definition: single, + choices: ["a0", "a1"], + }) + ).toThrow(/invalid question record/); + expect(() => + normalizeRecord({ + ...base, + answers: { wrong: "Text" }, + }) + ).toThrow(/invalid question record/); +}); + +test("option origins restore without a source and reject malformed optional sources", () => { + let record = { + ...base, + optionOrigins: { a0: { origin: "planner" as const, rationale: "Repository evidence" } }, + }; + expect(normalizeRecord(record).optionOrigins.a0).toEqual(record.optionOrigins.a0); + let cited = { + ...record, + optionOrigins: { + a0: { + origin: "planner" as const, + source: { + messageId: "m1", + author: { kind: "member" as const, handle: "jev" }, + quote: "Lexical", + start: 0, + end: 7, + role: "option" as const, + }, + }, + }, + }; + expect(normalizeRecord(cited).optionOrigins.a0).toEqual(cited.optionOrigins.a0); + let question = { + ...cited, + origin: "conversation" as const, + threadId: "thread-a", + optionOrigins: { + a0: { + ...cited.optionOrigins.a0, + source: { ...cited.optionOrigins.a0.source, role: "question" as const }, + }, + }, + }; + expect(normalizeRecord(question).optionOrigins.a0).toEqual(question.optionOrigins.a0); + expect(() => normalizeRecord({ ...question, origin: "planner" })).toThrow( + /invalid question record/, + ); + expect(() => + normalizeRecord({ + ...cited, + optionOrigins: { + a0: { + ...cited.optionOrigins.a0, + source: { ...cited.optionOrigins.a0.source, role: "reason" }, + }, + }, + }) + ).toThrow(/invalid question record/); +}); diff --git a/apps/server/src/questions/records.ts b/apps/server/src/questions/records.ts new file mode 100644 index 00000000..e4d5f29d --- /dev/null +++ b/apps/server/src/questions/records.ts @@ -0,0 +1,80 @@ +/** Durable decision-card fields and legacy questionnaire defaults. */ + +import { limits as dialectLimits } from "@chopin/dialect"; +import { invalid, object, STATUSES, text, unix } from "./record-fields"; +import { + answers, + choices, + editors, + history, + optionOrigins, + options, + prose, +} from "./record-validation"; +import type { Record } from "./record-types"; + +export { matchesQuestionSource, questionMentionsOption } from "./record-provenance"; +export type { DecisionEntry, OptionOrigin, Record } from "./record-types"; + +export function isOpenStatus(status: Record["status"]): boolean { + return status === "open" || status === "reopened"; +} + +/** Fill only fields that did not exist on stored Planner questionnaires. */ +export function normalizeRecord(raw: unknown): Record { + let value = object(raw); + text(value.id); + if (!STATUSES.has(value.status as string)) invalid(); + let pending = value.origin === "conversation" && typeof value.threadId === "string" + && !!value.threadId.trim() + && (value.status === "open" || value.status === "reopened" || value.status === "discarded"); + let known = options(value.definition, pending); + if (Object.hasOwn(value, "resolver")) text(value.resolver); + if (Object.hasOwn(value, "at")) unix(value.at); + if ( + Object.hasOwn(value, "origin") && value.origin !== "planner" + && value.origin !== "conversation" + ) invalid(); + if (Object.hasOwn(value, "threadId")) text(value.threadId, dialectLimits.MAX_ID); + if (Object.hasOwn(value, "owner")) text(value.owner); + if (Object.hasOwn(value, "decidedAt")) unix(value.decidedAt); + if (Object.hasOwn(value, "choices")) choices(value.choices, known); + if (Object.hasOwn(value, "answers")) answers(value.answers, known); + if (Object.hasOwn(value, "history")) history(value.history, known); + if (Object.hasOwn(value, "prose")) prose(value.prose); + if (Object.hasOwn(value, "optionOrigins")) { + optionOrigins( + value.optionOrigins, + new Set(known.options.keys()), + value.origin === "conversation" && typeof value.threadId === "string", + ); + } + if (Object.hasOwn(value, "editors")) editors(value.editors); + if (Object.hasOwn(value, "appended")) { + let appended = object(value.appended); + let seen = new Set(); + if (Object.keys(appended).length > known.options.size) invalid(); + for (let [key, id] of Object.entries(appended)) { + if ( + !/^[A-Za-z0-9_-]{8,64}$/.test(key) || typeof id !== "string" + || !known.options.has(id) || seen.has(id) + ) invalid(); + seen.add(id); + } + } + let record = value as Record; + return { + ...record, + origin: Object.hasOwn(value, "origin") ? record.origin : "planner", + history: Object.hasOwn(value, "history") ? record.history : [], + optionOrigins: Object.hasOwn(value, "optionOrigins") ? record.optionOrigins : {}, + editors: Object.hasOwn(value, "editors") ? record.editors : [], + ...(!Object.hasOwn(value, "owner") && record.resolver !== undefined + ? { owner: record.resolver } + : {}), + ...(record.status === "answered" && !Object.hasOwn(value, "decidedAt") + && record.at !== undefined + ? { decidedAt: record.at } + : {}), + }; +} diff --git a/apps/server/src/questions/service.ts b/apps/server/src/questions/service.ts index f007c2ce..52bbbef9 100644 --- a/apps/server/src/questions/service.ts +++ b/apps/server/src/questions/service.ts @@ -404,11 +404,12 @@ export async function addOption( }; return; } + let definition = Question.decision(entry.definition); let applied = typeof msg.key === "string" && Object.hasOwn(record.appended ?? {}, msg.key) ? record.appended![msg.key] : undefined; let existing = applied - ? entry.definition.questions[0].options.find(option => option.id === applied) + ? definition.questions[0].options.find(option => option.id === applied) : undefined; if (existing) { outcome = { @@ -417,7 +418,7 @@ export async function addOption( id: msg.id, ok: true, option: existing, - definition: entry.definition, + definition, repeated: true, }; return; @@ -434,7 +435,7 @@ export async function addOption( return; } - let result = Question.appendOption(entry.definition, { + let result = Question.appendOption(definition, { question: msg.question, key: msg.key, label: msg.label, diff --git a/apps/server/src/questions/store-edit.test.ts b/apps/server/src/questions/store-edit.test.ts new file mode 100644 index 00000000..ec0be2a8 --- /dev/null +++ b/apps/server/src/questions/store-edit.test.ts @@ -0,0 +1,38 @@ +import { describe, expect, it } from "bun:test"; +import * as Question from "@chopin/question"; + +import * as Store from "./store"; +import { asked } from "./store.test-fixtures"; + +// Original callback from archive 446a9779a937fa5be7cd3eb52fd7f3023d691ed2, +// apps/server/src/questions/store.test.ts. +describe("Store.edit editors", () => { + it("credits applied edits, but not duplicate patches or refused edits", () => { + let questions = asked(); + let entry = Store.get(questions, "w")!; + let model = Question.crdt.Model.fromBinary(entry.model.toBinary()) + .fork() as unknown as Question.Model; + model.api.val(["q", "choice"]).set("a"); + let patch = model.api.flush(); + if (!patch) throw new Error("selection made no patch"); + let binary = [...patch.toBinary()]; + + expect(Store.edit(questions, "w", binary, "ana")).toMatchObject({ + open: true, + accepted: true, + applied: true, + }); + expect(Store.get(questions, "w")!.editors).toEqual(new Set(["ana"])); + expect(Store.edit(questions, "w", binary, "ben")).toMatchObject({ + open: true, + accepted: true, + applied: false, + }); + expect(Store.get(questions, "w")!.editors).toEqual(new Set(["ana"])); + let claim = Store.claimSubmit(questions, "w", 1, "ana"); + if (!claim.ok) throw new Error("selection was not claimable"); + expect(Store.edit(questions, "w", binary, "cy")).toMatchObject({ accepted: false }); + expect(Store.get(questions, "w")!.editors).toEqual(new Set(["ana"])); + Store.rollback(questions, claim.claim); + }); +}); diff --git a/apps/server/src/questions/store-lifecycle.ts b/apps/server/src/questions/store-lifecycle.ts new file mode 100644 index 00000000..7bfe0c53 --- /dev/null +++ b/apps/server/src/questions/store-lifecycle.ts @@ -0,0 +1,27 @@ +import * as Question from "@chopin/question"; + +import type { Definition } from "@chopin/question"; +import type { Questions } from "./store-types"; + +/** Reopen the same card with a fresh draft and no waiting Planner turn. */ +export function reopen( + questions: Questions, + id: string, + definition: Definition, + widget?: string, +): void { + if (questions.open.has(id)) Question.reject("Questionnaire is already open"); + let accepted = Question.identified(definition); + let model = Question.create(accepted); + Question.read(model, accepted); + questions.closed.delete(id); + questions.open.set(id, { + id, + definition: accepted, + ...(widget ? { widget } : {}), + model, + revision: 0, + presence: new Map(), + editors: new Set(), + }); +} diff --git a/apps/server/src/questions/store-options.test.ts b/apps/server/src/questions/store-options.test.ts new file mode 100644 index 00000000..49ec8c42 --- /dev/null +++ b/apps/server/src/questions/store-options.test.ts @@ -0,0 +1,77 @@ +import { describe, expect, it } from "bun:test"; +import * as Question from "@chopin/question"; + +import * as Store from "./store"; +import { asked } from "./store.test-fixtures"; + +// Original callbacks from archive 446a9779a937fa5be7cd3eb52fd7f3023d691ed2. +describe("Store.addOption", () => { + it("grows the definition and shared draft together", () => { + let questions = asked(); + let result = Store.addOption(questions, "w", "b", " GitHub Apps "); + expect(result).toMatchObject({ + ok: true, + revision: 1, + option: { id: "b", label: "GitHub Apps" }, + }); + let entry = Store.get(questions, "w")!; + expect(entry.definition.questions[0].options.map(option => option.label)).toEqual([ + "Auth0", + "GitHub Apps", + ]); + expect(Question.read(entry.model, entry.definition).q!.options.b).toBe(false); + expect(Store.claimSubmit(questions, "w", 0, "ana")).toMatchObject({ + ok: false, + reason: "stale", + current: 1, + }); + }); + + it("reports the second of two identical adds as a duplicate", () => { + let questions = asked(); + Store.addOption(questions, "w", "b", "GitHub Apps"); + expect(Store.addOption(questions, "w", "c", "github apps")).toMatchObject({ + ok: false, + reason: "duplicate", + }); + expect(Store.get(questions, "w")!.definition.questions[0].options).toHaveLength(2); + }); + + it("refuses an invalid label, a full card, a resolving card, and a closed card", () => { + let questions = asked(); + expect(Store.addOption(questions, "w", "b", " ")).toMatchObject({ + ok: false, + reason: "invalid", + }); + let claim = Store.claimCancel(questions, "w", "ana"); + expect(Store.addOption(questions, "w", "b", "GitHub Apps")).toMatchObject({ + ok: false, + reason: "closed", + }); + if (claim.ok) Store.commit(questions, claim.claim); + expect(Store.addOption(questions, "w", "b", "GitHub Apps")).toMatchObject({ + ok: false, + reason: "closed", + }); + + let full = asked(); + for (let i = 1; i < 10; i++) { + expect(Store.addOption(full, "w", `o${i}`, `Option ${i}`)).toMatchObject({ ok: true }); + } + expect(Store.addOption(full, "w", "overflow", "Overflow")).toMatchObject({ + ok: false, + reason: "full", + }); + }); + + it("reverts to exactly the previous definition, model, and revision", () => { + let questions = asked(); + let before = Store.before(questions, "w")!; + Store.addOption(questions, "w", "b", "GitHub Apps"); + Store.revert(questions, "w", before); + let entry = Store.get(questions, "w")!; + expect(entry.definition.questions[0].options).toHaveLength(1); + expect(entry.revision).toBe(0); + expect(Question.read(entry.model, entry.definition).q!.options).toEqual({ a: false }); + }); +}); diff --git a/apps/server/src/questions/store-options.ts b/apps/server/src/questions/store-options.ts new file mode 100644 index 00000000..2105f58a --- /dev/null +++ b/apps/server/src/questions/store-options.ts @@ -0,0 +1,167 @@ +import * as Question from "@chopin/question"; + +import type { DecisionDefinition, Definition, Model } from "@chopin/question"; +import type { Open, Questions } from "./store-types"; + +export type AddedOption = + | { ok: true; definition: DecisionDefinition; option: Question.Option; revision: number } + | { ok: false; reason: "full" | "duplicate" | "invalid" | "closed"; message: string }; + +export type Retitled = + | { ok: true; definition: DecisionDefinition; revision: number; applied: boolean } + | { ok: false; reason: "invalid" | "closed"; message: string }; + +/** Change only the display label of an existing option; the draft keeps its option ID. */ +export function relabelOption( + questions: Questions, + id: string, + optionId: string, + label: string, + expectedRevision: number, +): { definition: DecisionDefinition; revision: number } { + let entry = questions.open.get(id); + if (!entry || entry.claim || entry.definition.questions.length !== 1) { + throw new Error("decision is no longer open"); + } + if (entry.revision !== expectedRevision) throw new Error("stale decision revision"); + let question = entry.definition.questions[0]!; + let option = question.options.find(item => item.id === optionId); + if (!option) throw new Error("unknown decision option"); + if (option.label === label) throw new Error("option label is unchanged"); + if ( + question.options.some(item => + item.id !== optionId + && item.label.trim().toLocaleLowerCase() === label.trim().toLocaleLowerCase() + ) + ) { + throw new Error("duplicate option label"); + } + let definition = Question.decision({ + questions: [{ + ...question, + options: question.options.map(item => item.id === optionId ? { ...item, label } : item), + }], + }); + Question.read(entry.model, definition); + entry.definition = definition; + entry.revision++; + if (entry.suggested) entry.suggested = { ...entry.suggested, revision: entry.revision }; + return { definition, revision: entry.revision }; +} + +/** Reword one open decision while retaining its shared human draft. */ +export function retitle(questions: Questions, id: string, text: string): Retitled { + let entry = questions.open.get(id); + if (!entry || entry.claim || entry.definition.questions.length !== 1) { + return { ok: false, reason: "closed", message: "This decision is no longer open" }; + } + let question = typeof text === "string" ? text.trim() : ""; + if (!question || question.length > Question.limits.MAX_QUESTION) { + return { ok: false, reason: "invalid", message: "A question is 1–1000 characters" }; + } + let current = entry.definition.questions[0]!; + if (current.question === question) { + return { + ok: true, + definition: Question.decision(entry.definition), + revision: entry.revision, + applied: false, + }; + } + let definition = Question.decision({ questions: [{ ...current, question }] }); + entry.definition = definition; + entry.revision++; + if (entry.suggested) entry.suggested = { ...entry.suggested, revision: entry.revision }; + return { ok: true, definition, revision: entry.revision, applied: true }; +} + +export type Before = { + definition: Definition; + model: Model; + revision: number; + suggested?: Open["suggested"]; +}; + +/** Capture the three fields changed by an option addition. */ +export function before(questions: Questions, id: string): Before | undefined { + let entry = questions.open.get(id); + return entry && { + definition: entry.definition, + model: entry.model, + revision: entry.revision, + ...(entry.suggested ? { suggested: entry.suggested } : {}), + }; +} + +/** Grow an open decision without changing a person's existing draft choice. */ +export function addOption( + questions: Questions, + id: string, + optionId: string, + label: string, +): AddedOption { + let entry = questions.open.get(id); + if (!entry || entry.claim) { + return { ok: false, reason: "closed", message: "This decision is no longer open" }; + } + if (entry.definition.questions.length !== 1) { + return { + ok: false, + reason: "closed", + message: "Options cannot be added to this questionnaire", + }; + } + let outcome = Question.addOption( + Question.decision(entry.definition), + entry.model, + optionId, + label, + ); + if (!outcome.ok) return outcome; + entry.definition = outcome.definition; + entry.model = outcome.model; + entry.revision++; + if (entry.suggested) entry.suggested = { ...entry.suggested, revision: entry.revision }; + return { + ok: true, + definition: outcome.definition, + option: outcome.option, + revision: entry.revision, + }; +} + +/** Restore the original shape if a caller discards an uncommitted addition. */ +export function revert(questions: Questions, id: string, previous: Before): void { + let entry = questions.open.get(id); + if (!entry) return; + entry.definition = previous.definition; + entry.model = previous.model; + entry.revision = previous.revision; + entry.suggested = previous.suggested; +} + +/** Keep draft edits and resolution claims off a card during its fenced option commit. */ +export function reserveOption(questions: Questions, id: string): boolean { + let entry = questions.open.get(id); + if (!entry || entry.claim) return false; + entry.claim = "option"; + return true; +} + +export function releaseOption(questions: Questions, id: string): void { + let entry = questions.open.get(id); + if (entry?.claim === "option") entry.claim = undefined; +} + +/** Hold a draft while its edit is fenced, so no submit reads uncommitted state. */ +export function reserveEdit(questions: Questions, id: string): boolean { + let entry = questions.open.get(id); + if (!entry || entry.claim) return false; + entry.claim = "edit"; + return true; +} + +export function releaseEdit(questions: Questions, id: string): void { + let entry = questions.open.get(id); + if (entry?.claim === "edit") entry.claim = undefined; +} diff --git a/apps/server/src/questions/store-reopen.test.ts b/apps/server/src/questions/store-reopen.test.ts new file mode 100644 index 00000000..92b28b0f --- /dev/null +++ b/apps/server/src/questions/store-reopen.test.ts @@ -0,0 +1,29 @@ +import { describe, expect, it } from "bun:test"; +import * as Question from "@chopin/question"; + +import * as Store from "./store"; +import { asked, legacy } from "./store.test-fixtures"; + +// Original callbacks from archive 446a9779a937fa5be7cd3eb52fd7f3023d691ed2. +describe("Store.reopen", () => { + it("reuses identity with a fresh draft, no Planner waiter, and no old tombstone", () => { + let questions = asked(); + let claim = Store.claimCancel(questions, "w", "ana"); + if (!claim.ok) throw new Error("setup"); + Store.commit(questions, claim.claim); + let definition = legacy(); + Store.reopen(questions, "w", definition, "w"); + let entry = Store.get(questions, "w")!; + expect(entry.definition).toEqual(definition); + expect(entry.revision).toBe(0); + expect(entry.settle).toBeUndefined(); + expect(questions.closed.has("w")).toBe(false); + expect(Question.read(entry.model, entry.definition).q1!.choice).toBeNull(); + expect(Question.read(entry.model, entry.definition).q2!.choice).toBeNull(); + expect(Store.addOption(questions, "w", "new", "Another")).toMatchObject({ + ok: false, + reason: "closed", + }); + expect(() => Store.reopen(questions, "w", definition, "w")).toThrow(/already open/); + }); +}); diff --git a/apps/server/src/questions/store-restore.test.ts b/apps/server/src/questions/store-restore.test.ts new file mode 100644 index 00000000..ac90e2be --- /dev/null +++ b/apps/server/src/questions/store-restore.test.ts @@ -0,0 +1,72 @@ +import { describe, expect, it } from "bun:test"; +import * as Question from "@chopin/question"; + +import * as Store from "./store"; + +import { legacy } from "./store.test-fixtures"; + +// Original restoration callback: archive 446a9779a937fa5be7cd3eb52fd7f3023d691ed2, +// apps/server/src/questions/store.test.ts. + +describe("Store.restore shared definitions", () => { + it("restores a legacy multi-question draft with its original question IDs", () => { + let definition = legacy(); + let restored = Store.restore([{ + id: "w", + definition, + widget: "w", + model: [...Question.create(definition).toBinary()], + revision: 3, + }]); + expect(Store.snapshot(restored, "w")).toMatchObject({ + open: true, + definition, + revision: 3, + }); + expect(Store.outstanding(restored)[0]?.definition).toEqual(definition); + }); + it("restores an empty pending card without replacing its durable ID or draft mode", () => { + let definition = { + questions: [{ + id: "pending-question", + header: "Choice", + question: "Which option?", + multiple: false, + options: [], + }], + }; + let stored = { + id: "pending-card", + definition, + model: [...Question.create(definition).toBinary()], + revision: 7, + }; + let restored = Store.restore([stored]); + let entry = Store.get(restored, "pending-card")!; + expect(entry.definition).toEqual(definition); + expect(Question.read(entry.model, entry.definition)).toEqual({ + "pending-question": { mode: "choices", choice: null, custom: "", options: {} }, + }); + expect(Store.dump(restored)).toEqual([stored]); + }); + + it("rejects malformed stored definitions instead of only checking their question count", () => { + let definition = { questions: [legacy().questions[0]!] }; + let model = [...Question.create(definition).toBinary()]; + for ( + let invalid of [ + { ...definition, extra: true }, + { questions: [{ ...definition.questions[0]!, multiple: "false" }] }, + ] + ) { + expect(() => + Store.restore([{ + id: "invalid", + definition: invalid as Question.Definition, + model, + revision: 0, + }]) + ).toThrow(Question.QuestionError); + } + }); +}); diff --git a/apps/server/src/questions/store-retitle.test.ts b/apps/server/src/questions/store-retitle.test.ts new file mode 100644 index 00000000..58071850 --- /dev/null +++ b/apps/server/src/questions/store-retitle.test.ts @@ -0,0 +1,78 @@ +import { describe, expect, it } from "bun:test"; +import * as Question from "@chopin/question"; + +import * as Store from "./store"; +import { asked } from "./store.test-fixtures"; + +// Original callbacks from archive 446a9779a937fa5be7cd3eb52fd7f3023d691ed2. +describe("Store.retitle", () => { + it("changes one decision question without changing the human draft or option identities", () => { + let questions = asked(); + let entry = Store.get(questions, "w")!; + let chosen = Question.crdt.Model.fromBinary(entry.model.toBinary()) + .fork() as unknown as Question.Model; + chosen.api.val(["q", "choice"]).set("a"); + let patch = chosen.api.flush(); + if (!patch) throw new Error("selection made no patch"); + expect(Store.edit(questions, "w", [...patch.toBinary()], "ana")).toMatchObject({ + open: true, + accepted: true, + }); + entry = Store.get(questions, "w")!; + let before = [...entry.model.toBinary()]; + let result = Store.retitle(questions, "w", " Which auth system should ship first? "); + expect(result).toMatchObject({ ok: true, applied: true, revision: 2 }); + entry = Store.get(questions, "w")!; + expect(entry.definition.questions[0]?.question).toBe("Which auth system should ship first?"); + expect(entry.definition.questions[0]?.options).toEqual([ + { id: "a", label: "Auth0", description: "" }, + ]); + expect([...entry.model.toBinary()]).toEqual(before); + expect(Question.read(entry.model, entry.definition).q?.choice).toBe("a"); + expect(Store.restore(Store.dump(questions)).open.get("w")?.revision).toBe(2); + }); + + it("keeps an advisory suggestion tied to the new revision and avoids a repeat bump", () => { + let questions = asked(); + Store.suggest(questions, "w", { optionId: "a", messageIds: ["m1"] }); + expect(Store.retitle(questions, "w", "Why use Auth0?")).toMatchObject({ + ok: true, + applied: true, + revision: 2, + }); + expect(Store.get(questions, "w")?.suggested).toEqual({ + optionId: "a", + messageIds: ["m1"], + revision: 2, + }); + expect(Store.retitle(questions, "w", " Why use Auth0? ")).toMatchObject({ + ok: true, + applied: false, + revision: 2, + }); + expect(Store.restore(Store.dump(questions)).open.get("w")?.suggested?.revision).toBe(2); + }); + + it("refuses invalid text, a resolving decision, and a closed decision", () => { + let questions = asked(); + expect(Store.retitle(questions, "w", " ")).toMatchObject({ + ok: false, + reason: "invalid", + }); + expect(Store.retitle(questions, "w", "x".repeat(1_001))).toMatchObject({ + ok: false, + reason: "invalid", + }); + let claim = Store.claimCancel(questions, "w", "ana"); + expect(Store.retitle(questions, "w", "Why now?")).toMatchObject({ + ok: false, + reason: "closed", + }); + if (!claim.ok) throw new Error("setup"); + Store.commit(questions, claim.claim); + expect(Store.retitle(questions, "w", "Why now?")).toMatchObject({ + ok: false, + reason: "closed", + }); + }); +}); diff --git a/apps/server/src/questions/store-suggestion-queue.test.ts b/apps/server/src/questions/store-suggestion-queue.test.ts new file mode 100644 index 00000000..957f84fd --- /dev/null +++ b/apps/server/src/questions/store-suggestion-queue.test.ts @@ -0,0 +1,89 @@ +import { describe, expect, it } from "bun:test"; +import * as Question from "@chopin/question"; + +import * as Store from "./store"; +import { asked } from "./store.test-fixtures"; + +// Exact archive callbacks: 446a9779a937fa5be7cd3eb52fd7f3023d691ed2, +// apps/server/src/questions/store.test.ts. +describe("Store.suggest queue", () => { + it("applies a human selection prepared before the server suggestion", () => { + let questions = asked(); + Store.addOption(questions, "w", "b", "GitHub Apps"); + let before = Store.get(questions, "w")!; + before.model = before.model.fork(65_537); + let human = Question.crdt.Model.fromBinary(before.model.toBinary()) + .fork(65_536) as unknown as Question.Model; + human.api.val(["q", "mode"]).set("choices"); + human.api.val(["q", "choice"]).set("b"); + let patch = human.api.flush(); + if (!patch) throw new Error("selection made no patch"); + expect(Store.suggest(questions, "w", { optionId: "a", messageIds: ["m1"] })) + .toMatchObject({ ok: true }); + expect(Store.edit(questions, "w", [...patch.toBinary()], "ana")).toMatchObject({ + open: true, + accepted: true, + applied: true, + }); + let entry = Store.get(questions, "w")!; + expect(Question.read(entry.model, entry.definition).q!.choice).toBe("b"); + expect(entry.suggested).toBeUndefined(); + expect(entry.editors).toContain("ana"); + }); + + it("keeps a delayed human selection after several changed server suggestions", () => { + let questions = asked(); + Store.addOption(questions, "w", "b", "GitHub Apps"); + let entry = Store.get(questions, "w")!; + entry.model = entry.model.fork(65_537); + let human = Question.crdt.Model.fromBinary(entry.model.toBinary()) + .fork(65_536) as unknown as Question.Model; + human.api.val(["q", "mode"]).set("choices"); + human.api.val(["q", "choice"]).set("b"); + let patch = human.api.flush(); + if (!patch) throw new Error("selection made no patch"); + for (let [index, optionId] of ["a", "b", "a", "b", "a"].entries()) { + expect(Store.suggest(questions, "w", { optionId, messageIds: [`m${index}`] })) + .toMatchObject({ ok: true }); + } + expect(Store.edit(questions, "w", [...patch.toBinary()], "ana")) + .toMatchObject({ open: true, accepted: true, applied: true }); + expect(Question.read(entry.model, entry.definition).q!.choice).toBe("b"); + expect(entry.suggested).toBeUndefined(); + }); + + it("keeps two queued human choices and converges peers after repeated suggestions", () => { + let questions = asked(); + Store.addOption(questions, "w", "b", "GitHub Apps"); + Store.addOption(questions, "w", "c", "Own service"); + let entry = Store.get(questions, "w")!; + entry.model = entry.model.fork(65_537); + let human = Question.crdt.Model.fromBinary(entry.model.toBinary()) + .fork(65_536) as unknown as Question.Model; + let peer = Question.crdt.Model.fromBinary(entry.model.toBinary()) + .fork(65_538) as unknown as Question.Model; + human.api.val(["q", "mode"]).set("choices"); + human.api.val(["q", "choice"]).set("b"); + let first = human.api.flush(); + human.api.val(["q", "mode"]).set("choices"); + human.api.val(["q", "choice"]).set("c"); + let second = human.api.flush(); + if (!first || !second) throw new Error("selections made no patches"); + for (let [index, optionId] of ["a", "b", "a", "b", "a"].entries()) { + let suggestion = Store.suggest(questions, "w", { + optionId, + messageIds: [`m${index}`], + }); + if (!suggestion.ok) throw new Error("suggestion was refused"); + expect(suggestion.patch).toEqual([]); + } + for (let patch of [first, second]) { + expect(Store.edit(questions, "w", [...patch.toBinary()], "ana")) + .toMatchObject({ open: true, accepted: true, applied: true }); + peer.applyPatch(patch); + } + expect(Question.read(entry.model, entry.definition).q!.choice).toBe("c"); + expect(Question.read(human, entry.definition).q!.choice).toBe("c"); + expect(Question.read(peer, entry.definition).q!.choice).toBe("c"); + }); +}); diff --git a/apps/server/src/questions/store-suggestion-state.test.ts b/apps/server/src/questions/store-suggestion-state.test.ts new file mode 100644 index 00000000..93b478f4 --- /dev/null +++ b/apps/server/src/questions/store-suggestion-state.test.ts @@ -0,0 +1,120 @@ +import { describe, expect, it } from "bun:test"; +import * as Question from "@chopin/question"; + +import * as Store from "./store"; +import { asked } from "./store.test-fixtures"; + +// Exact archive callbacks: 446a9779a937fa5be7cd3eb52fd7f3023d691ed2, +// apps/server/src/questions/store.test.ts. +describe("Store.suggest state", () => { + it("retains a known suggestion without changing the human-owned draft", () => { + let questions = asked(); + let result = Store.suggest(questions, "w", { optionId: "a", messageIds: ["m1"] }); + expect(result).toMatchObject({ ok: true, revision: 1 }); + let entry = Store.get(questions, "w")!; + expect(Question.read(entry.model, entry.definition).q!.choice).toBeNull(); + expect(entry.suggested).toEqual({ optionId: "a", messageIds: ["m1"], revision: 1 }); + expect(Store.restore(Store.dump(questions)).open.get("w")!.suggested).toEqual({ + optionId: "a", + messageIds: ["m1"], + revision: 1, + }); + }); + + it("replaying the same suggestion is a no-op, while new evidence can move it", () => { + let questions = asked(); + Store.addOption(questions, "w", "b", "GitHub Apps"); + expect(Store.suggest(questions, "w", { optionId: "a", messageIds: ["m1"] })) + .toMatchObject({ ok: true, revision: 2 }); + expect(Store.suggest(questions, "w", { optionId: "a", messageIds: ["m1"] })) + .toEqual({ ok: true, patch: [], revision: 2 }); + expect(Store.suggest(questions, "w", { optionId: "b", messageIds: ["m2"] })) + .toMatchObject({ ok: true, revision: 3 }); + expect(Store.get(questions, "w")!.suggested).toEqual({ + optionId: "b", + messageIds: ["m2"], + revision: 3, + }); + }); + + it("does not replace a human selection and clears a suggestion only after an applied edit", () => { + let questions = asked(); + expect(Store.addOption(questions, "w", "b", "GitHub Apps")).toMatchObject({ ok: true }); + let entry = Store.get(questions, "w")!; + let chosen = entry.model.clone(); + chosen.api.val(["q", "choice"]).set("b"); + let patch = chosen.api.flush(); + if (!patch) throw new Error("selection made no patch"); + expect(Store.edit(questions, "w", [...patch.toBinary()], "ana")).toMatchObject({ + accepted: true, + applied: true, + }); + expect(Store.suggest(questions, "w", { optionId: "a", messageIds: ["m1"] })) + .toEqual({ ok: false, reason: "chosen" }); + expect(Question.read(entry.model, entry.definition).q!.choice).toBe("b"); + + let fresh = asked(); + Store.addOption(fresh, "w", "b", "GitHub Apps"); + Store.suggest(fresh, "w", { optionId: "a", messageIds: ["m1"] }); + let suggested = Store.get(fresh, "w")!; + let human = suggested.model.clone(); + human.api.val(["q", "choice"]).set("b"); + let humanPatch = human.api.flush(); + if (!humanPatch) throw new Error("selection made no patch"); + expect(Store.edit(fresh, "w", [...humanPatch.toBinary()], "ben")).toMatchObject({ + accepted: true, + applied: true, + }); + expect(Store.get(fresh, "w")!.suggested).toBeUndefined(); + }); + + it("rejects unknown options, closed cards, and malformed stored sources", () => { + let questions = asked(); + expect(Store.suggest(questions, "w", { optionId: "missing", messageIds: [] })) + .toEqual({ ok: false, reason: "unknown" }); + expect(Store.suggest(questions, "missing", { optionId: "a", messageIds: [] })) + .toEqual({ ok: false, reason: "closed" }); + expect(Store.suggest(questions, "w", { optionId: "a", messageIds: ["m1"] })) + .toMatchObject({ ok: true }); + let stored = Store.dump(questions)[0]!; + expect(() => + Store.restore([{ + ...stored, + suggested: { + ...stored.suggested!, + optionId: "missing", + }, + }]) + ) + .toThrow(/suggestion/); + expect(() => + Store.restore([{ + ...stored, + suggested: { ...stored.suggested!, messageIds: ["x".repeat(201)] }, + }]) + ).toThrow(/suggestion/); + expect(() => + Store.restore([{ + ...stored, + suggested: { + ...stored.suggested!, + revision: 0, + }, + }]) + ).toThrow(/suggestion/); + let chosen = Store.get(questions, "w")!.model.clone(); + chosen.api.val(["q", "choice"]).set("a"); + expect(() => + Store.restore([{ + ...stored, + model: [...chosen.toBinary()], + }]) + ).toThrow(/suggestion/); + expect(() => + Store.restore([{ + ...stored, + suggested: { ...stored.suggested!, extra: true }, + } as never]) + ).toThrow(/suggestion/); + }); +}); diff --git a/apps/server/src/questions/store-suggestion-submit.test.ts b/apps/server/src/questions/store-suggestion-submit.test.ts new file mode 100644 index 00000000..d32b1e4c --- /dev/null +++ b/apps/server/src/questions/store-suggestion-submit.test.ts @@ -0,0 +1,58 @@ +import { describe, expect, it } from "bun:test"; + +import * as Store from "./store"; +import { asked } from "./store.test-fixtures"; + +// Exact archive callbacks: 446a9779a937fa5be7cd3eb52fd7f3023d691ed2, +// apps/server/src/questions/store.test.ts. +describe("Store.suggest submit", () => { + it("claims only the current visible suggestion while the draft is untouched", () => { + let questions = asked(); + Store.addOption(questions, "w", "b", "GitHub Apps"); + let first = Store.suggest(questions, "w", { optionId: "a", messageIds: ["m1"] }); + if (!first.ok) throw new Error("suggestion was refused"); + expect(Store.claimSubmit(questions, "w", first.revision, "ana")).toMatchObject({ + ok: false, + reason: "invalid", + }); + let moved = Store.suggest(questions, "w", { optionId: "b", messageIds: ["m2"] }); + if (!moved.ok) throw new Error("suggestion was refused"); + expect(Store.claimSubmit(questions, "w", first.revision, "ana", "a")).toEqual({ + ok: false, + reason: "stale", + current: moved.revision, + }); + expect(Store.claimSubmit(questions, "w", moved.revision, "ana", "a")).toMatchObject({ + ok: false, + reason: "invalid", + }); + let claim = Store.claimSubmit(questions, "w", moved.revision, "ana", "b"); + if (!claim.ok) throw new Error("current suggestion was refused"); + expect(claim.answers).toEqual([{ + question: "What auth system should we use?", + choices: ["GitHub Apps"], + optionIds: ["b"], + }]); + Store.rollback(questions, claim.claim); + }); + + it("refuses a suggestion fallback after a human draft edit", () => { + let questions = asked(); + let suggestion = Store.suggest(questions, "w", { optionId: "a", messageIds: ["m1"] }); + if (!suggestion.ok) throw new Error("suggestion was refused"); + let entry = Store.get(questions, "w")!; + let human = entry.model.fork(); + human.api.val(["q", "mode"]).set("custom"); + let patch = human.api.flush(); + if (!patch) throw new Error("human edit made no patch"); + expect(Store.edit(questions, "w", [...patch.toBinary()], "ana")).toMatchObject({ + accepted: true, + applied: true, + }); + expect(entry.suggested).toBeUndefined(); + expect(Store.claimSubmit(questions, "w", entry.revision, "ana", "a")).toMatchObject({ + ok: false, + reason: "invalid", + }); + }); +}); diff --git a/apps/server/src/questions/store-suggestions.ts b/apps/server/src/questions/store-suggestions.ts new file mode 100644 index 00000000..60300288 --- /dev/null +++ b/apps/server/src/questions/store-suggestions.ts @@ -0,0 +1,50 @@ +import * as Question from "@chopin/question"; + +import type { Draft } from "@chopin/question"; +import type { Questions } from "./store-types"; + +export function untouched(draft: Draft): boolean { + return draft.mode === "choices" && draft.choice === null && draft.custom.length === 0 + && !Object.values(draft.options).some(Boolean); +} + +/** Persist or clear an advisory conversation option without editing the shared draft. */ +export function suggest( + questions: Questions, + id: string, + suggestion?: { optionId: string; messageIds: string[] }, +): { ok: true; patch: number[]; revision: number } | { + ok: false; + reason: "closed" | "chosen" | "unknown"; +} { + let entry = questions.open.get(id); + if (!entry || entry.claim) return { ok: false, reason: "closed" }; + if (!suggestion) { + if (!entry.suggested) return { ok: true, patch: [], revision: entry.revision }; + entry.revision++; + entry.suggested = undefined; + return { ok: true, patch: [], revision: entry.revision }; + } + let question = entry.definition.questions[0]; + if ( + entry.definition.questions.length !== 1 || !question || question.multiple + || !question.options.some(option => option.id === suggestion.optionId) + || !Array.isArray(suggestion.messageIds) || suggestion.messageIds.length > 16 + || suggestion.messageIds.some(id => typeof id !== "string" || !id || id.length > 200) + || new Set(suggestion.messageIds).size !== suggestion.messageIds.length + ) return { ok: false, reason: "unknown" }; + let draft = Question.read(entry.model, entry.definition)[question.id]!; + if (!untouched(draft)) return { ok: false, reason: "chosen" }; + if ( + entry.suggested?.optionId === suggestion.optionId + && entry.suggested.messageIds.length === suggestion.messageIds.length + && entry.suggested.messageIds.every((id, index) => id === suggestion.messageIds[index]) + ) return { ok: true, patch: [], revision: entry.revision }; + entry.revision++; + entry.suggested = { + optionId: suggestion.optionId, + messageIds: [...suggestion.messageIds], + revision: entry.revision, + }; + return { ok: true, patch: [], revision: entry.revision }; +} diff --git a/apps/server/src/questions/store-types.ts b/apps/server/src/questions/store-types.ts new file mode 100644 index 00000000..b821f11d --- /dev/null +++ b/apps/server/src/questions/store-types.ts @@ -0,0 +1,59 @@ +import type { Answer, Definition, Model } from "@chopin/question"; + +export type Collaborator = { + client: string; + handle: string; + question?: string; + field?: "choices" | "custom"; +}; + +export type Ended = + | { status: "answered"; answers: Answer[]; resolver: string } + | { status: "cancelled"; resolver: string }; + +export type Open = { + id: string; + definition: Definition; + /** The plan node this belongs to, when it has one. */ + widget?: string; + model: Model; + revision: number; + presence: Map; + /** Handles whose edits were accepted, in first-edit order. */ + editors: Set; + /** An advisory chat pre-selection; the draft remains human-owned. */ + suggested?: { optionId: string; messageIds: string[]; revision: number }; + /** Set while a resolution is in flight; blocks edits and rival claims. */ + claim?: "submit" | "cancel" | "option" | "edit"; + /** Resolves the promise the agent is waiting on. */ + settle?: (ended: Ended) => void; +}; + +export type Closed = { result: Ended; revision: number; expires: number }; + +export type Claim = { + id: string; + entry: Open; + result: Ended; +}; + +export type Questions = { + open: Map; + /** + * Tombstones. + * + * A submit that arrives just after somebody else's would otherwise be told + * the questionnaire never existed, which reads as an error rather than as + * "they got there first". + */ + closed: Map; +}; + +export type StoredOpen = { + id: string; + definition: Definition; + widget?: string; + model: number[]; + revision: number; + suggested?: { optionId: string; messageIds: string[]; revision: number }; +}; diff --git a/apps/server/src/questions/store.test-fixtures.ts b/apps/server/src/questions/store.test-fixtures.ts new file mode 100644 index 00000000..a88e2f0a --- /dev/null +++ b/apps/server/src/questions/store.test-fixtures.ts @@ -0,0 +1,32 @@ +import * as Question from "@chopin/question"; + +import * as Store from "./store"; + +// Exact archive helpers: 446a9779a937fa5be7cd3eb52fd7f3023d691ed2, +// apps/server/src/questions/store.test.ts. +export function asked() { + let questions = Store.create(); + let definition = Question.decision({ + questions: [{ + id: "q", + header: "Auth", + question: "What auth system should we use?", + multiple: false, + options: [{ id: "a", label: "Auth0", description: "" }], + }], + }); + void Store.ask(questions, "w", definition, "w"); + return questions; +} + +export function legacy() { + return { + questions: ["q1", "q2"].map(id => ({ + id, + header: id, + question: `${id}?`, + multiple: false, + options: [{ id: `${id}-a`, label: "One", description: "" }], + })), + }; +} diff --git a/apps/server/src/questions/store.ts b/apps/server/src/questions/store.ts index 32069839..92dc00b2 100644 --- a/apps/server/src/questions/store.ts +++ b/apps/server/src/questions/store.ts @@ -16,64 +16,38 @@ import * as Question from "@chopin/question"; -import type { Answer, DecisionDefinition, Definition, Drafts, Model } from "@chopin/question"; +import { untouched } from "./store-suggestions"; + +import type { Answer, DecisionDefinition, Definition, Drafts } from "@chopin/question"; +import type { + Claim, + Closed, + Collaborator, + Ended, + Open, + Questions, + StoredOpen, +} from "./store-types"; + +export { reopen } from "./store-lifecycle"; +export { + addOption, + before, + relabelOption, + releaseEdit, + releaseOption, + reserveEdit, + reserveOption, + retitle, + revert, +} from "./store-options"; +export type { AddedOption, Before, Retitled } from "./store-options"; +export { suggest } from "./store-suggestions"; +export type { Claim, Collaborator, Ended, Questions, StoredOpen } from "./store-types"; /** How long a resolved questionnaire is remembered, for late arrivals. */ const CLOSED_TTL = 5 * 60 * 1_000; -export type Collaborator = { - client: string; - handle: string; - question?: string; - field?: "choices" | "custom"; -}; - -export type Ended = - | { status: "answered"; answers: Answer[]; resolver: string } - | { status: "cancelled"; resolver: string }; - -type Open = { - id: string; - definition: DecisionDefinition; - /** The plan node this belongs to, when it has one. */ - widget?: string; - model: Model; - revision: number; - presence: Map; - /** Set while a resolution is in flight; blocks edits and rival claims. */ - claim?: "submit" | "cancel"; - /** Resolves the promise the agent is waiting on. */ - settle?: (ended: Ended) => void; -}; - -type Closed = { result: Ended; revision: number; expires: number }; - -export type Claim = { - id: string; - entry: Open; - result: Ended; -}; - -export type Questions = { - open: Map; - /** - * Tombstones. - * - * A submit that arrives just after somebody else's would otherwise be told - * the questionnaire never existed, which reads as an error rather than as - * "they got there first". - */ - closed: Map; -}; - -export type StoredOpen = { - id: string; - definition: DecisionDefinition; - widget?: string; - model: number[]; - revision: number; -}; - export function create(): Questions { return { open: new Map(), closed: new Map() }; } @@ -85,6 +59,7 @@ export function dump(questions: Questions): StoredOpen[] { ...(entry.widget ? { widget: entry.widget } : {}), model: [...entry.model.toBinary()], revision: entry.revision, + ...(entry.suggested ? { suggested: entry.suggested } : {}), })); } @@ -99,14 +74,36 @@ export function restore(entries: StoredOpen[]): Questions { !Array.isArray(entry.model) || entry.model.some(value => !Number.isInteger(value) || value < 0 || value > 255) ) Question.reject("Questionnaire model is invalid"); - let definition = Question.decision(entry.definition); + let definition = Question.identified(entry.definition); + if (entry.suggested !== undefined) { + let suggested = entry.suggested; + if ( + !suggested || typeof suggested !== "object" || Array.isArray(suggested) + || Object.keys(suggested).sort().join(",") !== "messageIds,optionId,revision" + || typeof suggested.optionId !== "string" + || !Number.isSafeInteger(suggested.revision) || suggested.revision < 1 + || suggested.revision !== entry.revision + || !definition.questions[0]?.options.some(option => option.id === suggested.optionId) + || definition.questions.length !== 1 || definition.questions[0]?.multiple + || !Array.isArray(suggested.messageIds) || suggested.messageIds.length > 16 + || suggested.messageIds.some(id => typeof id !== "string" || !id || id.length > 200) + || new Set(suggested.messageIds).size !== suggested.messageIds.length + ) Question.reject("Questionnaire suggestion is invalid"); + } + let model = Question.restore(entry.model, definition); + if (entry.suggested) { + let draft = Question.read(model, definition)[definition.questions[0]!.id]!; + if (!untouched(draft)) Question.reject("Questionnaire suggestion is invalid"); + } questions.open.set(entry.id, { id: entry.id, definition, ...(entry.widget ? { widget: entry.widget } : {}), - model: Question.restore(entry.model, definition), + model, revision: entry.revision, + ...(entry.suggested ? { suggested: entry.suggested } : {}), presence: new Map(), + editors: new Set(), }); } return questions; @@ -154,6 +151,7 @@ export function ask( model, revision: 0, presence: new Map(), + editors: new Set(), settle, }); }); @@ -173,7 +171,7 @@ export function redefine( questions: Questions, id: string, definition: DecisionDefinition, -): { ok: true; previous: DecisionDefinition } | { ok: false; reason: "resolved" | "resolving" } { +): { ok: true; previous: Definition } | { ok: false; reason: "resolved" | "resolving" } { let entry = questions.open.get(id); if (!entry) return { ok: false, reason: "resolved" }; if (entry.claim) return { ok: false, reason: "resolving" }; @@ -186,7 +184,7 @@ export function redefine( export function restoreDefinition( questions: Questions, id: string, - definition: DecisionDefinition, + definition: Definition, ): void { let entry = questions.open.get(id); if (entry) entry.definition = definition; @@ -195,7 +193,7 @@ export function restoreDefinition( /** Everything still open, for a client that has just joined. */ export function outstanding( questions: Questions, -): Array<{ id: string; definition: DecisionDefinition; widget?: string }> { +): Array<{ id: string; definition: Definition; widget?: string }> { return [...questions.open.values()].map(entry => ({ id: entry.id, definition: entry.definition, @@ -206,7 +204,7 @@ export function outstanding( export type Opened = | { open: true; - definition: DecisionDefinition; + definition: Definition; model: number[]; revision: number; presence: Collaborator[]; @@ -237,7 +235,7 @@ export type Edited = * `Question.apply`. What is decided here is who may ask: a questionnaire that * is resolving takes no more edits, because its answer has already been read. */ -export function edit(questions: Questions, id: string, binary: number[]): Edited { +export function edit(questions: Questions, id: string, binary: number[], editor?: string): Edited { let entry = questions.open.get(id); if (!entry) { let ended = questions.closed.get(id); @@ -263,6 +261,8 @@ export function edit(questions: Questions, id: string, binary: number[]): Edited entry.model = outcome.model; entry.revision++; + entry.suggested = undefined; + if (editor) entry.editors.add(editor); return { open: true, accepted: true, applied: true, revision: entry.revision }; } @@ -333,6 +333,7 @@ export function claimSubmit( id: string, revision: number, resolver: string, + suggestedOptionId?: string, ): { ok: true; claim: Claim; answers: Answer[]; widget?: string } | SubmitRefusal { let ended = questions.closed.get(id); if (ended) return resolved(ended); @@ -355,6 +356,17 @@ export function claimSubmit( }; } + if (suggestedOptionId !== undefined) { + let question = entry.definition.questions[0]; + let draft = question && drafts[question.id]; + if ( + entry.definition.questions.length !== 1 || !question || question.multiple || !draft + || !untouched(draft) || !entry.suggested + || entry.suggested.optionId !== suggestedOptionId + || !question.options.some(option => option.id === suggestedOptionId) + ) return { ok: false, reason: "invalid", message: "Suggestion is no longer current" }; + drafts = { ...drafts, [question.id]: { ...draft, choice: suggestedOptionId } }; + } let outcome = Question.derive(entry.definition, drafts); if (!outcome.ok) return { ok: false, reason: "invalid", message: outcome.message }; diff --git a/packages/dialect/src/limits.ts b/packages/dialect/src/limits.ts index 0fbf44ea..ab30a8e9 100644 --- a/packages/dialect/src/limits.ts +++ b/packages/dialect/src/limits.ts @@ -20,6 +20,9 @@ export const MAX_COLLAB_BYTES = 4 * 1024 * 1024; export const MAX_TABLE_ROWS = 100; export const MAX_TABLE_COLUMNS = 20; +/** Matches the conversation thread ID bound in conversation-plan validation. */ +export const MAX_ID = 200; + /** Image nodes per plan. Each is a remote fetch when the plan renders. */ export const MAX_IMAGES = 100; diff --git a/packages/protocol/question.d.ts b/packages/protocol/question.d.ts index e22e5c9f..a85489c9 100644 --- a/packages/protocol/question.d.ts +++ b/packages/protocol/question.d.ts @@ -84,6 +84,8 @@ export declare namespace Question { export type Answer = { question: string; choices?: string[]; + /** Ids of the chosen options, alongside their labels. */ + optionIds?: string[]; custom?: string; }; @@ -106,14 +108,14 @@ export declare namespace Question { /** A new questionnaire, announced to the room. */ export type Asked = KIND<"question:asked"> & { id: string; - definition: DecisionDefinition; + definition: Definition; /** Present once the questionnaire has a node in the plan. */ widget?: string; }; /** Every open questionnaire, sent when a client joins. */ export type Sync = KIND<"question:sync"> & { - open: Array<{ id: string; definition: DecisionDefinition; widget?: string }>; + open: Array<{ id: string; definition: Definition; widget?: string }>; }; export namespace Open { @@ -124,7 +126,7 @@ export declare namespace Question { & ( | { open: true; - definition: DecisionDefinition; + definition: Definition; /** json-joy model, as bytes. */ model: number[]; revision: number; diff --git a/packages/question/src/answer.ts b/packages/question/src/answer.ts index 799c64b5..20a2c12a 100644 --- a/packages/question/src/answer.ts +++ b/packages/question/src/answer.ts @@ -18,9 +18,7 @@ export type Outcome = /** * Derive answers from a draft. * - * Answers carry the question text and chosen labels rather than identifiers: - * the agent reads them as prose, and they stay meaningful in a transcript long - * after the definition is gone. + * Answers carry readable labels for the agent and identifiers for the record. */ export function derive(definition: Definition, drafts: Drafts): Outcome { let answers: Answer[] = []; @@ -58,6 +56,7 @@ export function derive(definition: Definition, drafts: Drafts): Outcome { answers.push({ question: question.question, choices: selected.map(option => option.label), + optionIds: selected.map(option => option.id), }); } diff --git a/packages/question/src/draft.ts b/packages/question/src/draft.ts index fa0036a2..94311157 100644 --- a/packages/question/src/draft.ts +++ b/packages/question/src/draft.ts @@ -39,9 +39,7 @@ export function create(definition: Definition): Model { } questions[question.id] = crdt.schema.obj({ - mode: crdt.schema.val( - crdt.schema.con(question.options.length ? "choices" : "custom"), - ), + mode: crdt.schema.val(crdt.schema.con("choices")), choice: crdt.schema.val(crdt.schema.con(null)), options: crdt.schema.obj(options), // A CRDT string, so two people typing a custom answer merge rather diff --git a/packages/question/src/index.ts b/packages/question/src/index.ts index 63ff6312..9fd9752d 100644 --- a/packages/question/src/index.ts +++ b/packages/question/src/index.ts @@ -12,7 +12,15 @@ export * as limits from "./limits"; -export { appendOption, assertCallId, decision, normalize, QuestionError, reject } from "./schema"; +export { + appendOption, + assertCallId, + decision, + identified, + normalize, + QuestionError, + reject, +} from "./schema"; export type { Answer, Appended, DecisionDefinition, Definition, Item, Option } from "./schema"; export { answered, apply, assertPatch, create, read, restore } from "./draft"; @@ -21,6 +29,9 @@ export type { Applied, Draft, Drafts, Mode, Model } from "./draft"; export { derive, incomplete, summarize } from "./answer"; export type { Outcome } from "./answer"; +export { addOption } from "./options"; +export type { Added } from "./options"; + /** * The CRDT itself. * diff --git a/packages/question/src/limits.ts b/packages/question/src/limits.ts index 7a3fb939..2390e242 100644 --- a/packages/question/src/limits.ts +++ b/packages/question/src/limits.ts @@ -7,6 +7,8 @@ export const MAX_QUESTIONS = 10; export const MAX_OPTIONS = 20; +/** Options a person may grow a decision card to. */ +export const MAX_DECISION_OPTIONS = 10; /** The most options a question may hold once members start appending their own. */ export const MAX_SHARED_OPTIONS = 10; export const MAX_HEADER = 80; diff --git a/packages/question/src/options.test.ts b/packages/question/src/options.test.ts new file mode 100644 index 00000000..10be63f7 --- /dev/null +++ b/packages/question/src/options.test.ts @@ -0,0 +1,124 @@ +import { describe, expect, it } from "bun:test"; + +import { create, read } from "./draft"; +import { derive } from "./answer"; +import * as limits from "./limits"; +import { addOption } from "./options"; +import { decision, identified, normalize } from "./schema"; + +function start(count = 2) { + let definition = decision(normalize({ + questions: [{ + header: "Auth", + question: "What auth system should we use?", + multiple: false, + options: Array.from({ length: count }, (_, index) => ({ + label: `Option ${index}`, + description: "", + })), + }], + })); + return { definition, model: create(definition) }; +} + +describe("addOption", () => { + it("appends an option to the definition and an unselected register to the draft", () => { + let { definition, model } = start(); + let result = addOption(definition, model, "o9", " GitHub Apps "); + + expect(result.ok).toBe(true); + if (!result.ok) return; + expect(result.option).toEqual({ id: "o9", label: "GitHub Apps", description: "" }); + expect(result.definition.questions[0]!.options.map(option => option.id)).toEqual([ + "o0", + "o1", + "o9", + ]); + let drafts = read(result.model, result.definition); + expect(drafts.q0!.options).toEqual({ o0: false, o1: false, o9: false }); + }); + + it("keeps an existing selection", () => { + let { definition, model } = start(); + model.api.val(["q0", "choice"]).set("o1"); + let result = addOption(definition, model, "o9", "Roll our own"); + expect(result.ok && read(result.model, result.definition).q0!.choice).toBe("o1"); + }); + + it("does not mutate its inputs", () => { + let { definition, model } = start(); + let before = [...model.toBinary()]; + addOption(definition, model, "o9", "Roll our own"); + expect([...model.toBinary()]).toEqual(before); + expect(definition.questions[0]!.options).toHaveLength(2); + }); + + it("refuses a duplicate label, ignoring case and surrounding space", () => { + let { definition, model } = start(); + let result = addOption(definition, model, "o9", " option 1 "); + expect(result).toMatchObject({ ok: false, reason: "duplicate" }); + }); + + it("refuses an existing option ID without clearing its draft selection", () => { + let { definition } = start(); + let multiple = decision({ + questions: [{ ...definition.questions[0]!, multiple: true }], + }); + let model = create(multiple); + model.api.val(["q0", "options", "o1"]).set(true); + model.api.flush(); + let before = [...model.toBinary()]; + + expect(addOption(multiple, model, "o1", "Another option")).toMatchObject({ + ok: false, + reason: "duplicate", + }); + expect(read(model, multiple).q0!.options.o1).toBe(true); + expect([...model.toBinary()]).toEqual(before); + }); + + it("refuses an empty or over-long label", () => { + let { definition, model } = start(); + expect(addOption(definition, model, "o9", " ")).toMatchObject({ + ok: false, + reason: "invalid", + }); + expect(addOption(definition, model, "o9", "x".repeat(limits.MAX_LABEL + 1))) + .toMatchObject({ ok: false, reason: "invalid" }); + }); + + it("refuses once the card holds the decision-card maximum", () => { + let { definition, model } = start(limits.MAX_DECISION_OPTIONS); + expect(addOption(definition, model, "o99", "One more")).toMatchObject({ + ok: false, + reason: "full", + }); + }); +}); + +it("a pending card grows its first option and derives its selected label and ID", () => { + let definition = decision(identified({ + questions: [{ + id: "pending", + header: "Auth", + question: "Which system?", + multiple: false, + options: [], + }], + })); + let model = create(definition); + let added = addOption(definition, model, "stable-option", "GitHub Apps"); + expect(added.ok).toBe(true); + if (!added.ok) return; + expect(read(added.model, added.definition).pending!.mode).toBe("choices"); + added.model.api.val(["pending", "choice"]).set("stable-option"); + added.model.api.flush(); + expect(derive(added.definition, read(added.model, added.definition))).toEqual({ + ok: true, + answers: [{ + question: "Which system?", + choices: ["GitHub Apps"], + optionIds: ["stable-option"], + }], + }); +}); diff --git a/packages/question/src/options.ts b/packages/question/src/options.ts new file mode 100644 index 00000000..bfe5f454 --- /dev/null +++ b/packages/question/src/options.ts @@ -0,0 +1,60 @@ +/** + * Growing a decision. + * + * The one way a definition changes after it is asked: people add options. + * The definition and the shared draft change together, because the draft's + * shape is derived from the definition and `read` rejects any mismatch. + */ + +import { crdt, read } from "./draft"; +import * as limits from "./limits"; +import { decision } from "./schema"; + +import type { Model } from "./draft"; +import type { DecisionDefinition, Option } from "./schema"; + +export type Added = + | { ok: true; definition: DecisionDefinition; model: Model; option: Option } + | { ok: false; reason: "full" | "duplicate" | "invalid"; message: string }; + +export function addOption( + definition: DecisionDefinition, + model: Model, + id: string, + label: string, +): Added { + let question = definition.questions[0]!; + let text = label.trim(); + if (!text) return { ok: false, reason: "invalid", message: "An option needs a label" }; + if (text.length > limits.MAX_LABEL) { + return { + ok: false, + reason: "invalid", + message: `An option label is at most ${limits.MAX_LABEL} characters`, + }; + } + if (question.options.length >= limits.MAX_DECISION_OPTIONS) { + return { + ok: false, + reason: "full", + message: `A decision holds at most ${limits.MAX_DECISION_OPTIONS} options`, + }; + } + let folded = text.toLowerCase(); + if (question.options.some(option => option.label.trim().toLowerCase() === folded)) { + return { ok: false, reason: "duplicate", message: "That is already an option" }; + } + if (question.options.some(option => option.id === id)) { + return { ok: false, reason: "duplicate", message: "That option ID already exists" }; + } + + let option: Option = { id, label: text, description: "" }; + let next = decision({ questions: [{ ...question, options: [...question.options, option] }] }); + + let copy = model.clone(); + copy.api.obj([question.id, "options"]).set({ [id]: crdt.schema.val(crdt.schema.con(false)) }); + copy.api.flush(); + read(copy, next); + + return { ok: true, definition: next, model: copy, option }; +} diff --git a/packages/question/src/question.test.ts b/packages/question/src/question.test.ts index 9c6d9b97..f31d9855 100644 --- a/packages/question/src/question.test.ts +++ b/packages/question/src/question.test.ts @@ -3,6 +3,7 @@ import { describe, expect, it } from "bun:test"; import { derive, incomplete, summarize } from "./answer"; import { answered, apply, assertPatch, crdt, create, read } from "./draft"; import * as limits from "./limits"; +import { identified } from "./index"; import { appendOption, normalize, QuestionError } from "./schema"; import type { Definition } from "./schema"; @@ -63,6 +64,213 @@ describe("normalize", () => { }); }); +describe("identified", () => { + it("preserves stored IDs, text, and order without copying the definition", () => { + let source = { + questions: [{ + id: "question-z", + header: " Rollout ", + question: "How?", + multiple: false, + options: [ + { id: "option-z", label: "Canary", description: "" }, + { id: "option-a", label: "Blue-green", description: "" }, + ], + }, { + id: "question-a", + header: "Timing", + question: "When?", + multiple: false, + options: [{ id: "option-t", label: "Today", description: "" }], + }], + }; + let before = structuredClone(source); + expect(identified(source)).toBe(source); + expect(source).toEqual(before); + expect(source.questions.map(question => question.id)).toEqual(["question-z", "question-a"]); + expect(source.questions[0]!.options.map(option => option.id)).toEqual(["option-z", "option-a"]); + }); + it("accepts a pending single-choice card with no quoted options but keeps tool input strict", () => { + let pending = { + questions: [{ + id: "q1", + header: "Auth", + question: "What auth system should we use?", + multiple: false, + options: [], + }], + }; + expect(identified(pending)).toBe(pending); + expect(read(create(pending), pending).q1).toEqual({ + mode: "choices", + choice: null, + options: {}, + custom: "", + }); + expect(derive(pending, read(create(pending), pending)).ok).toBe(false); + expect(() => normalize(tool({ options: [] }))).toThrow(/at least one option/); + expect(() => + identified({ + questions: [{ ...pending.questions[0], multiple: true }], + }) + ).toThrow(/invalid options/); + }); + + it("keeps stable IDs and refuses oversized raw values with surrounding spaces", () => { + let source = { + questions: [{ + id: "q1", + header: "Rollout", + question: "How?", + multiple: false, + options: [{ id: "o1", label: "Canary", description: "" }], + }], + }; + expect(identified(source)).toBe(source); + expect(() => + identified({ + questions: [{ ...source.questions[0], id: ` ${"q".repeat(limits.MAX_CALL_ID)} ` }], + }) + ).toThrow(/exceeds/); + expect(() => + identified({ + questions: [{ ...source.questions[0], header: ` ${"x".repeat(limits.MAX_HEADER)} ` }], + }) + ).toThrow(/exceeds/); + expect(() => + identified({ + questions: [{ + ...source.questions[0], + options: [{ + ...source.questions[0]!.options[0], + label: ` ${"x".repeat(limits.MAX_LABEL)} `, + }], + }], + }) + ).toThrow(/exceeds/); + }); + + it("rejects duplicate question IDs and option IDs across the questionnaire", () => { + let question = { + id: "q1", + header: "Rollout", + question: "How?", + multiple: false, + options: [{ id: "o1", label: "Canary", description: "" }], + }; + expect(() => identified({ questions: [question, { ...question }] })) + .toThrow(/duplicate question IDs/); + expect(() => identified({ questions: [question, { ...question, id: "q2" }] })) + .toThrow(/duplicate option IDs/); + expect(() => + identified({ + questions: [{ ...question, options: [...question.options, ...question.options] }], + }) + ) + .toThrow(/duplicate option IDs/); + }); + + it("rejects unknown fields at every stored definition level", () => { + let question = { + id: "q1", + header: "Rollout", + question: "How?", + multiple: false, + options: [{ id: "o1", label: "Canary", description: "" }], + }; + for ( + let source of [ + { questions: [question], extra: true }, + { questions: [{ ...question, extra: true }] }, + { questions: [{ ...question, options: [{ ...question.options[0], extra: true }] }] }, + ] + ) expect(() => identified(source)).toThrow(/invalid fields/); + }); + + it("enforces question and option counts and rejects empty options for multiple questions", () => { + let question = { + id: "q1", + header: "Rollout", + question: "How?", + multiple: false, + options: [{ id: "o1", label: "Canary", description: "" }], + }; + let questions = Array.from({ length: limits.MAX_QUESTIONS }, (_, index) => ({ + ...question, + id: `q${index}`, + options: [{ ...question.options[0], id: `o${index}` }], + })); + expect(identified({ questions }).questions).toHaveLength(limits.MAX_QUESTIONS); + expect(() => identified({ questions: [] })).toThrow(/invalid question count/); + expect(() => identified({ questions: [...questions, { ...question, id: "extra" }] })) + .toThrow(/invalid question count/); + let options = Array.from({ length: limits.MAX_OPTIONS }, (_, index) => ({ + ...question.options[0], + id: `o${index}`, + })); + expect(identified({ questions: [{ ...question, options }] }).questions[0]!.options) + .toHaveLength(limits.MAX_OPTIONS); + expect(() => + identified({ + questions: [{ + ...question, + options: [...options, { id: "extra", label: "Extra", description: "" }], + }], + }) + ) + .toThrow(/invalid options/); + expect(() => + identified({ questions: [{ ...question, options: [] }, { ...question, id: "q2" }] }) + ) + .toThrow(/invalid options/); + expect(() => identified({ questions: [{ ...question, multiple: "yes" }] })) + .toThrow(/invalid options/); + }); + + it("bounds every raw text field and rejects blank identifiers and required text", () => { + let question = { + id: "q1", + header: "Rollout", + question: "How?", + multiple: false, + options: [{ id: "o1", label: "Canary", description: "" }], + }; + for ( + let [field, maximum] of [ + ["id", limits.MAX_CALL_ID], + ["header", limits.MAX_HEADER], + ["question", limits.MAX_QUESTION], + ] as const + ) { + expect(() => identified({ questions: [{ ...question, [field]: " " }] })).toThrow( + QuestionError, + ); + expect(() => + identified({ questions: [{ ...question, [field]: ` ${"x".repeat(maximum)} ` }] }) + ) + .toThrow(/exceeds/); + } + for ( + let [field, maximum] of [ + ["id", limits.MAX_CALL_ID], + ["label", limits.MAX_LABEL], + ["description", limits.MAX_DESCRIPTION], + ] as const + ) { + let options = [{ ...question.options[0], [field]: ` ${"x".repeat(maximum)} ` }]; + expect(() => identified({ questions: [{ ...question, options }] })).toThrow(/exceeds/); + if (field !== "description") { + expect(() => + identified({ + questions: [{ ...question, options: [{ ...question.options[0], [field]: " " }] }], + }) + ) + .toThrow(QuestionError); + } + } + }); +}); + describe("draft", () => { let definition: Definition = normalize(tool()); @@ -121,7 +329,7 @@ describe("draft", () => { describe("derive", () => { let definition = normalize(tool()); - it("returns the chosen labels, not identifiers", () => { + it("returns the chosen labels and option identifiers", () => { let outcome = derive(definition, { q0: { mode: "choices", choice: "o0", options: {}, custom: "" }, }); @@ -129,7 +337,7 @@ describe("derive", () => { expect(outcome.ok).toBe(true); if (!outcome.ok) return; expect(outcome.answers).toEqual([ - { question: "How should we deploy?", choices: ["Canary"] }, + { question: "How should we deploy?", choices: ["Canary"], optionIds: ["o0"] }, ]); }); @@ -163,6 +371,7 @@ describe("derive", () => { expect(outcome.ok).toBe(true); if (!outcome.ok) return; expect(outcome.answers[0]!.choices).toEqual(["Canary", "Blue-green"]); + expect(outcome.answers[0]!.optionIds).toEqual(["o0", "o1"]); }); it("reports the first unanswered question", () => { diff --git a/packages/question/src/schema.ts b/packages/question/src/schema.ts index b19d4fe7..40dd7883 100644 --- a/packages/question/src/schema.ts +++ b/packages/question/src/schema.ts @@ -53,6 +53,12 @@ function text(value: unknown, name: string, max: number, optional = false): stri return result; } +/** A stored definition keeps its original string, so enforce the bound on that string. */ +function storedText(value: unknown, name: string, max: number, optional = false): void { + text(value, name, max, optional); + if ((value as string).length > max) fail(`${name} exceeds ${max} characters`); +} + /** * Validate an agent's tool input into a frozen definition. * @@ -124,6 +130,54 @@ export function normalize(raw: unknown): Definition { return Object.freeze({ questions }); } +/** Validate a stored or wire definition without changing its durable IDs or order. */ +export function identified(raw: unknown): Definition { + let source = record(raw, "Questionnaire definition"); + exact(source, ["questions"], "Questionnaire definition"); + if ( + !Array.isArray(source.questions) || source.questions.length === 0 + || source.questions.length > limits.MAX_QUESTIONS + ) { + fail("Questionnaire definition has an invalid question count"); + } + let questionIds = new Set(); + let optionIds = new Set(); + for (let [index, candidate] of source.questions.entries()) { + let question = record(candidate, `Question ${index + 1}`); + exact(question, ["id", "header", "question", "options", "multiple"], `Question ${index + 1}`); + storedText(question.id, `Question ${index + 1} id`, limits.MAX_CALL_ID); + let id = question.id as string; + if (questionIds.has(id)) fail("Questionnaire has duplicate question IDs"); + questionIds.add(id); + storedText(question.header, `Question ${index + 1} header`, limits.MAX_HEADER); + storedText(question.question, `Question ${index + 1}`, limits.MAX_QUESTION); + if ( + typeof question.multiple !== "boolean" + || !Array.isArray(question.options) + || question.options.length === 0 && (source.questions.length !== 1 || question.multiple) + || question.options.length > limits.MAX_OPTIONS + ) { + fail(`Question ${index + 1} has invalid options or multiple value`); + } + for (let [position, candidate] of question.options.entries()) { + let option = record(candidate, `Question ${index + 1} option ${position + 1}`); + exact(option, ["id", "label", "description"], `Question ${index + 1} option ${position + 1}`); + storedText(option.id, `Question ${index + 1} option id`, limits.MAX_CALL_ID); + let optionId = option.id as string; + if (optionIds.has(optionId)) fail("Questionnaire has duplicate option IDs"); + optionIds.add(optionId); + storedText(option.label, `Question ${index + 1} option label`, limits.MAX_LABEL); + storedText( + option.description, + `Question ${index + 1} option description`, + limits.MAX_DESCRIPTION, + true, + ); + } + } + return raw as Definition; +} + /** Narrow a questionnaire to the shape owned by one durable decision card. */ export function decision(definition: Definition): DecisionDefinition { if (definition.questions.length !== 1) {