Resolve require-async-entrypoint-catch by binding, not bare name

[Copilot]

require-async-entrypoint-catch was matching bare calls by identifier text alone, so shadowed same-name locals could be reported as module-scope async entrypoints. This made the rule unsound in both directions: false positives for local sync bindings and missed distinctions between separate same-name bindings.

• Scope-aware entrypoint matching

  • Replace module-wide name tracking with binding resolution through ESLint scope lookup.
  • Only report when the callee resolves to the actual module-scope async FunctionDeclaration.
  • Stop lookup at the first shadowing binding so local const, function, or other definitions do not fall through to an outer same-name entrypoint.

• Rule coverage for shadowing

  • Add RuleTester cases for a shadowed local synchronous binding with the same name.
  • Add RuleTester cases for a shadowed nested async binding with the same name.
  • Preserve existing true-positive behavior for real module-scope async entrypoints such as main().

• Behavioral effect

  • The rule now distinguishes these cases by resolved binding rather than text:

async function report() {}

function buildThing() {
  const report = () => {};
  report(); // no longer treated as the module-scope async entrypoint
}

[github-actions]

⚠️ PR Code Quality Reviewer failed during code quality review.

[github-actions]

✅ Test Quality Sentinel completed test quality analysis.

[github-actions]

🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅

[github-actions]

✅ Design Decision Gate 🏗️ completed the design decision gate check.

No ADR enforcement needed: PR #43349 does not have the 'implementation' label and has 0 new lines of code in business logic directories (threshold: 100).

[Copilot]

Pull request overview

This PR makes the require-async-entrypoint-catch ESLint rule scope-aware by resolving identifier callees to their actual bindings via ESLint scope lookup, preventing false positives/negatives when a module-scope async function name is shadowed by a local binding.

Changes:

• Replace module-wide name tracking with scope/binding resolution so Identifier callees are matched to the correct FunctionDeclaration.
• Only report when the resolved binding is a module-scope async FunctionDeclaration, stopping at the first shadowing binding.
• Add RuleTester coverage for shadowed local sync and shadowed local async bindings with the same name.

Show a summary per file

File	Description
eslint-factory/src/rules/require-async-entrypoint-catch.ts	Switches entrypoint detection from name-based tracking to scope-resolved binding checks.
eslint-factory/src/rules/require-async-entrypoint-catch.test.ts	Adds valid test cases ensuring shadowed locals don’t trigger module-scope entrypoint reports.

Review details

Tip

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

• Files reviewed: 2/2 changed files
• Comments generated: 0
• Review effort level: Low

[github-actions]

🧪 Test Quality Sentinel Report

✅ Test Quality Score: 100/100 — Excellent

Analyzed 2 test case(s): 2 design, 0 implementation, 0 violation(s).

📊 Metrics (2 tests)

Metric	Value
Analyzed	2 (Go: 0, TS: 2)
✅ Design	2 (100%)
⚠️ Implementation	0 (0%)
Edge/error coverage	2 (100%)
Duplicate clusters	0
Inflation	No (14 test lines / 24 prod lines = 0.58:1)
🚨 Violations	0

Both new test cases are valid-case scenarios added inside an existing it() block, each exercising the scope-resolution fix introduced by this PR.

Test	File	Classification	Issues
Valid: shadowed local const binding	require-async-entrypoint-catch.test.ts	behavioral_contract / design_test / high_value	None
Valid: shadowed local async function declaration	require-async-entrypoint-catch.test.ts	behavioral_contract / design_test / high_value	None

Verdict

✅ Passed. 0% implementation tests (threshold: 30%). Both new test cases target the exact regression the PR fixes — bare-name false positives when a module-scope async function is shadowed by a local binding — and verify the behavioral contract rather than implementation details. No guideline violations.

References:

• §28701059913

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

• awmgmcpg

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "awmgmcpg"

See Network Configuration for more information.

🧪 Test quality analysis by Test Quality Sentinel · 30 AIC · ⌖ 10.7 AIC · ⊞ 6.8K · ◷
_{Comment /review to run again}

[github-actions]

✅ Test Quality Sentinel: 100/100. 0% implementation tests (threshold: 30%). Both new test cases are behavioral contracts targeting the exact regression fixed by this PR.

[github-actions]

Skills-Based Review 🧠

Applied /diagnosing-bugs, /tdd, and /codebase-design — no blocking issues, but a few targeted improvements are worth addressing.

📋 Key Themes & Highlights

Key Themes

• False-negative coverage gap: The two new valid tests cover the shadowing cases, but there is no paired invalid test confirming that a module-scope report() call is still flagged when a sibling function shadows the name. Adding this completes the regression story.
• defs[0] assumption: getResolvedFunctionDeclaration reads only the first definition; in the presence of var re-declarations or merged TypeScript declarations, a later def that is a FunctionDeclaration would be missed. Using Array.find over all defs is safer and explicit.
• SourceCodeScope type locality: The inferred type alias is declared inside create(), hiding it from callers and tests. A module-level alias (or the public TSESLint.Scope.Scope) is easier to navigate and reuse.

Positive Highlights

• ✅ Root cause correctly identified and addressed — replacing a global name-set with scope-aware binding resolution eliminates the entire class of false positives.
• ✅ The scope-walk logic is clean and short; early-exit on the first shadowing definition prevents unnecessary traversal.
• ✅ The two new valid test cases are well-commented and directly mirror the motivating example in the PR description.
• ✅ Removing the two-pass approach (collect then check) simplifies the rule to a single-pass visitor.

🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · 52.5 AIC · ⌖ 5.61 AIC · ⊞ 6.6K
_{Comment /matt to run again}

[github-actions]

[/tdd] Missing regression test: no case verifies that a module-scope report() call is still flagged when a same-name shadow exists in a sibling function. Without this, a future reader cannot tell whether the scope-walk might accidentally suppress real positives.

💡 Suggested invalid test to add

{
  // Module-scope async is still flagged even though another function shadows the name locally.
  code: `async function report() { return 42; }
function buildThing() {
  const report = () => { return 1; };
}
report();`,
  errors: [{ messageId: "requireCatch", data: { name: "report" } }],
},

This is the complementary case to the two valid tests added here: the module-scope entrypoint should still produce an error when called from the module scope alongside a sibling that shadows the name. This guards against false negatives.

@copilot please address this.

[Copilot]

Fixed in 0fd21c5: added the sibling-shadow invalid regression so the module-scope report() call is still flagged when a sibling function shadows the name locally.

[github-actions]

[/diagnosing-bugs] variable?.defs[0] silently ignores multiply-defined bindings — if a variable is declared more than once (e.g. var re-declarations or merged declaration files in TypeScript), only the first def is examined. The subsequent if (variable?.defs.length) early-exit also fires for length === 0 on an empty array (though in practice ESLint populates defs), so the logic is subtler than it looks.

💡 Suggested hardening

Consider iterating over all defs before concluding the binding is not a FunctionDeclaration:

const hasDefs = variable && variable.defs.length > 0;
const funcDef = variable?.defs.find(
  d => d.type === "FunctionName" && d.node.type === AST_NODE_TYPES.FunctionDeclaration
);
if (funcDef) return funcDef.node as TSESTree.FunctionDeclaration;
if (hasDefs) return null; // shadowed by a non-function binding

This makes the intent explicit: stop at the first scope that knows the name, whether or not it is a function.

@copilot please address this.

[Copilot]

Fixed in 0fd21c5: the scope lookup now uses a FunctionDeclaration-specific predicate across variable.defs instead of only inspecting defs[0], while still stopping at the first scope that binds the name.

[github-actions]

[/codebase-design] type SourceCodeScope is declared inside create(), making it invisible to tests and other helpers. Moving it to module scope (or using the public TSESLint.Scope.Scope type from @typescript-eslint/utils) would make the type reusable and self-documenting.

💡 Suggestion

import { TSESLint } from "`@typescript-eslint/utils`";

// module-level alias, visible to tests and future helpers
type SourceCodeScope = TSESLint.Scope.Scope;

If the inferred type from sourceCode.getScope differs from the public API type, document why so future maintainers understand the constraint.

@copilot please address this.

[Copilot]

Fixed in 0fd21c5: SourceCodeScope now lives at module scope and uses TSESLint.Scope.Scope so the alias is reusable outside create().

[github-actions]

Review

The scope-aware binding resolution is the correct ESLint pattern and the implementation is sound.

What was checked

• getResolvedFunctionDeclaration logic — starts at call-site scope, walks up, stops at first shadowing binding (any non-FunctionName def), returns null for shadowed names. ✅
• Reporter guard (line 74) — reports only when the resolved declaration is async AND its parent is Program. Correctly handles nested helpers and any shadowed declaration. ✅
• Fixer — insertTextAfter on the CallExpression node correctly produces fn().catch(...) with the original ; unchanged. ✅
• New tests — both shadowing cases (local const and nested async function with the same name) cover the core bug being fixed. ✅
• Existing invalid cases — still pass since module-scope FunctionDeclaration resolves correctly through the scope chain. ✅

No bugs, no security issues, no regressions identified. Clean fix.

🧵 Reviewed using Impeccable skills by Impeccable Skills Reviewer · 63 AIC · ⌖ 5.78 AIC · ⊞ 4.9K

[gh-aw-bot]

@copilot please run the pr-finisher skill and address the remaining review feedback before rerunning checks. Open items:

• github-actions review thread: hardcoded digest literals in docker_firewall_pin_compile_test.go should derive from embedded pins (Resolve require-async-entrypoint-catch by binding, not bare name #43349)
• github-actions review thread: add a paired invalid regression test so the module-scope entrypoint case still gets flagged when shadowing exists (Resolve require-async-entrypoint-catch by binding, not bare name #43349)

Generated by 👨‍🍳 PR Sous Chef · 8.28 AIC · ⌖ 5.03 AIC · ⊞ 4.3K · ◷
_{Comment /souschef to run again}

[Copilot]

@copilot please run the pr-finisher skill and address the remaining review feedback before rerunning checks. Open items:

• github-actions review thread: hardcoded digest literals in `docker_firewa...

Addressed in 0fd21c5: the sibling-shadow invalid regression is in place, and docker_firewall_pin_compile_test.go now derives its digest expectations from embedded container pins instead of hardcoded literals. Local validation passed (npm test in eslint-factory, targeted go test, make test, make recompile, and make agent-report-progress).