Skip to content

Commit e337b9c

Browse files
alicodingclaude
andauthored
fix: dev-loop instance guards -- concurrent-start refusal + per-rebuild reap (BACKLOG Standing #8) (#52)
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. Claude-Session: https://claude.ai/code/session_018pkViCNAuZp2vBv2K9AbUh Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent ef99dbe commit e337b9c

5 files changed

Lines changed: 400 additions & 1 deletion

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

internal/devguard/guard.go

Lines changed: 186 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,186 @@
1+
// Package main implements devguard, the `task dev` concurrent-start
2+
// check (docs/goals/BACKLOG.md Standing #8b, owner-hit 2026-08-12
3+
// evening: THREE concurrent mill.dev.app instances in the dock, a real
4+
// crash risk on a 16GB machine). Root cause: `task dev` run a second
5+
// time while a first session was already live -- the existing
6+
// orphan-sweep (Taskfile.yml's `dev:` task, goal 0029) unconditionally
7+
// kills whatever's on the Vite port and any leftover mill.dev.app
8+
// process before starting, which is exactly correct for a genuine
9+
// orphan (a SIGHUP'd terminal's leftover, goal 0029's own target) but
10+
// WRONG for a second concurrent `task dev`: it would kill the FIRST
11+
// session's live vite/app instead of refusing to start, leaving two
12+
// half-torn-down dev loops running against the same data files
13+
// (CLAUDE.md's own never-two-data-sharing-instances rule).
14+
//
15+
// devguard runs as the FIRST step of Taskfile.yml's `dev:` task, before
16+
// the destructive sweep steps: it checks for an already-running
17+
// `wails3 dev` process for THIS repo (the authoritative "is a session
18+
// already live" signal -- matched on the config path Taskfile.yml
19+
// always passes, not just the bare "wails3 dev" substring, so an
20+
// unrelated Wails project's own dev loop elsewhere on the machine never
21+
// false-positives) and exits non-zero, naming the conflicting PID, if
22+
// one is found. Task aborts the whole `dev:` task on the first failing
23+
// step (its own default), so the sweep below never runs in that case.
24+
//
25+
// A bare occupied Vite port with NO live `wails3 dev` process is
26+
// deliberately NOT a block condition here -- that's exactly the
27+
// orphaned-vite-from-a-SIGHUP'd-terminal case goal 0029's sweep already
28+
// exists to clean up safely; blocking on the port alone would break
29+
// that legitimate recovery path. The port is still checked and
30+
// reported as corroborating detail in the refusal message when a live
31+
// process IS found, per this item's own "check vite port + running
32+
// wails3 dev process" spec -- just not as an independent trigger.
33+
package main
34+
35+
import (
36+
"flag"
37+
"fmt"
38+
"os"
39+
"os/exec"
40+
"strconv"
41+
"strings"
42+
)
43+
44+
// process is one line of `ps -axwwo pid=,command=` output. A plain
45+
// struct (not tied to exec.Cmd) so parsing/decision logic below is
46+
// unit-testable without actually running ps -- see guard_test.go.
47+
type process struct {
48+
pid int
49+
command string
50+
}
51+
52+
// parseProcesses parses `ps -axwwo pid=,command=` output. Tolerant of
53+
// the leading whitespace ps pads the pid field with; skips any line
54+
// that doesn't start with a valid integer PID rather than failing the
55+
// whole scan over one malformed line.
56+
func parseProcesses(output string) []process {
57+
var procs []process
58+
for _, line := range strings.Split(output, "\n") {
59+
line = strings.TrimSpace(line)
60+
if line == "" {
61+
continue
62+
}
63+
fields := strings.SplitN(line, " ", 2)
64+
pid, err := strconv.Atoi(fields[0])
65+
if err != nil {
66+
continue
67+
}
68+
command := ""
69+
if len(fields) == 2 {
70+
command = strings.TrimSpace(fields[1])
71+
}
72+
procs = append(procs, process{pid: pid, command: command})
73+
}
74+
return procs
75+
}
76+
77+
// wailsDevMarker is the exact combined substring that identifies THIS
78+
// repo's own `wails3 dev` invocation, spelled precisely the way
79+
// Taskfile.yml's `dev:` task always invokes it (`wails3 dev -config
80+
// ./build/config.yml -port ...`). Deliberately ONE combined string, not
81+
// two independent "wails3 dev" + "build/config.yml" checks: a wails3 v3
82+
// project's default scaffold always names its config `build/config.yml`
83+
// relative to ITS OWN root, so a bare AND of the two loose substrings
84+
// would false-positive on a DIFFERENT wails3 project's own dev loop
85+
// running elsewhere on the same machine (e.g. `-config
86+
// ./other-project/build/config.yml` contains "build/config.yml" too).
87+
// The combined "-config ./build/config.yml" substring only matches this
88+
// repo's own relative invocation path, confirmed against a real
89+
// false-positive this exact scenario produced in guard_test.go before
90+
// being tightened to this shape.
91+
const wailsDevMarker = "-config ./build/config.yml"
92+
93+
// findWailsDevProcess returns the first process that looks like this
94+
// repo's own already-running `wails3 dev` supervisor, excluding
95+
// selfPID (devguard's own process never matches "wails3 dev" in
96+
// practice, but excluding it keeps the function correct regardless).
97+
// Returns nil if none is running.
98+
func findWailsDevProcess(procs []process, selfPID int) *process {
99+
for i := range procs {
100+
p := procs[i]
101+
if p.pid == selfPID {
102+
continue
103+
}
104+
if strings.Contains(p.command, "wails3 dev") && strings.Contains(p.command, wailsDevMarker) {
105+
return &p
106+
}
107+
}
108+
return nil
109+
}
110+
111+
// parsePIDList parses `lsof -ti :<port>` output -- one PID per line,
112+
// empty when nothing is bound to the port.
113+
func parsePIDList(output string) []int {
114+
var pids []int
115+
for _, line := range strings.Split(output, "\n") {
116+
line = strings.TrimSpace(line)
117+
if line == "" {
118+
continue
119+
}
120+
if pid, err := strconv.Atoi(line); err == nil {
121+
pids = append(pids, pid)
122+
}
123+
}
124+
return pids
125+
}
126+
127+
func joinInts(ints []int) string {
128+
strs := make([]string, len(ints))
129+
for i, n := range ints {
130+
strs[i] = strconv.Itoa(n)
131+
}
132+
return strings.Join(strs, ", ")
133+
}
134+
135+
// blockedMessage formats the refusal Taskfile.yml's `dev:` task prints
136+
// before exiting non-zero -- names the actual conflicting PID so the
137+
// owner can act on it directly (kill it, or find its terminal) instead
138+
// of guessing. Only called once devProc is known non-nil (main's own
139+
// gate); portPIDs is optional corroborating detail.
140+
func blockedMessage(devProc *process, portPIDs []int, port int) string {
141+
var b strings.Builder
142+
b.WriteString("task dev is already running -- Mill's own never-two-data-sharing-instances rule (CLAUDE.md) forbids a second concurrent dev loop.\n")
143+
fmt.Fprintf(&b, " wails3 dev is already running (PID %d).\n", devProc.pid)
144+
if len(portPIDs) > 0 {
145+
fmt.Fprintf(&b, " Vite dev port %d is also bound (PID %s).\n", port, joinInts(portPIDs))
146+
}
147+
b.WriteString("Stop the existing session first (kill the PID above, or Ctrl-C its terminal) before starting a new one.")
148+
return b.String()
149+
}
150+
151+
func main() {
152+
port := flag.Int("port", 9245, "the Vite dev-server port to check")
153+
flag.Parse()
154+
155+
// Deliberately exec.Command, not exec.CommandContext: this is a
156+
// short-lived, one-shot CLI invocation (not a long-running server
157+
// request) with no cancellation source to plumb through --
158+
// mirrors internal/adapters/procexec.go's own identical precedent
159+
// and reasoning. Args are fully static (ps) or a parsed int flag
160+
// formatted into a port spec (lsof, never untrusted/user-supplied
161+
// text), not a shell-injection-shaped input.
162+
psOutput, psErr := exec.Command("ps", "-axwwo", "pid=,command=").Output() //nolint:gosec,noctx // static args, one-shot CLI tool, no request context to plumb through
163+
// lsof exits non-zero with empty output when nothing is bound to
164+
// the port -- the common case, not a real error worth surfacing;
165+
// its error is deliberately ignored here, only the (possibly empty)
166+
// output matters.
167+
lsofOutput, _ := exec.Command("lsof", "-ti", fmt.Sprintf(":%d", *port)).Output() //nolint:gosec,noctx // port is a parsed int flag, not untrusted input; one-shot CLI tool
168+
169+
if psErr != nil {
170+
// Fails OPEN, not closed: a missing/broken `ps` on the host is
171+
// a worse regression than the orphan-accumulation bug this
172+
// guards against -- never block every dev-loop start over a
173+
// tooling gap.
174+
fmt.Fprintf(os.Stderr, "devguard: couldn't list processes (%v) -- skipping the concurrent-start check\n", psErr)
175+
os.Exit(0)
176+
}
177+
178+
procs := parseProcesses(string(psOutput))
179+
devProc := findWailsDevProcess(procs, os.Getpid())
180+
if devProc == nil {
181+
os.Exit(0)
182+
}
183+
portPIDs := parsePIDList(string(lsofOutput))
184+
fmt.Fprintln(os.Stderr, blockedMessage(devProc, portPIDs, *port))
185+
os.Exit(1)
186+
}

0 commit comments

Comments
 (0)