Problem
-------
1. Re-clicking an already-confirmed confirmation link (e.g. /confirm?token=…)
returned from confirmSubscription because the SQL WHERE
clause required . The caller then threw an
ActionError('invalid or expired') which rendered the 'Email not confirmed'
error page — misleading for someone who had already confirmed.
2. A second PUT /subscription/email with the same email+frequency could
silently bypass the upsert path when getSubscriptionByPubkey found the
active row but updateSubscription returned it unchanged (email and
frequency matched). While the unique index prevented a true duplicate
INSERT, the code path was fragile and the regression test was missing.
Changes
-------
database.ts:
- confirmSubscription now returns { sub, alreadyConfirmed } | undefined.
First it tries the existing UPDATE (unconfirmed tokens only). If that
returns no rows, it looks up the key directly: if the row exists and is
already confirmed, returns { sub, alreadyConfirmed: true }. If the row
doesn't exist or is unsubscribed, returns undefined (invalid/expired).
- Exported new ConfirmResult type for callers.
actions.ts:
- confirmSubscriptionAction destructures the new return type.
- Only registers the cron job on fresh confirmation (not re-confirms).
- Returns the ConfirmResult so the route can distinguish the two cases.
server.ts:
- /confirm route checks result.alreadyConfirmed and renders
confirm-already.html instead of confirm-success.html.
pages/confirm-already.html:
- New page with title 'Email already confirmed' and an info message
explaining the address was already confirmed.
Tests:
- test/confirm-already-confirmed.test.ts — NEW (3 tests): first confirm
succeeds with alreadyConfirmed=false; second confirm returns
alreadyConfirmed=true; nonexistent token returns undefined.
- test/duplicate-subscription.test.ts — NEW (4 tests): full cycle of
register → confirm → re-register → assert one active row with
unchanged key, verifying the upsert is idempotent.
- Adapted 3 existing test files to destructure the new ConfirmResult.
Bug: runJob computed since = sub.last_digest_at, fetched events with
received_at > since, sent the digest (slow), then deleted ALL events
with received_at > since. Any /notify event that arrived between the
fetch and the delete was also received_at > since, so it was deleted
without ever being sent in a digest.
Two changes:
1. Delete by exact event IDs (src/database.ts, src/worker/email.ts):
Added deleteEventsByIds(subscriptionId, eventIds) which deletes
only the events that were actually fetched + sent. The old
timestamp-based delete is retained but no longer called from runJob.
2. Re-fetch subscription on each cron tick (src/worker/email.ts):
createJob's closure captured the original sub, so sub.last_digest_at
stayed stale in memory. Every subsequent tick recomputed since from
the old value, re-fetching and re-sending duplicate events. Now each
tick re-fetches the subscription from the DB via getSubscriptionById
before calling runJob.
Fixes bead mailship-091
Switch the subscribe endpoint from POST to an idempotent PUT so clients can
always upsert without a stateful lookup first.
- Add GET /subscription/email?pubkey= lookup so clients can avoid re-POSTing.
- insertSubscription no longer clears confirmed_at unless the email address
changes; a frequency change keeps the existing confirmation.
- registerSubscription only sends a confirmation email when the subscription
is new, unconfirmed, or the email address changed.
- Update integration test and README for the PUT endpoint.
- test/integration.sh: runs full E2E flow against fresh server
- pnpm test and pnpm test:server scripts added
- Root endpoint handles missing web UI gracefully (returns JSON)
- database.ts reads DATA_DIR env var for test isolation
- .gitignore updated for test artifacts