fix(console): read the SETTINGS_LOCKED key from error.details, tolerating both shapes (objectstack#4224) (#3079)

`SettingsView.onSave` rendered its lock toast from `err.payload.error.key` — a
sibling of `code`/`message` inside `error` that `ApiErrorSchema` never declared.
It reached the console only because that 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 the key to `error.details.key`, the declared slot. This is
the console half and lands first: `lockedKeyOf` in `settings/api.ts` reads the
declared slot with the old position as fallback, so the toast keeps naming the
locked key against servers on either side of the change. Both branches are pinned
in `api.test.ts`, so removing either is a red test naming the server version it
breaks. Also stops appending `undefined` when no position carries a key.

Author: os-zhuang

.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.

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');
       }

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();
+  });
+});

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<SettingsListResponse> {
   const res = await fetch(BASE, { credentials: 'include', headers: jsonHeaders() });
   return jsonOrThrow<SettingsListResponse>(res);