From 37122745b6e0deab4faad25e032e91d6ec309645 Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Sat, 24 Oct 2026 01:37:11 +0800 Subject: [PATCH] fix: retain status observations when session cleanup fails --- src/application/DirectAnalysisCleanup.ts | 3 +- src/application/DirectAnalysisStatus.ts | 53 +++++---- .../boundary/cli/directAnalysisStatus.test.ts | 104 ++++++++++++++++++ 3 files changed, 135 insertions(+), 25 deletions(-) create mode 100644 tests/boundary/cli/directAnalysisStatus.test.ts diff --git a/src/application/DirectAnalysisCleanup.ts b/src/application/DirectAnalysisCleanup.ts index 0be6d875c6..569f183af4 100644 --- a/src/application/DirectAnalysisCleanup.ts +++ b/src/application/DirectAnalysisCleanup.ts @@ -1,4 +1,3 @@ -import type { Evidence } from "../domain/evidence.js"; import type { AnalysisError } from "../domain/analysisErrorBase.js"; import { projectAnalysisError } from "../domain/analysisErrorProjection.js"; import { jsonValueSchema, type JsonValue } from "../domain/jsonValue.js"; @@ -33,7 +32,7 @@ export const withSessionCleanup = async ( /** Project typed one-shot outcomes only after execution and cleanup settle. */ export const directAnalysisCleanupFailure = ( - result: Result, + result: Result, error: AnalysisError, ): JsonValue => { const cleanup = projectAnalysisError(error); diff --git a/src/application/DirectAnalysisStatus.ts b/src/application/DirectAnalysisStatus.ts index 8a17414396..041f6acd5b 100644 --- a/src/application/DirectAnalysisStatus.ts +++ b/src/application/DirectAnalysisStatus.ts @@ -10,6 +10,11 @@ import { sqliteDatabaseCapability } from "./sqlite/SqliteDatabaseCapability.js"; import { CURRENT_ENTRYPOINT_PATH } from "../process/CurrentEntrypointPath.js"; import { createCatalogIdentity } from "../catalogIdentity.js"; import { compactPresentationForMcpInputSchemaProfile } from "../config/mcpInputSchemaProfile.js"; +import { + directAnalysisCleanupFailure, + withSessionCleanup, +} from "./DirectAnalysisCleanup.js"; +import { ok } from "../domain/result.js"; /** Read target-free provider status and operation availability. */ export const runSessionStatus = async ( @@ -22,28 +27,30 @@ export const runSessionStatus = async ( const config = dependencies.readConfiguration(); if (!config.ok) return { error: projectAnalysisError(config.error) }; const session = await dependencies.createBinarySession(config.value, logger); - try { - const status = jsonObjectSchema.parse(await session.status()); - return { - ...status, - capabilities: [ - ...providerCapabilitySet.array().parse(status.capabilities), - ...providerCapabilitySets([ - sqliteDatabaseCapability(process.versions.node), - ]), - ], - server_identity: createServerIdentity({ - startedAt: new Date().toISOString(), - commandPath: CURRENT_ENTRYPOINT_PATH, - catalog: createCatalogIdentity( - compactPresentationForMcpInputSchemaProfile( - config.value.mcpInputSchemaProfile, + return withSessionCleanup( + session, + async () => { + const status = jsonObjectSchema.parse(await session.status()); + return { + ...status, + capabilities: [ + ...providerCapabilitySet.array().parse(status.capabilities), + ...providerCapabilitySets([ + sqliteDatabaseCapability(process.versions.node), + ]), + ], + server_identity: createServerIdentity({ + startedAt: new Date().toISOString(), + commandPath: CURRENT_ENTRYPOINT_PATH, + catalog: createCatalogIdentity( + compactPresentationForMcpInputSchemaProfile( + config.value.mcpInputSchemaProfile, + ), + config.value.mcpOutputSchemaProfile ?? "full", ), - config.value.mcpOutputSchemaProfile ?? "full", - ), - }), - }; - } finally { - await session.close(); - } + }), + }; + }, + (status, cleanup) => directAnalysisCleanupFailure(ok(status), cleanup), + ); }; diff --git a/tests/boundary/cli/directAnalysisStatus.test.ts b/tests/boundary/cli/directAnalysisStatus.test.ts new file mode 100644 index 0000000000..09427a412d --- /dev/null +++ b/tests/boundary/cli/directAnalysisStatus.test.ts @@ -0,0 +1,104 @@ +import { Cli } from "incur"; +import { expect, it, vi } from "vitest"; +import { runSessionStatus } from "../../../src/application/DirectAnalysisStatus.js"; +import { registerUtilityCommands } from "../../../src/cli/utilityCommands.js"; +import { parseConfig } from "../../../src/config/parseConfig.js"; +import { isCliOperationFailure } from "../../../src/cliLogging.js"; +import { ProviderCleanupError } from "../../../src/domain/providerCleanupError.js"; +import { err, ok } from "../../../src/domain/result.js"; +import { silentLogger } from "../../../src/logger.js"; +import { createTestBinarySession } from "../../fixtures/binarySession.js"; +import { observed } from "../../fixtures/analysisExecution.js"; + +const createStatusSession = () => + createTestBinarySession(() => ({ + close: async () => ok(null), + execute: async () => observed(null), + })); + +it.each(["capabilities", "providers"])( + "reports failed %s finalization with the completed status", + async (command) => { + const session = createStatusSession(); + const cleanup = new ProviderCleanupError( + "fixture", + ["retained profile reader"], + { reason: "profile reader close failed" }, + { remediationAction: "Retry the retained profile reader cleanup." }, + ); + const close = vi + .spyOn(session, "close") + .mockResolvedValueOnce(err(cleanup)); + const cli = Cli.create({ name: "rea" }); + registerUtilityCommands( + cli, + silentLogger, + {}, + { + runSessionStatus: () => + runSessionStatus({ + readConfiguration: () => parseConfig({}), + createBinarySession: () => session, + }), + runProviderAnalysis: async () => null, + }, + ); + let stdout = ""; + try { + await cli.serve([command, "--json"], { + env: {}, + stdout: (text) => { + stdout += text; + }, + exit: () => {}, + }); + const output: unknown = JSON.parse(stdout); + expect(output).toMatchObject({ + code: "cleanup_incomplete", + remediation: { action: "Retry the retained profile reader cleanup." }, + details: { + resources: ["retained profile reader"], + partial_observation: { + capabilities: expect.any(Array), + server_identity: expect.any(Object), + }, + }, + }); + expect(isCliOperationFailure(output)).toBe(true); + expect(close).toHaveBeenCalledTimes(1); + } finally { + close.mockRestore(); + await session.close(); + } + }, +); + +it.each([false, true])( + "keeps status failure when final cleanup also fails: %s", + async (cleanupFails) => { + const session = createStatusSession(); + const failure = new Error("status observation failed"); + const cleanup = new ProviderCleanupError("fixture", ["reader"], {}); + const status = vi.spyOn(session, "status").mockRejectedValueOnce(failure); + const close = vi + .spyOn(session, "close") + .mockResolvedValueOnce(cleanupFails ? err(cleanup) : ok(null)); + try { + const result = runSessionStatus({ + readConfiguration: () => parseConfig({}), + createBinarySession: () => session, + }); + if (cleanupFails) + await expect(result).rejects.toMatchObject({ + cause: failure, + errors: [failure, cleanup], + }); + else await expect(result).rejects.toBe(failure); + expect(close).toHaveBeenCalledTimes(1); + } finally { + status.mockRestore(); + close.mockRestore(); + await session.close(); + } + }, +);