morluto/rea

Fork 0

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

[PR #3819] [MERGED] fix(javascript): resolve child ownership at receiver reads #5921

New issue

Closed

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

bsakaguchi commented

2026-10-31 01:10:28 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/3819
Author: @Xinglan233
Created: 10/15/2026
Status: ✅ Merged
Merged: 10/16/2026
Merged by: @morluto

Base: main ← Head: fix/3362-child-handle-lifetimes

📝 Commits (3)

2179d4f fix(javascript): resolve child ownership at receiver reads
d5b66f6 fix(javascript): preserve deferred field child candidates
6835cbe fix(javascript): merge main into child ownership fix

📊 Changes

4 files changed (+558 additions, -24 deletions)

View changed files

➕ src/domain/javascript/javascriptSemanticChildProcesses.test.ts (+263 -0)
📝 src/domain/javascript/javascriptSemanticChildProcesses.ts (+88 -23)
📝 src/domain/javascript/javascriptSemanticMutationInitializers.test.ts (+64 -1)
➕ tests/acceptance/applications/javascriptChildHandleOwnership.test.ts (+143 -0)

📄 Description

Summary

Prevent replaced child handles from retaining resolved signal and exit-listener ownership of an earlier spawned child. Unproven links remain candidates with explicit child-process unknowns; unchanged children retain resolved ownership.

Fixes #3362. Prepared with OpenAI Codex assistance.

Problem and expected behavior

import { spawn } from "node:child_process";
let child = spawn("program");
child = { kill() {}, on() {} };
child.kill();
child.on("exit", () => {});
const stable = spawn("program");
stable.kill();

The extractor matched receivers to the spawn inventory by binding ID and marked a unique match complete, even after replacement. A resolved relationship must identify the admitted receiver at its read; uniqueness in the historical inventory is insufficient.

Change and scope

Select receiver initializers at each read and follow direct aliases at their capture sites. Same-body replacement with a new spawn selects that child. Preserve uncertainty for unsupported replacements, conditional/cross-function writes, cycles, projections and defaults; retain the existing scalar awaited-spawn convention.

Reuse the initializer owner's existing statement positions for ordering proof. Stable initialized lexical declarations can retain identity across captures; assignment-only and var sources require proven same-body ordering. Alias traversal is iterative. Existing graph projection emits candidate edges and unknowns without adapter-specific repairs.

This is a routine static-analysis fix within the contribution scope; separate maintainer scope approval is not applicable. It is independent of timer PR #3794 and does not include stdio issue #3355.

Contract and boundary impact

Semantic owner and earliest changed stage: domain child-process interaction recovery and shared initializer ordering; reads under deferred instance field values retain all candidate origins.
Contribution scope fit and linked maintainer decision: ordinary bug fix; not applicable.
CLI and MCP/tool-catalog contract: schemas and tool inventory unchanged; ownership resolution and existing unknowns are corrected in both adapters.
Provider, bridge, target-format, or platform compatibility: no provider or bridge changes; static JavaScript source analysis only.
Evidence, artifact, provenance, or reconstruction contract: preserve spawn identity and source ranges; unproven ownership becomes candidate plus explicit uncertainty.
Target selection, authority, upstream behavior, and cleanup: unchanged; analyzed target calls are never executed.
Generated metadata, package, or installation impact: none; no dependency or release metadata changes.

Expected effects:

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.

These describe the analysis. Verification starts REA's own CLI/MCP processes, not the fixture's processes.

Evidence and regression coverage

Executed on Linux x64, Node 24.18.0/npm 11.16.0. The exact issue fixture was run through the actual public CLI and stdio MCP; their complete Evidence matched at both baseline and fixed revisions.

Fixture observation	Baseline	Fixed
line 4 signal → line 2 child	resolved	candidate
line 2 child → line 5 exit listener	resolved	candidate
line 7 signal → line 6 stable child	resolved	resolved
child-process ownership unknowns	0	2

Baseline af6a9980669129aae882c9216766fc8d9f544079: 15 of the initial 16 domain regressions failed, and the public CLI acceptance regression failed on the stale resolved links. At that revision, the domain file had 22 regressions covering replacement ordering, new children, alias captures, conditional/compound/destructuring/update/loop writes, cross-function initialization, defaults, await, cycles, direct receivers and shadowing. Acceptance asserts endpoint lines, uncertainty, complete CLI/MCP Evidence equality and MCP responsiveness. Independent review findings were reproduced and corrected before final verification.

Remaining proof gaps and conservative limits are listed below; full baseline/final Evidence and failed attempts are retained in the task handoff.

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

On commit 2179d4f3efd4248072b38378f249ae26852f5733, based on main 4692bb33300c7cb77b095b6cc0c33da861c1b860:

npm run build:cached — passed.
npm run check — passed, including typecheck, lint/module boundaries, formatting, knip, metadata and architecture guards; lint reported 0 errors with repository warnings retained.
node node_modules/vitest/vitest.mjs run src/domain/javascript/ src/application/javascript/ tests/boundary/filesystem/referenceSourceShadowedRequire.test.ts — 82 files, 2010 tests passed.
npm run test:focused -- tests/acceptance/applications/javascriptChildHandleOwnership.test.ts tests/acceptance/applications/javascriptBuiltinDefaultsCli.test.ts — 2 files, 7 tests passed.
npm run docs:check — passed, including 5 README conformance tests.
npm run verify:test-discovery — all 1303 files discovered exactly once.
Separate actual public CLI/stdio MCP run of the exact fixture — CLI exit 0, MCP success/ping, identical complete Evidence and corrected results shown above.
Patch application, incremental Git bundle integrity and git diff --check — passed.

Instance-field review correction

Current head: d5b66f6a3b7f55576004435cf815590d2097d569. Verified on macOS arm64 with Node 24.18.0/npm 11.16.0.

The review fixture let child = spawn('old'); class Stop { field = child.kill(); } child = spawn('new'); new Stop(); reproduced at 2179d4f3: the field signal was resolved only to old, omitted new, and reported no ownership unknown. Instance field values execute on construction; the read was incorrectly ordered at class declaration. The shared ordering owner now reuses one deferred-field predicate for read ordering and top-level effect ordering. Both children remain candidates with one ambiguous-target unknown; static field values and computed keys retain class-definition ordering, and stable lexical captures stay resolved.

Before the production fix, 6 child-ownership regressions, 3 shared-ordering regressions and the new CLI/MCP acceptance failed for the reported behavior. Coverage includes public/private/accessor values, nested fields, class expressions, assignment-only initialization, static values, computed keys, methods/closures and stable bindings. Accessor coverage uses the shared Babel AST boundary; parser support was not expanded.
Relevant JavaScript domain/application and reference-import boundary tests: 82 files, 2031 tests passed. Final focused producer regressions after test-only refinements: 46 tests passed.
npm run test:focused -- tests/acceptance/applications/javascriptChildHandleOwnership.test.ts tests/acceptance/applications/javascriptBuiltinDefaultsCli.test.ts: 2 files, 8 tests passed.
npm run check passed, including typecheck, lint/module boundaries, formatting, knip, metadata and architecture guards; lint retained repository warnings with 0 errors. Runtime compilation, npm run verify:test-discovery (1303 files), normal pre-commit/pre-push checks, git diff --check and incremental bundle verification passed.
Separate actual public CLI/stdio MCP runs at the original and fixed heads returned successful results, identical complete Evidence across adapters and successful MCP ping. The instance-field signal changed from old-only resolved to old/new candidates plus explicit uncertainty; a stable-child control stayed resolved. Fixture spawn/kill calls were never executed.
GitHub CI (run 38676037050) and PR title (run 38676037007) for this head both report action_required, with no jobs executed; they have not passed. Maintainer action remains outstanding.
Initial restricted-sandbox CLI attempts returned cleanup_incomplete because process-table inspection was denied; valid CLI/MCP runs subsequently verified REA's own worker cleanup. An intermediate AST-test type error was corrected before the final passing check. Full repository tests/coverage, installed-package distribution, Windows, external engines/providers, third-party clients and performance measurements remain unverified; no full interprocedural class-construction proof is claimed.

Compatibility, safety, and release

Breaking changes or migration steps: no API migration. Previously resolved ownership can become candidate plus unknown; default graph queries exclude candidate edges.
Conservative boundary: a stable var child used in another block, closure or loop may become partial when the limited ordering proof cannot establish initialization. Stable const/let captures and admitted same-body assignments remain resolved. Full interprocedural/control-flow modeling is not claimed.
Real Hopper/Ghidra, browser, or OS coverage: Linux x64 initial fix and macOS arm64 instance-field correction, both source-checkout static analysis via actual CLI/MCP. Installed-npm, Windows, native engines, browsers and third-party MCP clients were not verified.
Full repository tests/coverage were not run locally; relevant local tests and checks passed, with the full deterministic suite left to CI. CPU/RSS/token costs were not measured; no performance claim is made.
Package or release metadata impact: none.
Security, privacy, process, or containment review: no new target execution, network, mutation or authority; target source remains inert.

Author readiness

Human owner: @Xinglan233

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/3819 **Author:** [@Xinglan233](https://github.com/Xinglan233) **Created:** 10/15/2026 **Status:** ✅ Merged **Merged:** 10/16/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `fix/3362-child-handle-lifetimes` --- ### 📝 Commits (3) - [`2179d4f`](https://github.com/morluto/rea/commit/2179d4f3efd4248072b38378f249ae26852f5733) fix(javascript): resolve child ownership at receiver reads - [`d5b66f6`](https://github.com/morluto/rea/commit/d5b66f6a3b7f55576004435cf815590d2097d569) fix(javascript): preserve deferred field child candidates - [`6835cbe`](https://github.com/morluto/rea/commit/6835cbe3d20f0c69431370063cedfb473f72e167) fix(javascript): merge main into child ownership fix ### 📊 Changes **4 files changed** (+558 additions, -24 deletions) <details> <summary>View changed files</summary> ➕ `src/domain/javascript/javascriptSemanticChildProcesses.test.ts` (+263 -0) 📝 `src/domain/javascript/javascriptSemanticChildProcesses.ts` (+88 -23) 📝 `src/domain/javascript/javascriptSemanticMutationInitializers.test.ts` (+64 -1) ➕ `tests/acceptance/applications/javascriptChildHandleOwnership.test.ts` (+143 -0) </details> ### 📄 Description ## Summary Prevent replaced child handles from retaining resolved signal and exit-listener ownership of an earlier spawned child. Unproven links remain candidates with explicit child-process unknowns; unchanged children retain resolved ownership. Fixes #3362. Prepared with OpenAI Codex assistance. ## Problem and expected behavior ```js import { spawn } from "node:child_process"; let child = spawn("program"); child = { kill() {}, on() {} }; child.kill(); child.on("exit", () => {}); const stable = spawn("program"); stable.kill(); ``` The extractor matched receivers to the spawn inventory by binding ID and marked a unique match complete, even after replacement. A resolved relationship must identify the admitted receiver at its read; uniqueness in the historical inventory is insufficient. ## Change and scope Select receiver initializers at each read and follow direct aliases at their capture sites. Same-body replacement with a new spawn selects that child. Preserve uncertainty for unsupported replacements, conditional/cross-function writes, cycles, projections and defaults; retain the existing scalar awaited-spawn convention. Reuse the initializer owner's existing statement positions for ordering proof. Stable initialized lexical declarations can retain identity across captures; assignment-only and `var` sources require proven same-body ordering. Alias traversal is iterative. Existing graph projection emits candidate edges and unknowns without adapter-specific repairs. This is a routine static-analysis fix within the [contribution scope](https://github.com/morluto/rea/blob/main/CONTRIBUTING.md#contribution-scope); separate maintainer scope approval is not applicable. It is independent of timer PR #3794 and does not include stdio issue #3355. ## Contract and boundary impact - Semantic owner and earliest changed stage: domain child-process interaction recovery and shared initializer ordering; reads under deferred instance field values retain all candidate origins. - Contribution scope fit and linked maintainer decision: ordinary bug fix; not applicable. - CLI and MCP/tool-catalog contract: schemas and tool inventory unchanged; ownership resolution and existing unknowns are corrected in both adapters. - Provider, bridge, target-format, or platform compatibility: no provider or bridge changes; static JavaScript source analysis only. - Evidence, artifact, provenance, or reconstruction contract: preserve spawn identity and source ranges; unproven ownership becomes candidate plus explicit uncertainty. - Target selection, authority, upstream behavior, and cleanup: unchanged; analyzed target calls are never executed. - Generated metadata, package, or installation impact: none; no dependency or release metadata changes. Expected effects: - [x] 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. These describe the analysis. Verification starts REA's own CLI/MCP processes, not the fixture's processes. ## Evidence and regression coverage Executed on Linux x64, Node 24.18.0/npm 11.16.0. The exact issue fixture was run through the actual public CLI and stdio MCP; their complete Evidence matched at both baseline and fixed revisions. | Fixture observation | Baseline | Fixed | | ----------------------------------- | -------- | --------- | | line 4 signal → line 2 child | resolved | candidate | | line 2 child → line 5 exit listener | resolved | candidate | | line 7 signal → line 6 stable child | resolved | resolved | | child-process ownership unknowns | 0 | 2 | Baseline `af6a9980669129aae882c9216766fc8d9f544079`: 15 of the initial 16 domain regressions failed, and the public CLI acceptance regression failed on the stale resolved links. At that revision, the domain file had 22 regressions covering replacement ordering, new children, alias captures, conditional/compound/destructuring/update/loop writes, cross-function initialization, defaults, await, cycles, direct receivers and shadowing. Acceptance asserts endpoint lines, uncertainty, complete CLI/MCP Evidence equality and MCP responsiveness. Independent review findings were reproduced and corrected before final verification. Remaining proof gaps and conservative limits are listed below; full baseline/final Evidence and failed attempts are retained in the task handoff. For evidence-bearing changes: - [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 On commit `2179d4f3efd4248072b38378f249ae26852f5733`, based on main `4692bb33300c7cb77b095b6cc0c33da861c1b860`: - `npm run build:cached` — passed. - `npm run check` — passed, including typecheck, lint/module boundaries, formatting, knip, metadata and architecture guards; lint reported 0 errors with repository warnings retained. - `node node_modules/vitest/vitest.mjs run src/domain/javascript/ src/application/javascript/ tests/boundary/filesystem/referenceSourceShadowedRequire.test.ts` — 82 files, 2010 tests passed. - `npm run test:focused -- tests/acceptance/applications/javascriptChildHandleOwnership.test.ts tests/acceptance/applications/javascriptBuiltinDefaultsCli.test.ts` — 2 files, 7 tests passed. - `npm run docs:check` — passed, including 5 README conformance tests. - `npm run verify:test-discovery` — all 1303 files discovered exactly once. - Separate actual public CLI/stdio MCP run of the exact fixture — CLI exit 0, MCP success/ping, identical complete Evidence and corrected results shown above. - Patch application, incremental Git bundle integrity and `git diff --check` — passed. ### Instance-field review correction Current head: `d5b66f6a3b7f55576004435cf815590d2097d569`. Verified on macOS arm64 with Node 24.18.0/npm 11.16.0. The review fixture `let child = spawn('old'); class Stop { field = child.kill(); } child = spawn('new'); new Stop();` reproduced at `2179d4f3`: the field signal was resolved only to old, omitted new, and reported no ownership unknown. Instance field values execute on construction; the read was incorrectly ordered at class declaration. The shared ordering owner now reuses one deferred-field predicate for read ordering and top-level effect ordering. Both children remain candidates with one `ambiguous-target` unknown; static field values and computed keys retain class-definition ordering, and stable lexical captures stay resolved. - Before the production fix, 6 child-ownership regressions, 3 shared-ordering regressions and the new CLI/MCP acceptance failed for the reported behavior. Coverage includes public/private/accessor values, nested fields, class expressions, assignment-only initialization, static values, computed keys, methods/closures and stable bindings. Accessor coverage uses the shared Babel AST boundary; parser support was not expanded. - Relevant JavaScript domain/application and reference-import boundary tests: 82 files, 2031 tests passed. Final focused producer regressions after test-only refinements: 46 tests passed. - `npm run test:focused -- tests/acceptance/applications/javascriptChildHandleOwnership.test.ts tests/acceptance/applications/javascriptBuiltinDefaultsCli.test.ts`: 2 files, 8 tests passed. - `npm run check` passed, including typecheck, lint/module boundaries, formatting, knip, metadata and architecture guards; lint retained repository warnings with 0 errors. Runtime compilation, `npm run verify:test-discovery` (1303 files), normal pre-commit/pre-push checks, `git diff --check` and incremental bundle verification passed. - Separate actual public CLI/stdio MCP runs at the original and fixed heads returned successful results, identical complete Evidence across adapters and successful MCP ping. The instance-field signal changed from old-only resolved to old/new candidates plus explicit uncertainty; a stable-child control stayed resolved. Fixture spawn/kill calls were never executed. - GitHub CI ([run 38676037050](https://github.com/morluto/rea/actions/runs/38676037050)) and PR title ([run 38676037007](https://github.com/morluto/rea/actions/runs/38676037007)) for this head both report `action_required`, with no jobs executed; they have not passed. Maintainer action remains outstanding. - Initial restricted-sandbox CLI attempts returned `cleanup_incomplete` because process-table inspection was denied; valid CLI/MCP runs subsequently verified REA's own worker cleanup. An intermediate AST-test type error was corrected before the final passing check. Full repository tests/coverage, installed-package distribution, Windows, external engines/providers, third-party clients and performance measurements remain unverified; no full interprocedural class-construction proof is claimed. ## Compatibility, safety, and release - Breaking changes or migration steps: no API migration. Previously resolved ownership can become candidate plus unknown; default graph queries exclude candidate edges. - Conservative boundary: a stable `var` child used in another block, closure or loop may become partial when the limited ordering proof cannot establish initialization. Stable `const`/`let` captures and admitted same-body assignments remain resolved. Full interprocedural/control-flow modeling is not claimed. - Real Hopper/Ghidra, browser, or OS coverage: Linux x64 initial fix and macOS arm64 instance-field correction, both source-checkout static analysis via actual CLI/MCP. Installed-npm, Windows, native engines, browsers and third-party MCP clients were not verified. - Full repository tests/coverage were not run locally; relevant local tests and checks passed, with the full deterministic suite left to CI. CPU/RSS/token costs were not measured; no performance claim is made. - Package or release metadata impact: none. - Security, privacy, process, or containment review: no new target execution, network, mutation or authority; target source remains inert. ## Author readiness Human owner: @Xinglan233 - [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

2026-10-31 01:10:28 +00:00

closed this issue
added the
pull-request
label

bsakaguchi referenced this issue

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

[GH-ISSUE #8430] [Bug] Snapshot refusal does not identify which binding differs #6603

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

No description provided.

Rows
Columns