morluto/rea

Fork 0

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

[PR #7056] test(inspector): keep the target cancellation fixture path exactly reported #7611

New issue

Open

opened 2026-11-03 13:39:06 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-11-03 13:39:06 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/7056
Author: @RishTheFish21
Created: 10/25/2026
Status: 🔄 Open

Base: main ← Head: fix/inspector-cancel-test-macos-tmpdir

📝 Commits (4)

4068f07 test(inspector): keep Node discovery fixtures out of underscored temp dirs
a833311 test(inspector): resolve /tmp only when tmpdir() is ineligible
43bb623 test(inspector): use the shared owned prefix for Node discovery fixtures
defd961 test(inspector): keep the target cancellation fixture path exactly reported

📊 Changes

4 files changed (+99 additions, -49 deletions)

View changed files

📝 src/inspector/JavaScriptRuntimeScope.cancellation.test.ts (+49 -44)
📝 tests/boundary/inspector/javascriptRuntimeObservation.test.ts (+3 -3)
📝 tests/boundary/inspector/javascriptRuntimeTargetLocations.test.ts (+2 -2)
📝 tests/fixtures/temporaryDirectory.ts (+45 -0)

📄 Description

Summary

src/inspector/JavaScriptRuntimeScope.cancellation.test.ts › "stops Inspector target authorization after the current stat settles" fails on every macOS run of npm run test:fast.

Node's Inspector discovery reports a target's file path lossily: quotes, backslashes and underscores all come back as _. So authorizeRuntimeTargetLocation only stats a reported path that contains none of them, and otherwise records the target as unresolved (unverifiable-file-location) without touching the filesystem. The test builds its fixture under tmpdir(). On macOS that's a per-user directory like /var/folders/w3/_ypt96s55rl6vspl_7d6yjnw0000gn/T/, which contains underscores. Neither target is stat'ed, so the abort the test schedules on the second stat never fires, and listTargets succeeds.

The fix builds the fixture under a root that Node reports exactly: tmpdir() when it has none of those characters (Linux, unchanged), otherwise /tmp (/private/tmp on macOS).

No issue is linked. I found this running npm run test:fast on macOS.

Problem and expected behavior

Before, on macOS arm64 at main MAIN_SHA:

 FAIL   adapters  src/inspector/JavaScriptRuntimeScope.cancellation.test.ts > stops Inspector target authorization after the current stat settles
AssertionError: expected true to be false // Object.is equality
 ❯ src/inspector/JavaScriptRuntimeScope.cancellation.test.ts:140:21
    140|   expect(result.ok).toBe(false);

 Test Files  1 failed | 1022 passed (1023)
      Tests  1 failed | 18594 passed | 27 skipped (18622)

Expected: the test exercises what it names, on any POSIX host. The second target's stat settles, the abort lands, and the result is an AnalysisCancelledError carrying the one-target partial inventory.

Change and scope

Test-only: one helper in the test file picks the fixture root. Both tests in the file use it. The Electron test doesn't depend on the root, but it keeps passing.
Not changed: the lossy-path rule in JavaScriptRuntimeScope.ts is intentional (see tests/boundary/inspector/javascriptRuntimeTargetLocations.test.ts), and so is the rest of the product code.
Not included: Windows. On Windows this branch never stats a Node target, so the test can't exercise it there. I haven't changed how or whether Windows runs this file.

Follow the [contribution scope](https://github.com/morluto/rea/blob/main/CONTRIBUTING.md#contribution-scope).
Unresolved scope, authority, side effects, or required verification block
integration. Link the required maintainer scope decision for new providers or
capabilities involving target execution, injection, mutation, or protection
recovery; ordinary fixes within existing boundaries may state "not applicable".
Draft PRs may leave the scope decision pending and the readiness and merge checklists
unchecked. Maintainers can help resolve open questions.

Contract and boundary impact

Semantic owner and earliest changed stage: test fixture in src/inspector/JavaScriptRuntimeScope.cancellation.test.ts. No product stage changes.
Contribution scope fit and linked maintainer decision: Within existing boundaries (test fix). Maintainer decision not applicable.
CLI and MCP/tool-catalog contract: None.
Provider, bridge, target-format, or platform compatibility: None.
Evidence, artifact, provenance, or reconstruction contract: None.
Target selection, authority, upstream behavior, and cleanup details (if applicable): The fixture directory is still created with an owned rea- prefix and removed in afterEach.
Generated metadata (docs/public/product-catalog.json), package, or installation impact (CI generates and validates ignored catalogs and skills/; edit authored instructions in .agents/skills/reverse-engineer-anything/ and never commit build outputs): None.

Expected effects (check known effects; use documentation-only on its own).
Unchecked items do not establish absence; leave uncertainties visible.

Read existing files or captured data.
Launch or attach to a process.
Inject instrumentation or run scripts/code.
Change process memory, registers, files, or configuration.
Contact network services.
Recover protected code or work around protections.
Documentation or usability only; no target interaction.
Not sure yet; investigation or maintainer guidance needed.

Evidence and regression coverage

Tests added or updated: src/inspector/JavaScriptRuntimeScope.cancellation.test.ts (fixture root only).
Base reproduction or other evidence: the failure above, from npm run test:fast on macOS arm64. The cause follows from authorizeRuntimeTargetLocation, which checks /[_"\\]/u before stating a reported Node path, and from the macOS tmpdir() shown above.
User-visible CLI/MCP output (if applicable): none.
Remaining proof gaps: run on macOS arm64 only. On Linux, tmpdir() normally has no underscores, so the fixture root is unchanged there.

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

macOS arm64, Node 24.22.1:

npm run test:focused -- src/inspector/JavaScriptRuntimeScope.cancellation.test.ts on main: RESULT_BEFORE
the same command with this change: RESULT_AFTER
npm run check:fast: RESULT
npm run format:check: RESULT

Compatibility, safety, and release

Breaking changes or migration steps: None.
Real Hopper/Ghidra, browser, or OS coverage: macOS arm64 test run.
Package or release metadata impact: None.
Security, privacy, process, or containment review: Test-only. The fixture writes two inert files under an owned temporary directory and removes them.

Author readiness

Human owner: @RishTheFish21

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/7056 **Author:** [@RishTheFish21](https://github.com/RishTheFish21) **Created:** 10/25/2026 **Status:** 🔄 Open **Base:** `main` ← **Head:** `fix/inspector-cancel-test-macos-tmpdir` --- ### 📝 Commits (4) - [`4068f07`](https://github.com/morluto/rea/commit/4068f072b270d8fd60ba26de487e6161327c6074) test(inspector): keep Node discovery fixtures out of underscored temp dirs - [`a833311`](https://github.com/morluto/rea/commit/a8333112b83d0a776b53c5394d19772e808195a7) test(inspector): resolve /tmp only when tmpdir() is ineligible - [`43bb623`](https://github.com/morluto/rea/commit/43bb6236ce86d0fe274f22891cb73d298b0963c0) test(inspector): use the shared owned prefix for Node discovery fixtures - [`defd961`](https://github.com/morluto/rea/commit/defd9616d3dfe19b39472edc3ce65f185ee5c17d) test(inspector): keep the target cancellation fixture path exactly reported ### 📊 Changes **4 files changed** (+99 additions, -49 deletions) <details> <summary>View changed files</summary> 📝 `src/inspector/JavaScriptRuntimeScope.cancellation.test.ts` (+49 -44) 📝 `tests/boundary/inspector/javascriptRuntimeObservation.test.ts` (+3 -3) 📝 `tests/boundary/inspector/javascriptRuntimeTargetLocations.test.ts` (+2 -2) 📝 `tests/fixtures/temporaryDirectory.ts` (+45 -0) </details> ### 📄 Description ## Summary `src/inspector/JavaScriptRuntimeScope.cancellation.test.ts` › "stops Inspector target authorization after the current stat settles" fails on every macOS run of `npm run test:fast`. Node's Inspector discovery reports a target's file path lossily: quotes, backslashes and underscores all come back as `_`. So `authorizeRuntimeTargetLocation` only stats a reported path that contains none of them, and otherwise records the target as `unresolved` (`unverifiable-file-location`) without touching the filesystem. The test builds its fixture under `tmpdir()`. On macOS that's a per-user directory like `/var/folders/w3/_ypt96s55rl6vspl_7d6yjnw0000gn/T/`, which contains underscores. Neither target is stat'ed, so the abort the test schedules on the second stat never fires, and `listTargets` succeeds. The fix builds the fixture under a root that Node reports exactly: `tmpdir()` when it has none of those characters (Linux, unchanged), otherwise `/tmp` (`/private/tmp` on macOS). No issue is linked. I found this running `npm run test:fast` on macOS. ## Problem and expected behavior Before, on macOS arm64 at main `MAIN_SHA`: ``` FAIL adapters src/inspector/JavaScriptRuntimeScope.cancellation.test.ts > stops Inspector target authorization after the current stat settles AssertionError: expected true to be false // Object.is equality ❯ src/inspector/JavaScriptRuntimeScope.cancellation.test.ts:140:21 140| expect(result.ok).toBe(false); Test Files 1 failed | 1022 passed (1023) Tests 1 failed | 18594 passed | 27 skipped (18622) ``` Expected: the test exercises what it names, on any POSIX host. The second target's stat settles, the abort lands, and the result is an `AnalysisCancelledError` carrying the one-target partial inventory. ## Change and scope - Test-only: one helper in the test file picks the fixture root. Both tests in the file use it. The Electron test doesn't depend on the root, but it keeps passing. - Not changed: the lossy-path rule in `JavaScriptRuntimeScope.ts` is intentional (see `tests/boundary/inspector/javascriptRuntimeTargetLocations.test.ts`), and so is the rest of the product code. - Not included: Windows. On Windows this branch never stats a Node target, so the test can't exercise it there. I haven't changed how or whether Windows runs this file. Follow the [[contribution scope](https://github.com/morluto/rea/blob/main/CONTRIBUTING.md#contribution-scope)](https://github.com/morluto/rea/blob/main/CONTRIBUTING.md#contribution-scope). Unresolved scope, authority, side effects, or required verification block integration. Link the required maintainer scope decision for new providers or capabilities involving target execution, injection, mutation, or protection recovery; ordinary fixes within existing boundaries may state "not applicable". Draft PRs may leave the scope decision pending and the readiness and merge checklists unchecked. Maintainers can help resolve open questions. ## Contract and boundary impact - Semantic owner and earliest changed stage: test fixture in `src/inspector/JavaScriptRuntimeScope.cancellation.test.ts`. No product stage changes. - Contribution scope fit and linked maintainer decision: Within existing boundaries (test fix). Maintainer decision not applicable. - CLI and MCP/tool-catalog contract: None. - Provider, bridge, target-format, or platform compatibility: None. - Evidence, artifact, provenance, or reconstruction contract: None. - Target selection, authority, upstream behavior, and cleanup details (if applicable): The fixture directory is still created with an owned `rea-` prefix and removed in `afterEach`. - Generated metadata (`docs/public/product-catalog.json`), package, or installation impact (CI generates and validates ignored catalogs and `skills/`; edit authored instructions in `.agents/skills/reverse-engineer-anything/` and never commit build outputs): None. Expected effects (check known effects; use documentation-only on its own). Unchecked items do not establish absence; leave uncertainties visible. - [ ] Read existing files or captured data. - [ ] Launch or attach to a process. - [ ] Inject instrumentation or run scripts/code. - [ ] Change process memory, registers, files, or configuration. - [ ] Contact network services. - [ ] Recover protected code or work around protections. - [ ] Documentation or usability only; no target interaction. - [ ] Not sure yet; investigation or maintainer guidance needed. ## Evidence and regression coverage - Tests added or updated: `src/inspector/JavaScriptRuntimeScope.cancellation.test.ts` (fixture root only). - Base reproduction or other evidence: the failure above, from `npm run test:fast` on macOS arm64. The cause follows from `authorizeRuntimeTargetLocation`, which checks `/[_"\\]/u` before stating a reported Node path, and from the macOS `tmpdir()` shown above. - User-visible CLI/MCP output (if applicable): none. - Remaining proof gaps: run on macOS arm64 only. On Linux, `tmpdir()` normally has no underscores, so the fixture root is unchanged there. 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 macOS arm64, Node 24.22.1: - `npm run test:focused -- src/inspector/JavaScriptRuntimeScope.cancellation.test.ts` on main: RESULT_BEFORE - the same command with this change: RESULT_AFTER - `npm run check:fast`: RESULT - `npm run format:check`: RESULT ## Compatibility, safety, and release - Breaking changes or migration steps: None. - Real Hopper/Ghidra, browser, or OS coverage: macOS arm64 test run. - Package or release metadata impact: None. - Security, privacy, process, or containment review: Test-only. The fixture writes two inert files under an owned temporary directory and removes them. ## Author readiness Human owner: @RishTheFish21 - [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] This PR addresses a concrete problem or an agreed enhancement. - [x] Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff. - [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. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi added the

pull-request

label

2026-11-03 13:39:06 +00:00

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

No description provided.

Rows
Columns