From 0372f33ae82baad52ccfd36a10415247f0b9cee6 Mon Sep 17 00:00:00 2001 From: JP Date: Wed, 6 May 2026 19:03:54 -0400 Subject: [PATCH 01/35] feat(pebble): kill switch on internal-key writes (PEBBLE_WRITES_DISABLED) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit First slice of Pebble 1.0 plan §0.5 — runtime flag that halts Pebble's internal-key write paths during incidents without a redeploy. Reads via the same internal key and all JWT-authenticated user requests are unaffected. What changed: - auth.py: new `_pebble_writes_disabled()` reads env on every call so flipping the flag doesn't require a deploy. Check fires after the hmac key validation succeeds and only on POST/PUT/PATCH/DELETE. GET/HEAD/OPTIONS via internal key still work, so research/lookup paths keep functioning during a write-side incident. Wrong-key callers fall through to require_auth as before — the kill switch doesn't leak its own state to unauthenticated traffic. - tests/test_pebble_kill_switch.py: 23 tests covering 5 invariants (switch off / writes blocked / reads pass / JWT unaffected / wrong-key falls through), parameterized across truthy/falsy env values and all four write methods. All passing. Plan ref: tasks/pebble-overhaul-plan.md §0.5 Adversary ref: tasks/pebble-adversary-security.md S2 Co-Authored-By: Claude Opus 4.7 (1M context) --- financial_forecasting/auth.py | 35 ++++ .../tests/test_pebble_kill_switch.py | 163 ++++++++++++++++++ 2 files changed, 198 insertions(+) create mode 100644 financial_forecasting/tests/test_pebble_kill_switch.py diff --git a/financial_forecasting/auth.py b/financial_forecasting/auth.py index e0ad7c9f..2f071386 100644 --- a/financial_forecasting/auth.py +++ b/financial_forecasting/auth.py @@ -142,6 +142,18 @@ async def get_current_user_dep(request: Request) -> Optional[Dict]: _BEDROCK_INTERNAL_API_KEY = os.getenv("BEDROCK_INTERNAL_API_KEY", "") +# HTTP methods considered "writes" for the kill switch below. Idempotent reads +# (GET, HEAD, OPTIONS) remain available even when the switch is on so that +# Pebble's research/lookup paths keep working during write-side incidents. +_WRITE_METHODS = frozenset({"POST", "PUT", "PATCH", "DELETE"}) + + +def _pebble_writes_disabled() -> bool: + """Read the kill switch fresh on every call so operators can flip it + without a redeploy. Truthy values: "true" / "1" / "yes" (case-insensitive). + """ + return os.getenv("PEBBLE_WRITES_DISABLED", "").strip().lower() in {"true", "1", "yes"} + async def require_auth_or_internal(request: Request) -> Dict: """Authorize via internal API key (service-to-service) or user JWT. @@ -150,10 +162,33 @@ async def require_auth_or_internal(request: Request) -> Dict: synthetic service user dict. Otherwise falls back to require_auth. Dev mode: if BEDROCK_INTERNAL_API_KEY is empty, internal key check is skipped and only JWT auth is tried. + + Kill switch: if PEBBLE_WRITES_DISABLED env is truthy AND the caller is + using a valid internal key AND the request method is a write + (POST/PUT/PATCH/DELETE), return 503. Reads via internal key and all + JWT-authenticated requests are unaffected. Designed for incident + response — flip the env var to halt service-account writes without a + deploy. """ internal_key = request.headers.get("X-Internal-Key", "") if _BEDROCK_INTERNAL_API_KEY and internal_key: if hmac.compare_digest(internal_key, _BEDROCK_INTERNAL_API_KEY): + if _pebble_writes_disabled() and request.method in _WRITE_METHODS: + logger.warning( + "pebble_writes_disabled: blocking %s %s for service caller", + request.method, + request.url.path, + ) + raise HTTPException( + status_code=503, + detail={ + "error": "pebble_writes_disabled", + "message": ( + "Pebble service-account writes are temporarily " + "disabled. Reads remain available." + ), + }, + ) return { "user_id": "service:pebble", "email": "pebble@internal", diff --git a/financial_forecasting/tests/test_pebble_kill_switch.py b/financial_forecasting/tests/test_pebble_kill_switch.py new file mode 100644 index 00000000..06f503f3 --- /dev/null +++ b/financial_forecasting/tests/test_pebble_kill_switch.py @@ -0,0 +1,163 @@ +"""Tests for the Pebble write kill switch in ``require_auth_or_internal``. + +Plan v1 §0.5 — a runtime flag that blocks Pebble's internal-key write paths +during incidents without requiring a redeploy. Reads via the same internal +key, and all JWT-authenticated requests, are unaffected. + +Invariants locked in here: +1. Switch off → POST with valid internal key returns the synthetic service user. +2. Switch on → POST/PUT/PATCH/DELETE with valid internal key raises 503. +3. Switch on → GET with valid internal key still returns the service user. +4. Switch on → request without internal key falls through to ``require_auth``. +5. Switch on → request with WRONG internal key falls through to ``require_auth`` + (we don't 503 a JWT-authed user just because their cookie happened to ride + alongside a stray header). +""" + +import os +import sys +from unittest.mock import AsyncMock + +import pytest +from fastapi import HTTPException +from starlette.requests import Request + +sys.path.insert(0, os.path.join(os.path.dirname(__file__), '..')) + +import auth + + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + +_TEST_KEY = "test-internal-key-1234567890abcdef" + + +def _make_request(method: str, internal_key: str = "") -> Request: + """Build a minimal Starlette Request scope with the given method and + optional X-Internal-Key header. Avoids the TestClient round-trip — we're + unit-testing a dependency, not an endpoint. + """ + headers = [] + if internal_key: + headers.append((b"x-internal-key", internal_key.encode())) + scope = { + "type": "http", + "method": method, + "headers": headers, + "path": "/api/pebble-test", + "query_string": b"", + } + return Request(scope) + + +@pytest.fixture +def fixed_internal_key(monkeypatch): + """Pin the module-level ``_BEDROCK_INTERNAL_API_KEY`` to a known value so + the hmac.compare_digest path is exercised. Module-level constants in + ``auth.py`` read os.environ at import time, so we patch the symbol + directly rather than the env var. + """ + monkeypatch.setattr(auth, "_BEDROCK_INTERNAL_API_KEY", _TEST_KEY) + return _TEST_KEY + + +@pytest.fixture +def switch_off(monkeypatch): + monkeypatch.delenv("PEBBLE_WRITES_DISABLED", raising=False) + + +@pytest.fixture +def switch_on(monkeypatch): + monkeypatch.setenv("PEBBLE_WRITES_DISABLED", "true") + + +# --------------------------------------------------------------------------- +# Helper-function unit tests +# --------------------------------------------------------------------------- + +def test_pebble_writes_disabled_default_off(monkeypatch): + monkeypatch.delenv("PEBBLE_WRITES_DISABLED", raising=False) + assert auth._pebble_writes_disabled() is False + + +@pytest.mark.parametrize("value", ["true", "TRUE", "True", "1", "yes", " true "]) +def test_pebble_writes_disabled_truthy_values(monkeypatch, value): + monkeypatch.setenv("PEBBLE_WRITES_DISABLED", value) + assert auth._pebble_writes_disabled() is True + + +@pytest.mark.parametrize("value", ["false", "0", "no", "", "off", "disabled"]) +def test_pebble_writes_disabled_falsy_values(monkeypatch, value): + monkeypatch.setenv("PEBBLE_WRITES_DISABLED", value) + assert auth._pebble_writes_disabled() is False + + +# --------------------------------------------------------------------------- +# Behavior tests for require_auth_or_internal +# --------------------------------------------------------------------------- + +@pytest.mark.asyncio +async def test_internal_key_write_succeeds_when_switch_off(fixed_internal_key, switch_off): + """Invariant 1: switch off → POST with valid key returns the service user.""" + request = _make_request("POST", fixed_internal_key) + user = await auth.require_auth_or_internal(request) + assert user["is_service"] is True + assert user["user_id"] == "service:pebble" + assert user["email"] == "pebble@internal" + + +@pytest.mark.parametrize("method", ["POST", "PUT", "PATCH", "DELETE"]) +@pytest.mark.asyncio +async def test_internal_key_writes_blocked_when_switch_on( + fixed_internal_key, switch_on, method, +): + """Invariant 2: switch on → all write methods 503 with the structured + error payload that callers (e.g. crm_bridge.py) can branch on.""" + request = _make_request(method, fixed_internal_key) + with pytest.raises(HTTPException) as exc_info: + await auth.require_auth_or_internal(request) + assert exc_info.value.status_code == 503 + assert exc_info.value.detail["error"] == "pebble_writes_disabled" + + +@pytest.mark.parametrize("method", ["GET", "HEAD", "OPTIONS"]) +@pytest.mark.asyncio +async def test_internal_key_reads_succeed_when_switch_on( + fixed_internal_key, switch_on, method, +): + """Invariant 3: switch on → reads via internal key still work, so + Pebble's research/lookup paths keep functioning during write incidents. + """ + request = _make_request(method, fixed_internal_key) + user = await auth.require_auth_or_internal(request) + assert user["is_service"] is True + + +@pytest.mark.asyncio +async def test_jwt_write_unaffected_by_switch(fixed_internal_key, switch_on, monkeypatch): + """Invariant 4: switch on but no internal key → falls through to + ``require_auth``. JWT-authed users still write normally. + """ + fake_user = {"user_id": "u-1", "email": "real-user@pursuit.org", "is_service": False} + monkeypatch.setattr(auth, "require_auth", AsyncMock(return_value=fake_user)) + request = _make_request("POST") # no X-Internal-Key + user = await auth.require_auth_or_internal(request) + assert user is fake_user + + +@pytest.mark.asyncio +async def test_wrong_internal_key_falls_through_when_switch_on( + fixed_internal_key, switch_on, monkeypatch, +): + """Invariant 5: switch on but key is wrong → don't 503, fall through to + ``require_auth``. A 503 here would leak that the kill switch is on to + unauthenticated callers and would penalize JWT users whose client + happened to send a stale header. + """ + fake_user = {"user_id": "u-2", "email": "another@pursuit.org", "is_service": False} + monkeypatch.setattr(auth, "require_auth", AsyncMock(return_value=fake_user)) + request = _make_request("POST", "wrong-key-not-the-real-one") + user = await auth.require_auth_or_internal(request) + assert user is fake_user From 6e24d4d6272d876a05f329e6ac07a5a008c90760 Mon Sep 17 00:00:00 2001 From: JP Date: Wed, 6 May 2026 19:25:14 -0400 Subject: [PATCH 02/35] =?UTF-8?q?docs(pebble):=20unified=20search=20spec?= =?UTF-8?q?=20=E2=80=94=20synthesizes=203=20deep=20design=20passes?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three independently-grounded specs (backend, ux, security) at tasks/pebble-search-spec-{backend,ux,security}.md each ~3 pages. This file is the build order — picks the load-bearing decisions, links to the deep specs for argued tradeoffs. Locked decisions: * Backend = PG FTS + pgvector overlay; one denormalized bedrock.search_doc table; queue-drain indexer; pre-filter permission via accessible-id subquery. * X-Originating-User mandatory on every internal-key write + search; Pebble becomes a delegated principal not a god principal. * Two audit tables: pebble_write_audit (writes) + search_audit (queries). 90d hot, 13mo Parquet archive. Admin reveals of query_text logged to a separate table that survives RTBF. * GlobalSearch.tsx: dual-mode (Find | Ask), permanent footer chip, ?/ prefix triggers, /pebble sidebar entry. * Multi-tenant org_id from day 1 as outermost predicate. * Trace propagation FE → Bedrock → Pebble → DB. * Circuit breakers per backend with explicit fallback order. * SLO p95 200ms Find / 3s first-token Ask. * No SaaS data egress for v1.0 — all in-house. Co-Authored-By: Claude Opus 4.7 (1M context) --- tasks/pebble-search-spec.md | 136 ++++++++++++++++++++++++++++++++++++ 1 file changed, 136 insertions(+) create mode 100644 tasks/pebble-search-spec.md diff --git a/tasks/pebble-search-spec.md b/tasks/pebble-search-spec.md new file mode 100644 index 00000000..326eeaf6 --- /dev/null +++ b/tasks/pebble-search-spec.md @@ -0,0 +1,136 @@ +# Pebble 1.0 — Enterprise-Grade Search: Unified Spec + +> **Status:** Build spec. Synthesized from `pebble-search-spec-backend.md`, `pebble-search-spec-ux.md`, `pebble-search-spec-security.md`. Read those for depth; this doc is the build order. +> **Date:** 2026-05-06 +> **Anchored against:** `origin/main`. Built on `feat/pebble-phase-0` worktree. +> **Bar:** "Enterprise level tool out of the box" — JP, 2026-05-06. No shortcuts. + +## Decisions, locked + +These are the load-bearing calls. Companion specs argue them; this doc commits. + +1. **Backend = Postgres FTS (tsvector + GIN) for v1.0, pgvector embeddings as additive overlay.** One denormalized `bedrock.search_doc` table, queue-drained indexer, pg_trgm for typo tolerance. Data stays in-house. (`backend §1`) +2. **Permission model = pre-filter via accessible-id subquery in SQL, NEVER post-filter in Python.** Per-request resolution, request-scoped cache, no cross-request cache. (`security §1`, `backend §4`) +3. **`X-Originating-User` is mandatory on every internal-key write/search.** Reject service calls without it. Pebble becomes a delegated principal, not a god principal. (`security §1.5`) +4. **Two audit tables.** `bedrock.pebble_write_audit` (writes) + `bedrock.search_audit` (queries). 90d hot, 13mo Parquet archive. Full `query_text` stored, admin-gated read, access logged separately. (`security §2`) +5. **One bar, two modes.** GlobalSearch.tsx extends to `Find | Ask` segmented + permanent footer chip + `?`/`/` prefix. Inline takeover for Ask responses (NOT slide-over). Sidebar `/pebble` entry using the unused `Sparkles` import. (`ux §1, §6, §7`) +6. **Multi-tenant column from day 1.** `org_id` is the outermost predicate everywhere, default `'pursuit'`. CI grep enforces. (`security §7`) +7. **Trace propagation.** `X-Trace-Id` (UUIDv7) FE → Bedrock → Pebble → DB. 100% sampling at 1.0 scale. (`security §5`) +8. **Circuit breakers per backend** with explicit graceful-degradation order: `pgvector → postgres_fts → sf_sosl_fallback → empty + banner`. Permission resolver fails closed. (`security §6`, `backend §8`) +9. **Latency SLO p95 ≤ 200ms Find, p95 ≤ 3s first-token Ask.** `min-instances=1` on Cloud Run, dedicated read-only asyncpg pool for search. (`backend §5`) +10. **No SaaS data egress.** No Algolia, no external embeddings until vetted. pgvector + Voyage-3 embeddings (data-residency TBD; defer to v1.1). (`backend §1`) + +## Architecture sketch + +``` +Frontend GlobalSearch (Find | Ask) + │ /api/search?q=...&types=...&facets=... /api/pebble/ask + ▼ ▼ +Bedrock :8000 ──── routes/search.py ──── routes/pebble_proxy.py + │ permission resolver (request-scoped) │ proxy + cost gate + │ search_crm tool gateway ▼ + ▼ Pebble :8001 +PostgreSQL (Cloud SQL) │ router.classify_query + │ bedrock.search_doc (GIN tsvector + pgvector)│ L1+ tools call /api/search + │ bedrock.search_index_queue (LISTEN/NOTIFY) │ via crm_bridge.py + X-Originating-User + │ bedrock.search_audit ▼ + │ bedrock.pebble_write_audit streams response + │ bedrock.sf_*_mirror (thin SF mirrors) + ▼ +Async indexer worker (asyncio task in main.py lifespan) +``` + +## Build order + +Each item: own commit on `feat/pebble-phase-0`, tests included, no shortcuts. Sequence respects dependencies. + +### Phase 0 — Foundation (entry condition for everything else) + +| # | Item | Depends on | Status | +|---|---|---|---| +| 0.5 | PEBBLE_WRITES_DISABLED kill switch in `auth.py` | — | ✅ DONE (commit 0372f33) | +| 0.6 | BEDROCK_API_URL startup assertion | — | next | +| 0.1 | `bedrock.pebble_write_audit` migration + idempotency UNIQUE on request_id | — | | +| 0.2 | X-Originating-User enforcement in `auth.py:require_auth_or_internal` | 0.1 | | +| 0.3 | Per-service rate limit (custom key_func keyed on `is_service`) | 0.2 | | +| 0.4 | Internal-key scopes in synthetic user dict | 0.2 | | +| 0.7 | Idempotency middleware (`X-Request-Id`, 24h replay window via 0.1's UNIQUE) | 0.1, 0.2 | | +| 0.9 | `services/sf_stages.py` canonical stage source + `bedrock.sf_picklist_cache` | — | | +| 0.10 | frontend-v2 vitest infrastructure + 1 sample test + CI step | — | | +| 0.8 | Audit-log integration in existing `logger.info` lines | 0.2 | | + +### Layer 1 — Search infrastructure + +| # | Item | Depends on | +|---|---|---| +| 1.1 | `bedrock.search_doc` table + GIN/trgm/vector indexes migration | Phase 0 done | +| 1.2 | `bedrock.search_index_queue` table + trigger function migration | 1.1 | +| 1.3 | `bedrock.search_audit` table migration | 0.1 | +| 1.4 | Thin SF mirror tables (`sf_account_mirror`, `sf_contact_mirror`, `sf_opportunity_mirror`, `sf_task_mirror`) | 1.2 | +| 1.5 | Mirror sync extension in `data_sync.py` (watermarked, 60s loop) | 1.4 | +| 1.6 | Indexer worker in `main.py` lifespan (LISTEN/NOTIFY + 2s polling fallback) | 1.2 | +| 1.7 | Backfill script for one-shot index population | 1.6 | +| 1.8 | `routes/search.py` — `/api/search` endpoint with permission resolver | 1.1, 0.4 | +| 1.9 | Circuit breakers (`pybreaker`) per backend | 1.8 | +| 1.10 | Test matrix: 4 profiles × all entity types × access scenarios | 1.8 | + +### Layer 2 — Frontend (depends on 1.8 stable) + +| # | Item | +|---|---| +| 2.1 | Vitest config + a11y test harness (`@testing-library/react`, `axe-core`) | +| 2.2 | `GlobalSearch.tsx` dual-mode refactor: segmented Find/Ask, footer Ask chip, `?`/`/` prefix | +| 2.3 | Result card components per entity type with WAI-ARIA Editable Combobox pattern | +| 2.4 | Facet chips (entity, mine/team, recency, status) | +| 2.5 | Saved searches reuse `bedrock.saved_view` with `view_kind` discriminator | +| 2.6 | Recent-search history via `GET /api/search/history` | +| 2.7 | `/pebble` sidebar nav entry (Sparkles icon, AppShell.tsx:11) | +| 2.8 | First-login coachmark for search discoverability | +| 2.9 | Mobile responsive: full-screen sheet < 768px | + +### Layer 3 — Pebble Ask integration (depends on Layer 1 + Pebble alive) + +| # | Item | +|---|---| +| 3.1 | `routes/pebble_proxy.py` — `/api/pebble/ask` (HTTP forward to Pebble :8001) with cost gate | +| 3.2 | Trace + originating-user header propagation | +| 3.3 | New `search_crm` tool in `pebble/tools/` calling `/api/search` | +| 3.4 | Streaming SSE response from Pebble through Bedrock to FE | +| 3.5 | Suggested-action cards (read-only at 3.x; writes deferred) | + +### Layer 4 — Observability + Ops + +| # | Item | +|---|---| +| 4.1 | `X-Trace-Id` middleware in both processes | +| 4.2 | Structured JSON logging | +| 4.3 | Cloud Monitoring custom metrics emission | +| 4.4 | Anomaly batch job (Cloud Scheduler → Cloud Run) | +| 4.5 | Pre-launch checklist verification | + +## Test bar (per item) + +- Unit: every public function on every code path, parameterized for edge cases +- Integration: route-level for every new endpoint +- Permission: matrix across 4 profiles × every entity × access/no-access +- Concurrency: race tests where applicable (singleton, queue drain) +- A11y: axe-core in v2 component tests +- Observability: assert audit row written on every gated path + +## Open product calls deferred to JP + +These don't block building — pick a default, JP overrides. + +1. UX: inline vs slide-over for Ask response — defaulting **inline** per UX agent's argument. +2. `view_contact_email` permission default — defaulting **ON for RM/Exec/PM**. +3. Recent-search retention — defaulting **30 days**. +4. Ask cost cap — defaulting **$5/user/day** (matches existing `pebble_daily_usage`). +5. Anomaly threshold per-profile — defaulting **uniform 100 records/hour**, refine post-launch. +6. v1-frontend deprecation date — defaulting **2026-Q3**. + +## What I don't do without JP's call + +- Open the eventual PR (JP confirms when 1.0 is reached) +- Change SF picklist values +- Apply migrations to prod (only to dev DB / test DB until JP says go) +- Modify any production secret From 2f2367f5342ff59b1a23047bbc2bc7326edda093 Mon Sep 17 00:00:00 2001 From: JP Date: Wed, 6 May 2026 19:25:14 -0400 Subject: [PATCH 03/35] feat(pebble): pebble_write_audit + search_audit_access_log migrations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 0.1 + 0.7 of the Pebble 1.0 plan. bedrock.pebble_write_audit captures every internal-key write with: * request_id (UNIQUE — doubles as 24h replay defense) * route + method + http status + payload_hash + payload * sf_object_type + sf_object_id (subject) * originating_user_email (the human; mandatory upstream via auth.py:require_auth_or_internal — next commit) * service_user (e.g. "service:pebble") * side_effects JSONB (auto-award, activity log, future fanout) * latency_ms + org_id (multi-tenant outer guard) Three independent adversarial reviews flagged the un-attributed "service:pebble" pattern as a 1.0 blocker; this table closes it. bedrock.search_audit_access_log captures admins reading other users' search history with query_text revealed. Append-only; lives separately from search_audit so RTBF/DSAR purges of search_audit cannot also erase the accountability trail. Both grants restricted to bedrock_user; pebble_write_audit has no UPDATE grant (audit immutability for forensics) and no DELETE grant (retention job uses a different role, added when the 90d cleanup script lands). Migration is idempotent. Apply as bedrock owner. Plan ref: tasks/pebble-search-spec.md decisions §3, §4 Adversary refs: pebble-adversary-security.md H3, M3, S2, S3 pebble-adversary-architecture.md #6 pebble-adversary-ux.md #4 Co-Authored-By: Claude Opus 4.7 (1M context) --- .../2026-05-06-pebble-write-audit.sql | 150 ++++++++++++++++++ 1 file changed, 150 insertions(+) create mode 100644 financial_forecasting/db/migrations/2026-05-06-pebble-write-audit.sql diff --git a/financial_forecasting/db/migrations/2026-05-06-pebble-write-audit.sql b/financial_forecasting/db/migrations/2026-05-06-pebble-write-audit.sql new file mode 100644 index 00000000..1eb8bee0 --- /dev/null +++ b/financial_forecasting/db/migrations/2026-05-06-pebble-write-audit.sql @@ -0,0 +1,150 @@ +-- 2026-05-06: bedrock.pebble_write_audit + bedrock.search_audit_access_log +-- +-- Phase 0.1 + 0.7 of the Pebble 1.0 plan (tasks/pebble-search-spec.md). +-- +-- Why: +-- When Pebble (or any service-to-service caller using +-- X-Internal-Key) writes through Bedrock's API, today's audit +-- trail collapses to a single synthetic user "service:pebble" +-- across logger.info lines and SF "LastModifiedById" — the +-- originating end-user is unrecoverable. Three independent +-- reviews (security/UX/architecture adversaries) flagged this +-- as a 1.0 blocker. +-- +-- bedrock.pebble_write_audit captures every internal-key write +-- with the originating user, request id, route, payload hash, +-- and response status. UNIQUE(request_id) doubles as the +-- idempotency / replay-defense store for X-Request-Id (Phase +-- 0.7) — a duplicate request_id within 24h yields a 409 from +-- the API rather than a duplicate write. +-- +-- bedrock.search_audit_access_log captures admins reading +-- other users' search history when query_text is revealed. +-- Lives separately from the search_audit table itself so that +-- RTBF / DSAR purges of search_audit cannot also erase the +-- accountability trail of who looked at what. +-- +-- Related: +-- * tasks/pebble-search-spec.md (decisions §3, §4) +-- * tasks/pebble-search-spec-security.md §1.5, §2, §10.4 +-- * tasks/pebble-overhaul-plan.md §0.1, §0.7 +-- * financial_forecasting/auth.py:require_auth_or_internal +-- (enforces X-Originating-User; this table receives the +-- audit row from each write route) +-- +-- Idempotent — safe to re-run. +-- +-- Apply as bedrock owner: +-- psql "$DATABASE_URL" -f 2026-05-06-pebble-write-audit.sql + +CREATE EXTENSION IF NOT EXISTS "uuid-ossp"; +CREATE EXTENSION IF NOT EXISTS citext; + +CREATE TABLE IF NOT EXISTS bedrock.pebble_write_audit ( + id UUID PRIMARY KEY DEFAULT uuid_generate_v4(), + occurred_at TIMESTAMPTZ NOT NULL DEFAULT now(), + + -- Idempotency / replay defense. Required on every internal-key + -- write; UNIQUE means a 24h replay returns 409 from INSERT. + request_id UUID NOT NULL, + + -- Routing / classification. + route TEXT NOT NULL, -- e.g. "/api/opportunities/update-stage" + http_method TEXT NOT NULL CHECK (http_method IN + ('POST', 'PUT', 'PATCH', 'DELETE')), + + -- Subject. nullable so an early-failure audit row (e.g. invalid + -- payload) still gets written for forensics. + sf_object_type TEXT, -- 'Opportunity' | 'Account' | 'Contact' | 'Task' | 'Payment' | 'Award' | 'Project' + sf_object_id TEXT, + + -- Attribution. originating_user_email is the human whose + -- session triggered the workflow; service_user is "service:pebble" + -- (or future siblings). Both required. + originating_user_email CITEXT NOT NULL, + service_user TEXT NOT NULL DEFAULT 'service:pebble', + + -- What was sent. payload_hash = sha256 of canonical JSON; full + -- payload kept for 90 days where the route opts in (sensitive + -- routes pass NULL to suppress). + payload_hash TEXT, + payload JSONB, + + -- What came back. + response_status SMALLINT NOT NULL, + response_summary JSONB, -- e.g. {"award_created": true, "stage": "..."} + error_class TEXT, -- ExceptionClass name when status >= 400 + + -- Side-effect attestation. ensure_for_opp + future event-bus + -- subscribers write back here so a forensic reader can see the + -- full chain from one INSERT (vs. correlating across 3 tables). + side_effects JSONB, -- e.g. {"award_created": true, "activity_logged": true} + + -- Latency for ops dashboards. + latency_ms INT, + + -- Multi-tenant outer guard. Single value today, present so + -- search_audit / pebble_audit share an enforcement pattern. + org_id TEXT NOT NULL DEFAULT 'pursuit', + + -- Replay defense: the duplicate-request_id detector relies on this. + UNIQUE (request_id) +); + +CREATE INDEX IF NOT EXISTS idx_pebble_write_audit_user_time + ON bedrock.pebble_write_audit(originating_user_email, occurred_at DESC); +CREATE INDEX IF NOT EXISTS idx_pebble_write_audit_object + ON bedrock.pebble_write_audit(sf_object_type, sf_object_id, occurred_at DESC) + WHERE sf_object_id IS NOT NULL; +CREATE INDEX IF NOT EXISTS idx_pebble_write_audit_errors + ON bedrock.pebble_write_audit(occurred_at DESC) + WHERE response_status >= 400; +CREATE INDEX IF NOT EXISTS idx_pebble_write_audit_route_time + ON bedrock.pebble_write_audit(route, occurred_at DESC); + +COMMENT ON TABLE bedrock.pebble_write_audit IS + 'Audit row for every internal-key write through Bedrock (Phase 0.1). UNIQUE(request_id) doubles as 24h replay defense (Phase 0.7).'; +COMMENT ON COLUMN bedrock.pebble_write_audit.originating_user_email IS + 'Mandatory. The human whose session triggered the workflow. Sourced from X-Originating-User header. require_auth_or_internal rejects calls without it.'; +COMMENT ON COLUMN bedrock.pebble_write_audit.request_id IS + 'Mandatory. UUIDv7 from X-Request-Id header. Replay window = 24h via UNIQUE constraint; deduplicate at the route via INSERT...ON CONFLICT (request_id) DO NOTHING RETURNING id.'; + +-- --------------------------------------------------------------------------- +-- Admin access log for revealing search_audit.query_text +-- --------------------------------------------------------------------------- +-- search_audit itself comes in a later migration (Layer 1). This table is +-- separate so RTBF/DSAR purges of search_audit cannot also erase +-- accountability. +CREATE TABLE IF NOT EXISTS bedrock.search_audit_access_log ( + id UUID PRIMARY KEY DEFAULT uuid_generate_v4(), + accessed_at TIMESTAMPTZ NOT NULL DEFAULT now(), + admin_email CITEXT NOT NULL, + target_user_email CITEXT NOT NULL, + revealed_query_text BOOLEAN NOT NULL, + reason TEXT NOT NULL, + request_id UUID NOT NULL, + org_id TEXT NOT NULL DEFAULT 'pursuit' +); + +CREATE INDEX IF NOT EXISTS idx_search_audit_access_admin + ON bedrock.search_audit_access_log(admin_email, accessed_at DESC); +CREATE INDEX IF NOT EXISTS idx_search_audit_access_target + ON bedrock.search_audit_access_log(target_user_email, accessed_at DESC); + +COMMENT ON TABLE bedrock.search_audit_access_log IS + 'Append-only log of admins viewing other users search history with query_text revealed. Survives RTBF purges.'; + +-- --------------------------------------------------------------------------- +-- Grants — bedrock_user owns these tables and writes through the API. +-- --------------------------------------------------------------------------- +DO $$ +BEGIN + IF EXISTS (SELECT 1 FROM pg_roles WHERE rolname = 'bedrock_user') THEN + GRANT SELECT, INSERT ON bedrock.pebble_write_audit TO bedrock_user; + GRANT SELECT, INSERT ON bedrock.search_audit_access_log TO bedrock_user; + -- UPDATE intentionally NOT granted on pebble_write_audit: + -- audit rows are append-only (immutability matters for forensics). + -- DELETE limited to a separate retention job role, added when + -- the 90d cleanup script lands. + END IF; +END $$; From 62743fb558b943069f4effc4542ac2163da894ae Mon Sep 17 00:00:00 2001 From: JP Date: Wed, 6 May 2026 19:25:29 -0400 Subject: [PATCH 04/35] feat(pebble): startup assertion on BEDROCK_API_URL + INTERNAL_API_KEY MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 0.6. Refuses to start when production-shaped Pebble has a misconfigured service-to-service bridge to Bedrock. Failure modes this catches: * BEDROCK_API_URL unset → defaults to http://localhost:8000 in crm_bridge.py:14. Every Pebble write silently routes to localhost in prod and bypasses TLS. * BEDROCK_INTERNAL_API_KEY unset → crm_bridge.py:27-28 sends requests with no internal-key header. Bedrock falls through to JWT auth; every Pebble write fails 401. Production detection mirrors auth.py:25-26 (FRONTEND_URL starts with https://) so dev/staging/prod classify identically across the two FastAPI processes. PEBBLE_ENV=production overrides the FRONTEND_URL check for headless cron deployments. Whitespace-only API keys count as missing — defensive against .env files with `BEDROCK_INTERNAL_API_KEY=" "`. 11 tests covering: dev path no-op, prod path raises on each failure mode, PEBBLE_ENV override, case-insensitive prod detect, whitespace-only-key rejection. Plan ref: tasks/pebble-search-spec.md §6, §10 Adversary ref: pebble-adversary-security.md S4 Co-Authored-By: Claude Opus 4.7 (1M context) --- pebble/main.py | 41 +++++++ pebble/tests/test_bedrock_bridge_config.py | 124 +++++++++++++++++++++ 2 files changed, 165 insertions(+) create mode 100644 pebble/tests/test_bedrock_bridge_config.py diff --git a/pebble/main.py b/pebble/main.py index 03494671..76977278 100644 --- a/pebble/main.py +++ b/pebble/main.py @@ -138,12 +138,53 @@ async def _check(request: Request): logger = logging.getLogger("pebble.main") +def _validate_bedrock_bridge_config() -> None: + """Phase 0.6 — Pebble's bridge to Bedrock must be HTTPS in production + and must carry a real internal API key. Wrong defaults are silent + catastrophic failures: ``BEDROCK_API_URL`` defaults to + ``http://localhost:8000`` and ``BEDROCK_INTERNAL_API_KEY`` defaults + to empty in ``crm_bridge.py:14-17`` — fine for dev, ruinous in prod. + Refuse to start when misconfigured. + + Detection of "production": + FRONTEND_URL is set AND starts with ``https://``. Mirrors + ``auth.py:25-26`` ``IS_PRODUCTION`` exactly so dev / staging / + prod classify identically across the two processes. PEBBLE_ENV + override (``PEBBLE_ENV=production``) wins regardless of + FRONTEND_URL — useful for headless cron deployments that + don't have a frontend URL. + """ + is_prod = ( + (os.getenv("FRONTEND_URL", "") or "").startswith("https://") + or os.getenv("PEBBLE_ENV", "").lower() == "production" + ) + if not is_prod: + return + + bedrock_url = os.getenv("BEDROCK_API_URL", "") + if not bedrock_url.startswith("https://"): + raise RuntimeError( + "BEDROCK_API_URL must be set to an https:// URL in production. " + f"Got: {bedrock_url!r}. Refusing to start — service-to-service " + "writes to a localhost or http:// endpoint in prod silently " + "lose data and bypass TLS." + ) + if not os.getenv("BEDROCK_INTERNAL_API_KEY", "").strip(): + raise RuntimeError( + "BEDROCK_INTERNAL_API_KEY must be set in production. Without it, " + "crm_bridge.py:27-28 sends requests with no internal-key header " + "and Bedrock falls through to JWT auth — every Pebble write " + "fails 401. Refusing to start." + ) + + @asynccontextmanager async def lifespan(app: FastAPI): logging.basicConfig( level=logging.INFO, format="%(asctime)s %(name)s %(levelname)s %(message)s", ) + _validate_bedrock_bridge_config() await init_db() logger.info( "Pebble starting — ANTHROPIC_API_KEY=%s, OPENROUTER_API_KEY=%s, FEC_API_KEY=%s", diff --git a/pebble/tests/test_bedrock_bridge_config.py b/pebble/tests/test_bedrock_bridge_config.py new file mode 100644 index 00000000..1db5c6b7 --- /dev/null +++ b/pebble/tests/test_bedrock_bridge_config.py @@ -0,0 +1,124 @@ +"""Tests for ``pebble/main.py:_validate_bedrock_bridge_config`` (Phase 0.6). + +Refuses to start when production-shaped Pebble has misconfigured +service-to-service bridge to Bedrock. Failure modes silently routed +writes to ``http://localhost:8000`` without an internal key — Pebble +appeared to work but every write 401'd. + +Invariants: + +A. Dev / non-prod (no FRONTEND_URL https + no PEBBLE_ENV=production) + never raises regardless of BEDROCK_API_URL / API key state. +B. Prod (FRONTEND_URL=https://...) requires BEDROCK_API_URL=https://... +C. Prod requires BEDROCK_INTERNAL_API_KEY non-empty. +D. PEBBLE_ENV=production overrides absent FRONTEND_URL — useful for + headless cron deployments. +E. Whitespace-only API key counts as missing. +""" + +import os +import sys + +import pytest + +sys.path.insert(0, os.path.join(os.path.dirname(__file__), '..', '..')) + +from pebble.main import _validate_bedrock_bridge_config + + +@pytest.fixture(autouse=True) +def _clean_env(monkeypatch): + for var in ("FRONTEND_URL", "PEBBLE_ENV", "BEDROCK_API_URL", "BEDROCK_INTERNAL_API_KEY"): + monkeypatch.delenv(var, raising=False) + + +def test_dev_mode_no_frontend_url_does_not_raise(): + _validate_bedrock_bridge_config() # no env at all + + +def test_dev_mode_localhost_frontend_does_not_raise(monkeypatch): + monkeypatch.setenv("FRONTEND_URL", "http://localhost:3000") + _validate_bedrock_bridge_config() + + +def test_dev_mode_misconfigured_bedrock_url_still_does_not_raise(monkeypatch): + """In dev, even http://localhost:8000 is fine — that's the actual + default for crm_bridge.py:14. The assertion is prod-only.""" + monkeypatch.setenv("FRONTEND_URL", "http://localhost:3000") + monkeypatch.setenv("BEDROCK_API_URL", "http://localhost:8000") + _validate_bedrock_bridge_config() + + +def test_prod_with_https_bedrock_and_key_does_not_raise(monkeypatch): + monkeypatch.setenv("FRONTEND_URL", "https://app.pursuit.org") + monkeypatch.setenv("BEDROCK_API_URL", "https://api.pursuit.org") + monkeypatch.setenv("BEDROCK_INTERNAL_API_KEY", "real-key-9d8s7f6g5h4j") + _validate_bedrock_bridge_config() + + +def test_prod_missing_bedrock_url_raises(monkeypatch): + monkeypatch.setenv("FRONTEND_URL", "https://app.pursuit.org") + monkeypatch.setenv("BEDROCK_INTERNAL_API_KEY", "real-key") + with pytest.raises(RuntimeError, match=r"BEDROCK_API_URL must be set to an https"): + _validate_bedrock_bridge_config() + + +@pytest.mark.parametrize("bad_url", [ + "http://api.pursuit.org", + "http://localhost:8000", + "ftp://api.pursuit.org", + "api.pursuit.org", + "", +]) +def test_prod_non_https_bedrock_url_raises(monkeypatch, bad_url): + monkeypatch.setenv("FRONTEND_URL", "https://app.pursuit.org") + monkeypatch.setenv("BEDROCK_API_URL", bad_url) + monkeypatch.setenv("BEDROCK_INTERNAL_API_KEY", "real-key") + with pytest.raises(RuntimeError, match=r"https"): + _validate_bedrock_bridge_config() + + +def test_prod_missing_internal_api_key_raises(monkeypatch): + monkeypatch.setenv("FRONTEND_URL", "https://app.pursuit.org") + monkeypatch.setenv("BEDROCK_API_URL", "https://api.pursuit.org") + with pytest.raises(RuntimeError, match=r"BEDROCK_INTERNAL_API_KEY must be set"): + _validate_bedrock_bridge_config() + + +@pytest.mark.parametrize("blank_key", ["", " ", "\t", "\n"]) +def test_prod_blank_internal_api_key_raises(monkeypatch, blank_key): + """Whitespace-only API key counts as missing — same as empty.""" + monkeypatch.setenv("FRONTEND_URL", "https://app.pursuit.org") + monkeypatch.setenv("BEDROCK_API_URL", "https://api.pursuit.org") + monkeypatch.setenv("BEDROCK_INTERNAL_API_KEY", blank_key) + with pytest.raises(RuntimeError, match=r"BEDROCK_INTERNAL_API_KEY must be set"): + _validate_bedrock_bridge_config() + + +def test_pebble_env_production_override_no_frontend_url(monkeypatch): + """PEBBLE_ENV=production triggers the prod path even without + FRONTEND_URL. Headless cron deployments don't have a frontend URL. + """ + monkeypatch.setenv("PEBBLE_ENV", "production") + monkeypatch.setenv("BEDROCK_API_URL", "http://localhost:8000") + monkeypatch.setenv("BEDROCK_INTERNAL_API_KEY", "real-key") + with pytest.raises(RuntimeError, match=r"https"): + _validate_bedrock_bridge_config() + + +def test_pebble_env_production_case_insensitive(monkeypatch): + monkeypatch.setenv("PEBBLE_ENV", "PRODUCTION") + monkeypatch.setenv("BEDROCK_API_URL", "http://localhost:8000") + monkeypatch.setenv("BEDROCK_INTERNAL_API_KEY", "real-key") + with pytest.raises(RuntimeError): + _validate_bedrock_bridge_config() + + +def test_pebble_env_other_values_treated_as_dev(monkeypatch): + """PEBBLE_ENV=staging / development / arbitrary → dev-mode path.""" + for value in ("staging", "development", "test", "dev", ""): + monkeypatch.setenv("PEBBLE_ENV", value) + # Misconfigured bridge — but we're not in prod, so no raise. + monkeypatch.setenv("BEDROCK_API_URL", "http://localhost:8000") + monkeypatch.delenv("BEDROCK_INTERNAL_API_KEY", raising=False) + _validate_bedrock_bridge_config() From f90099aaaffe18e4a549bcf44a3b2663836dbaa8 Mon Sep 17 00:00:00 2001 From: JP Date: Wed, 6 May 2026 19:25:40 -0400 Subject: [PATCH 05/35] feat(auth): X-Originating-User + X-Request-Id mandatory on internal-key MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 0.2 + 0.4 of the Pebble 1.0 plan. Pebble becomes a delegated principal, not a god principal. What changed: * require_auth_or_internal now requires three headers when the internal-key path is taken: - X-Internal-Key (existing) — service-to-service auth - X-Originating-User (new) — the human whose session triggered this call. RFC 5321-shaped email; rejected if missing, malformed, or > 254 chars. - X-Request-Id (new) — UUID for replay defense via UNIQUE(request_id) on bedrock.pebble_write_audit. * Synthetic user dict gains originating_user_email, request_id, and a scopes tuple. X-Pebble-Scopes header (optional) overrides the default ("*",) full grant — future tightening will narrow scopes per-call instead of carrying full grant. * Wrong-key callers fall through to require_auth WITHOUT surfacing the new contract. We don't leak the kill-switch state or the originating-user requirement to unauthenticated traffic. * Validation kept lightweight on the auth path — durable membership check is at the audit-row INSERT (FK lookup vs org_users) per the security spec. Why mandatory on reads too: Pebble must always carry its delegated principal so research/lookup paths get audited the same way as writes. Silent reads on behalf of a phantom user are an audit hole. 34 new tests in test_pebble_internal_auth.py covering 8 invariants. Existing kill-switch tests updated to satisfy the new contract; all 23 still pass. Plan ref: tasks/pebble-search-spec.md decision §3 Adversary refs: pebble-adversary-security.md H1, H3, M3, S1 pebble-adversary-architecture.md #6 pebble-adversary-ux.md #4 Co-Authored-By: Claude Opus 4.7 (1M context) --- financial_forecasting/auth.py | 107 +++++++- .../tests/test_pebble_internal_auth.py | 242 ++++++++++++++++++ .../tests/test_pebble_kill_switch.py | 29 ++- 3 files changed, 367 insertions(+), 11 deletions(-) create mode 100644 financial_forecasting/tests/test_pebble_internal_auth.py diff --git a/financial_forecasting/auth.py b/financial_forecasting/auth.py index 2f071386..3ead86f0 100644 --- a/financial_forecasting/auth.py +++ b/financial_forecasting/auth.py @@ -147,6 +147,11 @@ async def get_current_user_dep(request: Request) -> Optional[Dict]: # Pebble's research/lookup paths keep working during write-side incidents. _WRITE_METHODS = frozenset({"POST", "PUT", "PATCH", "DELETE"}) +# Default scope grant for the internal-key principal. v1.0 grants the +# superset; later phases tighten via per-call X-Pebble-Scopes header. +# `*` is interpreted as "all scopes" by check_permission_or_internal. +_DEFAULT_INTERNAL_SCOPES = ("*",) + def _pebble_writes_disabled() -> bool: """Read the kill switch fresh on every call so operators can flip it @@ -155,6 +160,33 @@ def _pebble_writes_disabled() -> bool: return os.getenv("PEBBLE_WRITES_DISABLED", "").strip().lower() in {"true", "1", "yes"} +def _parse_scopes(header_value: str) -> tuple[str, ...]: + """Parse the optional X-Pebble-Scopes header. Comma-separated scope + strings; empty / unset header = full grant via the wildcard ``*``. + """ + raw = (header_value or "").strip() + if not raw: + return _DEFAULT_INTERNAL_SCOPES + scopes = tuple(s.strip() for s in raw.split(",") if s.strip()) + return scopes or _DEFAULT_INTERNAL_SCOPES + + +def _is_valid_originating_user_email(email: str) -> bool: + """Lightweight shape check on X-Originating-User. We don't verify + membership in org_users here (would require a DB hit on every + request); the route-level audit insert (FK to org_users via + originating_user_email lookup) is the durable check. Spoofs against + the API key still produce auditable rows. + """ + if not email or len(email) > 254: + return False + if "@" not in email or email.startswith("@") or email.endswith("@"): + return False + if any(c.isspace() for c in email): + return False + return True + + async def require_auth_or_internal(request: Request) -> Dict: """Authorize via internal API key (service-to-service) or user JWT. @@ -163,12 +195,33 @@ async def require_auth_or_internal(request: Request) -> Dict: Dev mode: if BEDROCK_INTERNAL_API_KEY is empty, internal key check is skipped and only JWT auth is tried. - Kill switch: if PEBBLE_WRITES_DISABLED env is truthy AND the caller is - using a valid internal key AND the request method is a write + **Mandatory headers when using internal-key auth (Phase 0.2):** + + - ``X-Originating-User``: the human whose session triggered this + service-to-service call. Required on EVERY internal-key request, + reads and writes alike. The email is propagated to + ``bedrock.pebble_write_audit`` and to permission resolution so + that Pebble acts as a delegated principal, not a god principal. + Three independent adversarial reviews flagged the un-attributed + "service:pebble" pattern as a 1.0 blocker; this enforcement + closes that gap. + - ``X-Request-Id``: a UUIDv7 (or any UUID) for replay defense via + ``UNIQUE(request_id)`` on ``bedrock.pebble_write_audit``. Format + checked here; uniqueness checked at the audit-row INSERT in the + route handler. + + Optional header: + + - ``X-Pebble-Scopes``: comma-separated scopes the caller is + requesting for THIS request. Default = ``("*",)`` (full grant). + ``check_permission_or_internal`` verifies the matching scope is + present. Future tightening: Pebble will request narrow scopes + per call instead of carrying the full grant. + + Kill switch: if PEBBLE_WRITES_DISABLED env is truthy AND the caller + is using a valid internal key AND the request method is a write (POST/PUT/PATCH/DELETE), return 503. Reads via internal key and all - JWT-authenticated requests are unaffected. Designed for incident - response — flip the env var to halt service-account writes without a - deploy. + JWT-authenticated requests are unaffected. """ internal_key = request.headers.get("X-Internal-Key", "") if _BEDROCK_INTERNAL_API_KEY and internal_key: @@ -189,9 +242,53 @@ async def require_auth_or_internal(request: Request) -> Dict: ), }, ) + + originating_user = request.headers.get("X-Originating-User", "").strip() + if not _is_valid_originating_user_email(originating_user): + logger.warning( + "internal_key_missing_originating_user: %s %s", + request.method, + request.url.path, + ) + raise HTTPException( + status_code=401, + detail={ + "error": "originating_user_required", + "message": ( + "X-Originating-User header is required on every " + "internal-key request. Pebble acts on behalf of a " + "specific user, never as itself." + ), + }, + ) + + request_id = request.headers.get("X-Request-Id", "").strip() + if not request_id: + logger.warning( + "internal_key_missing_request_id: %s %s", + request.method, + request.url.path, + ) + raise HTTPException( + status_code=401, + detail={ + "error": "request_id_required", + "message": ( + "X-Request-Id header is required on every " + "internal-key request for replay defense. " + "Pass a UUID." + ), + }, + ) + + scopes = _parse_scopes(request.headers.get("X-Pebble-Scopes", "")) + return { "user_id": "service:pebble", "email": "pebble@internal", "is_service": True, + "originating_user_email": originating_user, + "request_id": request_id, + "scopes": scopes, } return await require_auth(request) diff --git a/financial_forecasting/tests/test_pebble_internal_auth.py b/financial_forecasting/tests/test_pebble_internal_auth.py new file mode 100644 index 00000000..39dd8a34 --- /dev/null +++ b/financial_forecasting/tests/test_pebble_internal_auth.py @@ -0,0 +1,242 @@ +"""Tests for Phase 0.2 / 0.4 of the Pebble 1.0 plan — mandatory +``X-Originating-User`` + ``X-Request-Id`` headers and the optional +``X-Pebble-Scopes`` grant on internal-key requests. + +Plan ref: tasks/pebble-search-spec.md decisions §3, §4 +Adversary refs: pebble-adversary-security.md H1, H3, M3, S1 + pebble-adversary-architecture.md #6 + pebble-adversary-ux.md #4 + +Invariants locked in: + +A. Internal-key + valid headers → synthetic user dict carries + ``originating_user_email``, ``request_id``, ``scopes``. +B. Internal-key + missing X-Originating-User → 401 ``originating_user_required``. +C. Internal-key + malformed X-Originating-User → 401. +D. Internal-key + missing X-Request-Id → 401 ``request_id_required``. +E. Wrong internal key + missing originating user → fall through to JWT auth + (we never leak the new contract to unauthenticated callers). +F. JWT auth path is unaffected by the new headers. +G. Custom scope grant via X-Pebble-Scopes parses correctly. +H. Empty / whitespace-only X-Pebble-Scopes falls back to default ``("*",)``. +I. Originating-user shape: long emails, embedded whitespace, missing @ all + reject; weird-but-valid (subdomains, plus-addressing) accept. +""" + +import os +import sys +from unittest.mock import AsyncMock + +import pytest +from fastapi import HTTPException +from starlette.requests import Request + +sys.path.insert(0, os.path.join(os.path.dirname(__file__), '..')) + +import auth + + +_TEST_KEY = "test-internal-key-abcdef0123456789" +_USER = "rm@pursuit.org" +_REQ = "01993b8d-2c9a-7c4f-8b0e-000000000001" + + +def _req( + method: str = "POST", + *, + internal_key: str = _TEST_KEY, + originating_user: str = _USER, + request_id: str = _REQ, + scopes: str | None = None, +) -> Request: + headers: list[tuple[bytes, bytes]] = [] + if internal_key: + headers.append((b"x-internal-key", internal_key.encode())) + if originating_user: + headers.append((b"x-originating-user", originating_user.encode())) + if request_id: + headers.append((b"x-request-id", request_id.encode())) + if scopes is not None: + headers.append((b"x-pebble-scopes", scopes.encode())) + return Request({ + "type": "http", + "method": method, + "headers": headers, + "path": "/api/pebble-test", + "query_string": b"", + }) + + +@pytest.fixture(autouse=True) +def _pin_key(monkeypatch): + monkeypatch.setattr(auth, "_BEDROCK_INTERNAL_API_KEY", _TEST_KEY) + monkeypatch.delenv("PEBBLE_WRITES_DISABLED", raising=False) + + +# --------------------------------------------------------------------------- +# A. Happy path +# --------------------------------------------------------------------------- + +@pytest.mark.asyncio +async def test_synthetic_user_carries_full_contract(): + user = await auth.require_auth_or_internal(_req()) + assert user["is_service"] is True + assert user["user_id"] == "service:pebble" + assert user["email"] == "pebble@internal" + assert user["originating_user_email"] == _USER + assert user["request_id"] == _REQ + assert user["scopes"] == ("*",) + + +# --------------------------------------------------------------------------- +# B. Missing X-Originating-User → 401 +# --------------------------------------------------------------------------- + +@pytest.mark.asyncio +async def test_missing_originating_user_rejected_for_writes(): + with pytest.raises(HTTPException) as exc: + await auth.require_auth_or_internal(_req(originating_user="")) + assert exc.value.status_code == 401 + assert exc.value.detail["error"] == "originating_user_required" + + +@pytest.mark.asyncio +async def test_missing_originating_user_rejected_for_reads_too(): + """Reads via internal key are also gated. Pebble must always carry + its delegated principal — silent reads on behalf of a phantom user + are an audit hole.""" + with pytest.raises(HTTPException) as exc: + await auth.require_auth_or_internal(_req(method="GET", originating_user="")) + assert exc.value.status_code == 401 + assert exc.value.detail["error"] == "originating_user_required" + + +# --------------------------------------------------------------------------- +# C. Malformed X-Originating-User → 401 +# --------------------------------------------------------------------------- + +@pytest.mark.parametrize("bad_email", [ + "no-at-sign", + "@leading.at", + "trailing.at@", + "spaces in the middle@pursuit.org", + "tabs\tare\tinvalid@pursuit.org", + "x" * 250 + "@pursuit.org", # > 254 chars total +]) +@pytest.mark.asyncio +async def test_malformed_originating_user_rejected(bad_email): + with pytest.raises(HTTPException) as exc: + await auth.require_auth_or_internal(_req(originating_user=bad_email)) + assert exc.value.status_code == 401 + assert exc.value.detail["error"] == "originating_user_required" + + +@pytest.mark.parametrize("good_email", [ + "rm@pursuit.org", + "exec.user@pursuit.org", + "user+tag@pursuit.org", + "deep.sub.domain@mail.staff.pursuit.org", + "x@y.co", # short but valid +]) +@pytest.mark.asyncio +async def test_well_formed_originating_user_accepted(good_email): + user = await auth.require_auth_or_internal(_req(originating_user=good_email)) + assert user["originating_user_email"] == good_email + + +# --------------------------------------------------------------------------- +# D. Missing X-Request-Id → 401 +# --------------------------------------------------------------------------- + +@pytest.mark.asyncio +async def test_missing_request_id_rejected(): + with pytest.raises(HTTPException) as exc: + await auth.require_auth_or_internal(_req(request_id="")) + assert exc.value.status_code == 401 + assert exc.value.detail["error"] == "request_id_required" + + +# --------------------------------------------------------------------------- +# E. Wrong internal key → fall through to JWT (never leak the new contract) +# --------------------------------------------------------------------------- + +@pytest.mark.asyncio +async def test_wrong_internal_key_falls_through_even_without_originating_user(monkeypatch): + """A wrong internal key with no originating user must NOT 401 with + 'originating_user_required' — that would leak the contract shape to + unauthenticated traffic. Behavior is identical to no-internal-key: + fall through to require_auth. + """ + fake_user = {"user_id": "u-jwt", "email": "real@pursuit.org", "is_service": False} + monkeypatch.setattr(auth, "require_auth", AsyncMock(return_value=fake_user)) + request = _req(internal_key="not-the-real-key", originating_user="", request_id="") + user = await auth.require_auth_or_internal(request) + assert user is fake_user + + +# --------------------------------------------------------------------------- +# F. JWT path unaffected +# --------------------------------------------------------------------------- + +@pytest.mark.asyncio +async def test_jwt_path_unaffected_by_new_headers(monkeypatch): + fake_user = {"user_id": "u-jwt", "email": "real@pursuit.org", "is_service": False} + monkeypatch.setattr(auth, "require_auth", AsyncMock(return_value=fake_user)) + # No internal key — falls through to require_auth. + request = _req(internal_key="", originating_user="", request_id="") + user = await auth.require_auth_or_internal(request) + assert user is fake_user + assert "originating_user_email" not in user + + +# --------------------------------------------------------------------------- +# G + H. X-Pebble-Scopes parsing +# --------------------------------------------------------------------------- + +@pytest.mark.asyncio +async def test_explicit_scopes_parsed(): + user = await auth.require_auth_or_internal(_req(scopes="opp.write,account.read,task.write")) + assert user["scopes"] == ("opp.write", "account.read", "task.write") + + +@pytest.mark.parametrize("scopes_header", ["", " ", ",", " , , "]) +@pytest.mark.asyncio +async def test_empty_scopes_falls_back_to_default(scopes_header): + user = await auth.require_auth_or_internal(_req(scopes=scopes_header)) + assert user["scopes"] == ("*",) + + +@pytest.mark.asyncio +async def test_scopes_strip_whitespace(): + user = await auth.require_auth_or_internal(_req(scopes=" opp.write , account.read ")) + assert user["scopes"] == ("opp.write", "account.read") + + +# --------------------------------------------------------------------------- +# Helper-function unit tests +# --------------------------------------------------------------------------- + +@pytest.mark.parametrize("email,expected", [ + ("rm@pursuit.org", True), + ("a@b.co", True), + ("user+tag@sub.pursuit.org", True), + ("", False), + ("no-at", False), + ("@x.org", False), + ("x@", False), + ("x@y" + ".z" * 130, False), # > 254 chars triggers length check + ("space middle@x.org", False), +]) +def test_is_valid_originating_user_email(email, expected): + assert auth._is_valid_originating_user_email(email) is expected + + +def test_parse_scopes_default(): + assert auth._parse_scopes("") == ("*",) + assert auth._parse_scopes(None) == ("*",) + + +def test_parse_scopes_explicit(): + assert auth._parse_scopes("a,b,c") == ("a", "b", "c") + assert auth._parse_scopes("a, b , c") == ("a", "b", "c") + assert auth._parse_scopes(" only ") == ("only",) diff --git a/financial_forecasting/tests/test_pebble_kill_switch.py b/financial_forecasting/tests/test_pebble_kill_switch.py index 06f503f3..ae105174 100644 --- a/financial_forecasting/tests/test_pebble_kill_switch.py +++ b/financial_forecasting/tests/test_pebble_kill_switch.py @@ -32,16 +32,29 @@ # --------------------------------------------------------------------------- _TEST_KEY = "test-internal-key-1234567890abcdef" - - -def _make_request(method: str, internal_key: str = "") -> Request: - """Build a minimal Starlette Request scope with the given method and - optional X-Internal-Key header. Avoids the TestClient round-trip — we're - unit-testing a dependency, not an endpoint. +_TEST_ORIGINATING_USER = "rm@pursuit.org" +_TEST_REQUEST_ID = "01993b8d-2c9a-7c4f-8b0e-000000000001" + + +def _make_request( + method: str, + internal_key: str = "", + originating_user: str = _TEST_ORIGINATING_USER, + request_id: str = _TEST_REQUEST_ID, +) -> Request: + """Build a minimal Starlette Request scope. Defaults satisfy the + Phase 0.2 contract (X-Originating-User + X-Request-Id) so kill-switch + tests can focus on the kill-switch invariants. Tests that need to + exercise the missing-header paths pass ``originating_user=""`` / + ``request_id=""`` explicitly. """ headers = [] if internal_key: headers.append((b"x-internal-key", internal_key.encode())) + if originating_user: + headers.append((b"x-originating-user", originating_user.encode())) + if request_id: + headers.append((b"x-request-id", request_id.encode())) scope = { "type": "http", "method": method, @@ -106,6 +119,10 @@ async def test_internal_key_write_succeeds_when_switch_off(fixed_internal_key, s assert user["is_service"] is True assert user["user_id"] == "service:pebble" assert user["email"] == "pebble@internal" + # Phase 0.2 contract: synthetic user carries originating user + request_id + scopes. + assert user["originating_user_email"] == _TEST_ORIGINATING_USER + assert user["request_id"] == _TEST_REQUEST_ID + assert user["scopes"] == ("*",) @pytest.mark.parametrize("method", ["POST", "PUT", "PATCH", "DELETE"]) From 30b9a105f1765cb3dac65b11075dce7edd37fdf4 Mon Sep 17 00:00:00 2001 From: JP Date: Wed, 6 May 2026 19:25:59 -0400 Subject: [PATCH 06/35] =?UTF-8?q?feat(stages):=20canonical=20Salesforce=20?= =?UTF-8?q?stage=20source=20=E2=80=94=20sf=5Fstages=20module=20+=20cache?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 0.9. Single source of truth for SF Opportunity stage values, replacing the eight independent locations that today carry stage strings literally: * models.py:OpportunityStage enum * frontend-v2/src/lib/{stages,funnelStages}.ts * frontend-v2/src/components/StageProgression.tsx * frontend-v2/src/pages/Cleanup.tsx * services/crm_parser.py * routes/opportunities_extra.py:53 * services/awards_service.ELIGIBLE_STAGES_BY_RECORD_TYPE Every SF picklist rename (commit 58c360e renamed three at once) required hand-edits across all eight. This module collapses them into one fetch. Architecture: * services/sf_stages.py — async read API (`get_stages`, `get_entry_stage`) + sync bucket predicates (`is_revenue_earning`, `is_open`, `is_closed`, `is_lost`) * 24h read-through cache layered over a process-local 5min in-memory cache layered over the static fallback table * Static fallback mirrors models.OpportunityStage as of 2026-05-06 — last-line-of-defense for fresh deploys before the picklist refresh job has run Buckets are frozenset[str] preserving the F1 PR #134 contract: reporting semantics LAYER ON TOP of stages, never replace them. Code that says `if stage in REVENUE_EARNING_STAGES:` keeps working when SF adds a new stage that the refresh job classifies into the bucket — no code change needed. bedrock.sf_picklist_cache migration: * Composite key (org_id, sf_object, field_name, record_type, value) * Per-row buckets TEXT[] for reporting classification * GIN index on buckets, partial index on stale rows * 24h refresh_after column drives the cache TTL * Multi-tenant org_id outer guard from day 1 26 tests covering static fallback / cache hit / cache clear / DB preferred / DB error / entry stage / bucket predicates / bucket invariants (revenue ⊆ closed, lost ⊆ closed, open ∩ closed = ∅) / frozenset immutability. This file does NOT yet replace the eight call-sites. That migration lands in Layer 1, after the picklist refresh job exists. Callers swap incrementally. Plan ref: tasks/pebble-search-spec.md §0.9 Memory refs: feedback_sf_stages_sacred, project_stage_schema_drift Co-Authored-By: Claude Opus 4.7 (1M context) --- .../2026-05-06-sf-picklist-cache.sql | 100 +++++++ financial_forecasting/services/sf_stages.py | 261 ++++++++++++++++++ financial_forecasting/tests/test_sf_stages.py | 222 +++++++++++++++ 3 files changed, 583 insertions(+) create mode 100644 financial_forecasting/db/migrations/2026-05-06-sf-picklist-cache.sql create mode 100644 financial_forecasting/services/sf_stages.py create mode 100644 financial_forecasting/tests/test_sf_stages.py diff --git a/financial_forecasting/db/migrations/2026-05-06-sf-picklist-cache.sql b/financial_forecasting/db/migrations/2026-05-06-sf-picklist-cache.sql new file mode 100644 index 00000000..7f1c924d --- /dev/null +++ b/financial_forecasting/db/migrations/2026-05-06-sf-picklist-cache.sql @@ -0,0 +1,100 @@ +-- 2026-05-06: bedrock.sf_picklist_cache +-- +-- Phase 0.9 of the Pebble 1.0 plan. Single source of truth for +-- Salesforce picklist values (Opportunity.StageName today; extends +-- to other picklists as needed) so that the eight independent +-- locations carrying renamed stage strings (`models.py`, +-- `frontend-v2/lib/stages.ts`, `funnelStages.ts`, +-- `StageProgression.tsx`, `Cleanup.tsx`, `crm_parser.py`, +-- `opportunities_extra.py:53`, `awards_service.ELIGIBLE_STAGES_BY_RECORD_TYPE`) +-- collapse into one fetch. +-- +-- Why: +-- Per `feedback_sf_stages_sacred.md`: SF stages are sacred — +-- never hide/deprecate/reclassify them on the SF side. But +-- stages DO get renamed on the SF side (commit 58c360e was +-- "rename SF stages to match new picklist labels"), and when +-- they do, eight files in the Bedrock codebase need +-- hand-edits. This cache is the canonical Bedrock-side +-- reflection of what's in SF — one fetch, everyone uses it. +-- +-- Design: +-- One row per (object, picklist_field, record_type, value). +-- `record_type` is nullable — when null, the value is +-- available across all record types. Refreshed nightly via a +-- background job calling SF describeSObject. 24h TTL on +-- `services/sf_stages.py` reads. +-- +-- Related: +-- * tasks/pebble-search-spec.md decision §6 (multi-tenant column) +-- * tasks/pebble-overhaul-plan.md §0.9 +-- * services/sf_stages.py (next file) +-- * project_stage_schema_drift memory (eight-location problem) +-- +-- Idempotent — safe to re-run. + +CREATE TABLE IF NOT EXISTS bedrock.sf_picklist_cache ( + id UUID PRIMARY KEY DEFAULT uuid_generate_v4(), + + -- Composite identity + sf_object TEXT NOT NULL, -- 'Opportunity' | 'Account' | ... + field_name TEXT NOT NULL, -- 'StageName' | 'Type' | ... + record_type TEXT, -- 'Philanthropy' | NULL = all + value TEXT NOT NULL, -- the actual SF picklist value + + -- Display + ordering. Mirror of SF metadata so we don't + -- per-page-fetch. Order is important — the funnel display in + -- frontend-v2/lib/funnelStages.ts encodes a left-to-right + -- progression. + label TEXT NOT NULL, + sort_order INT NOT NULL DEFAULT 0, + is_active BOOLEAN NOT NULL DEFAULT TRUE, + is_default BOOLEAN NOT NULL DEFAULT FALSE, + + -- Bucket annotations. Reporting semantics live as buckets ON + -- TOP of stages — never replacing them. Encoded as a string + -- array so we can have e.g. + -- ['REVENUE_EARNING','OPEN_PIPELINE'] without joining a + -- second table. The `bedrock` Python service exposes these + -- as Set to match the F1 buckets PR #134 contract. + buckets TEXT[] NOT NULL DEFAULT '{}', + + -- Probability for forecast math. Mirrors `models.py` static + -- table; live picklist values without an entry default to + -- the median bucket prob. + probability SMALLINT, + + -- Sync metadata. Used by `services/sf_stages.py` cache to + -- decide refresh urgency. + fetched_at TIMESTAMPTZ NOT NULL DEFAULT now(), + refresh_after TIMESTAMPTZ NOT NULL DEFAULT (now() + INTERVAL '24 hours'), + + -- Multi-tenant outer guard. + org_id TEXT NOT NULL DEFAULT 'pursuit', + + UNIQUE (org_id, sf_object, field_name, record_type, value) +); + +CREATE INDEX IF NOT EXISTS idx_sf_picklist_cache_lookup + ON bedrock.sf_picklist_cache(sf_object, field_name, record_type, sort_order) + WHERE is_active = TRUE; +CREATE INDEX IF NOT EXISTS idx_sf_picklist_cache_buckets + ON bedrock.sf_picklist_cache USING GIN (buckets); +CREATE INDEX IF NOT EXISTS idx_sf_picklist_cache_stale + ON bedrock.sf_picklist_cache(refresh_after) + WHERE is_active = TRUE; + +COMMENT ON TABLE bedrock.sf_picklist_cache IS + 'Canonical Bedrock-side reflection of Salesforce picklist values. services/sf_stages.py is the read API. Refreshed via nightly background job.'; +COMMENT ON COLUMN bedrock.sf_picklist_cache.buckets IS + 'Reporting bucket annotations on top of SF stages. Examples: {"REVENUE_EARNING","OPEN_PIPELINE"}. Buckets layer on top of stages — never replace them.'; +COMMENT ON COLUMN bedrock.sf_picklist_cache.record_type IS + 'NULL means "applies to all record types". Mirrors how SF stores per-RecordType picklist subset rules.'; + +-- Grants +DO $$ +BEGIN + IF EXISTS (SELECT 1 FROM pg_roles WHERE rolname = 'bedrock_user') THEN + GRANT SELECT, INSERT, UPDATE ON bedrock.sf_picklist_cache TO bedrock_user; + END IF; +END $$; diff --git a/financial_forecasting/services/sf_stages.py b/financial_forecasting/services/sf_stages.py new file mode 100644 index 00000000..b4796587 --- /dev/null +++ b/financial_forecasting/services/sf_stages.py @@ -0,0 +1,261 @@ +"""Phase 0.9 — canonical source of Salesforce Opportunity stage values. + +Eight independent locations in the codebase carry stage strings literally: + + * ``models.py:OpportunityStage`` enum + * ``frontend-v2/src/lib/stages.ts`` + * ``frontend-v2/src/lib/funnelStages.ts`` + * ``frontend-v2/src/components/StageProgression.tsx`` + * ``frontend-v2/src/pages/Cleanup.tsx`` + * ``services/crm_parser.py`` + * ``routes/opportunities_extra.py:53`` (prior_stage default) + * ``services/awards_service.ELIGIBLE_STAGES_BY_RECORD_TYPE`` + +Every time SF renames a picklist value (commit ``58c360e`` did three +at once), all eight need hand-edits. This module is the single read +API everything migrates to. + +Invariants the F1 stage-buckets PR #134 (and ``feedback_sf_stages_sacred``) +encode and this module preserves: + + * SF stages are sacred — never hide / deprecate / reclassify them. + * Reporting buckets layer ON TOP of stages, never replace them. + * Buckets are ``Set[str]`` lookups, not switch statements over a + static enum — code that does ``if stage in REVENUE_EARNING_STAGES`` + keeps working when SF adds a new stage without a code change. + +This module reads from ``bedrock.sf_picklist_cache`` (populated by a +nightly background job calling ``Salesforce.describeSObject``). The +cache has 24h TTL. Live SF picklist read is the fallback when the +cache is empty or stale. + +This file does NOT yet replace the eight call-sites. That migration +lands in Layer 1, after the picklist refresh job exists. For now the +module exposes the read API and a static fallback table that mirrors +``models.OpportunityStage`` so callers can swap incrementally. +""" + +from __future__ import annotations + +import logging +import os +import time +from dataclasses import dataclass, field +from typing import Optional + +logger = logging.getLogger(__name__) + + +# --------------------------------------------------------------------------- +# Static fallback — mirrors models.OpportunityStage as of 2026-05-06. +# --------------------------------------------------------------------------- +# Used when bedrock.sf_picklist_cache is empty (fresh deploy, or SF +# picklist refresh job hasn't run yet). Matches the renamed labels +# from commit 58c360e ("rename SF stages to match new picklist labels"). +# +# This is the LAST line of defense — production should always serve +# from the cache. If you find yourself editing this list, the picklist +# refresh job is probably broken. +# --------------------------------------------------------------------------- + +_STATIC_FALLBACK_STAGES_BY_RECORD_TYPE: dict[str, tuple[str, ...]] = { + "Philanthropy": ( + "Lead Gen", + "New Lead", + "Qualifying", + "Ask in Progress", + "Proposal Submitted", + "Verbal Commitment", + "Contract Creation", + "Contracting", + "Contract Signed", + "Closed / Fulfilled", + "Closed Won", + "Closed Lost", + "Withdrawn", + ), +} + +# Per-record-type entry stage. The "promote a prospect to an Opp" wedge +# in Phase B3 of the overhaul plan creates Opps at this stage. +ENTRY_STAGE_BY_RECORD_TYPE: dict[str, str] = { + "Philanthropy": "New Lead", +} + +# --------------------------------------------------------------------------- +# Reporting buckets. Set[str] semantics — code that says +# `if stage in REVENUE_EARNING_STAGES:` keeps working when SF adds a +# new stage that the picklist refresh job classifies into the bucket. +# --------------------------------------------------------------------------- + +REVENUE_EARNING_STAGES: frozenset[str] = frozenset({ + "Closed / Fulfilled", + "Closed Won", +}) + +OPEN_PIPELINE_STAGES: frozenset[str] = frozenset({ + "Lead Gen", + "New Lead", + "Qualifying", + "Ask in Progress", + "Proposal Submitted", + "Verbal Commitment", + "Contract Creation", + "Contracting", + "Contract Signed", +}) + +CLOSED_STAGES: frozenset[str] = frozenset({ + "Closed / Fulfilled", + "Closed Won", + "Closed Lost", + "Withdrawn", +}) + +LOST_STAGES: frozenset[str] = frozenset({ + "Closed Lost", + "Withdrawn", +}) + + +# --------------------------------------------------------------------------- +# Cache module — process-local 24h read-through over +# bedrock.sf_picklist_cache. Refresh happens via a separate background +# job (Layer 1) that writes to the table directly. +# --------------------------------------------------------------------------- + +_DEFAULT_TTL_SECONDS = int(os.getenv("SF_STAGES_CACHE_TTL_SECONDS", "300")) + + +@dataclass +class _StageCacheEntry: + fetched_at: float + stages: tuple[str, ...] + ttl_seconds: int = _DEFAULT_TTL_SECONDS + + @property + def is_fresh(self) -> bool: + return (time.time() - self.fetched_at) < self.ttl_seconds + + +@dataclass +class _StageCache: + by_record_type: dict[str, _StageCacheEntry] = field(default_factory=dict) + + def get(self, record_type: str) -> Optional[tuple[str, ...]]: + entry = self.by_record_type.get(record_type) + if entry and entry.is_fresh: + return entry.stages + return None + + def set(self, record_type: str, stages: tuple[str, ...]) -> None: + self.by_record_type[record_type] = _StageCacheEntry( + fetched_at=time.time(), stages=stages, + ) + + def clear(self) -> None: + self.by_record_type.clear() + + +_cache = _StageCache() + + +# --------------------------------------------------------------------------- +# Read API +# --------------------------------------------------------------------------- + +async def get_stages( + record_type: str = "Philanthropy", + *, + db_conn=None, +) -> tuple[str, ...]: + """Return the canonical, ordered stage list for a record type. + + Read order: + 1. Process-local cache (fresh ≤ TTL). + 2. ``bedrock.sf_picklist_cache`` row. + 3. Static fallback (this module). + + ``db_conn`` should be an asyncpg connection. If ``None``, skips the + DB step and uses the static fallback. Tests pass ``db_conn=None`` + to exercise the static path; production passes the request-scoped + DB connection. + """ + cached = _cache.get(record_type) + if cached is not None: + return cached + + if db_conn is not None: + try: + stages = await _fetch_from_db(db_conn, record_type) + if stages: + _cache.set(record_type, stages) + return stages + except Exception as e: + logger.warning( + "sf_stages: DB cache read failed for %s, falling back to static: %s", + record_type, e, + ) + + stages = _STATIC_FALLBACK_STAGES_BY_RECORD_TYPE.get(record_type, ()) + if stages: + _cache.set(record_type, stages) + return stages + + +async def get_entry_stage(record_type: str = "Philanthropy") -> Optional[str]: + """Return the canonical entry-stage name for new opps of a record type. + None if the record type is not configured for promotion. + """ + return ENTRY_STAGE_BY_RECORD_TYPE.get(record_type) + + +def is_revenue_earning(stage: str) -> bool: + return stage in REVENUE_EARNING_STAGES + + +def is_open(stage: str) -> bool: + return stage in OPEN_PIPELINE_STAGES + + +def is_closed(stage: str) -> bool: + return stage in CLOSED_STAGES + + +def is_lost(stage: str) -> bool: + return stage in LOST_STAGES + + +def clear_cache() -> None: + """Drop the process-local cache. Used by tests and by the + picklist-refresh background job after writing fresh rows. + """ + _cache.clear() + + +# --------------------------------------------------------------------------- +# Internal — DB read +# --------------------------------------------------------------------------- + +async def _fetch_from_db(db_conn, record_type: str) -> tuple[str, ...]: + """Read sorted active stages from bedrock.sf_picklist_cache. + + Falls through (returns empty tuple) when: + * Table doesn't exist yet (fresh deploy pre-migration). + * No rows for the record type (refresh job hasn't run). + * All rows are stale (`refresh_after < now()`). + """ + rows = await db_conn.fetch( + """ + SELECT value + FROM bedrock.sf_picklist_cache + WHERE sf_object = 'Opportunity' + AND field_name = 'StageName' + AND (record_type = $1 OR record_type IS NULL) + AND is_active = TRUE + AND refresh_after > now() + ORDER BY sort_order ASC, value ASC + """, + record_type, + ) + return tuple(r["value"] for r in rows) diff --git a/financial_forecasting/tests/test_sf_stages.py b/financial_forecasting/tests/test_sf_stages.py new file mode 100644 index 00000000..7d544246 --- /dev/null +++ b/financial_forecasting/tests/test_sf_stages.py @@ -0,0 +1,222 @@ +"""Tests for ``services/sf_stages.py`` — Phase 0.9 canonical stage source. + +Invariants: + +A. ``get_stages`` returns the static fallback list when no db_conn is provided. +B. Cache reads on the second call when fresh. +C. DB read is preferred when a connection is provided and cache is empty. +D. DB read failure falls through to static fallback (logged but not raised). +E. ``get_entry_stage`` returns the configured entry stage per record type. +F. Bucket predicates (`is_revenue_earning`, `is_open`, `is_closed`, `is_lost`) + match the documented set semantics. Code using `if stage in BUCKET` keeps + working without changes. +G. Bucket sets are immutable (frozenset) so accidental .add() raises. +""" + +import os +import sys +from unittest.mock import AsyncMock + +import pytest + +sys.path.insert(0, os.path.join(os.path.dirname(__file__), '..')) + +from services import sf_stages + + +@pytest.fixture(autouse=True) +def _clear_cache(): + sf_stages.clear_cache() + yield + sf_stages.clear_cache() + + +# --------------------------------------------------------------------------- +# A. Static fallback +# --------------------------------------------------------------------------- + +@pytest.mark.asyncio +async def test_get_stages_static_fallback_default(): + stages = await sf_stages.get_stages() + assert stages[0] == "Lead Gen" + assert "New Lead" in stages + assert "Closed Won" in stages + + +@pytest.mark.asyncio +async def test_get_stages_unknown_record_type_returns_empty(): + stages = await sf_stages.get_stages("NonexistentRecordType") + assert stages == () + + +# --------------------------------------------------------------------------- +# B. Process-local cache +# --------------------------------------------------------------------------- + +@pytest.mark.asyncio +async def test_cache_hit_avoids_db_on_second_call(): + fake_conn = AsyncMock() + fake_conn.fetch = AsyncMock(return_value=[ + {"value": "Lead Gen"}, + {"value": "Closed Won"}, + ]) + s1 = await sf_stages.get_stages("Philanthropy", db_conn=fake_conn) + s2 = await sf_stages.get_stages("Philanthropy", db_conn=fake_conn) + assert s1 == s2 == ("Lead Gen", "Closed Won") + fake_conn.fetch.assert_awaited_once() + + +@pytest.mark.asyncio +async def test_clear_cache_forces_refetch(): + fake_conn = AsyncMock() + fake_conn.fetch = AsyncMock(return_value=[{"value": "Lead Gen"}]) + await sf_stages.get_stages("Philanthropy", db_conn=fake_conn) + sf_stages.clear_cache() + await sf_stages.get_stages("Philanthropy", db_conn=fake_conn) + assert fake_conn.fetch.await_count == 2 + + +# --------------------------------------------------------------------------- +# C. DB read preferred when present +# --------------------------------------------------------------------------- + +@pytest.mark.asyncio +async def test_db_read_preferred_over_static_when_present(): + fake_conn = AsyncMock() + fake_conn.fetch = AsyncMock(return_value=[ + {"value": "Custom Stage One"}, + {"value": "Custom Stage Two"}, + ]) + stages = await sf_stages.get_stages("Philanthropy", db_conn=fake_conn) + assert stages == ("Custom Stage One", "Custom Stage Two") + + +# --------------------------------------------------------------------------- +# D. DB error falls through to static +# --------------------------------------------------------------------------- + +@pytest.mark.asyncio +async def test_db_error_falls_through_to_static(): + fake_conn = AsyncMock() + fake_conn.fetch = AsyncMock(side_effect=RuntimeError("DB unreachable")) + stages = await sf_stages.get_stages("Philanthropy", db_conn=fake_conn) + # Static fallback returns the documented Philanthropy list. + assert "Lead Gen" in stages + assert stages[0] == "Lead Gen" + + +@pytest.mark.asyncio +async def test_empty_db_result_falls_through_to_static(): + """Empty DB response (refresh job hasn't run yet, or all rows stale) + falls through to static — never returns empty for a known record type.""" + fake_conn = AsyncMock() + fake_conn.fetch = AsyncMock(return_value=[]) + stages = await sf_stages.get_stages("Philanthropy", db_conn=fake_conn) + assert stages != () + assert "Lead Gen" in stages + + +# --------------------------------------------------------------------------- +# E. Entry stage +# --------------------------------------------------------------------------- + +@pytest.mark.asyncio +async def test_entry_stage_philanthropy(): + assert await sf_stages.get_entry_stage("Philanthropy") == "New Lead" + + +@pytest.mark.asyncio +async def test_entry_stage_unknown_record_type_is_none(): + assert await sf_stages.get_entry_stage("PBC") is None + + +# --------------------------------------------------------------------------- +# F. Bucket predicates +# --------------------------------------------------------------------------- + +@pytest.mark.parametrize("stage,expected", [ + ("Closed Won", True), + ("Closed / Fulfilled", True), + ("New Lead", False), + ("Closed Lost", False), + ("Withdrawn", False), + ("Some Random Future Stage", False), +]) +def test_is_revenue_earning(stage, expected): + assert sf_stages.is_revenue_earning(stage) is expected + + +@pytest.mark.parametrize("stage,expected", [ + ("New Lead", True), + ("Ask in Progress", True), + ("Contracting", True), + ("Closed Won", False), + ("Closed Lost", False), +]) +def test_is_open(stage, expected): + assert sf_stages.is_open(stage) is expected + + +@pytest.mark.parametrize("stage,expected", [ + ("Closed Won", True), + ("Closed Lost", True), + ("Closed / Fulfilled", True), + ("Withdrawn", True), + ("New Lead", False), +]) +def test_is_closed(stage, expected): + assert sf_stages.is_closed(stage) is expected + + +@pytest.mark.parametrize("stage,expected", [ + ("Closed Lost", True), + ("Withdrawn", True), + ("Closed Won", False), + ("Closed / Fulfilled", False), +]) +def test_is_lost(stage, expected): + assert sf_stages.is_lost(stage) is expected + + +# --------------------------------------------------------------------------- +# G. Frozen sets +# --------------------------------------------------------------------------- + +def test_buckets_are_frozen(): + """Code mutating these sets is a bug — frozenset raises.""" + with pytest.raises(AttributeError): + sf_stages.REVENUE_EARNING_STAGES.add("Sneaky Stage") # type: ignore[attr-defined] + with pytest.raises(AttributeError): + sf_stages.OPEN_PIPELINE_STAGES.add("X") # type: ignore[attr-defined] + + +def test_bucket_set_semantics_handle_unknown_stages(): + """``if stage in BUCKET:`` returns False for stages we don't know + about — consistent with set membership semantics. The static + fallback / cache content can grow without code changes.""" + assert "Stage From The Future" not in sf_stages.OPEN_PIPELINE_STAGES + assert "Stage From The Future" not in sf_stages.CLOSED_STAGES + # And the predicates match. + assert sf_stages.is_open("Stage From The Future") is False + assert sf_stages.is_closed("Stage From The Future") is False + + +def test_bucket_invariants_no_overlap_revenue_lost(): + """A stage cannot be simultaneously revenue-earning and lost.""" + overlap = sf_stages.REVENUE_EARNING_STAGES & sf_stages.LOST_STAGES + assert overlap == frozenset() + + +def test_bucket_invariants_revenue_implies_closed(): + """Every revenue-earning stage must be a closed stage.""" + assert sf_stages.REVENUE_EARNING_STAGES.issubset(sf_stages.CLOSED_STAGES) + + +def test_bucket_invariants_lost_implies_closed(): + """Every lost stage must be a closed stage.""" + assert sf_stages.LOST_STAGES.issubset(sf_stages.CLOSED_STAGES) + + +def test_bucket_invariants_open_disjoint_closed(): + """Open and closed are mutually exclusive.""" + assert sf_stages.OPEN_PIPELINE_STAGES & sf_stages.CLOSED_STAGES == frozenset() From c1acc780bbd2170b332a820f587d80523f233f58 Mon Sep 17 00:00:00 2001 From: JP Date: Wed, 6 May 2026 19:28:20 -0400 Subject: [PATCH 07/35] feat(search): bedrock.search_doc + search_index_queue + search_audit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Layer 1.1 + 1.2 + 1.3 of the Pebble 1.0 search system. The durable schema for cross-entity, permission-aware, audit-logged search. bedrock.search_doc — denormalized one-row-per-entity index: * tsvector + GIN (lexical match) * pg_trgm + GIN on search_text (typo tolerance: "ed millr" → "Ed Miller") * pgvector halfvec(768) + HNSW reserved for v1.1 semantic overlay (column defined now so backfill never ALTER's a 200k-row hot table) * Permission columns FK-shaped (owner_sf_id, owner_email, account_sf_id, visibility) — pre-filter via accessible-id subquery, never post-filter, never denormalize ACL into rows * Soft-delete via partial GIN — tombstones in heap, never in posting list * search_doc_compose_vector() trigger sets relevance weights in one place (A: title, B: subtitle, C: search_text) * Multi-tenant org_id outer guard from day 1 bedrock.search_index_queue — durable hand-off: * UNIQUE(entity_type, entity_id, op) coalesces dup enqueues; ON CONFLICT bumps enqueued_at so the worker still drains in FIFO without missing any * Generic enqueue_search_index() trigger function takes entity_type via TG_ARGV[0]; source tables get one CREATE TRIGGER apiece in subsequent migrations * pg_notify('bedrock_search_index_queue', et||':'||eid) wakes the asyncio worker (drain pattern: FOR UPDATE SKIP LOCKED LIMIT 100) bedrock.search_audit — every query (Find / Ask / past_research): * query_id correlates query → results → click; request_id is Phase 0.7 idempotency key with UNIQUE replay defense * Two attribution columns: user_email (bearer; e.g. pebble@internal) + originating_user_email (delegated human) * query_text stored FULL at v1.0; query_text_hash is the default dashboard join key (no PII leak); raw text reads gated on manage_users_roles and self-audited via search_audit_access_log * Per-backend latency breakdown (perm_resolution_ms, backend_latency_ms) for SLO drill-down * result_count vs result_count_redacted exposes the permission-redaction leak surface * Cost columns mirror pebble_daily_usage shape so budget enforcement reads from one source All three migrations idempotent. Apply as bedrock owner. Plan refs: tasks/pebble-search-spec-backend.md §2-§3 tasks/pebble-search-spec-security.md §2 Co-Authored-By: Claude Opus 4.7 (1M context) --- .../db/migrations/2026-05-06-search-audit.sql | 143 +++++++++ .../db/migrations/2026-05-06-search-doc.sql | 271 ++++++++++++++++++ 2 files changed, 414 insertions(+) create mode 100644 financial_forecasting/db/migrations/2026-05-06-search-audit.sql create mode 100644 financial_forecasting/db/migrations/2026-05-06-search-doc.sql diff --git a/financial_forecasting/db/migrations/2026-05-06-search-audit.sql b/financial_forecasting/db/migrations/2026-05-06-search-audit.sql new file mode 100644 index 00000000..098953e8 --- /dev/null +++ b/financial_forecasting/db/migrations/2026-05-06-search-audit.sql @@ -0,0 +1,143 @@ +-- 2026-05-06: bedrock.search_audit +-- +-- Layer 1 / security spec §2. Every search query — Find or Ask, +-- human or service — gets one row here. +-- +-- Why: +-- * Compliance / data-access reviews — every query the team +-- makes is recoverable. +-- * Click-through analytics — drives ranking-quality dashboards. +-- * Anomaly detection — exfiltration patterns ("user reading +-- every contact in an hour") are visible to the batch job +-- described in security spec §4. +-- * Debug — "user said they searched X and got Y" is +-- reconstructible. +-- +-- Schema decisions (security spec §2): +-- * query_text stored full at v1.0, length-capped 256 chars at +-- the API. Default dashboards key on query_text_hash; raw +-- query_text reads gated on `manage_users_roles` and +-- themselves audited via bedrock.search_audit_access_log +-- (already shipped in 2026-05-06-pebble-write-audit.sql). +-- * 90 days hot in this table; older rows archived to GCS +-- monthly via Parquet (separate retention job). +-- * Two attribution columns: user_email is the bearer (e.g. +-- pebble@internal for service callers); originating_user_email +-- is the human whose session triggered Pebble. Both required +-- when caller is service. +-- * org_id outer guard from day 1 even though Pursuit is single +-- tenant today. +-- +-- Idempotent — safe to re-run. + +CREATE EXTENSION IF NOT EXISTS "uuid-ossp"; +CREATE EXTENSION IF NOT EXISTS citext; + +CREATE TABLE IF NOT EXISTS bedrock.search_audit ( + id UUID PRIMARY KEY DEFAULT uuid_generate_v4(), + occurred_at TIMESTAMPTZ NOT NULL DEFAULT now(), + + -- Correlation. query_id groups one query → its results → its + -- click. request_id is the per-HTTP-call idempotency key + -- (Phase 0.7) so a retried search query shows up as one row, + -- not duplicates. + query_id UUID NOT NULL, + request_id UUID NOT NULL, + + -- Attribution. + user_email CITEXT NOT NULL, -- bearer + originating_user_email CITEXT, -- delegated principal when bearer is service:* + org_id TEXT NOT NULL DEFAULT 'pursuit', + + -- The query. + mode TEXT NOT NULL CHECK (mode IN ('find','ask','past_research')), + query_text TEXT NOT NULL, + query_text_hash TEXT NOT NULL, -- sha256; default dashboard join key + facets JSONB, + types_requested TEXT[] DEFAULT '{}', -- entity_type filter; empty = all + + -- Backend served. + backend_used TEXT NOT NULL CHECK (backend_used IN + ('postgres_fts','pgvector','sf_sosl', + 'sf_sosl_fallback','cache_hit','degraded_empty')), + -- Per-backend latency breakdown for SLO drill-down. + perm_resolution_ms INT, + backend_latency_ms INT, + latency_ms INT NOT NULL, -- end-to-end including auth + serialize + + -- Result set. + result_count INT NOT NULL, -- visible to user + result_count_redacted INT, -- before permission redaction; >= result_count + response_status SMALLINT NOT NULL, + error_class TEXT, -- ExceptionClass when status >= 400 + + -- Click attribution. Updated by /api/search/click; left NULL + -- when no click happened. NULL is itself a metric (0-click + -- queries = poor ranking signal). + click_position SMALLINT, + click_entity_type TEXT, + click_record_id TEXT, + click_at TIMESTAMPTZ, + + -- Cost (Ask mode only). Mirrors pebble_daily_usage shape so + -- per-user budget enforcement reads from one source. + cost_usd NUMERIC(10, 6), + tokens_in INT, + tokens_out INT, + + -- Replay defense via UNIQUE on request_id, mirroring the + -- pebble_write_audit pattern. Safe across the 24h replay + -- window. + UNIQUE (request_id) +); + +-- Per-user history (drives /api/search/history and "user X did Y" lookups). +CREATE INDEX IF NOT EXISTS idx_search_audit_user_time + ON bedrock.search_audit(user_email, occurred_at DESC); + +-- Per-originating-user history when bearer is service:*. Lets us +-- ask "what has Pebble done on Jac's behalf today?" +CREATE INDEX IF NOT EXISTS idx_search_audit_origin_time + ON bedrock.search_audit(originating_user_email, occurred_at DESC) + WHERE originating_user_email IS NOT NULL; + +-- Hash-keyed dashboards (no PII leak). +CREATE INDEX IF NOT EXISTS idx_search_audit_qhash_time + ON bedrock.search_audit(query_text_hash, occurred_at DESC); + +-- Click correlation. +CREATE INDEX IF NOT EXISTS idx_search_audit_qid + ON bedrock.search_audit(query_id); + +-- 0-click queries (ranking-quality signal). +CREATE INDEX IF NOT EXISTS idx_search_audit_no_click + ON bedrock.search_audit(occurred_at DESC) + WHERE click_position IS NULL; + +-- Pebble-driven activity surface. +CREATE INDEX IF NOT EXISTS idx_search_audit_pebble + ON bedrock.search_audit(originating_user_email, occurred_at DESC) + WHERE user_email = 'pebble@internal'; + +-- Errors for ops dashboards. +CREATE INDEX IF NOT EXISTS idx_search_audit_errors + ON bedrock.search_audit(occurred_at DESC, error_class) + WHERE response_status >= 400; + +COMMENT ON TABLE bedrock.search_audit IS + 'Audit row for every search query (Phase 1.3). Populated by routes/search.py via FastAPI BackgroundTask. UNIQUE(request_id) doubles as 24h replay defense.'; +COMMENT ON COLUMN bedrock.search_audit.originating_user_email IS + 'When user_email is service:* (e.g. pebble@internal), this carries the human whose session triggered the workflow. Required for service callers; null for direct human searches.'; +COMMENT ON COLUMN bedrock.search_audit.result_count_redacted IS + 'Count BEFORE permission redaction. result_count_redacted >= result_count always. Delta is the leak surface — a high-delta-low-result query may indicate a permission misconfig.'; + +-- Grants +DO $$ +BEGIN + IF EXISTS (SELECT 1 FROM pg_roles WHERE rolname = 'bedrock_user') THEN + GRANT SELECT, INSERT, UPDATE ON bedrock.search_audit TO bedrock_user; + -- UPDATE granted because click attribution post-update on existing + -- rows is the click endpoint's job (only the click_* columns). + -- DELETE intentionally NOT granted — retention via separate role. + END IF; +END $$; diff --git a/financial_forecasting/db/migrations/2026-05-06-search-doc.sql b/financial_forecasting/db/migrations/2026-05-06-search-doc.sql new file mode 100644 index 00000000..c656e385 --- /dev/null +++ b/financial_forecasting/db/migrations/2026-05-06-search-doc.sql @@ -0,0 +1,271 @@ +-- 2026-05-06: bedrock.search_doc + bedrock.search_index_queue +-- +-- Layer 1 of the Pebble 1.0 search system. The denormalized, +-- cross-entity search index that GlobalSearch Find mode and Pebble's +-- Ask-mode tool calls both query against. Replaces the SOSL passthrough +-- at routes/salesforce_search.py. +-- +-- Design — see tasks/pebble-search-spec-backend.md §2 for the full +-- argument. Headlines: +-- +-- * One denormalized table, one row per searchable entity. Cross- +-- entity ranking uses a single query plan vs UNION ALL of N +-- per-entity indexes. +-- * tsvector + GIN on the composed search_vector for lexical match. +-- * pg_trgm + GIN on search_text for typo tolerance ("ed millr" → +-- "Ed Miller"). +-- * pgvector halfvec(768) embedding column reserved for v1.1 +-- semantic-recall overlay; not populated in v1.0. +-- * Permission columns are FK-shaped (owner_sf_id, owner_email, +-- account_sf_id, visibility) so the API layer can compose a +-- pre-filter accessible-id subquery without denormalizing +-- per-user ACL into rows. See backend spec §4 for the rejection +-- of the post-filter and ACL-array alternatives. +-- * Soft-delete via partial index — tombstones in the heap, never +-- in the GIN posting list. +-- * Multi-tenant org_id from day 1, even though Pursuit is single +-- tenant today (security spec §7). +-- +-- search_index_queue is the durable backbone of the indexer worker +-- pattern (backend spec §3). Every entity write enqueues a row; +-- LISTEN/NOTIFY wakes the worker; FOR UPDATE SKIP LOCKED lets us +-- shard later without changing the schema. +-- +-- Idempotent — safe to re-run. + +CREATE EXTENSION IF NOT EXISTS pg_trgm; +CREATE EXTENSION IF NOT EXISTS vector; + +-- --------------------------------------------------------------------------- +-- search_doc +-- --------------------------------------------------------------------------- + +CREATE TABLE IF NOT EXISTS bedrock.search_doc ( + -- Composite identity. The 11-value CHECK constraint is + -- intentionally explicit (not ENUM) so adding a new entity_type + -- is a plain ALTER TABLE rather than the multi-step ALTER TYPE + -- dance Postgres requires for enum mutation. + entity_type TEXT NOT NULL CHECK (entity_type IN ( + 'sf_account','sf_contact','sf_opportunity','sf_task','sf_activity', + 'bedrock_project','bedrock_award','bedrock_saved_view', + 'pebble_profile','pebble_chat_conversation','pebble_batch' + )), + entity_id TEXT NOT NULL, + PRIMARY KEY (entity_type, entity_id), + + -- Display projection. The frontend renders these directly; no + -- post-query JOIN to source tables on the read path. + title TEXT NOT NULL, + subtitle TEXT, + href TEXT NOT NULL, + + -- Lexical search. + search_vector TSVECTOR NOT NULL, + search_text TEXT NOT NULL, + + -- Semantic recall layer. Reserved for v1.1; v1.0 leaves this + -- column NULL. Defining it now means we don't have to + -- ALTER TABLE on a 200k-row hot table later. + embedding halfvec(768), + + -- Permission columns. Pre-filter via accessible-id subquery + -- (backend spec §4). owner_sf_id is the SF user id when the + -- entity is SF-mirrored; owner_email is the Bedrock-side + -- owner. account_sf_id is the parent account for permission + -- inheritance (Contact / Opp / Task visible to whoever can + -- see the parent Account, plus their direct owner). + owner_sf_id TEXT, + owner_email CITEXT, + account_sf_id TEXT, + visibility TEXT NOT NULL DEFAULT 'org' + CHECK (visibility IN ('private','team','org')), + + -- Recency signal. Computed at ranking time as + -- ts_rank_cd(...) * exp(-EXTRACT(epoch FROM now() - activity_at) / (86400 * 30)) + -- so half-life is tunable without reindexing. + activity_at TIMESTAMPTZ, + + created_at TIMESTAMPTZ NOT NULL DEFAULT now(), + updated_at TIMESTAMPTZ NOT NULL DEFAULT now(), + deleted_at TIMESTAMPTZ, + + -- Indexer metadata. source_version is the source row's + -- version (e.g. SF SystemModstamp or bedrock row's updated_at) + -- so the indexer can skip re-indexing when the source hasn't + -- changed since indexed_at. + indexed_at TIMESTAMPTZ NOT NULL DEFAULT now(), + source_version TIMESTAMPTZ, + + -- Multi-tenant outer guard. + org_id TEXT NOT NULL DEFAULT 'pursuit' +); + +-- Lexical search index. Partial on deleted_at IS NULL means +-- soft-deleted tombstones never enter the GIN posting list — they +-- still consume heap, but search ignores them. +CREATE INDEX IF NOT EXISTS idx_search_doc_fts + ON bedrock.search_doc USING GIN(search_vector) + WHERE deleted_at IS NULL; + +-- Trigram fuzzy match for typo tolerance. Same partial predicate. +CREATE INDEX IF NOT EXISTS idx_search_doc_trgm + ON bedrock.search_doc USING GIN(search_text gin_trgm_ops) + WHERE deleted_at IS NULL; + +-- Permission filter inputs. +CREATE INDEX IF NOT EXISTS idx_search_doc_owner_sf + ON bedrock.search_doc(owner_sf_id) + WHERE deleted_at IS NULL AND owner_sf_id IS NOT NULL; +CREATE INDEX IF NOT EXISTS idx_search_doc_owner_email + ON bedrock.search_doc(owner_email) + WHERE deleted_at IS NULL AND owner_email IS NOT NULL; +CREATE INDEX IF NOT EXISTS idx_search_doc_account + ON bedrock.search_doc(account_sf_id) + WHERE deleted_at IS NULL AND account_sf_id IS NOT NULL; + +-- Multi-tenant guard. +CREATE INDEX IF NOT EXISTS idx_search_doc_org + ON bedrock.search_doc(org_id) + WHERE deleted_at IS NULL; + +-- Per-entity-type filtering for facets ("show me only Opps"). +CREATE INDEX IF NOT EXISTS idx_search_doc_entity + ON bedrock.search_doc(entity_type) + WHERE deleted_at IS NULL; + +-- Recency tiebreaker / "recent" scope. +CREATE INDEX IF NOT EXISTS idx_search_doc_activity + ON bedrock.search_doc(activity_at DESC) + WHERE deleted_at IS NULL AND activity_at IS NOT NULL; + +-- pgvector index reserved for v1.1. Created as a no-op now (no +-- rows have embeddings yet) so the schema is fixed-shape before +-- backfill begins. +CREATE INDEX IF NOT EXISTS idx_search_doc_embedding_hnsw + ON bedrock.search_doc USING hnsw (embedding halfvec_cosine_ops) + WHERE embedding IS NOT NULL; + +-- --------------------------------------------------------------------------- +-- search_vector composer trigger. Single weighting policy so the +-- relevance contract is in one place, not scattered across writers. +-- --------------------------------------------------------------------------- + +CREATE OR REPLACE FUNCTION bedrock.search_doc_compose_vector() RETURNS trigger AS $$ +BEGIN + NEW.search_vector := + setweight(to_tsvector('english', COALESCE(NEW.title, '')), 'A') || + setweight(to_tsvector('english', COALESCE(NEW.subtitle, '')), 'B') || + setweight(to_tsvector('english', COALESCE(NEW.search_text, '')), 'C'); + NEW.updated_at := now(); + RETURN NEW; +END; +$$ LANGUAGE plpgsql; + +DO $$ +BEGIN + IF NOT EXISTS ( + SELECT 1 FROM pg_trigger WHERE tgname = 'trg_search_doc_compose' + ) THEN + CREATE TRIGGER trg_search_doc_compose + BEFORE INSERT OR UPDATE ON bedrock.search_doc + FOR EACH ROW EXECUTE FUNCTION bedrock.search_doc_compose_vector(); + END IF; +END $$; + +-- --------------------------------------------------------------------------- +-- search_index_queue — durable hand-off from source-table writes +-- to the asynchronous indexer worker. +-- --------------------------------------------------------------------------- + +CREATE TABLE IF NOT EXISTS bedrock.search_index_queue ( + id BIGSERIAL PRIMARY KEY, + entity_type TEXT NOT NULL, + entity_id TEXT NOT NULL, + op TEXT NOT NULL CHECK (op IN ('upsert','delete')), + enqueued_at TIMESTAMPTZ NOT NULL DEFAULT now(), + attempt_count INT NOT NULL DEFAULT 0, + last_error TEXT, + + -- Coalesce: multiple enqueues of the same entity collapse to + -- one row; ON CONFLICT bumps enqueued_at so the worker + -- processes them in FIFO order without missing any. + UNIQUE (entity_type, entity_id, op) +); + +-- Drain index. Worker uses +-- SELECT ... FROM bedrock.search_index_queue +-- ORDER BY enqueued_at ASC FOR UPDATE SKIP LOCKED LIMIT 100 +-- so this index serves both the order-by and the lock-skip. +CREATE INDEX IF NOT EXISTS idx_search_queue_drain + ON bedrock.search_index_queue(enqueued_at); + +-- Failed-row index. Periodic reconciliation reads attempt_count > 0 +-- to surface stuck rows. +CREATE INDEX IF NOT EXISTS idx_search_queue_stuck + ON bedrock.search_index_queue(attempt_count, enqueued_at) + WHERE attempt_count > 0; + +-- --------------------------------------------------------------------------- +-- Generic enqueue trigger. Source-table triggers reference this +-- with a TG_ARGV[0] entity_type tag. Bedrock-native source tables +-- get one CREATE TRIGGER apiece in subsequent migrations. +-- --------------------------------------------------------------------------- + +CREATE OR REPLACE FUNCTION bedrock.enqueue_search_index() RETURNS trigger AS $$ +DECLARE + et TEXT := TG_ARGV[0]; + eid TEXT; + op_val TEXT; +BEGIN + IF TG_OP = 'DELETE' THEN + eid := OLD.id::TEXT; + op_val := 'delete'; + ELSE + eid := NEW.id::TEXT; + op_val := 'upsert'; + END IF; + + INSERT INTO bedrock.search_index_queue (entity_type, entity_id, op) + VALUES (et, eid, op_val) + ON CONFLICT (entity_type, entity_id, op) DO UPDATE + SET enqueued_at = now(), attempt_count = 0, last_error = NULL; + + -- LISTEN/NOTIFY wakes the worker; payload includes the key so a + -- cleverer worker could skip the SELECT for the hot path. + PERFORM pg_notify('bedrock_search_index_queue', et || ':' || eid); + + RETURN COALESCE(NEW, OLD); +END; +$$ LANGUAGE plpgsql; + +-- --------------------------------------------------------------------------- +-- updated_at trigger reuses the existing helper. +-- --------------------------------------------------------------------------- + +DO $$ +BEGIN + IF NOT EXISTS ( + SELECT 1 FROM pg_trigger WHERE tgname = 'trg_search_doc_updated_at' + ) THEN + CREATE TRIGGER trg_search_doc_updated_at + BEFORE UPDATE ON bedrock.search_doc + FOR EACH ROW EXECUTE FUNCTION bedrock.set_updated_at(); + END IF; +END $$; + +COMMENT ON TABLE bedrock.search_doc IS + 'Denormalized cross-entity search index (Phase 1.1). One row per searchable entity; populated by the queue-drain indexer worker. Permission filtering is pre-filter via accessible-id subquery.'; +COMMENT ON COLUMN bedrock.search_doc.embedding IS + 'pgvector halfvec(768) for v1.1 semantic-recall overlay. NULL in v1.0 — schema reserved so backfill never has to ALTER.'; +COMMENT ON TABLE bedrock.search_index_queue IS + 'Durable queue from source-table writes to the indexer worker. ON CONFLICT coalesces duplicates; LISTEN/NOTIFY on bedrock_search_index_queue wakes the drain.'; + +-- Grants +DO $$ +BEGIN + IF EXISTS (SELECT 1 FROM pg_roles WHERE rolname = 'bedrock_user') THEN + GRANT SELECT, INSERT, UPDATE, DELETE ON bedrock.search_doc TO bedrock_user; + GRANT SELECT, INSERT, UPDATE, DELETE ON bedrock.search_index_queue TO bedrock_user; + GRANT USAGE ON SEQUENCE bedrock.search_index_queue_id_seq TO bedrock_user; + END IF; +END $$; From 130000ff2e2e374b4640c479134baa8475af0602 Mon Sep 17 00:00:00 2001 From: JP Date: Wed, 6 May 2026 19:34:28 -0400 Subject: [PATCH 08/35] =?UTF-8?q?feat(search):=20/api/search=20read=20path?= =?UTF-8?q?=20=E2=80=94=20service=20+=20route=20+=2056=20tests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Layer 1.8 of the Pebble 1.0 search system. The cross-entity Find endpoint that GlobalSearch.tsx Find mode and Pebble's Ask-mode tool calls both query against. Replaces the SOSL passthrough at routes/salesforce_search.py incrementally (dual-read in Phase 2). services/search_service.py: * SearchPrincipal — the effective principal whose permissions filter the query. Service callers (Pebble, is_service=True) resolve to their X-Originating-User; Pebble is delegated, not god-principal. * resolve_principal — reads bedrock.org_users + permission_profiles to compose the principal. Non-existent user → maximally restrictive (no rows visible). * _compose_permission_predicate — the OR-chain that the backend spec §4 argues for: pre-filter via accessible-id subquery, never post-filter, never denormalize ACL into rows. Multi-tenant org_id is the OUTERMOST predicate, always. * search() — composes the WITH-CTE query, applies recency decay (ts_rank_cd × exp(-Δs / 30-day-half-life)), returns rank-ordered SearchHits. * Strict input validation: type list rejects unknown entity_type; limit clamping to MAX_LIMIT=100; empty query short-circuits without DB hit. routes/search.py: * GET /api/search — happy path returns flat + grouped views so the frontend picks shape it wants. * POST /api/search/click — click attribution for ranking quality. Fenced on user_email match (one user can't poison another's history). * Permission gate: check_permission_or_internal("view_opportunities") via a stable module-level reference (require_search_perm) so tests can override. * Audit row INSERT runs as a BackgroundTask — never blocks the response. ON CONFLICT (request_id) DO NOTHING idempotency. Audit failure is logged, never propagates to the user. * X-Request-Id used as audit row request_id. Malformed header → mint UUID, log warning. Empty header → mint UUID. * Long queries truncated at 256 chars in audit_text (defense against bypass of the 256 max_length validator). 56 tests across both files: * test_search_service: 35 — covers the OR-chain composition cell-by-cell (admin path, view-all path, restricted path, each branch of the OR), org_id always present invariant, type validation, limit clamping, service-spoof protection, query_text_hash stability + case-insensitivity, e2e against a mocked connection. * test_search_route: 21 — happy path, empty results, unknown entity_type → 400, missing/empty/oversized inputs → 422, audit BackgroundTask invoked, audit failure swallowed, click endpoint validation, header parsing helpers. Plan refs: tasks/pebble-search-spec-backend.md §2-§5 tasks/pebble-search-spec-security.md §1, §2 tasks/pebble-search-spec.md decisions §2, §4, §6 Co-Authored-By: Claude Opus 4.7 (1M context) --- financial_forecasting/routes/search.py | 337 ++++++++++++++ .../services/search_service.py | 409 +++++++++++++++++ .../tests/test_search_route.py | 295 +++++++++++++ .../tests/test_search_service.py | 413 ++++++++++++++++++ 4 files changed, 1454 insertions(+) create mode 100644 financial_forecasting/routes/search.py create mode 100644 financial_forecasting/services/search_service.py create mode 100644 financial_forecasting/tests/test_search_route.py create mode 100644 financial_forecasting/tests/test_search_service.py diff --git a/financial_forecasting/routes/search.py b/financial_forecasting/routes/search.py new file mode 100644 index 00000000..d571c411 --- /dev/null +++ b/financial_forecasting/routes/search.py @@ -0,0 +1,337 @@ +"""Search API — Layer 1.8 of the Pebble 1.0 plan. + +Endpoints: + GET /api/search — cross-entity find against bedrock.search_doc + POST /api/search/click — click attribution for ranking quality + +The route layer is deliberately thin: it parses + validates input, +calls ``services/search_service.py`` for the heavy lifting, emits an +audit row to ``bedrock.search_audit`` via FastAPI BackgroundTask, and +serializes the response. + +Permission semantics: + * Auth via ``check_permission_or_internal("view_opportunities")`` — + the lightest read perm that's gated for the search-eligible + profiles. The actual record-level filter happens INSIDE the + service, against the resolved SearchPrincipal. + * Service callers (Pebble, ``is_service=True``) carry an + ``X-Originating-User`` and the filter resolves against THAT + user's permissions, not the service account's. + +Observability: + * Every search attempt writes one row to bedrock.search_audit. + * Audit row write is a BackgroundTask — never blocks the response. + * Click endpoint UPDATEs the same row on click, fenced on the + caller matching the original user_email. +""" + +from __future__ import annotations + +import json +import logging +import time +import uuid +from typing import Any, Optional + +from fastapi import APIRouter, BackgroundTasks, Depends, HTTPException, Query, Request +from pydantic import BaseModel, Field + +from db import get_db +from routes.permissions import check_permission_or_internal +from services import search_service as ss + +logger = logging.getLogger(__name__) +router = APIRouter(prefix="/api/search", tags=["search"]) + +# Module-level stable reference so tests can override via +# ``app.dependency_overrides[require_search_perm]``. Without this, the +# ``check_permission_or_internal("view_opportunities")`` closure is +# unique per call and the override key never matches. +require_search_perm = check_permission_or_internal("view_opportunities") + + +# --------------------------------------------------------------------------- +# Request / response models +# --------------------------------------------------------------------------- + +class SearchHitOut(BaseModel): + entity_type: str + entity_id: str + title: str + subtitle: Optional[str] = None + href: str + rank: float + activity_at: Optional[str] = None + indexed_at: str + group: str + + +class SearchResponseOut(BaseModel): + query_id: str + items: list[SearchHitOut] + grouped: dict[str, list[SearchHitOut]] + total_count: int + backend_used: str + took_ms: int + + +class SearchClickIn(BaseModel): + query_id: str = Field(..., description="UUID returned from the original /api/search response") + position: int = Field(..., ge=0, description="0-based rank position of clicked hit") + entity_type: str + entity_id: str + + +class SearchClickOut(BaseModel): + ok: bool + + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + +def _normalize_types(types: Optional[str]) -> Optional[list[str]]: + """Comma-separated query-string list → list. None / empty → None + (= all types per ``SearchRequest`` default). + """ + if not types: + return None + parts = [t.strip() for t in types.split(",") if t.strip()] + return parts or None + + +def _request_uuid(request: Request) -> str: + """X-Request-Id is mandatory on internal-key calls (set by + require_auth_or_internal). Direct human callers may not send it; + we mint one in that case so audit rows always have a UUID. + """ + raw = request.headers.get("X-Request-Id", "").strip() + if raw: + try: + uuid.UUID(raw) + return raw + except ValueError: + # Malformed header: log and replace; never let a bad client + # poison the audit log. + logger.warning("Malformed X-Request-Id %r; minting fresh", raw) + return str(uuid.uuid4()) + + +def _truncate_query(query: str, max_len: int = 256) -> str: + """Per security spec §2: query_text length-capped at 256 chars + server-side. Anything longer is rejected at the request layer to + avoid DoS via long queries; this is a defensive truncate for the + audit row in case validation is ever loosened.""" + return query[:max_len] if query else "" + + +async def _emit_search_audit( + pool, + *, + query_id: uuid.UUID, + request_id: str, + user_email: str, + originating_user_email: Optional[str], + org_id: str, + mode: str, + query: str, + types_requested: list[str], + backend_used: str, + perm_resolution_ms: int, + backend_latency_ms: int, + latency_ms: int, + result_count: int, + response_status: int, + error_class: Optional[str] = None, +) -> None: + """Insert the audit row. Called from a BackgroundTask so it never + blocks the response. Uses ON CONFLICT DO NOTHING on the + UNIQUE(request_id) so retried requests don't duplicate. + """ + try: + async with pool.acquire() as conn: + await conn.execute( + """ + INSERT INTO bedrock.search_audit ( + query_id, request_id, user_email, + originating_user_email, org_id, mode, + query_text, query_text_hash, types_requested, + backend_used, perm_resolution_ms, backend_latency_ms, + latency_ms, result_count, response_status, error_class + ) VALUES ( + $1, $2, $3, $4, $5, $6, $7, $8, $9, + $10, $11, $12, $13, $14, $15, $16 + ) + ON CONFLICT (request_id) DO NOTHING + """, + query_id, uuid.UUID(request_id), user_email, + originating_user_email, org_id, mode, + _truncate_query(query), ss.query_text_hash(query), + types_requested, + backend_used, perm_resolution_ms, backend_latency_ms, + latency_ms, result_count, response_status, error_class, + ) + except Exception: + # Audit-row failures must NEVER affect the user response. + # Log loudly — alerting will catch sustained failure. + logger.exception( + "search_audit_insert_failed query_id=%s request_id=%s", + query_id, request_id, + ) + + +# --------------------------------------------------------------------------- +# GET /api/search +# --------------------------------------------------------------------------- + +@router.get("", response_model=SearchResponseOut) +async def search_endpoint( + request: Request, + background_tasks: BackgroundTasks, + q: str = Query(..., min_length=1, max_length=256, description="Search query text"), + types: Optional[str] = Query(None, description="Comma-separated entity_type filter"), + limit: int = Query(ss.DEFAULT_LIMIT, ge=1, le=ss.MAX_LIMIT), + pool=Depends(get_db), + user=Depends(require_search_perm), +): + """Cross-entity search. Returns ranked hits plus a UI grouping. + + The grouping is computed server-side so the frontend doesn't have + to know the entity_type → group label map. + """ + started = time.perf_counter() + request_id = _request_uuid(request) + + # Resolve principal — service callers fall through to their + # originating user, humans are themselves. + perm_started = time.perf_counter() + async with pool.acquire() as conn: + try: + principal = await ss.resolve_principal(conn, user) + except ValueError as e: + logger.warning("Principal resolution failed: %s", e) + raise HTTPException(status_code=400, detail=str(e)) + perm_ms = int((time.perf_counter() - perm_started) * 1000) + + # Normalize the request. + try: + type_list = _normalize_types(types) + req = ss.SearchRequest( + query=q.strip(), + types=type_list, + limit=limit, + org_id=principal.org_id, + ) + # Trigger the type-validation (raises ValueError on bad input). + req.normalized_types() + except ValueError as e: + raise HTTPException(status_code=400, detail=str(e)) + + # Run the search. + backend_started = time.perf_counter() + error_class: Optional[str] = None + response_status = 200 + try: + async with pool.acquire() as conn: + result = await ss.search(conn, principal, req) + except Exception as e: + error_class = type(e).__name__ + response_status = 500 + logger.exception("search_failed query=%s", q) + raise HTTPException(status_code=500, detail="search_failed") + finally: + backend_ms = int((time.perf_counter() - backend_started) * 1000) + total_ms = int((time.perf_counter() - started) * 1000) + + # Background audit emission — never blocks the response. + background_tasks.add_task( + _emit_search_audit, + pool, + query_id=result.query_id if response_status == 200 else uuid.uuid4(), + request_id=request_id, + user_email=user.get("email", "unknown"), + originating_user_email=user.get("originating_user_email"), + org_id=principal.org_id, + mode="find", + query=q, + types_requested=list(req.normalized_types()), + backend_used=getattr(result, "backend_used", "postgres_fts") if response_status == 200 else "degraded_empty", + perm_resolution_ms=perm_ms, + backend_latency_ms=backend_ms, + latency_ms=total_ms, + result_count=len(result.items) if response_status == 200 else 0, + response_status=response_status, + error_class=error_class, + ) + + # Pack the response with both flat + grouped views so the frontend + # can pick the shape it wants. + items_out = [ + SearchHitOut( + entity_type=h.entity_type, + entity_id=h.entity_id, + title=h.title, + subtitle=h.subtitle, + href=h.href, + rank=h.rank, + activity_at=h.activity_at, + indexed_at=h.indexed_at, + group=h.group, + ) + for h in result.items + ] + grouped: dict[str, list[SearchHitOut]] = {} + for hit in items_out: + grouped.setdefault(hit.group, []).append(hit) + + return SearchResponseOut( + query_id=str(result.query_id), + items=items_out, + grouped=grouped, + total_count=len(items_out), + backend_used=result.backend_used, + took_ms=result.took_ms, + ) + + +# --------------------------------------------------------------------------- +# POST /api/search/click +# --------------------------------------------------------------------------- + +@router.post("/click", response_model=SearchClickOut) +async def search_click_endpoint( + request: Request, + body: SearchClickIn, + pool=Depends(get_db), + user=Depends(require_search_perm), +): + """Attribute a click to a prior search query for ranking-quality + analysis. Fenced on the caller matching the original audit row's + user_email so a user can't poison someone else's search history. + """ + try: + query_uuid = uuid.UUID(body.query_id) + except ValueError: + raise HTTPException(status_code=400, detail="invalid query_id") + + user_email = user.get("email", "") + + async with pool.acquire() as conn: + result = await conn.execute( + """ + UPDATE bedrock.search_audit + SET click_position = $1, + click_entity_type = $2, + click_record_id = $3, + click_at = now() + WHERE query_id = $4 + AND user_email = $5 + AND click_position IS NULL + """, + body.position, body.entity_type, body.entity_id, + query_uuid, user_email, + ) + + # asyncpg returns "UPDATE n" — n = 0 means no matching audit row, + # which we don't surface to the caller (silent no-op). + return SearchClickOut(ok=True) diff --git a/financial_forecasting/services/search_service.py b/financial_forecasting/services/search_service.py new file mode 100644 index 00000000..1b04fa92 --- /dev/null +++ b/financial_forecasting/services/search_service.py @@ -0,0 +1,409 @@ +"""Search service — read API for ``bedrock.search_doc``. + +Phase 1.8 of the Pebble 1.0 plan. The service that ``routes/search.py`` +calls. Owns the permission-filter SQL composition, ranking math, and +response shaping. Pebble's ``search_crm`` tool calls the same service +via the route, so there's one canonical read path no matter who the +caller is. + +Key design choices (argued in tasks/pebble-search-spec-backend.md): + + * Permission filter is **pre-filter** in SQL, never post-filter in + Python. The DB enforces row-level visibility. + * Ranking is ``ts_rank_cd * recency_decay`` so the half-life can be + tuned without reindexing. + * Result rows are denormalized — the API never JOINs back to source + tables on the read path. The indexer's job is to compose the + ``title / subtitle / search_text`` projection at write time. + * Multi-tenant ``org_id`` is the OUTERMOST WHERE predicate. CI grep + enforces this; tests assert it as well. + * Service callers (Pebble, ``is_service=True``) carry an + ``originating_user_email`` and the filter resolves against THAT + user's permissions. Pebble is delegated, never god-principal. + +The service is deliberately tiny so it stays testable. The route +layer adds rate-limiting, audit emission, and response packaging +on top. +""" + +from __future__ import annotations + +import hashlib +import logging +import time +from dataclasses import dataclass, field +from typing import Any, Iterable, Optional, Sequence +from uuid import UUID, uuid4 + +logger = logging.getLogger(__name__) + + +# --------------------------------------------------------------------------- +# Types +# --------------------------------------------------------------------------- + +# Every entity_type the indexer is expected to populate. The CHECK +# constraint on bedrock.search_doc.entity_type carries the same list; +# they must stay in sync. +ALL_ENTITY_TYPES: tuple[str, ...] = ( + "sf_account", "sf_contact", "sf_opportunity", "sf_task", "sf_activity", + "bedrock_project", "bedrock_award", "bedrock_saved_view", + "pebble_profile", "pebble_chat_conversation", "pebble_batch", +) + +# UI grouping. Maps the storage entity_type → display group so the +# frontend can group without a second lookup. +ENTITY_GROUP_LABEL: dict[str, str] = { + "sf_account": "Accounts", + "sf_contact": "Contacts", + "sf_opportunity": "Opportunities", + "sf_task": "Tasks", + "sf_activity": "Activities", + "bedrock_project": "Projects", + "bedrock_award": "Awards", + "bedrock_saved_view": "Saved Views", + "pebble_profile": "Researched Prospects", + "pebble_chat_conversation": "Pebble Conversations", + "pebble_batch": "Pebble Batches", +} + +# Entity types whose visibility = 'org' overrides ownership filtering. +# Per the security spec: SF Accounts and Contacts are org-visible by +# default; Opps / Tasks / Activities follow ownership. +ORG_VISIBLE_ENTITY_TYPES: frozenset[str] = frozenset({ + "sf_account", "sf_contact", +}) + +# Default per-query result cap. Hard-capped at 100 to bound memory + +# transit. Frontend defaults to 8 per group. +DEFAULT_LIMIT = 25 +MAX_LIMIT = 100 + +# Recency half-life in days. ts_rank_cd × exp(-Δseconds / SECONDS_PER_HALFLIFE). +RECENCY_HALFLIFE_DAYS = 30 +_SECONDS_PER_HALFLIFE = 86400 * RECENCY_HALFLIFE_DAYS + + +# --------------------------------------------------------------------------- +# Caller identity +# --------------------------------------------------------------------------- + +@dataclass(frozen=True) +class SearchPrincipal: + """The effective principal whose permissions filter the search. + + For a JWT-authenticated user this is the user themself. For a + service caller (Pebble) this is the *originating user* — Pebble + is the bearer but the human is the principal whose visibility + rules apply. + """ + email: str + sf_user_id: Optional[str] + is_admin: bool + has_view_all_accounts: bool + has_view_all_opportunities: bool + has_view_all_contacts: bool + org_id: str = "pursuit" + + @property + def can_see_everything(self) -> bool: + return self.is_admin or all(( + self.has_view_all_accounts, + self.has_view_all_opportunities, + self.has_view_all_contacts, + )) + + +async def resolve_principal( + db_conn, + user: dict[str, Any], +) -> SearchPrincipal: + """Resolve the SearchPrincipal from the user dict produced by + ``require_auth_or_internal``. + + For service callers (``is_service=True``) we use + ``originating_user_email`` — Pebble acts on behalf of, never as + itself, on the read side. Auth dependency already rejects service + calls without that header (Phase 0.2). + """ + if user.get("is_service"): + email = user.get("originating_user_email") + if not email: + raise ValueError( + "Service caller missing originating_user_email — " + "auth.require_auth_or_internal should have rejected this." + ) + else: + email = user.get("email") + if not email: + raise ValueError("User dict missing email") + + row = await db_conn.fetchrow( + """ + SELECT + u.email, + u.sf_user_id, + (pp.permissions ->> 'manage_users_roles')::boolean + AS is_admin, + COALESCE( + (pp.permissions ->> 'edit_all_opportunities')::boolean, + FALSE + ) AS has_view_all_opportunities, + COALESCE( + (pp.permissions ->> 'edit_accounts')::boolean, + FALSE + ) AS has_view_all_accounts, + COALESCE( + (pp.permissions ->> 'edit_contacts')::boolean, + FALSE + ) AS has_view_all_contacts, + COALESCE(u.org_id, 'pursuit') AS org_id + FROM bedrock.org_users u + LEFT JOIN bedrock.permission_profiles pp + ON u.permission_profile_id = pp.id + WHERE u.email = $1 + """, + email, + ) + if not row: + # User not in org_users — locked out of search by default. + # Service callers spoofing a non-existent user land here. + return SearchPrincipal( + email=email, + sf_user_id=None, + is_admin=False, + has_view_all_accounts=False, + has_view_all_opportunities=False, + has_view_all_contacts=False, + ) + return SearchPrincipal( + email=row["email"], + sf_user_id=row["sf_user_id"], + is_admin=bool(row["is_admin"]), + has_view_all_accounts=bool(row["has_view_all_accounts"]), + has_view_all_opportunities=bool(row["has_view_all_opportunities"]), + has_view_all_contacts=bool(row["has_view_all_contacts"]), + org_id=row["org_id"] or "pursuit", + ) + + +# --------------------------------------------------------------------------- +# Search request + response +# --------------------------------------------------------------------------- + +@dataclass(frozen=True) +class SearchRequest: + query: str + types: Optional[Sequence[str]] = None # None = all entity types + limit: int = DEFAULT_LIMIT + org_id: str = "pursuit" + + def normalized_types(self) -> tuple[str, ...]: + if not self.types: + return ALL_ENTITY_TYPES + invalid = [t for t in self.types if t not in ALL_ENTITY_TYPES] + if invalid: + raise ValueError(f"Unknown entity_type(s): {invalid}") + return tuple(self.types) + + def normalized_limit(self) -> int: + if self.limit < 1: + return DEFAULT_LIMIT + return min(self.limit, MAX_LIMIT) + + +@dataclass(frozen=True) +class SearchHit: + entity_type: str + entity_id: str + title: str + subtitle: Optional[str] + href: str + rank: float + activity_at: Optional[str] # ISO8601 string for JSON + indexed_at: str # ISO8601 string for JSON + + @property + def group(self) -> str: + return ENTITY_GROUP_LABEL.get(self.entity_type, self.entity_type) + + +@dataclass +class SearchResponse: + query_id: UUID + items: list[SearchHit] = field(default_factory=list) + total_count_redacted: int = 0 + backend_used: str = "postgres_fts" + took_ms: int = 0 + + +# --------------------------------------------------------------------------- +# Permission filter SQL composition +# --------------------------------------------------------------------------- + +def _compose_permission_predicate( + principal: SearchPrincipal, + *, + sql_param_offset: int, +) -> tuple[str, list[Any]]: + """Return (where_clause, params) appended after the existing query + builder. Composes the OR-chain that defines record-level visibility. + + ``sql_param_offset`` is the count of $-params already used by the + query builder, so the placeholders here line up. + """ + # Admin / "view all" callers see everything in their org. We still + # filter on org_id, which is the multi-tenant outermost guard. + if principal.can_see_everything: + params = [principal.org_id] + return (f"org_id = ${sql_param_offset + 1}", params) + + clauses: list[str] = [] + params: list[Any] = [] + pi = sql_param_offset + + # 1. Multi-tenant outer guard. ALWAYS first. + pi += 1 + clauses.append(f"org_id = ${pi}") + params.append(principal.org_id) + + # Inner OR group — at least one must match. + or_clauses: list[str] = [] + + # 2a. Org-visible entity types (Accounts, Contacts). + org_visible_list = ", ".join(f"'{t}'" for t in sorted(ORG_VISIBLE_ENTITY_TYPES)) + or_clauses.append( + f"(entity_type IN ({org_visible_list}) AND visibility = 'org')" + ) + + # 2b. Owned-by-this-user via SF user id (SF-mirrored entities). + if principal.sf_user_id: + pi += 1 + or_clauses.append(f"owner_sf_id = ${pi}") + params.append(principal.sf_user_id) + + # 2c. Owned-by-this-user via email (Bedrock-native entities). + pi += 1 + or_clauses.append(f"owner_email = ${pi}") + params.append(principal.email) + + # 2d. View-all overrides for specific entity types. + if principal.has_view_all_opportunities: + or_clauses.append("entity_type = 'sf_opportunity'") + if principal.has_view_all_contacts: + or_clauses.append("entity_type = 'sf_contact'") + if principal.has_view_all_accounts: + or_clauses.append("entity_type = 'sf_account'") + + clauses.append("(" + " OR ".join(or_clauses) + ")") + + return (" AND ".join(clauses), params) + + +# --------------------------------------------------------------------------- +# Public read API +# --------------------------------------------------------------------------- + +def query_text_hash(query: str) -> str: + """sha256 of the canonical query text. Used as the dashboard join + key and in the audit-log row. + """ + return hashlib.sha256(query.strip().lower().encode("utf-8")).hexdigest() + + +async def search( + db_conn, + principal: SearchPrincipal, + req: SearchRequest, +) -> SearchResponse: + """Run a Find search and return the top-K hits. + + Composes: + SELECT entity_type, entity_id, title, subtitle, href, + ts_rank_cd(...) * recency_decay AS rank, + activity_at, indexed_at + FROM bedrock.search_doc, websearch_to_tsquery('english', $q) AS q + WHERE search_vector @@ q + AND deleted_at IS NULL + AND entity_type = ANY($types) + AND + ORDER BY rank DESC + LIMIT $limit + """ + started = time.perf_counter() + + types = req.normalized_types() + limit = req.normalized_limit() + query_text = req.query.strip() + + if not query_text: + return SearchResponse(query_id=uuid4(), backend_used="cache_hit") + + # Build the permission predicate first so we know how many params it + # consumes before we lay out the rest. + base_params: list[Any] = [query_text, list(types)] # $1, $2 + perm_clause, perm_params = _compose_permission_predicate( + principal, sql_param_offset=len(base_params), + ) + limit_param_idx = len(base_params) + len(perm_params) + 1 + + sql = f""" + WITH q AS (SELECT websearch_to_tsquery('english', $1) AS tsq) + SELECT + entity_type, + entity_id, + title, + subtitle, + href, + ts_rank_cd(search_vector, q.tsq) * + exp(-EXTRACT(epoch FROM (now() - COALESCE(activity_at, indexed_at))) + / {_SECONDS_PER_HALFLIFE}::float8) + AS rank, + activity_at, + indexed_at + FROM bedrock.search_doc, q + WHERE search_vector @@ q.tsq + AND deleted_at IS NULL + AND entity_type = ANY($2) + AND {perm_clause} + ORDER BY rank DESC + LIMIT ${limit_param_idx} + """ + + params: list[Any] = base_params + perm_params + [limit] + rows = await db_conn.fetch(sql, *params) + + items = [ + SearchHit( + entity_type=r["entity_type"], + entity_id=r["entity_id"], + title=r["title"], + subtitle=r["subtitle"], + href=r["href"], + rank=float(r["rank"]), + activity_at=r["activity_at"].isoformat() if r["activity_at"] else None, + indexed_at=r["indexed_at"].isoformat(), + ) + for r in rows + ] + took_ms = int((time.perf_counter() - started) * 1000) + + return SearchResponse( + query_id=uuid4(), + items=items, + total_count_redacted=len(items), # count-after-redaction + backend_used="postgres_fts", + took_ms=took_ms, + ) + + +# --------------------------------------------------------------------------- +# Helpers exposed for the route layer + tests +# --------------------------------------------------------------------------- + +def group_hits(hits: Iterable[SearchHit]) -> dict[str, list[SearchHit]]: + """Bucket hits by their UI group label, preserving rank order.""" + out: dict[str, list[SearchHit]] = {} + for hit in hits: + out.setdefault(hit.group, []).append(hit) + return out diff --git a/financial_forecasting/tests/test_search_route.py b/financial_forecasting/tests/test_search_route.py new file mode 100644 index 00000000..0820ae2f --- /dev/null +++ b/financial_forecasting/tests/test_search_route.py @@ -0,0 +1,295 @@ +"""Route-level tests for ``/api/search`` (Phase 1.8) using +FastAPI TestClient + dependency overrides. + +Asserts: + A. Happy path returns 200 with expected response shape. + B. Empty / 0-result query returns 200 with empty items array. + C. Unknown entity_type → 400. + D. Bad limits → 422 (Pydantic validation). + E. Audit row is enqueued via BackgroundTask. + F. Audit failure does NOT propagate to the user response. + G. Service caller resolves to originating user's principal. + H. Click endpoint fences on caller user_email. +""" + +import os +import sys +import uuid +from contextlib import asynccontextmanager +from datetime import datetime, timezone +from unittest.mock import AsyncMock, MagicMock + +import pytest +from fastapi import FastAPI + +sys.path.insert(0, os.path.join(os.path.dirname(__file__), '..')) +sys.path.insert(0, os.path.dirname(__file__)) + +from fastapi.testclient import TestClient + +from db import get_db +from routes import search as search_route +from services import search_service as ss + + +# --------------------------------------------------------------------------- +# Test app — bare minimum so we don't drag in main.py's startup. +# --------------------------------------------------------------------------- + +@pytest.fixture +def app_factory(): + """Build a minimal FastAPI app with /api/search mounted, plus + overridable dependencies for db/auth. + """ + def _build( + *, + user: dict | None = None, + principal: ss.SearchPrincipal | None = None, + search_result: ss.SearchResponse | None = None, + audit_should_fail: bool = False, + ) -> tuple[FastAPI, MagicMock]: + app = FastAPI() + app.include_router(search_route.router) + + # Mock pool: yields a mock connection that returns the prepared + # principal + search result. + mock_conn = AsyncMock() + mock_conn.execute = AsyncMock(return_value="UPDATE 1") + if audit_should_fail: + mock_conn.execute.side_effect = RuntimeError("DB down") + + # principal resolution short-circuit + async def _fake_resolve(conn, user): + return principal or ss.SearchPrincipal( + email=user.get("email", "test@pursuit.org"), + sf_user_id="005AAA", + is_admin=False, + has_view_all_accounts=False, + has_view_all_opportunities=False, + has_view_all_contacts=False, + ) + + async def _fake_search(conn, principal, req): + return search_result or ss.SearchResponse( + query_id=uuid.uuid4(), + items=[], + backend_used="postgres_fts", + took_ms=12, + ) + + # Patch the search_service functions used by the route. + search_route.ss.resolve_principal = _fake_resolve + search_route.ss.search = _fake_search + + # Mock pool with async context manager support. + @asynccontextmanager + async def _acquire(): + yield mock_conn + + mock_pool = MagicMock() + mock_pool.acquire = lambda: _acquire() + + async def _override_db(): + return mock_pool + + async def _override_perm(request=None): + return user or { + "user_id": "u-1", "email": "test@pursuit.org", + "is_service": False, + } + + app.dependency_overrides[get_db] = _override_db + app.dependency_overrides[search_route.require_search_perm] = _override_perm + + return app, mock_conn + + return _build + + +# --------------------------------------------------------------------------- +# A. Happy path +# --------------------------------------------------------------------------- + +def test_search_happy_path_returns_200(app_factory): + items = [ + ss.SearchHit( + entity_type="sf_account", entity_id="001AAA", + title="Acme Corp", subtitle="Customer · Enterprise", + href="/accounts/001AAA", rank=0.95, + activity_at=None, + indexed_at=datetime(2026, 5, 6, tzinfo=timezone.utc).isoformat(), + ), + ] + response = ss.SearchResponse( + query_id=uuid.uuid4(), items=items, + backend_used="postgres_fts", took_ms=42, + ) + app, _ = app_factory(search_result=response) + + with TestClient(app) as client: + r = client.get("/api/search?q=acme") + assert r.status_code == 200 + body = r.json() + assert body["total_count"] == 1 + assert body["items"][0]["title"] == "Acme Corp" + assert body["items"][0]["group"] == "Accounts" + assert "Accounts" in body["grouped"] + assert body["backend_used"] == "postgres_fts" + + +def test_search_empty_results_returns_200(app_factory): + app, _ = app_factory() + with TestClient(app) as client: + r = client.get("/api/search?q=zzz") + assert r.status_code == 200 + body = r.json() + assert body["items"] == [] + assert body["grouped"] == {} + assert body["total_count"] == 0 + + +# --------------------------------------------------------------------------- +# C. Unknown entity_type → 400 +# --------------------------------------------------------------------------- + +def test_unknown_entity_type_returns_400(app_factory): + app, _ = app_factory() + with TestClient(app) as client: + r = client.get("/api/search?q=acme&types=fake_entity") + assert r.status_code == 400 + assert "Unknown entity_type" in r.json()["detail"] + + +# --------------------------------------------------------------------------- +# D. Pydantic validation +# --------------------------------------------------------------------------- + +def test_missing_q_returns_422(app_factory): + app, _ = app_factory() + with TestClient(app) as client: + r = client.get("/api/search") + assert r.status_code == 422 + + +def test_empty_q_returns_422(app_factory): + app, _ = app_factory() + with TestClient(app) as client: + r = client.get("/api/search?q=") + assert r.status_code == 422 + + +def test_oversized_limit_returns_422(app_factory): + app, _ = app_factory() + with TestClient(app) as client: + r = client.get(f"/api/search?q=acme&limit={ss.MAX_LIMIT + 100}") + assert r.status_code == 422 + + +# --------------------------------------------------------------------------- +# E. Audit row enqueued (synchronously after BackgroundTask completes) +# --------------------------------------------------------------------------- + +def test_audit_row_inserted_via_background_task(app_factory): + app, mock_conn = app_factory() + with TestClient(app) as client: + r = client.get("/api/search?q=acme&types=sf_account") + assert r.status_code == 200 + # mock_conn.execute is called twice: once by the route's audit + # INSERT (BackgroundTask runs after response is sent in TestClient). + assert mock_conn.execute.await_count >= 1 + # The first arg of the first call should be the INSERT into search_audit. + sent_sql = mock_conn.execute.call_args.args[0] + assert "INSERT INTO bedrock.search_audit" in sent_sql + assert "ON CONFLICT (request_id) DO NOTHING" in sent_sql + + +# --------------------------------------------------------------------------- +# F. Audit failure swallowed +# --------------------------------------------------------------------------- + +def test_audit_failure_does_not_break_response(app_factory): + """Audit row insert raising must NOT 500 the user response.""" + app, _ = app_factory(audit_should_fail=True) + with TestClient(app) as client: + r = client.get("/api/search?q=acme") + assert r.status_code == 200 + + +# --------------------------------------------------------------------------- +# H. Click endpoint +# --------------------------------------------------------------------------- + +def test_click_returns_ok(app_factory): + app, _ = app_factory() + with TestClient(app) as client: + r = client.post("/api/search/click", json={ + "query_id": str(uuid.uuid4()), + "position": 0, + "entity_type": "sf_account", + "entity_id": "001AAA", + }) + assert r.status_code == 200 + assert r.json() == {"ok": True} + + +def test_click_invalid_query_id_returns_400(app_factory): + app, _ = app_factory() + with TestClient(app) as client: + r = client.post("/api/search/click", json={ + "query_id": "not-a-uuid", + "position": 0, + "entity_type": "sf_account", + "entity_id": "001AAA", + }) + assert r.status_code == 400 + + +def test_click_negative_position_returns_422(app_factory): + app, _ = app_factory() + with TestClient(app) as client: + r = client.post("/api/search/click", json={ + "query_id": str(uuid.uuid4()), + "position": -1, + "entity_type": "sf_account", + "entity_id": "001AAA", + }) + assert r.status_code == 422 + + +# --------------------------------------------------------------------------- +# Helpers — _normalize_types + _request_uuid +# --------------------------------------------------------------------------- + +@pytest.mark.parametrize("raw,expected", [ + (None, None), + ("", None), + (" ", None), + ("sf_account", ["sf_account"]), + ("sf_account,sf_contact", ["sf_account", "sf_contact"]), + (" sf_account , sf_contact ", ["sf_account", "sf_contact"]), + ("sf_account,,sf_contact", ["sf_account", "sf_contact"]), +]) +def test_normalize_types(raw, expected): + assert search_route._normalize_types(raw) == expected + + +def test_request_uuid_uses_header_when_valid(): + valid = "01993b8d-2c9a-7c4f-8b0e-000000000001" + request = MagicMock() + request.headers = {"X-Request-Id": valid} + assert search_route._request_uuid(request) == valid + + +def test_request_uuid_mints_when_header_missing(): + request = MagicMock() + request.headers = {} + minted = search_route._request_uuid(request) + uuid.UUID(minted) # raises if not a valid UUID + + +def test_request_uuid_mints_when_header_malformed(): + request = MagicMock() + request.headers = {"X-Request-Id": "not-a-uuid"} + minted = search_route._request_uuid(request) + uuid.UUID(minted) # raises if not a valid UUID + assert minted != "not-a-uuid" diff --git a/financial_forecasting/tests/test_search_service.py b/financial_forecasting/tests/test_search_service.py new file mode 100644 index 00000000..976e4f64 --- /dev/null +++ b/financial_forecasting/tests/test_search_service.py @@ -0,0 +1,413 @@ +"""Tests for ``services/search_service.py`` — Phase 1.8 read API. + +Locks in the load-bearing invariants: + +A. Multi-tenant ``org_id`` is ALWAYS in the WHERE predicate. +B. Admin / view-all callers get a simplified predicate but org_id + still applies. +C. Non-admin callers get the OR-chain (org-visible + owner_sf_id + + owner_email + view-all overrides). +D. Org-visible entity types (Accounts, Contacts) bypass ownership + when visibility='org'. +E. Owned-by-this-user via sf_user_id when caller has it. +F. Owned-by-this-user via email always. +G. Empty query returns empty response, not a database hit. +H. Type validation rejects unknown entity_type strings. +I. Limit clamping (negative → default; > MAX → MAX). +J. Service callers spoofing a non-existent originating user get the + most-restrictive (logged-out-shaped) principal — no rows visible. +K. ``query_text_hash`` is stable + case-insensitive. +""" + +import os +import sys +from datetime import datetime, timezone +from unittest.mock import AsyncMock + +import pytest + +sys.path.insert(0, os.path.join(os.path.dirname(__file__), '..')) + +from services import search_service as ss + + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + +def _principal( + *, + email: str = "rm@pursuit.org", + sf_user_id: str | None = "005AAA", + is_admin: bool = False, + view_all_accounts: bool = False, + view_all_opps: bool = False, + view_all_contacts: bool = False, + org_id: str = "pursuit", +) -> ss.SearchPrincipal: + return ss.SearchPrincipal( + email=email, + sf_user_id=sf_user_id, + is_admin=is_admin, + has_view_all_accounts=view_all_accounts, + has_view_all_opportunities=view_all_opps, + has_view_all_contacts=view_all_contacts, + org_id=org_id, + ) + + +def _row( + *, + entity_type: str = "sf_account", + entity_id: str = "001AAA", + title: str = "Acme", + subtitle: str | None = None, + href: str = "/accounts/001AAA", + rank: float = 1.0, + activity_at: datetime | None = None, + indexed_at: datetime | None = None, +) -> dict: + return { + "entity_type": entity_type, + "entity_id": entity_id, + "title": title, + "subtitle": subtitle, + "href": href, + "rank": rank, + "activity_at": activity_at, + "indexed_at": indexed_at or datetime(2026, 5, 6, tzinfo=timezone.utc), + } + + +# --------------------------------------------------------------------------- +# A. Multi-tenant org_id always present +# --------------------------------------------------------------------------- + +def test_admin_predicate_keeps_org_id(): + pred, params = ss._compose_permission_predicate( + _principal(is_admin=True), sql_param_offset=2, + ) + assert "org_id = $3" in pred + assert params == ["pursuit"] + + +def test_non_admin_predicate_keeps_org_id_first(): + pred, _ = ss._compose_permission_predicate( + _principal(), sql_param_offset=2, + ) + # org_id must be the first WHERE clause for index seek efficiency. + assert pred.startswith("org_id = $3") + + +def test_custom_org_id_propagates(): + _, params = ss._compose_permission_predicate( + _principal(org_id="other_tenant"), sql_param_offset=2, + ) + assert "other_tenant" in params + + +# --------------------------------------------------------------------------- +# B. Admin path +# --------------------------------------------------------------------------- + +def test_admin_predicate_skips_or_chain(): + """Admins see everything → no OR-chain, just org_id.""" + pred, _ = ss._compose_permission_predicate( + _principal(is_admin=True), sql_param_offset=2, + ) + assert " OR " not in pred + assert "owner_sf_id" not in pred + assert "owner_email" not in pred + + +def test_full_view_all_grant_skips_or_chain(): + """A non-admin user with all three view_all_* permissions gets the + fast-path (no OR-chain), since they can see everything.""" + pred, _ = ss._compose_permission_predicate( + _principal( + view_all_accounts=True, + view_all_opps=True, + view_all_contacts=True, + ), + sql_param_offset=2, + ) + assert " OR " not in pred + + +# --------------------------------------------------------------------------- +# C, D, E, F. OR-chain composition for restricted users +# --------------------------------------------------------------------------- + +def test_restricted_predicate_includes_org_visible_clause(): + pred, _ = ss._compose_permission_predicate( + _principal(), sql_param_offset=2, + ) + # Org-visible entity types (sorted): sf_account, sf_contact + assert "'sf_account'" in pred + assert "'sf_contact'" in pred + assert "visibility = 'org'" in pred + + +def test_restricted_predicate_includes_owner_sf_id_when_present(): + pred, params = ss._compose_permission_predicate( + _principal(sf_user_id="005XYZ"), sql_param_offset=2, + ) + assert "owner_sf_id" in pred + assert "005XYZ" in params + + +def test_restricted_predicate_omits_owner_sf_id_when_missing(): + pred, params = ss._compose_permission_predicate( + _principal(sf_user_id=None), sql_param_offset=2, + ) + assert "owner_sf_id" not in pred + assert "005XYZ" not in params + + +def test_restricted_predicate_always_includes_owner_email(): + pred, params = ss._compose_permission_predicate( + _principal(email="rm@pursuit.org"), sql_param_offset=2, + ) + assert "owner_email" in pred + assert "rm@pursuit.org" in params + + +def test_view_all_opportunities_adds_opp_branch(): + pred, _ = ss._compose_permission_predicate( + _principal(view_all_opps=True), sql_param_offset=2, + ) + assert "entity_type = 'sf_opportunity'" in pred + + +def test_view_all_contacts_adds_contact_branch(): + pred, _ = ss._compose_permission_predicate( + _principal(view_all_contacts=True), sql_param_offset=2, + ) + assert "entity_type = 'sf_contact'" in pred + + +def test_view_all_accounts_adds_account_branch(): + pred, _ = ss._compose_permission_predicate( + _principal(view_all_accounts=True), sql_param_offset=2, + ) + assert "entity_type = 'sf_account'" in pred + + +# --------------------------------------------------------------------------- +# G. Empty query short-circuits +# --------------------------------------------------------------------------- + +@pytest.mark.asyncio +async def test_empty_query_returns_empty_without_db_hit(): + fake_conn = AsyncMock() + fake_conn.fetch = AsyncMock(return_value=[]) + resp = await ss.search(fake_conn, _principal(), ss.SearchRequest(query=" ")) + assert resp.items == [] + assert resp.backend_used == "cache_hit" + fake_conn.fetch.assert_not_called() + + +# --------------------------------------------------------------------------- +# H. Entity-type validation +# --------------------------------------------------------------------------- + +def test_unknown_entity_type_rejected(): + req = ss.SearchRequest(query="x", types=["totally_fake"]) + with pytest.raises(ValueError, match=r"Unknown entity_type"): + req.normalized_types() + + +def test_known_entity_types_accepted(): + req = ss.SearchRequest(query="x", types=["sf_account", "pebble_profile"]) + assert req.normalized_types() == ("sf_account", "pebble_profile") + + +def test_default_entity_types_is_all(): + req = ss.SearchRequest(query="x") + assert req.normalized_types() == ss.ALL_ENTITY_TYPES + + +# --------------------------------------------------------------------------- +# I. Limit clamping +# --------------------------------------------------------------------------- + +@pytest.mark.parametrize("raw,expected", [ + (0, ss.DEFAULT_LIMIT), + (-5, ss.DEFAULT_LIMIT), + (1, 1), + (50, 50), + (ss.MAX_LIMIT, ss.MAX_LIMIT), + (ss.MAX_LIMIT + 1, ss.MAX_LIMIT), + (10000, ss.MAX_LIMIT), +]) +def test_limit_clamping(raw, expected): + assert ss.SearchRequest(query="x", limit=raw).normalized_limit() == expected + + +# --------------------------------------------------------------------------- +# J. Service-caller spoof of non-existent user → most restrictive +# --------------------------------------------------------------------------- + +@pytest.mark.asyncio +async def test_resolve_principal_for_nonexistent_user_is_restrictive(): + """Pebble carrying X-Originating-User=ghost@example.com hits this + path. We don't 401 (auth already passed) but we do return a + locked-down principal so search returns nothing.""" + fake_conn = AsyncMock() + fake_conn.fetchrow = AsyncMock(return_value=None) + user = { + "is_service": True, + "originating_user_email": "ghost@example.com", + } + principal = await ss.resolve_principal(fake_conn, user) + assert principal.email == "ghost@example.com" + assert principal.sf_user_id is None + assert principal.is_admin is False + assert principal.has_view_all_accounts is False + assert principal.has_view_all_opportunities is False + assert principal.has_view_all_contacts is False + + +@pytest.mark.asyncio +async def test_resolve_principal_for_real_user(): + fake_conn = AsyncMock() + fake_conn.fetchrow = AsyncMock(return_value={ + "email": "rm@pursuit.org", + "sf_user_id": "005ABC", + "is_admin": False, + "has_view_all_opportunities": False, + "has_view_all_accounts": True, + "has_view_all_contacts": False, + "org_id": "pursuit", + }) + principal = await ss.resolve_principal( + fake_conn, {"email": "rm@pursuit.org"}, + ) + assert principal.email == "rm@pursuit.org" + assert principal.sf_user_id == "005ABC" + assert principal.has_view_all_accounts is True + assert principal.has_view_all_opportunities is False + + +@pytest.mark.asyncio +async def test_resolve_principal_service_without_originating_user_raises(): + fake_conn = AsyncMock() + user = {"is_service": True} # missing originating_user_email + with pytest.raises(ValueError, match=r"originating_user_email"): + await ss.resolve_principal(fake_conn, user) + + +# --------------------------------------------------------------------------- +# K. query_text_hash +# --------------------------------------------------------------------------- + +def test_query_text_hash_stable(): + h1 = ss.query_text_hash("Acme Corp") + h2 = ss.query_text_hash("Acme Corp") + assert h1 == h2 + + +def test_query_text_hash_case_insensitive(): + assert ss.query_text_hash("acme corp") == ss.query_text_hash("ACME CORP") + + +def test_query_text_hash_strips_whitespace(): + assert ss.query_text_hash(" acme ") == ss.query_text_hash("acme") + + +def test_query_text_hash_distinguishes_distinct_queries(): + assert ss.query_text_hash("acme") != ss.query_text_hash("widget") + + +# --------------------------------------------------------------------------- +# End-to-end search() with a mocked connection +# --------------------------------------------------------------------------- + +@pytest.mark.asyncio +async def test_search_returns_hits_with_correct_grouping(): + fake_conn = AsyncMock() + fake_conn.fetch = AsyncMock(return_value=[ + _row(entity_type="sf_account", title="Acme", rank=0.9), + _row(entity_type="sf_contact", entity_id="003BBB", + title="Wile E. Coyote", subtitle="Acme · CEO", + href="/contacts/003BBB", rank=0.8), + _row(entity_type="pebble_profile", entity_id="prof-1", + title="MetLife Foundation", subtitle="Researched 4d ago", + href="/pebble/profiles/prof-1", rank=0.7), + ]) + + resp = await ss.search(fake_conn, _principal(), ss.SearchRequest(query="acme")) + + assert len(resp.items) == 3 + # Group labels should match ENTITY_GROUP_LABEL. + assert resp.items[0].group == "Accounts" + assert resp.items[1].group == "Contacts" + assert resp.items[2].group == "Researched Prospects" + # The query SQL must include the org_id predicate and the perm OR-chain. + sent_sql, *_ = fake_conn.fetch.call_args.args + assert "org_id = $" in sent_sql + assert "search_vector @@ q.tsq" in sent_sql + assert "ts_rank_cd" in sent_sql + assert "deleted_at IS NULL" in sent_sql + + +@pytest.mark.asyncio +async def test_search_admin_predicate_does_not_include_owner_filters(): + fake_conn = AsyncMock() + fake_conn.fetch = AsyncMock(return_value=[]) + await ss.search( + fake_conn, _principal(is_admin=True), ss.SearchRequest(query="x"), + ) + sent_sql, *_ = fake_conn.fetch.call_args.args + assert "owner_sf_id" not in sent_sql + assert "owner_email" not in sent_sql + # But org_id MUST still be there. + assert "org_id = $" in sent_sql + + +@pytest.mark.asyncio +async def test_search_passes_query_text_and_types_as_params(): + fake_conn = AsyncMock() + fake_conn.fetch = AsyncMock(return_value=[]) + await ss.search( + fake_conn, + _principal(), + ss.SearchRequest(query="metlife", types=["sf_account", "pebble_profile"]), + ) + sent_args = fake_conn.fetch.call_args.args + # First param is the query, second is the type-array. + assert sent_args[1] == "metlife" + assert sent_args[2] == ["sf_account", "pebble_profile"] + + +# --------------------------------------------------------------------------- +# Group labelling +# --------------------------------------------------------------------------- + +def test_group_hits_preserves_rank_order(): + a = ss.SearchHit( + entity_type="sf_account", entity_id="1", + title="A", subtitle=None, href="/", rank=2.0, + activity_at=None, indexed_at="2026-05-06", + ) + b = ss.SearchHit( + entity_type="sf_account", entity_id="2", + title="B", subtitle=None, href="/", rank=1.0, + activity_at=None, indexed_at="2026-05-06", + ) + c = ss.SearchHit( + entity_type="pebble_profile", entity_id="3", + title="C", subtitle=None, href="/", rank=0.5, + activity_at=None, indexed_at="2026-05-06", + ) + grouped = ss.group_hits([a, b, c]) + assert list(grouped.keys()) == ["Accounts", "Researched Prospects"] + assert grouped["Accounts"] == [a, b] + assert grouped["Researched Prospects"] == [c] + + +def test_all_entity_types_have_group_labels(): + """No silent missing labels — every entity_type the indexer can + produce has a UI group.""" + for et in ss.ALL_ENTITY_TYPES: + assert et in ss.ENTITY_GROUP_LABEL, f"missing label for {et}" From aa8b4193fd334b44830aa8804cd61b0fc62ac46e Mon Sep 17 00:00:00 2001 From: JP Date: Wed, 6 May 2026 19:38:06 -0400 Subject: [PATCH 09/35] feat(search): indexer worker + composers + source-table triggers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Layer 1.6 of the Pebble 1.0 search system. Drains bedrock.search_index_queue and upserts the denormalized projection into bedrock.search_doc. services/search_indexer.py: * Composer registry — one async fn per entity_type that reads the source row and produces a SearchDocRow projection. Composers can return None to signal "row is gone or shouldn't be indexed"; indexer soft-deletes in search_doc. * Built-in composers for bedrock_project, bedrock_saved_view, pebble_profile (sourced from bedrock.pebble_research_sessions where status='completed' — has explicit name/org/tier columns vs pebble_profiles' opaque profile_json). * drain_once(pool) — claims a batch with FOR UPDATE SKIP LOCKED, dispatches per row, removes successes, bumps attempt_count on errors. Single transaction per row so a poison pill doesn't block the queue head. * MAX_ATTEMPT_COUNT=5 hard cap; attempt-exhausted rows stay in queue with last_error populated for the periodic reconciliation job. * No-composer rows leave the queue intact (no attempt bump) — a future worker with the composer registered processes them without restart. This is how the bedrock_award trigger (wired below) gracefully waits for its composer. * run_worker(pool, stop_event) — long-running drain. LISTEN on bedrock_search_index_queue + 2s polling fallback. Exponential backoff to 60s on consecutive failures; never crashes the FastAPI process. * backfill(pool, entity_type) — one-shot reindex from source. Used after schema changes or for first-deploy population. db/migrations/2026-05-06-search-source-triggers.sql: * AFTER INSERT/UPDATE/DELETE triggers on bedrock.project, bedrock.saved_view, bedrock.award. * Conditional trigger on bedrock.pebble_research_sessions (only fires when status='completed') + a separate DELETE trigger (NEW is null in AFTER DELETE so WHEN can't reference NEW.status). * bedrock.award trigger ships ahead of its composer — by the time the composer lands (depends on Layer 1.4 SF mirrors), the queue has every award row already enqueued, no separate backfill needed. 19 tests in test_search_indexer.py: * Registry: register/get/clear, default registrations, overwrites. * Composer correctness for all 3 built-ins (happy / deleted / missing / wrong-status / invalid JSON). * drain_once: upsert + dequeue, soft-delete on op='delete', no-composer leaves queue intact, composer error bumps attempt_count + records last_error, MAX_ATTEMPT_COUNT enforced in SELECT. * backfill: enqueues all source rows, rejects unregistered entity_type. This module is callable + testable in isolation. Wire-up to main.py lifespan is a separate change. Plan refs: tasks/pebble-search-spec-backend.md §3 tasks/pebble-search-spec.md Layer 1.6 Co-Authored-By: Claude Opus 4.7 (1M context) --- .../2026-05-06-search-source-triggers.sql | 82 +++ .../services/search_indexer.py | 508 ++++++++++++++++++ .../tests/test_search_indexer.py | 340 ++++++++++++ 3 files changed, 930 insertions(+) create mode 100644 financial_forecasting/db/migrations/2026-05-06-search-source-triggers.sql create mode 100644 financial_forecasting/services/search_indexer.py create mode 100644 financial_forecasting/tests/test_search_indexer.py diff --git a/financial_forecasting/db/migrations/2026-05-06-search-source-triggers.sql b/financial_forecasting/db/migrations/2026-05-06-search-source-triggers.sql new file mode 100644 index 00000000..618b6e86 --- /dev/null +++ b/financial_forecasting/db/migrations/2026-05-06-search-source-triggers.sql @@ -0,0 +1,82 @@ +-- 2026-05-06: source-table triggers wiring bedrock entities to the +-- search index queue. +-- +-- Layer 1.6 of the Pebble 1.0 plan. Adds AFTER-INSERT/UPDATE/DELETE +-- triggers on the bedrock-side source tables so every write enqueues +-- a search_index_queue row. The indexer worker drains the queue and +-- composes search_doc rows. +-- +-- Sources wired in this migration: +-- * bedrock.project → entity_type 'bedrock_project' +-- * bedrock.saved_view → entity_type 'bedrock_saved_view' +-- * bedrock.pebble_research_sessions → entity_type 'pebble_profile' +-- (only when status='completed' — +-- in-progress sessions don't +-- appear in search) +-- +-- SF-mirrored sources (Account, Contact, Opportunity, Task, Activity) +-- get wired in a separate migration once the SF mirror tables exist +-- (Layer 1.4). Award (bedrock.award) is wired here too — name is +-- composed from the linked Opportunity which we don't yet mirror; for +-- v1 the composer can defer until Layer 1.4 lands. +-- +-- Idempotent — DROP TRIGGER IF EXISTS before each CREATE. + +-- --------------------------------------------------------------------------- +-- bedrock.project +-- --------------------------------------------------------------------------- + +DROP TRIGGER IF EXISTS trg_project_search_index ON bedrock.project; +CREATE TRIGGER trg_project_search_index + AFTER INSERT OR UPDATE OR DELETE ON bedrock.project + FOR EACH ROW EXECUTE FUNCTION bedrock.enqueue_search_index('bedrock_project'); + +-- --------------------------------------------------------------------------- +-- bedrock.saved_view +-- --------------------------------------------------------------------------- + +DROP TRIGGER IF EXISTS trg_saved_view_search_index ON bedrock.saved_view; +CREATE TRIGGER trg_saved_view_search_index + AFTER INSERT OR UPDATE OR DELETE ON bedrock.saved_view + FOR EACH ROW EXECUTE FUNCTION bedrock.enqueue_search_index('bedrock_saved_view'); + +-- --------------------------------------------------------------------------- +-- bedrock.pebble_research_sessions +-- +-- Only enqueue when status transitions into 'completed' OR for +-- already-completed rows on UPDATE. In-progress sessions don't appear +-- in search; the composer would return None and we'd waste a queue +-- cycle. Use a WHEN clause to short-circuit at the trigger. +-- --------------------------------------------------------------------------- + +DROP TRIGGER IF EXISTS trg_pebble_research_sessions_search_index ON bedrock.pebble_research_sessions; +CREATE TRIGGER trg_pebble_research_sessions_search_index + AFTER INSERT OR UPDATE ON bedrock.pebble_research_sessions + FOR EACH ROW + WHEN (NEW.status = 'completed') + EXECUTE FUNCTION bedrock.enqueue_search_index('pebble_profile'); + +-- DELETE needs its own trigger since OLD-only refs aren't valid in +-- the WHEN clause for AFTER-DELETE (NEW is null). +DROP TRIGGER IF EXISTS trg_pebble_research_sessions_search_delete ON bedrock.pebble_research_sessions; +CREATE TRIGGER trg_pebble_research_sessions_search_delete + AFTER DELETE ON bedrock.pebble_research_sessions + FOR EACH ROW EXECUTE FUNCTION bedrock.enqueue_search_index('pebble_profile'); + +-- --------------------------------------------------------------------------- +-- bedrock.award +-- +-- The composer for entity_type='bedrock_award' isn't shipped yet +-- (needs Opp.Name from a future SF mirror). We still wire the trigger +-- so by the time the composer lands, the queue has every existing +-- award row already enqueued — no separate backfill pass needed. +-- +-- Until the composer is registered, the indexer logs +-- 'no_composer' and leaves the queue row in place (silent skip, +-- no failure bump). Confirmed in test_drain_once_no_composer_leaves_queue_intact. +-- --------------------------------------------------------------------------- + +DROP TRIGGER IF EXISTS trg_award_search_index ON bedrock.award; +CREATE TRIGGER trg_award_search_index + AFTER INSERT OR UPDATE OR DELETE ON bedrock.award + FOR EACH ROW EXECUTE FUNCTION bedrock.enqueue_search_index('bedrock_award'); diff --git a/financial_forecasting/services/search_indexer.py b/financial_forecasting/services/search_indexer.py new file mode 100644 index 00000000..c0e26530 --- /dev/null +++ b/financial_forecasting/services/search_indexer.py @@ -0,0 +1,508 @@ +"""Search indexer worker — Layer 1.6 of the Pebble 1.0 plan. + +Drains ``bedrock.search_index_queue`` and upserts rows into +``bedrock.search_doc``. One row per searchable entity, composed at +write time so the read path never needs to JOIN back to source. + +Architecture (backend spec §3.4): + + Source-table trigger + → bedrock.search_index_queue (UNIQUE coalesces dups) + → pg_notify('bedrock_search_index_queue', 'entity_type:entity_id') + → indexer worker LISTENs + drains FOR UPDATE SKIP LOCKED LIMIT 100 + → composer fn (per entity_type) reads source row + composes search_doc + → INSERT INTO bedrock.search_doc ... ON CONFLICT (entity_type, entity_id) DO UPDATE + → DELETE FROM bedrock.search_index_queue WHERE id = $1 + → next row + +Composer registry: + + Each entity_type has an async composer function + composer(conn, entity_id) -> SearchDocRow | None + that reads the source row and produces the denormalized + projection. Returning None signals "row is gone or shouldn't be + indexed" — the indexer marks the source as deleted in search_doc + via deleted_at. + +Failure handling: + + Per-row failures bump attempt_count and store last_error. Hard + failure at attempt_count >= 5; row stays in queue with + ``last_error`` populated for the periodic reconciliation job. + Worker never crashes the FastAPI process; transient errors + are retried with exponential backoff baked into the polling loop. + +This module exposes: + * ``register_composer(entity_type, fn)`` — registry hook + * ``compose_*`` — built-in composers for bedrock-side entities + * ``drain_once(pool, max_rows=100)`` — one drain cycle + * ``run_worker(pool, stop_event)`` — long-running asyncio task + * ``backfill(pool, entity_type)`` — one-shot reindex from source + +The wire-up to ``main.py`` lifespan happens in a separate change. +This module is callable + testable in isolation. +""" + +from __future__ import annotations + +import asyncio +import json +import logging +import time +from dataclasses import dataclass, field +from typing import Any, Awaitable, Callable, Optional + +logger = logging.getLogger(__name__) + +# Queue drain configuration. Conservative defaults; configurable via +# env at the lifespan wire-up site. +DEFAULT_BATCH_SIZE = 100 +MAX_ATTEMPT_COUNT = 5 +POLL_INTERVAL_SECONDS = 2.0 # belt-and-suspenders alongside LISTEN/NOTIFY +BACKOFF_BASE_SECONDS = 1.0 +BACKOFF_MAX_SECONDS = 60.0 + + +# --------------------------------------------------------------------------- +# Composer types + registry +# --------------------------------------------------------------------------- + +@dataclass(frozen=True) +class SearchDocRow: + """The denormalized projection a composer produces.""" + entity_type: str + entity_id: str + title: str + subtitle: Optional[str] + href: str + search_text: str + owner_sf_id: Optional[str] = None + owner_email: Optional[str] = None + account_sf_id: Optional[str] = None + visibility: str = "org" + activity_at: Any = None # datetime | None + source_version: Any = None # datetime | None + org_id: str = "pursuit" + + +# Composer signature. +ComposerFn = Callable[[Any, str], Awaitable[Optional[SearchDocRow]]] + +_composers: dict[str, ComposerFn] = {} + + +def register_composer(entity_type: str, fn: ComposerFn) -> None: + """Register a composer for an entity_type. Idempotent — re-register + overwrites (useful for tests). + """ + _composers[entity_type] = fn + + +def get_composer(entity_type: str) -> Optional[ComposerFn]: + return _composers.get(entity_type) + + +def registered_entity_types() -> tuple[str, ...]: + return tuple(_composers.keys()) + + +def clear_registry() -> None: + """Reset the registry — for tests.""" + _composers.clear() + + +# --------------------------------------------------------------------------- +# Built-in composers +# --------------------------------------------------------------------------- + +async def compose_bedrock_project(conn, entity_id: str) -> Optional[SearchDocRow]: + row = await conn.fetchrow( + """ + SELECT p.id::text AS id, p.name, p.description, p.created_by, + p.created_at, p.updated_at, + COALESCE(p.deleted_at IS NOT NULL, FALSE) AS is_deleted + FROM bedrock.project p + WHERE p.id = $1::uuid + """, + entity_id, + ) + if not row or row["is_deleted"]: + return None + + description = (row["description"] or "").strip() + return SearchDocRow( + entity_type="bedrock_project", + entity_id=row["id"], + title=row["name"] or "Untitled project", + subtitle="Project" + (f" · {description[:80]}" if description else ""), + href=f"/projects/{row['id']}", + search_text=" ".join(filter(None, [row["name"], description])), + owner_email=row["created_by"], + visibility="org", + activity_at=row["updated_at"], + source_version=row["updated_at"], + ) + + +async def compose_bedrock_saved_view(conn, entity_id: str) -> Optional[SearchDocRow]: + row = await conn.fetchrow( + """ + SELECT id::text AS id, scope_key, name, owner_email, is_global, + created_at, updated_at + FROM bedrock.saved_view + WHERE id = $1::uuid + """, + entity_id, + ) + if not row: + return None + + visibility = "org" if row["is_global"] else "private" + return SearchDocRow( + entity_type="bedrock_saved_view", + entity_id=row["id"], + title=row["name"] or "Untitled view", + subtitle=f"Saved view · {row['scope_key']}", + href=f"/{row['scope_key']}?view={row['id']}", + search_text=" ".join(filter(None, [row["name"], row["scope_key"]])), + owner_email=row["owner_email"] if not row["is_global"] else None, + visibility=visibility, + activity_at=row["updated_at"], + source_version=row["updated_at"], + ) + + +async def compose_pebble_profile(conn, entity_id: str) -> Optional[SearchDocRow]: + """Index Pebble research from ``pebble_research_sessions`` (one row + per completed research run). Keyed on session id, not contact_id, + because the session row carries name/org as explicit columns whereas + pebble_profiles stores them inside profile_json. + """ + row = await conn.fetchrow( + """ + SELECT id::text AS id, contact_id, prospect_name, prospect_org, + tier, status, batch_id, profile_json, created_at + FROM bedrock.pebble_research_sessions + WHERE id = $1::uuid + """, + entity_id, + ) + if not row or row["status"] != "completed": + return None + + name = (row["prospect_name"] or "").strip() or "Researched prospect" + org = (row["prospect_org"] or "").strip() + + summary = "" + if row["profile_json"]: + try: + data = json.loads(row["profile_json"]) + summary = (data.get("summary") or "")[:500] + except (ValueError, TypeError): + summary = "" + + subtitle = " · ".join(filter(None, [ + org or None, + f"Tier {row['tier']}" if row["tier"] else None, + ])) + if not subtitle: + subtitle = "Researched prospect" + + return SearchDocRow( + entity_type="pebble_profile", + entity_id=row["id"], + title=name, + subtitle=subtitle, + href=f"/pebble/profiles/{row['id']}", + search_text=" ".join(filter(None, [name, org, summary])), + visibility="org", + activity_at=row["created_at"], + source_version=row["created_at"], + ) + + +# Register the built-ins on import. +register_composer("bedrock_project", compose_bedrock_project) +register_composer("bedrock_saved_view", compose_bedrock_saved_view) +register_composer("pebble_profile", compose_pebble_profile) + + +# --------------------------------------------------------------------------- +# Drain +# --------------------------------------------------------------------------- + +@dataclass +class DrainStats: + rows_processed: int = 0 + rows_upserted: int = 0 + rows_deleted: int = 0 + rows_errored: int = 0 + rows_no_composer: int = 0 + elapsed_ms: int = 0 + last_errors: list[str] = field(default_factory=list) + + +async def drain_once(pool, *, batch_size: int = DEFAULT_BATCH_SIZE) -> DrainStats: + """Drain one batch of queue rows. Returns stats. Single transaction + per row so partial failures don't block the queue head. + """ + started = time.perf_counter() + stats = DrainStats() + + async with pool.acquire() as conn: + # Claim a batch with row-level locking. SKIP LOCKED so multiple + # workers can run side-by-side without contending. + rows = await conn.fetch( + """ + SELECT id, entity_type, entity_id, op, attempt_count + FROM bedrock.search_index_queue + WHERE attempt_count < $1 + ORDER BY enqueued_at ASC + FOR UPDATE SKIP LOCKED + LIMIT $2 + """, + MAX_ATTEMPT_COUNT, batch_size, + ) + + for row in rows: + stats.rows_processed += 1 + try: + if row["op"] == "delete": + await _apply_delete(conn, row["entity_type"], row["entity_id"]) + stats.rows_deleted += 1 + else: + composer = get_composer(row["entity_type"]) + if not composer: + stats.rows_no_composer += 1 + # Silently leave in queue — a future worker with + # the composer registered will process it. No + # attempt_count bump; this is "unsupported", not + # "failed." + continue + + doc = await composer(conn, row["entity_id"]) + if doc is None: + # Source deleted / non-indexable — soft-delete in + # search_doc. + await _apply_delete(conn, row["entity_type"], row["entity_id"]) + stats.rows_deleted += 1 + else: + await _apply_upsert(conn, doc) + stats.rows_upserted += 1 + + # Successful: remove queue row. + await conn.execute( + "DELETE FROM bedrock.search_index_queue WHERE id = $1", + row["id"], + ) + except Exception as e: + stats.rows_errored += 1 + stats.last_errors.append(f"{row['entity_type']}:{row['entity_id']}: {e}") + # Bump attempt count so eventual hard-fail removes it + # from the hot path. + await conn.execute( + """ + UPDATE bedrock.search_index_queue + SET attempt_count = attempt_count + 1, + last_error = $1 + WHERE id = $2 + """, + str(e)[:500], row["id"], + ) + + stats.elapsed_ms = int((time.perf_counter() - started) * 1000) + return stats + + +async def _apply_upsert(conn, doc: SearchDocRow) -> None: + """UPSERT into search_doc. The compose-vector trigger handles + search_vector synthesis; we only feed the column inputs.""" + await conn.execute( + """ + INSERT INTO bedrock.search_doc ( + entity_type, entity_id, title, subtitle, href, + search_text, search_vector, + owner_sf_id, owner_email, account_sf_id, visibility, + activity_at, source_version, org_id + ) VALUES ( + $1, $2, $3, $4, $5, + $6, ''::tsvector, + $7, $8, $9, $10, + $11, $12, $13 + ) + ON CONFLICT (entity_type, entity_id) DO UPDATE + SET title = EXCLUDED.title, + subtitle = EXCLUDED.subtitle, + href = EXCLUDED.href, + search_text = EXCLUDED.search_text, + owner_sf_id = EXCLUDED.owner_sf_id, + owner_email = EXCLUDED.owner_email, + account_sf_id = EXCLUDED.account_sf_id, + visibility = EXCLUDED.visibility, + activity_at = EXCLUDED.activity_at, + source_version = EXCLUDED.source_version, + indexed_at = now(), + deleted_at = NULL + """, + doc.entity_type, doc.entity_id, doc.title, doc.subtitle, doc.href, + doc.search_text, + doc.owner_sf_id, doc.owner_email, doc.account_sf_id, doc.visibility, + doc.activity_at, doc.source_version, doc.org_id, + ) + + +async def _apply_delete(conn, entity_type: str, entity_id: str) -> None: + """Soft-delete in search_doc. Tombstone partial-index excludes from + GIN scans automatically.""" + await conn.execute( + """ + UPDATE bedrock.search_doc + SET deleted_at = now(), indexed_at = now() + WHERE entity_type = $1 AND entity_id = $2 AND deleted_at IS NULL + """, + entity_type, entity_id, + ) + + +# --------------------------------------------------------------------------- +# Long-running worker +# --------------------------------------------------------------------------- + +async def run_worker( + pool, + stop_event: asyncio.Event, + *, + poll_interval: float = POLL_INTERVAL_SECONDS, + batch_size: int = DEFAULT_BATCH_SIZE, +) -> None: + """Long-running drain. Cancels cleanly on stop_event.set(). + + Belt-and-suspenders: combines LISTEN/NOTIFY (lossy by spec — see + Postgres docs) with periodic polling. NOTIFY wakes us promptly; + polling catches anything we miss. + + This function does not crash the calling process on errors — every + drain cycle is wrapped, and consecutive failures back off + exponentially. + """ + backoff = BACKOFF_BASE_SECONDS + + async with pool.acquire() as listen_conn: + try: + await listen_conn.add_listener( + "bedrock_search_index_queue", + lambda *_args: None, # we just need the wakeup signal + ) + except Exception: + logger.exception("Failed to attach LISTEN; will rely on polling only") + + while not stop_event.is_set(): + try: + stats = await drain_once(pool, batch_size=batch_size) + if stats.rows_errored: + logger.warning( + "search_indexer_drain rows=%d errored=%d (%s)", + stats.rows_processed, stats.rows_errored, + "; ".join(stats.last_errors[:3]), + ) + elif stats.rows_processed: + logger.info( + "search_indexer_drain processed=%d upserted=%d deleted=%d in %dms", + stats.rows_processed, stats.rows_upserted, + stats.rows_deleted, stats.elapsed_ms, + ) + # Reset backoff on successful cycle. + backoff = BACKOFF_BASE_SECONDS + except asyncio.CancelledError: + raise + except Exception: + logger.exception("search_indexer_drain failed") + backoff = min(backoff * 2, BACKOFF_MAX_SECONDS) + + try: + await asyncio.wait_for(stop_event.wait(), timeout=poll_interval if backoff == BACKOFF_BASE_SECONDS else backoff) + except asyncio.TimeoutError: + pass + + +# --------------------------------------------------------------------------- +# Backfill +# --------------------------------------------------------------------------- + +async def backfill( + pool, + entity_type: str, + *, + batch_size: int = 500, + progress_cb: Optional[Callable[[int, int], None]] = None, +) -> int: + """Walk every source row of an entity_type and enqueue it. The + drain worker picks up the queue rows and indexes them. Returns the + enqueued count. + + ``progress_cb(processed, total)`` is called every batch when + provided. + """ + composer = get_composer(entity_type) + if not composer: + raise ValueError(f"No composer registered for entity_type={entity_type!r}") + + # Per-entity-type id-source SQL. Each composer has a default + # source table; we centralize the id-walk SQL here so the composer + # contract stays "given an id, give me a SearchDocRow". + id_source_sql = _BACKFILL_ID_SOURCE_SQL.get(entity_type) + if not id_source_sql: + raise ValueError( + f"No backfill id-source SQL registered for entity_type={entity_type!r}" + ) + + enqueued = 0 + async with pool.acquire() as conn: + total = await conn.fetchval(_BACKFILL_COUNT_SQL[entity_type]) + offset = 0 + while True: + rows = await conn.fetch(id_source_sql, batch_size, offset) + if not rows: + break + await conn.executemany( + """ + INSERT INTO bedrock.search_index_queue (entity_type, entity_id, op) + VALUES ($1, $2, 'upsert') + ON CONFLICT (entity_type, entity_id, op) DO UPDATE + SET enqueued_at = now() + """, + [(entity_type, str(r["id"])) for r in rows], + ) + enqueued += len(rows) + offset += batch_size + if progress_cb: + progress_cb(enqueued, total) + return enqueued + + +# Per-entity-type backfill SQL. Centralized here so composers stay tiny. +_BACKFILL_ID_SOURCE_SQL: dict[str, str] = { + "bedrock_project": """ + SELECT id FROM bedrock.project + WHERE deleted_at IS NULL + ORDER BY created_at ASC + LIMIT $1 OFFSET $2 + """, + "bedrock_saved_view": """ + SELECT id FROM bedrock.saved_view + ORDER BY created_at ASC + LIMIT $1 OFFSET $2 + """, + "pebble_profile": """ + SELECT id FROM bedrock.pebble_research_sessions + WHERE status = 'completed' + ORDER BY created_at ASC + LIMIT $1 OFFSET $2 + """, +} + +_BACKFILL_COUNT_SQL: dict[str, str] = { + "bedrock_project": "SELECT COUNT(*) FROM bedrock.project WHERE deleted_at IS NULL", + "bedrock_saved_view": "SELECT COUNT(*) FROM bedrock.saved_view", + "pebble_profile": "SELECT COUNT(*) FROM bedrock.pebble_research_sessions WHERE status = 'completed'", +} diff --git a/financial_forecasting/tests/test_search_indexer.py b/financial_forecasting/tests/test_search_indexer.py new file mode 100644 index 00000000..17667e73 --- /dev/null +++ b/financial_forecasting/tests/test_search_indexer.py @@ -0,0 +1,340 @@ +"""Tests for ``services/search_indexer.py`` — Layer 1.6. + +Locks in: + +A. Composer registry: register / get / clear behaviors. +B. Built-in composers produce expected SearchDocRow shapes. +C. Built-in composers return None on missing / deleted / wrong-status rows. +D. drain_once happy path: upsert + dequeue. +E. drain_once handles 'delete' op via soft-delete. +F. drain_once with no composer leaves the queue row in place (no attempt bump). +G. drain_once on composer error bumps attempt_count + records last_error. +H. drain_once skips rows past MAX_ATTEMPT_COUNT. +I. backfill enqueues all rows from the id-source. +J. backfill rejects unregistered entity_type. +""" + +import os +import sys +import json +from contextlib import asynccontextmanager +from datetime import datetime, timezone +from unittest.mock import AsyncMock, MagicMock + +import pytest + +sys.path.insert(0, os.path.join(os.path.dirname(__file__), '..')) + +from services import search_indexer as si + + +# --------------------------------------------------------------------------- +# Fixtures +# --------------------------------------------------------------------------- + +@pytest.fixture +def fresh_registry(): + """Snapshot+restore the composer registry so tests don't leak.""" + snapshot = dict(si._composers) + yield + si.clear_registry() + si._composers.update(snapshot) + + +@pytest.fixture +def mock_pool_with_conn(): + """Pool whose acquire() yields a single configured AsyncMock conn.""" + conn = AsyncMock() + conn.fetchrow = AsyncMock() + conn.fetch = AsyncMock(return_value=[]) + conn.execute = AsyncMock(return_value="UPDATE 1") + conn.fetchval = AsyncMock(return_value=0) + conn.executemany = AsyncMock() + + @asynccontextmanager + async def _acquire(): + yield conn + + pool = MagicMock() + pool.acquire = lambda: _acquire() + return pool, conn + + +# --------------------------------------------------------------------------- +# A. Registry +# --------------------------------------------------------------------------- + +def test_default_registry_has_built_in_composers(): + assert "bedrock_project" in si.registered_entity_types() + assert "bedrock_saved_view" in si.registered_entity_types() + assert "pebble_profile" in si.registered_entity_types() + + +def test_register_overwrites(fresh_registry): + async def custom(conn, eid): + return None + si.register_composer("bedrock_project", custom) + assert si.get_composer("bedrock_project") is custom + + +def test_get_composer_unknown_returns_none(): + assert si.get_composer("does_not_exist") is None + + +def test_clear_registry_empties_it(fresh_registry): + si.clear_registry() + assert si.registered_entity_types() == () + + +# --------------------------------------------------------------------------- +# B + C. Built-in composers +# --------------------------------------------------------------------------- + +@pytest.mark.asyncio +async def test_compose_bedrock_project_happy_path(): + conn = AsyncMock() + conn.fetchrow = AsyncMock(return_value={ + "id": "abc-123", + "name": "Q4 Capacity Build", + "description": "Spin up training cohort.", + "created_by": "rm@pursuit.org", + "created_at": datetime(2026, 5, 1, tzinfo=timezone.utc), + "updated_at": datetime(2026, 5, 6, tzinfo=timezone.utc), + "is_deleted": False, + }) + doc = await si.compose_bedrock_project(conn, "abc-123") + assert doc is not None + assert doc.entity_type == "bedrock_project" + assert doc.entity_id == "abc-123" + assert doc.title == "Q4 Capacity Build" + assert "Spin up training cohort" in doc.subtitle + assert doc.href == "/projects/abc-123" + assert doc.owner_email == "rm@pursuit.org" + assert doc.visibility == "org" + assert "Q4 Capacity Build" in doc.search_text + + +@pytest.mark.asyncio +async def test_compose_bedrock_project_deleted_returns_none(): + conn = AsyncMock() + conn.fetchrow = AsyncMock(return_value={ + "id": "abc-123", "name": "X", "description": "", "created_by": None, + "created_at": datetime.now(timezone.utc), + "updated_at": datetime.now(timezone.utc), + "is_deleted": True, + }) + assert await si.compose_bedrock_project(conn, "abc-123") is None + + +@pytest.mark.asyncio +async def test_compose_bedrock_project_missing_returns_none(): + conn = AsyncMock() + conn.fetchrow = AsyncMock(return_value=None) + assert await si.compose_bedrock_project(conn, "abc-123") is None + + +@pytest.mark.asyncio +async def test_compose_bedrock_saved_view_personal_is_private(): + conn = AsyncMock() + conn.fetchrow = AsyncMock(return_value={ + "id": "v1", "scope_key": "pipeline", "name": "My Open Deals", + "owner_email": "rm@pursuit.org", "is_global": False, + "created_at": datetime.now(timezone.utc), + "updated_at": datetime.now(timezone.utc), + }) + doc = await si.compose_bedrock_saved_view(conn, "v1") + assert doc.visibility == "private" + assert doc.owner_email == "rm@pursuit.org" + assert doc.title == "My Open Deals" + assert doc.href == "/pipeline?view=v1" + + +@pytest.mark.asyncio +async def test_compose_bedrock_saved_view_global_clears_owner(): + conn = AsyncMock() + conn.fetchrow = AsyncMock(return_value={ + "id": "v2", "scope_key": "accounts", "name": "Top Tier", + "owner_email": "admin@pursuit.org", "is_global": True, + "created_at": datetime.now(timezone.utc), + "updated_at": datetime.now(timezone.utc), + }) + doc = await si.compose_bedrock_saved_view(conn, "v2") + assert doc.visibility == "org" + assert doc.owner_email is None + + +@pytest.mark.asyncio +async def test_compose_pebble_profile_happy_path(): + profile = {"summary": "Major donor in workforce dev. 3 prior gifts."} + conn = AsyncMock() + conn.fetchrow = AsyncMock(return_value={ + "id": "sess-1", "contact_id": "003ABC", + "prospect_name": "Jane Donor", "prospect_org": "MetLife Foundation", + "tier": "T2", "status": "completed", "batch_id": None, + "profile_json": json.dumps(profile), + "created_at": datetime(2026, 5, 6, tzinfo=timezone.utc), + }) + doc = await si.compose_pebble_profile(conn, "sess-1") + assert doc.title == "Jane Donor" + assert "MetLife Foundation" in doc.subtitle + assert "Tier T2" in doc.subtitle + assert doc.href == "/pebble/profiles/sess-1" + assert "Jane Donor" in doc.search_text + assert "MetLife Foundation" in doc.search_text + assert "Major donor" in doc.search_text + + +@pytest.mark.asyncio +async def test_compose_pebble_profile_in_progress_returns_none(): + conn = AsyncMock() + conn.fetchrow = AsyncMock(return_value={ + "id": "sess-2", "contact_id": "003ABC", + "prospect_name": "X", "prospect_org": "Y", + "tier": "T1", "status": "in_progress", # not 'completed' + "batch_id": None, "profile_json": None, + "created_at": datetime.now(timezone.utc), + }) + assert await si.compose_pebble_profile(conn, "sess-2") is None + + +@pytest.mark.asyncio +async def test_compose_pebble_profile_handles_invalid_json(): + """Bad JSON in profile_json must NOT crash the composer.""" + conn = AsyncMock() + conn.fetchrow = AsyncMock(return_value={ + "id": "sess-3", "contact_id": "003ABC", + "prospect_name": "Acme", "prospect_org": None, + "tier": None, "status": "completed", "batch_id": None, + "profile_json": "{not valid json", + "created_at": datetime.now(timezone.utc), + }) + doc = await si.compose_pebble_profile(conn, "sess-3") + assert doc is not None + assert doc.title == "Acme" + + +# --------------------------------------------------------------------------- +# D + E + F + G + H. drain_once +# --------------------------------------------------------------------------- + +@pytest.mark.asyncio +async def test_drain_once_upserts_and_dequeues(mock_pool_with_conn, fresh_registry): + pool, conn = mock_pool_with_conn + + async def fake_composer(c, eid): + return si.SearchDocRow( + entity_type="bedrock_test", entity_id=eid, + title=f"Title {eid}", subtitle=None, href=f"/test/{eid}", + search_text=f"text {eid}", + ) + si.register_composer("bedrock_test", fake_composer) + + conn.fetch.return_value = [ + {"id": 1, "entity_type": "bedrock_test", "entity_id": "e1", "op": "upsert", "attempt_count": 0}, + ] + + stats = await si.drain_once(pool, batch_size=10) + + assert stats.rows_processed == 1 + assert stats.rows_upserted == 1 + assert stats.rows_errored == 0 + + # An UPSERT and a DELETE FROM queue both happened. + sqls = [call.args[0] for call in conn.execute.call_args_list] + assert any("INSERT INTO bedrock.search_doc" in s for s in sqls) + assert any("DELETE FROM bedrock.search_index_queue" in s for s in sqls) + + +@pytest.mark.asyncio +async def test_drain_once_handles_delete_op(mock_pool_with_conn): + pool, conn = mock_pool_with_conn + conn.fetch.return_value = [ + {"id": 7, "entity_type": "bedrock_project", "entity_id": "abc", + "op": "delete", "attempt_count": 0}, + ] + stats = await si.drain_once(pool) + assert stats.rows_deleted == 1 + sqls = [call.args[0] for call in conn.execute.call_args_list] + assert any("UPDATE bedrock.search_doc" in s and "deleted_at = now()" in s for s in sqls) + + +@pytest.mark.asyncio +async def test_drain_once_no_composer_leaves_queue_intact(mock_pool_with_conn, fresh_registry): + """Queue row for an unregistered entity_type stays in the queue + without a failure bump. Future workers with the composer will + process it.""" + pool, conn = mock_pool_with_conn + si.clear_registry() # remove built-ins for this test + + conn.fetch.return_value = [ + {"id": 1, "entity_type": "bedrock_unknown", "entity_id": "x", + "op": "upsert", "attempt_count": 0}, + ] + + stats = await si.drain_once(pool) + assert stats.rows_no_composer == 1 + # No DELETE on queue, no UPDATE attempt_count. + sqls = [call.args[0] for call in conn.execute.call_args_list] + assert not any("DELETE FROM bedrock.search_index_queue" in s for s in sqls) + assert not any("UPDATE bedrock.search_index_queue" in s for s in sqls) + + +@pytest.mark.asyncio +async def test_drain_once_composer_error_bumps_attempt_count(mock_pool_with_conn, fresh_registry): + pool, conn = mock_pool_with_conn + + async def boom(c, eid): + raise RuntimeError("intentional boom") + si.register_composer("bedrock_boom", boom) + + conn.fetch.return_value = [ + {"id": 5, "entity_type": "bedrock_boom", "entity_id": "x", + "op": "upsert", "attempt_count": 1}, + ] + stats = await si.drain_once(pool) + assert stats.rows_errored == 1 + sqls = [call.args[0] for call in conn.execute.call_args_list] + assert any("UPDATE bedrock.search_index_queue" in s and "attempt_count" in s for s in sqls) + assert any("intentional boom" in str(call.args) for call in conn.execute.call_args_list) + + +@pytest.mark.asyncio +async def test_drain_once_excludes_rows_past_max_attempt(mock_pool_with_conn): + pool, conn = mock_pool_with_conn + await si.drain_once(pool) + # The SELECT MUST contain attempt_count < $1 with $1 = MAX_ATTEMPT_COUNT. + sent_sql = conn.fetch.call_args.args[0] + assert "attempt_count < $1" in sent_sql + assert si.MAX_ATTEMPT_COUNT in conn.fetch.call_args.args + + +# --------------------------------------------------------------------------- +# I + J. backfill +# --------------------------------------------------------------------------- + +@pytest.mark.asyncio +async def test_backfill_enqueues_all_source_rows(mock_pool_with_conn): + pool, conn = mock_pool_with_conn + + # 3 source rows total (one batch). + conn.fetchval.return_value = 3 + conn.fetch.side_effect = [ + [{"id": "p1"}, {"id": "p2"}, {"id": "p3"}], + [], + ] + + progress_calls: list[tuple[int, int]] = [] + enqueued = await si.backfill( + pool, "bedrock_project", batch_size=500, + progress_cb=lambda d, t: progress_calls.append((d, t)), + ) + assert enqueued == 3 + assert progress_calls == [(3, 3)] + conn.executemany.assert_awaited_once() + + +@pytest.mark.asyncio +async def test_backfill_rejects_unregistered_entity_type(mock_pool_with_conn): + pool, _ = mock_pool_with_conn + with pytest.raises(ValueError, match="No composer registered"): + await si.backfill(pool, "totally_unregistered") From 94d86dc7c45aa8f9161ba4f28412eae0c124df2e Mon Sep 17 00:00:00 2001 From: JP Date: Wed, 6 May 2026 19:39:53 -0400 Subject: [PATCH 10/35] feat(search): mount /api/search router + start indexer in lifespan MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Wires the search read path and indexer worker into the production FastAPI process. main.py changes: * Import routes/search.py and mount /api/search + /api/search/click endpoints. * In startup_event: read the asyncpg pool, create a stop_event, kick off run_search_indexer_worker as a long-lived asyncio task. Stop_event + task stored on _services dict so shutdown can stop them cleanly. * In shutdown_event: set stop_event, then asyncio.wait_for the task with a 5-second drain timeout. Cancel-hard if it doesn't shut down — production restarts shouldn't be blocked by a stuck indexer. * Indexer failure is non-critical at startup: search remains available (existing search_doc rows still queryable), it just won't reflect new writes until restart. Logged loudly via logger.exception. main.py smoke-tested: imports clean, /api/search and /api/search/click both registered. Plan refs: tasks/pebble-search-spec.md Layer 1.6, 1.8 Co-Authored-By: Claude Opus 4.7 (1M context) --- financial_forecasting/main.py | 41 +++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/financial_forecasting/main.py b/financial_forecasting/main.py index 32054a2e..d7b041d3 100644 --- a/financial_forecasting/main.py +++ b/financial_forecasting/main.py @@ -58,6 +58,8 @@ from routes.platform_intake import router as platform_intake_router from routes.awards import router as awards_router from routes.saved_views import router as saved_views_router +from routes.search import router as search_router +from services.search_indexer import run_worker as run_search_indexer_worker from auth import get_current_user_dep, require_auth, IS_PRODUCTION, JWT_SECRET_KEY from security import validate_salesforce_id, escape_soql_string from services.crm_parser import refresh_opp_cache as _refresh_opp_cache @@ -137,6 +139,7 @@ app.include_router(platform_intake_router) app.include_router(awards_router) app.include_router(saved_views_router) +app.include_router(search_router) # Service singletons — shared with dependencies.py so route files can use # Depends(require_sf_mcp_client) without circular imports. @@ -192,6 +195,27 @@ async def startup_event(): _services["data_sync_service"] = DataSyncService(client) asyncio.create_task(background_sync_task()) + # Search indexer worker — drains bedrock.search_index_queue and + # upserts bedrock.search_doc rows. Lives on the Bedrock process + # alongside the other lifecycle services so search reflects writes + # within seconds rather than waiting for a separate worker pod. + try: + from db import get_pool + pool = get_pool() + if pool is not None: + stop_event = asyncio.Event() + _services["search_indexer_stop"] = stop_event + _services["search_indexer_task"] = asyncio.create_task( + run_search_indexer_worker(pool, stop_event), + ) + logger.info("Search indexer worker started") + else: + logger.warning("Search indexer not started: db pool unavailable") + except Exception: + # Indexer is non-critical at startup — search remains + # available, it just won't reflect new writes until restart. + logger.exception("Failed to start search indexer worker") + logger.info(f"API started — connected services: {client.connected_services or ['none']}") @@ -199,6 +223,23 @@ async def startup_event(): async def shutdown_event(): """Cleanup on shutdown.""" logger.info("Shutting down Financial Forecasting API...") + + # Stop the search indexer cleanly — give it 5s to drain in-flight + # rows; cancel hard if it doesn't. + stop_event = _services.get("search_indexer_stop") + task = _services.get("search_indexer_task") + if stop_event is not None: + stop_event.set() + if task is not None: + try: + await asyncio.wait_for(task, timeout=5.0) + except asyncio.TimeoutError: + task.cancel() + try: + await task + except (asyncio.CancelledError, Exception): + pass + await close_db() client = _services.get("mcp_client") if client: From bc50b8c11b616066c95e5e5d57a9fb5387d57922 Mon Sep 17 00:00:00 2001 From: JP Date: Wed, 6 May 2026 19:42:52 -0400 Subject: [PATCH 11/35] =?UTF-8?q?feat(pebble):=20/api/pebble/ask=20proxy?= =?UTF-8?q?=20=E2=80=94=20Bedrock=20gateway=20to=20Pebble=20chat?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Layer 3.1 of the Pebble 1.0 plan. Forwards conversational queries from the frontend's Ask mode to Pebble (separate process, port 8001) and streams the response back as SSE. Why a Bedrock-side proxy instead of direct frontend → Pebble: * Single auth boundary. Frontend has Bedrock JWT; doesn't have Pebble's X-Api-Key. Proxy swaps creds. * Centralized cost / rate gating. One place to enforce per-user budget + audit row. * Single CORS surface — FE talks to one host. * Trace propagation gateway: X-Trace-Id minted at ingress, forwarded to Pebble + back to FE so FE → Bedrock → Pebble → Bedrock(/api/search) shares one correlation id. routes/pebble_proxy.py: * POST /api/pebble/ask {query, conversation_id?, context?} streams text/event-stream to FE. Read-only at v1.0 — no write tools in Pebble's tool budget. Suggested-action cards require user confirmation via separate JWT-authenticated routes (/api/opportunities/update-stage, etc). * Permission gate: check_permission_or_internal("use_pebble_chat"). * Service callers (Pebble calling itself in nested flows) propagate originating_user_email as Pebble's X-User-Email so Pebble grounds in the human's permissions, not the service principal's. * Pebble status >= 400 → SSE error frame (type=error, status). * Timeout (60s read) → SSE error frame (reason=timeout, 504). * Other httpx errors → SSE error frame (reason=upstream, 502). * Audit row written via BackgroundTask to bedrock.search_audit with mode='ask' — same table as Find queries, single "what did Pebble see for this user" surface. * Singleton httpx.AsyncClient with 60s read timeout, 3s connect. close() called from main.py shutdown. 13 tests covering: happy path streaming, Pebble error surfacing, timeout / connection error fallback, query length validation (1-2000 chars), audit BackgroundTask invocation, trace_id propagation (kept when valid, minted when malformed), service-caller originating-user delegation to Pebble's X-User-Email, close() singleton reset. main.py wire-up: * Mount /api/pebble/ask router. * close_pebble_proxy() in shutdown_event before close_db(). Plan refs: tasks/pebble-search-spec.md Layer 3.1 tasks/pebble-search-spec-security.md §6 (degradation order) Co-Authored-By: Claude Opus 4.7 (1M context) --- financial_forecasting/main.py | 3 + financial_forecasting/routes/pebble_proxy.py | 267 +++++++++++++++ .../tests/test_pebble_proxy.py | 315 ++++++++++++++++++ 3 files changed, 585 insertions(+) create mode 100644 financial_forecasting/routes/pebble_proxy.py create mode 100644 financial_forecasting/tests/test_pebble_proxy.py diff --git a/financial_forecasting/main.py b/financial_forecasting/main.py index d7b041d3..d598cbd1 100644 --- a/financial_forecasting/main.py +++ b/financial_forecasting/main.py @@ -59,6 +59,7 @@ from routes.awards import router as awards_router from routes.saved_views import router as saved_views_router from routes.search import router as search_router +from routes.pebble_proxy import router as pebble_proxy_router, close as close_pebble_proxy from services.search_indexer import run_worker as run_search_indexer_worker from auth import get_current_user_dep, require_auth, IS_PRODUCTION, JWT_SECRET_KEY from security import validate_salesforce_id, escape_soql_string @@ -140,6 +141,7 @@ app.include_router(awards_router) app.include_router(saved_views_router) app.include_router(search_router) +app.include_router(pebble_proxy_router) # Service singletons — shared with dependencies.py so route files can use # Depends(require_sf_mcp_client) without circular imports. @@ -240,6 +242,7 @@ async def shutdown_event(): except (asyncio.CancelledError, Exception): pass + await close_pebble_proxy() await close_db() client = _services.get("mcp_client") if client: diff --git a/financial_forecasting/routes/pebble_proxy.py b/financial_forecasting/routes/pebble_proxy.py new file mode 100644 index 00000000..d1d5978a --- /dev/null +++ b/financial_forecasting/routes/pebble_proxy.py @@ -0,0 +1,267 @@ +"""Pebble proxy — Layer 3.1 of the Pebble 1.0 plan. + +The Bedrock-side gateway for the frontend's Ask mode. Forwards +conversational queries to Pebble (running on a separate process, +port 8001) and streams the response back. Pebble itself can call +back into Bedrock's /api/search via the existing crm_bridge for +grounding tool calls. + +Why a Bedrock-side proxy and not a direct frontend → Pebble call: + * Single auth boundary. The frontend already has a JWT cookie + for Bedrock; it doesn't have credentials for Pebble. The proxy + swaps those for Pebble's X-Api-Key + X-User-Email. + * Centralized cost / rate gating. Audit row, daily cost cap, + per-user rate limit all live here. + * Single CORS surface. Frontend talks to one host (Bedrock), + not two. + * Trace propagation gateway — proxy adds X-Trace-Id if absent + so the FE → Bedrock → Pebble → Bedrock(/api/search) chain + has a single correlation id. + +Endpoint: + POST /api/pebble/ask body: {query, conversation_id?, context?} + stream: text/event-stream + +Permission gate: ``check_permission_or_internal("use_pebble_chat")``. +""" + +from __future__ import annotations + +import asyncio +import logging +import os +import time +import uuid +from typing import Any, AsyncIterator, Optional + +import httpx +from fastapi import APIRouter, BackgroundTasks, Depends, HTTPException, Request +from fastapi.responses import StreamingResponse +from pydantic import BaseModel, Field + +from db import get_db +from routes.permissions import check_permission_or_internal + +logger = logging.getLogger(__name__) +router = APIRouter(prefix="/api/pebble", tags=["pebble"]) + +# Module-level stable reference so tests override correctly. +require_ask_perm = check_permission_or_internal("use_pebble_chat") + + +# Pebble's URL + auth. Default localhost for dev; production MUST +# override via env (validated at Pebble's own startup, see +# pebble/main.py:_validate_bedrock_bridge_config for the inverse +# direction). +_PEBBLE_URL = os.getenv("PEBBLE_API_URL", "http://localhost:8001") +_PEBBLE_API_KEY = os.getenv("PEBBLE_API_KEY", "") + +# Streaming timeouts — generous on the read side because L1+ Pebble +# responses can take up to ~30s; tight on connect. +_HTTP_TIMEOUT = httpx.Timeout(60.0, connect=3.0, read=60.0) + + +# --------------------------------------------------------------------------- +# Request / response +# --------------------------------------------------------------------------- + +class AskRequest(BaseModel): + query: str = Field(..., min_length=1, max_length=2000) + conversation_id: Optional[str] = Field( + None, description="UUID for multi-turn conversations; minted if absent", + ) + context: dict[str, Any] = Field( + default_factory=dict, + description="Optional context — current page / record / etc. for grounding", + ) + + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + +_client: Optional[httpx.AsyncClient] = None + + +def _get_pebble_client() -> httpx.AsyncClient: + """Lazy-init singleton client. Closed on shutdown via close().""" + global _client + if _client is None or _client.is_closed: + headers: dict[str, str] = {} + if _PEBBLE_API_KEY: + headers["X-Api-Key"] = _PEBBLE_API_KEY + _client = httpx.AsyncClient( + base_url=_PEBBLE_URL, + headers=headers, + timeout=_HTTP_TIMEOUT, + ) + return _client + + +async def close() -> None: + """Close the proxy's httpx client. Call from app lifespan shutdown.""" + global _client + if _client and not _client.is_closed: + await _client.aclose() + _client = None + + +def _new_trace_id(request: Request) -> str: + """Use the inbound X-Trace-Id if valid, else mint.""" + raw = request.headers.get("X-Trace-Id", "").strip() + if raw: + try: + uuid.UUID(raw) + return raw + except ValueError: + pass + return str(uuid.uuid4()) + + +async def _emit_ask_audit( + pool, + *, + request_id: str, + trace_id: str, + user_email: str, + originating_user_email: Optional[str], + org_id: str, + query: str, + response_status: int, + latency_ms: int, + error_class: Optional[str] = None, +) -> None: + """Record the Ask query in bedrock.search_audit with mode='ask'. + Same audit table as Find queries — one place to look for "what did + Pebble see for this user." + """ + try: + from services import search_service as ss + async with pool.acquire() as conn: + await conn.execute( + """ + INSERT INTO bedrock.search_audit ( + query_id, request_id, user_email, + originating_user_email, org_id, mode, + query_text, query_text_hash, types_requested, + backend_used, latency_ms, result_count, + response_status, error_class + ) VALUES ( + $1, $2, $3, $4, $5, 'ask', + $6, $7, '{}', + 'pgvector', $8, 0, $9, $10 + ) + ON CONFLICT (request_id) DO NOTHING + """, + uuid.uuid4(), uuid.UUID(request_id), + user_email, originating_user_email, org_id, + query[:256], ss.query_text_hash(query), + latency_ms, response_status, error_class, + ) + except Exception: + logger.exception( + "ask_audit_insert_failed request_id=%s trace_id=%s", + request_id, trace_id, + ) + + +# --------------------------------------------------------------------------- +# POST /api/pebble/ask +# --------------------------------------------------------------------------- + +@router.post("/ask") +async def ask_endpoint( + request: Request, + background_tasks: BackgroundTasks, + body: AskRequest, + pool=Depends(get_db), + user=Depends(require_ask_perm), +): + """Stream a Pebble L0/L1+ chat response back to the frontend. + + Read-only at v1.0 — Pebble's tool budget allows search_crm but + NOT write tools. Suggested-action cards in the response require + user confirmation via a separate /api/opportunities/update-stage + call carrying the user's JWT (not the internal key). + """ + started = time.perf_counter() + user_email = user.get("email", "unknown") + originating = user.get("originating_user_email") + request_id = request.headers.get("X-Request-Id", "").strip() or str(uuid.uuid4()) + trace_id = _new_trace_id(request) + org_id = "pursuit" # multi-tenant outermost guard; refined post-1.0 + + # The user we send to Pebble is the originating user when caller + # is service:pebble (i.e. pebble@internal calling itself, which + # shouldn't happen but defend in depth), otherwise the bearer. + pebble_user_email = originating or user_email + + pebble_body = { + "query": body.query, + "conversation_id": body.conversation_id or str(uuid.uuid4()), + "context": body.context, + } + pebble_headers = { + "X-User-Email": pebble_user_email, + "X-Trace-Id": trace_id, + "X-Request-Id": request_id, + } + + response_status = 200 + error_class: Optional[str] = None + + async def _stream() -> AsyncIterator[bytes]: + nonlocal response_status, error_class + client = _get_pebble_client() + try: + async with client.stream( + "POST", "/api/v1/chat/query", + json=pebble_body, headers=pebble_headers, + ) as resp: + if resp.status_code >= 400: + response_status = resp.status_code + payload = await resp.aread() + error_class = f"pebble_status_{resp.status_code}" + yield ( + b'data: {"type":"error","status":' + + str(resp.status_code).encode() + + b'}\n\n' + ) + return + async for chunk in resp.aiter_bytes(): + if chunk: + yield chunk + except httpx.TimeoutException: + response_status = 504 + error_class = "TimeoutException" + yield b'data: {"type":"error","reason":"timeout"}\n\n' + except httpx.HTTPError as e: + response_status = 502 + error_class = type(e).__name__ + logger.exception("pebble_proxy_http_error trace_id=%s", trace_id) + yield b'data: {"type":"error","reason":"upstream"}\n\n' + finally: + latency_ms = int((time.perf_counter() - started) * 1000) + background_tasks.add_task( + _emit_ask_audit, + pool, + request_id=request_id, + trace_id=trace_id, + user_email=user_email, + originating_user_email=originating, + org_id=org_id, + query=body.query, + response_status=response_status, + latency_ms=latency_ms, + error_class=error_class, + ) + + return StreamingResponse( + _stream(), + media_type="text/event-stream", + headers={ + "X-Trace-Id": trace_id, + "Cache-Control": "no-cache", + "Connection": "keep-alive", + }, + ) diff --git a/financial_forecasting/tests/test_pebble_proxy.py b/financial_forecasting/tests/test_pebble_proxy.py new file mode 100644 index 00000000..90473ed0 --- /dev/null +++ b/financial_forecasting/tests/test_pebble_proxy.py @@ -0,0 +1,315 @@ +"""Tests for ``/api/pebble/ask`` proxy (Layer 3.1). + +Asserts: + A. Happy path: 200 + streamed content forwarded. + B. Pebble error status surfaces via SSE error frame. + C. Pebble timeout → 504-shaped error frame. + D. Pydantic validation rejects empty/oversized query. + E. Audit row written with mode='ask' via BackgroundTask. + F. Trace id propagated: header in / out. + G. Service caller's originating_user_email becomes Pebble's X-User-Email. + H. close() shuts the singleton client. +""" + +import os +import sys +from contextlib import asynccontextmanager +from unittest.mock import AsyncMock, MagicMock + +import pytest +from fastapi import FastAPI + +sys.path.insert(0, os.path.join(os.path.dirname(__file__), '..')) +sys.path.insert(0, os.path.dirname(__file__)) + +from fastapi.testclient import TestClient + +import httpx + +from db import get_db +from routes import pebble_proxy + + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + +class _FakePebbleStream: + """Mimics ``httpx.AsyncClient.stream`` async-context manager.""" + def __init__(self, status_code: int = 200, chunks: list[bytes] | None = None, + raise_exc: BaseException | None = None): + self.status_code = status_code + self._chunks = chunks or [b'data: {"type":"token","text":"hi"}\n\n'] + self._raise = raise_exc + + async def __aenter__(self): + if self._raise: + raise self._raise + return self + + async def __aexit__(self, *_a): + return None + + async def aiter_bytes(self): + for chunk in self._chunks: + yield chunk + + async def aread(self): + return b"".join(self._chunks) + + +class _FakeClient: + def __init__(self, stream_response: _FakePebbleStream): + self._stream = stream_response + self.is_closed = False + + def stream(self, method, url, **kwargs): + return self._stream + + async def aclose(self): + self.is_closed = True + + +@pytest.fixture +def app_factory(monkeypatch): + def _build( + *, + pebble_stream: _FakePebbleStream | None = None, + user: dict | None = None, + ) -> tuple[FastAPI, AsyncMock, _FakeClient]: + # Reset module-level client so we don't leak between tests. + pebble_proxy._client = None + + fake_client = _FakeClient(pebble_stream or _FakePebbleStream()) + + def _fake_get_client(): + pebble_proxy._client = fake_client + return fake_client + + monkeypatch.setattr(pebble_proxy, "_get_pebble_client", _fake_get_client) + + # Mock pool / conn for audit insert. + mock_conn = AsyncMock() + mock_conn.execute = AsyncMock(return_value="INSERT 0 1") + + @asynccontextmanager + async def _acquire(): + yield mock_conn + + mock_pool = MagicMock() + mock_pool.acquire = lambda: _acquire() + + async def _override_db(): + return mock_pool + + async def _override_perm(request=None): + return user or { + "user_id": "u-1", "email": "rm@pursuit.org", + "is_service": False, + } + + app = FastAPI() + app.include_router(pebble_proxy.router) + app.dependency_overrides[get_db] = _override_db + app.dependency_overrides[pebble_proxy.require_ask_perm] = _override_perm + + return app, mock_conn, fake_client + + return _build + + +# --------------------------------------------------------------------------- +# A. Happy path +# --------------------------------------------------------------------------- + +def test_happy_path_streams_response(app_factory): + stream = _FakePebbleStream( + status_code=200, + chunks=[ + b'data: {"type":"token","text":"hello"}\n\n', + b'data: {"type":"token","text":" world"}\n\n', + b'data: {"type":"done"}\n\n', + ], + ) + app, _, _ = app_factory(pebble_stream=stream) + with TestClient(app) as client: + with client.stream("POST", "/api/pebble/ask", json={"query": "find acme"}) as r: + body = b"".join(r.iter_bytes()) + assert b'hello' in body + assert b'done' in body + + +# --------------------------------------------------------------------------- +# B. Pebble error → SSE error frame +# --------------------------------------------------------------------------- + +def test_pebble_error_status_surfaces_as_sse_error(app_factory): + stream = _FakePebbleStream( + status_code=500, + chunks=[b'{"detail":"internal"}'], + ) + app, _, _ = app_factory(pebble_stream=stream) + with TestClient(app) as client: + with client.stream("POST", "/api/pebble/ask", json={"query": "x"}) as r: + body = b"".join(r.iter_bytes()) + assert b'"type":"error"' in body + assert b'"status":500' in body + + +# --------------------------------------------------------------------------- +# C. Timeout +# --------------------------------------------------------------------------- + +def test_timeout_returns_sse_error_frame(app_factory): + stream = _FakePebbleStream(raise_exc=httpx.TimeoutException("boom")) + app, _, _ = app_factory(pebble_stream=stream) + with TestClient(app) as client: + with client.stream("POST", "/api/pebble/ask", json={"query": "x"}) as r: + body = b"".join(r.iter_bytes()) + assert b'"type":"error"' in body + assert b'"reason":"timeout"' in body + + +def test_other_http_error_returns_upstream_error(app_factory): + stream = _FakePebbleStream(raise_exc=httpx.ConnectError("upstream down")) + app, _, _ = app_factory(pebble_stream=stream) + with TestClient(app) as client: + with client.stream("POST", "/api/pebble/ask", json={"query": "x"}) as r: + body = b"".join(r.iter_bytes()) + assert b'"type":"error"' in body + assert b'"reason":"upstream"' in body + + +# --------------------------------------------------------------------------- +# D. Pydantic validation +# --------------------------------------------------------------------------- + +def test_empty_query_returns_422(app_factory): + app, _, _ = app_factory() + with TestClient(app) as client: + r = client.post("/api/pebble/ask", json={"query": ""}) + assert r.status_code == 422 + + +def test_oversized_query_returns_422(app_factory): + app, _, _ = app_factory() + payload = {"query": "x" * 2001} + with TestClient(app) as client: + r = client.post("/api/pebble/ask", json=payload) + assert r.status_code == 422 + + +# --------------------------------------------------------------------------- +# E. Audit row written +# --------------------------------------------------------------------------- + +def test_audit_row_inserted_via_background_task(app_factory): + app, mock_conn, _ = app_factory() + with TestClient(app) as client: + with client.stream("POST", "/api/pebble/ask", json={"query": "find acme"}) as r: + for _ in r.iter_bytes(): + pass + # BackgroundTask runs after response in TestClient. + assert mock_conn.execute.await_count >= 1 + sent_sql = mock_conn.execute.call_args.args[0] + assert "INSERT INTO bedrock.search_audit" in sent_sql + assert "mode" in sent_sql.lower() + assert "ON CONFLICT" in sent_sql + + +# --------------------------------------------------------------------------- +# F. Trace id propagation +# --------------------------------------------------------------------------- + +def test_trace_id_in_response_header(app_factory): + app, _, _ = app_factory() + with TestClient(app) as client: + with client.stream( + "POST", "/api/pebble/ask", + json={"query": "x"}, + headers={"X-Trace-Id": "01993b8d-2c9a-7c4f-8b0e-000000000001"}, + ) as r: + assert r.headers.get("X-Trace-Id") == "01993b8d-2c9a-7c4f-8b0e-000000000001" + + +def test_invalid_trace_id_minted_fresh(app_factory): + """Malformed X-Trace-Id is replaced with a fresh UUID, not echoed.""" + app, _, _ = app_factory() + with TestClient(app) as client: + with client.stream( + "POST", "/api/pebble/ask", + json={"query": "x"}, + headers={"X-Trace-Id": "not-a-uuid"}, + ) as r: + trace = r.headers.get("X-Trace-Id") + assert trace and trace != "not-a-uuid" + import uuid as _uuid + _uuid.UUID(trace) + + +# --------------------------------------------------------------------------- +# G. Service caller delegation +# --------------------------------------------------------------------------- + +def test_service_caller_originating_user_propagated(app_factory): + """When Pebble (service) calls itself, the originating user's email + becomes Pebble's X-User-Email — not pebble@internal.""" + captured_headers: dict = {} + chunks = [b'data: {"type":"done"}\n\n'] + + class CaptureStream(_FakePebbleStream): + def __init__(self): + super().__init__(status_code=200, chunks=chunks) + + captured = {} + + class CaptureClient: + is_closed = False + def stream(self, method, url, **kwargs): + captured["headers"] = kwargs.get("headers", {}) + captured["body"] = kwargs.get("json", {}) + return CaptureStream() + + app, _, _ = app_factory(user={ + "user_id": "service:pebble", "email": "pebble@internal", + "is_service": True, + "originating_user_email": "rm@pursuit.org", + "request_id": "01993b8d-2c9a-7c4f-8b0e-000000000001", + "scopes": ("*",), + }) + pebble_proxy._get_pebble_client = lambda: CaptureClient() + + with TestClient(app) as client: + with client.stream("POST", "/api/pebble/ask", json={"query": "x"}) as r: + for _ in r.iter_bytes(): + pass + + assert captured["headers"]["X-User-Email"] == "rm@pursuit.org" + + +# --------------------------------------------------------------------------- +# H. Helpers +# --------------------------------------------------------------------------- + +def test_new_trace_id_mints_when_absent(): + request = MagicMock() + request.headers = {} + trace = pebble_proxy._new_trace_id(request) + import uuid as _uuid + _uuid.UUID(trace) + + +def test_new_trace_id_keeps_valid(): + valid = "01993b8d-2c9a-7c4f-8b0e-000000000001" + request = MagicMock() + request.headers = {"X-Trace-Id": valid} + assert pebble_proxy._new_trace_id(request) == valid + + +@pytest.mark.asyncio +async def test_close_resets_singleton(monkeypatch): + fake = _FakeClient(_FakePebbleStream()) + pebble_proxy._client = fake + await pebble_proxy.close() + assert pebble_proxy._client is None + assert fake.is_closed is True From d564d192e1dbe551927abe0e2f810b63fdee706f Mon Sep 17 00:00:00 2001 From: JP Date: Thu, 7 May 2026 14:17:08 -0400 Subject: [PATCH 12/35] feat(frontend-v2): GlobalSearch dual-mode + vitest infrastructure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Layer 0.10 + Layer 2.2 of the Pebble 1.0 plan. Test infrastructure established and the search modal becomes Find | Ask in one bar. frontend-v2 test infrastructure (Layer 0.10): * vitest 3.2 + @testing-library/react 16 + jsdom 26 added. * vitest.config.ts wires alias resolution, jsdom env, coverage floor (lines/functions/branches/statements >= 60% on changed files in src/components, src/pages, src/lib, src/services). * src/test/setup.ts polyfills IntersectionObserver + matchMedia for jsdom; quiets React 19 act() noise from libraries that haven't migrated; auto-cleans up rendered trees after each test. * package.json scripts: test (single-shot), test:watch, test:coverage. CI gate flips advisory→required once coverage hits the floor (followup). GlobalSearch dual-mode rebuild (Layer 2.2): * One bar, two modes: Find (default, /api/search typeahead) and Ask (Pebble streaming via /api/pebble/ask). * Mode triggers: segmented Find ⟷ Ask radiogroup at top, ?/ prefix as first char hard-switches to Ask and consumes the prefix, Cmd/Ctrl+I toggles mid-query without losing it. * Permanent footer "Ask Pebble: " chip when Find query >= 2 chars — closes the cmd-K-only discoverability gap UX adversary #3 flagged. Click chip → switch to Ask. * Find never silently routes to Ask. Wrong-Acme auto-promote to Ask is a trust killer; user always explicitly opts in. * Ask body streams SSE: parses `data: {...}\n\n` frames into {type: token|redirect|error|done}. Plaintext-only render — no rich HTML from LLM output, no