morluto/rea

Fork 0

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

[PR #2087] [CLOSED] feat(artifacts): inspect classic iNES container regions #5018

New issue

Closed

opened 2026-10-31 00:58:13 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 00:58:13 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/2087
Author: @Zhong-fan
Created: 10/12/2026
Status: ❌ Closed

Base: main ← Head: feat/2072-ines-container

📝 Commits (10+)

414307c feat(artifacts): inspect classic iNES container regions
574f6bb fix(artifacts): preserve iNES source disappearance as integrity failure
5ba8b2f fix(artifacts): revalidate iNES source before parsing header
e6d4d7f fix(artifacts): validate admitted roots before classification
9b89161 fix(artifacts): normalize source loss during root hashing
2f0e4d7 fix(artifacts): bind iNES extraction admission to inventory
ca646b6 test(artifacts): clarify non-executing inspection wording
7cba3ac Merge remote-tracking branch 'origin/main' into feat/2072-ines-container
071f632 fix: preserve binary plist detection with extended magic probe
db24a47 fix: reconcile iNES inspection with upstream summary delivery

📊 Changes

53 files changed (+3169 additions, -204 deletions)

View changed files

📝 .agents/skills/reverse-engineer-anything/references/native-and-artifacts.md (+10 -0)
📝 .github/workflows/ci.yml (+7 -0)
📝 docs/.vitepress/config.ts (+1 -0)
➕ docs/ines-containers.md (+72 -0)
📝 scripts/ci/scopes.mjs (+7 -0)
📝 scripts/lib/browser-popup-e2e.mjs (+55 -10)
📝 scripts/lib/browser-runtime-fixture-lifecycle.mjs (+90 -9)
📝 src/application/BinaryTargetResolver.ts (+61 -24)
📝 src/artifacts/ArtifactHash.ts (+3 -1)
📝 src/artifacts/ArtifactProvider.ts (+0 -1)
📝 src/artifacts/ArtifactReader.ts (+1 -1)
➕ src/artifacts/InesArtifactReader.ts (+187 -0)
📝 src/artifacts/extraction/ArtifactExtraction.ts (+78 -25)
📝 src/artifacts/inventory/ArtifactGraphConstruction.ts (+13 -1)
📝 src/artifacts/inventory/classify.ts (+72 -12)
📝 src/artifacts/inventory/hashStableRootArtifact.ts (+48 -12)
📝 src/artifacts/inventory/reader.ts (+10 -0)
📝 src/artifacts/inventory/scanCanonical.ts (+22 -0)
📝 src/artifacts/inventory/scanReader.ts (+38 -0)
📝 src/artifacts/inventory/types.ts (+5 -0)

...and 33 more files

📄 Description

Summary

Add the container-only first milestone of #2072: classic iNES routing, file-region inventory and byte-exact reconstruction through existing artifact tools. The broader 6502/emulator work remains in #2072 and #1641.

Problem and expected behavior

Valid cartridge bytes named cartridge.data were rejected as an unsupported native binary. They should open as an archive and return original header facts, region offsets, sizes and SHA-256 identities.

Change and scope

One domain parser owns the header/layout. Preserve header, trainer, PRG/CHR units and trailing bytes; verify their actual concatenation against the source digest using the same bytes read for region hashes.
Reuse retained descriptors and safe extraction. Handle short reads, source changes and cancellation consistently; choose the format from current bytes before inventory and choose extraction readers from that fresh manifest.
Express optional container facts at the output-schema owner, removing duplicate graph parsing and downstream field repairs.
Complete the CI follow-up: browser startup waits for a responsive fixture page within its existing budget; cleanup covers content drain plus shutdown, retries survive request cancellation, and macOS explicitly runs iNES and Mach-O summary/page checks.

Supported scope is classic iNES roots with clean reserved bytes. NES 2.0, UNIF, dirty headers and truncation are refused. CPU mapping, native 6502 analysis, emulation and nested cartridge traversal remain outside this increment.

Contract and boundary impact

Semantic owner/earliest stage: domain/inesContainer.ts; signature routing before native detection.
Scope/maintainer decision: offline artifact support and ordinary fixes within existing authority; new execution-provider approval is not applicable.
CLI/MCP: existing open_binary, inspect_artifact, extract_artifact; additive ines format, metadata and observations.
Provider/platform: portable Node reads; existing artifact provider and CLI/MCP workflows.
Evidence/reconstruction: exact source bytes, offsets, hashes and physical region order; header declarations remain distinct from runtime behavior.
Selection/cleanup: explicit database choice remains authoritative; failed closes retain their cause and owner. Existing browser launch/connect authority is preserved.
Metadata: authored guide/skill updated; CI regenerates ignored catalogs and skills.

Expected effects:

Read existing files.
Launch/attach to a process — existing browser cleanup follow-up.
Change files — extracted regions and private browser-profile cleanup.

Evidence and regression coverage

Executed source-owned fixtures cover counts, trainer/zero-CHR/tail, hashes/reconstruction, short reads, source changes, cancellation and close ownership.
Compiled CLI and real SDK stdio MCP verify inventory/extraction parity, failed target-switch recovery and refresh to all eight ZIP families.
Regressions failed before the routing, cleanup-retry, admission-cancellation and lifecycle-schema fixes. The ZIP contract now shares main's #2478 and #2493 implementations.

Representative executed output (excerpt):

{"header":{"format":"ines","mapper":19,"trainer_present":true},"reconstruction":{"bytes":41492,"verified":true}}

Tests independently check region offsets/hashes, the source digest and extracted concatenation. Added regressions reproduce stale plist/PE routing, an extra full payload read, and changed-file admission incorrectly reported as I/O. The Windows race changes the real file between descriptor stat and admission lstat; it now reports integrity failure.

Observed, derived and inferred claims remain distinguishable.
Identity, provenance and failed attempts remain preserved.
Unsupported or incomplete outcomes remain visible.

Validation performed

Current head 480a467, integrated with main cae7620:

npm run check:ci, npm run docs:check, commit/push checks and post-commit npm run format:check — passed without relaxing checks. The commit tree matches the tested tree.
npm run test:focused -- <14 files> on Windows (Node 24.18.0, npm 11.16.0): 218 passed, 9 platform skips. Covers Evidence/snapshot/raw-6502 boundaries, iNES inventory/extraction, compiled CLI and SDK stdio MCP, replacement formats, cancellation, browser fixture readiness/cleanup and Windows launcher isolation.
Linux Vitest on an isolated snapshot of the same tree: 68 passed across Evidence, snapshot, raw-binary and snapshot persistence tests, including the actual POSIX private-file mode assertion.
npm run verify:browser:runtime with real Windows Chrome 155 — passed the public CLI/MCP runtime workflow. Earlier Windows/Linux real-browser and source-map checks at 00f3169 also passed; the startup helper is unchanged in this merge.
Upstream CI for this head is pending; it has not passed yet.

Compatibility, safety, and release

Breaking changes/migrations: none; format and observations are additive.
OS/provider coverage: source-owned iNES workflows on Linux, macOS and Windows; real browser verification in CI. This lane requires no native analysis engine or emulator.
Package/release: no dependency, version or installer changes; no build outputs committed.
Containment: bounded header/count parsing, streamed payloads, stable source identity and owned output. Fixtures use source-owned bytes.

Before merge

Concrete problem and related issue.
Material scope and authority resolved.
One focused outcome and conventional title.
Tests cover changed behavior and meaningful failures.
Owning docs, contracts and generated metadata updated.
Representative CLI/MCP output included.
Final diff checked 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/2087 **Author:** [@Zhong-fan](https://github.com/Zhong-fan) **Created:** 10/12/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `feat/2072-ines-container` --- ### 📝 Commits (10+) - [`414307c`](https://github.com/morluto/rea/commit/414307c9d561f7811d03752e530f104faf88abd0) feat(artifacts): inspect classic iNES container regions - [`574f6bb`](https://github.com/morluto/rea/commit/574f6bb3bdcaedc42ab8564f5073a7d46da061e8) fix(artifacts): preserve iNES source disappearance as integrity failure - [`5ba8b2f`](https://github.com/morluto/rea/commit/5ba8b2f6bfa3ba5e967ad1163ea048205069165d) fix(artifacts): revalidate iNES source before parsing header - [`e6d4d7f`](https://github.com/morluto/rea/commit/e6d4d7fb8452cfa54680935408e33e4474c689d8) fix(artifacts): validate admitted roots before classification - [`9b89161`](https://github.com/morluto/rea/commit/9b8916152badd33249ac0045ad4ee072863935ef) fix(artifacts): normalize source loss during root hashing - [`2f0e4d7`](https://github.com/morluto/rea/commit/2f0e4d79ea540de214c6daa04b6bc4a2b186ce3e) fix(artifacts): bind iNES extraction admission to inventory - [`ca646b6`](https://github.com/morluto/rea/commit/ca646b6cb9894e9491714e71a7209f46892a7a12) test(artifacts): clarify non-executing inspection wording - [`7cba3ac`](https://github.com/morluto/rea/commit/7cba3acaa42d3e02de2d28ce035c90efd443a34f) Merge remote-tracking branch 'origin/main' into feat/2072-ines-container - [`071f632`](https://github.com/morluto/rea/commit/071f632e92ac1c98126e93886a7c0daa91c3b2fc) fix: preserve binary plist detection with extended magic probe - [`db24a47`](https://github.com/morluto/rea/commit/db24a47c1513b51269289d2768b588c397e69ab2) fix: reconcile iNES inspection with upstream summary delivery ### 📊 Changes **53 files changed** (+3169 additions, -204 deletions) <details> <summary>View changed files</summary> 📝 `.agents/skills/reverse-engineer-anything/references/native-and-artifacts.md` (+10 -0) 📝 `.github/workflows/ci.yml` (+7 -0) 📝 `docs/.vitepress/config.ts` (+1 -0) ➕ `docs/ines-containers.md` (+72 -0) 📝 `scripts/ci/scopes.mjs` (+7 -0) 📝 `scripts/lib/browser-popup-e2e.mjs` (+55 -10) 📝 `scripts/lib/browser-runtime-fixture-lifecycle.mjs` (+90 -9) 📝 `src/application/BinaryTargetResolver.ts` (+61 -24) 📝 `src/artifacts/ArtifactHash.ts` (+3 -1) 📝 `src/artifacts/ArtifactProvider.ts` (+0 -1) 📝 `src/artifacts/ArtifactReader.ts` (+1 -1) ➕ `src/artifacts/InesArtifactReader.ts` (+187 -0) 📝 `src/artifacts/extraction/ArtifactExtraction.ts` (+78 -25) 📝 `src/artifacts/inventory/ArtifactGraphConstruction.ts` (+13 -1) 📝 `src/artifacts/inventory/classify.ts` (+72 -12) 📝 `src/artifacts/inventory/hashStableRootArtifact.ts` (+48 -12) 📝 `src/artifacts/inventory/reader.ts` (+10 -0) 📝 `src/artifacts/inventory/scanCanonical.ts` (+22 -0) 📝 `src/artifacts/inventory/scanReader.ts` (+38 -0) 📝 `src/artifacts/inventory/types.ts` (+5 -0) _...and 33 more files_ </details> ### 📄 Description ## Summary Add the container-only first milestone of #2072: classic iNES routing, file-region inventory and byte-exact reconstruction through existing artifact tools. The broader 6502/emulator work remains in #2072 and #1641. ## Problem and expected behavior Valid cartridge bytes named `cartridge.data` were rejected as an unsupported native binary. They should open as an archive and return original header facts, region offsets, sizes and SHA-256 identities. ## Change and scope - One domain parser owns the header/layout. Preserve header, trainer, PRG/CHR units and trailing bytes; verify their actual concatenation against the source digest using the same bytes read for region hashes. - Reuse retained descriptors and safe extraction. Handle short reads, source changes and cancellation consistently; choose the format from current bytes before inventory and choose extraction readers from that fresh manifest. - Express optional container facts at the output-schema owner, removing duplicate graph parsing and downstream field repairs. - Complete the CI follow-up: browser startup waits for a responsive fixture page within its existing budget; cleanup covers content drain plus shutdown, retries survive request cancellation, and macOS explicitly runs iNES and Mach-O summary/page checks. Supported scope is classic iNES roots with clean reserved bytes. NES 2.0, UNIF, dirty headers and truncation are refused. CPU mapping, native 6502 analysis, emulation and nested cartridge traversal remain outside this increment. ## Contract and boundary impact - Semantic owner/earliest stage: `domain/inesContainer.ts`; signature routing before native detection. - Scope/maintainer decision: offline artifact support and ordinary fixes within existing authority; new execution-provider approval is not applicable. - CLI/MCP: existing `open_binary`, `inspect_artifact`, `extract_artifact`; additive `ines` format, metadata and observations. - Provider/platform: portable Node reads; existing artifact provider and CLI/MCP workflows. - Evidence/reconstruction: exact source bytes, offsets, hashes and physical region order; header declarations remain distinct from runtime behavior. - Selection/cleanup: explicit database choice remains authoritative; failed closes retain their cause and owner. Existing browser launch/connect authority is preserved. - Metadata: authored guide/skill updated; CI regenerates ignored catalogs and skills. Expected effects: - [x] Read existing files. - [x] Launch/attach to a process — existing browser cleanup follow-up. - [x] Change files — extracted regions and private browser-profile cleanup. ## Evidence and regression coverage - Executed source-owned fixtures cover counts, trainer/zero-CHR/tail, hashes/reconstruction, short reads, source changes, cancellation and close ownership. - Compiled CLI and real SDK stdio MCP verify inventory/extraction parity, failed target-switch recovery and refresh to all eight ZIP families. - Regressions failed before the routing, cleanup-retry, admission-cancellation and lifecycle-schema fixes. The ZIP contract now shares main's #2478 and #2493 implementations. Representative executed output (excerpt): ```json {"header":{"format":"ines","mapper":19,"trainer_present":true},"reconstruction":{"bytes":41492,"verified":true}} ``` Tests independently check region offsets/hashes, the source digest and extracted concatenation. Added regressions reproduce stale plist/PE routing, an extra full payload read, and changed-file admission incorrectly reported as I/O. The Windows race changes the real file between descriptor stat and admission lstat; it now reports integrity failure. - [x] Observed, derived and inferred claims remain distinguishable. - [x] Identity, provenance and failed attempts remain preserved. - [x] Unsupported or incomplete outcomes remain visible. ## Validation performed Current head `480a467`, integrated with main `cae7620`: - `npm run check:ci`, `npm run docs:check`, commit/push checks and post-commit `npm run format:check` — passed without relaxing checks. The commit tree matches the tested tree. - `npm run test:focused -- <14 files>` on Windows (Node 24.18.0, npm 11.16.0): 218 passed, 9 platform skips. Covers Evidence/snapshot/raw-6502 boundaries, iNES inventory/extraction, compiled CLI and SDK stdio MCP, replacement formats, cancellation, browser fixture readiness/cleanup and Windows launcher isolation. - Linux Vitest on an isolated snapshot of the same tree: 68 passed across Evidence, snapshot, raw-binary and snapshot persistence tests, including the actual POSIX private-file mode assertion. - `npm run verify:browser:runtime` with real Windows Chrome 155 — passed the public CLI/MCP runtime workflow. Earlier Windows/Linux real-browser and source-map checks at `00f3169` also passed; the startup helper is unchanged in this merge. - [Upstream CI for this head](https://github.com/morluto/rea/actions/runs/38407856237) is pending; it has not passed yet. ## Compatibility, safety, and release - Breaking changes/migrations: none; format and observations are additive. - OS/provider coverage: source-owned iNES workflows on Linux, macOS and Windows; real browser verification in CI. This lane requires no native analysis engine or emulator. - Package/release: no dependency, version or installer changes; no build outputs committed. - Containment: bounded header/count parsing, streamed payloads, stable source identity and owned output. Fixtures use source-owned bytes. ## Before merge - [x] Concrete problem and related issue. - [x] Material scope and authority resolved. - [x] One focused outcome and conventional title. - [x] Tests cover changed behavior and meaningful failures. - [x] Owning docs, contracts and generated metadata updated. - [x] Representative CLI/MCP output included. - [x] Final diff checked 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-10-31 00:58:13 +00:00

closed this issue
added the
pull-request
label

bsakaguchi referenced this issue

2026-11-01 21:42:41 +00:00

[PR #5018] [MERGED] fix(tests): preserve temporary fixture ownership #6736

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

No description provided.

Rows
Columns