Tick Send to Chopin by default on a new comment

apps/server/src/comments-planner.test.ts

@@ -197,6 +197,31 @@ describe("sending a new comment to the Planner", () => {
 		expect(plan.chat.waiting).toHaveLength(20);
 	});
 
+	it("refuses before saving anything when no remote Planner is attached", async () => {
+		let { broadcasts, context, plan } = await opened({ busy: false });
+		context.plannerNotAttached = async () => "Attach a Planner to this document first.";
+		let ana = member("ana");
+
+		let answered = await start(context, ana);
+		expect(answered).toMatchObject({
+			ok: false,
+			reason: "busy",
+			code: "planner-not-attached",
+			message: "Attach a Planner to this document first.",
+		});
+		expect(plan.threads.size).toBe(0);
+		expect(working(broadcasts)).toHaveLength(0);
+
+		let open = idOf(await start(context, ana, "Plain note.", null));
+		let replied = await reply(context, ana, open, "Now you, Chopin.");
+		expect(replied).toMatchObject({ ok: false, code: "planner-not-attached" });
+		expect(plan.threads.get(open)?.notes).toHaveLength(1);
+		expect(plan.chat.waiting).toHaveLength(0);
+
+		// An ordinary comment is not the Planner's business, attached or not.
+		expect(await reply(context, ana, open, "Just us.", null)).toMatchObject({ ok: true });
+	});
+
 	it("saves the note but says so when no Planner is running", async () => {
 		let { broadcasts, context, plan } = await opened({ agent: false });
 		let ana = member("ana");

apps/server/src/comments/service.ts

@@ -320,14 +320,16 @@ function said(text: string): string | undefined {
 /** How the Planner is reached, for a note sent to it. Absent for an ordinary note. */
 export type Addressed = Chat.Room;
 
-type Admitted = { planner?: Wire.Planner } | { refused: string };
+type Admitted =
+	| { planner?: Wire.Planner }
+	| { refused: string; code?: "planner-not-attached" };
 
 /**
  * Whether a note sent to the Planner can be acted on, asked before it is saved.
  *
- * A full queue refuses the note rather than saving one nobody will act on. A
- * server without a Planner still takes it — it is a comment either way — and
- * says so. A child's note is admitted by its root's queue, which runs its turn,
+ * A full queue refuses the note rather than saving one nobody will act on, as
+ * does a remote Planner nobody has attached. A server without a Planner still
+ * takes it — it is a comment either way — and says so. A child's note is admitted by its root's queue, which runs its turn,
  * and refused if the child no longer belongs to a live root.
  */
 async function admit(
@@ -345,7 +347,12 @@ async function admit(
 			console.error("[comments] could not reach the root document's Chat:", err);
 			return { refused: "Chopin can't be reached from this document right now." };
 		}
-	} else admitted = Chat.admission(context, thread);
+	} else {
+		// Under HARNESS=remote a turn with no Planner attached would never run.
+		let unattached = context.config.agent ? await context.plannerNotAttached?.() : undefined;
+		if (unattached) return { refused: unattached, code: "planner-not-attached" };
+		admitted = Chat.admission(context, thread);
+	}
 	if (admitted === "busy") {
 		return { refused: "Chopin has too much waiting already. Try again when it has caught up." };
 	}
@@ -361,8 +368,19 @@ export async function start(
 	msg: Request<Wire.Start.Ask>,
 	context?: Addressed,
 ): Promise<void> {
-	let refuse = (reason: "invalid" | "full" | "busy", message: string) =>
-		reply(ws, msg.rid, { kind: "comment:start", ts: 0, ok: false, reason, message });
+	let refuse = (
+		reason: "invalid" | "full" | "busy",
+		message: string,
+		code?: "planner-not-attached",
+	) =>
+		reply(ws, msg.rid, {
+			kind: "comment:start",
+			ts: 0,
+			ok: false,
+			reason,
+			message,
+			...(code ? { code } : {}),
+		});
 
 	let full = Store.room(plan.threads);
 	if (full) return refuse("full", full.message);
@@ -372,7 +390,7 @@ export async function start(
 
 	let admitted = await admit(msg, context);
 	if ("refused" in admitted) {
-		return refuse(msg.to === "planner" ? "busy" : "invalid", admitted.refused);
+		return refuse(msg.to === "planner" ? "busy" : "invalid", admitted.refused, admitted.code);
 	}
 
 	let passage: Wired.Passage;
@@ -409,15 +427,23 @@ export async function respond(
 	msg: Request<Wire.Reply.Ask>,
 	context?: Addressed,
 ): Promise<void> {
-	let refuse = (reason: "invalid" | "busy", message: string) =>
-		reply(ws, msg.rid, { kind: "comment:reply", ts: 0, id: msg.id, ok: false, reason, message });
+	let refuse = (reason: "invalid" | "busy", message: string, code?: "planner-not-attached") =>
+		reply(ws, msg.rid, {
+			kind: "comment:reply",
+			ts: 0,
+			id: msg.id,
+			ok: false,
+			reason,
+			message,
+			...(code ? { code } : {}),
+		});
 
 	let text = said(msg.text);
 	if (!text) return refuse("invalid", "A comment needs something in it.");
 
 	let admitted = await admit(msg, context, msg.id);
 	if ("refused" in admitted) {
-		return refuse(msg.to === "planner" ? "busy" : "invalid", admitted.refused);
+		return refuse(msg.to === "planner" ? "busy" : "invalid", admitted.refused, admitted.code);
 	}
 
 	let outcome = Store.reply(

apps/web/src/room-workspace.tsx

@@ -627,15 +627,16 @@ export function RoomWorkspace(
 		};
 	});
 	let typing = useDocumentEditing(room);
+	let plannerDetached = plannerAttachment.wire === wire && !!plannerAttachment.planner
+		&& plannerAttachment.planner.state !== "attached";
 	let build = useBuildPlan({
 		canEdit: effectiveCanEdit && !(archivedAt ?? metadata.archivedAt) && status === "connected",
 		connected: status === "connected",
 		enabled: profile.implementation,
 		hasContent: hasPlanContent,
 		live: liveBuild,
 		planner: agent,
-		plannerDetached: plannerAttachment.wire === wire && !!plannerAttachment.planner
-			&& plannerAttachment.planner.state !== "attached",
+		plannerDetached,
 		room,
 		unanswered,
 		userId,
@@ -1369,6 +1370,7 @@ export function RoomWorkspace(
 				// A child has no Decisions pane to open, so its decisions stay inline.
 				onShowDecision={presentation.type === "child" ? undefined : showDecision}
 				planner={agent}
+				commentPlanner={agent && !plannerDetached}
 				commentPresentation={mode === "split" ? "popover" : "sheet"}
 				connection={status === "deleted" ? "closed" : status}
 				key={workspaceArchivedAt ? "archived" : "active"}

e2e/comment-send-off.e2e.ts

@@ -7,7 +7,10 @@ test("a comment sent to Chopin with no Planner running is saved and says Chopin
 	await content(page).locator("p").first().selectText();
 	await page.getByRole("button", { name: "Comment on this passage", exact: true }).click();
 	let card = page.getByRole("dialog", { name: "New comment" });
-	await card.getByRole("checkbox", { name: "Send to Chopin" }).check();
+	// No Planner runs here, so a new comment is not sent to Chopin unless asked.
+	let address = card.getByRole("checkbox", { name: "Send to Chopin" });
+	await expect(address).not.toBeChecked();
+	await address.check();
 	await card.getByRole("textbox", { name: "Comment" }).fill("Can you shorten this?");
 	await card.getByRole("button", { name: "Post comment", exact: true }).click();
 

e2e/harness-comments.e2e.ts

@@ -25,7 +25,11 @@ test("typing @chopin addresses Chopin, who replies in the thread while Chat reco
 	let field = card.getByRole("textbox", { name: "Comment" });
 	let box = card.getByRole("checkbox", { name: "Send to Chopin" });
 
-	await expect(box).not.toBeChecked();
+	// With a Planner running, a new comment goes to Chopin unless it is unticked.
+	await expect(box).toBeChecked();
+	await expect(card.locator(".plan-comment-address-chip")).toHaveText("@Chopin");
+	await box.uncheck();
+	await expect(card.locator(".plan-comment-address-chip")).toHaveCount(0);
 	await field.pressSequentially("@chopin tighten this");
 	await expect(box).toBeChecked();
 	await expect(card.locator(".plan-comment-address-chip")).toHaveText("@Chopin");

e2e/planner-attachment.e2e.ts

@@ -9,8 +9,8 @@ import { spawn } from "node:child_process";
 import { SQL } from "bun";
 
 import { connectPlannerLink } from "../packages/planner-link/src/client";
-import { createChannel, registerTestDatabase } from "./database";
-import { authenticate, expect, ready, roomPath, test } from "./room";
+import { createChannel, registerTestDatabase, seedChannel } from "./database";
+import { authenticate, content, expect, ready, roomPath, test } from "./room";
 import { HARNESS, HOST, ROOT } from "./servers";
 
 import type { Page } from "@playwright/test";
@@ -173,6 +173,58 @@ test("Chat says live who holds the document's remote Planner", async ({ page })
 	await expect(status).toHaveAttribute("data-planner-state", "none");
 });
 
+/** Open a new comment on the document's first paragraph. */
+async function draft(page: Page) {
+	await page.waitForFunction(() => document.getAnimations().every(a => a.playState !== "running"));
+	await content(page).locator("p").first().selectText();
+	await page.getByRole("button", { name: "Comment on this passage", exact: true }).click();
+	return page.getByRole("dialog", { name: "New comment" });
+}
+
+test("a new comment is sent to Chopin by default only while a Planner is attached", async ({ page }) => {
+	test.setTimeout(90_000);
+	let database = await freshDatabase();
+	await migrate(database);
+	await remoteServer(database);
+	registerTestDatabase(PORT, database);
+	let room = crypto.randomUUID();
+	await createChannel(PORT, room);
+	await seedChannel(PORT, room, "# Remote comments\nThe harp needs new strings.\n");
+
+	await authenticate(page, HANDLE, ORIGIN);
+	await page.goto(`${ORIGIN}${roomPath(room)}`);
+	await ready(page);
+	await expect(plannerStatus(page)).toHaveAttribute("data-planner-state", "none");
+
+	let card = await draft(page);
+	let box = card.getByRole("checkbox", { name: "Send to Chopin" });
+	await expect(box).not.toBeChecked();
+
+	// Ticked anyway, the server refuses it, says how to attach one, and keeps the words.
+	await box.check();
+	let field = card.getByRole("textbox", { name: "Comment" });
+	await field.fill("Can you shorten this?");
+	await card.getByRole("button", { name: "Post comment", exact: true }).click();
+	await expect(card.getByRole("alert")).toContainText("No Planner is attached to this document.");
+	await expect(field).toHaveValue("Can you shorten this?");
+	await page.keyboard.press("Escape");
+	await expect(card).toHaveCount(0);
+
+	let client = await connectPlannerLink({
+		url: ORIGIN,
+		token: `ghu_e2e_${HANDLE}_1`,
+		document: room,
+		handlers: { turn() {} },
+	});
+	try {
+		await expect(plannerStatus(page)).toHaveAttribute("data-planner-state", "attached");
+		await expect((await draft(page)).getByRole("checkbox", { name: "Send to Chopin" }))
+			.toBeChecked();
+	} finally {
+		await client.detach();
+	}
+});
+
 type Probe = { frames: string[]; closed?: { code: number; at: number } };
 
 /** Open a bare socket from the page, so its cookie and origin are the browser's own. */

packages/editor/src/comment-layer.tsx

@@ -1147,6 +1147,7 @@ export function CommentLayer({ store }: { store: ThreadStore }) {
 			<DraftCard
 				onCancel={cancelDraft}
 				onSend={(text, to) => store.start(text, to)}
+				planner={options.commentPlanner !== false}
 				sheet
 			/>
 		);
@@ -1406,6 +1407,7 @@ export function CommentLayer({ store }: { store: ThreadStore }) {
 							<DraftCard
 								onCancel={cancelDraft}
 								onSend={(text, to) => store.start(text, to)}
+								planner={options.commentPlanner !== false}
 							/>
 						),
 						className: "plan-comment-card",

packages/editor/src/comments.test.tsx

@@ -175,12 +175,28 @@ describe("displayName", () => {
 });
 
 describe("Sending to Chopin", () => {
-	it("offers Send to Chopin, unticked, beside the send", () => {
+	it("offers Send to Chopin, ticked, beside the send of a new comment", () => {
 		let markup = renderToStaticMarkup(<DraftCard onCancel={() => {}} onSend={() => {}} />);
 
 		expect(markup).toContain("Send to Chopin");
+		expect(markup).toMatch(/<input[^>]*\bchecked\b/);
+		expect(markup).toContain('class="plan-comment-address-chip"');
+	});
+
+	it("leaves a new comment's Send to Chopin unticked where no Planner runs", () => {
+		let markup = renderToStaticMarkup(
+			<DraftCard onCancel={() => {}} onSend={() => {}} planner={false} />,
+		);
+
 		expect(markup).toMatch(/<input[^>]*type="checkbox"/);
-		expect(markup).not.toMatch(/<input[^>]*checked/);
+		expect(markup).not.toMatch(/<input[^>]*\bchecked\b/);
+		expect(markup).not.toContain("plan-comment-address-chip");
+	});
+
+	it("leaves a reply's Send to Chopin unticked", () => {
+		let markup = render(view(), true);
+
+		expect(markup).not.toMatch(/<input[^>]*\bchecked\b/);
 	});
 
 	it("keeps the reply's Send to Chopin folded away until the field is used", () => {

packages/editor/src/comments.tsx

@@ -301,13 +301,16 @@ export function mention(
  * text and is never sent: the destination is.
  */
 function Composer({
+	addressed: initial = false,
 	autoFocus,
 	expanded,
 	mode,
 	onCancel,
 	onSend,
 	onTyping,
 }: {
+	/** Start with Send to Chopin ticked, and return to it after each send. */
+	addressed?: boolean;
 	autoFocus?: boolean;
 	/** Keep Send to Chopin showing, as a sheet's pinned composer does. */
 	expanded?: boolean;
@@ -317,7 +320,7 @@ function Composer({
 	onTyping?: (writing: boolean) => void;
 }) {
 	let [text, setText] = useState("");
-	let [to, setTo] = useState(false);
+	let [to, setTo] = useState(initial);
 	let [focused, setFocused] = useState(false);
 	let [coarse, setCoarse] = useState(false);
 	let [indent, setIndent] = useState(0);
@@ -380,7 +383,7 @@ function Composer({
 			return;
 		}
 		setText("");
-		setTo(false);
+		setTo(initial);
 		setRefusal(undefined);
 		onTyping?.(false);
 		let sent = onSend(value, addressed ? "planner" : undefined);
@@ -718,11 +721,13 @@ export function ThreadList(
 export type DraftCardProps = {
 	onSend: (text: string, to?: "planner") => void | Promise<Sent>;
 	onCancel: () => void;
+	/** False when no Planner runs here, so a new comment is not sent to Chopin by default. */
+	planner?: boolean;
 	sheet?: boolean;
 };
 
 /** A new comment: only the composer, with no header to say what the card already shows. */
-export function DraftCard({ onCancel, onSend, sheet }: DraftCardProps) {
+export function DraftCard({ onCancel, onSend, planner = true, sheet }: DraftCardProps) {
 	return (
 		<article
 			aria-label="New comment"
@@ -732,7 +737,13 @@ export function DraftCard({ onCancel, onSend, sheet }: DraftCardProps) {
 			data-plan-comment-draft
 		>
 			<Pinned sheet={sheet}>
-				<Composer autoFocus mode="new" onCancel={onCancel} onSend={onSend} />
+				<Composer
+					addressed={planner}
+					autoFocus
+					mode="new"
+					onCancel={onCancel}
+					onSend={onSend}
+				/>
 			</Pinned>
 		</article>
 	);

packages/editor/src/plan-editor.tsx

@@ -114,6 +114,8 @@ export type PlanEditorProps = {
 	onShowDecision?: (questionnaireId: string) => void;
 	/** False when no Planner will review where decisions live. */
 	planner?: boolean;
+	/** False when no Planner can take a turn now, so a new comment is not sent to Chopin by default. */
+	commentPlanner?: boolean;
 	evidence?: (questionnaireId: string) => ReactNode | null;
 	/** Durable Research Workspace state and actions supplied by the host app. */
 	research?: ResearchStore;
@@ -180,6 +182,7 @@ export function PlanEditor(
 		onShowDecision,
 		evidence,
 		planner,
+		commentPlanner,
 		readOnly,
 		research,
 		researchLauncher,
@@ -487,6 +490,7 @@ export function PlanEditor(
 						onShowDecision,
 						evidence,
 						planner,
+						commentPlanner,
 						research,
 						researchDrafts,
 						researchLauncher,
@@ -525,6 +529,7 @@ export function PlanEditor(
 			onShowDecision,
 			evidence,
 			planner,
+			commentPlanner,
 			research,
 			researchDrafts,
 			researchLauncher,