From 027c2aac87517f992bab65681377e1c299b55460 Mon Sep 17 00:00:00 2001 From: Aaron Breckenridge Date: Thu, 10 Sep 2026 09:33:21 -0500 Subject: [PATCH] Add email-deliverability (MX lookup) rule to review prompts Reviewers must now flag any code that accepts an email address without checking the domain's MX records first, so the address is deliverable. Sole exception: logging-only collection where no message will be sent. --- prompts/_shared/email-deliverability-rules.md | 25 +++++++++++++++++++ prompts/claude-synthesize-thesis-first.md | 15 ++++++----- prompts/claude-synthesize.md | 15 ++++++----- prompts/codex-first-pass.md | 15 ++++++----- 4 files changed, 52 insertions(+), 18 deletions(-) create mode 100644 prompts/_shared/email-deliverability-rules.md diff --git a/prompts/_shared/email-deliverability-rules.md b/prompts/_shared/email-deliverability-rules.md new file mode 100644 index 0000000..bcb188d --- /dev/null +++ b/prompts/_shared/email-deliverability-rules.md @@ -0,0 +1,25 @@ +When the diff lets an email address be added — a form field, an API endpoint, an +admin/console path, an import, a mailing list signup, or any other place that accepts +an address for later use — the code must verify the address is deliverable with an MX +record lookup on the address's domain before the address is accepted (or before anything +is queued that will later send to it). Treat accepting an email address without that +check as a blocking finding, and flag it with the file/line and the suggested check. + +- MX lookup: query the domain's MX records (e.g. Ruby's `Resolv::DNS`, or whatever + resolver the app already uses) and reject the address when the domain has no MX record + — that domain runs no mail server, so no message to it can ever be delivered. An MX + lookup only checks that the domain is set up to receive mail; it must not block on the + mailbox existing and must not send a test message to the address. +- Don't block on resolver choice: the point is that the domain's mail servers were + actually checked before the address is accepted. Whether the lookup lives in the model + (validation), a service object, or a job matters less than that it happens at all. +- Watch for lookups that never run: a check that only runs on a happy path, is gated + behind a flag that defaults off, swallows errors into a nil-safe `rescue` that accepts + the address anyway, or verifies format only (a regex for `name@domain`) does NOT satisfy + this rule. If the add succeeds when the lookup fails or is skipped, flag it. + +The ONE exception is logging-only collection: when the code records an address solely +because it observed that the address exists (e.g. stashing an email surfaced in a third +party's payload, or noting it for the record) and nothing in the change — or in any code +that will consume what this change stores — intends to send an actual message to that +address, the MX lookup is not required, and don't flag the absence of one. diff --git a/prompts/claude-synthesize-thesis-first.md b/prompts/claude-synthesize-thesis-first.md index 240c35b..e5fd64e 100644 --- a/prompts/claude-synthesize-thesis-first.md +++ b/prompts/claude-synthesize-thesis-first.md @@ -28,23 +28,26 @@ Synthesize a single review decision for pull request #{{PR}}. 6. Enforce these BiggerPockets member-privacy rules and treat a genuine violation as a blocking issue: {{@prompts/_shared/privacy-rules.md}} -7. Check for batch/task performance hot spots per these rules — an N+1 or O(n^2)+ +7. Enforce the email-deliverability rules and treat a genuine violation as a + blocking issue: + {{@prompts/_shared/email-deliverability-rules.md}} +8. Check for batch/task performance hot spots per these rules — an N+1 or O(n^2)+ pattern on a full-table #process/#collection is a genuine finding, not a nitpick; flag it with a suggested fix: {{@prompts/_shared/perf-rules.md}} -8. Check how the diff parses structured values, per these rules: +9. Check how the diff parses structured values, per these rules: {{@prompts/_shared/parsing-rules.md}} -9. Check that in-app navigational links use React Router's Link rather than a raw `` +10. Check that in-app navigational links use React Router's Link rather than a raw `` tag, per these rules: {{@prompts/_shared/navigation-rules.md}} -10. Check whether the diff renames, moves, or deletes a name that is persisted outside +11. Check whether the diff renames, moves, or deletes a name that is persisted outside the codebase and read back after deploy, per these rules: {{@prompts/_shared/rename-compatibility-rules.md}} -11. Validate which of Codex's findings are real (discard false positives), add any genuine +12. Validate which of Codex's findings are real (discard false positives), add any genuine issues Codex missed, and (when a ticket is available) judge genuine misses of the ticket's intent or clear scope creep — but give credit when the author went beyond the literal acceptance criteria in a sound way rather than flagging it as non-compliant. -12. Decide ONE verdict. Be pragmatic: use "request_changes" only when there is at least one +13. Decide ONE verdict. Be pragmatic: use "request_changes" only when there is at least one genuine, blocking issue (such as a bug, regression, privacy violation, half-finished task, placeholder, or deferred work); otherwise "approve". A change that exceeds the AC without breaking the ticket's intent is a reason to approve, not to block. diff --git a/prompts/claude-synthesize.md b/prompts/claude-synthesize.md index 8f69f76..1a7e41a 100644 --- a/prompts/claude-synthesize.md +++ b/prompts/claude-synthesize.md @@ -28,23 +28,26 @@ Synthesize a single review decision for pull request #{{PR}}. 6. Enforce these BiggerPockets member-privacy rules and treat a genuine violation as a blocking issue: {{@prompts/_shared/privacy-rules.md}} -7. Check for batch/task performance hot spots per these rules — an N+1 or O(n^2)+ +7. Enforce the email-deliverability rules and treat a genuine violation as a + blocking issue: + {{@prompts/_shared/email-deliverability-rules.md}} +8. Check for batch/task performance hot spots per these rules — an N+1 or O(n^2)+ pattern on a full-table #process/#collection is a genuine finding, not a nitpick; flag it with a suggested fix: {{@prompts/_shared/perf-rules.md}} -8. Check how the diff parses structured values, per these rules: +9. Check how the diff parses structured values, per these rules: {{@prompts/_shared/parsing-rules.md}} -9. Check that in-app navigational links use React Router's Link rather than a raw `` +10. Check that in-app navigational links use React Router's Link rather than a raw `` tag, per these rules: {{@prompts/_shared/navigation-rules.md}} -10. Check whether the diff renames, moves, or deletes a name that is persisted outside +11. Check whether the diff renames, moves, or deletes a name that is persisted outside the codebase and read back after deploy, per these rules: {{@prompts/_shared/rename-compatibility-rules.md}} -11. Validate which of Codex's findings are real (discard false positives), add any genuine +12. Validate which of Codex's findings are real (discard false positives), add any genuine issues Codex missed, and (when a ticket is available) judge genuine misses of the ticket's intent or clear scope creep — but give credit when the author went beyond the literal acceptance criteria in a sound way rather than flagging it as non-compliant. -12. Decide ONE verdict. Be pragmatic: use "request_changes" only when there is at least one +13. Decide ONE verdict. Be pragmatic: use "request_changes" only when there is at least one genuine, blocking issue (such as a bug, regression, privacy violation, half-finished task, placeholder, or deferred work); otherwise "approve". A change that exceeds the AC without breaking the ticket's intent is a reason to approve, not to block. diff --git a/prompts/codex-first-pass.md b/prompts/codex-first-pass.md index 51ab15a..232dfc1 100644 --- a/prompts/codex-first-pass.md +++ b/prompts/codex-first-pass.md @@ -22,19 +22,22 @@ verbatim and handed to a second reviewer, so do not add conversational preamble. 4. Enforce these BiggerPockets member-privacy rules and flag any violation with a file/line reference: {{@prompts/_shared/privacy-rules.md}} -5. {{@prompts/_shared/migration-data-rule.md}} -6. Check the diff for batch/task performance hot spots per these rules, and flag each +5. Enforce the email-deliverability rules and flag any violation with a + file/line reference: + {{@prompts/_shared/email-deliverability-rules.md}} +6. {{@prompts/_shared/migration-data-rule.md}} +7. Check the diff for batch/task performance hot spots per these rules, and flag each genuine one with a file/line reference and a suggested fix: {{@prompts/_shared/perf-rules.md}} -7. Check how the diff parses structured values, per these rules: +8. Check how the diff parses structured values, per these rules: {{@prompts/_shared/parsing-rules.md}} -8. Check that in-app navigational links use React Router's Link rather than a raw `` +9. Check that in-app navigational links use React Router's Link rather than a raw `` tag, per these rules: {{@prompts/_shared/navigation-rules.md}} -9. Check whether the diff renames, moves, or deletes a name that is persisted outside +10. Check whether the diff renames, moves, or deletes a name that is persisted outside the codebase and read back after deploy, per these rules: {{@prompts/_shared/rename-compatibility-rules.md}} -10. Report concrete issues — bugs, regressions, security problems, member-privacy +11. Report concrete issues — bugs, regressions, security problems, member-privacy violations, incomplete tasks/half-measures/placeholders/deferred work, and genuine misses of the ticket's intent or clear scope creep — each with a file/line reference and a brief rationale. Do not list "doesn't match acceptance criteria" as an issue by itself; only