rust acp client for extension methods#8227
Conversation
![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: 6151b81d7c
ℹ️ 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(rename_all = "camelCase")] | ||
| pub struct AddExtensionRequest { | ||
| pub session_id: String, | ||
| /// Extension configuration (see ExtensionConfig variants: Stdio, StreamableHttp, Builtin, Platform). | ||
| #[serde(default)] |
There was a problem hiding this comment.
Adding #[serde(default)] to AddExtensionRequest.config makes config optional in generated schema/SDK types, so callers can now send _goose/extensions/add without configuration and still pass client-side validation. The server path still deserializes req.config into ExtensionConfig in on_add_extension, which rejects null with invalid_params, so this regression shifts a previously enforced contract into a runtime request failure for any client that omits config.
Useful? React with 👍 / 👎.
alexhancock
left a comment
alexhancock
left a comment
There was a problem hiding this comment.
Nice!
My only feedback would be on API design. Would there be a way to make it so the client can call something like connect_with without a callback to contain the rest of the interactions?
Something like
client = Client.
...
.connect_with(transport).await
client.send_request(InitializeRequest::new(ProtocolVersion::LATEST))
etc
|
@alexhancock that part is all sacp's api. I think what you'd use if you want something like that is spawn_connection |
DOsinga
added
the
needs_human
Move custom_requests.rs and the acp_client example out of goose-acp into a new lightweight goose-sdk crate so other Rust binaries can link it to talk to Goose over ACP without pulling in heavy server deps. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0f7bcfede4
ℹ️ 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(default)] | ||
| pub session: serde_json::Value, |
There was a problem hiding this comment.
session field in session/get response
Adding #[serde(default)] to GetSessionResponse.session weakens the wire contract by turning a required response payload into an optional/null-defaulted field in generated schema/SDK types. With this change, a malformed session/get response that omits session will deserialize as null instead of failing fast, which hides protocol regressions and pushes bad data downstream.
Useful? React with 👍 / 👎.
tokio and tokio-util are only needed by the example, not the library. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b1e0dfc355
ℹ️ 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".
| #[derive(Debug, Default, Clone, Serialize, Deserialize, JsonSchema, JsonRpcResponse)] | ||
| pub struct ReadResourceResponse { | ||
| /// The resource result from the extension (MCP ReadResourceResult). | ||
| #[serde(default)] |
There was a problem hiding this comment.
result in resource/read responses
Removing strict deserialization here by adding #[serde(default)] weakens the wire contract for _goose/resource/read: generated schema/SDK types now treat result as optional, so a malformed response that omits result is accepted as null instead of failing fast. That hides protocol regressions and can propagate invalid resource payloads to downstream callers that assume a real MCP result object is present.
Useful? React with 👍 / 👎.
| #[derive(Debug, Default, Clone, Serialize, Deserialize, JsonSchema, JsonRpcResponse)] | ||
| pub struct ImportSessionResponse { | ||
| /// The imported session object. | ||
| #[serde(default)] |
There was a problem hiding this comment.
session mandatory in import session response
Adding #[serde(default)] on ImportSessionResponse.session turns a previously required response field into an optional/null-defaulted one in generated schema and client types. If _goose/session/import ever returns an incomplete payload, clients will now deserialize it as session = null instead of surfacing a protocol error, which makes bad server responses harder to detect and debug.
Useful? React with 👍 / 👎.
* origin/main: (32 commits) docs: rework homepage and add aaif migration blog post (#8356) chore(aaif): rename a bunch of repository references (#8152) fix: use OPENAI_API_KEY secret for recipe security scanner (#8358) feat: configurable extension timeouts via ACP _meta and global default (#8295) fix: hide hidden extensions in UI (#8346) refactor: skills as its own platform ext (#8244) fix baseUrl (#8347) Fix desktop slash commands (#8341) fix(cli): display platform-correct secrets path in keyring config dialog (#8328) feat(acp): add reusable ACP provider controls (#8314) fix: resolve MDX compilation error in using-goosehints.md (#8332) fix: use v1beta1 API version for Google/MaaS models on GCP Vertex AI (#8278) docs: add MCP Roots guide (#8252) rust acp client for extension methods (#8227) fix: reconsolidate split tool-call messages to follow OpenAI format (#7921) fix: clean up MCP subprocesses after abrupt parent exit (#8242) build: raise default stack reserve to 8 MB (#8234) fix(config): honour GOOSE_DISABLE_KEYRING from config.yaml at startup (#8219) feat: add configurable fast_model for declarative providers (#8194) fix(authentication): Allow connecting to Oauth servers that use protected-resource fallback instead of the WWW-authenticate header (#8148) ...
3 participants
Make the extension methods usable directly with sacp.
The right way to do this would be to have a separate crate with the custom methods that doesn't depend on the rest of goose