Compare commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
60e57fde31 | ||
|
|
bc0fd6ee60 | ||
|
|
9c77965c51 | ||
|
|
49df4c2e9e |
@@ -933,7 +933,7 @@ describe("structured connection: approvals", () => {
|
|||||||
conn.dispose();
|
conn.dispose();
|
||||||
});
|
});
|
||||||
|
|
||||||
it("removes the buttons once the approval resolves, even with no accompanying transcript delta", () => {
|
it("hides a standalone resolved approval entirely when no tool group absorbs it", () => {
|
||||||
const ws = new FakeSocket();
|
const ws = new FakeSocket();
|
||||||
const conn = mount({ sockets: [ws] });
|
const conn = mount({ sockets: [ws] });
|
||||||
ws.open();
|
ws.open();
|
||||||
@@ -949,7 +949,68 @@ describe("structured connection: approvals", () => {
|
|||||||
// Resolved on another client: state clears with no transcript delta at all.
|
// Resolved on another client: state clears with no transcript delta at all.
|
||||||
ws.emit({ type: "reduced_state", state: idleState });
|
ws.emit({ type: "reduced_state", state: idleState });
|
||||||
expect(conn.el.querySelector(".st-approval-acts")).toBeNull();
|
expect(conn.el.querySelector(".st-approval-acts")).toBeNull();
|
||||||
expect(conn.el.querySelector(".st-approval-resolved")).not.toBeNull();
|
// Standalone resolved approval is hidden — no card, no compact line.
|
||||||
|
expect(conn.el.querySelector(".st-row-approval")).toBeNull();
|
||||||
|
conn.dispose();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("absorbs a resolved approval into the tool group as a compact line", () => {
|
||||||
|
const ws = new FakeSocket();
|
||||||
|
const conn = mount({ sockets: [ws] });
|
||||||
|
ws.open();
|
||||||
|
ws.emit({
|
||||||
|
type: "transcript_snapshot",
|
||||||
|
snapshot: {
|
||||||
|
cursor: 0,
|
||||||
|
rows: [
|
||||||
|
row("user:1", "user", { content: { text: "go" } }),
|
||||||
|
toolRow("tool:1", "first"),
|
||||||
|
row("approval:1", "approval", { toolCall: { title: "Write file.ts" }, options: allApprovalOptions }),
|
||||||
|
toolRow("tool:2", "second"),
|
||||||
|
],
|
||||||
|
},
|
||||||
|
});
|
||||||
|
ws.emit({ type: "reduced_state", state: idleState });
|
||||||
|
// 2+ tools in the turn → tool group exists; resolved approval is inside it.
|
||||||
|
const group = conn.el.querySelector(".st-tool-group");
|
||||||
|
expect(group).not.toBeNull();
|
||||||
|
const compact = group!.querySelector(".st-approval-compact");
|
||||||
|
expect(compact).not.toBeNull();
|
||||||
|
expect(compact!.textContent).toContain("Write file.ts");
|
||||||
|
expect(compact!.textContent).toContain("approved");
|
||||||
|
// No full-card approval anywhere on the transcript.
|
||||||
|
expect(conn.el.querySelector(".st-approval")).toBeNull();
|
||||||
|
conn.dispose();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("keeps a pending approval standalone even when a tool group exists in the same turn", () => {
|
||||||
|
const ws = new FakeSocket();
|
||||||
|
const conn = mount({ sockets: [ws] });
|
||||||
|
ws.open();
|
||||||
|
ws.emit({
|
||||||
|
type: "transcript_snapshot",
|
||||||
|
snapshot: {
|
||||||
|
cursor: 0,
|
||||||
|
rows: [
|
||||||
|
row("user:1", "user", { content: { text: "go" } }),
|
||||||
|
toolRow("tool:1", "first"),
|
||||||
|
row("approval:1", "approval", { toolCall: { title: "Write file.ts" }, options: allApprovalOptions }),
|
||||||
|
toolRow("tool:2", "second"),
|
||||||
|
],
|
||||||
|
},
|
||||||
|
});
|
||||||
|
ws.emit({
|
||||||
|
type: "reduced_state",
|
||||||
|
state: { ...idleState, pending_approval: { nonce: "n1", request: { sessionId: "s1", toolCall: {}, options: allApprovalOptions } } },
|
||||||
|
});
|
||||||
|
// Pending approval stays outside the tool group as a full card.
|
||||||
|
const approvalCard = conn.el.querySelector(".st-approval");
|
||||||
|
expect(approvalCard).not.toBeNull();
|
||||||
|
expect(conn.el.querySelectorAll(".st-approval-acts button").length).toBeGreaterThan(0);
|
||||||
|
// It should not be inside the tool group.
|
||||||
|
const group = conn.el.querySelector(".st-tool-group");
|
||||||
|
expect(group).not.toBeNull();
|
||||||
|
expect(group!.querySelector(".st-approval")).toBeNull();
|
||||||
conn.dispose();
|
conn.dispose();
|
||||||
});
|
});
|
||||||
|
|
||||||
|
|||||||
+37
-16
@@ -402,13 +402,15 @@ export function createStructuredConnection(
|
|||||||
return el;
|
return el;
|
||||||
}
|
}
|
||||||
|
|
||||||
/** Group tool rows (and intermediate agent text between them) by turn.
|
/** Group tool rows (and intermediate agent text / resolved approvals between
|
||||||
* An agent_text/agent_thought row is "intermediate" when a tool row follows
|
* them) by turn. An agent_text/agent_thought row is "intermediate" when a
|
||||||
* it in the same turn — it gets absorbed into the group so the collapsed
|
* tool row follows it in the same turn — it gets absorbed into the group so
|
||||||
* block preserves chronological order. The *last* agent text with no
|
* the collapsed block preserves chronological order. Resolved approval rows
|
||||||
* subsequent tool stays standalone (the prominent final answer).
|
* (not the currently pending one) are absorbed the same way. The *last*
|
||||||
* Returns groups keyed by `turn:<userId>`, each with `toolCount` (for the
|
* agent text with no subsequent tool stays standalone (the prominent final
|
||||||
* header label) and `rows` (tools + intermediate text, in order). */
|
* answer). Returns groups keyed by `turn:<userId>`, each with `toolCount`
|
||||||
|
* (for the header label) and `rows` (tools + intermediate text/approvals,
|
||||||
|
* in order). */
|
||||||
function activityGroupsByTurn(): Map<string, { toolCount: number; rows: TranscriptRow[] }> {
|
function activityGroupsByTurn(): Map<string, { toolCount: number; rows: TranscriptRow[] }> {
|
||||||
// First: count tools per turn to know which turns qualify for grouping.
|
// First: count tools per turn to know which turns qualify for grouping.
|
||||||
const toolCounts = new Map<string, number>();
|
const toolCounts = new Map<string, number>();
|
||||||
@@ -422,33 +424,35 @@ export function createStructuredConnection(
|
|||||||
// Second: build groups with interleaved text for turns with 2+ tools.
|
// Second: build groups with interleaved text for turns with 2+ tools.
|
||||||
const groups = new Map<string, { toolCount: number; rows: TranscriptRow[] }>();
|
const groups = new Map<string, { toolCount: number; rows: TranscriptRow[] }>();
|
||||||
turnId = "before-first-message";
|
turnId = "before-first-message";
|
||||||
// Pending text rows that might be intermediate (not yet confirmed by a
|
// Pending rows that might be intermediate (not yet confirmed by a
|
||||||
// following tool). If no tool follows, they stay standalone.
|
// following tool). If no tool follows, they stay standalone.
|
||||||
let pendingText: TranscriptRow[] = [];
|
let pending: TranscriptRow[] = [];
|
||||||
let currentGid = `turn:${turnId}`;
|
let currentGid = `turn:${turnId}`;
|
||||||
for (const row of rows) {
|
for (const row of rows) {
|
||||||
if (row.kind === "user") {
|
if (row.kind === "user") {
|
||||||
turnId = row.id;
|
turnId = row.id;
|
||||||
currentGid = `turn:${turnId}`;
|
currentGid = `turn:${turnId}`;
|
||||||
pendingText = [];
|
pending = [];
|
||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
const count = toolCounts.get(currentGid) ?? 0;
|
const count = toolCounts.get(currentGid) ?? 0;
|
||||||
if (count < 2) continue;
|
if (count < 2) continue;
|
||||||
if (row.kind === "agent_text" || row.kind === "agent_thought") {
|
const isIntermediateText = row.kind === "agent_text" || row.kind === "agent_thought";
|
||||||
pendingText.push(row);
|
const isResolvedApproval = row.kind === "approval" && row.id !== pendingApprovalRowId;
|
||||||
|
if (isIntermediateText || isResolvedApproval) {
|
||||||
|
pending.push(row);
|
||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
if (row.kind === "tool_call" || row.kind === "tool_diff") {
|
if (row.kind === "tool_call" || row.kind === "tool_diff") {
|
||||||
const group = groups.get(currentGid) ?? { toolCount: count, rows: [] };
|
const group = groups.get(currentGid) ?? { toolCount: count, rows: [] };
|
||||||
// Flush any pending text — a tool follows, so it's intermediate.
|
// Flush any pending rows — a tool follows, so they're intermediate.
|
||||||
group.rows.push(...pendingText);
|
group.rows.push(...pending);
|
||||||
pendingText = [];
|
pending = [];
|
||||||
group.rows.push(row);
|
group.rows.push(row);
|
||||||
groups.set(currentGid, group);
|
groups.set(currentGid, group);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
// pendingText at end of a turn = final text, stays outside (not flushed).
|
// pending at end of a turn = final text/approvals, stays outside (not flushed).
|
||||||
return groups;
|
return groups;
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -500,6 +504,10 @@ export function createStructuredConnection(
|
|||||||
children.push(groupEl);
|
children.push(groupEl);
|
||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
|
// Resolved approvals not absorbed into a tool group are hidden — the
|
||||||
|
// tool row itself already identifies the action, so a standalone
|
||||||
|
// "answered" card adds nothing. Pending approvals always render.
|
||||||
|
if (row.kind === "approval" && row.id !== pendingApprovalRowId) continue;
|
||||||
const rowEl = buildRowElement(row);
|
const rowEl = buildRowElement(row);
|
||||||
if (!rowEl) continue;
|
if (!rowEl) continue;
|
||||||
rowEls.set(row.id, rowEl);
|
rowEls.set(row.id, rowEl);
|
||||||
@@ -551,6 +559,19 @@ export function createStructuredConnection(
|
|||||||
rows.push(delta.row);
|
rows.push(delta.row);
|
||||||
} else {
|
} else {
|
||||||
rows[i] = delta.row;
|
rows[i] = delta.row;
|
||||||
|
// Surgical DOM swap: replace just this row's element instead of
|
||||||
|
// tearing down and rebuilding the entire transcript, which causes
|
||||||
|
// visible flashing and breaks text selection.
|
||||||
|
const oldEl = rowEls.get(delta.row.id);
|
||||||
|
if (oldEl) {
|
||||||
|
const newEl = buildRowElement(delta.row);
|
||||||
|
if (newEl) {
|
||||||
|
oldEl.replaceWith(newEl);
|
||||||
|
rowEls.set(delta.row.id, newEl);
|
||||||
|
}
|
||||||
|
syncAvailableCommands();
|
||||||
|
return;
|
||||||
|
}
|
||||||
}
|
}
|
||||||
} else {
|
} else {
|
||||||
const i = rowIndex.get(delta.id);
|
const i = rowIndex.get(delta.id);
|
||||||
|
|||||||
@@ -368,13 +368,14 @@ describe("buildTranscriptRow: approval", () => {
|
|||||||
expect(el.querySelector(".st-approval-path")!.textContent).toBe("/etc/x");
|
expect(el.querySelector(".st-approval-path")!.textContent).toBe("/etc/x");
|
||||||
});
|
});
|
||||||
|
|
||||||
it("hides the buttons and shows an answered note once resolved", () => {
|
it("renders a compact one-liner once resolved", () => {
|
||||||
const el = buildTranscriptRow(approvalRow(allThreeOptions), {
|
const el = buildTranscriptRow(approvalRow(allThreeOptions), {
|
||||||
pending: false,
|
pending: false,
|
||||||
onDecide: () => Promise.resolve(),
|
onDecide: () => Promise.resolve(),
|
||||||
});
|
});
|
||||||
expect(el.querySelector(".st-approval-acts")).toBeNull();
|
expect(el.querySelector(".st-approval-acts")).toBeNull();
|
||||||
expect(el.querySelector(".st-approval-resolved")!.textContent).toBe("answered");
|
expect(el.classList.contains("st-approval-compact")).toBe(true);
|
||||||
|
expect(el.textContent).toContain("approved");
|
||||||
});
|
});
|
||||||
|
|
||||||
it("invokes onDecide with the clicked decision and disables the buttons", () => {
|
it("invokes onDecide with the clicked decision and disables the buttons", () => {
|
||||||
|
|||||||
+41
-37
@@ -704,6 +704,16 @@ function offeredDecisions(options: AcpPermissionOption[]): StructuredApprovalDec
|
|||||||
function buildApprovalRow(row: TranscriptRow, approval: ApprovalRowState): HTMLElement {
|
function buildApprovalRow(row: TranscriptRow, approval: ApprovalRowState): HTMLElement {
|
||||||
const request = (row.data ?? {}) as { toolCall?: AcpToolCallLike; options?: AcpPermissionOption[] };
|
const request = (row.data ?? {}) as { toolCall?: AcpToolCallLike; options?: AcpPermissionOption[] };
|
||||||
const toolCall = request.toolCall ?? {};
|
const toolCall = request.toolCall ?? {};
|
||||||
|
|
||||||
|
// Resolved approvals render as a compact one-liner inside a tool group
|
||||||
|
// (or are hidden entirely when standalone — see rebuildTranscript).
|
||||||
|
if (!approval.pending) {
|
||||||
|
const wrap = el("div", "st-row st-row-approval st-approval-compact");
|
||||||
|
const label = toolCall.title ?? toolKindLabel(toolCall.kind);
|
||||||
|
wrap.textContent = `${label} — approved`;
|
||||||
|
return wrap;
|
||||||
|
}
|
||||||
|
|
||||||
const wrap = el("div", "st-row st-row-approval");
|
const wrap = el("div", "st-row st-row-approval");
|
||||||
const card = el("div", "st-approval");
|
const card = el("div", "st-approval");
|
||||||
wrap.appendChild(card);
|
wrap.appendChild(card);
|
||||||
@@ -723,45 +733,39 @@ function buildApprovalRow(row: TranscriptRow, approval: ApprovalRowState): HTMLE
|
|||||||
card.appendChild(pathEl);
|
card.appendChild(pathEl);
|
||||||
}
|
}
|
||||||
|
|
||||||
if (approval.pending) {
|
const acts = el("div", "st-approval-acts");
|
||||||
const acts = el("div", "st-approval-acts");
|
const mk = (label: string, decision: StructuredApprovalDecision, cls: string) => {
|
||||||
const mk = (label: string, decision: StructuredApprovalDecision, cls: string) => {
|
const btn = document.createElement("button");
|
||||||
const btn = document.createElement("button");
|
btn.type = "button";
|
||||||
btn.type = "button";
|
btn.className = cls ? `act ${cls}` : "act";
|
||||||
btn.className = cls ? `act ${cls}` : "act";
|
btn.textContent = label;
|
||||||
btn.textContent = label;
|
btn.addEventListener("click", () => {
|
||||||
btn.addEventListener("click", () => {
|
const buttons = [...acts.querySelectorAll<HTMLButtonElement>("button")];
|
||||||
const buttons = [...acts.querySelectorAll<HTMLButtonElement>("button")];
|
buttons.forEach((b) => (b.disabled = true));
|
||||||
buttons.forEach((b) => (b.disabled = true));
|
approval.onDecide(decision).catch(() => {
|
||||||
approval.onDecide(decision).catch(() => {
|
// Rejects only when this approval is still pending (see
|
||||||
// Rejects only when this approval is still pending (see
|
// ApprovalRowState) — restore the controls so the operator can
|
||||||
// ApprovalRowState) — restore the controls so the operator can
|
// retry; the error itself is surfaced by the connection.
|
||||||
// retry; the error itself is surfaced by the connection.
|
buttons.forEach((b) => (b.disabled = false));
|
||||||
buttons.forEach((b) => (b.disabled = false));
|
|
||||||
});
|
|
||||||
});
|
});
|
||||||
return btn;
|
});
|
||||||
};
|
return btn;
|
||||||
const offered = offeredDecisions(request.options ?? []);
|
};
|
||||||
const specs: [StructuredApprovalDecision, string, string][] = [
|
const offered = offeredDecisions(request.options ?? []);
|
||||||
["allow_once", "Allow once", "primary"],
|
const specs: [StructuredApprovalDecision, string, string][] = [
|
||||||
["allow_for_session", "Allow for session", ""],
|
["allow_once", "Allow once", "primary"],
|
||||||
["deny", "Deny", "deny"],
|
["allow_for_session", "Allow for session", ""],
|
||||||
];
|
["deny", "Deny", "deny"],
|
||||||
for (const [decision, label, cls] of specs) {
|
];
|
||||||
if (offered.includes(decision)) acts.appendChild(mk(label, decision, cls));
|
for (const [decision, label, cls] of specs) {
|
||||||
}
|
if (offered.includes(decision)) acts.appendChild(mk(label, decision, cls));
|
||||||
if (acts.children.length > 0) {
|
}
|
||||||
card.appendChild(acts);
|
if (acts.children.length > 0) {
|
||||||
} else {
|
card.appendChild(acts);
|
||||||
const none = el("div", "st-approval-resolved");
|
|
||||||
none.textContent = "no decision offered";
|
|
||||||
card.appendChild(none);
|
|
||||||
}
|
|
||||||
} else {
|
} else {
|
||||||
const resolved = el("div", "st-approval-resolved");
|
const none = el("div", "st-approval-resolved");
|
||||||
resolved.textContent = "answered";
|
none.textContent = "no decision offered";
|
||||||
card.appendChild(resolved);
|
card.appendChild(none);
|
||||||
}
|
}
|
||||||
|
|
||||||
return wrap;
|
return wrap;
|
||||||
|
|||||||
@@ -3040,6 +3040,16 @@ input.leaf-rename:focus {
|
|||||||
color: var(--faint);
|
color: var(--faint);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/* compact resolved approval inside a tool group — same muted treatment
|
||||||
|
as intermediate agent text so the audit trail is preserved without
|
||||||
|
drawing attention to itself. */
|
||||||
|
.st-approval-compact {
|
||||||
|
color: var(--muted);
|
||||||
|
font-size: var(--fs-12);
|
||||||
|
padding: var(--sp-1) var(--sp-2);
|
||||||
|
font-style: italic;
|
||||||
|
}
|
||||||
|
|
||||||
.act {
|
.act {
|
||||||
appearance: none;
|
appearance: none;
|
||||||
font: inherit;
|
font: inherit;
|
||||||
|
|||||||
Reference in New Issue
Block a user