upgrade goose sdk and tui to be compatible with the latest agentclientprotocol/sdk package#8667
Conversation
…tprotocol/sdk package
* main: feat(hooks): add Husky git hooks for ui/goose2 (#8577) fix: links in chat could not be opened (#8544) fix: run setup before dev and dev-debug in goose2 justfile (#8718) Manage skills as sources over ACP (#8675) handle full node paths in goose2 kill recipe (#8709) overhaul provider inventory and agent/model selection (#8652) Remove unused import (#8676) delete the goose2 migration plan prompt (#8678) Add health score badge to README (#8677) feat(goose2): voice dictation via direct-ACP pattern (#8609) consistently use actions-rust-lang/setup-rust-toolchain (#8671) fix(developer): run shell tool under bash/sh regardless of login shell (#8659) refactor(providers): extract shared OAuth device-flow helper (#8619) Add a goose2 release workflow (#8629) chore(deps): bump ncipollo/release-action from 1.20.0 to 1.21.0 (#8664) docs: add blog post about Mesh LLM provider option (#8655) fix: append /chat/completions for prefixed v1 base URLs (#8521) Reset ChatGPT Codex auth during OAuth setup (#8569) chore(deps): bump EmbarkStudios/cargo-deny-action from 2.0.15 to 2.0.17 (#8665) Add dependabot config for pnpm workspace, cargo, and actions (#8660)
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bd306827d1
ℹ️ 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 resp = await client.goose.GooseProvidersModels({ | ||
| providerName: provider.name, | ||
| }); | ||
| const knownModels = provider.knownModels?.map((model) => model.name) ?? []; |
There was a problem hiding this comment.
Load models from provider inventory, not details metadata
This now builds the model picker from provider.knownModels, but that field comes from _goose/providers/details static metadata (crates/goose-acp/src/server.rs on_get_provider_details, lines 2839–2846) rather than the runtime inventory/refresh path (_goose/providers/inventory). As a result, providers that discover models dynamically can show an incomplete list here, so valid models may disappear from the picker unless users manually type them. Querying provider inventory for the selected provider (and optionally triggering refresh) would preserve the previous behavior of showing current model options.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cb4e5722bb
ℹ️ 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".
| await client.setSessionConfigOption({ | ||
| sessionId, | ||
| configId: "provider", | ||
| value: provider.name, | ||
| }); |
There was a problem hiding this comment.
Update provider and model atomically for sessions
Splitting session updates into two setSessionConfigOption calls creates a partial-failure path: this first call updates provider without a model, and the server’s update_provider path uses ACP_CURRENT_MODEL when switching providers without an explicit model (crates/goose-acp/src/server.rs, around update_provider fallback logic at lines 2389-2394). If the subsequent model update fails (for example, manual model entry typo or stale model list), the UI reports an error after the provider has already been changed, leaving the session in an unintended intermediate model state.
Useful? React with 👍 / 👎.
alexhancock
left a comment
There was a problem hiding this comment.
I tested with the TUI and it works great via cd ui/text && npm start
Thank you! |
|
Hi @alexhancock, Would you mind taking another look at items 2, 3, and 4 under ‘Questions and Discussions’? I would like to make sure the versions are correct (same version for all three npm packages or different versions are ok) Thank you! cc: @jamadeo |
* main: feat: extend goose2 context window ux with auto-compaction (#8721) improve goose2 agent management flows (#8737) alexhancock/tui-improvements (#8736) fix: add strict:false to Responses API tools and gpt-5.4 to known models (#8636) persist and reliably apply chat model selection (#8734) merge goose-acp crate into goose (#8726) docs: AGENTS.md section on goose2 desktop backend architecture (#8732) feat: goose2 message bubble + action tray (#8720) consolidate provider ACP methods onto inventory (#8710) ci: declare and enforce MSRV of 1.91.1 (#8670) fix(ui): correct grammar in apps view description (#8668) (#8679) Stop load openai fast model for openapi compatible custom endpoint (#8644)
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 78c96b4ae5
ℹ️ 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 optionId = params.options?.[0]?.optionId ?? "approve"; | ||
| return { | ||
| outcome: { | ||
| outcome: "selected", | ||
| optionId, |
There was a problem hiding this comment.
Prompt for tool permissions instead of auto-selecting first option
This callback always returns params.options?.[0], so any permission request is auto-approved without user input; in Goose ACP, the first option is currently allow_always (crates/goose/src/acp/server.rs lines 1400-1404), which can persistently whitelist a tool (crates/goose/src/agents/tool_execution.rs lines 138-141). In practice, when a tool call requires approval, the TUI will silently grant permanent permission rather than asking, which is a security regression. The same pattern is duplicated in runTextMode as well.
Useful? React with 👍 / 👎.
* main: (41 commits) removed the specific code owner for documentation change (#8749) fix(providers): handle missing delta field in streaming chunks (#8700) refactor(providers): extract http_status module and rename handle_status_openai_compat (#8620) fix(providers/openai): accept streaming chunks with both reasoning fields (#8715) feat: associate threads with projects (#8745) upgrade goose sdk and tui to be compatible with the latest agentclientprotocol/sdk package (#8667) feat: extend goose2 context window ux with auto-compaction (#8721) improve goose2 agent management flows (#8737) alexhancock/tui-improvements (#8736) fix: add strict:false to Responses API tools and gpt-5.4 to known models (#8636) persist and reliably apply chat model selection (#8734) merge goose-acp crate into goose (#8726) docs: AGENTS.md section on goose2 desktop backend architecture (#8732) feat: goose2 message bubble + action tray (#8720) consolidate provider ACP methods onto inventory (#8710) ci: declare and enforce MSRV of 1.91.1 (#8670) fix(ui): correct grammar in apps view description (#8668) (#8679) Stop load openai fast model for openapi compatible custom endpoint (#8644) feat(hooks): add Husky git hooks for ui/goose2 (#8577) fix: links in chat could not be opened (#8544) ...
* main: (34 commits) fix(goose-server): cache TLS cert to disk to avoid slow startup on first launch (#8174) feat: add Exa AI-powered search tool (#8487) fix: preprompt would show after loading session (#8744) commands to acp+ migration: extensions management (#8733) feat: desktop notification when goose finishes a task (#8647) harden code review skill for async state and default-resolution bugs (#8740) Feature/at agent mention (#8571) fix: removed hardcoded dependency of goose-acp-macro (#8753) perf: split agent setup into staged phases to reduce startup blocking (#8746) Add /skills command (#8600) Replace deprecated Claude ACP package links (#8625) removed the specific code owner for documentation change (#8749) fix(providers): handle missing delta field in streaming chunks (#8700) refactor(providers): extract http_status module and rename handle_status_openai_compat (#8620) fix(providers/openai): accept streaming chunks with both reasoning fields (#8715) feat: associate threads with projects (#8745) upgrade goose sdk and tui to be compatible with the latest agentclientprotocol/sdk package (#8667) feat: extend goose2 context window ux with auto-compaction (#8721) improve goose2 agent management flows (#8737) alexhancock/tui-improvements (#8736) ...
Summary
Question and Discussions
@alexhancock can you please help testing TUI? I started the Tui (even with the old version) locally, I cannot move the cursor or use the keys to show the existing provider and models on the Tui. Maybe I don't know how to use it properly
I noticed ui/sdk, ui/text and ui/goose-binary have the same version. I guess this is easier to manage the versions for these packages in the same repo. Shall i upgrade the goose-binary to have the same version as sdk and text
Shall we use
changesetto do the changelog and versioning, I guess changeset probably have better flow to add changelog, or we can manually do it. Either way works. WDYT?If we want to have same versions for these 3 packages, I can add another check in the publish npm workflow.
Testing
Have trouble testing at the moment