fix(eslint-factory): recognize single-assignment alias of spawn error as valid guard by Copilot · Pull Request #46199 · github/gh-aw

Copilot · 2026-07-17T09:49:57Z

require-spawnsync-error-check emits a false positive when result.error is assigned to an intermediate variable that is then guarded — both in the simple-identifier and object-pattern branches.

Root cause

isGuardingErrorUsage has no handler for VariableDeclarator, so const e = result.error always returns false, and the later if (e) throw e guard is never connected back to result.error.

Changes

New helper isAssignedToGuardedAlias — when node is the init of a single-identifier VariableDeclarator, resolves the alias variable and checks if any of its references satisfy isGuardingErrorUsage. One level of indirection only.
Simple-identifier branch — also accepts isAssignedToGuardedAlias(sourceCode, parent) on the result.error MemberExpression.
Object-pattern branch — also accepts isAssignedToGuardedAlias(sourceCode, ref.identifier) on the destructured error binding.
Rule description — documents alias support and calls out that passing result to a helper function remains out of scope.

Test coverage added

// valid: simple-identifier alias
const result = spawnSync("git", ["status"]);
const e = result.error;
if (e) throw e;

// valid: object-pattern alias
const { status, error } = spawnSync("zip", ["-v"]);
const e = error;
if (e) throw e;

// invalid: alias assigned but never guarded
const result = spawnSync("git", ["status"]);
const e = result.error;
core.info(String(e)); // → missingErrorCheck

Generated by 👨‍🍳 PR Sous Chef · 8.3 AIC · ⌖ 10.9 AIC · ⊞ 7.5K · ◷
_{Comment /souschef to run again}

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

… as valid guard Fixes false positive in require-spawnsync-error-check when result.error (or a destructured error binding) is assigned to an intermediate variable that is then tested in a guard condition. Adds isAssignedToGuardedAlias() which detects the pattern: const e = result.error; // or: const e = error; (destructured) if (e) throw e; Updates both the simple-identifier and object-pattern analysis branches to also accept this one-level alias indirection. Updated rule description documents the new alias support and notes that passing result to a helper function remains out of scope. Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

github-actions · 2026-07-17T14:46:25Z

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

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

github-actions · 2026-07-17T14:46:28Z

✅ PR Code Quality Reviewer completed the code quality review.

github-actions · 2026-07-17T14:46:47Z

✅ Test Quality Sentinel completed test quality analysis.

github-actions · 2026-07-17T14:46:48Z

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

Copilot

Pull request overview

Adds one-level guarded aliases for spawnSync error checks.

Changes:

Adds alias resolution for direct and destructured errors.
Adds valid and invalid alias tests.
Changes the generated auto-upgrade schedule.

Show a summary per file

File	Description
`require-spawnsync-error-check.ts`	Implements guarded alias detection.
`require-spawnsync-error-check.test.ts`	Tests alias handling.
`agentic-auto-upgrade.yml`	Changes the weekly schedule.

Review details

Tip

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

Files reviewed: 3/3 changed files
Comments generated: 2
Review effort level: Medium

Copilot · 2026-07-17T15:30:41Z

+  const variable = findVariableByName(sourceCode, node, idNode.name);
+  if (!variable) return false;
+
+  return variable.references.some(ref => ref.identifier.type === AST_NODE_TYPES.Identifier && isGuardingErrorUsage(ref.identifier));


Reverted in the latest commit. The cron schedule is back to "21 3 * * 5" (the value generated from the repository slug). This change should not have been part of this PR.

github-actions

The alias-guard fix is correct and well-tested. The isAssignedToGuardedAlias helper properly restricts to single-identifier VariableDeclarator init, preventing false-negatives for deeper alias chains. Both the simple-identifier and object-pattern branches are covered with matching test cases (valid + invalid). No issues found.

🧵 Reviewed using Impeccable skills by Impeccable Skills Reviewer · 18 AIC · ⌖ 4.9 AIC · ⊞ 5K

github-actions

Skills-Based Review 🧠

Applied /diagnosing-bugs and /tdd — commenting with minor suggestions, no blocking issues.

📋 Key Themes & Highlights

Key Themes

Missing symmetric test: the destructured-alias-unguarded path lacks an invalid test to mirror the simple-identifier case (see inline comment)
Redundant type guard in isAssignedToGuardedAlias: ref.identifier is always an Identifier, so the .type === AST_NODE_TYPES.Identifier check is unnecessary noise

Positive Highlights

✅ Root cause correctly identified and fixed at the right level
✅ One-level-of-indirection scope limit is well-reasoned and documented
✅ Both simple-identifier and object-pattern branches updated consistently
✅ Rule description updated to document alias support and scope boundary
✅ Good test coverage for the new behavior

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

github-actions · 2026-07-17T14:55:18Z

+        },
      ],
    });
  });


[/tdd] Missing symmetrical invalid test: the destructured-alias-then-unguarded path has no dedicated test case.

The existing invalid test at line 91 covers the simple-identifier alias that is used non-guardingly (const e = result.error; core.info(String(e))). The object-pattern branch is only tested in the valid direction. Adding the symmetric invalid case strengthens confidence that isAssignedToGuardedAlias doesn't accidentally accept unguarded destructured aliases.

💡 Suggested invalid test

{ // destructured alias assigned but never guarded code: `const { error } = spawnSync("git", ["status"]); const e = error; core.info(String(e));`, errors: [{ messageId: "missingErrorCheck" }], },

@copilot please address this.

github-actions · 2026-07-17T14:55:18Z

+  if (idNode.type !== AST_NODE_TYPES.Identifier) return false;
+
+  const variable = findVariableByName(sourceCode, node, idNode.name);
+  if (!variable) return false;


[/diagnosing-bugs] The ref.identifier.type === AST_NODE_TYPES.Identifier guard inside isAssignedToGuardedAlias is redundant — the caller already extracted ref.identifier which is always an Identifier.

Removing the check makes the intent clearer and avoids future confusion about why the type guard is needed.

💡 Suggested simplification

return variable.references.some(ref => isGuardingErrorUsage(ref.identifier));

@copilot please address this.

github-actions · 2026-07-17T14:55:33Z

🧪 Test Quality Sentinel Report

✅ Test Quality Score: 80/100 — Excellent

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

📊 Metrics (3 tests)

Metric	Value
Analyzed	3 (TypeScript/Vitest)
✅ Design	3 (100%)
⚠️ Implementation	0 (0%)
Edge/error coverage	1 (33.3%)
Duplicate clusters	0
Inflation	No (0.35:1 ratio)
🚨 Violations	0

Test	File	Classification	Issues
Single-assignment alias (direct member)	require-spawnsync-error-check.test.ts:40-41	Design test	None — covers new feature
Single-assignment alias (destructuring)	require-spawnsync-error-check.test.ts:42-43	Design test	None — covers variant
Alias without guarding (negative case)	require-spawnsync-error-check.test.ts:94-97	Design test	None — critical edge case

✅ Test Coverage Analysis

Strengths:

✅ All 3 tests are design tests verifying behavioral contracts (not implementation details)
✅ Positive cases: tests confirm aliasing with both direct member access and destructuring
✅ Negative case: tests reject aliasing when the alias is not used as a guard
✅ Test inflation ratio: 0.35 (9 test lines : 26 prod lines) — well below 2:1 threshold
✅ Clear comments explain intent of each test case
✅ No violations: no mocking libraries, proper test structure
✅ Covers critical edge case: prevents false negatives where alias exists but doesn't guard

Coverage Summary:
The added tests validate the exact behavioral contract of the new isAssignedToGuardedAlias() function:

Accepts: error aliased to a const, then the alias checked in a guard (2 cases with variants)
Rejects: error aliased but only used in non-guarding context (1 case)

Verdict

✅ Passed. 0% implementation tests (threshold: 30%). All tests target behavioral contracts.

Quality assessment: Tests precisely cover the new feature and include a critical negative case that prevents the rule from creating false positives. High-value test additions for the scope.

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 · 15.1 AIC · ⌖ 11.3 AIC · ⊞ 6.8K · ◷
_{Comment /review to run again}

github-actions

✅ Test Quality Sentinel: 80/100. 0% implementation tests (threshold: 30%). All 3 tests verify behavioral contracts with comprehensive edge-case coverage. The negative case (alias without guarding) is particularly valuable for preventing false positives.

github-actions

🔎 Code quality review by PR Code Quality Reviewer · 45.6 AIC · ⌖ 4.42 AIC · ⊞ 5.6K
_{Comment /review to run again}

github-actions · 2026-07-17T14:56:09Z

+  const variable = findVariableByName(sourceCode, node, idNode.name);
+  if (!variable) return false;
+
+  return variable.references.some(ref => ref.identifier.type === AST_NODE_TYPES.Identifier && isGuardingErrorUsage(ref.identifier));


Write references are iterated needlessly before reaching isAssignedToGuardedAlias, adding subtle noise to the logic.

💡 Detail

variable.references includes both read and write references. The write reference is the binding site itself (e.g. the error identifier in the id position of const { ..., error } = spawnSync(...)). When that write-ref identifier is passed to isGuardingErrorUsage or isAssignedToGuardedAlias:

isGuardingErrorUsage will walk up through its VariableDeclarator parent and return false — harmless.

isAssignedToGuardedAlias checks parent.init !== node; since the write-ref identifier is the id, not the init, it returns false immediately — also harmless.

But the intent is only ever to check read references, and silently skipping the write ref by relying on early-exit logic in two different helpers is fragile. A future change to either helper could accidentally treat a write reference as a valid guard. Filtering explicitly makes the contract clear:

return variable.references.some( ref => !ref.isWrite() && ref.identifier.type === AST_NODE_TYPES.Identifier && (isGuardingErrorUsage(ref.identifier) || isAssignedToGuardedAlias(sourceCode, ref.identifier)) );

Same pattern applies to the identical block in the simple-identifier branch (line ~196).

…or-check

gh-aw-bot · 2026-07-17T15:13:06Z

@copilot please run the pr-finisher skill, address unresolved review feedback, investigate any failed checks listed below, refresh the branch if needed, and rerun checks once ready.

Unresolved review threads (newest first):

github-actions[bot]: fix(eslint-factory): recognize single-assignment alias of spawn error as valid guard #46199 (comment)
github-actions[bot]: fix(eslint-factory): recognize single-assignment alias of spawn error as valid guard #46199 (comment)
github-actions[bot]: fix(eslint-factory): recognize single-assignment alias of spawn error as valid guard #46199 (comment)
Copilot: fix(eslint-factory): recognize single-assignment alias of spawn error as valid guard #46199 (comment)
Copilot: fix(eslint-factory): recognize single-assignment alias of spawn error as valid guard #46199 (comment)

Generated by pr-sous-chef: https://github.com/github/gh-aw/actions/runs/29590499747

Generated by 👨‍🍳 PR Sous Chef · 8.3 AIC · ⌖ 10.9 AIC · ⊞ 7.5K · ◷
_{Comment /souschef to run again}

Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>

…as; revert unrelated schedule change Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>

github-actions · 2026-07-18T09:23:08Z

🎉 This pull request is included in a new release.

Release: v0.82.13

Initial plan

75c334b

Copilot AI assigned Copilot and pelikhan Jul 17, 2026

Copilot started work on behalf of pelikhan July 17, 2026 09:50 View session

Copilot AI linked an issue Jul 17, 2026 that may be closed by this pull request

eslint-factory: require-spawnsync-error-check — false positive when result.error is guarded through an intermediate variable #46150

Closed

Copilot AI and others added 3 commits July 17, 2026 09:55

chore: initial plan (no code changes yet)

925c8fd

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

fix(eslint-factory): correct isAssignedToGuardedAlias docstring accuracy

b8bf111

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

Copilot AI changed the title ~~[WIP] Fix false positive in require-spawnsync-error-check rule~~ fix(eslint-factory): recognize single-assignment alias of spawn error as valid guard Jul 17, 2026

Copilot AI requested a review from pelikhan July 17, 2026 10:03

Copilot finished work on behalf of pelikhan July 17, 2026 10:03

github-actions Bot added pr-action:batch_review pr-agent:copilot pr-batch:eslint-fixes pr-priority:medium pr-risk:low pr-type:bug labels Jul 17, 2026

pelikhan marked this pull request as ready for review July 17, 2026 14:45

Copilot AI review requested due to automatic review settings July 17, 2026 14:45

Copilot started reviewing on behalf of pelikhan July 17, 2026 14:46 View session

Copilot AI reviewed Jul 17, 2026

View reviewed changes

github-actions Bot mentioned this pull request Jul 17, 2026

[aw] No-Op Runs #43499

Open

github-actions Bot approved these changes Jul 17, 2026

View reviewed changes

github-actions Bot reviewed Jul 17, 2026

View reviewed changes

github-actions Bot approved these changes Jul 17, 2026

View reviewed changes

github-actions Bot reviewed Jul 17, 2026

View reviewed changes

Merge branch 'main' into copilot/eslint-factory-require-spawnsync-err…

d498bb0

…or-check

github-actions Bot mentioned this pull request Jul 17, 2026

[pr-sous-chef] Run report — 0 nudged, 0 skipped #46103

Closed

Copilot started work on behalf of gh-aw-bot July 17, 2026 15:13 View session

Copilot AI and others added 2 commits July 17, 2026 15:18

chore: planning - no code changes yet

307b1b8

Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>

fix(eslint-factory): reject mutable aliases in isAssignedToGuardedAli…

99a2a48

…as; revert unrelated schedule change Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>

Copilot finished work on behalf of gh-aw-bot July 17, 2026 15:31

Copilot AI requested a review from gh-aw-bot July 17, 2026 15:31

pelikhan merged commit cfbab8e into main Jul 18, 2026

pelikhan deleted the copilot/eslint-factory-require-spawnsync-error-check branch July 18, 2026 05:30

This was referenced Jul 18, 2026

Smoke Test: MAI-Code-1-Flash - 29632319598 #46346

Closed

Smoke Test: MAI-Code-1-Flash - 29638034909 #46385

Open

Uh oh!

Conversation

Copilot AI commented Jul 17, 2026 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Root cause

Changes

Test coverage added

Uh oh!

github-actions Bot commented Jul 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Jul 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Jul 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Jul 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Review details

Uh oh!

Copilot AI Jul 17, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Skills-Based Review 🧠

Key Themes

Positive Highlights

Uh oh!

github-actions Bot Jul 17, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Jul 17, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Jul 17, 2026

🧪 Test Quality Sentinel Report

Verdict

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Jul 17, 2026

Choose a reason for hiding this comment

Uh oh!

gh-aw-bot commented Jul 17, 2026

Uh oh!

github-actions Bot commented Jul 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Copilot AI commented Jul 17, 2026 •

edited by github-actions Bot

Loading

github-actions Bot commented Jul 17, 2026 •

edited

Loading

github-actions Bot commented Jul 17, 2026 •

edited

Loading

github-actions Bot commented Jul 17, 2026 •

edited

Loading

github-actions Bot commented Jul 17, 2026 •

edited

Loading