feat(webui): cross-platform picker paths with env expansion; drop native picker - #28
Merged
Merged
Conversation
basic-features/03: cross-platform path input, env/tilde expansion, and
removal of the native OS picker per ticket feedback. The native picker
(zenity/kdialog/osascript/PowerShell) was both unreliable in the
target environments and out of scope; the in-product
WorkspacePickerModal is now the only workspace-pick surface.
1. Cross-platform path expansion — server/lib/workspace.js
expandUserPath(input):
- '~', '~/foo', '~\bar' -> home + remainder
- '$HOME', '$USERPROFILE' -> POSIX-style
- '%USERPROFILE%', '%TEMP%' -> Windows-style
- '%VAR%' / '$VAR' inside path -> mid-path interpolation
- 'C:\...', '/abs/path' -> passes through; path.resolve
handles backslashes natively on
Windows, on POSIX a Windows-style
path survives verbatim so the user
sees an actionable ENOENT
- '"abs"' / ''abs'' -> wrapping quotes stripped
- empty / whitespace / non-string -> null (caller surfaces
"path required" error)
- allow-list for env vars: HOME / USERPROFILE / TMPDIR / TEMP /
TMP. Unknown $VAR / %VAR% is left literal — strict allow-list
is the standard shell-quoting answer; arbitrary interpolation
would let an unprivileged user probe env-var presence. TMPDIR/
TEMP/TMP fall through the alias chain so '%TEMP%' resolves
when only TMPDIR is set (the typical POSIX case).
2. Containment error carries the allowed roots — the picker renders
'Path must be under: ...' instead of a bare 'illegal':
- server/lib/workspace.js#browseWorkspace and #assertWorkspacePath
now return {ok:false, error, roots: [...]} on rejection;
roots[] is the actionable hint.
- lib/api.ts#browseWorkspace is rewritten to bypass the generic
request() helper — that helper throws on non-OK and would
strip the roots[] payload. The route returns the same body on
200 and 400; the picker treats them identically.
- components/panels.tsx WorkspaceBrowseTab stores errorRoots in
state and renders `data-testid="workspace-picker-error-roots"`
below the error message when present.
- i18n: 'workspace.picker.mustBeUnder' (en + zh).
3. Native picker removed:
- server/lib/workspace.js: pickDirectoryNative + tryKdialog +
the entire native-spawn block deleted.
- server/routes/workspace.js: handleWorkspacePick deleted;
pickDirectoryNative import dropped.
- server/app.js: 'POST /api/workspace/pick' route + 'routes/'
listing entry removed.
- webapp/components/panels.tsx: 'Open native picker' button +
nativePick + nativeSupported deleted. Toolbar now reads
'parent, path, home, root, mkdir' (no native).
- webapp/lib/api.ts: pickWorkspaceNative wrapper deleted.
- lib/i18n.ts: 'workspace.picker.native[Unsupported]' keys
deleted (en + zh).
- docs/API.md + API.zh-CN.md: section removed.
- test/server/app-hono.test.js: 'POST /api/workspace/pick'
route listing entry removed.
Tests
- test/lib/workspace-paths.check.mjs (new): 20 cases for
expandUserPath (POSIX + Windows shapes, mid-path interpolation,
trailing separators, wrapping quotes, allow-list strictness)
and the containment error payload. The Windows-shape assertions
use the verbatim env-var value; the platform-dependent join is
exercised separately by integration tests on POSIX.
- test/routes/workspace.check.mjs: 'browse containment rejection'
test pins the new error payload (status 400 + error + roots[],
no path).
- webapp/test/workspace-picker-paths.test.ts (new): 5 cases
scanning source for the native-picker removal and the errorRoots
UI slot. Same source-scan pattern as workspace-picker-wire.test.ts
from basic-features/02 — the webapp suite has no jsdom / RTL so
a real render smoke test is out of scope, and the previous
ticket's acceptance feedback established this is the lightweight
alternative the webapp side can afford.
Live self-check (mandatory per acceptance feedback):
PORT=18092 MCODE_WEBUI_DEV_FRONTEND_PORT=18093
MCODE_WEBUI_DATA_DIR=/tmp/dev-wpp-check pnpm webui:dev; agent-browser:
- chip click -> dropdown opens (no crash)
- choose-new -> Level-2 modal opens
- browse tab -> path input visible
- '~/acer09' + Enter -> listing shows real subdirs
(taikong-jiqi / pelican-saturn /
qingming / qingming-3d / review —
confirms tilde expansion landed
on /home/acer09/acer09)
- '/var/log/private-out-of-roots-dir' + Enter
-> error renders with the roots list:
'Path must be under: /home/acer09,
/home/acer09/.../demo002, /tmp'
- no-project -> chip text becomes 'tmp'
(no-workspace)
- toolbar no longer renders 'Open native picker'
Dev server stopped after.
Gates
- pnpm typecheck (root) - 0 errors
- pnpm --filter @mavis/webui webapp:typecheck - 0 errors
- pnpm test:webapp - 207 / 207 / 0 fail
- pnpm build - passes (6253 source files)
- pnpm check:source - passes (4562 files)
- targeted server tests (workspace-paths,
workspace.check, app-hono, models) - 43 pass / 1 skipped /
0 fail (the skip is a
win32-only case)
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
1 participant
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.
What
User feedback: the picker's path input must work across win/mac/linux with system environment variables and absolute paths — too many "path illegal" failures — and the native OS picker is not wanted.
expandUserPath(lib/workspace.js):~/~/foo/~\bar; allow-listed env vars$HOME/$USERPROFILE/$TMPDIR/$TEMP/$TMPand Windows-style%USERPROFILE%/%TEMP%/%TMP%with mid-path interpolation; alias fall-through (%TEMP%resolves viaTMPDIR); unknown variables left literal (no env probing); wrapping quotes stripped. Expansion runs BEFORE containment validation.{error, roots[]}and the picker renders "路径必须在以下位置之一: …" instead of a bare failure./api/workspace/pickroute + handler, lib helpers, webapp api wrapper, toolbar button, i18n keys, API docs — the UI was the only consumer.Acceptance (independent agent, live browser)
PASS-WITH-CONCERNS — all 5 ticket criteria met:
~/$HOME/文档expansion browsed; out-of-roots error renders the allowed-roots list and recovers without reload; adversarial~/..and$HOME/../../etcboth containment-rejected (realpath-based check after expansion); native picker gone (UI + grep); regression clean on Browse/mkdir/confirm/Recents/chip/conversation entry. Gates re-run fresh: typecheck 0, test:webapp 207/207, workspace-paths 20/20, build ✓, check:source ✓.Non-blocking notes: relative paths resolve against server CWD (documented; not the browsed dir); UNC paths uncovered;
%TEMP%with no TEMP/TMP/TMPDIR set falls back to HOME (intentional).Full
pnpm verifydeferred to CI (windows-latest will exercise the Windows path shapes).