morluto/rea

Fork 0

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

[PR #1332] [CLOSED] fix(javascript): distinguish prefix-only builtins from packages #2807

New issue

Closed

opened 2026-10-17 21:10:30 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-17 21:10:30 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/1332
Author: @ac1982
Created: 10/9/2026
Status: ❌ Closed

Base: main ← Head: fix/javascript-prefix-only-builtins

📝 Commits (1)

0c4fcf6 fix(javascript): distinguish prefix-only builtins from packages

📊 Changes

2 files changed (+68 additions, -9 deletions)

View changed files

📝 src/application/javascript/JavaScriptArtifactPathResolution.ts (+2 -9)
📝 src/application/javascript/JavaScriptModuleSpecifierPaths.test.ts (+66 -0)

📄 Description

Summary

Resolve installed packages named test, sqlite, and sea instead of classifying them as Node builtins. Their builtin counterparts require the node: prefix.

No linked issue: this is a reproduced correctness bug found while examining current package resolution. Searches of issues and open/closed PRs found no matching fix.

Problem and expected behavior

With node_modules/sqlite/index.js present, Node loads require("sqlite"), but REA reports an unresolved dependency. Stripping node: from every builtinModules entry incorrectly turns prefix-only modules into bare builtins.

Change and scope

Use Node's isBuiltin on the actual declared specifier. Existing scheme handling and unsupported package-subpath handling remain in place. Tests execute real Node import and require against temporary packages and inspect the public application analysis graph.

Contract and boundary impact

Semantic owner and earliest changed stage: application bare-module classification before artifact lookup.
CLI and MCP/tool-catalog contract: unchanged tools and schemas; shared analysis now retains the correct dependency target.
Provider, bridge, target-format, or platform compatibility: static JavaScript package resolution; verified on Linux arm64 with Node 24.18.0.
Evidence, artifact, provenance, or reconstruction contract: existing graph identities and source evidence; corrected resolved paths.
Process execution, authorization, cleanup, or containment impact: none in production; tests execute their own inert fixtures.
Generated metadata, package, or installation impact: none.

Evidence and regression coverage

Both new public-analysis tests fail on base d83cbd2a: Node loads the packages, but REA returns a null resolved path. Existing 49 cases pass.
Corrected cases cover import and require for all three names, with fs and node:test remaining external even alongside a local fs package.
Built CLI analyze-javascript-application <fixture> --json returns {"specifier":"sqlite","resolved_path":"node_modules/sqlite/index.js"} in the import edge.
Observed, derived, and inferred claims remain distinguishable.
Artifact identity and source provenance remain preserved.
Unsupported and uncertain outcomes remain visible.

Validation performed

Node 24.18.0 / npm 11.16.0:

npm run test:local -- src/application/javascript/JavaScriptModuleSpecifierPaths.test.ts src/application/javascript/JavaScriptArtifactPathResolution.test.ts src/application/javascript/JavaScriptApplicationService.test.ts — 100 passed.
npm run test:focused -- tests/boundary/javascript/packageConditionalExports.test.ts tests/boundary/filesystem/javascriptPackagePrecedence.test.ts — 57 passed.
npm run check — all six tasks passed, warnings and no errors.
Built CLI reproduction and git diff --check — passed.

Compatibility, safety, and release

No migration, dependency, metadata, or execution-authority changes. Real Hopper/Ghidra/browser verification and the complete repository test suite were not run; those providers are unchanged. Hosted CI is separate from local validation.

Review checklist

Concrete problem and one focused outcome with a conventional title.
Reason for no linked issue stated; duplicate search completed.
Observable behavior and builtin controls covered.
Existing contracts/docs remain accurate; no generated changes needed.
Representative CLI output included.
Final diff reviewed for unrelated changes, credentials, and unsupported claims.

Prepared with OpenAI Codex 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/1332 **Author:** [@ac1982](https://github.com/ac1982) **Created:** 10/9/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/javascript-prefix-only-builtins` --- ### 📝 Commits (1) - [`0c4fcf6`](https://github.com/morluto/rea/commit/0c4fcf685f879b29ea0ec236ac8bd101c69f1388) fix(javascript): distinguish prefix-only builtins from packages ### 📊 Changes **2 files changed** (+68 additions, -9 deletions) <details> <summary>View changed files</summary> 📝 `src/application/javascript/JavaScriptArtifactPathResolution.ts` (+2 -9) 📝 `src/application/javascript/JavaScriptModuleSpecifierPaths.test.ts` (+66 -0) </details> ### 📄 Description ## Summary Resolve installed packages named `test`, `sqlite`, and `sea` instead of classifying them as Node builtins. Their builtin counterparts require the `node:` prefix. No linked issue: this is a reproduced correctness bug found while examining current package resolution. Searches of issues and open/closed PRs found no matching fix. ## Problem and expected behavior With `node_modules/sqlite/index.js` present, Node loads `require("sqlite")`, but REA reports an unresolved dependency. Stripping `node:` from every `builtinModules` entry incorrectly turns prefix-only modules into bare builtins. ## Change and scope Use Node's `isBuiltin` on the actual declared specifier. Existing scheme handling and unsupported package-subpath handling remain in place. Tests execute real Node import and require against temporary packages and inspect the public application analysis graph. ## Contract and boundary impact - Semantic owner and earliest changed stage: application bare-module classification before artifact lookup. - CLI and MCP/tool-catalog contract: unchanged tools and schemas; shared analysis now retains the correct dependency target. - Provider, bridge, target-format, or platform compatibility: static JavaScript package resolution; verified on Linux arm64 with Node 24.18.0. - Evidence, artifact, provenance, or reconstruction contract: existing graph identities and source evidence; corrected resolved paths. - Process execution, authorization, cleanup, or containment impact: none in production; tests execute their own inert fixtures. - Generated metadata, package, or installation impact: none. ## Evidence and regression coverage - Both new public-analysis tests fail on base `d83cbd2a`: Node loads the packages, but REA returns a null resolved path. Existing 49 cases pass. - Corrected cases cover import and require for all three names, with `fs` and `node:test` remaining external even alongside a local fs package. - Built CLI `analyze-javascript-application <fixture> --json` returns `{"specifier":"sqlite","resolved_path":"node_modules/sqlite/index.js"}` in the import edge. - [x] Observed, derived, and inferred claims remain distinguishable. - [x] Artifact identity and source provenance remain preserved. - [x] Unsupported and uncertain outcomes remain visible. ## Validation performed Node 24.18.0 / npm 11.16.0: - `npm run test:local -- src/application/javascript/JavaScriptModuleSpecifierPaths.test.ts src/application/javascript/JavaScriptArtifactPathResolution.test.ts src/application/javascript/JavaScriptApplicationService.test.ts` — 100 passed. - `npm run test:focused -- tests/boundary/javascript/packageConditionalExports.test.ts tests/boundary/filesystem/javascriptPackagePrecedence.test.ts` — 57 passed. - `npm run check` — all six tasks passed, warnings and no errors. - Built CLI reproduction and `git diff --check` — passed. ## Compatibility, safety, and release No migration, dependency, metadata, or execution-authority changes. Real Hopper/Ghidra/browser verification and the complete repository test suite were not run; those providers are unchanged. Hosted CI is separate from local validation. ## Review checklist - [x] Concrete problem and one focused outcome with a conventional title. - [x] Reason for no linked issue stated; duplicate search completed. - [x] Observable behavior and builtin controls covered. - [x] Existing contracts/docs remain accurate; no generated changes needed. - [x] Representative CLI output included. - [x] Final diff reviewed for unrelated changes, credentials, and unsupported claims. Prepared with OpenAI Codex assistance. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-17 21:10:30 +00:00

closed this issue
added the
pull-request
label

bsakaguchi referenced this issue

2026-10-31 01:06:24 +00:00

[PR #2807] [MERGED] fix(sqlite): keep triggers whose declaration spells the table in another case #5409

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

No description provided.

Rows
Columns