Skip to content

Commit 8f53588

Browse files
committed
wip: Branding Setup entry moves with the namespace; plugin-auth binds the brand only where branding is offered
Claude-Session: https://claude.ai/code/session_01UmqzD97e14iws6hmVhgZ5y Co-authored-by: Claude <noreply@anthropic.com>
1 parent 801e5ac commit 8f53588

6 files changed

Lines changed: 219 additions & 9 deletions

File tree

‎packages/cli/scripts/check-app-nav-i18n.mjs‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -368,6 +368,41 @@ const CONTRIBUTORS = [
368368
return { plugin: new DatasourceAdminServicePlugin({}) };
369369
},
370370
},
371+
{
372+
// `nav_settings_branding` (#24766). The plugin contributes it from its
373+
// `kernel:ready` hook, and only where no organization wall stands, so this
374+
// entry boots it through init → start → kernel:ready on a context of its
375+
// own that captures `kernel:ready` — the shared context below fires no
376+
// hooks — with no `tenancy` service, which is the `single` reading, and
377+
// returns what it registered.
378+
source: '@objectstack/service-settings (Setup entries of the single-posture namespaces)',
379+
apps: ['setup'],
380+
async load() {
381+
const { SettingsServicePlugin } = await import('@objectstack/service-settings');
382+
const manifests = [];
383+
const ready = [];
384+
const own = {
385+
getService: (name) => {
386+
if (name === 'manifest') return { register: (m) => manifests.push(m) };
387+
throw new Error(`Service '${name}' not found`);
388+
},
389+
registerService: () => {},
390+
registerServiceFactory: () => {},
391+
replaceService: () => {},
392+
getServiceScoped: async () => undefined,
393+
getServices: () => new Map(),
394+
hook: (name, fn) => { if (name === 'kernel:ready') ready.push(fn); },
395+
trigger: async () => {},
396+
logger: { info: () => {}, warn: () => {}, error: () => {}, debug: () => {} },
397+
getKernel: () => undefined,
398+
};
399+
const plugin = new SettingsServicePlugin({ env: {}, registerRoutes: false });
400+
await plugin.init(own);
401+
await plugin.start(own);
402+
for (const fn of ready) await fn();
403+
return { manifests };
404+
},
405+
},
371406
{
372407
source: '@objectstack/mcp (CONNECT_AGENT_UI_BUNDLE)',
373408
// The one contributor that lands in BOTH apps: `CONNECT_AGENT_UI_BUNDLE`

‎packages/platform-objects/src/apps/setup-nav.contributions.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,9 @@ export const SETUP_NAV_CONTRIBUTIONS: NavigationContribution[] = [
127127
// the app's base `setup.access`).
128128
{ id: 'nav_settings_localization', type: 'url', label: 'Localization', url: '/apps/setup/system/settings/localization', icon: 'globe' },
129129
{ id: 'nav_settings_company', type: 'url', label: 'Company', url: '/apps/setup/system/settings/company', icon: 'building-2' },
130-
{ id: 'nav_settings_branding', type: 'url', label: 'Branding', url: '/apps/setup/system/settings/branding', icon: 'palette' },
130+
// `nav_settings_branding` is contributed by `@objectstack/service-settings`
131+
// with the `branding` namespace, which is offered only where no
132+
// organization wall stands (#24766) — so the entry lives and dies with it.
131133
{ id: 'nav_settings_auth', type: 'url', label: 'Authentication', url: '/apps/setup/system/settings/auth', icon: 'lock-keyhole', requiredPermissions: ['manage_platform_settings'] },
132134
{ id: 'nav_settings_mail', type: 'url', label: 'Email', url: '/apps/setup/system/settings/mail', icon: 'mail', requiredPermissions: ['manage_platform_settings'] },
133135
{ id: 'nav_settings_storage', type: 'url', label: 'File Storage', url: '/apps/setup/system/settings/storage', icon: 'hard-drive', requiredPermissions: ['manage_platform_settings'] },

‎packages/plugins/plugin-auth/src/auth-plugin.test.ts‎

Lines changed: 46 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import { AuthManager } from './auth-manager';
66
import type { PluginContext } from '@objectstack/core';
77
import { assertEngineFindOnePredicate, assertEngineUpdateDispatch } from '@objectstack/objectql';
88
import { bindCatalogFromTables } from './__tests__/security-catalog.testkit.js';
9+
import { SettingsService, brandingSettingsManifest } from '@objectstack/service-settings';
910

1011
describe('AuthPlugin', () => {
1112
let mockContext: PluginContext;
@@ -460,12 +461,21 @@ describe('AuthPlugin', () => {
460461
let hookCapture: ReturnType<typeof createHookCapture>;
461462
let setAppNameSpy: ReturnType<typeof vi.spyOn>;
462463

463-
const makeSettings = (resolved: { value: unknown; source: string } | Error) => ({
464+
// `listManifests` lists `branding`, as the settings service does where no
465+
// organization wall stands — the binding is gated on that listing (#24766).
466+
const makeSettings = (
467+
resolved: { value: unknown; source: string } | Error,
468+
opts: { brandingListed?: boolean } = {},
469+
) => ({
464470
get: vi.fn(async () => {
465471
if (resolved instanceof Error) throw resolved;
466472
return resolved;
467473
}),
468474
subscribe: vi.fn(),
475+
listManifests: vi.fn(() => [
476+
{ namespace: 'localization' },
477+
...(opts.brandingListed === false ? [] : [{ namespace: 'branding' }]),
478+
]),
469479
});
470480

471481
const bootWithSettings = async (settings: unknown) => {
@@ -517,6 +527,41 @@ describe('AuthPlugin', () => {
517527
expect.stringContaining('failed to apply branding.workspace_name'),
518528
);
519529
});
530+
531+
// #24766 — under a walled tenancy posture service-settings does not offer
532+
// the `branding` namespace. The binding then stays unbound: no read, no
533+
// subscription, and no `warn` on a boot that is working as designed.
534+
// The double's `get` would THROW for `branding` (as the real service does
535+
// for an unregistered namespace), so a binding that skipped the listing
536+
// check would log the warn this pins away.
537+
it('a walled boot (branding not listed) neither reads it nor warns about it', async () => {
538+
const settings = makeSettings(new Error("Unknown settings namespace: 'branding'"), { brandingListed: false });
539+
await bootWithSettings(settings);
540+
541+
expect(settings.get).not.toHaveBeenCalledWith('branding', 'workspace_name', expect.anything());
542+
expect(settings.subscribe).not.toHaveBeenCalledWith('branding', expect.any(Function));
543+
const warns = (mockContext.logger.warn as ReturnType<typeof vi.fn>).mock.calls.map((c) => String(c[0]));
544+
expect(warns.filter((w) => w.includes('branding'))).toEqual([]);
545+
expect(setAppNameSpy).not.toHaveBeenCalled();
546+
});
547+
548+
// The same two postures through the REAL settings service: withheld under a
549+
// wall (the manifest never registered, so a read would throw
550+
// UnknownNamespaceError), offered and saved under `single`.
551+
it('real SettingsService: no branding registered → unbound and silent; registered and saved → applied', async () => {
552+
const walled = new SettingsService({ env: {} });
553+
await bootWithSettings(walled);
554+
const warns = (mockContext.logger.warn as ReturnType<typeof vi.fn>).mock.calls.map((c) => String(c[0]));
555+
expect(warns.filter((w) => w.includes('branding'))).toEqual([]);
556+
expect(setAppNameSpy).not.toHaveBeenCalled();
557+
setAppNameSpy.mockRestore();
558+
559+
const single = new SettingsService({ env: {} });
560+
single.registerManifest(brandingSettingsManifest);
561+
await single.set('branding', 'workspace_name', 'Acme Single');
562+
await bootWithSettings(single);
563+
expect(setAppNameSpy).toHaveBeenCalledWith('Acme Single');
564+
});
520565
});
521566

522567
/**

‎packages/plugins/plugin-auth/src/auth-plugin.ts‎

Lines changed: 30 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1112,12 +1112,36 @@ export class AuthPlugin implements Plugin {
11121112
);
11131113
}
11141114
};
1115-
await applyBrand();
1116-
if (typeof settings.subscribe === 'function') {
1117-
settings.subscribe('branding', () => {
1118-
void applyBrand();
1119-
});
1120-
ctx.logger.info('Auth: bound appName to settings namespace=branding');
1115+
// [#24766] Bound only where the settings service LISTS the
1116+
// `branding` namespace — the test the runtime-config door makes.
1117+
// Under a walled tenancy posture service-settings withholds it
1118+
// (`UNWALLED_ONLY_NAMESPACES`): there is no value to bind, and
1119+
// the read would fail into the warn above on every correct walled
1120+
// boot. The configured `appName` (e.g. `OS_APP_NAME`) stays in
1121+
// force, which is all a walled read of organization-less rows
1122+
// ever produced. A service with no listing is treated the same.
1123+
let brandingOffered = false;
1124+
try {
1125+
const listManifests = (settings as {
1126+
listManifests?: () => ReadonlyArray<{ namespace?: unknown } | undefined>;
1127+
}).listManifests;
1128+
brandingOffered = typeof listManifests === 'function'
1129+
&& listManifests.call(settings).some((m) => m?.namespace === 'branding');
1130+
} catch (err: any) {
1131+
// Kept local so the locale bindings below still run.
1132+
ctx.logger.warn(
1133+
'Auth: could not list settings namespaces, branding.workspace_name not bound: ' +
1134+
(err?.message ?? err),
1135+
);
1136+
}
1137+
if (brandingOffered) {
1138+
await applyBrand();
1139+
if (typeof settings.subscribe === 'function') {
1140+
settings.subscribe('branding', () => {
1141+
void applyBrand();
1142+
});
1143+
ctx.logger.info('Auth: bound appName to settings namespace=branding');
1144+
}
11211145
}
11221146

11231147
// #2815 — bind the auth SMS locale to the deployment default

‎packages/services/service-settings/src/settings-branding-posture.pin.test.ts‎

Lines changed: 50 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@
2222

2323
import { describe, expect, it, vi } from 'vitest';
2424
import { ObjectKernel } from '@objectstack/core';
25+
import { ObjectQL } from '@objectstack/objectql';
26+
import { SETUP_APP, SETUP_NAV_CONTRIBUTIONS } from '@objectstack/platform-objects';
2527
import type { IHttpRequest, IHttpResponse, IHttpServer, RouteHandler } from '@objectstack/spec/contracts';
2628
import { SettingsServicePlugin } from './settings-service-plugin.js';
2729
import type { SettingsService } from './settings-service.js';
@@ -64,6 +66,10 @@ async function boot(tenancy: Tenancy) {
6466
throw new Error('tenancy backend unavailable');
6567
});
6668
}
69+
// Every manifest the plugin registers — its own, and the Setup entries it
70+
// contributes with an offered namespace.
71+
const registered: unknown[] = [];
72+
kernel.registerService('manifest', { register: (m: unknown) => { registered.push(m); } });
6773
const getService = kernel.getService.bind(kernel);
6874
const registerService = kernel.registerService.bind(kernel);
6975
const warnings: string[] = [];
@@ -109,7 +115,35 @@ async function boot(tenancy: Tenancy) {
109115
const getBranding = () =>
110116
call('GET /api/settings/:namespace', { method: 'GET', params: { namespace: 'branding' } as any });
111117

112-
return { service, listed, putBranding, getBranding, warnings };
118+
return { service, listed, putBranding, getBranding, warnings, setupNavIds: () => mergedSetupNavIds(registered) };
119+
}
120+
121+
/**
122+
* The Setup app's navigation as the registry serves it: the shell and
123+
* `@objectstack/platform-objects`' own contributions, plus whatever this boot
124+
* registered, folded by the real `applyNavContributions`. So the static half
125+
* (`SETUP_NAV_CONTRIBUTIONS` no longer carrying the entry) and the contributed
126+
* half are judged together, on the merged tree the console reads.
127+
*/
128+
function mergedSetupNavIds(registered: unknown[]): { ids: string[]; configuration: string[] } {
129+
const engine = new ObjectQL();
130+
engine.registry.logLevel = 'silent';
131+
engine.registerApp({
132+
id: 'com.objectstack.test.setup-shell', namespace: 'sys', version: '0.0.0', type: 'plugin', scope: 'system',
133+
name: 'Setup shell (fixture)', apps: [SETUP_APP], navigationContributions: SETUP_NAV_CONTRIBUTIONS,
134+
});
135+
for (const m of registered) engine.registerApp(m);
136+
const ids: string[] = [];
137+
const walk = (items: any[] | undefined) => {
138+
for (const item of items ?? []) {
139+
if (item?.id) ids.push(item.id);
140+
walk(item?.children);
141+
}
142+
};
143+
const navigation = (engine.registry.getApp('setup') as { navigation?: any[] } | undefined)?.navigation;
144+
walk(navigation);
145+
const group = (navigation ?? []).find((item) => item?.id === 'group_configuration');
146+
return { ids, configuration: (group?.children ?? []).map((item: any) => item?.id) };
113147
}
114148

115149
describe('the Branding namespace under a walled posture (#24766)', () => {
@@ -137,6 +171,13 @@ describe('the Branding namespace under a walled posture (#24766)', () => {
137171
expect(state.status).toBe(404);
138172
expect(state.body.error.code).toBe('UNKNOWN_NAMESPACE');
139173
});
174+
175+
it('Setup serves no Branding entry, while its sibling settings entries stay', async () => {
176+
const { ids } = (await boot({ kind: 'service', posture })).setupNavIds();
177+
expect(ids).not.toContain('nav_settings_branding');
178+
// Control: the fold is real and the Configuration group is populated.
179+
expect(ids).toEqual(expect.arrayContaining(['nav_settings_company', 'nav_settings_localization']));
180+
});
140181
});
141182
}
142183
});
@@ -158,6 +199,14 @@ describe('the Branding namespace where no wall stands (#24766)', () => {
158199
expect(read.status).toBe(200);
159200
expect(read.body.data.values.workspace_name).toMatchObject({ value: 'Acme', source: 'tenant' });
160201
});
202+
203+
it(`${label}: Setup serves the Branding entry exactly once, in Configuration`, async () => {
204+
const { ids, configuration } = (await boot(tenancy)).setupNavIds();
205+
expect(ids.filter((id) => id === 'nav_settings_branding')).toHaveLength(1);
206+
// In the Configuration group, after the platform-objects entries.
207+
expect(configuration).toContain('nav_settings_branding');
208+
expect(configuration.indexOf('nav_settings_branding')).toBeGreaterThan(configuration.indexOf('nav_settings_feature_flags'));
209+
});
161210
}
162211
});
163212

‎packages/services/service-settings/src/settings-service-plugin.ts‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,35 @@ import { settingsBuiltinTranslations } from './translations/index.js';
7878
*/
7979
const UNWALLED_ONLY_NAMESPACES: ReadonlySet<string> = new Set(['branding']);
8080

81+
/**
82+
* The Setup entry of each {@link UNWALLED_ONLY_NAMESPACES} member, contributed
83+
* WITH the namespace (#24766, the amendment's "no Setup entry" half).
84+
*
85+
* It used to be a static item of `@objectstack/platform-objects`'
86+
* `SETUP_NAV_CONTRIBUTIONS`, so a walled deployment listed a page the server
87+
* does not serve. It now lives and dies with the capability — the
88+
* `@objectstack/mcp` `connect-ui.ts` pattern (cloud ADR-0009): registered
89+
* through the `manifest` service from the same `kernel:ready` step that offers
90+
* the namespace, and never where the namespace is withheld. The labels stay
91+
* keyed `apps.setup.navigation.<id>` in platform-objects' bundles, which is
92+
* where the console resolves every Setup entry, contributed or static.
93+
*
94+
* A factory, not a shared literal: the nav fold pushes contributed items into
95+
* the merged tree by reference.
96+
*/
97+
const UNWALLED_ONLY_SETUP_NAV: Readonly<Record<string, () => Record<string, unknown>>> = {
98+
branding: () => ({
99+
id: 'nav_settings_branding',
100+
type: 'url',
101+
label: 'Branding',
102+
url: '/apps/setup/system/settings/branding',
103+
icon: 'palette',
104+
}),
105+
};
106+
107+
/** The bundle id the Setup entries register under — distinct from the plugin's own manifest. */
108+
const UNWALLED_ONLY_SETUP_NAV_BUNDLE_ID = `${SETTINGS_PLUGIN_ID}.setup-nav`;
109+
81110
/** Configuration options for the SettingsServicePlugin. */
82111
export interface SettingsServicePluginOptions {
83112
/**
@@ -574,6 +603,32 @@ export class SettingsServicePlugin implements Plugin {
574603
this.service!.registerManifest(m);
575604
this.registerActionHandlers(m.namespace);
576605
}
606+
// The Setup entries travel with the namespaces just offered — and only then.
607+
const items = this.heldUntilPostureKnown
608+
.map((m) => UNWALLED_ONLY_SETUP_NAV[m.namespace]?.())
609+
.filter((item): item is Record<string, unknown> => item !== undefined);
610+
if (items.length === 0) return;
611+
let manifestService: { register(m: unknown): void } | undefined;
612+
try {
613+
manifestService = ctx.getService<{ register(m: unknown): void }>('manifest');
614+
} catch {
615+
// No manifest service (lean kernels, tests): no app registry to contribute to.
616+
return;
617+
}
618+
manifestService?.register?.({
619+
id: UNWALLED_ONLY_SETUP_NAV_BUNDLE_ID,
620+
namespace: 'sys',
621+
version: SETTINGS_PLUGIN_VERSION,
622+
type: 'plugin',
623+
scope: 'system',
624+
name: 'Settings Service — Setup entries',
625+
description: 'Setup navigation for the settings namespaces offered only where no organization wall stands.',
626+
navigationContributions: [
627+
// After platform-objects' Configuration entries (priority 100), before
628+
// later contributors' (service-messaging, 120).
629+
{ app: 'setup', group: 'group_configuration', priority: 110, items },
630+
],
631+
});
577632
}
578633

579634
/**

0 commit comments

Comments
 (0)