Address review comment

Author: nabramow

CLAUDE.md

@@ -44,7 +44,8 @@ make mypy
 - gRPC for API (defined in `/app/proto`)
 - Background jobs in `couchers/jobs/handlers.py`
 - Notifications system in `couchers/notifications/`
-- Always run `make format` and `make mypy` after modifying backend code
+- Always run `make format` and `make mypy` after modifying backend code. mypy MUST pass — a failing mypy is never acceptable, so fix it before moving on (don't dismiss errors as "pre-existing")
+- If mypy or tests fail with import errors or missing symbols from generated proto modules (`couchers.proto.*` — e.g. a message type that exists in a `.proto` source but not in the generated `*_pb2.py`), your locally generated protos are stale: run `make protos` to regenerate them, then re-check
 - NEVER try-catch an exception and silently throw it away or just log it. By and large you don't need to wrap code in try-catch blocks, we already handle exceptions
 - Use `enum.auto()` for all enums (except in the rare case that they are inherently ordinal and we use that order in business logic)
 - Put relationships and constraints at the end of models

app/.gitlab-ci.yml

@@ -737,6 +737,11 @@ preview:backend:
   inherit:
     # no docker login
     default: false
+  # serialize comment writes within a PR (see preview:pr-comment-stub)
+  resource_group: preview-comment-$CI_COMMIT_REF_SLUG
+  before_script:
+    # aws-base has no python; needed for the schema diff and the PR comment
+    - apt-get update -qq && apt-get install -y --no-install-recommends python3
   script:
     # Create cleaned schema.sql with git hash header (only if schema file exists)
     - |
@@ -752,6 +757,12 @@ preview:backend:
             | cat -s
         } > artifacts/schema_dumps/schema.sql
       fi
+    # Render a highlighted HTML diff of this branch's schema against develop's
+    - |
+      if [ -f "artifacts/schema_dumps/schema.sql" ]; then
+        curl -fsSL "https://develop--schema.$PREVIEW_DOMAIN/schema.sql" -o develop-schema.sql || true
+        python3 app/scripts/schema_diff_html.py develop-schema.sql artifacts/schema_dumps/schema.sql artifacts/schema_dumps/diff.html
+      fi
     # Upload coverage report to S3
     - aws s3 rm s3://$AWS_PREVIEW_BUCKET/bcov/$CI_COMMIT_SHORT_SHA/ --recursive
     - aws s3 cp htmlcov s3://$AWS_PREVIEW_BUCKET/bcov/$CI_COMMIT_SHORT_SHA/ --recursive
@@ -771,6 +782,8 @@ preview:backend:
     - aws s3 cp artifacts/test_artifacts s3://$AWS_PREVIEW_BUCKET/test-artifacts/$CI_COMMIT_REF_SLUG/ --recursive 2>/dev/null || true
     - echo "Done, test artifacts available at https://$CI_COMMIT_SHORT_SHA--test-artifacts.$PREVIEW_DOMAIN/ and https://$CI_COMMIT_REF_SLUG--test-artifacts.$PREVIEW_DOMAIN/"
     - echo "Sample emails index at https://$CI_COMMIT_SHORT_SHA--test-artifacts.$PREVIEW_DOMAIN/emails/index.html and https://$CI_COMMIT_REF_SLUG--test-artifacts.$PREVIEW_DOMAIN/emails/index.html"
+    # write the backend items into the sticky PR comment (no-op off a PR)
+    - python3 app/scripts/pr_preview_comment.py --items schema,schema-diff,emails,bcov
   artifacts:
     paths:
       - artifacts/schema_dumps
@@ -830,12 +843,16 @@ preview:wcov:
   inherit:
     # no docker login
     default: false
+  resource_group: preview-comment-$CI_COMMIT_REF_SLUG
+  before_script:
+    - apt-get update -qq && apt-get install -y --no-install-recommends python3
   script:
     - aws s3 rm s3://$AWS_PREVIEW_BUCKET/wcov/$CI_COMMIT_SHORT_SHA/ --recursive
     - aws s3 cp artifacts/lcov-report s3://$AWS_PREVIEW_BUCKET/wcov/$CI_COMMIT_SHORT_SHA/ --recursive
     - aws s3 rm s3://$AWS_PREVIEW_BUCKET/wcov/$CI_COMMIT_REF_SLUG/ --recursive
     - aws s3 cp artifacts/lcov-report s3://$AWS_PREVIEW_BUCKET/wcov/$CI_COMMIT_REF_SLUG/ --recursive
     - echo "Done, coverage report available at https://$CI_COMMIT_SHORT_SHA--wcov.$PREVIEW_DOMAIN/ and https://$CI_COMMIT_REF_SLUG--wcov.$PREVIEW_DOMAIN/"
+    - python3 app/scripts/pr_preview_comment.py --items wcov
   rules:
     - if: ($BUILD_WEB == "true") && ($DO_CHECKS == "true") && ($CI_COMMIT_BRANCH == $RELEASE_BRANCH)
     - if: ($BUILD_WEB == "true") && ($DO_CHECKS == "true") && ($CI_COMMIT_BRANCH != $RELEASE_BRANCH)
@@ -961,31 +978,48 @@ preview:pr-comment-stub:
   image: python:3.14-slim
   inherit:
     default: false
-  # serialize comment writes across concurrent pipelines
-  resource_group: pr-preview-comment
+  # serialize all writes to one PR's comment; different PRs don't contend
+  resource_group: preview-comment-$CI_COMMIT_REF_SLUG
   script:
-    - pip install --quiet --no-cache-dir requests
     - python app/scripts/pr_preview_comment.py --stub
   rules:
     - if: $CI_COMMIT_BRANCH != $RELEASE_BRANCH
 
-preview:pr-comment:
+preview:pr-comment-mobile:
   needs:
     - job: preview:pr-comment-stub
       artifacts: false
     - job: preview:mobile-ota-devtool
       artifacts: false
-      optional: true
   stage: preview
   image: python:3.14-slim
   inherit:
     default: false
-  resource_group: pr-preview-comment
+  resource_group: preview-comment-$CI_COMMIT_REF_SLUG
   variables:
     OTA_PLATFORMS: "ios android"
   script:
-    - pip install --quiet --no-cache-dir requests
-    - python app/scripts/pr_preview_comment.py
+    - python app/scripts/pr_preview_comment.py --items mobile
+  # mirror preview:mobile-ota-devtool so this runs exactly when the OTA upload does
+  rules:
+    - if: ($BUILD_MOBILE == "true") && ($CI_COMMIT_BRANCH != $RELEASE_BRANCH)
+      changes:
+        - app/proto/**/*
+        - app/mobile/**/*
+        - app/scripts/**/*
+
+# Vercel owns the web deploy, so this resolves the preview URL from its API
+preview:pr-comment-web:
+  needs:
+    - job: preview:pr-comment-stub
+      artifacts: false
+  stage: preview
+  image: python:3.14-slim
+  inherit:
+    default: false
+  resource_group: preview-comment-$CI_COMMIT_REF_SLUG
+  script:
+    - python app/scripts/pr_preview_comment.py --items web
   rules:
     - if: $CI_COMMIT_BRANCH != $RELEASE_BRANCH
 
@@ -1006,7 +1040,9 @@ build:mobile-native-devtool:
     - npm ci
     - npm install -g eas-cli
     - bash scripts/devtool-build.sh ios
+    - bash scripts/devtool-build.sh ios-sim
     - bash scripts/devtool-build.sh android
+    - bash scripts/devtool-build.sh index
   rules:
     - if: ($BUILD_MOBILE == "true") && ($CI_COMMIT_BRANCH == $RELEASE_BRANCH)
 
@@ -1304,6 +1340,9 @@ preview:protos:
   inherit:
     # no docker login
     default: false
+  resource_group: preview-comment-$CI_COMMIT_REF_SLUG
+  before_script:
+    - apt-get update -qq && apt-get install -y --no-install-recommends python3
   script:
     - python_sum=$(sha256sum app/proto/gen/python.tar.gz | head -c 64)
     - ts_sum=$(sha256sum app/proto/gen/ts.tar.gz | head -c 64)
@@ -1333,6 +1372,7 @@ preview:protos:
     - aws s3 cp app/proto/gen/index.html s3://$AWS_PREVIEW_BUCKET/protos/$CI_COMMIT_SHORT_SHA/
     - aws s3 cp app/proto/gen/index.html s3://$AWS_PREVIEW_BUCKET/protos/$CI_COMMIT_REF_SLUG/
     - echo "Done, protos available at https://$CI_COMMIT_SHORT_SHA--protos.$PREVIEW_DOMAIN/ and https://$CI_COMMIT_REF_SLUG--protos.$PREVIEW_DOMAIN/"
+    - python3 app/scripts/pr_preview_comment.py --items protos
 
 # Having this here frees us from having to list out all the needs in each release below.
 # List out all that need to be waited for until releasing a new version

app/backend.dev.env

@@ -63,10 +63,6 @@ LISTMONK_API_USERNAME=...
 LISTMONK_API_KEY=...
 LISTMONK_LIST_ID=3
 
-RECAPTHCA_PROJECT_ID=...
-RECAPTHCA_API_KEY=...
-RECAPTHCA_SITE_KEY=...
-
 SENTRY_ENABLED=0
 SENTRY_URL=...
 

app/backend/feature-flags.dev.json

@@ -4,7 +4,7 @@
   "strong_verification_enabled": false,
   "log_native_ota_requests": true,
   "donations_enabled": false,
-  "recaptcha_enabled": false,
+  "antibot_enabled": false,
   "postal_verification_enabled": false,
   "listmonk_enabled": false,
   "notification_translations_enabled": true,

app/backend/proto/internal/jobs.proto

@@ -15,13 +15,17 @@ message SendEmailPayload {
   // source data as to where this email came from
   string source_data = 8;
   reserved 9; // Previous "attachments" field, which had mime_type+filename subfields.
-  repeated EmailAttachmentV2 attachments = 10;
+  repeated EmailPart attachments = 10;
+  repeated EmailPart html_related_parts = 11; // MIME parts which are multipart/related to the HTML body.
 }
 
-message EmailAttachmentV2 {
+// A MIME part of an SMPT email, usable for attached files or inline images.
+message EmailPart {
   bytes data = 1;
   string content_disposition = 2; // The Content-Disposition header, including parameters
   string content_type = 3; // The Content-Type header, including parameters
+  string content_id = 4; // The Content-ID header, including parameters
+  string data_file_path = 5; // The server-side path to the file containing the data (mutually exclusive with "data").
 }
 
 message HandleNotificationPayload {

app/backend/pyproject.toml

@@ -139,7 +139,7 @@ module = ["couchers.migrations.versions.*"]
 ignore_errors = true
 
 [[tool.mypy.overrides]]
-module = ["http_ece", "py_vapid", "luhn", "sqlalchemy_utils.*", "growthbook", "user_agents", "pyroscope"]
+module = ["http_ece", "py_vapid", "luhn", "sqlalchemy_utils.*", "growthbook", "user_agents", "pyroscope", "ics"]
 # These libraries don't have type stubs or py.typed markers
 ignore_missing_imports = true
 

app/backend/src/couchers/abuse.py

@@ -0,0 +1,55 @@
+from couchers.context import CouchersContext
+from couchers.db import session_scope
+from couchers.metrics import observe_nonvisible_user_access
+from couchers.models import (
+    NonvisibleUserAccess,
+    NonvisibleUserAccessType,
+    NonvisibleUserState,
+    User,
+)
+
+
+def nonvisible_user_state(user: User) -> NonvisibleUserState | None:
+    if user.banned_at is not None:
+        return NonvisibleUserState.banned
+    if user.shadowed_at is not None:
+        return NonvisibleUserState.shadowed
+    if user.deleted_at is not None:
+        return NonvisibleUserState.deleted
+    return None
+
+
+def maybe_log_nonvisible_user_access(
+    context: CouchersContext,
+    user: User,
+    *,
+    access_type: NonvisibleUserAccessType,
+    actor_user_id: int | None,
+) -> None:
+    target_state = nonvisible_user_state(user)
+    if target_state is None:
+        return
+
+    if actor_user_id == user.id:
+        ip_address = context.get_header("x-couchers-real-ip")
+        user_agent = context.get_header("user-agent")
+        sofa = context._sofa
+    else:
+        ip_address = None
+        user_agent = None
+        sofa = None
+
+    with session_scope() as session:
+        session.add(
+            NonvisibleUserAccess(
+                access_type=access_type,
+                target_user_id=user.id,
+                target_state=target_state,
+                actor_user_id=actor_user_id,
+                ip_address=ip_address,
+                user_agent=user_agent,
+                sofa=sofa,
+            )
+        )
+
+    observe_nonvisible_user_access(access_type, target_state)

app/backend/src/couchers/config.py

@@ -108,10 +108,6 @@ class Config:
     LISTMONK_API_USERNAME: str
     LISTMONK_API_KEY: str
     LISTMONK_LIST_ID: int
-    # Google recaptcha antibot (gated at runtime by the `recaptcha_enabled` feature flag)
-    RECAPTHCA_PROJECT_ID: str
-    RECAPTHCA_API_KEY: str
-    RECAPTHCA_SITE_KEY: str
     # Whether we're in test
     IN_TEST: bool = False
     # Dev-only override file; when set, flags are read from it instead of GrowthBook.
@@ -196,8 +192,8 @@ def check(self) -> None:
                 raise Exception("Listmonk credentials must be configured in production")
 
             # The following features are gated at runtime by feature flags (`strong_verification_enabled`,
-            # `postal_verification_enabled`, `recaptcha_enabled`), which can be flipped on remotely at any
-            # time, so prod must always have their credentials present.
+            # `postal_verification_enabled`), which can be flipped on remotely at any time, so prod must
+            # always have their credentials present.
             if not self.IRIS_ID_PUBKEY or not self.IRIS_ID_SECRET or not self.VERIFICATION_DATA_PUBLIC_KEY:
                 raise Exception("Iris ID credentials must be configured in production")
             if (
@@ -208,8 +204,6 @@ def check(self) -> None:
                 or not self.MYPOSTCARD_CAMPAIGN_ID
             ):
                 raise Exception("MyPostcard API credentials must be configured in production")
-            if not self.RECAPTHCA_PROJECT_ID or not self.RECAPTHCA_API_KEY or not self.RECAPTHCA_SITE_KEY:
-                raise Exception("reCAPTCHA credentials must be configured in production")
 
             if self.FEATURE_FLAGS_FILE_OVERRIDE_PATH:
                 raise Exception("FEATURE_FLAGS_FILE_OVERRIDE_PATH is dev-only and must not be set in production")

app/backend/src/couchers/context.py

@@ -4,6 +4,7 @@
 
 from couchers import experimentation
 from couchers.i18n import LocalizationContext
+from couchers.i18n.locales import get_translation_component
 
 if TYPE_CHECKING:
     from growthbook import GrowthBook
@@ -122,17 +123,30 @@ def abort(self, status_code: grpc.StatusCode, error_message: str) -> NoReturn:
             context.abort(status_code, error_message)
 
     def abort_with_error_code(
-        self, status_code: grpc.StatusCode, error_message_id: str, *, substitutions: dict[str, str | int] | None = None
+        self,
+        status_code: grpc.StatusCode,
+        error_message_id: str,
+        *,
+        substitutions: dict[str, str | int] | None = None,
     ) -> NoReturn:
         """
         Raises an error that's returned to the user, but error_message_id should be an entry from translateable errors
+
+        error_message_id may be namespaced with a translation component, like i18next, e.g. "admin:object_not_found"
+        looks up "object_not_found" in the "admin" component (where admin/editor errors live). Without a prefix the
+        "main" component is used.
         """
         if not self.__is_interactive:
             raise NonInteractiveAbortException(status_code, error_message_id)
         else:
             context = cast(grpc.ServicerContext, self._grpc_context)
+            component, _, error_name = error_message_id.rpartition(":")
             # Get the translated error message using the user's language preference
-            error_message = self.localization.localize_string(f"errors.{error_message_id}", substitutions=substitutions)
+            error_message = self.localization.localize_string(
+                f"errors.{error_name}",
+                i18next=get_translation_component(component or "main"),
+                substitutions=substitutions,
+            )
             context.abort(status_code, error_message)
 
     def set_cookies(self, cookies: list[str]) -> None: