From 03db5a21bfec61d1e672f42bd81c13fe3d0cca5e Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 30 Jul 2026 16:57:51 +0000 Subject: [PATCH] fix(console): read the SETTINGS_LOCKED key from `error.details`, tolerating both shapes (objectstack#4224) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `SettingsView.onSave` rendered its lock toast from `err.payload.error.key`. That key was a SIBLING of `code`/`message` inside `error`, a position `ApiErrorSchema` never declared — it reached the console only because the schema is a plain `z.object` and strips undeclared keys instead of rejecting them, so nothing on either side of the wire ever flagged it. objectstack#4224 moves it to `error.details.key`, the slot the contract does declare. This is the console's half and ships FIRST: the read is now the declared slot with the old position as fallback, so the toast keeps naming the locked key against servers on either side of that change rather than degrading to "Locked by environment: undefined" during the window where the two repos are on different versions. The fallback goes once the oldest supported server carries the fix. It lives in `settings/api.ts` as `lockedKeyOf` rather than inline in the view, because that module already owns what the server's error body looks like (`jsonOrThrow` is what builds `err.payload`) and because a compat shim no test exercises is one the next reader deletes half of. Both branches are pinned, so removing either is a red test naming the server version it breaks. Also stops interpolating a missing key: with neither position carrying one the toast reads "Locked by environment" rather than appending `undefined`. This was the only in-console reader of the four keys objectstack#4224 relocated — a repo-wide grep for `namespace` / `reason` / `fields` in that position finds no consumer. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Tv2NYMBTrzVVRWiWWuyec5 --- .../settings-locked-key-tolerant-read.md | 26 ++++++++ .../src/pages/settings/SettingsView.tsx | 8 ++- apps/console/src/pages/settings/api.test.ts | 64 +++++++++++++++++++ apps/console/src/pages/settings/api.ts | 29 +++++++++ 4 files changed, 125 insertions(+), 2 deletions(-) create mode 100644 .changeset/settings-locked-key-tolerant-read.md create mode 100644 apps/console/src/pages/settings/api.test.ts diff --git a/.changeset/settings-locked-key-tolerant-read.md b/.changeset/settings-locked-key-tolerant-read.md new file mode 100644 index 0000000000..aeb379fca9 --- /dev/null +++ b/.changeset/settings-locked-key-tolerant-read.md @@ -0,0 +1,26 @@ +--- +"@object-ui/console": patch +--- + +fix(settings): read the locked key from `error.details`, tolerating both wire shapes — objectstack#4224 + +`SettingsView.onSave` rendered the `SETTINGS_LOCKED` toast from +`err.payload.error.key`. That key was a SIBLING of `code`/`message` inside +`error`, a position `ApiErrorSchema` never declared — it reached the console only +because the schema is a plain `z.object` and silently strips what it does not +declare, so nothing ever failed to flag it. objectstack#4224 moves it into +`error.details`, the slot the contract does declare. + +This is the console's half, and it ships **first**: the read is now +`error.details?.key ?? error.key`, so the toast keeps naming the locked key +against servers on either side of that change rather than degrading to +`Locked by environment: undefined` during the window where the two repos are on +different versions. The fallback can go once the oldest supported server carries +the fix. + +Also stops interpolating a missing key: when neither position carries one the +toast now reads `Locked by environment` rather than appending `undefined`. + +This was the only in-console reader of the four keys objectstack#4224 relocated +(`namespace`, `key`, `reason`, `fields`) — a repo-wide grep for the other three +finds no consumer. diff --git a/apps/console/src/pages/settings/SettingsView.tsx b/apps/console/src/pages/settings/SettingsView.tsx index 16116dcbd6..5280f95f6a 100644 --- a/apps/console/src/pages/settings/SettingsView.tsx +++ b/apps/console/src/pages/settings/SettingsView.tsx @@ -15,6 +15,7 @@ import { getIcon } from '../../utils/getIcon'; import { SettingsField } from './SettingsField'; import { getSettingsNamespace, + lockedKeyOf, runSettingsAction, saveSettingsNamespace, } from './api'; @@ -128,8 +129,11 @@ export function SettingsView() { setDraft({}); toast.success('Settings saved'); } catch (err: any) { - if (err?.payload?.error?.code === 'SETTINGS_LOCKED') { - toast.error(`Locked by environment: ${err.payload.error.key}`); + const apiError = err?.payload?.error; + if (apiError?.code === 'SETTINGS_LOCKED') { + // `lockedKeyOf` reads both wire positions — see its note (objectstack#4224). + const key = lockedKeyOf(apiError); + toast.error(key ? `Locked by environment: ${key}` : 'Locked by environment'); } else { toast.error(err?.message ?? 'Save failed'); } diff --git a/apps/console/src/pages/settings/api.test.ts b/apps/console/src/pages/settings/api.test.ts new file mode 100644 index 0000000000..fa8e7ebd59 --- /dev/null +++ b/apps/console/src/pages/settings/api.test.ts @@ -0,0 +1,64 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * `lockedKeyOf` — the two wire positions of a `SETTINGS_LOCKED` key + * (objectstack#4224). + * + * Why this is pinned rather than left as an inline `??`: it is a COMPAT read, + * and the failure mode of a compat read is that someone deletes the half they + * did not know was load-bearing. Both branches are asserted here, so removing + * either one is a red test that names the server version it breaks — which is + * also the signal for when the old branch may legitimately go. + * + * The old position (`error.key`) was never declared by `ApiErrorSchema`; it + * reached the console only because the schema is a plain `z.object` and strips + * undeclared keys instead of rejecting them. That is exactly why no test on + * either side of the wire ever caught it. + */ + +import { describe, it, expect } from 'vitest'; +import { lockedKeyOf } from './api'; + +describe('lockedKeyOf', () => { + it('reads the declared slot — a server carrying objectstack#4224', () => { + expect( + lockedKeyOf({ + code: 'SETTINGS_LOCKED', + message: "Setting 'branding.workspace_name' is locked (locked-by-env).", + details: { namespace: 'branding', key: 'workspace_name', reason: 'locked-by-env' }, + }), + ).toBe('workspace_name'); + }); + + it('falls back to the pre-#4224 sibling — a server without it', () => { + expect( + lockedKeyOf({ + code: 'SETTINGS_LOCKED', + message: "Setting 'branding.workspace_name' is locked (locked-by-env).", + namespace: 'branding', + key: 'workspace_name', + reason: 'locked-by-env', + }), + ).toBe('workspace_name'); + }); + + it('prefers the declared slot when a body somehow carries both', () => { + expect(lockedKeyOf({ key: 'old_key', details: { key: 'new_key' } })).toBe('new_key'); + }); + + it('yields undefined rather than a string when no position carries a key', () => { + // The toast branches on this: no key means "Locked by environment" rather + // than "Locked by environment: undefined". + expect(lockedKeyOf({ code: 'SETTINGS_LOCKED', message: 'Locked.' })).toBeUndefined(); + expect(lockedKeyOf({ details: {} })).toBeUndefined(); + expect(lockedKeyOf({ details: null })).toBeUndefined(); + expect(lockedKeyOf({ key: '' })).toBeUndefined(); + expect(lockedKeyOf({ key: 42 })).toBeUndefined(); + expect(lockedKeyOf(undefined)).toBeUndefined(); + expect(lockedKeyOf('not an object')).toBeUndefined(); + }); +}); diff --git a/apps/console/src/pages/settings/api.ts b/apps/console/src/pages/settings/api.ts index 90a4af98f4..82783928ba 100644 --- a/apps/console/src/pages/settings/api.ts +++ b/apps/console/src/pages/settings/api.ts @@ -28,6 +28,35 @@ const jsonHeaders = (): HeadersInit => ({ Accept: 'application/json', }); +/** + * The env-locked key named by a `SETTINGS_LOCKED` error, read from whichever + * position the serving version puts it in (objectstack#4224). + * + * It used to arrive as `error.key` — a SIBLING of `code`/`message` that + * `ApiErrorSchema` never declared. That body was accepted only because the + * schema is a plain `z.object` and strips undeclared keys rather than rejecting + * them, so nothing on either side ever flagged it. objectstack#4224 moves it to + * `error.details.key`, the slot the contract does declare. + * + * Declared home first, old position second, so the console names the key against + * servers on either side of that change instead of rendering `undefined` for the + * duration of the window. Drop the second read once the oldest supported server + * carries the fix. + * + * Lives here rather than inline in the view because this module already owns + * what the server's error body looks like (`jsonOrThrow` is what builds + * `err.payload`), and because a compat shim that no test exercises is a compat + * shim that gets deleted by the next person who reads only one of the two + * shapes. + */ +export function lockedKeyOf(apiError: unknown): string | undefined { + if (!apiError || typeof apiError !== 'object') return undefined; + const e = apiError as { key?: unknown; details?: { key?: unknown } | null }; + const declared = e.details && typeof e.details === 'object' ? e.details.key : undefined; + const found = declared ?? e.key; + return typeof found === 'string' && found.length > 0 ? found : undefined; +} + export async function listSettingsManifests(): Promise { const res = await fetch(BASE, { credentials: 'include', headers: jsonHeaders() }); return jsonOrThrow(res);