refactor(api): drop the legacy boxId DTO field and regenerate API clients#744
Merged
G4614 merged 2 commits intoJun 12, 2026
Merged
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
This was referenced Jun 12, 2026
G4614
pushed a commit
that referenced
this pull request
Jun 12, 2026
## Summary Stacked on #744. Implements the identity follow-up discussed on #735 (see the "Identity model note" comment there): the engine gains a CRI-style orchestrator reference, and the runner stops squatting the engine's `name` slot to carry the control-plane box id. ```text before after ────── ───── runner: WithName(cloudId) runner: WithExternalRef(cloudId) engine: name = cloudId (squatted) engine: external_ref = cloudId name unavailable for users name free for a future user-facing box name ``` Industry precedent (verified sources): - CRI `PodSandboxMetadata.uid` — the orchestrator's id travels as a dedicated metadata field; the runtime mints its own id ([cri-api api.proto](https://github.com/kubernetes/cri-api/blob/master/pkg/apis/runtime/v1/api.proto), `message PodSandboxMetadata`) - ECS agent attaches `com.amazonaws.ecs.task-arn` as a docker label ([docker_task_engine.go](https://github.com/aws/amazon-ecs-agent/blob/master/agent/engine/docker_task_engine.go)) - The caller-supplied-id pattern (OCI runtime spec `create <container-id>`) applies to stateless executors (runc/firecracker), not to stateful daemons like this engine — hence a reference field, not id injection. ## What changed - **engine**: `BoxOptions.external_ref` (opaque, json-persisted via the existing config blob — no DB migration; `lookup_box` already scans configs). Duplicate refs are rejected at create. Lookup order: exact id → name → external_ref → id prefix. `BoxInfo` exposes it, omitted when unset. - **C SDK**: `boxlite_options_set_external_ref`, `CBoxInfo.external_ref` (NULL when unset, freed on drop — the leak-count test now expects 5 inner CStrings), header regenerated by the crate build. - **Go SDK**: `WithExternalRef`, `BoxInfo.ExternalRef`. - **Node SDK**: field filled with `None` (orchestrator-facing knob; not exposed in JS options yet). - **CLI**: `inspect` shows `ExternalRef` only when present; `list` columns unchanged (nothing empty is rendered). - **runner**: `WithName(dto.Id)` → `WithExternalRef(dto.Id)`; restart recovery (`runtime.Get(dto.Id)`) resolves through the external_ref match. ## Verification - engine lib tests: 735/735; the new `lookup_box_by_external_ref` test was verified two-sided (fails with the lookup clause reverted, passes restored) - boxlite-c: 55/55 - CLI / Node / Python crates compile; `go vet` clean on `sdks/go` and `apps/runner/pkg/boxlite` (final cgo link needs the prebuilt `libboxlite.a`, absent in this worktree) ## Merge order #743 → #735 → #744 → this PR. Self-contained: reverting is closing this PR.
The Box entity's identity collapsed into a single 12-char id; the BoxDto/AdminBoxItem 'boxId' fields had been kept as value mirrors of 'id' purely for API-contract shape. Remove them and everything that existed only to serve the dual identity: - BoxDto / admin overview DTO: boxId field deleted; WorkspaceDto inherits the change. - admin observability correlation: no more dual-id matching. - dashboard: box-identity helpers read .id; drop dual-id cache updates and uuid-shaped-name special case (post-rebuild a name can never be a uuid). - regenerate the 4 API clients inside a BoxLite box (CI-faithful): Box/AdminBoxItem/Workspace models lose boxId in TS + Go clients. API jest: 112/112. Dashboard tsc clean after regen.
## Summary Stacked on #744. Implements the identity follow-up discussed on #735 (see the "Identity model note" comment there): the engine gains a CRI-style orchestrator reference, and the runner stops squatting the engine's `name` slot to carry the control-plane box id. ```text before after ────── ───── runner: WithName(cloudId) runner: WithExternalRef(cloudId) engine: name = cloudId (squatted) engine: external_ref = cloudId name unavailable for users name free for a future user-facing box name ``` Industry precedent (verified sources): - CRI `PodSandboxMetadata.uid` — the orchestrator's id travels as a dedicated metadata field; the runtime mints its own id ([cri-api api.proto](https://github.com/kubernetes/cri-api/blob/master/pkg/apis/runtime/v1/api.proto), `message PodSandboxMetadata`) - ECS agent attaches `com.amazonaws.ecs.task-arn` as a docker label ([docker_task_engine.go](https://github.com/aws/amazon-ecs-agent/blob/master/agent/engine/docker_task_engine.go)) - The caller-supplied-id pattern (OCI runtime spec `create <container-id>`) applies to stateless executors (runc/firecracker), not to stateful daemons like this engine — hence a reference field, not id injection. ## What changed - **engine**: `BoxOptions.external_ref` (opaque, json-persisted via the existing config blob — no DB migration; `lookup_box` already scans configs). Duplicate refs are rejected at create. Lookup order: exact id → name → external_ref → id prefix. `BoxInfo` exposes it, omitted when unset. - **C SDK**: `boxlite_options_set_external_ref`, `CBoxInfo.external_ref` (NULL when unset, freed on drop — the leak-count test now expects 5 inner CStrings), header regenerated by the crate build. - **Go SDK**: `WithExternalRef`, `BoxInfo.ExternalRef`. - **Node SDK**: field filled with `None` (orchestrator-facing knob; not exposed in JS options yet). - **CLI**: `inspect` shows `ExternalRef` only when present; `list` columns unchanged (nothing empty is rendered). - **runner**: `WithName(dto.Id)` → `WithExternalRef(dto.Id)`; restart recovery (`runtime.Get(dto.Id)`) resolves through the external_ref match. ## Verification - engine lib tests: 735/735; the new `lookup_box_by_external_ref` test was verified two-sided (fails with the lookup clause reverted, passes restored) - boxlite-c: 55/55 - CLI / Node / Python crates compile; `go vet` clean on `sdks/go` and `apps/runner/pkg/boxlite` (final cgo link needs the prebuilt `libboxlite.a`, absent in this worktree) ## Merge order #743 → #735 → #744 → this PR. Self-contained: reverting is closing this PR.
6c4776a to
2c04345
Compare
1ca097a to
423d995
Compare
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 free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
Stacked on #735. The Box entity's identity collapsed there into a single 12-char id; the
boxIdfields onBoxDto/ the admin overview DTO were kept only as value mirrors ofidto avoid touching the API contract and generated clients in that PR. This PR removes the mirror everywhere:BoxDto.boxId, admin overviewboxId, dual-id correlation matching in admin observability — all removed.box-identityhelpers read.id; dual-id cache updates and the uuid-shaped-name special case removed (after the DB rebuild a box name can never be a uuid).boxIdremoval on theBox/AdminBoxItem/Workspacemodels in the TS and Go clients.:boxId(they carry the single id), other tables'boxIdFK column names, and the analyticsModelsBoxUsage.boxId(a real data field, not the mirror).Verification
tsc --noEmitclean against the regenerated clientboxlite run node:22-bookworm); output is byte-for-byte what CI's api-client-drift check regeneratesMerge order
#743 (remove TS cloud SDK) → #735 (image pipeline + single box identity) → this PR.