Skip to content

Commit 05da75f

Browse files
alicodingclaude
andcommitted
feat: one-shot dock bounce on parked approvals while away (BACKLOG Standing #9)
NotifyPendingApproval's away branch now requests a single informational dock bounce (window.Flash, wails beta.6's macOS support) alongside the OS notification and floating prompt -- same kernel attention-layer class as the dock badge (ADR-0035). The notify adapter's cgo send aborts headless, so only the nil-window guard is unit-testable; the real bounce is a manual-only desktop check (testing.md). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FJ8wStsHyu7XPLTspNjMnQ
1 parent bae7ada commit 05da75f

5 files changed

Lines changed: 62 additions & 5 deletions

File tree

.claude/rules/testing.md

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,14 @@ layer per capability," never "a seed per thing":
128128
(the seeded integrations' endpoints), non-blocking.
129129
- **Manual-only registry** — OS-bound checks (hotkey delivery, real
130130
clipboard, tray) listed explicitly with reasons, never silently
131-
absent (see goal 0010's enforcement).
131+
absent (see goal 0010's enforcement). Non-seed instance: the
132+
away-attention dock bounce (`dockBounceFn`,
133+
`settingsservice_attention.go`) — only its nil-window guard is
134+
unit-testable (the notify adapter's cgo send aborts headless, so
135+
the full away branch can't run under `go test`); the real
136+
bounce-once-on-a-parked-approval behavior is OS-bound and
137+
CI-invisible — verify it desktop-mode by parking an approval while
138+
unfocused.
132139
- **Dev-loop timing checks** — a non-seed instance of the same manual-
133140
only discipline, outside goal 0010's seed/NodeType registry (that
134141
machinery is keyed to seeded artifacts; this isn't one). Goal 0029's

docs/SPEC.md

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2505,9 +2505,12 @@ turns out to solve this without touching that dispute).
25052505
executes at-most-once. **The away-user attention layer (ADR-0032 §3)
25062506
is also built**: the sidebar's already-computed pending-decision count
25072507
mirrors to the dock badge (`internal/adapters/dockbadge`, wraps Wails3's
2508-
`dock.DockService`), and a new pending item while the window is
2509-
unfocused fires a real OS notification (`internal/adapters/notify`,
2510-
wraps `notifications.NotificationService`) — an MCP write's carries
2508+
`dock.DockService`), and a new pending item while the user is away
2509+
fires a real OS notification (`internal/adapters/notify`,
2510+
wraps `notifications.NotificationService`) plus a one-shot dock
2511+
bounce (`window.Flash`, a single informational attention request —
2512+
never repeating/critical; same kernel attention-layer class as the
2513+
badge) — an MCP write's notification carries
25112514
Approve/Deny actions resolving directly; a guardrail/human-review
25122515
park's opens/focuses the window instead (typed input may be needed).
25132516
Both adapters are desktop-only (server-mode build-tag stub, same

docs/goals/BACKLOG.md

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -217,7 +217,16 @@ live-review material, interleaved during owner reviews, not a lane.**
217217
6. [x] ⌘?/⌘/ multi-binding keybinding alias (tech debt, split from goal 0015's remainder 2026-08-12) — DELIVERED 2026-08-13: `Command` grew an optional `extraBindings: KeyCombo[]` alongside `defaultBinding` (`shared/commands.ts`, backward-compatible); `shared/keybinding.ts`'s `keyFromEventCode` gained `/` support (shift-independent, same as every other key — the Shift mod is what distinguishes ⌘/ from ⌘?, both on the physical Slash key). `palette.open` carries both as `extraBindings`, checked against the full registry + `RESERVED_COMBOS` first (no collision — nothing else uses `/`). `dispatchCommandForEvent` checks a command's effective (override-aware) primary plus its extras every dispatch; extras themselves are deliberately NOT override-checked this pass (Settings' recorder-based rebinding UI still edits only the primary). `views/KeyboardShortcutsSection.tsx` renders extras as read-only secondary `KeyComboChip`s next to the primary's click-to-rebind button. Vitest covers dispatch-matches-either-binding + override-doesn't-disable-extras + no-extraBindings-backward-compat; `keymap.spec.ts` gained both a live ⌘//⌘⇧+/ → palette-opens case and a Settings-renders-the-two-read-only-chips case.
218218
7. [ ] [0021 — MCP dogfood gap closure](0021-mcp-dogfood-gap-closure.md) Phase 2 (orchestrator-driven live MCP probing) + Phase 3 (that probe's gap closure) DELIVERED 2026-08-13: the live author-from-scratch + full `run_workflow_stepped` probe found the loop mechanics themselves sound, plus two interop gaps — inconsistent identifier argument names across tools (`id` vs. `runId` vs. `workflowId`) and `run_workflow` always landing `test` kind with no way to opt out (silently excluding real agent-triggered production runs from Home's metrics). Both fixed same day: every workflow-identifying tool now accepts a canonical `workflowId` alongside its original `id` (one shared `resolve()` helper, backward compatible); `run_workflow` gained an optional `test` boolean (default `false` → new `RunKindMCP`, counted in Home's automation metrics like a real trigger fire; `test:true` → `RunKindTest`, excluded, matching the UI's Test button) — `run_workflow_stepped` stays unconditionally `test` kind (debug surface). Real-MCP-client Go tests prove both. Left UNCHECKED / not archived: the goal file's own separate Phase 4 (Confluence-markdown-fidelity fixture corpus + M365 bridge dry run) remains genuinely open, substantial, unaddressed work outside this round's scope — the file's own acceptance bar is the owner calling the whole surface real-use-ready, not one phase closing.
219219
8. [x] Dev-loop instance guards (tech debt, owner-hit 2026-08-12 evening: THREE concurrent `mill.dev.app` instances in the dock, real crash risk on the 16GB machine) — DELIVERED 2026-08-13, both mechanical fixes landed: (a) per-rebuild reap — `build/config.yml`'s `dev_mode.executes` gained a `type: blocking` `pkill -f "bin/mill.dev.app/Contents/MacOS/mill" || true` step right before the `primary` `wails3 task run` step (blocking steps re-run every reload cycle, confirmed directly against the vendored `github.com/atterpac/refresh` engine source — its own `Primary` case already SHOULD kill-then-restart via a process-group SIGKILL, but a live `task dev` session running during this item's own investigation was caught red-handed with two concurrent `mill.dev.app` processes, one orphaned into a foreign process group refresh's own tracking never reaped; root cause not fully pinned to one line since it's in a vendored third-party dependency, so this reap is an independent, pattern-based backstop rather than a patch to code this repo doesn't own — same shape Taskfile.yml's own start-of-session sweep already used); (b) `task dev` now refuses a second concurrent start — `internal/devguard` (a real Go package, `package main`, unit-tested: `guard_test.go` covers process-list parsing, the wails3-dev-process matcher incl. a real false-positive it caught and fixed against a differently-pathed sibling project, port-PID parsing, and message formatting) runs as the FIRST step of `Taskfile.yml`'s `dev:` task, checks for an already-running `wails3 dev` process for this exact repo, and exits non-zero naming the conflicting PID (+ any port occupancy as corroborating detail) before the destructive sweep steps can run — verified live against a genuinely running session (correctly detected + refused, naming the real PID). Manual-only registry entry added (`.claude/skills/run-mill/SKILL.md`) for what CI structurally can't prove: real per-rebuild-orphan-prevention across several live Go-triggered rebuilds, and a genuine second-terminal `task dev` invocation actually refusing to start.
220-
9. [ ] Dock-bounce on parked approvals (small, unlocked by wails beta.6's Flash() gaining macOS support via NSApp requestUserAttention — PR #44's changelog finding; Mill calls Flash nowhere today) — the attention stack (goal 0023/ADR-0032's away-user layer) gains a one-shot dock bounce when an approval parks while the user is away; kernel attention-layer surface per ADR-0035 (same class as the dock badge), NOT a new composition path. Tiny: one call site in the existing NotifyPendingApproval flow + manual-only registry entry (real dock behavior isn't CI-testable).
220+
9. [x] Dock-bounce on parked approvals — DELIVERED 2026-08-13 exactly
221+
as scoped: `dockBounceFn` seam + one call site in
222+
`NotifyPendingApproval`'s away branch (`window.Flash`
223+
NSInformationalRequest, bounces once and self-completes — never
224+
repeating/critical), same kernel attention-layer class as the dock
225+
badge (ADR-0035); nil-window guard unit-tested, the full away
226+
branch isn't headless-testable (notify's cgo send aborts under
227+
`go test` — discovered here, recorded in testing.md's manual-only
228+
note along with the desktop-mode verification step); SPEC §3.6's
229+
attention-layer paragraph updated.
221230
10. [x] resizable-table.spec.ts drag-timing flake, PROPER fix (3 confirmed recurrences POST-hardening: PR #24 original, #43's run, #44's run — the expect.poll hardening from PR #33's wave was insufficient) — DELIVERED 2026-08-13. A keyboard-based resize alternative was checked and ruled out (`shared/ResizableTable.tsx`'s drag handle is built entirely on pointerdown/pointermove/pointerup, no keyboard path at all — building one would be a real feature addition, not a test fix). Landed instead: `test.describe.configure({ mode: 'serial' })` makes the file's own never-interleave requirement explicit rather than an incidental side effect of global config; `waitForStableBoundingBox` replaces the old non-null-only `expect.poll` with a poll for the box being IDENTICAL across two consecutive reads (a non-null box mid-reflow was always possible the old check couldn't see); the synthesized drag itself moved from one batched `page.mouse.move(..., { steps: N })` call to discrete, individually-awaited moves (a browser can coalesce rapid pointermove events within one CDP command — a real, documented behavior via the `PointerEvent.getCoalescedEvents()` API, not Playwright-specific); and two more `expect.poll`s wait for the drag's actual DOM effect and the `localStorage` persist to land before the next step depends on them, instead of assuming the previous Playwright command's own resolution implies the page's JS listener already ran. Verified via 5 separate fresh `npx playwright test` invocations (10/10 passed) — an in-process `--repeat-each` stress loop on the same worker turned out to be a self-confounding methodology (accumulating browser/worker degradation unrelated to the fix, not representative of a real CI run) once cross-checked against genuinely fresh runs.
222231
11. [x] 0030 second-pass linters (gocritic/prealloc/contextcheck/sqlclosecheck — named future work in goal 0028) + `.ls-lint.yml` gains a root `node_modules` ignore (gap found 2026-08-12: a stray root node_modules broke root-file-naming; tiny, rides this or any PR) — DELIVERED 2026-08-13. All four enabled in `.golangci.yml`; triaged to zero findings on both build-tag variants (default + `server`) — 14 real findings fixed (1 `gocritic` assignOp in a test file, 13 `prealloc` slice-capacity hints across `seedproof_test.go` and `millmcpservice.go`'s resource-index readers), zero `contextcheck`/`sqlclosecheck` findings, zero rules needed a scoped tweak or a `//nolint` suppression this pass. `.ls-lint.yml`'s root `ignore:` list gained `node_modules` with a comment explaining the root-scoped-recursive-rule interaction.
223232

internal/services/settingssvc/settingsservice_attention.go

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"github.com/alicoding/mill/internal/adapters/dockbadge"
1010
"github.com/alicoding/mill/internal/adapters/idletime"
1111
"github.com/alicoding/mill/internal/adapters/notify"
12+
"github.com/wailsapp/wails/v3/pkg/application"
1213
)
1314

1415
// The away-user attention layer (docs/adr/0032 §3, sharpened by
@@ -102,6 +103,18 @@ func (s *SettingsService) SetPendingBadge(count int) {
102103
}
103104
}
104105

106+
// dockBounceFn is NotifyPendingApproval's one seam to the OS dock
107+
// bounce: window.Flash maps to a single NSInformationalRequest on
108+
// macOS (bounces once and self-completes; disabling is a no-op) and a
109+
// taskbar flash on Windows. A package var so the away-branch wiring is
110+
// unit-testable; the real dock behavior is OS-bound and stays a
111+
// manual-only check (.claude/rules/testing.md).
112+
var dockBounceFn = func(w *application.WebviewWindow) {
113+
if w != nil {
114+
w.Flash(true)
115+
}
116+
}
117+
105118
// notificationID encodes which pending-item KIND a delivered
106119
// notification was for directly into its own ID (kind + ":" + id) --
107120
// the simplest way for the OnNotificationResponse routing below to
@@ -131,6 +144,16 @@ func (s *SettingsService) NotifyPendingApproval(id, description, kind string, fo
131144
// not-harder-than-baseline lock).
132145
return nil
133146
}
147+
// One-shot dock bounce, same kernel attention-layer class as the
148+
// dock badge (docs/adr/0032 §3, ADR-0035's kernel/composition
149+
// boundary): an away user gets a single informational bounce per
150+
// newly-parked item, alongside the notification and the floating
151+
// prompt -- never a repeating/critical request.
152+
s.mu.Lock()
153+
w := s.window
154+
s.mu.Unlock()
155+
dockBounceFn(w)
156+
134157
const title = "Mill: approval needed"
135158
notifID := notificationID(kind, id)
136159
var sendErr error
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
package settingssvc
2+
3+
import "testing"
4+
5+
// go test never runs against a live application.New() (see
6+
// settingsservice_menu_test.go), and the notify adapter's cgo send
7+
// aborts in a headless test process -- so NotifyPendingApproval's full
8+
// away branch is not callable here. This covers the nil-window guard
9+
// only (same reasoning as settingsservice_approvalprompt_test.go); the
10+
// real dock bounce is OS-bound and manual-only
11+
// (.claude/rules/testing.md).
12+
13+
func TestDockBounceFn_NilWindow_DoesNotPanic(t *testing.T) {
14+
dockBounceFn(nil)
15+
}

0 commit comments

Comments
 (0)