fix(email): make the docs and mock match the live API#110
Merged
Conversation
Ran the whole email surface end-to-end against a live account today -- set, read the mailed token from the mailbox, verify, remove, replay, restore -- and found the SDK describing a system that does not exist. All of this shipped in #106, which I reviewed and approved this morning without once running it against the server. Design ruled 2026-07-20: VERIFY-THEN-ATTACH is intended. The server does not attach an address until the mailed token is redeemed. That is the safer design: a pending set_email cannot detach an already-confirmed recovery address, so someone holding an API key cannot strip the recovery path by pointing it at an address they control. Three things were wrong. 1. get_email docstring claimed "`email` is populated as soon as set_email succeeds, but stays unverified until the mailed link is redeemed". It is not. Observed on a verified account AND a freshly-emptied one, polled at +2s, +10s and +30s: the pending address never appears. Now documents verify-then-attach and the security property that follows from it. 2. verify_email docstring claimed a `{"status": ..., "email": str}` return. The server returns `{"email": str, "email_verified": bool}` -- no status key at all. 3. The MOCK matched the wrong docs rather than the server, in both directions: mock verify_email -> {"status": "verified", "email": ...} live verify_email -> {"email": ..., "email_verified": true} No overlap on the discriminating field, so a caller writing `resp["status"] == "verified"` against the mock gets a KeyError in production. That is precisely the failure a testing mock exists to prevent, and it shipped because every test asserted the mock against itself; nothing ever compared it to the API. The mock's get_email default was worse than a wrong value: {email: <address>, email_verified: False} is UNREACHABLE under verify-then-attach, since email_verified is exactly `email is not None`. The original intent -- make callers handle the not-ready case instead of assuming a usable address -- was right, and is preserved by defaulting to the genuinely not-ready state {None, False} rather than an impossible one. Tests pin exact key sets, so an added or dropped field fails in CI rather than in a consumer's production logs, plus an invariant test asserting the mock can never emit the unreachable state. Everything else behaved exactly as documented and is recorded here rather than re-tested later: the verification token is genuinely single-use (replay returns the same opaque 400 as a garbage token, indistinguishable); remove_email is uniform and idempotent, byte-identical whether or not anything was attached; set_email to an address at a domain I own with no mailbox returns the identical verification_pending envelope, so the no-enumeration property holds; mail arrives in the same second as the call. ruff check + ruff format + mypy clean, 1043 passed / 157 skipped.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Ran the email surface end-to-end against a live account today — set, read the mailed token from the mailbox, verify, remove, replay, restore — and found the SDK describing a system that does not exist.
All of this shipped in #106, which I reviewed and approved this morning without once running it against the server. I checked the reasoning carefully and never checked the artifact.
Design ruled 2026-07-20: verify-then-attach is intended. The server does not attach an address until the mailed token is redeemed — the safer design, since a pending
set_emailcannot detach an already-confirmed recovery address.Three things were wrong
1.
get_emaildocstring claimed "emailis populated as soon asset_emailsucceeds, but stays unverified". It is not. Polled at +2s, +10s and +30s on both a verified account and a freshly-emptied one — the pending address never appears.2.
verify_emaildocstring claimed{"status": ..., "email": str}. The server returns{"email": str, "email_verified": bool}— nostatuskey at all.3. The mock matched the wrong docs rather than the server, in both directions:
No overlap on the discriminating field, so a caller writing
resp["status"] == "verified"against the mock gets aKeyErrorin production. Exactly the failure a mock exists to prevent — and it shipped because every test asserted the mock against itself.The mock default was worse than a wrong value
{email: <address>, email_verified: False}is unreachable under verify-then-attach:email_verifiedis exactlyemail is not None. The original intent — make callers handle the not-ready case — was right, and is preserved by defaulting to the genuinely not-ready{None, False}instead of an impossible state.Tests now pin exact key sets, plus an invariant test asserting the mock can never emit the unreachable state.
Everything else behaved as documented
Recorded here rather than re-tested later: the token is genuinely single-use (replay returns the same opaque 400 as a garbage token);
remove_emailis uniform and idempotent;set_emailto an address at a domain I own with no mailbox returns the identical envelope, so the no-enumeration property holds; mail arrives in the same second as the call.ruff check+ruff format+mypyclean · 1043 passed / 157 skipped