Skip to content

fix(providers): warn on unrecognized THINKER_CATALOG_SYNC and align MCP routing status - #153

Merged
patrickbrandao merged 1 commit into
mainfrom
fix/75-catalog-sync-followups
Oct 14, 2026
Merged

patrickbrandao merged 1 commit into
mainfrom
fix/75-catalog-sync-followups

Conversation

@patrickbrandao

Copy link
Copy Markdown
Owner

Part of #75 (backend items 1 to 4; item 5 is the dashboard toast, left to frontend-dev).

What changed

  1. Unrecognized value warning. THINKER_CATALOG_SYNC set to anything other than true/1/yes/on/empty or false/0/no/off (e.g. disabled, a typo) keeps sync on, as before, but now logs one warning at boot from PricingSyncService.onModuleInit, listing the accepted values. The configured value is never echoed.
  2. Real value in logs. The "sync disabled" lines (OpenRouter, models.dev, OpenCode Go) show the configured off value (THINKER_CATALOG_SYNC=off) instead of always =false. Only a known off value is echoed.
  3. MCP status aligned with REST. New resolveRoutingStatus() (routing/routing-status.ts) is used by both GET /api/v1/routing/:agent/status and the MCP manifest_routing_status tool. MCP no longer reports pricing_cache_empty for an agent that can route (right after boot or after a failed fetch), and a tenant with only inactive connections now reports no_provider in MCP too.
  4. Egress e2e runs are independent. The two OpenCode Go blocks of no-phonehome-egress.e2e-spec.ts reset the schema, run the committed migrations and seed their own provider, so any block runs alone (-t). Runs 2 and 3 also assert no remote DNS lookup.

Docs: docs/dev/manifest-guide.md (accepted values, warning, shared status helper, e2e layout).

Not done here: the "No telemetry" CIMD mention in docker/DOCKER_README.md / .env.example from the review comment (docker files are being edited by another agent; small docs follow-up).

Tests

  • npx jest (backend): 476 suites, 9109 tests passed.
  • npx tsc -p tsconfig.test.json, eslint on changed files: clean.
  • npm run test:e2e:docker -- test/no-phonehome-egress.e2e-spec.ts: 24 passed; with -t "catalog sync on" alone: 2 passed, 22 skipped.

qa-docker: runtime behavior changed only in boot logging and the MCP status tool; a quick boot check with THINKER_CATALOG_SYNC=disabled and =off is enough.

🤖 Generated with Claude Code

…CP routing status

- Warn once at boot (PricingSyncService) when THINKER_CATALOG_SYNC is neither
  an on nor an off value, without echoing it; list the accepted values.
- "Sync disabled" log lines show the configured off value instead of =false.
- Extract resolveRoutingStatus() and use it from both the REST status route
  and the MCP manifest_routing_status tool, so an empty pricing cache no
  longer blocks an agent that can route in MCP, and inactive-only providers
  report no_provider in both.
- no-phonehome-egress e2e: the OpenCode Go blocks reset and migrate their own
  schema, and every boot asserts no remote DNS lookup.

Part of #75 (items 1 to 4; item 5 is frontend).

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

Copy link
Copy Markdown
Owner Author

Review: #153 fix(providers): warn on unrecognized THINKER_CATALOG_SYNC and align MCP routing status

Reviewed git diff origin/main...a6fc716.

Blockers: none.
Should-fix: none.

Nits

  • packages/backend/src/common/utils/catalog-sync.ts describeCatalogSyncSetting: it falls back to the literal false when the raw value is not a known off value. That can only happen if a caller logs while sync is on, which no caller does today. Fine as is.
  • The PR body defers the CIMD "No telemetry" wording in docker/DOCKER_README.md / .env.example. Track it in an issue.

Checks

  • No new outbound destination. The egress e2e now prepares its own database in each block and checks that no remote hostname is resolved.
  • The unrecognized-value warning does not echo the value, so nothing pasted by mistake ends up in the logs. Sync stays on, as before.
  • resolveRoutingStatus keeps the REST semantics: pricing_cache_empty is reported only when no tier routes. MCP now agrees with REST, and an MCP status no longer fails when the cache is empty but the agent can route. Tenant scoping is unchanged (agent.tenant_id).
  • Changeset present, Conventional Commit title, no new manifest identifiers.
  • CI on a6fc716: all green. Unit (shared) failed on an npm ECONNRESET during npm ci and passed on rerun.
  • qa-docker: the PR proposes a light boot check (THINKER_CATALOG_SYNC=disabled / off). It still needs to run before merge.

Head revisado: a6fc716
Veredito: aprovado

@patrickbrandao

Copy link
Copy Markdown
Owner Author

Review follow-ups are tracked in #158.

@patrickbrandao

Copy link
Copy Markdown
Owner Author

qa-docker: PR #153 (PASS)

Image thinker:qa-153 built from the PR head (linux/arm64, 428.8 MB, build 42s with warm cache). PostgreSQL 16.15 (postgres:16-alpine). Isolated Compose project, run-scoped volumes, mock provider only, random 64-hex THINKER_ENCRYPTION_KEY. For the disabled run the sync stays on by design, so openrouter.ai, models.dev and raw.githubusercontent.com were mapped to 127.0.0.1 in the container (extra_hosts); the fetches failed locally and no real host was contacted.

Step Result
Boot with THINKER_CATALOG_SYNC=disabled, health, setup, key pass
Boot warning: "THINKER_CATALOG_SYNC has an unrecognized value, so catalog sync stays on. Accepted values: true, 1, yes, on (default) or false, 0, no, off." pass
Warning does not echo the configured value (CATALOG_SYNC=disabled appears 0 times in the log) pass
THINKER_CATALOG_SYNC=off: no warning, logs "THINKER_CATALOG_SYNC=off: OpenRouter pricing sync disabled, using bundled data only" and the same for models.dev pass
THINKER_CATALOG_SYNC=Off: the log shows the configured value (=Off) pass
/v1/models, OpenAI chat, Anthropic /v1/messages, Responses via mock pass
contracts/gateway (9 tests) pass (9/9)
Backend e2e (test:e2e:docker, includes the updated no-phonehome-egress spec) pass (71 suites, 601 tests, 493s)
Teardown, no run-labelled volumes left pass

Nothing flaky. Tested SHA: a6fc716

@patrickbrandao
patrickbrandao marked this pull request as ready for review October 14, 2026 14:08
@patrickbrandao
patrickbrandao merged commit aa6482c into main Oct 14, 2026
29 of 30 checks passed
@patrickbrandao
patrickbrandao deleted the fix/75-catalog-sync-followups branch October 14, 2026 14:08
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