Conversation
Reaching your own stats meant opening an event or the championship and finding yourself in a results table. /me now resolves the viewer to their Driver row and links straight to it. Driver.msrUid has been a unique column since the init migration with no read or write path anywhere; this is what finally uses it, so no migration is needed. Matching falls back to Driver.nameOnlyHash, computed at login from the MSR profile before the full surname is discarded, and only when exactly one Driver carries the hash -- the same rule ingest already uses for merge/adopt. computeNameOnlyHash moves to pii.ts (re-exported from ingest.ts) so the OAuth callback doesn't pull better-sqlite3 into its bundle.
The target is a Route Handler that 302s cross-origin to MSR, so the App Router client fetched an RSC payload for it, failed on the redirect, and fell back to a browser navigation -- running OAuth step 1 twice per click, minting two request tokens and writing lc_msr_req twice. landing.tsx already used a plain <a>.
feat(me): link a signed-in user to their own driver stats
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved critical and moderate issues affect driver privacy and the completeness and access control of /me statistics.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds /me self-driver linking using MSR identity hashes, OAuth-time claiming, and cross-league statistics.
Changes:
- Adds driver resolution, claiming, session hashing, and tests.
- Adds the
/meresults card and fixes OAuth navigation. - Updates product, architecture, PII, and repository documentation.
File summaries
| File | Summary | Review findings |
|---|---|---|
docs/PRD.md |
Updates self-results and PII requirements. | — |
docs/BUILD.md |
Documents the implementation architecture. | — |
apps/web/tests/driver-self.test.ts |
Adds driver resolution and claiming tests. | — |
apps/web/src/lib/session.ts |
Adds the session name hash. | — |
apps/web/src/lib/pii.ts |
Adds the reusable name-hash helper. | — |
apps/web/src/lib/ingest.ts |
Re-exports the hash helper. | — |
apps/web/src/lib/driver-self.ts |
Resolves and claims driver rows. | Critical (3 votes): Fallback resolution can return a row already claimed by another user, exposing that user's stats. |
apps/web/src/app/me/page.tsx |
Renders self-results and statistics links. | Moderate (3 votes): The full-stats link can omit leagues when no default league exists. Moderate (2 votes): Aggregation can include statistics from leagues inaccessible to the session. |
apps/web/src/app/login/page.tsx |
Uses browser navigation for OAuth. | — |
apps/web/src/app/api/auth/msr/callback/route.ts |
Computes the hash and claims drivers during login. | — |
AGENTS.md |
Updates repository architecture guidance. | — |
Review details
Suppressed comments (1)
apps/web/src/lib/driver-self.ts:110
- Filtering on
msrUid: nullbefore applying the exactly-one check makes ambiguity depend on ownership. With two same-name Driver rows where one is already linked to another user, this sees only the unclaimed row and claims it, violating the no-guess rule and potentially assigning the other user's stats. Count all rows for the hash, then require the sole row to be unclaimed.
where: { nameOnlyHash, msrUid: null },
select: { id: true },
take: 2,
});
if (candidates.length !== 1) return;
- Files reviewed: 11/11 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| where: { nameOnlyHash: session.nameOnlyHash }, | ||
| select: { id: true, firstName: true, lastInitial: true }, | ||
| take: 2, | ||
| }); | ||
| if (candidates.length !== 1) return { status: "unmatched" }; |
There was a problem hiding this comment.
Confirmed and fixed in 65070b0. I reproduced it as a failing test before changing anything: with a single row carrying hash H already claimed by U2, resolveSelfDriver({ msrUid: "U1", nameOnlyHash: H }) returned linked to U2's row.
The root cause is one you named precisely in the suppressed comment on claimSelfDriver, and it applies to both functions: counting candidates over the unclaimed subset makes ambiguity depend on ownership rather than on the data. Both queries now count every row carrying the hash and require the sole row to be unclaimed. A row claimed by this user can't reach that branch, since the findUnique on msrUid above it already returned.
Two regression tests added: a sole row owned by another user, and a claimed row plus an unclaimed row sharing one hash (which previously claimed the unclaimed one on a genuinely ambiguous name).
| if (leagueSlugs.length > 0 && !leagueSlugs.includes(defaultLeagueSlug)) { | ||
| return `/l/${leagueSlugs[0]}/drivers/${driverId}`; |
There was a problem hiding this comment.
Agreed, fixed in 65070b0. The inconsistency was real: the card aggregated every league while the link could only reach one of them.
Resolved together with the access-scoping comment below. The card now resolves the driver's leagues through checkLeagueAccess and, when the default league is not among the accessible ones, pins both the summary and the link to the same single league — so the count never includes events the destination won't show. There is no route that spans an arbitrary subset of leagues (/drivers/[id] is the only cross-league view and it gates on the default league), so narrowing the summary is the honest option rather than promising a destination that doesn't exist.
| const [driverSeasons, history, defaultLeague] = await Promise.all([ | ||
| listSeasonsForDriver(self.driverId), | ||
| buildDriverHistory(self.driverId, { leagueIds: "all" }), | ||
| getLeagueConfig(), |
There was a problem hiding this comment.
Agreed, fixed in 65070b0 — and it was reachable sooner than the comment suggests. On this deployment the only league with accessGate: "required" is the default one (pca-rmr; rmsolo is optional), so the case that actually bites today is a driver with default-league results who is not an admitted member: the card showed their event count and best finish, and the link then redirected them.
/me now resolves each of the driver's leagues through checkLeagueAccess and passes only the allowed ids to buildDriverHistory, instead of leagueIds: "all". Cost is low — checkLeagueAccess short-circuits to allow for non-required gates with no session or DB read. When nothing is accessible the card says so rather than rendering a zeroed summary.
Worth noting separately, and deliberately out of scope here: /drivers/[id]?league=all has the same shape and predates this PR. It gates on the default league but then renders every league's rows, so any default-league member can already see another league's data for a driver. Harmless on the current deployment because the only non-default league is public, but it's the same bug one level up and should be fixed on its own.
Both lookups counted candidates over the unclaimed subset, which made ambiguity depend on ownership rather than on the data. Two consequences, both reproduced as tests before the fix: - A sole row already claimed by another user resolved as *this* viewer's, showing them someone else's stats. Two humans sharing a full name is all it took. - A claimed row plus an unclaimed row sharing one name looked like a single confident candidate, so the claim fired on a genuinely ambiguous hash. Both queries now count every row carrying the hash and require the sole row to be unclaimed. Also scope the /me card to leagues the session can actually open. /me has no league gate of its own, so it was summarizing events from a league whose own stats route would redirect the viewer -- reachable today, since the gated league is the default one. When the default league is out of reach there is no multi-league destination, so the summary pins to the single league it links to instead of counting events the link won't show. Reported by Copilot on #176.
|
Filed the pre-existing |
Promotes #175 from
developtomain. Already merged to develop and being smoke-tested against the Vercel preview at staging.launchcontrol.club.What this ships
Reaching your own stats meant opening an event or the championship and finding yourself in a results table.
/menow resolves the signed-in viewer to theirDriverrow and links straight to it. Addresses driver feedback.Driver.msrUidhas been a unique column since theinitmigration with no read or write path anywhere in the codebase, so this needed no migration. Resolution prefers that explicit link, then falls back toDriver.nameOnlyHashand only matches when exactly oneDrivercarries the hash, the same "exactly one candidate or don't guess" rule ingest already uses for merge/adopt. The join key exists because the OAuth callback computes the digest from the MSR profile while it still has the full surname, then discards it; the hash goes in the session, the surname is still never persisted.computeNameOnlyHashmoved fromingest.tstopii.ts(re-exported, so existing importers are untouched) becauseingest.tstop-level-importsbetter-sqlite3and the OAuth callback should not pull a native SQLite driver into its bundle.Reads and writes are split deliberately:
resolveSelfDriveris pure read so rendering/mecarries no side effect, and the single write isclaimSelfDriver, called from the callback and wrapped so it can never fail a login. Cross-league aggregation needed no new code:buildDriverHistoryalready acceptsleagueIds: "all".The second commit is unrelated to the feature but was found while smoke-testing it: the
/loginsign-in button was anext/linkpointing at a Route Handler that 302s cross-origin to MSR, so the App Router client fetched an RSC payload for it, failed on the redirect, and fell back to a browser navigation, running OAuth step 1 twice per click and minting two request tokens.landing.tsxalready used a plain anchor.Reviewer notes
The PII rule in
docs/PRD.mdpreviously said the full last name is used only transiently to compute the identity hash. It is now also used at login to compute the full-name hash, so the wording was widened and the line drawn explicitly: storing a one-way digest is in scope, storing the surname is not. That is a policy change rather than a code comment and is the part of this PR most worth explicit sign-off.Verification
lint,typecheck,test(728 passing across 62 files) andbuildall clean. Newtests/driver-self.test.tscovers 12 cases across both functions, including an ambiguous hash, a row already claimed by another user, and a legacy row with a nullnameOnlyHash.Smoke-tested locally against real dev data: MSR login,
/merenders the results card, link lands on the right driver page. Not yet verified anywhere: theunmatchedandunlinkableempty states have unit coverage only and have never been rendered in a browser, and the new card has not been checked at mobile and desktop breakpoints.