diff --git a/apps/desktop/e2e/human-requests.spec.ts b/apps/desktop/e2e/human-requests.spec.ts index e5abc262..20b462b6 100644 --- a/apps/desktop/e2e/human-requests.spec.ts +++ b/apps/desktop/e2e/human-requests.spec.ts @@ -111,7 +111,7 @@ test("Codex questions: the card answers by its schema, the CLI hears once, the s expect(replies(logs)).toEqual([]); const token = card.getByLabel("Token"); await expect(token).toHaveAttribute("type", "password"); - await token.fill("s3cr3t-token"); + await token.fill(" s3cr3t-token "); await page.screenshot({ path: test.info().outputPath("codex-question-card.png") }); await tid(card, "question-submit").click(); @@ -119,7 +119,7 @@ test("Codex questions: the card answers by its schema, the CLI hears once, the s await expect(tid(card, "question-answer")).toHaveText(["staging", "(hidden)"]); await expect(items(page, "assistant").last()).toContainText("Deploying to staging."); await expect(currentRow(page)).toHaveAttribute("data-status", "idle"); - expect(replies(logs)).toEqual([{ request: 901, result: { answers: { "q-env": { answers: ["staging"] }, "q-token": { answers: ["s3cr3t-token"] } } } }]); + expect(replies(logs)).toEqual([{ request: 901, result: { answers: { "q-env": { answers: ["staging"] }, "q-token": { answers: [" s3cr3t-token "] } } } }]); // A second answer (an old window, or a double click) is refused and reaches nothing. const threadId = (await currentRow(page).getAttribute("data-thread-id"))!; const again = await page.evaluate(async (id) => { @@ -217,3 +217,30 @@ test("with the journal on, a restart shows answered and stopped cards inert, say fs.rmSync(logs, { recursive: true, force: true }); } }); + +test("uncertain question delivery stays inert after restart and never reads as answered", async () => { + const { home, repo } = seedHome(); + const statePath = path.join(home, "app/state.json"); + const state = JSON.parse(fs.readFileSync(statePath, "utf8")); + const at = new Date().toISOString(); + state.threads = [{ id: "uncertain", projectId: "p1", title: "Uncertain answer", createdAt: at, updatedAt: at, cwd: repo, backend: "mock", model: "mock", mode: "chat", plan: false, status: "idle" }]; + fs.writeFileSync(statePath, JSON.stringify(state)); + fs.writeFileSync(path.join(home, "app/threads/uncertain.json"), JSON.stringify([ + { id: "u", kind: "user", text: "Deploy", at }, + { id: "a", kind: "approval", question: "Deploy?", answer: "yes", delivery: "sending", at }, + { id: "q", kind: "question", questions: [{ id: "env", header: "Environment", text: "Which?", options: null, allowOther: true, secret: false }], blocking: true, answers: { env: ["staging"] }, delivery: "sending", at }, + ])); + const { app, page } = await launch(home); + try { + await page.locator('[data-testid="thread-row"][data-thread-id="uncertain"]').click(); + const card = items(page, "question").first(); + await expect(card).toContainText("Delivery uncertain"); + await expect(card).toHaveAttribute("data-answered", "false"); + const approval = items(page, "approval").first(); + await expect(approval).toContainText("Delivery uncertain"); + await expect(approval).toHaveAttribute("data-answered", "false"); + await expect(tid(card, "question-submit")).toHaveCount(0); + const stored = JSON.parse(fs.readFileSync(path.join(home, "app/threads/uncertain.json"), "utf8")); + expect(stored.find((item: { id: string }) => item.id === "q").delivery).toBe("uncertain"); + } finally { await app.close(); } +}); diff --git a/apps/desktop/src/main/engine/backends/claude.ts b/apps/desktop/src/main/engine/backends/claude.ts index c10b571f..6c24a79a 100644 --- a/apps/desktop/src/main/engine/backends/claude.ts +++ b/apps/desktop/src/main/engine/backends/claude.ts @@ -1,3 +1,4 @@ +import { actionDigest } from "../../../shared/human-requests.js"; import { AgentTracker } from "./agents.js"; import type { CliExecutable } from "../cli-path.js"; import { generateTitle } from "../titles.js"; @@ -8,7 +9,7 @@ import { StringDecoder } from "node:string_decoder"; import { PAGES_TOOLS } from "../../../shared/pages.js"; import { KNOWLEDGE_TOOLS } from "../../../shared/knowledge.js"; import type { ApprovalRequest, Backend, ModelInfo, StopReport, TitleOptions, TurnAttachment, TurnOptions, TurnResult, TurnSink } from "./types.js"; -import { LineBuffer, shortJson, stderrTail } from "./types.js"; +import { LineBuffer, shortJson, stderrTail, writeJsonResponse } from "./types.js"; import { claudeCall, UsageMeter, withUsage } from "./usage.js"; import { cliEnvironment, cliSpawn, health, installation, probe } from "./health.js"; import type { ApprovalAnswer, BackendHealth } from "../../../shared/types.js"; @@ -165,6 +166,7 @@ export class ClaudeBackend implements Backend { const drainMs = this.timing.drainMs ?? DRAIN_MS; // `can_use_tool` requests waiting on an answer, by request id. The CLI's `control_cancel_request` aborts one. const requests = new Map(); + const seenRequests = new Map(); // The process's own end: `exit` may precede `close` by however long a descendant holds its pipes. let exit: { code: number | null; signal: NodeJS.Signals | null } | undefined; // A failed `result` ends the turn once the process has exited (or `drainMs` passed), so the stderr it @@ -290,6 +292,8 @@ export class ClaudeBackend implements Backend { /* closed */ } }; + const writeResponse = (o: unknown) => finished + ? Promise.reject(new Error("request transport unavailable")) : writeJsonResponse(child.stdin, o); // Turn settlement must not cancel escalation while the child is still alive. const terminate = () => { if (exit || child.exitCode !== null || child.signalCode != null) return; @@ -351,7 +355,7 @@ export class ClaudeBackend implements Backend { let part = 0; const observe = (o: Observation) => sink.observe({ ...o, ...(msg.uuid ? { source: `${msg.uuid}#${part++}` } : {}) }); const native = (item: string, block?: number): NativeRef => ({ runtime, ...(session.id ? { thread: session.id } : {}), item, ...(block !== undefined ? { part: String(block) } : {}) }); - const handled = this.handle(msg, sink, { cwd: opts.cwd, started, write, requests, stopping: () => stopAt !== undefined, init, agents, tasks, background, listed, blocks, denied, observe, native }); + const handled = this.handle(msg, sink, { cwd: opts.cwd, started, write, writeResponse, requests, seenRequests, stopping: () => stopAt !== undefined, init, agents, tasks, background, listed, blocks, denied, observe, native }); if (msg.type === "control_request") { approvals++; void handled.finally(() => { @@ -384,7 +388,7 @@ export class ClaudeBackend implements Backend { } /** Everything but `result`, which `runTurn` handles itself because it decides when the turn ends. */ - private async handle(msg: ClaudeMessage, sink: TurnSink, ctx: { agents: AgentTracker; tasks: Map; background: Set; listed: Set; cwd?: string; started: Map; write: (o: unknown) => void; requests: Map; stopping: () => boolean; init: Record; blocks: Blocks; denied: Set; observe: (o: Observation) => void; native: (item: string, block?: number) => NativeRef }): Promise { + private async handle(msg: ClaudeMessage, sink: TurnSink, ctx: { agents: AgentTracker; tasks: Map; background: Set; listed: Set; cwd?: string; started: Map; write: (o: unknown) => void; writeResponse: (o: unknown) => Promise; requests: Map; seenRequests: Map; stopping: () => boolean; init: Record; blocks: Blocks; denied: Set; observe: (o: Observation) => void; native: (item: string, block?: number) => NativeRef }): Promise { // Subagent text/thinking belongs to that agent, never to the parent's streaming buffer. if (msg.parent_tool_use_id && ["assistant", "user", "stream_event"].includes(msg.type)) { const tool = msg.message?.content?.find((b) => b.type === "tool_use"); @@ -543,6 +547,17 @@ export class ClaudeBackend implements Backend { ctx.write({ type: "control_response", response: { subtype: "error", request_id: msg.request_id, error: `unsupported control request ${req?.subtype ?? "?"}` } }); return; } + if (msg.request_id) { + const digest = actionDigest(req); + if (ctx.seenRequests.has(msg.request_id)) { + if (ctx.seenRequests.get(msg.request_id) !== digest) { + ctx.requests.get(msg.request_id)?.abort(); + sink.notice("warn", "Claude reused a request id for a changed action; the original request was withdrawn. A new request id is required."); + } + return; + } + ctx.seenRequests.set(msg.request_id, digest); + } const tool = req.tool_name ?? "tool"; const title = toolTitle(tool, req.input ?? {}); const request: ApprovalRequest = { @@ -563,22 +578,25 @@ export class ClaudeBackend implements Backend { sink.approval(request, withdrawn.signal), new Promise((resolve) => withdrawn.signal.addEventListener("abort", () => resolve(undefined), { once: true })), ]); + if (answer === undefined || withdrawn.signal.aborted) return; + const allow = answer !== "no"; + if (!allow && req.tool_use_id) ctx.denied.add(req.tool_use_id); + const response = { + type: "control_response", + response: { + subtype: "success", + request_id: msg.request_id, + response: allow + ? { behavior: "allow", updatedInput: req.input ?? {}, ...(answer === "always" && req.permission_suggestions?.length ? { updatedPermissions: req.permission_suggestions } : {}) } + : { behavior: "deny", message: denyMessage(sink.refusedByRule?.(request)) }, + }, + }; + const write = () => ctx.writeResponse(response); + if (request.native && sink.deliverRequest) await sink.deliverRequest(request.native, write); + else ctx.write(response); } finally { if (id && ctx.requests.get(id) === withdrawn) ctx.requests.delete(id); } - if (answer === undefined || withdrawn.signal.aborted) return; - const allow = answer !== "no"; - if (!allow && req.tool_use_id) ctx.denied.add(req.tool_use_id); - ctx.write({ - type: "control_response", - response: { - subtype: "success", - request_id: msg.request_id, - response: allow - ? { behavior: "allow", updatedInput: req.input ?? {}, ...(answer === "always" && req.permission_suggestions?.length ? { updatedPermissions: req.permission_suggestions } : {}) } - : { behavior: "deny", message: denyMessage(sink.refusedByRule?.(request)) }, - }, - }); return; } default: diff --git a/apps/desktop/src/main/engine/backends/codex.ts b/apps/desktop/src/main/engine/backends/codex.ts index 9d7783ae..55c67d77 100644 --- a/apps/desktop/src/main/engine/backends/codex.ts +++ b/apps/desktop/src/main/engine/backends/codex.ts @@ -7,13 +7,13 @@ import { generateTitle } from "../titles.js"; import type { ChildProcess } from "node:child_process"; import { randomUUID } from "node:crypto"; import type { Backend, ModelInfo, NativeRequest, TitleOptions, TurnOptions, TurnResult, TurnSink } from "./types.js"; -import { LineBuffer, stderrTail } from "./types.js"; +import { LineBuffer, stderrTail, writeJsonResponse } from "./types.js"; import { codexCall, UsageMeter, withUsage } from "./usage.js"; import { cliEnvironment, cliSpawn, health, installation } from "./health.js"; import type { AgentActivity, BackendHealth } from "../../../shared/types.js"; import type { MessagePhase, NativeRef, PlanStepStatus, SettledOutcome } from "../../../shared/conversation-events.js"; import { boundOutput, byteLength, memoryEvidence } from "../../../shared/evidence.js"; -import { codexAnswerResponse, codexQuestions } from "../../../shared/human-requests.js"; +import { actionDigest, codexAnswerResponse, codexQuestions } from "../../../shared/human-requests.js"; /** * Codex's own wording when the sign-in it loaded at startup no longer matches ~/.codex/auth.json @@ -49,7 +49,7 @@ export class CodexBackend implements Backend { * Server requests waiting on a person, keyed `generation:id`. Codex's `serverRequest/resolved` for one still * here means Codex settled it itself (codex-cli 0.163.0): the card is withdrawn and no answer is sent. */ - private readonly openRequests = new Map(); + private readonly openRequests = new Map(); private get bin(): string { return typeof this.executable === "function" ? this.executable() : this.executable; } @@ -98,6 +98,8 @@ export class CodexBackend implements Backend { this.child = null; this.ready = null; this.loaded.clear(); + for (const request of this.openRequests.values()) request.controller.abort(); + this.openRequests.clear(); for (const p of this.pending.values()) p.reject(error); this.pending.clear(); for (const finish of this.disconnects) finish(error); @@ -288,6 +290,7 @@ export class CodexBackend implements Backend { const context = [opts.knowledge?.instructions, opts.pages?.instructions, opts.images?.instructions, text].filter(value => value !== undefined).join("\n\n"); const input = opts.plan ? `${PLAN_PREFIX}\n\n${context}` : context; const runtime = this.generation; + const seenRequests = new Map(); const agents = new AgentTracker(sink, runtime); const activities = new Set(); // Output is kept bounded as it streams (its last 256 KiB), with the total seen. @@ -351,7 +354,8 @@ export class CodexBackend implements Backend { return; } if (msg.method === "serverRequest/resolved") { - this.openRequests.get(`${runtime}:${String(p.requestId)}`)?.abort(); + const request = this.openRequests.get(`${runtime}:${JSON.stringify(p.requestId)}`); + if (request && (!p.threadId || p.threadId === request.thread)) request.controller.abort(); return; } if (p.threadId && p.threadId !== tid) { @@ -365,7 +369,7 @@ export class CodexBackend implements Backend { const state = status.type === "active" ? (status.activeFlags?.length ? "waiting" : "running") : "failed"; agents.update(id, { state, detail: state === "waiting" ? "Waiting for approval or input" : undefined }); } else if (msg.id !== undefined && msg.method) { - void this.handleServerRequest(msg, sink); + void this.handleServerRequest(msg, sink, seenRequests); } else if (msg.method === "turn/completed") { const turn = p.turn as { status?: string; error?: { message?: string } } | undefined; agents.update(id, { state: turn?.status === "completed" ? "completed" : turn?.status === "interrupted" ? "stopped" : "failed", detail: turn?.error?.message }, turn?.error?.message); @@ -384,7 +388,7 @@ export class CodexBackend implements Backend { if (messageTurn && messageTurn !== turnId) return; // Server → client requests (approvals). if (msg.id !== undefined && msg.method) { - void this.handleServerRequest(msg, sink); + void this.handleServerRequest(msg, sink, seenRequests); return; } if (msg.method === "item/started" || msg.method === "item/completed") { @@ -548,20 +552,38 @@ export class CodexBackend implements Backend { return paths; } - private async handleServerRequest(msg: RpcMessage, sink: TurnSink): Promise { - const key = `${this.generation}:${String(msg.id)}`; + private async handleServerRequest(msg: RpcMessage, sink: TurnSink, seen: Map): Promise { + const key = `${this.generation}:${JSON.stringify(msg.id)}`; + const digest = actionDigest({ method: msg.method, params: msg.params }); + if (seen.has(key)) { + if (seen.get(key) !== digest) { + this.openRequests.get(key)?.controller.abort(); + sink.notice("warn", "Codex reused a request id for a changed action; the original request was withdrawn. A new request id is required."); + } + return; + } + seen.set(key, digest); const withdrawn = new AbortController(); - this.openRequests.set(key, withdrawn); + this.openRequests.set(key, { controller: withdrawn, thread: (msg.params as Record | undefined)?.threadId }); try { await this.answerServerRequest(msg, sink, withdrawn.signal); } finally { - if (this.openRequests.get(key) === withdrawn) this.openRequests.delete(key); + if (this.openRequests.get(key)?.controller === withdrawn) this.openRequests.delete(key); } } private async answerServerRequest(msg: RpcMessage, sink: TurnSink, withdrawn: AbortSignal): Promise { const p = (msg.params ?? {}) as Record; - const native: NativeRequest = { runtime: this.generation, request: String(msg.id), ...(typeof p.itemId === "string" && p.itemId ? { item: p.itemId } : {}) }; + const native: NativeRequest = { runtime: this.generation, request: JSON.stringify(msg.id), ...(typeof p.itemId === "string" && p.itemId ? { item: p.itemId } : {}) }; + const deliver = async (result: unknown) => { + const child = this.child; + const write = () => { + if (withdrawn.aborted || this.generation !== native.runtime || !child || child !== this.child || child.exitCode !== null) return Promise.reject(new Error("request transport unavailable")); + return writeJsonResponse(child.stdin, { id: msg.id, result }); + }; + if (sink.deliverRequest) await sink.deliverRequest(native, write); + else await write(); + }; if (msg.method === "item/commandExecution/requestApproval") { const cmd = stripShell(String(p.command ?? "")); const cwd = typeof p.cwd === "string" && p.cwd ? p.cwd : undefined; @@ -573,7 +595,7 @@ export class CodexBackend implements Backend { native, }, withdrawn); if (withdrawn.aborted) return; - this.respond(msg.id, { decision: answer === "yes" ? "accept" : answer === "always" ? "acceptForSession" : "decline" }); + await deliver({ decision: answer === "yes" ? "accept" : answer === "always" ? "acceptForSession" : "decline" }); } else if (msg.method === "item/fileChange/requestApproval") { const grantRoot = typeof p.grantRoot === "string" && p.grantRoot ? p.grantRoot : undefined; const paths = this.takeChangePaths(p); @@ -586,7 +608,7 @@ export class CodexBackend implements Backend { native, }, withdrawn); if (withdrawn.aborted) return; - this.respond(msg.id, { decision: answer === "yes" ? "accept" : answer === "always" ? "acceptForSession" : "decline" }); + await deliver({ decision: answer === "yes" ? "accept" : answer === "always" ? "acceptForSession" : "decline" }); } else if (msg.method === "item/permissions/requestApproval") { const permissions = requestedPermissions(p.permissions); if (!permissions) { @@ -596,7 +618,7 @@ export class CodexBackend implements Backend { } const answer = await sink.approval({ question: "Allow these additional permissions for this turn?", detail: JSON.stringify({ ...p, permissions }, null, 2), canAlways: false, action: { backend: "codex", tool: "permissions", title: "grant additional permissions", escalation: true }, native }, withdrawn); if (withdrawn.aborted) return; - this.respond(msg.id, { permissions: answer === "yes" ? permissions : {}, scope: "turn" }); + await deliver({ permissions: answer === "yes" ? permissions : {}, scope: "turn" }); } else if (msg.method === "item/tool/requestUserInput" && sink.question) { // Experimental in codex-cli 0.163.0. A request Modex cannot read is declined whole, never half-answered. const asked = codexQuestions(p); @@ -608,7 +630,7 @@ export class CodexBackend implements Backend { const reply = await sink.question({ ...asked, native }, withdrawn); if (withdrawn.aborted) return; // A withdrawn question is an error reply, never an empty answer the model could take as the user's. - if ("answers" in reply) this.respond(msg.id, codexAnswerResponse(reply.answers)); + if ("answers" in reply) await deliver(codexAnswerResponse(reply.answers)); else this.send({ id: msg.id, error: { code: -32000, message: reply.withdrawn === "stopped" ? "withdrawn by modex: the turn was stopped" : "withdrawn by modex: the turn ended before anyone answered" } }); } else { sink.notice("warn", `Codex asked for ${msg.method}, which Modex does not support yet; declined.`); diff --git a/apps/desktop/src/main/engine/backends/mock.ts b/apps/desktop/src/main/engine/backends/mock.ts index d3222515..653d8026 100644 --- a/apps/desktop/src/main/engine/backends/mock.ts +++ b/apps/desktop/src/main/engine/backends/mock.ts @@ -1,3 +1,4 @@ +import { randomUUID } from "node:crypto"; import { Agent, MockProvider, defaultConfig, systemPrompt, type ModexConfig, type UI } from "@modex/core"; import type { ApprovalAction, Backend, ModelInfo, TurnOptions, TurnResult, TurnSink } from "./types.js"; import { LocalItems } from "./local-items.js"; @@ -28,13 +29,20 @@ export class MockBackend implements Backend { const cfg: ModexConfig = { ...defaultConfig({ MODEX_HOME: this.home }), ...policy, model: "mock", home: this.home, writable_roots: opts.addDirs ?? [] }; const handle = opts.resume ?? `mock-${Date.now().toString(36)}`; sink.session(handle); + const runtime = randomUUID(); const ui: UI = { info: (m) => sink.notice("info", m), assistant: () => {}, tool: () => {}, warn: (m) => sink.notice("warn", m), error: (m) => sink.notice("error", m), - confirm: (question, detail) => sink.approval({ question, detail, canAlways: true, action: mockAction(question, opts.cwd) }), + confirm: async (question, detail) => { + const native = { runtime, request: randomUUID() }; + const answer = await sink.approval({ question, detail, canAlways: true, action: mockAction(question, opts.cwd), native }); + let consumed = !sink.deliverRequest; + await sink.deliverRequest?.(native, async () => { consumed = true; }); + return consumed ? answer : "no"; + }, prompt: async () => null, close: () => {}, }; diff --git a/apps/desktop/src/main/engine/backends/types.ts b/apps/desktop/src/main/engine/backends/types.ts index 53401c7f..2543f3a4 100644 --- a/apps/desktop/src/main/engine/backends/types.ts +++ b/apps/desktop/src/main/engine/backends/types.ts @@ -1,3 +1,4 @@ +import type { Writable } from "node:stream"; import { StringDecoder } from "node:string_decoder"; import type { AgentActivity, ApprovalAnswer, BackendHealth, BackendId, Mode, ModelInfo } from "../../../shared/types.js"; import type { NativeRef, Observation } from "../../../shared/conversation-events.js"; @@ -92,6 +93,9 @@ export interface TurnSink { * without a person to ask leave it out, and the backend declines the request. */ question?(req: QuestionRequest, withdrawn?: AbortSignal): Promise; + /** Revalidate a local decision immediately before writing, then record the transport receipt. + * A completed write is not proof of remote execution. Never retry an uncertain write. */ + deliverRequest?(native: NativeRequest, write: () => Promise): Promise; /** * After `approval(req)` answered "no": the "when" text of the approval rule that refused it on the * user's behalf, so the backend can tell the model why. Undefined when a human answered. @@ -232,3 +236,27 @@ export function shortJson(v: unknown, max = 2000): string { const s = typeof v === "string" ? v : JSON.stringify(v, null, 2); return s.length > max ? s.slice(0, max) + "\n…" : s; } + + +/** A local write receipt only: it does not acknowledge remote execution. Never retries. */ +export function writeJsonResponse(stdin: Writable | null | undefined, payload: unknown): Promise { + return new Promise((resolve, reject) => { + if (!stdin?.writable || stdin.destroyed || stdin.writableEnded) return reject(new Error("request transport unavailable")); + let settled = false; + const cleanup = () => { stdin.off("error", failed); stdin.off("close", closed); }; + const finish = (error?: Error | null) => { + if (settled) return; + settled = true; + if (error) { + // Writable invokes its callback before emitting the associated error event. + setImmediate(cleanup); + reject(new Error("request transport unavailable")); + } else { cleanup(); resolve(); } + }; + const failed = () => finish(new Error("request transport unavailable")); + const closed = () => finish(new Error("request transport closed before its receipt")); + stdin.once("error", failed); + stdin.once("close", closed); + try { stdin.write(JSON.stringify(payload) + "\n", finish); } catch { failed(); } + }); +} diff --git a/apps/desktop/src/main/engine/companion.ts b/apps/desktop/src/main/engine/companion.ts index 120ca0f2..962d73f1 100644 --- a/apps/desktop/src/main/engine/companion.ts +++ b/apps/desktop/src/main/engine/companion.ts @@ -387,9 +387,16 @@ export function mobileItem(item: ThreadItem): Record { case "assistant": return { id: item.id, kind: item.kind, text: item.text, at: item.at, ...(item.phase ? { phase: item.phase } : {}), ...(item.outcome ? { outcome: item.outcome } : {}) }; case "thinking": return { id: item.id, kind: item.kind, text: item.text, at: item.at, ...(item.reasoning ? { reasoning: item.reasoning } : {}), ...(item.outcome ? { outcome: item.outcome } : {}) }; case "user": case "notice": return { id: item.id, kind: item.kind, text: item.text, at: item.at, ...(item.kind === "notice" ? { level: item.level } : {}) }; - // Who answered, whether it reached the CLI and why an unanswered card closed ride along the same way. - case "approval": return { id: item.id, kind: item.kind, question: item.question, detail: item.detail, answer: item.answer, ...(item.decidedBy?.source === "policy" ? { auto: item.decidedBy.ruleId.slice("policy:".length) } : item.decidedBy && item.decidedBy.decision !== "ask" ? { auto: "rule" } : {}), - ...(item.answeredBy ? { answeredBy: item.answeredBy } : {}), ...(item.delivery ? { delivery: item.delivery } : {}), ...(item.withdrawn ? { withdrawn: item.withdrawn } : {}), at: item.at }; + // A phone that reads only `answer` would show an uncertain or withdrawn card as decided, so those + // become notices. While a decision is sending, the card keeps its answer: the phone shows what was + // decided, and the answer means it offers no buttons. + case "approval": + if (item.delivery === "uncertain" || item.withdrawn) return { + id: item.id, kind: "notice", level: "info", at: item.at, + ...(item.answeredBy ? { answeredBy: item.answeredBy } : {}), ...(item.delivery ? { delivery: item.delivery } : {}), ...(item.withdrawn ? { withdrawn: item.withdrawn } : {}), + text: `${item.question}\n${item.delivery === "uncertain" ? "Delivery uncertain. Not resent; check the agent on your Mac." : "Request withdrawn. Nothing more can be approved."}`, + }; + return { id: item.id, kind: item.kind, question: item.question, detail: item.detail, answer: item.answer, ...(item.decidedBy?.source === "policy" ? { auto: item.decidedBy.ruleId.slice("policy:".length) } : item.decidedBy && item.decidedBy.decision !== "ask" ? { auto: "rule" } : {}), ...(item.answeredBy ? { answeredBy: item.answeredBy } : {}), ...(item.delivery ? { delivery: item.delivery } : {}), ...(item.withdrawn ? { withdrawn: item.withdrawn } : {}), at: item.at }; // A phone that reads only title and status shows every finished tool as done, so a finished tool // that did not succeed says how it ended in its title. case "tool": return { id: item.id, kind: item.kind, title: item.status === "done" && (item.outcome ? item.outcome !== "succeeded" : item.ok === false) ? `${item.title} · ${toolStatusLabel(item)}` : item.title, @@ -399,7 +406,7 @@ export function mobileItem(item: ThreadItem): Record { case "plan": return { id: item.id, kind: item.kind, text: `Plan · ${item.steps.filter((s) => s.status === "completed").length} of ${item.steps.length} reported done`, steps: item.steps, revision: item.revision, ...(item.outcome ? { outcome: item.outcome } : {}), at: item.at }; // Phones cannot answer questions yet: a notice every client can show, with the questions and never the answers. case "question": return { id: item.id, kind: "notice", level: "info", at: item.at, - text: `The agent asked: ${item.questions.map((q) => q.text).join(" · ")}\n${item.answers ? (item.delivery === "uncertain" ? "Answered on your Mac; Modex can't tell whether the agent received it." : "Answered on your Mac.") : item.withdrawn ? "Not answered." : "Answer it in Modex on your Mac."}` }; + text: `The agent asked: ${item.questions.map((q) => q.text).join(" · ")}\n${item.delivery === "uncertain" ? "Delivery uncertain. Not resent; check the agent on your Mac." : item.delivery === "sending" ? "Sending answer on your Mac." : item.answers ? "Answered on your Mac." : item.withdrawn ? "Not answered." : "Answer it in Modex on your Mac."}` }; // A kind this build does not know, such as one a newer Modex saved before a downgrade: an inert // notice, never `undefined`, which goes out as null and makes the phone reject the whole snapshot. // Its own fields are not forwarded, since they may hold anything. diff --git a/apps/desktop/src/main/engine/images/fixture.ts b/apps/desktop/src/main/engine/images/fixture.ts index 32bca93f..bec30b75 100644 --- a/apps/desktop/src/main/engine/images/fixture.ts +++ b/apps/desktop/src/main/engine/images/fixture.ts @@ -65,13 +65,19 @@ export const imageFixtureBackend: Backend = { local.assistant( "Offline image fixture: this is a simulated generation job.", ); + const native = { runtime: "offline-image", request: "create-image" }; const answer = await sink.approval({ + native, canAlways: false, question: "Create the offline fixture image?", detail: "Writes one synthetic PNG inside this image job's output folder. No provider or network request is made.", }); - if (answer === "no" || signal.aborted) return { status: "interrupted" }; + if (signal.aborted) return { status: "interrupted" }; + let consumed = false; + if (sink.deliverRequest) await sink.deliverRequest(native, async () => { consumed = true; }); + else consumed = true; + if (!consumed || answer === "no" || signal.aborted) return { status: "interrupted" }; if (!options.images) return { status: "failed", diff --git a/apps/desktop/src/main/engine/runner.ts b/apps/desktop/src/main/engine/runner.ts index a2951910..11b071e9 100644 --- a/apps/desktop/src/main/engine/runner.ts +++ b/apps/desktop/src/main/engine/runner.ts @@ -49,9 +49,9 @@ export interface DurableSendAdmission { } interface PreparedAdmission extends DurableSendAdmission { readonly rawText: string } -type PendingRequest = +type PendingRequest = { cleanup?: () => void; policyCurrent?: () => boolean; claim?: () => { binding: RequestBinding; digest: string } } & ( | { kind: "approval"; settle: (answer: ApprovalAnswer) => void } - | { kind: "question"; questions: Question[]; settle: (reply: QuestionReply) => void }; + | { kind: "question"; questions: Question[]; settle: (reply: QuestionReply) => void }); interface Live { abort: AbortController | null; @@ -218,12 +218,10 @@ export class ThreadRunner { if ((item.kind === "tool" || item.kind === "thinking" || item.kind === "assistant") && item.origin && item.outcome === "running") { item = item.kind === "assistant" ? { ...item, outcome: "unknown" } : item.kind === "tool" ? { ...item, status: "done", outcome: "unknown", ok: okFor("unknown") } : { ...item, status: "done", outcome: "unknown" }; } + if ((item.kind === "approval" || item.kind === "question") && item.delivery === "sending") return { ...item, delivery: "uncertain" }; // A card nobody answered: the process that could take the answer is gone, so it closes as ended. if (item.kind === "approval" && !item.answer) return { ...item, answer: "no", withdrawn: "ended" }; if (item.kind === "question" && !item.answers && !item.withdrawn) return { ...item, withdrawn: "ended" }; - // An answer taken but not recorded as handed to the CLI: nobody can tell whether the CLI got it, and it is - // never sent again. - if ((item.kind === "approval" || item.kind === "question") && item.delivery === "sending") return { ...item, delivery: "uncertain" }; if (item.kind !== "tool" || !item.agent || !["running", "waiting"].includes(item.agent.state)) return item; return { ...item, status: "done", ok: undefined, agent: { ...item.agent, state: "unknown", detail: "Modex restarted without a final agent status" } }; }); @@ -768,7 +766,7 @@ export class ThreadRunner { }); // Each run of the backend is one attempt with its own sink; ending it settles whatever it left open. const runOn = async (ran: Thread): Promise => { - const attempt = this.beginAttempt(threadId, l, { thread: threadId, turn: turnId, attempt: newId(), backend: backend.id, ...(account ? { account } : {}) }); + const attempt = this.beginAttempt(threadId, l, { thread: threadId, turn: turnId, attempt: newId(), backend: ran.backend, ...((ran.codexAccount ?? account) ? { account: ran.codexAccount ?? account } : {}) }); try { const result = await backend.runTurn( describeAttachments(text, turnFiles), @@ -946,11 +944,9 @@ export class ThreadRunner { const accepted = this.accept(l, itemId, { kind: "approval", answer }, by); const pending = l.pending.get(itemId); if (!accepted.ok || pending?.kind !== "approval") return accepted.ok ? { ok: false, reason: "invalid" } : accepted; - l.pending.delete(itemId); this.patchItem(threadId, itemId, { answer, answeredBy: by, delivery: "sending" }); if (l.pending.size === 0 && l.status === "waiting") this.setStatus(threadId, "running"); pending.settle(answer); - this.sent(threadId, l, itemId); return { ok: true }; } @@ -967,21 +963,13 @@ export class ThreadRunner { } const accepted = this.accept(l, itemId, { kind: "question", answers: answers as Record }, by); if (!accepted.ok || pending?.kind !== "question") return accepted.ok ? { ok: false, reason: "invalid" } : accepted; - const given = answers as Record; - l.pending.delete(itemId); + const given = structuredClone(answers) as Record; this.patchItem(threadId, itemId, { answers: publicAnswers(pending.questions, given), answeredBy: by, delivery: "sending" }); if (l.pending.size === 0 && l.status === "waiting") this.setStatus(threadId, "running"); pending.settle({ answers: given }); - this.sent(threadId, l, itemId); return { ok: true }; } - /** Records that an accepted answer was handed to the CLI, in the broker and on the card. */ - private sent(threadId: string, l: Live, itemId: string): void { - l.requests = settleRequest(l.requests, itemId, "sent"); - this.patchItem(threadId, itemId, { delivery: "sent" }); - } - /** Takes one answer for a waiting request through the broker, or says why not. */ private accept(l: Live, itemId: string, resolution: Resolution, by: string): AnswerResult { const record = l.requests.requests[itemId]; @@ -993,8 +981,25 @@ export class ThreadRunner { const answered = item.kind === "approval" ? Boolean(item.answer) && !item.withdrawn : Boolean(item.answers); return { ok: false, reason: answered ? "already-resolved" : "withdrawn" }; } - const result = resolveRequest(l.requests, itemId, resolution, { binding: record.request.binding, digest: record.request.digest, by }); - if (!result.ok) return { ok: false, reason: result.reason }; + if (record.state === "pending") { + const binding = record.request.binding; + const current = this.o.store.thread(binding.thread); + if (!current || current.backend !== binding.backend || (current.codexAccount ?? null) !== (binding.account ?? null) || + (binding.attempt && l.attempt !== binding.attempt)) { + this.withdraw(binding.thread, l, itemId, "ended"); + return { ok: false, reason: "wrong-identity" }; + } + } + if (record.state === "pending" && l.pending.get(itemId)?.policyCurrent?.() === false) { + this.withdraw(record.request.binding.thread, l, itemId, "ended"); + return { ok: false, reason: "changed-action" }; + } + const claim = l.pending.get(itemId)?.claim?.() ?? { binding: record.request.binding, digest: record.request.digest }; + const result = resolveRequest(l.requests, itemId, resolution, { ...claim, by }); + if (!result.ok) { + if (result.reason === "changed-action" || result.reason === "wrong-identity") this.withdraw(record.request.binding.thread, l, itemId, "ended"); + return { ok: false, reason: result.reason }; + } l.requests = result.state; return { ok: true }; } @@ -1008,8 +1013,10 @@ export class ThreadRunner { const pending = l.pending.get(itemId); if (!pending) return; l.pending.delete(itemId); + pending.cleanup?.(); + const resolving = l.requests.requests[itemId]?.state === "resolving"; l.requests = withdrawRequests(l.requests, { id: itemId }); - this.patchItem(threadId, itemId, pending.kind === "approval" ? { answer: "no", withdrawn: why } : { withdrawn: why }, { persist }); + this.patchItem(threadId, itemId, resolving ? { delivery: "uncertain", withdrawn: why } : pending.kind === "approval" ? { answer: "no", withdrawn: why } : { withdrawn: why }, { persist }); if (why === "native" && l.pending.size === 0 && l.status === "waiting") this.setStatus(threadId, "running"); if (pending.kind === "approval") pending.settle("no"); else pending.settle({ withdrawn: why }); @@ -1204,6 +1211,12 @@ export class ThreadRunner { const abort = l.abort; const attempt = turn?.attempt; const active = () => this.live.get(threadId) === l && l.abort === abort && abort !== null && (attempt === undefined || l.attempt === attempt); + const authority = () => { + const current = this.o.store.thread(threadId); + return active() && !abort?.signal.aborted && Boolean(current) && current!.backend === (turn?.backend ?? l.backend) && + (current!.codexAccount ?? null) === (turn?.account ?? null); + }; + const deliveries = new Set(); // Who asked: this turn and attempt, and the CLI's own request id when it gives one (else the card's id). const bind = (id: string, native?: NativeRequest): RequestBinding => ({ thread: threadId, turn: turn?.turn ?? "", attempt: attempt ?? "", backend: turn?.backend ?? this.o.store.thread(threadId)?.backend ?? "mock", @@ -1227,21 +1240,36 @@ export class ThreadRunner { if (id) this.toolLogs.append(threadId, id, chunk); }, approval: (req, withdrawn) => { + const policyDigest = () => actionDigest({ rules: this.o.store.settings.approval_rules, gate: this.o.store.settings.approval_gate, mode: this.o.store.thread(threadId)?.mode }); + const policy = policyDigest(); + const policyCurrent = () => policyDigest() === policy; + const decided = (answer: "yes" | "no", decidedBy?: ApprovalReceipt): ApprovalAnswer => { + if (!authority() || withdrawn?.aborted || !policyCurrent()) return "no"; + const approvalMode = this.o.store.thread(threadId)?.approvals; + const automaticPolicyCurrent = () => policyCurrent() && this.o.store.thread(threadId)?.approvals === approvalMode; + const id = newId(); + const claim = () => ({ binding: bind(id, req.native), digest: actionDigest(req.action ?? { question: req.question, detail: req.detail ?? null }) }); + const close = () => this.withdraw(threadId, l, id, "native"); + l.requests = registerRequest(l.requests, { kind: "approval", id, ...claim(), action: req.action ?? null, canAlways: req.canAlways }); + l.requests = resolveRequest(l.requests, id, { kind: "approval", answer }, { ...claim(), by: "policy" }).state; + l.pending.set(id, { kind: "approval", claim, policyCurrent: automaticPolicyCurrent, cleanup: () => withdrawn?.removeEventListener("abort", close), settle: () => {} }); + withdrawn?.addEventListener("abort", close, { once: true }); + this.addItem(threadId, { id, kind: "approval", question: req.question, detail: req.detail, canAlways: req.canAlways, ...(req.action ? { title: req.action.title } : {}), answer, delivery: "sending", ...(decidedBy ? { decidedBy } : {}), at: at() }); + return answer; + }; if (this.unattended.has(threadId)) { // A routine runs read-only with no one to ask: refuse, and say so on the card. - if (active()) this.addItem(threadId, { id: newId(), kind: "approval", question: req.question, detail: req.detail, ...(req.action ? { title: req.action.title } : {}), answer: "no", decidedBy: UNATTENDED_RECEIPT, at: at() }); - return Promise.resolve("no" as const); + return Promise.resolve(decided("no", UNATTENDED_RECEIPT)); } // Stopped, superseded, or withdrawn by the CLI: nothing more is shown or decided for this request. - const gone = () => !active() || Boolean(l.abort?.signal.aborted) || Boolean(withdrawn?.aborted); + const gone = () => !authority() || !policyCurrent() || Boolean(l.abort?.signal.aborted) || Boolean(withdrawn?.aborted); // Today's flow: a card the user answers. Rules can only add a receipt to it. const askUser = (decidedBy?: ApprovalReceipt) => new Promise((resolve) => { if (gone()) return resolve("no"); const id = newId(); const close = () => this.withdraw(threadId, l, id, "native"); - l.pending.set(id, { kind: "approval", settle: (answer) => { - withdrawn?.removeEventListener("abort", close); + l.pending.set(id, { kind: "approval", policyCurrent, claim: () => ({ binding: bind(id, req.native), digest: actionDigest(req.action ?? { question: req.question, detail: req.detail ?? null }) }), cleanup: () => withdrawn?.removeEventListener("abort", close), settle: (answer) => { resolve(answer); } }); l.requests = registerRequest(l.requests, { kind: "approval", id, binding: bind(id, req.native), action: req.action ?? null, digest: actionDigest(req.action ?? { question: req.question, detail: req.detail ?? null }), canAlways: req.canAlways }); @@ -1257,8 +1285,7 @@ export class ThreadRunner { const policy = this.o.store.thread(threadId)?.approvals; if (policyAnswer(policy, req.action, decidedBy) !== "yes") return askUser(decidedBy); if (gone()) return "no" as const; - this.addItem(threadId, { id: newId(), kind: "approval", question: req.question, detail: req.detail, canAlways: req.canAlways, ...(req.action ? { title: req.action.title } : {}), answer: "yes", decidedBy: policyReceipt(policy!), at: at() }); - return "yes" as const; + return decided("yes", policyReceipt(policy!)); }; if (!thread || !project || !gateApplies(req.action, settings.approval_rules, settings.approval_gate, project.path)) return Promise.resolve(byPolicy()); const signal = withdrawn && abort ? AbortSignal.any([abort.signal, withdrawn]) : abort?.signal ?? withdrawn; @@ -1267,7 +1294,7 @@ export class ThreadRunner { if (gone()) return "no"; // Answered by a rule: the card lands already answered, and the thread never waits on it. if (r.decidedBy) { - this.addItem(threadId, { id: newId(), kind: "approval", question: req.question, detail: req.detail, canAlways: req.canAlways, ...(req.action ? { title: req.action.title } : {}), answer: r.answer, decidedBy: r.decidedBy, at: at() }); + decided(r.answer, r.decidedBy); if (r.answer === "no") refusals.set(req, r.decidedBy.when); } return r.answer; @@ -1283,8 +1310,7 @@ export class ThreadRunner { if (!active() || l.abort?.signal.aborted) return resolve({ withdrawn: "stopped" }); const id = newId(); const close = () => this.withdraw(threadId, l, id, "native"); - l.pending.set(id, { kind: "question", questions: req.questions, settle: (reply) => { - withdrawn?.removeEventListener("abort", close); + l.pending.set(id, { kind: "question", questions: req.questions, claim: () => ({ binding: bind(id, req.native), digest: actionDigest(req.questions) }), cleanup: () => withdrawn?.removeEventListener("abort", close), settle: (reply) => { resolve(reply); } }); l.requests = registerRequest(l.requests, { kind: "question", id, binding: bind(id, req.native), questions: req.questions, blocking: req.blocking, digest: actionDigest(req.questions) }); @@ -1294,6 +1320,39 @@ export class ThreadRunner { withdrawn?.addEventListener("abort", close, { once: true }); }); }, + deliverRequest: async (native, write) => { + const key = JSON.stringify([native.runtime, native.request]); + if (deliveries.has(key)) return; + deliveries.add(key); // Claim before the first await, including policy decisions. + const record = Object.values(l.requests.requests).find(({ request }) => { + const b = request.binding; + return b.runtime === native.runtime && b.nativeRequestId === native.request && b.item === native.item; + }); + // Policy answers have no waiting human card. Keep their existing decision semantics. + if (!record) { + if (authority()) { + try { await write(); } catch { /* no retry of policy decisions */ } + } + return; + } + if (record.state !== "resolving") return; + const id = record.request.id; + const current = this.o.store.thread(threadId); + const claim = l.pending.get(id)?.claim?.(); + if (l.pending.get(id)?.policyCurrent?.() === false || (claim && (claim.digest !== record.request.digest || actionDigest(claim.binding) !== actionDigest(record.request.binding))) || !active() || abort?.signal.aborted || !current || current.backend !== record.request.binding.backend || + (current.codexAccount ?? null) !== (record.request.binding.account ?? null)) { + this.withdraw(threadId, l, id, "ended"); + return; + } + let outcome: "sent" | "uncertain" = "uncertain"; + try { await write(); outcome = "sent"; } catch { /* no retry and no transport error body in receipts */ } + if (l.requests.requests[id]?.state !== "resolving") return; + l.requests = settleRequest(l.requests, id, outcome); + l.pending.get(id)?.cleanup?.(); + l.pending.delete(id); + this.patchItem(threadId, id, { delivery: outcome }); + if (l.pending.size === 0 && l.status === "waiting") this.setStatus(threadId, "running"); + }, refusedByRule: (req) => refusals.get(req), notice: (level, text) => { if (active()) this.addItem(threadId, { id: newId(), kind: "notice", level, text, at: at() }); }, session: (handle) => { diff --git a/apps/desktop/src/renderer/components/ThreadView.tsx b/apps/desktop/src/renderer/components/ThreadView.tsx index ed1c365a..f3c632fe 100644 --- a/apps/desktop/src/renderer/components/ThreadView.tsx +++ b/apps/desktop/src/renderer/components/ThreadView.tsx @@ -373,7 +373,7 @@ function ApprovalItem({ item, onAnswer, onPolicy }: { item: ApprovalEntry; onAns const r = item.decidedBy; if (r && item.answer && r.decision !== "ask") return ; return ( -
+
⚠ {item.question} @@ -382,7 +382,7 @@ function ApprovalItem({ item, onAnswer, onPolicy }: { item: ApprovalEntry; onAns {item.detail &&
{item.detail}
} {item.answer ? ( <> -
{item.withdrawn ? WITHDRAWN[item.withdrawn] : item.answer === "yes" ? "Approved" : item.answer === "always" ? "Approved — always for this command" : "Denied"}
+
{item.delivery === "uncertain" ? "Delivery uncertain — not resent" : item.delivery === "sending" ? "Sending decision…" : item.withdrawn ? WITHDRAWN[item.withdrawn] : item.answer === "yes" ? "Approved" : item.answer === "always" ? "Approved — always for this command" : "Denied"}
{!item.withdrawn && answerNote(item) &&
{answerNote(item)}
} ) : ( @@ -405,7 +405,7 @@ function ApprovalReceiptRow({ item, receipt: r }: { item: ApprovalEntry; receipt const title = item.title ?? item.question; const via = `${receiptVia(r)} · ${r.ms} ms${jevNotConsulted(r) ? " · Jev not consulted" : ""}`; return ( -
+
+ {item.delivery && item.delivery !== "sent" &&
{item.delivery === "uncertain" ? "Delivery uncertain — not resent" : "Sending decision…"}
} {open && (
{item.question}
diff --git a/apps/desktop/src/renderer/components/transcript/QuestionCard.tsx b/apps/desktop/src/renderer/components/transcript/QuestionCard.tsx index 33efc41b..2be1e794 100644 --- a/apps/desktop/src/renderer/components/transcript/QuestionCard.tsx +++ b/apps/desktop/src/renderer/components/transcript/QuestionCard.tsx @@ -34,12 +34,12 @@ export function QuestionCard({ item, onAnswer }: { item: QuestionEntry; onAnswer const [sending, setSending] = useState(false); const open = !item.answers && !item.withdrawn; const typed = (q: Question) => !q.options || choice[q.id] === OTHER; - const value = (q: Question) => (typed(q) ? text[q.id] ?? "" : choice[q.id] ?? "").trim(); + const value = (q: Question) => (typed(q) ? text[q.id] ?? "" : choice[q.id] ?? ""); const submit = async () => { setSending(true); setError(null); try { - const result = await onAnswer(item.id, Object.fromEntries(item.questions.map((q) => [q.id, value(q) ? [value(q)] : []]))); + const result = await onAnswer(item.id, Object.fromEntries(item.questions.map((q) => [q.id, value(q).trim() ? [value(q)] : []]))); if (result && !result.ok) setError(result.error ?? REFUSED[result.reason]); } catch (err) { setError((err as Error).message); @@ -48,7 +48,7 @@ export function QuestionCard({ item, onAnswer }: { item: QuestionEntry; onAnswer } }; return ( -
+
? {item.questions.length === 1 ? "The agent has a question" : `The agent has ${item.questions.length} questions`} @@ -86,7 +86,7 @@ export function QuestionCard({ item, onAnswer }: { item: QuestionEntry; onAnswer
))} {item.answers && answerNote(item) &&
{answerNote(item)}
} - {item.withdrawn ?
{CLOSED[item.withdrawn]}
: open && ( + {item.delivery === "uncertain" || item.delivery === "sending" ?
{item.delivery === "uncertain" ? "Delivery uncertain — not resent. Check the agent before answering again." : "Sending answer…"}
: item.withdrawn ?
{CLOSED[item.withdrawn]}
: open && (
{error && {error}} diff --git a/apps/desktop/src/shared/human-requests.ts b/apps/desktop/src/shared/human-requests.ts index f1d31e89..40ed14ad 100644 --- a/apps/desktop/src/shared/human-requests.ts +++ b/apps/desktop/src/shared/human-requests.ts @@ -75,7 +75,7 @@ export function emptyRequests(): RequestsState { export function register(state: RequestsState, request: HumanRequest): RequestsState { if (state.requests[request.id]) return state; - return { ...state, requests: { ...state.requests, [request.id]: { request, state: "pending" } } }; + return { ...state, requests: { ...state.requests, [request.id]: { request: structuredClone(request), state: "pending" } } }; } /** @@ -91,8 +91,10 @@ export function resolve(state: RequestsState, id: string, resolution: Resolution if (!sameBinding(record.request.binding, claim.binding)) return refuse("wrong-identity"); if (claim.digest !== record.request.digest) return refuse("changed-action"); if (resolution.kind !== record.request.kind) return refuse("invalid"); + if (resolution.kind === "approval" && record.request.kind === "approval" && + (!["yes", "no", "always"].includes(resolution.answer) || (resolution.answer === "always" && !record.request.canAlways))) return refuse("invalid"); if (resolution.kind === "question" && record.request.kind === "question" && !validateAnswers(record.request.questions, resolution.answers).ok) return refuse("invalid"); - return { ok: true, state: { ...state, requests: { ...state.requests, [id]: { ...record, state: "resolving", resolution, by: claim.by } } } }; + return { ok: true, state: { ...state, requests: { ...state.requests, [id]: { ...record, state: "resolving", resolution: structuredClone(resolution), by: claim.by } } } }; } /** Records the outcome of sending an accepted answer. Only a `resolving` request changes. */ @@ -160,11 +162,16 @@ export function publicAnswers(questions: readonly Question[], answers: Record(); for (const q of params.questions) { if (!isRecord(q) || typeof q.id !== "string" || !q.id || typeof q.question !== "string") return null; + if (ids.has(q.id) || (q.header !== undefined && typeof q.header !== "string") || + (q.isOther !== undefined && typeof q.isOther !== "boolean") || (q.isSecret !== undefined && typeof q.isSecret !== "boolean")) return null; + ids.add(q.id); let options: QuestionOption[] | null = null; if (q.options !== null && q.options !== undefined) { if (!Array.isArray(q.options) || !q.options.every((o) => isRecord(o) && typeof o.label === "string" && (o.description === undefined || typeof o.description === "string"))) return null; + if (new Set(q.options.map(o => (o as Record).label)).size !== q.options.length) return null; options = (q.options as { label: string; description?: string }[]).map((o) => ({ label: o.label, description: o.description ?? "" })); } questions.push({ id: q.id, header: typeof q.header === "string" ? q.header : "", text: q.question, options, allowOther: q.isOther === true || options === null, secret: q.isSecret === true }); diff --git a/apps/desktop/test/companion.test.ts b/apps/desktop/test/companion.test.ts index 74f3f68b..006f3101 100644 --- a/apps/desktop/test/companion.test.ts +++ b/apps/desktop/test/companion.test.ts @@ -493,3 +493,21 @@ test("the phone acts on a thread created on the Mac, whose id is the full UUID i fs.rmSync(home, { recursive: true, force: true }); } }); + +test("companion fallback makes uncertain and resolving deliveries explicit without exposing answers", async () => { + const { mobileItem } = await import("../src/main/engine/companion.js"); + const base = { id: "question", kind: "question" as const, questions: [{ id: "token", header: "Token", text: "Token?", options: null, allowOther: true, secret: true }], blocking: true, answers: { token: ["(hidden)"] }, at: "now" }; + for (const delivery of ["sending", "uncertain"] as const) { + const mobile = mobileItem({ ...base, delivery }); + assert.doesNotMatch(String(mobile.text), /Answered on your Mac/); + assert.match(String(mobile.text), delivery === "uncertain" ? /Delivery uncertain/ : /Sending answer/); + } + const approval = mobileItem({ id: "approval", kind: "approval", question: "Deploy?", answer: "yes", delivery: "uncertain", at: "now" }); + assert.equal(approval.kind, "notice"); + assert.match(String(approval.text), /Delivery uncertain/); + // A decision still sending stays an approval with its answer, so the phone shows it as decided. + const sending = mobileItem({ id: "approval", kind: "approval", question: "Deploy?", answer: "yes", answeredBy: "phone", delivery: "sending", at: "now" }); + assert.equal(sending.kind, "approval"); + assert.equal(sending.answer, "yes"); + assert.equal(sending.delivery, "sending"); +}); diff --git a/apps/desktop/test/human-requests-runner.test.ts b/apps/desktop/test/human-requests-runner.test.ts index 06896dbd..3126c10c 100644 --- a/apps/desktop/test/human-requests-runner.test.ts +++ b/apps/desktop/test/human-requests-runner.test.ts @@ -64,7 +64,7 @@ test("an approval withdrawn by Stop refuses a late answer as withdrawn", async ( }); /** A Codex app-server whose turn asks one requestUserInput question (codex-cli 0.163.0 shape) and finishes when answered. */ -function codexAsking(proc: FakeProcess, params: Record) { +function codexAsking(proc: FakeProcess, params: Record, method = "item/tool/requestUserInput", finish = true) { const replies: unknown[] = []; proc.stdin.on("data", (d: Buffer) => { for (const line of d.toString().split("\n")) { @@ -75,13 +75,13 @@ function codexAsking(proc: FakeProcess, params: Record) { else if (msg.method === "turn/start") { proc.emitLine({ id: msg.id, result: { turn: { id: "tu" } } }); proc.emitLine({ method: "turn/started", params: { threadId: "thr", turn: { id: "tu" } } }); - proc.emitLine({ id: 7, method: "item/tool/requestUserInput", params: { threadId: "thr", turnId: "tu", itemId: "ask-1", isBlocking: true, autoResolutionMs: null, ...params } }); + proc.emitLine({ id: 7, method, params: { threadId: "thr", turnId: "tu", itemId: "ask-1", isBlocking: true, autoResolutionMs: null, ...params } }); } else if (msg.method === "turn/interrupt") { proc.emitLine({ id: msg.id, result: {} }); proc.emitLine({ method: "turn/completed", params: { threadId: "thr", turn: { id: "tu", status: "interrupted", error: null } } }); } else if (msg.id === 7 && !msg.method) { replies.push(msg.result ?? { error: msg.error }); - proc.emitLine({ method: "turn/completed", params: { threadId: "thr", turn: { id: "tu", status: "completed", error: null } } }); + if (finish) setImmediate(() => proc.emitLine({ method: "turn/completed", params: { threadId: "thr", turn: { id: "tu", status: "completed", error: null } } })); } } }); @@ -176,3 +176,477 @@ test("Codex: a question Codex resolves itself (serverRequest/resolved) closes as assert.deepEqual(replies, [], "Codex already settled it"); } finally { await h.runner.dispose(); await codex.dispose(); } }); + + +test("Codex: withdrawal after local acceptance but before the native write leaves delivery uncertain", async () => { + const proc = new FakeProcess(); + const replies = codexAsking(proc, { questions }); + const codex = new CodexBackend("codex", fakeSpawn(proc).spawn); + const h = await harness(codex); + try { + const sent = h.runner.send(h.thread.id, "deploy it"); + await waitFor(() => h.runner.items(h.thread.id).some((i) => i.kind === "question")); + const card = h.runner.items(h.thread.id).find((i) => i.kind === "question")!; + assert.deepEqual(h.runner.answerQuestion(h.thread.id, card.id, { "q-env": ["staging"], "q-token": ["s3cr3t"] }), { ok: true }); + // The promise resumes in a microtask. Native withdrawal can win before any response is written. + proc.emitLine({ method: "serverRequest/resolved", params: { threadId: "thr", requestId: 7 } }); + await new Promise(resolve => setImmediate(resolve)); + assert.deepEqual(replies, [], "the CLI received no answer"); + const saved = h.runner.items(h.thread.id).find((i) => i.id === card.id) as ThreadItem & { delivery?: string }; + assert.equal(saved.delivery, "uncertain", "local acceptance must not be presented as confirmed delivery"); + proc.emitLine({ method: "turn/completed", params: { threadId: "thr", turn: { id: "tu", status: "completed", error: null } } }); + await sent; + assert.ok(!JSON.stringify([new Store(h.home).items(h.thread.id), h.events]).includes("s3cr3t")); + } finally { await h.runner.dispose(); await codex.dispose(); } +}); + + +test("Codex: Stop after local question acceptance prevents the answer write and records uncertainty", async () => { + const proc = new FakeProcess(); + const replies = codexAsking(proc, { questions }); + const codex = new CodexBackend("codex", fakeSpawn(proc).spawn); + const h = await harness(codex); + try { + const sent = h.runner.send(h.thread.id, "deploy it"); + await waitFor(() => h.runner.items(h.thread.id).some((i) => i.kind === "question")); + const card = h.runner.items(h.thread.id).find((i) => i.kind === "question")!; + h.runner.answerQuestion(h.thread.id, card.id, { "q-env": ["staging"], "q-token": ["secret"] }); + h.runner.stop(h.thread.id); + await sent; + assert.deepEqual(replies, [], "an accepted answer must not escape after Stop"); + assert.equal((h.runner.items(h.thread.id).find((i) => i.id === card.id) as { delivery?: string }).delivery, "uncertain"); + } finally { await h.runner.dispose(); await codex.dispose(); } +}); + + +test("Codex: an account change between acceptance and delivery cannot send the old answer", async () => { + const proc = new FakeProcess(); + const replies = codexAsking(proc, { questions }); + const codex = new CodexBackend("codex", fakeSpawn(proc).spawn); + const h = await harness(codex); + try { + const sent = h.runner.send(h.thread.id, "deploy it"); + await waitFor(() => h.runner.items(h.thread.id).some((i) => i.kind === "question")); + const card = h.runner.items(h.thread.id).find((i) => i.kind === "question")!; + h.runner.answerQuestion(h.thread.id, card.id, { "q-env": ["staging"], "q-token": ["secret"] }); + h.store.updateThread(h.thread.id, { codexAccount: "replacement" }); + await new Promise(resolve => setImmediate(resolve)); + assert.deepEqual(replies, []); + assert.equal((h.runner.items(h.thread.id).find((i) => i.id === card.id) as { delivery?: string }).delivery, "uncertain"); + proc.emitLine({ method: "turn/completed", params: { threadId: "thr", turn: { id: "tu", status: "completed", error: null } } }); + await sent; + } finally { await h.runner.dispose(); await codex.dispose(); } +}); + +test("Codex: a failed write stays uncertain and a second client cannot resend the answer", async () => { + const proc = new FakeProcess(); + const replies = codexAsking(proc, { questions }); + const codex = new CodexBackend("codex", fakeSpawn(proc).spawn); + const h = await harness(codex); + try { + const sent = h.runner.send(h.thread.id, "deploy it"); + await waitFor(() => h.runner.items(h.thread.id).some((i) => i.kind === "question")); + const card = h.runner.items(h.thread.id).find((i) => i.kind === "question")!; + const original = proc.stdin.write.bind(proc.stdin); + let writes = 0; + proc.stdin.write = ((chunk: string, callback: (error?: Error) => void) => { + if (JSON.parse(chunk).id !== 7) return original(chunk); + writes++; + callback(new Error("transport failed")); + return false; + }) as typeof proc.stdin.write; + h.runner.answerQuestion(h.thread.id, card.id, { "q-env": ["staging"], "q-token": ["failure-only-token"] }); + await waitFor(() => (h.runner.items(h.thread.id).find((i) => i.id === card.id) as { delivery?: string }).delivery === "uncertain"); + assert.deepEqual(h.runner.answerQuestion(h.thread.id, card.id, { "q-env": ["production"], "q-token": ["failure-only-token"] }, "phone"), { ok: false, reason: "already-resolved" }); + assert.equal(writes, 1); + assert.deepEqual(replies, []); + proc.emitLine({ method: "turn/completed", params: { threadId: "thr", turn: { id: "tu", status: "completed", error: null } } }); + await sent; + assert.ok(!JSON.stringify([h.events, new Store(h.home).items(h.thread.id)]).includes("failure-only-token")); + } finally { await h.runner.dispose(); await codex.dispose(); } +}); + + +test("Codex: an approval records sent only after its response write completes", async () => { + const proc = new FakeProcess(); + const replies = codexAsking(proc, { command: "npm test" }, "item/commandExecution/requestApproval", false); + const codex = new CodexBackend("codex", fakeSpawn(proc).spawn); + const h = await harness(codex); + try { + const sent = h.runner.send(h.thread.id, "test it"); + await waitFor(() => h.runner.items(h.thread.id).some((i) => i.kind === "approval")); + const card = h.runner.items(h.thread.id).find((i) => i.kind === "approval")!; + assert.deepEqual(h.runner.answer(h.thread.id, card.id, "yes"), { ok: true }); + assert.equal((h.runner.items(h.thread.id).find((i) => i.id === card.id) as { delivery?: string }).delivery, "sending"); + await waitFor(() => (h.runner.items(h.thread.id).find((i) => i.id === card.id) as { delivery?: string }).delivery === "sent"); + assert.deepEqual(replies, [{ decision: "accept" }]); + proc.emitLine({ method: "turn/completed", params: { threadId: "thr", turn: { id: "tu", status: "completed", error: null } } }); + await sent; + } finally { await h.runner.dispose(); await codex.dispose(); } +}); + +test("Claude: the native approval write settles its delivery receipt", async () => { + const { ClaudeBackend } = await import("../src/main/engine/backends/claude.js"); + const proc = new FakeProcess(); + const claude = new ClaudeBackend("claude", fakeSpawn(proc).spawn); + const h = await harness(claude); + try { + const sent = h.runner.send(h.thread.id, "test it"); + await proc.waitFor(line => line.includes('"type":"user"')); + proc.emitLine({ type: "control_request", request_id: "r1", request: { subtype: "can_use_tool", tool_name: "Bash", input: { command: "npm test" }, tool_use_id: "tool1" } }); + await waitFor(() => h.runner.items(h.thread.id).some((i) => i.kind === "approval")); + const card = h.runner.items(h.thread.id).find((i) => i.kind === "approval")!; + h.runner.answer(h.thread.id, card.id, "yes"); + await waitFor(() => (h.runner.items(h.thread.id).find((i) => i.id === card.id) as { delivery?: string }).delivery === "sent"); + assert.equal(proc.sent(m => m.type === "control_response" && m.response.request_id === "r1").length, 1); + proc.close(0); + await sent; + } finally { proc.close(0); await h.runner.dispose(); await claude.dispose(); } +}); + +test("a changed backend cannot accept an old card", async () => { + const proc = new FakeProcess(); + codexAsking(proc, { questions }); + const codex = new CodexBackend("codex", fakeSpawn(proc).spawn); + const h = await harness(codex); + try { + const sent = h.runner.send(h.thread.id, "deploy it"); + await waitFor(() => h.runner.items(h.thread.id).some((i) => i.kind === "question")); + const card = h.runner.items(h.thread.id).find((i) => i.kind === "question")!; + h.runner.updateThread(h.thread.id, { backend: "claude" }); + assert.deepEqual(h.runner.answerQuestion(h.thread.id, card.id, { "q-env": ["staging"], "q-token": ["token"] }), { ok: false, reason: "wrong-identity" }); + proc.emitLine({ method: "turn/completed", params: { threadId: "thr", turn: { id: "tu", status: "completed", error: null } } }); + await sent; + } finally { await h.runner.dispose(); await codex.dispose(); } +}); + +test("an accepted answer is immutable until the backend writes it", async () => { + const proc = new FakeProcess(); + const replies = codexAsking(proc, { questions }); + const codex = new CodexBackend("codex", fakeSpawn(proc).spawn); + const h = await harness(codex); + try { + const sent = h.runner.send(h.thread.id, "deploy it"); + await waitFor(() => h.runner.items(h.thread.id).some((i) => i.kind === "question")); + const card = h.runner.items(h.thread.id).find((i) => i.kind === "question")!; + const answers = { "q-env": ["staging"], "q-token": ["token"] }; + h.runner.answerQuestion(h.thread.id, card.id, answers); + answers["q-env"][0] = "production"; + await sent; + assert.deepEqual(replies, [{ answers: { "q-env": { answers: ["staging"] }, "q-token": { answers: ["token"] } } }]); + } finally { await h.runner.dispose(); await codex.dispose(); } +}); + +test("a changed action cannot use the original approval card", async () => { + const request = { question: "Allow Bash?", canAlways: true, action: { backend: "claude" as const, tool: "Bash", title: "test", input: { command: "npm test" } } }; + const backend: Backend = { id: "claude", listModels: async () => [], dispose: async () => {}, async runTurn(_t, _o, sink) { + await sink.approval(request); + return { status: "completed" }; + } }; + const h = await harness(backend); + try { + const sent = h.runner.send(h.thread.id, "go"); + await waitFor(() => h.runner.items(h.thread.id).some((i) => i.kind === "approval")); + const card = h.runner.items(h.thread.id).find((i) => i.kind === "approval")!; + request.action.input.command = "deploy"; + assert.deepEqual(h.runner.answer(h.thread.id, card.id, "yes"), { ok: false, reason: "changed-action" }); + h.runner.stop(h.thread.id); + await sent; + } finally { h.runner.stop(h.thread.id); await h.runner.dispose(); } +}); + +test("a policy decision cannot write after the thread switches backend", async () => { + let ready = false; + let release!: () => void; + let writes = 0; + const held = new Promise(resolve => { release = resolve; }); + const native = { runtime: "runtime", request: "request", item: "item" }; + const backend: Backend = { id: "codex", listModels: async () => [], dispose: async () => {}, async runTurn(_t, _o, sink) { + const answer = await sink.approval({ question: "Allow test?", canAlways: true, native, action: { backend: "codex", tool: "command", title: "test", input: { command: "npm test" } } }); + assert.equal(answer, "yes"); + ready = true; + await held; + await sink.deliverRequest!(native, async () => { writes++; }); + return { status: "completed" }; + } }; + const h = await harness(backend); + try { + h.runner.updateThread(h.thread.id, { approvals: "yolo" }); + const sent = h.runner.send(h.thread.id, "go"); + await waitFor(() => ready); + h.runner.updateThread(h.thread.id, { backend: "claude" }); + release(); + await sent; + assert.equal(writes, 0); + } finally { release(); await h.runner.dispose(); } +}); + +test("Codex: replaying a native question retains one card and its original cancellation controller", async () => { + const proc = new FakeProcess(); + const replies = codexAsking(proc, { questions }); + const codex = new CodexBackend("codex", fakeSpawn(proc).spawn); + const h = await harness(codex); + try { + const sent = h.runner.send(h.thread.id, "go"); + await waitFor(() => h.runner.items(h.thread.id).some(i => i.kind === "question")); + proc.emitLine({ id: 7, method: "item/tool/requestUserInput", params: { threadId: "thr", turnId: "tu", itemId: "ask-1", isBlocking: true, autoResolutionMs: null, questions } }); + await new Promise(resolve => setImmediate(resolve)); + assert.equal(h.runner.items(h.thread.id).filter(i => i.kind === "question").length, 1); + proc.emitLine({ method: "serverRequest/resolved", params: { threadId: "thr", requestId: 7 } }); + await waitFor(() => (h.runner.items(h.thread.id).find(i => i.kind === "question") as { withdrawn?: string }).withdrawn === "native"); + assert.deepEqual(replies, []); + proc.emitLine({ method: "turn/completed", params: { threadId: "thr", turn: { id: "tu", status: "completed", error: null } } }); + await sent; + } finally { await h.runner.dispose(); await codex.dispose(); } +}); + +test("Codex: reusing a native question id for changed questions withdraws the original", async () => { + const proc = new FakeProcess(); + const replies = codexAsking(proc, { questions }); + const codex = new CodexBackend("codex", fakeSpawn(proc).spawn); + const h = await harness(codex); + try { + const sent = h.runner.send(h.thread.id, "go"); + await waitFor(() => h.runner.items(h.thread.id).some(i => i.kind === "question")); + proc.emitLine({ id: 7, method: "item/tool/requestUserInput", params: { threadId: "thr", turnId: "tu", itemId: "ask-1", isBlocking: true, autoResolutionMs: null, questions: [{ ...questions[0], question: "Changed action?" }] } }); + await new Promise(resolve => setImmediate(resolve)); + const cards = h.runner.items(h.thread.id).filter(i => i.kind === "question"); + assert.equal(cards.length, 1); + assert.equal((cards[0] as { withdrawn?: string }).withdrawn, "native"); + assert.deepEqual(replies, []); + proc.emitLine({ method: "turn/completed", params: { threadId: "thr", turn: { id: "tu", status: "completed", error: null } } }); + await sent; + } finally { await h.runner.dispose(); await codex.dispose(); } +}); + +test("Claude: a replayed permission request keeps one card; changed input withdraws it", async () => { + const { ClaudeBackend } = await import("../src/main/engine/backends/claude.js"); + const proc = new FakeProcess(); + const claude = new ClaudeBackend("claude", fakeSpawn(proc).spawn); + const h = await harness(claude); + try { + const sent = h.runner.send(h.thread.id, "go"); + await proc.waitFor(line => line.includes('"type":"user"')); + const request = { type: "control_request", request_id: "r1", request: { subtype: "can_use_tool", tool_name: "Bash", input: { command: "npm test" }, tool_use_id: "tool1" } }; + proc.emitLine(request); + await waitFor(() => h.runner.items(h.thread.id).some(i => i.kind === "approval")); + proc.emitLine(request); + await new Promise(resolve => setImmediate(resolve)); + assert.equal(h.runner.items(h.thread.id).filter(i => i.kind === "approval").length, 1); + proc.emitLine({ ...request, request: { ...request.request, input: { command: "deploy" } } }); + await waitFor(() => (h.runner.items(h.thread.id).find(i => i.kind === "approval") as { withdrawn?: string }).withdrawn === "native"); + assert.equal(proc.sent(m => m.type === "control_response").length, 0); + proc.close(0); + await sent; + } finally { proc.close(0); await h.runner.dispose(); await claude.dispose(); } +}); + +test("a policy approval reports uncertain when its transport write fails", async () => { + const native = { runtime: "runtime", request: "request" }; + const backend: Backend = { id: "claude", listModels: async () => [], dispose: async () => {}, async runTurn(_t, _o, sink) { + const answer = await sink.approval({ question: "Allow test?", canAlways: true, native, action: { backend: "claude", tool: "Bash", title: "test", input: { command: "npm test" } } }); + assert.equal(answer, "yes"); + await sink.deliverRequest!(native, async () => { throw new Error("closed"); }); + return { status: "completed" }; + } }; + const h = await harness(backend); + try { + h.runner.updateThread(h.thread.id, { approvals: "yolo" }); + await h.runner.send(h.thread.id, "go"); + const card = h.runner.items(h.thread.id).find(i => i.kind === "approval") as { delivery?: string }; + assert.equal(card.delivery, "uncertain"); + } finally { await h.runner.dispose(); } +}); + +test("the offline backend acknowledges consuming its approval without leaving an uncertain card", async () => { + const { MockBackend } = await import("../src/main/engine/backends/mock.js"); + const fs = await import("node:fs"); + const path = await import("node:path"); + const home = tmpdir("modex-offline-delivery-"); + const script = path.join(home, "script.json"); + fs.writeFileSync(script, JSON.stringify({ steps: [{ tool_calls: [{ name: "apply_patch", arguments: { patch: "*** Begin Patch\n*** Add File: NOTE.md\n+hello\n*** End Patch" } }] }, { content: "Done" }] })); + const backend = new MockBackend(() => script, home, 0); + const h = await harness(backend); + try { + h.runner.updateThread(h.thread.id, { mode: "chat" }); + const sent = h.runner.send(h.thread.id, "go"); + await waitFor(() => h.runner.items(h.thread.id).some(i => i.kind === "approval")); + const card = h.runner.items(h.thread.id).find(i => i.kind === "approval")!; + h.runner.answer(h.thread.id, card.id, "no"); + await sent; + assert.equal((h.runner.items(h.thread.id).find(i => i.id === card.id) as { delivery?: string }).delivery, "sent"); + } finally { await h.runner.dispose(); } +}); + +test("Codex: a Writable error during an answer becomes an uncertain receipt without an uncaught stream error", async () => { + const proc = new FakeProcess(); + codexAsking(proc, { questions }); + const codex = new CodexBackend("codex", fakeSpawn(proc).spawn); + const h = await harness(codex); + try { + const sent = h.runner.send(h.thread.id, "go"); + await waitFor(() => h.runner.items(h.thread.id).some(i => i.kind === "question")); + const card = h.runner.items(h.thread.id).find(i => i.kind === "question")!; + proc.stdin._write = (_chunk, _encoding, callback) => callback(new Error("pipe failed")); + h.runner.answerQuestion(h.thread.id, card.id, { "q-env": ["staging"], "q-token": ["stream-error-token"] }); + await new Promise(resolve => setImmediate(resolve)); + assert.equal((h.runner.items(h.thread.id).find(i => i.id === card.id) as { delivery?: string }).delivery, "uncertain"); + proc.emitLine({ method: "turn/completed", params: { threadId: "thr", turn: { id: "tu", status: "completed", error: null } } }); + await sent; + } finally { await h.runner.dispose(); await codex.dispose(); } +}); + +test("Codex: stdin closing before the write callback leaves delivery uncertain", async () => { + const proc = new FakeProcess(); + codexAsking(proc, { questions }); + const codex = new CodexBackend("codex", fakeSpawn(proc).spawn); + const h = await harness(codex); + try { + const sent = h.runner.send(h.thread.id, "go"); + await waitFor(() => h.runner.items(h.thread.id).some(i => i.kind === "question")); + const card = h.runner.items(h.thread.id).find(i => i.kind === "question")!; + proc.stdin.write = (() => false) as typeof proc.stdin.write; + h.runner.answerQuestion(h.thread.id, card.id, { "q-env": ["staging"], "q-token": ["close-token"] }); + await new Promise(resolve => setImmediate(resolve)); + proc.stdin.emit("close"); + await waitFor(() => (h.runner.items(h.thread.id).find(i => i.id === card.id) as { delivery?: string }).delivery === "uncertain"); + proc.emitLine({ method: "turn/completed", params: { threadId: "thr", turn: { id: "tu", status: "completed", error: null } } }); + await sent; + } finally { await h.runner.dispose(); await codex.dispose(); } +}); + +test("changing approval rules invalidates an already displayed approval", async () => { + const native = { runtime: "runtime", request: "request" }; + let writes = 0; + const backend: Backend = { id: "claude", listModels: async () => [], dispose: async () => {}, async runTurn(_t, _o, sink) { + await sink.approval({ question: "Allow deploy?", canAlways: true, native, action: { backend: "claude", tool: "Bash", title: "deploy", input: { command: "deploy" } } }); + await sink.deliverRequest!(native, async () => { writes++; }); + return { status: "completed" }; + } }; + const h = await harness(backend); + try { + const sent = h.runner.send(h.thread.id, "go"); + await waitFor(() => h.runner.items(h.thread.id).some(i => i.kind === "approval")); + const card = h.runner.items(h.thread.id).find(i => i.kind === "approval")!; + h.store.updateSettings({ approval_gate: { enabled: true, threshold: 0.8, timeout_ms: 1000 }, approval_rules: [{ id: "never", when: "deploy", decision: "never", match: "Bash: deploy", enabled: true }] }); + assert.deepEqual(h.runner.answer(h.thread.id, card.id, "yes"), { ok: false, reason: "changed-action" }); + await sent; + assert.equal(writes, 0); + } finally { h.runner.stop(h.thread.id); await h.runner.dispose(); } +}); + +test("revoking YOLO prevents its accepted automatic decision from being written", async () => { + let ready = false; + let release!: () => void; + let writes = 0; + const held = new Promise(resolve => { release = resolve; }); + const native = { runtime: "runtime", request: "request", item: "item" }; + const backend: Backend = { id: "codex", listModels: async () => [], dispose: async () => {}, async runTurn(_t, _o, sink) { + const answer = await sink.approval({ question: "Allow test?", canAlways: true, native, action: { backend: "codex", tool: "command", title: "test", input: { command: "npm test" } } }); + assert.equal(answer, "yes"); + ready = true; + await held; + await sink.deliverRequest!(native, async () => { writes++; }); + return { status: "completed" }; + } }; + const h = await harness(backend); + try { + h.runner.updateThread(h.thread.id, { approvals: "yolo" }); + const sent = h.runner.send(h.thread.id, "go"); + await waitFor(() => ready); + h.runner.updateThread(h.thread.id, { approvals: "ask" }); + release(); + await sent; + assert.equal(writes, 0); + } finally { release(); await h.runner.dispose(); } +}); + +test("a delivery is claimed before awaiting its write callback", async () => { + const native = { runtime: "runtime", request: "request" }; + let writes = 0; + let release!: () => void; + const held = new Promise(resolve => { release = resolve; }); + const backend: Backend = { id: "claude", listModels: async () => [], dispose: async () => {}, async runTurn(_t, _o, sink) { + await sink.approval({ question: "Allow?", canAlways: true, native, action: { backend: "claude", tool: "Bash", title: "test", input: { command: "npm test" } } }); + const first = sink.deliverRequest!(native, async () => { writes++; await held; }); + await sink.deliverRequest!(native, async () => { writes++; }); + release(); + await first; + return { status: "completed" }; + } }; + const h = await harness(backend); + try { + h.runner.updateThread(h.thread.id, { approvals: "yolo" }); + await h.runner.send(h.thread.id, "go"); + assert.equal(writes, 1); + } finally { release(); await h.runner.dispose(); } +}); + +test("changing mode invalidates a queued automatic approval", async () => { + let ready = false; + let release!: () => void; + let writes = 0; + const held = new Promise(resolve => { release = resolve; }); + const native = { runtime: "runtime", request: "request", item: "item" }; + const backend: Backend = { id: "codex", listModels: async () => [], dispose: async () => {}, async runTurn(_t, _o, sink) { + const answer = await sink.approval({ question: "Allow test?", canAlways: true, native, action: { backend: "codex", tool: "command", title: "test", input: { command: "npm test" } } }); + assert.equal(answer, "yes"); + ready = true; + await held; + await sink.deliverRequest!(native, async () => { writes++; }); + return { status: "completed" }; + } }; + const h = await harness(backend); + try { + h.runner.updateThread(h.thread.id, { approvals: "yolo", mode: "agent" }); + const sent = h.runner.send(h.thread.id, "go"); + await waitFor(() => ready); + h.runner.updateThread(h.thread.id, { mode: "chat" }); + release(); + await sent; + assert.equal(writes, 0); + } finally { release(); await h.runner.dispose(); } +}); + +test("image execution override retains the admitted thread binding and confirms consumption", async () => { + const home = tmpdir("modex-image-requests-"); + const store = new Store(home); + let consumed = false; + const native = { runtime: "offline-image", request: "approval" }; + const imageBackend: Backend = { id: "mock", listModels: async () => [], dispose: async () => {}, async runTurn(_t, _o, sink) { + const answer = await sink.approval({ question: "Create offline image?", canAlways: false, native }); + if (answer === "no") return { status: "interrupted" }; + await sink.deliverRequest?.(native, async () => { consumed = true; }); + return { status: "completed" }; + } }; + const runner = new ThreadRunner({ home, store, emit: () => {}, imageBackend }); + const thread = await runner.createImageThread(gitRepo(), "image-job", "claude", "cli"); + const sent = runner.send(thread.id, "generate"); + try { + await waitFor(() => runner.items(thread.id).some(i => i.kind === "approval")); + const card = runner.items(thread.id).find(i => i.kind === "approval")!; + assert.deepEqual(runner.answer(thread.id, card.id, "yes"), { ok: true }); + await sent; + assert.equal(consumed, true); + assert.equal((runner.items(thread.id).find(i => i.id === card.id) as Extract).delivery, "sent"); + } finally { await runner.dispose(); } +}); + +test("offline image denial is acknowledged without generating an image", async () => { + const { imageFixtureBackend } = await import("../src/main/engine/images/fixture.js"); + const home = tmpdir("modex-image-denial-"); + const store = new Store(home); + const runner = new ThreadRunner({ home, store, emit: () => {}, imageBackend: imageFixtureBackend }); + const thread = await runner.createImageThread(gitRepo(), "image-job", "claude", "cli"); + const sent = runner.send(thread.id, "generate"); + try { + await waitFor(() => runner.items(thread.id).some(i => i.kind === "approval")); + const card = runner.items(thread.id).find(i => i.kind === "approval")!; + assert.deepEqual(runner.answer(thread.id, card.id, "no"), { ok: true }); + await sent; + const result = runner.items(thread.id).find(i => i.id === card.id) as Extract; + assert.deepEqual([result.answer, result.delivery], ["no", "sent"]); + const fs = await import("node:fs"); + assert.equal(fs.readdirSync(thread.cwd).some(name => name.endsWith(".png")), false); + } finally { await runner.dispose(); } +}); diff --git a/apps/desktop/test/human-requests.test.ts b/apps/desktop/test/human-requests.test.ts index 77fadf85..70ce4f1e 100644 --- a/apps/desktop/test/human-requests.test.ts +++ b/apps/desktop/test/human-requests.test.ts @@ -35,7 +35,7 @@ test("a wrong thread, account, attempt or runtime, an unknown id or a changed ac const state = register(emptyRequests(), approval); const reason = (r: ReturnType) => (r.ok ? "ok" : r.reason); assert.equal(reason(resolve(state, "nope", yes, { binding, digest: approval.digest, by: "desktop" })), "unknown"); - for (const wrong of [{ thread: "T2" }, { attempt: "A2" }, { runtime: "R2" }, { account: "other" }, { nativeRequestId: "18" }] as Partial[]) { + for (const wrong of [{ thread: "T2" }, { turn: "U2" }, { backend: "claude" }, { item: "other" }, { attempt: "A2" }, { runtime: "R2" }, { account: "other" }, { nativeRequestId: "18" }] as Partial[]) { assert.equal(reason(resolve(state, "req-1", yes, { binding: { ...binding, ...wrong }, digest: approval.digest, by: "desktop" })), "wrong-identity", JSON.stringify(wrong)); } const changed = actionDigest({ ...action, input: { command: "npm test && rm -rf /" } }); @@ -112,3 +112,29 @@ test("a closed card says when the phone answered and when nobody can tell whethe assert.equal(answerNote({ answeredBy: "desktop", delivery: "uncertain" }), "Modex stopped before it could confirm the agent received this answer."); assert.equal(answerNote({ answeredBy: "phone", delivery: "uncertain" }), "Answered on your phone. Modex stopped before it could confirm the agent received this answer."); }); + +test("registered actions and identity are snapshots, not mutable caller references", () => { + const request = structuredClone(approval); + const state = register(emptyRequests(), request); + request.binding.runtime = "changed"; + (request.action as typeof action).input.command = "changed"; + assert.equal(state.requests[request.id]!.request.binding.runtime, binding.runtime); + assert.deepEqual((state.requests[request.id]!.request as typeof approval).action, action); +}); + +test("approval values and unsupported always decisions fail closed", () => { + const request = { ...approval, canAlways: false }; + const state = register(emptyRequests(), request); + for (const answer of ["always", "unexpected", null]) { + const result = resolve(state, request.id, { kind: "approval", answer } as never, { binding, digest: request.digest, by: "desktop" }); + assert.equal(result.ok, false); + if (!result.ok) assert.equal(result.reason, "invalid"); + } +}); + +test("Codex questions reject ambiguous ids and malformed sensitivity flags", () => { + const q = { id: "q", header: "Token", question: "Token?", isOther: true, isSecret: true, options: null }; + for (const questions of [[q, q], [{ ...q, isSecret: "true" }], [{ ...q, isOther: "false" }], [{ ...q, header: 7 }], [{ ...q, options: [{ label: "same" }, { label: "same" }] }]]) { + assert.equal(codexQuestions({ questions }), null); + } +}); diff --git a/apps/desktop/test/journal-resume.test.ts b/apps/desktop/test/journal-resume.test.ts index d441cb29..2b9e3dd1 100644 --- a/apps/desktop/test/journal-resume.test.ts +++ b/apps/desktop/test/journal-resume.test.ts @@ -264,8 +264,9 @@ test("Modex killed right after an approval is answered: pre-crash cursors reload const home = ${JSON.stringify(home)}, threadId = ${JSON.stringify(thread.id)}; const journal = new EventJournal(path.join(home, "app", "events.sqlite")); const backend = { id: "claude", listModels: async () => [], dispose: async () => {}, async runTurn(_text, _opts, sink) { - const answer = await sink.approval({ question: "Allow Bash: $ deploy?", canAlways: false }); - fs.appendFileSync(path.join(home, "decisions.log"), answer + "\\n"); + const native = { runtime: "crash", request: "deploy" }; + const answer = await sink.approval({ question: "Allow Bash: $ deploy?", canAlways: false, native }); + await sink.deliverRequest(native, async () => { fs.appendFileSync(path.join(home, "decisions.log"), answer + "\\n"); }); process.kill(process.pid, "SIGKILL"); return { status: "completed" }; } }; @@ -523,8 +524,9 @@ test("one mixed transcript converges on every path, survives hostile frames, and sink.observe({ type: "tool.start", native: at("t1"), tool: { name: "shell", title: "$ ls", args: { command: "ls" } } }); sink.observe({ type: "tool.done", native: at("t1"), outcome: "succeeded", progress: { output: "a\nb\n" } }); sink.observe({ type: "tool.update", native: at("t1"), progress: { output: "a late frame after the tool ended" } }); - const answer = await sink.approval({ question: "Allow Bash: $ rm -rf build?", canAlways: false }); - decisions.push(answer); + const native = { runtime: "attempt-1", request: "rm" }; + const answer = await sink.approval({ question: "Allow Bash: $ rm -rf build?", canAlways: false, native }); + await sink.deliverRequest!(native, async () => { decisions.push(answer); }); sink.observe({ type: "tool.start", native: at("t2"), tool: { name: "shell", title: "$ rm -rf build", args: { command: "rm -rf build" } } }); sink.observe({ type: "tool.done", native: at("t2"), outcome: answer === "yes" ? "succeeded" : "denied" }); sink.observe({ type: "message.start", native: at("m2"), phase: "final" }); diff --git a/apps/desktop/test/request-journal.test.ts b/apps/desktop/test/request-journal.test.ts index cc5bc61c..f52a1bb8 100644 --- a/apps/desktop/test/request-journal.test.ts +++ b/apps/desktop/test/request-journal.test.ts @@ -31,7 +31,9 @@ const lifecycle = (c: Card) => ({ answer: c.answer ?? null, answeredBy: c.answer /** A Claude-shaped backend whose turn asks one approval and records every decision it hears. */ function asking(heard: ApprovalAnswer[]): Backend { return { id: "claude", listModels: async () => [], dispose: async () => {}, async runTurn(_t, _o, sink: TurnSink) { - heard.push(await sink.approval({ question: "Allow Bash: $ deploy?", canAlways: false, action: { backend: "claude", tool: "Bash", title: "$ deploy", input: { command: "deploy" } } })); + const native = { runtime: "journal-fixture", request: "approval" }; + heard.push(await sink.approval({ native, question: "Allow Bash: $ deploy?", canAlways: false, action: { backend: "claude", tool: "Bash", title: "$ deploy", input: { command: "deploy" } } })); + await sink.deliverRequest?.(native, async () => {}); return { status: "completed" }; } }; } @@ -219,7 +221,7 @@ function codexAsking(proc: FakeProcess) { ] } }); } else if (msg.id === 7 && !msg.method) { replies.push(msg.result ?? { error: msg.error }); - proc.emitLine({ method: "turn/completed", params: { threadId: "thr", turn: { id: "tu", status: "completed", error: null } } }); + setImmediate(() => proc.emitLine({ method: "turn/completed", params: { threadId: "thr", turn: { id: "tu", status: "completed", error: null } } })); } } }); diff --git a/docs/human-requests.md b/docs/human-requests.md index e0e53cfc..f226b3d6 100644 --- a/docs/human-requests.md +++ b/docs/human-requests.md @@ -1,6 +1,6 @@ # Human requests -How a CLI's approvals and questions reach a person and how their answers go back, exactly once. This +How a CLI's approvals and questions reach a person and how one local answer is accepted and its delivery is recorded. This is the contract from [#276](https://github.com/TypeSafeAI/modex/issues/276) (CF-3 of tracker [#273](https://github.com/TypeSafeAI/modex/issues/273)). It builds on the conversation identity in [conversation-events.md](conversation-events.md) and on CF-2's withdrawn cards @@ -82,10 +82,14 @@ Refusals, in the order `resolve` checks them: | `changed-action` | The digest differs from the request's. | | `invalid` | The answer is the wrong kind, or the answers fail `validateAnswers`. | -`uncertain` means an answer was accepted but the request went away before the send was recorded, so -whether the CLI received the answer is unknown. It never reads as resolved. Today's backends hand an -accepted answer to the CLI's stdin in the same tick, so the runner records `sent` at once. The state -exists for transports where the hand-off can fail after acceptance. +`answer` and `answerQuestion` return `{ ok: true }` for local acceptance. The request stays +`resolving` until the backend's stdin write callback succeeds. The card then records +`delivery: "sent"`. This receipt proves a local transport handoff, not remote execution. + +If Stop, native withdrawal, an obsolete identity, changed approval rules, a write error, or a closed +input wins before that receipt, the card records `delivery: "uncertain"`. It stays inert and is never +resent automatically. After restart, saved `sending` cards become `uncertain`; saved `sent` and +`uncertain` receipts remain unchanged. Check the agent before starting a new request. ## Runner @@ -104,7 +108,12 @@ type AnswerResult = { ok: true } | { ok: false; reason: Refusal; error?: string - **Binding.** The sink builds each request's binding from the attempt's `TurnRef` and the backend's `NativeRequest` (`{ runtime, request, item? }`). Clients name a card by its item id. Modex mints that id for each request, so it stands for the binding. The runner supplies the binding and the - digest when it resolves. + digest when it resolves. Main snapshots the request and accepted answer, revalidates the current + backend, account, attempt, native identity, action, and approval-rule configuration at acceptance + and immediately before the write. A changed action, mode, or rule configuration invalidates its + card. Automatic decisions also revalidate the standing approval mode; changing a manual card + from Ask to YOLO retains its existing behavior. + Request IDs remain scoped to the runtime; Codex numeric and string IDs stay distinct. - **Questions.** `TurnSink.question(req, withdrawn?)` adds a `question` card. A blocking question sets the thread to `waiting`; a non-blocking one lets the turn run on. `answerQuestion` validates the answers and gives `error` text for an invalid one (for example "Token needs an answer."). @@ -112,13 +121,19 @@ type AnswerResult = { ok: true } | { ok: false; reason: Refusal; error?: string - **Withdrawal.** `withdraw(threadId, l, itemId, "native" | "stopped" | "ended")` closes a card once. An approval closes as `answer: "no"` plus `withdrawn`, so older clients show it closed. A question closes with `withdrawn` alone, and its sink settles with `{ withdrawn }`, never with empty - answers. + answers. A decision already accepted remains an inert `uncertain` receipt if withdrawal wins. - **Restart.** When a thread first loads in a new process, any approval without an answer and any question without answers or a withdrawal is closed as `withdrawn: "ended"`. The change is saved, and emitted as - `item_update`, so the journal and paired clients see it too. + `item_update`, so the journal and paired clients see it too. Interrupted `sending` receipts become + `uncertain` and never revive a send. - **Routines.** An unattended routine has nobody to ask. Approvals are refused with a receipt, as before. Questions are declined with a warning notice. +Each native request is handled once per turn. An identical replay retains the original card and +cancellation controller. Reusing an ID for a changed action withdraws the original request and requires +a new ID. Delivery is claimed synchronously before awaiting its write callback, so concurrent handlers +cannot write two answers for one native ID. This is a local guard, not an exactly-once remote guarantee. + ## Native mappings ### Codex (codex-cli 0.163.0, `app-server`) @@ -149,6 +164,10 @@ type AnswerResult = { ok: true } | { ok: false; reason: Refusal; error?: string - a free-text question gets a text field; - a secret question gets a password field. + Sending and uncertain cards show their delivery state and have no submit control. An uncertain + card does not read as answered. Secret input is sent with its original whitespace; validation only + checks that it is not empty. + A refusal is shown on the card. An answered card lists the answers, with a secret one shown as "(hidden)". A closed card says why, in the approval card's words. While a question waits, the turn header reads "Waiting for your answer" instead of "Waiting for approval". An answered or closed card @@ -156,7 +175,8 @@ type AnswerResult = { ok: true } | { ok: false; reason: Refusal; error?: string - **Phone.** The companion still answers approvals only. It refuses an answer to anything that is not an open approval, and a runner refusal becomes a 409. A question reaches the phone as an `info` notice that carries the question text and its state ("Answer it in Modex on your Mac.", "Answered on your - Mac.", "Not answered."), never the answers. + Mac.", "Not answered."), never the answers. Sending and uncertain receipts are explicit notices, + including for approvals, so older clients cannot mistake them for successful approvals. - **Store demo.** `thread:answer` returns `{ ok: true }`. The demo asks no questions, so `thread:answer-question` refuses with `unknown`. - **Older paired desktops.** A paired desktop names the item kinds it draws in an @@ -195,8 +215,9 @@ paired desktops and the phone carry it with no new event type. This is the reque - **Two clients at once.** The desktop and the phone race through one broker: one answer is accepted, the card names who gave it, and the other is refused, with a 409 for the phone. - **The phone** gets `answeredBy`, `delivery` and `withdrawn` on approvals. Today's iPhone build reads - only `answer`, so a withdrawn approval still reads as declined there. A question's notice says when - delivery is uncertain. + only `answer`, so an approval whose delivery is uncertain, or that was withdrawn, reaches it as an + `info` notice that says so. A decision still sending keeps its answer, so the phone shows it as + decided. A question's notice says when its answer is sending or its delivery is uncertain. - **Who answered** is the client that made the decision: this Mac's window (`desktop`), a desktop paired through the host (`paired-desktop`), or the phone. Switching a thread to YOLO answers what is waiting as whichever of them switched it. @@ -211,6 +232,20 @@ export. Tests check the saved store and the emitted events (`test/human-requests file under `MODEX_HOME` with the journal on and the phone's snapshot (`test/request-journal.test.ts`), and every file under `MODEX_HOME/app` with the journal off and on (`e2e/human-requests.spec.ts`). +## Repair verification + +The repair reproduces local acceptance followed by native withdrawal before the backend resumes. +It also covers Stop, changed backend/account/action/rules, immutable answers, duplicate native IDs, +failed write callbacks, real Writable errors, stdin close before a callback, policy receipts, and +restart rendering. `sent` is always a local write receipt. The offline backend acknowledges consuming +its scripted decision through the same callback boundary. + +Installed CLI versions checked on 2026-10-18: `codex-cli 0.163.0` and Claude Code `2.1.289`. +The schemas and scripted fixtures below are source/protocol evidence. They are not a live native +question acceptance run or human keyboard/VoiceOver acceptance. Those gates remain open in #276. +The preview approval gate remains off by default; Never rules, escalation rules, routine refusal, +Ask/Always/YOLO, and permission-profile validation keep their existing decisions. + ## Tests - `test/human-requests.test.ts` covers the pure broker: diff --git a/docs/reviews/2026-10-18-cf3-delivery.md b/docs/reviews/2026-10-18-cf3-delivery.md new file mode 100644 index 00000000..537aee75 --- /dev/null +++ b/docs/reviews/2026-10-18-cf3-delivery.md @@ -0,0 +1,67 @@ +# CF-3 answer delivery repair + +An accepted answer now stays `sending` until the backend reports its local write receipt. +Withdrawal, Stop, stale authority, or transport failure before that receipt produces an inert +`uncertain` card. Modex never retries that decision automatically. A successful write does not +prove remote execution. + +This repairs [#293](https://github.com/TypeSafeAI/modex/pull/293), for +[#276](https://github.com/TypeSafeAI/modex/issues/276). The repair branch is based on current main +`b804e9c6`, after #293, CF-6 request journal/reconnect, and CF-7 clarification landed. +It preserves their `sending` delivery state and `answeredBy` receipts. + +## Evidence by requirement + +| Requirement | Evidence | +| --- | --- | +| Acceptance is distinct from delivery | `human-requests-runner.test.ts`: acceptance followed synchronously by native withdrawal or Stop writes no answer and records uncertainty. Native approval receipts wait for the write callback. | +| Stale identity and actions cannot authorize work | Runner tests change backend, account, action, approval rules, standing automatic policy, and mode before handoff. Broker tests reject wrong thread, turn, attempt, backend, account, item, runtime, and native request ID. Requests and accepted answers are cloned. | +| One local answer and one write attempt | Broker duplicate-client tests; identical native replay retains its card/controller, changed replay withdraws it, and a held write callback cannot admit a second delivery. Both Codex and Claude ingress are covered. | +| Transport failures remain uncertain | Failed callback, actual Writable error, and close before callback fixtures. Restart E2E converts saved `sending` cards to inert `uncertain` cards. No reconnect or restart path resends the decision. | +| Sensitive answers stay private and unchanged | Question E2E sends a password value with surrounding whitespace unchanged, stores only `(hidden)`, and scans every file under the test home. Runner tests scan persisted items and emitted events. Companion tests omit question answers entirely. | +| Policies and compatibility remain intact | Existing Ask/Always/YOLO, explicit Never, escalation, routine refusal, and permission-profile tests remain the required regression gates. Manual Ask-to-YOLO resolution remains supported. Phones receive uncertain or withdrawn approvals as notices they can render without approving them. A decision still sending keeps its answer, so the phone shows it as decided, with no buttons (`companion.test.ts`, and the paired iPhone test's "Approved" step). The offline backend acknowledges local consumption explicitly. | + +The regression tests were observed failing before their corresponding repairs. These are protocol +fixtures and automated app tests. They are not human keyboard-only or VoiceOver acceptance. + +## Installed native schema + +On 2026-10-18, `codex --version` reported `codex-cli 0.163.0`; `claude --version` on this +session's PATH reported `2.1.289 (Claude Code)`. No live coding turn was run for this repair. +The earlier CF-2 live receipt for Claude Code 2.1.302 remains separate evidence. + +Both stable and experimental output from `codex app-server generate-json-schema` contain the +`item/tool/requestUserInput` schema, whose description marks it experimental. The generated +request has required `threadId`, `turnId`, `itemId`, `isBlocking`, and `questions`. Its response +requires `answers`. `serverRequest/resolved` requires `threadId` and `requestId`. + +Generated schema SHA-256 values: + +- Request: `fda15b62e7446105b59c8ed85c908abed2a47db70653e57c92c47a2f204fa77a`. +- Response: `5073b245f741fa8ce62ec52e4dd1809b7831f9c4284c279a78abc9b171f8e3f2`. +- Resolution notification: `4f03d586a5af04edd912cad08cc97eceb658e7e06744565e6d41a93e04a55966`. + +Schema generation establishes the installed protocol shape. Actual native question acceptance, +human accessibility acceptance and phone question entry remain open +in #276. The preview approval gate remains off by default. + +## Verification status + +Current-main verification: + +- `pnpm build`: passed. +- `pnpm typecheck`: passed across all workspace packages. +- Focused runner, broker, journal and companion tests: 61 passed. +- Full root `pnpm test`, with test files serialized through a local ignored Node launcher: + 1,789 passed, 16 skipped, no failures. No tests were filtered or timeouts changed. +- Full `MODEX_TEST_OPEN_KNOWLEDGE=1 pnpm test:e2e`: desktop 347 passed, 1 skipped, + 4 failed, plus an `afterAll` shutdown hook error. Site tests did not start in that command. +- Retried the complete app, approval and Markdown narrow specs on unchanged source: 19 passed. + This covers all four failed cases and their shutdown hooks. The full-run failures remain recorded; + the first app stall occurred before an approval existed, and process sampling showed a native + modal. Disk space repeatedly fell below 400 MB during the run. These observations do not prove + disk pressure caused every failure. +- Separate site E2E: 7 passed. + +The image override and denial regressions were both observed failing before repair. Independent +source review identified the denial receipt omission; the fixed case now passes. The preview gate remains off by default. No native question or human accessibility acceptance is claimed. diff --git a/docs/status.md b/docs/status.md index fc1c4863..910faaf0 100644 --- a/docs/status.md +++ b/docs/status.md @@ -189,6 +189,12 @@ and [landing verification](reviews/2026-10-11-landing-clarity.md). its own update banner, and uploading Companion 0.2.0 (7) to TestFlight, which carries thread archiving from the phone. [Review](reviews/2026-10-18-v0.2.0-release-review.md). +- **CF-3 human-request reconciliation (#276, #293).** Native question cards and one accepted + answer per request are under review. The repair separates local acceptance from the native + write receipt, keeps interrupted sends uncertain without retries, and preserves sensitive-answer + redaction. Fixture acceptance and live native acceptance remain separate gates. + [Human requests](human-requests.md). + - **Native conversation fidelity** ([tracker #273](https://github.com/TypeSafeAI/modex/issues/273)). Slices that landed are under "On main, unreleased". The tracker keeps the dependency order and each slice's evidence, and its child issues link their open PRs.