feat(acp): migrate system/file Tauri commands to ACP+#8849
feat(acp): migrate system/file Tauri commands to ACP+#8849fresh3nough wants to merge 2 commits intoaaif-goose:mainfrom
Conversation
fresh3nough
force-pushed
the
issue-8692-system-acp-migration
branch
from
e01f4de to
57620c9
Compare
![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: e01f4def4d
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| return Promise.resolve("/tmp/avatars"); | ||
| case "save_persona_avatar_bytes": | ||
| return Promise.resolve("avatar.png"); |
There was a problem hiding this comment.
These invoke stubs were removed, but the Playwright ACP mock still has no _goose/system/* handlers; unrecognized ACP calls fall through to {} in handleAcpRequest. Since ui/goose2/src/shared/api/system.ts now calls typed ACP methods (GooseSystemHomeDir, GooseSystemPathExists, etc.), the generated zod parsers expect fields like path/exists and throw in mocked runs, which breaks or degrades filesystem-dependent UI flows under the e2e fixture.
Useful? React with 👍 / 👎.
Move `get_home_dir`, `save_exported_session_file`, `path_exists`, `list_directory_entries`, `inspect_attachment_paths`, `list_files_for_mentions` and `read_image_attachment` from the goose2 Tauri shell into a new `goose::system_ops` module exposed over ACP+ via the `_goose/system/*` namespace. The desktop shell now opens save dialogs via the Tauri JS `@tauri-apps/plugin-dialog` plugin and persists the chosen file via a generic `_goose/system/write_file` ACP method. The Tauri shell drops its `base64`, `dirs`, `ignore`, and `mime_guess` dependencies along with the migrated commands, keeping the shell layer thin per the goose2 architecture rule. Existing system_ops behaviour is exercised through ported unit tests under `crates/goose/src/system_ops.rs`. Closes aaif-goose#8692 Signed-off-by: fresh3nough <anonwurcod@proton.me>
fresh3nough
force-pushed
the
issue-8692-system-acp-migration
branch
from
57620c9 to
c5e13ee
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c5e13ee73d
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| serde = { version = "1", features = ["derive"] } | ||
| serde_json = "1" | ||
| dirs = "6.0.0" | ||
| log = "0.4.29" | ||
| tokio = { version = "1.50.0", features = ["full"] } | ||
| uuid = { version = "1", features = ["v4", "serde"] } |
There was a problem hiding this comment.
dirs to goose2 dependencies
The dirs crate was removed from ui/goose2/src-tauri/Cargo.toml, but this package still imports dirs::home_dir/dirs::data_dir in several compiled modules (for example commands/path_resolver.rs, commands/projects.rs, services/personas.rs, and services/acp/goose_serve.rs). Without a direct dependency, the goose2 Tauri crate will fail to build with unresolved-crate errors, so desktop checks/builds are blocked until dirs is restored (or every remaining dirs:: usage is removed).
Useful? React with 👍 / 👎.
…merge-after-nits, aaif-goose/goose#8846 merge-as-is
The system_ops migration in c5e13ee dropped `dirs` from the goose2 Tauri shell, but several pre-existing files still rely on it for home/data directory lookups: - src/commands/agent_setup.rs (PATH building) - src/commands/path_resolver.rs (~ expansion) - src/commands/projects.rs (projects dir) - src/services/acp/goose_serve.rs (default working dir) - src/services/personas.rs (personas/avatars/agents dirs) Re-add `dirs = "6.0.0"` so `cargo clippy -- -D warnings` and the desktop build succeed again. The migrated system_ops module continues to use the dependency from `crates/goose` only. Signed-off-by: fresh3nough <anonwurcod@proton.me>
1 participant
Migrates the remaining file/system Tauri commands listed in #8692 to ACP+ so the goose2 desktop shell stays a thin wrapper over
goose serve.Summary
goose::system_opsmodule containing the filesystem/attachment/mention helpers that previously lived inui/goose2/src-tauri/src/commands/system.rs._goose/system/*namespace via custom requests defined incrates/goose-sdk/src/custom_requests.rsand handlers incrates/goose/src/acp/server.rs.ui/goose2/src/shared/api/system.tsto call the generated SDK methods instead ofinvoke(). Existing call sites are unchanged because the public function signatures stay the same.@tauri-apps/plugin-dialog(a shell-side concern) and then writes through the new generic_goose/system/write_fileACP method.base64,dirs,ignore, andmime_guesscrates from the goose2 Tauri shell, removes the registeredcommands::system::*invoke handlers, and updates the Playwright Tauri mock to drop the migrated invoke cases.Migrated commands
commands::system::get_home_dir→_goose/system/home_dircommands::system::save_exported_session_file→@tauri-apps/plugin-dialog+_goose/system/write_filecommands::system::path_exists→_goose/system/path_existscommands::system::list_directory_entries→_goose/system/list_directory_entriescommands::system::inspect_attachment_paths→_goose/system/inspect_attachment_pathscommands::system::list_files_for_mentions→_goose/system/list_files_for_mentionscommands::system::read_image_attachment→_goose/system/read_image_attachmentTests
crates/goose/src/system_ops.rsso the migrated logic is still covered (cargo test -p goose system_ops→ 14 passing).cargo fmt --all -- --check,cargo clippy -p goose --all-targets -- -D warningspnpm testinui/goose2(475 passing) andjust check(biome + file-size + i18n + typecheck)Closes #8692