morluto/rea

Fork 0

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

[PR #2324] [MERGED] fix(ida): preserve native coverage and verify returned values #5164

New issue

Closed

opened 2026-10-31 01:04:27 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 01:04:27 +00:00

Owner

📋 Pull Request Information

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

Base: main ← Head: fix/ida-native-verification-oracles

📝 Commits (3)

85c36ab fix(ida): preserve native coverage and verify returned values
d8baca6 Merge remote-tracking branch 'origin/main' into fix/ida-native-verification-oracles
d1e0b10 test(ghidra): derive header probe sizes from fixture bytes

📊 Changes

7 files changed (+446 additions, -21 deletions)

View changed files

📝 docs/ida-provider.md (+2 -0)
📝 experiments/analysis-efficiency/ida-native-windows.md (+19 -0)
📝 scripts/verify-real-ida.mjs (+89 -4)
📝 src/ghidra/GhidraInstallation.test.ts (+14 -16)
📝 src/ida/IdaNativeProtocol.test.ts (+36 -0)
📝 src/ida/IdaNativeProtocol.ts (+15 -1)
➕ tests/boundary/providers/ida/verifierObservations.test.ts (+271 -0)

📄 Description

Summary

Follow up the two late review findings on #1544: reject unknown native callee truncation and verify returned values in the real-provider lane.

Native callee successes require an explicit more marker; missing markers remain protocol failures. Upstream failures retain their original reason and observations.
Require the resolved procedure in inventory/search. Compare address, assembly, pseudocode, instruction windows and relationship results with the CLI/MCP dossier.
Add caller-selected --expected-string and --expected-xref positive oracles. Without independent oracles, report string/xref correctness as unverified. Callee parity does not establish semantic completeness.
Remove the unsupported raw-Evidence-parity claim and record the verified scope accurately.
Repair the CI-blocking Ghidra test fixtures: derive complete-file probe sizes from their bytes, avoiding stale ELF32 size constants in ELF64 cases. Runtime parser and installation behavior are unchanged.

Scope and contracts

Fixes within the existing attached native IDA profile and verification harness, plus the diagnosed CI fixture correction; a new-provider or mutation scope decision is not applicable. Existing CLI/MCP tool schemas, caller choices and legacy/modern behavior are preserved. The parser owns native coverage interpretation. No database save/close, target execution, installation, GUI startup or new analysis cache is introduced. Validation adds constant work to a native callee page; verification reuses the responses already requested.

Validation

207 focused IDA/provider/public-entry tests passed; public-entry cases use an explicitly simulated native provider and owned process supervision, not a real engine.
Serial typecheck/lint/format/knip/metadata checks, architecture guards, test discovery and docs checks passed. Local builds/tests used one logical CPU and bounded Node heaps.
Real Windows IDA Pro 9.3 with native MCP upstream c9d15d98e8a143fc74f6e14037c48f63889b6d6c, including its navigation correction and a local loopback listener: eight production CLI/MCP checks passed on the unchanged public fixture. The expected CRT literal was confirmed independently in fixture bytes; the expected reference source address was identified from main assembly matching the C source.
A response-only loopback proxy in front of the actual IDA Pro provider reproduced the old verifier passing empty inventories and xrefs. The corrected verifier rejected those replies with positive oracles, empty string results, and omitted callee markers (six live before/after cases). The comparison used verifier source d1aaeee40 with the same current adapter runtime; it is not an engine-performance comparison.
Windows IDA Free, native macOS and stock release behavior remain unverified by these runs. Personal paths, license registration information, binaries and raw private reports are excluded. Test inputs were unchanged and never executed; attached GUI databases remained open.

See the verification and historical performance note. Its before/after measurements retain their historical source pins; this parser/verification correction adds no performance improvement claim.

Review findings addressed

CI and review must complete on this PR's current head before merge.

CI baseline diagnosis

The initial run on 85c36abe3 failed referenceSourceImportStatus.test.ts in test shard 2 and the Windows lane. Independent main CI on f611d8ce7 fails the exact same cases: two Linux full-output preflight cases and those two plus the Windows readable-root case. Main commit 570c40cde changed full-output failures to ok: false / error, while those assertions still expect ok: true / data. This PR does not touch that CLI output implementation or those tests. This is a confirmed main baseline failure; unrelated code is kept out of this fix.

The refreshed run on d8baca68f encountered three Ghidra decompiler-header fixture failures in src/ghidra/GhidraInstallation.test.ts. Independent main CI on the exact integrated base c462a4ebd has the same three failures in both Windows and shard 4. The Ghidra test, implementation, and ELF-header producer are identical to main. The initial diagnosis kept unrelated code out. Following the maintainer request to fix CI, commit d1e0b1095 repairs only the inconsistent fixture metadata: ELF64 bytes are 64 bytes, while the old probes declared a 52-byte file. A shared test helper derives size from the actual buffer for complete-file probes; the expected compatible/incompatible/unknown outcomes remain unchanged. All three failures reproduced locally before the correction, and all 115 relevant Ghidra installation/doctor and ELF-header/target tests pass afterward. This does not add real-Ghidra provider verification.

Current integration status

Manual Codex review completed on 85c36abe3 without findings. Its initial CI failures were independently reproduced on main, as documented above. Main subsequently fixed the stale assertions in 8224aeba9. This branch now incorporates main c462a4ebd through merge d8baca68f; no separate unrelated CLI edits were added to this PR. On integrated head d8baca68f, all 207 focused tests and serial typecheck/lint/format/knip/metadata checks passed, including the pre-push gate. Automated Codex review completed on d8baca68f without findings. That run completed with 32 successful jobs; only the Ghidra cases failed in Windows and shard 5, causing the two aggregate failures. Commit d1e0b1095 fixes those fixtures; 115 related tests, serial typecheck/lint/format and commit hooks pass. On corrected head d1e0b1095, automated review completed without findings or unresolved threads. CI run 38327862093 completed successfully: all 36 jobs passed, including all six test shards, coverage/timing, the Windows capability/package lane, real Apple CLI/MCP workflows, and CI required.

Merged as 37b7550ecc916acc8619fa72e5c05af45702733f after exact-head review and all 36 CI jobs passed. The two late #1544 review threads have been explained and resolved.

Test cleanup removed the portable Windows runtime, Node and transient reports and probes. Attached test GUI inputs/databases and compact real-provider receipts are retained; no GUI database was saved or closed.

_{🔄 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/2324 **Author:** [@N0zoM1z0](https://github.com/N0zoM1z0) **Created:** 10/12/2026 **Status:** ✅ Merged **Merged:** 10/13/2026 **Merged by:** [@N0zoM1z0](https://github.com/N0zoM1z0) **Base:** `main` ← **Head:** `fix/ida-native-verification-oracles` --- ### 📝 Commits (3) - [`85c36ab`](https://github.com/morluto/rea/commit/85c36abe3b3dfb62e6e02261b26586bdefb70856) fix(ida): preserve native coverage and verify returned values - [`d8baca6`](https://github.com/morluto/rea/commit/d8baca68ff08390a1d96df70659f6999d2c62c07) Merge remote-tracking branch 'origin/main' into fix/ida-native-verification-oracles - [`d1e0b10`](https://github.com/morluto/rea/commit/d1e0b1095f70fd55c7c9d97332b5c15e2aa2d2bd) test(ghidra): derive header probe sizes from fixture bytes ### 📊 Changes **7 files changed** (+446 additions, -21 deletions) <details> <summary>View changed files</summary> 📝 `docs/ida-provider.md` (+2 -0) 📝 `experiments/analysis-efficiency/ida-native-windows.md` (+19 -0) 📝 `scripts/verify-real-ida.mjs` (+89 -4) 📝 `src/ghidra/GhidraInstallation.test.ts` (+14 -16) 📝 `src/ida/IdaNativeProtocol.test.ts` (+36 -0) 📝 `src/ida/IdaNativeProtocol.ts` (+15 -1) ➕ `tests/boundary/providers/ida/verifierObservations.test.ts` (+271 -0) </details> ### 📄 Description ## Summary Follow up the two late review findings on #1544: reject unknown native callee truncation and verify returned values in the real-provider lane. - Native callee successes require an explicit `more` marker; missing markers remain protocol failures. Upstream failures retain their original reason and observations. - Require the resolved procedure in inventory/search. Compare address, assembly, pseudocode, instruction windows and relationship results with the CLI/MCP dossier. - Add caller-selected `--expected-string` and `--expected-xref` positive oracles. Without independent oracles, report string/xref correctness as `unverified`. Callee parity does not establish semantic completeness. - Remove the unsupported raw-Evidence-parity claim and record the verified scope accurately. - Repair the CI-blocking Ghidra test fixtures: derive complete-file probe sizes from their bytes, avoiding stale ELF32 size constants in ELF64 cases. Runtime parser and installation behavior are unchanged. ## Scope and contracts Fixes within the existing attached native IDA profile and verification harness, plus the diagnosed CI fixture correction; a new-provider or mutation scope decision is not applicable. Existing CLI/MCP tool schemas, caller choices and legacy/modern behavior are preserved. The parser owns native coverage interpretation. No database save/close, target execution, installation, GUI startup or new analysis cache is introduced. Validation adds constant work to a native callee page; verification reuses the responses already requested. ## Validation - 207 focused IDA/provider/public-entry tests passed; public-entry cases use an explicitly simulated native provider and owned process supervision, not a real engine. - Serial typecheck/lint/format/knip/metadata checks, architecture guards, test discovery and docs checks passed. Local builds/tests used one logical CPU and bounded Node heaps. - **Real Windows IDA Pro 9.3** with native MCP upstream `c9d15d98e8a143fc74f6e14037c48f63889b6d6c`, including its navigation correction and a local loopback listener: eight production CLI/MCP checks passed on the unchanged public fixture. The expected CRT literal was confirmed independently in fixture bytes; the expected reference source address was identified from `main` assembly matching the C source. - A response-only loopback proxy in front of the actual IDA Pro provider reproduced the old verifier passing empty inventories and xrefs. The corrected verifier rejected those replies with positive oracles, empty string results, and omitted callee markers (six live before/after cases). The comparison used verifier source `d1aaeee40` with the same current adapter runtime; it is not an engine-performance comparison. - Windows IDA Free, native macOS and stock release behavior remain unverified by these runs. Personal paths, license registration information, binaries and raw private reports are excluded. Test inputs were unchanged and never executed; attached GUI databases remained open. See [the verification and historical performance note](experiments/analysis-efficiency/ida-native-windows.md). Its before/after measurements retain their historical source pins; this parser/verification correction adds no performance improvement claim. ## Review findings addressed - https://github.com/morluto/rea/pull/1544#discussion_r4251472623 - https://github.com/morluto/rea/pull/1544#discussion_r4251472628 CI and review must complete on this PR's current head before merge. ## CI baseline diagnosis The initial run on `85c36abe3` failed `referenceSourceImportStatus.test.ts` in test shard 2 and the Windows lane. Independent [main CI on `f611d8ce7`](https://github.com/morluto/rea/actions/runs/38298674473) fails the exact same cases: two Linux full-output preflight cases and those two plus the Windows readable-root case. Main commit `570c40cde` changed full-output failures to `ok: false` / `error`, while those assertions still expect `ok: true` / `data`. This PR does not touch that CLI output implementation or those tests. This is a confirmed main baseline failure; unrelated code is kept out of this fix. The refreshed run on `d8baca68f` encountered three Ghidra decompiler-header fixture failures in `src/ghidra/GhidraInstallation.test.ts`. Independent [main CI on the exact integrated base `c462a4ebd`](https://github.com/morluto/rea/actions/runs/38326444005) has the same three failures in both Windows and shard 4. The Ghidra test, implementation, and ELF-header producer are identical to main. The initial diagnosis kept unrelated code out. Following the maintainer request to fix CI, commit `d1e0b1095` repairs only the inconsistent fixture metadata: ELF64 bytes are 64 bytes, while the old probes declared a 52-byte file. A shared test helper derives size from the actual buffer for complete-file probes; the expected compatible/incompatible/unknown outcomes remain unchanged. All three failures reproduced locally before the correction, and all 115 relevant Ghidra installation/doctor and ELF-header/target tests pass afterward. This does not add real-Ghidra provider verification. ## Current integration status Manual Codex review completed on `85c36abe3` without findings. Its initial CI failures were independently reproduced on main, as documented above. Main subsequently fixed the stale assertions in `8224aeba9`. This branch now incorporates main `c462a4ebd` through merge `d8baca68f`; no separate unrelated CLI edits were added to this PR. On integrated head `d8baca68f`, all 207 focused tests and serial typecheck/lint/format/knip/metadata checks passed, including the pre-push gate. Automated Codex review completed on `d8baca68f` without findings. That run completed with 32 successful jobs; only the Ghidra cases failed in Windows and shard 5, causing the two aggregate failures. Commit `d1e0b1095` fixes those fixtures; 115 related tests, serial typecheck/lint/format and commit hooks pass. On corrected head `d1e0b1095`, automated review completed without findings or unresolved threads. [CI run 38327862093](https://github.com/morluto/rea/actions/runs/38327862093) completed successfully: all 36 jobs passed, including all six test shards, coverage/timing, the Windows capability/package lane, real Apple CLI/MCP workflows, and `CI required`. Merged as `37b7550ecc916acc8619fa72e5c05af45702733f` after exact-head review and all 36 CI jobs passed. The two late #1544 review threads have been explained and resolved. Test cleanup removed the portable Windows runtime, Node and transient reports and probes. Attached test GUI inputs/databases and compact real-provider receipts are retained; no GUI database was saved or closed. --- <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:27 +00:00

closed this issue
added the
pull-request
label

bsakaguchi referenced this issue

2026-11-01 21:43:05 +00:00

[PR #5165] [CLOSED] docs(proposal): Science Studio tabular import and column typing #6794

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

No description provided.

Rows
Columns