morluto/rea

Fork 0

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

[PR #4234] [MERGED] fix(ida): ignore released worker rows when verifying close #6137

New issue

Closed

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

bsakaguchi commented

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

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/4234
Author: @alucardigo
Created: 10/15/2026
Status: ✅ Merged
Merged: 10/22/2026
Merged by: @morluto

Base: main ← Head: fix/ida-released-worker-row

📝 Commits (3)

15e841a fix(ida): ignore released worker rows when verifying close
7ab6395 fix(ida): suppress released rows only for worker backends
e7aed18 refactor(ida): state the released-row predicate as equality

📊 Changes

2 files changed (+123 additions, -5 deletions)

View changed files

📝 src/ida/IdaSessionClient.test.ts (+94 -0)
📝 src/ida/IdaSessionClient.ts (+29 -5)

📄 Description

Summary

A successful headless IDA analysis is returned as a failed run and its private workspace is retained, because the post-close inventory check treats a worker observed mid-teardown as a still-registered session. Refs #3056 (the remaining symptom there, after 6.5.0 fixed the hang).

Problem and expected behavior

Smallest trigger: rea decompile <function> --provider ida against a real IDA 9.2 idalib supervisor. 11 of 12 stock 6.5.0 runs end in cleanup_incomplete / IDA worker remains registered after close.

Violated invariant: #closeOnce concludes that a worker is still registered from a row that identifies no live session. Upstream idb_list can observe the owning worker mid-teardown and return its identity already cleared, while input_path still names the private copy:

{"session_id": "", "input_path": "<private workspace>\\cat.exe", "owned": false,
 "is_active": false, "adopted": false, "backend": "worker", "pid": 45344}

The check matches on either the session ID or the private input path. The blanked session_id cannot match the first arm; the path arm does. Expected: a row that asserts no identity, no ownership and no activity does not block release.

Change and scope

isReleasedWorkerRow excludes exactly that shape from the private-input arm, and only for backend: "worker". The two arms keep their distinct meanings: the session-ID arm proves identity, and a genuine peer on the same input retains a session ID, so same-input protection is unchanged. A gui row is never suppressed, because it belongs to an IDA instance REA does not control and the observed teardown shape covers workers only.

Intentionally not included: no retry or grace period around the inventory call, and no change to upstream expectations. A short re-poll would cover the same ground ÔÇö the row disappears within ~1 s ÔÇö and I am happy to reshape it that way if you prefer that direction.

Contribution scope: ordinary fix within existing boundaries; no new provider or capability, so no maintainer scope decision is required.

Contract and boundary impact

Semantic owner and earliest changed stage: IdaSessionClient#closeOnce, cleanup stage only.
Contribution scope fit and linked maintainer decision: existing-boundary fix; not applicable.
CLI and MCP/tool-catalog contract: unchanged.
Provider, bridge, target-format, or platform compatibility: IDA headless provider only; no format or platform gate changed.
Evidence, artifact, provenance, or reconstruction contract: unchanged. Runs that previously failed now return the Evidence they had already produced.
Target selection, authority, upstream behavior, and cleanup details: no target interaction changes. The change only decides whether REA releases its own private workspace; blocking semantics for session-ID matches and for gui rows are preserved.
Generated metadata, package, or installation impact: none.

Expected effects (left unchecked: this change adds no target interaction; it alters only REA's release decision for its own private copy).

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.

Evidence and regression coverage

Evidence is executed, not derived. The payload above was captured in-process at the idb_list parse site during a failing run; a transparent stdio proxy could not be used, because its added per-message latency makes the run pass (8/8 passing with it, 11/12 failing without).

Tests added or updated: three in src/ida/IdaSessionClient.test.ts, each verified to fail without the behaviour it guards.

test	fails when
a closed worker lingering without an identity releases cleanly	the predicate is removed
a same-input `gui` row with a blank identity still blocks	the predicate does not check `backend`
a same-input peer that keeps a session ID still blocks	the private-input arm is dropped

Base reproduction or other evidence: on 6.5.0 stock, 11 of 12 real-provider runs failed; with the change, 6 of 6 passed and retained no workspace. Measured timings tie the outcome to the race ÔÇö post-close idb_list returned in ~1.02 s with an empty inventory on passing runs, and in 522 ms with the tear-down row on the failing one.
User-visible CLI/MCP output: unchanged on success; affected runs now return their analysis result instead of cleanup_incomplete.
Remaining proof gaps: the failing payload could not be captured on the wire, only in-process. Reproduced on one host, one target and one upstream commit (c133c385). No evidence about gui rows in real teardown ÔÇö which is why they stay blocking.

For evidence-bearing changes:

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

npm run test:focused -- src/ida/IdaSessionClient.test.ts ÔÇö 64 passed (64).
npx vitest run src/ida/IdaSessionClient.test.ts -t "blanked-identity" with the predicate neutralised ÔÇö 1 failed, 2 passed, as intended.
npx vitest run src/ida/IdaSessionClient.test.ts -t "blanked-identity" with the backend check removed ÔÇö the gui guard failed, as intended.
turbo run typecheck lint via the pre-commit hook ÔÇö 2 successful, 2 total.
rea decompile "" __main --provider ida --format json, real IDA 9.2 ÔÇö 11/12 failures on stock, 6/6 passes with the change.

Compatibility, safety, and release

Breaking changes or migration steps: none.
Real Hopper/Ghidra, browser, or OS coverage: Windows 11 x64 with real IDA Pro 9.2 (idalib) and upstream mrexodia/ida-pro-mcp at c133c385. No Hopper, Ghidra or browser path touched.
Package or release metadata impact: none.
Security, privacy, process, or containment review: the change narrows when a workspace is released, never widening authority. A gui row and any row carrying a session ID keep blocking, so a session REA does not own cannot cause its private copy to be removed.

Author readiness

Human owner: @alucardigo

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/4234 **Author:** [@alucardigo](https://github.com/alucardigo) **Created:** 10/15/2026 **Status:** ✅ Merged **Merged:** 10/22/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `fix/ida-released-worker-row` --- ### 📝 Commits (3) - [`15e841a`](https://github.com/morluto/rea/commit/15e841af162602b5dd8c637fff4a415083966981) fix(ida): ignore released worker rows when verifying close - [`7ab6395`](https://github.com/morluto/rea/commit/7ab639521709a4628fc74c62b8ee5294e307b95b) fix(ida): suppress released rows only for worker backends - [`e7aed18`](https://github.com/morluto/rea/commit/e7aed184cc4510744efab3dc4694ed7f77f6e513) refactor(ida): state the released-row predicate as equality ### 📊 Changes **2 files changed** (+123 additions, -5 deletions) <details> <summary>View changed files</summary> 📝 `src/ida/IdaSessionClient.test.ts` (+94 -0) 📝 `src/ida/IdaSessionClient.ts` (+29 -5) </details> ### 📄 Description ## Summary A successful headless IDA analysis is returned as a failed run and its private workspace is retained, because the post-close inventory check treats a worker observed mid-teardown as a still-registered session. Refs #3056 (the remaining symptom there, after 6.5.0 fixed the hang). ## Problem and expected behavior Smallest trigger: `rea decompile <function> --provider ida` against a real IDA 9.2 `idalib` supervisor. 11 of 12 stock 6.5.0 runs end in `cleanup_incomplete` / `IDA worker remains registered after close.` Violated invariant: `#closeOnce` concludes that a worker is still registered from a row that identifies no live session. Upstream `idb_list` can observe the owning worker mid-teardown and return its identity already cleared, while `input_path` still names the private copy: ```json {"session_id": "", "input_path": "<private workspace>\\cat.exe", "owned": false, "is_active": false, "adopted": false, "backend": "worker", "pid": 45344} ``` The check matches on either the session ID or the private input path. The blanked `session_id` cannot match the first arm; the path arm does. Expected: a row that asserts no identity, no ownership and no activity does not block release. ## Change and scope `isReleasedWorkerRow` excludes exactly that shape from the private-input arm, and only for `backend: "worker"`. The two arms keep their distinct meanings: the session-ID arm proves identity, and a genuine peer on the same input retains a session ID, so same-input protection is unchanged. A `gui` row is never suppressed, because it belongs to an IDA instance REA does not control and the observed teardown shape covers workers only. Intentionally not included: no retry or grace period around the inventory call, and no change to upstream expectations. A short re-poll would cover the same ground ÔÇö the row disappears within ~1 s ÔÇö and I am happy to reshape it that way if you prefer that direction. Contribution scope: ordinary fix within existing boundaries; no new provider or capability, so no maintainer scope decision is required. ## Contract and boundary impact - Semantic owner and earliest changed stage: `IdaSessionClient#closeOnce`, cleanup stage only. - Contribution scope fit and linked maintainer decision: existing-boundary fix; not applicable. - CLI and MCP/tool-catalog contract: unchanged. - Provider, bridge, target-format, or platform compatibility: IDA headless provider only; no format or platform gate changed. - Evidence, artifact, provenance, or reconstruction contract: unchanged. Runs that previously failed now return the Evidence they had already produced. - Target selection, authority, upstream behavior, and cleanup details: no target interaction changes. The change only decides whether REA releases **its own** private workspace; blocking semantics for session-ID matches and for `gui` rows are preserved. - Generated metadata, package, or installation impact: none. Expected effects (left unchecked: this change adds no target interaction; it alters only REA's release decision for its own private copy). - [ ] 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. ## Evidence and regression coverage Evidence is executed, not derived. The payload above was captured in-process at the `idb_list` parse site during a failing run; a transparent stdio proxy could not be used, because its added per-message latency makes the run pass (8/8 passing with it, 11/12 failing without). - Tests added or updated: three in `src/ida/IdaSessionClient.test.ts`, each verified to fail without the behaviour it guards. | test | fails when | |---|---| | a closed worker lingering without an identity releases cleanly | the predicate is removed | | a same-input `gui` row with a blank identity still blocks | the predicate does not check `backend` | | a same-input peer that keeps a session ID still blocks | the private-input arm is dropped | - Base reproduction or other evidence: on 6.5.0 stock, 11 of 12 real-provider runs failed; with the change, 6 of 6 passed and retained no workspace. Measured timings tie the outcome to the race ÔÇö post-close `idb_list` returned in ~1.02 s with an empty inventory on passing runs, and in 522 ms with the tear-down row on the failing one. - User-visible CLI/MCP output: unchanged on success; affected runs now return their analysis result instead of `cleanup_incomplete`. - Remaining proof gaps: the failing payload could not be captured on the wire, only in-process. Reproduced on one host, one target and one upstream commit (`c133c385`). No evidence about `gui` rows in real teardown ÔÇö which is why they stay blocking. For evidence-bearing changes: - [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 - `npm run test:focused -- src/ida/IdaSessionClient.test.ts` ÔÇö 64 passed (64). - `npx vitest run src/ida/IdaSessionClient.test.ts -t "blanked-identity"` with the predicate neutralised ÔÇö 1 failed, 2 passed, as intended. - `npx vitest run src/ida/IdaSessionClient.test.ts -t "blanked-identity"` with the `backend` check removed ÔÇö the `gui` guard failed, as intended. - `turbo run typecheck lint` via the pre-commit hook ÔÇö 2 successful, 2 total. - `rea decompile "" __main --provider ida --format json`, real IDA 9.2 ÔÇö 11/12 failures on stock, 6/6 passes with the change. ## Compatibility, safety, and release - Breaking changes or migration steps: none. - Real Hopper/Ghidra, browser, or OS coverage: Windows 11 x64 with real IDA Pro 9.2 (`idalib`) and upstream `mrexodia/ida-pro-mcp` at `c133c385`. No Hopper, Ghidra or browser path touched. - Package or release metadata impact: none. - Security, privacy, process, or containment review: the change narrows when a workspace is released, never widening authority. A `gui` row and any row carrying a session ID keep blocking, so a session REA does not own cannot cause its private copy to be removed. ## Author readiness Human owner: @alucardigo - [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. --- 🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

bsakaguchi

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

closed this issue
added the
pull-request
label

bsakaguchi referenced this issue

2026-11-01 21:45:46 +00:00

[PR #6137] [MERGED] fix(reconstruction): fall back to a kind-based title for blank graph labels #7151

bsakaguchi changed title from ~~[PR #4234] [MERGED] fix(ida): ignore released worker rows when verifying close~~ to [PR #4234] fix(ida): ignore released worker rows when verifying close

2026-11-03 13:31:58 +00:00

bsakaguchi changed title from ~~[PR #4234] fix(ida): ignore released worker rows when verifying close~~ to [PR #4234] [MERGED] fix(ida): ignore released worker rows when verifying close

2026-11-03 14:33:41 +00:00

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

No description provided.

Rows
Columns