diff --git a/docs/release-control/v6/internal/subsystems/patrol-intelligence.md b/docs/release-control/v6/internal/subsystems/patrol-intelligence.md index 7fa074f84..53872e1c9 100644 --- a/docs/release-control/v6/internal/subsystems/patrol-intelligence.md +++ b/docs/release-control/v6/internal/subsystems/patrol-intelligence.md @@ -781,6 +781,9 @@ approval consumers must treat the approval queue as `soonest expiry first`, then higher risk, then older request time, rather than inheriting raw API order. Approval-linked findings must follow that same ordering so multi-approval `Review` actions jump to the most urgent finding instead of an arbitrary one. +Malformed or missing approval timestamps must sort after valid timestamps and +must not produce non-deterministic comparator results in the shared urgency +utility. Patrol fix approvals also inherit the unified action-governance preflight contract: queued fixes must keep their plan-level dry-run availability, safety checks, verification steps, approval policy, and action id in the shared diff --git a/frontend-modern/src/utils/__tests__/approvalRiskPresentation.test.ts b/frontend-modern/src/utils/__tests__/approvalRiskPresentation.test.ts index 686f5f14f..825dd1cd6 100644 --- a/frontend-modern/src/utils/__tests__/approvalRiskPresentation.test.ts +++ b/frontend-modern/src/utils/__tests__/approvalRiskPresentation.test.ts @@ -1,5 +1,8 @@ import { describe, expect, it } from 'vitest'; -import { getApprovalRiskPresentation } from '@/utils/approvalRiskPresentation'; +import { + getApprovalRiskPresentation, + sortPendingApprovalsByUrgency, +} from '@/utils/approvalRiskPresentation'; describe('approvalRiskPresentation', () => { it('maps high and critical risk to danger styling', () => { @@ -30,4 +33,78 @@ describe('approvalRiskPresentation', () => { it('handles missing values as unknown', () => { expect(getApprovalRiskPresentation().label).toBe('unknown'); }); + + it('sorts approvals by soonest expiry before risk', () => { + const approvals = sortPendingApprovalsByUrgency([ + approval({ id: 'critical-later', expiresAt: '2026-05-07T10:10:00Z', riskLevel: 'critical' }), + approval({ id: 'low-sooner', expiresAt: '2026-05-07T10:05:00Z', riskLevel: 'low' }), + ]); + + expect(approvals.map(({ id }) => id)).toEqual(['low-sooner', 'critical-later']); + }); + + it('sorts same-expiry approvals by descending risk', () => { + const approvals = sortPendingApprovalsByUrgency([ + approval({ id: 'low', riskLevel: 'low' }), + approval({ id: 'high', riskLevel: 'high' }), + approval({ id: 'critical', riskLevel: 'critical' }), + approval({ id: 'medium', riskLevel: 'medium' }), + ]); + + expect(approvals.map(({ id }) => id)).toEqual(['critical', 'high', 'medium', 'low']); + }); + + it('sorts same-expiry and same-risk approvals by older request time', () => { + const approvals = sortPendingApprovalsByUrgency([ + approval({ id: 'newer', requestedAt: '2026-05-07T10:02:00Z' }), + approval({ id: 'older', requestedAt: '2026-05-07T10:01:00Z' }), + ]); + + expect(approvals.map(({ id }) => id)).toEqual(['older', 'newer']); + }); + + it('sorts malformed or missing expiry values after valid expiries', () => { + const approvals = sortPendingApprovalsByUrgency([ + approval({ id: 'missing-expiry', expiresAt: undefined }), + approval({ id: 'malformed-expiry', expiresAt: 'not-a-date' }), + approval({ id: 'valid-expiry', expiresAt: '2026-05-07T10:05:00Z' }), + ]); + + expect(approvals.map(({ id }) => id)).toEqual([ + 'valid-expiry', + 'missing-expiry', + 'malformed-expiry', + ]); + }); + + it('sorts malformed or missing request times after valid request times', () => { + const approvals = sortPendingApprovalsByUrgency([ + approval({ id: 'missing-request', requestedAt: undefined }), + approval({ id: 'malformed-request', requestedAt: 'not-a-date' }), + approval({ id: 'valid-request', requestedAt: '2026-05-07T10:01:00Z' }), + ]); + + expect(approvals.map(({ id }) => id)).toEqual([ + 'valid-request', + 'missing-request', + 'malformed-request', + ]); + }); }); + +function approval(overrides: { + id: string; + expiresAt?: string | undefined; + requestedAt?: string | undefined; + riskLevel?: string; +}) { + const hasOverride = (key: 'expiresAt' | 'requestedAt') => + Object.prototype.hasOwnProperty.call(overrides, key); + + return { + id: overrides.id, + expiresAt: hasOverride('expiresAt') ? overrides.expiresAt : '2026-05-07T10:05:00Z', + requestedAt: hasOverride('requestedAt') ? overrides.requestedAt : '2026-05-07T10:00:00Z', + riskLevel: overrides.riskLevel ?? 'medium', + }; +} diff --git a/frontend-modern/src/utils/approvalRiskPresentation.ts b/frontend-modern/src/utils/approvalRiskPresentation.ts index 9e561585f..6d6a64ac2 100644 --- a/frontend-modern/src/utils/approvalRiskPresentation.ts +++ b/frontend-modern/src/utils/approvalRiskPresentation.ts @@ -5,10 +5,10 @@ export interface ApprovalRiskPresentation { const APPROVAL_RISK_SORT_ORDER: Record = { critical: 0, - high: 0, - medium: 1, - low: 2, - unknown: 3, + high: 1, + medium: 2, + low: 3, + unknown: 4, }; function normalizeApprovalRiskLevel(level?: string): string { @@ -50,16 +50,29 @@ export function getApprovalRiskSortOrder(level?: string): number { return APPROVAL_RISK_SORT_ORDER[normalized] ?? APPROVAL_RISK_SORT_ORDER.unknown; } +function getApprovalTimestampSortValue(value?: string | null): number { + const parsed = Date.parse(value ?? ''); + return Number.isFinite(parsed) ? parsed : Number.POSITIVE_INFINITY; +} + +function compareApprovalTimestamps(a?: string | null, b?: string | null): number { + const aValue = getApprovalTimestampSortValue(a); + const bValue = getApprovalTimestampSortValue(b); + + if (aValue === bValue) return 0; + return aValue < bValue ? -1 : 1; +} + export function sortPendingApprovalsByUrgency< - T extends { expiresAt: string; requestedAt: string; riskLevel?: string }, + T extends { expiresAt?: string | null; requestedAt?: string | null; riskLevel?: string }, >(approvals: T[]): T[] { return [...approvals].sort((a, b) => { - const expiryDiff = new Date(a.expiresAt).getTime() - new Date(b.expiresAt).getTime(); + const expiryDiff = compareApprovalTimestamps(a.expiresAt, b.expiresAt); if (expiryDiff !== 0) return expiryDiff; const riskDiff = getApprovalRiskSortOrder(a.riskLevel) - getApprovalRiskSortOrder(b.riskLevel); if (riskDiff !== 0) return riskDiff; - return new Date(a.requestedAt).getTime() - new Date(b.requestedAt).getTime(); + return compareApprovalTimestamps(a.requestedAt, b.requestedAt); }); }