morluto/rea

Fork 0

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

[PR #3466] [CLOSED] fix(electron): block app.moveToApplicationsFolder during capture #5757

New issue

Closed

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

bsakaguchi commented

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

Owner

📋 Pull Request Information

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

Base: main ← Head: fix/capture-electron-block-moveToApplicationsFolder

📝 Commits (7)

6abd592 fix(electron): block app.moveToApplicationsFolder during capture
09729c0 fix(electron): assert the blocked move never reaches the original
4612b27 fix(electron): consolidate moveToApplicationsFolder under updater classification
b06f839 Merge upstream main ce89706145 into fix/capture-electron-block-moveToApplicationsFolder
8ee83de fix(electron): migrate moveToApplicationsFolder test to current hook harness
45d1284 fix(electron): record blocked moveToApplicationsFolder as app-lifecycle
902167a Merge upstream main into fix/capture-electron-block-moveToApplicationsFolder

📊 Changes

2 files changed (+68 additions, -3 deletions)

View changed files

📝 scripts/electron-active-hook-boundaries.cjs (+10 -2)
📝 tests/boundary/browser/electronActiveHook.test.ts (+58 -1)

📄 Description

Summary

Fixes #3452. capture-electron-scenario now blocks app.moveToApplicationsFolder like the other OS-integration effects, instead of letting the target move the operator's Electron runtime into /Applications mid-capture.

Problem and expected behavior

The active hook blocked app.relaunch, setAsDefaultProtocolClient, and setLoginItemSettings, but a target calling app.moveToApplicationsFolder(...) during capture still executed it: Electron moved the runtime out of the scratch directory, the app quit, and the capture died with a closed-browser error. The docs say the hook "blocks and records ... OS-integration effects" (docs/electron-observation.md).

Change and scope

Added moveToApplicationsFolder to the blocked app methods in patchApplicationEffects (scripts/electron-active-hook-boundaries.cjs).
The blocked call is recorded as an app-lifecycle timeline event, matching the classification used in #3471 as requested, so both PRs share one event kind; relaunch stays updater. It returns false, matching Electron's boolean return where false means the move did not happen.
Out of scope: shell-side or other methods, docs changes (the docs already describe this behavior).

Contract and boundary impact

Semantic owner and earliest changed stage: active-hook boundary patches, scripts/electron-active-hook-boundaries.cjs.
Contribution scope fit and linked maintainer decision: ordinary fix within existing boundaries; event classification aligned with #3471 per the maintainer's consolidation request.
CLI and MCP/tool-catalog contract: none.
Timeline event kind stays inside the existing electronActiveTimelineEventSchema enum (app-lifecycle); no schema change.
Expected effects: none of the listed effects; the change only stops the target process from relocating itself.

Evidence and regression coverage

Tests added: new case in tests/boundary/browser/electronActiveHook.test.ts using the current runNode and writeElectronFixture helpers. It asserts the call is blocked, returns false, does not run the conflict handler, records an app-lifecycle event with phase blocked, and that relaunch is still recorded as updater. The retention test now pins the updater expectation to relaunch.
Remaining proof gaps: no real-Electron run on macOS, where the method has its real effect; the boundary test uses the repo's fake Electron module, like the existing hook tests.

Validation performed

npx vitest run tests/boundary/browser/electronActiveHook.test.ts
npm run check:fast (typecheck and lint)
npm run verify:test-discovery

Compatibility, safety, and release

No breaking changes. A blocked moveToApplicationsFolder now returns false instead of moving; the target keeps running in place, which is the intended capture containment.
Real Hopper/Ghidra, browser, or OS coverage: none ran (no macOS host here); fake-module boundary test only.

Author readiness

Human owner: @sx4im

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.

_{🔄 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/3466 **Author:** [@sx4im](https://github.com/sx4im) **Created:** 10/14/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/capture-electron-block-moveToApplicationsFolder` --- ### 📝 Commits (7) - [`6abd592`](https://github.com/morluto/rea/commit/6abd5929bec5bd123d999df6c75a728dc93445c4) fix(electron): block app.moveToApplicationsFolder during capture - [`09729c0`](https://github.com/morluto/rea/commit/09729c01ff241a3cf36e9ad8f531c970968ce904) fix(electron): assert the blocked move never reaches the original - [`4612b27`](https://github.com/morluto/rea/commit/4612b277650d22d41b2bedec2fe9d789dd8906a9) fix(electron): consolidate moveToApplicationsFolder under updater classification - [`b06f839`](https://github.com/morluto/rea/commit/b06f83931de5fc73563bcc9bddc4cab634a81c6b) Merge upstream main ce89706145 into fix/capture-electron-block-moveToApplicationsFolder - [`8ee83de`](https://github.com/morluto/rea/commit/8ee83dee98c961921bad984271f78e447a2685dd) fix(electron): migrate moveToApplicationsFolder test to current hook harness - [`45d1284`](https://github.com/morluto/rea/commit/45d12841728d722d25d90365cb0a597281f3cb56) fix(electron): record blocked moveToApplicationsFolder as app-lifecycle - [`902167a`](https://github.com/morluto/rea/commit/902167aa99f8e27ccc4bf3658ceea5436f449074) Merge upstream main into fix/capture-electron-block-moveToApplicationsFolder ### 📊 Changes **2 files changed** (+68 additions, -3 deletions) <details> <summary>View changed files</summary> 📝 `scripts/electron-active-hook-boundaries.cjs` (+10 -2) 📝 `tests/boundary/browser/electronActiveHook.test.ts` (+58 -1) </details> ### 📄 Description ## Summary Fixes #3452. `capture-electron-scenario` now blocks `app.moveToApplicationsFolder` like the other OS-integration effects, instead of letting the target move the operator's Electron runtime into /Applications mid-capture. ## Problem and expected behavior The active hook blocked `app.relaunch`, `setAsDefaultProtocolClient`, and `setLoginItemSettings`, but a target calling `app.moveToApplicationsFolder(...)` during capture still executed it: Electron moved the runtime out of the scratch directory, the app quit, and the capture died with a closed-browser error. The docs say the hook "blocks and records ... OS-integration effects" (docs/electron-observation.md). ## Change and scope - Added `moveToApplicationsFolder` to the blocked `app` methods in `patchApplicationEffects` (scripts/electron-active-hook-boundaries.cjs). - The blocked call is recorded as an `app-lifecycle` timeline event, matching the classification used in #3471 as requested, so both PRs share one event kind; `relaunch` stays `updater`. It returns `false`, matching Electron's boolean return where false means the move did not happen. - Out of scope: shell-side or other methods, docs changes (the docs already describe this behavior). ## Contract and boundary impact - Semantic owner and earliest changed stage: active-hook boundary patches, scripts/electron-active-hook-boundaries.cjs. - Contribution scope fit and linked maintainer decision: ordinary fix within existing boundaries; event classification aligned with #3471 per the maintainer's consolidation request. - CLI and MCP/tool-catalog contract: none. - Timeline event kind stays inside the existing `electronActiveTimelineEventSchema` enum (`app-lifecycle`); no schema change. - Expected effects: none of the listed effects; the change only stops the target process from relocating itself. ## Evidence and regression coverage - Tests added: new case in tests/boundary/browser/electronActiveHook.test.ts using the current `runNode` and `writeElectronFixture` helpers. It asserts the call is blocked, returns false, does not run the conflict handler, records an `app-lifecycle` event with phase `blocked`, and that `relaunch` is still recorded as `updater`. The retention test now pins the `updater` expectation to `relaunch`. - Remaining proof gaps: no real-Electron run on macOS, where the method has its real effect; the boundary test uses the repo's fake Electron module, like the existing hook tests. ## Validation performed - `npx vitest run tests/boundary/browser/electronActiveHook.test.ts` - `npm run check:fast` (typecheck and lint) - `npm run verify:test-discovery` ## Compatibility, safety, and release - No breaking changes. A blocked `moveToApplicationsFolder` now returns false instead of moving; the target keeps running in place, which is the intended capture containment. - Real Hopper/Ghidra, browser, or OS coverage: none ran (no macOS host here); fake-module boundary test only. ## Author readiness Human owner: @sx4im - [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. --- <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:12 +00:00

closed this issue
added the
pull-request
label

bsakaguchi referenced this issue

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

[PR #5757] [CLOSED] fix(setup): preserve equivalent launcher registrations #7042

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

No description provided.

Rows
Columns