feat(app-shell,data-objectstack): the designer states its package on the publish step (#5420)

.changeset/designer-publish-package-binding.md

@@ -0,0 +1,29 @@
+---
+'@object-ui/data-objectstack': minor
+'@object-ui/app-shell': minor
+---
+
+The metadata designer states its package on the publish step, not only on the save (#5420)
+
+Studio's designer save→publish loop bound the draft to a software package on the
+save (`PUT ?mode=draft&package=<id>`) and then sealed it with a publish that named
+no package at all. `objectstack#10354` (shipped in `@objectstack/rest` 17.2.0) taught
+`POST /meta/:type/:name/publish` to accept `?package=<id>`, so the second call can now
+state the same binding the first one already states.
+
+- `MetadataClient.publish()` accepts `packageId` and sends `?package=<id>`, the same
+ wire spelling and the same `encodeURIComponent` treatment `save()` gives it.
+- `MetadataResourceEditPage` reads the binding for BOTH steps from one derivation
+ (`readActivePackageBinding`), so the two calls of one loop cannot drift apart. The
+ `?package=all` "show everything" scope keeps folding to "no package".
+
+The parameter is **omitted**, never sent empty, when the designer holds no binding.
+Empty and absent are the same to the framework's normaliser today, but absent is the
+shape the save door already followed, and the framework's promotion path branches on
+the key being present downstream.
+
+What this buys is **reachability**, not speed: it lets `#9612`'s package-closure
+narrowing at the runtime publish gate fire on an HTTP-driven promotion at all. That
+narrowing has a second, independent gate this does not touch — objects carrying no
+`_packageId` provenance are kept unconditionally — so on a tenant-authored overlay
+corpus stating the package still narrows nothing.

packages/app-shell/src/views/metadata-admin/ResourceEditPage.publishPackageBinding.test.tsx

@@ -0,0 +1,213 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+
+/**
+ * The designer's save->publish loop states ONE package, in BOTH steps —
+ * objectui#5420, the consumer half of objectstack#10354.
+ *
+ * ## The loop this pins
+ *
+ * `MetadataResourceEditPage` is the designer whose Save writes a draft
+ * (`PUT ?mode=draft&package=<id>`) and whose Publish seals it
+ * (`POST .../publish`). Before this card the second call named no package at
+ * all, so #9612's package-closure narrowing at the runtime publish gate could
+ * never fire on an HTTP-driven promotion. Now both steps read the binding from
+ * `readActivePackageBinding()` — one derivation, so the two calls of one loop
+ * cannot drift apart.
+ *
+ * ## The acceptance criterion, restated
+ *
+ * "The designer states the binding it already knows, so the narrowing is
+ * REACHABLE." Explicitly NOT "publishing got faster": narrowing has a second,
+ * independent gate this does not touch (`narrowObjectsToPackageClosure` keeps
+ * every object carrying no `_packageId` provenance, unconditionally, and a
+ * tenant-authored overlay corpus carries none), so on such a corpus stating the
+ * package narrows nothing. Nothing here asserts a latency claim.
+ *
+ * ## Which assertions survive a revert, and why the pair is needed
+ *
+ * The BOUND case fails on a revert — reverted, `doPublish` calls
+ * `client.publish(type, name)` with no third argument at all, so both the
+ * "options is an object" and the "packageId equals the save's value" pins go
+ * red.
+ *
+ * The UNBOUND case's key-absence pin (`not.toHaveProperty('packageId')`) would
+ * ALSO pass on a revert — absence is exactly what the old door did, and no
+ * absence assertion can distinguish those two worlds by itself. It is not
+ * aimed at the revert: it is the counter-probe for the other failure mode, the
+ * one a lone "publish now sends the package" test is trivially satisfiable by,
+ * namely always sending it. Its revert-sensitive companion sits beside it in
+ * the same case: `expect(options).toBeTypeOf('object')` is red on a revert
+ * (undefined) and green on both correct and always-send, so the two together
+ * separate all three worlds. Both directions run in the same file, as the card
+ * requires.
+ */
+
+import '@testing-library/jest-dom/vitest';
+import * as React from 'react';
+import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
+import { render, screen, fireEvent, cleanup, waitFor } from '@testing-library/react';
+import { MemoryRouter } from 'react-router-dom';
+
+const PAGE = {
+ name: 'home',
+ label: 'Home',
+ type: 'home',
+ template: 'default',
+ regions: [{ name: 'main', components: [{ type: 'text', id: 'b1' }] }],
+};
+
+/**
+ * The two option bags this suite reads. Spelled out (rather than letting
+ * `vi.fn(async () => ...)` infer a zero-argument mock) because the assertions
+ * index into `mock.calls[0]` — an inferred zero-arg mock types that as `[]`,
+ * and every index into it is a compile error the vitest run would never show.
+ */
+type SaveOpts = { force?: boolean; mode?: string; packageId?: string };
+type PublishOpts = { message?: string; packageId?: string };
+
+const mockClient = {
+ list: vi.fn(async () => []),
+ listDrafts: vi.fn(async () => []),
+ layered: vi.fn(async (_type: string, _name: string, _opts?: { packageId?: string }) => ({
+ effective: PAGE,
+ code: PAGE,
+ editable: true,
+ })),
+ // A pending draft is what makes the Publish button exist at all.
+ getDraft: vi.fn(async (_type: string, _name: string, _opts?: { packageId?: string }) => ({ item: PAGE })),
+ get: vi.fn(async () => null),
+ save: vi.fn(async (_type: string, _name: string, _item: unknown, _opts?: SaveOpts) => ({})),
+ publish: vi.fn(async (_type: string, _name: string, _opts?: PublishOpts) => ({
+ success: true,
+ version: 4,
+ })),
+ reset: vi.fn(async () => ({})),
+ references: vi.fn(async () => []),
+};
+
+vi.mock('./useMetadata', async (importOriginal) => {
+ const mod = await importOriginal<typeof import('./useMetadata')>();
+ return {
+ ...mod,
+ useMetadataClient: () => mockClient,
+ useMetadataTypes: () => ({
+ entries: [{ type: 'page', name: 'page', label: 'Page', allowOrgOverride: true }],
+ }),
+ };
+});
+
+import { MetadataResourceEditPage } from './ResourceEditPage';
+import { registerMetadataPreview, getMetadataPreview } from './preview-registry';
+
+/**
+ * Canvas stand-in. Only job: hand the test a way to dirty the draft, which is
+ * what arms the real save door. Turning a canvas gesture into a patch is
+ * `PageBlockCanvas`'s own concern and is tested there.
+ */
+function StubPageCanvas({ onPatch }: { onPatch?: (patch: Record<string, unknown>) => void }) {
+ return (
+ <button type="button" onClick={() => onPatch?.({ label: 'Edited in the designer' })}>
+ patch the draft
+ </button>
+ );
+}
+
+const realPagePreview = getMetadataPreview('page');
+
+/** Put the package scope on the real URL — the same place the loop reads it. */
+function atPackageScope(search: string) {
+ window.history.replaceState(null, '', `/metadata/page/home${search}`);
+ return `/metadata/page/home${search}`;
+}
+
+beforeEach(() => {
+ for (const fn of Object.values(mockClient)) (fn as unknown as { mockClear: () => void }).mockClear();
+ registerMetadataPreview('page', StubPageCanvas as never);
+});
+
+afterEach(() => {
+ cleanup();
+ if (realPagePreview) registerMetadataPreview('page', realPagePreview);
+ window.history.replaceState(null, '', '/');
+});
+
+function renderAt(entry: string) {
+ render(
+ <MemoryRouter initialEntries={[entry]}>
+ <MetadataResourceEditPage type="page" name="home" />
+ </MemoryRouter>,
+ );
+}
+
+/**
+ * Two doors call the SAME `doPublish` — the toolbar button and the
+ * "pending changes" banner button. Both are asserted present so a future
+ * refactor cannot quietly leave one of them on a second publish path.
+ */
+function publishButtons() {
+ const all = screen.getAllByRole('button', { name: /^Publish$/ });
+ expect(all.length).toBe(2);
+ return all;
+}
+const publishButton = () => publishButtons()[0]!;
+
+/** Dirty the draft and let the real save door fire (autosave, 1500 ms). */
+async function saveOnce() {
+ fireEvent.click(await screen.findByRole('button', { name: 'patch the draft' }));
+ await waitFor(() => expect(mockClient.save).toHaveBeenCalled(), { timeout: 8000 });
+}
+
+describe('MetadataResourceEditPage — save and publish state ONE package (#5420)', () => {
+ it('bound: publish states the SAME id the save states, from the same source', async () => {
+ renderAt(atPackageScope('?package=com.example.showcase'));
+ await waitFor(() => expect(publishButton()).toBeInTheDocument(), { timeout: 8000 });
+
+ await saveOnce();
+ const saveOpts = mockClient.save.mock.calls[0]![3];
+ expect(saveOpts).toMatchObject({ mode: 'draft', packageId: 'com.example.showcase' });
+
+ await waitFor(() => expect(publishButton()).toBeEnabled(), { timeout: 8000 });
+ fireEvent.click(publishButton());
+ await waitFor(() => expect(mockClient.publish).toHaveBeenCalled(), { timeout: 8000 });
+
+ const [type, name, publishOpts] = mockClient.publish.mock.calls[0]!;
+ expect([type, name]).toEqual(['page', 'home']);
+ // One value, one spelling: byte-identical to what the save stated.
+ expect(publishOpts).toEqual({ packageId: 'com.example.showcase' });
+ expect(publishOpts?.packageId).toBe(saveOpts?.packageId);
+ });
+
+ it('unbound: no package on the URL — the key is ABSENT on the publish, not empty', async () => {
+ renderAt(atPackageScope(''));
+ await waitFor(() => expect(publishButton()).toBeInTheDocument(), { timeout: 8000 });
+ await waitFor(() => expect(publishButton()).toBeEnabled(), { timeout: 8000 });
+
+ fireEvent.click(publishButton());
+ await waitFor(() => expect(mockClient.publish).toHaveBeenCalled(), { timeout: 8000 });
+
+ const publishOpts = mockClient.publish.mock.calls[0]![2];
+ // Revert-sensitive half: reverted, there is no third argument at all.
+ expect(publishOpts).toBeTypeOf('object');
+ // Always-send-sensitive half: the key must be absent, never `''`.
+ expect(publishOpts).not.toHaveProperty('packageId');
+ expect(Object.keys(publishOpts!)).toEqual([]);
+ });
+
+ it("unbound: `?package=all` is the show-everything scope, not a package id", async () => {
+ renderAt(atPackageScope('?package=all'));
+ await waitFor(() => expect(publishButton()).toBeInTheDocument(), { timeout: 8000 });
+
+ // The save folds `all` away too — pin both halves of the fold in one run so
+ // the two calls cannot disagree about what `all` means.
+ await saveOnce();
+ expect(mockClient.save.mock.calls[0]![3]).not.toHaveProperty('packageId');
+
+ await waitFor(() => expect(publishButton()).toBeEnabled(), { timeout: 8000 });
+ fireEvent.click(publishButton());
+ await waitFor(() => expect(mockClient.publish).toHaveBeenCalled(), { timeout: 8000 });
+
+ const publishOpts = mockClient.publish.mock.calls[0]![2];
+ expect(publishOpts).toBeTypeOf('object');
+ expect(publishOpts).not.toHaveProperty('packageId');
+ });
+});

packages/app-shell/src/views/metadata-admin/ResourceEditPage.tsx

@@ -229,6 +229,49 @@ function extractDraftBody(
  : null;
 }
 
+/**
+ * The software-package binding this editor is authoring under, read from the
+ * ONE place the save->publish loop states it: `?package=` on the editor URL.
+ *
+ * ## Why this is a function and not two inline reads
+ *
+ * Both steps of the loop send this value — `doSave` binds the draft row to the
+ * package (`PUT ?package=`), and since objectstack#10354 `doPublish` states the
+ * same package on the promotion (`POST .../publish?package=`) so #9612's
+ * package-closure narrowing at the runtime publish gate is reachable from an
+ * HTTP-driven promotion at all. One value, one spelling, both steps — which
+ * means one derivation too. A second inline copy in the publish path would be
+ * free to drift from the save path (most easily on the `'all'` fold below),
+ * and the two calls would then disagree about which package the edit belongs
+ * to while both looking correct in isolation.
+ *
+ * ## The `'all'` fold
+ *
+ * `?package=all` is the metadata list's "show everything" scope, NOT a package
+ * literally named `all`; the framework's normaliser folds `all` and the empty
+ * value together to mean "env-local overlay, no package". Folded here to
+ * `undefined` so both callers OMIT the parameter rather than sending it empty.
+ * The two are the same to that normaliser today, so this is not a behaviour
+ * difference against the current server — omit-when-unbound is simply the
+ * shape this door already had, and the loop's two calls must not disagree.
+ *
+ * Read at call time rather than per render because the editor URL's package
+ * scope can move under the component (`setSearchParams`), and the value that
+ * must be stated is the one in force when the request is issued.
+ *
+ * Deliberately NOT `ownerPackageId` (the router-read `?package=` used to scope
+ * layered/draft READS): that one does not fold `'all'`, so reusing it here
+ * would send `package=all` as if it were a package id.
+ */
+function readActivePackageBinding(): string | undefined {
+ try {
+ const p = new URLSearchParams(window.location.search).get('package');
+ return p && p !== 'all' ? p : undefined;
+ } catch {
+ return undefined;
+ }
+}
+
 /**
  * Decide whether the validation-diagnostics banner should render at all.
  *
@@ -1302,14 +1345,7 @@ function MetadataResourceEditPageImpl({
  // real package scope is carried in the URL (`?package=`). The backend
  // stamps it on create and preserves an existing binding on update, so
  // env-local overlays (no `?package=`) are unaffected.
- const activePackage = (() => {
- try {
- const p = new URLSearchParams(window.location.search).get('package');
- return p && p !== 'all' ? p : undefined;
- } catch {
- return undefined;
- }
- })();
+ const activePackage = readActivePackageBinding();
  await client.save<any>(type, savedName, itemToSave, {
  force,
  mode: 'draft',
@@ -1461,7 +1497,16 @@ function MetadataResourceEditPageImpl({
  setPublishing(true);
  setError(null);
  try {
- await client.publish<any>(type, name);
+ // State the SAME package the save step already stated — read from the
+ // same single source, so the two calls of one loop can never disagree.
+ // Absent (not empty) when the designer holds no binding: the framework
+ // branches on the KEY BEING PRESENT downstream, where a present-but-null
+ // package pins the draft lookup to unbound rows and a packaged draft
+ // stops being found (`no_draft`) — see objectstack#10354's own warning.
+ const activePackage = readActivePackageBinding();
+ await client.publish<any>(type, name, {
+ ...(activePackage ? { packageId: activePackage } : {}),
+ });
  const [lay, draftResp] = await Promise.all([
  client.layered<any>(type, name),
  client.getDraft<any>(type, name).catch(() => null),