Bundle Goose behind a single runtime choice - #7858
salman1993 wants to merge 11 commits into
Conversation
Signed-off-by: Salman Mohammed <smohammed@squareup.com>
🔐 Codex Security Review
|
Signed-off-by: Salman Mohammed <smohammed@squareup.com>
|
🤖 Updated onboarding screenshot. One Goose choiceGoose uses its existing icon and the bundled executable in internal macOS builds. Buzz remains recommended.
|
salman1993
changed the title
Signed-off-by: Salman Mohammed <smohammed@squareup.com>
Signed-off-by: Salman Mohammed <smohammed@squareup.com> # Conflicts: # desktop/src-tauri/src/managed_agents/discovery/tests.rs
|
🤖 Reviewed
I did not run the full test suite or signed-app flow; these are source-path findings. The companion packaging changes in squareup/buzz-releases#99 are needed for the installed sidecar and provenance manifest. |
|
🤖 Checked current We will preserve that behavior in this PR: bundled defaults will fill missing values only below the existing Buzz overrides and Goose file settings. Switching to the bundled executable should not itself switch an existing user's provider/model. We will also align settings display and create-agent validation with that precedence, including honoring advanced |
Signed-off-by: Salman Mohammed <smohammed@squareup.com>
Signed-off-by: Salman Mohammed <smohammed@squareup.com>
Signed-off-by: Salman Mohammed <smohammed@squareup.com>
Signed-off-by: Salman Mohammed <smohammed@squareup.com>
Signed-off-by: Salman Mohammed <smohammed@squareup.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![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: b1c2aa92dd
ℹ️ 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".
| if file_value.is_none() && std::env::var(&key).is_err() { | ||
| env.entry(key).or_insert(value); |
There was a problem hiding this comment.
When Desktop inherits GOOSE_PROVIDER or GOOSE_MODEL from its parent process and no Goose file value exists, this condition suppresses the bundled fallback but never copies the inherited value into EffectiveAgentEnv. goose_requirements only checks that map and the config file, so it reports the corresponding field missing and can start the otherwise configured agent in setup mode, even though the spawned child would inherit the value. Insert the inherited value into the effective map at the intended precedence, or make readiness evaluate the same inherited environment as spawn.
Useful? React with 👍 / 👎.
Signed-off-by: Salman Mohammed <smohammed@squareup.com>
Signed-off-by: Salman Mohammed <smohammed@squareup.com>
GitHub Actions
Error
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 514c970c28
ℹ️ 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".
| if args.is_empty() { | ||
| args.push("acp".into()); | ||
| } |
There was a problem hiding this comment.
When a bundled Goose agent has nonempty direct-sidecar arguments, such as --with-builtin developer, remote deployment changes the executable to the external goose CLI but adds acp only when the argument list is empty. The resulting payload runs goose --with-builtin developer rather than goose acp --with-builtin developer, so it starts the normal CLI instead of an ACP process and the deployed harness cannot connect. Prepend acp whenever translating bundled Goose to the remote CLI, while avoiding a duplicate for legacy argument lists that already begin with it.
Useful? React with 👍 / 👎.
| let remote_goose = !local | ||
| && cfg!(all(feature = "bundled-goose", target_os = "macos")) | ||
| && matches!(effective_command.as_str(), "goose" | "goose-acp"); | ||
| let runtime_meta = known_acp_runtime(&effective_command); |
There was a problem hiding this comment.
In a bundled build, a custom harness pointing to an absolute external Goose binary such as /opt/homebrew/bin/goose is normalized by known_acp_runtime to the bundled Goose metadata. The local descriptor therefore applies the internal provider/model defaults even though custom catalog entries intentionally expose an empty configuration_defaults map; with no explicit or file settings, the supposedly external harness silently launches with the internal provider and model, and its UI metadata disagrees with spawn behavior. Determine bundled-default eligibility from the selected runtime/catalog entry or canonical bundled command rather than from the executable basename alone.
Useful? React with 👍 / 👎.
|
closing this PR since we decided to bundle goose in the new buzz 1.0 app. just merged here: block/buzz-app#497 |
This branch was successfully deployed
1 participant
Summary
Show one Goose choice with its existing icon. In internal macOS builds, it uses the pinned executable included with Buzz. Buzz Agent remains the default.
Existing Goose runtime selections use the bundled executable on their next local launch. Saved
goose-bundledpilot selections load asgoose. Builds without bundling retain the external Goose CLI.Build
goose-acpfrom a pinned upstream commit using the lean profile with native TLS and keyring support. Resolve the bundled executable from the app, and accept build-supplied provider/model defaults below existing Buzz selections, environment overrides, and Goose file settings. Launch, settings, and model discovery honor existing file settings. Build defaults travel separately from custom harness overrides. The bundled model applies only when the effective provider matches its bundled provider. Remote deployments preserve the existinggoose acpcommand and explicit user settings, without bundled provider/model defaults. Absolute external Goose pins remain supported.The bundle is pinned to Goose
4dea9b483efbd2541d43500b8ed3c044c65e6d2f, which includes the successful-tool-call warning fix and live model metadata. Theonline-model-metafeature is enabled, but this revision only initializes it in the full CLI. The leangoose-acpexecutable still uses the embedded catalog until upstream adds startup initialization.The existing Goose configuration and credential locations are shared. This pilot does not establish parity for upstream shell cancellation or output limits.
Related issue
Related to #7742
Testing
Onboarding screenshot showing the single Goose choice.
just bundled-goose: built the pinned Apple Silicon artifact, about 15 MiB; only system dynamic-library dependencies.BUZZ_TEST_GOOSE_ACPset to the staged binary andBUZZ_TEST_BIN_DIRset to the built Buzz binaries,cargo test -p buzz-acp real_goose_native_git_shell -- --ignored --nocapturepassed. The real Goose process used a scripted local provider and verified signed commits/tags, identity, credential scope, and key cleanup.BUZZ_BUILD_BUNDLED_GOOSE_PROVIDER=databricks_v2 BUZZ_BUILD_BUNDLED_GOOSE_MODEL=test-model cargo test --manifest-path desktop/src-tauri/Cargo.toml --features bundled-goose --libpassed. Focused regressions also cover settings-display precedence for explicit and inherited model/provider choices.The model-discovery regression was reproduced locally before the fix: a competing catalog entry replaced the bundled default. The strengthened test waits for that entry to appear before checking the default. All four Goose onboarding flows and the full local
just cigate pass after the review fixes. The bundled-feature suite now passes 3,393 tests (19 ignored), including saved pilot selections, discovery subprocess environment, provider/model pairing, and the shared remote deployment fixture. macOS CI runs this suite with populated build defaults.Review regressions:
pnpm --dir desktop test:e2e:smoke onboarding-agent-defaults.spec.ts --grep "bundled Goose|create Goose"passed all four flows. These cover file choices, environment overrides, provider switching, and creation without saved defaults. Removing the provider-pairing fix makes the switching test fail. Isolated native tests read a real Goose config file, verify the discovery child environment, and check launch/display precedence. Independent agent review found no remaining blockers.The review fixes need a fresh human retest before marking this PR ready. With the existing internal build environment, run
just desktop-standalone --features bundled-goosein the PR checkout; no Goose artifact rebuild is needed. Verify that existing Goose settings survive and that switching providers does not retain the bundled Databricks model when there is no explicit/file model override. Then send a prompt using the intended provider.Signed app packaging, Intel macOS, and live Databricks/relay use still need qualification in the internal build.
Generated with Codex