Fix agent chat 500, backfill missing cloud migrations, fix offline app schema drift#89
Merged
Merged
Conversation
apiFetch/baseFetchJson discarded the JSON error body on any non-2xx
response and threw a bare "Failed to fetch ... : 500" — the actual
reason (which every server route already returns as { error: "..." })
never reached the user or the console. This has been masking the real
cause of several bugs investigated in this session. Now the thrown
Error carries the server's actual message (falling back to the generic
one if the body isn't JSON), with the status code attached.
Also fixed two real bugs found while chasing a 500 on
POST /api/agents/:id/chat: buildAgentContext ordered the `projects`
scope by a `created_at` column that doesn't exist on that table (only
`updated_at` does), which 400'd on every single call — silently, since
none of the context fetches checked r.error before assigning `r.data ||
[]`. Added error logging to all of them so a broken scope is visible in
server logs instead of just silently degrading to an empty list.
Note: the `meetings` scope query still fails — the `meetings` table
does not exist in the production database at all, despite being a
documented table and having a full CRUD API in server.ts. That's a
separate, larger issue (a whole feature backed by a missing table) that
needs a decision on whether to create the table or retire the feature.
The meetings table never existed in production despite a full CRUD API in server.ts (/api/meetings*) and a documented Meeting type — every call 404'd. Columns match exactly what server.ts reads/writes: type constrained to the three MeetingType values, project_id/proposal_id/ tender_id as nullable TEXT FKs (matching those tables' TEXT ids), plus meeting_photos and meeting_attendees for the endpoints that join on them. RLS enabled with the same tenant_isolation policy pattern used everywhere else in schema.sql. Applied directly to the production Supabase project; this file commits it to the migration history for anyone provisioning a new instance.
Two more tables/columns referenced in server.ts were missing from
production, found via a full audit of every .from('...') call against
the actual deployed schema:
- join_requests: existed only in schema.sql/migrate_add_join_requests.sql,
never applied — broke the whole "request to join an existing agency"
flow (/api/agency-setup/join, /api/team/join-requests*).
- document_diffusions + 6 documents columns (indice, doc_statut,
emetteur, approbateur, date_approbation, doc_type) from
migrate_documents_qualite.sql — also never applied, breaking
/api/documents/:id/diffusions*. That migration's own source also
never enabled RLS on the new table (every other table in this schema
has a tenant_isolation policy) — added it here.
Applied both directly to the production Supabase project.
Investigated whether the Windows desktop build (its own embedded Postgres, bootstrapped by electron/pgBootstrap.cjs) suffers the same class of schema drift just fixed on the cloud Supabase project. It does, structurally: applyLocalSchema() only ever ran on a database's very first launch (isFirstRun, gated on pgdata/ not existing yet) — any migrate_*.sql file added to the repo after a user's first launch would never be applied to their existing local install, since there was no other trigger for it. New cloud migrations (meetings, join_requests, etc.) would silently never reach already-installed Windows users, forever. - electron/applySchema.cjs: track applied files in a new _local_applied_migrations bookkeeping table (same local-only pattern as server/localPendingPush.ts) and only apply files not yet recorded. Runs the same on a brand new database (applies everything) and an existing one (applies just what's new). - electron/pgBootstrap.cjs: call it on every launch, not just the first. Critically, the existing "delete pgdata/ and retry" failure path is now scoped to isFirstRun only — a migration failure on an existing, populated install must never wipe the user's real data; it now just fails to start and leaves the local DB untouched. - supabase/migrate_add_agents.sql: its 13-row system-template seed INSERT relied on ON CONFLICT DO NOTHING against UNIQUE(tenant_id, slug), but tenant_id IS NULL on every seed row and Postgres never considers two NULLs equal for a unique constraint — confirmed empirically against production (a duplicate insert was silently accepted, not rejected). Every existing offline install would have replayed this file once under the new tracking and ended up with 26 duplicate template agents. Now guarded with WHERE NOT EXISTS so the whole seed is skipped once any system template already exists.
The previous version re-threw (and even stopped Postgres) when applying pending migrations failed on an already-initialized local install — that propagates through startOfflineDataStack() -> startServer() -> createWindow() in electron/main.cjs, which shows a hard error screen instead of loading the app at all. That's wrong for an offline-first app: a transient failure (no network, disk hiccup, whatever) while catching up on new migrations must never block access to an existing, already-working local database. Now that failure is swallowed: Postgres keeps running untouched at whatever schema it already has, the app starts normally, and the next launch retries automatically (unapplied migrations simply stay unrecorded in _local_applied_migrations, same as before).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
apiFetch/baseFetchJsondiscarded the server's JSON error body on any failure and threw a bare"Failed to fetch ... 500"— the actual reason (which every server route already returns) never reached the user or console. Now the real message surfaces, with the status code attached.buildAgentContextordered theprojectsscope by acreated_atcolumn that doesn't exist on that table (onlyupdated_atdoes), 400'ing silently on every agent chat call since none of the context fetches checked the error before defaulting to an empty list. Added logging to all of them.server.tsagainst the actual production schema: Chorus Pro (settings/invoices/situations/marches_entreprises),meetings/meeting_photos/meeting_attendees(a whole feature backed by a table that didn't exist),join_requests,documentsqualité columns +document_diffusions,tenantsStancer billing columns,ordres_de_serviceMPRO columns,receptionsMPRO columns, andsituationsSuper PDP columns. Re-ran the full audit afterward — comes back clean.migrate_*.sqlfiles on a database's very first launch — any migration added to the repo after a user's first install would never reach their existing local database. Added a_local_applied_migrationsbookkeeping table so new migrations get picked up incrementally on every launch, and made a migration-catch-up failure on an existing install non-fatal (never blocks app startup, never deletes user data, retries automatically next launch) instead of the previous behavior of stopping Postgres and refusing to start.migrate_add_agents.sql's 13-row system-template seed relied onON CONFLICT DO NOTHING, buttenant_id IS NULLon every row and Postgres never treats two NULLs as conflicting for a unique constraint (verified empirically against production in a rolled-back transaction) — every existing offline install would have ended up with 26 duplicate template agents. Now guarded withWHERE NOT EXISTS.Test plan
npm run lint(tsc --noEmit) passesmigrate_add_agents.sqlidempotency fix against production in a rolled-back transaction (count stayed at 13, not 26)Generated by Claude Code