feat: configure workspace worktree storage#1268
Conversation
|
| Filename | Overview |
|---|---|
| packages/protocol/src/messages.ts | Adds WorkspaceSetWorktreeStoragePath request/response schemas and appends worktreeStoragePath to WorkspaceDescriptorPayloadSchema with .optional().default(null) — backward compatible. |
| packages/client/src/daemon-client.ts | Adds setWorkspaceWorktreeStoragePath RPC and passes worktreeStoragePath through createPaseoWorktree; missing daemon capability gate for the new RPC. |
| packages/server/src/server/worktree/commands.ts | Extends list/archive/slug-resolution commands to respect per-workspace storage paths; resolvePersistedWorktreeOwnership contains a dirname-fallback logic bug that bypasses ownership validation for workspaces with null worktreeStoragePath. |
| packages/server/src/server/session.ts | Adds handler for workspace.set_worktree_storage_path.request that validates the workspace, expands tilde, upserts the record, and emits an update — logic looks correct. |
| packages/server/src/utils/worktree.ts | Adds resolvePaseoWorktreesRoot helper and extends isPaseoOwnedWorktreeCwd/listPaseoWorktrees to accept custom storage root; implementation looks correct. |
| packages/server/src/server/workspace-registry.ts | Adds nullable optional worktreeStoragePath to PersistedWorkspaceRecordSchema with .transform fallback — properly backward compatible with existing records. |
| packages/server/src/server/paseo-worktree-service.ts | Routes createPaseoWorktree through custom storage path resolution and propagates it to upsertWorkspaceForWorktree; logic for inheriting from source workspace looks correct. |
| packages/app/src/screens/project-settings-screen.tsx | Adds WorktreeStorageEditor component with directory picker (Electron-only), inline save, and reset; UI pattern looks correct but depends on empty workspaceId fallback in projects.ts. |
| packages/app/src/utils/projects.ts | Adds workspaceId and worktreeStoragePath to ProjectHostEntry; canonical?.id ?? "" produces an empty string when no canonical workspace exists, which would cause a misleading error in the settings UI. |
| packages/server/src/server/workspace-git-service.ts | Passes worktreeStoragePath through listWorktrees and resolvePersistedWorktreeStorageRoot to worktree listing; cache key updated to include storage path, preventing stale results. |
Sequence Diagram
sequenceDiagram
participant UI as ProjectSettingsScreen
participant Client as DaemonClient
participant Session as Session (daemon)
participant Registry as WorkspaceRegistry
participant WS as WorkspaceGitService
UI->>Client: setWorkspaceWorktreeStoragePath(workspaceId, path)
Client->>Session: workspace.set_worktree_storage_path.request
Session->>Registry: get(workspaceId)
Registry-->>Session: PersistedWorkspaceRecord
Session->>Session: expandUserPath(path) → absolute
Session->>Registry: "upsert({ ...workspace, worktreeStoragePath })"
Session-->>Client: workspace.set_worktree_storage_path.response
Session->>Client: workspace_update (descriptor with worktreeStoragePath)
Note over UI,WS: On next createWorktree / listWorktrees / archive
Session->>Registry: list() → find source workspace
Registry-->>Session: worktreeStoragePath
Session->>WS: "createWorktree({ worktreeStoragePath })"
WS->>WS: resolvePaseoWorktreesRoot(cwd, worktreeStoragePath)
WS-->>Session: WorktreeConfig at custom path
Comments Outside Diff (1)
-
packages/server/src/server/worktree/commands.ts, line 183-213 (link)dirnamefallback bypasses ownership check for any registered worktreeresolvePersistedWorktreeOwnershipfalls through toworktreeRoot = workspace.worktreeStoragePath ?? dirname(normalizedTarget)whenworktreeStoragePathis null. BecausenormalizedTargetalways starts withdirname(normalizedTarget) + sep, theresolvedCwd.startsWith(customRootPrefix)test inisPaseoOwnedWorktreeCwdis always true, so any workspace record in the registry withkind: "worktree"andworktreeStoragePath: nullwill pass the ownership check even if the path is not under any Paseo-controlled directory. This is inconsistent with howcreate-agent-lifecycle-dispatch.tsandarchive-if-safe.tshandle the same case — both passworkspace.worktreeStoragePath ?? undefined, which skips the custom-root branch entirely and lets the default check fail normally.The fix is to return
null(skip the fallback) whenworktreeStoragePathis null; only the custom-storage code path needs this override.
Reviews (1): Last reviewed commit: "feat: configure workspace worktree stora..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| return { customName: payload.customName }; | ||
| } | ||
|
|
||
| async setWorkspaceWorktreeStoragePath( | ||
| workspaceId: string, | ||
| worktreeStoragePath: string | null, | ||
| requestId?: string, | ||
| ): Promise<{ worktreeStoragePath: string | null }> { | ||
| const payload: WorkspaceSetWorktreeStoragePathPayload = await this.sendCorrelatedSessionRequest( | ||
| { | ||
| requestId, | ||
| message: { | ||
| type: "workspace.set_worktree_storage_path.request", | ||
| workspaceId, | ||
| worktreeStoragePath, | ||
| }, | ||
| responseType: "workspace.set_worktree_storage_path.response", | ||
| timeout: 10000, | ||
| }, | ||
| ); | ||
| if (!payload.accepted) { | ||
| throw new Error(payload.error ?? "setWorkspaceWorktreeStoragePath rejected"); | ||
| } |
There was a problem hiding this comment.
setWorkspaceWorktreeStoragePath sends workspace.set_worktree_storage_path.request unconditionally. Per CLAUDE.md, any new RPC that requires a new daemon capability must be guarded by a server_info.features.* flag, with a // COMPAT(featureName): added in v0.X.Y comment marking the cleanup site. Without the gate, a new client connected to an old daemon will call this RPC, get no recognised response, and surface a timeout error in the project settings toast instead of the expected "Update the host to use this" message.
Context Used: CLAUDE.md (source)
| return { | ||
| serverId: group.serverId, | ||
| serverName: group.serverName, | ||
| workspaceId: canonical?.id ?? "", |
There was a problem hiding this comment.
canonical is undefined (e.g. a project whose only workspaces are worktrees), workspaceId falls back to "". The WorktreeStorageEditor will then call setWorkspaceWorktreeStoragePath("") which the server rejects with "Workspace not found", surfacing a confusing error toast. Prefer a more explicit sentinel so the UI can decide whether to render the editor at all.
| workspaceId: canonical?.id ?? "", | |
| workspaceId: canonical?.id ?? null, |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
1 participant
Summary
Verification