[codex] Normalize monorepo Docker build paths#730
Conversation
📝 WalkthroughWalkthroughThis PR refactors the Nx workspace structure, upgrades the Docker build pipeline to multi-stage compilation, simplifies box state lifecycle by removing legacy build states, adds new box mutation hooks with improved cache invalidation, and updates event handling to treat only ERROR state as "destroyed." ChangesWorkspace Restructuring & Box State Lifecycle Refactor
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 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 |
## Split This PR is the frontend contract-fix layer. The Docker packaging work is stacked separately in #730. ```text main | v #729 dashboard/API contract fixes | v #730 monorepo Docker packaging ``` ## Problem The dashboard was still using several assumptions from the older API/SDK shape. Once the API/dashboard were built through the production path, those mismatches became hard build blockers. ```text Generated API / SDK surface | |-- CreateBox request expects resources.{cpu,memory,disk} |-- Box API exposes start/delete/recover/stop as distinct methods |-- BoxState no longer has BUILD_FAILED / BUILDING_ARTIFACT / PENDING_BUILD |-- Runner DTO no longer exposes currentArtifactCount v Dashboard stale call sites | |-- cpu/memory/disk sent at top level |-- copied mutation hooks exported the wrong hook names/actions |-- UI referenced removed BoxState enum values |-- Runner details rendered a removed field v Dashboard typecheck / production build failure ``` ## What Changed ```text CreateBoxSheet cpu, memory, disk -> resources.cpu, resources.memory, resources.disk Box mutation hooks useStartBoxMutation -> boxApi.startBox useDeleteBoxMutation -> boxApi.deleteBox useRecoverBoxMutation -> boxApi.recoverBox useStopBoxMutation -> boxApi.stopBox + detailRef invalidation Box state UI removed stale generated enum references added UNKNOWN_DEFAULT_OPEN_API handling Runner details removed currentArtifactCount display ``` ## Why This Is Separate This PR only makes the dashboard match the current generated API/SDK contract. It does not change Docker, Nx, SST, package layout, or runtime image packaging. That keeps the frontend compile fixes reviewable before #730 changes the build pipeline. ## Validation ```text yarn tsc --project dashboard/tsconfig.app.json --noEmit NX_DAEMON=false NX_SKIP_NX_CACHE=true \ VITE_BASE_API_URL=%BOXLITE_BASE_API_URL% \ yarn nx build dashboard --configuration=production --nxBail=true --output-style=stream ``` Result: dashboard typecheck and production build pass. Existing Vite/OpenTelemetry/chunk-size warnings still appear, but the build exits successfully.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/dashboard/src/hooks/mutations/useStartBoxMutation.ts (1)
11-36: ⚖️ Poor tradeoffConsider extracting shared mutation hook logic.
The four box mutation hooks (start, delete, recover, stop) share near-identical structure: same variable interface pattern, same cache invalidation logic for
boxIdanddetailRef, differing only in the API method called. A factory function or higher-order hook could reduce duplication and improve maintainability.♻️ Potential abstraction approach
// Common hook factory const createBoxMutationHook = <TVariables extends { boxId: string; detailRef?: string }>( mutationFn: (api: BoxApi, variables: TVariables, orgId?: string) => Promise<void> ) => { return () => { const { boxApi } = useApi() const { selectedOrganization } = useSelectedOrganization() const queryClient = useQueryClient() return useMutation({ mutationFn: (variables: TVariables) => mutationFn(boxApi, variables, selectedOrganization?.id), onSuccess: (_, { boxId, detailRef }) => { queryClient.invalidateQueries({ queryKey: queryKeys.boxes.detail(selectedOrganization?.id ?? '', boxId), }) if (detailRef && detailRef !== boxId) { queryClient.invalidateQueries({ queryKey: queryKeys.boxes.detail(selectedOrganization?.id ?? '', detailRef), }) } }, }) } } // Usage export const useStartBoxMutation = createBoxMutationHook<StartBoxVariables>( (api, { boxId }, orgId) => api.startBox(boxId, orgId) )🤖 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/mutations/useStartBoxMutation.ts` around lines 11 - 36, The four hooks (useStartBoxMutation, useDeleteBoxMutation, useRecoverBoxMutation, useStopBoxMutation) duplicate the same useMutation wiring and cache invalidation; extract that shared logic into a factory (e.g., createBoxMutationHook) that accepts a mutation function (signature: (api: BoxApi, vars, orgId?) => Promise<void>) and returns a hook that wires boxApi, selectedOrganization, queryClient, mutationFn and the onSuccess invalidation for queryKeys.boxes.detail; then replace useStartBoxMutation to call the factory with the specific API call (e.g., (api, {boxId}, orgId) => api.startBox(boxId, orgId)) and do the same for the other box hooks to remove duplication while keeping existing behavior.
🤖 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.
Nitpick comments:
In `@apps/dashboard/src/hooks/mutations/useStartBoxMutation.ts`:
- Around line 11-36: The four hooks (useStartBoxMutation, useDeleteBoxMutation,
useRecoverBoxMutation, useStopBoxMutation) duplicate the same useMutation wiring
and cache invalidation; extract that shared logic into a factory (e.g.,
createBoxMutationHook) that accepts a mutation function (signature: (api:
BoxApi, vars, orgId?) => Promise<void>) and returns a hook that wires boxApi,
selectedOrganization, queryClient, mutationFn and the onSuccess invalidation for
queryKeys.boxes.detail; then replace useStartBoxMutation to call the factory
with the specific API call (e.g., (api, {boxId}, orgId) => api.startBox(boxId,
orgId)) and do the same for the other box hooks to remove duplication while
keeping existing behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 727ec501-03d3-4c10-bbf6-f5932efc61eb
⛔ Files ignored due to path filters (1)
apps/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (24)
.dockerignoreapps/api/Dockerfileapps/api/project.jsonapps/api/tsconfig.jsonapps/daemon/project.jsonapps/dashboard/project.jsonapps/dashboard/src/components/Box/CreateBoxSheet.tsxapps/dashboard/src/components/BoxTable/BoxState.tsxapps/dashboard/src/components/BoxTable/constants.tsapps/dashboard/src/components/BoxTable/state-icons.tsxapps/dashboard/src/components/RunnerDetailsSheet.tsxapps/dashboard/src/hooks/mutations/useDeleteBoxMutation.tsapps/dashboard/src/hooks/mutations/useRecoverBoxMutation.tsapps/dashboard/src/hooks/mutations/useStartBoxMutation.tsapps/dashboard/src/hooks/mutations/useStopBoxMutation.tsapps/dashboard/src/hooks/useBoxWsSync.tsapps/dashboard/src/lib/utils/box.tsapps/dashboard/src/pages/Boxes.tsxapps/infra/sst.config.tsapps/otel-collector/project.jsonapps/package.jsonapps/proxy/project.jsonapps/runner/project.jsonapps/ssh-gateway/project.json
💤 Files with no reviewable changes (2)
- apps/dashboard/src/components/RunnerDetailsSheet.tsx
- apps/infra/sst.config.ts
The pre-commit lint-fix hook runs `make lint:fix` repo-wide. Nine apps/api TypeScript files and four apps/runner Go files on the #730 baseline don't match the fixer's output (CI lints only changed files, so drift accumulated), so every commit reproduces this churn and aborts. Commit the fixer's own output once to clear the loop; verified idempotent.
…rt PRIMARY TD Two fixes from run #38's symptoms: 1. Dockerfile.source ts-node startup failed with: TS5083: Cannot read file '/boxlite/apps/tsconfig.base.json' Root cause: codex PR #730 (Normalize monorepo Docker build paths) moved the apps Nx workspace root from repo-root to apps/. After that, apps/api/tsconfig.json's `extends: ../tsconfig.base.json` resolves to apps/tsconfig.base.json (not the repo-root one). Dockerfile.source still placed it at /boxlite/ with WORKDIR /boxlite, so the extends never resolved. Fix: mirror the codex production Dockerfile layout — WORKDIR /boxlite/apps, place tsconfig.base.json there, COPY apps/api/ to api/ and apps/libs/ to libs/. ENTRYPOINT paths drop the apps/ prefix (api/tsconfig.app.json, api/src/main.ts). 2. Deploy-Api step false-positive: ECS DeploymentCircuitBreaker auto-rolled back the broken Api:20 task definition, leaving the service stable on the previous Api:16. `aws ecs wait services-stable` returned success, ALB targets stayed healthy (old tasks were never deregistered), and /api/health responded 2xx — all answering from the OLD image. The workflow believed the deploy succeeded; pytest then ran against the old API, masking the broken new image. Fix: after `wait services-stable`, query the PRIMARY deployment's taskDefinition and assert it equals NEW_TD_ARN. If different, ECS rolled back — fail the step and dump stop reasons from the most recent stopped tasks for diagnosis. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Merging the substantive bits of #745 and #746 here so PR #724's deploy-app-services CI can actually push an otel-collector image: * otel-collector/builder-config.yaml: fix module paths after #730 moved everything under apps/ (was: ../../../api-client-go etc., now: ../../../apps/api-client-go). The OCB build was failing with `filepath does not exist: /boxlite/otel-collector/exporter`. (#745) * otel-collector/Dockerfile: apk add python3 make g++ so node-gyp can compile any musl-incompatible native add-on during workspace `yarn install`. Kept even after dropping dockerode below — small safety net in case future deps reintroduce a native build. (#745) * apps/package.json: drop unused dockerode + @types/dockerode. They were inherited from the Daytona import in #460 but never imported in source. Drops the dockerode -> docker-modem -> ssh2 -> cpu-features chain; cpu-features was the optional native addon that forced the toolchain workaround in the first place. (#746) * apps/yarn.lock: regenerated (-161 lines). * apps/api-client-go/api/openapi.yaml: regenerate to catch up with main's existing `assignedRoleIds.default: []` drift so the api-client-drift PR check passes.
What Changed
This PR changes the API image from an ad-hoc monorepo Docker build into a normal repo-root-context, multi-stage production build.
Validation
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Refactor
Chores