diff --git a/e2e/scenarios/policies-duplicate.test.ts b/e2e/scenarios/policies-duplicate.test.ts new file mode 100644 index 000000000..7c775dbe7 --- /dev/null +++ b/e2e/scenarios/policies-duplicate.test.ts @@ -0,0 +1,159 @@ +// Cross-target (browser): the row's Duplicate menu prefills the add form +// with the source row's pattern, action, and owner, and focuses the pattern +// input with its content selected so the user can tweak the pattern in one +// keystroke. Submitting the form then writes the duplicated rule as its own +// row. The product guarantees this scenario pins: +// +// 1. The Duplicate menu item exists on every row's overflow menu. +// 2. Clicking it copies the row's `pattern` and `action` into the add form +// (the action select reflects the source row's verb label). +// 3. The pattern input is focused after the prefill, the UX promise that +// "I can just type" instead of "I have to click the input first". +// 4. After editing the pattern and submitting, BOTH rows appear in the +// list and the server-side `policies.list` reflects both. +// +// This is the only UI surface today that exercises the form's `prefill` +// prop. Regressions to the prefill nonce, the focus timing, or the field +// copy logic all surface as a `waitFor` timeout in this scenario. +import { randomBytes } from "node:crypto"; + +import { expect } from "@effect/vitest"; +import { Effect } from "effect"; +import { composePluginApi } from "@executor-js/api/server"; + +import { scenario } from "../src/scenario"; +import { Api, Browser, Target } from "../src/services"; + +const coreApi = composePluginApi([] as const); + +scenario( + "Policies · the row's Duplicate menu prefills the add form for a one-keystroke clone", + { timeout: 120_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(coreApi, identity); + + const suffix = randomBytes(4).toString("hex"); + const prefix = `policies-dup-${suffix}.`; + const originalPattern = `${prefix}alpha`; + const copyPattern = `${prefix}beta`; + + const cleanup = Effect.gen(function* () { + const policies = yield* client.policies.list().pipe(Effect.orElseSucceed(() => [])); + yield* Effect.forEach( + policies.filter((p) => p.pattern.startsWith(prefix)), + (p) => + client.policies + .remove({ params: { policyId: p.id }, payload: { owner: p.owner } }) + .pipe(Effect.ignore), + ); + }).pipe(Effect.ignore); + + yield* Effect.gen(function* () { + yield* browser.session(identity, async ({ page, step }) => { + const patternInput = page.getByPlaceholder("vercel.dns.* or *"); + const formActionSelect = page.locator("form").getByRole("combobox").first(); + const cardContent = page.locator('[data-slot="card-stack-content"]'); + const row = (text: string) => + cardContent.locator('[data-slot="card-stack-entry"]').filter({ hasText: text }); + + await step("Open the policies page and add a Block rule", async () => { + await page.goto("/policies", { waitUntil: "networkidle" }); + await page.getByRole("heading", { name: "Policies", exact: true }).waitFor(); + await patternInput.fill(originalPattern); + // Switch the form's action to Block so the duplicate prefill has a + // non-default action to verify. + await formActionSelect.click(); + await page.getByRole("option", { name: "Block", exact: true }).click(); + await page.getByRole("button", { name: "Add policy", exact: true }).click(); + await row(originalPattern).waitFor(); + // Settle the POST before the next step opens a menu, per the e2e + // style guide ("settle the network before opening menus"). The + // optimistic render attaches the row immediately, but Radix's + // pointer listeners on the overflow trigger only bind cleanly on + // the post-confirm re-render. Without this wait, a fast hover+click + // can land on a stale trigger and the dropdown silently no-ops. + await page.waitForLoadState("networkidle"); + }); + + await step("Open the row's overflow menu and click Duplicate", async () => { + // Hover the row to materialize the opacity-0 overflow trigger, then + // target it by data-slot rather than a positional `getByRole`, + // the row also contains the badge's role="combobox" trigger, which + // can change the last-button heuristic if the DOM order ever moves. + await row(originalPattern).hover(); + const trigger = row(originalPattern).locator('[data-slot="dropdown-menu-trigger"]'); + // Wait for the trigger to be visible (group-hover transition is + // opacity-based), then click without `force`, matching the + // policies-round-trip overflow pattern. The selfhost dev server + // boot can be slow enough that a force-click races the trigger's + // opacity transition and the Radix open handler never fires. + await trigger.waitFor({ state: "visible" }); + await trigger.click(); + // The DropdownMenuContent is portaled to body, not the row; wait + // for it to mount before targeting the menu item, so a timeout + // here means "the menu never opened", not "the item is missing". + const menu = page.locator('[data-slot="dropdown-menu-content"]'); + await menu.waitFor(); + await menu.getByRole("menuitem", { name: "Duplicate", exact: true }).click(); + }); + + await step("The form prefilled with the source pattern and action", async () => { + expect( + await patternInput.inputValue(), + "the pattern input carries the source row's pattern verbatim", + ).toBe(originalPattern); + expect( + await formActionSelect.textContent(), + "the action select carries the source row's verb label", + ).toContain("Block"); + }); + + await step("The pattern input is focused, ready to be tweaked", async () => { + // Asserting on the focused element's id rather than a boolean + // identity check, a regression where focus lands on the wrong + // element will print the actual id instead of bare `false`, which + // is the e2e/AGENTS.md "values not booleans" rule. + await expect + .poll( + () => + patternInput.evaluate( + () => (document.activeElement as HTMLElement | null)?.id ?? "", + ), + { + message: "Duplicate focuses the pattern input by id", + timeout: 2_000, + }, + ) + .toBe("policy-pattern"); + }); + + await step("Tweak the pattern and submit the copy", async () => { + await patternInput.fill(copyPattern); + await page.getByRole("button", { name: "Add policy", exact: true }).click(); + await row(copyPattern).waitFor(); + }); + + await step("Both rows appear in the rendered list", async () => { + await row(originalPattern).waitFor(); + await row(copyPattern).waitFor(); + }); + }); + + // Server-side truth on a fresh read: both rules persisted, both + // org-owned, both carrying the action the form was holding at submit + // (Block was set before the first add and the prefill preserved it). + const policies = yield* client.policies.list(); + const mine = policies + .filter((p) => p.pattern.startsWith(prefix)) + .map((p) => `${p.owner} ${p.pattern} ${p.action}`) + .sort(); + expect(mine, "both duplicated rules persisted with the source's action").toEqual( + [`org ${originalPattern} block`, `org ${copyPattern} block`].sort(), + ); + }).pipe(Effect.ensuring(cleanup)); + }), +); diff --git a/e2e/scenarios/policies-landing.test.ts b/e2e/scenarios/policies-landing.test.ts new file mode 100644 index 000000000..bd8050276 --- /dev/null +++ b/e2e/scenarios/policies-landing.test.ts @@ -0,0 +1,79 @@ +// Cross-target (browser): a fresh workspace lands on `/policies` with an +// explainer empty state and a gated add form. The existing `policies-ui` +// scenario covers authoring rules from the tool tree; this scenario only +// pins the landing surface for a workspace that has never authored a rule. +// +// Asserts on: +// +// 1. The page renders the "Policies" heading and the rationale paragraph +// under it (scoped to its `

`, not a bare text match). +// 2. The Active policies card-stack header is present, with the empty- +// state explainer reading "No policies yet. Tools fall back to their +// plugin's default approval behavior.", the product's guarantee that +// absence-of-rule is a resolved default, not a loading state. +// 3. The add-policy form's pattern input exists and the submit button is +// gated. Asserted as a value read of the `disabled` attribute, a +// regression prints the actual element state instead of `false`. +import { expect } from "@effect/vitest"; +import { Effect } from "effect"; + +import { scenario } from "../src/scenario"; +import { Browser, Target } from "../src/services"; + +scenario( + "Policies · a fresh workspace lands on an explainer empty state with a gated add form", + { timeout: 90_000 }, + Effect.gen(function* () { + const target = yield* Target; + const browser = yield* Browser; + const identity = yield* target.newIdentity(); + + yield* browser.session(identity, async ({ page, step }) => { + await step("Open the policies page on a fresh workspace", async () => { + await page.goto("/policies", { waitUntil: "networkidle" }); + await page.getByRole("heading", { name: "Policies", exact: true }).waitFor(); + }); + + await step("The rationale paragraph explains what policies do", async () => { + await page + .locator("p") + .filter({ + hasText: + "Override default approval behavior for tools. The most restrictive matched action wins.", + }) + .waitFor(); + }); + + await step("The Active policies card stack carries the empty-state explainer", async () => { + await page + .locator('[data-slot="card-stack-header"]') + .filter({ hasText: "Active policies" }) + .waitFor(); + // Scope to the card stack's content area: a regression where the + // explainer text leaks out of the empty state into a row body would + // still satisfy a bare `getByText`. + await page + .locator('[data-slot="card-stack-content"]') + .getByText( + "No policies yet. Tools fall back to their plugin's default approval behavior.", + { exact: true }, + ) + .waitFor(); + }); + + await step("The add form is reachable and its submit is gated", async () => { + const patternInput = page.getByPlaceholder("vercel.dns.* or *"); + await patternInput.waitFor(); + const addButton = page.getByRole("button", { name: "Add policy", exact: true }); + await addButton.waitFor(); + // Read the actual `disabled` attribute, a present attribute serializes + // as the empty string; a regression that ungates the button drops the + // attribute and this reads back as `null`. + expect( + await addButton.getAttribute("disabled"), + "Add policy is disabled until a valid pattern is typed", + ).toBe(""); + }); + }); + }), +); diff --git a/e2e/scenarios/policies-lifecycle.test.ts b/e2e/scenarios/policies-lifecycle.test.ts new file mode 100644 index 000000000..a144b397a --- /dev/null +++ b/e2e/scenarios/policies-lifecycle.test.ts @@ -0,0 +1,106 @@ +// Cross-target: the full policies CRUD round-trip through the typed +// HttpApiClient. The existing `policies.test.ts` scenario pins create + list +// only, the gaps this scenario closes are `update` (both full-payload and +// action-only partial, the shape the row badge's `handleUpdate` sends) and +// `remove`. Asserts on: +// +// 1. The create response carries a non-empty `position` (the fractional- +// indexing key the policies page's sort and reorder math both depend +// on, a regression that drops it would silently break ordering). +// 2. A full update returns the new pattern + action; a subsequent partial +// update (action only, no pattern in the payload) flips the action and +// leaves the pattern intact. +// 3. The list reflects the latest server-side values between writes. +// 4. After remove, list returns success and does NOT contain the id, +// asserted as `expect(ids).not.toContain(created.id)` so a regression +// prints the leaked ids instead of `false`. +import { randomBytes } from "node:crypto"; + +import { expect } from "@effect/vitest"; +import { Effect } from "effect"; +import { composePluginApi } from "@executor-js/api/server"; + +import { scenario } from "../src/scenario"; +import { Api, Target } from "../src/services"; + +const coreApi = composePluginApi([] as const); + +scenario( + "Policies · an existing policy can be re-targeted, partially edited, and removed", + {}, + Effect.gen(function* () { + const target = yield* Target; + const { client: apiClient } = yield* Api; + const identity = yield* target.newIdentity(); + const client = yield* apiClient(coreApi, identity); + + // Selfhost shares one bootstrap-admin workspace across scenarios, so + // every pattern carries a per-run suffix and the finalizer removes any + // row carrying it, even if a mid-test failure skips the explicit remove. + const suffix = randomBytes(4).toString("hex"); + const prefix = `policies-lc-${suffix}.`; + const initialPattern = `${prefix}alpha`; + const renamedPattern = `${prefix}beta`; + + const cleanup = Effect.gen(function* () { + const policies = yield* client.policies.list().pipe(Effect.orElseSucceed(() => [])); + yield* Effect.forEach( + policies.filter((p) => p.pattern.startsWith(prefix)), + (p) => + client.policies + .remove({ params: { policyId: p.id }, payload: { owner: p.owner } }) + .pipe(Effect.ignore), + ); + }).pipe(Effect.ignore); + + yield* Effect.gen(function* () { + const created = yield* client.policies.create({ + payload: { owner: "org", pattern: initialPattern, action: "block" }, + }); + expect(created.pattern, "create response echoes the requested pattern").toBe(initialPattern); + expect(created.action, "create response echoes the requested action").toBe("block"); + // The page's sort and reorder math both depend on a non-empty position; + // a regression that ever leaves it blank would silently break ordering + // without raising on any single create. + expect(created.position, "create response carries a fractional-indexing key").not.toBe(""); + + // Full payload: pattern AND action change in one update, the path the + // page itself never sends today, but `policies.update` advertises. + const renamed = yield* client.policies.update({ + params: { policyId: created.id }, + payload: { owner: "org", pattern: renamedPattern, action: "approve" }, + }); + expect(renamed.pattern, "full update applied the new pattern").toBe(renamedPattern); + expect(renamed.action, "full update applied the new action").toBe("approve"); + + // Partial payload: action only, no pattern, the exact shape the row + // badge's `handleUpdate` sends. The server should flip the action and + // leave the pattern intact. + const switched = yield* client.policies.update({ + params: { policyId: created.id }, + payload: { owner: "org", action: "require_approval" }, + }); + expect(switched.action, "partial update flipped the action").toBe("require_approval"); + expect(switched.pattern, "partial update preserved the pattern").toBe(renamedPattern); + + // List reflects the latest values between writes. + const afterEdit = yield* client.policies.list(); + const myEntry = afterEdit.find((p) => p.id === created.id); + expect(myEntry, "the edited row appears in list with the latest values").toMatchObject({ + pattern: renamedPattern, + action: "require_approval", + }); + + yield* client.policies.remove({ + params: { policyId: created.id }, + payload: { owner: "org" }, + }); + + const afterRemove = yield* client.policies.list(); + expect( + afterRemove.map((p) => p.id), + "the removed id is gone from the list", + ).not.toContain(created.id); + }).pipe(Effect.ensuring(cleanup)); + }), +); diff --git a/e2e/scenarios/policies-round-trip.test.ts b/e2e/scenarios/policies-round-trip.test.ts new file mode 100644 index 000000000..340522dc7 --- /dev/null +++ b/e2e/scenarios/policies-round-trip.test.ts @@ -0,0 +1,143 @@ +// Cross-target (browser): the full UI lifecycle of a policy. The user opens +// `/policies` on an empty workspace, submits a Require approval rule through +// the add form, flips its action to Always run via the row's inline badge +// select, then removes it via the row's overflow menu, and watches the +// empty state return. Covers the surfaces `policies-lifecycle` (API) and +// `policies-landing` (empty state) intentionally leave out: that the +// rendered page itself is the authoring surface, not just a read view. +// +// Selfhost shares one bootstrap-admin workspace, so the pattern carries a +// per-run suffix and the finalizer removes any row that survived a mid-test +// failure via the API. +import { randomBytes } from "node:crypto"; + +import { expect } from "@effect/vitest"; +import { Effect } from "effect"; +import { composePluginApi } from "@executor-js/api/server"; + +import { scenario } from "../src/scenario"; +import { Api, Browser, Target } from "../src/services"; + +const coreApi = composePluginApi([] as const); + +scenario( + "Policies · a user can add, re-target, and remove a rule from the policies page", + { timeout: 120_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(coreApi, identity); + + const suffix = randomBytes(4).toString("hex"); + const prefix = `policies-rt-${suffix}.`; + const pattern = `${prefix}alpha`; + + const cleanup = Effect.gen(function* () { + const policies = yield* client.policies.list().pipe(Effect.orElseSucceed(() => [])); + yield* Effect.forEach( + policies.filter((p) => p.pattern.startsWith(prefix)), + (p) => + client.policies + .remove({ params: { policyId: p.id }, payload: { owner: p.owner } }) + .pipe(Effect.ignore), + ); + }).pipe(Effect.ignore); + + yield* Effect.gen(function* () { + yield* browser.session(identity, async ({ page, step }) => { + const cardContent = page.locator('[data-slot="card-stack-content"]'); + const row = () => + cardContent.locator('[data-slot="card-stack-entry"]').filter({ hasText: pattern }); + + await step("Open the policies page on a fresh workspace", async () => { + await page.goto("/policies", { waitUntil: "networkidle" }); + await page.getByRole("heading", { name: "Policies", exact: true }).waitFor(); + // The empty-state explainer guarantees this workspace has never + // authored a rule, the precondition this scenario depends on. + await cardContent + .getByText( + "No policies yet. Tools fall back to their plugin's default approval behavior.", + { exact: true }, + ) + .waitFor(); + }); + + await step("Submit a Require approval rule through the add form", async () => { + await page.getByPlaceholder("vercel.dns.* or *").fill(pattern); + // The form's action select defaults to Require approval, submit + // the form as-is to also pin that default. + await page.getByRole("button", { name: "Add policy", exact: true }).click(); + // Settle the POST before the next step opens the badge or overflow + // dropdown, per the e2e style guide. The optimistic render lands + // the row immediately, but Radix's pointer listeners on the badge + // and overflow triggers only bind on the post-confirm re-render. + await page.waitForLoadState("networkidle"); + }); + + await step("The new row appears with the pattern and Require approval badge", async () => { + await row().waitFor(); + // The row's inline badge select is the first combobox inside the + // card-stack content (the add form's combobox sits outside it). + const rowBadge = row().getByRole("combobox"); + await rowBadge.waitFor(); + expect( + await rowBadge.textContent(), + "the row badge shows the rule's current action verbatim", + ).toContain("Require approval"); + }); + + await step("Flip the action to Always run via the row badge select", async () => { + await row().getByRole("combobox").click(); + // "Always run" is the verb label for the `approve` action. + await page.getByRole("option", { name: "Always run", exact: true }).click(); + }); + + await step("The badge reflects the new Always run action", async () => { + const rowBadge = row().getByRole("combobox"); + // Wait for the badge to actually flip (optimistic updates can + // take a frame); reading textContent at the right moment is the + // assertion. + await expect + .poll(async () => rowBadge.textContent(), { + message: "the row badge flipped to the new action", + timeout: 5_000, + }) + .toContain("Always run"); + }); + + await step("Remove the rule via the row's overflow menu", async () => { + // Hover the row to materialize the opacity-0 overflow trigger, what + // a real user does, then click without `force`. The menu content + // is portaled to body, so wait for it explicitly before targeting + // the menu item. + await row().hover(); + await row().locator('[data-slot="dropdown-menu-trigger"]').click(); + const menu = page.locator('[data-slot="dropdown-menu-content"]'); + await menu.waitFor(); + await menu.getByRole("menuitem", { name: "Remove", exact: true }).click(); + }); + + await step("The row disappears and the empty state returns", async () => { + await row().waitFor({ state: "detached" }); + await cardContent + .getByText( + "No policies yet. Tools fall back to their plugin's default approval behavior.", + { exact: true }, + ) + .waitFor(); + }); + }); + + // Server-side: a fresh read shows zero rows carrying this scenario's + // prefix. Pins that the UI's remove path actually deleted (vs. just + // optimistic-updated the cache). + const policies = yield* client.policies.list(); + expect( + policies.filter((p) => p.pattern.startsWith(prefix)).map((p) => p.pattern), + "no rows from this scenario survive on the server", + ).toEqual([]); + }).pipe(Effect.ensuring(cleanup)); + }), +); diff --git a/packages/react/src/api/analytics.tsx b/packages/react/src/api/analytics.tsx index b4f0f66a9..d7b57b005 100644 --- a/packages/react/src/api/analytics.tsx +++ b/packages/react/src/api/analytics.tsx @@ -1,7 +1,7 @@ import * as React from "react"; // --------------------------------------------------------------------------- -// Product-analytics seam — same DI shape as ./error-reporting: a module-level +// Product-analytics seam, same DI shape as ./error-reporting: a module-level // client set by a provider the HOST mounts, and a free `trackEvent` function // callsites import directly (works outside React, e.g. oauth-popup callbacks). // No client mounted (local, self-host, cloudflare, tests) → every call is a @@ -15,14 +15,14 @@ import * as React from "react"; // BROWSER-ONLY by design: during SSR the host mounts no client (cloud's is // undefined when `window` is absent), and on shared-module-scope runtimes // (Cloudflare Workers) every SSR render resets the singleton to null. Server- -// side product events need their own seam — do not route them through this one. +// side product events need their own seam, do not route them through this one. // -// PROPERTY RULES — properties must never carry: +// PROPERTY RULES, properties must never carry: // - secrets, tokens, credential values, copied clipboard contents // - emails, person/org names, or any user-entered free text // - connection names or tool ADDRESSES (both embed user-entered label text; // integration slugs and spec-derived tool names are fine) -// - policy patterns (user-entered globs) — use `pattern_kind` instead +// - policy patterns (user-entered globs), use `pattern_kind` instead // Identity attaches via posthog identify/group (the host's concern), never as // event properties. // --------------------------------------------------------------------------- @@ -103,6 +103,10 @@ export interface AnalyticsEvents { policy_action_changed: { action: string; owner: Owner; success: boolean }; policy_removed: { owner: Owner; success: boolean }; policy_reordered: { owner: Owner; direction: "up" | "down"; success: boolean }; + // Row's Duplicate menu, fires when the form is prefilled, BEFORE the + // duplicated row is submitted. Drop in `success` if Duplicate ever becomes + // a server call (today the form-fill is purely client-side). + policy_duplicated: { action: string; owner: Owner }; // ── API keys ───────────────────────────────────────────────────────────── api_key_created: { success: boolean }; @@ -178,7 +182,7 @@ export type AnalyticsClient = ( let currentAnalyticsClient: AnalyticsClient | null = null; /** - * Imperative injection point — what `AnalyticsProvider` uses, and the hook for + * Imperative injection point, what `AnalyticsProvider` uses, and the hook for * non-React hosts (or tests). Pass `null` to restore the no-op default. */ export const setAnalyticsClient = (client: AnalyticsClient | null): void => { @@ -200,7 +204,7 @@ export const trackEvent = ( }; /** - * Declarative mount for React hosts — sets the module-level client during + * Declarative mount for React hosts, sets the module-level client during * render, exactly like `FrontendErrorReporterProvider` does for error * reporting. Mount once at the app root, ABOVE any tree that fires events * (in cloud that is the document root, not ExecutorProvider, because the diff --git a/packages/react/src/pages/policies.tsx b/packages/react/src/pages/policies.tsx index e709e111e..425d95f42 100644 --- a/packages/react/src/pages/policies.tsx +++ b/packages/react/src/pages/policies.tsx @@ -1,4 +1,4 @@ -import { useState } from "react"; +import { useEffect, useRef, useState } from "react"; import { useAtomSet, useAtomValue } from "@effect/atom-react"; import * as AsyncResult from "effect/unstable/reactivity/AsyncResult"; import * as Exit from "effect/Exit"; @@ -68,7 +68,7 @@ const POLICY_OWNERS: readonly { readonly owner: Owner; readonly label: string }[ ]; // --------------------------------------------------------------------------- -// Sort comparator — owner rank, then fractional-indexing key, then id as a +// Sort comparator, owner rank, then fractional-indexing key, then id as a // stable tiebreak. Identical positions can briefly happen across racing // inserts; without the tiebreak the rendered order flips between refetches, and // `generateKeyBetween` would also throw if asked to insert "between" two equal @@ -85,7 +85,7 @@ const comparePolicy = (posA: string, idA: string, posB: string, idB: string): nu // Pattern matching + validation come from the SDK so the UI's "matches N tools" // preview and the add-policy validation use the EXACT same grammar the executor -// enforces — including mid-segment wildcards (`integration.*.*.tool`), which the +// enforces, including mid-segment wildcards (`integration.*.*.tool`), which the // connection-aware policy model now relies on. (Re-exported below for callers.) const matchesPattern = matchPattern; @@ -98,10 +98,42 @@ function AddPolicyForm(props: { owner: Owner; onOwnerChange: (owner: Owner) => void; busy: boolean; + /** Each click on a row's Duplicate menu bumps `nonce` so the effect below + * re-syncs even when the source policy's pattern matches the form's + * current value. */ + prefill?: { pattern: string; action: ToolPolicyAction; nonce: number }; }) { const [pattern, setPattern] = useState(""); const [action, setAction] = useState("require_approval"); + const patternInputRef = useRef(null); const valid = isValidPattern(pattern); + + // When a row's Duplicate menu fires, copy its pattern + action into the + // form and focus the pattern input with its content selected so the user + // can tweak the pattern in one keystroke. Selecting (not just focusing) is + // the difference between "I have to clear it first" and "I can just type". + // The parent constructs a new prefill object on every Duplicate click, + // so nonce/pattern/action always change together; depending on all three + // primitives reads cleanly to the next editor and never double-fires. + const prefillNonce = props.prefill?.nonce; + const prefillPattern = props.prefill?.pattern; + const prefillAction = props.prefill?.action; + useEffect(() => { + if (prefillNonce === undefined) return; + setPattern(prefillPattern ?? ""); + setAction(prefillAction ?? "require_approval"); + // setTimeout instead of requestAnimationFrame so this fires AFTER + // Radix's DropdownMenu close (which also uses setTimeout(0) to restore + // focus to its trigger), same task queue, but this is enqueued in the + // commit phase, so it lands after Radix's queued restoration. Combined + // with `onCloseAutoFocus` preventing restoration in the row's menu, + // this leaves the input as the only thing fighting for focus. + const id = setTimeout(() => { + patternInputRef.current?.focus(); + patternInputRef.current?.select(); + }, 0); + return () => clearTimeout(id); + }, [prefillNonce, prefillPattern, prefillAction]); // Non-org hosts (local/desktop) have one local workspace. New local policies // are org-owned internally to match the v1->v2 migration. const ownerDisplay = useOwnerDisplay(); @@ -129,6 +161,7 @@ function AddPolicyForm(props: { setPattern(e.target.value)} @@ -203,8 +236,16 @@ function PolicyRow(props: { onChangeAction: (action: ToolPolicyAction) => void; onMoveUp: () => void; onMoveDown: () => void; + onDuplicate: () => void; showOwnerLabel: boolean; }) { + // Tracks why the row's dropdown last closed, so onCloseAutoFocus can + // suppress Radix's default trigger-refocus only for Duplicate (which is + // sending focus to the form's pattern input). Move up / Move down keep the + // trigger in the DOM, and a keyboard user needs Radix's default restoration + // to land back on it, suppressing for those would drop focus to . + // Remove unmounts the row entirely, so trigger-refocus is moot either way. + const closeReasonRef = useRef<"duplicate" | null>(null); return ( @@ -255,13 +296,36 @@ function PolicyRow(props: { - + { + // Only suppress Radix's default trigger-refocus when Duplicate + // fired: that path is sending focus to the form's pattern input + // and the default restoration would yank it back. For Move + // up/Move down the trigger persists and keyboard users need it + // re-focused; for Remove the row is unmounted and the default + // path silently lands at body either way. + if (closeReasonRef.current === "duplicate") { + e.preventDefault(); + } + closeReasonRef.current = null; + }} + > Move up Move down + { + closeReasonRef.current = "duplicate"; + props.onDuplicate(); + }} + > + Duplicate +