diff --git a/packages/coding-agent/.changes/dead-kernel-memo.md b/packages/coding-agent/.changes/dead-kernel-memo.md new file mode 100644 index 0000000000..0f67b1dda7 --- /dev/null +++ b/packages/coding-agent/.changes/dead-kernel-memo.md @@ -0,0 +1 @@ +- A Python kernel that dies after a successful startup is restarted on the next use instead of every call being handed the dead kernel forever, and skill-MCP tools advertise their real input schemas again under mcp>=2 (the SDK renamed the field to input_schema). diff --git a/packages/coding-agent/src/core/kernel/repl-manager.ts b/packages/coding-agent/src/core/kernel/repl-manager.ts index a78ae3282a..cb77c472fd 100644 --- a/packages/coding-agent/src/core/kernel/repl-manager.ts +++ b/packages/coding-agent/src/core/kernel/repl-manager.ts @@ -1499,4 +1499,8 @@ export class ReplKernelManager { get isRunning(): boolean { return this.state === "running"; } + + get isDefunct(): boolean { + return this.state === "shutdown"; + } } diff --git a/packages/coding-agent/src/core/kernel/shared.ts b/packages/coding-agent/src/core/kernel/shared.ts index 25d43b9178..418e796b24 100644 --- a/packages/coding-agent/src/core/kernel/shared.ts +++ b/packages/coding-agent/src/core/kernel/shared.ts @@ -278,6 +278,8 @@ export interface KernelShutdownOptions { export interface KernelClient { readonly ownerSessionId: string | undefined; readonly isRunning: boolean; + /** Terminal: the kernel died or was torn down; only a fresh manager can serve again. */ + readonly isDefunct: boolean; start(options?: KernelStartOptions): Promise; execute(code: string, opts?: ExecuteOptions): Promise; shutdown(opts?: KernelShutdownOptions): Promise; diff --git a/packages/coding-agent/src/core/tools/ipython.ts b/packages/coding-agent/src/core/tools/ipython.ts index 6c8501b928..a03c609443 100644 --- a/packages/coding-agent/src/core/tools/ipython.ts +++ b/packages/coding-agent/src/core/tools/ipython.ts @@ -388,6 +388,12 @@ export class IpythonKernelProvisioner { if (signal?.aborted) { return Promise.reject(createAbortError()); } + // A kernel that died for good must not be handed out again; a manager + // mid-protocol-repair (idle/starting) recovers itself and keeps the memo. + if (this.startedManager?.isDefunct) { + this.managerPromise = undefined; + this.startedManager = undefined; + } let cleanupProgressListener: (() => void) | undefined; if (onProgress && !this.startedManager) { this.startupListeners.add(onProgress); diff --git a/packages/coding-agent/test/ipython-provisioner.test.ts b/packages/coding-agent/test/ipython-provisioner.test.ts index 02bb0ef4a8..d5c317800c 100644 --- a/packages/coding-agent/test/ipython-provisioner.test.ts +++ b/packages/coding-agent/test/ipython-provisioner.test.ts @@ -306,6 +306,46 @@ describe("IpythonKernelProvisioner", () => { expect(provisioner.manager).toBe(manager); }); + it("drops a dead kernel memo so ensure() restarts instead of reusing it", async () => { + const { python, countRuns } = writeFakePython(); + const provisioner = new IpythonKernelProvisioner(tempDir, { python }); + const dead = { isRunning: false, isDefunct: true } as unknown as KernelClient; + Object.assign( + provisioner as unknown as { + managerPromise: Promise; + startedManager: KernelClient; + }, + { + managerPromise: Promise.resolve(dead), + startedManager: dead, + }, + ); + + await expect(provisioner.ensure()).rejects.toThrow(/Kernel exited before ready/); + expect(countRuns()).toBe(1); + }); + + it("keeps the memo for a kernel that is repairing itself, not defunct", async () => { + const { countRuns } = writeFakePython(); + const provisioner = new IpythonKernelProvisioner(tempDir, {}); + const repairing = { isRunning: false, isDefunct: false } as unknown as KernelClient; + Object.assign( + provisioner as unknown as { + managerPromise: Promise; + startedManager: KernelClient; + }, + { + managerPromise: Promise.resolve(repairing), + startedManager: repairing, + }, + ); + + // A protocol repair parks the SAME manager in idle/starting while it + // respawns; a second provisioner kernel would split the snapshot dir. + await expect(provisioner.ensure()).resolves.toBe(repairing); + expect(countRuns()).toBe(0); + }); + it("removes startup progress listeners when an ensure caller is aborted", async () => { const provisioner = new IpythonKernelProvisioner(tempDir, {}); Object.assign( diff --git a/prime-agent-runtime/src/rlm/mcp_base.py b/prime-agent-runtime/src/rlm/mcp_base.py index 1f3d142795..2969f603bb 100644 --- a/prime-agent-runtime/src/rlm/mcp_base.py +++ b/prime-agent-runtime/src/rlm/mcp_base.py @@ -260,14 +260,18 @@ async def _ensure_tools(self) -> None: async with AsyncExitStack() as stack: session = await self._open_session(stack) resp = await session.list_tools() - self._tools = { - t.name: { + tools: dict[str, Any] = {} + for t in resp.tools: + # mcp>=2 exposes the pydantic field input_schema; inputSchema is the wire alias. + schema = getattr(t, "input_schema", None) + if schema is None: + schema = getattr(t, "inputSchema", None) + tools[t.name] = { "name": t.name, "description": getattr(t, "description", "") or "", - "inputSchema": getattr(t, "inputSchema", None) or {}, + "inputSchema": schema if isinstance(schema, dict) else {}, } - for t in resp.tools - } + self._tools = tools async def call_tool(self, tool: str, arguments: dict[str, Any] | None = None) -> Any: """Call ``tool`` on the server and return its parsed result. diff --git a/prime-agent-runtime/test/test_mcp_base.py b/prime-agent-runtime/test/test_mcp_base.py index 98acfaa1b9..2504f1780d 100644 --- a/prime-agent-runtime/test/test_mcp_base.py +++ b/prime-agent-runtime/test/test_mcp_base.py @@ -169,6 +169,29 @@ def test_auto_bound_tool_calls_session(self): self.assertEqual(out, {"issues": [1, 2]}) self.assertEqual(session.calls, [("list_issues", {"team": "Eng"})]) + def test_snake_case_input_schema_surfaces(self): + # mcp>=2 Tool objects expose input_schema (pydantic field name), not inputSchema. + Tool = type("Tool", (), {}) + tool = Tool() + tool.name = "list_issues" + tool.description = "List issues" + tool.input_schema = {"type": "object", "properties": {"team": {"type": "string"}}} + session = _FakeSession(tools=[], result=None) + + async def list_tools(): + resp = type("Resp", (), {})() + resp.tools = [tool] + return resp + + session.list_tools = list_tools + self._write_auth( + {"type": "oauth", "access": "t", "refresh": "r", "expires": (time.time() + 3600) * 1000} + ) + with self._patch_session(session): + integration = _Integration() + tools = _run(integration.list_tools()) + self.assertEqual(tools[0]["inputSchema"], tool.input_schema) + def test_unknown_tool_raises_with_available_list(self): session = _FakeSession(tools=[("list_issues", "", {})], result=None) self._write_auth(