feat(payroll): Gusto rates import, hours export, period lock, crew bug reports (pipeline v2 phase 5)

[Clarion1631]

Phase 5 of the receipt-pipeline rebuild: Gusto rates in, hours out, crew bug button

Plan: docs/plans/RECEIPT-PIPELINE-V2-PLAN.md (decision 6), spec: docs/plans/PHASE-5-GUSTO-AND-MOBILE-RELEASE-SPEC.md. Pairs with gtr-probuild-mobile PR #3 (app 1.1.2).

Why

Pay rates live in Gusto; ProBuild costs labor from User.hourlyRate and a $0 rate silently costs labor at zero. There was no way to hand approved hours to Gusto, and the old /api/gusto/export route had no role check at all. Crew could not file bugs because the help-chat routes were ADMIN-only.

What

• Payroll rates panel (Company → Team Members): hourly + burden, User.lastRateSyncAt, Gusto employee-CSV import with a preview diff (matched member's email shown; name-only matches badged; commas refused).
• $0-rate guard: clock-out returns 422 ZERO_RATE_BLOCKED and leaves the punch open; block-by-default with ADMIN/FINANCE and salaried emails exempt; manager PATCH mirror; red badge on the manager time page.
• Hours export GET /api/time-entries/export/gusto (ADMIN or financialReports): regular/OT per employee per period reusing overtime.ts (per workweek, never re-derived) and never re-deducting meals; 409 while any in-range entry is open, needsReview, zero-duration, or an unsettled DEFERRED day; summary + detail CSVs with formula-lead neutralization; deterministic exportHash over both CSVs. Old ungated route deleted.
• Payroll period lock: PayrollPeriod table; lock recomputes blockers + hash inside one transaction; 423 PERIOD_LOCKED enforced in every TimeEntry writer (API routes, server actions, timeclock actions, core create).
• Help-chat bug routes accept any active staff via authenticateMobileOrSession, exact proxy allowlist entries.
• src/lib/payroll-config.ts: PAYROLL_PERIOD, PAYROLL_WEEK_START, PAYROLL_SALARIED_EMAILS (defaults pending Justin).

Review

Independent checker PASS. Codex unavailable at review time (OpenAI capacity); an Opus deep review found 3 blockers (lock bypass via four other writers, salaried MANAGER stranded by the rate guard, unsettled DEFERRED day exporting at full pay) and 5 majors (check-then-act lock, CSV injection, dropped zero-hour entries, hash determinism, rate-import evidence); all fixed. Codex review still to run via the CI gate.

Tests

npm run test:unit 623/623, npm run build clean.

Deploy order

1. node scripts/apply-payroll-phase5.mjs against prod (additive, idempotent).
2. Merge. Confirm PAYROLL_* defaults with Justin before the first real export.

🤖 Generated with Claude Code

Prod state — the DDL is ALREADY APPLIED (2026-09-02, accidentally)

The deploy step for this PR is verification-only. Do not run a real apply.

On 2026-09-02 I ran node -e "import('./scripts/apply-payroll-phase5.mjs')" to inspect the module's exports. The script called config({ path: ".env.production.local" }) at module scope, so the import loaded production credentials and executed the entire migration as a side effect. Confirmed afterwards by a read-only query, and independently re-verified by the coordinator:

• TimeEntry_userId_fkey and TimeEntry_projectId_fkey → confdeltype = 'r' (RESTRICT, live)
• User.payType, User.lastRateSyncAt, PayrollPeriod + discardedAt / discardedById / discardedReason + the PayrollPeriod_discard_unlocked CHECK: all present
• payType set for 0 users — the seed wrote nothing. PAYROLL_SALARIED_EMAILS is unset in prod, and the hardcoded CJ/Richard default had already been removed, so no pay type was guessed for anyone.

Live consequence until this merges. The FK change is on production while the deployed build still deletes users and projects without clearing their time entries first. So right now, on prod, deleting a user or project that has any time entry fails with a foreign-key error instead of succeeding. That is the intended end state — the old CASCADE silently destroyed paid payroll history — but it is live ahead of the code that handles it. Decision (Justin notified): leave it; failing loudly beats cascading paid hours. A revert to CASCADE is available on request.

Deploy step for this PR:

node scripts/apply-payroll-phase5.mjs --dry-run

Expected output against prod's current state: [dry-run] nothing to do — all N objects this script manages are already present. If it reports anything missing, stop and read before running a real apply.

--dry-run is now genuinely read-only for the whole script. It previously gated only the payType seed and executed every DDL statement regardless, which would have made it a lie in exactly the situation it is now being used for. It returns before the statement loop, and a CI test drops an object, dry-runs, and asserts it is still missing.

Root cause fixed on this branch. Every side effect in the script now lives in main() behind an isMainModule guard, so importing the module does nothing. Tests assert module scope contains no config({ path:, no new PrismaClient(, and no $executeRawUnsafe. Guarding the other apply scripts the same way is spun off as a separate task and deliberately not touched here.

Deploy note — pay types are deliberately NOT guessed

scripts/apply-payroll-phase5.mjs seeds User.payType = SALARY only for the emails in PAYROLL_SALARIED_EMAILS (default: CJ + Richard). Everyone else stays NULL on purpose. The payroll export returns 409 unknownPayType for anyone with hours in the period until a human sets their pay type on Company -> Team Members.

This is fail-closed by design: a stored value beats the env fallback, so blanket-stamping people HOURLY would have permanently mislabelled any salaried person missing from that list (Gusto pays them a salary and the exported hours), and fixing the env var afterwards would have changed nothing.

Expect the first export after deploy to refuse until the roster is filled in.

Tests

CI run for 986441de: https://github.com/Clarion1631/probuild/actions/runs/33646250470 — all three jobs green.

Job	Result
Migrations reproduce production	success
Playwright E2E Tests	success
Build + Bundle Size	success

Playwright: 508 passed (5.1m), 0 failed. The job runs bare npx playwright test, so the whole e2e/ directory is in scope, money-pipeline.spec.ts (42 tests) included. Worth being precise about the evidence: the CI reporter is github, which annotates failures only and emits no per-spec line for passes — so the pass line above is the aggregate, and the money-pipeline guarantee rests on "it is in the run and the run had zero failures", not on a spec-scoped line I can quote.

Two-connection concurrency proof (new). Most concurrency tests on this branch drive one branch through an injected fake, which shows the branch exists but not that PostgreSQL serializes the way the code assumes. tests/payroll-rate-lock-db.test.ts opens two real connections against the migrations job's throwaway Postgres and makes them contend. From this run:

# Subtest: FOR SHARE on the owner row BLOCKS a concurrent rate write until settlement commits
# Subtest: two settlements for DIFFERENT days do not block each other
# tests 2
# pass 2
# fail 0
# skipped 0

It is opt-in by PAYROLL_LOCK_TEST_URL and skips in a normal unit run, so it can never touch a developer database.

Unit: 973 pass, 0 fail, 2 skipped (the DB tests above, which run in CI).

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated
probuild	Ready	Preview	Sep 2, 2026 10:10pm UTC

[github-actions]

Required changes:

1. Period locking is still racy. actions.ts locks only existing PayrollPeriod rows, then inserts the lock and recomputes. Concurrent writers cannot see the uncommitted lock and can update TimeEntry after the final recompute but before/after commit. Likewise, FOR UPDATE over an empty result does not prevent two concurrent overlapping-period inserts. Coordinate lock creation and every hours writer with the same transactional advisory lock/database enforcement, and add real concurrent-database tests.

2. Manual time entries permanently block export. gusto-export-core.ts classifies every endTime === null row as open, but both time-expense-core.ts and timeclock/actions.ts create completed, positive-duration entries without endTime. Those rows produce a perpetual 409 and are excluded from totals. Establish an explicit completed/manual representation and backfill existing rows.

3. The import preview cannot reliably save a subset. RatesImport.tsx selects only changed email matches, while previewFingerprint() fingerprints every matched row. The apply action fingerprints only submitted rows, so a normal CSV containing unchanged or unselected name-only matches fails as “changed since preview.” Bind evidence to each selected row or rederive the selected subset from the original CSV server-side.

4. Duplicate CSV rows still write the first value. rate-import.ts invalidates only the second occurrence; the first remains matched and is preselected. A conflicting duplicate therefore silently applies its first rate. Reject every row in a duplicated-user group or reject the entire preview.

5. The stale-preview protection has its own TOCTOU race. Current rates are read and compared outside the write transaction at actions.ts, while the transaction starts at actions.ts. A concurrent manual edit between those points is overwritten. Lock and revalidate the users inside the same transaction, or make each update conditional on the previewed old values.

6. Payroll authorization is inconsistent. Payroll actions/export allow ADMIN or financialReports, but /api/users allows only MANAGER/ADMIN. Thus FINANCE with the intended permission cannot populate the rates panel, while a MANAGER without that permission can see payroll data and unusable controls. Provide a payroll-scoped roster endpoint/page using the identical permission gate rather than loosening the full users endpoint.

7. The zero-rate fix remains bypassable and mislabels salaried staff. createTimeEntryFromStoredRatesCore() can still create completed labor at a stored $0 rate without blocking or flagging it. Separately, the manager badge omits payType at manager/time-entries/page.tsx, so an explicitly salaried manager outside the environment email list is falsely marked “No pay rate.” Apply the same canonical rule to stored-rate manual creation and pass payType everywhere it is evaluated.

8. lastRateSyncAt is not truthful across all rate writers. /api/users PATCH still updates hourly/burden rates without updating the timestamp. Either remove that rate-writing path or update the timestamp atomically there too.

9. The export endpoint bypasses the shared strict date validator. route.ts checks only the string shape. A value such as 2026-02-31 reaches startOfDateInTimeZone() and throws, yielding a 500 instead of a 400. Use validatePayrollRange() as the comments and tests claim.

10. The new company-wide range query has no supporting index. gusto-export-db.ts filters all TimeEntry rows by startTime, but the model’s indexes at schema.prisma omit that column. Add a startTime index to both the Prisma migration and production apply script.

11. Lock enforcement uses a different timezone from lock/export creation. Export and lock creation use resolveCompanyTimeZone(), while payroll-period.ts defaults enforcement to the hard-coded Los Angeles constant. A configured company timezone changes period/workweek boundaries, causing incorrect freezing near boundaries. Use one resolved timezone consistently and preserve the relevant lock-time configuration.

VERDICT: REQUEST_CHANGES

[github-actions]

1. [Blocker] Meal settlement can mutate an already locked period. Clock-out and PATCH release the payroll advisory lock before calling settleDay() (clock-out route, PATCH route). settleDay() then uses only its separate WA-break lock (wa-breaks-db.ts). A locker can commit between those transactions, after which settlement rewrites paid hours and costs. The explicit deferred-settlement action has the same race. Settlement and the initiating write must remain under one payroll shared lock, with a real concurrent-Postgres regression test.

2. [Blocker] Writers validate stale timestamps, so concurrent edits bypass the lock. withPayrollWriteTx() checks dates captured before its transaction (payroll-period.ts). The clock-out update only predicates on id, userId, and endTime (time-entries route). Another writer can move the row, a locker can then lock its new period, and the stale writer can subsequently close/delete/move it without checking its actual stored startTime. Re-read and row-lock each target inside the transaction before checking periods; bulk operations need the same treatment.

3. [Blocker] A locked export is not reproducible because most inputs to its hash remain mutable. Historical exports are rebuilt from current user status/name/email/pay type and current Gusto mappings (gusto-export-db.ts, gusto-export-db.ts). Changing payType after locking can retroactively include or exclude someone from the summary. Logistics routing also changes the project and cost-code fields hashed into the detail CSV without any period guard (logistics route, actions.ts). Snapshot the locked export or all relevant inputs, and enforce 423 on post-lock recoding that affects the exported detail.

4. [Major] Rate-import preview evidence is forgeable and omits the old pay type. rowFingerprint() is plain client-reproducible concatenation and contains no previous payType (rate-import.ts). Apply recomputes it using the submitted new pay type, while the compare-and-set only constrains hourlyRate (actions.ts, actions.ts). A concurrent null→SALARY correction can therefore be overwritten by a stale HOURLY preview, and a caller can fabricate an arbitrary “previewed” update. Use a server-authenticated/persisted preview token containing both old values and include both in the transactional CAS.

5. [Major] The manual-entry backfill changes paid hours into raw shift spans. The migration synthesizes endTime = startTime + durationHours (migration.sql), and new manual entries do likewise (time-expense-core.ts). Later settleDayPlan() interprets that span as raw worked time and may deduct a meal; persistence also reprices it using the employee’s current rate (wa-breaks.ts, wa-breaks-db.ts). An eight-hour historical manual entry can silently become 7.5 hours at a newly imported rate. Preserve manual-entry semantics and historical effective rates during settlement.

6. [Major] The “Settle deferred days” action cannot clear every blocker it reports. Export readiness checks the full workweek envelope (gusto-export-core.ts), but the action only queries DEFERRED rows inside the literal pay period (actions.ts). A deferred row just outside a midweek or Sunday-start period blocks export indefinitely while the offered button ignores it. Query and settle the same envelope used by readiness and locking.

VERDICT: REQUEST_CHANGES

[github-actions]

This is not safe to merge.

1. Critical — payroll CSV snapshots lack RLS. migration.sql creates PayrollPeriod in Supabase’s exposed public schema and stores employee names, emails, and hours, but neither the migration nor apply script enables RLS or revokes Data API roles. Next.js authorization does not protect direct PostgREST access. Enable and verify RLS with no public policies, or explicitly revoke anon/authenticated access.

2. Critical — deployment permanently applies unconfirmed payroll assumptions. apply-payroll-phase5.mjs hardcodes CJ/Richard as salaried and then marks every other activated crew/manager hourly. Yet the PR says these defaults are pending Justin. Because stored payType overrides the environment fallback, correcting PAYROLL_SALARIED_EMAILS later does nothing. A wrong seed can exclude hourly staff from Gusto or mishandle salaried staff. Leave payType null until confirmed, or require an explicit confirmed mapping before seeding.

3. Critical — a company-time-zone change bypasses the frozen export. gusto-export-db.ts identifies a period by exact UTC timestamps, while route.ts derives those timestamps using today’s company time zone. After a zone change, the same date keys no longer find the locked row; line 74 consequently serves a live CSV instead of its snapshot, even though the range overlaps a lock. Persist stable local range keys or locate using the lock’s stored zone, and refuse all live downloads overlapping a lock without a resolvable snapshot.

4. High — non-admin payroll users can overwrite a locked period’s audit record. actions.ts unconditionally upserts and updates lockedAt, lockedById, exportHash, and both snapshots. The UI hiding the button is not authorization. A financialReports user can invoke the server action directly and “re-lock” an already locked period, replacing the file supposedly sent to payroll. Reject an already locked row with an in-transaction CAS; require the explicit ADMIN unlock path first.

5. High — the production DDL verifier can report success with a broken schema. apply-payroll-phase5.mjs verifies only 9 columns and omits timeZone, summaryCsvSnapshot, and detailCsvSnapshot. Missing any of them causes immediate Prisma P2022 failures after deployment. Verify all columns, exact types, FK/index definitions, and the RLS state.

6. High — advisory locks are acquired in conflicting orders. wa-breaks-db.ts takes the per-day lock before the payroll shared lock, while clock-out/edit paths take the payroll lock before the per-day lock. With an exclusive period-lock request queued, this can form a three-transaction deadlock and abort a clock-out, settlement, or lock. Establish one global ordering—payroll lock first, then day lock—on every path.

7. Medium — CSV injection protection is incomplete. gusto-export-core.ts checks only the literal first character. Formula payloads hidden behind whitespace or invisible characters can survive and become active when spreadsheet software normalizes the cell. Strip or scan all dangerous leading whitespace before checking =, +, -, and @, and add regression cases.

8. Required verification is missing. The lock tests are predominantly pure fakes and source-text tripwires; they do not exercise PostgreSQL advisory locks, row locks, transaction races, RLS, or the raw array binding. Add disposable-Postgres integration tests for concurrent lock/write/re-lock operations and time-zone changes, and provide the repository-required green money-pipeline E2E result.

VERDICT: REQUEST_CHANGES

[github-actions]

Three release blockers:

1. src/app/api/time-entries/[id]/route.ts:413 fails to revalidate ownership under the row lock. It rereads only startTime and updatedAt, then prices and settles using stale existing.userId (:421, :454). If a concurrent manual edit reassigns A’s entry to B, the PATCH can commit B’s entry with A’s rates and settle A’s day. Because authorization also used the stale owner, A can modify B’s reassigned entry. The telemetry branch has the same authorization race via an unconditional update at :133. Reread userId under lock, reauthorize, and reject ownership changes with ENTRY_MOVED; add a real concurrent-reassignment test.

2. DELETE has the same owner-change hole. Its guard rereads only startTime at route.ts:551. deleteEntryAndSettle() then discovers the current owner at wa-breaks-db.ts:263, but only acquires another day lock when the date changed (:266). An A→B reassignment on the same date therefore settles B’s day while holding only A’s day lock; concurrent B settlement can interleave and overwrite paid hours, deductions, or costs. Treat either userId or day changes as movement and retry with the correct complete lock set.

3. The production --dry-run verifier is not trustworthy. EXPECTED_OBJECTS at scripts/apply-payroll-phase5.mjs:58 checks mostly name existence, while omitting script-managed RLS state entirely. It also accepts same-named tables/indexes in other schemas and does not verify index columns, column definitions, or CHECK expressions. Consequently :293 can print “all objects … present” when payroll RLS is disabled or a money-critical object has the wrong definition. Verify schema-qualified definitions and relrowsecurity for all three protected tables, and add corruption/drift tests—not merely a test that drops one correctly named index.

VERDICT: REQUEST_CHANGES

[github-actions]

1. Concurrent PATCH edits can silently overwrite each other. The handler computes payroll fields from the initial read, but later compares updatedAt against the value re-read under the row lock—not against the initial value. That predicate is effectively guaranteed to match. A concurrent edit changing endTime, meal state, or review flags while retaining the same owner/start time is overwritten with stale calculations. Compare the locked row to existing.updatedAt, or perform all state-dependent calculations from the locked row. Add a same-owner concurrent-edit regression test. route.ts

2. The PATCH zero-rate safeguard still has a race that can book an unflagged $0 entry. zeroRate and the review flag are derived before the transaction, while the authoritative owner re-read only changes pricing/refusal. With acknowledgeZeroRate=true, a rate that becomes zero before the locked read is accepted and repriced to zero, but no needsReview flag is added because the stale outer zeroRate was false. Conversely, a concurrently fixed rate gets a false $0 warning. The live locked rate must drive both the decision and appendZeroRateReview. route.ts

3. The GitHub idempotency lease is shorter than the resume path’s maximum execution time. The lease lasts 120 seconds, but a resumed submission performs a marker search and issue creation sequentially, each with a 90-second timeout. After 120 seconds another retry can acquire the lease and create a second issue while the first attempt is still running. The DB fencing prevents stale state updates; it cannot undo the duplicate external issue. Use one absolute provider deadline, renew the lease, or size the lease beyond the combined calls plus margin. submission-guard.ts, request route, github.ts

4. The supposedly removed salaried-email guess still exists at runtime. With PAYROLL_SALARIED_EMAILS unset in production, salariedEmails() falls back to hardcoded CJ/Richard addresses, and zeroRateBlocks() uses that fallback to exempt them. This directly contradicts the deploy note’s fail-closed claim and can permit unflagged $0 closes without a stored human-confirmed payType. Remove the hardcoded runtime default and update the tests/environment documentation accordingly. payroll-config.ts, pay-rate-guard.ts

5. An unset pay type blocks payroll even when the employee has no hours in the period. The exporter explicitly iterates every supplied user, while the DB supplies every active null-pay-type crew/manager. Thus a newly activated zero-hour employee can prevent exporting everybody else’s approved payroll. This contradicts both the function contract and PR statement that unknownPayType applies to people with hours in the period. Scope the blocker to in-period participants or explicitly change the advertised behavior. gusto-export-core.ts, gusto-export-db.ts

VERDICT: REQUEST_CHANGES

[github-actions]

1. P0 — Payroll settlement can mutate locked hours after a timezone change. The guard interprets dayKey using the current configured timezone at wa-breaks-db.ts:129, but settlement selects rows using the hardcoded Los Angeles converter at wa-breaks-db.ts:187 and company-day.ts:16. For example, after locking in Los Angeles and switching to New York, 2026-08-31 is checked at 04:00Z, before a Los Angeles envelope beginning at 07:00Z, so the guard passes; settlement then updates Los Angeles-day entries whose timestamps are inside the locked envelope. Use one explicit timezone throughout settlement—or, preferably, guard the exact row timestamps that will be rewritten—and add a cross-timezone regression test proving locked rows cannot change.

2. P1 — The supposedly removed salaried-email guess remains active in the example configuration. .env.example:34 says it shows built-in defaults and .env.example:44 assigns CJ and Richard, while payroll-config.ts:34 explicitly says nobody is guessed and sets an empty default. Following the example makes those addresses bypass the zero-rate guard before a human assigns payType, allowing unreviewed $0 closes contrary to the PR’s fail-closed deployment policy. Make the example unset/empty and remove all stale “default CJ + Richard” documentation.

3. P2 — The bug-report throttle does not enforce “five reports in an hour.” submission-guard.ts:163 truncates to a fixed UTC-hour bucket, and submission-guard.ts:432 counts only that bucket. A caller can submit five reports at 14:59 and five more at 15:00, creating ten GitHub issues within seconds. The rolling-window helper already exists but is unused. Implement a transactional rolling-hour limit, or explicitly document and test the weaker fixed-window policy.

VERDICT: REQUEST_CHANGES

[github-actions]

1. High — Clock-out can falsely return 423 PERIOD_LOCKED after a company timezone change. The injected loader in route.ts omits PayrollPeriod.timeZone. The fail-fast check therefore reconstructs historical lock envelopes using the current company timezone, despite the authoritative transaction correctly using the stored lock timezone. This can strand an otherwise valid open punch. Select timeZone here or use the canonical loadLockedPeriods() implementation.

2. Medium — Lock errors report the wrong payroll dates after a timezone change. payroll-period.ts formats periodStart and periodEnd with the global COMPANY_TIME_ZONE and its parameter type discards period.timeZone. Enforcement preserves the original lock timezone, but the user-facing error does not. Format the range using the locked period’s stored timezone.

3. Medium — The help-report rate limit is trivially burstable. submission-guard.ts buckets counters by wall-clock hour, allowing five reports at 12:59:59 and another five at 13:00:00. That contradicts the module’s “five submissions per hour” rolling-window helpers and weakens the abuse control on an endpoint that creates GitHub issues. Implement an atomic rolling-window/token-bucket limit, with a boundary test.

VERDICT: REQUEST_CHANGES

[github-actions]

I found two release-blocking holes. I reviewed the matching PR #441 head (9ad3098); the local sandbox prevented direct reads of /tmp/pr.diff.

1. Payroll settlement can mutate locked entries after a time-zone change. wa-breaks-db.ts converts a Los Angeles day key into one midnight instant and assertDayUnlockedInTx() tests only that instant against an envelope stored in another time zone. For a period locked in Pacific/Honolulu, LA midnight is three hours before the locked envelope, so the guard passes; settlement then selects and rewrites entries later that same LA day which are inside the locked period. Check interval overlap for the entire settlement day—or validate every affected row’s actual startTime—and add a west-of-LA regression test.

2. The route authorization is undermined by missing Supabase RLS. The migration enables RLS/revokes for PayrollPeriod, User, and HelpSubmissionQuota, but not TimeEntry or HelpRequest (migration, RLS inventory). Under standard Supabase public-schema grants, an anon/authenticated key can bypass the new export authorization and help-route controls through PostgREST, exposing payroll rows and crew reports. Enable RLS with no client policies, revoke anon/authenticated, and cover both tables in the apply-script drift checks and tests.

VERDICT: REQUEST_CHANGES

[github-actions]

1. P0 — Parent deletion still destroys payroll history. payroll-parent-delete.ts:123 deletes every time entry not covered by a current lock, and both user deletion and project deletion invoke it. Historical paid entries predate PayrollPeriod, so merging converts production’s safe FK refusal back into silent payroll-history destruction. Refuse deletion when any time entries exist, or soft-delete/archive the parent; “unlocked” does not mean “unpaid” or disposable.

2. P1 — Rate-import approvals remain replayable after pay-type changes. The row signature includes payType and lastRateSyncAt at actions.ts:15457, but setUserPayType changes only payType at actions.ts:15557. A HOURLY → SALARY → HOURLY cycle restores the exact signed state, making an old approval valid again. Add a monotonic payroll revision updated by every rate/pay-type writer—or advance the signed stamp on pay-type changes—and test this A→B→A case.

3. P1 — PAYROLL_SALARIED_EMAILS cannot actually unblock payroll export. gusto-export-db.ts:325 classifies a null-pay-type user named in the environment as salaried, but gusto-export-core.ts:394 independently blocks every null pay type. Consequently the UI and zero-rate guard treat the employee as salaried while every export returns 409. Either honor the explicit salaried-email configuration in blocker evaluation or remove the claimed runtime override and require stored payType consistently.

VERDICT: REQUEST_CHANGES

[github-actions]

Scope note: the local runner failed before executing shell commands (bwrap networking error), so I reviewed the matching PR #441 at head d4789463 through GitHub rather than reading /tmp/pr.diff directly. CI being green does not cover the races below. I also applied the Supabase security and PostgreSQL lock-order guidance; the RLS setup looks coherent, but the transaction locking does not.

1. Clock-out can overwrite concurrently approved payroll state. The handler builds update data from the pre-transaction existing record, including mealSkipStatus and other review/logistics state (route.ts). Inside the transaction it rereads only startTime under FOR UPDATE (route.ts), and its final CAS checks only startTime and endTime (route.ts). A manager can approve a meal skip or change routing after the initial read; clock-out then acquires the lock and silently writes stale state. Reread every state-dependent field after locking and recompute from that snapshot, or CAS on updatedAt and reject moved entries. Add a real concurrent approval/edit-versus-clock-out regression test.

2. Deferred-day settlement mixes configurable timezone day keys with hardcoded Los Angeles windows. The action derives dayKey using resolveCompanyTimeZone() (actions.ts), then passes it to settleDay (actions.ts). That implementation interprets the key using COMPANY_TIME_ZONE/Los Angeles (wa-breaks-db.ts). For another configured timezone, an entry near midnight is settled against the wrong date or excluded. Worse, a zero-row result is still counted as settled. Use one timezone definition end-to-end, verify the blocker was actually cleared before incrementing the count, and test a non-Pacific midnight boundary.

3. Opposite User/TimeEntry lock orders create a PostgreSQL deadlock. Mark-for-review locks the TimeEntry first (actions.ts) and later locks its owner FOR UPDATE (actions.ts). Deferred settlement locks the User FOR SHARE first (wa-breaks-db.ts) and then updates time entries (wa-breaks-db.ts). Concurrent execution can make each transaction wait for the other until PostgreSQL aborts one. Establish one global lock order—or stop taking an exclusive user lock merely to read rates—and add a two-connection settlement-versus-review test.

4. The signed rate-import preview remains replayable after some pay-type changes. applyRateChangeInTx advances lastRateSyncAt only when hourly/burden rates change (pay-rate-write.ts); pay-type-only writes through several user APIs leave it unchanged. Only setUserPayType separately advances it (actions.ts). Therefore HOURLY → SALARY → HOURLY through another writer restores every value covered by an old preview token, allowing that stale preview to be confirmed. Introduce a monotonic payroll revision or advance the signed stamp from every rate/pay-type writer, with route-level replay tests.

5. The help throttle is trivially burstable at an hour boundary. It truncates timestamps to a fixed UTC-hour bucket (submission-guard.ts) and counts only that bucket (submission-guard.ts). A staff account can submit five reports immediately before the boundary and five immediately after, creating ten external GitHub issues within seconds. The declared rolling-window constants are unused. Implement the intended transactional rolling window/token bucket, or explicitly change and test the product contract as fixed-window throttling.

VERDICT: REQUEST_CHANGES

[github-actions]

Reviewed the matching PR #441 head (99959e55) because the sandbox could not read /tmp/pr.diff directly. Applying the project’s Supabase/Postgres locking guidance exposed a real lock-order violation.

Required changes:

1. Stale Gusto approvals remain replayable. pay-rate-write.ts advances lastRateSyncAt only for hourly/burden changes, despite several API routes allowing pay-type-only writes. A HOURLY → SALARY → HOURLY cycle restores every signed preview field, allowing an old import approval to overwrite newer payroll decisions. Use a monotonic payroll revision—or collision-proof advancement—for every rate or pay-type mutation, with replay tests through every public writer.

2. The claimed global row-lock order is violated. payroll-period.ts locks only declared entry IDs, after which wa-breaks-db.ts locks every closed row in a 72-hour superset. Concurrent edits to closed entries on adjacent days take different day locks, each lock its own row, then each settlement can request the other row in the opposite order—a PostgreSQL deadlock. Use exact timezone-aware day bounds or collect and sort the complete row set before acquiring any row lock. Add a real two-connection adjacent-day test.

3. The authorization expansion lets ordinary crew create automated agent tasks. bug-widget-auth.ts admits every active role, while bug-fix/route.ts creates GitHub issues labeled agent-task and explicitly hands them to Phantom. The mobile feature uses the ordinary request endpoint; it does not require granting field crew access to this automation-triggering path. Keep /bug-fix privileged or strip automation labels for unprivileged callers, and test that crew cannot trigger an agent task.

4. “Five per hour” is bypassable at every UTC hour boundary. submission-guard.ts keys quota by a fixed clock-hour bucket. Five submissions at 12:59:59 followed by five at 13:00:00 permits ten reports within seconds. The rolling-window helper is present but unused. Implement an atomic sliding-window/token-bucket limit and test concurrent submissions across a bucket boundary.

VERDICT: REQUEST_CHANGES