Commit Graph

182 Commits

Author SHA1 Message Date
Alex Newman c648d5d8d2 feat: Knowledge Agents — queryable corpora from claude-mem (#1653)
* feat: add knowledge agent types, store, builder, and renderer

Phase 1 of Knowledge Agents feature. Introduces corpus compilation
pipeline that filters observations from the database into portable
corpus files stored at ~/.claude-mem/corpora/.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* feat: add corpus CRUD HTTP endpoints and wire into worker service

Phase 2 of Knowledge Agents. Adds CorpusRoutes with 5 endpoints
(build, list, get, delete, rebuild) and registers them during
worker background initialization alongside SearchRoutes.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* feat: add KnowledgeAgent with V1 SDK prime/query/reprime

Phase 3 of Knowledge Agents. Uses Agent SDK V1 query() with
resume and disallowedTools for Q&A-only knowledge sessions.
Auto-reprimes on session expiry. Adds prime, query, and reprime
HTTP endpoints to CorpusRoutes.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* feat: add MCP tools and skill for knowledge agents

Phase 4 of Knowledge Agents. Adds build_corpus, list_corpora,
prime_corpus, and query_corpus MCP tools delegating to worker
HTTP endpoints. Includes /knowledge-agent skill with workflow docs.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: handle SDK process exit in KnowledgeAgent, add e2e test

The Agent SDK may throw after yielding all messages when the
Claude process exits with a non-zero code. Now tolerates this
if session_id/answer were already captured. Adds comprehensive
e2e test script (31 assertions) orchestrated via tmux-cli.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: use settings model ID instead of hardcoded model in KnowledgeAgent

Reads CLAUDE_MEM_MODEL from user settings via getModelId(), matching
the existing SDKAgent pattern. No more hardcoded model assumptions.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* feat: improve knowledge agents developer experience

Add public documentation page, rebuild/reprime MCP tools, and actionable
error messages. DX review scored knowledge agents 4/10 — core engineering
works (31/31 e2e) but the feature was invisible. This addresses
discoverability (docs, cross-links), API completeness (missing MCP tools),
and error quality (fix/example fields in error responses).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* docs: add quick start guide to knowledge agents page

Covers the three main use cases upfront: creating an agent, asking a
single question, and starting a fresh conversation with reprime. Includes
keeping-it-current section for rebuild + reprime workflow.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: address code review issues — path traversal, session safety, prompt injection

- Block path traversal in CorpusStore with alphanumeric name validation and resolved path check
- Harden system prompt against instruction injection from untrusted corpus content
- Validate question field as non-empty string in query endpoint
- Only persist session_id after successful prime (not null on failure)
- Persist refreshed session_id after query execution
- Only auto-reprime on session resume errors, not all query failures
- Add fenced code block language tags to SKILL.md

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: address remaining code review issues — e2e robustness, MCP validation, docs

- Harden e2e curl wrappers with connect-timeout, fallback to HTTP 000 on transport failure
- Use curl_post wrapper consistently for all long-running POST calls
- Add runtime name validation to all corpus MCP tool handlers
- Fix docs: soften hallucination guarantee to probabilistic claim
- Fix architecture diagram: add missing rebuild_corpus and reprime_corpus tools

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: enforce string[] type in safeParseJsonArray for corpus data integrity

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: add blank line before fenced code blocks in SKILL.md maintenance section

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-08 17:30:20 -07:00
Alex Newman abd55977ca fix(mcp): MCP server crashes with Cannot find module 'bun:sqlite' under Node (#1645)
* fix(mcp): MCP server crashes with Cannot find module 'bun:sqlite' under Node

The MCP server bundle (mcp-server.cjs) ships with `#!/usr/bin/env node` so
it must run under Node, but commit 2b60dd29 added an import of
`ensureWorkerStarted` from worker-service.ts. That import transitively pulls
in DatabaseManager → bun:sqlite, blowing up at top-level require under Node.

The bundle ballooned from ~358KB (v11.0.1) to ~1.96MB (v12.0.0) and crashed
on every spawn, breaking the MCP server entirely for Codex/MCP-only clients
and any flow that boots the MCP tool surface.

Fix:

1. Extract `ensureWorkerStarted` and the Windows spawn-cooldown helpers
   into a new lightweight module `src/services/worker-spawner.ts` that
   only imports from infrastructure/ProcessManager, infrastructure/HealthMonitor,
   shared/*, and utils/logger — no SQLite, no ChromaSync, no DatabaseManager.

2. The new helper takes the worker script path explicitly so callers
   running under Node (mcp-server) can pass `worker-service.cjs` while
   callers already inside the worker (worker-service self-spawn) pass
   `__filename`. worker-service.ts keeps a thin wrapper for back-compat.

3. mcp-server.ts now imports from worker-spawner.js and resolves
   WORKER_SCRIPT_PATH via __dirname so the daemon can be auto-started
   for MCP-only clients without dragging in the entire worker bundle.

4. resolveWorkerRuntimePath() now searches for Bun on every platform
   (not just Windows). worker-service.cjs requires Bun at runtime, so
   when the spawner is invoked from a Node process the Unix branch can
   no longer fall through to process.execPath (= node).

5. spawnDaemon's Unix branch now calls resolveWorkerRuntimePath() instead
   of hardcoding process.execPath, fixing the same Node-spawning-Node bug
   for the actual subprocess launch on Linux/macOS.

After:
- mcp-server.cjs is 384KB again with zero `bun:sqlite` references
- node mcp-server.cjs initializes and serves tools/list + tools/call
  (verified via JSON-RPC against the running worker)
- ProcessManager test suite updated for the new cross-platform Bun
  resolution behavior; full suite has the same pre-existing failures
  as main, no regressions

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(mcp): address PR #1645 review feedback (round 1)

Per Claude Code Review on PR #1645:

1. mcp-server.ts: log a warning when both __dirname and import.meta.url
   resolution fail. The cwd() fallback is essentially dead code for the
   CJS bundle but if it ever fires it gives the user a breadcrumb instead
   of a silently-wrong WORKER_SCRIPT_PATH.

2. mcp-server.ts: existsSync check on WORKER_SCRIPT_PATH at module load.
   Surfaces a clear "worker-service.cjs not found at expected path" log
   line for partial installs / dev environments instead of letting the
   failure surface as a generic spawnDaemon error later.

3. ProcessManager.ts: explanatory comment on the Windows `return 0`
   sentinel in spawnDaemon. Documents that PowerShell Start-Process
   doesn't return a PID and that callers MUST use `pid === undefined`
   for failure detection — never falsy checks like `if (!pid)`.

Items 4 (no direct unit tests for the worker-spawner Windows cooldown
helpers) and 5 (process-manager.test.ts uses real ~/.claude-mem path)
are deferred — the reviewer flagged the latter as out of scope, and
the former needs an injectable-I/O refactor that isn't appropriate
for a hotfix bugfix PR.

Verified: build clean, mcp-server.cjs still 384KB / zero bun:sqlite,
JSON-RPC tools/list still returns the 7-tool surface, ProcessManager
test suite still 43/43.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(spawner): mkdir CLAUDE_MEM_DATA_DIR before writing Windows cooldown marker

Per CodeRabbit on PR #1645: on a fresh user profile, the data dir may not
exist yet when markWorkerSpawnAttempted() runs. writeFileSync would throw
ENOENT, the catch would swallow it, and the marker would never be created
— defeating the popup-loop protection this helper exists to provide.

mkdirSync(dir, { recursive: true }) is a no-op when the directory already
exists, so it's safe to call on every spawn attempt.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* docs(spawner): add APPROVED OVERRIDE annotations for cooldown marker catches

Per CodeRabbit on PR #1645: silent catch blocks at spawn-cooldown sites
should carry the APPROVED OVERRIDE annotation that the rest of the
codebase uses (see ProcessManager.ts:689, BaseRouteHandler.ts:82,
ChromaSync.ts:288).

Both catches are intentional best-effort:
- markWorkerSpawnAttempted: if mkdir/writeFileSync fails, the worker
  spawn itself will almost certainly fail too. Surfacing that downstream
  is far more useful than a noisy log line about a lock file.
- clearWorkerSpawnAttempted: a stale marker is harmless. Worst case is
  one suppressed retry within the cooldown window, then self-heals.

No behaviour change. Resolves the second half of CodeRabbit's lines
38-65 comment on worker-spawner.ts.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(mcp): address PR #1645 review feedback (round 2)

Round 2 of Claude Code Review feedback on PR #1645:

Build guardrail (most important — protects the regression this PR fixes):

- scripts/build-hooks.js: post-build check that fails the build if
  mcp-server.cjs ever contains a `bun:sqlite` reference. This is the
  exact regression PR #1645 fixed; future contributors will get an
  immediate, actionable error if a transitive import re-introduces it.
  Verified the check trips when violated.

Code clarity:

- src/servers/mcp-server.ts: drop dead `_originalLog` capture — it was
  never restored. Less code is fewer bugs.

- src/servers/mcp-server.ts: elevate `cwd()` fallback log from WARN to
  ERROR. Per reviewer: a wrong WORKER_SCRIPT_PATH means worker auto-start
  silently fails, so the breadcrumb should be loud and searchable.

- src/services/worker-service.ts: extended doc comment on the
  `ensureWorkerStartedShared(port, __filename)` wrapper explaining why
  `__filename` is the correct script path here (CJS bundle = compiled
  worker-service.cjs) and why mcp-server.ts can't use the same trick.

- src/services/infrastructure/ProcessManager.ts: inline comment on the
  `env.BUN === 'bun'` bare-command guard explaining why it's reachable
  even though `isBunExecutablePath('bun')` is true (pathExists returns
  false for relative names, so the second branch is what fires).

Coverage:

- src/services/infrastructure/ProcessManager.ts: add `/usr/bin/bun` to
  the Linux candidate paths so apt-installed Bun on Debian/Ubuntu is
  found without falling through to the PATH lookup.

Out-of-scope items (deferred with rationale in PR replies):

- Unit tests for ensureWorkerStarted / Windows cooldown helpers — needs
  injectable-I/O refactor unsuitable for a hotfix.
- Sentinel object for Windows spawnDaemon `0` — broader API change.
- Windows Scoop install path — follow-up for a future PR.
- runOneTimeChromaMigration placement, aggressiveStartupCleanup,
  console.log redirect timing, platform timeout multiplier — all
  pre-existing and unrelated to this regression.

Verified: build clean, guardrail trips on simulated violation,
mcp-server.cjs still 0 bun:sqlite refs, ProcessManager tests 43/43.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(mcp): address PR #1645 review feedback (round 3)

Round 3 of Claude Code Review feedback on PR #1645:

ProcessManager.ts: improve actionability of "Bun not found" errors

Both Windows and Unix branches of spawnDaemon previously logged a vague
"Failed to locate Bun runtime" message when resolveWorkerRuntimePath()
returned null. Replaced with an actionable message that names the install
URL and explains *why* Bun is required (worker uses bun:sqlite). The
existing null-guard at the call sites already prevents passing null to
child_process.spawn — only the error text changed.

scripts/build-hooks.js: refine bun:sqlite guardrail to match actual
require() calls only

The previous coarse `includes('bun:sqlite')` check tripped on its own
improved error message, which legitimately mentions "bun:sqlite" by name.
Switched to a regex that matches `require("bun:sqlite")` /
`require('bun:sqlite')` (with optional whitespace, handles both quote
styles, handles minified output) so error messages and inline comments
can reference the module name without false positives. Verified the
regex still trips on real violations (both spaced and minified forms)
and correctly ignores string-literal mentions.

Other round-3 items (verified, not changed):

- TOOL_ENDPOINT_MAP: reviewer flagged as dead code, but it IS used at
  lines 250 and 263 by the search and timeline tool handlers. False
  positive — kept as-is.
- if (!pid) callsites: grepped src/, zero offenders. The Windows `0`
  PID sentinel contract is safe; only the in-line documentation comment
  in ProcessManager.ts mentions the anti-pattern.
- callWorkerAPIPost double-wrapping: pre-existing intentional behavior
  (only used by /api/observations/batch which returns raw data, not
  the MCP {content:[...]} shape). Unrelated to this regression.
- Snap path / startParentHeartbeat / main().catch / test for non-
  existent workerScriptPath / etc — pre-existing or out of scope for
  this hotfix, deferred per established disposition.

Verified: build clean, guardrail still trips on real violations,
mcp-server.cjs has 0 require("bun:sqlite") calls, JSON-RPC tools/list
returns the 7-tool surface, ProcessManager tests 43/43.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* test(spawnDaemon): contract test for Windows 0 PID success sentinel

Per CodeRabbit nitpick on PR #1645 commit 7a96b3b9: add a focused test
that documents the spawnDaemon return contract so any future contributor
who introduces `if (!pid)` against a spawnDaemon return value (or its
wrapper) sees a failing assertion explaining why the falsy check is
incorrect.

The test deliberately exercises the JS-level semantics rather than
mocking PowerShell — a true mocked Windows test would require
refactoring spawnDaemon to take an injectable execSync, which is a
larger change than this hotfix should carry. The contract assertions
here catch the same regression class (treating Windows success as
failure) without that refactor.

Verified: bun test tests/infrastructure/process-manager.test.ts now
passes 44/44 (was 43/43).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(mcp): address PR #1645 review feedback (round 4)

Round 4 of Claude Code Review feedback on PR #1645 (review of round-3
commit 193286f9):

tests/infrastructure/process-manager.test.ts: replace require('fs')
with the already-imported statSync. Reviewer correctly flagged that
the file uses ESM-style named imports everywhere else and the inline
require() calls would break under strict ESM. Two callsites updated
in the touchPidFile test.

src/services/infrastructure/ProcessManager.ts: hoist
resolveWorkerRuntimePath() and the `Bun runtime not found` error
handling out of both branches in spawnDaemon. Both Windows and Unix
branches need the same Bun lookup, and resolving once before the OS
branch split avoids a duplicate execSync('which bun')/where bun in the
no-well-known-path fallback. The error message is also DRY now —
single source of truth instead of two near-identical strings.

CodeRabbit confirmed in its previous reply that "All actionable items
across all four review rounds are fully resolved" — these two minor
items from claude-review of round 3 are the only remaining cleanup.

Verified: build clean, ProcessManager tests still 44/44.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(mcp): address PR #1645 review feedback (round 5)

Round 5 of Claude Code Review feedback on PR #1645:

src/services/worker-spawner.ts: drop `export` from internal helpers

`shouldSkipSpawnOnWindows`, `markWorkerSpawnAttempted`, and
`clearWorkerSpawnAttempted` were exported even though they were
private in worker-service.ts and nothing outside this module needs
them. Removing the `export` keyword keeps the public surface to just
`ensureWorkerStarted` and prevents future callers from bypassing the
spawn lifecycle.

scripts/build-hooks.js: broaden guardrail to all bun:* modules

Previously the regex only caught `require("bun:sqlite")`, but every
module in the `bun:` namespace (bun:ffi, bun:test, etc.) is Bun-only
and would crash mcp-server.cjs the same way under Node. Generalized
the regex to `require("bun:[a-z][a-z0-9_-]*")` so a transitive import
of any Bun-only module fails the build instead of shipping a broken
bundle. Verified the new regex still trips on bun:sqlite, bun:ffi,
bun:test, and correctly ignores string-literal mentions in error
messages.

src/servers/mcp-server.ts: attribute root cause when dirname resolution fails

Previously, if `__dirname`/`import.meta.url` resolution failed and we
fell back to `process.cwd()`, the user would see two warnings: an
error about the dirname fallback AND a separate warning about the
missing worker bundle. The second warning hides the root cause —
someone debugging would assume the install is broken when really it's
a dirname-resolution failure. Track the failure with a flag and emit
a single root-cause-attributing log line in the existence-check
branch instead. The dirname fallback paths are still functionally
unreachable in CJS deployment; this just makes the failure mode
unmistakable if it ever does fire.

Out of scope (consistent with prior rounds):
- darwin/linux split for non-Windows candidate paths (benign today)
- Integration test for non-existent workerScriptPath (test coverage
  gap deferred since rounds 1-2)
- Defer existsSync check to first ensureWorkerStarted call (current
  module-init check is the loud signal we want)

Already addressed in earlier rounds:
- resolveWorkerRuntimePath() called twice in spawnDaemon → hoisted in
  round 4 (b2c114b4)
- _originalLog dead code → removed in round 2 (7a96b3b9)

Verified: build clean, broadened guardrail trips on bun:sqlite,
bun:ffi, and bun:test (and ignores string literals), MCP server
serves the 7-tool surface, ProcessManager tests still 44/44.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(mcp): address PR #1645 review feedback (round 6)

Round 6 of Claude Code Review feedback on PR #1645:

src/services/worker-spawner.ts: validate workerScriptPath at entry

Add an empty-string + existsSync guard at the top of ensureWorkerStarted.
Without this, a partial install or upstream path-resolution regression
just surfaces as a low-signal child_process error from spawnDaemon. The
explicit log line at the entry point makes that class of bug much
easier to diagnose. The mcp-server.ts module-init existsSync check
already covers this for the MCP-server caller, but defending at the
spawner level reinforces the contract for any future caller.

src/services/worker-spawner.ts: document SettingsDefaultsManager
dependency boundary in the module header

The spawner imports from SettingsDefaultsManager, ProcessManager, and
HealthMonitor. None of those currently touch bun:sqlite, but if any
of them ever does, the spawner's SQLite-free contract silently breaks.
The build guardrail in build-hooks.js is the only thing that catches
it. Header comment now flags this so future contributors audit
transitive imports when adding helpers from the shared/infrastructure
layers.

src/services/infrastructure/ProcessManager.ts: add /snap/bin/bun

Ubuntu Snap install path. Now alongside the existing apt path
(/usr/bin/bun) and Homebrew/Linuxbrew paths. The PATH lookup catches
it as fallback, but listing it explicitly avoids paying for an
execSync('which bun') in the common case.

src/servers/mcp-server.ts: elevate missing-bundle log warn → error

A missing worker-service.cjs means EVERY MCP tool call that needs the
worker silently fails. That's a broken-install state, not a transient
condition — match the severity of the dirname-fallback branch above
(which is already ERROR).

Out of scope (consistent with prior rounds, reviewer agrees these are
appropriately deferred):
- Streaming bundle read in build-hooks.js (nit at current 384KB size)
- Unit tests for ensureWorkerStarted / cooldown helpers
- Integration test for non-existent workerScriptPath

Verified: build clean, broadened guardrail still trips on bun:* imports
and ignores string literals, MCP server serves the 7-tool surface,
ProcessManager tests still 44/44.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(mcp): defer WORKER_SCRIPT_PATH check to first call (round 7)

Round 7 of Claude Code Review feedback on PR #1645:

src/servers/mcp-server.ts: extract module-level existsSync check into
checkWorkerScriptPath() and call it lazily from ensureWorkerConnection()
instead of at module load.

The early-warning intent is preserved (the check still fires before any
actual spawn attempt), but tests/tools that import this module without
booting the MCP server no longer see noisy ERROR-level log lines for a
worker bundle they never intended to start. The check is cheap and
idempotent, so calling it on every auto-start attempt is fine.

The two failure-mode branches (dirname-resolution failure vs simple
missing-bundle) remain unchanged — the function body is identical to
the previous module-level if-block, just hoisted into a function and
called from ensureWorkerConnection().

False positive (no change needed):
- Reviewer flagged `mkdirSync` as a dead import in worker-spawner.ts,
  but it IS used at line 71 in markWorkerSpawnAttempted (the round-1
  ENOENT fix CodeRabbit explicitly asked for).

Out of scope:
- Volta path (~/.volta/bin/bun) — PATH fallback handles it; nit per
  reviewer
- worker-spawner.ts unit tests — needs injectable I/O, deferred
  consistently since round 1

Verified: build clean, tests 44/44, smoke test 7-tool surface.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(mcp): address PR #1645 review feedback (round 8)

Round 8 of Claude Code Review feedback on PR #1645:

tests/services/worker-spawner.test.ts: NEW FILE — unit tests for the
ensureWorkerStarted entry-point validation guards added in round 6.
Covers the empty-string and non-existent-path cases without requiring
the broader injectable-I/O refactor that the deeper spawn lifecycle
tests would need. 2 new passing tests.

src/services/infrastructure/ProcessManager.ts: memoize
resolveWorkerRuntimePath() for the no-options call site (which is what
spawnDaemon uses). Caches both successful resolutions and the
not-found result so repeated spawn attempts (crash loops, health
thrashing) don't repeatedly hit statSync on candidate paths. Tests
that pass options bypass the cache entirely so existing test cases
remain deterministic. Added resetWorkerRuntimePathCache() exported
for test isolation only.

src/servers/mcp-server.ts: rename checkWorkerScriptPath() →
warnIfWorkerScriptMissing(). Per reviewer: the old name implied a
boolean check but the function returns void and has side effects. New
name is more accurate.

DEFENDED (no change made):
- Reviewer asked to elevate process.cwd() fallback to a synchronous
  throw at module load. This conflicts with round 7 feedback which
  asked to defer the existsSync check to first call to avoid noisy
  test logs. The current lazy approach is the right compromise: it
  fires before any actual spawn attempt, attributes the root cause,
  and doesn't pollute test imports. Throwing at module load would
  crash before stdio is wired up, which is much harder to debug than
  the lazy log line.
- Reviewer asked to grep for `if (!pid)` callsites — already verified
  in round 3, zero offenders in src/.

Out of scope:
- Volta path (~/.volta/bin/bun) — PATH fallback handles it; reviewer
  marked as nit
- Deeper unit tests for ensureWorkerStarted spawn lifecycle (PID file
  cleanup, health checks, etc.) — needs injectable I/O, deferred
  consistently since round 1

Verified: build clean, ProcessManager tests still 44/44, new
worker-spawner tests 2/2, smoke test serves 7 tools.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(spawner): clear Windows cooldown marker on all healthy paths (round 9)

Round 9 of PR #1645 review feedback.

src/services/worker-spawner.ts: clear stale Windows cooldown marker
on every healthy-return path

Per CodeRabbit (genuine bug):

The .worker-start-attempted marker was previously only cleared after
a spawn initiated by ensureWorkerStarted itself succeeded. If a
previous auto-start failed, then the worker became healthy via
another session or a manual start, the early-return success branches
(existing live PID, fast-path health check, port-in-use waitForHealth)
would leave the stale marker behind. A subsequent genuine outage
inside the 2-minute cooldown window would then be incorrectly
suppressed on Windows.

Now calls clearWorkerSpawnAttempted() on all three healthy success
paths in addition to the existing post-spawn path. The function is
already a no-op on non-Windows, so the change is risk-free for Linux
and macOS callers.

src/servers/mcp-server.ts: more actionable error when auto-start fails

Per claude-review: when ensureWorkerStarted returns false (or throws),
the caller currently logs a generic "Worker auto-start failed" line.
Updated both error sites to explicitly call out which MCP tools will
fail (search/timeline/get_observations) and to point at earlier log
lines for the specific cause. Helps users distinguish "worker is just
not running" from "tools are broken".

DEFENDED (no change):
- Sentinel object for Windows spawnDaemon 0 PID — broader API change,
  out of scope, deferred consistently since round 1
- Spawner lifecycle tests beyond input validation — needs injectable
  I/O, deferred consistently
- Concurrent cooldown marker race on Windows — pre-existing,
  out of scope
- stripHardcodedDirname() regex fragility assertion — pre-existing,
  out of scope

Verified: build clean, ProcessManager tests 44/44, worker-spawner
tests 2/2, smoke test 7-tool surface.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(spawner): don't cache null Bun-not-found result (round 10)

Round 10 of PR #1645 review feedback.

src/services/infrastructure/ProcessManager.ts: only cache successful
resolveWorkerRuntimePath() results

Genuine bug from claude-review: the round-8 memoization cached BOTH
successful resolutions AND the not-found `null` result. If Bun isn't
on PATH at the moment the MCP server first tries to spawn the worker
— e.g., on a fresh install where the user installs Bun in another
terminal and retries — every subsequent ensureWorkerConnection call
would return the cached `null` and fail with a misleading "Bun not
found" error even though Bun is now available.

The fix is the one-line change the reviewer suggested: only cache
when `result !== null`. Crash loops still get the fast-path memoized
success; recovery from a fresh-install Bun install still works.

src/servers/mcp-server.ts: rename warnIfWorkerScriptMissing →
errorIfWorkerScriptMissing

Per claude-review: the function uses logger.error but the name says
"warn" — name/level mismatch. Renamed to match. The function still
serves the same purpose (defensive lazy check), just with an accurate
name.

DEFENDED (no change):
- Discriminated union for mcpServerDirResolutionFailed flag — current
  approach works, the noise is minimal, and the alternative would
  add type complexity for a path that's functionally unreachable in
  CJS deployment
- macOS /usr/local/bin/bun "missing" — already in the Linux/macOS
  candidate list at line 137 (false positive from reviewer)
- nix store path — out of scope, PATH fallback handles it
- Long build-hooks.js error message — verbosity is intentional, this
  message only fires on a real regression and the diagnostic value is
  worth the line wrap
- Spawner lifecycle test coverage gap — needs injectable I/O,
  deferred consistently

Verified: build clean, ProcessManager tests 44/44, worker-spawner
tests 2/2, smoke test 7-tool surface.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(mcp): bundle size budget guardrail (round 11)

Round 11 of PR #1645 review feedback.

scripts/build-hooks.js: secondary bundle-size budget guardrail

Per claude-review: the existing `require("bun:*")` regex catches the
specific regression class we already know about, but if esbuild ever
changes how it emits external module specifiers, the regex could
silently miss the regression. A bundle-size budget catches the
structural symptom (worker-service.ts dragged into the bundle blew
the size from ~358KB to ~1.96MB) regardless of how the imports look.

Set the ceiling at 600KB. Current size is ~384KB; the broken v12.0.0
bundle was ~1920KB. Plenty of headroom for legitimate growth without
incentivizing bundle bloat or false positives. Both guardrails fire
independently — one is regex-based, one is size-based — so a
regression has to defeat both to ship.

tests/services/worker-spawner.test.ts: comment about port irrelevance

Per claude-review: the hardcoded port values in the validation-guard
tests are arbitrary because the path validation short-circuits before
any network I/O. Added a comment explaining this so future readers
don't waste time wondering why specific ports were picked.

DEFENDED (no change):

- clearWorkerSpawnAttempted on the unhealthy-live-PID return path:
  reviewer asked to clear the marker here too, but the current
  behavior is correct. The marker tracks "recently attempted a spawn"
  and exists to prevent rapid PowerShell-popup loops. If a wedged
  process is currently using the port, the spawn isn't actually
  happening on this code path (the helper returns false without
  reaching the spawn step). When the wedged process eventually dies
  and a subsequent call hits the spawn path, the marker correctly
  suppresses repeated retry attempts within the 2-minute cooldown.
  Clearing the marker on the unhealthy-return path would defeat
  exactly the popup-loop protection the marker exists to provide.

- execSync in lookupBinaryInPath blocks event loop: pre-existing
  concern, not introduced by this PR. Reviewer notes "fires once,
  result cached". Not in scope for a hotfix.

- Tracking issue for spawner lifecycle test gap: out of scope for
  this PR; the gap is documented in the test file's header comment
  with a back-reference to PR #1645.

Verified: build clean, both guardrails functional (size budget is
under the new ceiling), ProcessManager tests 44/44, worker-spawner
tests 2/2, smoke test 7-tool surface.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(mcp): eliminate double error log when worker bundle is missing (round 12)

Round 12 of PR #1645 review feedback.

src/servers/mcp-server.ts: errorIfWorkerScriptMissing() now only logs
when the dirname-fallback attribution path is needed

Previously a missing worker-service.cjs would produce two ERROR log
lines on the same code path:
1. errorIfWorkerScriptMissing() in ensureWorkerConnection()
2. The existsSync guard inside ensureWorkerStarted()

The simple "missing bundle" case is fully covered by the spawner's
own existsSync guard. The mcp-server.ts function now ONLY logs when
mcpServerDirResolutionFailed is true — that's the mcp-server-specific
root-cause attribution that the spawner cannot provide on its own.

Net effect: same single error log per bug class, cleaner triage.

DEFENDED (no change):

- mkdirSync error propagation in markWorkerSpawnAttempted: reviewer
  worried that mkdirSync/writeFileSync exceptions could escape, but
  the entire body is already wrapped in try/catch with an APPROVED
  OVERRIDE annotation. False positive.
- clearWorkerSpawnAttempted on healthy paths: reviewer asked a
  clarifying question, not a change request. The behavior is
  intentional — the cooldown marker exists to prevent rapid
  PowerShell-popup loops from a series of failed spawns; a healthy
  worker means the marker has served its purpose and a future
  outage should NOT be suppressed. Will explain in PR reply.
- __filename ESM concern in worker-service.ts wrapper: already
  documented in round 4 with an extended comment about the CJS
  bundle context and why mcp-server.ts can't use the same trick.
- Spawn lifecycle integration tests: deferred consistently since
  round 1; gap is documented in worker-spawner.test.ts header.

Verified: build clean, ProcessManager tests 44/44, worker-spawner
tests 2/2, smoke test 7-tool surface.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* test(spawner): add bare-command BUN env override coverage

Final round of PR #1645 review feedback: while preparing to merge, I
noticed CodeRabbit's round-5 CHANGES_REQUESTED review on commit
3570d2f0 included an unaddressed nitpick — the env-driven bare-command
branch in resolveWorkerRuntimePath() (returning a bare 'bun' unchanged
when BUN or BUN_PATH is set that way) had no test coverage and could
regress without any failing assertion.

Added a focused test that exercises the env: { BUN: 'bun' } branch
specifically. 47/47 tests pass (was 46/46).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-07 18:08:36 -07:00
Alex Newman 25bb93a995 fix: address PR #1641 review comments (round 2)
- Remove duplicate TranscriptWatcher/config imports in worker-service.ts
- Use normalizePlatformSource in handleSessionInitByClaudeId for consistency
- Don't skip DB completion when session not in memory (completeByClaudeId)
- Add try-catch around fetch in useContextPreview refresh callback
- Deduplicate store.getAllProjects() call in DataRoutes
- Fix malformed comment separators in migration runner
- Fix missing closing brace and JSDoc opener (merge artifact) in migration runner

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-07 13:22:58 -07:00
Alex Newman cbb68ad9e1 fix: worker startup crash and missing observation columns
Two bugs fixed:

1. SessionCompletionHandler called dbManager.getSessionStore() during
   WorkerService construction, before DB initialization. Changed to
   accept DatabaseManager and defer the call to runtime.

2. migration009 (generated_by_model, relevance_count columns) only ran
   via the deprecated MigrationRunner path, never through SessionStore's
   migration chain. Added addObservationModelColumns() to SessionStore
   constructor. Checks column existence directly since schema_versions
   may have been marked applied without the ALTER TABLE succeeding.

Also removed duplicate transcriptWatcher declaration and shutdown block
(merge artifact).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-07 12:20:10 -07:00
Alex Newman 6250a194dd Merge branch 'pr-1472' into integration/validation-batch
# Conflicts:
#	plugin/scripts/context-generator.cjs
#	plugin/scripts/mcp-server.cjs
#	plugin/scripts/worker-service.cjs
#	plugin/ui/viewer-bundle.js
#	src/cli/handlers/context.ts
#	src/services/sqlite/SessionStore.ts
#	src/services/sqlite/migrations/runner.ts
#	src/services/worker-service.ts
#	src/shared/SettingsDefaultsManager.ts
2026-04-06 14:23:18 -07:00
Alex Newman d570909bf1 Merge branch 'pr-1491' into integration/validation-batch
# Conflicts:
#	plugin/scripts/mcp-server.cjs
#	plugin/scripts/worker-service.cjs
#	src/shared/hook-constants.ts
2026-04-06 14:20:05 -07:00
Henry Gimenez da Costa 753837bff3 fix(windows): isMainModule CJS branch fails on Bun — add CLAUDE_MEM_MANAGED fallback
On Bun/Windows, `require.main !== module` in CJS mode causes the worker
to exit silently with code 0. The wrapper already sets CLAUDE_MEM_MANAGED=true
when spawning the inner worker, so checking this env var is a safe fallback
that doesn't affect standalone execution.

Ref #1450 (incomplete fix in PR #1518 — ESM path fixed but CJS branch not).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-05 02:45:57 -03:00
Alex Newman 2495f98496 refactor: consolidate MCP factory, add non-TTY support, auto-detect transcript watchers
- Phase 1: Replace 5 duplicate MCP installers with config-driven factory, extract
  shared context-injection and json-utils utilities, fix process.execPath usage
- Phase 2: Add non-TTY fallback for @clack/prompts to prevent ENOENT in CI/Docker
- Phase 3: Wire GeminiCliHooksInstaller through hook command framework with adapter
- Phase 4: Auto-start transcript watchers on worker boot when config exists

Net -107 lines via DRY consolidation of duplicated installer logic.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-04 00:35:55 -07:00
Oracle Public Cloud User 4589b34eab fix: decouple mcp health from loopback self-check 2026-03-30 16:37:56 +00:00
Ryan Malia b0f1a458cf fix: log warning when readiness times out on reused-worker path (#1491)
Mirror the fresh-spawn path's timeout logging for debugging parity.
CodeRabbit nitpick on PR #1491.

Co-Authored-By: CC <noreply@anthropic.com>
2026-03-30 03:47:08 -07:00
Ryan Malia 83f61177c7 fix: address CodeRabbit review feedback on PR #1491
- Update POST_SPAWN_WAIT test assertion from 5000 to 15000 to match
  the constant change in hook-constants.ts
- Remove redundant readPidFile() from aggressiveStartupCleanup() —
  start() writes the new PID before this runs, so it always returns
  process.pid (already protected)
- Add waitForReadiness() to the reused-worker path in
  ensureWorkerStarted() to prevent concurrent hooks from racing
  past a cold-starting worker's initialization guard

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-03-30 03:43:36 -07:00
Ryan Malia 88b47f9e9c fix: prevent worker daemon from being killed by its own hooks (#1490)
Three independent fixes for worker daemon instability:

1. Remove version mismatch auto-restart from ensureWorkerStarted() (#1435).
   The marketplace bundle ships with __DEFAULT_PACKAGE_VERSION__ unbaked,
   causing BUILT_IN_VERSION to fall back to "development". This creates a
   100% reproducible mismatch on every hook call, killing a healthy worker
   and often failing to restart. Same pattern across #566, #665, #667,
   #669, #689, #1124, #1145 (8+ releases).

2. Add process.ppid and PID-file PID to aggressiveStartupCleanup()
   exclusions (#1426). Without this, a newly spawned daemon SIGKILLs
   the hook process that spawned it and any already-running worker
   the PID file points to.

3. Increase POST_SPAWN_WAIT from 5s to 15s (#1423). The 5s timeout was
   sized for Linux (<1s startup) but macOS ARM64 cold starts take 6-8s
   with Chroma enabled.
2026-03-30 03:43:36 -07:00
Alex Newman 07ab7000a8 fix: patch 7 critical bugs affecting all non-dev-machine users and Windows
1. Fix esbuild inlining build-machine __dirname as string literal — use
   CJS-compatible runtime banner with require("node:url").fileURLToPath
   across worker-service, mcp-server, and context-generator builds.

2. Fix isMainModule check missing .cjs extension and Windows backslash
   path normalization.

3. Wrap extractLastMessage in try-catch to prevent infinite Stop hook
   feedback loop on malformed transcripts (exit 0 instead of exit 2).

4. Replace heavy SessionEnd hook (Node→Bun→1.7MB CJS→HTTP) with
   lightweight inline node -e one-liner (~200ms vs >1s).

5. Add 7 Gemini/OpenRouter error patterns to unrecoverablePatterns
   circuit breaker to prevent 77K+ retry loops on expired API keys.

6. Preserve CLAUDE_CODE_OAUTH_TOKEN and CLAUDE_CODE_GIT_BASH_PATH in
   sanitizeEnv instead of stripping them with the CLAUDE_CODE_ prefix.

7. Use PowerShell -EncodedCommand for spawnDaemon to fix path quoting
   when Windows usernames contain spaces.

Closes #1515, #1495, #1475, #1465, #1500, #1513, #1512, #1450, #1460,
#1486, #1449, #1481, #1451, #1480, #1453, #1445

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-28 15:20:29 -07:00
huakson 2b60dd2932 feat: isolate Claude and Codex session sources
Persist platform_source across session creation, transcript ingestion, API query paths, and viewer state so Claude and Codex data can coexist without bleeding into each other.

- add platform-source normalization helpers and persist platform_source in sdk_sessions via migration 24 with backfill and indexing
- thread platformSource through CLI hooks, transcript processing, context generation, pagination, search routes, SSE payloads, and session management
- expose source-aware project catalogs, viewer tabs, context preview selectors, and source badges for observations, prompts, and summaries
- start the transcript watcher from the worker for transcript-based clients and preserve platform source during Codex ingestion
- auto-start the worker from the MCP server for MCP-only clients and tighten stdio-driven cleanup during shutdown
- keep createSDKSession backward compatible with existing custom-title callers while allowing explicit platform source forwarding
2026-03-24 08:46:18 -03:00
Alex Newman 4d7bec4d05 fix: stop spinner from spinning forever (#1440)
* fix: stop spinner from spinning forever due to orphaned DB messages

The activity spinner never stopped because isAnySessionProcessing() queried
ALL pending/processing messages in the database, including orphaned messages
from dead sessions that no generator would ever process.

Root cause: isAnySessionProcessing() used hasAnyPendingWork() which is a
global DB scan. Changed it to use getTotalQueueDepth() which only checks
sessions in the active in-memory Map.

Additional fixes:
- Add terminateSession() to enforce restart-or-terminate invariant
- Fix 3 zombie paths in .finally() handler that left sessions alive
- Clean up idle sessions from memory on successful completion
- Remove redundant bare isProcessing:true broadcast
- Replace inline require() with proper accessor
- Add 8 regression tests for session termination invariant

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: address review findings — idle-timeout race, double broadcast, query amplification

- Move pendingCount check before idle-timeout termination to prevent
  abandoning fresh messages that arrive between idle abort and .finally()
- Move broadcastProcessingStatus() inside restart branch only — the else
  branch already broadcasts via removeSessionImmediate callback
- Compute queueDepth once in broadcastProcessingStatus() and derive
  isProcessing from it, eliminating redundant double iteration

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-21 14:13:10 -07:00
Alex Newman 80a8c90a1a feat: add embedded Process Supervisor for unified process lifecycle (#1370)
* feat: add embedded Process Supervisor for unified process lifecycle management

Consolidates scattered process management (ProcessManager, GracefulShutdown,
HealthMonitor, ProcessRegistry) into a unified src/supervisor/ module.

New: ProcessRegistry with JSON persistence, env sanitizer (strips CLAUDECODE_*
vars), graceful shutdown cascade (SIGTERM → 5s wait → SIGKILL with tree-kill
on Windows), PID file liveness validation, and singleton Supervisor API.

Fixes #1352 (worker inherits CLAUDECODE env causing nested sessions)
Fixes #1356 (zombie TCP socket after Windows reboot)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* feat: add session-scoped process reaping to supervisor

Adds reapSession(sessionId) to ProcessRegistry for killing session-tagged
processes on session end. SessionManager.deleteSession() now triggers reaping.
Tightens orphan reaper interval from 60s to 30s.

Fixes #1351 (MCP server processes leak on session end)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* feat: add Unix domain socket support for worker communication

Introduces socket-manager.ts for UDS-based worker communication, eliminating
port 37777 collisions between concurrent sessions. Worker listens on
~/.claude-mem/sockets/worker.sock by default with TCP fallback.

All hook handlers, MCP server, health checks, and admin commands updated to
use socket-aware workerHttpRequest(). Backwards compatible — settings can
force TCP mode via CLAUDE_MEM_WORKER_TRANSPORT=tcp.

Fixes #1346 (port 37777 collision across concurrent sessions)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: remove in-process worker fallback from hook command

Removes the fallback path where hook scripts started WorkerService in-process,
making the worker a grandchild of Claude Code (killed by sandbox). Hooks now
always delegate to ensureWorkerStarted() which spawns a fully detached daemon.

Fixes #1249 (grandchild process killed by sandbox)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* feat: add health checker and /api/admin/doctor endpoint

Adds 30-second periodic health sweep that prunes dead processes from the
supervisor registry and cleans stale socket files. Adds /api/admin/doctor
endpoint exposing supervisor state, process liveness, and environment health.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* test: add comprehensive supervisor test suite

64 tests covering all supervisor modules: process registry (18 tests),
env sanitizer (8), shutdown cascade (10), socket manager (15), health
checker (5), and supervisor API (6). Includes persistence, isolation,
edge cases, and cross-module integration scenarios.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: revert Unix domain socket transport, restore TCP on port 37777

The socket-manager introduced UDS as default transport, but this broke
the HTTP server's TCP accessibility (viewer UI, curl, external monitoring).
Since there's only ever one worker process handling all sessions, the
port collision rationale for UDS doesn't apply. Reverts to TCP-only,
removing ~900 lines of unnecessary complexity.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* chore: remove dead code found in pre-landing review

Remove unused `acceptingSpawns` field from Supervisor class (written but
never read — assertCanSpawn uses stopPromise instead) and unused
`buildWorkerUrl` import from context handler.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* updated gitignore

* fix: address PR review feedback - downgrade HTTP logging, clean up gitignore, harden supervisor

- Downgrade request/response HTTP logging from info to debug to reduce noise
- Remove unused getWorkerPort imports, use buildWorkerUrl helper
- Export ENV_PREFIXES/ENV_EXACT_MATCHES from env-sanitizer, reuse in Server.ts
- Fix isPidAlive(0) returning true (should be false)
- Add shutdownInitiated flag to prevent signal handler race condition
- Make validateWorkerPidFile testable with pidFilePath option
- Remove unused dataDir from ShutdownCascadeOptions
- Upgrade reapSession log from debug to warn
- Rename zombiePidFiles to deadProcessPids (returns actual PIDs)
- Clean up gitignore: remove duplicate datasets/, stale ~*/ and http*/ patterns
- Fix tests to use temp directories instead of relying on real PID file

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-16 14:49:23 -07:00
laihenyi 626654f816 fix: prevent infinite restart loop on FOREIGN KEY constraint errors (#1334)
The pending-work-restart logic had no retry limit, causing infinite loops
when sessions encountered FOREIGN KEY constraint failures. This led to
2000+ error log entries per minute and eventual worker crash via SIGTERM.

Two fixes:
1. Add 'FOREIGN KEY constraint failed' to unrecoverable error patterns
   so it short-circuits immediately instead of falling through to restart
2. Add MAX_PENDING_RESTARTS (3) limit to pending-work-restart path as a
   safety net for any future unhandled persistent errors

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
2026-03-12 20:03:48 -07:00
Nir Alfasi 38d9ac7adb fix: prevent zombie subprocess accumulation by only trusting exitCode (#1226) (#1325)
proc.killed only means Node sent a signal — the process can still be alive.
This caused premature pool slot release, allowing unbounded process spawning.

- ensureProcessExit: remove proc.killed from early-exit checks, only trust exitCode
- Fix 3 call-site guards that skipped cleanup for signaled-but-alive processes
- Add TOTAL_PROCESS_HARD_CAP=10 safety net in waitForSlot()
- After SIGKILL, wait up to 1s via exit event instead of blind 200ms sleep
- Reduce reaper interval from 5min to 1min, idle threshold from 2min to 1min

Closes #1226

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
2026-03-12 19:59:42 -07:00
Alex Newman ad3d236cec fix: resolve hook crashes and CLAUDE_PLUGIN_ROOT fallback (#1215, #1220) (#1229)
* fix: resolve PostToolUse hook crashes and 5s latency (#1220)

Three compounding bugs caused hook failures:

1. Missing break statements in worker-service.ts switch — if async
   code threw before process.exit(), execution fell through to
   subsequent cases. Added break to all 7 cases missing them.

2. Unhandled promise rejection on main() — added .catch() that logs
   the error and exits 0 (per project exit code strategy: don't block
   Claude Code or leave Windows Terminal tabs open).

3. Redundant start commands in hooks.json — PostToolUse,
   UserPromptSubmit, and Stop groups each had a standalone start
   command that was redundant (the hook case already calls
   ensureWorkerStarted internally). The redundant start also caused
   5s latency via bun-runner.js collectStdin() timeout since Claude
   Code never closes stdin.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix: add CLAUDE_PLUGIN_ROOT fallback for Stop hooks (#1215)

Upstream Claude Code bug (anthropics/claude-code#24529) leaves
CLAUDE_PLUGIN_ROOT unset for Stop hooks on macOS and ALL hooks
on Linux. Two-layer defense:

1. Shell-level: hooks.json commands now use inline fallback
   _R="${CLAUDE_PLUGIN_ROOT}"; [ -z "$_R" ] && _R="$HOME/...";
   falling back to the known marketplace install path.

2. Script-level: bun-runner.js self-resolves plugin root from
   its own filesystem location via import.meta.url, and fixes
   broken /scripts/... paths that result from empty expansion.

Added test to verify all hook commands include the fallback path.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
2026-02-24 19:31:26 -05:00
Alex Newman c6f932988a Fix 30+ root-cause bugs across 10 triage phases (#1214)
* MAESTRO: fix ChromaDB core issues — Python pinning, Windows paths, disable toggle, metadata sanitization, transport errors

- Add --python version pinning to uvx args in both local and remote mode (fixes #1196, #1206, #1208)
- Convert backslash paths to forward slashes for --data-dir on Windows (fixes #1199)
- Add CLAUDE_MEM_CHROMA_ENABLED setting for SQLite-only fallback mode (fixes #707)
- Sanitize metadata in addDocuments() to filter null/undefined/empty values (fixes #1183, #1188)
- Wrap callTool() in try/catch for transport errors with auto-reconnect (fixes #1162)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* MAESTRO: fix data integrity — content-hash deduplication, project name collision, empty project guard, stuck isProcessing

- Add SHA-256 content-hash deduplication to observations INSERT (store.ts, transactions.ts, SessionStore.ts)
- Add content_hash column via migration 22 with backfill and index
- Fix project name collision: getCurrentProjectName() now returns parent/basename
- Guard against empty project string with cwd-derived fallback
- Fix stuck isProcessing: hasAnyPendingWork() resets processing messages older than 5 minutes
- Add 12 new tests covering all four fixes

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* MAESTRO: fix hook lifecycle — stderr suppression, output isolation, conversation pollution prevention

- Suppress process.stderr.write in hookCommand() to prevent Claude Code showing diagnostic
  output as error UI (#1181). Restores stderr in finally block for worker-continues case.
- Convert console.error() to logger.warn()/error() in hook-command.ts and handlers/index.ts
  so all diagnostics route to log file instead of stderr.
- Verified all 7 handlers return suppressOutput: true (prevents conversation pollution #598, #784).
- Verified session-complete is a recognized event type (fixes #984).
- Verified unknown event types return no-op handler with exit 0 (graceful degradation).
- Added 10 new tests in tests/hook-lifecycle.test.ts covering event dispatch, adapter defaults,
  stderr suppression, and standard response constants.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* MAESTRO: fix worker lifecycle — restart loop coordination, stale transport retry, ENOENT shutdown race

- Add PID file mtime guard to prevent concurrent restart storms (#1145):
  isPidFileRecent() + touchPidFile() coordinate across sessions
- Add transparent retry in ChromaMcpManager.callTool() on transport
  error — reconnects and retries once instead of failing (#1131)
- Wrap getInstalledPluginVersion() with ENOENT/EBUSY handling (#1042)
- Verified ChromaMcpManager.stop() already called on all shutdown paths

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* MAESTRO: fix Windows platform support — uvx.cmd spawn, PowerShell $_ elimination, windowsHide, FTS5 fallback

- Route uvx spawn through cmd.exe /c on Windows since MCP SDK lacks shell:true (#1190, #1192, #1199)
- Replace all PowerShell Where-Object {$_} pipelines with WQL -Filter server-side filtering (#1024, #1062)
- Add windowsHide: true to all exec/spawn calls missing it to prevent console popups (#1048)
- Add FTS5 runtime probe with graceful fallback when unavailable on Windows (#791)
- Guard FTS5 table creation in migrations, SessionSearch, and SessionStore with try/catch

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* MAESTRO: fix skills/ distribution — build-time verification and regression tests (#1187)

Add post-build verification in build-hooks.js that fails if critical
distribution files (skills, hooks, plugin manifest) are missing. Add
10 regression tests covering skill file presence, YAML frontmatter,
hooks.json integrity, and package.json files field.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* MAESTRO: fix MigrationRunner schema initialization (#979) — version conflict between parallel migration systems

Root cause: old DatabaseManager migrations 1-7 shared schema_versions table with
MigrationRunner's 4-22, causing version number collisions (5=drop tables vs add column,
6=FTS5 vs prompt tracking, 7=discovery_tokens vs remove UNIQUE).  initializeSchema()
was gated behind maxApplied===0, so core tables were never created when old versions
were present.

Fixes:
- initializeSchema() always creates core tables via CREATE TABLE IF NOT EXISTS
- Migrations 5-7 check actual DB state (columns/constraints) not just version tracking
- Crash-safe temp table rebuilds (DROP IF EXISTS _new before CREATE)
- Added missing migration 21 (ON UPDATE CASCADE) to MigrationRunner
- Added ON UPDATE CASCADE to FK definitions in initializeSchema()
- All changes applied to both runner.ts and SessionStore.ts

Tests: 13 new tests in migration-runner.test.ts covering fresh DB, idempotency,
version conflicts, crash recovery, FK constraints, and data integrity.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* MAESTRO: fix 21 test failures — stale mocks, outdated assertions, missing OpenClaw guards

Server tests (12): Added missing workerPath and getAiStatus to ServerOptions
mocks after interface expansion. ChromaSync tests (3): Updated to verify
transport cleanup in ChromaMcpManager after architecture refactor. OpenClaw (2):
Added memory_ tool skipping and response truncation to prevent recursive loops
and oversized payloads. MarkdownFormatter (2): Updated assertions to match
current output. SettingsDefaultsManager (1): Used correct default key for
getBool test. Logger standards (1): Excluded CLI transcript command from
background service check.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* MAESTRO: fix Codex CLI compatibility (#744) — session_id fallbacks, unknown platform tolerance, undefined guard

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* MAESTRO: fix Cursor IDE integration (#838, #1049) — adapter field fallbacks, tolerant session-init validation

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* MAESTRO: fix /api/logs OOM (#1203) — tail-read replaces full-file readFileSync

Replace readFileSync (loads entire file into memory) with readLastLines()
that reads only from the end of the file in expanding chunks (64KB → 10MB cap).
Prevents OOM on large log files while preserving the same API response shape.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* MAESTRO: fix Settings CORS error (#1029) — explicit methods and allowedHeaders in CORS config

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* MAESTRO: add session custom_title for agent attribution (#1213) — migration 23, endpoint + store support

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* MAESTRO: prevent CLAUDE.md/AGENTS.md writes inside .git/ directories (#1165)

Add .git path guard to all 4 write sites to prevent ref corruption when
paths resolve inside .git internals.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* MAESTRO: fix plugin disabled state not respected (#781) — early exit check in all hook entry points

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* MAESTRO: fix UserPromptSubmit context re-injection on every turn (#1079) — contextInjected session flag

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* MAESTRO: fix stale AbortController queue stall (#1099) — lastGeneratorActivity tracking + 30s timeout

Three-layer fix:
1. Added lastGeneratorActivity timestamp to ActiveSession, updated by
   processAgentResponse (all agents), getMessageIterator (queue yields),
   and startGeneratorWithProvider (generator launch)
2. Added stale generator detection in ensureGeneratorRunning — if no
   activity for >30s, aborts stale controller, resets state, restarts
3. Added AbortSignal.timeout(30000) in deleteSession to prevent
   indefinite hang when awaiting a stuck generator promise

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
2026-02-23 19:34:35 -05:00
Alex Newman 7966c6cba9 fix: rename save_memory and fix MCP search instructions + startup hook (#1210)
* fix: rename save_memory to save_observation and fix MCP search instructions

Stop the primary agent from proactively saving memories by renaming
save_memory to save_observation with a neutral description. Remove
"Saving Memories" section from SKILL.md. Update context formatters
and output styles to reference the mem-search skill instead of raw
MCP tool names.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix: split SessionStart hooks so smart-install failure doesn't block worker start

smart-install.js and worker-start were in the same hook group, so if
smart-install exited non-zero the worker never started. Split into
separate hook groups so they run independently.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix: worker startup waits for readiness before hooks fire

Move initializationCompleteFlag to set after DB/search init (not MCP),
add waitForReadiness() polling /api/readiness, and extract shared
pollEndpointUntilOk helper to DRY up health/readiness checks.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
2026-02-23 03:30:31 -05:00
Alex Newman e788fd3676 fix: prevent duplicate worker daemons and zombie processes (#1178)
* fix: prevent duplicate worker daemons and zombie processes

Three root causes of chroma-mcp timeouts:

1. HTTP shutdown (POST /api/admin/shutdown) closed resources but never
   called process.exit(). Zombie workers stayed alive, background tasks
   reconnected to chroma-mcp, spawning duplicate subprocesses that all
   contended for the same persistent data directory.

2. No guard against concurrent daemon startup. When hooks fired
   simultaneously, multiple daemons started before either wrote a PID
   file. The loser got EADDRINUSE but stayed alive because signal
   handlers registered in the constructor prevented exit.

3. Corrupt 147GB HNSW index file caused all chroma queries to timeout
   (MCP error -32001). Data fix: deleted corrupt collection, backfill
   rebuilds from SQLite.

Code fixes:
- Add PID-based guard in daemon startup: exit if PID file process alive
- Add port-based guard in daemon startup: exit if port already bound
  (runs before WorkerService constructor registers keepalive handlers)
- Add process.exit(0) after HTTP shutdown/restart completes

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix: aggressive startup cleanup and one-time chroma wipe for upgrade

Kill orphaned worker-service.cjs and chroma-mcp processes immediately
at startup (no age gate) while keeping 30-min threshold for mcp-server.
Wipe corrupt chroma data once on upgrade from pre-v10.3 versions —
backfill rebuilds from SQLite automatically.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix: wrap shutdown handlers in try/finally to guarantee process.exit

If onShutdown() or onRestart() threw, process.exit(0) was never reached,
leaving the daemon alive as a zombie. Also removed redundant require('fs')
calls in process-manager tests where ESM imports already existed.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
2026-02-18 20:10:28 -05:00
Alex Newman 40daf8f3fa feat: replace WASM embeddings with persistent chroma-mcp MCP connection (#1176)
* feat: replace WASM embeddings with persistent chroma-mcp MCP connection

Replace ChromaServerManager (npx chroma run + chromadb npm + ONNX/WASM)
with ChromaMcpManager, a singleton stdio MCP client that communicates with
chroma-mcp via uvx. This eliminates native binary issues, segfaults, and
WASM embedding failures that plagued cross-platform installs.

Key changes:
- Add ChromaMcpManager: singleton MCP client with lazy connect, auto-reconnect,
  connection lock, and Zscaler SSL cert support
- Rewrite ChromaSync to use MCP tool calls instead of chromadb npm client
- Handle chroma-mcp's non-JSON responses (plain text success/error messages)
- Treat "collection already exists" as idempotent success
- Wire ChromaMcpManager into GracefulShutdown for clean subprocess teardown
- Delete ChromaServerManager (no longer needed)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix: address PR review — connection guard leak, timer leak, async reset

- Clear connecting guard in finally block to prevent permanent reconnection block
- Clear timeout after successful connection to prevent timer leak
- Make reset() async to await stop() before nullifying instance
- Delete obsolete chroma-server-manager test (imports deleted class)
- Update graceful-shutdown test to use chromaMcpManager property name

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix: prevent chroma-mcp spawn storm — zombie cleanup, stale onclose guard, reconnect backoff

Three bugs caused chroma-mcp processes to accumulate (92+ observed):

1. Zombie on timeout: failed connections left subprocess alive because
   only the timer was cleared, not the transport. Now catch block
   explicitly closes transport+client before rethrowing.

2. Stale onclose race: old transport's onclose handler captured `this`
   and overwrote the current connection reference after reconnect,
   orphaning the new subprocess. Now guarded with reference check.

3. No backoff: every failure triggered immediate reconnect. With
   backfill doing hundreds of MCP calls, this created rapid-fire
   spawning. Added 10s backoff on both connection failure and
   unexpected process death.

Also includes ChromaSync fixes from PR review:
- queryChroma deduplication now preserves index-aligned arrays
- SQL injection guard on backfill ID exclusion lists

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
2026-02-18 18:32:38 -05:00
Alex Newman 5d79bb7a7a fix: prevent zombie process accumulation by verifying subprocess exit (#1168) (#1175)
Two changes fix the observer process resource leak:

1. Add ensureProcessExit to generator finally blocks in SessionRoutes and
   worker-service, matching the pattern already working in SDKAgent.

2. Add stale session reaper (every 2m) that removes sessions with no active
   generator and no pending work after 15m idle. This unblocks the orphan
   reaper which previously skipped processes for "active" sessions.

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
2026-02-18 16:33:23 -05:00
Alex Newman b88251bc8b fix: self-healing claimNextMessage prevents stuck processing messages (#1159)
* fix: self-healing claimNextMessage prevents stuck processing messages

claimAndDelete → claimNextMessage with atomic self-healing: resets stale
processing messages (>60s) back to pending before claiming. Eliminates
stuck messages from generator crashes without external timers. Removes
redundant idle-timeout reset in worker-service.ts. Adds QUEUE to logger
Component type.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix: update stale comments in SessionQueueProcessor to reflect claim-confirm pattern

Comments still referenced the old claim-and-delete pattern after the
claimNextMessage rename. Updated to accurately describe the current
lifecycle where messages are marked as processing and stay in DB until
confirmProcessed() is called.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix: move Date.now() inside transaction and extract stale threshold constant

- Move Date.now() inside claimNextMessage transaction closure so timestamp
  is fresh if WAL contention causes retry
- Extract STALE_PROCESSING_THRESHOLD_MS to module-level constant
- Add comment clarifying strict < boundary semantics

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
2026-02-17 23:15:46 -05:00
Alex Newman ca8421611c fix: backfill Chroma vector DB for all projects on startup (#1154)
* fix: backfill all Chroma projects on worker startup

ChromaSync.ensureBackfilled() existed but was never called. After
v10.2.2's bun cache clear destroyed the ONNX model cache, Chroma only
had ~2 days of embeddings while SQLite had 49k+ observations.

- Add static backfillAllProjects() to ChromaSync — iterates all projects
  in SQLite, creates temporary ChromaSync per project, runs smart diff
- Call backfillAllProjects() fire-and-forget on worker startup
- Add 'CHROMA_SYNC' to logger Component type (pre-existing gap)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix: sanitize project names for Chroma collection naming

Replace characters outside [a-zA-Z0-9._-] with underscores so projects
like "YC Stuff" map to collection "cm__YC_Stuff" instead of failing
Chroma's collection name validation.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix: route backfill to shared cm__claude-mem collection, harden sanitization

- Use single ChromaSync('claude-mem') in backfillAllProjects() instead of
  per-project instances, matching how DatabaseManager and SearchManager
  operate — fixes critical bug where backfilled data landed in orphaned
  collections that no search path reads from
- Strip trailing non-alphanumeric chars from sanitized collection names
  to satisfy Chroma's end-character constraint
- Guard backfill behind Chroma server readiness to avoid N spurious error
  logs when Chroma failed to start
- Use CHROMA_SYNC log component consistently for backfill messages

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* refactor: pass project as parameter to ensureBackfilled instead of mutating instance state

Eliminates shared mutable state in backfillAllProjects() loop. Project
scoping is now passed explicitly via parameter to both ensureBackfilled()
and getExistingChromaIds(), keeping a single Chroma connection while
avoiding fragile instance property mutation across iterations.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
2026-02-17 22:47:46 -05:00
Alex Newman f24251118e fix: bun install, node-addon-api for sharp, consolidate PendingMessageStore (#1140)
* fix: use bun install in sync, add node-addon-api for sharp, consolidate PendingMessageStore

- Switch sync-marketplace from npm to bun install
- Add node-addon-api as dev dep so sharp builds under bun
- Consolidate duplicate PendingMessageStore instantiation in worker-service finally block

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* build assets

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
2026-02-16 18:05:42 -05:00
Alex Newman d2e926fbf7 fix: post-merge breakage (Gemini, idle timeout, sharp cache) (#1138)
* fix: add gemini-3-flash to validModels array

The model was defined in the type union and RPM limits but missing from
the runtime validModels array, causing silent fallback to gemini-2.5-flash.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix: skip processing when Gemini returns empty observation response

Empty responses were silently consuming messages from the queue via
processAgentResponse. Now skips processing on empty content, leaving
the message in processing status for stale recovery.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix: prevent idle timeout from triggering infinite restart loop

When a session hits the 3-minute idle timeout, the finally block was
seeing stale processing messages and restarting the generator endlessly.
Now tracks idle timeout as a distinct exit reason via session flag,
resets stale messages, and skips restart.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix: clear stale Bun native module cache on update

Bun's global cache retains sharp/libvips native binaries with broken
dylib references after version upgrades. Clear ~/.bun/install/cache/@img/
before install in both the end-user (smart-install) and dev (sync-marketplace)
paths to prevent ERR_DLOPEN_FAILED errors in Chroma sync.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix: address PR review feedback (empty summary response, session-scoped reset, shell injection)

- Apply same empty-response guard to summary path as observation path in GeminiAgent
- Add optional sessionDbId param to resetStaleProcessingMessages for session-scoped resets
- Use JSON.stringify for gitignore pattern escaping, filter negation patterns

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
2026-02-16 17:46:30 -05:00
Alex Newman c27314f896 fix: address PR review comments for chroma server lifecycle 2026-02-13 23:39:30 -05:00
Alex Newman ed313db742 Merge main into feat/chroma-http-server
Resolve conflicts between Chroma HTTP server PR and main branch changes
(folder CLAUDE.md, exclusion settings, Zscaler SSL, transport cleanup).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-02-13 21:02:54 -05:00
Alex Newman 5de728612e chore: bump version to 10.0.6
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-02-13 00:02:37 -05:00
Alex Newman f05f9ca735 Merge remote-tracking branch 'origin/main' into openclaw-installer
# Conflicts:
#	plugin/scripts/mcp-server.cjs
#	plugin/scripts/worker-service.cjs
2026-02-12 22:04:03 -05:00
Alex Newman 05e904e613 feat: enhance /api/health with version, uptime, workerPath, and AI status
Replace hardcoded TEST-008 build ID with real package version. Add worker
filesystem path, uptime counter, and AI provider status (including last
interaction success/failure tracking) to the health endpoint response.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-02-12 21:16:22 -05:00
Alex Newman 98d87d7573 chore: bump version to 10.0.4
Reverts v10.0.3 chroma-mcp spawn storm fix (broken release).
Restores codebase to v10.0.2 state.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-02-11 21:36:34 -05:00
Alex Newman 3f01baebfe Merge remote-tracking branch 'origin/main' into fix/chroma-mcp-spawn-storm
# Conflicts:
#	src/services/worker-service.ts
#	tests/infrastructure/process-manager.test.ts
2026-02-11 15:43:08 -05:00
Rod Boev a3f9e7f638 fix: prevent chroma-mcp spawn storm with 5-layer defense (641 processes → max 2)
During SIGHUP testing with 6+ active sessions, ChromaSync.ensureConnection()
had no mutex — concurrent fire-and-forget syncObservation() calls each spawned
a chroma-mcp subprocess via StdioClientTransport, creating 641 orphans in ~5min.
Error-driven reconnection formed a positive feedback loop amplifying the storm.

Defense layers:
- Layer 0: Connection mutex via promise memoization (prevents concurrent spawns)
- Layer 1: Pre-spawn process count guard using execFileSync('ps') (kills excess)
- Layer 2: Hardened close() with try-finally + Unix pkill in GracefulShutdown
- Layer 3: Count-based orphan reaper in ProcessManager (not age-based)
- Layer 4: Circuit breaker stops retries after 3 consecutive failures for 60s

Closes #1063, closes #695
Relates to #1010, #707
2026-02-11 07:19:28 -05:00
Rod Boev 4e67393d27 fix: prevent daemon silent death from SIGHUP + unhandled errors
Root cause: registerSignalHandlers() handled SIGTERM/SIGINT but not
SIGHUP. When the parent hook process exits, the kernel sends SIGHUP
to the daemon, causing immediate termination (default signal action).

Belt-and-suspenders fix:
1. SIGHUP handler: ignore in daemon mode, graceful shutdown otherwise
2. setsid: spawn daemon in new session on Linux (prevents SIGHUP delivery)
3. Global unhandledRejection/uncaughtException guards in daemon mode
2026-02-11 00:35:53 -05:00
Rod Boev 418e38ee46 fix: hook resilience and worker lifecycle improvements (#957, #923, #984, #987, #1042)
Reduce timeouts to eliminate 10-30s startup delay when worker is dead
(common on WSL2 after hibernate). Add stale PID detection, graceful
error handling across all handlers, and error classification that
distinguishes worker unavailability from handler bugs.

- HEALTH_CHECK 30s→3s, new POST_SPAWN_WAIT (5s), PORT_IN_USE_WAIT (3s)
- isProcessAlive() with EPERM handling, cleanStalePidFile()
- getPluginVersion() try-catch for shutdown race (#1042)
- isWorkerUnavailableError: transport+5xx+429→exit 0, 4xx→exit 2
- No-op handler for unknown event types (#984)
- Wrap all handler fetch calls in try-catch for graceful degradation
- CLAUDE_MEM_HEALTH_TIMEOUT_MS env var override with validation
2026-02-10 15:34:35 -05:00
Alex Newman 5969d670d0 chore: bump version to 9.1.1
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-02-07 02:18:44 -05:00
Alex Newman 8dfcb5e612 chore: bump version to 9.1.0
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-02-07 01:05:38 -05:00
Alex Newman ff503d08a7 MAESTRO: Merge PR #657 - Add generate/clean CLI commands for CLAUDE.md management
Cherry-picked source changes from PR #657 (224 commits behind main).
Adds `claude-mem generate` and `claude-mem clean` CLI commands:
- New src/cli/claude-md-commands.ts with generateClaudeMd() and cleanClaudeMd()
- Worker service generate/clean case handlers with --dry-run support
- CLAUDE_MD logger component type
- Uses shared isDirectChild from path-utils.ts (DRY improvement over PR original)

Skipped from PR: 91 CLAUDE.md file deletions (stale), build artifacts,
.claude/plans/ dev artifact, smart-install.js shell alias auto-injection
(aggressive profile modification without consent).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-02-06 05:52:54 -05:00
Alex Newman 98920bd860 MAESTRO: Merge PR #662 - Add save_memory MCP tool for manual memory storage
Adds save_memory MCP tool allowing users to manually save observations
for semantic search. Source changes cherry-picked from PR #662 by
@darconada (build artifact conflicts resolved by direct application).

Closes #645.

Co-Authored-By: darconadalabarga <darconada@arsys.es>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-02-06 04:13:44 -05:00
Alex Newman 5dffb1ebb0 MAESTRO: fix(hooks): add session-complete handler to enable orphan reaper cleanup
Cherry-picked from PR #844 by @thusdigital. Sessions stayed in active
sessions map forever after summarize, causing the orphan reaper to think
all processes were still active. Adds session-complete as Stop phase 2
hook that calls POST /api/sessions/complete to remove sessions from the
active map, allowing the reaper to correctly identify and clean up
orphaned worker processes. Fixes #842.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-02-06 03:23:13 -05:00
Alex Newman da1d2cd36a MAESTRO: fix(db): prevent FK constraint failures on worker restart
Cherry-picked source changes from PR #889 by @Et9797. Fixes #846.

Key changes:
- Add ensureMemorySessionIdRegistered() guard in SessionStore.ts
- Add ON UPDATE CASCADE migration (schema v21) for observations and session_summaries FK constraints
- Change message queue from claim-and-delete to claim-confirm pattern (PendingMessageStore.ts)
- Add spawn deduplication and unrecoverable error detection in SessionRoutes.ts and worker-service.ts
- Add forceInit flag to SDKAgent for stale session recovery

Build artifacts skipped (pre-existing dompurify dep issue). Path fixes (HealthMonitor.ts, worker-utils.ts)
already merged via PR #634.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-02-06 03:16:17 -05:00
Alex Newman 2d40afe7ef fix: provider-aware recovery and stale session cleanup (PR #741)
Applies PR #741 by @licutis onto main, resolving conflicts with
recently merged PRs #693, #937, and #627. Adds getActiveAgent() to
WorkerService so startup-recovery uses the correct provider instead
of hardcoding SDKAgent. Also cleans up sessions stuck 'active' for
6+ hours and their pending messages before processing orphaned queues.

Co-Authored-By: licutis <43884712+licutis@users.noreply.github.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-02-06 01:58:00 -05:00
jayvenn21 f24bba21e9 fix(worker): gracefully process orphaned pending messages after session termination 2026-02-06 01:47:43 -05:00
Rajiv Sinclair 27aa98c269 fix: wait for database initialization before processing session-init requests
Fixes a race condition where the UserPromptSubmit hook could call
/api/sessions/init before the database is fully initialized, resulting
in a 500 error with "Database not initialized".

Root cause:
- Worker starts HTTP server immediately for fast startup
- Health endpoint (/api/health) returns OK when server is listening
- session-init hook waits for health check, then calls /api/sessions/init
- Database initialization happens in background, creating a race window

Solution:
- Add early handler for /api/sessions/init that waits for initialization
- Uses same pattern as existing /api/context/inject handler
- Returns 503 with retry message if initialization times out

The fix ensures hooks receive proper responses even during the brief
startup window when the server is listening but DB isn't ready yet.

Co-Authored-By: Claude Code <noreply@anthropic.com>
2026-02-05 22:23:52 -05:00
Alex Newman 0ecb387f58 MAESTRO: Implement Windows spawn guard to prevent repeated worker popups (closes #921)
Adds file-based cooldown lock in ensureWorkerStarted() so failed worker
spawn attempts on Windows don't produce repeated bun.exe terminal popups.
Based on the approach from PR #931, integrated into the refactored code.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-02-05 18:59:53 -05:00
Rod Boev b8b88d998c fix: fail open on /api/context/inject during initialization
The /api/context/inject endpoint previously blocked for up to 5 minutes
(300s timeout) waiting for initializationComplete, which includes MCP
connection setup. On Windows, the MCP connection can hang indefinitely,
causing the context hook to never return and blocking Claude Code startup.

This change makes the endpoint fail open: if the worker hasn't finished
initializing, return empty context immediately instead of blocking. The
hook completes fast, and context becomes available on subsequent prompts
once initialization finishes in the background.

Closes #958
2026-02-05 18:22:08 -05:00
Alex Newman 4df9f61347 refactor: implement in-process worker architecture for hooks (#722)
* fix: stop generating empty CLAUDE.md files

- Return empty string instead of "No recent activity" when no observations exist
- Skip writing CLAUDE.md files when formatted content is empty
- Remove redundant "auto-generated by claude-mem" HTML comment
- Clean up 98 existing empty CLAUDE.md files across the codebase
- Update tests to expect empty string for empty input

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* build assets

* refactor: implement in-process worker architecture for hooks

Replaces spawn-based worker startup with in-process architecture:
- Hook processes now become the worker when port 37777 is free
- Eliminates Windows spawn issues (NO SPAWN rule)
- SessionStart chains: smart-install && stop && context

Key changes:
- worker-service.ts: hook case starts WorkerService in-process
- hook-command.ts: skipExit option prevents process.exit() when hosting worker
- hooks.json: single chained command replaces separate start/hook commands
- worker-utils.ts: ensureWorkerRunning() returns boolean, doesn't block
- handlers: graceful fallback when worker unavailable

All 761 tests pass. Manual verification confirms hook stays alive as worker.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* context

* a

* MAESTRO: Mark PR #722 test verification task complete

All 797 tests passed (3 skipped, 0 failed) after merge conflict resolution.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* MAESTRO: Mark PR #722 build verification task complete

* MAESTRO: Mark PR #722 code review task complete

Code review verified:
- worker-service.ts hook case starts WorkerService in-process
- hook-command.ts has skipExit option
- hooks.json uses single chained command
- worker-utils.ts ensureWorkerRunning() returns boolean

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* MAESTRO: Mark PR #722 conflict resolution push task complete

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
2026-02-04 19:49:15 -05:00