feat(node-sdk): migrate the SDK method surface to agent-core-v2 - #2262
Conversation
|
commit: |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cdb566045a
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const { app } = bootstrap( | ||
| { | ||
| homeDir: this.homeDir, | ||
| configPath: this.configPath, | ||
| clientVersion: this.identity?.version, | ||
| }, | ||
| [...logSeed(resolveLoggingConfig({ homeDir: this.homeDir, env: process.env }))], | ||
| ); |
There was a problem hiding this comment.
When an SDK or CLI caller supplies identity, this bootstrap only seeds logging, so agent-core-v2 falls back to an empty IHostRequestHeaders. Consequently, model and WebSearch requests from the new TUI route omit the User-Agent and X-Msh-* device/version headers that the v1 client supplies and the existing v2 print composition root explicitly seeds; build createKimiDefaultHeaders({ homeDir, ...identity }) and include hostRequestHeadersSeed(...) here.
Useful? React with 👍 / 👎.
| const agent = await this.materializeMainAgent(handle, { | ||
| model: input.model, | ||
| thinking: input.thinking, | ||
| }); |
There was a problem hiding this comment.
When the requested model is unknown, materializeMainAgent throws after sessionLifecycle.create has already persisted and indexed the session. The caller receives a rejected createSession, but the failed ID remains live/listed and a retry reports session_already_exists; pass the binding into the lifecycle create operation so its built-in rollback covers the failure, or otherwise remove the newly created session and its persisted index entry on initialization errors.
Useful? React with 👍 / 👎.
| for (const [domain, domainPatch] of Object.entries(patch)) { | ||
| if (domainPatch === undefined) continue; | ||
| await this.klient.global.config.set({ domain, patch: domainPatch }); | ||
| } |
There was a problem hiding this comment.
When a patch contains multiple domains and a later domain fails validation or persistence, every earlier config.set has already been written, even though setConfig rejects. For example, a valid providers update followed by an invalid thinking section leaves the provider change on disk; the v1 method validates the merged document before its single atomic write, so callers do not expect a failed update to partially mutate configuration.
Useful? React with 👍 / 👎.
| const { app } = bootstrap( | ||
| { | ||
| homeDir: this.homeDir, | ||
| configPath: this.configPath, | ||
| clientVersion: this.identity?.version, | ||
| }, | ||
| [...logSeed(resolveLoggingConfig({ homeDir: this.homeDir, env: process.env }))], | ||
| ); |
There was a problem hiding this comment.
When callers pass options.telemetry (as runShell does), it is retained for KimiHarness events but never installed as an app-scope telemetry appender. The bootstrapped v2 ITelemetryService therefore keeps only its null appender, dropping engine-originated events such as session lifecycle, LLM, tool, compaction, and task telemetry that the v1 SDK forwards through the supplied client.
Useful? React with 👍 / 👎.
- add SDKRpcClientV2 wiring the v2 engine (DI x Scope) in-process via the klient memory transport, with getExperimentalFeatures migrated to klient.global.flags.list() and unmigrated methods failing fast - export createKimiHarnessV2 / SDKRpcClientV2 from the SDK index - wire the experimental v2 gate into the CLI interactive shell (run-shell) - extend build-dts to bundle agent-core-v2 and klient declarations
- add engineAccessor escape hatch exposing the in-process engine's app-scope service accessor for SDK methods the klient facade does not cover yet - implement listWorkspaceSkills via ISkillDiscovery plus the v2 user/project root helpers and BUILTIN_SKILLS (plugin skills and skillDirs still gaps) - add a v1-v2 parity test pinning identical return values per migrated method, with understood gaps listed explicitly in KNOWN_DIFFS
- implement the remaining SDKRpcClientBase methods on SDKRpcClientV2, routed through the klient facade where covered, the engineAccessor escape hatch where the engine has a service, or SDK-side rebuilds on v2 primitives where only primitives exist (config shape mapping, global mcp.json store, MCP OAuth flows, importContext, session warnings, print background policy) - translate the v2 event stream into the v1 Event union and bridge approval/question/user_tool interactions per live session - rebuild resume replay by folding the v2 wire.jsonl through the v1 agent restore pipeline, so resumed sessions render history again - keep deleteSession as not_implemented; the v2 engine has no delete capability - extend the v1-v2 parity suite to every migrated method, pinning understood engine differences in KNOWN_DIFFS - add the dev:cli:v2 root script to launch the TUI on the v2 engine
cdb5660 to
22aa5c4
Compare
The v2 SDK client accepted KimiHarnessOptions.skillDirs (the CLI's --skills-dir) but never seeded it into the engine, so explicit skill dirs were silently dropped on the v2 TUI route and the Skill tool could not find skills from them. Seed skillCatalogRuntimeOptions at bootstrap and let listWorkspaceSkills resolve the explicit dirs as the user source, matching the engine's session skill catalog.
…lag again Drop the dedicated KIMI_CODE_TUI_V2 switch: the TUI v2 harness route is gated by KIMI_CODE_EXPERIMENTAL_FLAG, the same master switch as the kimi -p v2 route. The gate tests are kept with updated assertions, and dev:cli:v2 sets the master flag again.
1 participant
Related Issue
No linked issue — the problem is explained below.
Problem
The CLI's interactive surface (TUI) runs entirely on the legacy v1 engine (
agent-core) through@moonshot-ai/kimi-code-sdk. There was no way to boot the TUI on the agent-core-v2 engine, and no mechanism to tell whether a v2-backed SDK client behaves identically to the v1 one.What changed
Builds on the previous two commits in this branch (v2 harness MVP, skills migration + parity gate) and completes the migration of the SDK's method surface:
SDKRpcClientV2now implements everySDKRpcClientBasemethod (75 overrides), routed through the klient facade where covered, the in-processengineAccessorescape hatch where the engine has a service but the facade does not, or SDK-side rebuilds on v2 primitives where only primitives exist (config shape mapping, globalmcp.jsonwrites, MCP OAuth flows,importContext, session warnings, print background policy).deleteSessionstaysnot_implemented— the v2 engine has no delete capability.Eventunion, and approval/question/user_tool interactions are bridged per live session so TUI handlers work unchanged.wire.jsonlthrough the v1 agent restore pipeline, so the TUI renders history again when resuming a session on v2.KNOWN_DIFFSso the accepted gap list can only shrink.KIMI_CODE_TUI_V2env switch is set (deliberately separate from the masterKIMI_CODE_EXPERIMENTAL_FLAG, which keeps gating onlykimi -p), with a rootpnpm dev:cli:v2script. The default path is unchanged.telemetry = falsein config).No changeset: the v2 route only activates behind an experimental env switch, so there is no user-perceivable change on the shipped surface.
Checklist
gen-changesetsskill, or this PR needs no changeset.gen-docsskill, or this PR needs no doc update.