feat(runtime): host-local model catalogs for Cursor and Kimi (Phase 1)#1820
feat(runtime): host-local model catalogs for Cursor and Kimi (Phase 1)#1820yuezengwu wants to merge 5 commits into
Conversation
Discover model ids from the bound computer (agent models / ~/.kimi-code) via a correlated daemon RPC, and render DEFAULT + catalog + custom id fallbacks in the agent Model picker. Co-authored-by: Cursor <cursoragent@cursor.com>
Rewrite Cursor/Kimi parsers without polynomial regexes, fix Biome export order/formatting, and prepare a feat/-prefixed branch for CI. Co-authored-by: Cursor <cursoragent@cursor.com>
Keep host-local discovery + GET models API in this PR; revert web UI so frontend ownership stays with kiven against the shared catalog contract. Co-authored-by: Cursor <cursoragent@cursor.com>
baixiaohang
left a comment
There was a problem hiding this comment.
Review by baixiaohang-code-reviewer - PR #1820
Recommendation: request changes
- Rationale: the new synchronous daemon RPC is process-local in a multi-replica server, and Kimi discovery does not follow all configuration forms supported by Kimi Code.
Risk level: A
- Path baseline: includes
apps/cli/**andpackages/client/**→ A - Semantic lift: none (already highest)
Review findings
❌ 1. The request/reply path only works when the HTTP request lands on the same server process that owns the daemon WebSocket. sendToClient and pendingClientReplies are both process-local, while clients.instance_id and the notifier architecture explicitly support replicas. On another replica this returns a false 503 for a connected computer; a result received by the socket-owning replica also cannot resolve a waiter stored elsewhere. Route both command and correlated result across replicas (or use a durable shared rendezvous), and cover the cross-instance case. [packages/server/src/api/clients.ts:115, packages/server/src/services/connection-manager.ts:242]
❌ 2. Kimi discovery always reads ~/.kimi-code/config.toml, ignoring the official KIMI_CODE_HOME relocation. A daemon can run Kimi correctly from $KIMI_CODE_HOME/config.toml while this endpoint reports the catalog unavailable or, worse, returns a stale default-home catalog. Derive the path from the same effective environment as the runtime and add a custom-home case. Kimi documents the relocation here: https://moonshotai.github.io/kimi-code/en/configuration/config-files.html#config-file-location. [packages/client/src/runtime/capabilities/discover-models.ts:188]
❌ 3. The hand-written TOML scanner recognizes only quoted model table headers ([models."..."]) and double-quoted scalar values. Valid Kimi configurations may use bare aliases such as [models.gemini-3-pro-preview]; those currently produce an empty catalog even though Kimi loads them. Please parse the supported TOML shape without narrowing valid syntax (prefer the provider parser/library if available), and test bare aliases plus the effective default_model. Kimi documents model aliases and quoting rules here: https://moonshotai.github.io/kimi-code/en/configuration/config-files.html#models. [packages/client/src/runtime/capabilities/discover-models.ts:105-145]
Action taken
- Submitted request changes on the replacement PR; the original PR #1818 is closed.
Route provider-models commands/results across server replicas via PG NOTIFY plus durable clients.metadata rendezvous, and parse Kimi catalogs with smol-toml including KIMI_CODE_HOME and bare model aliases. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Addressed review findings in d988e2b:
|
baixiaohang
left a comment
There was a problem hiding this comment.
Review by baixiaohang-code-reviewer - PR #1820
Recommendation: request changes
- Rationale: the Kimi fixes are complete, but the new cross-replica rendezvous can still lose or misroute valid daemon results under normal concurrency and connection replacement.
Risk level: A
- Path baseline: includes
apps/cli/**andpackages/client/**→ A - Semantic lift: none (already highest)
PR summary
- Author / repo: yuezengwu / agent-team-foundation/first-tree
- Problem: operators need the model picker to reflect the Cursor/Kimi catalog on the computer that actually runs the agent, without moving provider credentials into First Tree Cloud.
- Approach: a correlated HTTP → Server → daemon RPC discovers the host catalog; this update adds PG NOTIFY fan-out, a PostgreSQL rendezvous for large results, KIMI_CODE_HOME support, and standards-compliant TOML parsing.
- Impacted modules: CLI daemon, Client connection/discovery, Server client WebSocket/API/notifier, shared wire schemas.
Review findings
❌ 1. storeModelCatalogRpcResult performs a SELECT followed by replacement of the entire clients.metadata object. Two concurrent catalog replies can both read the same map and the later UPDATE drops the other ref; a concurrent capabilities or update-attempt write can also be erased (or erase the RPC entry). This makes the new shared rendezvous non-atomic and can both time out a valid request and regress unrelated client state. Store the ref with an atomic JSONB operation or a dedicated keyed table, preserve sibling metadata writers, and cover concurrent refs plus a sibling metadata update. [R4 / packages/server/src/services/provider-models-rpc.ts:54]
❌ 2. The durable result is still usable only when the volatile result NOTIFY arrives. notifyDaemonClientCommandResult swallows publish failures, LISTEN/NOTIFY can be missed during a listener reconnect, and the HTTP route only awaits the process-local promise; on timeout it returns 503 without checking the catalog already stored in PostgreSQL. Add a race-safe database fallback (at minimum read the correlated result before treating the timeout as failure; polling or another durable wake strategy is also viable) and test a stored result with a lost wake. [R4 / packages/server/src/api/clients.ts:142]
❌ 3. Command fan-out is not scoped to the DB-authoritative clients.instance_id. Each replica sends whenever its process-local map still has an open socket for the client id. After a cross-replica reconnect/takeover, the previous replica can retain an open socket even though its heartbeat no longer matches the current instance, so both old and current daemons receive the same ref and whichever catalog arrives first wins. Carry the target instance through the command and accept/deliver only on that current instance (including the result path), with a replacement-connection case. [R4 / packages/server/src/api/agent/ws-client.ts:302]
✅ 4. The prior Kimi findings are resolved: discovery now honors KIMI_CODE_HOME and parses quoted/bare TOML model aliases with a real TOML parser.
Action taken
- Submitted request changes on head
d988e2bf.
Atomic jsonb_set for catalog refs and capabilities, DB fallback when the result wake is lost, and instance_id-scoped command/result delivery so stale sockets after takeover cannot win the RPC. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Addressed the second review round:
|
baixiaohang
left a comment
There was a problem hiding this comment.
Review by baixiaohang-code-reviewer - PR #1820
Recommendation: request changes
- Rationale: the previous rendezvous fixes are substantially implemented, but connection ownership can still change between validation and persistence, and the new atomic map permanently accumulates large per-request results.
Risk level: A
- Path baseline: includes
apps/cli/**andpackages/client/**→ A - Semantic lift: none (already highest)
PR summary
- Author / repo: yuezengwu / agent-team-foundation/first-tree
- Problem: operators need Cursor/Kimi model choices to come from the computer actually running the provider, without moving provider credentials into First Tree Cloud.
- Approach: the web-facing GET endpoint performs a correlated Server → daemon RPC; commands are routed to the socket-owning replica, catalogs are discovered locally and persisted in PostgreSQL, and result wakes fan back to the HTTP-serving replica.
- Impacted modules: CLI daemon, Client discovery/connection handling, Server client API/WebSocket/notifier/services, shared wire schemas.
Review findings
❌ 1. The result-ownership guard is still a check-then-act race. The handler first SELECTs clients.instance_id, then calls storeModelCatalogRpcResult, whose UPDATE is guarded only by clients.id. A reconnect can move ownership to another replica between those statements; the old socket then persists its catalog and wakes the waiter even though it is no longer authoritative. Make ownership validation and persistence one atomic operation (for example, UPDATE with both client id and expected instance id and require a returned row), and also reject a socket that has already been replaced locally. Cover takeover between validation and the durable write. [R4 / packages/server/src/api/agent/ws-client.ts:1801, packages/server/src/services/provider-models-rpc.ts:58]
❌ 2. The atomic ref merge removed the previous physical age/count pruning. MAX_AGE_MS now only makes reads ignore old entries; every result still appends a new UUID key and no path deletes it. Legitimate repeated GETs therefore grow clients.metadata forever with catalogs that may already be too large for NOTIFY, and the WS handler also persists any schema-valid UUID result even when no waiter exists on that replica. Restore a physical bound/cleanup atomically (or move rendezvous rows to a keyed table with expiry), and cover enough sequential refs to prove stored data is actually bounded rather than merely unreadable. [R3 / packages/server/src/services/provider-models-rpc.ts:19, packages/server/src/services/provider-models-rpc.ts:61, packages/server/src/api/agent/ws-client.ts:1815]
✅ 3. The earlier non-atomic sibling overwrite, lost-wake false 503, and target-instance command fan-out findings are otherwise addressed in the new paths.
Action taken
- Submitted request changes on head
278be4fb.
|
Closing in favor of the single GitLab delivery MR for this Phase 1 work: https://gitlab.staging.unispark.dev/first-tree/first-tree/-/merge_requests/30 Owner confirmed one MR only; GitHub draft #1820 is retired. |
Summary
agent models,~/.kimi-code/config.toml) via a correlated daemon RPC (provider-models:list→provider-models:result).GET /api/v1/clients/:clientId/providers/:provider/modelsreturningProviderModelCatalogfor the web to consume.Test plan
discover-modelsparsers; server GET models endpoint~/.kimi-code/config.toml: API returns configured models + defaultModelId