Skip to content

Make check-in reachable, or nothing can be submitted - #327

Merged
aamoghS merged 2 commits into
mainfrom
hotfix/checkin-unreachable
Aug 9, 2026
Merged

Make check-in reachable, or nothing can be submitted#327
aamoghS merged 2 commits into
mainfrom
hotfix/checkin-unreachable

Conversation

@aamoghS

@aamoghS aamoghS commented Aug 9, 2026

Copy link
Copy Markdown
Member

Blocker introduced by #325. D2 made submitting a project require checked_in. Nothing in the product could set that status.

The chain

  1. scanParticipantPass — the badge scan at the door — inserts a hackathon_event_attendee row and deliberately leaves the roster alone. Its own comment says so: "A scan changes one event's attendee count and nothing else."

  2. No screen ever passes checked_in to updateParticipantStatus / batchUpdateParticipantStatus. AttendeesTab only ever sends approved, rejected or waitlisted; a repo-wide grep for checked_in under sites/ finds only display and filter uses.

  3. So approved was the terminal reachable status, and every submission returned:

    You need to check in at the event before submitting. Find a volunteer and have your badge scanned.

    — telling the attendee to do the exact thing they had just done. Re-scanning could not clear it. No project could be submitted for the entire event.

That was right while checked_in was only a label. It stopped being right the moment submission depended on it, and I did not notice when I made that change.

The fix

The first scan is the check-in. An approved participant is promoted on their first scan and checkedInAt is stamped once — so the arrival time keeps pointing at when they arrived rather than at their most recent meal. Cache eviction stays narrow (the event, plus the one person scanned) because this runs at every door station all weekend; the old blanket pattern wiped every attendee's cached registrations on every scan.

Plus a manual check-in action in the attendees table. The scan needs an event to scan somebody into, and submission must not depend on the schedule having been built yet. Offered only for people already accepted — the server refuses the rest, per the transition guard added in #325.

The qr-checkin test that asserted the old behaviour is updated, not deleted — it was correct under the old contract, and the comment now records why the contract changed.

How it was found

An adversarial review pass after #325 merged: two independent agents traced every writer of registrationStatus and every UI call site and reached the same conclusion separately. Worth saying plainly — Greptile reviewed this exact diff and did not catch it, and neither did 423 passing tests, because every one of them sets the fixture status directly and so cannot see that no code path produces it.

Verification

typecheck · 425 tests · lint --max-warnings 0 · build. The promotion is mutation-tested: forcing it off fails the test.


Note

Medium Risk
Changes door-scan registration state and project-submission eligibility at high volume, but scope is limited to scanParticipantPass with transactional writes and compare-and-set guards against race overwrite.

Overview
Fixes a release blocker from #325: project submission requires checked_in, but badge scans only recorded per-event attendance and never updated the roster—so attendees were told to scan again with no way to succeed.

scanParticipantPass now runs attendance insert and roster promotion in one transaction (so a failed promotion rolls back attendance and a rescan can recover). On the first scan of an approved participant it sets registrationStatus to checked_in and stamps checkedInAt once. The update uses a compare-and-set (WHERE registrationStatus = 'approved') so a rejection between read and write cannot be overwritten. Cache eviction stays narrow: event list always; participant caches only when someone was newly promoted. The mutation returns checkedIn indicating whether promotion happened.

Attendees admin UI adds a manual Check in action for approved rows (for cases before schedule/events exist), wired to existing updateParticipantStatus.

Tests flip the old “scan does not update roster” expectation and add coverage for promotion, no restamp when already checked in, and the WHERE guard.

Reviewed by Cursor Bugbot for commit 1150903. Bugbot is set up for automated code reviews on this repo. Configure here.

#325 made submitting a project require `checked_in`. Nothing in the product
could set it.

`scanParticipantPass` — the badge scan at the door — recorded attendance for
one event and deliberately left the roster alone, and no screen ever passed
`checked_in` to `updateParticipantStatus`: the attendees table only ever sends
approved, rejected or waitlisted. So the terminal reachable status was
`approved`, and every submission returned "You need to check in at the event
before submitting. Find a volunteer and have your badge scanned" — telling the
attendee to do the exact thing they had just done. No project could be
submitted for the entire event.

The first scan is now the check-in: an approved participant is promoted and
`checkedInAt` is stamped once, so the arrival time keeps pointing at when they
arrived rather than at their most recent meal. Cache eviction stays narrow —
the event, plus the one person scanned — because this runs at every door
station all weekend.

Also adds a manual check-in action to the attendees table. The scan needs an
event to scan somebody into, and submission must not depend on the schedule
having been built yet.

The qr-checkin test that asserted the old behaviour is updated rather than
deleted: it was right while the status was only a label, and the comment now
records why that changed.

Found by an adversarial review pass after #325 merged — two independent agents
traced every writer of registrationStatus and every UI call site. Greptile
reviewed the same diff and did not catch it, and neither did 423 passing tests,
because every one of them sets the fixture status directly.

Verified: typecheck, 425 tests, lint --max-warnings 0, build. The promotion is
mutation-tested.
@cursor

cursor Bot commented Aug 9, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_16c8b634-e335-4fcf-a193-f2159d820615)

@github-actions

github-actions Bot commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

@github-actions github-actions Bot added the bug Something isn't working label Aug 9, 2026
@github-actions

github-actions Bot commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

Visit the preview URL for this PR (updated for commit 1150903):

https://hacklytics2027--pr-327-2x05y56j.web.app

(expires Sun, 16 Aug 2026 03:01:05 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: c48ba34db61581e25fe2978355160b5eefe0e83f

@greptile-apps

greptile-apps Bot commented Aug 9, 2026

Copy link
Copy Markdown

Greptile Summary

The PR makes badge scanning an atomic check-in operation and adds a schedule-independent manual check-in action.

  • Promotes only participants whose current status remains approved, preventing a stale scan from reversing an administrative status change.
  • Commits event attendance and roster promotion in one transaction so failed promotions remain recoverable by rescanning.
  • Evicts caches only for the scanned participant and exposes whether promotion occurred.
  • Adds and updates tests for promotion, timestamp preservation, concurrent status changes, and QR check-in behavior.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
packages/api/src/routers/hackathon/admin.ts Atomically records attendance and conditionally promotes the participant using a current-status compare-and-set, resolving both previously reported failures.
sites/mainweb/components/admin/hackathons/AttendeesTab.tsx Adds a manual check-in action for approved attendees through the existing guarded status-update procedure.
packages/api/src/.internal-tests/hackathon-admin-edge.test.ts Covers initial promotion, timestamp preservation, and the concurrent administrative-status-change regression.
packages/api/src/.internal-tests/qr-checkin.test.ts Updates the QR check-in contract test to require roster promotion alongside event attendance.

Sequence Diagram

sequenceDiagram
    participant Scanner
    participant API as scanParticipantPass
    participant DB
    Scanner->>API: Scan approved participant
    API->>DB: Begin transaction
    API->>DB: Insert event attendance
    API->>DB: "UPDATE participant WHERE status = approved"
    alt Participant still approved
        DB-->>API: Updated row
        API->>DB: Commit attendance and promotion
        API-->>Scanner: "checkedIn = true"
    else Status changed concurrently
        DB-->>API: No updated row
        API->>DB: Commit attendance only
        API-->>Scanner: "checkedIn = false"
    end
Loading

Reviews (2): Last reviewed commit: "fix(hackathon): make the scan promotion ..." | Re-trigger Greptile

Comment thread packages/api/src/routers/hackathon/admin.ts Outdated
Both points from Greptile on #327 — the same family as the earlier ones.

**A concurrent revocation could be overwritten.** The status was read at the
top of the procedure and the write went out by participant id alone, so an
organiser rejecting or waitlisting somebody between the two had that decision
silently reversed — handing submission rights back to a person who had just
been removed from the event. `registrationStatus = 'approved'` is now in the
WHERE, so the promotion is a compare-and-set and the loser changes nothing.

**A half-finished scan could not be retried.** The attendance insert committed
before the promotion, so a failure in between left attendance recorded and the
roster untouched — and the retry hits the duplicate guard ("already checked
into"), which meant the promotion could never happen and that attendee could
not submit for the rest of the event. Both writes are one transaction now, so
a failure rolls the attendance back and a rescan is the fix.

Verified: typecheck, 426 tests, lint --max-warnings 0, build. The compare-and-set
is mutation-tested — dropping the status predicate fails the new test.
@cursor

cursor Bot commented Aug 9, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_017d193b-8775-4d4e-afa2-4b92b9e38763)

@aamoghS

aamoghS commented Aug 9, 2026

Copy link
Copy Markdown
Member Author

Both correct, and both the same family as the earlier findings — a read and a write that are not the same moment. Fixed in 1150903.

Concurrent revocation overwritten — the status was read at the top of the procedure and the write went out by participant id alone, so an organiser rejecting or waitlisting somebody between the two had that decision silently reversed. That is the worst possible direction for this particular bug: it hands submission rights back to a person who was just removed from the event. registrationStatus = 'approved' is now in the WHERE, so the promotion is a compare-and-set and the loser changes nothing. checkedIn in the response is derived from what the statement actually matched, not from the stale read.

Half-finished scan could not be retried — also right, and it fails closed in the worst way: the attendance insert committed first, so a failure before the promotion left attendance recorded and the roster untouched, and the retry hits the duplicate guard ("already checked into"). The promotion could then never happen and that attendee could not submit for the rest of the event. Both writes are one transaction now, so a failure rolls the attendance back and a rescan is the fix.

Tests: the compare-and-set is mutation-tested (dropping the status predicate fails the new test), and the "already checked in" case now asserts the arrival timestamp does not move rather than that no statement is issued — the statement is issued, it just matches nothing.

Gate: typecheck · 426 tests · lint --max-warnings 0 · build.

@aamoghS
aamoghS merged commit fcd007d into main Aug 9, 2026
17 of 19 checks passed
@greptile-apps

greptile-apps Bot commented Aug 9, 2026

Copy link
Copy Markdown

Want your agent to iterate on Greptile's feedback? Try greploops.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant