diff --git a/actions/setup/js/log_parser_bootstrap.cjs b/actions/setup/js/log_parser_bootstrap.cjs index 2cd007a9de5..bb334590380 100644 --- a/actions/setup/js/log_parser_bootstrap.cjs +++ b/actions/setup/js/log_parser_bootstrap.cjs @@ -295,7 +295,7 @@ async function runLogParser(options) { } } - core.summary.addRaw(fullMarkdown).write(); + await core.summary.addRaw(fullMarkdown).write(); } else { // Fallback path: markdown exists but no structured log entries were parsed. // Suppress the "parsed successfully" message for Claude since it always produces @@ -327,7 +327,7 @@ async function runLogParser(options) { fullMarkdown += "\n" + safeOutputsMarkdown; } } - core.summary.addRaw(fullMarkdown).write(); + await core.summary.addRaw(fullMarkdown).write(); } } else { core.error(`Failed to parse ${parserName} log`); diff --git a/actions/setup/js/parse_firewall_logs.cjs b/actions/setup/js/parse_firewall_logs.cjs index 90113117b0b..196f7e7d83a 100644 --- a/actions/setup/js/parse_firewall_logs.cjs +++ b/actions/setup/js/parse_firewall_logs.cjs @@ -76,7 +76,7 @@ async function main() { requestsByDomain, }); - core.summary.addRaw(summary).write(); + await core.summary.addRaw(summary).write(); core.info("Firewall log summary generated successfully"); } catch (error) { core.setFailed(`${ERR_PARSE}: ${error instanceof Error ? error.message : String(error)}`); diff --git a/actions/setup/js/parse_mcp_gateway_log.cjs b/actions/setup/js/parse_mcp_gateway_log.cjs index 1fc2f0c378c..e8b83fa253b 100644 --- a/actions/setup/js/parse_mcp_gateway_log.cjs +++ b/actions/setup/js/parse_mcp_gateway_log.cjs @@ -201,7 +201,7 @@ function generateTokenUsageSummary(summary) { * so callers don't need to chain addRaw() + write() themselves. * @param {typeof import('@actions/core')} coreObj - The GitHub Actions core object */ -function writeStepSummaryWithTokenUsage(coreObj) { +async function writeStepSummaryWithTokenUsage(coreObj) { if (!fs.existsSync(TOKEN_USAGE_PATH)) { coreObj.debug(`No token-usage.jsonl found at: ${TOKEN_USAGE_PATH}`); } else { @@ -233,7 +233,7 @@ function writeStepSummaryWithTokenUsage(coreObj) { coreObj.summary.addRaw(timelineMd); } - coreObj.summary.write(); + await coreObj.summary.write(); } /** @@ -904,7 +904,7 @@ async function main() { setAICreditsRateLimitOutput(core, aiCreditsRateLimitError); setUnknownModelAICreditsOutput(core, unknownModelAICredits); - writeStepSummaryWithTokenUsage(core); + await writeStepSummaryWithTokenUsage(core); return; } } else { @@ -935,7 +935,7 @@ async function main() { } setAICreditsRateLimitOutput(core, aiCreditsRateLimitError); setUnknownModelAICreditsOutput(core, unknownModelAICredits); - writeStepSummaryWithTokenUsage(core); + await writeStepSummaryWithTokenUsage(core); return; } @@ -974,7 +974,7 @@ async function main() { core.info("MCP gateway log files are empty or missing"); setAICreditsRateLimitOutput(core, aiCreditsRateLimitError); setUnknownModelAICreditsOutput(core, unknownModelAICredits); - writeStepSummaryWithTokenUsage(core); + await writeStepSummaryWithTokenUsage(core); return; } diff --git a/actions/setup/js/parse_mcp_scripts_logs.cjs b/actions/setup/js/parse_mcp_scripts_logs.cjs index 075a876339e..0471c78efc3 100644 --- a/actions/setup/js/parse_mcp_scripts_logs.cjs +++ b/actions/setup/js/parse_mcp_scripts_logs.cjs @@ -63,7 +63,7 @@ async function main() { // Generate step summary const summary = generateMCPScriptsSummary(allLogEntries); - core.summary.addRaw(summary).write(); + await core.summary.addRaw(summary).write(); } catch (error) { core.setFailed(`${ERR_PARSE}: ${getErrorMessage(error)}`); } diff --git a/actions/setup/js/upload_assets.cjs b/actions/setup/js/upload_assets.cjs index 46b2ea10959..e0a8217bd21 100644 --- a/actions/setup/js/upload_assets.cjs +++ b/actions/setup/js/upload_assets.cjs @@ -197,7 +197,7 @@ async function main() { core.summary.addRaw(`- [\`${asset.fileName}\`](${asset.url}) → \`${asset.targetFileName}\` (${asset.size} bytes)`); } } - core.summary.write(); + await core.summary.write(); } else { core.info("No new assets to upload"); } diff --git a/eslint-factory/eslint.config.cjs b/eslint-factory/eslint.config.cjs index 07fb1eed8f9..ffc0f025370 100644 --- a/eslint-factory/eslint.config.cjs +++ b/eslint-factory/eslint.config.cjs @@ -17,6 +17,7 @@ module.exports = [ "gh-aw-custom/no-unsafe-promise-catch-error-property": "warn", "gh-aw-custom/prefer-get-error-message": "warn", "gh-aw-custom/require-async-entrypoint-catch": "warn", + "gh-aw-custom/require-await-core-summary-write": "warn", "gh-aw-custom/require-json-parse-try-catch": "warn", "gh-aw-custom/require-parseInt-radix": "warn", }, diff --git a/eslint-factory/src/index.ts b/eslint-factory/src/index.ts index ce230b13441..854ea1d1f0f 100644 --- a/eslint-factory/src/index.ts +++ b/eslint-factory/src/index.ts @@ -3,6 +3,7 @@ import { noUnsafeCatchErrorPropertyRule } from "./rules/no-unsafe-catch-error-pr import { noUnsafePromiseCatchErrorPropertyRule } from "./rules/no-unsafe-promise-catch-error-property"; import { preferGetErrorMessageRule } from "./rules/prefer-get-error-message"; import { requireAsyncEntrypointCatchRule } from "./rules/require-async-entrypoint-catch"; +import { requireAwaitCoreSummaryWriteRule } from "./rules/require-await-core-summary-write"; import { requireJsonParseTryCatchRule } from "./rules/require-json-parse-try-catch"; import { requireParseIntRadixRule } from "./rules/require-parseInt-radix"; @@ -17,6 +18,7 @@ const plugin = { "no-unsafe-promise-catch-error-property": noUnsafePromiseCatchErrorPropertyRule, "prefer-get-error-message": preferGetErrorMessageRule, "require-async-entrypoint-catch": requireAsyncEntrypointCatchRule, + "require-await-core-summary-write": requireAwaitCoreSummaryWriteRule, "require-json-parse-try-catch": requireJsonParseTryCatchRule, "require-parseInt-radix": requireParseIntRadixRule, }, diff --git a/eslint-factory/src/rules/require-await-core-summary-write.test.ts b/eslint-factory/src/rules/require-await-core-summary-write.test.ts new file mode 100644 index 00000000000..44a9b9e62a4 --- /dev/null +++ b/eslint-factory/src/rules/require-await-core-summary-write.test.ts @@ -0,0 +1,166 @@ +import { RuleTester } from "eslint"; +import { describe, expect, it } from "vitest"; +import { requireAwaitCoreSummaryWriteRule } from "./require-await-core-summary-write"; + +const cjsRuleTester = new RuleTester({ + languageOptions: { + ecmaVersion: 2022, + sourceType: "commonjs", + }, +}); + +describe("require-await-core-summary-write", () => { + it("uses the correct docs URL", () => { + expect(requireAwaitCoreSummaryWriteRule.meta.docs.url).toBe("https://github.com/github/gh-aw/tree/main/eslint-factory#require-await-core-summary-write"); + }); + + it("valid: awaited calls are not flagged", () => { + cjsRuleTester.run("require-await-core-summary-write", requireAwaitCoreSummaryWriteRule, { + valid: [ + `async function f() { await core.summary.write(); }`, + `async function f() { await core.summary.addRaw(x).write(); }`, + `async function f() { await core.summary.addHeading("h").addRaw(x).write(); }`, + `async function f() { await coreObj.summary.write(); }`, + ], + invalid: [], + }); + }); + + it("valid: returned and assigned calls are not flagged", () => { + cjsRuleTester.run("require-await-core-summary-write", requireAwaitCoreSummaryWriteRule, { + valid: [ + `async function f() { return core.summary.write(); }`, + `async function f() { const p = core.summary.write(); }`, + `async function f() { return core.summary.addRaw(x).write(); }`, + // assignment expression (without declaration) also propagates the Promise + `async function f() { let p; p = core.summary.write(); }`, + ], + invalid: [], + }); + }); + + it("invalid: bare core.summary.write() is flagged", () => { + cjsRuleTester.run("require-await-core-summary-write", requireAwaitCoreSummaryWriteRule, { + valid: [], + invalid: [ + { + code: `async function f() { core.summary.write(); }`, + errors: [{ messageId: "requireAwait", suggestions: [{ messageId: "addAwait", output: `async function f() { await core.summary.write(); }` }] }], + }, + { + code: `const f = async () => { core.summary.write(); };`, + errors: [{ messageId: "requireAwait", suggestions: [{ messageId: "addAwait", output: `const f = async () => { await core.summary.write(); };` }] }], + }, + { + code: `const f = async function() { core.summary.write(); };`, + errors: [{ messageId: "requireAwait", suggestions: [{ messageId: "addAwait", output: `const f = async function() { await core.summary.write(); };` }] }], + }, + ], + }); + }); + + it("invalid: chained core.summary.addRaw(x).write() is flagged", () => { + cjsRuleTester.run("require-await-core-summary-write", requireAwaitCoreSummaryWriteRule, { + valid: [], + invalid: [ + { + code: `async function f() { core.summary.addRaw(summary).write(); }`, + errors: [{ messageId: "requireAwait", suggestions: [{ messageId: "addAwait", output: `async function f() { await core.summary.addRaw(summary).write(); }` }] }], + }, + { + code: `async function f() { core.summary.addHeading("Title").addRaw(body).write(); }`, + errors: [{ messageId: "requireAwait", suggestions: [{ messageId: "addAwait", output: `async function f() { await core.summary.addHeading("Title").addRaw(body).write(); }` }] }], + }, + ], + }); + }); + + it("invalid: coreObj alias and computed access are flagged", () => { + cjsRuleTester.run("require-await-core-summary-write", requireAwaitCoreSummaryWriteRule, { + valid: [], + invalid: [ + { + code: `async function f() { coreObj.summary.write(); }`, + errors: [{ messageId: "requireAwait", suggestions: [{ messageId: "addAwait", output: `async function f() { await coreObj.summary.write(); }` }] }], + }, + { + code: `async function f() { core.summary["write"](); }`, + errors: [{ messageId: "requireAwait", suggestions: [{ messageId: "addAwait", output: `async function f() { await core.summary["write"](); }` }] }], + }, + ], + }); + }); + + it("valid: unrelated .write() calls are not flagged", () => { + cjsRuleTester.run("require-await-core-summary-write", requireAwaitCoreSummaryWriteRule, { + valid: [`fs.write(fd, buffer);`, `stream.write(data);`, `core.info("hello");`, `foo.bar.write();`, `fs.summary.write();`, `db.summary.write();`, `foo.bar.summary.write();`], + invalid: [], + }); + }); + + it("invalid: flagged outside async function — no suggestion offered", () => { + cjsRuleTester.run("require-await-core-summary-write", requireAwaitCoreSummaryWriteRule, { + valid: [], + invalid: [ + { + // Top-level call: flagged, but no suggestion (await is not valid here) + code: `core.summary.write();`, + errors: [{ messageId: "requireAwait", suggestions: [] }], + }, + { + // Inside a non-async function: flagged, but no suggestion + code: `function f() { core.summary.write(); }`, + errors: [{ messageId: "requireAwait", suggestions: [] }], + }, + { + code: `const f = () => { core.summary.write(); };`, + errors: [{ messageId: "requireAwait", suggestions: [] }], + }, + ], + }); + }); + + it("suggestion: inserts 'await ' before the expression", () => { + cjsRuleTester.run("require-await-core-summary-write", requireAwaitCoreSummaryWriteRule, { + valid: [], + invalid: [ + { + code: `async function f() { core.summary.write(); }`, + errors: [ + { + messageId: "requireAwait", + suggestions: [{ messageId: "addAwait", output: `async function f() { await core.summary.write(); }` }], + }, + ], + }, + { + code: `async function f() { core.summary.addRaw(summary).write(); }`, + errors: [ + { + messageId: "requireAwait", + suggestions: [{ messageId: "addAwait", output: `async function f() { await core.summary.addRaw(summary).write(); }` }], + }, + ], + }, + { + code: `async function f() { coreObj.summary.write(); }`, + errors: [ + { + messageId: "requireAwait", + suggestions: [{ messageId: "addAwait", output: `async function f() { await coreObj.summary.write(); }` }], + }, + ], + }, + { + code: `(async () => { core.summary.write(); })()`, + errors: [ + { + messageId: "requireAwait", + suggestions: [{ messageId: "addAwait", output: `(async () => { await core.summary.write(); })()` }], + }, + ], + }, + ], + }); + }); +}); diff --git a/eslint-factory/src/rules/require-await-core-summary-write.ts b/eslint-factory/src/rules/require-await-core-summary-write.ts new file mode 100644 index 00000000000..aede472e3d4 --- /dev/null +++ b/eslint-factory/src/rules/require-await-core-summary-write.ts @@ -0,0 +1,125 @@ +import { ESLintUtils, TSESLint, TSESTree } from "@typescript-eslint/utils"; + +const createRule = ESLintUtils.RuleCreator(name => `https://github.com/github/gh-aw/tree/main/eslint-factory#${name}`); + +const ASYNC_FUNCTION_TYPES = new Set(["FunctionDeclaration", "FunctionExpression", "ArrowFunctionExpression"]); + +/** + * Checks whether a MemberExpression property is "write" (direct or computed string-literal access). + */ +function isWriteProperty(node: TSESTree.MemberExpression): boolean { + const property = node.property; + const isDirectAccess = !node.computed && property.type === "Identifier" && property.name === "write"; + const isComputedAccess = node.computed && property.type === "Literal" && property.value === "write"; + return isDirectAccess || isComputedAccess; +} + +function getRootIdentifier(node: TSESTree.Node): string | null { + if (node.type === "Identifier") return node.name; + if (node.type === "MemberExpression") return getRootIdentifier(node.object); + if (node.type === "CallExpression") return getRootIdentifier(node.callee); + return null; +} + +function isCoreLikeIdentifier(name: string): boolean { + return /^core/i.test(name); +} + +/** + * Checks whether a node is rooted in a `.summary` member access, possibly through + * a chain of method calls (e.g., `core.summary.addRaw(x).write()`). + * + * Accepted patterns (non-exhaustive): + * - `core.summary` + * - `core.summary.addRaw(x)` + * - `core.summary.addHeading(...).addRaw(x)` + * - `coreObj.summary` (any identifier alias) + */ +function rootsSummary(node: TSESTree.Node): boolean { + const rootIdentifier = getRootIdentifier(node); + if (!rootIdentifier || !isCoreLikeIdentifier(rootIdentifier)) return false; + if (node.type === "MemberExpression") { + const property = node.property; + const isSummaryProp = (!node.computed && property.type === "Identifier" && property.name === "summary") || (node.computed && property.type === "Literal" && property.value === "summary"); + if (isSummaryProp) return true; + } + if (node.type === "CallExpression" && node.callee.type === "MemberExpression") { + return rootsSummary(node.callee.object); + } + return false; +} + +/** + * Returns true when the statement is directly inside an async function body. + * Walking up the ancestors, the first function boundary found determines + * whether `await` is currently valid — the suggestion is only safe to apply + * in an async context. + */ +function isInsideAsyncFunction(ancestors: TSESTree.Node[]): boolean { + for (let i = ancestors.length - 1; i >= 0; i--) { + const ancestor = ancestors[i]; + if (ASYNC_FUNCTION_TYPES.has(ancestor.type)) { + return (ancestor as TSESTree.FunctionDeclaration | TSESTree.FunctionExpression | TSESTree.ArrowFunctionExpression).async; + } + } + return false; +} + +export const requireAwaitCoreSummaryWriteRule = createRule({ + name: "require-await-core-summary-write", + meta: { + type: "problem", + hasSuggestions: true, + docs: { + description: + "Require core.summary.write() calls to be awaited; the returned Promise is silently discarded when called without await, which can truncate or drop the step summary if the process exits before the microtask queue drains.", + }, + schema: [], + messages: { + requireAwait: "core.summary.write() returns a Promise that must be awaited; omitting await silently discards the promise and can cause the step summary to be truncated or missing.", + addAwait: "Insert 'await' before the expression.", + }, + }, + defaultOptions: [], + create(context) { + return { + ExpressionStatement(node) { + const expr = node.expression; + + // Only flag bare expression statements — AwaitExpression, ReturnStatement, + // VariableDeclaration, and AssignmentExpression propagate the Promise to the + // caller and are not flagged (zero false positives on existing correct uses). + if (expr.type !== "CallExpression") return; + + const callee = expr.callee; + if (callee.type !== "MemberExpression") return; + + // Property must be `write` (direct or computed string-literal access) + if (!isWriteProperty(callee)) return; + + // Object must trace back through a `.summary` member access + if (!rootsSummary(callee.object)) return; + + // Only offer the `await` suggestion when already inside an async function — + // applying `await` outside an async context would produce a syntax error. + const ancestors = context.sourceCode.getAncestors(node); + const suggest = isInsideAsyncFunction(ancestors) + ? [ + { + messageId: "addAwait" as const, + fix(fixer: TSESLint.RuleFixer) { + return fixer.insertTextBefore(expr, "await "); + }, + }, + ] + : []; + + context.report({ + node: expr, + messageId: "requireAwait", + suggest, + }); + }, + }; + }, +});