[PR #2831] [MERGED] fix(source-maps): accept omitted names during browser collection #5421

Closed
opened 2026-10-31 01:06:30 +00:00 by bsakaguchi · 0 comments
Owner

📋 Pull Request Information

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

Base: main ← Head: fix/source-map-optional-names


📝 Commits (1)

  • 8fcf996 fix(source-maps): accept omitted names during browser collection

📊 Changes

2 files changed (+98 additions, -5 deletions)

View changed files

📝 src/javascript/sourceMaps/SourceMapFormat.ts (+0 -5)
📝 tests/boundary/browser/webSourceMapFetcher.test.ts (+98 -0)

📄 Description

Summary

Fixes #2826.

Collect valid HTTP source maps that omit the optional names field, preserving their original-source artifacts and mappings. Previously, adding only "names": [] changed collection from invalid to included.

Problem and expected behavior

{"version":3,"sources":["original.js"],"sourcesContent":["export const answer = 42;"],"mappings":"AAAA"} contains no named mapping and is valid without names under ECMA-426 section 9. Browser collection should include it with one original source and one mapping while retaining the exact fetched text. On the base revision, collection instead returns invalid, no map artifact, zero originals and zero mappings.

Change and scope

Remove the browser-only mandatory-names guard in the shared source-map format owner. Its existing leaf schema already supplies [] for omission and validates present values; its decoded mapping owner continues validating name indices. This applies equally to ordinary maps and embedded indexed leaves, without adding a second normalization rule. No unrelated source-map policy changes are included.

This is a routine format compatibility fix within existing contribution scope; a separate maintainer scope decision is not applicable.

Contract and boundary impact

  • Semantic owner and earliest changed stage: SourceMapFormat.ts, structural validation after the collector receives a map.
  • Contribution scope fit and linked maintainer decision: existing source-map inspection; no new provider or execution capability.
  • CLI and MCP/tool-catalog contract: schemas and parameters are unchanged; valid omitted-names maps now contribute the existing artifact/source/mapping result fields.
  • Provider, bridge, target-format, or platform compatibility: version 3 regular/indexed source maps; no provider or bridge changes.
  • Evidence, artifact, provenance, or reconstruction contract: preserve exact fetched JSON, byte count, digest, resolved original sources, mapping coordinates, and recovered module edges. Invalid present names and out-of-range name indices remain invalid.
  • Target selection, authority, upstream behavior, and cleanup details: existing origin admission and HTTP resource cleanup are unchanged. The regression uses owned loopback fixtures and closes its servers.
  • Generated metadata, package, or installation impact: none; no contract or dependency changes.

Expected effects of the existing workflow:

  • 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.

The fix adds no new target interaction or network request; it changes admission of already-fetched map bytes.

Evidence and regression coverage

  • Tests added or updated: real-HTTP collector regressions for regular maps and indexed leaves with nonzero offsets; omitted names and explicit-empty controls; null, numeric, non-string-array values and a name-index reference into an omitted array remain rejected.
  • Base reproduction: at 9680da76c84a6833635dc7a5df607806d899f804, both new regular/indexed regressions fail because omission returns invalid; the other 73 tests in the file pass. After the fix, all 108 selected tests pass.
  • Observable result: the standalone production-collector HTTP reproduction changes {status:"invalid", artifactRetained:false, originalSources:0, mappings:0} to {status:"included", artifactRetained:true, originalSources:1, mappings:1}. The explicit-empty control remains included. This is executed collector evidence, not a full CLI/CDP journey.
  • Remaining proof gaps: no real Chrome capture/export run; the changed boundary is covered directly with real HTTP.

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/browser/webSourceMapFetcher.test.ts tests/boundary/browser/webSourceMapNestedCoordinates.test.ts src/javascript/sourceMaps/TraceSourceMap.test.ts — 108 tests passed.
  • Standalone issue reproduction through the compiled collector and real loopback HTTP — reproduced on the base and passed after the fix.
  • npm run check — attempted; blocked by unrelated base formatting in tests/fixtures/javaClassFile.ts and max-lines-per-function lint errors in tests/boundary/providers/ghidra/ghidraLauncher.test.ts, src/ghidra/GhidraFunctionValues.test.ts, and src/domain/webBundleAnalyzer.test.ts. The same formatting and lint failures were independently reproduced in a clean checkout of base 9680da7; those files are unchanged by this PR.
  • npm run typecheck, npm run knip, npm run verify:module-boundaries, npm run verify:architecture-guards, and npm run verify:test-discovery — passed; all 1,078 test files discovered exactly once.
  • node_modules/.bin/oxfmt --check src/javascript/sourceMaps/SourceMapFormat.ts tests/boundary/browser/webSourceMapFetcher.test.ts — passed.
  • node_modules/.bin/oxlint --deny-warnings src/javascript/sourceMaps/SourceMapFormat.ts tests/boundary/browser/webSourceMapFetcher.test.ts — zero errors or warnings.
  • git diff --check — passed.

Compatibility, safety, and release

  • Breaking changes or migration steps: none.
  • Real Hopper/Ghidra, browser, or OS coverage: Linux x64/WSL production HTTP boundary; full browser capture not run. Native analysis providers are not involved.
  • Package or release metadata impact: none.
  • Security, privacy, process, or containment review: existing allowed origins, fetch behavior, resource budgets, invalid-name validation, and server cleanup retained. The fixture JavaScript is parsed for dependency evidence and is not executed.

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.

Full-repository formatting/lint failures were independently reproduced on clean base 9680da7. After the separate passing checks above, the branch was pushed with HUSKY=0 for that push only.

Integration note

Related independent fix: #2829. Both PRs target main and fix separate issues. Their deletions touch adjacent validation blocks in SourceMapFormat.ts; a local three-way merge reports a textual conflict between them. Once either merges, the other needs a small rebase retaining both removals. There is no semantic dependency, and the regression additions merge cleanly.


🔄 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/2831 **Author:** [@shoyann](https://github.com/shoyann) **Created:** 10/13/2026 **Status:** ✅ Merged **Merged:** 10/13/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `fix/source-map-optional-names` --- ### 📝 Commits (1) - [`8fcf996`](https://github.com/morluto/rea/commit/8fcf996b16d1566475db6851634332bc260e007e) fix(source-maps): accept omitted names during browser collection ### 📊 Changes **2 files changed** (+98 additions, -5 deletions) <details> <summary>View changed files</summary> 📝 `src/javascript/sourceMaps/SourceMapFormat.ts` (+0 -5) 📝 `tests/boundary/browser/webSourceMapFetcher.test.ts` (+98 -0) </details> ### 📄 Description ## Summary Fixes #2826. Collect valid HTTP source maps that omit the optional `names` field, preserving their original-source artifacts and mappings. Previously, adding only `"names": []` changed collection from `invalid` to `included`. ## Problem and expected behavior `{"version":3,"sources":["original.js"],"sourcesContent":["export const answer = 42;"],"mappings":"AAAA"}` contains no named mapping and is valid without `names` under [ECMA-426 section 9](https://tc39.es/ecma426/#sec-source-map-format). Browser collection should include it with one original source and one mapping while retaining the exact fetched text. On the base revision, collection instead returns `invalid`, no map artifact, zero originals and zero mappings. ## Change and scope Remove the browser-only mandatory-`names` guard in the shared source-map format owner. Its existing leaf schema already supplies `[]` for omission and validates present values; its decoded mapping owner continues validating name indices. This applies equally to ordinary maps and embedded indexed leaves, without adding a second normalization rule. No unrelated source-map policy changes are included. This is a routine format compatibility fix within existing [contribution scope](https://github.com/morluto/rea/blob/main/CONTRIBUTING.md#contribution-scope); a separate maintainer scope decision is not applicable. ## Contract and boundary impact - Semantic owner and earliest changed stage: `SourceMapFormat.ts`, structural validation after the collector receives a map. - Contribution scope fit and linked maintainer decision: existing source-map inspection; no new provider or execution capability. - CLI and MCP/tool-catalog contract: schemas and parameters are unchanged; valid omitted-`names` maps now contribute the existing artifact/source/mapping result fields. - Provider, bridge, target-format, or platform compatibility: version 3 regular/indexed source maps; no provider or bridge changes. - Evidence, artifact, provenance, or reconstruction contract: preserve exact fetched JSON, byte count, digest, resolved original sources, mapping coordinates, and recovered module edges. Invalid present `names` and out-of-range name indices remain invalid. - Target selection, authority, upstream behavior, and cleanup details: existing origin admission and HTTP resource cleanup are unchanged. The regression uses owned loopback fixtures and closes its servers. - Generated metadata, package, or installation impact: none; no contract or dependency changes. Expected effects of the existing workflow: - [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. - [x] 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. The fix adds no new target interaction or network request; it changes admission of already-fetched map bytes. ## Evidence and regression coverage - Tests added or updated: real-HTTP collector regressions for regular maps and indexed leaves with nonzero offsets; omitted names and explicit-empty controls; null, numeric, non-string-array values and a name-index reference into an omitted array remain rejected. - Base reproduction: at `9680da76c84a6833635dc7a5df607806d899f804`, both new regular/indexed regressions fail because omission returns `invalid`; the other 73 tests in the file pass. After the fix, all 108 selected tests pass. - Observable result: the standalone production-collector HTTP reproduction changes `{status:"invalid", artifactRetained:false, originalSources:0, mappings:0}` to `{status:"included", artifactRetained:true, originalSources:1, mappings:1}`. The explicit-empty control remains included. This is executed collector evidence, not a full CLI/CDP journey. - Remaining proof gaps: no real Chrome capture/export run; the changed boundary is covered directly with real HTTP. 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/browser/webSourceMapFetcher.test.ts tests/boundary/browser/webSourceMapNestedCoordinates.test.ts src/javascript/sourceMaps/TraceSourceMap.test.ts` — 108 tests passed. - Standalone issue reproduction through the compiled collector and real loopback HTTP — reproduced on the base and passed after the fix. - `npm run check` — attempted; blocked by unrelated base formatting in `tests/fixtures/javaClassFile.ts` and max-lines-per-function lint errors in `tests/boundary/providers/ghidra/ghidraLauncher.test.ts`, `src/ghidra/GhidraFunctionValues.test.ts`, and `src/domain/webBundleAnalyzer.test.ts`. The same formatting and lint failures were independently reproduced in a clean checkout of base `9680da7`; those files are unchanged by this PR. - `npm run typecheck`, `npm run knip`, `npm run verify:module-boundaries`, `npm run verify:architecture-guards`, and `npm run verify:test-discovery` — passed; all 1,078 test files discovered exactly once. - `node_modules/.bin/oxfmt --check src/javascript/sourceMaps/SourceMapFormat.ts tests/boundary/browser/webSourceMapFetcher.test.ts` — passed. - `node_modules/.bin/oxlint --deny-warnings src/javascript/sourceMaps/SourceMapFormat.ts tests/boundary/browser/webSourceMapFetcher.test.ts` — zero errors or warnings. - `git diff --check` — passed. ## Compatibility, safety, and release - Breaking changes or migration steps: none. - Real Hopper/Ghidra, browser, or OS coverage: Linux x64/WSL production HTTP boundary; full browser capture not run. Native analysis providers are not involved. - Package or release metadata impact: none. - Security, privacy, process, or containment review: existing allowed origins, fetch behavior, resource budgets, invalid-name validation, and server cleanup retained. The fixture JavaScript is parsed for dependency evidence and is not executed. ## 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. Full-repository formatting/lint failures were independently reproduced on clean base `9680da7`. After the separate passing checks above, the branch was pushed with `HUSKY=0` for that push only. ## Integration note Related independent fix: #2829. Both PRs target `main` and fix separate issues. Their deletions touch adjacent validation blocks in `SourceMapFormat.ts`; a local three-way merge reports a textual conflict between them. Once either merges, the other needs a small rebase retaining both removals. There is no semantic dependency, and the regression additions merge cleanly. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
bsakaguchi 2026-10-31 01:06:30 +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#5421
No description provided.