mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-04 11:01:16 +01:00
fix(ui): expose redacted arguments in pending tool reviews
Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
4461905b92
commit
17236e1736
7 files changed
+147
-6
No files matched your search
@@ -28,7 +28,7 @@ Agent access: [companion](../agent-access-map/questions-and-approvals.md).
|
||||
resolution. A visible request is not necessarily actionable by that viewer.
|
||||
- `governance`: `questions-and-approvals/governance` — pending/history views, approve/reject, revision request, and
|
||||
resubmission for formal board approvals.
|
||||
- `tool-review`: `questions-and-approvals/tool-review` — approve once, always allow the displayed scope, or decline;
|
||||
- `tool-review`: `questions-and-approvals/tool-review` — inspect the bounded server-redacted arguments and audit hash before approving once, always allowing the displayed scope, or declining;
|
||||
decision status and provider execution outcome remain distinct.
|
||||
- `continuation`: `questions-and-approvals/continuation` — one recorded response leads to the eligible waiting run or
|
||||
continuation; repeated clicks, reloads, and competing tabs do not repeat work.
|
||||
@@ -98,7 +98,7 @@ Use these checks alongside the entry-point recipes below and the operation recor
|
||||
| `questions-and-approvals/plan-review` | Open the exact requested plan revision; reject one request and accept another. Inspect saved revision, work mode, owner, and the resulting continuation. | Edit after requesting review and reject stale acceptance. Test finalization readiness and bound completion review separately. In Agent Chat, inspect the later project/task creation and its source-plan provenance; acceptance alone is not a completed handoff. |
|
||||
| `questions-and-approvals/audience` | Repeat a request for a named person, named agent, human-only resolver, and independent reviewer. Inspect actionability and final attribution. | Test creator exclusion, wrong audience/company, private task, missing or cancelled run, and changed responsible-user access. A human comment used by an agent remains subject to that agent’s resolver authority. |
|
||||
| `questions-and-approvals/governance` | Create and inspect a formal request, comment, request revision, resubmit, then approve or reject distinct requests. Reload history and linked task state. | Test board decision authority separately from agent request/comment/resubmit authority. A repeated decision cannot repeat activation. Current pending status, payload revision, and governed side effects must agree. |
|
||||
| `questions-and-approvals/tool-review` | Invoke a harmless Ask first action, compare task and Apps review surfaces, then exercise Approve once, Always allow, and Decline separately. | No provider call before approval. Repeat decisions and compare invocation count. Change the tool definition, revoke access, or set Off before a decision. Record the approval decision and provider outcome independently. |
|
||||
| `questions-and-approvals/tool-review` | Invoke a harmless Ask first action with a proposal longer than its preview. Open Arguments and audit hash on task and Apps review surfaces and inspect the tail before a decision. Then exercise Approve once, Always allow, and Decline separately. | No provider call before approval. Repeat decisions and compare invocation count. Change the tool definition, revoke access, or set Off before a decision. Record the approval decision and provider outcome independently. |
|
||||
| `questions-and-approvals/continuation` | After answering, inspect the saved response, delivery receipt, eligible run, and useful result in the original task/thread. | Reload, race two tabs, and interrupt delivery after saving. Require one continuation without repeated external actions. Reassignment, Stop, pause, budget, and external-thread boundaries remain effective. |
|
||||
|
||||
### `task-thread`
|
||||
|
||||
@@ -1263,7 +1263,7 @@ describe("IssueThreadInteractionCard", () => {
|
||||
});
|
||||
|
||||
describe("IssueThreadInteractionCard tool-action card", () => {
|
||||
it("shows only the request description, app icon, and decision controls", () => {
|
||||
it("keeps the request description and decisions primary with optional argument inspection", () => {
|
||||
const host = renderCard({ interaction: pendingToolActionWriteInteraction, onAcceptInteraction: vi.fn(), onRejectInteraction: vi.fn() });
|
||||
expect(host.textContent).toContain("Add 1 row");
|
||||
expect(host.querySelector('[role="img"]')?.getAttribute("aria-label")).toBe("Google Sheets");
|
||||
@@ -1282,6 +1282,32 @@ describe("IssueThreadInteractionCard tool-action card", () => {
|
||||
expect(host.textContent).toContain("Launch checklist");
|
||||
});
|
||||
|
||||
it("renders only the server-redacted proposal as plain text when inspected", async () => {
|
||||
const interaction = structuredClone(pendingToolActionWriteInteraction);
|
||||
interaction.payload.toolAction!.argumentsSummaryJson = JSON.stringify({ text: '<img src="x" onerror="alert(1)">', apiKey: "[redacted]" });
|
||||
const accept = vi.fn();
|
||||
const host = renderCard({ interaction, onAcceptInteraction: accept });
|
||||
const details = host.querySelector("details")!;
|
||||
expect(details.open).toBe(false);
|
||||
await act(async () => details.querySelector("summary")!.click());
|
||||
expect(details.open).toBe(true);
|
||||
expect(details.querySelector("pre")?.textContent).toBe(interaction.payload.toolAction!.argumentsSummaryJson);
|
||||
expect(details.querySelector("img")).toBeNull();
|
||||
expect(details.textContent).toContain("[redacted]");
|
||||
expect(accept).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it.each([undefined, { secret: "must not render" }, "x".repeat(20_001)])("withholds missing, malformed or oversized proposal details (%#)", summary => {
|
||||
const interaction = structuredClone(pendingToolActionWriteInteraction);
|
||||
Object.assign(interaction.payload.toolAction!, { argumentsSummaryJson: summary });
|
||||
const host = renderCard({ interaction });
|
||||
expect(host.querySelector("details")).toBeNull();
|
||||
expect(host.textContent).toContain("Request arguments are unavailable.");
|
||||
expect(host.textContent).not.toContain("must not render");
|
||||
expect(host.querySelector("pre")).toBeNull();
|
||||
expect(Array.from(host.querySelectorAll("button")).find(button => button.textContent === "Approve & run")?.disabled).toBe(true);
|
||||
});
|
||||
|
||||
it("keeps the destructive request warning and approval styling", () => {
|
||||
const host = renderCard({ interaction: pendingToolActionDestructiveInteraction, onAcceptInteraction: vi.fn(), onRejectInteraction: vi.fn() });
|
||||
expect(host.textContent).toContain("cannot be undone");
|
||||
|
||||
@@ -1799,6 +1799,12 @@ function RequestToolActionCard({
|
||||
const resolutionErrorMessage = useResolutionErrorMessage();
|
||||
const isPending = state === "pending";
|
||||
const variant = payload.risk === "destructive" ? "destructive" : "cta";
|
||||
// This server projection is already redacted and bounded by the interaction schema.
|
||||
const inspectableArguments = typeof payload.argumentsSummaryJson === "string"
|
||||
&& payload.argumentsSummaryJson.trim().length > 0 && payload.argumentsSummaryJson.length <= 20_000
|
||||
&& typeof payload.argumentsHash === "string" && payload.argumentsHash.trim().length > 0
|
||||
&& payload.argumentsHash.length <= 255;
|
||||
|
||||
|
||||
useEffect(() => {
|
||||
if (!isPending) setWorking(null);
|
||||
@@ -1828,6 +1834,17 @@ function RequestToolActionCard({
|
||||
{!isPending ? <ToolActionResolution state={state} interaction={interaction} /> : null}
|
||||
</div>
|
||||
</div>
|
||||
{isPending ? inspectableArguments ? (
|
||||
<details className="mt-3">
|
||||
<summary className="cursor-pointer text-xs font-medium text-muted-foreground">
|
||||
Arguments and audit hash
|
||||
</summary>
|
||||
<pre className="mt-2 max-h-40 overflow-auto whitespace-pre-wrap break-words font-mono text-xs text-foreground">
|
||||
{payload.argumentsSummaryJson}
|
||||
</pre>
|
||||
<p className="mt-1 break-all font-mono text-xs text-muted-foreground">{payload.argumentsHash}</p>
|
||||
</details>
|
||||
) : <p className="mt-3 text-xs text-muted-foreground">Request arguments are unavailable.</p> : null}
|
||||
{isPending ? (
|
||||
<DecisionCardFooter leading={footerSlot} className="mt-3">
|
||||
<div className={cn(!footerSlot && "mt-3", "flex flex-wrap items-center justify-end gap-2")}>
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
// @vitest-environment jsdom
|
||||
|
||||
import { act, forwardRef, useImperativeHandle, type ForwardedRef } from "react";
|
||||
import { act, forwardRef, useImperativeHandle, useState, type ForwardedRef } from "react";
|
||||
import { flushSync } from "react-dom";
|
||||
import { createRoot, type Root } from "react-dom/client";
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
@@ -13,6 +13,7 @@ import { ThemeProvider } from "@/context/ThemeContext";
|
||||
import { TooltipProvider } from "@/components/ui/tooltip";
|
||||
import {
|
||||
expiredSecretProposalInteraction,
|
||||
pendingToolActionLongProposalInteraction,
|
||||
pendingAskUserQuestionsInteraction,
|
||||
pendingRequestCheckboxConfirmationInteraction,
|
||||
pendingRequestItemVerdictsInteraction,
|
||||
@@ -56,6 +57,8 @@ vi.mock("@/components/MarkdownEditor", () => ({
|
||||
}),
|
||||
}));
|
||||
|
||||
(globalThis as typeof globalThis & { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true;
|
||||
|
||||
function createRequestConfirmation(
|
||||
overrides: Partial<RequestConfirmationInteraction> = {},
|
||||
): RequestConfirmationInteraction {
|
||||
@@ -118,6 +121,40 @@ describe("TaskChatInteractionCard", () => {
|
||||
container.remove();
|
||||
});
|
||||
|
||||
it("opens the real tool review and reveals the proposal tail before approval", async () => {
|
||||
const accept = vi.fn();
|
||||
const reject = vi.fn();
|
||||
const interaction = structuredClone(pendingToolActionLongProposalInteraction);
|
||||
function ReviewSurface() {
|
||||
const [reviewing, setReviewing] = useState(false);
|
||||
return <TaskChatInteractionCard item={interactionItem(interaction)}
|
||||
presentation={reviewing ? "takeover" : "timeline"}
|
||||
onReviewRequest={() => setReviewing(true)}
|
||||
onAcceptInteraction={accept} onRejectInteraction={reject} />;
|
||||
}
|
||||
flushSync(() => root.render(<TooltipProvider><ThemeProvider><ReviewSurface /></ThemeProvider></TooltipProvider>));
|
||||
expect(container.textContent).not.toContain("November 8, 2030");
|
||||
await act(async () => {
|
||||
Array.from(container.querySelectorAll("button")).find(button => button.textContent === "Review request")!.click();
|
||||
});
|
||||
const details = container.querySelector("details")!;
|
||||
expect(details).not.toBeNull();
|
||||
expect(details.open).toBe(false);
|
||||
expect(container.querySelector("pre")?.textContent).toBe(interaction.payload.toolAction!.argumentsSummaryJson);
|
||||
expect(details.textContent).toContain("November 8, 2030");
|
||||
expect(details.textContent).toContain("[redacted]");
|
||||
expect(details.textContent).toContain(interaction.payload.toolAction!.argumentsHash);
|
||||
await act(async () => details.querySelector("summary")!.click());
|
||||
expect(details.open).toBe(true);
|
||||
expect(accept).not.toHaveBeenCalled();
|
||||
expect(reject).not.toHaveBeenCalled();
|
||||
await act(async () => {
|
||||
Array.from(container.querySelectorAll("button")).find(button => button.textContent === "Approve & run")!.click();
|
||||
});
|
||||
expect(accept).toHaveBeenCalledExactlyOnceWith(interaction, undefined, undefined, false);
|
||||
expect(reject).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("does not render a pending confirmation in the timeline", () => {
|
||||
flushSync(() => {
|
||||
root.render(
|
||||
|
||||
@@ -656,6 +656,16 @@ export const pendingToolActionWriteInteraction = createToolActionConfirmationInt
|
||||
id: "interaction-tool-action-pending-write",
|
||||
});
|
||||
|
||||
// A legitimate humanized preview is shorter than the redacted argument body.
|
||||
const longToolProposal = "The release note covers the agreed accessibility checklist, rollout owners, support contacts, and readiness review for the public launch. Launch date: November 8, 2030.";
|
||||
export const pendingToolActionLongProposalInteraction = createToolActionConfirmationInteraction({
|
||||
id: "interaction-tool-action-long-proposal",
|
||||
toolAction: {
|
||||
previewMarkdown: `Add the public launch note.\n\n**Text:** ${longToolProposal.slice(0, 137)}…`,
|
||||
argumentsSummaryJson: JSON.stringify({ text: longToolProposal, apiKey: "[redacted]" }, null, 2),
|
||||
},
|
||||
});
|
||||
|
||||
export const pendingToolActionDestructiveInteraction = createToolActionConfirmationInteraction({
|
||||
id: "interaction-tool-action-pending-destructive",
|
||||
toolAction: {
|
||||
|
||||
@@ -5,8 +5,12 @@ import { createRoot } from "react-dom/client";
|
||||
import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import { ReviewQueueCard } from "./ReviewQueueCard";
|
||||
import { pendingToolActionLongProposalInteraction } from "@/fixtures/issueThreadInteractionFixtures";
|
||||
import { ThemeProvider } from "@/context/ThemeContext";
|
||||
import { TooltipProvider } from "@/components/ui/tooltip";
|
||||
|
||||
const listActionRequestsMock = vi.hoisted(() => vi.fn());
|
||||
const listInteractionsMock = vi.hoisted(() => vi.fn());
|
||||
const approveActionRequestMock = vi.hoisted(() => vi.fn());
|
||||
const createTrustRuleFromActionRequestMock = vi.hoisted(() => vi.fn());
|
||||
const pushToastMock = vi.hoisted(() => vi.fn());
|
||||
@@ -21,7 +25,12 @@ vi.mock("@/api/tools", () => ({
|
||||
},
|
||||
}));
|
||||
|
||||
vi.mock("@/context/CompanyContext", () => ({
|
||||
vi.mock("@/api/issues", () => ({
|
||||
issuesApi: { listInteractions: (...args: unknown[]) => listInteractionsMock(...args) },
|
||||
}));
|
||||
|
||||
vi.mock("@/context/CompanyContext", async importOriginal => ({
|
||||
...await importOriginal<typeof import("@/context/CompanyContext")>(),
|
||||
useCompany: () => ({
|
||||
selectedCompanyId: "company-1",
|
||||
selectedCompany: { id: "company-1", name: "Paperclip" },
|
||||
@@ -123,6 +132,34 @@ describe("ReviewQueueCard", () => {
|
||||
await flushReact();
|
||||
}
|
||||
|
||||
it("shows the linked interaction's redacted proposal tail in Apps before a decision", async () => {
|
||||
const interaction = structuredClone(pendingToolActionLongProposalInteraction);
|
||||
const item = pendingRequest();
|
||||
const linked = { ...item, request: { ...item.request, interactionId: interaction.id } };
|
||||
listActionRequestsMock.mockResolvedValue({ actionRequests: [linked] });
|
||||
listInteractionsMock.mockResolvedValue([interaction]);
|
||||
const client = new QueryClient({ defaultOptions: { queries: { retry: false } } });
|
||||
root = createRoot(container);
|
||||
await act(async () => root.render(<QueryClientProvider client={client}><TooltipProvider><ThemeProvider>
|
||||
<ReviewQueueCard emptyState="reassure" />
|
||||
</ThemeProvider></TooltipProvider></QueryClientProvider>));
|
||||
await flushReact();
|
||||
await vi.waitFor(() => expect(container.querySelector("details")).not.toBeNull());
|
||||
const details = container.querySelector("details")!;
|
||||
expect(details).not.toBeNull();
|
||||
expect(details.open).toBe(false);
|
||||
expect(details.textContent).toContain("November 8, 2030");
|
||||
expect(details.querySelector("pre")?.textContent).toBe(interaction.payload.toolAction!.argumentsSummaryJson);
|
||||
await act(async () => details.querySelector("summary")!.click());
|
||||
expect(details.open).toBe(true);
|
||||
expect(approveActionRequestMock).not.toHaveBeenCalled();
|
||||
expect(createTrustRuleFromActionRequestMock).not.toHaveBeenCalled();
|
||||
expect(listInteractionsMock).toHaveBeenCalledWith("issue-1");
|
||||
await act(async () => { buttonContaining("Approve & run")!.click(); });
|
||||
await vi.waitFor(() => expect(approveActionRequestMock).toHaveBeenCalledExactlyOnceWith("company-1", "request-1", false));
|
||||
expect(createTrustRuleFromActionRequestMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("submits approval and remembered permission as one atomic decision", async () => {
|
||||
const calls: string[] = [];
|
||||
approveActionRequestMock.mockImplementation(async () => {
|
||||
|
||||
@@ -35,6 +35,7 @@ import {
|
||||
notCreatorRequestConfirmationInteraction,
|
||||
pendingToolActionDestructiveInteraction,
|
||||
pendingToolActionWriteInteraction,
|
||||
pendingToolActionLongProposalInteraction,
|
||||
runningToolActionInteraction,
|
||||
issueThreadInteractionComments,
|
||||
issueThreadInteractionEvents,
|
||||
@@ -918,7 +919,7 @@ export const ToolActionPendingWrite: Story = {
|
||||
<StoryFrame>
|
||||
<ScenarioCard
|
||||
title="Pending · write"
|
||||
description="A write tool call awaits approval: identity header, WRITE risk badge, humanized preview, collapsible technical details, expiry countdown, and an Approve & run CTA."
|
||||
description="A write tool call awaits approval: app identity, a short preview, optional redacted arguments and audit hash, and Approve & run or Decline."
|
||||
>
|
||||
<ToolActionCard
|
||||
interaction={pendingToolActionWriteInteraction}
|
||||
@@ -929,6 +930,19 @@ export const ToolActionPendingWrite: Story = {
|
||||
),
|
||||
};
|
||||
|
||||
export const ToolActionLongProposal: Story = {
|
||||
render: () => (
|
||||
<StoryFrame>
|
||||
<ScenarioCard
|
||||
title="Pending · proposal beyond the short preview"
|
||||
description="Open Arguments and audit hash to inspect the server-redacted proposal, including its final launch date, before approving. Inspection does not submit a decision."
|
||||
>
|
||||
<ToolActionCard interaction={pendingToolActionLongProposalInteraction} interactive />
|
||||
</ScenarioCard>
|
||||
</StoryFrame>
|
||||
),
|
||||
};
|
||||
|
||||
export const ToolActionPendingDestructive: Story = {
|
||||
render: () => (
|
||||
<StoryFrame>
|
||||
|
||||
Reference in new issue
Block a user