From 2e21f69d38a9c45467dc4aa113313e381ec0ac1a Mon Sep 17 00:00:00 2001 From: "claude[bot]" Date: Sat, 29 Aug 2026 17:55:54 +0000 Subject: [PATCH] fix(app-shell): refuse a provision payload whose `data` carries no `environment` row MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `provisionProductionEnvironment`'s envelope check 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 objectui#6629 fixed, reachable again by a producer change alone. A flat payload is a producer contract violation, not a second dialect, so it is now refused. That routes a producer regression to this call's already documented failure path: the sole caller wraps it in try/catch, logs a warning, and the onboarding gate re-provisions lazily on first navigation. The refusal carries its own diagnostic, distinct from the missing-envelope one. The anti-alias pin updates in step, from "resolves with nothing" to "rejects" — a strictly stronger assertion, since a reintroduced `data.environment ?? data` alias resolves there and fails it either way. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01CRJge11jso9TpXRWFt1Z49 --- ...app-shell-provision-envelope-shape-6707.md | 31 ++++++++++++ .../__tests__/provisionEnvironment.test.ts | 48 +++++++++++++++---- .../organizations/provisionEnvironment.ts | 36 +++++++++++--- 3 files changed, 99 insertions(+), 16 deletions(-) create mode 100644 .changeset/app-shell-provision-envelope-shape-6707.md diff --git a/.changeset/app-shell-provision-envelope-shape-6707.md b/.changeset/app-shell-provision-envelope-shape-6707.md new file mode 100644 index 000000000..fdf65fd3c --- /dev/null +++ b/.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. diff --git a/packages/app-shell/src/console/organizations/__tests__/provisionEnvironment.test.ts b/packages/app-shell/src/console/organizations/__tests__/provisionEnvironment.test.ts index 7651b91fe..7eed72c19 100644 --- a/packages/app-shell/src/console/organizations/__tests__/provisionEnvironment.test.ts +++ b/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 () => { diff --git a/packages/app-shell/src/console/organizations/provisionEnvironment.ts b/packages/app-shell/src/console/organizations/provisionEnvironment.ts index 25a3c615a..e0b61b1e2 100644 --- a/packages/app-shell/src/console/organizations/provisionEnvironment.ts +++ b/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 }; }