diff --git a/core-web/libs/sdk/create-app/README.md b/core-web/libs/sdk/create-app/README.md index 516aee76dd45..ddc4f3c0b4b0 100644 --- a/core-web/libs/sdk/create-app/README.md +++ b/core-web/libs/sdk/create-app/README.md @@ -8,10 +8,10 @@ Beta. Behavior and flags may change. ## Requirements -- Node.js + npm +- Node.js 22.22.3+ and npm - Git - Docker (for `--local` or `--starter`) -- Internet access (downloads templates and docker-compose) +- Internet access (downloads templates; pulls Docker images) ## Which SDK Version Should I Use? @@ -89,22 +89,27 @@ Flow: 2. Checks dotCMS health at `/api/v1/appconfiguration`. 3. Authenticates (up to 3 attempts). 4. Reads `defaultSite` from `/api/v1/site/defaultSite`. -5. Configures UVE via `/api/v1/apps/dotema-config-v2/{siteId}`. +5. Configures UVE via `/api/v1/apps/dotema-config-v2/{siteId}`. **Optional** — if this fails the + CLI warns, explains how to finish it by hand, and carries on. 6. Scaffolds selected frontend and runs `npm install`. -7. Prints framework-specific env setup instructions. +7. Writes `.env` with your host, site ID and token (see [Your `.env`](#your-env)). ### 2) Local mode (`--local`) Flow: 1. Validates Docker availability. -2. Validates required ports: `8082`, `8443`, `9200`, `9600`. -3. Downloads docker-compose from dotCMS main repo. -4. Runs `docker compose up -d`. -5. Waits for local health check. +2. Checks the ports this stack publishes: `8082`, `8443` and `8090`. A dotCMS already running on + `8082` is not treated as a conflict — see [If dotCMS is already running](#if-dotcms-is-already-running). +3. Writes the **bundled** `docker-compose.yml` into the project directory (see + [The bundled Docker stack](#the-bundled-docker-stack)). +4. Runs `docker compose up -d --wait`, which blocks until every service reports healthy, streaming + progress and elapsed time so a long first pull is never a silent spinner. +5. Waits for readiness on `/dotmgt/readyz`, falling back to `/api/v1/appconfiguration`. 6. Authenticates with default local credentials (`admin@dotcms.com` / `admin`). -7. Reads `defaultSite`, configures UVE, scaffolds frontend, runs `npm install`. -8. Prints framework-specific env setup instructions. +7. Reads `defaultSite`, configures UVE (optional — a failure warns and continues), scaffolds the + frontend, runs `npm install`. +8. Writes `.env` with your host, site ID and token (see [Your `.env`](#your-env)). ### 3) Starter-only local mode (`--starter `) @@ -113,7 +118,7 @@ Flow: Flow: 1. Same Docker and port checks as local mode. -2. Downloads docker-compose. +2. Writes the bundled `docker-compose.yml`. 3. Rewrites `CUSTOM_STARTER_URL` in `docker-compose.yml`. 4. Also passes `CUSTOM_STARTER_URL` in compose environment at runtime. 5. Starts containers and waits for health check. @@ -121,6 +126,77 @@ Flow: Use this when your starter is not compatible with the default frontend sample flow. +## The bundled Docker stack + +`--local` and `--starter` write a `docker-compose.yml` that **ships inside this package**. It is +no longer downloaded from the `dotCMS/core` repository at run time, so the stack you get is the one +this CLI version was tested against, rather than whatever is currently on `main`. + +The stack is `db` (PostgreSQL), `opensearch`, and `dotcms`. `dotcms` starts only after both +dependencies report **healthy**, and carries `restart: unless-stopped`, so it no longer races +Postgres and exit at startup. + +### Published ports + +| Port | Binding | Purpose | +| --- | --- | --- | +| `8082` | all interfaces | dotCMS HTTP | +| `8443` | all interfaces | dotCMS HTTPS | +| `8090` | **`127.0.0.1` only** | dotCMS management endpoints | + +PostgreSQL and OpenSearch publish **no** ports — they are reachable only from inside the compose +network, so running your own Postgres or OpenSearch on the usual ports does not conflict. + +> **Why 8090 is loopback-only.** It serves `/dotmgt/livez`, `/dotmgt/readyz`, `/dotmgt/health` and +> `/dotmgt/metrics`, and dotCMS authorizes those purely by the port a request arrives on — there is +> no credential check and no IP allow-list. Binding it to `0.0.0.0` would expose your instance's +> health and metrics to everyone on your network. It is bound to `127.0.0.1` deliberately; do not +> "fix" it to a wildcard. + +### Using a different compose file + +Set `DOTCMS_COMPOSE_URL` to fetch one from a URL instead of using the bundled file: + +```bash +DOTCMS_COMPOSE_URL=https://example.com/my-compose.yml npx @dotcms/create-app my-app --local +``` + +This is an escape hatch for hotfixes. The file must keep a single-line `CUSTOM_STARTER_URL:` entry +or `--starter` will fail against it. + +## If dotCMS is already running + +A dotCMS on `8082` from a previous run is not a conflict — the CLI probes it and offers a choice: + +``` +⚠ Found a dotCMS already running at http://localhost:8082 + Docker project "my-app" · Up 8 minutes (healthy) + +? How would you like to continue? +❯ Use this instance for my project + Replace it with a clean instance + Cancel +``` + +**Replace** stops that Docker project and removes its volumes (`docker compose -p down -v`) +before starting fresh. It is offered only when a Compose project owns the port — something started +outside Compose is not the CLI's to destroy. + +Reuse requires the instance to pass a readiness check **and** issue a token; anything else on `8082` +is still a hard failure. + +In a non-interactive run (CI, or no TTY) the CLI auto-reuses and prints a notice. It never replaces +without being asked. + +## Your `.env` + +The CLI writes `.env` into your project with the values the scaffolded app reads — `NEXT_PUBLIC_*` +for Next.js, `PUBLIC_*` for Astro. You do not need to copy anything by hand. + +An existing `.env` is never overwritten: the CLI prints the values instead so you can merge them. +Angular has no `.env` — it reads a TypeScript `environment` object, so the values are printed for +you to paste into the environment files. + ## Examples Interactive: @@ -172,9 +248,10 @@ Docker not available: Ports already in use: -- macOS/Linux: `lsof -i :8082` -- Windows: `netstat -ano | findstr ":8082"` -- Stop conflicting services or run `docker compose down`. +- If it is a dotCMS from a previous run, the CLI offers to reuse or replace it — see + [If dotCMS is already running](#if-dotcms-is-already-running). +- Otherwise, find the owner: `lsof -i :8082` (macOS/Linux) or + `netstat -ano | findstr ":8082"` (Windows), then stop it or run `docker compose down`. `zip END header not found` during starter load: @@ -186,13 +263,19 @@ Ports already in use: Build: ```sh -yarn nx build sdk-create-app --skip-nx-cache +pnpm nx build sdk-create-app --skip-nx-cache ``` Lint: ```sh -yarn nx lint sdk-create-app +pnpm nx lint sdk-create-app +``` + +Verify the package ships correctly (asserts the compose asset reaches `dist/` and the npm tarball): + +```sh +pnpm nx verify-package sdk-create-app ``` Dist output: diff --git a/core-web/libs/sdk/create-app/assets/docker-compose.yml b/core-web/libs/sdk/create-app/assets/docker-compose.yml new file mode 100644 index 000000000000..fd949299ecc6 --- /dev/null +++ b/core-web/libs/sdk/create-app/assets/docker-compose.yml @@ -0,0 +1,86 @@ +# Bundled with @dotcms/create-app. Written to the project directory by the CLI. +# Implements contracts/compose-service-contract.md C1-C8. +services: + db: + image: pgvector/pgvector:pg18 + command: postgres -c 'max_connections=400' -c 'shared_buffers=128MB' + environment: + POSTGRES_USER: 'dotcmsdbuser' + POSTGRES_PASSWORD: 'password' + POSTGRES_DB: 'dotcms' + volumes: + - dbdata:/var/lib/postgresql + networks: [db_net] + healthcheck: + test: ['CMD-SHELL', 'pg_isready -U dotcmsdbuser -d dotcms -h localhost -p 5432'] + interval: 10s + timeout: 5s + retries: 5 + restart: unless-stopped + + opensearch: + image: opensearchproject/opensearch:1 + environment: + cluster.name: 'elastic-cluster' + discovery.type: 'single-node' + bootstrap.memory_lock: 'true' + OPENSEARCH_JAVA_OPTS: '-Xmx1G' + ulimits: + memlock: { soft: -1, hard: -1 } + nofile: { soft: 65536, hard: 65536 } + volumes: + - opensearch-data:/usr/share/opensearch/data + networks: [opensearch-net] + healthcheck: + test: + [ + 'CMD-SHELL', + 'curl -sk https://localhost:9200 -u admin:admin | grep -q cluster_name' + ] + interval: 10s + timeout: 5s + retries: 12 + restart: unless-stopped + + dotcms: + image: dotcms/dotcms:latest + environment: + CMS_JAVA_OPTS: '-Xmx1g ' + LANG: 'C.UTF-8' + TZ: 'UTC' + DB_BASE_URL: 'jdbc:postgresql://db/dotcms' + DB_USERNAME: 'dotcmsdbuser' + DB_PASSWORD: 'password' + DOT_ES_AUTH_BASIC_PASSWORD: 'admin' + DOT_ES_ENDPOINTS: 'https://opensearch:9200' + DOT_INITIAL_ADMIN_PASSWORD: 'admin' + DOT_DOTCMS_CLUSTER_ID: 'dotcms-production' + CUSTOM_STARTER_URL: 'https://repo.dotcms.com/artifactory/libs-release-local/com/dotcms/starter/20260630/starter-20260630.zip' + depends_on: + db: + condition: service_healthy + opensearch: + condition: service_healthy + volumes: + - cms-shared:/data/shared + networks: [db_net, opensearch-net] + healthcheck: + test: ['CMD', 'curl', '-f', 'http://127.0.0.1:8090/dotmgt/livez'] + interval: 30s + timeout: 10s + retries: 5 + start_period: 180s + restart: unless-stopped + ports: + - '8082:8082' + - '8443:8443' + - '127.0.0.1:8090:8090' + +networks: + db_net: + opensearch-net: + +volumes: + cms-shared: + dbdata: + opensearch-data: diff --git a/core-web/libs/sdk/create-app/jest.config.ts b/core-web/libs/sdk/create-app/jest.config.ts index 349d82767b9c..9848ddcdced0 100644 --- a/core-web/libs/sdk/create-app/jest.config.ts +++ b/core-web/libs/sdk/create-app/jest.config.ts @@ -5,6 +5,22 @@ export default { transform: { '^.+\\.[tj]s$': ['ts-jest', { tsconfig: '/tsconfig.spec.json' }] }, + /* + * This package is `"type": "module"` and every one of its runtime dependencies is + * ESM-only (inquirer, ora, chalk, execa, axios — plus ~50 transitive micro-packages + * such as string-width, restore-cursor, npm-run-path...). Jest runs the specs as + * CommonJS, so anything left untransformed inside node_modules blows up with + * "SyntaxError: Cannot use import statement outside a module" the moment a spec + * imports this CLI's own source. + * + * Other libs in this workspace solve the same problem with a named allow-list + * (see libs/edit-content, apps/dotcms-ui, libs/portlets/dot-agents: + * `node_modules/(?!...y-protocols|lib0|@tiptap...)`). Here the list would need to + * enumerate ~51 packages and would silently rot on every dependency bump, so the + * whole of node_modules is transformed instead. The dependency tree of this CLI is + * small: a cold, uncached run of the full suite costs ~3s. + */ + transformIgnorePatterns: [], moduleFileExtensions: ['ts', 'js', 'html'], coverageDirectory: '../../../coverage/libs/sdk/create-app' }; diff --git a/core-web/libs/sdk/create-app/package.json b/core-web/libs/sdk/create-app/package.json index b23c80578ea7..b0cc54af0c7b 100644 --- a/core-web/libs/sdk/create-app/package.json +++ b/core-web/libs/sdk/create-app/package.json @@ -8,10 +8,10 @@ }, "files": [ "*.js", - "README.md" + "README.md", + "assets/**" ], "dependencies": { - "axios": "^1.13.2", "chalk": "^5.6.2", "commander": "^14.0.2", "execa": "^9.6.0", diff --git a/core-web/libs/sdk/create-app/project.json b/core-web/libs/sdk/create-app/project.json index 06abfab11251..b8a8328ba4eb 100644 --- a/core-web/libs/sdk/create-app/project.json +++ b/core-web/libs/sdk/create-app/project.json @@ -29,7 +29,6 @@ "inquirer", "execa", "cfonts", - "axios", "fs-extra", "commander" ], @@ -45,6 +44,11 @@ "input": "libs/sdk/create-app", "glob": "package.json", "output": "." + }, + { + "input": "libs/sdk/create-app/assets", + "glob": "**/*", + "output": "assets" } ], "esbuildOptions": { @@ -71,6 +75,7 @@ } }, "test": { + "dependsOn": ["verify-package", "verify-compose-static"], "options": { "passWithNoTests": true }, @@ -80,6 +85,30 @@ "coverage": true } } + }, + "verify-package": { + "executor": "nx:run-commands", + "dependsOn": ["build"], + "options": { + "command": "bash libs/sdk/create-app/scripts/verify-package.sh", + "cwd": "." + }, + "cache": true, + "inputs": [ + "{projectRoot}/assets/**", + "{projectRoot}/package.json", + "{projectRoot}/project.json", + "{projectRoot}/scripts/verify-package.sh" + ] + }, + "verify-compose-static": { + "executor": "nx:run-commands", + "cache": true, + "inputs": ["{projectRoot}/assets/**", "{projectRoot}/scripts/verify-cold-start.sh"], + "options": { + "command": "bash libs/sdk/create-app/scripts/verify-cold-start.sh --static", + "cwd": "." + } } } } diff --git a/core-web/libs/sdk/create-app/scripts/verify-cold-start.sh b/core-web/libs/sdk/create-app/scripts/verify-cold-start.sh new file mode 100755 index 000000000000..52e8c976bdff --- /dev/null +++ b/core-web/libs/sdk/create-app/scripts/verify-cold-start.sh @@ -0,0 +1,358 @@ +#!/usr/bin/env bash +# +# verify-cold-start.sh — acceptance checks for the compose file that +# @dotcms/create-app ships (../assets/docker-compose.yml). +# +# This checks the CLI's OWN bundled stack, not the shared +# docker/docker-compose-examples/single-node-demo-site example, which this work +# deliberately leaves untouched (see cli-design-decisions.md D4). +# +# It is the executable form of the manual procedure in +# specs/37262-create-app-docker-uve/quickstart.md steps 1-3. It exists because the +# behavior it checks (dependency ordering, restart on exit, port binding) cannot be +# unit-tested: it needs a real Docker daemon, real image pulls, and a multi-minute +# starter import. See issue #37262. +# +# Covers: +# T005 cold start: db + opensearch healthy before dotcms; dotcms healthy unaided +# T006 recovery: dotcms exits on its own -> restarted by the policy, not left exited +# T007 exposure: 8090 reachable on loopback, REFUSED on the LAN address +# T008 regression: the CUSTOM_STARTER_URL line shape installed CLIs depend on +# +# Usage: +# ./verify-cold-start.sh # full run (destroys volumes, ~10 min cold) +# ./verify-cold-start.sh --static # config-only checks, no containers (fast) +# +# Exit 0 = all assertions passed. + +set -uo pipefail + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +COMPOSE_FILE="${COMPOSE_FILE:-$SCRIPT_DIR/../assets/docker-compose.yml}" +WAIT_TIMEOUT="${WAIT_TIMEOUT:-600}" +RESTART_GRACE="${RESTART_GRACE:-30}" +# dotCMS needs ~46s to boot, and T006 restarts it just before the exposure check. +HEALTH_GRACE="${HEALTH_GRACE:-240}" + +PASS=0 +FAIL=0 + +ok() { printf ' \033[32mPASS\033[0m %s\n' "$1"; PASS=$((PASS + 1)); } +bad() { printf ' \033[31mFAIL\033[0m %s\n' "$1"; FAIL=$((FAIL + 1)); } +info() { printf ' %s\n' "$1"; } +section() { printf '\n\033[1m%s\033[0m\n' "$1"; } + +compose() { docker compose -f "$COMPOSE_FILE" "$@"; } + +# Health state of a compose service's container, or "none" when it declares no +# healthcheck. Distinguishing "no healthcheck" from "unhealthy" matters: the bug +# being fixed is the absence of a healthcheck, not a failing one. +# NOTE: `compose ps -q` lists RUNNING containers only, so a killed container reads +# back as "missing" and the failure gets misdiagnosed as "container gone" when the +# real state is "exited and never restarted". Always use -a here. +health_of() { + local svc="$1" cid + cid="$(compose ps -aq "$svc" 2>/dev/null | head -1)" + [ -n "$cid" ] || { echo "missing"; return; } + docker inspect -f '{{if .State.Health}}{{.State.Health.Status}}{{else}}none{{end}}' "$cid" 2>/dev/null || echo "missing" +} + +state_of() { + local svc="$1" cid + cid="$(compose ps -aq "$svc" 2>/dev/null | head -1)" + [ -n "$cid" ] || { echo "missing"; return; } + docker inspect -f '{{.State.Status}}' "$cid" 2>/dev/null || echo "missing" +} + +# --------------------------------------------------------------------------- +# T008 + static half of T005 — assertions on the compose file itself. +# These run without Docker and are the cheapest guard on the highest-consequence +# regression in this change. +# --------------------------------------------------------------------------- +static_checks() { + section "Static checks (compose file)" + + # T008 — CUSTOM_STARTER_URL line shape. + # + # updateDockerComposeStarterUrl() in core-web/libs/sdk/create-app/src/index.ts + # rewrites this file with the regex below when --starter is passed, and THROWS + # if nothing matches. Every already-installed CLI carries that code, so breaking + # this line shape disables --starter for all of them with no release able to + # reach them. Grep the RAW file, not `docker compose config`: the CLI regexes + # the bytes on disk, not the normalized config. + if grep -qE '^[[:space:]]*["'"'"']?CUSTOM_STARTER_URL["'"'"']?[[:space:]]*:[[:space:]]*.+$' "$COMPOSE_FILE"; then + ok "T008 CUSTOM_STARTER_URL matches the rewrite regex installed CLIs use" + else + bad "T008 CUSTOM_STARTER_URL line shape broken — --starter would throw in every installed CLI" + fi + + # Ordering is declared, not just observed. A passing runtime check can happen by + # luck on a warm machine; the declaration is what makes it reproducible. + local cfg + cfg="$(compose config 2>/dev/null)" + if [ -z "$cfg" ]; then + bad "compose config failed to render — the file is invalid" + return + fi + + if printf '%s' "$cfg" | grep -A3 -E '^\s+db:' | grep -q 'condition: service_healthy' \ + || printf '%s' "$cfg" | python3 -c ' +import sys,re +cfg = sys.stdin.read() +m = re.search(r"^ dotcms:.*?(?=^ \S|\Z)", cfg, re.S | re.M) +sys.exit(0 if m and "service_healthy" in m.group(0) else 1) +'; then + ok "T005 dotcms depends_on declares a service_healthy condition" + else + bad "T005 dotcms depends_on has no service_healthy condition — it can start against a cold Postgres" + fi + + for svc in db opensearch dotcms; do + if printf '%s' "$cfg" | python3 -c " +import sys,re +cfg = sys.stdin.read() +m = re.search(r'^ $svc:.*?(?=^ \S|\Z)', cfg, re.S | re.M) +sys.exit(0 if m and 'healthcheck:' in m.group(0) else 1) +"; then + ok "T005 $svc declares a healthcheck" + else + bad "T005 $svc has no healthcheck" + fi + + if printf '%s' "$cfg" | python3 -c " +import sys,re +cfg = sys.stdin.read() +m = re.search(r'^ $svc:.*?(?=^ \S|\Z)', cfg, re.S | re.M) +sys.exit(0 if m and 'restart:' in m.group(0) else 1) +"; then + ok "T005 $svc declares a restart policy" + else + bad "T005 $svc has no restart policy — it stays dead after an exit" + fi + done + + # T007 (static half) — 8090 must be published, and bound to loopback only. + # The management port is authorized purely by the port a request arrives on: + # no credential check, no IP allowlist (InfrastructureManagementFilter). A + # wildcard binding puts /dotmgt/health and /dotmgt/metrics on the local network. + if grep -qE '^[[:space:]]*-[[:space:]]*["'"'"']?127\.0\.0\.1:8090:8090' "$COMPOSE_FILE"; then + ok "T007 8090 published on loopback only" + elif grep -qE '^[[:space:]]*-[[:space:]]*["'"'"']?8090:8090' "$COMPOSE_FILE"; then + bad "T007 8090 published on 0.0.0.0 — unauthenticated /dotmgt/* exposed to the network" + else + bad "T007 8090 not published — the CLI cannot use /dotmgt/readyz" + fi +} + +# --------------------------------------------------------------------------- +# T005 — cold start with no manual intervention. +# --------------------------------------------------------------------------- +runtime_cold_start() { + section "Cold start (destroys volumes; this is the real test)" + info "docker compose down -v" + compose down -v >/dev/null 2>&1 + + # Report whether this is a genuine cold start. With images already cached the + # whole stack can come up in seconds, which is NOT the scenario users hit. + if docker image inspect dotcms/dotcms:latest >/dev/null 2>&1; then + info "NOTE: dotcms/dotcms:latest is already cached — this is a warm start." + info " The reported race (dotcms beating Postgres) is timing-dependent and" + info " may not reproduce here. Run 'docker rmi dotcms/dotcms:latest' first" + info " for a true cold start." + fi + + info "docker compose up -d --wait --wait-timeout $WAIT_TIMEOUT" + local started rc + started=$(date +%s) + compose up -d --wait --wait-timeout "$WAIT_TIMEOUT" + rc=$? + local elapsed=$(( $(date +%s) - started )) + info "took ${elapsed}s, exit=$rc" + + # `--wait` is only as good as the healthchecks. For a service with NO healthcheck + # it waits for "running", not "ready" — so on the unfixed file it exits 0 within + # seconds and reports every service "Healthy" while dotCMS is still booting. That + # false green is why --wait alone does not fix the CLI; it needs the healthchecks. + if [ $rc -eq 0 ] && [ "$(health_of dotcms)" = "none" ]; then + bad "T005 'up --wait' exited 0 in ${elapsed}s but dotcms has NO healthcheck — this is a false green, not readiness" + elif [ $rc -eq 0 ]; then + ok "T005 'up --wait' exited 0 — every service reached ready without help" + else + bad "T005 'up --wait' exited $rc — a service never became ready (this is the reported bug)" + fi + + for svc in db opensearch dotcms; do + local h s + h="$(health_of "$svc")"; s="$(state_of "$svc")" + case "$h" in + healthy) ok "T005 $svc is healthy" ;; + none) bad "T005 $svc has no healthcheck (state=$s) — readiness is unobservable" ;; + *) bad "T005 $svc health=$h state=$s" ;; + esac + done + + # The reported symptom, asserted directly: dotcms must not have died on the way up. + local cid restarts + cid="$(compose ps -q dotcms 2>/dev/null)" + if [ -n "$cid" ]; then + restarts="$(docker inspect -f '{{.RestartCount}}' "$cid" 2>/dev/null || echo '?')" + if [ "$restarts" = "0" ]; then + ok "T005 dotcms never had to restart — it did not race its dependencies" + else + info "dotcms RestartCount=$restarts (it recovered, but it still lost the race)" + ok "T005 dotcms is up (recovered via restart policy after $restarts restart(s))" + fi + fi +} + +# --------------------------------------------------------------------------- +# T006 — recovery. Compose restart policies react to container EXIT, not to health +# status. But they also do NOT react to an EXTERNALLY initiated stop: Docker treats +# `docker kill` as a user-requested stop and deliberately declines to restart, so +# asserting a restart after `docker kill` can never pass. +# +# measured 2026-08-31 (docker 29.4): +# self-exit -> RestartCount=4, status=running <- policy applied +# docker kill -> RestartCount=0, status=exited <- policy skipped +# +# So we make the container exit ON ITS OWN, which is also what actually happened in +# #37262 (dotcms died because Postgres was not accepting connections yet). +# PID 1 is tini and the JVM is a descendant of it, so the JVM can be signalled from +# inside the container; tini then reaps it and exits. Signalling PID 1 directly would +# not work — the kernel refuses SIGKILL to PID 1 from within its own PID namespace. +# --------------------------------------------------------------------------- +runtime_restart() { + section "Recovery after an unexpected exit" + local cid before + cid="$(compose ps -q dotcms 2>/dev/null)" + if [ -z "$cid" ]; then + bad "T006 no dotcms container to crash" + return + fi + before="$(docker inspect -f '{{.RestartCount}}' "$cid" 2>/dev/null || echo 0)" + + info "killing the JVM inside $cid (self-exit, not docker kill)" + if ! docker exec "$cid" bash -c 'pkill -9 -f "^/.*java" || pkill -9 java' >/dev/null 2>&1; then + info "pkill returned non-zero (process may already be gone); continuing" + fi + + local waited=0 s + while [ $waited -lt $RESTART_GRACE ]; do + sleep 3; waited=$((waited + 3)) + s="$(state_of dotcms)" + [ "$s" = "running" ] && [ "$(docker inspect -f '{{.RestartCount}}' "$cid" 2>/dev/null || echo 0)" -gt "$before" ] && break + done + + s="$(state_of dotcms)" + local after + after="$(docker inspect -f '{{.RestartCount}}' "$cid" 2>/dev/null || echo 0)" + if [ "$s" = "running" ] && [ "$after" -gt "$before" ]; then + ok "T006 dotcms exited and was restarted by the policy (${waited}s, RestartCount ${before}->${after})" + elif [ "$s" = "running" ]; then + bad "T006 dotcms is running but RestartCount did not move (${before}->${after}) — the JVM kill did not take, so the restart policy was never exercised" + else + bad "T006 dotcms state=$s after ${RESTART_GRACE}s — it stayed dead, exactly as reported" + fi +} + +# --------------------------------------------------------------------------- +# T007 — the management port answers on loopback and NOT on the LAN. +# --------------------------------------------------------------------------- +# Waits for dotcms to be healthy again. +# +# T006 deliberately crashes the container immediately before this, and dotCMS takes ~46s to boot. +# Without this wait, T007 probed a container that was 3 seconds into a restart and reported +# "the management port is not published" — accusing a correct compose file of a defect it does +# not have. Verified 2026-09-01: against a healthy stack the same probe returns 200 on loopback +# and is refused on the LAN address, exactly as AC-011 requires. +# +# The wait lives here rather than at the end of T006 so this check does not depend on what ran +# before it. +wait_until_healthy() { + local svc="$1" waited=0 + while [ $waited -lt "$HEALTH_GRACE" ]; do + [ "$(health_of "$svc")" = "healthy" ] && return 0 + sleep 5; waited=$((waited + 5)) + done + + return 1 +} + +runtime_exposure() { + section "Management port exposure" + + if ! wait_until_healthy dotcms; then + bad "T007 dotcms did not return to healthy within ${HEALTH_GRACE}s — cannot test exposure" + return + fi + + # Probe livez, not readyz. This is an EXPOSURE test — it asks whether the port is + # reachable on loopback, and livez is what the container healthcheck guarantees. + # readyz lags it: measured 2026-08-31, readyz returned 503 for a few seconds after + # `up --wait` already reported the container Healthy, which would flake this check. + local loopback_answers=1 + if curl -fsS --max-time 10 http://127.0.0.1:8090/dotmgt/livez >/dev/null 2>&1; then + ok "T007 /dotmgt/livez answers on 127.0.0.1:8090" + loopback_answers=0 + else + bad "T007 /dotmgt/livez unreachable on 127.0.0.1:8090 — the management port is not published" + fi + + local lan="${LAN_ADDR:-}" + if [ -z "$lan" ] && command -v ipconfig >/dev/null 2>&1; then + lan="$(ipconfig getifaddr en0 2>/dev/null || true)" + fi + [ -n "$lan" ] || lan="$(hostname -I 2>/dev/null | awk '{print $1}')" + + # A security assertion that cannot run must FAIL, not pass quietly. This check is the only + # thing standing behind AC-011 — that unauthenticated /dotmgt/health and /dotmgt/metrics are + # not on the network — and an earlier version of this function skipped it with `info` while + # the suite still printed "0 failed". A green summary that omits this check is worse than a + # red one, because nobody goes looking. + if [ -z "$lan" ]; then + bad "T007 no LAN address found — AC-011 is UNVERIFIED, not satisfied. Pass LAN_ADDR= to check it explicitly." + return + fi + + # Guard against a vacuous pass: if 8090 is not published at all, the LAN refusal + # below is trivially true and proves nothing about the binding. Only treat the + # refusal as meaningful once loopback actually answers. + # + # This reuses the probe above rather than re-issuing one, and it must use the SAME + # endpoint. It previously probed readyz while the check above used livez, and readyz + # lags livez by a few seconds after a restart — so straight after T006 the guard saw a + # 503, declared 8090 unpublished, and silently skipped the AC-011 assertion while the + # suite still reported "16 passed, 0 failed". + if [ "$loopback_answers" -ne 0 ]; then + info "T007 8090 does not answer on loopback either — skipping the LAN check as vacuous" + return + fi + + if curl -fsS --max-time 5 "http://$lan:8090/dotmgt/livez" >/dev/null 2>&1; then + bad "T007 8090 ANSWERS on $lan — unauthenticated /dotmgt/health and /dotmgt/metrics are on the network" + else + ok "T007 8090 refused on $lan (loopback-only, as required)" + fi +} + +# --------------------------------------------------------------------------- + +main() { + printf '\033[1mverify-cold-start.sh\033[0m — %s\n' "$COMPOSE_FILE" + + static_checks + + if [ "${1:-}" = "--static" ]; then + info "--static given; skipping runtime checks" + else + runtime_cold_start + runtime_restart + runtime_exposure + fi + + section "Summary" + printf ' %d passed, %d failed\n\n' "$PASS" "$FAIL" + [ "$FAIL" -eq 0 ] +} + +main "$@" diff --git a/core-web/libs/sdk/create-app/scripts/verify-package.sh b/core-web/libs/sdk/create-app/scripts/verify-package.sh new file mode 100755 index 000000000000..fdee7fb3f25f --- /dev/null +++ b/core-web/libs/sdk/create-app/scripts/verify-package.sh @@ -0,0 +1,61 @@ +#!/usr/bin/env bash +# +# AC-013 — the bundled compose file must be in the PUBLISHED PACKAGE. +# +# Why this exists as a separate check, over the build output: +# +# Asserting the two manifests is necessary but not sufficient. `packaging.spec.ts` reads +# package.json and project.json from the SOURCE TREE, and verify-cold-start.sh --static +# resolves the asset relative to src/. Both pass identically whether or not the file ever +# ships. A wrong `output:` in the esbuild assets entry satisfies every one of them and still +# publishes a package with no compose file — and then every local-Docker run fails at its +# first step, which is exactly what AC-013 exists to prevent. +# +# So this asserts the artifact: the file is in dist at the path the CLI resolves at runtime, +# and npm would actually put it in the tarball. +# +# Usage: ./verify-package.sh [dist-dir] (defaults to the nx output path) +# Exit 0 = the asset ships. + +set -uo pipefail + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +DIST="${1:-$SCRIPT_DIR/../../../../dist/libs/sdk/create-app}" +ASSET_REL="assets/docker-compose.yml" + +PASS=0 +FAIL=0 +ok() { printf ' \033[32mPASS\033[0m %s\n' "$1"; PASS=$((PASS + 1)); } +bad() { printf ' \033[31mFAIL\033[0m %s\n' "$1"; FAIL=$((FAIL + 1)); } + +printf '\n\033[1mverify-package.sh\033[0m — %s\n\n' "$DIST" + +if [ ! -d "$DIST" ]; then + bad "no build output at $DIST — run: pnpm nx build sdk-create-app" + printf '\n\033[1mSummary\033[0m\n %d passed, %d failed\n\n' "$PASS" "$FAIL" + exit 1 +fi + +# 1. The file is where the CLI will look for it. resolveComposeSource() walks up from the +# bundle entry, so the asset must sit beside index.js exactly as it does in the source tree. +if [ -f "$DIST/$ASSET_REL" ]; then + ok "$ASSET_REL is in the build output" +else + bad "$ASSET_REL is MISSING from the build output — check project.json's esbuild \`assets\` (input/glob/output)" +fi + +# 2. npm would actually pack it. This is the half that `files` in package.json controls, and it +# is a separate failure from the esbuild copy above: either alone ships a broken package. +PACKED="$(cd "$DIST" && npm pack --dry-run --json 2>/dev/null | grep -o "\"path\": *\"[^\"]*\"" | sed 's/.*: *"//; s/"$//')" + +if [ -z "$PACKED" ]; then + bad "npm pack --dry-run produced no file list — cannot confirm the tarball contents" +elif printf '%s\n' "$PACKED" | grep -qx "$ASSET_REL"; then + ok "npm pack includes $ASSET_REL in the tarball" +else + bad "npm pack does NOT include $ASSET_REL — check \`files\` in package.json" + printf ' tarball contains: %s\n' "$(printf '%s ' $PACKED)" +fi + +printf '\n\033[1mSummary\033[0m\n %d passed, %d failed\n\n' "$PASS" "$FAIL" +[ "$FAIL" -eq 0 ] diff --git a/core-web/libs/sdk/create-app/src/api/index.ts b/core-web/libs/sdk/create-app/src/api/index.ts index dfc3a1e0f0cb..5f8d6fcac1db 100644 --- a/core-web/libs/sdk/create-app/src/api/index.ts +++ b/core-web/libs/sdk/create-app/src/api/index.ts @@ -1,20 +1,14 @@ -import axios from 'axios'; import chalk from 'chalk'; import { DOTCMS_SITE_API, DOTCMS_EMA_CONFIG_API, DOTCMS_TOKEN_API } from '../constants'; -import { FailedToGetDefaultSiteError, FailedToSetUpUVEConfig } from '../errors'; +import { FailedToGetDefaultSiteError } from '../errors'; import { Ok, type Result, Err } from '../result'; +import { httpPost, httpGet, isHttpError } from '../utils/http'; -import type { - DefaultSiteResponse, - GetUserTokenRequest, - GetUserTokenResponse, - UVEConfigRequest, - UVEConfigResponse -} from '../types'; +import type { DefaultSiteResponse, GetUserTokenRequest, GetUserTokenResponse } from '../types'; function getSafeErrorDetails(err: unknown): string { - if (axios.isAxiosError(err)) { + if (isHttpError(err)) { const details = [ err.response?.status ? `status=${err.response.status}` : null, err.response?.statusText ? `statusText=${err.response.statusText}` : null, @@ -22,7 +16,7 @@ function getSafeErrorDetails(err: unknown): string { err.message ? `message=${err.message}` : null ].filter(Boolean); - return details.length > 0 ? details.join(', ') : 'Axios request failed'; + return details.length > 0 ? details.join(', ') : 'HTTP request failed'; } if (err instanceof Error) { @@ -48,11 +42,11 @@ export class DotCMSApi { const endpoint = url || this.defaultTokenApi; try { - const res = await axios.post(endpoint, payload); + const res = await httpPost(endpoint, payload); return Ok(res.data.entity.token); } catch (err) { // Provide specific error messages based on error type - if (axios.isAxiosError(err)) { + if (isHttpError(err)) { if (err.response?.status === 401) { return Err( chalk.red('\n❌ Authentication failed\n\n') + @@ -96,8 +90,8 @@ export class DotCMSApi { }): Promise> { try { const endpoint = (url || this.defaultSiteApi) + 'defaultSite'; - const res = await axios.get(endpoint, { - headers: { Authorization: `Bearer ${authenticationToken}` } + const res = await httpGet(endpoint, { + token: authenticationToken }); return Ok(res.data); } catch (err) { @@ -105,28 +99,4 @@ export class DotCMSApi { return Err(new FailedToGetDefaultSiteError()); } } - - /** Setup UVE Config */ - static async setupUVEConfig({ - payload, - siteId, - authenticationToken, - url - }: { - payload: UVEConfigRequest; - siteId: string; - authenticationToken: string; - url?: string; - }): Promise> { - try { - const endpoint = (url || this.defaultUveConfigApi) + siteId; - const res = await axios.post(endpoint, payload, { - headers: { Authorization: `Bearer ${authenticationToken}` } - }); - return Ok(res.data.entity); - } catch (err) { - console.error(`failed to setup UVE config: ${getSafeErrorDetails(err)}`); - return Err(new FailedToSetUpUVEConfig()); - } - } } diff --git a/core-web/libs/sdk/create-app/src/asks.spec.ts b/core-web/libs/sdk/create-app/src/asks.spec.ts new file mode 100644 index 000000000000..167c1061f534 --- /dev/null +++ b/core-web/libs/sdk/create-app/src/asks.spec.ts @@ -0,0 +1,45 @@ +import { readFileSync } from 'node:fs'; +import { resolve } from 'node:path'; + +/** + * Guard against an inquirer-version trap that costs more to diagnose than to prevent. + * + * This package uses inquirer 13, which is built on @inquirer/prompts. There the single-choice + * prompt is `select`. `list` is the inquirer 8/9 name: it is not registered, so the prompt + * renders its MESSAGE and then silently renders no choices at all. Nothing throws, nothing + * warns, and the user is left staring at a question with no answers under it. + * + * That shipped once (#37262) and survived a round of "fixes" aimed at the wrong cause, because + * the symptom looks like a rendering/styling problem rather than a wrong prompt type. + */ +describe('prompt types are the ones inquirer 13 actually registers', () => { + const source = readFileSync(resolve(__dirname, 'asks.ts'), 'utf8'); + + it("never uses type: 'list' — inquirer 13 calls it 'select'", () => { + const offenders = source + .split('\n') + .map((line, i) => ({ line: line.trim(), number: i + 1 })) + .filter(({ line }) => /type:\s*['"]list['"]/.test(line) && !line.startsWith('//')); + + expect(offenders).toEqual([]); + }); + + it('uses only prompt types this inquirer version registers', () => { + const registered = [ + 'input', + 'select', + 'checkbox', + 'confirm', + 'password', + 'expand', + 'editor', + 'number', + 'rawlist', + 'search' + ]; + const used = [...source.matchAll(/^\s*type:\s*['"]([a-z]+)['"]/gm)].map((m) => m[1]); + + expect(used.length).toBeGreaterThan(0); + expect(used.filter((t) => !registered.includes(t))).toEqual([]); + }); +}); diff --git a/core-web/libs/sdk/create-app/src/asks.ts b/core-web/libs/sdk/create-app/src/asks.ts index 00b429aab7df..98aa84766e82 100644 --- a/core-web/libs/sdk/create-app/src/asks.ts +++ b/core-web/libs/sdk/create-app/src/asks.ts @@ -1,3 +1,4 @@ +import chalk from 'chalk'; import fs from 'fs-extra'; import inquirer from 'inquirer'; @@ -206,12 +207,22 @@ export async function prepareDirectory(basePath: string, projectName: string) { return targetPath; // empty → OK } + // A docker-compose.yml here means a previous run left a stack behind — possibly one that is + // still running. Emptying the directory would delete the only file that can tear it down, + // stranding containers the user then has to hunt for by hand. That is the CLI destroying its + // own recovery path, which is the shape of the failure in #37262, so it is refused outright + // rather than folded into the blanket "all files will be deleted" confirmation. + const composePath = path.join(targetPath, 'docker-compose.yml'); + const hasComposeFile = fs.existsSync(composePath); + // Directory not empty → warn user const ans = await inquirer.prompt([ { type: 'confirm', name: 'confirm', - message: `⚠️ Directory "${targetPath}" is not empty. All files inside will be deleted. Continue?`, + message: hasComposeFile + ? `⚠️ Directory "${targetPath}" contains a docker-compose.yml from a previous run. Everything EXCEPT that file will be deleted. Continue?` + : `⚠️ Directory "${targetPath}" is not empty. All files inside will be deleted. Continue?`, default: false } ]); @@ -221,8 +232,80 @@ export async function prepareDirectory(basePath: string, projectName: string) { process.exit(1); } - // Empty directory - await fs.emptyDir(targetPath); + if (hasComposeFile) { + const preserved = await fs.readFile(composePath); + await fs.emptyDir(targetPath); + await fs.writeFile(composePath, preserved); + } else { + await fs.emptyDir(targetPath); + } return targetPath; } + +/** + * Asked when a healthy dotCMS is already listening on 8082. + * + * The earlier version said "A dotCMS instance is already running on port 8082. What would you + * like to do?" and offered only reuse or quit. That states a fact and then abandons the user: + * it never says WHAT is running, and anyone who did not want that instance had to leave the CLI + * and run docker by hand. Replacing it is the documented recovery for a bricked instance + * (#37262), so the CLI can now do it. + * + * `canReplace` is false when nothing identifiable owns the ports — something started outside + * compose is not ours to destroy, so the option is withheld rather than offered and then failed. + */ +export async function askPortConflictAction({ + description, + canReplace +}: { + description: string; + canReplace: boolean; +}): Promise<'reuse' | 'replace' | 'cancel'> { + console.log( + '\n' + + chalk.yellow('⚠ Found a dotCMS already running at ') + + chalk.cyan('http://localhost:8082') + + '\n' + + chalk.gray(` ${description}`) + + '\n' + ); + + // One line per choice. A `\n` inside a choice name breaks inquirer's line accounting and + // the list renders blank — the hint belongs in `description`, which it prints under the + // highlighted option. + const choices: { + name: string; + value: 'reuse' | 'replace' | 'cancel'; + description: string; + }[] = [ + { + name: 'Use this instance for my project', + value: 'reuse', + description: 'Fastest. Keeps its existing content.' + } + ]; + + if (canReplace) { + choices.push({ + name: 'Replace it with a clean instance', + value: 'replace', + description: 'Stops it and DELETES its data, then starts fresh.' + }); + } + + choices.push({ + name: 'Cancel', + value: 'cancel', + description: 'Change nothing and exit.' + }); + + const { action } = await inquirer.prompt([ + // `select`, NOT `list`. Inquirer 13 is built on @inquirer/prompts, where the type is + // `select`; `list` is the inquirer 8/9 name and is not registered, so the message renders + // and the choices silently do not. Every other prompt in this file already uses `select`. + { type: 'select', name: 'action', message: 'How would you like to continue?', choices } + ]); + + return action; +} diff --git a/core-web/libs/sdk/create-app/src/compose/compose-source.spec.ts b/core-web/libs/sdk/create-app/src/compose/compose-source.spec.ts new file mode 100644 index 000000000000..6030a53f4c0b --- /dev/null +++ b/core-web/libs/sdk/create-app/src/compose/compose-source.spec.ts @@ -0,0 +1,153 @@ +/** + * Contract spec for `src/compose/compose-source.ts` (task T010, dotCMS #37262). + * + * This file is written BEFORE the implementation and therefore DEFINES the API the + * implementation must satisfy. The module does not exist yet — the failing import is the + * deliberate Red state of TDD. + * + * --------------------------------------------------------------------------------------- + * API PINNED BY THIS SPEC + * --------------------------------------------------------------------------------------- + * + * export type ComposeSource = BundledComposeSource | RemoteComposeSource; + * + * interface ComposeSourceBase { + * readonly describe: string; // human-readable, shown in diagnostics (D4a) + * read(): Promise; // returns the file CONTENTS, never writes to disk + * } + * + * interface BundledComposeSource extends ComposeSourceBase { + * readonly kind: 'bundled'; + * readonly path: string; // absolute path to the packaged asset + * } + * + * interface RemoteComposeSource extends ComposeSourceBase { + * readonly kind: 'remote'; + * readonly url: string; + * } + * + * export function resolveComposeSource(): ComposeSource; + * + * `kind` is the discriminant that makes "the default path performs no network access" + * assertable BY CONSTRUCTION — no http mocking required. A `'bundled'` source carries a + * filesystem `path` and no `url`; there is nothing for it to fetch. + * + * Behaviour pinned: + * 1. No `DOTCMS_COMPOSE_URL` -> bundled asset at `/assets/docker-compose.yml`. + * 2. `DOTCMS_COMPOSE_URL=` -> remote source carrying exactly that URL verbatim. + * 3. `DOTCMS_COMPOSE_URL=''` -> falsy override, falls back to bundled (D4a uses a + * truthiness check; an empty var must not disable the + * default source). + * 4. `resolveComposeSource()` reads the env var at CALL time, not at module load time, + * so the escape hatch works without a re-import. + * + * Contract: specs/37262-create-app-docker-uve/contracts/compose-service-contract.md — C7. + * Decision: specs/37262-create-app-docker-uve/cli-design-decisions.md — D4/D4a. + */ + +import { resolveComposeSource } from './compose-source'; + +const ENV_VAR = 'DOTCMS_COMPOSE_URL'; +const OVERRIDE_URL = + 'https://raw.githubusercontent.com/dotCMS/core/main/docker/docker-compose-examples/single-node-demo-site/docker-compose.yml'; + +describe('resolveComposeSource', () => { + let savedOverride: string | undefined; + + beforeEach(() => { + savedOverride = process.env[ENV_VAR]; + delete process.env[ENV_VAR]; + }); + + afterEach(() => { + if (savedOverride === undefined) { + delete process.env[ENV_VAR]; + } else { + process.env[ENV_VAR] = savedOverride; + } + }); + + describe('by default (no DOTCMS_COMPOSE_URL)', () => { + it('resolves to the bundled asset shipped inside the npm package', () => { + const source = resolveComposeSource(); + + expect(source.kind).toBe('bundled'); + }); + + it('identifies the local packaged file by path, not by URL', () => { + const source = resolveComposeSource(); + + if (source.kind !== 'bundled') { + throw new Error( + `expected the default source to be 'bundled', got '${source.kind}'` + ); + } + + expect(typeof source.path).toBe('string'); + expect(source.path.length).toBeGreaterThan(0); + // An absolute filesystem path to the shipped asset — resolvable from the + // installed package, not relative to the user's cwd. + expect(source.path.startsWith('/')).toBe(true); + expect(source.path).toMatch(/assets[\\/]docker-compose\.yml$/); + }); + + it('performs no network access by construction — it carries no URL to fetch', () => { + const source = resolveComposeSource(); + + // The security/reliability point of #37262: today's downloadFile() uses a bare + // https.get with no timeout, no retry and no redirect handling. A 'bundled' + // source has no URL at all, so that code path is unreachable by default. + expect(source.kind).toBe('bundled'); + expect(source).not.toHaveProperty('url'); + expect(JSON.stringify(source)).not.toMatch(/https?:/); + }); + + it('exposes the ComposeSource shape: a describe string and a read() returning contents', () => { + const source = resolveComposeSource(); + + expect(typeof source.describe).toBe('string'); + expect(source.describe.length).toBeGreaterThan(0); + expect(typeof source.read).toBe('function'); + }); + + it('falls back to the bundled asset when DOTCMS_COMPOSE_URL is set but empty', () => { + process.env[ENV_VAR] = ''; + + expect(resolveComposeSource().kind).toBe('bundled'); + }); + }); + + describe('with DOTCMS_COMPOSE_URL set', () => { + it('resolves to a remote source', () => { + process.env[ENV_VAR] = OVERRIDE_URL; + + expect(resolveComposeSource().kind).toBe('remote'); + }); + + it('carries the override URL verbatim', () => { + process.env[ENV_VAR] = OVERRIDE_URL; + + const source = resolveComposeSource(); + + if (source.kind !== 'remote') { + throw new Error( + `expected the override source to be 'remote', got '${source.kind}'` + ); + } + + expect(source.url).toBe(OVERRIDE_URL); + expect(source).not.toHaveProperty('path'); + expect(typeof source.read).toBe('function'); + }); + + it('reads the environment variable at call time, so the escape hatch needs no re-import', () => { + expect(resolveComposeSource().kind).toBe('bundled'); + + process.env[ENV_VAR] = OVERRIDE_URL; + expect(resolveComposeSource().kind).toBe('remote'); + + delete process.env[ENV_VAR]; + expect(resolveComposeSource().kind).toBe('bundled'); + }); + }); +}); diff --git a/core-web/libs/sdk/create-app/src/compose/compose-source.ts b/core-web/libs/sdk/create-app/src/compose/compose-source.ts new file mode 100644 index 000000000000..efc557ec49be --- /dev/null +++ b/core-web/libs/sdk/create-app/src/compose/compose-source.ts @@ -0,0 +1,165 @@ +import fs from 'fs-extra'; + +import path from 'path'; + +/** + * Environment variable that overrides the packaged compose file with a remote one. + * + * Documented escape hatch (D4a): it is intentionally a truthiness check, so exporting the + * variable as an empty string does NOT disable the bundled default. + */ +export const COMPOSE_URL_ENV_VAR = 'DOTCMS_COMPOSE_URL'; + +/** Relative location of the packaged asset inside the published npm package. */ +const ASSET_RELATIVE_PATH = path.join('assets', 'docker-compose.yml'); + +/** Timeout applied to the remote read, in milliseconds. */ +const REMOTE_READ_TIMEOUT_MS = 15000; + +interface ComposeSourceBase { + /** Human-readable origin, shown in diagnostics (D4a). */ + readonly describe: string; + /** Resolves with the compose file CONTENTS — this never writes to disk. */ + read(): Promise; +} + +/** The compose file shipped inside the npm package. Carries no URL: it cannot hit the network. */ +export interface BundledComposeSource extends ComposeSourceBase { + readonly kind: 'bundled'; + /** Absolute path to the packaged asset. */ + readonly path: string; +} + +/** A compose file fetched from `DOTCMS_COMPOSE_URL`. */ +export interface RemoteComposeSource extends ComposeSourceBase { + readonly kind: 'remote'; + readonly url: string; +} + +export type ComposeSource = BundledComposeSource | RemoteComposeSource; + +/** + * Best starting directory for locating the packaged asset, under BOTH module systems. + * + * `src/` ships as ESM (`"type": "module"`) but Jest compiles the specs to CommonJS + * (`tsconfig.spec.json` sets `"module": "commonjs"`), where a bare `import.meta.url` is a + * compile-time syntax error. Rather than reach for `eval` to smuggle `import.meta` past the + * CommonJS emit — which works, but trips esbuild's `direct-eval` warning and defeats bundler + * analysis — this only needs to be *approximately* right: `resolveBundledAssetPath()` below + * walks up from here until it finds the asset. + * + * - CommonJS (Jest): `__dirname` is defined. + * - ESM (the shipped bundle): `__dirname` is not, but this is a `bin` entry point, so + * `process.argv[1]` is the bundle itself and its directory is the package root. + */ +function currentModuleDir(): string { + if (typeof __dirname === 'string') { + return __dirname; + } + + const entryPoint = process.argv[1]; + + if (typeof entryPoint === 'string' && entryPoint.length > 0) { + return path.dirname(entryPoint); + } + + return process.cwd(); +} + +/** + * Walks up from this module looking for the packaged `assets/docker-compose.yml`. + * + * The layout differs between the checked-out source (`src/compose/…` -> `/assets`) and the + * published bundle (`/index.js` -> `/assets`), so the asset is located by search + * rather than by a hardcoded number of `..` segments. + */ +function resolveBundledAssetPath(): string { + let dir = currentModuleDir(); + let fallback = path.resolve(dir, ASSET_RELATIVE_PATH); + + for (;;) { + const candidate = path.join(dir, ASSET_RELATIVE_PATH); + + if (fs.existsSync(candidate)) { + return candidate; + } + + // Remember the package root as the best guess if the asset is missing entirely. + if (fs.existsSync(path.join(dir, 'package.json'))) { + fallback = candidate; + } + + const parent = path.dirname(dir); + + if (parent === dir) { + return fallback; + } + + dir = parent; + } +} + +function createBundledSource(): BundledComposeSource { + const assetPath = resolveBundledAssetPath(); + + return { + kind: 'bundled', + path: assetPath, + describe: `bundled compose file (${assetPath})`, + read: () => fs.readFile(assetPath, 'utf8') + }; +} + +function createRemoteSource(url: string): RemoteComposeSource { + return { + kind: 'remote', + url, + describe: `remote compose file from ${COMPOSE_URL_ENV_VAR}`, + read: async () => { + const controller = new AbortController(); + const timer = setTimeout(() => controller.abort(), REMOTE_READ_TIMEOUT_MS); + + try { + const response = await fetch(url, { + signal: controller.signal, + redirect: 'follow' + }); + + if (!response.ok) { + throw new Error( + `Failed to download compose file from ${url}: ${response.status} ${response.statusText}` + ); + } + + return await response.text(); + } catch (error) { + if (error instanceof Error && error.name === 'AbortError') { + throw new Error( + `Timed out after ${REMOTE_READ_TIMEOUT_MS}ms downloading compose file from ${url}` + ); + } + + throw error; + } finally { + clearTimeout(timer); + } + } + }; +} + +/** + * Resolves where the docker compose file comes from. + * + * Returns the asset bundled in the package unless `DOTCMS_COMPOSE_URL` is set to a non-empty + * value, in which case the URL is used verbatim. The environment is read on every call, so the + * escape hatch takes effect without re-importing this module. + */ +export function resolveComposeSource(): ComposeSource { + const override = process.env[COMPOSE_URL_ENV_VAR]; + + if (override) { + return createRemoteSource(override); + } + + return createBundledSource(); +} diff --git a/core-web/libs/sdk/create-app/src/exit-state.spec.ts b/core-web/libs/sdk/create-app/src/exit-state.spec.ts new file mode 100644 index 000000000000..1aace54ebed0 --- /dev/null +++ b/core-web/libs/sdk/create-app/src/exit-state.spec.ts @@ -0,0 +1,501 @@ +/** + * Contract spec for `src/exit-state.ts` (task T022/T028/T029, dotCMS #37262). + * + * Written BEFORE the implementation, so this file DEFINES the API the implementation must + * satisfy. `src/exit-state.ts` does not exist yet — the failing import is the deliberate Red + * state of TDD (constitution Principle V). + * + * --------------------------------------------------------------------------------------- + * WHY THIS MODULE EXISTS + * --------------------------------------------------------------------------------------- + * + * Contract X1: once `token` and `siteId` are non-null, EVERY terminal path — success, handled + * failure, or unexpected throw — emits `host`, `token`, `siteId` and writes `.env`. + * + * Today the only code that prints the token is `displayFinalSteps()` (`src/index.ts:532`), and + * it sits downstream of all 17 `process.exit(1)` call sites — 13 of them inside the single + * `try` opened at `src/index.ts:93`. So a run that already holds a working token can, and does, + * exit having printed nothing. `finally` cannot fix this: it does NOT run on `process.exit()` + * (D1). A `process.on('exit')` handler does, and it also covers exit paths nobody has written + * yet — which is the whole point of the decision. + * + * --------------------------------------------------------------------------------------- + * API PINNED BY THIS SPEC + * --------------------------------------------------------------------------------------- + * + * export interface RecoverableState { + * host: string; // e.g. 'http://localhost:8082' + * token: string; // the issued API token — the thing that must never be lost + * siteId: string; // resolved default site identifier + * projectDirectory: string; // absolute dir that receives `.env` + * framework?: string; // decides which variable names go in the block + * } + * + * // Merges into module-level state. Callers record what they know, when they know it + * // (`projectDirectory` is known long before `token`), so this takes a Partial. + * export function recordRecoverableState(state: Partial): void; + * + * // Registers the single `process.on('exit')` handler. Idempotent: calling it more than + * // once must NOT register a second listener. + * export function installExitStateHandler(): void; + * + * // Test seam: unregisters the handler and clears recorded state. Exists so specs cannot + * // leak a live exit listener (or a stale token) into each other. + * export function resetExitState(): void; + * + * Behaviour pinned: + * 1. With host+token+siteId recorded, the handler prints all three to STDOUT. Stdout — not + * stderr — because a wrapper piping the CLI's output is exactly the consumer that must be + * able to recover these values. + * 2. The handler is registered on `'exit'`, never on `'beforeExit'`. `'beforeExit'` is + * skipped on `process.exit()`, which is the case that actually loses the token today. + * 3. `.env` is written when absent, and contains the recorded values (D6: always `.env`, + * every framework). + * 4. An existing `.env` is left byte-for-byte alone; the paste block is printed instead + * (D6 / X8 / research R7). + * 5. Nothing is printed and nothing is written when no token has been recorded — there is + * no successful state to recover, so a plain `--help` run stays silent. + * 6. The handler is synchronous: `writeFileSync`, never `writeFile`/`fs.promises`, and it + * returns `undefined`, not a promise. Node runs no async work during `'exit'`, so an + * async write is the same as no write at all. + * 7. Registering twice prints once and writes once. + * + * NOTE ON HOW THE HANDLER IS FIRED HERE + * A spec cannot exit the process it is running in, and emitting `'exit'` on the real `process` + * would also fire Jest's own listeners. So these tests capture the listener the module adds + * (by diffing `process.listeners('exit')` across `installExitStateHandler()`) and invoke just + * that one, with the exit code Node itself would pass: `0` for an ordinary exit, non-zero for + * an explicit `process.exit(1)`. The "is it on the right event" half of the guarantee is + * asserted structurally, in the `'beforeExit'` test. + * + * Contract: specs/37262-create-app-docker-uve/contracts/cli-exit-contract.md — X1 (and X8). + * Decisions: specs/37262-create-app-docker-uve/cli-design-decisions.md — D1, D6. + * Acceptance criterion: AC-004. + */ + +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; + +import { + flushRecoverableState, + installExitStateHandler, + recordRecoverableState, + resetExitState +} from './exit-state'; + +type ExitListener = (code: number) => unknown; + +const HOST = 'http://localhost:8082'; +const TOKEN = 'eyJhbGciOiJIUzI1NiJ9.recoverable-token-37262'; +const SITE_ID = '48190c8c-42c4-46af-8d1a-0cd5db894797'; + +describe('exit-state (contract X1 — no successful state is ever discarded)', () => { + let tmpDir: string; + let envPath: string; + let stdout: string[]; + let listenersBefore: ExitListener[]; + + /** The listeners `installExitStateHandler()` added, and nothing else. */ + const installedListeners = (): ExitListener[] => + (process.listeners('exit') as ExitListener[]).filter( + (listener) => !listenersBefore.includes(listener) + ); + + /** Simulate Node emitting `'exit'` with `code`, without touching Jest's own listeners. */ + const fireExit = (code = 0): unknown[] => + installedListeners().map((listener) => listener(code)); + + const output = (): string => stdout.join(''); + + beforeEach(() => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'dotcms-exit-state-')); + envPath = path.join(tmpDir, '.env'); + + stdout = []; + jest.spyOn(console, 'log').mockImplementation((...args: unknown[]) => { + stdout.push(args.map(String).join(' ') + '\n'); + }); + jest.spyOn(process.stdout, 'write').mockImplementation((chunk: unknown) => { + stdout.push(String(chunk)); + + return true; + }); + + listenersBefore = process.listeners('exit') as ExitListener[]; + }); + + afterEach(() => { + // Unregister before restoring the spies, so a leaked handler cannot print into a + // later test — or into Jest's own shutdown. + resetExitState(); + installedListeners().forEach((listener) => process.off('exit', listener)); + + jest.restoreAllMocks(); + fs.rmSync(tmpDir, { recursive: true, force: true }); + }); + + describe('printing recovered state', () => { + /** + * When `.env` is written, the token belongs in the FILE, not echoed into the terminal. + * The CLI used to print it twice — once in a "paste this into .env" block and again in + * the recovery block — while having already written the file, so it both duplicated a + * JWT into scrollback and told the user to do something already done. + */ + it('confirms the file it wrote, and does NOT echo the token into scrollback', () => { + installExitStateHandler(); + recordRecoverableState({ + host: HOST, + token: TOKEN, + siteId: SITE_ID, + projectDirectory: tmpDir, + framework: 'nextjs' + }); + + fireExit(0); + + expect(output()).toContain('.env'); + expect(output()).toContain(HOST); + expect(output()).toContain(SITE_ID); + // The value is safe on disk; scrollback and CI logs do not need a copy. + expect(output()).not.toContain(TOKEN); + expect(fs.readFileSync(envPath, 'utf8')).toContain(TOKEN); + }); + + it('DOES print the token when it could not write it anywhere', () => { + fs.writeFileSync(envPath, 'PRE_EXISTING=1\n', 'utf8'); + + installExitStateHandler(); + recordRecoverableState({ + host: HOST, + token: TOKEN, + siteId: SITE_ID, + projectDirectory: tmpDir, + framework: 'nextjs' + }); + + fireExit(0); + + // Nothing was written, so the terminal is the only place the run survives. + expect(output()).toContain(TOKEN); + }); + + it('prints host, token and siteId once state has been recorded', () => { + recordRecoverableState({ + host: HOST, + token: TOKEN, + siteId: SITE_ID, + projectDirectory: tmpDir, + framework: 'angular' + }); + installExitStateHandler(); + + fireExit(0); + + expect(output()).toContain(HOST); + expect(output()).toContain(TOKEN); + expect(output()).toContain(SITE_ID); + }); + + it('prints on an ordinary exit (code 0)', () => { + recordRecoverableState({ + host: HOST, + token: TOKEN, + siteId: SITE_ID, + projectDirectory: tmpDir, + framework: 'angular' + }); + installExitStateHandler(); + + fireExit(0); + + expect(output()).toContain(TOKEN); + }); + + it('prints on an explicit process.exit(1) — the path `finally` never reaches', () => { + recordRecoverableState({ + host: HOST, + token: TOKEN, + siteId: SITE_ID, + projectDirectory: tmpDir, + framework: 'angular' + }); + installExitStateHandler(); + + // Node passes the code given to `process.exit()` straight to `'exit'` listeners. + // This is the 17-call-site case the whole module exists for. + fireExit(1); + + expect(output()).toContain(HOST); + expect(output()).toContain(TOKEN); + expect(output()).toContain(SITE_ID); + }); + + it("registers on 'exit', not on 'beforeExit'", () => { + const beforeExitCount = process.listeners('beforeExit').length; + + installExitStateHandler(); + + // 'beforeExit' does not fire on process.exit(); registering there would reproduce + // the exact bug this module fixes. + expect(installedListeners()).toHaveLength(1); + expect(process.listeners('beforeExit')).toHaveLength(beforeExitCount); + }); + }); + + /** + * Found by running the CLI end to end (T054), not by any unit test that existed. + * + * The recovery block and the written file MUST use the same variable names the scaffolded + * app actually reads, and those differ per framework: Next.js takes `NEXT_PUBLIC_*`, Astro + * takes `PUBLIC_*`, and Angular does not use a dotenv file at all — it reads a TypeScript + * `environment` object. A .env that looks plausible but names the token wrong is worse than + * no .env: `npm run dev` fails to authenticate and nothing says why. + */ + /** + * The handler runs at process exit, so anything it prints necessarily lands AFTER the + * "Next Steps" block — which meant a successful run reported its connection details twice, + * once inside the summary and once tacked on the end. + * + * `flushRecoverableState()` lets the success path do the write and claim the reporting, so + * the details appear inside Next Steps where they belong. The handler then stays silent — + * it is a fallback for paths that never reach the summary, not a second announcement. + */ + describe('who reports the state', () => { + it('flush writes the file and hands back what to render', () => { + recordRecoverableState({ + host: HOST, + token: TOKEN, + siteId: SITE_ID, + projectDirectory: tmpDir, + framework: 'nextjs' + }); + + const report = flushRecoverableState(); + + expect(report).toMatchObject({ + wroteEnv: true, + filename: '.env', + host: HOST, + siteId: SITE_ID + }); + expect(fs.readFileSync(envPath, 'utf8')).toContain(TOKEN); + }); + + it('the exit handler stays silent once flush has reported', () => { + installExitStateHandler(); + recordRecoverableState({ + host: HOST, + token: TOKEN, + siteId: SITE_ID, + projectDirectory: tmpDir, + framework: 'nextjs' + }); + + flushRecoverableState(); + fireExit(0); + + // No second announcement after the summary the caller already printed. + expect(output()).toBe(''); + }); + + it('the exit handler still speaks when nothing flushed — the recovery case', () => { + installExitStateHandler(); + recordRecoverableState({ + host: HOST, + token: TOKEN, + siteId: SITE_ID, + projectDirectory: tmpDir, + framework: 'nextjs' + }); + + fireExit(0); + + expect(output()).toContain(HOST); + }); + + it('flush returns null when there is nothing to report', () => { + expect(flushRecoverableState()).toBeNull(); + }); + }); + + describe('the written file matches what the scaffolded app reads', () => { + it('uses NEXT_PUBLIC_ names for a Next.js project', () => { + installExitStateHandler(); + recordRecoverableState({ + host: HOST, + token: TOKEN, + siteId: SITE_ID, + projectDirectory: tmpDir, + framework: 'nextjs' + }); + fireExit(0); + + const written = fs.readFileSync(envPath, 'utf8'); + + expect(written).toContain(`NEXT_PUBLIC_DOTCMS_AUTH_TOKEN=${TOKEN}`); + expect(written).toContain(`NEXT_PUBLIC_DOTCMS_HOST=${HOST}`); + expect(written).toContain(`NEXT_PUBLIC_DOTCMS_SITE_ID=${SITE_ID}`); + // The bare name is what the bug wrote; it must not appear on its own. + expect(written).not.toMatch(/^\s*DOTCMS_AUTH_TOKEN=/m); + }); + + it('uses PUBLIC_ names for an Astro project', () => { + installExitStateHandler(); + recordRecoverableState({ + host: HOST, + token: TOKEN, + siteId: SITE_ID, + projectDirectory: tmpDir, + framework: 'astro' + }); + fireExit(0); + + const written = fs.readFileSync(envPath, 'utf8'); + + expect(written).toContain(`PUBLIC_DOTCMS_AUTH_TOKEN=${TOKEN}`); + expect(written).not.toContain('NEXT_PUBLIC_DOTCMS_AUTH_TOKEN='); + }); + + it('writes no .env for Angular, which reads a TypeScript environment object', () => { + installExitStateHandler(); + recordRecoverableState({ + host: HOST, + token: TOKEN, + siteId: SITE_ID, + projectDirectory: tmpDir, + framework: 'angular' + }); + fireExit(0); + + expect(fs.existsSync(envPath)).toBe(false); + // The values still have to reach the user. + expect(output()).toContain(TOKEN); + }); + }); + + describe('.env (D6 — always `.env`, write-if-absent)', () => { + it('writes .env with the recorded values when the file is absent', () => { + recordRecoverableState({ + host: HOST, + token: TOKEN, + siteId: SITE_ID, + projectDirectory: tmpDir, + framework: 'nextjs' + }); + installExitStateHandler(); + + expect(fs.existsSync(envPath)).toBe(false); + + fireExit(0); + + expect(fs.existsSync(envPath)).toBe(true); + + const written = fs.readFileSync(envPath, 'utf8'); + expect(written).toContain(HOST); + expect(written).toContain(TOKEN); + expect(written).toContain(SITE_ID); + }); + + it('leaves an existing .env untouched and prints the paste block instead', () => { + const existing = + '# shipped by the scaffolded example\nNEXT_PUBLIC_DOTCMS_HOST=keep-me\n'; + fs.writeFileSync(envPath, existing, 'utf8'); + + recordRecoverableState({ + host: HOST, + token: TOKEN, + siteId: SITE_ID, + projectDirectory: tmpDir, + framework: 'nextjs' + }); + installExitStateHandler(); + + fireExit(0); + + // Byte-for-byte: the user's file is never clobbered. + expect(fs.readFileSync(envPath, 'utf8')).toBe(existing); + + // ...but the values still have to reach the user, as a block they can paste. + expect(output()).toContain(HOST); + expect(output()).toContain(TOKEN); + expect(output()).toContain(SITE_ID); + expect(output()).toContain('.env'); + }); + }); + + describe('nothing to recover', () => { + it('prints nothing and writes nothing when no token has been recorded', () => { + recordRecoverableState({ host: HOST, projectDirectory: tmpDir }); + installExitStateHandler(); + + fireExit(0); + + expect(output()).toBe(''); + expect(fs.readdirSync(tmpDir)).toEqual([]); + }); + + it('prints nothing and writes nothing when nothing at all has been recorded', () => { + installExitStateHandler(); + + fireExit(0); + + expect(output()).toBe(''); + expect(fs.readdirSync(tmpDir)).toEqual([]); + }); + }); + + describe('the handler is synchronous (D1)', () => { + it('writes with writeFileSync, never writeFile, and returns no promise', () => { + const writeFileSync = jest.spyOn(fs, 'writeFileSync'); + const writeFile = jest.spyOn(fs, 'writeFile'); + const writeFileAsync = jest.spyOn(fs.promises, 'writeFile'); + + recordRecoverableState({ + host: HOST, + token: TOKEN, + siteId: SITE_ID, + projectDirectory: tmpDir + }); + installExitStateHandler(); + + const returned = fireExit(0); + + expect(writeFileSync).toHaveBeenCalled(); + expect(writeFile).not.toHaveBeenCalled(); + expect(writeFileAsync).not.toHaveBeenCalled(); + + // Node runs nothing async during 'exit', so a returned thenable would mean the + // write never lands. + returned.forEach((value) => { + expect(value).toBeUndefined(); + }); + + // No `await`, no tick: the file is already on disk the instant the call returns. + expect(fs.existsSync(envPath)).toBe(true); + }); + }); + + describe('idempotent installation', () => { + it('registering twice prints once and writes once', () => { + const writeFileSync = jest.spyOn(fs, 'writeFileSync'); + + recordRecoverableState({ + host: HOST, + token: TOKEN, + siteId: SITE_ID, + projectDirectory: tmpDir + }); + + installExitStateHandler(); + installExitStateHandler(); + + expect(installedListeners()).toHaveLength(1); + + fireExit(0); + + expect(writeFileSync).toHaveBeenCalledTimes(1); + expect(output().split(SITE_ID)).toHaveLength(2); // reported exactly once + }); + }); +}); diff --git a/core-web/libs/sdk/create-app/src/exit-state.ts b/core-web/libs/sdk/create-app/src/exit-state.ts new file mode 100644 index 000000000000..892b7135b0cb --- /dev/null +++ b/core-web/libs/sdk/create-app/src/exit-state.ts @@ -0,0 +1,201 @@ +import fs from 'node:fs'; +import path from 'node:path'; + +import { getEnvFileSpec } from './utils'; + +/** + * Guarantees that a run never throws away state it already obtained. + * + * The failure reported in #37262 is a contract violation more than a bug: the CLI held a + * working API token and site ID, hit a non-essential configuration error, called + * `process.exit(1)`, and left the user with an empty directory — the token and site ID never + * printed, never written anywhere. Everything up to that point had *succeeded*. + * + * `displayFinalSteps()` is the only code that prints those values, and it sits downstream of + * all 17 `process.exit` call sites (13 of them inside the single `try` opened at + * `src/index.ts:93`). A `finally` cannot close that gap: `finally` does not run on + * `process.exit()`. A `process.on('exit')` handler does — and it also covers exit paths nobody + * has written yet, which is the durable part of the decision (D1). + * + * Everything here is synchronous. Node runs no async work during `'exit'`, so an async write + * is indistinguishable from no write at all. + */ + +export interface RecoverableState { + /** Instance base URL — e.g. `http://localhost:8082`. */ + host: string; + /** The issued API token: the thing that must never be lost. */ + token: string; + siteId: string; + /** Absolute directory that receives `.env`. */ + projectDirectory: string; + /** Chooses the variable names in the printed block. */ + framework?: string; +} + +let recorded: Partial = {}; +let handler: (() => void) | null = null; +/** Set once someone has surfaced the state, so the exit handler does not repeat it. */ +let reported = false; + +/** What the caller needs to render the connection details itself. */ +export interface RecoverableReport { + wroteEnv: boolean; + /** The dotenv file written, or null for frameworks that use none (Angular). */ + filename: string | null; + host: string; + siteId: string; + token: string; + /** The env body, for the cases where the caller must show it rather than a file path. */ + contents: string; +} + +/** + * Records what the run knows so far. Callers merge in values as they arrive — the project + * directory is settled long before a token exists — so this deliberately takes a `Partial` + * rather than demanding the whole shape up front. + */ +export function recordRecoverableState(state: Partial): void { + recorded = { ...recorded, ...state }; +} + +/** True once there is genuinely something worth recovering. */ +function hasRecoverableState( + state: Partial +): state is RecoverableState & { projectDirectory: string } { + return Boolean(state.host && state.token && state.siteId); +} + +/** + * Delegates to the single owner of the env shape (`getEnvFileSpec`) rather than hand-rolling it. + * + * An earlier version of this file wrote its own variable names, and got them wrong: it emitted + * `DOTCMS_AUTH_TOKEN` where Next.js reads `NEXT_PUBLIC_DOTCMS_AUTH_TOKEN`, so the `.env` looked + * correct and the app silently could not authenticate. + */ +function envFileFor(state: RecoverableState) { + return getEnvFileSpec(state.framework, state.host, state.siteId, state.token); +} + +/** + * Writes the env file if it is missing and claims responsibility for reporting. + * + * The success path calls this so the details can appear inside its own summary; the exit + * handler then has nothing left to say. Without it the handler could only ever append, which + * is how a run came to print its connection details twice. + */ +export function flushRecoverableState(): RecoverableReport | null { + if (!hasRecoverableState(recorded)) { + return null; + } + + const state = recorded as RecoverableState; + const envFile = envFileFor(state); + let wroteEnv = false; + + if (state.projectDirectory && envFile.filename) { + const envPath = path.join(state.projectDirectory, envFile.filename); + + // Write-if-absent (D6). An existing file is the user's — or the scaffolded example's — + // and silently overwriting it would trade one kind of data loss for another. + if (!fs.existsSync(envPath)) { + try { + fs.writeFileSync( + envPath, + `# Written by @dotcms/create-app so this run is never lost.\n${envFile.contents}`, + 'utf8' + ); + wroteEnv = true; + } catch { + // Never let recovery reporting be the thing that fails the run. + } + } + } + + reported = true; + + return { + wroteEnv, + filename: envFile.filename, + host: state.host, + siteId: state.siteId, + token: state.token, + contents: envFile.contents + }; +} + +/** Standalone report, used only when nothing else surfaced the state. */ +function emit(): void { + const report = flushRecoverableState(); + + if (!report) { + return; + } + + if (report.wroteEnv) { + console.log( + [ + '', + `Wrote ${report.filename} with your dotCMS connection details.`, + ` host : ${report.host}`, + ` site id : ${report.siteId}`, + ` token : stored in ${report.filename}` + ].join('\n') + ); + + return; + } + + // Nothing written, so the terminal is the only place this run survives (contract X1). + console.log( + [ + '', + 'dotCMS connection details for this run:', + ` host : ${report.host}`, + ` site id : ${report.siteId}`, + ` token : ${report.token}`, + '', + report.filename + ? `Add these to your ${report.filename}:` + : 'Configuration for your project:', + ...report.contents.trimEnd().split('\n') + ].join('\n') + ); +} + +/** + * Registers the single `'exit'` handler. + * + * Deliberately `'exit'` and never `'beforeExit'`: `'beforeExit'` is skipped on + * `process.exit()`, which is precisely the path that loses the token today. + * + * Idempotent — repeated calls must not stack listeners, or the recovery block prints twice. + */ +export function installExitStateHandler(): void { + if (handler) { + return; + } + + handler = () => { + // Silent when the run already reported for itself — this is a fallback, not a second + // announcement. + if (reported) { + return; + } + + emit(); + }; + + process.on('exit', handler); +} + +/** Test seam: unregister the handler and clear state so specs cannot leak into each other. */ +export function resetExitState(): void { + if (handler) { + process.off('exit', handler); + handler = null; + } + + recorded = {}; + reported = false; +} diff --git a/core-web/libs/sdk/create-app/src/git/index.ts b/core-web/libs/sdk/create-app/src/git/index.ts index a68ead0df8a7..9702c6422417 100644 --- a/core-web/libs/sdk/create-app/src/git/index.ts +++ b/core-web/libs/sdk/create-app/src/git/index.ts @@ -3,7 +3,7 @@ import fs from 'fs-extra'; import path from 'path'; -import { downloadFile } from '../utils'; +import { resolveComposeSource } from '../compose/compose-source'; import type { SupportedFrontEndFrameworks } from '../types'; @@ -63,25 +63,32 @@ export const cloneFrontEndSample = async ({ } }; +/** + * Writes the docker-compose file into the project directory. + * + * The file is **bundled with this package** rather than fetched from `main` at run + * time. The previous behaviour downloaded + * `docker/docker-compose-examples/single-node-demo-site/docker-compose.yml`, which + * meant every installed CLI silently picked up whatever was on `main` — so the + * shared example could not be hardened without shipping that change, unversioned, + * to consumers who never asked for it. Owning the file removes that coupling; the + * shared example is now left untouched (issue #37262, AC-010). + * + * `DOTCMS_COMPOSE_URL` keeps remote fetching one env var away for field hotfixes. + */ export async function downloadDockerCompose(directory: string) { - // 6. Download docker-compose file - const dockerUrl = - 'https://raw.githubusercontent.com/dotCMS/core/main/docker/docker-compose-examples/single-node-demo-site/docker-compose.yml'; - + const source = resolveComposeSource(); const dockerComposePath = path.join(directory, 'docker-compose.yml'); - await downloadFile(dockerUrl, dockerComposePath); -} - -export async function moveDockerComposeOneLevelUp(directory: string) { - const sourcePath = path.join(directory, 'docker-compose.yml'); - const targetPath = path.join(directory, '..', 'docker-compose.yml'); - await fs.rename(sourcePath, targetPath); + const contents = await source.read(); + await fs.writeFile(dockerComposePath, contents); } -export async function moveDockerComposeBack(directory: string) { - const sourcePath = path.join(directory, '..', 'docker-compose.yml'); - const targetPath = path.join(directory, 'docker-compose.yml'); - - await fs.rename(sourcePath, targetPath); -} +/* + * `moveDockerComposeOneLevelUp` / `moveDockerComposeBack` used to live here. They are gone, + * not merely unused: they moved the compose file into the parent — the user's cwd — with no + * guard and no `finally`, which is both defects review found on this branch (a clobbered + * `docker-compose.yml`, and a stranded one after a failure). They had no callers, so leaving + * them would only invite the bug back. `withComposeFileMovedAside` in `utils/compose-move.ts` + * is the one supported way to do this. + */ diff --git a/core-web/libs/sdk/create-app/src/index.ts b/core-web/libs/sdk/create-app/src/index.ts index 871c759a176c..a3bd8af65250 100644 --- a/core-web/libs/sdk/create-app/src/index.ts +++ b/core-web/libs/sdk/create-app/src/index.ts @@ -16,7 +16,8 @@ import { askPasswordForDotcmsCloud, askProjectName, askUserNameForDotcmsCloud, - prepareDirectory + prepareDirectory, + askPortConflictAction } from './asks'; import { CLOUD_HEALTH_CHECK_RETRIES, @@ -26,15 +27,14 @@ import { } from './constants'; import { FailedToCreateFrontendProjectError, FailedToDownloadDockerComposeError } from './errors'; import { - cloneFrontEndSample, - downloadDockerCompose, - moveDockerComposeBack, - moveDockerComposeOneLevelUp -} from './git'; + flushRecoverableState, + installExitStateHandler, + recordRecoverableState +} from './exit-state'; +import { cloneFrontEndSample, downloadDockerCompose } from './git'; import { type Result, Ok, Err } from './result'; import { checkDockerAvailability, - checkPortsAvailability, displayDependencies, fetchWithRetry, finalStepsForAngularAndAngularSSR, @@ -44,9 +44,16 @@ import { getDockerDiagnostics, getDotcmsApisByBaseUrl, getPortByFramework, - getUVEConfigValue, - installDependenciesForProject + installDependenciesForProject, + findBusyPorts } from './utils'; +import { withComposeFileMovedAside } from './utils/compose-move'; +import { formatRetryReport, isSuccessStatus, type RetryReporter } from './utils/fetch-retry'; +import { httpGet } from './utils/http'; +import { reportInstallResult } from './utils/install'; +import { describePortOwner, resolvePortConflict } from './utils/ports'; +import { waitForReadiness } from './utils/readiness'; +import { applyStarterUrl } from './utils/starter-url'; import { normalizeUrl, validateAndNormalizeFramework, @@ -54,11 +61,28 @@ import { validateProjectName, validateUrl } from './utils/validation'; +import { configureUVE } from './uve/configure-uve'; import type { DotCmsCliOptions, SupportedFrontEndFrameworks } from './types'; +/** Budget for `docker compose up --wait`: a cold run pulls ~2GB and imports the demo starter. */ +const COMPOSE_WAIT_TIMEOUT_SECONDS = 600; + +/** How often the wait ticker repaints. Frequent enough to look alive, rare enough not to churn. */ +const PROGRESS_TICK_MS = 2000; + // Supported values +/** Host the bundled compose stack publishes dotCMS on. */ +const LOCAL_DOTCMS_HOST = 'http://localhost:8082'; + +/** Management port, published on loopback only by the bundled compose file. */ +const LOCAL_MANAGEMENT_HOST = 'http://127.0.0.1:8090'; + +// Registered before anything can fail: once a token exists, every terminal path — including +// the 17 process.exit() sites that `finally` cannot reach — prints it and writes .env (X1). +installExitStateHandler(); + const program = new Command(); program @@ -118,8 +142,6 @@ program const urlDotcmsInstance = normalizeUrl(urlInput); const healthApiURL = getDotcmsApisByBaseUrl(urlDotcmsInstance).DOTCMS_HEALTH_API; - const emaConfigApiURL = - getDotcmsApisByBaseUrl(urlDotcmsInstance).DOTCMS_EMA_CONFIG_API; const siteApiURL = getDotcmsApisByBaseUrl(urlDotcmsInstance).DOTCMS_SITE_API; const tokenApiUrl = getDotcmsApisByBaseUrl(urlDotcmsInstance).DOTCMS_TOKEN_API; @@ -127,7 +149,10 @@ program const healthCheckResult = await isDotcmsRunning( healthApiURL, - CLOUD_HEALTH_CHECK_RETRIES + CLOUD_HEALTH_CHECK_RETRIES, + (report) => { + spinner.text = formatRetryReport(report); + } ); if (!healthCheckResult.ok) { @@ -209,25 +234,29 @@ program const selectedFramework = validatedFramework ?? (await askFramework()); - const setUpUVE = await DotCMSApi.setupUVEConfig({ - payload: { - configuration: { - hidden: false, - value: getUVEConfigValue( - `http://localhost:${getPortByFramework(selectedFramework as SupportedFrontEndFrameworks)}` - ) - } - }, + recordRecoverableState({ + host: urlDotcmsInstance, + token: dotcmsToken.val, siteId: defaultSite.val.entity.identifier, - authenticationToken: dotcmsToken.val, - url: emaConfigApiURL + projectDirectory: finalDirectory, + framework: selectedFramework }); - if (!setUpUVE.ok) { - spinner.fail('Failed to setup UVE configuration in Dotcms.'); - process.exit(1); - } else { + // Optional step: a failure here must not cost the user the run (contract X2). + const uveOutcome = await configureUVE({ + host: urlDotcmsInstance, + siteId: defaultSite.val.entity.identifier, + token: dotcmsToken.val, + mode: 'remote', + frontendUrl: `http://localhost:${getPortByFramework(selectedFramework as SupportedFrontEndFrameworks)}`, + report: (message) => spinner.info(message) + }); + + if (uveOutcome.kind === 'configured') { spinner.succeed(`Configured the Universal Visual Editor`); + } else { + spinner.warn('Skipped Universal Visual Editor configuration.'); + console.log(chalk.yellow(uveOutcome.message)); } await startScaffoldingFrontEnd({ spinner, selectedFramework, finalDirectory }); console.log(chalk.white(`✅ Project setup complete!`)); @@ -253,59 +282,145 @@ program } spinner.succeed('Docker is available'); - // STEP 2 — Check if required ports are available + // STEP 2 — Check if required ports are available. + // + // A busy 8082 is not automatically a conflict: after a successful run it is this + // CLI's own dotCMS. Refusing to start there is what made reproduction step 6 + // unrecoverable, so probe before failing (AC-006, decision D3). spinner.start('Checking port availability...'); - const portsAvailable = await checkPortsAvailability(); - if (!portsAvailable.ok) { - spinner.fail('Required ports are busy'); - console.error(portsAvailable.val); - process.exit(1); - } - spinner.succeed('All required ports are available'); + const busyPorts = await findBusyPorts(); + const portOutcome = await resolvePortConflict({ + busyPorts, + isInteractive: Boolean(process.stdout.isTTY) && !process.env.CI, + host: LOCAL_DOTCMS_HOST, + probeInstance: async () => { + // Reusable means usable for what happens next: it must answer readiness AND + // be able to issue a token. A half-dead instance is still a hard failure. + const running = await isDotcmsRunning(undefined, 1); + if (!running.ok) { + return false; + } - // STEP 3 — Download docker-compose - spinner.start('Downloading Docker Compose configuration...'); - const downloaded = await downloadTheDockerCompose({ - directory: finalDirectory + const probeToken = await DotCMSApi.getAuthToken({ + payload: { + user: DOTCMS_USER.username, + password: DOTCMS_USER.password, + expirationDays: '1', + label: 'create-app reuse probe' + } + }); + + return probeToken.ok; + }, + owner: await describePortOwner(8082, (cmd, args) => execa(cmd, args)), + askAction: (context) => { + spinner.stop(); + + return askPortConflictAction(context); + }, + notify: (message) => spinner.info(message) }); - if (!downloaded.ok) { - spinner.fail('Failed to download Docker Compose file.'); + + if (portOutcome.kind === 'abort') { + spinner.fail('Required ports are busy'); + console.error(chalk.red(portOutcome.message)); process.exit(1); } - spinner.succeed('Docker Compose configuration downloaded'); - // STEP 4 — Run docker-compose - spinner.start('Starting dotCMS containers...'); - const ran = await runDockerCompose({ - directory: finalDirectory, - starterUrl: options.starter - }); - if (!ran.ok) { - spinner.fail('Failed to start Docker containers'); - const errorMessage = ran.val instanceof Error ? ran.val.message : String(ran.val); - console.error( - chalk.red('\n❌ Docker Compose failed to start\n\n') + - chalk.white('Error details:\n') + - chalk.gray(errorMessage) + - '\n\n' + - chalk.yellow('Common solutions:\n') + - chalk.white(' • Ensure Docker Desktop is running\n') + - chalk.white(' • Try: ') + - chalk.cyan('docker compose down') + - chalk.white(' then run this command again\n') + - chalk.white(' • Check Docker logs for more details\n') - ); - process.exit(1); + if (portOutcome.kind === 'replace') { + // The documented recovery for a bricked instance, done here so the user does not + // have to leave the CLI for it. `-v` is the point: keeping the volumes keeps the + // corruption, and the instance comes back just as broken (#37268). + spinner.start(`Removing the existing "${portOutcome.project}" stack...`); + await execa('docker', ['compose', '-p', portOutcome.project, 'down', '-v'], { + reject: false + }); + spinner.succeed(`Removed the existing "${portOutcome.project}" stack`); } - spinner.succeed('dotCMS containers started successfully.'); + const reusingExistingInstance = portOutcome.kind === 'reuse'; + + spinner.succeed( + reusingExistingInstance + ? 'Reusing the dotCMS already running on 8082' + : 'All required ports are available' + ); + + // STEPS 3 & 4 — provision the stack, UNLESS we are reusing one that is already up. + // + // Skipping these is the entire point of the reuse decision. Writing a second compose + // file and running `up` against ports the existing stack already holds fails with + // "port is already allocated" — turning a recoverable situation back into the dead + // end AC-006 exists to remove. + if (!reusingExistingInstance) { + // STEP 3 — Download docker-compose + spinner.start('Downloading Docker Compose configuration...'); + const downloaded = await downloadTheDockerCompose({ + directory: finalDirectory + }); + if (!downloaded.ok) { + spinner.fail('Failed to download Docker Compose file.'); + process.exit(1); + } + spinner.succeed('Docker Compose configuration downloaded'); + + // STEP 4 — Run docker-compose + spinner.start('Starting dotCMS containers...'); + const ran = await runDockerCompose({ + directory: finalDirectory, + starterUrl: options.starter, + onProgress: (message) => { + spinner.text = message; + } + }); + if (!ran.ok) { + spinner.fail('Failed to start Docker containers'); + const errorMessage = + ran.val instanceof Error ? ran.val.message : String(ran.val); + console.error( + chalk.red('\n❌ Docker Compose failed to start\n\n') + + chalk.white('Error details:\n') + + chalk.gray(errorMessage) + + '\n\n' + + chalk.yellow('Common solutions:\n') + + chalk.white(' • Ensure Docker Desktop is running\n') + + chalk.white(' • Try: ') + + chalk.cyan('docker compose down') + + chalk.white(' then run this command again\n') + + chalk.white(' • Check Docker logs for more details\n') + ); + process.exit(1); + } + + spinner.succeed('dotCMS containers started successfully.'); + } spinner.start('Verifying if dotCMS is running...'); - const healthCheckResult = await isDotcmsRunning( - DOTCMS_HEALTH_API, - LOCAL_HEALTH_CHECK_RETRIES - ); + // Prefer /dotmgt/readyz on the management port now that the bundled compose file + // publishes it. `--wait` returning healthy only proves the instance is LIVE — the + // container healthcheck probes livez — and readyz was measured lagging it by a few + // seconds. Probing the app endpoint alone would let the CLI start making API calls + // in that window. The app endpoint stays as the fallback for images that do not + // serve the management endpoints (AC-009, P1 readiness switch). + const readiness = await waitForReadiness({ + readyzUrl: `${LOCAL_MANAGEMENT_HOST}/dotmgt/readyz`, + fallbackUrl: DOTCMS_HEALTH_API, + get: (url) => httpGet(url, { timeoutMs: 10000, acceptAnyStatus: true }), + attempts: LOCAL_HEALTH_CHECK_RETRIES, + delayMs: 5000, + onAttempt: (attempt, attempts, detail) => { + spinner.text = formatRetryReport({ + attempt, + totalAttempts: attempts, + reason: detail, + nextDelayMs: 5000 + }); + } + }); + + const healthCheckResult: Result = + readiness.kind === 'ready' ? Ok(true) : Err(readiness.detail); if (!healthCheckResult.ok) { spinner.fail('dotCMS failed to start properly'); @@ -353,29 +468,36 @@ program spinner.succeed(`Retrieved default site (${defaultSite.val.entity.identifier})`); } - const setUpUVE = await DotCMSApi.setupUVEConfig({ - payload: { - configuration: { - hidden: false, - value: getUVEConfigValue( - `http://localhost:${getPortByFramework(selectedFramework as SupportedFrontEndFrameworks)}` - ) - } - }, + recordRecoverableState({ + host: LOCAL_DOTCMS_HOST, + token: dotcmsToken.val, siteId: defaultSite.val.entity.identifier, - authenticationToken: dotcmsToken.val + projectDirectory: finalDirectory, + framework: selectedFramework }); - if (!setUpUVE.ok) { - spinner.fail('Failed to setup UVE configuration in Dotcms.'); - process.exit(1); - } else { + // Optional step: a failure here must not cost the user the run (contract X2). + const uveOutcome = await configureUVE({ + host: LOCAL_DOTCMS_HOST, + siteId: defaultSite.val.entity.identifier, + token: dotcmsToken.val, + mode: 'local', + frontendUrl: `http://localhost:${getPortByFramework(selectedFramework as SupportedFrontEndFrameworks)}`, + report: (message) => spinner.info(message) + }); + + if (uveOutcome.kind === 'configured') { spinner.succeed(`Configured the Universal Visual Editor`); + } else { + spinner.warn('Skipped Universal Visual Editor configuration.'); + console.log(chalk.yellow(uveOutcome.message)); } - // required since git requires empty directory - moveDockerComposeOneLevelUp(finalDirectory); - await startScaffoldingFrontEnd({ spinner, selectedFramework, finalDirectory }); - moveDockerComposeBack(finalDirectory); + // git needs an empty directory, so the compose file steps aside — inside a + // try/finally, because a scaffolding failure used to strand it in the parent and + // leave the user unable to `docker compose down` the stack still running (AC-008). + await withComposeFileMovedAside(finalDirectory, () => + startScaffoldingFrontEnd({ spinner, selectedFramework, finalDirectory }) + ); console.log(chalk.white(`✅ Project setup complete!`)); const relativePath = getDisplayPath(finalDirectory, process.cwd()); displayFinalSteps({ @@ -452,10 +574,12 @@ async function downloadTheDockerCompose({ async function runDockerCompose({ directory, - starterUrl + starterUrl, + onProgress }: { directory: string; starterUrl?: string; + onProgress?: (message: string) => void; }): Promise> { try { // console.log(chalk.cyan("🐳 Starting Docker containers... (This might take some time)")); @@ -466,14 +590,88 @@ async function runDockerCompose({ const env = starterUrl ? { ...process.env, CUSTOM_STARTER_URL: starterUrl } : process.env; - await execa('docker', ['compose', 'up', '-d'], { cwd: directory, env }); - await execa('docker', ['ps'], { cwd: directory }); + // `--wait` blocks until every service with a healthcheck reports healthy, so the + // success message below is only reached when the stack is genuinely usable. Without + // it, `up -d` returns as soon as the containers are *created* — which is how the CLI + // came to report "containers started successfully" about a dotcms that had already + // exited (issue #37262, AC-002). + // + // The timeout is generous because a cold run pulls ~2GB and then imports the demo + // starter. The bundled compose file sets `start_period: 180s` on dotcms; a boot + // measured at ~46s leaves plenty of head-room inside this budget. + const subprocess = execa( + 'docker', + [ + 'compose', + 'up', + '-d', + '--wait', + '--wait-timeout', + String(COMPOSE_WAIT_TIMEOUT_SECONDS) + ], + { cwd: directory, env } + ); + + // Feedback has to be continuous for the WHOLE wait, which can be ten minutes on a cold + // machine: a ~2GB pull followed by the demo-starter import. Ten minutes of motionless + // spinner is the symptom this issue was actually reported for, so two things run here. + // + // 1. Compose's own progress. It writes `Waiting`/`Healthy` transitions and pull progress + // to STDERR, not stdout, and execa swallows both by default. + // 2. A ticker, because compose can itself go quiet for minutes at a time while a single + // layer downloads or the starter imports. Elapsed time moving is what distinguishes + // "still working" from "hung", and only the ticker can show that during the silence. + let lastLine = 'starting containers'; + const startedAt = Date.now(); + + const absorb = (chunk: Buffer | string) => { + const line = String(chunk) + .split('\n') + .map((part) => part.trim()) + .filter(Boolean) + .pop(); + + if (line) { + lastLine = line; + } + }; + + subprocess.stdout?.on('data', absorb); + subprocess.stderr?.on('data', absorb); + + const ticker = setInterval(() => { + const elapsed = Math.round((Date.now() - startedAt) / 1000); + onProgress?.(`${lastLine} (${elapsed}s elapsed)`); + }, PROGRESS_TICK_MS); - // console.log(chalk.green("✔ Docker containers started successfully!\n")); + try { + await subprocess; + } finally { + clearInterval(ticker); + } return Ok(undefined); } catch (err) { - return Err(err as Error); + // A --wait timeout is otherwise indistinguishable from a hang. Say what state the stack + // reached, so the failure names itself instead of leaving the user to go digging. + const detail = await describeComposeState(directory); + + return Err(new Error(`${(err as Error).message}${detail}`)); + } +} + +/** Per-service state, appended to a compose failure so the error is self-describing. */ +async function describeComposeState(directory: string): Promise { + try { + const { stdout } = await execa( + 'docker', + ['compose', 'ps', '--format', '{{.Service}}: {{.State}} {{.Status}}'], + { cwd: directory } + ); + + return stdout.trim() ? `\n\nContainer state:\n${stdout.trim()}` : ''; + } catch { + return ''; } } @@ -486,29 +684,27 @@ async function updateDockerComposeStarterUrl({ }): Promise { const composePath = path.join(directory, 'docker-compose.yml'); const composeContents = await fs.readFile(composePath, 'utf-8'); - const updatedContents = composeContents.replace( - /^(\s*["']?CUSTOM_STARTER_URL["']?\s*:\s*).+$/m, - `$1"${starterUrl}"` - ); - - if (updatedContents === composeContents) { - throw new Error( - 'CUSTOM_STARTER_URL entry not found in docker-compose.yml. Unable to apply --starter value.' - ); - } + // The string rewrite lives in ./utils/starter-url so a Jest spec can pin it against the + // real bundled asset — it throws when the CUSTOM_STARTER_URL entry is missing. + const updatedContents = applyStarterUrl(composeContents, starterUrl); await fs.writeFile(composePath, updatedContents); } -async function isDotcmsRunning(url?: string, retries = 60): Promise> { +async function isDotcmsRunning( + url?: string, + retries = 60, + onRetry?: RetryReporter +): Promise> { try { // console.log(chalk.cyan("Waiting for DotCMS to be up ....")); - const res = await fetchWithRetry(url ?? DOTCMS_HEALTH_API, retries, 5000); - if (res && res.status === 200) { - // console.log(chalk.green("✔ DotCMS container started sucessfully!\n")); + const res = await fetchWithRetry(url ?? DOTCMS_HEALTH_API, retries, 5000, 10000, onRetry); + // `isSuccessStatus`, not `=== 200`: fetchWithRetry resolves on any 2xx, so demanding + // exactly 200 here rejected responses it had already accepted. + if (res && isSuccessStatus(res.status)) { return Ok(true); } - return Err('dotCMS health check returned non-200 status'); + return Err('dotCMS health check returned a non-success status'); } catch (error) { const errorMessage = error instanceof Error ? error.message : String(error); return Err(errorMessage); @@ -528,13 +724,16 @@ function displayFinalSteps({ siteId: string; host: string; }) { + // Claim the reporting before the summary prints, so the connection details land INSIDE it + // rather than being appended by the exit handler afterwards. + const connection = flushRecoverableState(); + switch (selectedFramework) { case 'nextjs': { finalStepsForNextjs({ projectPath: relativePath, - token: token, - siteId: siteId, - urlDotCMSInstance: host + urlDotCMSInstance: host, + connection }); break; } @@ -559,9 +758,8 @@ function displayFinalSteps({ case 'astro': { finalStepsForAstro({ projectPath: relativePath, - token: token, - siteId: siteId, - urlDotCMSInstance: host + urlDotCMSInstance: host, + connection }); break; } @@ -585,7 +783,15 @@ async function startScaffoldingFrontEnd({ if (!created.ok) { spinner.fail(`Failed to scaffold frontend project (${selectedFramework}).`); - process.exit(1); + // `throw`, NOT process.exit: this runs inside withComposeFileMovedAside, and + // process.exit skips `finally` — which would strand docker-compose.yml in the parent + // directory on the exact failure that `finally` exists to survive (AC-008). The outer + // catch prints the message and still exits 1, so the exit contract is unchanged. + // + // The underlying error, not a copy of the spinner text: the outer catch prints + // whatever it gets, and repeating the line above would just say it twice. This one + // names the likely causes (git missing, no network). + throw created.val; } // TODO need to insert here the dependices step @@ -594,11 +800,14 @@ async function startScaffoldingFrontEnd({ `📦 Installing dependencies...\n\n ${displayDependencies(selectedFramework as SupportedFrontEndFrameworks)}` ); const result = await installDependenciesForProject(finalDirectory); - if (!result) { + // `result.ok`, never `!result`: Err() is `{ok:false, val}` — a truthy object — so the old + // `if (!result)` guard was unreachable and a failed install reported success (contract X7). + const installReport = reportInstallResult(result); + + if (installReport.kind === 'failed') { spinner.fail( - `Failed to install dependencies. Please check if npm is installed in your system` + `Failed to install dependencies (${installReport.reason}). Check that npm is installed and on your PATH.` ); - process.exit(1); } else { spinner.succeed(`Dependencies installed`); } diff --git a/core-web/libs/sdk/create-app/src/packaging.spec.ts b/core-web/libs/sdk/create-app/src/packaging.spec.ts new file mode 100644 index 000000000000..c2e5f89c9105 --- /dev/null +++ b/core-web/libs/sdk/create-app/src/packaging.spec.ts @@ -0,0 +1,178 @@ +import fs from 'fs'; +import path from 'path'; + +/** + * Packaging guard for the bundled Docker compose file (dotCMS issue #37262, AC-013). + * + * The CLI ships its own `assets/docker-compose.yml`. For that file to actually reach + * users it must be declared in BOTH manifests: + * + * 1. package.json -> "files" (what npm publishes) + * 2. project.json -> targets.build.options.assets (what esbuild copies into dist) + * + * Miss either one and the published package has no compose file, so every + * local-Docker run of the CLI fails at its very first step. That is the most + * likely way to break this release, hence a test instead of a code review. + */ + +const PROJECT_ROOT = path.resolve(__dirname, '..'); +const PACKAGE_JSON_PATH = path.join(PROJECT_ROOT, 'package.json'); +const PROJECT_JSON_PATH = path.join(PROJECT_ROOT, 'project.json'); + +/** Path of the compose asset, relative to the package/project root. */ +const COMPOSE_ASSET_PATH = 'assets/docker-compose.yml'; + +/** Project root as spelled inside project.json asset entries (workspace-relative). */ +const WORKSPACE_PROJECT_ROOT = 'libs/sdk/create-app'; + +type ProjectJsonAsset = string | { input?: string; glob?: string; output?: string }; + +function readJson(filePath: string): Record { + return JSON.parse(fs.readFileSync(filePath, 'utf-8')); +} + +/** Normalize a manifest entry: drop `./` prefixes and trailing slashes. */ +function normalize(entry: string): string { + return entry.trim().replace(/^\.\//, '').replace(/\/+$/, ''); +} + +/** Turn a (possibly globbed) path into a matcher, honoring `*` and `**`. */ +function globToRegExp(pattern: string): RegExp { + const GLOBSTAR = '<>'; + const source = pattern + .split('/') + .map((segment) => + segment === '**' + ? GLOBSTAR + : segment.replace(/[.+^${}()|[\]\\?]/g, '\\$&').replace(/\*/g, '[^/]*') + ) + .join('/') + .split(`${GLOBSTAR}/`) + .join('(?:.*/)?') + .split(GLOBSTAR) + .join('.*'); + + return new RegExp(`^${source}$`); +} + +/** + * Would this entry ship `assets/docker-compose.yml`? + * + * Deliberately tolerant of any reasonable spelling: a bare directory (`assets`), + * a trailing slash (`assets/`), a glob (`assets/**`, `assets/*`, `assets/*.yml`) + * or the explicit file path all count. + */ +function shipsComposeAsset(entry: string): boolean { + const normalized = normalize(entry); + + if (!normalized) { + return false; + } + + // A directory entry ships everything under it. + if (COMPOSE_ASSET_PATH === normalized || COMPOSE_ASSET_PATH.startsWith(`${normalized}/`)) { + return true; + } + + return globToRegExp(normalized).test(COMPOSE_ASSET_PATH); +} + +/** Flatten a project.json asset entry (string or `{ input, glob, output }`) to a path. */ +function toAssetPath(asset: ProjectJsonAsset): string { + if (typeof asset === 'string') { + return normalize(asset); + } + + const input = normalize(asset?.input ?? ''); + const glob = normalize(asset?.glob ?? ''); + + return [input, glob].filter(Boolean).join('/'); +} + +/** project.json paths are workspace-relative; make them package-relative. */ +function toProjectRelative(assetPath: string): string { + const normalized = normalize(assetPath); + + return normalized.startsWith(`${WORKSPACE_PROJECT_ROOT}/`) + ? normalized.slice(WORKSPACE_PROJECT_ROOT.length + 1) + : normalized; +} + +function fail(lines: string[]): never { + throw new Error(`\n${lines.join('\n')}\n`); +} + +describe('@dotcms/create-app packaging', () => { + describe('package.json "files"', () => { + it('ships assets/docker-compose.yml to npm', () => { + const pkg = readJson(PACKAGE_JSON_PATH); + const files = pkg['files']; + + if (!Array.isArray(files)) { + fail([ + `${PACKAGE_JSON_PATH} has no "files" array.`, + `Add one that includes "${COMPOSE_ASSET_PATH}" or npm will publish the CLI without the compose file.` + ]); + } + + const entries = files as string[]; + const matches = entries.filter( + (entry) => typeof entry === 'string' && shipsComposeAsset(entry) + ); + + if (matches.length === 0) { + fail([ + 'package.json will NOT publish the bundled Docker compose file.', + ` manifest : ${PACKAGE_JSON_PATH}`, + ` "files" : ${JSON.stringify(entries)}`, + ` missing : an entry covering "${COMPOSE_ASSET_PATH}"`, + '', + ' Fix: add "assets/**" (or "assets", or "assets/docker-compose.yml") to the "files" array.', + ' Without it the published package has no compose file and every local-Docker', + ' run of the CLI fails at its first step. (issue #37262, AC-013)' + ]); + } + + expect(matches.length).toBeGreaterThan(0); + }); + }); + + describe('project.json build assets', () => { + it('copies assets/docker-compose.yml into the build output', () => { + const project = readJson(PROJECT_JSON_PATH); + const assets = ( + project as { + targets?: { build?: { options?: { assets?: ProjectJsonAsset[] } } }; + } + ).targets?.build?.options?.assets; + + if (!Array.isArray(assets)) { + fail([ + `${PROJECT_JSON_PATH} has no targets.build.options.assets array.`, + `Add one that copies "${COMPOSE_ASSET_PATH}" into the build output.` + ]); + } + + const resolved = assets.map((asset) => toProjectRelative(toAssetPath(asset))); + const matches = resolved.filter(shipsComposeAsset); + + if (matches.length === 0) { + fail([ + 'project.json will NOT copy the bundled Docker compose file into dist.', + ` manifest : ${PROJECT_JSON_PATH}`, + ` assets : ${JSON.stringify(assets)}`, + ` resolved : ${JSON.stringify(resolved)}`, + ` missing : an entry covering "${COMPOSE_ASSET_PATH}"`, + '', + ' Fix: add to targets.build.options.assets either the string', + ` "${WORKSPACE_PROJECT_ROOT}/${COMPOSE_ASSET_PATH}" or the object`, + ` { "input": "${WORKSPACE_PROJECT_ROOT}/assets", "glob": "**/*", "output": "assets" }.`, + ' Without it the compose file never lands in dist, so the published package', + ' ships without it and every local-Docker run fails. (issue #37262, AC-013)' + ]); + } + + expect(matches.length).toBeGreaterThan(0); + }); + }); +}); diff --git a/core-web/libs/sdk/create-app/src/utils/compose-move.spec.ts b/core-web/libs/sdk/create-app/src/utils/compose-move.spec.ts new file mode 100644 index 000000000000..b34239581753 --- /dev/null +++ b/core-web/libs/sdk/create-app/src/utils/compose-move.spec.ts @@ -0,0 +1,148 @@ +/** + * Contract spec for the compose-file round trip around scaffolding + * (task T033, dotCMS #37262, AC-008). + * + * THE BUG. `docker-compose.yml` is moved OUT of the project before scaffolding, because git + * requires an empty directory to clone into, then moved BACK afterwards. If scaffolding fails in + * between, the move-back never runs and the compose file is stranded outside the project — so + * the user cannot even `docker compose down` the stack that is still running. The recovery path + * is destroyed by the failure it is meant to survive, which is the shape of this whole issue. + * + * The fix is `try/finally`: the file comes back whether scaffolding succeeds or throws, and the + * original error still propagates — a `finally` that swallows the cause would trade one silent + * failure for another. + * + * TWO CONSTRAINTS THIS SPEC EXISTS TO HOLD, both regressions found in review: + * + * 1. The holding spot is a private temp dir, never the parent. The parent is the user's cwd, + * and moving there with `overwrite` destroyed a compose file they already had. + * 2. `action` must signal failure by THROWING. `process.exit` skips `finally`, so a caller + * that exits instead of throwing silently reintroduces the stranded-file bug — which is + * exactly what `startScaffoldingFrontEnd` did (see src/index.ts). + * + * API PINNED + * export async function withComposeFileMovedAside( + * directory: string, + * action: () => Promise + * ): Promise; + */ + +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; + +import { withComposeFileMovedAside } from './compose-move'; + +const COMPOSE = 'docker-compose.yml'; +const CONTENTS = 'services:\n dotcms:\n image: dotcms/dotcms:latest\n'; + +describe('withComposeFileMovedAside', () => { + let parentDir: string; + let projectDir: string; + + beforeEach(() => { + parentDir = fs.mkdtempSync(path.join(os.tmpdir(), 'dotcms-compose-move-')); + projectDir = path.join(parentDir, 'my-app'); + fs.mkdirSync(projectDir); + fs.writeFileSync(path.join(projectDir, COMPOSE), CONTENTS, 'utf8'); + }); + + afterEach(() => { + fs.rmSync(parentDir, { recursive: true, force: true }); + }); + + const inProject = () => fs.existsSync(path.join(projectDir, COMPOSE)); + const inParent = () => fs.existsSync(path.join(parentDir, COMPOSE)); + + it('moves the file aside for the action and restores it on success', async () => { + let sawEmptyProjectDir = false; + + const result = await withComposeFileMovedAside(projectDir, async () => { + // git needs the directory empty — this is the whole reason for the dance. + sawEmptyProjectDir = !inProject(); + + return 'scaffolded'; + }); + + expect(sawEmptyProjectDir).toBe(true); + expect(result).toBe('scaffolded'); + expect(inProject()).toBe(true); + expect(inParent()).toBe(false); + }); + + it('restores the file when the action THROWS — the bug being fixed', async () => { + await expect( + withComposeFileMovedAside(projectDir, async () => { + throw new Error('scaffolding failed'); + }) + ).rejects.toThrow('scaffolding failed'); + + expect(inProject()).toBe(true); + }); + + it('leaves no orphan in the parent directory after a failure', async () => { + await expect( + withComposeFileMovedAside(projectDir, async () => { + throw new Error('scaffolding failed'); + }) + ).rejects.toThrow(); + + // The stranded copy is what made the old failure unrecoverable: `docker compose down` + // needs this file to be where the user is standing. + expect(inParent()).toBe(false); + }); + + it('propagates the original error rather than swallowing it in the finally', async () => { + const cause = new Error('framework template not found'); + + await expect( + withComposeFileMovedAside(projectDir, async () => { + throw cause; + }) + ).rejects.toBe(cause); + }); + + it('does nothing surprising when there is no compose file to move', async () => { + fs.rmSync(path.join(projectDir, COMPOSE)); + + const result = await withComposeFileMovedAside(projectDir, async () => 'ok'); + + expect(result).toBe('ok'); + expect(inParent()).toBe(false); + }); + + it('preserves the file contents across the round trip', async () => { + await withComposeFileMovedAside(projectDir, async () => undefined); + + expect(fs.readFileSync(path.join(projectDir, COMPOSE), 'utf8')).toBe(CONTENTS); + }); + + // The parent directory is the user's cwd. The first implementation moved our compose file + // there with `overwrite: true`, so a `docker-compose.yml` the user already had was silently + // destroyed — and the `finally` then moved OUR file into the project, leaving no copy of + // theirs anywhere. Scaffolding into a directory that already runs its own compose stack is + // an ordinary thing to do, so this is data loss on a normal path. + it('does not touch a docker-compose.yml the user already has in the parent directory', async () => { + const theirs = 'services:\n their-own-app:\n image: nginx\n'; + fs.writeFileSync(path.join(parentDir, COMPOSE), theirs, 'utf8'); + + await withComposeFileMovedAside(projectDir, async () => 'scaffolded'); + + expect(fs.readFileSync(path.join(parentDir, COMPOSE), 'utf8')).toBe(theirs); + expect(fs.readFileSync(path.join(projectDir, COMPOSE), 'utf8')).toBe(CONTENTS); + }); + + it('leaves the parent compose file alone even when the action fails', async () => { + const theirs = 'services:\n their-own-app:\n image: nginx\n'; + fs.writeFileSync(path.join(parentDir, COMPOSE), theirs, 'utf8'); + + await expect( + withComposeFileMovedAside(projectDir, async () => { + throw new Error('scaffolding failed'); + }) + ).rejects.toThrow('scaffolding failed'); + + expect(fs.readFileSync(path.join(parentDir, COMPOSE), 'utf8')).toBe(theirs); + expect(fs.readFileSync(path.join(projectDir, COMPOSE), 'utf8')).toBe(CONTENTS); + }); +}); diff --git a/core-web/libs/sdk/create-app/src/utils/compose-move.ts b/core-web/libs/sdk/create-app/src/utils/compose-move.ts new file mode 100644 index 000000000000..4079153183b7 --- /dev/null +++ b/core-web/libs/sdk/create-app/src/utils/compose-move.ts @@ -0,0 +1,48 @@ +import fs from 'fs-extra'; + +import os from 'node:os'; +import path from 'node:path'; + +const COMPOSE_FILE = 'docker-compose.yml'; + +/** + * Runs `action` with `docker-compose.yml` moved out of `directory`, then puts it back. + * + * The move is required because git clones into an empty directory. The `finally` is required + * because without it a scaffolding failure strands the compose file outside the project — + * leaving the user unable to `docker compose down` the stack that is still running. The failure + * destroyed the recovery path (AC-008). + * + * The holding spot is a private temp directory, NOT the parent. The parent is the user's cwd: + * moving there with `overwrite` silently destroyed a `docker-compose.yml` the user already had, + * and the `finally` then moved our file into the project, so the original was unrecoverable. + * A fresh `mkdtemp` per call also means concurrent runs cannot collide. + * + * The original error is deliberately allowed to propagate: a `finally` that swallowed it would + * replace one silent failure with another. + */ +export async function withComposeFileMovedAside( + directory: string, + action: () => Promise +): Promise { + const inProject = path.join(directory, COMPOSE_FILE); + const moved = fs.existsSync(inProject); + + if (!moved) { + return await action(); + } + + const holdingDir = await fs.mkdtemp(path.join(os.tmpdir(), 'dotcms-create-app-compose-')); + const asideNext = path.join(holdingDir, COMPOSE_FILE); + + await fs.move(inProject, asideNext); + + try { + return await action(); + } finally { + if (fs.existsSync(asideNext)) { + await fs.move(asideNext, inProject, { overwrite: true }); + } + await fs.remove(holdingDir); + } +} diff --git a/core-web/libs/sdk/create-app/src/utils/fetch-retry.spec.ts b/core-web/libs/sdk/create-app/src/utils/fetch-retry.spec.ts new file mode 100644 index 000000000000..021601107d71 --- /dev/null +++ b/core-web/libs/sdk/create-app/src/utils/fetch-retry.spec.ts @@ -0,0 +1,116 @@ +/** + * Contract spec for retry reporting (tasks T040/T042, dotCMS #37262, AC-009). + * + * THE BUG. `fetchWithRetry` calls `console.log` directly between attempts + * (`src/utils/index.ts`, in the catch block) while an `ora` spinner is running. A spinner owns + * the last terminal line and repaints it; anything else writing to stdout at the same time gets + * shredded. That is the mangled block in the original report: + * + * ✔ dotCMS containers started successfully. + * ⏳ dotCMS not ready (attempt 1/60) - ECONNRESET - Retrying in 5s... + * + * The fix is not to silence the retries — the user needs them, a ten-minute wait with no output + * is the very thing being fixed — but to hand them to the caller, who owns the spinner and can + * render them without fighting it. + * + * API PINNED + * export interface RetryReport { + * attempt: number; // 1-based + * totalAttempts: number; + * reason: string; // human-readable cause of THIS attempt's failure + * nextDelayMs: number; + * } + * export type RetryReporter = (report: RetryReport) => void; + * export function formatRetryReport(report: RetryReport): string; + * export function describeRequestFailure(error: unknown): string; + * + * `fetchWithRetry` takes an optional `onRetry: RetryReporter`. When omitted it stays silent — + * silence is the correct default for a library function; the CLI supplies a reporter that routes + * through its spinner. + * + * ALSO PINNED HERE — the status contract. `fetchWithRetry` accepts any 2xx + * (`validateStatus: status >= 200 && status < 300`) but `isDotcmsRunning` then demands exactly + * `res.status === 200`. A 204 is therefore success to one and failure to the other. One rule: + * any 2xx is success. + */ + +import { describeRequestFailure, formatRetryReport, isSuccessStatus } from './fetch-retry'; +import { HttpError } from './http'; + +describe('describeRequestFailure', () => { + // The CLI no longer uses axios (see utils/http.ts), so failures arrive as HttpError. + const httpError = (init: { status?: number | null; code?: string; statusText?: string }) => + new HttpError('request failed', init); + + it('names a refused connection in words the user can act on', () => { + expect(describeRequestFailure(httpError({ status: null, code: 'ECONNREFUSED' }))).toMatch( + /refus/i + ); + }); + + it('names a timeout', () => { + expect(describeRequestFailure(httpError({ status: null, code: 'ETIMEDOUT' }))).toMatch( + /timeout/i + ); + }); + + it('reports an HTTP status when the server did answer', () => { + const described = describeRequestFailure( + httpError({ status: 503, statusText: 'Service Unavailable' }) + ); + + expect(described).toContain('503'); + }); + + it('falls back to something printable for a non-axios failure', () => { + expect(describeRequestFailure(new Error('boom'))).toContain('boom'); + expect(typeof describeRequestFailure('plain string')).toBe('string'); + }); +}); + +describe('formatRetryReport', () => { + const report = { + attempt: 3, + totalAttempts: 60, + reason: 'Connection refused', + nextDelayMs: 5000 + }; + + it('reports progress through the budget, so a long wait is legible', () => { + const line = formatRetryReport(report); + + expect(line).toContain('3'); + expect(line).toContain('60'); + }); + + it('includes the reason and the next delay', () => { + const line = formatRetryReport(report); + + expect(line).toContain('Connection refused'); + expect(line).toMatch(/5\s*s/); + }); + + it('returns a single line — a spinner repaints one line, so a multi-line report tears', () => { + expect(formatRetryReport(report)).not.toContain('\n'); + }); +}); + +describe('isSuccessStatus — one rule for both callers', () => { + it.each([200, 201, 202, 204, 299])('treats %i as success', (status) => { + expect(isSuccessStatus(status)).toBe(true); + }); + + it.each([199, 300, 400, 403, 500, 503])('treats %i as failure', (status) => { + expect(isSuccessStatus(status)).toBe(false); + }); + + /** + * The mismatch this exists to close: `fetchWithRetry` resolved on any 2xx, then + * `isDotcmsRunning` rejected anything that was not exactly 200. A 204 slipped through the + * first and was refused by the second, so the CLI could report a healthy instance as + * unreachable. + */ + it('accepts 204, which the old `status === 200` check rejected', () => { + expect(isSuccessStatus(204)).toBe(true); + }); +}); diff --git a/core-web/libs/sdk/create-app/src/utils/fetch-retry.ts b/core-web/libs/sdk/create-app/src/utils/fetch-retry.ts new file mode 100644 index 000000000000..ad819acab914 --- /dev/null +++ b/core-web/libs/sdk/create-app/src/utils/fetch-retry.ts @@ -0,0 +1,68 @@ +import { isHttpError } from './http'; + +/** + * Retry reporting, split out so it can be rendered by whoever owns the terminal. + * + * `fetchWithRetry` used to `console.log` between attempts while an `ora` spinner was running. + * A spinner owns and repaints the last line, so concurrent writes tear — which is exactly the + * mangled retry block in the report for #37262. Handing the caller a structured report lets the + * CLI route it through the spinner instead of fighting it, without going silent: a ten-minute + * wait with no output is the symptom being fixed, not the fix. + */ + +export interface RetryReport { + /** 1-based, so it reads the way it prints. */ + attempt: number; + totalAttempts: number; + reason: string; + nextDelayMs: number; +} + +export type RetryReporter = (report: RetryReport) => void; + +/** + * One rule for what counts as a successful response. + * + * `fetchWithRetry` accepted any 2xx while `isDotcmsRunning` demanded exactly 200, so a 204 was + * success to one and failure to the other and a healthy instance could be reported unreachable. + */ +export function isSuccessStatus(status: number): boolean { + return status >= 200 && status < 300; +} + +/** Turns a request failure into something worth showing a user mid-wait. */ +export function describeRequestFailure(error: unknown): string { + if (isHttpError(error)) { + if (error.code === 'ECONNREFUSED') { + return 'Connection refused - service not accepting connections yet'; + } + + if (error.code === 'ETIMEDOUT') { + return 'Connection timeout - service too slow or not responding'; + } + + if (error.response) { + return `HTTP ${error.response.status}: ${error.response.statusText}`; + } + + return error.code || error.message; + } + + if (error instanceof Error) { + return error.message; + } + + return String(error); +} + +/** + * A single line: a spinner repaints one line, and a multi-line report tears across the repaint. + */ +export function formatRetryReport({ + attempt, + totalAttempts, + reason, + nextDelayMs +}: RetryReport): string { + return `dotCMS not ready (attempt ${attempt}/${totalAttempts}) - ${reason} - retrying in ${Math.round(nextDelayMs / 1000)}s`; +} diff --git a/core-web/libs/sdk/create-app/src/utils/http.spec.ts b/core-web/libs/sdk/create-app/src/utils/http.spec.ts new file mode 100644 index 000000000000..d2564f29608e --- /dev/null +++ b/core-web/libs/sdk/create-app/src/utils/http.spec.ts @@ -0,0 +1,174 @@ +/** + * Contract spec for `src/utils/http.ts` — the CLI's only HTTP client. + * + * WHY THIS EXISTS. `semgrep-dotcms` flagged axios on this PR with two High findings, both the + * same root cause: axios's Node adapter does not clear `Proxy-Authorization` when a request that + * went through an authenticated proxy is redirected to a target that does not use that proxy, so + * the proxy credentials leak to the redirect origin. + * + * Bumping axios would close those two CVEs. Removing it closes the class: this package was the + * only lib in the workspace depending on axios, it is in esbuild's `external` list — so it is a + * real install for anyone running `npx @dotcms/create-app` — and `compose-source.ts` in this same + * package already used native `fetch`. Node >= 22.22.3 is required here (`.nvmrc`), where `fetch` + * is stable. + * + * The fetch spec requires stripping `Authorization` on a cross-origin redirect, which is the + * protection axios's Node adapter was missing. + * + * API PINNED + * export interface HttpResponse { status: number; data: T } + * export class HttpError extends Error { + * status: number | null; // null when the request never got a response + * code?: string; // ECONNREFUSED, ETIMEDOUT, ... + * response?: { status: number; statusText: string }; + * } + * export function isHttpError(e: unknown): e is HttpError; + * export function httpGet(url, opts?): Promise>; + * export function httpPost(url, body, opts?): Promise>; + * + * opts: { token?: string; timeoutMs?: number; acceptAnyStatus?: boolean } + * + * Throw-on-non-2xx is the default because that is what every existing call site expects; + * `acceptAnyStatus` is the readiness probe's case, where a 503 is data rather than a failure. + */ + +import { httpGet, httpPost, HttpError, isHttpError } from './http'; + +const URL_OK = 'http://localhost:8082/api/v1/thing'; + +function jsonResponse(status: number, body: unknown, statusText = 'OK') { + return new Response(JSON.stringify(body), { + status, + statusText, + headers: { 'Content-Type': 'application/json' } + }); +} + +describe('http', () => { + let fetchSpy: jest.SpyInstance; + + beforeEach(() => { + fetchSpy = jest.spyOn(globalThis, 'fetch'); + }); + + afterEach(() => { + jest.restoreAllMocks(); + }); + + describe('httpGet', () => { + it('returns status and parsed body on success', async () => { + fetchSpy.mockResolvedValue(jsonResponse(200, { entity: 'ok' })); + + await expect(httpGet(URL_OK)).resolves.toEqual({ status: 200, data: { entity: 'ok' } }); + }); + + it('sends the bearer token when given one', async () => { + fetchSpy.mockResolvedValue(jsonResponse(200, {})); + + await httpGet(URL_OK, { token: 'abc123' }); + + const [, init] = fetchSpy.mock.calls[0]; + expect(new Headers(init.headers).get('authorization')).toBe('Bearer abc123'); + }); + + it('sends no Authorization header when no token is given', async () => { + fetchSpy.mockResolvedValue(jsonResponse(200, {})); + + await httpGet(URL_OK); + + const [, init] = fetchSpy.mock.calls[0]; + expect(new Headers(init.headers).has('authorization')).toBe(false); + }); + + it('throws an HttpError carrying the status on a non-2xx', async () => { + fetchSpy.mockResolvedValue(jsonResponse(403, { message: 'forbidden' }, 'Forbidden')); + + const err = await httpGet(URL_OK).catch((e) => e); + + expect(isHttpError(err)).toBe(true); + expect(err.status).toBe(403); + // `response.status` is the shape configureUVE's statusOf() reads. + expect(err.response?.status).toBe(403); + }); + + it.each([200, 201, 204, 299])('treats %i as success', async (status) => { + fetchSpy.mockResolvedValue(new Response(null, { status })); + + await expect(httpGet(URL_OK)).resolves.toMatchObject({ status }); + }); + + it('does not throw on a non-2xx when acceptAnyStatus is set', async () => { + fetchSpy.mockResolvedValue(jsonResponse(503, {}, 'Service Unavailable')); + + // The readiness probe's case: a 503 means "still starting", not "request failed". + await expect(httpGet(URL_OK, { acceptAnyStatus: true })).resolves.toMatchObject({ + status: 503 + }); + }); + + it('surfaces a transport failure as an HttpError with a null status', async () => { + fetchSpy.mockRejectedValue( + Object.assign(new TypeError('fetch failed'), { cause: { code: 'ECONNREFUSED' } }) + ); + + const err = await httpGet(URL_OK).catch((e) => e); + + expect(isHttpError(err)).toBe(true); + expect(err.status).toBeNull(); + expect(err.code).toBe('ECONNREFUSED'); + }); + + it('times out rather than hanging, and reports it as a timeout', async () => { + fetchSpy.mockImplementation( + (_url: string, init: RequestInit) => + new Promise((_resolve, reject) => { + init.signal?.addEventListener('abort', () => + reject(Object.assign(new Error('aborted'), { name: 'AbortError' })) + ); + }) + ); + + const err = await httpGet(URL_OK, { timeoutMs: 20 }).catch((e) => e); + + expect(isHttpError(err)).toBe(true); + expect(err.code).toBe('ETIMEDOUT'); + }); + + it('tolerates a body that is not JSON', async () => { + fetchSpy.mockResolvedValue(new Response('plain text', { status: 200 })); + + await expect(httpGet(URL_OK)).resolves.toMatchObject({ status: 200 }); + }); + }); + + describe('httpPost', () => { + it('sends a JSON body and the content type', async () => { + fetchSpy.mockResolvedValue(jsonResponse(200, { entity: 'Ok' })); + + await httpPost(URL_OK, { hello: 'world' }, { token: 'abc123' }); + + const [, init] = fetchSpy.mock.calls[0]; + expect(init.method).toBe('POST'); + expect(init.body).toBe(JSON.stringify({ hello: 'world' })); + expect(new Headers(init.headers).get('content-type')).toContain('application/json'); + }); + + it('throws an HttpError on a non-2xx', async () => { + fetchSpy.mockResolvedValue(jsonResponse(500, {}, 'Server Error')); + + const err = await httpPost(URL_OK, {}).catch((e) => e); + + expect(isHttpError(err)).toBe(true); + expect(err.status).toBe(500); + }); + }); + + describe('isHttpError', () => { + it('recognises its own errors and nothing else', () => { + expect(isHttpError(new HttpError('x', { status: 404 }))).toBe(true); + expect(isHttpError(new Error('plain'))).toBe(false); + expect(isHttpError('a string')).toBe(false); + expect(isHttpError(null)).toBe(false); + }); + }); +}); diff --git a/core-web/libs/sdk/create-app/src/utils/http.ts b/core-web/libs/sdk/create-app/src/utils/http.ts new file mode 100644 index 000000000000..286de2b758f0 --- /dev/null +++ b/core-web/libs/sdk/create-app/src/utils/http.ts @@ -0,0 +1,142 @@ +/** + * The CLI's only HTTP client, built on Node's native `fetch`. + * + * This package used axios, which `semgrep-dotcms` flagged with two High findings on #37264 — + * both the same root cause: axios's Node adapter does not clear `Proxy-Authorization` when a + * request that went through an authenticated proxy is redirected to a target that does not use + * that proxy, leaking the proxy credentials to the redirect origin. + * + * Bumping axios closes those two CVEs; removing it closes the class. This package was the only + * lib in the workspace depending on axios, and it sits in esbuild's `external` list — so it is a + * real install for everyone running `npx @dotcms/create-app`, not just a build-time concern. + * `compose-source.ts` here already used native `fetch`, so the inconsistency was ours. + * + * Node >= 22.22.3 is required (`.nvmrc`), where `fetch` is stable. The fetch spec requires + * stripping `Authorization` on a cross-origin redirect — the protection axios's Node adapter + * was missing. + */ + +export interface HttpResponse { + status: number; + data: T; +} + +export interface HttpOptions { + /** Sent as `Authorization: Bearer `. */ + token?: string; + timeoutMs?: number; + /** + * Return non-2xx responses instead of throwing. The readiness probe needs this: a 503 from + * `/dotmgt/readyz` means "still starting", which is data, not a failed request. + */ + acceptAnyStatus?: boolean; +} + +export class HttpError extends Error { + /** HTTP status, or null when the request never got a response at all. */ + readonly status: number | null; + /** Transport-level code — ECONNREFUSED, ETIMEDOUT — when there was no response. */ + readonly code?: string; + /** Kept in axios's shape so existing `error.response.status` readers keep working. */ + readonly response?: { status: number; statusText: string }; + + constructor( + message: string, + init: { status?: number | null; code?: string; statusText?: string } + ) { + super(message); + this.name = 'HttpError'; + this.status = init.status ?? null; + this.code = init.code; + + if (typeof init.status === 'number') { + this.response = { status: init.status, statusText: init.statusText ?? '' }; + } + } +} + +export function isHttpError(error: unknown): error is HttpError { + return error instanceof HttpError; +} + +const DEFAULT_TIMEOUT_MS = 10000; + +function isSuccess(status: number): boolean { + return status >= 200 && status < 300; +} + +/** Best-effort JSON. A health endpoint may answer 204, or plain text; neither is an error. */ +async function readBody(response: Response): Promise { + const text = await response.text().catch(() => ''); + + if (!text) { + return undefined as T; + } + + try { + return JSON.parse(text) as T; + } catch { + return text as unknown as T; + } +} + +async function request( + url: string, + init: RequestInit, + { token, timeoutMs = DEFAULT_TIMEOUT_MS, acceptAnyStatus = false }: HttpOptions +): Promise> { + // fetch has no timeout of its own; without this a dead instance hangs the CLI. + const controller = new AbortController(); + const timer = setTimeout(() => controller.abort(), timeoutMs); + + const headers = new Headers(init.headers); + + if (token) { + headers.set('Authorization', `Bearer ${token}`); + } + + let response: Response; + + try { + response = await fetch(url, { ...init, headers, signal: controller.signal }); + } catch (error) { + const aborted = (error as Error)?.name === 'AbortError'; + const cause = (error as { cause?: { code?: string } })?.cause; + + throw new HttpError( + aborted + ? `Request to ${url} timed out after ${timeoutMs}ms` + : `Request to ${url} failed: ${(error as Error)?.message ?? String(error)}`, + { status: null, code: aborted ? 'ETIMEDOUT' : cause?.code } + ); + } finally { + clearTimeout(timer); + } + + const data = await readBody(response); + + if (!isSuccess(response.status) && !acceptAnyStatus) { + throw new HttpError(`Request failed with status code ${response.status}`, { + status: response.status, + statusText: response.statusText + }); + } + + return { status: response.status, data }; +} + +export function httpGet(url: string, options: HttpOptions = {}) { + return request(url, { method: 'GET' }, options); +} + +export function httpPost(url: string, body: unknown, options: HttpOptions = {}) { + return request( + url, + { + method: 'POST', + body: JSON.stringify(body), + headers: { 'Content-Type': 'application/json' } + }, + options + ); +} diff --git a/core-web/libs/sdk/create-app/src/utils/index.ts b/core-web/libs/sdk/create-app/src/utils/index.ts index e2955be7d806..c154169d2e4b 100644 --- a/core-web/libs/sdk/create-app/src/utils/index.ts +++ b/core-web/libs/sdk/create-app/src/utils/index.ts @@ -1,12 +1,12 @@ -import axios from 'axios'; import chalk from 'chalk'; import { execa } from 'execa'; -import fs from 'fs-extra'; -import https from 'https'; import net from 'net'; import path from 'path'; +import { describeRequestFailure, type RetryReporter } from './fetch-retry'; +import { httpGet, isHttpError } from './http'; +import { REQUIRED_PORTS } from './ports'; import { escapeShellPath } from './validation'; import { @@ -30,7 +30,7 @@ import type { SupportedFrontEndFrameworks } from '../types'; * @param retries - Number of retry attempts (default: 5) * @param delay - Delay between retries in milliseconds (default: 5000) * @param requestTimeout - Per-request timeout in milliseconds (default: 10000) - * @returns Promise resolving to axios response + * @returns Promise resolving to the HTTP response * @throws Error with detailed failure information after all retries exhausted * * @remarks @@ -42,46 +42,35 @@ export async function fetchWithRetry( url: string, retries = 5, delay = 5000, - requestTimeout = 10000 // Per-request timeout in milliseconds + requestTimeout = 10000, // Per-request timeout in milliseconds + /** + * Where retry progress goes. Omitted means silent: this function must not write to stdout + * itself, because the caller usually has an `ora` spinner repainting the last line and + * concurrent writes tear it (AC-009). + */ + onRetry?: RetryReporter ) { const errors: string[] = []; let lastError: unknown; for (let i = 0; i < retries; i++) { try { - return await axios.get(url, { - timeout: requestTimeout, - // Accept any 2xx status code as success (health endpoints may return 200, 201, 204, etc.) - validateStatus: (status) => status >= 200 && status < 300 - }); + // Any 2xx is success — the same rule isDotcmsRunning applies, so a 204 cannot be + // accepted here and rejected there. httpGet throws on anything else. + return await httpGet(url, { timeoutMs: requestTimeout }); } catch (err) { lastError = err; - // Track error for debugging with more context - let errorMsg = ''; - if (axios.isAxiosError(err)) { - if (err.code === 'ECONNREFUSED') { - errorMsg = 'Connection refused - service not accepting connections'; - } else if (err.code === 'ETIMEDOUT' || err.code === 'ECONNABORTED') { - errorMsg = 'Connection timeout - service too slow or not responding'; - } else if (err.response) { - errorMsg = `HTTP ${err.response.status}: ${err.response.statusText}`; - } else { - errorMsg = err.code || err.message; - } - } else { - errorMsg = String(err); - } + const errorMsg = describeRequestFailure(err); errors.push(`Attempt ${i + 1}: ${errorMsg}`); if (i === retries - 1) { // Last attempt failed - provide comprehensive error const errorType = - axios.isAxiosError(lastError) && lastError.code === 'ECONNREFUSED' + isHttpError(lastError) && lastError.code === 'ECONNREFUSED' ? 'Connection Refused' - : axios.isAxiosError(lastError) && - (lastError.code === 'ETIMEDOUT' || lastError.code === 'ECONNABORTED') + : isHttpError(lastError) && lastError.code === 'ETIMEDOUT' ? 'Timeout' : 'Connection Failed'; @@ -108,11 +97,13 @@ export async function fetchWithRetry( ); } - console.log( - chalk.yellow(`⏳ dotCMS not ready (attempt ${i + 1}/${retries})`) + - chalk.gray(` - ${errorMsg}`) + - chalk.gray(` - Retrying in ${delay / 1000}s...`) - ); + // Reported, never printed — see the onRetry doc above. + onRetry?.({ + attempt: i + 1, + totalAttempts: retries, + reason: errorMsg, + nextDelayMs: delay + }); await new Promise((r) => setTimeout(r, delay)); } } @@ -156,69 +147,84 @@ export function getDotcmsApisByBaseUrl(baseUrl: string) { }; } -/** Utility to download a file using https */ -export function downloadFile(url: string, dest: string): Promise { - return new Promise((resolve, reject) => { - const file = fs.createWriteStream(dest); +/** + * The connection details, rendered inside the Next Steps block. + * + * This used to be printed by the exit handler, which necessarily runs last — so a successful + * run showed its details after the summary that was supposed to contain them. It belongs here, + * where the reader is already looking. + */ +export function renderConnectionSummary(report: { + wroteEnv: boolean; + filename: string | null; + host: string; + siteId: string; + contents: string; +}) { + if (report.wroteEnv && report.filename) { + console.log( + chalk.green(` ✔ Your dotCMS credentials are already in ${report.filename}\n`) + + chalk.gray(` host : ${report.host}\n`) + + chalk.gray(` site id : ${report.siteId}\n`) + ); - https - .get(url, (response) => { - if (response.statusCode !== 200) { - return reject(new Error(`Failed to download file: ${response.statusCode}`)); - } + return; + } - response.pipe(file); - file.on('finish', () => file.close(() => resolve())); - }) - .on('error', (err) => { - fs.unlink(dest); - reject(err); - }); - }); + // No file was written — the framework has none, one already exists, or the write failed. + console.log( + chalk.white( + report.filename + ? ` Add these to your ${report.filename}:\n` + : ' Configuration for your project:\n' + ) + + chalk.gray( + report.contents + .trimEnd() + .split('\n') + .map((l) => ` ${l}`) + .join('\n') + ) + + '\n' + ); } + export function finalStepsForNextjs({ projectPath, urlDotCMSInstance, - siteId, - token + connection }: { projectPath: string; urlDotCMSInstance: string; - siteId: string; - token: string; + connection?: Parameters[0] | null; }) { console.log('\n'); console.log(chalk.white('━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━\n')); console.log(chalk.greenBright('📋 Next Steps:\n')); + if (connection) { + renderConnectionSummary(connection); + } + console.log( chalk.white('1. Navigate to your project:\n') + chalk.gray(` $ cd ${escapeShellPath(projectPath)}\n`) ); + // No "create a .env and paste this" step: the CLI writes the file itself, and the exit + // handler confirms it. Telling the user to do it as well duplicated the token into + // scrollback and asked them to redo work that was already done. console.log( - chalk.white('2. Create your environment file:\n') + chalk.gray(' $ touch .env\n') - ); - - console.log(chalk.white('3. Add your dotCMS configuration to ') + chalk.green('.env') + ':\n'); - - console.log(chalk.white('──────────────────────────────────────────────\n')); - console.log(chalk.white(getEnvVariablesForNextJS(urlDotCMSInstance, siteId, token))); - console.log(chalk.white('\n──────────────────────────────────────────────\n')); - - console.log(chalk.gray(' 💡 Tip: Copy the block above and paste into your .env file\n')); - - console.log( - chalk.white('4. Start your development server:\n') + chalk.gray(' $ npm run dev\n') + chalk.white('2. Start your development server:\n') + chalk.gray(' $ npm run dev\n') ); console.log( - chalk.white('5. Open your browser:\n') + chalk.gray(' → http://localhost:3000\n') + chalk.white('3. Open your browser:\n') + chalk.gray(' → http://localhost:3000\n') ); console.log( - chalk.white('6. Edit your page content in dotCMS:\n') + + chalk.white('4. Edit your page content in dotCMS:\n') + chalk.gray(` → ${urlDotCMSInstance}/dotAdmin/#/edit-page?url=/index\n`) ); @@ -232,46 +238,37 @@ export function finalStepsForNextjs({ export function finalStepsForAstro({ projectPath, urlDotCMSInstance, - siteId, - token + connection }: { projectPath: string; urlDotCMSInstance: string; - siteId: string; - token: string; + connection?: Parameters[0] | null; }) { console.log('\n'); console.log(chalk.white('━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━\n')); console.log(chalk.greenBright('📋 Next Steps:\n')); + if (connection) { + renderConnectionSummary(connection); + } + console.log( chalk.white('1. Navigate to your project:\n') + chalk.gray(` $ cd ${escapeShellPath(projectPath)}\n`) ); + // The CLI writes the env file itself; see finalStepsForNextjs for why the paste step went. console.log( - chalk.white('2. Create your environment file:\n') + chalk.gray(' $ touch .env\n') + chalk.white('2. Start your development server:\n') + chalk.gray(' $ npm run dev\n') ); - console.log(chalk.white('3. Add your dotCMS configuration to ') + chalk.green('.env') + ':\n'); - - console.log(chalk.white('──────────────────────────────────────────────\n')); - console.log(chalk.white(getEnvVariablesForAstro(urlDotCMSInstance, siteId, token))); - console.log(chalk.white('\n──────────────────────────────────────────────\n')); - - console.log(chalk.gray(' 💡 Tip: Copy the block above and paste into your .env file\n')); - console.log( - chalk.white('4. Start your development server:\n') + chalk.gray(' $ npm run dev\n') + chalk.white('3. Open your browser:\n') + chalk.gray(' → http://localhost:3000\n') ); console.log( - chalk.white('5. Open your browser:\n') + chalk.gray(' → http://localhost:3000\n') - ); - - console.log( - chalk.white('6. Edit your page content in dotCMS:\n') + + chalk.white('4. Edit your page content in dotCMS:\n') + chalk.gray(` → ${urlDotCMSInstance}/dotAdmin/#/edit-page?url=/index\n`) ); @@ -339,6 +336,57 @@ export function finalStepsForAngularAndAngularSSR({ console.log(chalk.blueBright('💬 Community: ') + chalk.white('https://community.dotcms.com\n')); } +/** + * The environment a scaffolded project needs, in the shape that project actually reads. + * + * One owner for this, because there are two consumers — the block printed in the final steps + * and the `.env` written by the exit-state handler — and they MUST agree. They did not: the + * handler wrote a hand-rolled `DOTCMS_AUTH_TOKEN` while Next.js reads + * `NEXT_PUBLIC_DOTCMS_AUTH_TOKEN`, so the file looked right and the app could not authenticate. + * Found by running the CLI end to end (#37262, T054). + * + * `filename` is null for frameworks that do not use a dotenv file at all — Angular reads a + * TypeScript `environment` object, so writing `.env` there would be cargo-culting. + */ +export interface EnvFileSpec { + filename: string | null; + contents: string; +} + +export function getEnvFileSpec( + framework: string | undefined, + host: string, + siteId: string, + token: string +): EnvFileSpec { + if (framework === 'astro') { + return { + filename: '.env', + contents: dedentEnv(getEnvVariablesForAstro(host, siteId, token)) + }; + } + + if (framework === 'angular' || framework === 'angular-ssr') { + return { + filename: null, + contents: dedentEnv(getEnvVariablesForAngular(host, siteId, token)) + }; + } + + return { filename: '.env', contents: dedentEnv(getEnvVariablesForNextJS(host, siteId, token)) }; +} + +/** The builders below indent for terminal display; a written file must not carry that. */ +function dedentEnv(block: string): string { + return ( + block + .split('\n') + .map((line) => line.trim()) + .filter(Boolean) + .join('\n') + '\n' + ); +} + function getEnvVariablesForNextJS(host: string, siteId: string, token: string) { return ` NEXT_PUBLIC_DOTCMS_AUTH_TOKEN=${token} @@ -476,24 +524,30 @@ function isPortAvailable(port: number): Promise { * Checks if required dotCMS ports are available * @returns Result with true if all ports available, or error message with busy ports */ -export async function checkPortsAvailability(): Promise> { - const requiredPorts = [ - { port: 8082, service: 'dotCMS HTTP' }, - { port: 8443, service: 'dotCMS HTTPS' }, - { port: 9200, service: 'Elasticsearch HTTP' }, - { port: 9600, service: 'Elasticsearch Transport' } - ]; +/** + * Which of the required ports are taken. + * + * Separate from `checkPortsAvailability` because a busy 8082 is not automatically a conflict: + * it may be a dotCMS from a previous successful run, which `resolvePortConflict` can reuse + * rather than refuse (AC-006). + */ +export async function findBusyPorts(): Promise<{ port: number; service: string }[]> { const busyPorts: { port: number; service: string }[] = []; - // Check all ports - for (const { port, service } of requiredPorts) { + for (const { port, service } of REQUIRED_PORTS) { const available = await isPortAvailable(port); if (!available) { busyPorts.push({ port, service }); } } + return busyPorts; +} + +export async function checkPortsAvailability(): Promise> { + const busyPorts = await findBusyPorts(); + if (busyPorts.length > 0) { const errorMsg = chalk.red('\n❌ Required ports are already in use\n\n') + @@ -603,7 +657,9 @@ export async function getDockerDiagnostics(directory?: string): Promise diagnostics.push(chalk.gray(' docker logs ')); diagnostics.push(chalk.white(' 3. Restart the containers:')); diagnostics.push(chalk.gray(' docker compose down && docker compose up -d')); - diagnostics.push(chalk.white(' 4. Check if ports 8082, 8443, 9200, and 9600 are available\n')); + // Must track REQUIRED_PORTS (utils/ports.ts): 9200/9600 are no longer published by the + // bundled stack, and 8090 now is. + diagnostics.push(chalk.white(' 4. Check if ports 8082, 8443, and 8090 are available\n')); return diagnostics.join('\n'); } diff --git a/core-web/libs/sdk/create-app/src/utils/install.spec.ts b/core-web/libs/sdk/create-app/src/utils/install.spec.ts new file mode 100644 index 000000000000..222c117e04f9 --- /dev/null +++ b/core-web/libs/sdk/create-app/src/utils/install.spec.ts @@ -0,0 +1,73 @@ +/** + * Contract spec for reporting a failed dependency install (task T032, dotCMS #37262, AC-007). + * + * THE BUG (contract X7). `installDependenciesForProject()` returns a `Result`. `Err(val)` is + * `{ ok: false, val }` — a TRUTHY OBJECT. The caller at `src/index.ts` tested `if (!result)`, + * which is therefore never true, so the failure branch was unreachable and a failed + * `npm install` was reported to the user as success. + * + * This is the same class of mistake as the rest of this issue: a green signal that does not + * mean what it claims. The fix is to branch on `result.ok`. + * + * API PINNED + * export type InstallReport = + * | { kind: 'installed' } + * | { kind: 'failed'; reason: string }; + * export function reportInstallResult(result: Result): InstallReport; + * + * A string discriminant, deliberately: `strict: false` in this workspace means TypeScript will + * not narrow a union on a boolean-literal discriminant — and a boolean here would be repeating + * the very mistake being fixed. + */ + +import { reportInstallResult } from './install'; + +import { Err, Ok } from '../result'; + +describe('reportInstallResult', () => { + it('reports success when the install succeeded', () => { + expect(reportInstallResult(Ok(undefined))).toEqual({ kind: 'installed' }); + }); + + it('reports FAILURE when the install failed — the branch that was unreachable', () => { + const report = reportInstallResult(Err(new Error('npm exited with code 1'))); + + expect(report.kind).toBe('failed'); + }); + + it('surfaces the underlying reason rather than swallowing it', () => { + const report = reportInstallResult(Err(new Error('ENOENT: npm not found'))); + + if (report.kind !== 'failed') { + throw new Error('expected a failure report'); + } + + expect(report.reason).toContain('npm not found'); + }); + + it('handles a non-Error failure value without losing it', () => { + const report = reportInstallResult(Err('exit status 127')); + + if (report.kind !== 'failed') { + throw new Error('expected a failure report'); + } + + expect(report.reason).toContain('127'); + }); + + /** + * The trap itself, pinned so nobody reintroduces `if (!result)`. + * + * This test asserts a property of `Err`, not of the code under test: an error Result is a + * truthy object, so negating it can never detect failure. It is here because the original + * bug is invisible on inspection — `if (!result)` reads like a perfectly ordinary guard. + */ + it('documents why `if (!result)` could never work: Err() is truthy', () => { + const failure = Err(new Error('boom')); + + expect(Boolean(failure)).toBe(true); + expect(!failure).toBe(false); + // The only correct discriminator: + expect(failure.ok).toBe(false); + }); +}); diff --git a/core-web/libs/sdk/create-app/src/utils/install.ts b/core-web/libs/sdk/create-app/src/utils/install.ts new file mode 100644 index 000000000000..d7534d30b89e --- /dev/null +++ b/core-web/libs/sdk/create-app/src/utils/install.ts @@ -0,0 +1,28 @@ +import type { Result } from '../result'; + +/** + * Turns a dependency-install `Result` into an explicit report. + * + * This exists because of contract X7. `Err(val)` is `{ ok: false, val }` — a truthy object — and + * the caller tested `if (!result)`, which is never true. The failure branch was therefore + * unreachable and a failed `npm install` was reported to the user as success. Branching on a + * string-discriminated report makes that mistake impossible to repeat by accident. + */ +export type InstallReport = { kind: 'installed' } | { kind: 'failed'; reason: string }; + +function describe(value: unknown): string { + if (value instanceof Error) { + return value.message; + } + + return typeof value === 'string' ? value : JSON.stringify(value); +} + +export function reportInstallResult(result: Result): InstallReport { + // `result.ok`, never `!result` — see the spec's "Err() is truthy" case. + if (result.ok) { + return { kind: 'installed' }; + } + + return { kind: 'failed', reason: describe(result.val) }; +} diff --git a/core-web/libs/sdk/create-app/src/utils/ports.spec.ts b/core-web/libs/sdk/create-app/src/utils/ports.spec.ts new file mode 100644 index 000000000000..d720fb120cc8 --- /dev/null +++ b/core-web/libs/sdk/create-app/src/utils/ports.spec.ts @@ -0,0 +1,318 @@ +/** + * Contract spec for port-conflict resolution (task T031, dotCMS #37262, AC-006). + * + * Written before the implementation; this file defines the API. + * + * THE BUG. Reproduction step 6: once a run leaves a dotCMS stack up, re-running the CLI aborts + * with "Required ports are already in use", because `checkPortsAvailability()` hard-fails on + * 8082/8443/9200/9600 — exactly the ports a SUCCESSFUL previous run now holds. The CLI's own + * side effect blocks its own recovery, which is what made the reported failure unrecoverable + * rather than merely annoying. + * + * DECISION D3 (already taken; this spec implements it, it does not re-open it): + * - Probe before failing. A busy 8082 with a healthy dotCMS behind it is a reusable instance, + * not a conflict. + * - Non-interactive (CI, or no TTY): auto-reuse, but PRINT A NOTICE. "Silent" means no prompt, + * not no output — a scripted run that quietly attaches to an unknown instance is precisely + * the failure this is meant to prevent. + * - Interactive: a real choice, reuse OR abort. Someone who did not expect a dotCMS on 8082 + * needs to stop and look, not be pushed onward. + * - Only reuse something that passes readiness AND can issue a token. A busy port with + * anything else behind it stays a hard failure. + * + * API PINNED + * export interface BusyPort { port: number; service: string } + * export type PortConflictOutcome = + * | { kind: 'free' } + * | { kind: 'reuse'; host: string } + * | { kind: 'abort'; message: string }; + * export function resolvePortConflict(options: { + * busyPorts: BusyPort[]; + * isInteractive: boolean; + * host: string; + * probeInstance: () => Promise; // readiness AND token issuance + * askReuse: () => Promise; + * notify: (message: string) => void; + * }): Promise; + * + * A STRING discriminant, not a boolean: this workspace sets "strict": false, and without + * strictNullChecks TypeScript will not narrow a union on a boolean-literal discriminant. + * + * Contract: contracts/cli-exit-contract.md X6. Decision: cli-design-decisions.md D3. + */ + +import { readFileSync } from 'node:fs'; +import { resolve } from 'node:path'; + +import { REQUIRED_PORTS, resolvePortConflict } from './ports'; + +const HOST = 'http://localhost:8082'; + +const DOTCMS_HTTP: { port: number; service: string } = { port: 8082, service: 'dotCMS HTTP' }; +const DOTCMS_HTTPS: { port: number; service: string } = { port: 8443, service: 'dotCMS HTTPS' }; +const FOREIGN: { port: number; service: string } = { port: 9200, service: 'Elasticsearch HTTP' }; + +/** + * What a previous run of THIS CLI actually leaves behind. Measured end to end (T054 step 5b): + * the bundled stack publishes 8082 and 8443, so a reusable instance holds BOTH. + * + * The original fixtures used 8082 alone, which no real stack ever produces — so every test + * passed while the reuse path could not trigger in practice, and reproduction step 6 stayed + * broken. The realistic set is the point of these cases. + */ +const A_REAL_RUNNING_STACK = [DOTCMS_HTTP, DOTCMS_HTTPS]; + +function options(overrides: Partial[0]> = {}) { + return { + busyPorts: [], + isInteractive: false, + host: HOST, + probeInstance: jest.fn().mockResolvedValue(true), + askAction: jest.fn().mockResolvedValue('reuse'), + owner: { project: 'my-app', description: 'Docker project "my-app" · healthy' }, + notify: jest.fn(), + ...overrides + }; +} + +describe('resolvePortConflict', () => { + let exitSpy: jest.SpyInstance; + + beforeEach(() => { + exitSpy = jest.spyOn(process, 'exit').mockImplementation(((code?: number) => { + throw new Error(`process.exit(${code}) must never be called here`); + }) as never); + }); + + afterEach(() => { + jest.restoreAllMocks(); + }); + + describe('nothing in the way', () => { + it('proceeds when no port is busy, without probing or prompting', async () => { + const opts = options(); + + const outcome = await resolvePortConflict(opts); + + expect(outcome.kind).toBe('free'); + expect(opts.probeInstance).not.toHaveBeenCalled(); + expect(opts.askAction).not.toHaveBeenCalled(); + }); + }); + + describe('a healthy dotCMS is already on 8082', () => { + it('reuses it without prompting when non-interactive', async () => { + const opts = options({ busyPorts: A_REAL_RUNNING_STACK, isInteractive: false }); + + const outcome = await resolvePortConflict(opts); + + expect(outcome).toEqual({ kind: 'reuse', host: HOST }); + expect(opts.askAction).not.toHaveBeenCalled(); + }); + + it('still prints a notice when it auto-reuses (D3: silent means no prompt, not no output)', async () => { + const opts = options({ busyPorts: A_REAL_RUNNING_STACK, isInteractive: false }); + + await resolvePortConflict(opts); + + expect(opts.notify).toHaveBeenCalled(); + + const said = (opts.notify as jest.Mock).mock.calls.flat().join('\n'); + + expect(said).toContain('8082'); + expect(said).toMatch(/reus/i); + }); + + it('asks the user when interactive, and reuses on yes', async () => { + const opts = options({ + busyPorts: A_REAL_RUNNING_STACK, + isInteractive: true, + askAction: jest.fn().mockResolvedValue('reuse') + }); + + const outcome = await resolvePortConflict(opts); + + expect(opts.askAction).toHaveBeenCalledTimes(1); + expect(outcome).toEqual({ kind: 'reuse', host: HOST }); + }); + + it('aborts on no — the prompt is a real choice, not a formality', async () => { + const opts = options({ + busyPorts: A_REAL_RUNNING_STACK, + isInteractive: true, + askAction: jest.fn().mockResolvedValue('cancel') + }); + + const outcome = await resolvePortConflict(opts); + + expect(outcome.kind).toBe('abort'); + }); + }); + + describe('the port is busy but it is not a usable dotCMS', () => { + it('aborts when the probe fails, naming the busy port', async () => { + const opts = options({ + busyPorts: [DOTCMS_HTTP], + isInteractive: false, + probeInstance: jest.fn().mockResolvedValue(false) + }); + + const outcome = await resolvePortConflict(opts); + + expect(outcome.kind).toBe('abort'); + + if (outcome.kind === 'abort') { + expect(outcome.message).toContain('8082'); + } + }); + + it('never reuses an instance whose readiness or token issuance failed', async () => { + const opts = options({ + busyPorts: [DOTCMS_HTTP], + isInteractive: true, + probeInstance: jest.fn().mockResolvedValue(false) + }); + + const outcome = await resolvePortConflict(opts); + + expect(outcome.kind).toBe('abort'); + // It must not even offer the choice: there is nothing safe to reuse. + expect(opts.askAction).not.toHaveBeenCalled(); + }); + + it('aborts when a required port other than 8082 is taken', async () => { + const opts = options({ busyPorts: [FOREIGN], isInteractive: false }); + + const outcome = await resolvePortConflict(opts); + + expect(outcome.kind).toBe('abort'); + + if (outcome.kind === 'abort') { + expect(outcome.message).toContain('9200'); + } + }); + }); + + describe("the busy set must match what this CLI's own stack publishes", () => { + it('reuses when 8082 AND 8443 are held — the shape a real previous run leaves', async () => { + const opts = options({ busyPorts: A_REAL_RUNNING_STACK, isInteractive: false }); + + expect(await resolvePortConflict(opts)).toEqual({ kind: 'reuse', host: HOST }); + }); + + it('aborts when a port this stack does not publish is also held', async () => { + const opts = options({ + busyPorts: [...A_REAL_RUNNING_STACK, FOREIGN], + isInteractive: false + }); + + const outcome = await resolvePortConflict(opts); + + expect(outcome.kind).toBe('abort'); + }); + + it('aborts when 8443 is held but 8082 is free — nothing is answering to reuse', async () => { + const opts = options({ busyPorts: [DOTCMS_HTTPS], isInteractive: false }); + + expect((await resolvePortConflict(opts)).kind).toBe('abort'); + }); + }); + + /** + * "A dotCMS is already running. What would you like to do?" told the user a fact and then + * abandoned them: the only choices were reuse or quit, so anyone who did NOT want that + * instance had to leave the CLI and run docker by hand. Replacing it in place is the + * documented recovery for a bricked instance (#37262), so the CLI should be able to do it. + */ + describe('replacing the running instance', () => { + it('offers replace when the port owner is a compose project we can stop', async () => { + const opts = options({ + busyPorts: A_REAL_RUNNING_STACK, + isInteractive: true, + askAction: jest.fn().mockResolvedValue('replace') + }); + + const outcome = await resolvePortConflict(opts); + + expect(outcome).toEqual({ kind: 'replace', project: 'my-app' }); + }); + + it('tells the prompt whether replacing is even possible', async () => { + const askAction = jest.fn().mockResolvedValue('reuse'); + + await resolvePortConflict( + options({ busyPorts: A_REAL_RUNNING_STACK, isInteractive: true, askAction }) + ); + expect(askAction).toHaveBeenCalledWith(expect.objectContaining({ canReplace: true })); + + askAction.mockClear(); + + // No compose project — something started outside compose. Stopping it is not ours to do. + await resolvePortConflict( + options({ + busyPorts: A_REAL_RUNNING_STACK, + isInteractive: true, + askAction, + owner: { description: 'an unknown process' } + }) + ); + expect(askAction).toHaveBeenCalledWith(expect.objectContaining({ canReplace: false })); + }); + + it('never replaces without being asked — non-interactive stays reuse-only', async () => { + const askAction = jest.fn(); + const opts = options({ + busyPorts: A_REAL_RUNNING_STACK, + isInteractive: false, + askAction + }); + + const outcome = await resolvePortConflict(opts); + + // Destroying an instance is not something to infer from the absence of a TTY. + expect(outcome.kind).toBe('reuse'); + expect(askAction).not.toHaveBeenCalled(); + }); + + it('passes the owner description through so the prompt can say what it found', async () => { + const askAction = jest.fn().mockResolvedValue('cancel'); + + await resolvePortConflict( + options({ busyPorts: A_REAL_RUNNING_STACK, isInteractive: true, askAction }) + ); + + expect(askAction).toHaveBeenCalledWith( + expect.objectContaining({ description: expect.stringContaining('my-app') }) + ); + }); + }); + + describe('contract X2 — abort is a value, not an exit', () => { + it('never calls process.exit on any path', async () => { + await resolvePortConflict(options()); + await resolvePortConflict(options({ busyPorts: [DOTCMS_HTTP] })); + await resolvePortConflict( + options({ busyPorts: [FOREIGN], probeInstance: jest.fn().mockResolvedValue(false) }) + ); + + expect(exitSpy).not.toHaveBeenCalled(); + }); + }); +}); + +/** + * Guards against the drift that broke this in the first place: the CLI checked 9200/9600, + * inherited from the shared compose example, while the bundled stack publishes neither. Anyone + * running their own OpenSearch on 9200 was blocked for ports this stack never uses. + */ +describe('the ports the CLI checks match the ports its stack publishes', () => { + it('checks exactly the published ports, no more and no fewer', () => { + const asset = readFileSync(resolve(__dirname, '../../assets/docker-compose.yml'), 'utf8'); + const published = [...asset.matchAll(/^\s*-\s*'(?:[\d.]+:)?(\d+):\d+'/gm)].map((m) => + Number(m[1]) + ); + + expect(published.length).toBeGreaterThan(0); + expect([...REQUIRED_PORTS].map((p) => p.port).sort()).toEqual([...published].sort()); + }); +}); diff --git a/core-web/libs/sdk/create-app/src/utils/ports.ts b/core-web/libs/sdk/create-app/src/utils/ports.ts new file mode 100644 index 000000000000..320a03ef577d --- /dev/null +++ b/core-web/libs/sdk/create-app/src/utils/ports.ts @@ -0,0 +1,198 @@ +/** + * Decides what to do when a required port is already taken. + * + * Reproduction step 6 of #37262: after a successful run leaves a dotCMS stack up, re-running the + * CLI aborts with "Required ports are already in use" — on exactly the ports that success now + * holds. The CLI's own side effect blocked its own recovery, which is what turned an annoying + * failure into an unrecoverable one. + * + * A busy 8082 with a healthy dotCMS behind it is not a conflict; it is an instance to reuse. + * Anything else still fails, and it fails as a returned value rather than a `process.exit` + * (contract X2). + */ + +export interface BusyPort { + port: number; + service: string; +} + +export type PortConflictOutcome = + | { kind: 'free' } + | { kind: 'reuse'; host: string } + /** Stop the compose project holding the ports, wipe its volumes, then provision fresh. */ + | { kind: 'replace'; project: string } + | { kind: 'abort'; message: string }; + +/** What the user is choosing between, and enough context to choose. */ +export interface PortOwner { + /** Compose project holding the ports, when there is one we could safely stop. */ + project?: string; + /** Human-readable, e.g. `Docker project "my-app" · healthy · up 4 minutes`. */ + description: string; +} + +export type PortConflictAction = 'reuse' | 'replace' | 'cancel'; + +export interface ResolvePortConflictOptions { + busyPorts: BusyPort[]; + /** False in CI or when stdout is not a TTY — a prompt there would hang a scripted run. */ + isInteractive: boolean; + host: string; + /** Must confirm BOTH readiness and token issuance; a half-dead instance is not reusable. */ + probeInstance: () => Promise; + /** + * Asks the user what to do. `canReplace` is false when nothing owns the ports that we could + * safely stop — something started outside compose is not ours to destroy. + */ + askAction: (context: { + description: string; + canReplace: boolean; + }) => Promise; + notify: (message: string) => void; + owner?: PortOwner; +} + +const DOTCMS_HTTP_PORT = 8082; + +/** + * The ports the bundled stack publishes — and therefore the only ones worth checking. + * + * This list used to include 9200 and 9600, inherited from the shared compose example back when + * the CLI downloaded it. The bundled asset publishes neither (OpenSearch has no `ports:` at all), + * so the CLI was refusing to run for anyone with their own OpenSearch on 9200 over a conflict + * that could not happen. A spec pins this list against the asset so the two cannot drift again. + */ +export const REQUIRED_PORTS = [ + { port: 8082, service: 'dotCMS HTTP' }, + { port: 8443, service: 'dotCMS HTTPS' }, + { port: 8090, service: 'dotCMS management' } +]; + +function listPorts(busyPorts: BusyPort[]): string { + return busyPorts.map(({ port, service }) => ` • Port ${port} (${service})`).join('\n'); +} + +function abortMessage(busyPorts: BusyPort[], detail: string): PortConflictOutcome { + return { + kind: 'abort', + message: [ + 'Required ports are already in use:', + listPorts(busyPorts), + '', + detail, + '', + 'Stop whatever is holding them, or stop an existing stack with:', + ' docker compose down' + ].join('\n') + }; +} + +export async function resolvePortConflict( + options: ResolvePortConflictOptions +): Promise { + const { busyPorts, isInteractive, host, probeInstance, askAction, notify, owner } = options; + + if (busyPorts.length === 0) { + return { kind: 'free' }; + } + + // Reuse is only meaningful when the thing in the way IS a stack this CLI would have started: + // every busy port must be one of ours, and 8082 must be among them or nothing is answering + // to reuse. + // + // This previously required 8082 to be the ONLY busy port, which no real instance ever + // produces — a running stack holds 8082 and 8443 together, so the reuse path could never + // fire and reproduction step 6 stayed broken despite passing unit tests. Found by running + // the CLI against a real bricked instance (T054 step 5b). + const ours = new Set(REQUIRED_PORTS.map(({ port }) => port)); + const looksLikeOurStack = + busyPorts.every(({ port }) => ours.has(port)) && + busyPorts.some(({ port }) => port === DOTCMS_HTTP_PORT); + + if (!looksLikeOurStack) { + return abortMessage( + busyPorts, + 'These are not ports a previous run of this CLI would be holding on its own.' + ); + } + + const reusable = await probeInstance(); + + if (!reusable) { + return abortMessage( + busyPorts, + `Something is listening on ${DOTCMS_HTTP_PORT}, but it did not answer as a usable dotCMS.` + ); + } + + if (!isInteractive) { + // D3: non-interactive means "do not block a scripted run with a prompt" — it does not + // mean do it quietly. Attaching to an unknown instance without saying so is the failure + // this branch exists to avoid. + notify( + `dotCMS is already running on ${DOTCMS_HTTP_PORT} — reusing it (non-interactive run).` + ); + + return { kind: 'reuse', host }; + } + + // A real choice, with a way out that does not mean leaving the CLI. Replacing is only + // offered when we can identify a compose project to stop — something started outside compose + // is not ours to destroy, whatever the user picks. + const canReplace = Boolean(owner?.project); + const action = await askAction({ + description: owner?.description ?? `something on port ${DOTCMS_HTTP_PORT}`, + canReplace + }); + + if (action === 'replace' && owner?.project) { + return { kind: 'replace', project: owner.project }; + } + + if (action === 'cancel') { + return { + kind: 'abort', + message: `Left the dotCMS already running on ${DOTCMS_HTTP_PORT} untouched, as requested.` + }; + } + + return { kind: 'reuse', host }; +} + +/** + * Who is holding the port, and can we safely stop them? + * + * Reads the compose labels off whatever publishes 8082. A container started outside compose has + * no project label, and then `project` is undefined — the caller must not offer to stop it. + */ +export async function describePortOwner( + port: number, + run: (cmd: string, args: string[]) => Promise<{ stdout: string }> +): Promise { + try { + const { stdout } = await run('docker', [ + 'ps', + '--filter', + `publish=${port}`, + '--format', + '{{.Label "com.docker.compose.project"}}\t{{.Status}}\t{{.Names}}' + ]); + + const line = stdout.trim().split('\n').filter(Boolean)[0]; + + if (!line) { + return undefined; + } + + const [project, status, name] = line.split('\t'); + + return { + project: project || undefined, + description: project + ? `Docker project "${project}" · ${status}` + : `container ${name} · ${status} · not managed by Docker Compose` + }; + } catch { + return undefined; + } +} diff --git a/core-web/libs/sdk/create-app/src/utils/readiness.spec.ts b/core-web/libs/sdk/create-app/src/utils/readiness.spec.ts new file mode 100644 index 000000000000..d60dbc4204de --- /dev/null +++ b/core-web/libs/sdk/create-app/src/utils/readiness.spec.ts @@ -0,0 +1,277 @@ +/** + * Contract spec for the readiness probe (task T041, dotCMS #37262, AC-009 / contract X5). + * + * Written before the implementation; this file defines the API. + * + * WHY THIS CHANGES. Today the CLI's only readiness signal is `/api/v1/appconfiguration` on 8082 + * (`getDotcmsApisByBaseUrl` → `DOTCMS_HEALTH_API`). That endpoint was chosen as a workaround: the + * purpose-built probes were unreachable because of IP ACL restrictions from the Docker host + * (issue #34509). The bundled compose file now publishes the management port itself + * (`127.0.0.1:8090:8090`), so `/dotmgt/readyz` is reachable from the host and the workaround is no + * longer needed. `readyz` is the right signal: it does not depend on the web application being up. + * + * MEASURED, 2026-08-31, against a real stack on released `dotcms/dotcms:latest`: + * - Both `/dotmgt/livez` and `/dotmgt/readyz` exist on the released image. + * - `docker compose up --wait` reporting "healthy" means LIVE, not READY. The container + * healthcheck probes `livez`; `readyz` was still answering 503 for a few seconds AFTER + * `--wait` had already returned, and only then flipped to 200 "ready". + * - Therefore **a 503 from `readyz` is the normal "still starting" state, not an error**. That is + * the single most important case in this file: treating it as a failure would make the CLI + * abort on a stack that is booting exactly as designed. + * + * API PINNED + * export type Readiness = + * | { kind: 'ready' } + * | { kind: 'not-ready'; detail: string }; + * export function probeReadiness(options: { + * readyzUrl: string; // /dotmgt/readyz on 8090 + * fallbackUrl: string; // /api/v1/appconfiguration on 8082 + * get: (url: string) => Promise<{ status: number }>; // injected: no test touches the network + * }): Promise; + * + * A STRING discriminant, not a boolean: this workspace sets `"strict": false` in + * tsconfig.base.json, and without `strictNullChecks` TypeScript will not narrow a union on a + * boolean-literal discriminant. + * + * THE STATUS CONTRACT (X5). Today `fetchWithRetry` accepts any 2xx (`validateStatus: status >= 200 + * && status < 300`) while `isDotcmsRunning` re-narrows to `res.status === 200` — so a 204 is + * "success" to one and failure to the other. One rule from here on: **any 2xx counts as ready**, + * and callers MUST NOT re-narrow. Pinned below with an explicit 204 case. + * + * ONE probe, not a retry loop. `probeReadiness` answers "is it ready *right now*" and returns a + * value; waiting and retrying belong to the caller. + * + * Contract: contracts/cli-exit-contract.md X5 (readiness) and X2 (a result is a value, never an + * exit). Spec: spec.md AC-009. + */ + +import { probeReadiness } from './readiness'; + +const READYZ_URL = 'http://127.0.0.1:8090/dotmgt/readyz'; +const FALLBACK_URL = 'http://localhost:8082/api/v1/appconfiguration'; + +/** A `get` that answers per-URL, so a test can say "readyz 404, fallback 200" in one line. */ +function responder(byUrl: Record) { + return jest.fn(async (url: string) => { + const answer = byUrl[url]; + + if (answer === undefined) { + throw new Error(`connect ECONNREFUSED (${url})`); + } + + if (answer instanceof Error) { + throw answer; + } + + return { status: answer }; + }); +} + +function options(overrides: Partial[0]> = {}) { + return { + readyzUrl: READYZ_URL, + fallbackUrl: FALLBACK_URL, + get: responder({ [READYZ_URL]: 200 }), + ...overrides + }; +} + +describe('probeReadiness', () => { + let exitSpy: jest.SpyInstance; + + beforeEach(() => { + exitSpy = jest.spyOn(process, 'exit').mockImplementation(((code?: number) => { + throw new Error(`process.exit(${code}) must never be called here`); + }) as never); + }); + + afterEach(() => { + jest.restoreAllMocks(); + }); + + describe('readyz is the preferred signal', () => { + it('reports ready on a 200 from readyz', async () => { + const opts = options({ get: responder({ [READYZ_URL]: 200 }) }); + + const readiness = await probeReadiness(opts); + + expect(readiness).toEqual({ kind: 'ready' }); + }); + + it('does not touch the fallback when readyz answers', async () => { + const opts = options({ + get: responder({ [READYZ_URL]: 200, [FALLBACK_URL]: 200 }) + }); + + await probeReadiness(opts); + + const asked = (opts.get as jest.Mock).mock.calls.map(([url]) => url); + + expect(asked).toEqual([READYZ_URL]); + expect(asked).not.toContain(FALLBACK_URL); + }); + }); + + describe('503 from readyz — still starting, NOT an error (measured 2026-08-31)', () => { + it('reports not-ready rather than throwing or erroring out', async () => { + const opts = options({ get: responder({ [READYZ_URL]: 503 }) }); + + const readiness = await probeReadiness(opts); + + expect(readiness.kind).toBe('not-ready'); + }); + + it('words the detail as "still starting", not as a failure', async () => { + const opts = options({ get: responder({ [READYZ_URL]: 503 }) }); + + const readiness = await probeReadiness(opts); + + if (readiness.kind === 'not-ready') { + // `compose up --wait` reporting healthy only means livez passed; readyz stays 503 + // for a few more seconds. The user must be told the stack is booting, not that + // something went wrong. + expect(readiness.detail).toMatch(/start/i); + expect(readiness.detail).not.toMatch(/error|fail/i); + } + }); + + it('does not fall back on a 503 — readyz answered, and its answer is "not yet"', async () => { + // The fallback exists for stacks whose compose predates the published 8090 port, not + // as a second opinion. `/api/v1/appconfiguration` can answer 200 while readyz is still + // 503, so consulting it here would report a booting stack as ready. + const opts = options({ + get: responder({ [READYZ_URL]: 503, [FALLBACK_URL]: 200 }) + }); + + const readiness = await probeReadiness(opts); + + expect(readiness.kind).toBe('not-ready'); + expect((opts.get as jest.Mock).mock.calls.map(([url]) => url)).not.toContain( + FALLBACK_URL + ); + }); + }); + + describe('older images without /dotmgt/readyz fall back to appconfiguration', () => { + it('falls back on a 404 and reports ready when the fallback answers 200', async () => { + const opts = options({ + get: responder({ [READYZ_URL]: 404, [FALLBACK_URL]: 200 }) + }); + + const readiness = await probeReadiness(opts); + + expect(readiness).toEqual({ kind: 'ready' }); + expect((opts.get as jest.Mock).mock.calls.map(([url]) => url)).toEqual([ + READYZ_URL, + FALLBACK_URL + ]); + }); + + it('falls back when readyz is unreachable (port 8090 not published)', async () => { + const opts = options({ + get: responder({ + [READYZ_URL]: new Error('connect ECONNREFUSED 127.0.0.1:8090'), + [FALLBACK_URL]: 200 + }) + }); + + const readiness = await probeReadiness(opts); + + expect(readiness).toEqual({ kind: 'ready' }); + expect((opts.get as jest.Mock).mock.calls.map(([url]) => url)).toContain(FALLBACK_URL); + }); + + it('reports not-ready and names both URLs when the fallback fails too', async () => { + const opts = options({ + get: responder({ + [READYZ_URL]: new Error('connect ECONNREFUSED 127.0.0.1:8090'), + [FALLBACK_URL]: new Error('connect ECONNREFUSED 127.0.0.1:8082') + }) + }); + + const readiness = await probeReadiness(opts); + + expect(readiness.kind).toBe('not-ready'); + + if (readiness.kind === 'not-ready') { + // Whoever reads this line needs to know what was tried, or they will guess. + expect(readiness.detail).toContain(READYZ_URL); + expect(readiness.detail).toContain(FALLBACK_URL); + } + }); + + it('reports not-ready when the fallback answers a non-2xx', async () => { + const opts = options({ + get: responder({ [READYZ_URL]: 404, [FALLBACK_URL]: 500 }) + }); + + const readiness = await probeReadiness(opts); + + expect(readiness.kind).toBe('not-ready'); + }); + }); + + describe('the status contract — any 2xx is ready, on either URL', () => { + it.each([200, 201, 204])('treats %i from readyz as ready', async (status) => { + const readiness = await probeReadiness( + options({ get: responder({ [READYZ_URL]: status }) }) + ); + + expect(readiness).toEqual({ kind: 'ready' }); + }); + + it('treats a 204 as ready — the one status the two current call sites disagree about', async () => { + // `fetchWithRetry` accepts it (validateStatus: 200-299); `isDotcmsRunning` rejects it + // (`res.status === 200`). This assertion is the tie-break: 204 is ready. + const readiness = await probeReadiness( + options({ get: responder({ [READYZ_URL]: 204 }) }) + ); + + expect(readiness).toEqual({ kind: 'ready' }); + }); + + it('treats a 204 from the fallback as ready as well', async () => { + const readiness = await probeReadiness( + options({ get: responder({ [READYZ_URL]: 404, [FALLBACK_URL]: 204 }) }) + ); + + expect(readiness).toEqual({ kind: 'ready' }); + }); + + it('treats a 3xx as not ready — a redirect is not an answer', async () => { + const readiness = await probeReadiness( + options({ get: responder({ [READYZ_URL]: 302, [FALLBACK_URL]: 302 }) }) + ); + + expect(readiness.kind).toBe('not-ready'); + }); + }); + + describe('contract X2 — the probe returns a value, it never exits and never throws', () => { + const everyPath = [ + { [READYZ_URL]: 200 }, + { [READYZ_URL]: 204 }, + { [READYZ_URL]: 503 }, + { [READYZ_URL]: 404, [FALLBACK_URL]: 200 }, + { [READYZ_URL]: 404, [FALLBACK_URL]: 500 }, + { [READYZ_URL]: new Error('ECONNREFUSED'), [FALLBACK_URL]: new Error('ECONNREFUSED') }, + {} + ]; + + it('never throws, whatever the two endpoints do', async () => { + for (const byUrl of everyPath) { + await expect(probeReadiness(options({ get: responder(byUrl) }))).resolves.toEqual( + expect.objectContaining({ kind: expect.any(String) }) + ); + } + }); + + it('never calls process.exit on any path', async () => { + for (const byUrl of everyPath) { + await probeReadiness(options({ get: responder(byUrl) })); + } + + expect(exitSpy).not.toHaveBeenCalled(); + }); + }); +}); diff --git a/core-web/libs/sdk/create-app/src/utils/readiness.ts b/core-web/libs/sdk/create-app/src/utils/readiness.ts new file mode 100644 index 000000000000..e7799be7d809 --- /dev/null +++ b/core-web/libs/sdk/create-app/src/utils/readiness.ts @@ -0,0 +1,111 @@ +import { isSuccessStatus } from './fetch-retry'; + +/** + * Answers one question: will the call we are about to make succeed? + * + * The CLI probed `/api/v1/appconfiguration` because the management endpoints were unreachable + * from the Docker host (issue #34509). The bundled compose file now publishes 8090 on loopback, + * so `/dotmgt/readyz` — the purpose-built readiness endpoint, which does not depend on the web + * app being up — is available and preferred. + * + * Measured against a real stack on 2026-08-31: `docker compose up --wait` reporting *healthy* + * means LIVE, not READY. The container healthcheck probes `livez`, and `readyz` returned 503 for + * a few seconds after `--wait` had already returned, then 200. So a 503 here is the ordinary + * "still starting" state and not an error. + */ + +export type Readiness = { kind: 'ready' } | { kind: 'not-ready'; detail: string }; + +export interface ProbeReadinessOptions { + /** `/dotmgt/readyz` on the management port. */ + readyzUrl: string; + /** `/api/v1/appconfiguration`, for images that do not serve the management endpoints. */ + fallbackUrl: string; + get: (url: string) => Promise<{ status: number }>; +} + +const STILL_STARTING = 503; + +export async function probeReadiness({ + readyzUrl, + fallbackUrl, + get +}: ProbeReadinessOptions): Promise { + let readyzStatus: number | null = null; + + try { + const { status } = await get(readyzUrl); + readyzStatus = status; + + if (isSuccessStatus(status)) { + return { kind: 'ready' }; + } + + if (status === STILL_STARTING) { + // Authoritative: readyz answered and said not yet. Do NOT consult the fallback here. + // `appconfiguration` can return 200 while the stack is still coming up, so a second + // opinion would report a booting instance as ready — the exact failure being fixed. + return { kind: 'not-ready', detail: 'dotCMS is still starting up' }; + } + } catch { + // Unreachable — fall through to the fallback below. + } + + // Anything else from readyz (404 on an older image, a transport failure) means the endpoint + // cannot be trusted to exist, so fall back to the app endpoint the CLI used before. + try { + const { status } = await get(fallbackUrl); + + if (isSuccessStatus(status)) { + return { kind: 'ready' }; + } + + return { + kind: 'not-ready', + detail: `${readyzUrl} answered ${readyzStatus ?? 'nothing'} and ${fallbackUrl} answered ${status}` + }; + } catch (error) { + const reason = error instanceof Error ? error.message : String(error); + + return { + kind: 'not-ready', + detail: `neither ${readyzUrl} nor ${fallbackUrl} could be reached (${reason})` + }; + } +} + +/** + * Polls {@link probeReadiness} until the instance is ready or the budget runs out. + * + * Kept separate from `fetchWithRetry` because the semantics differ: this never throws, and a 503 + * is a normal intermediate state rather than a failed attempt. The reporter is injected for the + * same reason as everywhere else in this CLI — the caller owns the spinner (AC-009). + */ +export async function waitForReadiness(options: { + readyzUrl: string; + fallbackUrl: string; + get: (url: string) => Promise<{ status: number }>; + attempts: number; + delayMs: number; + onAttempt?: (attempt: number, attempts: number, detail: string) => void; +}): Promise { + const { attempts, delayMs, onAttempt, ...probe } = options; + + let last: Readiness = { kind: 'not-ready', detail: 'not probed yet' }; + + for (let attempt = 1; attempt <= attempts; attempt++) { + last = await probeReadiness(probe); + + if (last.kind === 'ready') { + return last; + } + + onAttempt?.(attempt, attempts, last.detail); + + if (attempt < attempts) { + await new Promise((resolve) => setTimeout(resolve, delayMs)); + } + } + + return last; +} diff --git a/core-web/libs/sdk/create-app/src/utils/starter-url.spec.ts b/core-web/libs/sdk/create-app/src/utils/starter-url.spec.ts new file mode 100644 index 000000000000..58fcd84dbf22 --- /dev/null +++ b/core-web/libs/sdk/create-app/src/utils/starter-url.spec.ts @@ -0,0 +1,278 @@ +import { existsSync, readFileSync } from 'fs'; +import path from 'path'; + +import { applyStarterUrl } from './starter-url'; + +/** + * AC-012 — `npx @dotcms/create-app --starter ` must keep working against the compose + * file the CLI now *bundles* (previously it was downloaded at runtime). + * + * Contract under test — `src/utils/starter-url.ts` must export: + * + * export function applyStarterUrl(composeContents: string, starterUrl: string): string + * + * A pure string -> string rewrite: no `fs`, no `path`, no `directory` argument. The caller + * (`updateDockerComposeStarterUrl` in `src/index.ts`) keeps the read/write. It rewrites the + * single line matching: + * + * /^(\s*["']?CUSTOM_STARTER_URL["']?\s*:\s*).+$/m + * + * to `$1""`, and throws when nothing matched. + */ + +/** + * The regex the CLI ships. Mirrored here on purpose: this spec is the guard that the bundled + * asset keeps a line this exact pattern can match, so it must not import the implementation's + * copy — a change to the implementation regex has to fail here loudly, not silently agree. + */ +const CUSTOM_STARTER_URL_LINE = /^(\s*["']?CUSTOM_STARTER_URL["']?\s*:\s*).+$/m; + +/** + * The real file shipped inside the published package: `libs/sdk/create-app/assets/docker-compose.yml`. + * + * NOTE: resolved as `../../assets` (not `../assets`) because this spec sits in `src/utils/`, + * two levels below the package root where `assets/` lives. + */ +const BUNDLED_COMPOSE_PATH = path.resolve(__dirname, '../../assets/docker-compose.yml'); + +const NEW_STARTER_URL = 'https://downloads.dotcms.com/starters/my-custom-starter.zip'; + +/** Reads the bundled asset, failing with a diagnosable message rather than an ENOENT stack. */ +function readBundledCompose(): string { + if (!existsSync(BUNDLED_COMPOSE_PATH)) { + throw new Error( + `AC-012 regression guard cannot run: the bundled compose asset is MISSING at ` + + `${BUNDLED_COMPOSE_PATH}.\n` + + `The CLI ships its own compose file — create it (task T013) so \`--starter\` has ` + + `something to rewrite. This test must never be skipped: without the asset there is ` + + `nothing guarding the CUSTOM_STARTER_URL line shape that installed CLIs depend on.` + ); + } + + return readFileSync(BUNDLED_COMPOSE_PATH, 'utf-8'); +} + +describe('applyStarterUrl', () => { + describe('rewriting a conventional CUSTOM_STARTER_URL line', () => { + it('replaces the existing value with the new starter url', () => { + const compose = [ + 'services:', + ' dotcms:', + ' environment:', + " CUSTOM_STARTER_URL: 'https://downloads.dotcms.com/starters/old.zip'", + ' DB_BASE_URL: "jdbc:postgresql://db/dotcms"' + ].join('\n'); + + const result = applyStarterUrl(compose, NEW_STARTER_URL); + + expect(result).toContain(`CUSTOM_STARTER_URL: "${NEW_STARTER_URL}"`); + expect(result).not.toContain('old.zip'); + }); + + it('leaves every other line byte-identical', () => { + const compose = [ + 'services:', + ' dotcms:', + ' environment:', + " CUSTOM_STARTER_URL: 'https://downloads.dotcms.com/starters/old.zip'", + ' DB_BASE_URL: "jdbc:postgresql://db/dotcms"' + ].join('\n'); + + const result = applyStarterUrl(compose, NEW_STARTER_URL).split('\n'); + const original = compose.split('\n'); + + expect(result).toHaveLength(original.length); + original.forEach((line, index) => { + if (line.includes('CUSTOM_STARTER_URL')) { + expect(result[index]).not.toEqual(line); + } else { + expect(result[index]).toEqual(line); + } + }); + }); + + it('preserves the surrounding indentation exactly', () => { + const compose = ` CUSTOM_STARTER_URL: 'https://old.example.com/starter.zip'`; + + const result = applyStarterUrl(compose, NEW_STARTER_URL); + + expect(result).toEqual(` CUSTOM_STARTER_URL: "${NEW_STARTER_URL}"`); + }); + + it('rewrites only the first matching line, since the regex is not global', () => { + const compose = [ + ' CUSTOM_STARTER_URL: https://first.example.com/a.zip', + ' CUSTOM_STARTER_URL: https://second.example.com/b.zip' + ].join('\n'); + + const result = applyStarterUrl(compose, NEW_STARTER_URL).split('\n'); + + expect(result[0]).toEqual(` CUSTOM_STARTER_URL: "${NEW_STARTER_URL}"`); + expect(result[1]).toEqual(' CUSTOM_STARTER_URL: https://second.example.com/b.zip'); + }); + }); + + describe('key spellings the regex allows', () => { + it.each([ + ['unquoted', ' CUSTOM_STARTER_URL: https://old.example.com/starter.zip'], + ['double-quoted key', ` "CUSTOM_STARTER_URL": 'https://old.example.com/a.zip'`], + ['single-quoted key', ` 'CUSTOM_STARTER_URL': "https://old.example.com/a.zip"`], + ['no space after colon', ' CUSTOM_STARTER_URL:https://old.example.com/a.zip'], + ['space before colon', ' CUSTOM_STARTER_URL : https://old.example.com/a.zip'], + ['top-level, no indentation', 'CUSTOM_STARTER_URL: https://old.example.com/a.zip'] + ])('rewrites the %s form', (_label, line) => { + const result = applyStarterUrl(line, NEW_STARTER_URL); + + expect(result).toContain(`"${NEW_STARTER_URL}"`); + expect(result).not.toContain('old.example.com'); + }); + + it('keeps the key quoting the asset author chose', () => { + const result = applyStarterUrl( + ` "CUSTOM_STARTER_URL": 'https://old.example.com/a.zip'`, + NEW_STARTER_URL + ); + + expect(result).toEqual(` "CUSTOM_STARTER_URL": "${NEW_STARTER_URL}"`); + }); + }); + + describe('when the key is absent', () => { + it('throws an actionable error naming CUSTOM_STARTER_URL and --starter', () => { + const compose = [ + 'services:', + ' dotcms:', + ' environment:', + ' DB_BASE_URL: x' + ].join('\n'); + + expect(() => applyStarterUrl(compose, NEW_STARTER_URL)).toThrow( + /CUSTOM_STARTER_URL entry not found/ + ); + expect(() => applyStarterUrl(compose, NEW_STARTER_URL)).toThrow(/--starter/); + }); + + it('throws on an empty compose file rather than returning it unchanged', () => { + expect(() => applyStarterUrl('', NEW_STARTER_URL)).toThrow( + /CUSTOM_STARTER_URL entry not found/ + ); + }); + + it('throws on the `- CUSTOM_STARTER_URL=value` env list form, which the regex cannot match', () => { + // The reformat that would silently break `--starter` for users if it ever reached + // the bundled asset. It must fail loudly here instead. + const compose = [ + 'services:', + ' dotcms:', + ' environment:', + ' - CUSTOM_STARTER_URL=https://old.example.com/starter.zip' + ].join('\n'); + + expect(() => applyStarterUrl(compose, NEW_STARTER_URL)).toThrow( + /CUSTOM_STARTER_URL entry not found/ + ); + }); + }); + + describe('AC-012 regression guard — the real bundled asset', () => { + it('ships a CUSTOM_STARTER_URL line the rewrite regex matches, exactly once', () => { + const compose = readBundledCompose(); + + const keyLines = compose + .split('\n') + .filter((line) => line.includes('CUSTOM_STARTER_URL')); + + expect(keyLines).toHaveLength(1); + expect(CUSTOM_STARTER_URL_LINE.test(compose)).toBe(true); + }); + + it('rewrites successfully when run against the asset the package ships', () => { + const compose = readBundledCompose(); + + const result = applyStarterUrl(compose, NEW_STARTER_URL); + + expect(result).not.toEqual(compose); + expect(result).toContain(`"${NEW_STARTER_URL}"`); + }); + + it('changes the CUSTOM_STARTER_URL line and nothing else', () => { + const compose = readBundledCompose(); + const original = compose.split('\n'); + + const result = applyStarterUrl(compose, NEW_STARTER_URL).split('\n'); + + expect(result).toHaveLength(original.length); + + const changed = original + .map((line, index) => (result[index] === line ? null : index)) + .filter((index): index is number => index !== null); + + expect(changed).toHaveLength(1); + expect(original[changed[0]]).toContain('CUSTOM_STARTER_URL'); + }); + + it('preserves the asset indentation, key spelling and colon spacing', () => { + const compose = readBundledCompose(); + const findKeyLine = (text: string) => + text.split('\n').find((line) => line.includes('CUSTOM_STARTER_URL')) as string; + + const originalLine = findKeyLine(compose); + // Scoped to the single line on purpose: `\s*` matches newlines, so exec'ing the + // multiline regex over the whole file can capture a preceding line break too. + const prefix = ( + /^(\s*["']?CUSTOM_STARTER_URL["']?\s*:\s*)/.exec(originalLine) as RegExpExecArray + )[1]; + + const rewrittenLine = findKeyLine(applyStarterUrl(compose, NEW_STARTER_URL)); + + expect(rewrittenLine).toEqual(`${prefix}"${NEW_STARTER_URL}"`); + }); + + it('leaves the rewritten file still matching the regex, so a re-run is possible', () => { + const compose = readBundledCompose(); + + const once = applyStarterUrl(compose, NEW_STARTER_URL); + const twice = applyStarterUrl(once, 'https://downloads.dotcms.com/starters/other.zip'); + + expect(once).toContain('CUSTOM_STARTER_URL'); + expect(CUSTOM_STARTER_URL_LINE.test(once)).toBe(true); + expect(twice).toContain('"https://downloads.dotcms.com/starters/other.zip"'); + expect(twice).not.toContain(NEW_STARTER_URL); + }); + + it('keeps the value on a single line — a YAML block scalar would silently corrupt it', () => { + // Documented hazard, and the reason the asset must keep CUSTOM_STARTER_URL on one + // line: with a block scalar the regex matches the `>-` marker, the URL is written + // over it, and the continuation line is orphaned into invalid YAML. No throw, no + // warning — the user just gets the wrong starter. + const blockScalar = [ + ' CUSTOM_STARTER_URL: >-', + ' https://old.example.com/starter.zip' + ].join('\n'); + + const corrupted = applyStarterUrl(blockScalar, NEW_STARTER_URL); + + expect(corrupted).toContain(`CUSTOM_STARTER_URL: "${NEW_STARTER_URL}"`); + expect(corrupted).toContain(' https://old.example.com/starter.zip'); + + // The asset itself must therefore never use that shape. + expect(readBundledCompose()).not.toMatch(/CUSTOM_STARTER_URL\s*:\s*[|>]/); + }); + + // `String.replace` with a replacement STRING expands `$1`, `$&`, `` $` ``, `$'` and + // `$$`. Passing the URL through that expanded those sequences instead of writing them, + // so a starter URL containing `$` was silently rewritten into a different URL — no + // throw, no warning, wrong starter. A replacer function takes the value verbatim. + it.each([ + ['$1', 'https://example.com/starter$1.zip'], + ['$&', 'https://example.com/starter$&.zip'], + ['$$', 'https://example.com/starter$$.zip'], + ["$'", "https://example.com/starter$'.zip"], + ['a signed URL', 'https://example.com/s.zip?sig=ab$1cd&x=$&'] + ])('writes a URL containing %s verbatim', (_label, url) => { + const rewritten = applyStarterUrl(readBundledCompose(), url); + + expect(rewritten).toContain(`"${url}"`); + }); + }); +}); diff --git a/core-web/libs/sdk/create-app/src/utils/starter-url.ts b/core-web/libs/sdk/create-app/src/utils/starter-url.ts new file mode 100644 index 000000000000..6e9dd322fc16 --- /dev/null +++ b/core-web/libs/sdk/create-app/src/utils/starter-url.ts @@ -0,0 +1,45 @@ +/** + * Rewrite of the `CUSTOM_STARTER_URL` line in a docker-compose file. + * + * This lives in its own module — split out of `updateDockerComposeStarterUrl` in + * `src/index.ts` — purely so it can be pinned by a Jest spec (`starter-url.spec.ts`). + * The transformation is a single regex against a hand-maintained YAML asset: a harmless + * looking reformat of that one line (a block scalar, or the `- KEY=value` env-list form) + * silently breaks `--starter` for every already-installed CLI, with no error at install + * time. Keeping it pure — string in, string out, no `fs` — is what lets the spec run the + * real bundled asset through it on every build. See dotCMS issue #37262, AC-012. + */ + +/** + * Matches the `CUSTOM_STARTER_URL` mapping line, capturing everything up to and including + * the colon and its trailing whitespace so the author's indentation and key quoting survive + * the rewrite. Deliberately not global: only the first entry is rewritten. + */ +const CUSTOM_STARTER_URL_LINE = /^(\s*["']?CUSTOM_STARTER_URL["']?\s*:\s*).+$/m; + +/** + * Replaces the value of the first `CUSTOM_STARTER_URL` entry with `starterUrl`. + * + * @param composeContents contents of a docker-compose file + * @param starterUrl the starter URL to write in, as passed to `--starter` + * @returns the compose contents with that one line rewritten + * @throws if no `CUSTOM_STARTER_URL` mapping entry is present — an empty file and the + * `- CUSTOM_STARTER_URL=value` env-list form both fail loudly rather than + * silently dropping the user's `--starter` value. + */ +export function applyStarterUrl(composeContents: string, starterUrl: string): string { + if (!CUSTOM_STARTER_URL_LINE.test(composeContents)) { + throw new Error( + 'CUSTOM_STARTER_URL entry not found in docker-compose.yml. Unable to apply --starter value.' + ); + } + + // A replacer FUNCTION, not a replacement string: in a string, `$1`/`$&`/`$'` are + // substitution patterns, so any `$` in the user's --starter URL was rewritten into + // something else entirely — silently, which is the one failure mode this module exists + // to prevent. A function receives the URL verbatim. + return composeContents.replace( + CUSTOM_STARTER_URL_LINE, + (_match, prefix: string) => `${prefix}"${starterUrl}"` + ); +} diff --git a/core-web/libs/sdk/create-app/src/uve/configure-uve.spec.ts b/core-web/libs/sdk/create-app/src/uve/configure-uve.spec.ts new file mode 100644 index 000000000000..d3130eb5eef1 --- /dev/null +++ b/core-web/libs/sdk/create-app/src/uve/configure-uve.spec.ts @@ -0,0 +1,526 @@ +/** + * Contract spec for `src/uve/configure-uve.ts` (tasks T019/T020/T021, dotCMS #37262). + * + * Written BEFORE the implementation, so this file DEFINES the API the implementation must + * satisfy. The module does not exist yet — the failing import is the deliberate Red state + * of TDD. + * + * --------------------------------------------------------------------------------------- + * API PINNED BY THIS SPEC + * --------------------------------------------------------------------------------------- + * + * export type UveMode = 'local' | 'remote'; + * + * export interface ConfigureUveOptions { + * host: string; // e.g. 'http://localhost:8082' — no trailing slash + * siteId: string; // resolved default site identifier + * token: string; // API token, sent as `Authorization: Bearer ` + * mode: UveMode; // 'local' = CLI-owned Docker stack, 'remote' = --dotcms-url + * frontendUrl: string; // value written into configuration.value + * maxRetries?: number; // POST attempts, spent on 5xx only + * retryDelayMs?: number; // backoff between 5xx retries; 0 in tests + * report?: (message: string) => void; // X4: caller-supplied reporter, not console.log + * } + * + * export type UveFailurePhase = 'probe' | 'write'; + * export type UveFailureReason = 'forbidden' | 'server-error' | 'unreachable' | 'unknown'; + * + * export type UveOutcome = + * | { readonly kind: 'configured' } + * | { + * readonly kind: 'failed'; + * readonly phase: UveFailurePhase; + * readonly reason: UveFailureReason; + * readonly status: number | null; // HTTP status; null for transport failures + * readonly message: string; // ready-to-print, MODE-DEPENDENT + * }; + * + * export function configureUVE(options: ConfigureUveOptions): Promise; + * + * Why a `kind` discriminant rather than `Result`: `Err()` is `{ok: false, val}`, which + * is TRUTHY — exactly the trap contract X7 documents at `src/index.ts:597`, where + * `if (!result)` never fires. `outcome.kind === 'configured'` cannot be misread that way, and + * the caller sets `RunState.uveConfigured` from it. + * + * Why a STRING discriminant specifically: this workspace compiles with `strict: false` + * (`tsconfig.base.json`), and without `strictNullChecks` TypeScript does not narrow a union + * on a boolean-literal discriminant — `{configured: true} | {configured: false, message}` + * leaves `outcome.message` unreachable at every call site. A string literal narrows + * correctly, which is also why `ComposeSource` (same feature) discriminates on `kind`. + * + * `frontendUrl` is a fifth field beyond the four the contract text abbreviates + * (`{ host, siteId, token, mode }`): the POST body cannot be built without it. Both existing + * call sites already compute it as `http://localhost:${getPortByFramework(selectedFramework)}`. + * It is the RAW origin — `configureUVE` wraps it with `getUVEConfigValue` itself, so the + * serialized shape the endpoint expects lives in one place rather than at every call site. + * + * --------------------------------------------------------------------------------------- + * BEHAVIOUR PINNED (contract X2 + X3, acceptance criteria AC-003 + AC-005) + * --------------------------------------------------------------------------------------- + * + * 1. NON-FATAL (X2). No path calls `process.exit`. Asserted directly against a spy, for a + * 403, a 500 and a network error. This module replaces the two duplicated fatal blocks + * at `src/index.ts:226-228` and `src/index.ts:369-371`. + * 2. PROBE ONCE (X3). Exactly one `GET` of `/api/v1/apps/dotema-config-v2/` + * precedes the `POST` to the same resource. It is a probe, NOT a poll — one call, even + * when it fails. A failed probe means no `POST` at all. + * 3. RETRY 5xx ONLY (X3). Never retries a `403` or any other `4xx`. Measured: after an + * interrupted starter import the endpoint returned 403 on 193 consecutive attempts over + * ~7 minutes with zero successes — a poll would spin forever. + * 4. MODE-DEPENDENT 403 MESSAGE (X3). `'local'` = the bricked first boot: unrecoverable, + * recreate with `docker compose down -v`, reference #37268, and DO NOT offer the manual + * UVE setup guide (it fails identically). `'remote'` = the user's own server: there is + * no stack to recreate, so `docker compose down -v` MUST NEVER be emitted; name the + * site ID and the app key `dotema-config-v2`, and DO offer the manual guide. + * + * Contract: specs/37262-create-app-docker-uve/contracts/cli-exit-contract.md — X2, X3. + * Data model: specs/37262-create-app-docker-uve/data-model.md — §3 `UVEAppConfig`. + */ + +import { configureUVE } from './configure-uve'; + +import { getUVEConfigValue } from '../utils'; +import { HttpError, httpGet, httpPost } from '../utils/http'; + +// The CLI dropped axios for native fetch (utils/http.ts) after semgrep flagged the +// Proxy-Authorization redirect leak. The http module is mocked rather than `fetch` itself so +// these cases stay about configureUVE's CONTRACT — probe once, retry 5xx only, mode-dependent +// guidance — while http.spec.ts covers the transport. HttpError stays real, because the +// outcome's `status` is derived from it. +jest.mock('../utils/http', () => { + const actual = jest.requireActual('../utils/http'); + + return { ...actual, httpGet: jest.fn(), httpPost: jest.fn() }; +}); + +const mockedHttp = { get: httpGet as jest.Mock, post: httpPost as jest.Mock }; + +const HOST = 'http://localhost:8082'; +const SITE_ID = '48190c8c-42c4-46af-8d1a-0cd5db894797'; +const TOKEN = 'eyJhbGciOiJIUzI1NiJ9.test-token.signature'; +const FRONTEND_URL = 'http://localhost:3000'; + +/** The app key is part of the resource path AND of the remote-mode guidance. */ +const UVE_APP_KEY = 'dotema-config-v2'; +/** Stable slug of the headless UVE guide the CLI links as the "manual steps" (X2.2). */ +const MANUAL_STEPS_SLUG = 'uve-headless-config'; + +type Outcome = Awaited>; + +const baseOptions = { + host: HOST, + siteId: SITE_ID, + token: TOKEN, + frontendUrl: FRONTEND_URL, + // Keep the 5xx retry loop instantaneous; the delay itself is not under test. + retryDelayMs: 0, + maxRetries: 3 +}; + +/** A non-2xx, as utils/http throws it. */ +function httpError(status: number, statusText = 'Error') { + return new HttpError(`Request failed with status code ${status}`, { status, statusText }); +} + +/** A transport-level failure: no HTTP response at all. */ +function networkError(code = 'ECONNREFUSED') { + return new HttpError(`connect ${code} 127.0.0.1:8082`, { status: null, code }); +} + +function okResponse(data: unknown = { entity: 'Ok' }) { + return { status: 200, data }; +} + +/** Strip chalk styling so message assertions are about words, not escape codes. */ +// eslint-disable-next-line no-control-regex +const ANSI_PATTERN = /\u001b\[[0-9;]*m/g; + +/** + * Everything the user could possibly see for this outcome: the returned message plus + * anything the module reported or printed. Negative assertions ("`down -v` never appears in + * remote output") are only meaningful over the union — a leak through `console.warn` is as + * wrong as one through `message`. + */ +function visibleOutput(outcome: Outcome, reported: string[], printed: string[]): string { + const message = outcome.kind === 'failed' ? outcome.message : ''; + + return [message, ...reported, ...printed].join('\n').replace(ANSI_PATTERN, ''); +} + +function expectFailure(outcome: Outcome) { + if (outcome.kind !== 'failed') { + throw new Error('expected configureUVE to report failure, but it reported success'); + } + + return outcome; +} + +describe('configureUVE', () => { + let exitSpy: jest.SpyInstance; + let logSpy: jest.SpyInstance; + let warnSpy: jest.SpyInstance; + let errorSpy: jest.SpyInstance; + let reported: string[]; + let printed: string[]; + + const report = (message: string) => { + reported.push(message); + }; + + beforeEach(() => { + mockedHttp.get.mockReset(); + mockedHttp.post.mockReset(); + + reported = []; + printed = []; + + exitSpy = jest.spyOn(process, 'exit').mockImplementation(((code?: number) => { + throw new Error( + `configureUVE called process.exit(${code}) — contract X2 forbids it on every path` + ); + }) as never); + + const capture = (...args: unknown[]) => { + printed.push(args.map(String).join(' ')); + }; + + logSpy = jest.spyOn(console, 'log').mockImplementation(capture); + warnSpy = jest.spyOn(console, 'warn').mockImplementation(capture); + errorSpy = jest.spyOn(console, 'error').mockImplementation(capture); + }); + + afterEach(() => { + exitSpy.mockRestore(); + logSpy.mockRestore(); + warnSpy.mockRestore(); + errorSpy.mockRestore(); + }); + + describe('X2 — the UVE step is non-fatal: it never calls process.exit', () => { + it('does not exit on a 403 in mode "local"', async () => { + mockedHttp.get.mockResolvedValue(okResponse()); + mockedHttp.post.mockRejectedValue(httpError(403, 'Forbidden')); + + const outcome = await configureUVE({ ...baseOptions, mode: 'local', report }); + + expect(exitSpy).not.toHaveBeenCalled(); + expect(expectFailure(outcome).reason).toBe('forbidden'); + }); + + it('does not exit on a 403 in mode "remote"', async () => { + mockedHttp.get.mockResolvedValue(okResponse()); + mockedHttp.post.mockRejectedValue(httpError(403, 'Forbidden')); + + const outcome = await configureUVE({ ...baseOptions, mode: 'remote', report }); + + expect(exitSpy).not.toHaveBeenCalled(); + expect(expectFailure(outcome).reason).toBe('forbidden'); + }); + + it('does not exit on a 500 once the retry budget is exhausted', async () => { + mockedHttp.get.mockResolvedValue(okResponse()); + mockedHttp.post.mockRejectedValue(httpError(500, 'Internal Server Error')); + + const outcome = await configureUVE({ ...baseOptions, mode: 'local', report }); + + expect(exitSpy).not.toHaveBeenCalled(); + expect(expectFailure(outcome).reason).toBe('server-error'); + }); + + it('does not exit on a network error', async () => { + mockedHttp.get.mockRejectedValue(networkError()); + + const outcome = await configureUVE({ ...baseOptions, mode: 'remote', report }); + + expect(exitSpy).not.toHaveBeenCalled(); + + const failure = expectFailure(outcome); + expect(failure.reason).toBe('unreachable'); + expect(failure.status).toBeNull(); + }); + + it('does not throw — the caller warns and continues to scaffolding', async () => { + mockedHttp.get.mockResolvedValue(okResponse()); + mockedHttp.post.mockRejectedValue(httpError(403, 'Forbidden')); + + await expect( + configureUVE({ ...baseOptions, mode: 'local', report }) + ).resolves.toBeDefined(); + }); + + it("reports failure through kind === 'failed', not a truthy Err", async () => { + mockedHttp.get.mockResolvedValue(okResponse()); + mockedHttp.post.mockRejectedValue(httpError(403, 'Forbidden')); + + const outcome = await configureUVE({ ...baseOptions, mode: 'local', report }); + + // X7's trap: `Err()` is `{ok: false, val}` — truthy — so `if (!outcome)` would + // silently treat this failure as a success. The discriminant must be a field. + expect(Boolean(outcome)).toBe(true); + expect(outcome.kind).toBe('failed'); + }); + + it('carries a non-empty, printable message on every failure', async () => { + mockedHttp.get.mockResolvedValue(okResponse()); + mockedHttp.post.mockRejectedValue(httpError(500, 'Internal Server Error')); + + const failure = expectFailure( + await configureUVE({ ...baseOptions, mode: 'local', report }) + ); + + expect(typeof failure.message).toBe('string'); + expect(failure.message.length).toBeGreaterThan(0); + }); + }); + + describe('X3 — the GET is a single probe, not a poll', () => { + it('probes the UVE app resource exactly once before writing', async () => { + mockedHttp.get.mockResolvedValue(okResponse()); + mockedHttp.post.mockResolvedValue(okResponse()); + + await configureUVE({ ...baseOptions, mode: 'local', report }); + + expect(mockedHttp.get).toHaveBeenCalledTimes(1); + + const [probeUrl] = mockedHttp.get.mock.calls[0]; + expect(probeUrl).toContain(HOST); + expect(probeUrl).toContain(UVE_APP_KEY); + expect(probeUrl).toContain(SITE_ID); + }); + + it('writes to the same resource it probed', async () => { + mockedHttp.get.mockResolvedValue(okResponse()); + mockedHttp.post.mockResolvedValue(okResponse()); + + await configureUVE({ ...baseOptions, mode: 'local', report }); + + const [probeUrl] = mockedHttp.get.mock.calls[0]; + const [writeUrl, body] = mockedHttp.post.mock.calls[0]; + + expect(writeUrl).toBe(probeUrl); + // NOT a bare `FRONTEND_URL`. The endpoint takes the serialized UVE config object + // that `getUVEConfigValue` produces; posting the raw origin is accepted with a 200 + // and silently leaves the editor misconfigured — a green signal that means nothing, + // which is the same failure shape as the bug this issue is about. + expect(body).toEqual({ + configuration: { hidden: false, value: getUVEConfigValue(FRONTEND_URL) } + }); + }); + + it('authenticates both calls with the run token', async () => { + mockedHttp.get.mockResolvedValue(okResponse()); + mockedHttp.post.mockResolvedValue(okResponse()); + + await configureUVE({ ...baseOptions, mode: 'local', report }); + + // The `Authorization: Bearer` header itself is http.spec.ts's business; what matters + // here is that configureUVE passes the run's token to BOTH calls, not just the write. + const [, getOptions] = mockedHttp.get.mock.calls[0]; + const [, , postOptions] = mockedHttp.post.mock.calls[0]; + + expect(getOptions?.token).toBe(TOKEN); + expect(postOptions?.token).toBe(TOKEN); + }); + + it('proceeds to the POST when the probe returns 200', async () => { + mockedHttp.get.mockResolvedValue(okResponse()); + mockedHttp.post.mockResolvedValue(okResponse()); + + const outcome = await configureUVE({ ...baseOptions, mode: 'local', report }); + + expect(mockedHttp.post).toHaveBeenCalledTimes(1); + expect(outcome.kind).toBe('configured'); + }); + + it('never POSTs when the probe is forbidden, and never re-probes', async () => { + mockedHttp.get.mockRejectedValue(httpError(403, 'Forbidden')); + + const outcome = await configureUVE({ ...baseOptions, mode: 'local', report }); + + expect(mockedHttp.get).toHaveBeenCalledTimes(1); + expect(mockedHttp.post).not.toHaveBeenCalled(); + + const failure = expectFailure(outcome); + expect(failure.phase).toBe('probe'); + expect(failure.reason).toBe('forbidden'); + expect(failure.status).toBe(403); + }); + + it('never polls the probe on a network error', async () => { + mockedHttp.get.mockRejectedValue(networkError()); + + await configureUVE({ ...baseOptions, mode: 'local', report }); + + expect(mockedHttp.get).toHaveBeenCalledTimes(1); + expect(mockedHttp.post).not.toHaveBeenCalled(); + }); + }); + + describe('X3 — the POST retries on 5xx only', () => { + it('retries a 500 and succeeds on the second attempt', async () => { + mockedHttp.get.mockResolvedValue(okResponse()); + mockedHttp.post + .mockRejectedValueOnce(httpError(500, 'Internal Server Error')) + .mockResolvedValueOnce(okResponse()); + + const outcome = await configureUVE({ ...baseOptions, mode: 'local', report }); + + expect(mockedHttp.post).toHaveBeenCalledTimes(2); + // Still a single probe — the retry re-POSTs, it does not re-GET. + expect(mockedHttp.get).toHaveBeenCalledTimes(1); + expect(outcome.kind).toBe('configured'); + }); + + it('gives up after the retry budget on a persistent 5xx', async () => { + mockedHttp.get.mockResolvedValue(okResponse()); + mockedHttp.post.mockRejectedValue(httpError(503, 'Service Unavailable')); + + const outcome = await configureUVE({ + ...baseOptions, + mode: 'local', + maxRetries: 3, + report + }); + + expect(mockedHttp.post).toHaveBeenCalledTimes(3); + + const failure = expectFailure(outcome); + expect(failure.phase).toBe('write'); + expect(failure.reason).toBe('server-error'); + expect(failure.status).toBe(503); + }); + + it('NEVER retries a 403 — the second attempt is never made', async () => { + mockedHttp.get.mockResolvedValue(okResponse()); + // If the implementation retried, this queued 200 would turn the run green and + // hide the defect. A 403 here is terminal: measured at 193 consecutive + // failures over ~7 minutes with zero successes. + mockedHttp.post + .mockRejectedValueOnce(httpError(403, 'Forbidden')) + .mockResolvedValueOnce(okResponse()); + + const outcome = await configureUVE({ ...baseOptions, mode: 'local', report }); + + expect(mockedHttp.post).toHaveBeenCalledTimes(1); + expect(outcome.kind).toBe('failed'); + expect(expectFailure(outcome).status).toBe(403); + }); + + it('does not retry any other 4xx either', async () => { + for (const status of [400, 401, 404, 422]) { + mockedHttp.get.mockReset(); + mockedHttp.post.mockReset(); + mockedHttp.get.mockResolvedValue(okResponse()); + mockedHttp.post + .mockRejectedValueOnce(httpError(status)) + .mockResolvedValueOnce(okResponse()); + + const outcome = await configureUVE({ ...baseOptions, mode: 'local', report }); + + expect(mockedHttp.post).toHaveBeenCalledTimes(1); + expect(outcome.kind).toBe('failed'); + } + + expect(exitSpy).not.toHaveBeenCalled(); + }); + }); + + describe('X3 — a 403 in mode "local" means the instance is unrecoverable', () => { + let output: string; + + beforeEach(async () => { + mockedHttp.get.mockResolvedValue(okResponse()); + mockedHttp.post.mockRejectedValue(httpError(403, 'Forbidden')); + + const outcome = await configureUVE({ ...baseOptions, mode: 'local', report }); + output = visibleOutput(outcome, reported, printed); + }); + + it('tells the user to recreate the stack with `docker compose down -v`', () => { + expect(output).toMatch(/docker\s+compose\s+down\s+-v/); + }); + + it('says the instance cannot be repaired in place', () => { + expect(output).toMatch(/unrecoverable|recreate|re-create|start over|from scratch/i); + }); + + it('references the backend defect #37268', () => { + expect(output).toContain('37268'); + }); + + it('does NOT offer the manual UVE setup steps — they fail identically', () => { + expect(output).not.toContain(MANUAL_STEPS_SLUG); + expect(output).not.toMatch(/dev\.dotcms\.com/); + }); + }); + + describe('X3 — a 403 in mode "remote" is a token permission problem', () => { + let output: string; + + beforeEach(async () => { + mockedHttp.get.mockResolvedValue(okResponse()); + mockedHttp.post.mockRejectedValue(httpError(403, 'Forbidden')); + + const outcome = await configureUVE({ ...baseOptions, mode: 'remote', report }); + output = visibleOutput(outcome, reported, printed); + }); + + it('reports that the API token lacks permission on the resolved site', () => { + expect(output).toMatch(/permission/i); + expect(output).toMatch(/token/i); + }); + + it('names the site ID and the app key so the fix is actionable', () => { + expect(output).toContain(SITE_ID); + expect(output).toContain(UVE_APP_KEY); + }); + + it('DOES offer the manual UVE setup steps — on this path they work', () => { + expect(output).toContain(MANUAL_STEPS_SLUG); + }); + + it('NEVER suggests `docker compose down -v` — there is no stack to recreate', () => { + expect(output).not.toMatch(/down\s+-v/); + expect(output).not.toMatch(/docker/i); + }); + + it('does not reference #37268 — that defect is about the local first boot', () => { + expect(output).not.toContain('37268'); + }); + }); + + describe('success', () => { + it('reports success when the probe is 200 and the POST succeeds', async () => { + mockedHttp.get.mockResolvedValue(okResponse()); + mockedHttp.post.mockResolvedValue(okResponse()); + + const outcome = await configureUVE({ ...baseOptions, mode: 'local', report }); + + expect(outcome.kind).toBe('configured'); + expect(outcome).not.toHaveProperty('reason'); + expect(exitSpy).not.toHaveBeenCalled(); + }); + + it('reports success in mode "remote" the same way', async () => { + mockedHttp.get.mockResolvedValue(okResponse()); + mockedHttp.post.mockResolvedValue(okResponse()); + + const outcome = await configureUVE({ ...baseOptions, mode: 'remote', report }); + + expect(outcome.kind).toBe('configured'); + expect(mockedHttp.get).toHaveBeenCalledTimes(1); + expect(mockedHttp.post).toHaveBeenCalledTimes(1); + }); + + it('emits no failure guidance on the happy path', async () => { + mockedHttp.get.mockResolvedValue(okResponse()); + mockedHttp.post.mockResolvedValue(okResponse()); + + const outcome = await configureUVE({ ...baseOptions, mode: 'local', report }); + const output = visibleOutput(outcome, reported, printed); + + expect(output).not.toMatch(/down\s+-v/); + expect(output).not.toContain('37268'); + }); + }); +}); diff --git a/core-web/libs/sdk/create-app/src/uve/configure-uve.ts b/core-web/libs/sdk/create-app/src/uve/configure-uve.ts new file mode 100644 index 000000000000..dfac405bab1f --- /dev/null +++ b/core-web/libs/sdk/create-app/src/uve/configure-uve.ts @@ -0,0 +1,263 @@ +import { getUVEConfigValue } from '../utils'; +import { httpGet, httpPost } from '../utils/http'; + +/** + * Single owner of Universal Visual Editor configuration. + * + * This replaces two byte-for-byte identical fatal blocks — `src/index.ts:226-228` (the + * existing-instance path) and `src/index.ts:369-371` (the local-Docker path) — each of which + * called `process.exit(1)` when the UVE call failed. Because UVE setup runs *before* + * scaffolding, that exit left the user with an empty directory and threw away a working API + * token and site ID that had already been obtained. See issue #37262. + * + * Nothing here calls `process.exit` and nothing here throws. Failure is a return value the + * caller warns on and continues past. + */ + +/** Which entry path the run came in on. It decides what a 403 *means*, and so what to advise. */ +export type UveMode = 'local' | 'remote'; + +export interface ConfigureUveOptions { + /** Instance base URL, no trailing slash — e.g. `http://localhost:8082`. */ + host: string; + siteId: string; + /** API token, sent as `Authorization: Bearer `. */ + token: string; + mode: UveMode; + /** The front-end origin the editor should load — e.g. `http://localhost:3000`. */ + frontendUrl: string; + /** POST attempts. Spent on 5xx only. */ + maxRetries?: number; + retryDelayMs?: number; + /** + * Caller-supplied reporter. Contract X4: this module must not `console.log` directly, + * because retry chatter interleaving with an active `ora` spinner is what produced the + * mangled output in the original report. + */ + report?: (message: string) => void; +} + +export type UveFailurePhase = 'probe' | 'write'; +export type UveFailureReason = 'forbidden' | 'server-error' | 'unreachable' | 'unknown'; + +export type UveOutcome = + | { readonly kind: 'configured' } + | { + readonly kind: 'failed'; + readonly phase: UveFailurePhase; + readonly reason: UveFailureReason; + /** HTTP status, or `null` when the request never got a response. */ + readonly status: number | null; + readonly message: string; + }; + +/** App key of the UVE configuration app. Part of the resource path and of remote guidance. */ +const UVE_APP_KEY = 'dotema-config-v2'; + +const HEADLESS_UVE_GUIDE = + 'https://dev.dotcms.com/docs/author/pages-and-visual-editing/universal-visual-editor/uve-headless-config'; + +const DEFAULT_MAX_RETRIES = 3; +const DEFAULT_RETRY_DELAY_MS = 2000; + +function uveResourceUrl(host: string, siteId: string): string { + return `${host.replace(/\/+$/, '')}/api/v1/apps/${UVE_APP_KEY}/${siteId}`; +} + +function statusOf(error: unknown): number | null { + const response = (error as { response?: { status?: number } })?.response; + + return typeof response?.status === 'number' ? response.status : null; +} + +function reasonFor(status: number | null): UveFailureReason { + if (status === null) { + return 'unreachable'; + } + + if (status === 403) { + return 'forbidden'; + } + + if (status >= 500) { + return 'server-error'; + } + + return 'unknown'; +} + +/** + * A 403 is terminal, and what to do about it depends entirely on which stack you are talking to. + * + * On the CLI's own Docker stack it means an interrupted first boot never wrote the site's + * permission rows: measured, the endpoint returned 403 on 193 consecutive attempts over ~7 + * minutes with zero successes, and configuring UVE by hand fails for exactly the same reason. + * The only fix is to recreate the instance. + * + * On a server the user supplied there is no stack to recreate, and telling them to run + * `docker compose down -v` would be actively wrong — destructive advice aimed at the wrong + * machine. There it is an ordinary permissions problem and the manual steps do work. + * + * Do not merge these two messages. + */ +function forbiddenMessage(mode: UveMode, siteId: string): string { + if (mode === 'local') { + return [ + 'The Universal Visual Editor could not be configured: the instance rejected the request (403).', + '', + 'This local instance is unrecoverable. Its first boot was interrupted, so the starter', + 'import never wrote the permission rows for the demo site, and a restart does not repair', + 'them. Configuring the editor by hand would fail the same way.', + '', + 'Recreate the instance from scratch:', + ' docker compose down -v && docker compose up -d --wait', + '', + 'Tracked as dotCMS issue #37268.' + ].join('\n'); + } + + return [ + 'The Universal Visual Editor could not be configured: the instance rejected the request (403).', + '', + 'The API token does not have permission to write app configuration on the target site.', + ` site id : ${siteId}`, + ` app key : ${UVE_APP_KEY}`, + '', + 'Check that the token belongs to a user who can administer that site, then finish the', + 'setup by hand:', + ` ${HEADLESS_UVE_GUIDE}` + ].join('\n'); +} + +function genericMessage( + mode: UveMode, + siteId: string, + detail: string, + { withGuide }: { withGuide: boolean } +): string { + const lines = [ + `The Universal Visual Editor could not be configured: ${detail}`, + '', + 'Your project is unaffected and setup will continue. To finish the editor configuration', + 'later, use these values:', + ` site id : ${siteId}`, + ` app key : ${UVE_APP_KEY}` + ]; + + // The guide is manual-setup advice, so it is withheld in exactly the case where manual + // setup cannot work (local 403). Everything else is self-serviceable. + if (withGuide) { + lines.push('', ` ${HEADLESS_UVE_GUIDE}`); + } + + void mode; + + return lines.join('\n'); +} + +function delay(ms: number): Promise { + return ms > 0 ? new Promise((resolve) => setTimeout(resolve, ms)) : Promise.resolve(); +} + +export async function configureUVE(options: ConfigureUveOptions): Promise { + const { + host, + siteId, + token, + mode, + frontendUrl, + maxRetries = DEFAULT_MAX_RETRIES, + retryDelayMs = DEFAULT_RETRY_DELAY_MS, + report + } = options; + + const url = uveResourceUrl(host, siteId); + const notify = (message: string) => report?.(message); + + // Read before write, exactly once. A poll would be wrong here: the failure this guards + // against never clears, so waiting for it to clear never terminates (contract X3). + try { + await httpGet(url, { token }); + } catch (error) { + const status = statusOf(error); + const reason = reasonFor(status); + const message = + reason === 'forbidden' + ? forbiddenMessage(mode, siteId) + : genericMessage( + mode, + siteId, + status === null + ? 'the instance could not be reached.' + : `the instance answered ${status} when the current configuration was read.`, + { withGuide: true } + ); + + notify(message); + + return { kind: 'failed', phase: 'probe', reason, status, message }; + } + + const payload = { + configuration: { + hidden: false, + // The endpoint expects the serialized UVE config object, not a bare URL. Building + // it here rather than at the call sites is the point of this module owning the + // operation — a caller passing the raw origin would be accepted with a 200 and + // silently leave the editor misconfigured. + value: getUVEConfigValue(frontendUrl) + } + }; + + let lastStatus: number | null = null; + + for (let attempt = 1; attempt <= Math.max(1, maxRetries); attempt++) { + try { + await httpPost(url, payload, { token }); + + return { kind: 'configured' }; + } catch (error) { + const status = statusOf(error); + lastStatus = status; + + // Only 5xx is genuinely transient. 4xx — 403 above all — is a decision, not a + // hiccup, and retrying it just burns the user's time before the same answer. + const retryable = status !== null && status >= 500 && attempt < Math.max(1, maxRetries); + + if (!retryable) { + const reason = reasonFor(status); + const message = + reason === 'forbidden' + ? forbiddenMessage(mode, siteId) + : genericMessage( + mode, + siteId, + status === null + ? 'the instance could not be reached.' + : `the instance answered ${status}.`, + { withGuide: true } + ); + + notify(message); + + return { kind: 'failed', phase: 'write', reason, status, message }; + } + + notify(`dotCMS answered ${status}; retrying (${attempt}/${maxRetries})`); + await delay(retryDelayMs); + } + } + + /* istanbul ignore next -- the loop above always returns; this satisfies the type checker. */ + const message = genericMessage(mode, siteId, `the instance answered ${lastStatus}.`, { + withGuide: true + }); + + return { + kind: 'failed', + phase: 'write', + reason: reasonFor(lastStatus), + status: lastStatus, + message + }; +} diff --git a/core-web/libs/sdk/create-app/tsconfig.spec.json b/core-web/libs/sdk/create-app/tsconfig.spec.json index 06c7e9af9b69..9350d0a4fa76 100644 --- a/core-web/libs/sdk/create-app/tsconfig.spec.json +++ b/core-web/libs/sdk/create-app/tsconfig.spec.json @@ -3,7 +3,6 @@ "compilerOptions": { "outDir": "../../../dist/out-tsc", "module": "commonjs", - "moduleResolution": "node10", "types": ["jest", "node"] }, "include": ["jest.config.ts", "src/**/*.test.ts", "src/**/*.spec.ts", "src/**/*.d.ts"] diff --git a/specs/37262-create-app-docker-uve/cli-design-decisions.md b/specs/37262-create-app-docker-uve/cli-design-decisions.md new file mode 100644 index 000000000000..909fa2bf1e55 --- /dev/null +++ b/specs/37262-create-app-docker-uve/cli-design-decisions.md @@ -0,0 +1,351 @@ +# CLI design decisions — open before implementation + +**Feature**: `37262-create-app-docker-uve` · **Blocks**: US2, US3, US4 (not US1) +**Status**: **ALL RESOLVED** 2026-08-28 · **Raised**: 2026-08-28 + +The contracts in [contracts/cli-exit-contract.md](./contracts/cli-exit-contract.md) state what the +CLI must *guarantee*. They did not state *how*. D1–D3 were raised before implementation; D4–D8 came +out of answering them. **All are now decided** — see the summary table at the end. + +The largest change is **D4**: the CLI now ships its **own** compose file instead of editing the +shared `single-node-demo-site` example. That removes the biggest risk in the whole plan. + +--- + +## D1 — How is "no successful state is ever discarded" actually implemented? + +**Contract X1 today says**: *"Implemented as an emit step that runs from a `finally`-equivalent +position, not from the happy path."* + +**That is wrong.** `finally` does not run when `process.exit()` is called. Verified: + +```js +process.on('exit', (c) => console.log(`[process.on(exit)] RAN (code=${c})`)); +try { process.exit(3); } finally { console.log('[finally] RAN'); } +``` +``` + in try + [process.on(exit)] RAN (code=3) ← only this + exit code: 3 +``` + +And there are **17 `process.exit(1)` call sites** — `src/asks.ts:221`, and sixteen in +`src/index.ts`, of which **13 sit inside the single `try` opened at `index.ts:93`**. So the current +wording cannot be implemented against the current control flow. + +### Option A — `process.on('exit')` backstop *(recommended)* + +Register one handler that prints `host`/`token`/`siteId` and writes `.env` from `RunState`. + +- **Cost**: one new call site. No existing `process.exit()` has to move. +- **Strongest property**: it also catches exits nobody anticipated — including future ones. The + guarantee stops depending on remembering to route every error path correctly. +- **Constraint**: the handler is synchronous. `.env` must use `writeFileSync`, and it cannot + prompt or await. +- **Risk checked**: stdout truncation on exit. Tested at 200 lines to both a pipe and a file — + **200/200 survived**. The recoverable-state block is ~5 lines, well inside that. + +### Option B — convert inner exits to throws + +Replace the 13 in-`try` `process.exit(1)` calls with `throw new CliError(…)`, caught by the +existing top-level `catch` at `index.ts:388`, which emits and then exits. + +- **Cost**: 13 call sites, touching every error path in the file. +- **Better**: explicit, ordinary control flow; trivially unit-testable; permits async work + (prompts, awaited writes) on the way out. +- **Worse**: a large diff in exactly the code the user is trusting after a bad experience, and a + future `process.exit()` added anywhere silently re-opens the hole Option A closes structurally. + +**Recommendation: Option A everywhere. Option B is not needed at all.** + +An earlier draft said "B for the UVE path", which was imprecise. The UVE site +(`src/index.ts:369-371`) does not need throwing or catching — X2 requires the run to **continue**, +so the `process.exit(1)` there is simply deleted and replaced with ordinary control flow: + +```ts +if (!setUpUVE.ok) { + run.uveConfigured = false; + warnUveFailed(setUpUVE.val, run); // terminal-403 vs other, per contract X3 +} else { + spinner.succeed('Configured the Universal Visual Editor'); +} +// falls through to scaffolding either way +``` + +So the whole change is: one `process.on('exit')` handler, plus deleting one `process.exit(1)`. +No 13-site refactor. + +**Open question for you**: accept the synchronous-handler constraint (`writeFileSync`, no prompt on +the way out)? + +--- + +## D2 — What is the UVE poll budget? — **RESOLVED BY MEASUREMENT: the premise was wrong** + +**Original question**: X3 says "poll `GET` until 200"; for how long? + +**Answer: it does not matter, because the 403 never clears.** Measured 2026-08-28 on an +M5 / 64GB MacBook Pro, dotCMS constrained to 2 CPUs / 4G. + +### Measurement 1 — clean boot: there is no settling window + +| Signal | First success | +|---|---| +| `POST /api/v1/authentication/api-token` | **46s** | +| `GET /api/v1/apps/dotema-config-v2/{site}` | **46s** | +| `/dotmgt/livez`, `/dotmgt/readyz` | 48s | +| `/api/v1/appconfiguration` (current CLI probe) | 49s | + +The UVE endpoint was usable **2 seconds before `readyz` went green**. Server logs show why: +the starter import (T+20s) and the ES reindex (T+44s) both complete *inside* Tomcat startup +(`Server startup in [36517] milliseconds`), and the HTTP connector does not accept traffic until +after them. On a clean boot **nothing answers while the instance is still settling** — so the +race described in the spec's original root cause 2 cannot occur. + +> Hardware caveat: 46s is best-case. A 4-core laptop with a cold image cache will be +> substantially slower, and the ~1.5GB pull is additional. The *ordering* above is structural +> and should hold regardless; the absolute numbers should not be quoted as typical. + +### Measurement 2 — interrupted boot: the 403 reproduces and is PERMANENT + +Reproducing the reporter's actual path — dotCMS killed 25s in (mid `com.dotmarketing.beans.Tree` +import), then hand-started, as in reproduction step 4: + +``` +T+39s appconfiguration 200 — CLI proceeds +T+41s api-token -> 200 +T+41s defaultSite -> 200 +T+41s UVE GET -> 403 UVE POST -> 403 + ... 193 consecutive attempts over ~7 minutes, zero successes ... +T+440s UVE GET -> 403 UVE POST -> 403 +``` + +Server-side cause: + +``` +DotSecurityException: User 'Admin User [ID: dotcms.org.1][email:admin@dotcms.com]' + does not have READ permissions on Site 'demo.dotcms.com' +``` + +The interrupted import left the site's permission rows unwritten. The restart re-ran +`Task00004LoadStarter` and Tomcat started cleanly, but the permissions never appeared. **The +instance does not recover.** Only `docker compose down -v` and a fresh start fixes it. + +### Consequences + +1. **The read-before-write gate (X3 / US3) is the wrong fix.** Polling `GET` until 200 would poll + forever against a condition that never clears. +2. **A poll budget should be SHORT, not long.** Any budget merely adds silence before the same + warning. ~15-30s is generous. +3. **The warning text in X2 must change.** "Configure UVE manually at this URL" is useless advice + here — manual configuration fails identically. The correct message is that the instance is in a + broken state from an interrupted first boot and must be recreated with `docker compose down -v`. +4. **Fixing root cause 1 removes root cause 2.** No crash -> no interrupted import -> no 403. + US1 (compose) is the actual fix; the CLI work is damage limitation for when it happens anyway. +5. **The spec's hypothesised mechanism was wrong** — not license gating, not a transient race, and + not `user.isAdmin()` swallowing an exception via `Try.of(...).getOrElse(false)`. It is missing + permission data. The P2 backend non-goal should be re-pointed accordingly. + +**Recommendation**: replace the poll-until-200 gate with a **single** `GET` probe. On 403, skip the +write and emit the "recreate your instance" guidance. Keep retry only for `5xx`, which is a genuine +transient class. + +**Limits of this evidence**: one host, one starter, one image; kill point fixed at 25s. Which +kill-points corrupt and which do not is unmapped, and *why* a re-run import leaves permissions +missing is a backend question deserving its own issue. + +## D3 — What does "offer to reuse" do when there is no terminal? + +**Contract X6 says**: *"the CLI offers to reuse that instance instead of exiting."* "Offer" implies +an `inquirer` prompt — and prompts are how this CLI asks everything (`src/asks.ts` uses +`inquirer.prompt` in five places). + +**The problem**: there is **no** `--yes`, `--ci`, or non-interactive flag in the option list +(`src/index.ts:71-88`). A prompt in a scripted or CI run has nothing to read from and hangs — which +is a worse failure than the "Required ports are already in use" error we are replacing. + +| Option | Behavior | +|---|---| +| **`process.stdout.isTTY` check** *(recommended)* | Prompt when interactive; auto-reuse with a printed notice when not. No new API surface. | +| Add `--yes` / `--reuse` flag | Explicit and scriptable, but new public CLI surface that must then be documented and supported. | +| Always auto-reuse, never prompt | Simplest, but silently attaches to an instance the user may not have meant to use. | + +A second, smaller question rides along: **how much do we verify before reusing?** Something is +answering on 8082 — but is it a *suitable* dotCMS? Minimum bar should be that the readiness probe +and token issuance both succeed; otherwise treat the port as busy-and-unusable and fail as today. + +**DECISION (Freddy, 2026-08-28): silent auto-reuse on CI only. Otherwise ask, and let the user +stop right there.** + +```ts +const isCI = Boolean(process.env.CI) || !process.stdout.isTTY; + +if (isCI) { + console.log(chalk.yellow('⚠ dotCMS already running on 8082 — reusing it (non-interactive).')); + reuse = true; // decide, never block a scripted run +} else { + reuse = await askReuseOrAbort(); // { Reuse this instance | Abort } +} +``` + +Two points this settles: + +- **The prompt must offer abort, not just reuse.** "Ask" means a real choice — a user who did not + expect a dotCMS on 8082 needs to stop and look, not be pushed forward. +- **Even the CI path prints a notice.** Silent means "no prompt", not "no output": a scripted run + that quietly attaches to an unknown instance is exactly the failure this is meant to avoid. + +**Edge case folded in**: no TTY but no `CI` env var either (a piped local run). Treated as CI — +there is nobody to answer the prompt, so blocking is the worst option. The printed notice is what +makes it recoverable. + +**Still required before reusing**: the instance must pass readiness **and** token issuance. +Something answering on 8082 is not necessarily a usable dotCMS, and adopting a stranger's instance +would wire the user's project to the wrong CMS. + +--- + +## Summary + +| # | Decision | Recommendation | Blocks | +|---|---|---|---| +| D1 | Emit mechanism for X1 | `process.on('exit')` backstop + throw for the UVE path | T027, T028, T029 | +| D2 | UVE poll budget | 60s, named constants | T040 | +| D3 | Reuse when non-interactive | `isTTY`; auto-reuse; verify readiness + token first | T047, T048 | + +Once these are settled I will correct **X1** in the contract — it is currently a guarantee that +cannot be implemented as written — and record D2/D3 alongside X3 and X6. + + +--- + +## D4 — The CLI owns its compose file *(decided: bundle it in the package)* + +**Problem**: the original plan edited `docker/docker-compose-examples/single-node-demo-site/docker-compose.yml`, +which is fetched from `main` at runtime and also used directly by README readers. Every hardening +step we wanted (gate on OpenSearch, publish 8090, healthchecks that `--wait` depends on) was a +behavior change shipped unversioned to consumers who never asked for it — and gating on OpenSearch +in particular introduced a way for dotCMS to **never start** if that probe later broke (e.g. an +`opensearch:1` → `:2` bump invalidating `admin:admin`). + +**Decision**: give the CLI its own compose file, **bundled in the npm package**, and leave the +shared demo example untouched. + +- File ships at `core-web/libs/sdk/create-app/assets/docker-compose.yml`. +- No runtime download — this removes `downloadFile`'s missing timeout, redirect handling and retry, + and the unpinned `main` URL, in one move. +- **Installed CLIs (≤1.2.5) keep fetching the old shared file and are not repaired.** Accepted + knowingly: this tool starts fresh local instances, is not a CI dependency, no known users have it + in CI — and `npx @dotcms/create-app` resolves to the latest published version anyway, so only a + warm npx cache stays behind. + +**Consequence**: the "ships unversioned to every consumer" risk — previously the single largest in +this work — no longer applies. Nothing else reads the CLI's file. + +### D4a — Keep it easy to swap back to remote + +Reading the file is an interface, so bundled and remote are interchangeable: + +```ts +export interface ComposeSource { + readonly describe: string; // shown in diagnostics + read(): Promise; +} + +export const bundledCompose: ComposeSource = { /* fs.readFile of the shipped asset */ }; +export const remoteCompose = (url: string): ComposeSource => ({ /* hardened fetch */ }); + +export function resolveComposeSource(): ComposeSource { + const override = process.env.DOTCMS_COMPOSE_URL; + return override ? remoteCompose(override) : bundledCompose; +} +``` + +The call site obtains **contents** and writes them, rather than downloading straight to disk as +today — that shape change is what makes the sources swappable. `DOTCMS_COMPOSE_URL` allows a +field hotfix with no code change or release. `updateDockerComposeStarterUrl` still rewrites the +file on disk afterwards, so `--starter` is unaffected. + +**Packaging gotchas** (the asset will silently not ship otherwise): `package.json` `files` is +`["*.js", "README.md"]`, and `project.json`'s esbuild `assets` lists only README and package.json. +Both need the compose file added. + +--- + +## D5 — How strict is the CLI's file? *(decided: strict, `start_period: 180s`)* + +Now that nothing else consumes it, strictness costs nothing: + +- `dotcms` `depends_on` gates on **both** `db` and `opensearch` at `condition: service_healthy`. +- OpenSearch probe: `curl -sk https://localhost:9200 -u admin:admin | grep -q cluster_name` + — **verified on this stack, succeeds at 15s**; `curl` is present in the OpenSearch image. +- `dotcms` healthcheck on `http://127.0.0.1:8090/dotmgt/livez`, **`start_period: 180s`** + (~4× the measured ~46s boot; above both precedents — lgtm 120s, metrics-monitoring 20s), plus + `restart: unless-stopped`. Erring high is free: the first successful probe ends the window, so a + 46s boot leaves it at 46s regardless. Erring low is not: the container is marked `unhealthy` and + `docker compose up --wait` **aborts** on an instance that would have been fine. +- **Rejected alternative — a credential-free OpenSearch probe** (accept `200` or `401` from the + HTTP layer, dropping the `admin:admin` coupling). The coupling is this probe's only real + exposure, but it is contained: the image tag is pinned to major `1`, so the `:1 → :2` bump that + would invalidate the default credentials requires a deliberate edit to this very file by whoever + then owns the probe. A proven probe beats an unproven one on the critical path. +- Management port published **loopback-only**: `127.0.0.1:8090:8090` (D-rationale in research R3). + +--- + +## D6 — `.env` filename *(decided: always `.env`)* + +The examples disagree with the CLI today: `nextjs`, `astro` and `nextjs-experiments` ship +`.env.local.example`; `angular-ssr` and `vuejs` ship `.env.example`; `angular` ships neither — while +the CLI tells everyone `touch .env`. + +**Decision: always write `.env`**, regardless of framework. Functionally safe — Next.js and Astro +both read `.env` in addition to `.env.local` — and one filename is simpler to implement and explain. +Written **if absent**; if a file is already there it is left alone and the values are printed +instead. + +--- + +## D7 — `--wait-timeout` and feedback *(decided: 600s, with continuous feedback)* + +`--wait-timeout 600`. Pull time is spent before the container starts, so it does not consume this +budget; a timeout here means something is genuinely wrong. + +**Conditional on continuous UI feedback for the whole wait** — ten minutes of frozen spinner is the +failure this issue was reported for. Requires both: + +1. streaming `docker compose up --wait`'s own per-container `Waiting → Healthy` transitions, which + `execa` currently swallows; and +2. a ticker showing elapsed time and per-service state, polled every ~2s. + +This tightens AC-009 and contract X4: feedback must be continuous, not merely "pull progress +visible". + +--- + +## D8 — Image tag *(decided: keep `latest` for now)* + +The bundled file **could** pin `dotcms/dotcms:` under ADR-0019, since the SDK +version is the dotCMS release version — which would make image/starter drift impossible. + +**Deferred.** `latest` stays for now, so the drift the issue flagged (`latest` paired with a +hardcoded `starter-20260630`) **remains an open risk**, and ADR-0019 alignment is postponed rather +than resolved. Bundling makes this easy to revisit later. + +--- + +## Summary of decisions + +| # | Decision | Outcome | +|---|---|---| +| D1 | Emit mechanism for X1 | `process.on('exit')` handler prints state **and** writes `.env` with `writeFileSync`; the UVE `process.exit(1)` is simply deleted. No 13-site refactor. | +| D2 | UVE poll budget | Moot — a 403 is terminal. Single probe, retry `5xx` only. | +| D3 | Reuse when non-interactive | Silent auto-reuse on CI (or no TTY) **with a printed notice**; otherwise prompt offering **reuse or abort**. Reuse only an instance passing readiness + token issuance. | +| D4 | Compose file ownership | CLI ships its own, bundled; shared demo example untouched; `DOTCMS_COMPOSE_URL` swaps to remote. | +| D5 | Strictness | Gate on both services healthy; `start_period: 180s`; loopback 8090. Credential-free probe rejected — coupling contained by the major-tag pin. | +| D6 | `.env` filename | Always `.env`, write-if-absent. | +| D7 | `--wait-timeout` | 600s, conditional on continuous feedback. | +| D8 | Image tag | `latest` for now; drift risk and ADR-0019 alignment stay open. | + +**Out of scope, confirmed (T066)**: #37268 (interrupted boot bricks the instance), image-tag +pinning / ADR-0019 alignment, and #35096 (E2E suite incl. fault injection). diff --git a/specs/37262-create-app-docker-uve/contracts/cli-exit-contract.md b/specs/37262-create-app-docker-uve/contracts/cli-exit-contract.md new file mode 100644 index 000000000000..48f47ef16070 --- /dev/null +++ b/specs/37262-create-app-docker-uve/contracts/cli-exit-contract.md @@ -0,0 +1,176 @@ +# Contract: `@dotcms/create-app` exit behavior + +**Consumer**: the person running `npx @dotcms/create-app`, and any script wrapping it. + +The reported failure is a contract violation: the CLI held a working token and site ID and exited +without printing either. These are the guarantees the fix establishes. + +--- + +## X1 — No successful state is ever discarded + +> Once `token` and `siteId` are non-null, **every** terminal path — success, handled failure, or +> unexpected throw — emits `host`, `token`, `siteId`, and writes `.env`. + +This is the single most important guarantee here: it converts every future unanticipated failure +from total loss into a recoverable one. + +**Mechanism (D1, decided)**: a single `process.on('exit')` handler that prints `host`/`token`/ +`siteId` and writes `.env` with `writeFileSync`. It must be synchronous — no `await`, no prompting. + +An earlier note here said "from a `finally`-equivalent position"; that was wrong and is withdrawn. +`finally` does **not** run on `process.exit()`, and there are 17 such call sites (13 inside the +single `try` at `src/index.ts:93`). The exit hook covers all of them, including paths nobody has +written yet — which is the point. Verified: stdout survives the handler at 200 lines to both a pipe +and a file; the state block is ~5 lines. + +## X2 — Optional steps are non-fatal + +> **Single owner.** Both UVE call sites — `src/index.ts:226` (existing instance, `--dotcms-url`) and +> `src/index.ts:369` (local Docker) — are replaced by one +> `configureUVE({ host, siteId, token, mode })`, where `mode` is `'local' | 'remote'`. It owns the +> probe, the retry policy, the non-fatal contract and the messaging, and it **contains no +> `process.exit`** — it returns an outcome the caller warns on and continues past. The two sites had +> already drifted apart once; a third could miss the contract entirely. A grep for `process.exit` in +> the UVE path returning nothing is part of X2. + +UVE configuration is **optional**. Its failure MUST: + +1. warn — never `process.exit`; +2. print the [headless UVE guide](https://dev.dotcms.com/docs/author/pages-and-visual-editing/universal-visual-editor/uve-headless-config) + with this run's `host`, `siteId`, and app key `dotema-config-v2`; +3. continue to scaffolding; +4. exit **0** with a complete project. + +**Exit-code change**: a run that previously exited `1` now exits `0` with a warning. Deliberate, and +called out in the spec's Regression Risk — any wrapper asserting the old behavior will see it. + +## X3 — Probe once before writing; never retry a 403 + +The UVE `POST` is attempted only after a `GET` of the same resource returns 200. + +- The `GET` is a **single probe**, not a poll. +- `POST` retries on `5xx` only — the one genuinely transient class. +- `POST` does **not** retry on `403`, `401`, or any other `4xx`. + +**Why no retry on 403** (this reverses the original contract). Measurement during planning showed a +403 here is not transient and never clears: after an interrupted starter import the site's +permission rows are missing, and the endpoint returned 403 on **193 consecutive attempts over ~7 +minutes**, with zero successes. Polling would spin forever. Against a cleanly-booted instance the +same call returns 200 within ~46s of `docker compose up`, before `/dotmgt/readyz` is even green — +so there is no settling window to wait out. + +**On 403 the CLI MUST NOT offer manual UVE setup steps.** Manual configuration fails identically, +for the same missing permissions. It must instead report that the instance is unrecoverable from an +interrupted first boot and must be recreated: + +``` +docker compose down -v && docker compose up -d --wait +``` + +**…but only in `mode: 'local'`.** The advice is mode-dependent, because the same status code means +different things on the two paths: + +| `mode` | What a 403 means | What the CLI says | +|---|---|---| +| `'local'` | The bricked boot — an interrupted starter import never wrote the site's permission rows | Instance is unrecoverable; recreate with `docker compose down -v`. **Do not** offer manual steps: they fail identically. Reference #37268. | +| `'remote'` | The user's own server. There is no stack to recreate. The API token lacks permission on the resolved site. | Report the permission problem, name the site ID and app key `dotema-config-v2`, and **do** link the manual steps — on this path they work. | + +Suggesting `docker compose down -v` to someone who pointed the CLI at their own dotCMS is actively +wrong advice, which is why the mode is part of the contract rather than a presentation detail. + +Root cause and evidence: see spec.md "Cause 2", and #37268 for the backend defect. + +## X4 — Progress is truthful + +- "Containers started successfully" is printed only when containers are actually running and + healthy — via `docker compose up -d --wait --wait-timeout 600` (D7). +- **Feedback MUST be continuous for the entire wait** — up to ten minutes. Ten minutes of frozen + spinner is the failure this issue was reported for, so "pull progress is visible" is not enough. + Two sources, both required: + 1. stream `docker compose up --wait`'s own per-container `Waiting → Healthy` transitions, which + `execa` currently swallows; and + 2. a ticker showing elapsed time and per-service state, refreshed every ~2s. +- A `--wait` timeout is reported with diagnostics, not left as a silent block (research R4: a wrong + probe hangs `--wait`; it does not cause a restart loop). +- Image-pull progress is streamed; no silent multi-minute spinner. +- Retry messages never interleave with an active `ora` spinner — `fetchWithRetry` takes a + caller-supplied reporter instead of calling `console.log` directly. + +## X5 — Readiness probe + +Primary: `GET http://127.0.0.1:8090/dotmgt/readyz` (published by contract C4). +Fallback: `GET /api/v1/appconfiguration` on 8082, for stacks whose compose predates C4. + +A response is "reachable" on any 2xx — `fetchWithRetry` already accepts 2xx, so the caller MUST NOT +re-narrow to `=== 200` (the current mismatch at `src/index.ts:507`). + +## X6 — Re-runs are possible + +A busy port is not by itself fatal. If 8082 is busy **and** answers as a healthy dotCMS, the CLI +reuses that instance instead of exiting. Only a busy port that is *not* dotCMS is an error. + +**Interactivity (D3, decided)**: + +| Context | Behavior | +|---|---| +| TTY | **Prompt**, offering *reuse* or *abort*. Someone who did not expect a dotCMS on 8082 needs to stop and look, not be pushed forward. | +| `CI` env var set, or no TTY | **Auto-reuse, but print a notice.** "Silent" means no prompt, not no output — a scripted run quietly attaching to an unknown instance is the failure this guards against. A piped local run has nobody to answer, so blocking is the worst option. | + +Reuse requires the instance to pass **readiness *and* token issuance**. Something answering on 8082 +is not necessarily a usable dotCMS, and adopting a stranger's instance would wire the project to the +wrong CMS. + +Corollary: the CLI must not offer to empty a directory whose `docker-compose.yml` is the only way to +tear down the instance it is about to reuse. + +## X7 — Failures are reported as failures + +`installDependenciesForProject` returns `Result`. `Err()` is `{ok: false, val}` — +**truthy** — so `if (!result)` at `src/index.ts:597` never fires and a failed `npm install` reports +success. Callers MUST branch on `result.ok`. + +Fixing this makes a previously-unreachable branch reachable: runs with a broken npm that silently +"succeeded" will now correctly fail. Intended, and a visible behavior change. + +## X8 — Filesystem side effects are unwound + +`moveDockerComposeOneLevelUp` / `moveDockerComposeBack` MUST be paired in `try/finally`. Today the +scaffolding between them calls `process.exit(1)` internally, so the restore never runs and +`docker-compose.yml` is stranded in the parent directory (`src/index.ts:376-378`). + +`.env` is written when absent; when already present it is left alone and the block is printed +instead (research R7). + +**Filename (D6, decided): always `.env`**, for every framework. The examples disagree with each +other — `nextjs`/`astro`/`nextjs-experiments` ship `.env.local.example`, `angular-ssr`/`vuejs` ship +`.env.example`, `angular` ships neither — while the CLI today tells everyone `touch .env`. One +filename is simpler, and it is functionally safe: Next.js and Astro both read `.env` as well as +`.env.local`. + +Ordering note: `cloneFrontEndSample` deletes everything in the target directory except `examples/`, +so a `.env` written *before* scaffolding would be destroyed. Writing from the exit hook (X1) happens +after scaffolding, so it survives. + +## X9 — The compose file is bundled, not downloaded + +The CLI ships its own compose file inside the npm package and **writes** it to the project +directory. It does not fetch it, and it does not modify the shared +`single-node-demo-site` example (D4). + +Delivery goes through a swappable source so remote fetching stays one env var away: + +```ts +resolveComposeSource() // bundled asset, unless DOTCMS_COMPOSE_URL is set +``` + +Obtaining **contents** and writing them — rather than downloading to a path, as today — is what +makes the two sources interchangeable. This also removes `downloadFile`'s missing timeout, absent +redirect handling and lack of retry from the default path entirely. + +--- + +## Verification + +Unit-testable: X1, X2, X3, X5, X6, X7, X8 — see the Test Strategy table in [plan.md](../plan.md). +X4's compose behavior is manual — see [quickstart.md](../quickstart.md). diff --git a/specs/37262-create-app-docker-uve/contracts/compose-service-contract.md b/specs/37262-create-app-docker-uve/contracts/compose-service-contract.md new file mode 100644 index 000000000000..29d1d46dc53b --- /dev/null +++ b/specs/37262-create-app-docker-uve/contracts/compose-service-contract.md @@ -0,0 +1,126 @@ +# Contract: the CLI's bundled compose file + +**File**: `core-web/libs/sdk/create-app/assets/docker-compose.yml` +**Consumer**: `@dotcms/create-app` only — it is shipped **inside the npm package**, not downloaded. + +> **The shared `docker/docker-compose-examples/single-node-demo-site/docker-compose.yml` is NOT +> changed by this work.** It keeps serving README readers and already-installed CLIs (≤1.2.5) +> exactly as before. See [cli-design-decisions.md](../cli-design-decisions.md) D4. + +Because nothing else reads this file, it can be strict without imposing on anyone. That is the +whole reason for owning it. + +--- + +## C1 — Startup ordering + +`dotcms` MUST NOT start until `db` and `opensearch` report **healthy**. + +```yaml +depends_on: + db: + condition: service_healthy + opensearch: + condition: service_healthy +``` + +*Deviates from the sibling examples, which use `service_started` for opensearch. Safe here in a way +it was not on the shared file: an unattended CLI wants the strongest ordering available, and a +future OpenSearch image bump that invalidated the probe would affect only this CLI's own stack — +not README readers or other examples.* + +## C2 — Health probes + +| Service | Probe | Source | +|---|---|---| +| `db` | `pg_isready -U dotcmsdbuser -d dotcms -h localhost -p 5432` | already present; now actually consumed | +| `opensearch` | `curl -sk https://localhost:9200 -u admin:admin \| grep -q cluster_name` | adapted from `single-node-os-migration` — `-k` for the self-signed cert, `-u` because `DOT_ES_AUTH_BASIC_PASSWORD: 'admin'` | +| `dotcms` | `curl -f http://127.0.0.1:8090/dotmgt/livez` | matches `lgtm-observability` / `single-node-metrics-monitoring` | + +`dotcms` MUST use **`start_period: 180s`** — roughly 4x the measured ~46s boot. (Precedents: lgtm +120s, metrics-monitoring 20s; this is above both.) + +Erring high is deliberate and costs nothing: `start_period` is the window in which failing probes do +not count toward `retries`, and **the first successful probe ends it immediately**, so a stack that +boots in 46s leaves the window at 46s whatever the ceiling. Erring low does cost. Once the window +elapses, probes start counting, `retries` is exhausted, the container is marked `unhealthy`, and +`docker compose up --wait` **aborts** — abandoning an instance that would have been healthy moments +later. `restart: unless-stopped` cannot rescue it, because Compose restart policies react to +container *exit*, not health status. + +> An earlier draft of this contract recorded the risk of a short window as "`--wait` blocks until +> timeout". That is backwards; the corrected direction is what justifies 180s over 120s. + +The OpenSearch probe is **verified on this stack**: it succeeds at ~15s, and `curl` is present in +the `opensearchproject/opensearch:1` image. + +`curl` is present in the image (`Dockerfile:47`), so `CMD`-form probes are valid. + +## C3 — Restart policy + +`db`, `opensearch` and `dotcms` MUST all declare `restart: unless-stopped`. + +**Semantics that matter**: this reacts to container **exit**, not to `unhealthy` (research R4). It +is what stops the reported failure — dotCMS dying and staying dead — and it cannot cause a +health-driven restart loop. + +## C4 — Published ports + +| Port | Binding | Contract | +|---|---|---| +| `8082`, `8443` | `0.0.0.0` | unchanged — the application | +| `9200`, `9600` | `0.0.0.0` | unchanged | +| `8090` | **`127.0.0.1:8090:8090`** | management port; loopback ONLY | + +**8090 MUST NOT be published on `0.0.0.0`.** `InfrastructureManagementFilter` authorizes by arrival +port with no credential check and no IP allowlist (research R3), so a wildcard binding exposes +`/dotmgt/health` and `/dotmgt/metrics` to the local network. The container's own healthcheck uses +`127.0.0.1` internally and is unaffected. + +## C5 — `--starter` compatibility (**do not break**) + +The file MUST retain a line matching: + +``` +/^(\s*["']?CUSTOM_STARTER_URL["']?\s*:\s*).+$/m +``` + +`updateDockerComposeStarterUrl` (`src/index.ts:487`) rewrites the file **on disk after it is +written** with this regex when `--starter` is passed, and **throws if there is no match**. +Converting that key to a YAML block scalar, an anchor, or `- CUSTOM_STARTER_URL=…` list form breaks +`--starter`. + +Less severe than before — the blast radius is now this CLI version rather than every installed one — +but still a silent break. **Two** guards cover it, and neither subsumes the other: + +- `core-web/libs/sdk/create-app/scripts/verify-cold-start.sh --static` (T008) greps the **file** for + the line shape installed CLIs depend on. No Docker; safe to run on every PR. +- A Jest spec runs `updateDockerComposeStarterUrl()` itself against the real bundled asset and + asserts the **function's** output (AC-012). + +## C6 — Documentation + +`core-web/libs/sdk/create-app/README.md` MUST state that the stack publishes 8090 on loopback, what +it serves, and that the compose file is bundled rather than downloaded — including the +`DOTCMS_COMPOSE_URL` escape hatch for pointing at a remote file instead (D4a). + +## C7 — Delivery + +The file is read through the `ComposeSource` interface (D4a) and **written** to the project +directory, never downloaded to it. `resolveComposeSource()` returns the bundled asset unless +`DOTCMS_COMPOSE_URL` is set. The asset MUST be listed in both `package.json` `files` and +`project.json`'s esbuild `assets`, or it will silently not ship. + +## C8 — Image tag + +The file pins **no** dotCMS version: `dotcms/dotcms:latest` stays for now (D8). The drift risk the +issue flagged — `latest` paired with a hardcoded `starter-20260630` — is therefore **still open**, +and ADR-0019 alignment (SDK version = dotCMS release version) is deferred, not resolved. + +--- + +## Verification + +`core-web/libs/sdk/create-app/scripts/verify-cold-start.sh` — cold start with `--wait`, +`docker kill` recovery, loopback-only exposure, and the `CUSTOM_STARTER_URL` guard for C5. Run +`--static` for the config-only assertions (no Docker required). diff --git a/specs/37262-create-app-docker-uve/data-model.md b/specs/37262-create-app-docker-uve/data-model.md new file mode 100644 index 000000000000..3933e46638de --- /dev/null +++ b/specs/37262-create-app-docker-uve/data-model.md @@ -0,0 +1,147 @@ +# Phase 1 Data Model: create-app local Docker start failure + permanent UVE 403 + +**Feature**: `37262-create-app-docker-uve` · **Plan**: [plan.md](./plan.md) + +This fix introduces no persisted data — no DB table, no OpenSearch mapping, no serialized state. +The "entities" that matter are three in-memory / on-disk shapes whose **lifecycle** is the actual +bug: state that exists, is valid, and gets thrown away. + +--- + +## 1. `RunState` — the CLI's accumulated, recoverable state + +The central entity. Today it exists only as loose locals in `main()`; the defect is that it has no +identity, so nothing guarantees it survives an early exit. + +| Field | Type | Available from | Notes | +|---|---|---|---| +| `host` | `string` | before any network call | `http://localhost:8082` (`DOTCMS_HOST`) | +| `finalDirectory` | `string` | after prompts | target scaffold directory | +| `selectedFramework` | `SupportedFrontEndFrameworks` | after prompts | drives port + env var names | +| `composePath` | `string \| null` | after compose download | needed for `docker compose down` recovery | +| `token` | `string \| null` | after `getAuthToken` | **currently discarded on UVE failure** | +| `siteId` | `string \| null` | after `getDefaultSite` | **currently discarded on UVE failure** | +| `uveConfigured` | `boolean` | after UVE call | new — false must not be fatal | +| `scaffolded` | `boolean` | after clone + install | new | + +### State transitions + +``` +prompts ──▶ ports checked ──▶ compose downloaded ──▶ containers up ──▶ readyz green + │ + ┌───────────┴───────────┐ + ▼ ▼ + token issued (fail: no token) + │ + site resolved + │ + ┌───────────────┴───────────────┐ + ▼ ▼ + UVE GET 200 → POST ok UVE unavailable + uveConfigured = true uveConfigured = false + │ │ + └───────────────┬───────────────┘ + ▼ + scaffold (clone + install) + ▼ + write .env + emit RunState +``` + +**Invariant (this is the fix):** once `token` and `siteId` are non-null, **every** terminal path — +success, handled failure, or unexpected throw — emits them and writes `.env`. The UVE branch merges +back into the main line instead of terminating it. + +**Validation rules** +- `token`/`siteId` are never logged before they are non-null (avoids printing `null` as a value). +- `.env` is written only when absent (research R7); when present, the block is printed instead. +- `composePath` must be non-null before any recovery instruction that says `docker compose down`. + +--- + +## 2. `ComposeTopology` — the service dependency + health graph + +Not a runtime object; the contract **the CLI's own bundled compose file** encodes +(`core-web/libs/sdk/create-app/assets/docker-compose.yml`). The shared `single-node-demo-site` +example is not changed by this work — see cli-design-decisions.md D4. The bug is a missing edge and +two missing health states. + +| Service | Healthcheck | Restart | `dotcms` depends on it via | +|---|---|---|---| +| `db` | exists today (`pg_isready`), **unused** | `unless-stopped` ✓ | `condition: service_healthy` ← **new** | +| `opensearch` | **none** → add (`curl -sk … \| grep -q cluster_name`, from `single-node-os-migration`) | **none** → `unless-stopped` | `condition: service_healthy` ← **new** | +| `dotcms` | **none** → add (`curl -f http://127.0.0.1:8090/dotmgt/livez`, `start_period: 180s`) | **none** → `unless-stopped` | — | + +**Health states** (Docker semantics, per research R4): + +``` +starting ──(within start_period, failures ignored)──▶ healthy + │ │ + └──(retries exhausted after start_period)──▶ unhealthy +``` + +- `service_healthy` gates dependents on `healthy`. +- `docker compose up --wait` blocks until all services are `healthy` or the wait times out. +- **`restart:` does not react to `unhealthy`** — only to container *exit*. An unhealthy container + is not restarted by Compose. + +**Port bindings** + +| Published | Binding | Rationale | +|---|---|---| +| `8082`, `8443` | `0.0.0.0` (unchanged) | the app itself; users browse to it | +| `9200`, `9600` | `0.0.0.0` (unchanged) | existing behavior, out of scope | +| `8090` | **`127.0.0.1` only** ← new | management port is unauthenticated (research R3) | + +**Compatibility constraint**: the file must retain a line matching +`/^(\s*["']?CUSTOM_STARTER_URL["']?\s*:\s*).+$/m`, or `--starter` throws. This forbids converting +that key to a block scalar or `- KEY=value` list form. (Blast radius is now this CLI version rather +than every installed one, since the file is bundled — but it is still a silent break.) + +--- + +## 3. `UVEAppConfig` — the payload and its readiness precondition + +Unchanged in shape; what changes is when it may be written. + +| Field | Type | Source | +|---|---|---| +| `siteId` | `string` | `RunState.siteId` — path segment on `/api/v1/apps/dotema-config-v2/{siteId}` | +| `configuration.hidden` | `boolean` | constant `false` | +| `configuration.value` | `string` | `getUVEConfigValue(http://localhost:${getPortByFramework(framework)})` | + +**Precondition (new).** A `POST` is permitted only after a **single** `GET` of the same resource +returns 200. The `GET` is a probe, not a poll. The `POST` retries on `5xx` only. + +**A 403 is terminal, not transient.** Measured: after an interrupted starter import the endpoint +returned 403 on 193 consecutive attempts over ~7 minutes. Retrying or polling cannot succeed, +because the site's permission rows were never written. On a clean boot the same call returns 200 at +~46s — earlier than `/dotmgt/readyz` — so there is no window to wait out either. + +**Failure semantics (new).** `Err` from this call sets `uveConfigured = false` and is non-fatal. +The message depends on *why*: + +| Status | Meaning | What the CLI says | +|---|---|---| +| `403` | permissions missing from an interrupted first boot — unrecoverable | Recreate the instance: `docker compose down -v && docker compose up -d --wait`. **Do not** offer manual UVE steps; they fail identically. | +| `5xx` | genuinely transient | Retry; on exhaustion, warn and continue | +| other | unexpected | Warn with the [headless UVE guide](https://dev.dotcms.com/docs/author/pages-and-visual-editing/universal-visual-editor/uve-headless-config), `host`, `siteId`, and app key `dotema-config-v2` | + +In every case scaffolding continues and the run exits 0. + +--- + +## Relationships + +``` +ComposeTopology ──guarantees──▶ dotCMS reachable & data-plane settled + │ + ▼ + RunState.token, .siteId + │ + ┌───────────────┴───────────────┐ + ▼ ▼ + UVEAppConfig (optional) .env + printed state (mandatory) +``` + +The arrow that does not exist today is the mandatory one: `RunState` reaching output regardless of +what happens to `UVEAppConfig`.