Add snapshot test with platform extensions#7573
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 08f21a45c1
ℹ️ 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".
| let mut extensions: Vec<ExtensionInfo> = PLATFORM_EXTENSIONS | ||
| .iter() | ||
| .map(|(_, def)| { | ||
| let client = (def.client_factory)(context.clone()); |
There was a problem hiding this comment.
Avoid constructing side-effecting clients in this snapshot test
Calling def.client_factory here instantiates every platform extension, but at least one constructor (AppsManagerClient::new) performs filesystem writes to Paths::in_data_dir(...) via fs::create_dir_all and ensure_default_apps (crates/goose/src/agents/platform_extensions/apps.rs:101-114). That makes this unit snapshot test non-hermetic: it mutates user/CI state and can fail in read-only or home-less environments even though prompt rendering itself is pure.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e75beca6e0
ℹ️ 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".
| let mut extensions: Vec<ExtensionInfo> = PLATFORM_EXTENSIONS | ||
| .values() | ||
| .map(|def| { | ||
| let client = (def.client_factory)(context.clone()); |
There was a problem hiding this comment.
Avoid side effects when constructing platform clients
Instantiating every extension via def.client_factory here makes the snapshot test non-hermetic, because at least AppsManagerClient::new performs real filesystem setup (fs::create_dir_all and default app writes in crates/goose/src/agents/platform_extensions/apps.rs:101-114) under Paths::in_data_dir(...) rather than the test temp dir. In CI or developer environments with read-only/missing home directories (see Paths::get_dir expecting a home dir), this test can fail for environment reasons unrelated to prompt rendering and can also mutate persistent user state.
Useful? React with 👍 / 👎.
…m-extension-pr * origin/main: Update CODEOWNERS for team restructuring (#7574) Add snapshot test with platform extensions (#7573) Handle Bedrock 'prompt is too long' error (#7550) feat: make pctx/Code Mode an optional dependency via 'code-mode' feature (#7567) chore(release): release version 1.26.0 (minor) (#7512) feat: allow goose askai bot to search goose codebase (#7508) Revert "Reapply "fix: prevent crashes in long-running Electron sessions"" Reapply "fix: prevent crashes in long-running Electron sessions" Revert "fix: prevent crashes in long-running Electron sessions" fix: replace unwrap() with graceful error in scheduler execute_job (#7436) fix: Dictation API error message shows incorrect limit (#7423) fix(acp): Use ACP schema types for session/list (#7409) fix(desktop): make bundle and updater asset naming configurable (#7337) fix(openai): preserve order in Responses API history (#7500) Use the correct Goose emoji 🪿 instead of Swan in README.md (#7485) fix: prevent crashes in long-running Electron sessions
…ate snapshot JS/TS caller resolution: find_enclosing_fn now only treats variable_declarator as a function scope when its value is an arrow_function or function expression. Calls like const data = load() inside function process() now correctly attribute to process, not data. Instructions: removed reference to the developer extension's tree tool — our extension shouldn't tell agents about other extensions' tools. Snapshot: updated all_platform_extensions snapshot to include the analyze extension instructions (merged from #7573).
Summary
Type of Change
AI Assistance
Testing
Related Issues
Relates to #ISSUE_ID
Discussion: LINK (if any)
Screenshots/Demos (for UX changes)
Before:
After: