Skip to content

Fix broken workflows, close security holes, demo-readiness polish - #4

Merged
rogerdemello merged 5 commits into
mainfrom
fix/workflow-bugs
Aug 19, 2026
Merged

Fix broken workflows, close security holes, demo-readiness polish#4
rogerdemello merged 5 commits into
mainfrom
fix/workflow-bugs

Conversation

@rogerdemello

Copy link
Copy Markdown
Owner

Summary

End-to-end testing of every API workflow surfaced 7 broken flows and 3 security holes. This PR fixes all of them, cleans lint to zero (now enforced), adds regression tests, and documents the architecture and a demo script.

Broken workflows fixed

  • PDF export 500d every time — Puppeteer 21 with headless: true looks for chrome-headless-shell; use headless: new (Render/Docker unaffected)
  • Salesforce demo mode could connect but never sync — demo-mode sync now imports canned deals, so connect → sync → analyze is fully demonstrable
  • Every OAuth reconnect created a duplicate integration.single() errors on >1 rows, defeating the upsert; use order/limit/maybeSingle
  • Integrations that ever synced could not be deleted — SyncLog children removed first
  • Zero-new-records sync logged as FAILURE — only real errors fail now
  • npm run seed crashed on an undefined variable (documented setup step)
  • Semantic search unavailablesemantic_search.sql now self-contained (adds company_id it references); new storage_bucket.sql for document upload

Security

  • /api/users returned bcrypt password hashes to unauthenticated callers and allowed anyone to create/promote ADMINs — now authed, sanitized, admin-gated, with regression tests
  • /api/files and /api/templates now require auth
  • CREDENTIALS.md (shared default password) removed from the repo — rotate anything ever committed
  • Server prefers SUPABASE_SERVICE_ROLE_KEY when set, ready for RLS

Quality & docs

  • 34 lint warnings → 0, with --max-warnings 0 so CI holds the line
  • New tests: 55 passing (was 51); typecheck clean; production build clean
  • ARCHITECTURE.md (system diagram + trade-offs), docs/DEMO_SCRIPT.md, README setup now matches reality (db push + manual SQL, not migrate deploy)

Verification

Every flow re-tested live after the fixes: create → analyze (Azure OpenAI) → status change → PDF export (200, real PDF) → semantic search (ranked results) → demo CRM sync (3 deals imported, idempotent re-sync) → users endpoint (no hashes).

Post-merge checklist

  • Run npx prisma db execute --file prisma/manual/storage_bucket.sql once (enables document upload)
  • Rotate Supabase/Azure/Google/HubSpot keys that were ever committed
  • Set PRODUCTION_URL/FRONTEND_URL in the Render dashboard

Roger Demello added 5 commits August 17, 2026 10:25
- PDF export: use headless new — headless: true makes Puppeteer 21 look
  for chrome-headless-shell, which is never installed, so every export 500d
- OAuth callbacks: existing-integration lookups used .single(), which errors
  once duplicates exist, so every reconnect inserted another copy; use
  order/limit/maybeSingle instead
- Integration sync: demo-mode connections (credentials.demo) now import
  canned deals so connect->sync->analyze is demonstrable without real CRM
  credentials; previously demo Salesforce could connect but never sync
- Integration sync: a run with zero new records no longer logs as FAILURE
- Integration delete: remove SyncLog children first (no DB cascade), so
  integrations that have synced can actually be deleted
- seed.ts: define demoUser — the documented `npm run seed` step crashed on a
  ReferenceError
- semantic_search.sql: add the company_id column the match_proposals
  function references (live DBs predate company scoping); new
  storage_bucket.sql sets up the proposal-files bucket + policies
- scripts/cleanup-integrations.ts: one-off dedupe + starter templates
- /api/users returned bcrypt password hashes to unauthenticated callers and
  let anyone create or promote ADMIN users; responses now strip password and
  mutations are admin-only, with regression tests
- /api/files and /api/templates now require auth
- Supabase server client prefers SUPABASE_SERVICE_ROLE_KEY when set (keeps
  the API working once RLS is enabled); documented in .env.example and
  render.yaml
- remove CREDENTIALS.md from the repo (was committed with a shared default
  password; rotate any credentials that were ever committed)
- fix every unused-import/var, useless-escape and useless-catch warning
- real fixes for react-hooks/exhaustive-deps: memoize checkConnection,
  convert hasShownOfflineWarning to a ref, justify the two intentional
  navigation-scoped effects inline
- scope react-refresh/only-export-components off for shadcn/ui generated
  components (they export variants alongside components by design)
- lint script now fails on any warning so CI keeps the bar
- ARCHITECTURE.md: system diagram and the reasoning behind the split
  Prisma/PostgREST data access, demo-first auth, AI guardrails, pgvector
  fallback, and the Render/Docker deployment
- docs/DEMO_SCRIPT.md: rehearsed 7-minute storyline with a Q&A cheat sheet
- README: setup now matches reality (db push + the two manual SQL files;
  there are no migration files for migrate deploy to apply)
- schema.prisma: declare Proposal.company_id so db push does not drop the
  column the API and match_proposals rely on
Copilot AI lite review requested due to automatic review settings August 19, 2026 17:21

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@rogerdemello
rogerdemello merged commit 722fbcd into main Aug 19, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants