Feature: chore/drop-dangling-export to dev - #329
Closed
github-actions[bot] wants to merge 11 commits into
Closed
Conversation
* adding * feat(club): decouple membership from the hackathon edition A club membership was welded to a hackathon edition: `member` was `unique(userId, hackathonId)` NOT NULL and every read resolved through `resolveCurrentHackathonId`. The day next year's edition flipped to `open`, every paying member silently became a non-member — the portal showed the pay button, club check-in threw at the door, initiatives locked out, and re-paying discarded whatever months were left. Membership is now annual and belongs to a person: `unique(userId)`, and the edition clause is gone from every read site (registration, member, events, initiative, portal-context, verify-email). `membership_history` is finally written on join and renewal, because `hackathonId` had been doing accidental duty as the only record of which year somebody was a member. Also here: - The Stripe payment row commits before any membership work. The two shared a transaction, so a grant that threw rolled back the payment record as well — the customer was charged and nothing anywhere recorded it, and none of the recovery paths could help because they look for a row that was never written. - Mock mode grants a real membership through the production confirm path, so the club half can be developed without a Stripe key. The old test asserted a returned shape, which is how "Access Granted with nothing written" survived. - verify-email calls the shared membership service instead of its own third copy, which restarted the term from today and wrote no history. The database change is deliberately NOT applied by `drizzle-kit push` — push offers to truncate when adding the unique constraint. Apply packages/db/ddl/2026-08-08-membership-decouple.sql by hand; it back-fills membership_history before dropping the column it replaces. Verified: typecheck, 373 tests, lint --max-warnings 0, build. * fix(stripe): recover a payment whose membership grant failed Both points raised by Greptile on #316. **A linked payment could never be retried.** The webhook records the payment first and grants the membership after — deliberately, because sharing a transaction meant a failed grant rolled the payment row back and lost the charge entirely. But that ordering leaves a real state: payment row linked to the user, no membership. Every recovery path skipped already-linked payments (`if (existing.linkedUserId) continue;`), so that state was permanent — the customer was charged, the payment was on file, and nothing ever retried. reconcileMyPayments now treats "linked" as "not proof of a grant" and checks membership_history instead. Every grant writes a history row, so a payment with no history row at or after its own timestamp was never honoured. That also distinguishes a failed grant from a membership that was granted a year ago and has since lapsed, which must not be silently renewed off an old payment. Both directions are tested, and the guard was mutation-tested. **The DDL file overstated what it covers.** It handles only the change drizzle-kit push cannot be trusted with (the unique constraint, where push offers to truncate), but the same release also changes the judging tables. The file now says so and gives the order: apply it, run migrate:push for the rest, then push must report "No changes detected". Verified: typecheck, 375 tests, lint --max-warnings 0, build. ---------
…live funnel (#318) * feat(hackathon): close the notification loop Four things the product collected data for and then never used. **The interest list is finally told.** It exists for exactly one moment — registration opening — and nothing sent it, so the runbook told organisers to hand-compose an announcement instead. Registration Controls now offers "Email N interested" the moment an edition is open, 500 at a time, stamping hackathon_interest.registration_open_email_sent_at as each one goes out. A closed tab resumes rather than re-sending. **Announcements survive a closed tab.** The send loop runs in an organiser's browser and walked the audience by offset across separate requests, re-resolving it each time — so any row that moved between batches shifted the window, and a half-finished send could only be repeated from the start. Composing now freezes the message and its audience into rows (hackathon_announcement and hackathon_announcement_recipient), and each recipient is marked as their message leaves. Unfinished sends are listed and resumable, and "exactly once" is a unique constraint rather than arithmetic in a browser. **/hacklytics survives registration opening.** getUpcoming filtered to `announced` only, so the single public entrance to the hackathon — the 2027 site's only CTA and the navbar both land there — went blank with "Nothing announced yet" at the exact moment it mattered. It now covers announced, open and in_progress, and swaps the interest form for a register CTA. **/submit says whether it is open.** The page never queried the window it was gated on, so an attendee could write a full description and learn it was refused only on submit. Both now share computeSubmissionWindow. Also: one email template layer. Every message is {subject, heading, paragraphs[], cta} with the text always escaped and a plain-text alternative built from the same content; the acceptance email was folded onto it from its own hand-written HTML and its own escaping. Database: packages/db/ddl/2026-08-08-notifications.sql. Additive only — safe to apply before this deploys. Verified: typecheck, 391 tests, lint --max-warnings 0, build. * fix(notifications): claim recipients before sending, and sanitise webhook logs Greptile on #318, plus the CodeQL alerts on main. **Overlapping batches could mail the same person twice.** Both send loops selected rows with no sent marker and wrote the marker afterwards, so two requests — two organisers, or one impatient double-click — read the same rows and both sent to them. Each loop now claims its batch with a single atomic UPDATE and works only from what it won; the loser gets a smaller batch instead of a duplicate delivery. A claim older than fifteen minutes is reclaimable, so a request killed by a deploy or a timeout does not strand its recipients and make the send permanently unfinishable. **The interest send reported itself finished while recipients were pending.** `done` was inferred from the batch size, so a batch where some addresses failed came back done with those people never mailed. Failures are now marked — left pending, a permanently bad address is retried on every batch and the send can never finish — and `done` is counted from the table rather than inferred. **Externally-controlled format string / log injection** (CodeQL, 2 high + 2 medium on main). The Stripe webhook interpolated ids from the request body into `console.error` messages. The mock branch parses that body without verifying a signature, so those values are not always Stripe's: a newline forges log entries and a `%s` is read as a format directive. Ids now go through `safeLogId` and are passed as arguments, never as the message. Verified: typecheck, 391 tests, lint --max-warnings 0, build. The claim guard was mutation-tested.
* feat(judging): make judging work without an organiser improvising **Approving a judge builds their queue.** `setActive` flipped a flag and created nothing; `judge.register` always writes an assignment row and `assignToHackathon` refuses anyone who already has one — so the documented recruitment path produced an active judge whose portal said "All Done" having judged nothing. The only remedy was `assignJudgesToProjects`, which deletes and rebuilds every queue in the event and is unsafe once judging is live. Approval now builds the queue itself, and only when the queue is empty: a judge suspended mid-event and reinstated comes back to the queue they were part-way through. **Late submissions reach a judge.** `promoteSubmissions` returned `queuesNeedRebuild` and nothing consumed it beyond a banner, so a project promoted after assignment sat in nobody's queue, received zero votes, and was then dropped from the standings entirely by the zero-vote rule — the team simply absent. Promotion now appends those projects to the queues that already exist, at the coverage the existing queues give a project, without reordering a single row anyone is part-way through. The banner now means only "these projects reached no judge", which is a track mismatch a human has to resolve. **A judge's track has to exist.** Free text went straight into the routing column, and any string not on the edition classified them as sponsor/special and filtered their pool to zero — invisible to them and to the organiser, and uncorrectable because assignToHackathon refuses a second assignment. Both entry points now validate against the edition's own tracks and challenges, both pickers offer them, and `updateAssignmentTrack` can fix a wrong one (rebuilding the queue unless the judge has already scored). **Recruitment stays open when registration closes.** `closed` — the natural step before recruiting judges — emptied /judge/register with no explanation. Also: judges are told they were approved (the success screen promised it), applicants get a pending state where the apply button used to sit and throw, and results can be announced from the same screen that publishes them. Verified: typecheck, 405 tests, lint --max-warnings 0, build. * fix(judging): serialise queue creation, refuse a track change that has scores Both points from Greptile on #320. **Two approvals could build the queue twice.** "Is the queue empty? then build it" is a read followed by a write, and judge_queue has no unique on (judge, project) to fall back on — so a double-click, or two organisers working the same list, both saw an empty queue and both built one, handing the judge every project twice. The check and the build now happen in one transaction behind a lock on the judge row: the second request sees what the first wrote and does nothing. **Changing a track after scoring left the queue disagreeing with it.** The track was written and the rebuild skipped, so the judge carried on scoring the pool they were routed to before with nothing on screen saying so. It now refuses, names how many projects they have already scored, and offers an explicit override — the pattern assignJudgesToProjects already uses. Forcing keeps every completed slot (skipProject marks one done without writing a vote, so they cannot be rebuilt from the votes table) and rebuilds only the remainder, numbering the new slots after the kept ones. Both guards are mutation-tested: removing the lock fails the lock test, and removing the refusal fails two. Verified: typecheck, 410 tests, lint --max-warnings 0, build.
* feat(admin): staff, memberships, audit log and the club event surfaces Six procedures that existed with no screen behind them, and one page that lied. **Staff and roles.** `admin.create`/`update`/`list` had no caller, so the only way to grant the volunteer tier was an INSERT against production — while /scan's own rejection screen told people to "ask an organiser to add you as event staff". /admin/staff finds somebody by their exact sign-in email (exact, not a prefix search: this is the lookup that precedes handing out a role, and a partial-match list is how the wrong Alex gets scanner access) and grants, changes or deactivates it. **Memberships.** A cash payer at a table, a comped officer, a refund that has to be honoured — none come through Stripe and none had any path but SQL. /admin/members searches, grants, extends, shortens and ends. Months are added to whatever term is left rather than restarting it, so comping somebody mid-year does not silently shorten them. Every write records a membership_history row with the typed reason and an audit entry at critical — handing out a paid membership for free is exactly the action a record needs to exist for. **The audit log is readable.** `audit.list` existed with no screen while retention prunes routine rows at 90 days, so the evidence expired before anyone could look at it. **The interest list is readable.** The four questions the public form collects were shown to no organiser at all. **The member profile.** The columns, `member.register`/`update` and SkillsInterestsInput all existed with nothing calling any of them. A Membership tab on /settings writes them and shows status and history. **/events told the truth.** It rendered a hardcoded "No upcoming events scheduled" no matter what was in the database, because `events.list` had no caller anywhere — club events existed only for whoever was standing in front of the QR code. It is now read server-side (the tRPC provider is mounted only inside the portal route group, and this page's whole audience is signed-out). Also: the club event form can set capacity, which the schema, the row lock and the "Event is full" gate have always supported and no screen could reach. Verified: typecheck, 420 tests, lint --max-warnings 0, build. * fix(admin): make membership writes atomic and profile fields clearable Three points from Greptile on #323. **A membership could be changed with no record of why.** The member update and its membership_history row were separate statements, so a failure between them left a moved term and nothing saying who moved it or on what grounds — and the history is now the only record of which years somebody was a member. Both writes are one transaction. **Two staff extending the same person lost one of the grants.** The new term was computed from a row read outside any lock, so both read the same end date, both wrote, and the second silently overwrote the first — twelve months paid for and gone. The row is now read with SELECT … FOR UPDATE inside the transaction that writes it. **An emptied profile field came straight back.** `undefined` was the only "not set" value the form could send, so clearing a field sent nothing, the server skipped the column, and the next read restored the old value — a save that reported success and changed nothing. Optional fields are nullable now, null means clear, and the form sends it. `""` is not usable for this: those fields validate as URLs and as min-length strings, so an empty string is a validation error rather than a clear. The lock is mutation-tested — removing `.for("update")` fails the test — which needed the file's select mock upgraded to the traced chain judge-edge already uses, since a chain of fixed stubs cannot show whether a lock was taken. Verified: typecheck, 422 tests, lint --max-warnings 0, build.
* chore: cleanup, and the decisions that were left open **The second sanitizer is gone.** `sanitizeInput` stripped markup, truncated every string at 10,000 characters and rejected any prose containing SQL keywords — "select a track from the list" among them. Nothing called it: the request path runs `scrubMarkup`, which refuses executable markup and passes everything else through byte for byte. Only the tests called it, so the suite was describing behaviour the product did not have. They now target the real one. **`publicProcedure` requires a database.** Without it a public query with no connection fell through to its own `if (!db)` branch, which returns null — and the funnel page renders that as "nothing announced yet". A misconfigured deploy looked like an empty product instead of a broken one. **Cache invalidation actually evicts.** `deletePattern` anchors with `$`, so the wildcard-free entries matched no real key — and because the map HAS an entry, the namespace fallback was skipped, so nothing at all was evicted. Also adds the caller's own keys to events.checkIn, so a member who just scanned sees the change. **Security logging goes to stderr**, not to a file under the source tree that does not exist in the container, where the ENOENT was swallowed by an empty callback — the one message saying security logging had stopped was itself silently dropped. Dead exports removed with it. Decisions, all recorded in PLAN.md §5: - **D2** — registration is an application. `approved` to form a team, `checked_in` to submit: acceptance is a decision made weeks earlier and says nothing about whether somebody turned up, and judging is in person against a table number. Every participant is created `pending` and `pending` used to pass, so teams formed and projects were submitted with no review at all. - **D4** — points removed. Every club check-in carried the schema default and the attendee CSV exported that constant as a "Points" column: a fabricated number presented as data. - **D5** — a `not_accepted` audience, excluded from every other one so it can only be sent on purpose, with a drafted message prefilled into an untouched form. - **D7** — the security headers ship report-only until CSP_ENFORCE=true, with /api/csp-report collecting violations; the 238-line module nothing imported is deleted. `camera` is deliberately absent from Permissions-Policy: /scan reads QR codes through getUserMedia. - **D9** — /docs is linked from the admin nav. - **W22** — initiative decisions are emailed. People applied and heard nothing. Database: packages/db/ddl/2026-08-08-drop-points.sql (D4). Verified: typecheck, 420 tests, lint --max-warnings 0, build. * fix: gate check-in on acceptance, and cap CSP reports before buffering Both points from Greptile on #325. **`checked_in` was reachable from any status.** D2 makes it an authorisation state — submitting a project requires it — so setting it on somebody still pending hands them the whole event with no review having happened. The attendees screen offers "Select all N matching", so one wrong click could do that to every applicant at once. The single-participant path now refuses and names the remedy: accept them first, then check them in. The batch path filters in the WHERE instead of refusing whole, because a 2000-row selection with a few unreviewed applicants in it should still admit everybody else — and it now reports how many rows it skipped, so a check-in run that quietly did less than it looked like cannot pass for a complete one. **The CSP report cap was documentation, not a limit.** `await request.text()` buffers the entire request before the length check runs, so the endpoint had already accepted whatever was sent. Content-Length is checked first, and an absent or unparseable one is refused: this endpoint is unauthenticated and a browser sending a violation report always declares a length. The post-read check stays, because Content-Length is the sender's claim rather than the truth. The transition guard is mutation-tested. Verified: typecheck, 423 tests, lint --max-warnings 0, build.
* fix(hackathon): make check-in reachable, or nothing can be submitted #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. * fix(hackathon): make the scan promotion atomic and conditional 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.
#325 deleted packages/api/src/middleware/http-security.ts and left its subpath in the package exports map, so `@query/api/middleware/http-security` — the specifier the manifest says is valid — resolves to a file that is not there. Nothing imports it, so nothing is broken today; this only removes the trap. Three entries were already dangling before this: ./middleware, ./client and ./trpc-server all point at files that do not exist. Left alone deliberately — they predate this work and removing them is a separate decision — but they are worth a look. Verified: typecheck, 426 tests, lint --max-warnings 0, build.
Member
|
Superseded. This branch's work is already on |
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.
Automated PR tracking changes from
chore/drop-dangling-exportintodev.