morluto/rea

Fork 0

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

[PR #3433] feat(javascript): recover synchronous child-process creation #5742

New issue

Open

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

bsakaguchi commented

2026-10-31 01:09:06 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/3433
Author: @MartOcd1709
Created: 10/14/2026
Status: 🔄 Open

Base: main ← Head: feat/sync-child-process

📝 Commits (2)

0c9b5b6 feat(javascript): recover synchronous child-process creation
3b8d050 docs(javascript): describe synchronous child-process coverage in graph limitations

📊 Changes

5 files changed (+113 additions, -8 deletions)

View changed files

📝 src/application/javascript/JavaScriptSemanticGraphBuilder.ts (+1 -1)
➕ src/domain/javascript/childProcessSyncCreation.test.ts (+74 -0)
📝 src/domain/javascript/javascriptSemanticChildProcesses.ts (+20 -4)
📝 src/domain/javascript/javascriptSemanticDataEffectHelpers.ts (+9 -1)
📝 src/domain/javascript/javascriptSemanticIr.ts (+9 -2)

📄 Description

Summary

Recover execSync, spawnSync and execFileSync as child-process facts. Today synchronous command execution produces no child-process node, so the commands are invisible in the semantic graph.

Refs #3431. This is a draft pending the scope question there, because open PR #2041 currently asserts the opposite behavior; see below.

Problem and expected behavior

CHILD_PROCESS_METHODS contains only spawn, exec, execFile and fork. For:

import { execSync } from "node:child_process";
execSync("id");

ir.childProcessSpawns is [] on main. Expected: one candidate with method: "execSync", command: "id". Synchronous calls return completed output, not a ChildProcess, so const out = execSync(…); out.on("exit", …) must not link a listener or signal.

Change and scope

Add the three methods to CHILD_PROCESS_METHODS, the IR method union and BUILTIN_RECEIVER_METHODS.child_process, so the namespace form cp.execSync() resolves.
execSync(command, options) follows exec: options come from args[1], and argvCount stays null. spawnSync and execFileSync follow spawn/execFile argument positions.
Sync candidates are excluded from interaction linking.
Not included: fs writes (scope question in #3431), CommonJS require resolution (#2041), dynamic import() destructuring, class-field aliases.

Conflict note: #2041 adds these methods to BUILTIN_RECEIVER_METHODS, and its test retains asynchronous child effects alongside native %s expects cp.execSync(...) to produce no spawn. If the maintainer decision favors recovering sync processes, I'll rebase on #2041 and update that expectation. Otherwise this PR can be closed.

Follow the contribution scope. This extends an existing static extractor within its family; I'm awaiting confirmation in #3431.

Contract and boundary impact

Semantic owner and earliest changed stage: domain JavaScript semantic IR (javascriptSemanticChildProcesses.ts candidate collection).
Contribution scope fit and linked maintainer decision: pending in #3431.
CLI and MCP/tool-catalog contract: no tool or schema-shape change. child-process node method gains three string values. (Unknown: whether any consumer exhaustively matches method. tsc --noEmit found none.)
Provider, bridge, target-format, or platform compatibility: none.
Evidence, artifact, provenance, or reconstruction contract: more child-process nodes and spawns relations for sync call sites. Fingerprint effect sets may now include child-process for affected functions.
Target selection, authority, upstream behavior, and cleanup details: not applicable (static).
Generated metadata, package, or installation impact: none.

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.

Evidence and regression coverage

Tests added or updated: src/domain/javascript/childProcessSyncCreation.test.ts.
1. Named and namespace imports with options, argv and stdio.
2. No listener linking for sync results.
Base reproduction: on main, both tests fail (expected [] to deeply equal [ 'execSync', 'spawnSync' ]). On this branch they pass.
User-visible CLI output, executed. 15 standalone files with sync calls, taken from 6 popular npm MCP server packages (npm pack, never executed), each analyzed alone because large packages hit semantic_graph_file_node_ceiling:

sync call sites recovered child-process (method *Sync)

rea-agents 6.4.0 15 0

this branch 15 11

Examples now recovered:
- chrome-devtools-mcp build/src/BrowserManager.js, build/src/bin/check-latest-version.js
- @heroku/mcp-server dist/tools/deploy-to-heroku.js, dist/utils/tarball.js
- @browsermcp/mcp dist/index.js
- mcp-server-kubernetes dist/security/kubectl-flags.js
- @wonderwhy-er/desktop-commander dist/utils/system-info.js
The 4 misses are await import("child_process") destructuring (2) and static spawnSync = spawnSync class-field aliases (2). Async methods in those files are missed the same way.
Remaining proof gaps: the full npm test and real-provider lanes were not run (not affected). Token and CPU cost were not measured; added nodes are bounded by sync call sites.

	sync call sites	recovered `child-process` (method `*Sync`)
rea-agents 6.4.0	15	0
this branch	15	11

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

npx vitest run src/domain/javascript/childProcessSyncCreation.test.ts src/domain/javascript/childProcessOptionsRegression.test.ts: 5 passed.
npx vitest run src/domain/javascript src/application/javascript: 80 files, 1931 tests passed (after rebasing on current main).
npx tsc --noEmit: exit 0.
npx oxfmt --check on touched files: clean.
npx oxlint on touched files: no new findings (4 existing warnings in untouched functions).
npm run verify:test-discovery: "Discovered all 1268 test files exactly once." (run before rebase)

Compatibility, safety, and release

Breaking changes or migration steps: none intended. The method union widens.
Real Hopper/Ghidra, browser, or OS coverage: not applicable.
Package or release metadata impact: none.
Security, privacy, process, or containment review: static only. No execution, and no secrecy inference.

Author readiness

Human owner: @MartOcd1709

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. (The semantic-graph limitation text in JavaScriptSemanticGraphBuilder.ts now says sync creation is covered and listeners/signals apply only to async handles. src/application/javascript: 299/299 pass.)
User-visible CLI/MCP changes include representative output.
I checked the final diff for secrets, unrelated cleanup, and unsupported claims.

🤖 Generated with Claude Code

_{🔄 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/3433 **Author:** [@MartOcd1709](https://github.com/MartOcd1709) **Created:** 10/14/2026 **Status:** 🔄 Open **Base:** `main` ← **Head:** `feat/sync-child-process` --- ### 📝 Commits (2) - [`0c9b5b6`](https://github.com/morluto/rea/commit/0c9b5b65dde2e0e3973a8fdcb7a0186bc9f13637) feat(javascript): recover synchronous child-process creation - [`3b8d050`](https://github.com/morluto/rea/commit/3b8d05026fd226027bdd5ab79eda4a0188264c0e) docs(javascript): describe synchronous child-process coverage in graph limitations ### 📊 Changes **5 files changed** (+113 additions, -8 deletions) <details> <summary>View changed files</summary> 📝 `src/application/javascript/JavaScriptSemanticGraphBuilder.ts` (+1 -1) ➕ `src/domain/javascript/childProcessSyncCreation.test.ts` (+74 -0) 📝 `src/domain/javascript/javascriptSemanticChildProcesses.ts` (+20 -4) 📝 `src/domain/javascript/javascriptSemanticDataEffectHelpers.ts` (+9 -1) 📝 `src/domain/javascript/javascriptSemanticIr.ts` (+9 -2) </details> ### 📄 Description ## Summary Recover `execSync`, `spawnSync` and `execFileSync` as `child-process` facts. Today synchronous command execution produces no child-process node, so the commands are invisible in the semantic graph. Refs #3431. This is a draft pending the scope question there, because open PR #2041 currently asserts the opposite behavior; see below. ## Problem and expected behavior `CHILD_PROCESS_METHODS` contains only `spawn`, `exec`, `execFile` and `fork`. For: ```js import { execSync } from "node:child_process"; execSync("id"); ``` `ir.childProcessSpawns` is `[]` on main. Expected: one candidate with `method: "execSync"`, `command: "id"`. Synchronous calls return completed output, not a `ChildProcess`, so `const out = execSync(…); out.on("exit", …)` must not link a listener or signal. ## Change and scope - Add the three methods to `CHILD_PROCESS_METHODS`, the IR `method` union and `BUILTIN_RECEIVER_METHODS.child_process`, so the namespace form `cp.execSync()` resolves. - `execSync(command, options)` follows `exec`: options come from `args[1]`, and `argvCount` stays `null`. `spawnSync` and `execFileSync` follow `spawn`/`execFile` argument positions. - Sync candidates are excluded from interaction linking. - **Not included:** fs writes (scope question in #3431), CommonJS `require` resolution (#2041), dynamic `import()` destructuring, class-field aliases. **Conflict note:** #2041 adds these methods to `BUILTIN_RECEIVER_METHODS`, and its test `retains asynchronous child effects alongside native %s` expects `cp.execSync(...)` to produce no spawn. If the maintainer decision favors recovering sync processes, I'll rebase on #2041 and update that expectation. Otherwise this PR can be closed. Follow the [contribution scope](https://github.com/morluto/rea/blob/main/CONTRIBUTING.md#contribution-scope). This extends an existing static extractor within its family; I'm awaiting confirmation in #3431. ## Contract and boundary impact - Semantic owner and earliest changed stage: domain JavaScript semantic IR (`javascriptSemanticChildProcesses.ts` candidate collection). - Contribution scope fit and linked maintainer decision: pending in #3431. - CLI and MCP/tool-catalog contract: no tool or schema-shape change. `child-process` node `method` gains three string values. (Unknown: whether any consumer exhaustively matches `method`. `tsc --noEmit` found none.) - Provider, bridge, target-format, or platform compatibility: none. - Evidence, artifact, provenance, or reconstruction contract: more `child-process` nodes and `spawns` relations for sync call sites. Fingerprint effect sets may now include `child-process` for affected functions. - Target selection, authority, upstream behavior, and cleanup details: not applicable (static). - Generated metadata, package, or installation impact: none. 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. ## Evidence and regression coverage - **Tests added or updated:** `src/domain/javascript/childProcessSyncCreation.test.ts`. 1. Named and namespace imports with options, argv and stdio. 2. No listener linking for sync results. - **Base reproduction:** on main, both tests fail (`expected [] to deeply equal [ 'execSync', 'spawnSync' ]`). On this branch they pass. - **User-visible CLI output, executed.** 15 standalone files with sync calls, taken from 6 popular npm MCP server packages (`npm pack`, never executed), each analyzed alone because large packages hit `semantic_graph_file_node_ceiling`: | | sync call sites | recovered `child-process` (method `*Sync`) | |---|---|---| | rea-agents 6.4.0 | 15 | 0 | | this branch | 15 | 11 | Examples now recovered: - `chrome-devtools-mcp` `build/src/BrowserManager.js`, `build/src/bin/check-latest-version.js` - `@heroku/mcp-server` `dist/tools/deploy-to-heroku.js`, `dist/utils/tarball.js` - `@browsermcp/mcp` `dist/index.js` - `mcp-server-kubernetes` `dist/security/kubectl-flags.js` - `@wonderwhy-er/desktop-commander` `dist/utils/system-info.js` The 4 misses are `await import("child_process")` destructuring (2) and `static spawnSync = spawnSync` class-field aliases (2). Async methods in those files are missed the same way. - **Remaining proof gaps:** the full `npm test` and real-provider lanes were not run (not affected). Token and CPU cost were not measured; added nodes are bounded by sync call sites. 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 - `npx vitest run src/domain/javascript/childProcessSyncCreation.test.ts src/domain/javascript/childProcessOptionsRegression.test.ts`: 5 passed. - `npx vitest run src/domain/javascript src/application/javascript`: 80 files, 1931 tests passed (after rebasing on current main). - `npx tsc --noEmit`: exit 0. - `npx oxfmt --check` on touched files: clean. - `npx oxlint` on touched files: no new findings (4 existing warnings in untouched functions). - `npm run verify:test-discovery`: "Discovered all 1268 test files exactly once." (run before rebase) ## Compatibility, safety, and release - Breaking changes or migration steps: none intended. The `method` union widens. - Real Hopper/Ghidra, browser, or OS coverage: not applicable. - Package or release metadata impact: none. - Security, privacy, process, or containment review: static only. No execution, and no secrecy inference. ## Author readiness Human owner: @MartOcd1709 - [ ] 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 - [x] 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. - [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. (The semantic-graph limitation text in `JavaScriptSemanticGraphBuilder.ts` now says sync creation is covered and listeners/signals apply only to async handles. `src/application/javascript`: 299/299 pass.) - [ ] User-visible CLI/MCP changes include representative output. - [x] I checked the final diff for secrets, unrelated cleanup, and unsupported claims. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- <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-10-31 01:09:06 +00:00

bsakaguchi referenced this issue

2026-11-01 21:46:00 +00:00

[PR #6180] [CLOSED] docs(errors): correct stale capability-unknown and comparison-rationale rows #7188