You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
When the rule flags a sync child_process call inside a non-async enclosing function whose result is retained (requiresAsyncConversion returns true), it emits preferActionsExecSyncContext:
"...migrating this call requires converting the enclosing (currently non-async) function — and every one of its callers up the chain — to async/await."
requiresAsyncConversion only inspects the single nearest enclosing function (findEnclosingFunction walks up to the first Function node and stops — it never inspects that function's own callers):
So the "every one of its callers up the chain" clause is asserted unconditionally whenever the immediate enclosing function is non-async — it is never actually verified against the real call graph. In practice this overstates the fix cost whenever the callers are already async, which is exactly what's grounded live in this codebase.
Grounded evidence
get_current_branch.cjs — flagged function, all 4 live callers already async
actions/setup/js/get_current_branch.cjs exports a synchronous getCurrentBranch(customCwd) that uses execSync and would trigger preferActionsExecSyncContext (non-async enclosing function, retained/returned result). All of its production call sites are already inside async functions:
actions/setup/js/handle_agent_failure.cjs:159 — inside async function findPullRequestForCurrentBranch()
actions/setup/js/handle_agent_failure.cjs:3745 — a few lines after await findPullRequestForCurrentBranch(), same async context
actions/setup/js/safe_outputs_handlers.cjs:879 — inside an async handler that already uses await a few lines earlier (line 865 context)
actions/setup/js/safe_outputs_handlers.cjs:1337 — same pattern
In all four cases, the real fix is: make getCurrentBranch itself async (swap execSync for await exec.getExecOutput(...)), then add await at each of the 4 call sites — a purely local, one-hop change. There is no further "chain" to convert; the message's blanket claim doesn't match the actual migration cost here.
Why this matters
This is a weak/misleading diagnostic: it asserts a specific, scary-sounding migration cost ("every one of its callers up the chain") without verifying it, which can make a trivial one-hop fix look like a large refactor and discourage someone from acting on the finding. The check is easy to make accurate for the common case since ESLint has no-cost access to whether known, statically-resolvable direct callers are async — even a shallow one-level check (are direct callers already inside await expressions calling this function?) would tighten the message meaningfully. At minimum, the message should not present an unverified claim as fact.
Acceptance criteria
Soften preferActionsExecSyncContext wording so it doesn't assert a caller-chain cascade as fact — e.g. "...may require converting the enclosing function, and possibly its callers, to async/await" — or split into two messages depending on whether the enclosing function is exported/called elsewhere in ways the rule can actually inspect.
Add a test case mirroring get_current_branch.cjs's shape (sync helper function using execSync, called from elsewhere) to document the current message behavior, so future changes to the wording are deliberate.
Consider (optional, larger scope) a best-effort direct-caller check within the same file: if all statically-resolvable same-file call sites of the enclosing function already sit inside await-using async functions, use a message that doesn't claim a wider cascade.
Overview
When the rule flags a sync
child_processcall inside a non-async enclosing function whose result is retained (requiresAsyncConversionreturns true), it emitspreferActionsExecSyncContext:requiresAsyncConversiononly inspects the single nearest enclosing function (findEnclosingFunctionwalks up to the first Function node and stops — it never inspects that function's own callers):So the "every one of its callers up the chain" clause is asserted unconditionally whenever the immediate enclosing function is non-async — it is never actually verified against the real call graph. In practice this overstates the fix cost whenever the callers are already async, which is exactly what's grounded live in this codebase.
Grounded evidence
get_current_branch.cjs — flagged function, all 4 live callers already async
actions/setup/js/get_current_branch.cjsexports a synchronousgetCurrentBranch(customCwd)that usesexecSyncand would triggerpreferActionsExecSyncContext(non-async enclosing function, retained/returned result). All of its production call sites are already insideasyncfunctions:actions/setup/js/handle_agent_failure.cjs:159— insideasync function findPullRequestForCurrentBranch()actions/setup/js/handle_agent_failure.cjs:3745— a few lines afterawait findPullRequestForCurrentBranch(), same async contextactions/setup/js/safe_outputs_handlers.cjs:879— inside an async handler that already usesawaita few lines earlier (line 865 context)actions/setup/js/safe_outputs_handlers.cjs:1337— same patternIn all four cases, the real fix is: make
getCurrentBranchitselfasync(swapexecSyncforawait exec.getExecOutput(...)), then addawaitat each of the 4 call sites — a purely local, one-hop change. There is no further "chain" to convert; the message's blanket claim doesn't match the actual migration cost here.Why this matters
This is a weak/misleading diagnostic: it asserts a specific, scary-sounding migration cost ("every one of its callers up the chain") without verifying it, which can make a trivial one-hop fix look like a large refactor and discourage someone from acting on the finding. The check is easy to make accurate for the common case since ESLint has no-cost access to whether known, statically-resolvable direct callers are async — even a shallow one-level check (are direct callers already inside
awaitexpressions calling this function?) would tighten the message meaningfully. At minimum, the message should not present an unverified claim as fact.Acceptance criteria
preferActionsExecSyncContextwording so it doesn't assert a caller-chain cascade as fact — e.g. "...may require converting the enclosing function, and possibly its callers, to async/await" — or split into two messages depending on whether the enclosing function is exported/called elsewhere in ways the rule can actually inspect.get_current_branch.cjs's shape (sync helper function usingexecSync, called from elsewhere) to document the current message behavior, so future changes to the wording are deliberate.await-using async functions, use a message that doesn't claim a wider cascade.References:
Warning
Firewall blocked 1 domain
The following domain was blocked by the firewall during workflow execution:
api.anthropic.comTo allow these domains, add them to the
network.allowedlist in your workflow frontmatter:See Network Configuration for more information.