Skip to content

Commit 57e7607

Browse files
authored
Answer each approval and question once, and show Codex questions as cards (CF-3) (#293)
* Define the human-request contract and a pure broker with one resolution shared/human-requests.ts: approvals and native questions as one versioned contract. A request is bound to thread, turn, attempt, account, runtime and native request id plus a digest of the exact action; it accepts one local answer, and an unknown id, a wrong identity, a changed action, a withdrawn request or a second answer authorises nothing. Stop withdraws pending requests; one whose answer was being sent becomes uncertain, never resolved. Codex's experimental item/tool/requestUserInput (codex-cli 0.163.0) maps to typed questions, answers are validated against them, and secret answers are hidden from anything shown, saved or exported. Not wired in yet: that consumes #275's interfaces once #286 lands. Refs #276 Coven-Harness: Claude Code / claude-opus-5-5 * Answer each request once, route Codex questions to a card, and close stale cards The runner keeps the turn's approvals and questions in the human-request broker. The first answer is taken; every other one is refused with a reason (already resolved, withdrawn, unknown, invalid) that thread:answer now returns and the phone receives as a 409. Codex item/tool/requestUserInput (codex-cli 0.163.0, experimental) becomes a question card, answered through thread:answer-question. Answers are checked against the questions before anything reaches Codex. A secret answer goes to the CLI only and is saved as "(hidden)". Stop sends an error reply, never an empty answer. When serverRequest/resolved arrives for a request still open, Codex settled it, so the card closes as withdrawn and no answer is sent. Approvals now carry the CLI's own request id (Codex JSON-RPC id, Claude request_id). Cards a dead process left open close as ended when their thread loads, saved and emitted. Phones see a question as a notice to answer on the Mac. Part of #276. Coven-Harness: Claude Code / claude-opus-5-5 * Show question cards with a control for each question Options are radio buttons, with Other when the question takes free text. A free-text question gets a text field, and a secret one a password field. A refused answer says why on the card. An answered card lists the answers, with a secret shown as (hidden). A closed card says why, in the approval card's words. The turn header reads "Waiting for your answer" while a question waits. An answered or closed question folds into the turn's work; an open one never does. The e2e drives a fixture codex-cli 0.163.0 app-server through the real UI. It covers an incomplete answer refused in place, one reply carrying the answers, a refused second answer, Stop giving an error reply, and no secret anywhere under MODEX_HOME/app. Part of #276. Coven-Harness: Claude Code / claude-opus-5-5 * Document the human-request contract and record CF-3 in status docs/human-requests.md covers the broker's exported signatures, its states and refusals, the runner's binding and withdrawal rules, the Codex 0.163.0 and Claude 2.1.302 mappings, the clients, how secrets are kept, the tests, and what is left for CF-6 and CF-7. Part of #276. Coven-Harness: Claude Code / claude-opus-5-5 * Keep a new tab's late focus from taking the caret away Opening a tab (⌘⇧B) focuses its address bar twice: once when the tab becomes active, and again in an animation frame that newTab queues for when the panel was not yet visible. Chromium can hold frames back for an inactive or busy window. The late focus then landed after the person, or a test, had already moved on, and took the caret out of the composer mid-typing. That failed CI on #293: store-desktop.spec.ts's browser-tools recovery case filled the composer just as the late frame ran. The text went into the address bar and the composer stayed empty (job 117782462586). The same case failed on #294's first run. The late focus now applies only if focus has not moved since the tab opened. The new workspace.spec.ts case holds animation frames, moves to the composer, releases the frames and types. It failed first: the composer lost focus when the frame ran. Coven-Harness: Claude Code / claude-opus-5-5
1 parent a4039b2 commit 57e7607

25 files changed

Lines changed: 1228 additions & 35 deletions
Lines changed: 151 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,151 @@
1+
import { test, expect, type ElectronApplication, type Page } from "@playwright/test";
2+
import fs from "node:fs";
3+
import os from "node:os";
4+
import path from "node:path";
5+
import { launch, seedHome, tid, items } from "./support";
6+
7+
// Native questions through the real CodexBackend, runner, IPC and UI (#276). The local executable below speaks
8+
// codex-cli 0.163.0's app-server protocol for `item/tool/requestUserInput` (experimental) the way its schema
9+
// describes it: questions with ids, headers, options, `isOther` and `isSecret`, answered with `{ answers }`.
10+
// It is a scripted fixture, not live-model evidence. Its log lives outside MODEX_HOME so the secret check
11+
// below can read every file Modex wrote.
12+
13+
function fixture(dir: string): string {
14+
const cli = path.join(dir, "codex-questions.cjs");
15+
fs.writeFileSync(cli, `#!${process.execPath}
16+
const fs = require('node:fs');
17+
if (process.argv.includes('--version')) { console.log('codex-cli 0.163.0'); process.exit(0); }
18+
if (process.argv.includes('login') || process.argv.includes('auth')) { console.log('Logged in using ChatGPT'); process.exit(0); }
19+
const log = ${JSON.stringify(path.join(dir, "codex-replies.jsonl"))};
20+
const emit = (o) => process.stdout.write(JSON.stringify(o) + '\\n');
21+
const questions = [
22+
{ id: 'q-env', header: 'Environment', question: 'Which environment?', isOther: false, isSecret: false, options: [{ label: 'staging', description: 'Safe to try' }, { label: 'production', description: 'Live traffic' }] },
23+
{ id: 'q-token', header: 'Token', question: 'Paste the deploy token', isOther: true, isSecret: true, options: null },
24+
];
25+
let turn = 0, asked = 0, open = null;
26+
const cwds = new Map();
27+
require('node:readline').createInterface({ input: process.stdin }).on('line', (line) => {
28+
const msg = JSON.parse(line);
29+
const p = msg.params || {};
30+
if (msg.method === 'initialize') return emit({ id: msg.id, result: { userAgent: 'codex-questions/0.163.0' } });
31+
if (msg.method === 'initialized') return;
32+
if (msg.method === 'account/read') return emit({ id: msg.id, result: { account: { type: 'chatgpt' }, requiresOpenaiAuth: true } });
33+
if (msg.method === 'model/list') return emit({ id: msg.id, result: { data: [{ id: 'fixture', model: 'fixture', displayName: 'Fixture', isDefault: true }], nextCursor: null } });
34+
if (msg.method === 'thread/start' || msg.method === 'thread/resume') {
35+
const id = p.threadId || 'thr-' + cwds.size;
36+
cwds.set(id, p.cwd || '');
37+
return emit({ id: msg.id, result: { thread: { id } } });
38+
}
39+
if (msg.method === 'turn/start' && /modex-title-/.test(cwds.get(p.threadId) || '')) {
40+
// Thread naming runs in its own folder and asks nothing.
41+
const id = 'title-' + (++turn);
42+
emit({ id: msg.id, result: { turn: { id } } });
43+
emit({ method: 'item/completed', params: { threadId: p.threadId, turnId: id, item: { id: 'name-' + turn, type: 'agentMessage', text: 'Deploy', phase: 'final_answer' } } });
44+
emit({ method: 'turn/completed', params: { threadId: p.threadId, turn: { id, status: 'completed', error: null } } });
45+
return;
46+
}
47+
if (msg.method === 'turn/start') {
48+
const id = 'turn-' + (++turn);
49+
open = { thread: p.threadId, turn: id, request: 900 + (++asked), done: false };
50+
emit({ id: msg.id, result: { turn: { id } } });
51+
emit({ method: 'turn/started', params: { threadId: p.threadId, turn: { id } } });
52+
emit({ id: open.request, method: 'item/tool/requestUserInput', params: { threadId: p.threadId, turnId: id, itemId: 'ask-' + asked, isBlocking: true, autoResolutionMs: null, questions } });
53+
return;
54+
}
55+
const finish = (status, text) => {
56+
if (!open || open.done) return;
57+
open.done = true;
58+
if (text) emit({ method: 'item/completed', params: { threadId: open.thread, turnId: open.turn, item: { id: 'say-' + turn, type: 'agentMessage', text, phase: 'final_answer' } } });
59+
emit({ method: 'turn/completed', params: { threadId: open.thread, turn: { id: open.turn, status, error: null } } });
60+
};
61+
if (msg.method === 'turn/interrupt') { emit({ id: msg.id, result: {} }); return finish('interrupted'); }
62+
if (open && msg.id === open.request && !msg.method) {
63+
fs.appendFileSync(log, JSON.stringify({ request: msg.id, result: msg.result, error: msg.error }) + '\\n');
64+
if (msg.result) finish('completed', 'Deploying to ' + msg.result.answers['q-env'].answers[0] + '.');
65+
return;
66+
}
67+
if (msg.id !== undefined && msg.method) emit({ id: msg.id, error: { code: -32601, message: 'fixture does not implement ' + msg.method } });
68+
});
69+
`, { mode: 0o755 });
70+
return cli;
71+
}
72+
73+
type Reply = { request: number; result?: { answers: Record<string, { answers: string[] }> }; error?: { code: number; message: string } };
74+
const replies = (dir: string): Reply[] => {
75+
const file = path.join(dir, "codex-replies.jsonl");
76+
return fs.existsSync(file) ? fs.readFileSync(file, "utf8").trim().split("\n").filter(Boolean).map((line) => JSON.parse(line) as Reply) : [];
77+
};
78+
/** Every file under `dir`, read as text. */
79+
const everything = (dir: string): string => fs.readdirSync(dir, { recursive: true, withFileTypes: true })
80+
.filter((entry) => entry.isFile()).map((entry) => fs.readFileSync(path.join(entry.parentPath, entry.name), "utf8")).join("\n");
81+
82+
async function send(page: Page, text: string): Promise<void> {
83+
await page.keyboard.press("Meta+n");
84+
await expect(tid(page, "draft-view")).toBeVisible();
85+
await tid(page, "composer-input").fill(text);
86+
await tid(page, "send").click();
87+
}
88+
const currentRow = (page: Page) => page.locator('[data-testid="thread-row"][aria-current="true"]');
89+
90+
test("Codex questions: the card answers by its schema, the CLI hears once, the secret stays out, Stop withdraws", async () => {
91+
const { home } = seedHome({ default_backend: "codex", default_mode: "agent" });
92+
const logs = fs.mkdtempSync(path.join(os.tmpdir(), "modex-e2e-codex-questions-"));
93+
const statePath = path.join(home, "app/state.json");
94+
const state = JSON.parse(fs.readFileSync(statePath, "utf8"));
95+
state.settings.codex_bin = fixture(logs);
96+
fs.writeFileSync(statePath, JSON.stringify(state));
97+
let app: ElectronApplication | undefined;
98+
try {
99+
const opened = await launch(home); app = opened.app;
100+
const page = opened.page;
101+
102+
// 1. The card shows each question with its own control; an incomplete answer is refused in place.
103+
await send(page, "deploy it");
104+
const card = items(page, "question").first();
105+
await expect(tid(card, "question")).toHaveCount(2);
106+
await expect(currentRow(page)).toHaveAttribute("data-status", "waiting");
107+
await expect(tid(page, "turn-label")).toHaveText(/^Waiting for your answer · \d+s$/);
108+
await card.getByRole("radio", { name: "staging" }).check();
109+
await tid(card, "question-submit").click();
110+
await expect(tid(card, "question-error")).toHaveText("Token needs an answer.");
111+
expect(replies(logs)).toEqual([]);
112+
const token = card.getByLabel("Token");
113+
await expect(token).toHaveAttribute("type", "password");
114+
await token.fill("s3cr3t-token");
115+
await page.screenshot({ path: test.info().outputPath("codex-question-card.png") });
116+
await tid(card, "question-submit").click();
117+
118+
// 2. Codex hears the answers once; the card shows them with the secret hidden; the turn carries on.
119+
await expect(tid(card, "question-answer")).toHaveText(["staging", "(hidden)"]);
120+
await expect(items(page, "assistant").last()).toContainText("Deploying to staging.");
121+
await expect(currentRow(page)).toHaveAttribute("data-status", "idle");
122+
expect(replies(logs)).toEqual([{ request: 901, result: { answers: { "q-env": { answers: ["staging"] }, "q-token": { answers: ["s3cr3t-token"] } } } }]);
123+
// A second answer (an old window, or a double click) is refused and reaches nothing.
124+
const threadId = (await currentRow(page).getAttribute("data-thread-id"))!;
125+
const again = await page.evaluate(async (id) => {
126+
const list = await window.modex!.invoke("thread:items", { threadId: id }) as { id: string; kind: string }[];
127+
const asked = list.find((i) => i.kind === "question")!;
128+
return window.modex!.invoke("thread:answer-question", { threadId: id, itemId: asked.id, answers: { "q-env": ["production"], "q-token": ["other"] } });
129+
}, threadId);
130+
expect(again).toEqual({ ok: false, reason: "already-resolved" });
131+
expect(replies(logs)).toHaveLength(1);
132+
await page.screenshot({ path: test.info().outputPath("codex-question-answered.png") });
133+
134+
// 3. Stop while a question waits: the card closes as stopped, and Codex gets an error, never an empty answer.
135+
await send(page, "deploy again");
136+
const second = items(page, "question").first();
137+
await expect(tid(second, "question-submit")).toBeVisible();
138+
await tid(page, "stop").click();
139+
await expect(tid(second, "question-closed")).toHaveText("Not answered — the turn was stopped");
140+
await expect(tid(second, "question-submit")).toHaveCount(0);
141+
await expect(currentRow(page)).toHaveAttribute("data-status", "idle");
142+
await expect.poll(() => replies(logs).length).toBe(2);
143+
expect(replies(logs)[1]).toMatchObject({ request: 902, error: { code: -32000 } });
144+
145+
// Nothing Modex saved (threads, journal, state) holds the secret.
146+
expect(everything(path.join(home, "app")).includes("s3cr3t-token")).toBe(false);
147+
} finally {
148+
await app?.close();
149+
fs.rmSync(logs, { recursive: true, force: true });
150+
}
151+
});

‎apps/desktop/e2e/workspace.spec.ts‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -182,6 +182,27 @@ test("a delayed browser snapshot cannot replace an address being edited", async
182182
await expect(input).toHaveValue("https://example.com/fast");
183183
});
184184

185+
test("a new tab's late focus never takes the caret from where you started typing", async () => {
186+
// Hold animation frames, as Chromium does for an inactive or busy window.
187+
await page.evaluate(() => {
188+
const w = window as unknown as { __frames: FrameRequestCallback[]; __raf: typeof requestAnimationFrame };
189+
w.__frames = []; w.__raf = window.requestAnimationFrame;
190+
window.requestAnimationFrame = (callback) => { w.__frames.push(callback); return w.__frames.length; };
191+
});
192+
await page.keyboard.press("Meta+Shift+b");
193+
await expect(tid(page, "workspace-address")).toBeFocused();
194+
await tid(page, "composer-input").focus();
195+
await page.evaluate(() => {
196+
const w = window as unknown as { __frames: FrameRequestCallback[]; __raf: typeof requestAnimationFrame };
197+
window.requestAnimationFrame = w.__raf;
198+
for (const callback of w.__frames.splice(0)) callback(performance.now());
199+
});
200+
await expect(tid(page, "composer-input")).toBeFocused();
201+
await page.keyboard.type("Keep this draft");
202+
await expect(tid(page, "composer-input")).toHaveValue("Keep this draft");
203+
await expect(tid(page, "workspace-address")).toHaveValue("");
204+
});
205+
185206
test("superseded navigation, overlays, guest shortcuts and window destruction preserve browser lifecycle", async () => {
186207
const { createServer } = await import("node:http");
187208
const server = createServer((req, res) => { if (req.url === "/slow") return; res.writeHead(200, { "Content-Type": "text/html" }); res.end("<title>Fast destination</title><h1>Ready</h1>"); });

‎apps/desktop/src/main/commands.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,7 @@ export function registerCommands(handle: RegisterCommand, {
9797
handle("thread:retry", ({ threadId }) => startTurn(threadId, runner.retry(threadId)));
9898
handle("thread:stop", ({ threadId }) => runner.stop(threadId));
9999
handle("thread:answer", ({ threadId, itemId, answer }) => runner.answer(threadId, itemId, answer));
100+
handle("thread:answer-question", ({ threadId, itemId, answers }) => runner.answerQuestion(threadId, itemId, answers));
100101
handle("thread:update", ({ threadId, patch }) => runner.updateThread(threadId, patch));
101102
handle("thread:archive", ({ threadId, archived }) => runner.setArchived(threadId, archived));
102103
handle("thread:delete", async ({ threadId, removeWorktree }) => {

‎apps/desktop/src/main/engine/backends/claude.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -550,6 +550,7 @@ export class ClaudeBackend implements Backend {
550550
detail: shortJson(req.input ?? {}),
551551
canAlways: Boolean(req.permission_suggestions?.length),
552552
action: { backend: "claude", tool, title, cwd: ctx.cwd, input: req.input ?? {} },
553+
...(msg.request_id ? { native: { runtime: ctx.native("").runtime, request: msg.request_id, ...(req.tool_use_id ? { item: req.tool_use_id } : {}) } } : {}),
553554
};
554555
// The CLI may withdraw the prompt (`control_cancel_request`) while it waits: the sink closes its card,
555556
// this request is done, and no answer is sent, since the CLI ignores one for a request it cancelled.

‎apps/desktop/src/main/engine/backends/codex.ts‎

Lines changed: 44 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,14 @@ import type { CliExecutable } from "../cli-path.js";
66
import { generateTitle } from "../titles.js";
77
import type { ChildProcess } from "node:child_process";
88
import { randomUUID } from "node:crypto";
9-
import type { Backend, ModelInfo, TitleOptions, TurnOptions, TurnResult, TurnSink } from "./types.js";
9+
import type { Backend, ModelInfo, NativeRequest, TitleOptions, TurnOptions, TurnResult, TurnSink } from "./types.js";
1010
import { LineBuffer, stderrTail } from "./types.js";
1111
import { codexCall, UsageMeter, withUsage } from "./usage.js";
1212
import { cliEnvironment, cliSpawn, health, installation } from "./health.js";
1313
import type { AgentActivity, BackendHealth } from "../../../shared/types.js";
1414
import type { MessagePhase, NativeRef, PlanStepStatus, SettledOutcome } from "../../../shared/conversation-events.js";
1515
import { boundOutput, byteLength, memoryEvidence } from "../../../shared/evidence.js";
16+
import { codexAnswerResponse, codexQuestions } from "../../../shared/human-requests.js";
1617

1718
/**
1819
* Codex's own wording when the sign-in it loaded at startup no longer matches ~/.codex/auth.json
@@ -44,6 +45,11 @@ export class CodexBackend implements Backend {
4445
private userAgent: string | null = null;
4546
/** Opaque id of the running app-server process; native thread, turn and item ids are unique only within it. */
4647
private generation = "";
48+
/**
49+
* Server requests waiting on a person, keyed `generation:id`. Codex's `serverRequest/resolved` for one still
50+
* here means Codex settled it itself (codex-cli 0.163.0): the card is withdrawn and no answer is sent.
51+
*/
52+
private readonly openRequests = new Map<string, AbortController>();
4753

4854
private get bin(): string { return typeof this.executable === "function" ? this.executable() : this.executable; }
4955

@@ -344,6 +350,10 @@ export class CodexBackend implements Backend {
344350
}
345351
return;
346352
}
353+
if (msg.method === "serverRequest/resolved") {
354+
this.openRequests.get(`${runtime}:${String(p.requestId)}`)?.abort();
355+
return;
356+
}
347357
if (p.threadId && p.threadId !== tid) {
348358
const id = String(p.threadId);
349359
if (!agents.has(id)) return;
@@ -538,7 +548,19 @@ export class CodexBackend implements Backend {
538548
}
539549

540550
private async handleServerRequest(msg: RpcMessage, sink: TurnSink): Promise<void> {
551+
const key = `${this.generation}:${String(msg.id)}`;
552+
const withdrawn = new AbortController();
553+
this.openRequests.set(key, withdrawn);
554+
try {
555+
await this.answerServerRequest(msg, sink, withdrawn.signal);
556+
} finally {
557+
if (this.openRequests.get(key) === withdrawn) this.openRequests.delete(key);
558+
}
559+
}
560+
561+
private async answerServerRequest(msg: RpcMessage, sink: TurnSink, withdrawn: AbortSignal): Promise<void> {
541562
const p = (msg.params ?? {}) as Record<string, unknown>;
563+
const native: NativeRequest = { runtime: this.generation, request: String(msg.id), ...(typeof p.itemId === "string" && p.itemId ? { item: p.itemId } : {}) };
542564
if (msg.method === "item/commandExecution/requestApproval") {
543565
const cmd = stripShell(String(p.command ?? ""));
544566
const cwd = typeof p.cwd === "string" && p.cwd ? p.cwd : undefined;
@@ -547,7 +569,9 @@ export class CodexBackend implements Backend {
547569
detail: [p.reason ? `Reason: ${String(p.reason)}` : "", `cwd: ${String(p.cwd ?? "")}`].filter(Boolean).join("\n"),
548570
canAlways: true,
549571
action: { backend: "codex", tool: "command", title: cmd, ...(cwd ? { cwd } : {}), input: { command: cmd } },
550-
});
572+
native,
573+
}, withdrawn);
574+
if (withdrawn.aborted) return;
551575
this.respond(msg.id, { decision: answer === "yes" ? "accept" : answer === "always" ? "acceptForSession" : "decline" });
552576
} else if (msg.method === "item/fileChange/requestApproval") {
553577
const grantRoot = typeof p.grantRoot === "string" && p.grantRoot ? p.grantRoot : undefined;
@@ -558,7 +582,9 @@ export class CodexBackend implements Backend {
558582
canAlways: true,
559583
// Paths only: the patch body stays out of the action, so no gate can forward it.
560584
action: { backend: "codex", tool: "fileChange", title: "apply file changes", input: { ...(paths.length ? { paths } : {}), ...(grantRoot ? { grantRoot } : {}) }, ...(grantRoot ? { escalation: true } : {}) },
561-
});
585+
native,
586+
}, withdrawn);
587+
if (withdrawn.aborted) return;
562588
this.respond(msg.id, { decision: answer === "yes" ? "accept" : answer === "always" ? "acceptForSession" : "decline" });
563589
} else if (msg.method === "item/permissions/requestApproval") {
564590
const permissions = requestedPermissions(p.permissions);
@@ -567,8 +593,22 @@ export class CodexBackend implements Backend {
567593
this.respond(msg.id, { permissions: {}, scope: "turn" });
568594
return;
569595
}
570-
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 } });
596+
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);
597+
if (withdrawn.aborted) return;
571598
this.respond(msg.id, { permissions: answer === "yes" ? permissions : {}, scope: "turn" });
599+
} else if (msg.method === "item/tool/requestUserInput" && sink.question) {
600+
// Experimental in codex-cli 0.163.0. A request Modex cannot read is declined whole, never half-answered.
601+
const asked = codexQuestions(p);
602+
if (!asked) {
603+
sink.notice("warn", "Codex asked a question Modex could not read; it was declined.");
604+
this.send({ id: msg.id, error: { code: -32602, message: "modex could not read this requestUserInput" } });
605+
return;
606+
}
607+
const reply = await sink.question({ ...asked, native }, withdrawn);
608+
if (withdrawn.aborted) return;
609+
// A withdrawn question is an error reply, never an empty answer the model could take as the user's.
610+
if ("answers" in reply) this.respond(msg.id, codexAnswerResponse(reply.answers));
611+
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" } });
572612
} else {
573613
sink.notice("warn", `Codex asked for ${msg.method}, which Modex does not support yet; declined.`);
574614
this.send({ id: msg.id, error: { code: -32601, message: `unsupported by modex: ${msg.method}` } });

0 commit comments

Comments
 (0)