Add conflict detection and resolution for meeting postponements

Implement optimistic locking for meeting postponements to prevent concurrent edits. The server now requires an `updatedAt` token for postpone requests, returning a 409 conflict error if the meeting has been modified since the token was issued. The client displays a user-friendly prompt allowing users to reapply changes with the latest token.

Replit-Commit-Author: Agent
Replit-Commit-Session-Id: 77cfe984-2c65-4152-bb7a-0df28274fe66
Replit-Commit-Checkpoint-Type: full_checkpoint
Replit-Commit-Event-Id: 1abdc3d2-c834-4a37-b104-397852e4732a
Replit-Commit-Screenshot-Url: https://storage.googleapis.com/screenshot-production-us-central1/c3c252e4-c83d-40ca-9fff-99a3ea60701e/77cfe984-2c65-4152-bb7a-0df28274fe66/8gwn7Xm
Replit-Helium-Checkpoint-Created: true

artifacts/api-server/src/routes/executive-meetings.ts

@@ -794,6 +794,15 @@ const alertActionSchema = z.object({
 });
 const postponeMinutesSchema = z.object({
   minutes: z.number().int().min(1).max(24 * 60),
+  // #283: optimistic-locking token. The client must send the
+  // `updatedAt` it last observed for this meeting; if another user
+  // (or the same user from another tab) mutated the meeting in
+  // between, we reject with HTTP 409 + stale_meeting so the UI can
+  // surface "already postponed by X" instead of double-shifting.
+  // Required by design — there is no opt-out path. To "apply anyway"
+  // after a conflict, the client refetches (or reuses
+  // conflict.lastModifiedAt) and resubmits with the fresher token.
+  expectedUpdatedAt: z.string().datetime(),
 });
 const rescheduleSchema = z.object({
   meetingDate: dateSchema,
@@ -997,6 +1006,24 @@ router.post(
     // #273: lock-then-mutate inside a single transaction so concurrent
     // postpone/reschedule/cancel requests serialize and audit logs reflect
     // the actual committed transition.
+    type StaleConflict = {
+      ok: false;
+      status: 409;
+      error: string;
+      code: "stale_meeting";
+      conflict: {
+        currentStartTime: string | null;
+        currentEndTime: string | null;
+        currentStatus: string;
+        lastModifiedAt: string;
+        lastActor: {
+          id: number | null;
+          username: string | null;
+          displayNameAr: string | null;
+          displayNameEn: string | null;
+        } | null;
+      };
+    };
     type TxResult =
       | {
           ok: true;
@@ -1005,7 +1032,8 @@ router.post(
           newEnd: string;
           conflicts: boolean;
         }
-      | { ok: false; status: number; error: string; code: string };
+      | { ok: false; status: number; error: string; code: string }
+      | StaleConflict;
     const txResult = await db.transaction(async (tx): Promise<TxResult> => {
       const [locked] = await tx
         .select()
@@ -1015,6 +1043,48 @@ router.post(
       if (!locked) {
         return { ok: false, status: 404, error: "Meeting not found", code: "not_found" };
       }
+      // #283: Optimistic-locking guard. The client always sends the
+      // `updatedAt` it last observed; if the row has been modified
+      // since, refuse to mutate and return the current state + the
+      // last actor's display info so the UI can show
+      // "Already postponed by Ahmed at 10:05 — apply N more minutes?"
+      // We do this *inside* the FOR UPDATE lock so concurrent
+      // postpone-minutes calls are serialized: the second one sees the
+      // first one's committed update and returns 409. There is no
+      // opt-out — to apply after a conflict, the client must resubmit
+      // with the freshly-seen `lastModifiedAt`, so a third concurrent
+      // mutation between conflict-display and apply-anyway will trigger
+      // another 409 instead of silently stacking.
+      const observedIso = new Date(body.expectedUpdatedAt).toISOString();
+      const currentIso = locked.updatedAt.toISOString();
+      if (observedIso !== currentIso) {
+        let lastActor: StaleConflict["conflict"]["lastActor"] = null;
+        if (locked.updatedBy != null) {
+          const [u] = await tx
+            .select({
+              id: usersTable.id,
+              username: usersTable.username,
+              displayNameAr: usersTable.displayNameAr,
+              displayNameEn: usersTable.displayNameEn,
+            })
+            .from(usersTable)
+            .where(eq(usersTable.id, locked.updatedBy));
+          lastActor = u ?? null;
+        }
+        return {
+          ok: false,
+          status: 409,
+          error: "Meeting was modified by someone else",
+          code: "stale_meeting",
+          conflict: {
+            currentStartTime: locked.startTime,
+            currentEndTime: locked.endTime,
+            currentStatus: locked.status,
+            lastModifiedAt: currentIso,
+            lastActor,
+          },
+        };
+      }
       const startMin = parseTimeToMinutes(locked.startTime);
       const endMin = parseTimeToMinutes(locked.endTime);
       if (startMin == null || endMin == null) {
@@ -1043,7 +1113,14 @@ router.post(
       const newEnd = formatMinutesAsTime(endMin + body.minutes);
       await tx
         .update(executiveMeetingsTable)
-        .set({ startTime: newStart, endTime: newEnd, status: "postponed" })
+        .set({
+          startTime: newStart,
+          endTime: newEnd,
+          status: "postponed",
+          // #283: stamp the actor so the next stale_meeting reply can
+          // attribute "postponed by X" to a real user.
+          updatedBy: userId,
+        })
         .where(eq(executiveMeetingsTable.id, id));
       await logAudit(tx, {
         action: "meeting_postponed_minutes",
@@ -1080,7 +1157,18 @@ router.post(
       };
     });
     if (!txResult.ok) {
-      res.status(txResult.status).json({ error: txResult.error, code: txResult.code });
+      // #283: Stale-conflict responses carry an extra `conflict` payload
+      // (current state + last actor display info) so the UI can render
+      // a "tried to postpone but X already postponed it to 10:05" prompt
+      // instead of the generic toast.
+      const payload: Record<string, unknown> = {
+        error: txResult.error,
+        code: txResult.code,
+      };
+      if (txResult.code === "stale_meeting" && "conflict" in txResult) {
+        payload.conflict = txResult.conflict;
+      }
+      res.status(txResult.status).json(payload);
       return;
     }
     void emitExecutiveMeetingsDaysChanged([txResult.meetingDate]);

artifacts/api-server/tests/executive-meetings-postpone-race.test.mjs

@@ -0,0 +1,295 @@
+// #283: Concurrent-postpone race test.
+// When two users postpone the same meeting at roughly the same moment,
+// the second writer must NOT silently stack their N minutes on top of
+// the first writer's already-shifted time. Instead, the server should
+// return 409 stale_meeting with a `conflict` payload identifying who
+// won the race so the UI can surface a confirmation prompt.
+//
+// We exercise three scenarios:
+//   1. A → 200 (first wins).
+//   2. B with the *original* updatedAt → 409 stale_meeting + conflict
+//      payload that names A as the lastActor.
+//   3. B with the *fresh* updatedAt (re-fetched after losing the race)
+//      → 200; the meeting now reflects A's + B's shifts in order, and
+//      we can also confirm the "force apply anyway" path by sending
+//      no expectedUpdatedAt at all.
+import { test, before, after } from "node:test";
+import assert from "node:assert/strict";
+import pg from "pg";
+
+const API_BASE = process.env.TEST_API_BASE ?? "http://localhost:8080";
+const DATABASE_URL = process.env.DATABASE_URL;
+if (!DATABASE_URL) {
+  throw new Error("DATABASE_URL must be set to run these tests");
+}
+
+const TEST_PASSWORD_HASH =
+  "$2b$10$Bs636ukPMyz01nKrsi.5m.JlDXSN22AVCvn8cgPWWDbo5yJRQX2vu";
+const TEST_PASSWORD = "TestPass123!";
+
+const pool = new pg.Pool({ connectionString: DATABASE_URL });
+
+const created = { userIds: [], meetingIds: [] };
+
+function uniqueName(prefix) {
+  return `${prefix}_${Date.now().toString(36)}_${Math.random()
+    .toString(36)
+    .slice(2, 8)}`;
+}
+
+async function createUser(prefix, roleName, displayNameEn) {
+  const username = uniqueName(prefix);
+  const { rows } = await pool.query(
+    `INSERT INTO users (username, email, password_hash, display_name_en, preferred_language, is_active)
+     VALUES ($1, $2, $3, $4, 'en', true) RETURNING id`,
+    [username, `${username}@example.com`, TEST_PASSWORD_HASH, displayNameEn],
+  );
+  const id = rows[0].id;
+  created.userIds.push(id);
+  for (const r of ["user", roleName]) {
+    await pool.query(
+      `INSERT INTO user_roles (user_id, role_id)
+       SELECT $1, id FROM roles WHERE name = $2
+       ON CONFLICT DO NOTHING`,
+      [id, r],
+    );
+  }
+  return { id, username };
+}
+
+function extractCookie(res) {
+  const setCookie = res.headers.get("set-cookie");
+  if (!setCookie) return null;
+  return (
+    setCookie
+      .split(",")
+      .map((c) => c.split(";")[0].trim())
+      .find((c) => c.startsWith("connect.sid=")) ?? null
+  );
+}
+
+async function login(username, password) {
+  const res = await fetch(`${API_BASE}/api/auth/login`, {
+    method: "POST",
+    headers: { "Content-Type": "application/json" },
+    body: JSON.stringify({ username, password }),
+  });
+  assert.equal(res.status, 200, `login should succeed for ${username}`);
+  const cookie = extractCookie(res);
+  assert.ok(cookie, "login response should set a session cookie");
+  return cookie;
+}
+
+async function api(cookie, method, path, body) {
+  const init = {
+    method,
+    headers: {
+      Cookie: cookie,
+      ...(body !== undefined ? { "Content-Type": "application/json" } : {}),
+    },
+  };
+  if (body !== undefined) init.body = JSON.stringify(body);
+  return fetch(`${API_BASE}${path}`, init);
+}
+
+let userA = null;
+let cookieA = null;
+let userB = null;
+let cookieB = null;
+
+before(async () => {
+  // executive_coord_lead carries the mutate role required by the
+  // postpone/reschedule endpoints — see MUTATE_ROLES in the route.
+  userA = await createUser("em_race_a", "executive_coord_lead", "Alice");
+  cookieA = await login(userA.username, TEST_PASSWORD);
+  userB = await createUser("em_race_b", "executive_coord_lead", "Bob");
+  cookieB = await login(userB.username, TEST_PASSWORD);
+});
+
+after(async () => {
+  if (created.meetingIds.length > 0) {
+    await pool.query(
+      `DELETE FROM executive_meeting_attendees WHERE meeting_id = ANY($1::int[])`,
+      [created.meetingIds],
+    );
+    await pool.query(
+      `DELETE FROM executive_meeting_audit_logs WHERE entity_id = ANY($1::int[]) AND entity_type = 'meeting'`,
+      [created.meetingIds],
+    );
+    await pool.query(
+      `DELETE FROM executive_meetings WHERE id = ANY($1::int[])`,
+      [created.meetingIds],
+    );
+  }
+  if (created.userIds.length > 0) {
+    await pool.query(
+      `DELETE FROM executive_meeting_notification_prefs WHERE user_id = ANY($1::int[])`,
+      [created.userIds],
+    );
+    await pool.query(`DELETE FROM user_roles WHERE user_id = ANY($1::int[])`, [
+      created.userIds,
+    ]);
+    await pool.query(`DELETE FROM users WHERE id = ANY($1::int[])`, [
+      created.userIds,
+    ]);
+  }
+  await pool.end();
+});
+
+const today = new Date().toISOString().slice(0, 10);
+
+async function createMeeting() {
+  // Create through the API so updatedAt and updatedBy are set the same
+  // way real meetings are. Use a fixed afternoon window so we don't
+  // race the alert loop or the noTimeWindow guard.
+  const res = await api(cookieA, "POST", "/api/executive-meetings", {
+    titleAr: "اجتماع التزامن",
+    titleEn: "Concurrency meeting",
+    meetingDate: today,
+    startTime: "14:00",
+    endTime: "14:30",
+    status: "scheduled",
+  });
+  assert.ok(
+    res.status === 200 || res.status === 201,
+    `meeting create should succeed (got ${res.status})`,
+  );
+  const body = await res.json();
+  const meeting = body.meeting ?? body;
+  created.meetingIds.push(meeting.id);
+  return meeting;
+}
+
+async function fetchMeeting(cookie, id) {
+  // /api/executive-meetings/days serves the same shape the alert UI
+  // sees, but the simpler GET-by-id endpoint is enough here.
+  const res = await api(cookie, "GET", `/api/executive-meetings/${id}`);
+  assert.equal(res.status, 200, "meeting fetch should succeed");
+  return res.json();
+}
+
+test("postpone-minutes: second concurrent writer hits 409 stale_meeting", async () => {
+  const meeting = await createMeeting();
+  // Both clients capture the same updatedAt — this is the race window.
+  const initial = await fetchMeeting(cookieA, meeting.id);
+  const staleUpdatedAt = initial.updatedAt;
+  assert.ok(staleUpdatedAt, "meeting payload must include updatedAt");
+
+  // A wins.
+  const aRes = await api(
+    cookieA,
+    "POST",
+    `/api/executive-meetings/${meeting.id}/postpone-minutes`,
+    { minutes: 5, expectedUpdatedAt: staleUpdatedAt },
+  );
+  assert.equal(aRes.status, 200, "first postpone should succeed");
+  const aBody = await aRes.json();
+  assert.equal(aBody.meeting.startTime.slice(0, 5), "14:05");
+
+  // B loses — sends the now-stale token.
+  const bRes = await api(
+    cookieB,
+    "POST",
+    `/api/executive-meetings/${meeting.id}/postpone-minutes`,
+    { minutes: 5, expectedUpdatedAt: staleUpdatedAt },
+  );
+  assert.equal(bRes.status, 409, "second postpone should be rejected as stale");
+  const bBody = await bRes.json();
+  assert.equal(bBody.code, "stale_meeting");
+  assert.ok(bBody.conflict, "stale response must include conflict payload");
+  assert.equal(bBody.conflict.currentStartTime?.slice(0, 5), "14:05");
+  assert.ok(bBody.conflict.lastActor, "conflict must name the last actor");
+  assert.equal(bBody.conflict.lastActor.username, userA.username);
+  assert.equal(bBody.conflict.lastActor.displayNameEn, "Alice");
+
+  // Confirm the start time did NOT silently shift again.
+  const afterB = await fetchMeeting(cookieA, meeting.id);
+  assert.equal(
+    afterB.startTime.slice(0, 5),
+    "14:05",
+    "B's stale call must not have stacked another shift",
+  );
+});
+
+test("postpone-minutes: 'apply anyway' uses freshly-seen token from conflict", async () => {
+  const meeting = await createMeeting();
+
+  // A wins.
+  const aRes = await api(
+    cookieA,
+    "POST",
+    `/api/executive-meetings/${meeting.id}/postpone-minutes`,
+    { minutes: 10, expectedUpdatedAt: meeting.updatedAt },
+  );
+  assert.equal(aRes.status, 200);
+  const aBody = await aRes.json();
+  assert.equal(aBody.meeting.startTime.slice(0, 5), "14:10");
+
+  // B sees a 409 with conflict.lastModifiedAt naming the new state…
+  const conflictRes = await api(
+    cookieB,
+    "POST",
+    `/api/executive-meetings/${meeting.id}/postpone-minutes`,
+    { minutes: 5, expectedUpdatedAt: meeting.updatedAt },
+  );
+  assert.equal(conflictRes.status, 409);
+  const conflictBody = await conflictRes.json();
+  const freshToken = conflictBody.conflict.lastModifiedAt;
+  assert.ok(freshToken, "conflict must include lastModifiedAt for retry");
+
+  // …and "apply anyway" mirrors the client by resubmitting with that
+  // freshly-seen token. A third concurrent mutation between conflict
+  // display and click would return 409 again — proving no silent stack.
+  const retryRes = await api(
+    cookieB,
+    "POST",
+    `/api/executive-meetings/${meeting.id}/postpone-minutes`,
+    { minutes: 5, expectedUpdatedAt: freshToken },
+  );
+  assert.equal(retryRes.status, 200, "apply-anyway with fresh token succeeds");
+  const retryBody = await retryRes.json();
+  assert.equal(
+    retryBody.meeting.startTime.slice(0, 5),
+    "14:15",
+    "apply-anyway shifts on top of A's already-shifted state (10 + 5)",
+  );
+});
+
+test("postpone-minutes: missing expectedUpdatedAt is a 400 (required by design)", async () => {
+  const meeting = await createMeeting();
+  const res = await api(
+    cookieA,
+    "POST",
+    `/api/executive-meetings/${meeting.id}/postpone-minutes`,
+    { minutes: 5 },
+  );
+  assert.equal(
+    res.status,
+    400,
+    "the optimistic-locking token is mandatory — no opt-out path",
+  );
+});
+
+test("postpone-minutes: B refetches and retries cleanly with fresh updatedAt", async () => {
+  const meeting = await createMeeting();
+
+  // A shifts by 5.
+  await api(
+    cookieA,
+    "POST",
+    `/api/executive-meetings/${meeting.id}/postpone-minutes`,
+    { minutes: 5, expectedUpdatedAt: meeting.updatedAt },
+  );
+
+  // B refetches → gets the new updatedAt → retries → 200.
+  const refreshed = await fetchMeeting(cookieB, meeting.id);
+  const retryRes = await api(
+    cookieB,
+    "POST",
+    `/api/executive-meetings/${meeting.id}/postpone-minutes`,
+    { minutes: 5, expectedUpdatedAt: refreshed.updatedAt },
+  );
+  assert.equal(retryRes.status, 200);
+  const retryBody = await retryRes.json();
+  assert.equal(retryBody.meeting.startTime.slice(0, 5), "14:10");
+});