feat: remote environments for SSH and Docker execution targets (phase 1: creation-time binding) - #4062
feat: remote environments for SSH and Docker execution targets (phase 1: creation-time binding)#40627Sageer wants to merge 342 commits into
Conversation
…store Create-seeded remote bindings now dispatch RuntimeSetBinding at create time so the binding is durable from the start, and the binding service restore hook, when replay produced no binding, peeks the agent's own wire scope for the last RuntimeSetBinding and reseeds it instead of persisting the in-memory seed default. Restored non-main bindings are background-reconnected and rerooted at the binding cwd with never-fail semantics; a binding whose runtime declaration is gone stays bound and fails explicitly with runtime.not_found at use rather than falling back to local.
…he local session cwd Hook execution failures (spawn failure, timeout, non-zero non-block exit) were silently swallowed behind fail-open catch paths. They are now logged through ILogService and reported on a new onDidHookError event; the main agent surfaces the first failure per session as a one-shot hook.result warning while block decisions stay byte-identical fail-open. Every agent-scope trigger call site now passes the session's local cwd explicitly instead of falling back to the process cwd, so hooks always execute on the client machine with the session's local working directory, including sessions bound to a remote runtime. Docs (en+zh) state that hooks are client-side with the local session cwd, that no project-level hook source exists, and that target-side hooks are a future, undesigned concept.
…and retain decoder chunks ManagedRemoteRuntime.connect() bypassed connectInflight for connected views, so a reconnect of a previously-connected runtime never surfaced 'connecting'/whenReady and acquireWhenReady failed fast with runtime.unavailable instead of awaiting the reconnect. Route the connected-view branch through the same tracked path as the pending branch; on success the view settles on its inner's status so a view that joined another view's connect stays usable. LineFrameDecoder.push() re-concatenated the whole accumulated prefix with every chunk, costing O(n^2) copying for a large frame delivered in small transport chunks. Retain chunk views and copy only once when a frame completes; framing, boundary, and protocol-violation behavior is unchanged.
…repo-wide Runtime->Environment across agent-core-v2, remote-exec, node-sdk, kap-server, klient, acp-server, apps/kimi-code, apps/vis, apps/kimi-inspect and the en/zh docs: the Runtime interface and its family (registry, lease, binding, provider, unit host, workspace view), the [environments] config section and .kimi-code/environments.toml, the /environment slash command and --environment flag, the session environment REST/WS surface, and the SDK session methods. The Runtime.environment field becomes host. Durable wire surfaces break clean with no shims (the feature is experimental): the environment.set_binding event type, environment.status.changed, the runtimeBinding/runtime.binding state keys, the tui.toml status-line slot id, and the klient IPC service key. Unrelated runtime concepts (goal actor enum, MCP runtime names, model runtime config, worker runtimes, kimiCu, the remote_runtime experimental flag) keep their names.
Rename the leftovers the round-1 review enumerated: the REST error string and OpenAPI descriptions on the environment surface, the environment-provider error strings, the remaining type names (EnvironmentReadStreamSource, EnvironmentStdioTransport, the test wire types), execution-target locals and test fake variables, and revert the session-status variable to runtimeStatus to match its family.
Rename the createGeneration locals to environment (declaration + usages), switch three lease-value longhands to shorthand, and retitle the kap-server environment route describe block.
Rename the last batch of runtime-flavored factories, locals, params, and test titles across the test tree to environment (registries, binding, program, session, workspace, plan, media, kap-server, node-sdk, acp-server, remote-exec), switch the remaining lease-value longhands to shorthand, and keep the unrelated runtime zones (actor runtime, worker runtimes, FiberRuntime, kimiCu, MCP runtime names, model runtime config) intact.
Remove the remote_runtime experimental flag entirely: the flag definition, its KIMI_CODE_EXPERIMENTAL_REMOTE_RUNTIME env var, and every enabled() gate across the engine (provider attach, session manager, agent binding, workspace roots), the remote-exec provider, the node-sdk session surface, the kap-server REST routes, and the TUI (slash command, footer slot, mention suggester, feedback attachment). Remote environments are now unconditionally on. Also register the environment.status.changed event in the kap-server zod agentEventSchema so it enters the AsyncAPI contract, document the feature as stable in the en+zh guides and references (dropping flag instructions and experimental labels), complete the server-api environment endpoint docs with the declare route, and fold the five pending experimental changesets and the rename changeset into a single graduation changeset — the feature had no prior release, so this is its first user-facing announcement.
The trust prompt only listed gated MCP servers even though the SDK already returns gatedEnvironments with each declaration's full launch command line. An untrusted repo could declare launchers in .kimi-code/environments.toml and the prompt never showed what the user authorized. Render the launchers like the MCP targets, with the same control-character sanitization.
The remote-exec provider re-resolved declarations only on config-section changes and the project file watch. A workspace materialized while untrusted kept project environments unregistered after the user accepted trust, and a project default then seeded sessions with an id absent from the registry. Program now re-fires the local generation's trust changes on a stable onDidChangeTrust event (re-subscribed across generation rebuilds), the manager passes it through EnvironmentProviderContext, and the provider reconciles on every flip in both directions.
The node-sdk and kap-server each carried an identical unguarded read-merge-write of .kimi-code/environments.toml, so two concurrent adds (or one via REST plus one via SDK) read the same snapshot and the last write silently dropped the other entry. Consolidate the helper into agent-core-v2 with a per-path promise-chain mutex so updates serialize per file; both call sites now share it.
…ect-local config Two graduation-blocking finds from the live smoke: - The native print path bootstrapped the engine without ever attaching the remote-exec provider, so declared environments never registered and kimi -p --environment <id> always failed 'environment does not exist'. Attach the provider after bootstrap, mirroring the SDK client. - FileProjectLocalConfigService treated only Node errnos as path-missing, so a remote HostFsError (no Node-style cause) for a missing .kimi-code/local.toml crashed session startup with storage.io_failed instead of reading as no project-local config. Accept the fs-domain not-found codes too.
The flag was parsed and honored in print mode, but the interactive TUI's startup options never carried it and the lazy first session was created without environmentId — kimi --environment <id> silently started a local session. Thread it through TUIStartupOptions into the lazy creation path with the same first-session-only consumption as --agent.
…n the busy frame Container.invalidate() only clears child render caches; it never schedules a repaint, so a switch/reconnect/declare failure that resolved after the keypress cycle left the dialog stuck on 'Connecting…' until the next key. Thread requestRender through the three environment dialogs' options (the MigrationScreenComponent precedent), expose it on SlashCommandHost, and invoke it after every async setBusy/showError/setOptions. Also sweeps the stale '(experimental remote environment)' header comments.
Undo across an environment switch left the binding on the switched-to environment: the binding service's onDidRestore hook runs only on the first restore (didRunRestoreHooks), so a rerun restore after an undo rebuilt the environmentBindingKey projection at the cut point but never re-applied it to the live binding — and any path falling back to peekPersistedBinding would re-read the untruncated wire log and re-apply the newest (post-switch) binding, which is right for resume and wrong for undo. Register an AgentConversationUndoParticipant in the binding service: it reads the cut-point projection (or the seed binding when the cut predates the first binding event) and re-applies it with the full side effects — session workDir, background reconnect, environment reminder on a machine identity change, and the onDidChange fire so the footer slot follows. The resume path (hook + peek fallback) is untouched and the two paths cannot fight: the hook is once-per-dispatcher, the participant runs only on undo. The mechanism is event-general, so M49's model-initiated EnvironmentSetBinding ops ride the same revert path.
The dialogs now invoke the host's requestRender after async state changes; the command-flow test's host mock predates the method and crashed the switch/reconnect/add flows silently.
- Drop the last two stale '(experimental remote environment)' comments (commands/environment.ts header, footer environment slot). - Remove the dead ENVIRONMENT_UNAVAILABLE entry from the declare route's OpenAPI errors block — nothing in the declare handler throws it. - Brace the two new void-expression arrows (program.ts trust re-fire, remoteEnvironmentProvider trust listener) and drop the four unnecessary WireRecord assertions in the undo-test stub, restoring the lint warning count to baseline. - Correct the switch endpoint's cwd docs (en+zh): non-local switches require cwd (40001 when missing); the entry's defaultCwd applies only in the createSession seed path.
Four no-confusing-void-expression warnings from the Bug A wiring — same shape as the program.ts and remoteEnvironmentProvider.ts arrows braced in the previous sweep.
The remote-environment footer slot rendered the connecting status as static warning-colored text. Reuse the shared braille frame set and 80ms interval (the same set the thinking indicator uses) to tick a spinner ahead of the environment id. The timer is strictly bounded to the connecting status: it starts when the slot enters connecting and stops the moment the status moves on (or the footer is disposed), repainting each frame through the standard onRefresh path. Other statuses render exactly as before and the local environment still renders nothing.
… routes Keep the read-only GET environment binding and GET environments list endpoints; phase 1 binds environments at session creation only.
…est surfaces Keep the read-only getEnvironment/listEnvironments session APIs; phase 1 binds environments at session creation only. The klient wire contract shrinks to agentEnvironmentBindingService.current.
…pletion Phase 1 binds environments at startup (--environment) or session creation; the in-session manager, cwd/add dialogs, remote @ completion, and the undo-time binding refresh move to the follow-up change phase.
…e binding narrowing
…onment tests at session creation
🦋 Changeset detectedLatest commit: f62ca41 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 71376da927
ℹ️ 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".
| @@ -217,6 +221,10 @@ export class SessionLifecycleService extends Disposable implements ISessionLifec | |||
| : await agents.create({ | |||
There was a problem hiding this comment.
Create the main agent for a configured default environment
When [environments].default is used instead of passing an explicit environmentId, SessionManager.create resolves the remote environment into the effective options, but the SDK and REST creation paths do not provide mainAgentBinding. This guard then leaves the main agent lazy, and the later ensureMainAgent() call uses the local seed, so the remote environment is connected but Bash and file tools run locally. Create or bind the main agent whenever the effective non-local environment is selected, not only when a profile binding was supplied.
Useful? React with 👍 / 👎.
| async readBytes(path: string, n?: number, offset = 0): Promise<Uint8Array> { | ||
| if (n !== undefined) { | ||
| const result = await this.readRange(path, offset, n); | ||
| return decodeBase64(result.dataBase64); |
There was a problem hiding this comment.
Chunk explicit remote byte-range reads
For a remote file between 1 MiB and file history's 4 MiB limit, FileHistoryService.readCurrent calls readBytes(absolute, info.size + 1). This branch forwards that value as one fs/readFile.maxBytes, but the executor rejects values above FS_READ_FILE_MAX_BYTES (1 MiB), and file history catches the failure as unreadable; remote edits to such files therefore get no history or undo snapshot. Split explicit n reads into protocol-sized chunks before issuing the RPC.
Useful? React with 👍 / 👎.
| - **Switch with `change_environment`**: pass an environment `id` (`local` or a declared id) and optionally a `cwd` (falls back to the declaration's `defaultCwd`). The target connects eagerly — a connection or `cwd` validation failure is reported immediately and changes nothing — and the switch itself takes effect as soon as the tool call completes: the next tool call in the same turn already runs on the new environment. When other tool calls are still executing in parallel, the call instead fails with an error naming the in-flight count — retry once they have finished, and their work is never yanked mid-flight. The reminder with the new environment's details arrives with the next turn. | ||
| - **Create a temporary environment with `connect`**: pass a launcher spec — `{ type: "ssh", host: "..." }`, `{ type: "docker", container: "..." }`, or `{ type: "command", command: "...", args: [...] }`, with an optional `id`. The environment connects right away and is registered in the workspace like a declared one, but nothing is written to `config.toml`: a temporary environment vanishes when the process exits, and a session resumed onto it finds it gone. A dropped connection can be retried in the same process. | ||
| - **Bind a subagent with the `environment` parameter**: the `Agent` tool accepts an optional `environment` id; the spawned subagent binds to that environment (at its `defaultCwd`) instead of inheriting the parent's binding. Resumed subagents keep their own binding. |
There was a problem hiding this comment.
Remove unsupported environment-switching instructions
This phase-one build does not register change_environment, the environment argument of Agent, or an /environment TUI command, but the new guide advertises all three as usable. Users who enable the documented flag or follow these switching steps will find the actions unavailable; remove or revise these sections to describe the creation-time binding that this commit actually ships.
Useful? React with 👍 / 👎.
| } else if (targetKey === 'environments' && isPlainObject(value)) { | ||
| result[targetKey] = transformPlainObject(value); |
There was a problem hiding this comment.
Accept snake_case fields in environment declarations
When users follow the configuration guide's TOML naming convention and set fields such as default_cwd or remote_bin inside [environments.<id>], this branch only camel-cases the outer environments table. The strict environment-entry schema then rejects those nested snake_case keys, making the whole config invalid instead of creating the declared remote environment. Transform each declaration entry as well, or explicitly support these keys before validation.
Useful? React with 👍 / 👎.
Resolve conflicts by keeping main's revert intent on behavior and this branch's environment plumbing on structure: drop the realpath re-resolution checks from the agent file tools, the hardened git-config probe from the background git service, the trust argument from WorkspaceDirsService, and the local.toml trust gating, while retaining the acquire/pinnedGeneration/ EnvironmentWorkspaceView pipeline. Delete the test files main removed and the test cases covering the reverted hardening; keep the read-only environment documentation additions alongside the restored local.toml behavior.
Honor main's trust-env override (#4059) on top of the environment refactor: keep the AsyncEmitter trust service alongside KIMI_CODE_TRUST_WORKSPACE, and pass bootstrap at the shared WorkspaceTrustService construction site.
- remove phase-2 leftovers with no production callers: ephemeral connector,
declare(), EnvironmentRegistry.replace(), agentEnvironment.reconnect(),
MCP envVars, and fileHistory cross-environment capture
- simplify the remote protocol: exempt process/write from the per-request
timeout and drop the writeId retry/dedup chain, merge process/terminate
into process/signal with TERM->KILL escalation (keeping the
terminate-before-start tombstone), drop the initialized notification and
the client-side in-flight queue
- launch the docker executor via sh -c like ssh and delete the tilde home
probe and its local-runner plumbing
- unify {environmentId,cwd} key encoding behind environmentBindingId
- fail session creation explicitly when the environment is unavailable
instead of silently falling back to the local controller
- planService reuses environmentTempTarget with a local-only tmpdir fallback
- sync the remote README and affected docs
|
@codex review |
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
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". |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3ec7e2e7e8
ℹ️ 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".
| @@ -123,7 +123,7 @@ export async function registerApiV1Routes( | |||
| core, | |||
| { sessionEventCursor: (sessionId) => opts.broadcaster.getCursor(sessionId) }, | |||
| ); | |||
| registerRuntimeRoutes(apiV1 as unknown as Parameters<typeof registerRuntimeRoutes>[0], core); | |||
| registerEnvironmentRoutes(apiV1 as unknown as Parameters<typeof registerEnvironmentRoutes>[0], core); | |||
There was a problem hiding this comment.
Keep the published runtime endpoints available
Existing REST clients that call GET or POST /api/v1/sessions/{id}/runtime now receive a 404: this replaces the runtime route registration, while the replacement registers only the new environment GET routes. Those endpoints were part of the documented API, so retain compatibility aliases (or version the API) before removing them; otherwise already released desktop/web and direct REST clients cannot read or switch their bindings.
AGENTS.md reference: AGENTS.md:L111-L113
Useful? React with 👍 / 👎.
| async getEnvironment(): Promise<AgentEnvironmentBinding> { | ||
| this.ensureOpen(); | ||
| return this.rpc.getRuntime({ sessionId: this.id }); | ||
| return this.rpc.getEnvironment({ sessionId: this.id }); | ||
| } |
There was a problem hiding this comment.
Retain Session runtime methods as compatibility aliases
SDK callers compiled against the previous release use session.getRuntime() and session.switchRuntime(runtimeId), but this replacement exposes only getEnvironment() and removes the runtime binding type and switch method. Updating to this release therefore breaks existing SDK integrations at compile time (and removes their switching capability); preserve deprecated runtime aliases or make this an explicitly versioned breaking release.
AGENTS.md reference: AGENTS.md:L111-L113
Useful? React with 👍 / 👎.
| Read a text file from the local filesystem. | ||
|
|
||
| The path may be a `kimi-file://` attachment reference. Its bytes come from the current session's storage, independently of the workspace runtime. Next Read keeps the reference so pagination also works after a fork. For a binary attachment, the error includes a server-local path when available; a converter must be able to access that filesystem. ReadMediaFile accepts the same reference for images and videos. | ||
| The path may be a `kimi-file://` attachment reference. Its bytes come from the current session's storage, independently of the workspace environment. Next Read keeps the reference so pagination also works after a fork. For a binary attachment, the error includes a server-local path when available; a converter must be able to access that filesystem. ReadMediaFile accepts the same reference for images and videos. |
There was a problem hiding this comment.
Describe Read as targeting the bound environment
For every remote-bound session, ReadTool now resolves its filesystem from the active environment, but its prompt still begins by telling the model it reads from the “local filesystem.” This description is delivered to the very agents that must operate on remote target paths, so it conflicts with the binding reminder and can lead them to give incorrect path guidance; describe the bound environment/target filesystem instead.
AGENTS.md reference: AGENTS.md:L107-L109
Useful? React with 👍 / 👎.
| `resume could not connect environment ${boundEnvironmentId}; session ${sessionId} cannot be loaded until the environment is available`, | ||
| { error }, | ||
| ); | ||
| throw error; |
There was a problem hiding this comment.
Preserve disconnected bindings when resuming
When a persisted remote environment is offline, this catch rethrows the connection error instead of materializing the session with its saved disconnected binding. SessionManager.resume therefore rejects and the TUI reports “Failed to resume session,” even though the new remote-environment guide promises that the session remains open and its first tool call retries; users cannot inspect or replay an existing remote session during an outage.
Useful? React with 👍 / 👎.
- drop protocol fields no client sends (read/write followSymlinks, createdAtMs, pipeStdin) and their server branches - remove product code only referenced by tests: Program.suggestFiles, Program.onDidChangeTrust, MCP originalsDir, kap-server resolveWorkspaceInstance, unused SlashCommandHost/KimiTUI members - merge duplicated logic: connect+acquireWhenReady into EnvironmentRegistry, prompts/skills media targets into environmentMediaTargets, realpath-existing-prefix into one helper, decodeBase64 reuse, single rg target mapping, node-sdk declarations via IEnvironmentDeclarationService - simplify fireAndForgetTrigger, sendWrite, inline launcherLabel and environmentEntryType, drop redundant exports EnvironmentRegistry.acquireWhenReady now triggers connect like the agent-level path already did; the two providerFactory tests pin the immediate post-failure error via acquire() instead.
|
追加一轮 KISS 清理(a5d975563,35 文件 +144/-447):
验证:全仓 typecheck 与 lint 通过;agent-core-v2 / kap-server / node-sdk / apps-kimi-code 全量测试通过,除以下 PR 基线(3ec7e2e)已存在的问题(已用基线 worktree 对照确认与本提交无关):
|
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a5d9755637
ℹ️ 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".
| try { | ||
| stat = await this.hostFs.stat(root); | ||
| } catch (error) { | ||
| const code = (unwrapErrorCause(error) as NodeJS.ErrnoException | undefined)?.code; | ||
| if (code === 'ENOENT' || code === 'ENOTDIR') { | ||
| throw new Error2(ErrorCodes.FS_PATH_NOT_FOUND, `workspace root ${root} does not exist`); | ||
| } | ||
| throw error; | ||
| } catch { | ||
| stat = undefined; | ||
| } |
There was a problem hiding this comment.
Reject invalid local workspace roots
When an SDK caller supplies a missing workDir (or a path below a regular file), this catch converts the failed local stat into a successful workspace registration; SDKRpcClientV2.createSession forwards that value to ISessionManager.create without another filesystem check. Before this change, createOrTouch returned fs.path_not_found, so callers can now receive and persist a session/workspace that only fails later when it first accesses the filesystem. Keep validating the local workspace root at the SDK/session boundary while allowing environmentCwd to be the separate remote target path.
AGENTS.md reference: AGENTS.md:L99-L101
Useful? React with 👍 / 👎.
| cwd: z.string().optional(), | ||
| executor: z.enum(['local', 'kaos']).optional(), | ||
| runtime_id: z.string().min(1).optional(), | ||
| environment_id: z.string().min(1).optional(), |
There was a problem hiding this comment.
Preserve the
runtime_id MCP configuration alias
Existing mcp.json stdio entries using runtime_id are parsed through McpServerConfigSchema.parse in configLoader.ts, but this replacement leaves that legacy key unknown and Zod strips it. McpConnectionManager then falls back to local, so an MCP server that previously ran in a configured remote runtime silently starts on the host after upgrade. Accept or migrate runtime_id as a deprecated alias (or make this an explicit breaking release) rather than changing the execution target without an error.
AGENTS.md reference: AGENTS.md:L119-L121
Useful? React with 👍 / 👎.
| @@ -6,7 +6,7 @@ import { | |||
| } from '@moonshot-ai/agent-core-v2/os/interface/terminal'; | |||
|
|
|||
| export const createTerminalRequestSchema = engineCreateTerminalRequestSchema.extend({ | |||
| runtime_id: z.string().min(1).optional(), | |||
| environment_id: z.string().min(1).optional(), | |||
There was a problem hiding this comment.
Preserve
runtime_id for terminal creation requests
Previously released clients send runtime_id to POST /sessions/{id}/terminals; the replacement schema strips that now-unknown field, and registerTerminalsRoutes defaults the missing environment_id to local. A client requesting a terminal in a non-local runtime therefore receives a successful terminal on the host instead of an incompatibility error or the requested target. Continue accepting runtime_id as an alias until the REST API is versioned.
AGENTS.md reference: AGENTS.md:L119-L121
Useful? React with 👍 / 👎.
| const result = z.object({ environment_id: z.string().min(1).optional() }).passthrough().safeParse(req.body ?? {}); | ||
| if (!result.success) { | ||
| reply.send(errEnvelope(ErrorCode.VALIDATION_FAILED, 'request body must be an object', req.id)); | ||
| return; | ||
| } | ||
| const { runtime_id, ...request } = result.data; | ||
| const runtimeId = runtime_id ?? 'local'; | ||
| const { environment_id, ...request } = result.data; | ||
| const environmentId = environment_id ?? 'local'; |
There was a problem hiding this comment.
Preserve
runtime_id for session filesystem requests
Previously released clients select a non-local target for every session filesystem action with runtime_id. This new selector leaves that key as ordinary request data and defaults the missing environment_id to local; the per-action Zod schemas then discard runtime_id. A remote fs:read, fs:mkdir, search, or git request can therefore succeed against the host filesystem after upgrade. Accept runtime_id as a deprecated alias or reject it explicitly instead of silently changing the target.
AGENTS.md reference: AGENTS.md:L119-L121
Useful? React with 👍 / 👎.
| sessionId: input.id, | ||
| workDir, | ||
| additionalDirs: input.additionalDirs, | ||
| environmentId: input.environmentId, | ||
| environmentCwd: input.environmentCwd, |
There was a problem hiding this comment.
Await remote provider registration before session creation
When an SDK caller constructs SDKRpcClientV2 and immediately creates a session using a declared remote environment (or the configured default), provider attachment can still be waiting on the environment/config readiness path. This call then reaches ISessionManager.create before the declaration has been registered, so the valid environment is reported unavailable; retrying later succeeds. Await remoteEnvironmentProvider before resolving or creating an environment-bound session so configured targets do not fail based on timing.
AGENTS.md reference: AGENTS.md:L99-L101
Useful? React with 👍 / 👎.
Released clients and mcp.json entries that only set runtime_id otherwise silently fall back to local after the runtime-to-environment rename. Cover MCP stdio config, terminal creation, and the session fs/search/suggest/download routes; environment_id wins when both keys are present.
…session use ISessionManager.create again rejects a missing or non-directory workDir with fs.path_not_found for local bindings; remote bindings keep deferred root validation, and createOrTouch stays lenient. SDKRpcClientV2 awaits the remote environment provider attach before create/resume/fork/reload and temporary resume, so declared or persisted remote environments no longer report unavailable or fall back to a local controller based on registration timing.
|
@codex review |
|
Codex Review: Didn't find any major issues. You're on a roll. Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
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". |
Cold resume reads the binding from state.json instead of scanning the agent wire. An unregistered or unreachable remote binding fails the resume instead of opening a local controller. Provider attach is part of EnvironmentService.ready, so create and resume wait in one place.
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep them coming! Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
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". |
Requirement or Bug
支持将 agent 会话在创建时绑定到 SSH 或 Docker 执行环境。本 PR 是 #3856 的 phase 1,不包含会话内环境切换。
Bug Reproduction Steps
N/A
Root Cause
N/A
Code Changes
ACP 仍保留窄化后的
bind(environmentId, cwd):ACP client terminal 是进程内动态注册的 session environment,new/load/resume/fork 都需要把 main agent 绑定到该环境。不在本 PR 的 phase 2 surface:
change_environment/connectagent tools、Agent tool 的environment参数、/environment管理器、REST/SDK switch 与 declare endpoints,以及 binding switch undo。Behavior Changes and Affected Users
[environments]、--environment或 session-create 参数绑定 SSH/Docker/command 环境--environment/newdisconnected和连接错误,不额外写 transcript notice;下一次工具调用尝试重连ssh_hosts/sshHosts受影响模块与验证:
agent-core-v2:环境 registry/binding、远程协议、session/subagent 生命周期、Plan 文件恢复;本轮 focused tests 368 项通过。apps/kimi-code:启动、/new、footer 与失败进度;本轮 focused tests 417 项通过。acp-server:动态 session environment binding 保持不变;149 项测试通过。node-sdk:session 环境状态与生命周期;71 项测试通过。kap-server:环境 REST route 与 API snapshot;10 项测试通过。agent-core-v2、kap-server、node-sdk、klient、acp-server、apps/kimi-codetypecheck 通过;repository lint 与 no-comments check 通过。已知待 undraft 前处理:其余
docs/与现有 changesets 仍有来自 #3856 的 phase-2 描述,需要按 phase 1 重新收口;EnvironmentBinding.cwd尚未在 bind 时 canonicalize。Checklist
gen-changesetsskill; no additional changeset was added. The fail-closed resume and session-meta binding are unreleased phase-1 behavior, not a change from the last release.gen-docsskill; the English and Chinese server API references match the phase-1 response shape.