From adf8e254ab810a2d2cbf67afeb53c5bdf2bc948a Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Wed, 12 Aug 2026 15:58:55 -0700 Subject: [PATCH] fix(ui): let read-only operators copy task results (#122841) * fix(ui): let read-only operators copy task results * refactor(ui): consolidate operator scope checks --- ui/src/app/operator-access.test.ts | 12 ++++ ui/src/app/operator-access.ts | 88 ++++++++++++--------------- ui/src/pages/tasks/tasks-page.test.ts | 60 +++++++++++++++++- ui/src/pages/tasks/tasks-page.ts | 5 +- ui/src/pages/tasks/tasks.e2e.test.ts | 79 ++++++++++++++++++++++++ ui/src/pages/tasks/view.ts | 45 +++++++------- 6 files changed, 212 insertions(+), 77 deletions(-) diff --git a/ui/src/app/operator-access.test.ts b/ui/src/app/operator-access.test.ts index a5c7cc9782e7..70c11f6ea495 100644 --- a/ui/src/app/operator-access.test.ts +++ b/ui/src/app/operator-access.test.ts @@ -4,6 +4,7 @@ import type { ApplicationGatewaySnapshot } from "./gateway.ts"; import { hasOperatorApprovalsAccess, hasOperatorPairingAccess, + hasOperatorReadAccess, readGatewayOperatorAccess, } from "./operator-access.ts"; @@ -72,6 +73,17 @@ describe("readGatewayOperatorAccess", () => { }); }); +describe("hasOperatorReadAccess", () => { + it("accepts read, implied write/admin, and legacy access but rejects unrelated scopes", () => { + expect(hasOperatorReadAccess(null)).toBe(true); + expect(hasOperatorReadAccess({ role: "operator", scopes: ["operator.read"] })).toBe(true); + expect(hasOperatorReadAccess({ role: "operator", scopes: ["operator.write"] })).toBe(true); + expect(hasOperatorReadAccess({ role: "operator", scopes: ["operator.admin"] })).toBe(true); + expect(hasOperatorReadAccess({ role: "operator" })).toBe(true); + expect(hasOperatorReadAccess({ role: "operator", scopes: ["operator.pairing"] })).toBe(false); + }); +}); + describe("hasOperatorPairingAccess", () => { it("requires pairing scope while keeping admin and legacy auth compatible", () => { expect(hasOperatorPairingAccess(null)).toBe(false); diff --git a/ui/src/app/operator-access.ts b/ui/src/app/operator-access.ts index 70284593a9b3..247b594ec5d7 100644 --- a/ui/src/app/operator-access.ts +++ b/ui/src/app/operator-access.ts @@ -10,6 +10,32 @@ type GatewayOperatorAccess = Readonly<{ canGrantApprovals: boolean; }>; +type OperatorAuth = { role?: string; scopes?: readonly string[] } | null; +type OperatorScope = + | "operator.read" + | "operator.write" + | "operator.admin" + | "operator.pairing" + | "operator.approvals"; + +function hasOperatorScope( + auth: OperatorAuth, + requestedScope: OperatorScope, + missingAuthHasAccess: boolean, +): boolean { + if (!auth) { + return missingAuthHasAccess; + } + if (!auth.scopes) { + return true; + } + return roleScopesAllow({ + role: auth.role ?? "operator", + requestedScopes: [requestedScope], + allowedScopes: auth.scopes, + }); +} + export function readGatewayOperatorAccess( snapshot: Pick | null | undefined, ): GatewayOperatorAccess { @@ -25,60 +51,22 @@ export function readGatewayOperatorAccess( }; } -export function hasOperatorWriteAccess( - auth: { role?: string; scopes?: readonly string[] } | null, -): boolean { - if (!auth?.scopes) { - return true; - } - return roleScopesAllow({ - role: auth.role ?? "operator", - requestedScopes: ["operator.write"], - allowedScopes: auth.scopes, - }); +export function hasOperatorWriteAccess(auth: OperatorAuth): boolean { + return hasOperatorScope(auth, "operator.write", true); } -export function hasOperatorAdminAccess( - auth: { role?: string; scopes?: readonly string[] } | null, -): boolean { - if (!auth?.scopes) { - return true; - } - return roleScopesAllow({ - role: auth.role ?? "operator", - requestedScopes: ["operator.admin"], - allowedScopes: auth.scopes, - }); +export function hasOperatorReadAccess(auth: OperatorAuth): boolean { + return hasOperatorScope(auth, "operator.read", true); } -export function hasOperatorPairingAccess( - auth: { role?: string; scopes?: readonly string[] } | null, -): boolean { - if (!auth) { - return false; - } - if (!auth.scopes) { - return true; - } - return roleScopesAllow({ - role: auth.role ?? "operator", - requestedScopes: ["operator.pairing"], - allowedScopes: auth.scopes, - }); +export function hasOperatorAdminAccess(auth: OperatorAuth): boolean { + return hasOperatorScope(auth, "operator.admin", true); } -export function hasOperatorApprovalsAccess( - auth: { role?: string; scopes?: readonly string[] } | null, -): boolean { - if (!auth) { - return false; - } - if (!auth.scopes) { - return true; - } - return roleScopesAllow({ - role: auth.role ?? "operator", - requestedScopes: ["operator.approvals"], - allowedScopes: auth.scopes, - }); +export function hasOperatorPairingAccess(auth: OperatorAuth): boolean { + return hasOperatorScope(auth, "operator.pairing", false); +} + +export function hasOperatorApprovalsAccess(auth: OperatorAuth): boolean { + return hasOperatorScope(auth, "operator.approvals", false); } diff --git a/ui/src/pages/tasks/tasks-page.test.ts b/ui/src/pages/tasks/tasks-page.test.ts index 095d40d03a74..ddd60ad3b0ce 100644 --- a/ui/src/pages/tasks/tasks-page.test.ts +++ b/ui/src/pages/tasks/tasks-page.test.ts @@ -23,13 +23,16 @@ function deferred() { return { promise, resolve }; } -function createGateway(client: GatewayBrowserClient) { +function createGateway( + client: GatewayBrowserClient, + hello: ApplicationGatewaySnapshot["hello"] = null, +) { const snapshot: ApplicationGatewaySnapshot = { client, phase: "connected", offlineStable: false, canvasPluginSurfaceUrl: null, - hello: null, + hello, assistantAgentId: null, sessionKey: "main", lastError: null, @@ -420,6 +423,59 @@ describe("TasksPage active pagination", () => { }); describe("TasksPage cancellation lifecycle", () => { + it("lets a read-only operator copy a retained result without mutation controls", async () => { + const retained = createTask("task-read-only-retained", "completed", { + deliveryStatus: "failed", + terminalOutcome: "blocked", + terminalSummary: "Synthetic retained task completed.", + }); + const copiedResult = "Synthetic retained result for read-only operator proof."; + const request = vi.fn((method: string) => + Promise.resolve( + method === "tasks.get" + ? { task: { ...retained, result: copiedResult } } + : { tasks: [retained] }, + ), + ); + const source = createGateway( + { request } as unknown as GatewayBrowserClient, + { + auth: { role: "operator", scopes: ["operator.read"] }, + } as ApplicationGatewaySnapshot["hello"], + ); + const page = document.createElement("openclaw-tasks-page") as TasksPageTestElement; + page.context = createContext(source.gateway); + const writeText = vi.fn(async () => undefined); + const originalClipboard = Object.getOwnPropertyDescriptor(navigator, "clipboard"); + Object.defineProperty(navigator, "clipboard", { + configurable: true, + value: { writeText }, + }); + try { + document.body.append(page); + await vi.waitFor(() => expect(page.tasks).toHaveLength(1)); + + const copyButton = [...page.querySelectorAll("button")].find( + (button) => button.textContent?.trim() === "Copy result", + ); + expect(copyButton).toBeDefined(); + const text = page.textContent ?? ""; + expect(text).not.toContain("Retry delivery"); + expect(text).not.toContain("Dismiss delivery"); + expect(text).not.toContain("Cancel"); + + copyButton?.click(); + await vi.waitFor(() => expect(writeText).toHaveBeenCalledWith(copiedResult)); + expect(request).toHaveBeenCalledWith("tasks.get", { taskId: retained.taskId }); + } finally { + if (originalClipboard) { + Object.defineProperty(navigator, "clipboard", originalClipboard); + } else { + Reflect.deleteProperty(navigator, "clipboard"); + } + } + }); + it("qualifies unscoped task session links with the selected agent", async () => { const request = vi.fn(async () => ({ tasks: [ diff --git a/ui/src/pages/tasks/tasks-page.ts b/ui/src/pages/tasks/tasks-page.ts index cb908cafa4b9..e9c10768b738 100644 --- a/ui/src/pages/tasks/tasks-page.ts +++ b/ui/src/pages/tasks/tasks-page.ts @@ -5,7 +5,7 @@ import { state } from "lit/decorators.js"; import type { GatewayBrowserClient } from "../../api/gateway.ts"; import { titleForRoute } from "../../app-navigation.ts"; import { applicationContext, type ApplicationContext } from "../../app/context.ts"; -import { hasOperatorWriteAccess } from "../../app/operator-access.ts"; +import { hasOperatorReadAccess, hasOperatorWriteAccess } from "../../app/operator-access.ts"; import { renderAgentScopeControl } from "../../components/agent-scope-control.ts"; import { t } from "../../i18n/index.ts"; import { watchAgentScope } from "../../lib/agents/index.ts"; @@ -404,7 +404,8 @@ class TasksPage extends OpenClawLightDomElement { hello: this.context.gateway.snapshot.hello, }), connected: this.gateway.connected, - // tasks.cancel needs operator.write; read-only operators get no button. + canCopy: hasOperatorReadAccess(this.context.gateway.snapshot.hello?.auth ?? null), + // Task mutations need operator.write; read-only operators get no mutation buttons. canCancel: hasOperatorWriteAccess(this.context.gateway.snapshot.hello?.auth ?? null), loading: this.listTask.status === TaskStatus.PENDING, error: this.error, diff --git a/ui/src/pages/tasks/tasks.e2e.test.ts b/ui/src/pages/tasks/tasks.e2e.test.ts index 177c519a0943..e5c6f4fce3ee 100644 --- a/ui/src/pages/tasks/tasks.e2e.test.ts +++ b/ui/src/pages/tasks/tasks.e2e.test.ts @@ -65,6 +65,23 @@ const failedTask = { error: "Worker exited", }; +const readOnlyRetainedTask = { + id: "synthetic-retained-task", + taskId: "synthetic-retained-task", + kind: "subagent", + runtime: "subagent", + status: "completed", + title: "Sanitized retained task", + agentId: "main", + createdAt: baseTime - 60_000, + updatedAt: baseTime - 50_000, + deliveryStatus: "dismissed", + terminalOutcome: "blocked", + terminalSummary: "Synthetic task completed; delivery was dismissed.", +}; + +const readOnlyRetainedResult = "Synthetic retained result copied by a read-only operator."; + const pageTwoSentinel = { id: "task-page-two-sentinel", taskId: "task-page-two-sentinel", @@ -229,4 +246,66 @@ suite.define(() => { await rm(rawVideoDir, { force: true, recursive: true }); } }); + + it("lets an operator.read-only user copy a retained result without mutations", async () => { + await mkdir(artifactDir, { recursive: true }); + const context = await suite.browser.newContext({ + locale: "en-US", + serviceWorkers: "block", + viewport: { width: 1440, height: 900 }, + }); + await context.grantPermissions(["clipboard-read", "clipboard-write"], { + origin: new URL(suite.server.baseUrl).origin, + }); + const page = await context.newPage(); + try { + const gateway = await installMockGateway(page, { + operatorScopes: ["operator.read"], + methodResponses: { + "tasks.list": { + cases: [ + { + match: { agentId: "main", limit: 500, status: ["queued", "running"] }, + response: { tasks: [] }, + }, + { + match: { agentId: "main", limit: 200 }, + response: { tasks: [readOnlyRetainedTask] }, + }, + ], + }, + "tasks.get": { + task: { ...readOnlyRetainedTask, result: readOnlyRetainedResult }, + }, + }, + }); + + const response = await page.goto(`${suite.server.baseUrl}tasks`); + expect(response?.status()).toBe(200); + const task = page.locator('[data-task-id="synthetic-retained-task"]'); + await task.waitFor({ state: "visible" }); + await task.scrollIntoViewIfNeeded(); + expect(await task.textContent()).toContain("Completed; result delivery was dismissed."); + expect(await task.getByRole("button", { name: "Retry delivery" }).count()).toBe(0); + expect(await task.getByRole("button", { name: "Dismiss delivery" }).count()).toBe(0); + expect(await task.getByRole("button", { name: /Cancel/ }).count()).toBe(0); + await page.screenshot({ + path: path.join(artifactDir, "04-read-only-retained-result.png"), + }); + + const copyButton = task.getByRole("button", { name: "Copy result" }); + await copyButton.waitFor({ state: "visible" }); + await copyButton.click(); + const getRequest = await gateway.waitForRequest("tasks.get"); + expect(getRequest.params).toEqual({ taskId: readOnlyRetainedTask.taskId }); + await expect + .poll(() => page.evaluate(() => navigator.clipboard.readText())) + .toBe(readOnlyRetainedResult); + expect(await gateway.getRequests("tasks.retry")).toHaveLength(0); + expect(await gateway.getRequests("tasks.dismiss")).toHaveLength(0); + expect(await gateway.getRequests("tasks.cancel")).toHaveLength(0); + } finally { + await context.close(); + } + }); }); diff --git a/ui/src/pages/tasks/view.ts b/ui/src/pages/tasks/view.ts index 55684f33065c..2e5c01e91c3c 100644 --- a/ui/src/pages/tasks/view.ts +++ b/ui/src/pages/tasks/view.ts @@ -25,6 +25,7 @@ type TasksProps = { agentId: string; mainKey: string; connected: boolean; + canCopy: boolean; canCancel: boolean; loading: boolean; error: string | null; @@ -115,36 +116,34 @@ function renderTask(task: TaskSummary, props: TasksProps) { ${cancelling ? t("tasksPage.cancelling") : t("common.cancel")} ` : nothing} - ${retainedResult && props.canCancel + ${retainedResult && props.canCopy + ? html`` + : nothing} + ${recoverableDelivery && props.canCancel ? html` + - ${recoverableDelivery - ? html` - - - ` - : nothing} ` : nothing}