refactor(app-shell): call owd-sharing's isExternalWider from ObjectSettingsPanel, pinned across both surfaces

.changeset/owd-width-single-home-5477.md

@@ -0,0 +1,25 @@
+---
+'@object-ui/app-shell': patch
+---
+
+`ObjectSettingsPanel` now calls `isExternalWider` from `owd-sharing.ts` instead of
+re-declaring `OWD_WIDTH` and the ADR-0090 D11 width comparison inline (objectui#5477).
+
+`owd-sharing.ts` states its own purpose — it is "the single home" for the pieces the
+per-object Settings tab and the package-level OWD overview must agree on, the D11
+"external ≤ internal" comparison among them — and the Settings tab was the one surface
+not calling it. The two implementations were verified equivalent before the swap, over
+the full domain of both dials plus `undefined`, the rejected legacy aliases and
+prototype-chain keys: 144 input pairs, zero disagreements. The module's extra `!!`
+truthiness guards are redundant at this call site, which already normalizes both dials
+to `''`, and `'' in OWD_WIDTH` is false regardless. So no author-visible verdict
+changes — the same three pairs warn, and the same twenty-two stay calm.
+
+The substantive half is the pin that keeps it that way. `ObjectSettingsPanel.owdAgreement.test.tsx`
+drives BOTH surfaces over the full 5×5 cross-product of the values their dials offer and
+requires all three legs — the Settings tab's rendered warning, the overview's per-row
+error, and `isExternalWider` itself — to agree on every pair, with the violating pairs
+pinned by name so the sweep cannot pass vacuously. Re-inlining a drifted copy into either
+surface, swapping the `(internal, external)` argument order, or refining D11 in only one
+place now fails a test instead of silently leaving the authoring surface enforcing the
+old rule.

packages/app-shell/src/views/studio-design/ObjectSettingsPanel.owdAgreement.test.tsx

@@ -0,0 +1,184 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+
+/**
+ * AGREEMENT PIN — the two OWD authoring surfaces must reach the SAME ADR-0090
+ * D11 verdict (objectui#5477).
+ *
+ * `owd-sharing.ts` declares itself "the single home for the pieces they must
+ * agree on", but nothing held that claim up: `ObjectSettingsPanel` re-declared
+ * `OWD_WIDTH` and the width comparison inline, and no test compared the two
+ * surfaces. The copies happened to be equivalent, so the split cost nothing
+ * *today* — the hazard was a future D11 refinement landing in the module while
+ * the per-object Settings tab, the surface an author actually sets the dial on,
+ * silently kept the old rule.
+ *
+ * Why this is not the tautology it could have been: asserting that the panel
+ * CALLS `isExternalWider` would prove nothing about whether the comparison is
+ * the same one — a re-inlined copy calls nothing at all, and a call spy stays
+ * green when the (internal, external) arguments are swapped. So the assertion
+ * here is behavioural, and it runs across the boundary the drift would cross —
+ * the two SURFACES:
+ *
+ * • `ObjectSettingsPanel` — the per-object Settings tab, one pair at a time.
+ * • `PackageOwdOverviewPanel` — the package-level overview, N rows at once.
+ *
+ * Both are driven over the FULL cross-product of the five values their dials
+ * offer (unset + the four canonical OWD models — 25 pairs). Each surface's
+ * verdict is read off what it RENDERS, never off an internal symbol, and all
+ * three legs — surface A, surface B, and `isExternalWider` itself — must agree
+ * on every pair. Re-inline a drifted copy into either surface, swap the
+ * argument order, or refine D11 in only one place, and this goes red naming
+ * the offending pairs.
+ */
+
+import { describe, it, expect, vi } from 'vitest';
+import { render, within } from '@testing-library/react';
+import React from 'react';
+import { ObjectSettingsPanel } from './ObjectSettingsPanel';
+import { PackageOwdOverviewPanel } from './PackageOwdOverviewPanel';
+import { isExternalWider } from './owd-sharing';
+
+vi.mock('sonner', () => ({ toast: { success: vi.fn(), error: vi.fn() } }));
+
+/**
+ * Everything an author can actually put on either dial: unset, plus the four
+ * canonical OWD models (ADR-0090 D4). `controlled_by_parent` is deliberately
+ * included on BOTH axes — it sits off the width ordering, and "off the axis
+ * never trips D11" is exactly the kind of edge a re-inlined copy gets wrong.
+ */
+const OWD_DIAL_VALUES = ['', 'private', 'public_read', 'public_read_write', 'controlled_by_parent'] as const;
+
+const label = (v: string) => v || '(unset)';
+const pairLabel = (internal: string, external: string) => `${label(internal)} → ${label(external)}`;
+
+const baseDraft = {
+ fields: {
+ name: { type: 'text', label: 'Name' },
+ status: { type: 'select', label: 'Status' },
+ },
+};
+
+/** An object body carrying exactly this OWD pair (unset → key absent). */
+function bodyFor(internal: string, external: string): Record<string, unknown> {
+ const body: Record<string, unknown> = { ...baseDraft };
+ if (internal) body.sharingModel = internal;
+ if (external) body.externalSharingModel = external;
+ return body;
+}
+
+/**
+ * Surface A's verdict, read off the rendered D11 hint. The branch is encoded
+ * twice — amber styling AND the "wider" copy — so both are read and required
+ * to agree; a half-updated branch must not be able to read as "calm".
+ */
+function settingsPanelVerdict(internal: string, external: string): boolean {
+ const view = render(
+ <ObjectSettingsPanel
+ name="leave_request"
+ draft={bodyFor(internal, external)}
+ onPatch={vi.fn()}
+ locale="en-US"
+ />,
+ );
+ const desc = within(view.container).getByTestId('owd-external-desc');
+ const styledAsViolation = /amber/.test(desc.className);
+ const copyIsViolation = /WIDER than the internal sharing model/.test(desc.textContent ?? '');
+ view.unmount();
+ if (styledAsViolation !== copyIsViolation) {
+ throw new Error(
+ `ObjectSettingsPanel half-rendered the D11 branch for ${pairLabel(internal, external)}: ` +
+ `amber=${styledAsViolation} copy=${copyIsViolation}`,
+ );
+ }
+ return styledAsViolation;
+}
+
+/** Row id for the overview's one-object-per-pair fixture package. */
+const rowName = (internal: string, external: string) =>
+ `owd_${internal || 'unset'}__${external || 'unset'}`;
+
+/** A metadata client serving one object per (internal, external) pair. */
+function makeClient(objects: Record<string, Record<string, unknown>>) {
+ return {
+ list: async (type: string) =>
+ type === 'object' ? Object.keys(objects).map((name) => ({ name, label: name })) : [],
+ listDrafts: async () => [],
+ layered: async (_type: string, name: string) => ({ effective: objects[name] ?? {}, code: null }),
+ getDraft: async () => null,
+ save: async () => ({}),
+ } as unknown as React.ComponentProps<typeof PackageOwdOverviewPanel>['client'];
+}
+
+/**
+ * Surface B's verdicts for every pair, in ONE render: the overview flags a row
+ * by rendering `owd-error-<name>` beside its external dial, so presence of that
+ * node IS the row's verdict.
+ */
+async function overviewVerdicts(): Promise<(internal: string, external: string) => boolean> {
+ const objects: Record<string, Record<string, unknown>> = {};
+ for (const internal of OWD_DIAL_VALUES) {
+ for (const external of OWD_DIAL_VALUES) {
+ objects[rowName(internal, external)] = bodyFor(internal, external);
+ }
+ }
+ const view = render(
+ <PackageOwdOverviewPanel
+ client={makeClient(objects)}
+ packageId="com.example.owd_agreement_pin"
+ locale="en-US"
+ />,
+ );
+ const scope = within(view.container);
+ // The table is filled from an async load; wait for the last row to exist.
+ await scope.findByTestId(`owd-row-${rowName('controlled_by_parent', 'controlled_by_parent')}`);
+ const flagged = new Set(
+ Array.from(view.container.querySelectorAll('[data-testid^="owd-error-"]')).map((el) =>
+ (el.getAttribute('data-testid') ?? '').replace('owd-error-', ''),
+ ),
+ );
+ // Every pair must have produced a row — otherwise "not flagged" would be
+ // indistinguishable from "never rendered", and the sweep would go vacuous.
+ for (const internal of OWD_DIAL_VALUES) {
+ for (const external of OWD_DIAL_VALUES) {
+ scope.getByTestId(`owd-row-${rowName(internal, external)}`);
+ }
+ }
+ return (internal, external) => flagged.has(rowName(internal, external));
+}
+
+describe('OWD D11 — the two authoring surfaces agree (objectui#5477)', () => {
+ it('flags exactly the same (internal, external) pairs on both surfaces and in owd-sharing', async () => {
+ const overview = await overviewVerdicts();
+
+ const disagreements: string[] = [];
+ const violations: string[] = [];
+ for (const internal of OWD_DIAL_VALUES) {
+ for (const external of OWD_DIAL_VALUES) {
+ const settings = settingsPanelVerdict(internal, external);
+ const overviewSaysWider = overview(internal, external);
+ const shared = isExternalWider(internal || undefined, external || undefined);
+ if (settings) violations.push(pairLabel(internal, external));
+ if (settings !== overviewSaysWider || settings !== shared) {
+ disagreements.push(
+ `${pairLabel(internal, external)}: ObjectSettingsPanel=${settings} ` +
+ `PackageOwdOverviewPanel=${overviewSaysWider} isExternalWider=${shared}`,
+ );
+ }
+ }
+ }
+
+ expect(disagreements).toEqual([]);
+
+ // Non-vacuity anchor. Three-way agreement alone would still pass if every
+ // leg went silently false (a broken render flags nothing, a broken helper
+ // returns nothing) — so pin WHICH pairs the shared axis calls a violation:
+ // private < public_read < public_read_write, and nothing involving unset or
+ // the off-axis `controlled_by_parent`.
+ expect(violations).toEqual([
+ 'private → public_read',
+ 'private → public_read_write',
+ 'public_read → public_read_write',
+ ]);
+ expect(violations.length).toBeLessThan(OWD_DIAL_VALUES.length ** 2);
+ });
+});

packages/app-shell/src/views/studio-design/ObjectSettingsPanel.tsx

@@ -44,6 +44,7 @@ import { Settings2, ShieldCheck, Sparkles, ToggleRight, X } from 'lucide-react';
 import { getMetadataDefaultInspector } from '../metadata-admin/default-inspector-registry.js';
 import { readFields } from '../metadata-admin/previews/object-fields-io.js';
 import { t, tFormat, type SupportedLocale } from '../metadata-admin/i18n.js';
+import { isExternalWider } from './owd-sharing.js';
 
 export function ObjectSettingsPanel({
  name,
@@ -121,13 +122,18 @@ export function ObjectSettingsPanel({
  // Defaults to private when unset; must never be WIDER than the internal
  // model (ordering: private < public_read < public_read_write — the D7
  // security-posture linter rejects the wider shape at publish).
+ //
+ // The width comparison itself comes from `owd-sharing.ts`, which exists to be
+ // the SINGLE home for the pieces this tab and the package-level OWD overview
+ // (`PackageOwdOverviewPanel`) must agree on. This used to re-declare
+ // `OWD_WIDTH` and the comparison inline; the two were equivalent but nothing
+ // held them together, so a D11 refinement landing in the module would have
+ // left this tab — the surface an author actually sets the dial on — silently
+ // enforcing the old rule (objectui#5477). Note the argument order: the module
+ // takes (internal, external), the reverse of how the violation reads.
  const externalSharingModel =
  typeof draft.externalSharingModel === 'string' ? draft.externalSharingModel : '';
- const OWD_WIDTH: Record<string, number> = { private: 0, public_read: 1, public_read_write: 2 };
- const externalWider =
- externalSharingModel in OWD_WIDTH &&
- sharingModel in OWD_WIDTH &&
- OWD_WIDTH[externalSharingModel] > OWD_WIDTH[sharingModel];
+ const externalWider = isExternalWider(sharingModel, externalSharingModel);
 
  return (
  <div className="flex min-h-0 flex-1 flex-col gap-4 overflow-auto">