stop handing the client a layout it cannot use

The layout columns defaulted to '[]' — an empty array for a column whose only legal contents are a
LayoutNode object — and every upsert that omitted a layout wrote it. Creating a dashboard from the
dashboard list is exactly that path, so the key came back present, the client's `key in state` check
preferred it over the caller's default, and normalizeLayout called .children.map on it and threw.

Three layers, because none of them was enforcing anything:

- the columns are nullable with no default: NULL means "none stored", which is the truth
- getAllDashboardState omits the key when what is stored is not an object, so rows written before this
  are repaired by the next write rather than crashing the read
- useDashboardState checks kind-compatibility before casting jsonb to T, and falls back to the caller's
  default when it does not match. Only object-shaped defaults are guarded — a wrong primitive is a
  cosmetic surprise, a wrong container is a crash.

Verified against the live DB: creating a dashboard with no layout no longer emits a ws-layout key, and
a row hand-set back to '[]' is omitted too.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
2026-08-07 09:08:15 +00:00
co-authored by Claude Opus 5
parent 64961d49f5
commit ef036dfcd5
3 changed files with 57 additions and 8 deletions
@@ -23,7 +23,7 @@ export async function getAllDashboardState(userId: number): Promise<Record<strin
result['workspaces'] = sorted.map((d) => ({ id: d.id, name: d.name, ...(d.config as object) })); result['workspaces'] = sorted.map((d) => ({ id: d.id, name: d.name, ...(d.config as object) }));
for (const d of dashRows) { for (const d of dashRows) {
result[`ws-layout-${d.id}`] = d.layout; if (isLayoutish(d.layout)) result[`ws-layout-${d.id}`] = d.layout;
result[`ws-terminals-${d.id}`] = d.terminals; result[`ws-terminals-${d.id}`] = d.terminals;
result[`ws-host-terminals-${d.id}`] = d.hostTerminals; result[`ws-host-terminals-${d.id}`] = d.hostTerminals;
for (const [prefix, value] of Object.entries(asRecord(d.panelState))) { for (const [prefix, value] of Object.entries(asRecord(d.panelState))) {
@@ -43,7 +43,7 @@ export async function getAllDashboardState(userId: number): Promise<Record<strin
// Screens // Screens
for (const s of screenRows) { for (const s of screenRows) {
result[`screens/${s.name}`] = s.layout; if (isLayoutish(s.layout)) result[`screens/${s.name}`] = s.layout;
} }
return result; return result;
@@ -52,6 +52,18 @@ export async function getAllDashboardState(userId: number): Promise<Record<strin
const asRecord = (value: unknown): Record<string, unknown> => const asRecord = (value: unknown): Record<string, unknown> =>
value && typeof value === 'object' && !Array.isArray(value) ? (value as Record<string, unknown>) : {}; value && typeof value === 'object' && !Array.isArray(value) ? (value as Record<string, unknown>) : {};
/**
* Omit a layout the client cannot use rather than hand it over.
*
* A `LayoutNode` is an object. The columns used to default to `'[]'`, and every path that upserts without
* one wrote it — so the key was present, the client's `key in state` check preferred it over the caller's
* default, and `normalizeLayout` called `.children.map` on an array of nothing and threw during render.
* Dropping the key instead lets the caller's default win and the next write repairs the row. The column is
* nullable now, so this only catches rows written before that; keep it anyway — it is the read-side half of
* a rule nothing else enforces.
*/
const isLayoutish = (value: unknown): boolean => !!value && typeof value === 'object' && !Array.isArray(value);
// ── Dashboard CRUD ── // ── Dashboard CRUD ──
type UpsertDashboardData = { type UpsertDashboardData = {
@@ -83,7 +95,7 @@ export async function upsertDashboard(userId: number, id: string, data: UpsertDa
userId, userId,
name: data.name ?? id, name: data.name ?? id,
config: data.config ?? {}, config: data.config ?? {},
layout: data.layout ?? [], layout: data.layout ?? null,
terminals: data.terminals ?? {}, terminals: data.terminals ?? {},
hostTerminals: data.hostTerminals ?? {}, hostTerminals: data.hostTerminals ?? {},
sortOrder: data.sortOrder ?? 0, sortOrder: data.sortOrder ?? 0,
@@ -158,7 +170,7 @@ export async function upsertScreen(userId: number, name: string, data: UpsertScr
.values({ .values({
userId, userId,
name, name,
layout: (data.layout ?? []) as never, layout: (data.layout ?? null) as never,
terminals: (data.terminals ?? {}) as never, terminals: (data.terminals ?? {}) as never,
hostTerminals: (data.hostTerminals ?? {}) as never, hostTerminals: (data.hostTerminals ?? {}) as never,
updatedAt: now, updatedAt: now,
@@ -10,7 +10,11 @@ export const dashboards = pgTable(
.references(() => users.id, { onDelete: 'cascade' }), .references(() => users.id, { onDelete: 'cascade' }),
name: text('name').notNull(), name: text('name').notNull(),
config: jsonb('config').notNull().default({}), config: jsonb('config').notNull().default({}),
layout: jsonb('layout').notNull().default([]), // Nullable with NO default, deliberately. It held `'[]'` — an empty *array* for a column whose only
// legal contents are a `LayoutNode` object — so every upsert that omitted a layout wrote a value the
// client then preferred over its own default and crashed traversing. NULL says "none stored", which is
// the truth, and `getAllDashboardState` omits the key entirely so the caller's default wins.
layout: jsonb('layout'),
// `{}`, not `[]` — every writer treats these as a panelId → sessionId map, and `screens` and // `{}`, not `[]` — every writer treats these as a panelId → sessionId map, and `screens` and
// `dashboard_defaults` already default them correctly. The array default only ever produced a shape // `dashboard_defaults` already default them correctly. The array default only ever produced a shape
// the client had to spread away on first write. // the client had to spread away on first write.
@@ -36,7 +40,8 @@ export const screens = pgTable(
.notNull() .notNull()
.references(() => users.id, { onDelete: 'cascade' }), .references(() => users.id, { onDelete: 'cascade' }),
name: text('name').notNull(), name: text('name').notNull(),
layout: jsonb('layout').notNull().default([]), // Same as `dashboards.layout` above — nullable, no default.
layout: jsonb('layout'),
terminals: jsonb('terminals').notNull().default({}), terminals: jsonb('terminals').notNull().default({}),
hostTerminals: jsonb('host_terminals').notNull().default({}), hostTerminals: jsonb('host_terminals').notNull().default({}),
updatedAt: timestamp('updated_at', { withTimezone: true }).notNull().defaultNow(), updatedAt: timestamp('updated_at', { withTimezone: true }).notNull().defaultNow(),
+34 -2
View File
@@ -36,12 +36,12 @@ export function useDashboardState<T>(key: string, defaultValue: T) {
} }
}, [isSuccess, key, defaultValue, queryClient]); }, [isSuccess, key, defaultValue, queryClient]);
const value = key in state ? (state[key] as T) : defaultValue; const value = readValue(state, key, defaultValue);
const setValue = useCallback( const setValue = useCallback(
(update: T | ((prev: T) => T)) => { (update: T | ((prev: T) => T)) => {
const currentState = queryClient.getQueryData<UserState>(QUERY_KEY) ?? {}; const currentState = queryClient.getQueryData<UserState>(QUERY_KEY) ?? {};
const currentValue = key in currentState ? (currentState[key] as T) : defaultValue; const currentValue = readValue(currentState, key, defaultValue);
const newValue = typeof update === 'function' ? (update as (prev: T) => T)(currentValue) : update; const newValue = typeof update === 'function' ? (update as (prev: T) => T)(currentValue) : update;
queryClient.setQueryData(QUERY_KEY, { ...currentState, [key]: newValue }); queryClient.setQueryData(QUERY_KEY, { ...currentState, [key]: newValue });
@@ -58,6 +58,38 @@ export function useDashboardState<T>(key: string, defaultValue: T) {
return { key, value, setValue, isLoaded: isSuccess }; return { key, value, setValue, isLoaded: isSuccess };
} }
/**
* Read a stored value, or the caller's default when what is stored cannot be that value.
*
* Everything here arrives from jsonb as `unknown` and used to be cast straight to `T`. Nothing validates
* it on the way in either, so one wrong-shaped row — a layout column that defaulted to `[]`, a
* `panelId → sessionId` map that arrived as an array — became a `.children.map is not a function` during
* render, which is a white screen with no recovery short of SQL.
*
* The check is kind-compatibility, not a schema: only object-shaped defaults are guarded, because those
* are the values whose consumers traverse or spread them. A wrong *primitive* is a cosmetic surprise; a
* wrong container is a crash. Deliberately does NOT write the correction back — a read should not
* overwrite the server, and the next real `setValue` repairs the row anyway.
*/
function readValue<T>(state: UserState, key: string, defaultValue: T): T {
if (!(key in state)) return defaultValue;
const stored = state[key];
const wantsContainer = typeof defaultValue === 'object' && defaultValue !== null;
if (!wantsContainer) return stored as T;
const isContainer = typeof stored === 'object' && stored !== null;
if (isContainer && Array.isArray(stored) === Array.isArray(defaultValue)) return stored as T;
if (!warned.has(key)) {
warned.add(key);
console.warn(`[dashboard-state] ignoring stored "${key}" — wrong shape, using the default instead`, stored);
}
return defaultValue;
}
const warned = new Set<string>();
/** /**
* Undo an optimistic write the server refused, and say so. * Undo an optimistic write the server refused, and say so.
* *