[PR #3559] [CLOSED] fix(electron): block and record shell.trashItem #5823

Closed
opened 2026-10-31 01:09:42 +00:00 by bsakaguchi · 0 comments
Owner

📋 Pull Request Information

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

Base: main ← Head: fix/electron-hook-trash-item


📝 Commits (10+)

  • 22ec8bd Merge remote-tracking branch 'origin/main' into codex/ci-followups-20261015
  • 57f3002 fix(javascript): reject Webpack getters replaced through any identity write (#3860)
  • ab45bf6 fix(ghidra): refuse PSP targets with unexamined ABI declarations (#3904)
  • fc712b3 fix(javascript): keep releases through a replaced handle as candidates (#3897)
  • c61a24b fix(browser): bound CDP handshakes and command responses (#3555)
  • d94ef22 fix(cli): route app bundles with trailing separators to native analysis (#3511)
  • 3e88e21 fix(electron): reject active scenario capture when inspect fuse is disabled (#3453) (#3605)
  • ce89706 fix(javascript): preserve exact static property keys (#3480)
  • 73c72d5 fix(mcp): keep a snapshot open ahead of later calls on the same connection (#3661)
  • f5b1dce docs: celebrate 250,000 GitHub stars 🎉 (#3971)

📊 Changes

1145 files changed (+140341 additions, -13392 deletions)

View changed files

📝 .agents/skills/reverse-engineer-anything/SKILL.md (+16 -2)
📝 .agents/skills/reverse-engineer-anything/references/android-applications.md (+3 -1)
📝 .agents/skills/reverse-engineer-anything/references/javascript-applications.md (+44 -1)
📝 .agents/skills/reverse-engineer-anything/references/native-and-artifacts.md (+8 -6)
📝 .github/workflows/ci.yml (+16 -1)
➕ .github/workflows/ghidra-bridge-sources.yml (+48 -0)
📝 .github/workflows/real-cutter.yml (+3 -3)
📝 .github/workflows/real-packet-capture.yml (+1 -1)
📝 AGENTS.md (+2 -0)
📝 CONTRIBUTING.md (+21 -1)
📝 README.md (+3 -2)
📝 THIRD_PARTY_NOTICES.md (+1 -0)
📝 bridge/cutter/rea_bridge.py (+6 -5)
📝 bridge/hopper_bridge.py (+9 -1)
📝 bridge/native/rea_lldb_tracer.py (+48 -4)
📝 bridge/native/test_rea_lldb_tracer.py (+132 -0)
📝 docs/.vitepress/config.ts (+1 -0)
📝 docs/adb-device-analysis.md (+18 -4)
📝 docs/android-analysis.md (+19 -2)
📝 docs/binary-diagnostics.md (+18 -0)

...and 80 more files

📄 Description

Summary

Fixes #3474. The active hook omitted shell.trashItem, allowing a target to reach native trash dispatch without a blocked attempt. Add it to the existing shell return-contract table: reject the promise and record the existing shape-only event. Preserve the synchronous shell contracts and original contributor @chengwudi1's work.

Scope and approach

The shell patch remains the shared producer for CLI/MCP. No tool/schema/cache or target authority changes. This is an ordinary fix within existing instrumentation scope; a new capability decision is not applicable. Replace the redundant direct mock-context unit with stronger staged-hook coverage. Save a reproducible Linux real-app runner and the selectable resource budget (4 GiB default preserved; explicit8 GiB supported).

Executed evidence

  • Fresh-main regression fails: the native trash stub executes and resolves. 88 focused hook/projection/MCP cases pass after the fix, covering async rejection, suppressed native effects, sync return contracts and retained-event shapes. Controlled MCP cases do not establish a real MCP provider run.
  • Real unchanged VSCodium1.140.06880 UI, explicit official Electron43.7.3: the same Explorer delete action gives native invocations 1→0, blocked attempts 0→1, and the owned22,177 B README copy remains byte-identical after blocking. The native-call observer preserves receiver, arguments and return object. Native OS Trash completion on the baseline is not established.
  • This real-app comparison uses the production shell factory with a bounded test recorder. Full hook/provider/CLI/MCP workflow NOT_EXECUTED there. Earlier packaged-preload, Inspector and navigation failures are preserved; #2974/#4494/#4282 and raw-wrapper mutation #4513 remain separate. No application callbacks, IPC handlers or vendor bytes were replaced.
  • npm run check:fast: 2 tasks pass (initial run uncached; final run one cached). npm run docs:check: pass. Default4 GiB resource smoke: pass. Real Windows/macOS and full check:pr were not run.

Candidate resource scope: 15.030s, observed CPU 14.738146s, peak 1,092,239,360 B; complete-family observation unverified. Window-close API completes; all owned scopes are independently inactive/dead. Heavy work was serial,1CPU/8GiB hard/6GiB early/no swap/192tasks. Peak includes file cache; counters are at payload exit, not final whole-scope counters. Baseline waits for absent blocked evidence, so no speed comparison or token benefit is claimed. Agent costs and cold/warm repeats are unmeasured.

Verification, reproduction and saved cohorts and adjacent JSON retain source/input hashes and failures. Regenerable apps/runtimes/profiles/raw logs are excluded. AI-assisted updates above preserve the author's original initial-readiness record below; they do not attest to a new human review.

Current CI and review

Run38983861225 fails the binary-target cancelled-admission cleanup assertion (RegularFileOperationCancelled versus the exact caller Error); Portable tests/CI required fail in consequence. Reproduced on clean, rebuilt actual main parent0c7b6d834: 9 pass / 1 same failure using npm run test:focused -- tests/process-global/binaryTargetAdmissionCancellation.test.ts. Its test and admission/resolver/hash/error owners and toolchain are byte-identical to this head (10 paths). This is a verified inherited failure, not a diagnosis or fix. See evidence and scope. CI jobs:29 success/4 cancelled/3 failed. No required check is bypassed; merge/postmerge verification remain pending.

Current-head Greptile check succeeds, score3/5. Its two findings concern unchanged main JavaScript loop/Kuna owners; replies explain the producing boundaries and limitations. Both threads are resolved for scope only, without claiming those behaviors fixed. No current own-shell finding was reported. Codex review is unavailable due repository quota.

Author readiness

Human owner: @chengwudi1

  • 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/3559 **Author:** [@chengwudi1](https://github.com/chengwudi1) **Created:** 10/14/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/electron-hook-trash-item` --- ### 📝 Commits (10+) - [`22ec8bd`](https://github.com/morluto/rea/commit/22ec8bdb2f7d4626587f23b1eed4a207df600386) Merge remote-tracking branch 'origin/main' into codex/ci-followups-20261015 - [`57f3002`](https://github.com/morluto/rea/commit/57f30029b14a3035a0141c9249d63d2ee5e9018a) fix(javascript): reject Webpack getters replaced through any identity write (#3860) - [`ab45bf6`](https://github.com/morluto/rea/commit/ab45bf641e95a56a9d6cb8d246818a3d6925eb41) fix(ghidra): refuse PSP targets with unexamined ABI declarations (#3904) - [`fc712b3`](https://github.com/morluto/rea/commit/fc712b3750cbe16755c4f78c1f90fdf09fbaca31) fix(javascript): keep releases through a replaced handle as candidates (#3897) - [`c61a24b`](https://github.com/morluto/rea/commit/c61a24b8c5a2c58f969628b7d3bec62ffed7f0ae) fix(browser): bound CDP handshakes and command responses (#3555) - [`d94ef22`](https://github.com/morluto/rea/commit/d94ef22d70f02e56f043ff030fc6a025a19e56d9) fix(cli): route app bundles with trailing separators to native analysis (#3511) - [`3e88e21`](https://github.com/morluto/rea/commit/3e88e217052df44d9e96d2e2f93fe6ae33262afe) fix(electron): reject active scenario capture when inspect fuse is disabled (#3453) (#3605) - [`ce89706`](https://github.com/morluto/rea/commit/ce89706145dd98fbaa8df8f062fdc80777136940) fix(javascript): preserve exact static property keys (#3480) - [`73c72d5`](https://github.com/morluto/rea/commit/73c72d5745ae982b789074d1069480b9cc850bbd) fix(mcp): keep a snapshot open ahead of later calls on the same connection (#3661) - [`f5b1dce`](https://github.com/morluto/rea/commit/f5b1dce881f310989db4d764c7c2452c2ab38e04) docs: celebrate 250,000 GitHub stars 🎉 (#3971) ### 📊 Changes **1145 files changed** (+140341 additions, -13392 deletions) <details> <summary>View changed files</summary> 📝 `.agents/skills/reverse-engineer-anything/SKILL.md` (+16 -2) 📝 `.agents/skills/reverse-engineer-anything/references/android-applications.md` (+3 -1) 📝 `.agents/skills/reverse-engineer-anything/references/javascript-applications.md` (+44 -1) 📝 `.agents/skills/reverse-engineer-anything/references/native-and-artifacts.md` (+8 -6) 📝 `.github/workflows/ci.yml` (+16 -1) ➕ `.github/workflows/ghidra-bridge-sources.yml` (+48 -0) 📝 `.github/workflows/real-cutter.yml` (+3 -3) 📝 `.github/workflows/real-packet-capture.yml` (+1 -1) 📝 `AGENTS.md` (+2 -0) 📝 `CONTRIBUTING.md` (+21 -1) 📝 `README.md` (+3 -2) 📝 `THIRD_PARTY_NOTICES.md` (+1 -0) 📝 `bridge/cutter/rea_bridge.py` (+6 -5) 📝 `bridge/hopper_bridge.py` (+9 -1) 📝 `bridge/native/rea_lldb_tracer.py` (+48 -4) 📝 `bridge/native/test_rea_lldb_tracer.py` (+132 -0) 📝 `docs/.vitepress/config.ts` (+1 -0) 📝 `docs/adb-device-analysis.md` (+18 -4) 📝 `docs/android-analysis.md` (+19 -2) 📝 `docs/binary-diagnostics.md` (+18 -0) _...and 80 more files_ </details> ### 📄 Description ## Summary Fixes #3474. The active hook omitted `shell.trashItem`, allowing a target to reach native trash dispatch without a blocked attempt. Add it to the existing shell return-contract table: reject the promise and record the existing shape-only event. Preserve the synchronous shell contracts and original contributor @chengwudi1's work. ## Scope and approach The shell patch remains the shared producer for CLI/MCP. No tool/schema/cache or target authority changes. This is an ordinary fix within existing instrumentation scope; a new capability decision is not applicable. Replace the redundant direct mock-context unit with stronger staged-hook coverage. Save a reproducible Linux real-app runner and the selectable resource budget (4 GiB default preserved; explicit8 GiB supported). ## Executed evidence - Fresh-main regression fails: the native trash stub executes and resolves. **88 focused hook/projection/MCP cases pass** after the fix, covering async rejection, suppressed native effects, sync return contracts and retained-event shapes. Controlled MCP cases do not establish a real MCP provider run. - **Real unchanged VSCodium1.140.06880 UI**, explicit official Electron43.7.3: the same Explorer delete action gives native invocations **1→0**, blocked attempts **0→1**, and the owned22,177 B README copy remains byte-identical after blocking. The native-call observer preserves receiver, arguments and return object. Native OS Trash completion on the baseline is **not established**. - This real-app comparison uses the **production shell factory with a bounded test recorder**. **Full hook/provider/CLI/MCP workflow NOT_EXECUTED** there. Earlier packaged-preload, Inspector and navigation failures are preserved; #2974/#4494/#4282 and raw-wrapper mutation #4513 remain separate. No application callbacks, IPC handlers or vendor bytes were replaced. - `npm run check:fast`: **2 tasks pass** (initial run uncached; final run one cached). `npm run docs:check`: **pass**. Default4 GiB resource smoke: **pass**. Real Windows/macOS and full `check:pr` were not run. Candidate resource scope: **15.030s**, observed CPU **14.738146s**, peak **1,092,239,360 B**; complete-family observation **unverified**. Window-close API completes; all owned scopes are independently inactive/dead. Heavy work was serial,1CPU/8GiB hard/6GiB early/no swap/192tasks. Peak includes file cache; counters are at payload exit, not final whole-scope counters. Baseline waits for absent blocked evidence, so no speed comparison or token benefit is claimed. Agent costs and cold/warm repeats are unmeasured. [Verification, reproduction and saved cohorts](docs/verification/electron-trash-item-2026-10-16.md) and adjacent JSON retain source/input hashes and failures. Regenerable apps/runtimes/profiles/raw logs are excluded. AI-assisted updates above preserve the author's original initial-readiness record below; they do not attest to a new human review. ## Current CI and review [Run38983861225](https://github.com/morluto/rea/actions/runs/38983861225) fails the binary-target cancelled-admission cleanup assertion (`RegularFileOperationCancelled` versus the exact caller Error); Portable tests/CI required fail in consequence. **Reproduced on clean, rebuilt actual main parent`0c7b6d834`: 9 pass / 1 same failure** using `npm run test:focused -- tests/process-global/binaryTargetAdmissionCancellation.test.ts`. Its test and admission/resolver/hash/error owners and toolchain are byte-identical to this head (10 paths). This is a verified inherited failure, not a diagnosis or fix. See [evidence and scope](https://github.com/morluto/rea/pull/3559#issuecomment-6227079959). CI jobs:29 success/4 cancelled/3 failed. No required check is bypassed; merge/postmerge verification remain pending. Current-head Greptile check succeeds, score**3/5**. Its two findings concern unchanged main JavaScript loop/Kuna owners; replies explain the producing boundaries and limitations. Both threads are resolved for scope only, without claiming those behaviors fixed. No current own-shell finding was reported. Codex review is unavailable due repository quota. ## Author readiness Human owner: @chengwudi1 - [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:42 +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#5823
No description provided.