Skip to content

fix(backend): stop probing Ollama from the public setup status route - #113

Merged
patrickbrandao merged 2 commits into
mainfrom
fix/109-no-ollama-probe-on-setup
Oct 12, 2026
Merged

patrickbrandao merged 2 commits into
mainfrom
fix/109-no-ollama-probe-on-setup

Conversation

@patrickbrandao

Copy link
Copy Markdown
Owner

Closes #109

What

GET /api/v1/setup/status is a public route that the dashboard calls on first load (GuestGuard.tsx, Setup.tsx). On every call it pinged ${OLLAMA_HOST}/api/tags with a 3 s timeout, even with no Ollama provider configured, and nothing read the result. This PR removes that probe.

  • Backend: ollamaAvailable is gone from the status response and SetupService.isOllamaAvailable() is removed. Ollama is now reached only through the existing operator actions:

    • connecting Ollama, which runs OllamaSyncService.sync() from POST /api/v1/routing/:agentName/providers;
    • POST /api/v1/routing/ollama/sync;
    • discovery and proxying for a registered Ollama provider.
  • Frontend: ollamaAvailable is gone from services/setup-status.ts, along with checkIsOllamaAvailable(), its tests and the three component-test mocks. No component used it.

  • Shared types: there is no shared type for this response. It lives in the controller and the frontend service, so packages/shared is unchanged.

  • E2E: the first boot of test/no-phonehome-egress.e2e-spec.ts now points OLLAMA_HOST, THINKER_LMSTUDIO_BASE_URL and THINKER_LLAMACPP_BASE_URL at TCP listeners on ephemeral 127.0.0.1 ports, called traps. Each trap accepts and drops connections, so it also catches non-fetch sockets and never reaches a runner that happens to run on the machine. The boot runs with no provider configured. The test calls /setup/status twice and fails if:

    • the body still has ollamaAvailable;
    • any trap saw a connection;
    • the egress recorder logged a fetch to a runner, port 11434 or host.{docker,containers}.internal.

    test/setup.e2e-spec.ts pins the exact field list.

  • Docs: the egress section of docs/dev/manifest-guide.md now has a "Local runners (Setup: não sondar o Ollama local sem provedor configurado #109)" paragraph.

Before the fix, the new e2e failed. With the ollamaAvailable assertion temporarily commented out, the Ollama trap counted 2 connections, one per call, so the trap catches the old probe.

Tests

Command Result
npm run build --workspace=packages/shared ok
backend npx tsc --noEmit -p tsconfig.json ok
frontend npx tsc --noEmit -p tsconfig.json ok
npm run lint backend / frontend 0 errors (323 / 49 existing warnings, unchanged)
backend npx jest 472 suites, 9048 tests passed
frontend TZ=UTC npx vitest run 201 files, 4175 tests passed
TZ=UTC npm run test:e2e:docker --workspace=packages/backend 70 suites, 593 tests passed

An earlier full e2e run had one failure: proxy.e2e-spec.ts › "accepts X-Session-Key header without error" got a 503 while the network was down. That test reaches a real upstream, and the failure is unrelated to this change. It passed on a rerun of that spec and on the full rerun above.

Draft

This changes runtime behavior. It stays in draft until qa-docker checks, with passive capture, that boot, setup and dashboard navigation with no provider configured make no connection to host.docker.internal:11434.

🤖 Generated with Claude Code

GET /api/v1/setup/status pinged ${OLLAMA_HOST}/api/tags (3 s timeout)
on every call, and the dashboard calls it on first load, so a fresh
install contacted host.docker.internal:11434 with no Ollama provider
configured. Nothing read the result.

- backend: drop ollamaAvailable from the status response and
  SetupService.isOllamaAvailable(). Ollama is now reached only through
  the existing operator actions: connecting Ollama, which runs
  OllamaSyncService.sync(), and POST /api/v1/routing/ollama/sync.
- frontend: drop ollamaAvailable from setup-status.ts and
  checkIsOllamaAvailable(), plus its test and mocks.
- e2e: the first boot of no-phonehome-egress points OLLAMA_HOST,
  THINKER_LMSTUDIO_BASE_URL and THINKER_LLAMACPP_BASE_URL at loopback TCP
  traps. It then calls /setup/status with no provider configured and
  fails on any connection or recorded fetch to a runner. setup.e2e-spec
  pins the field list.
- docs: the egress section of manifest-guide.md documents the rule.

The response type is local to the controller and the frontend service;
packages/shared has none.

Closes #109

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

Copy link
Copy Markdown
Owner Author

Review of #113: fix(backend): stop probing Ollama from the public setup status route

I reviewed head 8e6313908ca5d400b3a82cf5d23ff0ce03601219 (one commit on origin/main) in a detached worktree, using git diff origin/main...8e63139. I also read the surrounding code at that SHA.

Summary: I found no blockers and no should-fixes. The change is a pure removal of the probe, and the new e2e really catches the old behavior. No high-risk criterion applies.

Checks

Remaining callers of the removed field and functions

  • git grep -i 'ollamaAvailable|isOllamaAvailable|checkIsOllamaAvailable' over the whole tree at the head finds only the new regression assertions and the docs paragraph.
  • There are no hits in packages/cli, packages/shared, MCP tools, docs/ or any frontend component.
  • Frontend consumers of services/setup-status.ts only use checkNeedsSetup (GuestGuard, Setup), checkSocialProviders, checkLocalLlmHost, checkEmailConfigured, checkMcpEnabled and createFirstAdmin. All of these are untouched.

Ollama is contacted only on operator actions. The PR leaves three paths that read OLLAMA_HOST:

  • OllamaSyncService.sync(), called from provider.controller.ts:191 (connecting ollama) and model.controller.ts:119 (POST ollama/sync). Both routes are authenticated, not @Public().
  • Discovery for a registered provider (provider-model-fetcher.service.ts:1037).
  • Proxying (provider-endpoints.ts:577).

setup.service.ts no longer imports OLLAMA_HOST. Nothing else at boot or on public routes reaches a runner.

The new e2e (test/no-phonehome-egress.e2e-spec.ts:138-169, 307-397):

  • The traps call listen(0, '127.0.0.1') and keep the socket bound, so there is no free-port race and nothing listens on a non-loopback interface.
  • The env override is applied after the parent env is copied (...extraEnv last), so the child really points OLLAMA_HOST and the LM Studio and llama.cpp variables at the traps. http://127.0.0.1:<port> passes invalidBaseUrlReason.
  • egress-recorder.cjs lets loopback fetch through to realFetch, so the removed fetch(\${OLLAMA_HOST}/api/tags`)` would have reached the Ollama trap. That matches the PR's report that the trap counted 2 connections before the fix.
  • The trap counters also cover the boot window, since the traps are open before bootServer. They also see non-fetch sockets.
  • The first boot starts from an empty schema, and no earlier test in that block creates a user, so needsSetup === true holds.
  • setup.e2e-spec.ts pins the exact field list.

Setup flow:

  • POST /api/v1/setup/admin and needsSetup() are unchanged, and the first-admin e2e in setup.e2e-spec.ts still runs.
  • GuestGuard and Setup.tsx read only needsSetup.
  • The SetupStatusResponse/SetupStatusResult change in the frontend only removes a field. It is backward-compatible with an old backend that still sends the field, because the field is ignored.

Security:

  • No credentials are involved.
  • No query or tenant scoping changes.
  • There is no new outbound destination. The PR removes one: an unconditional loopback/host-alias probe from a public route, which an unauthenticated caller could trigger repeatedly.

Project rules:

Tests and CI:

  • New behavior is covered at the right level: e2e for the route and egress, plus the unit specs updated.
  • The PR body reports local lint, typecheck, build, jest, vitest and e2e runs.
  • Check-runs on 8e63139 (--paginate) are all success: Lint, Unit (backend/frontend/cli/shared), Backend e2e (PostgreSQL 16), Compose smoke test, install.sh e2e, CodeQL, dependency-review.

qa-docker: pending. Runtime behavior changes, and the PR is still a draft until qa-docker checks, with passive capture, that there is no connection to host.docker.internal:11434 (the acceptance criterion of #109). This is a separate merge gate.

Findings

Blockers: none.

Should-fix: none.

Nits (optional):

  1. packages/backend/test/no-phonehome-egress.e2e-spec.ts:394: the recorder filter only matches :11434 and the host aliases. The LM Studio and llama.cpp traps depend on the env overrides being accepted.
    • Failure scenario: if getLocalServerBaseUrl ever rejects the override (a variable rename, a stricter validator), it silently falls back to http://localhost:1234 or :8080. A probe there would hit neither the traps nor this regex, so the lmstudio and llamacpp halves of the test would stop guarding anything without failing.
    • Fix: match :(11434|1234|8080)\b as well, or assert that server.output() has no ignoring it and using warning for the two variables.
  2. packages/backend/src/setup/setup.service.spec.ts:88 and packages/frontend/tests/services/setup-status.test.ts:146: these "method/export is absent" assertions are cheap, but they only guard the names. A probe reintroduced under another name passes them. The e2e is the real guard, so you can keep them or drop them.

Out of scope (pre-existing, not introduced here)

Verdict

  • No blockers.
  • None of the high-risk criteria applies:
    • no auth, crypto or key handling;
    • no migration;
    • no tenant-isolation change;
    • no governance, ADR, workflow or release files.
  • There are no should-fixes, so nothing needs to be ticketed before merge.
  • The remaining gates are qa-docker (pending) and taking the PR out of draft. CI is already green on this SHA.

Head revisado: 8e63139
Veredito: aprovado

@patrickbrandao

Copy link
Copy Markdown
Owner Author

qa-docker: PR #113 validated in Docker

Tested SHA: 8e6313908ca5d400b3a82cf5d23ff0ce03601219 (head confirmed with gh). Compared against origin/main at d18f1efb26a772640132f20b9a187978620c1747.

Image thinker:qa113-pr (arm64, 429 MB, build 18 s with warm layer cache). Base images: node:24-alpine, node:24-slim and distroless nodejs24-debian13:nonroot (the digests pinned in the Dockerfile). Postgres: postgres:16-alpine, 16.15. The built dist of the PR image has no ollamaAvailable or isOllamaAvailable; the main image has both.

Step Result
Build PR and main images pass
Passive capture (tcpdump in the app netns), fresh volume, first admin created through the dashboard in headless Chromium, 15 routes browsed, then 90 s idle see below
Operator action: Ollama provider against a host mock on 11434 pass
Gateway smoke through a host mock (chat completions, messages with Bearer and x-api-key, responses/Codex, stream and non-stream) pass (17/18 checks; the single miss is a wrong test expectation about the key prefix, see notes)
Contracts contracts/gateway against the running container pass (6/6)
Backend e2e TZ=UTC npm run test:e2e:docker --workspace=packages/backend (shared built first) pass (70 suites, 593 tests, 167 s)
Teardown, with label checks pass (no labeled volume or container left)

Probe count (SYN packets from the app container, no provider configured, nothing listening on the host ports)

Destination main PR
host.docker.internal:11434 19 0
host.docker.internal:1234 0 0
host.docker.internal:8080 0 0
any other address outside the compose networks 0 0

On main, the 19 probes to :11434 came one per GET /api/v1/setup/status while the pages loaded (GuestGuard, Setup and the signed-in routes); none arrived during the idle window. On the PR, the only SYNs were the browser to :2099, the healthcheck/loopback and the app to Postgres :5432. /api/v1/setup/status on the PR no longer carries ollamaAvailable; the dashboard loads /setup, creates the admin, signs in and redirects /login, /setup and /register to /overview, with no page errors.

With a mock listening on 11434, three extra GET /api/v1/setup/status calls produced no hit on the mock (request count unchanged).

Operator action still works

With the mock on host.docker.internal:11434: POST /api/v1/routing/ollama/sync returned {"count":1} and the mock saw GET /api/tags; connecting the ollama provider returned 201 and triggered discovery (Discovered 1 models for provider ollama), and ollama/qa-mock-model appeared in /v1/models.

Notes (not caused by this PR, same on main)

  • New agent keys have the prefix mnfst_, not thk_ (legacy rename still pending).
  • The first boot logs a Better Auth "Database schema mismatch / Missing tables" ERROR before migrations run. It appears on main too.
  • Page titles still say "Manifest".

Nothing flaky: every step passed on the first run. Isolation: unique Compose projects with the volume override (name: !reset null, resolved names carried the run id), no use of host ports 5432/5433, no real provider or external host contacted (catalog sync disabled in the test env).

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

Copy link
Copy Markdown
Owner Author

Re-review of #113 at a7d4009 (merge of origin/main)

The previous review approved 8e6313908ca5d400b3a82cf5d23ff0ce03601219 (comment). The new head a7d40092701e94042f627fc6d660316d69781039 is a merge commit:

The PR change is unchanged

  • I compared git diff origin/main...a7d4009 with the approved change git diff d18f1ef 8e63139 (d18f1ef was the old merge base). After dropping index and @@ lines, the two diffs are identical: the same 12 files, +103/-169.
  • The only difference is one hunk offset in docs/dev/manifest-guide.md (-618 became -626). The 31 lines that main added earlier in that file account for it. The added "Local runners (Setup: não sondar o Ollama local sem provedor configurado #109)" paragraph is byte-identical.
  • The merge brings in no other change to the files this PR touches. Everything else in 8e63139..a7d4009 is what main already has.
  • git grep at a7d4009 for ollamaAvailable, isOllamaAvailable and checkIsOllamaAvailable still finds only the regression assertions and the docs paragraph. Main added no new caller.

CI on a7d4009

All 20 check-runs (--paginate) completed with success:

  • Lint, which now also covers backend/test
  • Typecheck
  • Build
  • Scripts
  • Changeset check
  • Unit (backend, frontend, cli, shared)
  • Backend e2e on PostgreSQL 16 with TZ=UTC and TZ=America/Sao_Paulo
  • Docker build for linux/amd64 and linux/arm64
  • Compose smoke test
  • install.sh end-to-end
  • CodeQL (3 runs, including actions)
  • dependency-review
  • .github/dependabot.yml

qa-docker

qa-docker passed on 8e63139 (comment):

  • host.docker.internal:11434 saw 19 SYNs on main and 0 on the PR, captured passively with no provider configured.
  • :1234 and :8080 saw none on either side.
  • Setup, sign-in and redirects work.
  • POST ollama/sync and connecting Ollama still reach the mock.

The merge changes no runtime file this PR touches, and CI re-ran the e2e suite and the compose smoke on the merged tree, so the result carries over to a7d4009. Whether the qa-docker gate needs a fresh run on the merge commit is the Principal's call. From the review side, no re-run is needed.

Findings

Nothing changes from the previous review:

  • Blockers: none.
  • Should-fix: none.
  • Nits: the same two optional nits (the e2e regex at no-phonehome-egress.e2e-spec.ts:394 and the name-only absence tests).
  • Out of scope: the same observation about the unscoped POST ollama/sync. It does not block this PR.

No high-risk criterion applies.

Head revisado: a7d4009
Veredito: aprovado

@patrickbrandao
patrickbrandao marked this pull request as ready for review October 12, 2026 02:28
@patrickbrandao
patrickbrandao merged commit 365fb69 into main Oct 12, 2026
20 checks passed
@patrickbrandao
patrickbrandao deleted the fix/109-no-ollama-probe-on-setup branch October 12, 2026 02:28
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.

Setup: não sondar o Ollama local sem provedor configurado

1 participant