diff --git a/.github/workflows/embedding-runtime-test.yml b/.github/workflows/embedding-runtime-test.yml new file mode 100644 index 0000000..fd0a496 --- /dev/null +++ b/.github/workflows/embedding-runtime-test.yml @@ -0,0 +1,31 @@ +name: embedding-runtime-test + +on: + pull_request: + paths: + - "checks/embedding-runtime.sh" + - "tests/embedding-runtime-test.sh" + - ".github/workflows/embedding-runtime-test.yml" + push: + branches: [main] + paths: + - "checks/embedding-runtime.sh" + - "tests/embedding-runtime-test.sh" + +permissions: + contents: read + +concurrency: + group: embedding-runtime-test-${{ github.ref }} + cancel-in-progress: true + +jobs: + test: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v6 + + # `curl` and `ollama` are stubbed through a PATH shim, so this needs no + # network and no Ollama on the runner. + - name: Run the embedding-runtime check suite + run: bash tests/embedding-runtime-test.sh diff --git a/CLAUDE.md b/CLAUDE.md index a768b46..6ec9198 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -13,6 +13,7 @@ The consequence that matters most: **nothing here executes from the repo.** Edit | Task | Command | |---|---| | Run the hook test suite | `bash tests/kb-gate-test.sh` | +| Run the embedding-check suite | `bash tests/embedding-runtime-test.sh` | | Verify the SYNC blocks agree | the snippet below (full version at the bottom of `SYNC-BLOCKS.md`) | | Check the manifest parses | `python3 -c "import yaml;yaml.safe_load(open('techpack.yaml'))"` | | Install a change locally | `mcs sync --global`, or `mcs sync` inside a project | @@ -46,6 +47,13 @@ Two harness details are load-bearing rather than incidental: **That dispatcher deliberately omits `set -e` and `set -u`**, unlike `sync-memories.sh` which uses `set -uo pipefail`. Its file header explains why and lists rules that are load-bearing: fail open, never `exit 2`, never call `docs-mcp-server` (too slow for `PreToolUse`), log every evaluation. Read that header before editing it. +**The Ollama components gate on the endpoint, not on Ollama.** All three (`ollama`, `ollama-service`, `ollama-nomic-embed`) probe an OpenAI-compatible endpoint on `localhost:11434` — `/v1/models` for reachability, `/v1/embeddings` for the model — so a machine already served by llama.cpp or LM Studio installs nothing. Never reintroduce an `/api/*` check: those are Ollama-proprietary and 404 elsewhere, which is what used to install Ollama.app over a working provider. Read the header of `checks/embedding-runtime.sh` before editing any of it. Four things that are not visible from the checks themselves: + +- **It relies on `isAlreadyInstalled` being ANY-pass** across a component's `doctorChecks`. Load-bearing, and mcs has tightened that logic before for causing silent skips — if the installer starts firing on healthy machines, look there first. +- **`displayName` is provider-neutral on purpose**, because mcs prints it in `" already installed, skipping"` and in the component picker. The `id`s stay Ollama-shaped: nothing displays them, and four dependency lists reference them. +- **The endpoint URL is duplicated** across `techpack.yaml`, `checks/embedding-runtime.sh` and `hooks/sync-memories.sh`, with "keep in step" comments in each. Making it configurable needs an mcs change first — prompt values reach neither doctor-check args nor `shell:` commands. +- **`mcs pack validate` warns that `checks/embedding-runtime.sh` is unreferenced. That is expected.** Doctor-check script paths are absent from `ExternalPackManifest.referencedPaths`. Do **not** silence it by adding `checks/` to `ignore:` — mcs accepts that entry silently, and it would suppress update notifications for a file carrying behaviour. The fix belongs in mcs. + **Project-root and library derivation must match across two hooks.** `sync-memories.sh` and `resolve_paths()` in `kb-gate.sh` both resolve git toplevel → `CLAUDE_PROJECT_DIR` → `$PWD`, and derive the library name as the root directory's basename. kb-gate quotes that name back to Claude, and it has to be the one sync-memories indexed. Both files carry "keep in sync" comments. **Three text blocks must stay byte-identical across three files.** `capture-rules`, `strip-the-anchors`, and `applies-to` appear in both `SKILL.md`s and in `SYNC-BLOCKS.md`, enforced by `.github/workflows/sync-blocks.yml`. Two rules when touching them: diff --git a/README.md b/README.md index ea1f929..74995b1 100644 --- a/README.md +++ b/README.md @@ -18,10 +18,22 @@ mcs sync --global # 3. install globally (~/.claude) mcs doctor # 4. verify everything is healthy ``` -**Prerequisites:** macOS, [Claude Code](https://docs.anthropic.com/en/docs/claude-code), and [Ollama](https://ollama.com) (local embeddings runtime). `mcs` installs the rest (Node, `gh`, `jq`) automatically. +**Prerequisites:** macOS, [Claude Code](https://docs.anthropic.com/en/docs/claude-code), and an embeddings runtime. `mcs` installs the rest (Node, `gh`, `jq`) automatically. If nothing is already serving embeddings, the pack installs [Ollama](https://ollama.com) and pulls `nomic-embed-text` for you. Global is the recommended scope — this pack has no per-project config, so installing once makes memory available in every project automatically. To scope it to a single repo instead, run `mcs sync` from inside that repo. +### Already run your own embeddings server? + +The pack needs one thing: an OpenAI-compatible `/v1/embeddings` on `localhost:11434` that returns a vector for `nomic-embed-text`. It does not care what serves it. If llama.cpp, LM Studio, or vLLM is already answering there, nothing is installed and nothing is pulled — no Ollama, no second server competing for the port. + +Three things to know if you bring your own: + +- **Alias the model name.** llama.cpp's router matches the request's `model` field against a section name or alias, so `nomic-embed-text` won't resolve on its own. Add `alias = nomic-embed-text` to that model's section in your preset file. +- **The endpoint has to actually embed.** `/v1/embeddings` needs a model that supports embeddings and a pooling type other than `none`. A server that's up and answering `/v1/models` still fails the model check if it can only chat — which is the intended diagnosis, not a bug. +- **Wipe the store when you switch providers.** `docs-mcp-server` pins the embedding model *name* when the store is first created, so swapping the thing behind that name changes your vectors while every check still passes. Run `rm -rf ~/Library/Application\ Support/docs-mcp-server`; the session-start hook re-indexes. + +Non-default ports and remote hosts aren't configurable yet — the endpoint is currently fixed at `localhost:11434`. + --- ## How it works @@ -87,8 +99,10 @@ Memories come in two flavors, both stored as version-controlled, human-readable memory/ ├── techpack.yaml # Manifest — defines all components ├── config/settings.json # Disables built-in auto-memory +├── checks/ +│ └── embedding-runtime.sh # Doctor check: is any embedding runtime available? ├── hooks/ -│ ├── sync-memories.sh # Ollama health + memory indexing/reindexing +│ ├── sync-memories.sh # Endpoint health + memory indexing/reindexing │ ├── continuous-learning-activator.sh # Knowledge extraction reminder │ └── kb-gate.sh # Keeps KB lookups ahead of delegated discovery ├── skills/ diff --git a/checks/embedding-runtime.sh b/checks/embedding-runtime.sh new file mode 100755 index 0000000..bdd42bf --- /dev/null +++ b/checks/embedding-runtime.sh @@ -0,0 +1,40 @@ +#!/usr/bin/env bash +# Doctor check: is an embedding runtime available at all? +# +# Provider-neutral by design. The pack consumes an OpenAI-compatible +# /v1/embeddings endpoint and does not care who serves it — Ollama, llama.cpp, +# LM Studio, vLLM, a remote host. Ollama is only the fallback we install when +# nothing else answers. +# +# Two conditions must OR into a single doctor line, which is why this is a +# script and not two `commandExists` checks: `isOptional` is silently ignored by +# commandExists (only hookEventExists honours it), so the second check would +# show a hard red on a machine that deliberately runs something else. +# +# Exit codes: 0=pass 1=fail 2=warn 3=skip +# +# No `set -e`: the first curl is expected to fail on the fallback path. +set -uo pipefail + +# Hardcoded on purpose. An env override here would steer only this check while +# the probes in techpack.yaml and hooks/sync-memories.sh stayed on localhost, so +# setting it would let this pass against a remote endpoint, skip the Ollama +# install, and then have ollama-service's localhost check fail and start a local +# Ollama anyway — the port fight this whole arrangement exists to avoid. +# Keep this URL in step with those two files. +BASE="http://localhost:11434/v1" + +if curl -sf --max-time 3 "$BASE/models" >/dev/null 2>&1; then + echo "OpenAI-compatible server responding at $BASE" + exit 0 +fi + +# Installed but not serving. This arm is load-bearing: without it a stopped +# Ollama would fail the check and re-run the interactive installer on every sync. +if command -v ollama >/dev/null 2>&1; then + echo "Ollama installed (not currently serving)" + exit 0 +fi + +echo "no embedding runtime found at $BASE" +exit 1 diff --git a/hooks/sync-memories.sh b/hooks/sync-memories.sh index a406be4..a44606b 100755 --- a/hooks/sync-memories.sh +++ b/hooks/sync-memories.sh @@ -24,8 +24,20 @@ TIMESTAMP_FILE="$project_root/.claude/.memories-last-indexed" # Exit early if no memories directory [ -d "$MEMORIES_DIR" ] || exit 0 -# Exit early if Ollama is not running -curl -s --max-time 2 http://localhost:11434/api/tags >/dev/null 2>&1 || exit 0 +# Exit early unless some OpenAI-compatible server is reachable. Two fixes over +# the previous `curl -s .../api/tags`: +# -f — without it curl exits 0 on an HTTP 404, so any server that +# merely accepted the connection passed the gate. +# /v1/models — /api/tags is Ollama-proprietary and 404s on every other +# provider. This one is answered by all of them. +# Deliberately the cheap probe, not an embed round-trip: this runs on EVERY +# prompt (registered on UserPromptSubmit as well as SessionStart), and almost +# every one of those exits at the staleness check just below. An embed request +# here would mean a discarded forward pass per prompt, plus a reset keep_alive +# timer pinning the model in memory. Whether the endpoint can actually embed is +# proven by the indexer itself failing below — which no longer hides. +# Keep this URL in step with the probes in techpack.yaml. +curl -sf --max-time 3 http://localhost:11434/v1/models >/dev/null 2>&1 || exit 0 # --- Staleness check --- # If timestamp file exists and nothing changed, nothing to do. @@ -52,14 +64,21 @@ export DOCS_MCP_EMBEDDING_MODEL="openai:nomic-embed-text" existing_url=$(docs-mcp-server list 2>/dev/null \ | jq -r --arg name "$repo_name" '.[] | select(.name == $name) | .versions[0].sourceUrl // empty') +# Advance the timestamp only on success, and keep the output of a failure around. +# +# Touching unconditionally made a failure self-concealing: the staleness check +# above would see a fresh timestamp and skip every later run, so one failed index +# meant the KB was silently never indexed again. Gating the touch fixes that, but +# on its own it trades silence for a full re-attempt every prompt with still no +# way to see why — hence the log. It is removed on success, so its presence is +# itself the signal that indexing is broken, and it holds the reason. +ERROR_LOG="$project_root/.claude/.memories-index.log" + if [ "$existing_url" = "file://$MEMORIES_DIR" ]; then docs-mcp-server refresh "$repo_name" \ - --silent >/dev/null 2>&1 + --silent >"$ERROR_LOG" 2>&1 else docs-mcp-server scrape "$repo_name" \ "file://$MEMORIES_DIR" \ - --silent >/dev/null 2>&1 -fi - -# Mark indexing time for subsequent staleness checks -touch "$TIMESTAMP_FILE" + --silent >"$ERROR_LOG" 2>&1 +fi && { touch "$TIMESTAMP_FILE"; rm -f "$ERROR_LOG"; } diff --git a/techpack.yaml b/techpack.yaml index 17d4502..a287067 100644 --- a/techpack.yaml +++ b/techpack.yaml @@ -43,55 +43,88 @@ components: description: Lightweight JSON processor brew: jq + # ── Embedding provider ────────────────────────────────────────────────── + # What the pack actually needs is one capability: an OpenAI-compatible + # /v1/embeddings endpoint serving nomic-embed-text. It does not need Ollama. + # So every check below probes the endpoint rather than the runtime, and Ollama + # is only the fallback installed when nothing already answers. That is what + # keeps a machine running llama.cpp / LM Studio / vLLM on this port from + # getting Ollama installed on top of it — and from `open Ollama.app` starting + # a second server that fights the first for the port. + # + # These gates work because `ComponentExecutor.isAlreadyInstalled` skips a + # `shell:` install when ANY of the component's doctorChecks passes. - id: ollama - displayName: Ollama - description: Local LLM runtime (compatible with all Apple Silicon) + # displayName, not id, is what mcs prints in " already installed, + # skipping" and in the component picker — so it has to be provider-neutral + # too, or a llama.cpp user is told about an Ollama they never installed. The + # id stays: nothing shows it, and four dependency lists reference it. + displayName: Embedding runtime + description: Local embedding runtime — installed only when nothing already serves the endpoint type: configuration shell: "curl -fsSL https://ollama.com/install.sh | sh" shellInteractive: true doctorChecks: - - type: commandExists - name: "Ollama installed" + # A script, not two commandExists checks, because "endpoint responding OR + # ollama installed" has to collapse into ONE doctor line: `isOptional` is + # silently ignored by commandExists, so a second check would show a hard + # red on a machine that deliberately runs something else. + - type: shellScript + name: "Embedding runtime available" section: AI Models - command: ollama - args: ["--version"] + command: checks/embedding-runtime.sh - id: ollama-service - displayName: Ollama service - description: Ensure Ollama is running + displayName: Embedding endpoint + description: Ensure something is serving the embedding endpoint type: configuration dependencies: [ollama] - shell: "open /Applications/Ollama.app --args hidden" + # `open` covers the Ollama.app install; `brew services` covers a brew one, + # which has no .app at all — the old single-arm command could never start a + # brew-installed Ollama. The same string is reused as the fixCommand below, + # so both arms must stay fast: fixCommands are killed at 10s, which is also + # why `brew install` must never appear here. + shell: "open /Applications/Ollama.app --args hidden 2>/dev/null || brew services start ollama" doctorChecks: + # /v1/models, not /api/tags: the former is OpenAI-compatible and answered + # by any provider, the latter is Ollama-proprietary and 404s on the rest. + # Keep this URL in step with checks/embedding-runtime.sh and + # hooks/sync-memories.sh, which probe the same endpoint. - type: commandExists - name: "Ollama service running" + name: "Embedding endpoint responding" section: AI Models command: curl - args: ["-sf", "http://localhost:11434/api/tags"] - fixCommand: "open /Applications/Ollama.app --args hidden" + args: ["-sf", "--max-time", "3", "http://localhost:11434/v1/models"] + fixCommand: "open /Applications/Ollama.app --args hidden 2>/dev/null || brew services start ollama" - id: ollama-nomic-embed - displayName: nomic-embed-text model + displayName: Embedding model (nomic-embed-text) description: Embedding model for docs-mcp-server type: configuration dependencies: [ollama-service] - shell: "ollama pull nomic-embed-text" + # Only Ollama can be pulled into. A provider that is up but cannot embed + # fails the check below, which is the right diagnosis — but without this + # guard the pull would then run on a machine that has no `ollama` at all. + # llama.cpp has no download-only command, so there is nothing to call there. + shell: > + command -v ollama >/dev/null 2>&1 && ollama pull nomic-embed-text + || echo "No Ollama here — your endpoint must serve an embedding model itself." doctorChecks: - - type: commandExists - name: "nomic-embed-text model" - section: AI Models - command: curl - args: ["-sf", "http://localhost:11434/api/show", "-d", "{\"name\":\"nomic-embed-text\"}"] - # Probes the exact endpoint docs-mcp-server calls. /api/show only reads the - # manifest — it does not load the model. This check forces a full load+embed - # round-trip, so a runner crash (e.g. Ollama/ggml incompatibility) surfaces - # as HTTP 500 and fails the check. + # Probes the exact endpoint docs-mcp-server calls, and is provider-neutral. + # A manifest read (Ollama's /api/show) would not load the model, so a runner + # crash (e.g. an Ollama/ggml incompatibility) passes it while every real + # embedding request 500s. This forces a full load+embed round-trip. - type: commandExists name: "nomic-embed-text embedding endpoint" section: AI Models command: curl args: - "-sf" + # commandExists checks carry no mcs-side timeout, so curl's own is the + # only bound. Generous enough for a cold model load, short enough that + # a wedged server cannot hang `mcs doctor` indefinitely. + - "--max-time" + - "20" - "-X" - "POST" - "-H" @@ -267,6 +300,7 @@ components: description: Ignores memory files from version control gitignore: - ".claude/.memories-last-indexed" + - ".claude/.memories-index.log" - ".claude/.kb-gate/" - ".claude/.kb-gate.log" diff --git a/tests/embedding-runtime-test.sh b/tests/embedding-runtime-test.sh new file mode 100755 index 0000000..d3b5f87 --- /dev/null +++ b/tests/embedding-runtime-test.sh @@ -0,0 +1,153 @@ +#!/bin/bash +# +# Tests for checks/embedding-runtime.sh. +# +# The check answers one question — "is an embedding runtime available at all?" — +# by OR-ing two conditions: an OpenAI-compatible endpoint responding, or an +# `ollama` binary on PATH. mcs uses it two ways at once, which is why both arms +# matter: as a doctor line, and (via `isAlreadyInstalled`, which skips a `shell:` +# install when ANY doctorCheck passes) as the gate on the Ollama installer. +# +# Two setup details are load-bearing, not incidental: +# - `curl` and `ollama` are stubbed through a PATH shim, so no test touches the +# network or the developer's real Ollama. A run that reached a live endpoint +# would pass or fail based on the machine rather than the code. +# - The arm tally at the end is derived from the message the script printed, +# never incremented straight-line. That is the whole point of it: if the shim +# ever failed to take effect and real `curl` answered a live Ollama, the +# "endpoint down" group would still exit 0 and its assertion would still +# pass — but via the wrong arm. Counting what the script actually said is +# what turns that into a loud failure instead of a vacuous green. + +set -uo pipefail + +repo_root=$(cd "$(dirname "$0")/.." && pwd) +src="$repo_root/checks/embedding-runtime.sh" +[ -f "$src" ] || { + echo "FATAL: $src not found" + exit 1 +} + +work=$(mktemp -d) +trap 'rm -rf "$work"' EXIT + +shim="$work/bin" +mkdir -p "$shim" + +pass=0 +fail=0 +endpoint_arm=0 # times the endpoint arm decided the outcome +binary_arm=0 # times the fallback binary arm decided it + +ok() { + pass=$((pass + 1)) + printf ' ok %s\n' "$1" +} + +bad() { + fail=$((fail + 1)) + printf ' FAIL %s\n' "$1" +} + +group() { + printf '\n%s\n' "$1" +} + +# Stub curl. $1: the exit code it should return. The script branches only on +# success vs failure, never on the specific code, so one failure value is enough +# to cover them all — 7 (connection refused) stands in for 22 (`-f` on 4xx/5xx). +stub_curl() { + cat >"$shim/curl" <"$shim/ollama" + chmod +x "$shim/ollama" + else + rm -f "$shim/ollama" + fi +} + +# Run with ONLY the shim plus the minimum real tools on PATH, so a stray system +# `ollama` cannot satisfy the fallback arm by accident. +run_check() { + PATH="$shim:/usr/bin:/bin" bash "$src" 2>&1 + return $? +} + +# Credit whichever arm the script says it took. Never called unconditionally. +record_arm() { + case "$1" in + *"responding at"*) endpoint_arm=$((endpoint_arm + 1)) ;; + *"not currently serving"*) binary_arm=$((binary_arm + 1)) ;; + esac +} + +assert_exit() { + local label=$1 expected=$2 actual=$3 + if [ "$actual" -eq "$expected" ]; then + ok "$label (exit $actual)" + else + bad "$label — expected exit $expected, got $actual" + fi +} + +assert_says() { + local label=$1 needle=$2 haystack=$3 + case "$haystack" in + *"$needle"*) ok "$label" ;; + *) bad "$label — output did not mention '$needle': $haystack" ;; + esac +} + +# ========================================================================== +group "endpoint responding — passes regardless of who serves it" + +stub_curl 0 +stub_ollama absent # nothing Ollama-ish anywhere +out=$(run_check) +rc=$? +record_arm "$out" +assert_exit "a reachable endpoint with no Ollama installed still passes" 0 "$rc" +assert_says "wording stays provider-neutral" "OpenAI-compatible" "$out" + +# ========================================================================== +group "endpoint down, Ollama installed — the installed-but-stopped case" + +stub_curl 7 +stub_ollama present +out=$(run_check) +rc=$? +record_arm "$out" +assert_exit "a stopped Ollama does not re-trigger the installer" 0 "$rc" +assert_says "says why it passed" "not currently serving" "$out" + +# ========================================================================== +group "endpoint down, nothing installed — the only failing case" + +stub_curl 7 +stub_ollama absent +out=$(run_check) +rc=$? +record_arm "$out" +assert_exit "no runtime at all fails" 1 "$rc" +assert_says "reports what it probed" "no embedding runtime found" "$out" +assert_says "probes the pack-wide endpoint" "localhost:11434" "$out" + +# ========================================================================== + +printf '\n%s\n' "-----------------------------------------" +if [ "$endpoint_arm" -eq 0 ] || [ "$binary_arm" -eq 0 ]; then + printf 'FAIL: one arm never decided an outcome (endpoint=%d, binary=%d).\n' \ + "$endpoint_arm" "$binary_arm" + printf ' The OR is not being exercised — suspect the PATH shim.\n' + exit 1 +fi +printf '%d passed, %d failed (both arms exercised)\n' "$pass" "$fail" +[ "$fail" -eq 0 ] || exit 1