morluto/rea

Fork 0

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

[PR #3298] [CLOSED] fix(javascript): honor node-ID seed matching options #5710

New issue

Closed

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

bsakaguchi commented

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

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/3298
Author: @jpdefo
Created: 10/14/2026
Status: ❌ Closed

Base: main ← Head: fix/node-id-seed-matching

📝 Commits (2)

32e357e fix(javascript): honor node-ID seed matching options
27bd5f3 Merge remote-tracking branch 'origin/main' into fix/node-id-seed-matching

📊 Changes

2 files changed (+105 additions, -14 deletions)

View changed files

📝 src/domain/javascript/javascriptFeatureSeed.ts (+5 -14)
➕ tests/acceptance/applications/applicationNodeIdSeedCli.test.ts (+100 -0)

📄 Description

Summary

Fixes #3189. trace-application-feature accepts substring and case-insensitive node-ID seeds but previously compared IDs with strict equality. A partial ID or an uppercase ID with case sensitivity disabled returned no-match even when the node existed.

Change and scope

Remove the node-ID equality shortcut. Treat node_id as its sole candidate field and use the existing shared normalization and exact/contains matcher. Returned IDs retain their original spelling, basis: "node-id", and field: "node_id". Labels and properties cannot match a node-ID seed.

This is a routine correction to existing static graph queries. Advance scope approval is not applicable. CLI and MCP already share this domain matcher. Request/output schemas, graph identity, provider behavior, package metadata, and execution authority are unchanged. No additional target interaction is introduced.

Evidence and regression coverage

On original main 535c8ef3031d6808c8e03986d513b4507931b777, the new compiled CLI regression failed five of ten scenarios: partial-ID containment, uppercase containment, uppercase exact matching without case sensitivity, default matching options on an uppercase ID, and containment selecting all node IDs. Exact-ID and negative controls passed.

After the fix, all ten scenarios pass. The test analyzes a local CommonJS fixture through the compiled CLI, then traces its authenticated Evidence through the compiled CLI. It verifies complete match records, match counts, and coverage status. For example, partial-ID containment now returns one match and complete-within-source instead of zero matches and no-match.

The fixture is parsed as inert source. Test processes and temporary files use the existing test-owned cleanup fixtures.

Validation performed

Linux x64 under WSL, pinned Node 24.18.0 and npm 11.16.0. Revalidated after merging main 0351328e in 27bd5f3c, including upstream uninstall-fixture correction a154b975:

npm ci and npm run build:cached: passed.
npm run test:focused -- tests/boundary/filesystem/clientConfigurationBom.test.ts tests/boundary/filesystem/clientLifecycle.test.ts tests/acceptance/applications/applicationNodeIdSeedCli.test.ts src/domain/javascript/javascriptFeatureSeed.test.ts src/domain/javascript/javascriptFeatureSeed.array_members.test.ts tests/boundary/filesystem/javascript-applications/javascriptApplicationEmptyLiterals.test.ts tests/boundary/mcp/applicationEvidenceMcp.test.ts: 146 tests passed across seven files, including the previously failing BOM restoration test. Includes existing MCP parity coverage, not an added MCP-specific node-ID matrix.
npm run check: passed, including types, lint, formatting, metadata, module boundaries, and architecture guards.
npm run verify:test-discovery: all 1,266 test files discovered exactly once.
git diff --check: passed.

Hosted CI 38539980764 has a source-suite failure in javascriptStaticAnalysisModuleIndex.test.ts, case keeps a top-level wrapper require redeclaration: the expected ./wrapper.cjs specifier is null. I reproduced the same failure with the node-ID change removed, restoring the production source of base 0351328e (26 tests passed, 1 failed in that file). This is separate from #3189 and is not suppressed or repaired in this PR. Other hosted jobs are still running. Native Windows execution remains unverified. No runtime Electron or native-provider claim is made.

Author readiness

Human owner: @jpdefo

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

Concrete reproduced problem, linked issue, and one focused outcome.
Existing capability and authority boundaries preserved.
Regression fails on the affected base and passes after the correction.
Documentation already describes the requested matching behavior. No generated metadata change is required.
Required hosted checks passed on the current revision.
Human owner completed the readiness declarations.

_{🔄 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/3298 **Author:** [@jpdefo](https://github.com/jpdefo) **Created:** 10/14/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/node-id-seed-matching` --- ### 📝 Commits (2) - [`32e357e`](https://github.com/morluto/rea/commit/32e357ed03571ac94ecb729bc476d37e8ba203b7) fix(javascript): honor node-ID seed matching options - [`27bd5f3`](https://github.com/morluto/rea/commit/27bd5f3c0b97034bf472602cd96259fc07da2a4e) Merge remote-tracking branch 'origin/main' into fix/node-id-seed-matching ### 📊 Changes **2 files changed** (+105 additions, -14 deletions) <details> <summary>View changed files</summary> 📝 `src/domain/javascript/javascriptFeatureSeed.ts` (+5 -14) ➕ `tests/acceptance/applications/applicationNodeIdSeedCli.test.ts` (+100 -0) </details> ### 📄 Description ## Summary Fixes #3189. `trace-application-feature` accepts substring and case-insensitive node-ID seeds but previously compared IDs with strict equality. A partial ID or an uppercase ID with case sensitivity disabled returned `no-match` even when the node existed. ## Change and scope Remove the node-ID equality shortcut. Treat `node_id` as its sole candidate field and use the existing shared normalization and exact/contains matcher. Returned IDs retain their original spelling, `basis: "node-id"`, and `field: "node_id"`. Labels and properties cannot match a node-ID seed. This is a routine correction to existing static graph queries. Advance scope approval is not applicable. CLI and MCP already share this domain matcher. Request/output schemas, graph identity, provider behavior, package metadata, and execution authority are unchanged. No additional target interaction is introduced. ## Evidence and regression coverage On original main `535c8ef3031d6808c8e03986d513b4507931b777`, the new compiled CLI regression failed five of ten scenarios: partial-ID containment, uppercase containment, uppercase exact matching without case sensitivity, default matching options on an uppercase ID, and containment selecting all node IDs. Exact-ID and negative controls passed. After the fix, all ten scenarios pass. The test analyzes a local CommonJS fixture through the compiled CLI, then traces its authenticated Evidence through the compiled CLI. It verifies complete match records, match counts, and coverage status. For example, partial-ID containment now returns one match and `complete-within-source` instead of zero matches and `no-match`. The fixture is parsed as inert source. Test processes and temporary files use the existing test-owned cleanup fixtures. ## Validation performed Linux x64 under WSL, pinned Node 24.18.0 and npm 11.16.0. Revalidated after merging main `0351328e` in `27bd5f3c`, including upstream uninstall-fixture correction `a154b975`: - `npm ci` and `npm run build:cached`: passed. - `npm run test:focused -- tests/boundary/filesystem/clientConfigurationBom.test.ts tests/boundary/filesystem/clientLifecycle.test.ts tests/acceptance/applications/applicationNodeIdSeedCli.test.ts src/domain/javascript/javascriptFeatureSeed.test.ts src/domain/javascript/javascriptFeatureSeed.array_members.test.ts tests/boundary/filesystem/javascript-applications/javascriptApplicationEmptyLiterals.test.ts tests/boundary/mcp/applicationEvidenceMcp.test.ts`: 146 tests passed across seven files, including the previously failing BOM restoration test. Includes existing MCP parity coverage, not an added MCP-specific node-ID matrix. - `npm run check`: passed, including types, lint, formatting, metadata, module boundaries, and architecture guards. - `npm run verify:test-discovery`: all 1,266 test files discovered exactly once. - `git diff --check`: passed. Hosted CI [38539980764](https://github.com/morluto/rea/actions/runs/38539980764) has a source-suite failure in `javascriptStaticAnalysisModuleIndex.test.ts`, case `keeps a top-level wrapper require redeclaration`: the expected `./wrapper.cjs` specifier is null. I reproduced the same failure with the node-ID change removed, restoring the production source of base `0351328e` (26 tests passed, 1 failed in that file). This is separate from #3189 and is not suppressed or repaired in this PR. Other hosted jobs are still running. Native Windows execution remains unverified. No runtime Electron or native-provider claim is made. ## Author readiness Human owner: @jpdefo - [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 - [x] Concrete reproduced problem, linked issue, and one focused outcome. - [x] Existing capability and authority boundaries preserved. - [x] Regression fails on the affected base and passes after the correction. - [x] Documentation already describes the requested matching behavior. No generated metadata change is required. - [ ] Required hosted checks passed on the current revision. - [x] Human owner completed the readiness declarations. --- <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:49 +00:00

closed this issue
added the
pull-request
label

bsakaguchi referenced this issue

2026-11-01 21:45:12 +00:00

[PR #5810] [MERGED] fix(javascript): count one IPC observation per site-resolved channel #7075

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

No description provided.

Rows
Columns