diff --git a/maps/feature-map/questions-and-approvals.md b/maps/feature-map/questions-and-approvals.md index 8c400bfd5c1..6f217864f50 100644 --- a/maps/feature-map/questions-and-approvals.md +++ b/maps/feature-map/questions-and-approvals.md @@ -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` diff --git a/ui/src/components/IssueThreadInteractionCard.test.tsx b/ui/src/components/IssueThreadInteractionCard.test.tsx index fd1953ac26b..25f6849efe3 100644 --- a/ui/src/components/IssueThreadInteractionCard.test.tsx +++ b/ui/src/components/IssueThreadInteractionCard.test.tsx @@ -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,52 @@ 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: '', 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.each(["argumentsSummaryJson", "argumentsHash"] as const)("rejects oversized whitespace before trimming %s", field => { + const interaction = structuredClone(pendingToolActionWriteInteraction); + const oversized = " ".repeat(field === "argumentsSummaryJson" ? 20_001 : 256); + interaction.payload.toolAction![field] = oversized; + const trim = String.prototype.trim; + let oversizedTrimCalls = 0; + const spy = vi.spyOn(String.prototype, "trim").mockImplementation(function (this: string) { + if (String(this) === oversized) oversizedTrimCalls++; + return trim.call(this); + }); + try { + const host = renderCard({ interaction }); + expect(host.querySelector("details")).toBeNull(); + expect(host.textContent).toContain("Request arguments are unavailable."); + expect(oversizedTrimCalls).toBe(0); + } finally { + spy.mockRestore(); + } + }); + 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"); diff --git a/ui/src/components/IssueThreadInteractionCard.tsx b/ui/src/components/IssueThreadInteractionCard.tsx index c3ba890e901..38281d787d1 100644 --- a/ui/src/components/IssueThreadInteractionCard.tsx +++ b/ui/src/components/IssueThreadInteractionCard.tsx @@ -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.length <= 20_000 && payload.argumentsSummaryJson.trim().length > 0 + && typeof payload.argumentsHash === "string" && payload.argumentsHash.length <= 255 + && payload.argumentsHash.trim().length > 0; + useEffect(() => { if (!isPending) setWorking(null); @@ -1828,6 +1834,17 @@ function RequestToolActionCard({ {!isPending ? : null} + {isPending ? inspectableArguments ? ( +
+ + Arguments and audit hash + + 
+            {payload.argumentsSummaryJson}
+
+

{payload.argumentsHash}

+
+ ) :

Request arguments are unavailable.

: null} {isPending ? (
diff --git a/ui/src/components/task-chat/TaskChatInteractionCard.test.tsx b/ui/src/components/task-chat/TaskChatInteractionCard.test.tsx index 2bd4ca44564..2640ae8a041 100644 --- a/ui/src/components/task-chat/TaskChatInteractionCard.test.tsx +++ b/ui/src/components/task-chat/TaskChatInteractionCard.test.tsx @@ -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 { @@ -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 setReviewing(true)} + onAcceptInteraction={accept} onRejectInteraction={reject} />; + } + flushSync(() => root.render()); + 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( diff --git a/ui/src/fixtures/issueThreadInteractionFixtures.ts b/ui/src/fixtures/issueThreadInteractionFixtures.ts index 8eea3363cb6..e4a49d57ab1 100644 --- a/ui/src/fixtures/issueThreadInteractionFixtures.ts +++ b/ui/src/fixtures/issueThreadInteractionFixtures.ts @@ -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: { diff --git a/ui/src/pages/apps/ReviewQueueCard.test.tsx b/ui/src/pages/apps/ReviewQueueCard.test.tsx index e8171650057..aa3766e3d6b 100644 --- a/ui/src/pages/apps/ReviewQueueCard.test.tsx +++ b/ui/src/pages/apps/ReviewQueueCard.test.tsx @@ -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(), 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( + + )); + 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 () => { diff --git a/ui/storybook/stories/questions-and-approvals/issue-thread-interactions.stories.tsx b/ui/storybook/stories/questions-and-approvals/issue-thread-interactions.stories.tsx index 9a33db7e2ed..3e981571a8a 100644 --- a/ui/storybook/stories/questions-and-approvals/issue-thread-interactions.stories.tsx +++ b/ui/storybook/stories/questions-and-approvals/issue-thread-interactions.stories.tsx @@ -35,6 +35,7 @@ import { notCreatorRequestConfirmationInteraction, pendingToolActionDestructiveInteraction, pendingToolActionWriteInteraction, + pendingToolActionLongProposalInteraction, runningToolActionInteraction, issueThreadInteractionComments, issueThreadInteractionEvents, @@ -918,7 +919,7 @@ export const ToolActionPendingWrite: Story = { ( + + + + + + ), +}; + export const ToolActionPendingDestructive: Story = { render: () => (