stop the dashboard PATCH dispatcher losing writes
Four defects, one shape: a write that returns 200 and lands nowhere.
- Unknown keys were dropped by a chain of `if (match) continue` with no `else`. The three prefixes
CommandTerminalWrapper actually writes — tmux, nvim, claude-code — were among them, so those panel
maps lived in the React Query cache only: every reload minted a fresh uuid and abandoned a running
pty. They now live in a `panel_state` bag on the dashboard row, and an unmatched key 400s.
- `ws-terminals-{id}: null` fell through to an upsert, writing NULL into a NOT NULL column on a live
dashboard and re-INSERTing a deleted one. Renaming a dashboard sends exactly that, paired with
`ws-layout-{id}: null`, so the old slug came back as a zombie row in the dashboards list.
- HostTerminalWrapper and CommandTerminalWrapper built their state key straight from `dashboardId`,
which is a workspace *key* (`ws-layout-<id>`), while TerminalWrapper stripped the prefix. The server
read the un-stripped form back as a dashboard id and created it. One rule now, in state-key.ts.
Verified against the live server: unknown key 400s, the three prefixes round-trip, a null on a live
dashboard is a no-op, and the rename sequence leaves no zombie.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -56,11 +56,14 @@ export {
|
||||
export {
|
||||
getAllDashboardState,
|
||||
upsertDashboard,
|
||||
updateDashboard,
|
||||
deleteDashboard,
|
||||
setDashboardPanelState,
|
||||
upsertScreen,
|
||||
deleteScreen,
|
||||
getDefaults,
|
||||
upsertDefaults,
|
||||
setDefaultsPanelState,
|
||||
} from './queries/dashboards';
|
||||
|
||||
export {
|
||||
|
||||
@@ -26,6 +26,9 @@ export async function getAllDashboardState(userId: number): Promise<Record<strin
|
||||
result[`ws-layout-${d.id}`] = d.layout;
|
||||
result[`ws-terminals-${d.id}`] = d.terminals;
|
||||
result[`ws-host-terminals-${d.id}`] = d.hostTerminals;
|
||||
for (const [prefix, value] of Object.entries(asRecord(d.panelState))) {
|
||||
result[`ws-${prefix}-${d.id}`] = value;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -33,6 +36,9 @@ export async function getAllDashboardState(userId: number): Promise<Record<strin
|
||||
if (defaultsRow) {
|
||||
result['ws-terminals-default'] = defaultsRow.terminals;
|
||||
result['ws-host-terminals-default'] = defaultsRow.hostTerminals;
|
||||
for (const [prefix, value] of Object.entries(asRecord(defaultsRow.panelState))) {
|
||||
result[`ws-${prefix}-default`] = value;
|
||||
}
|
||||
}
|
||||
|
||||
// Screens
|
||||
@@ -43,6 +49,9 @@ export async function getAllDashboardState(userId: number): Promise<Record<strin
|
||||
return result;
|
||||
}
|
||||
|
||||
const asRecord = (value: unknown): Record<string, unknown> =>
|
||||
value && typeof value === 'object' && !Array.isArray(value) ? (value as Record<string, unknown>) : {};
|
||||
|
||||
// ── Dashboard CRUD ──
|
||||
|
||||
type UpsertDashboardData = {
|
||||
@@ -54,37 +63,75 @@ type UpsertDashboardData = {
|
||||
sortOrder?: number;
|
||||
};
|
||||
|
||||
function buildDashboardSet(data: UpsertDashboardData, now: Date): Record<string, unknown> {
|
||||
const set: Record<string, unknown> = { updatedAt: now };
|
||||
if (data.name !== undefined) set.name = data.name;
|
||||
if (data.config !== undefined) set.config = data.config;
|
||||
if (data.layout !== undefined) set.layout = data.layout;
|
||||
if (data.terminals !== undefined) set.terminals = data.terminals;
|
||||
if (data.hostTerminals !== undefined) set.hostTerminals = data.hostTerminals;
|
||||
if (data.sortOrder !== undefined) set.sortOrder = data.sortOrder;
|
||||
return set;
|
||||
}
|
||||
|
||||
export async function upsertDashboard(userId: number, id: string, data: UpsertDashboardData): Promise<void> {
|
||||
const now = new Date();
|
||||
if (await updateDashboard(userId, id, data)) return;
|
||||
|
||||
await db.insert(dashboards).values({
|
||||
id,
|
||||
userId,
|
||||
name: data.name ?? id,
|
||||
config: data.config ?? {},
|
||||
layout: data.layout ?? [],
|
||||
terminals: data.terminals ?? {},
|
||||
hostTerminals: data.hostTerminals ?? {},
|
||||
sortOrder: data.sortOrder ?? 0,
|
||||
createdAt: now,
|
||||
updatedAt: now,
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* Update a dashboard only if it already exists, and report whether it did.
|
||||
*
|
||||
* `upsertDashboard` INSERTs on an unknown id, which is right for the two create paths and wrong for every
|
||||
* other key: renaming a dashboard PATCHes `ws-layout-old: null` and `ws-terminals-old: null` together, and
|
||||
* the second key used to re-INSERT the row the first had just deleted — named after its own slug, so the
|
||||
* old name reappeared in the dashboard list as a zombie.
|
||||
*/
|
||||
export async function updateDashboard(userId: number, id: string, data: UpsertDashboardData): Promise<boolean> {
|
||||
const updated = await db
|
||||
.update(dashboards)
|
||||
.set(buildDashboardSet(data, new Date()))
|
||||
.where(and(eq(dashboards.userId, userId), eq(dashboards.id, id)))
|
||||
.returning({ id: dashboards.id });
|
||||
return updated.length > 0;
|
||||
}
|
||||
|
||||
/** Write one prefix of a dashboard's `panelState` bag, leaving its siblings alone. */
|
||||
export async function setDashboardPanelState(
|
||||
userId: number,
|
||||
id: string,
|
||||
prefix: string,
|
||||
value: unknown,
|
||||
): Promise<boolean> {
|
||||
const existing = await db
|
||||
.select()
|
||||
.select({ panelState: dashboards.panelState })
|
||||
.from(dashboards)
|
||||
.where(and(eq(dashboards.userId, userId), eq(dashboards.id, id)))
|
||||
.then((rows) => rows[0]);
|
||||
if (!existing) return false;
|
||||
|
||||
if (existing) {
|
||||
const set: Record<string, unknown> = { updatedAt: now };
|
||||
if (data.name !== undefined) set.name = data.name;
|
||||
if (data.config !== undefined) set.config = data.config;
|
||||
if (data.layout !== undefined) set.layout = data.layout;
|
||||
if (data.terminals !== undefined) set.terminals = data.terminals;
|
||||
if (data.hostTerminals !== undefined) set.hostTerminals = data.hostTerminals;
|
||||
if (data.sortOrder !== undefined) set.sortOrder = data.sortOrder;
|
||||
await db.update(dashboards).set(set).where(eq(dashboards.id, id));
|
||||
} else {
|
||||
await db.insert(dashboards).values({
|
||||
id,
|
||||
userId,
|
||||
name: data.name ?? id,
|
||||
config: data.config ?? {},
|
||||
layout: data.layout ?? [],
|
||||
terminals: data.terminals ?? [],
|
||||
hostTerminals: data.hostTerminals ?? [],
|
||||
sortOrder: data.sortOrder ?? 0,
|
||||
createdAt: now,
|
||||
updatedAt: now,
|
||||
});
|
||||
}
|
||||
const next = { ...asRecord(existing.panelState) };
|
||||
if (value === null) delete next[prefix];
|
||||
else next[prefix] = value;
|
||||
|
||||
await db
|
||||
.update(dashboards)
|
||||
.set({ panelState: next, updatedAt: new Date() })
|
||||
.where(and(eq(dashboards.userId, userId), eq(dashboards.id, id)));
|
||||
return true;
|
||||
}
|
||||
|
||||
export async function deleteDashboard(userId: number, id: string): Promise<void> {
|
||||
@@ -157,3 +204,18 @@ export async function upsertDefaults(userId: number, data: UpsertDefaultsData):
|
||||
set,
|
||||
});
|
||||
}
|
||||
|
||||
/** The `panelState` equivalent of `upsertDefaults` — for panels living on a screen rather than a dashboard. */
|
||||
export async function setDefaultsPanelState(userId: number, prefix: string, value: unknown): Promise<void> {
|
||||
const now = new Date();
|
||||
const [row] = await db.select().from(dashboardDefaults).where(eq(dashboardDefaults.userId, userId));
|
||||
|
||||
const next = { ...asRecord(row?.panelState) };
|
||||
if (value === null) delete next[prefix];
|
||||
else next[prefix] = value;
|
||||
|
||||
await db
|
||||
.insert(dashboardDefaults)
|
||||
.values({ userId, panelState: next as never, updatedAt: now })
|
||||
.onConflictDoUpdate({ target: dashboardDefaults.userId, set: { panelState: next, updatedAt: now } });
|
||||
}
|
||||
|
||||
@@ -11,8 +11,16 @@ export const dashboards = pgTable(
|
||||
name: text('name').notNull(),
|
||||
config: jsonb('config').notNull().default({}),
|
||||
layout: jsonb('layout').notNull().default([]),
|
||||
terminals: jsonb('terminals').notNull().default([]),
|
||||
hostTerminals: jsonb('host_terminals').notNull().default([]),
|
||||
// `{}`, 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
|
||||
// the client had to spread away on first write.
|
||||
terminals: jsonb('terminals').notNull().default({}),
|
||||
hostTerminals: jsonb('host_terminals').notNull().default({}),
|
||||
// Panel-scoped session maps that have no column of their own, keyed by the app's state prefix:
|
||||
// `{ tmux: { <panelId>: <sessionId> }, nvim: {…}, 'claude-code': {…} }`. They used to have no home at
|
||||
// all — the PATCH dispatcher dropped them and returned 200, so every reload minted a fresh uuid and
|
||||
// abandoned the running pty. Living on the dashboard row means they are deleted with it.
|
||||
panelState: jsonb('panel_state').notNull().default({}),
|
||||
sortOrder: integer('sort_order').notNull().default(0),
|
||||
createdAt: timestamp('created_at', { withTimezone: true }).notNull().defaultNow(),
|
||||
updatedAt: timestamp('updated_at', { withTimezone: true }).notNull().defaultNow(),
|
||||
@@ -44,5 +52,6 @@ export const dashboardDefaults = pgTable('dashboard_defaults', {
|
||||
.unique(),
|
||||
terminals: jsonb('terminals').notNull().default({}),
|
||||
hostTerminals: jsonb('host_terminals').notNull().default({}),
|
||||
panelState: jsonb('panel_state').notNull().default({}),
|
||||
updatedAt: timestamp('updated_at', { withTimezone: true }).notNull().defaultNow(),
|
||||
});
|
||||
|
||||
@@ -2,14 +2,23 @@ import { createRouter } from '@@/create-router';
|
||||
import {
|
||||
getAllDashboardState,
|
||||
upsertDashboard,
|
||||
updateDashboard,
|
||||
deleteDashboard,
|
||||
upsertScreen,
|
||||
deleteScreen,
|
||||
upsertDefaults,
|
||||
setDashboardPanelState,
|
||||
setDefaultsPanelState,
|
||||
} from 'officerdb';
|
||||
|
||||
export const dashboardsRouter = createRouter();
|
||||
|
||||
// Panel-scoped session maps with no column of their own, written by `CommandTerminalWrapper` as
|
||||
// `ws-<statePrefix>-<dashboardId>`. This is an allow-list because `ws-<prefix>-<id>` cannot be split
|
||||
// without one — both halves may contain dashes. Keep it in step with the `statePrefix` props in
|
||||
// `apps/Terminal/index.tsx`; the 400 at the bottom of the PATCH loop is what tells you when it is not.
|
||||
const PANEL_STATE_PREFIXES = ['tmux', 'nvim', 'claude-code'];
|
||||
|
||||
// GET /dashboards
|
||||
dashboardsRouter.get('/', async (ctx) => {
|
||||
const userId = ctx.get('user').id;
|
||||
@@ -57,11 +66,18 @@ dashboardsRouter.patch('/', async (ctx) => {
|
||||
continue;
|
||||
}
|
||||
|
||||
// ws-terminals-{id}
|
||||
// ws-terminals-{id} / ws-host-terminals-{id}
|
||||
//
|
||||
// A null means "forget this key". It arrives paired with `ws-layout-{id}: null` on a rename, by which
|
||||
// point the row is already gone — so dropping it silently is the whole job. These branches used to
|
||||
// fall through to an upsert, which wrote NULL into a NOT NULL column on a live dashboard (23502) and
|
||||
// re-INSERTed a deleted one as a zombie named after its own slug.
|
||||
const wsTerminalsMatch = key.match(/^ws-terminals-(.+)$/);
|
||||
if (wsTerminalsMatch) {
|
||||
const id = wsTerminalsMatch[1]!;
|
||||
await upsertDashboard(userId, id, { terminals: value });
|
||||
if (value !== null && !(await updateDashboard(userId, id, { terminals: value }))) {
|
||||
return ctx.json({ error: `no dashboard "${id}" to write "${key}" to` }, 404);
|
||||
}
|
||||
continue;
|
||||
}
|
||||
|
||||
@@ -69,7 +85,21 @@ dashboardsRouter.patch('/', async (ctx) => {
|
||||
const wsHostTerminalsMatch = key.match(/^ws-host-terminals-(.+)$/);
|
||||
if (wsHostTerminalsMatch) {
|
||||
const id = wsHostTerminalsMatch[1]!;
|
||||
await upsertDashboard(userId, id, { hostTerminals: value });
|
||||
if (value !== null && !(await updateDashboard(userId, id, { hostTerminals: value }))) {
|
||||
return ctx.json({ error: `no dashboard "${id}" to write "${key}" to` }, 404);
|
||||
}
|
||||
continue;
|
||||
}
|
||||
|
||||
// ws-{prefix}-{id} for the prefixes that have no column of their own
|
||||
const prefix = PANEL_STATE_PREFIXES.find((p) => key.startsWith(`ws-${p}-`));
|
||||
if (prefix) {
|
||||
const id = key.slice(`ws-${prefix}-`.length);
|
||||
if (id === 'default') {
|
||||
await setDefaultsPanelState(userId, prefix, value);
|
||||
} else if (!(await setDashboardPanelState(userId, id, prefix, value))) {
|
||||
return ctx.json({ error: `no dashboard "${id}" to write "${key}" to` }, 404);
|
||||
}
|
||||
continue;
|
||||
}
|
||||
|
||||
@@ -85,6 +115,10 @@ dashboardsRouter.patch('/', async (ctx) => {
|
||||
continue;
|
||||
}
|
||||
|
||||
// No silent drops. An unmatched key used to return 200 with a fresh state blob, so a whole key family
|
||||
// in active use could persist nowhere at all and look like it had — which is exactly what happened to
|
||||
// the three prefixes above.
|
||||
return ctx.json({ error: `unknown dashboard-state key "${key}"` }, 400);
|
||||
}
|
||||
|
||||
const state = await getAllDashboardState(userId);
|
||||
|
||||
@@ -4,6 +4,7 @@ import { useDashboardState } from 'state/useDashboardState';
|
||||
import { useGlobal } from 'hooks/useGlobal';
|
||||
import type { TerminalConnectionState } from './Terminal';
|
||||
import { TerminalView } from './Terminal';
|
||||
import { terminalStateKey } from './state-key';
|
||||
|
||||
const EMPTY_TERMINALS: Record<string, string> = {};
|
||||
|
||||
@@ -15,7 +16,7 @@ type CommandTerminalWrapperProps = {
|
||||
|
||||
export const CommandTerminalWrapper = ({ panelId, command, statePrefix }: CommandTerminalWrapperProps) => {
|
||||
const { dashboardId, cwd } = useWorkspace();
|
||||
const stateKey = dashboardId ? `ws-${statePrefix}-${dashboardId}` : `ws-${statePrefix}-default`;
|
||||
const stateKey = terminalStateKey(statePrefix, dashboardId);
|
||||
const { value: terminals, setValue: setTerminals } = useDashboardState<Record<string, string>>(
|
||||
stateKey,
|
||||
EMPTY_TERMINALS,
|
||||
|
||||
@@ -4,12 +4,13 @@ import { useDashboardState } from 'state/useDashboardState';
|
||||
import { useGlobal } from 'hooks/useGlobal';
|
||||
import type { TerminalConnectionState } from './Terminal';
|
||||
import { TerminalView } from './Terminal';
|
||||
import { terminalStateKey } from './state-key';
|
||||
|
||||
const EMPTY_TERMINALS: Record<string, string> = {};
|
||||
|
||||
export const HostTerminalWrapper = ({ panelId }: { panelId: string }) => {
|
||||
const { dashboardId, cwd } = useWorkspace();
|
||||
const stateKey = dashboardId ? `ws-host-terminals-${dashboardId}` : 'ws-host-terminals-default';
|
||||
const stateKey = terminalStateKey('host-terminals', dashboardId);
|
||||
const { value: terminals, setValue: setTerminals } = useDashboardState<Record<string, string>>(
|
||||
stateKey,
|
||||
EMPTY_TERMINALS,
|
||||
|
||||
@@ -4,16 +4,13 @@ import { useDashboardState } from 'state/useDashboardState';
|
||||
import { useGlobal } from 'hooks/useGlobal';
|
||||
import type { TerminalConnectionState } from './Terminal';
|
||||
import { TerminalView } from './Terminal';
|
||||
import { terminalStateKey } from './state-key';
|
||||
|
||||
const EMPTY_TERMINALS: Record<string, string> = {};
|
||||
|
||||
export const TerminalWrapper = ({ panelId }: { panelId: string }) => {
|
||||
const { dashboardId, cwd } = useWorkspace();
|
||||
const stateKey = (() => {
|
||||
const wsMatch = dashboardId?.match(/^ws-layout-(.+)$/);
|
||||
if (wsMatch) return `ws-terminals-${wsMatch[1]}`;
|
||||
return 'ws-terminals-default';
|
||||
})();
|
||||
const stateKey = terminalStateKey('terminals', dashboardId);
|
||||
const { value: terminals, setValue: setTerminals } = useDashboardState<Record<string, string>>(
|
||||
stateKey,
|
||||
EMPTY_TERMINALS,
|
||||
|
||||
@@ -0,0 +1,16 @@
|
||||
/**
|
||||
* The dashboard-state key a terminal panel stores its `panelId → sessionId` map under.
|
||||
*
|
||||
* `useWorkspace().dashboardId` is the workspace *key*, not an id — `ws-layout-<id>` for a dashboard and
|
||||
* `screens/<name>` for a screen. Three wrappers derived a key from it by three different rules, and two of
|
||||
* them left the prefix on: `ws-host-terminals-ws-layout-my-dash` parses back out as a dashboard called
|
||||
* `ws-layout-my-dash`, which the server then created, and which showed up in the Dashboards list as a real
|
||||
* dashboard. One rule, in one place, is the fix for that class.
|
||||
*
|
||||
* Screens fall back to `-default`, which is the behaviour `TerminalWrapper` already had: a screen has no
|
||||
* dashboard row to hang the map on, and panel ids are unique across the app.
|
||||
*/
|
||||
export function terminalStateKey(prefix: string, dashboardId: string | null): string {
|
||||
const id = dashboardId?.match(/^ws-layout-(.+)$/)?.[1];
|
||||
return `ws-${prefix}-${id ?? 'default'}`;
|
||||
}
|
||||
Reference in New Issue
Block a user