Skip to content

Commit f1b3bda

Browse files
alicodingclaude
andcommitted
fix: dev-loop instance guards -- concurrent-start refusal + per-rebuild reap (BACKLOG Standing #8)
Two mechanical fixes for the owner-hit THREE-concurrent-mill.dev.app incident (real crash risk on a 16GB machine): (a) Per-rebuild reap: build/config.yml's dev_mode.executes gains a `type: blocking` pkill step right before the `primary` `wails3 task run` step. Blocking steps re-run on every reload cycle (confirmed directly against the vendored github.com/atterpac/refresh engine source), backstopping refresh's own kill-then-restart regardless of whether it actually reaps the previous instance -- a live task dev session was caught, mid-investigation, with two concurrent mill.dev.app processes, one orphaned into a foreign process group refresh's own tracking never caught. Root cause not fully pinned to one line since it's inside a vendored third-party dependency; this reap is an independent, pattern-based backstop rather than a patch to code this repo doesn't own. (b) Concurrent-start guard: internal/devguard (a real Go package, unit-tested) 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 before the existing destructive sweep steps can run -- which would otherwise silently kill a genuinely live first session's own vite/app instead of refusing to start. Verified live against a real running session (correctly detected and refused, naming the actual 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. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018pkViCNAuZp2vBv2K9AbUh
1 parent 8ecad1d commit f1b3bda

7 files changed

Lines changed: 417 additions & 2 deletions

File tree

.claude/skills/run-mill/SKILL.md

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -193,6 +193,29 @@ real desktop build has no automatable hook for these:
193193
four visible×focused combinations) — only the real-window wiring
194194
around it is manual-only.
195195

196+
**Manual-only, a different class of gap: `task dev`'s own concurrent-
197+
start guard and per-rebuild reap (docs/goals/BACKLOG.md Standing #8,
198+
`internal/devguard`, `Taskfile.yml`'s `dev:` task, `build/config.yml`'s
199+
`dev_mode.executes`).** `internal/devguard`'s own decision logic (does
200+
a `ps`/`lsof` snapshot show an existing session) is unit-tested
201+
directly (`guard_test.go`) and was verified live against a genuinely
202+
running `task dev` session during this item's own build (correctly
203+
named the real PID and refused). What CI structurally cannot prove:
204+
CI never runs `task dev` itself (no live file watcher, no real Go
205+
recompile-and-relaunch cycle — the exact reasoning
206+
`.claude/rules/testing.md`'s "Dev-loop timing checks" entry already
207+
gives for `BuildIdentityBadge`'s go-stale state), so (a) the
208+
PER-REBUILD reap in `build/config.yml` actually preventing orphan
209+
accumulation across SEVERAL real Go-triggered rebuilds within one live
210+
session, and (b) a genuine second `task dev` terminal invocation
211+
actually refusing to start (not just the guard binary run standalone),
212+
both stay real desktop-mode manual checks: start `task dev`, touch a
213+
watched `.go` file several times in a row and confirm `ps aux | grep
214+
mill.dev.app` never shows more than one instance after each relaunch,
215+
then (in a second terminal, same repo) run `task dev` again and
216+
confirm it exits immediately naming the first session's PID rather
217+
than launching a second window.
218+
196219
Verification for all five stays a real desktop-mode manual check:
197220
launch via `task dev`, set a summon hotkey in Settings, press it from
198221
another app, confirm the panel appears floating/frameless above

Taskfile.yml

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,28 @@ tasks:
116116
its only flags are -config/-port/-nocolour/-s), so there's no
117117
manual override -- the fingerprint IS the mechanism, which is why
118118
not wiping bin/ (above) matters.
119+
120+
A second concurrent `task dev` REFUSES to start (internal/devguard,
121+
docs/goals/BACKLOG.md Standing #8b), naming the already-running
122+
PID instead of silently killing the first session's own vite/app.
123+
build/config.yml's dev_mode.executes also reaps any leftover
124+
mill.dev.app before each Go-rebuild relaunch WITHIN one session
125+
-- the one residual, CI-unprovable gap (a real live wedge/rebuild
126+
timing under an actually-running watcher) is named explicitly in
127+
the manual-only registry (.claude/skills/run-mill/SKILL.md), not
128+
silently assumed fixed.
119129
cmds:
130+
# Concurrent-start guard (docs/goals/BACKLOG.md Standing #8b,
131+
# owner-hit 2026-08-12 evening: THREE concurrent mill.dev.app
132+
# instances, a real crash risk on a 16GB machine). Runs BEFORE the
133+
# destructive sweep below: a second `task dev` invoked while a
134+
# first is already live must REFUSE to start, not silently kill
135+
# the first session's own vite/app the way the sweep would --
136+
# internal/devguard checks for an already-running `wails3 dev`
137+
# process for this repo and exits non-zero naming its PID, which
138+
# aborts this whole task before the sweep steps ever run (Task's
139+
# own default: stop on the first failing cmd).
140+
- go run ./internal/devguard -port {{.VITE_PORT}}
120141
# Defensive orphan sweep (researched root cause, SPEC §3.8): the
121142
# `wails3 dev` supervisor only traps SIGINT+SIGTERM, not SIGHUP, so
122143
# closing the terminal tab (rather than Ctrl-C) kills the supervisor
@@ -125,7 +146,10 @@ tasks:
125146
# `task dev` has no memory of it and launches a SECOND window. This
126147
# kills any such leftover before starting, so a fresh `task dev`
127148
# always begins from exactly one live instance. `|| true`: a clean
128-
# start (no orphan) is the normal case, not an error.
149+
# start (no orphan) is the normal case, not an error. Only reached
150+
# once the guard above has already confirmed no LIVE `wails3 dev`
151+
# is running, so anything found here is safely presumed a genuine
152+
# orphan, never a second session's own live process.
129153
- pkill -f "{{.BIN_DIR}}/{{.APP_NAME}}.dev.app/Contents/MacOS/{{.APP_NAME}}" || true
130154
# Same failure mode, the vite side (goal 0029, hit live tonight):
131155
# a supervisor killed via SIGHUP can leave the vite dev server

build/config.yml

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,33 @@ dev_mode:
5656
type: blocking
5757
- cmd: wails3 task common:dev:frontend
5858
type: background
59+
# Per-rebuild reap (docs/goals/BACKLOG.md Standing #8a, owner-hit
60+
# 2026-08-12 evening: THREE concurrent mill.dev.app instances,
61+
# traced live to this exact gap). `type: blocking` steps re-run on
62+
# EVERY reload cycle (refresh's own process.ProcessManager.Reload,
63+
# github.com/atterpac/refresh -- confirmed directly against the
64+
# vendored source, not assumed), unlike `background` above (first
65+
# run only). The `primary` step below (wails3 task run) is SUPPOSED
66+
# to have its own previous instance killed automatically by
67+
# refresh's ProcessManager before each restart (a process-group
68+
# SIGKILL) -- but a live-running dev session was directly observed
69+
# with two concurrent mill.dev.app processes from the SAME session,
70+
# one of them orphaned into a foreign process group refresh's own
71+
# tracking never reaped (root cause not fully pinned down: possibly
72+
# a globally-installed `wails3` CLI binary built against a
73+
# different `refresh` version than what this repo's own go.mod
74+
# pins for the app itself). Rather than patch a vendored third-party
75+
# dependency this repo doesn't own, this step is an independent,
76+
# pattern-based reap -- the same `pkill -f` shape Taskfile.yml's own
77+
# dev: task already uses for its start-of-session sweep -- that
78+
# runs regardless of whether refresh's own internal kill succeeds,
79+
# so an orphan can no longer survive past the NEXT rebuild cycle
80+
# even when refresh's own tracking misses it. Matches the exact
81+
# path Taskfile.yml's darwin:run task builds
82+
# (bin/mill.dev.app/Contents/MacOS/mill); `|| true` since the
83+
# common case (nothing stale yet) isn't an error.
84+
- cmd: pkill -f "bin/mill.dev.app/Contents/MacOS/mill" || true
85+
type: blocking
5986
- cmd: wails3 task run
6087
type: primary
6188

docs/SPEC.md

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4213,6 +4213,22 @@ recorded as a real design input (`OPEN`), never silently dropped.
42134213
incidents: `task dev`'s start sweep also clears an orphaned vite-port
42144214
listener (`lsof -ti :9245`), and a non-blocking pre-start disk check
42154215
warns (never blocks) below 2GB free, naming `go clean -cache`.
4216+
**Update (2026-08-13, BACKLOG.md Standing #8): the start-of-session
4217+
sweep above only ever ran ONCE, at `task dev` startup — orphans could
4218+
still accumulate WITHIN one long-running session, one per Go-rebuild
4219+
cycle, directly caught live (a running session had two concurrent
4220+
`mill.dev.app` processes, one orphaned into a foreign process group
4221+
`atterpac/refresh`'s own tracking never reaped — root cause not fully
4222+
pinned to one line in a vendored dependency this repo doesn't own).**
4223+
Two mechanical fixes: `build/config.yml`'s `dev_mode.executes` gained
4224+
a `type: blocking` reap step (`pkill -f` the `.dev.app` binary,
4225+
re-runs every reload cycle, confirmed against the vendored engine
4226+
source) right before the `primary` run step, backstopping refresh's
4227+
own kill regardless of whether it succeeds; and `internal/devguard` (a
4228+
real, unit-tested Go package) runs as `Taskfile.yml`'s `dev:` task's
4229+
first step, refusing a second concurrent `task dev` by naming the
4230+
already-running PID instead of letting the existing sweep silently
4231+
kill a genuinely live first session's own vite/app.
42164232

42174233
## 9.5 Platform kernel & extension contract
42184234

docs/goals/BACKLOG.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,7 @@ live-review material, interleaved during owner reviews, not a lane.**
202202
5. [x] Workflow pins/favorites (tech debt, split from goal 0015's remainder 2026-08-12) — DELIVERED 2026-08-13: `pinnedWorkflowIds: string[]` + `togglePinnedWorkflow` on `shared/store.ts`'s existing zustand `persist` (same localStorage tier as `activeWorkTabKey`, goal 0033's precedent — no new Go surface). `app/workflowFrecency.ts`'s new `sortWorkflowsByPinnedAndFrecency` partitions pinned (in pin-order) above the existing frecency-sorted unpinned tail, reusing `sortWorkflowsByFrecency` rather than a second algorithm. A Primer `PinIcon` `IconButton` trailing-visual pin toggle on both the Quick Panel's and ⌘K palette's workflow rows (muted outline unpinned, accent-colored "filled" once pinned) — found and fixed a real Primer interaction bug along the way: `ActionList.Item`'s own `TrailingVisual` wraps children in a `VisualWrap` span with `pointer-events: none` (trailing visuals are decorative-only by the library's own convention), which silently ate every click on the toggle until `pointer-events: auto` was added back on the button itself. Vitest covers the pinned-above-frecency/pin-order/unpinned-id-dropped/no-mutation cases; `quick-panel.spec.ts` gained a full pin→sort→unpin→revert→reload-persists e2e case.
203203
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.
204204
7. [ ] [0021 — MCP dogfood gap closure](0021-mcp-dogfood-gap-closure.md) Phase 2: orchestrator-driven live MCP probing against the locked-down-enterprise use cases (the mandate names the orchestrator as the prober — self-driveable, exploratory; produces the next ranked gap list). Phase 3 judgments that need the owner surface as they're found.
205-
8. [ ] 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) — two confirmed root causes, both get mechanical fixes: (a) `wails3 dev`'s Go-rebuild cycle relaunches the app WITHOUT killing the previous instance, so orphans accumulate one per rebuild during heavy agent waves — extend goal 0029's start-sweep into a per-rebuild reap (kill any existing `bin/mill.dev.app` process before the new launch; find the right hook in the Taskfile's dev target or wails3 dev's own lifecycle); (b) `task dev` ran twice concurrently (two backgrounded watchers, each with its own app+vite) — the dev target refuses to start when an instance is already running (vite-port 9245 check + wails3-dev process check, clear message naming the existing PID). This makes the standing never-two-data-sharing-instances rule ENFORCED instead of remembered.
205+
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.
206206
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).
207207
10. [ ] 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) — the parallel-worker drag-timing race needs a structural fix: serialize the spec (test.describe.configure mode serial in its own worker), or replace the synthesized drag with keyboard-based column resize if the component supports it, or a deterministic wait on the drag handle's post-layout geometry. Not another timeout bump — three strikes means the approach changes.
208208
11. [ ] 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).

0 commit comments

Comments
 (0)