morluto/rea

Fork 0

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

[PR #4757] [MERGED] fix(javascript): report oversized recovery inputs as file-size limits #6391

New issue

Closed

opened 2026-10-31 01:14:07 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 01:14:07 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/4757
Author: @kaoru0822-kitauji
Created: 10/17/2026
Status: ✅ Merged
Merged: 10/17/2026
Merged by: @kaoru0822-kitauji

Base: main ← Head: fix/js-recovery-size-limit

📝 Commits (1)

0fb7813 fix(javascript): report oversized recovery inputs as file-size limits

📊 Changes

4 files changed (+175 additions, -8 deletions)

View changed files

📝 docs/javascript-recovery.md (+5 -2)
📝 src/javascript/recovery/RecoveryFiles.ts (+45 -5)
➕ tests/boundary/javascript-recovery/wakaruInputAllowance.test.ts (+122 -0)
📝 tests/boundary/javascript-recovery/wakaruIntegration.test.ts (+3 -1)

📄 Description

Summary

Fixes #4629

recover_javascript_sources refused a 67108865-byte (64 MiB + 1) UTF-8 input as a retryable invalid_request with issue reason invalid_value. The 67108864-byte allowance appeared only in message text, and that text also called the refusal an "Invalid analysis output". Malformed UTF-8 got the same reason. Now an input over the total input allowance is a non-retryable resource_constraint with resource: "file-size" and the budget in reported_limits. Malformed UTF-8 is a separate invalid_format issue. The limit is still enforced.

Problem and expected behavior

Smallest trigger: one 64 MiB + 1 byte UTF-8 .js file passed as path.

Root cause: scanRecoveryFile threw a plain AnalysisOutputError when a file went over the byte bound it was given. snapshotRecoveryInput then caught that error, along with read failures and UTF-8 decode failures, and turned all of them into a single invalid_value issue through recoveryInputFailure. That throws away the size fact and the bound at the point that knows what the bound means (the remaining operation-wide input allowance). The provider, service and projection pass that error through unchanged, so later layers cannot get the size facts back.

Expected: the refusal stays, but it says it is about input size, gives the allowance in structured fields, and does not invite retrying the same input. Malformed UTF-8 stays a format problem and can be told apart from size. This matches how other input byte budgets are already reported (for example GoBinaryProvider and the PE fix in #4290).

Change and scope

src/javascript/recovery/RecoveryFiles.ts
- The scanner's two byte-bound refusals (the stat size check, and growth during streaming) now throw a private RecoveryFileSizeError. It is a subclass of AnalysisOutputError that also carries the bound it enforced. Other scanner consumers (engine fingerprint, staged output, report reads) still get an AnalysisOutputError with the same reason, so their behavior does not change.
- snapshotRecoveryInput owns the meaning of the bound. It reports it as AnalysisResourceConstraintError(..., "file-size", ..., { maximum_input_bytes, remaining_input_bytes }, { retryable: false, remediationAction }). remaining_input_bytes explains refusals with several inputs, where the file that trips the limit can be smaller than the total allowance.
- UTF-8 decoding is checked separately and reported as { path: ["path"], reason: "invalid_format", expected: "UTF-8" }. Other read failures still go through recoveryInputFailure (invalid_value) as before.
docs/javascript-recovery.md: documents how the two refusals are classified.
Not included: the non-regular-file (FIFO) refusal message still says "no larger than N bytes". Its classification (invalid_value) is unchanged.

Contribution scope: not applicable. This is an ordinary fix inside the existing recovery boundary, with no new capability or authority.

Contract and boundary impact

Semantic owner and earliest changed stage: snapshotRecoveryInput in src/javascript/recovery/RecoveryFiles.ts, during input admission and before the engine starts.
Contribution scope fit and linked maintainer decision: not applicable (bug fix in an existing boundary).
CLI and MCP/tool-catalog contract: the error classification changes for both adapters through the shared projection. Over-limit inputs move from invalid_request to resource_constraint, and retryable goes from true to false. Malformed UTF-8 stays invalid_request, but its issue reason goes from invalid_value to invalid_format. Tool schemas and catalog are unchanged.
Provider, bridge, target-format, or platform compatibility: none. The Wakaru invocation is unchanged.
Evidence, artifact, provenance, or reconstruction contract: none. Successful recovery is unchanged, and the refusal still happens before the output tree is created.
Target selection, authority, upstream behavior, and cleanup details: unchanged. The output directory stays absent, and the engine is not launched.
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:
- New: tests/boundary/javascript-recovery/wakaruInputAllowance.test.ts. It goes through JavaScriptRecoveryService → WakaruProvider → projectAnalysisError. Only engine command resolution is mocked, because admission happens before launch. A launcher that throws if called checks that the engine is never started. It covers:
  - An exactly 64 MiB input is admitted by snapshotRecoveryInputs.
  - A 64 MiB + 1 input gives resource_constraint, retryable: false, resource: "file-size", and reported_limits { maximum_input_bytes: 67108864, remaining_input_bytes: 67108864 }.
  - A 20-byte primary plus a 64 MiB additional input reports remaining_input_bytes: 67108844.
  - Malformed UTF-8 (ff) gives invalid_format with no resource.
  - Each refusal leaves the output directory absent.
- Updated: in tests/boundary/javascript-recovery/wakaruIntegration.test.ts, the Linux x64 case "rejects aggregate input overflow before launching the engine" asserted the old invalid_request code. It now expects resource_constraint.
Base reproduction: the new test fails on origin/main (2/2 failed). I checked this before the change, and again after rebasing by restoring the main version of RecoveryFiles.ts. It passes with the fix.

User-visible CLI/MCP output: the shared projection used by both adapters, printed from the test with the fixture root replaced by <fixture>.

Before (main), 64 MiB + 1:

{"code":"invalid_request","category":"invalid_input","message":"Analysis input is invalid. Check the arguments and try again.","retryable":true,"remediation":{"action":"Correct the listed arguments and retry."},"details":{"operation":"recover_javascript_sources","issues":[{"path":["path"],"reason":"invalid_value","message":"Cannot snapshot UTF-8 JavaScript input <fixture>/over.js: Invalid analysis output for recover_javascript_sources: Expected a regular file no larger than 67108864 bytes: <fixture>/over.js"}]}}

After:

{"code":"resource_constraint","category":"resource_constraint","message":"Selected JavaScript input <fixture>/over.js exceeds the 67108864 bytes remaining of the 67108864-byte total input allowance","retryable":false,"remediation":{"action":"Select JavaScript inputs whose combined size is within maximum_input_bytes. Do not truncate required source; retrying the unchanged selection will fail again."},"details":{"operation":"recover_javascript_sources","resource":"file-size","reason":"Selected JavaScript input <fixture>/over.js exceeds the 67108864 bytes remaining of the 67108864-byte total input allowance","reported_limits":{"maximum_input_bytes":67108864,"remaining_input_bytes":67108864}}}

Malformed UTF-8, before: "issues":[{"path":["path"],"reason":"invalid_value","message":"Cannot snapshot UTF-8 JavaScript input <fixture>/invalid.js: The encoded data was not valid for encoding utf-8"}]. After: "issues":[{"path":["path"],"reason":"invalid_format","message":"JavaScript input is not valid UTF-8: <fixture>/invalid.js","expected":"UTF-8"}].

Remaining proof gaps:
- Validation ran on macOS arm64 only. The Linux x64-only suites (wakaruIntegration.test.ts, javascriptRecoveryMcp.test.ts) were skipped locally, so the updated integration assertion has not been run.
- There was no real Wakaru run and no subprocess CLI run. On this host resolveWakaruCommand refuses non-Linux platforms before admission.
- Growth past the limit while streaming goes through the same typed refusal, but no test exercises it here.

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

npm run test:focused -- tests/boundary/javascript-recovery/wakaruInputAllowance.test.ts on origin/main source: 2 failed (expected base failure). I ran it before the change and again after the rebase.
npm run test:focused -- tests/boundary/javascript-recovery/wakaruInputAllowance.test.ts tests/boundary/javascript-recovery/wakaruIntegration.test.ts tests/boundary/javascript-recovery/wakaruPublicationCancellation.test.ts tests/boundary/mcp/javascriptRecoveryMcp.test.ts src/javascript/recovery/RecoveryFiles.test.ts src/javascript/recovery/WakaruCommand.test.ts src/application/javascript/JavaScriptRecoveryService.test.ts src/domain/analysisErrorProjection.test.ts: 6 files passed and 2 skipped (Linux x64 only); 76 tests passed, 40 skipped.
npx tsc --noEmit -p .: passed.
npm run docs:check: passed.
npx oxfmt and npx oxlint on the changed files: no errors. There is one existing max-params warning on snapshotRecoveryInput, which already had 5 parameters on main.

Compatibility, safety, and release

Breaking changes or migration steps: none declared. Callers that matched on invalid_request/invalid_value for over-limit recovery inputs will now see resource_constraint/file-size, and malformed UTF-8 now has reason invalid_format. The limit and successful recovery are unchanged.
Real Hopper/Ghidra, browser, or OS coverage: not applicable. Real Wakaru on Linux x64 was not exercised (see gaps).
Package or release metadata impact: none.
Security, privacy, process, or containment review: the limit is still enforced before any engine launch. The error carries the caller-selected path, as before, and no new data is retained.

Author readiness

Human owner:

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.

🤖 Generated with Claude Code

_{🔄 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/4757 **Author:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Created:** 10/17/2026 **Status:** ✅ Merged **Merged:** 10/17/2026 **Merged by:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Base:** `main` ← **Head:** `fix/js-recovery-size-limit` --- ### 📝 Commits (1) - [`0fb7813`](https://github.com/morluto/rea/commit/0fb78133724de06b516aa0e91f4f8f90cb84a723) fix(javascript): report oversized recovery inputs as file-size limits ### 📊 Changes **4 files changed** (+175 additions, -8 deletions) <details> <summary>View changed files</summary> 📝 `docs/javascript-recovery.md` (+5 -2) 📝 `src/javascript/recovery/RecoveryFiles.ts` (+45 -5) ➕ `tests/boundary/javascript-recovery/wakaruInputAllowance.test.ts` (+122 -0) 📝 `tests/boundary/javascript-recovery/wakaruIntegration.test.ts` (+3 -1) </details> ### 📄 Description ## Summary Fixes #4629 `recover_javascript_sources` refused a 67108865-byte (64 MiB + 1) UTF-8 input as a retryable `invalid_request` with issue reason `invalid_value`. The 67108864-byte allowance appeared only in message text, and that text also called the refusal an "Invalid analysis output". Malformed UTF-8 got the same reason. Now an input over the total input allowance is a non-retryable `resource_constraint` with `resource: "file-size"` and the budget in `reported_limits`. Malformed UTF-8 is a separate `invalid_format` issue. The limit is still enforced. ## Problem and expected behavior Smallest trigger: one 64 MiB + 1 byte UTF-8 `.js` file passed as `path`. Root cause: `scanRecoveryFile` threw a plain `AnalysisOutputError` when a file went over the byte bound it was given. `snapshotRecoveryInput` then caught that error, along with read failures and UTF-8 decode failures, and turned all of them into a single `invalid_value` issue through `recoveryInputFailure`. That throws away the size fact and the bound at the point that knows what the bound means (the remaining operation-wide input allowance). The provider, service and projection pass that error through unchanged, so later layers cannot get the size facts back. Expected: the refusal stays, but it says it is about input size, gives the allowance in structured fields, and does not invite retrying the same input. Malformed UTF-8 stays a format problem and can be told apart from size. This matches how other input byte budgets are already reported (for example `GoBinaryProvider` and the PE fix in #4290). ## Change and scope - `src/javascript/recovery/RecoveryFiles.ts` - The scanner's two byte-bound refusals (the `stat` size check, and growth during streaming) now throw a private `RecoveryFileSizeError`. It is a subclass of `AnalysisOutputError` that also carries the bound it enforced. Other scanner consumers (engine fingerprint, staged output, report reads) still get an `AnalysisOutputError` with the same reason, so their behavior does not change. - `snapshotRecoveryInput` owns the meaning of the bound. It reports it as `AnalysisResourceConstraintError(..., "file-size", ..., { maximum_input_bytes, remaining_input_bytes }, { retryable: false, remediationAction })`. `remaining_input_bytes` explains refusals with several inputs, where the file that trips the limit can be smaller than the total allowance. - UTF-8 decoding is checked separately and reported as `{ path: ["path"], reason: "invalid_format", expected: "UTF-8" }`. Other read failures still go through `recoveryInputFailure` (`invalid_value`) as before. - `docs/javascript-recovery.md`: documents how the two refusals are classified. - Not included: the non-regular-file (FIFO) refusal message still says "no larger than N bytes". Its classification (`invalid_value`) is unchanged. Contribution scope: not applicable. This is an ordinary fix inside the existing recovery boundary, with no new capability or authority. ## Contract and boundary impact - Semantic owner and earliest changed stage: `snapshotRecoveryInput` in `src/javascript/recovery/RecoveryFiles.ts`, during input admission and before the engine starts. - Contribution scope fit and linked maintainer decision: not applicable (bug fix in an existing boundary). - CLI and MCP/tool-catalog contract: the error classification changes for both adapters through the shared projection. Over-limit inputs move from `invalid_request` to `resource_constraint`, and `retryable` goes from true to false. Malformed UTF-8 stays `invalid_request`, but its issue reason goes from `invalid_value` to `invalid_format`. Tool schemas and catalog are unchanged. - Provider, bridge, target-format, or platform compatibility: none. The Wakaru invocation is unchanged. - Evidence, artifact, provenance, or reconstruction contract: none. Successful recovery is unchanged, and the refusal still happens before the output tree is created. - Target selection, authority, upstream behavior, and cleanup details: unchanged. The output directory stays absent, and the engine is not launched. - 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: - New: `tests/boundary/javascript-recovery/wakaruInputAllowance.test.ts`. It goes through `JavaScriptRecoveryService` → `WakaruProvider` → `projectAnalysisError`. Only engine command resolution is mocked, because admission happens before launch. A launcher that throws if called checks that the engine is never started. It covers: - An exactly 64 MiB input is admitted by `snapshotRecoveryInputs`. - A 64 MiB + 1 input gives `resource_constraint`, `retryable: false`, `resource: "file-size"`, and `reported_limits` `{ maximum_input_bytes: 67108864, remaining_input_bytes: 67108864 }`. - A 20-byte primary plus a 64 MiB additional input reports `remaining_input_bytes: 67108844`. - Malformed UTF-8 (`ff`) gives `invalid_format` with no `resource`. - Each refusal leaves the output directory absent. - Updated: in `tests/boundary/javascript-recovery/wakaruIntegration.test.ts`, the Linux x64 case "rejects aggregate input overflow before launching the engine" asserted the old `invalid_request` code. It now expects `resource_constraint`. - Base reproduction: the new test fails on `origin/main` (2/2 failed). I checked this before the change, and again after rebasing by restoring the main version of `RecoveryFiles.ts`. It passes with the fix. - User-visible CLI/MCP output: the shared projection used by both adapters, printed from the test with the fixture root replaced by `<fixture>`. Before (main), 64 MiB + 1: ```json {"code":"invalid_request","category":"invalid_input","message":"Analysis input is invalid. Check the arguments and try again.","retryable":true,"remediation":{"action":"Correct the listed arguments and retry."},"details":{"operation":"recover_javascript_sources","issues":[{"path":["path"],"reason":"invalid_value","message":"Cannot snapshot UTF-8 JavaScript input <fixture>/over.js: Invalid analysis output for recover_javascript_sources: Expected a regular file no larger than 67108864 bytes: <fixture>/over.js"}]}} ``` After: ```json {"code":"resource_constraint","category":"resource_constraint","message":"Selected JavaScript input <fixture>/over.js exceeds the 67108864 bytes remaining of the 67108864-byte total input allowance","retryable":false,"remediation":{"action":"Select JavaScript inputs whose combined size is within maximum_input_bytes. Do not truncate required source; retrying the unchanged selection will fail again."},"details":{"operation":"recover_javascript_sources","resource":"file-size","reason":"Selected JavaScript input <fixture>/over.js exceeds the 67108864 bytes remaining of the 67108864-byte total input allowance","reported_limits":{"maximum_input_bytes":67108864,"remaining_input_bytes":67108864}}} ``` Malformed UTF-8, before: `"issues":[{"path":["path"],"reason":"invalid_value","message":"Cannot snapshot UTF-8 JavaScript input <fixture>/invalid.js: The encoded data was not valid for encoding utf-8"}]`. After: `"issues":[{"path":["path"],"reason":"invalid_format","message":"JavaScript input is not valid UTF-8: <fixture>/invalid.js","expected":"UTF-8"}]`. - Remaining proof gaps: - Validation ran on macOS arm64 only. The Linux x64-only suites (`wakaruIntegration.test.ts`, `javascriptRecoveryMcp.test.ts`) were skipped locally, so the updated integration assertion has not been run. - There was no real Wakaru run and no subprocess CLI run. On this host `resolveWakaruCommand` refuses non-Linux platforms before admission. - Growth past the limit while streaming goes through the same typed refusal, but no test exercises it here. 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 - `npm run test:focused -- tests/boundary/javascript-recovery/wakaruInputAllowance.test.ts` on `origin/main` source: 2 failed (expected base failure). I ran it before the change and again after the rebase. - `npm run test:focused -- tests/boundary/javascript-recovery/wakaruInputAllowance.test.ts tests/boundary/javascript-recovery/wakaruIntegration.test.ts tests/boundary/javascript-recovery/wakaruPublicationCancellation.test.ts tests/boundary/mcp/javascriptRecoveryMcp.test.ts src/javascript/recovery/RecoveryFiles.test.ts src/javascript/recovery/WakaruCommand.test.ts src/application/javascript/JavaScriptRecoveryService.test.ts src/domain/analysisErrorProjection.test.ts`: 6 files passed and 2 skipped (Linux x64 only); 76 tests passed, 40 skipped. - `npx tsc --noEmit -p .`: passed. - `npm run docs:check`: passed. - `npx oxfmt` and `npx oxlint` on the changed files: no errors. There is one existing `max-params` warning on `snapshotRecoveryInput`, which already had 5 parameters on main. ## Compatibility, safety, and release - Breaking changes or migration steps: none declared. Callers that matched on `invalid_request`/`invalid_value` for over-limit recovery inputs will now see `resource_constraint`/`file-size`, and malformed UTF-8 now has reason `invalid_format`. The limit and successful recovery are unchanged. - Real Hopper/Ghidra, browser, or OS coverage: not applicable. Real Wakaru on Linux x64 was not exercised (see gaps). - Package or release metadata impact: none. - Security, privacy, process, or containment review: the limit is still enforced before any engine launch. The error carries the caller-selected path, as before, and no new data is retained. ## Author readiness Human owner: - [ ] 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. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- <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:07 +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#6391

No description provided.

Rows
Columns