mirror of
https://github.com/morluto/rea.git
synced 2026-11-06 06:46:10 +00:00
fix(mcp): name the close_binary retry for retained cleanup (#8329)
* fix(mcp): name the close_binary retry for retained cleanup A close_binary that retired its target but could not confirm cleanup returned cleanup_incomplete with retryable: false and only the generic "Review the reported local resources" remediation, although the session keeps every failed owner and the next close_binary on the same connection retries them. Agents could abandon the connection that owns the only retry. The MCP close tool now passes its retry operation to the session. The session binds that typed AnalysisCleanupRetry to a failed retirement only while it retains the cleanup for its next close; the close tool binds it to process-capture failures its own scope retains and to compositions whose session part was offered. Projection then reports recovery.kind=retry_cleanup and appends a conditional instruction: keep the connection, let the condition settle or resolve it, call close_binary without snapshot_path and check the result. The hint disappears once that cleanup resolves. retryable keeps describing the failed operation. One-shot CLI closes, open_binary, analysis tools and failed snapshot saves (which keep the target open) offer no retry. Cleanup composition copies the owner's remediation without a retry offer, so the text appears once. Fixes #8211 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test: derive close retry guidance from its owning formatters The new close_binary retry assertions froze ProviderCleanupError's explanatory sentence, so a harmless wording edit would break them. The MCP test now compares delivered remediation with analysisErrorRemediationAction applied to the delivered typed error plus cleanupRetryRemediation. The domain tests use the same formatters, check that unbound and released errors carry no retry text, and check that a bound composition adds the offer once. Recovery fields, cleanup state and retry behavior keep separate assertions. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
c534e249d1
commit
8a6ddec83a
16 files changed
+659
-32
No files matched your search
@@ -60,6 +60,7 @@ import {
|
||||
} from "./BinarySessionExecution.js";
|
||||
import { closeAnalysisClient } from "../AnalysisClientCleanup.js";
|
||||
import { analysisErrorWithCleanupFailure } from "../../domain/analysisErrorCleanup.js";
|
||||
import type { AnalysisCleanupRetry } from "../../domain/analysisErrorRecovery.js";
|
||||
import { analysisErrorWithCancellation } from "../../domain/analysisErrorCancellation.js";
|
||||
import { ProviderAdapterError } from "../../domain/providerAdapterError.js";
|
||||
import { profilelessNativeDispatchProfile } from "./ProfilelessNativeWorkflowSnapshot.js";
|
||||
@@ -867,11 +868,15 @@ export class BinarySession
|
||||
close(
|
||||
options: Pick<ExecutionOptions, "progress"> & {
|
||||
readonly retainProviderDocuments?: boolean;
|
||||
readonly cleanupRetry?: AnalysisCleanupRetry;
|
||||
} = {},
|
||||
): Promise<Result<null, AnalysisError>> {
|
||||
return this.#serialize(async (admittedThrough) => {
|
||||
await this.#drainCalls(admittedThrough);
|
||||
return this.#closeActive(options);
|
||||
return this.#offerCleanupRetry(
|
||||
await this.#closeActive(options),
|
||||
options.cleanupRetry,
|
||||
);
|
||||
});
|
||||
}
|
||||
|
||||
@@ -879,7 +884,9 @@ export class BinarySession
|
||||
closeWithSnapshot(
|
||||
path: string,
|
||||
overwrite: boolean,
|
||||
options: Pick<ExecutionOptions, "progress"> = {},
|
||||
options: Pick<ExecutionOptions, "progress"> & {
|
||||
readonly cleanupRetry?: AnalysisCleanupRetry;
|
||||
} = {},
|
||||
): Promise<Result<SavedAnalysisSnapshot, AnalysisError>> {
|
||||
return this.#serialize(async (admittedThrough) => {
|
||||
await this.#drainCalls(admittedThrough);
|
||||
@@ -897,13 +904,43 @@ export class BinarySession
|
||||
workflow_entries: snapshot.value.workflow_entries.length,
|
||||
evidence_records: snapshot.value.evidence_bundle.records.length,
|
||||
};
|
||||
const closed = await this.#closeActive(options);
|
||||
// A failed export or save returned above with the target still open, so
|
||||
// only a failed retirement can offer the caller's cleanup retry.
|
||||
const closed = this.#offerCleanupRetry(
|
||||
await this.#closeActive(options),
|
||||
options.cleanupRetry,
|
||||
);
|
||||
return closed.ok
|
||||
? ok(receipt)
|
||||
: err(retainSavedAnalysisSnapshot(closed.error, receipt));
|
||||
});
|
||||
}
|
||||
|
||||
/** Offer the caller's retry only while this session retains the failed cleanup. */
|
||||
#offerCleanupRetry(
|
||||
closed: Result<null, AnalysisError>,
|
||||
retry: AnalysisCleanupRetry | undefined,
|
||||
): Result<null, AnalysisError> {
|
||||
if (
|
||||
!closed.ok &&
|
||||
retry !== undefined &&
|
||||
closed.error.cleanupIncomplete &&
|
||||
this.#retainsCloseCleanup()
|
||||
)
|
||||
closed.error.bindCleanupRetry(retry);
|
||||
return closed;
|
||||
}
|
||||
|
||||
/** Owners the next close retries; external mutation owners retry elsewhere. */
|
||||
#retainsCloseCleanup(): boolean {
|
||||
return (
|
||||
this.#pendingCleanup !== undefined ||
|
||||
this.#pendingBundleResourceCleanup.length > 0 ||
|
||||
this.hasPendingFileResources ||
|
||||
this.#providerRouter.hasPendingOwnedResources()
|
||||
);
|
||||
}
|
||||
|
||||
async #closeActive(
|
||||
options: Pick<ExecutionOptions, "progress"> & {
|
||||
readonly retainProviderDocuments?: boolean;
|
||||
@@ -1128,10 +1165,7 @@ export class BinarySession
|
||||
providerOperationHealth: client?.operationHealthSnapshot?.() ?? null,
|
||||
cleanupPending:
|
||||
this.#externalMutationCleanupFailure?.cleanupIncomplete === true ||
|
||||
this.#pendingCleanup !== undefined ||
|
||||
this.#pendingBundleResourceCleanup.length > 0 ||
|
||||
this.hasPendingFileResources ||
|
||||
this.#providerRouter.hasPendingOwnedResources(),
|
||||
this.#retainsCloseCleanup(),
|
||||
});
|
||||
}
|
||||
|
||||
|
||||
@@ -11,6 +11,7 @@ import type { AnalysisProfileCommitment } from "../../domain/analysisProfile.js"
|
||||
import type { AnalysisProviderSelector } from "../../domain/providerSelection.js";
|
||||
import type { BinarySessionDetail } from "../../contracts/sessionStatusContract.js";
|
||||
import type { AnalysisError } from "../../domain/analysisErrorBase.js";
|
||||
import type { AnalysisCleanupRetry } from "../../domain/analysisErrorRecovery.js";
|
||||
import type { Result } from "../../domain/result.js";
|
||||
import type { EvidenceIntegrityError } from "../../domain/evidenceErrors.js";
|
||||
import type { AnalysisSnapshotLoader } from "./BinarySessionOpen.js";
|
||||
@@ -76,16 +77,23 @@ export interface BinarySessionPort
|
||||
path: string,
|
||||
options?: Pick<ExecutionOptions, "signal">,
|
||||
): Promise<Result<ResolvedAppBundleResourceRoot, AnalysisError>>;
|
||||
/**
|
||||
* `cleanupRetry` names the caller's same-session retry; the session binds it
|
||||
* to a failed close only while it retains that cleanup for the next close.
|
||||
*/
|
||||
close(
|
||||
options?: Pick<ExecutionOptions, "progress"> & {
|
||||
readonly retainProviderDocuments?: boolean;
|
||||
readonly cleanupRetry?: AnalysisCleanupRetry;
|
||||
},
|
||||
): Promise<Result<null, AnalysisError>>;
|
||||
/** Save a snapshot and close while excluding later provider requests. */
|
||||
closeWithSnapshot(
|
||||
path: string,
|
||||
overwrite: boolean,
|
||||
options?: Pick<ExecutionOptions, "progress">,
|
||||
options?: Pick<ExecutionOptions, "progress"> & {
|
||||
readonly cleanupRetry?: AnalysisCleanupRetry;
|
||||
},
|
||||
): Promise<Result<SavedAnalysisSnapshot, AnalysisError>>;
|
||||
/** Default complete detail; summary omits provider diagnostics and candidate capabilities. */
|
||||
status(options?: {
|
||||
|
||||
Reference in new issue
Block a user