From 2160f7500c20b1c9b9a9e0e563a490bbb13a3f9a Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Sun, 16 Aug 2026 16:04:18 -0700 Subject: [PATCH] FN-9132: order approval audits by lifecycle on timestamp ties Ensure approval audit histories preserve lifecycle chronology when events share a timestamp. - rank tied audit events by the declared approval lifecycle before audit ID - cover tied and distinct timestamps through module and public store surfaces - document the diagnosed satellite assertion and add a patch changeset Files changed: .changeset/fn-9132-approval-audit-order.md | 7 +++ .../suite-only-flakes-observed-register.md | 27 +++++++++++ ...oval-request-audit-project-isolation.pg.test.ts | 2 +- .../postgres/approval-request-lifecycle.pg.test.ts | 53 +++++++++++++++++++++- .../async-stores/async-approval-request-store.ts | 17 +++++-- 5 files changed, 100 insertions(+), 6 deletions(-) Fusion-Task-Id: FN-9132 Fusion-Task-Lineage: 2f49e896-69bf-48c8-b813-4b1b2b72fc65 Co-authored-by: Fusion (runfusion.ai) --- .changeset/fn-9132-approval-audit-order.md | 7 +++ .../suite-only-flakes-observed-register.md | 27 ++++++++++ ...request-audit-project-isolation.pg.test.ts | 2 +- .../approval-request-lifecycle.pg.test.ts | 53 ++++++++++++++++++- .../async-approval-request-store.ts | 17 ++++-- 5 files changed, 100 insertions(+), 6 deletions(-) create mode 100644 .changeset/fn-9132-approval-audit-order.md diff --git a/.changeset/fn-9132-approval-audit-order.md b/.changeset/fn-9132-approval-audit-order.md new file mode 100644 index 0000000000..5cefd54212 --- /dev/null +++ b/.changeset/fn-9132-approval-audit-order.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: Keep approval audit timelines in lifecycle order when events share a timestamp. +category: fix +dev: `getApprovalAuditHistory` now applies an event lifecycle-rank tiebreak before audit ID. diff --git a/docs/solutions/test-failures/suite-only-flakes-observed-register.md b/docs/solutions/test-failures/suite-only-flakes-observed-register.md index 04a8ec7b26..42d8b591f0 100644 --- a/docs/solutions/test-failures/suite-only-flakes-observed-register.md +++ b/docs/solutions/test-failures/suite-only-flakes-observed-register.md @@ -295,3 +295,30 @@ It is the companion to entries 4, 5, and 8: FN-8936 fixed detached test-node han | `pnpm lint`, `pnpm verify:fast`, `pnpm build` | **passed** | No UI surface changed; this was a state-ownership and regression-coverage repair. The existing patch changeset remains applicable because Planning Mode behavior is user-visible. + +## 12. Satellite approval audit lifecycle ordering assertion + +- **File:** `packages/core/src/__tests__/postgres/satellite-stores.pg.test.ts` +- **Exact test:** `PostgreSQL satellite stores (U6 consolidated, shared harness) > PostgreSQL satellite DB-injected stores (VAL-DATA-016) > ApprovalRequestStore: replayed/conflicting decisions 409, grants expire, ownership enforced` +- **Owner:** FN-9132 +- **Observed tree/SHA:** deterministic pre-fix reproduction on `b31be1ba7c7415b9ee20c4c76875c961be73a0c3`; structural fix begins at `c3e3a2648a`. +- **Observed frequency:** co-observed in retained FN-9125 12-worker PostgreSQL-directory, FN-9129 4-worker full-core run 1, and FN-9130 loaded-measurement evidence. + +Verbatim observed failure: + +``` +FAIL src/__tests__/postgres/satellite-stores.pg.test.ts > PostgreSQL satellite stores (U6 consolidated, shared harness) > PostgreSQL satellite DB-injected stores (VAL-DATA-016) > ApprovalRequestStore: replayed/conflicting decisions 409, grants expire, ownership enforced +AssertionError: expected [ 'approved', 'created' ] to deeply equal [ 'created', 'approved' ] +``` + +| run | result | +|---|---| +| retained FN-9125 PostgreSQL directory, 12 workers | **failed** with the verbatim ordering assertion | +| retained FN-9129 full core, 4 workers run 1 | **failed** with the verbatim ordering assertion | +| FN-9132 deterministic one-worker frozen-Date repro, pre-fix | **failed**; both rows existed with identical `createdAt` values | +| FN-9132 targeted lifecycle, project-isolation, satellite, and dashboard-route suites | **passed** post-fix | +| FN-9132 PostgreSQL directory, 12 workers | **passed**; 173 files, 1370 tests passed, 1 skipped | + +**Resolved 2026-08-16 (FN-9132):** This was a product ordering defect in `getApprovalAuditHistory`, not PostgreSQL DDL contention, harness identity reuse, or test timing. `appendAuditEvent` creates deterministic IDs containing the event type, while the read ordered tied timestamps by `id ASC`; that lexically placed `approved` before `created`. The read now applies a lifecycle rank derived from `APPROVAL_REQUEST_AUDIT_EVENT_TYPES`, followed by ID only as a final total-order tiebreak. Regression coverage freezes `Date` around real create/decide/complete writes and proves tied approved, denied, and completed states, distinct timestamps, mixed ties, project isolation, and the public store delegate. No timeout, retry, worker-count, skip, assertion weakening, or quarantine change was made; `quarantinedCoreTests` remains empty. + +This resolves the previously unclassified “unrelated satellite-store ordering failure” mentions in entry 1's 12-worker verification table, entry 2's 12-worker verification table, and entry 11's FN-9129 4-worker run table. Those sightings are now classified separately from their entries' identity and DDL investigations. diff --git a/packages/core/src/__tests__/postgres/approval-request-audit-project-isolation.pg.test.ts b/packages/core/src/__tests__/postgres/approval-request-audit-project-isolation.pg.test.ts index 9da3a6a8c4..bd410208a1 100644 --- a/packages/core/src/__tests__/postgres/approval-request-audit-project-isolation.pg.test.ts +++ b/packages/core/src/__tests__/postgres/approval-request-audit-project-isolation.pg.test.ts @@ -178,6 +178,6 @@ pgDescribe("approval request audit project isolation", () => { { projectId: "order-project", id: "c", requestId: "apr-order", eventType: "completed", actorId: "agent", actorType: "agent", actorName: "Agent", createdAt: "2026-08-12T16:00:00.000Z" }, ]); expect((await getApprovalAuditHistory(h.layer().db, "apr-order", "order-project")).map((event) => event.id)) - .toEqual(["a", "b", "c"]); + .toEqual(["b", "a", "c"]); }); }); diff --git a/packages/core/src/__tests__/postgres/approval-request-lifecycle.pg.test.ts b/packages/core/src/__tests__/postgres/approval-request-lifecycle.pg.test.ts index fbe93b7732..71d9f65cf5 100644 --- a/packages/core/src/__tests__/postgres/approval-request-lifecycle.pg.test.ts +++ b/packages/core/src/__tests__/postgres/approval-request-lifecycle.pg.test.ts @@ -1,4 +1,5 @@ -import { it, expect, beforeAll, beforeEach, afterEach, afterAll } from "vitest"; +import { it, expect, vi, beforeAll, beforeEach, afterEach, afterAll } from "vitest"; +import { ApprovalRequestStore } from "../../agents/approval-request-store.js"; import type { AsyncDataLayer } from "../../postgres/data-layer.js"; import { pgDescribe, @@ -65,6 +66,56 @@ pgDescribe("approval request lifecycle security (PostgreSQL)", () => { return store; } + it("orders audit history by lifecycle within timestamp ties across store surfaces", async () => { + const store = await import("../../async-stores/async-approval-request-store.js"); + const publicStore = new ApprovalRequestStore(null, { asyncLayer: ctx.layer }); + const targetAction = { category: "shell", action: "exec", summary: "run cmd", resourceType: "host", resourceId: "local", context: { cmd: "ls" } }; + const assertHistory = async (id: string, expectedTypes: string[]) => { + const moduleHistory = await store.getApprovalAuditHistory(ctx.layer.db, id); + const delegatedHistory = await publicStore.getAuditHistory(id); + expect(moduleHistory.map((event) => event.eventType)).toEqual(expectedTypes); + expect(delegatedHistory.map((event) => event.eventType)).toEqual(expectedTypes); + }; + const create = (id: string) => store.createApprovalRequest(ctx.layer, { id, requester: REQUESTER, targetAction }); + + vi.useFakeTimers({ toFake: ["Date"] }); + try { + const tiedAt = new Date("2026-08-16T22:26:00.000Z"); + vi.setSystemTime(tiedAt); + await create("apr-tied-approved"); + await store.decideApprovalRequest(ctx.layer, "apr-tied-approved", "approved", { actor: DECIDER }); + const tiedApproved = await store.getApprovalAuditHistory(ctx.layer.db, "apr-tied-approved"); + expect(tiedApproved.map((event) => event.createdAt)).toEqual([tiedAt.toISOString(), tiedAt.toISOString()]); + await assertHistory("apr-tied-approved", ["created", "approved"]); + + await create("apr-tied-denied"); + await store.decideApprovalRequest(ctx.layer, "apr-tied-denied", "denied", { actor: DECIDER }); + await assertHistory("apr-tied-denied", ["created", "denied"]); + + await create("apr-tied-completed"); + await store.decideApprovalRequest(ctx.layer, "apr-tied-completed", "approved", { actor: DECIDER }); + await store.markApprovalRequestCompleted(ctx.layer, "apr-tied-completed", { actor: DECIDER }); + await assertHistory("apr-tied-completed", ["created", "approved", "completed"]); + + vi.setSystemTime(new Date("2026-08-16T22:27:00.000Z")); + await create("apr-distinct"); + vi.setSystemTime(new Date("2026-08-16T22:28:00.000Z")); + await store.decideApprovalRequest(ctx.layer, "apr-distinct", "approved", { actor: DECIDER }); + vi.setSystemTime(new Date("2026-08-16T22:29:00.000Z")); + await store.markApprovalRequestCompleted(ctx.layer, "apr-distinct", { actor: DECIDER }); + await assertHistory("apr-distinct", ["created", "approved", "completed"]); + + vi.setSystemTime(new Date("2026-08-16T22:30:00.000Z")); + await create("apr-mixed"); + await store.decideApprovalRequest(ctx.layer, "apr-mixed", "approved", { actor: DECIDER }); + vi.setSystemTime(new Date("2026-08-16T22:31:00.000Z")); + await store.markApprovalRequestCompleted(ctx.layer, "apr-mixed", { actor: DECIDER }); + await assertHistory("apr-mixed", ["created", "approved", "completed"]); + } finally { + vi.useRealTimers(); + } + }); + it("a same-verdict replay is rejected as an invalid transition", async () => { const store = await seed("apr-replay"); await store.decideApprovalRequest(ctx.layer, "apr-replay", "approved", { actor: DECIDER }); diff --git a/packages/core/src/async-stores/async-approval-request-store.ts b/packages/core/src/async-stores/async-approval-request-store.ts index 1b966c7d76..8016a07d9b 100644 --- a/packages/core/src/async-stores/async-approval-request-store.ts +++ b/packages/core/src/async-stores/async-approval-request-store.ts @@ -24,6 +24,7 @@ import { projectScopeFor, type AsyncDataLayer, type DbTransaction } from "../pos // FNXC:ApprovalLifecycleSecurity 2026-07-26-12:25: import { isApprovalRequestExpired } from "../types/agents/agents.js"; import { + APPROVAL_REQUEST_AUDIT_EVENT_TYPES, isValidApprovalRequestTransition, normalizeApprovalRequestActionCategory, type ApprovalRequest, @@ -357,7 +358,12 @@ export async function markApprovalRequestCompleted( } /** - * Get the audit history for a request, ordered by createdAt ASC. + * Get the audit history for a request ordered by timestamp, then lifecycle rank, then ID. + * + * FNXC:ApprovalAuditOrdering 2026-08-16-22:26: + * Deterministic audit IDs embed the event type. Sorting same-millisecond records by ID + * therefore put `approved` before `created`, narrating an approval before its request. + * Derive the tiebreak rank from the declared lifecycle order; never infer lifecycle from IDs. * * FNXC:ApprovalAuditProjectIsolation 2026-08-12-15:37: * Owner and superuser connections can enable `fusion.project_bypass`, so RLS cannot backstop this bare request-id lookup. Scope in SQL from the public ApprovalRequestStore layer binding; projectScopeFor intentionally treats blank and whitespace-only bindings as unbound even though fusion_assign_project_id preserves whitespace writes literally. @@ -367,12 +373,15 @@ export async function getApprovalAuditHistory( requestId: string, projectId?: string, ): Promise { + const eventType = schema.project.approvalRequestAuditEvents.eventType; + const lifecycleRank = sql`CASE ${eventType} ${sql.join( + APPROVAL_REQUEST_AUDIT_EVENT_TYPES.map((type, rank) => sql`WHEN ${type} THEN ${rank}`), + sql` `, + )} ELSE ${APPROVAL_REQUEST_AUDIT_EVENT_TYPES.length} END`; const rows = await handle .select() .from(schema.project.approvalRequestAuditEvents) .where(and(eq(schema.project.approvalRequestAuditEvents.requestId, requestId), projectScopeFor(schema.project.approvalRequestAuditEvents.projectId, projectId))) - .orderBy( - sql`${schema.project.approvalRequestAuditEvents.createdAt} ASC, ${schema.project.approvalRequestAuditEvents.id} ASC`, - ); + .orderBy(sql`${schema.project.approvalRequestAuditEvents.createdAt} ASC, ${lifecycleRank} ASC, ${schema.project.approvalRequestAuditEvents.id} ASC`); return rows.map((row) => rowToAuditEvent(row as ApprovalRequestAuditEventRow)); }