Skip to content

Fix email verification flow with enhanced templates and frontend URLs#464

Merged
jfrench9 merged 4 commits into
mainfrom
bugfix/email-verification-flow
Mar 12, 2026
Merged

Fix email verification flow with enhanced templates and frontend URLs#464
jfrench9 merged 4 commits into
mainfrom
bugfix/email-verification-flow

Conversation

@jfrench9
Copy link
Copy Markdown
Member

Summary

Fixes the email verification flow by updating the default app source detection, enhancing AWS SES email templates, and wiring frontend URL parameters through the full deployment stack. These changes ensure that verification emails render correctly and link users back to the appropriate frontend application (RoboSystems, RoboLedger, or RoboInvestor).

Key Accomplishments

Email Template & Content Improvements

  • Overhauled AWS SES email templates with improved content formatting and structure
  • Refactored email rendering logic in robosystems/operations/aws/ses.py (~366 lines restructured) for better maintainability and consistent output across all email types

App Source Detection Fix

  • Updated the default app source in detect_app_source to 'robosystems', correcting misrouted verification flows that relied on an incorrect default

Frontend URL Configuration Across Deployment Stack

  • Added frontend URL parameters for RoboSystems, RoboLedger, and RoboInvestor to:
    • GitHub Actions deployment workflows (deploy-api, deploy-dagster, prod, staging)
    • CloudFormation templates (api.yaml, dagster.yaml)
    • Application configuration layer (env.py, parameter_store.py, validation.py)
  • Removed stale secrets manager references that are no longer needed
  • Updated bootstrap provisioning to support the new parameter definitions

Auth Utilities

  • Adjusted auth router utilities to align with the corrected app source logic

Breaking Changes

  • Environment configuration: Deployments now require frontend URL parameters for all three applications (RoboSystems, RoboLedger, RoboInvestor) to be defined in parameter store. Existing environments must be updated with these new parameters before deploying this change.
  • The default app source change from the previous value to 'robosystems' may affect any downstream logic that relied on the old default.

Testing Notes

  • Unit tests updated in tests/operations/aws/test_ses.py and tests/config/test_validation.py to reflect new email formatting and validation parameter additions
  • Verify end-to-end email verification flow for each app (RoboSystems, RoboLedger, RoboInvestor) in staging to confirm correct redirect URLs in verification emails
  • Confirm that emails render properly across major email clients (Gmail, Outlook, Apple Mail)

Infrastructure Considerations

  • CloudFormation stacks for both API and Dagster services require updates to accept and propagate the new frontend URL parameters
  • CI/CD workflows for all environments (staging and production) have been updated to pass the new parameters during deployment
  • Parameter store entries for the three frontend URLs must be provisioned prior to stack updates — the bootstrap tooling has been updated accordingly
  • No database migrations required

🤖 Generated with Claude Code

Branch Info:

  • Source: bugfix/email-verification-flow
  • Target: main
  • Type: bugfix

Co-Authored-By: Claude noreply@anthropic.com

…stor in deployment workflows and CloudFormation templates

- Introduced new parameters for frontend URLs in deploy-api.yml and deploy-dagster.yml workflows.
- Updated prod.yml and staging.yml to include these parameters for production and staging environments.
- Added corresponding parameters in CloudFormation templates (api.yaml and dagster.yaml) for consistent configuration.
- Enhanced bootstrap.sh to set up SES email identity for transactional emails, ensuring proper email handling.
- Updated environment configuration to retrieve frontend URLs from environment variables instead of Secrets Manager.
- Introduced a new base HTML template for transactional emails, improving layout and branding consistency.
- Refactored email templates for verification, password reset, and welcome emails to utilize the new structure.
- Updated email subject lines for consistency in casing and branding.
- Added utility functions for rendering buttons and paragraphs in email content.
- Adjusted tests to reflect changes in email subject lines and content structure.
- Changed the default return value in the detect_app_source function from 'roboledger' to 'robosystems' to reflect the main hub designation.
- This adjustment ensures consistency in app source detection across the application.
@jfrench9
Copy link
Copy Markdown
Member Author

@claude please review this PR

@claude
Copy link
Copy Markdown

claude Bot commented Mar 12, 2026

Claude finished @jfrench9's task in 2m 29s —— View job


PR Review: Fix email verification flow

Tasks:

  • Gather context and understand the request
  • Read changed files
  • Provide comprehensive review feedback

Overall this is a well-scoped fix with a clear change chain from CloudFormation → CI/CD → config → email templates. The architecture is correct and the default-URL safety nets are a nice touch. A few issues worth addressing before merge, from most to least critical.


Bugs

1. Default app fallback is inconsistent — display name and URL mismatch

In all three send_* methods in ses.py, when an unknown app value is passed, the URL falls back to roboledger but the display name falls back to RoboSystems:

# ses.py:330, 368, 401
base_url = self.app_urls.get(app, self.app_urls["roboledger"])   # → RoboLedger URL

# ses.py:341, 379, 410
"app_name": app_display_names.get(app, "RoboSystems"),          # → "RoboSystems" name

This would produce an email that says "Thanks for creating a RoboSystems account" with a verification link pointing to roboledger.ai. The test test_default_app_fallback (line 217) passes because it only checks the URL, not the display name. Either pick "roboledger" as the default for both, or "robosystems" for both (consistent with the detect_app_source default change). Fix this →


Code Quality

2. app_display_names dict is duplicated three times

The same dictionary literal is copy-pasted across send_verification_email (line 333), send_password_reset_email (line 371), and send_welcome_email (line 402). Should be a module-level constant alongside the brand colors:

_APP_DISPLAY_NAMES = {
    "roboledger": "RoboLedger",
    "roboinvestor": "RoboInvestor",
    "robosystems": "RoboSystems",
}

Fix this →

3. print() calls in env.py and parameter_store.py

The new code in env.py (preload block) and parameter_store.py (empty SSM value warning) uses print() instead of the project's logging infrastructure. For env.py specifically, standard logging can't be used due to circular imports, but a logging.getLogger(__name__) at the module level (like parameter_store.py already does) works fine since it doesn't import from robosystems.logger.

# parameter_store.py already has this at the top:
logger = logging.getLogger(__name__)

# The new print() calls should be logger.warning() calls instead

4. Frontend URLs not validated in EnvValidator

_validate_urls in validation.py validates DATABASE_URL, VALKEY_URL, GRAPH_API_URL but not the three new frontend URL env vars. Given that the whole PR is about wiring these URLs correctly, adding format validation would catch misconfiguration early:

url_vars = [
    "DATABASE_URL",
    "VALKEY_URL",
    "GRAPH_API_URL",
    "ROBOSYSTEMS_URL",
    "ROBOLEDGER_URL",
    "ROBOINVESTOR_URL",
]

Infrastructure Notes

5. dagster.yaml injects frontend URLs into both task definitions

The frontend URLs are added to both the daemon task (line 667) and what appears to be the worker task (line 869). This is correct since both ECS tasks share the same env, but it's worth confirming the daemon actually sends emails (or whether only one task definition needs them). If the daemon never sends emails, this is harmless but adds noise.

6. Bootstrap ensure_ses_production_access relies on bash dynamic scoping

In bootstrap.sh, ensure_ses_production_access references ${email_domain:-robosystems.ai} but email_domain is a local variable in setup_ses_identity. This works because bash local variables are dynamically scoped (visible to called functions), but it's fragile — if ensure_ses_production_access is ever called independently, it silently uses the default. Consider passing email_domain as an explicit argument.


Minor

7. PR description says "parameter store" but code uses CloudFormation env vars

"Deployments now require frontend URL parameters for all three applications... to be defined in parameter store."

The actual implementation passes them as CloudFormation parameters → ECS task environment variables (get_str_env), not SSM Parameter Store. The description is slightly misleading for future operators reading the deployment runbook.

8. Unrelated scope in env.pypreload_feature_flags at import time

The change from except ImportError to except Exception and the addition of preload_feature_flags() called at module import time is a startup behavior change that isn't directly related to the email verification fix. It's a reasonable improvement but mixing it here makes the PR harder to reason about. Not a blocker, just worth noting for reviewers.


Test Coverage

The test in test_default_app_fallback (line 217) only asserts the URL falls back correctly — it doesn't check the display name, which is where the bug (issue #1) hides. After fixing the fallback inconsistency, the test should be extended to assert "RoboSystems" (or whichever name is chosen as the consistent default) appears in the email body.


Summary: Two things should be fixed before merging — the display name / URL fallback mismatch (issue #1, actual user-visible bug) and the duplicated app_display_names dict (issue #2, DRY). The rest are improvements worth doing but not blockers.

Comment thread robosystems/config/env.py
# calls don't each make a separate SSM API call (which can fail under load).
_preloaded_flags = preload_feature_flags()
if _preloaded_flags:
print(f"Preloaded {len(_preloaded_flags)} feature flags from SSM")
Comment thread robosystems/config/env.py
if _preloaded_flags:
print(f"Preloaded {len(_preloaded_flags)} feature flags from SSM")
elif os.getenv("ENVIRONMENT", "dev") in ("prod", "staging"):
print("WARNING: No feature flags loaded from SSM. All flags will use defaults.")
Comment thread robosystems/config/env.py
Comment on lines +79 to +82
print(
f"WARNING: Parameter store unavailable ({type(_e).__name__}: {_e}). "
"All feature flags will use defaults (false)."
)
- Introduced a centralized mapping for app display names to improve consistency in email templates.
- Updated the default app handling to use 'robosystems' across various email functions.
- Adjusted tests to verify the correct fallback to 'robosystems' in email content.
@jfrench9 jfrench9 merged commit 4fc90bc into main Mar 12, 2026
7 checks passed
@jfrench9 jfrench9 deleted the bugfix/email-verification-flow branch March 12, 2026 18:36
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.

1 participant