morluto/rea

Fork 0

mirror of https://github.com/morluto/rea.git synced 2026-11-05 06:45:22 +00:00

[PR #3472] [CLOSED] fix(setup): keep the completed Hopper installation when the final open fails #5763

New issue

Closed

opened 2026-10-31 01:09:14 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 01:09:14 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/3472
Author: @LuckVd
Created: 10/14/2026
Status: ❌ Closed

Base: main ← Head: fix/3209-hopper-open-failure

📝 Commits (2)

59bf160 fix(setup): keep the completed Hopper installation when the final open fails
63418b3 Merge remote-tracking branch 'origin/main' into sweep/3472

📄 Description

Summary

On macOS, rea setup --install-hopper verified, copied, and launcher-checked the Hopper bundle, then let a failure of the final open <installed.app> convenience command fall into the shared install-stage catch, which maps every throw to reason: "download". A completed installation was therefore reported as download_failed / needs_human, told the user to check the network, and never recorded installed_hopper in appliedActions. Fixes #3209.

Problem and expected behavior

Smallest trigger: successful install whose final open command exits non-zero (for example no GUI session to activate the app).
Violated invariant: a verified, launcher-ready installation is a completed install fact; a post-install convenience failure must not reclassify it.
Expected: setup reports the installation as applied with a retained warning, instead of a download failure.

Change and scope

installMacHopper now treats openApplication as best-effort: an open failure is captured as Hopper application open failed: … on the installed result's existing cleanupFailure warning channel, and the final warning merge combines it with a DMG cleanup failure instead of either overwriting the other. Everything before the open step keeps its distinct failure reasons; the shared catch remains for genuinely install-stage throws. No new capability and no installer behavior change — only result classification of the final step. Contribution scope: not applicable (ordinary fix within existing boundaries).

Contract and boundary impact

Semantic owner and earliest changed stage: the macOS Hopper installer owns the open step and produces the warning; Setup.ts keeps consuming the installed result's existing warning channel (installed_hopper is now recorded and progress state becomes warning).
Contribution scope fit and linked maintainer decision: not applicable.
CLI and MCP/tool-catalog contract: the failing-open scenario's setup result changes from failed/download_failed to installed with a warning detail (source-derived shape below); tool catalog untouched.
Provider, bridge, target-format, or platform compatibility: macOS setup only; no change when the open succeeds.
Evidence, artifact, provenance, or reconstruction contract: none.
Generated metadata: none.

Expected effects:

Read existing files or captured data.

rea setup's install effects are unchanged; only the classification/reporting of the final open step changed.

Representative output shape (source-derived from the new tests, not executed on macOS):

{
  "status": "installed",
  "appliedActions": ["installed_hopper"],
  "progress": { "state": "warning", "detail": "<launcher path>; Hopper application open failed: …" }
}

Evidence and regression coverage

Tests added or updated: MacHopper.test.ts — a failed final open now resolves installed with the open warning while DMG cleanup still runs; open + DMG-cleanup failures combine as "Hopper application open failed: …; Hopper DMG cleanup failed: …".
Base reproduction: source-derived — on the base revision the open throw lands in the shared catch mapped to reason: "download" (MacHopper.ts install-stage catch), which setupInstallFailure renders as download_failed; #3209 documents the same path from a real macOS run.
Remaining proof gaps: validation machine is Linux; the real open behavior on macOS was not exercised here. Coverage is fixture-level against the installer contract.

Validation performed

npm run test:local -- src/application/MacHopper.test.ts src/application/Setup.configuration.test.ts — 28 passed.
npm run verify:test-discovery — passed.
npm run check:fast (typecheck, lint) — passed (2 tasks, not cached).

Compatibility, safety, and release

Breaking changes or migration steps: none; the failing-open scenario's result becomes more accurate (installed + warning instead of failed).
Real Hopper/Ghidra, browser, or OS coverage: macOS Hopper run not available on this machine; see remaining proof gaps.
Package or release metadata impact: none.
Security, privacy, process, or containment review: none needed; no new commands or effects beyond the existing open step.

Author readiness

Human owner: @LuckVd

I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work.
I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps.
I declare this PR ready for initial maintainer review.

Before merge

This PR addresses a concrete problem or an agreed enhancement.
Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff.
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/3472 **Author:** [@LuckVd](https://github.com/LuckVd) **Created:** 10/14/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/3209-hopper-open-failure` --- ### 📝 Commits (2) - [`59bf160`](https://github.com/morluto/rea/commit/59bf160298ad6caec00011d40000accb3b98258c) fix(setup): keep the completed Hopper installation when the final open fails - [`63418b3`](https://github.com/morluto/rea/commit/63418b32c4c5dd30f7afe5529a1c7e8b903895a8) Merge remote-tracking branch 'origin/main' into sweep/3472 ### 📄 Description  ## Summary On macOS, `rea setup --install-hopper` verified, copied, and launcher-checked the Hopper bundle, then let a failure of the final `open <installed.app>` convenience command fall into the shared install-stage catch, which maps every throw to `reason: "download"`. A completed installation was therefore reported as `download_failed` / `needs_human`, told the user to check the network, and never recorded `installed_hopper` in `appliedActions`. Fixes #3209. ## Problem and expected behavior - Smallest trigger: successful install whose final `open` command exits non-zero (for example no GUI session to activate the app). - Violated invariant: a verified, launcher-ready installation is a completed install fact; a post-install convenience failure must not reclassify it. - Expected: setup reports the installation as applied with a retained warning, instead of a download failure. ## Change and scope `installMacHopper` now treats `openApplication` as best-effort: an open failure is captured as `Hopper application open failed: …` on the installed result's existing `cleanupFailure` warning channel, and the final warning merge combines it with a DMG cleanup failure instead of either overwriting the other. Everything before the open step keeps its distinct failure reasons; the shared catch remains for genuinely install-stage throws. No new capability and no installer behavior change — only result classification of the final step. Contribution scope: not applicable (ordinary fix within existing boundaries). ## Contract and boundary impact - Semantic owner and earliest changed stage: the macOS Hopper installer owns the open step and produces the warning; `Setup.ts` keeps consuming the installed result's existing warning channel (`installed_hopper` is now recorded and progress state becomes `warning`). - Contribution scope fit and linked maintainer decision: not applicable. - CLI and MCP/tool-catalog contract: the failing-open scenario's setup result changes from `failed`/`download_failed` to `installed` with a warning detail (source-derived shape below); tool catalog untouched. - Provider, bridge, target-format, or platform compatibility: macOS setup only; no change when the open succeeds. - Evidence, artifact, provenance, or reconstruction contract: none. - Generated metadata: none. Expected effects: - [x] Read existing files or captured data. `rea setup`'s install effects are unchanged; only the classification/reporting of the final open step changed. Representative output shape (source-derived from the new tests, not executed on macOS): ```json { "status": "installed", "appliedActions": ["installed_hopper"], "progress": { "state": "warning", "detail": "<launcher path>; Hopper application open failed: …" } } ``` ## Evidence and regression coverage - Tests added or updated: `MacHopper.test.ts` — a failed final open now resolves `installed` with the open warning while DMG cleanup still runs; open + DMG-cleanup failures combine as `"Hopper application open failed: …; Hopper DMG cleanup failed: …"`. - Base reproduction: source-derived — on the base revision the open throw lands in the shared catch mapped to `reason: "download"` (`MacHopper.ts` install-stage catch), which `setupInstallFailure` renders as `download_failed`; #3209 documents the same path from a real macOS run. - Remaining proof gaps: validation machine is Linux; the real `open` behavior on macOS was not exercised here. Coverage is fixture-level against the installer contract. ## Validation performed - `npm run test:local -- src/application/MacHopper.test.ts src/application/Setup.configuration.test.ts` — 28 passed. - `npm run verify:test-discovery` — passed. - `npm run check:fast` (typecheck, lint) — passed (2 tasks, not cached). ## Compatibility, safety, and release - Breaking changes or migration steps: none; the failing-open scenario's result becomes more accurate (installed + warning instead of failed). - Real Hopper/Ghidra, browser, or OS coverage: macOS Hopper run not available on this machine; see remaining proof gaps. - Package or release metadata impact: none. - Security, privacy, process, or containment review: none needed; no new commands or effects beyond the existing `open` step. ## Author readiness  Human owner: @LuckVd - [x] I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work. - [x] I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps. - [x] I declare this PR ready for initial maintainer review. ## Before merge  - [ ] This PR addresses a concrete problem or an agreed enhancement. - [ ] Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff. - [ ] 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. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-31 01:09:14 +00:00

closed this issue
added the
pull-request
label

bsakaguchi referenced this issue

2026-11-01 21:44:56 +00:00

[PR #5763] [MERGED] fix(artifacts): report target applicability in session discovery #7044

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#5763

No description provided.

Rows
Columns