From b08352d464bdd21df428b744e12306b3735c3b3c Mon Sep 17 00:00:00 2001 From: Ali Al Dallal Date: Sun, 16 Aug 2026 21:41:01 -0400 Subject: [PATCH 1/2] feat: guardrail rule authoring -- rule-from-park, in-context edit, Review rules audit (goal 0078) --- .../mill/internal/domain/guardrail/models.ts | 10 +- .../services/guardrailsvc/guardrailservice.ts | 16 ++ frontend/e2e/fixtures/server.ts | 8 + frontend/e2e/guardrail-authoring.spec.ts | 176 +++++++++++++++++ frontend/src/app/ApprovalPrompt.tsx | 17 +- .../src/composition/NodeGuardrailSection.tsx | 87 ++++++++- frontend/src/locales/en/app.json | 1 + frontend/src/locales/en/composition.json | 7 +- frontend/src/locales/en/views.json | 55 +++++- .../src/shared/GuardrailRuleDialog.module.css | 17 ++ frontend/src/shared/GuardrailRuleDialog.tsx | 158 +++++++++++++++ frontend/src/shared/bindings.ts | 2 + frontend/src/shared/commands.ts | 12 ++ .../src/shared/guardrailRuleScope.test.ts | 97 +++++++++ frontend/src/shared/guardrailRuleScope.ts | 123 ++++++++++++ frontend/src/shared/uiSignalStore.ts | 10 + frontend/src/views/GuardrailRulesPanel.tsx | 184 ++++++++++++++++++ frontend/src/views/ReviewAlwaysRuleDialog.tsx | 139 +++++++++++++ frontend/src/views/ReviewView.tsx | 72 ++++++- internal/domain/guardrail/guardrail.go | 31 +-- .../services/guardrailsvc/guardrailservice.go | 38 ++++ .../guardrailsvc/guardrailservice_test.go | 88 +++++++++ 22 files changed, 1318 insertions(+), 30 deletions(-) create mode 100644 frontend/e2e/guardrail-authoring.spec.ts create mode 100644 frontend/src/shared/GuardrailRuleDialog.module.css create mode 100644 frontend/src/shared/GuardrailRuleDialog.tsx create mode 100644 frontend/src/shared/guardrailRuleScope.test.ts create mode 100644 frontend/src/shared/guardrailRuleScope.ts create mode 100644 frontend/src/views/GuardrailRulesPanel.tsx create mode 100644 frontend/src/views/ReviewAlwaysRuleDialog.tsx diff --git a/frontend/bindings/github.com/alicoding/mill/internal/domain/guardrail/models.ts b/frontend/bindings/github.com/alicoding/mill/internal/domain/guardrail/models.ts index 4dba34cf..6fbfc3b5 100644 --- a/frontend/bindings/github.com/alicoding/mill/internal/domain/guardrail/models.ts +++ b/frontend/bindings/github.com/alicoding/mill/internal/domain/guardrail/models.ts @@ -87,8 +87,14 @@ export interface Rule { * canvas Inspector, but a breakpoint borrows the same Rule/park * plumbing without being policy itself). The canvas Inspector's * "Breakpoint" toggle may only ever create/delete a SourceDebug - * rule; Configure > Guardrails (when it returns) governs policy - * rules exclusively. + * rule; policy rules are authored through the three-door model + * instead (goal 0078): rule-from-park (a parked run's "Always + * allow/deny…"), edit-in-context (a step's own matching rules, + * listed and editable but never created from the step editor), and + * the Review "Rules" audit view (create/edit/delete for + * completeness) -- one rule store (GuardrailService's CRUD) under + * all three doors, never a Configure entity (no other data + * references a rule by ID). */ "Source": string; } diff --git a/frontend/bindings/github.com/alicoding/mill/internal/services/guardrailsvc/guardrailservice.ts b/frontend/bindings/github.com/alicoding/mill/internal/services/guardrailsvc/guardrailservice.ts index 176df100..f1141272 100644 --- a/frontend/bindings/github.com/alicoding/mill/internal/services/guardrailsvc/guardrailservice.ts +++ b/frontend/bindings/github.com/alicoding/mill/internal/services/guardrailsvc/guardrailservice.ts @@ -53,6 +53,22 @@ export function Rules(): $CancellablePromise { return $Call.ByID(1214226525); } +/** + * RulesForStep returns every stored POLICY rule (Source != SourceDebug) + * whose non-empty scope fields all match the given workflow step -- + * door 2's "Rules for this step" list (NodeGuardrailSection) and door + * 1's rule-from-park scope prefill both need this without duplicating + * guardrail's own scope-match logic in the frontend (goal 0078). An + * unknown workflow/node returns nil rather than an error -- callers + * treat "no rules apply" and "no such step" the same way (an empty + * list), matching TestRules' node-resolution but without its + * error-on-unknown-step behavior, since this is a passive list, not a + * dry-run request naming a specific step. + */ +export function RulesForStep(workflowID: string, nodeID: string): $CancellablePromise { + return $Call.ByID(930742602, workflowID, nodeID); +} + /** * TestRules dry-runs the current rule set against one real workflow * step -- §8's locked testability requirement: see what would happen diff --git a/frontend/e2e/fixtures/server.ts b/frontend/e2e/fixtures/server.ts index d57a9212..539b5b8a 100644 --- a/frontend/e2e/fixtures/server.ts +++ b/frontend/e2e/fixtures/server.ts @@ -67,6 +67,14 @@ export const UPDATES_SOURCE_SERVER_BASE_PORT = 9760 export const UPDATES_SOURCE_MCP_BASE_PORT = 9780 export const UPDATES_RELEASE_SERVER_BASE_PORT = 9790 export const UPDATES_RELEASE_MCP_BASE_PORT = 9810 +// guardrail-authoring.spec.ts's own dedicated pair (goal 0078): the +// full rule-from-park -> unstick -> audit-edit -> policy-removed loop +// asserts exact rule counts/groupings in the Rules audit view, which +// the standard per-worker pool can't guarantee stays uncontaminated by +// another spec file sharing that worker's one server -- same +// own-server-own-ports reasoning as persistence/scale/mirror above. +export const GUARDRAIL_AUTHORING_SERVER_BASE_PORT = 9840 +export const GUARDRAIL_AUTHORING_MCP_BASE_PORT = 9860 async function waitForHealth(url: string, proc: ChildProcessWithoutNullStreams, timeoutMs: number): Promise { const deadline = Date.now() + timeoutMs diff --git a/frontend/e2e/guardrail-authoring.spec.ts b/frontend/e2e/guardrail-authoring.spec.ts new file mode 100644 index 00000000..283696fe --- /dev/null +++ b/frontend/e2e/guardrail-authoring.spec.ts @@ -0,0 +1,176 @@ +import { chromium, expect, test } from '@playwright/test' +import { mkdtempSync, rmSync } from 'node:fs' +import { tmpdir } from 'node:os' +import path from 'node:path' +import { withClipboardLock } from './fixtures/clipboardLock' +import { + GUARDRAIL_AUTHORING_MCP_BASE_PORT, + GUARDRAIL_AUTHORING_SERVER_BASE_PORT, + spawnMillServer, + type SpawnedServer, +} from './fixtures/server' + +// The full guardrail rule-authoring loop across all three doors (goal +// 0078), driven through the seeded "Example: Run copied code" workflow +// -- the cheapest seeded ask example that parks deterministically with +// no external network call (code-execution runs a real but local `echo` +// command, then a real clipboard write, same seed codeexec.spec.ts +// already proves the ambient gate against). The seed IS the proof +// (.claude/rules/testing.md): rule-from-park unsticks the workflow, +// edit-in-context and the audit view both see and change the same +// rule, and removing it really restores the parked default. +// +// Runs on its own dedicated server (fixtures/server.ts's +// GUARDRAIL_AUTHORING_* ports), not the standard per-worker pool: this +// spec asserts EXACT rule counts/groupings in the Rules audit view, +// which must never be contaminated by another spec file sharing a +// worker's server. +const SEED = 'Example: Run copied code' + +// eslint-disable-next-line no-empty-pattern -- this test needs `testInfo` (the second arg), not any fixture. +test('Rule-from-park unsticks the workflow, edit-in-context and the audit view see the same rule, and removing it restores the parked default', async ({}, testInfo) => { + const idx = testInfo.parallelIndex + const dir = mkdtempSync(path.join(tmpdir(), `mill-e2e-guardrail-authoring-${idx}-`)) + const settingsPath = path.join(dir, 'settings.json') + const executionDbPath = path.join(dir, 'execution.db') + const backupDir = path.join(dir, 'backups') + const port = GUARDRAIL_AUTHORING_SERVER_BASE_PORT + idx + const mcpPort = GUARDRAIL_AUTHORING_MCP_BASE_PORT + idx + + let server: SpawnedServer | undefined + const browser = await chromium.launch() + try { + server = await spawnMillServer({ port, mcpPort, settingsPath, executionDbPath, backupDir }) + const page = await browser.newPage() + + // code-execution's apply step really writes to the OS clipboard + // once allowed -- every run below (approved-from-park, then the + // auto-allowed second run) needs the lock end to end + // (.claude/rules/testing.md); the THIRD run below parks and gets + // denied, never reaching the clipboard step, but stays inside the + // same lock for simplicity. + await withClipboardLock(async () => { + await page.goto(`${server!.baseURL}/`) + await page.getByRole('link', { name: 'Workflows' }).click() + const row = page.locator('[data-testid="inventory-row"][data-entity="workflow"]').filter({ has: page.getByText(SEED, { exact: true }) }) + await expect(row).toBeVisible() + + // --- Door 1: rule-from-park --- + await row.getByRole('button', { name: `Run ${SEED}`, exact: true }).click() + await page.getByRole('link', { name: 'Review' }).click() + const item = page.getByTestId('review-item').filter({ hasText: SEED }).first() + await expect(item).toBeVisible({ timeout: 10_000 }) + + await item.getByTestId('review-always-menu').click() + // The ActionMenu.Overlay portals outside the row's own DOM + // subtree -- scoped to `page`, not `item`. + await page.getByTestId('review-always-allow').click() + + const alwaysDialog = page.getByRole('dialog', { name: 'Always allow' }) + await expect(alwaysDialog).toBeVisible() + await expect(alwaysDialog.getByTestId('review-always-context')).toContainText('Code: run command in Example: Run copied code') + // Least-privilege default: "Only this step" starts selected. + await expect(alwaysDialog.getByTestId('review-always-scope-step')).toBeChecked() + const ruleNameInput = alwaysDialog.getByTestId('review-always-rule-name') + await expect(ruleNameInput).toHaveValue('Allow Code: run command in Example: Run copied code') + + await alwaysDialog.getByRole('button', { name: 'Save rule and approve' }).click() + await expect(alwaysDialog).toBeHidden() + await expect(page.getByTestId('review-item').filter({ hasText: SEED })).toHaveCount(0, { timeout: 10_000 }) + + // --- The unstick proof: a second run of the same workflow never parks --- + await page.getByRole('link', { name: 'Workflows' }).click() + await row.getByRole('button', { name: `Run ${SEED}`, exact: true }).click() + await page.waitForTimeout(1_000) // no observable "definitely didn't park" event to await; a park would show within this window + await expect(page.getByTestId('review-pending-count')).toHaveCount(0) + await page.getByRole('link', { name: 'Review' }).click() + await expect(page.getByTestId('review-item').filter({ hasText: SEED })).toHaveCount(0) + + // --- Door 2: edit-in-context on the step that now carries the rule --- + await page.getByRole('link', { name: 'Workflows' }).click() + await row.click() + // A row opens the canvas read-only (view mode) -- the inspector's + // own fieldset (NodeConfigFields.tsx) disables every descendant + // control, including the rule kebab below, until Edit is clicked. + await page.getByTestId('edit-workflow').click() + await page.locator('[data-id="example-codeexec-step"]').click() + const stepRuleRow = page.getByTestId('node-guardrail-rule-row').filter({ hasText: 'Allow Code: run command in Example: Run copied code' }) + await expect(stepRuleRow).toBeVisible() + await expect(stepRuleRow).toContainText('allow') + + await stepRuleRow.getByTestId('node-guardrail-rule-menu').click() + await page.getByTestId('node-guardrail-rule-edit').click() + const editDialog = page.getByRole('dialog', { name: 'Edit rule' }) + await expect(editDialog).toBeVisible() + await editDialog.getByTestId('guardrail-rule-name').fill('Allow the sandboxed echo step') + await editDialog.getByRole('button', { name: 'Save rule' }).click() + await expect(editDialog).toBeHidden() + await expect(page.getByTestId('node-guardrail-rule-row').filter({ hasText: 'Allow the sandboxed echo step' })).toBeVisible() + + // --- Door 3: the Review "Rules" audit view sees the same, edited rule --- + await page.getByRole('link', { name: 'Review' }).click() + await page.getByTestId('review-tab-rules').click() + const panel = page.getByTestId('guardrail-rules-panel') + await expect(panel).toBeVisible() + const group = page.getByTestId('guardrail-rules-group').filter({ has: page.getByText(SEED, { exact: true }) }) + await expect(group).toBeVisible() + const rulesRow = group.getByTestId('guardrail-rule-row').filter({ hasText: 'Allow the sandboxed echo step' }) + await expect(rulesRow).toBeVisible() + await expect(rulesRow.getByTestId('guardrail-rule-sentence')).toHaveText('Only this step — Code: run command in Example: Run copied code') + + // Remove it from the audit view -- the policy-removal half of the proof. + await rulesRow.getByTestId('guardrail-rule-menu').click() + // Same portal caveat as the "Always…" menu above. + await page.getByTestId('guardrail-rule-remove').click() + await page.getByRole('button', { name: 'Delete' }).click() + await expect(page.getByTestId('guardrail-rules-group')).toHaveCount(0) + await expect(page.getByTestId('guardrail-rules-empty')).toBeVisible() + + // --- The rule is really gone: a third run parks again --- + await page.getByRole('link', { name: 'Workflows' }).click() + await row.getByRole('button', { name: `Run ${SEED}`, exact: true }).click() + await page.getByRole('link', { name: 'Review' }).click() + const thirdParked = page.getByTestId('review-item').filter({ hasText: SEED }).first() + await expect(thirdParked).toBeVisible({ timeout: 10_000 }) + await thirdParked.getByTestId('review-deny').click() + await expect(page.getByTestId('review-item').filter({ hasText: SEED })).toHaveCount(0, { timeout: 10_000 }) + }) + + await page.close() + } finally { + await browser.close() + if (server) await server.stop() + rmSync(dir, { recursive: true, force: true }) + } +}) + +// eslint-disable-next-line no-empty-pattern -- this test needs `testInfo` (the second arg), not any fixture. +test('Door 2: selecting a step with no matching rule shows the "nothing applies" default', async ({}, testInfo) => { + const idx = testInfo.parallelIndex + const dir = mkdtempSync(path.join(tmpdir(), `mill-e2e-guardrail-authoring-nodefault-${idx}-`)) + const settingsPath = path.join(dir, 'settings.json') + const executionDbPath = path.join(dir, 'execution.db') + const backupDir = path.join(dir, 'backups') + // Offset from the main test's own port pair (same worker parallelIndex + // would otherwise collide across these two tests in this file). + const port = GUARDRAIL_AUTHORING_SERVER_BASE_PORT + 10 + idx + const mcpPort = GUARDRAIL_AUTHORING_MCP_BASE_PORT + 10 + idx + + let server: SpawnedServer | undefined + const browser = await chromium.launch() + try { + server = await spawnMillServer({ port, mcpPort, settingsPath, executionDbPath, backupDir }) + const page = await browser.newPage() + await page.goto(`${server.baseURL}/`) + await page.getByRole('link', { name: 'Workflows' }).click() + const row = page.locator('[data-testid="inventory-row"][data-entity="workflow"]').filter({ has: page.getByText(SEED, { exact: true }) }) + await row.click() + await page.locator('[data-id="example-codeexec-step"]').click() + await expect(page.getByTestId('node-guardrail-no-rules')).toBeVisible() + await expect(page.getByTestId('node-guardrail-no-rules')).toHaveText('No rules apply to this step. Its defaults decide.') + } finally { + await browser.close() + if (server) await server.stop() + rmSync(dir, { recursive: true, force: true }) + } +}) diff --git a/frontend/src/app/ApprovalPrompt.tsx b/frontend/src/app/ApprovalPrompt.tsx index 4205f4e9..bfe68495 100644 --- a/frontend/src/app/ApprovalPrompt.tsx +++ b/frontend/src/app/ApprovalPrompt.tsx @@ -134,9 +134,20 @@ export function ApprovalPrompt() { ) : ( - + + + {/* Same navigation as "Open in Mill" -- Review's Queue tab + (door 1's landing spot) already shows this exact card + with its own "Always…" action once there. A distinct + button exists purely so the rule-authoring path is + discoverable straight from the toast, not buried behind + a generic "open the run" label. */} + + )} {error && {error}} diff --git a/frontend/src/composition/NodeGuardrailSection.tsx b/frontend/src/composition/NodeGuardrailSection.tsx index bd857a82..e63cea44 100644 --- a/frontend/src/composition/NodeGuardrailSection.tsx +++ b/frontend/src/composition/NodeGuardrailSection.tsx @@ -1,10 +1,13 @@ import { useEffect, useState } from 'react' import { useTranslation } from 'react-i18next' -import { Stack, Text } from '@primer/react' -import { BugIcon, ShieldIcon } from '@primer/octicons-react' +import { ActionList, ActionMenu, IconButton, Stack, Text } from '@primer/react' +import { BugIcon, KebabHorizontalIcon, ShieldIcon } from '@primer/octicons-react' import { GuardrailService } from '../shared/bindings' -import type { RuleTestResult } from '../shared/bindings' +import type { GuardrailRule, RuleTestResult } from '../shared/bindings' import { StatusStamp } from '../shared/StatusStamp' +import type { StatusStampVariant } from '../shared/StatusStamp' +import { GuardrailRuleDialog } from '../shared/GuardrailRuleDialog' +import { useConfirmDelete } from '../shared/useConfirmDelete' import { useNodeBreakpoint } from './breakpoints' import styles from '../shared/ListCard.module.css' @@ -12,10 +15,14 @@ import styles from '../shared/ListCard.module.css' // Update): shows the step's LIVE verdict -- the same evaluation the // execution gate runs, so what you see here is what a run will do (§1's // what-you-see-is-what-I-see thesis applied to the guardrail itself). -// Deliberately NOT an authoring surface: rules are policy, authored in -// Configure > Guardrails only -- putting rule creation on the step -// editor made policy look like step config, which it isn't (corrected -// directly in discussion). +// +// Rule AUTHORING stays off the step editor -- putting rule creation +// here made policy look like step config, which it isn't. Policy is +// authored through the three-door model instead (goal 0078): +// rule-from-park (a parked run's own "Always allow/deny…"), this +// section's own "Rules for this step" list below (edit/remove an +// already-matching rule, never create one from scratch), and the +// Review "Rules" audit view (create/edit/delete for completeness). // // The breakpoint status line below is a NAMED EXCEPTION to that rule // (docs/adr/0031 item 1): a breakpoint borrows the guardrail Rule/park @@ -36,16 +43,37 @@ function effectTextFor(t: (key: string) => string): Record { } } +function effectVariant(effect: string): StatusStampVariant { + return effect === 'deny' ? 'danger' : effect === 'ask' ? 'caution' : 'success' +} + export function NodeGuardrailSection({ workflowId, nodeId }: { workflowId: string; nodeId: string }) { const { t } = useTranslation('composition') const EFFECT_TEXT = effectTextFor(t) const [verdict, setVerdict] = useState(null) const breakpoint = useNodeBreakpoint(nodeId) + // Door 2 (goal 0078): the rules that actually apply to this step, + // edit/remove only -- see this file's header for why create stays + // off this panel. + const [rules, setRules] = useState([]) + const [editing, setEditing] = useState(null) + + const refreshRules = () => { + GuardrailService.RulesForStep(workflowId, nodeId).then((r) => setRules(r ?? [])).catch(() => setRules([])) + } useEffect(() => { GuardrailService.TestRules(workflowId, nodeId).then(setVerdict).catch(() => setVerdict(null)) + refreshRules() + // eslint-disable-next-line react-hooks/exhaustive-deps -- refreshRules is derived from workflowId/nodeId, not independent reactive state }, [workflowId, nodeId]) + const { requestDelete, dialog: confirmDialog } = useConfirmDelete({ + entityType: 'rule', + labelOf: (r) => r.Label, + onConfirm: (r) => { GuardrailService.DeleteRule(r.ID).then(refreshRules).catch(() => {}) }, + }) + return ( {verdict && ( @@ -84,6 +112,51 @@ export function NodeGuardrailSection({ workflowId, nodeId }: { workflowId: strin {t('nodeGuardrailSection.breakpointDescription')} + + + {t('nodeGuardrailSection.rulesForStepHeading')} + + {rules.length === 0 ? ( + + {t('nodeGuardrailSection.noRulesApply')} + + ) : ( + + {rules.map((rule) => ( + + + {rule.Label} + {rule.Effect} + + + + + + + + setEditing(rule)} data-testid="node-guardrail-rule-edit"> + {t('nodeGuardrailSection.editMenuItem')} + + requestDelete(rule)} data-testid="node-guardrail-rule-remove"> + {t('nodeGuardrailSection.removeMenuItem')} + + + + + + ))} + + )} + {confirmDialog} + {editing && ( + setEditing(null)} onSaved={refreshRules} /> + )} ) } diff --git a/frontend/src/locales/en/app.json b/frontend/src/locales/en/app.json index 9498cdac..aa2756d4 100644 --- a/frontend/src/locales/en/app.json +++ b/frontend/src/locales/en/app.json @@ -80,6 +80,7 @@ "approvalPrompt": { "moreWaiting": "+{{count}} more waiting", "openInMill": "Open in Mill", + "setRule": "Set a rule…", "empty": "No pending approvals." }, "appSidebar": { diff --git a/frontend/src/locales/en/composition.json b/frontend/src/locales/en/composition.json index f40471d0..bd8c1bf9 100644 --- a/frontend/src/locales/en/composition.json +++ b/frontend/src/locales/en/composition.json @@ -197,7 +197,12 @@ "removeIt": "remove it", "addOne": "add one", "breakpointBadge": "Breakpoint", - "breakpointDescription": "A run pauses here to let you inspect and edit its data before it continues -- a debugging aid, not policy." + "breakpointDescription": "A run pauses here to let you inspect and edit its data before it continues -- a debugging aid, not policy.", + "rulesForStepHeading": "Rules for this step", + "noRulesApply": "No rules apply to this step. Its defaults decide.", + "editMenuItem": "Edit…", + "removeMenuItem": "Remove", + "kebabAriaLabel": "Actions for {{label}}" }, "nodeInspector": { "nodeType": "Step type", diff --git a/frontend/src/locales/en/views.json b/frontend/src/locales/en/views.json index 25aca08e..32433f9b 100644 --- a/frontend/src/locales/en/views.json +++ b/frontend/src/locales/en/views.json @@ -247,6 +247,59 @@ "resume": "Resume", "approveAndResume": "Approve and resume", "stop": "Stop", - "recentlyResolved": "Recently resolved" + "recentlyResolved": "Recently resolved", + "tabs": { + "queue": "Queue", + "rules": "Rules" + }, + "alwaysButton": "Always…", + "alwaysAllowMenuItem": "Always allow…", + "alwaysDenyMenuItem": "Always deny…", + "alwaysAllowTitle": "Always allow", + "alwaysDenyTitle": "Always deny", + "alwaysContextLine": "{{nodeType}} in {{workflow}} asked to run.", + "alwaysAppliesToLabel": "Applies to", + "alwaysRuleNameLabel": "Rule name", + "alwaysSaveAndApprove": "Save rule and approve", + "alwaysSaveAndDeny": "Save rule and deny", + "alwaysCancel": "Cancel", + "alwaysScopeStep": "Only this step — {{nodeType}} in {{workflow}}", + "alwaysScopeWorkflow": "Any step in {{workflow}}", + "alwaysScopeNodeType": "Every {{nodeType}} step, in any workflow", + "alwaysScopeRequest": "Any step calling {{request}}" + }, + "guardrailRulesPanel": { + "countLabel": "{{count}} rule{{plural}}", + "newRuleButton": "New rule", + "emptyHeading": "No rules yet.", + "emptyDescription": "Each step's defaults decide: external steps ask before running, everything else runs.", + "groupEverywhere": "Everywhere", + "groupStepType": "By step type", + "groupRequest": "By connector request", + "editMenuItem": "Edit…", + "removeMenuItem": "Remove", + "kebabAriaLabel": "Actions for {{label}}" + }, + "guardrailRuleDialog": { + "editTitle": "Edit rule", + "newTitle": "New rule", + "nameLabel": "Rule name", + "effectLabel": "Effect", + "effectOptions": { + "allow": "Allow", + "ask": "Ask", + "deny": "Deny" + }, + "appliesToLabel": "Applies to", + "scopeOptions": { + "everywhere": "Everywhere", + "workflow": "A workflow", + "nodeType": "A step type", + "request": "A connector request" + }, + "conditionSummary": "Condition (advanced)", + "conditionCaption": "An expression over the step's payload, attributes, and config. Leave empty to always apply.", + "save": "Save rule", + "cancel": "Cancel" } } diff --git a/frontend/src/shared/GuardrailRuleDialog.module.css b/frontend/src/shared/GuardrailRuleDialog.module.css new file mode 100644 index 00000000..108f43e6 --- /dev/null +++ b/frontend/src/shared/GuardrailRuleDialog.module.css @@ -0,0 +1,17 @@ +.conditionSection { + margin-top: 4px; +} +.conditionSummary { + cursor: pointer; + padding: 4px 0; + font-size: 12.5px; + font-weight: 650; + color: var(--fgColor-muted); + user-select: none; +} +.conditionSummary:hover { + color: var(--fgColor-default); +} +.scopeSelect { + margin: 4px 0 8px 28px; +} diff --git a/frontend/src/shared/GuardrailRuleDialog.tsx b/frontend/src/shared/GuardrailRuleDialog.tsx new file mode 100644 index 00000000..6e0b073b --- /dev/null +++ b/frontend/src/shared/GuardrailRuleDialog.tsx @@ -0,0 +1,158 @@ +import { useState } from 'react' +import { useTranslation } from 'react-i18next' +import { Dialog, FormControl, Radio, RadioGroup, Select, Text, TextInput, Textarea } from '@primer/react' +import { GuardrailEffect, GuardrailService } from './bindings' +import type { GuardrailRule } from './bindings' +import { useAppStore } from './store' +import { resolveScopeChoice, ruleScopeSentence, RULE_SCOPE_EVERYWHERE_SENTENCE } from './guardrailRuleScope' +import styles from './GuardrailRuleDialog.module.css' +import listStyles from './ListCard.module.css' + +type NewScopeKind = 'everywhere' | 'workflow' | 'nodeType' | 'request' +type EffectValue = 'allow' | 'ask' | 'deny' + +// The shared New/Edit rule form (door 2's "Rules for this step" Edit… +// action, door 3's Review "Rules" audit view) -- one form so a rule +// created or edited from either surface behaves identically. Scope is +// choosable only in NEW mode: an existing rule's scope renders as a +// static sentence (guardrail.Rule's scope fields are set once at +// create time; rescoping means delete and recreate, not an in-place +// edit). +export function GuardrailRuleDialog({ rule, onClose, onSaved }: { + rule?: GuardrailRule + onClose: () => void + onSaved: () => void +}) { + const { t } = useTranslation('views') + const workflows = useAppStore((s) => s.workflows) + const nodeTypes = useAppStore((s) => s.nodeTypes) + const requests = useAppStore((s) => s.requests) + + const [label, setLabel] = useState(rule?.Label ?? '') + const [effect, setEffect] = useState((rule?.Effect as EffectValue) ?? 'allow') + const [scopeKind, setScopeKind] = useState('workflow') + const [workflowId, setWorkflowId] = useState(workflows?.[0]?.ID ?? '') + const [nodeTypeId, setNodeTypeId] = useState(nodeTypes?.[0]?.ID ?? '') + const [requestId, setRequestId] = useState(requests?.[0]?.ID ?? '') + const [condition, setCondition] = useState(rule?.Condition ?? '') + const [saving, setSaving] = useState(false) + const [error, setError] = useState('') + + const editScope = rule ? resolveScopeChoice(rule, workflows, nodeTypes, requests) : null + + const save = async () => { + setSaving(true) + setError('') + try { + if (rule) { + await GuardrailService.UpdateRule({ ...rule, Label: label, Effect: effect as GuardrailEffect, Condition: condition }) + } else { + const scopeFields = scopeKind === 'workflow' ? { WorkflowID: workflowId } + : scopeKind === 'nodeType' ? { NodeTypeID: nodeTypeId } + : scopeKind === 'request' ? { RequestID: requestId } + : {} + await GuardrailService.CreateRule({ + ID: '', Label: label, Effect: effect as GuardrailEffect, Condition: condition, Source: '', + NodeTypeID: '', RequestID: '', WorkflowID: '', NodeID: '', + ...scopeFields, + }) + } + onSaved() + onClose() + } catch (err) { + setError(String(err)) + } finally { + setSaving(false) + } + } + + return ( + void save(), disabled: saving || !label.trim() }, + ]} + > + + {t('guardrailRuleDialog.nameLabel')} + setLabel(e.target.value)} data-testid="guardrail-rule-name" block /> + + + {t('guardrailRuleDialog.effectLabel')} + + + {rule ? ( + + {t('guardrailRuleDialog.appliesToLabel')} + + {editScope ? ruleScopeSentence(editScope) : RULE_SCOPE_EVERYWHERE_SENTENCE} + + + ) : ( + v && setScopeKind(v as NewScopeKind)}> + {t('guardrailRuleDialog.appliesToLabel')} + + + {t('guardrailRuleDialog.scopeOptions.everywhere')} + + + + {t('guardrailRuleDialog.scopeOptions.workflow')} + + {scopeKind === 'workflow' && ( + + )} + + + {t('guardrailRuleDialog.scopeOptions.nodeType')} + + {scopeKind === 'nodeType' && ( + + )} + + + {t('guardrailRuleDialog.scopeOptions.request')} + + {scopeKind === 'request' && ( + + )} + + )} +
+ {t('guardrailRuleDialog.conditionSummary')} + + {t('guardrailRuleDialog.conditionCaption')} +