diff --git a/docs/reverse-engineering-tools.md b/docs/reverse-engineering-tools.md index a44cdd990b..2fa2441ea3 100644 --- a/docs/reverse-engineering-tools.md +++ b/docs/reverse-engineering-tools.md @@ -96,6 +96,10 @@ Persistent command Evidence retains the caller-selected and resolved startup pat Session status retains up to 64 recent protocol frames. Each history entry keeps a complete-character UTF-8 prefix of at most 16 KiB, plus a truncation marker when needed; `recent_output_truncated` reports omitted history. This history budget also accounts for replacement characters from invalid UTF-8. It does not shorten command Evidence, whose output budget defaults to 16 MiB. Rizin debug startup waits 30000 ms with no runtime override. Each command frame defaults to 120000 ms; its deadline, command-output budget, session capacity and pending-command queue use the [shared debugger launch settings](#debugger-launch-settings). The 16777216-byte frame cap and 4096-frame command cap remain fixed. Evidence names the cap that stopped the command. The one-shot `command_timeout_ms` setting does not change a debug session. +### Stopped debugger sessions + +A Rizin or GDB session stops accepting commands when REA stops it, for example after an output or frame limit, a command deadline, cancellation or a protocol failure, or when the debugger process exits. REA keeps the stopped session and the first reason it observed until `close_rizin_debug_session` or `close_gdb_session` releases it, including after a retried cleanup succeeds. `rizin_debug_session_status` and `gdb_session_status` then report `state: "closed"` with that reason in `closed_reason`. A later `rizin_debug_command` or `gdb_console` on the session fails with `capability_unavailable`; `details.reason` carries the same reason, and the remediation is to close the session and start a new one. A `session_id` that this server never returned, or that close already released, fails with `invalid_request` instead. + ### Debugger launch settings GDB and Rizin share these settings in the environment that launches REA: diff --git a/src/contracts/gdbToolContracts.ts b/src/contracts/gdbToolContracts.ts index 418abc4e9a..ee15fc43ec 100644 --- a/src/contracts/gdbToolContracts.ts +++ b/src/contracts/gdbToolContracts.ts @@ -63,7 +63,7 @@ export const GDB_TOOL_CONTRACTS = [ ...toolContractMetadata("gdb_session_status"), kind: "native-provider", description: - "Return the observed lifecycle state and bounded recent MI records and process diagnostics for one persistent REA GDB session; truncation and invalid-UTF-8 fields report discarded or lossy data.", + "Return the observed lifecycle state and bounded recent MI records and process diagnostics for one persistent REA GDB session; closed_reason says why a closed session stopped, and truncation and invalid-UTF-8 fields report discarded or lossy data.", inputSchema: z.strictObject({ session_id: sessionId }), outputSchema: z.strictObject({ session_id: sessionId, @@ -79,6 +79,12 @@ export const GDB_TOOL_CONTRACTS = [ diagnostics_truncated: z.boolean(), inferior_tty: z.string().nullable(), inferior_tty_verified: z.boolean(), + closed_reason: z + .string() + .exactOptional() + .describe( + "Present when state is closed: why the session stopped accepting commands. Close it and start a new session to continue.", + ), }), examples: [ { diff --git a/src/contracts/rizinDebugToolContracts.ts b/src/contracts/rizinDebugToolContracts.ts index 7266834ebd..0c9ac55860 100644 --- a/src/contracts/rizinDebugToolContracts.ts +++ b/src/contracts/rizinDebugToolContracts.ts @@ -72,7 +72,7 @@ export const RIZIN_DEBUG_TOOL_CONTRACTS = [ ...toolContractMetadata("rizin_debug_session_status"), kind: "native-provider", description: - "Return observed lifecycle state and bounded recent NUL-framed output and process diagnostics for one Rizin debugger session; truncation fields report discarded data, and output_invalid_utf8 reports whether invalid UTF-8 has been observed in session frame bytes; recent output may contain replacement characters.", + "Return observed lifecycle state and bounded recent NUL-framed output and process diagnostics for one Rizin debugger session; closed_reason says why a closed session stopped, truncation fields report discarded data, and output_invalid_utf8 reports whether invalid UTF-8 has been observed in session frame bytes; recent output may contain replacement characters.", inputSchema: z.strictObject({ session_id: sessionId }), outputSchema: z.strictObject({ session_id: sessionId, @@ -82,6 +82,12 @@ export const RIZIN_DEBUG_TOOL_CONTRACTS = [ recent_output_truncated: z.boolean(), diagnostics_truncated: z.boolean(), output_invalid_utf8: z.boolean(), + closed_reason: z + .string() + .exactOptional() + .describe( + "Present when state is closed: why the session stopped accepting commands. Close it and start a new session to continue.", + ), }), examples: [ { diff --git a/src/domain/debuggerSessionErrors.ts b/src/domain/debuggerSessionErrors.ts new file mode 100644 index 0000000000..99ec96a5e7 --- /dev/null +++ b/src/domain/debuggerSessionErrors.ts @@ -0,0 +1,58 @@ +import { + AnalysisCapabilityUnavailableError, + AnalysisInputError, +} from "./analysisErrorCore.js"; + +/** Tool names and wording for one persistent debugger's session errors. */ +export interface DebuggerSessionTools { + readonly providerId: string; + readonly name: string; + readonly start: string; + readonly close: string; +} + +/** A session_id this server does not hold, unlike a retained stopped session. */ +export const unknownDebuggerSessionError = ( + tools: DebuggerSessionTools, + operation: string, +): AnalysisInputError => + new AnalysisInputError( + operation, + { + conformsToInputContract: true, + userMessage: `No ${tools.name} debug session has this session_id in this REA server.`, + remediationAction: `Use a session_id that ${tools.start} returned in this server process; ${tools.close} releases a session's ID. Call ${tools.start} to begin a new session.`, + }, + [ + { + path: ["session_id"], + reason: "invalid_value", + message: `No ${tools.name} debug session has this session_id in this REA server.`, + }, + ], + ); + +/** Refuse a command on a retained session that stopped, naming why it stopped. */ +export const stoppedDebuggerSessionError = ( + tools: DebuggerSessionTools, + operation: string, + reason: string, +): AnalysisCapabilityUnavailableError => + new AnalysisCapabilityUnavailableError(tools.providerId, operation, reason, { + userMessage: `The ${tools.name} debug session stopped and accepts no more commands. ${reason}`, + remediationAction: `Call ${tools.close} with this session_id to release the stopped session, then call ${tools.start} to begin a new one. Earlier commands may have changed the target, so confirm their effects before repeating them.`, + }); + +export const RIZIN_DEBUG_SESSION_TOOLS = { + providerId: "rizin", + name: "Rizin", + start: "start_rizin_debug_session", + close: "close_rizin_debug_session", +} as const satisfies DebuggerSessionTools; + +export const GDB_SESSION_TOOLS = { + providerId: "gdb", + name: "GDB", + start: "start_gdb_session", + close: "close_gdb_session", +} as const satisfies DebuggerSessionTools; diff --git a/src/gdb/GdbSessionManager.ts b/src/gdb/GdbSessionManager.ts index 3044130959..8c515f4979 100644 --- a/src/gdb/GdbSessionManager.ts +++ b/src/gdb/GdbSessionManager.ts @@ -13,6 +13,11 @@ import { AnalysisInputError, AnalysisTimeoutError, } from "../domain/analysisErrorCore.js"; +import { + GDB_SESSION_TOOLS, + stoppedDebuggerSessionError, + unknownDebuggerSessionError, +} from "../domain/debuggerSessionErrors.js"; import { AnalysisError, type AnalysisErrorOptions, @@ -542,6 +547,8 @@ interface GdbSession { noInferiorVerified: boolean; /** Sticky admission seal once destructive cleanup starts, including failed stops. */ cleanupStarted: boolean; + /** close_gdb_session started cleanup, so a successful retry releases the ID. */ + closeRequested: boolean; stopAttempt: ReturnType | undefined; readonly pending: Map; readonly cleanupFailures: Set; @@ -566,17 +573,8 @@ interface GdbSession { recentRecordsTruncated: boolean; } -const gdbConsoleUnavailable = (): AnalysisInputError => - new AnalysisInputError("gdb_console", undefined, [ - { - path: ["session_id"], - reason: "invalid_value", - message: "GDB session is unavailable or closed.", - }, - ]); - const gdbCloseUnavailable = (): AnalysisInputError => - new AnalysisInputError("close_gdb_session"); + unknownDebuggerSessionError(GDB_SESSION_TOOLS, "close_gdb_session"); /** Owns persistent GDB/MI sessions independently from the active binary target. */ export class GdbSessionManager { @@ -769,7 +767,7 @@ export class GdbSessionManager { "start_gdb_session", "memory", `GDB session capacity reached (${String(this.#maxSessions.value)} sessions).`, - "Close a GDB session before starting another.", + "Close a GDB session before starting another; a stopped session that gdb_session_status reports closed still holds its slot until close_gdb_session releases it.", this.#maxSessions, { max_sessions: this.#maxSessions.value, @@ -903,6 +901,7 @@ export class GdbSessionManager { // during startup, so no inferior can exist until execute() is called. noInferiorVerified: true, cleanupStarted: false, + closeRequested: false, stopAttempt: undefined, pending: new Map(), cleanupFailures: new Set(), @@ -1256,15 +1255,13 @@ export class GdbSessionManager { > > { const session = this.#sessions.get(sessionId); - if ( - this.#closing || - session === undefined || - !session.ready || - session.cleanupStarted || - session.exited || - session.protocolError !== undefined - ) - return err(gdbConsoleUnavailable()); + if (session === undefined) + return err(unknownDebuggerSessionError(GDB_SESSION_TOOLS, "gdb_console")); + const stopped = this.#stopReason(session); + if (stopped !== undefined) + return err( + stoppedDebuggerSessionError(GDB_SESSION_TOOLS, "gdb_console", stopped), + ); if (signal?.aborted) return err(cancelledWithCallerReason("gdb_console", signal)); try { @@ -1273,14 +1270,16 @@ export class GdbSessionManager { async () => { // Retained cleanup ownership and a delayed exit event do not imply // command readiness. Recheck admission when this queued turn starts. - if ( - this.#closing || - this.#sessions.get(sessionId) !== session || - session.cleanupStarted || - session.exited || - session.protocolError !== undefined - ) - return err(gdbConsoleUnavailable()); + // Every path that releases a session first records why it stopped. + const stoppedWhileQueued = this.#stopReason(session); + if (stoppedWhileQueued !== undefined) + return err( + stoppedDebuggerSessionError( + GDB_SESSION_TOOLS, + "gdb_console", + stoppedWhileQueued, + ), + ); session.noInferiorVerified = false; session.knownInferiorIdentities.clear(); if (commandMayChangeInferiorOutput(command)) @@ -1455,21 +1454,17 @@ export class GdbSessionManager { readonly diagnostics_truncated: boolean; readonly inferior_tty: string | null; readonly inferior_tty_verified: boolean; + readonly closed_reason?: string; } | undefined { const session = this.#sessions.get(sessionId); if (session === undefined) return undefined; const diagnostics = session.supervisor.snapshot(); + const stopped = this.#stopReason(session); return { session_id: session.id, mi_version: session.miVersion, - state: - session.cleanupStarted || - session.exited || - session.protocolError !== undefined || - !session.ready - ? "closed" - : "ready", + state: stopped === undefined ? "ready" : "closed", recent_mi_records: [...session.lines], recent_mi_records_truncated: session.recentRecordsTruncated, mi_invalid_utf8: session.invalidUtf8Observed, @@ -1481,9 +1476,24 @@ export class GdbSessionManager { diagnostics_truncated: diagnostics.diagnosticTruncated === true, inferior_tty: session.inferiorTty ?? null, inferior_tty_verified: session.inferiorTtyVerified, + ...(stopped === undefined ? {} : { closed_reason: stopped }), }; } + /** Why a retained session accepts no commands, or undefined while it is ready. */ + #stopReason(session: GdbSession): string | undefined { + if (session.protocolError !== undefined) return session.protocolError; + if (this.#closing) + return "REA is shutting down and closing every GDB session."; + if (session.closeRequested) + return "close_gdb_session started releasing this session, and its owned cleanup has not completed."; + if (session.exited) + return `The GDB process exited (exit code ${String(session.exitCode)}, signal ${String(session.exitSignal)}); a command such as quit, or a crash, can end it.`; + if (session.cleanupStarted || !session.ready) + return "REA stopped this GDB session."; + return undefined; + } + async close( sessionId: string, ): Promise< @@ -1549,6 +1559,7 @@ export class GdbSessionManager { // Refusing inferior disposition above leaves console remediation // available; from here onward only owned cleanup may be retried. session.cleanupStarted = true; + session.closeRequested = true; if (groupCount === 0) { session.noInferiorVerified = true; targetState = "none_observed"; @@ -1588,6 +1599,7 @@ export class GdbSessionManager { async #finishSessionCleanup( session: GdbSession, ): Promise> { + session.closeRequested = true; const stopped = await this.#stopDebuggerOnly(session); if (stopped.status === "incomplete") return err( @@ -1614,7 +1626,10 @@ export class GdbSessionManager { stopped.reason, ), ); - this.#sessions.delete(session.id); + // The caller still holds a published session's ID; keep it addressable + // so commands and status report why it stopped until it is closed. + if (!session.ready || session.closeRequested) + this.#sessions.delete(session.id); } return ok(null); } @@ -2209,6 +2224,7 @@ export class GdbSessionManager { "The unrestricted GDB console can access local shell, filesystem, network, and target controls.", ], }); + session.protocolError ??= `A GDB command exceeded its ${String(acceptedDeadline.value)} ms deadline; owned-session shutdown was requested because output correlation can no longer be guaranteed.`; void this.#stopDebuggerOnly(session).then((stopped) => { if ( stopped.status === "incomplete" && diff --git a/src/rizin/RizinDebugSessionManager.ts b/src/rizin/RizinDebugSessionManager.ts index f0bc894c5a..5dc1335797 100644 --- a/src/rizin/RizinDebugSessionManager.ts +++ b/src/rizin/RizinDebugSessionManager.ts @@ -22,6 +22,11 @@ import { AnalysisInputError, AnalysisTimeoutError, } from "../domain/analysisErrorCore.js"; +import { + RIZIN_DEBUG_SESSION_TOOLS, + stoppedDebuggerSessionError, + unknownDebuggerSessionError, +} from "../domain/debuggerSessionErrors.js"; import type { AnalysisError, AnalysisErrorOptions, @@ -224,9 +229,18 @@ interface RizinDebugSession { frameChunks: Buffer[]; bufferedFrameBytes: number; ready: boolean; + /** start returned this session's ID, so the caller must close it. */ + published: boolean; + closeRequested: boolean; exited: boolean; commandTail: Promise; protocolError: string | undefined; + /** + * Why the session stopped accepting commands. Cleanup synchronization + * clears protocolError once the transport is correlated again, but the + * session stays stopped, so the diagnosis is retained here. + */ + stopReason: string | undefined; cleanupSynchronization: | { readonly marker: string; @@ -364,7 +378,7 @@ export class RizinDebugSessionManager { "start_rizin_debug_session", "memory", `Rizin session capacity reached (${String(this.#maxSessions.value)} sessions).`, - "Close a Rizin debug session before starting another.", + "Close a Rizin debug session before starting another; a stopped session that rizin_debug_session_status reports closed still holds its slot until close_rizin_debug_session releases it.", this.#maxSessions, { max_sessions: this.#maxSessions.value, @@ -609,9 +623,12 @@ export class RizinDebugSessionManager { frameChunks: [], bufferedFrameBytes: 0, ready: false, + published: false, + closeRequested: false, exited: false, commandTail: Promise.resolve(), protocolError: undefined, + stopReason: undefined, cleanupSynchronization: undefined, commandCaptureActive: false, commandOutputDropped: false, @@ -651,6 +668,7 @@ export class RizinDebugSessionManager { if (session.protocolError !== undefined) throw new Error(session.protocolError); session.ready = true; + session.published = true; return ok({ session_id: id, backend: input.backend ?? null }); } catch (caught) { const protocolFailure = session.protocolError; @@ -719,15 +737,22 @@ export class RizinDebugSessionManager { > > { const session = this.#sessions.get(sessionId); - if ( - session === undefined || - !session.ready || - session.exited || - session.protocolError !== undefined || - session.closing || - this.#closingAll - ) - return err(new AnalysisInputError("rizin_debug_command")); + if (session === undefined) + return err( + unknownDebuggerSessionError( + RIZIN_DEBUG_SESSION_TOOLS, + "rizin_debug_command", + ), + ); + const stopped = this.#stopReason(session); + if (stopped !== undefined) + return err( + stoppedDebuggerSessionError( + RIZIN_DEBUG_SESSION_TOOLS, + "rizin_debug_command", + stopped, + ), + ); if (signal?.aborted) return err(rizinCancellation("rizin_debug_command", signal)); if (session.pendingCommandCount >= this.#maxPendingCommands.value) @@ -782,14 +807,15 @@ export class RizinDebugSessionManager { acquired = true; if (signal?.aborted) return err(rizinCancellation("rizin_debug_command", signal)); - if ( - !session.ready || - session.exited || - session.closing || - this.#closingAll || - session.protocolError !== undefined - ) - return err(new AnalysisInputError("rizin_debug_command")); + const stoppedWhileQueued = this.#stopReason(session); + if (stoppedWhileQueued !== undefined) + return err( + stoppedDebuggerSessionError( + RIZIN_DEBUG_SESSION_TOOLS, + "rizin_debug_command", + stoppedWhileQueued, + ), + ); if (hasUnclaimedRizinFrames(session)) { const message = "Unsolicited Rizin frames arrived before a command; command output can no longer be safely correlated."; @@ -1068,27 +1094,40 @@ export class RizinDebugSessionManager { readonly recent_output_truncated: boolean; readonly diagnostics_truncated: boolean; readonly output_invalid_utf8: boolean; + readonly closed_reason?: string; } | undefined { const session = this.#sessions.get(sessionId); - return session === undefined - ? undefined - : { - session_id: session.id, - state: - session.exited || - session.protocolError !== undefined || - session.closing || - !session.ready - ? "closed" - : "ready", - inferior_tty: session.inferiorTty, - recent_output: [...session.history], - recent_output_truncated: session.historyTruncated, - diagnostics_truncated: - session.supervisor.snapshot().diagnosticTruncated === true, - output_invalid_utf8: session.invalidUtf8Observed, - }; + if (session === undefined) return undefined; + const stopped = this.#stopReason(session); + return { + session_id: session.id, + state: stopped === undefined ? "ready" : "closed", + inferior_tty: session.inferiorTty, + recent_output: [...session.history], + recent_output_truncated: session.historyTruncated, + diagnostics_truncated: + session.supervisor.snapshot().diagnosticTruncated === true, + output_invalid_utf8: session.invalidUtf8Observed, + ...(stopped === undefined ? {} : { closed_reason: stopped }), + }; + } + + /** Why a retained session accepts no commands, or undefined while it is ready. */ + #stopReason(session: RizinDebugSession): string | undefined { + const diagnosed = session.stopReason ?? session.protocolError; + if (diagnosed !== undefined) return diagnosed; + if (this.#closingAll) + return "REA is shutting down and closing every Rizin debug session."; + if (session.closeRequested) + return "close_rizin_debug_session started releasing this session, and its owned cleanup has not completed."; + if (session.exited) { + const { exitCode, signalCode } = session.launched.process; + return `The Rizin process exited (exit code ${String(exitCode)}, signal ${String(signalCode)}); a command such as q, or a crash, can end it.`; + } + if (session.closing || !session.ready) + return "REA stopped this Rizin debug session."; + return undefined; } async close( @@ -1098,7 +1137,13 @@ export class RizinDebugSessionManager { return err(new AnalysisInputError("close_rizin_debug_session")); const session = this.#sessions.get(sessionId); if (session === undefined) - return err(new AnalysisInputError("close_rizin_debug_session")); + return err( + unknownDebuggerSessionError( + RIZIN_DEBUG_SESSION_TOOLS, + "close_rizin_debug_session", + ), + ); + session.closeRequested = true; session.closing = true; const stopped = await this.#stopDebuggerOnly(session); if (stopped.status === "incomplete") @@ -1141,7 +1186,10 @@ export class RizinDebugSessionManager { stopped.reason, ), ); - this.#sessions.delete(session.id); + // The caller still holds a published session's ID; keep it addressable + // so commands and status report why it stopped until it is closed. + if (!session.published || session.closeRequested) + this.#sessions.delete(session.id); } return ok(null); } @@ -1516,6 +1564,7 @@ export class RizinDebugSessionManager { clearTimeout(timer); session.cleanupSynchronization = undefined; if (cause === undefined) { + session.stopReason ??= session.protocolError; session.protocolError = undefined; session.commandOutputDropped = false; resolve(); diff --git a/src/server/registerGdbTools.ts b/src/server/registerGdbTools.ts index 7e13a40f0e..c47a2898cf 100644 --- a/src/server/registerGdbTools.ts +++ b/src/server/registerGdbTools.ts @@ -2,6 +2,10 @@ import type { McpServer, ServerContext } from "@modelcontextprotocol/server"; import type { Logger } from "pino"; import type { GdbSessionManager } from "../gdb/GdbSessionManager.js"; +import { + GDB_SESSION_TOOLS, + unknownDebuggerSessionError, +} from "../domain/debuggerSessionErrors.js"; import type { EvidenceWriter } from "../application/investigation/InvestigationRecordPort.js"; import type { EvidenceLookup } from "../application/EvidenceInputResolver.js"; import { @@ -10,7 +14,6 @@ import { } from "../application/analysisView/AnalysisViewService.js"; import { toolContract } from "../contracts/toolContracts.js"; import { err, ok } from "../domain/result.js"; -import { AnalysisInputError } from "../domain/analysisErrorCore.js"; import { runOwnedCleanupWithExternalMutationAdmission, runWithExternalMutationAdmission, @@ -117,7 +120,12 @@ export const registerGdbTools = ( const result = manager.status(input.session_id); if (result === undefined) return delivery.toCallToolResult( - err(invalidSession("gdb_session_status")), + err( + unknownDebuggerSessionError( + GDB_SESSION_TOOLS, + "gdb_session_status", + ), + ), status, ); return delivery.toCallToolResult( @@ -150,6 +158,3 @@ export const registerGdbTools = ( }, ); }; - -const invalidSession = (operation: string): AnalysisInputError => - new AnalysisInputError(operation); diff --git a/src/server/registerRizinDebugTools.ts b/src/server/registerRizinDebugTools.ts index 858c4041db..04d4ae49cb 100644 --- a/src/server/registerRizinDebugTools.ts +++ b/src/server/registerRizinDebugTools.ts @@ -3,7 +3,10 @@ import type { Logger } from "pino"; import type { EvidenceWriter } from "../application/investigation/InvestigationRecordPort.js"; import { toolContract } from "../contracts/toolContracts.js"; -import { AnalysisInputError } from "../domain/analysisErrorCore.js"; +import { + RIZIN_DEBUG_SESSION_TOOLS, + unknownDebuggerSessionError, +} from "../domain/debuggerSessionErrors.js"; import type { RizinDebugSessionManager } from "../rizin/RizinDebugSessionManager.js"; import { runOwnedCleanupWithExternalMutationAdmission, @@ -99,7 +102,13 @@ export const registerRizinDebugTools = ( const result = manager.status(input.session_id); if (result === undefined) return delivery.toCallToolResult( - { ok: false, error: new AnalysisInputError(status.name) }, + { + ok: false, + error: unknownDebuggerSessionError( + RIZIN_DEBUG_SESSION_TOOLS, + status.name, + ), + }, status, ); return delivery.toCallToolResult( diff --git a/tests/boundary/mcp/gdbCancellationMcp.test.ts b/tests/boundary/mcp/gdbCancellationMcp.test.ts index 492ab15807..38e2f4b595 100644 --- a/tests/boundary/mcp/gdbCancellationMcp.test.ts +++ b/tests/boundary/mcp/gdbCancellationMcp.test.ts @@ -3,6 +3,7 @@ import { tmpdir } from "node:os"; import { join } from "node:path"; import { Client, InMemoryTransport } from "@modelcontextprotocol/client"; +import type { CallToolResult } from "@modelcontextprotocol/server"; import { McpServer } from "@modelcontextprotocol/server"; import { Ajv2020 } from "ajv/dist/2020.js"; import { expect, it } from "vitest"; @@ -28,6 +29,7 @@ import { createCacheProvider, createTestBinarySession, } from "../../fixtures/binarySession.js"; +import { parseMcpToolError } from "../../fixtures/mcpToolError.js"; import { validateDebuggerStatusResponse, waitForStartedFile, @@ -222,8 +224,32 @@ process.stdin.on("data", async (chunk) => { "gdb_session_status", sessionId, ); - expect(status.state).toBe("closed"); + expect(status).toMatchObject({ + state: "closed", + closed_reason: expect.stringContaining( + "A GDB MI command was cancelled", + ), + }); expect(status).toHaveProperty("diagnostics_truncated"); + // A later command names the retained reason instead of an argument error. + expect( + parseMcpToolError( + (await client.callTool({ + name: "gdb_console", + arguments: { session_id: sessionId, command: "info registers" }, + })) as CallToolResult, + ).error, + ).toMatchObject({ + code: "capability_unavailable", + message: expect.stringContaining( + "The GDB debug session stopped and accepts no more commands. A GDB MI command was cancelled", + ), + remediation: { + action: expect.stringMatching( + /close_gdb_session.*start_gdb_session/u, + ), + }, + }); await manager.closeAll(); expect(stopResults).toContain("stopped"); } finally { diff --git a/tests/boundary/mcp/rizinStoppedSessionMcp.test.ts b/tests/boundary/mcp/rizinStoppedSessionMcp.test.ts new file mode 100644 index 0000000000..81ea4a813b --- /dev/null +++ b/tests/boundary/mcp/rizinStoppedSessionMcp.test.ts @@ -0,0 +1,154 @@ +import { Client, InMemoryTransport } from "@modelcontextprotocol/client"; +import type { CallToolResult } from "@modelcontextprotocol/server"; +import { McpServer } from "@modelcontextprotocol/server"; +import { expect, it } from "vitest"; + +import { closeVerifierResources } from "../../../scripts/lib/verifier-cleanup.mjs"; +import { toolContract } from "../../../src/contracts/toolContracts.js"; +import { silentLogger } from "../../../src/logger.js"; +import { registerRizinDebugTools } from "../../../src/server/registerRizinDebugTools.js"; +import { createToolResultDelivery } from "../../../src/server/toolResult.js"; +import { parseMcpToolError } from "../../fixtures/mcpToolError.js"; +import { validateDebuggerStatusResponse } from "../../support/reverseDebuggerMcpTestHelpers.js"; +import { createInjectedProcess } from "../providers/RizinDebugSessionCoverage.fixtures.js"; +import { + createRizinTestManager, + rizinTestInferiorTty, +} from "../providers/RizinTestInferiorTty.js"; + +const test = it.skipIf(process.platform === "win32"); + +const LIMIT_REASON = "Rizin command output exceeded the 8 byte command limit."; + +// The injected stream simulates Rizin's NUL-framed protocol; it is not Rizin. +test.each([ + { cleanupRetry: false, command: "noisy", reason: LIMIT_REASON }, + { cleanupRetry: true, command: "noisy", reason: LIMIT_REASON }, + { + cleanupRetry: false, + command: "q", + reason: "The Rizin process exited (exit code null, signal SIGTERM)", + }, +])( + "reports why a Rizin session stopped through MCP after $command (cleanup retry: $cleanupRetry)", + async ({ cleanupRetry, command, reason }) => { + let stops = 0; + const injected = createInjectedProcess({ + onWrite: (text, control) => { + const marker = /!echo (.+)\n/u.exec(text)?.[1]; + if (marker === undefined) return; + if (text.startsWith("noisy\n")) + control.output(`too much output\0${marker}\0`); + else if (text.startsWith("q\n")) setImmediate(() => control.exit()); + else if (text.startsWith("!echo __REA_CLEANUP_SYNC_")) + control.output(`${marker}\0`); + else if (text.startsWith("olj\n")) control.output(`[]\0${marker}\0`); + }, + }); + const manager = createRizinTestManager( + { REA_DEBUGGER_MAX_COMMAND_OUTPUT_BYTES: "8" }, + { + spawnProcess: injected.spawnProcess, + // The injected stream has no OS process; detach through the protocol. + stopProcess: async (session) => { + stops += 1; + if (cleanupRetry && stops === 1) + return { status: "incomplete", reason: "injected stop pending" }; + if (session.exited) return { status: "already_exited" }; + await session.detachInferior?.(); + injected.control.exit(); + return { status: "stopped" }; + }, + }, + ); + const server = new McpServer({ name: "rizin-stopped", version: "1" }); + registerRizinDebugTools( + server, + manager, + silentLogger, + createToolResultDelivery(undefined), + ); + const client = new Client({ name: "rizin-stopped", version: "1" }); + const [clientTransport, serverTransport] = + InMemoryTransport.createLinkedPair(); + const call = (name: string, arguments_: Record) => + client.callTool({ + name, + arguments: arguments_, + }) as Promise; + try { + await server.connect(serverTransport); + await client.connect(clientTransport); + const { session_id } = toolContract( + "start_rizin_debug_session", + ).outputSchema.parse( + ( + await call("start_rizin_debug_session", { + path: "fixture.bin", + inferior_tty: await rizinTestInferiorTty(), + }) + ).structuredContent, + ); + + const stopping = await call("rizin_debug_command", { + session_id, + command, + }); + if (cleanupRetry) + expect(parseMcpToolError(stopping).error.code).toBe( + "cleanup_incomplete", + ); + else expect(stopping.isError).not.toBe(true); + + // The next call retries retained cleanup first, then refuses the + // command with the retained diagnosis instead of an argument error. + const later = parseMcpToolError( + await call("rizin_debug_command", { session_id, command: "dr" }), + ).error; + expect(later).toMatchObject({ + code: "capability_unavailable", + message: expect.stringContaining( + "The Rizin debug session stopped and accepts no more commands.", + ), + remediation: { + action: expect.stringMatching( + /close_rizin_debug_session.*start_rizin_debug_session/u, + ), + }, + details: { reason: expect.stringContaining(reason) }, + }); + expect(later.message).toContain(reason); + expect(stops).toBe(cleanupRetry ? 2 : 1); + + const status = await validateDebuggerStatusResponse( + client, + "rizin_debug_session_status", + session_id, + ); + expect(status).toMatchObject({ + state: "closed", + closed_reason: expect.stringContaining(reason), + }); + + expect( + (await call("close_rizin_debug_session", { session_id })).isError, + ).not.toBe(true); + // A released ID is unknown, which differs from a retained stopped session. + expect( + parseMcpToolError( + await call("rizin_debug_command", { session_id, command: "dr" }), + ).error, + ).toMatchObject({ + code: "invalid_request", + message: + "No Rizin debug session has this session_id in this REA server.", + }); + } finally { + await closeVerifierResources([ + () => client.close(), + () => server.close(), + () => manager.closeAll(), + ]); + } + }, +); diff --git a/tests/boundary/providers/GdbSessionManagerCoverage.test.ts b/tests/boundary/providers/GdbSessionManagerCoverage.test.ts index 9795035257..b498ebb6f4 100644 --- a/tests/boundary/providers/GdbSessionManagerCoverage.test.ts +++ b/tests/boundary/providers/GdbSessionManagerCoverage.test.ts @@ -695,9 +695,9 @@ it("rejects GDB commands queued before closeAll when their turn arrives", async await expect(queued).resolves.toMatchObject({ ok: false, error: { - _tag: "AnalysisInputError", + _tag: "AnalysisCapabilityUnavailableError", operation: "gdb_console", - issues: [{ path: ["session_id"], reason: "invalid_value" }], + reason: expect.stringContaining("REA is shutting down"), }, }); await closing; @@ -732,9 +732,9 @@ it("rejects GDB commands queued behind a session close", async () => { await expect(queued).resolves.toMatchObject({ ok: false, error: { - _tag: "AnalysisInputError", + _tag: "AnalysisCapabilityUnavailableError", operation: "gdb_console", - issues: [{ path: ["session_id"], reason: "invalid_value" }], + reason: expect.stringContaining("close_gdb_session started releasing"), }, }); expect(commands.some((command) => command.includes('"run"'))).toBe(false); @@ -785,9 +785,9 @@ it.each(["close", "retryCleanupFailures"] as const)( const unavailable = { ok: false, error: { - _tag: "AnalysisInputError", + _tag: "AnalysisCapabilityUnavailableError", operation: "gdb_console", - issues: [{ path: ["session_id"], reason: "invalid_value" }], + reason: expect.stringContaining("close_gdb_session started releasing"), }, }; // Finish the barrier before assertions so a regression cannot strand teardown. @@ -1874,7 +1874,10 @@ it.each([ const command = await manager.execute(sessionId, "show version"); expect(command).toMatchObject({ ok: false, - error: { _tag: "AnalysisInputError" }, + error: { + _tag: "AnalysisCapabilityUnavailableError", + operation: "gdb_console", + }, }); expect(commands).not.toContain('-interpreter-exec console "show version"'); expect(fake.children[0]?.exitCode).toBeNull(); @@ -2301,7 +2304,11 @@ it("rejects the command queued behind a timed-out GDB request", async () => { const rejected = await queued; expect(rejected.ok).toBe(false); - if (!rejected.ok) expect(rejected.error._tag).toBe("AnalysisInputError"); + if (!rejected.ok) + expect(rejected.error).toMatchObject({ + _tag: "AnalysisCapabilityUnavailableError", + reason: expect.stringContaining("timed out"), + }); }); it("retains a multi-run console command through its completion query before dispatching a follower", async () => { @@ -2469,7 +2476,7 @@ it("retains unknown completion and partial Evidence when the completion query fa await manager.execute(started.value.session_id, "show version"), ).toMatchObject({ ok: false, - error: { _tag: "AnalysisInputError" }, + error: { _tag: "AnalysisCapabilityUnavailableError" }, }); }); @@ -2600,7 +2607,7 @@ it.each([false, true])( if (follower !== undefined) expect(await follower).toMatchObject({ ok: false, - error: { _tag: "AnalysisInputError" }, + error: { _tag: "AnalysisCapabilityUnavailableError" }, }); expect(commands.some((command) => command.includes("show version"))).toBe( false, @@ -3995,6 +4002,8 @@ it.each(["incomplete", "rejected"] as const)( fake.children[0]?.exit(0, null); return { status: "stopped" }; } + // That simulated verification stands for later stops of the session. + if (stopCalls > 3) return { status: "already_exited" }; return stopOwnedDebuggerProcess(session, name); }, }); @@ -4044,15 +4053,25 @@ it.each(["incomplete", "rejected"] as const)( ); expect(subsequent).toMatchObject({ ok: false, - error: { _tag: "AnalysisInputError" }, + error: { + _tag: "AnalysisCapabilityUnavailableError", + reason: expect.stringContaining("GDB stdin write failed"), + }, }); expect((await manager.retryCleanupFailures()).ok).toBe(false); expect(stopCalls).toBe(2); if (!failed.ok) expect(failed.error.cleanupIncomplete).toBe(true); expect((await manager.retryCleanupFailures()).ok).toBe(true); expect(stopCalls).toBe(3); - expect(manager.status(started.value.session_id)).toBeUndefined(); if (!failed.ok) expect(failed.error.cleanupIncomplete).toBe(false); + // The caller still holds the published ID, so the diagnosis outlives + // cleanup until close_gdb_session releases it. + expect(manager.status(started.value.session_id)).toMatchObject({ + state: "closed", + closed_reason: expect.stringContaining("GDB stdin write failed"), + }); + expect((await manager.close(started.value.session_id)).ok).toBe(true); + expect(manager.status(started.value.session_id)).toBeUndefined(); }, ); diff --git a/tests/boundary/providers/RizinDebugSessionCoverage.part3.test.ts b/tests/boundary/providers/RizinDebugSessionCoverage.part3.test.ts index 1ac7bc939f..0f433bdefd 100644 --- a/tests/boundary/providers/RizinDebugSessionCoverage.part3.test.ts +++ b/tests/boundary/providers/RizinDebugSessionCoverage.part3.test.ts @@ -1,10 +1,7 @@ import { afterEach, expect, it, vi } from "vitest"; import { withVerifierCleanup } from "../../../scripts/lib/verifier-cleanup.mjs"; import { awaitAcknowledgement } from "../../fixtures/lifecycleAcknowledgement.js"; -import { - AnalysisCancelledError, - AnalysisInputError, -} from "../../../src/domain/analysisErrorCore.js"; +import { AnalysisCancelledError } from "../../../src/domain/analysisErrorCore.js"; import { stopOwnedDebuggerProcess } from "../../../src/process/ownedDebuggerSession.js"; import { rizinTestInferiorTty } from "./RizinTestInferiorTty.js"; import { @@ -355,7 +352,13 @@ it.skipIf(process.platform === "win32")( expect((await first).ok).toBe(true); const second = await queued; expect(second.ok).toBe(false); - if (!second.ok) expect(second.error).toBeInstanceOf(AnalysisInputError); + if (!second.ok) + expect(second.error).toMatchObject({ + _tag: "AnalysisCapabilityUnavailableError", + reason: expect.stringContaining( + "The Rizin process exited (exit code null, signal SIGTERM)", + ), + }); expect(writes.some((text) => text.includes("second\n"))).toBe(false); }, );