Use cloud Box ID as primary identity#800
Conversation
📝 WalkthroughWalkthroughThe PR collapses the dual Box identity system—a UUID primary key ( ChangesSingle Box identity migration
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 37937787db
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/api-client-go/api/openapi.yaml (1)
6704-6711:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate remaining Box ID examples to the new public ID shape.
Several generated examples still show UUIDs, 6-character IDs, or
box_-prefixed IDs for Box identity fields. Use the same 12-character public ID shape everywhere these fields representbox(id).Representative example updates
- boxId: "123456" + boxId: aB3cD4eF5gH6 ... - boxId: 123e4567-e89b-12d3-a456-426614174000 + boxId: aB3cD4eF5gH6 ... - id: box_abc123 + id: aB3cD4eF5gH6 ... - description: Box ID - example: box_abc123 + description: The public 12-character Box ID + example: aB3cD4eF5gH6Also applies to: 6726-6735, 6754-6771, 6804-6816, 8003-8022, 9343-9358
🤖 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 6704 - 6711, The Box ID examples throughout the OpenAPI specification are inconsistent, using 6-character IDs, UUIDs, or box_-prefixed formats instead of a uniform 12-character public ID shape. Update all boxId example values across the file to use the consistent 12-character public ID format (matching the shape used elsewhere in the specification for box identity fields). This includes updates needed at lines 6704-6711 (anchor location showing the boxId example), 6726-6735, 6754-6771, 6804-6816, 8003-8022, and 9343-9358, where all boxId example values should be replaced with the same standardized 12-character public ID format.
🧹 Nitpick comments (1)
apps/api-client-go/api/openapi.yaml (1)
6389-6392: ⚡ Quick winMake the 12-character Box ID contract machine-readable.
Box.idnow documents the public 12-character identity, but the schema still accepts any string. AddminLength/maxLengthand, if the alphabet is fixed, apatternso generated clients and docs preserve the new contract.Proposed schema tightening
id: description: The public 12-character Box ID example: aB3cD4eF5gH6 + minLength: 12 + maxLength: 12 type: 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-client-go/api/openapi.yaml` around lines 6389 - 6392, The Box.id field in the OpenAPI schema currently only documents that it is a 12-character Box ID but does not enforce this constraint in the schema itself. Add minLength and maxLength properties both set to 12 to enforce exactly 12-character IDs. Additionally, if the 12-character Box ID uses a fixed alphabet or character set, add a pattern property with a regular expression that matches the allowed character set to make the contract machine-readable for generated clients and documentation.
🤖 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/libs/api-client/src/docs/BoxApi.md`:
- Line 822: The sort parameter table row at line 822 in BoxApi.md has a
malformed type definition cell that creates extra columns, breaking the Markdown
table structure (MD056 error). Clean up the sort parameter row by removing the
duplicate/conflicting type definitions. The current row appears to have both a
shortened array type and a full Array type definition concatenated together.
Replace this with a single, properly formatted type definition for the sort
parameter that matches the table structure and includes all valid sort field
options (id, name, state, region, updatedAt, createdAt). Ensure the row has the
correct number of columns to align with the table header and other parameter
rows.
---
Outside diff comments:
In `@apps/api-client-go/api/openapi.yaml`:
- Around line 6704-6711: The Box ID examples throughout the OpenAPI
specification are inconsistent, using 6-character IDs, UUIDs, or box_-prefixed
formats instead of a uniform 12-character public ID shape. Update all boxId
example values across the file to use the consistent 12-character public ID
format (matching the shape used elsewhere in the specification for box identity
fields). This includes updates needed at lines 6704-6711 (anchor location
showing the boxId example), 6726-6735, 6754-6771, 6804-6816, 8003-8022, and
9343-9358, where all boxId example values should be replaced with the same
standardized 12-character public ID format.
---
Nitpick comments:
In `@apps/api-client-go/api/openapi.yaml`:
- Around line 6389-6392: The Box.id field in the OpenAPI schema currently only
documents that it is a 12-character Box ID but does not enforce this constraint
in the schema itself. Add minLength and maxLength properties both set to 12 to
enforce exactly 12-character IDs. Additionally, if the 12-character Box ID uses
a fixed alphabet or character set, add a pattern property with a regular
expression that matches the allowed character set to make the contract
machine-readable for generated clients and documentation.
🪄 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: 4c48193e-9d2c-4316-b13c-1eb5468ee56b
📒 Files selected for processing (40)
apps/api-client-go/api/openapi.yamlapps/api-client-go/api_box.goapps/api-client-go/model_admin_box_item.goapps/api-client-go/model_box.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/dto/box.dto.spec.tsapps/api/src/box/dto/box.dto.tsapps/api/src/box/dto/list-boxes-query.dto.tsapps/api/src/box/entities/box.entity.spec.tsapps/api/src/box/entities/box.entity.tsapps/api/src/box/repositories/box.repository.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/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/BoxTable/BoxTableActions.tsxapps/dashboard/src/components/BoxTable/columns.tsxapps/dashboard/src/components/BoxTable/types.tsapps/dashboard/src/components/BoxTable/useBoxTable.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.test.tsapps/dashboard/src/lib/box-identity.tsapps/libs/api-client/src/api/box-api.tsapps/libs/api-client/src/docs/AdminBoxItem.mdapps/libs/api-client/src/docs/Box.mdapps/libs/api-client/src/docs/BoxApi.mdapps/libs/api-client/src/models/admin-box-item.tsapps/libs/api-client/src/models/box.tsapps/runner/pkg/api/dto/box.goapps/runner/pkg/boxlite/client.goapps/runner/pkg/boxlite/daemon_env_test.go
💤 Files with no reviewable changes (8)
- apps/libs/api-client/src/docs/AdminBoxItem.md
- apps/api/src/admin/services/overview.service.ts
- apps/libs/api-client/src/models/admin-box-item.ts
- apps/dashboard/src/components/BoxTable/types.ts
- apps/api/src/admin/dto/admin-overview.dto.ts
- apps/api-client-go/model_admin_box_item.go
- apps/dashboard/src/components/BoxTable/BoxTableActions.tsx
- apps/runner/pkg/api/dto/box.go
…800 Tokyo Api was redeployed with #800 ("Use cloud Box ID as primary identity") which also switched the supportedImages allowlist from digest-pinned to tag-pinned: Supported: ghcr.io/boxlite-ai/boxlite-agent-base:20260605-p0-r3 ghcr.io/boxlite-ai/boxlite-agent-python:20260605-p0-r3 ghcr.io/boxlite-ai/boxlite-agent-node:20260605-p0-r3 The workflow was still injecting the previous digest pin (ghcr.io/.../boxlite-agent-base@sha256:834dcb...), which the new allowlist no longer accepts. Every create_box leaked HTTP 400 "Unsupported image" and downstream cascade-failed ~38 e2e cases that all start from POST /boxes. The two CLI smokes that don't create a box (test_cli_whoami + test_cli_ls) still PASSED, confirming the auth / list path is healthy and the failure was image-side only. Switch the env to the tag form so the workflow matches whatever allowlist is currently deployed. A future hardening (separate change) would have the test step read Tokyo Api's /api/v1/config / supportedImages at runtime and pick the first entry — that way an allowlist swap downstream wouldn't require touching this yml again.
…800 Tokyo Api was redeployed with #800 ("Use cloud Box ID as primary identity") which also switched the supportedImages allowlist from digest-pinned to tag-pinned: Supported: ghcr.io/boxlite-ai/boxlite-agent-base:20260605-p0-r3 ghcr.io/boxlite-ai/boxlite-agent-python:20260605-p0-r3 ghcr.io/boxlite-ai/boxlite-agent-node:20260605-p0-r3 The workflow was still injecting the previous digest pin (ghcr.io/.../boxlite-agent-base@sha256:834dcb...), which the new allowlist no longer accepts. Every create_box leaked HTTP 400 "Unsupported image" and downstream cascade-failed ~38 e2e cases that all start from POST /boxes. The two CLI smokes that don't create a box (test_cli_whoami + test_cli_ls) still PASSED, confirming the auth / list path is healthy and the failure was image-side only. Switch the env to the tag form so the workflow matches whatever allowlist is currently deployed. A future hardening (separate change) would have the test step read Tokyo Api's /api/v1/config / supportedImages at runtime and pick the first entry — that way an allowlist swap downstream wouldn't require touching this yml again.
…rkflow yml The previous 'put BOXLITE_E2E_IMAGE in e2e-cloud-test.yml' approach drifts every time Tokyo Api's curated-images-constant is updated: #758 → digest-pinned (834dcb...) → workflow updated to send digest #800 → tag-pinned (:20260605-p0-r3) → workflow had to be updated AGAIN (digest now rejected, 38 cases cascade-fail at POST /boxes) The fix: don't trust the workflow's env; query the deployed API at session start. conftest._discover_supported_image now: 1. honors BOXLITE_E2E_IMAGE if explicitly set (override path for reproducibility audits) 2. otherwise sends POST /boxes with a sentinel image to the active credential profile's API, catches the 400 body's 'Supported images: a, b, c' list, returns the first entry (matches the server's own assertSupportedImage(undefined) default) 3. falls back to 'alpine:3.23' on any error — tests still 4xx-fail loudly so the regression is visible The discovered value is also pinned to os.environ['BOXLITE_E2E_IMAGE'] so the C / Go / Node entry-driver subprocesses inherit it without each test re-implementing the probe. The workflow's BOXLITE_E2E_IMAGE injection is removed (replaced with a comment explaining the probe). Subsequent allowlist swaps will pick up automatically.
…es probe
Make every case in scripts/test/e2e/cases/ runnable against any
profile pointing at any deployed stack (local boxlite serve, Tokyo
e2e-ci, Singapore dev), removing the hard-codes that only worked
against a local boxlite serve.
(1) Profile name. Every per-file _profile() helper used to hardcode
"p1"; now reads BOXLITE_E2E_PROFILE env (default "p1" for
backwards compat). 8 case files updated (test_c_entry,
test_go_entry, test_node_entry, test_error_code_mapping,
test_errors, test_quota_enforcement, test_runner_concurrency,
test_shutdown).
(2) Box id format. UUID_RE only accepted the 36-char UUID form, but
the local runtime mints 12-char Base62 and a REST server may
return a ULID. Replace with BOX_ID_RE matching all three
(UUID / ULID / 12-char Base62). 4 case files updated
(test_c_entry, test_cli_entry, test_cli_detach_recovery,
test_go_entry, test_node_entry).
(3) Image allowlist. The previous design relied on the workflow yml
injecting BOXLITE_E2E_IMAGE at the value the deployed API
accepted — every supportedImages allowlist update (#758 →
digest-pinned; #800 → tag-pinned :20260605-p0-r3) required a
workflow update to keep create_box from cascading 38 4xx FAILs.
conftest now probes the deployed API at session start (POST
/boxes with a sentinel out-of-allowlist image; parse the 400
body's "Supported images: a, b, c" list; pick the first) and
pins the discovered value back to os.environ so the C / Go /
Node entry-driver subprocesses inherit it.
BOXLITE_E2E_IMAGE remains as an override for reproducibility
audits.
(4) Whoami assertion. test_cli_whoami_against_local_api hard-coded
"boxlite-admin" + "http://localhost:3000" — only ever passed
against a local boxlite serve. Now reads the active profile's
URL from credentials.toml and asserts it appears in whoami
output + "Not logged in" doesn't.
(5) Path-bypass guard gating. The C / Go / Node / CLI entry smokes
all ended with a runner_hits_for_box assert that requires
journalctl access to the host running boxlite-runner. On the
cloud profiles that runner lives on a remote EC2; the autouse
Python fixture already bypasses via BOXLITE_E2E_SKIP_PATH_VERIFY
but the subprocess smokes did not. conftest exposes
path_verify_skipped() (single truthy reading of the env) and
the 4 entry tests + detach_recovery gate their hits-check on
it. Box id / driver output assertions still run.
(6) Image / drain robustness in test_exec_timeout: drain(ex) is
moved behind a short asyncio.wait_for and reordered after
ex.wait() — the REST runner's stream pumps don't reliably
observe stream closure when the workload terminates via SIGKILL,
so the previous shape blocked indefinitely on cloud.
…es probe
Make every case in scripts/test/e2e/cases/ runnable against any
profile pointing at any deployed stack (local boxlite serve, Tokyo
e2e-ci, Singapore dev), removing the hard-codes that only worked
against a local boxlite serve.
(1) Profile name. Every per-file _profile() helper used to hardcode
"p1"; now reads BOXLITE_E2E_PROFILE env (default "p1" for
backwards compat). 8 case files updated (test_c_entry,
test_go_entry, test_node_entry, test_error_code_mapping,
test_errors, test_quota_enforcement, test_runner_concurrency,
test_shutdown).
(2) Box id format. UUID_RE only accepted the 36-char UUID form, but
the local runtime mints 12-char Base62 and a REST server may
return a ULID. Replace with BOX_ID_RE matching all three
(UUID / ULID / 12-char Base62). 4 case files updated
(test_c_entry, test_cli_entry, test_cli_detach_recovery,
test_go_entry, test_node_entry).
(3) Image allowlist. The previous design relied on the workflow yml
injecting BOXLITE_E2E_IMAGE at the value the deployed API
accepted — every supportedImages allowlist update (#758 →
digest-pinned; #800 → tag-pinned :20260605-p0-r3) required a
workflow update to keep create_box from cascading 38 4xx FAILs.
conftest now probes the deployed API at session start (POST
/boxes with a sentinel out-of-allowlist image; parse the 400
body's "Supported images: a, b, c" list; pick the first) and
pins the discovered value back to os.environ so the C / Go /
Node entry-driver subprocesses inherit it.
BOXLITE_E2E_IMAGE remains as an override for reproducibility
audits.
(4) Whoami assertion. test_cli_whoami_against_local_api hard-coded
"boxlite-admin" + "http://localhost:3000" — only ever passed
against a local boxlite serve. Now reads the active profile's
URL from credentials.toml and asserts it appears in whoami
output + "Not logged in" doesn't.
(5) Path-bypass guard gating. The C / Go / Node / CLI entry smokes
all ended with a runner_hits_for_box assert that requires
journalctl access to the host running boxlite-runner. On the
cloud profiles that runner lives on a remote EC2; the autouse
Python fixture already bypasses via BOXLITE_E2E_SKIP_PATH_VERIFY
but the subprocess smokes did not. conftest exposes
path_verify_skipped() (single truthy reading of the env) and
the 4 entry tests + detach_recovery gate their hits-check on
it. Box id / driver output assertions still run.
(6) Image / drain robustness in test_exec_timeout: drain(ex) is
moved behind a short asyncio.wait_for and reordered after
ex.wait() — the REST runner's stream pumps don't reliably
observe stream closure when the workload terminates via SIGKILL,
so the previous shape blocked indefinitely on cloud.
…es probe
Make every case in scripts/test/e2e/cases/ runnable against any
profile pointing at any deployed stack (local boxlite serve, Tokyo
e2e-ci, Singapore dev), removing the hard-codes that only worked
against a local boxlite serve.
(1) Profile name. Every per-file _profile() helper used to hardcode
"p1"; now reads BOXLITE_E2E_PROFILE env (default "p1" for
backwards compat). 8 case files updated (test_c_entry,
test_go_entry, test_node_entry, test_error_code_mapping,
test_errors, test_quota_enforcement, test_runner_concurrency,
test_shutdown).
(2) Box id format. UUID_RE only accepted the 36-char UUID form, but
the local runtime mints 12-char Base62 and a REST server may
return a ULID. Replace with BOX_ID_RE matching all three
(UUID / ULID / 12-char Base62). 4 case files updated
(test_c_entry, test_cli_entry, test_cli_detach_recovery,
test_go_entry, test_node_entry).
(3) Image allowlist. The previous design relied on the workflow yml
injecting BOXLITE_E2E_IMAGE at the value the deployed API
accepted — every supportedImages allowlist update (#758 →
digest-pinned; #800 → tag-pinned :20260605-p0-r3) required a
workflow update to keep create_box from cascading 38 4xx FAILs.
conftest now probes the deployed API at session start (POST
/boxes with a sentinel out-of-allowlist image; parse the 400
body's "Supported images: a, b, c" list; pick the first) and
pins the discovered value back to os.environ so the C / Go /
Node entry-driver subprocesses inherit it.
BOXLITE_E2E_IMAGE remains as an override for reproducibility
audits.
(4) Whoami assertion. test_cli_whoami_against_local_api hard-coded
"boxlite-admin" + "http://localhost:3000" — only ever passed
against a local boxlite serve. Now reads the active profile's
URL from credentials.toml and asserts it appears in whoami
output + "Not logged in" doesn't.
(5) Path-bypass guard gating. The C / Go / Node / CLI entry smokes
all ended with a runner_hits_for_box assert that requires
journalctl access to the host running boxlite-runner. On the
cloud profiles that runner lives on a remote EC2; the autouse
Python fixture already bypasses via BOXLITE_E2E_SKIP_PATH_VERIFY
but the subprocess smokes did not. conftest exposes
path_verify_skipped() (single truthy reading of the env) and
the 4 entry tests + detach_recovery gate their hits-check on
it. Box id / driver output assertions still run.
(6) Image / drain robustness in test_exec_timeout: drain(ex) is
moved behind a short asyncio.wait_for and reordered after
ex.wait() — the REST runner's stream pumps don't reliably
observe stream closure when the workload terminates via SIGKILL,
so the previous shape blocked indefinitely on cloud.
…es probe
Make every case in scripts/test/e2e/cases/ runnable against any
profile pointing at any deployed stack (local boxlite serve, Tokyo
e2e-ci, Singapore dev), removing the hard-codes that only worked
against a local boxlite serve.
(1) Profile name. Every per-file _profile() helper used to hardcode
"p1"; now reads BOXLITE_E2E_PROFILE env (default "p1" for
backwards compat). 8 case files updated (test_c_entry,
test_go_entry, test_node_entry, test_error_code_mapping,
test_errors, test_quota_enforcement, test_runner_concurrency,
test_shutdown).
(2) Box id format. UUID_RE only accepted the 36-char UUID form, but
the local runtime mints 12-char Base62 and a REST server may
return a ULID. Replace with BOX_ID_RE matching all three
(UUID / ULID / 12-char Base62). 4 case files updated
(test_c_entry, test_cli_entry, test_cli_detach_recovery,
test_go_entry, test_node_entry).
(3) Image allowlist. The previous design relied on the workflow yml
injecting BOXLITE_E2E_IMAGE at the value the deployed API
accepted — every supportedImages allowlist update (#758 →
digest-pinned; #800 → tag-pinned :20260605-p0-r3) required a
workflow update to keep create_box from cascading 38 4xx FAILs.
conftest now probes the deployed API at session start (POST
/boxes with a sentinel out-of-allowlist image; parse the 400
body's "Supported images: a, b, c" list; pick the first) and
pins the discovered value back to os.environ so the C / Go /
Node entry-driver subprocesses inherit it.
BOXLITE_E2E_IMAGE remains as an override for reproducibility
audits.
(4) Whoami assertion. test_cli_whoami_against_local_api hard-coded
"boxlite-admin" + "http://localhost:3000" — only ever passed
against a local boxlite serve. Now reads the active profile's
URL from credentials.toml and asserts it appears in whoami
output + "Not logged in" doesn't.
(5) Path-bypass guard gating. The C / Go / Node / CLI entry smokes
all ended with a runner_hits_for_box assert that requires
journalctl access to the host running boxlite-runner. On the
cloud profiles that runner lives on a remote EC2; the autouse
Python fixture already bypasses via BOXLITE_E2E_SKIP_PATH_VERIFY
but the subprocess smokes did not. conftest exposes
path_verify_skipped() (single truthy reading of the env) and
the 4 entry tests + detach_recovery gate their hits-check on
it. Box id / driver output assertions still run.
(6) Image / drain robustness in test_exec_timeout: drain(ex) is
moved behind a short asyncio.wait_for and reordered after
ex.wait() — the REST runner's stream pumps don't reliably
observe stream closure when the workload terminates via SIGKILL,
so the previous shape blocked indefinitely on cloud.
Summary
box.idand remove the legacyBox.boxIdentity/DTO/API-client alias.iddirectly.Box/ admin box item models and list sorting no longer exposeboxId.Notes
box.idasvarchar(12)and nobox.boxIdcolumn.box.idrows orbox.boxIdcolumns exist.box_last_activity.boxIdandssh_access.boxIdremain as relation column names; both foreign-key tobox(id)and are not the removedBox.boxIdfield.{boxId}are path variable names for the public box identity and remain valid.Validation
make lint:appspassed with existing warnings only.make test:appspassed (36API suites /130tests; app Go test targets passed or matched cache).VERSION=0.0.0-dev make build:appspassed.git diff --checkpassed locally.