Make approval urgency sorting deterministic

This commit is contained in:
rcourtman 2026-05-07 16:45:18 +01:00
parent 8061670b10
commit 8b3ff78ec6
3 changed files with 101 additions and 8 deletions

View file

@ -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

View file

@ -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',
};
}

View file

@ -5,10 +5,10 @@ export interface ApprovalRiskPresentation {
const APPROVAL_RISK_SORT_ORDER: Record<string, number> = {
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);
});
}