fix: Revert memory MCP to per-session stdio (fix Cowork regression)

.claude-plugin/plugin.json

@@ -1,19 +1,18 @@
 {
   "name": "workbench-core",
   "description": "Core infrastructure for Claude Code: persistent agent identity, session lifecycle hooks, operational memory, and meta skills. Provides a customizable framework where users configure the agent name, memory path, and persona files.",
-  "version": "0.12.0",
+  "version": "0.13.0",
   "author": {
     "name": "Mike Bronner",
     "url": "https://github.com/mike-bronner"
   },
   "repository": "https://github.com/mike-bronner/core",
   "mcpServers": {
     "memory": {
-      "type": "http",
-      "url": "http://127.0.0.1:${WORKBENCH_MEMORY_PORT:-8765}/mcp",
-      "headers": {
-        "Authorization": "Bearer ${WORKBENCH_MEMORY_TOKEN}"
-      }
+      "command": "bash",
+      "args": [
+        "${CLAUDE_PLUGIN_ROOT}/hooks/mcp-memory.sh"
+      ]
     }
   }
 }

hooks/hooks.json

@@ -15,10 +15,6 @@
       {
         "matcher": "",
         "hooks": [
-          {
-            "type": "command",
-            "command": "bash \"${CLAUDE_PLUGIN_ROOT}/hooks/memory-server-up.sh\""
-          },
           {
             "type": "command",
             "command": "bash \"${CLAUDE_PLUGIN_ROOT}/hooks/session-warmup.sh\""

hooks/lib/memory-vacuum.sh

@@ -91,3 +91,62 @@ memory_vacuum() {
 # Default logger: a plain stderr line. Callers with their own _log (the
 # supervisor) pass it as $2 so vacuum output joins the server log.
 _memory_vacuum_log() { echo "memory-vacuum: $*" >&2; }
+
+# memory_vacuum_locked: race-safe wrapper around memory_vacuum for the
+# per-session stdio launcher (hooks/mcp-memory.sh).
+#
+# WHY a lock here but not in the supervisor: the shared-server supervisor holds
+# the spawn lock and VACUUMs in a guaranteed "no server alive" window, so it
+# calls memory_vacuum directly. The stdio launcher has no such window — it runs
+# once per session and sibling sessions' servers may be live. This wrapper adds
+# a NON-BLOCKING atomic-mkdir lock so that among concurrently starting launchers
+# exactly one attempts the VACUUM and the rest skip immediately (one session
+# VACUUMs, others skip if held) rather than piling onto the same index.
+#
+# The lock only dedups concurrent *launchers*. A VACUUM that still contends with
+# a live sibling server's writes is handled one layer down: memory_vacuum sets a
+# SQLite busy timeout and treats a busy/locked index as skip-and-continue, so it
+# degrades safely — no corruption, no blocking. That SQLite-level busy handler is
+# the true safety net; this mkdir lock is a cheap best-effort dedup on top of it.
+# Skipping when a sibling server holds the index is the honest multi-session
+# tradeoff the Mac knowingly re-accepts by choosing per-session stdio.
+#
+# Staleness is PID-liveness, never wall-clock (mirrors memory-server-up.sh's
+# lock): a crashed launcher's lock is stolen once when its pid is dead. Because
+# the SQLite busy handler already prevents corruption, a lost steal-race just
+# means a redundant skip — so this deliberately omits the heavier lock-generation
+# nonce the spawn lock carries. Never fails the caller; releases the lock inline
+# (no EXIT trap — the launcher execs the server after this, where a trap would
+# fire in the wrong process).
+#
+# Args: $1 = index sqlite path (as memory_vacuum). $2 = optional logger name.
+memory_vacuum_locked() {
+  local index_path="$1"
+  local logger="${2:-_memory_vacuum_log}"
+  local lock_dir="${index_path%/*}/vacuum.lock"
+  local pid_file="$lock_dir/pid"
+
+  # Non-blocking claim: mkdir is atomic, so exactly one concurrent launcher wins.
+  if ! mkdir "$lock_dir" 2>/dev/null; then
+    local holder=""
+    [ -f "$pid_file" ] && holder="$(cat "$pid_file" 2>/dev/null)"
+    if [ -n "$holder" ] && kill -0 "$holder" 2>/dev/null; then
+      "$logger" "vacuum: lock held by live pid $holder; skipping"
+      return 0
+    fi
+    # Stale lock (holder dead or never stamped) — steal once and retry the claim.
+    "$logger" "vacuum: stealing stale lock (holder '${holder:-none}' not alive)"
+    rm -rf "$lock_dir" 2>/dev/null || true
+    if ! mkdir "$lock_dir" 2>/dev/null; then
+      "$logger" "vacuum: lost re-claim race; another launcher owns the lock; skipping"
+      return 0
+    fi
+  fi
+
+  # We hold the lock. Stamp our pid for the liveness check above, run the gated
+  # VACUUM, then always release (rm -rf, since the dir holds pid).
+  echo "$$" > "$pid_file" 2>/dev/null || true
+  memory_vacuum "$index_path" "$logger"
+  rm -rf "$lock_dir" 2>/dev/null || true
+  return 0
+}

hooks/mcp-memory.sh

@@ -41,13 +41,23 @@ HOOKS_DIR="${HOOKS_DIR:-$SCRIPT_DIR}"
 . "$HOOKS_DIR/lib/memory-env.sh"
 memory_load_env
 
-# --- Index reclamation moved out of this launcher ----------------------------
-# The gated full VACUUM used to run here on every per-session connect, which
-# could race a sibling session holding the same index. Under the shared-server
-# model index maintenance is the lazy-start supervisor's job: it runs the gated
-# VACUUM (hooks/lib/memory-vacuum.sh) at a confirmed cold start, inside the spawn
-# lock, with no server alive — so there is no writer to race. This stdio launcher
-# is the escape hatch / in-flight-old-session path; it no longer VACUUMs.
+# --- Out-of-band index reclamation: race-guarded gated VACUUM ----------------
+# Per-session stdio means N sessions can start concurrently, and a sibling
+# session's server may already hold the index. memory_vacuum_locked serializes
+# the attempt with a non-blocking mkdir lock — one launcher VACUUMs, the rest
+# skip immediately — and, one layer down, memory_vacuum's SQLite busy timeout
+# makes a VACUUM that still contends with a live sibling writer skip safely
+# rather than block or corrupt. Gated (freelist threshold) + cooled down
+# (once/day) inside the lib, so most boots skip with near-zero added latency.
+# Reuses the same lib the shared-server supervisor uses — no reinvention.
+#
+# Hard rules carry over from the old inline VACUUM: this step must NEVER fail the
+# launcher and must NEVER touch stdout (the MCP stdio channel). memory_vacuum
+# routes all output through the _log function (stderr) passed here and treats
+# every error (busy/locked, missing sqlite3, absent file) as skip-and-continue.
+# shellcheck source=hooks/lib/memory-vacuum.sh
+. "$HOOKS_DIR/lib/memory-vacuum.sh"
+memory_vacuum_locked "$MARKDOWN_VAULT_MCP_INDEX_PATH" _log
 # -----------------------------------------------------------------------------
 
 # --- Resolve/install the server binary via the shared install library --------

hooks/session-warmup.sh

@@ -25,16 +25,16 @@ SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
 HOOKS_DIR="${CLAUDE_PLUGIN_ROOT:+$CLAUDE_PLUGIN_ROOT/hooks}"
 HOOKS_DIR="${HOOKS_DIR:-$SCRIPT_DIR}"
 
-# Memory path/cache/mcp-name/port come from the shared resolver (precedence:
-# WORKBENCH_* override → config.json → default). It also exports the full
-# MARKDOWN_VAULT_MCP_* set, harmless here. memory_load_env sets MEMORY_PATH,
-# CACHE_PATH, MCP_NAME, MEMORY_PORT.
+# Memory path/cache come from the shared resolver (precedence: WORKBENCH_*
+# override → config.json → default). It also exports the full MARKDOWN_VAULT_MCP_*
+# set, harmless here. memory_load_env sets MEMORY_PATH and CACHE_PATH, both used
+# below. memory-probe.sh is deliberately NOT sourced any more: the shared-server
+# health check was disabled when the memory transport reverted to per-session
+# stdio (v0.13.0) — there is no external server to probe. See the breadcrumb
+# where that block used to live, further down.
 # shellcheck source=hooks/lib/memory-env.sh
 . "$HOOKS_DIR/lib/memory-env.sh"
-# shellcheck source=hooks/lib/memory-probe.sh
-. "$HOOKS_DIR/lib/memory-probe.sh"
 memory_load_env
-MCP_SERVER_NAME="$MCP_NAME"
 
 # Config resolution for warmup-only fields (agent_name, identity_files).
 # Prefer the current data dir; fall back to the pre-rename location so users
@@ -398,41 +398,20 @@ if [ "$SOURCE" = "startup" ]; then
   find "$CACHE_PATH" -name "summary-writer-*.log" -delete 2>/dev/null
 fi
 
-# ──────────── Memory server health check (startup only) ────────────
-# The memory server is now a shared HTTP server, lazy-started by the
-# memory-server-up hook that runs just before this warmup. Probe its actual
-# health (identity-checked) rather than guessing from the index file's presence.
-# Only surface a notice when something is wrong — a healthy/coming-up server is
-# the common case and needs no words.
-if [ "$SOURCE" = "startup" ]; then
-  case "$(memory_probe)" in
-    UP|BUILDING)
-      : # serving (BUILDING = bound, index still building, search available) — quiet.
-      ;;
-    PORT_DRIFT)
-      printf '## ⚠ Memory server port drift\n\n'
-      printf 'The running memory server bound a different port than configured (port `%s`).\n' "$MEMORY_PORT"
-      printf 'This usually means `WORKBENCH_MEMORY_PORT` in `~/.claude/settings.json` and the\n'
-      printf 'recorded `%s/server.port` disagree. Reconcile them (or run `/workbench-core:memory-status`), then restart Claude Code.\n\n' "$CACHE_PATH"
-      ;;
-    DOWN_FOREIGN)
-      printf '## ⚠ Memory server port conflict\n\n'
-      printf 'Another process is listening on memory port `%s` that is not the `%s` vault.\n' "$MEMORY_PORT" "$MCP_SERVER_NAME"
-      printf 'The shared server was not started to avoid a conflict. Free the port or set a different\n'
-      printf '`WORKBENCH_MEMORY_PORT`, then restart. See `/workbench-core:memory-status`.\n\n'
-      ;;
-    DOWN_FAILED)
-      printf '## ⚠ Memory server failed to start\n\n'
-      printf 'The last attempt to start the shared memory server failed (see `%s/server.log`).\n' "$CACHE_PATH"
-      printf 'Memory search and write will fail until it comes up. Run `/workbench-core:memory-status` to diagnose.\n\n'
-      ;;
-    *)  # DOWN_NONE — not up yet; the up-hook just kicked a spawn that binds in ~2s.
-      printf '## ℹ Memory server starting\n\n'
-      printf 'The shared memory server is starting in the background (binds in ~2s; the client retries the connection).\n'
-      printf 'If memory tools are unavailable this turn, they should work shortly — or next session.\n\n'
-      ;;
-  esac
-fi
+# ──────────── Memory server health check — disabled (per-session stdio) ────────
+# A shared-HTTP health probe lived here through v0.12: it ran memory_probe and
+# surfaced port-drift / conflict / failed / starting notices for the lazy-started
+# shared server. It was removed in v0.13.0 when the transport reverted to a
+# per-session stdio server: stdio spawns the server in-process per session, so
+# there is NO external listener to probe — the probe returned DOWN_NONE every
+# startup and printed a phantom "Memory server starting" notice. The MCP host's
+# own connect error is the real signal for a broken stdio launcher now.
+#
+# To RE-ENABLE the shared HTTP server, restore this block and the memory-probe.sh
+# source near the top of this file (both are in git history), re-add the
+# memory-server-up.sh SessionStart hook in hooks/hooks.json, and switch
+# plugin.json's memory MCP back to the http transport. See README, section
+# "Memory server transport — re-enabling the shared HTTP server (optional)".
 
 # ──────────── Recall-hook liveness check (startup only) ────────────
 # memory-recall.sh stamps last-attempt on every substantive prompt. A stamp

hooks/test-memory-status.sh

@@ -0,0 +1,67 @@
+#!/bin/bash
+# Tests for scripts/memory-status.sh — the per-session stdio status report.
+# Run directly: ./test-memory-status.sh
+# Drives the script in a sandbox (fake vault/cache, no real config) and asserts
+# it reports the stdio transport, resolved paths, and launcher presence, carries
+# no leftover HTTP/port/token vocabulary, and always exits 0. No server, no net.
+
+set -u
+HOOKS="$(cd "$(dirname "$0")" && pwd)"
+REPO_ROOT="$(cd "$HOOKS/.." && pwd)"
+STATUS="$REPO_ROOT/scripts/memory-status.sh"
+PASS=0
+FAIL=0
+
+SANDBOX=$(mktemp -d)
+trap 'rm -rf "$SANDBOX"' EXIT
+mkdir -p "$SANDBOX/vault" "$SANDBOX/cache"
+
+run_status() {
+  env WORKBENCH_MEMORY_PATH="$SANDBOX/vault" \
+      WORKBENCH_MEMORY_CACHE="$SANDBOX/cache" \
+      WORKBENCH_MCP_SERVER_NAME="test-vault" \
+      WORKBENCH_CONFIG_FILE="$SANDBOX/nope.json" \
+      CLAUDE_PLUGIN_ROOT="$REPO_ROOT" \
+      bash "$STATUS" "$@" 2>&1
+}
+
+assert_contains() {
+  local desc="$1" output="$2" needle="$3"
+  if printf '%s' "$output" | grep -qF "$needle"; then
+    PASS=$((PASS + 1)); echo "  ✅ $desc"
+  else
+    FAIL=$((FAIL + 1)); echo "  ❌ $desc — expected: $needle"
+  fi
+}
+
+assert_missing() {
+  local desc="$1" output="$2" needle="$3"
+  if printf '%s' "$output" | grep -qiF "$needle"; then
+    FAIL=$((FAIL + 1)); echo "  ❌ $desc — should NOT contain: $needle"
+  else
+    PASS=$((PASS + 1)); echo "  ✅ $desc"
+  fi
+}
+
+echo "status: reports the stdio transport and resolved facts, exits 0:"
+OUT=$(run_status); RC=$?
+assert_contains "reports per-session stdio transport"         "$OUT" "per-session stdio"
+assert_contains "reports the vault path"                      "$OUT" "$SANDBOX/vault"
+assert_contains "reports the cache path"                      "$OUT" "$SANDBOX/cache"
+assert_contains "reports the configured server name"          "$OUT" "test-vault"
+assert_contains "reports the launcher present"                "$OUT" "mcp-memory.sh (present)"
+assert_contains "says there is no shared server to start/stop" "$OUT" "no shared server to start or stop"
+[ "$RC" -eq 0 ] && { PASS=$((PASS+1)); echo "  ✅ exits 0"; } || { FAIL=$((FAIL+1)); echo "  ❌ non-zero exit ($RC)"; }
+
+echo "no leftover shared-HTTP vocabulary in the report:"
+assert_missing "no bearer-token wording" "$OUT" "bearer"
+assert_missing "no hardcoded 8765 port"  "$OUT" "8765"
+
+echo "start/stop: prints the not-applicable note and still shows status:"
+OUT=$(run_status stop)
+assert_contains "explains stop is N/A under stdio"  "$OUT" "does not apply to the per-session stdio server"
+assert_contains "still shows status after the note" "$OUT" "per-session stdio"
+
+echo
+echo "$PASS passed, $FAIL failed"
+[ "$FAIL" -eq 0 ]

hooks/test-memory-vacuum.sh

@@ -59,6 +59,28 @@ assert_no_file() {
   else FAIL=$((FAIL + 1)); echo "  ❌ $desc — should be absent: $path"; fi
 }
 
+assert_no_dir() {
+  local desc="$1" path="$2"
+  if [ ! -d "$path" ]; then PASS=$((PASS + 1)); echo "  ✅ $desc"
+  else FAIL=$((FAIL + 1)); echo "  ❌ $desc — dir should be absent: $path"; fi
+}
+
+# run_vacuum_locked <index> [env...] — call the race-guarded wrapper the stdio
+# launcher uses, capturing its stderr log.
+run_vacuum_locked() {
+  local index="$1"; shift
+  env "$@" bash -c '. "'"$LIB"'"; memory_vacuum_locked "'"$index"'"' 2>&1
+}
+
+assert_no_run() {
+  local desc="$1" output="$2"
+  if printf '%s' "$output" | grep -qF "running VACUUM"; then
+    FAIL=$((FAIL + 1)); echo "  ❌ $desc — VACUUM ran but should have been skipped"
+  else
+    PASS=$((PASS + 1)); echo "  ✅ $desc"
+  fi
+}
+
 echo "freelist over threshold → VACUUM runs, file shrinks, stamp written:"
 IDX="$SANDBOX/over/vault-index.sqlite"; mkdir -p "$SANDBOX/over"
 build_index "$IDX"
@@ -113,6 +135,38 @@ RC=$?
 assert_contains "logs sqlite3-missing skip" "$OUT" "sqlite3 not on PATH"
 [ "$RC" -eq 0 ] && { PASS=$((PASS+1)); echo "  ✅ returns 0 when sqlite3 absent"; } || { FAIL=$((FAIL+1)); echo "  ❌ non-zero when sqlite3 absent"; }
 
+echo "locked wrapper: free lock → VACUUM runs and the lock is released:"
+IDX="$SANDBOX/lockfree/vault-index.sqlite"; mkdir -p "$SANDBOX/lockfree"
+build_index "$IDX"
+OUT=$(run_vacuum_locked "$IDX" WORKBENCH_MEMORY_VACUUM_THRESHOLD_MB=1)
+assert_contains "runs VACUUM when the lock is free" "$OUT" "running VACUUM"
+assert_no_dir   "lock dir released after the run"    "$SANDBOX/lockfree/vacuum.lock"
+
+echo "locked wrapper: lock held by a LIVE pid → skip, holder untouched:"
+IDX="$SANDBOX/lockheld/vault-index.sqlite"; mkdir -p "$SANDBOX/lockheld"
+build_index "$IDX"
+mkdir -p "$SANDBOX/lockheld/vacuum.lock"
+sleep 60 & HOLDER=$!
+disown 2>/dev/null || true   # silence the job-control "Terminated" notice on kill
+echo "$HOLDER" > "$SANDBOX/lockheld/vacuum.lock/pid"
+OUT=$(run_vacuum_locked "$IDX" WORKBENCH_MEMORY_VACUUM_THRESHOLD_MB=1)
+kill "$HOLDER" 2>/dev/null
+assert_contains "logs the live-holder skip" "$OUT" "held by live pid"
+assert_no_run   "does not VACUUM under a live lock" "$OUT"
+assert_file     "live holder's lock left intact" "$SANDBOX/lockheld/vacuum.lock/pid"
+
+echo "locked wrapper: STALE lock (dead pid) → stolen, VACUUM runs, lock released:"
+IDX="$SANDBOX/lockstale/vault-index.sqlite"; mkdir -p "$SANDBOX/lockstale"
+build_index "$IDX"
+mkdir -p "$SANDBOX/lockstale/vacuum.lock"
+# A guaranteed-dead pid: spawn a trivial child, then reap it so kill -0 fails.
+sh -c 'exit 0' & DEAD=$!; wait "$DEAD" 2>/dev/null
+echo "$DEAD" > "$SANDBOX/lockstale/vacuum.lock/pid"
+OUT=$(run_vacuum_locked "$IDX" WORKBENCH_MEMORY_VACUUM_THRESHOLD_MB=1)
+assert_contains "steals the stale lock"          "$OUT" "stealing stale lock"
+assert_contains "runs VACUUM after the steal"    "$OUT" "running VACUUM"
+assert_no_dir   "lock released after stolen run" "$SANDBOX/lockstale/vacuum.lock"
+
 echo
 echo "$PASS passed, $FAIL failed"
 [ "$FAIL" -eq 0 ]