Uh oh!
There was an error while loading. Please reload this page.
fix(app-shell): refuse a provision payload whose data carries no environment row - #6821
Merged
Merged
Conversation
…nvironment` row `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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CRJge11jso9TpXRWFt1Z49
Contributor
✅ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. 📦 Bundle Size Report
Size Limits
|
os-sales
marked this pull request as ready for review
August 29, 2026 22:15
Uh oh!
There was an error while loading. Please reload this page.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes#6707
Implements the ruled option B (2026-08-29, maintainer, batch #8):
provisionProductionEnvironmentrefuses a 2xx whosedatacarries noenvironmentrow, instead of resolving best-effort.The hard precondition, and how it was met
The ruling made this conditional: "the wire shape must be CONFIRMED against the cloud producer before the throw condition is written" — because the only in-repo artifact that ever pinned this payload before #6629 was a hand-written mock pinning the bug shape, and a tightening written against a wrong belief is worse than the leniency it replaces.
This lane cannot read
objectstack-ai/cloud. It did not infer the shape from this consumer. The confirmation comes from three producer-side sources inobjectstack-ai/objectstack, all naming the same handler:packages/client/src/index.ts(environments.create) onorigin/mainrecords, in a code comment: "both measured against the cloud repo'smainon 2026-08-28 (the handler ispackages/service-cloud/src/routes/environment-lifecycle.ts, POST/cloud/environments, which builds its body key by key)" — and states "the single-row key isenvironment. This route has NEVER emitted aprojectkey", plus "The keys the route DOES send besideenvironment(warnings,durationMs, and a conditionalhostnameAssignment)". That declaration isunwrapResponseof{ environment }, changed away fromprojectby objectstack#12866 precisely because the route never sentproject.@objectstack/spec,packages/spec/src/cloud/environment.zod.ts—ProvisionEnvironmentResponseSchemadeclaresenvironmentrequired (not optional).ok({ environment: result.environment, warnings: result.warnings, durationMs: result.durationMs, ...(result.hostnameAssignment ? { hostnameAssignment: result.hostnameAssignment } : {}) }).⭐ And the maintainer independently ruled on that shape ten minutes later: objectstack#12883, batch #11, adopts the four-key inline wire shape (
environment+warnings+durationMs+ optionalhostnameAssignment) as what the route sends. Triage had proposed resolving both cards from one reading; that is what happened.credentialrequired, and the handler quote does not send it (recorded on objectstack#12883 by thedomain:clilane, 2026-08-29). That divergence is real and unresolved, but it concernscredentialonly — every source agreesenvironmentis present and unconditional, which is the single fact this throw condition rests on. Nothing here readscredential.The change
The envelope check catches a missing
dataand says nothing aboutdata's shape. So after #6629 a producer that regressed to a flat payload would once again resolve successfully withidandhostnamebothundefined— 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; tolerating it is how the original defect stayed invisible.The refusal carries its own diagnostic, distinct from the missing-envelope one, so a logged warning still separates "the control plane did not wrap the payload" from "it did not put the row where it says it does". The guard also rejects a non-object
environment(a string there produces the identical silent outcome), mirroring the envelope check one line above.Blast radius, re-measured in this PR rather than inherited
CreateWorkspaceDialog.tsx:151. Already insidetry { … } catch (provisionErr) { console.warn(…) }. The inner catch does not re-throw, soonCreated?.(org)still fires — workspace creation is unaffected; only the eager provision degrades to the lazy onboarding gate.await provisionProductionEnvironment(…)), which independently confirms the ruling's reason for leaving option C not-taken.CreateWorkspaceDialog.test.tsx,org-i18n-holdouts-4474.test.tsx) bothvi.mockthe module, so neither is affected.packages/app-shell/src/console/organizations/— not inhooks/orlayout/, so it does not overlap the unmerged PR fix(app-shell): one reading for sys_activity.type, and the unrecognised case stops claiming update #6814.The anti-alias pin, updated in the same PR as ruled
does not fall back to a flat data shape…becamerejects a flat data shape instead of falling back to it. The new assertion is strictly stronger than the one it replaces: a reintroduceddata.environment ?? dataalias would resolve{ id: 'flat-1' }there and fail it either way. A second pin covers the producer-regression case (envelope intact,environmentrow absent) and asserts the diagnostic is not the envelope one, so the two conditions cannot be collapsed into one message.Verification — all readings on
2e21f69d3pnpm exec vitest run packages/app-shell/src/console/organizations/→Test Files 12 passed (12),Tests 103 passed (103).origin/main(tests kept), confirmed on disk by blob hash — on-disk25a3c615…equals theorigin/mainblob and differs from theHEADblob, and the throw string went to 0 occurrences. Result:Tests 2 failed | 6 passed (8)— exactly the two new/updated pins go red, the other six stay green, so neither pin is vacuous. Restore proven the same way: on-disk hash back toe0b61b1e…,git diff HEADempty, throw string back to 1. No rebuild leg was needed — the test imports../provisionEnvironmentby relative path, so it resolves to source, never through a packageexportsfield intodist/.pnpm --filter @object-ui/app-shell run type-check→ exit 0. It runs two configs, and both edited files are genuinely covered:--listFilesshows the basetsconfig.jsonexcludes test files (0 hits) buttsconfig.test.jsonincludes the edited test file (1 hit), so this is a measurement, not a green that skipped the new code.pnpm --filter @object-ui/app-shell run lint→0 errors, 2789 warnings, exit 0. (A stricter exploratory run with--no-inline-configsurfaced 15 errors; all 15 are in files this PR never touches and are suppressed by inline disables the real gate honours. Both edited files are 0 errors / 0 warnings under the stricter run too.)check:control-bytes,check:vi-mock-specifiers,check:spec-symbols,check-changeset-presence.mjs,check-changeset-no-major.mjs— all exit 0. Plus a control-character self-scan of the three changed files: clean.eslintitself chose the population (1016 files inpackages/app-shell, counted from--format json, not estimated), andeslint.config.jsdeclares noprojectServiceand noparserOptions.project— type-aware linting is off, so each file's verdict is a function of that file plus the shared config. This diff changes neither the config nor any other file, so no untouched file's verdict can move. CI runs the full farm regardless.Generated by Claude Code
Generated by Claude Code