fix(sandbox): add sandbox to the root command#128
Conversation
|
@codex review |
🦋 Changeset detectedLatest commit: a3c92c7 The changes in this PR will be included in the next version bump. This PR includes changesets to release 7 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a6968a29d9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
| const name = (app as { name?: string }).name ?? options.appId; | ||
| const appName = (app as { name?: string }).name; | ||
| const name = appName ? sandboxNameFor(appName) : options.appId; |
There was a problem hiding this comment.
When Sandbox.get() is used on sandboxes created by earlier SDK versions without an explicit name, their MC app name is already sandbox-<hex> because the old generated name included the prefix. This unconditionally strips that prefix, so reconnecting by app ID changes sandbox.name/toHandle().name from sandbox-<hex> to <hex>, which can break callers that key persisted handles or UI state by the sandbox name. Please distinguish newly prefixed app names from legacy names, or preserve the app name when that distinction is unavailable.
Useful? React with 👍 / 👎.
|
|
||
| const appId = await createApp(client, { | ||
| name, | ||
| name: appNameFor(name), |
There was a problem hiding this comment.
When callers pass a sandbox name between 93 and 100 characters, the name previously fit the MC app schema, but adding sandbox- here sends a value over the 100-character API limit (AddApplicationRequest.name in packages/openapi-client/specs/magic-containers.json caps it at 100). This makes otherwise valid Sandbox.create({ name }) calls fail at create time; validate the user-facing sandbox name against the reduced limit or otherwise account for the prefix before calling createApp.
Useful? React with 👍 / 👎.
acaf87a to
3068bfe
Compare
3 participants
No description provided.