docs(ci): make the preview notes say what the code does (#1573)

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

Author: FelixTJDietrich

.changeset/preview-notes-say-what-the-code-does.md

@@ -0,0 +1,5 @@
+---
+---
+
+Comment and documentation corrections in the preview deployment tooling; no deployed behaviour
+changes.

.github/workflows/cleanup-preview.yml

@@ -61,7 +61,7 @@ jobs:
           COOLIFY_WEBHOOK_SECRET: ${{ secrets.COOLIFY_PREVIEW_WEBHOOK_SECRET }}
         run: bun scripts/coolify-preview.ts close
 
-      - name: Keep a verified cleanup tombstone
+      - name: Record that teardown was requested
         id: tombstone
         if: steps.close.outcome == 'success'
         uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0

docker/preview/README.md

@@ -51,8 +51,7 @@ Coolify runs every preview of this application under one Compose project named a
 UUID, so a network defined here would be `<uuid>_backend` for all of them at once, private-looking
 and shared. `staging-shared` is joined by name, which makes it a decision rather than a side effect.
 
-The application server runs without Linux capabilities and with `no-new-privileges`. PostgreSQL
-keeps a PR-specific volume.
+PostgreSQL keeps a PR-specific volume; nothing else in the stack keeps state.
 
 Deployment authority is deliberately the same as push authority;
 [ADR 0035](../../docs/decisions/0035-pull-request-previews-are-label-gated.md) records why, and what
@@ -72,10 +71,9 @@ memory limit, a network every preview would share, or a flipped integration swit
 
 ## Host capacity
 
-Each stack is capped at about 2.8 GiB once running (2 GiB application server, 512 MiB PostgreSQL,
-256 MiB webapp); the seed loader adds 512 MiB while it runs and then exits. Its CPU ceilings total
-2.75 across the four services, so several previews oversubscribe a small host on paper. That is intended: a preview is idle almost all the time, and a
-ceiling stops one stack from taking the box rather than reserving capacity for it.
+Memory and CPU ceilings are set per service in `compose.app.yaml`; budget roughly 3 GiB per slot.
+The CPU ceilings deliberately oversubscribe a small host: a preview is idle almost all the time, and
+a ceiling stops one stack taking the box rather than reserving capacity for it.
 
 `PREVIEW_MAX_ACTIVE` caps concurrent previews and defaults to 3. It has to leave roughly 3 GiB of
 memory per slot in whatever staging is not already using — check `free -g` on the host before raising
@@ -168,10 +166,9 @@ draft sends Coolify a signed close event, and the GitHub deployment is then mark
 slot is free. Coolify queues that teardown and answers immediately, so the workflow reports that the
 request was accepted, not that the containers are gone.
 
-Nothing here checks the host. That is a deliberate scope choice, not an oversight: no leaked preview
-stack has been observed, and verifying it would mean a standing SSH credential and a root-owned
-binary installed out of band. If leaks do turn up, the nightly `Preview reconcile` workflow is where
-that check belongs — it already walks every retained preview and re-sends the close event.
+Nothing here checks the host. That is a deliberate scope choice: verifying teardown would mean a
+standing credential on the deployment host. If leaks turn up, the nightly `Preview reconcile`
+workflow is where that check belongs — it already walks every retained preview and re-sends the close
+event.
 
-Watch the host's disk and container list for the first few weeks. Preview images are pulled per
-commit and are never removed by this system.
+Preview images are pulled per commit and are never removed by this system.

docker/preview/compose.app.yaml

@@ -2,22 +2,16 @@
 #
 # Coolify receives the opted-in PR head through its signed manual webhook and re-reads this file from
 # that commit, so a pull request's own copy defines its stack. The controller refuses to deploy a pull
-# request that edits this file, and scripts/check-preview-stack.ts asserts the sandbox properties
-# below on every pull request.
+# request that changes anything under docker/preview/, and scripts/check-preview-stack.ts asserts the
+# sandbox properties below on every pull request.
 #
 # Every service runs the image CI published for this exact commit — the same artifacts staging and
 # production run, buildpack-built application server included. Nothing is built here, so a preview
 # exercises the shipped runtime rather than a second recipe that could pass where the real one fails.
 #
-# This restates the reference stack rather than including it. Compose merges per key, so inheriting
+# This restates the reference stack rather than including it: Compose merges per key, so inheriting
 # would arrive with sync, notifications and the agent digest requirement switched on, and with the
-# Docker socket mounted — ADR 0035 option 6 has the rendered evidence. scripts/check-preview-stack.ts
-# fails when the reference gains a variable this file neither sets nor records as omitted.
-#
-# This file declares no networks. Coolify runs every preview of this application under one Compose
-# project — the project name is the application UUID, with no pull request in it — so a network named
-# here would be `<uuid>_backend`, shared by every preview at once. Coolify instead gives each preview
-# its own network and attaches its proxy to that, which is what keeps the stacks apart.
+# Docker socket mounted. ADR 0035 option 6 has the rendered evidence.
 
 services:
   postgres:
@@ -34,7 +28,7 @@ services:
     cap_drop:
       - ALL
     # The entrypoint chowns its data directory and drops to the postgres user; without these it exits
-    # with "failed switching to 'postgres'". Verified by running the stack, not inferred.
+    # with "failed switching to 'postgres'".
     cap_add:
       - CHOWN
       - DAC_OVERRIDE
@@ -58,20 +52,15 @@ services:
           memory: 512M
           pids: 256
 
-  # Clones the running staging database into this preview so it starts with real data. Reads over
-  # the network with a role that can only read: staging's data volume is never mounted, and no
-  # container here holds the Docker daemon. Every review entry point is paused, and the instance
-  # identity dropped, before the application server is allowed to boot. Runs once — a redeploy finds
-  # the marker table and exits.
-  #
-  # This is the image staging itself runs, so pg_dump always matches the server it reads.
+  # Clones staging into this preview over the network with a role that can only read; staging's data
+  # volume is never mounted. This is the image staging itself runs, so pg_dump always matches the
+  # server it reads.
   seed-loader:
     image: ghcr.io/ls1intum/hephaestus/postgres:${SOURCE_COMMIT:?Coolify supplies the deployed commit}
     restart: "no"
     depends_on:
       postgres:
         condition: service_healthy
-    # Reaches staging's Postgres by name; its own database it reaches on Coolify's preview network.
     networks:
       - staging-shared
     environment:
@@ -188,8 +177,8 @@ services:
         DELETE FROM login_provider;
         SQL
 
-        # The marker promises the policy is in force, so verify it against the database rather than
-        # trusting psql's exit status.
+        # Every statement above succeeds when it matches nothing, so the marker is earned by
+        # counting rather than by psql's exit status.
         echo "Verifying the preview policy took effect"
         LIVE=$$(psql -h "$$TARGET" -U hephaestus -d hephaestus -tAX -v ON_ERROR_STOP=1 -c "
           SELECT (SELECT count(*) FROM workspace
@@ -260,17 +249,15 @@ services:
       DATABASE_USERNAME: hephaestus
       DATABASE_PASSWORD: ${POSTGRES_PASSWORD}
       # The integration consumer reads staging's JetStream, so a preview sees the events a shared
-      # GitHub App delivers there rather than an empty broker. The agent job queue runs on
-      # PostgreSQL, not NATS, and stays local to this stack's own database.
+      # GitHub App delivers there rather than an empty broker.
       NATS_SERVER: ${NATS_SERVER:-nats://nats-server:4222}
       NATS_ENABLED: "true"
       # Per-deploy durable name, so previews get their own JetStream consumers instead of competing
       # for one and stealing each other's messages.
       NATS_DURABLE_CONSUMER_NAME: ${SERVICE_NAME_APPSERVER:-appserver}-consumer
-      # A preview is deleted, not shut down, so it never removes the durables it created on the
-      # shared stream. Far shorter than the 30d a long-lived deployment uses, because a preview is
-      # abandoned the day its pull request merges. Still long enough that a redeploy, or a testing
-      # session paused overnight, resumes on its own cursor instead of skipping to the head.
+      # A preview is deleted, not shut down, so it never removes its durables from the shared
+      # stream. Shorter than the shipped 30d because a preview is abandoned at merge, and long
+      # enough that a redeploy resumes on its own cursor.
       HEPHAESTUS_INTEGRATION_CONSUMER_INACTIVE_THRESHOLD: 72h
       HEPHAESTUS_AUTH_ISSUER: https://${SERVICE_FQDN_WEBAPP}
       HEPHAESTUS_AUTH_STATE_COOKIE_KEY: ${HEPHAESTUS_AUTH_STATE_COOKIE_KEY}
@@ -294,8 +281,8 @@ services:
       SENTRY_DSN: ""
       WEBHOOK_SECRET: ${WEBHOOK_SECRET}
       WEBHOOK_EXTERNAL_URL: ""
-    # Only the application server joins staging's network, and only to reach its broker. The
-    # database and the SPA stay on Coolify's per-preview network.
+    # Joined to reach staging's broker; this stack's own database and SPA stay on Coolify's
+    # per-preview network.
     networks:
       - staging-shared
     depends_on:
@@ -312,8 +299,8 @@ services:
       test: ["CMD", "/workspace/health-check"]
       interval: 15s
       timeout: 5s
-      # A first boot runs Liquibase against an empty database on a shared host, and the webapp waits
-      # on this. Nothing here benefits from failing fast, and failing fast fails the whole deploy.
+      # First boot runs Liquibase over a freshly restored staging dump, on a host shared with
+      # staging.
       retries: 10
       start_period: 300s
     logging:

docs/contributor/ci-cd.mdx

@@ -84,7 +84,7 @@ This is more efficient than a custom workflow and doesn't consume CI minutes.
 
 | Environment       | Eligibility | Deploys on |
 | ----------------- | ----------- | ---------- |
-| Preview (Coolify) | Same-repository PR with the `preview` label | Every push; never waits for CI |
+| Preview (Coolify) | Same-repository PR with the `preview` label | Every push; waits for its images, never for tests |
 | Staging           | `main` | Verified signed release |
 | Production        | Required reviewer | The same verified signed release |
 

docs/decisions/0035-pull-request-previews-are-label-gated.md

@@ -49,8 +49,8 @@ reviewer wants *in order to* review.
    silently deploy it, which surprises the approver.
 3. **A GitHub Environment with required reviewers.** Rejected *for previews*. It is the correct
    primitive and it is what production uses, but it gates every individual deployment behind a click
-   — the same per-commit friction as option 2, with a nicer button. A required-reviewer environment gates every
-   individual deployment behind a click. Deleting such an environment also deletes its secrets and protection rules
+   — the same per-commit friction as option 2, with a nicer button.
+   Deleting such an environment also deletes its secrets and protection rules
    and fails any job waiting on them, and `GITHUB_TOKEN` cannot do it at all — which is why cleanup
    here marks deployments inactive and never deletes environments.
 4. **The `preview` label as opt-in state, push access as the authority.** Chosen — see below.

scripts/check-preview-stack.ts

@@ -18,12 +18,10 @@ const REFERENCE_FILE = "docker/compose.app.yaml";
 const OWN_IMAGE_PREFIX = "ghcr.io/ls1intum/hephaestus/";
 
 /**
- * Reference variables a preview deliberately does not set. Inheriting the reference stack instead of
- * restating it was tried and rejected: Compose merges per key, so the sandbox would arrive with
- * `LEADERBOARD_NOTIFICATION_ENABLED`, `MONITORING_BACKFILL_ENABLED` and an hourly `MONITORING_SYNC_CRON`
- * switched on, and a *new* integration would arrive on too. Restating is the safe direction; this list
- * is what stops it from being a silent one — a variable added to the reference and not considered here
- * fails the build.
+ * Reference variables a preview deliberately does not set. Inheriting the reference instead of
+ * restating it was tried and rejected — Compose merges per key, so integrations arrive switched on
+ * (ADR 0035, option 6). This list is what keeps restating from being a silent omission: a variable
+ * added to the reference and not considered here fails the build.
  */
 const DELIBERATELY_OMITTED = new Set([
 	// Integrations a preview never reaches: no GitLab, Outline, Slack or PostHog credentials exist.
@@ -70,11 +68,6 @@ const DELIBERATELY_OMITTED = new Set([
 	"HEPHAESTUS_AUTH_BOOTSTRAP_TOKEN",
 ]);
 
-/**
- * A preview restates the reference stack rather than including it, so a variable added to the
- * reference reaches previews only if someone puts it there. This turns that from a silent omission
- * into a red check naming the variable.
- */
 export function findEnvDrift(referenceText: string, previewText: string): string[] {
 	const reference = readComposeServices(referenceText).get("application-server");
 	const preview = readComposeServices(previewText).get("appserver");
@@ -113,18 +106,17 @@ const RENDER_ENV: Record<string, string> = {
 	SERVICE_FQDN_APPSERVER: "pr1.api.example.com",
 };
 
-/** Values the server refuses to start without: it validates them before the context is built, so an
- * empty one here is a preview that restart-loops rather than a preview that misbehaves quietly. */
 /**
- * Capabilities a service may add back after `cap_drop: ALL`. Both entries are servers whose root
- * process prepares a directory and then runs its real work as another user; each set is the minimum
- * found by running the image with less. The application server needs none.
+ * Capabilities a service may add back after `cap_drop: ALL`. Each set is the minimum found by running
+ * the image with less; the application server needs none.
  */
 const ALLOWED_CAPABILITIES: Record<string, readonly string[]> = {
 	postgres: ["CHOWN", "DAC_OVERRIDE", "FOWNER", "SETGID", "SETUID"],
 	webapp: ["CHOWN", "SETGID", "SETUID"],
 };
 
+/** Values the server refuses to start without: it validates them before the context is built, so an
+ * empty one here is a preview that restart-loops rather than a preview that misbehaves quietly. */
 const REQUIRED_NON_EMPTY = ["HEPHAESTUS_TRUSTED_PROXIES", "WEBHOOK_SECRET"];
 
 /**
@@ -233,11 +225,8 @@ export function findViolations(stack: unknown): string[] {
 	}
 	// Coolify runs every preview of this application under one Compose project, named after the
 	// application UUID with no pull request in it. A network defined here is therefore `<uuid>_<name>`
-	// for all of them at once, and one preview reaches another's database over it — a shared network
-	// that reads as private. An external one is the opposite: it names a network that already exists,
-	// so joining it is a decision someone made rather than a side effect of the project name.
-	// `default` is Compose's own and carries the project name too, but nothing joins it once no
-	// service names a network.
+	// for all of them at once — a shared network that reads as private. An external one names a
+	// network that already exists, so joining it is a decision rather than a side effect.
 	for (const [name, network] of records(stack.networks)) {
 		if (name === "default") continue;
 		if (network.external === true && typeof network.name === "string" && network.name !== "") {

scripts/coolify-preview.ts

@@ -77,8 +77,8 @@ const DEPLOYMENT_STATES = new Set([
 	"in_progress",
 	"queued",
 ]);
-// Coolify pulls three published images and starts them; the images are already built by the time
-// this runs, so the budget covers pulls and startup rather than a build.
+// The images are already built by the time this runs, so this budget covers Coolify pulling and
+// starting them rather than a build.
 const BUILD_BUDGET_MS = 1_200_000;
 // Coolify reports `finished` once the containers are up; the preview URL only answers once the
 // application server clears its own healthcheck start period. This is additive on purpose: clamping

scripts/preview-controller.test.ts

@@ -3,7 +3,7 @@ import { afterEach, beforeEach, describe, it } from "node:test";
 
 import {
 	assess,
-	CLEANUP_VERIFIED_DESCRIPTION,
+	TEARDOWN_REQUESTED_DESCRIPTION,
 	create,
 	finalize,
 	inactivate,
@@ -280,7 +280,7 @@ void describe("preview host capacity", () => {
 			github: makeGitHub({
 				deployments: occupants(3),
 				statuses: {
-					102: [{ description: CLEANUP_VERIFIED_DESCRIPTION, state: "inactive" }],
+					102: [{ description: TEARDOWN_REQUESTED_DESCRIPTION, state: "inactive" }],
 				},
 			}),
 			context: makeContext(),
@@ -556,7 +556,7 @@ void it("keeps the verified tombstone record when cleanup inactivates a preview"
 
 	await inactivate({ github, context: makeContext(), core: makeCore() });
 
-	assert.deepEqual(descriptions, [CLEANUP_VERIFIED_DESCRIPTION]);
+	assert.deepEqual(descriptions, [TEARDOWN_REQUESTED_DESCRIPTION]);
 	assert.equal(deletions, 0);
 });
 

scripts/preview-controller.ts

@@ -79,7 +79,8 @@ const TRUSTED_ASSOCIATIONS = new Set(["COLLABORATOR", "MEMBER", "OWNER"]);
 const COMPARE_FILE_LIMIT = 300;
 const DEFAULT_MAX_ACTIVE = 3;
 const LIVE_STATES = new Set(["in_progress", "pending", "queued", "success"]);
-const CLEANUP_VERIFIED_DESCRIPTION = "Preview teardown requested; awaiting Coolify reconciliation.";
+const TEARDOWN_REQUESTED_DESCRIPTION =
+	"Preview teardown requested; awaiting Coolify reconciliation.";
 
 const hasPreviewLabel = (pull: PullRequest): boolean =>
 	pull.labels.some((label) => label.name === PREVIEW_LABEL);
@@ -90,9 +91,9 @@ const maxActivePreviews = (): number =>
 		: DEFAULT_MAX_ACTIVE;
 
 /**
- * Environments holding a slot on the shared preview host. A verified cleanup tombstone is retained so
- * the nightly reconcile can repeat Coolify cleanup, but its host resources are gone, so it holds
- * nothing.
+ * Environments still holding a slot. A pull request whose teardown has been requested does not hold
+ * one: the request is the same close event Coolify acts on, and the nightly reconcile re-sends it if
+ * that was missed. Nothing here reads the host, so a slot is freed on the request, not on proof.
  */
 const occupiedEnvironments = async (
 	github: GitHubApi,
@@ -115,10 +116,10 @@ const occupiedEnvironments = async (
 			per_page: 1,
 		});
 		const latest = statuses.data[0]?.state;
-		// A verified tombstone has had its resources removed, and a failed deploy never took the
-		// host — counting either would let a run of failures report a full host that is empty.
+		// A requested teardown has been handed to Coolify and a failed deploy never reached the host,
+		// so neither holds anything — counting them would report a full host that is empty.
 		if (latest === "failure" || latest === "error") continue;
-		if (latest === "inactive" && statuses.data[0]?.description === CLEANUP_VERIFIED_DESCRIPTION) {
+		if (latest === "inactive" && statuses.data[0]?.description === TEARDOWN_REQUESTED_DESCRIPTION) {
 			continue;
 		}
 		occupied.add(deployment.environment);
@@ -389,7 +390,7 @@ async function retireDeployments(
 const inactivate = async ({ github, context }: ControllerInput): Promise<void> => {
 	const { owner, repo } = context.repo;
 	await retireDeployments(github, owner, repo, requiredEnv(process.env, "ENVIRONMENT"), {
-		description: CLEANUP_VERIFIED_DESCRIPTION,
+		description: TEARDOWN_REQUESTED_DESCRIPTION,
 		forceStatus: true,
 		keepRecords: true,
 	});
@@ -420,7 +421,7 @@ const retire = async ({ github, context }: ControllerInput): Promise<void> => {
 };
 
 export {
-	CLEANUP_VERIFIED_DESCRIPTION,
+	TEARDOWN_REQUESTED_DESCRIPTION,
 	PREVIEW_LABEL,
 	assess,
 	create,