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