From a4358fb3dc8ebb1b8a0ec3a252317bce4719a941 Mon Sep 17 00:00:00 2001 From: Rhys Sullivan Date: Fri, 12 Jun 2026 11:34:41 -0700 Subject: [PATCH 1/2] Add browser e2e for tool-tree policy menus (per-tool + category) An OpenAPI integration with two connected accounts drives the account-grouped tool tree: the per-tool row menu blocks one tool, the category row menu sets require_approval on the group, and the scenario pins the connection-wildcarded patterns both menus surface, indicator precedence (leaf rule over category rule) across both accounts, the Clear affordance, the /policies rows, and the persisted server-side owner/pattern/action ordering. --- e2e/scenarios/policies-ui.test.ts | 234 ++++++++++++++++++++++++++++++ 1 file changed, 234 insertions(+) create mode 100644 e2e/scenarios/policies-ui.test.ts diff --git a/e2e/scenarios/policies-ui.test.ts b/e2e/scenarios/policies-ui.test.ts new file mode 100644 index 000000000..c5be589ab --- /dev/null +++ b/e2e/scenarios/policies-ui.test.ts @@ -0,0 +1,234 @@ +// Cross-target (browser): authoring tool policies from the integration's tool +// tree. An OpenAPI integration with two connected accounts renders the +// account-grouped tree; the per-tool row menu writes an exact-tool rule and +// the category (group) row menu writes a subtree rule. The product promises +// under test: +// +// 1. Both menus surface the REAL stored pattern (connection-wildcarded +// `integration.*.*.tool`) before anything is written. +// 2. A leaf rule and a category rule coexist: the more specific leaf rule +// keeps precedence over the later category rule, which covers the rest +// of its group. +// 3. Rules are connection-agnostic: set from one account's section, they +// govern the other account's rows too, and the menu there shows the +// active rule with a Clear option. +// 4. The rules materialize as manageable rows on /policies and persist +// server-side with exactly the owner/pattern/action the UI promised. +import { randomBytes } from "node:crypto"; + +import { expect } from "@effect/vitest"; +import { Effect } from "effect"; +import { composePluginApi } from "@executor-js/api/server"; +import { openApiHttpPlugin } from "@executor-js/plugin-openapi/api"; +import { AuthTemplateSlug, ConnectionName, IntegrationSlug } from "@executor-js/sdk/shared"; + +import { scenario } from "../src/scenario"; +import { Api, Browser, Target } from "../src/services"; + +const api = composePluginApi([openApiHttpPlugin()] as const); + +const TEMPLATE_API_KEY = AuthTemplateSlug.make("apiKey"); + +/** Two tagged groups so the tree renders a `records` category (two leaves) + * next to an unrelated `checks` category the rules must not touch. Tag → + * group segment, operationId → leaf segment: `records.list`, + * `records.create`, `checks.ping`. Never contacted over the network. */ +const recordsSpec = JSON.stringify({ + openapi: "3.0.3", + info: { title: "Records API", version: "1.0.0" }, + paths: { + "/records": { + get: { + operationId: "list", + tags: ["records"], + summary: "List records", + responses: { "200": { description: "ok" } }, + }, + post: { + operationId: "create", + tags: ["records"], + summary: "Create a record", + responses: { "200": { description: "ok" } }, + }, + }, + "/checks": { + get: { + operationId: "ping", + tags: ["checks"], + summary: "Ping", + responses: { "200": { description: "ok" } }, + }, + }, + }, +}); + +scenario( + "Policies · the tool tree's per-tool menu and category menu both author working rules", + { timeout: 180_000 }, + Effect.gen(function* () { + const target = yield* Target; + const browser = yield* Browser; + const { client: apiClient } = yield* Api; + + const identity = yield* target.newIdentity(); + const client = yield* apiClient(api, identity); + + const suffix = randomBytes(4).toString("hex"); + const integration = IntegrationSlug.make(`polui${suffix}`); + const alpha = ConnectionName.make(`alpha${suffix}`); + const beta = ConnectionName.make(`beta${suffix}`); + const accounts = [alpha, beta] as const; + + // The UI hides owner/connection segments; a rule authored on a node is + // stored connection-wildcarded so it spans every account. + const leafPattern = `${integration}.*.*.records.create`; + const categoryPattern = `${integration}.*.*.records.*`; + + // Selfhost scenarios share one workspace — remove everything this one + // made (policies, connections, the integration) even on failure. + const cleanup = Effect.gen(function* () { + const policies = yield* client.policies.list(); + yield* Effect.forEach( + policies.filter((p) => p.pattern.startsWith(`${integration}.`)), + (p) => + client.policies + .remove({ params: { policyId: p.id }, payload: { owner: p.owner } }) + .pipe(Effect.ignore), + ); + yield* Effect.forEach(accounts, (name) => + client.connections + .remove({ params: { owner: "org", integration, name } }) + .pipe(Effect.ignore), + ); + yield* client.openapi.removeSpec({ params: { slug: integration } }); + }).pipe(Effect.ignore); + + yield* Effect.gen(function* () { + // An integration plus two connected accounts: tools materialize per + // connection, so the Tools tab groups the tree by account. + yield* client.openapi.addSpec({ + payload: { + spec: { kind: "blob", value: recordsSpec }, + slug: integration, + baseUrl: "http://127.0.0.1:59999", // never contacted — tools derive from the spec + authenticationTemplate: [ + { + slug: "apiKey", + type: "apiKey", + headers: { authorization: ["Bearer ", { type: "variable", name: "token" }] }, + }, + ], + }, + }); + yield* Effect.forEach(accounts, (name) => + client.connections.create({ + payload: { + owner: "org", + name, + integration, + template: TEMPLATE_API_KEY, + identityLabel: `${name} key`, + value: `sk-${name}`, + }, + }), + ); + + yield* browser.session(identity, async ({ page, step }) => { + // The Accounts tab also lists connection names; scope all tree + // lookups to the active tab panel so locators stay strict. + const sectionFor = (connection: string) => + page.getByRole("tabpanel").locator("section").filter({ hasText: connection }); + // Group rows are the only tree buttons carrying aria-expanded. + const closedGroup = (connection: string, text: string) => + sectionFor(connection).locator('button[aria-expanded="false"]').filter({ hasText: text }); + const policyMenuFor = (connection: string, node: string) => + sectionFor(connection).getByRole("button", { + name: `Set policy for ${node}`, + exact: true, + }); + // A leaf's policy dot, scoped to ITS row — the same effective policy + // (and thus the same indicator label) can legitimately sit on several + // rows at once, so an unscoped label lookup would not be unique. + const leafIndicator = (connection: string, leaf: string, label: string) => + sectionFor(connection) + .getByRole("button") + .filter({ hasText: leaf }) + .getByLabel(label, { exact: true }); + + await step("Open the integration's Tools tab", async () => { + await page.goto(`/integrations/${integration}`, { waitUntil: "networkidle" }); + await page.getByRole("tab", { name: "Tools" }).click(); + await sectionFor(alpha).waitFor(); + await sectionFor(beta).waitFor(); + }); + + await step("Expand the records category in the first account", async () => { + await closedGroup(alpha, integration).click(); + await closedGroup(alpha, "records").click(); + await policyMenuFor(alpha, `${integration}.records.create`).waitFor(); + }); + + await step("Block records.create from the per-tool menu", async () => { + await policyMenuFor(alpha, `${integration}.records.create`).click(); + // The menu is headed by the exact pattern it will store. + await page.getByText(leafPattern, { exact: true }).waitFor(); + await page.getByRole("menuitem", { name: "Block" }).click(); + await leafIndicator(alpha, "create", `Blocked (matched ${leafPattern})`).waitFor(); + }); + + await step("Require approval for the whole records category", async () => { + await policyMenuFor(alpha, `${integration}.records.*`).click(); + await page.getByText(categoryPattern, { exact: true }).waitFor(); + await page.getByRole("menuitem", { name: "Require approval" }).click(); + }); + + await step( + "The category rule covers the sibling leaf; the leaf rule keeps winning", + async () => { + await leafIndicator( + alpha, + "list", + `Require approval (matched ${categoryPattern})`, + ).waitFor(); + await leafIndicator(alpha, "create", `Blocked (matched ${leafPattern})`).waitFor(); + }, + ); + + await step("The same rules govern the second account's rows", async () => { + await closedGroup(beta, integration).click(); + await closedGroup(beta, "records").click(); + await leafIndicator(beta, "create", `Blocked (matched ${leafPattern})`).waitFor(); + await leafIndicator( + beta, + "list", + `Require approval (matched ${categoryPattern})`, + ).waitFor(); + }); + + await step("Reopening the menu offers to clear the active rule", async () => { + await policyMenuFor(beta, `${integration}.records.create`).click(); + await page.getByRole("menuitem", { name: "Clear" }).waitFor(); + await page.keyboard.press("Escape"); + }); + + await step("Both rules are manageable rows on the Policies page", async () => { + await page.goto("/policies", { waitUntil: "networkidle" }); + await page.getByText(leafPattern, { exact: true }).waitFor(); + await page.getByText(categoryPattern, { exact: true }).waitFor(); + }); + }); + + // Server-side truth, on a fresh read: exactly the two authored rules, + // org-owned, with the more specific leaf rule placed above the later + // category rule so it keeps precedence. + const policies = yield* client.policies.list(); + const mine = policies + .filter((p) => p.pattern.startsWith(`${integration}.`)) + .sort((a, b) => (a.position < b.position ? -1 : a.position > b.position ? 1 : 0)); + expect( + mine.map((p) => `${p.owner} ${p.pattern} ${p.action}`), + "the UI-authored rules persisted with the leaf rule above the category rule", + ).toEqual([`org ${leafPattern} block`, `org ${categoryPattern} require_approval`]); + }).pipe(Effect.ensuring(cleanup)); + }), +); From 5325a3a788e67856865dc0c480ef620adb07d900 Mon Sep 17 00:00:00 2001 From: Rhys Sullivan Date: Fri, 12 Jun 2026 17:23:49 -0700 Subject: [PATCH 2/2] Fix tool-detail policy badge writing unmatchable patterns; cover it in e2e MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The detail header's PolicyBadgeMenu passed the display id (e.g. google.gmail.users.getProfile) straight to onSetPolicy, but dynamic tools are policy-matched on integration.owner.connection.tool — so a rule authored from the badge never matched anything, and the menu never recognized an existing rule (no checkmark, no Clear). Route the badge through the same toPolicyPattern bridge the tree rows use (static tools keep their verbatim address), thread the static flag from both pages, and show the real stored pattern as the menu header. The policies-ui scenario now drives the badge too: it must show the inherited category rule, author an exact Always-run rule whose pattern actually matches (badge flips to the more specific rule), and Clear must restore inheritance. --- e2e/scenarios/policies-ui.test.ts | 29 ++++++++++++++++++- packages/react/src/components/tool-detail.tsx | 23 ++++++++++----- .../react/src/pages/integration-detail.tsx | 1 + packages/react/src/pages/tools.tsx | 15 ++++++---- 4 files changed, 53 insertions(+), 15 deletions(-) diff --git a/e2e/scenarios/policies-ui.test.ts b/e2e/scenarios/policies-ui.test.ts index c5be589ab..dec30dd4e 100644 --- a/e2e/scenarios/policies-ui.test.ts +++ b/e2e/scenarios/policies-ui.test.ts @@ -12,7 +12,10 @@ // 3. Rules are connection-agnostic: set from one account's section, they // govern the other account's rows too, and the menu there shows the // active rule with a Clear option. -// 4. The rules materialize as manageable rows on /policies and persist +// 4. The tool detail header's policy badge is the same authoring surface: +// it writes the same stored pattern, recognizes its own rule afterward +// (the Clear affordance), and Clear really removes the rule. +// 5. The rules materialize as manageable rows on /policies and persist // server-side with exactly the owner/pattern/action the UI promised. import { randomBytes } from "node:crypto"; @@ -83,6 +86,7 @@ scenario( // stored connection-wildcarded so it spans every account. const leafPattern = `${integration}.*.*.records.create`; const categoryPattern = `${integration}.*.*.records.*`; + const listLeafPattern = `${integration}.*.*.records.list`; // Selfhost scenarios share one workspace — remove everything this one // made (policies, connections, the integration) even on failure. @@ -211,6 +215,29 @@ scenario( await page.keyboard.press("Escape"); }); + await step("Open the tool detail for records.list", async () => { + await sectionFor(beta).getByRole("button").filter({ hasText: "list" }).click(); + // The header badge reflects the inherited category rule. + await page.getByRole("button", { name: `Matched policy: ${categoryPattern}` }).waitFor(); + }); + + await step("The detail badge authors an Always run rule for the exact tool", async () => { + await page.getByRole("button", { name: `Matched policy: ${categoryPattern}` }).click(); + // The badge menu is headed by the exact pattern it will store. + await page.getByText(listLeafPattern, { exact: true }).waitFor(); + await page.getByRole("menuitem", { name: "Always run" }).click(); + // The written rule must actually match this tool: the badge flips + // to the new, more specific rule. + await page.getByRole("button", { name: `Matched policy: ${listLeafPattern}` }).waitFor(); + }); + + await step("The badge recognizes its own rule and Clear removes it", async () => { + await page.getByRole("button", { name: `Matched policy: ${listLeafPattern}` }).click(); + await page.getByRole("menuitem", { name: "Clear" }).click(); + // Back to inheriting the category rule. + await page.getByRole("button", { name: `Matched policy: ${categoryPattern}` }).waitFor(); + }); + await step("Both rules are manageable rows on the Policies page", async () => { await page.goto("/policies", { waitUntil: "networkidle" }); await page.getByText(leafPattern, { exact: true }).waitFor(); diff --git a/packages/react/src/components/tool-detail.tsx b/packages/react/src/components/tool-detail.tsx index 637ea1015..01c723c95 100644 --- a/packages/react/src/components/tool-detail.tsx +++ b/packages/react/src/components/tool-detail.tsx @@ -161,6 +161,9 @@ export function ToolDetail(props: { address: ToolAddress; /** Policy id `.` — the tree path and display value. */ toolName: string; + /** True for plugin-contributed static tools (policy-matched on their + * address verbatim, not the connection-wildcarded pattern). */ + staticTool?: boolean; /** Resolved effective policy — user-authored or plugin-default, * unified into one shape. Surfaces in the header. */ policy?: EffectivePolicy; @@ -226,6 +229,7 @@ export function ToolDetail(props: { void; onClearPolicy?: (pattern: string) => void; }) { const interactive = !!props.onSetPolicy; + // The same pattern bridge the tree rows apply — the pattern WRITTEN and the + // pattern LOOKED UP must be the same string, or the menu authors rules that + // never match and can't see its own rule afterward. + const pattern = props.staticTool ? props.toolName : toPolicyPattern(props.toolName); // The "Clear" affordance only makes sense when there's a user rule // pinned to this exact tool id — clearing a wildcard rule from a // single tool's detail header would silently affect siblings. - const hasExactUserRule = - props.policy?.source === "user" && props.policy.pattern === toPolicyPattern(props.toolName); + const hasExactUserRule = props.policy?.source === "user" && props.policy.pattern === pattern; const currentAction = hasExactUserRule ? props.policy?.action : undefined; if (!interactive) { @@ -433,13 +443,10 @@ function PolicyBadgeMenu(props: { - {props.toolName} + {pattern} {POLICY_ACTIONS_IN_ORDER.map((action) => ( - props.onSetPolicy?.(props.toolName, action)} - > + props.onSetPolicy?.(pattern, action)}> {POLICY_ACTION_LABEL[action]} {currentAction === action && ( @@ -452,7 +459,7 @@ function PolicyBadgeMenu(props: { <> props.onClearPolicy?.(props.toolName)} + onSelect={() => props.onClearPolicy?.(pattern)} className="text-muted-foreground" > Clear diff --git a/packages/react/src/pages/integration-detail.tsx b/packages/react/src/pages/integration-detail.tsx index bdcd7757d..812e40170 100644 --- a/packages/react/src/pages/integration-detail.tsx +++ b/packages/react/src/pages/integration-detail.tsx @@ -531,6 +531,7 @@ export function IntegrationDetailPage(props: { namespace: string }) { void policyActions.set(pattern, action)} onClearPolicy={(pattern) => void policyActions.clear(pattern)} diff --git a/packages/react/src/pages/tools.tsx b/packages/react/src/pages/tools.tsx index 0d6c1c0a8..39670412e 100644 --- a/packages/react/src/pages/tools.tsx +++ b/packages/react/src/pages/tools.tsx @@ -51,14 +51,15 @@ export function ToolsPage() { [policyList], ); - // Address → the full per-connection tool address, so the detail view can fetch - // the right schema for the selected `.` id. - const addressById = useMemo(() => { - const map = new Map(); + // Address + static flag per selection id, so the detail view can fetch the + // right schema for the selected `.` id and its policy + // badge can write the matching pattern form. + const selectionById = useMemo(() => { + const map = new Map(); if (!AsyncResult.isSuccess(tools)) return map; for (const t of tools.value as readonly ToolRow[]) { const id = policyId(t); - if (!map.has(id)) map.set(id, t.address); + if (!map.has(id)) map.set(id, { address: t.address, static: t.static === true }); } return map; }, [tools]); @@ -89,7 +90,8 @@ export function ToolsPage() { () => summaries.find((t) => t.id === selectedToolId) ?? null, [summaries, selectedToolId], ); - const selectedAddress = selectedToolId ? (addressById.get(selectedToolId) ?? null) : null; + const selection = selectedToolId ? (selectionById.get(selectedToolId) ?? null) : null; + const selectedAddress = selection?.address ?? null; return (
@@ -140,6 +142,7 @@ export function ToolsPage() { void policyActions.set(pattern, action)} onClearPolicy={(pattern) => void policyActions.clear(pattern)}