[PR #2685] [MERGED] fix(snapshot): classify non-object roots as invalid content #5312

New issue

Closed

opened 2026-10-31 01:05:39 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 01:05:39 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/2685
Author: @jiangjiang248
Created: 10/13/2026
Status: ✅ Merged
Merged: 10/13/2026
Merged by: @morluto

Base: main ← Head: fix/snapshot-root-classification

📝 Commits (1)

5e3e312 fix(snapshot): classify non-object roots as invalid content

📊 Changes

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

View changed files

📝 src/application/JsonInputFile.ts (+6 -2)
➕ tests/boundary/filesystem/analysisSnapshotInput.test.ts (+88 -0)

📄 Description

Fixes #2675

Summary

Readable snapshots containing [], null, or scalar roots currently return execution_failure with reason io and permission advice. Classify these rejected documents as invalid content instead, consistently for native and streamed JSON reads.

Problem and expected behavior

The object-member reader throws an unclassified TypeError after successfully reading a non-object JSON root. JsonFiles consequently treats it as a filesystem exception. A document-shape rejection should take the existing typed input-error path, retain its explanation, and leave file-selection failures distinct.

Change and scope

Return the existing invalidJson result at both root checks in readJsonObjectFile. The shared Evidence-file boundary then reports evidence_integrity_mismatch / invalid-json. The change stays at the producing boundary; it does not broaden catch clauses or reinterpret arbitrary TypeErrors as malformed input.

Add filesystem-boundary regressions using real temporary files and the production snapshot reader and error projection. Exercise arrays, null, numbers, strings, and booleans, both with small input and with 9 MiB of whitespace to select streaming. Controls cover syntax errors, missing snapshot members, valid snapshots, missing files, directories, and unchanged input bytes.

Contract and boundary impact

Semantic owner and earliest changed stage: src/application/JsonInputFile.ts, object-root validation after JSON decoding.
Contribution scope fit and linked maintainer decision: ordinary correction within existing read-only snapshot loading; advance scope decision not applicable.
CLI and MCP/tool-catalog contract: existing error vocabulary, schemas, and commands retained. Only erroneous rejection classification changes.
Provider, bridge, target-format, or platform compatibility: provider-independent; verified on Windows with Node 24.18.0 and npm 11.16.0.
Evidence, artifact, provenance, or reconstruction contract: valid snapshot decoding and snapshot-member validation retained; rejected input bytes remain unchanged and the root-shape cause message is preserved.
Target selection, authority, upstream behavior, and cleanup details: same selected regular file and existing file-handle ownership boundary; no new execution, mutation, or provider authority.
Generated metadata, package, or installation impact: none; no schema, package, authored skill, or generated-catalog change. Existing documentation and contract vocabulary remain applicable.

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: tests/boundary/filesystem/analysisSnapshotInput.test.ts.
Base reproduction: on 6a05248d464387c0ce4ec1577b4c1b24d3d355c7, the new suite produced 10 failed / 3 passed. All ten non-object-root cases returned execution_failure / io instead of the expected content classification. The final suite passes 13/13.
Representative production error projection after the fix, observed through projectAnalysisError:

{
  "code": "evidence_integrity_mismatch",
  "message": "Evidence file is not valid JSON. Repair or recreate the file and try again.",
  "details": { "reason": "invalid-json" }
}

This excerpt shows selected projection fields, not a captured CLI/MCP transport response.

Remaining proof gaps: no installed-package CLI/MCP session, real Hopper/Ghidra/IDA lane, Linux/macOS run, or complete test suite was executed. This fix concerns the shared file/error boundary and needs no analysis engine.
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/filesystem/analysisSnapshotInput.test.ts — 13 passed on the final diff; also builds the runtime and skill prerequisites.
npx --no-install vitest run tests/boundary/filesystem/analysisSnapshot.test.ts tests/boundary/filesystem/cliJsonInput.test.ts --testNamePattern "analysis snapshot member decoding|analysis snapshot file ownership|CLI JSON streamed input" — 27 passed, 10 outside-selection tests skipped. Includes existing cancellation, file ownership, syntax, UTF-8, and streaming controls.
npm run check — passed: typecheck, lint, format, Knip, metadata, module boundaries, and architecture guards. Lint reported 0 errors and repository warnings.
npm run verify:test-discovery — all 1,034 test files discovered exactly once.
npx --no-install oxfmt --check src/application/JsonInputFile.ts tests/boundary/filesystem/analysisSnapshotInput.test.ts — passed.

Compatibility, safety, and release

Breaking changes or migration steps: none; callers now receive the existing content-error classification for previously misclassified documents.
Real Hopper/Ghidra, browser, or OS coverage: Windows filesystem boundary only; no real-provider claim.
Package or release metadata impact: none.
Security, privacy, process, or containment review: no new capability or authority; no private targets, credentials, generated outputs, or binaries included.

AI assistance

Codex assisted with issue investigation, implementation, regression tests, validation, and this PR description. The change is submitted through the authorized contributor account; this disclosure does not claim personal manual authorship or human mastery of the codebase.

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/2685 **Author:** [@jiangjiang248](https://github.com/jiangjiang248) **Created:** 10/13/2026 **Status:** ✅ Merged **Merged:** 10/13/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `fix/snapshot-root-classification` --- ### 📝 Commits (1) - [`5e3e312`](https://github.com/morluto/rea/commit/5e3e312a8a9e9abe9986d717cc56195a1310f5e7) fix(snapshot): classify non-object roots as invalid content ### 📊 Changes **2 files changed** (+94 additions, -2 deletions) <details> <summary>View changed files</summary> 📝 `src/application/JsonInputFile.ts` (+6 -2) ➕ `tests/boundary/filesystem/analysisSnapshotInput.test.ts` (+88 -0) </details> ### 📄 Description Fixes #2675 ## Summary Readable snapshots containing `[]`, `null`, or scalar roots currently return `execution_failure` with reason `io` and permission advice. Classify these rejected documents as invalid content instead, consistently for native and streamed JSON reads. ## Problem and expected behavior The object-member reader throws an unclassified `TypeError` after successfully reading a non-object JSON root. `JsonFiles` consequently treats it as a filesystem exception. A document-shape rejection should take the existing typed input-error path, retain its explanation, and leave file-selection failures distinct. ## Change and scope Return the existing `invalidJson` result at both root checks in `readJsonObjectFile`. The shared Evidence-file boundary then reports `evidence_integrity_mismatch` / `invalid-json`. The change stays at the producing boundary; it does not broaden catch clauses or reinterpret arbitrary `TypeError`s as malformed input. Add filesystem-boundary regressions using real temporary files and the production snapshot reader and error projection. Exercise arrays, null, numbers, strings, and booleans, both with small input and with 9 MiB of whitespace to select streaming. Controls cover syntax errors, missing snapshot members, valid snapshots, missing files, directories, and unchanged input bytes. ## Contract and boundary impact - Semantic owner and earliest changed stage: `src/application/JsonInputFile.ts`, object-root validation after JSON decoding. - Contribution scope fit and linked maintainer decision: ordinary correction within existing read-only snapshot loading; advance scope decision not applicable. - CLI and MCP/tool-catalog contract: existing error vocabulary, schemas, and commands retained. Only erroneous rejection classification changes. - Provider, bridge, target-format, or platform compatibility: provider-independent; verified on Windows with Node 24.18.0 and npm 11.16.0. - Evidence, artifact, provenance, or reconstruction contract: valid snapshot decoding and snapshot-member validation retained; rejected input bytes remain unchanged and the root-shape cause message is preserved. - Target selection, authority, upstream behavior, and cleanup details: same selected regular file and existing file-handle ownership boundary; no new execution, mutation, or provider authority. - Generated metadata, package, or installation impact: none; no schema, package, authored skill, or generated-catalog change. Existing documentation and contract vocabulary remain applicable. 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: `tests/boundary/filesystem/analysisSnapshotInput.test.ts`. - Base reproduction: on `6a05248d464387c0ce4ec1577b4c1b24d3d355c7`, the new suite produced **10 failed / 3 passed**. All ten non-object-root cases returned `execution_failure` / `io` instead of the expected content classification. The final suite passes **13/13**. - Representative production error projection after the fix, observed through `projectAnalysisError`: ```json { "code": "evidence_integrity_mismatch", "message": "Evidence file is not valid JSON. Repair or recreate the file and try again.", "details": { "reason": "invalid-json" } } ``` This excerpt shows selected projection fields, not a captured CLI/MCP transport response. - Remaining proof gaps: no installed-package CLI/MCP session, real Hopper/Ghidra/IDA lane, Linux/macOS run, or complete test suite was executed. This fix concerns the shared file/error boundary and needs no analysis engine. - [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/filesystem/analysisSnapshotInput.test.ts` — 13 passed on the final diff; also builds the runtime and skill prerequisites. - `npx --no-install vitest run tests/boundary/filesystem/analysisSnapshot.test.ts tests/boundary/filesystem/cliJsonInput.test.ts --testNamePattern "analysis snapshot member decoding|analysis snapshot file ownership|CLI JSON streamed input"` — 27 passed, 10 outside-selection tests skipped. Includes existing cancellation, file ownership, syntax, UTF-8, and streaming controls. - `npm run check` — passed: typecheck, lint, format, Knip, metadata, module boundaries, and architecture guards. Lint reported 0 errors and repository warnings. - `npm run verify:test-discovery` — all 1,034 test files discovered exactly once. - `npx --no-install oxfmt --check src/application/JsonInputFile.ts tests/boundary/filesystem/analysisSnapshotInput.test.ts` — passed. ## Compatibility, safety, and release - Breaking changes or migration steps: none; callers now receive the existing content-error classification for previously misclassified documents. - Real Hopper/Ghidra, browser, or OS coverage: Windows filesystem boundary only; no real-provider claim. - Package or release metadata impact: none. - Security, privacy, process, or containment review: no new capability or authority; no private targets, credentials, generated outputs, or binaries included. ## AI assistance Codex assisted with issue investigation, implementation, regression tests, validation, and this PR description. The change is submitted through the authorized contributor account; this disclosure does not claim personal manual authorship or human mastery of the codebase. ## 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. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-31 01:05:39 +00:00