[PR #4484] test(inspector): keep Node discovery fixtures out of underscored temp dirs #6269

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

📋 Pull Request Information

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

Base: main ← Head: fix/macos-node-discovery-tmpdir


📝 Commits (2)

  • 8d1bc53 test(inspector): keep Node discovery fixtures out of underscored temp dirs
  • 813d8a0 test(inspector): resolve /tmp only when tmpdir() is ineligible

📊 Changes

3 files changed (+52 additions, -5 deletions)

View changed files

📝 tests/boundary/inspector/javascriptRuntimeObservation.test.ts (+3 -3)
📝 tests/boundary/inspector/javascriptRuntimeTargetLocations.test.ts (+2 -2)
📝 tests/fixtures/temporaryDirectory.ts (+47 -0)

📄 Description

Summary

Four inspector boundary tests cannot pass on macOS hosts whose per-user temporary directory contains an underscore. Their fixtures are created under realpath(os.tmpdir()), and REA deliberately treats a Node-reported path containing _ as unverifiable. This keeps those fixtures in a temporary directory whose canonical path cannot be confused with Node's lossy rewriting. Test-only change; no product behaviour changes.

Problem and expected behavior

authorizeRuntimeTargetLocation (src/inspector/JavaScriptRuntimeScope.ts) marks a Node-reported POSIX file:// path containing _, " or \ as unverifiable-file-location, because Node's Inspector discovery metadata replaces quotes and backslashes with underscores. That rule is correct and unchanged.

On macOS, os.tmpdir() is a per-user directory of the shape /private/var/folders/<x>/<y>/T, and <y> often contains _. Every fixture that createTestTempDirectory creates there is therefore reported as unresolved. These tests expect a verified file location, so they fail on such hosts:

  • javascriptRuntimeObservation.test.ts: "includes targets outside the old caller root filter"
  • javascriptRuntimeTargetLocations.test.ts: "interprets entry#hash.js / entry%percent.js / entry%23hash.js as a literal Node discovery path on POSIX"

Linux /tmp contains none of those characters, so the tests pass there.

Change and scope

tests/fixtures/temporaryDirectory.ts gains createNodeDiscoveryTempDirectory(prefix):

  • On POSIX it uses the canonical temporary directory when it is free of _, " and \. Only otherwise does it resolve the canonical /tmp, so a host without /tmp still works when tmpdir() is clean.
  • It fails loudly when neither is clean, rather than silently producing an ambiguous fixture.
  • Windows keeps createTestTempDirectory, since Node discovery paths are never verified there.
  • Cleanup is the same onTestFinished → removeTestWorkspace path.

The two inspector test files use it for their fixture roots. No assertion changes.

Contribution scope: an ordinary test fix within existing boundaries; a maintainer scope decision is not applicable.

Contract and boundary impact

  • Semantic owner and earliest changed stage: test fixtures only.
  • Contribution scope fit and linked maintainer decision: not applicable (test-only fix).
  • CLI and MCP/tool-catalog contract: none.
  • Provider, bridge, target-format, or platform compatibility: none in product code. Test fixtures on macOS may now live under /private/tmp.
  • Evidence, artifact, provenance, or reconstruction contract: none.
  • Target selection, authority, upstream behavior, and cleanup details: unchanged; fixture cleanup uses the existing helper.
  • Generated metadata, package, or installation impact: none.

Evidence and regression coverage

  • Tests added or updated: the two inspector test files now build their fixture roots with the new helper.
  • Base reproduction (macOS 27 arm64 host whose TMPDIR contains _, Node 24.18.0 from .nvmrc): npx vitest run tests/boundary/inspector/javascriptRuntimeObservation.test.ts tests/boundary/inspector/javascriptRuntimeTargetLocations.test.ts at fe2a398dd fails 4 of 38 tests, the 4 named above. Each receives { kind: "unresolved", reason: "unverifiable-file-location" }.
  • With this change, on the same host: 38/38 pass.
  • Remaining proof gaps: not run on Linux or Windows locally. On Linux the helper resolves to realpath(os.tmpdir()) exactly as before, unless TMPDIR itself contains _, " or \. On Windows it is the old helper. CI covers both.

Validation performed

  • npx vitest run on the two files at fe2a398dd: 4 failed / 34 passed (38).
  • The same at this branch: 38 passed with the default (underscored) TMPDIR, and 38 passed with a clean TMPDIR (primary used).
  • npm run check:fast: passed (2/2 tasks).
  • npm run check: passed (6/6 tasks; architecture guards passed across 1455 source files).
  • npm run verify:test-discovery: discovered all 1417 test files exactly once.

Compatibility, safety, and release

  • Breaking changes or migration steps: none.
  • Real Hopper/Ghidra, browser, or OS coverage: macOS host only, as above.
  • Package or release metadata impact: none (tests only).
  • Security, privacy, process, or containment review: not applicable; no product code changes.

Author readiness

Human owner: @Kenmege (the declarations below are left for the human owner, per the template)

  • 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 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/4484 **Author:** [@Kenmege](https://github.com/Kenmege) **Created:** 10/16/2026 **Status:** 🔄 Open **Base:** `main` ← **Head:** `fix/macos-node-discovery-tmpdir` --- ### 📝 Commits (2) - [`8d1bc53`](https://github.com/morluto/rea/commit/8d1bc53a43098488632f4206a3a8b4b0e6d2416b) test(inspector): keep Node discovery fixtures out of underscored temp dirs - [`813d8a0`](https://github.com/morluto/rea/commit/813d8a010eb5ca820957629042aa6581a3c76e46) test(inspector): resolve /tmp only when tmpdir() is ineligible ### 📊 Changes **3 files changed** (+52 additions, -5 deletions) <details> <summary>View changed files</summary> 📝 `tests/boundary/inspector/javascriptRuntimeObservation.test.ts` (+3 -3) 📝 `tests/boundary/inspector/javascriptRuntimeTargetLocations.test.ts` (+2 -2) 📝 `tests/fixtures/temporaryDirectory.ts` (+47 -0) </details> ### 📄 Description ## Summary Four inspector boundary tests cannot pass on macOS hosts whose per-user temporary directory contains an underscore. Their fixtures are created under `realpath(os.tmpdir())`, and REA deliberately treats a Node-reported path containing `_` as unverifiable. This keeps those fixtures in a temporary directory whose canonical path cannot be confused with Node's lossy rewriting. Test-only change; no product behaviour changes. ## Problem and expected behavior `authorizeRuntimeTargetLocation` (`src/inspector/JavaScriptRuntimeScope.ts`) marks a Node-reported POSIX `file://` path containing `_`, `"` or `\` as `unverifiable-file-location`, because Node's Inspector discovery metadata replaces quotes and backslashes with underscores. That rule is correct and unchanged. On macOS, `os.tmpdir()` is a per-user directory of the shape `/private/var/folders/<x>/<y>/T`, and `<y>` often contains `_`. Every fixture that `createTestTempDirectory` creates there is therefore reported as unresolved. These tests expect a verified `file` location, so they fail on such hosts: - `javascriptRuntimeObservation.test.ts`: "includes targets outside the old caller root filter" - `javascriptRuntimeTargetLocations.test.ts`: "interprets entry#hash.js / entry%percent.js / entry%23hash.js as a literal Node discovery path on POSIX" Linux `/tmp` contains none of those characters, so the tests pass there. ## Change and scope `tests/fixtures/temporaryDirectory.ts` gains `createNodeDiscoveryTempDirectory(prefix)`: - On POSIX it uses the canonical temporary directory when it is free of `_`, `"` and `\`. Only otherwise does it resolve the canonical `/tmp`, so a host without `/tmp` still works when `tmpdir()` is clean. - It fails loudly when neither is clean, rather than silently producing an ambiguous fixture. - Windows keeps `createTestTempDirectory`, since Node discovery paths are never verified there. - Cleanup is the same `onTestFinished` → `removeTestWorkspace` path. The two inspector test files use it for their fixture roots. No assertion changes. Contribution scope: an ordinary test fix within existing boundaries; a maintainer scope decision is not applicable. ## Contract and boundary impact - Semantic owner and earliest changed stage: test fixtures only. - Contribution scope fit and linked maintainer decision: not applicable (test-only fix). - CLI and MCP/tool-catalog contract: none. - Provider, bridge, target-format, or platform compatibility: none in product code. Test fixtures on macOS may now live under `/private/tmp`. - Evidence, artifact, provenance, or reconstruction contract: none. - Target selection, authority, upstream behavior, and cleanup details: unchanged; fixture cleanup uses the existing helper. - Generated metadata, package, or installation impact: none. ## Evidence and regression coverage - Tests added or updated: the two inspector test files now build their fixture roots with the new helper. - Base reproduction (macOS 27 arm64 host whose `TMPDIR` contains `_`, Node 24.18.0 from `.nvmrc`): `npx vitest run tests/boundary/inspector/javascriptRuntimeObservation.test.ts tests/boundary/inspector/javascriptRuntimeTargetLocations.test.ts` at fe2a398dd fails 4 of 38 tests, the 4 named above. Each receives `{ kind: "unresolved", reason: "unverifiable-file-location" }`. - With this change, on the same host: 38/38 pass. - Remaining proof gaps: not run on Linux or Windows locally. On Linux the helper resolves to `realpath(os.tmpdir())` exactly as before, unless TMPDIR itself contains `_`, `"` or `\`. On Windows it is the old helper. CI covers both. ## Validation performed - `npx vitest run` on the two files at fe2a398dd: 4 failed / 34 passed (38). - The same at this branch: 38 passed with the default (underscored) TMPDIR, and 38 passed with a clean `TMPDIR` (primary used). - `npm run check:fast`: passed (2/2 tasks). - `npm run check`: passed (6/6 tasks; architecture guards passed across 1455 source files). - `npm run verify:test-discovery`: discovered all 1417 test files exactly once. ## Compatibility, safety, and release - Breaking changes or migration steps: none. - Real Hopper/Ghidra, browser, or OS coverage: macOS host only, as above. - Package or release metadata impact: none (tests only). - Security, privacy, process, or containment review: not applicable; no product code changes. ## Author readiness Human owner: @Kenmege (the declarations below are left for the human owner, per the template) - [ ] 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. --- <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#6269
No description provided.