Skip to content

refactor(api): drop the legacy boxId DTO field and regenerate API clients#744

Merged
G4614 merged 2 commits into
feat/box-curated-image-bootfrom
refactor/drop-legacy-boxid-field
Jun 12, 2026
Merged

refactor(api): drop the legacy boxId DTO field and regenerate API clients#744
G4614 merged 2 commits into
feat/box-curated-image-bootfrom
refactor/drop-legacy-boxid-field

Conversation

@law-chain-hot

Copy link
Copy Markdown
Contributor

Summary

Stacked on #735. The Box entity's identity collapsed there into a single 12-char id; the boxId fields on BoxDto / the admin overview DTO were kept only as value mirrors of id to avoid touching the API contract and generated clients in that PR. This PR removes the mirror everywhere:

before (after #735)                 after (this PR)
───────────────────                 ───────────────
entity/DB:   id  ✅ single          unchanged
BoxDto:      id + boxId (same       id only
             value, legacy shape)
admin DTO:   id + boxId mirror      id only
clients:     Box/AdminBoxItem/      boxId field gone (TS + Go)
             Workspace carry boxId
dashboard:   reads .boxId ?? .id    reads .id
  • API: BoxDto.boxId, admin overview boxId, dual-id correlation matching in admin observability — all removed.
  • Dashboard: box-identity helpers 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).
  • Generated clients: regenerated with the repo's own codegen inside a disposable BoxLite box (CI-faithful: Linux + JDK + GNU sed, no host gofmt drift). Drift is exactly the boxId removal on the Box / AdminBoxItem / Workspace models in the TS and Go clients.
  • Untouched on purpose: route/query parameter names like :boxId (they carry the single id), other tables' boxId FK column names, and the analytics ModelsBoxUsage.boxId (a real data field, not the mirror).

Verification

  • API jest: 38 suites / 112 tests pass
  • Dashboard: tsc --noEmit clean against the regenerated client
  • Regen ran in a BoxLite box (boxlite run node:22-bookworm); output is byte-for-byte what CI's api-client-drift check regenerates

Merge order

#743 (remove TS cloud SDK) → #735 (image pipeline + single box identity) → this PR.

@law-chain-hot law-chain-hot requested a review from a team as a code owner June 12, 2026 08:20
@coderabbitai

coderabbitai Bot commented Jun 12, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 538fc26d-fa1b-4c04-8525-772a91c74ea4

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch refactor/drop-legacy-boxid-field

Comment @coderabbitai help to get the list of available commands and usage tips.

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.
law-chain-hot and others added 2 commits June 12, 2026 22:25
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.
@law-chain-hot law-chain-hot force-pushed the feat/box-curated-image-boot branch from 6c4776a to 2c04345 Compare June 12, 2026 14:27
@law-chain-hot law-chain-hot force-pushed the refactor/drop-legacy-boxid-field branch from 1ca097a to 423d995 Compare June 12, 2026 14:28

@G4614 G4614 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

zggg

@G4614 G4614 merged commit 1b29a0c into feat/box-curated-image-boot Jun 12, 2026
3 checks passed
@G4614 G4614 deleted the refactor/drop-legacy-boxid-field branch June 12, 2026 14:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants