Add reuse discipline and PR surface reporting#736
Merged
Conversation
peterdrier
added
enhancement
Owner
Author
PR Surface ReportCompared LOC
Classification notes: EF MigrationsStatus: OK (0 real migration file(s); max 1 per PR) No EF migration files changed. New Files
Surface InventoryPublic types
Interface methods
Service/repository methods
Routes/actions
DI registrations
Package references
Changed Files13 file(s) changed. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c5378f17c5
ℹ️ 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".
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
|
Reviewed commit c5378f1. 1 inline finding posted. |
|
The preview deployment for humans-qa failed. 🔴 Open Build Logs | Open Application Logs Last updated at: 2026-05-23 21:34:53 CET |
3 tasks
peterdrier
added a commit
that referenced
this pull request
Jun 30, 2026
* feat(gate): add Gate admissions section (Phase 1a backend)
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>
* feat(gate): full Gate admissions feature — UI, GDPR/merge/retention, 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>
* fix(gate): post-review cleanup — GateAdmit policy, vendor flag, fail-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>
* fix(gate): correct vendor check-in payload field (check_in_at) + record 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>
* feat(gate): pre-fill claim screen with the gate-shift roster
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>
* fix(gate): inline cutoff-warning banner link; window roster to shifts 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>
* fix(gate): apply design-review fixes to the terminal UI (sunlight/glanceability)
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>
* fix(gate): chromeless kiosk layout for the tablet (match the spec)
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>
* feat(gate): lock the kiosk account to /Gate; make the freshness line 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>
* fix(gate): land kiosk login on /Gate; cache-bust the gate.js module import
- 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>
* feat(gate): data model for personal staff PINs + attributable supervisor 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>
* feat(gate): hardened PIN service layer (per security peer review)
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>
* feat(gate): hardened PIN brute-force throttle + keypad view models
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>
* feat(gate): personal-PIN claim keypad + attributable supervisor override (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>
* style(gate): centre the PIN keypad and override panel vertically
- 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>
* style(gate): satisfy dotnet format (whitespace in new PIN test)
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>
* fix(gate): nav link, kiosk name-search, per-user PIN throttle, claimant 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>
* migrations: consolidate Gate in-flight stack into single AddGateSection
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>
---------
Co-authored-by: veryaaron <mail@aaronross.com>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: Peter Drier <peter@Peters-MacBook-Pro-2.local>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Adds repo-level reuse discipline for agents and PR authors, plus a deterministic PR surface report that comments when a draft PR is marked ready for review.
Why
Agent-generated changes have been adding durable surface too readily. This makes reuse-first behavior explicit in project rules, tightens autonomous prompts, and gives reviewers a mechanical surface/LOC report.
Existing surface checked
CLAUDE.md+memory/atom pattern for durable rules..claude/skills/*convention for the reuse-review gate.actions/github-scriptcomment-update pattern instead of adding app/runtime code.UI changes / screenshots
None.
Checklist
mainonpeterdrier/Humans(the QA fork). No direct commits tomain?memory/process/no-direct-to-main.md.origin/main, notupstream/main.memory/process/reuse-first-change-discipline.mdwithmemory/INDEX.mdentry.Reviewer notes
The PR Surface Report workflow uses
pull_request_targetand checks out the base branch, then fetches the PR head only forgit diff. It does not execute PR code. It posts or updates a single bot comment marked with<!-- pr-surface-report -->and fails if more than one real EF migration file is changed.Test plan
python -B scripts/pr-surface-report.py --base origin/main --head HEAD --output local-pr-surface-report.md --json-output local-pr-surface-report.jsongit diff --check origin/main...HEADdotnet build Humans.slnx -v quiet? passed with existing analyzer/obsolete warnings.dotnet test Humans.slnx -v quiet --filter "FullyQualifiedName!~Integration"dotnet format Humans.slnx --verify-no-changes --verbosity diagnostic --exclude src/Humans.Infrastructure/Migrations/Full
dotnet test Humans.slnx -v quietwas also attempted. It failed only in two existing Store integration tests (Pay_with_valid_amount_redirects_to_Stripe_session_urlandPay_with_amount_over_balance_redirects_back_to_order_without_calling_Stripe) becauseGET /Store/Order/{id}returned 302 instead of 200 while harvesting antiforgery tokens. This PR does not touch app runtime or Store code.