[PR #542] [MERGED] fix(inspector): preserve lossy Node discovery locations #2063

New issue

Closed

opened 2026-10-16 21:13:52 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-16 21:13:52 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/542
Author: @N0zoM1z0
Created: 10/5/2026
Status: ✅ Merged
Merged: 10/5/2026
Merged by: @N0zoM1z0

Base: main ← Head: fix/inspector-target-locations-530

📝 Commits (1)

7864c5a fix(inspector): preserve lossy Node discovery locations

📊 Changes

22 files changed (+654 additions, -106 deletions)

View changed files

📝 .github/workflows/ci.yml (+6 -2)
📝 docs/javascript-runtime-observation.md (+32 -0)
📝 docs/product-catalog.json (+2 -2)
📝 docs/testing.md (+7 -0)
📝 docs/verification/managed-conformance-manifest.json (+2 -2)
📝 scripts/verify-real-javascript-runtime-observation.mjs (+183 -74)
📝 skills/reverse-engineer-anything/SKILL.md (+1 -1)
📝 src/browser/JavaScriptRuntimeScope.ts (+44 -3)
📝 src/browser/V8InspectorCaptureProjection.ts (+7 -1)
📝 src/browser/V8InspectorEndpoint.ts (+2 -2)
📝 src/browser/V8InspectorProvider.ts (+14 -3)
📝 src/contracts/javascriptRuntimeObservationToolContracts.ts (+2 -2)
📝 src/domain/javascriptRuntimeObservation.ts (+17 -1)
📝 src/domain/javascriptRuntimeReconciliationMatching.ts (+8 -0)
📝 src/domain/javascriptRuntimeReconciliationParsing.ts (+15 -2)
📝 src/domain/javascriptRuntimeReconciliationResult.ts (+5 -3)
📝 src/domain/javascriptRuntimeReconciliationRuntime.ts (+19 -5)
📝 src/domain/javascriptRuntimeReconciliationSchemas.ts (+6 -1)
📝 src/generatedMcpToolCatalog.ts (+1 -1)
➕ tests/boundary/browser/javascriptRuntimeTargetLocations.test.ts (+195 -0)

...and 2 more files

📄 Description

Summary

Closes #530.

Node's Inspector discovery response is not a canonical file URL. It concatenates file:// with a pathname and replaces backslashes and double quotes with underscores; drive colons are retained. The issue's proposed underscore-to-path repair cannot reliably recover the original pathname and can identify a different existing file.

Keep Windows and other lossy Node discovery file locations eligible by the explicitly selected endpoint and exact target ID. Preserve the reported URL as unresolved, without guessing a filesystem path.
Interpret unambiguous POSIX Node discovery pathnames literally, including #, %, and encoded-looking %23 names. Apply this producer-specific behavior only to Node targets reporting a Node runtime product.
Validate loaded script URLs independently through the existing file boundary. Bare file://, remote file hosts, unsupported schemes, page targets, and non-Node products retain their existing validation.
Carry unresolved target locations through static/runtime reconciliation with unknown confidence and no inferred file match; verified loaded scripts still match independently.
Expand the real Inspector lane to cover production CLI and stdio MCP on Linux and Windows, retaining the existing ESM fixture and adding special-filename CommonJS fixtures.

Output example

The input remains the explicit Inspector endpoint and target ID. Discovery can now return this target location:

{
  "kind": "unresolved",
  "reported_url": "file://C:_tools_entry.js",
  "reason": "unverifiable-file-location"
}

Observation exposes the same target location and an explicit unknown; separately verified Debugger.scriptParsed entries keep their file locations. Reconciliation retains the unresolved object in runtime_captures[].target_location and reports runtime-location-unresolved. Canonical contracts and generated catalogs were regenerated together.

Verification

CI run: all checks passed, including both real Inspector jobs, the Windows capability/package lane, Linux/macOS package integration, and every test shard.

Fail-before regression: four genuine failures on the original implementation (Windows discovery, literal #, literal %, and an existing underscore alias).
Focused boundary, CLI/MCP, and reconciliation tests; the 13 new regressions passed on three repeated runs.
npm run docs:generate
npm run check:pr: 367 test files; 1,937 passed, 2 existing platform skips; typecheck, lint, formatting, dependency/catalog metadata and generated-document checks passed.
npm run verify:inspector: real owned Node processes through CLI and stdio MCP; 8 cases passed on Linux and 7 on Windows (ESM baseline plus CommonJS filenames; double quotes are not legal Windows filenames). Both real-platform CI jobs passed with Node v24.18.0.
npm run verify:package with command-local temporary storage outside the checkout and unrelated provider environment variables cleared.
npm pack --dry-run

No real Hopper/Ghidra verification was performed: this change affects the separate V8 Inspector provider. Development used the existing repository-local toolchain switch; host defaults were unchanged.

Upstream behavior: Node Inspector discovery construction, Node discovery escaping.

_{🔄 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/542 **Author:** [@N0zoM1z0](https://github.com/N0zoM1z0) **Created:** 10/5/2026 **Status:** ✅ Merged **Merged:** 10/5/2026 **Merged by:** [@N0zoM1z0](https://github.com/N0zoM1z0) **Base:** `main` ← **Head:** `fix/inspector-target-locations-530` --- ### 📝 Commits (1) - [`7864c5a`](https://github.com/morluto/rea/commit/7864c5a50e4e7b56a76c5665ac50a9db45823dcc) fix(inspector): preserve lossy Node discovery locations ### 📊 Changes **22 files changed** (+654 additions, -106 deletions) <details> <summary>View changed files</summary> 📝 `.github/workflows/ci.yml` (+6 -2) 📝 `docs/javascript-runtime-observation.md` (+32 -0) 📝 `docs/product-catalog.json` (+2 -2) 📝 `docs/testing.md` (+7 -0) 📝 `docs/verification/managed-conformance-manifest.json` (+2 -2) 📝 `scripts/verify-real-javascript-runtime-observation.mjs` (+183 -74) 📝 `skills/reverse-engineer-anything/SKILL.md` (+1 -1) 📝 `src/browser/JavaScriptRuntimeScope.ts` (+44 -3) 📝 `src/browser/V8InspectorCaptureProjection.ts` (+7 -1) 📝 `src/browser/V8InspectorEndpoint.ts` (+2 -2) 📝 `src/browser/V8InspectorProvider.ts` (+14 -3) 📝 `src/contracts/javascriptRuntimeObservationToolContracts.ts` (+2 -2) 📝 `src/domain/javascriptRuntimeObservation.ts` (+17 -1) 📝 `src/domain/javascriptRuntimeReconciliationMatching.ts` (+8 -0) 📝 `src/domain/javascriptRuntimeReconciliationParsing.ts` (+15 -2) 📝 `src/domain/javascriptRuntimeReconciliationResult.ts` (+5 -3) 📝 `src/domain/javascriptRuntimeReconciliationRuntime.ts` (+19 -5) 📝 `src/domain/javascriptRuntimeReconciliationSchemas.ts` (+6 -1) 📝 `src/generatedMcpToolCatalog.ts` (+1 -1) ➕ `tests/boundary/browser/javascriptRuntimeTargetLocations.test.ts` (+195 -0) _...and 2 more files_ </details> ### 📄 Description ## Summary Closes #530. Node's Inspector discovery response is not a canonical file URL. It concatenates `file://` with a pathname and replaces backslashes and double quotes with underscores; drive colons are retained. The issue's proposed underscore-to-path repair cannot reliably recover the original pathname and can identify a different existing file. - Keep Windows and other lossy Node discovery file locations eligible by the explicitly selected endpoint and exact target ID. Preserve the reported URL as `unresolved`, without guessing a filesystem path. - Interpret unambiguous POSIX Node discovery pathnames literally, including `#`, `%`, and encoded-looking `%23` names. Apply this producer-specific behavior only to Node targets reporting a Node runtime product. - Validate loaded script URLs independently through the existing file boundary. Bare `file://`, remote file hosts, unsupported schemes, page targets, and non-Node products retain their existing validation. - Carry unresolved target locations through static/runtime reconciliation with `unknown` confidence and no inferred file match; verified loaded scripts still match independently. - Expand the real Inspector lane to cover production CLI and stdio MCP on Linux and Windows, retaining the existing ESM fixture and adding special-filename CommonJS fixtures. ### Output example The input remains the explicit Inspector endpoint and target ID. Discovery can now return this target location: ```json { "kind": "unresolved", "reported_url": "file://C:_tools_entry.js", "reason": "unverifiable-file-location" } ``` Observation exposes the same target location and an explicit unknown; separately verified `Debugger.scriptParsed` entries keep their file locations. Reconciliation retains the unresolved object in `runtime_captures[].target_location` and reports `runtime-location-unresolved`. Canonical contracts and generated catalogs were regenerated together. ## Verification [CI run](https://github.com/morluto/rea/actions/runs/37286077594): all checks passed, including both real Inspector jobs, the Windows capability/package lane, Linux/macOS package integration, and every test shard. - Fail-before regression: four genuine failures on the original implementation (Windows discovery, literal `#`, literal `%`, and an existing underscore alias). - Focused boundary, CLI/MCP, and reconciliation tests; the 13 new regressions passed on three repeated runs. - `npm run docs:generate` - `npm run check:pr`: 367 test files; 1,937 passed, 2 existing platform skips; typecheck, lint, formatting, dependency/catalog metadata and generated-document checks passed. - `npm run verify:inspector`: real owned Node processes through CLI and stdio MCP; 8 cases passed on Linux and 7 on Windows (ESM baseline plus CommonJS filenames; double quotes are not legal Windows filenames). Both real-platform CI jobs passed with Node v24.18.0. - `npm run verify:package` with command-local temporary storage outside the checkout and unrelated provider environment variables cleared. - `npm pack --dry-run` No real Hopper/Ghidra verification was performed: this change affects the separate V8 Inspector provider. Development used the existing repository-local toolchain switch; host defaults were unchanged. Upstream behavior: [Node Inspector discovery construction](https://github.com/nodejs/node/blob/v24.18.0/src/inspector_io.cc), [Node discovery escaping](https://github.com/nodejs/node/blob/v24.18.0/src/inspector_socket_server.cc). --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-16 21:13:52 +00:00