Reports: the LLM reliably used company_enrichment for prose fields but inconsistently populated the parallel Finding-list/string-list fields from the same evidence, even with progressively more explicit prompting. Add a code-level backfill (products, recent developments, financial signals, strategic initiatives, regulatory signals, risks/opportunities mirrored from SWOT, unknowns, monitoring recommendations) that only ever fills in what the model left empty, never overwrites what it produced. Enrichment tab: reorder sections (Products/Recent updates before Customers/Competitors) and add a per-section "Refresh" button that re-fetches just one of NinjaPear's six independent per-company endpoints when it came back empty - confirmed live that a data-coverage gap (e.g. Amazon returning no products) is real provider behavior, not a bug. Auth: the first account registered on a deployment with zero existing admins is now auto-promoted to admin, closing the chicken-and-egg gap where the only path to admin access was direct DB access. Self-heals if the last admin ever deletes their account. Also bumps nginx's proxy_read_timeout for api.ciagent.org to cover the enrichment refresh's synchronous funding-endpoint call (up to 5 minutes per NinjaPear's docs). Co-Authored-By: Claude Sonnet 5 <[email protected]>
73 KiB
73 KiB
Known Limitations
This file is updated as each phase lands. It exists so nothing is silently claimed to work when it doesn't.
Scope decisions (see PLAN.md)
- Customer review source: implemented as a real
SourceCollectorinterface with a documented fixture/mock adapter, not a live scraper - there's no reliable free public API, and the spec explicitly instructs against fabricating results for sources without one. Swapping in a real provider later means implementing one class against the existingSourceCollectorprotocol. - Patent source: as of Phase 14,
PatentSourceCollectorattempts a real call to USPTO's Open Data Portal whenUSPTO_API_KEYis configured (a free key via account registration), falling back to the same honest fixture/disabled behavior as before when it isn't. See the Phase 14 section below. - Nubela/NinjaPear company-enrichment API: considered and dropped in Phase 14 (believed enterprise-only), then wired up in Phase 15 once the user found and paid for an individual tier that Phase 14's research missed - see the Phase 15 section below.
- Job posting collector: generic HTML extraction from a company's own careers page is real. Board-specific APIs (LinkedIn, Indeed, etc.) are not implemented — most require paid access or prohibit automated collection in their terms.
- JS-rendered page collection (Playwright): the spec calls for Playwright for JS-heavy pages. The website/custom-URL collectors in this build use static HTML fetching (httpx + trafilatura/BeautifulSoup) only. A
PlaywrightRenderCollectorwould slot in behind the sameSourceCollectorinterface; not implemented in this pass to keep the collector container lightweight. Sites that require JS rendering will collect an empty/thin document and the source will be flagged rather than silently failing. - PDF report export: Markdown and JSON export are implemented; PDF export is documented as a later enhancement per the spec.
- OAuth social login: not implemented; the spec marks this as an optional later enhancement.
AUTH_MODE=jwt(email/password) andAUTH_MODE=local(dev) cover the MVP.
Frontend dependency notes
npm auditreports a moderate esbuild advisory (dev-server-only, requires a malicious site to reach your local Vite dev/test server while it's running — not exploitable in production or CI) and two high advisories inside Next.js's own vendoredpostcss/sharp(used only by the Image Optimization pipeline, which this app does not currently use vianext/image). Tracked upstream; will clear on Next's next patch release. Re-runnpm auditafternpm installto check current status.
Infrastructure
- Docker Desktop must be running before
docker compose up; this was verified during Phase 1 (see TASKS.md). - On Windows + Docker Desktop, the frontend dev container's file watcher does not always pick up newly created route files through the bind mount (edits to existing files hot-reload fine). If a new page 404s right after adding it,
docker compose restart webpicks it up. This is a bind-mount/watcher quirk, not an app bug. web'snode_modulesand.nextare both anonymous Docker volumes (docker-compose.yml's- /app/node_modulesand- /app/.nextlines, needed so the container's Linux-built native modules and build cache don't get clobbered by the host's bind-mountedapps/web). This means adding a new npm dependency and runningdocker compose build webis not enough - the anonymous volumes from the container's first-everuppersist across rebuilds and plaindocker compose restart, so a new package still 404s asModule not found, and (observed live during Phase 11) an edited existing file can silently keep serving its pre-edit output even after a restart, because Next dev's on-disk.nextcache survives the restart untouched.docker compose restart webalone is not a reliable way to pick up source changes made while the container was already running under Docker Desktop on Windows - the fix in both cases is the same:docker compose rm -f -s -v web(removes the container and its anonymous volumes) thendocker compose up -d webto recreate it clean. The mirror-image gotcha to the Python one below - same root cause (a persistent layer masking a rebuilt image/changed source), different mechanism (anonymous volume vs. stale image tag).
Auth (Phase 2)
- Access/refresh tokens are stored in
localStorageon the frontend for simplicity (seeapps/web/lib/api-client.ts). This is a reasonable tradeoff for a local-first MVP but is XSS-exposed compared to httpOnly cookies; a production multi-user deployment should move to cookie-based storage with CSRF protection before going live publicly. - There is no email verification, password reset, or account lockout after repeated failed logins yet (rate limiting mitigates brute force but doesn't lock the account). Password reset is called out in the spec as a "documented future capability" for the MVP.
- Admin role (
User.is_admin) now gates several real endpoints (require_admin- Server secrets, IP bans, Database viewer, system logs, unban-request review) - this note originally said none existed yet, long since outdated. The first account ever registered on a deployment with zero existing admins is now automatically promoted to admin (auth_service.register, checked viaUserRepository.count_admins) - otherwise the only path to admin access was direct DB manipulation, a real chicken-and-egg problem for anyone self-hosting from a clean clone (see DEPLOYMENT.md §8). Checked by admin count, not total user count, so it also self-heals if the last admin ever deletes their own account - the next registration becomes admin again rather than leaving the deployment permanently admin-less. A benign race is possible if two people register in the same instant on a brand-new, zero-admin deployment (both could become admin) - acceptable since it only ever matters once, before any real traffic exists, and DEPLOYMENT.md tells operators to register immediately after bringing the stack up, before opening the firewall.
Collection pipeline (Phase 4)
- Auto-discovery (
collection_service.discover_sources_for_company) is not wired intoPOST /companiesyet — it makes several live outbound HTTP calls (robots.txt, sitemap, GitHub search, SEC EDGAR search) which don't belong on the synchronous request path. It's called directly today (see integration tests) and will be triggered as a background job once Celery exists (Phase 5), matching the spec's "System discovers company sources" step in the async pipeline diagram. - The
SCRAPER_DOMAIN_DELAY_SECONDSper-domain rate limit and the robots.txt cache are in-process dictionaries. Fine for a single API/worker process; a multi-worker Celery deployment would need a shared store (Redis) for these to be true global limits rather than per-worker ones. - SEC EDGAR collector stores filing metadata (form type, date, accession number, link) rather than parsing full filing document bodies — full-text filing parsing is a meaningfully larger scope (SEC filings are large, structurally inconsistent HTML/XBRL documents) left for a future pass.
- No collector renders JavaScript (no Playwright integration in this pass — see the Non-goals note in PLAN.md). A JS-heavy page will collect whatever static HTML is served, which may be thin or empty; the source will still report success with fewer/emptier documents rather than silently failing.
Background processing (Phase 5)
- Docker image drift on new Python deps:
api,worker, andbeatall build from the sameinfrastructure/docker/api.Dockerfile, but each is a separate image. Adding a dependency toapps/api/pyproject.tomland only runningdocker compose restart apileavesworker/beaton stale images (they'll crash withModuleNotFoundError). Rundocker compose build api worker beat(ordocker compose up --build) after any dependency change, not just a restart. @celery_app.taskvs@shared_task: this codebase defines exactly one Celery app (app.tasks.celery_app.celery_app) and every task is bound to it explicitly with@celery_app.task(...). Using@shared_taskhere silently binds the task to Celery's implicit default app instead — which doesn't have our Redis broker ortask_always_eagerconfig — and.delay()will try to connect to a default RabbitMQ broker and fail. Discovered and fixed during this phase; don't reintroduce@shared_taskinapp/tasks/*.- Nested event loop under eager mode: Celery's synchronous task functions bridge into the app's async service layer via
asyncio.run(...). UnderCELERY_TASK_ALWAYS_EAGER=true(tests, or any.delay()call made from inside an async FastAPI route handler), the task body executes synchronously inside the caller's already-running event loop, and a plainasyncio.run()raisesRuntimeError.app/tasks/base.py::run_async_taskdetects this and falls back to running the coroutine in a dedicated thread with its own loop. This only matters for eager/in-process execution; a real worker process never has this problem. - The per-domain rate limiter and robots.txt cache noted in the Phase 4 section are still in-process only — a multi-worker Celery deployment (
--concurrency> 1, or multiple worker containers) would need a shared store (Redis) for these to be true global limits. - No task deduplication beyond the single-active-run-per-company check (
MonitoringRunRepository.get_active_for_company). There's no distributed lock, so a very tight race (two beat ticks within milliseconds) could theoretically enqueue two runs; not observed in testing and low-impact if it happened (the second run just collects again).
Change detection (Phase 6)
- No semantic (Layer 4) comparison yet. The spec describes an optional LLM/embedding-based layer for cases where the text diff is ambiguous. That needs the LLM provider interface, which is Phase 7. Layers 1-3 (hash/structured/text) plus deterministic scoring already produce real, correctly-classified detections; Layer 4 would improve recall on subtle wording-only changes, not correctness of what's already detected.
- Single-source only.
independent_source_countis hardcoded to1inchange_detection_service.py- cross-source corroboration (e.g. a press release and a job posting both pointing at the same expansion) is the spec's "cross-source synthesis" task, which is Phase 7 (LLM analysis) scope. Every current detection is scored as uncorroborated, which the formula already discounts significantly by design. - Leadership/price extraction are regex heuristics, not NLP - see the docstring in
app/change_detection/extractors.py. They catch common phrasings ("named CEO", "$49/month") and will miss unusual ones or produce occasional false positives on lookalike phrasing. Phase 7's LLM-based extraction task is the higher-fidelity replacement/supplement. - Structured diff only tracks the URL set, not deeper structured fields (specific job titles changing wording, individual price line-items on a page with several). A URL/title appearing or disappearing is caught; a price changing on a page that lists ten prices, where the page's overall URL set doesn't change, is caught by the text-diff price extractor instead (which does work across the whole page, not per-item).
LLM analysis (Phase 7)
AnthropicLLMProvideris now verified live end-to-end - the user supplied a realANTHROPIC_API_KEY(claude-sonnet-5) and, combined with a realBRAVE_SEARCH_API_KEY, the full pipeline was driven for real: company discovery (Discover -> Review, real search + real structured extraction), company creation, the pre-existing lazy source-discovery-on-first-run crawl ofstripe.com(23 items collected across 3 sources, 0 failures), and baseline report generation via a realapi.anthropic.com/v1/messagescall. The resulting report was coherent and correctly epistemically humble - it explicitly named what it could and couldn't substantiate from the evidence actually collected (no fabricated financials/leadership claims). This is the strongest live-verification signal in the whole app: real evidence in, real grounded analysis out, exactly as the architecture intends.Ollamaremains structurally complete but unverified - no local Ollama instance is available in this environment; unit-tested with the client mocked only. SetLLM_PROVIDER=ollama+ a running Ollama instance to exercise it for real.- MockLLMProvider is deliberately non-generic - each of the six analysis tasks has a purpose-built builder function that reads the evidence block and produces genuinely evidence-derived output (real counts, titles, severities), rather than a reflection-based generic filler. This is intentional (see
app/analysis/llm/mock.pydocstring) but means adding a new analysis task requires writing its own mock builder, not just a new Pydantic schema. - Tasks A (relevance), B (extraction), and C (synthesis) are implemented and tested but not yet wired into the collection/report pipeline - Task D (report generation) currently consumes raw
SourceDocument/DetectedChangerows directly rather than pre-filtering through Task A or pre-extracting via Task B/C. Wiring them in would improve report quality (e.g. filtering irrelevant documents before they reach the report prompt) but the report already works correctly without it, sincereport_service.py's evidence-gathering only pulls documents/changes already scoped to the company. - A report is regenerated only on baseline (first evidence) or when a run detects an actual change - not on every scheduled run, to keep LLM cost proportional to real activity rather than to schedule cadence. See
app/tasks/collection.py. - No token/cost usage tracking or per-user LLM budget yet -
LLM_MAX_TOKENS_PER_REQUESTandLLM_MAX_RETRIESbound a single call, but there's no aggregate usage dashboard (spec section 30's "Display usage statistics in settings" is Phase 10/not-yet-done). - Report list/string fields are backfilled from
company_enrichmentwhen the LLM leaves them empty - confirmed live across multiple real Anthropic calls (even with an explicit worked example in the prompt) that the model reliably uses enrichment data for prose fields (company_overview,market_positioning) but is inconsistent about mirroring the same data into list-shaped fields (products_and_services,financial_signals,strategic_initiatives, etc.).report_service.py's_backfill_from_enrichment/_mirror_swot_into_flat_lists/_backfill_reflective_sectionsfill these in mechanically fromcompany_enrichment.products/recent_updates/funding/specialties, from the model's own SWOT output, and from the actual presence/absence of evidence - never overwriting content the model did produce, and never inventing anything not already present in real evidence.key_inferred_projectsis the one field left with no such fallback - it asks for specific, speculative in-progress projects, which has no honest mechanical source (unlike a specialty list or a funding round, there's no real "raw project" data to transcribe), so it stays genuinely empty unless the model or actual source documents/detected changes surface something.
Notifications (Phase 8)
TwilioSmsProviderwas verified live against a real Twilio account, and the auth/transport layer works correctly - the user supplied realTWILIO_ACCOUNT_SID/TWILIO_AUTH_TOKEN/TWILIO_FROM_NUMBER, added a real SMS notification destination through the Settings UI, and triggered a real "Send test notification." The request reachedapi.twilio.comwith correct auth and was rejected with a real Twilio error (400, code572006,"Invalid template name. Trial accounts can only use predefined SMS templates."), which the app surfaced cleanly inline on the destination row rather than crashing or swallowing it -TwilioSmsProvider.send()'s error-passthrough path (previously only respx-mocked) is now confirmed correct end-to-end. This exposed a real product-level blocker, not a code bug: Twilio trial accounts no longer support arbitrary custom SMS body text at all - they restrict outbound SMS to Twilio's own predefined templates (order confirmations, appointment reminders, etc.), confirmed via Twilio's trial-restrictions docs. Since every alert message this app sends is dynamically generated (company name, severity, summary - seeapp/notifications/message_builder.py), custom-body SMS cannot work on a Twilio trial account at all, for any app, not just this one. There is no code-side workaround. The account needs to be upgraded off trial (a payment method added, converting it to a standard pay-as-you-go account) before real SMS alerts can be verified end-to-end. Once upgraded, the provider itself is expected to work as-is - the failure observed was purely Twilio's trial content policy, not a malformed request.TelnyxSmsProvideradded as a second SMS vendor, selected viaSMS_PROVIDER=twilio|telnyx(app/notifications/telnyx_sms.py,app/notifications/factory.py) - a plain Bearer-authenticated REST POST tohttps://api.telnyx.com/v2/messages, same no-SDK shape asTwilioSmsProvider. Unit-tested with the HTTP call respx-mocked (success, API-error with Telnyx'serrors[]shape, and not-configured paths). The user then supplied realTELNYX_API_KEY/TELNYX_FROM_NUMBERcredentials and drove a real "Send test notification" through the Settings UI. First attempt correctly surfaced a real403("Only pre-verified destinations are allowed at this account level"- Telnyx trial accounts cap outbound SMS to a single verified destination number). After the user verified the destination number in the Telnyx portal, a retry got a real200 OKfromapi.telnyx.comand the UI showed "Test sent" - but the text never actually arrived on the phone.- This exposed a real gap: a
200 OKfromPOST /v2/messagesonly means "Telnyx accepted the send request," not "the carrier delivered it." PollingGET /v2/messages/{id}afterward (not something the app does automatically - no webhook receiver exists in this dev stack) showed the true outcome:status: "delivery_failed", error40010-"The sending number is not 10DLC-registered but is required to be by the carrier."US carriers (T-Mobile/AT&T/Verizon) require standard 10-digit long-code numbers sending A2P SMS to go through 10DLC campaign registration - this is an industry-wide carrier requirement, not a Telnyx or app-specific issue, and Twilio has the identical requirement. There is no code-side workaround; the sending number's 10DLC campaign (or a toll-free number's Toll-Free Verification, a separate and often-faster alternative) must complete registration in the Telnyx portal before delivery will actually succeed. The app'ssend()methods for both SMS providers report success based only on the initial API response, not eventual carrier delivery status - a known simplification, not fixed here, since confirming real delivery would require either polling every send (adds latency/cost per message) or a publicly-reachable webhook endpoint (not available in this local dev stack). Worth revisiting if/when this deploys somewhere with a public URL. - Per the user's direction, SMS is switched back off (
NOTIFICATION_SMS_ENABLED=false) while 10DLC registration is pending, and this now applies consistently everywhere SMS could be triggered - a real gap was found and fixed in the same pass:alert_service.send_test_notification(the "Test" button) previously ignoredNOTIFICATION_SMS_ENABLEDentirely and would call a configured SMS provider for real regardless of the kill switch; only the real-alert-dispatch path (create_alert_for_change) checked it. Fixed so both paths short-circuit identically - verified live (the "Test" button now returns "SMS delivery is currently disabled..." with zero calls toapi.telnyx.com, confirmed via API logs before/after). The Settings page's "Add destination" form also now shows a specific inline notice when SMS is selected while disabled, explaining why (10DLC registration pending) rather than just that it won't work -apps/web/app/(app)/settings/page.tsx. 211 backend tests (up from 210), 18 frontend tests unchanged. - No delivery retry.
NotificationDelivery.attempt_countexists in the schema for future use, butalert_service.create_alert_for_changecurrently makes exactly one send attempt per destination and records whatever result comes back - a transient SMTP/Twilio failure is recorded asFAILEDand not retried. A retry task (e.g. exponential backoff via the existingnotificationsCelery queue) would be the natural follow-up. - No alert-level dedup/digest across runs. Change-level dedup/cooldown already exists (Phase 6 - exact-repeat suppression within 24h), but there's no "don't re-alert on the same underlying issue" or "batch several changes from one run into a single digest email" logic; every alert-worthy
DetectedChangeproduces its own Alert and its own set of notification sends. - Email/SMS body text is not localized or user-customizable -
app/notifications/message_builder.pyproduces one fixed English format per channel. Per-user notification templates are not implemented. NotificationDestination.verifiedis tracked on the model but never set totrueanywhere - there's no verification-code/confirmation-link flow yet (a destination is usable for real sends as soon as it's created, gated only byenabled+minimum_severity). Verification is a natural Phase 10+ hardening item, especially for SMS where sending to an unverified number has cost/abuse implications.
Fixture demo + tests (Phase 9)
- The switchable live-demo mechanism built in this phase (the dev-only
app/dev/fixtures.pyHTTP-serving/version-switch endpoints, the Settings page's "Acme Mobility demo" panel,scripts/seed_acme_demo.py,scripts/switch_fixture.py, the SSRF allowlist setting that existed only to support it, and the Playwright E2E spec that drove it through the UI) has since been removed entirely - it added a live, always-mounted-outside-production HTTP surface and a dedicated SSRF-guard carve-out for a one-off manual demo workflow that wasn't worth the ongoing maintenance/security surface. The underlying fixture HTML files (apps/api/tests/fixtures/acme_mobility/v1|v2/*.html) and the backend regression test that reads them straight off disk (apps/api/tests/integration/test_acme_fixture_demo.py) are unaffected and remain the automated way this pipeline's baseline→change→alert behavior gets locked in (leadership/price/content-modified/no-change, all four outcomes).
Hardening & docs (Phase 10)
- Rate limits are per-IP (
slowapi's defaultget_remote_addresskey func), not per-user. Multiple users behind the same NAT/corporate proxy share a limit bucket. Fine for a local-first MVP; a real multi-tenant deployment behind a load balancer would want to key on the authenticated user id instead (falling back to IP for unauthenticated routes like register/login). - The daily manual-run cap (
MAX_MANUAL_RUNS_PER_DAY) counts per company, not per user. A user with several companies gets the cap on each independently rather than a combined daily budget. This matches how the setting reads ("manual runs per day") but is worth confirming against actual intent if usage patterns suggest otherwise. - Correlation-id propagation into Celery's threaded eager-mode fallback required a real fix, not just a wrapper.
ThreadPoolExecutordoesn't copycontextvarsinto the new thread by default, which would have silently droppedrequest_id/run_id/task_idfrom log lines whenever a task ran via the threaded fallback path inapp/tasks/base.py::run_async_task(used underCELERY_TASK_ALWAYS_EAGER=trueor when.delay()is called from inside an already-running event loop). Fixed by explicitly capturingcontextvars.copy_context()and running the executor call through it - see the comment inrun_async_task. Worth knowing about if a new thread-hopping code path is added elsewhere in the task layer; the same gap would reappear unless it goes through the same helper. - Data retention only covers
SourceDocument.Snapshot,DetectedChange,Report,Alert, andNotificationDeliveryall persist indefinitely regardless ofDATA_RETENTION_DAYS- a deliberate scope decision (seeSourceDocumentRepository.delete_older_than's docstring and the FK-cascade risk it documents), not an oversight, but it does mean actual storage growth from those tables is unbounded in a long-running deployment. Extending retention to them safely would need cascade-aware deletion (e.g. only delete aDetectedChangeif noAlertreferences it) rather than a blind age-based purge. - No admin/ops endpoint to trigger the retention purge on demand - it only runs on the Celery Beat schedule (3am UTC daily). Verified live by invoking
app.tasks.maintenance.purge_expired_data.delay()directly against the Docker stack rather than through an HTTP route.
Company discovery & provider completion (Phase 11)
BraveSearchProvideris now verified live - the user supplied a realBRAVE_SEARCH_API_KEYand the discover flow was driven through the browser against the live Docker stack; all realapi.search.brave.comqueries (official website, headquarters, competitors, "formerly known as") returned200 OKwith real result data. This is the one external provider in the whole app that has actually been confirmed working against its real API, not just unit-tested with the HTTP layer mocked.GeminiLLMProvider's structured-output schema had a real bug, found and fixed via this live verification:CompanyProfileExtraction.public_identifierswas originallydict[str, str], which Pydantic turns into an open-ended (additionalProperties) JSON schema - the Gemini Developer API (free-tier key auth, as opposed to Vertex AI Enterprise mode) rejects that shape outright withValueError: additionalProperties is only supported in Gemini Enterprise Agent Platform mode. This is exactly the class of bug a mocked-SDK unit test cannot catch, since the mock never runs the real schema-conversion code path. Fixed by changing the field tolist[PublicIdentifier](a fixed{key, value}object shape) inapp/prompts/company_profile.py, withdiscovery_service.pyconverting the list back to adict[str, str]for the (unaffected) public API response shape - seeARCHITECTURE.md. Any future Gemini-targeted structured-output schema must avoid free-formdict/Mappingfields for this same reason.- After that fix, a real
GEMINI_API_KEYstill could not be verified end-to-end - Google returned429 RESOURCE_EXHAUSTEDwithlimit: 0for bothgenerate_content_free_tier_requestsandgenerate_content_free_tier_input_token_countongemini-2.0-flash. Alimit: 0(not "quota exceeded from usage") typically means the free tier isn't actually active for that API key's Google Cloud project - e.g. the Generative Language API needs to be explicitly enabled in Cloud Console, or the AI Studio account needs to complete a one-time terms/region check - rather than anything this app's code can work around. If you hit this: check ai.dev/rate-limit for that project, confirm the key was generated from an AI Studio project with the free tier actually enabled, and retry - the request/response plumbing itself (schema, retries, error surfacing) is confirmed correct up to the point Google's API rejects the call. The endpoint fails gracefully either way:POST /companies/discoverreturns a clean error response and the wizard shows "Something went wrong. Please try again." rather than crashing. MockSearchProvider's domain guess is a plausibility heuristic, not a real lookup (app/search/mock.py::_guess_domain- lowercases the name and strips all non-alphanumeric characters, then appends.com). For a real company name this often happens to resolve to that company's actual site; for a fictional/test name it can resolve to an unrelated real domain (observed live:"Acme Mobility"→acmemobility.com, a real parked/for-sale domain). This is expected mock behavior, not a bug - the mock is explicitly documented as synthetic/dev-only, and the monitoring pipeline correctly handles the resulting real-world 0-evidence case by finishing the run withstatus=failedrather than crashing.- The discovery endpoint's LLM extraction quality is entirely a function of
MockLLMProvider's regex heuristics under the sandbox default (app/analysis/llm/mock.py::_build_company_profile, e.g._HQ_RE/_FORMERLY_RE) - it only fills inheadquarters/aliaseswhen the fetched evidence text happens to contain a matching phrasing ("headquartered in...", "formerly known as..."). Real company websites frequently don't phrase things this way, so most discovered profiles will legitimately come back with several blank fields under mock providers - this is why the Review step's blank-field placeholders ("Not found - fill in") are load-bearing UI, not just cosmetic. A realLLM_PROVIDER(anthropic/gemini/ollama) with real search evidence would fill these in far more often. - No confidence indicator per discovered field. The wizard shows that a field came back blank (via placeholder text) but doesn't distinguish "the LLM was highly confident" from "the LLM guessed from a weak snippet" for fields that were filled in - deliberately deferred (see the plan's out-of-scope section) to avoid scope creep;
sources_consultedgives basic transparency into what evidence existed without a full confidence UI. - Discovery is a real, metered API call (one
SearchProvider.search()per targeted query plus oneLLMProvidercall) and has no per-user quota beyond the flat5/minuterate limit onPOST /companies/discover- a user re-running discovery repeatedly (e.g. tweaking hints and re-submitting) re-pays the full search+LLM cost each time rather than there being any caching/memoization of prior discovery calls for the same name. _resolve_official_websiteskips a fixed list of reference/social hosts (Wikipedia, LinkedIn, Crunchbase, etc.) when picking from real search results, but only when a better-ranked alternative exists in the same result set (app/services/discovery_service.py::_NON_CORPORATE_HOSTS). Found live: Brave's top result for"Stripe official website"was Stripe's Wikipedia article, notstripe.com; taking it at face value fed the wrong base domain into every downstream source-preview collector (observed: a brokenen.wikipedia.org/careersURL in the "sources to monitor" preview). Fixed by preferring the first non-reference-host result when one is present in the top 3, falling back to the top result otherwise (a genuinely obscure company with only a Wikipedia page as a real hit should still resolve to it, notNone). This is a heuristic over a fixed hostname list, not a general solution - a well-known company whose actual corporate site is itself hosted on one of these domains, or whose only real presence is a listed host not in_NON_CORPORATE_HOSTS, wouldn't be caught by this fix.
UI polish & bug-fix round (Phase 12)
- Competitor cross-linking matches by exact (case-insensitive) company name only (
apps/web/app/(app)/companies/[id]/page.tsx::monitoredByName) - it does not check the target company's aliases, nor does it do fuzzy/partial matching. A competitor listed as"PayPal, Inc."would not match an already-monitored company named plain"PayPal", and would incorrectly route to the wizard's "add new" path instead of the existing company. Since competitor names come from LLM extraction (or user-typed hints), this can happen whenever the discovered/typed competitor name doesn't exactly match the monitored company'snamefield. Worth revisiting if this becomes a frequent papercut - matching againstaliasestoo, or a normalized/fuzzy comparison, would close most of the gap. - The "generate report before any evidence exists" case is now flagged with a warning, not prevented. The Latest Report tab shows an inline notice when
runs.length === 0, but "Generate report now" is never actually disabled - a user can still click through and get a thin, evidence-free (but honestly-labeled) report. This is intentional: there's no strong reason to hard-block report generation, just to make sure the user understands what they're about to get before they ask for it. - The new Snapshots tab has no pagination (
GET /companies/{id}/snapshotscaps at 50, newest-first, same pattern as monitoring-run history) - for a company with a long monitoring history across many sources, older snapshots beyond the cap are simply not shown. No UI affordance to page past the cap yet; would need one if 50 turns out to be too few for real usage patterns. - Snapshot text/structured summaries render as plain text/JSON, not a diff view. The Snapshots tab shows what a snapshot contained, not what changed between it and the previous snapshot for the same source - that comparison already exists internally (
DetectedChangerows, shown via Alerts/Monitoring history) but isn't cross-linked from a snapshot row to theDetectedChangeit produced (if any). A "view what changed from the prior snapshot" link would be a natural follow-up.
Second UI polish & bug-fix round (Phase 13)
- Duplicate-name detection is a client-side heuristic, not authoritative.
findPossibleDuplicate(apps/web/app/(app)/companies/new/page.tsx) strips punctuation and common legal suffixes (Inc/LLC/Corp/etc.) then checks exact or substring match against the user's currently-loaded company list - it won't catch a genuinely different-looking name for the same company (e.g. "Facebook" vs "Meta"), and a slow/faileduseCompanies()fetch means the check silently finds nothing rather than blocking. The backend's_unique_display_nameis the actual hard guarantee (uniqueness per user, filesystem-style auto-suffix) - the frontend warning is a courtesy to catch obvious cases before paying for a real search+LLM discovery call, not a substitute for it. - Notification-destination company-linking requires at least one company to exist before a destination can be created (
company_idshasmin_length=1inNotificationDestinationCreate) - a brand-new user with zero companies can't pre-register a notification destination; the Settings page's Add form correctly reflects this ("Add a company first…") but it's a real ordering constraint, not just a UI nicety. - The one-time migration's dedup step is lossy for
NotificationDeliveryhistory: when two destination rows shared the same (user, type, value), the newer duplicate(s) were deleted and their delivery history cascaded away with them (seeTASKS.mdPhase 13 and the migration's own docstring,60a25ddfc6a3). Acceptable for this app's current scale/stage - delivery history isn't relied on for anything beyond the Alert detail view - but worth knowing if delivery audit history ever becomes load-bearing. Company.notification_linksandNotificationDestination.company_linksboth need explicit ORMcascade="all, delete-orphan"(not just the DB-levelON DELETE CASCADEin the migration) because SQLite - used for local dev and the entire test suite - doesn't enforce foreign-key constraints without an explicitPRAGMA foreign_keys=ONthis app doesn't set. This was caught before it became a real bug (tests exercise the SQLite path and would have caught silent cleanup failures), but it's a sharp edge worth remembering for any future cascade-delete relationship added to this codebase: anondelete="CASCADE"in the migration alone is not sufficient, the ORM relationship needs its own cascade declaration and the parent object needs the child eager-loaded before deletion (seeCompanyRepository._with_relations's comment).- Report generation's new
company_profileevidence block has no per-field confidence/staleness signal. If a company's discovered profile turns out to be wrong (e.g. a bad search result at onboarding time) or goes stale (the company rebrands, moves HQ), the report will confidently repeat that stale/wrong fact indefinitely, since there's no re-verification step and no way for the report prompt to know the profile data's age or original confidence. Editing the company's profile fields (Configuration tab) is the only way to correct this today.
First UI polish & bug-fix round (Phase 12)
- Competitor cross-linking matches by exact (case-insensitive) company name only (
apps/web/app/(app)/companies/[id]/page.tsx::monitoredByName) - it does not check the target company's aliases, nor does it do fuzzy/partial matching. A competitor listed as"PayPal, Inc."would not match an already-monitored company named plain"PayPal", and would incorrectly route to the wizard's "add new" path instead of the existing company. Since competitor names come from LLM extraction (or user-typed hints), this can happen whenever the discovered/typed competitor name doesn't exactly match the monitored company'snamefield. Worth revisiting if this becomes a frequent papercut - matching againstaliasestoo, or a normalized/fuzzy comparison, would close most of the gap. - The "generate report before any evidence exists" case is now flagged with a warning, not prevented. The Latest Report tab shows an inline notice when
runs.length === 0, but "Generate report now" is never actually disabled - a user can still click through and get a thin, evidence-free (but honestly-labeled) report. This is intentional: there's no strong reason to hard-block report generation, just to make sure the user understands what they're about to get before they ask for it. - The new Snapshots tab has no pagination (
GET /companies/{id}/snapshotscaps at 50, newest-first, same pattern as monitoring-run history) - for a company with a long monitoring history across many sources, older snapshots beyond the cap are simply not shown. No UI affordance to page past the cap yet; would need one if 50 turns out to be too few for real usage patterns. - Snapshot text/structured summaries render as plain text/JSON, not a diff view. The Snapshots tab shows what a snapshot contained, not what changed between it and the previous snapshot for the same source - that comparison already exists internally (
DetectedChangerows, shown via Alerts/Monitoring history) but isn't cross-linked from a snapshot row to theDetectedChangeit produced (if any). A "view what changed from the prior snapshot" link would be a natural follow-up.
New intelligence sources & per-source scheduling (Phase 14)
PatentSourceCollector's real USPTO branch is now live-verified (Phase 15), and the live pass found real schema bugs that mocked tests couldn't catch - fixed: the sort field needed to beapplicationMetaData.filingDate, notfilingDate(was causing a hard500);inventionTitle/filingDate/abstractTextall live underapplicationMetaData, not at the entry's top level (was producing "Untitled patent filing" with no dates for every real result); USPTO returns404for "no matching records" rather than200with an empty array, now treated as an honest emptyACTIVEresult instead of aFAILEDone (same non-error empty-result precedent asGovContractCollector).- Querying USPTO by assignee/company name is not supported by this endpoint at all - confirmed by inspecting a real response's full field list (for a query that did return 110k+ real results by inventor name), which contains no assignee/organization field anywhere. USPTO's Patent Application Search reliably supports inventor-name and application-number lookups only. Worked around, not fixed:
PatentSourceCollector.collect()now searches USPTO by each of the company's known leadership names instead (from NinjaPear enrichment,CompanyContext.leadership_names, threaded through incollection_service.to_company_context) - live-verified against Stripe, returning 19 real patent documents by searching its executives' names. This is a real, working signal, but a heuristic one: there is no way to confirm a patent found this way actually belongs to the monitored company (vs. a same-named person, or work done at a prior employer) - every such document is trust-scored at 0.5 (vs. what a verified-assignee match would warrant) and its content explicitly states which leadership name it matched on, so the report LLM's confidence labeling reflects the uncertainty rather than treating it as confirmed fact. Two real constraints this implies: (1) a company gets zero patent results until NinjaPear enrichment has actually completed and found a leadership team (noNINJAPEAR_API_KEYconfigured means patents stays empty too, even with a valid USPTO key) - self-heals on the next scheduled collection once enrichment finishes, since leadership names are re-read from the DB on every run, not cached at discovery time; (2) capped at the first 5 leadership names per company (_MAX_INVENTOR_SEARCHESinapp/collectors/patents.py) to bound the number of USPTO calls per run - USPTO itself is free/keyless, so this cap is about politeness and run time, not cost. GovContractCollectoralways reports a source as discoverable for every company, including obviously-private ones - matchingSecEdgarCollector's existing precedent for non-public companies. A private company simply gets zero contract results (not an error); there's no attempt to guess whether a company is likely to have federal contracts before offering the source.- Google News RSS's query is just the company's
namefield, url-encoded - no disambiguation for a company name that collides with something unrelated (e.g. a common word, or a same-named but different company). A company with a highly generic name will get noisy/irrelevant results; the existing evidence-grounded report generation still won't fabricate claims from noise, but the raw collected documents themselves aren't filtered for relevance at the collector layer. - Per-source scheduling has no cron/custom-interval UI - the Sources tab's "Check frequency"
Selectonly offers the fixed cadences (hourly through monthly), matchingMONITORING_FREQUENCIESminuscustom; the backend (SourceUpdateschema,validate_and_compute_next_run) fully supports a per-sourcecustomoverride withinterval_minutes/cron_expression, but there's no frontend control to set one - would need the same extra interval/cron inputs the company-level Configuration tab doesn't have either (it also has no dedicated custom-schedule UI beyond raw form fields). - A newly-set or newly-cleared per-source override resets
next_checktoNone, meaning "due on the very next scheduler tick" rather than computing a real futurenext_checkimmediately (unlike the company-levelMonitorConfiguration, which does recomputenext_runimmediately on a schedule change). This is a deliberate choice to match how a brand-new source already behaves (checked ASAP), but it does mean setting a slow cadence (e.g. monthly) on a source still triggers one immediate check before the monthly cadence actually takes effect - there's no way to say "start counting from now, don't check immediately." - The scheduler's due-ness check queries every enabled
MonitorConfigurationand then does a Python-side per-company source scan (SourceRepository.company_has_due_work, called once per enabled company per Beat tick) rather than a single SQL query joining across companies and sources. Fine at this app's scale (a personal/small-team tool with a handful to dozens of companies); would need to move the OR-with-NULL-fallback due-ness logic into SQL if the company count grew large enough for this to become a real per-tick cost.
NinjaPear company enrichment (Phase 15)
- Live-verified against a real NinjaPear account and a real company (Stripe) - and the initial implementation, written from a scraped/summarized reading of the JS-rendered
nubela.co/docs, had several real schema bugs the live pass caught and fixed: identity iswebsite-only (there is no name-based company lookup at all - a company with noofficial_websitenow fails fast with a clear error instead of attempting doomed calls, seeenrichment_service.enrich_company's upfront guard);employee_countandindustrycome back as raw numbers, not strings; funding'sinvestorsare objects ({name, type, website}), not plain strings; response field names throughout differ from the initial guesses (executivesnotleadership_team,total_funds_raised/funding_rounds/round_typenottotal_raised/rounds/round_name,competitors[].website/competition_reasonnotname/reason,x_profile_urlnotprofile_url, work-email/profile lookups takefirst_name/last_name/domainnot a single name string); and the credit-balance endpoint is/api/v1/meta/credit-balancereturningcredit_balance, not/company/credit-balancereturningbalance. All fixed and re-verified live - a real run against Stripe returned real leadership bios, work emails, X profiles, funding history, competitors-with-reasons, live blog updates, and inferred customers, landing onstatus: partial(one funding-parsing bug, since fixed, and one legitimate 404 for a board member not in NinjaPear's database) for 34 real credits. - Credit-cost tracking (
CompanyEnrichment.credits_spent, the Settings-page balance display) is an estimate, not billing-accurate. Per-call costs inenrichment_service._CREDIT_COSTSare transcribed from NinjaPear's published pricing page at a point in time and don't account for per-item add-on charges NinjaPear may apply (e.g. funding's "+1 per investor," customer listing's "+2 per company returned") - these are approximated as flat per-call costs. Useful for a rough sense of spend, not a source of truth; NinjaPear's own dashboard/credit-balance endpoint is the actual authority (which is why/system/statusalso surfaces the real balance separately, not just the estimated spend). - The "Similar People" endpoint is not wired in at all - it's a role/company-anchored prospecting tool ("find people like X at competitor Y") with no natural trigger at onboarding time, since no specific role is ever selected. Would need its own UI (e.g. "find people like this one" on a leadership-team entry) to make sense, not an automatic onboarding call.
- NinjaPear's suggested competitors are never merged into
Company.competitors(the user-typed/reviewed list that drives Phase 12's competitor cross-linking) - shown only in the Enrichment tab, with their stated reasons, kept deliberately separate so a background API call can never silently change what the user explicitly reviewed. A "add as competitor" action would be a reasonable follow-up if this becomes a papercut. - Person-level lookups (work email, profile) are capped at
NINJAPEAR_MAX_LEADERSHIP_LOOKUPS(default 5) per company, applied to however many leadership members NinjaPear's Company Details call happens to return, in whatever order it returns them - there's no ranking by seniority/relevance before the cap is applied, so for a company with a large leadership team, which specific people get resolved is effectively arbitrary. - No automatic re-enrichment - by design (see Phase 15's Context in
TASKS.md, onboarding-only was the explicit scoping decision to bound cost), soCompanyEnrichment.datacan go stale indefinitely (a funding round happens, a leadership change occurs) with no scheduled refresh. A manual, per-section refresh does exist (POST /companies/{id}/enrichment/sections/{section}/refresh,enrichment_service.refresh_section): each of NinjaPear's six per-company endpoints (details, funding, updates, competitors, products, customers) maps to its own UI section, and a "Refresh" button appears next to a section's empty-state message ("None found.") so a genuine provider-side data gap (confirmed live: NinjaPear's real API returnsproducts: []for Amazon specifically, with no error - not a bug, just no product data on their end for that company) can be manually retried without re-running the whole enrichment or burning credits on the sections that already succeeded. Runs synchronously in the request (not queued via Celery like onboarding enrichment), since it's a single provider call rather than the full six-plus-leadership fan-out - nginx'sproxy_read_timeoutonapi.ciagent.orgis set to 320s to coverfunding's documented up-to-5-minute worst case. GET /system/status's live credit-balance check adds one outbound HTTPS call (to NinjaPear) to every load of that endpoint whenever a key is configured - guarded so a failure there can never break the rest of system status (returnsnullfor the balance, logs a warning), but it does mean the Settings page's load time now has a dependency onnubela.co's availability/latency for that one field, not just the app's own DB/Redis health.
Live logging, API-key visibility, and enrichment-tab formatting (Phase 17)
- Localhost detection (
_is_localhostinapps/api/app/api/v1/system.py) does not work as intended under Docker Desktop for Windows/Mac. It checksrequest.client.host in ("127.0.0.1", "::1")- correct and sufficient for a bare-metal/non-Dockerized deployment, or Docker on native Linux withnetwork_mode: host. But under Docker Desktop's default networking (this project's actualdocker compose upsetup on Windows), every published-port connection - whether it originates from the host machine itself (curl localhost:8000) or from another machine on the LAN - gets NATed throughdocker-proxyand arrives at the API container sourced from the bridge network's gateway address (observed live:172.18.0.1), never from a literal loopback address. Live-verified: acurl http://localhost:8000/api/v1/system/statusrun directly on the Windows host still reportsis_localhost: false. This was a deliberate fail-closed choice once discovered mid-implementation, not an oversight left unfixed: the alternative (treating the bridge gateway IP as "trusted local") would be actively insecure, since a genuine LAN neighbor hitting the same published port is indistinguishable from the host machine at that same gateway IP - Docker's NAT erases the distinction this feature exists to draw. (The Settings page's old admin-only API Keys box, gated on this check alone, has since been replaced by per-user API keys plus an admin-only-but-not-localhost-gated "Server secrets" box - see the addendum on that below - so this finding's practical impact today is narrower:ADDITIONAL_TRUSTED_LOCAL_IPS, see the Phase 19 addendum, is the intended opt-in fix for genuinely-local dev traffic that needs to look local.) - NinjaPear competitor/customer
namefields are provider-supplied and inconsistent in shape - sometimes a bare URL (https://paypal.com), sometimes already a clean name.companyNameFromUrl(apps/web/lib/format.ts) does a best-effort strip-and-title-case on anything URL-shaped; there's no verification the derived label (e.g. "Staxpayments" fromstaxpayments.com) matches the company's actual public branding (e.g. "Stax Payments") - it's a formatting heuristic, not a lookup against real company names. - The Redis-backed log capture (
apps/api/app/core/logging.py,app:logslist, capped at 500) is best-effort and ungated by user/tenant - any authenticated user of this single-tenant app can see every log line the whole application produced (across all companies, all users, if this app were ever extended to multi-tenant), including other users' company names and error details in thecontextfield (secrets are redacted by_redact_secrets, but business data isn't). Fine for this app's current single-operator scope; would need per-tenant log scoping before this pattern could ship in a real multi-user product. It's also a plain capped list with no admin-facing way to clear/filter it beyond what's newest, and a Redis flush (FLUSHDB/restart) silently empties the whole feed with no persistence. - The four API-key-status categories (
internal_error/api_error/important/normal) are derived purely from structlog level (error/critical→ red,warning→ yellow,infowith an explicitimportant=Truekwarg → blue, else white) - this is a coarse mapping, not a semantic classification. Alogger.warning(...)call that isn't actually about an external API failure (there are a few in this codebase that aren't) still shows up labeled "API error"; nothing currently opts intoimportant=True, so the blue category has no real entries yet until a call site is deliberately updated to pass it.
Per-connection auth: loopback stays free, everyone else logs in (Phase 18)
- Under Docker Desktop's networking, the developer also sees the login screen.
get_current_user(apps/api/app/auth/dependencies.py) now only hands out the fixed "Local Developer" account when bothAUTH_MODE=localand the request's actual TCP peer is literal loopback (app.core.security.is_localhost,127.0.0.1/::1). Per the Phase 17 finding this same check was built for (API-key visibility), Docker Desktop's port-publishing NATs every request - including the host machine's own browser hittinglocalhost:3000via the Docker-mapped port - through the bridge gateway IP, never literal loopback. That means running this app viadocker compose(the documented setup) requires a real registered account for everyone, including the person running the stack, not just LAN/WAN visitors. This is the correct, secure outcome (the alternative - trusting the Docker bridge IP as "local" - would let a LAN neighbor bypass login too, since they'd be indistinguishable from the host at that IP) but is worth knowing going in: the fastest way to get the zero-login convenience back for local development is running the API directly (uvicorn app.main:app, bypassing Docker's NAT hop entirely) rather than throughdocker compose. - Accounts are fully data-isolated per user (existing behavior, unchanged by this phase - every
Company,NotificationDestination, etc. has always belonged to auser_id) - a new WAN/LAN visitor who registers starts with zero companies, not a copy of the "Local Developer" account's existing demo data (Stripe, Nvidia, the Acme Mobility fixture, etc.). There's no account-linking or data-migration path from the local-dev user to a real account. - The password-strength meter on the register page is a purely client-side visual affordance, not a second source of truth - it scores length/case/symbol variety heuristically to give live feedback, but the actual enforced policy is still just the existing Zod schema (10+ characters, at least one letter and one digit) mirrored server-side by
RegisterRequest's validators. A password can show as "Fair" or "Good" on the meter and still be the minimum-viable accepted password; the meter never blocks or requires more than the schema does.
Email verification, escalating lockout/ban, Turnstile, split logging (Phase 19)
get_client_ip'strusted_proxy_ip_headermust be configured correctly before this app ever sits behind Cloudflare (or any reverse proxy), or IP-based throttling/banning breaks in one of two ways. Today (TRUSTED_PROXY_IP_HEADER=unset), every consumer -ip_throttle_service,is_localhost, the unban-request cooldown - readsrequest.client.hostdirectly, which is correct only when nothing sits between the client and this app. Once deployed behind Cloudflare's proxy, every request'srequest.client.hostbecomes Cloudflare's edge IP, not the real visitor's - unset, this either bans/throttles everyone behind Cloudflare together as if they were one IP (one abusive visitor locks out every legitimate one), or - if Cloudflare's IP itself gets banned - locks out the entire app. SettingTRUSTED_PROXY_IP_HEADER=CF-Connecting-IPfixes this by reading the real visitor IP Cloudflare forwards - but this header must only be trusted once it's actually known the proxy is in front (never trust a client-suppliable header blindly); this is a deploy-time config change, not something the code can safely auto-detect.- A real production bug was found and fixed during this phase, not just a test artifact: the original implementation called
ip_throttle_service.record_attempt(..., RESEND_VERIFICATION, ...)unconditionally on everyPOST /auth/register, intending to start the "first resend allowed in 30s" cooldown immediately. Since IP-level throttling is deliberately IP-scoped (not account-scoped, so one abusive account can't be worked around by re-registering), this meant every registration from a shared IP counted against the same ladder - an office, a NAT'd household, or (as caught by the backend test suite sharing one fake IP across ~200 tests) any high-volume signup source would eventually exhaust the resend ladder, escalate through all 6 timeouts, and permanently ban that shared IP, taking down registration/login/reset for everyone behind it. Fixed by removing that call entirely: the escalation ladder forresend_verificationnow only starts from the first manual resend click (POST /auth/resend-verification), and "first resend allowed in 30s" is enforced client-side only (a UI cooldown) for the very first send, not backend-enforced. This is a real, intentional deviation from the original plan's literal wording ("first resend should be allowed in 30 seconds" was originally read as governing the very first send; it now governs resend #2 onward). - IP-level throttling/banning is IP-scoped by design, which has the flip side of the bug above: a genuinely malicious actor sharing a NAT/VPN egress IP with legitimate users can still get that whole shared IP banned through repeated failed logins or resend/reset spam against real accounts -
record_attemptonfailed_login/resend_resetis still called for every attempt, since those actions can't skip IP-level protection without losing the abuse-prevention this feature exists for.TRUSTED_PROXY_IP_HEADER(see above) narrows the blast radius once real per-visitor IPs are available; a corporate/university NAT with no such header will always share fate at this layer, and the manual/unban-requestflow is the intended (if annoying) escape hatch. - Account lockout and email verification are both keyed by account (
User.email_verified/User.locked_at), not IP - a locked or unverified account stays locked/unverified from every IP until the user resets their password or verifies their email, regardless of which IP most recently triggered the lockout. Only the throttle/ban layer (ip_throttle_state/ip_bans) is IP-scoped; these are two independent dimensions that happen to escalate in lockstep for the specific case ofLOGIN_BACKOFF_SECONDS/LOGIN_LOCKOUT_THRESHOLD(both derived from the same 11-stage array, seeauth_service.py), but a determined attacker rotating IPs against one specific account is still stopped by the account-level lock, not just the (bypassable-by-IP-rotation) IP throttle. - Security transactional email (verification codes, reset codes, lockout notices) picks the Resend HTTP API when
RESEND_API_KEYis set, else falls back to the SMTP provider (security_email_service.py) - a misconfigured/expiredRESEND_API_KEYin production silently degrades to attempting SMTP instead (which will itself fail loudly ifSMTP_HOSTisn't configured either) rather than surfacing a specific "Resend is broken" signal anywhere in the UI - failures are logged (get_logger) but not otherwise surfaced to the registering/resetting user beyond the generic success message the enumeration-safe endpoints always return. - The bundled local Mailpit container was removed (originally added in Phase 8/9 as a zero-config local mail sink for alert email, and reused by Phase 19's security email as the no-Resend-account fallback) - the project's real
.envhad already been pointed at Resend's own SMTP relay for both alerts and security email, making Mailpit dead weight in practice (never actually reached by the running app; the ~500 stray messages found in it during Phase 19 testing turned out to be an unrelated side effect of running the localpytestsuite outside Docker, which defaultsSMTP_HOSTtolocalhostand happened to reach Mailpit's host-published port). Net effect: there is no bundled way to test real email delivery locally anymore without a real SMTP relay or a real Resend account -.env.example's SMTP block now ships blank rather than pointing at a fakemailpithostname. The unban-request admin notification (unban_service.py) also moved off a fixedadmin_notification_emailsetting onto every account withis_admin=True(UserRepository.list_admin_emails) - multiple admins now all get notified, and the previous fake[email protected]default (which only ever worked because Mailpit intercepted it) is gone. - Turnstile enforcement could not be live-verified by this agent - Cloudflare Turnstile is explicitly designed to be unsolvable by automation, and this dev environment (Docker) means even the agent's own browser-tool traffic isn't loopback (see the Phase 17/18 Docker-NAT finding above), so the widget is always presented once a
turnstile_secretis configured (via.envor the admin-set override, see the system-secrets addendum below). Full live verification (register/login/forgot-password through the actual widget, plus a real Resend-delivered code if desired) requires a human to solve the challenge - tracked as the final Phase 19 verification step. - The public
POST /unban-requestsendpoint has no Turnstile/CAPTCHA of its own - only a flat3/minuterate limit (slowapi) and a 24-hour per-IP cooldown enforced at the DB level (UnbanRequestRepository.within_cooldown). An IP that is not yet banned could still be used to spam the admin's inbox up to the rate limit before the cooldown kicks in on the second request; low-impact (each request is a single email + one log line, and the cooldown caps sustained abuse to one message per IP per day) but a deliberate scope decision, not an oversight - adding Turnstile here would require serving the challenge to a possibly-already-banned visitor, which is awkward given Turnstile itself is skipped for banned/localhost callers elsewhere in this app's model. GET /auth/security-eventsis capped at the 100 most recent events per user (UserSecurityEventRepository.list_for_user, hardcodedlimit=100) with no pagination - a long-lived, frequently-logged-in account will eventually have older events silently fall off the visible list. No admin-facing purge/retention policy exists for this table either (unlikeSourceDocument'sDATA_RETENTION_DAYS, see the Phase 10 section above) - it grows unboundedly in the DB even though only the newest 100 rows are ever shown.IpBan.reasonis a short fixed string naming which action type triggered the ban (resend_verification,resend_reset,failed_login, ormanual_admin_banfor an admin-initiated ban - see below) rather than a detailed forensic record (no timestamp history of the individual attempts that led to it, no association with which account(s) were being targeted) - sufficient for the admin Settings-page IP Bans panel to show why, not a full audit trail. Theuser_security_eventstable is the closer thing to an audit trail, but it's scoped per-account, not per-IP, so correlating "this IP got banned" with "these were the accounts it was hammering" requires manually cross-referencingip_addressacross both tables today.- Admins can now ban an IP directly (
POST /admin/ip-bans, the Settings page's "Ban an IP manually" field) - bypasses the offense-count escalation ladder entirely (ip_throttle_service), a deliberate manual override rather than something the automated abuse-detection path produces. Rejects an already-banned IP with 409 rather than silently no-oping, and validates the address is a real IPv4/IPv6 literal (422 otherwise) - it does not accept CIDR ranges or hostnames. - Unban requests now have real Accept/Reject actions (
POST /admin/unban-requests/{id}/accept,DELETE /admin/unban-requests/{id}) instead of being a read-only queue an admin had to separately go find the IP in the bans list to act on. Accept unbans the IP (full pardon - clears throttle state too, not just the ban row) and removes the request; Reject removes the request without touching the ban, so the IP stays blocked. Either way the request is gone from the pending list afterward - there's no "resolved but kept for history" state, so once acted on, a request leaves no trace of the decision beyond whatever happened to the ban/throttle rows themselves. ADDITIONAL_TRUSTED_LOCAL_IPS(app.core.security.is_localhost) is a narrow, opt-in escape hatch for the Docker-NAT findings above - a comma-separated list of extra IPs treated as loopback-equivalent, on top of real127.0.0.1/::1. Added because Cloudflare Turnstile crashes both the Browser pane and the Claude Code process itself when it renders (observed live; the widget is designed to be unsolvable by automation, and this failure mode is worse than a stuck challenge), so an agent's own browser-tool traffic againstlocalhost:3000/8000needsis_localhostto actually resolve true to avoid ever rendering it - matching how a real developer running the stack viadocker composewould want Turnstile skipped too. Left empty by default (strict). This project's.envsets it to the observed Docker bridge gateway IP (172.18.0.1) for local dev. This must never be set in a real deployment - unlikeTRUSTED_PROXY_IP_HEADER(which reads a header a trusted proxy controls), this is a flat IP allowlist; if that IP is ever reachable by anyone other than the actual host machine (e.g. a misconfigured network, or a bridge subnet shared with untrusted containers), it would incorrectly grant them the same loopback-only privileges (Turnstile bypass, and - ifAUTH_MODE=local- the fixed local-dev-user auto-login too, see the Phase 18 section above).
Admin-managed server secrets: Turnstile site key/secret move off .env
- The old admin-only, localhost-only "API keys" Settings box (
/system/api-keys, Phase 17) has been removed entirely, replaced by two independent mechanisms with a deliberately different visibility model each: per-user API keys (Anthropic/Brave/NinjaPear/USPTO - every user manages their own, no admin/localhost gate at all, since each user only ever sees their own value) and a new admin-only (but not localhost-gated) "Server secrets" box for values that are genuinely global rather than per-user - today just the Cloudflare Turnstile site key and secret.SystemSecret(app/models/system_secret.py) is a true singleton-per-key table (one row perSystemSecretKey, not scoped to any user), encrypted at rest the same way asUserApiKey, with the identical fallback-to-.env-when-unset pattern viasystem_secret_service.get_effective_settings. - The Turnstile site key is no longer delivered to the frontend via a
NEXT_PUBLIC_*build-time env var. It used to be baked into the Next.js bundle at container-build time (NEXT_PUBLIC_TURNSTILE_SITE_KEY), which meant an admin-updated value could never take effect without a full frontend rebuild - defeating the point of making it admin-editable. It's now served live by the already-public, unauthenticatedGET /system/statusendpoint (turnstile_site_key, resolved through the same effective-settings fallback), andTurnstileWidgettakes it as a prop instead of readingprocess.envdirectly. This does mean the site key round-trips through one more network hop (an extra field on a call the login/register pages already make) rather than being inlined - a negligible cost for a value that isn't itself secret. system_secretsrows are true global singletons with no natural per-test isolation (unlikeuser_api_keys, which is naturally isolated by a randomized per-testuser_id) -test_system_secrets.pyhas to explicitly clear the table before and after every test via an autouse fixture to avoid cross-test pollution within a full-suite run. Worth remembering if this pattern is extended to more server-wide keys later: any new key added toSystemSecretKeyinherits the same test-isolation requirement.- The real
.envTURNSTILE_SITE_KEY/TURNSTILE_SECRET/RESEND_API_KEYvalues on the production deployment have since been moved into this DB-backed storage via the admin API and removed from.envoutright (all three nowconfigured: true, set throughPUT /system/secrets/{key}using the deployment's own admin account rather than hand-edited into.env) - the earlier state of this note (values copied into the DB but also left in.env"to purge later") has been resolved;.envis no longer a source of truth for these three at all on that deployment.NEXT_PUBLIC_TURNSTILE_SITE_KEYspecifically was already removed from.env/.env.example/docker-compose.ymlearlier, since nothing reads it anymore (see above).
Account activity logging expansion: local-dev sign-ins, secret/key updates, known IPs
- The local-dev bypass (
AUTH_MODE=local+ loopback) now logs alogin_successevent too, even though it has no real login step -get_or_create_local_user(app/services/auth_service.py) runs on every authenticated request for that account, so logging unconditionally would flood Account activity with one event per request. A 30-minute cooldown (LOCAL_DEV_LOGIN_LOG_COOLDOWN_MINUTES) treats "no login_success logged for this account in the last 30 minutes" as a proxy for "a new session," rather than trying to detect real session boundaries that don't exist in this bypass. This is a heuristic, not a precise signal - reopening the app twice within 30 minutes shows one sign-in, not two, and an idle tab left open for hours making periodic background requests wouldn't show repeated sign-ins either unless a genuine 30+ minute gap occurs. - Updating a Server secret (admin) or one of your own per-user API keys now writes a
server_secret_updated/api_key_updatedevent to Account activity (system_secret_service.set_secret/user_api_key_service.set_key, both now require the acting user's id and IP). Deliberately logged under the acting user's own account either way - for Server secrets that's whichever admin made the change, not some app-wide "system" pseudo-account - so an admin's Account activity is also a partial audit trail of their own admin actions. Clearing a key/secret (setting it blank) is logged identically to setting a real value - the event type doesn't distinguish "set" from "cleared," only the row's ownconfiguredstate (visible via the Settings page, not the event log itself) tells you which happened. - New
user_known_ipstable captures every distinct IP an account has signed in from (app/models/user_known_ip.py), one row per(user_id, ip_address)pair withfirst_seen_at/last_seen_at, updated on every recorded sign-in (both real login and the local-dev bypass, gated by the same cooldown above for the latter). This is pure data capture for now - nothing currently reads this table or surfaces it anywhere in the UI; it exists as the foundation for a possible future "new device/location" security feature, per explicit request.UserKnownIpRepository.record_login's return value (whether the IP was new) is already threaded through but currently unused by any caller. - IPs were already being recorded per-login inside
user_security_events(everylogin_success/login_failedrow has always carriedip_address) -user_known_ipsdoesn't replace that, it's a deliberately separate, deduplicated view: the security-events log is an append-only history of every attempt, whileuser_known_ipsanswers "what's the current set of IPs this account has ever used" without needing to scan and dedupe the (much larger, unbounded-growth - see the Phase 19 addendum above) events table.
Settings -> Database viewer (Adminer, local-dev/admin only)
- This feature deliberately does not reuse the app's own JWT/admin session to gate access, because it can't. The frontend's access token lives only in
window.localStorage(apps/web/lib/api-client.ts), never a cookie, and is attached only as a JS-constructedAuthorizationheader on this app's ownfetch()calls - a plain browser navigation to a different subdomain (db.ciagent.org) carries none of that. Instead,apps/api/app/api/v1/db_viewer.pymints a short-lived bootstrap token from a live,require_admin-gated session, which Nginx'sdb.ciagent.orgblock exchanges for an independent, cookie-based session scoped to that subdomain only. Both token types reuse the sameJWT_SECRET/TokenTypemachinery as real login tokens (apps/api/app/core/security.py) rather than introducing a second signing secret. - The
verifyendpoint re-loads the user and re-checksis_adminfrom the database on every request (not just at token-mint time), so revoking someone's admin flag takes effect on their very next request through Nginx'sauth_request- but a DB_VIEWER_SESSION cookie already issued is otherwise valid for its full 60-minute lifetime; there's no server-side session revocation list, only the liveis_admincheck and natural expiry. - Local dev's Adminer (
docker-compose.yml, port127.0.0.1:8081) has no authentication of its own at all - it relies entirely on the port being bound to loopback only, matching this app's existing "loopback is inherently trusted" philosophy elsewhere (e.g. the local-dev auth bypass itself). Anyone who can reachlocalhost:8081on that machine - including another local user account on a shared machine - has full Postgres access with no further gate. - Adminer itself has no read-only mode - the feature was explicitly requested as "view and edit," so there's no additional restriction at the Adminer-config layer beyond Nginx's session gate (production) or loopback binding (local dev). The real Postgres username/password, required by Adminer's own login form, is the only remaining layer once past those.
Further limitations are appended per-phase below.