From bf2e30330e3e67d5332c002c388a6a5701f5e57b Mon Sep 17 00:00:00 2001 From: Val Alexander Date: Sun, 18 Oct 2026 11:26:36 -0500 Subject: [PATCH 1/3] Record each clarification and its answer in a turn's task record taskRecord lists every question the CLI asked during the turn, from its question cards, with the answer as the card shows it: a secret one reads "(hidden)". An unanswered question reads "not answered" with why it closed (the agent withdrew it, the turn was stopped, or it ended first), and an answer Modex could not confirm the CLI received says so. Both count as not settled. Nothing is filled in that the card does not hold, and a turn with no questions has no Clarifications section. The unit test failed first: the record had no clarifications. Refs #280 Coven-Harness: Claude Code / claude-opus-5-5 --- apps/desktop/src/shared/task-context.ts | 32 ++++++++++++++++++++++++- apps/desktop/test/task-context.test.ts | 31 ++++++++++++++++++++++++ 2 files changed, 62 insertions(+), 1 deletion(-) diff --git a/apps/desktop/src/shared/task-context.ts b/apps/desktop/src/shared/task-context.ts index e41d6dd9..fb9f04b5 100644 --- a/apps/desktop/src/shared/task-context.ts +++ b/apps/desktop/src/shared/task-context.ts @@ -20,9 +20,22 @@ export interface TaskRecord { /** Changed files outside the project: counted, never named. */ outsideFiles: number; commands: TaskCommand[]; + /** Each question the CLI asked during the turn, one entry per question on its cards. */ + clarifications: TaskClarification[]; answer?: string; } +/** A question the CLI asked, with the answer as its card shows it. Nothing is filled in that the card does not hold. */ +export interface TaskClarification { + question: string; + /** The answers as shown, saved and exported (a secret one is "(hidden)"), or null when nobody answered. */ + answer: string[] | null; + /** Why an unanswered question closed (docs/human-requests.md). */ + closed?: "native" | "stopped" | "ended"; + /** Answered, but Modex stopped before it could confirm the CLI received the answer. */ + uncertain?: true; +} + const PATCH_TOOLS = new Set(["apply_patch", "Edit", "MultiEdit", "Write", "NotebookEdit"]); const SETTLED_WELL = new Set(["succeeded"]); @@ -34,7 +47,14 @@ export function taskRecord(items: readonly ThreadItem[], opts: { threadId: strin const files = new Set(); let outsideFiles = 0; const commands: TaskCommand[] = []; + const clarifications: TaskClarification[] = []; for (const item of rest) { + if (item.kind === "question") { + for (const question of item.questions) { + const answer = item.answers ? item.answers[question.id] ?? null : null; + clarifications.push({ question: question.text, answer, ...(!answer && item.withdrawn ? { closed: item.withdrawn } : {}), ...(answer && item.delivery === "uncertain" ? { uncertain: true as const } : {}) }); + } + } if (item.kind !== "tool") continue; if (PATCH_TOOLS.has(item.name)) { for (const file of patchFiles(item)) { @@ -50,7 +70,7 @@ export function taskRecord(items: readonly ThreadItem[], opts: { threadId: strin return { title: opts.title, threadId: opts.threadId, request: user?.kind === "user" ? user.text : "", ...(plan?.kind === "plan" ? { plan: { steps: plan.steps, from: plan.from, revision: plan.revision } } : {}), - files: [...files], outsideFiles, commands, + files: [...files], outsideFiles, commands, clarifications, ...(answer?.kind === "assistant" && answer.text.trim() ? { answer: answer.text.trim() } : {}), }; } @@ -62,6 +82,8 @@ const FROM_WORDS: Record = { "codex-plan": "Codex's plan", " export function taskMarkdown(record: TaskRecord): string { const lines: string[] = [`# ${record.title}`, ""]; if (record.request.trim()) lines.push(...record.request.trim().split("\n").map((line) => `> ${line}`), ""); + const clarifications = record.clarifications ?? []; + if (clarifications.length) lines.push("## Clarifications", "", ...clarifications.map((c) => `- ${clarificationLine(c)}`), ""); if (record.plan) { lines.push("## Plan", ""); for (const step of record.plan.steps) lines.push(`- [${step.status === "completed" ? "x" : " "}] ${step.text} (${STEP_WORDS[step.status]})`); @@ -82,12 +104,20 @@ export function taskMarkdown(record: TaskRecord): string { const loose = [ ...record.commands.filter((command) => !SETTLED_WELL.has(command.outcome)).map(commandLine), ...(record.plan?.steps.filter((step) => step.status !== "completed").map((step) => `${step.text} (${STEP_WORDS[step.status]})`) ?? []), + ...clarifications.filter((c) => !c.answer || c.uncertain).map(clarificationLine), ]; if (loose.length) lines.push("## Not settled", "", ...loose.map((line) => `- ${line}`), ""); lines.push("---", `From modex thread ${record.threadId}.`); return lines.join("\n") + "\n"; } +const CLOSED_WORDS: Record, string> = { native: "the agent withdrew it", stopped: "the turn was stopped", ended: "the turn ended first" }; + +function clarificationLine(c: TaskClarification): string { + if (!c.answer) return `${c.question} — not answered${c.closed ? `: ${CLOSED_WORDS[c.closed]}` : ""}`; + return `${c.question} — ${c.answer.join(", ")}${c.uncertain ? " (Modex could not confirm the agent received this answer)" : ""}`; +} + function commandLine(command: TaskCommand): string { const words: Record = { running: "running", succeeded: "succeeded", failed: "failed", denied: "denied", interrupted: "stopped", unknown: "status unknown" }; const exit = typeof command.exitCode === "number" ? `exit code ${command.exitCode}` : command.exitCode === null ? "no exit code" : "exit code not reported"; diff --git a/apps/desktop/test/task-context.test.ts b/apps/desktop/test/task-context.test.ts index 204b8c06..cb8b53c0 100644 --- a/apps/desktop/test/task-context.test.ts +++ b/apps/desktop/test/task-context.test.ts @@ -118,6 +118,37 @@ test("a task record summarises request, plan, files, checks and loose ends, with for (const absent of ["PRIVATE REASONING", "LOTS OF LOG", "/Users/val", "verified", "Verified"]) assert.ok(!md.includes(absent), `must not include: ${absent}`); }); +test("a task record lists each clarification with its answer, a secret hidden, and says when one was not answered or not confirmed; it invents none", () => { + const at = "2026-10-18T09:00:00.000Z"; + const q = (id: string, header: string, text: string, secret = false) => ({ id, header, text, options: null, allowOther: true, secret }); + const items: ThreadItem[] = [ + { id: "u1", kind: "user", text: "Deploy the fix", at }, + { id: "ask-1", kind: "question", blocking: true, questions: [q("env", "Environment", "Which environment?"), q("token", "Token", "Paste the deploy token", true)], + answers: { env: ["staging"], token: ["(hidden)"] }, answeredBy: "desktop", delivery: "sent", at }, + { id: "ask-2", kind: "question", blocking: true, questions: [q("branch", "Branch", "Which branch?")], withdrawn: "stopped", at }, + { id: "ask-3", kind: "question", blocking: true, questions: [q("notes", "Notes", "Add release notes?")], answers: { notes: ["yes"] }, answeredBy: "phone", delivery: "uncertain", at }, + { id: "ask-4", kind: "question", blocking: false, questions: [q("region", "Region", "Which region?")], withdrawn: "native", at }, + { id: "f", kind: "assistant", text: "Deployed to staging.", phase: "final", outcome: "succeeded", at }, + ] as ThreadItem[]; + const record = taskRecord(items, { threadId: "T", title: "Deploy", cwd: "/repo" }); + assert.deepEqual(record.clarifications, [ + { question: "Which environment?", answer: ["staging"] }, + { question: "Paste the deploy token", answer: ["(hidden)"] }, + { question: "Which branch?", answer: null, closed: "stopped" }, + { question: "Add release notes?", answer: ["yes"], uncertain: true }, + { question: "Which region?", answer: null, closed: "native" }, + ]); + const md = taskMarkdown(record); + for (const expected of ["## Clarifications", "- Which environment? — staging", "- Paste the deploy token — (hidden)", "- Which branch? — not answered: the turn was stopped", + "- Add release notes? — yes (Modex could not confirm the agent received this answer)", "- Which region? — not answered: the agent withdrew it"]) { + assert.ok(md.includes(expected), `missing: ${expected}\n${md}`); + } + assert.match(md, /## Not settled\n[\s\S]*Which branch\? — not answered[\s\S]*Add release notes\?/); + assert.ok(!/Which environment\?[^\n]*\n[\s\S]*## Not settled[\s\S]*Which environment/.test(md), "an answered, sent clarification is settled"); + // A turn with no questions has no section, and old records read as before. + assert.equal(taskMarkdown(taskRecord(items.filter((i) => i.kind !== "question"), { threadId: "T", title: "Deploy", cwd: "/repo" })).includes("Clarifications"), false); +}); + test("Save task to Notes writes the turn's record into its project's notes; saving again updates it, and a note edited since is kept", async () => { const { Store } = await import("../src/main/engine/store.js"); const { ThreadRunner } = await import("../src/main/engine/runner.js"); From f059875612efe93b423c74dd7394ab61b251abe3 Mon Sep 17 00:00:00 2001 From: Val Alexander Date: Sun, 18 Oct 2026 11:26:36 -0500 Subject: [PATCH 2/3] Prove a plan clarification end to end, and document it e2e/plan-clarification.spec.ts runs a scripted codex-cli 0.163.0 app-server that reports a plan, asks a clarifying question through the question card, and plans on from the answer. The answer reaches Codex once, Save to Notes records it with the secret hidden, and a second clarification stopped unanswered is saved as "not answered: the turn was stopped" with no answer filled in. No file under MODEX_HOME holds the secret. Against the record code without clarifications it failed at "## Clarifications" in the saved note. docs/task-context.md describes clarifications in the record. Refs #280 Coven-Harness: Claude Code / claude-opus-5-5 --- apps/desktop/e2e/plan-clarification.spec.ts | 139 ++++++++++++++++++++ docs/task-context.md | 26 +++- 2 files changed, 160 insertions(+), 5 deletions(-) create mode 100644 apps/desktop/e2e/plan-clarification.spec.ts diff --git a/apps/desktop/e2e/plan-clarification.spec.ts b/apps/desktop/e2e/plan-clarification.spec.ts new file mode 100644 index 00000000..4763adf2 --- /dev/null +++ b/apps/desktop/e2e/plan-clarification.spec.ts @@ -0,0 +1,139 @@ +import { test, expect } from "@playwright/test"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { currentRow, items, launch, seedHome, tid } from "./support"; + +/** + * A plan clarification (docs/task-context.md, CF-7 #280): the CLI reports a plan, asks a clarifying question + * through the question card, and carries on once it is answered. The answer reaches the CLI once, and Save + * to Notes records it with the secret hidden. A clarification nobody answered says so and gains no answer. + * The executable below is a scripted codex-cli 0.163.0 app-server (`turn/plan/updated`, then + * `item/tool/requestUserInput`), not a provider. + */ +function fixture(dir: string): string { + const cli = path.join(dir, "codex-clarify.cjs"); + fs.writeFileSync(cli, `#!${process.execPath} +const fs = require('node:fs'); +if (process.argv.includes('--version')) { console.log('codex-cli 0.163.0'); process.exit(0); } +if (process.argv.includes('login') || process.argv.includes('auth')) { console.log('Logged in using ChatGPT'); process.exit(0); } +const log = ${JSON.stringify(path.join(dir, "codex-replies.jsonl"))}; +const emit = (o) => process.stdout.write(JSON.stringify(o) + '\\n'); +const questions = [ + { id: 'q-env', header: 'Environment', question: 'Which environment should the plan target?', isOther: false, isSecret: false, options: [{ label: 'staging', description: 'Safe to try' }, { label: 'production', description: 'Live traffic' }] }, + { id: 'q-token', header: 'Token', question: 'Paste the deploy token', isOther: true, isSecret: true, options: null }, +]; +let turn = 0, open = null; +const cwds = new Map(); +const plan = (thread, id, a, b) => emit({ method: 'turn/plan/updated', params: { threadId: thread, turnId: id, explanation: null, plan: [{ step: 'Pick the environment', status: a }, { step: 'Deploy', status: b }] } }); +require('node:readline').createInterface({ input: process.stdin }).on('line', (line) => { + const msg = JSON.parse(line); + const p = msg.params || {}; + if (msg.method === 'initialize') return emit({ id: msg.id, result: { userAgent: 'codex-clarify/0.163.0' } }); + if (msg.method === 'initialized') return; + if (msg.method === 'account/read') return emit({ id: msg.id, result: { account: { type: 'chatgpt' }, requiresOpenaiAuth: true } }); + if (msg.method === 'model/list') return emit({ id: msg.id, result: { data: [{ id: 'fixture', model: 'fixture', displayName: 'Fixture', isDefault: true }], nextCursor: null } }); + if (msg.method === 'thread/start' || msg.method === 'thread/resume') { + const id = p.threadId || 'thr-' + cwds.size; + cwds.set(id, p.cwd || ''); + return emit({ id: msg.id, result: { thread: { id } } }); + } + if (msg.method === 'turn/start' && /modex-title-/.test(cwds.get(p.threadId) || '')) { + const id = 'title-' + (++turn); + emit({ id: msg.id, result: { turn: { id } } }); + emit({ method: 'item/completed', params: { threadId: p.threadId, turnId: id, item: { id: 'name-' + turn, type: 'agentMessage', text: 'Deploy plan', phase: 'final_answer' } } }); + return emit({ method: 'turn/completed', params: { threadId: p.threadId, turn: { id, status: 'completed', error: null } } }); + } + if (msg.method === 'turn/start') { + const id = 'turn-' + (++turn); + open = { thread: p.threadId, turn: id, request: 900 + turn, done: false }; + emit({ id: msg.id, result: { turn: { id } } }); + emit({ method: 'turn/started', params: { threadId: p.threadId, turn: { id } } }); + plan(p.threadId, id, 'inProgress', 'pending'); + emit({ id: open.request, method: 'item/tool/requestUserInput', params: { threadId: p.threadId, turnId: id, itemId: 'ask-' + turn, isBlocking: true, autoResolutionMs: null, questions } }); + return; + } + const finish = (status, text) => { + if (!open || open.done) return; + open.done = true; + if (text) emit({ method: 'item/completed', params: { threadId: open.thread, turnId: open.turn, item: { id: 'say-' + turn, type: 'agentMessage', text, phase: 'final_answer' } } }); + emit({ method: 'turn/completed', params: { threadId: open.thread, turn: { id: open.turn, status, error: null } } }); + }; + if (msg.method === 'turn/interrupt') { emit({ id: msg.id, result: {} }); return finish('interrupted'); } + if (open && msg.id === open.request && !msg.method) { + fs.appendFileSync(log, JSON.stringify({ request: msg.id, result: msg.result, error: msg.error }) + '\\n'); + if (msg.result) { + plan(open.thread, open.turn, 'completed', 'completed'); + finish('completed', 'Planned the deploy to ' + msg.result.answers['q-env'].answers[0] + '.'); + } + return; + } + if (msg.id !== undefined && msg.method) emit({ id: msg.id, error: { code: -32601, message: 'fixture does not implement ' + msg.method } }); +}); +`, { mode: 0o755 }); + return cli; +} + +type Reply = { request: number; result?: { answers: Record }; error?: { code: number } }; +const replies = (dir: string): Reply[] => { + const file = path.join(dir, "codex-replies.jsonl"); + return fs.existsSync(file) ? fs.readFileSync(file, "utf8").trim().split("\n").filter(Boolean).map((line) => JSON.parse(line) as Reply) : []; +}; +const everything = (dir: string): string => fs.readdirSync(dir, { recursive: true, withFileTypes: true }) + .filter((entry) => entry.isFile()).map((entry) => fs.readFileSync(path.join(entry.parentPath, entry.name), "utf8")).join("\n"); + +test("a plan clarification is answered on its card, reaches the CLI once, and is saved to Notes with the secret hidden; an unanswered one says so", async () => { + const { home, repo } = seedHome({ default_backend: "codex", default_mode: "agent" }); + const logs = fs.mkdtempSync(path.join(os.tmpdir(), "modex-e2e-clarify-")); + const statePath = path.join(home, "app/state.json"); + const state = JSON.parse(fs.readFileSync(statePath, "utf8")); + state.settings.codex_bin = fixture(logs); + fs.writeFileSync(statePath, JSON.stringify(state)); + const { app, page } = await launch(home); + try { + await page.keyboard.press("Meta+n"); + await tid(page, "composer-input").fill("Plan the deploy"); + await page.keyboard.press("Meta+Enter"); + // The plan as reported, then the clarification on the existing question card. + await expect(tid(items(page, "plan"), "plan-step")).toHaveText(["In progress Pick the environment", "Not started Deploy"]); + const card = items(page, "question").first(); + await expect(currentRow(page)).toHaveAttribute("data-status", "waiting"); + await card.getByRole("radio", { name: "staging" }).check(); + await card.getByLabel("Token").fill("s3cr3t-deploy-token"); + await tid(card, "question-submit").click(); + await expect(items(page, "assistant").last()).toContainText("Planned the deploy to staging."); + await expect(currentRow(page)).toHaveAttribute("data-status", "idle"); + await expect(tid(items(page, "plan"), "plan-step")).toHaveText(["Reported done Pick the environment", "Reported done Deploy"]); + expect(replies(logs).map((reply) => reply.result)).toEqual([{ answers: { "q-env": { answers: ["staging"] }, "q-token": { answers: ["s3cr3t-deploy-token"] } } }]); + + // Save to Notes: the clarification is in the record, the secret is not. + await tid(page, "save-task-note").last().click(); + await expect(tid(page, "task-note-saved").last()).toContainText("Saved to Notes · Task:"); + const notes = async () => (await page.evaluate(() => window.modex!.invoke("space:list", undefined))) as { title: string; markdown: string }[]; + const first = (await notes()).find((note) => note.title.startsWith("Task:"))!; + expect(first.markdown).toContain("## Clarifications"); + expect(first.markdown).toContain("- Which environment should the plan target? — staging"); + expect(first.markdown).toContain("- Paste the deploy token — (hidden)"); + expect(first.markdown).not.toContain("s3cr3t-deploy-token"); + + // A second turn asks again, and Stop closes the question unanswered: the CLI gets an error, the record no answer. + await tid(page, "composer-input").fill("Plan it again"); + await page.keyboard.press("Meta+Enter"); + await expect(tid(items(page, "question").nth(1), "question-submit")).toBeVisible(); + await tid(page, "stop").click(); + await expect(currentRow(page)).toHaveAttribute("data-status", "idle"); + await expect.poll(() => replies(logs).length).toBe(2); + expect(replies(logs)[1]).toMatchObject({ error: { code: -32000 } }); + await tid(page, "save-task-note").last().click(); + await expect(tid(page, "task-note-saved").last()).toContainText("Saved to Notes · Task:"); + const second = (await notes()).filter((note) => note.title.startsWith("Task:")).find((note) => note.markdown.includes("Plan it again"))!; + expect(second.markdown).toContain("- Which environment should the plan target? — not answered: the turn was stopped"); + expect(second.markdown).toMatch(/## Not settled\n[\s\S]*Paste the deploy token — not answered: the turn was stopped/); + expect(second.markdown).not.toContain("— staging"); + expect(replies(logs)).toHaveLength(2); + expect(everything(home).includes("s3cr3t-deploy-token")).toBe(false); + } finally { + await app.close(); + for (const dir of [home, repo, logs]) fs.rmSync(dir, { recursive: true, force: true }); + } +}); diff --git a/docs/task-context.md b/docs/task-context.md index 503f4ea9..e5660251 100644 --- a/docs/task-context.md +++ b/docs/task-context.md @@ -28,14 +28,32 @@ outcome is the turn's. ## The task record -`taskRecord` (`shared/task-context.ts`) summarises one turn: the request, the latest plan, the files -its patch tools changed, the commands it ran with outcome and exit code, the final answer, and what -is not settled. Not settled means commands that did not succeed and plan steps not reported done. +`taskRecord` (`shared/task-context.ts`) summarises one turn: the request, the questions the CLI asked +and their answers, the latest plan, the files its patch tools changed, the commands it ran with +outcome and exit code, the final answer, and what is not settled. Not settled means commands that did +not succeed, plan steps not reported done, and clarifications nobody answered or whose answer Modex +could not confirm the CLI received. `taskMarkdown` renders the record as portable Markdown that ends with a backlink to the thread. It leaves out reasoning, command output, and any path outside the project. Files inside the project are listed relative to the thread's working directory; files outside it are counted, not named. +## Clarifications + +A CLI that needs to ask before it plans or acts asks through the [question card](human-requests.md): +Codex's `item/tool/requestUserInput`, with a control for each question. The answer reaches the CLI +once, and the plan carries on from it. Claude Code 2.1.302 sends Modex no structured questions. + +The record lists every question the turn asked, in order, with the answer as its card shows it: + +- an answered question gives its answer, and a secret one reads "(hidden)"; +- an unanswered one reads "not answered", with why it closed: the agent withdrew it, the turn was + stopped, or the turn ended first; +- an answer Modex could not confirm the CLI received says so (see + [across replay and reconnect](human-requests.md#across-replay-and-reconnect)). + +Nothing is filled in that the card does not hold. + ## Save to Notes Under each finished turn, **Save to Notes** (`task:save-note`, `ThreadRunner.saveTaskNote`) writes the @@ -51,8 +69,6 @@ saves on its own, and an agent cannot use this. ## Not done yet -- Clarification through question cards waits for the human-request lifecycle - ([#276](https://github.com/TypeSafeAI/modex/issues/276)). - Verified steps, and marking earlier verification stale. Verification needs a check's observed result tied to the code revision the step changed. - An on-demand context map of relevant files and tests beyond those the turn changed. From 5f0959d8875cb7834bbccef7392b944b8c0bffd7 Mon Sep 17 00:00:00 2001 From: Val Alexander Date: Sun, 18 Oct 2026 12:11:18 -0500 Subject: [PATCH 3/3] Record CF-7's clarifications in status Refs #280 Coven-Harness: Claude Code / claude-opus-5-5 --- docs/status.md | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/docs/status.md b/docs/status.md index 22a84dbb..b98a1ed7 100644 --- a/docs/status.md +++ b/docs/status.md @@ -114,8 +114,11 @@ and [landing verification](reviews/2026-10-11-landing-clarity.md). - **Fixes those checks found.** Opening a thread for the first time no longer resends and rewrites every item in it, which cost a 1,400-item transcript about 55 ms. An approval card in a narrow window wraps its text and buttons instead of pushing Deny out of view. - - **Still open:** CF-4's remaining Markdown checks (#277), CF-7's clarifications, live-provider - acceptance and human VoiceOver checks. + - **Clarifications in the task record (CF-7 #280, second part).** A question Codex asks while it + plans is answered on its card, and the turn's record and Save to Notes list it with its answer, + a secret hidden. One nobody answered reads "not answered" with why, and is never filled in. + - **Still open:** CF-4's remaining Markdown checks (#277), live-provider acceptance and human + VoiceOver checks. - **Every inline error can be copied, and most can be retried.** A shared `ErrorNotice` replaces bare error text across the terminal, Changes diff, Files, Canvas, browser and