From 9949b4a21a45acfa2519e42cd5864666396687f4 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 14 Jul 2026 09:24:01 +0000 Subject: [PATCH 1/6] Initial plan From 54ccb81e9c1811cf4eb3be3ed2a3e858478f765c Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 14 Jul 2026 09:42:22 +0000 Subject: [PATCH 2/6] fix eslint require-spawnsync-error-check for destructuring and guarding Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- .../require-spawnsync-error-check.test.ts | 16 ++- .../rules/require-spawnsync-error-check.ts | 115 +++++++++++++++--- 2 files changed, 113 insertions(+), 18 deletions(-) diff --git a/eslint-factory/src/rules/require-spawnsync-error-check.test.ts b/eslint-factory/src/rules/require-spawnsync-error-check.test.ts index d0fb03aa83c..ded6ee08743 100644 --- a/eslint-factory/src/rules/require-spawnsync-error-check.test.ts +++ b/eslint-factory/src/rules/require-spawnsync-error-check.test.ts @@ -23,8 +23,8 @@ describe("require-spawnsync-error-check", () => { `const result = childProcess.spawnSync("git", ["status"]); if (result.error) throw result.error;`, // child_process.spawnSync, checks result.error `const result = child_process.spawnSync("curl", ["-v"]); if (result.error) { throw result.error; } if (result.status !== 0) throw new Error("x");`, - // result is accessed via .error destructuring equivalent — property access - `const r = spawnSync("zip", ["-v"]); const e = r.error; if (e) throw e;`, + // destructured binding includes error and guards on it + `const { status, error } = spawnSync("zip", ["-v"]); if (error) throw error; if (status !== 0) throw new Error("x");`, ], invalid: [], }); @@ -46,6 +46,18 @@ describe("require-spawnsync-error-check", () => { code: `const result = child_process.spawnSync("curl", ["--version"]); return result.stdout;`, errors: [{ messageId: "missingErrorCheck" }], }, + { + code: `const result = spawnSync("git", ["status"]); core.info(String(result.error)); if (result.status !== 0) throw new Error("failed");`, + errors: [{ messageId: "missingErrorCheck" }], + }, + { + code: `const { status } = spawnSync("git", ["status"]); if (status !== 0) throw new Error("failed");`, + errors: [{ messageId: "missingErrorCheck" }], + }, + { + code: `const { status, error } = spawnSync("git", ["status"]); core.info(String(error)); if (status !== 0) throw new Error("failed");`, + errors: [{ messageId: "missingErrorCheck" }], + }, ], }); }); diff --git a/eslint-factory/src/rules/require-spawnsync-error-check.ts b/eslint-factory/src/rules/require-spawnsync-error-check.ts index 727b87382c9..34993a9da7b 100644 --- a/eslint-factory/src/rules/require-spawnsync-error-check.ts +++ b/eslint-factory/src/rules/require-spawnsync-error-check.ts @@ -1,4 +1,4 @@ -import { AST_NODE_TYPES, ESLintUtils, TSESTree } from "@typescript-eslint/utils"; +import { AST_NODE_TYPES, ESLintUtils, TSESLint, TSESTree } from "@typescript-eslint/utils"; const createRule = ESLintUtils.RuleCreator(name => `https://github.com/github/gh-aw/tree/main/eslint-factory#${name}`); @@ -8,6 +8,71 @@ const SPAWNSYNC_NAME = "spawnSync"; // Known namespace aliases for the child_process module. const CHILD_PROCESS_OBJECTS = new Set(["childProcess", "child_process"]); +type ScopeType = ReturnType; +type ScopeVariable = ScopeType["variables"][number]; + +function isGuardingErrorUsage(node: TSESTree.Expression): boolean { + let current: TSESTree.Node = node; + + while (current.parent) { + const parent: TSESTree.Node = current.parent; + + if ((parent.type === AST_NODE_TYPES.IfStatement || parent.type === AST_NODE_TYPES.WhileStatement || parent.type === AST_NODE_TYPES.DoWhileStatement || parent.type === AST_NODE_TYPES.ForStatement) && parent.test === current) { + return true; + } + + if (parent.type === AST_NODE_TYPES.ConditionalExpression && parent.test === current) { + return true; + } + + if (parent.type === AST_NODE_TYPES.LogicalExpression && (parent.operator === "&&" || parent.operator === "||") && (parent.left === current || parent.right === current)) { + return true; + } + + if ((parent.type === AST_NODE_TYPES.ThrowStatement || parent.type === AST_NODE_TYPES.ReturnStatement) && parent.argument === current) { + return true; + } + + if (parent.type === AST_NODE_TYPES.UnaryExpression && parent.operator === "!" && parent.argument === current) { + current = parent; + continue; + } + + break; + } + + return false; +} + +function findVariableByName(sourceCode: Readonly, node: TSESTree.Node, varName: string): ScopeVariable | undefined { + let scope: ReturnType | null = sourceCode.getScope(node); + while (scope) { + const variable = scope.set.get(varName); + if (variable) return variable; + scope = scope.upper; + } + return undefined; +} + +function getErrorBindingNames(node: TSESTree.ObjectPattern): string[] { + const names: string[] = []; + + for (const property of node.properties) { + if (property.type !== AST_NODE_TYPES.Property || property.computed || property.key.type !== AST_NODE_TYPES.Identifier || property.key.name !== "error") { + continue; + } + + const value = property.value; + if (value.type === AST_NODE_TYPES.Identifier) { + names.push(value.name); + } else if (value.type === AST_NODE_TYPES.AssignmentPattern && value.left.type === AST_NODE_TYPES.Identifier) { + names.push(value.left.name); + } + } + + return names; +} + /** * Returns true when the expression is a call to spawnSync (either bare or namespaced). * Matched forms: @@ -45,7 +110,8 @@ export const requireSpawnSyncErrorCheckRule = createRule({ description: "Require spawnSync result variables in actions/setup/js scripts to check result.error in addition to result.status. " + "When spawnSync cannot spawn the child process (e.g. ENOENT, ETIMEDOUT), result.status is null and result.error holds the actual Error — " + - "checking only result.status silently swallows spawn-level failures or reports a misleading 'exit null' message.", + "checking only result.status silently swallows spawn-level failures or reports a misleading 'exit null' message. " + + "Scope: this rule checks variable declarator initializers (including object destructuring) and does not analyze AssignmentExpression forms (`result = spawnSync(...)`) or inline chains (`spawnSync(...).status`).", }, schema: [], messages: { @@ -64,30 +130,47 @@ export const requireSpawnSyncErrorCheckRule = createRule({ if (!node.init) return; if (!isSpawnSyncCall(node.init)) return; - // Only handle simple identifier bindings: const result = spawnSync(...) - if (node.id.type !== AST_NODE_TYPES.Identifier) return; + if (node.id.type === AST_NODE_TYPES.ObjectPattern) { + const errorBindingNames = getErrorBindingNames(node.id); + if (errorBindingNames.length === 0) { + context.report({ node: node.init, messageId: "missingErrorCheck" }); + return; + } - const varName = node.id.name; + const hasGuardingErrorCheck = errorBindingNames.some(varName => { + const variable = findVariableByName(sourceCode, node, varName); + if (!variable) return false; + return variable.references.some(ref => ref.identifier.type === AST_NODE_TYPES.Identifier && isGuardingErrorUsage(ref.identifier)); + }); - // Locate the variable in scope (may be in an outer scope). - let scope: ReturnType | null = sourceCode.getScope(node); - let variable: (typeof scope)["variables"][number] | undefined; - while (scope) { - variable = scope.set.get(varName); - if (variable) break; - scope = scope.upper; + if (!hasGuardingErrorCheck) { + context.report({ node: node.init, messageId: "missingErrorCheck" }); + } + return; } + // Only handle simple identifier bindings: const result = spawnSync(...) + if (node.id.type !== AST_NODE_TYPES.Identifier) return; + + const variable = findVariableByName(sourceCode, node, node.id.name); if (!variable) return; - // Check whether any reference to this variable reads the .error property. - const hasErrorCheck = variable.references.some(ref => { + // Check whether any reference to this variable uses .error in a guarding position. + const hasGuardingErrorCheck = variable.references.some(ref => { const id = ref.identifier; const parent = id.parent; - return parent !== undefined && parent.type === AST_NODE_TYPES.MemberExpression && !parent.computed && parent.object === id && parent.property.type === AST_NODE_TYPES.Identifier && parent.property.name === "error"; + return ( + parent !== undefined && + parent.type === AST_NODE_TYPES.MemberExpression && + !parent.computed && + parent.object === id && + parent.property.type === AST_NODE_TYPES.Identifier && + parent.property.name === "error" && + isGuardingErrorUsage(parent) + ); }); - if (!hasErrorCheck) { + if (!hasGuardingErrorCheck) { context.report({ node: node.init, messageId: "missingErrorCheck" }); } }, From ca26474a4faa11fa68864eec6cf4512da723925b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 14 Jul 2026 09:55:02 +0000 Subject: [PATCH 3/6] refine spawnsync guard semantics and tests Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- .../require-spawnsync-error-check.test.ts | 2 ++ .../src/rules/require-spawnsync-error-check.ts | 18 +++++++++++++++--- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/eslint-factory/src/rules/require-spawnsync-error-check.test.ts b/eslint-factory/src/rules/require-spawnsync-error-check.test.ts index ded6ee08743..3f9ffa399b9 100644 --- a/eslint-factory/src/rules/require-spawnsync-error-check.test.ts +++ b/eslint-factory/src/rules/require-spawnsync-error-check.test.ts @@ -23,6 +23,8 @@ describe("require-spawnsync-error-check", () => { `const result = childProcess.spawnSync("git", ["status"]); if (result.error) throw result.error;`, // child_process.spawnSync, checks result.error `const result = child_process.spawnSync("curl", ["-v"]); if (result.error) { throw result.error; } if (result.status !== 0) throw new Error("x");`, + // logging is fine when there is also a guard + `const result = spawnSync("git", ["status"]); if (result.error) throw result.error; core.info(String(result.error));`, // destructured binding includes error and guards on it `const { status, error } = spawnSync("zip", ["-v"]); if (error) throw error; if (status !== 0) throw new Error("x");`, ], diff --git a/eslint-factory/src/rules/require-spawnsync-error-check.ts b/eslint-factory/src/rules/require-spawnsync-error-check.ts index 34993a9da7b..840e1720878 100644 --- a/eslint-factory/src/rules/require-spawnsync-error-check.ts +++ b/eslint-factory/src/rules/require-spawnsync-error-check.ts @@ -7,6 +7,11 @@ const SPAWNSYNC_NAME = "spawnSync"; // Known namespace aliases for the child_process module. const CHILD_PROCESS_OBJECTS = new Set(["childProcess", "child_process"]); +const CONDITIONAL_TEST_PARENTS = new Set([AST_NODE_TYPES.IfStatement, AST_NODE_TYPES.WhileStatement, AST_NODE_TYPES.DoWhileStatement, AST_NODE_TYPES.ForStatement]); + +function isConditionalTestParent(node: TSESTree.Node): node is TSESTree.IfStatement | TSESTree.WhileStatement | TSESTree.DoWhileStatement | TSESTree.ForStatement { + return CONDITIONAL_TEST_PARENTS.has(node.type); +} type ScopeType = ReturnType; type ScopeVariable = ScopeType["variables"][number]; @@ -17,7 +22,7 @@ function isGuardingErrorUsage(node: TSESTree.Expression): boolean { while (current.parent) { const parent: TSESTree.Node = current.parent; - if ((parent.type === AST_NODE_TYPES.IfStatement || parent.type === AST_NODE_TYPES.WhileStatement || parent.type === AST_NODE_TYPES.DoWhileStatement || parent.type === AST_NODE_TYPES.ForStatement) && parent.test === current) { + if (isConditionalTestParent(parent) && parent.test === current) { return true; } @@ -25,7 +30,7 @@ function isGuardingErrorUsage(node: TSESTree.Expression): boolean { return true; } - if (parent.type === AST_NODE_TYPES.LogicalExpression && (parent.operator === "&&" || parent.operator === "||") && (parent.left === current || parent.right === current)) { + if (parent.type === AST_NODE_TYPES.LogicalExpression && ((parent.operator === "&&" && (parent.left === current || parent.right === current)) || (parent.operator === "||" && parent.left === current))) { return true; } @@ -58,7 +63,13 @@ function getErrorBindingNames(node: TSESTree.ObjectPattern): string[] { const names: string[] = []; for (const property of node.properties) { - if (property.type !== AST_NODE_TYPES.Property || property.computed || property.key.type !== AST_NODE_TYPES.Identifier || property.key.name !== "error") { + // Intentionally only support static `error` keys (`{ error }` / `{ error: err }`). + // Computed keys are ignored. + if (property.type !== AST_NODE_TYPES.Property || property.computed) { + continue; + } + + if (property.key.type !== AST_NODE_TYPES.Identifier || property.key.name !== "error") { continue; } @@ -66,6 +77,7 @@ function getErrorBindingNames(node: TSESTree.ObjectPattern): string[] { if (value.type === AST_NODE_TYPES.Identifier) { names.push(value.name); } else if (value.type === AST_NODE_TYPES.AssignmentPattern && value.left.type === AST_NODE_TYPES.Identifier) { + // Support `{ error = defaultValue }` by tracking the bound identifier name. names.push(value.left.name); } } From 7e09c14dc36110efe3a66c05efca4c110983cadd Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 14 Jul 2026 11:09:39 +0000 Subject: [PATCH 4/6] chore: start pr finisher pass Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- .github/skills/agentic-workflows/SKILL.md | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/skills/agentic-workflows/SKILL.md b/.github/skills/agentic-workflows/SKILL.md index 72145f10bda..aee174f47cd 100644 --- a/.github/skills/agentic-workflows/SKILL.md +++ b/.github/skills/agentic-workflows/SKILL.md @@ -22,6 +22,7 @@ Load these files from `github/gh-aw` (they are not available locally). - `.github/aw/charts-trending.md` - `.github/aw/charts.md` - `.github/aw/cli-commands.md` +- `.github/aw/configure-agentic-engine.md` - `.github/aw/context.md` - `.github/aw/create-agentic-workflow-trigger-details.md` - `.github/aw/create-agentic-workflow.md` From cffa6a3e65c48c311946d18ae82b300d013b68a3 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 14 Jul 2026 11:29:46 +0000 Subject: [PATCH 5/6] fix spawnSync guard-walk edge cases Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- .../require-spawnsync-error-check.test.ts | 26 +++++++++++++++++++ .../rules/require-spawnsync-error-check.ts | 22 +++++++++++++--- 2 files changed, 44 insertions(+), 4 deletions(-) diff --git a/eslint-factory/src/rules/require-spawnsync-error-check.test.ts b/eslint-factory/src/rules/require-spawnsync-error-check.test.ts index 3f9ffa399b9..4d3ce09a60c 100644 --- a/eslint-factory/src/rules/require-spawnsync-error-check.test.ts +++ b/eslint-factory/src/rules/require-spawnsync-error-check.test.ts @@ -25,8 +25,18 @@ describe("require-spawnsync-error-check", () => { `const result = child_process.spawnSync("curl", ["-v"]); if (result.error) { throw result.error; } if (result.status !== 0) throw new Error("x");`, // logging is fine when there is also a guard `const result = spawnSync("git", ["status"]); if (result.error) throw result.error; core.info(String(result.error));`, + // logging before a later guard is still valid + `const result = spawnSync("git", ["status"]); core.debug(String(result.error)); if (result.error) throw result.error;`, // destructured binding includes error and guards on it `const { status, error } = spawnSync("zip", ["-v"]); if (error) throw error; if (status !== 0) throw new Error("x");`, + // renamed destructuring bindings are supported + `const { error: spawnError } = spawnSync("zip", ["-v"]); if (spawnError !== undefined) throw spawnError;`, + // string-literal keys are supported when they bind error + `const { "error": spawnError } = spawnSync("zip", ["-v"]); if (spawnError) throw spawnError;`, + // comparison-based guards should count + `const result = spawnSync("git", ["status"]); if (result.error !== undefined) throw result.error;`, + // result.error on the right side of || can still guard when the full expression is the test + `const result = spawnSync("git", ["status"]); if (result.status !== 0 || result.error) throw result.error;`, ], invalid: [], }); @@ -60,6 +70,22 @@ describe("require-spawnsync-error-check", () => { code: `const { status, error } = spawnSync("git", ["status"]); core.info(String(error)); if (status !== 0) throw new Error("failed");`, errors: [{ messageId: "missingErrorCheck" }], }, + { + code: `const result = spawnSync("git", ["status"]); const cached = result.error && result.error.message; return cached;`, + errors: [{ messageId: "missingErrorCheck" }], + }, + { + code: `const result = spawnSync("git", ["status"]); if (result.status !== 0 && result.error) core.info(String(result.error));`, + errors: [{ messageId: "missingErrorCheck" }], + }, + { + code: `const result = spawnSync("git", ["status"]); const fallback = result.error || new Error("fallback"); throw fallback;`, + errors: [{ messageId: "missingErrorCheck" }], + }, + { + code: `const result = spawnSync("git", ["status"]); const maybeError = result.error ?? null; return maybeError;`, + errors: [{ messageId: "missingErrorCheck" }], + }, ], }); }); diff --git a/eslint-factory/src/rules/require-spawnsync-error-check.ts b/eslint-factory/src/rules/require-spawnsync-error-check.ts index 840e1720878..bc7c6b8b5fd 100644 --- a/eslint-factory/src/rules/require-spawnsync-error-check.ts +++ b/eslint-factory/src/rules/require-spawnsync-error-check.ts @@ -30,8 +30,13 @@ function isGuardingErrorUsage(node: TSESTree.Expression): boolean { return true; } - if (parent.type === AST_NODE_TYPES.LogicalExpression && ((parent.operator === "&&" && (parent.left === current || parent.right === current)) || (parent.operator === "||" && parent.left === current))) { - return true; + if (parent.type === AST_NODE_TYPES.LogicalExpression) { + if (parent.left === current || (parent.operator === "||" && parent.right === current)) { + current = parent; + continue; + } + + break; } if ((parent.type === AST_NODE_TYPES.ThrowStatement || parent.type === AST_NODE_TYPES.ReturnStatement) && parent.argument === current) { @@ -43,6 +48,11 @@ function isGuardingErrorUsage(node: TSESTree.Expression): boolean { continue; } + if (parent.type === AST_NODE_TYPES.BinaryExpression && (parent.left === current || parent.right === current)) { + current = parent; + continue; + } + break; } @@ -59,6 +69,10 @@ function findVariableByName(sourceCode: Readonly, node: TSE return undefined; } +function isGuardingReferenceIdentifier(node: TSESTree.Identifier | TSESTree.JSXIdentifier): boolean { + return node.type === AST_NODE_TYPES.Identifier && isGuardingErrorUsage(node); +} + function getErrorBindingNames(node: TSESTree.ObjectPattern): string[] { const names: string[] = []; @@ -69,7 +83,7 @@ function getErrorBindingNames(node: TSESTree.ObjectPattern): string[] { continue; } - if (property.key.type !== AST_NODE_TYPES.Identifier || property.key.name !== "error") { + if ((property.key.type !== AST_NODE_TYPES.Identifier || property.key.name !== "error") && (property.key.type !== AST_NODE_TYPES.Literal || property.key.value !== "error")) { continue; } @@ -152,7 +166,7 @@ export const requireSpawnSyncErrorCheckRule = createRule({ const hasGuardingErrorCheck = errorBindingNames.some(varName => { const variable = findVariableByName(sourceCode, node, varName); if (!variable) return false; - return variable.references.some(ref => ref.identifier.type === AST_NODE_TYPES.Identifier && isGuardingErrorUsage(ref.identifier)); + return variable.references.some(ref => isGuardingReferenceIdentifier(ref.identifier)); }); if (!hasGuardingErrorCheck) { From c094827f4f02a23a6e4b61147636affde38ddb26 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 14 Jul 2026 11:37:41 +0000 Subject: [PATCH 6/6] clarify spawnSync guard handling Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- .../src/rules/require-spawnsync-error-check.ts | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/eslint-factory/src/rules/require-spawnsync-error-check.ts b/eslint-factory/src/rules/require-spawnsync-error-check.ts index bc7c6b8b5fd..dc84d7373cd 100644 --- a/eslint-factory/src/rules/require-spawnsync-error-check.ts +++ b/eslint-factory/src/rules/require-spawnsync-error-check.ts @@ -31,6 +31,10 @@ function isGuardingErrorUsage(node: TSESTree.Expression): boolean { } if (parent.type === AST_NODE_TYPES.LogicalExpression) { + // Guarding intent depends on how the full logical expression is used: + // climb through the left operand of either operator and the right operand of `||`, + // but reject the right operand of `&&` because it executes conditionally and + // does not establish an independent guard. if (parent.left === current || (parent.operator === "||" && parent.right === current)) { current = parent; continue; @@ -69,8 +73,8 @@ function findVariableByName(sourceCode: Readonly, node: TSE return undefined; } -function isGuardingReferenceIdentifier(node: TSESTree.Identifier | TSESTree.JSXIdentifier): boolean { - return node.type === AST_NODE_TYPES.Identifier && isGuardingErrorUsage(node); +function isErrorKey(node: TSESTree.PropertyName): boolean { + return (node.type === AST_NODE_TYPES.Identifier && node.name === "error") || (node.type === AST_NODE_TYPES.Literal && node.value === "error"); } function getErrorBindingNames(node: TSESTree.ObjectPattern): string[] { @@ -83,7 +87,7 @@ function getErrorBindingNames(node: TSESTree.ObjectPattern): string[] { continue; } - if ((property.key.type !== AST_NODE_TYPES.Identifier || property.key.name !== "error") && (property.key.type !== AST_NODE_TYPES.Literal || property.key.value !== "error")) { + if (!isErrorKey(property.key)) { continue; } @@ -166,7 +170,7 @@ export const requireSpawnSyncErrorCheckRule = createRule({ const hasGuardingErrorCheck = errorBindingNames.some(varName => { const variable = findVariableByName(sourceCode, node, varName); if (!variable) return false; - return variable.references.some(ref => isGuardingReferenceIdentifier(ref.identifier)); + return variable.references.some(ref => ref.identifier.type === AST_NODE_TYPES.Identifier && isGuardingErrorUsage(ref.identifier)); }); if (!hasGuardingErrorCheck) {