[PR #4851] [CLOSED] fix(native): keep plutil's decode diagnostic when inspect_plist refuses a file #6442

Closed
opened 2026-10-31 01:14:30 +00:00 by bsakaguchi · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/4851
Author: @apitoagents
Created: 10/17/2026
Status: ❌ Closed

Base: main ← Head: fix/plutil-diagnostic-stdout


📝 Commits (1)

  • 8477374 fix(native): keep plutil's decode diagnostic when inspect_plist refuses a file

📊 Changes

2 files changed (+19 additions, -5 deletions)

View changed files

📝 src/native/NativeMacOSProvider.ts (+14 -4)
📝 tests/boundary/cli/nativePlistSelectionFailureCli.test.ts (+5 -1)

📄 Description

Summary

Fixes #4849.

On macOS, inspect_plist reported an undecodable plist as plutil could not decode it as a property list (). The parenthesis was always empty because the helper read plutil's stderr, while plutil writes its decode diagnostic to the stream selected by -o, which is stdout for -o -. The error now carries plutil's text, for example (Property List error: Unknown character ' ' (0x20) in <integer> on line 3 / JSON error: …).

Problem and expected behavior

0674facd4 intended to "keep plutil's decode diagnostic", but plutilDiagnostic(xml.value.stderr, path) only looked at stderr. Spawning the exact command from Node (plutil -convert xml1 -o - -- <file>, LC_ALL=C LANG=C) returns status 1, empty stderr and the full diagnostic on stdout; the same holds for -convert json -o -, -o <file> and -lint on macOS 13.2.1. Expected: the user sees which element and line plutil refused.

Change and scope

  • src/native/NativeMacOSProvider.ts: plutilDiagnostic takes the capture and uses the first non-empty of stderr then stdout, stripping the path prefix as before. If both streams are empty it says plutil printed no diagnostic instead of leaving an empty parenthesis. The comment records plutil's stream behaviour.
  • tests/boundary/cli/nativePlistSelectionFailureCli.test.ts: the existing Contents/Text.plist case now asserts the diagnostic text (Property List error: Unexpected character n at line 1, which fails on main and passes with this change.

Intentionally not included: the error class for a malformed plist that is itself the selected target (inspect-plist /path/bad.plist without --relative-path) is still capability_unavailable / "no readable default plist". That is a contract question noted in #4849 for the maintainers.

Scope decision: not applicable (routine fix within existing read-only boundaries).

Contract and boundary impact

  • Semantic owner and earliest changed stage: native-macos provider, inspect_plist result projection.
  • Contribution scope fit and linked maintainer decision: routine fix; not applicable.
  • CLI and MCP/tool-catalog contract: unchanged codes, remediation and shapes. Only the explanatory text inside the parenthesis changes from empty to plutil's diagnostic.
  • Provider, bridge, target-format, or platform compatibility: macOS only; Linux and Windows do not use plutil.
  • Evidence, artifact, provenance, or reconstruction contract: unchanged.
  • Target selection, authority, upstream behavior, and cleanup details: not applicable.
  • Generated metadata, package, or installation impact: none.

Expected effects:

  • 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/cli/nativePlistSelectionFailureCli.test.ts (Contents/Text.plist case asserts the plutil diagnostic). Executed on main with the test change only: 1 failed, 4 passed. With the source change: 5 passed.

  • Base reproduction (executed): node scripts/rea.mjs inspect-plist <bad.plist> --json on main 742b6f587 ends with plutil could not decode it as a property list ().; after the change the same command prints (Property List error: Unknown character ' ' (0x20) in <integer> on line 3 / JSON error: JSON text did not start with array or object and option to allow fragments not set. around line 1, column 0.).

  • User-visible CLI/MCP output: see above; exit code 1 before and after.

  • Remaining proof gaps: plutil stream behaviour verified on macOS 13.2.1 only. Newer macOS releases were not checked; reading both streams covers either placement.

  • 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

  • npm run test:focused -- tests/boundary/cli/nativePlistSelectionFailureCli.test.ts — 5 passed (macOS arm64, Node 24.15.0)
  • Same test with the source change stashed — 1 failed (the new assertion), 4 passed
  • npm run check:fast — typecheck and lint successful
  • npm run build then the CLI reproduction above — diagnostic present

Compatibility, safety, and release

  • Breaking changes or migration steps: none.
  • Real Hopper/Ghidra, browser, or OS coverage: real /usr/bin/plutil on macOS 13.2.1; not applicable to engines.
  • Package or release metadata impact: none.
  • Security, privacy, process, or containment review: the diagnostic names the selected path and plutil's parse position; no new inputs, processes or network access.

Author readiness

Human owner: @apitoagents

  • 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/4851 **Author:** [@apitoagents](https://github.com/apitoagents) **Created:** 10/17/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/plutil-diagnostic-stdout` --- ### 📝 Commits (1) - [`8477374`](https://github.com/morluto/rea/commit/84773748001c8d944d262baa8a61fd820bcdbbe6) fix(native): keep plutil's decode diagnostic when inspect_plist refuses a file ### 📊 Changes **2 files changed** (+19 additions, -5 deletions) <details> <summary>View changed files</summary> 📝 `src/native/NativeMacOSProvider.ts` (+14 -4) 📝 `tests/boundary/cli/nativePlistSelectionFailureCli.test.ts` (+5 -1) </details> ### 📄 Description ## Summary Fixes #4849. On macOS, `inspect_plist` reported an undecodable plist as `plutil could not decode it as a property list ()`. The parenthesis was always empty because the helper read plutil's stderr, while plutil writes its decode diagnostic to the stream selected by `-o`, which is stdout for `-o -`. The error now carries plutil's text, for example `(Property List error: Unknown character ' ' (0x20) in <integer> on line 3 / JSON error: …)`. ## Problem and expected behavior `0674facd4` intended to "keep plutil's decode diagnostic", but `plutilDiagnostic(xml.value.stderr, path)` only looked at stderr. Spawning the exact command from Node (`plutil -convert xml1 -o - -- <file>`, `LC_ALL=C LANG=C`) returns `status 1`, empty stderr and the full diagnostic on stdout; the same holds for `-convert json -o -`, `-o <file>` and `-lint` on macOS 13.2.1. Expected: the user sees which element and line plutil refused. ## Change and scope - `src/native/NativeMacOSProvider.ts`: `plutilDiagnostic` takes the capture and uses the first non-empty of stderr then stdout, stripping the path prefix as before. If both streams are empty it says `plutil printed no diagnostic` instead of leaving an empty parenthesis. The comment records plutil's stream behaviour. - `tests/boundary/cli/nativePlistSelectionFailureCli.test.ts`: the existing `Contents/Text.plist` case now asserts the diagnostic text `(Property List error: Unexpected character n at line 1`, which fails on main and passes with this change. Intentionally not included: the error class for a malformed plist that is itself the selected target (`inspect-plist /path/bad.plist` without `--relative-path`) is still `capability_unavailable` / "no readable default plist". That is a contract question noted in #4849 for the maintainers. Scope decision: not applicable (routine fix within existing read-only boundaries). ## Contract and boundary impact - Semantic owner and earliest changed stage: `native-macos` provider, `inspect_plist` result projection. - Contribution scope fit and linked maintainer decision: routine fix; not applicable. - CLI and MCP/tool-catalog contract: unchanged codes, remediation and shapes. Only the explanatory text inside the parenthesis changes from empty to plutil's diagnostic. - Provider, bridge, target-format, or platform compatibility: macOS only; Linux and Windows do not use plutil. - Evidence, artifact, provenance, or reconstruction contract: unchanged. - Target selection, authority, upstream behavior, and cleanup details: not applicable. - Generated metadata, package, or installation impact: none. Expected effects: - [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/cli/nativePlistSelectionFailureCli.test.ts` (`Contents/Text.plist` case asserts the plutil diagnostic). Executed on main with the test change only: 1 failed, 4 passed. With the source change: 5 passed. - Base reproduction (executed): `node scripts/rea.mjs inspect-plist <bad.plist> --json` on main `742b6f587` ends with `plutil could not decode it as a property list ().`; after the change the same command prints `(Property List error: Unknown character ' ' (0x20) in <integer> on line 3 / JSON error: JSON text did not start with array or object and option to allow fragments not set. around line 1, column 0.)`. - User-visible CLI/MCP output: see above; exit code 1 before and after. - Remaining proof gaps: plutil stream behaviour verified on macOS 13.2.1 only. Newer macOS releases were not checked; reading both streams covers either placement. - [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 - `npm run test:focused -- tests/boundary/cli/nativePlistSelectionFailureCli.test.ts` — 5 passed (macOS arm64, Node 24.15.0) - Same test with the source change stashed — 1 failed (the new assertion), 4 passed - `npm run check:fast` — typecheck and lint successful - `npm run build` then the CLI reproduction above — diagnostic present ## Compatibility, safety, and release - Breaking changes or migration steps: none. - Real Hopper/Ghidra, browser, or OS coverage: real `/usr/bin/plutil` on macOS 13.2.1; not applicable to engines. - Package or release metadata impact: none. - Security, privacy, process, or containment review: the diagnostic names the selected path and plutil's parse position; no new inputs, processes or network access. ## Author readiness Human owner: @apitoagents - [ ] 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 - [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. - [ ] 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-10-31 01:14:30 +00:00
Sign in to join this conversation.
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#6442
No description provided.