wta: resolve agent session cwd via session/list namespace + fallback#265
Open
yeelam-gordon wants to merge 15 commits into
Open
wta: resolve agent session cwd via session/list namespace + fallback#265yeelam-gordon wants to merge 15 commits into
yeelam-gordon wants to merge 15 commits into
Conversation
A custom ACP agent launched inside WSL (e.g. `wsl.exe -d Ubuntu -- copilot
--acp --stdio`) failed every session/new with `Directory path must be
absolute: C:\WINDOWS\system32`. WT reports a WSL pane's cwd as the Windows
launcher dir (System32), and that path is neither POSIX-absolute nor
reachable inside the distro, so the Linux CLI rejects it.
Add protocol::acp::wsl_path::resolve_session_cwd, which:
- drops junk launcher paths (System32, Windows, the WT install dir);
- for a WSL agent, translates a real Windows path via wslpath -a, else
falls back to the distro \C:\Users\yeelam (cached per distro), else /root;
- for a native agent, passes a real path through, else %USERPROFILE%.
Wire it into wta-master's new_session/load_session (the single chokepoint
that knows the configured agent_cmd and forwards to the agent CLI) and into
the direct run_inner path. The spawn cwd stays a Windows path because the
immediate child is wsl.exe. Pure logic is unit-tested via a mock resolver.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes ACP session/new / session/load failures for agents launched inside WSL by translating or replacing the incoming pane cwd (often C:\Windows\System32) into a usable path in the agent’s namespace (POSIX for WSL agents, %USERPROFILE% for native agents). It introduces a dedicated resolver and wires it into both the direct client path and the master forwarding chokepoint.
Changes:
- Add
resolve_session_cwdlogic for WSL-launched agents (Windows → POSIX viawslpath -a, fallback to distro$HOME//root) and native agents (junk →%USERPROFILE%). - Invoke cwd resolution from both
wta-master(new_session+load_session) and the directrun_innersession creation path. - Track the raw configured
cli.agentlauncher string in master state so forwarding can correctly detect WSL launchers.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| tools/wta/src/protocol/acp/wsl_path.rs | New resolver module implementing WSL/native cwd normalization and unit tests. |
| tools/wta/src/protocol/acp/mod.rs | Exposes the new wsl_path module. |
| tools/wta/src/protocol/acp/client.rs | Uses resolver in direct run_inner path before sending session/new. |
| tools/wta/src/master/mod.rs | Stores agent_cmd and rewrites forwarded cwd for new_session/load_session. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
This comment has been minimized.
This comment has been minimized.
Re-pivot of the WSL-agent cwd fix per design review. Replaces the agent_cmd string-parsing approach with one that learns the agent's path namespace from the agent itself and converts accordingly. - New module cwd_format.rs: classify(), detect_format() (from the agent's own session/list cwds), pick_value() (drops junk System32/Windows down to %USERPROFILE%), and two idempotent converters to_windows_format / to_linux_format (C:\x <-> /mnt/c/x; non-/mnt POSIX -> %USERPROFILE%), plus build_attempts() and looks_like_cwd_error(). - wta-master learns the namespace once at startup via the agent's session/list (capability-gated; empty/unsupported -> None) and caches it. new_session converts the incoming cwd into that namespace and retries down an ordered candidate list, letting the agent own cwd-rejection error drive the fallback (final floor /tmp for POSIX). load_session applies the single-shot conversion. - Direct (non-master) run_inner path mirrors the same logic. Why: the old code sent std::env::current_dir() (= C:\WINDOWS\system32 for the packaged helper) to session/new; a WSL agent rejects it as not absolute. A native agent accepts it, which is why the bug only hit WSL agents. Detection is wrapper-proof (no launcher/profile parsing) because the agent reports its own namespace. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
yeelam-gordon
changed the title
This comment has been minimized.
This comment has been minimized.
yeelam-gordon
commented
Jun 11, 2026
yeelam-gordon
commented
Jun 11, 2026
yeelam-gordon
commented
Jun 11, 2026
yeelam-gordon
commented
Jun 11, 2026
yeelam-gordon
commented
Jun 11, 2026
The pipe-mode bootstrap created its first session with std::env::current_dir() (= C:\WINDOWS\system32 for the packaged helper), so a WSL agent's session never started in the user's pane cwd (e.g. /home/yeelam). Resolve the active pane cwd first (the WSL pane reports /home/yeelam via shell integration) and let master convert it into the agent's namespace; None still falls back to the process cwd. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ent-cwd # Conflicts: # tools/wta/src/master/mod.rs # tools/wta/src/protocol/acp/client.rs
This comment has been minimized.
This comment has been minimized.
…y predicate, test env isolation, spelling - classify(): recognize UNC / extended-length paths (\\server\share, \\?\C:\…, \\wsl$\…) as Windows so valid absolute Windows cwds aren't dropped to %USERPROFILE% / /tmp. - looks_like_cwd_error(): drop the over-broad "directory" substring; match only absolute / does-not-exist / cannot-be-accessed / not-a-directory so unrelated errors aren't treated as retryable. - tests: add a scoped_env guard (serializes via a Mutex + restores prior values on drop, incl. panic-unwind) and wrap every test that mutates USERPROFILE/SystemRoot, removing parallel-test env leakage. - reword "cwds" -> "cwd values"/cwd_values and "automount" -> "auto-mount" to satisfy check-spelling without allowlist churn. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Addresses Copilot review: an unset/empty USERPROFILE previously fell straight through to std::env::current_dir() (= C:\WINDOWS\system32 for the packaged helper), re-seeding the junk dir we aim to avoid. Now mirrors WTA's home-resolution convention (history_loader::home_dir): USERPROFILE then HOME (both skipped when empty), with the process cwd only as the last resort. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Addresses Copilot review: the previous USERPROFILE->HOME->cwd fallback could return a POSIX HOME (e.g. Git Bash /home/u) or a junk current_dir (C:\WINDOWS\system32), breaking to_windows_format's "returns a Windows path" contract. Now: USERPROFILE -> HOME only when it classifies as Windows -> %SystemDrive%\ (e.g. C:\), never current_dir(). Added a test asserting an empty USERPROFILE + POSIX HOME yields C:\. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
…o avoid "cwds" Addresses Copilot review: - classify(): a drive-relative path like C:foo is not absolute -> now returns None (only C:, C:\..., C:/... are Windows), matching the doc and avoiding a bogus absolute conversion. - mod.rs: cwd_format is crate-internal -> pub(crate). - rename try_session_cwds -> try_session_cwd_candidates so check-spelling stops flagging the "cwds" token (no allowlist churn). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Addresses Copilot review: pick_value's doc said it falls back to "%USERPROFILE% (then the process cwd)", but since user_profile_dir was hardened it no longer uses current_dir() (it ends at %SystemDrive%\ to avoid seeding System32). Updated the comment to match. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ent-cwd # Conflicts: # tools/wta/src/master/mod.rs # tools/wta/src/protocol/acp/client.rs
Addresses Copilot review: - path_eq_ci now strips \\?\ / \\.\ prefixes before comparison, so a verbatim path like \\?\C:\Windows\System32 is recognized as junk by is_junk (previously it could slip through as a seed cwd). Added a pick_value assertion. - try_session_cwd_candidates logs a benign retryable cwd-rejection at info! (the fallback ladder working as designed) and reserves warn! for the terminal/non-retryable failure, so logs don't look unhealthy. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Per review feedback: with only two namespaces an agent can want, the Option/None "indeterminate" result and the drive-relative special case were needless. classify() is now binary — POSIX iff the path starts with '/', Windows otherwise (drive-letter, UNC, extended-length all fall out naturally). Simplifies to_windows_format/to_linux_format (no None arm) and user_profile_dir's HOME check. Strengthened tests: classify covers bare-drive/forward-slash/UNC/verbatim/relative + leading-whitespace POSIX; detect_format skips blank entries; added a Windows<->Linux round-trip. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
…false positive The round-trip test used C:\repo\sub; check-spelling parsed the backslash sequence into an unrecognized "epo" token. Reuse the already-allowed C:\Users\me literal (same coverage), no behavior change. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
A custom ACP agent launched inside WSL (e.g.
wsl.exe -d Ubuntu -- /home/<user>/.local/bin/copilot --acp --stdio) fails everysession/newwith:The cwd sent to
session/newcomes fromstd::env::current_dir()(the packaged helper's own dir =C:\WINDOWS\system32). A WSL agent validates the path in the Linux namespace and rejects it; a Windows-native agent accepts that same path, which is why the bug only ever reproduced with WSL agents.Approach (wrapper-proof — no launcher/profile parsing)
Learn the agent's path namespace from the agent itself, then convert the cwd into it. Validated empirically against both WSL and native copilot (
session/listreturns each agent's cwds in its own namespace;session/newrejects bad paths with a recognizable error).New module
cwd_format.rs:detect_format()— infer Windows vs POSIX from the cwds the agent reports insession/list(leading/= POSIX, drive-letter = Windows). Authoritative regardless of how the agent was launched (wsl.exe, a.cmdwrapper,cmd /c …).pick_value()— drop junk launcher dirs (System32,Windows) and empty down to%USERPROFILE%, so we never seed a session in System32.to_windows_format()/to_linux_format()— idempotent converters (C:\x⇄/mnt/c/x; non-/mntPOSIX →%USERPROFILE%). The caller just calls the one matching the agent and never reasons about the source format.build_attempts()+looks_like_cwd_error()— ordered candidates with a/tmpfloor for POSIX.wta-masterdetects the namespace once at startup via the agent'ssession/list(capability-gated; empty/unsupported → unknown → try both) and caches it.new_sessionconverts the incoming cwd into that namespace and retries down the candidate list, letting the agent's own cwd-rejection error drive the fallback.load_sessiondoes the single-shot conversion.The direct (non-master)
run_innerpath mirrors the same logic on its own connection.Behavior
/home/yeelam)Q:\repo)/mnt/q/repo%USERPROFILE%→/mnt/c/Users/<you>%USERPROFILE%session/list)/tmpTests
8 unit tests in
cwd_format.rs(classify, detect_format, idempotent converters, pick_value junk-drop, build_attempts for each target); all pass. 35/35 existing master tests pass;cargo build --target x86_64-pc-windows-msvcclean.Verification
After the fix,
wta-main_master.logshowsnew_sessionlanding on a namespace-correct cwd (/home/<you>or/mnt/...) instead ofC:\WINDOWS\system32, and the WSL agent pane reaches a healthy prompt.Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com