Repository navigation
fix(mobile): detect agents inside the workspace's WSL distro - #19886
welldoingk wants to merge 3 commits into
Conversation
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
- RPC params for
preflight.detectAgents/preflight.refreshAgents— swappedparams: nullfor an optional{ wslDistro?, wslDefault? }schema mapped onto the existingPreflightRuntimeContext, so paired clients can name a distro exactly like the Electron IPC twin. - Mobile agent loader — reads the distro from the workspace's UNC path via shared
parseWslUncPathand forwards it; the SSH/detectRemoteAgentsbranch is untouched and still wins. getWorktreePathFromMobileWorktreeId— new path-half parser for the${repoId}::${path}worktree id, reusing shared parsing rather than a second copy.- Tests — 3 RPC cases (context forwarded, malformed target rejected before probing, no params = host-local) and 2 loader cases (WSL-hosted and host-filesystem workspaces), all pinning exact behavior.
I traced the wire-compat claim against dispatcher-request-parsing.ts and it holds: an old host (params: null) discards incoming params and answers host-local, and a new host parses an old client's absent params as {} → undefined context → host-local. The non-strict zod object also strips the commands key that detectRemoteAgents forwards to a remote host, so that path is unchanged. The distro read is consistent with the desktop's getWslDistroFromPath (parseWslUncPath(path).distro), and importing src/shared/wsl-paths is an established mobile pattern. Both touched test suites pass locally.
DeepSeek Pro (free via Pullfrog for OSS) ｜ 𝕏
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Advanced Run ID: 📥 CommitsReviewing files that changed from the base of the PR and between 864403603eeb14d4e0ab25107628a8d46b89c15f and abbdf75. ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughThe preflight RPC methods now accept optional Priority: ➖ Normal Severity of issue fixed: Medium Merge Risk: ⚪ Minimal · up to WSL workspaces forward their distro to agent detection while host-local behavior remains unchanged; no concrete user-impacting risk is established. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
6462ae9 to
cabf63e
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
mobile/src/session/mobile-new-tab-agent-loader.ts (1)
63-76: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winAdd a legacy fallback for
preflight.detectAgents.When a WSL worktree loads agent options, the mobile client sends
{ wslDistro }. The previous host contract usesparams: null, so that host rejects the request.loadMobileNewTabAgentOptionsthen throws on the failed response and does not load agent options. The protocol negotiation has no method-level capability or retry fallback for this change. Retry without parameters when the parameterized request is rejected, or gate the request on an explicitly negotiated capability.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Advanced
Run ID: 4bc95216-e89c-4d92-9836-82cb0b46fc92
📥 Commits
Reviewing files that changed from the base of the PR and between 6462ae997f0e38af62ef8d9ad155131283b30fe2 and cabf63ec4db24144535c61d2214b2f6e1f181c8f.
📒 Files selected for processing (2)
src/main/runtime/rpc/methods/preflight.tssrc/shared/rpc-contract/preflight-params.ts
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.
There was a problem hiding this comment.
Important
The incremental commit moved the preflight params schemas into the shared src/shared/rpc-contract/ directory but did not regenerate the generated params catalog, so verify:rpc-params-catalog fails on this branch. See the inline comment.
Reviewed changes
- Consolidated preflight params schemas into
src/shared/rpc-contract/preflight-params.ts— replaced the inlinePreflightCheck/PreflightDetectAgents/PreflightDetectRemoteAgentsdefinitions inpreflight.tswith imports from the shared module, addingPreflightDetectAgentsthere. - Made
toPreflightRuntimeContext's parameter type explicit — swappedz.infer<typeof PreflightDetectAgents>for an inline{ wslDistro?, wslDefault? }object shape; behavior is unchanged.
DeepSeek Pro (free via Pullfrog for OSS) ｜ 𝕏
There was a problem hiding this comment.
ℹ️ No critical issues — one minor edge-case divergence inline.
Reviewed changes
- Regenerated
rpc-params-catalog.generated.ts—preflight.detectAgents/preflight.refreshAgentsnow map toPreflightDetectAgents(import added), resolving theverify:rpc-params-catalogfailure from the prior review;pnpm run verify:rpc-params-catalogpasses locally. - Forwarded the WSL detection target from paired web clients —
web-host-capability-api.tsroutesdetectAgents/refreshAgentsargs through the newtoPreflightDetectAgentsParamsinstead of dropping them. - Added
toPreflightDetectAgentsParamsand tests — flattens the richerPreflightRuntimeContextinto the wire schema, mirroring the host'sgetPreflightWslTargetprecedence (resolved WSL project runtime, then explicit distro, then the default flag; null/blank dropped).
DeepSeek Pro (free via Pullfrog for OSS) ｜ 𝕏
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
- Added a
repair-requiredshort-circuit for paired web clients —isPreflightRepairRequirednow gates bothdetectAgentsandrefreshAgentsinweb-host-capability-api.ts, so a WSL-pinned project with a broken config sends no probe and answers empty instead of probing the Windows PATH. - Corrected the parity comment —
toPreflightDetectAgentsParamsnow documents thatrepair-requirednever reaches the wire, replacing the inaccurate "same as the desktop twin" claim. - Extended test coverage — a new
web-host-capability-apicase assertscallRuntimeResultis never invoked for a repair-required context, and theweb-preflight-detect-paramstest was split to pin the new flag independently.
This closes the last open thread from the prior review. I traced the fix against the desktop twin getPreflightWslTarget (src/main/ipc/preflight-runtime-target.ts:18), which throws for repair-required; the web client now short-circuits to the same user-visible outcome (empty list) without inventing wire state the schema cannot carry.
DeepSeek Pro (free via Pullfrog for OSS) ｜ 𝕏
The Electron IPC handler for agent detection takes a PreflightRuntimeContext, so a desktop WSL workspace probes inside its distro. The runtime RPC twin that paired clients call declared `params: null`, so a Windows host always answered from its own PATH -- where a WSL-only install is absent and the mobile New Tab drawer shows "No Enabled Agents". Accept the same optional WSL target on preflight.detectAgents/refreshAgents, and have the mobile new-tab loader read the distro from the workspace's own UNC path. Hosts that predate the parameter ignore it and keep answering host-local.
The web twin of the preload preflight API took no arguments, so the runtime
context its callers pass -- `refreshAgents(context)` in the local detected-agent
store and `detectAgents({ wslDistro })` in skill install -- was dropped before the
request left the client. A zero-arg function is assignable to the optional-arg
signature, so this never failed typecheck. Forwarding the raw context would
regress instead: it carries `wslDistro: null` and `projectRuntime`, and the wire
schema rejects null. `toPreflightDetectAgentsParams` maps it with the host's own
`getPreflightWslTarget` precedence.
Also regenerates the RPC params catalog, stale since the schemas moved into
`src/shared/rpc-contract/`, which `verify:rpc-params-catalog` gates.
Flattening a repair-required project runtime to no-target sent the paired web client to the Windows PATH for a WSL-pinned project, caching launchable-looking host agents as a successful detect. The desktop twin refuses that probe, so skip the request and answer with the empty fallback instead.
8644036 to
abbdf75
Compare
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
- Rebased onto current
main— the mobile agent loader now runs on the typed read operations (newTabSettingsRead,newTabRepoListRead,preflightDetectAgentsRead, each returning{ reply, interpret }). The PR-authored logic is unchanged:getWorktreeWslDistrostill reads the distro from the worktree path viaparseWslUncPathand forwards{ wslDistro }only when present. - No changes to the RPC schema, host method, or web preload forwarding — those files are byte-identical to the revision the prior review covered.
preflight.detectAgents/preflight.refreshAgentsstill map toPreflightDetectAgents, and the web preload still gates a repair-required runtime withisPreflightRepairRequired. - All prior findings addressed — the stale RPC params catalog and the repair-required divergence threads are resolved and remain fixed.
I verified the delta by diffing the 11 PR files between 8644036 and abbdf75: only mobile-new-tab-agent-loader.ts and rpc-params-catalog.generated.ts differ, and those are main drift from the rebase rather than new PR logic. pnpm run verify:rpc-params-catalog passes, and the 16 focused preflight / web-preload tests pass locally. No settings.new-tab-agents recording scenario uses a WSL worktree path, so the fenced mobile goldens stay byte-identical.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
|
Consolidated into #20101, rebased on current main together with its New Workspace follow-up so the whole fix reviews and lands as one change. Same commits, same tests; nothing dropped. |
ELI5
If your workspaces live inside WSL, your phone's New Tab drawer said "No Enabled Agents" — not even Claude or Codex — while the desktop app listed them all for the same workspace. The desktop knows how to look inside the distro because it tells the detector which distro to check; the phone had no way to say that, so the host looked at the Windows side of the machine, where a WSL-first user has no agents installed. Now the request can name the distro, and hosts that never heard of that field keep behaving exactly as before. The paired web client had the same hole one layer up — its copy of the preflight API took no arguments at all, so the distro its own callers already passed was thrown away before the request left the browser — and that is fixed here too.
What Changed
src/main/runtime/rpc/methods/preflight.ts:preflight.detectAgentsandpreflight.refreshAgentsswapparams: nullfor an optional schema carryingwslDistro/wslDefault, mapped to thePreflightRuntimeContextthe Electron IPC twin already accepts. No target named still means host-local, sogetPreflightWslTargetkeeps distinguishing "no context" from "context that selected nothing". ThewslDistroconstraint copies the oneaccounts.tsalready uses for the same value.mobile/src/session/mobile-new-tab-agent-loader.ts: the local branch reads the distro out of the workspace's own path and sends it. SSH/connection resolution is untouched.mobile/src/session/mobile-session-route-helpers.ts:getWorktreePathFromMobileWorktreeId, the path half of the existinggetRepoIdFromMobileWorktreeId. Parsing reuses sharedparseWslUncPathrather than a second copy.src/renderer/src/web/preload-api/web-host-capability-api.ts: the web twin of the preload API declareddetectAgents: async () =>andrefreshAgents: () =>, so the context its callers pass —refreshAgents(context)inlocal-detected-agent-state.tsanddetectAgents({ wslDistro })in skill install — was dropped before the call. A zero-arg function is assignable to the optional-arg signature, so this never failed typecheck. Both now accept and forward it.src/renderer/src/web/preload-api/web-preflight-detect-params.ts(new): forwarding the raw context would regress rather than fix, because it carrieswslDistro: string | nullplusprojectRuntime, and the wire schema rejectsnull. This maps it with the host's owngetPreflightWslTargetprecedence (resolved project runtime, then explicit distro, then the default flag), andisPreflightRepairRequiredskips the probe entirely for a runtime that needs repair, matching the desktop's refusal instead of listing Windows-side agents a WSL-pinned project cannot launch.src/shared/rpc-contract/rpc-params-catalog.generated.ts: regenerated, since the new schema lives in the directorygenerate-rpc-params-catalog.mjsscans andverify:rpc-params-cataloggates it.Why
The two entry points into agent detection are twins, but only one could be told where to look:
getPreflightWslTarget(undefined)returnsnullon win32, so the probe ran against the Windows PATH. On a WSL-first machine that PATH holds no agent CLI, so the drawer was empty. The desktop never hit this becausegetLocalAgentPreflightContextderives the distro from the workspace path.Reading the distro from the route rather than adding a resolver: a worktree id is
${repoId}::${path}, the path is the host's own spelling of the workspace, andparseWslUncPathis the existing answer for "which distro is this path in". A non-WSL path yieldsnulland the call goes out with no params, unchanged.Linked Issue
Fixes #19885
Visual Proof
Android client paired to a Windows host, WSL workspace, New Tab drawer. Both shots keep the workspace header, the tab bar, and the drawer; the terminal area between them is cropped out because it was showing unrelated session content.
BEFORE — stock client + host: the agent section is a single disabled "No Enabled Agents" row, while Terminal / Browser / Markdown Note below it work fine.
AFTER — patched client + host: Claude, Codex, OpenCode, Antigravity, Hermes. Gemini and Pi are absent because this profile has them in
disabledTuiAgents, which is the correct filtering, not a miss.Testing
Verified on a real device against a real host, not only in unit tests.
1. Live host RPC, stock vs patched, same machine, same minute. Both apps were queried over their own runtime named pipe (the transport the bundled CLI uses):
[][]{"wslDistro":"Ubuntu-24.04"}[]["claude","codex","opencode","pi","gemini","antigravity","hermes"](824ms){"wslDistro":"NoSuchDistro-xyz"}[][](88ms){"wslDistro":""}[]invalid_argument: Too small: expected string to have >=1 charactersRow 2 on the left is the bug; row 2 on the right is the fix, and its 824ms is a real
wsl.exeround trip. Row 3 is the discriminator: a distro that does not exist answers nothing, so the parameter really does route the probe into WSL instead of silently falling back to the host. Row 4 shows the guard is live. The stock column also shows why this is wire-safe in the old direction:params: nullmakes the dispatcher discard request params, so a new client against an old host is exactly today's behavior, never an error.2. On device. Android release build of this branch installed on a phone, paired to a Windows host running this branch, WSL workspace — see Visual Proof above.
3. Automated.
pnpm exec vitest run --config config/vitest.config.ts src/main/runtime/rpc/methods/preflight.test.ts— 6 passed (3 new)cd mobile && vitest run src/session/mobile-new-tab-agent-loader.test.ts— 4 passed (2 new)pnpm exec vitest run --config config/vitest.config.ts src/renderer/src/web/preload-api— 11 passed (10 new)src/main/runtime/rpc/**182 files / 1702 passed; mobilesrc/session/**156 files / 1479 passed;src/renderer/src/store/slices+ preflight RPC 327 files / 3149 passedpnpm run verify:rpc-params-catalog— clean (this is the gate the regenerated catalog answers)pnpm run tc:node,pnpm run tc:web, mobiletsc --noEmit— cleanoxlint,oxfmt --check,check:code-quality:changed,check:max-lines-ratchet— clean, 0 new findingsPlatforms actually exercised: Windows host + WSL guest + Android client. macOS and Linux are unaffected —
getPreflightWslTargetreturnsnulloff win32, so the new field is inert there.AI Disclosure
Written with Claude Opus 4.5 (via OmO). Every claim above is from a command run in this session; the two screenshots are from the reporter's own phone.
Review
getPreflightWslTarget, which returnsnullunlessprocess.platform === 'win32'. macOS and Linux hosts ignore it. No path, shortcut, or shell assumption added.docs/reference/remote-wire-compatibility.mdRule 1 this is a new optional field on an existing method, and no reader requires it. New client + old host: the old host declaresparams: null, soparseRpcRequestParamsdiscards params and answers host-local — the current behavior, not a failure. Old client + new host: absent params parse to{}, every field optional, contextundefined, same code path as before. No protocol version bump. The SSH branch (preflight.detectRemoteAgents) is untouched, and its resolution still wins over the new one.repair-requiredproject runtime cannot be expressed on the wire, and the desktop twin throws instead of probing. The web client therefore sends no request at all and answers with its empty fallback, which is what that user already sees on desktop — never a host-local list that looks launchable.wslDistrocrosses a trust boundary from a paired client, so it is validated at the boundary —z.string().trim().min(1).max(255), matching the existing constraint on the same value inaccounts.ts. It reacheswsl.exeas an argv element viarunWslProcess, never as shell text, and an unknown distro yields an empty result rather than a fallback to another host. A malformed value is rejected before any process spawns.Agent skill upstream boundary
Notes
Adjacent but distinct, and deliberately left alone:
hostId: "local", so it lands on the local branch either way; the two changes compose.NewWorktreeModaland the tasks workspace-create path resolve detection the same context-less way. Same class of gap, out of scope here because both run before a worktree exists, so the distro has to come from the selected repo's path rather than the worktree id this PR reads. Tracked in [Bug][Mobile]: Create-workspace surfaces list no agents for a WSL-hosted repo (detectAgents sent without a distro) #20060.preflight.refreshAgentsis included because it is the same signature and the same miss; leaving it would make Refresh silently disagree with the list it refreshes.Checklist