fix: detect remote agents using the execution context's platform, not the client's#2510
Conversation
…platform Agent detection chose `where` vs `which` from the local `process.platform`, but `resolveCommandPath` runs the command through the execution context — which is the remote host for SSH projects. A Windows client connected to a POSIX remote ran `where claude` on Linux, which fails, so no agents were ever detected (and post-install probes reported not-detected-after-install). Expose `isWindows` on IExecutionContext (true only for a Windows local host; always false for POSIX SSH remotes) and pick the resolve command from it, so detection follows where the command actually executes.
Add probe tests asserting a POSIX SSH context resolves with `which` even when the client is Windows, and a Windows host uses `where`. Set `isWindows` on existing IExecutionContext test mocks for the new required field.
Greptile SummaryThis PR fixes agent detection failures when Emdash runs on Windows and connects to a remote Linux/macOS project over SSH. The root cause was
Confidence Score: 4/5Safe to merge; the core fix is minimal and correct, and local behaviour is entirely unchanged. The fix itself is clean and well-tested. Four test files still construct IExecutionContext mocks without the new isWindows field, relying on a type-cast bypass to compile. The hardcoded isWindows = false in SshExecutionContext is correct today but undocumented as a limitation in the class itself, leaving a silent failure path for any future SSH-to-Windows target. dependency-manager.test.ts, git-repo-utils.test.ts, remote-shell-profile.test.ts, and project-settings.test.ts all have incomplete IExecutionContext mocks. ssh-execution-context.ts carries the hardcoded isWindows = false assumption worth noting inline.
|
| Filename | Overview |
|---|---|
| apps/emdash-desktop/src/main/core/execution-context/types.ts | Adds readonly isWindows: boolean to IExecutionContext with clear JSDoc explaining why consumers must use it instead of process.platform. |
| apps/emdash-desktop/src/main/core/dependencies/probe.ts | Replaces module-level process.platform constant with a resolveCmd(ctx) helper that reads ctx.isWindows, directly fixing the cross-platform SSH detection bug. |
| apps/emdash-desktop/src/main/core/execution-context/local-execution-context.ts | Adds readonly isWindows = process.platform === 'win32'; local behavior is unchanged. |
| apps/emdash-desktop/src/main/core/execution-context/ssh-execution-context.ts | Adds readonly isWindows = false — correct for all current POSIX SSH targets but hardcoded; a future SSH-to-Windows target would silently probe with which and fail. |
| apps/emdash-desktop/src/main/core/dependencies/probe.test.ts | New test file covering POSIX/Windows resolution and the regression scenario (Windows client + POSIX SSH context uses which); all five test cases are targeted and clear. |
| apps/emdash-desktop/src/main/core/dependencies/dependency-manager.test.ts | Mock factory omits the new isWindows field unlike all other updated test files; passes only because as unknown as IExecutionContext bypasses the type check and undefined falls through to which. |
Sequence Diagram
sequenceDiagram
participant P as probe.ts resolveCommandPath()
participant LOCAL as LocalExecutionContext
participant SSH as SshExecutionContext
Note over P: resolveCmd(ctx) called
alt Local context on Windows
P->>LOCAL: ctx.isWindows → true
LOCAL-->>P: "where"
P->>LOCAL: exec("where", ["claude"])
else Local context on macOS/Linux
P->>LOCAL: ctx.isWindows → false
LOCAL-->>P: "which"
P->>LOCAL: exec("which", ["claude"])
else SSH context (any client OS)
P->>SSH: ctx.isWindows → false (always)
SSH-->>P: "which"
P->>SSH: exec("which", ["claude"])
Note over SSH: Runs on remote POSIX host
end
Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
apps/emdash-desktop/src/main/core/execution-context/ssh-execution-context.ts:40
**Hardcoded `false` assumes all SSH targets are POSIX**
`isWindows = false` is correct for the documented use-case (remote Linux/macOS servers), and the JSDoc on the interface acknowledges this. However, the assumption is baked into the class and will silently misbehave if an SSH-to-Windows target (e.g. OpenSSH on Windows Server) is ever introduced — `which` does not exist there and every probe would fail just like the original bug. Worth a TODO comment flagging this limitation so it is visible if a Windows SSH target needs to be supported later.
Reviews (1): Last reviewed commit: "test(dependencies): cover platform-aware..." | Re-trigger Greptile
| export class SshExecutionContext implements IExecutionContext { | ||
| readonly root?: string; | ||
| readonly supportsLocalSpawn = false; | ||
| readonly isWindows = false; |
There was a problem hiding this comment.
Hardcoded
false assumes all SSH targets are POSIX
isWindows = false is correct for the documented use-case (remote Linux/macOS servers), and the JSDoc on the interface acknowledges this. However, the assumption is baked into the class and will silently misbehave if an SSH-to-Windows target (e.g. OpenSSH on Windows Server) is ever introduced — which does not exist there and every probe would fail just like the original bug. Worth a TODO comment flagging this limitation so it is visible if a Windows SSH target needs to be supported later.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/emdash-desktop/src/main/core/execution-context/ssh-execution-context.ts
Line: 40
Comment:
**Hardcoded `false` assumes all SSH targets are POSIX**
`isWindows = false` is correct for the documented use-case (remote Linux/macOS servers), and the JSDoc on the interface acknowledges this. However, the assumption is baked into the class and will silently misbehave if an SSH-to-Windows target (e.g. OpenSSH on Windows Server) is ever introduced — `which` does not exist there and every probe would fail just like the original bug. Worth a TODO comment flagging this limitation so it is visible if a Windows SSH target needs to be supported later.
How can I resolve this? If you propose a fix, please make it concise.
Problem
Closes #2474.
When Emdash runs on Windows and connects to a remote SSH project on a Linux server, installed agents (Claude Code, opencode, …) are never detected even though they are on the remote
PATH. Install ends with "The agent was not detected after installation."Root cause: the binary-resolve command was chosen from the local client platform:
But
resolveCommandPath()runs that command throughctx.exec(), which for remote projects is anSshExecutionContextexecuting on the remote host. A Windows client therefore ranwhere claudeon the Linux server —wheredoesn't exist there, so every probe failed and all agents were reported missing. The post-install probe failed the same way (not-detected-after-install). macOS/Linux clients were unaffected becausewhichexists on the remote, which is why this went unnoticed.Solution
Pick the resolve command from the platform the context executes on, not the local client:
readonly isWindowstoIExecutionContext.LocalExecutionContext→process.platform === 'win32'(preserves existing behavior).SshExecutionContext→false(SSH remotes are POSIX).probe.tsresolves withctx.isWindows ? 'where' : 'which'.Local macOS/Linux and local Windows behavior is unchanged; the only behavioral change is that a remote POSIX host is now probed with
whichregardless of the client OS.Testing
probe.test.tscovers POSIX vs Windows resolution and the regression directly: a POSIX SSH context resolves withwhicheven when the client is Windows.IExecutionContexttest mocks for the new required field.pnpm run typecheck,pnpm run lint,pnpm run format:checkall pass; affected suites (probe, dependency-manager, execution-context, terminals, agent-hooks, git) green.