[eslint-refiner] ESLint Refiner Daily Report 2026-08-05 #50497
Closed
Replies: 1 comment
|
This discussion was automatically closed because it expired on 2026-08-06T06:14:29.776Z.
|
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.
ESLint Refiner — Daily Report (2026-08-05)
Reviewed the newest custom rule and filed two grounded refinement issues. Also surfaced a large review backlog that's built up since the last logged run.
Key metrics
eslint-factoryrequire-nan-check-after-env-numeric-parse(newest, added 2026-08-03, PR #49962)actions/setup/jscode)Backlog note
27 days elapsed since the last logged review run, during which 24 new rules shipped without a corresponding review (
eslint-minerhas been landing roughly one new rule/day). This run picked the newest rule per the established "lead with the newest never-reviewed rule" convention, but the backlog itself is now the biggest structural risk to this workflow's usefulness — a quarter of the ruleset has never had a false-positive/false-negative pass. Recommend future runs either increase throughput (2-3 rules/run) or explicitly work the backlog oldest-first for a few runs to pay it down. Tracked in repo-memory for continuity.Findings:
require-nan-check-after-env-numeric-parseThis rule flags
parseInt/parseFloat/Number.parseInt/Number.parseFloat/Number()calls onprocess.envvalues that are never checked withisNaN()/Number.isNaN(). Grounding against real call sites inactions/setup/js/**turned up two independent, live defect classes:Issue 1 — False positives: Number.isFinite() and truthiness guards not recognized
The rule's guard-detector only recognizes
isNaN(x)/Number.isNaN(x). Four live sites are already correctly guarded but get flagged anyway:build_checkout_manifest.cjs:26-28— guarded with!Number.isFinite(count) || count < 0check_workflow_timestamp_api.cjs:117,123— guarded withNumber.isFinite(runId)awf_reflect.cjs:147-148— guarded withNumber.isFinite(configuredDelay) && configuredDelay >= 0write_daily_aic_usage_cache.cjs:51-52— guarded withif (!runId)(NaN is falsy, so this correctly rejects it)Acceptance criteria: recognize
Number.isFinite/isFinitecalls and direct truthiness checks (if (!x),if (x), ternary test position) as valid guards; add tests for all four patterns; re-verify the four live sites stop being flagged.Issue 2 — False negative: ternary-wrapped parse call escapes detection entirely, real bug risk
The rule only tracks a variable when its declarator's
initis directly aCallExpression. When the whole parse call is wrapped in a ternary —const x = process.env.FOO ? parseInt(process.env.FOO, 10) : default— the variable is never tracked, guarded or not.Live and risky:
safe_outputs_handlers.cjs:480-482setsmaxSizeKBthis way with zero NaN guard anywhere in the file, then comparessizeKB > maxSizeKB. A malformedGH_AW_ASSETS_MAX_SIZE_KBenv var silently disables the asset size limit (NaNcomparisons are alwaysfalse) — exactly the failure mode this rule exists to catch, but invisible to it today.Acceptance criteria: unwrap
ConditionalExpressioninits and check both branches for a numeric-parse-from-env call; add an invalid-case test for this shape; keep existing ternary-inside-argument tests passing; verify thesafe_outputs_handlers.cjs:480pattern gets flagged.Confirmed the rule's core detection (bare, unguarded
parseInt(process.env.X)-style declarations) works correctly as true positives/negatives at five other live sites:push_repo_memory.cjs,notify_comment_error.cjs,check_rate_limit.cjs,safe-outputs-mcp-server.cjs,handle_noop_message.cjs.Issues created
require-nan-check-after-env-numeric-parse: false positives on Number.isFinite() and truthiness NaN guardsrequire-nan-check-after-env-numeric-parse: ternary-wrapped env parse escapes detection, risking silent bypass of file-size guardNext actions
require-return-after-core-setfailed, 2026-07-10, as the longest-unreviewed candidate) or pick up whatever new rule ships after 2026-08-03.memory/eslint-refinerbranch) updated with full findings, grounding evidence, and the backlog note for continuity.All reactions