-
Notifications
You must be signed in to change notification settings - Fork 4
feat: cut MCP surface to ratified tools #409
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6057,6 +6057,124 @@ export class AgentEngine { | |
| }; | ||
| } | ||
|
|
||
| /** Resume a captured CLI session on a fresh surface while preserving its | ||
| * stable public agent ID. This is the explicit counterpart to crash recovery. */ | ||
| async resumeAgent( | ||
| agentId: string, | ||
| opts?: { workspace?: string }, | ||
| ): Promise<SpawnAgentResult> { | ||
| const agent = | ||
| this.registry.get(agentId) ?? this.stateMgr.readState(agentId); | ||
| if (!agent) { | ||
| throw new Error(`Agent not found: ${agentId}`); | ||
| } | ||
| if (!TERMINAL_STATES.has(agent.state)) { | ||
| throw new Error( | ||
| `Agent "${agent.agent_id}" is ${agent.state}; explicit resume requires a terminal agent`, | ||
| ); | ||
| } | ||
| if (!agent.cli_session_id) { | ||
| throw new Error( | ||
| `Agent "${agent.agent_id}" has no captured CLI session to resume`, | ||
| ); | ||
| } | ||
| const resumeCommand = buildResumeCommand( | ||
| agent.cli, | ||
| agent.repo, | ||
| agent.cli_session_id, | ||
| agent.launcher_name, | ||
| ); | ||
| const requestedWorkspace = opts?.workspace ?? agent.workspace_id ?? undefined; | ||
| this.spawnGuard.check(requestedWorkspace); | ||
|
|
||
| let surface: CreatedAgentSurface | null = null; | ||
| let surfaceBound = false; | ||
| let recordReopened = false; | ||
| try { | ||
| surface = await this.createAgentSurface(requestedWorkspace, { | ||
| role: inferRecordRole(agent), | ||
| parentAgent: agent.parent_agent_id | ||
| ? this.registry.get(agent.parent_agent_id) | ||
| : null, | ||
| repo: agent.repo, | ||
| worktree: Boolean(agent.worktree_path), | ||
| }); | ||
| this.assertSurfaceObserverEpochCurrent( | ||
| surface.observerEpoch, | ||
| "explicit agent resume", | ||
| ); | ||
| const workspace = surface.actual_workspace ?? surface.workspace; | ||
| await this.client.focusSurface(surface.surface, { | ||
| workspace, | ||
| beforeMutation: async () => { | ||
| this.assertSurfaceObserverEpochCurrent( | ||
| surface!.observerEpoch, | ||
| "explicit agent resume focus", | ||
| ); | ||
| }, | ||
| }); | ||
|
|
||
| const creating = this.stateMgr.reopenForResume(agent.agent_id); | ||
| recordReopened = true; | ||
| this.registry.set(agent.agent_id, creating); | ||
| const rebound = this.stateMgr.updateRecord(agent.agent_id, { | ||
| surface_id: surface.surface, | ||
| surface_uuid: surface.surface_id ?? null, | ||
| surface_observer_id: surface.observerId, | ||
| surface_provenance: "cmuxlayer_spawn", | ||
| workspace_id: workspace, | ||
| user_killed: false, | ||
| deletion_intent: false, | ||
| error: null, | ||
| pid: null, | ||
| }); | ||
| this.registry.set(agent.agent_id, rebound); | ||
| surfaceBound = true; | ||
| const booting = this.stateMgr.transition(agent.agent_id, "booting", { | ||
| error: null, | ||
| pid: null, | ||
| cli_session_id: agent.cli_session_id, | ||
| }); | ||
| this.registry.set(agent.agent_id, booting); | ||
| await this.sendLaunchCommand( | ||
| surface.surface, | ||
| workspace, | ||
| resumeCommand, | ||
| agent.agent_id, | ||
| surface.observerEpoch, | ||
| ); | ||
|
Comment on lines
+6117
to
+6145
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift Require fresh completion evidence after resumption.
Add a resume epoch or equivalent gate. Ignore transcript completion evidence that predates the resume until the resumed CLI produces fresh readiness or activity evidence. Clear stale completion markers during the reopen flow. Add a regression test that runs a lifecycle sweep after 🤖 Prompt for AI Agents |
||
| await this.reconcileRolePlacements("spawn", { | ||
| agentIds: new Set([agent.agent_id]), | ||
| }); | ||
| return { | ||
| agent_id: agent.agent_id, | ||
| parent_agent_id: agent.parent_agent_id, | ||
| surface_id: surface.surface, | ||
| workspace_id: workspace, | ||
| state: "booting", | ||
| model: agent.model, | ||
| cwd: agent.launch_cwd ?? undefined, | ||
| }; | ||
| } catch (error) { | ||
| if (surface && !surfaceBound) { | ||
| await this.cleanupUnboundCreatedSurface(surface, "agent-placement"); | ||
| } | ||
| if (recordReopened) { | ||
| try { | ||
| const failed = this.stateMgr.transition(agent.agent_id, "error", { | ||
| error: `Explicit resume failed: ${ | ||
| error instanceof Error ? error.message : String(error) | ||
| }`, | ||
| }); | ||
| this.registry.set(agent.agent_id, failed); | ||
| } catch { | ||
| // Preserve the original failure. | ||
| } | ||
| } | ||
| throw error; | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Cascade-kill all agents in the subtree rooted at rootId. | ||
| * Uses DFS post-order (children before root). Continues on failures (best-effort). | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,15 +3,8 @@ | |
| /** | ||
| * cmuxlayer — Terminal multiplexer MCP server for AI agent workspace orchestration. | ||
| * | ||
| * 42 registered MCP tools with a 12-tool default palette (keep in sync with | ||
| * server.ts and the total-tool-count assertion): | ||
| * Default (12): spawn_agent, send_to, wait_for, read_screen, my_agents, | ||
| * list_agents, broadcast, close_surface, dispatch_to_agent, | ||
| * list_surfaces, control_health, stop_agent | ||
| * Remaining tools are INTERIM ToolSearch-deferred and remain callable; | ||
| * reorder_surface is the single approved deletion. | ||
| * Legacy aliases retire next release; the broader deferral is deliberately | ||
| * reversible pending the MCP-vs-CLI/programmatic architecture rethink. | ||
| * Nine public MCP tools. Internal lifecycle and compatibility handlers remain | ||
| * engine implementation details and are never registered on the MCP surface. | ||
|
Comment on lines
+6
to
+7
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win Qualify the claim for test-only compatibility registration. Test mode registers internal compatibility handlers for in-process probes. The phrase “never registered on the MCP surface” conflicts with that supported path. State that this applies to the default production surface, or document the test-only exception. Based on learnings: “Ensure tool definitions and descriptions remain stable across backend changes — only rewrite connector implementations, not tool interfaces.” 🤖 Prompt for AI AgentsSource: Learnings |
||
| */ | ||
|
|
||
| import { renderDoctorJson, renderDoctorText, runDoctor } from "./doctor.js"; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟠 High
src/agent-engine.ts:6122resumeAgentpreserves the previous run'stask_done_candidate_atandtask_done_detected_at, so the next sweep can immediately mark the newly booting resume asdonefrom stale transcript or screen evidence before resumed work runs. Clear both completion markers when rebinding the record.Also found in 2 other location(s)
src/server.ts:6120src/state-manager.ts:388🚀 Reply "fix it for me" or copy this AI Prompt for your agent: