fix(rest): give POST /packages/publish an owner on every boot (#7563)

.changeset/packages-publish-mount-or-404.md

@@ -0,0 +1,41 @@
+---
+"@objectstack/rest": patch
+---
+
+Give `POST /api/v1/packages/publish` an owner on every boot (#7563)
+
+On a live showcase boot, `POST /api/v1/packages/publish` answered **405** with
+`Allow: DELETE, GET, HEAD, PATCH`. Not one of those verbs belongs to the publish
+surface — `POST` is the only verb it has ever had. They are `/packages/:id`'s
+method set, offered because with the publish route unmounted that pattern was
+the only registration still matching the path, with `id = "publish"`. A caller
+was told "this path exists, use another method", and every method on offer would
+have operated on a package literally named `publish`.
+
+Two facts produced it, and both are repaired.
+
+The REST package registrar was gated on `ctx.getService('package')` resolving at
+the single instant `RestApiPlugin.start()` ran. `objectstack serve` registers the
+capability providers (`requires: ['marketplace']` → `PackageServicePlugin`)
+*after* `createRestApiPlugin`, and start order follows registration order for
+plugins with no dependency edge between them — so the deployments that do
+compose a package service are precisely the ones that answered "no" at mount
+time. The service is now handed to the registrar as a resolver and read per
+request, which makes the answer independent of composition order instead of
+silently encoding it.
+
+And `POST /packages/publish` has no dispatcher twin, so "not mounted" never
+degraded to the 404 the composition documented — it degraded to a sibling's 405.
+It therefore mounts unconditionally and answers its own honest 404, naming the
+surface rather than a package id, where no package service is composed. The
+other three package routes deliberately do **not** follow: each shadows a live
+dispatcher twin at a byte-identical pattern, so mounting them without a service
+would replace three working routes with a degraded refusal.
+
+The route-ledger ↔ live-mount parity gate (#7526) had this row **pinned** as
+unobservable, reasoned as "the registrar is service-gated and this boot composes
+none". The reason was true and the conclusion was wrong: an unmounted route is
+not automatically an unanswered one. The pin is deleted (the route is now
+observable for real), and the pin rule itself is tightened — a pinned path that
+some *other* pattern answers now fails the gate, because that is the disguise
+the gate already refuses for every unpinned row.

packages/qa/dogfood/test/route-ledger-live-mount-parity.dogfood.test.ts

@@ -113,20 +113,33 @@ function collectLedgerRows(): LedgerRow[] {
  * Rows this boot structurally cannot observe, each with the reason.
  *
  * ⚠️ READ THIS BEFORE ADDING A LINE. A pin is NOT "this route is allowed to be
- * missing" — it is "this boot cannot see it", and the gate asserts BOTH
- * directions of that claim: a pinned row that turns out to be reachable fails
- * too, so the set can only shrink by accident and never grow by accident. The
- * moment a pin starts meaning "we know it is broken", it has become the
- * declaration-instead-of-observation this whole file exists to end. A route
- * that is broken gets fixed or gets an issue, not a line here.
+ * missing" — it is "NOTHING answers this path on this boot", and the gate
+ * asserts BOTH halves of that claim: a pinned row that turns out to be
+ * reachable fails, and (since #7563) so does one whose path is answered by a
+ * DIFFERENT pattern. So the set can only shrink by accident and never grow by
+ * accident. The moment a pin starts meaning "we know it is broken", it has
+ * become the declaration-instead-of-observation this whole file exists to end.
+ * A route that is broken gets fixed or gets an issue, not a line here.
+ *
+ * ## What #7563 taught this list
+ *
+ * `POST /api/v1/packages/publish` used to be pinned here, reasoned as "the
+ * registrar is service-gated and this boot composes no `package` service".
+ * That reason was true and the conclusion was wrong: an unmounted route is not
+ * automatically an UNANSWERED one. With nobody owning the path, the
+ * dispatcher's `/packages/:id` matched it (`id = "publish"`) and the router
+ * answered 405 with THAT route's `Allow` set — the "LEDGERED BUT NOT MOUNTED,
+ * and DISGUISED" failure this gate spells out for every unpinned row, invisible
+ * for the one class that had been excused from the check. A conditional mount
+ * was therefore the one shape the gate could not model, so the pin's second
+ * half below is now checked as strictly as the first. The route itself mounts
+ * unconditionally as of #7563 and needs no pin at all.
  */
 const UNEXERCISED_BY_THIS_BOOT: Record<string, string> = {
  '* /api/v1/auth/**':
  'plugin-auth mounts one rawApp.all() catch-all on Hono directly, so no auth route ever passes through the IHttpServer port. Audited against better-auth\'s live auth.api table by auth-route-ledger.conformance.test.ts instead',
  '* /api/v1/apps/**':
  'the ADR-0121 declarative-endpoint carve-out is a setFallbackHandler seam, not a route — being invisible to a route table is the property that makes it incapable of shadowing one (#5040 §1-C)',
- 'POST /api/v1/packages/publish':
- 'the marketplace publish registrar mounts only when a `package` service occupies the slot (direct-mount-composition.ts); the showcase registers none. Its presence half is already guarded by rest-route-ledger.conformance.test.ts against a capably-mocked RestServer',
 };
 
 /** Segments a probe path uses for `:params` — must match no literal segment. */
@@ -282,6 +295,57 @@ describe('route ledger ↔ live mount parity (#7526)', () => {
  expect(stale, `\n${stale.join('\n')}\n`).toEqual([]);
  });
 
+ // ── …and a pin means NOTHING answers, not "something else answers" (#7563) ─
+ //
+ // The half the pin list was missing. "This boot does not mount it" and "this
+ // boot does not ANSWER it" are different claims, and only the second one
+ // makes a pin safe: a pinned path some other registration matches hands the
+ // caller that route's answer, which is strictly more misleading than the 404
+ // the pin implies. `POST /api/v1/packages/publish` was pinned here and
+ // absorbed by `/packages/:id` for exactly that reason (#7563).
+ //
+ // ⚠️ THE PROBE IS ACROSS ALL VERBS, and that is the whole subject. The way
+ // this class actually surfaced was NOT a same-method disguise: nothing
+ // registers POST on `/packages/:id`, so `resolveMountedRoute('POST', …)`
+ // answers `undefined` and a method-scoped check sees a clean absence. The
+ // adapter's 405 seam does not work that way — `allowedMethodsForPath()`
+ // matches the concrete PATH against every registered pattern IGNORING the
+ // request's method, and answers 405 with whatever verbs that turns up. So a
+ // pinned path is only genuinely unanswered when NO verb matches it; one that
+ // matches under some other verb answers 405 + `Allow`, naming another route's
+ // methods, which is the defect this file's pin list shipped.
+ //
+ // A row genuinely served by a broader pattern is not this: the ledger says so
+ // with `servedBy`, and direction 1 checks it there.
+ const PROBED_VERBS = ['GET', 'POST', 'PUT', 'PATCH', 'DELETE'] as const;
+
+ it('no pinned path is matched by any OTHER pattern, under any verb (#7563)', () => {
+ const disguised: string[] = [];
+
+ for (const [key, reason] of Object.entries(UNEXERCISED_BY_THIS_BOOT)) {
+ const sp = key.indexOf(' ');
+ const pattern = key.slice(sp + 1);
+ // A `**` row names a prefix family rather than one resolvable path, so
+ // there is no concrete probe to build; the check above already asserts
+ // nothing is mounted under the prefix.
+ if (pattern.includes('*')) continue;
+
+ const path = probePath(pattern);
+ for (const verb of PROBED_VERBS) {
+ const resolved = server.resolveMountedRoute!(verb, path);
+ if (!resolved || resolved.pattern === pattern) continue;
+ disguised.push(
+ `${key} — pinned as unobservable ("${reason}"), but \`${resolved.pattern}\` matches ${path} under `
+ + `${verb}. The pin claims a caller gets nothing here; a caller actually gets that route's answer — `
+ + `its 405 + \`Allow\` when the verbs differ, its body when they do not. Mount an owner for this path `
+ + '(so it can 404 for itself), or declare `servedBy` if that pattern legitimately serves it.',
+ );
+ }
+ }
+
+ expect(disguised, `\n${disguised.join('\n')}\n`).toEqual([]);
+ });
+
  // ── Direction 2: every live mount is ledgered ─────────────────────────────
  it('every mounted route is ledgered', () => {
  const exact = new Set(ledgerRows.filter((r) => !isWildcardRow(r)).map((r) => `${r.method} ${r.pattern}`));
@@ -325,6 +389,27 @@ describe('route ledger ↔ live mount parity (#7526)', () => {
  .toEqual({ method: 'GET', pattern: '/api/v1/meta/:type' });
  });
 
+ // The publish path, pinned in both the currencies that matter: which
+ // registration the router hands it to, and what a caller actually receives.
+ it('POST /packages/publish is owned by the publish route, not absorbed by /packages/:id (#7563)', async () => {
+ // `/packages/:id` is mounted (by the dispatcher) and would match this path
+ // under GET/DELETE/PATCH — which is the whole reason the 405 was built from
+ // its method set. The publish registration has to win the POST.
+ expect(mounted.map((m) => `${m.method} ${m.pattern}`)).toContain('GET /api/v1/packages/:id');
+ expect(server.resolveMountedRoute!('POST', '/api/v1/packages/publish'))
+ .toEqual({ method: 'POST', pattern: '/api/v1/packages/publish' });
+
+ // …and on the wire. This boot composes no `package` service, so the honest
+ // answer is the publish route's own 404 naming the surface — never a 405
+ // advertising `DELETE, GET, HEAD, PATCH`, which are `/packages/:id`'s verbs
+ // over a package whose id is the literal string `publish`.
+ const token = await stack.signIn();
+ const res = await stack.apiAs(token, 'POST', '/packages/publish', {});
+ expect(res.status).toBe(404);
+ expect(res.headers.get('Allow')).toBeNull();
+ expect((await res.json())?.error?.message).toContain('marketplace publish surface');
+ }, 60_000);
+
  // The other two defects, pinned as live-router facts rather than as prose.
  it('the three #7526 routes resolve to themselves and not to a catch-all sibling', () => {
  expect(server.resolveMountedRoute!('GET', '/api/v1/meta/object/lead/published'))

packages/rest/src/direct-mount-composition.ts

@@ -20,18 +20,26 @@
  * Each registrar returns the array it iterated to mount, and that array is what
  * gets recorded on the `RestServer`. So:
  *
- * - a registrar this boot called ⇒ its routes are enumerable through
- * `getRoutes()` and appear in `GET {apiPath}/openapi.json`;
- * - a registrar this boot skipped (no `package` service) ⇒ nothing is
- * recorded, nothing is documented, and the 404 a caller would get from that
- * deployment is what the document says too.
+ * - a route this boot mounted ⇒ it is enumerable through `getRoutes()` and
+ * appears in `GET {apiPath}/openapi.json`;
+ * - a route this boot skipped (a package route needing a `package` service
+ * that is not there) ⇒ nothing is recorded, nothing is documented, and the
+ * 404 a caller would get from that deployment is what the document says too.
  *
- * The service gate stays exactly where it was — here, at composition — and the
- * record follows it rather than restating it. What is deliberately NOT recorded
- * is any verdict about a service that a later phase could still contradict: the
- * federation routes mount unconditionally and decide per request whether the
- * `external-datasource` service is there (503 if not), so this file records
- * them as mounted and says nothing about federation being available.
+ * [#7563] That second bullet promised a 404 and, for `POST /packages/publish`,
+ * did not get one: with no owner for the path, the dispatcher's
+ * `/packages/:id` matched it (`id = "publish"`) and the router answered 405
+ * with THAT route's `Allow` set. The publish route therefore mounts on every
+ * boot and answers its own honest 404 — see `package-routes.ts` for why the
+ * other three must not follow it.
+ *
+ * The service gate stays exactly where it was — around the package registrar's
+ * routes — and the record follows it rather than restating it. What is
+ * deliberately NOT recorded is any verdict about a service that a later phase
+ * could still contradict: the federation routes mount unconditionally and
+ * decide per request whether the `external-datasource` service is there (503 if
+ * not), so this file records them as mounted and says nothing about federation
+ * being available.
  */
 
 import type { PluginContext } from '@objectstack/core';
@@ -73,27 +81,49 @@ export function mountAndRecordDirectRoutes(composition: DirectMountComposition):
  const enableProjectScoping = composition.enableProjectScoping ?? false;
  const projectResolution = composition.projectResolution ?? 'auto';
 
- // Package management routes — only when the service backing them exists.
+ // Package management routes. [#7563] The registrar is called on EVERY boot
+ // and the `package` service is handed to it as a RESOLVER, not as a
+ // resolved instance — the gate did not move, it stopped being a
+ // boot-instant snapshot. `objectstack serve` registers the capability
+ // providers (`requires: ['marketplace']` → `PackageServicePlugin`) after
+ // `createRestApiPlugin`, and start order follows registration order for
+ // plugins with no edge between them, so asking once here answered "no
+ // package service" on precisely the deployments that have one.
+ //
+ // `registerPackageRoutes` decides what that resolver's answer means per
+ // route: `POST /packages/publish` mounts either way (nobody else serves it,
+ // and an unowned path is answered by a `/packages/:id` sibling's 405
+ // instead of a 404 — #7563), the other three only when a service is there
+ // (they shadow live dispatcher twins). It reports back exactly what it
+ // mounted, so the record still follows the gate rather than restating it.
+ const resolvePackageService = () => {
+ try {
+ return ctx.getService<PackageService>('package');
+ } catch {
+ // Not registered (yet) — an absence, not a failure.
+ return undefined;
+ }
+ };
  try {
- const packageService = ctx.getService<PackageService>('package');
- if (packageService) {
- // `required` scoping serves ONLY the scoped variant; `auto` serves
- // both. Unchanged from the pre-#5822 plugin — expressed as the list
- // of bases so the mount and the record cannot disagree about it.
- const scopedBase = `${versionedBase}/environments/:environmentId`;
- const bases = enableProjectScoping
- ? (projectResolution === 'required' ? [scopedBase] : [versionedBase, scopedBase])
- : [versionedBase];
- for (const base of bases) {
- recorder.recordDirectMountedRoutes(
- registerPackageRoutes(server, packageService, base, { protocol, resolveExecutionContext }),
- );
- }
- ctx.logger.info('Package management routes registered');
+ // `required` scoping serves ONLY the scoped variant; `auto` serves
+ // both. Unchanged from the pre-#5822 plugin — expressed as the list
+ // of bases so the mount and the record cannot disagree about it.
+ const scopedBase = `${versionedBase}/environments/:environmentId`;
+ const bases = enableProjectScoping
+ ? (projectResolution === 'required' ? [scopedBase] : [versionedBase, scopedBase])
+ : [versionedBase];
+ for (const base of bases) {
+ recorder.recordDirectMountedRoutes(
+ registerPackageRoutes(server, resolvePackageService, base, { protocol, resolveExecutionContext }),
+ );
  }
- } catch (e) {
- // Package service not available, skip
- ctx.logger.debug('Package service not available, package routes skipped');
+ ctx.logger.info('Package management routes registered');
+ } catch (e: any) {
+ // Nothing is recorded on this path, for the same reason the federation
+ // arm below records nothing when it throws: a registrar that failed
+ // part-way may have mounted some rows, and under-claiming is the safe
+ // direction.
+ ctx.logger.warn('Package management routes registration failed', { error: e?.message });
  }
 
  // External Datasource Federation routes (ADR-0015): catalog / draft /