diff --git a/.github/skills/agentic-workflows/SKILL.md b/.github/skills/agentic-workflows/SKILL.md index 22e2accc575..995e0a670cc 100644 --- a/.github/skills/agentic-workflows/SKILL.md +++ b/.github/skills/agentic-workflows/SKILL.md @@ -31,6 +31,7 @@ Load these files from `github/gh-aw` (they are not available locally). - `.github/aw/debug-agentic-workflow.md` - `.github/aw/dependabot.md` - `.github/aw/deployment-status.md` +- `.github/aw/designer-mappings.md` - `.github/aw/designer.md` - `.github/aw/evals.md` - `.github/aw/experiments.md` @@ -100,4 +101,3 @@ After loading the matching workflow prompt or skill, follow it directly: - Design long-running multi-agent research workflows: `.github/aw/multi-agent-research.md` When the task involves OTEL, OTLP, traces, observability backends, or telemetry-driven analysis, also read and follow `skills/otel-queries/SKILL.md` after loading the matching workflow prompt or skill. - diff --git a/eslint-factory/src/rules/prefer-structured-clone.test.ts b/eslint-factory/src/rules/prefer-structured-clone.test.ts index 1fadf3c3a90..064dc1c3da3 100644 --- a/eslint-factory/src/rules/prefer-structured-clone.test.ts +++ b/eslint-factory/src/rules/prefer-structured-clone.test.ts @@ -80,4 +80,56 @@ describe("prefer-structured-clone", () => { ], }); }); + + it("invalid: no suggestion when the cloned identifier carries function-valued properties", () => { + cjsRuleTester.run("prefer-structured-clone", preferStructuredCloneRule, { + valid: [], + invalid: [ + { + // Shape of actions/setup/js/safe_outputs_tools_loader.cjs: the JSON round-trip + // intentionally drops `tool.handler`, which is re-attached afterwards. + // structuredClone would throw DataCloneError here. + code: [ + `function attachHandlers(tools, handlers) {`, + ` tools.forEach(tool => {`, + ` tool.handler = args => handlers.defaultHandler(args);`, + ` });`, + `}`, + `function register(tool) {`, + ` const toolToRegister = JSON.parse(JSON.stringify(tool));`, + ` if (tool.handler) {`, + ` toolToRegister.handler = tool.handler;`, + ` }`, + ` return toolToRegister;`, + `}`, + ].join("\n"), + errors: [{ messageId: "preferStructuredClone", suggestions: [] }], + }, + { + code: [`function register(tool) {`, ` if (typeof tool.handler === "function") {`, ` }`, ` return JSON.parse(JSON.stringify(tool));`, `}`].join("\n"), + errors: [{ messageId: "preferStructuredClone", suggestions: [] }], + }, + { + code: [`const config = { handler: () => {} };`, `const clone = JSON.parse(JSON.stringify(config));`].join("\n"), + errors: [{ messageId: "preferStructuredClone", suggestions: [] }], + }, + { + // JSON-sourced tool (actions/setup/js/generate_safe_outputs_tools.cjs shape): + // no function-valued property anywhere, so the suggestion is still offered. + code: [`const tools = JSON.parse(readFileSync(path, "utf8"));`, `for (const tool of tools) {`, ` const enhancedTool = JSON.parse(JSON.stringify(tool));`, `}`].join("\n"), + errors: [ + { + messageId: "preferStructuredClone", + suggestions: [ + { + messageId: "replaceWithStructuredClone", + output: [`const tools = JSON.parse(readFileSync(path, "utf8"));`, `for (const tool of tools) {`, ` const enhancedTool = structuredClone(tool);`, `}`].join("\n"), + }, + ], + }, + ], + }, + ], + }); + }); }); diff --git a/eslint-factory/src/rules/prefer-structured-clone.ts b/eslint-factory/src/rules/prefer-structured-clone.ts index 416ceab4dc2..0311607db17 100644 --- a/eslint-factory/src/rules/prefer-structured-clone.ts +++ b/eslint-factory/src/rules/prefer-structured-clone.ts @@ -31,6 +31,23 @@ function isPlainJsonStringifyCall(node: TSESTree.CallExpression): boolean { return node.arguments.length === 1; } +/** + * Returns the property name of a non-computed member expression, or a string literal + * computed access, and undefined otherwise. + */ +function getStaticPropertyName(node: TSESTree.MemberExpression): string | undefined { + if (!node.computed && node.property.type === AST_NODE_TYPES.Identifier) return node.property.name; + if (node.computed && node.property.type === AST_NODE_TYPES.Literal && typeof node.property.value === "string") return node.property.value; + return undefined; +} + +/** + * Returns true when the expression is a function literal (`function () {}` or `() => {}`). + */ +function isFunctionLiteral(node: TSESTree.Node): boolean { + return node.type === AST_NODE_TYPES.FunctionExpression || node.type === AST_NODE_TYPES.ArrowFunctionExpression; +} + export const preferStructuredCloneRule = createRule({ name: "prefer-structured-clone", meta: { @@ -38,7 +55,7 @@ export const preferStructuredCloneRule = createRule({ hasSuggestions: true, docs: { description: - "Prefer structuredClone(...) over JSON.parse(JSON.stringify(...)) for deep-cloning plain data in actions/setup/js scripts. The JSON round-trip is slower, silently drops values it cannot represent (undefined, functions, Date becomes a string), and throws on circular references, whereas structuredClone (Node >=17, available globally in the Node 24 runtime this action targets) clones plain objects and JSON-safe data directly.", + 'Prefer structuredClone(...) over JSON.parse(JSON.stringify(...)) for deep-cloning plain data in actions/setup/js scripts. The JSON round-trip is slower, silently drops values it cannot represent (undefined, functions, Date becomes a string), and throws on circular references, whereas structuredClone (Node >=17, available globally in the Node 24 runtime this action targets) clones plain objects and JSON-safe data directly. The autofix suggestion assumes the cloned value never carries function-valued properties: JSON.stringify silently drops functions while structuredClone throws DataCloneError, so code that intentionally relies on the drop-and-reattach idiom must not be rewritten. The suggestion is therefore withheld when the cloned expression is an identifier whose properties are assigned function literals (or checked with `typeof x.prop === "function"`) anywhere in the same file; the diagnostic is still reported so the round trip can be reviewed by hand.', }, schema: [], messages: { @@ -50,7 +67,41 @@ export const preferStructuredCloneRule = createRule({ create(context) { const sourceCode = context.sourceCode; + // Identifiers observed carrying function-valued properties somewhere in the file. + // Cloning those with structuredClone would throw DataCloneError, so the suggestion + // is withheld for them (the diagnostic is still reported). + const identifiersWithFunctionProperties = new Set(); + const candidates: { node: TSESTree.CallExpression; clonedExpression: TSESTree.Node }[] = []; + return { + AssignmentExpression(node) { + if (node.left.type !== AST_NODE_TYPES.MemberExpression) return; + if (node.left.object.type !== AST_NODE_TYPES.Identifier) return; + if (!isFunctionLiteral(node.right)) return; + identifiersWithFunctionProperties.add(node.left.object.name); + }, + + VariableDeclarator(node) { + // `const x = { handler: () => {} }` also carries a function-valued property. + if (node.id.type !== AST_NODE_TYPES.Identifier) return; + if (!node.init || node.init.type !== AST_NODE_TYPES.ObjectExpression) return; + const hasFunctionProperty = node.init.properties.some(property => property.type === AST_NODE_TYPES.Property && (isFunctionLiteral(property.value) || property.method)); + if (hasFunctionProperty) identifiersWithFunctionProperties.add(node.id.name); + }, + + BinaryExpression(node) { + // `typeof x.prop === "function"` is strong evidence that `x` carries a function property. + if (node.operator !== "===" && node.operator !== "==" && node.operator !== "!==" && node.operator !== "!=") return; + const [typeofSide, literalSide] = node.left.type === AST_NODE_TYPES.UnaryExpression ? [node.left, node.right] : [node.right, node.left]; + if (typeofSide.type !== AST_NODE_TYPES.UnaryExpression || typeofSide.operator !== "typeof") return; + if (literalSide.type !== AST_NODE_TYPES.Literal || literalSide.value !== "function") return; + const argument = typeofSide.argument; + if (argument.type !== AST_NODE_TYPES.MemberExpression) return; + if (argument.object.type !== AST_NODE_TYPES.Identifier) return; + if (getStaticPropertyName(argument) === undefined) return; + identifiersWithFunctionProperties.add(argument.object.name); + }, + CallExpression(node) { if (!isJsonParseCall(node)) return; if (node.arguments.length !== 1) return; @@ -59,21 +110,30 @@ export const preferStructuredCloneRule = createRule({ if (innerArg.type !== AST_NODE_TYPES.CallExpression) return; if (!isPlainJsonStringifyCall(innerArg)) return; - const clonedExpressionText = sourceCode.getText(innerArg.arguments[0]); + candidates.push({ node, clonedExpression: innerArg.arguments[0] }); + }, + + "Program:exit"() { + for (const { node, clonedExpression } of candidates) { + const clonedExpressionText = sourceCode.getText(clonedExpression); + const carriesFunctionProperties = clonedExpression.type === AST_NODE_TYPES.Identifier && identifiersWithFunctionProperties.has(clonedExpression.name); - context.report({ - node, - messageId: "preferStructuredClone", - data: { arg: clonedExpressionText }, - suggest: [ - { - messageId: "replaceWithStructuredClone", - fix(fixer: TSESLint.RuleFixer) { - return fixer.replaceText(node, `structuredClone(${clonedExpressionText})`); - }, - }, - ], - }); + context.report({ + node, + messageId: "preferStructuredClone", + data: { arg: clonedExpressionText }, + suggest: carriesFunctionProperties + ? [] + : [ + { + messageId: "replaceWithStructuredClone", + fix(fixer: TSESLint.RuleFixer) { + return fixer.replaceText(node, `structuredClone(${clonedExpressionText})`); + }, + }, + ], + }); + } }, }; },