morluto/rea

Fork 0

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

[PR #2940] [CLOSED] fix(javascript): report inline HTML scripts that analysis skips #5505

New issue

Closed

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

bsakaguchi commented

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

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/2940
Author: @rileygramlich
Created: 10/13/2026
Status: ❌ Closed

Base: main ← Head: fix/html-inline-scripts

📝 Commits (3)

c97aca7 fix(javascript): report inline HTML scripts that analysis skips
c0b916e fix(javascript): bound inline script records and index HTML lines
80fe1ec fix(javascript): use parse5 coordinates and truncated inline coverage

📊 Changes

11 files changed (+562 additions, -57 deletions)

View changed files

📝 docs/javascript-artifact-reconstruction.md (+8 -0)
📝 src/application/javascript/JavaScriptArtifactAnalysis.ts (+62 -33)
📝 src/application/javascript/JavaScriptArtifactAnalysisTypes.ts (+14 -0)
📝 src/application/javascript/JavaScriptArtifactGraphBuilder.ts (+28 -12)
📝 src/application/javascript/JavaScriptArtifactGraphContext.ts (+36 -2)
📝 src/application/javascript/JavaScriptArtifactGraphStructure.ts (+31 -0)
📝 src/application/javascript/JavaScriptSemanticGraphResources.test.ts (+1 -0)
📝 src/domain/javascript/htmlArtifactReferences.test.ts (+79 -4)
📝 src/domain/javascript/htmlArtifactReferences.ts (+120 -6)
📝 tests/boundary/filesystem/javascriptHtmlEntrypoints.test.ts (+182 -0)
📝 tests/fixtures/javascriptSemanticGraph.ts (+1 -0)

📄 Description

Summary

Fixes #2834.

analyze-javascript-application now reports inline JavaScript in local HTML that it does not analyze. Each skipped body gets its HTML file, script type and exact source range, and graph coverage becomes partial instead of complete.

Problem and expected behavior

HTML extraction read only <script src> references. Inline classic and module bodies were never parsed, and no limitation mentioned them. In Capacitor and Cordova apps the app logic usually sits inline in www/index.html. For those apps the result covered only cordova.js and reported complete, while the inline endpoints, storage writes and imports were missing.

With the fixture from #2834, the result is now:

coverage: partial
limitations: www/index.html has 2 inline JavaScript bodies starting at lines 5, 9 that were not analyzed; endpoints, storage, imports and other facts in them are absent from the evidence.
unknown parse-html-inline-scripts scope:
  inline_script_count: 2
  inline_scripts: [{"script_type":"classic","location":{"start":{"line":5,"column":8},"end":{"line":8,"column":0}}},
                   {"script_type":"module","location":{"start":{"line":9,"column":22},"end":{"line":9,"column":67}}}]

Change and scope

This is the issue's "at minimum" outcome: make the omission visible and structured. Analyzing the bodies needs an embedded-module identity in the evidence graph, which the issue suggests discussing first, so it is not included here.

Extraction (htmlArtifactReferences): also returns inline script bodies. Only script elements in the HTML namespace with no src attribute count, because a browser ignores the body when src is present, even when it is empty. The body must also be non-blank. The type and language attributes are classified as in HTML's "prepare the script element": JavaScript MIME essences and an empty type count as classic, and module counts as module. Import maps, speculation rules and data blocks such as application/ld+json or text/template are not JavaScript and are not reported. Body ranges come from the body's text node, so an unclosed <script> at the end of the file still has a correct end.
Analysis: adds html_inline_scripts, with HTML path, script type and body range, next to html_scripts. Each HTML file with inline scripts adds one limitation naming the file and start lines.
Graph: each such HTML file gets one unknown scope through the existing addUnavailableStaticParseScope, with operation parse-html-inline-scripts and unavailable evidence. The scope is contained by the file node, and the structured inline_scripts list sits in its properties. A small optional details field on that helper carries the list. Coverage becomes partial, the same way other parse gaps are handled.
The gap is attached to the HTML file node, not to an Electron renderer role, so plain HTML pages with inline scripts are not newly classified as renderers.
Bounds (from review): every inline body is counted, but exact locations are kept for at most 100 per HTML file (HTML_INLINE_SCRIPT_LOCATION_LIMIT), under the html_inline_script_locations_per_file limit. The limitation text lists at most 10 starting lines.
Ranges (from review): script ranges come straight from the parse5 locations extraction already reads, not from rescanning the HTML for each offset. External script ranges use them too. Both HTML observation lists are appended in loops rather than by spreading.
Truncation (from review): when the per-file bound drops locations, the scope's evidence is truncated: true with the exact omitted count. Graph coverage is truncated: true with omitted_count: null, because the unanalyzed bodies hide an unknown number of facts.
Script types (from review): only a type attribute is whitespace-stripped, and only for the JavaScript MIME check, so type=" module " is a data block, as in HTML.
Docs: docs/javascript-artifact-reconstruction.md describes the new behaviour.

Not included: inline event-handler attributes (onclick), javascript: URLs and SVG <script> elements.

This is an ordinary fix within existing boundaries, so no maintainer scope decision is needed.

Contract and boundary impact

Semantic owner and earliest changed stage: HTML artifact reference extraction (src/domain/javascript/htmlArtifactReferences.ts). Analysis and graph code consume its facts without re-parsing the HTML.
Contribution scope fit and linked maintainer decision: not applicable (bug fix to static, read-only analysis).
CLI and MCP/tool-catalog contract: no schema change. Results gain a limitation, an unknown node per affected HTML file, and partial coverage where inline scripts exist.
Provider, bridge, target-format, or platform compatibility: none. The existing parse5 parser is used.
Evidence, artifact, provenance, or reconstruction contract: the omission is recorded as an explicit unknown with the HTML file's digest and exact body ranges. Nothing is derived from the bodies.
Target selection, authority, upstream behavior, and cleanup details: not applicable.
Generated metadata, package, or installation impact: none.
Read existing files or captured data.

Evidence and regression coverage

Tests added or updated:
- tests/boundary/filesystem/javascriptHtmlEntrypoints.test.ts: the #2834 Cordova fixture, through reconstructJavaScriptArtifact. It checks partial coverage, one parse-html-inline-scripts scope with both exact ranges and types, unavailable evidence carrying the HTML digest, the limitation text, and that the external cordova.js load is still projected. A second case checks that external scripts, an import map and a blank inline script keep coverage complete with no inline limitation.
- Review follow-up: a 5,000-body page checks the exact count, 100 retained locations, the named limit and the limitation text. Inline body ranges are compared with parse5 across the eight existing line-ending cases (LF, CRLF, bare CR, mixed, astral, U+2028). A domain test parses 150,000 bodies without a RangeError.
- src/domain/javascript/htmlArtifactReferences.test.ts: 20 classification cases (type and language variants, src and empty src, blank body, JSON, JSON-LD, import map, speculation rules, template, VBScript, comments, <template>, SVG), plus exact body offsets for closed and unclosed scripts.
Base reproduction: the new boundary test fails on origin/main, where coverage is complete and no gap is reported. The parser cases fail there too, because extraction returns no inlineScripts.
Remaining proof gaps: none known for the reported behaviour. Inline bodies are still unanalyzed by design.
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 -- <every test file that touches HTML, analyzeJavaScriptArtifactFiles, reconstructJavaScriptArtifact or analyze-javascript-application> (64 files): 820 passed after both review rounds.
npm run check:fast: typecheck, lint and module boundaries passed.
npm run docs:check: passed.
npm run verify:test-discovery: all 1112 test files discovered exactly once.
node scripts/rea.mjs analyze-javascript-application <#2834 fixture> --format json after npm run build:cached: output shown above.

Compatibility, safety, and release

Breaking changes or migration steps: none. Artifacts with inline HTML JavaScript now report partial coverage instead of complete, which is the intended correction.
Real Hopper/Ghidra, browser, or OS coverage: not applicable. Linux x64, Node 24.18.0.
Package or release metadata impact: none.
Security, privacy, process, or containment review: inline bodies are located but never parsed as JavaScript or executed.

Related: #2935 also targets this issue. It adds a limitation string only: graph coverage stays complete, the type attribute is not classified (so JSON-LD, import maps and template blocks would be reported as skipped JavaScript), and locations exist only inside the message text. This PR adds the structured unknown scope, partial coverage, spec-based type classification and a boundary test.

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/2940 **Author:** [@rileygramlich](https://github.com/rileygramlich) **Created:** 10/13/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/html-inline-scripts` --- ### 📝 Commits (3) - [`c97aca7`](https://github.com/morluto/rea/commit/c97aca7f3da2b68744144221881243daa74abf44) fix(javascript): report inline HTML scripts that analysis skips - [`c0b916e`](https://github.com/morluto/rea/commit/c0b916efac4d4d75943adae9ac288249f8c2ff7b) fix(javascript): bound inline script records and index HTML lines - [`80fe1ec`](https://github.com/morluto/rea/commit/80fe1ecfdf89b18ec958ccb927d7e260b02a5efc) fix(javascript): use parse5 coordinates and truncated inline coverage ### 📊 Changes **11 files changed** (+562 additions, -57 deletions) <details> <summary>View changed files</summary> 📝 `docs/javascript-artifact-reconstruction.md` (+8 -0) 📝 `src/application/javascript/JavaScriptArtifactAnalysis.ts` (+62 -33) 📝 `src/application/javascript/JavaScriptArtifactAnalysisTypes.ts` (+14 -0) 📝 `src/application/javascript/JavaScriptArtifactGraphBuilder.ts` (+28 -12) 📝 `src/application/javascript/JavaScriptArtifactGraphContext.ts` (+36 -2) 📝 `src/application/javascript/JavaScriptArtifactGraphStructure.ts` (+31 -0) 📝 `src/application/javascript/JavaScriptSemanticGraphResources.test.ts` (+1 -0) 📝 `src/domain/javascript/htmlArtifactReferences.test.ts` (+79 -4) 📝 `src/domain/javascript/htmlArtifactReferences.ts` (+120 -6) 📝 `tests/boundary/filesystem/javascriptHtmlEntrypoints.test.ts` (+182 -0) 📝 `tests/fixtures/javascriptSemanticGraph.ts` (+1 -0) </details> ### 📄 Description ## Summary Fixes #2834. `analyze-javascript-application` now reports inline JavaScript in local HTML that it does not analyze. Each skipped body gets its HTML file, script type and exact source range, and graph coverage becomes `partial` instead of `complete`. ## Problem and expected behavior HTML extraction read only `<script src>` references. Inline classic and module bodies were never parsed, and no limitation mentioned them. In Capacitor and Cordova apps the app logic usually sits inline in `www/index.html`. For those apps the result covered only `cordova.js` and reported `complete`, while the inline endpoints, storage writes and imports were missing. With the fixture from #2834, the result is now: ```text coverage: partial limitations: www/index.html has 2 inline JavaScript bodies starting at lines 5, 9 that were not analyzed; endpoints, storage, imports and other facts in them are absent from the evidence. unknown parse-html-inline-scripts scope: inline_script_count: 2 inline_scripts: [{"script_type":"classic","location":{"start":{"line":5,"column":8},"end":{"line":8,"column":0}}}, {"script_type":"module","location":{"start":{"line":9,"column":22},"end":{"line":9,"column":67}}}] ``` ## Change and scope This is the issue's "at minimum" outcome: make the omission visible and structured. Analyzing the bodies needs an embedded-module identity in the evidence graph, which the issue suggests discussing first, so it is not included here. - **Extraction** (`htmlArtifactReferences`): also returns inline script bodies. Only `script` elements in the HTML namespace with no `src` attribute count, because a browser ignores the body when `src` is present, even when it is empty. The body must also be non-blank. The `type` and `language` attributes are classified as in HTML's "prepare the script element": JavaScript MIME essences and an empty type count as classic, and `module` counts as module. Import maps, speculation rules and data blocks such as `application/ld+json` or `text/template` are not JavaScript and are not reported. Body ranges come from the body's text node, so an unclosed `<script>` at the end of the file still has a correct end. - **Analysis**: adds `html_inline_scripts`, with HTML path, script type and body range, next to `html_scripts`. Each HTML file with inline scripts adds one limitation naming the file and start lines. - **Graph**: each such HTML file gets one `unknown` scope through the existing `addUnavailableStaticParseScope`, with operation `parse-html-inline-scripts` and `unavailable` evidence. The scope is contained by the file node, and the structured `inline_scripts` list sits in its properties. A small optional `details` field on that helper carries the list. Coverage becomes `partial`, the same way other parse gaps are handled. - The gap is attached to the HTML file node, not to an Electron renderer role, so plain HTML pages with inline scripts are not newly classified as renderers. - **Bounds** (from review): every inline body is counted, but exact locations are kept for at most 100 per HTML file (`HTML_INLINE_SCRIPT_LOCATION_LIMIT`), under the `html_inline_script_locations_per_file` limit. The limitation text lists at most 10 starting lines. - **Ranges** (from review): script ranges come straight from the parse5 locations extraction already reads, not from rescanning the HTML for each offset. External script ranges use them too. Both HTML observation lists are appended in loops rather than by spreading. - **Truncation** (from review): when the per-file bound drops locations, the scope's evidence is `truncated: true` with the exact omitted count. Graph coverage is `truncated: true` with `omitted_count: null`, because the unanalyzed bodies hide an unknown number of facts. - **Script types** (from review): only a `type` attribute is whitespace-stripped, and only for the JavaScript MIME check, so `type=" module "` is a data block, as in HTML. - **Docs**: `docs/javascript-artifact-reconstruction.md` describes the new behaviour. Not included: inline event-handler attributes (`onclick`), `javascript:` URLs and SVG `<script>` elements. This is an ordinary fix within existing boundaries, so no maintainer scope decision is needed. ## Contract and boundary impact - Semantic owner and earliest changed stage: HTML artifact reference extraction (`src/domain/javascript/htmlArtifactReferences.ts`). Analysis and graph code consume its facts without re-parsing the HTML. - Contribution scope fit and linked maintainer decision: not applicable (bug fix to static, read-only analysis). - CLI and MCP/tool-catalog contract: no schema change. Results gain a limitation, an `unknown` node per affected HTML file, and `partial` coverage where inline scripts exist. - Provider, bridge, target-format, or platform compatibility: none. The existing parse5 parser is used. - Evidence, artifact, provenance, or reconstruction contract: the omission is recorded as an explicit unknown with the HTML file's digest and exact body ranges. Nothing is derived from the bodies. - Target selection, authority, upstream behavior, and cleanup details: not applicable. - Generated metadata, package, or installation impact: none. - [x] Read existing files or captured data. ## Evidence and regression coverage - Tests added or updated: - `tests/boundary/filesystem/javascriptHtmlEntrypoints.test.ts`: the #2834 Cordova fixture, through `reconstructJavaScriptArtifact`. It checks `partial` coverage, one `parse-html-inline-scripts` scope with both exact ranges and types, `unavailable` evidence carrying the HTML digest, the limitation text, and that the external `cordova.js` load is still projected. A second case checks that external scripts, an import map and a blank inline script keep coverage `complete` with no inline limitation. - Review follow-up: a 5,000-body page checks the exact count, 100 retained locations, the named limit and the limitation text. Inline body ranges are compared with parse5 across the eight existing line-ending cases (LF, CRLF, bare CR, mixed, astral, U+2028). A domain test parses 150,000 bodies without a `RangeError`. - `src/domain/javascript/htmlArtifactReferences.test.ts`: 20 classification cases (type and language variants, `src` and empty `src`, blank body, JSON, JSON-LD, import map, speculation rules, template, VBScript, comments, `<template>`, SVG), plus exact body offsets for closed and unclosed scripts. - Base reproduction: the new boundary test fails on `origin/main`, where coverage is `complete` and no gap is reported. The parser cases fail there too, because extraction returns no `inlineScripts`. - Remaining proof gaps: none known for the reported behaviour. Inline bodies are still unanalyzed by design. - [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 -- <every test file that touches HTML, analyzeJavaScriptArtifactFiles, reconstructJavaScriptArtifact or analyze-javascript-application>` (64 files): 820 passed after both review rounds. - `npm run check:fast`: typecheck, lint and module boundaries passed. - `npm run docs:check`: passed. - `npm run verify:test-discovery`: all 1112 test files discovered exactly once. - `node scripts/rea.mjs analyze-javascript-application <#2834 fixture> --format json` after `npm run build:cached`: output shown above. ## Compatibility, safety, and release - Breaking changes or migration steps: none. Artifacts with inline HTML JavaScript now report `partial` coverage instead of `complete`, which is the intended correction. - Real Hopper/Ghidra, browser, or OS coverage: not applicable. Linux x64, Node 24.18.0. - Package or release metadata impact: none. - Security, privacy, process, or containment review: inline bodies are located but never parsed as JavaScript or executed. Related: #2935 also targets this issue. It adds a limitation string only: graph coverage stays `complete`, the `type` attribute is not classified (so JSON-LD, import maps and template blocks would be reported as skipped JavaScript), and locations exist only inside the message text. This PR adds the structured unknown scope, `partial` coverage, spec-based type classification and a boundary test. ## 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:07:09 +00:00

closed this issue
added the
pull-request
label

bsakaguchi referenced this issue

2026-11-01 21:44:39 +00:00

[PR #5633] [MERGED] fix(errors): name fixed allowances in resource-constraint recovery #7007

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#5505

No description provided.

Rows
Columns