Merge pull request #53 from claude-hdb/fix/create-order

fix(apply): create databases and services before the applications that need them (#45)

docs/semantics.md

@@ -188,6 +188,30 @@ softened by an implementation detail):
   (`instant_deploy` on create, `/deploy` or `/restart` on update) — API
   mutations land in Coolify's DB, not in running containers, so a create or
   update that skipped this would silently not take effect.
+- **Apply acts in dependency order, not manifest order** — `database` →
+  `service` → `application` (#45), for creates *and* updates: a redeploy is a
+  redeploy, and an application restarted against a database whose own pending
+  change has not landed is the same failure one apply later. The order cannot be
+  computed — nothing in a manifest declares that `core` needs `postgres`, no
+  resource names another, so there is no graph to walk — so it is a fixed
+  kind-order (`KIND_ORDER` in `apply.ts`). The direction between the three kinds
+  is not in question, and three is few enough to legislate. `cast destroy` tears
+  down in its exact reverse: things come up in the order their dependencies
+  allow and go down in the reverse. Only the *acting* order changes; the diff
+  report still reads in manifest order (a resource is read where its author
+  wrote it), and nothing about clean/orphans/placement moves with it.
+
+  What that buys, and what it does not: an application is no longer created and
+  deployed against databases that **do not exist**, which made a first apply's
+  deploy fail by construction — a full build, a red deployment, and an operator
+  told to ignore it. It is **not** a readiness barrier.
+  `DeployController@deploy_resource` (v4.1.2) *queues*:
+  `queue_application_deployment(...)` for an application and
+  `StartDatabase::dispatch($resource)` for a database (only a service starts
+  synchronously, `StartService::run`). So cast orders its **requests**, and
+  Coolify runs them on its own queues. An app whose first boot must find a
+  *listening* database still races it; apply guarantees the database exists and
+  was asked to start first, not that it is up.
 - Direction is one-way, manifest → Coolify, always.
 - **Environment guards:** `apply` refuses if a var matching that environment's
   `forbidden_var_patterns` is present in the resolved env **at all, regardless

src/apply.ts

@@ -12,6 +12,35 @@ export type Executor = {
   redeploy(uuid: string, kind: ResourceKind): Promise<void>;
 };
 
+// The order apply acts in, by kind.
+//
+// It is a FIXED order and not a computed graph because there is no graph to
+// compute: nothing in a manifest declares that `core` needs `postgres` — no
+// resource names another, anywhere — so the dependency edges do not exist to be
+// walked. What does exist is the direction between kinds, and it is not in
+// question: applications talk to databases and services, never the reverse.
+// Three kinds is few enough to legislate.
+//
+// Ranked as a Record<ResourceKind, number> on purpose: a fourth ResourceKind
+// does not COMPILE until someone decides where it goes. A list + `indexOf`
+// would rank an unranked kind -1 — i.e. ahead of databases — which is exactly
+// the bug this ordering exists to fix (#45), reintroduced silently for the new
+// kind.
+const KIND_RANK: Record<ResourceKind, number> = {
+  database: 0,
+  service: 1,
+  application: 2,
+};
+
+// The forward order, spelled out: databases → services → applications. Derived
+// from the ranks rather than written twice, so the two can never drift apart.
+// `cast destroy` (#43) tears down in its exact reverse — things come up in the
+// order their dependencies allow and go down in the reverse — and a follow-up
+// unifies the two constants in one place.
+export const KIND_ORDER: readonly ResourceKind[] = (
+  Object.keys(KIND_RANK) as ResourceKind[]
+).sort((a, b) => KIND_RANK[a] - KIND_RANK[b]);
+
 export function applyHostnameOverlay(
   desired: Desired[],
   overlay: Record<string, string[] | Record<string, string[]>>,
@@ -66,6 +95,11 @@ export async function applyPlan(
       "apply requires a full diff (session token with read:sensitive) — refusing on a structural report",
     );
   }
+  // Every change is checked before the first one is acted on — that is the
+  // guarantee ("fails loudly, before any mutation"), and it is why this is a
+  // separate full scan and not a check folded into the ordered walk below. A
+  // fold would let the databases (which now sort first) be created before the
+  // application whose un-updatable drift refuses the run.
   for (const c of report.changes) {
     const blocked = c.fieldDiffs.filter(
       (f) => !f.updatable && c.op === "update",
@@ -76,8 +110,31 @@ export async function applyPlan(
       );
     }
   }
+  // Act in dependency order, not manifest order (#45).
+  //
+  // `desiredFromManifest` emits applications first and `computeDiff` preserves
+  // that, so a walk in report order creates the compose app — and deploys it,
+  // three lines down — before the Postgres and Redis it talks to exist at all.
+  // A guaranteed-red first deploy, every time.
+  //
+  // Creates AND updates, not just creates: a redeploy is a redeploy. An
+  // application restarted against a database whose own pending change has not
+  // been applied yet is the same failure, one apply later.
+  //
+  // A COPY, never a sort in place: `report.changes` is what `renderDiff` prints
+  // and what a fleet run reports on, and that reading order is the manifest's,
+  // deliberately — a resource is read where its author wrote it. Only the acting
+  // order changes here. Nothing about WHAT apply does (clean, orphans,
+  // placement, the refusals above) moves with it.
+  //
+  // Stable (ES2019 guarantees it), so within a kind the manifest's order
+  // survives. Within-kind order carries no meaning, but a run that reshuffles
+  // its own resources every time is noise in an operator's terminal.
+  const ordered = [...report.changes].sort(
+    (a, b) => KIND_RANK[a.kind] - KIND_RANK[b.kind],
+  );
   const mutated: string[] = [];
-  for (const c of report.changes) {
+  for (const c of ordered) {
     const spec = desired.find((d) => d.kind === c.kind && d.name === c.name);
     let uuid: string;
     let didMutate = c.op === "create";

test/apply.test.ts

@@ -1,10 +1,11 @@
 import { describe, expect, it } from "vitest";
 import {
   type Executor,
+  KIND_ORDER,
   applyHostnameOverlay,
   applyPlan,
 } from "../src/apply.js";
-import { type Desired, computeDiff } from "../src/diff.js";
+import { type Desired, type Live, computeDiff } from "../src/diff.js";
 
 const desired: Desired[] = [
   {
@@ -15,12 +16,12 @@ const desired: Desired[] = [
   },
 ];
 
-function recorder() {
+function recorder(uuidFor: (name: string) => string = () => "new-uuid") {
   const calls: string[] = [];
   const exec: Executor = {
     createResource: async (c) => {
       calls.push(`create ${c.name}`);
-      return "new-uuid";
+      return uuidFor(c.name);
     },
     updateFields: async (uuid, _k, fields) => {
       calls.push(`update ${uuid} ${Object.keys(fields).join(",")}`);
@@ -124,6 +125,138 @@ describe("applyPlan", () => {
   });
 });
 
+// The order resolve.ts actually emits (`desiredFromManifest`: applications,
+// then databases, then services) and `computeDiff` faithfully preserves. This
+// is the input that used to build and deploy `core` against nothing.
+const manifestOrder: Desired[] = [
+  {
+    kind: "application",
+    name: "core",
+    fields: { build_pack: "dockercompose" },
+    env: { vars: { DATABASE_URL: { value: "postgres://x", secret: true } } },
+  },
+  { kind: "database", name: "postgres", fields: { type: "postgresql" } },
+  { kind: "database", name: "redis", fields: { type: "redis" } },
+  { kind: "service", name: "metabase", fields: { type: "metabase" } },
+];
+const named = (name: string) => `${name}-uuid`;
+
+describe("applyPlan ordering (#45)", () => {
+  it("creates databases, then services, then applications — never the manifest's order", async () => {
+    const { calls, exec } = recorder(named);
+    const report = computeDiff(manifestOrder, [], "full");
+    // The report itself reads in manifest order: application first.
+    expect(report.changes.map((c) => c.name)).toEqual([
+      "core",
+      "postgres",
+      "redis",
+      "metabase",
+    ]);
+    const r = await applyPlan(report, manifestOrder, exec);
+    // …and apply ACTS in dependency order. `core` is created and deployed last,
+    // by which point both databases and the service exist. Within a kind the
+    // manifest's order survives (postgres before redis) — the sort is stable.
+    expect(calls).toEqual([
+      "create postgres",
+      "redeploy postgres-uuid",
+      "create redis",
+      "redeploy redis-uuid",
+      "create metabase",
+      "redeploy metabase-uuid",
+      "create core",
+      "env core-uuid",
+      "redeploy core-uuid",
+    ]);
+    expect(r.mutated).toEqual(["postgres", "redis", "metabase", "core"]);
+  });
+
+  it("orders updates too, not only creates", async () => {
+    // The apply that adds a Redis and points an existing app at it: the
+    // database must be created and started before the app redeploys onto it.
+    const { calls, exec } = recorder(named);
+    const withRedis: Desired[] = [
+      {
+        kind: "application",
+        name: "core",
+        fields: { build_pack: "dockercompose" },
+        env: {
+          vars: { REDIS_URL: { value: "redis://redis:6379", secret: false } },
+        },
+      },
+      { kind: "database", name: "redis", fields: { type: "redis" } },
+    ];
+    const live: Live[] = [
+      {
+        kind: "application",
+        name: "core",
+        uuid: "u-core",
+        fields: { build_pack: "dockercompose" },
+        env: {},
+      },
+    ];
+    const r = await applyPlan(
+      computeDiff(withRedis, live, "full"),
+      withRedis,
+      exec,
+    );
+    expect(calls).toEqual([
+      "create redis",
+      "redeploy redis-uuid",
+      "env u-core",
+      "redeploy u-core",
+    ]);
+    expect(r.mutated).toEqual(["redis", "core"]);
+  });
+
+  it("still refuses non-updatable drift before ANY mutation, even one that now sorts first", async () => {
+    // The regression the reorder could have introduced: the database sorts
+    // ahead of the application, so a check folded into the ordered walk would
+    // create postgres and only then refuse. The refusal is a full scan first.
+    const { calls, exec } = recorder(named);
+    const live: Live[] = [
+      {
+        kind: "application",
+        name: "core",
+        uuid: "u-core",
+        fields: { build_pack: "nixpacks" }, // NON_UPDATABLE drift
+        env: { DATABASE_URL: "postgres://x" },
+      },
+    ];
+    await expect(
+      applyPlan(computeDiff(manifestOrder, live, "full"), manifestOrder, exec),
+    ).rejects.toThrow(/build_pack/);
+    expect(calls).toEqual([]);
+  });
+
+  it("does not reorder the report itself — the diff reads in manifest order", async () => {
+    // renderDiff and the fleet summary read `report.changes`; sorting it in
+    // place would silently reshuffle what the operator sees.
+    const { exec } = recorder(named);
+    const report = computeDiff(manifestOrder, [], "full");
+    await applyPlan(report, manifestOrder, exec);
+    expect(report.changes.map((c) => c.name)).toEqual([
+      "core",
+      "postgres",
+      "redis",
+      "metabase",
+    ]);
+    // and the rest of the report is untouched by ordering
+    expect(report.clean).toBe(false);
+    expect(report.orphans).toEqual([]);
+  });
+
+  it("exports the forward kind-order, whose reverse is the teardown order", () => {
+    expect(KIND_ORDER).toEqual(["database", "service", "application"]);
+    // `cast destroy` (#43) is the exact reverse — up in dependency order, down
+    // in reverse. It defines its own constant today; a follow-up unifies them.
+    expect([...KIND_ORDER].reverse()).toEqual([
+      "application",
+      "service",
+      "database",
+    ]);
+  });
+});
+
 describe("applyHostnameOverlay", () => {
   it("replaces only domains of named apps", () => {
     const out = applyHostnameOverlay(desired, {