diff --git a/AGENTS.md b/AGENTS.md index 6e031b9340..1f26ffbe24 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -20,6 +20,8 @@ Give each interpretation and resource a clear owner. Consumers must use that own Keep proof of ownership separate from validity, editability, and readiness. Malformed owned data may still be safely removable. Unresolved identity or applicability must remain unknown rather than be treated as absent, foreign, or aligned. +For lifecycle changes, follow [ownership and completion invariants](docs/tool-design.md#own-the-lifecycle) and select the applicable [lifecycle regressions](docs/testing.md#lifecycle-regressions). Include late handshakes, cancellation during final cleanup, mixed child outcomes, and cleanup retry where those stages exist; active-execution cancellation alone does not cover finalization. + Assess brittleness in touched code during implementation, debugging, review, and maintenance. Fix the producing representation or ownership rule before adding downstream repairs. Normalize once at the owning boundary using format-aware parsers. Remove superseded representations, aliases, wrappers, and redundant tests after verifying affected consumers. Introduce abstractions only when they remove observed complexity. When fixing a bug, inspect the producing boundary and affected consumers for duplicated interpretation or unclear ownership. Complete any directly related refactor in the same change and remove superseded code. Passing regression tests alone does not establish that the brittleness is resolved. If related cleanup remains, explain what remains and why. diff --git a/bridge/hopper_bridge.py b/bridge/hopper_bridge.py index 0425cc0d91..07a8adff51 100644 --- a/bridge/hopper_bridge.py +++ b/bridge/hopper_bridge.py @@ -1057,6 +1057,10 @@ def _serve_connection(connection): ): raise PermissionError("Invalid bridge capability") authenticated = True + # The connection grace period only bounds unauthenticated idle + # clients. Native analysis and a connected owner's idle time + # remain unbounded after capability validation. + connection.settimeout(None) _write_message(file, {"id": request_id, "event": { "type": "progress", "phase": "hopper_bridge", @@ -1148,13 +1152,17 @@ def _run(): server.bind(REA_SOCKET) os.chmod(REA_SOCKET, 0o600) server.listen(1) + # A bridge can start after its caller's startup/cleanup deadline. Bound + # the first accept as well as reconnects so an orphan cannot pin Hopper's + # dedicated Python thread and prevent later documents from starting REA. + server.settimeout(REA_RECONNECT_TIMEOUT_SECONDS) try: while True: try: connection, _ = server.accept() except socket.timeout: break - connection.settimeout(None) + connection.settimeout(REA_RECONNECT_TIMEOUT_SECONDS) if _serve_connection(connection): break # Match REA's cleanup handshake budget without pinning Hopper's diff --git a/bridge/native/rea_lldb_tracer.py b/bridge/native/rea_lldb_tracer.py index fcec3ea92b..5495b2fd59 100644 --- a/bridge/native/rea_lldb_tracer.py +++ b/bridge/native/rea_lldb_tracer.py @@ -555,6 +555,38 @@ def _failed_launch_description(process): return None +def _retire_failed_launch(process, config, listener, stdout, stderr): + """Reconcile an SBProcess returned alongside a failed Launch error.""" + if not process.IsValid(): + stdout.close() + stderr.close() + return False + + pid = process.GetProcessID() + if pid > 0: + # Even an already-exited process needs its PID published so the host + # can distinguish a completed launch failure from an unknown target. + with open(config["pid_path"], "w") as handle: + handle.write(str(pid)) + + if process.GetState() != lldb.eStateExited: + stdout.mark_launched() + stderr.mark_launched() + if pid > 0: + # Hold a surviving target until the host has captured its start + # identity. If that proof is unavailable, Kill still addresses + # this exact SBProcess created by the current launch request. + _wait_for_identity_ack(config) + process.Kill() + _wait_for_exit(listener, process, 5) + + exit_observed = process.GetState() == lldb.eStateExited + expected_exit = exit_observed + stdout.close(expected_exit=expected_exit) + stderr.close(expected_exit=expected_exit) + return exit_observed + + def trace(debugger, config): global _TRACE_CONTEXT debugger.SetAsync(True) @@ -630,8 +662,9 @@ def trace(debugger, config): ) process = target.Launch(_launch_info(target, config, listener), error) if not error.Success() or not process.IsValid(): - stdout.close() - stderr.close() + exit_observed = _retire_failed_launch( + process, config, listener, stdout, stderr + ) return { "status": "launch-error", "error": error.GetCString(), @@ -640,15 +673,26 @@ def trace(debugger, config): if process.IsValid() else {} ), + **( + {"target_process_exit_observed": True} + if exit_observed + else {} + ), } launch_failure = _failed_launch_description(process) if launch_failure is not None: - stdout.close(expected_exit=True) - stderr.close(expected_exit=True) + exit_observed = _retire_failed_launch( + process, config, listener, stdout, stderr + ) return { "status": "launch-error", "error": launch_failure, "target_process_created": True, + **( + {"target_process_exit_observed": True} + if exit_observed + else {} + ), } stdout.mark_launched() stderr.mark_launched() diff --git a/bridge/native/test_rea_lldb_tracer.py b/bridge/native/test_rea_lldb_tracer.py index 308ed00336..2fe616b1e4 100644 --- a/bridge/native/test_rea_lldb_tracer.py +++ b/bridge/native/test_rea_lldb_tracer.py @@ -15,6 +15,8 @@ fake_lldb = sys.modules.setdefault("lldb", types.ModuleType("lldb")) fake_lldb.LLDB_INVALID_ADDRESS = -1 fake_lldb.eLaunchFlagDebug = 2 fake_lldb.eLaunchFlagStopAtEntry = 4 +fake_lldb.eStateExited = 10 +fake_lldb.SBEvent = lambda: object() spec = importlib.util.spec_from_file_location( "rea_lldb_tracer", os.path.join(os.path.dirname(__file__), "rea_lldb_tracer.py") ) @@ -23,6 +25,136 @@ spec.loader.exec_module(tracer) class NativeTraceBudgetTests(unittest.TestCase): + def test_failed_launch_does_not_infer_exit_from_a_failed_kill(self): + class Process: + def IsValid(self): + return True + + def GetProcessID(self): + return 12345 + + def GetState(self): + return 6 + + def Kill(self): + return None + + class Sink: + close_exit = None + + def mark_launched(self): + pass + + def close(self, expected_exit=False): + self.close_exit = expected_exit + + with tempfile.TemporaryDirectory() as directory: + stdout, stderr = Sink(), Sink() + with patch.object(tracer, "_wait_for_identity_ack", return_value="unavailable"), patch.object( + tracer, "_wait_for_exit", return_value=None + ): + exited = tracer._retire_failed_launch( + Process(), {"pid_path": os.path.join(directory, "pid")}, object(), stdout, stderr + ) + self.assertFalse(exited) + self.assertFalse(stdout.close_exit) + self.assertFalse(stderr.close_exit) + + def test_failed_launch_publishes_pid_and_retires_live_process(self): + class Process: + state = 6 + killed = False + + def IsValid(self): + return True + + def GetProcessID(self): + return 12345 + + def GetState(self): + return self.state + + def Kill(self): + self.killed = True + self.state = tracer.lldb.eStateExited + + class Sink: + launched = False + close_exit = None + + def mark_launched(self): + self.launched = True + + def close(self, expected_exit=False): + self.close_exit = expected_exit + + with tempfile.TemporaryDirectory() as directory: + pid_path = os.path.join(directory, "pid") + process = Process() + stdout, stderr = Sink(), Sink() + + def acknowledge(config): + with open(config["pid_path"], "r") as pid_file: + self.assertEqual(pid_file.read(), "12345") + return "readable" + + with patch.object(tracer, "_wait_for_identity_ack", acknowledge): + exited = tracer._retire_failed_launch( + process, + {"pid_path": pid_path}, + object(), + stdout, + stderr, + ) + + self.assertTrue(exited) + self.assertTrue(process.killed) + self.assertTrue(stdout.launched) + self.assertTrue(stderr.launched) + self.assertTrue(stdout.close_exit) + self.assertTrue(stderr.close_exit) + + def test_failed_launch_records_when_valid_process_is_already_exited(self): + class Process: + def IsValid(self): + return True + + def GetProcessID(self): + return 12345 + + def GetState(self): + return tracer.lldb.eStateExited + + def Kill(self): + raise AssertionError("an exited process must not be killed") + + class Sink: + close_exit = None + + def mark_launched(self): + raise AssertionError("an exited process has no output to drain") + + def close(self, expected_exit=False): + self.close_exit = expected_exit + + with tempfile.TemporaryDirectory() as directory: + pid_path = os.path.join(directory, "pid") + stdout, stderr = Sink(), Sink() + with patch.object( + tracer, + "_wait_for_identity_ack", + side_effect=AssertionError("exit is already observed"), + ): + exited = tracer._retire_failed_launch( + Process(), {"pid_path": pid_path}, object(), stdout, stderr + ) + with open(pid_path, "r") as pid_file: + self.assertEqual(pid_file.read(), "12345") + + self.assertTrue(exited) + self.assertTrue(stdout.close_exit) + self.assertTrue(stderr.close_exit) + def test_pre_target_integrity_failure_reports_no_process_created(self): original_debugger = getattr(tracer.lldb, "SBDebugger", None) diff --git a/docs/cli.md b/docs/cli.md index b087b1b902..c0f8eefff4 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -168,6 +168,12 @@ be verified. See [MCP contracts](mcp-contracts.md) for session lifecycle and [Ghidra first-query deadlines](mcp-contracts.md#ghidra-first-query-deadlines-and-recovery) for import, client timeouts and recovery. +If one-shot analysis fails and closing its session also fails, the original +error details remain inline and `details.cleanup_error` describes the later +close failure. Each diagnostic retains its own reported resources; the later +failure does not establish whether an earlier resource was released. Captured +output and partial observations remain in the original details. + ## Save and reuse analysis snapshots A snapshot retains successful analysis results for later queries. REA reuses diff --git a/docs/installation.md b/docs/installation.md index ef36e5f96b..a8a13ac82c 100644 --- a/docs/installation.md +++ b/docs/installation.md @@ -635,6 +635,11 @@ bridge readiness. For large targets, set `REA_HOPPER_STARTUP_TIMEOUT_MS` to a decimal integer from 1 to 2,147,483,647 milliseconds in the environment that starts REA. CLI and MCP use the same setting; invalid values fail configuration validation. An MCP client's own request timeout must also allow enough time. +`open_binary` validates and selects a target without waiting for native provider +readiness. During Hopper startup, `binary_session.provider_operation_health` +reports `busy` at stage `launch`; `analysis_activity` describes bridge requests +and can remain idle until the socket is ready. Cleanup work reports `busy` at +stage `connection`. If macOS denies Apple Events, allow the application that starts REA under **System Settings → Privacy & Security → Automation**, then retry. REA releases @@ -644,9 +649,12 @@ Arguments containing quotes, backslashes, or line breaks remain unchanged but cannot establish that a failed handoff opened no document. Other failed handoffs retain ownership until document closure is confirmed. If a timed-out bridge becomes ready later, `close_binary` connects to that run, authenticates its -identity, and requests document shutdown. After a connection drops, the bridge -allows 30 seconds for an authenticated cleanup reconnect, then releases Hopper's -Python execution thread. Expiry does not establish document closure, so REA +identity, and requests document shutdown. Once its listener is ready, the bridge +allows 30 seconds for the first connection and capability validation. After a +connection drops, it allows the same grace period for an authenticated cleanup +reconnect, then releases Hopper's Python execution thread. Initial analysis is +outside this listener grace period, and authenticated native work is not bounded +by it. Expiry does not establish document closure, so REA retains unconfirmed ownership. Retry close while the bridge remains available. If a Linux launcher exists but cannot start, inspect its shared libraries: @@ -736,9 +744,17 @@ Once document and process closure are confirmed, the lease can be released even if temporary-file cleanup fails. Another client can then launch, but the owning client must finish its retained cleanup before starting again. An unconfirmed external document can be retried while its authenticated bridge -remains connected. If that bridge has disconnected, another close cannot confirm -the document: inspect and close the reported document in Hopper before ending -the owning REA connection. +remains available. The same authentication and process-identity capture apply +when the first successful handshake occurs during cleanup after a startup +timeout. REA can confirm closure if that exact authenticated process exits, +including while a shutdown request is in flight. A helper exit alone is +insufficient, and an application that never authenticated has no verified +process identity. If no matching bridge becomes available, inspect and close +the reported document in Hopper before ending the owning REA connection. +Export the Evidence bundle before `close_binary` clears session records. +`binary_session.cleanup_pending` remains true while provider cleanup is retained, +and provider health remains visible even with `open: false`; that flag alone +does not establish successful cleanup. ### Hopper in CI diff --git a/docs/mcp-contracts.md b/docs/mcp-contracts.md index 80606fb9ec..2d85ef857a 100644 --- a/docs/mcp-contracts.md +++ b/docs/mcp-contracts.md @@ -393,15 +393,25 @@ concrete version, selection source, and complete analysis profile through choices return typed selection details. A selected provider is never replaced automatically after a runtime failure. -Every successful target transition allocates `binary_session.analysis_run.run_id` -before any provider startup. `process_lineage` is `not_observed` until a dynamic -provider starts, then becomes `snapshots` with every started provider's identity +Each provider client receives `binary_session.analysis_run.run_id` before any +provider startup, including clients whose startup fails. `process_lineage` is +`not_observed` until a dynamic provider starts, then becomes `snapshots` with +every started provider's identity and retained ownership observation. Each observation records `observed_at` and is `unavailable` with a reason when ownership could not be revalidated, or `verified` with launcher PID, parent PID, process group, and descendants observed at that bounded check. These are historical snapshots, not live process inventories, and do not claim that no short-lived descendant existed. +`binary_session.cleanup_pending` reports whether the session retains a provider +client whose cleanup has not succeeded. It can be true with `open: false`; +target retirement does not establish resource release. Health, activity, and +lineage remain available under that client's original `analysis_run.run_id` +where the provider supplies them. Successful cleanup clears the retained run; +a replacement client receives a fresh run ID. Follow the cleanup error's +reported resources and recovery action before reopening; a missing health +observation does not establish an idle provider. + ## Progress and cancellation REA accepts ordinary `tools/call` progress tokens. The numeric `progress` counts @@ -428,6 +438,10 @@ Native call tracing and process capture retain available observations in `details.partial_observation` on failure, including when cleanup succeeds. The observation reports its partial coverage; cleanup details describe host state separately from the execution failure. +For these tools, cancellation observed during result finalization or owned +cleanup also returns typed cancellation with the available observations. +A normal target exit does not establish request completion. Incomplete cleanup +still takes precedence and retains its resource diagnostics. A process capture that fails during a finalization interval keeps every signal attempt and its delivery result in `details.partial_observation`, with `elapsed_ms` and `exit_code` null when no exit was observed. diff --git a/docs/native-investigation.md b/docs/native-investigation.md index dfb4de3567..ced5a6283a 100644 --- a/docs/native-investigation.md +++ b/docs/native-investigation.md @@ -451,6 +451,11 @@ The run ends when the process exits, `max_events` entries are recorded, Bounded termination reports partial coverage. REA checks process termination using a launch-time OS start identity; if ownership or cleanup cannot be verified, the result or error preserves that uncertainty. The result reports: +Launch errors keep the fact that LLDB created a process separate from whether +it remains alive. An exited `SBProcess` is reported as exited only after LLDB +confirms that state; unresolved ownership remains retained for cleanup retry. +Cancellation observed while output and Evidence are finalized returns partial +cancellation Evidence, even if the target has already exited. - the outcome, exit status and captured stdout/stderr (1 MiB kept per stream while excess bytes are drained and counted without growing capture files). @@ -481,6 +486,11 @@ with partial coverage. Available output prefixes remain inline; their byte counts are observed lower bounds when final drain counters are unavailable. The original failure classification and cleanup uncertainty remain separate. An interrupted journal row is ignored with an explicit limitation. +If the trace completed before cancellation or runtime-directory cleanup failed, +the retained observation also keeps its exit status, termination proof, launch +identity, breakpoint results and final stream counters. Those fields remain +absent when the interrupted producer did not report them; partial coverage alone +does not mean that every retained fact is incomplete. Only entries are observed: return values, floating-point and stack arguments, and inlined or `objc_direct` calls are not. The target runs with the current diff --git a/docs/testing.md b/docs/testing.md index a8817bfb3c..7ce92cec6e 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -199,6 +199,44 @@ Measure slow cases before pruning capacity coverage. Retain inputs beyond the formerly failing size/depth and distinguish cold runs, warm builds, coverage, and toolchain conditions when comparing timings. +## Lifecycle regressions + +For changes to resource acquisition, readiness, cancellation, or cleanup, select +the affected owners and applicable scenarios below. Include temporary snapshots +and one-shot commands. Use public CLI/MCP workflows where practical and a +controlled production seam for interleavings that cannot be reproduced reliably. + +| Scenario | Assertion and control | +| --------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | +| Target admitted; startup still pending | Status preserves the readiness distinction and observed startup activity; readiness requires the provider's documented proof. | +| First handshake is late, absent, invalid, or recovered during close | Initial and recovery paths retain the same authenticated identity facts. Foreign run/token controls cannot authorize cleanup; authenticated idle/native work survives the unauthenticated listener deadline. | +| Launch returns an error with a valid process handle | Cover already-exited and still-live owned processes. Verify observed exit separately from creation; failed termination or unavailable identity remains unresolved. | +| Cancellation arrives after workload exit, during result reads or temporary-root removal | Return typed cancellation with available partial observations and actual cleanup status. The non-aborted control succeeds; existing primary/cleanup failures keep their precedence. | +| Children have mixed health or cleanup outcomes | Missing peer health does not erase an observed failure or activity. Idle plus unknown stays unknown. Retry a failed child without closing a successful peer again, including after a close-option change. | +| Cleanup fails after success or primary failure | Public output retains the completed observation or primary diagnostic, remaining resources, and owner recovery action. Restore the denied resource and verify the owning cleanup retry. | +| Target exits without an acknowledgement, or transport disappears while it stays alive | Positive owned exit proof resolves target lifetime; other resources still need cleanup. Lost transport alone does not prove exit. Verify unrelated/attached targets survive and same-connection recovery works when supported. | + +Synchronize cancellation with the finalization boundary being tested; a fixed +delay after dispatch does not establish that phase. Use markers, barriers, or +owned cleanup seams while preserving real cleanup where feasible. Check server +settlement and retained resources even if the MCP SDK rejects the client promise +before the handler finishes. A cancellation during active execution does not +cover cancellation during final cleanup. + +For atomic writes, exercise cancellation on both sides of the documented commit +point and preserve any publication receipt. A late interruption does not prove +that no output was published. + +Representative regressions include +[process cleanup-time SIGINT](https://github.com/morluto/rea/blob/e78455338fe2d06c4cd03d59cbf33473d754998b/tests/boundary/process/processCaptureCancellationAdapters.test.ts), +[late authenticated cleanup](https://github.com/morluto/rea/blob/e78455338fe2d06c4cd03d59cbf33473d754998b/tests/boundary/providers/hopper/hopperClient.handshakeRecovery.test.ts), +[LLDB launch and finalization](https://github.com/morluto/rea/blob/e78455338fe2d06c4cd03d59cbf33473d754998b/tests/boundary/providers/native/lldbCallTracerFailures.test.ts), +[composed child ownership](https://github.com/morluto/rea/blob/e78455338fe2d06c4cd03d59cbf33473d754998b/src/application/binary/CompositeProvider.test.ts), +and [public CLI cleanup denial](https://github.com/morluto/rea/blob/e78455338fe2d06c4cd03d59cbf33473d754998b/tests/boundary/cli/wasmCleanupCli.test.ts). +These controlled probes do not replace the matching real-provider lane. Record +the base failure, passing control, exact host/provider, verified cleanup, and +unavailable coverage; a missing engine is not a pass. + ## Behavioral depths | Depth | Location | What it proves | diff --git a/docs/tool-design.md b/docs/tool-design.md index 7231301838..e28e6e6886 100644 --- a/docs/tool-design.md +++ b/docs/tool-design.md @@ -68,6 +68,45 @@ satisfy. A plan without execution does not establish runtime behavior. constraints. Report pagination, truncation, cancellation, and partial failure. An assumed agent budget is insufficient justification for a limit. +## Own the lifecycle + +For tools that acquire resources, identify the owner and the observations that +establish each transition. Include temporary files and one-shot commands, as +well as persistent engine sessions. Use the existing provider contracts; +providers need not expose identical state machines. + +- Keep target admission, provider readiness, active work, observed process exit, + and resource release separate. A valid process handle, successful launcher, + or selected target establishes only its own fact. Publish available identity + and exit observations even when launch fails; retain unknown ownership. +- Authenticate and capture identity through the same boundary on initial and + recovery handshakes. Bound abandoned unauthenticated listeners where the + protocol permits it; that deadline must not expire authenticated idle sessions + or long-running native work. Listener expiry does not prove target closure. +- For cancellable observations, carry cancellation through result consumption + and awaited cleanup. Before returning success, check the signal after the + last awaited finalization step. + Retain collected observations under typed cancellation. Preserve an existing + primary failure and let incomplete cleanup retain its own classification. + Client cancellation alone does not prove native work stopped. + For atomic publication, document the cancellation cutoff and preserve any + committed output's publication receipt. +- Release only owned resources. Attaching owns a connection or session unless + the provider contract explicitly establishes target ownership. A helper exit, + missing reply, or missing PID does not establish target exit. +- Retain successful child cleanup and retry only unresolved children, including + when close options change. Preserve observed child failure or activity when + another child's health is unavailable; idle plus missing coverage stays + unknown. +- Await one-shot cleanup without discarding the completed result or primary + diagnostic. Preserve partial observations, remaining resources, and the cleanup + owner's recovery instructions. CLI adapters should reuse + [`finishAnalysisCommand`](https://github.com/morluto/rea/blob/main/src/cli/finishAnalysisCommand.ts) where its contract + applies rather than duplicate close/finally handling. + +Exercise the applicable [lifecycle regression scenarios](testing.md#lifecycle-regressions) +through the producing owner and affected CLI/MCP consumers. + ## Implement and evaluate Put provider protocols/parsing in adapters and shared CLI/MCP workflows in the diff --git a/src/application/DirectAnalysisCleanup.ts b/src/application/DirectAnalysisCleanup.ts index 0a49f0d5f4..84adb05094 100644 --- a/src/application/DirectAnalysisCleanup.ts +++ b/src/application/DirectAnalysisCleanup.ts @@ -46,10 +46,13 @@ export const directAnalysisCleanupFailure = ( ...primary.data, code: "cleanup_incomplete", retryable: false, + remediation: cleanup.remediation, details: { + // Primary fields describe the original failure; the later close + // assessment, including its resources, belongs to cleanup_error. + resources: error.cleanupResources.slice(), ...primary.data.details, cleanup: "incomplete", - resources: error.cleanupResources.slice(), execution_failure: primary.data.details !== undefined && "execution_failure" in primary.data.details @@ -86,6 +89,7 @@ export const managedAnalysisCleanupFailure = ( "close_binary", { cause: error, + remediationAction: cleanup.remediation.action, cleanup: { reason: cleanup.message, resources: error.cleanupResources }, diagnostics: { cleanup_error: cleanup, diff --git a/src/application/binary/BinarySession.ts b/src/application/binary/BinarySession.ts index 151e36ac59..2494e511d4 100644 --- a/src/application/binary/BinarySession.ts +++ b/src/application/binary/BinarySession.ts @@ -55,12 +55,15 @@ const ENHANCED_OPERATIONS: ReadonlySet = new Set( ENHANCED_TOOL_CONTRACTS.map(({ name }) => name), ); -interface SessionBinding { - readonly target: BinaryTarget; +interface SessionRun { readonly client: AnalysisClient; - readonly profile: AnalysisProfileCommitment | null; readonly route: SessionProviderRoute; readonly runId: string; +} + +interface SessionBinding extends SessionRun { + readonly target: BinaryTarget; + readonly profile: AnalysisProfileCommitment | null; nativeSourceAdmitted: boolean; } @@ -79,8 +82,7 @@ export class BinarySession #externalMutationCleanupFailure: AnalysisError | undefined; #pendingCleanup: | { - readonly client: AnalysisClient; - readonly providerId: string; + readonly run: SessionRun; readonly retainDocument: boolean; } | undefined; @@ -318,10 +320,7 @@ export class BinarySession const previous = this.#active; this.#active = undefined; if (previous !== undefined) { - const closed = await this.#retireClient( - previous.client, - previous.route.identity.id, - ); + const closed = await this.#retireClient(previous); if (!closed.ok) { // A failed open retires the provider cache, not the investigation. // Preserve Evidence and Unknown history for export and recovery. @@ -339,22 +338,15 @@ export class BinarySession ); const runId = randomUUID(); const client = route.createClient(target, { runId }); + const run: SessionRun = { client, route, runId }; const started = await client.execute("health", {}, options); if (!started.ok) { - return err( - await this.#failedOpen( - client, - route.identity.id, - previous, - started.error, - ), - ); + return err(await this.#failedOpen(run, previous, started.error)); } if (isAborted(options.signal)) { return err( await this.#failedOpen( - client, - route.identity.id, + run, previous, new AnalysisCancelledError("open_binary"), ), @@ -362,10 +354,8 @@ export class BinarySession } this.#active = { target, - client, profile, - route, - runId, + ...run, nativeSourceAdmitted: false, }; this.#clearRuntimeAvailability(); @@ -378,14 +368,7 @@ export class BinarySession 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 err(await this.#failedOpen(run, previous, imported.error)); } } return ok(target); @@ -505,18 +488,14 @@ export class BinarySession options.progress, options.retainProviderDocuments, ) - : await this.#retireClient( - previous.client, - previous.route.identity.id, - { - ...(options.progress === undefined - ? {} - : { progress: options.progress }), - ...(options.retainProviderDocuments === true - ? { retainDocument: true } - : {}), - }, - ); + : await this.#retireClient(previous, { + ...(options.progress === undefined + ? {} + : { progress: options.progress }), + ...(options.retainProviderDocuments === true + ? { retainDocument: true } + : {}), + }); if (clearRecords) this.clearSessionRecords(); this.#clearRuntimeAvailability(); const fileResources = await this.closeWriteResources(); @@ -532,15 +511,13 @@ export class BinarySession } async #retireClient( - client: AnalysisClient, - providerId: string, + run: SessionRun, options: Pick & { readonly retainDocument?: boolean; } = {}, ): Promise> { this.#pendingCleanup = { - client, - providerId, + run, retainDocument: options.retainDocument === true, }; return this.#retryPendingCleanup(options.progress); @@ -557,8 +534,8 @@ export class BinarySession this.#pendingCleanup = pending; } const closed = await closeAnalysisClient( - pending.client, - pending.providerId, + pending.run.client, + pending.run.route.identity.id, { ...(progress === undefined ? {} : { progress }), ...(pending.retainDocument ? { retainDocument: true } : {}), @@ -569,12 +546,11 @@ export class BinarySession } async #failedOpen( - client: AnalysisClient, - providerId: string, + run: SessionRun, previous: SessionBinding | undefined, primary: AnalysisError, ): Promise { - const closed = await this.#retireClient(client, providerId); + const closed = await this.#retireClient(run); if (!closed.ok) return analysisErrorWithCleanupFailure( primary, @@ -596,18 +572,18 @@ export class BinarySession /** Describe the active binary session. */ status(): JsonValue { + const run = this.#active ?? this.#pendingCleanup?.run; + const client = run?.client; return binarySessionStatus({ target: this.#active?.target, route: this.#currentRoute(), router: this.#providerRouter, runtimeUnavailability: this.#runtimeUnavailability, - runId: this.#active?.runId, - runtimeLineageSnapshots: - this.#active?.client.runtimeLineageSnapshots?.() ?? [], - requestActivitySnapshots: - this.#active?.client.requestActivitySnapshots?.() ?? [], - providerOperationHealth: - this.#active?.client.operationHealthSnapshot?.() ?? null, + runId: run?.runId, + runtimeLineageSnapshots: client?.runtimeLineageSnapshots?.() ?? [], + requestActivitySnapshots: client?.requestActivitySnapshots?.() ?? [], + providerOperationHealth: client?.operationHealthSnapshot?.() ?? null, + cleanupPending: this.#pendingCleanup !== undefined, }); } @@ -943,20 +919,19 @@ export class BinarySession if (previous === undefined) return ok(null); const runId = randomUUID(); const client = previous.route.createClient(previous.target, { runId }); + const run: SessionRun = { client, route: previous.route, runId }; const started = await client.execute("health", {}); if (started.ok) { this.#active = { target: previous.target, - client, profile: previous.profile, - route: previous.route, - runId, + ...run, nativeSourceAdmitted: false, }; this.#clearRuntimeAvailability(); return ok(null); } - const closed = await this.#retireClient(client, previous.route.identity.id); + const closed = await this.#retireClient(run); return closed.ok ? ok(null) : err( diff --git a/src/application/binary/BinarySessionStatus.ts b/src/application/binary/BinarySessionStatus.ts index 15e0cfd32c..5234f8b7aa 100644 --- a/src/application/binary/BinarySessionStatus.ts +++ b/src/application/binary/BinarySessionStatus.ts @@ -30,6 +30,7 @@ interface BinarySessionStatusInput { readonly runtimeLineageSnapshots: readonly ProviderRuntimeLineageSnapshot[]; readonly requestActivitySnapshots: readonly ProviderRequestActivitySnapshot[]; readonly providerOperationHealth?: ProviderOperationHealth | null; + readonly cleanupPending?: boolean; } /** Project internal provider routing state into the caller-visible session status. */ @@ -42,6 +43,7 @@ export const binarySessionStatus = ({ runtimeLineageSnapshots, requestActivitySnapshots, providerOperationHealth, + cleanupPending = false, }: BinarySessionStatusInput): JsonValue => { const routeCapabilities = [...route.capabilities.values()] .filter(({ operation }) => operation !== "inventory_artifact") @@ -84,6 +86,7 @@ export const binarySessionStatus = ({ ), })); const common = { + cleanup_pending: cleanupPending, capabilities, analysis_provider_binding: analysisProviderBinding, analysis_provider_candidates: analysisProviderCandidates, diff --git a/src/application/binary/CompositeProvider.test.ts b/src/application/binary/CompositeProvider.test.ts index 4e94e08915..56334fa1c2 100644 --- a/src/application/binary/CompositeProvider.test.ts +++ b/src/application/binary/CompositeProvider.test.ts @@ -9,6 +9,7 @@ import { type ProviderIdentity, } from "../AnalysisProvider.js"; import { ProviderCleanupError } from "../../domain/providerCleanupError.js"; +import { projectAnalysisError } from "../../domain/analysisErrorProjection.js"; import { err, ok } from "../../domain/result.js"; import { CompositeProvider } from "./CompositeProvider.js"; @@ -44,10 +45,15 @@ const failingProvider = ( resource: string, ) => { const providerIdentity = identity(id); - const cleanupError = new ProviderCleanupError(id, [resource], { - reason: `${id} cleanup is unresolved`, - child_marker: `${id}-detail`, - }); + const cleanupError = new ProviderCleanupError( + id, + [resource], + { + reason: `${id} cleanup is unresolved`, + child_marker: `${id}-detail`, + }, + { remediationAction: `Resolve ${resource} before retrying ${id}.` }, + ); const close = vi.fn(async () => err(cleanupError)); const provider: AnalysisProvider = { identity: () => providerIdentity, @@ -98,6 +104,13 @@ it("retains every failed child resource across composite cleanup retries", async }, }); expect(closed.error).toBeInstanceOf(ProviderCleanupError); + const projected = projectAnalysisError(closed.error); + expect(projected.remediation.action).toContain( + "Resolve left-db before retrying provider-left.", + ); + expect(projected.remediation.action).toContain( + "Resolve right-db before retrying provider-right.", + ); expect(closed.error.diagnostics).toMatchObject({ failures: [ { @@ -127,3 +140,97 @@ it("preserves one child's typed cleanup error unchanged", async () => { if (closed.ok) throw new Error("Expected child cleanup failure"); expect(closed.error).toBe(child.cleanupError); }); + +it("does not close a child again after its cleanup succeeded", async () => { + const firstIdentity = identity("provider-first"); + const secondIdentity = identity("provider-second"); + let secondCloseAttempts = 0; + const firstClose = vi.fn(async () => ok(null)); + const secondClose = vi.fn(async () => { + secondCloseAttempts += 1; + return secondCloseAttempts === 1 + ? err(new ProviderCleanupError("provider-second", ["second-db"], {})) + : ok(null); + }); + const makeProvider = ( + providerIdentity: ProviderIdentity, + operation: Exclude, + close: AnalysisClient["close"], + ): AnalysisProvider => ({ + identity: () => providerIdentity, + capabilities: () => [operationCapability(providerIdentity, operation)], + createClient: () => ({ + execute: async () => ok(createAnalysisExecution(null, providerIdentity)), + close, + }), + }); + const client = new CompositeProvider([ + makeProvider(firstIdentity, "address_name", firstClose), + makeProvider(secondIdentity, "procedure_info", secondClose), + ]).createClient(executableTarget); + await client.execute("address_name", {}); + await client.execute("procedure_info", {}); + + expect((await client.close({ retainDocument: true })).ok).toBe(false); + expect((await client.close()).ok).toBe(true); + expect(firstClose).toHaveBeenCalledTimes(1); + expect(secondClose).toHaveBeenCalledTimes(2); + expect((await client.close({ retainDocument: true })).ok).toBe(true); + expect(firstClose).toHaveBeenCalledTimes(1); + expect(secondClose).toHaveBeenCalledTimes(2); +}); + +it.each(["busy", "unknown", "idle", undefined] as const)( + "keeps partial health truthful with child state %s and an unobserved peer", + async (state) => { + const observedIdentity = identity("provider-observed"); + const unobservedIdentity = identity("provider-unobserved"); + const observed: AnalysisProvider = { + identity: () => observedIdentity, + capabilities: () => [ + operationCapability(observedIdentity, "address_name"), + ], + createClient: () => ({ + execute: async () => + ok(createAnalysisExecution(null, observedIdentity)), + operationHealthSnapshot: () => + state === undefined + ? undefined + : { + state, + stage: state === "idle" ? null : "analysis", + retryAction: + state === "busy" + ? "wait" + : state === "idle" + ? null + : "unknown", + exitCode: null, + requests: [], + }, + close: async () => ok(null), + }), + }; + const unobserved: AnalysisProvider = { + identity: () => unobservedIdentity, + capabilities: () => [ + operationCapability(unobservedIdentity, "procedure_info"), + ], + createClient: () => ({ + execute: async () => + ok(createAnalysisExecution(null, unobservedIdentity)), + close: async () => ok(null), + }), + }; + const client = new CompositeProvider([observed, unobserved]).createClient( + executableTarget, + ); + await client.execute("address_name", {}); + await client.execute("procedure_info", {}); + + if (state === "idle" || state === undefined) + expect(client.operationHealthSnapshot?.()).toBeUndefined(); + else expect(client.operationHealthSnapshot?.()).toMatchObject({ state }); + expect((await client.close()).ok).toBe(true); + }, +); diff --git a/src/application/binary/CompositeProvider.ts b/src/application/binary/CompositeProvider.ts index faf3f87055..c7cac1333e 100644 --- a/src/application/binary/CompositeProvider.ts +++ b/src/application/binary/CompositeProvider.ts @@ -1,6 +1,7 @@ import { AnalysisCapabilityUnavailableError } from "../../domain/analysisErrorCore.js"; import { ProviderCleanupError } from "../../domain/providerCleanupError.js"; import { ProviderAdapterError } from "../../domain/providerAdapterError.js"; +import { projectAnalysisError } from "../../domain/analysisErrorProjection.js"; import { err, ok } from "../../domain/result.js"; import type { BinaryTarget } from "../../domain/binaryTargetTypes.js"; import type { AnalysisProfileCommitment } from "../../domain/analysisProfile.js"; @@ -27,18 +28,20 @@ const compositeProviderIdentity = ( version: null, }); -const closeCompositeClients = - ( - identity: ProviderIdentity, - clients: ReadonlyMap, - ): AnalysisClient["close"] => - async (options) => { +const closeCompositeClients = ( + identity: ProviderIdentity, + clients: ReadonlyMap, +): AnalysisClient["close"] => { + const completed = new Set(); + return async (options) => { const outcomes = await Promise.all( - [...clients.entries()].map(([provider, client]) => - closeAnalysisClient(client, provider.identity().id, options).then( - (result) => ({ providerId: provider.identity().id, result }), - ), - ), + [...clients.entries()].map(async ([provider, client]) => { + const providerId = provider.identity().id; + if (completed.has(client)) return { providerId, result: ok(null) }; + const result = await closeAnalysisClient(client, providerId, options); + if (result.ok) completed.add(client); + return { providerId, result }; + }), ); const failures = outcomes.flatMap(({ providerId, result }) => result.ok ? [] : [{ providerId, error: result.error }], @@ -68,6 +71,13 @@ const closeCompositeClients = })), }, { + remediationAction: [ + ...new Set( + failures.map( + ({ error }) => projectAnalysisError(error).remediation.action, + ), + ), + ].join("\n"), cause: new AggregateError( failures.map(({ error }) => error), "Composed provider cleanup failed.", @@ -76,6 +86,7 @@ const closeCompositeClients = ), ); }; +}; const executeCompositeHealth = async ( identity: ProviderIdentity, @@ -213,13 +224,21 @@ export class CompositeProvider implements AnalysisProvider { const snapshots = [...clients.values()].map((client) => client.operationHealthSnapshot?.(), ); + const failed = snapshots.find( + (snapshot) => + snapshot !== undefined && + (snapshot.state === "exited" || + snapshot.state === "unreachable" || + snapshot.state === "not_started"), + ); + if (failed !== undefined) return failed; + const nonIdle = snapshots.find( + (snapshot) => snapshot !== undefined && snapshot.state !== "idle", + ); + if (nonIdle !== undefined) return nonIdle; if (snapshots.some((snapshot) => snapshot === undefined)) return undefined; - return ( - snapshots.find( - (snapshot) => snapshot !== undefined && snapshot.state !== "idle", - ) ?? snapshots[0] - ); + return snapshots[0]; }, close, }; diff --git a/src/application/closeOwnedWebResources.ts b/src/application/closeOwnedWebResources.ts index 51b3e6f691..e6fbf428a8 100644 --- a/src/application/closeOwnedWebResources.ts +++ b/src/application/closeOwnedWebResources.ts @@ -1,3 +1,7 @@ +import { AnalysisError } from "../domain/analysisErrorBase.js"; +import { ProviderCleanupError } from "../domain/providerCleanupError.js"; +import { projectAnalysisError } from "../domain/analysisErrorProjection.js"; + /** Attempt every web workflow cleanup owner and report all failures. */ export const closeOwnedWebResources = async ( close: readonly ((() => Promise) | undefined)[], @@ -9,7 +13,55 @@ export const closeOwnedWebResources = async ( ), ); const failures = results.flatMap((result) => - result.status === "rejected" ? [result.reason] : [], + result.status === "rejected" ? [result.reason as unknown] : [], ); - if (failures.length > 0) throw new AggregateError(failures, message); + if (failures.length === 1) { + const [failure] = failures; + if (failure instanceof AnalysisError) throw failure; + throw new ProviderCleanupError( + "web-workflow", + [], + { reason: failure instanceof Error ? failure.message : String(failure) }, + { cause: failure, operation: "close_web_workflow" }, + ); + } + if (failures.length > 1) { + const normalized = failures.map((cause) => ({ + cause, + projection: + cause instanceof AnalysisError + ? projectAnalysisError(cause) + : undefined, + resources: cause instanceof AnalysisError ? cause.cleanupResources : [], + })); + const recoveryActions = [ + ...new Set( + normalized.flatMap(({ projection }) => + projection === undefined ? [] : [projection.remediation.action], + ), + ), + ]; + throw new ProviderCleanupError( + "web-workflow", + [...new Set(normalized.flatMap(({ resources }) => resources))], + { + failures: normalized.map( + ({ cause, projection }) => + projection ?? { + message: cause instanceof Error ? cause.message : String(cause), + }, + ), + }, + { + cause: new AggregateError( + normalized.map(({ cause }) => cause), + message, + ), + operation: "close_web_workflow", + ...(recoveryActions.length === 0 + ? {} + : { remediationAction: recoveryActions.join("; ") }), + }, + ); + } }; diff --git a/src/browser/WebArtifactCleanup.ts b/src/browser/WebArtifactCleanup.ts new file mode 100644 index 0000000000..d5243aea8c --- /dev/null +++ b/src/browser/WebArtifactCleanup.ts @@ -0,0 +1,75 @@ +import { ArtifactReaderFailure } from "../artifacts/ArtifactReader.js"; +import { AnalysisError } from "../domain/analysisErrorBase.js"; +import { projectAnalysisError } from "../domain/analysisErrorProjection.js"; +import { ProviderCleanupError } from "../domain/providerCleanupError.js"; + +export const webArtifactCleanupFailures = ( + owner: string, + operation: string, + failures: readonly { readonly resource: string; readonly cause: unknown }[], +): ProviderCleanupError | undefined => { + if (failures.length === 0) return undefined; + const normalized = failures.map(({ resource, cause }) => { + if (cause instanceof AnalysisError) { + const projection = projectAnalysisError(cause); + return { + cause, + projection, + observation: { + reason: projection.message, + resources: + cause.cleanupResources.length === 0 + ? [resource] + : cause.cleanupResources, + }, + }; + } + return { + cause, + projection: undefined, + observation: ArtifactReaderFailure.cleanupObservation(cause, resource), + }; + }); + const recoveryActions = [ + ...new Set( + normalized.flatMap(({ projection }) => + projection === undefined ? [] : [projection.remediation.action], + ), + ), + ]; + return new ProviderCleanupError( + owner, + [ + ...new Set( + normalized.flatMap(({ observation }) => observation.resources), + ), + ], + { + reason: normalized + .map(({ observation }) => observation.reason) + .join("; "), + failures: normalized.map(({ cause, projection, observation }) => + projection !== undefined + ? projection + : { + name: cause instanceof Error ? cause.name : "UnknownError", + message: cause instanceof Error ? cause.message : String(cause), + resource: [...observation.resources], + }, + ), + }, + { + cause: + normalized.length === 1 + ? normalized[0]!.cause + : new AggregateError( + normalized.map(({ cause }) => cause), + `${owner} cleanup failed`, + ), + operation, + ...(recoveryActions.length === 0 + ? {} + : { remediationAction: recoveryActions.join("; ") }), + }, + ); +}; diff --git a/src/browser/assets/WebSourceLocationArtifacts.ts b/src/browser/assets/WebSourceLocationArtifacts.ts index 7330694698..24c4d22ddf 100644 --- a/src/browser/assets/WebSourceLocationArtifacts.ts +++ b/src/browser/assets/WebSourceLocationArtifacts.ts @@ -8,6 +8,7 @@ import { type WebSourceLocationInput, } from "../../domain/webSourceLocation.js"; import { err, ok } from "../../domain/result.js"; +import { webArtifactCleanupFailures } from "../WebArtifactCleanup.js"; import { LocalWebScriptArtifacts } from "./LocalWebScriptArtifacts.js"; import { decodeWebArtifact, webArtifactReadError } from "./WebArtifactReads.js"; @@ -40,14 +41,23 @@ export class LocalWebSourceLocationArtifacts implements WebSourceLocationArtifac ? [] : [Promise.resolve().then(closeScript)]), ]); - const failures = results.flatMap((result) => - result.status === "rejected" ? [result.reason] : [], + const failures = results.flatMap((result, index) => + result.status === "rejected" + ? [ + { + cause: result.reason, + resource: + index === 0 ? "web source-location artifacts" : "web scripts", + }, + ] + : [], ); - if (failures.length > 0) - throw new AggregateError( - failures, - "Web source-location artifact cleanup failed", - ); + const error = webArtifactCleanupFailures( + "web-source-location-artifacts", + "trace_web_source_location", + failures, + ); + if (error !== undefined) throw error; } /** Preserve actual map identity separately from the selected deployment URL. */ diff --git a/src/browser/modules/WebModuleArtifacts.ts b/src/browser/modules/WebModuleArtifacts.ts index 928025f090..61cd261e9f 100644 --- a/src/browser/modules/WebModuleArtifacts.ts +++ b/src/browser/modules/WebModuleArtifacts.ts @@ -7,6 +7,7 @@ import type { ExecutionOptions } from "../../application/AnalysisProvider.js"; import { ArtifactResourceScope } from "../../artifacts/ArtifactResourceScope.js"; import { jsonObjectSchema } from "../../domain/jsonValue.js"; import { err, ok } from "../../domain/result.js"; +import { webArtifactCleanupFailures } from "../WebArtifactCleanup.js"; import type { WebModuleTraceInput } from "../../domain/webModuleTrace.js"; import { LocalWebScriptArtifacts } from "../assets/LocalWebScriptArtifacts.js"; import { @@ -43,11 +44,22 @@ export class LocalWebModuleArtifacts implements WebModuleArtifactPort { ? [] : [Promise.resolve().then(closeScript)]), ]); - const failures = results.flatMap((result) => - result.status === "rejected" ? [result.reason] : [], + const failures = results.flatMap((result, index) => + result.status === "rejected" + ? [ + { + cause: result.reason, + resource: index === 0 ? "web module artifacts" : "web scripts", + }, + ] + : [], ); - if (failures.length > 0) - throw new AggregateError(failures, "Web module artifact cleanup failed"); + const error = webArtifactCleanupFailures( + "web-module-artifacts", + "trace_web_module_imports", + failures, + ); + if (error !== undefined) throw error; } /** Read selected source and optional map without website requests. */ diff --git a/src/cli/adbCommands.ts b/src/cli/adbCommands.ts index 34b777e8a0..6f5d5d9eb0 100644 --- a/src/cli/adbCommands.ts +++ b/src/cli/adbCommands.ts @@ -1,16 +1,15 @@ import { z } from "incur"; -import { AnalysisError } from "../domain/analysisErrorBase.js"; import { AnalysisInputError } from "../domain/analysisErrorCore.js"; import { AdbDeviceAnalysisService } from "../application/adb/AdbDeviceAnalysisService.js"; import type { AdbDeviceAnalysisPort } from "../application/adb/AdbDeviceAnalysisPort.js"; import { createAdbDeviceAnalysisProvider } from "../composition/adb.js"; import { CLI_COMMANDS } from "../cliCommandNames.js"; import { logCliCommand } from "../cliLogging.js"; -import { projectAnalysisError } from "../domain/analysisErrorProjection.js"; import type { AdbOperation } from "../domain/adb/adbDeviceAnalysis.js"; import type { Logger } from "pino"; import type { CliInstance } from "./types.js"; import { withCommandCancellation } from "./commandCancellation.js"; +import { finishAnalysisCommand } from "./finishAnalysisCommand.js"; /** Expose the same explicit ADB requests through one-shot CLI commands. */ export const registerAdbCommands = ( @@ -24,17 +23,14 @@ export const registerAdbCommands = ( const service = new AdbDeviceAnalysisService(provider); const execute = (name: string, operation: AdbOperation, input: unknown) => withCommandCancellation((signal) => - logCliCommand(logger, name, async () => { - const result = await service.execute(operation, input, { signal }); - try { - await provider.close(); - } catch (cause) { - if (cause instanceof AnalysisError) - return projectAnalysisError(cause); - throw cause; - } - return result.ok ? result.value : projectAnalysisError(result.error); - }), + logCliCommand(logger, name, () => + finishAnalysisCommand( + () => service.execute(operation, input, { signal }), + { close: () => provider.close() }, + "adb", + operation, + ), + ), ); const serial = z diff --git a/src/cli/androidCommands.ts b/src/cli/androidCommands.ts index 540dd295dd..99cbee0b46 100644 --- a/src/cli/androidCommands.ts +++ b/src/cli/androidCommands.ts @@ -1,14 +1,13 @@ import { z } from "incur"; -import { AnalysisError } from "../domain/analysisErrorBase.js"; import { AndroidAnalysisService } from "../application/android/AndroidAnalysisService.js"; import { createAndroidAnalysisProvider } from "../composition/android.js"; import { CLI_COMMANDS } from "../cliCommandNames.js"; import { logCliCommand } from "../cliLogging.js"; -import { projectAnalysisError } from "../domain/analysisErrorProjection.js"; import type { AndroidOperation } from "../domain/android/androidAnalysis.js"; import type { Logger } from "pino"; import type { CliInstance } from "./types.js"; import { withCommandCancellation } from "./commandCancellation.js"; +import { finishAnalysisCommand } from "./finishAnalysisCommand.js"; /** Expose the same explicit APK requests through one-shot CLI commands. */ export const registerAndroidCommands = ( @@ -20,17 +19,14 @@ export const registerAndroidCommands = ( const service = new AndroidAnalysisService(provider); const execute = (name: string, operation: AndroidOperation, input: unknown) => withCommandCancellation((signal) => - logCliCommand(logger, name, async () => { - const result = await service.execute(operation, input, { signal }); - try { - await provider.close(); - } catch (cause) { - if (cause instanceof AnalysisError) - return projectAnalysisError(cause); - throw cause; - } - return result.ok ? result.value : projectAnalysisError(result.error); - }), + logCliCommand(logger, name, () => + finishAnalysisCommand( + () => service.execute(operation, input, { signal }), + { close: () => provider.close() }, + "jadx", + operation, + ), + ), ); const path = z.string().describe("Local standalone APK path"); const className = z.string().describe("Exact fully qualified class name"); diff --git a/src/cli/apktoolCommands.ts b/src/cli/apktoolCommands.ts index 217889f5af..6bd6f755b2 100644 --- a/src/cli/apktoolCommands.ts +++ b/src/cli/apktoolCommands.ts @@ -1,11 +1,9 @@ import { z } from "incur"; -import { AnalysisError } from "../domain/analysisErrorBase.js"; import { ApktoolResourceAnalysisService } from "../application/apktool/ApktoolResourceAnalysisService.js"; import type { ApktoolResourceAnalysisPort } from "../application/apktool/ApktoolResourceAnalysisPort.js"; import { createApktoolResourceAnalysisProvider } from "../composition/apktool.js"; import { CLI_COMMANDS } from "../cliCommandNames.js"; import { logCliCommand } from "../cliLogging.js"; -import { projectAnalysisError } from "../domain/analysisErrorProjection.js"; import { apktoolInputSchemas, type ApktoolOperation, @@ -13,6 +11,7 @@ import { import type { Logger } from "pino"; import type { CliInstance } from "./types.js"; import { withCommandCancellation } from "./commandCancellation.js"; +import { finishAnalysisCommand } from "./finishAnalysisCommand.js"; /** Expose the same explicit Apktool requests through one-shot CLI commands. */ export const registerApktoolCommands = ( @@ -26,17 +25,14 @@ export const registerApktoolCommands = ( const service = new ApktoolResourceAnalysisService(provider); const execute = (name: string, operation: ApktoolOperation, input: unknown) => withCommandCancellation((signal) => - logCliCommand(logger, name, async () => { - const result = await service.execute(operation, input, { signal }); - try { - await provider.close(); - } catch (cause) { - if (cause instanceof AnalysisError) - return projectAnalysisError(cause); - throw cause; - } - return result.ok ? result.value : projectAnalysisError(result.error); - }), + logCliCommand(logger, name, () => + finishAnalysisCommand( + () => service.execute(operation, input, { signal }), + { close: () => provider.close() }, + "apktool", + operation, + ), + ), ); cli.command(CLI_COMMANDS.inspectApktoolClient, { diff --git a/src/cli/evmCommands.ts b/src/cli/evmCommands.ts index 7d18066c99..b2872eea5d 100644 --- a/src/cli/evmCommands.ts +++ b/src/cli/evmCommands.ts @@ -1,11 +1,11 @@ import { z } from "incur"; import { evmInterfaceModule } from "../composition/evm.js"; import { logCliCommand } from "../cliLogging.js"; -import { projectAnalysisError } from "../domain/analysisErrorProjection.js"; import type { Logger } from "pino"; import type { CliInstance } from "./types.js"; import { withCommandCancellation } from "./commandCancellation.js"; import { absoluteCallerPath } from "../filesystem/callerPath.js"; +import { finishAnalysisCommand } from "./finishAnalysisCommand.js"; /** Resolve operator paths and preserve explicit encoding through the shared workflow. */ export const registerEvmCommands = ( @@ -23,27 +23,22 @@ export const registerEvmCommands = ( .describe("Selected carrier representation"), }), run: ({ args }) => - withCommandCancellation(async (signal) => { - try { - return await logCliCommand( - logger, - evmInterfaceModule.cliCommand, - async () => { - const result = await service.inspect( + withCommandCancellation((signal) => + logCliCommand(logger, evmInterfaceModule.cliCommand, () => + finishAnalysisCommand( + () => + service.inspect( { path: absoluteCallerPath(args.path), encoding: args.encoding, }, { signal }, - ); - return result.ok - ? result.value - : projectAnalysisError(result.error); - }, - ); - } finally { - await service.close(); - } - }), + ), + service, + "evmole-interface", + "inspect_evm_interface", + ), + ), + ), }); }; diff --git a/src/cli/firmwareCommands.ts b/src/cli/firmwareCommands.ts index ff584c0258..a19307a08c 100644 --- a/src/cli/firmwareCommands.ts +++ b/src/cli/firmwareCommands.ts @@ -5,12 +5,12 @@ import { CLI_COMMANDS } from "../cliCommandNames.js"; import { logCliCommand } from "../cliLogging.js"; import { firmwareInputSchemas } from "../domain/firmware/firmwareAnalysis.js"; import { localPathStringSchema } from "../domain/localPath.js"; -import { projectAnalysisError } from "../domain/analysisErrorProjection.js"; import type { FirmwareOperation } from "../domain/firmware/firmwareAnalysis.js"; import type { Logger } from "pino"; import type { CliInstance } from "./types.js"; import { withCommandCancellation } from "./commandCancellation.js"; import { absoluteCallerPath } from "../filesystem/callerPath.js"; +import { finishAnalysisCommand } from "./finishAnalysisCommand.js"; /** One-shot firmware CLI commands backed by the same workflows as MCP. */ export const registerFirmwareCommands = ( @@ -29,14 +29,14 @@ export const registerFirmwareCommands = ( input: unknown, ) => withCommandCancellation((signal) => - logCliCommand(logger, name, async () => { - try { - const result = await service.execute(operation, input, { signal }); - return result.ok ? result.value : projectAnalysisError(result.error); - } finally { - await service.close(); - } - }), + logCliCommand(logger, name, () => + finishAnalysisCommand( + () => service.execute(operation, input, { signal }), + service, + operation === "inspect_firmware_regions" ? "binwalk" : "unblob", + operation, + ), + ), ); const path = localPathStringSchema.describe( "Local firmware path; never executed", diff --git a/src/cli/flutterCommands.ts b/src/cli/flutterCommands.ts index a1be8e6487..03c72d260d 100644 --- a/src/cli/flutterCommands.ts +++ b/src/cli/flutterCommands.ts @@ -1,15 +1,14 @@ import { z } from "incur"; -import { AnalysisError } from "../domain/analysisErrorBase.js"; import { FlutterBuildAnalysisService } from "../application/flutter/FlutterBuildAnalysisService.js"; import type { FlutterBuildAnalysisPort } from "../application/flutter/FlutterBuildAnalysisPort.js"; import { createFlutterBuildAnalysisProvider } from "../composition/flutter.js"; import { CLI_COMMANDS } from "../cliCommandNames.js"; import { logCliCommand } from "../cliLogging.js"; -import { projectAnalysisError } from "../domain/analysisErrorProjection.js"; import type { FlutterOperation } from "../domain/flutter/flutterBuildAnalysis.js"; import type { Logger } from "pino"; import type { CliInstance } from "./types.js"; import { withCommandCancellation } from "./commandCancellation.js"; +import { finishAnalysisCommand } from "./finishAnalysisCommand.js"; /** One flutter command: CLI shape and the request it produces. */ interface FlutterCommandSpec { @@ -74,23 +73,19 @@ export const registerFlutterCommands = ( : { args: spec.argumentSchema, options: spec.optionSchema }), run: ({ args, options }) => withCommandCancellation((signal) => - logCliCommand(logger, spec.command, async () => { - const settled = await new FlutterBuildAnalysisService( - provider, - ).execute(spec.operation, spec.toInput(args, options ?? {}), { - signal, - }); - try { - await provider.close(); - } catch (cause) { - if (cause instanceof AnalysisError) - return projectAnalysisError(cause); - throw cause; - } - return settled.ok - ? settled.value - : projectAnalysisError(settled.error); - }), + logCliCommand(logger, spec.command, () => + finishAnalysisCommand( + () => + new FlutterBuildAnalysisService(provider).execute( + spec.operation, + spec.toInput(args, options ?? {}), + { signal }, + ), + { close: () => provider.close() }, + "flutter", + spec.operation, + ), + ), ), }); } diff --git a/src/cli/javascriptRecoveryCommands.ts b/src/cli/javascriptRecoveryCommands.ts index 7e13ba081f..0920ccb13c 100644 --- a/src/cli/javascriptRecoveryCommands.ts +++ b/src/cli/javascriptRecoveryCommands.ts @@ -3,10 +3,10 @@ import { JavaScriptRecoveryService } from "../application/javascript/JavaScriptR import { createJavaScriptRecoveryProvider } from "../composition/javascriptRecovery.js"; import { CLI_COMMANDS } from "../cliCommandNames.js"; import { logCliCommand } from "../cliLogging.js"; -import { projectAnalysisError } from "../domain/analysisErrorProjection.js"; import type { Logger } from "pino"; import type { CliInstance } from "./types.js"; import { withCommandCancellation } from "./commandCancellation.js"; +import { finishAnalysisCommand } from "./finishAnalysisCommand.js"; /** Share source recovery with MCP while keeping CLI spelling at this boundary. */ export const registerJavaScriptRecoveryCommands = ( @@ -47,13 +47,11 @@ export const registerJavaScriptRecoveryCommands = ( ), }), run: ({ args, options }) => - withCommandCancellation(async (signal) => { - try { - return await logCliCommand( - logger, - CLI_COMMANDS.recoverJavaScriptSources, - async () => { - const result = await service.recover( + withCommandCancellation((signal) => + logCliCommand(logger, CLI_COMMANDS.recoverJavaScriptSources, () => + finishAnalysisCommand( + () => + service.recover( { path: args.path, output_directory: args.outputDirectory, @@ -68,15 +66,12 @@ export const registerJavaScriptRecoveryCommands = ( : { rewrite_level: options.rewriteLevel }), }, { signal }, - ); - return result.ok - ? result.value - : projectAnalysisError(result.error); - }, - ); - } finally { - await service.close(); - } - }), + ), + service, + "wakaru", + "recover_javascript_sources", + ), + ), + ), }); }; diff --git a/src/cli/jebCommands.ts b/src/cli/jebCommands.ts index 9ed78b9070..01ba546074 100644 --- a/src/cli/jebCommands.ts +++ b/src/cli/jebCommands.ts @@ -1,15 +1,14 @@ import { z } from "incur"; -import { AnalysisError } from "../domain/analysisErrorBase.js"; import { JebAnalysisService } from "../application/jeb/JebAnalysisService.js"; import type { JebAnalysisPort } from "../application/jeb/JebAnalysisPort.js"; import { createJebAnalysisProvider } from "../composition/jeb.js"; import { CLI_COMMANDS } from "../cliCommandNames.js"; import { logCliCommand } from "../cliLogging.js"; -import { projectAnalysisError } from "../domain/analysisErrorProjection.js"; import type { JebOperation } from "../domain/jeb/jebAnalysis.js"; import type { Logger } from "pino"; import type { CliInstance } from "./types.js"; import { withCommandCancellation } from "./commandCancellation.js"; +import { finishAnalysisCommand } from "./finishAnalysisCommand.js"; /** Expose the same explicit JEB requests through one-shot CLI commands. */ export const registerJebCommands = ( @@ -21,17 +20,14 @@ export const registerJebCommands = ( const service = new JebAnalysisService(provider); const execute = (name: string, operation: JebOperation, input: unknown) => withCommandCancellation((signal) => - logCliCommand(logger, name, async () => { - const result = await service.execute(operation, input, { signal }); - try { - await provider.close(); - } catch (cause) { - if (cause instanceof AnalysisError) - return projectAnalysisError(cause); - throw cause; - } - return result.ok ? result.value : projectAnalysisError(result.error); - }), + logCliCommand(logger, name, () => + finishAnalysisCommand( + () => service.execute(operation, input, { signal }), + { close: () => provider.close() }, + "jeb", + operation, + ), + ), ); cli.command(CLI_COMMANDS.inspectJebClient, { diff --git a/src/cli/pcapCommands.ts b/src/cli/pcapCommands.ts index 1b53e47cc8..e59adc691d 100644 --- a/src/cli/pcapCommands.ts +++ b/src/cli/pcapCommands.ts @@ -1,16 +1,14 @@ import { z } from "incur"; -import { AnalysisError } from "../domain/analysisErrorBase.js"; -import { analysisErrorWithCleanupFailure } from "../domain/analysisErrorCleanup.js"; import { PcapAnalysisService } from "../application/pcap/PcapAnalysisService.js"; import type { PcapAnalysisPort } from "../application/pcap/PcapAnalysisPort.js"; import { createPcapAnalysisProvider } from "../composition/pcap.js"; import { CLI_COMMANDS } from "../cliCommandNames.js"; import { logCliCommand } from "../cliLogging.js"; -import { projectAnalysisError } from "../domain/analysisErrorProjection.js"; import type { PcapOperation } from "../domain/pcap/pcapAnalysis.js"; import type { Logger } from "pino"; import type { CliInstance } from "./types.js"; import { withCommandCancellation } from "./commandCancellation.js"; +import { finishAnalysisCommand } from "./finishAnalysisCommand.js"; /** Expose the same explicit pcap requests through one-shot CLI commands. */ export const registerPcapCommands = ( @@ -22,27 +20,14 @@ export const registerPcapCommands = ( const service = new PcapAnalysisService(provider); const execute = (name: string, operation: PcapOperation, input: unknown) => withCommandCancellation((signal) => - logCliCommand(logger, name, async () => { - const result = await service.execute(operation, input, { signal }); - try { - await provider.close(); - } catch (cause) { - if (cause instanceof AnalysisError) { - if (result.ok) cause.retainPartialObservation(result.value); - return projectAnalysisError( - result.ok - ? cause - : analysisErrorWithCleanupFailure( - result.error, - cause, - operation, - ), - ); - } - throw cause; - } - return result.ok ? result.value : projectAnalysisError(result.error); - }), + logCliCommand(logger, name, () => + finishAnalysisCommand( + () => service.execute(operation, input, { signal }), + { close: () => provider.close() }, + "pcap", + operation, + ), + ), ); cli.command(CLI_COMMANDS.inspectPcapClient, { diff --git a/src/cli/sqliteCommands.ts b/src/cli/sqliteCommands.ts index 39e5165f98..582fce271f 100644 --- a/src/cli/sqliteCommands.ts +++ b/src/cli/sqliteCommands.ts @@ -2,13 +2,11 @@ import { z } from "incur"; import { createSqliteDatabaseService } from "../composition/sqlite.js"; import { CLI_COMMANDS } from "../cliCommandNames.js"; import { logCliCommand } from "../cliLogging.js"; -import { AnalysisError } from "../domain/analysisErrorBase.js"; -import { analysisErrorWithCleanupFailure } from "../domain/analysisErrorCleanup.js"; -import { projectAnalysisError } from "../domain/analysisErrorProjection.js"; import type { Logger } from "pino"; import type { CliInstance } from "./types.js"; import { withCommandCancellation } from "./commandCancellation.js"; import { absoluteCallerPath } from "../filesystem/callerPath.js"; +import { finishAnalysisCommand } from "./finishAnalysisCommand.js"; /** Resolve an operator-selected SQLite snapshot through the shared inspection workflow. */ export const registerSqliteCommands = ( @@ -51,61 +49,32 @@ export const registerSqliteCommands = ( }), run: ({ args, options }) => withCommandCancellation((signal) => - logCliCommand(logger, CLI_COMMANDS.inspectSqliteDatabase, async () => { - const inspection = await service - .inspect( - { - path: absoluteCallerPath(args.path), - ...(options.table === undefined - ? {} - : { table: options.table }), - ...(options.rowLimit === undefined - ? {} - : { row_limit: options.rowLimit }), - ...(options.column === undefined - ? {} - : { columns: options.column }), - ...(options.schemaScope === undefined - ? {} - : { schema_scope: options.schemaScope }), - }, - { signal }, - ) - .then( - (result) => ({ result }), - (cause: unknown) => ({ cause }), - ); - try { - await service.close(); - } catch (cause: unknown) { - const primary = - "cause" in inspection - ? inspection.cause - : inspection.result.ok - ? undefined - : inspection.result.error; - if ( - primary instanceof AnalysisError && - cause instanceof AnalysisError - ) - throw analysisErrorWithCleanupFailure( - primary, - cause, - "inspect_sqlite_database", - ); - if ("cause" in inspection) - throw new AggregateError( - [inspection.cause, cause], - "SQLite inspection and provider cleanup both failed", - { cause: inspection.cause }, - ); - throw cause; - } - if ("cause" in inspection) throw inspection.cause; - return inspection.result.ok - ? inspection.result.value - : projectAnalysisError(inspection.result.error); - }), + logCliCommand(logger, CLI_COMMANDS.inspectSqliteDatabase, () => + finishAnalysisCommand( + () => + service.inspect( + { + path: absoluteCallerPath(args.path), + ...(options.table === undefined + ? {} + : { table: options.table }), + ...(options.rowLimit === undefined + ? {} + : { row_limit: options.rowLimit }), + ...(options.column === undefined + ? {} + : { columns: options.column }), + ...(options.schemaScope === undefined + ? {} + : { schema_scope: options.schemaScope }), + }, + { signal }, + ), + service, + "sqlite", + "inspect_sqlite_database", + ), + ), ), }); }; diff --git a/src/cli/wasmCommands.ts b/src/cli/wasmCommands.ts index db2c3d4321..681442b9d9 100644 --- a/src/cli/wasmCommands.ts +++ b/src/cli/wasmCommands.ts @@ -9,6 +9,7 @@ import type { Logger } from "pino"; import type { CliInstance } from "./types.js"; import { withCommandCancellation } from "./commandCancellation.js"; import { absoluteCallerPath } from "../filesystem/callerPath.js"; +import { finishAnalysisCommand } from "./finishAnalysisCommand.js"; /** Resolve all caller-selected artifact paths through the shared workflow. */ export const registerWasmCommands = ( @@ -56,13 +57,11 @@ export const registerWasmCommands = ( .describe("Explicit local WASM candidates"), }), run: ({ args, options }) => - withCommandCancellation(async (signal) => { - try { - return await logCliCommand( - logger, - wasmArtifactModule.cliCommand, - async () => { - const result = await service.inspect( + withCommandCancellation((signal) => + logCliCommand(logger, wasmArtifactModule.cliCommand, () => + finishAnalysisCommand( + () => + service.inspect( { path: absoluteCallerPath(args.path), glue_paths: options.glue_paths?.map((path) => @@ -73,15 +72,12 @@ export const registerWasmCommands = ( ), }, { signal }, - ); - return result.ok - ? result.value - : projectAnalysisError(result.error); - }, - ); - } finally { - await service.close(); - } - }), + ), + service, + "wabt-artifact", + "inspect_wasm_artifact", + ), + ), + ), }); }; diff --git a/src/cli/webModuleCommands.ts b/src/cli/webModuleCommands.ts index 837a4a7bdd..a7795aa95d 100644 --- a/src/cli/webModuleCommands.ts +++ b/src/cli/webModuleCommands.ts @@ -2,12 +2,12 @@ import { z } from "incur"; import { createWebModuleTraceService } from "../composition/webModules.js"; import { CLI_COMMANDS } from "../cliCommandNames.js"; import { logCliCommand } from "../cliLogging.js"; -import { projectAnalysisError } from "../domain/analysisErrorProjection.js"; import { webModuleTraceInputSchema } from "../domain/webModuleTrace.js"; import type { Logger } from "pino"; import type { CliInstance } from "./types.js"; import { cliIntegerArgument } from "./options.js"; import { withCommandCancellation } from "./commandCancellation.js"; +import { finishAnalysisCommand } from "./finishAnalysisCommand.js"; const importMapInputSchema = webModuleTraceInputSchema.shape.import_map.unwrap(); @@ -40,13 +40,11 @@ export const registerWebModuleCommands = ( .describe("HTTP(S) import-map base URL; requires --import-map-path"), }), run: ({ args, options }) => - withCommandCancellation(async (signal) => { - try { - return await logCliCommand( - logger, - CLI_COMMANDS.traceWebModuleImports, - async () => { - const result = await service.trace( + withCommandCancellation((signal) => + logCliCommand(logger, CLI_COMMANDS.traceWebModuleImports, () => + finishAnalysisCommand( + () => + service.trace( { manifest_path: args.manifestPath, script_index: args.scriptIndex, @@ -64,15 +62,12 @@ export const registerWebModuleCommands = ( }), }, { signal }, - ); - return result.ok - ? result.value - : projectAnalysisError(result.error); - }, - ); - } finally { - await service.close(); - } - }), + ), + service, + "web-module-trace", + "trace_web_module_imports", + ), + ), + ), }); }; diff --git a/src/cli/webSourceLocationCommands.ts b/src/cli/webSourceLocationCommands.ts index 49eb77f6b7..961081c2d2 100644 --- a/src/cli/webSourceLocationCommands.ts +++ b/src/cli/webSourceLocationCommands.ts @@ -2,11 +2,11 @@ import { z } from "incur"; import { createWebSourceLocationService } from "../composition/webSourceLocations.js"; import { CLI_COMMANDS } from "../cliCommandNames.js"; import { logCliCommand } from "../cliLogging.js"; -import { projectAnalysisError } from "../domain/analysisErrorProjection.js"; import type { Logger } from "pino"; import type { CliInstance } from "./types.js"; import { cliIntegerArgument } from "./options.js"; import { withCommandCancellation } from "./commandCancellation.js"; +import { finishAnalysisCommand } from "./finishAnalysisCommand.js"; /** Expose the same explicit selected map/point contract through the CLI. */ export const registerWebSourceLocationCommands = ( @@ -35,13 +35,11 @@ export const registerWebSourceLocationCommands = ( ), }), run: ({ args }) => - withCommandCancellation(async (signal) => { - try { - return await logCliCommand( - logger, - CLI_COMMANDS.traceWebSourceLocation, - async () => { - const result = await service.trace( + withCommandCancellation((signal) => + logCliCommand(logger, CLI_COMMANDS.traceWebSourceLocation, () => + finishAnalysisCommand( + () => + service.trace( { manifest_path: args.manifestPath, script_index: args.scriptIndex, @@ -52,15 +50,12 @@ export const registerWebSourceLocationCommands = ( generated_position: { line: args.line, column: args.column }, }, { signal }, - ); - return result.ok - ? result.value - : projectAnalysisError(result.error); - }, - ); - } finally { - await service.close(); - } - }), + ), + service, + "web-source-location", + "trace_web_source_location", + ), + ), + ), }); }; diff --git a/src/contracts/toolOutputSchemaPrimitives.ts b/src/contracts/toolOutputSchemaPrimitives.ts index a00a43d10f..bee7c7eae3 100644 --- a/src/contracts/toolOutputSchemaPrimitives.ts +++ b/src/contracts/toolOutputSchemaPrimitives.ts @@ -463,6 +463,7 @@ export const expandToolAvailability = ( export const sessionProvider = z .object({ + cleanup_pending: z.boolean(), capabilities: z.array(providerCapabilitySet), analysis_run: analysisRun, analysis_activity: analysisActivity, diff --git a/src/domain/analysisErrorCleanup.ts b/src/domain/analysisErrorCleanup.ts index c6a796f905..2256c00932 100644 --- a/src/domain/analysisErrorCleanup.ts +++ b/src/domain/analysisErrorCleanup.ts @@ -19,6 +19,7 @@ export const analysisErrorWithCleanupFailure = ( { cause: primary }, ), userMessage: projectAnalysisError(primary).message, + remediationAction: projected.remediation.action, ...(primary.capturedOutput === undefined ? {} : { capturedOutput: primary.capturedOutput }), diff --git a/src/domain/analysisErrorPresentation.ts b/src/domain/analysisErrorPresentation.ts index 955b48538d..6b4f1d15fc 100644 --- a/src/domain/analysisErrorPresentation.ts +++ b/src/domain/analysisErrorPresentation.ts @@ -27,6 +27,7 @@ import { HopperTimeoutError, } from "./hopperErrors.js"; import { ProviderSelectionError } from "./providerSelectionError.js"; +import { ProviderAdapterError } from "./providerAdapterError.js"; import { UnknownRegistryError } from "./unknownRegistryError.js"; import { providerRetryAction } from "./providerOperationHealth.js"; import type { AnalysisError, AnalysisErrorTag } from "./analysisErrorBase.js"; @@ -35,6 +36,11 @@ import type { AnalysisErrorProjection } from "./analysisErrorProjection.js"; export const analysisErrorRemediationAction = ( error: AnalysisError, ): string => { + if ( + error instanceof ProviderAdapterError && + error.remediationAction !== undefined + ) + return error.remediationAction; if ( error instanceof BrowserObservationError && error.reason === "service_worker_blocked" @@ -66,7 +72,7 @@ export const analysisErrorRemediationAction = ( ); if (error instanceof HopperTimeoutError) return error.operation === undefined - ? "Inspect Hopper for a loader or license dialog and review details.launcher. Correct the loader configuration or complete Hopper setup, then open the target again." + ? "Review details.launcher and inspect Hopper for ongoing initial analysis or a pending loader, demo, or license dialog. For a large target, increase REA_HOPPER_STARTUP_TIMEOUT_MS in the environment that starts REA and allow at least that long in the MCP client. Resolve retained cleanup before reopening." : error.providerState === "busy" ? "Check binary_session.analysis_activity, wait for the active Hopper request to finish, then retry." : "Check binary_session for Hopper health, then retry the operation."; @@ -287,7 +293,7 @@ export const analysisErrorUserMessage = (error: AnalysisError): string => { const hopperErrorUserMessage = (error: AnalysisError): string | undefined => { if (error instanceof HopperTimeoutError) { if (error.operation === undefined) - return "REA timed out waiting for Hopper bridge readiness. Hopper may be waiting for a loader or license dialog; inspect its window and the captured launcher outcome before retrying."; + return "REA timed out waiting for Hopper bridge readiness, which includes initial analysis. Hopper may still be analyzing the target or waiting for a loader, demo, or license dialog. Review the captured launcher outcome and resolve retained cleanup before retrying."; const request = error.operation; return error.providerState === "busy" ? `Hopper timed out during ${request} while the provider remained busy. Check binary_session.analysis_activity, wait for the active request to finish, then retry.` diff --git a/src/domain/native/nativeCallPartialObservation.ts b/src/domain/native/nativeCallPartialObservation.ts index 904113dc53..ac3575cf9c 100644 --- a/src/domain/native/nativeCallPartialObservation.ts +++ b/src/domain/native/nativeCallPartialObservation.ts @@ -2,37 +2,41 @@ import { z } from "zod"; import { nativeCallEventSchema, nativeCapturedOutputSchema, + nativeCallObservationResultSchema, } from "./nativeCallObservation.js"; -/** Native call evidence retained when observation fails before a full result. */ +const completed = nativeCallObservationResultSchema.shape; + +/** Native facts retained on failure, including completed facts before finalization. */ export const nativeCallPartialObservationSchema = z.strictObject({ kind: z.literal("native-call-observation"), - target: z.strictObject({ - path: z.string(), - sha256: z.string().regex(/^[0-9a-f]{64}$/u), - architecture: z.string(), - arguments: z.array(z.string()), - environment: z.record(z.string(), z.string()), - working_directory: z.string().nullable(), + target: completed.target.omit({ launch_identity: true }).extend({ + launch_identity: completed.target.shape.launch_identity.optional(), }), - process: z.strictObject({ + process: completed.process.partial().extend({ pid: z.number().int().positive().nullable(), stdout: nativeCapturedOutputSchema.nullable(), stderr: nativeCapturedOutputSchema.nullable(), other_stops: z.array(z.string()), }), - debugger: z.strictObject({ version: z.string().nullable() }), + debugger: completed.debugger + .partial() + .extend({ version: z.string().nullable() }), + breakpoints: completed.breakpoints.optional(), events: z.array(nativeCallEventSchema), - coverage: z.strictObject({ - status: z.literal("partial"), - reason: z.enum([ - "cancelled", - "timeout", - "tracer-failure", - "capture-failure", - "cleanup-failure", - ]), - }), + coverage: completed.coverage + .omit({ status: true }) + .partial() + .extend({ + status: z.literal("partial"), + reason: z.enum([ + "cancelled", + "timeout", + "tracer-failure", + "capture-failure", + "cleanup-failure", + ]), + }), limitations: z.array(z.string()), }); diff --git a/src/domain/providerAdapterError.ts b/src/domain/providerAdapterError.ts index 02bb178a8e..966181195f 100644 --- a/src/domain/providerAdapterError.ts +++ b/src/domain/providerAdapterError.ts @@ -6,12 +6,14 @@ import { /** A provider adapter failed outside its more precise typed variants. */ export interface ProviderAdapterErrorOptions extends AnalysisErrorOptions { readonly userMessage?: string; + readonly remediationAction?: string; } /** A provider adapter failed outside its more precise typed variants. */ export class ProviderAdapterError extends AnalysisError { readonly _tag = "ProviderAdapterError"; override readonly userMessage: string | undefined; + readonly remediationAction: string | undefined; constructor( readonly providerId: string, @@ -20,5 +22,6 @@ export class ProviderAdapterError extends AnalysisError { ) { super(`Provider ${providerId} adapter failed during ${operation}`, options); this.userMessage = options.userMessage; + this.remediationAction = options.remediationAction; } } diff --git a/src/domain/providerCleanupError.ts b/src/domain/providerCleanupError.ts index 257d27e08e..a21a6bceaf 100644 --- a/src/domain/providerCleanupError.ts +++ b/src/domain/providerCleanupError.ts @@ -17,6 +17,7 @@ export class ProviderCleanupError extends ProviderAdapterError { options?: ErrorOptions & { readonly operation?: string; readonly partialObservation?: AnalysisPartialObservation; + readonly remediationAction?: string; }, ) { const { diff --git a/src/flutter/FlutterBuildProvider.ts b/src/flutter/FlutterBuildProvider.ts index ea5d560253..85924b5e79 100644 --- a/src/flutter/FlutterBuildProvider.ts +++ b/src/flutter/FlutterBuildProvider.ts @@ -25,6 +25,7 @@ import { import { AnalysisProtocolError } from "../domain/analysisErrorCore.js"; import { compareUnicodeCodePoints } from "../domain/unicodeCodePointOrder.js"; import { AnalysisError } from "../domain/analysisErrorBase.js"; +import { ProviderCleanupError } from "../domain/providerCleanupError.js"; import { err, ok, type Result } from "../domain/result.js"; import { ArtifactReaderFailure, @@ -523,18 +524,47 @@ export class FlutterBuildProvider { /** Retain failed snapshot cleanup ownership and retry it on close. */ async close(): Promise { - const failures: unknown[] = []; + const failures: { + readonly root: string; + readonly causes: readonly unknown[]; + }[] = []; for (const root of [...this.ownedRoots.keys()]) { if (!(await this.discardRoot(root))) - failures.push( - new AggregateError( - this.cleanupFailures.get(root)!, - `snapshot root retained: ${root}`, - ), - ); + failures.push({ root, causes: this.cleanupFailures.get(root)! }); } if (failures.length > 0) - throw new AggregateError(failures, "Flutter snapshot cleanup failed"); + throw new ProviderCleanupError( + FLUTTER_PROVIDER_IDENTITY.id, + failures.map(({ root }) => root), + { + reason: failures + .flatMap(({ causes }) => + causes.map((cause) => + cause instanceof Error ? cause.message : String(cause), + ), + ) + .join("; "), + retained_roots: failures.map(({ root, causes }) => ({ + path: root, + failures: causes.map((cause) => ({ + name: cause instanceof Error ? cause.name : "UnknownError", + message: cause instanceof Error ? cause.message : String(cause), + ...(cause instanceof Error && + "code" in cause && + typeof cause.code === "string" + ? { code: cause.code } + : {}), + })), + })), + }, + { + cause: new AggregateError( + failures.flatMap(({ causes }) => causes), + "Flutter snapshot cleanup failed", + ), + operation: "close_flutter_build", + }, + ); } async execute( diff --git a/src/hopper/HopperCleanup.ts b/src/hopper/HopperCleanup.ts index ac4cce6594..a317d773cd 100644 --- a/src/hopper/HopperCleanup.ts +++ b/src/hopper/HopperCleanup.ts @@ -99,6 +99,12 @@ export const cleanupHopperSession = async ( ) input.releaseTransport(input.socket); await stopProcess(input, state); + // A late authenticated application may exit while shutdown is in flight. + await confirmExternalApplicationExit( + input.resources, + input.socket, + input.releaseTransport, + ); recordUnconfirmedDocument(input, state); await report(input.progress, 0.75, "removing Hopper private runtime files"); if ( @@ -379,15 +385,33 @@ const cleanupOutcome = ( ): Result => { if (state.issues.length === 0) return ok(null); return err( - new ProviderCleanupError("hopper", [...state.resources], { - issues: cleanupIssueDetails(state.issues), - ...(input.resources.runtimeRoot === undefined - ? {} - : { runtime_root: input.resources.runtimeRoot.path }), - ...(input.activeRequest === null - ? {} - : { active_request: activityDetails(input.activeRequest) }), - }), + new ProviderCleanupError( + "hopper", + [...state.resources], + { + issues: cleanupIssueDetails(state.issues), + ...(input.resources.runtimeRoot === undefined + ? {} + : { runtime_root: input.resources.runtimeRoot.path }), + ...(input.activeRequest === null + ? {} + : { active_request: activityDetails(input.activeRequest) }), + ...(state.resources.has("hopper-document") + ? { + authenticated_application: + input.resources.externalApplication === undefined + ? "not_observed" + : input.resources.externalApplication.identity.state, + } + : {}), + }, + state.resources.has("hopper-document") + ? { + remediationAction: + "Export the Evidence bundle before close_binary clears session records. Let Hopper finish initial analysis or complete its pending dialog, then retry close_binary on this connection so REA can authenticate and close the retained document. If no matching bridge becomes available, manually close the reported document in Hopper, then restart the MCP connection. A helper exit or open:false does not establish that retained cleanup is complete.", + } + : undefined, + ), ); }; diff --git a/src/hopper/HopperClient.ts b/src/hopper/HopperClient.ts index a0668c3a77..4f319207e6 100644 --- a/src/hopper/HopperClient.ts +++ b/src/hopper/HopperClient.ts @@ -142,7 +142,12 @@ export class HopperClient { this.#rememberProcessFailure(error, []); }; readonly #onSocketClose = (): void => { - if (this.#closing) return; + if (this.#closing) { + this.#failAll( + new HopperProcessError(null, { providerState: "unreachable" }), + ); + return; + } const launch = this.#resources.launch; if ( launch?.providerLifetime === "launcher-process" && @@ -215,6 +220,24 @@ export class HopperClient { * A recorded exit remains visible after request activity returns to idle. */ operationHealth(): ProviderOperationHealth { + if ( + this.#closing || + this.#closePromise !== undefined || + (!this.#ready && + this.#startupController !== undefined && + this.#operationFailure === undefined) + ) { + return { + state: "busy", + stage: + this.#closing || this.#closePromise !== undefined + ? "connection" + : "launch", + retryAction: "wait", + exitCode: null, + requests: [], + }; + } const failure = this.#operationFailure; if (failure !== undefined) { const requests: readonly ProviderOperationRequest[] = @@ -340,6 +363,7 @@ export class HopperClient { this.#operationFailure = undefined; this.#startupFailureResult = undefined; this.#resources.shutdownConfirmed = false; + this.#resources.externalApplication = undefined; const deadline = new ProviderStartupDeadline( this.#options.startupTimeoutMs, signal, @@ -437,9 +461,31 @@ export class HopperClient { : health.error, ); } - const parsed = parseHopperServerInfo(health.value, runId); + const parsed = await this.#authenticateServerInfo(health.value, runId); if (!parsed.ok) return this.#startupFailure(parsed.error); + await this.#lineage.observe(this.#resources.launch); + return parsed; + } + + /** Bind process identity for both initial and late cleanup handshakes. */ + async #authenticateServerInfo( + value: JsonValue, + runId: string, + ): Promise> { + const parsed = parseHopperServerInfo(value, runId); + if (!parsed.ok) return parsed; if (this.#resources.launch?.providerLifetime === "external-application") { + const application = this.#resources.externalApplication; + if ( + application !== undefined && + application.pid !== parsed.value.processId + ) + return err( + new HopperProtocolError( + "Hopper bridge process changed within the same run", + ), + ); + if (application?.identity.state === "readable") return parsed; let identity; try { identity = await observeProcessStartIdentity(parsed.value.processId); @@ -460,7 +506,6 @@ export class HopperClient { }, }; } - await this.#lineage.observe(this.#resources.launch); return parsed; } @@ -631,7 +676,11 @@ export class HopperClient { timeoutMs: HOPPER_SHUTDOWN_TIMEOUT_MS, }, ); - if (health.ok && parseHopperServerInfo(health.value, runId).ok) return; + if ( + health.ok && + (await this.#authenticateServerInfo(health.value, runId)).ok + ) + return; this.#releaseTransport(); } finally { this.#closing = false; diff --git a/src/native/LldbCallTracer.ts b/src/native/LldbCallTracer.ts index 126852c7e2..46fa3258ae 100644 --- a/src/native/LldbCallTracer.ts +++ b/src/native/LldbCallTracer.ts @@ -41,6 +41,7 @@ import { } from "../process/ProviderDeadline.js"; import { PrivateRuntimeRoot } from "../process/PrivateRuntimeRoot.js"; import { resolveXcrunTool, type ResolvedTool } from "./CommandRunner.js"; +import { projectNativeCalls } from "./NativeCallObservation.js"; import { readLldbObservationJournal, readBoundedPrefix, @@ -124,6 +125,7 @@ const tracerOutputSchema = z.discriminatedUnion("status", [ ]), error: z.string().nullable(), target_process_created: z.boolean().optional(), + target_process_exit_observed: z.boolean().optional(), }), ]); @@ -285,6 +287,16 @@ export class LldbCallTracer implements NativeCallTracer { ), ); } + if (result.ok && signal?.aborted === true) + return err( + new AnalysisCancelledError(OPERATION, { + partialObservation: partialObservationFromTrace( + request, + result.value, + "cancelled", + ), + }), + ); return result; } @@ -421,11 +433,16 @@ export class LldbCallTracer implements NativeCallTracer { exited.kind === "exited" ? await readTracerOutput(paths.result) : undefined; - let targetTermination: TargetTermination = exited.targetLaunched - ? await ensureTerminated(paths.pid, exited.targetIdentity, true) - : "terminated"; - // Only an explicit bridge phase fact may prove that no target process was - // created. A missing PID or an integrity-error status alone is ambiguous. + // An explicit bridge observation can prove no process was created or that + // the created process exited. A missing PID alone remains ambiguous. + const bridgeObservedExit = + output !== undefined && + output.status !== "traced" && + output.target_process_exit_observed === true; + let targetTermination: TargetTermination = + exited.targetLaunched && !bridgeObservedExit + ? await ensureTerminated(paths.pid, exited.targetIdentity, true) + : "terminated"; if ( output !== undefined && output.status !== "traced" && @@ -692,27 +709,23 @@ const partialNativeObservation = async (options: { const partialObservationFromTrace = ( request: Parameters[0], trace: NativeCallTrace, -): NativeCallPartialObservation => - projectLldbPartialObservation({ - target: { + reason: NativeCallPartialObservation["coverage"]["reason"] = "cleanup-failure", +): NativeCallPartialObservation => { + const observation = projectNativeCalls( + { path: request.executable, sha256: request.expectedSha256, architecture: request.architecture, - arguments: request.input.arguments, - environment: request.input.environment, - working_directory: request.input.working_directory ?? null, }, - pid: trace.run.pid, - stdout: trace.stdout, - stderr: trace.stderr, - version: trace.run.version, - journal: { - events: trace.run.events, - otherStops: trace.run.other_stops, - limitations: [], - }, - reason: "cleanup-failure", - }); + request.input, + trace, + ); + return { + kind: "native-call-observation", + ...observation, + coverage: { ...observation.coverage, status: "partial", reason }, + }; +}; const lldbRuntimePaths = (directory: string) => ({ config: join(directory, "config.json"), diff --git a/src/process/capture/ProcessHarness.ts b/src/process/capture/ProcessHarness.ts index f411718aab..017050803a 100644 --- a/src/process/capture/ProcessHarness.ts +++ b/src/process/capture/ProcessHarness.ts @@ -377,6 +377,7 @@ const finishProcessRun = async (options: { readonly progress: ProcessCaptureProgressTracker; readonly progressCounts: () => ProcessCaptureProgressCounts; readonly resourceScope?: ProcessCaptureResourceScope; + readonly signal?: AbortSignal; }): Promise => { options.progress.closeLive(); await options.stopSampler(); @@ -401,6 +402,9 @@ const finishProcessRun = async (options: { options.resourceScope === undefined ? await releaseProcessResources(cleanupOptions) : await options.resourceScope.release(cleanupOptions); + let { capture, executionFailure } = options; + if (executionFailure === undefined && options.signal?.aborted) + executionFailure = processCaptureCancelled(); const observedExit = options.observations?.exit.state === "available" ? options.observations.exit.value.reason @@ -409,14 +413,10 @@ const finishProcessRun = async (options: { phase: "cleanup", counts: options.progressCounts(), terminal: true, - disposition: captureProgressDisposition( - observedExit, - options.executionFailure, - ), + disposition: captureProgressDisposition(observedExit, executionFailure), cleanup, }); const cleanupFailure = cleanupReportFailure(cleanup); - let { capture, executionFailure } = options; let verifiedCapture: ProcessCapture | undefined; if (capture !== undefined && cleanupFailure === undefined) { const settlement: UnverifiedProcessCapture["settlement"] = @@ -445,6 +445,10 @@ const finishProcessRun = async (options: { rootPid === undefined ? { scenario: options.scenario } : { scenario: options.scenario, rootPid }; + // Cancellation remains authoritative until finalization has settled, even + // when the workload exited normally before cleanup began. + if (executionFailure === undefined && options.signal?.aborted) + executionFailure = processCaptureCancelled(); return resolveProcessResult( cleanupFailure === undefined ? verifiedCapture : capture, executionFailure, @@ -830,6 +834,7 @@ const runProcessScenario = async ( progress: progressTracker, progressCounts, ...(resourceScope === undefined ? {} : { resourceScope }), + ...(signal === undefined ? {} : { signal }), }); }; diff --git a/tests/boundary/cli/wasmCleanupCli.test.ts b/tests/boundary/cli/wasmCleanupCli.test.ts new file mode 100644 index 0000000000..ce5718b04f --- /dev/null +++ b/tests/boundary/cli/wasmCleanupCli.test.ts @@ -0,0 +1,125 @@ +import { chmod, mkdir, readFile, writeFile } from "node:fs/promises"; +import { execFileSync } from "node:child_process"; +import { join } from "node:path"; +import { fileURLToPath } from "node:url"; + +import { expect, it } from "vitest"; + +import { analysisErrorProjectionSchema } from "../../../src/contracts/errorSchemas.js"; +import { parseEvidence } from "../../../src/domain/evidence.js"; +import { runCliJson } from "../../fixtures/cliJsonProcess.js"; +import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; + +const canDenyCleanup = process.platform !== "win32" && process.getuid?.() !== 0; + +it.runIf(canDenyCleanup)( + "retains wasm evidence and primary diagnostics when real scratch cleanup is denied", + async () => { + const directory = await createTestTempDirectory("rea-wasm-cleanup-cli-"); + const tools = join(directory, "tools"); + const artifact = join(directory, "module.wasm"); + const locked = join(directory, "locked-paths.txt"); + await mkdir(tools); + const wasm = Buffer.from( + "0061736d01000000010401600000020b0103656e7603666f6f0000030201000503010001071002066d656d6f727902000372756e00010a0601040001010b000c04746573747061796c6f6164", + "hex", + ); + expect(wasm).toHaveLength(76); + await writeFile(artifact, wasm); + execFileSync(process.execPath, [ + "-e", + "WebAssembly.compile(require('node:fs').readFileSync(process.argv[1])).catch(() => { process.exitCode = 1; });", + artifact, + ]); + const objdump = fileURLToPath( + new URL("../../fixtures/wasm/objdump.txt", import.meta.url), + ); + const makeTool = async (name: string, body: string) => { + const path = join(tools, name); + await writeFile(path, `#!/bin/sh\n${body}\n`, { mode: 0o700 }); + await chmod(path, 0o700); + }; + const deny = `mkdir denied\nprintf '%s\\n' "$PWD/denied" >> "$REA_LOCKED_PATHS"\nprintf marker > denied/marker\nchmod 000 denied`; + await makeTool( + "wasm-validate", + `if [ "$1" = "--version" ]; then echo '1.0.42'; exit 0; fi\nif [ "$REA_PRIMARY_FAILURE" = "1" ]; then ${deny}\n echo 'fixture validation rejected' >&2\n exit 1\nfi\nexit 0`, + ); + await makeTool( + "wasm-objdump", + `if [ "$1" = "--version" ]; then echo '1.0.42'; exit 0; fi\ncat "$REA_FIXTURE_OBJDUMP"`, + ); + await makeTool( + "wasm2wat", + `if [ "$1" = "--version" ]; then echo '1.0.42'; exit 0; fi\nif [ "$REA_DENY_CLEANUP" = "1" ]; then ${deny}; fi\nprintf '%s\\n' '(module (@custom "test" "payload"))'`, + ); + + const environment = { + ...process.env, + REA_WABT_BIN_DIRECTORY: tools, + REA_FIXTURE_OBJDUMP: objdump, + REA_LOCKED_PATHS: locked, + TMPDIR: directory, + TMP: directory, + TEMP: directory, + }; + const invoke = (extra: NodeJS.ProcessEnv = {}) => + runCliJson(["inspect-wasm-artifact", artifact, "--format", "json"], { + ...environment, + ...extra, + }); + const restoreDeniedDirectories = async () => { + let paths: string[] = []; + try { + paths = (await readFile(locked, "utf8")).trim().split("\n"); + } catch (cause: unknown) { + if ( + cause instanceof Error && + "code" in cause && + cause.code === "ENOENT" + ) + return; + throw cause; + } + for (const path of paths) await chmod(path, 0o700); + }; + + try { + const control = parseEvidence(await invoke()); + expect(control.normalized_result).toMatchObject({ validation: "valid" }); + + const completedThenCleanupFails = analysisErrorProjectionSchema.parse( + await invoke({ REA_DENY_CLEANUP: "1" }), + ); + expect(completedThenCleanupFails).toMatchObject({ + code: "cleanup_incomplete", + details: { + cleanup: "incomplete", + partial_observation: { + normalized_result: { validation: "valid" }, + }, + }, + }); + + await restoreDeniedDirectories(); + await writeFile(locked, ""); + const primaryAndCleanupFail = analysisErrorProjectionSchema.parse( + await invoke({ REA_PRIMARY_FAILURE: "1" }), + ); + expect(primaryAndCleanupFail).toMatchObject({ + code: "cleanup_incomplete", + details: { + execution_failure: "execution_failure", + diagnostics: { + primary_error: { code: "cleanup_incomplete" }, + cleanup_error: { code: "cleanup_incomplete" }, + }, + }, + }); + expect(JSON.stringify(primaryAndCleanupFail.details)).toContain( + "fixture validation rejected", + ); + } finally { + await restoreDeniedDirectories(); + } + }, +); diff --git a/tests/boundary/filesystem/flutterBuildProvider.test.ts b/tests/boundary/filesystem/flutterBuildProvider.test.ts index 3773378f20..a4f4deeac0 100644 --- a/tests/boundary/filesystem/flutterBuildProvider.test.ts +++ b/tests/boundary/filesystem/flutterBuildProvider.test.ts @@ -14,7 +14,10 @@ import { } from "../../../src/flutter/FlutterBuild.fixture.js"; import { snapshotSection } from "../../../src/flutter/FlutterBuild.fixture.js"; import { FlutterBuildProvider } from "../../../src/flutter/FlutterBuildProvider.js"; +import { FlutterBuildAnalysisService } from "../../../src/application/flutter/FlutterBuildAnalysisService.js"; +import { finishAnalysisCommand } from "../../../src/cli/finishAnalysisCommand.js"; import { projectAnalysisError } from "../../../src/domain/analysisErrorProjection.js"; +import { ProviderCleanupError } from "../../../src/domain/providerCleanupError.js"; import { AnalysisCancelledError, AnalysisResourceConstraintError, @@ -617,24 +620,49 @@ it("retains failed snapshot cleanup and retries it on close", async () => { bytes: libapp(REFERENCE_HASH), }, ]); - const spaces: string[] = []; + const roots: string[] = []; let failing = true; const provider = new FlutterBuildProvider({ removeRoot: async (path) => { + roots.push(path); if (failing) throw new Error("cleanup refused"); - spaces.push(path); await rm(path, { recursive: true, force: true }); }, }); - const result = await provider.execute({ - operation: "identify_flutter_build", - input: { path: apk }, + onTestFinished(async () => { + failing = false; + await provider.close(); }); - expect(result.ok).toBe(true); - await expect(provider.close()).rejects.toThrow(/snapshot cleanup failed/u); + const service = new FlutterBuildAnalysisService(provider); + const result = await finishAnalysisCommand( + () => + service.execute("identify_flutter_build", { + path: apk, + }), + { close: () => provider.close() }, + "flutter", + "identify_flutter_build", + ); + expect(result).toMatchObject({ + code: "cleanup_incomplete", + details: { + resources: [expect.stringContaining("rea-flutter-")], + partial_observation: { + operation: "identify_flutter_build", + subject: { local_path: apk }, + normalized_result: { flutter_detected: true }, + }, + }, + }); + expect(result).toMatchObject({ + details: { resources: [roots[0]] }, + }); + expect(roots).toHaveLength(2); + expect(roots[1]).toBe(roots[0]); failing = false; await provider.close(); - expect(spaces.length).toBeGreaterThanOrEqual(1); + expect(roots).toHaveLength(3); + expect(roots[2]).toBe(roots[0]); }); it("cancels before creating any snapshot root", async () => { @@ -1032,7 +1060,28 @@ it.each(["identify_flutter_build", "inspect_dart_aot"] as const)( const result = await provider.execute({ operation, input: { path } }); if (!result.ok) throw result.error; expect(removeRoot).not.toHaveBeenCalled(); - await expect(provider.close()).rejects.toThrow("snapshot cleanup failed"); + let cleanupFailure: unknown; + try { + await provider.close(); + } catch (cause) { + cleanupFailure = cause; + } + expect(cleanupFailure).toBeInstanceOf(ProviderCleanupError); + expect(cleanupFailure).toMatchObject({ + operation: "close_flutter_build", + cleanupResources: [expect.stringContaining("rea-flutter-")], + diagnostics: { + reason: expect.stringContaining("file handle close refused"), + retained_roots: [ + { + path: expect.stringContaining("rea-flutter-"), + failures: [ + { message: expect.stringContaining("file handle close refused") }, + ], + }, + ], + }, + }); expect(removeRoot).not.toHaveBeenCalled(); failClose = false; await provider.close(); @@ -1104,7 +1153,7 @@ it.each([0, 1, 2, 3])( }); expect(result.ok).toBe(false); expect(removeRoot).not.toHaveBeenCalled(); - await expect(provider.close()).rejects.toThrow("snapshot cleanup failed"); + await expect(provider.close()).rejects.toBeInstanceOf(ProviderCleanupError); failing = false; await provider.close(); expect(removeRoot).toHaveBeenCalledTimes(1); diff --git a/tests/boundary/filesystem/webArtifactReadOwnership.test.ts b/tests/boundary/filesystem/webArtifactReadOwnership.test.ts index 0a87913667..f679ff75e2 100644 --- a/tests/boundary/filesystem/webArtifactReadOwnership.test.ts +++ b/tests/boundary/filesystem/webArtifactReadOwnership.test.ts @@ -33,7 +33,11 @@ vi.mock("node:fs/promises", async (importOriginal) => { }); import { WebModuleTraceService } from "../../../src/application/WebModuleTraceService.js"; +import { finishAnalysisCommand } from "../../../src/cli/finishAnalysisCommand.js"; import { LocalWebModuleArtifacts } from "../../../src/browser/modules/WebModuleArtifacts.js"; +import { ProviderCleanupError } from "../../../src/domain/providerCleanupError.js"; +import { AnalysisCancelledError } from "../../../src/domain/analysisErrorCore.js"; +import { projectAnalysisError } from "../../../src/domain/analysisErrorProjection.js"; import { webModuleArtifactsFixture } from "../../fixtures/webModuleTrace.js"; import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; @@ -82,15 +86,74 @@ it("retains cancelled source read ownership until service close retries it", asy expect(blocked.error.cleanupIncomplete).toBe(true); expect(injected.closeAttempts).toBe(2); - await expect(service.close()).rejects.toThrow( - "Web module trace cleanup failed", - ); + let cleanupFailure: unknown; + try { + await service.close(); + } catch (cause) { + cleanupFailure = cause; + } + expect(cleanupFailure).toBeInstanceOf(ProviderCleanupError); + expect(cleanupFailure).toMatchObject({ + operation: "trace_web_module_imports", + cleanupResources: [sourcePath], + diagnostics: { reason: expect.stringContaining("fixture close refusal") }, + }); + expect((cleanupFailure as ProviderCleanupError).cause).toBeInstanceOf(Error); expect(resolver.close).toHaveBeenCalledTimes(1); expect(injected.closeAttempts).toBe(3); + const finalized = await finishAnalysisCommand( + () => service.trace(input), + service, + "web-module-artifacts", + "trace_web_module_imports", + ); + expect(finalized).toMatchObject({ + code: "cleanup_incomplete", + details: { resources: [sourcePath] }, + }); + expect(resolver.close).toHaveBeenCalledTimes(2); + injected.failClose = false; const attemptsBeforeRetry = injected.closeAttempts; await service.close(); expect(injected.closeAttempts).toBeGreaterThan(attemptsBeforeRetry); - expect(resolver.close).toHaveBeenCalledTimes(2); + expect(resolver.close).toHaveBeenCalledTimes(3); +}); + +it("preserves both web cleanup owners' resources, diagnostics and recovery actions", async () => { + const artifacts = new ProviderCleanupError( + "web-artifacts", + ["artifact-root", "shared-resource"], + { reason: "artifact cleanup denied" }, + { remediationAction: "Release the retained artifact lease." }, + ); + const resolver = new AnalysisCancelledError("web-resolution", { + cleanup: { + reason: "resolver shutdown unconfirmed", + resources: ["resolver-root", "shared-resource"], + }, + }); + const service = new WebModuleTraceService( + { + load: vi.fn(), + close: async () => { + throw artifacts; + }, + }, + { + resolve: vi.fn(), + close: async () => { + throw resolver; + }, + }, + ); + const projected = [artifacts, resolver].map(projectAnalysisError); + await expect(service.close()).rejects.toMatchObject({ + cleanupResources: ["artifact-root", "shared-resource", "resolver-root"], + diagnostics: { failures: projected }, + remediationAction: projected + .map(({ remediation }) => remediation.action) + .join("; "), + }); }); diff --git a/tests/boundary/mcp/sessionAvailabilityRecovery.test.ts b/tests/boundary/mcp/sessionAvailabilityRecovery.test.ts index 029863b391..a214f02d6f 100644 --- a/tests/boundary/mcp/sessionAvailabilityRecovery.test.ts +++ b/tests/boundary/mcp/sessionAvailabilityRecovery.test.ts @@ -4,6 +4,7 @@ import { expect, it } from "vitest"; import { toolContract } from "../../../src/contracts/toolContracts.js"; import { expandToolAvailability } from "../../../src/contracts/toolOutputSchemaPrimitives.js"; import { ProviderAdapterError } from "../../../src/domain/providerAdapterError.js"; +import { ProviderCleanupError } from "../../../src/domain/providerCleanupError.js"; import { err, ok } from "../../../src/domain/result.js"; import { silentLogger } from "../../../src/logger.js"; import { createServer } from "../../../src/server/createServer.js"; @@ -152,3 +153,102 @@ it("restores MCP tool availability after cancelling a target switch", async () = await session.close(); } }); + +it("keeps a retired run and its lineage visible over MCP until cleanup succeeds", async () => { + const [path] = await createBinarySessionTargets(); + let allowed = false; + const provider = createCacheProvider([]); + provider.createClient = (_target, _profile, context) => { + if (context === undefined) throw new Error("missing analysis run context"); + return { + execute: () => Promise.resolve(observed("fixture-name")), + runtimeLineageSnapshots: () => [ + { + provider: provider.identity(), + observation: { + status: "unavailable" as const, + observedAt: "2026-10-16T00:00:00Z", + runId: context.runId, + launcherPid: 123, + processGroupId: 123, + reason: "fixture identity could not be revalidated", + }, + }, + ], + close: () => + Promise.resolve( + allowed + ? ok(null) + : err( + new ProviderCleanupError("fixture", ["owned-runtime"], { + reason: "fixture cleanup denied", + }), + ), + ), + }; + }; + const session = createTestBinarySession(provider); + const server = createServer( + { kind: "session", session }, + { environment: {}, logger: silentLogger }, + ); + const client = new Client({ name: "retained-run", version: "1" }); + const [clientTransport, serverTransport] = + InMemoryTransport.createLinkedPair(); + const readStatus = async () => { + const reply = await client.callTool({ + name: "binary_session", + arguments: {}, + }); + return toolContract("binary_session").outputSchema.parse( + reply.structuredContent, + ).result; + }; + try { + await server.connect(serverTransport); + await client.connect(clientTransport); + expect((await readStatus()).analysis_run).toBeNull(); + expect( + (await client.callTool({ name: "open_binary", arguments: { path } })) + .isError, + ).not.toBe(true); + expect( + ( + await client.callTool({ + name: "address_name", + arguments: { address: "0x1000", document: "fixture" }, + }) + ).isError, + ).not.toBe(true); + const before = await readStatus(); + expect(before.analysis_run).toMatchObject({ + run_id: expect.any(String), + process_lineage: { status: "snapshots" }, + }); + for (let attempt = 0; attempt < 2; attempt++) { + expect( + (await client.callTool({ name: "close_binary", arguments: {} })) + .isError, + ).toBe(true); + expect(await readStatus()).toMatchObject({ + open: false, + cleanup_pending: true, + analysis_run: before.analysis_run, + }); + } + allowed = true; + expect( + (await client.callTool({ name: "close_binary", arguments: {} })).isError, + ).not.toBe(true); + expect(await readStatus()).toMatchObject({ + open: false, + cleanup_pending: false, + analysis_run: null, + }); + } finally { + allowed = true; + await client.close(); + await server.close(); + await session.close(); + } +}); diff --git a/tests/boundary/process/processCaptureCancellationAdapters.test.ts b/tests/boundary/process/processCaptureCancellationAdapters.test.ts index 694f83409e..4fd088d9c8 100644 --- a/tests/boundary/process/processCaptureCancellationAdapters.test.ts +++ b/tests/boundary/process/processCaptureCancellationAdapters.test.ts @@ -134,6 +134,92 @@ itWithCaptureCapability.each(["SIGINT", "SIGTERM"] as const)( 20_000, ); +itWithCaptureCapability( + "preserves completed observations as partial evidence when SIGINT arrives during CLI cleanup", + async () => { + const root = await createTestTempDirectory("rea-process-cleanup-cancel-"); + const scenario = join(root, "scenario.json"); + const cleanupMarker = join(root, "cleanup.json"); + await writeFile( + scenario, + JSON.stringify({ + executable: process.execPath, + arguments: ["-e", "console.log('completed-before-cleanup')"], + timeout_ms: 5_000, + }), + ); + const hook = fileURLToPath( + new URL("../../fixtures/processCleanupCancellation.mjs", import.meta.url), + ); + for (const cancel of [false, true]) { + const cli = spawn( + process.execPath, + [ + "--import", + hook, + "scripts/rea.mjs", + "capture-process", + scenario, + "--format", + "json", + ], + { + env: { + ...process.env, + TMPDIR: root, + REA_TEST_CANCEL_DURING_CLEANUP: cancel ? "1" : "0", + REA_TEST_CLEANUP_MARKER: cleanupMarker, + }, + stdio: ["ignore", "pipe", "pipe"], + }, + ); + let stdout = ""; + cli.stdout.on("data", (chunk: Buffer) => { + stdout += chunk.toString(); + }); + cli.stderr.resume(); + const closed = new Promise((resolve, reject) => { + cli.once("error", reject); + cli.once("close", resolve); + }); + onTestFinished(async () => { + if (cli.exitCode === null && cli.signalCode === null) + cli.kill("SIGKILL"); + await closed; + }); + expect(await closed).toBe(cancel ? 130 : 0); + const marker = z + .object({ path: z.string(), removed: z.literal(true) }) + .parse(JSON.parse(await readFile(cleanupMarker, "utf8"))); + await expect(access(marker.path)).rejects.toMatchObject({ + code: "ENOENT", + }); + const output: unknown = JSON.parse(stdout); + if (cancel) { + expect(analysisCliErrorEnvelopeSchema.parse(output)).toMatchObject({ + code: "cancelled", + details: { + cleanup: "complete", + partial_observation: { + capture: { exit: { code: 0 } }, + cleanup: { temporary_root: { state: "cleaned" } }, + }, + }, + }); + } else { + expect(output).toMatchObject({ + operation: "capture_process_scenario", + normalized_result: { + exit: { code: 0 }, + cleanup: { temporary_root: "removed" }, + }, + }); + } + } + }, + 30_000, +); + itWithCaptureCapability( "stops the owned descendant when an MCP request is cancelled during post-exit settlement", async () => { diff --git a/tests/boundary/providers/hopper/hopperBridgeReconnect.test.ts b/tests/boundary/providers/hopper/hopperBridgeReconnect.test.ts index 9a166d5604..b7b0f4cdc0 100644 --- a/tests/boundary/providers/hopper/hopperBridgeReconnect.test.ts +++ b/tests/boundary/providers/hopper/hopperBridgeReconnect.test.ts @@ -52,12 +52,30 @@ it.skipIf(process.platform === "win32")( listener_removed: true, thread_released: true, })), + { + mode: "authenticated-idle", + requests: [ + "health", + "health", + "health", + "shutdown_document", + "shutdown_document", + ], + listener_removed: true, + thread_released: true, + }, { mode: "expiry", requests: ["health"], listener_removed: true, thread_released: true, }, + ...["initial-expiry", "silent-expiry"].map((mode) => ({ + mode, + requests: [], + listener_removed: true, + thread_released: true, + })), ]); }, ); diff --git a/tests/boundary/providers/hopper/hopperClient.fixture.ts b/tests/boundary/providers/hopper/hopperClient.fixture.ts index 5ac37be249..0272b84873 100644 --- a/tests/boundary/providers/hopper/hopperClient.fixture.ts +++ b/tests/boundary/providers/hopper/hopperClient.fixture.ts @@ -36,6 +36,7 @@ export class HopperFixtureLauncher implements BridgeLauncher { readonly tokenOverride?: string, readonly diagnosticMode: "quiet" | "noisy" | "noisy_no_socket" = "quiet", readonly healthMode: "ready" | "gated" = "ready", + readonly shutdownMode: "acknowledge" | "exit-without-ack" = "acknowledge", ) {} launch(session: BridgeSession) { @@ -50,7 +51,7 @@ export class HopperFixtureLauncher implements BridgeLauncher { session.socketPath, this.tokenOverride ?? session.token, session.runId, - "acknowledge", + this.shutdownMode, this.diagnosticMode, this.healthMode, ], diff --git a/tests/boundary/providers/hopper/hopperClient.handshakeRecovery.test.ts b/tests/boundary/providers/hopper/hopperClient.handshakeRecovery.test.ts index f09f467842..faf69e347a 100644 --- a/tests/boundary/providers/hopper/hopperClient.handshakeRecovery.test.ts +++ b/tests/boundary/providers/hopper/hopperClient.handshakeRecovery.test.ts @@ -32,11 +32,13 @@ class ExternalBridgeLauncher implements BridgeLauncher { constructor( readonly identity: "matching" | "wrong-run" | "wrong-token" = "matching", healthMode: "ready" | "gated" = "ready", + shutdownMode: "acknowledge" | "exit-without-ack" = "acknowledge", ) { this.fixture = new HopperFixtureLauncher( identity === "wrong-token" ? "foreign-token" : undefined, "quiet", healthMode, + shutdownMode, ); } @@ -62,8 +64,13 @@ class ExternalBridgeLauncher implements BridgeLauncher { const setup = ( identity?: "matching" | "wrong-run" | "wrong-token", healthMode?: "ready" | "gated", + shutdownMode?: "acknowledge" | "exit-without-ack", ) => { - const launcher = new ExternalBridgeLauncher(identity, healthMode); + const launcher = new ExternalBridgeLauncher( + identity, + healthMode, + shutdownMode, + ); const client = new HopperClient({ launcher, startupTimeoutMs: 2_000 }); onTestFinished(async () => { await client.close(); @@ -245,3 +252,67 @@ it("preserves launcher diagnostics after a health deadline and reauthenticates f "shutdown_document", ]); }); + +it("recovers an application exit after the first valid handshake occurs during cleanup", async () => { + const { client, launcher } = setup("matching", "gated", "exit-without-ack"); + const starting = client.start(); + const first = await launcher.fixture.waitForRequest("health"); + const started = await starting; + if (started.ok) throw new Error("Expected the first handshake to time out"); + expect(projectAnalysisError(started.error)).toMatchObject({ + code: "cleanup_incomplete", + remediation: { + action: expect.stringContaining("restart the MCP connection"), + }, + details: { + diagnostics: { + primary_error: { code: "provider_timeout" }, + cleanup_error: { + details: { + diagnostics: { authenticated_application: "not_observed" }, + }, + }, + }, + }, + }); + + const closing = client.close(); + const next = await vi.waitFor(() => { + const request = launcher.fixture.requests.find( + ({ method, id }) => method === "health" && id !== first.id, + ); + if (request === undefined) + throw new Error("Waiting for cleanup authentication"); + return request; + }); + await launcher.fixture.release(next); + const closed = await closing; + if (!closed.ok) throw closed.error; + expect(launcher.fixture.requests.map(({ method }) => method)).toEqual([ + "health", + "health", + "shutdown_document", + ]); + const session = launcher.sessions[0]; + if (session === undefined) throw new Error("Fixture did not launch"); + await expect(access(session.directory)).rejects.toMatchObject({ + code: "ENOENT", + }); + const restarted = client.start(); + const health = await vi.waitFor(() => { + const request = launcher.fixture.requests.find( + ({ method, id }) => method === "health" && id > next.id, + ); + if (request === undefined) + throw new Error("Waiting for restarted authentication"); + return request; + }); + await launcher.fixture.release(health); + const ready = await restarted; + if (!ready.ok) throw ready.error; + const observed = await client.callTool("current_document"); + if (!observed.ok) throw observed.error; + expect(observed.value).toBe("fixture"); + const reclosed = await client.close(); + if (!reclosed.ok) throw reclosed.error; +}); diff --git a/tests/boundary/providers/hopper/hopperClient.health.test.ts b/tests/boundary/providers/hopper/hopperClient.health.test.ts index 66748051ef..f8267c7e86 100644 --- a/tests/boundary/providers/hopper/hopperClient.health.test.ts +++ b/tests/boundary/providers/hopper/hopperClient.health.test.ts @@ -64,6 +64,55 @@ const start = async (owned: boolean) => { return { client, launcher }; }; +it("reports launch work until the first authenticated handshake completes", async () => { + const fixture = new HopperFixtureLauncher(undefined, "quiet", "gated"); + let enterLaunch: (() => void) | undefined; + let releaseLaunch: (() => void) | undefined; + const entered = new Promise((resolve) => { + enterLaunch = resolve; + }); + const release = new Promise((resolve) => { + releaseLaunch = resolve; + }); + const client = new HopperClient({ + startupTimeoutMs: 10_000, + launcher: { + async launch(session) { + enterLaunch?.(); + await release; + return fixture.launch(session); + }, + }, + }); + onTestFinished(async () => { + releaseLaunch?.(); + await client.close(); + }); + const starting = client.callTool("current_document"); + await entered; + expect(client.requestActivity()).toBeNull(); + expect(client.operationHealth()).toMatchObject({ + state: "busy", + stage: "launch", + retryAction: "wait", + }); + releaseLaunch?.(); + const handshake = await fixture.waitForRequest("health"); + expect(client.operationHealth()).toMatchObject({ + state: "busy", + stage: "launch", + retryAction: "wait", + }); + await fixture.release(handshake); + const result = await starting; + if (!result.ok) throw result.error; + expect(result.value).toBe("fixture"); + expect(client.operationHealth()).toMatchObject({ + state: "idle", + stage: null, + }); +}); + describe("Hopper provider lifecycle health", () => { it("settles disconnected requests without attributing the helper exit to the GUI", async () => { const { client, launcher } = await start(false); diff --git a/tests/boundary/providers/native/lldbCallTracerFailures.test.ts b/tests/boundary/providers/native/lldbCallTracerFailures.test.ts index a365f67733..abc71ecb34 100644 --- a/tests/boundary/providers/native/lldbCallTracerFailures.test.ts +++ b/tests/boundary/providers/native/lldbCallTracerFailures.test.ts @@ -1,19 +1,21 @@ import { access, readFile, rm, writeFile } from "node:fs/promises"; import { dirname } from "node:path"; -import { describe, expect, it } from "vitest"; +import { describe, expect, it, onTestFinished, vi } from "vitest"; import { z } from "zod"; import { AnalysisCancelledError } from "../../../../src/domain/analysisErrorCore.js"; import { EvidenceIntegrityError } from "../../../../src/domain/evidenceErrors.js"; import { projectAnalysisError } from "../../../../src/domain/analysisErrorProjection.js"; import { nativeCallObservationInputSchema } from "../../../../src/domain/native/nativeCallObservation.js"; +import { nativeCallPartialObservationSchema } from "../../../../src/domain/native/nativeCallPartialObservation.js"; import type { NativeCallTracer } from "../../../../src/native/LldbCallTracer.js"; import { LldbCallTracer } from "../../../../src/native/LldbCallTracer.js"; import { err, ok } from "../../../../src/domain/result.js"; import { NativeCommandFailure } from "../../../../src/native/CommandRunner.js"; import { NativeMacOSProvider } from "../../../../src/native/NativeMacOSProvider.js"; import { createDeferred } from "../../../fixtures/binarySession.js"; +import { PrivateRuntimeRoot } from "../../../../src/process/PrivateRuntimeRoot.js"; import { NativeFixtureRunner, nativeMachoTarget, @@ -47,10 +49,43 @@ const request: Parameters[0] = { }; const fakeTool = { path: "/usr/bin/lldb", sha256: "b".repeat(64) }; +const successfulTracerOutput = { + status: "traced", + version: "lldb-fixture", + pid: 12345, + outcome: "exited", + resource_limit_reached: false, + target_output: { + stdout_bytes: 0, + stderr_bytes: 0, + stdout_truncated: false, + stderr_truncated: false, + stdout_complete: true, + stderr_complete: true, + }, + exit_status: 0, + exit_description: null, + killed: false, + terminated: true, + target_identity: { + loaded_image_sha256: null, + selected_file_sha256: request.expectedSha256, + file_device: "1", + file_inode: "2", + module_path: request.executable, + module_uuid: "fixture", + stable: true, + }, + elapsed_ms: 1, + breakpoints: [{ index: 0, location_count: 1, locations: [] }], + events: [EVENT], + other_stops: [], +}; const configSchema = z.object({ result_path: z.string(), observation_path: z.string(), stdout_capture_path: z.string(), + stderr_capture_path: z.string(), }); const configFromArguments = async (arguments_: readonly string[]) => { @@ -115,6 +150,102 @@ const fixtureTracer = ( }; }; +describe("LLDB completed trace retention through finalization", () => { + it.each([ + "after launch", + "during runtime cleanup", + "runtime cleanup failure", + ])("retains completed trace facts after %s", async (phase) => { + const controller = new AbortController(); + const close = PrivateRuntimeRoot.prototype.close; + let runtime: string | undefined; + const closeSpy = + phase !== "after launch" + ? vi + .spyOn(PrivateRuntimeRoot.prototype, "close") + .mockImplementation(async function (this: PrivateRuntimeRoot) { + if (phase === "runtime cleanup failure") + throw new Error("fixture runtime removal denied"); + await close.call(this); + controller.abort(); + }) + : undefined; + const tracer = new LldbCallTracer( + {}, + async (_executable, arguments_) => { + const { config } = await configFromArguments(arguments_); + runtime = dirname(config.result_path); + await writeFile(config.observation_path, ""); + await writeFile(config.stdout_capture_path, ""); + await writeFile(config.stderr_capture_path, ""); + await writeFile( + config.result_path, + JSON.stringify(successfulTracerOutput), + ); + if (phase === "after launch") controller.abort(); + return { + kind: "exited", + targetLaunched: false, + exitCode: 0, + output: "", + targetIdentity: undefined, + cleanupFailure: undefined, + }; + }, + async () => ok(fakeTool), + ); + onTestFinished(async () => { + closeSpy?.mockRestore(); + await tracer.close(); + }); + + const result = await tracer + .trace(request, controller.signal) + .finally(() => closeSpy?.mockRestore()); + if (result.ok) + throw new Error("expected finalization failure after LLDB exit"); + if (runtime === undefined) throw new Error("LLDB fixture did not launch"); + if (phase === "runtime cleanup failure") { + expect(result.error.cleanupResources).toEqual([runtime]); + await access(runtime); + expect((await tracer.close()).ok).toBe(true); + } else expect(result.error).toBeInstanceOf(AnalysisCancelledError); + await expect(access(runtime)).rejects.toMatchObject({ code: "ENOENT" }); + expect(result.error.partialObservation).toMatchObject({ + coverage: { + status: "partial", + reason: + phase === "runtime cleanup failure" ? "cleanup-failure" : "cancelled", + }, + target: { launch_identity: successfulTracerOutput.target_identity }, + debugger: { ...fakeTool, version: successfulTracerOutput.version }, + process: { + outcome: "exited", + exit_status: 0, + exit_description: null, + terminated: true, + elapsed_ms: 1, + stdout: { complete: true }, + }, + breakpoints: [ + { + index: 0, + request: { kind: "function", name: "main" }, + location_count: 1, + }, + ], + events: [EVENT], + }); + const partial = nativeCallPartialObservationSchema.parse( + result.error.partialObservation, + ); + expect(partial).toEqual(result.error.partialObservation); + expect(partial.limitations).not.toContain( + "Captured output byte counts are observed lower bounds because LLDB did not report final stream counters.", + ); + }); +}); + describe("LLDB failure retention through the production tracer", () => { it("waits for an active trace before client close removes its runtime root", async () => { const launchGate = createDeferred(); @@ -277,6 +408,45 @@ describe("LLDB pre-target phase proof", () => { }); describe("LLDB retains ambiguous target ownership", () => { + it("releases a launch failure after the bridge proves the process exited", async () => { + let configPath: string | undefined; + const tracer = new LldbCallTracer( + {}, + async (_executable, arguments_) => { + const resolved = await configFromArguments(arguments_); + configPath = resolved.configPath; + await writeFile( + resolved.config.result_path, + JSON.stringify({ + status: "launch-error", + error: "attach failed after the launched process exited", + target_process_created: true, + target_process_exit_observed: true, + }), + ); + return { + kind: "exited", + targetLaunched: true, + exitCode: 1, + output: "", + targetIdentity: undefined, + cleanupFailure: undefined, + }; + }, + async () => ok(fakeTool), + ); + + const result = await tracer.trace(request); + if (result.ok) throw new Error("expected LLDB launch failure"); + expect(projectAnalysisError(result.error).code).toBe( + "capability_unavailable", + ); + expect(result.error.cleanup).toBeUndefined(); + expect(configPath).toBeDefined(); + if (configPath !== undefined) + await expect(access(dirname(configPath))).rejects.toThrow(); + }); + it("keeps a post-target integrity rejection unresolved when its PID is missing", async () => { let runtime: string | undefined; const tracer = new LldbCallTracer( diff --git a/tests/boundary/providers/native/lldbRetainedObservations.test.ts b/tests/boundary/providers/native/lldbRetainedObservations.test.ts index 44ede43ee8..5c981a2a85 100644 --- a/tests/boundary/providers/native/lldbRetainedObservations.test.ts +++ b/tests/boundary/providers/native/lldbRetainedObservations.test.ts @@ -66,6 +66,10 @@ describe("LLDB retained observations", () => { truncated: true, }); expect(partial.process.stderr).toBeNull(); + expect(partial.process).not.toHaveProperty("exit_status"); + expect(partial.process).not.toHaveProperty("terminated"); + expect(partial.target).not.toHaveProperty("launch_identity"); + expect(partial).not.toHaveProperty("breakpoints"); expect(partial.limitations).toContain( "The target stderr capture was unavailable.", ); diff --git a/tests/composition/analysis-sessions/binarySessionCleanupRetry.test.ts b/tests/composition/analysis-sessions/binarySessionCleanupRetry.test.ts index 76f13c5ac4..76ea8712da 100644 --- a/tests/composition/analysis-sessions/binarySessionCleanupRetry.test.ts +++ b/tests/composition/analysis-sessions/binarySessionCleanupRetry.test.ts @@ -27,12 +27,20 @@ const retryFixture = async ( const targets = await createBinarySessionTargets(); let allowed = false; let created = 0; + const runIds: string[] = []; const cleanupError = new ProviderCleanupError("fixture", [runtime], { reason: "owned runtime removal denied", leftover_paths: [runtime], }); const client: AnalysisClient = { execute, + operationHealthSnapshot: () => ({ + state: "unreachable", + stage: "connection", + retryAction: "restart_provider", + exitCode: null, + requests: [], + }), close: async (options) => { if (!allowed) return err(cleanupError); await rm(runtime, { recursive: true, force: true }); @@ -40,10 +48,31 @@ const retryFixture = async ( return resultOk(null); }, }; - const session = createTestBinarySession(() => { + const session = createTestBinarySession((_target, _profile, context) => { + if (context === undefined) throw new Error("missing analysis run context"); + runIds.push(context.runId); created += 1; return created === 1 - ? client + ? { + ...client, + runtimeLineageSnapshots: () => [ + { + provider: { + id: "fixture", + name: "Fixture analysis provider", + version: "1", + }, + observation: { + status: "unavailable" as const, + observedAt: "2026-10-16T00:00:00Z", + runId: context.runId, + launcherPid: 123, + processGroupId: 123, + reason: "fixture lineage unavailable", + }, + }, + ], + } : { execute: () => Promise.resolve(ok(null)), close: () => Promise.resolve(resultOk(null)), @@ -59,6 +88,7 @@ const retryFixture = async ( runtime, document, cleanupError, + runIds, allowCleanup: () => { allowed = true; }, @@ -83,19 +113,47 @@ it.each(["close", "switch", "open after close"] as const)( transition === "switch" ? await attempt() : await session.close(); expect(initial).toEqual(err(cleanupError)); expect(session.activeTarget()).toBeUndefined(); + expect(session.status()).toMatchObject({ + open: false, + cleanup_pending: true, + analysis_run: { + run_id: fixture.runIds[0], + process_lineage: { + status: "snapshots", + snapshots: [ + { observation: { reason: "fixture lineage unavailable" } }, + ], + }, + }, + provider_operation_health: { state: "unreachable", stage: "connection" }, + }); expect(await session.execute("address_name", {})).toMatchObject({ ok: false, }); await access(runtime); expect(await attempt()).toEqual(err(cleanupError)); + expect(session.status()).toMatchObject({ + analysis_run: { run_id: fixture.runIds[0] }, + }); expect(fixture.created()).toBe(1); await access(runtime); fixture.allowCleanup(); expect((await attempt()).ok).toBe(true); + expect(session.status()).toMatchObject({ cleanup_pending: false }); await expect(access(runtime)).rejects.toMatchObject({ code: "ENOENT" }); expect(fixture.created()).toBe(transition === "close" ? 1 : 2); + expect(session.status()).toMatchObject({ + analysis_run: + transition === "close" + ? null + : { + run_id: fixture.runIds[1], + process_lineage: { status: "not_observed" }, + }, + }); + expect(new Set(fixture.runIds).size).toBe(fixture.created()); }, ); @@ -115,6 +173,14 @@ it("keeps a failed candidate for cleanup and preserves its startup failure", asy }, }); expect(fixture.session.activeTarget()).toBeUndefined(); + expect(fixture.session.status()).toMatchObject({ + open: false, + cleanup_pending: true, + analysis_run: { + run_id: fixture.runIds[0], + process_lineage: { status: "snapshots" }, + }, + }); fixture.allowCleanup(); expect((await fixture.session.close()).ok).toBe(true); await expect(access(fixture.runtime)).rejects.toMatchObject({ @@ -140,6 +206,14 @@ it("retains cancelled startup cleanup and the cancellation reason", async () => diagnostics: { primary_error: { code: "cancelled" } }, }, }); + expect(fixture.session.status()).toMatchObject({ + open: false, + cleanup_pending: true, + analysis_run: { + run_id: fixture.runIds[0], + process_lineage: { status: "snapshots" }, + }, + }); fixture.allowCleanup(); expect((await fixture.session.close()).ok).toBe(true); await expect(access(fixture.runtime)).rejects.toMatchObject({ @@ -187,7 +261,10 @@ it("keeps failed restoration resources available for a close retry", async () => const [first, second] = await createBinarySessionTargets(); let created = 0; let allowed = false; - const session = createTestBinarySession(() => { + const runIds: string[] = []; + const session = createTestBinarySession((_target, _profile, context) => { + if (context === undefined) throw new Error("missing analysis run context"); + runIds.push(context.runId); const index = ++created; return { execute: () => @@ -213,6 +290,12 @@ it("keeps failed restoration resources available for a close retry", async () => error: { cleanupIncomplete: true, cleanupResources: [runtime] }, }); expect(session.activeTarget()).toBeUndefined(); + expect(session.status()).toMatchObject({ + open: false, + cleanup_pending: true, + analysis_run: { run_id: runIds[2] }, + }); + expect(new Set(runIds).size).toBe(3); allowed = true; expect((await session.close()).ok).toBe(true); await expect(access(runtime)).rejects.toMatchObject({ code: "ENOENT" }); diff --git a/tests/composition/analysis-sessions/compositeProvider.test.ts b/tests/composition/analysis-sessions/compositeProvider.test.ts index d41229f968..fd6d230ee8 100644 --- a/tests/composition/analysis-sessions/compositeProvider.test.ts +++ b/tests/composition/analysis-sessions/compositeProvider.test.ts @@ -2,7 +2,10 @@ import { writeFile } from "node:fs/promises"; import { join } from "node:path"; import { describe, expect, it } from "vitest"; -import { createTestBinarySession } from "../../fixtures/binarySession.js"; +import { + createBinarySessionTargets, + createTestBinarySession, +} from "../../fixtures/binarySession.js"; import { CompositeProvider } from "../../../src/application/binary/CompositeProvider.js"; import type { AnalysisOperation, @@ -133,6 +136,65 @@ describe("composite analysis provider", () => { }); }); +describe("composite provider health status", () => { + it("keeps a known failed child visible in public status when another child has no health snapshot", async () => { + const [targetPath] = await createBinarySessionTargets(); + const failedIdentity: ProviderIdentity = { + id: "failed-child", + name: "Failed child", + version: "1", + }; + const unobservedIdentity: ProviderIdentity = { + id: "unobserved-child", + name: "Unobserved child", + version: "1", + }; + let failed = false; + const failedProvider: AnalysisProvider = { + identity: () => failedIdentity, + capabilities: () => [capability(failedIdentity, "address_name")], + createClient: () => ({ + execute: async () => ok(createAnalysisExecution(null, failedIdentity)), + operationHealthSnapshot: () => ({ + state: failed ? "exited" : "idle", + stage: failed ? "connection" : null, + retryAction: failed ? "restart_provider" : null, + exitCode: failed ? 7 : null, + requests: [], + }), + close: async () => ok(null), + }), + }; + const unobservedProvider: AnalysisProvider = { + identity: () => unobservedIdentity, + capabilities: () => [capability(unobservedIdentity, "procedure_info")], + createClient: () => ({ + execute: async () => + ok(createAnalysisExecution(null, unobservedIdentity)), + close: async () => ok(null), + }), + }; + const session = createTestBinarySession( + new CompositeProvider([failedProvider, unobservedProvider]), + ); + + expect((await session.open(targetPath)).ok).toBe(true); + expect((await session.execute("address_name", {})).ok).toBe(true); + expect((await session.execute("procedure_info", {})).ok).toBe(true); + failed = true; + + expect(session.status()).toMatchObject({ + provider_operation_health: { + state: "exited", + stage: "connection", + retry_action: "restart_provider", + exit_code: 7, + }, + }); + await session.close(); + }); +}); + const dynamicProvider = ( id: string, operation: Exclude, diff --git a/tests/composition/directAnalysisCleanup.test.ts b/tests/composition/directAnalysisCleanup.test.ts index 6e5bc24efa..c5d9231605 100644 --- a/tests/composition/directAnalysisCleanup.test.ts +++ b/tests/composition/directAnalysisCleanup.test.ts @@ -24,6 +24,7 @@ const cleanupError = new ProviderCleanupError( "fixture", ["fixture-provider-process"], { reason: "process exit could not be confirmed" }, + { remediationAction: "Close the retained fixture process before retrying." }, ); const primaryError = new AnalysisTimeoutError("read_bytes", 100, { @@ -111,6 +112,9 @@ describe("one-shot analysis cleanup outcomes", () => { code: "cleanup_incomplete", category: primary.category, message: primary.message, + remediation: { + action: "Close the retained fixture process before retrying.", + }, details: { execution_failure: primary.code, captured_output: primary.details?.captured_output, @@ -147,6 +151,42 @@ describe("one-shot analysis cleanup outcomes", () => { }); }); + it.each([false, true])( + "retains original cleanup diagnostics when final close fails: %s", + async (closeFails) => { + const [path] = await createBinarySessionTargets(); + const executionError = new AnalysisTimeoutError("read_bytes", 100, { + cleanup: { + reason: "snapshot removal failed during execution", + resources: ["primary-snapshot"], + }, + capturedOutput: primaryError.capturedOutput, + }); + const primary = projectAnalysisError(executionError); + const { dependencies } = factories({ + executionError, + ...(closeFails ? { cleanupError } : {}), + }); + const result = analysisCliErrorEnvelopeSchema.parse( + await runDirectAnalysis(dependencies, path, "read_bytes", {}), + ); + expect(result).toMatchObject({ + code: "cleanup_incomplete", + details: { + ...primary.details, + ...(closeFails + ? { cleanup_error: projectAnalysisError(cleanupError) } + : {}), + }, + }); + expect( + JSON.stringify(result).match(/retained observation/g), + ).toHaveLength(1); + if (!closeFails) + expect(result.details).not.toHaveProperty("cleanup_error"); + }, + ); + it("retains both unexpected execution failure and cleanup failure", async () => { const [path] = await createBinarySessionTargets(); const thrownExecution = new Error("unexpected execution rejection"); @@ -190,6 +230,9 @@ describe("managed one-shot cleanup outcomes", () => { if (result.ok) return; expect(projectAnalysisError(result.error)).toMatchObject({ code: "cleanup_incomplete", + remediation: { + action: "Close the retained fixture process before retrying.", + }, details: { resources: ["fixture-provider-process"], diagnostics: { diff --git a/tests/fixtures/fakeHopper.mjs b/tests/fixtures/fakeHopper.mjs index 2491988a42..2015b6b2a0 100644 --- a/tests/fixtures/fakeHopper.mjs +++ b/tests/fixtures/fakeHopper.mjs @@ -229,6 +229,7 @@ const server = createServer((socket) => { request.method === "shutdown" || request.method === "shutdown_document" ) { + if (shutdownMode === "exit-without-ack") process.exit(0); send({ id: request.id, result: shutdownResult() }); if (shutdownMode !== "cleanup-required") setTimeout(closeServer, 2); } else if (request.method === "hang") { diff --git a/tests/fixtures/hopperBridgeReconnectProbe.py b/tests/fixtures/hopperBridgeReconnectProbe.py index ff462519d4..b02957f9c8 100644 --- a/tests/fixtures/hopperBridgeReconnectProbe.py +++ b/tests/fixtures/hopperBridgeReconnectProbe.py @@ -73,8 +73,24 @@ def probe(path, socket_path, mode): if "event" not in value: return value + def finish(shutdown_count): + worker.join(2) + assert not worker.is_alive(), "Bridge pinned its execution thread" + assert not failures, failures + assert not Path(socket_path).exists(), "Bridge retained its closed listener" + assert requests.count("shutdown_document") == shutdown_count + return {"mode": mode, "requests": requests, "listener_removed": True, "thread_released": True} + + if mode == "initial-expiry": + return finish(0) + # bind() creates the socket path before listen() makes it connectable. client, file = connect(wait_until_listening=True) + if mode == "silent-expiry": + result = finish(0) + file.close() + client.close() + return result send(file, "health", "foreign-token" if mode == "unauthorized" else "owned-token") if mode == "lost-reply": assert entered.wait(2) @@ -82,6 +98,10 @@ def probe(path, socket_path, mode): assert reply(file)["error"]["type"] == "authorization" else: assert reply(file)["result"]["run_id"] == "owned-run" + if mode == "authenticated-idle": + time.sleep(0.3) + send(file, "health") + assert reply(file)["result"]["run_id"] == "owned-run" file.close() client.close() release.set() @@ -95,14 +115,9 @@ def probe(path, socket_path, mode): assert reply(file)["result"]["document_closed"] is True file.close() client.close() - worker.join(2) - assert not worker.is_alive(), "Bridge pinned its execution thread" - assert not failures, failures - assert not Path(socket_path).exists(), "Bridge retained its closed listener" - assert requests.count("shutdown_document") == (0 if mode == "expiry" else 2) - return {"mode": mode, "requests": requests, "listener_removed": True, "thread_released": True} + return finish(0 if mode == "expiry" else 2) if __name__ == "__main__": print(json.dumps([probe(sys.argv[1], str(Path(sys.argv[2]) / (mode + ".sock")), mode) - for mode in ("unauthorized", "disconnect", "lost-reply", "expiry")])) + for mode in ("unauthorized", "disconnect", "lost-reply", "authenticated-idle", "expiry", "initial-expiry", "silent-expiry")])) diff --git a/tests/fixtures/processCleanupCancellation.mjs b/tests/fixtures/processCleanupCancellation.mjs new file mode 100644 index 0000000000..0c54210f11 --- /dev/null +++ b/tests/fixtures/processCleanupCancellation.mjs @@ -0,0 +1,26 @@ +import fs from "node:fs"; +import { syncBuiltinESMExports } from "node:module"; +import { basename } from "node:path"; + +const remove = fs.promises.rm; +let observed = false; +fs.promises.rm = async function (path, ...args) { + const name = basename(String(path)); + if ( + observed || + !name.startsWith("rea-process-") || + name.startsWith("rea-process-token-") + ) + return remove.call(this, path, ...args); + observed = true; + if (process.env.REA_TEST_CANCEL_DURING_CLEANUP === "1") + process.kill(process.pid, "SIGINT"); + // Give the CLI signal handler a turn while real resource cleanup is pending. + await new Promise((resolve) => setTimeout(resolve, 100)); + await remove.call(this, path, ...args); + await fs.promises.writeFile( + process.env.REA_TEST_CLEANUP_MARKER, + JSON.stringify({ path: String(path), removed: true }), + ); +}; +syncBuiltinESMExports();