Skip to content

Gate the embedding provider on the endpoint, not on Ollama - #26

Closed
bguidolim wants to merge 1 commit into
mainfrom
bruno/embedding-capability-gate
Closed

Gate the embedding provider on the endpoint, not on Ollama#26
bguidolim wants to merge 1 commit into
mainfrom
bruno/embedding-capability-gate

Conversation

@bguidolim

Copy link
Copy Markdown
Collaborator

Why

The pack needs one capability — an OpenAI-compatible /v1/embeddings serving nomic-embed-text — but it gated on Ollama specifically. A machine already serving embeddings from llama.cpp, LM Studio or vLLM was therefore treated as having no provider at all: Ollama got installed over the top, and the service check (which probed an Ollama-proprietary endpoint that 404s everywhere else) then started a second server contending for the same port. mcs doctor reported three failures on a perfectly healthy machine.

Every check now probes the endpoint rather than the runtime, so Ollama is installed only when nothing already answers. Nothing is pulled for a non-Ollama provider, because there is no way to — llama.cpp has no download-only command, so the pack verifies and instructs instead.

Two pre-existing bugs in the indexing hook ride along, and are worth reviewing separately from the design change: its health gate used curl -s, which exits 0 on an HTTP 404 and so passed against any server that merely accepted a connection; and it advanced its staleness timestamp regardless of whether indexing succeeded, so a single failure meant the knowledge base was silently never indexed again. A failure now leaves its reason on disk and retries.

Test plan

  1. With Ollama stopped and nothing else on port 11434, run mcs sync --global → expect the interactive Ollama installer not to re-run, since the runtime is still installed.
  2. Serve /v1/models and /v1/embeddings from any non-Ollama server on 11434, then mcs sync --global → expect no Ollama install and no open Ollama.app; mcs doctor all green with no Ollama-specific lines.
  3. Serve only /v1/models (a provider that cannot embed) → expect the endpoint check green, the model check red, and no install attempt.
  4. mcs pack validate → expect one "not referenced" warning for the new check script. It is expected: doctor-check script paths sit outside mcs's referencedPaths. Do not silence it by adding the directory to ignore: — mcs accepts that entry silently and it would suppress update notifications for a file that carries behaviour.

- Every Ollama-specific check becomes a provider-neutral probe of the
  OpenAI-compatible endpoint, so a machine already served by llama.cpp or
  LM Studio installs nothing and no second server contends for the port
- Fix two pre-existing hook bugs: the health gate passed on an HTTP 404, and
  a failed index marked itself fresh so the KB was never reindexed again
- Make display names provider-neutral too, since mcs prints those during sync
@bguidolim

Copy link
Copy Markdown
Collaborator Author

Superseded by #27, which removes the thing this PR was gating.

The design here makes every check probe /v1/embeddings so any OpenAI-compatible server can provide embeddings instead of Ollama. #27 replaces docs-mcp-server and Ollama with qmd, which embeds in-process — so there is no provider to detect, no endpoint to probe, and no port to contend for. The new check script, its test and its workflow all describe a capability the pack no longer has.

The two pre-existing bug fixes that rode along are accounted for:

  • The curl -s health gate returning 0 on a 404 — gone with the gate itself; the hook now guards on command -v qmd.
  • The self-concealing staleness timestamp — carried over in 6a3fd77, including the failure log this PR introduced, now at .claude/.kb-index/memory-loop.log with a doctor check reporting it.

That second one is extended rather than copied. Gating the touch on a non-zero exit is not sufficient for qmd: it reports embed-lock contention as success, printing "Another embed process is already running. Skipping." and exiting 0. Since the indexing hook is registered on both SessionStart and UserPromptSubmit asynchronously, an overlapping run could mark the index fresh having embedded nothing, and the staleness check would then skip the pending work indefinitely. #27 adds a mutex around the reindex and confirms nothing is left pending before advancing the timestamp, so the log now also records that case.

The mcs pack validate observation in the test plan here still holds and is worth keeping in mind: doctor-check script paths sit outside mcs's referencedPaths, so a shellScript check's file draws a spurious "not referenced" warning, and silencing it via ignore: would suppress update notifications for a file that carries behaviour.

@bguidolim bguidolim closed this Aug 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant