test(mail): characterize production email contracts - #2526
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 18157c9300
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return Regex.Matches(decodedBody, "(?:href=|\\\"(?:target|url)\\\":\\s*)\\\"(?<url>[^\\\"]+)\\\"") | ||
| .Select(match => match.Groups["url"].Value) |
There was a problem hiding this comment.
GetUrls combines anchor destinations with JSON-LD target/url values, so the new assertions cannot prove that a visible link is clickable. For example, if the View Organization anchor loses its href, its dashboard URL remains in the template's JSON-LD; Assert.Contains/AssertContainsUrl and the route whitelist still pass. Extract and assert anchor hrefs separately from structured-data actions so these tests catch broken email CTAs.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d64336d454
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| if (!String.Equals(uri.Authority, baseUri.Authority, StringComparison.OrdinalIgnoreCase)) | ||
| { | ||
| Assert.Contains(uri.Authority, _expectedExternalHosts); |
There was a problem hiding this comment.
When an external link's scheme, path, query, or fragment changes but its host remains allowlisted, this assertion still passes—for example, http://github.com/wrong-page is accepted. This means accidental breakage of the FAQ, social-profile, or product links is no longer detected despite these tests characterizing known email destinations; compare against the complete expected external URLs or validate each host's allowed paths and HTTPS scheme.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5b6a0805ea
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
|
|
||
| Assert.DoesNotContain("/next/", uri.PathAndQuery, StringComparison.OrdinalIgnoreCase); | ||
| Assert.Matches(@"^/(?:event/[^/?#]+|stack/[^/?#]+(?:/(?:mark-fixed|ignored|discarded))?|project/[^/]+/(?:configure|error/(?:timeline|frequent|new))|account/(?:manage|verify)|organization/[^/]+/(?:dashboard|upgrade|frequent|manage)|signup|reset-password/[^/?#]+)(?:[/?].*)?$", uri.PathAndQuery); |
There was a problem hiding this comment.
When an internal destination accidentally gains a trailing segment, such as /organization/{id}/dashboard/typo, this pattern still accepts it through (?:[/?].*)?; the corresponding Assert.Contains checks also pass because the expected URL remains a prefix. Tighten the route patterns so only routes that intentionally accept additional path segments or queries do so, otherwise broken email links remain undetected.
Useful? React with 👍 / 👎.
| return Regex.Matches(decodedBody, "(?:href=|\\\"(?:target|url)\\\":\\s*)\\\"(?<url>[^\\\"]+)\\\"") | ||
| .Select(match => match.Groups["url"].Value) |
There was a problem hiding this comment.
When an email's JSON-LD is malformed—for example, by a missing brace or comma—this regex still extracts its target and url strings, so every new URL assertion can pass even though email clients cannot consume the action markup. Extract the JSON-LD script and deserialize it before validating its action fields so the tests also enforce valid structured data.
Useful? React with 👍 / 👎.
|
1 participant
Summary
Verification
Post-Deploy Monitoring & Validation
No additional operational monitoring is required because this PR changes tests only; production email code and rendered output are unchanged.