Skip to content

fix(watch): Scope runtime state per project - #158

Merged
JordanCoin merged 6 commits into
JordanCoin:mainfrom
reneleonhardt:fix/project-scoped-runtime-state
Aug 27, 2026
Merged

fix(watch): Scope runtime state per project#158
JordanCoin merged 6 commits into
JordanCoin:mainfrom
reneleonhardt:fix/project-scoped-runtime-state

Conversation

@reneleonhardt

Copy link
Copy Markdown
Contributor

What does this PR do?

Scopes mutable watcher state to the canonical project root and hardens watcher lifecycle handling:

  • Stores pid, state, events, and handoff files in a per-project runtime namespace.
  • Validates linked-worktree and runtime roots before reading or writing state.
  • Waits for detached watcher readiness and preserves bounded cancellation behavior.
  • Keeps setup configuration in the configured .codemap directory.

Why it matters

Projects sharing a setup root could collide on daemon state or stop one another’s watcher. Invalid linked-worktree metadata could also produce misleading success or raw git errors. This keeps ownership and lifecycle outcomes explicit.

CLI / MCP surface

No new commands, arguments, or MCP tools. Existing watch, hook, and handoff flows use project-scoped runtime state.

Security

Runtime markers are accepted only for the canonical owning project and verified process. Invalid roots and foreign ownership fail closed.

Developed with carefully directed, manually reviewed AI assistance.

Co-Authored-By: GPT-5.6 Sol codex@openai.com

reneleonhardt and others added 4 commits August 27, 2026 14:08
Namespace mutable files per canonical project and use one ownership-checked bridge for legacy daemon transitions.

Co-Authored-By: GPT-5.6 Sol <codex@openai.com>
Co-Authored-By: GPT-5.6 Sol <codex@openai.com>
Route MCP watcher control through the CLI owner and reject poisoned runtime markers without fallback writes.

Co-Authored-By: GPT-5.6 Sol <codex@openai.com>
Wait for detached watcher initialization before reporting CLI or MCP success, and preserve bounded lifecycle completion across request cancellation.

Co-Authored-By: GPT-5.6 Sol <codex@openai.com>
@reneleonhardt
reneleonhardt force-pushed the fix/project-scoped-runtime-state branch from ed0f36a to b196eed Compare August 27, 2026 14:16

Copy link
Copy Markdown
Owner

Reviewed in depth with live daemon testing (concurrent starts, kill -9 recovery, pid-reuse attacks, symlink/worktree keying — details at the bottom). The lifecycle core is genuinely solid, and the ownership gate on terminateDaemon fixes a real pre-existing hole where Unix would SIGTERM an unverified pid. One blocking item and one worth fixing in the same push:

1. BLOCKING — legacy-daemon detection misses the immediately-prior --setup-root layout, so upgrading duplicates daemons

ResolveActiveRuntime's legacy fallback (watch/state.go:152, via internal/projectpath/runtime.go:26-45) only covers the pre-#122 flat layout (<setup>/.codemap/watch.pid). It does not cover what main used until this PR for --setup-root projects: <setup>/.codemap/projects/<key>/watch.pid. Reproduced end-to-end: daemon started with a main-built binary under --setup-root, then watch status with this PR's binary → "not running"; watch start → a second live daemon watching the same directory (both pids alive, old and new pid files coexisting). That silently double-watches exactly the shared-setup-root scenario this PR is titled for. Fix: add <setup>/.codemap/projects/<ProjectKey> as a legacy candidate in ResolveActiveRuntime's list — and a test for it; TestResolveActiveRuntimeUsesOnlyExactlyOwnedLiveLegacyDaemon currently only exercises the flat layout.

2. Fix in the same push — Release() before Pid is read

main.go:809-816: os.Process.Release() sets Pid = -1 on Linux, and the code releases before reading. Every successful start prints "Watch daemon started (pid -1)" (confirmed), and on the readiness-timeout branch RemoveProcessPID(absRoot, cmd.Process.Pid) receives -1, so removePIDIfMatches no-ops and the just-published pid file is left behind (the next start recovers it via the stale path, but the designed cleanup never runs). Capture pid := cmd.Process.Pid before Release(), or defer the release past both uses.

Non-blocking

  • RuntimeCodemapDir (internal/projectpath/path.go:172-184) keeps a silent fail-open fallback on SelectRuntime error. Every current production caller pre-validates, so it's latent — but it undercuts the PR's own fail-closed invariant for the next caller. A checked counterpart (essentially CheckedRuntimeCodemapDir) routed through the watch-state call sites would close it.
  • mcp/main.go: handleStartWatch no longer populates the in-process watchers map (good — delegating to the subprocess gives MCP the same lock/readiness/ownership guarantees), which leaves the daemon != nil branches in handleStopWatch/handleGetActivity production-unreachable. Fine as a follow-up cleanup.

What held up under attack

Two-project isolation with kill -9 + restart (stale .pid.stale evidence preserved); pid-reuse — a stale pid file pointing at an unrelated live process gets ErrForeignDaemonPID and the process is never signaled; concurrent watch start for one project serialized by the transition lock (one daemon, clean "already running" for the loser, a dozen trials); symlink vs real path, trailing slash, and linked worktree vs primary all key identically; plain non-git directories unchanged; invalid .codemap state fails fast (~6ms) with a bounded error before forking. go vet clean (including a windows/amd64 cross-compile), full suite green apart from the three known root-environment tests, go test ./watch -count=3 and -race -count=2 stable.

Sequencing / issue notes

No conflicts with main or the open #159 (verified via merge-tree; the only shared file is main.go in disjoint regions — either merge order is safe). On the open issues: this meaningfully advances #155 (serialization + ownership; the reaper gap remains — a daemon whose project dir is deleted still becomes unstoppable, same as main), and doesn't touch #140/#151/#152 despite being adjacent — worth "Relates to #155" in the merge, nothing stronger.

With items 1–2 fixed, this is ready to land.


Generated by Claude Code

@reneleonhardt

Copy link
Copy Markdown
Contributor Author

Complied

  • Added migration detection and regression coverage for the immediately-prior project-scoped --setup-root daemon layout.
  • Captured the daemon PID before release and reused it for readiness cleanup and startup output.

@JordanCoin
JordanCoin merged commit d0649db into JordanCoin:main Aug 27, 2026
12 checks passed

Copy link
Copy Markdown
Owner

Verified and merged. Reproduced the migration scenario live against the new head: a daemon started by a main-built binary under --setup-root in the prior layout is now seen by the new binary's status, start refuses to duplicate ("already running", one process), and stop cleanly terminates it. The pid capture fix checks out in the diff and both new test assertions guard it. Full go vet + go test ./... clean against main. Solid work on the lifecycle hardening — the ownership gate before signaling closes a real pre-existing hole.


Generated by Claude Code

@reneleonhardt
reneleonhardt deleted the fix/project-scoped-runtime-state branch August 27, 2026 20:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants