morluto/rea

Fork 0

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

[PR #7454] test(windows): declare the native loader fixture optional header #7847

New issue

Open

opened 2026-11-05 01:48:06 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-11-05 01:48:06 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/7454
Author: @rudycelekli
Created: 10/27/2026
Status: 🔄 Open

Base: main ← Head: fix/rea-windows-loader-fixture

📝 Commits (1)

44b6640 test(windows): declare the native loader fixture optional header

📊 Changes

1 file changed (+2 additions, -1 deletions)

View changed files

📝 src/windows/WindowsNativeLoader.test.ts (+2 -1)

📄 Description

Summary

Restore the native-loader tests' valid PE fixture so they reach compatibility, digest and authority inspection instead of stopping at PE admission.

Problem and expected behavior

The fixture writes a PE32+ magic but leaves COFF SizeOfOptionalHeader zero. The artifact validator now reads only the declared optional-header bytes, correctly rejecting this fixture before eight tests reach their intended assertions. A valid-fixture test must declare the bytes it supplies.

Change and scope

Match the existing windowsNativeArtifact.test.ts fixture: allocate 152 bytes and declare a 64-byte optional header beginning at offset 88. The digest is still computed from the fixture bytes. Only WindowsNativeLoader.test.ts changes; production admission and negative assertions are unchanged.

The existing ARM64 and missing-manifest PRs touch this file but retain the undeclared header; this is a separate fixture repair. No matching issue is linked: the failure was observed directly in current source CI. This does not claim to repair all current main failures.

Contract and boundary impact

Semantic owner: the test's synthetic PE producer; production artifact validation stays unchanged.
Contribution scope: ordinary test repair; no new capability or maintainer scope decision required.
CLI/MCP, Evidence, provider, target-selection, cleanup, package and generated metadata contracts: unchanged.
Authority/effects: tests use in-memory synthetic bytes and injected loader callbacks; they do not load a native addon or execute a target.

Evidence and regression coverage

On unchanged upstream 178ab2629048c6860ac7adbcc512c442a11a1406, eight native-loader cases fail before their intended admission stage. The corrected loader suite passes all 22 cases. Together with the artifact validator's 29 tests, 51 cases pass, including malformed header sizes, architecture, digest and inspection controls. An independent review reran the same 51 cases successfully.

Validation performed

macOS, Node 24.22.1, npm 11.16.0; candidate 44b6640da6919a1a579ffbb0b688f3db37f3934e.

npm run test:local -- src/windows/WindowsNativeLoader.test.ts src/domain/windowsNativeArtifact.test.ts — 51 passed.
npm run check — five tasks passed, then Knip failed on existing main exports staticFileUrlPath, AndroidManifestComponentInventory and ObjcClassImportBindingCoverage. These were independently reproduced on unchanged upstream; the full gate is not passing. Later chained checks were not reached.
Normal pre-commit lint/format hook and git diff --check — passed.

Compatibility, safety, and release

No runtime or package changes. No actual Windows host, native addon, Ghidra or Hopper execution was performed; these tests establish the simulated loader boundary only. Hosted CI and genuine human readiness remain pending.

Author readiness

Human owner: @rudycelekli

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

Required final-head checks pass.
Human owner completes the readiness declarations.

_{🔄 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/7454 **Author:** [@rudycelekli](https://github.com/rudycelekli) **Created:** 10/27/2026 **Status:** 🔄 Open **Base:** `main` ← **Head:** `fix/rea-windows-loader-fixture` --- ### 📝 Commits (1) - [`44b6640`](https://github.com/morluto/rea/commit/44b6640da6919a1a579ffbb0b688f3db37f3934e) test(windows): declare the native loader fixture optional header ### 📊 Changes **1 file changed** (+2 additions, -1 deletions) <details> <summary>View changed files</summary> 📝 `src/windows/WindowsNativeLoader.test.ts` (+2 -1) </details> ### 📄 Description ## Summary Restore the native-loader tests' valid PE fixture so they reach compatibility, digest and authority inspection instead of stopping at PE admission. ## Problem and expected behavior The fixture writes a PE32+ magic but leaves COFF `SizeOfOptionalHeader` zero. The artifact validator now reads only the declared optional-header bytes, correctly rejecting this fixture before eight tests reach their intended assertions. A valid-fixture test must declare the bytes it supplies. ## Change and scope Match the existing `windowsNativeArtifact.test.ts` fixture: allocate 152 bytes and declare a 64-byte optional header beginning at offset 88. The digest is still computed from the fixture bytes. Only `WindowsNativeLoader.test.ts` changes; production admission and negative assertions are unchanged. The existing ARM64 and missing-manifest PRs touch this file but retain the undeclared header; this is a separate fixture repair. No matching issue is linked: the failure was observed directly in current source CI. This does not claim to repair all current main failures. ## Contract and boundary impact - Semantic owner: the test's synthetic PE producer; production artifact validation stays unchanged. - Contribution scope: ordinary test repair; no new capability or maintainer scope decision required. - CLI/MCP, Evidence, provider, target-selection, cleanup, package and generated metadata contracts: unchanged. - Authority/effects: tests use in-memory synthetic bytes and injected loader callbacks; they do not load a native addon or execute a target. ## Evidence and regression coverage On unchanged upstream `178ab2629048c6860ac7adbcc512c442a11a1406`, eight native-loader cases fail before their intended admission stage. The corrected loader suite passes all 22 cases. Together with the artifact validator's 29 tests, 51 cases pass, including malformed header sizes, architecture, digest and inspection controls. An independent review reran the same 51 cases successfully. ## Validation performed macOS, Node 24.22.1, npm 11.16.0; candidate `44b6640da6919a1a579ffbb0b688f3db37f3934e`. - `npm run test:local -- src/windows/WindowsNativeLoader.test.ts src/domain/windowsNativeArtifact.test.ts` — 51 passed. - `npm run check` — five tasks passed, then Knip failed on existing main exports `staticFileUrlPath`, `AndroidManifestComponentInventory` and `ObjcClassImportBindingCoverage`. These were independently reproduced on unchanged upstream; the full gate is not passing. Later chained checks were not reached. - Normal pre-commit lint/format hook and `git diff --check` — passed. ## Compatibility, safety, and release No runtime or package changes. No actual Windows host, native addon, Ghidra or Hopper execution was performed; these tests establish the simulated loader boundary only. Hosted CI and genuine human readiness remain pending. ## Author readiness Human owner: @rudycelekli - [ ] 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 - [ ] Required final-head checks pass. - [ ] Human owner completes the readiness declarations. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi added the

pull-request

label

2026-11-05 01:48:06 +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#7847

No description provided.

Rows
Columns