You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Review target: eslint-factory custom TypeScript-ESLint rules that lint actions/setup/js/**. The rule factory has grown fast — 46 rules now registered (eslint-factory/src/index.ts), up from 12 the last time this run's memory was current. Today's review targeted no-empty-catch-block, the newest rule and the one with zero prior review history.
Key metrics
Rules registered: 46 (live count from index.ts)
Issues filed today: 2 (non-duplicate, both grounded in live code)
Comment-recognition vocabulary too narrow.no-empty-catch-block only waives an empty catch when its comment contains the literal word "intentional" or "best effort"/"best-effort". Two live, genuinely-intentional empty catches don't use either phrase and would be flagged anyway: git_patch_utils.cjs:219-221 (// Cleanup failure is non-fatal.) and runtime_import.cjs:558-560 (// ...ignore and fall through). Asked for a broadened keyword set while preserving the existing narrow-vocabulary philosophy for vague comments like TODO.
No coverage for empty Promise .catch(() => {}) handlers. The rule only selects CatchClause (try/catch syntax) — it has no equivalent check for a Promise chain terminated in a no-op .catch() callback, which silently swallows rejections just as badly. Neither require-async-entrypoint-catch (only checks a .catch() is present, not that it does anything) nor no-unsafe-promise-catch-error-property (only fires when the callback does touch err unsafely) fills this gap. Two live sites (action_setup_otlp.cjs:198, action_conclusion_otlp.cjs:98) are currently legitimate and comment-documented, but nothing would catch an accidental empty handler introduced elsewhere.
Full grounding detail
True negatives (rule already works correctly): three other live empty catch {} sites — fuzz_template_substitution_harness.cjs:100, load_experiment_state_from_repo.cjs:36, pick_experiment.cjs:172 — are genuinely undocumented fallthrough catches and are correctly flagged as-is by the current rule. Those are application-code gaps to fix, not rule bugs.
Method: read no-empty-catch-block.ts + its test suite; grepped actions/setup/js/**/*.cjs (excluding *.test.cjs, which eslint.config.cjs explicitly ignores) for empty catch bodies and empty .catch(() => {}) promise callbacks; cross-checked sibling rules (require-async-entrypoint-catch, no-unsafe-promise-catch-error-property) to confirm no existing rule already covers the promise-callback gap.
Memory continuity note: this workflow's repo-memory branch (memory/eslint-refiner) had not been updated since 2026-07-08, even though the daily review kept running on GitHub every day through 2026-08-14 (rule count grew 12→46, ~30 issues filed and closed by the automated fix loop in between). Ground truth was reconstructed via a GitHub issue-title search (in:title "eslint-factory") rather than trusting the stale memory file, and the memory has been backfilled/updated accordingly so future runs don't repeat this gap.
Rules with zero review history found today (candidates for upcoming runs): no-core-exportvariable-non-string, no-json-stringify-set-or-map, prefer-get-error-message, prefer-get-error-message-over-string, require-mkdtempsync-try-catch, require-rmsync-try-catch, require-fs-io-try-catch, require-fetch-try-catch, require-fetch-timeout, require-nan-check-after-split-index-parse, prefer-structured-clone, require-execfilesync-try-catch.
Next actions
Await maintainer triage on the 2 new issues filed today.
Next run should lead with an unreviewed rule from the list above — no-json-stringify-set-or-map, require-fs-io-try-catch, and prefer-structured-clone are the top candidates given the pattern of grounded gaps found in reviewed siblings.
Once the comment-vocabulary broadening lands, re-verify both no-empty-catch-block grounded sites stop flagging, then re-check whether the promise-.catch() scope-gap fix (if implemented) still treats the two documented OTLP sites as valid.
reacted with thumbs up emoji reacted with thumbs down emoji reacted with laugh emoji reacted with hooray emoji reacted with confused emoji reacted with heart emoji reacted with rocket emoji reacted with eyes emoji
Uh oh!
There was an error while loading. Please reload this page.
Overview
Review target:
eslint-factorycustom TypeScript-ESLint rules that lintactions/setup/js/**. The rule factory has grown fast — 46 rules now registered (eslint-factory/src/index.ts), up from 12 the last time this run's memory was current. Today's review targetedno-empty-catch-block, the newest rule and the one with zero prior review history.Key metrics
index.ts)no-empty-catch-blockrequire-error-code-in-thrown-errorunderscore-suffix FP), eslint-factory: require-fetch-response-body-try-catch — "will crash the action" message overclaims given the codebase's entrypoi [Content truncated due to length] #52644 (require-fetch-response-body-try-catchoverclaiming message), eslint-factory: require-sync-exec-timeout treats explicittimeout: 0as satisfying the requirement, but 0 means "no timeout" #52645 (require-sync-exec-timeouttreatstimeout: 0as satisfying the check)Findings filed
no-empty-catch-blockonly waives an empty catch when its comment contains the literal word "intentional" or "best effort"/"best-effort". Two live, genuinely-intentional empty catches don't use either phrase and would be flagged anyway:git_patch_utils.cjs:219-221(// Cleanup failure is non-fatal.) andruntime_import.cjs:558-560(// ...ignore and fall through). Asked for a broadened keyword set while preserving the existing narrow-vocabulary philosophy for vague comments likeTODO..catch(() => {})handlers. The rule only selectsCatchClause(try/catch syntax) — it has no equivalent check for a Promise chain terminated in a no-op.catch()callback, which silently swallows rejections just as badly. Neitherrequire-async-entrypoint-catch(only checks a.catch()is present, not that it does anything) norno-unsafe-promise-catch-error-property(only fires when the callback does toucherrunsafely) fills this gap. Two live sites (action_setup_otlp.cjs:198,action_conclusion_otlp.cjs:98) are currently legitimate and comment-documented, but nothing would catch an accidental empty handler introduced elsewhere.Full grounding detail
True negatives (rule already works correctly): three other live empty
catch {}sites —fuzz_template_substitution_harness.cjs:100,load_experiment_state_from_repo.cjs:36,pick_experiment.cjs:172— are genuinely undocumented fallthrough catches and are correctly flagged as-is by the current rule. Those are application-code gaps to fix, not rule bugs.Method: read
no-empty-catch-block.ts+ its test suite; greppedactions/setup/js/**/*.cjs(excluding*.test.cjs, whicheslint.config.cjsexplicitly ignores) for empty catch bodies and empty.catch(() => {})promise callbacks; cross-checked sibling rules (require-async-entrypoint-catch,no-unsafe-promise-catch-error-property) to confirm no existing rule already covers the promise-callback gap.Memory continuity note: this workflow's repo-memory branch (
memory/eslint-refiner) had not been updated since 2026-07-08, even though the daily review kept running on GitHub every day through 2026-08-14 (rule count grew 12→46, ~30 issues filed and closed by the automated fix loop in between). Ground truth was reconstructed via a GitHub issue-title search (in:title "eslint-factory") rather than trusting the stale memory file, and the memory has been backfilled/updated accordingly so future runs don't repeat this gap.Rules with zero review history found today (candidates for upcoming runs):
no-core-exportvariable-non-string,no-json-stringify-set-or-map,prefer-get-error-message,prefer-get-error-message-over-string,require-mkdtempsync-try-catch,require-rmsync-try-catch,require-fs-io-try-catch,require-fetch-try-catch,require-fetch-timeout,require-nan-check-after-split-index-parse,prefer-structured-clone,require-execfilesync-try-catch.Next actions
no-json-stringify-set-or-map,require-fs-io-try-catch, andprefer-structured-cloneare the top candidates given the pattern of grounded gaps found in reviewed siblings.no-empty-catch-blockgrounded sites stop flagging, then re-check whether the promise-.catch()scope-gap fix (if implemented) still treats the two documented OTLP sites as valid.References:
All reactions