refactor: skills as its own platform ext#8244
Conversation
|
does this add a bit more tool use overhead vs one summon (?) tool? is summon on by default? (should it be? I think it is, which is fine for now) - but assume that skills can be even lower level? |
alexhancock
force-pushed
the
alexhancock/skills-to-pay-the-bills
branch
from
615bf9f to
fb9f08a
Compare
![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: fb9f08aae1
ℹ️ 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".
| Ok(ListToolsResult { | ||
| tools: vec![], | ||
| next_cursor: None, |
There was a problem hiding this comment.
This refactor moves skills out of summon, but the new skills extension exposes no tools, so the agent can only see skill names/descriptions and cannot fetch SKILL.md bodies or supporting files at runtime. In practice, workflows that rely on detailed skill instructions (especially builtin skills with substantial body content) now degrade to metadata-only hints because there is no replacement for the previous load(source: "<skill>") path.
Useful? React with 👍 / 👎.
| let info = InitializeResult::new(ServerCapabilities::builder().enable_tools().build()) | ||
| .with_server_info(Implementation::new(EXTENSION_NAME, "1.0.0").with_title("Summon")) | ||
| .with_instructions(instructions.unwrap_or_default()); | ||
| .with_server_info(Implementation::new(EXTENSION_NAME, "1.0.0").with_title("Summon")); |
There was a problem hiding this comment.
Removing summon-provided skill instructions here creates an upgrade regression for sessions created before the new hidden skills extension existed: those sessions load extensions from their persisted enabled_extensions state, so they may not automatically include skills, and now summon no longer provides any skill context. The result is that skill support can silently disappear in ongoing sessions after upgrade until users recreate/toggle extensions.
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) ...
This reverts commit fb04c89.
2 participants
Move skill support to its own hidden extension which will be default on, so that skill support is not dependent on the user choosing to enable
summon(Draft for now - need to verify and test this)