fix(app-shell): a package-door permission save keeps the pending draft's values

ui/packages/app-shell/src/views/metadata-admin/PermissionMatrixEditor.packageDoorDraftBase-26837.test.tsx

@@ -0,0 +1,218 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+
+/**
+ * The package door's save merges onto the PENDING DRAFT when one exists, not
+ * onto the published body (objectstack#26837).
+ *
+ * ## The defect these pins reach
+ *
+ * `doSave`'s package-door arm re-reads the set and merges this package's slice
+ * onto that read with `mergePermissionSlice`, which takes only the facets the
+ * editor can author from the edited body and everything else from the base.
+ * The base was `client.layered(...).effective` — the PUBLISHED body — while the
+ * save itself writes the package DRAFT. So a draft another door wrote (the
+ * card's reproduction: `PUT …?mode=draft&package=P` with `description` and
+ * `packageId`) lost those keys on the next Studio edit: the load displayed the
+ * draft, one checkbox autosaved, and the PUT carried the published values (or
+ * none) in their place. 200, no error.
+ *
+ * ## Why the pin drives the REAL editor
+ *
+ * `mergePermissionSlice` is correct for any base it is handed — its own pins
+ * (`permission-slice.authoredKeys.test.ts`, `permission-slice.facetDrift.test.ts`)
+ * call it directly and could never see this. The defect is the CHOICE of base,
+ * which only `doSave` makes, so these pins run the real page, the real load and
+ * the real autosave (objectui#11787: the package door has no Save button).
+ *
+ * ## The arms, and what each one's absence is read against
+ *
+ *  1. A pending draft whose non-authored keys differ from the published body:
+ *     every one of them is on the wire byte-for-byte, out-of-scope rows come
+ *     from the draft, and the edited row lands.
+ *  2. No pending draft: the base is still the published `effective`, so the
+ *     published body's own keys and other packages' rows are on the wire —
+ *     the control that the draft arm is not "the edited body, wholesale".
+ *  3. The save-time DRAFT read rejects: the save is refused on the same
+ *     channel as objectui#9420's layered refusal, `client.save` is never
+ *     called, and the control `getDraftCalls === 2` proves the read ran.
+ */
+
+import '@testing-library/jest-dom/vitest';
+import { afterEach, describe, expect, it, vi } from 'vitest';
+import { cleanup, fireEvent, render, screen, waitFor, within } from '@testing-library/react';
+import { MemoryRouter } from 'react-router-dom';
+
+/**
+ * The published record. `a_account` is the door's own package; `b_order` and
+ * `b_order.total` are another package's grants.
+ */
+const PUBLISHED = {
+  name: 'sales_perms',
+  label: 'Sales',
+  description: 'Published description',
+  objects: {
+    a_account: { allowRead: true, allowCreate: true },
+    b_order: { allowRead: true },
+  },
+  fields: { 'b_order.total': { readable: true, editable: false } },
+};
+
+/**
+ * The pending package draft another door wrote — every key the editor cannot
+ * author differs from {@link PUBLISHED}, and so does an out-of-scope row, so a
+ * value taken from the wrong base cannot pass for the right one.
+ */
+const DRAFT = {
+  name: 'sales_perms',
+  label: 'Sales',
+  description: 'Set through the API',
+  packageId: 'app.a',
+  objects: {
+    a_account: { allowRead: true, allowCreate: true },
+    b_order: { allowRead: true, allowEdit: true, viewAllRecords: true },
+  },
+  fields: { 'b_order.total': { readable: true, editable: true } },
+};
+
+/** The facets `EDITOR_AUTHORED_KEYS` does not name — the ones the base supplies. */
+const NON_AUTHORED = ['description', 'packageId'] as const;
+
+/** Distinctive slice of the refusal the author sees on the page's error strip. */
+const REFUSAL = /could not be re-read/;
+
+interface Server {
+  saved: Array<Record<string, unknown>>;
+  savedOpts: Array<Record<string, unknown> | undefined>;
+  /** `getDraft` calls seen so far — #1 is the load, #2 is `doSave`'s re-read. */
+  getDraftCalls: number;
+}
+
+function freshServer(): Server {
+  return { saved: [], savedOpts: [], getDraftCalls: 0 };
+}
+
+function makeClient(server: Server, opts: { draft: Record<string, unknown> | null; rejectSaveTimeDraft?: boolean }) {
+  return {
+    layered: async () => ({ effective: PUBLISHED, code: null, overlay: null, overlayScope: null }),
+    getDraft: async () => {
+      server.getDraftCalls += 1;
+      if (opts.rejectSaveTimeDraft && server.getDraftCalls > 1) throw new Error('draft read failed');
+      return opts.draft ? { type: 'permission', name: 'sales_perms', item: opts.draft, version: 'v1' } : null;
+    },
+    // The package declares `a_account` only.
+    list: async (type: string) => (type === 'object' ? [{ item: { name: 'a_account' } }] : []),
+    get: async () => null,
+    save: async (
+      _type: string,
+      _name: string,
+      payload: Record<string, unknown>,
+      saveOpts?: Record<string, unknown>,
+    ) => {
+      server.saved.push(payload);
+      server.savedOpts.push(saveOpts);
+      return payload;
+    },
+  } as any;
+}
+
+let clientImpl: any;
+
+vi.mock('./useMetadata', () => ({
+  useMetadataClient: () => clientImpl,
+  useMetadataTypes: () => ({
+    loading: false,
+    error: null,
+    entries: [{ type: 'permission', label: 'Permission', allowEnvironmentOverlay: true }],
+  }),
+}));
+
+vi.mock('./AssignedUsersSection', () => ({ AssignedUsersSection: () => null }));
+
+import { PermissionMatrixEditPage } from './PermissionMatrixEditor';
+
+afterEach(cleanup);
+
+function renderPackageDoor() {
+  return render(
+    <MemoryRouter>
+      <PermissionMatrixEditPage type="permission" name="sales_perms" packageId="app.a" />
+    </MemoryRouter>,
+  );
+}
+
+/** One ordinary edit: revoke every grant on the door's own `a_account` row. */
+function editAccountRow() {
+  const row = screen.getByText('a_account').closest('tr')!;
+  fireEvent.click(within(row).getByRole('button', { name: 'None' }));
+}
+
+/** The package door autosaves after the shared pause (1.5s). */
+const AUTOSAVE = { timeout: 4000 };
+
+describe('PermissionMatrixEditPage — the PACKAGE door saves onto the pending draft (objectstack#26837)', () => {
+  it('keeps every key the editor does not author from the pending draft, byte-for-byte', async () => {
+    const server = freshServer();
+    clientImpl = makeClient(server, { draft: DRAFT });
+    renderPackageDoor();
+    await screen.findByText('a_account');
+
+    editAccountRow();
+
+    await waitFor(() => expect(server.saved).toHaveLength(1), AUTOSAVE);
+    const body = server.saved[0] as any;
+
+    // The edited row lands…
+    expect(body.objects.a_account).toEqual({});
+    // …and the draft's non-authored keys survive, not the published ones.
+    for (const key of NON_AUTHORED) expect(body[key]).toEqual(DRAFT[key]);
+    // Out-of-scope rows come from the draft too: it is what this save replaces.
+    expect(body.objects.b_order).toEqual(DRAFT.objects.b_order);
+    expect(body.fields).toEqual(DRAFT.fields);
+    // CONTROL: the save-time draft read ran (#1 is the load).
+    expect(server.getDraftCalls).toBe(2);
+    expect(server.savedOpts[0]).toMatchObject({ mode: 'draft', packageId: 'app.a' });
+    expect(screen.queryByText(REFUSAL)).toBeNull();
+  });
+
+  it('CONTROL — with no pending draft the base is still the published body', async () => {
+    const server = freshServer();
+    clientImpl = makeClient(server, { draft: null });
+    renderPackageDoor();
+    await screen.findByText('a_account');
+
+    editAccountRow();
+
+    await waitFor(() => expect(server.saved).toHaveLength(1), AUTOSAVE);
+    const body = server.saved[0] as any;
+
+    expect(body.objects.a_account).toEqual({});
+    expect(body.description).toBe(PUBLISHED.description);
+    expect('packageId' in body).toBe(false);
+    expect(body.objects.b_order).toEqual(PUBLISHED.objects.b_order);
+    expect(body.fields).toEqual(PUBLISHED.fields);
+    expect(screen.queryByText(REFUSAL)).toBeNull();
+  });
+
+  it('REFUSES the save when the save-time draft read rejects, rather than fall back to the published body', async () => {
+    const server = freshServer();
+    clientImpl = makeClient(server, { draft: DRAFT, rejectSaveTimeDraft: true });
+    renderPackageDoor();
+    await screen.findByText('a_account');
+
+    editAccountRow();
+
+    // Settle on either arm — the refusal on screen, or a PUT landing.
+    await waitFor(
+      () => expect(server.saved.length > 0 || screen.queryByText(REFUSAL) !== null).toBe(true),
+      AUTOSAVE,
+    );
+
+    // CONTROL: the save-time draft read really ran and really rejected.
+    expect(server.getDraftCalls).toBe(2);
+    expect(server.saved).toEqual([]);
+    expect(server.savedOpts).toEqual([]);
+    // The same error strip objectui#9420's layered refusal renders into.
+    expect(screen.getByText(REFUSAL)).toBeInTheDocument();
+    expect(screen.queryByTestId('perm-autosaving')).toBeNull();
+  });
+});

ui/packages/app-shell/src/views/metadata-admin/PermissionMatrixEditor.tsx

@@ -266,6 +266,19 @@ function isArtifactBackedLayer(
   );
 }
 
+/**
+ * The editable body of a pending package draft, as `readDraftForSave` served
+ * it (the `{ type, name, item }` envelope, or a bare body), or `null` for "no
+ * draft". Read decorations come off here, at the unwrap, so neither the load
+ * nor the save's merge base carries `_diagnostics` / `_draft` (objectui#8181).
+ */
+function pendingDraftBody(
+  answer: { item?: PermissionSetDraft } | PermissionSetDraft | null,
+): PermissionSetDraft | null {
+  if (!answer) return null;
+  return stripReadDecorations((answer as { item?: PermissionSetDraft }).item ?? answer) as PermissionSetDraft;
+}
+
 /** Localized short label for an OWD value; falls back to the raw value. */
 function owdLabel(t: (k: string) => string, value: string): string {
   const key: Record<string, string> = {
@@ -867,13 +880,10 @@ export function PermissionMatrixEditPage({ type, name, packageId, onDraftSaved,
         // below re-bases on a fresh RAW `layered` read, which drops them — but
         // its `.catch(() => null)` arm falls back to this very body and
         // spreads it into `client.save`, so a failed layered read used to put
-        // `_diagnostics` / `_draft` on the wire. Strip at the unwrap, which is
-        // the one place the served envelope becomes an editable draft.
-        const draftBody = pendingDraft
-          ? (stripReadDecorations(
-              (pendingDraft as any).item ?? pendingDraft,
-            ) as PermissionSetDraft)
-          : null;
+        // `_diagnostics` / `_draft` on the wire. Strip at the unwrap,
+        // `pendingDraftBody`, the one function through which a served envelope
+        // becomes an editable body — here and in `doSave`'s merge base.
+        const draftBody = pendingDraftBody(pendingDraft);
         // Draft wins over the published baseline for display (D6).
         const effective: PermissionSetDraft = (draftBody ?? lay?.effective ??
           lay?.code ?? { name, objects: {} }) as PermissionSetDraft;
@@ -1280,19 +1290,36 @@ export function PermissionMatrixEditPage({ type, name, packageId, onDraftSaved,
         // purpose: a set that exists only as a package draft (what the Studio
         // Access pillar's "+ New" creates) has no published rows for anyone to
         // lose, and refusing there would block its first save.
+        //
+        // objectstack#26837 — and this door's save WRITES the pending draft, so
+        // when one exists it is the base, not the published body: the facets
+        // the editor cannot author (`description`, `packageId`, any key another
+        // door wrote) come from the draft, exactly as the load displayed it.
+        // Read through the load's own door, at the address this save writes.
+        // The pin the load recorded is left alone, so a draft that moved since
+        // the load is still refused as a version conflict. `readDraftForSave`
+        // resolves a REJECTED read as no answer AND no pin, while a read that
+        // found no draft pins `If-None-Match: *` — so the pair tells a failure
+        // apart from "no draft", and a failure refuses exactly like the
+        // layered re-read: falling back to the published body would drop the
+        // draft's values with a 200.
         let rereadFailed = false;
-        const fresh = await client
-          .layered<PermissionSetDraft>(type, payload.name)
-          // Kept on `.catch` so the rejection can never escape as an unhandled
-          // one; the refusal is raised below, inside this `try`'s own body, and
-          // reaches the author on the same error channel a failed
-          // `client.save` already uses.
-          .catch(() => {
-            rereadFailed = true;
-            return null;
-          });
+        const [fresh, draftRead] = await Promise.all([
+          client
+            .layered<PermissionSetDraft>(type, payload.name)
+            // Kept on `.catch` so the rejection can never escape as an
+            // unhandled one; the refusal is raised below, inside this `try`'s
+            // own body, and reaches the author on the same error channel a
+            // failed `client.save` already uses.
+            .catch(() => {
+              rereadFailed = true;
+              return null;
+            }),
+          readDraftForSave<{ item?: PermissionSetDraft } | PermissionSetDraft>(client, type, payload.name, packageId),
+        ]);
+        if (draftRead.answer === null && draftRead.pin === null) rereadFailed = true;
         if (rereadFailed) throw new Error(t('perm.save.rereadFailed'));
-        const base = (fresh?.effective ?? payload) as PermissionSetDraft;
+        const base = (pendingDraftBody(draftRead.answer) ?? fresh?.effective ?? payload) as PermissionSetDraft;
         toSave = mergePermissionSlice(base, payload, scope);
       }
       // ADR-0086 P2 (D6/D7). Package door → the set is metadata: write a DRAFT