feat(eslint): add require-await-core-summary-write rule #43170
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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 | ||
| */ | ||
| 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); | ||
| } | ||
|
|
||
| await coreObj.summary.write(); | ||
|
Contributor
There was a problem hiding this comment. SyntaxError: Line 204 declares 💡 Suggested fixMake the function async function writeStepSummaryWithTokenUsage(coreObj) {
// ... existing body ...
await coreObj.summary.write();
}
// In main() — all three call sites (lines 907, 938, 977):
await writeStepSummaryWithTokenUsage(core);Verified with Node.js: The new ESLint rule correctly refuses to offer the
Contributor
There was a problem hiding this comment. [/diagnosing-bugs] 💡 Correct fixThe ESLint rule's own suggestion logic correctly withholds the // Option A — make the function async
async function writeStepSummaryWithTokenUsage(coreObj) {
// ...
await coreObj.summary.write();
}
// then await every call site
// Option B — return the Promise (no change to callers needed)
function writeStepSummaryWithTokenUsage(coreObj) {
// ...
return coreObj.summary.write();
}The other four fixes ( @copilot please address this. |
||
| } | ||
|
Comment on lines
233
to
237
|
||
|
|
||
| /** | ||
|
|
@@ -904,7 +904,7 @@ async function main() { | |
|
|
||
| setAICreditsRateLimitOutput(core, aiCreditsRateLimitError); | ||
| setUnknownModelAICreditsOutput(core, unknownModelAICredits); | ||
| await writeStepSummaryWithTokenUsage(core); | ||
| return; | ||
| } | ||
| } else { | ||
|
|
@@ -935,7 +935,7 @@ async function main() { | |
| } | ||
| setAICreditsRateLimitOutput(core, aiCreditsRateLimitError); | ||
| setUnknownModelAICreditsOutput(core, unknownModelAICredits); | ||
| 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); | ||
| await writeStepSummaryWithTokenUsage(core); | ||
| return; | ||
| } | ||
|
|
||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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", () => { | ||
|
Contributor
There was a problem hiding this comment. No test for Every 💡 Suggested fixAdd at least these two invalid cases (one per missing function form): // async arrow function
{
code: `const f = async () => { core.summary.write(); };`,
errors: [{ messageId: "requireAwait", suggestions: [{ messageId: "addAwait", output: `const f = async () => { await core.summary.write(); };` }] }],
},
// async function expression
{
code: `const f = async function() { core.summary.write(); };`,
errors: [{ messageId: "requireAwait", suggestions: [{ messageId: "addAwait", output: `const f = async function() { await core.summary.write(); };` }] }],
},Also worth adding: a non-async arrow |
||
| 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(); }`, | ||
|
Contributor
There was a problem hiding this comment. [/tdd] The 💡 Fix the fixtureEither declare // declare p before assignment
`async function f() { let p; p = core.summary.write(); }`
// or use a VariableDeclaration to keep it simple
`async function f() { const p = core.summary.write(); }`This also means the assignment-expression path is only tested by the @copilot please address this. |
||
| 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, { | ||
|
Contributor
There was a problem hiding this comment. [/tdd] Missing edge-case test: bare 💡 Add the test case{
code: `(async () => { core.summary.write(); })()`,
errors: [{ messageId: "requireAwait", suggestions: [{ messageId: "addAwait", output: `(async () => { await core.summary.write(); })()` }] }],
},
@copilot please address this. |
||
| 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(); })()` }], | ||
| }, | ||
| ], | ||
| }, | ||
| ], | ||
| }); | ||
| }); | ||
| }); | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug:
awaitused inside a non-asyncfunction — this will throw a SyntaxError at runtime.writeStepSummaryWithTokenUsageis declared as a plain (non-async) function at line 204:Adding
await coreObj.summary.write()inside it producesSyntaxError: await is only valid in async functionsat parse time. The process will fail before writing the step summary.Two changes are required:
async:main()(~lines 907, 938, 977):Without both changes the bug the rule was meant to fix (unawaited
core.summary.write()) is traded for an outright syntax crash.@copilot please address this.