[PR #4499] [MERGED] fix(cutter): cancel discovery probes and queued work #6275

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

📋 Pull Request Information

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

Base: main ← Head: fix/cutter-discovery-cancel-3946


📝 Commits (3)

  • 1b0eb1b fix(cutter): cancel discovery probes and queued work
  • 0a4e6b9 fix(cutter): retain requests until socket closure
  • c68e401 fix(cutter): retain completed observations on late cancellation

📊 Changes

8 files changed (+421 additions, -38 deletions)

View changed files

📝 src/cli/cutterCommands.ts (+5 -2)
📝 src/cutter/CutterBridgeClient.ts (+41 -16)
📝 src/cutter/CutterBridgeService.ts (+45 -4)
📝 src/server/registerCutterTools.ts (+17 -14)
📝 tests/boundary/cli/cutterCommands.test.ts (+71 -1)
📝 tests/boundary/mcp/cutterCancellationMcp.test.ts (+102 -0)
📝 tests/boundary/providers/CutterBridgeClient.test.ts (+102 -1)
📝 tests/boundary/providers/CutterBridgeService.test.ts (+38 -0)

📄 Description

Summary

Fixes #3946. Cancelling Cutter discovery now closes its active socket probes and stops queued probes, rather than continuing discovery after the caller stops waiting. CLI SIGINT returns typed cancelled JSON and exit 130.

Problem and expected behavior

Nine private descriptors with silent loopback peers reproduce the bug: eight probes remain connected after MCP abort, then the ninth starts roughly 25 seconds later. Cancellation should reach those probes and prevent further admission; ordinary authenticated discovery must remain usable.

Change and scope

CLI and MCP pass the caller signal through the shared service to the client. The bounded worker queue stops admission and drains admitted workers before propagating failure. Each terminal bridge request now settles only after its owned socket emits close, so Promise settlement represents completed transport cleanup. The service returns the existing tagged Result/AnalysisCancelledError, preserving the abort reason rather than reporting cancellation as no live sessions. No cache or new concurrency limit is introduced.

Ordinary fix within existing Cutter discovery authority; no new execution, injection, mutation, or protection-recovery capability. A separate maintainer scope decision is not applicable.

Contract and boundary impact

  • Semantic owner and earliest changed stage: shared Cutter discovery, before status-probe admission; the client owns socket cancellation and worker draining.
  • CLI and MCP/tool-catalog contract: unchanged successful output and schemas; discovery cancellation uses the existing typed error projection. CLI signal handling now participates in the normal cancellation lifecycle.
  • Provider compatibility: unchanged Python bridge/protocol and descriptor parsing; native Linux Cutter 2.5.0 Qt5 verified. Windows signal/permission cases are not claimed.
  • Evidence/provenance: no Evidence shape change; opaque cancellation reasons remain attached to the typed error.
  • Authority and cleanup: only this request's status sockets are cancelled; no GUI termination, document mutation, or mutation-admission lock is added.
  • Generated metadata/package/install impact: none; no generated outputs or binaries committed.

Expected effects:

  • Read existing files or captured data.
  • Launch or attach to a process.
  • Inject instrumentation or run scripts/code.
  • Change process memory, registers, files, or configuration.
  • Contact network services.
  • Recover protected code or work around protections.
  • Documentation or usability only; no target interaction.
  • Not sure yet; investigation or maintainer guidance needed.

Network contact here is the existing authenticated loopback bridge status request. The native verification fixture launches its own GUI; discovery does not.

Evidence and regression coverage

  • Clean-base stdio reproduction: all eight sockets still active 503 ms after abort; ninth admitted at 24,989 ms. Its natural completion was not observed; the fixture cleaned it up.

  • Final head stdio: eight sockets close within 6 ms; zero active after 503 ms; no ninth admission; authenticated rediscovery succeeds on the same connection. Both runs finish with verified empty owned process lineage.

  • Durable MCP regression inspects actual TCP peer closure, queued admission and connection reuse. It fails on the affected base. CLI regression sends SIGINT to an owned CLI child and asserts typed cancelled, exit 130, peer closure and no queued ninth probe.

  • Review regression: an observer on a real TCP client proved typed cancellation previously returned before local close; the new test fails before the terminal-close fix and passes after it. Setup failure paths now enter cleanup before opening test listeners.

  • Late cancellation regression: abort from the actual command socket close callback previously produced success. The shared service now returns typed cancellation with the confirmed-complete Evidence and original reason. An actual CLI child interrupted during final receipt acknowledgement returned exit 130, code: "cancelled", and details.partial_observation.normalized_result.output: {"answer":42}. Confirmed completion does not acquire a false pending-native-work ledger.

  • Shared service tests cover pre-abort and abort after a port returns, including opaque EACCES/cleanup-shaped reasons without false cleanup ownership. Successful discovery retains its original object identity.

  • Official Cutter 2.5.0 Qt5 AppImage: two CLI and three stdio MCP calls read the same controlled ELF on clean base and final head; source bytes unchanged, MCP disconnect preserves the GUI, fixture-owned cleanup verified. Native GUI cancellation is not claimed.

  • Separate native metadata defects are tracked in #4497 and #4498. Qt6 rejects even the official sample Python plugin (upstream rizinorg/cutter#3659); no REA workaround is included. Existing descriptor-open/FIFO work belongs to #4471 / #4486.

  • 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

On final head c68e401153528cc50e25e3bf772e0664d0cf874c, rebased onto 180ba03bfddd5916251e81e97a500b1ede7a9eea:

  • npm run test:focused -- the 10 relevant Cutter CLI/MCP/provider and mutation-config/workflow files — 66 passed.
  • npm run check:ci — passed, including typecheck, lint, format, Knip, configuration and architecture guards.
  • npm run docs:check — passed.
  • Compiled public stdio cancellation/recovery probe and official native Qt5 CLI/MCP workflow — passed, as described above.

Equivalent healthy native task, three fresh-process workflows per revision, additional runs alternated, two CPU cores: base → fix medians wall 8.18 → 8.29 s, CPU 10.18 → 10.33 s, sampled process-family peak 1.049 → 1.055 GiB. First MCP discovery 43.7 → 44.9 ms, subsequent discovery 6.5 → 6.9 ms. Wall ranges 8.00–8.52 / 8.10–8.95 s. Shared host, uncontrolled OS caches/frequency: these are observations, not a speedup or stable cold-cache claim. Static checks ran serially below the 8 GiB total budget.

First completed task's whole captured wire plus CLI JSON serialized with o200k_base: 835,210 → 835,264 token proxy, including the exactly unchanged 829,341-token full catalog. Named two-tool definitions plus calls/CLI proxy: 8,517 → 8,571. These are serialization estimates, not model input or billing; differences include run-specific identities. No new response fields or extra calls are introduced. SDK output validation stayed enabled using the supported per-call tool definition.

Compatibility, safety, and release

  • No public schema, dependency, migration, or packaging change.
  • Linux actual Cutter Qt5 plus authenticated synthetic peers verified; actual native cancellation, cancellation during descriptor filesystem reads, Windows and macOS remain unverified.
  • Descriptor reads retain their existing bounded implementation; this fixes socket probes and their queue. No attached GUI is closed by the application change.
  • Detailed local evidence: /tmp/rea-3946-cutter-discovery-20261016/ (not a portable CI artifact; essential results are inline above).

Author readiness

Human owner: @N0zoM1z0

Recorded at the human owner's explicit instruction in this session to complete readiness declarations and request review. Diff review and checks described above were performed by the assistant; this does not claim an independent manual human review.

  • 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/4499 **Author:** [@N0zoM1z0](https://github.com/N0zoM1z0) **Created:** 10/16/2026 **Status:** ✅ Merged **Merged:** 10/16/2026 **Merged by:** [@N0zoM1z0](https://github.com/N0zoM1z0) **Base:** `main` ← **Head:** `fix/cutter-discovery-cancel-3946` --- ### 📝 Commits (3) - [`1b0eb1b`](https://github.com/morluto/rea/commit/1b0eb1b0059aa5312e628a4abcfd5b74091de9f1) fix(cutter): cancel discovery probes and queued work - [`0a4e6b9`](https://github.com/morluto/rea/commit/0a4e6b9f89dbf3cf7e4f902ba79eab2f03144f79) fix(cutter): retain requests until socket closure - [`c68e401`](https://github.com/morluto/rea/commit/c68e401153528cc50e25e3bf772e0664d0cf874c) fix(cutter): retain completed observations on late cancellation ### 📊 Changes **8 files changed** (+421 additions, -38 deletions) <details> <summary>View changed files</summary> 📝 `src/cli/cutterCommands.ts` (+5 -2) 📝 `src/cutter/CutterBridgeClient.ts` (+41 -16) 📝 `src/cutter/CutterBridgeService.ts` (+45 -4) 📝 `src/server/registerCutterTools.ts` (+17 -14) 📝 `tests/boundary/cli/cutterCommands.test.ts` (+71 -1) 📝 `tests/boundary/mcp/cutterCancellationMcp.test.ts` (+102 -0) 📝 `tests/boundary/providers/CutterBridgeClient.test.ts` (+102 -1) 📝 `tests/boundary/providers/CutterBridgeService.test.ts` (+38 -0) </details> ### 📄 Description ## Summary Fixes #3946. Cancelling Cutter discovery now closes its active socket probes and stops queued probes, rather than continuing discovery after the caller stops waiting. CLI SIGINT returns typed `cancelled` JSON and exit 130. ## Problem and expected behavior Nine private descriptors with silent loopback peers reproduce the bug: eight probes remain connected after MCP abort, then the ninth starts roughly 25 seconds later. Cancellation should reach those probes and prevent further admission; ordinary authenticated discovery must remain usable. ## Change and scope CLI and MCP pass the caller signal through the shared service to the client. The bounded worker queue stops admission and drains admitted workers before propagating failure. Each terminal bridge request now settles only after its owned socket emits `close`, so Promise settlement represents completed transport cleanup. The service returns the existing tagged `Result`/`AnalysisCancelledError`, preserving the abort reason rather than reporting cancellation as no live sessions. No cache or new concurrency limit is introduced. Ordinary fix within existing Cutter discovery authority; no new execution, injection, mutation, or protection-recovery capability. A separate maintainer scope decision is not applicable. ## Contract and boundary impact - Semantic owner and earliest changed stage: shared Cutter discovery, before status-probe admission; the client owns socket cancellation and worker draining. - CLI and MCP/tool-catalog contract: unchanged successful output and schemas; discovery cancellation uses the existing typed error projection. CLI signal handling now participates in the normal cancellation lifecycle. - Provider compatibility: unchanged Python bridge/protocol and descriptor parsing; native Linux Cutter 2.5.0 Qt5 verified. Windows signal/permission cases are not claimed. - Evidence/provenance: no Evidence shape change; opaque cancellation reasons remain attached to the typed error. - Authority and cleanup: only this request's status sockets are cancelled; no GUI termination, document mutation, or mutation-admission lock is added. - Generated metadata/package/install impact: none; no generated outputs or binaries committed. Expected effects: - [x] Read existing files or captured data. - [ ] Launch or attach to a process. - [ ] Inject instrumentation or run scripts/code. - [ ] Change process memory, registers, files, or configuration. - [x] Contact network services. - [ ] Recover protected code or work around protections. - [ ] Documentation or usability only; no target interaction. - [ ] Not sure yet; investigation or maintainer guidance needed. Network contact here is the existing authenticated loopback bridge status request. The native verification fixture launches its own GUI; discovery does not. ## Evidence and regression coverage - Clean-base stdio reproduction: all eight sockets still active 503 ms after abort; ninth admitted at 24,989 ms. Its natural completion was not observed; the fixture cleaned it up. - Final head stdio: eight sockets close within 6 ms; zero active after 503 ms; no ninth admission; authenticated rediscovery succeeds on the same connection. Both runs finish with verified empty owned process lineage. - Durable MCP regression inspects actual TCP peer closure, queued admission and connection reuse. It fails on the affected base. CLI regression sends SIGINT to an owned CLI child and asserts typed `cancelled`, exit 130, peer closure and no queued ninth probe. - Review regression: an observer on a real TCP client proved typed cancellation previously returned before local `close`; the new test fails before the terminal-close fix and passes after it. Setup failure paths now enter cleanup before opening test listeners. - Late cancellation regression: abort from the actual command socket close callback previously produced success. The shared service now returns typed cancellation with the confirmed-complete Evidence and original reason. An actual CLI child interrupted during final receipt acknowledgement returned exit 130, `code: "cancelled"`, and `details.partial_observation.normalized_result.output: {"answer":42}`. Confirmed completion does not acquire a false pending-native-work ledger. - Shared service tests cover pre-abort and abort after a port returns, including opaque EACCES/cleanup-shaped reasons without false cleanup ownership. Successful discovery retains its original object identity. - Official Cutter 2.5.0 Qt5 AppImage: two CLI and three stdio MCP calls read the same controlled ELF on clean base and final head; source bytes unchanged, MCP disconnect preserves the GUI, fixture-owned cleanup verified. Native GUI cancellation is not claimed. - Separate native metadata defects are tracked in #4497 and #4498. Qt6 rejects even the official sample Python plugin (upstream rizinorg/cutter#3659); no REA workaround is included. Existing descriptor-open/FIFO work belongs to #4471 / #4486. - [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 On final head `c68e401153528cc50e25e3bf772e0664d0cf874c`, rebased onto `180ba03bfddd5916251e81e97a500b1ede7a9eea`: - `npm run test:focused --` the 10 relevant Cutter CLI/MCP/provider and mutation-config/workflow files — **66 passed**. - `npm run check:ci` — passed, including typecheck, lint, format, Knip, configuration and architecture guards. - `npm run docs:check` — passed. - Compiled public stdio cancellation/recovery probe and official native Qt5 CLI/MCP workflow — passed, as described above. Equivalent healthy native task, **three fresh-process workflows per revision**, additional runs alternated, two CPU cores: base → fix medians wall **8.18 → 8.29 s**, CPU **10.18 → 10.33 s**, sampled process-family peak **1.049 → 1.055 GiB**. First MCP discovery **43.7 → 44.9 ms**, subsequent discovery **6.5 → 6.9 ms**. Wall ranges **8.00–8.52 / 8.10–8.95 s**. Shared host, uncontrolled OS caches/frequency: these are observations, not a speedup or stable cold-cache claim. Static checks ran serially below the 8 GiB total budget. First completed task's whole captured wire plus CLI JSON serialized with `o200k_base`: **835,210 → 835,264** token proxy, including the exactly unchanged **829,341**-token full catalog. Named two-tool definitions plus calls/CLI proxy: **8,517 → 8,571**. These are serialization estimates, not model input or billing; differences include run-specific identities. No new response fields or extra calls are introduced. SDK output validation stayed enabled using the supported per-call tool definition. ## Compatibility, safety, and release - No public schema, dependency, migration, or packaging change. - Linux actual Cutter Qt5 plus authenticated synthetic peers verified; actual native cancellation, cancellation during descriptor filesystem reads, Windows and macOS remain unverified. - Descriptor reads retain their existing bounded implementation; this fixes socket probes and their queue. No attached GUI is closed by the application change. - Detailed local evidence: `/tmp/rea-3946-cutter-discovery-20261016/` (not a portable CI artifact; essential results are inline above). ## Author readiness Human owner: @N0zoM1z0 Recorded at the human owner's explicit instruction in this session to complete readiness declarations and request review. Diff review and checks described above were performed by the assistant; this does not claim an independent manual human review. - [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 - [x] This PR addresses a concrete problem or an agreed enhancement. - [x] Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff. - [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 01:13:12 +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#6275
No description provided.