Harden tenant authorization and repair password-reset delivery

.github/copilot-instructions.md

@@ -60,6 +60,8 @@ make migrate        # Alembic upgrade head
 1. Run tests after every code change. After any edit to code or tests, run `make test-unit` (or `make test-unit-fast` during iteration). The change is not "done" until local tests pass. Run `make test-all` before pushing a PR.
 2. Commit and let CI run. After local tests pass, commit and push. Do not declare a change shippable based on local results alone — wait for CI on the branch.
 3. Merge only when CI is green. A PR may merge only after CI passes. If CI is red, fix the cause before merging. Do not bypass, force-merge, or skip required checks.
+4. Require successful Ollama AI review for the current PR head/base. Use `glm-5.3-flash` by default; see `docs/ai-pr-review.md`. Missing or skipped review is not approval.
+5. Builder agents may merge only when GitHub reports mergeable and all required checks/reviews pass. Reviewer agents must not merge.
 
 ## Testing rules
 

AGENTS.md

@@ -84,6 +84,8 @@ Before declaring a change done:
 1. Run tests after every code change. After any edit to code or tests, run `make test-unit` (or `make test-unit-fast` during iteration). The change is not "done" until local tests pass. Run `make test-all` before pushing a PR.
 2. Commit and let CI run. After local tests pass, commit and push. Do not declare a change shippable based on local results alone — wait for CI on the branch.
 3. Merge only when CI is green. A PR may merge only after CI passes. If CI is red, fix the cause before merging. Do not bypass, force-merge, or skip required checks.
+4. Require successful Ollama AI review for the current PR head/base. Use `glm-5.3-flash` by default; see `docs/ai-pr-review.md`. Missing or skipped review is not approval.
+5. Builder agents may merge only when GitHub reports mergeable and all required checks/reviews pass. Reviewer agents must not merge.
 
 ## Testing rules
 

CLAUDE.md

@@ -105,6 +105,8 @@ tests/setup_test_data.py       # Seed data for test DB
 
 **Multi-tenancy:** Every query MUST filter by `org_id`. Use `verify_org_member(person, org_id)` from `api/dependencies.py` to enforce org isolation.
 
+**Organizations:** Keep `POST /api/v1/organizations/` public only for creating an empty onboarding organization. Require membership for organization reads/listing and same-tenant admin access for update/delete/cancel/restore. Commit each lifecycle mutation and its audit record together. Public signup membership hardening remains tracked in #255.
+
 **RBAC:** Two roles: `volunteer` (view own data, manage availability) and `admin` (full CRUD, solver, invitations). Roles stored as JSON array on Person model.
 
 **Test auth mocking:** Unit tests auto-mock authentication via `conftest.py` (returns a test admin user). Integration tests use real auth. Mark tests with `@pytest.mark.no_mock_auth` to opt out of mocking.
@@ -119,68 +121,29 @@ Pytest markers: `@pytest.mark.unit`, `@pytest.mark.integration`, `@pytest.mark.s
 
 1. **Run tests after every code change.** After any edit to code or tests, run `make test-unit` (or `make test-unit-fast` during iteration). The change is not "done" until local tests pass. Run `make test-all` before pushing a PR.
 2. **Commit and let CI run.** After local tests pass, commit and push. Do not declare a change shippable based on local results alone — wait for CI on the branch.
-3. **Merge only when CI is green and Codex local review reports no blocking issues** (see next section).
-
-## PR Workflow With Codex Review
-
-PR review is run automatically by `openai/codex-action` in CI
-(`.github/workflows/codex-review.yml`). On every PR push, the action
-checks out the merge ref, runs Codex against the diff, and posts the
-verdict as a PR comment.
+3. **Merge only when CI and Ollama AI review pass and GitHub reports mergeable** (see next section).
 
-Prereq: the `OPENAI_API_KEY` repo secret must be set. Without it the
-precondition job emits a warning and skips review; the rest of CI
-still runs.
+## AI PR Review
 
-If you need to re-run a review (e.g., after fixing a finding), just
-push another commit — the workflow's concurrency group cancels the
-prior run and starts a fresh review on the new head.
+Run AI review through `.github/workflows/codex-review.yml` using Ollama Cloud,
+not `openai/codex-action`. Default to `glm-5.3-flash` at
+`https://ollama.com/api/chat`; configure `OLLAMA_API_KEY` as a GitHub Actions
+secret. Override the model or full chat endpoint with repository variables
+`OLLAMA_MODEL` and `OLLAMA_ENDPOINT`. See [setup and limits](docs/ai-pr-review.md).
 
-For local iteration before pushing, the legacy
-`openai/codex-plugin-cc` plugin still works:
+Require a successful `codex-pr-review-gate` result for the current PR head/base.
+Treat missing credentials, missing/binary/truncated patches, stale commits,
+provider errors, malformed responses, and blocking findings as failed review.
+Do not self-approve or treat a skipped review as approval.
 
-```
-git fetch origin main
-/codex:review --base origin/main
-```
+Builder agents may merge only after CI and AI review pass, GitHub reports the
+PR mergeable, and all required reviews/comments/conflicts are resolved.
+Reviewer agents must not merge. Keep a blocked PR open and fix or report the
+blocker; do not bypass checks or close the PR as a substitute for merging.
 
-Use it when you want a verdict without opening a PR, or when CI is
-unavailable. For routine PR review, lean on the CI action — it runs
-without anyone having to remember.
-
-Do not self-approve by posting `LGTM` markers.
-Do not require or wait for the old GitHub `codex-pr-review-gate` check.
-
-A PR may merge only when:
-1. CI is green.
-2. GitHub says the PR is mergeable.
-3. Codex local review reports no blocking issues.
-4. There are no unresolved review comments or merge conflicts.
-
-If Codex review reports blockers:
-1. Keep the PR open.
-2. Fix the issues.
-3. Run relevant local checks.
-4. Push a follow-up commit.
-5. Run Codex review again.
-
-If the PR has merge conflicts:
-1. Update the branch against the latest base branch.
-2. Resolve conflicts carefully.
-3. Run relevant local checks.
-4. Push the resolution.
-5. Run Codex review again.
-
-If CI passes and Codex review passes:
-- Merge the PR using the repository's normal merge method.
-- Do not manually close the PR as the success path.
-
-If GitHub blocks the merge:
-- Report the exact blocker.
-- Leave the PR open.
-
-Only close without merging if the work is abandoned, duplicated, or superseded,
-and leave a PR comment explaining why.
+Do not enable a required check in GitHub protection until its workflow has
+landed on the default branch and the check has appeared on a PR. Branch
+protection/ruleset configuration remains a separate administrative step.
 
 ## Common Gotchas
 

Makefile

@@ -1,22 +1,21 @@
 .PHONY: run dev stop restart setup install migrate test test-backend test-integration test-all test-coverage test-unit test-unit-fast test-unit-file test-with-timing clean clean-all pre-commit help check-poetry check-python check-deps install-poetry install-deps up down build logs shell db-shell redis-shell test-docker migrate-docker restart-api ps clean-docker check-docker ensure-test-deps prepare-test-data ensure-test-env
 
-export SKIP_TEST_DB_FIXTURES ?= true
+export SKIP_TEST_DB_FIXTURES ?= false
 
 TEST_SERVER_HOST ?= 0.0.0.0
 TEST_SERVER_PORT ?= 8000
 TEST_APP_URL ?= http://localhost:$(TEST_SERVER_PORT)
 TEST_API_BASE ?= $(TEST_APP_URL)/api
 
-# tests/conftest.py sets TESTING_FORCE_MEMORY=true, which makes
-# api/database.py bind to /tmp/signupflow_test.db no matter what
-# DATABASE_URL says. Point the Makefile at that same file so `rm` and
-# `setup_test_data` actually reset the database the suite reads. Using
-# ./test_roster.db here meant every reset was a no-op and the real DB
-# accumulated rows across runs until fixed-ID tests collided with 409.
-TEST_DB_PATH := /tmp/signupflow_test.db
+# Share one disposable database across this invocation's test tiers.
+# Independent make/pytest runs must never reset another checkout's database.
+TEST_DB_PATH := $(shell mktemp -d /tmp/signupflow-tests.XXXXXX)/signupflow_test.db
 TEST_DB_PATH_STRIPPED := $(patsubst /%,%,$(TEST_DB_PATH))
 TEST_DB_URL := sqlite:////$(TEST_DB_PATH_STRIPPED)
-export DATABASE_URL ?= $(TEST_DB_URL)
+ifneq ($(filter test% pre-commit prepare-test-data ensure-test-env,$(MAKECMDGOALS)),)
+export SIGNUPFLOW_TEST_DATABASE_URL := $(TEST_DB_URL)
+export DATABASE_URL := $(TEST_DB_URL)
+endif
 
 # Detect available Docker Compose command (v1 `docker-compose` or v2 `docker compose`)
 DOCKER_COMPOSE := $(shell \

README.md

@@ -370,6 +370,10 @@ Billing (Stripe), email (SendGrid), SMS (Twilio), and notification routers are n
 
 ## Testing
 
+Use the [church and basketball operational playbooks](docs/playbooks/README.md)
+for six-week acceptance scenarios, reproducible API/browser tests, and explicit
+manual release checks.
+
 ### Test Pyramid: 413 tests
 
 ```bash

api/core/models.py

@@ -163,6 +163,7 @@ class Assignment(BaseModel):
 
     event_id: str
     assignees: list[str]
+    assigned_roles: dict[str, str] = Field(default_factory=dict)
     resource_id: str | None = None
     team_ids: list[str] = Field(default_factory=list)
 

api/core/solver/heuristics.py

@@ -33,7 +33,7 @@ def __init__(self) -> None:
         self.change_min_enabled: bool = False
         self.change_min_weight: int = 100
         # Loose match (event_id, person_id) — see specs/020-solver-quality-changemin.
-        # Solver writes Assignment.role=NULL so a role-strict match would never hit.
+        # Match legacy published assignments too, which may have no saved role.
         self._prior_published_keys: set[tuple[str, str]] = set()
 
     def build_model(self, context: SolveContext) -> None:
@@ -175,7 +175,7 @@ def _assign_event(
         # Assign people to roles. Re-binds the name declared above; the
         # no-required-roles branch always returns before reaching here.
         assignees = []
-        people_map = {p.id: p for p in self.context.people}
+        assigned_roles: dict[str, str] = {}
 
         for req_role in required_roles:
             candidates = [p for p in self.context.people if req_role.role in p.roles]
@@ -185,6 +185,11 @@ def _assign_event(
             for person in candidates:
                 if person.id in assignees:
                     continue  # Already assigned to this event
+                if any(
+                    event.start < prior.end and event.end > prior.start
+                    for prior in person_events.get(person.id, [])
+                ):
+                    continue
 
                 # Skip if person is on vacation/time-off covering the event date.
                 # Vacation periods are inclusive on both ends.
@@ -234,10 +239,11 @@ def _assign_event(
             scored.sort(key=lambda x: x[0])
             for i in range(min(req_role.count, len(scored))):
                 assignees.append(scored[i][1].id)
+                assigned_roles[scored[i][1].id] = req_role.role
 
         # Check if we met role requirements
         for req_role in required_roles:
-            count = sum(1 for pid in assignees if req_role.role in people_map[pid].roles)
+            count = sum(1 for role in assigned_roles.values() if role == req_role.role)
             if count < req_role.count:
                 violations.hard.append(
                     Violation(
@@ -251,6 +257,7 @@ def _assign_event(
         return Assignment(
             event_id=event.id,
             assignees=assignees,
+            assigned_roles=assigned_roles,
             resource_id=event.resource_id,
             team_ids=event.team_ids,
         )

api/database.py

@@ -13,10 +13,6 @@
 
 # Database URL - can be configured via environment variable
 DATABASE_URL = os.getenv("DATABASE_URL", "sqlite:///./roster.db")
-# FORCE MEMORY FOR DEBUGGING
-# DATABASE_URL = "sqlite:///:memory:"
-if os.getenv("TESTING_FORCE_MEMORY") == "true":
-    DATABASE_URL = "sqlite:////tmp/signupflow_test.db"
 
 # Create engine with SQLite optimizations
 connect_args = {}

api/dependencies.py

@@ -1,6 +1,6 @@
 """Shared FastAPI dependencies for authentication and authorization."""
 
-from fastapi import Depends, HTTPException, Query, status
+from fastapi import Depends, HTTPException, status
 from fastapi.security import HTTPAuthorizationCredentials, HTTPBearer
 from sqlalchemy.orm import Session
 
@@ -13,18 +13,8 @@
 
 
 def check_admin_permission(person: Person) -> bool:
-    """Check if person has admin or super_admin role."""
-    if not person or not person.roles:
-        print(
-            f"DEBUG: check_admin_permission failed. Person: {person}, Roles: {person.roles if person else 'None'}"
-        )
-        return False
-    is_admin = "admin" in person.roles or "super_admin" in person.roles
-    if not is_admin:
-        print(
-            f"DEBUG: check_admin_permission failed. Person: {person.email}, Roles: {person.roles}"
-        )
-    return is_admin
+    """Grant administrative access only for the documented admin role."""
+    return bool(person and person.roles and "admin" in person.roles)
 
 
 def get_person_by_id(
@@ -53,17 +43,6 @@ def get_organization_by_id(
     return org
 
 
-def verify_admin_access(
-    person_id: str = Query(..., description="Person ID"),
-    db: Session = Depends(get_db),
-) -> Person:
-    """Verify person exists and has admin permissions."""
-    person = get_person_by_id(person_id, db)
-    if not check_admin_permission(person):
-        raise HTTPException(status_code=status.HTTP_403_FORBIDDEN, detail="Admin access required")
-    return person
-
-
 def verify_org_member(
     person: Person,
     org_id: str,
@@ -147,3 +126,10 @@ async def get_current_admin_user(current_user: Person = Depends(get_current_user
     if not check_admin_permission(current_user):
         raise HTTPException(status_code=status.HTTP_403_FORBIDDEN, detail="Admin access required")
     return current_user
+
+
+async def verify_admin_access(
+    current_user: Person = Depends(get_current_user),
+) -> Person:
+    """Authenticate legacy callers with the JWT admin dependency."""
+    return await get_current_admin_user(current_user)

api/routers/analytics.py

@@ -119,7 +119,12 @@ def get_schedule_health(
         "latest_solution": {
             "id": latest_solution.id,
             "health_score": latest_solution.health_score,
-            "assignment_count": latest_solution.assignment_count,
+            "assignment_count": (
+                db.query(Assignment)
+                .join(Solution, Assignment.solution_id == Solution.id)
+                .filter(Solution.org_id == org_id, Solution.id == latest_solution.id)
+                .count()
+            ),
             "created_at": latest_solution.created_at.isoformat(),
         }
         if latest_solution

api/routers/assignments.py

@@ -11,6 +11,8 @@
  `GET /events/assignments/all` for org-wide listing).
 """
 
+from typing import cast
+
 from fastapi import APIRouter, BackgroundTasks, Depends, HTTPException, Request, status
 from sqlalchemy.orm import Session
 
@@ -24,14 +26,24 @@
 )
 from api.schemas.common import ListResponse, PaginationParams, get_pagination_params
 from api.services import event_bus
+from api.services.assignment_visibility import member_visible_assignment
 from api.utils.audit_logger import log_audit_event
 
 router = APIRouter(prefix="/assignments", tags=["assignments"])
 
 
 def _load_own_assignment(assignment_id: int, current_user: Person, db: Session) -> Assignment:
     """Load an assignment that belongs to the caller; 404 if missing, 403 if not theirs."""
-    assignment = db.query(Assignment).filter(Assignment.id == assignment_id).first()
+    assignment = (
+        db.query(Assignment)
+        .join(Event, Assignment.event_id == Event.id)
+        .filter(
+            Assignment.id == assignment_id,
+            Event.org_id == current_user.org_id,
+            member_visible_assignment(cast(str, current_user.org_id)),
+        )
+        .first()
+    )
     if not assignment:
         raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="Assignment not found")
     if assignment.person_id != current_user.id:
@@ -178,6 +190,7 @@ def list_my_assignments(
         .filter(
             Assignment.person_id == current_user.id,
             Event.org_id == current_user.org_id,
+            member_visible_assignment(cast(str, current_user.org_id)),
         )
     )
     total = base.count()