Summary
require-execsync-try-catch (the newest rule, added since the last refiner run) requires execSync(...) calls to be wrapped in try/catch because execSync throws a ChildProcessError on non-zero exit or signal termination. But its throwing sibling execFileSync — which has identical throw-on-failure semantics — is not covered by any rule:
require-execsync-try-catch matches only execSync.require-spawnsync-error-check targets spawnSync, which does not throw (it returns { status, error }), so it is the wrong home for execFileSync.
This leaves an unguarded gap: an unwrapped execFileSync that exits non-zero crashes the action with no useful diagnostic — exactly the failure mode the execSync rule was created to prevent.
Grounding in actions/setup/js/**
execFileSync is used in several live (non-test) action scripts, and the maintainers already manually wrap it in try/catch — direct evidence that the invariant matters and that a lint guard would prevent regressions:
merge_remote_agent_github_folder.cjs:181,185,194,199,202 — all inside a try block (correctly protected).evaluate_outcomes.cjs:81 — inside try (correctly protected).safeoutputs_cli.cjs:41 — inside try (correctly protected).
A site that is not directly wrapped:
build_checkout_manifest.cjs:44 and :48 — execFileSync("git"/"gh", ...) inside default-arg factory lambdas (options.runGit || ((args) => execFileSync(...))), with no surrounding try. Whether every call site protects these depends on callers; a rule would make the requirement explicit rather than implicit.
All wrapped sites would remain true-negatives under the extended rule; only unprotected execFileSync would be newly flagged.
Proposed change
Cover execFileSync with the same try/catch requirement. Two acceptable shapes — pick whichever fits repo conventions:
- Extend
require-execsync-try-catch to also match execFileSync. The existing scope-resolution machinery already handles const { execSync } = require("child_process"), cp.execSync(...), const execSync = cp.execSync, and the ESM import { execSync } from "child_process" forms — generalizing the matched-method name from a single "execSync" to a Set(["execSync", "execFileSync"]) is a small change. Emit a method-specific diagnostic (Wrap execFileSync(...) in try/catch ...).
- Add a sibling rule
require-execfilesync-try-catch — consistent with how require-mkdirsync-try-catch was split out from require-fs-sync-try-catch (one rule per method family). This reuses try-catch-rule-utils (isInsideTryBlock, findEnclosingStatement, buildTryCatchSuggestion).
Acceptance criteria
execFileSync(...) (destructured, namespaced cp.execFileSync, aliased const execFileSync = cp.execFileSync, and ESM import { execFileSync }) sourced from child_process / node:child_process is flagged when not inside an enclosing try with a catch handler.execFileSync inside a protective try { ... } catch { ... } is NOT flagged (regression tests mirroring the existing execSync valid cases).execFileSync from a non-child_process module is ignored (scope-resolution parity with the execSync rule).- Suggestion wraps the enclosing statement using
buildTryCatchSuggestion with an execFileSync-specific TODO/error prefix.
- Re-verify against the corpus:
build_checkout_manifest.cjs:44/48 flagged; merge_remote_agent_github_folder.cjs, evaluate_outcomes.cjs:81, safeoutputs_cli.cjs:41 remain clean.
- Docs/README note the new coverage; decide explicitly whether
execFile (async, callback/promise) is in or out of scope and document it.
Filed by the ESLint Refiner daily review. Grounded statically against non-test .cjs in actions/setup/js/** (no live lint available in this environment).
Generated by 🤖 ESLint Refiner · age00 312 AIC · ⌖ 12.9 AIC · ⊞ 4.6K · ◷
Summary
require-execsync-try-catch(the newest rule, added since the last refiner run) requiresexecSync(...)calls to be wrapped in try/catch becauseexecSyncthrows aChildProcessErroron non-zero exit or signal termination. But its throwing siblingexecFileSync— which has identical throw-on-failure semantics — is not covered by any rule:require-execsync-try-catchmatches onlyexecSync.require-spawnsync-error-checktargetsspawnSync, which does not throw (it returns{ status, error }), so it is the wrong home forexecFileSync.This leaves an unguarded gap: an unwrapped
execFileSyncthat exits non-zero crashes the action with no useful diagnostic — exactly the failure mode the execSync rule was created to prevent.Grounding in
actions/setup/js/**execFileSyncis used in several live (non-test) action scripts, and the maintainers already manually wrap it in try/catch — direct evidence that the invariant matters and that a lint guard would prevent regressions:merge_remote_agent_github_folder.cjs:181,185,194,199,202— all inside atryblock (correctly protected).evaluate_outcomes.cjs:81— insidetry(correctly protected).safeoutputs_cli.cjs:41— insidetry(correctly protected).A site that is not directly wrapped:
build_checkout_manifest.cjs:44and:48—execFileSync("git"/"gh", ...)inside default-arg factory lambdas (options.runGit || ((args) => execFileSync(...))), with no surrounding try. Whether every call site protects these depends on callers; a rule would make the requirement explicit rather than implicit.All wrapped sites would remain true-negatives under the extended rule; only unprotected
execFileSyncwould be newly flagged.Proposed change
Cover
execFileSyncwith the same try/catch requirement. Two acceptable shapes — pick whichever fits repo conventions:require-execsync-try-catchto also matchexecFileSync. The existing scope-resolution machinery already handlesconst { execSync } = require("child_process"),cp.execSync(...),const execSync = cp.execSync, and the ESMimport { execSync } from "child_process"forms — generalizing the matched-method name from a single"execSync"to aSet(["execSync", "execFileSync"])is a small change. Emit a method-specific diagnostic (Wrap execFileSync(...) in try/catch ...).require-execfilesync-try-catch— consistent with howrequire-mkdirsync-try-catchwas split out fromrequire-fs-sync-try-catch(one rule per method family). This reusestry-catch-rule-utils(isInsideTryBlock,findEnclosingStatement,buildTryCatchSuggestion).Acceptance criteria
execFileSync(...)(destructured, namespacedcp.execFileSync, aliasedconst execFileSync = cp.execFileSync, and ESMimport { execFileSync }) sourced fromchild_process/node:child_processis flagged when not inside an enclosingtrywith acatchhandler.execFileSyncinside a protectivetry { ... } catch { ... }is NOT flagged (regression tests mirroring the existing execSync valid cases).execFileSyncfrom a non-child_processmodule is ignored (scope-resolution parity with the execSync rule).buildTryCatchSuggestionwith an execFileSync-specific TODO/error prefix.build_checkout_manifest.cjs:44/48flagged;merge_remote_agent_github_folder.cjs,evaluate_outcomes.cjs:81,safeoutputs_cli.cjs:41remain clean.execFile(async, callback/promise) is in or out of scope and document it.Filed by the ESLint Refiner daily review. Grounded statically against non-test
.cjsinactions/setup/js/**(no live lint available in this environment).