[PR #6280] [MERGED] fix(javascript): read utilityProcess.fork options from the argv slot #7253

New issue

Closed

opened 2026-11-01 21:46:30 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

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

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/6280
Author: @kaoru0822-kitauji
Created: 10/22/2026
Status: ✅ Merged
Merged: 10/22/2026
Merged by: @kaoru0822-kitauji

Base: main ← Head: fix/6208-utility-fork-options

📝 Commits (1)

38b2cc8 fix(javascript): read utilityProcess.fork options from the argv slot

📊 Changes

7 files changed (+126 additions, -36 deletions)

View changed files

📝 src/domain/javascript/electronStaticAnalysisBrowser.ts (+21 -6)
📝 src/domain/javascript/javascriptSemanticChildProcesses.ts (+9 -14)
📝 src/domain/javascript/javascriptSemanticExpressionShapes.ts (+28 -0)
📝 src/domain/javascript/javascriptStaticAnalysis.mapEndpoints.test.ts (+2 -0)
📝 src/domain/javascript/javascriptStaticAnalysis.ts (+2 -0)
📝 src/domain/javascript/javascriptStaticAnalysisState.ts (+3 -0)
📝 tests/boundary/filesystem/javascript-applications/electronOptionKeys.test.ts (+61 -16)

📄 Description

What broke: utilityProcess.fork("./worker.js", { serviceName: "gpu" }) reported service_name: null with exact confidence and complete coverage, because the extractor only read the third argument.
Fix: The options-selection rule (target[, argv][, options]) that child_process already used now lives in the shared expression-shape owner. Utility processes use it too, so second-slot options are read, and argv shapes that can't be proven stay unknown.
Risk: Low. The change is limited to the static utility-process extractor and a behavior-preserving refactor of child_process option selection. fork(path, argsVar, opts) where argsVar can't be proven to be an array now reports unknown (partial) instead of a name that may be wrong.

Summary

Fixes #6208

Electron's utilityProcess.fork treats a non-array object in the args slot as the options and then ignores the third argument. REA's static extractor always read argument three. As a result, the second-slot form lost its declared service name while still reporting complete coverage. The extractor also had related errors: it read a name from an ignored third argument, and it missed unknown options supplied through spreads or identifiers.

Problem and expected behavior

Trigger: utilityProcess.fork("./worker.js", { serviceName: "worker-gpu" }).
Violated invariant: an observed declared value must be preserved. If the value can't be interpreted, it must remain unknown with partial coverage (unknowns contract). Before this fix it was reported as absent with complete coverage.
New behavior: the options come from the slot Electron uses, and a value that can't be proven stays unknown.

Change and scope

optionsAfterOptionalArgv (in javascriptSemanticExpressionShapes.ts) is now the single owner of the (target[, argv][, options]) options-selection rule:
- The third slot is read only when the second argument is a proven argv array or a nullish placeholder (null, void 0, an unshadowed undefined).
- Otherwise the second argument is the options.
- Spreads are returned as-is, so callers keep them unknown.
- The result is unwrapped, so TS/parenthesized wrappers are transparent.
javascriptSemanticChildProcesses.ts now calls this helper instead of its inline copy. Behavior is unchanged; exec keeps its argv-free signature.
The static analysis finding context gets the shared createSemanticExpressionShapeResolver(bindings), so the Electron inspector reuses the same bounded argv and nullish proofs. The resolver is lazy and only walks the program when a named binding has to be proven.
inspectUtilityProcess now selects options through the helper. Nullish options and serviceName: null/undefined declare no name, matching Electron's options.serviceName != null check.
Not included:
- Resolving identifier option objects (const opts = {...}; fork(p, opts)). It stays unknown, as it did before when used as the third argument.
- Adding a separate per-observation "unknown" status field for service_name. Unknown is still reported through file-level partial coverage and the existing limitation, the same as the existing computed-key case.
Scope: an ordinary fix within existing static-analysis boundaries; no scope decision needed.

Contract and boundary impact

Semantic owner and earliest changed stage: optionsAfterOptionalArgv in src/domain/javascript/javascriptSemanticExpressionShapes.ts, consumed during the static Electron finding traversal.
Contribution scope fit and linked maintainer decision: not applicable (bug fix within the existing extractor).
CLI and MCP/tool-catalog contract: no schema change. service_name, label and coverage values change only for the affected call shapes.
Provider, bridge, target-format, or platform compatibility: none.
Evidence, artifact, provenance, or reconstruction contract: affected forms gain the correct name, or move from false "complete" to "partial". fork(p, opaqueArgs, {serviceName}) moves from exact name to unknown/partial, because the runtime may use opaqueArgs as the options.
Target selection, authority, upstream behavior, and cleanup details: not applicable.
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:
- tests/boundary/filesystem/javascript-applications/electronOptionKeys.test.ts now has a table of 19 utility-process argument shapes run through the shared application reconstruction workflow:
  - second-slot options, [], undefined, null, void 0, and a const argv array
  - wrapped options, and an ignored third argument
  - omitted options, options without a name, nullish options, a nullish name
  - identifier options, spread arguments, spread options, an unproven argv call
  - computed keys in both slots, and a shadowed undefined
- The earlier single computed-key test is folded into this table. Its observations[0] selection depended on observation ordering, so the table selects the observe-utility-process-fork observation by operation.
- I didn't run the new table on the base revision. Based on the baseline CLI matrix below, the second-slot, ignored-third, wrapped, identifier, spread, computed-key and unproven-argv rows would fail there.
- javascriptStaticAnalysis.mapEndpoints.test.ts supplies the new context field.
Base reproduction (executed, CLI rea analyze-javascript-application --format json on tiny authored Electron source directories; Electron was not launched). Projection: [service_name, confidence, coverage] of the observe-utility-process-fork observation:

fixture call args	main (`c537c52c9`)	this PR
`{ serviceName: "worker-gpu" }`	`null, exact, complete`	`"worker-gpu", exact, complete`
`[], { serviceName: "worker-gpu" }`	`"worker-gpu", exact, complete`	unchanged
`undefined, {…}` / `null, {…}`	`"worker-gpu", complete`	unchanged
omitted	`null, complete`	unchanged
`{ serviceName: "a" }, { serviceName: "b" }`	`"b", complete`	`"a", complete`
`({ serviceName: "worker-gpu" })`	`null, complete`	`"worker-gpu", complete`
`opts` (identifier)	`null, complete`	`null, partial`
`...rest`	`null, complete`	`null, partial`
`{ [k]: "worker-gpu" }`	`null, complete`	`null, partial`
`getArgs(), { serviceName: "worker-gpu" }`	`"worker-gpu", complete`	`null, partial`
shadowed `undefined`, `{…}`	`"worker-gpu", complete`	`null, partial`
`[], { serviceName: null }`	`null, partial`	`null, complete`

User-visible CLI/MCP output: shown in the table above (CLI). MCP uses the same application workflow; the MCP adapter itself was not rerun.
Remaining proof gaps:
- A top-level const args = [...] in a plain .js script without ES module syntax is classified as runtime context unknown. The shared shape resolver deliberately does not prove such bindings, so fork(p, args, {serviceName}) there now reports unknown/partial. Previously it reported the name, without checking what args actually is. Function-local or ES-module const argv arrays and inline [] resolve as before. Relaxing that resolver policy would also change child_process analysis, so it is out of scope here.
- Identifier option objects are not resolved; they remain unknown.

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

Local Node v26.5.0. The pinned 24.18.0 runtime was not available on this host.

CLI repro matrix above on the base and fixed builds (npm run build:cached, then rea analyze-javascript-application <fixture> --format json): results as tabulated.
npm run test:focused -- tests/boundary/filesystem/javascript-applications/electronOptionKeys.test.ts src/domain/javascript/electronStaticAnalysisOptions.test.ts src/domain/javascript/childProcessOptionsRegression.test.ts src/domain/javascript/javascriptSemanticChildProcesses.test.ts src/domain/javascript/javascriptStaticAnalysis.mapEndpoints.test.ts src/domain/javascript/javascriptStaticAnalysisCalleeBoundary.test.ts src/domain/javascript/javascriptSemanticExpressionShapes.test.ts tests/boundary/filesystem/javascript-applications/electronApplicationAnalysis.test.ts: 8 files, 302 tests passed.
npm run test:focused -- on the other utilityProcess-related suites (javascriptApplicationTypeScript, the javascriptStaticPath* tests, electronStaticAnalysisIdentity, javascriptStaticAnalysisSharedHelperWrites, javascriptStaticAnalysisDelegateEligibility, javascriptSemanticAnalysis.runtime_boundaries, tests/boundary/cli/javascriptSelfImportRelationships, tests/acceptance/applications/javascriptLiteralPercentPaths, plus electronOptionKeys): 13 files, 328 tests passed.
After rebasing onto c9b4cae9d: the 8-file focused set above was rerun (302 passed), and npm run check:fast and npm run format:check were rerun (both passed). The CLI matrix and the 13-file set ran on the pre-rebase base c537c52c9.
npm run check:fast: passed (typecheck and lint).
npm run format:check: passed.

Compatibility, safety, and release

Breaking changes or migration steps: none. Output schema is unchanged; values change only for the call shapes above.
Real Hopper/Ghidra, browser, or OS coverage: not applicable (static source analysis only; macOS arm64 local run).
Package or release metadata impact: none.
Security, privacy, process, or containment review: source is parsed as inert data; nothing is executed.

Author readiness

Human owner: @kaoru0822-kitauji

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.

🤖 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/6280 **Author:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Created:** 10/22/2026 **Status:** ✅ Merged **Merged:** 10/22/2026 **Merged by:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Base:** `main` ← **Head:** `fix/6208-utility-fork-options` --- ### 📝 Commits (1) - [`38b2cc8`](https://github.com/morluto/rea/commit/38b2cc8181c778937dfd0f426da79069da6aa0f5) fix(javascript): read utilityProcess.fork options from the argv slot ### 📊 Changes **7 files changed** (+126 additions, -36 deletions) <details> <summary>View changed files</summary> 📝 `src/domain/javascript/electronStaticAnalysisBrowser.ts` (+21 -6) 📝 `src/domain/javascript/javascriptSemanticChildProcesses.ts` (+9 -14) 📝 `src/domain/javascript/javascriptSemanticExpressionShapes.ts` (+28 -0) 📝 `src/domain/javascript/javascriptStaticAnalysis.mapEndpoints.test.ts` (+2 -0) 📝 `src/domain/javascript/javascriptStaticAnalysis.ts` (+2 -0) 📝 `src/domain/javascript/javascriptStaticAnalysisState.ts` (+3 -0) 📝 `tests/boundary/filesystem/javascript-applications/electronOptionKeys.test.ts` (+61 -16) </details> ### 📄 Description **What broke:** `utilityProcess.fork("./worker.js", { serviceName: "gpu" })` reported `service_name: null` with exact confidence and complete coverage, because the extractor only read the third argument. **Fix:** The options-selection rule `(target[, argv][, options])` that child_process already used now lives in the shared expression-shape owner. Utility processes use it too, so second-slot options are read, and argv shapes that can't be proven stay unknown. **Risk:** Low. The change is limited to the static utility-process extractor and a behavior-preserving refactor of child_process option selection. `fork(path, argsVar, opts)` where `argsVar` can't be proven to be an array now reports unknown (partial) instead of a name that may be wrong. ## Summary Fixes #6208 Electron's [`utilityProcess.fork`](https://github.com/electron/electron/blob/v43.0.0/lib/browser/api/utility-process.ts#L18-L31) treats a non-array object in the `args` slot as the options and then ignores the third argument. REA's static extractor always read argument three. As a result, the second-slot form lost its declared service name while still reporting complete coverage. The extractor also had related errors: it read a name from an ignored third argument, and it missed unknown options supplied through spreads or identifiers. ## Problem and expected behavior - Trigger: `utilityProcess.fork("./worker.js", { serviceName: "worker-gpu" })`. - Violated invariant: an observed declared value must be preserved. If the value can't be interpreted, it must remain unknown with partial coverage ([unknowns contract](https://github.com/morluto/rea/blob/main/docs/javascript-artifact-reconstruction.md#authority-and-unknowns)). Before this fix it was reported as absent with complete coverage. - New behavior: the options come from the slot Electron uses, and a value that can't be proven stays unknown. ## Change and scope - `optionsAfterOptionalArgv` (in `javascriptSemanticExpressionShapes.ts`) is now the single owner of the `(target[, argv][, options])` options-selection rule: - The third slot is read only when the second argument is a proven argv array or a nullish placeholder (`null`, `void 0`, an unshadowed `undefined`). - Otherwise the second argument is the options. - Spreads are returned as-is, so callers keep them unknown. - The result is unwrapped, so TS/parenthesized wrappers are transparent. - `javascriptSemanticChildProcesses.ts` now calls this helper instead of its inline copy. Behavior is unchanged; `exec` keeps its argv-free signature. - The static analysis finding context gets the shared `createSemanticExpressionShapeResolver(bindings)`, so the Electron inspector reuses the same bounded argv and nullish proofs. The resolver is lazy and only walks the program when a named binding has to be proven. - `inspectUtilityProcess` now selects options through the helper. Nullish options and `serviceName: null/undefined` declare no name, matching Electron's `options.serviceName != null` check. - Not included: - Resolving identifier option objects (`const opts = {...}; fork(p, opts)`). It stays unknown, as it did before when used as the third argument. - Adding a separate per-observation "unknown" status field for `service_name`. Unknown is still reported through file-level partial coverage and the existing limitation, the same as the existing computed-key case. - Scope: an ordinary fix within existing static-analysis boundaries; no scope decision needed. ## Contract and boundary impact - Semantic owner and earliest changed stage: `optionsAfterOptionalArgv` in `src/domain/javascript/javascriptSemanticExpressionShapes.ts`, consumed during the static Electron finding traversal. - Contribution scope fit and linked maintainer decision: not applicable (bug fix within the existing extractor). - CLI and MCP/tool-catalog contract: no schema change. `service_name`, `label` and coverage values change only for the affected call shapes. - Provider, bridge, target-format, or platform compatibility: none. - Evidence, artifact, provenance, or reconstruction contract: affected forms gain the correct name, or move from false "complete" to "partial". `fork(p, opaqueArgs, {serviceName})` moves from exact name to unknown/partial, because the runtime may use `opaqueArgs` as the options. - Target selection, authority, upstream behavior, and cleanup details: not applicable. - 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: - `tests/boundary/filesystem/javascript-applications/electronOptionKeys.test.ts` now has a table of 19 utility-process argument shapes run through the shared application reconstruction workflow: - second-slot options, `[]`, `undefined`, `null`, `void 0`, and a `const` argv array - wrapped options, and an ignored third argument - omitted options, options without a name, nullish options, a nullish name - identifier options, spread arguments, spread options, an unproven argv call - computed keys in both slots, and a shadowed `undefined` - The earlier single computed-key test is folded into this table. Its `observations[0]` selection depended on observation ordering, so the table selects the `observe-utility-process-fork` observation by operation. - I didn't run the new table on the base revision. Based on the baseline CLI matrix below, the second-slot, ignored-third, wrapped, identifier, spread, computed-key and unproven-argv rows would fail there. - `javascriptStaticAnalysis.mapEndpoints.test.ts` supplies the new context field. - Base reproduction (executed, CLI `rea analyze-javascript-application --format json` on tiny authored Electron source directories; Electron was not launched). Projection: `[service_name, confidence, coverage]` of the `observe-utility-process-fork` observation: | fixture call args | main (c537c52c9) | this PR | | --- | --- | --- | | `{ serviceName: "worker-gpu" }` | `null, exact, complete` | `"worker-gpu", exact, complete` | | `[], { serviceName: "worker-gpu" }` | `"worker-gpu", exact, complete` | unchanged | | `undefined, {…}` / `null, {…}` | `"worker-gpu", complete` | unchanged | | omitted | `null, complete` | unchanged | | `{ serviceName: "a" }, { serviceName: "b" }` | `"b", complete` | `"a", complete` | | `({ serviceName: "worker-gpu" })` | `null, complete` | `"worker-gpu", complete` | | `opts` (identifier) | `null, complete` | `null, partial` | | `...rest` | `null, complete` | `null, partial` | | `{ [k]: "worker-gpu" }` | `null, complete` | `null, partial` | | `getArgs(), { serviceName: "worker-gpu" }` | `"worker-gpu", complete` | `null, partial` | | shadowed `undefined`, `{…}` | `"worker-gpu", complete` | `null, partial` | | `[], { serviceName: null }` | `null, partial` | `null, complete` | - User-visible CLI/MCP output: shown in the table above (CLI). MCP uses the same application workflow; the MCP adapter itself was not rerun. - Remaining proof gaps: - A top-level `const args = [...]` in a plain `.js` script without ES module syntax is classified as runtime context `unknown`. The shared shape resolver deliberately does not prove such bindings, so `fork(p, args, {serviceName})` there now reports unknown/partial. Previously it reported the name, without checking what `args` actually is. Function-local or ES-module `const` argv arrays and inline `[]` resolve as before. Relaxing that resolver policy would also change child_process analysis, so it is out of scope here. - Identifier option objects are not resolved; they remain unknown. 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 Local Node v26.5.0. The pinned 24.18.0 runtime was not available on this host. - CLI repro matrix above on the base and fixed builds (`npm run build:cached`, then `rea analyze-javascript-application <fixture> --format json`): results as tabulated. - `npm run test:focused -- tests/boundary/filesystem/javascript-applications/electronOptionKeys.test.ts src/domain/javascript/electronStaticAnalysisOptions.test.ts src/domain/javascript/childProcessOptionsRegression.test.ts src/domain/javascript/javascriptSemanticChildProcesses.test.ts src/domain/javascript/javascriptStaticAnalysis.mapEndpoints.test.ts src/domain/javascript/javascriptStaticAnalysisCalleeBoundary.test.ts src/domain/javascript/javascriptSemanticExpressionShapes.test.ts tests/boundary/filesystem/javascript-applications/electronApplicationAnalysis.test.ts`: 8 files, 302 tests passed. - `npm run test:focused --` on the other utilityProcess-related suites (`javascriptApplicationTypeScript`, the `javascriptStaticPath*` tests, `electronStaticAnalysisIdentity`, `javascriptStaticAnalysisSharedHelperWrites`, `javascriptStaticAnalysisDelegateEligibility`, `javascriptSemanticAnalysis.runtime_boundaries`, `tests/boundary/cli/javascriptSelfImportRelationships`, `tests/acceptance/applications/javascriptLiteralPercentPaths`, plus `electronOptionKeys`): 13 files, 328 tests passed. - After rebasing onto c9b4cae9d: the 8-file focused set above was rerun (302 passed), and `npm run check:fast` and `npm run format:check` were rerun (both passed). The CLI matrix and the 13-file set ran on the pre-rebase base c537c52c9. - `npm run check:fast`: passed (typecheck and lint). - `npm run format:check`: passed. ## Compatibility, safety, and release - Breaking changes or migration steps: none. Output schema is unchanged; values change only for the call shapes above. - Real Hopper/Ghidra, browser, or OS coverage: not applicable (static source analysis only; macOS arm64 local run). - Package or release metadata impact: none. - Security, privacy, process, or containment review: source is parsed as inert data; nothing is executed. ## Author readiness Human owner: @kaoru0822-kitauji - [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. 🤖 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

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