[PR #6984] [MERGED] fix(cli): exclude inherited values from Markdown rows #7546

Closed
opened 2026-11-03 13:38:36 +00:00 by bsakaguchi · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/6984
Author: @ShuaixinHuang
Created: 10/24/2026
Status: ✅ Merged
Merged: 10/24/2026
Merged by: @morluto

Base: main ← Head: codex/fix-cli-table-owned-fields


📝 Commits (7)

  • 449905b fix(cli): exclude inherited values from Markdown rows
  • 9cf6f3b Merge commit 'dd02d8ff69' into codex/fix-cli-table-owned-fields
  • cdd41de Merge commit '9895f1870c' into codex/fix-cli-table-owned-fields
  • d042e4a Merge commit 'be2811e414' into codex/fix-cli-table-owned-fields
  • 4b251c2 test(verifiers): follow shared command output and identity
  • a4efa3c Merge shared verifier CI test repair
  • cba2f9f test(ci): synchronize upstream verifier assertion repair

📊 Changes

2 files changed (+44 additions, -3 deletions)

View changed files

📝 src/cli/cliOutputFormatter.ts (+5 -1)
📝 tests/boundary/cli/filteredArrayOutput.test.ts (+39 -2)

📄 Description

Summary

Keep absent fields blank in heterogeneous Markdown object tables instead of printing inherited JavaScript functions or prototype objects.

Problem and expected behavior

For [ {"constructor":"ctor","toString":"text","__proto__":"proto"}, {} ], the column union correctly includes the first record's own keys, but reading the second record with item[key] invents values such as function Object() { [native code] } and {}. A missing own field must remain absent; explicitly supplied values, including 0, false, and null, must remain visible.

Change and scope

Read table cells through an own-property check in the existing formatter, matching the Object.keys rule that creates the columns. Add a regression through registered CLI command dispatch and the production result writer. No separate issue: this PR contains the reproducer and fix.

Follow the contribution scope. This is an ordinary output correctness fix; a separate maintainer scope decision is not applicable.

Contract and boundary impact

  • Semantic owner and earliest changed stage: cliOutputFormatter.formatObjectTable, when projecting a record into Markdown cells.
  • Contribution scope fit and linked maintainer decision: existing CLI formatting; not applicable.
  • CLI and MCP/tool-catalog contract: CLI Markdown missing cells become blank; JSON values and tool schemas are unchanged.
  • Provider, bridge, target-format, or platform compatibility: JavaScript own-property semantics; no provider protocol change.
  • Evidence, artifact, provenance, or reconstruction contract: avoid reporting inherited values as supplied analysis facts; preserve source records.
  • Target selection, authority, upstream behavior, and cleanup details (if applicable): not applicable; formatter only.
  • Generated metadata (docs/public/product-catalog.json), package, or installation impact (CI generates and validates ignored catalogs and skills/; edit authored instructions in .agents/skills/reverse-engineer-anything/ and never commit build outputs): none; no schema/package change.

Expected effects (check known effects; use documentation-only on its own).
Unchecked items do not establish absence; leave uncertainties visible.

  • 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: existing filteredArrayOutput.test.ts now covers heterogeneous records through real command registration, Markdown output, and exact JSON preservation; the input is recursively frozen.
  • Base reproduction or other evidence: the new test failed on c350343c because the absent row contained inherited Object/toString functions and {}; it passes with the own-property check.
  • User-visible CLI/MCP output (if applicable): a record with only name: "second" now yields | second | | | | | |, while explicitly supplied 0, false, and null retain those spellings.
  • Remaining proof gaps: local execution used Windows and Node 24.22.1; Linux/macOS execution and the complete suite remain CI coverage. No external analysis engine was required or exercised.

For evidence-bearing changes:

  • 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

  • New selected regression before the fix — 1 failed for inherited values, 27 deselected.
  • npm run test:focused -- tests/boundary/cli/cliOutput.test.ts tests/boundary/cli/evidenceBundleCommands.test.ts tests/boundary/cli/filteredArrayOutput.test.ts tests/boundary/cli/markdownNestedFacts.test.ts — 4 files, 103 tests passed.
  • npm run check — 6 tasks passed; resolved module boundaries and architecture guards passed. Lint reported warnings and zero errors.
  • git diff --check — passed; normal pre-commit formatting/lint passed.

Compatibility, safety, and release

  • Breaking changes or migration steps: none; only absent cells stop displaying inherited values.
  • Real Hopper/Ghidra, browser, or OS coverage: Windows CLI boundary tests; no Hopper/Ghidra/browser execution.
  • Package or release metadata impact: none.
  • Security, privacy, process, or containment review: no added execution, network request, file mutation, or target access; this is an in-memory output projection.

Author readiness

Human owner: @ShuaixinHuang

  • 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.

The human owner instructed the assistant in this session to prepare all three changes and submit them directly for review; this PR is AI-assisted.

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.

Synchronization validation (2026-10-24)

Synchronized reviewed upstream 347b6e4d95 (#7002), which includes the earlier #6986, #6988 and #6994 repairs. Current head: cba2f9fdab. The complete feature patch is byte-for-byte identical to this PR's pre-repair feature patch and remains 2 files against that upstream revision; no verifier-test changes remain in the PR diff.

The preceding head's shard 6 had 13 failed / 1889 passed / 20 skipped from two shared test-contract mismatches. Those are historical results. A user-authorized local support patch 4b251c25 migrated the error diagnostics and run ID checks, then upstream merged #7002 concurrently. The resulting conflict was resolved by taking both upstream test files exactly. The Python cases now check VerifierCommandExitError, exit code 1 and snapshot.stderr.text. The ownership cases compare leader/descendant identities to the actual launcher-assigned nonce and distinguish the inherited identity. Archive rejection, ambient-import isolation, no extraction, process-exit-before-root-cleanup and cleanup-retry checks remain.

Clean-base comparison before the upstream merge (Windows, pinned Node 24.22.1/npm 11.16.0, Python 3.12.14, native Windows addon present): the exact focused two-file command for packedBridgeVerification.test.ts and packagedWasmVerifierOwnership.test.ts failed 10 / passed 14 / skipped 3 on be2811e414, then passed 24 / skipped 3 after the local migration. A controlled temporary bypass of ast.parse made the existing invalid-Python regression fail as intended (1 failed / 23 deselected); production bytes were restored. These are local repair evidence, not the final upstream-combination result.

On the final integrated ELF branch, npm run test:focused -- tests/boundary/process/packedBridgeVerification.test.ts tests/boundary/process/packagedWasmVerifierOwnership.test.ts tests/boundary/filesystem/verifierCommandOutput.test.ts tests/boundary/process/nativeVerifierPreflightOwnership.test.ts passed 29 / skipped 14 (four files). All 24 packed-Python cases and five command-output cases passed. Existing Windows skips cover three packaged-Wasm lifecycle cases, eight native-client preflight cases and three filesystem failure cases. No new skip was introduced; Linux CI must execute the POSIX coverage. This run does not establish real external-client/provider behavior.

Each final integrated branch passed npm run check (six tasks, module boundaries and architecture guards) and git diff --check; normal commit and push hooks passed. No CI gates, timeouts or skips were weakened. Upstream #7002's own green CI does not establish this PR's new-head result. Latest-head confirmation on 2026-10-24: CI required, Conventional PR title and PR readiness all succeeded for cba2f9fdab. Source tests and all eight test shards succeeded. Optional lanes retain their reported skips; the historical local Windows limits remain disclosed.

Earlier post-sync feature/shared validation passed 222 tests across six CLI and shared-regression files. After the earlier be2811 merge, the two import/type-erasure source files passed all 36 tests on this branch.

Earlier checks also passed all 113 reference-lifetime cases, six property-path regressions, and nine evaluation-gate tests. The former cyclic-path single case passed with the same 512 MiB heap / external 20 s guard (137 ms test time, 3.76 s elapsed); the baseline exhausted the heap. No completed-baseline speedup or process-family RSS claim is made.


🔄 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/6984 **Author:** [@ShuaixinHuang](https://github.com/ShuaixinHuang) **Created:** 10/24/2026 **Status:** ✅ Merged **Merged:** 10/24/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `codex/fix-cli-table-owned-fields` --- ### 📝 Commits (7) - [`449905b`](https://github.com/morluto/rea/commit/449905b9d4858fd3e79a7a1b6013e0fbffce3a5b) fix(cli): exclude inherited values from Markdown rows - [`9cf6f3b`](https://github.com/morluto/rea/commit/9cf6f3be55d933155bdedceb67b550f990212789) Merge commit 'dd02d8ff696385949b05459f2ab9b6ffda18a52c' into codex/fix-cli-table-owned-fields - [`cdd41de`](https://github.com/morluto/rea/commit/cdd41dea991c41f34322f8b662469723451c0556) Merge commit '9895f1870c5537e64f08e9ff145a0917696b7a50' into codex/fix-cli-table-owned-fields - [`d042e4a`](https://github.com/morluto/rea/commit/d042e4a39624da55eb165badbbc2b6b09226daf0) Merge commit 'be2811e41407f0149b3e8e12cb2bf0ffd5904b2d' into codex/fix-cli-table-owned-fields - [`4b251c2`](https://github.com/morluto/rea/commit/4b251c25d75e1bc400e730ee54d7f4acd2d2504b) test(verifiers): follow shared command output and identity - [`a4efa3c`](https://github.com/morluto/rea/commit/a4efa3ca2ec74a96b10c7f12e1c6455afab4e125) Merge shared verifier CI test repair - [`cba2f9f`](https://github.com/morluto/rea/commit/cba2f9fdabde7786ec1d6c26216b44f7ccddfbf5) test(ci): synchronize upstream verifier assertion repair ### 📊 Changes **2 files changed** (+44 additions, -3 deletions) <details> <summary>View changed files</summary> 📝 `src/cli/cliOutputFormatter.ts` (+5 -1) 📝 `tests/boundary/cli/filteredArrayOutput.test.ts` (+39 -2) </details> ### 📄 Description ## Summary Keep absent fields blank in heterogeneous Markdown object tables instead of printing inherited JavaScript functions or prototype objects. ## Problem and expected behavior For `[ {"constructor":"ctor","toString":"text","__proto__":"proto"}, {} ]`, the column union correctly includes the first record's own keys, but reading the second record with `item[key]` invents values such as `function Object() { [native code] }` and `{}`. A missing own field must remain absent; explicitly supplied values, including `0`, `false`, and `null`, must remain visible. ## Change and scope Read table cells through an own-property check in the existing formatter, matching the `Object.keys` rule that creates the columns. Add a regression through registered CLI command dispatch and the production result writer. No separate issue: this PR contains the reproducer and fix. Follow the [contribution scope](https://github.com/morluto/rea/blob/main/CONTRIBUTING.md#contribution-scope). This is an ordinary output correctness fix; a separate maintainer scope decision is not applicable. ## Contract and boundary impact - Semantic owner and earliest changed stage: `cliOutputFormatter.formatObjectTable`, when projecting a record into Markdown cells. - Contribution scope fit and linked maintainer decision: existing CLI formatting; not applicable. - CLI and MCP/tool-catalog contract: CLI Markdown missing cells become blank; JSON values and tool schemas are unchanged. - Provider, bridge, target-format, or platform compatibility: JavaScript own-property semantics; no provider protocol change. - Evidence, artifact, provenance, or reconstruction contract: avoid reporting inherited values as supplied analysis facts; preserve source records. - Target selection, authority, upstream behavior, and cleanup details (if applicable): not applicable; formatter only. - Generated metadata (`docs/public/product-catalog.json`), package, or installation impact (CI generates and validates ignored catalogs and `skills/`; edit authored instructions in `.agents/skills/reverse-engineer-anything/` and never commit build outputs): none; no schema/package change. Expected effects (check known effects; use documentation-only on its own). Unchecked items do not establish absence; leave uncertainties visible. - [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: existing `filteredArrayOutput.test.ts` now covers heterogeneous records through real command registration, Markdown output, and exact JSON preservation; the input is recursively frozen. - Base reproduction or other evidence: the new test failed on `c350343c` because the absent row contained inherited `Object`/`toString` functions and `{}`; it passes with the own-property check. - User-visible CLI/MCP output (if applicable): a record with only `name: "second"` now yields `| second | | | | | |`, while explicitly supplied `0`, `false`, and `null` retain those spellings. - Remaining proof gaps: local execution used Windows and Node 24.22.1; Linux/macOS execution and the complete suite remain CI coverage. No external analysis engine was required or exercised. For evidence-bearing changes: - [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 - New selected regression before the fix — 1 failed for inherited values, 27 deselected. - `npm run test:focused -- tests/boundary/cli/cliOutput.test.ts tests/boundary/cli/evidenceBundleCommands.test.ts tests/boundary/cli/filteredArrayOutput.test.ts tests/boundary/cli/markdownNestedFacts.test.ts` — 4 files, 103 tests passed. - `npm run check` — 6 tasks passed; resolved module boundaries and architecture guards passed. Lint reported warnings and zero errors. - `git diff --check` — passed; normal pre-commit formatting/lint passed. ## Compatibility, safety, and release - Breaking changes or migration steps: none; only absent cells stop displaying inherited values. - Real Hopper/Ghidra, browser, or OS coverage: Windows CLI boundary tests; no Hopper/Ghidra/browser execution. - Package or release metadata impact: none. - Security, privacy, process, or containment review: no added execution, network request, file mutation, or target access; this is an in-memory output projection. ## Author readiness Human owner: @ShuaixinHuang - [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. The human owner instructed the assistant in this session to prepare all three changes and submit them directly for review; this PR is AI-assisted. ## 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. - [x] I checked the final diff for secrets, unrelated cleanup, and unsupported claims. ## Synchronization validation (2026-10-24) Synchronized reviewed upstream 347b6e4d95 (#7002), which includes the earlier #6986, #6988 and #6994 repairs. Current head: cba2f9fdabde7786ec1d6c26216b44f7ccddfbf5. The complete feature patch is byte-for-byte identical to this PR's pre-repair feature patch and remains 2 files against that upstream revision; no verifier-test changes remain in the PR diff. The preceding head's shard 6 had 13 failed / 1889 passed / 20 skipped from two shared test-contract mismatches. Those are historical results. A user-authorized local support patch 4b251c25 migrated the error diagnostics and run ID checks, then upstream merged #7002 concurrently. The resulting conflict was resolved by taking both upstream test files exactly. The Python cases now check VerifierCommandExitError, exit code 1 and snapshot.stderr.text. The ownership cases compare leader/descendant identities to the actual launcher-assigned nonce and distinguish the inherited identity. Archive rejection, ambient-import isolation, no extraction, process-exit-before-root-cleanup and cleanup-retry checks remain. Clean-base comparison before the upstream merge (Windows, pinned Node 24.22.1/npm 11.16.0, Python 3.12.14, native Windows addon present): the exact focused two-file command for packedBridgeVerification.test.ts and packagedWasmVerifierOwnership.test.ts failed 10 / passed 14 / skipped 3 on be2811e414, then passed 24 / skipped 3 after the local migration. A controlled temporary bypass of ast.parse made the existing invalid-Python regression fail as intended (1 failed / 23 deselected); production bytes were restored. These are local repair evidence, not the final upstream-combination result. On the final integrated ELF branch, npm run test:focused -- tests/boundary/process/packedBridgeVerification.test.ts tests/boundary/process/packagedWasmVerifierOwnership.test.ts tests/boundary/filesystem/verifierCommandOutput.test.ts tests/boundary/process/nativeVerifierPreflightOwnership.test.ts passed 29 / skipped 14 (four files). All 24 packed-Python cases and five command-output cases passed. Existing Windows skips cover three packaged-Wasm lifecycle cases, eight native-client preflight cases and three filesystem failure cases. No new skip was introduced; Linux CI must execute the POSIX coverage. This run does not establish real external-client/provider behavior. Each final integrated branch passed npm run check (six tasks, module boundaries and architecture guards) and git diff --check; normal commit and push hooks passed. No CI gates, timeouts or skips were weakened. Upstream #7002's own green CI does not establish this PR's new-head result. Latest-head confirmation on 2026-10-24: [CI required](https://github.com/morluto/rea/actions/runs/40096655867/job/120883628042), Conventional PR title and PR readiness all succeeded for cba2f9fdabde7786ec1d6c26216b44f7ccddfbf5. Source tests and all eight test shards succeeded. Optional lanes retain their reported skips; the historical local Windows limits remain disclosed. Earlier post-sync feature/shared validation passed 222 tests across six CLI and shared-regression files. After the earlier be2811 merge, the two import/type-erasure source files passed all 36 tests on this branch. Earlier checks also passed all 113 reference-lifetime cases, six property-path regressions, and nine evaluation-gate tests. The former cyclic-path single case passed with the same 512 MiB heap / external 20 s guard (137 ms test time, 3.76 s elapsed); the baseline exhausted the heap. No completed-baseline speedup or process-family RSS claim is made. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
bsakaguchi 2026-11-03 13:38:36 +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#7546
No description provided.