Skip to content

Commit 8fd7a02

Browse files
authored
Merge pull request #377 from code-yeongyu/perf/mcp-reload-measure
feat(mcp): keep unchanged MCP servers alive across a classic /reload
2 parents 9196919 + 9962222 commit 8fd7a02

11 files changed

Lines changed: 519 additions & 60 deletions

File tree

packages/coding-agent/src/changes.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,13 @@
2121
- Root cause of the omo `team_wait` starvation forensics: member self-poller injections via `pi.sendUserMessage(..., { deliverAs: "followUp" })` vanished without a trace when the fresh-prompt path threw, leaving no record in the session JSONL while RPC-path `steer`/`follow_up` commands (which bypass `prompt()`) landed normally.
2222
- Interactive `prompt()` behavior is unchanged: a rejected interactive prompt still drops the input and surfaces the error to the user (pinned by `test/suite/regressions/pre-prompt-compaction-no-continue.test.ts`).
2323
- Coverage: `test/suite/agent-session-extension-injection.test.ts` pins retention for followUp and steer injections, exact-once delivery after recovery through the post-run drain, and no double-queueing on the streaming accept path.
24+
25+
## Reload-safe MCP preservation and extension-removal lifecycle event (2026-07-26)
26+
27+
- `session_extensions_removed` is emitted on the old extension runner when a `/reload` or a session replacement (`/new`, `/resume`, `/fork`, import) rebuilds the extension set. Its payload is `{ type: "session_extensions_removed", reason: SessionShutdownEvent["reason"], removed: Array<{ path, resolvedPath }> }`, allowing an extension that did not survive the rebuild to release resources after the new settings and active builtin set are known.
28+
- Unchanged MCP servers now survive a classic `/reload`: the shared service reattaches and reconciles by config hash, preserving live connections while replacing changed servers and disposing removed ones. Provider-scoped MCP services still dispose on reload because their factory creates a replacement instance.
29+
- If the MCP builtin itself is disabled during a reload or replacement, its removal event disposes the preserved classic service so stdio children cannot leak. For an otherwise wedged server, use `/mcp reconnect <name>` to force a fresh connection.
30+
2431
## Same-model-first transient retries and capped server waits (2026-07-26)
2532

2633
### What changed

packages/coding-agent/src/core/agent-session-runtime.ts

Lines changed: 39 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import type {
99
SessionShutdownEvent,
1010
SessionStartEvent,
1111
} from "./extensions/index.ts";
12-
import { emitSessionShutdownEvent } from "./extensions/runner.ts";
12+
import { type ExtensionRunner, emitSessionShutdownEvent } from "./extensions/runner.ts";
1313
import type { CreateAgentSessionResult } from "./sdk.ts";
1414
import { assertSessionCwdExists } from "./session-cwd.ts";
1515
import { SessionManager } from "./session-manager.ts";
@@ -89,6 +89,11 @@ export class AgentSessionRuntime {
8989
private _diagnostics: AgentSessionRuntimeDiagnostic[];
9090
private _modelFallbackMessage?: string;
9191
private readonly _launchProfile?: Readonly<AgentSessionLaunchProfile>;
92+
private _removedOnReplacement?: {
93+
oldRunner: ExtensionRunner;
94+
oldIdentities: Array<{ path: string; resolvedPath: string }>;
95+
reason: SessionShutdownEvent["reason"];
96+
};
9297

9398
constructor(
9499
_session: AgentSession,
@@ -181,7 +186,17 @@ export class AgentSessionRuntime {
181186
}
182187

183188
private async teardownCurrent(reason: SessionShutdownEvent["reason"], targetSessionFile?: string): Promise<void> {
184-
await emitSessionShutdownEvent(this.session.extensionRunner, {
189+
const oldRunner = this.session.extensionRunner;
190+
// Test hosts and partial runner implementations may lack identity introspection;
191+
// skip removal reporting there rather than break the replacement itself.
192+
if (typeof oldRunner.getExtensionIdentities === "function") {
193+
this._removedOnReplacement = {
194+
oldRunner,
195+
oldIdentities: oldRunner.getExtensionIdentities(),
196+
reason,
197+
};
198+
}
199+
await emitSessionShutdownEvent(oldRunner, {
185200
type: "session_shutdown",
186201
reason,
187202
targetSessionFile,
@@ -190,11 +205,26 @@ export class AgentSessionRuntime {
190205
this.session.dispose();
191206
}
192207

193-
private apply(result: CreateAgentSessionRuntimeResult): void {
208+
private async reportRemovedExtensions(): Promise<void> {
209+
const pending = this._removedOnReplacement;
210+
this._removedOnReplacement = undefined;
211+
if (!pending) return;
212+
const newRunner = this.session.extensionRunner;
213+
if (typeof newRunner.getExtensionIdentities !== "function") return;
214+
const newResolvedPaths = new Set(
215+
newRunner.getExtensionIdentities().map((extension) => extension.resolvedPath),
216+
);
217+
const removed = pending.oldIdentities.filter((extension) => !newResolvedPaths.has(extension.resolvedPath));
218+
if (removed.length === 0) return;
219+
await pending.oldRunner.emit({ type: "session_extensions_removed", reason: pending.reason, removed });
220+
}
221+
222+
private async apply(result: CreateAgentSessionRuntimeResult): Promise<void> {
194223
this._session = result.session;
195224
this._services = result.services;
196225
this._diagnostics = result.diagnostics;
197226
this._modelFallbackMessage = result.modelFallbackMessage;
227+
await this.reportRemovedExtensions();
198228
}
199229

200230
private async finishSessionReplacement(withSession?: (ctx: ReplacedSessionContext) => Promise<void>): Promise<void> {
@@ -223,7 +253,7 @@ export class AgentSessionRuntime {
223253
const sessionManager = SessionManager.open(sessionPath, undefined, options?.cwdOverride);
224254
assertSessionCwdExists(sessionManager, this.cwd);
225255
await this.teardownCurrent("resume", sessionManager.getSessionFile());
226-
this.apply(
256+
await this.apply(
227257
await this.createRuntime({
228258
cwd: sessionManager.getCwd(),
229259
agentDir: this.services.agentDir,
@@ -257,7 +287,7 @@ export class AgentSessionRuntime {
257287
}
258288

259289
await this.teardownCurrent("new", sessionManager.getSessionFile());
260-
this.apply(
290+
await this.apply(
261291
await this.createRuntime({
262292
cwd: this.cwd,
263293
agentDir: this.services.agentDir,
@@ -312,7 +342,7 @@ export class AgentSessionRuntime {
312342
const sessionManager = SessionManager.create(this.cwd, sessionDir);
313343
sessionManager.newSession({ parentSession: currentSessionFile });
314344
await this.teardownCurrent("fork", sessionManager.getSessionFile());
315-
this.apply(
345+
await this.apply(
316346
await this.createRuntime({
317347
cwd: this.cwd,
318348
agentDir: this.services.agentDir,
@@ -336,7 +366,7 @@ export class AgentSessionRuntime {
336366
throw new Error("Failed to create forked session");
337367
}
338368
await this.teardownCurrent("fork", sessionManager.getSessionFile());
339-
this.apply(
369+
await this.apply(
340370
await this.createRuntime({
341371
cwd: sessionManager.getCwd(),
342372
agentDir: this.services.agentDir,
@@ -356,7 +386,7 @@ export class AgentSessionRuntime {
356386
sessionManager.createBranchedSession(targetLeafId);
357387
}
358388
await this.teardownCurrent("fork", sessionManager.getSessionFile());
359-
this.apply(
389+
await this.apply(
360390
await this.createRuntime({
361391
cwd: this.cwd,
362392
agentDir: this.services.agentDir,
@@ -401,7 +431,7 @@ export class AgentSessionRuntime {
401431
const sessionManager = SessionManager.open(destinationPath, sessionDir, cwdOverride);
402432
assertSessionCwdExists(sessionManager, this.cwd);
403433
await this.teardownCurrent("resume", sessionManager.getSessionFile());
404-
this.apply(
434+
await this.apply(
405435
await this.createRuntime({
406436
cwd: sessionManager.getCwd(),
407437
agentDir: this.services.agentDir,

packages/coding-agent/src/core/agent-session.ts

Lines changed: 25 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4976,8 +4976,10 @@ export class AgentSession {
49764976

49774977
async reload(options?: { beforeSessionStart?: () => void | Promise<void> }): Promise<void> {
49784978
resetTimings("reload");
4979-
const previousFlagValues = this._extensionRunner.getFlagValues();
4980-
await emitSessionShutdownEvent(this._extensionRunner, { type: "session_shutdown", reason: "reload" });
4979+
const oldExtensionRunner = this._extensionRunner;
4980+
const oldExtensionIdentities = oldExtensionRunner.getExtensionIdentities();
4981+
const previousFlagValues = oldExtensionRunner.getFlagValues();
4982+
await emitSessionShutdownEvent(oldExtensionRunner, { type: "session_shutdown", reason: "reload" });
49814983
time("shutdown", "reload");
49824984
await this.settingsManager.reload();
49834985
this.syncQueueModesFromSettings();
@@ -5003,12 +5005,27 @@ export class AgentSession {
50035005
time("models", "reload");
50045006
await this._resourceLoader.reload({ settingsAlreadyReloadedFor: this.settingsManager });
50055007
time("resources", "reload");
5006-
this._buildRuntime({
5007-
activeToolNames: this.getActiveToolNames(),
5008-
flagValues: previousFlagValues,
5009-
includeAllExtensionTools: true,
5010-
});
5011-
time("runtime", "reload");
5008+
try {
5009+
this._buildRuntime({
5010+
activeToolNames: this.getActiveToolNames(),
5011+
flagValues: previousFlagValues,
5012+
includeAllExtensionTools: true,
5013+
});
5014+
} finally {
5015+
// An extension removed by this reload must be told even if the rebuild throws
5016+
// (e.g. _refreshToolRegistry rejecting an extension's tool metadata): the new
5017+
// runner is already installed without it, so nothing else would dispose it.
5018+
const newExtensionResolvedPaths = new Set(
5019+
this._extensionRunner.getExtensionIdentities().map((extension) => extension.resolvedPath),
5020+
);
5021+
const removed = oldExtensionIdentities.filter(
5022+
(extension) => !newExtensionResolvedPaths.has(extension.resolvedPath),
5023+
);
5024+
if (removed.length > 0) {
5025+
await oldExtensionRunner.emit({ type: "session_extensions_removed", reason: "reload", removed });
5026+
}
5027+
time("runtime", "reload");
5028+
}
50125029

50135030
const hasBindings =
50145031
this._extensionUIContext ||

packages/coding-agent/src/core/extensions/builtin/mcp/changes.md

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,22 @@
11
# mcp Extension Changes
22

3+
## Classic reload preserves unchanged MCP servers (2026-07-26)
4+
5+
### What changed
6+
- Classic (non-provider-scoped) MCP reloads keep the shared `McpService` alive. The reload-time `session_start` reattaches it and its existing config-hash reconciliation preserves unchanged servers while replacing changed definitions and disposing removed definitions.
7+
- Provider-scoped MCP services still dispose on `reload`, because rebuilding an extension factory creates a new scoped instance and preserving the old one would orphan its child processes.
8+
- Core now emits `{ type: "session_extensions_removed", reason: "reload", removed: Array<{ path, resolvedPath }> }` on the old runner after it knows the rebuilt extension set. MCP matches its builtin identity (`<builtin:mcp>`) in that event and disposes the preserved classic service when MCP is disabled during a reload.
9+
- `/mcp reconnect <name>` remains the explicit escape hatch for a server that is connected but wedged: it renews that server without requiring a full reload.
10+
11+
### Why
12+
- Spawning every MCP server again on every classic reload adds a fixed process startup cost even when config is unchanged. Preserving and reconciling retains healthy children, while the removal event closes the only gap where the preserved singleton otherwise loses its owning extension.
13+
14+
### Why extension system couldn't handle this alone
15+
- The core alone can identify removed extension entries but must remain resource-agnostic; MCP alone cannot know the post-reload builtin set at `session_shutdown`. The core event provides the lifecycle boundary and MCP owns the service-specific disposal.
16+
17+
### Expected merge conflict zones
18+
- LOW: `index.ts` lifecycle handlers; `service.ts` remains the config-hash reconciliation owner.
19+
320
## Raced background registration replays session state (2026-07-21)
421

522
### What changed

packages/coding-agent/src/core/extensions/builtin/mcp/index.ts

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@ import {
1616
} from "./skills.ts";
1717
import { reportMcpAsyncError, wrapAsync } from "./wrap.ts";
1818

19+
const MCP_BUILTIN_EXTENSION_PATH = "<builtin:mcp>";
20+
1921
export function createMcpExtension(service: McpService, sessionOwned = true): ExtensionFactory {
2022
return (pi: ExtensionAPI): void => {
2123
let attachPromise: Promise<void> | undefined;
@@ -149,11 +151,20 @@ export function createMcpExtension(service: McpService, sessionOwned = true): Ex
149151
wrapAsync(
150152
"mcp.session_shutdown",
151153
async (event) => {
154+
if (event.reason === "reload" && !sessionOwned) return;
152155
await service.handleSessionShutdown(event);
153-
// A classic extension instance can be reused by the test/legacy host
154-
// after reload. Its singleton is intentionally refreshed for that next
155-
// session; scoped factories keep their closed instance.
156-
if (!sessionOwned && event.reason === "reload" && service.isDisposed()) service = getMcpService();
156+
},
157+
sink,
158+
),
159+
);
160+
pi.on(
161+
"session_extensions_removed",
162+
wrapAsync(
163+
"mcp.session_extensions_removed",
164+
async (event) => {
165+
if (event.removed.some((extension) => extension.path === MCP_BUILTIN_EXTENSION_PATH)) {
166+
await service.dispose("reload");
167+
}
157168
},
158169
sink,
159170
),

packages/coding-agent/src/core/extensions/runner.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -591,6 +591,10 @@ export class ExtensionRunner {
591591
return this.extensions.map((e) => e.path);
592592
}
593593

594+
getExtensionIdentities(): Array<{ path: string; resolvedPath: string }> {
595+
return this.extensions.map(({ path, resolvedPath }) => ({ path, resolvedPath }));
596+
}
597+
594598
/**
595599
* Get all registered tools from all extensions. The first registration within a source tier
596600
* wins, while a non-builtin extension may override a builtin extension tool.

packages/coding-agent/src/core/extensions/types.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -773,6 +773,13 @@ export interface SessionShutdownEvent {
773773
targetSessionFile?: string;
774774
}
775775

776+
/** Fired on the old extension runner when a reload or session replacement rebuilds the runner and one or more extensions are absent from it. */
777+
export interface SessionExtensionsRemovedEvent {
778+
type: "session_extensions_removed";
779+
reason: SessionShutdownEvent["reason"];
780+
removed: Array<{ path: string; resolvedPath: string }>;
781+
}
782+
776783
/** Preparation data for tree navigation */
777784
export interface TreePreparation {
778785
targetId: string;
@@ -812,6 +819,7 @@ export type SessionEvent =
812819
| SessionBeforeCompactEvent
813820
| SessionCompactEvent
814821
| SessionShutdownEvent
822+
| SessionExtensionsRemovedEvent
815823
| SessionBeforeTreeEvent
816824
| SessionTreeEvent;
817825

@@ -1401,6 +1409,7 @@ export interface ExtensionAPI {
14011409
): void;
14021410
on(event: "session_compact", handler: ExtensionHandler<SessionCompactEvent>): void;
14031411
on(event: "session_shutdown", handler: ExtensionHandler<SessionShutdownEvent>): void;
1412+
on(event: "session_extensions_removed", handler: ExtensionHandler<SessionExtensionsRemovedEvent>): void;
14041413
on(event: "session_before_tree", handler: ExtensionHandler<SessionBeforeTreeEvent, SessionBeforeTreeResult>): void;
14051414
on(event: "session_tree", handler: ExtensionHandler<SessionTreeEvent>): void;
14061415
on(event: "context", handler: ExtensionHandler<ContextEvent, ContextEventResult>): void;

0 commit comments

Comments
 (0)