Repository navigation
feat: support Windows remote environments - #4610
Conversation
|
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![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: 813df0d604
ℹ️ 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".
| ); | ||
| } | ||
| if (this.environmentValue.pathClass !== 'posix') { | ||
| if (this.environmentValue.pathClass !== 'posix' && this.environmentValue.pathClass !== 'win32') { |
There was a problem hiding this comment.
When a Windows executor is controlled from Linux/macOS—or from a different Windows installation—file-tool inputs copied from Git Bash such as /usr/... or /tmp/... are translated incorrectly. getShellPathBridge() receives the remote shellPath but invokes existsSync/cygpath.exe on the controller, so it cannot resolve the target's MSYS mounts and leaves the path unchanged; the target then interprets it as a current-drive root path such as C:\usr\..., potentially reading or writing a different file. The new Windows CI loopback runs controller and executor on the same host and does not expose this. Path conversion needs to run on the executor or use target-provided mount information before win32 executors are admitted.
AGENTS.md reference: AGENTS.md:L104-L104
Useful? React with 👍 / 👎.
| if (process.platform === 'win32') { | ||
| if (entry.killTimer !== undefined) clearTimeout(entry.killTimer); | ||
| return; |
There was a problem hiding this comment.
When the controller is upgraded but the Windows target still runs a previous released executor, the handshake now accepts it because MIN_EXECUTOR_VERSION remains 0.1.0, even though this Windows-specific cleanup branch exists only in the new executor. The older ProcessManager sends every completed command into the POSIX exitedGroups reaper, where negative-PID probing is unsupported on Windows and can leave a recurring timer and map entry for every Bash, Grep, or Glob process. Negotiate this capability or raise the required executor version for win32 so users receive upgrade guidance instead of silently running the incompatible server.
AGENTS.md reference: AGENTS.md:L104-L104
Useful? React with 👍 / 👎.
| if (!(error instanceof HandshakeError)) return 'other'; | ||
| if (error.kind === 'executor-exit') { | ||
| return error.exitCode === 127 || error.exitCode === 126 ? 'missing' : 'other'; | ||
| return error.exitCode === 127 || error.exitCode === 126 || error.exitCode === 9009 ? 'missing' : 'other'; |
There was a problem hiding this comment.
When a Linux or macOS controller SSHes to a Windows target where cmd.exe cannot find kimi, the remote status is 9009, but the Unix controller observes only its low eight bits, 49, so this exact === 9009 check classifies the failure as other and omits the intended installation guidance. OpenSSH returns the remote command's status (OpenSSH ssh(1)), while POSIX wait() exposes only the low-order eight bits (POSIX wait(3p)). Detect the normalized status together with the Windows command-not-found stderr, rather than expecting 9009 to survive an SSH process running on Unix.
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: fedc5784f5
ℹ️ 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".
| if (normalized === '/' && env.pathResolution !== 'reject-unresolved') return normalized; | ||
| if (VIRTUAL_FS_PREFIXES.some((prefix) => normalized.startsWith(prefix))) return normalized; |
There was a problem hiding this comment.
On a Windows remote, file-tool inputs such as /dev/null or /proc/... hit this unconditional return before the new pathResolution === 'reject-unresolved' handling; the native filesystem RPC then treats the leading slash as a current-drive-root Windows path such as C:\dev\null, so it can silently access the wrong target data when that path exists. Fresh evidence beyond the prior review is this virtual-filesystem exception, which still bypasses the newly added remote rejection; make this exception local-only or reject these paths remotely.
AGENTS.md reference: AGENTS.md:L130-L132
Useful? React with 👍 / 👎.
| if (process.platform === 'win32') { | ||
| if (entry.killTimer !== undefined) clearTimeout(entry.killTimer); | ||
| return; |
There was a problem hiding this comment.
When terminating a Windows remote process, HostProcess.kill() waits for forced taskkill to finish; if the target exits and maybeClose() runs during that await, this branch sees no killTimer to clear, after which beginTermination() installs the grace timer anyway. Five seconds later that timer invokes taskkill again with the stale PID, which can terminate an unrelated process tree if Windows has already reused the PID. Skip the grace timer on Windows or recheck entry.closed after the awaited kill.
AGENTS.md reference: AGENTS.md:L104-L104
Useful? React with 👍 / 👎.
| if (/^[A-Za-z0-9_.-]+$/.test(remoteBin)) { | ||
| return [remoteBin, ...EXEC_SERVER_ARGV].join(' '); |
There was a problem hiding this comment.
For an existing POSIX SSH configuration whose bare remoteBin is also a shell reserved word, such as an executor symlink named if or time, this new fast path removes the quoting that previously forced command lookup. The remote shell now parses the name as syntax (if ... fails parsing and time ... runs the wrong command), even though the PR states POSIX launcher semantics are unchanged; exclude reserved words from this branch or reject them explicitly.
AGENTS.md reference: AGENTS.md:L110-L112
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: 50ad811cdb
ℹ️ 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".
| ); | ||
| } | ||
| if (this.environmentValue.pathClass !== 'posix') { | ||
| if (this.environmentValue.pathClass !== 'posix' && this.environmentValue.pathClass !== 'win32') { |
There was a problem hiding this comment.
When a Windows remote disconnects while a command is running, accepting win32 here makes StdioHost.shutdown() call ProcessManager.terminateAll(), which invokes killGroup() once before waiting and again for every snapshotted entry after it has exited. On Windows, HostProcess.kill() ignores the requested signal and runs forced taskkill /T /F both times, so the second call can terminate an unrelated process tree if the original PID has already been reused. Distinct from the fixed grace-timer path, this unconditional shutdown pass remains; await/remove exited Windows entries or skip their second kill.
AGENTS.md reference: AGENTS.md:L104-L104
Useful? React with 👍 / 👎.
| export function normalizeRemoteCwd(cwd: string): string { | ||
| if (!posixPath.isAbsolute(cwd) || cwd.includes('\0')) { | ||
| throw new EnvironmentError('environment.invalid_cwd', 'remote cwd must be an absolute POSIX path; relative paths and ~ are not expanded'); | ||
| const path = /^[A-Za-z]:[\\/]/.test(cwd) || cwd.startsWith('\\\\') ? win32Path : posixPath; |
There was a problem hiding this comment.
When a Windows remote uses the newly accepted UNC defaultCwd, or a file tool receives a UNC path, resolvePathAccessPath() runs pathe.resolve() after the shell-path bridge; pathe 2.0.3 converts \\server\share into /server/share, as shown by its upstream reproduction. Because the bridge has already run, that slash-rooted path reaches the Windows filesystem RPC and is interpreted as a current-drive root such as C:\server\share, so relative and absolute Read/Write/Edit/Grep operations can silently access machine-local files instead of the network share while Bash remains in the intended UNC workspace. Preserve UNC paths with win32 semantics or convert/reject the post-canonicalization result.
AGENTS.md reference: AGENTS.md:L130-L132
Useful? React with 👍 / 👎.
Signed-off-by: 7Sageer <sag77r@hotmail.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f8dbb1d328
ℹ️ 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".
| ); | ||
| } | ||
| if (this.environmentValue.pathClass !== 'posix') { | ||
| if (this.environmentValue.pathClass !== 'posix' && this.environmentValue.pathClass !== 'win32') { |
There was a problem hiding this comment.
This enables Windows targets as a user-visible CLI capability, but the cumulative PR diff contains no .changeset entry because the original feature changeset was subsequently deleted. As a result, the Changesets release flow will not attribute a CLI version bump or changelog entry to this feature; restore a minor changeset for @moonshot-ai/kimi-code.
AGENTS.md reference: AGENTS.md:L89-L90
Useful? React with 👍 / 👎.
|
@codex review |
|
Codex Review: Didn't find any major issues. 🎉 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 |
|
Codex Review: Didn't find any major issues. Nice work! 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 |
|
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". |
commit: |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 04bff0ca4c
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ); | ||
| } | ||
| if (this.environmentValue.pathClass !== 'posix') { | ||
| if (this.environmentValue.pathClass !== 'posix' && this.environmentValue.pathClass !== 'win32') { |
There was a problem hiding this comment.
When this gate admits a Windows executor, a command can have exited while its output streams are still open; onExit() sets exitCode, but maybeClose() leaves the entry in processes. The fresh residual is the initial shutdown pass: although the new final loop skips Windows, terminateAll() still sends SIGTERM to every snapshot entry. Windows HostProcess.kill() turns that into taskkill /T /F /PID, so if the PID has been reused this can kill an unrelated process tree. Skip the initial shutdown kill for entries whose exit code is already set.
AGENTS.md reference: AGENTS.md:L104-L104
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: 0e10beacd5
ℹ️ 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 const MIN_EXECUTOR_VERSION = '0.1.0'; | ||
| export const MIN_EXECUTOR_VERSION = '2.5.0'; |
There was a problem hiding this comment.
When a controller is upgraded to this release while a POSIX target still runs any previously accepted executor version (including 2.4.1), this global minimum rejects the connection before considering the target platform. The newly required lifecycle capability is Windows-specific and optional in the protocol, so these POSIX executors remain compatible but their remote environments become entirely unavailable until users manually upgrade every target. Keep the existing minimum for POSIX and enforce the new version or capability only for win32 executors.
AGENTS.md reference: AGENTS.md:L110-L112
Useful? React with 👍 / 👎.
Requirement or Bug
延续 #3856 与 #4062：Windows 机器作为 SSH 或自定义命令执行目标时，此前会在握手阶段因
pathClass !== "posix"被拒绝。这个 PR 让这类目标在安装 Git for Windows（Git Bash）后，可以创建和恢复远程会话，并执行文件与进程工具。Bug Reproduction Steps
N/A
Root Cause
N/A
Code Changes
核心思路是复用本机 Windows 已有的 Git Bash、Shell 路径桥接和进程后端，不新增 PowerShell 执行体系。控制端根据握手里的
pathClass选择目标路径语义；Windows executor 必须声明新的进程生命周期能力，现有 POSIX executor 与配置行为保持不变。-c命令构造和路径桥接，避免引入第二套 Shell 契约cygpath猜测windowsProcessLifecycleremoteBin = "kimi"，从目标PATH查找taskkill /T /F清理，退出后的限制写入文档具体变化：
normalizeRemoteCwd接受 Windows 驱动器路径并拒绝 UNC；POSIX 双斜杠输入仍按原逻辑规范化。createEnvironmentPath/createEnvironmentWorkspace，远端按握手返回的pathClass选择 mapper。cygpath；/tmp、/usr、/dev、/proc等无法确认挂载的路径明确报错，不静默读写其他目录。[A-Za-z0-9_.-]组成、且不是 shell 保留字的remoteBin使用跨 Shell 裸命令（这类 token 在 POSIX shell 中加不加引号语义一致）；含其他字符、~前缀或保留字的继续走旧 quoting。缺失时的安装指引使用同一判定。cmd 的 9009 以及 Unix OpenSSH 观察到的低 8 位 49 都能得到安装指引，49 必须同时匹配 cmd 缺失错误文本。Environment持有：本机环境提供会调用cygpath的实现，Windows 远端提供只做词法 drive 转换、其余 MSYS 路径一律拒绝的实现；HostEnvironmentInfo不携带路径解析策略，全局 bridge 缓存只服务本机。Windows executor 对非驱动器路径返回“必须是 Windows 驱动器路径”的明确错误。rg.exe与目标原生目录，不再执行chmod。taskkill完成后不再安装二次 grace kill timer，与本地 Windows 进程行为对齐。pathClass与工作目录；跨平台 loopback 覆盖 Windows 执行路径。阅读顺序：
packages/agent-core-v2/src/environment/environmentDefaults.ts：cwd 与路径语义。packages/agent-core-v2/src/remote/client/remoteEnvironmentProvider.ts：握手信息如何进入 Environment。packages/agent-core-v2/src/_base/execEnv/shellPathBridge.ts：本机转换与远端拒绝策略的边界。packages/agent-core-v2/src/remote/client/launchers.ts、connection.ts：跨 Shell 调用与 executor 能力协商。packages/agent-core-v2/src/remote/server/processManager.ts：Windows 进程退出边界。packages/agent-core-v2/src/os/backends/node-local/tools/rgLocator.ts：Windows rg 安装。测试、双语文档、CI 和 changeset 可略读。
Behavior Changes and Affected Users
windowsProcessLifecycle的 win32 executor；旧 Windows executor 与未知 path class 仍拒绝/tmp、/usr、/dev、/proc等路径直接报错，不调用控制端cygpath；UNC 也拒绝/c/...词法 drive 路径//开头 POSIX 路径的用户remoteBin调用[A-Za-z0-9_.-]组成且非 shell 保留字的名称不加引号；其他名称保留旧 quotingPATH中安装 executor 的用户；既有 POSIX 自定义remoteBin用户remoteBin = "kimi"PATH安装指引处理rg.exe，跳过chmodGrep、文件搜索的用户rg或手动放置到提示的目标路径taskkill没有修改系统提示词、工具描述、公开 API、session 元数据格式或旧 runtime 行为。现有 POSIX 用户的默认值、launcher quoting、进程回收和 cwd 规范化均有原测试覆盖。
验证结论：
pnpm run typecheck通过。pnpm run lint：0 错误；既有 warning 保留。测试文件与基线说明
packages/agent-core-v2/test/remote/session.test.ts改为跨平台 loopback，覆盖握手、文件读写、进程输出与终止。agent-core-v2 lint:imports时，未改动的src/human/agent/origin.ts在基线同样报既有 import 违规，本 PR 未处理。Checklist
gen-changesetsskill；后续按远端提交移除 changeset 文件。gen-docsskill：同步远程环境中英文指南与配置参考。