diff --git a/docs/SPEC.md b/docs/SPEC.md index ab7f9707..08c1e66e 100644 --- a/docs/SPEC.md +++ b/docs/SPEC.md @@ -2598,6 +2598,29 @@ the now-existing mechanism**, not built yet: the same `notify.SendPlain`/ `SendActionable` primitives this pass added would carry it, once a concrete "fire on X" event is chosen. +**Attention escalation — `LOCKED` and built (docs/goals/archive/0023- +attention-escalation.md, ADR-0032's Update).** The `document.hasFocus()`-only +presence gate above had a real bug (focused-but-idle read as present, +observed live) — corrected, plus two layers added: **(1)** an +idle-aware presence gate, `internal/adapters/idletime` (`ioreg -c +IOHIDSystem`, zero cgo) + `SettingsService.isAway` (away = unfocused OR +idle ≥ a Settings-configurable threshold, default 300s; an idletime +read error fails toward away, §8's posture); `NotifyPendingApproval` +now takes the frontend's `hasFocus()` reading as a param instead of +gating client-side. **(2)** a floating approval prompt at the same +away verdict — `#/approvalprompt`, ADR-0033's second-window mechanism +reused (deliberately not `HideOnFocusLost`), Approve/Deny for an MCP +write or "Open in Mill" for a guardrail park. **(3)** alert-style +authorization is now actually requested (`notify.Start` previously +never called `RequestNotificationAuthorization` at all); Settings +documents the System Settings → Notifications → Mill → Alerts toggle. +**(4)** a cross-device forward, `composition.SendJSONWebhook` (reuses +the integration-http/decision-outcome transport tail) + +`SettingsService.ForwardPendingApproval`, default-off, POSTs +`{kind, id, description, createdAt}` to a Settings-configured +HTTPRequest, independent of the presence gate — the layer that reaches +the owner with no local Mac to notify on at all. + **Still `OPEN`, real named gaps:** a menu-bar/dock presence toggle (see above); appearance settings beyond light/dark; a default working directory/scope (blocked on §6); fullscreen window-state tracking diff --git a/docs/adr/0032-mcp-write-approval-park-and-poll.md b/docs/adr/0032-mcp-write-approval-park-and-poll.md index 03417806..ce44fc90 100644 --- a/docs/adr/0032-mcp-write-approval-park-and-poll.md +++ b/docs/adr/0032-mcp-write-approval-park-and-poll.md @@ -172,3 +172,49 @@ this channel; unchanged. - ADR-0017's status gains a pointer here; SPEC §3.6 (MCP approval paragraph) and §3.7 (the notifications `OPEN` item, now partly resolved by this) update in the implementing change. + +## Update — the presence gate corrected from focus to idle+focus, plus a floating prompt and a cross-device forward (docs/goals/0023-attention-escalation.md, delivered) + +The `document.hasFocus()` gate this ADR's §3 originally specified had +a real, observed bug: a focused-but-unattended Mac (the window sitting +in the foreground while the user was away from the desk entirely) +never notified, since a focused window read as "present" regardless of +how long it had sat idle. Corrected, all four items delivered: + +- **Presence moved backend-side.** `internal/adapters/idletime` + (`ioreg -c IOHIDSystem`'s `HIDIdleTime` counter — zero cgo, no TCC + gate, confirmed directly) plus `SettingsService.isAway(focused)`: + away = unfocused OR idle ≥ threshold (default 300s, a Settings + knob). `NotifyPendingApproval` now takes the frontend's own + `document.hasFocus()` reading as a parameter instead of gating + client-side, so App.tsx's per-new-item loop always reports and the + backend makes the one presence decision. An idletime read error + (server mode, or a real desktop failure) fails TOWARD away — §8's + fail-safe posture applied here too. +- **A floating approval prompt**, not just the OS notification: the + same away verdict also shows a small always-on-top window + (`#/approvalprompt`, ADR-0033's second-window mechanism reused + verbatim — Hidden/Frameless/floating/hash-routed — deliberately NOT + `HideOnFocusLost`, since a decision prompt must not vanish just + because focus wandered; Escape is its one explicit dismiss). Shows + the oldest unresolved pending item; Approve/Deny inline for an MCP + write, "Open in Mill" for a guardrail/human-review park (blind + approval from the prompt is never offered, mirroring the + notification's own split above). +- **Alert-style authorization is now actually requested.** Checked + directly against the pinned notifications module's native + implementation: `RequestNotificationAuthorization` has no per-type + parameter to request Alert specifically — it always requests + `UNAuthorizationOptionAlert | Sound | Badge` as one fixed bundle, so + there was nothing to select, only something to call — which + `notify.Start` had never done at all until now (a real gap, not a + design choice). Backgrounded so app startup doesn't block on the + permission dialog. +- **A cross-device forward** for when there's no local Mac to notify + on at all: `composition.SendJSONWebhook` (the exact same transport + tail integration-http/decision-outcome's own webhook already share + — never a second HTTP client) plus `SettingsService.ForwardPendingApproval`, + a fire-and-forget, default-off, Settings-configured POST of + `{kind, id, description, createdAt}` to the owner's own + Configure-authored HTTPRequest (ntfy/Telegram/etc. — §1.1-clean), + independent of the presence gate above. diff --git a/docs/goals/0023-attention-escalation.md b/docs/goals/0023-attention-escalation.md deleted file mode 100644 index 20cef766..00000000 --- a/docs/goals/0023-attention-escalation.md +++ /dev/null @@ -1,35 +0,0 @@ -# 0023 — Attention escalation: decisions reach the human wherever they are - -## Goal -A parked decision (MCP write, guardrail ask) reliably reaches the -owner — including away-from-this-Mac — closing the twice-observed miss -("still very much background-ish pattern and not user attention -pattern"). Layered per the ADR-0032 research + owner discussion. - -## Plan -1. [ ] Floating approval prompt — the incoming-call/askpass pattern: - an always-on-top mini-window Mill draws over whatever app is - focused when a decision parks, Approve/Deny inline. REUSES the - Quick Panel's second-window mechanism (ADR-0033); same - focus-yield-on-dismiss mitigation. -2. [ ] Idle-aware presence gate: replace `document.hasFocus()` (a - focused window on an unattended Mac suppressed the notification by - design — observed live) with system idle time; "present" = - recently-active, not merely focused. Research the cgo-free macOS - idle-seconds option first. -3. [ ] Alert-style notification guidance: request .alert - authorization; Settings copy documents the System Settings → - Mill → Alerts toggle (Duo's own documented ask — banners - auto-dismiss in ~5s, structurally background-ish). Also verify - notification delivery under the dev bundle's ad-hoc signature - (ADR-0032's named unknown; owner observations pending). -4. [ ] Cross-device forward: Settings-configured forward of - pending-decision events to the owner's OWN HTTPRequest (ntfy/ - Telegram/etc.) — §1.1-clean (user-configured connector), the only - layer that reaches the owner at the work machine. - -## Acceptance -Owner at another machine (or idle) learns of a parked decision within -seconds via at least one configured layer, and can resolve it from the -floating prompt on return; the focused-but-idle suppression case is -demonstrably fixed. diff --git a/docs/goals/BACKLOG.md b/docs/goals/BACKLOG.md index ca4348f2..aa1dce6d 100644 --- a/docs/goals/BACKLOG.md +++ b/docs/goals/BACKLOG.md @@ -38,9 +38,12 @@ this pipeline and on this code)** fix the audited green-but-wrong class (12 silent persistence sites, unfailable test fake, uncovered safety-badge source); frontend half already largely clean -3. [ ] [0023 — Attention escalation](0023-attention-escalation.md) — - floating approval prompt + idle-aware presence + alert-style + - cross-device forward (twice-observed missed-decision pain) +3. [x] [0023 — Attention escalation](archive/0023-attention-escalation.md) — + delivered: floating approval prompt (ADR-0033's mechanism reused, + `#/approvalprompt`), idle-aware presence gate (`internal/adapters/idletime`, + backend-side `isAway`), alert-style authorization request (notify.Start), + cross-device forward (`composition.SendJSONWebhook`, + `ForwardPendingApproval`) — see ADR-0032's Update note **Ratified 2026-08-10 (owner): three groups, A→B→C. 0001 stays standing live-review material, interleaved during owner reviews, not a lane.** diff --git a/docs/goals/archive/0023-attention-escalation.md b/docs/goals/archive/0023-attention-escalation.md new file mode 100644 index 00000000..664af2fc --- /dev/null +++ b/docs/goals/archive/0023-attention-escalation.md @@ -0,0 +1,76 @@ +# 0023 — Attention escalation: decisions reach the human wherever they are + +## Goal +A parked decision (MCP write, guardrail ask) reliably reaches the +owner — including away-from-this-Mac — closing the twice-observed miss +("still very much background-ish pattern and not user attention +pattern"). Layered per the ADR-0032 research + owner discussion. + +## Plan +1. [x] Floating approval prompt — the incoming-call/askpass pattern: + an always-on-top mini-window Mill draws over whatever app is + focused when a decision parks, Approve/Deny inline. REUSES the + Quick Panel's second-window mechanism (ADR-0033); same + focus-yield-on-dismiss mitigation. Built: `main.go`'s + `approvalprompt` window (Hidden/Frameless/DisableResize/520×200/ + WindowCentered/Floating/CanJoinAllSpaces/HideOnEscape, deliberately + NOT HideOnFocusLost), `SettingsService.SetApprovalPromptWindow`/ + `showApprovalPrompt`/`DismissApprovalPrompt` + (settingsservice_approvalprompt.go), `app/ApprovalPrompt(App).tsx` + at the `#/approvalprompt` hash route — shows the oldest unresolved + pending item, Approve/Deny for an MCP write, "Open in Mill" for a + guardrail/human-review park (never blind-approve). E2e-covered + (`e2e/approval-prompt.spec.ts`); window-level behavior (floating + level, backend-triggered Show, Escape, focus-yield) stays + manual-only per `.claude/rules/testing.md`. +2. [x] Idle-aware presence gate: replace `document.hasFocus()` (a + focused window on an unattended Mac suppressed the notification by + design — observed live) with system idle time; "present" = + recently-active, not merely focused. Research the cgo-free macOS + idle-seconds option first. Built: `internal/adapters/idletime` + (`ioreg -c IOHIDSystem`'s `HIDIdleTime` counter, zero cgo, no TCC + gate — confirmed directly; unit-tested against a captured real + sample). The presence decision moved backend-side: + `SettingsService.isAway(focused)` — away = unfocused OR + idle≥threshold (default 300s, a Settings knob, + `GetAttentionIdleThreshold`/`SetAttentionIdleThreshold`) — and + `NotifyPendingApproval` now takes the frontend's own + `document.hasFocus()` as a param instead of gating client-side. An + idletime read error (server mode, or a real desktop failure) fails + TOWARD away, per §8's fail-safe posture. +3. [x] Alert-style notification guidance: request .alert + authorization; Settings copy documents the System Settings → + Mill → Alerts toggle (Duo's own documented ask — banners + auto-dismiss in ~5s, structurally background-ish). Also verify + notification delivery under the dev bundle's ad-hoc signature + (ADR-0032's named unknown; owner observations pending). Checked + directly against the pinned notifications module source + (`notifications_darwin.m`): `RequestNotificationAuthorization` has + no per-type parameter — it always requests + `UNAuthorizationOptionAlert | Sound | Badge` as one fixed bundle, + so there's nothing to select beyond calling it, which `notify.Start` + previously never did at all (a real, now-closed gap) — backgrounded + so app startup never blocks on the permission dialog. Settings + copy added naming the System Settings → Notifications → Mill → + Alerts toggle. Notification-delivery-under-dev-signing verification + stays an owner on-machine check, unblocked but not performed here. +4. [x] Cross-device forward: Settings-configured forward of + pending-decision events to the owner's OWN HTTPRequest (ntfy/ + Telegram/etc.) — §1.1-clean (user-configured connector), the only + layer that reaches the owner at the work machine. Built: + `composition.SendJSONWebhook` (the exact transport tail + integration-http/decision-outcome's own webhook already share — + never a second HTTP client), `SettingsService.ForwardPendingApproval` + (fire-and-forget, default off, gated on enabled+configured, fires + `{kind, id, description, createdAt}` as the request's whole body, + independent of the presence gate), a Settings section (enable + toggle + the ADR-0009 `EntityRefField` request picker, reused + directly — `views/` importing `configure/` is allowed by + `.dependency-cruiser.cjs`). Go-tested against a local `httptest` + server via the `SetHTTPRequestLookup` seam. + +## Acceptance +Owner at another machine (or idle) learns of a parked decision within +seconds via at least one configured layer, and can resolve it from the +floating prompt on return; the focused-but-idle suppression case is +demonstrably fixed. 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 4f3be776..176df100 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 @@ -24,7 +24,13 @@ import * as guardrail$0 from "../../domain/guardrail/models.js"; import * as $models from "./models.js"; /** - * CreateRule validates and stores a new rule, minting its ID. + * CreateRule validates and stores a new rule, minting its ID. On a + * persist failure the appended rule is rolled back rather than left + * live in memory only -- a rule that silently failed to save must not + * appear to be gating anything (docs/goals/0025 item 2's memory-vs- + * store consistency rule, applied to guardrail rules too since a + * phantom-saved rule here is worse than most: it's the thing deciding + * whether a step needs approval). */ export function CreateRule(rule: guardrail$0.Rule): $CancellablePromise { return $Call.ByID(3694735496, rule); @@ -32,7 +38,9 @@ export function CreateRule(rule: guardrail$0.Rule): $CancellablePromise { return $Call.ByID(1475597571, id); @@ -58,7 +66,7 @@ export function TestRules(workflowID: string, nodeID: string): $CancellablePromi /** * UpdateRule replaces an existing rule in place, same validation as - * create. + * create; rolls back to the previous rule value if the persist fails. */ export function UpdateRule(rule: guardrail$0.Rule): $CancellablePromise { return $Call.ByID(2711428157, rule); diff --git a/frontend/bindings/github.com/alicoding/mill/internal/services/settingssvc/settingsservice.ts b/frontend/bindings/github.com/alicoding/mill/internal/services/settingssvc/settingsservice.ts index f90209bc..920f3f79 100644 --- a/frontend/bindings/github.com/alicoding/mill/internal/services/settingssvc/settingsservice.ts +++ b/frontend/bindings/github.com/alicoding/mill/internal/services/settingssvc/settingsservice.ts @@ -54,12 +54,27 @@ export function CheckForUpdates(): $CancellablePromise<$models.UpdateCheckResult /** * ClearKeybinding removes commandID's override, if any, reverting it to - * its frontend-declared default binding. + * its frontend-declared default binding. Returns the persist error + * (docs/goals/0025 item 1) rather than swallowing it, and restores the + * removed override in memory if the save failed, so a user who thinks + * they cleared a binding doesn't have it silently come back after a + * restart while the UI shows it as already cleared. */ export function ClearKeybinding(commandID: string): $CancellablePromise { return $Call.ByID(3036020909, commandID); } +/** + * DismissApprovalPrompt hides the floating approval prompt and applies + * the same focus-yield mitigation DismissPanel already uses -- called + * by the prompt's own frontend once the last pending item resolves (or + * there's nothing left to show on mount), the DismissPanel-equivalent + * RPC for this window. + */ +export function DismissApprovalPrompt(): $CancellablePromise { + return $Call.ByID(2270399157); +} + /** * DismissPanel hides the Quick Panel and applies the focus-yield * mitigation -- the bound RPC the panel's own frontend calls after a @@ -71,6 +86,30 @@ export function DismissPanel(): $CancellablePromise { return $Call.ByID(3877156882); } +/** + * ForwardPendingApproval fires the configured cross-device forward for + * one new pending item -- fire-and-forget: it returns immediately, and + * a delivery failure is only slog-logged, never surfaced back to the + * caller or allowed to block anything. App.tsx's own per-new-item loop + * calls this alongside NotifyPendingApproval, unconditionally -- + * unlike the presence gate, forwarding doesn't depend on local + * focus/idle at all, since the whole point is reaching the owner when + * there may be no local Mac attention to gate on in the first place. + */ +export function ForwardPendingApproval(id: string, description: string, kind: string): $CancellablePromise { + return $Call.ByID(4270514639, id, description, kind); +} + +/** + * GetAttentionIdleThreshold returns the configured idle-seconds + * threshold: the presence gate below treats the user as away once + * idletime.Seconds() reaches this, even while the window is focused -- + * the fix for the focused-but-idle suppression case. + */ +export function GetAttentionIdleThreshold(): $CancellablePromise { + return $Call.ByID(420254759); +} + /** * GetBuildInfo reports which commit this running instance was actually * built from (settingsservice_buildinfo.go) -- surfaced in the footer @@ -82,6 +121,21 @@ export function GetBuildInfo(): $CancellablePromise<$models.BuildInfo> { return $Call.ByID(2673585232); } +/** + * GetForwardApprovalsEnabled reports whether the forward is armed. + */ +export function GetForwardApprovalsEnabled(): $CancellablePromise { + return $Call.ByID(3422319504); +} + +/** + * GetForwardApprovalsRequestID returns the configured HTTPRequest's ID + * (empty means unconfigured). + */ +export function GetForwardApprovalsRequestID(): $CancellablePromise { + return $Call.ByID(1205123449); +} + /** * GetLaunchAtLogin queries the real OS state (System Events' login * items list) rather than a persisted preference -- authoritative even @@ -169,16 +223,23 @@ export function ListWorkflowMinutesSaved(): $CancellablePromise<{ [_ in string]? } /** - * NotifyPendingApproval sends an actionable OS notification for a new - * pending item (docs/adr/0032 §3). kind "mcp-write" gets Approve/Deny - * action buttons resolving directly via ResolveMCPWrite; any other kind - * (a guardrail/human-review park) gets a plain notification whose - * default click shows+focuses the window instead -- typed input may be - * required to resolve those, so blind approval from a notification - * isn't offered. + * NotifyPendingApproval sends an actionable OS notification AND shows + * the floating approval prompt (docs/goals/0023 item 1) for a new + * pending item (docs/adr/0032 §3), but ONLY when isAway(focused) says + * the user is away -- the single decision point both surfaces share, so + * "notify" and "show the floating prompt" can never disagree about + * presence. focused is the caller's own document.hasFocus() reading + * (App.tsx) -- only the browser context knows that; everything else + * about presence (idle time, the threshold) is resolved in isAway. + * + * kind "mcp-write" gets Approve/Deny action buttons resolving directly + * via ResolveMCPWrite; any other kind (a guardrail/human-review park) + * gets a plain notification whose default click shows+focuses the main + * window instead -- typed input may be required to resolve those, so + * blind approval from a notification isn't offered. */ -export function NotifyPendingApproval(id: string, description: string, kind: string): $CancellablePromise { - return $Call.ByID(99139683, id, description, kind); +export function NotifyPendingApproval(id: string, description: string, kind: string, focused: boolean): $CancellablePromise { + return $Call.ByID(99139683, id, description, kind, focused); } /** @@ -243,6 +304,36 @@ export function RestoreSummonHotkey(): $CancellablePromise { return $Call.ByID(442997791); } +/** + * SetAttentionIdleThreshold persists seconds (a non-positive value + * resets to the default, mirroring a cleared Settings field rather than + * persisting a nonsensical threshold), returning the persist error -- + * same reasoning as every other settings toggle in this package: a + * save that silently didn't take effect leaves the user believing a + * threshold applies when it doesn't. + */ +export function SetAttentionIdleThreshold(seconds: number): $CancellablePromise { + return $Call.ByID(454955395, seconds); +} + +/** + * SetForwardApprovalsEnabled persists the toggle, returning the persist + * error (same reasoning as every other settings toggle here: a save + * that silently didn't take effect leaves the user believing the + * forward is armed when it isn't, or vice versa). + */ +export function SetForwardApprovalsEnabled(enabled: boolean): $CancellablePromise { + return $Call.ByID(1952175932, enabled); +} + +/** + * SetForwardApprovalsRequestID persists which Configure-authored + * HTTPRequest to forward pending-approval events through. + */ +export function SetForwardApprovalsRequestID(id: string): $CancellablePromise { + return $Call.ByID(60838469, id); +} + /** * SetKeybinding overrides commandID's binding to mods+key, rejecting a * combo already claimed by another command's override, or by a @@ -265,10 +356,24 @@ export function SetLaunchAtLogin(enabled: boolean): $CancellablePromise { return $Call.ByID(2080722787, enabled); } +/** + * SetMCPWriteApprovalRequired returns the persist error, same reasoning + * as SetMCPWriteEnabled -- relaxing this to "unattended" (required = + * false) failing to save silently is a security-relevant gap; + * re-tightening it (required = true) failing to save silently would be + * worse (the user believes approval is required again when it isn't). + */ export function SetMCPWriteApprovalRequired(required: boolean): $CancellablePromise { return $Call.ByID(4059084081, required); } +/** + * SetMCPWriteEnabled returns the persist error (docs/goals/0025 item 1) + * rather than swallowing it -- this is a security-relevant toggle + * (whether an external MCP client may write to this instance at all); + * a click that silently failed to take effect is exactly the kind of + * gap §8's fail-safe posture exists to prevent. + */ export function SetMCPWriteEnabled(enabled: boolean): $CancellablePromise { return $Call.ByID(213255560, enabled); } @@ -361,6 +466,9 @@ export function SuspendMenuAccelerators(): $CancellablePromise { /** * UnassignSummonHotkey removes the app-level summon hotkey, if any. + * Returns the persist error (docs/goals/0025 item 1) rather than + * swallowing it, restoring the in-memory record on failure so it + * matches what's still on disk. */ export function UnassignSummonHotkey(): $CancellablePromise { return $Call.ByID(2578647287); diff --git a/frontend/e2e/approval-prompt.spec.ts b/frontend/e2e/approval-prompt.spec.ts new file mode 100644 index 00000000..bc8cbfa3 --- /dev/null +++ b/frontend/e2e/approval-prompt.spec.ts @@ -0,0 +1,102 @@ +import type { Page } from '@playwright/test' +import type { Client } from '@modelcontextprotocol/sdk/client/index.js' +import { test, expect } from './fixtures/server' +import { clickRowAction } from './inventoryRow' +import { + connectMCPClient, exportWorkflowViaMCP, findWorkflowIdByLabel, + enableMCPWritesWithApprovalRequired, restoreMCPWriteDefaults, +} from './mcpTestClient' + +// Exercises the floating approval prompt's frontend at its hash route +// (/#/approvalprompt, docs/goals/0023-attention-escalation.md item 1, +// app/ApprovalPrompt.tsx) over real Go bindings (Wails3 server mode) -- +// same "the route itself is fully e2e-able headlessly" reasoning +// quick-panel.spec.ts already established for the sibling second +// window (ADR-0033's mechanism, reused here). What's NOT headlessly +// verifiable is the WINDOW-LEVEL behavior around it (floating window +// level, the backend-triggered Show() on an away verdict, Escape via +// the native HideOnEscape option, the focus-yield mitigation) -- those +// belong in the manual-only registry (.claude/skills/run-mill/SKILL.md) +// per .claude/rules/testing.md's own "manual-only registry... never +// silently absent" requirement. + +function workflowRow(page: Page, label: string) { + return page.locator('[data-testid="inventory-row"][data-entity="workflow"]', { has: page.getByText(label, { exact: true }) }) +} + +test('the approval prompt route renders standalone and shows nothing pending', async ({ page }) => { + await page.goto('/#/approvalprompt') + + const prompt = page.getByTestId('approval-prompt') + await expect(prompt).toBeVisible() + await expect(page.getByTestId('approval-prompt-empty')).toBeVisible() + + // No sidebar/PageLayout chrome -- this is the dedicated minimal shell + // (ApprovalPromptApp), not 's tree. + await expect(page.getByRole('link', { name: 'Workflows' })).toHaveCount(0) +}) + +test('a parked MCP write shows in the approval prompt, and Approve resolves it', async ({ page }, testInfo) => { + await enableMCPWritesWithApprovalRequired(page) + + // A source workflow to export/re-import (import always mints a new + // ID -- the exact gated write this lifecycle protects). + await page.getByRole('link', { name: 'Workflows' }).click() + await page.getByTestId('new-workflow').click() + await page.locator('[role="tabpanel"]:not([hidden])').last().getByLabel('Label').fill('E2E approval prompt source') + await page.locator('[role="tabpanel"]:not([hidden])').last().getByTestId('save-workflow').click() + await expect(workflowRow(page, 'E2E approval prompt source')).toBeVisible() + + const client = await connectMCPClient(testInfo.parallelIndex) + let importResultPromise: ReturnType + try { + const sourceId = await findWorkflowIdByLabel(client, 'E2E approval prompt source') + const exported = await exportWorkflowViaMCP(client, sourceId) + + // Fire the gated import -- parks as a durable pending record + // (docs/adr/0032), independent of whether this specific browser + // page happens to be watching for it. + importResultPromise = client.callTool({ name: 'import_workflow', arguments: { json: exported } }) + + // A fresh cross-document navigation to the approval prompt's own + // route (not a same-document hash change from an already-loaded + // page, same discipline quick-panel.spec.ts's own run-target test + // uses) picks up the already-parked write on its first fetch. + await page.goto('about:blank') + await page.goto('/#/approvalprompt') + + const description = page.getByTestId('approval-prompt-description') + await expect(description).toBeVisible({ timeout: 15_000 }) + await expect(description).toContainText('import a workflow') + + await page.getByTestId('approval-prompt-approve').click() + + // Resolved -- the prompt goes back to its empty state (no more + // pending items), which is also what drives the real window's own + // auto-hide (DismissApprovalPrompt). + await expect(page.getByTestId('approval-prompt-empty')).toBeVisible({ timeout: 10_000 }) + + // The write actually executed: a second workflow with the same + // label now exists (import always mints a new ID, never overwrites). + await page.goto('/') + await page.getByRole('link', { name: 'Workflows' }).click() + await expect(workflowRow(page, 'E2E approval prompt source')).toHaveCount(2, { timeout: 10_000 }) + + const result = await importResultPromise + if (result.isError) { + throw new Error(`import_workflow ultimately errored after approval: ${JSON.stringify(result.content)}`) + } + } finally { + await client.close() + } + + // Cleanup: both minted workflows, one at a time (the DOM shifts as + // each row disappears), and the settings toggle. + let remaining = await workflowRow(page, 'E2E approval prompt source').count() + while (remaining > 0) { + await clickRowAction(page, workflowRow(page, 'E2E approval prompt source').first(), 'Delete') + remaining -= 1 + await expect(workflowRow(page, 'E2E approval prompt source')).toHaveCount(remaining) + } + await restoreMCPWriteDefaults(page) +}) diff --git a/frontend/e2e/mcpTestClient.ts b/frontend/e2e/mcpTestClient.ts new file mode 100644 index 00000000..f62e427c --- /dev/null +++ b/frontend/e2e/mcpTestClient.ts @@ -0,0 +1,74 @@ +import type { Page } from '@playwright/test' +import { Client } from '@modelcontextprotocol/sdk/client/index.js' +import { StreamableHTTPClientTransport } from '@modelcontextprotocol/sdk/client/streamableHttp.js' +import { MCP_BASE_PORT, expect } from './fixtures/server' + +// Shared e2e helpers for driving Mill's real MCP server over HTTP +// (docs/adr/0025/0032) -- factored out of mcp-write-approval.spec.ts so +// a second spec needing a parked MCP write (the Review kind filter, +// docs/goals/0002 item 4) doesn't hand-roll a second copy. Not a +// *.spec.ts file itself -- Playwright only picks up files matching its +// own testMatch glob, same "plain helper module" shape as +// inventoryRow.ts. + +export async function connectMCPClient(workerIndex: number): Promise { + const client = new Client({ name: 'mill-e2e', version: '0.0.0' }) + const transport = new StreamableHTTPClientTransport(new URL(`http://127.0.0.1:${MCP_BASE_PORT + workerIndex}`)) + await client.connect(transport) + return client +} + +interface WorkflowIndexEntry { + id: string + label: string +} + +export async function findWorkflowIdByLabel(client: Client, label: string): Promise { + const result = await client.readResource({ uri: 'mill://workflows' }) + const first = result.contents[0] + const text = 'text' in first ? first.text : '' + const list = JSON.parse(text as string) as WorkflowIndexEntry[] + const found = list.find((w) => w.label === label) + if (!found) throw new Error(`workflow "${label}" not found in mill://workflows: ${text}`) + return found.id +} + +export async function exportWorkflowViaMCP(client: Client, id: string): Promise { + const result = await client.callTool({ name: 'export_workflow', arguments: { id } }) + if (result.isError) throw new Error(`export_workflow failed: ${JSON.stringify(result.content)}`) + const content = result.content as Array<{ type: string; text?: string }> + return content[0]?.text ?? '' +} + +// Writes ON, per-write approval left ON (the default) -- the shape most +// callers need, distinct from canvas-live-sync.spec.ts's own +// "unattended" helper which deliberately relaxes approval. +export async function enableMCPWritesWithApprovalRequired(page: Page): Promise { + await page.goto('/') + await page.getByRole('button', { name: 'Settings' }).click() + const writeCheckbox = page.getByTestId('mcp-write-enabled-checkbox') + await expect(writeCheckbox).toBeEnabled() + if (!(await writeCheckbox.isChecked())) { + await writeCheckbox.click() + await expect(writeCheckbox).toBeChecked() + } + const approvalCheckbox = page.getByTestId('mcp-write-approval-checkbox') + await expect(approvalCheckbox).toBeEnabled() + if (!(await approvalCheckbox.isChecked())) { + await approvalCheckbox.click() + await expect(approvalCheckbox).toBeChecked() + } +} + +// Leaves the shared e2e settings file the way a fresh install would +// look (.claude/rules/testing.md's cleanup discipline): write gate off, +// approval required stays on either way. +export async function restoreMCPWriteDefaults(page: Page): Promise { + await page.goto('/') + await page.getByRole('button', { name: 'Settings' }).click() + const writeCheckbox = page.getByTestId('mcp-write-enabled-checkbox') + if (await writeCheckbox.isChecked()) { + await writeCheckbox.click() + await expect(writeCheckbox).not.toBeChecked() + } +} diff --git a/frontend/src/app/App.tsx b/frontend/src/app/App.tsx index 33a7dfac..9c965c74 100644 --- a/frontend/src/app/App.tsx +++ b/frontend/src/app/App.tsx @@ -249,11 +249,15 @@ function App() { // asks the main window to navigate via SettingsService.OpenMainWindow, // which shows/focuses this window and emits this event. Empty-string // view (the panel's "Open Mill" row) means "just show the window," - // no navigation. + // no navigation. 'review' is the floating approval prompt's own + // "Open in Mill" row for a guardrail/human-review park + // (docs/goals/0023 item 1, app/ApprovalPrompt.tsx) -- same mechanism, + // one more target. useEffect(() => { return Events.On('mill-navigate', (evt) => { const target = evt.data as string; if (target === 'settings') setView({ kind: 'settings' }); + if (target === 'review') setView({ kind: 'review' }); }); }, [setView]); @@ -304,16 +308,26 @@ function App() { // PendingMCPWrites (MCPWriteApprovals' own RPC) -- no new backend // surface needed for a number that already exists two ways. // - // Away-user attention layer (docs/adr/0032 §3), folded into the same - // effect since it reads the same two lists: the total count mirrors - // to the dock badge on every change, and each NEW pending item (an id - // not seen on a previous refresh) fires an actionable OS notification - // if the window is unfocused when it arrives -- a present user - // already sees the in-app banner/Review row, so notifying them too - // would be double-noise (document.hasFocus() gate). notifiedIds - // tracks what's already been pushed so a later re-fetch of the same - // still-pending item, or the user simply refocusing later, never - // re-notifies. + // Away-user attention layer (docs/adr/0032 §3, sharpened by + // docs/goals/0023-attention-escalation.md items 1/2/4), folded into + // the same effect since it reads the same two lists: the total count + // mirrors to the dock badge on every change, and each NEW pending + // item (an id not seen on a previous refresh) is reported to the + // backend exactly once -- notifiedIds tracks what's already been + // reported so a later re-fetch of the same still-pending item never + // re-fires either call below. + // + // The presence decision itself (present vs. away) moved BACKEND-side + // (SettingsService.NotifyPendingApproval's own isAway) -- this effect + // only supplies its own document.hasFocus() reading and always calls + // it for every new item; the backend decides whether that's actually + // "present" (focused AND recently-active) or "away" (idle past the + // configured threshold, or unfocused), fixing the previously-observed + // focused-but-idle suppression bug a frontend-only gate couldn't see. + // ForwardPendingApproval (item 4, the cross-device forward) fires + // unconditionally alongside it -- forwarding doesn't depend on local + // focus/idle at all, since its whole point is reaching the owner when + // there may be no local Mac attention to gate on in the first place. const [reviewPendingCount, setReviewPendingCount] = useState(0); const notifiedIds = useRef>(new Set()); useEffect(() => { @@ -335,17 +349,11 @@ function App() { ...mcpPending.map((w) => ({ key: `mcp-write:${w.id}`, id: w.id, description: w.description, kind: 'mcp-write' })), ]; - if (document.hasFocus()) { - // A present user already sees these live -- remember them so - // looking away later doesn't retroactively notify for - // something that arrived while the window had focus. - for (const item of items) notifiedIds.current.add(item.key); - return; - } for (const item of items) { if (notifiedIds.current.has(item.key)) continue; notifiedIds.current.add(item.key); - void SettingsService.NotifyPendingApproval(item.id, item.description, item.kind).catch(() => {}); + void SettingsService.ForwardPendingApproval(item.id, item.description, item.kind).catch(() => {}); + void SettingsService.NotifyPendingApproval(item.id, item.description, item.kind, document.hasFocus()).catch(() => {}); } }); }; diff --git a/frontend/src/app/ApprovalPrompt.module.css b/frontend/src/app/ApprovalPrompt.module.css new file mode 100644 index 00000000..e0d023f9 --- /dev/null +++ b/frontend/src/app/ApprovalPrompt.module.css @@ -0,0 +1,26 @@ +/* The floating approval prompt (docs/goals/0023-attention-escalation.md + item 1). Primer design tokens only, per .claude/rules/frontend.md -- + no bespoke colors/spacing. One compact surface filling the whole + frameless window. */ + +.prompt { + display: flex; + align-items: center; + height: 100vh; + width: 100vw; + padding: var(--base-size-16, 16px); + background: var(--bgColor-default); + color: var(--fgColor-default); + box-sizing: border-box; + overflow: hidden; +} + +.muted { + color: var(--fgColor-muted); + margin: 0; +} + +.error { + color: var(--fgColor-danger); + margin: 0; +} diff --git a/frontend/src/app/ApprovalPrompt.tsx b/frontend/src/app/ApprovalPrompt.tsx new file mode 100644 index 00000000..d768b3eb --- /dev/null +++ b/frontend/src/app/ApprovalPrompt.tsx @@ -0,0 +1,145 @@ +import { useEffect, useMemo, useState } from 'react' +import { Button, Stack, Text } from '@primer/react' +import { ShieldIcon } from '@primer/octicons-react' +import { Events } from '@wailsio/runtime' +import { ExecutionService, SettingsService } from '../shared/bindings' +import type { RunSummary, MCPWriteRequest } from '../shared/bindings' +import styles from './ApprovalPrompt.module.css' + +// docs/goals/0023-attention-escalation.md item 1: the floating +// approval-prompt window's own content -- the incoming-call/askpass +// pattern. Shows the OLDEST unresolved pending item across both +// pending sources (a guardrail/human-review park, an MCP write) -- +// mirrors ReviewView.tsx's own two-source read (ExecutionService.ListRuns +// filtered on r.pending + SettingsService.PendingMCPWrites), but renders +// only the single oldest item rather than a full inbox, since this +// window is a small floating surface, not a second Review queue. +// +// An MCP write gets Approve/Deny inline (SettingsService.ResolveMCPWrite, +// the same RPC MCPWriteApprovals.tsx/ReviewView.tsx already call). A +// guardrail/human-review park gets an "Open in Mill" button instead -- +// never blind-approve, mirroring the OS-notification split +// (settingsservice_attention.go's own NotifyPendingApproval doc +// comment): typed input may be required to resolve one of those. +// +// After resolving (or once nothing is left to show), the window +// auto-hides via SettingsService.DismissApprovalPrompt -- the +// DismissPanel-equivalent RPC for this window (ADR-0033's own +// focus-yield mitigation applies there, Go-side). + +type PromptItem = + | { kind: 'mcp-write'; id: string; description: string; time: number } + | { kind: 'guardrail'; id: string; description: string; time: number } + +export function ApprovalPrompt() { + const [guardrail, setGuardrail] = useState([]) + const [mcpWrites, setMcpWrites] = useState([]) + const [loaded, setLoaded] = useState(false) + const [error, setError] = useState('') + + const refresh = () => { + Promise.all([ + ExecutionService.ListRuns().then((runs) => (runs ?? []).filter((r) => r.pending)).catch(() => []), + SettingsService.PendingMCPWrites().then((p) => p ?? []).catch(() => []), + ]).then(([g, m]) => { + setGuardrail(g) + setMcpWrites(m) + setLoaded(true) + }) + } + + // This window's own React tree is created once at Go startup and + // never remounted (Show()/Hide() just toggle native visibility, same + // as QuickPanel) -- refetch on every regained visibility/focus so a + // resolution made elsewhere (the Review queue, a canvas approve) is + // never stale here, plus the two push events every other pending + // surface already listens to, plus a short poll as a backstop. + useEffect(() => { + refresh() + const offGuardrail = Events.On('guardrail-pending-changed', refresh) + const offMCP = Events.On('mcp-write-approval', refresh) + const timer = window.setInterval(refresh, 2000) + const onVisible = () => { if (document.visibilityState === 'visible') refresh() } + document.addEventListener('visibilitychange', onVisible) + window.addEventListener('focus', refresh) + return () => { + offGuardrail() + offMCP() + window.clearInterval(timer) + document.removeEventListener('visibilitychange', onVisible) + window.removeEventListener('focus', refresh) + } + }, []) + + const items = useMemo(() => { + const g: PromptItem[] = guardrail.map((r) => ({ + kind: 'guardrail', + id: r.runID, + description: `${r.workflowLabel}: ${r.pending?.nodeTypeLabel || r.pending?.nodeTypeID || 'a step'} needs approval`, + time: Date.parse(r.startedAt), + })) + const m: PromptItem[] = mcpWrites.map((w) => ({ + kind: 'mcp-write', + id: w.id, + description: w.description, + time: Date.parse(w.createdAt), + })) + return [...g, ...m].sort((a, b) => a.time - b.time) + }, [guardrail, mcpWrites]) + + const oldest = items[0] ?? null + + // Auto-hide once there's nothing left to show -- gated on `loaded` so + // the very first render (before the first fetch resolves, both lists + // still empty) never dismisses a window that was just shown for a + // real item still in flight over the wire. + useEffect(() => { + if (loaded && items.length === 0) { + void SettingsService.DismissApprovalPrompt().catch(() => {}) + } + }, [loaded, items.length]) + + const resolveWrite = (id: string, approve: boolean) => { + setError('') + SettingsService.ResolveMCPWrite(id, approve).then(refresh).catch((err) => setError(String(err))) + } + + const openInMill = () => { + void SettingsService.OpenMainWindow('review').catch(() => {}) + void SettingsService.DismissApprovalPrompt().catch(() => {}) + } + + return ( +
+ {oldest && ( + + + + {oldest.description} + + {items.length > 1 && ( + +{items.length - 1} more waiting + )} + {oldest.kind === 'mcp-write' ? ( + + + + + ) : ( + + )} + {error && {error}} + + )} + {!oldest && loaded && ( + No pending approvals. + )} +
+ ) +} diff --git a/frontend/src/app/ApprovalPromptApp.tsx b/frontend/src/app/ApprovalPromptApp.tsx new file mode 100644 index 00000000..aedbf8da --- /dev/null +++ b/frontend/src/app/ApprovalPromptApp.tsx @@ -0,0 +1,22 @@ +import { ThemeProvider, BaseStyles } from '@primer/react' +import { ApprovalPrompt } from './ApprovalPrompt' +import { COLOR_MODE_STORAGE_KEY } from './theme' + +// docs/goals/0023-attention-escalation.md item 1: the dedicated shell +// for the floating approval prompt's own Wails window (loaded at the +// '#/approvalprompt' hash route, main.tsx) -- same shape as +// QuickPanelApp (ADR-0033), deliberately minimal, no PageLayout/ +// sidebar/work-tab-strip chrome. Reads the same persisted color-mode +// preference App.tsx's theme switcher writes so this window matches +// whatever the user last chose, rather than defaulting to 'auto' +// independently of it. +export function ApprovalPromptApp() { + const initialColorMode = (localStorage.getItem(COLOR_MODE_STORAGE_KEY) as 'light' | 'dark' | 'auto' | null) ?? 'auto' + return ( + + + + + + ) +} diff --git a/frontend/src/app/main.tsx b/frontend/src/app/main.tsx index f788c7ff..47ff3019 100644 --- a/frontend/src/app/main.tsx +++ b/frontend/src/app/main.tsx @@ -7,6 +7,7 @@ import '@primer/primitives/dist/css/functional/themes/dark.css' import { ThemeProvider, BaseStyles } from '@primer/react' import App from './App' import { QuickPanelApp } from './QuickPanelApp' +import { ApprovalPromptApp } from './ApprovalPromptApp' import { COLOR_MODE_STORAGE_KEY } from './theme' // Read once, synchronously, before the first render, to seed @@ -30,12 +31,20 @@ const initialColorMode = (localStorage.getItem(COLOR_MODE_STORAGE_KEY) as 'light // otherwise needs. QuickPanelApp owns its own ThemeProvider/BaseStyles // (it's a separate, minimal shell, not a view inside 's tree) -- // so the branch happens above that wrapper, not inside a shared one. +// +// The floating approval prompt (docs/goals/0023-attention-escalation.md +// item 1) reuses the exact same mechanism, a second hash route loading +// this same bundle -- ADR-0033's "the Quick Panel is now the reusable +// small-floating-second-window surface" consequence, applied. const isQuickPanel = window.location.hash === '#/quickpanel' +const isApprovalPrompt = window.location.hash === '#/approvalprompt' ReactDOM.createRoot(document.getElementById('root') as HTMLElement).render( {isQuickPanel ? ( + ) : isApprovalPrompt ? ( + ) : ( diff --git a/frontend/src/views/SettingsView.tsx b/frontend/src/views/SettingsView.tsx index 202c58ca..f0b6c094 100644 --- a/frontend/src/views/SettingsView.tsx +++ b/frontend/src/views/SettingsView.tsx @@ -1,10 +1,11 @@ import { useEffect, useState } from 'react' import { Browser } from '@wailsio/runtime' -import { Button, Checkbox, FormControl, Heading, Label, SegmentedControl, Stack, Text, useTheme } from '@primer/react' +import { Button, Checkbox, FormControl, Heading, Label, SegmentedControl, Stack, Text, TextInput, useTheme } from '@primer/react' import { SunIcon, MoonIcon, DeviceDesktopIcon, KeyIcon } from '@primer/octicons-react' import { SettingsService } from '../shared/bindings' import { describeCombo, keyFromEventCode, modsFromEvent, reservedByMacOS } from '../shared/keybinding' import { isAccessibilityError, ACCESSIBILITY_SETTINGS_URL } from '../composition/hotkeyCapture' +import { EntityRefField } from '../configure/EntityRefField' import KeyboardShortcutsSection from './KeyboardShortcutsSection' import styles from '../shared/ListCard.module.css' import PageContainer from '../shared/PageContainer' @@ -48,6 +49,13 @@ function SettingsView() { const [mcpWriteEnabled, setMCPWriteEnabledState] = useState(null) const [mcpApprovalRequired, setMCPApprovalRequiredState] = useState(null) + // Attention/notifications (docs/goals/0023-attention-escalation.md + // items 2/3/4): the idle-aware presence-gate threshold, and the + // cross-device forward's own enable toggle + configured HTTPRequest. + const [idleThreshold, setIdleThresholdState] = useState(null) + const [forwardEnabled, setForwardEnabledState] = useState(null) + const [forwardRequestID, setForwardRequestIDState] = useState('') + useEffect(() => { SettingsService.GetLaunchAtLogin() .then(setLaunchAtLoginState) @@ -61,6 +69,15 @@ function SettingsView() { SettingsService.GetMCPWriteApprovalRequired() .then(setMCPApprovalRequiredState) .catch(console.error) + SettingsService.GetAttentionIdleThreshold() + .then(setIdleThresholdState) + .catch(console.error) + SettingsService.GetForwardApprovalsEnabled() + .then(setForwardEnabledState) + .catch(console.error) + SettingsService.GetForwardApprovalsRequestID() + .then((id) => setForwardRequestIDState(id ?? '')) + .catch(console.error) }, []) // Same menu-accelerator-suspension bracket as @@ -128,6 +145,27 @@ function SettingsView() { SettingsService.UnassignSummonHotkey().then(() => setSummonBinding(null)).catch(console.error) } + // docs/goals/0023-attention-escalation.md item 2: committed on blur + // (a numeric field on every keystroke would spam SetAttentionIdleThreshold + // with half-typed values) -- a non-positive/empty value resets to the + // backend's own default, mirrored client-side by simply refetching. + const commitIdleThreshold = (raw: string) => { + const n = parseInt(raw, 10) + SettingsService.SetAttentionIdleThreshold(Number.isFinite(n) ? n : 0) + .then(() => SettingsService.GetAttentionIdleThreshold()) + .then(setIdleThresholdState) + .catch(console.error) + } + + const toggleForwardEnabled = (enabled: boolean) => { + SettingsService.SetForwardApprovalsEnabled(enabled).then(() => setForwardEnabledState(enabled)).catch(console.error) + } + + const setForwardRequestID = (id: string) => { + setForwardRequestIDState(id) + SettingsService.SetForwardApprovalsRequestID(id).catch(console.error) + } + const checkForUpdates = () => { setUpdateChecking(true) setUpdateStatus('') @@ -252,6 +290,59 @@ function SettingsView() { )} + Notifications + + A parked guardrail ask or MCP write notifies you and shows a floating approval prompt (docs/adr/0032, + docs/goals/0023) once you're away -- not merely unfocused, but idle past the threshold below, or + genuinely unfocused. A present, actively-using-Mill window is never double-noised. + + + Away after (seconds) + commitIdleThreshold(e.target.value)} + disabled={idleThreshold === null} + data-testid="attention-idle-threshold-input" + size="small" + /> + + How long the Mac must sit idle (no keyboard/mouse/trackpad input) while Mill is focused before + you're treated as away -- 300s (5 minutes) by default, matching Teams' own away-status + default. Losing focus entirely always counts as away regardless of this number. + + + + For the notification to alert instead of only appearing quietly in Notification Center, allow it in + System Settings → Notifications → Mill → Alerts (docs/goals/0023 item 3) -- Mill requests notification + permission on first launch, but macOS still defaults new apps to Banners, which auto-dismiss. + + + Forward pending approvals + + toggleForwardEnabled(e.target.checked)} + data-testid="forward-approvals-enabled-checkbox" + /> + Forward to another device + + Off by default (docs/goals/0023 item 4). When on, every new pending guardrail ask or MCP write fires + the integration below (e.g. ntfy/Telegram/a webhook receiver you configure) with{' '} + {'{kind, id, description, createdAt}'} as the body -- the only layer that reaches you + entirely away from this Mac. Fire-and-forget: a delivery failure never blocks the park. + + + {forwardEnabled && ( + + Integration + + + )} + Updates