diff --git a/DECISIONS.md b/DECISIONS.md index 7e9d6f0b..b0cc7fdc 100644 --- a/DECISIONS.md +++ b/DECISIONS.md @@ -1520,4 +1520,5 @@ One line per decision: `[YYYY-MM-DD] : `. Appended by agents and [2026-09-02] Stock GoTrue email-change redirects detected via a flow=email_change marker on emailRedirectTo rather than by sniffing ?message= / ?code= on every callback: GoTrue's PKCE redirect carries no type, and the same marker rides along on hook-built token_hash links, so one flag covers both link styles without touching signup/recovery/OAuth paths. [2026-09-02] Sign-off refusals: registered the ReconciliationSignoffError codes in structured-errors with a new thrown_message_sv flag instead of returning err.message from the routes: check:guards forbids raw caught-error messages in user-visible sinks, and the registry keeps the codes discoverable for agents while the dialog still gets the runtime text (dates, amounts). [2026-09-02] Nyckeltal "Resultat per månad" shows the exact per-month figures as an always-on list under the bars (#2198), not behind an "Anpassa" toggle: a preference would touch the type, the PUT schema, the strict preferences-body validator, the dialog and its tests for a switch nobody turns off. Per-bar compact labels are conditional on a glyph-width fit rule and fall back to the single latest label, so they never overlap. Left alone: the monthly path counts only posted entries while the year-total path also counts reversed originals (pinned as intended in tests/pg/kpi-report-aggregates-rpc.pg.test.ts), so a same-year storno makes the sum of months differ from Nettoresultat; visible as numbers now, founder call whether to align the two. +[2026-09-03] Kundorder PGRST201 fixed by hinting the three sales_order_items embeds AND by teaching scripts/checks/ambiguous-embed.mjs to parse composite FOREIGN KEY (a, b) constraints, instead of a bespoke source-scan test: the guard is the sanctioned owner of this class (decision 2026-09-01) and it missed the pair only because its parser was single-column; with the fix it derives the same 17 pairs prod reports and flags all three shipped sites on main. [2026-09-03] Email-change double-submit gate is a dedicated per-user claim table + SECURITY DEFINER RPC (migration 20260903083000), not idempotency_keys and not an advisory lock: idempotency_keys requires a company_id the account-level route does not have, and a transaction-scoped advisory lock cannot cover the GoTrue call that happens outside the transaction. diff --git a/app/api/sales-orders/route.ts b/app/api/sales-orders/route.ts index c7c4897a..602b7732 100644 --- a/app/api/sales-orders/route.ts +++ b/app/api/sales-orders/route.ts @@ -29,7 +29,7 @@ export const GET = withRouteContext('sales_order.list', async (request, { supaba orders = await fetchAllRows(({ from, to }) => { let qb = supabase .from('sales_orders') - .select('*, customer:customers(id, name, customer_number, customer_type), items:sales_order_items(*)') + .select('*, customer:customers(id, name, customer_number, customer_type), items:sales_order_items!sales_order_items_sales_order_id_fkey(*)') .eq('company_id', companyId) if (status) qb = qb.eq('status', status) if (customer_id) qb = qb.eq('customer_id', customer_id) diff --git a/extensions/general/mcp-server/server.ts b/extensions/general/mcp-server/server.ts index 970afe0a..3fa9ceae 100644 --- a/extensions/general/mcp-server/server.ts +++ b/extensions/general/mcp-server/server.ts @@ -7060,7 +7060,7 @@ export const tools: McpTool[] = [ let query = supabase .from('sales_orders') .select( - 'id, order_number, status, customer_id, order_date, requested_delivery_date, last_delivery_date, currency, subtotal, vat_amount, total, customer:customers(name), items:sales_order_items(id, line_type, quantity, delivered_qty, sort_order)', + 'id, order_number, status, customer_id, order_date, requested_delivery_date, last_delivery_date, currency, subtotal, vat_amount, total, customer:customers(name), items:sales_order_items!sales_order_items_sales_order_id_fkey(id, line_type, quantity, delivered_qty, sort_order)', { count: 'exact' }, ) .eq('company_id', companyId) diff --git a/lib/sales-orders/load.ts b/lib/sales-orders/load.ts index 9fed45a5..f8178d71 100644 --- a/lib/sales-orders/load.ts +++ b/lib/sales-orders/load.ts @@ -35,7 +35,7 @@ export async function loadSalesOrder( ): Promise> { const { data, error } = await supabase .from('sales_orders') - .select('*, customer:customers(*), items:sales_order_items(*)') + .select('*, customer:customers(*), items:sales_order_items!sales_order_items_sales_order_id_fkey(*)') .eq('id', orderId) .eq('company_id', companyId) .maybeSingle() diff --git a/scripts/checks/__tests__/ambiguous-embed.test.ts b/scripts/checks/__tests__/ambiguous-embed.test.ts index 2fc64ec5..44d6352b 100644 --- a/scripts/checks/__tests__/ambiguous-embed.test.ts +++ b/scripts/checks/__tests__/ambiguous-embed.test.ts @@ -245,15 +245,119 @@ describe('ambiguous-embed: pair derivation from the migration history', () => { ).toBe(0) }) + it('counts a composite foreign key next to a single-column one', () => { + // 20260902130000_sales_orders.sql + 20260902180000_sales_orders_hardening.sql: + // the composite (sales_order_id, company_id) guard is a second relationship + // in PostgREST's eyes. A single-column-only parser derived one edge here and + // let `items:sales_order_items(*)` ship un-hinted; prod answered PGRST201 + // on every kundorder load (2026-09-03). + const dir = migrationsRoot({ + '20260902130000_sales_orders.sql': ` + CREATE TABLE public.sales_order_items ( + id uuid PRIMARY KEY, + company_id uuid NOT NULL REFERENCES public.companies(id) ON DELETE CASCADE, + sales_order_id uuid NOT NULL REFERENCES public.sales_orders(id) ON DELETE CASCADE + ); + `, + '20260902180000_sales_orders_hardening.sql': ` + ALTER TABLE public.sales_order_items + DROP CONSTRAINT IF EXISTS sales_order_items_order_company_fkey; + ALTER TABLE public.sales_order_items + ADD CONSTRAINT sales_order_items_order_company_fkey + FOREIGN KEY (sales_order_id, company_id) + REFERENCES public.sales_orders (id, company_id) + ON DELETE CASCADE; + `, + }) + expect([...deriveAmbiguousPairs(dir)]).toEqual([pairKey('sales_order_items', 'sales_orders')]) + }) + + it('does not count a composite foreign key that REPLACED the single-column one', () => { + // 20260721101500_harden_tax_assessment_notices.sql shape: the old + // single-column constraint is dropped and the composite added, leaving one + // relationship. Prod agrees: tax_assessment_notices|fiscal_periods is not + // in the pg_constraint list. + const dir = migrationsRoot({ + '1_base.sql': ` + CREATE TABLE public.tax_assessment_notices ( + id uuid PRIMARY KEY, + company_id uuid NOT NULL REFERENCES public.companies(id), + fiscal_period_id uuid REFERENCES public.fiscal_periods(id) + ); + `, + '2_harden.sql': ` + ALTER TABLE public.tax_assessment_notices + DROP CONSTRAINT IF EXISTS tax_assessment_notices_fiscal_period_id_fkey; + ALTER TABLE public.tax_assessment_notices + ADD CONSTRAINT tax_assessment_notices_fiscal_period_company_fkey + FOREIGN KEY (fiscal_period_id, company_id) + REFERENCES public.fiscal_periods (id, company_id); + `, + }) + expect(deriveAmbiguousPairs(dir).size).toBe(0) + }) + + it('drops a composite edge when DROP COLUMN removes one of its columns', () => { + // Postgres drops every foreign key a column takes part in. Keeping the + // composite edge would arm a pair that no longer exists and reject valid + // embeds (CodeRabbit on PR #2207). + const base = ` + CREATE TABLE public.sales_order_items ( + id uuid PRIMARY KEY, + company_id uuid NOT NULL, + sales_order_id uuid NOT NULL REFERENCES public.sales_orders(id), + CONSTRAINT sales_order_items_order_company_fkey + FOREIGN KEY (sales_order_id, company_id) REFERENCES public.sales_orders(id, company_id) + ); + ` + expect([...deriveAmbiguousPairs(migrationsRoot({ '1_a.sql': base }))]).toEqual([ + pairKey('sales_order_items', 'sales_orders'), + ]) + expect( + deriveAmbiguousPairs( + migrationsRoot({ + '1_a.sql': base, + '2_b.sql': `ALTER TABLE public.sales_order_items DROP COLUMN company_id;`, + }), + ).size, + ).toBe(0) + // The dropped constraint's name is released too: a later DROP CONSTRAINT + // by that name must not delete an unrelated edge. + expect( + deriveAmbiguousPairs( + migrationsRoot({ + '1_a.sql': base, + '2_b.sql': `ALTER TABLE public.sales_order_items DROP COLUMN company_id;`, + '3_c.sql': `ALTER TABLE public.sales_order_items DROP CONSTRAINT IF EXISTS sales_order_items_order_company_fkey;`, + }), + ).size, + ).toBe(0) + }) + + it('reads a composite table-level constraint inside CREATE TABLE', () => { + const dir = migrationsRoot({ + '1_a.sql': ` + CREATE TABLE public.a ( + id uuid PRIMARY KEY, + company_id uuid NOT NULL, + b_id uuid REFERENCES public.b(id), + CONSTRAINT a_b_company_fkey FOREIGN KEY (b_id, company_id) REFERENCES public.b(id, company_id) + ); + `, + }) + expect([...deriveAmbiguousPairs(dir)]).toEqual([pairKey('a', 'b')]) + }) + it('matches the live schema on the real migration history', () => { - // Verified against prod (pwxtzglxptnnvjrpixpg) on 2026-09-01 with the - // pg_constraint query in ambiguous-embed.mjs: the same 15 pairs. + // Verified against prod (pwxtzglxptnnvjrpixpg) on 2026-09-03 with the + // pg_constraint query in ambiguous-embed.mjs: the same 17 pairs. const pairs = deriveAmbiguousPairs( path.join(__dirname, '..', '..', '..', 'supabase', 'migrations'), ) expect(pairs.has(pairKey('journal_entries', 'fiscal_periods'))).toBe(true) expect(pairs.has(pairKey('journal_entries', 'salary_runs'))).toBe(true) expect(pairs.has(pairKey('supplier_invoices', 'transactions'))).toBe(true) + expect(pairs.has(pairKey('sales_order_items', 'sales_orders'))).toBe(true) // Single-foreign-key pairs that legitimate code embeds without a hint. expect(pairs.has(pairKey('journal_entries', 'journal_entry_lines'))).toBe(false) expect(pairs.has(pairKey('supplier_invoice_payments', 'supplier_invoices'))).toBe(false) diff --git a/scripts/checks/ambiguous-embed.mjs b/scripts/checks/ambiguous-embed.mjs index f24e8e88..b5f5f817 100644 --- a/scripts/checks/ambiguous-embed.mjs +++ b/scripts/checks/ambiguous-embed.mjs @@ -47,7 +47,12 @@ * where c.contype = 'f' * group by 1, 2 having count(*) > 1 order by 1, 2; * - * On 2026-09-01 that returned the same 15 pairs the parser derives. + * On 2026-09-03 that returned the same 17 pairs the parser derives. Composite + * foreign keys count: until 2026-09-03 the parser only read single-column + * `FOREIGN KEY (col)`, so the composite (sales_order_id, company_id) guard in + * 20260902180000_sales_orders_hardening.sql was invisible to it and the + * un-hinted `items:sales_order_items(*)` embeds shipped, taking every + * kundorder list and detail load down with PGRST201 (fixed 2026-09-03). * * No baseline: the count is 0, any new un-hinted ambiguous embed is a hard * failure. @@ -92,10 +97,16 @@ export function deriveForeignKeys(migrationsDir) { // Unnamed foreign keys get Postgres's default `__fkey`. const byConstraint = new Map() + // `column` is one column or a composite list ("sales_order_id, company_id"). + // A composite foreign key is its own edge, distinct from a single-column one + // on its leading column: PostgREST counts both, which is what made + // sales_order_items -> sales_orders ambiguous (migration 20260902180000) + // while this parser, then single-column only, still derived one edge. const addEdge = (table, column, target, constraintName) => { - const key = `${table}.${column}` + const columns = column.split(',').map((c) => normIdent(c.trim())).filter(Boolean) + const key = `${table}.${columns.join(',')}` edges.set(key, target) - byConstraint.set(constraintName ?? `${table}_${column}_fkey`, key) + byConstraint.set(constraintName ?? `${table}_${columns.join('_')}_fkey`, key) } let files @@ -121,11 +132,12 @@ export function deriveForeignKeys(migrationsDir) { for (const m of stmt.matchAll(/(?:^|,)\s*([\w"]+)\s+[^,()]*?references\s+([\w".]+)/gis)) { addEdge(table, normIdent(m[1]), normIdent(m[2])) } - // `foreign key (col) references public.other(id)` as a table constraint. + // `foreign key (col[, col]) references public.other(id[, id])` as a + // table constraint, named or not. for (const m of stmt.matchAll( - /foreign\s+key\s*\(\s*([\w"]+)\s*\)\s*references\s+([\w".]+)/gi, + /(?:constraint\s+([\w"]+)\s+)?foreign\s+key\s*\(\s*([\w",\s]+?)\s*\)\s*references\s+([\w".]+)/gi, )) { - addEdge(table, normIdent(m[1]), normIdent(m[2])) + addEdge(table, m[2], normIdent(m[3]), m[1] && normIdent(m[1])) } continue } @@ -133,9 +145,9 @@ export function deriveForeignKeys(migrationsDir) { if (altered) { const table = normIdent(altered[1]) for (const m of stmt.matchAll( - /(?:add\s+constraint\s+([\w"]+)\s+)?foreign\s+key\s*\(\s*([\w"]+)\s*\)\s*references\s+([\w".]+)/gi, + /(?:add\s+constraint\s+([\w"]+)\s+)?foreign\s+key\s*\(\s*([\w",\s]+?)\s*\)\s*references\s+([\w".]+)/gi, )) { - addEdge(table, normIdent(m[2]), normIdent(m[3]), m[1] && normIdent(m[1])) + addEdge(table, m[2], normIdent(m[3]), m[1] && normIdent(m[1])) } for (const m of stmt.matchAll( /add\s+column\s+(?:if\s+not\s+exists\s+)?([\w"]+)\s+[^,;]*?references\s+([\w".]+)/gi, @@ -143,7 +155,17 @@ export function deriveForeignKeys(migrationsDir) { addEdge(table, normIdent(m[1]), normIdent(m[2])) } for (const m of stmt.matchAll(/drop\s+column\s+(?:if\s+exists\s+)?([\w"]+)/gi)) { - edges.delete(`${table}.${normIdent(m[1])}`) + // Postgres drops every foreign key the column takes part in, so a + // composite edge listing it goes too, not only the single-column key. + const column = normIdent(m[1]) + for (const key of [...edges.keys()]) { + if (!key.startsWith(`${table}.`)) continue + if (!key.slice(table.length + 1).split(',').includes(column)) continue + edges.delete(key) + for (const [name, edge] of [...byConstraint]) { + if (edge === key) byConstraint.delete(name) + } + } } for (const m of stmt.matchAll(/drop\s+constraint\s+(?:if\s+exists\s+)?([\w"]+)/gi)) { const edge = byConstraint.get(normIdent(m[1]))