[PR #1770] [MERGED] fix(reference): report sloppy scripts, PHP, C# and enclosing Git facts #4807

New issue

Closed

opened 2026-10-31 00:55:16 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 00:55:16 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/1770
Author: @flitzrrr
Created: 10/11/2026
Status: ✅ Merged
Merged: 10/12/2026
Merged by: @baron

Base: main ← Head: fix/reference-source-import-facts

📝 Commits (5)

c4b639c fix(reference): report sloppy scripts, PHP, C# and enclosing Git facts
641732e fix(reference): keep module strictness and classify C# and PHP as source
d4169d8 fix(reference): honor package module scope and keep unknown-mode strict errors
6598d42 fix(reference): bound package scopes and keep application parsing unchanged
a4ff51b refactor(reference): keep unmarked module modes unknown instead of resolving package scope

📊 Changes

9 files changed (+231 additions, -44 deletions)

View changed files

📝 docs/cli.md (+3 -1)
📝 src/application/ReferenceSourceImportEntries.ts (+9 -5)
📝 src/application/ReferenceSourceVcsAdapter.ts (+17 -11)
📝 src/cli/utilityCommands.ts (+1 -1)
📝 src/domain/javascript/javascriptSourceParser.ts (+62 -1)
📝 src/domain/referenceSourceClassification.ts (+4 -0)
📝 src/domain/referenceSourceImportParsing.ts (+37 -13)
📝 src/domain/referenceSourceRequireBindings.ts (+8 -12)
➕ tests/boundary/cli/referenceSourceLegacyFacts.test.ts (+90 -0)

📄 Description

Summary

import-reference-source records wrong or missing facts for an ordinary legacy subtree:

A valid sloppy-mode script is reported as a parse failure.
.php and .cs files have no language.
A directory inside a Git checkout is reported as having no version control.
importer_version is null.

This PR corrects all four at their owning boundaries. They are kept together because they are all facts of one import result.

I did not open a separate issue because the PR carries the full reproduction below.

Problem and expected behavior

Reproduction: a Git repository whose committed source/ directory contains legacy.js (var messages = {};\ndelete messages;\n), index.php and Program.cs. Then run rea import-reference-source <repo>/source --format json --filter-output languages,parse_failures,vcs,provenance.

Before (062723e4):

{"languages":["JavaScript"],"parse_failures":[{"path":"legacy.js","parser":"babel","reason":"Deleting local variable in strict mode. (2:0)"}],"vcs":{"kind":"none","head":null,"dirty":null},"provenance":{"importer":"rea-agents","importer_version":null,"caller":"rea-cli"}}

After:

{"languages":["C#","JavaScript","PHP"],"parse_failures":[],"vcs":{"kind":"git","head":"45763e758c7b0d62b7b7382650afffd515563b90","dirty":null},"provenance":{"importer":"rea-agents","importer_version":"6.4.0","caller":"rea-cli"}}

Causes:

Babel's sourceType: "unambiguous" parses as a module first. With errorRecovery, it keeps those strict-mode errors after falling back to program.sourceType === "script".
LANGUAGE_BY_EXTENSION had no php or cs entry.
ReferenceSourceVcsAdapter only checked <root>/.git. A missing entry there was reported as kind: "none", a wrong absence claim for a subtree of a checkout.
The CLI passed importerVersion: null although PRODUCT_IDENTITY.packageVersion is known.

Change and scope

parseRecoveredJavaScript in javascriptSourceParser.ts parses reference sources by extension: .mjs/.mts as modules, .cjs as scripts. Other sources fall back to a script when Babel settles on one, and errors only the module parse reports become an entry limitation. The application pipeline (parseJavaScriptSource) is unchanged.
php maps to PHP and cs to C#. This is detection only: neither language is parsed for imports.
The VCS adapter walks up to the nearest ancestor with a .git entry, as Git does for a subdirectory, and reads HEAD through the existing reader. It reports none only when no ancestor has one, and unknown on any other lookup error. dirty stays null, so whether the subtree matches HEAD remains unknown. docs/cli.md documents this.
The CLI records the package version as importer_version.

Not included:

.gitignore files above the selected root are still not applied to exclusions.
csproj and sln are not classified as manifests.
webBundleAnalyzerInspection keeps its own parse, because it does not report parser errors.

Contract and boundary impact

Semantic owner and earliest changed stage:
- JavaScript parser boundary (src/domain/javascript/javascriptSourceParser.ts)
- reference-source classification (src/domain/referenceSourceClassification.ts)
- VCS adapter (src/application/ReferenceSourceVcsAdapter.ts)
- CLI composition (src/cli/utilityCommands.ts)
CLI and MCP/tool-catalog contract: no schema changes; languages, parse_failures, vcs and provenance.importer_version values change as shown.
Provider, bridge, target-format, or platform compatibility: none.
Evidence, artifact, provenance, or reconstruction contract: fewer false parse failures and false parser-recovered semantic coverage. VCS identity is now reported for subtrees of a checkout.
Process execution, authorization, cleanup, or containment impact: VCS discovery now checks ancestor directories for .git and may read the enclosing repository's HEAD and refs outside the selected root, through the existing read path without a subprocess. This is documented in docs/cli.md.
Generated metadata (docs/public/product-catalog.json), package, or installation impact: none.

Evidence and regression coverage

Tests added or updated:
- tests/boundary/cli/referenceSourceLegacyFacts.test.ts: the compiled CLI on the reproduction above, asserting all four facts.
Base reproduction: the CLI test fails on 062723e4 at parse_failures and passes with this change.
User-visible CLI/MCP output: shown above.
Remaining proof gaps: Git ceiling directories and filesystem-boundary discovery rules are not modelled; discovery stops at the first ancestor with a .git entry. No Windows run (the import is unsupported on native Windows).

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

On macOS (Darwin 27.0, Apple Silicon), Node 24.18.0:

npm run test:fast — 375 files, 4270 passed, 7 skipped
npm run test:focused -- tests/boundary/filesystem/referenceSource*.test.ts tests/boundary/cli/referenceSource*.test.ts — 19 files, 69 passed, 2 skipped
npm run check — passed
npm run docs:check — passed
npm run verify:test-discovery — 814 test files discovered exactly once
npm run build:cached and the CLI run above — passed

Review follow-up

641732e5: .mjs/.mts parse as modules through one parser-owned rule; .cs/.php are classified as source.
d4169d85 / 6598d42f / a4ff51b3: unmarked sources (everything except .mjs/.mts/.cjs) keep an unknown mode and are not forced to script; module-only strict errors become an entry limitation. .cjs parses as a script. Package-scope resolution was tried and removed again, because it kept pulling in more of Node's lookup rules; this matches the require-binding analysis, which also treats unmarked paths as unknown. The application pipeline parses as on main.
The CLI test covers explicit modules, .cjs, unknown mode and classification; it fails on the previous head.
npm run test:fast — 4270 passed; reference-source boundary — 69 passed; npm run check:fast — passed.

Compatibility, safety, and release

Breaking changes or migration steps: none. Consumers that treated vcs.kind: "none" as authoritative for subdirectories now receive the enclosing repository's HEAD.
Real Hopper/Ghidra, browser, or OS coverage: not applicable; macOS only.
Package or release metadata impact: none.
Security, privacy, process, or containment review: the only new file access is read-only Git metadata of the nearest enclosing repository, as described above.

Review checklist

This PR addresses a concrete problem or an agreed enhancement.
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.

🤖 Generated with Claude Code

_{🔄 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/1770 **Author:** [@flitzrrr](https://github.com/flitzrrr) **Created:** 10/11/2026 **Status:** ✅ Merged **Merged:** 10/12/2026 **Merged by:** [@baron](https://github.com/baron) **Base:** `main` ← **Head:** `fix/reference-source-import-facts` --- ### 📝 Commits (5) - [`c4b639c`](https://github.com/morluto/rea/commit/c4b639c2e31a7587ed7571843c803b06bf866bfe) fix(reference): report sloppy scripts, PHP, C# and enclosing Git facts - [`641732e`](https://github.com/morluto/rea/commit/641732e58b3d4efdd598f801993a080038dc6574) fix(reference): keep module strictness and classify C# and PHP as source - [`d4169d8`](https://github.com/morluto/rea/commit/d4169d85ce657fa8a4c5c03ff825e0a925eac615) fix(reference): honor package module scope and keep unknown-mode strict errors - [`6598d42`](https://github.com/morluto/rea/commit/6598d42f5cd86a7aba3ca87cfb21d64ba752df9e) fix(reference): bound package scopes and keep application parsing unchanged - [`a4ff51b`](https://github.com/morluto/rea/commit/a4ff51b35e628c378d01ccebf42157c44586b849) refactor(reference): keep unmarked module modes unknown instead of resolving package scope ### 📊 Changes **9 files changed** (+231 additions, -44 deletions) <details> <summary>View changed files</summary> 📝 `docs/cli.md` (+3 -1) 📝 `src/application/ReferenceSourceImportEntries.ts` (+9 -5) 📝 `src/application/ReferenceSourceVcsAdapter.ts` (+17 -11) 📝 `src/cli/utilityCommands.ts` (+1 -1) 📝 `src/domain/javascript/javascriptSourceParser.ts` (+62 -1) 📝 `src/domain/referenceSourceClassification.ts` (+4 -0) 📝 `src/domain/referenceSourceImportParsing.ts` (+37 -13) 📝 `src/domain/referenceSourceRequireBindings.ts` (+8 -12) ➕ `tests/boundary/cli/referenceSourceLegacyFacts.test.ts` (+90 -0) </details> ### 📄 Description ## Summary `import-reference-source` records wrong or missing facts for an ordinary legacy subtree: - A valid sloppy-mode script is reported as a parse failure. - `.php` and `.cs` files have no language. - A directory inside a Git checkout is reported as having no version control. - `importer_version` is `null`. This PR corrects all four at their owning boundaries. They are kept together because they are all facts of one import result. I did not open a separate issue because the PR carries the full reproduction below. ## Problem and expected behavior Reproduction: a Git repository whose committed `source/` directory contains `legacy.js` (`var messages = {};\ndelete messages;\n`), `index.php` and `Program.cs`. Then run `rea import-reference-source <repo>/source --format json --filter-output languages,parse_failures,vcs,provenance`. Before (062723e4): ```json {"languages":["JavaScript"],"parse_failures":[{"path":"legacy.js","parser":"babel","reason":"Deleting local variable in strict mode. (2:0)"}],"vcs":{"kind":"none","head":null,"dirty":null},"provenance":{"importer":"rea-agents","importer_version":null,"caller":"rea-cli"}} ``` After: ```json {"languages":["C#","JavaScript","PHP"],"parse_failures":[],"vcs":{"kind":"git","head":"45763e758c7b0d62b7b7382650afffd515563b90","dirty":null},"provenance":{"importer":"rea-agents","importer_version":"6.4.0","caller":"rea-cli"}} ``` Causes: 1. Babel's `sourceType: "unambiguous"` parses as a module first. With `errorRecovery`, it keeps those strict-mode errors after falling back to `program.sourceType === "script"`. 2. `LANGUAGE_BY_EXTENSION` had no `php` or `cs` entry. 3. `ReferenceSourceVcsAdapter` only checked `<root>/.git`. A missing entry there was reported as `kind: "none"`, a wrong absence claim for a subtree of a checkout. 4. The CLI passed `importerVersion: null` although `PRODUCT_IDENTITY.packageVersion` is known. ## Change and scope 1. `parseRecoveredJavaScript` in `javascriptSourceParser.ts` parses reference sources by extension: `.mjs`/`.mts` as modules, `.cjs` as scripts. Other sources fall back to a script when Babel settles on one, and errors only the module parse reports become an entry limitation. The application pipeline (`parseJavaScriptSource`) is unchanged. 2. `php` maps to `PHP` and `cs` to `C#`. This is detection only: neither language is parsed for imports. 3. The VCS adapter walks up to the nearest ancestor with a `.git` entry, as Git does for a subdirectory, and reads HEAD through the existing reader. It reports `none` only when no ancestor has one, and `unknown` on any other lookup error. `dirty` stays `null`, so whether the subtree matches HEAD remains unknown. `docs/cli.md` documents this. 4. The CLI records the package version as `importer_version`. Not included: - `.gitignore` files above the selected root are still not applied to exclusions. - `csproj` and `sln` are not classified as manifests. - `webBundleAnalyzerInspection` keeps its own parse, because it does not report parser errors. ## Contract and boundary impact - Semantic owner and earliest changed stage: - JavaScript parser boundary (`src/domain/javascript/javascriptSourceParser.ts`) - reference-source classification (`src/domain/referenceSourceClassification.ts`) - VCS adapter (`src/application/ReferenceSourceVcsAdapter.ts`) - CLI composition (`src/cli/utilityCommands.ts`) - CLI and MCP/tool-catalog contract: no schema changes; `languages`, `parse_failures`, `vcs` and `provenance.importer_version` values change as shown. - Provider, bridge, target-format, or platform compatibility: none. - Evidence, artifact, provenance, or reconstruction contract: fewer false parse failures and false parser-recovered semantic coverage. VCS identity is now reported for subtrees of a checkout. - Process execution, authorization, cleanup, or containment impact: VCS discovery now checks ancestor directories for `.git` and may read the enclosing repository's HEAD and refs outside the selected root, through the existing read path without a subprocess. This is documented in `docs/cli.md`. - Generated metadata (`docs/public/product-catalog.json`), package, or installation impact: none. ## Evidence and regression coverage - Tests added or updated: - `tests/boundary/cli/referenceSourceLegacyFacts.test.ts`: the compiled CLI on the reproduction above, asserting all four facts. - Base reproduction: the CLI test fails on 062723e4 at `parse_failures` and passes with this change. - User-visible CLI/MCP output: shown above. - Remaining proof gaps: Git ceiling directories and filesystem-boundary discovery rules are not modelled; discovery stops at the first ancestor with a `.git` entry. No Windows run (the import is unsupported on native Windows). 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 On macOS (Darwin 27.0, Apple Silicon), Node 24.18.0: - `npm run test:fast` — 375 files, 4270 passed, 7 skipped - `npm run test:focused -- tests/boundary/filesystem/referenceSource*.test.ts tests/boundary/cli/referenceSource*.test.ts` — 19 files, 69 passed, 2 skipped - `npm run check` — passed - `npm run docs:check` — passed - `npm run verify:test-discovery` — 814 test files discovered exactly once - `npm run build:cached` and the CLI run above — passed ### Review follow-up - 641732e5: `.mjs`/`.mts` parse as modules through one parser-owned rule; `.cs`/`.php` are classified as source. - d4169d85 / 6598d42f / a4ff51b3: unmarked sources (everything except `.mjs`/`.mts`/`.cjs`) keep an unknown mode and are not forced to script; module-only strict errors become an entry limitation. `.cjs` parses as a script. Package-scope resolution was tried and removed again, because it kept pulling in more of Node's lookup rules; this matches the require-binding analysis, which also treats unmarked paths as unknown. The application pipeline parses as on main. - The CLI test covers explicit modules, `.cjs`, unknown mode and classification; it fails on the previous head. - `npm run test:fast` — 4270 passed; reference-source boundary — 69 passed; `npm run check:fast` — passed. ## Compatibility, safety, and release - Breaking changes or migration steps: none. Consumers that treated `vcs.kind: "none"` as authoritative for subdirectories now receive the enclosing repository's HEAD. - Real Hopper/Ghidra, browser, or OS coverage: not applicable; macOS only. - Package or release metadata impact: none. - Security, privacy, process, or containment review: the only new file access is read-only Git metadata of the nearest enclosing repository, as described above. ## Review checklist - [x] This PR addresses a concrete problem or an agreed enhancement. - [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. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-31 00:55:16 +00:00