morluto/rea

Fork 0

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

[PR #6425] [MERGED] fix(il2cpp): name the metadata file when linking finds malformed metadata #7351

New issue

Closed

opened 2026-11-01 21:47:15 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-11-01 21:47:15 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/6425
Author: @Sriram-PR
Created: 10/23/2026
Status: ✅ Merged
Merged: 10/23/2026
Merged by: @morluto

Base: main ← Head: fix/il2cpp-metadata-failure-names-metadata

📝 Commits (1)

935dad7 fix(il2cpp): name the metadata file when linking finds malformed metadata

📊 Changes

2 files changed (+61 additions, -3 deletions)

View changed files

📝 src/il2cpp/Il2cppMetadataProvider.ts (+10 -3)
📝 tests/boundary/il2cpp/il2cppMetadataProvider.test.ts (+51 -0)

📄 Description

Summary

When inspect_il2cpp_metadata gets a binary_path, some metadata names (generic parameter names, for example) are only read while linking against the binary. If one of those reads fails, the issue correctly points at path but the message calls the native binary corrupt: Selected artifact .../GameAssembly.dylib is corrupt: invalid-string: .... The same binary links fine once the metadata index is fixed.

Fixes #6409

Problem and expected behavior

Smallest trigger: version 39 metadata with one generic parameter whose name index points past the string table, a field typed as that parameter, and a matching native fixture. Metadata-only inspection succeeds (the name is read on demand, as documented). With binary_path, main returns invalid_request with path: ["path"], reason: "invalid_format" and a message naming the binary. Expected: the same refusal, with the message naming the metadata file.

Change and scope

The linking step's catch calls failureFor(cause, binaryPath, "binary_path", BINARY_BYTES), so inside failureForPrimary the path argument is the binary for every failure, including an Il2cppMetadataFormatFailure raised by a lazy metadata read. failureFor and failureForPrimary now take the metadata path as an optional last argument (defaulting to path, so metadata-only callers don't change), and metadata format failures use it in both the invalid_format message and the unsupported target.
Binary format failures and artifact read failures for the binary still name the binary.
Not included: no change to when names are read or to the issue field, which was already path.

Contribution scope: ordinary fix within existing boundaries; no scope decision needed.

Contract and boundary impact

Semantic owner and earliest changed stage: failureForPrimary in src/il2cpp/Il2cppMetadataProvider.ts, where the linking failure becomes an AnalysisError.
Contribution scope fit and linked maintainer decision: not applicable (bug fix, #6409).
CLI and MCP/tool-catalog contract: no schema change. Only the issue message (and the unsupported-target subject) for metadata failures during linking changes, from the binary path to the metadata path.
Provider, bridge, target-format, or platform compatibility: none.
Evidence, artifact, provenance, or reconstruction contract: none; the partial source observation is unchanged.
Target selection, authority, upstream behavior, and cleanup details: not applicable.
Generated metadata, package, or installation impact: none.

Expected effects (check known effects; use documentation-only on its own).
Unchecked items do not establish absence; leave uncertainties visible.

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

Tests added or updated: tests/boundary/il2cpp/il2cppMetadataProvider.test.ts adds names the metadata, not the binary, when linking reads a malformed metadata name, built from the issue's fixture edits. It expects invalid_format at ["path"] with Selected artifact <metadata path> is corrupt: invalid-string:, then fixes only the name index and expects the same binary to link.

Base reproduction: the issue's MCP/CLI test passes on main da9a580a (its assertion encodes the binary-path message). The new test fails on main's source with:

-       "message": StringContaining "Selected artifact /tmp/rea-il2cpp-link-metadata-name-5ek8Qs/global-metadata.dat is corrupt: invalid-string:",
+       "message": "Selected artifact /tmp/rea-il2cpp-link-metadata-name-5ek8Qs/GameAssembly.dylib is corrupt: invalid-string: generic parameter 0 name string index 119 has no terminated entry inside the 119-byte string section",

User-visible CLI/MCP output: the issue message now reads Selected artifact <dir>/global-metadata.dat is corrupt: invalid-string: generic parameter 0 name string index 119 has no terminated entry inside the 119-byte string section; code, field and reason are unchanged.
Remaining proof gaps: I didn't build a fixture for an unsupported metadata failure during linking; that branch gets the same path change by reading the code.

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

npx vitest run tests/boundary/il2cpp/il2cppMetadataProvider.test.ts: 21 passed (the new test fails on main's source)
npm run test:focused -- tests/boundary/il2cpp/*.test.ts tests/acceptance/il2cppMetadataCli.test.ts tests/boundary/cli/artifactInputLimits.test.ts tests/process-global/stableArtifactConsumers.test.ts: 208 passed
npx vitest run src/application/il2cpp/ src/domain/analysisView/il2cppMetadataView.test.ts src/native/metadata/Il2cppContainer.test.ts: 47 passed
npm run check:fast: typecheck and lint passed

Compatibility, safety, and release

Breaking changes or migration steps: none; only a diagnostic message changes.
Real Hopper/Ghidra, browser, or OS coverage: Linux x64, Node 24.18.0; no provider or Unity install involved.
Package or release metadata impact: none.
Security, privacy, process, or containment review: none; the native file is not loaded or executed.

Author readiness

Human owner: @Sriram-PR

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/6425 **Author:** [@Sriram-PR](https://github.com/Sriram-PR) **Created:** 10/23/2026 **Status:** ✅ Merged **Merged:** 10/23/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `fix/il2cpp-metadata-failure-names-metadata` --- ### 📝 Commits (1) - [`935dad7`](https://github.com/morluto/rea/commit/935dad7d205ab9cbe020880a047564ac3b90fcfa) fix(il2cpp): name the metadata file when linking finds malformed metadata ### 📊 Changes **2 files changed** (+61 additions, -3 deletions) <details> <summary>View changed files</summary> 📝 `src/il2cpp/Il2cppMetadataProvider.ts` (+10 -3) 📝 `tests/boundary/il2cpp/il2cppMetadataProvider.test.ts` (+51 -0) </details> ### 📄 Description ## Summary When `inspect_il2cpp_metadata` gets a `binary_path`, some metadata names (generic parameter names, for example) are only read while linking against the binary. If one of those reads fails, the issue correctly points at `path` but the message calls the native binary corrupt: `Selected artifact .../GameAssembly.dylib is corrupt: invalid-string: ...`. The same binary links fine once the metadata index is fixed. Fixes #6409 ## Problem and expected behavior Smallest trigger: version 39 metadata with one generic parameter whose name index points past the string table, a field typed as that parameter, and a matching native fixture. Metadata-only inspection succeeds (the name is read on demand, as documented). With `binary_path`, main returns `invalid_request` with `path: ["path"]`, `reason: "invalid_format"` and a message naming the binary. Expected: the same refusal, with the message naming the metadata file. ## Change and scope - The linking step's catch calls `failureFor(cause, binaryPath, "binary_path", BINARY_BYTES)`, so inside `failureForPrimary` the `path` argument is the binary for every failure, including an `Il2cppMetadataFormatFailure` raised by a lazy metadata read. `failureFor` and `failureForPrimary` now take the metadata path as an optional last argument (defaulting to `path`, so metadata-only callers don't change), and metadata format failures use it in both the `invalid_format` message and the `unsupported` target. - Binary format failures and artifact read failures for the binary still name the binary. - Not included: no change to when names are read or to the issue field, which was already `path`. Contribution scope: ordinary fix within existing boundaries; no scope decision needed. ## Contract and boundary impact - Semantic owner and earliest changed stage: `failureForPrimary` in `src/il2cpp/Il2cppMetadataProvider.ts`, where the linking failure becomes an `AnalysisError`. - Contribution scope fit and linked maintainer decision: not applicable (bug fix, #6409). - CLI and MCP/tool-catalog contract: no schema change. Only the issue message (and the unsupported-target subject) for metadata failures during linking changes, from the binary path to the metadata path. - Provider, bridge, target-format, or platform compatibility: none. - Evidence, artifact, provenance, or reconstruction contract: none; the partial source observation is unchanged. - Target selection, authority, upstream behavior, and cleanup details: not applicable. - Generated metadata, package, or installation impact: none. Expected effects (check known effects; use documentation-only on its own). Unchecked items do not establish absence; leave uncertainties visible. - [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. - [ ] 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 - Tests added or updated: `tests/boundary/il2cpp/il2cppMetadataProvider.test.ts` adds `names the metadata, not the binary, when linking reads a malformed metadata name`, built from the issue's fixture edits. It expects `invalid_format` at `["path"]` with `Selected artifact <metadata path> is corrupt: invalid-string:`, then fixes only the name index and expects the same binary to link. - Base reproduction: the issue's MCP/CLI test passes on main da9a580a (its assertion encodes the binary-path message). The new test fails on main's source with: ``` - "message": StringContaining "Selected artifact /tmp/rea-il2cpp-link-metadata-name-5ek8Qs/global-metadata.dat is corrupt: invalid-string:", + "message": "Selected artifact /tmp/rea-il2cpp-link-metadata-name-5ek8Qs/GameAssembly.dylib is corrupt: invalid-string: generic parameter 0 name string index 119 has no terminated entry inside the 119-byte string section", ``` - User-visible CLI/MCP output: the issue message now reads `Selected artifact <dir>/global-metadata.dat is corrupt: invalid-string: generic parameter 0 name string index 119 has no terminated entry inside the 119-byte string section`; code, field and reason are unchanged. - Remaining proof gaps: I didn't build a fixture for an `unsupported` metadata failure during linking; that branch gets the same path change by reading the code. 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 - `npx vitest run tests/boundary/il2cpp/il2cppMetadataProvider.test.ts`: 21 passed (the new test fails on main's source) - `npm run test:focused -- tests/boundary/il2cpp/*.test.ts tests/acceptance/il2cppMetadataCli.test.ts tests/boundary/cli/artifactInputLimits.test.ts tests/process-global/stableArtifactConsumers.test.ts`: 208 passed - `npx vitest run src/application/il2cpp/ src/domain/analysisView/il2cppMetadataView.test.ts src/native/metadata/Il2cppContainer.test.ts`: 47 passed - `npm run check:fast`: typecheck and lint passed ## Compatibility, safety, and release - Breaking changes or migration steps: none; only a diagnostic message changes. - Real Hopper/Ghidra, browser, or OS coverage: Linux x64, Node 24.18.0; no provider or Unity install involved. - Package or release metadata impact: none. - Security, privacy, process, or containment review: none; the native file is not loaded or executed. ## Author readiness Human owner: @Sriram-PR - [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:47:15 +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#7351

No description provided.

Rows
Columns