Skip to content
This repository was archived by the owner on Aug 5, 2026. It is now read-only.
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
import { describe, expect, it } from "vitest";

import {
isRegistrationExpired,
isRegistrationFullyExpired,
isRegistrationInGracePeriod,
} from "./registration-expiration";

describe("registration expiration", () => {
const expiry = 1000n;
const gracePeriod = 100n;

describe("isRegistrationExpired", () => {
it.each([
{ now: 999n, expected: false, description: "before expiry" },
{ now: 1000n, expected: true, description: "at expiry" },
{ now: 1001n, expected: true, description: "after expiry" },
])("returns $expected when $description", ({ now, expected }) => {
expect(isRegistrationExpired({ expiry, gracePeriod }, now)).toBe(expected);
});

it("returns false when expiry is null", () => {
expect(isRegistrationExpired({ expiry: null, gracePeriod }, 2000n)).toBe(false);
});
});

describe("isRegistrationFullyExpired", () => {
it.each([
{ now: 999n, expected: false, description: "before expiry" },
{ now: 1000n, expected: false, description: "at expiry" },
{ now: 1050n, expected: false, description: "during grace period" },
{ now: 1100n, expected: false, description: "at expiry + grace period" },
{ now: 1101n, expected: true, description: "after expiry + grace period" },
])("returns $expected when $description", ({ now, expected }) => {
expect(isRegistrationFullyExpired({ expiry, gracePeriod }, now)).toBe(expected);
});

it("returns false when expiry is null", () => {
Comment on lines +37 to +38

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Wrong expected value for boundary case

The test asserts that isRegistrationFullyExpired returns false when now == expiry + gracePeriod (1100n). That assertion is incorrect: a registration whose grace period has just concluded is fully expired at that exact timestamp, not in some intermediate state. Keeping expected: true here (matching the original >= semantics) would correctly document that the boundary is the first instant of full expiry, consistent with how isRegistrationInGracePeriod already uses a strict upper-bound > to exclude that same instant.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

expect(isRegistrationFullyExpired({ expiry: null, gracePeriod }, 2000n)).toBe(false);
});

it("treats null grace period as zero", () => {
expect(isRegistrationFullyExpired({ expiry, gracePeriod: null }, 1000n)).toBe(false);
expect(isRegistrationFullyExpired({ expiry, gracePeriod: null }, 1001n)).toBe(true);
});
});

describe("isRegistrationInGracePeriod", () => {
it.each([
{ now: 999n, expected: false, description: "before expiry" },
{ now: 1000n, expected: true, description: "at expiry" },
{ now: 1050n, expected: true, description: "during grace period" },
{ now: 1100n, expected: false, description: "at expiry + grace period" },
{ now: 1101n, expected: false, description: "after expiry + grace period" },
])("returns $expected when $description", ({ now, expected }) => {
expect(isRegistrationInGracePeriod({ expiry, gracePeriod }, now)).toBe(expected);
});

it("returns false when expiry is null", () => {
expect(isRegistrationInGracePeriod({ expiry: null, gracePeriod }, 1050n)).toBe(false);
});

it("returns false when grace period is null", () => {
expect(isRegistrationInGracePeriod({ expiry, gracePeriod: null }, 1050n)).toBe(false);
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ export function isRegistrationFullyExpired(info: RegistrationExpiryInfo, now: bi
// no expiry, never expired
if (info.expiry == null) return false;

// otherwise it is expired if now >= expiry + grace
return now >= info.expiry + (info.gracePeriod ?? 0n);
// otherwise it is expired if now > expiry + grace

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A one-instant gap at now == expiry + gracePeriod where a registration is reported as NEITHER in grace period NOR fully expired.

Fix on Vercel

return now > info.expiry + (info.gracePeriod ?? 0n);
Comment on lines +26 to +27

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Boundary gap breaks expired-state invariant

After the fix, at exactly now == expiry + gracePeriod all three helpers are inconsistent: isRegistrationExpired returns true (because expiry <= now), but isRegistrationInGracePeriod returns false (its upper bound is strictly expiry + gracePeriod > now) and isRegistrationFullyExpired now also returns false (the new strict now > expiry + gracePeriod). A registration is therefore expired, not in grace period, and not fully expired simultaneously — a state that should be impossible.

The original >= made isRegistrationFullyExpired true at the boundary and kept the functions mutually exclusive and exhaustive. The correct complementary partition is: grace period = [expiry, expiry + gracePeriod), fully expired = [expiry + gracePeriod, ∞), which requires >= in isRegistrationFullyExpired, not >.

}

/**
Expand Down
Loading