[PR #2305] [MERGED] fix(frida): drain admitted operations and honor queued cancellation #5150

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

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/2305
Author: @rohitg00
Created: 10/12/2026
Status: ✅ Merged
Merged: 10/13/2026
Merged by: @morluto

Base: main ← Head: fix/frida-operation-lifecycle


📝 Commits (10+)

  • 344fc71 fix(frida): drain admitted operations and honor queued cancellation
  • 32af2ab fix(frida): settle cancelled loads before queued resume
  • f01062f chore: merge upstream main into Frida lifecycle fixes
  • 68bf1ae test(frida): share lifecycle fixtures without duplicating assertions
  • 5792141 fix(frida): preserve operation admission before session close
  • 16d063c chore: merge upstream CI fixes into Frida lifecycle branch
  • 235517c chore: integrate upstream CI test correction
  • 443b404 test(apple): bound cold archive fixture compilation
  • 79ba2a8 chore: merge upstream CI and provenance test fixes
  • 1c53762 fix(frida): retain cancelled script creation evidence

📊 Changes

8 files changed (+1013 additions, -44 deletions)

View changed files

📝 docs/frida-instrumentation.md (+2 -2)
📝 src/application/frida/FridaInstrumentationManager.test.ts (+22 -0)
📝 src/application/frida/FridaInstrumentationPort.ts (+21 -2)
📝 src/application/frida/FridaInstrumentationService.test.ts (+72 -0)
📝 src/application/frida/FridaInstrumentationService.ts (+42 -6)
📝 src/frida/FridaInstrumentationManager.ts (+182 -20)
📝 src/server/registerFridaTools.ts (+29 -14)
➕ tests/boundary/mcp/fridaLifecycleMcp.test.ts (+643 -0)

📄 Description

Summary

Make Frida shutdown drain admitted operations and settle script-load cancellation before a queued resume can run. Cancelled operations no longer dispatch native effects after waiting for the session lock. Failed cleanup remains owned and retryable.

Related: #2535 and #2544.

Problem and expected behavior

A cancelled queued load, resume or unload could still execute, and shutdown could finish while attach or discovery was acquiring resources. Cancellation during native script loading also allowed a waiting resume to observe the cancelled script before service-level rollback ran.

The manager now owns admission, queue reservation, final load outcome and cleanup. An earlier operation reserves its session position synchronously, so a later close cannot overtake it. Cancellation during creation or loading releases the new script before returning. A failed unload retains the script identity and prevents premature resume. Once a load commits successfully, later cancellation cannot reclassify it; explicit unload releases it.

Change and scope

Drain admitted work, share close attempts, forward request cancellation and preserve cleanup ownership. Remove the service's superseded post-success rollback. Keep existing FIFO message capture, earlier scripts and caller-owned persistent sessions intact.

The Apple archive regression fixture also has a bounded 90-second compiler timeout within a 120-second test budget, preserving its assertions when a cold Swift cache exceeds the previous 30-second budget. Landed upstream CI and dyld fixture corrections are integrated without changing their implementation.

Contract and boundary impact

  • Semantic owner: manager admission and per-session native dispatch; the service projects the provider's settled outcome into Evidence.
  • Contribution scope: ordinary lifecycle correction within existing instrumentation authority; a new maintainer scope decision is not applicable.
  • CLI/MCP: schemas unchanged. Internal port calls accept optional cancellation signals; public error tags and partial-observation contracts remain intact.
  • Evidence: source identity, hashes, observed messages, script/session identities and cleanup uncertainty remain retained.
  • Authority and cleanup: selected-target behavior is unchanged. Cancellation does not detach persistent sessions, unload earlier scripts or kill targets. Failed cleanup remains retryable.
  • Package, installation and generated metadata: unchanged; owning Frida documentation is updated.

Expected existing effects:

  • Read existing files or captured data.
  • Launch or attach to a process.
  • Inject instrumentation or run scripts/code.

Evidence and regression coverage

Real MCP registration, Client/InMemoryTransport, service and manager tests with controlled native bindings cover queued cancellation, attach/discovery shutdown, synchronous admission, reentrant shutdown, creation/loading cancellation, failed cleanup and explicit retry. Ordering regressions fail before their producing-boundary fixes. Retained Evidence is checked against what queued native resume actually observed.

Cancelled-load results retain script identity and messages with script_cleanup: "released" or "incomplete_or_unknown"; committed successful loads remain explicitly unloadable.

  • Observed, derived and inferred claims remain distinguishable.
  • Artifact identity, source provenance and failed attempts remain preserved.
  • Unsupported, incomplete and uncertain outcomes remain visible.

Validation performed

After integrating upstream fb28ec1, using Node 24.18.0/npm 11.16.0 on macOS arm64:

  • npm run test:focused -- tests/boundary/mcp/fridaLifecycleMcp.test.ts src/application/frida/FridaInstrumentationManager.test.ts src/application/frida/FridaInstrumentationService.test.ts tests/boundary/mcp/fridaMcp.test.ts tests/boundary/cli/fridaCli.test.ts tests/boundary/cli/dylibResolutionCli.test.ts: 82 tests passed across six files, including the upstream dyld correction.
  • npm run check:ci: all eight tasks and architecture guards passed.
  • npm run docs:check: passed, including both README consistency tests.
  • git diff --check: passed. The upstream merge required no conflict resolution or additional source changes.

Earlier native Frida verification passed at 5792141 in a disposable Linux arm64 Debian Bookworm container with Frida 17.23.2: local spawn, marker message observed and successful cleanup. That receipt does not establish native verification of the final revision; later cancellation paths are verified with controlled bindings. The Apple fixture previously reproduced its timeout with a fresh Swift cache and passed with another fresh cache after the budget correction.

Compatibility, safety, and release

No breaking schema, dependency, migration or additional authority. Fresh final-revision native Frida verification, remote devices, Android/iOS and other native hosts remain unverified locally. Hosted CI is separate from local checks.

The previous hosted shard 3 failure is the existing ProviderDeadline.remainingMs() contradiction (expected zero after timeout, observed one millisecond). It remains tracked in open PR #2720; this PR does not duplicate that contributor's fix.

Before merge

  • Concrete problem, focused outcome and related issues stated.
  • Scope and existing authority are resolved.
  • Observable ordering, cancellation, failure and retry behavior are covered.
  • Owning documentation and representative Evidence are described.
  • Final diff checked for unrelated changes, secrets and unsupported claims.
  • Required hosted CI and final-revision real-provider evidence are complete.

🔄 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/2305 **Author:** [@rohitg00](https://github.com/rohitg00) **Created:** 10/12/2026 **Status:** ✅ Merged **Merged:** 10/13/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `fix/frida-operation-lifecycle` --- ### 📝 Commits (10+) - [`344fc71`](https://github.com/morluto/rea/commit/344fc714c1a80da09434004f3e1f8643173ace20) fix(frida): drain admitted operations and honor queued cancellation - [`32af2ab`](https://github.com/morluto/rea/commit/32af2ab2104a5876bd6808080de42239fcc88a10) fix(frida): settle cancelled loads before queued resume - [`f01062f`](https://github.com/morluto/rea/commit/f01062f295e7f24c0ab585b770e9ba21ff3a6f43) chore: merge upstream main into Frida lifecycle fixes - [`68bf1ae`](https://github.com/morluto/rea/commit/68bf1aeb9f9b625f68994f6a943e0306a841304d) test(frida): share lifecycle fixtures without duplicating assertions - [`5792141`](https://github.com/morluto/rea/commit/5792141ad3e6e26b36114347611271aa99c749c9) fix(frida): preserve operation admission before session close - [`16d063c`](https://github.com/morluto/rea/commit/16d063cf591d31db1a980230144d0b1108c23056) chore: merge upstream CI fixes into Frida lifecycle branch - [`235517c`](https://github.com/morluto/rea/commit/235517c2b41794f3ed0a08e00d172f3a5b7c7b6b) chore: integrate upstream CI test correction - [`443b404`](https://github.com/morluto/rea/commit/443b404caed1391f5348e4b5a21cc586b49bbb8c) test(apple): bound cold archive fixture compilation - [`79ba2a8`](https://github.com/morluto/rea/commit/79ba2a89d7d0a414e4f41b6bbd721e4d2a1cdec5) chore: merge upstream CI and provenance test fixes - [`1c53762`](https://github.com/morluto/rea/commit/1c537624238469e98cbe227df76de4494fe20fca) fix(frida): retain cancelled script creation evidence ### 📊 Changes **8 files changed** (+1013 additions, -44 deletions) <details> <summary>View changed files</summary> 📝 `docs/frida-instrumentation.md` (+2 -2) 📝 `src/application/frida/FridaInstrumentationManager.test.ts` (+22 -0) 📝 `src/application/frida/FridaInstrumentationPort.ts` (+21 -2) 📝 `src/application/frida/FridaInstrumentationService.test.ts` (+72 -0) 📝 `src/application/frida/FridaInstrumentationService.ts` (+42 -6) 📝 `src/frida/FridaInstrumentationManager.ts` (+182 -20) 📝 `src/server/registerFridaTools.ts` (+29 -14) ➕ `tests/boundary/mcp/fridaLifecycleMcp.test.ts` (+643 -0) </details> ### 📄 Description ## Summary Make Frida shutdown drain admitted operations and settle script-load cancellation before a queued resume can run. Cancelled operations no longer dispatch native effects after waiting for the session lock. Failed cleanup remains owned and retryable. Related: #2535 and #2544. ## Problem and expected behavior A cancelled queued load, resume or unload could still execute, and shutdown could finish while attach or discovery was acquiring resources. Cancellation during native script loading also allowed a waiting resume to observe the cancelled script before service-level rollback ran. The manager now owns admission, queue reservation, final load outcome and cleanup. An earlier operation reserves its session position synchronously, so a later close cannot overtake it. Cancellation during creation or loading releases the new script before returning. A failed unload retains the script identity and prevents premature resume. Once a load commits successfully, later cancellation cannot reclassify it; explicit unload releases it. ## Change and scope Drain admitted work, share close attempts, forward request cancellation and preserve cleanup ownership. Remove the service's superseded post-success rollback. Keep existing FIFO message capture, earlier scripts and caller-owned persistent sessions intact. The Apple archive regression fixture also has a bounded 90-second compiler timeout within a 120-second test budget, preserving its assertions when a cold Swift cache exceeds the previous 30-second budget. Landed upstream CI and dyld fixture corrections are integrated without changing their implementation. ## Contract and boundary impact - Semantic owner: manager admission and per-session native dispatch; the service projects the provider's settled outcome into Evidence. - Contribution scope: ordinary lifecycle correction within existing instrumentation authority; a new maintainer scope decision is not applicable. - CLI/MCP: schemas unchanged. Internal port calls accept optional cancellation signals; public error tags and partial-observation contracts remain intact. - Evidence: source identity, hashes, observed messages, script/session identities and cleanup uncertainty remain retained. - Authority and cleanup: selected-target behavior is unchanged. Cancellation does not detach persistent sessions, unload earlier scripts or kill targets. Failed cleanup remains retryable. - Package, installation and generated metadata: unchanged; owning Frida documentation is updated. Expected existing effects: - [x] Read existing files or captured data. - [x] Launch or attach to a process. - [x] Inject instrumentation or run scripts/code. ## Evidence and regression coverage Real MCP registration, Client/InMemoryTransport, service and manager tests with controlled native bindings cover queued cancellation, attach/discovery shutdown, synchronous admission, reentrant shutdown, creation/loading cancellation, failed cleanup and explicit retry. Ordering regressions fail before their producing-boundary fixes. Retained Evidence is checked against what queued native resume actually observed. Cancelled-load results retain script identity and messages with `script_cleanup: "released"` or `"incomplete_or_unknown"`; committed successful loads remain explicitly unloadable. - [x] Observed, derived and inferred claims remain distinguishable. - [x] Artifact identity, source provenance and failed attempts remain preserved. - [x] Unsupported, incomplete and uncertain outcomes remain visible. ## Validation performed After integrating upstream `fb28ec1`, using Node 24.18.0/npm 11.16.0 on macOS arm64: - `npm run test:focused -- tests/boundary/mcp/fridaLifecycleMcp.test.ts src/application/frida/FridaInstrumentationManager.test.ts src/application/frida/FridaInstrumentationService.test.ts tests/boundary/mcp/fridaMcp.test.ts tests/boundary/cli/fridaCli.test.ts tests/boundary/cli/dylibResolutionCli.test.ts`: 82 tests passed across six files, including the upstream dyld correction. - `npm run check:ci`: all eight tasks and architecture guards passed. - `npm run docs:check`: passed, including both README consistency tests. - `git diff --check`: passed. The upstream merge required no conflict resolution or additional source changes. Earlier native Frida verification passed at `5792141` in a disposable Linux arm64 Debian Bookworm container with Frida 17.23.2: local spawn, marker message observed and successful cleanup. That receipt does not establish native verification of the final revision; later cancellation paths are verified with controlled bindings. The Apple fixture previously reproduced its timeout with a fresh Swift cache and passed with another fresh cache after the budget correction. ## Compatibility, safety, and release No breaking schema, dependency, migration or additional authority. Fresh final-revision native Frida verification, remote devices, Android/iOS and other native hosts remain unverified locally. Hosted CI is separate from local checks. The previous hosted shard 3 failure is the existing `ProviderDeadline.remainingMs()` contradiction (expected zero after timeout, observed one millisecond). It remains tracked in open PR #2720; this PR does not duplicate that contributor's fix. ## Before merge - [x] Concrete problem, focused outcome and related issues stated. - [x] Scope and existing authority are resolved. - [x] Observable ordering, cancellation, failure and retry behavior are covered. - [x] Owning documentation and representative Evidence are described. - [x] Final diff checked for unrelated changes, secrets and unsupported claims. - [ ] Required hosted CI and final-revision real-provider evidence are complete. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
bsakaguchi 2026-10-31 01:04:21 +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#5150
No description provided.