morluto/rea

Fork 0

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

[PR #4047] [CLOSED] fix(plist): preserve real character data #6017

New issue

Closed

opened 2026-10-31 01:11:12 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 01:11:12 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/4047
Author: @JFSAS
Created: 10/15/2026
Status: ❌ Closed

Base: main ← Head: codex/plist-real-character-data

📝 Commits (5)

e31f5c5 fix(plist): preserve real character data
44da0cf fix(plist): avoid CDATA entity expansion
a33f082 chore(plist): reconcile upstream OpenStep handling
25acccc chore(plist): reconcile wide integer handling
43be57a fix(plist): avoid bigint metadata round trips

📊 Changes

14 files changed (+482 additions, -194 deletions)

View changed files

📝 docs/native-investigation.md (+3 -0)
📝 src/artifacts/apple/InterfaceBuilderAnalysis.ts (+3 -5)
📝 src/artifacts/apple/KeyedArchiveReader.ts (+2 -6)
📝 src/artifacts/apple/PlistNumberLiterals.ts (+5 -121)
📝 src/domain/apple/plistNumbers.ts (+36 -0)
📝 src/domain/propertyListKeys.test.ts (+121 -0)
📝 src/domain/propertyListKeys.ts (+122 -57)
📝 src/native/parsers/plist.test.ts (+2 -2)
📝 src/native/parsers/plist.ts (+4 -1)
📝 tests/acceptance/applications/interfaceBuilderPlistNumbers.test.ts (+12 -0)
➕ tests/acceptance/plistRealCharacterData.test.ts (+84 -0)
➕ tests/boundary/filesystem/plistRealDecodeExpansion.test.ts (+34 -0)
📝 tests/fixtures/keyedArchiveNumberMetadata.ts (+13 -2)
➕ tests/process-global/appBundleIntegerMetadata.test.ts (+41 -0)

📄 Description

Summary

Fixes #4042. Keyed-archive inspection now retains 1.25 from both <real>1<![CDATA[.25]]></real> and <real><![CDATA[1.25]]></real> through CLI and MCP. Previously these became 1 or an unknown non-finite real, while plain XML and native binary conversions were correct.

Change and scope

Reuse the existing XML preflight DOM to own real character data and numeric facts. Normalize each parsed real to a round-trippable exponential literal (at most 24 ASCII characters), removing its original character-data spans before the pinned value decoder. This eliminates CDATA entity expansion while retaining signed zero, literal-entity meaning and neighboring source line endings. XML consumers use those parsed number facts; remove the superseded raw XML number scanner and its entity decoder. The collector is shared with the upstream binary-number interpretation. It retains exact decimal text by normalizing its sign and leading zeroes, and uses Number only for the rounded-value association. This removes the unnecessary BigInt parse/format round trip from every consumer, including app Info.plist resolution.

Integer/real collisions remain ambiguous. Original archive bytes/digests, object identities, references, selection and limits remain intact. No adapter-specific value repair, dependency upgrade or new tool is introduced. The native JSON path continues to use its existing canonical plutil conversion.

Related #3426 and #3475 change this same XML boundary for keys and hexadecimal integers. This PR addresses real character data and does not incorporate their separate behavior changes.

The upstream OpenStep omission-count fix from #4105 is retained alongside the shared numberLiterals result. The merge also preserves #4324's 128-bit binary integer decoding, overlap validation and decode budgets. Both keyed-archive and Interface Builder consumers receive XML number facts from the shared DOM parser; the new upstream Interface Builder caller does not restore the superseded XML scanner. Its existing CLI regression now checks exact integer attributes alongside mixed-text and pure-CDATA reals.

Contract and boundary impact

Owner: parseXmlPropertyList; consumers receive numeric facts alongside decoded values.
Scope decision: not applicable; correction within existing inert plist inspection.
CLI/MCP schemas, provider authority, installation and package metadata: unchanged.
Evidence: corrected real values and truthful number-type limitations; original-byte identity retained.
No new target execution, mutation, network access or cleanup behavior.

Expected effect:

Read existing files or captured data.

Evidence and validation

Validated head 43be57a7d37305d6837195eff9ecb8dfd62d0a63 on main 4838f444939677107902549c59b73439e14d3ae9; original base 337d2e33c5aae40f20c64584c341a3593420b8e0; macOS arm64, Node 24.18.0 / npm 11.16.0.

The new compiled CLI/real stdio MCP regression fails on the base through both adapters, then passes after the fix.
Earlier native plutil controls independently returned 1.25, and the original four-case replay passed all eight CLI/MCP checks with unchanged source bytes and matching Evidence digests. Current-head CLI/stdio MCP acceptance tests again pass XML and binary controls.
25 focused parser/consumer/CLI/MCP files: 290 passed, 1 skipped. Coverage includes 16-byte binary integers and adjacent Int64 metadata, overlapping object-table rejection, decode budgets, integer/real ambiguity, encoding, depth, prototype keys, app metadata and Interface Builder CDATA attributes. This also includes the 5-million-digit app-resolver regression and real compiled app-bundle CLI/MCP encoding checks.
npm run check: passed typecheck, lint, formatting, knip, metadata, resolved module boundaries and architecture guards.
npm run docs:check: passed, including five consistency tests.
npm run verify:test-discovery: all 1,442 test files discovered exactly once.
git diff --check: passed.
At the previous merge head a33f08297, a public CLI/stdio MCP OpenStep smoke retained the neighboring value, reported one omitted __proto__ entry, and preserved source bytes and digest. Current-head parser tests continue to cover that behavior.

Numeric metadata resource regression

The app-opening review finding is valid and addressed in 43be57a7d. Adding numeric facts to the shared XML parser exposed unrelated Info.plist integers to an unnecessary arbitrary-precision parse/format round trip. Decimal metadata needs canonical source text and a rounded numeric key; it does not need a BigInt representation. The fix removes that representation in the shared collector, preserving all callers and avoiding a caller-specific bypass or arbitrary digit cutoff.

The regression exercises the real filesystem and resolveAppBundleExecutable with a 5,000,000-digit unrelated integer. A scoped conversion guard fails on the previous head and passes after the fix; it does not depend on a timing threshold.
An unguarded compiled-resolver sample on the same input fell from 1,307.37 ms to 30.40 ms. Before the fix, BigInt parsing took 359.87 ms and decimal formatting took 922.67 ms; neither conversion occurs afterward.
A seeded 1,009-case comparison (up to 4,096 significant digits) matched the prior BigInt-based metadata, including rounded keys and canonical decimal text. Committed cases cover signs, leading zeroes, safe-integer boundaries, negative zero and values beyond finite Number range.

Whole-workflow observations compare 25acccc40 with this fix using a 5,000,120-byte Info.plist. CLI runs analyze; stdio MCP performs initialize/open/shutdown. Both intentionally stop at the same unknown-provider diagnostic after resolving the app, so no disassembler starts. Output and original source bytes match in every run.

Workflow	Node cache / runs per version	Wall ms, before → after	CPU ms, before → after	Sampled peak family KiB, before → after
CLI	Cold / 1	17292.38 → 4307.02	5004.14 → 2315.30	323344 → 334368
CLI	Warm median / 3	11912.24 → 1994.19	5217.58 → 1992.63	316256 → 349792
stdio MCP	Cold / 1	4270.59 → 1990.55	4551.80 → 2288.68	567792 → 475888
stdio MCP	Warm median / 3	5287.77 → 2298.53	4451.68 → 2038.13	468368 → 402016

Versions alternate; separate Node compile caches are cold on first use and warm afterward. OS disk caches were not forced cold. CPU is wait4 user/system including waited children. Process-family RSS is sampled every 10 ms plus ps overhead and may miss brief peaks. CLI wall time varied substantially on the shared host, and RSS did not uniformly decrease. Output remains 2,489 bytes for CLI and 2,462 for MCP. Agent-token use was not measured. This removes the added arbitrary-precision work; file reading and DOM parsing still scale with input, and no new total Info.plist size or resident-memory cap is claimed.

Earlier CDATA resource and performance evidence

The resource and performance measurements below were executed at 44da0cf8f, before the upstream merges. The current merge preserves the XML real normalization implementation and retains upstream binary-width handling. These timings and near-limit capacity probes were not rerun at the current head.

The P1 review finding was reproduced on e31f5c59e: a 262,472-byte archive reached the second real XML parser as 1,311,036 bytes. The new production-boundary regression invokes both real parsers and fails on that expansion. After this change, the second parser receives 319 bytes for the same archive. The entity-escaping path has been removed.

A 66,060,616-byte (63 MiB payload) self-authored archive also completed through the public CLI and real stdio MCP, each with a 512 MiB JavaScript heap setting and 30-second deadline. Both returned the expected value and original digest, with source bytes unchanged. The corresponding owner probe's second parse was still 319 bytes. The old near-limit case was not executed. A seeded 2,007-value round-trip check matched the previous plain-text decoder, including negative zero, subnormals, extreme finite values and non-finite values.

The following whole-workflow measurements compare e31f5c59e with this fix on a 1,048,904-byte archive containing entity-like CDATA. This is well-formed XML admitted by the pinned decoder; it does not establish that Apple accepts the numeric suffix. Each run is a complete fresh CLI invocation or stdio MCP connection plus open/inspect/close. Separate Node compile caches were cold for the first run and warm for four subsequent alternating runs; OS disk caches were not forced cold.

Workflow	Cache / runs per version	Wall ms, before → after	CPU ms, before → after	Sampled peak family KiB, before → after
CLI	Cold / 1	1156.99 → 691.53	1188.21 → 947.27	490224 → 346640
CLI	Warm median / 4	690.06 → 472.76	963.19 → 709.11	484784 → 361072
stdio MCP	Cold / 1	951.31 → 698.23	1242.45 → 932.74	604000 → 483216
stdio MCP	Warm median / 4	739.95 → 539.82	1053.89 → 796.68	599568 → 475624

CPU uses wait4 user/system accounting for the process and waited children. Family RSS is sampled every 10 ms plus ps overhead, so brief peaks may be missed. Normalized results match exactly; complete stdout remains 2,352 bytes for CLI and 2,351 for MCP. No real-agent run or agent-token measurement was performed. These samples do not establish a universal speedup or total resident-memory cap. The initial DOM/input cost remains subject to the existing acquisition and depth boundaries; no entity-expansion cleanup remains.

Compatibility and limitations

No migration or public schema change. Binary and OpenStep decoding retain their existing routes. Malformed nested numeric content remains explicitly incomplete. Tests use self-authored artifacts; frequency in real applications, Linux/Windows behavior and the full CI suite have not been established locally. No real disassembler coverage is claimed. Prepared with AI assistance.

Author readiness

Human owner: @JFSAS

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.

_{🔄 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/4047 **Author:** [@JFSAS](https://github.com/JFSAS) **Created:** 10/15/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `codex/plist-real-character-data` --- ### 📝 Commits (5) - [`e31f5c5`](https://github.com/morluto/rea/commit/e31f5c59e7ecaa10ecd7c4f1c3fecaf49df9dbe2) fix(plist): preserve real character data - [`44da0cf`](https://github.com/morluto/rea/commit/44da0cf8fe8ccf959963201c338b211fa576ecdc) fix(plist): avoid CDATA entity expansion - [`a33f082`](https://github.com/morluto/rea/commit/a33f082970e9592463dd98ef754b3418e31b53b6) chore(plist): reconcile upstream OpenStep handling - [`25acccc`](https://github.com/morluto/rea/commit/25acccc4014086ee646d884787b8b422fb9a0a95) chore(plist): reconcile wide integer handling - [`43be57a`](https://github.com/morluto/rea/commit/43be57a7d37305d6837195eff9ecb8dfd62d0a63) fix(plist): avoid bigint metadata round trips ### 📊 Changes **14 files changed** (+482 additions, -194 deletions) <details> <summary>View changed files</summary> 📝 `docs/native-investigation.md` (+3 -0) 📝 `src/artifacts/apple/InterfaceBuilderAnalysis.ts` (+3 -5) 📝 `src/artifacts/apple/KeyedArchiveReader.ts` (+2 -6) 📝 `src/artifacts/apple/PlistNumberLiterals.ts` (+5 -121) 📝 `src/domain/apple/plistNumbers.ts` (+36 -0) 📝 `src/domain/propertyListKeys.test.ts` (+121 -0) 📝 `src/domain/propertyListKeys.ts` (+122 -57) 📝 `src/native/parsers/plist.test.ts` (+2 -2) 📝 `src/native/parsers/plist.ts` (+4 -1) 📝 `tests/acceptance/applications/interfaceBuilderPlistNumbers.test.ts` (+12 -0) ➕ `tests/acceptance/plistRealCharacterData.test.ts` (+84 -0) ➕ `tests/boundary/filesystem/plistRealDecodeExpansion.test.ts` (+34 -0) 📝 `tests/fixtures/keyedArchiveNumberMetadata.ts` (+13 -2) ➕ `tests/process-global/appBundleIntegerMetadata.test.ts` (+41 -0) </details> ### 📄 Description ## Summary Fixes #4042. Keyed-archive inspection now retains `1.25` from both `<real>1<![CDATA[.25]]></real>` and `<real><![CDATA[1.25]]></real>` through CLI and MCP. Previously these became `1` or an unknown non-finite real, while plain XML and native binary conversions were correct. ## Change and scope Reuse the existing XML preflight DOM to own real character data and numeric facts. Normalize each parsed real to a round-trippable exponential literal (at most 24 ASCII characters), removing its original character-data spans before the pinned value decoder. This eliminates CDATA entity expansion while retaining signed zero, literal-entity meaning and neighboring source line endings. XML consumers use those parsed number facts; remove the superseded raw XML number scanner and its entity decoder. The collector is shared with the upstream binary-number interpretation. It retains exact decimal text by normalizing its sign and leading zeroes, and uses `Number` only for the rounded-value association. This removes the unnecessary `BigInt` parse/format round trip from every consumer, including app Info.plist resolution. Integer/real collisions remain ambiguous. Original archive bytes/digests, object identities, references, selection and limits remain intact. No adapter-specific value repair, dependency upgrade or new tool is introduced. The native JSON path continues to use its existing canonical plutil conversion. Related #3426 and #3475 change this same XML boundary for keys and hexadecimal integers. This PR addresses real character data and does not incorporate their separate behavior changes. The upstream OpenStep omission-count fix from #4105 is retained alongside the shared `numberLiterals` result. The merge also preserves #4324's 128-bit binary integer decoding, overlap validation and decode budgets. Both keyed-archive and Interface Builder consumers receive XML number facts from the shared DOM parser; the new upstream Interface Builder caller does not restore the superseded XML scanner. Its existing CLI regression now checks exact integer attributes alongside mixed-text and pure-CDATA reals. ## Contract and boundary impact - Owner: `parseXmlPropertyList`; consumers receive numeric facts alongside decoded values. - Scope decision: not applicable; correction within existing inert plist inspection. - CLI/MCP schemas, provider authority, installation and package metadata: unchanged. - Evidence: corrected real values and truthful number-type limitations; original-byte identity retained. - No new target execution, mutation, network access or cleanup behavior. Expected effect: - [x] Read existing files or captured data. ## Evidence and validation Validated head `43be57a7d37305d6837195eff9ecb8dfd62d0a63` on main `4838f444939677107902549c59b73439e14d3ae9`; original base `337d2e33c5aae40f20c64584c341a3593420b8e0`; macOS arm64, Node 24.18.0 / npm 11.16.0. - The new compiled CLI/real stdio MCP regression fails on the base through both adapters, then passes after the fix. - Earlier native `plutil` controls independently returned `1.25`, and the original four-case replay passed all eight CLI/MCP checks with unchanged source bytes and matching Evidence digests. Current-head CLI/stdio MCP acceptance tests again pass XML and binary controls. - 25 focused parser/consumer/CLI/MCP files: **290 passed, 1 skipped**. Coverage includes 16-byte binary integers and adjacent Int64 metadata, overlapping object-table rejection, decode budgets, integer/real ambiguity, encoding, depth, prototype keys, app metadata and Interface Builder CDATA attributes. This also includes the 5-million-digit app-resolver regression and real compiled app-bundle CLI/MCP encoding checks. - `npm run check`: passed typecheck, lint, formatting, knip, metadata, resolved module boundaries and architecture guards. - `npm run docs:check`: passed, including five consistency tests. - `npm run verify:test-discovery`: all 1,442 test files discovered exactly once. - `git diff --check`: passed. - At the previous merge head `a33f08297`, a public CLI/stdio MCP OpenStep smoke retained the neighboring value, reported one omitted `__proto__` entry, and preserved source bytes and digest. Current-head parser tests continue to cover that behavior. ### Numeric metadata resource regression The [app-opening review finding](https://github.com/morluto/rea/pull/4047#discussion_r4298248176) is valid and addressed in `43be57a7d`. Adding numeric facts to the shared XML parser exposed unrelated Info.plist integers to an unnecessary arbitrary-precision parse/format round trip. Decimal metadata needs canonical source text and a rounded numeric key; it does not need a BigInt representation. The fix removes that representation in the shared collector, preserving all callers and avoiding a caller-specific bypass or arbitrary digit cutoff. - The regression exercises the real filesystem and `resolveAppBundleExecutable` with a 5,000,000-digit unrelated integer. A scoped conversion guard fails on the previous head and passes after the fix; it does not depend on a timing threshold. - An unguarded compiled-resolver sample on the same input fell from **1,307.37 ms to 30.40 ms**. Before the fix, BigInt parsing took 359.87 ms and decimal formatting took 922.67 ms; neither conversion occurs afterward. - A seeded **1,009-case** comparison (up to 4,096 significant digits) matched the prior BigInt-based metadata, including rounded keys and canonical decimal text. Committed cases cover signs, leading zeroes, safe-integer boundaries, negative zero and values beyond finite Number range. Whole-workflow observations compare `25acccc40` with this fix using a 5,000,120-byte Info.plist. CLI runs `analyze`; stdio MCP performs initialize/open/shutdown. Both intentionally stop at the same unknown-provider diagnostic after resolving the app, so no disassembler starts. Output and original source bytes match in every run. | Workflow | Node cache / runs per version | Wall ms, before → after | CPU ms, before → after | Sampled peak family KiB, before → after | | --- | --- | ---: | ---: | ---: | | CLI | Cold / 1 | 17292.38 → 4307.02 | 5004.14 → 2315.30 | 323344 → 334368 | | CLI | Warm median / 3 | 11912.24 → 1994.19 | 5217.58 → 1992.63 | 316256 → 349792 | | stdio MCP | Cold / 1 | 4270.59 → 1990.55 | 4551.80 → 2288.68 | 567792 → 475888 | | stdio MCP | Warm median / 3 | 5287.77 → 2298.53 | 4451.68 → 2038.13 | 468368 → 402016 | Versions alternate; separate Node compile caches are cold on first use and warm afterward. OS disk caches were not forced cold. CPU is `wait4` user/system including waited children. Process-family RSS is sampled every 10 ms plus `ps` overhead and may miss brief peaks. CLI wall time varied substantially on the shared host, and RSS did not uniformly decrease. Output remains 2,489 bytes for CLI and 2,462 for MCP. Agent-token use was not measured. This removes the added arbitrary-precision work; file reading and DOM parsing still scale with input, and no new total Info.plist size or resident-memory cap is claimed. <details> <summary>Earlier CDATA resource and performance evidence</summary> The resource and performance measurements below were executed at `44da0cf8f`, before the upstream merges. The current merge preserves the XML real normalization implementation and retains upstream binary-width handling. These timings and near-limit capacity probes were not rerun at the current head. The [P1 review finding](https://github.com/morluto/rea/pull/4047#discussion_r4281912739) was reproduced on `e31f5c59e`: a 262,472-byte archive reached the second real XML parser as **1,311,036 bytes**. The new production-boundary regression invokes both real parsers and fails on that expansion. After this change, the second parser receives **319 bytes** for the same archive. The entity-escaping path has been removed. A **66,060,616-byte** (63 MiB payload) self-authored archive also completed through the public CLI and real stdio MCP, each with a 512 MiB JavaScript heap setting and 30-second deadline. Both returned the expected value and original digest, with source bytes unchanged. The corresponding owner probe's second parse was still 319 bytes. The old near-limit case was not executed. A seeded **2,007-value** round-trip check matched the previous plain-text decoder, including negative zero, subnormals, extreme finite values and non-finite values. The following whole-workflow measurements compare `e31f5c59e` with this fix on a 1,048,904-byte archive containing entity-like CDATA. This is well-formed XML admitted by the pinned decoder; it does not establish that Apple accepts the numeric suffix. Each run is a complete fresh CLI invocation or stdio MCP connection plus open/inspect/close. Separate Node compile caches were cold for the first run and warm for four subsequent alternating runs; OS disk caches were not forced cold. | Workflow | Cache / runs per version | Wall ms, before → after | CPU ms, before → after | Sampled peak family KiB, before → after | | --- | --- | ---: | ---: | ---: | | CLI | Cold / 1 | 1156.99 → 691.53 | 1188.21 → 947.27 | 490224 → 346640 | | CLI | Warm median / 4 | 690.06 → 472.76 | 963.19 → 709.11 | 484784 → 361072 | | stdio MCP | Cold / 1 | 951.31 → 698.23 | 1242.45 → 932.74 | 604000 → 483216 | | stdio MCP | Warm median / 4 | 739.95 → 539.82 | 1053.89 → 796.68 | 599568 → 475624 | CPU uses `wait4` user/system accounting for the process and waited children. Family RSS is sampled every 10 ms plus `ps` overhead, so brief peaks may be missed. Normalized results match exactly; complete stdout remains 2,352 bytes for CLI and 2,351 for MCP. **No real-agent run or agent-token measurement was performed.** These samples do not establish a universal speedup or total resident-memory cap. The initial DOM/input cost remains subject to the existing acquisition and depth boundaries; no entity-expansion cleanup remains. </details> ## Compatibility and limitations No migration or public schema change. Binary and OpenStep decoding retain their existing routes. Malformed nested numeric content remains explicitly incomplete. Tests use self-authored artifacts; frequency in real applications, Linux/Windows behavior and the full CI suite have not been established locally. No real disassembler coverage is claimed. Prepared with AI assistance. ## Author readiness Human owner: @JFSAS - [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. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-31 01:11:12 +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#6017

No description provided.

Rows
Columns