morluto/rea

Fork 0

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

[PR #3117] [CLOSED] fix: stabilize reference tests and patch development qs #5627

New issue

Closed

opened 2026-10-31 01:08:06 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

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

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/3117
Author: @yogesh-43
Created: 10/14/2026
Status: ❌ Closed

Base: main ← Head: fix/upstream-reference-regressions-and-dev-audit

📝 Commits (1)

3d5b487 fix: stabilize reference tests and patch development qs

📊 Changes

5 files changed (+60 additions, -13 deletions)

View changed files

📝 licenses/npm-dependencies.txt (+4 -4)
📝 package-lock.json (+5 -4)
📝 package.json (+3 -0)
📝 tests/boundary/cli/referenceSourceSummary.test.ts (+39 -1)
📝 tests/boundary/filesystem/referenceSourceReader.test.ts (+9 -4)

📄 Description

Summary

Fix the reference-source exclusion test's dependence on filesystem enumeration order, add compiled CLI coverage for stable broken symlink targets, and patch the development-only qs dependency reported by npm audit.

Fixes #3114.

This branch is based on upstream main at f3534749e564421036ac445f536fc766b038208f. Upstream already corrected production ENOTDIR handling in 0f4c484be552a4d5e8e1d0fa072441bfa900c815; this PR adds regression coverage for that behavior and does not duplicate the production fix.

Problem and expected behavior

The test applies exclusions to normalized paths after no-follow type inspection expects callbacks in the order ignored, then kept. Node's filesystem enumeration does not guarantee that order. The same test failed on Linux when it received kept, then ignored, even though both entries were correctly inspected and the excluded directory was not traversed.

The assertion should verify exact invocation count and expected path/kind membership without requiring callback order. The resulting inventory must still contain only the retained file.

The earlier repository review also found two moderate npm audit findings through typed-rest-client -> qs 6.15.1 in Stryker's development toolchain. A null/undefined comma-array serialization crash was reproduced with that dependency. A scoped override to qs 6.16.0 fixes it without changing direct dependency versions or runtime dependencies.

Change and scope

tests/boundary/filesystem/referenceSourceReader.test.ts: assert exactly two expected exclusion checks using unordered membership; retain the assertion that only kept reaches the inventory. This preserves the original exclusion guarantee while removing the unsupported ordering requirement.
tests/boundary/cli/referenceSourceSummary.test.ts: launch the compiled CLI with two stable broken links: one through an absent directory (ENOENT), the other through a regular file (ENOTDIR). Parse the graph and verify the exact link-relative targets, target_state: "missing", and absence of false changed-link limitations.
package.json: override qs to 6.16.0 specifically within typed-rest-client.
package-lock.json: refresh the qs archive, integrity, and dependency metadata. Other locked package versions remain unchanged.
licenses/npm-dependencies.txt: regenerate the tracked inventory for the updated development dependency.

The symlink fixture can be reproduced from a built checkout on Linux without a native provider:

fixture_root=$(mktemp -d)
mkdir "$fixture_root/links"
printf 'export const value = 1;\n' > "$fixture_root/target.js"
ln -s ../target.js/child.js "$fixture_root/links/blocked.js"
node scripts/rea.mjs import-reference-source "$fixture_root" --json

The existing upstream behavior, now covered by the compiled CLI regression, retains ../target.js/child.js and marks it missing rather than claiming the unchanged link was replaced. Remove the temporary fixture after inspection.

Contract and boundary impact

Semantic owner and earliest changed stage: tests at the reference-source filesystem/compiled CLI boundaries, plus development dependency resolution. No production reference-source implementation changes.
Contribution scope fit and maintainer decision: routine regression and dependency maintenance within existing boundaries; no new provider or execution capability requires a separate scope decision.
CLI and MCP/tool-catalog contract: no schema, command, registration, or catalog changes. The regression verifies the existing graph contract.
Provider, bridge, target-format, or platform compatibility: no provider/bridge changes. Executed on Linux x86_64; the new CLI case follows its enclosing suite's Windows skip policy.
Evidence, artifact, provenance, or reconstruction contract: no representation changes; tests check preservation of the known target and accurate missing state.
Target selection, authority, upstream behavior, and cleanup: no added authority. Existing no-follow traversal, identity checks, cancellation, and cleanup remain unchanged. The regression uses owned temporary fixtures and CLI subprocesses.
Generated metadata/package impact: updated frozen lockfile and tracked license inventory. Runtime dependencies and direct dependency versions are unchanged. Ignored build/catalog outputs are not committed.

Expected effects of verification and dependency preparation:

Read existing files or captured data.
Launch owned test/CLI processes.
Run repository tests and development scripts on generated fixtures.
Update development dependency configuration and generated inventory.
Contact npm for installation/audit.
Recover protected code or work around protections.

These changes add no target mutation, injection, or production network workflow.

Evidence and regression coverage

Observed order-sensitive test failure on the earlier fork baseline: expected [["ignored", "directory"], ["kept", "file"]], received the reverse order. Source inspection confirmed the same assertion remained in current upstream before this patch.
The corrected test still proves exact entry membership/count and omission of the excluded directory.
The compiled CLI regression verifies both missing-target forms in a validated graph. Existing source-reader identity-change, cancellation, I/O, FIFO-race, and cleanup coverage was also run.
The vulnerable qs serialization crash was reproduced during the original review. The patched typed-rest-client handled null/undefined comma-array entries and preserved ordinary output across repeat, comma, brackets, and indices formats.

For evidence-bearing checks:

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

Executed again on this branch based on current upstream, using Node.js 24.18.0 and npm 11.16.0:

npm ci --no-audit --no-fund: passed with the updated frozen lockfile.
npm run check: passed type checking, lint, formatting, dependency-usage, metadata, module-boundary, and architecture checks; the prerequisite runtime build succeeded.
npm run docs:check: passed, including 5 documentation consistency tests and generated inventory validation.
npm run test:focused -- tests/boundary/filesystem/referenceSourceReader.test.ts tests/boundary/filesystem/referenceSourceImport.test.ts tests/boundary/filesystem/referenceSourceFailureDetails.test.ts tests/boundary/filesystem/referenceSourceCleanupOwnership.test.ts tests/boundary/filesystem/referenceSourceFifoRace.test.ts tests/boundary/cli/referenceSourceSummary.test.ts tests/boundary/cli/referenceSourceImportStatus.test.ts tests/boundary/cli/referenceSourceExclusions.test.ts: 8 files passed; 58 tests passed, 2 skipped.
npm run verify:test-discovery: all 1,242 test files discovered exactly once.
npm audit --json: zero vulnerabilities.
git diff --cached --check: passed.

The complete check:pr suite was not run on this upstream-based branch. The earlier full run on the old fork baseline was interrupted and is not claimed as a pass. CI and real native-provider/platform coverage remain outstanding.

Compatibility, safety, and release

Breaking changes/migrations: no application contract change. Use npm ci to refresh development dependencies.
Real Hopper/Ghidra, browser, or OS coverage: no real native-provider validation and no Windows/macOS execution; focused validation ran on Linux.
Package/release metadata: one development-only transitive update and its license inventory; no release version change.
Security/privacy/process review: the scoped override removes the recorded dependency findings. Production target interaction and resource ownership are unchanged.

Before merge

Concrete problems and regression gaps are described.
Scope and authority are resolved for these existing-boundary changes.
The title follows Conventional Commits and describes the final diff.
Related issue is linked.
Tests cover changed assertions and meaningful observable failure cases.
Generated dependency inventory is updated.
Representative CLI behavior is included.
Final diff checked for secrets, unrelated outputs, and unsupported claims.
Complete CI suite has passed.

_{🔄 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/3117 **Author:** [@yogesh-43](https://github.com/yogesh-43) **Created:** 10/14/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/upstream-reference-regressions-and-dev-audit` --- ### 📝 Commits (1) - [`3d5b487`](https://github.com/morluto/rea/commit/3d5b4873a7b6ccfcab8b26cbef55505847ed7f57) fix: stabilize reference tests and patch development qs ### 📊 Changes **5 files changed** (+60 additions, -13 deletions) <details> <summary>View changed files</summary> 📝 `licenses/npm-dependencies.txt` (+4 -4) 📝 `package-lock.json` (+5 -4) 📝 `package.json` (+3 -0) 📝 `tests/boundary/cli/referenceSourceSummary.test.ts` (+39 -1) 📝 `tests/boundary/filesystem/referenceSourceReader.test.ts` (+9 -4) </details> ### 📄 Description ## Summary Fix the reference-source exclusion test's dependence on filesystem enumeration order, add compiled CLI coverage for stable broken symlink targets, and patch the development-only `qs` dependency reported by npm audit. Fixes #3114. This branch is based on upstream `main` at `f3534749e564421036ac445f536fc766b038208f`. Upstream already corrected production `ENOTDIR` handling in `0f4c484be552a4d5e8e1d0fa072441bfa900c815`; this PR adds regression coverage for that behavior and does not duplicate the production fix. ## Problem and expected behavior The test `applies exclusions to normalized paths after no-follow type inspection` expects callbacks in the order `ignored`, then `kept`. Node's filesystem enumeration does not guarantee that order. The same test failed on Linux when it received `kept`, then `ignored`, even though both entries were correctly inspected and the excluded directory was not traversed. The assertion should verify exact invocation count and expected path/kind membership without requiring callback order. The resulting inventory must still contain only the retained file. The earlier repository review also found two moderate npm audit findings through `typed-rest-client` -> `qs` 6.15.1 in Stryker's development toolchain. A null/undefined comma-array serialization crash was reproduced with that dependency. A scoped override to `qs` 6.16.0 fixes it without changing direct dependency versions or runtime dependencies. ## Change and scope - `tests/boundary/filesystem/referenceSourceReader.test.ts`: assert exactly two expected exclusion checks using unordered membership; retain the assertion that only `kept` reaches the inventory. This preserves the original exclusion guarantee while removing the unsupported ordering requirement. - `tests/boundary/cli/referenceSourceSummary.test.ts`: launch the compiled CLI with two stable broken links: one through an absent directory (`ENOENT`), the other through a regular file (`ENOTDIR`). Parse the graph and verify the exact link-relative targets, `target_state: "missing"`, and absence of false changed-link limitations. - `package.json`: override `qs` to 6.16.0 specifically within `typed-rest-client`. - `package-lock.json`: refresh the `qs` archive, integrity, and dependency metadata. Other locked package versions remain unchanged. - `licenses/npm-dependencies.txt`: regenerate the tracked inventory for the updated development dependency. The symlink fixture can be reproduced from a built checkout on Linux without a native provider: ```bash fixture_root=$(mktemp -d) mkdir "$fixture_root/links" printf 'export const value = 1;\n' > "$fixture_root/target.js" ln -s ../target.js/child.js "$fixture_root/links/blocked.js" node scripts/rea.mjs import-reference-source "$fixture_root" --json ``` The existing upstream behavior, now covered by the compiled CLI regression, retains `../target.js/child.js` and marks it missing rather than claiming the unchanged link was replaced. Remove the temporary fixture after inspection. ## Contract and boundary impact - Semantic owner and earliest changed stage: tests at the reference-source filesystem/compiled CLI boundaries, plus development dependency resolution. No production reference-source implementation changes. - Contribution scope fit and maintainer decision: routine regression and dependency maintenance within existing boundaries; no new provider or execution capability requires a separate scope decision. - CLI and MCP/tool-catalog contract: no schema, command, registration, or catalog changes. The regression verifies the existing graph contract. - Provider, bridge, target-format, or platform compatibility: no provider/bridge changes. Executed on Linux x86_64; the new CLI case follows its enclosing suite's Windows skip policy. - Evidence, artifact, provenance, or reconstruction contract: no representation changes; tests check preservation of the known target and accurate missing state. - Target selection, authority, upstream behavior, and cleanup: no added authority. Existing no-follow traversal, identity checks, cancellation, and cleanup remain unchanged. The regression uses owned temporary fixtures and CLI subprocesses. - Generated metadata/package impact: updated frozen lockfile and tracked license inventory. Runtime dependencies and direct dependency versions are unchanged. Ignored build/catalog outputs are not committed. Expected effects of verification and dependency preparation: - [x] Read existing files or captured data. - [x] Launch owned test/CLI processes. - [x] Run repository tests and development scripts on generated fixtures. - [x] Update development dependency configuration and generated inventory. - [x] Contact npm for installation/audit. - [ ] Recover protected code or work around protections. These changes add no target mutation, injection, or production network workflow. ## Evidence and regression coverage - Observed order-sensitive test failure on the earlier fork baseline: expected `[["ignored", "directory"], ["kept", "file"]]`, received the reverse order. Source inspection confirmed the same assertion remained in current upstream before this patch. - The corrected test still proves exact entry membership/count and omission of the excluded directory. - The compiled CLI regression verifies both missing-target forms in a validated graph. Existing source-reader identity-change, cancellation, I/O, FIFO-race, and cleanup coverage was also run. - The vulnerable `qs` serialization crash was reproduced during the original review. The patched `typed-rest-client` handled null/undefined comma-array entries and preserved ordinary output across repeat, comma, brackets, and indices formats. For evidence-bearing checks: - [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 Executed again on this branch based on current upstream, using Node.js 24.18.0 and npm 11.16.0: - `npm ci --no-audit --no-fund`: passed with the updated frozen lockfile. - `npm run check`: passed type checking, lint, formatting, dependency-usage, metadata, module-boundary, and architecture checks; the prerequisite runtime build succeeded. - `npm run docs:check`: passed, including 5 documentation consistency tests and generated inventory validation. - `npm run test:focused -- tests/boundary/filesystem/referenceSourceReader.test.ts tests/boundary/filesystem/referenceSourceImport.test.ts tests/boundary/filesystem/referenceSourceFailureDetails.test.ts tests/boundary/filesystem/referenceSourceCleanupOwnership.test.ts tests/boundary/filesystem/referenceSourceFifoRace.test.ts tests/boundary/cli/referenceSourceSummary.test.ts tests/boundary/cli/referenceSourceImportStatus.test.ts tests/boundary/cli/referenceSourceExclusions.test.ts`: 8 files passed; **58 tests passed, 2 skipped**. - `npm run verify:test-discovery`: all **1,242** test files discovered exactly once. - `npm audit --json`: **zero vulnerabilities**. - `git diff --cached --check`: passed. The complete `check:pr` suite was not run on this upstream-based branch. The earlier full run on the old fork baseline was interrupted and is not claimed as a pass. CI and real native-provider/platform coverage remain outstanding. ## Compatibility, safety, and release - Breaking changes/migrations: no application contract change. Use `npm ci` to refresh development dependencies. - Real Hopper/Ghidra, browser, or OS coverage: no real native-provider validation and no Windows/macOS execution; focused validation ran on Linux. - Package/release metadata: one development-only transitive update and its license inventory; no release version change. - Security/privacy/process review: the scoped override removes the recorded dependency findings. Production target interaction and resource ownership are unchanged. ## Before merge - [x] Concrete problems and regression gaps are described. - [x] Scope and authority are resolved for these existing-boundary changes. - [x] The title follows Conventional Commits and describes the final diff. - [x] Related issue is linked. - [x] Tests cover changed assertions and meaningful observable failure cases. - [x] Generated dependency inventory is updated. - [x] Representative CLI behavior is included. - [x] Final diff checked for secrets, unrelated outputs, and unsupported claims. - [ ] Complete CI suite has passed. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

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

closed this issue
added the
pull-request
label

bsakaguchi referenced this issue

2026-11-01 21:44:38 +00:00

[PR #5627] [MERGED] test(evaluation): include native preparation in test deadlines #7003

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

No description provided.

Rows
Columns