fix(cli): align divergent CLI/MCP paths with canonical resolvers#306
Open
andreybavt wants to merge 6 commits into
Open
fix(cli): align divergent CLI/MCP paths with canonical resolvers#306andreybavt wants to merge 6 commits into
andreybavt wants to merge 6 commits into
Conversation
ktx ingest resolved the SQL-analysis port via resolveKtxCliSqlAnalysis, which honors the external-daemon env overrides, but `ktx sql` and the MCP server built their port by calling createManagedDaemonSqlAnalysisPort directly. On a uv-less host with an external daemon and the override set, ingest worked while `ktx sql` and MCP sql_execution ignored the override and tried to install the managed Python runtime — contradicting the documented contract and runtime-requirements, which already skip the managed runtime when those vars are set. Collapse the three call sites onto one env-override-aware resolver: add externalDaemonSqlAnalysisBaseUrl + resolveSqlAnalysisPort in managed-python-http.ts and route sql.ts, mcp-server-factory.ts, and resolveKtxCliSqlAnalysis through it. createManagedDaemonSqlAnalysisPort becomes @internal (test-only export).
Extract readGitRepoConnectionFields as the single reader of where the git-repo context-source drivers store their fields (dbt's snake_case repo_url/source_dir, lookml's camelCase repoUrl, metricflow's nested metricflow.*). `ktx connection test` now routes through it and, when a dbt connection points at a local source_dir, verifies the directory exists instead of running a remote repo reachability check that can never apply.
Route status config references through resolveKtxConfigReference so file: keys are read (not reported as ready by trusting the raw path), read git-repo connection fields through the shared readGitRepoConnectionFields, and stop reporting openai embeddings as ready from a bare OPENAI_API_KEY since resolveLocalKtxEmbeddingConfig requires a resolved openai.api_key.
…esolver The local Looker adapter understood only env: references, so a client_secret_ref written by `ktx setup` as a file: secret silently failed to resolve. Route it through resolveKtxConfigReference so file: and env: refs both work, matching the other connector paths.
…r call createManagedPythonSemanticLayerComputePort is now synchronous and resolves the managed runtime lazily on the first query/validate/generate call, caching the port on success. An MCP session that only searches never installs the Python runtime at boot, avoiding the eager-install crash loop on uv-less hosts.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Reconciles this branch's audit fixes with main's independently-landed work where they overlapped: - MCP Python-runtime install: main's #297 (createLazyManagedPython- SemanticLayerComputePort + self-provisioned uv, shipped in 0.12.0) supersedes this branch's redundant 96fb94c. Adopted main's canonical lazy port; dropped the duplicate single-function lazy fix. - mcp-server-factory.ts: kept main's lazy semantic-compute port AND this branch's resolveSqlAnalysisPort (honors KTX_SQL_ANALYSIS_URL/KTX_DAEMON_URL, a strict superset of main's createManagedDaemonSqlAnalysisPort). - doctor.test.ts: configured embeddings.openai.api_key on the ready-path tests to match 7692d92's corrected status semantics (status no longer reports openai embeddings ready from a bare OPENAI_API_KEY env var). Verified: type-check, build, default suite (3010 tests), connection/sl/ mcp-port excluded tests (67 tests), dead-code gate — all green.
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This branch audits divergent parallel implementations in the CLI, where behavior depended on which entry point a caller used rather than on its inputs.
ktx sqland the MCP server now honorKTX_SQL_ANALYSIS_URL/KTX_DAEMON_URL,ktx connection testsupports a local dbtsource_dir, andktx statuschecks now match the actual runtime resolvers. Lookerclient_secret_refis resolved through the canonical config resolver instead of a forked path. Finally,createManagedPythonSemanticLayerComputePortis now synchronous and installs the managed Python runtime lazily on the first semantic-layer call, so an MCP session that only searches never triggers an eager install at boot.