From 35fe8741b03c045536df534b64a1d6c6d4cd868c Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 28 Aug 2026 17:54:48 +0000 Subject: [PATCH] fix(app-shell): read the provisioned env from the nested `environment` row MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `POST /api/v1/cloud/environments` answers `{ success, data: { environment, warnings, durationMs, hostnameAssignment? } }` — the created row is one level down. `provisionProductionEnvironment` read `data` flat and returned it as a `ProvisionedEnvironment`, so `id` and `hostname` were always `undefined` and the envelope's siblings rode along in their place. Silent by construction: both fields are optional, the call is best-effort by contract (403/409 resolves to `alreadyProvisioned`) and the caller swallows genuine failures — so the function reported a successful provision carrying no environment at all, the exact outcome the strict envelope check in that file was written to prevent. That check verifies `data` is an object and nothing about its shape; it is left unchanged here. The row is now projected to `{ id, hostname }`. One dialect only: no `data.environment ?? data` alias, and a wrong-shaped `data` still resolves rather than throws. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01CRJge11jso9TpXRWFt1Z49 --- .../6629-provision-env-nested-envelope.md | 31 +++++++++++ .../__tests__/provisionEnvironment.test.ts | 55 ++++++++++++++++++- .../organizations/provisionEnvironment.ts | 24 +++++++- 3 files changed, 106 insertions(+), 4 deletions(-) create mode 100644 .changeset/6629-provision-env-nested-envelope.md diff --git a/.changeset/6629-provision-env-nested-envelope.md b/.changeset/6629-provision-env-nested-envelope.md new file mode 100644 index 0000000000..274500dcd6 --- /dev/null +++ b/.changeset/6629-provision-env-nested-envelope.md @@ -0,0 +1,31 @@ +--- +"@object-ui/app-shell": patch +--- + +`provisionProductionEnvironment` reads the created env from the nested `environment` row + +`POST /api/v1/cloud/environments` answers `{ success, data: { environment, warnings, +durationMs, hostnameAssignment? } }` — the created row sits one level down, under +`environment`. The consumer read `data` FLAT and returned it as a +`ProvisionedEnvironment`, so `id` and `hostname` were always `undefined` and the +envelope's siblings rode along in their place. + +The bug was silent by construction: both fields are optional on the type, the whole call +is best-effort by contract (a 403/409 resolves to `alreadyProvisioned: true`) and the +caller swallows genuine failures — so the function reported a successful provision +carrying no environment at all, which is the exact outcome the strict envelope check in +that file was written to prevent. That check verifies `data` is an object and nothing +about its shape. + +The fix reads ONE dialect: no `data.environment ?? data` alias, and the row is projected +to `{ id, hostname }` rather than returned whole. A wrong-shaped `data` still RESOLVES +rather than throws — tightening the envelope check to reject it would change behaviour on +the best-effort path the caller relies on swallowing, and is deliberately not folded in +here. + +Scored `patch`, not an empty "no release" declaration: this is shipped runtime code in a +published package whose return value is different, not a comment or a test-only change, +so an empty frontmatter would assert something false. Not `minor` — no new capability and +no API surface change; `ProvisionedEnvironment` is unchanged. The blast radius is small +today (the sole in-repo caller, `CreateWorkspaceDialog`, discards the return value, and +the symbol is not on the package barrel), but "small" is not "unreleased". 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 501f8b8112..7651b91fe5 100644 --- a/packages/app-shell/src/console/organizations/__tests__/provisionEnvironment.test.ts +++ b/packages/app-shell/src/console/organizations/__tests__/provisionEnvironment.test.ts @@ -4,7 +4,8 @@ * provisionProductionEnvironment — born-with-env contract. * * - posts `Production` + the explicit org id to the cloud env endpoint; - * - resolves the created env on 2xx; + * - resolves the created env on 2xx, reading it from the NESTED `environment` + * row the control plane wraps it in (objectui#6629); * - 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; @@ -38,7 +39,9 @@ beforeEach(() => { describe('provisionProductionEnvironment', () => { it('posts Production + the org id to the cloud env endpoint and returns the env', async () => { - authFetch.mockResolvedValue(res(200, { data: { id: 'env-1', hostname: 'os-abc.localhost' } })); + authFetch.mockResolvedValue( + res(200, { data: { environment: { id: 'env-1', hostname: 'os-abc.localhost' } } }), + ); const out = await provisionProductionEnvironment({ organizationId: 'org-123' }); @@ -50,6 +53,54 @@ describe('provisionProductionEnvironment', () => { expect(out).toMatchObject({ id: 'env-1', hostname: 'os-abc.localhost' }); }); + // objectui#6629 — ANTI-VACUITY PIN. The control plane's success payload nests + // the created row one level down — `{ environment, warnings, durationMs, + // hostnameAssignment? }` — so reading `data` FLAT left `id` and `hostname` + // permanently `undefined`. Nothing threw and nothing logged: both fields are + // optional on the type, the whole call is best-effort by contract, and the + // caller swallows failures — the only symptom was a "successful" provision + // carrying no environment at all. A test that merely asserts the fixed path + // is green would have been green BEFORE the fix too; this one is RED on the + // pre-fix implementation (which returns the wrapper, whose `id` is + // `undefined`), which is the whole reason it exists. + it('reads the created env from the nested `environment` row, not from the wrapper', async () => { + authFetch.mockResolvedValue( + res(201, { + success: true, + data: { + environment: { id: 'env-1', hostname: 'os-abc.localhost' }, + warnings: [], + durationMs: 42, + hostnameAssignment: { hostname: 'os-abc.localhost' }, + }, + }), + ); + + const out = await provisionProductionEnvironment({ organizationId: 'org-123' }); + + // `toEqual`, not `toMatchObject`: the wrapper's siblings (`warnings`, + // `durationMs`, `hostnameAssignment`) must not ride along into a value + // typed `ProvisionedEnvironment`. + expect(out).toEqual({ id: 'env-1', hostname: 'os-abc.localhost' }); + }); + + // 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 () => { + authFetch.mockResolvedValue( + res(200, { success: true, data: { id: 'flat-1', hostname: 'flat.localhost' } }), + ); + + const out = await provisionProductionEnvironment({ organizationId: 'org-123' }); + + expect(out.id).toBeUndefined(); + expect(out.hostname).toBeUndefined(); + }); + it('treats 403 (already has its production env) as success, not a failure', async () => { authFetch.mockResolvedValue(res(403, { success: false, error: 'PRODUCTION_ENV_LIMIT' })); diff --git a/packages/app-shell/src/console/organizations/provisionEnvironment.ts b/packages/app-shell/src/console/organizations/provisionEnvironment.ts index e7bd22708f..25a3c615a7 100644 --- a/packages/app-shell/src/console/organizations/provisionEnvironment.ts +++ b/packages/app-shell/src/console/organizations/provisionEnvironment.ts @@ -12,6 +12,10 @@ * is resolved from `organizationId` (preferred) → the better-auth active org → * the actor's first membership. * + * The endpoint answers `201 { success, data: { environment, warnings, + * durationMs, hostnameAssignment? } }` — the created row is nested under + * `environment`, NOT flat on `data`. + * * 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 @@ -81,12 +85,28 @@ export async function provisionProductionEnvironment(opts: { // successful provision carrying no env at all. Throwing routes it to the // caller's documented failure path: the onboarding gate provisions lazily on // first navigation. - const body = (await res.json().catch(() => null)) as { data?: ProvisionedEnvironment } | null; + const body = (await res.json().catch(() => null)) as { + data?: { environment?: { id?: string; hostname?: string } }; + } | null; const data = body?.data; if (!data || typeof data !== 'object') { throw new Error( 'Malformed control-plane response: expected a `{ success, data }` envelope from POST /cloud/environments', ); } - return data; + // ...and inside that envelope the created row sits ONE LEVEL DOWN, under + // `environment` — the handler answers `{ environment, warnings, durationMs, + // hostnameAssignment? }`, so `data.id` / `data.hostname` never existed on the + // wire and reading `data` flat reported a successful provision whose `id` and + // `hostname` were always `undefined` (objectui#6629). Projected explicitly + // rather than returned whole, so the envelope's siblings can't ride along + // 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. + const environment = data.environment; + return { id: environment?.id, hostname: environment?.hostname }; }