[PR #1927] [MERGED] fix(hopper): preserve observed process failures across requests #4916

Closed
opened 2026-10-31 00:56:46 +00:00 by bsakaguchi · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/1927
Author: @morluto
Created: 10/11/2026
Status: ✅ Merged
Merged: 10/11/2026
Merged by: @morluto

Base: main ← Head: codex/refactor-boundary-ownership


📝 Commits (3)

  • c9e44ee fix(hopper): preserve observed process failures across requests
  • 814877e test(hopper): consolidate retained cleanup and recover failed helper fixtures
  • aab78a6 docs(hopper): clarify first-use demo prompt and startup recovery

📊 Changes

17 files changed (+339 additions, -263 deletions)

View changed files

📝 README.md (+3 -1)
📝 docs/installation.md (+12 -2)
📝 src/domain/analysisErrorBase.ts (+3 -3)
📝 src/domain/analysisErrorPresentation.ts (+4 -1)
📝 src/domain/analysisErrorProjection.test.ts (+5 -2)
📝 src/domain/hopperErrors.ts (+34 -16)
📝 src/hopper/BridgeLauncher.ts (+3 -4)
📝 src/hopper/HopperCleanup.ts (+1 -3)
📝 src/hopper/HopperClient.ts (+44 -44)
📝 src/hopper/HopperRequestQueue.ts (+18 -19)
📝 tests/boundary/mcp/toolResult.test.ts (+16 -14)
📝 tests/boundary/providers/hopper/hopperClient.cleanup.test.ts (+0 -62)
📝 tests/boundary/providers/hopper/hopperClient.cleanupRetry.test.ts (+64 -43)
📝 tests/boundary/providers/hopper/hopperClient.health.test.ts (+62 -7)
📝 tests/boundary/providers/hopper/hopperClient.helperFailure.test.ts (+60 -17)
📝 tests/boundary/providers/hopper/hopperClient.startupFailures.test.ts (+3 -25)
📝 tests/fixtures/fakeHopper.mjs (+7 -0)

📄 Description

Summary

When an owned Hopper process stopped during a request, queue enrichment dropped its launcher diagnostics, and later requests reduced the observed exit to an unreachable socket. Preserve the exit code or signal, bounded output, and output-closure uncertainty for active, queued, and later callers. Related to #1808; that issue remains open because external GUI crash recovery is a separate ownership problem.

Problem and expected behavior

A signal exit has no numeric exit code. Callers should still receive the observed signal and an exited provider state, with their own operation and request ID. A native GUI helper's exit must remain distinct from an observed GUI exit.

Change and scope

Use named error context instead of seven positional arguments, preserve typed diagnostic context when correlating queued requests, and retain owned-process exit facts at the request boundary. Document the resulting diagnostics, the macOS first-use demo/license prompt, and pending-dialog troubleshooting in the README and installation guide.

Consolidate redundant external-document cleanup tests and recover failed helper fixtures through their exact authenticated bridge during teardown. This removes repeated 30-second cleanup waits while retaining negative acknowledgement, missing transport, backing-image retention, helper ownership, and cleanup retry coverage.

Contract and boundary impact

  • Semantic owner and earliest changed stage: Hopper process supervision and request correlation.
  • CLI and MCP/tool-catalog contract: additive details.launcher evidence on owned runtime failures; signal-aware failure messages; no catalog changes.
  • Provider, bridge, target-format, or platform compatibility: shared Hopper client; native GUI helper exits remain unknown GUI lifetime.
  • Evidence, artifact, provenance, or reconstruction contract: retain observed exit/output and per-request identity; preserve explicit stream-closure uncertainty.
  • Process execution, authorization, cleanup, or containment impact: no new process authority; tests use authenticated cleanup instead of assuming an external GUI exited with its helper.
  • Generated metadata, package, or installation impact: none; installation documentation explains diagnostics.

Evidence and regression coverage

  • Tests added or updated: numeric exit evidence; active/queued/later signal failure correlation; exact transport credential redaction while preserving source-looking values; disconnected external bridge recovery; consolidated cleanup ownership and retained resources.

  • Base reproduction: the numeric exit regression failed on the affected base because details.launcher was absent.

  • User-visible CLI/MCP output: provider_unavailable, provider_state: exited, observed signal in the message and details.launcher, operation-specific stage/request identity.

  • Remaining proof gaps: no real native GUI crash was induced. Native helper lifetime does not prove GUI lifetime.

  • Observed, derived, and inferred claims remain distinguishable.

  • Artifact identity, source provenance, and failed attempts remain preserved.

  • Unsupported, incomplete, unavailable, or uncertain outcomes remain visible.

Validation performed

  • npm run check — passed, including typecheck, lint, formatting, dependency/metadata checks and architecture guards.
  • Selected domain, request queue, cleanup, Hopper lifecycle/protocol/startup/launcher and MCP boundary tests — 152 tests in 13 files passed in 9.47 seconds.
  • npm run docs:check and npm run build:cached — passed.
  • npm run verify:fixtures — seven source-owned macOS fixtures passed.
  • node scripts/verify-real-hopper.mjs — passed against installed Hopper 6.6.0-demo after the user accepted its demo prompt: 199 successful boundary calls, 62 rejected calls, 172 schemas, complete 1,205-item procedure/string inventories, timeout/cancellation recovery, target switching and CLI/MCP parity. Native document closure, unrelated-process survival and empty owned descendant lineage verified.
  • The first cold startup failed with helper exit 2 (Cannot launch Hopper! Connection is invalid.); REA preserved its diagnostics and retained unconfirmed resources. The successful subsequent run does not erase that failed attempt.

Compatibility, safety, and release

  • Breaking changes or migration steps: none for CLI/MCP callers; internal constructor call sites migrated together.
  • Real Hopper/Ghidra, browser, or OS coverage: real macOS arm64 Hopper exercised above; other providers unchanged.
  • Package or release metadata impact: none.
  • Security, privacy, process, or containment review: bounded output retains local evidence and redacts only the transport credential; no GUI termination or unrelated-document cleanup authority added.

Review checklist

  • This PR addresses a concrete problem or an agreed enhancement.
  • The PR has one focused outcome and the title follows type(scope): outcome.
  • Related issue is linked, or the reason for not linking one is stated above.
  • Tests cover changed observable behavior and meaningful failure paths.
  • Owning docs, contracts, and generated metadata are updated where needed.
  • User-visible CLI/MCP changes include representative output.
  • I checked the final diff for secrets, unrelated cleanup, and unsupported claims.

🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/morluto/rea/pull/1927 **Author:** [@morluto](https://github.com/morluto) **Created:** 10/11/2026 **Status:** ✅ Merged **Merged:** 10/11/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `codex/refactor-boundary-ownership` --- ### 📝 Commits (3) - [`c9e44ee`](https://github.com/morluto/rea/commit/c9e44ee3c3ffc6ad8868d0ec94c516a8d9d5fc91) fix(hopper): preserve observed process failures across requests - [`814877e`](https://github.com/morluto/rea/commit/814877ec85c2015bc376c8d00c565434cff2d5ca) test(hopper): consolidate retained cleanup and recover failed helper fixtures - [`aab78a6`](https://github.com/morluto/rea/commit/aab78a6b41fbcbc9730473ff03cb62e9957d07fb) docs(hopper): clarify first-use demo prompt and startup recovery ### 📊 Changes **17 files changed** (+339 additions, -263 deletions) <details> <summary>View changed files</summary> 📝 `README.md` (+3 -1) 📝 `docs/installation.md` (+12 -2) 📝 `src/domain/analysisErrorBase.ts` (+3 -3) 📝 `src/domain/analysisErrorPresentation.ts` (+4 -1) 📝 `src/domain/analysisErrorProjection.test.ts` (+5 -2) 📝 `src/domain/hopperErrors.ts` (+34 -16) 📝 `src/hopper/BridgeLauncher.ts` (+3 -4) 📝 `src/hopper/HopperCleanup.ts` (+1 -3) 📝 `src/hopper/HopperClient.ts` (+44 -44) 📝 `src/hopper/HopperRequestQueue.ts` (+18 -19) 📝 `tests/boundary/mcp/toolResult.test.ts` (+16 -14) 📝 `tests/boundary/providers/hopper/hopperClient.cleanup.test.ts` (+0 -62) 📝 `tests/boundary/providers/hopper/hopperClient.cleanupRetry.test.ts` (+64 -43) 📝 `tests/boundary/providers/hopper/hopperClient.health.test.ts` (+62 -7) 📝 `tests/boundary/providers/hopper/hopperClient.helperFailure.test.ts` (+60 -17) 📝 `tests/boundary/providers/hopper/hopperClient.startupFailures.test.ts` (+3 -25) 📝 `tests/fixtures/fakeHopper.mjs` (+7 -0) </details> ### 📄 Description ## Summary When an owned Hopper process stopped during a request, queue enrichment dropped its launcher diagnostics, and later requests reduced the observed exit to an unreachable socket. Preserve the exit code or signal, bounded output, and output-closure uncertainty for active, queued, and later callers. Related to #1808; that issue remains open because external GUI crash recovery is a separate ownership problem. ## Problem and expected behavior A signal exit has no numeric exit code. Callers should still receive the observed signal and an `exited` provider state, with their own operation and request ID. A native GUI helper's exit must remain distinct from an observed GUI exit. ## Change and scope Use named error context instead of seven positional arguments, preserve typed diagnostic context when correlating queued requests, and retain owned-process exit facts at the request boundary. Document the resulting diagnostics, the macOS first-use demo/license prompt, and pending-dialog troubleshooting in the README and installation guide. Consolidate redundant external-document cleanup tests and recover failed helper fixtures through their exact authenticated bridge during teardown. This removes repeated 30-second cleanup waits while retaining negative acknowledgement, missing transport, backing-image retention, helper ownership, and cleanup retry coverage. ## Contract and boundary impact - Semantic owner and earliest changed stage: Hopper process supervision and request correlation. - CLI and MCP/tool-catalog contract: additive `details.launcher` evidence on owned runtime failures; signal-aware failure messages; no catalog changes. - Provider, bridge, target-format, or platform compatibility: shared Hopper client; native GUI helper exits remain unknown GUI lifetime. - Evidence, artifact, provenance, or reconstruction contract: retain observed exit/output and per-request identity; preserve explicit stream-closure uncertainty. - Process execution, authorization, cleanup, or containment impact: no new process authority; tests use authenticated cleanup instead of assuming an external GUI exited with its helper. - Generated metadata, package, or installation impact: none; installation documentation explains diagnostics. ## Evidence and regression coverage - Tests added or updated: numeric exit evidence; active/queued/later signal failure correlation; exact transport credential redaction while preserving source-looking values; disconnected external bridge recovery; consolidated cleanup ownership and retained resources. - Base reproduction: the numeric exit regression failed on the affected base because `details.launcher` was absent. - User-visible CLI/MCP output: `provider_unavailable`, `provider_state: exited`, observed signal in the message and `details.launcher`, operation-specific stage/request identity. - Remaining proof gaps: no real native GUI crash was induced. Native helper lifetime does not prove GUI lifetime. - [x] Observed, derived, and inferred claims remain distinguishable. - [x] Artifact identity, source provenance, and failed attempts remain preserved. - [x] Unsupported, incomplete, unavailable, or uncertain outcomes remain visible. ## Validation performed - `npm run check` — passed, including typecheck, lint, formatting, dependency/metadata checks and architecture guards. - Selected domain, request queue, cleanup, Hopper lifecycle/protocol/startup/launcher and MCP boundary tests — 152 tests in 13 files passed in 9.47 seconds. - `npm run docs:check` and `npm run build:cached` — passed. - `npm run verify:fixtures` — seven source-owned macOS fixtures passed. - `node scripts/verify-real-hopper.mjs` — passed against installed Hopper 6.6.0-demo after the user accepted its demo prompt: 199 successful boundary calls, 62 rejected calls, 172 schemas, complete 1,205-item procedure/string inventories, timeout/cancellation recovery, target switching and CLI/MCP parity. Native document closure, unrelated-process survival and empty owned descendant lineage verified. - The first cold startup failed with helper exit 2 (`Cannot launch Hopper! Connection is invalid.`); REA preserved its diagnostics and retained unconfirmed resources. The successful subsequent run does not erase that failed attempt. ## Compatibility, safety, and release - Breaking changes or migration steps: none for CLI/MCP callers; internal constructor call sites migrated together. - Real Hopper/Ghidra, browser, or OS coverage: real macOS arm64 Hopper exercised above; other providers unchanged. - Package or release metadata impact: none. - Security, privacy, process, or containment review: bounded output retains local evidence and redacts only the transport credential; no GUI termination or unrelated-document cleanup authority added. ## Review checklist - [x] This PR addresses a concrete problem or an agreed enhancement. - [x] The PR has one focused outcome and the title follows `type(scope): outcome`. - [x] Related issue is linked, or the reason for not linking one is stated above. - [x] Tests cover changed observable behavior and meaningful failure paths. - [x] Owning docs, contracts, and generated metadata are updated where needed. - [x] User-visible CLI/MCP changes include representative output. - [x] I checked the final diff for secrets, unrelated cleanup, and unsupported claims. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
bsakaguchi 2026-10-31 00:56:46 +00:00
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
morluto/rea#4916
No description provided.