Skip to content

Revert "fix(#756,#754,#753,#755): UTC date comparisons in claims, exclude hea…" #797

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
nonsobethel0-dev merged 1 commit into from
Oct 19, 2026
Merged

Revert "fix(#756,#754,#753,#755): UTC date comparisons in claims, exclude hea…" #797

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
52 changes: 0 additions & 52 deletions docs/CLAIMS_TIMEZONE_HANDLING.md
View file
Open in desktop

This file was deleted.

141 changes: 0 additions & 141 deletions src/claims/claims.service.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import { PrismaService } from '../prisma/prisma.service';
import { StatusEventsService } from '../common/events/status-events.service';
import { WebhooksService } from '../common/events/webhooks.service';
import { Prisma } from '@prisma/client';
import { ErrorCode } from '../common/errors/error-codes';

describe('ClaimsService', () => {
let service: ClaimsService;
Expand Down Expand Up @@ -970,144 +969,4 @@ describe('ClaimsService', () => {
expect(summary).toEqual(expect.objectContaining({ scanned: 2, revertedToActive: 1, skipped: 1 }));
});
});

// #756 — date comparisons in claim processing must use UTC against the
// UTC-stored policy.endTime so a server in a non-UTC timezone can't shift
// the coverage boundary by up to a day. These tests pin the comparison at
// a UTC midnight boundary and confirm the outcome is the same regardless
// of how the instant is described.
describe('UTC date comparisons (#756)', () => {
const originalGetTimezoneOffset = Date.prototype.getTimezoneOffset;

afterEach(() => {
// Restore the real timezone offset in case a test mocked it.
Date.prototype.getTimezoneOffset = originalGetTimezoneOffset;
});

// helper: build an active policy whose endTime is a UTC midnight boundary.
const policyEndingAt = (iso: string) => ({
...ACTIVE_POLICY,
status: 'ACTIVE' as const,
endTime: new Date(iso),
});

it('autoProcess returns Expired exactly at a UTC midnight boundary, not a day early', async () => {
// endTime is 2026-06-01T00:00:00.000Z. The instant one millisecond
// before it must be PendingFinalPeriod; exactly at it must be Expired.
// Simulate a server whose local clock is 5 hours behind UTC (UTC-5),
// so a naive local-time comparison would see "2026-05-31T19:00:00"
// and wrongly treat the policy as still active.
Date.prototype.getTimezoneOffset = () => 300; // UTC-5 returns +300
const base = new Date('2026-06-01T00:00:00.000Z');

mockPrismaService.policy.findUnique.mockResolvedValue(policyEndingAt('2026-06-01T00:00:00.000Z'));

// Force the comparison instant: temporarily override Date constructor
// behaviour by passing a policy whose endTime equals `base`. The
// service calls utcNow() internally; we assert the boundary using the
// recoverStuckProcessingPolicies path below, which accepts `now`
// explicitly, to avoid patching the global clock.
const summary = await service.recoverStuckProcessingPolicies(base);
// Not expired path — just confirm the helper doesn't throw and the
// comparison is consistent. The boundary semantics are asserted in the
// explicit-instant tests below.
expect(summary).toBeDefined();
});

it('recoverStuckProcessingPolicies uses UTC for the cutoff, not local time', async () => {
// A server at UTC-5 with a wall-clock of 2026-09-24T07:00:00 local
// has a UTC instant of 2026-09-24T12:00:00. The 30-minute cutoff must
// therefore be 2026-09-24T11:30:00 UTC, not 2026-09-24T06:30:00
// local (which would be 2026-09-24T11:30:00 UTC anyway because the
// cutoff is derived from the epoch). This test pins the expectation.
const nowUtc = new Date('2026-09-24T12:00:00.000Z');
mockPrismaService.policy.findMany.mockResolvedValue([]);

await service.recoverStuckProcessingPolicies(nowUtc);

expect(mockPrismaService.policy.findMany).toHaveBeenCalledWith(expect.objectContaining({
where: { status: 'PROCESSING', updatedAt: { lt: new Date('2026-09-24T11:30:00.000Z') } },
}));
});

it('autoProcess: a policy whose endTime is exactly now is Expired (UTC boundary, no off-by-one)', async () => {
// Build a policy whose endTime is exactly `now`. autoProcess should
// return Expired (not PendingFinalPeriod), proving the comparison is
// `now > endTime` using the UTC instant rather than a local-day offset.
const endTime = new Date('2026-06-01T00:00:00.000Z');
mockPrismaService.policy.findUnique.mockResolvedValue({
...ACTIVE_POLICY,
status: 'ACTIVE',
endTime,
});

// Override utcNow via the Date prototype so the internal utcNow()
// returns exactly `endTime`. We do this by making `new Date()` return
// a fixed instant.
const realDate = Date;
const fixedMs = endTime.getTime();
// eslint-disable-next-line @typescript-eslint/no-explicit-any
(global as any).Date = class extends realDate {
constructor(...args: unknown[]) {
if (args.length === 0) {
super(fixedMs);
} else {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
super(...(args as any));
}
}
static now() {
return fixedMs;
}
};

try {
const result = await service.autoProcess(POLICY_ID);
// endTime == now → not strictly greater than endTime for the Expired
// check, so the next guard (now < endTime) is also false, and the
// flow proceeds to the reservation step. This is the documented
// behaviour: the window is inclusive of endTime. What we're pinning
// is that the comparison is consistent and timezone-independent.
expect(result).not.toBe('PendingFinalPeriod');
} finally {
(global as any).Date = realDate;
}
});

it('submitClaim rejects with CLAIM_POLICY_EXPIRED when endTime has passed in UTC', async () => {
// endTime one second before the fixed now → expired in any timezone.
const now = new Date('2026-06-01T00:00:01.000Z');
const endTime = new Date('2026-06-01T00:00:00.000Z');
mockPrismaService.policy.findUnique.mockResolvedValue({
...ACTIVE_POLICY,
status: 'ACTIVE',
endTime,
});

const realDate = Date;
const fixedMs = now.getTime();
// eslint-disable-next-line @typescript-eslint/no-explicit-any
(global as any).Date = class extends realDate {
constructor(...args: unknown[]) {
if (args.length === 0) {
super(fixedMs);
} else {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
super(...(args as any));
}
}
static now() {
return fixedMs;
}
};

try {
await expect(service.submitClaim(CLAIMANT, POLICY_ID)).rejects.toMatchObject({
response: expect.objectContaining({ errorCode: ErrorCode.CLAIM_POLICY_EXPIRED }),
});
} finally {
(global as any).Date = realDate;
}
});
});
});
22 changes: 4 additions & 18 deletions src/claims/claims.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,6 @@ import { ErrorCode } from '../common/errors/error-codes';
import { compareScaledToThreshold, parseDecimalScaled, parseFixedPointBigInt } from '../common/utils/decimal';
import { financialBatch, financialTransaction, isSerializationFailure } from '../prisma/financial-transaction';
import { PolicyStateCacheService, DerivedPolicyState } from './policy-state-cache.service';
// #756 — UTC helpers centralise date construction so claim-window
// comparisons against Prisma's UTC-stored timestamps are auditable and
// never accidentally rely on server-local timezone behaviour.
import { utcNow } from '../common/utils/time.utils';

export type ClaimResult = 'Paid' | 'Rejected' | 'Expired' | 'AlreadyClaimed' | 'AlreadyProcessed' | 'PolicyNotActive' | 'PendingFinalPeriod';

Expand Down Expand Up @@ -127,14 +123,7 @@ export class ClaimsService {

// Expired check — policy.endTime is the authoritative reference, not the
// lazily-updated status column (the EXPIRED cron runs at best once an hour).
// #756 — `now` is constructed via utcNow() so the comparison is explicit
// about its UTC basis; policy.endTime is materialised by Prisma as UTC,
// and both operands are compared as epoch milliseconds, which are
// timezone-independent. Using utcNow() here keeps server-local timezone
// out of the comparison and prevents off-by-one-day errors when the
// server's local clock is in a non-UTC zone.
const now = utcNow();
if (now > policy.endTime) {
if (new Date() > policy.endTime) {
this.logger.warn(`Policy ${policyId} coverage period ended at ${policy.endTime.toISOString()}`);
return 'Expired';
}
Expand All @@ -147,7 +136,7 @@ export class ClaimsService {
// touching any state until endTime has actually passed — the next hourly
// tick will pick this policy back up (it's still ACTIVE with the same
// endTime) once the full period's data is available.
if (now < policy.endTime) {
if (new Date() < policy.endTime) {
this.logger.log(
`Policy ${policyId} endTime (${policy.endTime.toISOString()}) not yet reached — deferring trigger evaluation`,
);
Expand Down Expand Up @@ -523,7 +512,7 @@ export class ClaimsService {
* Every write is guarded on status = PROCESSING and the same updatedAt
* cutoff, so a policy that makes progress concurrently is left alone.
*/
async recoverStuckProcessingPolicies(now: Date = utcNow()): Promise<StuckPolicyRecoverySummary> {
async recoverStuckProcessingPolicies(now: Date = new Date()): Promise<StuckPolicyRecoverySummary> {
const configured = parseInt(this.config.get<string>('CLAIMS_STUCK_PROCESSING_MINUTES') ?? '', 10);
const stuckMinutes = Number.isFinite(configured) && configured > 0 ? configured : DEFAULT_STUCK_PROCESSING_MINUTES;
const cutoff = new Date(now.getTime() - stuckMinutes * 60 * 1000);
Expand Down Expand Up @@ -667,10 +656,7 @@ export class ClaimsService {

// Expired check — policy.endTime is the authoritative reference, not the
// lazily-updated status column (the EXPIRED cron runs at best once an hour).
// #756 — comparison uses utcNow() against the UTC-stored policy.endTime so
// a server running in a non-UTC timezone can't push a claim past the
// coverage boundary a day early or late.
if (utcNow() > policy.endTime) {
if (new Date() > policy.endTime) {
throw new ConflictException({
message: `Policy ${policyId} coverage period ended at ${policy.endTime.toISOString()}`,
errorCode: ErrorCode.CLAIM_POLICY_EXPIRED,
Expand Down
10 changes: 2 additions & 8 deletions src/claims/claims.worker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,6 @@ import { transition } from '../policy/policy-status.machine';
import { StatusEventsService } from '../common/events/status-events.service';
import { recordWorkerHeartbeat } from '../common/worker-heartbeat';
import { DistributedLockService } from '../redis/distributed-lock.service';
// #756 — UTC helpers make the timezone basis of date comparisons explicit.
import { utcNow, utcPlusMs } from '../common/utils/time.utils';

const BATCH_SIZE = 10;
const PRODUCTS_PAGE_SIZE = 100;
Expand Down Expand Up @@ -108,12 +106,8 @@ export class ClaimsWorker {
this.logger.error('Stuck PROCESSING recovery failed', err);
}

// #756 — `now` and `oneHourOut` are constructed via utcNow()/utcPlusMs()
// so the lookahead window is anchored to the same UTC basis as the
// UTC-stored policy.endTime; without this a server in a non-UTC zone
// could shift the scan window by up to a day.
const now = utcNow();
const oneHourOut = utcPlusMs(now, 60 * 60 * 1000);
const now = new Date();
const oneHourOut = new Date(now.getTime() + 60 * 60 * 1000);

const expiringPolicies = await this.prisma.policy.findMany({
where: {
Expand Down
5 changes: 0 additions & 5 deletions src/claims/policy-state-cache.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -206,11 +206,6 @@ export class PolicyStateCacheService implements OnModuleInit, OnModuleDestroy {
* Never caches failed computations.
*/
async getDerivedPolicyState(policyId: string, now: Date = new Date()): Promise<DerivedPolicyState | null> {
// #756 — default `now` is the current instant; comparisons below use
// epoch milliseconds (getTime / Date.now), which are timezone-
// independent, so the expiry evaluation is correct regardless of the
// server's local timezone. Callers may pass an explicit UTC Date for
// deterministic testing.
const nowMs = now.getTime();

// 1. Check in-flight computation (stampede protection)
Expand Down
45 changes: 0 additions & 45 deletions src/common/telemetry/opentelemetry.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import {
_resetOpenTelemetryState,
registerOtelSignalHandlers,
OpenTelemetryShutdownHook,
parseIgnoreUrls,
} from './opentelemetry';

describe('OpenTelemetry Graceful Shutdown (#715)', () => {
Expand Down Expand Up @@ -139,47 +138,3 @@ describe('OpenTelemetry Graceful Shutdown (#715)', () => {
expect(mockSdk.shutdown).toHaveBeenCalled();
});
});

// #754 — health check endpoints must not generate OpenTelemetry spans so
// load-balancer probes don't drown real request traces in noise.
describe('OpenTelemetry health probe exclusion (#754)', () => {
const originalEnv = process.env;

beforeEach(() => {
process.env = { ...originalEnv };
delete process.env.OTEL_HTTP_INSTRUMENTATION_IGNORE_URLS;
});

afterEach(() => {
process.env = originalEnv;
});

it('ignores the default health probe paths', () => {
const urls = parseIgnoreUrls();
expect(urls).toContain('/api/v1/health');
expect(urls).toContain('/health');
});

it('merges additional patterns from OTEL_HTTP_INSTRUMENTATION_IGNORE_URLS without losing defaults', () => {
process.env.OTEL_HTTP_INSTRUMENTATION_IGNORE_URLS = '/ready,/live';
const urls = parseIgnoreUrls();
expect(urls).toContain('/api/v1/health');
expect(urls).toContain('/health');
expect(urls).toContain('/ready');
expect(urls).toContain('/live');
});

it('dedupes overlapping patterns', () => {
process.env.OTEL_HTTP_INSTRUMENTATION_IGNORE_URLS = '/api/v1/health,/metrics';
const urls = parseIgnoreUrls();
expect(urls.filter((u) => u === '/api/v1/health')).toHaveLength(1);
expect(urls).toContain('/metrics');
});

it('always excludes the built-in health endpoints even when the env var is empty', () => {
process.env.OTEL_HTTP_INSTRUMENTATION_IGNORE_URLS = '';
const urls = parseIgnoreUrls();
expect(urls).toContain('/api/v1/health');
expect(urls).toContain('/health');
});
});
Loading
Loading