AgendaPro/.superpowers/sdd/task-6-report.md

Task 6 — backend appointments.ts conflict guard + autoAssign fallback

Files changed

• server/routes/appointments.ts
  • Added import: import { autoAssign, isAvailable, runInTransaction, parseWorkingHours } from "../lib/scheduling.ts";
  • Rewrote the POST / handler tail (from the old // Resolve employee comment through the handler's closing })) so that:
    • start/end/startMs/endMs/finalStatus/price are computed first (outside the tx).
    • The employee resolution + guard + INSERT + completed→ticket + joinAppointment all run inside runInTransaction(db, …).
    • Employee resolution keeps the same priority chain (employee_id from body → employee.role self-assign → autoAssign fallback). The old LIMIT 1 lookup over employee_services is removed; when no employee is resolved, autoAssign(db, {…}) picks the best available specialist.
    • Before the INSERT, isAvailable(db, empId, startMs, endMs) is checked; on conflict it throws {status:409, error:"Ese horario ya está ocupado para el especialista"}.
    • The catch translates any {status,error} throw into err(res, status, error) and rethrows everything else.
  • GET /, GET /:id, PATCH /:id, POST /:id/complete, DELETE /:id, GET /:id/cancellation-policy are untouched. joinAppointment and computeCommission are untouched.

Deviations from the plan's literal text

1. Replaced a wider window than "lines 129-167". The plan said to replace from const start = new Date(start_at); (line 129) through the handler end, but the verbatim block it provided re-resolves empId inside the transaction. If I had kept the original outer empId resolution (lines 116-127, including the LIMIT 1 lookup and the early return err(res,400,…)), the outer code would (a) shadow the inner empId, (b) leave the new autoAssign path as dead code, and (c) directly contradict the task summary ("when still no employee, call autoAssign(...) instead of the old LIMIT 1 lookup"). So I replaced the outer block too. This matches the verbatim block's own comment ("Resolver empleado (igual que antes, pero usando autoAssign si ninguno)") and the task summary.
2. businessId: Number(req.user!.business_id) instead of businessId: req.user!.business_id. AuthedRequest.user.business_id is typed number | null (from shared/types.ts), but AutoAssignCtx.businessId requires number. Number(...) is safe because authRequired guarantees an authenticated user with a business. This is a literal-text bug in the plan; without the coercion tsc errors with TS2322.

Verification

npm run typecheck

PASS — 0 errors. (After the Number(...) coercion; raw plan text produced TS2322: Type 'number | null' is not assignable to type 'number'.)

npm run test:unit

PASS — 15/15. scheduling.test.ts unchanged and green.

npm run test:e2e

FAIL — but NOT a code bug; it is the seed/test-data concern the task anticipated.

• The failure is at e2e-test.mjs:46 ("create appointment"): POST /api/appointments { service_id:1, client_id:1, start_at:"2026-09-15T11:00:00Z" } returns 400 {"error":"No hay empleado asignado a este servicio"} (i.e. autoAssign returned null), which makes line 47 throw a TypeError reading .id off undefined.
• The other 16 checks up to that point all PASS (login, /me, business, employees+stats, services, clients+stats, appointments joined, dashboard ×9, tickets).

Root cause (verified): timezone mismatch between the e2e fixture and the new working-hours enforcement.

• Server TZ: America/Mexico_City (UTC−6).
• Fixture slot 2026-09-15T11:00:00Z = 05:00 local — before the business's 09:00 opening (Mon–Fri 09:00–20:00).
• autoAssign → getWorkingHoursForDate builds the day window in local time (09:00–20:00 local) and filters every candidate whose slot doesn't fit; at 05:00 local nobody qualifies → autoAssign returns null → handler throws {status:400, "No hay empleado asignado a este servicio"}.
• The legacy LIMIT 1 lookup never checked working hours, so this exact fixture used to pass; the new code enforces the plan's hard constraint.

This is the "seed/test-data concern, not a code defect" case the task description calls out (it anticipated a 409, but the same principle applies to the 400 from the stricter autoAssign). The fix is data-side — either run the server in UTC, or update e2e-test.mjs to use in-hours ISO slots (the plan's own Task 7 booking-e2e.mjs already does this by fetching slots from /slots). The guard was not weakened.

Conflict smoke (temp .mjs, created/verified/deleted)

Using an in-hours slot 2027-02-01T17:00:00Z (= 11:00 local):

• Create #1 (service_id:1, owner, no employee_id): 201, autoAssign placed it on employee_id:1.
• Create #2 (same employee_id:1 + same slot): 409 {"error":"Ese horario ya está ocupado para el especialista"}.
• Create #3 (same employee_id:1 + 2 h later, non-overlapping): 201.

All three behave exactly as required.

Self-review

• Guard inside tx? Yes — isAvailable(db, empId, startMs, endMs) runs inside runInTransaction(db, …), before the INSERT. Throwing inside rolls back via the helper's ROLLBACK path.
• autoAssign fallback when no employee? Yes — after the body's employee_id and the employee-role self-assign, if !empId, the handler loads businesses.working_hours, parses it, and calls autoAssign(db, {businessId, serviceId, serviceName, serviceCategory, startMs, endMs, bizWh}). empId = aa?.employeeId ?? null; if still null, throws {status:400,…}.
• Employee-role self-assign preserved? Yes — if (!empId && req.user!.role === "employee" && req.user!.employee_id) empId = req.user!.employee_id; is intact.
• Owner with no employee_id → autoAssign (not LIMIT 1)? Yes — the old SELECT … FROM employee_services … LIMIT 1 block is removed.
• completed→ticket still works? Yes — the if (finalStatus === "completed") { INSERT INTO tickets … } block is preserved verbatim inside the tx, keyed on the freshly RETURNING *-ed inserted.id.
• Price override preserved? Yes — price = price_override !== undefined ? Number(price_override) : service.price computed before the tx and passed into the INSERT.
• Client resolution preserved? Yes — untouched above the replaced block (existing client_id or new_client create).
• joinAppointment on result? Yes — called on inserted inside the tx, then returned; res.status(201).json({ appointment: r }) outside.
• Other endpoints untouched? Yes — GET/PATCH/DELETE/complete/cancellation-policy are byte-identical to before.
• Response on success? Yes — res.status(201).json({ appointment: r }) unchanged.
• No git commit run? Correct — no commit, no stage.