Repository navigation
fix(javascript): read utilityProcess.fork options from the argv slot - #6280
Conversation
Electron's utilityProcess.fork(modulePath[, args][, options]) treats a
non-array object in the second argument as the options and ignores the
third. The static extractor only read the third argument, so
`fork("./worker.js", { serviceName })` lost the declared name while
reporting exact, complete coverage, and an object in the second slot let
an ignored third argument supply the name.
Move the optional-argv options selection that child_process already used
into the shared expression-shape owner and reuse it for utility
processes. Unproven argv shapes, spreads and identifier options remain
unknown with partial coverage; nullish options or a nullish serviceName
declare no name, matching Electron's runtime checks.
Fixes #6208
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
[Medium impact] The PR appears safe to merge; no actionable defects were found. SummaryThe PR fixes how
Acknowledged limits:
Reviews (1) · Last reviewed commit: "fix(javascript): read utilityProcess.for..." · Reviewed by Greptile |
What broke:
utilityProcess.fork("./worker.js", { serviceName: "gpu" })reportedservice_name: nullwith 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)whereargsVarcan'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.forktreats a non-array object in theargsslot 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
utilityProcess.fork("./worker.js", { serviceName: "worker-gpu" }).Change and scope
optionsAfterOptionalArgv(injavascriptSemanticExpressionShapes.ts) is now the single owner of the(target[, argv][, options])options-selection rule:null,void 0, an unshadowedundefined).javascriptSemanticChildProcesses.tsnow calls this helper instead of its inline copy. Behavior is unchanged;execkeeps its argv-free signature.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.inspectUtilityProcessnow selects options through the helper. Nullish options andserviceName: null/undefineddeclare no name, matching Electron'soptions.serviceName != nullcheck.const opts = {...}; fork(p, opts)). It stays unknown, as it did before when used as the third argument.service_name. Unknown is still reported through file-level partial coverage and the existing limitation, the same as the existing computed-key case.Contract and boundary impact
optionsAfterOptionalArgvinsrc/domain/javascript/javascriptSemanticExpressionShapes.ts, consumed during the static Electron finding traversal.service_name,labeland coverage values change only for the affected call shapes.fork(p, opaqueArgs, {serviceName})moves from exact name to unknown/partial, because the runtime may useopaqueArgsas the options.Expected effects:
Evidence and regression coverage
tests/boundary/filesystem/javascript-applications/electronOptionKeys.test.tsnow has a table of 19 utility-process argument shapes run through the shared application reconstruction workflow:[],undefined,null,void 0, and aconstargv arrayundefinedobservations[0]selection depended on observation ordering, so the table selects theobserve-utility-process-forkobservation by operation.javascriptStaticAnalysis.mapEndpoints.test.tssupplies the new context field.rea analyze-javascript-application --format jsonon tiny authored Electron source directories; Electron was not launched). Projection:[service_name, confidence, coverage]of theobserve-utility-process-forkobservation:{ serviceName: "worker-gpu" }null, exact, complete"worker-gpu", exact, complete[], { serviceName: "worker-gpu" }"worker-gpu", exact, completeundefined, {…}/null, {…}"worker-gpu", completenull, complete{ serviceName: "a" }, { serviceName: "b" }"b", complete"a", complete({ serviceName: "worker-gpu" })null, complete"worker-gpu", completeopts(identifier)null, completenull, partial...restnull, completenull, partial{ [k]: "worker-gpu" }null, completenull, partialgetArgs(), { serviceName: "worker-gpu" }"worker-gpu", completenull, partialundefined,{…}"worker-gpu", completenull, partial[], { serviceName: null }null, partialnull, completeconst args = [...]in a plain.jsscript without ES module syntax is classified as runtime contextunknown. The shared shape resolver deliberately does not prove such bindings, sofork(p, args, {serviceName})there now reports unknown/partial. Previously it reported the name, without checking whatargsactually is. Function-local or ES-moduleconstargv arrays and inline[]resolve as before. Relaxing that resolver policy would also change child_process analysis, so it is out of scope here.For evidence-bearing changes:
Validation performed
Local Node v26.5.0. The pinned 24.18.0 runtime was not available on this host.
npm run build:cached, thenrea 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, thejavascriptStaticPath*tests,electronStaticAnalysisIdentity,javascriptStaticAnalysisSharedHelperWrites,javascriptStaticAnalysisDelegateEligibility,javascriptSemanticAnalysis.runtime_boundaries,tests/boundary/cli/javascriptSelfImportRelationships,tests/acceptance/applications/javascriptLiteralPercentPaths, pluselectronOptionKeys): 13 files, 328 tests passed.npm run check:fastandnpm run format:checkwere rerun (both passed). The CLI matrix and the 13-file set ran on the pre-rebase base c537c52.npm run check:fast: passed (typecheck and lint).npm run format:check: passed.Compatibility, safety, and release
Author readiness
Human owner: @kaoru0822-kitauji
Before merge
type(scope): outcome.🤖 Generated with Claude Code