[PR #4572] fix(reference): keep the raw query and fragment on unresolved ESM paths #6314

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

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/4572
Author: @Ganesh2325
Created: 10/16/2026
Status: 🔄 Open

Base: main ← Head: fix/esm-import-url-suffix


📝 Commits (10+)

  • fab189f fix(reference): resolve query and fragment ESM imports to their pathname
  • 557fac3 Merge remote-tracking branch 'upstream/main' into fix/esm-import-url-suffix
  • f30475e fix(reference): interpret ESM URL components before inventory lookup
  • 4fda585 Merge remote-tracking branch 'upstream/main' into fix/esm-import-url-suffix
  • 21ba350 test(reference): leave encoded backslash ESM pathnames unresolved
  • adf85a7 Merge remote-tracking branch 'upstream/main' into fix/esm-import-url-suffix
  • cf394be refactor(reference): parse ESM specifiers once in importLookupPath
  • 5f5812c Merge remote-tracking branch 'upstream/main' into fix/esm-import-url-suffix
  • 0c42328 fix(reference): keep query and fragment text in unresolved ESM paths
  • 9d5fce1 Merge remote-tracking branch 'upstream/main' into fix/esm-import-url-suffix

📊 Changes

2 files changed (+125 additions, -6 deletions)

View changed files

📝 src/application/ReferenceSourceImportEntries.test.ts (+97 -0)
📝 src/application/ReferenceSourceImportEntries.ts (+28 -6)

📄 Description

Summary

Follows up #3916

Main already fixed #3916 in f2e5fd148: a relative ESM import with a query or fragment resolves to its URL pathname, and the declared spelling is kept separately in source_specifier. This PR no longer claims to close #3916. It is narrowed, as requested in the maintainer's review, to one remaining representation choice: the to of an unresolved source-import relationship keeps the raw ?query or #fragment after main's normalized pathname.

Problem and expected behavior

  • Smallest trigger: main.mjs holds import "./missing.mjs?v=1"; and no missing.mjs is inventoried.
  • On main: {"from_path":"main.mjs","to":"missing.mjs","source_specifier":"./missing.mjs?v=1","kind":"imports","resolution":"unresolved"}.
  • With this PR: {"from_path":"main.mjs","to":"missing.mjs?v=1","source_specifier":"./missing.mjs?v=1","kind":"imports","resolution":"unresolved"}.
  • Rationale: an unresolved edge names something the inventory does not contain, so its to stays faithful to what the source requested instead of silently dropping the query or fragment. The pathname part is still main's normalized, root-relative pathname, and source_specifier is unchanged.
  • Unchanged: resolved imports (./dep.mjs?v=1 still resolves to dep.mjs with resolution: "internal"), rejected escapes such as %2F (main's behavior, the raw specifier as to), the unresolved package-main target, and all CommonJS require paths.

Change and scope

Net diff against main: 2 files, +125/−6. Only src/application/ReferenceSourceImportEntries.ts and its test file change. The branch merged upstream main at 269630f4e (merge commit 9d5fce1dd, no rebase), taking main's resolver and tests, so the merged tree matched main. Commit a7edd1709 then adds only the behavior below.

  • importLookupPath, main's existing ESM parser, now also returns suffix: the raw ?query or #fragment, marker included (L398-L399, L432). The cut is made on the raw specifier at the first ? or # (L403-L406), the same boundary URL.parse uses, rather than read back from URL.search/URL.hash, which can re-encode characters.
  • resolveInternalSpecifier adds a small unresolvedAt helper (L464-L471) that appends that suffix to main's normalized pathname. It replaces main's three { to: normalized, resolution: "unresolved" } returns (L471, L520, L526). The suffix is empty for require lookups and for rejected ESM pathnames, so those to values are exactly main's.
  • Not included: anything about resolving the pathname itself, percent decoding, %5C handling, or source_specifier. Those are main's behavior from f2e5fd148 and are untouched.

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.

Contribution scope: an ordinary change within existing boundaries. A maintainer scope decision does not apply. Whether unresolved to should keep the raw suffix is the open representation decision the maintainer asked this PR to isolate.

Contract and boundary impact

  • Semantic owner and earliest changed stage: src/application/ReferenceSourceImportEntries.ts, when an ESM source-import relationship is left unresolved.
  • Contribution scope fit and linked maintainer decision: within the existing reference-source boundary; see the maintainer comment. Not applicable otherwise.
  • CLI and MCP/tool-catalog contract: no schema or flag change. Only the to string of an unresolved ESM relationship whose specifier contains a raw ? or # changes.
  • Provider, bridge, target-format, or platform compatibility: TypeScript-file imports also use source-import mode and get the same unresolved spelling.
  • Evidence, artifact, provenance, or reconstruction contract: an unresolved edge records the normalized pathname plus the raw query or fragment; source_specifier still records the declared spelling. Resolved edges are unchanged.
  • 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: 4 cases in src/application/ReferenceSourceImportEntries.test.ts, which uses main's existing cases unchanged:
  • Base reproduction or other evidence: on main, the same unresolved imports produce to without the suffix (for example missing.mjs for ./missing.mjs?v=1). The resolved control documents main's existing behavior, which this PR keeps.
  • User-visible CLI/MCP output (if applicable): the unresolved relationship becomes {"from_path":"main.mjs","to":"missing.mjs?v=1","source_specifier":"./missing.mjs?v=1","kind":"imports","resolution":"unresolved"}. Captured at the entry-parsing boundary; no end-to-end CLI output is captured.
  • Remaining proof gaps:
    • No MCP or real-provider run.
    • The full suite and check:pr were not run locally; CI on head a7edd17 covers them (see Validation performed).
    • Windows was not checked.

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

Local commands ran on Node 22.23.4 against head a7edd17. Each command exited 0.

  • npx vitest run src/application/ReferenceSourceImportEntries.test.ts: 18 passed.
  • npx vitest run over the reference-source set (src/application/ReferenceSourceImportEntries.test.ts, src/domain/referenceSourcePathIdentity.test.ts, src/application/ReferenceSourceImport, src/domain/referenceSource, src/reference/ReferenceSource, tests/boundary/filesystem/referenceSource, tests/boundary/cli/referenceSource): 42 files passed; 213 tests passed and 2 skipped.
  • npm run lint: 0 errors.
  • npm run typecheck: ok.
  • npm run build: ok.
  • Pre-push check:fast (typecheck and lint): ok.

CI on head a7edd17 (run 40547689573) passed: CI required, Static checks, Build package, Source tests, Portable tests, all eight test shards, Native package baseline (linux x64), Build bundled Windows x64 native controls, Classify changes and Greptile Review succeeded, and Conventional PR title and Validate author declaration passed. Deep static checks, Documentation and the optional capability lanes were skipped as untouched.

Compatibility, safety, and release

  • Breaking changes or migration steps: none to the schema or the CLI. The to of an unresolved ESM relationship with a raw ? or # gains that suffix compared with main. Resolved edges, rejected escapes and require edges are unchanged.
  • Real Hopper/Ghidra, browser, or OS coverage: not applicable to Hopper, Ghidra or browsers. Linux only; Windows was not checked.
  • Package or release metadata impact: none.
  • Security, privacy, process, or containment review: pure string handling over already-read source text. No new filesystem access, process launches or network access. The suffix is only appended to an unresolved to; it never affects inventory lookup or which file an edge targets.

Author readiness

Human owner: @Ganesh2325

  • 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 change was prepared with AI assistance.


🔄 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/4572 **Author:** [@Ganesh2325](https://github.com/Ganesh2325) **Created:** 10/16/2026 **Status:** 🔄 Open **Base:** `main` ← **Head:** `fix/esm-import-url-suffix` --- ### 📝 Commits (10+) - [`fab189f`](https://github.com/morluto/rea/commit/fab189f8b153e600a4e0af5da9e0715238477758) fix(reference): resolve query and fragment ESM imports to their pathname - [`557fac3`](https://github.com/morluto/rea/commit/557fac308b579972b0e77c33cf34c93dddab3e9d) Merge remote-tracking branch 'upstream/main' into fix/esm-import-url-suffix - [`f30475e`](https://github.com/morluto/rea/commit/f30475ed0e73002583960e6ddb6c4f4b19669f31) fix(reference): interpret ESM URL components before inventory lookup - [`4fda585`](https://github.com/morluto/rea/commit/4fda585aa3a7c27f3a6b8236ae417a46308ded21) Merge remote-tracking branch 'upstream/main' into fix/esm-import-url-suffix - [`21ba350`](https://github.com/morluto/rea/commit/21ba350fac57c44ad2b3958d0d73ab605113efef) test(reference): leave encoded backslash ESM pathnames unresolved - [`adf85a7`](https://github.com/morluto/rea/commit/adf85a7ed3290f4c07f7151577ce1430ee399c65) Merge remote-tracking branch 'upstream/main' into fix/esm-import-url-suffix - [`cf394be`](https://github.com/morluto/rea/commit/cf394be39cbdc282e58c87377eb610d31b311f28) refactor(reference): parse ESM specifiers once in importLookupPath - [`5f5812c`](https://github.com/morluto/rea/commit/5f5812cba069ebb06df3b3f2465abce9759c03b8) Merge remote-tracking branch 'upstream/main' into fix/esm-import-url-suffix - [`0c42328`](https://github.com/morluto/rea/commit/0c423282a23c02749f684df712e0bd7185499c11) fix(reference): keep query and fragment text in unresolved ESM paths - [`9d5fce1`](https://github.com/morluto/rea/commit/9d5fce1ddbe06e5d1fa82b48989944fa1193b3ed) Merge remote-tracking branch 'upstream/main' into fix/esm-import-url-suffix ### 📊 Changes **2 files changed** (+125 additions, -6 deletions) <details> <summary>View changed files</summary> 📝 `src/application/ReferenceSourceImportEntries.test.ts` (+97 -0) 📝 `src/application/ReferenceSourceImportEntries.ts` (+28 -6) </details> ### 📄 Description ## Summary Follows up #3916 Main already fixed #3916 in `f2e5fd148`: a relative ESM import with a query or fragment resolves to its URL pathname, and the declared spelling is kept separately in `source_specifier`. This PR no longer claims to close #3916. It is narrowed, as requested in [the maintainer's review](https://github.com/morluto/rea/pull/4572#issuecomment-6427907872), to one remaining representation choice: the `to` of an **unresolved** `source-import` relationship keeps the raw `?query` or `#fragment` after main's normalized pathname. ## Problem and expected behavior - Smallest trigger: `main.mjs` holds `import "./missing.mjs?v=1";` and no `missing.mjs` is inventoried. - On main: `{"from_path":"main.mjs","to":"missing.mjs","source_specifier":"./missing.mjs?v=1","kind":"imports","resolution":"unresolved"}`. - With this PR: `{"from_path":"main.mjs","to":"missing.mjs?v=1","source_specifier":"./missing.mjs?v=1","kind":"imports","resolution":"unresolved"}`. - Rationale: an unresolved edge names something the inventory does not contain, so its `to` stays faithful to what the source requested instead of silently dropping the query or fragment. The pathname part is still main's normalized, root-relative pathname, and `source_specifier` is unchanged. - Unchanged: resolved imports (`./dep.mjs?v=1` still resolves to `dep.mjs` with `resolution: "internal"`), rejected escapes such as `%2F` (main's behavior, the raw specifier as `to`), the unresolved package-main target, and all CommonJS `require` paths. ## Change and scope Net diff against `main`: 2 files, +125/−6. Only `src/application/ReferenceSourceImportEntries.ts` and its test file change. The branch merged upstream `main` at `269630f4e` (merge commit `9d5fce1dd`, no rebase), taking main's resolver and tests, so the merged tree matched `main`. Commit `a7edd1709` then adds only the behavior below. - [`importLookupPath`](https://github.com/morluto/rea/blob/a7edd17093919857af72cf5a0ffbcb97ae75807e/src/application/ReferenceSourceImportEntries.ts#L391-L435), main's existing ESM parser, now also returns `suffix`: the raw `?query` or `#fragment`, marker included ([L398-L399](https://github.com/morluto/rea/blob/a7edd17093919857af72cf5a0ffbcb97ae75807e/src/application/ReferenceSourceImportEntries.ts#L398-L399), [L432](https://github.com/morluto/rea/blob/a7edd17093919857af72cf5a0ffbcb97ae75807e/src/application/ReferenceSourceImportEntries.ts#L432)). The cut is made on the raw specifier at the first `?` or `#` ([L403-L406](https://github.com/morluto/rea/blob/a7edd17093919857af72cf5a0ffbcb97ae75807e/src/application/ReferenceSourceImportEntries.ts#L403-L406)), the same boundary `URL.parse` uses, rather than read back from `URL.search`/`URL.hash`, which can re-encode characters. - [`resolveInternalSpecifier`](https://github.com/morluto/rea/blob/a7edd17093919857af72cf5a0ffbcb97ae75807e/src/application/ReferenceSourceImportEntries.ts#L437-L539) adds a small `unresolvedAt` helper ([L464-L471](https://github.com/morluto/rea/blob/a7edd17093919857af72cf5a0ffbcb97ae75807e/src/application/ReferenceSourceImportEntries.ts#L464-L471)) that appends that suffix to main's normalized pathname. It replaces main's three `{ to: normalized, resolution: "unresolved" }` returns ([L471](https://github.com/morluto/rea/blob/a7edd17093919857af72cf5a0ffbcb97ae75807e/src/application/ReferenceSourceImportEntries.ts#L471), [L520](https://github.com/morluto/rea/blob/a7edd17093919857af72cf5a0ffbcb97ae75807e/src/application/ReferenceSourceImportEntries.ts#L520), [L526](https://github.com/morluto/rea/blob/a7edd17093919857af72cf5a0ffbcb97ae75807e/src/application/ReferenceSourceImportEntries.ts#L526)). The suffix is empty for `require` lookups and for rejected ESM pathnames, so those `to` values are exactly main's. - Not included: anything about resolving the pathname itself, percent decoding, `%5C` handling, or `source_specifier`. Those are main's behavior from `f2e5fd148` and are untouched. 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. Contribution scope: an ordinary change within existing boundaries. A maintainer scope decision does not apply. Whether unresolved `to` should keep the raw suffix is the open representation decision the maintainer asked this PR to isolate. ## Contract and boundary impact - Semantic owner and earliest changed stage: `src/application/ReferenceSourceImportEntries.ts`, when an ESM `source-import` relationship is left unresolved. - Contribution scope fit and linked maintainer decision: within the existing reference-source boundary; see the [maintainer comment](https://github.com/morluto/rea/pull/4572#issuecomment-6427907872). Not applicable otherwise. - CLI and MCP/tool-catalog contract: no schema or flag change. Only the `to` string of an unresolved ESM relationship whose specifier contains a raw `?` or `#` changes. - Provider, bridge, target-format, or platform compatibility: TypeScript-file `import`s also use `source-import` mode and get the same unresolved spelling. - Evidence, artifact, provenance, or reconstruction contract: an unresolved edge records the normalized pathname plus the raw query or fragment; `source_specifier` still records the declared spelling. Resolved edges are unchanged. - 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: 4 cases in `src/application/ReferenceSourceImportEntries.test.ts`, which uses main's existing cases unchanged: - [`keeps the raw query on an unresolved ESM pathname`](https://github.com/morluto/rea/blob/a7edd17093919857af72cf5a0ffbcb97ae75807e/src/application/ReferenceSourceImportEntries.test.ts#L660-L684): `./missing.mjs?v=1` → `to: "missing.mjs?v=1"`, and `./missing.mjs?v=1/..` → `to: "missing.mjs?v=1/.."` (the suffix is not dot-normalized), each with the original `source_specifier`. - [`keeps the raw fragment on an unresolved ESM pathname`](https://github.com/morluto/rea/blob/a7edd17093919857af72cf5a0ffbcb97ae75807e/src/application/ReferenceSourceImportEntries.test.ts#L686-L698): `./missing.mjs#a/../b` → `to: "missing.mjs#a/../b"`. - [`keeps the raw query when an unresolved ESM import is in a subdirectory`](https://github.com/morluto/rea/blob/a7edd17093919857af72cf5a0ffbcb97ae75807e/src/application/ReferenceSourceImportEntries.test.ts#L700-L721): `pkg/main.mjs` importing `./missing.mjs?v=1/..` → `to: "pkg/missing.mjs?v=1/.."`, and `main.mjs` importing `./pkg/missing.mjs?v=1` → `to: "pkg/missing.mjs?v=1"`. - [`leaves a resolved ESM query on the inventoried pathname`](https://github.com/morluto/rea/blob/a7edd17093919857af72cf5a0ffbcb97ae75807e/src/application/ReferenceSourceImportEntries.test.ts#L723-L738): control; `./dep.mjs?v=1` with `dep.mjs` inventoried still resolves to `to: "dep.mjs"`, `resolution: "internal"`. - Base reproduction or other evidence: on `main`, the same unresolved imports produce `to` without the suffix (for example `missing.mjs` for `./missing.mjs?v=1`). The resolved control documents main's existing behavior, which this PR keeps. - User-visible CLI/MCP output (if applicable): the unresolved relationship becomes `{"from_path":"main.mjs","to":"missing.mjs?v=1","source_specifier":"./missing.mjs?v=1","kind":"imports","resolution":"unresolved"}`. Captured at the entry-parsing boundary; no end-to-end CLI output is captured. - Remaining proof gaps: - No MCP or real-provider run. - The full suite and `check:pr` were not run locally; CI on head `a7edd17` covers them (see Validation performed). - Windows was not checked. 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 Local commands ran on Node 22.23.4 against head `a7edd17`. Each command exited 0. - `npx vitest run src/application/ReferenceSourceImportEntries.test.ts`: 18 passed. - `npx vitest run` over the reference-source set (`src/application/ReferenceSourceImportEntries.test.ts`, `src/domain/referenceSourcePathIdentity.test.ts`, `src/application/ReferenceSourceImport`, `src/domain/referenceSource`, `src/reference/ReferenceSource`, `tests/boundary/filesystem/referenceSource`, `tests/boundary/cli/referenceSource`): 42 files passed; 213 tests passed and 2 skipped. - `npm run lint`: 0 errors. - `npm run typecheck`: ok. - `npm run build`: ok. - Pre-push `check:fast` (typecheck and lint): ok. CI on head `a7edd17` ([run 40547689573](https://github.com/morluto/rea/actions/runs/40547689573)) passed: `CI required`, `Static checks`, `Build package`, `Source tests`, `Portable tests`, all eight test shards, `Native package baseline (linux x64)`, `Build bundled Windows x64 native controls`, `Classify changes` and `Greptile Review` succeeded, and `Conventional PR title` and `Validate author declaration` passed. `Deep static checks`, `Documentation` and the optional capability lanes were skipped as untouched. ## Compatibility, safety, and release - Breaking changes or migration steps: none to the schema or the CLI. The `to` of an unresolved ESM relationship with a raw `?` or `#` gains that suffix compared with main. Resolved edges, rejected escapes and `require` edges are unchanged. - Real Hopper/Ghidra, browser, or OS coverage: not applicable to Hopper, Ghidra or browsers. Linux only; Windows was not checked. - Package or release metadata impact: none. - Security, privacy, process, or containment review: pure string handling over already-read source text. No new filesystem access, process launches or network access. The suffix is only appended to an unresolved `to`; it never affects inventory lookup or which file an edge targets. ## Author readiness Human owner: @Ganesh2325 - [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 - [ ] 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 change was prepared with AI assistance. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
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#6314
No description provided.