[eslint-refiner] Daily ESLint Factory Refinement Report — 2026-08-23 #55005
Closed
Replies: 1 comment
|
This discussion was automatically closed because it expired on 2026-08-24T05:32:21.158Z.
|
0 replies
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Uh oh!
There was an error while loading. Please reload this page.
Overview
First refinement run since 2026-07-08 (a 46-day gap). In that time
eslint-factorygrew from 12 to 53 custom rules, so 41 rules had never been reviewed. This run reviewed 4 of the unreviewed rules in parallel (via background research agents), grounded findings against the liveactions/setup/js/**/*.cjscorpus (nonpm/liveeslintavailable — static grep-based verification only), and filed 3 issues.The standout result: one rule (
require-http-response-error-listener) is already correctly flagging a real, unfixed crash bug in production code — it just isn't blocking anything because every one of the 53 custom rules is registered at"warn"severity, not"error".Key metrics
require-fs-io-try-catch,no-caught-error-interpolation,require-http-response-error-listener,no-setfailed-then-exit-zero)eslint.config.cjs: 53/53 at"warn", 0 at"error"Issues filed
runtime_import.cjs: unhandled response'error'listener crashes on socket-level failures —require-http-response-error-listenercorrectly flagsfetchUrlContent(actions/setup/js/runtime_import.cjs:732-757): the.on('error', ...)is attached to the HTTP request object, not the response (res) passed to the callback. Node emits'error'on the response itself for mid-stream socket failures, so this crashes the action on a dropped connection during a remote@importfetch. Verified as a true positive by re-reading the rule's resolution logic against the exact call site — not a rule bug, a real one the rule already catches. It went unfixed because the diagnostic is a warning, not a CI-blocking error.no-caught-error-interpolation: extend to string-concatenation and single-hop const-alias — the rule only visitsTemplateLiteralnodes, so"Error: " + err(identical failure mode to`Error: ${err}`) andconst msg = err; \${msg}`` both escape detection. No live occurrence in the corpus (it's 100% compliant today), but neither gap has test coverage, so a future refactor could silently reintroduce the bug class.no-setfailed-then-exit-zero: forward-scan stops at nested block boundary — the rule only scans for a followingprocess.exit(0)within the same statement list ascore.setFailed(...). AsetFailednested one block deeper (e.g. inside anifwith noreturn/throw) followed by a siblingexit(0)in the enclosing block is invisible to it. No live occurrence — the codebase is currently disciplined — but it's a real gap in the rule's threat model.Full findings detail, including rules reviewed with no new gaps
require-fs-io-try-catch— reviewed, no issue filed. Both blind spots assumed going in (destructured/aliasedfsbindings, catch-lesstry{}finally{}) are already correctly handled by the sharedtry-catch-rule-utils.ts(scope-resolution walk + explicithandler != nullcheck). Live corpus check found zero bare unwrapped sync fs I/O calls and confirmedgit_patch_utils.cjs:208-217's try/catch/finally is correctly treated as protected. Only gap: the test suite is missing a catch-less-finallyregression case that sibling rules already have — too low-value to file on its own.require-http-response-error-listenerrule mechanics (why the finding above is a true positive, not a rule defect):protocolinruntime_import.cjsresolves via aurl.startsWith('https') ? https : httpternary — the rule'sConditionalExpressionhandling inisHttpModuleExpressionresolves this correctly since both branches arerequire('https')/require('http')bindings.getResponseCallbackcorrectly identifies theres =>arrow.hasErrorListenerscans all references ofresforres.on('error', ...)/res.once('error', ...)— onlyres.on('data', ...)andres.on('end', ...)exist, so it reports. Noeslint-disablepresent.no-caught-error-interpolationgrounding — grepped all non-test.cjsfor bare${err}/${error}/${e}interpolation, string-concatenation, and alias patterns. Every real catch/.then/.on('error')site (check_workflow_timestamp.cjs:61,67,create_issue.cjs:660,1213,mcp_server_core.cjs:1060,safe_outputs_tools_loader.cjs:288,419) already routes throughgetErrorMessage(err)or a precomputederrorMessagevariable — confirmed true negatives.no-setfailed-then-exit-zerogrounding — 58 files callcore.setFailed(; cross-referenced every one against everyprocess.exit(0)/process.exitCode = 0site. Onlyrender_template.cjshas both, in unrelated branches (setFailed+exit(1)at 151-152 vs. an unrelatedexit(0)at 163) — correctly not flagged. A secondary, lower-priority note was folded into the same issue:isProcessExitZeroexcludes computed member access (process['exit'](0)) while thesetFaileddetector supports it (core['setFailed'](...)) — an internal asymmetry, ungrounded in live code.Next actions
"warn"-severity rules for other live true-positive diagnostics that were correctly flagged but never fixed because nothing gates on them (same pattern as theruntime_import.cjsbug). This is likely higher-value than continuing to hunt for new rule-quality gaps, since the rules already exist and may already be silently catching real bugs.state.json); prioritize the resource/error-listener and control-flow families next, since they've been the highest-signal categories so far.no-exec-interpolated-commandalready has an open issue (no-exec-interpolated-command: zero coverage for the execApi parameter-alias pattern dominant in git_helpers.cjs and create_pull_ [Content truncated due to length] #53915, found during this run's dedupe check but not filed by us) — worth checking in on independently of this backlog.memory/eslint-refinerbranch) updated with the full reviewed-rules list, this run's findings, and refreshed operational notes (the git history foreslint-factory/src/ruleswas squashed at some point, so "find the newest rule via git log" no longer works as a heuristic — future runs should pick from the unreviewed-rules list directly).All reactions