Skip to content

Commit f90c7ce

Browse files
committed
Merge verified approval recovery into beta status recovery
Retain the historical beta approval documentation alongside the reviewed approval guards. Product, test and build inputs match the verified approval recovery; the combined build, type checks and full unit suite pass. Coven-Harness: Codex / gpt-6-astra
2 parents 69f4c11 + 87368ec commit f90c7ce

18 files changed

Lines changed: 430 additions & 43 deletions

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

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,11 @@ test("Settings scopes new rules, previews without saving, keeps inactive rules v
4949
await expect(preview()).toHaveAttribute("data-decision", "allow");
5050
await expect(preview()).toContainText("Exact match");
5151
await expect(preview()).toContainText("Preview only");
52+
await page.getByLabel("Sample action").fill("npm test && rm -rf src");
53+
await page.getByRole("button", { name: "Try it", exact: true }).click();
54+
await expect(preview()).toHaveAttribute("data-decision", "ask");
55+
await expect(preview()).toContainText("Allow became Ask because the command chains several commands.");
56+
await expect(preview()).toContainText("Jev not consulted");
5257
expect(await storedRules()).toHaveLength(1);
5358
expect((await page.evaluate(() => window.modex!.invoke("state:get", undefined))).threads).toHaveLength(0);
5459
await page.getByLabel("Sample action").fill("npm build");
@@ -121,6 +126,8 @@ test("automatic allow and never persist compact receipts; ask and project isolat
121126
await approval.getByRole("button", { name: "Deny", exact: true }).click();
122127
} else {
123128
await expect(tid(approval, "approval-receipt")).toContainText(decision === "allow" ? "Allowed:" : "Refused by rule");
129+
// No Jev in this environment: the allow had no destructive check, and its receipt says so.
130+
if (decision === "allow") await expect(tid(approval, "receipt-meta")).toContainText("Jev not consulted");
124131
await expect(approval.getByRole("button", { name: "Approve", exact: true })).toHaveCount(0);
125132
await tid(approval, "item-toggle").click();
126133
await expect(tid(approval, "item-body")).toContainText("write the sample");
@@ -159,3 +166,27 @@ test("a safety-downgraded receipt explains why a human must answer", async () =>
159166
await expect(card.getByRole("button", { name: "Deny", exact: true })).toBeVisible();
160167
await expect(card.getByRole("button", { name: "Always", exact: true })).toHaveCount(0);
161168
});
169+
170+
test("an allow rule covers one command: a chained shell command asks and says why", async () => {
171+
const script = path.join(home, "chained-script.json");
172+
fs.writeFileSync(script, JSON.stringify({ steps: [
173+
{ tool_calls: [{ name: "shell", arguments: { command: "echo one; touch chained.txt" } }] },
174+
{ content: "Finished." },
175+
] }));
176+
await page.evaluate(async ({ script, repo }) => {
177+
await window.modex!.invoke("settings:update", { mock_script: script, approval_gate: { enabled: true, threshold: 0.8, timeout_ms: 3000 }, approval_rules: [
178+
{ id: "echo", when: "print text", decision: "allow", project: repo, match: "shell: echo *", enabled: true },
179+
] });
180+
}, { script, repo });
181+
await page.keyboard.press("Meta+n");
182+
await tid(page, "composer-input").fill("chain two commands");
183+
await page.keyboard.press("Meta+Enter");
184+
const card = items(page, "approval");
185+
await expect(card).toHaveCount(1);
186+
await expect(card).toHaveAttribute("data-decided-by", "ask");
187+
await expect(currentRow(page)).toHaveAttribute("data-status", "waiting");
188+
await expect(tid(card, "approval-reason")).toHaveText('Asked because: the command chains several commands; rule "print text" would have allowed it · Jev not consulted');
189+
await card.getByRole("button", { name: "Deny", exact: true }).click();
190+
await expect(currentRow(page)).toHaveAttribute("data-status", "idle");
191+
expect(fs.existsSync(path.join(repo, "chained.txt"))).toBe(false);
192+
});

‎apps/desktop/src/main/engine/approvals/digest.ts‎

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ export interface DigestContext {
2121
}
2222

2323
export const MAX_COMMAND = 500;
24-
const MAX_TITLE = 200;
24+
export const MAX_TITLE = 200;
2525
const MAX_PATHS = 20;
2626
const MAX_PATH = 300;
2727
export const REDACTED = "«redacted»";
@@ -91,11 +91,25 @@ function pathsOf(action: ApprovalAction): string[] {
9191
return out.slice(0, MAX_PATHS);
9292
}
9393

94+
/** The title Jev sees, before clipping to MAX_TITLE. */
9495
function titleOf(action: ApprovalAction, command: string | undefined, paths: string[]): string {
95-
if (TITLE_FROM_TOOL.has(action.tool)) return clip(`${action.tool === "Write" ? "write" : "edit"} ${paths[0] ?? ""}`.trim(), MAX_TITLE);
96+
if (TITLE_FROM_TOOL.has(action.tool)) return `${action.tool === "Write" ? "write" : "edit"} ${paths[0] ?? ""}`.trim();
9697
// A title that repeats the command gets the scrubbed command, never the raw one.
97-
if (command && action.title.includes(command)) return clip(action.title.replace(command, scrubSecrets(command)), MAX_TITLE);
98-
return clip(scrubSecrets(action.title), MAX_TITLE);
98+
if (command && action.title.includes(command)) return action.title.replace(command, scrubSecrets(command));
99+
return scrubSecrets(action.title);
100+
}
101+
102+
/**
103+
* True when `digestAction` clips text Jev needs to judge the action: a command longer than
104+
* MAX_COMMAND, or a title longer than MAX_TITLE. A title that is only the command line ("$ cmd"
105+
* for Claude, the bare command for Codex) does not count when the command itself fits, because
106+
* Jev then reads the whole command in `command`. Paths are not considered.
107+
*/
108+
export function digestClipped(action: ApprovalAction): boolean {
109+
const raw = commandOf(action);
110+
if (raw && scrubSecrets(raw).length > MAX_COMMAND) return true;
111+
if (raw && (action.title === raw || action.title === `$ ${raw}`)) return false;
112+
return titleOf(action, raw, pathsOf(action)).length > MAX_TITLE;
99113
}
100114

101115
/** Builds the state Jev sees for one approval. Pure: same action, same digest. */
@@ -107,7 +121,7 @@ export function digestAction(action: ApprovalAction, ctx: DigestContext): Approv
107121
action: {
108122
backend: action.backend,
109123
tool: action.tool,
110-
title: titleOf(action, rawCommand, paths),
124+
title: clip(titleOf(action, rawCommand, paths), MAX_TITLE),
111125
...(action.cwd ? { cwd_name: path.basename(action.cwd) } : {}),
112126
...(action.escalation ? { escalation: true } : {}),
113127
},

‎apps/desktop/src/main/engine/approvals/gate.ts‎

Lines changed: 53 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
1-
import type { ApprovalAnswer, ApprovalGateConfig, ApprovalReceipt, ApprovalRule, Mode, RuleDecision } from "../../../shared/types.js";
1+
import type { ApprovalAnswer, ApprovalDowngrade, ApprovalGateConfig, ApprovalReceipt, ApprovalRule, Mode, RuleDecision } from "../../../shared/types.js";
22
import type { ApprovalAction } from "../backends/types.js";
33
import { JevError, type JevQuestion, type JevTransport } from "../routing/jev.js";
4-
import { digestAction } from "./digest.js";
4+
import { digestAction, digestClipped } from "./digest.js";
55

66
/**
77
* The approval gate: answers an approval from the user's plain-language rules before it reaches a
@@ -13,14 +13,19 @@ import { digestAction } from "./digest.js";
1313
* 3. Rules without one go to Jev (one request, one noul per rule, plus `destructive`), capped at
1414
* MAX_JEV_RULES. A rule applies when its noul ≥ threshold.
1515
* 4. Applying rules combine as never > ask > allow; nothing applies → ask.
16-
* 5. Safety downgrades no rule can override: allow on something Jev finds destructive (≥ 0.5) or on a
17-
* sandbox/permission escalation → ask.
18-
* 6. Jev unavailable → only the deterministic result counts, and the receipt says so.
16+
* 5. Safety downgrades no rule can override, labelled in this order: allow on a sandbox/permission
17+
* escalation, on something Jev finds destructive (≥ 0.5), on a command line that chains or
18+
* redirects commands, or on an action whose digest clipped text Jev needed → ask.
19+
* 6. Jev needed (language rules, or the destructive check on a match allow) but not configured or
20+
* unavailable → only the deterministic result counts, and the decision and receipt say Jev was
21+
* not consulted.
1922
* 7. Aborted while deciding → the approval is answered "no".
2023
*/
2124

2225
export const MAX_JEV_RULES = 12;
2326
export const DESTRUCTIVE_THRESHOLD = 0.5;
27+
/** The reason recorded when Jev was needed and there is no transport to ask. */
28+
export const JEV_NOT_CONFIGURED = "Jev is not configured or unavailable.";
2429

2530
export const DESTRUCTIVE_QUESTION: JevQuestion = {
2631
type: "noul",
@@ -56,8 +61,10 @@ export interface GateDecision {
5661
ms: number;
5762
/** Jev's `destructive` probability, when it was asked. */
5863
destructive?: number;
59-
/** Why a rule's allow became an ask, or that Jev could not be consulted. */
60-
downgraded?: "destructive" | "escalation" | "jev-unavailable";
64+
/** Why a rule's allow became an ask. */
65+
downgraded?: ApprovalDowngrade;
66+
/** Jev was needed for this decision but was not consulted; independent of `downgraded`. */
67+
jevNotConsulted?: true;
6168
/** Why Jev could not be consulted, when it could not. */
6269
jevError?: string;
6370
/** The turn stopped while the gate was deciding: the approval is answered "no". */
@@ -85,6 +92,7 @@ export function receiptFor(d: GateDecision): ApprovalReceipt | undefined {
8592
ms: d.ms,
8693
...(d.destructive !== undefined ? { destructive: round(d.destructive) } : {}),
8794
...(d.downgraded ? { downgraded: d.downgraded } : {}),
95+
...(d.jevNotConsulted ? { jevNotConsulted: true as const } : {}),
8896
};
8997
}
9098

@@ -116,6 +124,37 @@ export function matchesRule(match: string, action: Pick<ApprovalAction, "tool" |
116124
return candidates.some((c) => re.test(c));
117125
}
118126

127+
/** Tools whose approval runs a shell command line: Claude's Bash, Codex's command, the scripted engine's shell. */
128+
const COMMAND_TOOLS = new Set(["Bash", "command", "shell"]);
129+
130+
/**
131+
* Shell control operators and line breaks: `;`, `&` (so `&&`, `&`, `2>&1`), `|` (so `||`), backtick,
132+
* `$(`, `<` and `>` (so `<(` and `>(`), CR and LF. Quoting is deliberately not parsed, so
133+
* `grep "a|b"` counts too: a false positive only means a person is asked.
134+
*/
135+
const CHAINING = /[;&|`<>\r\n]|\$\(/;
136+
137+
/**
138+
* True when a command action's full command line (never the digest's clipped copy) chains,
139+
* redirects or substitutes commands. An allow rule covers one command, so such a line is asked.
140+
* Both the input's `command` and the title are checked; either one chaining is enough.
141+
*/
142+
export function chainsCommands(action: Pick<ApprovalAction, "tool" | "title" | "input">): boolean {
143+
if (!COMMAND_TOOLS.has(action.tool)) return false;
144+
const command = action.input?.command;
145+
const lines = [action.title.replace(/^\$ /, ""), ...(typeof command === "string" ? [command] : [])];
146+
return lines.some((line) => CHAINING.test(line));
147+
}
148+
149+
/** Why an allow must be asked instead; the first reason wins the label. */
150+
function downgradeOf(action: ApprovalAction, destructive: number | undefined): ApprovalDowngrade | undefined {
151+
if (action.escalation) return "escalation";
152+
if (destructive !== undefined && destructive >= DESTRUCTIVE_THRESHOLD) return "destructive";
153+
if (chainsCommands(action)) return "compound";
154+
if (digestClipped(action)) return "incomplete";
155+
return undefined;
156+
}
157+
119158
/** Rules sent to Jev: project-scoped first, then newest (later in the list), at most MAX_JEV_RULES. */
120159
export function jevRules(rules: ApprovalRule[]): ApprovalRule[] {
121160
return rules
@@ -184,9 +223,11 @@ export async function decide(action: ApprovalAction | undefined, ctx: GateContex
184223
// so the destructive downgrade can apply to an exact-match allow too.
185224
const unmatched = jevRules(rules.filter((r) => !r.match));
186225
const wouldAllow = combine(hits)?.rule.decision === "allow";
226+
const needsJev = unmatched.length > 0 || wouldAllow;
187227
let destructive: number | undefined;
188228
let jevError: string | undefined;
189-
if (ctx.transport && (unmatched.length || wouldAllow)) {
229+
if (needsJev && !ctx.transport) jevError = JEV_NOT_CONFIGURED;
230+
else if (needsJev && ctx.transport) {
190231
const questions: Record<string, JevQuestion> = { destructive: DESTRUCTIVE_QUESTION };
191232
unmatched.forEach((r, n) => { questions[`rule_${n}`] = ruleQuestion(r); });
192233
const state = digestAction(action, { project: { name: ctx.project.name, branch: ctx.project.branch }, mode: ctx.mode });
@@ -206,12 +247,10 @@ export async function decide(action: ApprovalAction | undefined, ctx: GateContex
206247
if (signal?.aborted) return { decision: "ask", source: "none", ms: ms(), aborted: true };
207248

208249
const best = combine(hits);
209-
const base = { ms: ms(), ...(destructive !== undefined ? { destructive } : {}), ...(jevError ? { jevError, downgraded: "jev-unavailable" as const } : {}) };
250+
const base = { ms: ms(), ...(destructive !== undefined ? { destructive } : {}), ...(jevError ? { jevError, jevNotConsulted: true as const } : {}) };
210251
if (!best) return { decision: "ask", source: "none", ...base };
211252
const decided: GateDecision = { decision: best.rule.decision, rule: best.rule, source: best.via, p: best.p, ...base };
212-
if (decided.decision === "allow") {
213-
if (action.escalation) return { ...decided, decision: "ask", downgraded: "escalation" };
214-
if (destructive !== undefined && destructive >= DESTRUCTIVE_THRESHOLD) return { ...decided, decision: "ask", downgraded: "destructive" };
215-
}
216-
return decided;
253+
// Never and Ask rules stand as they are; only a rule-driven allow can be downgraded.
254+
const downgraded = decided.decision === "allow" ? downgradeOf(action, destructive) : undefined;
255+
return downgraded ? { ...decided, decision: "ask", downgraded } : decided;
217256
}

‎apps/desktop/src/main/engine/approvals/policy.ts‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,26 @@
1-
import type { ApprovalPolicy, ApprovalReceipt } from "../../../shared/types.js";
1+
import type { ApprovalDowngrade, ApprovalPolicy, ApprovalReceipt } from "../../../shared/types.js";
22
import type { ApprovalAction } from "../backends/types.js";
33

4+
/** Downgrades Always allow must not override. */
5+
const HELD: ReadonlySet<string> = new Set<ApprovalDowngrade>(["destructive", "escalation", "compound", "incomplete"]);
6+
47
/**
58
* A thread's standing answer to approvals, applied only where the rules gate would have left the
69
* decision to a human (an explicit "Never" rule has already refused by then).
710
*
811
* - `ask`: nothing; the card appears.
912
* - `always`: yes, unless the request wants more sandbox access, or we cannot tell what it wants
10-
* (no structured action), or the rules gate sent it here (an Ask rule, or a destructive/escalation downgrade).
13+
* (no structured action), or the rules gate sent it here (an Ask rule, or a safety downgrade:
14+
* destructive, escalation, a chained command, or a command Jev could not see in full).
15+
* Jev not being consulted is not itself a reason to ask.
1116
* - `yolo`: yes to everything.
1217
*/
1318
export function policyAnswer(policy: ApprovalPolicy | undefined, action: ApprovalAction | undefined, gated?: ApprovalReceipt): "yes" | null {
1419
if (policy === "yolo") return "yes";
1520
if (policy !== "always") return null;
1621
if (!action || action.escalation) return null;
1722
// A rule that said "ask" or a safety downgrade is the user's own caution; only YOLO overrides it.
18-
if (gated && (gated.decision === "ask" || gated.downgraded === "destructive" || gated.downgraded === "escalation")) return null;
23+
if (gated && (gated.decision === "ask" || (gated.downgraded !== undefined && HELD.has(gated.downgraded)))) return null;
1924
return "yes";
2025
}
2126

‎apps/desktop/src/main/engine/approvals/preview.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,5 +52,6 @@ export async function previewApproval(raw: unknown, ctx: Context): Promise<Appro
5252
const result = await decide({ backend: req.backend, tool, title, cwd: project.path, escalation: req.escalation,
5353
...(tool === "Bash" || tool === "command" ? { input: { command: title.replace(/^\$ /, "") } } : {}),
5454
}, { rules, config: { ...ctx.config, enabled: true }, project: { root: project.path, name: project.name }, mode: req.mode, transport, model });
55-
return { ...result, ...(setupError ? { jevError: setupError, downgraded: result.downgraded ?? "jev-unavailable" } : {}), gateEnabled: ctx.config.enabled, jevAvailable: transport !== null };
55+
// A judge that failed to resolve is reported by name rather than as "not configured".
56+
return { ...result, ...(setupError ? { jevError: setupError } : {}), gateEnabled: ctx.config.enabled, jevAvailable: transport !== null };
5657
}

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,9 @@ function PreviewReceipt({ result: r }: { result: ApprovalPreview }) {
9090
<div>{r.source === "match" ? "Exact match" : r.source === "jev" ? `Jev ${r.p?.toFixed(2) ?? ""}` : "No match"} · {r.ms} ms{r.destructive !== undefined ? ` · destructive ${r.destructive.toFixed(2)}` : ""}</div>
9191
{r.downgraded === "destructive" && <p>Allow became Ask because the action may be destructive.</p>}
9292
{r.downgraded === "escalation" && <p>Allow became Ask because only you can grant additional permissions.</p>}
93-
{(!r.jevAvailable || r.downgraded === "jev-unavailable") && <p>Jev unavailable; only exact-match rules were considered.{r.jevError ? ` ${r.jevError}` : ""}</p>}
93+
{r.downgraded === "compound" && <p>Allow became Ask because the command chains several commands.</p>}
94+
{r.downgraded === "incomplete" && <p>Allow became Ask because Jev could not see the whole command.</p>}
95+
{(!r.jevAvailable || r.jevNotConsulted) && <p>Jev not consulted; only exact-match rules were considered.{r.jevError ? ` ${r.jevError}` : ""}</p>}
9496
{!r.gateEnabled && <p>Preview only — the approval gate is off.</p>}
9597
</div>;
9698
}

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { toolSummary } from "../../shared/tool-summary";
22
import { Fragment, useEffect, useMemo, useRef, useState } from "react";
33
import type { ApprovalPolicy, ApprovalReceipt, BackendId, FollowUp, Mode, ModelInfo, Project, Thread, ThreadItem, ThreadPatch, TurnFailure, TurnFix } from "../../shared/types";
44
import { failureReport } from "../../shared/failures";
5-
import { askedBecause, receiptVia } from "../../shared/approval-receipt";
5+
import { askedBecause, jevNotConsulted, receiptVia } from "../../shared/approval-receipt";
66
import { safeHref } from "../../shared/inline";
77
import { parseWebSearch, splitSources, type SourceLink, type WebSearch } from "../../shared/sources";
88
import type { AttachmentActions } from "../composer-attachments";
@@ -396,7 +396,7 @@ function ApprovalReceiptRow({ item, receipt: r }: { item: ApprovalEntry; receipt
396396
const [open, setOpen] = useState(false);
397397
const allowed = r.decision === "allow";
398398
const title = item.title ?? item.question;
399-
const via = `${receiptVia(r)} · ${r.ms} ms${r.downgraded === "jev-unavailable" ? " · Jev unavailable" : ""}`;
399+
const via = `${receiptVia(r)} · ${r.ms} ms${jevNotConsulted(r) ? " · Jev not consulted" : ""}`;
400400
return (
401401
<div className={`approval-receipt ${allowed ? "allowed" : "refused"} ${open ? "open" : ""}`} data-testid="item" data-item-kind="approval" data-answered="true" data-decided-by={r.decision}>
402402
<button className="receipt-head" data-testid="item-toggle" onClick={() => setOpen((v) => !v)} aria-expanded={open}>
@@ -415,7 +415,7 @@ function ApprovalReceiptRow({ item, receipt: r }: { item: ApprovalEntry; receipt
415415
{r.source === "policy"
416416
? `Approved because this thread is set to "${r.when}".`
417417
: <>{allowed ? "Approved once" : "Denied"} by your rule "{r.when}" ({receiptVia(r)}, {r.ms} ms){r.destructive !== undefined ? `; Jev put the chance that it is destructive at ${r.destructive.toFixed(2)}` : ""}.
418-
{r.downgraded === "jev-unavailable" ? " Jev was unavailable, so only rules with an exact match could apply." : ""}</>}
418+
{jevNotConsulted(r) ? " Jev was not consulted (not configured or unavailable), so only rules with an exact match could apply and nothing was checked for destructive effects." : ""}</>}
419419
</div>
420420
</div>
421421
)}

0 commit comments

Comments
 (0)