morluto/rea

Fork 0

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

[PR #6073] [CLOSED] fix(javascript): keep undecodable HTML reference discovery unknown #7142

New issue

Closed

opened 2026-11-01 21:45:42 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-11-01 21:45:42 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/6073
Author: @rawaneltheni
Created: 10/21/2026
Status: ❌ Closed

Base: main ← Head: fix/html-invalid-utf8-reference-scope

📝 Commits (2)

bbfe6d5 fix(javascript): keep undecodable HTML reference discovery unknown
f6e7e25 test(javascript): pin source selection order for undecodable HTML

📊 Changes

2 files changed (+65 additions, -0 deletions)

View changed files

📝 src/application/javascript/JavaScriptArtifactGraphStructure.ts (+7 -0)
📝 tests/boundary/filesystem/javascriptArtifactReconstruction.test.ts (+58 -0)

📄 Description

Summary

Fixes #5951.

When a selected HTML file is not valid UTF-8, analyze-javascript-application returns partial coverage but no unknown processing scope for that file. A consumer enumerating per-file gaps cannot tell that HTML script-reference discovery was skipped. Undecodable JavaScript and JSON already record their gaps; HTML did not.

With this change, the graph records an unavailable parse-html-script-references scope for the HTML file, in the same form as the existing JavaScript and JSON gaps.

Problem and expected behavior

Smallest trigger: a directory artifact whose index.html contains <script src="main.js"></script> followed by byte 0xff, with index.html included in source_paths (or no source_paths given).

The reader records the decode failure as text: { included: false, reason: "invalid-utf8" }. HTML extraction in JavaScriptArtifactAnalysis.ts is guarded by file.text.included, so it is skipped. The file-gap chain in JavaScriptArtifactGraphStructure.ts has branches for out-of-scope files, analysis failures and JavaScript parse failures, but none for HTML whose text is unavailable. The skipped phase therefore leaves no trace.

Violated invariant: the graph evidence contract requires unavailable processing to stay explicit. javascriptArtifactAnalysisScopes.html already assigns parse-html-script-references with artifact-bytes authority to HTML, and the per-file gap documentation describes that phase.

Expected and new behavior: the HTML inventory observation and partial coverage are unchanged. The graph additionally contains one unknown scope for index.html with operation: "parse-html-script-references", authority: "artifact-bytes", state: "unavailable" and the limitation HTML text was unavailable: invalid-utf8.

Change and scope

The fix is one else if branch in the existing file-gap chain, next to the JavaScript branch. It spreads javascriptArtifactAnalysisScopes.html rather than repeating the operation and authority strings.

Its position in the chain determines precedence, and both cases are deliberate:

HTML outside source_paths is caught by the earlier outside-analysis-scope branch and still reports only select-javascript-sources. The regression test now asserts this.
An interrupted analysis is caught by the earlier analysis_failure branch and still reports only the interruption.

Intentionally not included: no inline-script count, script reference or location is inferred from text that could not be decoded.

On the reason values: JavaScriptArtifactTextUnavailableReason has three members. outside-analysis-scope is handled by the first branch and not-applicable is produced only for native addons, so for HTML the new branch currently sees only invalid-utf8. The limitation interpolates file.text.reason, so a future reason reaches the same branch with an accurate message.

Follow the contribution scope.
Unresolved scope, authority, side effects, or required verification block
integration. Link the required maintainer scope decision for new providers or
capabilities involving target execution, injection, mutation, or protection
recovery; ordinary fixes within existing boundaries may state "not applicable".
Draft PRs may leave the scope decision pending and the readiness and merge checklists
unchecked. Maintainers can help resolve open questions.

Contract and boundary impact

Semantic owner and earliest changed stage: graph file-gap projection in src/application/javascript/JavaScriptArtifactGraphStructure.ts. The reader and analysis stages are unchanged.
Contribution scope fit and linked maintainer decision: ordinary fix within existing boundaries; not applicable.
CLI and MCP/tool-catalog contract: none. The output adds unknown nodes using the existing schema and operation name.
Provider, bridge, target-format, or platform compatibility: none.
Evidence, artifact, provenance, or reconstruction contract: graphs for selected HTML that cannot be decoded gain one unknown scope node and its edge. No existing node or edge changes.
Target selection, authority, upstream behavior, and cleanup details (if applicable): not applicable.
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.

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: tests/boundary/filesystem/javascriptArtifactReconstruction.test.ts, "keeps undecodable HTML reference discovery as an explicit unknown", built from the issue's invalid-HTML fixture. It asserts:
- with source_paths: ["main.js"], index.html has exactly one gap, select-javascript-sources, which pins the branch order described above;
- with no source_paths, coverage is partial and index.html has exactly one gap with the expected operation, limitation and artifact-bytes / unavailable / unknown evidence;
- no HTML script loads edge is produced, so the valid <script src> before the bad byte is not silently kept.
Base reproduction (executed): with the source change reverted to the parent commit, the new test fails with expected [] to have a length of 1 but got +0 (1 failed, 13 passed). With the change applied, it passes.

User-visible CLI/MCP output (executed): the issue's four-case reproduction script, run against a freshly compiled checkout of this branch:

{"case":"invalid-html","coverage":"partial","text_status":"invalid-utf8","parse_status":null,"unknown":[{"path":"index.html","operation":"parse-html-script-references"}],"html_loads":0}
{"case":"valid-empty-html","coverage":"complete","text_status":"included","parse_status":null,"unknown":[],"html_loads":0}
{"case":"valid-reference-html","coverage":"complete","text_status":"included","parse_status":null,"unknown":[],"html_loads":1}
{"case":"invalid-json-control","coverage":"partial","text_status":"included","parse_status":null,"unknown":[{"path":"data.json","operation":"parse-json-module"}],"html_loads":0}

On the base revision the invalid-html case reports "unknown":[]. The other three cases are unchanged.

Full unknown observation for index.html

{
  "properties": {
    "path": "index.html",
    "operation": "parse-html-script-references",
    "limitation": "HTML text was unavailable: invalid-utf8."
  },
  "evidence": {
    "authority": "artifact-bytes",
    "state": "unavailable",
    "confidence": "unknown",
    "artifact": {
      "available": true,
      "artifact_id": "art_247ad11414d9f747f4b91d1e0e0fe56f40963261c5f204233a195f6ae4efc1ad",
      "sha256": "247ad11414d9f747f4b91d1e0e0fe56f40963261c5f204233a195f6ae4efc1ad"
    },
    "location": {
      "available": true,
      "value": { "kind": "artifact-path", "path": "index.html" }
    },
    "extractor": {
      "name": "rea-javascript-artifact-reconstruction",
      "version": "1",
      "operation": "parse-html-script-references",
      "executable_sha256": null
    },
    "coverage": { "status": "partial", "truncated": false, "omitted_count": null, "limits": [] },
    "limitations": ["HTML text was unavailable: invalid-utf8."],
    "evidence_ids": []
  }
}

Remaining proof gaps:
- ASAR inputs were not exercised; they reach the same branch whenever an HTML entry is inventoried with unavailable text.
- Local runs used Node.js 22.23.2 on macOS arm64, not the pinned 24.18.0. CI run 39627062347 is awaiting maintainer approval, so the full suite has not yet run on this branch.

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/filesystem/javascriptArtifactReconstruction.test.ts tests/boundary/filesystem/javascriptArtifactGraphObservationLookup.test.ts tests/acceptance/applications/javascriptAnalysisLimits.test.ts: 3 files, 22 tests passed.
Same boundary test with JavaScriptArtifactGraphStructure.ts restored from the parent commit: 1 failed (the new test), 13 passed.
npm run test:local -- src/application/javascript/JavaScriptArtifactGraphStructure.test.ts: 16 passed.
npm run check:fast: typecheck and lint passed.
npm run compile followed by the issue's reproduction script: output shown above.

Compatibility, safety, and release

Breaking changes or migration steps: none. Consumers see one additional unknown scope node for undecodable HTML; no existing output changes.
Real Hopper/Ghidra, browser, or OS coverage: not applicable. Local macOS run only.
Package or release metadata impact: none.
Security, privacy, process, or containment review: no new reads, execution or network access. The branch only records a gap for bytes the reader has already read.

Author readiness

Human owner: @rawaneltheni

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/6073 **Author:** [@rawaneltheni](https://github.com/rawaneltheni) **Created:** 10/21/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/html-invalid-utf8-reference-scope` --- ### 📝 Commits (2) - [`bbfe6d5`](https://github.com/morluto/rea/commit/bbfe6d59ed8bdd011aecc2105da2f4ff477e134e) fix(javascript): keep undecodable HTML reference discovery unknown - [`f6e7e25`](https://github.com/morluto/rea/commit/f6e7e2555c40ddb9a1aad51f275ae1db46985c90) test(javascript): pin source selection order for undecodable HTML ### 📊 Changes **2 files changed** (+65 additions, -0 deletions) <details> <summary>View changed files</summary> 📝 `src/application/javascript/JavaScriptArtifactGraphStructure.ts` (+7 -0) 📝 `tests/boundary/filesystem/javascriptArtifactReconstruction.test.ts` (+58 -0) </details> ### 📄 Description ## Summary Fixes #5951. When a selected HTML file is not valid UTF-8, `analyze-javascript-application` returns partial coverage but no unknown processing scope for that file. A consumer enumerating per-file gaps cannot tell that HTML script-reference discovery was skipped. Undecodable JavaScript and JSON already record their gaps; HTML did not. With this change, the graph records an unavailable `parse-html-script-references` scope for the HTML file, in the same form as the existing JavaScript and JSON gaps. ## Problem and expected behavior Smallest trigger: a directory artifact whose `index.html` contains `<script src="main.js"></script>` followed by byte `0xff`, with `index.html` included in `source_paths` (or no `source_paths` given). The reader records the decode failure as `text: { included: false, reason: "invalid-utf8" }`. HTML extraction in `JavaScriptArtifactAnalysis.ts` is guarded by `file.text.included`, so it is skipped. The file-gap chain in `JavaScriptArtifactGraphStructure.ts` has branches for out-of-scope files, analysis failures and JavaScript parse failures, but none for HTML whose text is unavailable. The skipped phase therefore leaves no trace. Violated invariant: the graph evidence contract requires unavailable processing to stay explicit. `javascriptArtifactAnalysisScopes.html` already assigns `parse-html-script-references` with `artifact-bytes` authority to HTML, and the per-file gap documentation describes that phase. Expected and new behavior: the HTML inventory observation and partial coverage are unchanged. The graph additionally contains one unknown scope for `index.html` with `operation: "parse-html-script-references"`, `authority: "artifact-bytes"`, `state: "unavailable"` and the limitation `HTML text was unavailable: invalid-utf8.` ## Change and scope The fix is one `else if` branch in the existing file-gap chain, next to the JavaScript branch. It spreads `javascriptArtifactAnalysisScopes.html` rather than repeating the operation and authority strings. Its position in the chain determines precedence, and both cases are deliberate: - HTML outside `source_paths` is caught by the earlier `outside-analysis-scope` branch and still reports only `select-javascript-sources`. The regression test now asserts this. - An interrupted analysis is caught by the earlier `analysis_failure` branch and still reports only the interruption. Intentionally not included: no inline-script count, script reference or location is inferred from text that could not be decoded. On the reason values: `JavaScriptArtifactTextUnavailableReason` has three members. `outside-analysis-scope` is handled by the first branch and `not-applicable` is produced only for native addons, so for HTML the new branch currently sees only `invalid-utf8`. The limitation interpolates `file.text.reason`, so a future reason reaches the same branch with an accurate message. Follow the [contribution scope](https://github.com/morluto/rea/blob/main/CONTRIBUTING.md#contribution-scope). Unresolved scope, authority, side effects, or required verification block integration. Link the required maintainer scope decision for new providers or capabilities involving target execution, injection, mutation, or protection recovery; ordinary fixes within existing boundaries may state "not applicable". Draft PRs may leave the scope decision pending and the readiness and merge checklists unchecked. Maintainers can help resolve open questions. ## Contract and boundary impact - Semantic owner and earliest changed stage: graph file-gap projection in `src/application/javascript/JavaScriptArtifactGraphStructure.ts`. The reader and analysis stages are unchanged. - Contribution scope fit and linked maintainer decision: ordinary fix within existing boundaries; not applicable. - CLI and MCP/tool-catalog contract: none. The output adds unknown nodes using the existing schema and operation name. - Provider, bridge, target-format, or platform compatibility: none. - Evidence, artifact, provenance, or reconstruction contract: graphs for selected HTML that cannot be decoded gain one unknown scope node and its edge. No existing node or edge changes. - Target selection, authority, upstream behavior, and cleanup details (if applicable): not applicable. - 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. 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: `tests/boundary/filesystem/javascriptArtifactReconstruction.test.ts`, "keeps undecodable HTML reference discovery as an explicit unknown", built from the issue's invalid-HTML fixture. It asserts: - with `source_paths: ["main.js"]`, `index.html` has exactly one gap, `select-javascript-sources`, which pins the branch order described above; - with no `source_paths`, coverage is `partial` and `index.html` has exactly one gap with the expected operation, limitation and `artifact-bytes` / `unavailable` / `unknown` evidence; - no HTML script `loads` edge is produced, so the valid `<script src>` before the bad byte is not silently kept. - Base reproduction (executed): with the source change reverted to the parent commit, the new test fails with `expected [] to have a length of 1 but got +0` (1 failed, 13 passed). With the change applied, it passes. - User-visible CLI/MCP output (executed): the issue's four-case reproduction script, run against a freshly compiled checkout of this branch: ```json {"case":"invalid-html","coverage":"partial","text_status":"invalid-utf8","parse_status":null,"unknown":[{"path":"index.html","operation":"parse-html-script-references"}],"html_loads":0} {"case":"valid-empty-html","coverage":"complete","text_status":"included","parse_status":null,"unknown":[],"html_loads":0} {"case":"valid-reference-html","coverage":"complete","text_status":"included","parse_status":null,"unknown":[],"html_loads":1} {"case":"invalid-json-control","coverage":"partial","text_status":"included","parse_status":null,"unknown":[{"path":"data.json","operation":"parse-json-module"}],"html_loads":0} ``` On the base revision the `invalid-html` case reports `"unknown":[]`. The other three cases are unchanged. <details> <summary>Full unknown observation for <code>index.html</code></summary> ```json { "properties": { "path": "index.html", "operation": "parse-html-script-references", "limitation": "HTML text was unavailable: invalid-utf8." }, "evidence": { "authority": "artifact-bytes", "state": "unavailable", "confidence": "unknown", "artifact": { "available": true, "artifact_id": "art_247ad11414d9f747f4b91d1e0e0fe56f40963261c5f204233a195f6ae4efc1ad", "sha256": "247ad11414d9f747f4b91d1e0e0fe56f40963261c5f204233a195f6ae4efc1ad" }, "location": { "available": true, "value": { "kind": "artifact-path", "path": "index.html" } }, "extractor": { "name": "rea-javascript-artifact-reconstruction", "version": "1", "operation": "parse-html-script-references", "executable_sha256": null }, "coverage": { "status": "partial", "truncated": false, "omitted_count": null, "limits": [] }, "limitations": ["HTML text was unavailable: invalid-utf8."], "evidence_ids": [] } } ``` </details> - Remaining proof gaps: - ASAR inputs were not exercised; they reach the same branch whenever an HTML entry is inventoried with unavailable text. - Local runs used Node.js 22.23.2 on macOS arm64, not the pinned 24.18.0. CI run [39627062347](https://github.com/morluto/rea/actions/runs/39627062347) is awaiting maintainer approval, so the full suite has not yet run on this branch. 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 - `npm run test:focused -- tests/boundary/filesystem/javascriptArtifactReconstruction.test.ts tests/boundary/filesystem/javascriptArtifactGraphObservationLookup.test.ts tests/acceptance/applications/javascriptAnalysisLimits.test.ts`: 3 files, 22 tests passed. - Same boundary test with `JavaScriptArtifactGraphStructure.ts` restored from the parent commit: 1 failed (the new test), 13 passed. - `npm run test:local -- src/application/javascript/JavaScriptArtifactGraphStructure.test.ts`: 16 passed. - `npm run check:fast`: typecheck and lint passed. - `npm run compile` followed by the issue's reproduction script: output shown above. ## Compatibility, safety, and release - Breaking changes or migration steps: none. Consumers see one additional unknown scope node for undecodable HTML; no existing output changes. - Real Hopper/Ghidra, browser, or OS coverage: not applicable. Local macOS run only. - Package or release metadata impact: none. - Security, privacy, process, or containment review: no new reads, execution or network access. The branch only records a gap for bytes the reader has already read. ## Author readiness Human owner: @rawaneltheni - [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. ## 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-11-01 21:45:42 +00:00