fix(jobs): post-merge security fixes + ROADMAP update#19
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (15)
📝 WalkthroughWalkthroughThe PR implements reminder execution logic fixes: recipient reminders now short-circuit when disabled, owner digest emails are constrained to pending documents filtered by team/user scope, email subjects use Lingui's Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~14 minutes Possibly related PRs
Suggested labels
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Three blocking issues found in post-merge security review: - Recipient handler: added reminderEnabled gate so late-arriving jobs respect owner preference changes that occur after sweep queues the job - Digest handler: filter all envelopes by ownerReminderDigest (was checking firstEnvelope only — non-deterministic for mixed batches) - Digest handler: scope findMany to teamId + userId + PENDING status so completed documents cannot appear in digest email - Digest handler: replace JS ternary pluralization with Lingui plural() macro for correct i18n across all supported locales - google.ts: suppress pre-existing upstream TS2353 on apiKey property removed from GoogleVertexProviderSettings in current SDK version Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
34e9cf5 to
8cbc6d1
Compare
Summary
google.ts(pre-existing bug, unrelated to reminder feature)Security fixes (reminder email handlers)
BLOCKING-1 —
send-recipient-reminder-email.handler.tsAdded
reminderEnabledgate after thestatus !== PENDINGcheck. Without this, a job queued by the sweep could still fire after the document owner disables reminders, since the sweep check and handler execution are not atomic.BLOCKING-2 —
send-owner-reminder-digest-email.handler.tsReplaced single-envelope
ownerReminderDigestcheck with a filter across all envelopes in the batch. Previously, the check usedfirstEnvelope.documentMetaonly — non-deterministic for batches where documents have different email settings.BLOCKING-3 —
send-owner-reminder-digest-email.handler.tsAdded
status: DocumentStatus.PENDING,teamId, anduserIdscoping to thefindManyquery. Without the status filter, completed documents could appear in the digest email if they were completed between sweep time and handler execution.MEDIUM-2 —
send-owner-reminder-digest-email.handler.tsReplaced JavaScript ternary pluralization with Lingui
plural()macro in the email subject. Ternary-based pluralization breaks locales with more than two plural forms (Polish, Russian, Arabic, Czech).Test plan
tsc --noEmiton packages/lib)🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Tests