morluto/rea

Fork 0

mirror of https://github.com/morluto/rea.git synced 2026-11-05 07:47:29 +00:00

[PR #1644] [MERGED] test(javascript): retain resource ownership in deep-return fixture #4722

New issue

Closed

opened 2026-10-31 00:54:23 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 00:54:23 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/1644
Author: @evenafterallisaid
Created: 10/10/2026
Status: ✅ Merged
Merged: 10/10/2026
Merged by: @morluto

Base: main ← Head: test/javascript-deep-return-scope

📝 Commits (1)

b9aa229 test(javascript): retain resource ownership in deep-return fixture

📊 Changes

1 file changed (+4 additions, -5 deletions)

View changed files

📝 tests/boundary/filesystem/javascriptApplicationDeepReturns.test.ts (+4 -5)

📄 Description

summary

migrate the deep-return test's embedded child script to the existing JavaScript analysis helper that owns and closes an artifact resource scope. this repairs the test failure introduced by the caller-owned scope API migration.

problem and expected behavior

ffe0574 requires an ArtifactResourceScope for direct JavaScript analysis calls. this test's JavaScript string still called the compiled service without that argument, so typechecking did not catch the missed migration. it now fails with Cannot read properties of undefined (reading 'run') before exercising deep-source analysis.

the test should retain its default-stack regression while owning and awaiting artifact cleanup. the same failure appears in shard 4 of #1642: one test failed, 1523 passed, and 14 skipped. the source-map changes in that pr do not modify this call.

no issue is linked because this was identified directly from the ci failure; existing issues and prs were checked for overlap.

change and scope

the child imports analyzeJavaScriptApplicationWithOwnedScope from the existing verifier helper and reads its returned Evidence. the helper already passes a fresh scope to the real compiled analyzer, closes it on success or failure, and propagates analysis and cleanup errors. no new wrapper or production behavior is added.

contract and boundary impact

semantic owner and earliest changed stage: the test child script's direct analysis invocation.
CLI and MCP/tool-catalog contract: none.
provider, bridge, target-format, or platform compatibility: no production changes; the test follows the current caller-owned resource API.
evidence, artifact, provenance, or reconstruction contract: unchanged real analyzer Evidence and return-owner assertions.
process execution, authorization, cleanup, or containment impact: the child now awaits artifact cleanup before printing its result. process, memory, stack, timeout, and temporary-workspace bounds remain unchanged.
generated metadata, package, or installation impact: none.

evidence and regression coverage

test updated: the existing isolated child-process regression, retaining a 20,000-property chain, nested and class-method returns, and three distinct return-owner assertions.
base reproduction: the unchanged test fails on 0fa0b046 with the same undefined.run error as ci; it passes after the migration.
user-visible CLI/MCP output: not applicable; test-only migration.
remaining proof gaps: local verification ran on Windows; the Linux ci rerun is pending. no real disassembler behavior is claimed.

validation performed

npm run test:focused -- tests/boundary/filesystem/javascriptApplicationDeepReturns.test.ts before the edit — failed with the ci error.
the same command after the edit — one test passed, using the actual compiled analyzer in its isolated Node child.
npm run check — passed, including typecheck, lint, formatting, module boundaries, metadata, knip, and architecture guards; 635 lint warnings, zero errors.
git diff --check — passed.

compatibility, safety, and release

breaking changes or migration steps: none in this pr; test migration for the API change already on main.
real Hopper/Ghidra, browser, or OS coverage: pinned Node 24.18.0 and npm 11.16.0 on Windows, through a real analysis subprocess; no engine required.
package or release metadata impact: none.
security, privacy, process, or containment review: no runtime authority changes; the test now uses the existing cleanup owner instead of bypassing the required scope.

review checklist

this pr addresses a concrete ci failure.
the pr has one focused outcome and a conventional title.
the reason for not linking an issue is stated above.
the existing meaningful regression fails before and passes after the change.
no docs, contracts, or generated metadata require changes.
no user-visible CLI/MCP output changes.
the final diff was checked 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/1644 **Author:** [@evenafterallisaid](https://github.com/evenafterallisaid) **Created:** 10/10/2026 **Status:** ✅ Merged **Merged:** 10/10/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `test/javascript-deep-return-scope` --- ### 📝 Commits (1) - [`b9aa229`](https://github.com/morluto/rea/commit/b9aa22936774c83c06bd73f1b0e6af97f02de7b4) test(javascript): retain resource ownership in deep-return fixture ### 📊 Changes **1 file changed** (+4 additions, -5 deletions) <details> <summary>View changed files</summary> 📝 `tests/boundary/filesystem/javascriptApplicationDeepReturns.test.ts` (+4 -5) </details> ### 📄 Description ## summary migrate the deep-return test's embedded child script to the existing JavaScript analysis helper that owns and closes an artifact resource scope. this repairs the test failure introduced by the caller-owned scope API migration. ## problem and expected behavior [ffe0574](https://github.com/morluto/rea/commit/ffe0574e17b9d973860371083cb1a351be912131) requires an `ArtifactResourceScope` for direct JavaScript analysis calls. this test's JavaScript string still called the compiled service without that argument, so typechecking did not catch the missed migration. it now fails with `Cannot read properties of undefined (reading 'run')` before exercising deep-source analysis. the test should retain its default-stack regression while owning and awaiting artifact cleanup. the same failure appears in [shard 4 of #1642](https://github.com/morluto/rea/actions/runs/38084923695/job/114309471705): one test failed, 1523 passed, and 14 skipped. the source-map changes in that pr do not modify this call. no issue is linked because this was identified directly from the ci failure; existing issues and prs were checked for overlap. ## change and scope the child imports `analyzeJavaScriptApplicationWithOwnedScope` from the existing verifier helper and reads its returned Evidence. the helper already passes a fresh scope to the real compiled analyzer, closes it on success or failure, and propagates analysis and cleanup errors. no new wrapper or production behavior is added. ## contract and boundary impact - semantic owner and earliest changed stage: the test child script's direct analysis invocation. - CLI and MCP/tool-catalog contract: none. - provider, bridge, target-format, or platform compatibility: no production changes; the test follows the current caller-owned resource API. - evidence, artifact, provenance, or reconstruction contract: unchanged real analyzer Evidence and return-owner assertions. - process execution, authorization, cleanup, or containment impact: the child now awaits artifact cleanup before printing its result. process, memory, stack, timeout, and temporary-workspace bounds remain unchanged. - generated metadata, package, or installation impact: none. ## evidence and regression coverage - test updated: the existing isolated child-process regression, retaining a 20,000-property chain, nested and class-method returns, and three distinct return-owner assertions. - base reproduction: the unchanged test fails on `0fa0b046` with the same `undefined.run` error as ci; it passes after the migration. - user-visible CLI/MCP output: not applicable; test-only migration. - remaining proof gaps: local verification ran on Windows; the Linux ci rerun is pending. no real disassembler behavior is claimed. ## validation performed - `npm run test:focused -- tests/boundary/filesystem/javascriptApplicationDeepReturns.test.ts` before the edit — failed with the ci error. - the same command after the edit — one test passed, using the actual compiled analyzer in its isolated Node child. - `npm run check` — passed, including typecheck, lint, formatting, module boundaries, metadata, knip, and architecture guards; 635 lint warnings, zero errors. - `git diff --check` — passed. ## compatibility, safety, and release - breaking changes or migration steps: none in this pr; test migration for the API change already on main. - real Hopper/Ghidra, browser, or OS coverage: pinned Node 24.18.0 and npm 11.16.0 on Windows, through a real analysis subprocess; no engine required. - package or release metadata impact: none. - security, privacy, process, or containment review: no runtime authority changes; the test now uses the existing cleanup owner instead of bypassing the required scope. ## review checklist - [x] this pr addresses a concrete ci failure. - [x] the pr has one focused outcome and a conventional title. - [x] the reason for not linking an issue is stated above. - [x] the existing meaningful regression fails before and passes after the change. - [x] no docs, contracts, or generated metadata require changes. - [x] no user-visible CLI/MCP output changes. - [x] the final diff was checked 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

2026-10-31 00:54:23 +00:00

closed this issue
added the
pull-request
label

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

No description provided.

Rows
Columns