Skip to content

Commit d625eda

Browse files
authored
Carry each request's answer, who gave it and its delivery through the journal, replay and the phone (CF-6, request half) (#308)
* Record who answered each request and whether the answer reached the CLI A card is the record of its request across the journal, snapshots, paired desktops and the phone (CF-6, request half). An accepted answer now saves answeredBy ("desktop" or "phone") and delivery "sending" before the CLI hears anything, then delivery "sent" after the hand-off. They travel as whole field values in the existing item events, so the journal stores and replays them with no new event type. On a restart, a card still "sending" becomes "uncertain": Modex stopped between taking the answer and recording its hand-off. The answer is never sent again, and a late answer is refused. The phone gets answeredBy, delivery and withdrawn on approvals, and a question's notice says when delivery is uncertain. test/request-journal.test.ts failed first on each missing field: replay once and twice, a restart, desktop and phone answering at once (one accepted, the phone's a 409), a paired desktop resuming after the decision, an expired cursor, a crash mid-send, and a secret kept out of every file with the journal on. Refs #279 Coven-Harness: Claude Code / claude-opus-5-5 * Say on a card when the phone answered or delivery is uncertain answerNote adds "Answered on your phone." under a card the phone answered, and "Modex stopped before it could confirm the agent received this answer." when delivery is uncertain. An answer given in the window and sent adds nothing. Its unit test failed first (no such export). e2e/human-requests.spec.ts runs the Codex question fixture with the journal on, closes Modex, marks the answered card as if Modex died mid-send, and restarts: the answered and stopped cards are inert, the uncertain one says so, a second answer is refused, Codex hears nothing more, and no file holds the secret. Refs #279 Coven-Harness: Claude Code / claude-opus-5-5 * Document the request lifecycle across replay and reconnect docs/human-requests.md maps each broker step to the card fields that carry it, and says what a restart, replay, a late answer, two clients at once, the phone and the window do with them. docs/event-journal.md points there instead of saying the lifecycle is still to come. Refs #279 Coven-Harness: Claude Code / claude-opus-5-5 * Let the phone act on threads started on the Mac Since #181 (v0.0.12), a thread created from a Mac draft keeps the full UUID its draft reserved, but the companion's thread routes accepted only 8 hex characters. So the phone got a 404 when it answered an approval, sent a follow-up, set the approval policy or archived any thread started on the Mac; threads started on the phone, with short ids, worked. CF-8's fidelity e2e found it. The routes now take any id the store accepts, and the thread must still exist. The new companion test failed first with 404 for a UUID thread. Refs #279 Coven-Harness: Claude Code / claude-opus-5-5 * Record CF-6's request half and the phone fix in status Refs #279 Coven-Harness: Claude Code / claude-opus-5-5
1 parent 81798d7 commit d625eda

14 files changed

Lines changed: 475 additions & 22 deletions

File tree

‎apps/desktop/e2e/human-requests.spec.ts‎

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -149,3 +149,71 @@ test("Codex questions: the card answers by its schema, the CLI hears once, the s
149149
fs.rmSync(logs, { recursive: true, force: true });
150150
}
151151
});
152+
153+
test("with the journal on, a restart shows answered and stopped cards inert, says when an answer was never confirmed, and re-sends nothing", async () => {
154+
const { home } = seedHome({ default_backend: "codex", default_mode: "agent" });
155+
const logs = fs.mkdtempSync(path.join(os.tmpdir(), "modex-e2e-codex-questions-"));
156+
const statePath = path.join(home, "app/state.json");
157+
const state = JSON.parse(fs.readFileSync(statePath, "utf8"));
158+
state.settings.codex_bin = fixture(logs);
159+
fs.writeFileSync(statePath, JSON.stringify(state));
160+
const journal = process.env.MODEX_EVENT_JOURNAL;
161+
process.env.MODEX_EVENT_JOURNAL = "1";
162+
let app: ElectronApplication | undefined;
163+
try {
164+
let opened = await launch(home); app = opened.app;
165+
let page = opened.page;
166+
await send(page, "deploy it");
167+
const card = items(page, "question").first();
168+
await card.getByRole("radio", { name: "staging" }).check();
169+
await card.getByLabel("Token").fill("s3cr3t-token");
170+
await tid(card, "question-submit").click();
171+
await expect(tid(card, "question-answer")).toHaveText(["staging", "(hidden)"]);
172+
await expect(currentRow(page)).toHaveAttribute("data-status", "idle");
173+
await expect(tid(card, "answer-note")).toHaveCount(0);
174+
// A follow-up in the same thread asks again; Stop closes that card unanswered.
175+
await tid(page, "composer-input").fill("deploy again");
176+
await page.keyboard.press("Meta+Enter");
177+
await expect(tid(items(page, "question").nth(1), "question-submit")).toBeVisible();
178+
await tid(page, "stop").click();
179+
await expect(currentRow(page)).toHaveAttribute("data-status", "idle");
180+
const threadId = (await currentRow(page).getAttribute("data-thread-id"))!;
181+
const lifecycle = (list: { kind: string; answeredBy?: string; delivery?: string; withdrawn?: string }[]) =>
182+
list.filter((i) => i.kind === "question").map((i) => [i.answeredBy ?? null, i.delivery ?? null, i.withdrawn ?? null]);
183+
expect(lifecycle(await page.evaluate((id) => window.modex!.invoke("thread:items", { threadId: id }), threadId) as never)).toEqual([["desktop", "sent", null], [null, null, "stopped"]]);
184+
await app.close();
185+
app = undefined;
186+
expect(replies(logs)).toHaveLength(2);
187+
188+
// As if Modex had died after taking the first answer and before recording its hand-off to Codex.
189+
const file = path.join(home, "app", "threads", `${threadId}.json`);
190+
const saved = JSON.parse(fs.readFileSync(file, "utf8")) as { kind: string; delivery?: string }[];
191+
saved.find((i) => i.kind === "question")!.delivery = "sending";
192+
fs.writeFileSync(file, JSON.stringify(saved));
193+
194+
opened = await launch(home); app = opened.app;
195+
page = opened.page;
196+
await page.locator(`[data-testid="thread-row"][data-thread-id="${threadId}"] button.thread-main`).click();
197+
const [first, second] = [items(page, "question").first(), items(page, "question").nth(1)];
198+
await expect(tid(first, "question-answer")).toHaveText(["staging", "(hidden)"]);
199+
await expect(tid(first, "answer-note")).toHaveText("Modex stopped before it could confirm the agent received this answer.");
200+
await expect(tid(second, "question-closed")).toHaveText("Not answered — the turn was stopped");
201+
await expect(page.locator('[data-testid="question-submit"]')).toHaveCount(0);
202+
expect(lifecycle(await page.evaluate((id) => window.modex!.invoke("thread:items", { threadId: id }), threadId) as never)).toEqual([["desktop", "uncertain", null], [null, null, "stopped"]]);
203+
// An old window answering the uncertain card again is refused, and nothing reaches Codex.
204+
expect(await page.evaluate(async (id) => {
205+
const list = await window.modex!.invoke("thread:items", { threadId: id }) as { id: string; kind: string }[];
206+
return window.modex!.invoke("thread:answer-question", { threadId: id, itemId: list.find((i) => i.kind === "question")!.id, answers: { "q-env": ["production"], "q-token": ["other"] } });
207+
}, threadId)).toEqual({ ok: false, reason: "already-resolved" });
208+
expect(replies(logs)).toHaveLength(2);
209+
await app.close();
210+
app = undefined;
211+
// The journal recorded all of it, and no file Modex wrote holds the secret.
212+
expect(fs.existsSync(path.join(home, "app", "events.sqlite"))).toBe(true);
213+
expect(everything(path.join(home, "app")).includes("s3cr3t-token")).toBe(false);
214+
} finally {
215+
if (journal === undefined) delete process.env.MODEX_EVENT_JOURNAL; else process.env.MODEX_EVENT_JOURNAL = journal;
216+
await app?.close();
217+
fs.rmSync(logs, { recursive: true, force: true });
218+
}
219+
});

‎apps/desktop/src/main/engine/companion.ts‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -308,7 +308,9 @@ export class CompanionServer {
308308
this.reply(res, 201, { thread: mobileThread(thread, this.source.status(thread.id), this.source.pullRequest?.(thread.id)) });
309309
return;
310310
}
311-
const match = /^\/v1\/threads\/([a-f0-9]{8})\/(send|answer|policy|archive)$/.exec(url.pathname);
311+
// Any id the store accepts: 8 hex characters for threads made here or by older builds, the full UUID a
312+
// Mac draft reserved (#181). The thread must still exist.
313+
const match = /^\/v1\/threads\/([A-Za-z0-9_-]{1,128})\/(send|answer|policy|archive)$/.exec(url.pathname);
312314
if (req.method !== "POST" || !match) { this.reply(res, 404, { error: "Not found." }); return; }
313315
const threadId = match[1]!;
314316
const action = match[2]!;
@@ -385,7 +387,9 @@ export function mobileItem(item: ThreadItem): Record<string, unknown> {
385387
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 } : {}) };
386388
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 } : {}) };
387389
case "user": case "notice": return { id: item.id, kind: item.kind, text: item.text, at: item.at, ...(item.kind === "notice" ? { level: item.level } : {}) };
388-
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" } : {}), at: item.at };
390+
// Who answered, whether it reached the CLI and why an unanswered card closed ride along the same way.
391+
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" } : {}),
392+
...(item.answeredBy ? { answeredBy: item.answeredBy } : {}), ...(item.delivery ? { delivery: item.delivery } : {}), ...(item.withdrawn ? { withdrawn: item.withdrawn } : {}), at: item.at };
389393
// A phone that reads only title and status shows every finished tool as done, so a finished tool
390394
// that did not succeed says how it ended in its title.
391395
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,
@@ -395,7 +399,7 @@ export function mobileItem(item: ThreadItem): Record<string, unknown> {
395399
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 };
396400
// Phones cannot answer questions yet: a notice every client can show, with the questions and never the answers.
397401
case "question": return { id: item.id, kind: "notice", level: "info", at: item.at,
398-
text: `The agent asked: ${item.questions.map((q) => q.text).join(" · ")}\n${item.answers ? "Answered on your Mac." : item.withdrawn ? "Not answered." : "Answer it in Modex on your Mac."}` };
402+
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."}` };
399403
}
400404
}
401405

‎apps/desktop/src/main/engine/runner.ts‎

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -217,6 +217,9 @@ export class ThreadRunner {
217217
// A card nobody answered: the process that could take the answer is gone, so it closes as ended.
218218
if (item.kind === "approval" && !item.answer) return { ...item, answer: "no", withdrawn: "ended" };
219219
if (item.kind === "question" && !item.answers && !item.withdrawn) return { ...item, withdrawn: "ended" };
220+
// An answer taken but not recorded as handed to the CLI: nobody can tell whether the CLI got it, and it is
221+
// never sent again.
222+
if ((item.kind === "approval" || item.kind === "question") && item.delivery === "sending") return { ...item, delivery: "uncertain" };
220223
if (item.kind !== "tool" || !item.agent || !["running", "waiting"].includes(item.agent.state)) return item;
221224
return { ...item, status: "done", ok: undefined, agent: { ...item.agent, state: "unknown", detail: "Modex restarted without a final agent status" } };
222225
});
@@ -931,10 +934,10 @@ export class ThreadRunner {
931934
const pending = l.pending.get(itemId);
932935
if (!accepted.ok || pending?.kind !== "approval") return accepted.ok ? { ok: false, reason: "invalid" } : accepted;
933936
l.pending.delete(itemId);
934-
this.patchItem(threadId, itemId, { answer });
937+
this.patchItem(threadId, itemId, { answer, answeredBy: by, delivery: "sending" });
935938
if (l.pending.size === 0 && l.status === "waiting") this.setStatus(threadId, "running");
936939
pending.settle(answer);
937-
l.requests = settleRequest(l.requests, itemId, "sent");
940+
this.sent(threadId, l, itemId);
938941
return { ok: true };
939942
}
940943

@@ -953,13 +956,19 @@ export class ThreadRunner {
953956
if (!accepted.ok || pending?.kind !== "question") return accepted.ok ? { ok: false, reason: "invalid" } : accepted;
954957
const given = answers as Record<string, string[]>;
955958
l.pending.delete(itemId);
956-
this.patchItem(threadId, itemId, { answers: publicAnswers(pending.questions, given) });
959+
this.patchItem(threadId, itemId, { answers: publicAnswers(pending.questions, given), answeredBy: by, delivery: "sending" });
957960
if (l.pending.size === 0 && l.status === "waiting") this.setStatus(threadId, "running");
958961
pending.settle({ answers: given });
959-
l.requests = settleRequest(l.requests, itemId, "sent");
962+
this.sent(threadId, l, itemId);
960963
return { ok: true };
961964
}
962965

966+
/** Records that an accepted answer was handed to the CLI, in the broker and on the card. */
967+
private sent(threadId: string, l: Live, itemId: string): void {
968+
l.requests = settleRequest(l.requests, itemId, "sent");
969+
this.patchItem(threadId, itemId, { delivery: "sent" });
970+
}
971+
963972
/** Takes one answer for a waiting request through the broker, or says why not. */
964973
private accept(l: Live, itemId: string, resolution: Resolution, by: string): AnswerResult {
965974
const record = l.requests.requests[itemId];

‎apps/desktop/src/renderer/components/ThreadView.tsx‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import type { AnswerResult, ApprovalPolicy, ApprovalReceipt, BackendId, FollowUp
44
import { failureReport } from "../../shared/failures";
55
import { askedBecause, jevNotConsulted, receiptVia } from "../../shared/approval-receipt";
66
import { safeHref } from "../../shared/inline";
7+
import { answerNote } from "../../shared/human-requests";
78
import { parseWebSearch, splitSources, type SourceLink, type WebSearch } from "../../shared/sources";
89
import type { AttachmentActions } from "../composer-attachments";
910
import type { Attachment } from "../../shared/attachments";
@@ -376,7 +377,10 @@ function ApprovalItem({ item, onAnswer, onPolicy }: { item: ApprovalEntry; onAns
376377
{r?.decision === "ask" && <div className="approval-reason" data-testid="approval-reason">{askedBecause(r)}</div>}
377378
{item.detail && <pre className="detail">{item.detail}</pre>}
378379
{item.answer ? (
379-
<div className="approval-answer" data-testid="approval-answer" data-withdrawn={item.withdrawn}>{item.withdrawn ? WITHDRAWN[item.withdrawn] : item.answer === "yes" ? "Approved" : item.answer === "always" ? "Approved — always for this command" : "Denied"}</div>
380+
<>
381+
<div className="approval-answer" data-testid="approval-answer" data-withdrawn={item.withdrawn}>{item.withdrawn ? WITHDRAWN[item.withdrawn] : item.answer === "yes" ? "Approved" : item.answer === "always" ? "Approved — always for this command" : "Denied"}</div>
382+
{!item.withdrawn && answerNote(item) && <div className="answer-note" data-testid="answer-note" data-delivery={item.delivery}>{answerNote(item)}</div>}
383+
</>
380384
) : (
381385
<div className="row">
382386
<button className="btn primary small" onClick={() => onAnswer(item.id, "yes")}>Approve</button>

‎apps/desktop/src/renderer/components/transcript/QuestionCard.tsx‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { useState } from "react";
22
import type { AnswerResult, ThreadItem } from "../../../shared/types";
3-
import type { Question } from "../../../shared/human-requests";
3+
import { answerNote, type Question } from "../../../shared/human-requests";
44

55
type QuestionEntry = Extract<ThreadItem, { kind: "question" }>;
66

@@ -85,6 +85,7 @@ export function QuestionCard({ item, onAnswer }: { item: QuestionEntry; onAnswer
8585
)}
8686
</div>
8787
))}
88+
{item.answers && answerNote(item) && <div className="answer-note" data-testid="answer-note" data-delivery={item.delivery}>{answerNote(item)}</div>}
8889
{item.withdrawn ? <div className="approval-answer" data-testid="question-closed">{CLOSED[item.withdrawn]}</div> : open && (
8990
<div className="row">
9091
<button className="btn primary small" data-testid="question-submit" disabled={sending} onClick={() => void submit()}>Send answers</button>

‎apps/desktop/src/renderer/styles.css‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -641,6 +641,7 @@ a.thread-pr:hover { text-decoration: underline; }
641641
.shield { color: var(--warn); }
642642
.approval .detail { max-height: 200px; margin: 0 0 12px; padding: 8px 10px; overflow: auto; border-radius: var(--radius-sm); background: var(--bg-sunken); color: var(--text-2); white-space: pre-wrap; }
643643
.approval-answer { color: var(--text-3); font-size: var(--text-sm); }
644+
.answer-note { margin-top: 4px; color: var(--text-3); font-size: var(--text-sm); }
644645
.approval-reason { margin: -4px 0 10px; color: var(--text-3); font-size: var(--text-sm); }
645646
/* Question card: the approval frame, one block per question, each with its own control. */
646647
.question-field { margin: 0 0 12px; }

‎apps/desktop/src/shared/human-requests.ts‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
*
88
* Pure and serializable, shared by main (which owns the authoritative state) and clients.
99
*/
10-
import type { BackendId } from "./types.js";
10+
import type { AnswerDelivery, BackendId } from "./types.js";
1111

1212
export const HUMAN_REQUEST_SCHEMA = 1 as const;
1313

@@ -135,6 +135,18 @@ export function validateAnswers(questions: readonly Question[], answers: unknown
135135
return { ok: true, answers: answers as Record<string, string[]> };
136136
}
137137

138+
/**
139+
* What a closed card adds under its answer: that the phone gave it, and that nobody can tell whether the CLI
140+
* received it. An answer given in this window and sent, or saved before these fields existed, adds nothing.
141+
*/
142+
export function answerNote(card: { answeredBy?: string; delivery?: AnswerDelivery }): string | null {
143+
const notes = [
144+
card.answeredBy === "phone" ? "Answered on your phone." : "",
145+
card.delivery === "uncertain" ? "Modex stopped before it could confirm the agent received this answer." : "",
146+
].filter(Boolean);
147+
return notes.length ? notes.join(" ") : null;
148+
}
149+
138150
/** Answers as they may be shown, saved or exported: a secret answer is "(hidden)". */
139151
export function publicAnswers(questions: readonly Question[], answers: Record<string, string[]>): Record<string, string[]> {
140152
const secret = new Set(questions.filter((q) => q.secret).map((q) => q.id));

‎apps/desktop/src/shared/types.ts‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -119,14 +119,16 @@ export type ThreadItem =
119119
/**
120120
* `withdrawn` says why a card closed without a human answer: the CLI withdrew the request (`native`), the turn
121121
* was stopped (`stopped`), or it ended first (`ended`). Such a card also carries `answer: "no"`, so clients
122-
* that predate the field show it closed, never actionable.
122+
* that predate the field show it closed, never actionable. `answeredBy` and `delivery` are a person's answer's
123+
* lifecycle (docs/human-requests.md#across-replay-and-reconnect).
123124
*/
124-
| { id: string; kind: "approval"; question: string; detail?: string; canAlways?: boolean; title?: string; answer?: ApprovalAnswer; decidedBy?: ApprovalReceipt; withdrawn?: "native" | "stopped" | "ended"; at: string }
125+
| { id: string; kind: "approval"; question: string; detail?: string; canAlways?: boolean; title?: string; answer?: ApprovalAnswer; decidedBy?: ApprovalReceipt; withdrawn?: "native" | "stopped" | "ended"; answeredBy?: string; delivery?: AnswerDelivery; at: string }
125126
/**
126127
* A question the CLI asked a person (docs/human-requests.md; Codex `item/tool/requestUserInput`). `answers`
127-
* are the answers as they may be shown, saved or exported: a secret one is "(hidden)". `withdrawn` as on approvals.
128+
* are the answers as they may be shown, saved or exported: a secret one is "(hidden)". `withdrawn`, `answeredBy`
129+
* and `delivery` as on approvals.
128130
*/
129-
| { id: string; kind: "question"; questions: Question[]; blocking: boolean; answers?: Record<string, string[]>; withdrawn?: "native" | "stopped" | "ended"; at: string }
131+
| { id: string; kind: "question"; questions: Question[]; blocking: boolean; answers?: Record<string, string[]>; withdrawn?: "native" | "stopped" | "ended"; answeredBy?: string; delivery?: AnswerDelivery; at: string }
130132
/** `failure` is set on the error notice that ends a turn: it carries the CLI's message, a remedy, and debug context. */
131133
| { id: string; kind: "notice"; level: "info" | "warn" | "error"; text: string; at: string; failure?: TurnFailure; knowledge?: import("./knowledge.js").KnowledgeChange; pages?: import("./pages.js").PagesChange }
132134
/** Model reasoning: Codex reasoning summaries or Claude extended thinking. Collapsible in the UI. `reasoning` says which. */
@@ -136,6 +138,12 @@ export type ThreadItem =
136138
/** An Auto routing decision made before a turn: what was picked and why. */
137139
| { id: string; kind: "route"; backend: BackendId; model: string; effort?: string; fast: boolean; source: "jev" | "heuristic"; task: string; confidence: number; complexity: number; pinned: boolean; blocked?: true; held?: true; reasons: string[]; durationMs: number; at: string };
138140

141+
/**
142+
* Whether a person's accepted answer reached the CLI: `sending` from acceptance until the hand-off, then `sent`.
143+
* `uncertain` when Modex stopped in between, so nobody can tell; such an answer is never sent again.
144+
*/
145+
export type AnswerDelivery = "sending" | "sent" | "uncertain";
146+
139147
/** Why a turn did not complete, read from the CLI's own message (see shared/failures.ts). */
140148
export type FailureCode = "auth" | "not_installed" | "rate_limited" | "network" | "crashed" | "unsupported_history" | "unknown";
141149

0 commit comments

Comments
 (0)