feat: guardrail rule authoring -- rule-from-park, in-context edit, Review rules audit (goal 0078)

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;
 }

frontend/bindings/github.com/alicoding/mill/internal/services/guardrailsvc/guardrailservice.ts

@@ -53,6 +53,22 @@ export function Rules(): $CancellablePromise<guardrail$0.Rule[] | null> {
     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<guardrail$0.Rule[] | null> {
+    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

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<void> {
   const deadline = Date.now() + timeoutMs

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 })
+  }
+})

frontend/src/app/ApprovalPrompt.tsx

@@ -134,9 +134,20 @@ export function ApprovalPrompt() {
               </Button>
             </Stack>
           ) : (
-            <Button size="small" variant="primary" onClick={openInMill} data-testid="approval-prompt-open">
-              {t('approvalPrompt.openInMill')}
-            </Button>
+            <Stack direction="horizontal" gap="condensed">
+              <Button size="small" variant="primary" onClick={openInMill} data-testid="approval-prompt-open">
+                {t('approvalPrompt.openInMill')}
+              </Button>
+              {/* 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. */}
+              <Button size="small" variant="invisible" onClick={openInMill} data-testid="approval-prompt-set-rule">
+                {t('approvalPrompt.setRule')}
+              </Button>
+            </Stack>
           )}
           {error && <Text size="small" className={styles.error}>{error}</Text>}
         </Stack>