fix: use block-scope var destructuring for script variable injection

[salmanmkc]

Summary

Replace function-parameter injection in callAsyncFunction with var destructuring + block scope. This is a non-breaking fix that allows user scripts to shadow injected names (github, core, context, etc.) with const, let, or var without getting SyntaxError.

Problem

callAsyncFunction currently uses new AsyncFunction(...Object.keys(args), source), which makes every injected variable a function parameter. JavaScript forbids const/let redeclaration of function parameters, so user scripts like:

const { getOctokit } = require("@actions/github")

throw SyntaxError: Identifier 'getOctokit' has already been declared. This affects ~10 public workflows (MetaMask, github/migration-actions, etc.) and blocks exposing new bindings without breaking existing scripts.

Solution

Pass a single __scope__ parameter and destructure with var, then wrap user code in a block:

var { context, core, github, ... } = __scope__;
{
  // user script — const/let/var all work here
}

Why var?

• const/let in user code create block-scoped bindings that shadow the outer var — ✅
• var in user code redeclares the existing var — ✅ (would fail with const)
• Reassignment (github = x) still works — ✅ (would fail with const)

This preserves full backward compatibility with existing v7 scripts.

Defensive guards

• Identifier validation: All argument keys are checked against /^[a-zA-Z_$][a-zA-Z0-9_$]*$/ before interpolation into the code string. callAsyncFunction is an exported API, so this prevents code injection if called with untrusted keys.
• __scope__ collision guard: Rejects __scope__ as an argument name to prevent collision with the internal parameter.

Tests added (11 new)

Test	What it verifies
const redeclaration	Block-scope shadow works
let redeclaration	Block-scope shadow works
var redeclaration	Function-scope redeclaration works
Reassignment	foo = "new" works
Access without redeclaration	Injected values still accessible
Return from block scope	return propagates correctly
Top-level await	await works inside block
Syntax errors	Still throw SyntaxError
Invalid argument names	Rejects non-identifiers
__scope__ collision	Rejects reserved name
Original basic test	Preserved from v7

Breaking changes

None. This is fully backward compatible:

• All existing scripts work unchanged
• Scripts that previously worked with var redeclaration still work
• Scripts that previously worked with reassignment still work
• The only new capability is that const/let redeclaration now works too

[github-actions]

Hello from actions/github-script! (8ea1409)

[Copilot]

Pull request overview

This PR updates the callAsyncFunction script runner to inject bindings via a single __scope__ object plus var destructuring and a block wrapper, so user scripts can redeclare injected names with const/let without hitting parameter redeclaration SyntaxErrors.

Changes:

• Replace function-parameter injection with __scope__ + var {…} = __scope__ destructuring and a block-wrapped user script.
• Add runtime guards for invalid argument names and reserved __scope__ collisions.
• Add Jest tests covering redeclaration/shadowing behavior, reassignment, awaits, syntax errors, and guard failures.

Show a summary per file

File	Description
src/async-function.ts	Switches injection strategy to block-scoped shadowable bindings and adds defensive argument-name validation.
test/async-function.test.ts	Adds test coverage for shadowing/redeclaration semantics and the new validation guards.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

• Files reviewed: 2/2 changed files
• Comments generated: 3

[Copilot]

Wrapping the user-provided source in an extra block means directive prologues in user scripts (e.g. starting with 'use strict') are no longer at the function body top level, so strict mode will be silently ignored. This is a behavioral change compared to the previous new AsyncFunction(..., source) implementation and contradicts the PR’s “non-breaking” claim; consider preserving directive prologues (e.g. detect/hoist them) or explicitly documenting this change (and adding a regression test either way).

[Copilot]

The identifier guard only checks a simple ASCII identifier regex; it will accept reserved words that are invalid binding identifiers in an async function (notably await), which will still cause new AsyncFunction(...) to throw SyntaxError after passing validation. If this is meant to be a defensive/clear error for exported API callers, expand validation to reject reserved words (at least await) or validate by compiling a minimal binding declaration before interpolating.

[Copilot]

There’s no regression test for directive prologues (e.g. a script starting with 'use strict'). With the new block wrapper this behavior changes (strict mode would be ignored), so it would be good to add a test that locks in the intended semantics once the wrapper is adjusted.

Suggested change

	test('preserves directive prologues such as use strict', async () => {
	await expect(
	callAsyncFunction(
	{} as any,
	`
	'use strict'
	accidentalGlobal = 1
	`
	)
	).rejects.toBeInstanceOf(ReferenceError)
	})

[salmanmkc]

Closing in favor of PR #700 which includes both the getOctokit factory and the v9 upgrade. No need for a separate v6 backport.