P1: Verify complete password-reset delivery and recovery

[tomqwu]

Current Priority Decision

Owner direction, 2026-09-13: complete Church/Basketball business workflows and day-to-day operations for every role first; billing and platform readiness later.

Execution lane: phase: business-flow. Current business milestone: #289.
This section overrides older priority, start conditions, broad dependency order and launch estimates below. Keep the detailed technical recommendations where compatible. Execute only the NOW slice; do not expand a mixed issue into its whole production scope. Record completed slice receipts in #289; keep this issue open while retained later work remains.

NOW

Prove UI forgot/invitation -> actual generated message in owned local mail sink -> real link -> successful login, for the daily role flows. Preserve already-correct BackgroundTasks/fallback paths and test retry/expiry/replay without debug-token shortcuts.

LATER

Approved external-mailbox/provider delivery and production sender/domain reputation/configuration. Local captured delivery must be labeled local, not real internet delivery.

Validation stays local. No CI checks, hosted reviewers or Ollama code review. Preserve tenant isolation, real member responses and atomic roster changes. A deferred feature is not permission to expose an unfixed vulnerability. No deployment, paid-provider activation or production sign-off is authorized by this reprioritization.

Before implementation, read #252 and #289; finish one role/work package with tests, local review and affected docs/assets. The earlier completion receipt below covers whole-issue closure, not a requirement to finish every deferred package before the business milestone.

---

Current Implementation Handoff

Prepared 2026-09-13 for a lower-cost builder at source 21a4a804aa57580451edded04736b1f51aef7e48.
No CI checks. All implementation validation and code review run locally.
Use the shared builder contract and this issue's work packages; no xhigh model or automatic model upgrade is required. This is a detailed recommendation, not a claim that a smaller model cannot make mistakes or that tests have passed.

Risk/review focus: High: account recovery and real delivery evidence.
Start condition: Start only the NOW work package defined above. Use its local business prerequisites, not the entire older platform-release dependency list.

Source of Truth and Current State

• web/auth.py
• api/routers/password_reset.py
• api/services/email_service.py
• api/models.py
• tests/api/test_password_reset_email.py
• tests/api/test_password_reset.py
• tests/web/test_password_reset.py
• tests/mock_smtp_server.py

forgot_submit already receives/passes BackgroundTasks and attaches it to the response. EmailService already emits the web fallback /auth/reset/{token}; reset tokens are hashed/persisted and roster-only people are excluded. Remaining work is full rendered email/browser, failure/retry and release delivery verification.

This handoff supersedes stale implementation statements in the background below. Preserve existing successful behavior and tests. Recheck the current branch before editing; the baseline is a source pointer, not permission to discard newer changes.

Dependencies and Ownership

• NOW-slice prerequisite receipts: P1: Repair local test entry points, coverage and isolation #281, P1: Complete core scheduling with billing and paid delivery disabled #288; only the needed safe/local package must pass, not every platform acceptance item in those tickets. Full-release dependency designs below remain later work.
• Coordinate, but do not wait for entire tickets to close merely to inspect/write isolated tests: P0: Prevent privileged signup and enforce organization membership #255, P2: Validate production configuration after business-flow acceptance #258, P1: Protect browser workflows; defer distributed abuse limits #261, P1: Make workflow notifications reliable; defer multiworker rollout #266, P2: Run production release acceptance after business flows are accepted #271, P2: Regenerate the mobile API client and validate native release #191.
• The final commands below require P1: Repair local test entry points, coverage and isolation #281's safe local environment or an equivalently verified isolated setup. A missing dependency blocks only the slice that needs it; document that state.
• Shared source/ORM/migration changes are serialized per the master roadmap. Do not start concurrent edits to the same ownership area.

Recommended Decisions

1. Preserve generic known/unknown-email response and hashed one-time reset tokens. Never use a debug token response or DB token lookup as the acceptance link source.
2. Capture the actual generated email in an isolated local mail sink, parse its web fallback URL and drive that link in Playwright. Mobile custom-scheme handling is separate P2: Regenerate the mobile API client and validate native release #191 evidence.
3. Default delivery remains best effort until P1: Make workflow notifications reliable; defer multiworker rollout #266 supplies durable retry. Do not report queued/no-op email as delivered; explicitly record failed/disabled delivery and retry behavior without enumerating accounts.
4. On successful reset invalidate prior credentials according to existing password-version/refresh rules; do not provision login for invitation-only roster records.

Small Work Packages

Each item is one reviewable slice, not permission for one giant PR. Add the failing regression first; finish code, tests and affected docs for that slice together. Leave this issue open until all packages and original acceptance criteria are satisfied or explicitly revised by the owner.

• 262.1: Extend existing wiring tests to preserve implemented fixes, then add mail-body parsing/link-route checks.
• 262.2: Drive forgot -> captured email -> reset -> new login and prior-session rejection in the real browser with debug token output disabled.
• 262.3: Add expiry/replay/new-token-invalidates-old tests and failure/disabled/slow-send paths using local fakes and a live HTTP timing test where timing is claimed.
• 262.4: Attach authorized external inbox evidence only in the release slice; update current issue progress/title and P2: Reconcile platform assurances with later release evidence #284 docs.

Required Regression Cases

These are specifications for tests to add/retain, not claimed execution results. Each new negative case must assert unchanged unauthorized state and zero forbidden side effects.

• T262-01: Known and unknown email -> same public shape; unknown/roster-only person -> no reset token/send.
• T262-02: Captured email web link opens the real reset form; new password works and old password/session is rejected per current revocation contract.
• T262-03: Expired/replayed/superseded token -> controlled failure and unchanged password; malicious display name remains escaped in email.
• T262-04: Send fails/disabled/slow -> generic response, safe logs, no false delivery claim; a fresh request yields usable recovery without stale-token acceptance.

Local Commands and Evidence

Existing targeted commands (paths checked against the audit source; run only after the stated safe preflight):

poetry run pytest tests/api/test_password_reset_email.py tests/api/test_password_reset.py tests/web/test_password_reset.py -q
poetry run pytest tests/e2e/test_auth_flows.py -q

Also run the shared formatting/lint/touched-type/unit/full-suite and local review protocol from #252 for the final pushed revision. Add new targeted tests to these commands when implemented. Run API and browser tiers in separate processes. Native, PostgreSQL, image, provider and operator drills require their explicit environment; an unavailable tool/target is blocked/not run, never a pass.

Schema and Compatibility

Do not replace the existing PasswordResetToken model or re-add its migration. Durable retry schema belongs to #266; coordinate token-expiry implications.

Stop Conditions

No real mailbox/provider sends without separate approval. If delivery reliability requires durable jobs, stop that slice at #266 rather than pretend BackgroundTasks survives process loss.

After two failed focused repair attempts without new diagnostic evidence, stop the affected package and post the exact failure, commands, suspected boundary and needed decision. Do not silently broaden scope, weaken tests or upgrade models. A fresh local reviewer checks: Verify the email actually rendered contains the navigated URL, debug token output is off, and session/replay/roster-only protection is retained.

Completion Receipt

• Work-package and regression IDs above map to changed files and actual results.
• Commands, versions, dates, pass/fail/skip/not-run counts, logs/screenshots and tested head/base SHAs are linked.
• A separate local review records findings and resolution; self-review is labeled if used and is not misrepresented as independent review.
• Affected docs/README/playbooks/screenshots and dependency/roadmap status are reconciled, not left as unnamed follow-ups.
• If implementation is authorized through PR/merge, GitHub reports mergeable and the shared local-evidence requirements are met; reviewer agents never merge.
• No hosted CI check, status attestation, Ollama reviewer, live provider action, deployment, real-data purge or store submission was introduced by implication.

Copyable Builder Prompt

First read this issue's Current Priority Decision and #289. Run only its NOW slice.
If this issue is deferred, report that state instead of starting the older package list.
Implement the next ready work package in tomqwu/SignUpFlow issue #262.
Read its Current Implementation Handoff and #252 Builder Handoff Contract first.
Inspect current source and preserve newer/unrelated changes. Start with the
package's failing regression, then complete code, local tests, local review and
affected docs/assets together. Do not skip acceptance or invent passing evidence.
No CI checks or Ollama code review. Do not deploy, activate providers, purge real
data or submit to stores. Stop and report unmet prerequisites or policy decisions.
Record the package/test IDs and exact reviewed/tested source SHAs before claiming done.

---

Earlier Audit and Acceptance Context

Current policy (2026-09-13)

No CI checks. Everything is validated locally. This includes code review,
formatting, lint, type checks, migrations, all test tiers, security scans,
artifact checks and mobile validation. Do not add hosted jobs, required CI
statuses, synthetic success checks or an Ollama reviewer. GitHub is for source,
PRs, issues and publication, not validation.

Record commands, environment, results, limitations and reviewed head/base SHAs.
Builders merge only with completed local evidence and GitHub mergeability;
reviewer agents never merge. Real staging/provider/device acceptance remains
required where applicable, driven by authorized local operator tools.
Historical evidence and older comments do not override this policy.

Parent roadmap: #252

Priority: P1, blocks core production pilot. Phase: B. Suggested owner: Backend/web. Original estimate (superseded; re-estimate remaining work): 2-3 engineering days.

Historical audit evidence (recheck against current source)

web/auth.py:206 supplies a throwaway BackgroundTasks to request_password_reset and returns an unrelated TemplateResponse. The audit got 200 with zero executions of the mocked reset-email task. api/services/email_service.py:1053 builds /reset-password?token=... while the web app serves /auth/reset/{token}. Existing tests/web/test_password_reset.py exercises a token obtained from the API's debug response, not a received web-reset message.

Source baseline: GitHub main 214e3f3f17a582d5f9b2063be6872ea2b1d25714, audited 2026-09-09.

Implementation plan

1. Preserve the already-correct BackgroundTasks injection/response attachment. Add complete email-to-browser regression coverage, and adopt P1: Make workflow notifications reliable; defer multiworker rollout #266's durable messaging service only when its transaction/retry contract exists.
2. Preserve the existing correct /auth/reset/{token} fallback and mobile deep-link option. Verify configured origin and actual rendered invitation/reset email links against registered routes.
3. Add an end-to-end test starting at the web forgot form, capturing the outgoing email in a local mail sink and following the actual link to completion. Review invitation links with the same method.
4. Handle send failure with operator visibility and safe retry while preserving the generic user response. Never use DEBUG_RETURN_RESET_TOKEN as production delivery.
5. Test old-token invalidation, one-time redemption, roster-only people, invalid passwords and session revocation.

Acceptance criteria

• A known account's web reset request executes/queues exactly one observable email task.
• The received link resolves to the actual reset page and enables the intended account to reset once.
• Unknown accounts receive the same user-facing response and no email; no token is returned by production endpoints.
• Delivery failure is observable and retryable; no real external sends in automated tests.

Dependencies

#261

Validation

Add failing web-task and generated-link tests first. Run API/web reset suites plus a Playwright journey through a local mail sink. Validate actual provider delivery only in an approved staging mailbox.

Whole-repository audit scope (2026-09-13)

Baseline: 21a4a804aa57580451edded04736b1f51aef7e48. This addendum assigns full-scope follow-through; it is not a new test pass or production sign-off. No CI checks; all review and validation runs locally.

Prove complete reset/invitation delivery-to-link-to-form-to-session flows, not a token obtained through a debug response. Include web/auth.py BackgroundTasks integration, expiration, invalid/replayed token, wrong organization, mail failure and retry behavior. #279/#285 consume real browser link flows; #266 owns queued delivery reliability. All test sends are mocked/local sandbox by default; authorized external inbox acceptance remains a separate recorded release step.

Keep evidence and disposition synchronized with master roadmap #252 and documentation ledger #277. Close only after the remaining acceptance criteria have linked local results; a planning/audit note is not completion.

[tomqwu]

Delivery repair pushed in draft PR #272, commit c736236.

• Attach FastAPI background tasks to the browser forgot-password response so the send executes.
• Generate /auth/reset/{token} fallback links using the configured public web origin, normalize trailing slashes, and retain mobile deep links.
• Log unsuccessful sends (false or exception) without recipient/token data in the reset wrapper; preserve the generic response and allow a fresh request that invalidates the old token.
• New tests capture the outgoing email body, follow its actual browser URL through reset, verify one-time redemption, identical unknown-account responses/no email, and safe re-request after a simulated send failure. No real external mail was sent.

TDD: three new cases failed before the fix. API/web reset suite: 23 passed. Full web/contract suite: 221 passed. Full make test-all, Black, and Ruff passed. GitHub CI on c736236 is pending.

Keep this issue open: an actual Playwright/local-mail-sink journey, invitation fallback review, approved staging mailbox delivery, and durable queuing remain. Browser protections (#261) and durable messaging (#266) are not implemented by this change; no production-delivery claim or merge.

[tomqwu]

Validation update for draft PR #272, head c736236: GitHub CI passed, including backend suites, web/contract tests, Playwright, strict mypy, and the PostgreSQL Alembic migration check. Full mypy api remains advisory with existing typing debt.

Codex review was skipped because OPENAI_API_KEY is missing; this is not review approval. The PR remains draft and unmerged. Previously documented release follow-ups remain open, including PostgreSQL runtime/cascade coverage and staging delivery verification where applicable. No protection/ruleset/settings changes were made.

[tomqwu]

NOW-slice receipt: merged PR #304 (c2609e8).\n\nThe browser now proves forgot/invitation -> actual generated RFC 822 message in an owned local sink -> real route -> successful login, with no debug-token shortcut. Known/unknown public copy stays generic; old password and replayed token fail; configured failure and disabled delivery are distinguishable and retry behavior is covered. Church/Basketball run at 360px and 1440px. Exact-head full suite and mobile suite passed; see the PR receipt.\n\nKeeping this issue open because its Current Priority Decision explicitly retains authorized external mailbox/provider acceptance as LATER. No external provider was contacted or enabled.

[tomqwu]

Local recovery slice merged in #306 (dbafb60008c69d4ba39580dff23bac4e203e957d). Both access levels in both playbooks follow the real locally captured reset message; debug/DB token shortcuts are not used. Short API passwords are rejected before token claim, and old sessions/passwords/replays fail. External mailbox/provider acceptance and durable delivery remain open under their later authorized scopes.

[tomqwu]

Final completion receipt

Closed against current main 2c25f99 after reconciling PRs #304 and #306 with the current local-delivery acceptance.

• The browser forgot-password and invitation paths generate exactly one parseable message in an owned local mail sink and follow the real link through successful login.
• Known/unknown account responses remain non-enumerating; no production endpoint returns a reset token.
• Reset links are single-use; expiry, replay, weak-password, old-session, disabled-delivery, failed-delivery, and retry behavior are covered without debug-token or database shortcuts.
• Admin and volunteer actors in both Church and Basketball playbooks complete the recovery journey.

Current exact-head make test-all evidence: 1,710 passed, 21 explicitly skipped, 0 failed, 0 errors; report test-artifacts/local-validation/20260914T203506.257431Z-27095-14d9ac21/report.json. External mailbox/provider reputation remains correctly outside this local business issue and no live send was attempted. Local self-review found no unresolved issue acceptance gap.