Skip to content

fix(tests): align postmark reply-one leak regression test with configured inbound domain - #73

Open
detail-app[bot] wants to merge 1 commit into
mainfrom
detail/bug-fix/fix-tests-align-postmark-reply-one-leak-regression-088bfe
Open

detail-app[bot] wants to merge 1 commit into
mainfrom
detail/bug-fix/fix-tests-align-postmark-reply-one-leak-regression-088bfe

Conversation

@detail-app

@detail-app detail-app Bot commented Sep 25, 2026

Copy link
Copy Markdown
Contributor

Detail bug report: View on Detail

Bug

The reply-one auto-reply regression test (tests/testPostmarkReplyOneLeak.ts), added to guard against leaking literal null/undefined into the "you forgot to reply-all" auto-reply body, never actually reached the code path it was meant to lock. It hardcoded INBOUND_DOMAIN = 'labs.example', while the only committed test domain is test.local (tests/_setupEnv.ts / .env.test.example). The postmark handler filters recipients by e.split('@')[1] === config.email.inboundDomain, so the fixture address was filtered out, myToEmails was empty, and the handler bailed before the sendMail block — leaving captured.html undefined and all 3 subtests failing for the wrong reason. The test also lacked any env bootstrap, crashing on a clean checkout (src/config.ts throws when required vars are missing).

Fix

Made the test match the established convention used by its sibling tests/testPostmarkEmailSentId.ts:

  • Added import './_setupEnv'; as the first line so required env vars (including EMAIL_INBOUND_DOMAIN=test.local) are populated before src/config.ts validates them.
  • Changed INBOUND_DOMAIN from 'labs.example' to 'test.local' so the fixture recipient passes the inbound-domain filter and the handler reaches the reply-one auto-reply path.

No production code changed — the guard at src/email/postmark.ts:80 (${project?.event?.emailSignature || ''}) is already correct; this restores the test's ability to actually catch its removal.

Testing

  • Regression test passes: npx ts-node --transpile-only tests/testPostmarkReplyOneLeak.ts → 3/3 pass; with .env moved aside it still passes 3/3, confirming the test is self-sufficient via the shared harness (no clean-checkout crash).
  • Guards the intended regression: temporarily reverting ${...emailSignature || ''} → ${...emailSignature} in src/email/postmark.ts makes subtests 1 and 2 fail on \nnull\n / \nundefined\n (1 pass / 2 fail); restoring the guard returns to 3/3 — proving the suite now locks the guard. src/email/postmark.ts was left unchanged after verification.
  • No sibling regression: tests/testPostmarkEmailSentId.ts still passes 3/3.
  • Typecheck/build: npx tsc --skipLibCheck --noEmit exits 0.
  • Lint: npx eslint on the changed file fails only due to a pre-existing tooling mismatch (pinned @typescript-eslint/parser@3.10.1 is incompatible with the installed TypeScript 5.2.2, throwing DeprecationError: 'originalKeywordKind' on every .ts file, including untouched ones). After loading a compatible parser/plugin, the changed file reports only pre-existing violations (no-explicit-any on original as any scaffolding; no-secrets on the unchanged cid12345+es_abc123@ literal) — the 2-line fix introduces zero new violations.

Automatic Fixes PRs can be configured here.

@augmentcode

augmentcode Bot commented Sep 25, 2026

Copy link
Copy Markdown
Contributor
🤖 Augment PR Summary

Summary: This PR makes the Postmark reply-one regression test load the shared test environment before configuration is imported.
Changes: It updates the fixture inbound domain to test.local, matching the configured test domain so the test reaches the intended auto-reply path.

🤖 Was this summary useful? React with 👍 or 👎

@augmentcode augmentcode Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review completed. No suggestions at this time.

Comment augment review to trigger a new review at any time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

0 participants