[PR #1412] [CLOSED] fix: report oversized JSON input as a runtime string limit #2866

Closed
opened 2026-10-17 21:11:12 +00:00 by bsakaguchi · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/1412
Author: @imjimit07
Created: 10/10/2026
Status: ❌ Closed

Base: main ← Head: imjimit07-fix-cli-json-v8-string-limit


📝 Commits (1)

  • 5a6d937 fix: report oversized JSON input as a runtime string limit

📊 Changes

12 files changed (+192 additions, -41 deletions)

View changed files

📝 src/application/JsonFiles.ts (+10 -6)
📝 src/cliJsonInput.ts (+53 -29)
📝 src/cliJsonInputStatus.test.ts (+1 -0)
📝 src/contracts/errorSchemas.ts (+3 -1)
📝 src/domain/analysisErrorPresentation.ts (+4 -0)
📝 src/domain/analysisErrorProjection.test.ts (+1 -0)
📝 src/domain/analysisErrorProjection.ts (+2 -0)
📝 src/domain/evidenceErrors.ts (+7 -1)
📝 src/domain/safeJson.test.ts (+24 -1)
📝 src/domain/safeJson.ts (+42 -0)
📝 tests/boundary/filesystem/jsonUtf8Input.test.ts (+35 -0)
📝 tests/conformance/contracts/analysisErrorProjection.test.ts (+10 -3)

📄 Description

Summary

CLI JSON input files larger than V8's maximum string length (about 512 MiB) were misclassified as invalid-json because every TextDecoder failure was treated as invalid UTF-8. The bytes may be valid JSON, so the real failure is a runtime size constraint. This PR classifies the runtime single-string limit distinctly at both affected boundaries and reports it with observed sizes and remediation. Fixes #1331.

Problem and expected behavior

Smallest trigger: rea trace-application-feature trace.json where trace.json exceeds buffer.constants.MAX_STRING_LENGTH (536,870,888 code units on 64-bit hosts). Decoding throws ERR_STRING_TOO_LONG, which the previous code caught and reported as invalid JSON.

Violated invariant: a decode failure does not imply invalid UTF-8. Node signals the single-string limit as a plain Error with code: "ERR_STRING_TOO_LONG", while invalid UTF-8 raises a TypeError (ERR_ENCODING_INVALID_ENCODED_DATA). A byte-length pre-check cannot distinguish them either, because multi-byte content decodes to fewer code units than bytes (600 MB of 2-byte UTF-8 decodes fine).

Expected behavior: the error names the actual cause, an input too large for the runtime to hold as one string, with a size-specific reason and remediation instead of claiming the JSON is invalid.

Change and scope

  • Added a shared decodeUtf8Json helper in src/domain/safeJson.ts that decodes strict UTF-8 without throws and classifies failures as invalid-utf8 or too-large (by ERR_STRING_TOO_LONG code or the "Cannot create a string longer than" message).
  • CLI boundary (src/cliJsonInput.ts): oversized input now returns input_reason: "input-too-large" with an AnalysisResourceConstraintError projection carrying the input byte size, the single-string code-unit limit, and remediation advice. This mirrors the existing output-side limit handling in src/server/toolResultEncoding.ts.
  • Evidence-file boundary (src/application/JsonFiles.ts): readJsonFile maps the same decode failure to a new too-large reason, projected as resource_constraint instead of evidence_integrity_mismatch.
  • Envelope schema (src/contracts/errorSchemas.ts): the input_reason enum gains input-too-large.

Intentionally not included: streaming or incremental JSON parsing and Evidence-reference indirection (the issue's "ideally" suggestion). Those are a feature change; this PR fixes the misclassification.

Contract and boundary impact

  • Semantic owner and earliest changed stage: domain (safeJson decode classification), then application (evidence file reads), then the CLI adapter (JSON input parsing).
  • CLI and MCP/tool-catalog contract: the CLI error envelope gains input_reason: "input-too-large". The canonical projection union is unchanged; the new case reuses the existing resource_constraint code. The MCP tool catalog is unchanged.
  • Provider, bridge, target-format, or platform compatibility: none.
  • Evidence, artifact, provenance, or reconstruction contract: EvidenceFileError gains the "too-large" reason, projected as resource_constraint with a size-specific message.
  • Process execution, authorization, cleanup, or containment impact: none.
  • Generated metadata (docs/public/product-catalog.json), package, or installation impact: none. analysisErrorJsonSchema covers only the projection union, so docs/error-contract.schema.json is unaffected; no catalog or skill sources changed.

Evidence and regression coverage

  • Tests added or updated:
    • tests/boundary/filesystem/jsonUtf8Input.test.ts adds a real beyond-limit regression: it writes a MAX_STRING_LENGTH + 1 byte file and asserts both readJsonFile and parseCliJsonInput report the size constraint (runs in about 1.2 s).
    • src/domain/safeJson.test.ts covers decode classification (valid multi-byte UTF-8; invalid UTF-8 reported as invalid-utf8 with the TypeError cause).
    • src/cliJsonInputStatus.test.ts, src/domain/analysisErrorProjection.test.ts, and tests/conformance/contracts/analysisErrorProjection.test.ts enumerate the new reason.
  • Base reproduction: on the base revision the new boundary test fails because both boundaries report invalid-json / evidence_integrity_mismatch for the same file.
  • User-visible CLI/MCP output: the CLI now emits code: "resource_constraint", category: "resource_constraint", retryable: false, input_path, input_reason: "input-too-large", a message naming the byte count and the code-unit limit, details.reported_limits with input_bytes and max_string_code_units, and a remediation action advising a smaller subset.
  • Remaining proof gaps: none for this classification. The streaming-parse feature remains unimplemented by design.

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

  • node node_modules/vitest/vitest.mjs run src/domain/safeJson.test.ts src/cliJsonInputStatus.test.ts src/domain/analysisErrorProjection.test.ts tests/conformance/contracts/analysisErrorProjection.test.ts tests/boundary/filesystem/jsonUtf8Input.test.ts tests/boundary/filesystem/cliJsonInput.test.ts tests/boundary/filesystem/evidenceBundleFiles.test.ts — 98 passed; 2 pre-existing Windows failures (fs.symlink EPERM without Developer Mode), verified identical on the clean tree via stash.
  • node node_modules/vitest/vitest.mjs run tests/boundary/cli/jsonInputStatus.test.ts tests/process-global/cliJsonInputStatus.test.ts tests/boundary/mcp/oversizedErrorDeliveryMcp.test.ts src/application/UpdateRuntime.release.test.ts plus other consumer suites — passed after npm run compile; 3 pre-existing Windows failures in tests/boundary/cli/jsonInputStatus.test.ts (assertions expect raw backslash paths inside JSON-serialized output), verified identical on the clean tree.
  • npm run check:fast — typecheck and lint pass.
  • npm run compile — dist build succeeds.
  • npx oxfmt on the changed files — no modifications; the pre-commit hook (oxfmt + oxlint) also made no changes at commit time.

npm run test:focused could not run on this Windows host due to a pre-existing spawn bug in scripts/run-exclusive-command.mjs (shell: true splits C:\Program Files\nodejs\node.exe), so vitest and tsc were invoked directly. Real Hopper/Ghidra verification: not applicable, no provider code changed.

Compatibility, safety, and release

  • Breaking changes or migration steps: none. The input_reason enum gains a value; the same input previously produced invalid-json, and no previously valid call changes behavior.
  • Real Hopper/Ghidra, browser, or OS coverage: not applicable; no provider or platform behavior changed.
  • Package or release metadata impact: none.
  • Security, privacy, process, or containment review: none; local file reads only, no new data persisted or transmitted.

Review checklist

  • This PR addresses a concrete problem or an agreed enhancement.
  • 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/1412 **Author:** [@imjimit07](https://github.com/imjimit07) **Created:** 10/10/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `imjimit07-fix-cli-json-v8-string-limit` --- ### 📝 Commits (1) - [`5a6d937`](https://github.com/morluto/rea/commit/5a6d9375b47db92c71980568583f28ebfaff420d) fix: report oversized JSON input as a runtime string limit ### 📊 Changes **12 files changed** (+192 additions, -41 deletions) <details> <summary>View changed files</summary> 📝 `src/application/JsonFiles.ts` (+10 -6) 📝 `src/cliJsonInput.ts` (+53 -29) 📝 `src/cliJsonInputStatus.test.ts` (+1 -0) 📝 `src/contracts/errorSchemas.ts` (+3 -1) 📝 `src/domain/analysisErrorPresentation.ts` (+4 -0) 📝 `src/domain/analysisErrorProjection.test.ts` (+1 -0) 📝 `src/domain/analysisErrorProjection.ts` (+2 -0) 📝 `src/domain/evidenceErrors.ts` (+7 -1) 📝 `src/domain/safeJson.test.ts` (+24 -1) 📝 `src/domain/safeJson.ts` (+42 -0) 📝 `tests/boundary/filesystem/jsonUtf8Input.test.ts` (+35 -0) 📝 `tests/conformance/contracts/analysisErrorProjection.test.ts` (+10 -3) </details> ### 📄 Description ## Summary CLI JSON input files larger than V8's maximum string length (about 512 MiB) were misclassified as `invalid-json` because every `TextDecoder` failure was treated as invalid UTF-8. The bytes may be valid JSON, so the real failure is a runtime size constraint. This PR classifies the runtime single-string limit distinctly at both affected boundaries and reports it with observed sizes and remediation. Fixes #1331. ## Problem and expected behavior Smallest trigger: `rea trace-application-feature trace.json` where `trace.json` exceeds `buffer.constants.MAX_STRING_LENGTH` (536,870,888 code units on 64-bit hosts). Decoding throws `ERR_STRING_TOO_LONG`, which the previous code caught and reported as invalid JSON. Violated invariant: a decode failure does not imply invalid UTF-8. Node signals the single-string limit as a plain `Error` with `code: "ERR_STRING_TOO_LONG"`, while invalid UTF-8 raises a `TypeError` (`ERR_ENCODING_INVALID_ENCODED_DATA`). A byte-length pre-check cannot distinguish them either, because multi-byte content decodes to fewer code units than bytes (600 MB of 2-byte UTF-8 decodes fine). Expected behavior: the error names the actual cause, an input too large for the runtime to hold as one string, with a size-specific reason and remediation instead of claiming the JSON is invalid. ## Change and scope - Added a shared `decodeUtf8Json` helper in `src/domain/safeJson.ts` that decodes strict UTF-8 without throws and classifies failures as `invalid-utf8` or `too-large` (by `ERR_STRING_TOO_LONG` code or the "Cannot create a string longer than" message). - CLI boundary (`src/cliJsonInput.ts`): oversized input now returns `input_reason: "input-too-large"` with an `AnalysisResourceConstraintError` projection carrying the input byte size, the single-string code-unit limit, and remediation advice. This mirrors the existing output-side limit handling in `src/server/toolResultEncoding.ts`. - Evidence-file boundary (`src/application/JsonFiles.ts`): `readJsonFile` maps the same decode failure to a new `too-large` reason, projected as `resource_constraint` instead of `evidence_integrity_mismatch`. - Envelope schema (`src/contracts/errorSchemas.ts`): the `input_reason` enum gains `input-too-large`. Intentionally not included: streaming or incremental JSON parsing and Evidence-reference indirection (the issue's "ideally" suggestion). Those are a feature change; this PR fixes the misclassification. ## Contract and boundary impact - Semantic owner and earliest changed stage: domain (`safeJson` decode classification), then application (evidence file reads), then the CLI adapter (JSON input parsing). - CLI and MCP/tool-catalog contract: the CLI error envelope gains `input_reason: "input-too-large"`. The canonical projection union is unchanged; the new case reuses the existing `resource_constraint` code. The MCP tool catalog is unchanged. - Provider, bridge, target-format, or platform compatibility: none. - Evidence, artifact, provenance, or reconstruction contract: `EvidenceFileError` gains the `"too-large"` reason, projected as `resource_constraint` with a size-specific message. - Process execution, authorization, cleanup, or containment impact: none. - Generated metadata (`docs/public/product-catalog.json`), package, or installation impact: none. `analysisErrorJsonSchema` covers only the projection union, so `docs/error-contract.schema.json` is unaffected; no catalog or skill sources changed. ## Evidence and regression coverage - Tests added or updated: - `tests/boundary/filesystem/jsonUtf8Input.test.ts` adds a real beyond-limit regression: it writes a `MAX_STRING_LENGTH + 1` byte file and asserts both `readJsonFile` and `parseCliJsonInput` report the size constraint (runs in about 1.2 s). - `src/domain/safeJson.test.ts` covers decode classification (valid multi-byte UTF-8; invalid UTF-8 reported as `invalid-utf8` with the `TypeError` cause). - `src/cliJsonInputStatus.test.ts`, `src/domain/analysisErrorProjection.test.ts`, and `tests/conformance/contracts/analysisErrorProjection.test.ts` enumerate the new reason. - Base reproduction: on the base revision the new boundary test fails because both boundaries report `invalid-json` / `evidence_integrity_mismatch` for the same file. - User-visible CLI/MCP output: the CLI now emits `code: "resource_constraint"`, `category: "resource_constraint"`, `retryable: false`, `input_path`, `input_reason: "input-too-large"`, a message naming the byte count and the code-unit limit, `details.reported_limits` with `input_bytes` and `max_string_code_units`, and a remediation action advising a smaller subset. - Remaining proof gaps: none for this classification. The streaming-parse feature remains unimplemented by design. 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 - `node node_modules/vitest/vitest.mjs run src/domain/safeJson.test.ts src/cliJsonInputStatus.test.ts src/domain/analysisErrorProjection.test.ts tests/conformance/contracts/analysisErrorProjection.test.ts tests/boundary/filesystem/jsonUtf8Input.test.ts tests/boundary/filesystem/cliJsonInput.test.ts tests/boundary/filesystem/evidenceBundleFiles.test.ts` — 98 passed; 2 pre-existing Windows failures (`fs.symlink` EPERM without Developer Mode), verified identical on the clean tree via stash. - `node node_modules/vitest/vitest.mjs run tests/boundary/cli/jsonInputStatus.test.ts tests/process-global/cliJsonInputStatus.test.ts tests/boundary/mcp/oversizedErrorDeliveryMcp.test.ts src/application/UpdateRuntime.release.test.ts` plus other consumer suites — passed after `npm run compile`; 3 pre-existing Windows failures in `tests/boundary/cli/jsonInputStatus.test.ts` (assertions expect raw backslash paths inside JSON-serialized output), verified identical on the clean tree. - `npm run check:fast` — typecheck and lint pass. - `npm run compile` — dist build succeeds. - `npx oxfmt` on the changed files — no modifications; the pre-commit hook (oxfmt + oxlint) also made no changes at commit time. `npm run test:focused` could not run on this Windows host due to a pre-existing spawn bug in `scripts/run-exclusive-command.mjs` (`shell: true` splits `C:\Program Files\nodejs\node.exe`), so vitest and tsc were invoked directly. Real Hopper/Ghidra verification: not applicable, no provider code changed. ## Compatibility, safety, and release - Breaking changes or migration steps: none. The `input_reason` enum gains a value; the same input previously produced `invalid-json`, and no previously valid call changes behavior. - Real Hopper/Ghidra, browser, or OS coverage: not applicable; no provider or platform behavior changed. - Package or release metadata impact: none. - Security, privacy, process, or containment review: none; local file reads only, no new data persisted or transmitted. ## Review checklist - [x] This PR addresses a concrete problem or an agreed enhancement. - [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-17 21:11:12 +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#2866
No description provided.