Skip to content

Feature: stack/2-notifications to dev - #319

Closed
github-actions[bot] wants to merge 7 commits into
devfrom
stack/2-notifications
Closed

Feature: stack/2-notifications to dev#319
github-actions[bot] wants to merge 7 commits into
devfrom
stack/2-notifications

Conversation

@github-actions

@github-actions github-actions Bot commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

Automated PR tracking changes from stack/2-notifications into dev.

aamoghS and others added 6 commits August 5, 2026 12:38
* 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.

---------
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.
…hook 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.
@github-actions github-actions Bot added the dependencies Pull requests that update a dependency file label Aug 9, 2026
@github-actions

github-actions Bot commented Aug 9, 2026

Copy link
Copy Markdown
Contributor Author

Dependency Review

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

Scanned Files

None

@github-actions

github-actions Bot commented Aug 9, 2026

Copy link
Copy Markdown
Contributor Author

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

https://hacklytics2027--pr-319-9imf3yph.web.app

(expires Sun, 16 Aug 2026 00:02:15 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: c48ba34db61581e25fe2978355160b5eefe0e83f

// directive. Flagged by CodeQL as both.
console.error(
"[Stripe webhook] membership grant failed for checkout session",
safeLogId(session.id),
// Same reasoning as the checkout-session branch above.
console.error(
"[Stripe webhook] membership grant failed for payment intent",
safeLogId(pi.id),
@aamoghS

aamoghS commented Aug 9, 2026

Copy link
Copy Markdown
Member

Superseded. This branch's work is already on main (squash-merged via #316 / #318 / #320 / #323 / #325 / #327 / #328), and dev has been brought up to date from main in #315 — so there is nothing left for this tracking PR to carry. Closing to keep the queue readable rather than because the work was dropped.

@aamoghS aamoghS closed this Aug 9, 2026
@aamoghS
aamoghS deleted the stack/2-notifications branch August 9, 2026 03:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

⤵️ pull dependencies Pull requests that update a dependency file

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants