Rule
require-spawn-error-listener (eslint-factory/src/rules/require-spawn-error-listener.ts) — first review since it shipped 2026-08-09; no prior issues filed.
What the rule does
Flags const child = spawn(...) (async child_process.spawn) when no child.on("error", ...) / child.once("error", ...) listener is attached anywhere in scope. Without that listener, a failed launch (ENOENT/EACCES) becomes an uncaught 'error' event that crashes the process.
The gap
isSpawnBinding() only recognizes a callee identifier as spawn-derived through two initializer shapes:
- an
ImportSpecifier named spawn, or
- an
ObjectPattern destructured directly from require("child_process") / require("node:child_process").
Any other initializer shape returns false, so isSpawnCall never matches and the VariableDeclarator visitor bails out before checking for an error listener at all — the call site gets zero coverage from this rule, not even a chance at a false negative on the listener check itself.
The existing test suite documents one such gap as an accepted, deliberate scope limit: a plain alias const spawnImpl = spawn; const child = spawnImpl("ls", []); is asserted valid (i.e. never analyzed). That's a reasonable call for a bare alias with no live motivation. But the same blind spot also swallows a strictly more common idiom — a dependency-injection fallback const spawnImpl = options.spawnImpl ?? spawn; — and that shape is live in this codebase, not hypothetical.
Live grounding
actions/setup/js/copilot_sdk_sidecar.cjs:
128: const spawnImpl = options.spawnImpl ?? spawn;
...
131: const child = spawnImpl(options.command, args, {This file happens to already attach child.once("error", onError) later (line 159), so there's no live crash today. But because spawnImpl's initializer is a LogicalExpression (??), not a bare require() destructure, isSpawnBinding never matches it, so require-spawn-error-listener silently never evaluated this call site at all. If that error listener is ever refactored away, the exact ENOENT/EACCES-crash regression this rule exists to catch will slip through unnoticed — the rule's own scope-limitations comment doesn't mention DI-fallback bindings as an intentional exclusion, so this reads as an unintended gap rather than a documented one.
This is the same shape of defect as the CORE_ALIASES gap filed against 8 sibling rules for DI-style core parameter names (#49489) — factory code in this repo commonly injects an overridable implementation via options.xImpl ?? xImpl, and rules that pattern-match on literal require()/import shapes miss it.
Ask
- Extend
isSpawnBinding's Variable definition handling to recognize spawn reachable through a <name> = <expr> ?? spawn / <expr> || spawn fallback initializer (right operand is the identifier already resolved as spawn-bound), in addition to the existing ImportSpecifier / require-destructure cases.
- Add a test mirroring the
copilot_sdk_sidecar.cjs shape: const spawnImpl = options.spawnImpl ?? spawn; const child = spawnImpl(...); — invalid when no error listener is present, valid when child.on("error", ...) / .once("error", ...) is.
- Leave the existing plain-alias (
const spawnImpl = spawn) documented scope-limit test as-is unless a maintainer wants to widen it too — that shape still has no live motivation.
Acceptance criteria
Generated by 🤖 ESLint Refiner · agent · 262.1 AIC · ⌖ 28.7 AIC · ⊞ 4.7K · ◷
Rule
require-spawn-error-listener(eslint-factory/src/rules/require-spawn-error-listener.ts) — first review since it shipped 2026-08-09; no prior issues filed.What the rule does
Flags
const child = spawn(...)(asyncchild_process.spawn) when nochild.on("error", ...)/child.once("error", ...)listener is attached anywhere in scope. Without that listener, a failed launch (ENOENT/EACCES) becomes an uncaught 'error' event that crashes the process.The gap
isSpawnBinding()only recognizes a callee identifier as spawn-derived through two initializer shapes:ImportSpecifiernamedspawn, orObjectPatterndestructured directly fromrequire("child_process")/require("node:child_process").Any other initializer shape returns
false, soisSpawnCallnever matches and theVariableDeclaratorvisitor bails out before checking for an error listener at all — the call site gets zero coverage from this rule, not even a chance at a false negative on the listener check itself.The existing test suite documents one such gap as an accepted, deliberate scope limit: a plain alias
const spawnImpl = spawn; const child = spawnImpl("ls", []);is assertedvalid(i.e. never analyzed). That's a reasonable call for a bare alias with no live motivation. But the same blind spot also swallows a strictly more common idiom — a dependency-injection fallbackconst spawnImpl = options.spawnImpl ?? spawn;— and that shape is live in this codebase, not hypothetical.Live grounding
actions/setup/js/copilot_sdk_sidecar.cjs:This file happens to already attach
child.once("error", onError)later (line 159), so there's no live crash today. But becausespawnImpl's initializer is aLogicalExpression(??), not a barerequire()destructure,isSpawnBindingnever matches it, sorequire-spawn-error-listenersilently never evaluated this call site at all. If that error listener is ever refactored away, the exact ENOENT/EACCES-crash regression this rule exists to catch will slip through unnoticed — the rule's own scope-limitations comment doesn't mention DI-fallback bindings as an intentional exclusion, so this reads as an unintended gap rather than a documented one.This is the same shape of defect as the CORE_ALIASES gap filed against 8 sibling rules for DI-style
coreparameter names (#49489) — factory code in this repo commonly injects an overridable implementation viaoptions.xImpl ?? xImpl, and rules that pattern-match on literalrequire()/import shapes miss it.Ask
isSpawnBinding'sVariabledefinition handling to recognizespawnreachable through a<name> = <expr> ?? spawn/<expr> || spawnfallback initializer (right operand is the identifier already resolved as spawn-bound), in addition to the existingImportSpecifier/ require-destructure cases.copilot_sdk_sidecar.cjsshape:const spawnImpl = options.spawnImpl ?? spawn; const child = spawnImpl(...);— invalid when no error listener is present, valid whenchild.on("error", ...)/.once("error", ...)is.const spawnImpl = spawn) documented scope-limit test as-is unless a maintainer wants to widen it too — that shape still has no live motivation.Acceptance criteria
isSpawnBindingrecognizes<name> = <expr> ?? spawn/<expr> || spawnshaped declarators as spawn-bound.missingErrorListenerwhen no error listener is present.child.on("error", ...)/.once("error", ...)is present.copilot_sdk_sidecar.cjs:131is now covered by the rule (verified by re-reading the resolver logic against this exact call shape).