fix: removed hardcoded dependency of goose-acp-macro#8753
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: ce2cd9b8a9
ℹ️ 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".
| pkcs8 = { version = "0.10", default-features = false, features = ["alloc"], optional = true } | ||
| sec1 = { version = "0.7", default-features = false, features = ["der", "pkcs8"], optional = true } | ||
| goose-acp-macros = { version = "1.31.0", path = "../goose-acp-macros" } | ||
| goose-acp-macros = { path = "../goose-acp-macros" } |
There was a problem hiding this comment.
Removing the version from this path dependency makes crates/goose fail Cargo packaging checks, so release/publish workflows that package this crate will now break. I verified this in this repo with cargo package -p goose --allow-dirty --no-verify, which errors that all dependencies must have a version requirement and specifically flags goose-acp-macros; keeping a semver requirement (or workspace-managed equivalent) avoids that regression while still allowing local path resolution.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
We don’t currently use cargo package/cargo publish in this repo, so that packaging concern isn’t actionable for our workflow.
The issue we were actually hitting was canary/release version rewriting, where the hardcoded goose-acp-macros = "1.31.0" could drift from the workspace version. This also matches existing internal dependency usage here: other crates already use path-only internal deps like goose-sdk, goose-mcp, and goose in goose-cli/goose-server.
There was a problem hiding this comment.
yeah, if you revert it and try that cargo package command, it just fails on the next workspace dependency. This objection makes no sense
jamadeo
left a comment
jamadeo
left a comment
There was a problem hiding this comment.
I'm confused by Codex's objection here -- every other interdependency in the workspace omits the version
* main: 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)
* 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) ...
2 participants
Summary
Removed hardcoded dependency of
goose-acp-macroWhy: https://github.com/aaif-goose/goose/actions/runs/24745252740/job/72395273643 Canary build failed
Testing
Canary build