Skip to content

fix(SKY-9781): port prebuilt-UI Dockerfile + entrypoint from cloud - #5998

Merged
celalzamanoglu merged 4 commits into
mainfrom
celal/sky-9781-port-prebuilt-ui-dockerfile-to-oss
Jun 4, 2026
Merged

celalzamanoglu merged 4 commits into
mainfrom
celal/sky-9781-port-prebuilt-ui-dockerfile-to-oss

Conversation

@celalzamanoglu

@celalzamanoglu celalzamanoglu commented May 14, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Multi-stage Dockerfile.ui (builder + runtime), node:20.12-slim → node:24.14-slim, npm ci, NODE_OPTIONS=--max-old-space-size=4096, 8 new VITE_* placeholders (Clerk, PostHog, environment, log artifacts, code block, API path prefix, 2FA notifications).
  • entrypoint-skyvernui.sh: set -euo pipefail, escape_sed() with multi-line guard, 13 sed rules, scoped post-replacement sanity-check grep, EXIT trap for orphan cleanup, wait -n, batched find -exec +.
  • New Dockerfile.ui.dockerignore so the UI build context only pulls skyvern-frontend/ and the entrypoint script (excludes host-side node_modules, dist, .env*).

OSS-specific deviations from skyvern-cloud

  • Keep ARG APP_VERSION: build-docker-image.yml passes APP_VERSION=\${{ github.sha }} as a build-arg; dropping it would break version baking.
  • Keep VITE_BROWSER_STREAMING_MODE placeholder: still read by the frontend (skyvern-frontend/src/util/env.ts:14); docker-compose injects cdp for self-hosters. Dropping the placeholder would silently force vnc.
  • Keep OSS credentials fallback: uses SKYVERN_CREDENTIALS_FILE / /app/.skyvern/credentials.toml (the volume-mounted path in docker-compose). .streamlit/secrets.toml is the legacy path the OSS doctor migrates away from (skyvern/cli/doctor.py:31). Both __SKYVERN_API_KEY_PLACEHOLDER__ and YOUR_API_KEY are filtered (aligned with frontend's PLACEHOLDER_VALUES).

Test plan

  • Local smoke test: docker build -f Dockerfile.ui --build-arg APP_VERSION=test . succeeds on multi-stage build.
  • Local runtime test: container starts, entrypoint applies sed replacements, sanity-check grep passes, both servers (localServer.js + artifactServer.js) come up.
  • CI: build-docker-image.yml builds the UI image successfully on next release (cloud prod unaffected — runs on Vercel).
  • Self-hosted: pull :latest after publish and verify Clerk/PostHog/environment VITE_* overrides are honored at runtime.

🤖 Generated with Claude Code

Brings the prebuilt UI image (`public.ecr.aws/skyvern/skyvern-ui:latest`)
in line with cloud's #9115: multi-stage build, `node:24.14-slim`,
`npm ci`, larger Node heap for Vite rollup, additional `VITE_*`
placeholders (Clerk, PostHog, environment, log artifacts, code block,
API path prefix, 2FA notifications), safer entrypoint (`set -euo
pipefail`, `escape_sed()` w/ multi-line guard, scoped sanity-check
grep, EXIT trap, `wait -n`, batched `find -exec +`), and a
per-Dockerfile `Dockerfile.ui.dockerignore` so the build context only
pulls `skyvern-frontend/` + the entrypoint.

OSS-specific preservations vs. cloud:
- Keep `ARG APP_VERSION` (build-docker-image.yml passes it).
- Keep `VITE_BROWSER_STREAMING_MODE` placeholder (still read by the
  frontend; docker-compose sets it to `cdp` for self-hosters).
- Keep `SKYVERN_CREDENTIALS_FILE` / `/app/.skyvern/credentials.toml`
  fallback (the OSS path; `.streamlit/secrets.toml` is the legacy path
  the doctor migrates away from). Both `__SKYVERN_API_KEY_PLACEHOLDER__`
  and `YOUR_API_KEY` now fall through to the credentials file.

Cloud production unaffected (frontend runs on Vercel).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@celalzamanoglu
celalzamanoglu requested review from marcmuon, suchintan and wintonzheng and removed request for suchintan May 18, 2026 15:13

@marcmuon marcmuon left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM. Clean port: multi-stage build, layer caching, signal handling via tini, and post-replace sanity grep against Vite-mangled placeholders. OSS/cloud boundary respected.

Drop 5 placeholders that no OSS code path reads or that aren't documented
self-hoster knobs:

- VITE_CLERK_PUBLISHABLE_KEY, VITE_PUBLIC_POSTHOG_HOST, VITE_PUBLIC_POSTHOG_KEY:
  referenced only in skyvern-frontend/cloud/ (ClerkProvider, cloud Providers),
  which is not synced to OSS and does not exist in this repo. The OSS app
  shell (src/App.tsx) hardcodes PostHog and has no Clerk. Baking placeholders
  for these replaced strings nothing reads.
- VITE_ENVIRONMENT, VITE_API_PATH_PREFIX: read by src/ but not in
  skyvern-frontend/.env.example, so not advertised self-hoster knobs. The
  pre-existing image never had them (baked at build), so dropping returns
  them to the proven status quo.

Placeholders now match skyvern-frontend/.env.example exactly (8 vars):
the original 5 plus VITE_ENABLE_LOG_ARTIFACTS / VITE_ENABLE_CODE_BLOCK /
VITE_ENABLE_2FA_NOTIFICATIONS. Keeping those three also fixes a latent bug
where the build-time-only image silently ignored those documented .env knobs.

Verified: image builds, container starts, sed replaces all placeholders,
served bundle has none left.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Comment thread Dockerfile.ui Outdated
ENV VITE_ENVIRONMENT=__VITE_ENVIRONMENT_PLACEHOLDER__
ENV VITE_API_PATH_PREFIX=__VITE_API_PATH_PREFIX_PLACEHOLDER__
ENV VITE_ENABLE_2FA_NOTIFICATIONS=__VITE_ENABLE_2FA_NOTIFICATIONS_PLACEHOLDER__
ENV VITE_CLERK_PUBLISHABLE_KEY=__VITE_CLERK_PUBLISHABLE_KEY_PLACEHOLDER__

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why do we need clerk and posthog keys in OSs?

@celalzamanoglu
celalzamanoglu merged commit a0fc148 into main Jun 4, 2026
11 checks passed
@celalzamanoglu
celalzamanoglu deleted the celal/sky-9781-port-prebuilt-ui-dockerfile-to-oss branch June 4, 2026 06:37
@celalzamanoglu celalzamanoglu mentioned this pull request Jun 4, 2026
2 tasks
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.

3 participants