ADR — PR #1033
2026-05-19 10:30 — Remove unused import ‘eq’ from impersonate.ts
Section titled “2026-05-19 10:30 — Remove unused import ‘eq’ from impersonate.ts”- Comment: https://github.com/ERP-Unlocked/ordermatic/pull/1033/files#diff-1d2b3c4e5f6a7b8c9d0e1f2a3b4c5d6e7f8g9h0i1j2k3l4m5n6o7p8q9r0s1t2u3v4w5x6y7z8
- Priority: nice-to-have
- Decision: accept
- Rationale: Unused import adds noise to the codebase and can cause linter warnings
2026-05-19 10:31 — Remove unused import ‘gte’ from impersonate-end.ts
Section titled “2026-05-19 10:31 — Remove unused import ‘gte’ from impersonate-end.ts”- Comment: https://github.com/ERP-Unlocked/ordermatic/pull/1033/files#diff-2d3b4c5e6f7a8b9c0d1e2f3a4b5c6d7e8f9g0h1i2j3k4l5m6n7o8p9q0r1s2t3u4v5w6x7y8z9
- Priority: nice-to-have
- Decision: accept
- Rationale: Unused import adds noise to the codebase and can cause linter warnings
2026-05-19 10:32 — Verify structured logging implementation
Section titled “2026-05-19 10:32 — Verify structured logging implementation”- Comment: https://github.com/ERP-Unlocked/ordermatic/pull/1033/files#diff-3d4b5c6f7a8b9c0d1e2f3a4b5c6d7e8f9g0h1i2j3k4l5m6n7o8p9q0r1s2t3u4v5w6x7y8z9
- Priority: must-fix
- Decision: accept
- Rationale: Code already uses proper structured logging with Pino logger instead of console.error
2026-05-19 10:33 — Fix transaction safety in impersonate.ts
Section titled “2026-05-19 10:33 — Fix transaction safety in impersonate.ts”- Comment: https://github.com/ERP-Unlocked/ordermatic/pull/1033/files#diff-4d5b6c7f8a9b0c1d2e3f4a5b6c7d8e9f0g1h2i3j4k5l6m7n8o9p0q1r2s3t4u5v6w7x8y9z0
- Priority: must-fix
- Decision: accept
- Rationale: Wrapped session creation and audit log insertion in database transaction to ensure atomicity
2026-05-19 10:34 — Fix transaction safety in impersonate-end.ts
Section titled “2026-05-19 10:34 — Fix transaction safety in impersonate-end.ts”- Comment: https://github.com/ERP-Unlocked/ordermatic/pull/1033/files#diff-5d6b7c8f9a0b1c2d3e4f5a6b7c8d9e0f1g2h3i4j5k6l7m8n9o0p1q2r3s4t5u6v7w8x9y0z1
- Priority: must-fix
- Decision: accept
- Rationale: Wrapped bulk session termination and audit log insertion in database transaction to ensure atomicity
2026-05-20 — Close lookup/update race in impersonate-end.ts (PR #1053 CR feedback)
Section titled “2026-05-20 — Close lookup/update race in impersonate-end.ts (PR #1053 CR feedback)”- Comment: CodeRabbit on PR #1053 — single-session path had a race: findFirst → batch update was not atomic, so two concurrent end-impersonation requests could both write
sys_admin.impersonate.endaudit rows for the same already-terminated session. - Priority: must-fix
- Decision: accept
- Rationale: Replaced the find-then-batch-update pattern with a single conditional
UPDATE ... WHERE is_active = true RETURNING id, target_user_id. The audit row is only inserted when RETURNING yields rows, so a race-loser issues no duplicate audit and gets the 404 path. Both single-session and multi-session branches now use UPDATE-with-RETURNING as the source of truth for what was actually terminated.