Repository navigation
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 383 commits into
Conversation
EnvironmentError from registry acquire (unknown or disconnected environment id) fell through sendMappedError to the INTERNAL_ERROR fallback, returning 50001 with the raw stack string. Route it through the same environmentErrorCode mapping the environment routes use: unknown ids answer ENVIRONMENT_NOT_FOUND, disconnected environments ENVIRONMENT_UNAVAILABLE, with no stack in the envelope.
The footer badge already hides the local git state for remote-bound sessions, but statusLinePayload still fed the local cache branch into the custom status line JSON, showing the host branch while the session runs remote. Share the badge's remote check so the payload carries gitBranch null under the same condition.
stdio is the only supported transport, so the flag was pure ceremony. The light pre-dispatch in src/main.ts now accepts the bare [exec-server] argv shape alongside the explicit [exec-server, --listen, stdio] one, and the hidden Commander subcommand's --listen option defaults to stdio so both entry paths agree. --listen <other> still exits 2, and unknown options or excess args still fail in Commander instead of silently starting. The explicit spelling keeps working for existing launcher configs; the remote-exec client keeps emitting it since a new client may meet an old executor.
…cquire failure Route the three acquire sites in agentsMdReminderService (ensureSeeded, targetDirs, probeDir) through the M53 acquireEnvironmentLease helper so a disconnected environment binding abstains per-site instead of relying on the tool-did-execute hook's blanket catch. The helper gains an optional required-capabilities param; permission-policy callers stay untouched.
…o a view On a successful environment binding commit to a previously unvisited (environmentId, cwd) view, probe the AGENTS.md path chain for the new cwd on the target filesystem by reusing the agentsMdReminder probing machinery (loadAgentsMdDetailed) and inject a path-only project-context reminder (new project_context variant) that names the view, supersedes earlier directories' guidance, and lists the paths for the agent to read with its tools. The initial binding counts as visited; revisiting a view injects nothing. The existing environment reminder keeps its machine-identity semantics and may co-fire unmerged.
Home-level AGENTS.md files (brand home, ~/.agents) may already be reflected in the system prompt for sessions that started local, so claim the listed paths 'may not be reflected in your system prompt' instead of 'were not included'.
Add an agent_environment_tools experimental flag (default off). When enabled, the main agent gains a tool group for environment switching: - change_environment(id, cwd?) switches the session binding through the same machinery as /environment (connect, cwd validation, undo participant), taking effect at the turn boundary with the environment reminder queued for the next turn. Rejected in plan mode; asks in manual/yolo permission modes, allowed without asking in auto mode. - connect creates a temporary environment from a launcher spec (ssh/docker/command) without writing it to config, registered in the workspace registry so it does not persist. - the system prompt gains an available-environments section so the agent knows which ids exist. - the Agent tool gains an optional environment parameter to bind the spawned subagent to a chosen environment instead of inheriting the parent's binding. The tool group is not registered while tower mode is active.
- re-fetch the environment view after connect in the subagent spawn binding so the cwd fallback reads the connected host, not the stale pending view - reject the Agent tool environment parameter at execution time when the agent_environment_tools flag is off (resumes still ignore it) - dispose the executor connection when registering an ephemeral environment fails - fix a never-intersection type collapse in the spawn test
…record Remove boundCwd/reroot from the shared environment record and interface: a cwd switch now only updates the switching session's own binding, so it no longer replaces the shared connection generation or drains other sessions' tracked resources. Session resume/restore no longer reroots or background-connects the persisted environment, and session create no longer reroots the shared environment to the new session's cwd.
…solation Resuming a session no longer reconnects the restored remote binding in the background; the environment stays disconnected until an explicit reconnect. Rewrites the resume paragraph in both locales (the previous 'one exception' wording contradicted the new behavior; content removed for staleness) and drops the stale 'background reconnect failure after resume' trigger example from the refreshEnvironmentSlot doc comment.
…r own cwd Resume no longer connects a persisted remote environment; the first environment acquire connects on demand. Process calls carry the session's own cwd as an explicit spawn option, and remote program generations are keyed and rooted per session binding cwd instead of the local workspace root.
Narrow tool outputs to string before constructing errors and pass the full ExecutableToolContext (turnId, toolCallId) to execute.
…nection Business calls (fs/*, process/start, process/write, ...) now carry a per-request timeout that rejects only the stalled call with RequestTimeoutError; the connection and its generation survive, and a late response is discarded via a bounded expired-id set instead of faulting the connection as an unknown-id response. Control calls (environment/status health check) keep the kill-the-connection timeout, so a genuinely wedged executor still drains the environment exactly like a transport drop.
![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: b06fdb692f
ℹ️ 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".
| stagedDiscarded = true; | ||
| if (attachmentsSink === undefined || stagedPaths.size === 0) return; | ||
| const sink = attachmentsSink; | ||
| await Promise.all([...stagedPaths].map((path) => sink.remove(path).catch(() => undefined))); |
There was a problem hiding this comment.
When two prompt submissions materialize the same file ID/name or identical inline payload, they use the same deterministic target path. If request A writes that path, request B reuses it because its size matches, and then A fails during later profile/model/tool-policy validation, A's discardStaged() removes the file even if B has already enqueued successfully. B still returns success but its attachment path is now missing when the agent tries to read it; use request-unique staging paths or reference-count ownership before removing shared files.
AGENTS.md reference: AGENTS.md:L118-L120
Useful? React with 👍 / 👎.
| get agentProfiles(): IWorkspaceAgentProfileLoader { return this.requireGeneration().agentProfiles; } | ||
| get sessionControllerGeneration(): string { return this.requireGeneration().id; } | ||
| get state(): IWorkspaceStateService { return this.requireGeneration(LOCAL_ENVIRONMENT_ID).state; } | ||
| get dirs(): IWorkspaceDirs { return this.requireGeneration(LOCAL_ENVIRONMENT_ID).dirs; } |
There was a problem hiding this comment.
When /add-dir or Session.addAdditionalDir() is used by an SSH, Docker, or command-bound session, the SDK reaches workspace.program.dirs, and this getter now always returns the local generation even though the session was seeded from a separate remote generation's IWorkspaceDirs. A remote-only path is therefore checked on the host and rejected; if the path happens to exist on both machines, the call reports success while only the local roots change, so the remote session's tools continue rejecting the directory. Resolve the session's bound generation before applying the update.
AGENTS.md reference: AGENTS.md:L118-L120
Useful? React with 👍 / 👎.
|
@codex review |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1b8dcfad22
ℹ️ 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".
| environments: { | ||
| ...resolver, ready: this.environments.ready, |
There was a problem hiding this comment.
When a watched remote-environment declaration changes its host, container, or command while a session is live, applyLauncher rotates and disconnects the registry generation, but this controller retains the old generation's instructions, skills, and profiles while supplying agents an ID-based resolver that acquires the new generation. The next tool call therefore reconnects and executes on the new target using project context loaded from the old target, silently mixing policy and content across machines; pin the resolver to the controller generation or invalidate/rematerialize affected sessions.
AGENTS.md reference: AGENTS.md:L118-L120
Useful? React with 👍 / 👎.
| @@ -22,12 +22,11 @@ const mcpServerCommonFields = { | |||
|
|
|||
| export const mcpServerStdioConfigSchema = z.object({ | |||
| transport: z.literal('stdio'), | |||
| runtime_id: z.string().min(1).optional(), | |||
| environment_id: z.string().min(1).optional(), | |||
There was a problem hiding this comment.
executor in the Klient MCP schema
When an existing Klient or IPC caller sends a stdio MCP configuration containing the supported legacy executor: 'local', this narrowed Zod schema strips the property and its inferred facade input no longer accepts it. The engine schema and client-stdio compatibility test explicitly retain this value, so restore the optional field rather than narrowing the public wire contract.
AGENTS.md reference: packages/klient/AGENTS.md:L14-L18
Useful? React with 👍 / 👎.
| export function isExecServerArgv(argv: readonly string[]): boolean { | ||
| return hasExecServerShape(argv.slice(1)) || hasExecServerShape(argv.slice(2)); |
There was a problem hiding this comment.
In the supported no-script argv shape, any invocation whose arguments after the first option equal an executor shape is misclassified because argv.slice(2) is checked unconditionally. For example, kimi -p exec-server becomes ['exec-server'] after that slice and starts the stdio executor instead of running print mode with the prompt exec-server; it can then wait on stdin or exit successfully without producing the requested answer. Only accept the offset-two form when argv[1] is the duplicated executable/script slot.
Useful? React with 👍 / 👎.
| } from './question'; | ||
| export { projectHistoryToSessionUpdates } from './replay'; | ||
| export { AcpRuntimeProviderFactory } from './acp-terminal'; | ||
| export { AcpEnvironmentProviderFactory } from './acp-terminal'; |
There was a problem hiding this comment.
Downstream embeddings that import the previously exported AcpRuntimeProviderFactory from @moonshot-ai/acp-server now fail to type-check or load because the package barrel exposes only the renamed class and provides no compatibility alias. Keep a deprecated adapter or alias for the old export while introducing AcpEnvironmentProviderFactory, since this package is an external contract surface.
AGENTS.md reference: AGENTS.md:L110-L112
Useful? React with 👍 / 👎.
| @@ -41,11 +41,11 @@ export function registerBusinessSnapshotRoutes( | |||
| () => sessionWorkspaceAssociation(core, requestParams(req)['session_id'] ?? ''), | |||
| callTimeoutMs, | |||
| )); | |||
| app.get(`${basePath}/session/:session_id/agent/:agent_id/runtime-binding`, async (req, reply) => | |||
| app.get(`${basePath}/session/:session_id/agent/:agent_id/environment-binding`, async (req, reply) => | |||
There was a problem hiding this comment.
Existing desktop, extension, or inspector clients that call /api/v1/debug/session/{session_id}/agent/{agent_id}/runtime-binding now receive a 404 because only the renamed environment-binding route is registered and there is no compatibility alias. Retain the old path while introducing the new spelling, since the checked-in kap-server API surface is an externally consumed contract.
AGENTS.md reference: AGENTS.md:L110-L112
Useful? React with 👍 / 👎.
| @@ -242,6 +245,9 @@ export function registerSessionsRoutes( | |||
| const handle = await core.accessor.get(ISessionManager).create({ | |||
| workspaceId: touched.id, | |||
| workDir, | |||
| environmentId: body.environment_id, | |||
| environmentCwd: body.environment_cwd, | |||
| ensureMainAgent: body.environment_id !== undefined && body.environment_id !== 'local', | |||
There was a problem hiding this comment.
When a REST caller creates a session with environment_id: "local" and an environment_cwd, the schema accepts the request but this condition prevents creation of the main agent that would persist and apply that cwd. The lazily created agent is subsequently seeded with the default local binding, so the request succeeds while silently ignoring the requested working directory; materialize the binding when a cwd is supplied or reject this combination.
AGENTS.md reference: AGENTS.md:L118-L120
Useful? React with 👍 / 👎.
| const cwd = | ||
| this.config.cwd === undefined | ||
| ? base | ||
| : path.resolve(base, this.config.cwd); |
There was a problem hiding this comment.
When a project-root .mcp.json stdio entry selects a non-local environment_id, the config loader has already converted its cwd—including an omitted cwd—to the host's absolute Git-root path. Resolving that absolute value here preserves paths such as /Users/me/project on the SSH or Docker target, so the MCP process fails to spawn unless the target coincidentally has the same path; preserve the unnormalized cwd/origin or skip host normalization for remote entries.
AGENTS.md reference: AGENTS.md:L114-L116
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1f65ccc3f8
ℹ️ 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".
| environmentId: string, | ||
| required: readonly EnvironmentCapability[], | ||
| ): EnvironmentFsScope { | ||
| const lease = accessor.get(IEnvironmentService).acquire({ environmentId }, required); |
There was a problem hiding this comment.
When a filesystem caller in workspace B supplies an environmentId declared only by trusted workspace A, this helper ignores its workspaceId and acquires the app-wide registry entry directly. The new kap-server session/workspace filesystem routes and SDK suggestFiles all use this helper, so a client that guesses the ID can read or search workspace A's SSH/Docker target; call IEnvironmentDeclarationService.assertAllowed(workspaceId, environmentId) before acquisition.
AGENTS.md reference: AGENTS.md:L127-L129
Useful? React with 👍 / 👎.
| const agent = await this.agentScope(input.sessionId); | ||
| const lease = await agent.accessor.get(IEnvironmentService).acquireWhenReady({ environmentId: input.runtimeId }); | ||
| try { | ||
| const binding = agent.accessor.get(IAgentEnvironmentBindingService).bind(input.runtimeId); |
There was a problem hiding this comment.
When a legacy SDK caller switches to a declared remote runtime with defaultCwd, this binds only its ID and drops that cwd. The binding service consequently falls back to the host-local session cwd for relative tools and persists the incomplete binding, so operations can fail or target the wrong remote directory after both the switch and a resume; resolve the declaration's default cwd before binding.
AGENTS.md reference: AGENTS.md:L127-L129
Useful? React with 👍 / 👎.
| if (this.creationEnvironment?.environmentId !== undefined && this.creationEnvironment.environmentId !== 'local') { | ||
| // The remote session is created eagerly, so the session-less | ||
| // "No session yet" notice would contradict the live banner. | ||
| void this.ensureSession(); |
There was a problem hiding this comment.
With a configured remote default or --environment, this fire-and-forget call lets initMainTui() finish before session creation connects. If the target is unreachable, lazyCreateSession() catches the error and returns undefined, after which the outer startup path still reports “Ready” and leaves the UI sessionless, contrary to docs/en/guides/remote-environment.md, which says connection failures abort startup; await the eager creation and propagate failure or avoid reporting successful startup.
Useful? React with 👍 / 👎.
- Program returns to one local generation per workspace; remote project content (fs, git, AGENTS.md, project skills/profiles) is built per session through a workdir factory and disposed with the session. - SessionLifecycleService connects the target, validates the cwd, persists environmentId/environmentCwd in session meta at creation, and reads them back on resume; SessionManager only resolves the creation environment and routes to one controller per workspace. - Remote environments expose fs/process facades that survive reconnects. - Drop project-level .kimi-code/environments.toml with its trust gating, conflict detection, and environmentProject provenance; declarations come from the user [environments] config only. - Agent environment binding keeps a single replayable state key and no longer writes session meta.
- Seed each session with ISessionWorkdir (its environment and fs service); remote sessions get their session-owned workdir fs, local sessions the workspace fs. Remove createEnvironmentFs and its fabricated workspace context. - kap-server session fs actions and downloads use the session's own environment; workspace and root suggest/search routes run locally through a private rooted fs. Drop the runtime_id / environment_id parameters from these routes. - SDK suggestFiles takes a sessionId instead of an environment binding; the TUI completes remote @ mentions through the live session. - kimi-inspect stops sending environment_id; docs updated.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1fbd3b97f7
ℹ️ 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".
| lease: { track: (resource) => resource, dispose: () => {} }, | ||
| }; | ||
| } | ||
| const lease = core.accessor.get(IEnvironmentService).acquire({ environmentId }, ['fs']); |
There was a problem hiding this comment.
When /fs::content or /fs::mkdir is the first operation against a declared SSH, Docker, or command environment—or follows a dropped connection—the entry is pending or disconnected, and acquire() rejects it without invoking connect(). The new environment_id support therefore cannot open a cold target and continues failing after a disconnect until an unrelated session or tool reconnects it; use acquireWhenReady() here so both filesystem routes establish the explicitly selected environment on demand.
Useful? React with 👍 / 👎.
| const expectedGeneration = pinnedGeneration(inspected); | ||
| const view = new EnvironmentWorkspaceView(inspected, { | ||
| workDir: this.workspaceCtx.workDir, | ||
| additionalDirs: [...this.workspaceCtx.additionalDirs, ...this.skillCatalog.catalog.getSkillRoots()], |
There was a problem hiding this comment.
When an SSH or Docker session uses a built-in, user, explicit, extra, or plugin skill, Program.createWorkdir() merges those host-loaded sources with target-side project skills, so getSkillRoots() contains paths from both filesystems. Passing every root through this target EnvironmentWorkspaceView makes Read, Glob, Grep, Write, and Edit interpret host skill paths on the remote machine; skills that reference bundled scripts or templates through their expanded skill directory therefore fail, while a coincidentally matching remote path is incorrectly treated as an allowed skill root. Preserve each root's filesystem provenance or materialize host skills on the target instead of mapping every root remotely.
AGENTS.md reference: AGENTS.md:L127-L129
Useful? React with 👍 / 👎.
| const env = | ||
| this.options.environmentId === LOCAL_ENVIRONMENT_ID | ||
| ? mergeStdioEnv(this.config.env) | ||
| : (this.config.env ?? {}); |
There was a problem hiding this comment.
When a remotely executed stdio MCP configuration supplies HTTP_PROXY or HTTPS_PROXY, this branch bypasses mergeStdioEnv(), so the child never receives the derived NODE_USE_ENV_PROXY, case-normalized aliases, or reconciled NO_PROXY values. Node MCP servers that previously honored the configured proxy can therefore connect directly or fail behind a corporate proxy; normalize the target-side configured environment without merging the host process environment.
AGENTS.md reference: AGENTS.md:L127-L129
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a7a53311e3
ℹ️ 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".
| export class EnvironmentSetBinding extends AgentEvent2<z.infer<typeof environmentSetBindingSchema>> { | ||
| static override readonly type = 'environment.set_binding'; | ||
| static override readonly durable = true; | ||
| static override readonly schema = environmentSetBindingSchema; |
There was a problem hiding this comment.
When a session is created or resumed on a remote environment and the user later rolls back to the previous release, this new durable record is still written under wire protocol version 1.5. The older dispatcher treats environment.set_binding as an unknown record and skips it, while its session lifecycle ignores the new metadata fields and seeds the main agent with the local runtime; reopening the session can therefore silently execute commands intended for SSH or Docker on the host workspace. Bump the persisted protocol/version so older builds fail closed, or encode a compatible binding that cannot fall back to local.
AGENTS.md reference: AGENTS.md:L123-L125
Useful? React with 👍 / 👎.
| private applySessionWorkDir(binding: EnvironmentBinding): void { | ||
| if (this.scopeContext.agentId !== MAIN_AGENT_ID) return; | ||
| this.workspaceContext.setWorkDir(binding.cwd ?? this.session.cwd); |
There was a problem hiding this comment.
When an SSH or Docker session uses a target cwd such as /remote/app while its host workspace is /Users/me/app, this stores /remote/app in the session-wide workspace context. The terminal REST route explicitly defaults environment_id to local, but SessionTerminalService derives that local terminal's cwd from this shared state, so it tries to spawn the host PTY in /remote/app and fails—or opens an unrelated host directory when that path happens to exist. Preserve the host workspace roots for local terminals, or key the workspace state by environment.
AGENTS.md reference: AGENTS.md:L127-L129
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 591b2a5f8c
ℹ️ 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".
| ({ content: agentsMd, paths: agentsMdPaths } = await loadAgentsMdDetailed( | ||
| { fs: lease.environment.fs!, homeDir }, | ||
| workDir, | ||
| this.bootstrap.homeDir, | ||
| )); |
There was a problem hiding this comment.
When /init runs in a remote-bound session, this reload reads every personal source through the remote filesystem: the local bootstrap.homeDir is interpreted on the target, and ~/.agents/AGENTS.md comes from the target account. The normal remote workdir loader deliberately uses the local filesystem for personal instructions and the target filesystem only for project instructions, so the completion reminder can omit the user's active local directives and inject remote-home directives that were absent from the system prompt. Preserve that split filesystem provenance here as well.
AGENTS.md reference: AGENTS.md:L127-L129
Useful? React with 👍 / 👎.
| if ( | ||
| relativePath !== '' && | ||
| !relativePath.startsWith('..') && | ||
| !semantics.isAbsolute(relativePath) |
There was a problem hiding this comment.
For an in-workspace file whose basename starts with two dots, such as /workspace/..env, relative() returns ..env; this condition treats it as outside the workspace and stores the absolute path as the history key. Before this change the path was normalized to ..env, so contentAt(turnId, '..env') now returns no entry and change payloads unexpectedly expose an absolute path. Reject only .. itself or a ..${separator} prefix rather than every relative path beginning with ...
AGENTS.md reference: AGENTS.md:L111-L113
Useful? React with 👍 / 👎.
| } else if (targetKey === 'environments' && isPlainObject(value)) { | ||
| result[targetKey] = transformRecord(value, transformPlainObject); |
There was a problem hiding this comment.
When loadRuntimeConfigSafe reads multiple [environments.*] entries and any one entry is invalid, this new record section produces a validation path such as environments.bad, but salvageConfigData still treats only providers and models as entry-keyed. It therefore deletes the entire environments section, including a valid configured default, and session creation falls back to local instead of the requested remote target. Add environments to the entry-level salvage handling so one malformed sibling cannot discard valid execution-target configuration.
AGENTS.md reference: AGENTS.md:L131-L133
Useful? React with 👍 / 👎.
| for (let offset = 0; offset < data.byteLength; offset += FS_WRITE_FILE_CHUNK_BYTES) { | ||
| await this.writeMode(path, data.subarray(offset, offset + FS_WRITE_FILE_CHUNK_BYTES), offset === 0 ? mode : 'append'); |
There was a problem hiding this comment.
For payloads larger than 1 MiB, one logical writeText or writeBytes operation becomes an independently dispatched truncate followed by multiple append RPCs, while the executor runs filesystem requests concurrently. If parallel agents or prompt materializations write the same path, their chunks can interleave after either truncate, leaving a successful operation with bytes from both payloads rather than normal last-writer-wins behavior. Use an upload handle/temporary file plus rename, or otherwise serialize the complete write per path before exposing it.
AGENTS.md reference: AGENTS.md:L131-L133
Useful? React with 👍 / 👎.
| if (environment === undefined) { | ||
| return { ...binding, type: binding.environmentId === 'local' ? 'local' : 'command', status: 'disposed' }; |
There was a problem hiding this comment.
When a session remains bound to a declared SSH or Docker environment after its provider is disposed or fails to register, environment is undefined and this branch reports its public type as command. The checked-in provider-disposal flow already leaves such a binding with status disposed, so getEnvironmentState() returns misleading type information to SDK and TUI consumers precisely when diagnosing the unavailable target. Resolve the declaration before this early return and derive the disposed entry's actual type.
AGENTS.md reference: AGENTS.md:L123-L125
Useful? React with 👍 / 👎.
# Conflicts: # apps/kimi-code/test/cli/v2-run-print.test.ts # docs/en/reference/server-api.md # docs/zh/reference/server-api.md # packages/agent-core-v2/src/agent/tools/os/glob/globTool.ts # packages/agent-core-v2/src/agent/tools/os/grep/grepTool.ts # packages/agent-core-v2/src/app/mcpManagement/mcpManagementService.ts # packages/agent-core-v2/src/app/sessionManager/sessionManagerService.ts # packages/agent-core-v2/src/environment/fakeEnvironment.ts # packages/agent-core-v2/src/environment/localEnvironment.ts # packages/agent-core-v2/src/features/plan/planService.ts # packages/agent-core-v2/src/features/plan/tools/enter-plan-mode/enterPlanModeTool.ts # packages/agent-core-v2/src/os/backends/node-local/tools/rgLocator.ts # packages/agent-core-v2/src/os/interface/terminal.ts # packages/agent-core-v2/src/program/program.ts # packages/agent-core-v2/src/program/programDependencies.ts # packages/agent-core-v2/src/runtime/runtime.ts # packages/agent-core-v2/src/runtime/standaloneRuntime.ts # packages/agent-core-v2/src/session/terminal/terminalService.ts # packages/agent-core-v2/src/workspace/workspaceInstance/workspaceInstanceManagerService.ts # packages/agent-core-v2/test/agent/agentsMdReminder/agentsMdReminder.test.ts # packages/agent-core-v2/test/agent/changeNotifier/changeNotifier.test.ts # packages/agent-core-v2/test/agent/loop/loop.test.ts # packages/agent-core-v2/test/agent/runtimeBinding/runtimeBindingService.test.ts # packages/agent-core-v2/test/app/sessionManager/sessionManagerService.test.ts # packages/agent-core-v2/test/features/plan/planGuard.test.ts # packages/agent-core-v2/test/runtime/architectureBoundaries.test.ts # packages/agent-core-v2/test/runtime/runtimeRegistry.test.ts # packages/agent-core-v2/test/session/subagent/forkParity.test.ts # packages/agent-core-v2/test/session/terminal/terminalService.test.ts # packages/agent-core-v2/test/tool/tool.test.ts # packages/agent-core-v2/test/workspace/workspaceInstance/workspaceInstanceManager.test.ts # packages/kap-server/src/protocol/rest-terminal.ts # packages/kap-server/src/routes/terminals.ts # packages/kap-server/test/terminals.test.ts
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fbdbf02201
ℹ️ 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".
| @@ -425,11 +315,11 @@ export function registerFsRoutes(app: FsRouteHost, core: Scope): void { | |||
| operationId: 'workspaceFsSearch', | |||
| }, | |||
| async (req, reply) => { | |||
| const { workspace, runtime_id, ...searchRequest } = req.body; | |||
| let runtimeFs: RuntimeFsScope | undefined; | |||
| const { workspace, ...searchRequest } = req.body; | |||
There was a problem hiding this comment.
When a client from the previous release sends runtime_id to /workspace/fs::search, validation now strips that field and this path always resolves the local filesystem; the same regression affects workspace/root suggestions, session FS actions, and downloads. Requests intended for a remote runtime can therefore return host data or fail without any validation signal. Continue accepting runtime_id as a deprecated environment-id alias, or reject it explicitly instead of silently ignoring it.
AGENTS.md reference: AGENTS.md:L111-L113
Useful? React with 👍 / 👎.
| lease = this.environment.acquire(['fs']); | ||
| } catch (error) { | ||
| if (error instanceof EnvironmentError) return 'unreadable'; |
There was a problem hiding this comment.
When a remote connection has dropped just before an Edit or Write, acquire() throws and this catch converts the failure to unreadable, so capture() silently returns without recording the pre-edit bytes. The tool then uses acquireOrWhenReady(), reconnects, and mutates successfully, leaving the turn without an undo snapshot or change entry for that file; reconnect here before treating the file as unreadable so the first post-drop edit remains recoverable.
AGENTS.md reference: AGENTS.md:L131-L133
Useful? React with 👍 / 👎.
| this.mirrors.set(target.fs, mirrors); | ||
| const path = (target.path?.join ?? join)(target.dir, `${taskId}.log`); | ||
| const mirrored = mirrors.get(path); | ||
| if (mirrored?.scope === output.root.scope && mirrored.size === output.data.byteLength) return true; |
There was a problem hiding this comment.
When the target temp directory is cleared or a Docker/command target is replaced under the same environment, the remote environment keeps the same fs wrapper, so this size-only cache returns true without checking the current target. readTaskOutputSnapshot() then advertises a remote log path that no longer exists instead of falling back to the retained local output; truncated task results become unreadable beyond their preview. Include the environment generation in the cache key or stat/rewrite the mirror before returning its path.
AGENTS.md reference: AGENTS.md:L131-L133
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 893f58a339
ℹ️ 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".
| @@ -391,7 +438,7 @@ export interface ReloadSessionOptions { | |||
| export interface PlanInfo { | |||
| readonly id: string; | |||
| readonly content: string; | |||
| readonly path: string; | |||
| readonly path: string | null; | |||
There was a problem hiding this comment.
When an SDK consumer upgrades and handles SessionPlan using the previously guaranteed string path (for example, passing it directly to a filesystem API), widening this public property to string | null breaks their TypeScript build. The engine's planFileTarget() still always produces a path and status() never returns null for this field, so this is an unnecessary public-contract break; retain string unless a real pathless state is implemented and released as an explicit breaking change.
AGENTS.md reference: AGENTS.md:L123-L125
Useful? React with 👍 / 👎.
| @@ -17,6 +17,7 @@ export const createSessionOptionsSchema = z.object({ | |||
| sessionId: z.string().optional(), | |||
| workDir: z.string(), | |||
| additionalDirs: z.array(z.string()).optional(), | |||
| environmentId: z.string().optional(), | |||
There was a problem hiding this comment.
When a Klient caller explicitly selects a valid declared environment that omits defaultCwd, SessionManager.resolveCreationEnvironment() requires the caller to provide environmentCwd; however, this new wire schema accepts only environmentId, and the facade likewise cannot forward a cwd. Such environments can therefore be configured but never used through the public Klient creation API. Add the optional cwd to both the mirrored contract and facade input.
AGENTS.md reference: packages/klient/AGENTS.md:L14-L18
Useful? React with 👍 / 👎.
| if (n === 0) return new Uint8Array(0); | ||
| const chunks: Uint8Array[] = []; | ||
| let total = 0; | ||
| for await (const chunk of this.readChunks(path, offset)) { |
There was a problem hiding this comment.
When a remote caller requests a bounded read, this always starts readChunks() with its fixed 1 MiB RPC size and only truncates the received result afterward. The new remote download stream requests 64 KiB at a time, so a large download repeatedly transfers and decodes overlapping 1 MiB ranges—roughly 16× the required remote I/O—and even a 4 KiB binary sample can fetch 1 MiB. Pass the remaining requested length into readRange() instead of discarding the excess.
AGENTS.md reference: AGENTS.md:L127-L129
Useful? React with 👍 / 👎.
| size: result.size, | ||
| modifiedAtMs: result.mtimeMs, | ||
| mode: result.mode & 0o7777, | ||
| }; |
There was a problem hiding this comment.
When a remote file is atomically replaced by another file with the same size and preserved millisecond mtime, this metadata omits the inode that the local HostFileStat path supplies to buildEtag(). /fs::content and session downloads consequently generate the same ETag for the replacement and can return 304 Not Modified, leaving clients with stale content. Carry the inode or another generation token through FsGetMetadataResult and the remote stat mapping.
AGENTS.md reference: AGENTS.md:L127-L129
Useful? React with 👍 / 👎.
| NO_COLOR: '1', | ||
| TERM: 'dumb', | ||
| GIT_TERMINAL_PROMPT: process.env['GIT_TERMINAL_PROMPT'] ?? '0', | ||
| GIT_TERMINAL_PROMPT: '0', |
There was a problem hiding this comment.
When Kimi is started locally with an explicit GIT_TERMINAL_PROMPT value, every Bash command now overwrites it with 0. Before this refactor the tool passed the user's value through and used 0 only when unset, so upgrading silently changes local Git behavior even though adding remote execution need not alter the local path; retain the old fallback for local environments and apply any stricter default only to remote targets.
AGENTS.md reference: AGENTS.md:L127-L129
Useful? React with 👍 / 👎.
| cwd: state.workDir, | ||
| gitBranch: this.gitCache.getStatus()?.branch ?? null, | ||
| gitBranch: isRemoteEnvironment(state.environment) ? null : (this.gitCache.getStatus()?.branch ?? null), |
There was a problem hiding this comment.
When status_line.command is configured for a remote-bound session, the built-in footer uses the environment binding's cwd, but this JSON payload still sends the host-local state.workDir while only suppressing the local Git branch. Custom status scripts therefore display a plausible but incorrect working directory or run their own checks against the host workspace; populate cwd from state.environment.cwd for remote sessions.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 715dbe0c31
ℹ️ 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 key = `${environment.identity.environmentId} ${environment.identity.generation}`; | ||
| let pending = remoteRgResolutions.get(key); | ||
| if (pending === undefined) { | ||
| pending = resolveRemoteRgPath(probe, environment, options).catch((error: unknown) => { |
There was a problem hiding this comment.
When two Glob/Grep calls concurrently bootstrap ripgrep on the same remote generation, this caches the first call's options.signal in the promise shared by both. If that first tool is canceled while either version probe is in flight on a target without system ripgrep, a subsequent throwIfAborted rejects the shared promise, so the still-active second tool incorrectly fails too. Keep cancellation in the per-caller abortable(...) wrapper and run the cached resolution without a request-scoped signal.
Useful? React with 👍 / 👎.
| const local = this.environments.inspect(this.binding); | ||
| const context: IWorkspaceContext = { ...this.context, cwd }; | ||
| const state = own(new WorkspaceStateService(this.dependencies.appState)); | ||
| const localConfig = new FileProjectLocalConfigService(this.dependencies.bootstrap, hostFs); |
There was a problem hiding this comment.
When a remote session is created with --add-dir ~/shared, this constructs the remote FileProjectLocalConfigService with the host-local bootstrap. Its expandHome() therefore turns ~ into the client machine's home and then checks that path through the remote filesystem, so creation either fails or authorizes an unrelated coincident path. Pass the target environment's home directory, or an environment-specific path resolver, to preserve ~ semantics for creation-time additional directories.
AGENTS.md reference: AGENTS.md:L127-L129
Useful? React with 👍 / 👎.
| program: resolveProgramPath(spec.program), | ||
| args: [...(spec.args ?? [])], | ||
| env: commandLauncherEnv(spec.env), |
There was a problem hiding this comment.
When a command environment supplies env.PATH so that its named launcher is discoverable—for example command = "sandbox" with the binary only under /opt/sandbox/bin—this resolves the program against the ambient process PATH before constructing the configured launcher environment. Resolution therefore throws “not found” even though the spawned child would receive the requested PATH; compute the launcher environment first and pass its PATH to resolveProgramPath.
Useful? React with 👍 / 👎.
| ); | ||
| } | ||
| return { | ||
| dir: environment.path.join(tempDir, 'kimi-code', subdir), |
There was a problem hiding this comment.
When two sessions on the same remote environment prepare the same uploaded file concurrently, they use the same deterministic path under this shared $TMP/kimi-code/attachments directory. If request A writes the file, request B observes its matching size and enqueues a prompt referencing it, and A then fails later, A's discardStaged() deletes the path out from under B, leaving B's prompt with a missing attachment. Namespace remote staging by session or request, as the local session directory did, or make cleanup reference-safe.
AGENTS.md reference: AGENTS.md:L127-L129
Useful? React with 👍 / 👎.
Requirement or Bug
支持在创建会话时把 agent 绑定到 SSH / Docker / 自定义命令环境（#3856 的 phase 1，不含会话内切换环境）。
Bug Reproduction Steps
N/A
Root Cause
N/A
Code Changes
三个概念各管一件事：
不在本 PR：
change_environment/connect工具、Agent 工具的environment参数、/environment管理器、项目级.kimi-code/environments.toml。Behavior Changes and Affected Users
[environments]、--environment或 session-create 参数绑定远端default、不传--environmentconfig.invalid；ACP 仍从 local 初始化，再绑定客户端环境[environments]或 config.toml 的 TUI、print、ACP、SDK 用户local（ACP 自动）defaultCwd，--environment-cwd可覆盖；必须是 POSIX 绝对路径disconnected；下次工具调用重连，会话的 fs / git 服务继续可用@补全/new--environmentruntime_idfs:{action}、下载、/workspace/fs::search、/workspace/fs::suggest、/fs::suggest可按请求选环境（0.37.0 起，已写入文档）runtime_id/runtimeId，回放runtime.set_bindingkimi-file://；fork 与恢复后可读；注册失败报错并拒绝发送受影响模块与测试覆盖：
agent-core-v2：registry、binding、远程协议、Program 与会话 workdir、会话生命周期；全量 7493 通过，唯一失败的tool.test.tsforeground subagent timeout 在未改动的分支上同样超时。kap-server：环境路由、会话 fs 路由（含远程会话读写与下载、远程会话拒绝reveal）；全量 1516 通过，1 个 search 用例并发下偶发失败，单独运行通过。node-sdk：创建写入 meta、远端 cwd 无效时拒绝、恢复连接失败报错、按会话补全、附件、冷恢复；192 通过。apps/kimi-code4114、acp-server150、klient141、kimi-inspect118 全部通过；各包 typecheck、lint（0 errors）、no-comments 与 import 边界检查通过。Checklist
gen-changesetsskill；功能 changeset.changeset/remote-environments.md，Windows 修复 changeset.changeset/windows-remote-targets.md；fs 路由参数与旧 runtime API 删除经维护者确认，不新增 changeset。gen-docsskill；中英文远程环境指南、配置与 server API 文档已更新。