fix(agent-core-v2): gate plugin changes behind session baselines and reminders - #2702
Conversation
|
| Name | Type |
|---|---|
| @moonshot-ai/kimi-code | Patch |
Click here to learn what changesets are, and how to add one.
Click here if you're a maintainer who wants to add a changeset to this PR
…reminders
- capture a per-session MCP server baseline (ISessionMcpHandle.isBaselineServer)
so servers added mid-session (plugin install, mcp.json edit) never register
tools in live sessions; they take effect on /new, /reload, or resume, while
removed servers stay tombstoned and fail calls with a removal notice
- stop rebuilding the system prompt on plugin-source catalog changes: the
frozen skill listing and plugin sections cannot move anyway, and the rebuild
only churned the ${now} timestamp, invalidating the provider prompt cache
- freeze the Agent tool description's catalog profile list once the session
catalog has loaded, keeping the tools payload byte-stable across mutations
- append a plugin_change system reminder to live sessions on plugin mutations
(new IPluginService.onDidMutate; explicit reloadPlugins does not raise it)
- revert the TUI hint to "Run /new or /reload to apply plugin changes." and
update the plugin/MCP docs and changesets to the corrected contract
![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: a97244dcb4
ℹ️ 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".
| for (const entry of view.list()) { | ||
| baseline.add(entry.name); | ||
| } |
There was a problem hiding this comment.
When a session is created after a workspace server has already been disabled or removed, McpConnectionManager still exposes a disabled/removed entry, and this loop seeds that name into the new baseline. If the user re-enables or re-adds the same server while that session is open, isBaselineServer returns true and AgentMcpService registers its tools, even though servers added or enabled mid-session are supposed to wait for /new or /reload. Only seed entries that are active in the session's initial config, or otherwise keep tombstones baseline-only for sessions that existed before the removal.
Useful? React with 👍 / 👎.
| sessionManager, | ||
| new Set(Object.keys(servers)), | ||
| ); | ||
| const ready = Promise.all([this.ready, connect]).then(() => undefined); |
There was a problem hiding this comment.
For sessions created with ephemeral mcpServers, this combined ready does not freeze the baseline until the overlay manager finishes connecting. If the workspace manager is already ready and a plugin/config server is added while the ephemeral server is still connecting, sessionBaseline remains open, sees the new workspace entry through the merged view, and admits/registers it in the live session. Freeze the workspace portion on this.ready and treat only the explicit overlay names as open during overlay startup.
Useful? React with 👍 / 👎.
| this.plugins.onDidMutate(({ mutation }) => { | ||
| this.reminders.appendSystemReminder(renderPluginChangeReminder(mutation), { | ||
| kind: 'injection', | ||
| variant: PLUGIN_CHANGE_INJECTION_VARIANT, | ||
| }); |
There was a problem hiding this comment.
When a plugin install/enable/disable/remove happens, PluginService fires onDidReload, PluginSkillSource forwards that as a plugin skill-catalog change, and the existing listener above appends a fresh plugin_session_start reminder. This new mutation reminder therefore arrives alongside updated plugin instructions in the same open session, contradicting the /new or /reload guidance and letting newly installed session-start skills affect the current agent. Suppress the plugin-source refresh path for mutation-driven reloads, or replace it with only the change notice.
Useful? React with 👍 / 👎.
…utcomeMirror MoonshotAI#2666 imported LifecycleScope from #/_base/di/scope, which does not export it (it lives in #/app/scopes), breaking the package build and typecheck on main.
a97244d to
feaf619
Compare
commit: |
…nd overlay baseline leaks Codex review on the PR found two contract leaks: - a plugin mutation re-pulls the plugin skill source, and the existing catalog listener answered with a fresh plugin_session_start reminder — injecting the newly installed plugin's instructions into the live session alongside (and contradicting) the plugin_change notice. The session-start refresh now skips mutation-driven catalog changes (one per mutation, counted; explicit reloads keep the old refresh behavior). - a session created with ephemeral mcpServers kept its MCP baseline open until the overlay connect finished; a workspace server added in that window (plugin install, config edit) leaked into the live session through the merged view. The overlay handle's baseline now freezes on the workspace manager's initial load, with the ephemeral names baseline by construction.
Signed-off-by: Haozhe <yanghaozhe@moonshot.ai>
…reminders (MoonshotAI#2702) * fix(agent-core-v2): gate plugin changes behind session baselines and reminders - capture a per-session MCP server baseline (ISessionMcpHandle.isBaselineServer) so servers added mid-session (plugin install, mcp.json edit) never register tools in live sessions; they take effect on /new, /reload, or resume, while removed servers stay tombstoned and fail calls with a removal notice - stop rebuilding the system prompt on plugin-source catalog changes: the frozen skill listing and plugin sections cannot move anyway, and the rebuild only churned the ${now} timestamp, invalidating the provider prompt cache - freeze the Agent tool description's catalog profile list once the session catalog has loaded, keeping the tools payload byte-stable across mutations - append a plugin_change system reminder to live sessions on plugin mutations (new IPluginService.onDidMutate; explicit reloadPlugins does not raise it) - revert the TUI hint to "Run /new or /reload to apply plugin changes." and update the plugin/MCP docs and changesets to the corrected contract * fix(agent-core-v2): import LifecycleScope from app/scopes in sessionOutcomeMirror MoonshotAI#2666 imported LifecycleScope from #/_base/di/scope, which does not export it (it lives in #/app/scopes), breaking the package build and typecheck on main. * fix(agent-core-v2): close the mutation-driven session-start refresh and overlay baseline leaks Codex review on the PR found two contract leaks: - a plugin mutation re-pulls the plugin skill source, and the existing catalog listener answered with a fresh plugin_session_start reminder — injecting the newly installed plugin's instructions into the live session alongside (and contradicting) the plugin_change notice. The session-start refresh now skips mutation-driven catalog changes (one per mutation, counted; explicit reloads keep the old refresh behavior). - a session created with ephemeral mcpServers kept its MCP baseline open until the overlay connect finished; a workspace server added in that window (plugin install, config edit) leaked into the live session through the merged view. The overlay handle's baseline now freezes on the workspace manager's initial load, with the ephemeral names baseline by construction. * fix(agent-core-v2): drop duplicate LifecycleScope import in sessionOutcomeMirror test --------- Signed-off-by: Haozhe <yanghaozhe@moonshot.ai>
1 participant
Related Issue
No linked issue — this PR corrects the semantics introduced by #2694.
Problem
#2694 made plugin install/enable/disable/remove refresh workspace contributions immediately and showed an "apply immediately" hint on the v2 engine. That semantic is wrong for live sessions:
/newor/reload.${now}timestamp, rewriting the prompt and invalidating the provider's prompt cache on every mutation.What changed
Corrected contract: plugin changes never touch a live session's system prompt or tool set; the session is notified through a system reminder, and changes take effect on
/newor/reload.ISessionMcpHandle.isBaselineServercaptures the server names present when the session materializes (open to additions until the initial connect settles, then closed).AgentMcpServiceignores status changes from non-baseline servers — no tool registration, no status or discovery events — so servers added mid-session (plugin install,mcp.jsonedit) stay out of open sessions. Removed servers keep the feat(agent-core-v2): tombstone removed MCP servers and freeze plugin prompt inputs #2694 tombstone: registrations stay, calls fail with a removal notice. New sessions,/reload, and resume capture a fresh baseline.AgentProfileServiceno longer rebuilds the system prompt on plugin-source catalog changes (builtin-source changes still do), keeping the prompt byte-stable across plugin mutations.IPluginService.onDidMutate(raised by install/enable/disable/remove only — an explicitreloadPlugins()does not raise it, so a reloaded session does not inherit a stale notice) drives aplugin_changesystem reminder appended to every live session's main agent, naming the mutated plugin and pointing to/newor/reload.Checklist
gen-changesetsskill, or this PR needs no changeset.gen-docsskill, or this PR needs no doc update.