feat: opt-in observer autostart so instinct minting runs automatically#104
Conversation
Add instinct.observer.autostart: a .bough.yaml switch that makes the
UserPromptSubmit hook ensure the continuous-learning observer daemon is
running for the monorepo, so instincts are minted automatically once the
operator opts in — instead of remembering a manual `bough observer start`
per machine. The "opt-in once, then automatic" shape the operator asked for.
- config: instinct.observer.{autostart,interval_sec} (off / 10-min default,
floored at 60s). Absent block leaves the cost-safe defaults.
- hook.go: dispatchObserverAutostart wired on UserPromptSubmit (once per
turn, not per tool call). Silent (never touches the hook's stdout) and
best-effort: gate off / no config / already running / spawn failure are
all no-ops that cannot break the prompt.
- observer_daemon.go: extract startObserverDaemon (now shared by `bough
observer start` and the autostart gate so spawn + pid-file bookkeeping
cannot diverge) + observerDaemonRunning.
- doctor.go: report the autostart posture (OFF / ON idle / ON running) so a
background minting daemon is never silent.
Off by default — the daemon spawns claude --print, so bough never starts it
silently; minting stays subject to the self-DoS limiter. This auto-mints
instincts only; clustering them into skills/agents/commands stays the
explicit `bough evolve --generate`.
| } | ||
| logPath = layout.ObserverLog(ident.ID) | ||
| pidPath := observerPidFile(layout, ident.ID) | ||
| if running, existing := daemonRunning(pidPath, ident.Root); running { |
There was a problem hiding this comment.
Check-then-write-pidfile race: resolveObserverProject canonicalises every worktree of a monorepo to the same root/pid path, and dispatchObserverAutostart now runs this on every UserPromptSubmit from a fresh bough hook handle process. Two sessions in different worktrees of the same monorepo can race here - both call daemonRunning(pidPath, ...) before either has written the pid file, both see "not running", and both spawn their own detached daemon. Only the last pidfile write is visible to observer stop/doctor, so the other keeps ticking as an undiscoverable orphan, doubling the LLM call rate.
Adding a short-lived lock file (<pidPath>.lock, atomic O_EXCL, reclaimed if stale) around this check-then-spawn section so concurrent callers can't both pass the gate. Pushing that change now.
| if err != nil || !cfg.Instinct.Observer.Autostart { | ||
| return | ||
| } | ||
| _, _, _, _ = startObserverDaemon(root, observerAutostartInterval(cfg)) |
There was a problem hiding this comment.
This discards all four startObserverDaemon return values including err. If os.Executable() fails, child.Start() fails (permissions, ulimit, OOM), or the pid-file write fails, an operator who opted into autostart gets no daemon, no error, and no log entry anywhere - bough doctor will keep reporting "autostart ON - not running yet" forever with no trail explaining why. The manual bough observer start path surfaces the exact same failure as a clear error.
Since this hook's stdout carries the injected instinct block and must stay silent, logging the error to the observer's own log file (the same place status/doctor already point to) instead of discarding it. Pushing that change now.
| return | ||
| } | ||
| root := resolveMonorepoRoot(cwd) | ||
| cfgPath := filepath.Join(root, ".bough.yaml") |
There was a problem hiding this comment.
This ad hoc filepath.Join(root, ".bough.yaml") + BOUGH_CONFIG check never falls back to .worktree-isolation.yaml, which resolveConfigPath (and Load's own doc comment) still support as a deprecation-warned v0.3 name. A monorepo that has not renamed it yet sets instinct.observer.autostart: true there, loadConfigQuiet errors on the missing canonical path, and this silently returns without ever starting the daemon - bough doctor's autostart line then also (independently) reports OFF, masking the operator's actual config.
Switching to resolveConfigPath via a small shared resolveObserverConfig helper (also used by doctor.go's posture line) fixes the fallback and removes the undocumented BOUGH_CONFIG one-off in favor of the canonical --config flag resolution every other command uses. Pushing that change now.
| // no-ops that never break the prompt. The started daemon detaches its own | ||
| // stdio, and `bough doctor` surfaces its posture — minting is never | ||
| // silent, and it stays subject to the self-DoS limiter. | ||
| func dispatchObserverAutostart() { |
There was a problem hiding this comment.
This cwd -> resolveMonorepoRoot -> cfgPath (+ BOUGH_CONFIG override) -> loadConfigQuiet block is now copy-pasted a third time (the pre-existing dispatchEvolveClaudeMD has the same six lines, and doctor.go's new autostart-posture block reimplements it again).
Fixed together with the finding on line 519 below: extracted resolveObserverConfig (uses resolveConfigPath) and pointed doctor.go's autostart line at the same resolveConfigPath call, so this hook path and bough doctor's reported posture can't drift apart. Left the pre-existing dispatchEvolveClaudeMD copy alone since it isn't part of this diff and changing it would go beyond this PR's scope.
| if err != nil || !cfg.Instinct.Observer.Autostart { | ||
| return | ||
| } | ||
| _, _, _, _ = startObserverDaemon(root, observerAutostartInterval(cfg)) |
There was a problem hiding this comment.
Fair point: when the daemon is already running, this still redoes a full liveness check (including a ps subprocess spawn via pidIsObserverDaemon) on every single UserPromptSubmit turn.
Not fixing this one: since each bough hook handle invocation is a fresh process (no in-memory cache survives between turns), avoiding the ps call would need either a persistent cross-invocation cache (a new subsystem, disproportionate to this PR) or weakening the identity check to a bare syscall.Kill(pid, 0) - which would risk a false "already running" on a recycled pid and silently mask a real double-daemon state, exactly the failure class the finding above (and this PR's lock fix) is trying to close. Leaving it as a known, intentional trade-off rather than guessing at a fix.
…nor legacy config
- observer_daemon.go: startObserverDaemon had a check-then-write-pidfile
race. resolveObserverProject canonicalises every worktree of a
monorepo to the same root/pid path, and the autostart gate now runs
this on every UserPromptSubmit from a fresh process, so two sessions
in different worktrees could both observe "not running" and each
spawn their own orphaned daemon. Added a short-lived lock file
(<pidPath>.lock, atomic O_EXCL, reclaimed if stale after 10s) around
the check-then-spawn section. Also extracted defaultObserverIntervalSec
so the two CLI flag defaults and the autostart fallback share one
source of truth instead of three bare-600 literals.
- hook.go: dispatchObserverAutostart discarded all four
startObserverDaemon return values including the error, so a spawn
failure (os.Executable, child.Start, pid-file write) vanished with no
trace -- doctor would report "not running yet" forever with nothing
explaining why. Now logs the error to the observer's own log file
(never to this hook's stdout, which carries the injected instinct
block). Also replaced the ad hoc filepath.Join(root, ".bough.yaml") +
BOUGH_CONFIG resolution with resolveConfigPath via a new
resolveObserverConfig helper, restoring the legacy
.worktree-isolation.yaml fallback every other bough command honors --
without it, a monorepo still on the legacy name silently never
autostarted.
- doctor.go: pointed the autostart-posture line at the same
resolveConfigPath resolution (dropping its own ad hoc join), so
doctor's reported posture cannot diverge from what the hook actually
does for a legacy-config repo.
Tests: TestAcquireStartLock_{MutualExclusion,ReclaimsStaleLock,
FreshLockNotReclaimed} and TestResolveObserverConfig_LegacyConfigFallback.
Found by code review on PR #104. Not fixed (noted inline + on the PR):
the per-turn daemon liveness check still shells out to ps even when the
daemon is already running -- avoiding that needs a persistent
cross-invocation cache (a new subsystem) or a weaker identity check that
would risk masking a real double-daemon state, so left as a known
trade-off rather than guessing at a fix.
| } | ||
| defer release() | ||
|
|
||
| if running, existing := daemonRunning(pidPath, ident.Root); running { |
There was a problem hiding this comment.
Retrospective review found a still-live duplicate-daemon gap here (as part of a broader sweep — see PR #118 for the fix).
startObserverDaemon's check-then-spawn section trusts the pid file alone (daemonRunning), but observer stop / status / doctor's posture check already fall back to a process-table scan (findDaemonByRoot) when the pid file is missing, stale, or never captured a spawn race — the fix PR #47 shipped for #45. That fallback was never carried into this shared startObserverDaemon, so a live daemon left behind with no (or a wrong) pid file makes this spawn a second daemon for the same root.
That gap was tolerable while observer start was a rare, deliberate operator command. This PR wires the exact same gate onto every UserPromptSubmit via autostart, turning a rare edge case into a standing risk of silently doubling the minting rate — one instance of the exact failure class this review pass was told to watch for on an autostart feature.
Two related issues in the same area, also fixed in PR #118:
dispatchObserverAutostart(internal/cli/hook.go) only logs a spawn failure whenstartObserverDaemonreturns a non-emptylogPath— but it returns""for its two earliest failure modes (resolveObserverProject/EnsureProjectDirserroring), so those failures vanish with zero trace, contradicting this function's own doc comment.InstinctObserver.IntervalSec'svalidate:"omitempty,min=60"tag rejects the entire.bough.yamlfor a sub-60interval_sec, even thoughobserverAutostartIntervalalready floors that value gracefully at read time — a typo silently breaks every unrelated feature sharing the same config load.
fix: retrospective review fixes for merged PR #104
Why
Instinct minting (
bough observer start) was a manual, per-machine step. Thisadds
instinct.observer.autostart— the "opt-in once, then automatic" switch:with it on, the
UserPromptSubmithook ensures the observer daemon is runningfor the monorepo, so instincts mint automatically without remembering a manual
start. Independent of the Claude Code plugin PR (#103); this is pure bough core.
What
instinct.observer.{autostart,interval_sec}(off / 10-min default, floored 60s)dispatchObserverAutostarton UserPromptSubmit — once per turn, silent, best-effortstartObserverDaemon(shared withobserver start) +observerDaemonRunningOff by default — the daemon spawns
claude --print, so bough never starts itsilently; minting stays capped by the self-DoS limiter. Auto-mints instincts
only; clustering into skills/agents/commands stays the explicit
bough evolve --generate.Verification
Live smoke (fresh temp repo; daemon run under a claude-less PATH → zero LLM
calls): gate ON →
observer running, gate OFF →not running,bough doctorshows the posture, no residue.
go test ./internal/...+golangci-lintgreen.