# Clock-In/Out & Break Tracking — Audit Findings & Remediation Plan

**Date:** 2026-07-11
**Scope:** Entire punch/break tracking system (mobile Team app, in-store kiosk/workspace, aggregation, payroll feed, real-time board)
**Status:** Audit complete (read-only). No code changed. Remediation not yet started.
**Branching rule:** All fixes land on a dedicated branch (e.g. `punch-system-audit-fixes`), **never** on `050-everee-payroll-foundations`.

---

## 1. Executive summary

Punch state is reconstructed by **ordering raw event rows on a 1-second `punchTime`, with no locking, no transactions, and no unique constraint.** Under normal real-world behavior — double-taps, retry-on-timeout, forgotten clock-outs, split/overnight shifts, manager edits — the system silently produces **duplicate, orphaned, mislabeled, or invisible punches.** Five independent auditors converged on the same root causes.

**Confirmed non-causes (do not chase these):**
- No cron/background job auto-closes or deletes open punches. The "disappeared" symptom is not a cleaner.
- Store scoping is per-store DB (`kiosk_{typeNum}`) — structurally safe from cross-store leakage.
- Mobile clock in/out times are captured as server-now UTC (not client-supplied) — immune to client clock-skew / DST injection on those endpoints.

**Data model:** two write surfaces (mobile `MobileClockService`, kiosk `Workbook/TimePunchController`) share one **event-based** per-store table `scheduleTimePunches` (rows: `clockIn`/`clockOut`/`breakStart`/`breakEnd`, `punchTime` UTC). Aggregation (`TimePunchRepository`, `TimesheetController`, `OvertimeCalculator`) pairs events into hours that feed the **Everee pay run (spec 050/050b)**.

### `scheduleTimePunches` — verified DDL (per-store DB)
Source: `migrations/input/20251220_013_002_schedule_time_punches.json` + GPS/tips/run-lock ALTERs.
- PK `punchId`; `employeeId` (int, *documented* = `kiosk_users.users.id`, **no FK**); `shiftId`; `punchType` ENUM(clockIn,clockOut,breakStart,breakEnd); `punchTime` datetime UTC; `breakType` ENUM(paid,unpaid); manual/override/approval/edit/soft-delete columns; GPS columns; `submittedToEvereeAt` / `submittedToEvereeRunId` (payroll edit-lock signals).
- **Indexes:** `PRIMARY(punchId)`, `KEY idx_employee_time(employeeId, punchTime)` *(non-unique)*, `idx_shift`, `idx_punchType`, `idx_created`, `idx_deleted`.
- **There is NO unique constraint other than the PK.** Confirmed.

---

## 2. Findings register (traceability)

Severity: 🔴 Critical · 🟠 High · 🟡 Medium · ⚪ Low. "WS" = workstream (see §3).

| ID | Sev | Finding | Key location | WS |
|----|-----|---------|--------------|----|
| F-01 | 🔴 | Duplicate/orphaned open punches — check-then-insert with no lock/txn/unique key | `MobileClockService::clockIn` ~176-228; `TimePunchController::clockInForBuyerKiosk` 1052-1136; `TimePunchRepository::create` 255-297; table DDL | WS-1 |
| F-02 | 🔴 | Mobile app has **no** break-in/break-out endpoint | `routes/mobile-scheduling.php` 274-336; no `MobileClockController` break method | WS-3 |
| F-03 | 🔴 | Kiosk native break UI shows "Not Clocked In" while on break (`break` object never set) | `TimePunchController::getPunchStateForBuyerKiosk` 885-901; `time-punch.js` 577,607 | WS-3 |
| F-04 | 🔴 | Manager "Who's Working" board mislabels clocked-in as clocked_out; no break state | `ManagerDashboardService::getWhosWorking` 282-349 | WS-4 |
| F-05 | 🔴 | Mobile clock-ins never broadcast to Ably → live board stays stale | `MobileClockService::clockIn` 164-257 (no `WorkbookAbly` publish) | WS-4 |
| F-06 | 🟠 | No `punchId` tiebreaker; same-second punch resolves non-deterministically; strict `>` drops same-second pairs | `TimePunchRepository` getActiveSession 192-224, isOnBreak 232-247, getActiveBreakStartPunch 443 | WS-2 |
| F-07 | 🟠 | `isOnBreak()` reads global latest row, not the active session → stale break blocks clock-out | `TimePunchRepository::isOnBreak` 232-247 | WS-2 |
| F-08 | 🟠 | `getActiveSession` NOT-EXISTS heuristic misreads overnight/out-of-order/manual punches → false NOT_CLOCKED_IN | `TimePunchRepository::getActiveSession` 192-224 | WS-2 |
| F-09 | 🟠 | Open/forgotten sessions silently count as **0 hours** (no flag) | `calculateWorkedHours` 468-566 (`closeOpenSessionAtRangeEnd=false`); callers 1738-1751 | WS-5 |
| F-10 | 🟠 | Two divergent hours engines → `totalHours ≠ regular+OT+DT`; payroll reconciliation fails/pays wrong | `calculateWorkedHours` vs `OvertimeCalculator::calculateForWeek` 131-247; `PayRunCalculator` ~317 | WS-5 |
| F-11 | 🟠 | OT window: local weekStart compared to UTC punchTime + unclipped ±1-day fetch → OT inflated/mis-bucketed | `OvertimeCalculator::calculateForWeek` 137-144; `buildDailySegments` 375-488 | WS-5 |
| F-12 | 🟠 | WhenIWork stores: no `schedulingProvider==='wiw'` gate on mobile clock; every read `INNER JOIN users` drops WIW-external-id punches | `StoreContextMiddleware`; `TimePunchRepository` INNER JOINs 40,93,137,... | WS-8 |
| F-13 | 🟠 | `catch (Exception)` (not `Throwable`) in all write paths → silent unlogged PHP 8.5 500s | mobile controller 178-383; `TimePunchController` 1036,1323,1530,1755,1982 | WS-6 |
| F-14 | 🟠 | Geofence hard-blocks legit indoor clock-ins (accuracy>100m / just-outside) w/ no self-override | `GeofenceService::validateLocation` 59-96; `MobileClockService::clockIn` 182-196 | WS-7 |
| F-15 | 🟠 | Override approval double-apply — no compare-and-set on pending→approved | `ManagerDashboardService::approveOverride` 1796-1860 | WS-1 |
| F-16 | 🟠 | `AblyPublishThrottle` can silently drop punch broadcasts (per-channel 40/sec, no timepunch exemption) | `Core/AblyPublishThrottle.php` 45-77; `WorkbookAbly::publish` 80-82 | WS-4 |
| F-17 | 🟠 | Post-payroll edit lock unenforced — `submittedToEvereeAt` not checked in update/softDelete | `TimePunchRepository::update` 305-336, `softDelete` 349-370 | WS-5 |
| F-18 | 🟡 | Generic swallowed 500s hide whether the punch landed → drives retries (feeds F-01) | mobile controller catches | WS-6 |
| F-19 | 🟡 | `getClockStatus` never reports break state to the app | `MobileClockService::getClockStatus` 100-149 | WS-3 |
| F-20 | 🟡 | Unscheduled clock-ins never appear on the manager board | `ManagerDashboardService::getWhosWorking` 169-188,272 | WS-4 |
| F-21 | 🟡 | Cache invalidation clears only today's key → stale board across midnight/DST/non-Chicago | `TimePunchController` 218-289 | WS-4 |
| F-22 | 🟡 | Mobile `getHours` hard-codes 40h + Monday week → disagrees with payroll OT rules | `MobileClockService::getHours` 384,394-401 | WS-5 |
| F-23 | 🟡 | PIN `"0"` treated as no-PIN (`empty()`/truthy) → auth bypass | `TimePunchController` ~820,840 | WS-8 |
| F-24 | 🟡 | Override "pending" detected via brittle JSON `LIKE` (black-hole risk) | `ScheduleAuditRepository::findPendingOverrides` 325-348 | WS-8 |
| F-25 | 🟡 | AI-metrics cron jobs query a non-existent `timePunches` table → staffing metrics always empty | `HourlyMetricsCollectorJob` 196-210; `HourlyMetricsBackfillJob` 237-247 | WS-8 |
| F-26 | 🟡 | `punchType` enum/varchar drift on legacy stores → clock-in matches nothing | DDL vs `20260522_014_punch_type_enum_verification_note.json` | WS-8 |
| F-27 | 🟡 | Rounding accumulation: day totals rounded then summed ≠ week total | `buildDayBreakdown` 1659-1663; `calculateWeekTotals` 1679-1683 | WS-5 |
| F-28 | 🟡 | DST day-splitting mis-proration on transition days | `buildDayBreakdown` 1577-1583; `buildDailySegments` 413-416 | WS-5 |
| F-29 | ⚪ | `create()` return value never verified; success returned/broadcast on failed insert | `TimePunchRepository::create` 255-297 | WS-6 |
| F-30 | ⚪ | Client JS doesn't check `response.ok`; ambiguous failure message | `time-punch.js` clock/break handlers | WS-6 |
| F-31 | ⚪ | `GeofenceService` helper queries non-existent `storeName` column | `GeofenceService` 242-248 | WS-8 |
| F-32 | ⚪ | Runtime `CREATE TABLE IF NOT EXISTS` for punch log on every punch (should be a migration) | `TimePunchController::ensurePunchLogTable` 3165-3182 | WS-8 |
| F-33 | ⚪ | `checkEmployeeAvailability()` is a dead stub; availability warning never fires | `ManagerDashboardService` 1702-1707 | WS-8 |
| F-34 | ⚪ | Publish partial notification failures swallowed & uncounted | `ManagerDashboardService` ~2497-2503 | WS-8 |

---

## 3. Workstreams

Each item lists **fix approach**, **effort** (S ≤0.5d · M 0.5–2d · L 2–5d), **needs migration?**, and **acceptance criteria**.

### WS-1 · Punch write concurrency & at-most-once integrity  🔴 (F-01, F-15)
**Fix:** Serialize all native punch writes per `(typeNum, employeeId)` — wrap every check-then-insert (mobile clockIn/out; kiosk clockIn/out/breakStart/breakEnd/override) in a transaction with `SELECT … FOR UPDATE` on the employee's latest punch, **or** a per-employee advisory lock (`GET_LOCK('punch:{typeNum}:{employeeId}')`). Add a dedupe `UNIQUE(employeeId, punchType, punchTime)` (migration) to kill exact double-submits. Make override approval a compare-and-set (`UPDATE … WHERE auditId=:id AND status='pending'`, proceed only if `rowCount()===1`). Reuse the discipline from the `at-most-once-external-payment-submit-recovery` and `repository-immutability-boundary-compare-and-set` skills.
**Effort:** L · **Migration:** Yes (unique key; consider a dedicated status column for overrides).
**Acceptance:** concurrency test firing 2 simultaneous clock-ins yields exactly 1 open punch; 2 simultaneous override approvals create exactly 1 punch; exact duplicate submit is a no-op.

### WS-2 · State-derivation correctness  🟠 (F-06, F-07, F-08)
**Fix:** Add `, punchId DESC` tiebreaker to every state query; change `punchTime > tp.punchTime` correlations to `(punchTime > tp.punchTime OR (punchTime = tp.punchTime AND punchId > tp.punchId))`. Scope `isOnBreak()` to the active session (reuse `getActiveBreakStartPunch`). Derive "open session" from a single ordered pass (latest punch is clockIn/breakStart) rather than the NOT-EXISTS heuristic; ideally link subsequent punches to a `clockInPunchId`.
**Effort:** M · **Migration:** Optional (session-link column).
**Acceptance:** same-second clock-in→clock-out resolves to clocked-out deterministically; a prior-day unclosed break no longer blocks today's clock-out; overnight open shift is correctly reported as clocked-in.

### WS-3 · Break capture end-to-end  🔴 (F-02, F-03, F-19, supports F-07)
**Fix:** Add `POST /:typeNum/clock/break/start` + `/break/end` routes, `MobileClockController` methods, and `MobileClockService::startBreak()/endBreak()` with guards (requires open clock-in; start rejects if already on break; end rejects if not on break). Add a `break` object + `isOnBreak`/`breakStartedAt` to `getPunchStateForBuyerKiosk` **and** `getClockStatus`. Coordinate the new endpoints via `docs/api/mobile-agent-requests.md` and write to `../buyerkiosk-team/docs/backend-api-updates.md`.
**Effort:** M–L · **Migration:** No.
**Acceptance:** employee can start/end a break from the phone; kiosk shows "On Break" with a running timer; status API returns break state; break-without-clock-in and double-break are rejected with clear errors. (F-03 alone is an S fix — ship first.)

### WS-4 · Real-time & manager visibility  🔴 (F-04, F-05, F-16, F-20, F-21)
**Fix:** Broadcast mobile punches through `WorkbookAbly` via a shared helper both surfaces call. Rewrite `getWhosWorking()` to derive status from the latest punch (add on-break) and **union** employees with an open punch today (unscheduled workers). Exempt `workbook:timepunch:*` from the throttle (or raise threshold / durable delivery) and log any dropped action. Invalidate today ± adjacent local-day cache keys using the known store TZ (no Chicago fallback).
**Effort:** M · **Migration:** No.
**Acceptance:** mobile clock-in appears on the workspace board in real time; split-shift/clock-back-in shows clocked-in; on-break shows on-break; unscheduled clock-in appears; punch broadcasts survive a busy store; board is fresh across midnight for non-Chicago stores.

### WS-5 · Hours & payroll correctness  🟠 (F-09, F-10, F-11, F-17, F-22, F-27, F-28) — **payroll-blocking**
**Fix:** Extract ONE shared "pair + clip + break" routine; have `calculateWorkedHours`, `buildDailySegments`, `buildDayBreakdown`, `pairPunchesToSessions` all use it. Feed OT the same **UTC** window used for net hours and clip sessions to `[weekStart,weekEnd)`. Assert `abs(totalHours − (regular+OT+DT)) < ε` at timesheet save. Surface open sessions overlapping the period as "missing clock-out — resolve before export" blockers (do NOT auto-close). Enforce `submittedToEvereeAt IS NULL` (compare-and-set) in `update()`/`softDelete()`. Route mobile `getHours` through `OvertimeCalculator` + store week-start config. Sum in seconds, round once. Compute day boundaries as UTC instants from local midnights (DST-safe).
**Effort:** L · **Migration:** No (logic); guard clauses only.
**Acceptance:** `totalHours == regular+OT+DT` on every timesheet; OT boundary matches net-hours boundary; forgotten clock-out blocks export instead of paying 0; a submitted punch cannot be edited/deleted; mobile hours match the approved timesheet. **Pull this stream forward if an Everee run is imminent.**

### WS-6 · Error handling & diagnosability  🟠 (F-13, F-18, F-29, F-30)
**Fix:** `catch (\Throwable $e)` + `error_log($e)` in every punch write/aggregation path; return a stable error `code` distinguishing "definitely failed, safe to retry" vs "unknown." `create()` asserts `execute()`/`lastInsertId()>0` and throws on failure so success/Ably are not emitted for a non-persisted punch. Client checks `response.ok` and shows "verify with a manager" on ambiguous failures.
**Effort:** M · **Migration:** No. **Do early** — makes every other fix observable.
**Acceptance:** an injected TypeError in a write path is logged and returns a clean error; a simulated failed insert never returns success or broadcasts; app shows meaningful messages.

### WS-7 · Geofence UX  🟠 (F-14)
**Fix:** Make accuracy advisory (widen radius by accuracy) instead of a hard reject; on outside-geofence, allow clock-in flagged for review OR surface the employee override-request path prominently in the 403. Document the unconfigured-store fail-open decision explicitly.
**Effort:** S–M · **Migration:** No.
**Acceptance:** an indoor low-accuracy fix no longer hard-blocks; employee has a self-service path; behavior is documented.

### WS-8 · Edge cases & hardening  🟡⚪ (F-12, F-23, F-24, F-25, F-26, F-31, F-32, F-33, F-34)
**Fix (each small):** Gate mobile clock on `schedulingProvider==='wiw'` and switch reads to `LEFT JOIN` + COALESCE(users→uf_user→employees) so WIW-id punches aren't dropped (F-12). Treat PIN presence as `!== null && !== ''` (F-23). Move override status to a real indexed column (F-24). Repoint or disable the AI-metrics cron queries (F-25). Run the enum-verification migration and remediate straggler stores (F-26). Fix `GeofenceService` columns (F-31). Move punch-log DDL to a migration (F-32). Implement or flag the availability stub (F-33). Surface publish partial-notify failures (F-34).
**Effort:** S each · **Migration:** F-24, F-26, F-32 yes.
**Acceptance:** per-item.

---

## 4. Sequencing & dependencies

```
Phase 0 (stop the bleeding + see clearly)   WS-6  →  WS-1
Phase 1 (correctness)                        WS-2  +  WS-3      (parallel)
Phase 2 (visibility + money)                 WS-4  +  WS-5      (WS-5 payroll-blocking; pull forward if Everee run near)
Phase 3 (UX + hardening)                     WS-7  +  WS-8
Phase 4 (verify)                             Live Chrome E2E + punch-sequence regression suite
```

- **WS-6 precedes everything** — without `Throwable`/logging, later fixes are un-debuggable.
- **WS-1 before WS-2/WS-5** — locking removes the duplicate rows that corrupt state derivation and hours.
- **WS-2 before WS-5** — hours math must sit on correct state.
- **WS-3's F-03** (kiosk break shows "Not Clocked In") is the single highest-payoff-per-line fix — can ship in Phase 0 as a hotfix.
- **WS-5 (F-10/F-11/F-17)** is a hard gate for the Everee pay run; if payroll is imminent, promote WS-5 to Phase 1.

## 5. Verification (Phase 4)
- **Live Chrome E2E on dev2.buyerkiosk.com** for the browser-observable set: mobile clock-in → board updates; kiosk break → "On Break" + timer; geofence low-accuracy no longer blocks; rapid double-tap → single punch.
- **Integration regression suite** for the punch state machine: every invalid transition (double clock-in, break-without-clock-in, double break, break-end-without-start, clock-out-on-break, out-of-order, overnight, same-second) asserts correct guard + correct hours.
- Assert `totalHours == regular+OT+DT` as a persisted invariant test.

## 6. Notes
- Every finding was verified against **current** code by five independent read-only auditors on 2026-07-11 (line numbers fresh, not from memory).
- Relevant skills to apply during fixes: `at-most-once-external-payment-submit-recovery`, `repository-immutability-boundary-compare-and-set`, `utc-day-offset-dst-drift`, `buyerkiosk-wiw-exclusion-source-of-truth`, `taskengine-*` (for any recovery jobs).
- Migrations must go through the JSON migration system (`migrations/input/`), never manual DDL.

---

## 7. Cross-check addendum — post master sync (2026-07-11, later same day)

After the audit, `master` was fast-forwarded 24 commits (`7114de77a..968e047a2`) and `punch-system-audit-fixes` was rebased onto it. Four investigators re-verified every finding against the new code. **Line numbers in §2 are stale for the changed files; use the anchors below.**

**Score: 28 stand · 4 partially resolved · 2 resolved.** One NEW finding added (F-35).

### 7.1 Resolved by the pull — remove from remediation scope
| ID | Resolution |
|----|------------|
| F-09 | `detectStrandedClockIns`/`findStrandedClockInsInPunches` (TimesheetController 2874/2922) now hard-block approve (1022-1031), batch-approve (1098-1112), and export (409 `STRANDED_CLOCK_IN`, 1303-1327). No auto-close (as the audit advised). WS-5's "forgotten clock-out blocks export" acceptance criterion is already met. |
| F-28 | DST proration rewritten: day boundaries computed as UTC instants from local midnights, prorated by elapsed seconds (`buildDayBreakdown` 2217-2249; `OvertimeCalculator::buildDailySegments` 422-458) — the exact WS-5 fix. |

### 7.2 Partially resolved — scope shrinks, core remains
| ID | What changed | What remains |
|----|-------------|--------------|
| F-04 | Stale-punch filter added (`getWhosWorking` 341-343) mitigates the mislabel vector. | Break state still absent: `clockData` maps only clockIn/clockOut (350-354); `breakStart/breakEnd` rows fetched then dropped. No `on_break` status. |
| F-08 | `punchTime <= UTC_TIMESTAMP()` guards added on both sides of `getActiveSession` (197-235) — kills the future-dated-punch false NOT_CLOCKED_IN (commit 93bdb037b). | NOT-EXISTS heuristic itself unchanged; out-of-order/manual misreads remain. No session-link column. |
| F-11 | Unclipped double-count fixed: `array_intersect_key($dailySegments, $weekDayKeys)` clips to the 7 local days (`calculateForWeek` 146-158). | Local `$weekStart` still compared against UTC `punchTime`; `findByEmployeeAndDateRange` (TimePunchRepository 80-116) binds local wall-clock digits against UTC column — masked by the ±1-day pad, not fixed. |
| F-14 | Migration `20260625_001` adds `mobileClockInEnabled` (default 1) + `geofenceEnabled` (default 0) — enforcement now opt-in (`MobileClockService::clockIn` ~200). | `GeofenceService` itself unchanged (accuracy>100m hard-fail, no self-override), AND the migration backfills `geofenceEnabled=1` for every store with coordinates — configured stores keep the old hard-block. |

### 7.3 Notable STANDS corrections & fresh anchors
- **F-13**: `startBreak` (TimePunchController 1566) is already `\Throwable`; the other four write paths still catch `Exception` — clockIn 1072, clockOut 1359, endBreak 1806, overrideClockAction 2033.
- **F-17**: stronger than audited — `submittedToEvereeAt`/`submittedToEvereeRunId` have **zero references in `src/`**; the columns come only from the untracked spec-050b migration `20260603_006_schedule_time_punches_run_lock.json`. Enforcement must be coordinated with the payroll branch.
- **F-21**: TZ half addressed (store TZ resolved, Chicago only as error fallback, `invalidateSchedulePanelCache` 281-300); still deletes only today's single key.
- **F-27**: day-rounded-then-summed persists for display totals, but pay fields (regular/OT/DT) now come from the `Timesheet` object — money path sidesteps the accumulation.
- **Stale §2 location refs**: `calculateWorkedHours` lives in `TimePunchRepository` (516-624), not TimesheetController; `ScheduleAuditRepository` is at `MobileScheduling/Repositories/` (F-24 anchor `findPendingOverrides` 325-348, unchanged).
- **Unchanged files confirmed byte-identical** (findings carry as written): `GeofenceService` (F-14 internals, F-31), `AblyPublishThrottle`/`WorkbookAbly` (F-16), `ScheduleAuditRepository` (F-24), `time-punch.js` (F-30 — clock/break handlers at 732/791/841/889 still skip `response.ok`; only break-hint at 1010 checks), `HourlyMetricsCollectorJob`/`BackfillJob` (F-25), `MobileClockService::getHours` (F-22 — Monday + 40h hard-codes at 404/449-451; new `WeekCadence` helper wired only into `MobileRequestController`).

### 7.4 NEW finding from pulled code
| ID | Sev | Finding | Key location | WS |
|----|-----|---------|--------------|----|
| F-35 | 🟠 | **Mobile manager punch editing** (commits 5f3368e7a/7e081e6c7, route `manager/punches/:punchId/update`) is new write surface with no per-punch CAS/advisory lock (concurrent edits = last-write-wins via plain `TimePunchRepository::update` 321-364) and **no `submittedToEvereeAt` check** — a punch already sent to payroll is editable as long as its timesheet isn't approved/exported (`assertTimesheetWeekNotLocked` 2200-2203 is the only gate). Positives to keep: store-DB transaction, audit `logEdit`, old+new week recalc, `assertPunchTimeNotFuture` (2213-2224). | `ManagerDashboardService::updatePunch` 1320-1428 | WS-1 + WS-5 |

### 7.5 F-17 deferral (decided during Phase 2)
`submittedToEvereeAt`/`submittedToEvereeRunId` exist only in the unmerged spec-050b
payroll branch migration (`20260603_006_schedule_time_punches_run_lock.json`).
Enforcing the edit-lock against columns that don't exist on this branch is
untestable — F-17 lands WITH the payroll branch, where `TimePunchRepository::update()`
/ `softDelete()` gain the `AND submittedToEvereeAt IS NULL` compare-and-set.

### 7.5 Sequencing impact
- **WS-5 shrinks**: F-09 and F-28 done; F-11 is now only the TZ-window fix; F-17 must be built together with wiring the spec-050b run-lock migration. F-10 (engine divergence + ε-assert) unchanged.
- **WS-1 grows**: add F-35's `updatePunch` path to the same per-employee serialization + CAS treatment as clockIn/override-approval (F-15 confirmed still no CAS — read-check-then-write at `approveOverride` 1980-2044).
- Everything else in §4 sequencing stands.