Skip to content

Add snapshot test with platform extensions#7573

Merged
jamadeo merged 2 commits intomainfrom
jackamadeo/system-snapshot-platform-extensions
Feb 27, 2026
Merged

Add snapshot test with platform extensions#7573
jamadeo merged 2 commits intomainfrom
jackamadeo/system-snapshot-platform-extensions

Conversation

@jamadeo
Copy link
Copy Markdown
Collaborator

@jamadeo jamadeo commented Feb 27, 2026

Summary

Type of Change

  • Feature
  • Bug fix
  • Refactor / Code quality
  • Performance improvement
  • Documentation
  • Tests
  • Security fix
  • Build / Release
  • Other (specify below)

AI Assistance

  • This PR was created or reviewed with AI assistance

Testing

Related Issues

Relates to #ISSUE_ID
Discussion: LINK (if any)

Screenshots/Demos (for UX changes)

Before:

After:

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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());
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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());
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

@alexhancock alexhancock self-requested a review February 27, 2026 17:01
@jamadeo jamadeo merged commit 6e630f6 into main Feb 27, 2026
17 of 21 checks passed
@jamadeo jamadeo deleted the jackamadeo/system-snapshot-platform-extensions branch February 27, 2026 17:40
tlongwell-block added a commit that referenced this pull request Feb 27, 2026
…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
tlongwell-block added a commit that referenced this pull request Feb 27, 2026
…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).
craigwalkeruk pushed a commit to craigwalkeruk/custom-goose that referenced this pull request Mar 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants