fix(app-shell): refuse a provision payload whose data carries no environment row

.changeset/app-shell-provision-envelope-shape-6707.md

@@ -0,0 +1,31 @@
+---
+'@object-ui/app-shell': minor
+---
+
+`provisionProductionEnvironment` now REFUSES a success payload whose `data`
+carries no `environment` row, instead of resolving best-effort (objectui#6707).
+
+objectui#6629 fixed this consumer to read the created environment from the
+nested `environment` key, but deliberately left the envelope check alone: it
+catches a **missing** `data` and says nothing about `data`'s **shape**. So a
+producer that regressed to a flat payload would once again resolve successfully
+with `id` and `hostname` both `undefined` — the same silent outcome #6629 had
+just fixed, reachable again by a producer change alone.
+
+A flat payload is a producer contract violation, not a second dialect to be
+tolerated, and tolerating it is how the original defect stayed invisible. The
+call refuses it now, which routes a producer regression to this call's already
+documented failure path rather than a successful-looking no-op: the sole caller
+(`CreateWorkspaceDialog`) already wraps the call in `try`/`catch`, logs a
+warning, and lets the onboarding gate re-provision lazily on first navigation.
+Workspace creation itself is unaffected — the caller does not re-throw, and it
+never read this call's return value.
+
+The refusal carries its own diagnostic, distinct from the missing-envelope one,
+so a logged warning still distinguishes "the control plane did not wrap the
+payload" from "it did not put the row where it says it does".
+
+The wire shape this is written against is confirmed producer-side rather than
+inferred from this consumer — the distinction is load-bearing, because before
+#6629 the only in-repo artifact pinning this payload was a hand-written mock
+pinning the bug shape. Both sources are recorded on the function's docblock.

packages/app-shell/src/console/organizations/__tests__/provisionEnvironment.test.ts

@@ -5,7 +5,8 @@
  *
  * - posts `Production` + the explicit org id to the cloud env endpoint;
  * - resolves the created env on 2xx, reading it from the NESTED `environment`
- * row the control plane wraps it in (objectui#6629);
+ * row the control plane wraps it in (objectui#6629), and REFUSES a 2xx
+ * whose `data` carries no such row (objectui#6707);
  * - treats 403/409 ("org already has its production env" — e.g. the control
  * plane's auto-default-environment plugin won the race) as SUCCESS
  * (`alreadyProvisioned`), NOT a failure;
@@ -86,19 +87,48 @@ describe('provisionProductionEnvironment', () => {
 
  // Contract-first (AGENTS.md #0.1): the fix reads exactly ONE dialect. A flat
  // `data` is not a second accepted spelling of the payload, so its keys must
- // not be picked up — no `data.environment ?? data` alias. Note it still
- // RESOLVES rather than throws: rejecting a wrong-shaped `data` would change
- // behaviour on a path the caller currently relies on swallowing, and is
- // deliberately NOT folded into this fix (objectui#6629).
- it('does not fall back to a flat `data` shape when `environment` is absent', async () => {
+ // not be picked up — no `data.environment ?? data` alias.
+ //
+ // objectui#6707 turned this pin from "resolves with nothing" into "REJECTS".
+ // It used to assert the call still resolved, because refusing a wrong-shaped
+ // `data` changes behaviour on a path the caller relies on swallowing, and
+ // that was deliberately severed from #6629 as its own decision. It was ruled
+ // (2026-08-29, option B): a flat payload is a producer violation, not a
+ // second dialect, so it is refused. The assertion is also strictly STRONGER
+ // than the one it replaces — a reintroduced `data.environment ?? data` alias
+ // would resolve `{ id: 'flat-1' }` here and fail this test, exactly as it
+ // failed the old one.
+ it('rejects a flat `data` shape instead of falling back to it', async () => {
  authFetch.mockResolvedValue(
  res(200, { success: true, data: { id: 'flat-1', hostname: 'flat.localhost' } }),
  );
 
- const out = await provisionProductionEnvironment({ organizationId: 'org-123' });
+ await expect(provisionProductionEnvironment({ organizationId: 'org-123' })).rejects.toThrow(
+ /`data\.environment` is missing/,
+ );
+ });
+
+ // objectui#6707 — the producer-regression case the throw exists for, and it
+ // is NOT the same failure as a missing envelope. A `data` that is present and
+ // well-formed but carries no `environment` row (the other keys the handler
+ // really sends are here; `hostnameAssignment` is conditional, and its absence
+ // is the ordinary "no rename happened" case, never "unknown") must be refused
+ // with its OWN diagnostic. The two conditions answer different questions —
+ // "did the control plane wrap the payload?" vs "did it put the row where it
+ // says it does?" — so collapsing them into one message would cost whoever
+ // reads that logged warning the ability to tell a broken transport from a
+ // regressed producer. Hence the negative assertion, not just the positive one.
+ it('rejects an enveloped `data` whose `environment` row is absent, with its own diagnostic', async () => {
+ authFetch.mockResolvedValue(res(201, { success: true, data: { warnings: [], durationMs: 42 } }));
+
+ const err: unknown = await provisionProductionEnvironment({ organizationId: 'org-123' }).then(
+ () => null,
+ (e: unknown) => e,
+ );
 
- expect(out.id).toBeUndefined();
- expect(out.hostname).toBeUndefined();
+ expect(err).toBeInstanceOf(Error);
+ expect((err as Error).message).toMatch(/`data\.environment` is missing/);
+ expect((err as Error).message).not.toMatch(/envelope/i);
  });
 
  it('treats 403 (already has its production env) as success, not a failure', async () => {

packages/app-shell/src/console/organizations/provisionEnvironment.ts

@@ -16,6 +16,16 @@
  * durationMs, hostnameAssignment? } }` — the created row is nested under
  * `environment`, NOT flat on `data`.
  *
+ * That shape is CONFIRMED producer-side, not inferred from this consumer —
+ * the distinction matters, because the only in-repo artifact that ever pinned
+ * this payload before objectui#6629 was a hand-written mock pinning the BUG
+ * shape. Two independent producer-side sources, both in `objectstack-ai/
+ * objectstack`: `packages/client/src/index.ts` (`environments.create`) records
+ * the key set as measured against the cloud repo's `main` on 2026-08-28, naming
+ * the handler `packages/service-cloud/src/routes/environment-lifecycle.ts`;
+ * and `@objectstack/spec`'s `ProvisionEnvironmentResponseSchema`
+ * (`src/cloud/environment.zod.ts`) declares `environment` REQUIRED.
+ *
  * Idempotent + best-effort by contract:
  * - Some control planes auto-provision the production env on org create (the
  * `auto-default-environment` plugin). This call then races that plugin and
@@ -53,8 +63,9 @@ export interface ProvisionedEnvironment {
  * born-with-env convention used by the signup org.
  *
  * @throws on a genuine control-plane failure (5xx / network), or on a 2xx whose
- * body doesn't carry the contractual `{ success, data }` envelope. A 403/409
- * "already has its production env" is NOT an error — it resolves to
+ * body doesn't carry the contractual `{ success, data: { environment } }`
+ * shape — a wrong-shaped `data` is refused, not absorbed (objectui#6707). A
+ * 403/409 "already has its production env" is NOT an error — it resolves to
  * `{ alreadyProvisioned: true }`.
  */
 export async function provisionProductionEnvironment(opts: {
@@ -103,10 +114,21 @@ export async function provisionProductionEnvironment(opts: {
  // into a value typed `ProvisionedEnvironment`.
  //
  // Read ONE dialect (AGENTS.md #0.1): no `data.environment ?? data` alias — a
- // flat payload is a producer contract violation, not a second spelling. It
- // still RESOLVES rather than throws, because rejecting a wrong-shaped `data`
- // would change behaviour on the best-effort path the caller relies on
- // swallowing; that is a separate decision, not part of this fix.
+ // flat payload is a producer contract violation, not a second spelling — and
+ // the violation is REFUSED rather than absorbed (objectui#6707). The envelope
+ // check above only catches a MISSING `data`; it says nothing about `data`'s
+ // shape, so a producer that regressed to a flat payload would once again
+ // resolve successfully with `id` and `hostname` `undefined` — the same silent
+ // outcome #6629 fixed, reachable again by a producer change alone. Throwing
+ // routes it to the caller's documented failure path instead: the sole caller
+ // (`CreateWorkspaceDialog`) logs the warning and the onboarding gate
+ // re-provisions lazily on first navigation, so a producer regression becomes
+ // loud-ish and recoverable rather than a successful-looking no-op.
  const environment = data.environment;
- return { id: environment?.id, hostname: environment?.hostname };
+ if (!environment || typeof environment !== 'object') {
+ throw new Error(
+ 'Malformed control-plane response: `data.environment` is missing from POST /cloud/environments',
+ );
+ }
+ return { id: environment.id, hostname: environment.hostname };
 }