morluto/rea

Fork 0

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

[PR #5171] [MERGED] feat(objdump): select native disassembly by symbol #6798

New issue

Closed

opened 2026-11-01 21:43:06 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

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

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/5171
Author: @gensokyo-dev
Created: 10/19/2026
Status: ✅ Merged
Merged: 10/19/2026
Merged by: @gensokyo-dev

Base: main ← Head: fix/5159-objdump-symbol-selection

📝 Commits (6)

2ddcfd8 feat(objdump): select native disassembly by symbol
6697023 chore: merge latest main into objdump symbol selection
57a53ef fix(objdump): reject lossy Unicode symbol selectors
c4282c4 chore: merge latest main for objdump validation
4d263be fix(objdump): keep advertised symbol patterns portable
a4e3aa6 chore(objdump): integrate main CI repairs

📊 Changes

9 files changed (+253 additions, -25 deletions)

View changed files

📝 docs/reverse-engineering-tools.md (+3 -0)
📝 src/application/reverse/ReverseEngineeringService.ts (+12 -1)
📝 src/cli/reverseEngineeringCommands.ts (+11 -9)
📝 src/contracts/reverseEngineeringToolContracts.ts (+1 -1)
📝 src/contracts/toolInputReferences.test.ts (+39 -0)
📝 src/domain/reverseEngineering.ts (+29 -9)
📝 src/objdump/ObjdumpCommand.ts (+4 -1)
📝 tests/boundary/cli/reverseEngineeringCommands.test.ts (+25 -4)
➕ tests/composition/objdumpSelectedDisassembly.test.ts (+129 -0)

📄 Description

Summary

Fixes #5159. Known-symbol reads currently produce whole-code disassembly, sometimes exceeding MCP's response budget. Optional symbol on operation: "disassemble" now delegates selection to GNU objdump before capture. Omission preserves full disassembly.

Problem and expected behavior

rea inspect-with-objdump /usr/lib/x86_64-linux-gnu/libcurl.so.4 disassemble --symbol curl_version_info --json returns native symbol-scoped Evidence inline. Other operations, empty names and NUL-containing names reject selection before artifact/provider admission.

Change and scope

One domain operation list feeds CLI/MCP. A strict disassembly variant owns selection; the provider passes one --disassemble=<symbol> argument. The shared workflow preserves the native name, argv, digests and explicit coverage limitation in Evidence, omitting absent JSON parameters. No cache, tool, dependency or installer is added.

Contract and boundary impact

Semantic owner and earliest changed stage: domain input contract; native selection precedes collection/serialization.
Contribution scope fit and linked maintainer decision: existing read-only static provider; new execution/injection/mutation/recovery authority and a new scope decision are not applicable.
CLI and MCP/tool-catalog contract: additive --symbol / symbol, valid only for disassemble.
Provider, bridge, target-format, or platform compatibility: caller-supplied GNU objdump; verified on Linux x86-64 ELF with GNU 2.40.
Evidence, artifact, provenance, or reconstruction contract: full requested raw output and retained Evidence remain accessible; native matching/boundaries apply, and an empty match does not establish artifact-wide absence.
Target selection, authority, upstream behavior, and cleanup details: existing local artifact, supervision, cancellation, debug-link policy and resource limits.
Generated metadata, package, or installation impact: ignored metadata generated from source; no package/release changes.

Expected effects:

Read existing files or captured data.
Launch or attach to a process. (Existing analysis provider; no target execution.)
Inject instrumentation or run scripts/code.
Change process memory, registers, files, or configuration.
Contact network services.
Recover protected code or work around protections.

Evidence and regression coverage

Current integration head: a4e3aa6ed, with main 0fc8462be merged. The nine feature files are unchanged, and final local verification below was executed on this head.

293 focused tests / 28 files passed (2 skipped) on head a4e3aa6ed, integrated with main 0fc8462be. This includes 21 canonical symbol cases, the complete advertised-pattern compatibility check, both SDK schema dialects, provider shutdown, the previously failing source cancellation case, and all four prior replay/fixture/process-global failures repaired by main.
Real packaged CLI/MCP/native stdout equality passed for zlib, libcurl and no-match /usr/bin/true, including retained complete Evidence, invalid inputs, CLI rejection and connection reuse. Independent ELF64 PT_LOAD mapping checked the selected 8/422 source bytes earlier; the three final public-artifact MCP replies remain byte-identical after this main integration.
A controlled UTF-8 ELF reproduced wrong-symbol selection before the Unicode fix. Four malformed surrogate forms now fail through actual MCP; valid astral and literal U+FFFD names select distinct expected functions through CLI/MCP/GNU objdump. The fixture was not executed.
The canonical runtime owns UTF-16 validity. Published patterns use the portable hexadecimal NUL escape without surrogate ranges, which scalar Unicode regex engines cannot compile. SDK schema tests cover draft-2020-12/draft-07 with ordinary and Unicode regex modes; the actual full catalog compiles in JS ordinary/u/v modes, and a real Rust regex probe accepts both valid Unicode names and rejects NUL. No downstream normalization or second analysis implementation was added.

Validation performed

npm run build:cached — passed.
Focused source/contracts/CLI/MCP tests — 293 passed, 2 skipped across 28 files.
Packaged public-artifact and controlled Unicode CLI/MCP workflows — passed as above.
npm run check:fast — passed (whole-repository types, lint, module boundaries).
npm run docs:check — passed (6 artifact/document tasks, 5 README tests).

CI attribution: the old 57a53efa9 run exposed our advertised NUL/surrogate-pattern portability defect, fixed in 4d263beab. Main b999e7247, included in integrated main 0fc8462be, repairs the unrelated Native API replay assertion, ASAR fixture and two process-global mock-isolation failures. All four repaired cases pass in the new 293-test local selection. Hosted source/static/build/docs/reverse-quality, all five test shards and Greptile checks have passed. The hosted workflow records the final aggregate status for this head; normal merge remains subject to required checks.

Final verification resource receipts cover one CPU, sequential bounded scopes, completed owned cleanup and zero OOM kills. The maximum process-family peak was 4,303.94 MiB (4.20 GiB), within the 8 GiB hard cap. Build / focused regressions / Unicode probe / public-artifact probe / type+lint / docs took 110.929 / 193.366 / 76.519 / 91.805 / 211.302 / 21.109 seconds elapsed, and 110.269 / 163.171 / 59.675 / 71.547 / 210.187 / 20.857 seconds CPU respectively. These are final verification runs, not a repeated before/after cold-task benchmark.

Cold-task measurements below are pinned to baseline dfe8378cf and initial selection commit 2ddcfd84a, using the Node SDK on Linux. Cold task timing was not rerun after the input compatibility fixes or latest-main integration. Catalog-only estimates pinned to main 69aa5465c versus 4d263beab are 807,446 raw SDK tokens versus 807,550 with this change (+104); the hypothetical input-only function catalog is 183,259 versus 183,363. Only inspect_with_objdump changes in that catalog comparison. These are offline cl100k estimates, not actual client/model usage.

Task	Native stdout before → after	Estimated raw SDK task tokens, including discovery
zlib `zlibVersion`	1,057,333 → 405 bytes	3,063,907 → 809,192
curl `curl_version_info`	6,921,190 → 6,923 bytes	808,419 → 822,352

These are tiktoken/cl100k estimates, not model usage. Curl's baseline needs refusal → retained summary → a 14.56 MB local Evidence export (3 RPCs versus 1); local-file/model consumption is unmeasured, so no overall curl token saving is claimed. Completed cold trials include discovery, verification/oracles and owned cleanup. Ranges below retain every unprofiled trial (2 pairs for zlib; 4 pairs across default/120-second SDK deadline cohorts for curl):

Task	Elapsed seconds before → after	CPU seconds before → after	Family peak MiB before → after
zlib	19.21–21.04 → 19.26–19.50	18.66–20.55 → 18.81–18.94	660–705 → 554–566
curl	20.11–21.96 → 19.13–56.11	19.36–21.19 → 18.54–41.87	663–701 → 554–589

The extra cost of the 45–56 s selected-call outliers remains unexplained; no overall speedup or complex-agent effectiveness is claimed. Profiling found substantial SDK AJV cold compilation with an unchanged output schema. Trials used one CPU, warm OS caches and bounded Linux cgroups; CPU/memory.peak were observed at payload exit with an observer alive (not terminal whole-scope counters). Owned cleanup completed without OOM.

Compatibility, safety, and release

No breaking change, migration or release metadata. Symbol text remains one argv value, with no shell interpolation. Real verification covers GNU 2.40, Debian zlib 1.2.13/libcurl 7.88.1, Linux x86-64. Other platforms/formats and actual model usage remain unverified.

Post-merge integration check

Merged as 3ea0d9489. Main integrated the separate 49e67aa1d / dc936f433 process-ownership changes while this PR's CI was running; the nine feature files match the tested head. A fresh merged-main build passed, but the expanded 30-file check had 306 passed, 16 failed and 2 skipped, and actual MCP zlib selection returned cleanup_incomplete despite correct native output and exit 0.

The original file_headers operation without a symbol also returned the same cleanup error on independently built pre-feature main dc936f433, with native exit 0 and 151 bytes matching GNU objdump. An earlier parent control passed, so this is intermittent. The real Linux evidence is recorded on #4322; the precise cleanup root cause is not localized. Pre-merge required CI is green; successful post-merge provider verification is not claimed. No unrelated process-layer patch was added.

Author readiness

Human owner: @gensokyo-dev

Recorded on the owner’s explicit instruction on 2026-10-19: the owner confirmed the required human declarations, authorized the assistant to enter them, and declared the contribution ready to proceed. The technical review and executed validation documented above were performed by the assistant; this note does not claim an additional independent manual 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/5171 **Author:** [@gensokyo-dev](https://github.com/gensokyo-dev) **Created:** 10/19/2026 **Status:** ✅ Merged **Merged:** 10/19/2026 **Merged by:** [@gensokyo-dev](https://github.com/gensokyo-dev) **Base:** `main` ← **Head:** `fix/5159-objdump-symbol-selection` --- ### 📝 Commits (6) - [`2ddcfd8`](https://github.com/morluto/rea/commit/2ddcfd84a64804f650bb4175169bc72551ae9342) feat(objdump): select native disassembly by symbol - [`6697023`](https://github.com/morluto/rea/commit/6697023bd02c316f072c00d81e3bb7dcf0a7dda6) chore: merge latest main into objdump symbol selection - [`57a53ef`](https://github.com/morluto/rea/commit/57a53efa90a6cab73f7510deca1f6ef8d6eeed08) fix(objdump): reject lossy Unicode symbol selectors - [`c4282c4`](https://github.com/morluto/rea/commit/c4282c4017a44051e6400bcb1f33faed4d656635) chore: merge latest main for objdump validation - [`4d263be`](https://github.com/morluto/rea/commit/4d263beabf49834f9dea4e2a6a65b9e43fbb9d5d) fix(objdump): keep advertised symbol patterns portable - [`a4e3aa6`](https://github.com/morluto/rea/commit/a4e3aa6ed27d1029ae331f68abf50c259bf4f28b) chore(objdump): integrate main CI repairs ### 📊 Changes **9 files changed** (+253 additions, -25 deletions) <details> <summary>View changed files</summary> 📝 `docs/reverse-engineering-tools.md` (+3 -0) 📝 `src/application/reverse/ReverseEngineeringService.ts` (+12 -1) 📝 `src/cli/reverseEngineeringCommands.ts` (+11 -9) 📝 `src/contracts/reverseEngineeringToolContracts.ts` (+1 -1) 📝 `src/contracts/toolInputReferences.test.ts` (+39 -0) 📝 `src/domain/reverseEngineering.ts` (+29 -9) 📝 `src/objdump/ObjdumpCommand.ts` (+4 -1) 📝 `tests/boundary/cli/reverseEngineeringCommands.test.ts` (+25 -4) ➕ `tests/composition/objdumpSelectedDisassembly.test.ts` (+129 -0) </details> ### 📄 Description ## Summary Fixes #5159. Known-symbol reads currently produce whole-code disassembly, sometimes exceeding MCP's response budget. Optional `symbol` on `operation: "disassemble"` now delegates selection to GNU objdump before capture. Omission preserves full disassembly. ## Problem and expected behavior `rea inspect-with-objdump /usr/lib/x86_64-linux-gnu/libcurl.so.4 disassemble --symbol curl_version_info --json` returns native symbol-scoped Evidence inline. Other operations, empty names and NUL-containing names reject selection before artifact/provider admission. ## Change and scope One domain operation list feeds CLI/MCP. A strict disassembly variant owns selection; the provider passes one `--disassemble=<symbol>` argument. The shared workflow preserves the native name, argv, digests and explicit coverage limitation in Evidence, omitting absent JSON parameters. No cache, tool, dependency or installer is added. ## Contract and boundary impact - Semantic owner and earliest changed stage: domain input contract; native selection precedes collection/serialization. - Contribution scope fit and linked maintainer decision: existing read-only static provider; new execution/injection/mutation/recovery authority and a new scope decision are not applicable. - CLI and MCP/tool-catalog contract: additive `--symbol` / `symbol`, valid only for `disassemble`. - Provider, bridge, target-format, or platform compatibility: caller-supplied GNU objdump; verified on Linux x86-64 ELF with GNU 2.40. - Evidence, artifact, provenance, or reconstruction contract: full requested raw output and retained Evidence remain accessible; native matching/boundaries apply, and an empty match does not establish artifact-wide absence. - Target selection, authority, upstream behavior, and cleanup details: existing local artifact, supervision, cancellation, debug-link policy and resource limits. - Generated metadata, package, or installation impact: ignored metadata generated from source; no package/release changes. Expected effects: - [x] Read existing files or captured data. - [x] Launch or attach to a process. (Existing analysis provider; no target execution.) - [ ] Inject instrumentation or run scripts/code. - [ ] Change process memory, registers, files, or configuration. - [ ] Contact network services. - [ ] Recover protected code or work around protections. ## Evidence and regression coverage Current integration head: `a4e3aa6ed`, with main `0fc8462be` merged. The nine feature files are unchanged, and final local verification below was executed on this head. - **293 focused tests / 28 files passed (2 skipped) on head `a4e3aa6ed`**, integrated with main `0fc8462be`. This includes 21 canonical symbol cases, the complete advertised-pattern compatibility check, both SDK schema dialects, provider shutdown, the previously failing source cancellation case, and all four prior replay/fixture/process-global failures repaired by main. - Real packaged CLI/MCP/native stdout equality passed for zlib, libcurl and no-match `/usr/bin/true`, including retained complete Evidence, invalid inputs, CLI rejection and connection reuse. Independent ELF64 PT_LOAD mapping checked the selected 8/422 source bytes earlier; the three final public-artifact MCP replies remain byte-identical after this main integration. - A controlled UTF-8 ELF reproduced wrong-symbol selection before the Unicode fix. Four malformed surrogate forms now fail through actual MCP; valid astral and literal U+FFFD names select distinct expected functions through CLI/MCP/GNU objdump. The fixture was not executed. - The canonical runtime owns UTF-16 validity. Published patterns use the portable hexadecimal NUL escape without surrogate ranges, which scalar Unicode regex engines cannot compile. SDK schema tests cover draft-2020-12/draft-07 with ordinary and Unicode regex modes; the actual full catalog compiles in JS ordinary/`u`/`v` modes, and a real Rust regex probe accepts both valid Unicode names and rejects NUL. No downstream normalization or second analysis implementation was added. ## Validation performed - `npm run build:cached` — passed. - Focused source/contracts/CLI/MCP tests — 293 passed, 2 skipped across 28 files. - Packaged public-artifact and controlled Unicode CLI/MCP workflows — passed as above. - `npm run check:fast` — passed (whole-repository types, lint, module boundaries). - `npm run docs:check` — passed (6 artifact/document tasks, 5 README tests). **CI attribution:** the old `57a53efa9` run exposed our advertised NUL/surrogate-pattern portability defect, fixed in `4d263beab`. Main `b999e7247`, included in integrated main `0fc8462be`, repairs the unrelated Native API replay assertion, ASAR fixture and two process-global mock-isolation failures. All four repaired cases pass in the new 293-test local selection. Hosted source/static/build/docs/reverse-quality, all five test shards and Greptile checks have passed. The [hosted workflow](https://github.com/morluto/rea/actions/runs/39248496874) records the final aggregate status for this head; normal merge remains subject to required checks. Final verification resource receipts cover one CPU, sequential bounded scopes, completed owned cleanup and zero OOM kills. The maximum process-family peak was 4,303.94 MiB (4.20 GiB), within the 8 GiB hard cap. Build / focused regressions / Unicode probe / public-artifact probe / type+lint / docs took 110.929 / 193.366 / 76.519 / 91.805 / 211.302 / 21.109 seconds elapsed, and 110.269 / 163.171 / 59.675 / 71.547 / 210.187 / 20.857 seconds CPU respectively. These are final verification runs, not a repeated before/after cold-task benchmark. Cold-task measurements below are pinned to baseline `dfe8378cf` and initial selection commit `2ddcfd84a`, using the Node SDK on Linux. Cold task timing was not rerun after the input compatibility fixes or latest-main integration. Catalog-only estimates pinned to main `69aa5465c` versus `4d263beab` are 807,446 raw SDK tokens versus 807,550 with this change (+104); the hypothetical input-only function catalog is 183,259 versus 183,363. Only `inspect_with_objdump` changes in that catalog comparison. These are offline cl100k estimates, not actual client/model usage. | Task | Native stdout before → after | Estimated raw SDK task tokens, including discovery | | --- | ---: | ---: | | zlib `zlibVersion` | 1,057,333 → 405 bytes | 3,063,907 → 809,192 | | curl `curl_version_info` | 6,921,190 → 6,923 bytes | 808,419 → 822,352 | These are tiktoken/cl100k estimates, **not model usage**. Curl's baseline needs refusal → retained summary → a 14.56 MB local Evidence export (3 RPCs versus 1); local-file/model consumption is unmeasured, so no overall curl token saving is claimed. Completed cold trials include discovery, verification/oracles and owned cleanup. Ranges below retain every unprofiled trial (2 pairs for zlib; 4 pairs across default/120-second SDK deadline cohorts for curl): | Task | Elapsed seconds before → after | CPU seconds before → after | Family peak MiB before → after | | --- | ---: | ---: | ---: | | zlib | 19.21–21.04 → 19.26–19.50 | 18.66–20.55 → 18.81–18.94 | 660–705 → 554–566 | | curl | 20.11–21.96 → 19.13–56.11 | 19.36–21.19 → 18.54–41.87 | 663–701 → 554–589 | The extra cost of the 45–56 s selected-call outliers remains unexplained; no overall speedup or complex-agent effectiveness is claimed. Profiling found substantial SDK AJV cold compilation with an unchanged output schema. Trials used one CPU, warm OS caches and bounded Linux cgroups; CPU/memory.peak were observed at payload exit with an observer alive (not terminal whole-scope counters). Owned cleanup completed without OOM. ## Compatibility, safety, and release No breaking change, migration or release metadata. Symbol text remains one argv value, with no shell interpolation. Real verification covers GNU 2.40, Debian zlib 1.2.13/libcurl 7.88.1, Linux x86-64. Other platforms/formats and actual model usage remain unverified. ## Post-merge integration check Merged as `3ea0d9489`. Main integrated the separate `49e67aa1d` / `dc936f433` process-ownership changes while this PR's CI was running; the nine feature files match the tested head. A fresh merged-main build passed, but the expanded 30-file check had 306 passed, 16 failed and 2 skipped, and actual MCP zlib selection returned `cleanup_incomplete` despite correct native output and exit 0. The original `file_headers` operation without a symbol also returned the same cleanup error on independently built pre-feature main `dc936f433`, with native exit 0 and 151 bytes matching GNU objdump. An earlier parent control passed, so this is intermittent. The [real Linux evidence is recorded on #4322](https://github.com/morluto/rea/issues/4322#issuecomment-6267248389); the precise cleanup root cause is not localized. Pre-merge required CI is green; successful post-merge provider verification is not claimed. No unrelated process-layer patch was added. ## Author readiness Human owner: @gensokyo-dev Recorded on the owner’s explicit instruction on 2026-10-19: the owner confirmed the required human declarations, authorized the assistant to enter them, and declared the contribution ready to proceed. The technical review and executed validation documented above were performed by the assistant; this note does not claim an additional independent manual 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-11-01 21:43:06 +00:00

closed this issue
added the
pull-request
label

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

No description provided.

Rows
Columns