+ {title}
{entry.graphLabel ? (
for run {entry.graphLabel}
) : null}
+ ) : null}
+ {/* Only when there is something the target line did not
+ already say. `null` used to print verbatim, which reads as
+ a bug in the request. */}
+ {!finished && argumentsWorthShowing(entry.arguments, target) ? (
+
+ ) : null}
{lapsed && entry.state.kind === "lapsed" ? (
-
- No longer answerable — {lapsedExplanation(entry.state.reason)}.
+
+ {lapsedExplanation(entry.state.reason)}.
) : settled && entry.state.kind === "settled" ? (
-
- {entry.state.approved ? "Approved." : "Denied."}
+
+ {/* Only an always-allow can still be on screen once
+ settled, and nobody clicked this card — saying
+ "Approved." alone would credit the user with a decision
+ they were never shown. */}
+ {entry.state.via === "always-allowed"
+ ? "Covered by the permission you just granted — this one was never asked."
+ : entry.state.approved
+ ? "Approved."
+ : "Denied."}
@@ -3505,7 +3573,10 @@ export function AgentPanel({
onClick={() =>
transcriptRef.current?.scrollTo({ top: transcriptRef.current.scrollHeight, behavior: "smooth" })
}
- className="pointer-events-auto inline-flex h-7 w-7 items-center justify-center rounded-full border border-line-strong bg-raised text-ink-subtle shadow-md transition-colors hover:text-ink-strong active:bg-surface-2"
+ /* The halo is what separates a floating control from whatever it
+ happens to be over. Without it the button sits exactly on a
+ card's top border and reads as part of the card. */
+ className="pointer-events-auto inline-flex h-7 w-7 items-center justify-center rounded-full border border-line-strong bg-raised text-ink-subtle shadow-[0_0_0_5px_var(--surface-0),0_2px_8px_rgba(0,0,0,0.14)] transition-colors hover:text-ink-strong active:bg-surface-2"
>
diff --git a/client/src/lib/approvalRouter.test.ts b/client/src/lib/approvalRouter.test.ts
index f93aa9de..5c637a79 100644
--- a/client/src/lib/approvalRouter.test.ts
+++ b/client/src/lib/approvalRouter.test.ts
@@ -7,6 +7,8 @@ import {
APPROVAL_TTL_MS,
MAX_QUEUED_APPROVALS,
SETTLED_LINGER_MS,
+ approvalTarget,
+ argumentsWorthShowing,
emptyStore,
headApproval,
reduce,
@@ -291,7 +293,7 @@ describe("the queue", () => {
store = reduce(store, { kind: "SendAccepted", requestId: "r-2" });
expect(stateOf(store, "r-1")).toEqual({ kind: "pending" });
expect(stateOf(store, "r-3")).toEqual({ kind: "pending" });
- expect(stateOf(store, "r-2")).toEqual({ kind: "settled", approved: true });
+ expect(stateOf(store, "r-2")).toEqual({ kind: "settled", approved: true, via: "this-window" });
});
// Defect 5, second half: the queue that replaced the single slot wedged
@@ -343,3 +345,76 @@ describe("the queue", () => {
expect(entry.graphLabel).toBe("graph-7");
});
});
+
+// ---------------------------------------------------------------------------
+// Defect 7: one act rendered twice. An answered card kept its "Approve X?"
+// prompt up beside the transcript row the same click had just written, so a
+// finished turn read as a wall of unanswered prompts.
+// ---------------------------------------------------------------------------
+
+describe("what a finished request still shows", () => {
+ it("stops rendering a card this window answered, because the transcript has it", () => {
+ let store = reduce(emptyStore(), arrived("r-1"));
+ store = reduce(store, { kind: "UserAnswered", requestId: "r-1", approved: true, nowMs: 2_000 });
+ store = reduce(store, { kind: "SendAccepted", requestId: "r-1" });
+
+ expect(visibleApprovals(store).map((entry) => entry.requestId)).toEqual([]);
+ // Gone from the view, not from the map: a redelivered `Arrived` must still
+ // find it finished rather than resurrecting an answered prompt.
+ expect(stateOf(reduce(store, arrived("r-1")), "r-1")).toEqual({
+ kind: "settled",
+ approved: true,
+ via: "this-window",
+ });
+ });
+
+ it("keeps up a card core cleared under an always-allow, which has no row of its own", () => {
+ let store = reduce(emptyStore(), arrived("r-1"));
+ store = reduce(store, { kind: "Resolved", requestId: "r-1", outcome: "always-allowed" });
+ expect(visibleApprovals(store).map((entry) => entry.requestId)).toEqual(["r-1"]);
+ });
+
+ it("starts the eviction clock on a card that finished with no send in flight", () => {
+ // `finishedAtMs` is null here — nothing was in flight to inherit a start
+ // from. `null ?? nowMs` made the age permanently zero, so the card aged
+ // out never and sat in the transcript for the rest of the session.
+ let store = reduce(emptyStore(), arrived("r-1"));
+ store = reduce(store, { kind: "Resolved", requestId: "r-1", outcome: "always-allowed" });
+ expect(store.entries.get("r-1")!.finishedAtMs).toBeNull();
+
+ store = reduce(store, { kind: "Tick", nowMs: 5_000 });
+ expect(store.entries.get("r-1")!.finishedAtMs).toBe(5_000);
+ store = reduce(store, { kind: "Tick", nowMs: 5_000 + SETTLED_LINGER_MS });
+ expect(stateOf(store, "r-1")).toBe("absent");
+ });
+});
+
+describe("what a card says it is about", () => {
+ it("names the file, command or address the request would touch", () => {
+ expect(approvalTarget({ path: "src/game.ts" })).toBe("src/game.ts");
+ expect(approvalTarget({ file_path: " a.txt " })).toBe("a.txt");
+ expect(approvalTarget({ command: "rm -rf build" })).toBe("rm -rf build");
+ expect(approvalTarget({ url: "https://example.com" })).toBe("https://example.com");
+ });
+
+ it("has no target rather than a wrong one", () => {
+ expect(approvalTarget(null)).toBeNull();
+ expect(approvalTarget({ depth: 3 })).toBeNull();
+ expect(approvalTarget(["a.txt"])).toBeNull();
+ expect(approvalTarget({ path: " " })).toBeNull();
+ });
+
+ it("prints nothing a request did not carry, and nothing it already said", () => {
+ // `JSON.stringify` renders the first three as text the user then has to
+ // decide is not an error. "null" was on screen under a live prompt.
+ expect(argumentsWorthShowing(null, null)).toBe(false);
+ expect(argumentsWorthShowing(undefined, null)).toBe(false);
+ expect(argumentsWorthShowing({}, null)).toBe(false);
+ expect(argumentsWorthShowing([], null)).toBe(false);
+ // The lone key IS the target line directly above it.
+ expect(argumentsWorthShowing({ path: "a.txt" }, "a.txt")).toBe(false);
+ expect(argumentsWorthShowing({ path: "a.txt", encoding: "utf8" }, "a.txt")).toBe(true);
+ // No target line, so the JSON is the only thing describing the request.
+ expect(argumentsWorthShowing({ depth: 3 }, null)).toBe(true);
+ });
+});
diff --git a/client/src/lib/approvalStore.ts b/client/src/lib/approvalStore.ts
index 7015d1c1..a260d89a 100644
--- a/client/src/lib/approvalStore.ts
+++ b/client/src/lib/approvalStore.ts
@@ -21,10 +21,19 @@ export type LapsedReason =
| "session-changed"
| "panel-gone";
+/**
+ * Who produced the answer. Not decoration: a decision this window made is
+ * already written to the transcript as a permanent row, so its card is a
+ * duplicate the moment it settles and must stop rendering. An
+ * `always-allowed` settle has no row of its own — nobody clicked that card —
+ * so it is the one that has to stay up and say so.
+ */
+export type SettledVia = "this-window" | "always-allowed";
+
export type RequestState =
| { kind: "pending" }
| { kind: "answering"; approved: boolean; startedAtMs: number }
- | { kind: "settled"; approved: boolean }
+ | { kind: "settled"; approved: boolean; via: SettledVia }
| { kind: "lapsed"; reason: LapsedReason };
export type ApprovalEntry = {
@@ -194,7 +203,7 @@ export function reduce(store: ApprovalStore, event: ApprovalEvent): ApprovalStor
const entries = new Map(store.entries);
entries.set(event.requestId, {
...existing,
- state: { kind: "settled", approved: existing.state.approved },
+ state: { kind: "settled", approved: existing.state.approved, via: "this-window" },
finishedAtMs: existing.state.startedAtMs,
});
return withEntries(store, entries);
@@ -227,7 +236,7 @@ export function reduce(store: ApprovalStore, event: ApprovalEvent): ApprovalStor
if (event.outcome === "always-allowed") {
entries.set(event.requestId, {
...existing,
- state: { kind: "settled", approved: true },
+ state: { kind: "settled", approved: true, via: "always-allowed" },
finishedAtMs:
existing.state.kind === "answering" ? existing.state.startedAtMs : null,
});
@@ -241,7 +250,7 @@ export function reduce(store: ApprovalStore, event: ApprovalEvent): ApprovalStor
entries.set(event.requestId, {
...existing,
state: ours
- ? { kind: "settled", approved: existing.state.approved }
+ ? { kind: "settled", approved: existing.state.approved, via: "this-window" }
: { kind: "lapsed", reason: "resolved-elsewhere" },
finishedAtMs: existing.state.startedAtMs,
});
@@ -271,10 +280,20 @@ export function reduce(store: ApprovalStore, event: ApprovalEvent): ApprovalStor
entries.set(id, entry);
continue;
}
+ // A card can finish without a timestamp: nothing was in flight when
+ // core announced it, so there was no `startedAtMs` to inherit. Stamp
+ // it on the first tick that sees it rather than leaving it null —
+ // `null ?? nowMs` makes the age permanently zero, and a card that can
+ // never age out never leaves the transcript.
+ if (entry.finishedAtMs === null) {
+ changed = true;
+ entries.set(id, { ...entry, finishedAtMs: event.nowMs });
+ continue;
+ }
// Finished cards linger so the user can read the outcome, then go.
// Measured from when they finished, so an early lapse and a late
// expiry get the same reading window.
- if (event.nowMs - (entry.finishedAtMs ?? event.nowMs) >= SETTLED_LINGER_MS) {
+ if (event.nowMs - entry.finishedAtMs >= SETTLED_LINGER_MS) {
changed = true;
continue;
}
@@ -305,14 +324,22 @@ export function reduce(store: ApprovalStore, event: ApprovalEvent): ApprovalStor
* `answering` entries are never head: a send that hangs must not hide the rest
* of the queue behind it. Three parallel nodes prompt together and stay
* independently answerable in any order.
+ *
+ * A decision this window made drops out here rather than out of the map: the
+ * transcript row written on the same click is the durable record, so keeping
+ * the card up renders one act twice, in two shapes, saying two different
+ * things. The entry survives so a re-delivered `Arrived` still finds it
+ * finished and does not resurrect an answered prompt.
*/
export function visibleApprovals(store: ApprovalStore): ApprovalEntry[] {
- return [...store.entries.values()].sort((left, right) => {
- const leftBusy = left.state.kind === "answering" ? 1 : 0;
- const rightBusy = right.state.kind === "answering" ? 1 : 0;
- if (leftBusy !== rightBusy) return leftBusy - rightBusy;
- return left.order - right.order;
- });
+ return [...store.entries.values()]
+ .filter((entry) => !(entry.state.kind === "settled" && entry.state.via === "this-window"))
+ .sort((left, right) => {
+ const leftBusy = left.state.kind === "answering" ? 1 : 0;
+ const rightBusy = right.state.kind === "answering" ? 1 : 0;
+ if (leftBusy !== rightBusy) return leftBusy - rightBusy;
+ return left.order - right.order;
+ });
}
/** The card the promotion guard and keyboard focus apply to. */
@@ -320,6 +347,44 @@ export function headApproval(store: ApprovalStore): ApprovalEntry | null {
return visibleApprovals(store)[0] ?? null;
}
+/**
+ * The one thing about a request a user has to see before answering it: which
+ * file, command or address it would touch. Without it two `file_write` prompts
+ * are the same four words twice, and the transcript rows they leave behind are
+ * indistinguishable from each other.
+ *
+ * Keys are tried in order and the first string wins; an unrecognised tool
+ * simply has no target, which is why the full arguments stay available below.
+ */
+export function approvalTarget(args: unknown): string | null {
+ if (!args || typeof args !== "object" || Array.isArray(args)) return null;
+ const record = args as Record
;
+ for (const key of ["path", "file_path", "file", "filename", "command", "url", "query", "pattern", "name"]) {
+ const value = record[key];
+ if (typeof value === "string" && value.trim()) return value.trim();
+ }
+ return null;
+}
+
+/**
+ * Whether the full arguments say anything the target line did not.
+ *
+ * Two cases print nothing. A tool that takes no arguments arrives as `null` or
+ * `{}`, and `JSON.stringify` renders both as text the user then has to decide
+ * is not an error. A single-key `{ path }` is already the target line, and
+ * repeating it as JSON directly underneath is the same duplication in
+ * miniature.
+ */
+export function argumentsWorthShowing(args: unknown, target: string | null): boolean {
+ if (args === null || args === undefined) return false;
+ if (Array.isArray(args)) return args.length > 0;
+ if (typeof args === "object") {
+ const keys = Object.keys(args as object).length;
+ return target ? keys > 1 : keys > 0;
+ }
+ return true;
+}
+
/** Plain-language reason a card is no longer answerable. */
export function lapsedExplanation(reason: LapsedReason): string {
switch (reason) {
diff --git a/client/src/lib/types.ts b/client/src/lib/types.ts
index 1539d132..3f238bb4 100644
--- a/client/src/lib/types.ts
+++ b/client/src/lib/types.ts
@@ -120,6 +120,12 @@ export interface AgentMessage {
status?: "running" | "done" | "error";
/** Tool rows only: full output, shown when the row is expanded. */
detail?: string;
+ /**
+ * This row records a permission the user granted or refused, not work the
+ * agent did. Carried explicitly rather than sniffed from `content`, so the
+ * row's icon cannot be changed by rewording a sentence.
+ */
+ decision?: "approved" | "denied";
/** Provider tool-call identity; pairing must not rely on tool names. */
toolCallId?: string;
/** Client-owned Enter-level activity grouping identity. */