morluto/rea

Fork 0

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

[PR #3245] [CLOSED] fix(artifacts): respect ZIP legacy filename encoding #5673

New issue

Closed

opened 2026-10-31 01:08:30 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 01:08:30 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/3245
Author: @xu91102
Created: 10/14/2026
Status: ❌ Closed

Base: main ← Head: codex/zip-legacy-name-encoding

📝 Commits (2)

4d383bf fix(artifacts): respect ZIP legacy filename encoding
2e78035 Merge commit '29d3dcb7ea' into codex/zip-legacy-name-encoding

📊 Changes

2 files changed (+8 additions, -2 deletions)

View changed files

📝 src/artifacts/ZipArtifactReader.ts (+3 -0)
📝 tests/boundary/filesystem/artifactZipIntegrity.test.ts (+5 -2)

📄 Description

Summary

Unflagged ZIP names whose bytes also form valid UTF-8 were silently renamed during inspection. Two distinct legacy/UTF-8 members could consequently fail with Artifact path collision. Specify CP437 as the reader's legacy fallback so the existing flag and Unicode Path rules determine each name.

Fixes #3237

Problem and expected behavior

The bytes c3 a9 2e 74 78 74 mean ├⌐.txt with general-purpose bit 11 clear and é.txt with it set. Python zipfile reads both members and verifies their distinct contents and CRCs. On the affected base, REA instead named both é.txt. The shared reader now retains both correct names through inspection, inventory and extraction.

Change and scope

Set filenameEncoding: "cp437" at ZipArtifactReader, where raw directory names enter REA. In the locked zip.js implementation, bit 11 still selects UTF-8 and valid Unicode Path fields still override the legacy name. The existing filename matrix adds both flag controls; its ASCII carrier now matches the requested raw-name length.

Contribution scope: ordinary format-correctness fix within existing archive operations; a separate maintainer capability decision is not applicable.

Contract and boundary impact

Semantic owner and earliest changed stage: ZipArtifactReader name decoding, before inventory path identity.
CLI and MCP/tool-catalog contract: existing arguments and schemas; the affected names and resulting occurrence identities now match the stored format.
Provider/platform compatibility: existing ZIP-family reader; flag-selected UTF-8 and validated Unicode Path fields retain their behavior.
Evidence/provenance: archive and member content digests are unchanged; corrected names distinguish the two occurrences.
Target selection, authority and cleanup: existing selected-archive reads and extraction destination/resource owners.
Generated metadata/package/installation impact: none.

Expected existing workflow effects:

Read selected archive files.
Write extraction outputs when the existing extraction operation is selected.

Evidence and regression coverage

Base 500e32399a3bc827e17fb4adaa97db3bc0df29b5: public CLI wrong-name and two-member collision reproduced; Python independently reads both payloads. The corrected filename regression fails 1 test while 19 controls pass on that base.
Final public CLI: the legacy-only archive reports ├⌐.txt; the combined archive retains é.txt and ├⌐.txt.
Final actual stdio MCP: open_binary and inspect_artifact(detail: "complete") retain both names, verified member SHA-256 values and the unchanged archive SHA-256. Client/transport closure completes.
Final actual CLI extraction also produces both on-disk names and the exact legacy/utf8 bytes, preserving the original archive digest and verified containment.
Existing malformed UTF-8, Unicode Path, CRC, overlap, encrypted-member, duplicate-member, path and resource controls pass.
Early probe mistakes (fixture carrier length and invalid MCP detail: "full") are excluded from product failures and passing acceptance claims.

Validation performed

Pinned Node 24.18.0/npm 11.16.0, npm ci.
Final integrated head 2e78035eae7d6f0cea327a56741b8b21a102605a: ZIP reader/integrity/output and both artifact MCP suites plus EVM boundary suite: 56 passed, 29 skipped, 6 files. The skips are 1 Linux /proc control and 28 Linux x64 EVM cases; those EVM cases were not executed on macOS. The five ZIP suites retain their original 54 passing tests.
npm run check: all 6 tasks and architecture/module guards pass.
npm run verify:test-discovery: all 1252 test files discovered exactly once.
Actual compiled CLI and stdio MCP checks described above; independent Standards and Spec review on the fixed complete patch.

The original 4d383bf delivery had two completed public CLI runs per version using the same archive and locked dependencies. Elapsed seconds: before 2.62/1.27, after 1.31/1.42; user/system CPU seconds: before 1.90/0.42 and 1.54/0.19, after 1.46/0.19 and 1.61/0.22. Sampled process-family peak RSS was 343936–345504 KiB before and 338240–350000 KiB after. These are small whole-workflow observations with uncontrolled caches and estimated family RSS, not an isolated performance comparison. Token usage was not measured.

Compatibility, safety, and release

Ordinary correction of supported legacy names; existing caller parameters continue to work.
Executed on macOS arm64 with real ZIP bytes, Python zipfile, compiled CLI and stdio MCP. Windows and Linux execution remain unverified locally; the final commit's CI is reported separately.
Existing name/path, CRC/overlap, output and resource checks remain at their original boundaries. There are no new target execution or network effects.
Package/release metadata changes: none.

CI follow-up

The initial hosted CI 38522145048 failed two EVM boundary assertions in synthetic merge a874486 (base e579122 plus 4d383bf). Complete per-job logs show one transferred cleanup owner being overwritten by undefined and one reply-error expectation disagreeing with the shared unreadable_output contract. Portable replays those failures; remaining shards were cancelled by fail-fast. This is not a ZIP assertion failure, and the original CI remains FAIL.

Accepted main 29d3dcb including upstream 07540cbc, which retains ownership and bounded error output and aligns the diagnostic expectations; no parallel EVM implementation was added. Final ZIP contribution bytes and both reviewed files exactly match the independent review patch. Locked dependencies were reinstalled; final related tests, all six check tasks/guards, discovery and actual stdio inspection pass. Final Linux x64 EVM execution must be verified by the new hosted CI; macOS skips do not establish it. Required CI on the new pushed SHA is pending.

Before merge

Concrete issue and focused Conventional Commit title.
Existing contribution scope and authority apply.
Related issue linked; changed behavior and failure controls covered.
Public CLI/MCP representative output and verification boundaries stated.
Complete diff checked for unrelated changes and unsupported claims.
Required CI on the final pushed commit completed successfully.

_{🔄 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/3245 **Author:** [@xu91102](https://github.com/xu91102) **Created:** 10/14/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `codex/zip-legacy-name-encoding` --- ### 📝 Commits (2) - [`4d383bf`](https://github.com/morluto/rea/commit/4d383bfc59b7081d4c8b43da86fe4817596160b2) fix(artifacts): respect ZIP legacy filename encoding - [`2e78035`](https://github.com/morluto/rea/commit/2e78035eae7d6f0cea327a56741b8b21a102605a) Merge commit '29d3dcb7ea85b3be37cd81e977ae4e09cebdd2fc' into codex/zip-legacy-name-encoding ### 📊 Changes **2 files changed** (+8 additions, -2 deletions) <details> <summary>View changed files</summary> 📝 `src/artifacts/ZipArtifactReader.ts` (+3 -0) 📝 `tests/boundary/filesystem/artifactZipIntegrity.test.ts` (+5 -2) </details> ### 📄 Description ## Summary Unflagged ZIP names whose bytes also form valid UTF-8 were silently renamed during inspection. Two distinct legacy/UTF-8 members could consequently fail with `Artifact path collision`. Specify CP437 as the reader's legacy fallback so the existing flag and Unicode Path rules determine each name. Fixes #3237 ## Problem and expected behavior The bytes `c3 a9 2e 74 78 74` mean `├⌐.txt` with general-purpose bit 11 clear and `é.txt` with it set. Python `zipfile` reads both members and verifies their distinct contents and CRCs. On the affected base, REA instead named both `é.txt`. The shared reader now retains both correct names through inspection, inventory and extraction. ## Change and scope Set `filenameEncoding: "cp437"` at `ZipArtifactReader`, where raw directory names enter REA. In the locked zip.js implementation, bit 11 still selects UTF-8 and valid Unicode Path fields still override the legacy name. The existing filename matrix adds both flag controls; its ASCII carrier now matches the requested raw-name length. Contribution scope: ordinary format-correctness fix within existing archive operations; a separate maintainer capability decision is not applicable. ## Contract and boundary impact - Semantic owner and earliest changed stage: `ZipArtifactReader` name decoding, before inventory path identity. - CLI and MCP/tool-catalog contract: existing arguments and schemas; the affected names and resulting occurrence identities now match the stored format. - Provider/platform compatibility: existing ZIP-family reader; flag-selected UTF-8 and validated Unicode Path fields retain their behavior. - Evidence/provenance: archive and member content digests are unchanged; corrected names distinguish the two occurrences. - Target selection, authority and cleanup: existing selected-archive reads and extraction destination/resource owners. - Generated metadata/package/installation impact: none. Expected existing workflow effects: - [x] Read selected archive files. - [x] Write extraction outputs when the existing extraction operation is selected. ## Evidence and regression coverage - Base `500e32399a3bc827e17fb4adaa97db3bc0df29b5`: public CLI wrong-name and two-member collision reproduced; Python independently reads both payloads. The corrected filename regression fails 1 test while 19 controls pass on that base. - Final public CLI: the legacy-only archive reports `├⌐.txt`; the combined archive retains `é.txt` and `├⌐.txt`. - Final actual stdio MCP: `open_binary` and `inspect_artifact(detail: "complete")` retain both names, verified member SHA-256 values and the unchanged archive SHA-256. Client/transport closure completes. - Final actual CLI extraction also produces both on-disk names and the exact `legacy`/`utf8` bytes, preserving the original archive digest and verified containment. - Existing malformed UTF-8, Unicode Path, CRC, overlap, encrypted-member, duplicate-member, path and resource controls pass. - Early probe mistakes (fixture carrier length and invalid MCP `detail: "full"`) are excluded from product failures and passing acceptance claims. ## Validation performed - Pinned Node 24.18.0/npm 11.16.0, `npm ci`. - Final integrated head `2e78035eae7d6f0cea327a56741b8b21a102605a`: ZIP reader/integrity/output and both artifact MCP suites plus EVM boundary suite: **56 passed, 29 skipped**, 6 files. The skips are 1 Linux /proc control and 28 Linux x64 EVM cases; those EVM cases were not executed on macOS. The five ZIP suites retain their original 54 passing tests. - `npm run check`: all 6 tasks and architecture/module guards pass. - `npm run verify:test-discovery`: all 1252 test files discovered exactly once. - Actual compiled CLI and stdio MCP checks described above; independent Standards and Spec review on the fixed complete patch. The original `4d383bf` delivery had two completed public CLI runs per version using the same archive and locked dependencies. Elapsed seconds: before 2.62/1.27, after 1.31/1.42; user/system CPU seconds: before 1.90/0.42 and 1.54/0.19, after 1.46/0.19 and 1.61/0.22. Sampled process-family peak RSS was 343936–345504 KiB before and 338240–350000 KiB after. These are small whole-workflow observations with uncontrolled caches and estimated family RSS, not an isolated performance comparison. Token usage was not measured. ## Compatibility, safety, and release - Ordinary correction of supported legacy names; existing caller parameters continue to work. - Executed on macOS arm64 with real ZIP bytes, Python `zipfile`, compiled CLI and stdio MCP. Windows and Linux execution remain unverified locally; the final commit's CI is reported separately. - Existing name/path, CRC/overlap, output and resource checks remain at their original boundaries. There are no new target execution or network effects. - Package/release metadata changes: none. ## CI follow-up The initial hosted CI `38522145048` failed two EVM boundary assertions in synthetic merge `a874486` (base `e579122` plus `4d383bf`). Complete per-job logs show one transferred cleanup owner being overwritten by undefined and one reply-error expectation disagreeing with the shared `unreadable_output` contract. Portable replays those failures; remaining shards were cancelled by fail-fast. This is not a ZIP assertion failure, and the original CI remains FAIL. Accepted main `29d3dcb` including upstream `07540cbc`, which retains ownership and bounded error output and aligns the diagnostic expectations; no parallel EVM implementation was added. Final ZIP contribution bytes and both reviewed files exactly match the independent review patch. Locked dependencies were reinstalled; final related tests, all six check tasks/guards, discovery and actual stdio inspection pass. Final Linux x64 EVM execution must be verified by the new hosted CI; macOS skips do not establish it. Required CI on the new pushed SHA is pending. ## Before merge - [x] Concrete issue and focused Conventional Commit title. - [x] Existing contribution scope and authority apply. - [x] Related issue linked; changed behavior and failure controls covered. - [x] Public CLI/MCP representative output and verification boundaries stated. - [x] Complete diff checked for unrelated changes and unsupported claims. - [ ] Required CI on the final pushed commit completed successfully. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-31 01:08:30 +00:00

closed this issue
added the
pull-request
label

bsakaguchi referenced this issue

2026-11-01 21:44:49 +00:00

[PR #5673] [MERGED] test(cli): keep IPC trace input under the Linux spawn limit #7028

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

No description provided.

Rows
Columns