Skip to content

[codex] Normalize monorepo Docker build paths#730

Merged
DorianZheng merged 2 commits into
mainfrom
codex/monorepo-docker-packaging
Jun 11, 2026
Merged

[codex] Normalize monorepo Docker build paths#730
DorianZheng merged 2 commits into
mainfrom
codex/monorepo-docker-packaging

Conversation

@law-chain-hot

@law-chain-hot law-chain-hot commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

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.

BEFORE

SST / Nx
  |
  |-- Docker context: repo root
  |-- Dockerfile: apps/api/Dockerfile
  v
apps/api/Dockerfile
  |
  |-- single stage: node:24-slim AS boxlite
  |-- workdir: /boxlite/apps
  |-- yarn install --immutable
  |-- COPY apps/api        -> api
  |-- COPY apps/dashboard  -> dashboard
  |-- COPY apps/libs       -> libs
  |
  |-- Nx api config still used paths like:
  |      apps/api/src/main.ts
  |      apps/api/tsconfig.app.json
  |      apps/api/webpack.config.js
  |
  |   but the Nx workspace root is already apps/
  |      apps/ + apps/api/... = apps/apps/api/...
  |
  |-- final image kept build tools, source folders, full install
  v
node dist/apps/api/main.js

Problems:
  - path roots were mixed: repo root vs apps workspace root
  - production build could resolve paths as apps/apps/...
  - SST used CACHE_BUST/cache:false to force rebuilds
  - final image was also the build stage
AFTER

SST / Nx
  |
  |-- Docker context: repo root
  |-- Dockerfile: apps/api/Dockerfile
  |-- api:docker explicitly declares:
  |      context: ..
  |      file: apps/api/Dockerfile
  v
apps/api/Dockerfile
  |
  +-- base
  |     node:24-slim + corepack + bash/curl
  |
  +-- deps
  |     COPY apps/package.json apps/yarn.lock apps/.yarnrc.yml
  |     yarn install --immutable
  |
  +-- build
  |     COPY apps/api       -> api
  |     COPY apps/dashboard -> dashboard
  |     COPY apps/libs      -> libs
  |     yarn nx build api --configuration=production
  |     yarn nx build dashboard --configuration=production
  |
  +-- runtime-deps
  |     yarn workspaces focus --all --production
  |
  v
final boxlite image
  |
  |-- node_modules/                 production deps
  |-- dist/apps/api/                built API
  |-- dist/apps/dashboard/          built dashboard
  |-- no api/ or dashboard/ source folders
  v
node dist/apps/api/main.js

Path rule now:
  Docker paths are repo-root-relative:      apps/api/Dockerfile
  Nx project paths are apps-root-relative:  api/src/main.ts

Validation

Local:
  git diff --check
  yarn tsc --project api/tsconfig.app.json --noEmit
  yarn nx show project api --json

boxlite-dev:
  docker build --target boxlite -t boxlite-api:monorepo-docker-packaging -f apps/api/Dockerfile .
  docker run smoke:
    node --check dist/apps/api/main.js
    test -f dist/apps/dashboard/index.html
    require.resolve("tslib")
    test ! -d api && test ! -d dashboard

Result:
  Docker build passed
  Runtime smoke passed
  Image size: 275694591 bytes

Summary by CodeRabbit

Release Notes

  • New Features

    • Added start, delete, and recover box operations with improved state handling
    • Added active boxes metric to runner health details
  • Bug Fixes

    • Corrected box state error handling to properly distinguish build failures from system errors
    • Improved resource override behavior in box creation
  • Refactor

    • Optimized Docker build process with multi-stage builds for improved efficiency
    • Reorganized project configuration paths across applications
  • Chores

    • Enhanced build caching and artifact exclusion configuration

@coderabbitai

coderabbitai Bot commented Jun 11, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

This 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."

Changes

Workspace Restructuring & Box State Lifecycle Refactor

Layer / File(s) Summary
Nx Project Source Root Paths
apps/api/project.json, apps/api/tsconfig.json, apps/daemon/project.json, apps/dashboard/project.json, apps/proxy/project.json, apps/runner/project.json, apps/ssh-gateway/project.json, apps/otel-collector/project.json
Nx sourceRoot configuration updated from apps/* layout to relative */ paths across all projects to reflect directory structure reorganization. TypeScript config extends path corrected accordingly.
Docker Build Pipeline & Infrastructure
apps/api/Dockerfile, apps/api/project.json, apps/infra/sst.config.ts, .dockerignore
Dockerfile refactored into multi-stage build with separate base, deps, runtime-deps, and build stages using Yarn workspace focusing for production dependencies. API Webpack configuration updated with new source paths and generatePackageJson disabled. Docker target explicitly configured with context and file paths. Cache-busting removed from infrastructure config. Build artifacts and logs added to Docker ignore patterns.
Box State Lifecycle: Removing Build States & Adding Unknown
apps/dashboard/src/components/BoxTable/constants.ts, apps/dashboard/src/components/BoxTable/state-icons.tsx
STATE_LABEL_MAPPING and STATE_ICONS remove entries for BUILD_FAILED, BUILDING_ARTIFACT, and PENDING_BUILD; UNKNOWN_DEFAULT_OPEN_API added with "Unknown" label. Filter options updated to exclude BUILD_FAILED.
Box State UI Rendering & Event Handling
apps/dashboard/src/components/BoxTable/BoxState.tsx, apps/dashboard/src/lib/utils/box.ts, apps/dashboard/src/hooks/useBoxWsSync.ts, apps/dashboard/src/pages/Boxes.tsx
BoxState component narrowed to treat only ERROR (not BUILD_FAILED) as error case for error-styled UI. isTransitioning utility removes BUILDING_ARTIFACT check. WebSocket sync handlers and Boxes page event handlers simplified to map only ERROR→DESTROYED transitions when desired state is DESTROYED.
Box Operation Mutations & Cache Invalidation
apps/dashboard/src/hooks/mutations/useStartBoxMutation.ts, apps/dashboard/src/hooks/mutations/useDeleteBoxMutation.ts, apps/dashboard/src/hooks/mutations/useRecoverBoxMutation.ts, apps/dashboard/src/hooks/mutations/useStopBoxMutation.ts, apps/dashboard/src/components/Box/CreateBoxSheet.tsx
New useStartBoxMutation hook added; useDeleteBoxMutation and useRecoverBoxMutation implemented; useStopBoxMutation extended with optional detailRef for dual cache invalidation. CreateBoxSheet updated to conditionally build and send resources object only when overrides are present.
Dashboard Metrics Update
apps/dashboard/src/components/RunnerDetailsSheet.tsx
Health Metrics grid card changed from displaying Images count to displaying Active Boxes count.
Build Dependencies & CI Detection
apps/package.json
Postinstall script updated to use inline Node environment check (process.env.CI and process.env.HUSKY) instead of is-ci package. is-ci dependency removed; tslib moved from devDependencies to dependencies.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Suggested reviewers

  • DorianZheng

Poem

A rabbit hops through Docker stages clean,
Where Nx paths dance and flows between.
No more BUILD_FAILED states to mourn,
Just ERROR truth when boxes transform. 🐰
Cache invalidates with grace profound—
The workspace restructures, refactor-bound! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title '[codex] Normalize monorepo Docker build paths' directly and concisely summarizes the main change: restructuring the monorepo Docker build configuration to use normalized, root-relative paths and multi-stage builds.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/monorepo-docker-packaging

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

github-merge-queue Bot pushed a commit that referenced this pull request Jun 11, 2026
## 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.
Base automatically changed from codex/dashboard-build-contract-fixes to main June 11, 2026 07:25
@DorianZheng DorianZheng marked this pull request as ready for review June 11, 2026 07:25
@DorianZheng DorianZheng requested a review from a team as a code owner June 11, 2026 07:25
@DorianZheng DorianZheng enabled auto-merge June 11, 2026 07:25

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
apps/dashboard/src/hooks/mutations/useStartBoxMutation.ts (1)

11-36: ⚖️ Poor tradeoff

Consider 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 boxId and detailRef, 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

📥 Commits

Reviewing files that changed from the base of the PR and between baebe20 and 7787cf3.

⛔ Files ignored due to path filters (1)
  • apps/yarn.lock is excluded by !**/yarn.lock, !**/*.lock
📒 Files selected for processing (24)
  • .dockerignore
  • apps/api/Dockerfile
  • apps/api/project.json
  • apps/api/tsconfig.json
  • apps/daemon/project.json
  • apps/dashboard/project.json
  • apps/dashboard/src/components/Box/CreateBoxSheet.tsx
  • apps/dashboard/src/components/BoxTable/BoxState.tsx
  • apps/dashboard/src/components/BoxTable/constants.ts
  • apps/dashboard/src/components/BoxTable/state-icons.tsx
  • apps/dashboard/src/components/RunnerDetailsSheet.tsx
  • apps/dashboard/src/hooks/mutations/useDeleteBoxMutation.ts
  • apps/dashboard/src/hooks/mutations/useRecoverBoxMutation.ts
  • apps/dashboard/src/hooks/mutations/useStartBoxMutation.ts
  • apps/dashboard/src/hooks/mutations/useStopBoxMutation.ts
  • apps/dashboard/src/hooks/useBoxWsSync.ts
  • apps/dashboard/src/lib/utils/box.ts
  • apps/dashboard/src/pages/Boxes.tsx
  • apps/infra/sst.config.ts
  • apps/otel-collector/project.json
  • apps/package.json
  • apps/proxy/project.json
  • apps/runner/project.json
  • apps/ssh-gateway/project.json
💤 Files with no reviewable changes (2)
  • apps/dashboard/src/components/RunnerDetailsSheet.tsx
  • apps/infra/sst.config.ts

@DorianZheng DorianZheng disabled auto-merge June 11, 2026 07:48
@DorianZheng DorianZheng merged commit 5ae4b50 into main Jun 11, 2026
3 checks passed
@DorianZheng DorianZheng deleted the codex/monorepo-docker-packaging branch June 11, 2026 07:48
law-chain-hot added a commit that referenced this pull request Jun 11, 2026
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.
G4614 added a commit that referenced this pull request Jun 11, 2026
…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>
G4614 added a commit that referenced this pull request Jun 12, 2026
G4614 added a commit that referenced this pull request Jun 12, 2026
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants