feat: workspace browser with --workspace-root opt-in scoping#526
feat: workspace browser with --workspace-root opt-in scoping#526tiann merged 7 commits intotiann:mainfrom
Conversation
Add /browse route with a folder browser that lets users navigate filesystem directories on connected machines and launch sessions from any folder. Supports saved workspace paths and direct path input. The "Start Session" action pre-fills the NewSession form. - CLI: register machine-level `list-directory` RPC handler - Hub: add POST /machines/:id/list-directory route - Web: add WorkspaceBrowser component with git repo detection - Web: add /browse route with navigation from sessions sidebar - Web: support initialDirectory/initialMachineId in NewSession Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Adds a single new flag, \`--workspace-root <path>\` (with \`~\` / \`~/foo\` expansion), on \`hapi runner start\` and \`hapi runner start-sync\`. When set: - The runner reports the path in machine metadata. - The list-directory and spawn-session RPC handlers reject paths outside the root, so the web UI can't escape the configured tree even if someone crafts a request manually. - The /browse page in the web UI auto-opens that root, restricts the breadcrumb / go-up to its subtree, and shows directory entries with git-repo annotations. - The /sessions/new form keeps its existing free-text directory input plus autocomplete + recent-paths chips, and gains a small "Browse" button (next to the input) that opens /browse for picking a folder. - Reconnect-time metadata sync ensures stale records get the field filled in (or cleared when the flag is dropped on a later restart), so the hub state matches the CLI's intent. When unset: - Runner behaves like the legacy hapi (no scoping, no browse feature). - /browse renders an informative state pointing at the flag instead of blocking the user. - The /sessions/new form looks identical to the pre-change behavior; the "Browse" button is hidden. Includes a startup banner so \`runner start-sync\` no longer looks like it hung, and surfaces the workspace-root sync result on stdout. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
MachineCache.refreshMachine() rebuilt the metadata object from an explicit field allowlist, so any field not in the list (including the new workspaceRoot) was silently dropped on every read — even though it was correctly written to the store. Add workspaceRoot to the zod schema, the Machine interface, and the hand-rolled projection. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
When there are zero sessions the page used to be a vast blank rectangle with just the "0 sessions in 0 projects" caption. Render a centered empty state instead: a calendar/agenda icon, a short heading and hint, and two buttons — "Start a session" (→ /sessions/new) and "Browse workspace" (→ /browse). SessionList gains an optional onBrowse prop. Router wires it on the sessions page so the secondary button resolves; other callers can leave it unset to hide that button. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Add a short paragraph under "Runner management" in cli/README.md explaining what \`--workspace-root\` enables (scoped /browse tree, list/spawn enforcement, tilde expansion) and that omitting it keeps the legacy behavior. Mention the workspace browser in the top-level README's Features list. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Findings
-
[Major] Canonicalize workspace paths before enforcing scope —
resolvePath()only normalizes the string, butstat,readdir, and session spawn follow symlinks. WithworkspaceRoot=/safe, a symlink such as/safe/out -> /etcpasses the lexical check and lets/machines/:id/list-directoryorspawn-happy-sessionoperate outside the configured root. Evidencecli/src/api/apiMachine.ts:133,cli/src/api/apiMachine.ts:208.
Suggested fix:import { realpath, readdir, stat } from 'node:fs/promises' import { realpathSync } from 'node:fs' import { basename, dirname, isAbsolute, join, relative, resolve as resolvePath } from 'node:path' this.normalizedWorkspaceRoot = workspaceRoot ? realpathSync(workspaceRoot) : undefined private async resolveForWorkspaceCheck(path: string): Promise<string> { const absolute = resolvePath(path) try { return await realpath(absolute) } catch { const missing: string[] = [] let cursor = absolute while (cursor !== dirname(cursor)) { missing.unshift(basename(cursor)) cursor = dirname(cursor) try { return join(await realpath(cursor), ...missing) } catch { // keep walking to the nearest existing parent } } return absolute } }
-
[Minor] Reject a bare
--workspace-rootinstead of silently dropping it — whenhapi runner start --workspace-rootis passed without a value,valueisundefined, the flag is spliced out, and the runner starts withworkspaceRoot === undefinedinstead of failing. That can leave the detached runner in legacy unscoped mode after the user attempted to enable scoping. Evidencecli/src/commands/runner.ts:28.
Suggested fix:if (arg === '--workspace-root') { const next = args[i + 1] if (!next || next.startsWith('--')) { console.error('--workspace-root requires a non-empty path') process.exit(1) } value = next args.splice(i, 2) }
-
[Minor] Preserve the selected machine when opening Browse from New Session —
NewSessionpasses{ machineId, directory }toonChooseFolder, butNewSessionPageignores it and/browsefalls back tohapi:lastMachineIdor the first machine. If the user selects a different machine and clicks Browse before creating a session, the browser can open the wrong machine. Evidenceweb/src/router.tsx:419,web/src/components/WorkspaceBrowser.tsx:115.
Suggested fix:const handleChooseFolder = useCallback((args: { machineId: string | null }) => { navigate({ to: '/browse', search: args.machineId ? { machineId: args.machineId } : {} }) }, [navigate])
Summary
Review mode: initial
Scope enforcement needs canonical path checks before this ships. The other two findings are smaller but directly affect the new opt-in workflow and multi-machine browse behavior.
Testing
Not run (automation; bun typecheck failed because bun is not installed in this runner).
HAPI Bot
| return { success: false, error: 'Path is required' } | ||
| } | ||
|
|
||
| const targetPath = resolvePath(rawPath) |
There was a problem hiding this comment.
[MAJOR] Canonicalize workspace paths before enforcing scope.
resolvePath() only normalizes the string, but stat, readdir, and later session spawn follow symlinks. With workspaceRoot=/safe, a symlink such as /safe/out -> /etc passes this lexical check and lets browse/spawn operate outside the configured root. Canonicalize the workspace root and requested path, resolving the nearest existing parent for paths that may be created.
Suggested fix:
import { realpath, readdir, stat } from 'node:fs/promises'
import { realpathSync } from 'node:fs'
import { basename, dirname, isAbsolute, join, relative, resolve as resolvePath } from 'node:path'
this.normalizedWorkspaceRoot = workspaceRoot ? realpathSync(workspaceRoot) : undefined
private async resolveForWorkspaceCheck(path: string): Promise<string> {
const absolute = resolvePath(path)
try {
return await realpath(absolute)
} catch {
const missing: string[] = []
let cursor = absolute
while (cursor !== dirname(cursor)) {
missing.unshift(basename(cursor))
cursor = dirname(cursor)
try {
return join(await realpath(cursor), ...missing)
} catch {
// keep walking to the nearest existing parent
}
}
return absolute
}
}| for (let i = 0; i < args.length; i++) { | ||
| const arg = args[i] | ||
| let value: string | undefined | ||
| if (arg === '--workspace-root') { |
There was a problem hiding this comment.
[MINOR] Reject a bare --workspace-root instead of silently dropping it.
For hapi runner start --workspace-root, value is undefined, the flag is spliced out, and the runner starts with no workspace scope. That leaves the detached runner in legacy unscoped mode after the user attempted to enable scoping.
Suggested fix:
if (arg === '--workspace-root') {
const next = args[i + 1]
if (!next || next.startsWith('--')) {
console.error('--workspace-root requires a non-empty path')
process.exit(1)
}
value = next
args.splice(i, 2)
}| }) | ||
| }, [navigate, queryClient]) | ||
|
|
||
| const handleChooseFolder = useCallback(() => { |
There was a problem hiding this comment.
[MINOR] Preserve the selected machine when opening Browse from New Session.
NewSession passes { machineId, directory } to this callback, but the route ignores it and /browse falls back to hapi:lastMachineId or the first machine. If the user selects a different machine and clicks Browse before creating a session, the browser can open the wrong machine.
Suggested fix:
const handleChooseFolder = useCallback((args: { machineId: string | null }) => {
navigate({
to: '/browse',
search: args.machineId ? { machineId: args.machineId } : {}
})
}, [navigate])Three findings from the review bot: 1. [Major] Workspace-scope check was lexical only. With workspaceRoot = /safe, a symlink such as /safe/out -> /etc would pass the relative- path test and let list-directory / spawn-happy-session reach paths outside the configured root. realpath the workspaceRoot at construction time, and resolve every incoming path through realpath (walking up to the nearest existing parent for spawn targets that haven't been created yet) before the containment check. 2. [Minor] \`hapi runner start --workspace-root\` with no value used to drop the flag silently and start the runner unscoped. Now treats a missing or flag-shaped next argument as an error. 3. [Minor] /sessions/new's "Browse" button always opened /browse using localStorage's last-used machine, ignoring the user's current selection. NewSession already passes machineId in its callback; forward it through the /browse search params and seed WorkspaceBrowser with it. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Thanks for the review! All three findings addressed in 7ef8d26.
Typecheck is green across cli/web/hub. |
|
这次主要就加了一个 Workspace Root 概念。 视频介绍: https://www.bilibili.com/video/BV1UPodBjE6T 核心思路用户启动 runner 的时候带上 服务端会同时校验:所有 完全可选 / 向后兼容不传 也就是说老用户升级后什么都不用改,看不到任何变化;想用这个功能就加一个参数。 顺手带的两个小东西
用法# 关闭(默认,老行为)
hapi runner start
# 开启
hapi runner start --workspace-root ~/focus/
# 之后 Web 上 /browse 就能浏览 ~/focus/整个目录树
# 任意子目录都能直接 "Start Session" |
There was a problem hiding this comment.
Findings
- [Major] Enforce the browse opt-in on the runner RPC — when
--workspace-rootis omitted,normalizedWorkspaceRootisundefined,isWithinWorkspaceRoot()returnstrue, and the new/api/machines/:id/list-directorypath can still be called directly to list any directory on that runner. The UI hides Browse for no-root machines, but the backend/CLI does not enforce the documented “no/browsefeature” behavior. Evidencecli/src/api/apiMachine.ts:140.
Suggested fix:this.rpcHandlerManager.registerHandler<ListMachineDirectoryRequest, ListMachineDirectoryResponse>('list-directory', async (params) => { if (!this.normalizedWorkspaceRoot) { return { success: false, error: 'Workspace browsing is not enabled for this machine' } } const rawPath = typeof params?.path === 'string' ? params.path.trim() : '' // ...existing validation/listing logic })
Summary
- Review mode: follow-up after new commits
- The prior bot findings appear addressed, but the final state still exposes the new directory listing RPC without requiring the workspace-root opt-in. This is a backend enforcement gap, not just a UI state issue.
Testing
- Not run (automation; static review only). Suggested coverage: runner RPC test that
list-directoryreturns an error whenworkspaceRootis unset, and succeeds only inside a configured root.
HAPI Bot
| return { exists } | ||
| }) | ||
|
|
||
| this.rpcHandlerManager.registerHandler<ListMachineDirectoryRequest, ListMachineDirectoryResponse>('list-directory', async (params) => { |
There was a problem hiding this comment.
[MAJOR] Enforce the browse opt-in on the runner RPC.
When --workspace-root is omitted, normalizedWorkspaceRoot is undefined, isWithinWorkspaceRoot() returns true, and the new /api/machines/:id/list-directory path can still be called directly to list any directory on that runner. The UI hides Browse for no-root machines, but the backend/CLI does not enforce the documented “no /browse feature” behavior.
Suggested fix:
this.rpcHandlerManager.registerHandler<ListMachineDirectoryRequest, ListMachineDirectoryResponse>('list-directory', async (params) => {
if (!this.normalizedWorkspaceRoot) {
return { success: false, error: 'Workspace browsing is not enabled for this machine' }
}
const rawPath = typeof params?.path === 'string' ? params.path.trim() : ''
// ...existing validation/listing logic
})|
Video introduction: https://www.bilibili.com/video/BV1UPodBjE6T |
Without a configured workspaceRoot, isWithinWorkspaceRoot() returns true unconditionally, leaving the new list-directory RPC able to enumerate any path on the runner. The Web UI already hides Browse for these machines, but the backend should enforce the opt-in too. Refuse the RPC up front when no workspace root is configured.
There was a problem hiding this comment.
Findings
No findings.
Summary
Review mode: follow-up after new commits
No high-confidence issues found in the latest full diff. The prior bot finding about enforcing workspace browsing opt-in on the runner RPC appears addressed by the new list-directory guard.
Residual risk: static review only; I could not verify TypeScript/tests in this runner because bun is unavailable. Suggested coverage remains the workspace-root boundary behavior: list-directory disabled with no root, rejects outside-root paths, rejects symlink escapes, and spawn-session rejects outside-root directories.
Testing
Not run (automation; bun typecheck failed because bun is not installed in this environment).
HAPI Bot
What
Adds a workspace-scoped file browser to the web UI. Users opt in by passing
--workspace-root <path>tohapi runner start; without the flag everything behaves exactly like before.How it looks
/browsepage (file tree with breadcrumbs, git-repo annotations, "Start Session" CTA at the bottom)/sessionsheader that navigates to/browse/sessions/new(only shown when the selected machine has a workspace root)/sessionswhen zero sessions exist (icon + two CTAs, replaces the previous large blank area)Backward compatibility
Without
--workspace-root:hapi runner startworks exactly as today.workspaceRoot.list-directory/spawn-sessionRPC handlers do no path scoping (legacy behavior)./browseshows an informative "browsing is off" page (not a blocking error) pointing at the flag, with the standard sessions/new flow still fully usable./sessions/newlooks identical to today (text input + autocomplete + recent paths chips); the new "Browse" button is hidden.With
--workspace-root /some/path:{success:false, error:"Path is outside workspace root"}).~/foo) are expanded if the shell didn't.Commits
feat(web): add workspace browser for multi-directory navigation— base WorkspaceBrowser component +/browseroute, Hublist-directoryplumbing, web client wiring.feat: add --workspace-root opt-in scoping for /browse and session spawn— the flag, the metadata field, RPC enforcement, reconnect-time sync, browse-vs-no-browse UI states, "Browse" button on NewSession, startup banner so `runner start-sync` is no longer silent.fix(hub): preserve workspaceRoot when rehydrating machines from store—MachineCache.refreshMachine()rebuilt metadata from a hand-rolled field allowlist that dropped any new field. Without this fix the hub silently strips `workspaceRoot` on every read.feat(web): friendlier empty state on /sessions— replaces the empty area with a centered icon + heading + two CTAs.docs: document --workspace-root flag in cli/README and root README.Test plan
/foo` → `/browse` lands at `/foo`, breadcrumb stops at root, "Start Session" pre-fills directory.Notes
🤖 Generated with Claude Code