mirror of
https://github.com/morluto/rea
synced 2026-10-26 22:28:27 -04:00
fix(reverse): reconcile cleanup ownership and mutation diagnostics (#2799)
* feat(process): add owned debugger shutdown lifecycle * feat(reverse): add local inspection commands * feat(gdb): add persistent debugger integration * feat(cutter): add Cutter bridge integration * test(reverse): enforce complete integration coverage * fix(process): preserve debugger shutdown ownership * fix(reverse): retain evidence across interrupted commands * fix(gdb): honor captured provider environment * test(process): cover debugger shutdown retries * fix(reverse): retain evidence for interrupted providers * test(process): cover debugger shutdown retries * fix(reverse): retain evidence for interrupted providers * fix(server): snapshot provider environment at creation * fix(reverse): close review gaps with complete coverage * fix(reverse): address inspection review findings * fix(gdb): bound sessions and retain cleanup ownership * fix(ci): require reverse-tools quality lane * fix(process): unref inherited debugger output pipes * fix(reverse): address remaining PR 1960 findings * fix(gdb): address remaining PR 1961 findings * fix(reverse): account for unrestricted provider execution * fix(process): retain debugger ownership until safe shutdown * fix(process): preserve replacement debugger sessions * fix(reverse): preserve admission and output provenance * fix(process): retain exited debugger until output settles * fix(mcp): bound provider admission and debugger cleanup * fix(reverse): preserve failure provenance and classify E2BIG * fix(gdb): preserve startup cleanup and detach safety * fix(process): retain debugger ownership after detach failure * fix(reverse): preserve mutation and failure provenance * fix(gdb): address debugger review findings * fix(reverse): rebind sessions after external mutation * fix(process): cap debugger output after exit * fix(gdb): protect one-shot detach verification * feat(debugger): add reverse tool coverage gate * test(debugger): cover integrated failure paths * fix(gdb): bound and preserve session observations * fix(gdb): address review regressions * style: format merged integration files * fix(reverse): preserve session and output evidence * refactor(gdb): share repeated test and partial result setup * fix(gdb): invalidate TTY proof for Python aliases * fix(reverse): retain failure output byte counts * test(process): synchronize late debugger output * fix(gdb): invalidate TTY proof for wrapped commands * fix(gdb): recognize wit command abbreviation * test(reverse): wait for cancelled descendant readiness * test(reverse): bound descendant cancellation cleanup * style(reverse): format integration tests and adapters * style: format reverse engineering integration files * test(reverse): remove duplicate environment harness * test(gdb): share session startup assertions * fix(gdb): invalidate output proof for command abbreviations * test(reverse): share composition edge case scenarios * fix(ci): recognize verification and mutation tool entries * test(rizin): cover successful fake inferior detach * test(cli): align full output failure envelope * test(cli): isolate reverse command lifecycle service * test(process): share debugger output fixture setup * fix(binary): preserve high-bit Windows signature bytes * style(binary): format Windows signature parser * test(mcp): preserve call coverage in unchanged comparison * fix(ci): include reverse tools Vitest configs in lint cache * fix(mcp): preserve target content on delivery errors * ci: align duplication ceiling with expanded suite * test(reverse): allow signal admission mutants * fix(reverse): bound retained cleanup failures to process owners * fix(test): replace drifting mutation ranges with owned selectors * fix(reverse): admit persistent commands and retain cleanup ownership * fix(cutter): recover timed-out commands from exact receipts * fix(cutter): reject unsupported bridge request kinds * fix(ci): repair client format and shared verification fixtures * fix(native): preserve debugger cleanup ownership and bridge authentication * test(native): retain diagnostics without perfect coverage requirements * chore(licenses): refresh locked dependency inventory and notices * fix(test): exclude transient command locks from mutation sandboxes * fix(gdb): admit only platforms with safe detach verification * fix(cutter): retain bounded completion receipts until acknowledgement * fix(cutter): acknowledge commands rejected before UI dispatch * test(reverse): fail cancellation checks before fixture teardown * test(gdb): clean up the retained broken-stdin fixture owner * docs(test): clarify focused provider verification scope * fix(rizin): retain close cleanup failures * fix(gdb): wait for inferior stop and bound MI buffering * fix(cutter): retain failed receipt acknowledgements * fix(rizin): return retained session identity on rebind failure * fix(gdb): preserve startup cancellation and lossy status * fix(rizin): retain launch provenance and assemble frames once * fix(cutter): recover receipts across callers and bound discovery * docs: describe debugger evidence and bridge recovery * fix(process): keep cleanup incomplete while writers remain Stopping a debugger no longer reports success when inherited output streams stay open, so BinarySession cannot reopen a binary a child is still changing. Concurrent Cutter cleanup retries share one receipt acknowledgement instead of treating a consumed receipt as failure. * fix(debuggers): recover cleanup with captured process identities * fix(analysis): retire sessions after unrestricted commands * fix(cli): preserve completed evidence after cleanup recovery * fix(cutter): bound frame collection and coalesce cleanup * fix(windows): resolve selected provider paths absolutely * fix(gdb): report decoding loss in failed startup evidence * fix(rizin): scope decoding loss to collected command frames * test(mcp): cover snapshot retirement in reverse tools CI --------- Co-authored-by: Daniel Gonçalves Araujo <47118053+dandgabr@users.noreply.github.com> Co-authored-by: REA Maintainers <noreply@github.com> Co-authored-by: OpenAI Codex <codex@users.noreply.github.com> Co-authored-by: Cursor Agent <cursoragent@cursor.com>
This commit is contained in:
@@ -5,7 +5,10 @@ import {
|
||||
analysisProfilesEqual,
|
||||
type AnalysisProfileCommitment,
|
||||
} from "../../domain/analysisProfile.js";
|
||||
import { AnalysisCancelledError } from "../../domain/analysisErrorCore.js";
|
||||
import {
|
||||
AnalysisCancelledError,
|
||||
AnalysisOutputError,
|
||||
} from "../../domain/analysisErrorCore.js";
|
||||
import type { AnalysisError } from "../../domain/analysisErrorBase.js";
|
||||
import { err, ok, type Result } from "../../domain/result.js";
|
||||
import type { JsonValue } from "../../domain/jsonValue.js";
|
||||
@@ -73,6 +76,7 @@ export class BinarySession
|
||||
implements BinarySessionPort
|
||||
{
|
||||
#active: SessionBinding | undefined;
|
||||
#externalMutationCleanupFailure: AnalysisError | undefined;
|
||||
#pendingCleanup:
|
||||
| {
|
||||
readonly client: AnalysisClient;
|
||||
@@ -158,6 +162,10 @@ export class BinarySession
|
||||
);
|
||||
}
|
||||
|
||||
invalidateSnapshotForExternalMutation(): void {
|
||||
this.invalidateSnapshot();
|
||||
}
|
||||
|
||||
/** Return the selected immutable profile, optionally scoped to an operation. */
|
||||
analysisProfile(
|
||||
operation?: AnalysisOperation,
|
||||
@@ -245,110 +253,180 @@ export class BinarySession
|
||||
resolve: () => Promise<Result<ResolvedSessionOpen, AnalysisError>>,
|
||||
options: Pick<BinarySessionOpenOptions, "signal" | "snapshot">,
|
||||
): Promise<Result<BinaryTarget, AnalysisError>> {
|
||||
return this.#serialize(async (admittedThrough) => {
|
||||
if (isAborted(options.signal))
|
||||
return err(new AnalysisCancelledError("open_binary"));
|
||||
const resolved = await resolve();
|
||||
if (!resolved.ok) return resolved;
|
||||
const { target, route, sameTarget } = resolved.value;
|
||||
const { profile } = route;
|
||||
if (isAborted(options.signal))
|
||||
return err(new AnalysisCancelledError("open_binary"));
|
||||
const activeProfile = this.#active?.profile;
|
||||
const sameProfile =
|
||||
activeProfile === null || activeProfile === undefined
|
||||
? profile === null
|
||||
: profile !== null && analysisProfilesEqual(activeProfile, profile);
|
||||
await this.#drainCalls(admittedThrough);
|
||||
if (isAborted(options.signal))
|
||||
return err(new AnalysisCancelledError("open_binary"));
|
||||
const pendingClosed = await this.#retryPendingCleanup();
|
||||
if (!pendingClosed.ok) return pendingClosed;
|
||||
if (isAborted(options.signal))
|
||||
return err(new AnalysisCancelledError("open_binary"));
|
||||
const retained = sameTarget && sameProfile ? this.#active : undefined;
|
||||
if (retained !== undefined) {
|
||||
const fastPath = await this.#tryRetainedFastPath(
|
||||
retained,
|
||||
target,
|
||||
options,
|
||||
);
|
||||
if (fastPath !== undefined) return fastPath;
|
||||
}
|
||||
const previous = this.#active;
|
||||
this.#active = undefined;
|
||||
if (previous !== undefined) {
|
||||
const closed = await this.#retireClient(
|
||||
previous.client,
|
||||
previous.route.identity.id,
|
||||
);
|
||||
if (!closed.ok) {
|
||||
// A failed open retires the provider cache, not the investigation.
|
||||
// Preserve Evidence and Unknown history for export and recovery.
|
||||
this.clearSnapshot();
|
||||
this.#clearRuntimeAvailability();
|
||||
return closed;
|
||||
}
|
||||
}
|
||||
if (isAborted(options.signal))
|
||||
return err(
|
||||
await this.#restoreAfterFailedOpen(
|
||||
previous,
|
||||
new AnalysisCancelledError("open_binary"),
|
||||
),
|
||||
);
|
||||
const runId = randomUUID();
|
||||
const client = route.createClient(target, { runId });
|
||||
const started = await client.execute("health", {}, options);
|
||||
if (!started.ok) {
|
||||
return err(
|
||||
await this.#failedOpen(
|
||||
client,
|
||||
route.identity.id,
|
||||
previous,
|
||||
started.error,
|
||||
),
|
||||
);
|
||||
}
|
||||
if (isAborted(options.signal)) {
|
||||
return err(
|
||||
await this.#failedOpen(
|
||||
client,
|
||||
route.identity.id,
|
||||
previous,
|
||||
new AnalysisCancelledError("open_binary"),
|
||||
),
|
||||
);
|
||||
}
|
||||
this.#active = {
|
||||
return this.#serialize((admittedThrough) =>
|
||||
this.#openWithinTransition(admittedThrough, resolve, options),
|
||||
);
|
||||
}
|
||||
|
||||
async #openWithinTransition(
|
||||
admittedThrough: number,
|
||||
resolve: () => Promise<Result<ResolvedSessionOpen, AnalysisError>>,
|
||||
options: Pick<BinarySessionOpenOptions, "signal" | "snapshot">,
|
||||
): Promise<Result<BinaryTarget, AnalysisError>> {
|
||||
if (this.#externalMutationCleanupFailure?.cleanupIncomplete === true)
|
||||
return err(this.#externalMutationCleanupFailure);
|
||||
if (isAborted(options.signal))
|
||||
return err(new AnalysisCancelledError("open_binary"));
|
||||
const resolved = await resolve();
|
||||
if (!resolved.ok) return resolved;
|
||||
const { target, route, sameTarget } = resolved.value;
|
||||
const { profile } = route;
|
||||
if (isAborted(options.signal))
|
||||
return err(new AnalysisCancelledError("open_binary"));
|
||||
const activeProfile = this.#active?.profile;
|
||||
const sameProfile =
|
||||
activeProfile === null || activeProfile === undefined
|
||||
? profile === null
|
||||
: profile !== null && analysisProfilesEqual(activeProfile, profile);
|
||||
await this.#drainCalls(admittedThrough);
|
||||
if (isAborted(options.signal))
|
||||
return err(new AnalysisCancelledError("open_binary"));
|
||||
const pendingClosed = await this.#retryPendingCleanup();
|
||||
if (!pendingClosed.ok) return pendingClosed;
|
||||
if (isAborted(options.signal))
|
||||
return err(new AnalysisCancelledError("open_binary"));
|
||||
const retained = sameTarget && sameProfile ? this.#active : undefined;
|
||||
if (retained !== undefined) {
|
||||
const fastPath = await this.#tryRetainedFastPath(
|
||||
retained,
|
||||
target,
|
||||
client,
|
||||
profile,
|
||||
route,
|
||||
runId,
|
||||
nativeSourceAdmitted: false,
|
||||
};
|
||||
this.#clearRuntimeAvailability();
|
||||
this.resetSnapshotInvalidation();
|
||||
if (options.snapshot === undefined) {
|
||||
if (profile === null) this.clearSnapshot();
|
||||
else this.selectSnapshot(target, profile);
|
||||
options,
|
||||
);
|
||||
if (fastPath !== undefined) return fastPath;
|
||||
}
|
||||
const previous = this.#active;
|
||||
this.#active = undefined;
|
||||
if (previous !== undefined) {
|
||||
const closed = await this.#retireClient(
|
||||
previous.client,
|
||||
previous.route.identity.id,
|
||||
);
|
||||
if (!closed.ok) {
|
||||
// A failed open retires the provider cache, not the investigation.
|
||||
// Preserve Evidence and Unknown history for export and recovery.
|
||||
this.clearSnapshot();
|
||||
this.#clearRuntimeAvailability();
|
||||
return closed;
|
||||
}
|
||||
if (options.snapshot !== undefined) {
|
||||
const imported = this.importAnalysisSnapshot(options.snapshot);
|
||||
if (!imported.ok) {
|
||||
this.#active = undefined;
|
||||
return err(
|
||||
await this.#failedOpen(
|
||||
client,
|
||||
route.identity.id,
|
||||
previous,
|
||||
imported.error,
|
||||
),
|
||||
);
|
||||
}
|
||||
}
|
||||
if (isAborted(options.signal))
|
||||
return err(
|
||||
await this.#restoreAfterFailedOpen(
|
||||
previous,
|
||||
new AnalysisCancelledError("open_binary"),
|
||||
),
|
||||
);
|
||||
const runId = randomUUID();
|
||||
const client = route.createClient(target, { runId });
|
||||
const started = await client.execute("health", {}, options);
|
||||
if (!started.ok) {
|
||||
return err(
|
||||
await this.#failedOpen(
|
||||
client,
|
||||
route.identity.id,
|
||||
previous,
|
||||
started.error,
|
||||
),
|
||||
);
|
||||
}
|
||||
if (isAborted(options.signal)) {
|
||||
return err(
|
||||
await this.#failedOpen(
|
||||
client,
|
||||
route.identity.id,
|
||||
previous,
|
||||
new AnalysisCancelledError("open_binary"),
|
||||
),
|
||||
);
|
||||
}
|
||||
this.#active = {
|
||||
target,
|
||||
client,
|
||||
profile,
|
||||
route,
|
||||
runId,
|
||||
nativeSourceAdmitted: false,
|
||||
};
|
||||
this.#clearRuntimeAvailability();
|
||||
this.resetSnapshotInvalidation();
|
||||
if (options.snapshot === undefined) {
|
||||
if (profile === null) this.clearSnapshot();
|
||||
else this.selectSnapshot(target, profile);
|
||||
}
|
||||
if (options.snapshot !== undefined) {
|
||||
const imported = this.importAnalysisSnapshot(options.snapshot);
|
||||
if (!imported.ok) {
|
||||
this.#active = undefined;
|
||||
return err(
|
||||
await this.#failedOpen(
|
||||
client,
|
||||
route.identity.id,
|
||||
previous,
|
||||
imported.error,
|
||||
),
|
||||
);
|
||||
}
|
||||
return ok(target);
|
||||
}
|
||||
return ok(target);
|
||||
}
|
||||
|
||||
/** Serialize an external mutation and retire the potentially stale analysis. */
|
||||
withAdmittedExternalMutation<Value>(
|
||||
_path: string | undefined,
|
||||
signal: AbortSignal | undefined,
|
||||
operation: () => Promise<Result<Value, AnalysisError>>,
|
||||
retirementFailure: (
|
||||
value: Result<Value, AnalysisError>,
|
||||
error: AnalysisError,
|
||||
) => AnalysisError,
|
||||
operationName = "execute_rizin_command",
|
||||
cleanupRecovery?: (
|
||||
failure: AnalysisError | undefined,
|
||||
) => Promise<Result<null, AnalysisError>>,
|
||||
): Promise<Result<Result<Value, AnalysisError>, AnalysisError>> {
|
||||
return this.#serialize(async (admittedThrough) => {
|
||||
if (isAborted(signal))
|
||||
return err(new AnalysisCancelledError(operationName));
|
||||
await this.#drainCalls(admittedThrough);
|
||||
if (isAborted(signal))
|
||||
return err(new AnalysisCancelledError(operationName));
|
||||
|
||||
if (this.#externalMutationCleanupFailure?.cleanupIncomplete === true) {
|
||||
if (cleanupRecovery === undefined)
|
||||
return err(this.#externalMutationCleanupFailure);
|
||||
const recovered = await cleanupRecovery(
|
||||
this.#externalMutationCleanupFailure,
|
||||
);
|
||||
if (!recovered.ok) return recovered;
|
||||
if (this.#externalMutationCleanupFailure.cleanupIncomplete)
|
||||
return err(this.#externalMutationCleanupFailure);
|
||||
this.#externalMutationCleanupFailure = undefined;
|
||||
}
|
||||
|
||||
const pendingCleanup = await this.#retryPendingCleanup();
|
||||
if (!pendingCleanup.ok) return pendingCleanup;
|
||||
|
||||
const activeAtDispatch = this.#active;
|
||||
if (activeAtDispatch?.route.binding?.identity.id === "ghidra")
|
||||
return err(
|
||||
new AnalysisOutputError(
|
||||
operationName,
|
||||
"Close the active Ghidra session explicitly before an external mutation; rebinding would discard its ephemeral analysis edits.",
|
||||
),
|
||||
);
|
||||
// A protocol reply does not prove that background writers have stopped.
|
||||
// Retire the old analysis and require an explicit fresh open rather than
|
||||
// silently binding a snapshot while an unrestricted command can still write.
|
||||
const retireTarget = activeAtDispatch !== undefined;
|
||||
this.invalidateSnapshot();
|
||||
const value = await operation();
|
||||
this.#externalMutationCleanupFailure =
|
||||
!value.ok && value.error.cleanupIncomplete ? value.error : undefined;
|
||||
if (!retireTarget) return ok(value);
|
||||
|
||||
const retired = await this.#closeActive({}, false);
|
||||
if (!retired.ok) return err(retirementFailure(value, retired.error));
|
||||
return ok(value);
|
||||
});
|
||||
}
|
||||
|
||||
@@ -396,6 +474,7 @@ export class BinarySession
|
||||
options: Pick<ExecutionOptions, "progress"> & {
|
||||
readonly retainProviderDocuments?: boolean;
|
||||
},
|
||||
clearRecords = true,
|
||||
): Promise<Result<null, AnalysisError>> {
|
||||
const previous = this.#active;
|
||||
this.#active = undefined;
|
||||
@@ -417,7 +496,7 @@ export class BinarySession
|
||||
: {}),
|
||||
},
|
||||
);
|
||||
this.clearSessionRecords();
|
||||
if (clearRecords) this.clearSessionRecords();
|
||||
this.#clearRuntimeAvailability();
|
||||
const fileResources = await this.closeWriteResources();
|
||||
if (!closed.ok && !fileResources.ok)
|
||||
|
||||
@@ -48,6 +48,20 @@ export interface BinarySessionPort
|
||||
signal: AbortSignal | undefined,
|
||||
operation: (analysis: AnalysisOperationPort) => Promise<Value>,
|
||||
): Promise<Result<Value, AnalysisError>>;
|
||||
/** Serialize an external mutation and retire any active target afterward. */
|
||||
withAdmittedExternalMutation<Value>(
|
||||
_path: string | undefined,
|
||||
signal: AbortSignal | undefined,
|
||||
operation: () => Promise<Result<Value, AnalysisError>>,
|
||||
retirementFailure: (
|
||||
value: Result<Value, AnalysisError>,
|
||||
error: AnalysisError,
|
||||
) => AnalysisError,
|
||||
operationName?: string,
|
||||
cleanupRecovery?: (
|
||||
failure: AnalysisError | undefined,
|
||||
) => Promise<Result<null, AnalysisError>>,
|
||||
): Promise<Result<Result<Value, AnalysisError>, AnalysisError>>;
|
||||
open(
|
||||
path: string,
|
||||
options?: {
|
||||
@@ -82,6 +96,8 @@ export interface BinarySessionPort
|
||||
operation?: AnalysisOperation,
|
||||
): AnalysisProfileCommitment | undefined;
|
||||
allowsSnapshotReplay(operation: AnalysisOperation): boolean;
|
||||
/** Invalidate cached Evidence before an external command may mutate the target. */
|
||||
invalidateSnapshotForExternalMutation(): void;
|
||||
recordWorkflowSnapshot(
|
||||
input: WorkflowSnapshotRecordInput,
|
||||
): Result<null, EvidenceIntegrityError>;
|
||||
|
||||
Reference in New Issue
Block a user