Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9cd0731ad8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const workingDir = await resolveSessionCwd( | ||
| nextProject, | ||
| activeWorkspace?.path, | ||
| ); |
There was a problem hiding this comment.
handleProjectChange resolves the next session cwd using activeWorkspace?.path, but this value still points to the previous project's worktree at the moment the project is changed. Because resolveSessionCwd prioritizes activeWorkspacePath, ACP can be re-prepared with the old project directory before clearActiveWorkspace runs, so messages may continue executing in the wrong filesystem location (especially when no model is selected, since useChat may skip a later re-prepare). Resolve cwd from nextProject alone (or clear workspace first) during project switches.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6d09464cab
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
|
|
||
| export async function defaultGlobalArtifactRoot(): Promise<string> { | ||
| return (await resolvePath({ parts: ["~", ".goose", "artifacts"] })).path; |
There was a problem hiding this comment.
defaultGlobalArtifactRoot now returns the Tauri-resolved path verbatim, which is backslash-separated on Windows. Downstream artifact-policy logic still assumes /-separated roots (for example homeArtifactsRootFromRoots and stripRootArtifactsSegment in ArtifactPolicyContext), so Windows users can lose home-artifact root detection and fallback artifact link resolution/open behavior. Normalize this returned root (or normalize roots before policy checks) to keep path matching consistent across platforms.
Useful? React with 👍 / 👎.
baxen
left a comment
baxen
left a comment
There was a problem hiding this comment.
This looks good directionally. A few followups around the default path and artifact dir conventions but i plan to just send a followup PR instead - i think we can merge this one!
|
Yes I think this is the right way to do this. An alternative would be https://v2.tauri.app/plugin/file-system/. As for goose core, I think we'll want to settle on how projects should be implemented before we'd move it into there |
3 participants
Summary
This PR cleans up ui/goose2 ACP session working directory handling.
Changes
Renamed path-related variables and state so their purpose is clearer.
Introduced
sessionCwdSelection.tsto centralize ACP session working directory resolution.Added a Tauri-backed path resolver so path joining and ~ expansion are centralized instead of built ad hoc in ts.
Cleaned up the ACP boundary so acpPrepareSession now requires a real workingDir.
Discussion
I intentionally kept the Tauri backend thin in this PR. The backend only handles low-level path resolution, while frontend still decides which path should be used for the ACP session working directory. This gives us one place for OS-specific path handling like joining segments and expanding ~, without moving app concept (eg project) into the Tauri backend.
I also thought about pushing more of this into goose serve or even Goose core. But it becomes a bigger design question, we need to decide whether Goose core should know about project-related concepts , and if so, how the domain like project, workspace, artifact root, and session working directory should be represented (eg domain design)
Testing
Related Issues
Relates to #ISSUE_ID
Discussion: LINK (if any)
Screenshots/Demos (for UX changes)
Before:
After: