From eb9b16b2724db78084ca36e29b96034cf10eb736 Mon Sep 17 00:00:00 2001 From: Freddy Montes Date: Fri, 28 Aug 2026 05:36:27 -0600 Subject: [PATCH 01/11] docs(create-app): add issue-resolution spec for #37262 Spec-only PR 1 for the create-app local-Docker failure. Covers the compose ordering/restart defect, the transient UVE 403, and the CLI's discarding of recoverable state. Verified every claim in the issue against the tree before writing: - single-node-demo-site compose: dotcms has no depends_on condition, no restart policy, no healthcheck, and does not publish 8090; opensearch has no healthcheck; db's healthcheck exists but nothing consumes it - src/index.ts:370 exits before the scaffolding at :377 - src/index.ts:597 tests `if (!result)` against a truthy `{ ok: false, val }` - checkPortsAvailability() hard-fails on the ports a successful run holds - the package contains no spec files, so this establishes the harness Defers two P2 items as explicit non-goals: the user.isAdmin() exception swallowing (legacy Liferay, hot permission path, wide blast radius) and image-tag pinning (intersects binding ADR-0019). Neither blocks the P0 fix. Names compose reviewers from git blame, since .github/CODEOWNERS does not cover docker/ and the runtime-fetched compose file is the highest-blast-radius part of this change. Refs #37262 Co-Authored-By: Claude Opus 5 (1M context) --- specs/37262-create-app-docker-uve/spec.md | 344 ++++++++++++++++++++++ 1 file changed, 344 insertions(+) create mode 100644 specs/37262-create-app-docker-uve/spec.md diff --git a/specs/37262-create-app-docker-uve/spec.md b/specs/37262-create-app-docker-uve/spec.md new file mode 100644 index 00000000000..188c0b9b485 --- /dev/null +++ b/specs/37262-create-app-docker-uve/spec.md @@ -0,0 +1,344 @@ +# Issue Resolution Specification: create-app local Docker run never starts dotCMS, then a transient UVE 403 aborts the CLI and discards the project + +**Feature Branch**: `37262-create-app-docker-uve` + +**Created**: 2026-08-28 + +**Status**: Draft + +**Type**: Issue / Bug Resolution + +**Related GitHub Issue**: [#37262](https://github.com/dotCMS/core/issues/37262) + +**Input**: User description: "https://github.com/dotCMS/core/issues/37262" + +## Problem Statement *(mandatory)* + +`npx @dotcms/create-app` — the entry point promoted by the [Headless dotCMS Quick Start blog +post](https://www.dotcms.com/blog/headless-dotcms-quick-start-introducing-dotcms-create-app-cli) +— fails end to end when the user picks the "spin up dotCMS locally with Docker" path. Two +independent defects compound into total data loss for the run: + +1. **dotCMS never actually starts.** The compose file the CLI downloads lets the `dotcms` + container boot before Postgres and OpenSearch are accepting connections, and gives it no + restart policy — so it exits and stays exited. The CLI meanwhile reports "containers + started successfully" and burns its retry budget probing a container that is not running. + The user only gets past this by manually pressing ▶ in Docker Desktop. + +2. **A single non-essential config call aborts the whole run.** Once dotCMS is up, the CLI + obtains an API token and resolves the default site — both succeed — then the Universal + Visual Editor (UVE) app-configuration `POST` returns 403 and the CLI calls `process.exit(1)`. + Because UVE setup runs *before* scaffolding, the user is left with an empty directory: no + project, no `.env`, and the working token and site ID are discarded without ever being + printed. + +Recovery is then blocked by the CLI's own side effects: the port pre-check hard-fails on the +now-running dotCMS's ports, and the directory-clearing prompt would delete the +`docker-compose.yml` needed to tear that instance down. + +**Severity / Impact**: High — the documented first-run experience for headless dotCMS is +broken for every new user on the local-Docker path. Affects evaluators and new developers at +their very first contact with the product, and the failure destroys work that had already +succeeded rather than degrading gracefully. + +## Reproduction *(mandatory)* + +**Environment**: `@dotcms/create-app` 1.2.5 · `dotcms/dotcms:latest` · compose fetched at +runtime from `dotCMS/core@main` (`docker/docker-compose-examples/single-node-demo-site/docker-compose.yml`) +· macOS + Docker Desktop · no pre-existing dotCMS containers, cold image cache + +**Steps to Reproduce**: + +1. On a machine with Docker Desktop running and no dotCMS containers present, run + `npx @dotcms/create-app my-app`. +2. Answer the prompts: target directory `.`, "No - Spin up dotCMS locally with Docker", + framework "Next.js". +3. Observe the `dotcms` container start and immediately exit while `db` and `opensearch` are + still initializing. The CLI nonetheless prints "dotCMS containers started successfully" and + enters its health-check retry loop. +4. Manually start the `dotcms` container from Docker Desktop. Because Postgres and OpenSearch + are healthy by now, it boots. +5. Once the CLI's health check passes, observe `failed to setup UVE config: status=403, + code=ERR_BAD_REQUEST` and a non-zero exit with nothing scaffolded. +6. Re-run the same command. It now aborts at "Required ports are already in use", because the + dotCMS started in step 4 holds 8082/8443/9200/9600. + +**Expected Behavior**: + +- The compose stack brings dotCMS up on its own, ordered behind healthy `db` and `opensearch`, + and restarts it if it exits. +- The CLI reports container start truthfully, and its readiness signal reflects that the calls + it is about to make will succeed. +- A failure in optional UVE configuration warns and continues; the project is still scaffolded. +- No successful run state (host, token, site ID, `.env`) is ever discarded on exit. +- A second run is possible without tearing down a healthy instance. + +**Actual Behavior**: + +``` +✔ dotCMS containers started successfully. ← the container had already exited +⏳ dotCMS not ready (attempt 1/60) - ECONNRESET - Retrying in 5s... + … 11 more attempts … +✔ dotCMS is running locally at http://localhost:8082 +✔ Generated API authentication token ← discarded on exit +✔ Retrieved default site (8a7d5e23-da1e-…) ← discarded on exit +failed to setup UVE config: status=403, code=ERR_BAD_REQUEST +✖ Failed to setup UVE configuration in Dotcms. +``` + +Exit code 1, empty target directory. + +**Reproducibility**: The compose defect (steps 3–4) is deterministic on any cold start where +dotCMS wins the race against Postgres. The 403 (step 5) is timing-dependent: replaying the +CLI's exact three-call sequence against a fully-settled dotCMS returns 200 for all three, so it +reproduces when the CLI reaches the UVE call while the instance is still settling. + +## Scope of Investigation *(mandatory)* + +- **Affected area**: Three surfaces, in priority order. + 1. **Docker compose examples** — `docker/docker-compose-examples/single-node-demo-site/docker-compose.yml`. + This is the least hardened example in that directory; six siblings already use + `condition: service_healthy`, and `lgtm-observability/docker-compose.yml` is the model. + Verified in-repo: `dotcms` has `depends_on: [db, opensearch]` with no condition, no + `restart:`, no healthcheck, and does not publish 8090; `opensearch` has no healthcheck and + no `restart:`; only `db` defines a healthcheck, which nothing consumes. + 2. **`@dotcms/create-app` CLI** — `core-web/libs/sdk/create-app` (v1.2.5). Error handling, + readiness probing, ordering of side effects, and recovery. + 3. **Backend (follow-up only)** — the Apps API permission path that produces the 403. +- **Suspected surface**: The CLI and compose work is frontend/SDK + infrastructure — no Java. + The P2 backend follow-up touches **legacy** `com.liferay.portal.model.User` alongside modern + `com.dotcms.security.apps.AppsAPIImpl`, so it carries legacy-impact weight and is deliberately + separated from the P0/P1 fix. +- **Related known decisions**: + - **ADR-0019 — Date-Lockstep Versioning for the dotCMS SDKs (accepted, binding).** The SDK + version *is* the dotCMS release version it ships with, in both directions. This governs the + "pin the image tag" item: the CLI must not pull `dotcms/dotcms:latest` against a hardcoded + `starter-20260630` URL, and a CLI fix ships via a dotCMS release rather than a standalone + SDK publish. + - ADR-0016 (Docker container naming, *proposed*) is an unfilled template and imposes nothing. + - The plan phase formally consults `dotCMS/platform-adrs`. + +## Root-Cause Hypothesis + +**Cause 1 — compose ordering and restart policy (confirmed by reading the file).** `dotcms` +depends on `db` and `opensearch` without `condition: service_healthy`, so it starts against a +Postgres that is not yet accepting connections and dies. With no `restart:` policy it stays +dead. The `db` healthcheck that would have prevented this already exists and is simply unused. + +**Cause 2 — the 403 is a startup race, not a permissions problem.** Ruled out by evidence in +the report: license gating (`LicenseUtil.getLevel()` has returned `PLATFORM` unconditionally +since #31261, Feb 2025, making the `InvalidLicenseException` path dead on any current image); +Apps-portlet access (`GET /api/v1/apps` and `GET /api/v1/apps/dotema-config-v2/{siteId}` both +return 200 for a token minted the way the CLI mints one); and a wrong site ID. What remains is +`AppsAPIImpl.userDoesNotHaveAccess()` (`AppsAPIImpl.java:104`) calling `user.isAdmin()`, which +is wrapped in `Try.of(…).getOrElse(false)` (`com/liferay/portal/model/User.java:321`) — so +*any* exception during the role lookup silently reports "not an admin", becomes a +`DotSecurityException`, and maps to 403. + +The timing supports this: the CLI's readiness probe is `/api/v1/appconfiguration`, which answers +as soon as the web layer is up — it went green ~60s after container start, far too early for a +demo-starter import to have completed. The CLI then wrote app secrets to an instance still +settling roles, permissions and caches. + +**The readiness signal is therefore wrong.** dotCMS ships a real readiness probe at +`/dotmgt/readyz` (verified: responds `ready`, unauthenticated, no IP ACL) — but only on port +**8090**, which this compose does not publish (`/dotmgt/readyz` on 8080 is a 404). Even +`/readyz` is not sufficient: its registered checks cover CDI, memory, threads and the servlet +container, not "starter import finished". For a CLI the reliable rule is **readiness means the +call you are about to make succeeds** — gate the write on a successful read of the same +resource. + +**Cause 3 — the CLI discards recoverable state.** Independent of causes 1 and 2, and the reason +a transient failure becomes total loss. Verified in-repo: + +- UVE setup exits at `src/index.ts:370`; the clone and `npm install` at `:377` never run. +- Token and site ID are obtained successfully but only printed by `displayFinalSteps()`, which + is downstream of the exit. +- The UVE call has no retry, while authentication retries 3×. +- `checkPortsAvailability()` (`src/utils/index.ts:479`) hard-fails on 8082/8443/9200/9600 — + exactly the ports a successful previous run now holds. +- `prepareDirectory()` (`src/asks.ts:180`) offers to empty the target directory, which would + delete the `docker-compose.yml` needed for `docker compose down`. + +**Additional defects found while reading** (all verified in-repo): + +- `npm install` failure is unreachable: `installDependenciesForProject()` returns a `Result`, + but `src/index.ts:597` tests `if (!result)`. `Err()` returns `{ ok: false, val }` — a truthy + object, so the failure branch never fires and a failed install reports success. +- Orphaned compose file: `moveDockerComposeOneLevelUp()` runs at `src/index.ts:376`; if + scaffolding fails it calls `process.exit(1)` internally, so `moveDockerComposeBack()` at + `:378` never runs and `docker-compose.yml` is stranded in the parent directory. Needs + `try/finally`. +- Multi-minute silence: `execa('docker', ['compose','up','-d'])` swallows image-pull progress, + leaving a frozen spinner for the length of a ~1.5GB pull on a cold machine. +- Unguarded download: `downloadFile()` uses raw `https.get` with no timeout, no retry and no + redirect handling, against an unpinned `main` URL. +- Status mismatch: `fetchWithRetry` accepts any 2xx; `isDotcmsRunning` then demands exactly 200. +- Interleaved output: `fetchWithRetry` calls `console.log` while an `ora` spinner is active — + the cause of the mangled retry block in the log above. +- No tests: the package contains no spec file (confirmed — E2E suite tracked in #35096). + +## Fix Scope & Non-Goals *(mandatory)* + +**In scope**: + +*Compose (P0 — ships without a CLI release, because the CLI fetches this file from `main` at +runtime, so it reaches every already-installed CLI immediately):* + +- Add an `opensearch` healthcheck and `restart: unless-stopped`. +- Change `dotcms` `depends_on` to `condition: service_healthy` for both `db` and `opensearch`. +- Add a `dotcms` healthcheck against `http://127.0.0.1:8090/dotmgt/livez` with + `start_period: 180s`, plus `restart: unless-stopped`. +- Publish port `8090:8090`. + +*CLI (P0):* + +- The CLI never exits without printing recoverable state (host, token, site ID), and writes the + `.env` it already has every value for. +- UVE setup failure is non-fatal: warn, print manual setup steps, and continue to scaffolding. + The warning must link the user to the official headless UVE configuration guide — + + — alongside the concrete values they need (host, site ID, and the app key + `dotema-config-v2`), so the one unset setting is self-serviceable rather than a dead end. +- Gate the UVE write on a read — poll `GET` on the UVE app endpoint until 200, then `POST` with + retry on 401/403/5xx. + +*CLI (P1):* + +- Port check probes before failing: if dotCMS already answers on 8082, offer to reuse it rather + than exiting. +- Use `docker compose up -d --wait` and stream pull progress so the wait is visible. +- Fix the truthy-`Result` check at `src/index.ts:597`; wrap the compose move in `try/finally`. +- Switch readiness to `/dotmgt/readyz` on 8090 once the compose publishes it, keeping + `/api/v1/appconfiguration` as fallback. + +**Explicitly out of scope / non-goals**: + +- **Changing `user.isAdmin()` exception handling** (`com/liferay/portal/model/User.java:321`) + or `AppsAPIImpl.userDoesNotHaveAccess()`. This is a real defect — a transient role-lookup + failure should not read as a permission denial — but it is legacy Liferay code on a hot + permission path with a wide blast radius, and the P0 fix does not depend on it. Tracked as a + P2 follow-up, specified and planned separately. +- **Pinning the dotCMS image tag alongside `CUSTOM_STARTER_URL`.** Real drift risk + (`latest` + hardcoded `starter-20260630`) and it intersects binding ADR-0019, so it deserves + its own decision rather than being folded into a bug fix. P2 follow-up. +- **Landing the E2E suite from #35096.** That issue owns it. This fix adds unit-level tests for + the logic it changes; the fault-injection E2E case (kill dotCMS mid-run) belongs to #35096. +- Rewriting the CLI's `Result` type, prompt flow, or framework-scaffolding logic beyond the + specific defects listed above. +- Hardening the other compose examples in `docker/docker-compose-examples/`. Only + `single-node-demo-site` is fetched by the CLI; the rest are out of this fix's blast radius. +- Any change to the Apps REST API contract or the UVE app definition itself. + +## Regression Risk *(mandatory)* + +- **Blast radius**: + - *Compose:* `single-node-demo-site/docker-compose.yml` is fetched from `main` at runtime by + every installed `@dotcms/create-app`, so a change ships instantly and unversioned to all + existing CLI users — including older CLI versions that will not know about port 8090. This + cuts both ways: it is why the fix is P0 and reaches users without a release, and it is the + single largest regression risk in this work. The file is also used directly by users + following the demo-site README. The added `dotcms` healthcheck must not fail on images + where `/dotmgt/livez` behaves differently, or the container will be marked unhealthy and + (with `restart: unless-stopped`) flap. + - *CLI:* making UVE failure non-fatal changes the exit contract — a run that previously + exited 1 will now exit 0 with a warning. Any CI or script asserting on the old behavior + would see a behavior change. `--wait` on `docker compose up` changes how long the command + blocks and requires the healthchecks above to be correct, or the CLI hangs until timeout + instead of proceeding. + - Fixing the truthy-`Result` check at `src/index.ts:597` makes a previously-unreachable + failure branch reachable: runs with a broken npm that silently "succeeded" before will now + correctly fail. This is the intended fix, but it is a visible behavior change. +- **Backward compatibility**: No dotCMS API contract, serialized state, DB schema or ES mapping + changes. Publishing 8090 exposes the management port on the host for local demo stacks — + acceptable for a local developer stack, but it must be stated in the README rather than + introduced silently. Per ADR-0019, a CLI change ships as part of a dotCMS release, not a + standalone SDK publish. +- **Data considerations**: None. No migration, no repair of existing data. Users left with an + empty directory by the old behavior simply re-run; the P1 port-reuse work is what makes that + re-run possible without tearing down a healthy instance. + +### Required reviewers for the compose change + +`.github/CODEOWNERS` does not cover `docker/`, so the compose file has no automatic reviewer +despite being the highest-blast-radius part of this fix. Reviewers are therefore drawn from +`git blame` on the exact hunks being changed. Note that the two largest raw blame counts are +mechanical — a bulk restore (#27432) and a `pgvector` version bump (#29915) — so the list below +weights *who shaped the design* over line count: + +| Reviewer | Why they should review | +| --- | --- | +| **Steve Bolton** (`spbolton`) | Dominant blame on every hunk in scope — the `dotcms` service block, its `depends_on`, and its `ports`. Also authored `lgtm-observability/docker-compose.yml` (#32980), which is the `condition: service_healthy` pattern this change copies. Best-placed to say whether we are applying that pattern faithfully. | +| **Will Ezell** | Authored the OpenSearch 1.x + SSL setup in this file (#27754) and the Postgres 18 upgrade (#34236) that touched both this file and the model stack. Owns the current `db`/`opensearch` shape we are adding healthchecks to. | +| **Erick González** (`erickgonzalez`) | Most recent semantic change to the `dotcms` service (#36490, 2026-07-13) and prior starter-version work (#36362). Closest to the `CUSTOM_STARTER_URL` / starter-import behavior that the readiness race depends on. | +| **Daniel Colina** | Holds 13 lines of the `opensearch` block and part of the `depends_on` region via #29915. Secondary — loop in if the OpenSearch healthcheck shape is contested. | + +Two review questions to put to them explicitly, since neither is settled by this spec: + +1. **Publishing `8090:8090`** exposes the management port on the host for every user of this + demo stack, not just CLI users. Acceptable for a local demo, or should the CLI reach it + another way? +2. **`restart: unless-stopped` plus a `/dotmgt/livez` healthcheck** will flap the container if + that endpoint behaves differently on some image tag. Is `start_period: 180s` enough headroom + for a cold starter import on a slow machine? + +## Acceptance & Verification *(mandatory)* + +- **AC-001**: On a cold machine with no dotCMS containers, `npx @dotcms/create-app` on the + local-Docker path brings the stack up **without manual intervention** — `dotcms` starts only + after `db` and `opensearch` report healthy, and is restarted if it exits. Reproduction steps + 3 and 4 no longer occur. +- **AC-002**: The CLI's "containers started successfully" message is only printed when the + containers are actually running and healthy. +- **AC-003**: When the UVE configuration call fails for any reason, the CLI prints a warning, + **continues to scaffolding**, and exits 0 with a complete project. Reproduction step 5 no + longer aborts the run. The warning includes the headless UVE configuration guide + () + and the run-specific values needed to follow it — host, site ID, and app key + `dotema-config-v2`. A user who hits this path can finish the setup by hand without leaving + the terminal to go hunting for docs. +- **AC-004**: On **any** exit path after a token has been issued, the CLI prints the host, + token and site ID, and writes the `.env` file from the values it holds. No successful state + is discarded. +- **AC-005**: The UVE write is gated on a successful `GET` of the same resource, and retries on + 401/403/5xx rather than failing on first response. +- **AC-006** *(P1)*: With a healthy dotCMS already answering on 8082, a second run offers to + reuse it instead of aborting with "Required ports are already in use". Reproduction step 6 no + longer blocks. +- **AC-007** *(P1)*: A failed `npm install` causes the CLI to report failure — the branch at + `src/index.ts:597` is reachable and correct. +- **AC-008** *(P1)*: If scaffolding fails after `moveDockerComposeOneLevelUp()`, the compose + file is restored to the project directory (no orphan in the parent). +- **AC-009**: Image-pull progress is visible during `docker compose up`; no silent multi-minute + spinner. Retry messages do not interleave with an active `ora` spinner. +- **AC-010**: No regression to the other `docker/docker-compose-examples/*` stacks, and the + demo-site README documents the newly published 8090 port. + +- **Verification method**: + - *Compose:* `docker compose -f docker/docker-compose-examples/single-node-demo-site/docker-compose.yml up -d --wait` + from a cold state (no volumes, no cached images), asserting `dotcms` reaches healthy without + manual start; plus a fault-injection run (`docker kill` the `dotcms` container) asserting it + is restarted. + - *CLI unit tests:* the package currently has **no spec file at all**, so this fix establishes + the harness. Jest specs covering, at minimum: the `Result` truthiness fix, the non-fatal UVE + path, the read-before-write gate with a mocked 403-then-200 sequence, the `try/finally` + compose move, and the port-reuse probe. Per constitution Principle V these are written and + confirmed failing (Red) before the implementation lands. + - *Manual end-to-end:* the reproduction steps above, run cold on macOS + Docker Desktop, + verifying AC-001 through AC-006. + - The full fault-injection E2E suite is #35096's scope, not this fix's. + +## Assumptions + +- The reporter's finding that the three-call sequence returns 200 against a fully-started + dotCMS is taken as given; the fix targets the race rather than re-deriving the permission + analysis. +- `/dotmgt/livez` and `/dotmgt/readyz` are available on port 8090 on `dotcms/dotcms:latest` and + respond unauthenticated, as verified in the report. The plan should confirm this against the + specific image the compose file pins. +- Making UVE configuration optional is acceptable product behavior: a scaffolded project with + one unset editor configuration, printed manual steps and a link to the headless UVE guide is + strictly better than an empty directory. **Confirmed by the issue owner** — conditional on the + warning telling the user how to finish the setup, which AC-003 now requires. +- The compose change shipping unversioned to all installed CLI versions is acceptable and + desirable, given every affected version is currently broken on this path. From dcde7d92f78fd374752b7e121979c3500cb953d0 Mon Sep 17 00:00:00 2001 From: Freddy Montes Date: Fri, 28 Aug 2026 05:39:02 -0600 Subject: [PATCH 02/11] docs(create-app): correct the reviewer list to assignable collaborators MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The two strongest git-blame signals on the compose file — spbolton (dominant blame on every hunk in scope, and author of the lgtm-observability stack whose service_healthy pattern this change copies) and dcolina — are no longer collaborators on dotCMS/core, so GitHub rejects review requests for them. Replaces them with jcastro-dotcms (second-most-active docker/ contributor over the last 12 months) and records the resulting coverage gap explicitly: nobody currently assignable designed the pattern being copied, so the plan phase should read lgtm-observability/docker-compose.yml as the specification rather than rely on a reviewer to catch a faithful-copy error. Refs #37262 Co-Authored-By: Claude Opus 5 (1M context) --- specs/37262-create-app-docker-uve/spec.md | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/specs/37262-create-app-docker-uve/spec.md b/specs/37262-create-app-docker-uve/spec.md index 188c0b9b485..fe11eba5936 100644 --- a/specs/37262-create-app-docker-uve/spec.md +++ b/specs/37262-create-app-docker-uve/spec.md @@ -268,10 +268,19 @@ weights *who shaped the design* over line count: | Reviewer | Why they should review | | --- | --- | -| **Steve Bolton** (`spbolton`) | Dominant blame on every hunk in scope — the `dotcms` service block, its `depends_on`, and its `ports`. Also authored `lgtm-observability/docker-compose.yml` (#32980), which is the `condition: service_healthy` pattern this change copies. Best-placed to say whether we are applying that pattern faithfully. | -| **Will Ezell** | Authored the OpenSearch 1.x + SSL setup in this file (#27754) and the Postgres 18 upgrade (#34236) that touched both this file and the model stack. Owns the current `db`/`opensearch` shape we are adding healthchecks to. | +| **Will Ezell** (`wezell`) | Authored the OpenSearch 1.x + SSL setup in this file (#27754) and the Postgres 18 upgrade (#34236) that touched both this file and the model stack. Owns the current `db`/`opensearch` shape we are adding healthchecks to, and is the most senior still-active owner of `docker/`. | | **Erick González** (`erickgonzalez`) | Most recent semantic change to the `dotcms` service (#36490, 2026-07-13) and prior starter-version work (#36362). Closest to the `CUSTOM_STARTER_URL` / starter-import behavior that the readiness race depends on. | -| **Daniel Colina** | Holds 13 lines of the `opensearch` block and part of the `depends_on` region via #29915. Secondary — loop in if the OpenSearch healthcheck shape is contested. | +| **Jose Castro** (`jcastro-dotcms`) | Not a blame match on this file. Added to cover the gap below: second-most-active contributor to `docker/` over the last 12 months. | + +**Unavailable — the two strongest blame signals.** `spbolton` (Steve Bolton) holds dominant blame +on every hunk in scope *and* authored `lgtm-observability/docker-compose.yml` (#32980), the exact +`condition: service_healthy` pattern this change copies. `dcolina` (Daniel Colina) holds 13 lines +of the `opensearch` block via #29915. Neither is a collaborator on `dotCMS/core` any more (last +commits 2026-03-30 and 2026-04-07 respectively), so GitHub rejects a review request for them. + +This leaves a real coverage gap: **nobody currently assignable designed the pattern being copied.** +The plan phase should treat the lgtm-observability compose as the specification for correct +usage — reading it directly rather than relying on a reviewer to catch a faithful-copy error. Two review questions to put to them explicitly, since neither is settled by this spec: From 0fd8e160e81e3c5e3fbeb5274d1565b892da54ba Mon Sep 17 00:00:00 2001 From: Freddy Montes Date: Fri, 28 Aug 2026 05:59:24 -0600 Subject: [PATCH 03/11] docs(create-app): fold plan-phase research findings back into the spec MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Planning verified the spec's claims against the tree and four did not hold. Correcting them here, before sign-off, so reviewers approve what we will actually build. - "Six siblings use condition: service_healthy" — there are three, and NONE gates dotcms on opensearch being healthy; all use service_started. Gating on both is therefore a deliberate deviation from every precedent, not the house pattern the spec implied. Kept, with the rationale stated and using os-migration's proven probe. - The stated risk that a bad healthcheck would make restart: unless-stopped flap the container cannot happen: Compose restart policies react to container exit, not health status. Replaced with the real exposure — a wrong probe blocks `docker compose up --wait` until timeout — and required an explicit --wait-timeout. - Publishing 8090 changed from "8090:8090, acceptable for a local stack" to "127.0.0.1:8090:8090". InfrastructureManagementFilter authorizes purely by arrival port: no credential check, no IP allowlist, so a wildcard binding puts /dotmgt/health and /dotmgt/metrics on the local network. Added AC-011. - The package has no spec files but the Jest harness already exists, so this fix adds specs rather than establishing a harness. Recorded pnpm install as a Red-gate prerequisite and warned that passWithNoTests makes an empty run green. Also adds AC-012 for a compose/CLI compatibility landmine found while reading: updateDockerComposeStarterUrl rewrites the file with a regex and throws on no match, so reformatting CUSTOM_STARTER_URL would break --starter for every installed CLI. Notes that .env is new behavior, not a restoration, and downgrades the /dotmgt/livez-on-latest assumption to partly-verified with a gating check. Two of the three open review questions are now settled or reframed. Refs #37262 Co-Authored-By: Claude Opus 5 (1M context) --- specs/37262-create-app-docker-uve/spec.md | 106 +++++++++++++++++----- 1 file changed, 81 insertions(+), 25 deletions(-) diff --git a/specs/37262-create-app-docker-uve/spec.md b/specs/37262-create-app-docker-uve/spec.md index fe11eba5936..8c1efa60a9f 100644 --- a/specs/37262-create-app-docker-uve/spec.md +++ b/specs/37262-create-app-docker-uve/spec.md @@ -97,8 +97,12 @@ reproduces when the CLI reaches the UVE call while the instance is still settlin - **Affected area**: Three surfaces, in priority order. 1. **Docker compose examples** — `docker/docker-compose-examples/single-node-demo-site/docker-compose.yml`. - This is the least hardened example in that directory; six siblings already use - `condition: service_healthy`, and `lgtm-observability/docker-compose.yml` is the model. + This is the least hardened example in that directory. **Three** siblings already use + `condition: service_healthy` (`lgtm-observability`, `single-node-metrics-monitoring`, + `experiments`, plus `single-node-os-migration` via provision jobs) — + `lgtm-observability/docker-compose.yml` is the model. Note none of them gates `dotcms` + on OpenSearch being *healthy*; every one uses `db: service_healthy` + + `opensearch: service_started`. See Fix Scope for why this fix goes further. Verified in-repo: `dotcms` has `depends_on: [db, opensearch]` with no condition, no `restart:`, no healthcheck, and does not publish 8090; `opensearch` has no healthcheck and no `restart:`; only `db` defines a healthcheck, which nothing consumes. @@ -187,9 +191,21 @@ runtime, so it reaches every already-installed CLI immediately):* - Add an `opensearch` healthcheck and `restart: unless-stopped`. - Change `dotcms` `depends_on` to `condition: service_healthy` for both `db` and `opensearch`. + Gating on `db` is what fixes the reported crash. Gating on `opensearch` as well is a + **deliberate deviation** from all four existing examples, which use `service_started` there — + justified because this stack is driven by an unattended CLI, so an OpenSearch that is + up-but-not-ready is a failure with nobody present to diagnose it. The OpenSearch probe should + be the one already proven in `single-node-os-migration` (`-k` for the self-signed cert, + `-u admin:admin`, since this stack sets `DOT_ES_AUTH_BASIC_PASSWORD: 'admin'`), not a new one. - Add a `dotcms` healthcheck against `http://127.0.0.1:8090/dotmgt/livez` with `start_period: 180s`, plus `restart: unless-stopped`. -- Publish port `8090:8090`. +- Publish the management port **bound to loopback**: `127.0.0.1:8090:8090` — not `8090:8090`. + The management port is authorized purely by the port a request arrives on + (`InfrastructureManagementFilter`): no credential check, no IP allowlist. A wildcard binding + would put `/dotmgt/health` and `/dotmgt/metrics` on the local network for every user of this + demo stack. Loopback gives the CLI and the container's own healthcheck everything they need + (both already use `127.0.0.1`) and gives the network nothing. This is stricter than the two + existing examples that publish 8090. *CLI (P0):* @@ -239,21 +255,32 @@ runtime, so it reaches every already-installed CLI immediately):* cuts both ways: it is why the fix is P0 and reaches users without a release, and it is the single largest regression risk in this work. The file is also used directly by users following the demo-site README. The added `dotcms` healthcheck must not fail on images - where `/dotmgt/livez` behaves differently, or the container will be marked unhealthy and - (with `restart: unless-stopped`) flap. + where `/dotmgt/livez` behaves differently. Note the failure mode is **not** a restart loop: + Compose restart policies react to container *exit*, not to health status (health-driven + restart is a Swarm feature), so an unhealthy container is simply never marked ready. The + consequence is that `depends_on` and `--wait` block on it — see the CLI bullet below. + - *Compose ↔ installed CLIs:* the file MUST keep a line matching + `/^(\s*["']?CUSTOM_STARTER_URL["']?\s*:\s*).+$/m`. `updateDockerComposeStarterUrl` + (`src/index.ts:487`) rewrites the compose file with that regex when `--starter` is passed + and **throws if there is no match**. Reformatting that key into a YAML block scalar, an + anchor, or a `- KEY=value` list entry would break `--starter` for every already-installed + CLI — with no release able to reach them. - *CLI:* making UVE failure non-fatal changes the exit contract — a run that previously exited 1 will now exit 0 with a warning. Any CI or script asserting on the old behavior would see a behavior change. `--wait` on `docker compose up` changes how long the command - blocks and requires the healthchecks above to be correct, or the CLI hangs until timeout - instead of proceeding. + blocks and requires the healthchecks above to be correct: a wrong probe turns a working run + into a block until timeout. This — not restart flapping — is the real cost of getting the + healthcheck wrong, so `--wait` must be paired with an explicit `--wait-timeout` and a + timeout must degrade to reported diagnostics rather than a silent hang. - Fixing the truthy-`Result` check at `src/index.ts:597` makes a previously-unreachable failure branch reachable: runs with a broken npm that silently "succeeded" before will now correctly fail. This is the intended fix, but it is a visible behavior change. - **Backward compatibility**: No dotCMS API contract, serialized state, DB schema or ES mapping - changes. Publishing 8090 exposes the management port on the host for local demo stacks — - acceptable for a local developer stack, but it must be stated in the README rather than - introduced silently. Per ADR-0019, a CLI change ships as part of a dotCMS release, not a - standalone SDK publish. + changes. Publishing 8090 exposes an **unauthenticated** management surface + (`/dotmgt/health`, `/dotmgt/metrics`) to whoever can reach the binding — which is why the fix + binds it to `127.0.0.1` rather than the wildcard the issue originally proposed. It must still + be stated in the README rather than introduced silently. Per ADR-0019, a CLI change ships as + part of a dotCMS release, not a standalone SDK publish. - **Data considerations**: None. No migration, no repair of existing data. Users left with an empty directory by the old behavior simply re-run; the P1 port-reuse work is what makes that re-run possible without tearing down a healthy instance. @@ -282,14 +309,25 @@ This leaves a real coverage gap: **nobody currently assignable designed the patt The plan phase should treat the lgtm-observability compose as the specification for correct usage — reading it directly rather than relying on a reviewer to catch a faithful-copy error. -Two review questions to put to them explicitly, since neither is settled by this spec: - -1. **Publishing `8090:8090`** exposes the management port on the host for every user of this - demo stack, not just CLI users. Acceptable for a local demo, or should the CLI reach it - another way? -2. **`restart: unless-stopped` plus a `/dotmgt/livez` healthcheck** will flap the container if - that endpoint behaves differently on some image tag. Is `start_period: 180s` enough headroom - for a cold starter import on a slow machine? +Two review questions to put to them explicitly. Both were sharpened by the plan-phase research — +question 1 now carries a recommendation rather than being open, and question 2 was reframed +because its original premise was wrong: + +1. **Gating `dotcms` on `opensearch: service_healthy`** goes further than all four existing + examples, which use `service_started` there. Is the stricter gate right for a stack driven by + an unattended CLI, or should this match the house pattern? *(Recommendation: keep the stricter + gate, using `single-node-os-migration`'s proven probe.)* +2. **Is `start_period: 180s` enough headroom** for a cold demo-starter import on a slow machine? + This is above both precedents (lgtm 120s, metrics-monitoring 20s). The original question — + whether `restart: unless-stopped` would flap the container — turned out to rest on a false + premise: Compose restart policies react to container exit, not health status, so flapping + cannot occur. The real exposure is that a too-short `start_period` makes + `docker compose up --wait` block until timeout. + +*(Publishing 8090 was the third open question. It is now settled in Fix Scope: bind to +`127.0.0.1`, because the management port is authorized by arrival port with no credential check +and no IP allowlist. Flagging it here so reviewers see the decision rather than having to +rediscover the reasoning.)* ## Acceptance & Verification *(mandatory)* @@ -308,7 +346,10 @@ Two review questions to put to them explicitly, since neither is settled by this the terminal to go hunting for docs. - **AC-004**: On **any** exit path after a token has been issued, the CLI prints the host, token and site ID, and writes the `.env` file from the values it holds. No successful state - is discarded. + is discarded. Note the CLI does not write `.env` today at all — it only prints `touch .env` + plus a block to paste — so this is new behavior, not a restoration. `.env` is written when + absent; when the scaffolded example already ships one, it is left alone and the block is + printed as today. - **AC-005**: The UVE write is gated on a successful `GET` of the same resource, and retries on 401/403/5xx rather than failing on first response. - **AC-006** *(P1)*: With a healthy dotCMS already answering on 8082, a second run offers to @@ -322,20 +363,31 @@ Two review questions to put to them explicitly, since neither is settled by this spinner. Retry messages do not interleave with an active `ora` spinner. - **AC-010**: No regression to the other `docker/docker-compose-examples/*` stacks, and the demo-site README documents the newly published 8090 port. +- **AC-011**: Port 8090 is reachable on `127.0.0.1` and **not** on the host's LAN address — + the management surface is not exposed to the network. +- **AC-012**: `npx @dotcms/create-app --starter ` still works against the edited compose + file. The `CUSTOM_STARTER_URL` rewrite regex in `updateDockerComposeStarterUrl` must still + match, or every already-installed CLI loses `--starter`. - **Verification method**: - *Compose:* `docker compose -f docker/docker-compose-examples/single-node-demo-site/docker-compose.yml up -d --wait` from a cold state (no volumes, no cached images), asserting `dotcms` reaches healthy without manual start; plus a fault-injection run (`docker kill` the `dotcms` container) asserting it is restarted. - - *CLI unit tests:* the package currently has **no spec file at all**, so this fix establishes - the harness. Jest specs covering, at minimum: the `Result` truthiness fix, the non-fatal UVE + - *CLI unit tests:* the package contains **no spec file**, but the Jest harness is already in + place (`jest.config.ts`, `tsconfig.spec.json`, `@nx/jest/plugin`), so this fix adds specs + rather than a harness. Run with `pnpm nx test sdk-create-app` — note `project.json` sets + `passWithNoTests: true`, so an empty run reports green; confirm the new specs are actually + collected before trusting a pass. Jest specs covering, at minimum: the `Result` truthiness fix, the non-fatal UVE path, the read-before-write gate with a mocked 403-then-200 sequence, the `try/finally` compose move, and the port-reuse probe. Per constitution Principle V these are written and confirmed failing (Red) before the implementation lands. - *Manual end-to-end:* the reproduction steps above, run cold on macOS + Docker Desktop, - verifying AC-001 through AC-006. + verifying AC-001 through AC-006, plus AC-011 (a `curl` to the host's LAN address on 8090 + must be refused) and AC-012 (a `--starter` run against the edited file). - The full fault-injection E2E suite is #35096's scope, not this fix's. + - **Prerequisite**: `pnpm install` in `core-web/`. Without it `pnpm nx test` fails with + `nx: command not found`, and constitution Principle V's Red gate cannot be demonstrated. ## Assumptions @@ -343,8 +395,12 @@ Two review questions to put to them explicitly, since neither is settled by this dotCMS is taken as given; the fix targets the race rather than re-deriving the permission analysis. - `/dotmgt/livez` and `/dotmgt/readyz` are available on port 8090 on `dotcms/dotcms:latest` and - respond unauthenticated, as verified in the report. The plan should confirm this against the - specific image the compose file pins. + respond unauthenticated. **Partly verified**: `server.xml` defines the connector on + `${CMS_MANAGEMENT_PORT:-8090}`, so the port is live by default, and `curl` is present in the + image for the healthcheck. But the two existing compose examples that probe `/dotmgt/livez` + both run `dotcms/dotcms-test:1.0.0-SNAPSHOT`, not `latest` — so the endpoint must still be + confirmed against the released image before the healthcheck depends on it. This is the first + step of the plan's verification guide, and a failure there invalidates the compose design. - Making UVE configuration optional is acceptable product behavior: a scaffolded project with one unset editor configuration, printed manual steps and a link to the headless UVE guide is strictly better than an empty directory. **Confirmed by the issue owner** — conditional on the From 02a4952a2b728cf6787b3d0bf0d57271425a1bba Mon Sep 17 00:00:00 2001 From: Freddy Montes Date: Fri, 28 Aug 2026 09:01:51 -0600 Subject: [PATCH 04/11] =?UTF-8?q?docs(create-app):=20replace=20root=20caus?= =?UTF-8?q?e=202=20=E2=80=94=20the=20403=20is=20permanent=20damage,=20not?= =?UTF-8?q?=20a=20race?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Planning tested the spec's own hypothesis and disproved it. Correcting before sign-off so reviewers approve the real diagnosis. The spec claimed a transient startup race in which the CLI writes while roles and permissions are still settling, with user.isAdmin() swallowing an exception via Try.of(...).getOrElse(false). Two experiments (M5/64GB host, dotcms constrained to 2 CPUs / 4G) say otherwise. Clean boot has no settling window. The UVE endpoint is usable at 46s — two seconds BEFORE /dotmgt/readyz goes green — because the starter import (T+20s) and ES reindex (T+44s) both finish inside Tomcat startup and the connector accepts no traffic until after them. The hypothesised race cannot occur. The reporter's actual path does reproduce it, and permanently. Killing dotcms mid starter-import and hand-starting it (reproduction step 4) yields the reported log exactly — token 200, defaultSite 200, UVE 403 — and then 403 on 193 consecutive attempts over ~7 minutes with zero successes. The server says the admin user lacks READ permission on demo.dotcms.com: the interrupted import never wrote the site's permission rows, and the restart does not repair them. So cause 2 is a consequence of cause 1, not an independent defect, and fixing the compose file removes it. Design consequences, not just narrative: - AC-005 no longer polls until 200. A poll would never terminate; retry is restricted to 5xx and 403 explicitly does not retry. - On 403 the CLI must tell the user the instance is unrecoverable and to run `docker compose down -v` — offering manual UVE setup steps is wrong advice, since manual configuration fails identically. - The P2 backend non-goal is re-pointed: not isAdmin() exception swallowing, but the larger defect that any interrupted first boot silently bricks the instance while reporting a clean startup. Filed separately. Title and Reproducibility updated: the 403 is deterministic once the crash has happened, not timing-dependent. Refs #37262 Co-Authored-By: Claude Opus 5 (1M context) --- specs/37262-create-app-docker-uve/spec.md | 135 ++++++++++++++++------ 1 file changed, 100 insertions(+), 35 deletions(-) diff --git a/specs/37262-create-app-docker-uve/spec.md b/specs/37262-create-app-docker-uve/spec.md index 8c1efa60a9f..b28d33fc4c4 100644 --- a/specs/37262-create-app-docker-uve/spec.md +++ b/specs/37262-create-app-docker-uve/spec.md @@ -1,4 +1,4 @@ -# Issue Resolution Specification: create-app local Docker run never starts dotCMS, then a transient UVE 403 aborts the CLI and discards the project +# Issue Resolution Specification: create-app local Docker run never starts dotCMS, and the resulting broken instance 403s the UVE call and discards the project **Feature Branch**: `37262-create-app-docker-uve` @@ -30,7 +30,9 @@ independent defects compound into total data loss for the run: Visual Editor (UVE) app-configuration `POST` returns 403 and the CLI calls `process.exit(1)`. Because UVE setup runs *before* scaffolding, the user is left with an empty directory: no project, no `.env`, and the working token and site ID are discarded without ever being - printed. + printed. The 403 is not incidental: hand-starting the container in step 1 leaves the starter + import incomplete, the site's permissions unwritten, and the instance **permanently** unable + to serve the Apps API. The two defects are one causal chain, not two independent bugs. Recovery is then blocked by the CLI's own side effects: the port pre-check hard-fails on the now-running dotCMS's ports, and the directory-clearing prompt would delete the @@ -88,10 +90,12 @@ failed to setup UVE config: status=403, code=ERR_BAD_REQUEST Exit code 1, empty target directory. -**Reproducibility**: The compose defect (steps 3–4) is deterministic on any cold start where -dotCMS wins the race against Postgres. The 403 (step 5) is timing-dependent: replaying the -CLI's exact three-call sequence against a fully-settled dotCMS returns 200 for all three, so it -reproduces when the CLI reaches the UVE call while the instance is still settling. +**Reproducibility**: The compose defect (steps 3–4) occurs on a cold start where dotCMS wins the +race against Postgres; it is timing-dependent and did **not** reproduce on a fast machine with a +warm image cache. The 403 (step 5) is **deterministic once step 3 has happened**: killing dotCMS +mid starter-import and hand-starting it reproduces 403 on every subsequent attempt (193/193 over +~7 minutes). Against a cleanly-booted instance all three calls return 200, which is why the +original report read it as transient — it is not. See Root-Cause Hypothesis, Cause 2. ## Scope of Investigation *(mandatory)* @@ -129,28 +133,82 @@ depends on `db` and `opensearch` without `condition: service_healthy`, so it sta Postgres that is not yet accepting connections and dies. With no `restart:` policy it stays dead. The `db` healthcheck that would have prevented this already exists and is simply unused. -**Cause 2 — the 403 is a startup race, not a permissions problem.** Ruled out by evidence in -the report: license gating (`LicenseUtil.getLevel()` has returned `PLATFORM` unconditionally -since #31261, Feb 2025, making the `InvalidLicenseException` path dead on any current image); -Apps-portlet access (`GET /api/v1/apps` and `GET /api/v1/apps/dotema-config-v2/{siteId}` both -return 200 for a token minted the way the CLI mints one); and a wrong site ID. What remains is -`AppsAPIImpl.userDoesNotHaveAccess()` (`AppsAPIImpl.java:104`) calling `user.isAdmin()`, which -is wrapped in `Try.of(…).getOrElse(false)` (`com/liferay/portal/model/User.java:321`) — so -*any* exception during the role lookup silently reports "not an admin", becomes a -`DotSecurityException`, and maps to 403. - -The timing supports this: the CLI's readiness probe is `/api/v1/appconfiguration`, which answers -as soon as the web layer is up — it went green ~60s after container start, far too early for a -demo-starter import to have completed. The CLI then wrote app secrets to an instance still -settling roles, permissions and caches. - -**The readiness signal is therefore wrong.** dotCMS ships a real readiness probe at -`/dotmgt/readyz` (verified: responds `ready`, unauthenticated, no IP ACL) — but only on port -**8090**, which this compose does not publish (`/dotmgt/readyz` on 8080 is a 404). Even -`/readyz` is not sufficient: its registered checks cover CDI, memory, threads and the servlet -container, not "starter import finished". For a CLI the reliable rule is **readiness means the -call you are about to make succeeds** — gate the write on a successful read of the same -resource. +**Cause 2 — the 403 is permanent damage caused by Cause 1, not a startup race.** +*This supersedes the original hypothesis, which planning disproved by experiment.* + +The report proposed a transient race: the CLI writes while the instance is still settling roles +and permissions, and `AppsAPIImpl.userDoesNotHaveAccess()` calling `user.isAdmin()` — wrapped in +`Try.of(…).getOrElse(false)` — silently reports "not an admin". **Measurement does not support +that**, and two experiments replaced it (M5/64GB host, dotCMS constrained to 2 CPUs / 4G): + +*Experiment 1 — clean boot has no settling window at all.* + +| Signal | First success | +|---|---| +| `POST /api/v1/authentication/api-token` | **46s** | +| `GET /api/v1/apps/dotema-config-v2/{siteId}` | **46s** | +| `/dotmgt/livez`, `/dotmgt/readyz` | 48s | +| `/api/v1/appconfiguration` (the CLI's current probe) | 49s | + +The UVE endpoint is usable **two seconds before `readyz` goes 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 connector accepts no traffic until after +them. There is no window in which the API answers but the data plane is unready — so the +hypothesised race cannot occur on a clean boot. + +*Experiment 2 — the reporter's actual path reproduces it, permanently.* + +Reproducing reproduction step 4 (dotCMS killed mid starter-import, then hand-started): + +``` +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 +``` + +That is the reported log line for line. The server states the cause plainly: + +``` +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 never wrote the site's permission rows. The restart re-ran +`Task00004LoadStarter` and Tomcat came up clean, but the permissions never appeared. **The +instance does not recover** — only `docker compose down -v` and a fresh start does. + +**So the causal chain is**: Cause 1 (dotCMS races Postgres and exits) → user hand-starts the +crashed container → the starter import is left incomplete → site permissions are missing → +**every** Apps API call 403s, forever. Cause 2 is a *consequence* of Cause 1, not an independent +defect. **Fixing the compose file removes it.** + +What this rules out, on evidence rather than inference: it is not license gating +(`LicenseUtil.getLevel()` has returned `PLATFORM` unconditionally since #31261), not +Apps-portlet access, not a wrong site ID, and **not** `user.isAdmin()` swallowing an exception — +the permission data is genuinely absent, so `isAdmin()` has nothing to throw about. + +**Consequences for the fix** (these change the design, not just the narrative): + +- **Retrying or polling the UVE call cannot work.** A read-before-write gate that polls `GET` + until 200 would poll forever against a condition that never clears. A single probe is correct; + retry only `5xx`. +- **The failure guidance must change.** "Configure UVE manually at this URL" is useless advice + here — manual configuration fails identically. The CLI must say the instance is unrecoverable + from an interrupted first boot and must be recreated with `docker compose down -v`. +- **A separate backend defect is implied**: any interrupted first boot silently bricks the + instance, and a restart neither repairs nor reports it. That is broader than this issue and is + filed separately. + +**On the readiness signal.** Switching to `/dotmgt/readyz` on 8090 is still worth doing — it is +the purpose-built probe and does not depend on the web app — but it is a correctness tidy-up, +not the fix for the 403. Experiment 1 shows `/api/v1/appconfiguration` is not meaningfully late. + +*Evidence limits*: one host, one starter, one image; the kill point was 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, not a CLI one. **Cause 3 — the CLI discards recoverable state.** Independent of causes 1 and 2, and the reason a transient failure becomes total loss. Verified in-repo: @@ -230,11 +288,14 @@ runtime, so it reaches every already-installed CLI immediately):* **Explicitly out of scope / non-goals**: -- **Changing `user.isAdmin()` exception handling** (`com/liferay/portal/model/User.java:321`) - or `AppsAPIImpl.userDoesNotHaveAccess()`. This is a real defect — a transient role-lookup - failure should not read as a permission denial — but it is legacy Liferay code on a hot - permission path with a wide blast radius, and the P0 fix does not depend on it. Tracked as a - P2 follow-up, specified and planned separately. +- **The backend defect behind the 403.** Planning disproved the original `user.isAdmin()` + exception-swallowing hypothesis: the permission rows are genuinely absent, so there is no + exception to swallow. The real backend defect is larger and worse — **an interrupted first boot + silently bricks the instance**: the starter import leaves site permissions unwritten, a restart + re-runs `Task00004LoadStarter` and reports success, and every Apps API call 403s forever with no + warning to the user. That is out of scope here (it is a starter-import/permissions problem in + legacy `com.dotmarketing.*`, not a CLI one) and is filed separately. This fix removes the + *trigger* by stopping the crash. - **Pinning the dotCMS image tag alongside `CUSTOM_STARTER_URL`.** Real drift risk (`latest` + hardcoded `starter-20260630`) and it intersects binding ADR-0019, so it deserves its own decision rather than being folded into a bug fix. P2 follow-up. @@ -350,8 +411,12 @@ rediscover the reasoning.)* plus a block to paste — so this is new behavior, not a restoration. `.env` is written when absent; when the scaffolded example already ships one, it is left alone and the block is printed as today. -- **AC-005**: The UVE write is gated on a successful `GET` of the same resource, and retries on - 401/403/5xx rather than failing on first response. +- **AC-005**: The UVE write is gated on a single successful `GET` of the same resource. Retry + applies to `5xx` only. It must **not** retry or poll on 403: a 403 here means the instance's + permissions were never written by an interrupted starter import, and that never clears — 193 + consecutive attempts over ~7 minutes all returned 403. On 403 the CLI skips the write and tells + the user their instance is unrecoverable and must be recreated with `docker compose down -v`, + rather than offering manual UVE setup steps that would fail identically. - **AC-006** *(P1)*: With a healthy dotCMS already answering on 8082, a second run offers to reuse it instead of aborting with "Required ports are already in use". Reproduction step 6 no longer blocks. From 190efea4170c54cdd8d3be275d1a7b8598a1814a Mon Sep 17 00:00:00 2001 From: Freddy Montes Date: Fri, 28 Aug 2026 09:03:05 -0600 Subject: [PATCH 05/11] docs(create-app): cross-reference the backend defect as #37268 The interrupted-first-boot corruption behind the 403 is filed as #37268. Refs #37262 Co-Authored-By: Claude Opus 5 (1M context) --- specs/37262-create-app-docker-uve/spec.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/specs/37262-create-app-docker-uve/spec.md b/specs/37262-create-app-docker-uve/spec.md index b28d33fc4c4..ff3b0bc1d69 100644 --- a/specs/37262-create-app-docker-uve/spec.md +++ b/specs/37262-create-app-docker-uve/spec.md @@ -200,7 +200,7 @@ the permission data is genuinely absent, so `isAdmin()` has nothing to throw abo from an interrupted first boot and must be recreated with `docker compose down -v`. - **A separate backend defect is implied**: any interrupted first boot silently bricks the instance, and a restart neither repairs nor reports it. That is broader than this issue and is - filed separately. + filed as **#37268**. **On the readiness signal.** Switching to `/dotmgt/readyz` on 8090 is still worth doing — it is the purpose-built probe and does not depend on the web app — but it is a correctness tidy-up, @@ -294,7 +294,7 @@ runtime, so it reaches every already-installed CLI immediately):* silently bricks the instance**: the starter import leaves site permissions unwritten, a restart re-runs `Task00004LoadStarter` and reports success, and every Apps API call 403s forever with no warning to the user. That is out of scope here (it is a starter-import/permissions problem in - legacy `com.dotmarketing.*`, not a CLI one) and is filed separately. This fix removes the + legacy `com.dotmarketing.*`, not a CLI one) and is filed as **#37268**. This fix removes the *trigger* by stopping the crash. - **Pinning the dotCMS image tag alongside `CUSTOM_STARTER_URL`.** Real drift risk (`latest` + hardcoded `starter-20260630`) and it intersects binding ADR-0019, so it deserves From 62dca194984dde088d45cea3f03191f79244c6d9 Mon Sep 17 00:00:00 2001 From: Freddy Montes Date: Fri, 28 Aug 2026 09:17:37 -0600 Subject: [PATCH 06/11] docs(create-app): fix Fix Scope still instructing the removed UVE poll MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit AC-005 was corrected when the diagnosis changed, but the matching Fix Scope bullet was not — leaving the spec contradicting itself: one section said "poll GET until 200, retry on 401/403/5xx" while the other forbade exactly that. Caught by /speckit-analyze. Fix Scope now specifies a single probe, retry on 5xx only, and no retry on 403. Also splits the non-fatal UVE guidance in two, because the cases need opposite advice. On 403 the instance's permissions were never written, so manual UVE setup fails identically — pointing the user at the configuration guide would send them down a path that cannot work. That case tells them to recreate the instance and references #37268. Every other failure keeps the guide link plus host, site ID and app key. Refs #37262, #37268 Co-Authored-By: Claude Opus 5 (1M context) --- specs/37262-create-app-docker-uve/spec.md | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/specs/37262-create-app-docker-uve/spec.md b/specs/37262-create-app-docker-uve/spec.md index ff3b0bc1d69..e5026e12e85 100644 --- a/specs/37262-create-app-docker-uve/spec.md +++ b/specs/37262-create-app-docker-uve/spec.md @@ -269,13 +269,20 @@ runtime, so it reaches every already-installed CLI immediately):* - The CLI never exits without printing recoverable state (host, token, site ID), and writes the `.env` it already has every value for. -- UVE setup failure is non-fatal: warn, print manual setup steps, and continue to scaffolding. - The warning must link the user to the official headless UVE configuration guide — - - — alongside the concrete values they need (host, site ID, and the app key - `dotema-config-v2`), so the one unset setting is self-serviceable rather than a dead end. -- Gate the UVE write on a read — poll `GET` on the UVE app endpoint until 200, then `POST` with - retry on 401/403/5xx. +- UVE setup failure is non-fatal: warn and continue to scaffolding. The message depends on why + it failed, because the two cases need opposite advice: + - **403 (terminal)** — the instance's permissions were never written by an interrupted first + boot. Manual UVE configuration would fail identically, so **do not** offer the manual steps. + Tell the user the instance is unrecoverable and to run + `docker compose down -v && docker compose up -d --wait`, and reference #37268. + - **anything else** — link the official headless UVE configuration guide, + , + alongside the concrete values needed (host, site ID, app key `dotema-config-v2`), so the one + unset setting is self-serviceable rather than a dead end. +- Probe before the UVE write — a **single** `GET` on the UVE app endpoint; on 200, `POST` with + retry on `5xx` **only**. Never poll, and never retry a 403: measurement showed a 403 here is + terminal (193 consecutive failures over ~7 minutes), so a poll would spin forever. On 403 the + CLI reports the instance as unrecoverable and stops — see the terminal-403 message below. *CLI (P1):* From 5d4cf45fbe4b35c6de8bb740d2d5de09f6fb5357 Mon Sep 17 00:00:00 2001 From: Freddy Montes Date: Fri, 28 Aug 2026 14:34:49 -0600 Subject: [PATCH 07/11] =?UTF-8?q?docs(create-app):=20rescope=20=E2=80=94?= =?UTF-8?q?=20the=20CLI=20ships=20its=20own=20compose=20file?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The fix no longer modifies docker/docker-compose-examples/single-node-demo-site/docker-compose.yml. The CLI gets its own file, bundled in the npm package, and the shared example is left exactly as it is for README readers and installed CLIs. Reason: every hardening step this fix wanted was otherwise a behavior change shipped unversioned to consumers who never asked for it, because that file is fetched from main at runtime. Gating dotcms on opensearch health was the sharpest case — it introduces a way for dotCMS to never start if the probe later breaks (an opensearch:1 -> :2 bump invalidating admin:admin), where today it starts regardless. Owning the file makes strictness free. This removes what the spec itself called the single largest regression risk in the work. Two smaller risks replace it, both recorded: the bundled asset must be listed in package.json `files` AND project.json esbuild `assets` or it ships missing and every local-Docker run fails at step one (new AC-013); and strict gating means a future broken opensearch probe stops dotCMS starting, contained to this CLI's own stack. Accepted consequence: users on <=1.2.5 keep the old shared file and are not repaired. This starts fresh local instances rather than serving CI, no known users have it in CI, and npx resolves to the latest published version. AC-009 now requires continuous feedback for the whole wait, not just visible pull progress — ten minutes of frozen spinner is the failure this issue was reported for. AC-010 inverts to asserting docker/docker-compose-examples/* is UNCHANGED, verified by diff. Reviewer rationale updated: the blame-derived reviewers were chosen for a shared file this work no longer touches. Refs #37262 Co-Authored-By: Claude Opus 5 (1M context) --- specs/37262-create-app-docker-uve/spec.md | 143 ++++++++++++---------- 1 file changed, 81 insertions(+), 62 deletions(-) diff --git a/specs/37262-create-app-docker-uve/spec.md b/specs/37262-create-app-docker-uve/spec.md index e5026e12e85..0b164e0362e 100644 --- a/specs/37262-create-app-docker-uve/spec.md +++ b/specs/37262-create-app-docker-uve/spec.md @@ -244,26 +244,38 @@ a transient failure becomes total loss. Verified in-repo: **In scope**: -*Compose (P0 — ships without a CLI release, because the CLI fetches this file from `main` at -runtime, so it reaches every already-installed CLI immediately):* - -- Add an `opensearch` healthcheck and `restart: unless-stopped`. -- Change `dotcms` `depends_on` to `condition: service_healthy` for both `db` and `opensearch`. - Gating on `db` is what fixes the reported crash. Gating on `opensearch` as well is a - **deliberate deviation** from all four existing examples, which use `service_started` there — - justified because this stack is driven by an unattended CLI, so an OpenSearch that is - up-but-not-ready is a failure with nobody present to diagnose it. The OpenSearch probe should - be the one already proven in `single-node-os-migration` (`-k` for the self-signed cert, - `-u admin:admin`, since this stack sets `DOT_ES_AUTH_BASIC_PASSWORD: 'admin'`), not a new one. -- Add a `dotcms` healthcheck against `http://127.0.0.1:8090/dotmgt/livez` with - `start_period: 180s`, plus `restart: unless-stopped`. -- Publish the management port **bound to loopback**: `127.0.0.1:8090:8090` — not `8090:8090`. - The management port is authorized purely by the port a request arrives on - (`InfrastructureManagementFilter`): no credential check, no IP allowlist. A wildcard binding - would put `/dotmgt/health` and `/dotmgt/metrics` on the local network for every user of this - demo stack. Loopback gives the CLI and the container's own healthcheck everything they need - (both already use `127.0.0.1`) and gives the network nothing. This is stricter than the two - existing examples that publish 8090. +*Compose — the CLI ships its own file (P0):* + +The CLI gets its **own** compose file, bundled in the npm package at +`core-web/libs/sdk/create-app/assets/docker-compose.yml`. The shared +`docker/docker-compose-examples/single-node-demo-site/docker-compose.yml` is **not changed** — +it keeps serving README readers and already-installed CLIs exactly as today. + +This reverses the original plan, which hardened the shared file. Owning the file removes the +largest risk in this work: every hardening step we want (gating on OpenSearch health, publishing +8090, healthchecks that `--wait` depends on) was otherwise 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`. + +- `db` and `opensearch` both get healthchecks and `restart: unless-stopped`. The OpenSearch probe + is the one proven in `single-node-os-migration`, **verified on this stack at ~15s**. +- `dotcms` `depends_on` gates on `condition: service_healthy` for **both**. Safe here in a way it + was not on the shared file: nothing else reads this one. +- `dotcms` healthcheck on `http://127.0.0.1:8090/dotmgt/livez`, `start_period: 120s` (~2.5× the + measured ~46s boot), plus `restart: unless-stopped`. +- Management port published **loopback-only**: `127.0.0.1:8090:8090`. It is authorized purely by + arrival port — no credential check, no IP allowlist — so a wildcard binding would put + `/dotmgt/health` and `/dotmgt/metrics` on the local network. +- The file is **bundled, not downloaded**, removing `downloadFile`'s missing timeout, absent + redirect handling and lack of retry from the default path. A `ComposeSource` interface keeps + remote fetching one env var away (`DOTCMS_COMPOSE_URL`) for field hotfixes. +- The dotCMS image tag stays `latest` for now, so the drift the report flagged remains open. + +**Accepted consequence**: users on `@dotcms/create-app` ≤1.2.5 keep fetching the old shared file and +are not repaired. This is a tool for starting fresh local instances, 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. *CLI (P0):* @@ -317,49 +329,50 @@ runtime, so it reaches every already-installed CLI immediately):* ## Regression Risk *(mandatory)* - **Blast radius**: - - *Compose:* `single-node-demo-site/docker-compose.yml` is fetched from `main` at runtime by - every installed `@dotcms/create-app`, so a change ships instantly and unversioned to all - existing CLI users — including older CLI versions that will not know about port 8090. This - cuts both ways: it is why the fix is P0 and reaches users without a release, and it is the - single largest regression risk in this work. The file is also used directly by users - following the demo-site README. The added `dotcms` healthcheck must not fail on images - where `/dotmgt/livez` behaves differently. Note the failure mode is **not** a restart loop: - Compose restart policies react to container *exit*, not to health status (health-driven - restart is a Swarm feature), so an unhealthy container is simply never marked ready. The - consequence is that `depends_on` and `--wait` block on it — see the CLI bullet below. - - *Compose ↔ installed CLIs:* the file MUST keep a line matching - `/^(\s*["']?CUSTOM_STARTER_URL["']?\s*:\s*).+$/m`. `updateDockerComposeStarterUrl` - (`src/index.ts:487`) rewrites the compose file with that regex when `--starter` is passed - and **throws if there is no match**. Reformatting that key into a YAML block scalar, an - anchor, or a `- KEY=value` list entry would break `--starter` for every already-installed - CLI — with no release able to reach them. - - *CLI:* making UVE failure non-fatal changes the exit contract — a run that previously - exited 1 will now exit 0 with a warning. Any CI or script asserting on the old behavior - would see a behavior change. `--wait` on `docker compose up` changes how long the command - blocks and requires the healthchecks above to be correct: a wrong probe turns a working run - into a block until timeout. This — not restart flapping — is the real cost of getting the - healthcheck wrong, so `--wait` must be paired with an explicit `--wait-timeout` and a - timeout must degrade to reported diagnostics rather than a silent hang. - - Fixing the truthy-`Result` check at `src/index.ts:597` makes a previously-unreachable - failure branch reachable: runs with a broken npm that silently "succeeded" before will now - correctly fail. This is the intended fix, but it is a visible behavior change. + - *Compose:* **substantially reduced by the rescope.** The CLI's compose file is bundled in its + own npm package, so it reaches only users of the version that ships it. The shared + `single-node-demo-site` example — which is fetched from `main` by every installed CLI and read + directly by README users — is **not modified**, so it carries no risk at all. This was + previously the single largest regression risk in the work; owning the file removes it. + - *What the rescope costs:* users on ≤1.2.5 keep the old, broken shared file. Accepted (see Fix + Scope) because this starts fresh local instances rather than serving CI, and `npx` resolves to + the latest published version anyway. + - *New risk introduced by bundling:* if the compose asset is not listed in `package.json` `files` + **and** `project.json`'s esbuild `assets`, it ships missing and **every** local-Docker run fails + at the first step. Covered by AC-013 — this is the most likely way to break the release. + - *New risk introduced by strict gating:* `dotcms` now waits for `opensearch` healthy. If that + probe ever breaks — an `opensearch:1` → `:2` bump invalidating `admin:admin` is the realistic + case — dotCMS will not start at all, where today it would. Contained to this CLI's own stack, + and the probe is verified working at ~15s, but it is a genuine new failure mode. + - *CLI:* making UVE failure non-fatal changes the exit contract — a run that previously exited 1 + now exits 0 with a warning. Any CI or script asserting the old behavior would see it. + - Fixing the truthy-`Result` check at `src/index.ts:597` makes a previously-unreachable failure + branch reachable: runs with a broken npm that silently "succeeded" before will now correctly + fail. Intended, but a visible behavior change. + - *`CUSTOM_STARTER_URL`:* the file must keep a line matching + `/^(\s*["']?CUSTOM_STARTER_URL["']?\s*:\s*).+$/m`, or `updateDockerComposeStarterUrl` throws + and `--starter` breaks. Now scoped to this CLI version rather than every installed one, but + still a silent break that only the guard in `scripts/verify-cold-start.sh` would catch. - **Backward compatibility**: No dotCMS API contract, serialized state, DB schema or ES mapping changes. Publishing 8090 exposes an **unauthenticated** management surface - (`/dotmgt/health`, `/dotmgt/metrics`) to whoever can reach the binding — which is why the fix - binds it to `127.0.0.1` rather than the wildcard the issue originally proposed. It must still - be stated in the README rather than introduced silently. Per ADR-0019, a CLI change ships as - part of a dotCMS release, not a standalone SDK publish. + (`/dotmgt/health`, `/dotmgt/metrics`) to whoever can reach the binding — which is why it is bound + to `127.0.0.1` rather than the wildcard the issue originally proposed, and must be stated in the + CLI's README rather than introduced silently. Per ADR-0019, a CLI change ships as part of a + dotCMS release, not a standalone SDK publish. ADR-0019 alignment of the **image tag** is + explicitly deferred — the bundled file still uses `latest`, so the drift the report flagged + (`latest` + hardcoded `starter-20260630`) remains open. - **Data considerations**: None. No migration, no repair of existing data. Users left with an empty directory by the old behavior simply re-run; the P1 port-reuse work is what makes that re-run possible without tearing down a healthy instance. ### Required reviewers for the compose change -`.github/CODEOWNERS` does not cover `docker/`, so the compose file has no automatic reviewer -despite being the highest-blast-radius part of this fix. Reviewers are therefore drawn from -`git blame` on the exact hunks being changed. Note that the two largest raw blame counts are -mechanical — a bulk restore (#27432) and a `pgvector` version bump (#29915) — so the list below -weights *who shaped the design* over line count: +**Rescoped.** These reviewers were selected by `git blame` on the shared +`single-node-demo-site/docker-compose.yml` back when this work modified it. **It no longer does** — +the CLI ships its own file — so the blast radius that made their review essential is gone. They are +still the right people to sanity-check a *new* dotCMS compose stack (healthcheck shapes, ordering, +port exposure), but this is now a review of new code in the SDK tree rather than a change to +infrastructure they own. `.github/CODEOWNERS` covers neither path. | Reviewer | Why they should review | | --- | --- | @@ -431,15 +444,21 @@ rediscover the reasoning.)* `src/index.ts:597` is reachable and correct. - **AC-008** *(P1)*: If scaffolding fails after `moveDockerComposeOneLevelUp()`, the compose file is restored to the project directory (no orphan in the parent). -- **AC-009**: Image-pull progress is visible during `docker compose up`; no silent multi-minute - spinner. Retry messages do not interleave with an active `ora` spinner. -- **AC-010**: No regression to the other `docker/docker-compose-examples/*` stacks, and the - demo-site README documents the newly published 8090 port. +- **AC-009**: Feedback is **continuous for the entire wait**, which may be up to ten minutes + (`--wait-timeout 600`). Both required: `docker compose up --wait`'s own per-container + `Waiting → Healthy` transitions are streamed rather than swallowed, and a ticker shows elapsed + time plus per-service state, refreshed every ~2s. Image-pull progress is visible. Retry messages + do not interleave with an active `ora` spinner. +- **AC-010**: `docker/docker-compose-examples/*` is **unchanged** by this work — verified by diff, + not by inspection. The CLI's README documents the bundled compose file, the loopback 8090 port, + and the `DOTCMS_COMPOSE_URL` override. - **AC-011**: Port 8090 is reachable on `127.0.0.1` and **not** on the host's LAN address — the management surface is not exposed to the network. -- **AC-012**: `npx @dotcms/create-app --starter ` still works against the edited compose - file. The `CUSTOM_STARTER_URL` rewrite regex in `updateDockerComposeStarterUrl` must still - match, or every already-installed CLI loses `--starter`. +- **AC-012**: `npx @dotcms/create-app --starter ` still works against the bundled compose + file — the `CUSTOM_STARTER_URL` rewrite regex in `updateDockerComposeStarterUrl` must still match. +- **AC-013**: The bundled compose file is actually present in the published package. `package.json` + `files` and `project.json`'s esbuild `assets` must both list it, or it ships missing and every + local-Docker run fails at the first step. - **Verification method**: - *Compose:* `docker compose -f docker/docker-compose-examples/single-node-demo-site/docker-compose.yml up -d --wait` From 40fbc4e8206a67221d0038191841372a4e52e3da Mon Sep 17 00:00:00 2001 From: Freddy Montes Date: Mon, 31 Aug 2026 07:40:36 -0600 Subject: [PATCH 08/11] docs(create-app): settle the four open questions from spec review Review against the tree turned up five places where the spec and the code disagreed, and left four questions for the reviewer to decide. All are now resolved in the spec so PR 1 is approving what will actually be built. Decisions: * start_period is 180s, not the two conflicting values the spec carried (120s in Fix Scope, 180s in the review question). ~4x the measured ~46s boot and above both in-repo precedents. * The stricter opensearch: service_healthy gate is kept, using single-node-os-migration's proven probe. A credential-free variant was considered and rejected: the admin:admin coupling is contained by the major-version tag pin, and a proven probe beats an unproven one on the critical path. * The two duplicated UVE call sites collapse into one configureUVE() owner that contains no process.exit, rather than being patched in place. * AC-012 is enforced by a Jest spec against the real bundled asset. The scripts/verify-cold-start.sh reference is removed - that file does not exist and nothing tasked its creation. Corrections found while applying them: * Two wrong premises about start_period. A too-short window does not make `docker compose up --wait` block until timeout; it marks the container unhealthy and makes --wait abort early, abandoning an instance that would have been healthy, and restart: unless-stopped cannot rescue it because restart policies react to exit, not health. This is why overshooting start_period is free. * The 403 guidance cannot be shared between the two paths. On the local stack it is the bricked boot and `docker compose down -v` is the fix; on a user-supplied server there is no stack to recreate and a 403 means the token lacks permission on the site, where manual UVE setup does work. configureUVE() now takes a mode and AC-005 specifies both messages. * The compose verification method targeted the shared single-node-demo-site file that AC-010 requires to be unchanged, and which has no dotcms healthcheck for --wait to assess. Retargeted at the bundled asset. * Sibling counts corrected: four files use condition: service_healthy and three gate dotcms directly. single-node-os-migration gates dotcms on OpenSearch health transitively via provision jobs, so the stricter gate has prior art here and is not the clean break from precedent the spec claimed. One item remains open and is blocking: /dotmgt/livez is unconfirmed on the released dotcms/dotcms:latest image, and the whole compose design depends on it. Refs #37262 Co-Authored-By: Claude Opus 5 (1M context) --- specs/37262-create-app-docker-uve/spec.md | 179 +++++++++++++++------- 1 file changed, 126 insertions(+), 53 deletions(-) diff --git a/specs/37262-create-app-docker-uve/spec.md b/specs/37262-create-app-docker-uve/spec.md index 0b164e0362e..a9c3ab26c06 100644 --- a/specs/37262-create-app-docker-uve/spec.md +++ b/specs/37262-create-app-docker-uve/spec.md @@ -101,12 +101,18 @@ original report read it as transient — it is not. See Root-Cause Hypothesis, C - **Affected area**: Three surfaces, in priority order. 1. **Docker compose examples** — `docker/docker-compose-examples/single-node-demo-site/docker-compose.yml`. - This is the least hardened example in that directory. **Three** siblings already use - `condition: service_healthy` (`lgtm-observability`, `single-node-metrics-monitoring`, - `experiments`, plus `single-node-os-migration` via provision jobs) — - `lgtm-observability/docker-compose.yml` is the model. Note none of them gates `dotcms` - on OpenSearch being *healthy*; every one uses `db: service_healthy` + - `opensearch: service_started`. See Fix Scope for why this fix goes further. + This is the least hardened example in that directory. **Four** files in it use + `condition: service_healthy`; **three** of them gate `dotcms` directly + (`lgtm-observability` L88–96, `single-node-metrics-monitoring` L86–90, `experiments` + L137–141), and all three use `db: service_healthy` + `opensearch: service_started`. + `lgtm-observability/docker-compose.yml` is the model for the healthcheck shape — but note + it publishes 8090 on the **wildcard** (L195), which this fix deliberately does not copy. + The fourth, `single-node-os-migration`, gates `dotcms` on two provision jobs + (`service_completed_successfully`, L229–235) that each require + `opensearch: service_healthy` (L167–169, L189–191) — so it **does** gate on OpenSearch + health, transitively. This fix's stricter gate therefore has prior art in this repo, one + step removed; it is not the clean break from precedent an earlier draft of this spec + claimed. See Fix Scope. Verified in-repo: `dotcms` has `depends_on: [db, opensearch]` with no condition, no `restart:`, no healthcheck, and does not publish 8090; `opensearch` has no healthcheck and no `restart:`; only `db` defines a healthcheck, which nothing consumes. @@ -213,9 +219,17 @@ missing is a backend question, not a CLI one. **Cause 3 — the CLI discards recoverable state.** Independent of causes 1 and 2, and the reason a transient failure becomes total loss. Verified in-repo: -- UVE setup exits at `src/index.ts:370`; the clone and `npm install` at `:377` never run. +- UVE setup exits at `src/index.ts:369–371` (the `process.exit(1)` is `:371`, the `spinner.fail` + is `:370`); the compose move at `:376`, the clone and `npm install` at `:377` never run. +- **The same fatal block exists twice.** `src/index.ts:226–228` is byte-for-byte the same check on + the existing-instance (`--dotcms-url`) path, with `startScaffoldingFrontEnd()` at `:232` and + `displayFinalSteps()` at `:235` equally downstream of it. Both sites discard a working token. + The two paths need *opposite* 403 advice, however: on the local-Docker path a 403 means the + bricked boot and `docker compose down -v` is the fix, while on the existing-instance path the + user supplied their own server, there is no compose stack to recreate, and a 403 means a genuine + token or site-permission problem. See Fix Scope for the single owner that resolves this. - Token and site ID are obtained successfully but only printed by `displayFinalSteps()`, which - is downstream of the exit. + is downstream of the exit on both paths. - The UVE call has no retry, while authentication retries 3×. - `checkPortsAvailability()` (`src/utils/index.ts:479`) hard-fails on 8082/8443/9200/9600 — exactly the ports a successful previous run now holds. @@ -259,11 +273,21 @@ dotCMS to **never start** if that probe later broke, e.g. an `opensearch:1` → invalidating `admin:admin`. - `db` and `opensearch` both get healthchecks and `restart: unless-stopped`. The OpenSearch probe - is the one proven in `single-node-os-migration`, **verified on this stack at ~15s**. + is the one proven in `single-node-os-migration` (L61–65), + `curl -sk https://localhost:9200 -u admin:admin | grep -q cluster_name`, + **verified on this stack at ~15s**. **Decided** over a credential-free variant (accept `200` or + `401` from the HTTP layer, which would drop the `admin:admin` coupling entirely): the coupling is + the probe's only real exposure, and it is contained — the image tag is pinned to major `1`, so + the `opensearch:1` → `:2` bump that would invalidate the default credentials requires a + deliberate edit to *this* file by someone who then owns the probe. A proven probe beats an + unproven one on the critical path. - `dotcms` `depends_on` gates on `condition: service_healthy` for **both**. Safe here in a way it - was not on the shared file: nothing else reads this one. -- `dotcms` healthcheck on `http://127.0.0.1:8090/dotmgt/livez`, `start_period: 120s` (~2.5× the - measured ~46s boot), plus `restart: unless-stopped`. + was not on the shared file: nothing else reads this one. There is also transitive prior art for + it in `single-node-os-migration` — see Scope of Investigation. +- `dotcms` healthcheck on `http://127.0.0.1:8090/dotmgt/livez`, `start_period: 180s` (~4× the + measured ~46s boot), plus `restart: unless-stopped`. Overshooting `start_period` is free — the + first successful probe ends the window immediately — while undershooting it is not: see + Regression Risk. - Management port published **loopback-only**: `127.0.0.1:8090:8090`. It is authorized purely by arrival port — no credential check, no IP allowlist — so a wildcard binding would put `/dotmgt/health` and `/dotmgt/metrics` on the local network. @@ -279,14 +303,27 @@ so only a warm npx cache stays behind. *CLI (P0):* +- **A single owner for UVE configuration.** Both call sites — `src/index.ts:226` (existing + instance) and `: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 cause-specific messaging, and it **never + calls `process.exit`** — it returns an outcome the caller warns on and continues past. + **Decided** over fixing the two sites in place: the exit contract then lives in one place rather + than two that have already drifted once, and a third call site cannot silently miss it. This is + the only structural change in scope — the `Result` type, prompt flow and scaffolding logic stay + untouched (see non-goals). - The CLI never exits without printing recoverable state (host, token, site ID), and writes the - `.env` it already has every value for. -- UVE setup failure is non-fatal: warn and continue to scaffolding. The message depends on why - it failed, because the two cases need opposite advice: - - **403 (terminal)** — the instance's permissions were never written by an interrupted first - boot. Manual UVE configuration would fail identically, so **do not** offer the manual steps. - Tell the user the instance is unrecoverable and to run + `.env` it already has every value for. This holds on **both** paths. +- UVE setup failure is non-fatal: warn and continue to scaffolding. The message depends on why it + failed **and on which path is running**, because the cases need opposite advice: + - **403, `mode: 'local'` (terminal)** — the instance's permissions were never written by an + interrupted first boot. Manual UVE configuration would fail identically, so **do not** offer + the manual steps. Tell the user the instance is unrecoverable and to run `docker compose down -v && docker compose up -d --wait`, and reference #37268. + - **403, `mode: 'remote'`** — the user supplied their own server. There is no stack to recreate, + so `docker compose down -v` would be nonsense advice here. Report that the API token lacks + permission on the resolved site, name the site ID and app key `dotema-config-v2`, and link the + guide below so the setting can be applied by hand — which on this path will work. - **anything else** — link the official headless UVE configuration guide, , alongside the concrete values needed (host, site ID, app key `dotema-config-v2`), so the one @@ -321,9 +358,12 @@ so only a warm npx cache stays behind. - **Landing the E2E suite from #35096.** That issue owns it. This fix adds unit-level tests for the logic it changes; the fault-injection E2E case (kill dotCMS mid-run) belongs to #35096. - Rewriting the CLI's `Result` type, prompt flow, or framework-scaffolding logic beyond the - specific defects listed above. + specific defects listed above. The one exception, stated in Fix Scope, is extracting + `configureUVE()` so the two duplicated UVE call sites share a single owner — that is a + precondition for AC-003 holding on both paths, not a general refactor. - Hardening the other compose examples in `docker/docker-compose-examples/`. Only - `single-node-demo-site` is fetched by the CLI; the rest are out of this fix's blast radius. + `single-node-demo-site` is fetched by CLI versions ≤1.2.5; the rest are out of this fix's blast + radius, and from this version on the CLI fetches none of them. - Any change to the Apps REST API contract or the UVE app definition itself. ## Regression Risk *(mandatory)* @@ -351,8 +391,16 @@ so only a warm npx cache stays behind. fail. Intended, but a visible behavior change. - *`CUSTOM_STARTER_URL`:* the file must keep a line matching `/^(\s*["']?CUSTOM_STARTER_URL["']?\s*:\s*).+$/m`, or `updateDockerComposeStarterUrl` throws - and `--starter` breaks. Now scoped to this CLI version rather than every installed one, but - still a silent break that only the guard in `scripts/verify-cold-start.sh` would catch. + and `--starter` breaks. Substantially de-risked by the rescope: the regex and the file it + rewrites now ship in the same package, same repo, same version, so they cannot drift + independently the way they could when the file was fetched from `main` at runtime. Guarded by a + Jest spec that runs the rewrite against the real bundled asset — see AC-012. + - *`start_period` too short:* if the window elapses while dotCMS is still booting, probe failures + begin counting toward `retries` and the container is marked `unhealthy`, at which point + `docker compose up --wait` **aborts** rather than waiting — the CLI gives up on an instance that + would have been healthy moments later, and `restart: unless-stopped` will not rescue it because + restart policies react to exit, not health. This is why `start_period` is set generously at + `180s`: the first successful probe ends the window immediately, so overshooting costs nothing. - **Backward compatibility**: No dotCMS API contract, serialized state, DB schema or ES mapping changes. Publishing 8090 exposes an **unauthenticated** management surface (`/dotmgt/health`, `/dotmgt/metrics`) to whoever can reach the binding — which is why it is bound @@ -390,25 +438,33 @@ This leaves a real coverage gap: **nobody currently assignable designed the patt The plan phase should treat the lgtm-observability compose as the specification for correct usage — reading it directly rather than relying on a reviewer to catch a faithful-copy error. -Two review questions to put to them explicitly. Both were sharpened by the plan-phase research — -question 1 now carries a recommendation rather than being open, and question 2 was reframed -because its original premise was wrong: - -1. **Gating `dotcms` on `opensearch: service_healthy`** goes further than all four existing - examples, which use `service_started` there. Is the stricter gate right for a stack driven by - an unattended CLI, or should this match the house pattern? *(Recommendation: keep the stricter - gate, using `single-node-os-migration`'s proven probe.)* -2. **Is `start_period: 180s` enough headroom** for a cold demo-starter import on a slow machine? - This is above both precedents (lgtm 120s, metrics-monitoring 20s). The original question — - whether `restart: unless-stopped` would flap the container — turned out to rest on a false - premise: Compose restart policies react to container exit, not health status, so flapping - cannot occur. The real exposure is that a too-short `start_period` makes - `docker compose up --wait` block until timeout. - -*(Publishing 8090 was the third open question. It is now settled in Fix Scope: bind to -`127.0.0.1`, because the management port is authorized by arrival port with no credential check -and no IP allowlist. Flagging it here so reviewers see the decision rather than having to -rediscover the reasoning.)* +**All open questions are now settled.** Both review questions this section previously put to +reviewers have been decided by the issue owner, and the decisions are recorded in Fix Scope rather +than left for the reviewer to resolve. Kept here with their reasoning so reviewers see what was +chosen and can object, instead of having to rediscover it: + +1. **Gate `dotcms` on `opensearch: service_healthy`** — **decided: keep the stricter gate**, using + `single-node-os-migration`'s proven probe (L61–65). The framing this question originally carried + was wrong: it is not a clean break from precedent, because `single-node-os-migration` already + gates `dotcms` on OpenSearch health transitively, via provision jobs. A credential-free probe was + considered and rejected — the `admin:admin` coupling is contained by the major-version tag pin, + and a proven probe beats an unproven one on the critical path. See Scope of Investigation. +2. **`start_period`** — **decided: `180s`**, ~4× the measured ~46s boot and above both precedents + (lgtm 120s, metrics-monitoring 20s). Two earlier premises here were both wrong. The original + question, whether `restart: unless-stopped` would flap the container, rests on a false premise: + Compose restart policies react to container exit, not health status, so flapping cannot occur. + Its replacement — that a too-short `start_period` makes `docker compose up --wait` block until + timeout — is also wrong, and backwards: a too-short window marks the container `unhealthy` and + makes `--wait` **abort early**, which is the worse failure. Because the first successful probe + ends the window immediately, overshooting is free. See Regression Risk. +3. **Publishing 8090** — **decided: bind to `127.0.0.1`**, because the management port is authorized + by arrival port with no credential check and no IP allowlist. Note this deviates from + `lgtm-observability`, which binds the wildcard (L195); the deviation is deliberate. + +What remains for reviewers is not a decision but a **confirmation**: `/dotmgt/livez` must be +verified on the released `dotcms/dotcms:latest` image before the healthcheck can depend on it. Both +in-repo examples that probe it run `dotcms/dotcms-test`. This is the first step of the plan's +verification guide, and a failure there invalidates the compose design. See Assumptions. ## Acceptance & Verification *(mandatory)* @@ -419,8 +475,11 @@ rediscover the reasoning.)* - **AC-002**: The CLI's "containers started successfully" message is only printed when the containers are actually running and healthy. - **AC-003**: When the UVE configuration call fails for any reason, the CLI prints a warning, - **continues to scaffolding**, and exits 0 with a complete project. Reproduction step 5 no - longer aborts the run. The warning includes the headless UVE configuration guide + **continues to scaffolding**, and exits 0 with a complete project. This holds on **both** entry + paths — local Docker and existing instance (`--dotcms-url`) — because both go through the single + `configureUVE()` owner, which contains no `process.exit`. A grep for `process.exit` in the UVE + path returning nothing is part of this criterion. Reproduction step 5 no longer aborts the run. + The warning includes the headless UVE configuration guide () and the run-specific values needed to follow it — host, site ID, and app key `dotema-config-v2`. A user who hits this path can finish the setup by hand without leaving @@ -434,9 +493,13 @@ rediscover the reasoning.)* - **AC-005**: The UVE write is gated on a single successful `GET` of the same resource. Retry applies to `5xx` only. It must **not** retry or poll on 403: a 403 here means the instance's permissions were never written by an interrupted starter import, and that never clears — 193 - consecutive attempts over ~7 minutes all returned 403. On 403 the CLI skips the write and tells - the user their instance is unrecoverable and must be recreated with `docker compose down -v`, - rather than offering manual UVE setup steps that would fail identically. + consecutive attempts over ~7 minutes all returned 403. On 403 the CLI skips the write; the + message it then prints is **mode-dependent**. In `mode: 'local'` it reports the instance + unrecoverable and to recreate it with `docker compose down -v`, and does **not** offer manual UVE + setup steps, which would fail identically. In `mode: 'remote'` it reports instead that the API + token lacks permission on the resolved site and links the manual steps, which on a user-supplied + server will work — `docker compose down -v` must never be suggested there, as there is no stack + to recreate. - **AC-006** *(P1)*: With a healthy dotCMS already answering on 8082, a second run offers to reuse it instead of aborting with "Required ports are already in use". Reproduction step 6 no longer blocks. @@ -456,26 +519,36 @@ rediscover the reasoning.)* the management surface is not exposed to the network. - **AC-012**: `npx @dotcms/create-app --starter ` still works against the bundled compose file — the `CUSTOM_STARTER_URL` rewrite regex in `updateDockerComposeStarterUrl` must still match. + Enforced by a **Jest spec that loads the real bundled asset**, asserts the regex matches it and + asserts the rewritten line, so a reformat of that line fails CI on the PR that causes it. No + Docker and no cold start are required, which is what makes this cheap enough to gate every PR; + it is available because the rescope put the regex and the file it rewrites in the same package. - **AC-013**: The bundled compose file is actually present in the published package. `package.json` `files` and `project.json`'s esbuild `assets` must both list it, or it ships missing and every local-Docker run fails at the first step. - **Verification method**: - - *Compose:* `docker compose -f docker/docker-compose-examples/single-node-demo-site/docker-compose.yml up -d --wait` + - *Compose:* against **the bundled asset**, + `docker compose -f core-web/libs/sdk/create-app/assets/docker-compose.yml up -d --wait --wait-timeout 600` from a cold state (no volumes, no cached images), asserting `dotcms` reaches healthy without manual start; plus a fault-injection run (`docker kill` the `dotcms` container) asserting it - is restarted. + is restarted. **Not** the shared `single-node-demo-site` file — AC-010 requires that one to be + unchanged, and it has no `dotcms` healthcheck for `--wait` to assess, so it cannot satisfy this + assertion. (An earlier draft named it here; that was pre-rescope language.) - *CLI unit tests:* the package contains **no spec file**, but the Jest harness is already in place (`jest.config.ts`, `tsconfig.spec.json`, `@nx/jest/plugin`), so this fix adds specs rather than a harness. Run with `pnpm nx test sdk-create-app` — note `project.json` sets `passWithNoTests: true`, so an empty run reports green; confirm the new specs are actually - collected before trusting a pass. Jest specs covering, at minimum: the `Result` truthiness fix, the non-fatal UVE - path, the read-before-write gate with a mocked 403-then-200 sequence, the `try/finally` - compose move, and the port-reuse probe. Per constitution Principle V these are written and - confirmed failing (Red) before the implementation lands. + collected before trusting a pass. Jest specs covering, at minimum: the `Result` truthiness fix; + `configureUVE()` returning a non-fatal outcome instead of exiting, exercised for **both** + `mode: 'local'` and `mode: 'remote'` so the two 403 messages are asserted separately; the + read-before-write gate with a mocked 403-then-200 sequence; the `try/finally` compose move; the + port-reuse probe; and the `CUSTOM_STARTER_URL` rewrite run against the real bundled asset + (AC-012). Per constitution Principle V these are written and confirmed failing (Red) before the + implementation lands. - *Manual end-to-end:* the reproduction steps above, run cold on macOS + Docker Desktop, verifying AC-001 through AC-006, plus AC-011 (a `curl` to the host's LAN address on 8090 - must be refused) and AC-012 (a `--starter` run against the edited file). + must be refused). - The full fault-injection E2E suite is #35096's scope, not this fix's. - **Prerequisite**: `pnpm install` in `core-web/`. Without it `pnpm nx test` fails with `nx: command not found`, and constitution Principle V's Red gate cannot be demonstrated. From 522a38c3e00b2a602cff22856bffd5c34ec5963f Mon Sep 17 00:00:00 2001 From: Freddy Montes Date: Mon, 31 Aug 2026 10:50:22 -0600 Subject: [PATCH 09/11] docs(create-app): drop the docker/ reviewer list, obsoleted by the rescope The blame-derived reviewer list was selected against the shared single-node-demo-site/docker-compose.yml, back when this work modified that file. It no longer does: the compose file that ships lives in core-web/libs/sdk/create-app/assets/ and is reviewed as SDK code with the rest of the CLI change. With no infrastructure change to gate on its owners, the list is dropped rather than carried as courtesy CCs. Two points from that section survive on their own merits and are kept: * .github/CODEOWNERS covers neither docker/ nor the SDK tree, so PR 2 draws no automatic reviewer and one has to be requested by hand. * lgtm-observability/docker-compose.yml should be read directly as the reference for correct condition: service_healthy usage - its author is no longer a collaborator, so the file is the specification. It binds 8090 on the wildcard, which this design deliberately does not copy. Section renamed from "Required reviewers for the compose change" to "Compose design decisions". The four settled decisions and the /dotmgt/livez confirmation are unchanged. Refs #37262 Co-Authored-By: Claude Opus 5 (1M context) --- specs/37262-create-app-docker-uve/spec.md | 51 ++++++++++------------- 1 file changed, 21 insertions(+), 30 deletions(-) diff --git a/specs/37262-create-app-docker-uve/spec.md b/specs/37262-create-app-docker-uve/spec.md index a9c3ab26c06..cdcc67ecef8 100644 --- a/specs/37262-create-app-docker-uve/spec.md +++ b/specs/37262-create-app-docker-uve/spec.md @@ -413,35 +413,26 @@ so only a warm npx cache stays behind. empty directory by the old behavior simply re-run; the P1 port-reuse work is what makes that re-run possible without tearing down a healthy instance. -### Required reviewers for the compose change - -**Rescoped.** These reviewers were selected by `git blame` on the shared -`single-node-demo-site/docker-compose.yml` back when this work modified it. **It no longer does** — -the CLI ships its own file — so the blast radius that made their review essential is gone. They are -still the right people to sanity-check a *new* dotCMS compose stack (healthcheck shapes, ordering, -port exposure), but this is now a review of new code in the SDK tree rather than a change to -infrastructure they own. `.github/CODEOWNERS` covers neither path. - -| Reviewer | Why they should review | -| --- | --- | -| **Will Ezell** (`wezell`) | Authored the OpenSearch 1.x + SSL setup in this file (#27754) and the Postgres 18 upgrade (#34236) that touched both this file and the model stack. Owns the current `db`/`opensearch` shape we are adding healthchecks to, and is the most senior still-active owner of `docker/`. | -| **Erick González** (`erickgonzalez`) | Most recent semantic change to the `dotcms` service (#36490, 2026-07-13) and prior starter-version work (#36362). Closest to the `CUSTOM_STARTER_URL` / starter-import behavior that the readiness race depends on. | -| **Jose Castro** (`jcastro-dotcms`) | Not a blame match on this file. Added to cover the gap below: second-most-active contributor to `docker/` over the last 12 months. | - -**Unavailable — the two strongest blame signals.** `spbolton` (Steve Bolton) holds dominant blame -on every hunk in scope *and* authored `lgtm-observability/docker-compose.yml` (#32980), the exact -`condition: service_healthy` pattern this change copies. `dcolina` (Daniel Colina) holds 13 lines -of the `opensearch` block via #29915. Neither is a collaborator on `dotCMS/core` any more (last -commits 2026-03-30 and 2026-04-07 respectively), so GitHub rejects a review request for them. - -This leaves a real coverage gap: **nobody currently assignable designed the pattern being copied.** -The plan phase should treat the lgtm-observability compose as the specification for correct -usage — reading it directly rather than relying on a reviewer to catch a faithful-copy error. - -**All open questions are now settled.** Both review questions this section previously put to -reviewers have been decided by the issue owner, and the decisions are recorded in Fix Scope rather -than left for the reviewer to resolve. Kept here with their reasoning so reviewers see what was -chosen and can object, instead of having to rediscover it: +### Compose design decisions + +**No `docker/` reviewers are required.** An earlier draft carried a `git blame`-derived reviewer +list for `single-node-demo-site/docker-compose.yml`, chosen back when this work modified that file. +The rescope means it does not: what ships is a new compose file inside +`core-web/libs/sdk/create-app/assets/`, reviewed as SDK code along with the rest of the CLI change. +There is no change to infrastructure anyone owns, so the list is dropped rather than carried as +courtesy CCs. (`.github/CODEOWNERS` covers neither path, so PR 2 draws no automatic reviewer +either way — worth knowing when requesting review.) + +One piece of guidance from that analysis survives, because it is about the code and not about who +signs off: **the plan phase should read `lgtm-observability/docker-compose.yml` directly as the +reference for correct `condition: service_healthy` usage**, rather than assume the pattern was +copied faithfully. Its author is no longer a collaborator, so there is nobody to ask — the file is +the specification. Note it binds 8090 on the wildcard (L195), which this design deliberately does +not copy. + +**All open questions are now settled.** Both questions this section previously left open have been +decided by the issue owner, and the decisions are recorded in Fix Scope. Kept here with their +reasoning so a reviewer can see what was chosen and object, rather than having to rediscover it: 1. **Gate `dotcms` on `opensearch: service_healthy`** — **decided: keep the stricter gate**, using `single-node-os-migration`'s proven probe (L61–65). The framing this question originally carried @@ -461,7 +452,7 @@ chosen and can object, instead of having to rediscover it: by arrival port with no credential check and no IP allowlist. Note this deviates from `lgtm-observability`, which binds the wildcard (L195); the deviation is deliberate. -What remains for reviewers is not a decision but a **confirmation**: `/dotmgt/livez` must be +What remains is not a decision but a **confirmation**: `/dotmgt/livez` must be verified on the released `dotcms/dotcms:latest` image before the healthcheck can depend on it. Both in-repo examples that probe it run `dotcms/dotcms-test`. This is the first step of the plan's verification guide, and a failure there invalidates the compose design. See Assumptions. From 119fc3f07190c78081e6eeb37115d5ec104d7892 Mon Sep 17 00:00:00 2001 From: Freddy Montes Date: Tue, 1 Sep 2026 07:24:16 -0600 Subject: [PATCH 10/11] docs(create-app): correct two claims implementation disproved (#37262) Both were written into the spec during review and both turned out to be wrong. Correcting them in the spec rather than leaving the implementation to silently disagree with it. AC-012 described scripts/verify-cold-start.sh as a phantom reference that nothing created, and replaced it with a Jest guard. The script exists. It ships in this package, it was already written against the bundled asset rather than the shared example, and it already carries a --static mode that runs the config-only checks with no Docker daemon - measured at 9 passed / 0 failed in 0.3s. The claim was made after checking only the spec branch, where the file genuinely is absent because it lands one PR up the stack. What WAS wrong is the path: the script is under the package's own scripts/, not the repository root. Both guards are now named, because they assert different things and neither subsumes the other: --static checks that the FILE still matches the shape installed CLIs depend on, while the Jest spec runs applyStarterUrl() and checks the FUNCTION's output. Keeping only one leaves a real gap. The image-tag deferral cited ADR-0019 as the reason it "deserves its own decision". Reading ADR-0019 in full, that is backwards: the ADR's motivating problem is precisely a client pinned to `latest` against a mismatched instance producing a cryptic runtime failure, and under date-lockstep the SDK version IS the release version - so it supplies the tag to pin rather than complicating the choice. The deferral still stands, but on scope: `latest` is what ships today, so leaving it preserves the status quo rather than introducing new drift inside a bug fix. Refs #37262 Co-Authored-By: Claude Opus 5 (1M context) --- specs/37262-create-app-docker-uve/spec.md | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/specs/37262-create-app-docker-uve/spec.md b/specs/37262-create-app-docker-uve/spec.md index cdcc67ecef8..32bdede4057 100644 --- a/specs/37262-create-app-docker-uve/spec.md +++ b/specs/37262-create-app-docker-uve/spec.md @@ -353,8 +353,14 @@ so only a warm npx cache stays behind. legacy `com.dotmarketing.*`, not a CLI one) and is filed as **#37268**. This fix removes the *trigger* by stopping the crash. - **Pinning the dotCMS image tag alongside `CUSTOM_STARTER_URL`.** Real drift risk - (`latest` + hardcoded `starter-20260630`) and it intersects binding ADR-0019, so it deserves - its own decision rather than being folded into a bug fix. P2 follow-up. + (`latest` + hardcoded `starter-20260630`). **Deferred on scope, not on ADR grounds.** An earlier + revision said it "intersects binding ADR-0019, so it deserves its own decision"; reading + ADR-0019 in full, that is backwards. The ADR's *motivating problem* is precisely a client pinned + to `latest` against a mismatched instance producing "a cryptic runtime failure", and under + date-lockstep `@dotcms/create-app@X` corresponds to dotCMS release `X` by construction — so the + ADR supplies the version to pin rather than obstructing the choice. The honest reason to defer + is that `latest` is what ships today, so leaving it preserves the status quo instead of + introducing new drift inside a bug fix. P2 follow-up. - **Landing the E2E suite from #35096.** That issue owns it. This fix adds unit-level tests for the logic it changes; the fault-injection E2E case (kill dotCMS mid-run) belongs to #35096. - Rewriting the CLI's `Result` type, prompt flow, or framework-scaffolding logic beyond the @@ -510,10 +516,15 @@ verification guide, and a failure there invalidates the compose design. See Assu the management surface is not exposed to the network. - **AC-012**: `npx @dotcms/create-app --starter ` still works against the bundled compose file — the `CUSTOM_STARTER_URL` rewrite regex in `updateDockerComposeStarterUrl` must still match. - Enforced by a **Jest spec that loads the real bundled asset**, asserts the regex matches it and - asserts the rewritten line, so a reformat of that line fails CI on the PR that causes it. No - Docker and no cold start are required, which is what makes this cheap enough to gate every PR; - it is available because the rescope put the regex and the file it rewrites in the same package. + Enforced by **two guards, neither of which subsumes the other**: + `core-web/libs/sdk/create-app/scripts/verify-cold-start.sh --static` asserts the **file** still + matches the shape installed CLIs depend on (check T008), and a Jest spec runs + `applyStarterUrl()` against the real bundled asset and asserts the **function's** output. The + `--static` mode needs no Docker daemon and completes in well under a second, so both run on + every PR. An earlier revision of this spec described that script as a phantom reference that + nothing created; **that was wrong** — it exists, it ships in this package, and it was already + written against the bundled asset. What was genuinely wrong was the path: it is under the + package's own `scripts/`, not the repository root. - **AC-013**: The bundled compose file is actually present in the published package. `package.json` `files` and `project.json`'s esbuild `assets` must both list it, or it ships missing and every local-Docker run fails at the first step. From 022385d6c7286f2a506cc3dbed1d56747374d121 Mon Sep 17 00:00:00 2001 From: Freddy Montes Date: Tue, 1 Sep 2026 11:53:11 -0600 Subject: [PATCH 11/11] docs(create-app): act on the PR #37263 review findings (#37262) Four findings from @nicobytes and @zJaaal, all verified against the tree before acting. --wait was listed under "CLI (P1)" while AC-001 and AC-002 are P0 and have no other stated mechanism (@nicobytes). Shipping P0 without it would leave both unsatisfiable. Promoted to P0, and AC-009's continuous feedback travels with it for the same reason - a ten-minute silent wait is the symptom this issue was reported for. AC-009 previously carried no tier tag at all. Also stated explicitly, since the spec never did: all P1 items are in scope for this fix, not a follow-up; the tiers say what must land for the fix to be coherent versus what makes it good, not how it is split into PRs. AC-013 had no verification method (@nicobytes), which the spec itself calls the most likely way to break the release. The gate is now named, and named carefully: it asserts the BUILD OUTPUT, not the source tree. Checking the two manifests is necessary but not sufficient - a wrong `output:` in the esbuild assets entry satisfies a manifest check and still puts the file where the package does not carry it. Likewise a Jest spec or `verify-cold-start.sh --static` resolving the asset relative to src/ passes identically whether or not it ever ships, which is precisely the failure AC-013 exists to catch. The gate is a post-build assertion over dist/ plus `npm pack --dry-run` tarball contents. The OpenSearch "prior art" framing was mine and it was wrong (@zJaaal). single-node-os-migration is the tester harness for the unreleased OpenSearch 3.x migration; its gate exists so provision jobs create search users before dotCMS connects, not as boot ordering. All three non-migration examples use `opensearch: service_started`. So the stricter gate IS a deliberate deviation from every non-migration precedent, and calling it "prior art, one step removed" made a deviation sound like house practice. Withdrawn. (The reviewer also notes that file pins opensearch:1.3.20 while the demo stack floats on :1, so "proven in single-node-os-migration" was proven against a different image - the independent ~15s measurement on this stack is what actually carries the probe.) Cause 3 said moveDockerComposeOneLevelUp() "calls process.exit(1) internally" (@zJaaal). It does not - the exits are in startScaffoldingFrontEnd at :588 and :599. Conclusion unaffected, attribution corrected. The same finding surfaced a real defect the spec had missed: both moves are `async` and called WITHOUT await, so the rename may not have landed before the clone runs against a directory that must be empty. try/finally does not fix a floating promise. AC-008 now requires the awaits as well. Still open for a decision, not actioned here: @zJaaal asks whether a one-line `db: condition: service_healthy` should land in the shared single-node-demo-site example after all. AC-010 freezes it, which leaves every CLI <=1.2.5 user and every README reader on the broken file indefinitely, and that one change carries none of the risk the rescope removed. Refs #37263, #37262 Co-Authored-By: Claude Opus 5 (1M context) --- specs/37262-create-app-docker-uve/spec.md | 71 +++++++++++++++++------ 1 file changed, 53 insertions(+), 18 deletions(-) diff --git a/specs/37262-create-app-docker-uve/spec.md b/specs/37262-create-app-docker-uve/spec.md index 32bdede4057..144a421ef74 100644 --- a/specs/37262-create-app-docker-uve/spec.md +++ b/specs/37262-create-app-docker-uve/spec.md @@ -107,12 +107,15 @@ original report read it as transient — it is not. See Root-Cause Hypothesis, C L137–141), and all three use `db: service_healthy` + `opensearch: service_started`. `lgtm-observability/docker-compose.yml` is the model for the healthcheck shape — but note it publishes 8090 on the **wildcard** (L195), which this fix deliberately does not copy. - The fourth, `single-node-os-migration`, gates `dotcms` on two provision jobs - (`service_completed_successfully`, L229–235) that each require - `opensearch: service_healthy` (L167–169, L189–191) — so it **does** gate on OpenSearch - health, transitively. This fix's stricter gate therefore has prior art in this repo, one - step removed; it is not the clean break from precedent an earlier draft of this spec - claimed. See Fix Scope. + The fourth, `single-node-os-migration`, does gate `dotcms` on OpenSearch health, but only + transitively and for an unrelated reason: it is the tester harness for the **unreleased** + OpenSearch 3.x migration, and its provision jobs (`service_completed_successfully`, + L229–235, each requiring `opensearch: service_healthy` at L167–169 and L189–191) exist so + search users are created before dotCMS connects — not as a boot-ordering practice. It is + therefore not precedent for what this fix does. **Gating `dotcms` on OpenSearch health is a + deliberate deviation from every non-migration example in this repo**, and Regression Risk + owns the failure mode it introduces. An earlier revision of this spec called it "prior art, + one step removed"; that was too generous and is withdrawn. See Fix Scope. Verified in-repo: `dotcms` has `depends_on: [db, opensearch]` with no condition, no `restart:`, no healthcheck, and does not publish 8090; `opensearch` has no healthcheck and no `restart:`; only `db` defines a healthcheck, which nothing consumes. @@ -242,9 +245,15 @@ a transient failure becomes total loss. Verified in-repo: but `src/index.ts:597` tests `if (!result)`. `Err()` returns `{ ok: false, val }` — a truthy object, so the failure branch never fires and a failed install reports success. - Orphaned compose file: `moveDockerComposeOneLevelUp()` runs at `src/index.ts:376`; if - scaffolding fails it calls `process.exit(1)` internally, so `moveDockerComposeBack()` at - `:378` never runs and `docker-compose.yml` is stranded in the parent directory. Needs - `try/finally`. + scaffolding fails, `startScaffoldingFrontEnd()` exits the process (`:588`, `:599`) so + `moveDockerComposeBack()` at `:378` never runs and `docker-compose.yml` is stranded in the + parent directory. Needs `try/finally`. (An earlier revision attributed the exit to + `moveDockerComposeOneLevelUp()` itself; that was wrong, the conclusion is unaffected.) +- **Both moves are also unawaited.** `moveDockerComposeOneLevelUp()` and + `moveDockerComposeBack()` are `async` (`src/git/index.ts:76`, `:82`, each awaiting + `fs.rename`) and are called without `await`, so the rename may not have landed before the git + clone runs against a directory that must be empty. `try/finally` alone does not fix a floating + promise. - Multi-minute silence: `execa('docker', ['compose','up','-d'])` swallows image-pull progress, leaving a frozen spinner for the length of a ~1.5GB pull on a cold machine. - Unguarded download: `downloadFile()` uses raw `https.get` with no timeout, no retry and no @@ -333,12 +342,22 @@ so only a warm npx cache stays behind. terminal (193 consecutive failures over ~7 minutes), so a poll would spin forever. On 403 the CLI reports the instance as unrecoverable and stops — see the terminal-403 message below. +- **`docker compose up -d --wait` is P0, not P1.** An earlier revision listed it under P1, which + was wrong: AC-001 ("brings the stack up **without manual intervention**") and AC-002 ("only + printed when the containers are actually running and healthy") are both P0 and have no other + stated mechanism. Shipping P0 without `--wait` would leave them unsatisfiable. The continuous + feedback AC-009 requires travels with it, for the same reason — a ten-minute silent wait is the + symptom this issue was reported for. + *CLI (P1):* +**All P1 items below are in scope for this fix**, not a follow-up. They are tiered to say what +must land for the fix to be coherent (P0) versus what makes it good (P1), not to split delivery. + - Port check probes before failing: if dotCMS already answers on 8082, offer to reuse it rather than exiting. -- Use `docker compose up -d --wait` and stream pull progress so the wait is visible. -- Fix the truthy-`Result` check at `src/index.ts:597`; wrap the compose move in `try/finally`. +- Fix the truthy-`Result` check at `src/index.ts:597`; wrap the compose move in `try/finally` + **and await both moves** — they are `async` and currently called without `await`. - Switch readiness to `/dotmgt/readyz` on 8090 once the compose publishes it, keeping `/api/v1/appconfiguration` as fallback. @@ -442,8 +461,9 @@ reasoning so a reviewer can see what was chosen and object, rather than having t 1. **Gate `dotcms` on `opensearch: service_healthy`** — **decided: keep the stricter gate**, using `single-node-os-migration`'s proven probe (L61–65). The framing this question originally carried - was wrong: it is not a clean break from precedent, because `single-node-os-migration` already - gates `dotcms` on OpenSearch health transitively, via provision jobs. A credential-free probe was + was wrong in the other direction: `single-node-os-migration` is a migration harness whose gate + serves provision ordering, so it is **not** precedent, and the stricter gate is a deliberate + deviation from every non-migration example. A credential-free probe was considered and rejected — the `admin:admin` coupling is contained by the major-version tag pin, and a proven probe beats an unproven one on the critical path. See Scope of Investigation. 2. **`start_period`** — **decided: `180s`**, ~4× the measured ~46s boot and above both precedents @@ -503,8 +523,10 @@ verification guide, and a failure there invalidates the compose design. See Assu - **AC-007** *(P1)*: A failed `npm install` causes the CLI to report failure — the branch at `src/index.ts:597` is reachable and correct. - **AC-008** *(P1)*: If scaffolding fails after `moveDockerComposeOneLevelUp()`, the compose - file is restored to the project directory (no orphan in the parent). -- **AC-009**: Feedback is **continuous for the entire wait**, which may be up to ten minutes + file is restored to the project directory (no orphan in the parent). Both moves must also be + **awaited** — they are `async`, and `try/finally` around a floating promise does not guarantee + the rename landed before the clone runs against a directory that must be empty. +- **AC-009** *(P0 — it is what makes AC-001/AC-002 observable)*: Feedback is **continuous for the entire wait**, which may be up to ten minutes (`--wait-timeout 600`). Both required: `docker compose up --wait`'s own per-container `Waiting → Healthy` transitions are streamed rather than swallowed, and a ticker shows elapsed time plus per-service state, refreshed every ~2s. Image-pull progress is visible. Retry messages @@ -525,9 +547,16 @@ verification guide, and a failure there invalidates the compose design. See Assu nothing created; **that was wrong** — it exists, it ships in this package, and it was already written against the bundled asset. What was genuinely wrong was the path: it is under the package's own `scripts/`, not the repository root. -- **AC-013**: The bundled compose file is actually present in the published package. `package.json` - `files` and `project.json`'s esbuild `assets` must both list it, or it ships missing and every - local-Docker run fails at the first step. +- **AC-013**: The bundled compose file is actually present in the **published package**. + `package.json` `files` and `project.json`'s esbuild `assets` must both list it, or it ships + missing and every local-Docker run fails at the first step. + **Verified against the build output, not the source tree.** Asserting the two manifests is + necessary but not sufficient: a wrong `output:` in the esbuild `assets` entry satisfies a + manifest check and still puts the file somewhere the package does not carry. Equally, a Jest + spec or `verify-cold-start.sh --static` that resolves the asset relative to `src/` passes + identically whether or not it ever ships. The gate is therefore a post-build assertion over + `dist/libs/sdk/create-app` — the file exists at the path the CLI resolves at runtime, and + `npm pack --dry-run` lists it in the tarball contents. - **Verification method**: - *Compose:* against **the bundled asset**, @@ -548,6 +577,12 @@ verification guide, and a failure there invalidates the compose design. See Assu port-reuse probe; and the `CUSTOM_STARTER_URL` rewrite run against the real bundled asset (AC-012). Per constitution Principle V these are written and confirmed failing (Red) before the implementation lands. + - *Packaging (AC-013):* a post-build check over `dist/libs/sdk/create-app` asserting the + compose asset exists at the path the CLI resolves at runtime, plus `npm pack --dry-run` + listing it in the tarball contents. This is deliberately **not** a source-tree assertion: + checking `package.json`/`project.json`, or resolving the asset relative to `src/`, passes + identically whether or not the file ever ships — which is the exact failure AC-013 exists to + catch. Cheap enough to gate every PR; no Docker required. - *Manual end-to-end:* the reproduction steps above, run cold on macOS + Docker Desktop, verifying AC-001 through AC-006, plus AC-011 (a `curl` to the host's LAN address on 8090 must be refused).