Dynamically refresh skill instructions each turn - #9217
Conversation
Skills listed in the system prompt were computed once at session start and cached. Adding or modifying SKILL.md files mid-session had no effect on the system prompt until restart. Add a get_instructions() method to McpClientTrait with a default that reads from get_info(). SkillsClient overrides it to re-discover skills from disk on every call. Extension::get_instructions() now delegates to the client instead of reading the cached server_info snapshot. Closes #6382 Signed-off-by: Douwe Osinga <douwe@squareup.com>
![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: 5b359c2797
ℹ️ 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".
| fn get_instructions(&self) -> Option<String> { | ||
| self.server_info | ||
| .as_ref() | ||
| .and_then(|info| info.instructions.clone()) | ||
| self.client.get_instructions() |
There was a problem hiding this comment.
Delegating Extension::get_instructions() to client.get_instructions() now makes the skills extension run discover_skills() on every get_extensions_info() call (which happens whenever the system prompt is rebuilt each turn). discover_skills() performs recursive std::fs walking, so this introduces blocking filesystem I/O in the hot path and can noticeably increase response latency when users have many skills/supporting files. Consider caching/invalidation or moving the scan off the async worker path.
Useful? React with 👍 / 👎.
| } | ||
|
|
||
| fn get_instructions(&self) -> Option<String> { | ||
| let instructions = build_skill_instructions(&self.working_dir); |
There was a problem hiding this comment.
nit: i would inline the impl of build_skill_instructions here as it's only called once
MockClient had todo!() in get_info(). Now that Extension::get_instructions() delegates to the client's get_instructions() default (which calls get_info()), this panic was hit during scenario tests. Return None instead. Signed-off-by: Douwe Osinga <douwe@squareup.com>
Signed-off-by: Douwe Osinga <douwe@squareup.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4341f0e6d4
ℹ️ 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".
| } | ||
|
|
||
| fn get_instructions(&self) -> Option<String> { | ||
| let sources = discover_skills(Some(&self.working_dir)); |
There was a problem hiding this comment.
get_instructions() now always scans self.working_dir, but SkillsClient::new() falls back to std::env::current_dir() when no session is attached. In production, extensions can be added with session_id: None (for example via the extension-manager tool path), so this change can inject skills from the process CWD instead of the active session/project directory into the system prompt. Before this commit, instructions were not emitted in the no-session case, so this is a behavioral regression that can surface incorrect or unrelated skills.
Useful? React with 👍 / 👎.
* main: (102 commits) Dynamically refresh skill instructions each turn (#9217) Build non-vulkan linux variants using ubuntu 22.04 (#9211) fix(ui): show tool name in approval prompt (#9216) feat: add Atomic Chat as declarative OpenAI-compatible provider (#9210) chore: bump package.json versions from 0.19.1 to 0.20.0 (#9218) feat: support GOOSE_OAUTH_CALLBACK_PORT for stable OAuth redirect_uri (#9209) [RFC] feat(oauth): proactive token refresh to avoid re-auth on every session (#8386) fix: resolve Azure CLI on Windows by using az.cmd (#9215) fix: handle non-interactive terminal in goose configure on Windows (#9214) Better parsing of pasted html as markdown so agents understand (#9190) fix: persist accumulated cost in session DB to survive reload (#9191) fix(publish-npm): build binary from current SHA + add compat check (#9212) feat(desktop): add goose://new-session deep link to open fresh chat (#9196) Add PR previews using cloudflare pages (#9208) fix: prevent tool-use marker leakage in toolshim output (#8310) Prompt injection mitigation: update pattern-based detection (#9198) remove goose2 related skills (#9189) Switch GH pages deploy to actions/artifact workflow (#9025) fix(summon): re-apply canonical limits when delegate overrides model (#9183) Split code signing from build (#8587) ...
Signed-off-by: Douwe Osinga <douwe@squareup.com> Co-authored-by: Douwe Osinga <douwe@squareup.com>
2 participants
Summary
Skills listed in the system prompt were computed once at session start and cached in
server_info. Adding or modifyingSKILL.mdfiles mid-session had no effect on the system prompt until restart.Changes
McpClientTrait: Addget_instructions()with a default implementation that reads fromget_info(). This is a non-breaking addition — all existing implementors get the default.SkillsClient: Overrideget_instructions()to re-discover skills from disk on every call. The constructor no longer bakes instructions intoInitializeResult.Extension::get_instructions(): Delegate toself.client.get_instructions()instead of reading from the cachedserver_infosnapshot.The system prompt is rebuilt each turn, and
get_extensions_info()callsExtension::get_instructions()each time — so newly added, modified, or removed skills are now reflected immediately without session restart.This is safe for non-skill platform extensions and external MCP servers: they use the default
get_instructions()which reads from their staticget_info(), preserving existing behavior.Closes #6382