feat(box): create boxes from curated images#735
Closed
law-chain-hot wants to merge 11 commits into
Closed
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR refactors box identity and image handling across the entire system. It replaces curated image keys with pinned OCI references validated against a server-configured allowlist, migrates box identity from a UUID primary key plus separate boxId to a single 12-character public ID, and threads image selection through creation flows, API contracts, client libraries, and test infrastructure. ChangesCurated Image System and Supported Refs
Box Entity and Persistence Schema
Box Creation and Lifecycle
Box Lookup and Cache Refactoring
Admin Services and Observability
Runner Create Workflow
Runner Go DTO and Client
OpenAPI and Generated Clients
Dashboard Create Experience
Dashboard Identity and Templates
E2E Test Infrastructure
SDK External Reference Support
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly Related PRs
Suggested Reviewers
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/api/src/box/managers/box-actions/box-start.action.ts`:
- Around line 81-85: The code currently calls runnerAdapter.createBox
unconditionally which will throw for V0 runners (see RunnerAdapterV0.createBox);
before invoking createBox detect V0 (via the created runnerAdapter or
runner.version field) and for V0 runners immediately transition the box to an
ERROR state using this.updateBoxState(box, BoxState.ERROR, lockCode) (and return
SYNC_AGAIN) instead of calling createBox; otherwise for non-V0 runners proceed
with await runnerAdapter.createBox(...), then update to CREATING and return
SYNC_AGAIN.
In `@apps/api/src/box/services/box.service.warm-pool.spec.ts`:
- Line 10: The test currently asserts the warm-pool image contains the literal
substring "boxlite-agent-base", which is brittle when BOXLITE_SYSTEM_BASE_IMAGE
is overridden; change the assertion to compare the returned image to the
resolver output instead. Import and call the image resolver used by the service
(the same function that resolves BOX_IMAGE_REF_LABEL) and assert the warm-pool
image equals that resolver result (use BOX_IMAGE_REF_LABEL as the input),
replacing the substring check on the image string.
In `@openapi/box.openapi.yaml`:
- Around line 1527-1533: The OpenAPI spec's CreateBoxRequest.image field still
shows a raw OCI image default/example that contradicts the updated description
requiring curated image keys; update the default and example to a curated key
(e.g., default: "base" and example: "python") so the schema matches the
description and upstream behavior for CreateBoxRequest.image.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 4edce049-8bb0-4dc7-974b-530db1671fe2
📒 Files selected for processing (35)
apps/api/src/box/constants/curated-images.constant.spec.tsapps/api/src/box/constants/curated-images.constant.tsapps/api/src/box/dto/create-box.dto.tsapps/api/src/box/managers/box-actions/box-start.action.tsapps/api/src/box/runner-adapter/runnerAdapter.tsapps/api/src/box/runner-adapter/runnerAdapter.v0.tsapps/api/src/box/runner-adapter/runnerAdapter.v2.createBox.spec.tsapps/api/src/box/runner-adapter/runnerAdapter.v2.tsapps/api/src/box/services/box.service.replace-labels.spec.tsapps/api/src/box/services/box.service.tsapps/api/src/box/services/box.service.warm-pool.spec.tsapps/api/src/boxlite-rest/mappers/box-to-box.mapper.spec.tsapps/api/src/boxlite-rest/mappers/box-to-box.mapper.tsapps/dashboard/src/components/Box/CreateBoxSheet.tsxapps/dashboard/src/components/Playground/Box/CodeSnippets/python.tsapps/dashboard/src/components/Playground/Box/CodeSnippets/typescript.tsapps/dashboard/src/lib/onboarding-code-examples.tsapps/dashboard/src/providers/PlaygroundProvider.tsxapps/libs/sdk-typescript/src/BoxLite.tsapps/libs/sdk-typescript/src/__tests__/BoxLite.create-guards.test.tsopenapi/box.openapi.yamlscripts/test/e2e/README.mdscripts/test/e2e/bootstrap.shscripts/test/e2e/cases/conftest.pyscripts/test/e2e/cases/test_c_entry.pyscripts/test/e2e/cases/test_cli_detach_recovery.pyscripts/test/e2e/cases/test_cli_entry.pyscripts/test/e2e/cases/test_error_code_mapping.pyscripts/test/e2e/cases/test_go_entry.pyscripts/test/e2e/cases/test_node_entry.pyscripts/test/e2e/cases/test_quota_enforcement.pyscripts/test/e2e/fixture_setup.pyscripts/test/e2e/sdks/c/e2e_basic.cscripts/test/e2e/sdks/go/e2e_basic.goscripts/test/e2e/sdks/node/e2e_basic.ts
Comment on lines
+81
to
+85
| const runnerAdapter = await this.runnerAdapterFactory.create(runner) | ||
| await runnerAdapter.createBox(box, artifactRef) | ||
|
|
||
| await this.updateBoxState(box, BoxState.CREATING, lockCode) | ||
| return SYNC_AGAIN |
There was a problem hiding this comment.
Handle unsupported V0 create path before calling createBox.
This flow unconditionally calls runnerAdapter.createBox, but RunnerAdapterV0.createBox throws by design. For boxes on V0 runners, this can fail before a deterministic state transition to ERROR.
Suggested fix
const runnerAdapter = await this.runnerAdapterFactory.create(runner)
+ if (runner.apiVersion === '0') {
+ await this.updateBoxState(
+ box,
+ BoxState.ERROR,
+ lockCode,
+ undefined,
+ 'Box creation is not supported on runner API version 0',
+ )
+ return DONT_SYNC_AGAIN
+ }
await runnerAdapter.createBox(box, artifactRef)
await this.updateBoxState(box, BoxState.CREATING, lockCode)
return SYNC_AGAIN📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const runnerAdapter = await this.runnerAdapterFactory.create(runner) | |
| await runnerAdapter.createBox(box, artifactRef) | |
| await this.updateBoxState(box, BoxState.CREATING, lockCode) | |
| return SYNC_AGAIN | |
| const runnerAdapter = await this.runnerAdapterFactory.create(runner) | |
| if (runner.apiVersion === '0') { | |
| await this.updateBoxState( | |
| box, | |
| BoxState.ERROR, | |
| lockCode, | |
| undefined, | |
| 'Box creation is not supported on runner API version 0', | |
| ) | |
| return DONT_SYNC_AGAIN | |
| } | |
| await runnerAdapter.createBox(box, artifactRef) | |
| await this.updateBoxState(box, BoxState.CREATING, lockCode) | |
| return SYNC_AGAIN |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/api/src/box/managers/box-actions/box-start.action.ts` around lines 81 -
85, The code currently calls runnerAdapter.createBox unconditionally which will
throw for V0 runners (see RunnerAdapterV0.createBox); before invoking createBox
detect V0 (via the created runnerAdapter or runner.version field) and for V0
runners immediately transition the box to an ERROR state using
this.updateBoxState(box, BoxState.ERROR, lockCode) (and return SYNC_AGAIN)
instead of calling createBox; otherwise for non-V0 runners proceed with await
runnerAdapter.createBox(...), then update to CREATING and return SYNC_AGAIN.
| import { BoxService } from './box.service' | ||
| import { BoxClass } from '../enums/box-class.enum' | ||
| import { WarmPool } from '../entities/warm-pool.entity' | ||
| import { BOX_IMAGE_REF_LABEL } from '../constants/curated-images.constant' |
There was a problem hiding this comment.
Make the warm-pool image assertion deterministic across env overrides.
Line 38 can fail when BOXLITE_SYSTEM_BASE_IMAGE is overridden to a ref that doesn’t include boxlite-agent-base. Assert against the resolver output instead of a substring.
Suggested patch
-import { BOX_IMAGE_REF_LABEL } from '../constants/curated-images.constant'
+import { BOX_IMAGE_REF_LABEL, resolveCuratedImageRef } from '../constants/curated-images.constant'
@@
- expect(box.labels[BOX_IMAGE_REF_LABEL]).toContain('boxlite-agent-base')
+ expect(box.labels[BOX_IMAGE_REF_LABEL]).toBe(resolveCuratedImageRef(undefined))Also applies to: 38-38
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/api/src/box/services/box.service.warm-pool.spec.ts` at line 10, The test
currently asserts the warm-pool image contains the literal substring
"boxlite-agent-base", which is brittle when BOXLITE_SYSTEM_BASE_IMAGE is
overridden; change the assertion to compare the returned image to the resolver
output instead. Import and call the image resolver used by the service (the same
function that resolves BOX_IMAGE_REF_LABEL) and assert the warm-pool image
equals that resolver result (use BOX_IMAGE_REF_LABEL as the input), replacing
the substring check on the image string.
Comment on lines
+1527
to
1533
| description: | | ||
| OCI image reference. | ||
| Hosted multi-tenant deployments restrict this to a curated set of | ||
| image keys (`base`, `python`, `node`; default `base`) and reject | ||
| other values with 400. | ||
| default: "alpine:latest" | ||
| example: python:3.11-slim |
There was a problem hiding this comment.
Align CreateBoxRequest.image default/example with curated-key contract.
Line 1532 still defaults to alpine:latest and Line 1533 still examples python:3.11-slim, which conflicts with the curated-key contract in the updated description and upstream API behavior. This can produce invalid client requests.
📌 Proposed contract-aligned patch
image:
type: string
description: |
- OCI image reference.
- Hosted multi-tenant deployments restrict this to a curated set of
- image keys (`base`, `python`, `node`; default `base`) and reject
- other values with 400.
- default: "alpine:latest"
- example: python:3.11-slim
+ Curated image key.
+ Allowed values: `base`, `python`, `node`.
+ Values outside this allowlist are rejected with 400.
+ enum: [base, python, node]
+ default: base
+ example: base📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| description: | | |
| OCI image reference. | |
| Hosted multi-tenant deployments restrict this to a curated set of | |
| image keys (`base`, `python`, `node`; default `base`) and reject | |
| other values with 400. | |
| default: "alpine:latest" | |
| example: python:3.11-slim | |
| image: | |
| type: string | |
| description: | | |
| Curated image key. | |
| Allowed values: `base`, `python`, `node`. | |
| Values outside this allowlist are rejected with 400. | |
| enum: [base, python, node] | |
| default: base | |
| example: base |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@openapi/box.openapi.yaml` around lines 1527 - 1533, The OpenAPI spec's
CreateBoxRequest.image field still shows a raw OCI image default/example that
contradicts the updated description requiring curated image keys; update the
default and example to a curated key (e.g., default: "base" and example:
"python") so the schema matches the description and upstream behavior for
CreateBoxRequest.image.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/test/e2e/fixture_setup.py`:
- Around line 85-87: The SQL in patch_admin_quota() currently updates every
organization whose organization_user."isDefaultForUser" = true; restrict the
UPDATE to the intended admin org by adding a filter on the admin user id (e.g.
organization_user."userId" = admin_user_id variable passed into
patch_admin_quota()), or alternatively update the function docstring/logging to
state it intentionally bumps quotas for all default orgs; ensure the unique
symbol patch_admin_quota() and the SQL block referencing
organization_user."isDefaultForUser" are updated accordingly so the query only
targets the admin user's organization (or the docstring reflects the broader
scope).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 7a1498b3-1da5-4475-b51d-c42fa478e4d6
📒 Files selected for processing (1)
scripts/test/e2e/fixture_setup.py
Comment on lines
+85
to
+87
| FROM organization_user | ||
| WHERE organization_user."organizationId" = organization.id | ||
| AND organization_user."isDefaultForUser" = true; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check how organization_user.isDefaultForUser is used elsewhere to understand the data model
rg -n "isDefaultForUser" --type ts -C 2Repository: boxlite-ai/boxlite
Length of output: 12884
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show patch_admin_quota and surrounding fixture setup logic
FILE="scripts/test/e2e/fixture_setup.py"
if [ ! -f "$FILE" ]; then
echo "Missing file: $FILE" >&2
exit 1
fi
echo "== snippet: $FILE (lines 60-130) =="
nl -ba "$FILE" | sed -n '60,130p'
echo
echo "== find: patch_admin_quota call sites =="
rg -n "patch_admin_quota" "$FILE"
echo
echo "== find: where admin user is created / default org set =="
rg -n "admin|default.*org|isDefaultForUser|organization_user|organizationId" "$FILE"
echo
echo "== find: any DB reset/seed logic in this file =="
rg -n "reset|truncate|seed|fixtures|e2e|wipe|drop" "$FILE"Repository: boxlite-ai/boxlite
Length of output: 188
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="scripts/test/e2e/fixture_setup.py"
test -f "$FILE"
echo "== snippet: $FILE (lines 60-140) =="
python3 - <<'PY'
import itertools
path="scripts/test/e2e/fixture_setup.py"
start=60
end=140
with open(path,'r',encoding='utf-8') as f:
for i,line in enumerate(f, start=1):
if start <= i <= end:
print(f"{i:4d}: {line.rstrip()}")
elif i > end:
break
PY
echo
echo "== find: patch_admin_quota call sites =="
rg -n "patch_admin_quota" "$FILE" || true
echo
echo "== find: admin/default/org/isDefaultForUser/organization_user usage =="
rg -n "admin|default|isDefaultForUser|organization_user|organizationId" "$FILE" || true
echo
echo "== find: DB reset/seed/wipe/drop logic in this file =="
rg -n "reset|truncate|seed|fixtures|e2e|wipe|drop" "$FILE" || trueRepository: boxlite-ai/boxlite
Length of output: 4424
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== snippet: scripts/test/e2e/fixture_setup.py (lines 130-220) =="
python3 - <<'PY'
path="scripts/test/e2e/fixture_setup.py"
start=130
end=220
with open(path,'r',encoding='utf-8') as f:
for i,line in enumerate(f, start=1):
if start <= i <= end:
print(f"{i:4d}: {line.rstrip()}")
elif i > end:
break
PY
echo
echo "== find: calls to patch_admin_quota across repo =="
rg -n "patch_admin_quota" --hidden
echo
echo "== search: e2e user/org creation or default org setup =="
rg -n "isDefaultForUser|default organization|default org|organization_user" scripts/test/e2e || true
echo
echo "== search: e2e DB wipe/reset =="
rg -n "truncate|drop table|drop schema|wipe|reset db|sequelize db|typeorm migration:run|recreate|docker compose down|psql .* -c" scripts/test/e2e || trueRepository: boxlite-ai/boxlite
Length of output: 2941
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== snippet: scripts/test/e2e/fixture_setup.py (lines 130-220) =="
python3 - <<'PY'
path="scripts/test/e2e/fixture_setup.py"
start=130
end=220
with open(path,'r',encoding='utf-8') as f:
for i,line in enumerate(f, start=1):
if start <= i <= end:
print(f"{i:4d}: {line.rstrip()}")
elif i > end:
break
PY
echo
echo "== find: calls to patch_admin_quota across repo =="
rg -n "patch_admin_quota" --hidden || true
echo
echo "== search: e2e user/org creation or default org setup =="
rg -n "isDefaultForUser|default organization|default org|organization_user" scripts/test/e2e || true
echo
echo "== search: e2e DB wipe/reset =="
rg -n "truncate|drop table|drop schema|wipe|reset db|recreate|docker compose down|psql .* -c" scripts/test/e2e || trueRepository: boxlite-ai/boxlite
Length of output: 2941
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect e2e bootstrap around admin user/org creation
BOOT="scripts/test/e2e/bootstrap.sh"
test -f "$BOOT" && echo "== snippet: $BOOT (search hits around admin/default) =="
rg -n "admin|default|isDefaultForUser|organization_user|quota|org quotas|createdBy" "$BOOT" || true
echo
echo "== bootstrap.sh: show around first 'admin' hit (if any) =="
python3 - <<'PY'
import re
path="scripts/test/e2e/bootstrap.sh"
with open(path,'r',encoding='utf-8') as f:
lines=f.readlines()
hits=[]
for i,l in enumerate(lines, start=1):
if re.search(r'admin|default|isDefaultForUser|organization_user|quota|org quotas|createdBy', l, re.IGNORECASE):
hits.append(i)
if not hits:
print("No matching lines found.")
raise SystemExit(0)
# show up to first 5 hits
for i in hits[:5]:
a=max(1,i-15); b=min(len(lines), i+15)
print(f"\n--- context around line {i} ({path}) [{a}-{b}] ---")
for j in range(a,b+1):
print(f"{j:4d}: {lines[j-1].rstrip()}")
PY
echo
echo "== inspect test that references fixture_setup::patch_admin_quota =="
python3 - <<'PY'
path="scripts/test/e2e/cases/test_quota_enforcement.py"
import os
if not os.path.exists(path):
print("Missing:", path); raise SystemExit(0)
with open(path,'r',encoding='utf-8') as f:
lines=f.readlines()
# show entire file if <=250 lines else excerpt
if len(lines)<=250:
start=1; end=len(lines)
else:
start=1; end=250
for i in range(start,end+1):
print(f"{i:4d}: {lines[i-1].rstrip()}")
PYRepository: boxlite-ai/boxlite
Length of output: 14864
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== e2e README: teardown / wipe-data usage =="
sed -n '1,120p' scripts/test/e2e/README.md || true
echo
echo "== teardown.sh snippet around wipe-data options =="
python3 - <<'PY'
path="scripts/test/e2e/teardown.sh"
start=1; end=140
with open(path,'r',encoding='utf-8') as f:
lines=f.readlines()
for i in range(start-1, min(end, len(lines))):
print(f"{i+1:4d}: {lines[i].rstrip()}")
PY
echo
echo "== search: e2e cases create users/organizations =="
rg -n "organizations|organization|users|userId|organization_user|isDefaultForUser" scripts/test/e2e/cases || true
echo
echo "== search: any endpoint usage that might mint non-admin users =="
rg -n "/v1/(users|organizations|orgs|memberships)|POST .*users|POST .*org|create.*user|create.*org" scripts/test/e2e/cases || trueRepository: boxlite-ai/boxlite
Length of output: 8545
Verify patch_admin_quota() SQL scope matches “admin org” intent
scripts/test/e2e/fixture_setup.py’s docstring says to bump quotas for “the admin user”, but the SQL updates every organization whose membership row has organization_user."isDefaultForUser" = true—it does not filter by the admin userId. While there’s a unique index enforcing only one default org per user (organization_user_default_user_unique), multiple users’ default orgs can still be updated if the e2e DB is reused between runs.
Either narrow the UPDATE to the admin user/org, or update the docstring/log to reflect that it bumps quotas for all default orgs.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/test/e2e/fixture_setup.py` around lines 85 - 87, The SQL in
patch_admin_quota() currently updates every organization whose
organization_user."isDefaultForUser" = true; restrict the UPDATE to the intended
admin org by adding a filter on the admin user id (e.g.
organization_user."userId" = admin_user_id variable passed into
patch_admin_quota()), or alternatively update the function docstring/logging to
state it intentionally bumps quotas for all default orgs; ensure the unique
symbol patch_admin_quota() and the SQL block referencing
organization_user."isDefaultForUser" are updated accordingly so the query only
targets the admin user's organization (or the docstring reflects the broader
scope).
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/dashboard/src/lib/cloudBox.ts (1)
213-247: 💤 Low valueConsider increasing poll interval to reduce API load.
The 100ms polling interval is aggressive. For a box startup operation that typically takes seconds, a 500ms-1000ms interval would reduce API calls without noticeably affecting user experience.
♻️ Suggested change
-const POLL_INTERVAL_MS = 100 +const POLL_INTERVAL_MS = 500🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/dashboard/src/lib/cloudBox.ts` around lines 213 - 247, In waitUntilStarted, the current short poll interval (POLL_INTERVAL_MS) is too aggressive; change the polling pause to a higher value (e.g., 500–1000ms) to reduce API load by either updating the POLL_INTERVAL_MS constant to 500 (or 1000) or replacing the await delay(POLL_INTERVAL_MS) call with await delay(500) (or a named constant like SLOW_POLL_INTERVAL_MS) so box startup polling uses the larger interval; keep the rest of waitUntilStarted, delay usage, and timeout logic unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/dashboard/src/lib/cloudBox.ts`:
- Around line 404-423: getRunCommand currently joins params?.argv into an
unescaped string, allowing shell injection; change it to safely pass argv by
base64-encoding each element and reconstructing the argv array in the shell
instead of interpolating raw text. For each CodeLanguage branch (JAVASCRIPT,
TYPESCRIPT, PYTHON) replace the `argv` interpolation with a safe construction:
compute safeArgs = (params?.argv || []).map(a => base64Encode(a)) and inject
those base64 tokens, then in the generated shell command decode them into an
array (e.g. loop or use a read -r -d ''/IFS trick to decode each token with
printf '%s' '<b64>' | base64 -d) and expand as separate arguments using
"${ARGS[@]}" (or equivalent) when invoking node/ts-node/python; ensure you
update all three return templates in getRunCommand so no raw argv content is
directly concatenated into shell commands.
---
Nitpick comments:
In `@apps/dashboard/src/lib/cloudBox.ts`:
- Around line 213-247: In waitUntilStarted, the current short poll interval
(POLL_INTERVAL_MS) is too aggressive; change the polling pause to a higher value
(e.g., 500–1000ms) to reduce API load by either updating the POLL_INTERVAL_MS
constant to 500 (or 1000) or replacing the await delay(POLL_INTERVAL_MS) call
with await delay(500) (or a named constant like SLOW_POLL_INTERVAL_MS) so box
startup polling uses the larger interval; keep the rest of waitUntilStarted,
delay usage, and timeout logic unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: fd0b823d-9cc7-4a2c-82aa-07841d9f7e1f
📒 Files selected for processing (65)
apps/api/Dockerfileapps/dashboard/project.jsonapps/dashboard/src/components/Playground/Box/CodeSnippets/code-language.test.tsapps/dashboard/src/components/Playground/Box/CodeSnippets/index.tsapps/dashboard/src/components/Playground/Box/CodeSnippets/types.tsapps/dashboard/src/components/Playground/Box/CodeSnippets/typescript.tsapps/dashboard/src/components/Playground/Box/CodeSnippetsResponse.tsxapps/dashboard/src/components/Playground/Box/Parameters/Management.tsxapps/dashboard/src/components/Playground/Box/Parameters/ProcessCodeExecution.tsxapps/dashboard/src/components/Playground/VNC/Interaction/Display.tsxapps/dashboard/src/components/Playground/VNC/Interaction/Keyboard.tsxapps/dashboard/src/components/Playground/VNC/Interaction/Mouse.tsxapps/dashboard/src/components/Playground/VNC/Interaction/Screenshot.tsxapps/dashboard/src/contexts/PlaygroundContext.tsxapps/dashboard/src/hooks/mutations/useCreateBoxMutation.tsxapps/dashboard/src/hooks/useBoxSession.tsapps/dashboard/src/lib/cloudBox.tsapps/dashboard/src/lib/playground.tsxapps/dashboard/src/providers/PlaygroundProvider.tsxapps/dashboard/tsconfig.app.jsonapps/dashboard/vite.config.mtsapps/eslint.config.mjsapps/infra-local/scripts/stack-up.shapps/libs/sdk-typescript/LICENSEapps/libs/sdk-typescript/README.mdapps/libs/sdk-typescript/hooks/typedoc-custom.mjsapps/libs/sdk-typescript/jest.config.jsapps/libs/sdk-typescript/package.jsonapps/libs/sdk-typescript/project.jsonapps/libs/sdk-typescript/src/Box.tsapps/libs/sdk-typescript/src/BoxLite.tsapps/libs/sdk-typescript/src/CodeInterpreter.tsapps/libs/sdk-typescript/src/ComputerUse.tsapps/libs/sdk-typescript/src/FileSystem.tsapps/libs/sdk-typescript/src/Git.tsapps/libs/sdk-typescript/src/Image.tsapps/libs/sdk-typescript/src/LspServer.tsapps/libs/sdk-typescript/src/ObjectStorage.tsapps/libs/sdk-typescript/src/Process.tsapps/libs/sdk-typescript/src/PtyHandle.tsapps/libs/sdk-typescript/src/Volume.tsapps/libs/sdk-typescript/src/__tests__/BoxLite.create-guards.test.tsapps/libs/sdk-typescript/src/code-toolbox/BoxJsCodeToolbox.tsapps/libs/sdk-typescript/src/code-toolbox/BoxPythonCodeToolbox.tsapps/libs/sdk-typescript/src/code-toolbox/BoxTsCodeToolbox.tsapps/libs/sdk-typescript/src/errors/BoxliteError.tsapps/libs/sdk-typescript/src/index.tsapps/libs/sdk-typescript/src/types/Charts.tsapps/libs/sdk-typescript/src/types/CodeInterpreter.tsapps/libs/sdk-typescript/src/types/ExecuteResponse.tsapps/libs/sdk-typescript/src/types/Pty.tsapps/libs/sdk-typescript/src/utils/ArtifactParser.tsapps/libs/sdk-typescript/src/utils/Binary.tsapps/libs/sdk-typescript/src/utils/FileTransfer.tsapps/libs/sdk-typescript/src/utils/Import.tsapps/libs/sdk-typescript/src/utils/Multipart.tsapps/libs/sdk-typescript/src/utils/Runtime.tsapps/libs/sdk-typescript/src/utils/Stream.tsapps/libs/sdk-typescript/src/utils/WebSocket.tsapps/libs/sdk-typescript/src/utils/otel.decorator.tsapps/libs/sdk-typescript/tsconfig.jsonapps/libs/sdk-typescript/tsconfig.lib.jsonapps/libs/sdk-typescript/tsconfig.spec.jsonapps/libs/sdk-typescript/typedoc.jsonapps/tsconfig.base.json
💤 Files with no reviewable changes (48)
- apps/libs/sdk-typescript/jest.config.js
- apps/libs/sdk-typescript/src/types/CodeInterpreter.ts
- apps/libs/sdk-typescript/tsconfig.lib.json
- apps/libs/sdk-typescript/README.md
- apps/libs/sdk-typescript/src/ObjectStorage.ts
- apps/api/Dockerfile
- apps/libs/sdk-typescript/LICENSE
- apps/libs/sdk-typescript/src/utils/ArtifactParser.ts
- apps/libs/sdk-typescript/src/types/ExecuteResponse.ts
- apps/libs/sdk-typescript/src/utils/Stream.ts
- apps/libs/sdk-typescript/src/utils/WebSocket.ts
- apps/libs/sdk-typescript/src/errors/BoxliteError.ts
- apps/libs/sdk-typescript/src/CodeInterpreter.ts
- apps/libs/sdk-typescript/src/types/Pty.ts
- apps/dashboard/src/components/Playground/Box/CodeSnippets/typescript.ts
- apps/libs/sdk-typescript/src/LspServer.ts
- apps/libs/sdk-typescript/hooks/typedoc-custom.mjs
- apps/libs/sdk-typescript/src/PtyHandle.ts
- apps/libs/sdk-typescript/src/index.ts
- apps/libs/sdk-typescript/src/Volume.ts
- apps/libs/sdk-typescript/src/code-toolbox/BoxJsCodeToolbox.ts
- apps/libs/sdk-typescript/tsconfig.json
- apps/libs/sdk-typescript/src/utils/Multipart.ts
- apps/libs/sdk-typescript/project.json
- apps/libs/sdk-typescript/src/utils/FileTransfer.ts
- apps/libs/sdk-typescript/src/utils/Runtime.ts
- apps/libs/sdk-typescript/tsconfig.spec.json
- apps/libs/sdk-typescript/src/utils/Import.ts
- apps/libs/sdk-typescript/src/utils/Binary.ts
- apps/libs/sdk-typescript/package.json
- apps/libs/sdk-typescript/src/Box.ts
- apps/libs/sdk-typescript/typedoc.json
- apps/libs/sdk-typescript/src/code-toolbox/BoxTsCodeToolbox.ts
- apps/libs/sdk-typescript/src/types/Charts.ts
- apps/libs/sdk-typescript/src/Git.ts
- apps/tsconfig.base.json
- apps/dashboard/src/components/Playground/Box/CodeSnippets/code-language.test.ts
- apps/dashboard/tsconfig.app.json
- apps/libs/sdk-typescript/src/Image.ts
- apps/libs/sdk-typescript/src/BoxLite.ts
- apps/libs/sdk-typescript/src/FileSystem.ts
- apps/libs/sdk-typescript/src/utils/otel.decorator.ts
- apps/eslint.config.mjs
- apps/libs/sdk-typescript/src/ComputerUse.ts
- apps/libs/sdk-typescript/src/code-toolbox/BoxPythonCodeToolbox.ts
- apps/libs/sdk-typescript/src/Process.ts
- apps/libs/sdk-typescript/src/tests/BoxLite.create-guards.test.ts
- apps/dashboard/vite.config.mts
✅ Files skipped from review due to trivial changes (2)
- apps/infra-local/scripts/stack-up.sh
- apps/dashboard/src/components/Playground/VNC/Interaction/Display.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/dashboard/src/providers/PlaygroundProvider.tsx
DorianZheng
reviewed
Jun 11, 2026
| runnerInfo(signal?: AbortSignal): Promise<RunnerInfo> | ||
|
|
||
| boxInfo(boxId: string): Promise<RunnerBoxInfo> | ||
| createBox(box: Box, artifactRef: string): Promise<void> |
DorianZheng
reviewed
Jun 11, 2026
| id: box.id, | ||
| boxId: box.boxId, | ||
| userId: box.organizationId, | ||
| artifactRef, |
2f5aac6 to
02d599f
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/runner/pkg/api/dto/box.go (1)
7-30:⚠️ Potential issue | 🟠 Major | ⚖️ Poor tradeoffGPU support asymmetry between create/resize/recover DTOs.
CreateBoxDTOno longer hasGpuQuota, whileResizeBoxDTO(line 34) andRecoverBoxDTO(line 50) retain GPU fields. This means:
- New boxes cannot be created with GPU
- Existing boxes can be resized to add GPU
- GPU boxes can be recovered
If GPU support is not yet ready for box creation, consider documenting this explicitly. If this is an oversight,
GpuQuotashould be added back toCreateBoxDTO.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/runner/pkg/api/dto/box.go` around lines 7 - 30, CreateBoxDTO is missing the GpuQuota field while ResizeBoxDTO and RecoverBoxDTO still include it, causing asymmetry; either add a GpuQuota int64 (or *int64 if nullable) with appropriate json tag and validation to CreateBoxDTO to match ResizeBoxDTO/RecoverBoxDTO, or add a clear comment/docstring above CreateBoxDTO noting GPU creation is intentionally unsupported; update any related validation or consumers of CreateBoxDTO (e.g., box creation handler) to accept and process the new field if you choose to add it.
♻️ Duplicate comments (1)
apps/api/src/box/managers/box-actions/box-start.action.ts (1)
62-73:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winAdd V0 runner guard before calling createBox.
The code unconditionally calls
runnerAdapter.createBox(box), butRunnerAdapterV0rejects this operation by design (V0 runners don't support CREATE_BOX jobs). When a box assigned to a V0 runner reaches this path,createBoxwill throw before the box transitions to a deterministic error state.🛡️ Proposed fix to guard against V0 runners
const runner = await this.runnerService.findOneOrFail(box.runnerId) if (runner.state !== RunnerState.READY) { return DONT_SYNC_AGAIN } + if (runner.apiVersion === '0') { + await this.updateBoxState( + box, + BoxState.ERROR, + lockCode, + undefined, + 'Box creation is not supported on runner API version 0', + ) + return DONT_SYNC_AGAIN + } + const runnerAdapter = await this.runnerAdapterFactory.create(runner) await runnerAdapter.createBox(box)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/api/src/box/managers/box-actions/box-start.action.ts` around lines 62 - 73, handleRunnerBoxUnknownStateOnDesiredStateStart currently calls runnerAdapter.createBox unconditionally which will throw for V0 runners; before calling createBox, detect V0 runners (either by checking runner.version/adapterVersion === 'v0' or by testing runnerAdapter instanceof RunnerAdapterV0) and, if V0, avoid calling createBox and instead transition the box to a deterministic error state via updateBoxState(box, BoxState.ERROR_DETERMINISTIC, lockCode) and return DONT_SYNC_AGAIN; otherwise proceed to call runnerAdapter.createBox and then updateBoxState to CREATING and return SYNC_AGAIN.
🧹 Nitpick comments (1)
apps/api/src/box/constants/curated-images.constant.ts (1)
62-70: 💤 Low valueConsider using nullish coalescing for env var fallback.
Line 69 uses
||which treats an empty string as falsy and falls back tofallbackRef. While this is likely acceptable (env vars are typically either set to a value or undefined), using??would be more explicit about the intended behavior.♻️ Optional refinement
- return process.env[envVar] || fallbackRef + return process.env[envVar] ?? fallbackRef🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/api/src/box/constants/curated-images.constant.ts` around lines 62 - 70, In resolveCuratedImageRef change the environment fallback to use nullish coalescing instead of logical OR: when reading process.env[envVar] in the function resolveCuratedImageRef use the ?? operator so an empty string is not treated as missing (i.e., replace the current "process.env[envVar] || fallbackRef" expression with "process.env[envVar] ?? fallbackRef" while keeping the validateCuratedImageKey and CURATED_IMAGE_ENV lookup intact).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/api/src/box/runner-adapter/runnerAdapter.v2.ts`:
- Around line 118-141: The create-box flow can send zero quotas which fail Go's
validate:"min=1" checks; add strict min=1 validation for CPU/memory/disk in the
create-box path: annotate the numeric fields in CreateBoxDto with `@Min`(1) (and
keep `@IsNumber`) and/or add explicit checks/defaulting in
BoxService.createBoxInternal to throw on or coerce values <1 before calling
runner job creation; ensure the values used when building the payload in
RunnerAdapter.createBox (cpu -> cpuQuota, mem -> memoryQuota, disk ->
storageQuota) come from the validated/coerced fields so the runner job always
satisfies the Go DTO constraints.
---
Outside diff comments:
In `@apps/runner/pkg/api/dto/box.go`:
- Around line 7-30: CreateBoxDTO is missing the GpuQuota field while
ResizeBoxDTO and RecoverBoxDTO still include it, causing asymmetry; either add a
GpuQuota int64 (or *int64 if nullable) with appropriate json tag and validation
to CreateBoxDTO to match ResizeBoxDTO/RecoverBoxDTO, or add a clear
comment/docstring above CreateBoxDTO noting GPU creation is intentionally
unsupported; update any related validation or consumers of CreateBoxDTO (e.g.,
box creation handler) to accept and process the new field if you choose to add
it.
---
Duplicate comments:
In `@apps/api/src/box/managers/box-actions/box-start.action.ts`:
- Around line 62-73: handleRunnerBoxUnknownStateOnDesiredStateStart currently
calls runnerAdapter.createBox unconditionally which will throw for V0 runners;
before calling createBox, detect V0 runners (either by checking
runner.version/adapterVersion === 'v0' or by testing runnerAdapter instanceof
RunnerAdapterV0) and, if V0, avoid calling createBox and instead transition the
box to a deterministic error state via updateBoxState(box,
BoxState.ERROR_DETERMINISTIC, lockCode) and return DONT_SYNC_AGAIN; otherwise
proceed to call runnerAdapter.createBox and then updateBoxState to CREATING and
return SYNC_AGAIN.
---
Nitpick comments:
In `@apps/api/src/box/constants/curated-images.constant.ts`:
- Around line 62-70: In resolveCuratedImageRef change the environment fallback
to use nullish coalescing instead of logical OR: when reading
process.env[envVar] in the function resolveCuratedImageRef use the ?? operator
so an empty string is not treated as missing (i.e., replace the current
"process.env[envVar] || fallbackRef" expression with "process.env[envVar] ??
fallbackRef" while keeping the validateCuratedImageKey and CURATED_IMAGE_ENV
lookup intact).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: f9f27c0f-817f-461e-810a-935c8d9a4e3c
📒 Files selected for processing (101)
apps/api/Dockerfileapps/api/src/box/constants/curated-images.constant.spec.tsapps/api/src/box/constants/curated-images.constant.tsapps/api/src/box/dto/box.dto.tsapps/api/src/box/dto/create-box.dto.tsapps/api/src/box/entities/box.entity.tsapps/api/src/box/managers/box-actions/box-start.action.tsapps/api/src/box/runner-adapter/runnerAdapter.tsapps/api/src/box/runner-adapter/runnerAdapter.v0.tsapps/api/src/box/runner-adapter/runnerAdapter.v2.createBox.spec.tsapps/api/src/box/runner-adapter/runnerAdapter.v2.tsapps/api/src/box/services/box.service.tsapps/api/src/box/services/box.service.warm-pool.spec.tsapps/api/src/boxlite-rest/mappers/box-to-box.mapper.spec.tsapps/api/src/boxlite-rest/mappers/box-to-box.mapper.tsapps/api/src/migrations/1741087887225-migration.tsapps/dashboard/.storybook/main.tsapps/dashboard/project.jsonapps/dashboard/src/components/Box/CreateBoxSheet.tsxapps/dashboard/src/components/Playground/Box/CodeSnippets/code-language.test.tsapps/dashboard/src/components/Playground/Box/CodeSnippets/index.tsapps/dashboard/src/components/Playground/Box/CodeSnippets/python.tsapps/dashboard/src/components/Playground/Box/CodeSnippets/types.tsapps/dashboard/src/components/Playground/Box/CodeSnippets/typescript.tsapps/dashboard/src/components/Playground/Box/CodeSnippetsResponse.tsxapps/dashboard/src/components/Playground/Box/Parameters/Management.tsxapps/dashboard/src/components/Playground/Box/Parameters/ProcessCodeExecution.tsxapps/dashboard/src/components/Playground/VNC/Interaction/Display.tsxapps/dashboard/src/components/Playground/VNC/Interaction/Keyboard.tsxapps/dashboard/src/components/Playground/VNC/Interaction/Mouse.tsxapps/dashboard/src/components/Playground/VNC/Interaction/Screenshot.tsxapps/dashboard/src/contexts/PlaygroundContext.tsxapps/dashboard/src/hooks/mutations/useCreateBoxMutation.tsxapps/dashboard/src/hooks/useBoxSession.tsapps/dashboard/src/lib/cloudBox.tsapps/dashboard/src/lib/onboarding-code-examples.tsapps/dashboard/src/lib/playground.tsxapps/dashboard/src/providers/PlaygroundProvider.tsxapps/dashboard/tsconfig.app.jsonapps/dashboard/vite.config.mtsapps/eslint.config.mjsapps/infra-local/scripts/stack-up.shapps/libs/sdk-typescript/LICENSEapps/libs/sdk-typescript/README.mdapps/libs/sdk-typescript/hooks/typedoc-custom.mjsapps/libs/sdk-typescript/jest.config.jsapps/libs/sdk-typescript/package.jsonapps/libs/sdk-typescript/project.jsonapps/libs/sdk-typescript/src/Box.tsapps/libs/sdk-typescript/src/BoxLite.tsapps/libs/sdk-typescript/src/CodeInterpreter.tsapps/libs/sdk-typescript/src/ComputerUse.tsapps/libs/sdk-typescript/src/FileSystem.tsapps/libs/sdk-typescript/src/Git.tsapps/libs/sdk-typescript/src/Image.tsapps/libs/sdk-typescript/src/LspServer.tsapps/libs/sdk-typescript/src/ObjectStorage.tsapps/libs/sdk-typescript/src/Process.tsapps/libs/sdk-typescript/src/PtyHandle.tsapps/libs/sdk-typescript/src/Volume.tsapps/libs/sdk-typescript/src/__tests__/BoxLite.create-guards.test.tsapps/libs/sdk-typescript/src/code-toolbox/BoxJsCodeToolbox.tsapps/libs/sdk-typescript/src/code-toolbox/BoxPythonCodeToolbox.tsapps/libs/sdk-typescript/src/code-toolbox/BoxTsCodeToolbox.tsapps/libs/sdk-typescript/src/errors/BoxliteError.tsapps/libs/sdk-typescript/src/index.tsapps/libs/sdk-typescript/src/types/Charts.tsapps/libs/sdk-typescript/src/types/CodeInterpreter.tsapps/libs/sdk-typescript/src/types/ExecuteResponse.tsapps/libs/sdk-typescript/src/types/Pty.tsapps/libs/sdk-typescript/src/utils/ArtifactParser.tsapps/libs/sdk-typescript/src/utils/Binary.tsapps/libs/sdk-typescript/src/utils/FileTransfer.tsapps/libs/sdk-typescript/src/utils/Import.tsapps/libs/sdk-typescript/src/utils/Multipart.tsapps/libs/sdk-typescript/src/utils/Runtime.tsapps/libs/sdk-typescript/src/utils/Stream.tsapps/libs/sdk-typescript/src/utils/WebSocket.tsapps/libs/sdk-typescript/src/utils/otel.decorator.tsapps/libs/sdk-typescript/tsconfig.jsonapps/libs/sdk-typescript/tsconfig.lib.jsonapps/libs/sdk-typescript/tsconfig.spec.jsonapps/libs/sdk-typescript/typedoc.jsonapps/runner/pkg/api/dto/box.goapps/runner/pkg/boxlite/client.goapps/runner/pkg/boxlite/stubs.goapps/tsconfig.base.jsonscripts/test/e2e/README.mdscripts/test/e2e/bootstrap.shscripts/test/e2e/cases/conftest.pyscripts/test/e2e/cases/test_c_entry.pyscripts/test/e2e/cases/test_cli_detach_recovery.pyscripts/test/e2e/cases/test_cli_entry.pyscripts/test/e2e/cases/test_error_code_mapping.pyscripts/test/e2e/cases/test_go_entry.pyscripts/test/e2e/cases/test_node_entry.pyscripts/test/e2e/cases/test_quota_enforcement.pyscripts/test/e2e/fixture_setup.pyscripts/test/e2e/sdks/c/e2e_basic.cscripts/test/e2e/sdks/go/e2e_basic.goscripts/test/e2e/sdks/node/e2e_basic.ts
💤 Files with no reviewable changes (49)
- apps/libs/sdk-typescript/src/types/CodeInterpreter.ts
- apps/tsconfig.base.json
- apps/dashboard/.storybook/main.ts
- apps/libs/sdk-typescript/tsconfig.json
- apps/libs/sdk-typescript/src/utils/Multipart.ts
- apps/libs/sdk-typescript/project.json
- apps/libs/sdk-typescript/src/code-toolbox/BoxTsCodeToolbox.ts
- apps/dashboard/src/components/Playground/Box/CodeSnippets/code-language.test.ts
- apps/libs/sdk-typescript/src/ObjectStorage.ts
- apps/libs/sdk-typescript/src/BoxLite.ts
- apps/libs/sdk-typescript/typedoc.json
- apps/libs/sdk-typescript/src/Box.ts
- apps/libs/sdk-typescript/src/CodeInterpreter.ts
- apps/libs/sdk-typescript/src/errors/BoxliteError.ts
- apps/libs/sdk-typescript/src/utils/WebSocket.ts
- apps/libs/sdk-typescript/src/Volume.ts
- apps/libs/sdk-typescript/src/code-toolbox/BoxJsCodeToolbox.ts
- apps/libs/sdk-typescript/src/types/Pty.ts
- apps/dashboard/src/components/Playground/Box/CodeSnippets/typescript.ts
- apps/libs/sdk-typescript/src/types/ExecuteResponse.ts
- apps/libs/sdk-typescript/LICENSE
- apps/libs/sdk-typescript/src/tests/BoxLite.create-guards.test.ts
- apps/libs/sdk-typescript/src/utils/Binary.ts
- apps/libs/sdk-typescript/tsconfig.spec.json
- apps/libs/sdk-typescript/src/index.ts
- apps/libs/sdk-typescript/src/code-toolbox/BoxPythonCodeToolbox.ts
- apps/api/Dockerfile
- apps/libs/sdk-typescript/src/utils/Stream.ts
- apps/libs/sdk-typescript/src/utils/otel.decorator.ts
- apps/libs/sdk-typescript/README.md
- apps/libs/sdk-typescript/src/LspServer.ts
- apps/eslint.config.mjs
- apps/libs/sdk-typescript/src/Image.ts
- apps/libs/sdk-typescript/src/PtyHandle.ts
- apps/libs/sdk-typescript/tsconfig.lib.json
- apps/libs/sdk-typescript/src/Process.ts
- apps/libs/sdk-typescript/src/utils/FileTransfer.ts
- apps/libs/sdk-typescript/src/ComputerUse.ts
- apps/libs/sdk-typescript/jest.config.js
- apps/libs/sdk-typescript/src/FileSystem.ts
- apps/dashboard/tsconfig.app.json
- apps/libs/sdk-typescript/src/types/Charts.ts
- apps/libs/sdk-typescript/package.json
- apps/libs/sdk-typescript/hooks/typedoc-custom.mjs
- apps/libs/sdk-typescript/src/utils/Runtime.ts
- apps/libs/sdk-typescript/src/Git.ts
- apps/libs/sdk-typescript/src/utils/ArtifactParser.ts
- apps/dashboard/vite.config.mts
- apps/libs/sdk-typescript/src/utils/Import.ts
✅ Files skipped from review due to trivial changes (8)
- apps/dashboard/src/components/Playground/Box/CodeSnippets/types.ts
- scripts/test/e2e/sdks/go/e2e_basic.go
- scripts/test/e2e/cases/test_go_entry.py
- apps/api/src/migrations/1741087887225-migration.ts
- apps/infra-local/scripts/stack-up.sh
- scripts/test/e2e/README.md
- apps/dashboard/src/lib/onboarding-code-examples.ts
- scripts/test/e2e/cases/test_quota_enforcement.py
🚧 Files skipped from review as they are similar to previous changes (22)
- scripts/test/e2e/sdks/node/e2e_basic.ts
- apps/dashboard/project.json
- apps/api/src/box/runner-adapter/runnerAdapter.v0.ts
- apps/dashboard/src/components/Playground/VNC/Interaction/Display.tsx
- scripts/test/e2e/cases/test_c_entry.py
- scripts/test/e2e/bootstrap.sh
- apps/dashboard/src/components/Playground/Box/CodeSnippets/index.ts
- scripts/test/e2e/cases/test_error_code_mapping.py
- scripts/test/e2e/cases/test_cli_detach_recovery.py
- scripts/test/e2e/cases/test_node_entry.py
- apps/dashboard/src/lib/playground.tsx
- apps/dashboard/src/components/Playground/VNC/Interaction/Mouse.tsx
- scripts/test/e2e/cases/test_cli_entry.py
- apps/dashboard/src/components/Playground/VNC/Interaction/Screenshot.tsx
- scripts/test/e2e/cases/conftest.py
- apps/dashboard/src/providers/PlaygroundProvider.tsx
- apps/api/src/box/constants/curated-images.constant.spec.ts
- apps/dashboard/src/components/Playground/Box/Parameters/Management.tsx
- apps/dashboard/src/contexts/PlaygroundContext.tsx
- apps/dashboard/src/hooks/useBoxSession.ts
- scripts/test/e2e/fixture_setup.py
- apps/dashboard/src/lib/cloudBox.ts
Comment on lines
+11
to
+13
| UserId string `json:"userId,omitempty"` | ||
| OCIImageRef string `json:"ociImageRef" validate:"required"` | ||
| OsUser string `json:"osUser,omitempty"` |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if runner box creation has default logic for optional UserId/OsUser
# Find where CreateBoxDTO is consumed
rg -nP 'CreateBoxDTO' --type=go -g '**/runner/**' -A10 | rg -P '(UserId|OsUser)' -A3 -B3
# Check if there are default value assignments
ast-grep --pattern $'if $_.UserId == "" {
$$$
}'Repository: boxlite-ai/boxlite
Length of output: 1506
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find all reads/usages of CreateBoxDTO.UserId / OsUser
rg -n "CreateBoxDTO\.UserId|CreateBoxDTO\.OsUser|\.UserId\b|\.OsUser\b" apps/runner/pkg/boxlite -S || true
# Inspect client create and env wiring (look for daemonBoxEnv / Create)
rg -n "func \(.*\) Create|daemonBoxEnv|CreateBoxDTO" apps/runner/pkg/boxlite -S || true
# Show the most relevant files/sections for manual review with line numbers
for f in $(rg -l "daemonBoxEnv|func .*Create|CreateBoxDTO" apps/runner/pkg/boxlite -S); do
echo "----- $f (head 220) -----"
sed -n '1,220p' "$f" | cat -n
echo "----- $f (tail 220) -----"
tail -n 220 "$f" | cat -n
done
# Check other runner layers that might validate/populate fields from DTO to daemon
rg -n "UserId|OsUser|userId|osUser" apps/runner/pkg -S -g'*.go' || trueRepository: boxlite-ai/boxlite
Length of output: 50374
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show any references to UserId/OsUser inside the BoxLite client create flow
rg -n "boxDto\.(UserId|OsUser)|\.UserId\b|\.OsUser\b" apps/runner/pkg/boxlite/client.go -S || true
# Print just the Create function body (bounded) with line numbers
python3 - <<'PY'
import re, itertools, pathlib
path = pathlib.Path("apps/runner/pkg/boxlite/client.go")
txt = path.read_text(encoding="utf-8", errors="ignore").splitlines()
start=None
for i,line in enumerate(txt):
if re.match(r'func \(c \*Client\) Create\(', line):
start=i
break
if start is None:
raise SystemExit("Create not found")
# naive brace matching to end of function
brace=0
infunc=False
out=[]
for i in range(start, len(txt)):
line=txt[i]
if not infunc:
if "{" in line:
infunc=True
brace += line.count("{")-line.count("}")
else:
continue
else:
brace += line.count("{")-line.count("}")
out.append((i+1,line))
if infunc and brace==0:
break
for ln,l in out:
if "UserId" in l or "OsUser" in l or True:
print(f"{ln}:{l}")
PY
# Check if daemon env test or other unit tests mention UserId/OsUser
rg -n "UserId|OsUser" apps/runner/pkg/boxlite -S || trueRepository: boxlite-ai/boxlite
Length of output: 3419
CreateBoxDTO UserId / OsUser are unused by runner box creation
UserId string `json:"userId,omitempty"`
OCIImageRef string `json:"ociImageRef" validate:"required"`
OsUser string `json:"osUser,omitempty"`apps/runner/pkg/boxlite/client.go’s Client.Create and daemonBoxEnv never read boxDto.UserId or boxDto.OsUser (only Id, OCIImageRef, quotas, Env, Volumes, networking, and daemon env vars). RecoverBox does populate these DTO fields, but they’re still ignored when passed into Client.Create.
If the in-box daemon is expected to use UserId/OsUser, they aren’t currently propagated—document the omission or wire the values through the create/env path.
Contributor
Author
Identity model note (pre-empting a likely review question)Q: the engine mints its own BoxID and won't accept an external id — so how can the cloud-minted 12-char id be "the" box identity? A: the cloud id is never injected as the engine's id. It rides as the engine box name (unique, indexed, queryable), exactly the way kubelet attaches the pod UID to containers it doesn't own. Side by side: Consequences:
|
This was referenced Jun 12, 2026
G4614
added a commit
that referenced
this pull request
Jun 12, 2026
POST /api/snapshots and GET /api/snapshots?name=... no longer exist on the API — there is no snapshot controller in apps/api/src/**. PR #735 (feat(box): create boxes from curated images) replaced the pre-register-then-boot model with lazy curated-image pulls at box start, so the e2e test workflow no longer needs to seed snapshot rows. The pre-registration block was failing the "Configure pytest profile p1" step with `Cannot POST /api/snapshots` 404. The previous run got masked by the SQL-escape bug in quota init (now fixed); this surfaced as the next blocker.
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.
Minimal image rebuild after the box_template/catalog deletion: a hardcoded
allowlist of 3 curated images, no DB table. Runner pulls any OCI ref directly
(runtime.Create(artifactRef)), so the catalog lives in code.
- curated-images.constant.ts: keys 'base'|'python'|'node' -> ghcr digest refs
from BOXLITE_SYSTEM_{BASE,PYTHON,NODE}_IMAGE env (already set in sst.config),
resolveCuratedImageRef() validates at the API boundary (400 if off-list)
- create-box.dto (internal) gains image?; mapper now threads dto.image (was dropped)
- box.service.createBoxInternal: resolve ref, stash on box.labels['boxlite.io/image-ref']
(zero migration)
- runnerAdapter.createBox (v2): enqueue CREATE_BOX job with hand-built payload whose
key is artifactRef (NOT the generated client's snapshot field — V2 Go validate trap)
- box-start.action: read the label, call createBox, set CREATING; existing
handleCreateBoxJobCompletion drives CREATING->STARTED
api/src tsc 0. Contract = opaque keys (user can't pass arbitrary OCI ref). Images
stay private ghcr (runner token already wired). Dashboard 3-image picker is a
separate follow-up (mid-rebuild).
…ho curated key
Three follow-ups on the curated-image create path:
- replaceLabels must not let a user-supplied label overwrite the reserved
boxlite.io/image-ref label: it holds the resolved curated OCI ref the
runner pulls, so overwriting it would escape the curated allowlist and
make the runner pull an arbitrary image with its private-registry token.
- createForWarmPool must stash the default curated image ref on the same
reserved label; without it warm-pool boxes ERROR on start ("missing
image ref") and the refill loop recreates them indefinitely.
- Box responses now echo the curated key the box was created with
(reverse-mapped from the reserved label) instead of a hardcoded '';
the shared REST contract documents that hosted deployments restrict
image to the curated keys.
Unit tests cover the allowlist boundary (invalid key -> 400), the
CREATE_BOX payload contract (artifactRef key, never snapshot), the
reserved-label guard, the warm-pool default, and the create/echo mapping.
The API now accepts only curated image keys (base|python|node) on box create. Switch the suite's image literals from alpine:3.23 to the 'base' key; bootstrap maps the curated env overrides to the same public refs the suite pulled before, so runner-side behavior is unchanged. Drop fixture_setup's snapshot registration: it targets the /snapshots endpoint and snapshot table that the box_template removal deleted (bootstrap hard-fails on main today). e2e-stack has been red on main since that removal; this adaptation is a precondition for it to go green again.
…ner contract to ociImageRef The curated image key now lives on the Box entity (image column, default 'base') instead of hiding a resolved OCI ref in a reserved label. The key resolves to a pinned ref at the last point before the payload leaves the API: RunnerAdapterV2.createBox. This removes the replaceLabels sanitization (the attack surface is gone with the label) and the ref->key reverse lookup in the REST mapper, and survives image digest rotation since the DB stores the stable key. Runner contract changes (atomic with the adapter payload): - dto.CreateBoxDTO: ArtifactRef -> OCIImageRef (json ociImageRef) - drop dead fields GpuQuota and Registry; relax userId/osUser (unread in the create flow, previously validate:required) - API payload drops boxId/gpuQuota/authToken (unread by the runner) createBox(box, artifactRef) collapses to createBox(box).
…key translation layer - image is now a full pinned OCI ref at every layer: create request -> Box.image column -> CREATE_BOX payload -> runner runtime.Create. The curated allowlist shrinks to a thin boundary gate (supportedImages / assertSupportedImage) designed to be deleted when custom images land; rejections list the supported refs so callers can self-correct. - rename the runner contract field ociImageRef -> image (Go: Image, json "image"), aligning with the embedded SDKs' image parameter. - restore boxId in the CREATE_BOX payload so runner logs show the user-facing short id instead of the internal uuid. - e2e: stack-aware default image — fixture_setup records the local stack's base image in the credentials profile; conftest resolves BOXLITE_E2E_IMAGE env > profile default_image > pinned ghcr fallback. - drop WorkspaceDto's redundant image declaration (now inherited from BoxDto).
…-char box id A box now has exactly one identity: the 12-char base62 public id, minted in-memory at construction and used as the primary key, in CREATE_BOX payloads, as the engine VM name, in runner logs, and in every API response. The internal uuid (a Daytona leftover) is gone — it offered neither the small-ordered-key benefit nor the ABA protection that justify a dual-id scheme, and it kept leaking into runner logs and the in-box BOXLITE_BOX_ID env. - Box entity: id = generateBoxId(); the boxId column, its unique and org-scoped indexes, and the uuid default are removed (baseline migration updated; pre-launch DB rebuild). - Lookups collapse: findOneByIdOrName / getOrganizationId lose the boxId-first branch; lookup caches lose the by-box-id variants. - CREATE_BOX payload sends a single id; the Go DTO drops BoxId and client.go drops the publicBoxId fallback. - Admin overview mirrors id into the legacy boxId DTO field; API surface is unchanged. API jest: 112/112. Go pkg build+vet+gofmt clean (cmd link needs the prebuilt libboxlite.a absent in this worktree). Dashboard tsc clean.
6c4776a to
2c04345
Compare
law-chain-hot
added 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.
…ents (#744) ## 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: ```text 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.
G4614
added a commit
that referenced
this pull request
Jun 12, 2026
apps/libs/sdk-typescript was deleted in #738 (merged into #735 in this branch). The Dockerfile.source COPY line was kept by mistake and broke the Deploy API image build: failed to compute cache key: failed to calculate checksum of ref ... "/apps/libs/sdk-typescript": not found The remaining 4 libs (runner-api-client, api-client, analytics-api-client, toolbox-api-client) are what the API actually imports at runtime.
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (8)
apps/runner/pkg/boxlite/stubs.go (1)
95-108:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftPreserve the box’s original image during recovery.
This recreates every recovered box from
alpine:latest, so any Python/Node/customized box comes back with the wrong root filesystem. It also bypasses the PR’s pinned-image contract by reintroducing a mutable tag. Recovery needs the original persisted image ref (orinfo.Imagecaptured before destroy), not a hardcoded fallback.Possible direction
createDto := dto.CreateBoxDTO{ Id: boxId, - Image: "alpine:latest", + Image: recoverDto.Image, // or the pre-destroy runtime info image OsUser: recoverDto.OsUser,If
RecoverBoxDTOintentionally stays image-less, fetch and retain the current box image before callingDestroy.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/runner/pkg/boxlite/stubs.go` around lines 95 - 108, The recovery currently hardcodes "alpine:latest" when building createDto, losing the original box image; change createDto to use the persisted image instead (e.g., use recoverDto.Image or the captured info.Image that was saved before Destroy) so CreateBoxDTO.Id/Image/… preserves the original image; locate the createDto construction in stubs.go and replace the hardcoded "alpine:latest" with the recovered image field or fetch the box info image prior to destroy and pass that into CreateBoxDTO.apps/api-client-go/api/openapi.yaml (2)
9938-9979:⚠️ Potential issue | 🟠 Major | ⚡ Quick winStop documenting
idas the removed internal UUID.The
BoxandWorkspaceschemas still describeidas an internal UUID and keep UUID examples, but this PR’s contract replaces that with the single 12-character public box id. Leaving these docs as-is will keep generated docs and SDK consumers on the deprecated identity model.Suggested schema/docs update
Box: example: - id: fd955d93-e74a-48e7-9f2d-fcbe6dd9e920 + id: aB3xYz91KqLm @@ id: - description: The internal UUID of the box - example: fd955d93-e74a-48e7-9f2d-fcbe6dd9e920 + description: The canonical 12-character public box ID + example: aB3xYz91KqLm type: stringApply the same update to
Workspace.id.Also applies to: 12487-12529
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/api-client-go/api/openapi.yaml` around lines 9938 - 9979, The Box schema's id property is still documented as an internal UUID—update the Box schema (properties.id / example) to reflect the new 12-character public box id (replace UUID example with a 12-char example and change the description to indicate "12-character public box id"); make the same change for Workspace.id and any other id occurrences in this file (e.g., the other range mentioned) so generated docs/SDKs use the new public id format instead of the deprecated internal UUID.
13450-13472:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winReconcile
organizationIdwith its description and examples.Line 13469 renames the field to
organizationId, but Line 13470 still describes it as a public box id and the changed examples use a box-like value. One of those is wrong, and the generated admin docs will teach clients the wrong identifier semantics.Also applies to: 14788-14803
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/api-client-go/api/openapi.yaml` around lines 13450 - 13472, The OpenAPI schema's organizationId field is inconsistent: the property name is organizationId but its description and example look like a public box ID; update the schema so the semantics match—either (A) change the description to reflect an organization identifier (e.g., "Organization ID") and use an organization-style example (e.g., org_abc123) for the organizationId property and in the example object, or (B) if the field actually represents a public box ID, rename the property back to boxId everywhere (including the example object and properties section) and keep the box-style example (box_abc123); apply the same reconciliation for the repeated block at lines referenced (the second occurrence).apps/api-client-go/model_box.go (1)
22-25:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRegenerate the published client/docs from an updated
idschema description.The same stale contract text appears in
apps/api-client-go/model_box.goandapps/libs/api-client/src/docs/Workspace.md:idis still described as an internal UUID even though this PR makes it the single 12-character public box ID. The shared root cause looks to be the source OpenAPI/schema description, so please fix that once and regenerate both artifacts.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/api-client-go/model_box.go` around lines 22 - 25, The Box.Id field and its schema description are stale—its comment says "internal UUID" but the PR changes it to the public 12-character box ID; update the OpenAPI/schema description for the Box model (affecting the Box struct Id field) to describe it as the single 12-character public box ID, then regenerate the Go client and docs so model_box.go (Box.Id) and Workspace.md reflect the new description; ensure the change is made in the source OpenAPI schema that drives generation so both artifacts are updated.apps/api/src/box/services/box.service.box-id.spec.ts (1)
20-40:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winThe fallback half of this test isn't actually exercised.
The spec name says "falling back to name only", but
findOneresolves on the first call and Line 31 asserts there is only one query. Either trim the title to the ID-only behavior, or add a second case that returnsnullfor the ID lookup and verifies the name fallback.apps/api/src/box/services/box.service.ts (1)
463-468:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUse a case-sensitive operator for box ID search.
The migrated box IDs are base62, so case is significant.
ILike(${id}%)makes the ID prefix filter case-insensitive and can return the wrong rows once two IDs differ only by letter case. Keep the name search case-insensitive if you want, but theidbranch should use a case-sensitive predicate.Suggested fix
-import { Not, Repository, LessThan, In, JsonContains, FindOptionsWhere, ILike } from 'typeorm' +import { Not, Repository, LessThan, In, JsonContains, FindOptionsWhere, ILike, Like } from 'typeorm' … - const idFilter = ILike(`${id}%`) + const idFilter = Like(`${id}%`)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/api/src/box/services/box.service.ts` around lines 463 - 468, The id prefix filter is using ILike (case-insensitive) which breaks base62 box IDs; change the id filter to a case-sensitive operator (e.g., use Like instead of ILike) so idFilter = Like(`${id}%`) while leaving nameFilter/other options unchanged; update the usage in the same return block that spreads baseFindOptions and nameFilter so the id branch uses the new idFilter (symbols: ILike, Like, idFilter, baseFindOptions, nameFilter).apps/api/src/box/dto/box.dto.ts (1)
38-42:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUpdate the Swagger contract for
id.This property is no longer an internal UUID. The PR migrates
idto the single 12-char public box identifier, but the docs still describe a UUID and show a UUID example. That will mislead API consumers and any generated examples built from this schema.Suggested fix
`@ApiProperty`({ - description: 'The internal UUID of the box', - example: 'fd955d93-e74a-48e7-9f2d-fcbe6dd9e920', + description: 'The public 12-character box ID', + example: 'aB3dE6fH9jK2', }) id: string🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/api/src/box/dto/box.dto.ts` around lines 38 - 42, The ApiProperty for the id field currently describes and exemplifies a UUID but the code now uses a 12-character public box identifier; update the ApiProperty on the id property (the id field in box.dto.ts) to describe it as "The public 12-character box identifier" and change the example to a 12-char value (e.g. "abcd1234ef56" or similar) so Swagger and generated examples reflect the new public ID format; keep the type as string and preserve the ApiProperty decorator usage.apps/api/src/box/repositories/box.repository.ts (1)
202-229:⚠️ Potential issue | 🟠 Major | ⚡ Quick winInvalidate the previous auth token too.
When the token rotates, the stale cache entry lives under
previousBox.authToken, but this branch only removesupdatedBox.authToken. That can leave the old credential resolving from cache until TTL expiry. Evict the previous token key here, and keep the new-token invalidation too if you want to clear prior misses.Suggested fix
try { if (updatedBox.authToken !== previousBox.authToken) { this.boxLookupCacheInvalidationService.invalidate({ + authToken: previousBox.authToken, + }) + this.boxLookupCacheInvalidationService.invalidate({ authToken: updatedBox.authToken, }) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/api/src/box/repositories/box.repository.ts` around lines 202 - 229, In invalidateLookupCacheOnUpdate, when authToken changes you only invalidate updatedBox.authToken; also call boxLookupCacheInvalidationService.invalidate for the previous token (previousBox.authToken) so the stale cache key is removed; keep both invalidations (new and previous) and guard against empty/undefined tokens before calling invalidate to avoid no-op or invalid requests (refer to invalidateLookupCacheOnUpdate, updatedBox, previousBox, and boxLookupCacheInvalidationService.invalidate).
🧹 Nitpick comments (5)
apps/dashboard/src/components/Playground/Box/CodeSnippets/python.ts (1)
64-66: ⚡ Quick winDigest duplication: extract default base image ref to a shared constant.
The default base image digest
ghcr.io/boxlite-ai/boxlite-agent-base@sha256:834dcb...is hardcoded in five locations: the Python snippet generator inapps/dashboard/src/components/Playground/Box/CodeSnippets/python.ts(line 65), and four onboarding examples inapps/dashboard/src/lib/onboarding-code-examples.ts(lines 27, 57, 108, and 146). This violates the DRY principle: when the digest rotates, all five sites must be updated in lockstep. Extract the digest to a shared constant (or import it fromCreateBoxSheet.tsxifSUPPORTED_IMAGES[0]is already the single source of truth) to prevent drift and simplify future updates.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/dashboard/src/components/Playground/Box/CodeSnippets/python.ts` around lines 64 - 66, The hardcoded default base image digest string appears in python.ts and four onboarding examples; extract it into a single exported constant (e.g., DEFAULT_BASE_IMAGE or DEFAULT_AGENT_BASE_IMAGE) and import that constant everywhere instead of repeating the literal; alternatively import the canonical SUPPORTED_IMAGES[0] from CreateBoxSheet.tsx if that is the single source of truth; update references in apps/dashboard/src/components/Playground/Box/CodeSnippets/python.ts (the snippet that uses p.config.createBoxFromImage) and in the four entries in onboarding-code-examples.ts to use the shared constant/import.apps/dashboard/src/hooks/useBoxWsSync.ts (1)
49-55: 💤 Low valueRedundant detail cache update.
When
matchesActiveBoxreturns true andboxIdis defined,box.id === boxIdis guaranteed. Lines 52-53 therefore update the same detail cache entry twice. Consider removing line 52:♻️ Proposed simplification
const optimisticUpdate = (box: Box, state: BoxState) => { updateStateInListCache(box.id, state) if (boxId) { - updateStateInDetailCache(boxId, state) updateStateInDetailCache(box.id, state) } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/dashboard/src/hooks/useBoxWsSync.ts` around lines 49 - 55, In optimisticUpdate, when boxId is defined it is guaranteed to equal box.id, so the second call to updateStateInDetailCache(box.id, state) is redundant; remove that call and only call updateStateInDetailCache(boxId, state) (keep updateStateInListCache(box.id, state) unchanged) so the detail cache is updated exactly once; the relevant symbols are optimisticUpdate, updateStateInListCache, updateStateInDetailCache, box.id and boxId.apps/runner/pkg/boxlite/client.go (1)
223-227: ExternalRef lookup matches the SDK/runtime—don’t revert to WithName
WithExternalRefis designed to be retrievable viaruntime.Get(externalRef)(sdks/go/options.go) and the runtime resolvesidOrNameagainst ID, name, and external_ref for both get and remove paths.- The cold-path concern in this comment doesn’t hold:
runtime.Get(ctx, boxId)/runtime.ForceRemove(ctx, boxId)can resolveboxDto.Idviaexternal_ref.- Consistency gap:
apps/runner/pkg/boxlite/stubs.goresize recreation usesboxlite.WithName(boxId); if the goal is to keep the name slot free and useexternal_refconsistently, switch resize toWithExternalRef(boxId)(or update the PR objective/docs to reflect the external_ref-based identity).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/runner/pkg/boxlite/client.go` around lines 223 - 227, The code uses boxlite.WithExternalRef(boxDto.Id) which is correct because runtime.Get/ForceRemove resolve idOrName against id, name, and external_ref; ensure consistency by changing the resize recreation in apps/runner/pkg/boxlite/stubs.go from boxlite.WithName(boxId) to boxlite.WithExternalRef(boxId) (or alternatively update PR/docs to state you intentionally use names), and verify any other usages referencing box identity (boxDto.Id, boxId) follow the external_ref pattern rather than reverting to WithName.apps/api/src/box/dto/box.dto.spec.ts (1)
10-20: ⚡ Quick winCover the new required
imagemapping in this spec.This test now locks down the id migration, but
BoxDto.fromBox()could still drop or rename the newimagefield without failing anything here. Settingbox.imageand assertingdto.imagewould pin the new response contract too.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/api/src/box/dto/box.dto.spec.ts` around lines 10 - 20, The spec for BoxDto.identity doesn't assert the new image mapping; update the test so it sets the Box instance's image property (on the Box created in the test) and then assert that BoxDto.fromBox(...) preserves that value (i.e., expect(dto.image).toBe(box.image)). Locate the test that creates the Box and calls BoxDto.fromBox and add the box.image assignment and corresponding expectation to pin the new response contract.apps/api/src/box/entities/box.entity.ts (1)
92-95: ⚡ Quick winValidate
imageas an entity invariant, not only at insert time.
imageis now a required persisted field and is also sent straight through to the runner, but the entity still doesn't reject a missing/blank value inassertValid().BoxRepository.insert()already calls that method before writing, so this is the right boundary to fail fast with a domain error instead of a generic DB constraint failure.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/api/src/box/entities/box.entity.ts` around lines 92 - 95, The entity currently allows a missing/blank image to slip through to the DB; add a domain-level invariant in BoxEntity.assertValid() to validate that the image property is a non-empty, non-whitespace string and throw the same domain error type used elsewhere (e.g., a DomainError/ValidationError) when it fails; update any unit tests or call sites relying on BoxRepository.insert() to expect assertValid() to reject blank/missing image values. Ensure you reference the image field and assertValid() in your change so the repository-level insert continues to call the validator before persisting.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/api-client-go/api/openapi.yaml`:
- Around line 9999-10002: The CreateBox request schema is missing the image
property that Box exposes; update the OpenAPI spec so the CreateBox schema (used
by POST /box) includes the image field (matching Box.image) with type: string,
the same description "The OCI image ref the box boots from" and an example
(e.g., ghcr.io/boxlite-ai/boxlite-agent-python@sha256:80d562...); ensure POST
/box references the updated CreateBox schema so generated clients can send the
required OCI ref.
In `@apps/api-client-go/model_admin_box_item.go`:
- Line 27: The generated artifacts mislabel OrganizationId as a public box id;
update the source OpenAPI/DTO description for the AdminBoxItem model so the
OrganizationId field (OrganizationId in model_admin_box_item.go, organizationId
in admin-box-item.ts and AdminBoxItem.md) is described as the organization
identifier (not a second/public box id). Locate the AdminBoxItem schema/DTO in
the OpenAPI spec or the AdminBoxItem DTO source and change the field description
from mentioning "public box id" to "organization id" (or equivalent), then
re-run the generator to propagate the corrected description to
model_admin_box_item.go, admin-box-item.ts, and AdminBoxItem.md.
In `@apps/api-client-go/model_workspace.go`:
- Around line 34-35: The generated clients now require Workspace.image to be a
real OCI ref but the deprecated server DTO returns image: '' causing a contract
mismatch; fix by updating WorkspaceDto.fromBoxDto in
apps/api/src/box/dto/workspace.deprecated.dto.ts to populate the image field
with the actual OCI ref (e.g., derive or map from the box DTO/metadata) so it
returns a meaningful string, or alternatively make the published models
nullable/optional (change Image in apps/api-client-go/model_workspace.go to
*string / json:"image,omitempty" and the TS model in
apps/libs/api-client/src/models/workspace.ts to accept undefined/null) until the
deprecated response can supply a real value.
In `@apps/api/src/box/constants/curated-images.constant.ts`:
- Around line 39-40: The supportedImages function currently returns
process.env[envVar] || fallbackRef which allows mutable refs like ":latest";
change it to validate env overrides from SUPPORTED_IMAGE_SOURCES by checking the
env value (process.env[envVar]) is a pinned digest form (e.g., contains
'`@sha256`:' or matches your repo@sha256 regex) and only use it when valid,
otherwise fall back to fallbackRef; update supportedImages to perform this
validation for each ({ envVar, fallbackRef }) and ignore or reject non-digest
overrides so the allowlist remains pinned and deterministic.
In `@apps/api/src/box/entities/box.entity.spec.ts`:
- Around line 11-16: The test for Box should explicitly assert that the legacy
internal UUID field is gone: in the 'mints a single 12-character base62 id (no
separate internal UUID)' test, after constructing const box = new Box('us',
'data-loader'), add an assertion that the legacy property (box.boxId) is absent
(e.g., undefined or not present on the object) to ensure Box did not reintroduce
boxId; keep the existing assertions for box.id and box.name intact and place the
absence check alongside them.
In `@apps/api/src/migrations/1741087887225-migration.ts`:
- Line 73: You mutated an already-applied migration (Migration1741087887225) by
changing the CREATE TABLE "box" DDL; instead of editing that migration, add a
new forward migration that performs the breaking changes (e.g., ALTER TABLE
"box" ALTER COLUMN "id" TYPE character varying(12) USING ...; ALTER TABLE "box"
ALTER COLUMN "image" SET NOT NULL; and any other enum/default adjustments) or,
if incompatible, create a safe migration that creates a new table,
copies/transforms data, drops the old table and renames the new one; reference
the "box" table and the "id" and "image" columns and keep Migration1741087887225
unchanged.
In `@sdks/c/src/options.rs`:
- Around line 240-248: The setter options_set_external_ref currently swallows
conversion errors from c_str_to_string and returns void; change it to return a
BoxliteErrorCode (mirror boxlite_options_add_port) and propagate failures:
validate handle/external_ref, call c_str_to_string(external_ref) and on Err
return an appropriate BoxliteErrorCode instead of silently returning; on success
set (*handle).options.external_ref = Some(s). Alternatively, if you prefer
deferred validation, add a field on OptionsHandle (e.g., validation_error:
Option<String>) and store the error string there so creation fails
deterministically later; reference options_set_external_ref, c_str_to_string,
OptionsHandle, options.external_ref and boxlite_options_add_port when making the
changes.
In `@src/boxlite/src/runtime/options.rs`:
- Around line 317-322: The core validation must reject blank or whitespace-only
external_ref values: update BoxOptions::sanitize() to check the external_ref
Option<String> (trim the string) and if Some(trimmed).is_empty() return a
validation error (fail-fast) instead of leaving Some("") or whitespace, so
callers and the uniqueness logic in rt_impl.rs see only valid, non-empty
external_ref values; ensure the error type/message matches existing validation
error patterns.
In `@src/boxlite/src/runtime/rt_impl.rs`:
- Around line 369-377: The preflight external_ref check in create_inner is racy
and must be enforced at persistence; update BoxManager::add_box to reject a
duplicate config.options.external_ref (not just name/id) by querying for an
existing external_ref and returning BoxliteError::InvalidArgument when found,
and also add a DB-level UNIQUE constraint on the boxes.external_ref column and
map the DB unique-violation error to the same BoxliteError so concurrent creates
collapse cleanly; ensure provision_box() and any code paths that call add_box
rely on this persistent check (you can keep the preflight check in create_inner
as a fast-fail but it must not be the only enforcement).
In `@src/boxlite/src/runtime/types.rs`:
- Around line 292-294: The PartialEq implementation for BoxInfo currently omits
comparing external_ref, causing two BoxInfo instances with different
external_ref values to appear equal; update the equality logic in the BoxInfo
PartialEq (or switch to deriving PartialEq/Eq for struct BoxInfo) so that
external_ref is included in the comparison alongside the existing fields (e.g.,
compare self.external_ref == other.external_ref), ensuring BoxInfo equality
accounts for the new external_ref field.
---
Outside diff comments:
In `@apps/api-client-go/api/openapi.yaml`:
- Around line 9938-9979: The Box schema's id property is still documented as an
internal UUID—update the Box schema (properties.id / example) to reflect the new
12-character public box id (replace UUID example with a 12-char example and
change the description to indicate "12-character public box id"); make the same
change for Workspace.id and any other id occurrences in this file (e.g., the
other range mentioned) so generated docs/SDKs use the new public id format
instead of the deprecated internal UUID.
- Around line 13450-13472: The OpenAPI schema's organizationId field is
inconsistent: the property name is organizationId but its description and
example look like a public box ID; update the schema so the semantics
match—either (A) change the description to reflect an organization identifier
(e.g., "Organization ID") and use an organization-style example (e.g.,
org_abc123) for the organizationId property and in the example object, or (B) if
the field actually represents a public box ID, rename the property back to boxId
everywhere (including the example object and properties section) and keep the
box-style example (box_abc123); apply the same reconciliation for the repeated
block at lines referenced (the second occurrence).
In `@apps/api-client-go/model_box.go`:
- Around line 22-25: The Box.Id field and its schema description are stale—its
comment says "internal UUID" but the PR changes it to the public 12-character
box ID; update the OpenAPI/schema description for the Box model (affecting the
Box struct Id field) to describe it as the single 12-character public box ID,
then regenerate the Go client and docs so model_box.go (Box.Id) and Workspace.md
reflect the new description; ensure the change is made in the source OpenAPI
schema that drives generation so both artifacts are updated.
In `@apps/api/src/box/dto/box.dto.ts`:
- Around line 38-42: The ApiProperty for the id field currently describes and
exemplifies a UUID but the code now uses a 12-character public box identifier;
update the ApiProperty on the id property (the id field in box.dto.ts) to
describe it as "The public 12-character box identifier" and change the example
to a 12-char value (e.g. "abcd1234ef56" or similar) so Swagger and generated
examples reflect the new public ID format; keep the type as string and preserve
the ApiProperty decorator usage.
In `@apps/api/src/box/repositories/box.repository.ts`:
- Around line 202-229: In invalidateLookupCacheOnUpdate, when authToken changes
you only invalidate updatedBox.authToken; also call
boxLookupCacheInvalidationService.invalidate for the previous token
(previousBox.authToken) so the stale cache key is removed; keep both
invalidations (new and previous) and guard against empty/undefined tokens before
calling invalidate to avoid no-op or invalid requests (refer to
invalidateLookupCacheOnUpdate, updatedBox, previousBox, and
boxLookupCacheInvalidationService.invalidate).
In `@apps/api/src/box/services/box.service.ts`:
- Around line 463-468: The id prefix filter is using ILike (case-insensitive)
which breaks base62 box IDs; change the id filter to a case-sensitive operator
(e.g., use Like instead of ILike) so idFilter = Like(`${id}%`) while leaving
nameFilter/other options unchanged; update the usage in the same return block
that spreads baseFindOptions and nameFilter so the id branch uses the new
idFilter (symbols: ILike, Like, idFilter, baseFindOptions, nameFilter).
In `@apps/runner/pkg/boxlite/stubs.go`:
- Around line 95-108: The recovery currently hardcodes "alpine:latest" when
building createDto, losing the original box image; change createDto to use the
persisted image instead (e.g., use recoverDto.Image or the captured info.Image
that was saved before Destroy) so CreateBoxDTO.Id/Image/… preserves the original
image; locate the createDto construction in stubs.go and replace the hardcoded
"alpine:latest" with the recovered image field or fetch the box info image prior
to destroy and pass that into CreateBoxDTO.
---
Nitpick comments:
In `@apps/api/src/box/dto/box.dto.spec.ts`:
- Around line 10-20: The spec for BoxDto.identity doesn't assert the new image
mapping; update the test so it sets the Box instance's image property (on the
Box created in the test) and then assert that BoxDto.fromBox(...) preserves that
value (i.e., expect(dto.image).toBe(box.image)). Locate the test that creates
the Box and calls BoxDto.fromBox and add the box.image assignment and
corresponding expectation to pin the new response contract.
In `@apps/api/src/box/entities/box.entity.ts`:
- Around line 92-95: The entity currently allows a missing/blank image to slip
through to the DB; add a domain-level invariant in BoxEntity.assertValid() to
validate that the image property is a non-empty, non-whitespace string and throw
the same domain error type used elsewhere (e.g., a DomainError/ValidationError)
when it fails; update any unit tests or call sites relying on
BoxRepository.insert() to expect assertValid() to reject blank/missing image
values. Ensure you reference the image field and assertValid() in your change so
the repository-level insert continues to call the validator before persisting.
In `@apps/dashboard/src/components/Playground/Box/CodeSnippets/python.ts`:
- Around line 64-66: The hardcoded default base image digest string appears in
python.ts and four onboarding examples; extract it into a single exported
constant (e.g., DEFAULT_BASE_IMAGE or DEFAULT_AGENT_BASE_IMAGE) and import that
constant everywhere instead of repeating the literal; alternatively import the
canonical SUPPORTED_IMAGES[0] from CreateBoxSheet.tsx if that is the single
source of truth; update references in
apps/dashboard/src/components/Playground/Box/CodeSnippets/python.ts (the snippet
that uses p.config.createBoxFromImage) and in the four entries in
onboarding-code-examples.ts to use the shared constant/import.
In `@apps/dashboard/src/hooks/useBoxWsSync.ts`:
- Around line 49-55: In optimisticUpdate, when boxId is defined it is guaranteed
to equal box.id, so the second call to updateStateInDetailCache(box.id, state)
is redundant; remove that call and only call updateStateInDetailCache(boxId,
state) (keep updateStateInListCache(box.id, state) unchanged) so the detail
cache is updated exactly once; the relevant symbols are optimisticUpdate,
updateStateInListCache, updateStateInDetailCache, box.id and boxId.
In `@apps/runner/pkg/boxlite/client.go`:
- Around line 223-227: The code uses boxlite.WithExternalRef(boxDto.Id) which is
correct because runtime.Get/ForceRemove resolve idOrName against id, name, and
external_ref; ensure consistency by changing the resize recreation in
apps/runner/pkg/boxlite/stubs.go from boxlite.WithName(boxId) to
boxlite.WithExternalRef(boxId) (or alternatively update PR/docs to state you
intentionally use names), and verify any other usages referencing box identity
(boxDto.Id, boxId) follow the external_ref pattern rather than reverting to
WithName.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 0891e13a-88f1-4470-b80a-21c37e0eb930
📒 Files selected for processing (79)
apps/api-client-go/api/openapi.yamlapps/api-client-go/model_admin_box_item.goapps/api-client-go/model_box.goapps/api-client-go/model_workspace.goapps/api/src/admin/dto/admin-overview.dto.tsapps/api/src/admin/services/observability.service.spec.tsapps/api/src/admin/services/observability.service.tsapps/api/src/admin/services/overview.service.tsapps/api/src/box/constants/curated-images.constant.spec.tsapps/api/src/box/constants/curated-images.constant.tsapps/api/src/box/dto/box.dto.spec.tsapps/api/src/box/dto/box.dto.tsapps/api/src/box/dto/create-box.dto.tsapps/api/src/box/dto/workspace.deprecated.dto.tsapps/api/src/box/entities/box.entity.spec.tsapps/api/src/box/entities/box.entity.tsapps/api/src/box/managers/box-actions/box-start.action.tsapps/api/src/box/repositories/box.repository.tsapps/api/src/box/runner-adapter/runnerAdapter.tsapps/api/src/box/runner-adapter/runnerAdapter.v0.tsapps/api/src/box/runner-adapter/runnerAdapter.v2.createBox.spec.tsapps/api/src/box/runner-adapter/runnerAdapter.v2.tsapps/api/src/box/services/box-lookup-cache-invalidation.service.tsapps/api/src/box/services/box.service.box-id.spec.tsapps/api/src/box/services/box.service.tsapps/api/src/box/services/box.service.warm-pool.spec.tsapps/api/src/box/utils/box-id.util.tsapps/api/src/box/utils/box-lookup-cache.util.tsapps/api/src/boxlite-rest/mappers/box-to-box.mapper.spec.tsapps/api/src/boxlite-rest/mappers/box-to-box.mapper.tsapps/api/src/migrations/1741087887225-migration.tsapps/dashboard/src/components/Box/CreateBoxSheet.tsxapps/dashboard/src/components/BoxTable/BoxTableActions.tsxapps/dashboard/src/components/Playground/Box/CodeSnippets/python.tsapps/dashboard/src/components/admin/AdminPeopleBoxesView.tsxapps/dashboard/src/components/admin/adminDiagnoseTarget.tsapps/dashboard/src/components/admin/adminHelpers.tsapps/dashboard/src/hooks/useBoxWsSync.tsapps/dashboard/src/lib/box-identity.tsapps/dashboard/src/lib/cloudBox.tsapps/dashboard/src/lib/onboarding-code-examples.tsapps/dashboard/src/providers/PlaygroundProvider.tsxapps/libs/api-client/src/docs/AdminBoxItem.mdapps/libs/api-client/src/docs/Box.mdapps/libs/api-client/src/docs/Workspace.mdapps/libs/api-client/src/models/admin-box-item.tsapps/libs/api-client/src/models/box.tsapps/libs/api-client/src/models/workspace.tsapps/runner/pkg/api/dto/box.goapps/runner/pkg/boxlite/client.goapps/runner/pkg/boxlite/stubs.goopenapi/box.openapi.yamlscripts/test/e2e/README.mdscripts/test/e2e/bootstrap.shscripts/test/e2e/cases/conftest.pyscripts/test/e2e/cases/test_c_entry.pyscripts/test/e2e/cases/test_cli_detach_recovery.pyscripts/test/e2e/cases/test_cli_entry.pyscripts/test/e2e/cases/test_error_code_mapping.pyscripts/test/e2e/cases/test_go_entry.pyscripts/test/e2e/cases/test_node_entry.pyscripts/test/e2e/cases/test_quota_enforcement.pyscripts/test/e2e/fixture_setup.pyscripts/test/e2e/sdks/c/e2e_basic.cscripts/test/e2e/sdks/go/e2e_basic.goscripts/test/e2e/sdks/node/e2e_basic.tssdks/c/include/boxlite.hsdks/c/src/event_queue.rssdks/c/src/info.rssdks/c/src/options.rssdks/go/info.gosdks/go/options.gosdks/node/src/options.rssrc/boxlite/src/litebox/manager.rssrc/boxlite/src/rest/types.rssrc/boxlite/src/runtime/options.rssrc/boxlite/src/runtime/rt_impl.rssrc/boxlite/src/runtime/types.rssrc/cli/src/commands/inspect.rs
💤 Files with no reviewable changes (6)
- apps/api/src/box/dto/workspace.deprecated.dto.ts
- apps/api/src/admin/services/overview.service.ts
- apps/dashboard/src/components/BoxTable/BoxTableActions.tsx
- apps/api/src/box/utils/box-lookup-cache.util.ts
- apps/api/src/box/utils/box-id.util.ts
- apps/api/src/admin/dto/admin-overview.dto.ts
✅ Files skipped from review due to trivial changes (5)
- sdks/go/info.go
- apps/dashboard/src/components/admin/AdminPeopleBoxesView.tsx
- openapi/box.openapi.yaml
- apps/libs/api-client/src/docs/Box.md
- apps/dashboard/src/lib/cloudBox.ts
🚧 Files skipped from review as they are similar to previous changes (18)
- scripts/test/e2e/bootstrap.sh
- scripts/test/e2e/sdks/c/e2e_basic.c
- apps/api/src/box/managers/box-actions/box-start.action.ts
- scripts/test/e2e/sdks/go/e2e_basic.go
- scripts/test/e2e/cases/test_c_entry.py
- apps/api/src/boxlite-rest/mappers/box-to-box.mapper.ts
- scripts/test/e2e/cases/test_cli_entry.py
- apps/api/src/box/services/box.service.warm-pool.spec.ts
- scripts/test/e2e/cases/test_node_entry.py
- apps/api/src/box/runner-adapter/runnerAdapter.ts
- scripts/test/e2e/cases/test_go_entry.py
- apps/api/src/boxlite-rest/mappers/box-to-box.mapper.spec.ts
- apps/api/src/box/dto/create-box.dto.ts
- apps/dashboard/src/providers/PlaygroundProvider.tsx
- scripts/test/e2e/cases/test_quota_enforcement.py
- apps/api/src/box/runner-adapter/runnerAdapter.v0.ts
- apps/dashboard/src/components/Box/CreateBoxSheet.tsx
- scripts/test/e2e/fixture_setup.py
Comment on lines
+9999
to
+10002
| image: | ||
| description: The OCI image ref the box boots from | ||
| example: ghcr.io/boxlite-ai/boxlite-agent-python@sha256:80d562a57f4bc12def4e54dbdb9e7d26d3268fe0767a2955ab5ad718041145d6 | ||
| type: string |
There was a problem hiding this comment.
Add image to the CreateBox request schema.
This change makes Box.image part of the published response contract, but POST /box still points to a CreateBox schema with no image field. Generated clients from this spec cannot send the OCI ref that the updated API now requires, so they will produce invalid create requests.
Suggested OpenAPI fix
CreateBox:
example:
+ image: ghcr.io/boxlite-ai/boxlite-agent-python@sha256:80d562a57f4bc12def4e54dbdb9e7d26d3268fe0767a2955ab5ad718041145d6
name: MyBox
user: boxlite
env:
NODE_ENV: production
@@
properties:
+ image:
+ description: The pinned OCI image ref to boot the box from
+ example: ghcr.io/boxlite-ai/boxlite-agent-python@sha256:80d562a57f4bc12def4e54dbdb9e7d26d3268fe0767a2955ab5ad718041145d6
+ type: string
name:
description: "The name of the box. If not provided, the box ID will be used as the name"
example: MyBox
type: string
@@
+ required:
+ - image
type: objectAlso applies to: 10105-10110
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/api-client-go/api/openapi.yaml` around lines 9999 - 10002, The CreateBox
request schema is missing the image property that Box exposes; update the
OpenAPI spec so the CreateBox schema (used by POST /box) includes the image
field (matching Box.image) with type: string, the same description "The OCI
image ref the box boots from" and an example (e.g.,
ghcr.io/boxlite-ai/boxlite-agent-python@sha256:80d562...); ensure POST /box
references the updated CreateBox schema so generated clients can send the
required OCI ref.
| BoxId *string `json:"boxId,omitempty"` | ||
| // Organization ID | ||
| OrganizationId string `json:"organizationId"` | ||
| OrganizationId *string `json:"organizationId,omitempty"` |
There was a problem hiding this comment.
organizationId is misdocumented across the generated AdminBoxItem artifacts.
apps/api-client-go/model_admin_box_item.go, apps/libs/api-client/src/models/admin-box-item.ts, and apps/libs/api-client/src/docs/AdminBoxItem.md all describe organizationId as the public box id, even though this PR’s identity model makes id the single canonical public box identifier. Please correct the source OpenAPI/DTO description so these generated artifacts continue to describe the organization id rather than a second box id.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/api-client-go/model_admin_box_item.go` at line 27, The generated
artifacts mislabel OrganizationId as a public box id; update the source
OpenAPI/DTO description for the AdminBoxItem model so the OrganizationId field
(OrganizationId in model_admin_box_item.go, organizationId in admin-box-item.ts
and AdminBoxItem.md) is described as the organization identifier (not a
second/public box id). Locate the AdminBoxItem schema/DTO in the OpenAPI spec or
the AdminBoxItem DTO source and change the field description from mentioning
"public box id" to "organization id" (or equivalent), then re-run the generator
to propagate the corrected description to model_admin_box_item.go,
admin-box-item.ts, and AdminBoxItem.md.
Comment on lines
+34
to
+35
| // The OCI image ref the box boots from | ||
| Image string `json:"image"` |
There was a problem hiding this comment.
Published Workspace.image contract no longer matches the deprecated workspace response.
apps/api-client-go/model_workspace.go and apps/libs/api-client/src/models/workspace.ts now both require image to be a real OCI ref, but the supplied backend implementation in apps/api/src/box/dto/workspace.deprecated.dto.ts still returns image: ''. That is a cross-layer contract break: generated clients now advertise a meaningful required value that the server does not actually provide. Please either return the real image from WorkspaceDto.fromBoxDto or keep image optional/nullable until the deprecated response can satisfy the published contract.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/api-client-go/model_workspace.go` around lines 34 - 35, The generated
clients now require Workspace.image to be a real OCI ref but the deprecated
server DTO returns image: '' causing a contract mismatch; fix by updating
WorkspaceDto.fromBoxDto in apps/api/src/box/dto/workspace.deprecated.dto.ts to
populate the image field with the actual OCI ref (e.g., derive or map from the
box DTO/metadata) so it returns a meaningful string, or alternatively make the
published models nullable/optional (change Image in
apps/api-client-go/model_workspace.go to *string / json:"image,omitempty" and
the TS model in apps/libs/api-client/src/models/workspace.ts to accept
undefined/null) until the deprecated response can supply a real value.
Comment on lines
+39
to
+40
| export function supportedImages(): string[] { | ||
| return SUPPORTED_IMAGE_SOURCES.map(({ envVar, fallbackRef }) => process.env[envVar] || fallbackRef) |
There was a problem hiding this comment.
Reject non-digest env overrides here.
process.env[envVar] || fallbackRef accepts any override string, so a mis-set BOXLITE_SYSTEM_*_IMAGE like :latest widens the allowlist to a mutable ref. That breaks the pinned-image contract this PR is introducing and makes recreated boxes nondeterministic.
Suggested fix
export function supportedImages(): string[] {
- return SUPPORTED_IMAGE_SOURCES.map(({ envVar, fallbackRef }) => process.env[envVar] || fallbackRef)
+ return SUPPORTED_IMAGE_SOURCES.map(({ envVar, fallbackRef }) => {
+ const ref = process.env[envVar] || fallbackRef
+ if (!ref.includes('`@sha256`:')) {
+ throw new Error(`${envVar} must be a digest-pinned OCI ref, got '${ref}'`)
+ }
+ return ref
+ })
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| export function supportedImages(): string[] { | |
| return SUPPORTED_IMAGE_SOURCES.map(({ envVar, fallbackRef }) => process.env[envVar] || fallbackRef) | |
| export function supportedImages(): string[] { | |
| return SUPPORTED_IMAGE_SOURCES.map(({ envVar, fallbackRef }) => { | |
| const ref = process.env[envVar] || fallbackRef | |
| if (!ref.includes('`@sha256`:')) { | |
| throw new Error(`${envVar} must be a digest-pinned OCI ref, got '${ref}'`) | |
| } | |
| return ref | |
| }) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/api/src/box/constants/curated-images.constant.ts` around lines 39 - 40,
The supportedImages function currently returns process.env[envVar] ||
fallbackRef which allows mutable refs like ":latest"; change it to validate env
overrides from SUPPORTED_IMAGE_SOURCES by checking the env value
(process.env[envVar]) is a pinned digest form (e.g., contains '`@sha256`:' or
matches your repo@sha256 regex) and only use it when valid, otherwise fall back
to fallbackRef; update supportedImages to perform this validation for each ({
envVar, fallbackRef }) and ignore or reject non-digest overrides so the
allowlist remains pinned and deterministic.
Comment on lines
+11
to
16
| it('mints a single 12-character base62 id (no separate internal UUID)', () => { | ||
| const box = new Box('us', 'data-loader') | ||
|
|
||
| expect(box.id).toBeDefined() | ||
| expect(box.boxId).toHaveLength(BOX_ID_LENGTH) | ||
| expect(box.boxId).toMatch(BOX_ID_REGEX) | ||
| expect(box.boxId).not.toBe(box.id) | ||
| expect(box.id).toHaveLength(BOX_ID_LENGTH) | ||
| expect(box.id).toMatch(BOX_ID_REGEX) | ||
| expect(box.name).toBe('data-loader') |
There was a problem hiding this comment.
Assert the legacy boxId field is actually gone.
This test name says the entity has “no separate internal UUID”, but it still passes if Box accidentally exposes a leftover boxId property again. Add an explicit absence check so the regression is covered.
Suggested assertion
expect(box.id).toHaveLength(BOX_ID_LENGTH)
expect(box.id).toMatch(BOX_ID_REGEX)
+ expect(box).not.toHaveProperty('boxId')
expect(box.name).toBe('data-loader')📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| it('mints a single 12-character base62 id (no separate internal UUID)', () => { | |
| const box = new Box('us', 'data-loader') | |
| expect(box.id).toBeDefined() | |
| expect(box.boxId).toHaveLength(BOX_ID_LENGTH) | |
| expect(box.boxId).toMatch(BOX_ID_REGEX) | |
| expect(box.boxId).not.toBe(box.id) | |
| expect(box.id).toHaveLength(BOX_ID_LENGTH) | |
| expect(box.id).toMatch(BOX_ID_REGEX) | |
| expect(box.name).toBe('data-loader') | |
| it('mints a single 12-character base62 id (no separate internal UUID)', () => { | |
| const box = new Box('us', 'data-loader') | |
| expect(box.id).toHaveLength(BOX_ID_LENGTH) | |
| expect(box.id).toMatch(BOX_ID_REGEX) | |
| expect(box).not.toHaveProperty('boxId') | |
| expect(box.name).toBe('data-loader') |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/api/src/box/entities/box.entity.spec.ts` around lines 11 - 16, The test
for Box should explicitly assert that the legacy internal UUID field is gone: in
the 'mints a single 12-character base62 id (no separate internal UUID)' test,
after constructing const box = new Box('us', 'data-loader'), add an assertion
that the legacy property (box.boxId) is absent (e.g., undefined or not present
on the object) to ensure Box did not reintroduce boxId; keep the existing
assertions for box.id and box.name intact and place the absence check alongside
them.
| ) | ||
| await queryRunner.query( | ||
| `CREATE TABLE "box" ("id" character varying NOT NULL DEFAULT uuid_generate_v4(), "boxId" character varying(12) NOT NULL, "organizationId" uuid NOT NULL, "name" character varying NOT NULL, "region" character varying NOT NULL, "runnerId" uuid, "prevRunnerId" uuid, "class" "public"."box_class_enum" NOT NULL DEFAULT 'small', "state" "public"."box_state_enum" NOT NULL DEFAULT 'unknown', "desiredState" "public"."box_desiredstate_enum" NOT NULL DEFAULT 'started', "osUser" character varying NOT NULL, "errorReason" character varying, "recoverable" boolean NOT NULL DEFAULT false, "env" jsonb NOT NULL DEFAULT '{}', "public" boolean NOT NULL DEFAULT false, "networkBlockAll" boolean NOT NULL DEFAULT false, "networkAllowList" character varying, "labels" jsonb, "cpu" integer NOT NULL DEFAULT '2', "gpu" integer NOT NULL DEFAULT '0', "mem" integer NOT NULL DEFAULT '4', "disk" integer NOT NULL DEFAULT '10', "volumes" jsonb NOT NULL DEFAULT '[]', "createdAt" TIMESTAMP WITH TIME ZONE NOT NULL DEFAULT now(), "updatedAt" TIMESTAMP WITH TIME ZONE NOT NULL DEFAULT now(), "autoStopInterval" integer NOT NULL DEFAULT '15', "autoDeleteInterval" integer NOT NULL DEFAULT '-1', "pending" boolean NOT NULL DEFAULT false, "authToken" character varying NOT NULL, "daemonVersion" character varying, CONSTRAINT "box_organizationId_name_unique" UNIQUE ("organizationId", "name"), CONSTRAINT "box_id_pk" PRIMARY KEY ("id"))`, | ||
| `CREATE TABLE "box" ("id" character varying(12) NOT NULL, "organizationId" uuid NOT NULL, "name" character varying NOT NULL, "region" character varying NOT NULL, "runnerId" uuid, "prevRunnerId" uuid, "class" "public"."box_class_enum" NOT NULL DEFAULT 'small', "state" "public"."box_state_enum" NOT NULL DEFAULT 'unknown', "desiredState" "public"."box_desiredstate_enum" NOT NULL DEFAULT 'started', "osUser" character varying NOT NULL, "image" character varying NOT NULL, "errorReason" character varying, "recoverable" boolean NOT NULL DEFAULT false, "env" jsonb NOT NULL DEFAULT '{}', "public" boolean NOT NULL DEFAULT false, "networkBlockAll" boolean NOT NULL DEFAULT false, "networkAllowList" character varying, "labels" jsonb, "cpu" integer NOT NULL DEFAULT '2', "gpu" integer NOT NULL DEFAULT '0', "mem" integer NOT NULL DEFAULT '4', "disk" integer NOT NULL DEFAULT '10', "volumes" jsonb NOT NULL DEFAULT '[]', "createdAt" TIMESTAMP WITH TIME ZONE NOT NULL DEFAULT now(), "updatedAt" TIMESTAMP WITH TIME ZONE NOT NULL DEFAULT now(), "autoStopInterval" integer NOT NULL DEFAULT '15', "autoDeleteInterval" integer NOT NULL DEFAULT '-1', "pending" boolean NOT NULL DEFAULT false, "authToken" character varying NOT NULL, "daemonVersion" character varying, CONSTRAINT "box_organizationId_name_unique" UNIQUE ("organizationId", "name"), CONSTRAINT "box_id_pk" PRIMARY KEY ("id"))`, |
There was a problem hiding this comment.
Don’t mutate an already-applied migration for the breaking box table schema change.
In apps/api/src/migrations/1741087887225-migration.ts (line 73), updating the CREATE TABLE "box" shape while keeping the same Migration1741087887225 name means databases that already recorded this migration won’t re-run the new DDL. That can leave existing DBs on the old box schema while apps/api/src/box/entities/box.entity.ts expects the new definition (including the 12-char id and required image), risking runtime/data integrity issues.
Use a new forward migration for the breaking change, or ensure the release plan explicitly rebuilds/recreates the schema before starting the new API.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/api/src/migrations/1741087887225-migration.ts` at line 73, You mutated
an already-applied migration (Migration1741087887225) by changing the CREATE
TABLE "box" DDL; instead of editing that migration, add a new forward migration
that performs the breaking changes (e.g., ALTER TABLE "box" ALTER COLUMN "id"
TYPE character varying(12) USING ...; ALTER TABLE "box" ALTER COLUMN "image" SET
NOT NULL; and any other enum/default adjustments) or, if incompatible, create a
safe migration that creates a new table, copies/transforms data, drops the old
table and renames the new one; reference the "box" table and the "id" and
"image" columns and keep Migration1741087887225 unchanged.
Comment on lines
+240
to
+248
| pub unsafe fn options_set_external_ref(handle: *mut OptionsHandle, external_ref: *const c_char) { | ||
| unsafe { | ||
| if handle.is_null() || external_ref.is_null() { | ||
| return; | ||
| } | ||
| if let Ok(s) = c_str_to_string(external_ref) { | ||
| (*handle).options.external_ref = Some(s); | ||
| } | ||
| } |
There was a problem hiding this comment.
Don’t silently drop malformed external_ref values.
This helper no-ops on a NULL pointer and also drops c_str_to_string(external_ref) errors, so the caller can believe the orchestrator reference was attached when options.external_ref actually stays None. That breaks the new lookup/correlation contract without surfacing any error.
Please give this setter a real failure path like boxlite_options_add_port — e.g. return BoxliteErrorCode and plumb the error through the header/bindings, or store a deferred validation error on the options handle and fail creation deterministically later.
As per coding guidelines, "Fail fast on missing config or invalid inputs in Rust code" and "Never swallow errors silently."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@sdks/c/src/options.rs` around lines 240 - 248, The setter
options_set_external_ref currently swallows conversion errors from
c_str_to_string and returns void; change it to return a BoxliteErrorCode (mirror
boxlite_options_add_port) and propagate failures: validate handle/external_ref,
call c_str_to_string(external_ref) and on Err return an appropriate
BoxliteErrorCode instead of silently returning; on success set
(*handle).options.external_ref = Some(s). Alternatively, if you prefer deferred
validation, add a field on OptionsHandle (e.g., validation_error:
Option<String>) and store the error string there so creation fails
deterministically later; reference options_set_external_ref, c_str_to_string,
OptionsHandle, options.external_ref and boxlite_options_add_port when making the
changes.
Source: Coding guidelines
Comment on lines
+317
to
+322
| /// Opaque reference attached by an external orchestrator (e.g. the cloud | ||
| /// control plane's box id), mirroring CRI's `PodSandboxMetadata.uid`. | ||
| /// The engine never interprets it; it is unique when present and boxes | ||
| /// can be looked up by it. Locally created boxes leave it unset. | ||
| #[serde(default, skip_serializing_if = "Option::is_none")] | ||
| pub external_ref: Option<String>, |
There was a problem hiding this comment.
Reject blank external_ref values in core validation.
external_ref is documented as a unique lookup key, but direct Rust callers can still pass Some("") or whitespace-only values because BoxOptions::sanitize() never validates it. That creates a "present" reference that cannot be used reliably for lookup and weakens the create-time uniqueness contract in src/boxlite/src/runtime/rt_impl.rs.
Suggested fix
pub fn sanitize(&self) -> BoxliteResult<()> {
+ if let Some(external_ref) = &self.external_ref
+ && external_ref.trim().is_empty()
+ {
+ return Err(boxlite_shared::errors::BoxliteError::Config(
+ "external_ref must be non-empty when provided".to_string(),
+ ));
+ }
+
// Validate auto_remove + detach combinationAs per coding guidelines, "Fail fast on missing config or invalid inputs in Rust code" and "Perform validation at boundaries in Rust code — untrusted inputs should be checked where they enter."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/boxlite/src/runtime/options.rs` around lines 317 - 322, The core
validation must reject blank or whitespace-only external_ref values: update
BoxOptions::sanitize() to check the external_ref Option<String> (trim the
string) and if Some(trimmed).is_empty() return a validation error (fail-fast)
instead of leaving Some("") or whitespace, so callers and the uniqueness logic
in rt_impl.rs see only valid, non-empty external_ref values; ensure the error
type/message matches existing validation error patterns.
Source: Coding guidelines
Comment on lines
+369
to
+377
| // external_ref is an orchestrator-issued correlation key; like name it | ||
| // must resolve to at most one box, so reject duplicates at create time. | ||
| if let Some(ref external_ref) = options.external_ref | ||
| && self.box_manager.lookup_box(external_ref)?.is_some() | ||
| { | ||
| return Err(BoxliteError::InvalidArgument(format!( | ||
| "box with external_ref '{}' already exists", | ||
| external_ref | ||
| ))); |
There was a problem hiding this comment.
Move external_ref uniqueness enforcement into persistence, not just this preflight.
This guard does not actually make external_ref unique. Two concurrent create_inner calls can both pass Line 371 and then both persist because BoxManager::add_box still only rejects duplicate name/id, and provision_box() persists through the same path without running this check at all. Once that happens, lookup_box() will return whichever matching row it sees first for that external ref.
The fix needs to live where boxes are persisted: reject duplicate config.options.external_ref in BoxManager::add_box, and ideally back it with a DB-level unique constraint so the race collapses cleanly.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/boxlite/src/runtime/rt_impl.rs` around lines 369 - 377, The preflight
external_ref check in create_inner is racy and must be enforced at persistence;
update BoxManager::add_box to reject a duplicate config.options.external_ref
(not just name/id) by querying for an existing external_ref and returning
BoxliteError::InvalidArgument when found, and also add a DB-level UNIQUE
constraint on the boxes.external_ref column and map the DB unique-violation
error to the same BoxliteError so concurrent creates collapse cleanly; ensure
provision_box() and any code paths that call add_box rely on this persistent
check (you can keep the preflight check in create_inner as a fast-fail but it
must not be the only enforcement).
Comment on lines
+292
to
+294
| /// Orchestrator-issued external reference (optional; omitted when unset). | ||
| #[serde(default, skip_serializing_if = "Option::is_none")] | ||
| pub external_ref: Option<String>, |
There was a problem hiding this comment.
Include external_ref in BoxInfo equality.
Line 292 makes external_ref part of the public box metadata, but PartialEq below still ignores it, so two BoxInfo values with different external refs compare equal. That will hide regressions in any assertions or dedupe keyed off BoxInfo equality.
Suggested fix
impl PartialEq for BoxInfo {
fn eq(&self, other: &Self) -> bool {
self.id == other.id
+ && self.external_ref == other.external_ref
&& self.status == other.status
&& self.created_at == other.created_at
&& self.pid == other.pid
&& self.image == other.image
&& self.cpus == other.cpus📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| /// Orchestrator-issued external reference (optional; omitted when unset). | |
| #[serde(default, skip_serializing_if = "Option::is_none")] | |
| pub external_ref: Option<String>, | |
| impl PartialEq for BoxInfo { | |
| fn eq(&self, other: &Self) -> bool { | |
| self.id == other.id | |
| && self.external_ref == other.external_ref | |
| && self.status == other.status | |
| && self.created_at == other.created_at | |
| && self.pid == other.pid | |
| && self.image == other.image | |
| && self.cpus == other.cpus | |
| && self.memory_mib == other.memory_mib | |
| && self.labels == other.labels | |
| && self.health_status == other.health_status | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/boxlite/src/runtime/types.rs` around lines 292 - 294, The PartialEq
implementation for BoxInfo currently omits comparing external_ref, causing two
BoxInfo instances with different external_ref values to appear equal; update the
equality logic in the BoxInfo PartialEq (or switch to deriving PartialEq/Eq for
struct BoxInfo) so that external_ref is included in the comparison alongside the
existing fields (e.g., compare self.external_ref == other.external_ref),
ensuring BoxInfo equality accounts for the new external_ref field.
G4614
added a commit
that referenced
this pull request
Jun 12, 2026
…shed baseline The Tokyo e2e-ci DB was set up before #735 added 'image' to the 1741087887225-migration.ts baseline, so its migrations table marks the baseline as applied but the box table is missing the image column. Every box request 500s with: column "image" of relation "box" does not exist Add a post-baseline migration that idempotently ALTERs in the column. Fresh stacks that already provisioned via the new baseline see the DO $$ ... IF NOT EXISTS branch as a no-op; previously-deployed stacks (Tokyo e2e-ci, anything pinned to a pre-#735 baseline) get the column added at next API container start (RUN_MIGRATIONS=true on the SST stack).
G4614
added a commit
that referenced
this pull request
Jun 12, 2026
After the image column landed, e2e surfaced the next divergence: every INSERT on box now fails with null value in column "boxId" of relation "box" violates not-null constraint The pre-#735 Box entity had a separate `boxId` field (12-char base62) that defaulted to generateBoxId(); #735's 0e6b875 collapsed it into `id`, so the new entity no longer supplies boxId on INSERT. Pre-squash Tokyo stacks still carry the column with its NOT NULL constraint and no default, so every persist throws. Drop the column when it exists. Fresh stacks (no boxId column at all) go through the IF EXISTS no-op branch.
G4614
added a commit
that referenced
this pull request
Jun 12, 2026
Pre-#735/#736 box schemas accumulated several columns the new entity no longer touches — boxId, autoArchiveInterval, backup{ErrorReason, RegistryId,Snapshot}, snapshot{,Name}, template{,Id}, artifactRef. Any of them being NOT NULL is enough to fail every box INSERT. Drop them all up-front so the next deploy converges in one shot instead of chasing one bad column per e2e cycle.
pull Bot
pushed a commit
to CrazyForks/boxlite
that referenced
this pull request
Jun 12, 2026
…o main) (boxlite-ai#743) ## Summary - Re-applies boxlite-ai#738 onto `main`. boxlite-ai#738 was merged with base `feat/box-curated-image-boot` by mistake, so its content never reached `main`. - Removes the publishable `apps/libs/sdk-typescript` project and its Nx/build references. - Replaces Dashboard's `@boxlite-ai/sdk` usage with the Dashboard-private `cloudBox` adapter backed by generated REST clients. - Removes TypeScript Cloud SDK playground snippets so the UI no longer teaches users to install `@boxlite-ai/sdk`. ## Differences from boxlite-ai#738 Cherry-pick of squash commit `2f5aac6b` with three conflict resolutions against current `main`: - `PlaygroundProvider.tsx`: kept the `cloudBox` import side; `Image.debianSlim('3.13')` → `{ image: 'base' }` (identical to the boxlite-ai#738 resolved tree). - `apps/dashboard/.storybook/main.ts`: removed a stale `@boxlite-ai/sdk` vite alias added on `main` after boxlite-ai#738 branched. ## Architecture Context: why delete + rewrite (not a rename) ``` BEFORE AFTER ┌────────────────┐ ┌─────────────┐ ┌──────────────────────┐ │ A. Dashboard │ │ External TS │ │ A. Dashboard │ External TS users └───────┬────────┘ │ users │ │ ├ UI components │ │ │ └──────┬──────┘ │ ├ hooks │ │ ┌───────▼──────────────────▼──────┐ │ └ lib/cloudBox.ts ★NEW│ │ │ B. @boxlite-ai/sdk │ │ (458-line private │ │ │ (apps/libs/sdk-typescript) │ DELETED │ adapter) │ │ │ 9.8k lines, Daytona-derived │ └──────────┬────────────┘ │ └───────────────┬───────────────┘ │ │ ┌───────────────▼───────────────┐ ┌───────────▼─────────────────────▼──┐ │ C. @boxlite-ai/api-client │ │ C. @boxlite-ai/api-client │ │ (generated from OpenAPI) │ │ (unchanged, still published) │ └───────────────┬───────────────┘ └───────────────┬────────────────────┘ ▼ ▼ REST API (apps/api) REST API (apps/api) ``` **What each layer is.** `api-client` is machine-generated from the API's OpenAPI spec — it only knows how to call endpoints. The deleted SDK (B) and the new `cloudBox.ts` are the *handwritten glue* above it: form-params → DTO mapping, poll-until-started, attaching `.process`/`.fs` helpers to box objects. Glue logic is not in the OpenAPI contract, so it cannot be generated. **Why `cloudBox.ts` is `added`, not `renamed`.** It is a from-scratch minimal reimplementation of the <5% of the SDK the Dashboard actually used: 458 lines vs the SDK's 9.8k, only ~9 identical lines with the old `Box.ts` (`git diff -M20%` detects no rename). Marking it a rename would wrongly suggest the full SDK class hierarchy survived. **Why delete the SDK at all.** 1. *Provenance*: it is Daytona-derived AGPL code in a repo that competes with Daytona — licensing and optics liability. 2. *Maintenance*: every API change (renames, DTO removals, client regen) had to sweep 9.8k lines to serve a single internal consumer using <5% of it. 3. *Strategy*: our public SDK story is the dual-mode native SDKs (Python/Node/Go/C, embedded engine + optional REST). A parallel cloud-only TS SDK was a competing second "official way". **What external users keep.** The cloud REST API is unchanged; the generated clients (`@boxlite-ai/api-client`, `api-client-go`) remain published. Only the convenience wrapper is gone. **Known follow-ups (out of scope here).** - `cloudBox.ts` still carries the Daytona AGPL header — needs a clean-room pass. - Dead tail to remove once Playground is officially retired: hidden Playground routes, `toolbox-api-client` TS package (zero runtime callers), and the computer-use/git sections of `cloudBox.ts`. ## Verification - `vite build --minify false` error set diffed against an `origin/main` baseline worktree: no new errors beyond pre-existing `main` failures (line-number shifts only). - Note: `dashboard:build` is currently broken on `main` itself (lightningcss minify + `@boxlite-ai/api-client` resolution), unrelated to this change. ## Relation to boxlite-ai#735 `feat/box-curated-image-boot` (boxlite-ai#735) will be rebuilt on top of this branch so its diff contains only the curated-image work. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Dashboard now uses the CloudBox-backed API client for box creation and session flows. * **Bug Fixes** * Refactored creation/loading flows to improve reliability when creating or loading boxes. * **Removed Features** * TypeScript code snippet generation removed — Python is now the sole generated language. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Contributor
Author
|
Superseded by #753 — same image + single-identity + drop-boxId work consolidated into one PR against main, with external_ref stripped. |
G4614
added a commit
that referenced
this pull request
Jun 13, 2026
…id check, expand purge to cpu=999 Five wins toward a green e2e suite on Tokyo: 1. test_quota_enforcement.py + test_error_code_mapping.py: every hardcoded 'alpine:3.23' literal in a POST body switched to conftest.DEFAULT_IMAGE. The alpine string was rejected by the API image allowlist (HTTP 400 'Unsupported image') before reaching the actual cpu/memory/disk quota check, so each xfail-strict case passed-by-accident and tripped XPASS(strict). Sending the curated ghcr digest lets the request reach the quota path the test is meant to pin — which still silently clamps, so the cases now fail as expected and xfail captures them. 2. test_path_verification.py: module-level skipif when BOXLITE_E2E_PROFILE != 'default'. Both cases are LOCAL meta-tests (one greps credentials.toml for url=:3000, the other reads the boxlite-runner systemd journal) and can never succeed on the Tokyo cloud profile. 3. test_lifecycle::test_create_generates_unique_ids: post-#735 box ids are 12-char alphanumeric (BOX_ID_REGEX), no longer the 5-segment uuid the assertion was checking. 4. ci/e2e-cloud-test.yml purge step extends to rows whose cpu equals the over-quota literal (999) the XPASS tests POSTed back when the API didn't enforce. Those rows are still in the box table and blow up every list_info round-trip because the Python SDK types BoxInfo.cpus as u8 ('invalid value: integer 999, expected u8'). The 999 literal is provably from test pollution — way past max_cpu_per_box=4 and the ADMIN_TOTAL_CPU_QUOTA=32 envelope. 5. .gitignore: add .claude/.pr-reviewed.json so the claude review-state file isn't accidentally staged into review commits.
G4614
added a commit
that referenced
this pull request
Jun 13, 2026
…rrow types list_info kept blowing up with RuntimeError: ... invalid value: integer 999, expected u8 or RuntimeError: ... invalid value: integer 8192000000, expected u32 on the Tokyo e2e stack because the Box table still carries pre-#735 rows that the API created back when the per-box quota wasn't enforced (cpus=999, memory_mib past 4 GiB). serde_json's narrow integer deserializer aborts the whole list response when ANY row's cpu or memory exceeds the field's range, so a single bad row poisoned every test_list_info* / test_runtime_initialization_*. Widen the on-the-wire deserialization (BoxResponse.cpus: u32, BoxResponse.memory_mib: u64) so the boundary accepts whatever the server returns, then saturate-cast at the SDK boundary (BoxInfo.cpus: u8, BoxInfo.memory_mib: u32 are the long-standing public types and bindings rely on them). Legitimate values round-trip unchanged; legacy garbage clamps to the narrow type's MAX instead of crashing the list call. Two-side verified against cargo test -p boxlite --features rest test_box_response_legacy_oversize_cpus_memory_does_not_break_list - impl reverted to the prior narrow types → test FAILS at from_str with the exact production error 'invalid value: integer 999, expected u8'. - impl restored → all 5 box_response tests pass.
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
3 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.
Summary
BOXLITE_BOX_IDenv, and in every API response. The internal uuid (Daytona leftover) is removed.CreateBoxDTOfield renamed toimage(full OCI ref, aligned with the embedded SDKs'imageparameter);BoxId,GpuQuota,Registry, and theauthTokenpayload field are dropped;userId/osUserare no longervalidate:"required".How a box is created (lifecycle)
Notes for reviewers:
imageenv vars (BOXLITE_SYSTEM_{BASE,PYTHON,NODE}_IMAGE) now define the allowlist itself; bootstrap.sh points the local stack at public refs and fixture_setup records the stack's base image in the e2e profile.Breaking / deployment
varchar(12),imagecolumn added,boxIdcolumn removed). Requires a DB rebuild — agreed as the deployment plan since nothing is launched.Verification
go build ./pkg/... && go vet && gofmt -lclean (final cgo link needs the prebuiltlibboxlite.a, absent in the dev worktree — environment artifact)tsc --noEmitcleanFollow-up (stacked next)
boxIdDTO field (BoxDto/ admin overview DTO) which currently mirrorsidfor API-contract compatibility, regenerate the four API clients, and switch the dashboard reads from.boxIdto.id. Kept out of this PR so it stays free of generated files.Summary by CodeRabbit
Release Notes
New Features
Improvements
Documentation