Skip to content

Gate Admissions (gate scanner + personal staff PINs) — to QA#1066

Merged
peterdrier merged 19 commits into
mainfrom
feat/gate-admissions
Jun 30, 2026
Merged

Gate Admissions (gate scanner + personal staff PINs) — to QA#1066
peterdrier merged 19 commits into
mainfrom
feat/gate-admissions

Conversation

@peterdrier

Copy link
Copy Markdown
Owner

Gate Admissions — routed to QA (origin) from upstream PR 895

Brings @veryaaron's Gate admissions feature (originally opened as a cross-fork PR straight to upstream, nobodies-collective/Humans#895) through our origin → QA → prod flow. Feature authored by Aaron with Claude Code; reviewed and fixed-forward here before QA.

Posture sign-off: Peter has blessed making Humans the system-of-record for gate admission (append-only gate_scan_events) and the new public surface (gate_staff_pins, IGateService.GetEnrolledSupervisorIdsAsync, ITicketVendorService.CreateCheckInAsync).

What it does

Gate ticket scanning that decides entry against three layers: (1) ticket valid, (2) name matches photo ID (manual Yes/No), (3) before the general-entry cutoff a holder needs an Early Entry grant. Too-early/child-without-ID admits require an attributable supervisor override (server-verified PIN + live Admin/Board/TicketAdmin role). Staff claim the shared scanner with a personal 4-digit PIN (hashed via Identity). Includes GDPR export contributor, account-merge re-FK, daily retention purge, and a flag-gated (Gate:VendorMirrorEnabled, default off) TicketTailor check-in mirror.

Review findings fixed-forward in this branch

A full structured review (rule-conformance + code/security + a focused auth/PIN/override audit) found the override security model sound (no path admits a too-early/no-ID scan without a genuine server-verified supervisor PIN). Four issues were fixed on top of Aaron's work (commit fix(gate): …):

  • F1 — orphaned admin page: /Gate/Admin (the cutoff-setting screen) had no nav link, so the section shipped unreachable and every scan fell back to AMBER. Added a "Gate settings" entry to the Tickets admin nav.
  • F2 — dead kiosk search: the route-locked kiosk can't reach /api/profiles/search, so the claim-screen search box was dead. Added a Gate-scoped, name-only /Gate/Search (inside the route-lock; no email/directory exposure) and pointed the picker at it. Overrides stay tap-list-only.
  • S1 — gate-wide lockout DoS (Medium): PIN throttle keyed on the (proxy-collapsed) device IP, so 5 wrong PINs froze every claim/override at the terminal for 15 min. Now throttles per target user only (brute-force ceiling unchanged at 5/15 min).
  • S2 — forgeable attribution (Low–Med): ClaimPin accepted an arbitrary userId; now validates the claimant resolves to an active member before enrolling a PIN or stamping the session.

New tests: GateControllerClaimTests (per-user throttle, claimant validation, kiosk search).

Migration consolidation

The two in-flight Gate migrations were consolidated into one (/ef-regen) — origin/main had raced ahead with a later-timestamped migration, leaving them mid-chain. Snapshot restored from origin/main as the sanctioned /ef-regen first step; one fresh end-of-chain AddGateSection migration regenerated. Clears the "≤1 migration per PR" CI gate.

Verification

  • dotnet build clean; dotnet format --verify-no-changes clean.
  • Application.Tests 3825 passed; Web.Tests 339 passed.

Deferred to tracked issues (non-blocking)

PIN-set/supervisor-status disclosure at the kiosk (low, trusted device); roster sort sitting in the service layer; IGateRepository doc comment omits gate_staff_pins; vendor job enqueued-but-no-op when the flag is off.

To verify on this preview deploy before prod

  • Too-early → supervisor-override end-to-end against a real ticket (AdmittedEarlyOverride recorded).
  • Claim via the restored /Gate/Search name picker (non-rostered staffer).
  • /Gate/Admin reachable from the Tickets admin nav; setting the cutoff clears the AMBER fail-safe.
  • Per-user throttle: a locked-out staffer doesn't block others at the same terminal.
  • Before enabling Gate:VendorMirrorEnabled: verify the TicketTailor POST /v1/check_ins body against the live API.

Original feature: @veryaaron · 🤖 Review + fixes generated with Claude Code

veryaaron and others added 19 commits June 29, 2026 00:40
New vertical Gate section that decides event entry from a Ticket Tailor scan: server-side admission rules (validity, Early Entry cutoff, duplicate), append-only gate_scan_events audit + atomic per-barcode dedupe, and a server-clock GeneralEntryOpensAt cutoff. Backend + tests only; the UI, TT check-in writeback mirror, and name-mismatch fix link are deferred follow-ups documented in docs/features/gate-admissions.md. Leaves the read-only Scanner section invariant untouched.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…vendor mirror

Builds the Gate section out beyond the backend slice: GDPR IUserDataContributor (DSAR) + IUserMerge (re-FK on account merge) + GateRetentionJob; vendor check-in mirror (ITicketVendorService.CreateCheckInAsync -> TicketTailor POST /v1/check_ins + stub, GateVendorCheckInJob fire-and-forget on admit); and the terminal UI (GateController with tap-to-claim, session-derived scannedByUserId, green-on-Yes ID confirm, supervisor-PIN child waiver, leaderboard, settings) plus verdict views and tablet JS/CSS to the visual spec. Folds in two peer reviews: PIN compared via fixed-width hashes, server-side settings validation, vendor mirror fails fast on 4xx, JS in-flight lock + error card + id-confirm cue. Builds clean; Application.Tests + Web.Tests green (one pre-existing unrelated Shifts failure on main).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…safe cutoff, PIN throttle, docs

Addresses Peter's review of the Gate admissions PR.

Blockers:
- Add dedicated GateAdmit authorization policy for the write actions
  (Decision, Claim POST) so they no longer ride on the read-only
  ScannerAccess policy. Same principals today (incl. the shared gate-terminal
  account by well-known id), kept separate so they can diverge.
- Feature-flag the TicketTailor check-in mirror behind Gate:VendorMirrorEnabled
  (default off) in GateVendorCheckInJob until the POST /v1/check_ins payload is
  verified against a live key — a wrong body fails silently and permanently.
  ExecuteAsync signature left frozen (Hangfire-invoked).
- Fix the fail-open cutoff: an unset GeneralEntryOpensAt (Instant.MinValue
  sentinel) is now treated as "not configured" → new CutoffNotConfigured AMBER
  outcome (escalates to Unresolved, never a silent admit) plus a loud terminal
  banner. An admin must set the cutoff before doors.

Inline:
- Throttle supervisor-PIN attempts on /Gate/Decision per source IP (own bucket,
  mirrors GateLoginThrottle) — a static PIN on a write endpoint is brute-forceable.
- Docs: honor-system attribution note, Tickets cache freshness window, and the
  admin-sets-cutoff step in Gate.md; fix the stale "controller deferred" Gate row
  in _Index.md; route table now shows GateAdmit.
- Track the Postgres-backed dedupe race test as a debt-ledger follow-up
  (EF in-memory can't enforce the unique index).

Tests: GateAdmissionRules + GateService cover the not-configured fail-safe;
AuthorizationPolicyTests cover GateAdmit (roles, gate-terminal-by-id, anonymous).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…rd dead inbound signal

Verified the TicketTailor check-in payload read-only against the live API
(GET /v1/check_ins, /v1/issued_tickets, /v1/orders), per Peter's blocker 2.

- The check_in object is { issued_ticket_id, check_in_at (unix s), event_id,
  event_series_id, quantity }. The outbound CreateCheckInAsync body used
  `checked_in_at` — wrong; the live field is `check_in_at`. Fixed. id stays in
  the body (not the path); POST /v1/check_ins confirmed as the collection.
  Still unverified (needs one live POST): whether create requires event_id, so
  Gate:VendorMirrorEnabled stays default-off.

- Recorded a separate, pre-existing Tickets-section bug found during verification
  (debt-ledger inbox, issue #736): the inbound sync parses a nested `check_in`
  object the live API no longer returns (it returns a top-level `checked_in`
  string), so vendor check-in detection is always null and the gate's secondary
  CheckedInAtVendor dedupe signal never fires. Out of scope to fix in the Gate PR;
  gate_scan_events remains the dedupe authority so admission is unaffected.
  Documented the signal status in Gate.md.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
At shift start a staffer taps their name instead of searching. GateController
reads the active event's gate-team signups via the existing IShiftManagementService
(GetActiveAsync + GetBrowseShiftsAsync with IncludeSignups) and shows them as
one-tap picks on /Gate/Claim; the name/email search remains for helpers who
aren't rostered.

- Opt-in via Gate:RosterTeamId (GUID of the Shifts department that staffs the
  gate). Unset, no active event, or no signups -> search only (no behaviour change).
- De-dupes one pick per human across all gate shifts; excludes refused/bailed/
  cancelled/no-show signups.
- Reuses the existing cross-section Shifts read surface (same pattern as other
  controllers that read shift signups) — no new Shifts interface methods.
  Cross-section read Gate(Web) -> Shifts approved by Peter (verbal, 2026-06-29).
- Documented config + cross-section dependency in docs/sections/Gate.md.

Build clean; Gate + Architecture + Authorization (772) and Web (338) tests green.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… starting ±2h

- Banner: wrap the cutoff-not-set message in a <span> so the inline "Gate ▸ Admin"
  link flows in the sentence instead of being pushed out as a separate flex item;
  align icon to the first line. (Cosmetic; verified live.)
- Roster: move the claim-screen roster logic from the controller into GateService
  (GetShiftRosterAsync) — it's business logic, so it belongs below the controller.
  Now shows only volunteers whose gate shift starts within ±2h of now in the event
  zone (EventSettings.TimeZoneId), tracking shift change rather than the whole event
  roster. Controller is back to a thin call; the Gate→Shifts read stays via the
  existing IShiftManagementService. Added unit tests (in-window/out-of-window/dedupe/
  refused-excluded). Updated Gate.md.

Build clean; Gate+Architecture+Authorization (774) and Web (338) tests green.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…nceability)

From an independent design review for the rugged-tablet/desert-sun context:

- Claim screen never loaded gate.css (found via live smoke test) — the roster
  picks were falling back to Bootstrap defaults. Add the @section Styles link so
  the gate styling actually applies, and render the picks as big FILLED dark
  high-contrast buttons (outline buttons wash out in direct sun); cap the roster
  height so a large list scrolls instead of burying the search box.
- Cutoff-warning banner used #B26A00 — the exact hue of the Amber verdict card —
  so a persistent warning competed with Amber verdicts. Re-treat it as dark amber
  chrome with a bright outline; the verdict card stays the loudest thing on screen.
- Verdict reason text to full opacity (sub-100% opacity washes out first in sun);
  darker verdict badge background for contrast.

Verified live against a real DB (banner, filled roster, ±2h window all render).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
gate-admissions.md specified "a chromeless kiosk layout" but the views were wired
to _AdminLayout, so the terminal rendered as a frame inside the full Humans admin
shell (nav/sidebar/breadcrumb) instead of a purpose-built full-screen tablet page.

- Add _GateLayout: full-bleed, no admin chrome, just the gate UI + gate.css/js.
- Point Gate/_ViewStart at it for the tablet-facing views (Claim, scan terminal,
  Leaderboard); Admin.cshtml overrides back to _AdminLayout (desktop admin task).
- Kiosk body styling in gate.css.

The systems-account model (shared GateTerminal account, no roles/teams, tap-to-claim
the gate-shift member, name in header) was already on-plan; this restores the missing
full-screen layout. Verified live: Claim + scan terminal render chromeless and the
scan→verdict flow works; Admin keeps its chrome.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…readable

- Hard route restriction (Program.cs middleware): the shared GateTerminal kiosk
  account may only reach /Gate, /Account/GateLogin, /Account/Logout, and the
  access-denied page; any other path bounces it to /Gate. Defense in depth on top
  of its zero roles/teams so the systems account can't browse the rest of Humans.
- Freshness line: replace the raw microsecond ISO timestamp with a readable
  "loaded HH:mm · N min ago" that reddens once the terminal's been open >15 min
  (a once-a-minute text update, no network). Higher contrast than the old grey.

Build clean; Authorization + Gate tests green.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…mport

- GateLogin redirected to the old /Scanner section, which the new kiosk lockdown
  then bounced to /Gate — a wasted hop. Redirect straight to the gate terminal.
- The gate.js module was imported without a version query (unlike the CSS, which
  uses asp-append-version), so a deployed JS update could be masked by a cached
  module on the kiosk. Version the import via IFileVersionProvider. Found while
  verifying the new freshness indicator didn't render against a stale cached module.

Verified live as the real gate-terminal account: login -> chromeless claim ->
scan terminal; freshness shows "loaded HH:mm · just now"; all non-gate routes
bounce to /Gate.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…sor overrides

Stage 1/4 of the gate-PIN feature (NEW SURFACE — flagged for Peter's review).

- GateStaffPin entity + config (gate_staff_pins, Gate-owned, keyed by bare user-id):
  each staffer's hashed personal 4-digit device PIN, set once and reused across
  shifts to confirm who takes over the scanner and to authorize overrides.
- GateScanEvent.OverrideByUserId — records WHICH supervisor authorized an override
  admit (audit trail), a bare cross-section id like the other gate links.
- GateVerdict.AdmittedEarlyOverride — distinct verdict for a too-early admit let
  through by a named supervisor.
- One EF migration (auto-generated): adds the column + table. Down drops both.
- Fix AccountControllerGateLoginTests for the earlier /Gate login-redirect change
  (was still asserting the old /Scanner/Tickets target — missed when I only re-ran
  the Web suite at the time).

Behaviour unchanged until the flows land (stages 2-4). Build clean; Gate tests green.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Stage 2a/4. Repo + GateService surface for personal staff PINs, with the fixes a
peer-review pass flagged as blockers:

- Identity's IPasswordHasher<GateStaffPin> (vetted PBKDF2) — not hand-rolled crypto.
- SetOwnPinAsync REFUSES to enrol a supervisor (their PIN carries override power, so
  they're admin-enrolled out of band, never cold at the anonymous kiosk) and rejects
  trivial/malformed PINs (0000/1234/runs/repeats/non-4-digit).
- AdminSetPinAsync enrols any user (incl supervisors) — for the gate admin page.
- AuthorizeOverrideAsync is verify-only: enrolled AND correct PIN AND currently holds
  a gate-supervisor role (Admin/Board/TicketAdmin, server-verified via
  IRoleAssignmentService — the kiosk principal has none). Un-enrolled/non-supervisor
  rejected, never enrolled.
- Repo: Get/Upsert/Delete staff PIN; account-merge keeps the survivor's PIN (idempotent).
- GateScanEvent.OverrideByUserId is set-able so merge can re-point it.

Throttle + UI flows are stages 2b-3. Build clean; 113 Gate tests green (incl 4 new
PIN-security tests + architecture).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Stage 2b scaffolding for the claim/keypad flow.

- GatePinThrottle: 5 failed attempts then a 15-min lockout that does NOT self-reset
  (only a correct PIN clears it). Callers throttle on BOTH the target user-id and the
  source device so neither one supervisor's PIN nor one kiosk can grind the ~10k space.
  Stricter than GateLoginThrottle (which fully resets each minute) — addresses the
  peer review's "throttle too weak for per-user 4-digit secrets" blocker.
- GatePinViewModel + GatePinMode (Set / Verify / BlockedSupervisor) for the full-screen
  keypad.

Wired into DI. The keypad UI + controller flow land next. Build clean.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ide (stages 2b–4)

Completes the per-staffer PIN feature on top of the committed data model,
hardened service, and throttle.

Stage 2b — claim PIN keypad:
- POST /Gate/Claim now resolves the staffer's PIN status and renders a
  full-screen, chromeless keypad (Pin.cshtml) in the right mode (set a new
  PIN / verify an existing one / blocked for an un-enrolled supervisor);
  mode is re-derived server-side from GetPinStatusAsync, never a client hint.
- POST /Gate/ClaimPin throttles on BOTH the target user-id and the source
  device, then SetOwn/Verify, and on success stamps the scanner session from
  the verified user-id (attribution is a real PIN claim, not honour-system).
- Shared keypad: _PinPad partial + gate-keypad.js (a cache-busted *classic*
  script, so there is no module import to go stale on the kiosk) + gate-pin.js.

Stage 3 — precise too-early reason + supervisor override:
- GateScanResult carries EarliestEntryDate / Today / GeneralEntryDate so the
  too-early card distinguishes "early entry from {date} — today is {date}"
  from "no early entry — general entry opens {date}" (date only; the EE
  source stays server-side, privacy I2).
- The too-early card (and the child-without-ID waiver) admit only via a
  supervisor override: the supervisor is picked from an enrolled-supervisor
  tap-list (the route-locked kiosk can't reach /api/profiles/search) and
  enters their PIN; AuthorizeOverrideAsync re-verifies PIN + active role
  server-side. Recorded as AdmittedEarlyOverride / AdmittedChildWithAdult
  with OverrideByUserId = the authorizing supervisor. A forged
  childWithAdult/overrideEarly flag alone never admits.
- Retires the shared Gate:SupervisorPin (and its GateLoginThrottle use in
  Decision) in favour of the per-staffer PINs.

Stage 4 — admin enrol/reset + docs + tests:
- /Gate/Admin enrol (AdminSetPinAsync, incl. supervisors) and reset
  (ClearPinAsync) via the human-search picker (admins aren't route-locked).
- Gate.md updated (concepts, data model, routing, invariants, config,
  cross-section deps, freshness triggers). 5 new GateService unit tests.

Also fixes two pre-existing Gate kiosk bugs found during live verification:
- GateSettingsViewModel put [Range] on the record *property*; ASP.NET throws
  at POST-bind time for record types, so saving gate settings 500'd. Moved the
  validation to the constructor parameter ([param:]).
- body.gate-kiosk is a flex column (site.css); .gate-kiosk-main's auto margins
  suppressed the flex stretch and shrink-wrapped every kiosk page to its
  content width. Added width:100% so kiosk pages use the full 960px.

Live-verified end to end (local PG + Playwright): set/verify/blocked claim,
wrong-PIN error + throttle, admin enrol/reset, override tap-list + keypad,
full-width kiosk layout. Build clean; Gate 117 + Web 338 tests green.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- The full-screen claim keypad now fills the viewport and centres (min-height
  floor so a short screen still grows + scrolls, no clip) — no top-heavy gap in
  portrait.
- The supervisor-override scrim uses `justify-content: safe center` so the panel
  sits in the middle and its header clears the (dimmed) topbar, degrading to
  top-aligned + scrollable if a screen is ever too short.

Verified at the M80N's 1280×800 panel in both portrait and landscape.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
CI code-quality (dotnet format --verify-no-changes) flagged the continuation
indentation in GetEnrolledSupervisorIds_ReturnsOnlySupervisorsWithAPin.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…nt validation

Review fixes on top of the Gate admissions feature before it goes to QA:

- F1: add a "Gate settings" link to the Tickets admin nav — /Gate/Admin (the
  general-entry cutoff screen) was orphaned, so the section shipped unreachable
  and every scan fell back to AMBER until the cutoff was set.
- F2: the route-locked kiosk can't reach /api/profiles/search, so the claim
  screen's search box was dead. Add a Gate-scoped /Gate/Search (burner names
  only, inside the /Gate route-lock) and point the picker at it; drop the email
  affordance. Overrides stay tap-list-only.
- S1: throttle gate PIN entry per target user only, not per shared device/IP.
  The device key collapsed to the reverse-proxy IP, so 5 wrong PINs froze every
  claim and override at the terminal for 15 min (gate-wide-lockout DoS). The
  per-user bucket already caps brute force at 5 / 15 min.
- S2: validate the claimant resolves to an active member before enrolling a PIN
  or stamping the session — a direct ClaimPin POST with an arbitrary id could
  otherwise mis-attribute scans to a not-yet-enrolled colleague.

Adds GateControllerClaimTests covering per-user throttling, claimant validation,
and the kiosk search.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
# Conflicts:
#	src/Humans.Application/Interfaces/Tickets/ITicketVendorService.cs
#	src/Humans.Infrastructure/Services/StubTicketVendorService.cs
Replaces the two in-flight Gate migrations (AddGateSection + AddGateStaffPinAndOverride)
with one regenerated migration. origin/main raced ahead with a later-timestamped
migration (AddTicketAttendeeCheckedInAt, 20260630) during this PR's life, leaving the
Gate migrations mid-chain where `dotnet ef migrations remove` is unsafe — so the snapshot
was restored from origin/main as the authorized first step of /ef-regen
(see .claude/skills/ef-regen/SKILL.md), then `dotnet ef migrations add` produced one
consolidated migration at the end of the chain.

Net schema change (Gate section, never deployed to QA/prod — no data loss concern):
- gate_scan_events created (incl. OverrideByUserId audit column + unique AdmitDedupeKey
  index for the single-admit dedupe)
- gate_settings created
- gate_staff_pins created (hashed personal PINs)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@github-actions github-actions Bot added the db PR includes EF Core migration label Jun 30, 2026
@peterdrier peterdrier marked this pull request as ready for review June 30, 2026 09:35
@peterdrier peterdrier merged commit bfc03cc into main Jun 30, 2026
12 of 13 checks passed
@peterdrier peterdrier deleted the feat/gate-admissions branch June 30, 2026 09:36

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 779fb0215a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Found: true,
IsVoid: attendee.Status == TicketAttendeeStatus.Void,
AlreadyAdmittedLocally: priorAdmit is not null,
CheckedInAtVendor: attendee.Status == TicketAttendeeStatus.CheckedIn,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Use CheckedInAt for vendor duplicate detection

When a ticket has already been checked in through TicketTailor and the sync has run, the ticket row records that in TicketAttendee.CheckedInAt while the attendee Status remains Valid (the sync path applies check-ins without changing status). This leaves CheckedInAtVendor false here, so Gate will present the ticket as admissible and can record a second local admit instead of stopping as a duplicate for tickets first processed by the vendor app; the gate projection needs to carry and check the persisted check-in timestamp, not only the status enum.

Useful? React with 👍 / 👎.

await using var ctx = await factory.CreateDbContextAsync(ct);
return await ctx.Set<GateScanEvent>()
.AsNoTracking()
.Where(e => e.GuestUserId == userId || e.ScannedByUserId == userId)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Include override authorizations in user exports

Supervisor overrides store the authorizing supervisor's user id in OverrideByUserId, but this export query only returns rows where the user was the guest or scanner. A supervisor who authorized a too-early or child-without-ID admit therefore gets a DSAR export that omits records containing their identifier; include OverrideByUserId == userId here and distinguish that role in the exported slice.

Useful? React with 👍 / 👎.

profilePictureUrl = m.ProfilePictureUrl,
})
.ToList();
return Json(rows);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WARN — anonymous search response violates memory/code/search-endpoint-response-shape.md

Rule: "search/autocomplete endpoints return typed DTOs/records, not anonymous objects — anonymous JSON payloads drift easily and make it harder to reuse search behavior safely."

The Select(m => new { userId, displayName, detail, profilePictureUrl }) here is an anonymous type. Any JavaScript consumer that depends on these property names has no compile-time contract to catch accidental renames.

Fix: define a named record in GateViewModels.cs and project into it:

Suggested change
return Json(rows);
var rows = matches
.OrderByRelevance()
.Select(m => new GateSearchResult(m.UserId, m.BurnerName, m.ProfilePictureUrl))
.ToList();
return Json(rows);

Add to GateViewModels.cs:

public sealed record GateSearchResult(Guid UserId, string? DisplayName, string? ProfilePictureUrl);

(The detail field was always null, so it can be dropped.)

.SelectMany(s => s.Signups)
.Where(u => u.Status is SignupStatus.Pending or SignupStatus.Confirmed)
.DistinctBy(u => u.UserId)
.OrderBy(u => u.DisplayName, StringComparer.CurrentCultureIgnoreCase)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WARN — display sort in Application service violates memory/architecture/display-sort-in-controllers.md

Rule: "display ordering belongs at the presentation layer (controllers / views / view-model assembly), not in services or repositories." The atom is explicit that .OrderBy(...) for display ordering inside src/Humans.Application/Services/**/*.cs is a layer leak — "sort for display" is not the service's job.

This OrderBy(u => u.DisplayName, ...) is alphabetical for display. It should move to the controller.

Fix in GateService.GetShiftRosterAsync — drop the sort and leave the projected list:

Suggested change
.OrderBy(u => u.DisplayName, StringComparer.CurrentCultureIgnoreCase)
.Select(u => new GateRosterEntry(u.UserId, u.DisplayName))
.ToList();

Fix in GateController.BuildRosterAsync — sort after the service call:

var roster = await gate.GetShiftRosterAsync(teamId, ct);
return roster
    .OrderBy(r => r.DisplayName, StringComparer.CurrentCultureIgnoreCase)
    .Select(r => new GateRosterMember(r.UserId, r.DisplayName))
    .ToList();

@claude

claude Bot commented Jun 30, 2026

Copy link
Copy Markdown

Reviewed commit 779fb02. 2 inline finding(s) posted.

peterdrier added a commit that referenced this pull request Jun 30, 2026
Addresses two review findings on the Gate admissions PRs (#1066,
promoted in nobodies-collective#903):

- /Gate/Search returned anonymous objects, violating
  memory/code/search-endpoint-response-shape.md. Return the existing typed
  HumanLookupSearchResult record — the same shape /api/profiles/search already
  feeds this picker (Detail omitted; the kiosk search is name-only).
- GatePinThrottle's class summary still described the removed dual-key
  (user-id + device/IP) contract. Updated to the current per-target-user-only
  design and the reason the device key was dropped (shared reverse-proxy IP →
  gate-wide DoS lockout).

GateControllerClaimTests.Search now asserts the typed result directly instead of
re-serializing with default (PascalCase) options that don't match the MVC pipeline.

Co-authored-by: Peter Drier <peter@Peters-MacBook-Pro-2.local>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

db PR includes EF Core migration

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants