Recognize Number.isFinite(d.getTime()) as a valid invalid-date guard by Copilot · Pull Request #52968 · github/gh-aw

Copilot · 2026-08-15T21:15:29Z

require-invalid-date-check-before-compare only matched Number.isNaN(x.getTime()) / isNaN(x.getTime()) as an invalid-date guard, so code validated with Number.isFinite(...) — this repo's established idiom for timestamp validation in actions/setup/js — was reported as unguarded.

// previously reported as unguarded; now recognized
const d = new Date(input);
if (!Number.isFinite(d.getTime())) return;
if (d > threshold) doIt();

Changes

Guard matcher generalized: isGetTimeNaNCheck is replaced by extractGetTimeCheckTarget, which extracts the receiver of a <expr>.getTime() sole argument independent of the wrapping check function. The CallExpression handler now accepts isNaN/Number.isNaN and isFinite/Number.isFinite over x.getTime().
Polarity handling: for the isFinite form the enclosing ! negation is also registered as a guard node, so if (!Number.isFinite(d.getTime())) return; satisfies isExitingIfGuard. This reuses the negation logic already present for Date.parse guards, factored out into addGuardPathWithNegation. isNaN polarity is deliberately left unchanged, so a wrong-polarity if (!Number.isNaN(...)) return; still reports.
Unchanged reachability semantics: guardDominatesComparison, isExitingIfGuard, and guardDirectlyGatesComparison apply to the new shapes exactly as before.
Docs/messages: rule description and the requireInvalidDateCheck message now mention !Number.isFinite(x.getTime()) as an accepted check.
Tests: valid cases for exiting !Number.isFinite(d.getTime()) guards (including d.getTime() < x comparisons) and &&-gated Number.isFinite(...) / isFinite(...); invalid cases asserting guard-after-comparison, non-exiting, and nested-branch isFinite guards still report.

Note: five failures in require-fs-io-try-catch.test.ts are pre-existing on this branch and untouched by these changes.

Fixes require-invalid-date-check-before-compare: guard recognition misses the Number.isFinite(d.getTime()) idiom, matching this codeba [Content truncated due to length] #52441

Run context: https://github.com/github/gh-aw/actions/runs/31910845720> Generated by 👨‍🍳 PR Sous Chef · gpt54 · 12.7 AIC · ⌖ 6.91 AIC · ⊞ 8.7K · ◷

_{Comment /souschef to run again}

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

github-actions · 2026-08-15T21:39:36Z

🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅

🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer

github-actions · 2026-08-15T21:39:37Z

✅ Design Decision Gate 🏗️ completed the design decision gate check.

No ADR enforcement needed: PR does not have the 'implementation' label and has 0 new lines of code in business logic directories (threshold: 100).

🏗️ ADR gate enforced by Design Decision Gate 🏗️

github-actions · 2026-08-15T21:39:40Z

✅ Ponytail Reviewer completed successfully!

Ponytail review: no over-engineering found. The refactor (isGetTimeNaNCheck -> extractGetTimeCheckTarget, addGuardPathWithNegation extraction) actually reduces duplication by unifying negation-handling logic that previously existed separately for isFinite guards. No dead code, no speculative abstractions, no unnecessary dependencies. Lean already. Ship.

Generated by Ponytail Reviewer for #52968

github-actions · 2026-08-15T21:39:41Z

✅ PR Code Quality Reviewer completed the code quality review.

🔎 Code quality review by PR Code Quality Reviewer

github-actions · 2026-08-15T21:39:42Z

✅ Test Quality Sentinel completed test quality analysis.

🧪 Test quality analysis by Test Quality Sentinel

github-actions · 2026-08-15T21:43:05Z

Comment Memory

reviewed_at: 2026-08-15T21:40:00Z
review_event: COMMENT
top_themes:
  - no actionable blocking issues in changed lines
  - added coverage for Number.isFinite(getTime()) guards
files_reviewed:
  - eslint-factory/src/rules/require-invalid-date-check-before-compare.ts
  - eslint-factory/src/rules/require-invalid-date-check-before-compare.test.ts
comment_count: 0

Note

This comment is managed by comment memory.

It stores persistent context for this thread in the code block at the top of this comment.
Edit only the text inside the backtick fences; workflow metadata and the footer are regenerated automatically.

Learn more about comment memory

🔎 Code quality review by PR Code Quality Reviewer · gpt54 · 4.39 AIC · ⌖ 5.32 AIC · ⊞ 4.5K · ◷
_{Comment /review to run again}

github-actions

Verdict

COMMENT — I don't see a blocking defect in the changed lines.

Review notes

The rule change is internally consistent: it broadens getTime() guard recognition to isFinite/Number.isFinite, preserves the existing control-flow gating rules, and adds regression coverage for the newly accepted guard shapes plus a few non-dominating counterexamples.

The background grumpy-coder helper was unavailable in this environment, so the review relied on a direct pass over the diff.

🔎 Code quality review by PR Code Quality Reviewer · gpt54 · 4.39 AIC · ⌖ 5.32 AIC · ⊞ 4.5K
_{Comment /review to run again}

github-actions

The implementation is correct and well-tested. extractGetTimeCheckTarget cleanly generalizes the old boolean check to return the receiver node, addGuardPathWithNegation removes the duplicated negation-registration logic, and the CallExpression fall-through to the Date.parse path is preserved. Test coverage is thorough, including both positive (guarded) and negative (guard-after-comparison, non-exiting guard) cases.

🧵 Reviewed using Impeccable skills by Impeccable Skills Reviewer · sonnet46 · 17.9 AIC · ⌖ 7.01 AIC · ⊞ 5.6K

Copilot

Pull request overview

Extends invalid-date validation recognition to support Number.isFinite(d.getTime()) patterns.

Changes:

Generalizes .getTime() guard extraction.
Adds finite-check polarity handling, documentation, and tests.
Preserves existing reachability checks.

Show a summary per file

File	Description
`eslint-factory/src/rules/require-invalid-date-check-before-compare.ts`	Adds finite-check guard recognition.
`eslint-factory/src/rules/require-invalid-date-check-before-compare.test.ts`	Tests valid and ineffective finite guards.

Review details

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

Files reviewed: 2/2 changed files
Comments generated: 1
Review effort level: Balanced

+                if (isFiniteCallee) addGuardPathWithNegation(variable, node);
+                else addGuardPath(variable, sourceCode.getAncestors(node), node);


github-actions · 2026-08-15T21:46:53Z

🧪 Test Quality Sentinel Report

✅ Test Quality Score: 85/100 — Excellent

Analyzed 2 test(s): 2 design, 0 implementation, 0 violation(s).

Note: The changed test file is *.test.ts (TypeScript/vitest), which was not detected by the pre-fetch scanner (configured for *.test.cjs/*.test.js). Tests were analyzed directly from the PR diff.

📊 Metrics (2 tests)

Metric	Value
Analyzed	2 (Go: 0, JS/TS: 2)
✅ Design	2 (100%)
⚠️ Implementation	0 (0%)
Edge/error coverage	1 (50%)
Duplicate clusters	0
Inflation	No (test +32 lines / prod +50 additions = 0.64 ratio)
🚨 Violations	0

Test	File	Classification	Issues
`valid: Number.isFinite(d.getTime()) before comparison`	`require-invalid-date-check-before-compare.test.ts`	`design_test` / `behavioral_contract` / `high_value`	None
`invalid: guards that do not protect the comparison still report`	`require-invalid-date-check-before-compare.test.ts`	`design_test` / `behavioral_contract` / `high_value`	None

Test Function Details

valid: Number.isFinite(d.getTime()) before comparison — Verifies the new guard patterns are accepted: early-return !Number.isFinite(...), throw form, and &&-gated isFinite/Number.isFinite. Covers 4 valid code shapes that the rule must not flag. Happy-path only (as expected for valid-case table).

invalid: guards that do not protect the comparison still report — Verifies that guard-after-comparison, non-exiting guard (core.warning), and nested-branch guard still produce errors. Each case asserts exact messageId (requireInvalidDateCheck) and data (subject, operator, getTimeTarget), giving strong regression protection against over-eager guard acceptance.

Verdict

✅ Passed. 0% implementation tests (threshold: 30%). Both new tests verify behavioral contracts — valid and invalid guard shapes for the new Number.isFinite(d.getTime()) rule extension. No violations detected.

🧪 Test quality analysis by Test Quality Sentinel · sonnet46 · 44.6 AIC · ⌖ 8.65 AIC · ⊞ 7.9K · ◷
_{Comment /review to run again}

github-actions

Skills-Based Review 🧠

Applied /diagnosing-bugs and /tdd — requesting changes on two test coverage gaps.

📋 Key Themes & Highlights

Key Themes

Potential correctness gap: addGuardPathWithNegation always registers the bare call node as a valid guard path. A wrong-polarity if (Number.isFinite(d.getTime())) return; guard (exits on the valid date) may silently pass the rule — this needs a test to confirm it fires or a code fix if it doesn't.
Undocumented invariant: The PR description states !Number.isNaN(d.getTime()) (wrong-polarity isNaN) still reports, but no test asserts it.

Positive Highlights

✅ extractGetTimeCheckTarget is a clean, well-named generalization — returns the receiver directly instead of a boolean, avoiding the double traversal from the old code.
✅ Factoring negation-registration logic into addGuardPathWithNegation is a good refactor — removes the copy-paste between Date.parse and new Date(x) paths.
✅ Invalid test cases cover guard-after-comparison, non-exiting guards, and nested-branch guards — good breadth.

🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · sonnet46 · 50.2 AIC · ⌖ 8.6 AIC · ⊞ 7.7K
_{Comment /matt to run again}

Comments that could not be inline-anchored

eslint-factory/src/rules/require-invalid-date-check-before-compare.ts:303

[/tdd] Wrong-polarity isFinite guard may be silently accepted as valid.

addGuardPathWithNegation always registers the bare CallExpression node as one of the guard paths (line 263). When the code reads if (Number.isFinite(d.getTime())) return; (exits on a valid date — wrong polarity), isExitingIfGuard would match this bare call node as the if test and treat it as a satisfied guard, suppressing the error for a subsequent comparison that is still unprotected.

<details>
<summary…

eslint-factory/src/rules/require-invalid-date-check-before-compare.test.ts:215

[/tdd] The PR description says if (!Number.isNaN(d.getTime())) return; (wrong-polarity isNaN) still reports, but no test covers this.

The isNaN path uses the plain addGuardPath (not addGuardPathWithNegation), so a negated isNaN call would not be registered as a guard — the rule should correctly fire. Adding an explicit test case makes this a documented, regression-protected invariant rather than an undocumented assumption.

<details>
<summary>💡 Suggested test</summary>

it…

</details>

github-actions

✅ Test Quality Sentinel: 85/100 — Excellent. 0% implementation tests (threshold: 30%). Both new tests are design tests covering valid and invalid guard shapes for the Number.isFinite(d.getTime()) extension. No violations.

…mpare

gh-aw-bot · 2026-08-15T22:02:57Z

@copilot Quick triage for maintainer-ready follow-up:

Outstanding review items (newest first):

github-actions: Recognize Number.isFinite(d.getTime()) as a valid invalid-date guard #52968 (comment) — add or reference follow-up that closes the two remaining review asks: confirm the wrong-polarity if (Number.isFinite(d.getTime())) return; case still reports, and add explicit regression coverage for the if (!Number.isNaN(d.getTime())) return; invariant if it is intended to remain unsupported.

Failed checks from the compact candidate set:

None listed.

Branch update was requested automatically for this run when GitHub allows it. Please run the pr-finisher skill before handing this PR back to maintainers.

Run context: https://github.com/github/gh-aw/actions/runs/31910845720

Generated by 👨‍🍳 PR Sous Chef · gpt54 · 12.7 AIC · ⌖ 6.91 AIC · ⊞ 8.7K · ◷
_{Comment /souschef to run again}

github-actions · 2026-08-16T00:55:31Z

🎉 This pull request is included in a new release.

Release: v0.87.0

Initial plan

c1e1d5d

Copilot AI assigned Copilot and pelikhan Aug 15, 2026

Copilot started work on behalf of pelikhan August 15, 2026 21:15 View session

Copilot AI linked an issue Aug 15, 2026 that may be closed by this pull request

require-invalid-date-check-before-compare: guard recognition misses the Number.isFinite(d.getTime()) idiom, matching this codeba [Content truncated due to length] #52441

Closed

4 tasks

Recognize Number.isFinite(d.getTime()) as an invalid-date guard

f845b2a

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

Copilot AI changed the title ~~[WIP] Fix guard recognition for finite date checks~~ Recognize Number.isFinite(d.getTime()) as a valid invalid-date guard Aug 15, 2026

Copilot AI requested a review from pelikhan August 15, 2026 21:24

Copilot finished work on behalf of pelikhan August 15, 2026 21:24

pelikhan marked this pull request as ready for review August 15, 2026 21:38

Copilot AI balanced review requested due to automatic review settings August 15, 2026 21:38

Copilot started reviewing on behalf of pelikhan August 15, 2026 21:39 View session

github-actions Bot reviewed Aug 15, 2026

View reviewed changes

github-actions Bot approved these changes Aug 15, 2026

View reviewed changes

Copilot AI reviewed Aug 15, 2026

View reviewed changes

Comment thread eslint-factory/src/rules/require-invalid-date-check-before-compare.ts

Comment on lines +303 to +304

if (isFiniteCallee) addGuardPathWithNegation(variable, node);

else addGuardPath(variable, sourceCode.getAncestors(node), node);

github-actions Bot requested changes Aug 15, 2026

View reviewed changes

github-actions Bot approved these changes Aug 15, 2026

View reviewed changes

Merge branch 'main' into copilot/require-invalid-date-check-before-co…

247fd4d

…mpare

github-actions Bot mentioned this pull request Aug 15, 2026

[pr-sous-chef] PR Sous Chef run report #52798

Closed

pelikhan merged commit 042383a into main Aug 15, 2026
1 check failed

pelikhan deleted the copilot/require-invalid-date-check-before-compare branch August 15, 2026 22:03

Copilot stopped work on behalf of gh-aw-bot due to an error August 15, 2026 22:03
The session was cancelled by the user.

Copilot AI requested a review from gh-aw-bot August 15, 2026 22:03

		if (isFiniteCallee) addGuardPathWithNegation(variable, node);
		else addGuardPath(variable, sourceCode.getAncestors(node), node);

Uh oh!

Conversation

Copilot AI commented Aug 15, 2026 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

Uh oh!

github-actions Bot commented Aug 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Aug 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Aug 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Aug 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Aug 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Aug 15, 2026

Comment Memory

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Verdict

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Review details

Uh oh!

github-actions Bot commented Aug 15, 2026

🧪 Test Quality Sentinel Report

Test Function Details

Verdict

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Skills-Based Review 🧠

Key Themes

Positive Highlights

Comments that could not be inline-anchored

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Uh oh!

gh-aw-bot commented Aug 15, 2026

Uh oh!

Uh oh!

github-actions Bot commented Aug 16, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Copilot AI commented Aug 15, 2026 •

edited by github-actions Bot

Loading

github-actions Bot commented Aug 15, 2026 •

edited

Loading

github-actions Bot commented Aug 15, 2026 •

edited

Loading

github-actions Bot commented Aug 15, 2026 •

edited

Loading

github-actions Bot commented Aug 15, 2026 •

edited

Loading

github-actions Bot commented Aug 15, 2026 •

edited

Loading