* feat(ux): claude-mem UX improvements with installer enhancements
Squashed PR #2156 commits for clean rebase onto main:
- feat(installer): add provider selection, model prompt, worker auto-start
- refactor: rename *Agent provider classes to *Provider
- feat: add /learn-codebase skill and viewer welcome card
- feat(worker): inject welcome hint when project has zero observations
- fix(pr-2156): address greptile review comments
- fix(pr-2156): address coderabbit review comments
- fix(pr-2156): persist CLAUDE_MEM_PROVIDER for non-claude in non-TTY mode
- fix(pr-2156): file-backed settings reads in installer + env-first SKILL doc
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* build: rebuild plugin artifacts after rebase onto v12.4.7
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor(skills): strip claude-mem internals from learn-codebase
The learn-codebase skill, install next-step copy, WelcomeCard, and
welcome-hint previously walked the primary agent through worker endpoints
and synthetic observation payloads. The PostToolUse hook already captures
every Read/Edit the agent makes — the agent should have no awareness that
the memory layer exists. Collapse the skill to one instruction ("read every
source file in full") and rephrase touchpoints to describe only what the
user observes (Claude reading files), not what happens behind the scenes.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(sync): preflight version mismatch + settings-aware port resolution
Two related fixes for build-and-sync's worker restart step:
1. Read CLAUDE_MEM_WORKER_PORT from ~/.claude-mem/settings.json the same
way the worker does, instead of computing the default port from the
uid alone. Previously, users with a custom port saw a misleading
"Worker not running" message because the restart POST hit the wrong
port and got ECONNREFUSED.
2. Add a preflight check that aborts the sync when the running worker's
reported version does not match the version we are about to build.
Claude Code's plugin loader pins the worker to a specific cache
version per session, so syncing into a newer cache directory has no
effect until the user runs `claude plugin update thedotmack/claude-mem`
to bump the pin. The preflight surfaces this explicitly with the exact
command to run; --force bypasses it for intentional cases.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* docs(learn-codebase): note sed for partial reads of large files
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor: strip comments codebase-wide
Removed prose comments from all tracked source. Preserved directives
(@ts-ignore, eslint-disable, biome-ignore, prettier-ignore, triple-slash
references, webpack magic, shebangs). Deleted two tests that asserted
on comment text rather than runtime behavior.
Net: 401 files, -14,587 / +389 lines, -10.4% bytes.
Verified: typecheck passes, build passes, test count unchanged from
baseline (22 pre-existing fails, all unrelated).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor(installer): move runtime setup into npx, eliminate hook dead air
Smart-install ran 3 times during a fresh install — the worst run was silent,
fired by Claude Code's Setup hook after `claude plugin install`, producing
~30s of dead air that looked like the plugin was hung.
This change makes `npx claude-mem install` the single place heavy work
happens, with a visible spinner. Hooks become runtime-only.
- New `src/npx-cli/install/setup-runtime.ts` module: ensureBun, ensureUv,
installPluginDependencies, read/writeInstallMarker, isInstallCurrent.
Marker schema preserved exactly ({version, bun, uv, installedAt}) so
ContextBuilder and BranchManager readers keep working.
- `npx claude-mem install`: ungated copy/register/enable for every IDE,
inserts a "Setting up runtime" task with honest "first install can take
~30s" spinner. The claude-code shell-out to `claude plugin install` is
removed — npx already populated everything Claude reads.
- New `npx claude-mem repair` command for post-`claude plugin update`
recovery, force-reinstalls runtime.
- Setup hook now runs `plugin/scripts/version-check.js` (29ms wall) instead
of smart-install. Mismatch prints "run: npx claude-mem repair" on stderr.
Always exits 0 (non-blocking, per CLAUDE.md exit-code strategy).
- SessionStart loses the smart-install entry; 2 hooks remain (worker start,
context fetch).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* chore(installer): delete smart-install sources, retarget tests
- Delete scripts/smart-install.js + plugin/scripts/smart-install.js (both
are source files kept in sync manually; both must go).
- Delete tests/smart-install.test.ts (covered surface is gone).
- tests/plugin-scripts-line-endings: drop smart-install.js entry.
- tests/infrastructure/plugin-distribution: retarget two assertions at
version-check.js (the new Setup hook script).
- New tests/setup-runtime.test.ts: 9 tests covering marker read/write,
isInstallCurrent semantics. Marker schema invariant verified.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* docs(installer): describe npx-driven setup + version-check Setup hook
Sweep public docs and architecture notes to reflect the new flow:
npx installer does Bun/uv setup with a visible spinner; Setup hook runs
sub-100ms version-check.js; users hit `npx claude-mem repair` after a
`claude plugin update`.
- docs/architecture-overview.md: hook lifecycle table + npx flow paragraph
- docs/public/configuration.mdx: tree + hook config example
- docs/public/development.mdx: build output line
- docs/public/hooks-architecture.mdx: full rewrite of pre-hook section,
timing table, performance table
- docs/public/architecture/{overview,hooks,worker-service}.mdx: tree
comments, JSON config example, Bun requirement section
docs/reports/* untouched (historical incident reports).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(install): mergeSettings writes via USER_SETTINGS_PATH
Greptile P1 (#2156): `settingsFilePath()` only resolved
`process.env.CLAUDE_MEM_DATA_DIR`, while `getSetting()` reads via
`USER_SETTINGS_PATH` which `resolveDataDir()` populates from BOTH the env
var AND a `CLAUDE_MEM_DATA_DIR` entry persisted in
`~/.claude-mem/settings.json`. Result: a user with the data dir saved in
settings.json but not exported in their shell would have provider/model
settings silently written to `~/.claude-mem/settings.json` while
`getSetting()` read from `/custom/path/settings.json` — read/write split.
Drop `settingsFilePath()` and the now-unused `homedir` import; reuse the
already-imported `USER_SETTINGS_PATH` constant.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(cli): parse --provider, --model, --no-auto-start install flags
Greptile P1 (#2156): InstallOptions has fields `provider`, `model`,
`noAutoStart`, but the install case in the npx-cli switch only parsed
`--ide`. The other three flags were silently dropped — `npx claude-mem
install --provider gemini` was a no-op.
Extract a `parseInstallOptions(argv)` helper, share it between the bare
`npx claude-mem` and `npx claude-mem install` paths, and validate
`--provider` against the allowed set. Update help text accordingly.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(install): pipe runtime-setup output, always show IDE multiselect
Two issues caught in a docker test of the installer:
1. The bun.sh installer, uv installer, and `bun install` were using
stdio: 'inherit', dumping their stdout/stderr through clack's spinner
region — visible as raw "downloading uv 0.11.8…" / "Checked 58
installs across 38 packages…" text streaming under the spinner. Switch
to stdio: 'pipe' and surface captured stderr only on failure (via a
shared describeExecError() helper that includes stdout when stderr is
empty). Spinner stays clean on the happy path.
2. promptForIDESelection() silently picked claude-code when no IDEs were
detected, never showing the user the multiselect. On a fresh machine
with no IDEs present yet (e.g. our docker test container), the user
never got to choose. Now: always show the full IDE list when
interactive; mark detected ones with [detected] hints and pre-select
them; show a warn line if zero are detected explaining they should pick
what they plan to use. Non-TTY callers still get the silent
claude-code default at the call site (unchanged).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(install): skip marketplace work for claude-code-only, offer to install Claude Code
Two related UX fixes from a docker test:
**Delay between "Saved Claude model=…" and "Plugin files copied OK"**
After dropping the needsManualInstall gate, every install was unconditionally
running `copyPluginToMarketplace` (which copied the entire root node_modules
tree — thousands of files, dozens of seconds) and `runNpmInstallInMarketplace`
(npm install --production) even when only claude-code was selected. Neither
is needed for claude-code: that path uses the plugin cache dir + the
installed_plugins.json + enabledPlugins flag, all of which we already write.
- Drop `node_modules` from `copyPluginToMarketplace`'s allowed-entries list;
the dependency-install task populates it on the destination side anyway.
- Re-introduce `needsMarketplace = selectedIDEs.some(id => id !== 'claude-code')`
scoped *only* to `copyPluginToMarketplace`, `runNpmInstallInMarketplace`,
and the pre-install `shutdownWorkerAndWait` (also pointless for claude-code-
only flows since we're not overwriting the worker's running cache dir
source). All other tasks (cache copy, register, enable, runtime setup) stay
unconditional.
**Claude Code missing → silent install of an IDE that isn't there**
When the user picked claude-code on a machine without it (e.g. a fresh
container), the install completed but `claude` was unavailable and the only
hint was a generic warn line. Replace with an explicit pre-flight prompt:
Claude Code is not installed. Claude-mem works best in Claude Code, but
also works with the IDEs below.
? Install Claude Code now?
◆ Yes — install Claude Code (recommended)
◯ No — pick another IDE below
◯ Cancel installation
If the user picks "Yes", run `curl -fsSL https://claude.ai/install.sh | bash`
(or the PowerShell equivalent on Windows), then re-detect IDEs and proceed
with claude-code pre-selected. If the install fails or the user picks "No",
the multiselect still appears with claude-code visible (just unmarked
[detected]), so they can opt in or pick another IDE.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(install): detect Claude Code via `claude` CLI, not ~/.claude dir
The directory `~/.claude` can exist (e.g. mounted in Docker, or created
by tooling) without Claude Code actually being installed. Detect the
`claude` command in PATH instead so the installer correctly offers to
install Claude Code when missing.
* docs(learn-codebase): add reviewer note explaining the cost tradeoff
The skill intentionally reads every file in full to build a cognitive
cache that pays off across the rest of the project. Add a brief note
so reviewers (human or bot) understand the tradeoff before flagging
the unbounded read as a cost issue.
* fix: address Greptile P1 feedback on welcome hint and learn-codebase
- SearchRoutes: skip welcome hint when caller passes ?full=true so
explicit full-context requests aren't intercepted by the hint.
- learn-codebase: replace `sed` instruction with the Read tool's
offset/limit parameters, since Bash is gated in Claude Code by
default.
* feat(install): ASCII-animated logo splash on interactive install
Plays a ~1s bloom animation of the claude-mem sunburst logomark when
the installer starts in an interactive terminal — geometrically rendered
via 12 ray curves around a center disc, in the brand orange. The
wordmark and tagline type on alongside the final frame.
Auto-skipped on non-TTY, in CI, when NO_COLOR or CLAUDE_MEM_NO_BANNER
is set, or when the terminal is too narrow.
Inspired by ghostty +boo.
* feat(banner): replace rotation frames with angular-sector bloom generator
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat(banner): replace rotation frames with angular-sector bloom generator
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat(banner): three-act choreography renderer with radial gradient and diff redraw
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat(banner): update preview script to support small/medium/hero tier selection
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(docker): add COLORTERM=truecolor to test-installer sandbox
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat(install): auto-apply PATH for Claude Code with spinner UX
The Claude Code install.sh prints a Setup notes block telling users to
manually edit "your shell config file" to add ~/.local/bin to PATH —
which left fresh installs unable to launch claude from the command line.
After a successful install, detect ~/.local/bin/claude on disk and, if
the dir is missing from PATH, append the right export line to .zshrc /
.bash_profile / .bashrc / fish config (idempotent, marked with a
comment). Also updates process.env.PATH for the current install run.
Wraps the curl|bash install in a clack spinner (interactive only) so the
~4 minute native-build download doesn't look frozen — output is captured
silently and dumped on failure for debuggability. Non-interactive mode
keeps inherited stdio for CI logs.
Verified end-to-end in the test-installer docker sandbox: spinner
animates, .bashrc gets the export, fresh login shell resolves claude.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat(banner): video-frame ASCII renderer with three-act choreography
Generator switched from a single Jimp-rendered logo to pre-extracted
video frames concatenated with \x01 separators and gzip-deflated, ported
from ghostty's boo wire format. Renderer rewritten around three acts
(ignite → stagger bloom → text reveal + breathe) with adaptive sizing,
radial gradient, and diff-based redraw.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat(onboarding): unify install / SessionStart / viewer around one first-success moment
Three surfaces now point at the same north-star moment — open the viewer, do
anything in Claude Code, watch an observation appear within seconds — with the
same verbatim timing and privacy lines, and a single canonical "how it works"
explainer instead of three diverging copies.
- Canonical explainer at src/services/worker/onboarding-explainer.md served via
GET /api/onboarding/explainer; mirrored into plugin/skills/how-it-works/SKILL.md
- SessionStart welcome hint rewritten as third-person status (no imperatives
Claude tries to execute), pinned with a default-value regression test
- Post-install Next Steps reframed as "two paths": passive default + optional
/learn-codebase front-load; drops /mem-search and /knowledge-agent from this
surface; adds verbatim timing + privacy lines and /how-it-works link
- /api/stats response gains firstObservationAt for the viewer stat row
- Viewer WelcomeCard branches on observationCount === 0: empty state shows live
worker-connection dot + "waiting for activity"; has-data state shows
observations · projects · since [date] and two example prompts. v2 dismiss key
- jimp added to package.json to fix pre-existing banner-frame build break
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(banner): play unconditionally; only honor CLAUDE_MEM_NO_BANNER
The 128-col / TTY / CI / NO_COLOR gates silently swallowed the banner in
narrower terminals, CI logs, and any non-TTY pipe — including Docker runs
where -it should preserve the experience but column width was the wrong
gate. Remove the implicit gates; keep the explicit opt-out only.
If a frame wraps in a narrow terminal, that's better than the banner
not playing at all.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* revert(banner): restore 15:33 gating logic per user request
Reverts eb6fc157. Restores isBannerEnabled to the state at commit
8e448015 (2026-04-30 15:33): TTY check, !CI, !NO_COLOR, !CLAUDE_MEM_NO_BANNER,
and cols >= BANNER.width.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat(install): wrap remaining slow steps with spinners
Each IDE installer (Cursor, Gemini CLI, OpenCode, Windsurf, OpenClaw,
Codex CLI, MCP integrations) now runs inside a clack task spinner with
per-step progress messages instead of silent dynamic-import + cpSync.
Pre-overwrite worker shutdown (up to 10s) and the post-install health
probe (up to 3s) also get spinners.
Internal console.log/error/warn from each IDE installer is buffered
during the spinner; if the install fails, captured output is replayed
afterward via log.warn so users can see what broke.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(review): observation count + IDE pre-selection regressions
WelcomeCard's "no observations yet" empty state was triggered when a
project filter narrowed the feed to zero rows, even with thousands of
observations elsewhere. Source the count from global stats.database
to match firstObservationAt's scope.
Restore initialValues: [] in the IDE multiselect — pre-selecting every
detected IDE was the exact regression #2106 was filed for.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(install): trichotomy worker state + cache fallback for script path
ensureWorkerStarted now returns 'ready' | 'warming' | 'dead' instead of
boolean. The spawned-but-still-warming case (common in Docker cold
starts and slow first-time inits) was being misreported as 'did not
start', which contradicted the next-steps panel saying 'still starting
up'. Install task message and Next Steps headline now agree on the
actual state.
Also fixes the actual root cause of 'Worker did not start' on
claude-code-only installs: the worker script path was hardcoded to the
marketplace dir, which is left empty when no non-claude-code IDE is
selected. Now falls back to pluginCacheDirectory(version) when the
marketplace copy isn't present.
Verified end-to-end in docker/claude-mem with --ide claude-code,
--ide cursor, and a fresh container — install task and headline
agree on 'Worker ready at http://localhost:<port>' in all cases.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* docs: align CLAUDE.md and public docs with current code
Sweep across CLAUDE.md and 10 high-traffic docs/public/ MDX files to
remove point-in-time references and align with the actual current
shape of the codebase. Highlights:
- Hardcoded port 37777 → per-user formula (37700 + uid % 100) on the
front-door pages (introduction, installation, configuration,
architecture/overview, architecture/worker-service, troubleshooting,
hooks-architecture, platform-integration).
- Default model 'sonnet' → 'claude-haiku-4-5-20251001' (matches
SettingsDefaultsManager).
- Node 18 → 20 (matches package.json engines).
- Lifecycle hook count corrected (5 events).
- Removed the nonexistent 'Smart Install' component and pre-built
directory tree referencing files that no longer exist
(context-hook.ts, save-hook.ts, cleanup-hook.ts, etc.); replaced
with the real worker dispatcher shape.
- Removed CLAUDE.md '#2101' issue tag (kept the design rationale).
- Replaced obsolete hooks.json example with a description of the real
bun-runner.js / worker-service.cjs hook event shape.
Lower-traffic doc pages still hardcode 37777 — left for a separate
global pass.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* chore(scripts): land strip-comments around real parsers (postcss, remark, parse5)
Each language gets a real parser to locate comments, then we splice ranges
out of the original source. The library never serializes — that's how
remark-stringify produced 243 reformat-noise diffs in the first attempt
versus the 21 real strip targets here.
JS/TS/JSX -> ts.createSourceFile + getLeadingCommentRanges
CSS/SCSS -> postcss.parse + walkComments + node.source offsets
MD/MDX -> remark-parse (+ remark-mdx) + AST html / mdx-expression nodes
HTML -> parse5 with sourceCodeLocationInfo
shell/py -> kept hand-rolled hash stripper (no library worth the dep)
Preserves: shebangs, @ts-* directives, eslint-disable, biome-ignore,
prettier-ignore, triple-slash refs, webpack magic, /*! license keep,
@strip-comments-keep file marker. JS/TS handler runs a parse-roundtrip
check and refuses to write if syntax errors increased (catches the
worker-utils.ts breakage class from the 2026-04-29 attempt).
npm scripts:
strip-comments (apply)
strip-comments:check (CI-style, exits non-zero if changes needed)
strip-comments:dry-run (list, no writes)
Verified --check on this repo: 21 changes, -4.0% bytes, no parse-error
regressions, no reformat-suspect false positives.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor: strip comments codebase-wide via parser-backed tool
21 files changed, -17,550 bytes (-4.0%) of narrative comments removed
across .ts / .tsx / .js / .mjs and the .gitignore. JS/TS comments stripped
via ts.createSourceFile + getLeadingCommentRanges — same canonical lexer,
same behavior as the 2026-04-29 strip, no reformat noise.
Preexisting baseline (unchanged):
typecheck: 16 errors at HEAD, 16 errors after strip (line numbers shift,
no new error classes — verified via diff of sorted error lists)
build: fails at HEAD with CrushHooksInstaller.js unresolved import
(preexisting, unrelated to this strip)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(install): drop Crush integration references after extract
The Crush integration was extracted to its own branch on May 1, but the
import at install.ts:280 (and the case block + ide-detection entry +
McpIntegrations config + npx-cli help text) still referenced the now-
removed CrushHooksInstaller.js, breaking the build.
Removes:
- case 'crush' block in install.ts
- crush entry in ide-detection.ts
- CRUSH_CONFIG and registration in McpIntegrations.ts
- 'crush' from the IDE Identifiers help line in index.ts
Rebuilds worker-service.cjs to match.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* chore(banner): mark generated banner-frames.ts with @strip-comments-keep
Without this, every build/strip cycle ping-pongs five lines of doc
comments in and out of the auto-generated output. The keep-marker tells
strip-comments.ts to skip the file entirely.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(build): drop banner-frame regen from build script
generate-banner-frames.mjs requires PNG frames in /tmp/cmem-banner-frames
that only exist after the maintainer runs ffmpeg locally on the source
video. CI has neither the video nor the frames, so the build broke on
Windows. The output (src/npx-cli/banner-frames.ts) is committed, so the
regen is a one-shot dev step — not a build step. Run the script directly
when the video changes.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(worker): unstick the spinner — kill claim-self-lock, wake on fail, auto-broadcast
Three surgical changes that cure the stuck-spinner bug at the source.
Phase 1.1 (L9): claimNextMessage no longer self-excludes its own worker_pid.
A single UPDATE-RETURNING grabs the oldest pending row by id. Removes the
LiveWorkerPidsProvider plumbing that was never injected — Supervisor enforces
single-worker via PID file, so the multi-worker SQL was defending against a
configuration the project does not support.
Phase 1.2 (L19): SessionManager.markMessageFailed wraps PendingMessageStore.markFailed
and emits 'message' on the per-session EventEmitter. The iterator's waitForMessage
now wakes immediately on re-pend instead of parking for 3 minutes. ResponseProcessor
and SessionRoutes routed through the new wrapper.
Phase 1.3 (L24): PendingMessageStore takes an optional onMutate callback fired
from every mutator (enqueue, claimNextMessage, confirmProcessed, markFailed,
transitionMessagesTo, clearFailedOlderThan). SessionManager wires it; WorkerService
passes broadcastProcessingStatus. Ten manual broadcast calls deleted across
SessionCleanupHelper, SessionEventBroadcaster, SessionRoutes, DataRoutes, and
worker-service. Caller discipline becomes structurally impossible to forget.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor(worker): delete dead code — legacy routes, processPendingQueues, decorative guards
Pure deletions. Phase 2 of kill-the-asshole-gates.
- Legacy /sessions/:sessionDbId/* routes (handleSessionInit, handleObservations,
handleSummarize, handleSessionStatus, handleSessionDelete, handleSessionComplete)
bypassed all five ingest gates and were a parallel write path. Folded the
initializeSession + broadcastNewPrompt + syncUserPrompt + ensureGeneratorRunning
+ broadcastSessionStarted work into the canonical /api/sessions/init handler so
the hook makes one round trip instead of two.
- processPendingQueues (~104 lines, zero callers) — replaced in Phase 6 by a
one-statement startup sweep.
- spawnInProgress Map and crashRecoveryScheduled Set — decorative dedupe over
generatorPromise and stillExists checks that already provide the real safety.
- STALE_GENERATOR_THRESHOLD_MS — pre-empted live generators and raced with the
finally block; the 3min idle timeout already kills zombies.
- MAX_SESSION_WALL_CLOCK_MS — ran a SELECT on every observation to enforce 24h.
Runaway-spend protection lives in the API key, not in claude-mem.
- Missing-id 400 in shared.ts ingestObservation — Zod already enforces min(1)
on contentSessionId and toolName at the route schema.
- SessionCompletionHandler import + completionHandler field on SessionRoutes
(orphaned after handler deletions).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor(worker): SQL-backed getTotalQueueDepth — single source of truth
Was: iterate this.sessions.values() and sum getPendingCount per session.
Now: SELECT COUNT(*) FROM pending_messages WHERE status IN ('pending','processing').
The in-memory sessions Map drifted from the DB rows whenever a generator exited
without confirm/fail, leading to false-positive isProcessing in the UI. Phase 1.3's
auto-broadcast fires on every mutation, but it broadcast a stale Map count.
Reading from the DB makes the UI's spinner state match what the queue actually holds.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor(worker): typed abortReason replaces wasAborted boolean
Was: a boolean wasAborted that lumped every abort together. The finally block
branched on !wasAborted, so any abort skipped restart — including idle aborts
with pending work, which is exactly the case where we DO want to restart.
Now: ActiveSession.abortReason is a typed enum 'idle' | 'shutdown' | 'overflow'
| 'restart-guard'. The finally block consumes the reason and only skips restart
for 'shutdown' and 'restart-guard'. Idle and overflow aborts fall through, so
if pending work exists they trigger restart correctly.
Dropped 'stale' and 'wall-clock' from the union — Phase 2 deleted those paths.
Natural-completion abort (post-success) intentionally has no reason; it's not
gating restart logic.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor(worker): unify the two generator-exit finally blocks
Was: worker-service.ts:startSessionProcessor and SessionRoutes:ensureGeneratorRunning
each had their own ~70-line finally block with divergent restart-guard handling.
The worker-service path called terminateSession on RestartGuard trip and orphaned
pending rows (the L16 bug); the SessionRoutes path drained them. Two places to
update when rules changed.
Now: handleGeneratorExit in src/services/worker/session/GeneratorExitHandler.ts
owns the contract:
1. Always kill the SDK subprocess if alive.
2. Always drain processingMessageIds via sessionManager.markMessageFailed
(which wakes the iterator — Phase 1.2).
3. shutdown / restart-guard reasons: drain pending rows via
transitionMessagesTo('failed'), finalize, remove from Map. Fixes L16.
4. pendingCount=0: finalize normally and remove from Map.
5. pendingCount>0: backoff respawn via per-session respawnTimer (no global Set;
Phase 2.4 deleted that). RestartGuard trip drains to 'abandoned'.
Both finally blocks are now ~10-line wrappers that translate local state into the
canonical abortReason and delegate. Restored completionHandler injection into
SessionRoutes (was dropped in Phase 2 cleanup; needed by the unified helper for
finalizeSession).
Behavior change: SessionRoutes' previous "keep idle session in memory" was
deliberately replaced by the plan's "remove from Map on natural completion" —
next observation reinitializes via getMessageIterator → initializeSession.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat(worker): startup orphan sweep — reset 'processing' rows at boot
When the worker dies (crash, kill, restart), any pending_messages rows it left
in 'processing' state are by definition orphans (the only worker is dead).
Single SQL UPDATE at boot resets them to 'pending' so the iterator can claim
them again. Replaces the deleted processPendingQueues function (Phase 2.2).
Runs in initializeBackground after dbManager.initialize() and before the
initializationComplete middleware releases blocked HTTP requests, so no
in-flight request can race the sweep. NOT on a periodic timer — after boot,
every 'processing' row has a live consumer and a periodic sweep would race.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor(worker): simplify enqueue catch, replace memorySessionId throw with re-pend
7.1: queueObservation's catch was logging two ERROR-level messages and rethrowing.
The rethrow is correct (FK violations / disk full / schema drift should crash
loudly), but the verbose ERROR logging pretended the error was recoverable.
Reduced to one INFO line + rethrow.
7.2: ResponseProcessor's memorySessionId guard was throwing if the SDK hadn't
included session_id on the first user-yield, terminal-failing the entire batch.
Now warns and re-pends in-flight messages via sessionManager.markMessageFailed
(which wakes the iterator — Phase 1.2). The next iteration tries again with
memorySessionId hopefully captured.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(sync): mirror builds to installed-version cache for hot reload
When package.json bumps past Claude Code's installed pin, sync-marketplace
wrote new code to cache/<buildVersion>/ but the worker loaded from
cache/<installedVersion>/, so worker:restart reloaded the same old code.
Replace the exit-on-mismatch preflight with a mirror step: when versions
differ, also rsync plugin/ into cache/<installedVersion>/ so worker:restart
hot-reloads new code without a Claude Code session restart. The
build-version cache still gets written for the eventual
`claude plugin update`.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* chore: delete dead barrel files and orphan utilities
- src/sdk/index.ts (re-exports parser+prompts; nothing imported the barrel)
- src/services/Context.ts (re-exports ./context/index.js; no importers)
- src/services/integrations/index.ts (no importers)
- src/services/worker/Search.ts (3-line barrel of ./search/index.js)
- src/services/infrastructure/index.ts: drop CleanupV12_4_3 re-export
- src/utils/error-messages.ts (getWorkerRestartInstructions never imported)
- src/types/transcript.ts (170 LoC of types, zero importers)
- src/npx-cli/_preview.ts (banner dev preview, no script wires it)
Build + tests still pass; observations still flowing.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* chore(parser): drop unused detectLanguage
Only the user-grammar-aware variant detectLanguageWithUserGrammars()
is actually called.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* chore(types): drop unused SdkSessionRecord + ObservationWithContext
Both interfaces in src/types/database.ts had zero importers anywhere
in src or tests.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* chore(npx-cli): drop unused getDetectedIDEs + claudeMemDataDirectory
getDetectedIDEs has no callers — install.ts uses detectInstalledIDEs
directly. claudeMemDataDirectory has no callers either.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* chore(ProcessManager): drop dead orphan-reaper + signal-handler helpers
Each had zero callers in src/ or tests/:
- cleanupOrphanedProcesses + enumerateOrphanedProcesses
- ORPHAN_PROCESS_PATTERNS + ORPHAN_MAX_AGE_MINUTES
- forceKillProcess
- waitForProcessesExit
- createSignalHandler
- resetWorkerRuntimePathCache
The orphan reaper was retired in PATHFINDER Plan 02 ("OS process groups
replace hand-rolled reapers", commit 94d592f2) — these were the leftover
pieces. shutdown.ts uses the supervisor's own kill-pgid path instead.
parseElapsedTime kept (covered by tests/infrastructure/process-manager.test.ts).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* chore(scripts): delete 11 unreferenced DX/forensic scripts
None of these are referenced by package.json npm scripts or docs/.
All last touched on Apr 29 only as part of the comment-stripping
pass — the feature code itself is older and orphaned:
analyze-transformations-smart.js
debug-transcript-structure.ts
dump-transcript-readable.ts
endless-mode-token-calculator.js
extract-prompts-to-yaml.cjs
extract-rich-context-examples.ts
find-silent-failures.sh
fix-all-timestamps.ts
format-transcript-context.ts
test-transcript-parser.ts
transcript-to-markdown.ts
These are standalone tools — runtime behavior unchanged.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* chore(scripts): delete unused extraction/ and types/ subdirs
- scripts/extraction/{extract-all-xml.py, filter-actual-xml.py, README.md}
point at ~/Scripts/claude-mem/ — the user's pre-relocation path that no
longer exists. Zero references in package.json, src/, or tests/.
- scripts/types/export.ts duplicates ObservationRecord etc. and has no
importers (CodexCliInstaller imports transcripts/types, not this).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* chore(BranchManager): drop dead getInstalledPluginPath
OpenCodeInstaller has its own (used) getInstalledPluginPath; the
BranchManager copy never had any external callers.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* chore(ChromaSyncState): unexport DocKind (used internally only)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* test(gemini): drop stale earliestPendingTimestamp / processingMessageIds
Both fields were removed from ActiveSession in earlier queue-engine
cleanup. Tests had been silently keeping them because the mock sessions
use 'as any' to bypass strict typing, so the dead fields rode along
without complaint.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* chore: drop 3 unused module-level constants
- src/npx-cli/banner.ts: CURSOR_HOME, CLEAR_DOWN (banner uses
CLEAR_SCREEN which combines clear-down + cursor-home into a single
CSI sequence; the standalone constants were leftovers).
- src/services/worker/BranchManager.ts: DEFAULT_SHELL_TIMEOUT_MS
(BranchManager only uses GIT_COMMAND_TIMEOUT_MS / NPM_INSTALL_TIMEOUT_MS).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* chore(opencode-plugin): drop dead workerPost helper
Only the fire-and-forget variant (workerPostFireAndForget) is actually
called. workerPost was the await-result version with no remaining caller.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* chore: drop 8 truly-unused interface fields
Verified each by grepping for `.field`, `"field"`, `'field'`, and
`field:` patterns across src/ + tests/ + plugin/scripts. Where the
only remaining usage was the assignment site, removed the assignments too.
- GitHubStarsData: watchers_count, forks_count (only stargazers_count read)
- TableColumnInfo: dflt_value (PRAGMA returns it but no caller reads it)
- IndexInfo: seq (PRAGMA returns it but no caller reads it)
- ObservationRecord: source_files (legacy field, no readers)
- HookResult.hookSpecificOutput: permissionDecisionReason
- WatchTarget: rescanIntervalMs (set in config, never read)
- ShutdownResult: confirmedStopped (write-only — assigned but no
reader; updated all 3 return sites to drop it)
- ModePrompts: language_instruction (multilingual support never wired)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* chore(npx-cli): reuse InstallOptions type instead of inline duplicate
parseInstallOptions had its return type written out inline as an
anonymous duplicate of InstallOptions. Use the canonical type
(import type — zero bundle cost).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* chore(integrations): drop unused Platform type alias
The detectPlatform() function that returned this type was deleted earlier
in the branch (along with getScriptExtension that consumed it). The type
itself outlived its consumer; only string literals "Platform:" survive in
console.log diagnostics, which don't reference the alias.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(worker): broadcast processing_status when summarize is queued
broadcastSummarizeQueued was an empty no-op even though
handleSummarizeByClaudeId calls it after enqueueing. The PendingMessageStore
onMutate callback already fires broadcastProcessingStatus on enqueue, but
calling it explicitly from broadcastSummarizeQueued ensures the spinner
ticks on the moment a summary is requested even if the onMutate chain has
any timing race.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(worker): keep spinner on while summary generates
ClaudeProvider's SDK can pull multiple synthetic prompts (e.g.
observation + summarize) before producing responses. Each pull pushed
an ID to session.processingMessageIds. When the SDK's first
observation response came back, ResponseProcessor.confirmProcessed
deleted ALL pending message rows — including the still-in-flight
summary — so getTotalQueueDepth dropped to 0 and the spinner turned
off, even though the summary took another ~22s to actually generate.
Tag each in-flight message with its type ({id, type}) so the response
processor can pop only the FIFO message of the matching type
(observation vs summarize). The summary row stays in 'processing'
until its own response arrives, keeping the spinner lit through the
entire summary window.
Also updates Gemini/OpenRouter providers and GeneratorExitHandler for
the new shape.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(worker): clear summary from queue on any SDK response
Switch ResponseProcessor from type-aware FIFO matching to strict FIFO
popping (each SDK response → 1 in-flight message consumed). This way
the summary always clears when the SDK responds, even when the
response is unparseable or the summary doesn't actually generate
content — preventing stuck spinner / queue-depth-stuck-at-1.
Spinner behavior is preserved: messages enqueued after the summary
keep the queue depth elevated, and only when the SDK has responded
to every prompt does the queue drain to zero.
Also: when the consumed message is a 'summarize' and parsing fails,
treat it as best-effort and confirmProcessed (no retry) — summaries
that can't be parsed shouldn't keep retrying.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat(viewer): redesign welcome card and remove source filters
The first-start welcome card now explains the three feed card types
(observation/summary/prompt) with color-coded badges, points users at
the gear icon for settings and the project dropdown for filtering, and
plugs /mem-search for recall — replacing the old two-line "ask:" prompts.
Source filter tabs (Claude/Codex/etc.) are removed from the header.
Filtering by AI provider was nonsense from a user POV; the project
dropdown is the only header filter now. Source tracking is also
stripped from useSSE, usePagination, App state, and CSS.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(viewer): keep welcome card in feed column, swap rows for 3 squares
Two visible problems in the previous design: the card stretched
edge-to-edge while feed cards sit in a centered 650px column, and
the body was a stack of long horizontal rows that scanned line-by-line.
Both fixed: Feed now accepts a pinnedTop slot so the welcome card
renders inside the same .feed-content column as observation cards.
Body is now a 3-column grid of square feature blocks — Live feed,
Tune it, Recall it — each with a custom inline SVG illustration
(stacked cards with color-coded stripes, gear+sliders, magnifier
over cards). Old text-row sections (welcome-card-types,
welcome-card-tips, welcome-card-section, welcome-card-tip-icon)
are removed. Squares stack to one column under 600px.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat(viewer): convert welcome card to glassy modal with stylized logo
Card now opens as a centered modal with a frosted/glass backdrop
(blur + saturate) so it doubles as a proper help dialog when reopened
from the header's question-mark button. Removed the observation count,
project count, and "since" date — those don't make sense for a
first-launch surface and felt out of place in a help context.
Header art swapped from the small webp logomark to the new
high-resolution sun/sunburst PNG (claude-mem-logo-stylized.png),
shipped as a checked-in asset in src/ui and plugin/ui.
Bigger throughout: 28px h2, 16px tagline, 88px illustrations,
26px feature padding, 1:1 aspect-ratio squares. Backdrop click and
Esc both close. Mobile collapses the grid to one column and drops
the aspect-ratio constraint.
Reverted the unused pinnedTop slot on Feed.tsx since the welcome
card is now a true overlay rather than an in-feed pinned card.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(viewer): make welcome modal actually glassy
Previous version had a 55%-opacity black backdrop that almost fully
blocked the underlying UI — the "glass" was just a dark plate.
Now the backdrop is fully transparent (no darkening at all), the
panel itself drops to 55% bg-card opacity with its existing
backdrop-filter blur(28px) saturate(170%), and the feature squares
drop to 35% bg-tertiary so they layer as glass-on-glass over the
already-blurred panel. The header and feed below now read clearly
through the modal's frosted blur.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(viewer): bulletproof square features via padding-bottom + clamp() fluid type
Squares were rendering taller than wide because aspect-ratio is treated
as a minimum — content can push the box past 1:1. Switched to the
classic padding-bottom: 100% trick: percentage padding resolves against
the parent's width, so the box is ALWAYS W × W regardless of content.
Inner content sits in an absolutely-positioned flex column that can't
push the shell taller.
Whole modal is now desktop-first and fluid via clamp() — no media-query
stair-steps for type, padding, gaps, border-radius, illustration size,
or modal width. Single mobile breakpoint at <600px collapses the grid
to one column and reverts the padding-bottom trick so each feature can
grow to natural content height.
Tightened the three feature descriptions so they fit comfortably inside
the square at the desktop size.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* style(viewer): 15% black overlay + heavier modal shadow for elevation
Backdrop goes from transparent to rgba(0,0,0,0.15) — just enough
darkening to push the modal visually forward without burying the
underlying UI. Modal shadow stacked: 40px/120px ambient + 16px/48px
contact, both deeper, plus the existing inset 1px highlight.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(build): clear pending_messages queue on build-and-sync
Rewrites scripts/clear-failed-queue.ts to talk directly to SQLite via
bun:sqlite — the previous HTTP endpoints (/api/pending-queue/*) were
removed during the queue engine rewrite, so the script was orphaned.
Wires `npm run queue:clear` into `build-and-sync` so each rebuild
starts with a clean queue.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor(worker): collapse parser to binary valid/invalid + clearPendingForSession model
- Parser: { valid: true, observations, summary } | { valid: false } — drops kind/skipped enum dispatch
- ResponseProcessor: two branches only (parseable → store + clearPendingForSession; else → no-op)
- Drop processingMessageIds + per-message claim/confirm/markFailed lifecycle across 3 providers
- PendingMessageStore: 226 → 140 lines; remove markFailed/transitionMessagesTo/confirmProcessed/clearFailedOlderThan/getAllPending/peekPendingTypes... wait keep peekPendingTypes
- Schema migration v31+v32: drop retry_count, failed_at_epoch, completed_at_epoch, worker_pid columns
- SessionQueueProcessor: delete two 1s recovery sleeps (let iterator end on error)
- Server.ts/SettingsRoutes.ts: replace four magic-number setTimeout exit-flush patterns with flushResponseThen helper
- GeneratorExitHandler: 183 → 117 lines (drain in-flight loop gone)
Net: -181 lines. No more silent data loss via maxRetries=3.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(pr-2255): address review comments batch 1
- install.ts: needsMarketplace true when claude-code selected (P1, was no-op)
- install.ts: throw on invalid --model so CLI exits non-zero
- install.ts: skip worker health checks + adapt next-step copy when --no-auto-start
- install.ts: repair regenerates plugin cache when missing
- index.ts: readFlag rejects missing/flag-shaped values
- index.ts: route flag-first invocations (e.g. `--provider claude`) to install
- banner.ts: fail-open if frame payload decode throws
- SearchRoutes.ts: 5s TTL cache for settings reads on hot hook path (P2)
- detect-error-handling-antipatterns.ts: trailing-brace strip whitespace-tolerant
- investigate-timestamps.ts: compute Dec 2025 epochs at runtime (was Dec 2024)
- regenerate-claude-md.ts: include workingDir in fallback walker so root is covered
- sync-marketplace.cjs: parseWorkerPort validates 1..65535 before http.request
- sync-to-marketplace.sh: resolve SOURCE_DIR from script location, not cwd
- Dockerfile.test-installer: bash --login sources .bashrc via .bash_profile
- docs/configuration.mdx: drop nonexistent .worker.port file refs, use settings.json
- docs/architecture-overview.md: dynamic port + queue model after parser collapse
- docs/architecture/worker-service.mdx: dynamic port example + drop port-file claim
- docs/platform-integration.mdx: WORKER_BASE_URL pattern, drop hardcoded 37777
- install/public/install.sh: Node 20 floor (was 18) to match docs
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(pr-2255): reset claimed messages to pending on early-return paths
ResponseProcessor returns early in two cases:
- parser invalid (unparseable response)
- memorySessionId not yet captured
Both paths previously left the just-claimed message in `status='processing'`,
which counts toward `getPendingCount`. The generator-exit handler then sees
`pendingCount > 0` and respawns the generator, looping until the restart
guard trips and `clearPendingForSession` deletes the message — silent data
loss.
Calling `resetProcessingToPending` on these paths lets the next generator
pass re-claim the message and try again, instead of burning the restart
budget on no-op respawns.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(pr-2255): swebench fallback row + troubleshooting port path
- evals/swebench/run-batch.py: append fallback prediction row when
orchestrator future raises, preserving "never drop an instance" guarantee
- docs/troubleshooting.mdx: drop nonexistent .worker.port / worker.port file
references; use settings.json + /api/health for port discovery
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(pr-2255): memoize per-project observation count for welcome-hint hot path
handleContextInject runs on every PostToolUse hook (after every Read/Edit).
The welcome-hint block ran a COUNT(*) on observations for every call once
CLAUDE_MEM_WELCOME_HINT_ENABLED was true. Observation counts are
monotonically increasing — once a project has any observations it always
will — so cache the positive result in a Set and skip the COUNT(*) on
subsequent requests.
Combined with the 5s settings TTL added earlier, the steady-state cost on
the hook hot path drops to a Set lookup.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(pr-2255): use clearProcessingForSession on AI-success path
clearPendingForSession deletes ALL rows for the session. On the success
path of processAgentResponse, that's wrong: messages that arrived as
'pending' during the (1-5s) AI response latency get deleted along with
the 'processing' row we just consumed. In a hook burst (three quick
PostToolUse hooks), B and C land while A is in flight; A's success then
nukes B and C — silent data loss.
Add a status-scoped clearProcessingForSession to PendingMessageStore +
SessionManager, and use it in ResponseProcessor's success path. The
unconditional clearPendingForSession remains correct in
GeneratorExitHandler for hard-stop / restart-guard-trip paths.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Revert "fix(pr-2255): use clearProcessingForSession on AI-success path"
This reverts commit a08995299c30cbad36bddc3e5bddda7af8604b35.
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* 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>
Alex Newman