diff --git a/docs/semantics.md b/docs/semantics.md index 075613b..b52a35f 100644 --- a/docs/semantics.md +++ b/docs/semantics.md @@ -116,18 +116,17 @@ fails at runtime rather than at apply time. Live-state projection (`projectLiveFields`) reads both fields back for a compose app: `docker_compose_location` (a plain string on the GET model) and `docker_compose_domains`, which the GET model documents as a nullable -**string**, not the structured array the write side accepts — parsed -defensively as JSON back into the internal map, degrading to "field omitted" -(not a crash) on anything that doesn't parse as a well-formed array of -`{name, domain}`. Projecting both is what keeps a matching re-apply a true -no-op instead of a spurious PATCH + stack redeploy every run. **This parsing -path is unverified against a real Coolify instance** — the read-back is only -confirmed by an overlay apply followed by an overlay-edit re-apply against a -live instance, which has not yet happened. If a live instance turns out not to expose -`docker_compose_domains` on read, apply's idempotency guarantee for the -domains half breaks and needs the same warn-and-drop treatment as service -domains below — that also removes the cutover mechanism (domains no longer -flip via re-apply), so it would need a runbook amendment, not a silent fix. +**string**. A live Coolify 4.1.2 probe (cast#68) pinned the ACTUAL decoded +shape of that string: it is a service-**keyed object**, `{ "": +{ "domain": "" }, … }` — **not** the `[{name, domain}]` array +the write side sends and the OpenAPI implies. `parseDockerComposeDomains` +therefore decodes both shapes into the internal `{service: string[]}` map (the +object shape from the read, the array shape from the write-side round-trip and +legacy data), degrading to "field omitted" (not a crash) on anything that is +neither. This was the first of #68's two idempotency breaks: the object bailed +to `undefined`, so cast diffed the desired map against nothing and re-PATCHed + +redeployed the stack on every apply. Projecting both fields — decoded +correctly — is what keeps a matching re-apply a true no-op. **Hostname overlay, compose apps:** `--hostname-overlay ` accepts a per-service map value for a compose app's entry instead of the plain-app @@ -268,8 +267,25 @@ false` to actively guard against a UI flip to `true`, or omit it to leave the field alone (Coolify keeps `pack` and `is_static` independent, which is why this is an explicit field, not inferred from `pack`). The three commands are likewise conditional (an unset command means "let the build pack decide"), diffed only -when declared. `projectLiveFields` reads `is_static` back on every app so it is -there to compare when a manifest does declare it. `draft` emits all four when the +when declared. `projectLiveFields` reads `is_static` back so it is there to +compare when a manifest does declare it — **but only when Coolify actually +returns it, and 4.1.2 never does.** `is_static` is not an `applications` column; +it lives on the `ApplicationSetting` relation (`Application::settings()`), which +Coolify 4.1.2 serializes on **no** read endpoint — the model has no `$with`, and +neither `GET /applications`, the by-uuid GET, nor `@environment_details` +(`ProjectController`) load `settings`. So the field is simply absent from every +read (source-verified + a live probe, cast#68); the UI reads it in-process off +the model (Livewire), never via the API. Projecting `false` from that absence was +#68's second idempotency break: cast diffed `false → true` and re-PATCHed + +redeployed on every apply. +So when the live value is unreadable (`null`/absent), `projectLiveFields` omits +`is_static`, `fetchLive` flags the application `staticNotCompared`, and +`computeDiff` skips the comparison (a once-per-run warn says so) — **`is_static` +degrades to a create-time-only setting.** The create path still sends it +(`applicationApiFields`), so a fresh static app is stood up correctly; what is +lost is drift detection and in-place repair of a later UI flip, which the API +does not permit reading back. If a future Coolify returns a real boolean, it is +projected and diffed normally again. `draft` emits all four when the live box carries them (and `static` only alongside a `publish_directory`, so the draft always loads) — they used to sit in its `NO_HOME` list of settings a rebuild silently dropped, and `is_static` was not even there, which is exactly diff --git a/src/cli.ts b/src/cli.ts index b2098cb..a752788 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -279,12 +279,22 @@ export function databaseVersionFromImage(image: unknown): string | undefined { // Coolify's GET application model exposes `docker_compose_domains` as a // nullable string (reference/coolify-openapi-4.1.2.json ~line 12689), not -// the structured array the create/update request bodies accept (~line 353) — -// the live value is the same array-of-{name,domain} shape, JSON-encoded. -// Parses defensively: anything that isn't a JSON-encoded array of well-formed -// {name, domain} entries collapses to `undefined` rather than throwing, so a -// live instance that turns out not to expose this (unverified until Task 8 -// step 6) degrades to "field omitted", not a crash. +// the structured array the create/update request bodies accept (~line 353). +// The REAL read shape on a live Coolify 4.1.2 (cast#68) is NOT that array +// JSON-encoded — it is a service-KEYED OBJECT, JSON-encoded: +// {"api":{"domain":"https://api…"},"admin":{"domain":"https://…,https://…"}} +// i.e. { "": { "domain": "" }, … }. The +// original assumption (an array of {name,domain}) was wrong; a live probe +// pinned this as one of the two idempotency breaks in #68 — cast diffed the +// desired map against `undefined` forever because the object bailed out. +// Parses BOTH shapes into cast's internal `Record`: +// - object shape (real read): map[service] = domain.split(",") +// - legacy array shape (what applicationApiFields still WRITES, and what +// the vendored OpenAPI implies): map[name] = domain.split(",") +// Keeping the array branch keeps the write-side round-trip and its tests +// working. Anything that is not one of these two well-formed shapes (a JSON +// scalar, a parse error, an empty string) collapses to `undefined` rather +// than throwing — "field omitted", not a crash. export function parseDockerComposeDomains( raw: unknown, ): Record | undefined { @@ -295,16 +305,31 @@ export function parseDockerComposeDomains( } catch { return undefined; } - if (!Array.isArray(parsed)) return undefined; const map: Record = {}; - for (const entry of parsed) { - const name = (entry as { name?: unknown } | null)?.name; - const domain = (entry as { domain?: unknown } | null)?.domain; - if (typeof name === "string" && typeof domain === "string") { - map[name] = domain.split(",").filter(Boolean); + if (Array.isArray(parsed)) { + // Legacy / write-side shape: [{ name, domain }]. + for (const entry of parsed) { + const name = (entry as { name?: unknown } | null)?.name; + const domain = (entry as { domain?: unknown } | null)?.domain; + if (typeof name === "string" && typeof domain === "string") { + map[name] = domain.split(",").filter(Boolean); + } } + return map; } - return map; + if (parsed !== null && typeof parsed === "object") { + // Real read shape: { "": { "domain": "" } }. + for (const [service, value] of Object.entries( + parsed as Record, + )) { + const domain = (value as { domain?: unknown } | null)?.domain; + if (typeof domain === "string") { + map[service] = domain.split(",").filter(Boolean); + } + } + return map; + } + return undefined; } export function projectLiveFields( @@ -332,14 +357,33 @@ export function projectLiveFields( ? { docker_compose_location: raw.docker_compose_location } : {}), ...(composeDomains ? { docker_compose_domains: composeDomains } : {}), - // is_static is read on every application so it is there to compare WHEN a - // manifest declares `static:`. computeDiff compares only fields the DESIRED - // side declares, so an app whose manifest omits `static` never diffs on it - // (which is what keeps this from PATCHing is_static off an un-migrated - // static app), and the three commands are the same — live carrying them - // here is safe and never reads as spurious drift. Coolify's Application - // model casts is_static to boolean; tolerate a 1/0 defensively. - is_static: raw.is_static === true || raw.is_static === 1, + // is_static is read so it is there to compare WHEN a manifest declares + // `static:`. computeDiff compares only fields the DESIRED side declares, + // so an app whose manifest omits `static` never diffs on it (which is + // what keeps this from PATCHing is_static off an un-migrated static app), + // and the three commands are the same. + // + // ABSENT-BY-DESIGN (cast#68): `is_static` is NOT an `applications` column — + // it lives on the `ApplicationSetting` relation (`Application::settings()` + // hasOne). Coolify 4.1.2 never serializes that relation on any read: the + // Application model has no `$with`/`$appends`, neither `GET /applications` + // nor the by-uuid GET `->load('settings')`, and `@environment_details` + // eager-loads `applications` but not `applications.settings` + // (ProjectController v4.1.2). So the key is simply ABSENT — `raw.is_static` + // is undefined — verified against the coolify v4.1.2 source and a live + // probe. Projecting `false` from that made cast diff false→true and redeploy + // on every apply (#68's second idempotency break). So when the live value is + // UNREADABLE (null/undefined), omit is_static: fetchLive flags the app + // `staticNotCompared` and computeDiff skips the comparison (mirroring + // backup's not-compared path), degrading is_static to a CREATE-TIME-ONLY + // setting — the create path still sends it (applicationApiFields), so a + // later change to an EXISTING app's is_static is a UI act cast cannot + // reconcile. Preserves #63's intent as far as the read API allows. If a + // future Coolify DOES serialize a real boolean (true/false, or 1/0), project + // and diff it normally. + ...(raw.is_static == null + ? {} + : { is_static: raw.is_static === true || raw.is_static === 1 }), ...(raw.install_command ? { install_command: raw.install_command } : {}), ...(raw.build_command ? { build_command: raw.build_command } : {}), ...(raw.start_command ? { start_command: raw.start_command } : {}), @@ -533,6 +577,16 @@ export async function fetchLive( ...(kind === "database" && typeof i.internal_db_url === "string" ? { internalDbUrl: i.internal_db_url } : {}), + // is_static lives on the ApplicationSetting relation, which Coolify 4.1.2 + // never serializes on any read endpoint — so it is absent here (cast#68, + // source-verified). Flag the application so computeDiff skips the is_static + // comparison rather than reporting phantom false→true drift and redeploying + // every run. Only applications carry is_static, and only flag when the live + // value is truly absent — a real boolean (a future Coolify) is projected + // into `fields` above and diffed normally. + ...(kind === "application" && i.is_static == null + ? { staticNotCompared: true } + : {}), })); const live = [ ...map("application", env.applications), diff --git a/src/diff.ts b/src/diff.ts index b7cd8c1..9d2e937 100644 --- a/src/diff.ts +++ b/src/diff.ts @@ -42,6 +42,16 @@ export type Live = { // ${resource:.url} derives from (#60). Absent on applications/services, // and on a database whose URL the read could not see. internalDbUrl?: string; + // Coolify 4.1.2 returns `is_static: null` on the read path even for a + // genuinely-static application (cast#68), so the live value is UNREADABLE. + // Presence of this means: DO NOT COMPARE `is_static` for this application. + // Exactly like backupNotCompared, leaving is_static merely absent from + // `fields` is NOT equivalent — the desired side still declares it whenever + // the manifest sets `static:`, so computeDiff would diff true-against- + // undefined and report a phantom PATCH + redeploy every run. is_static stays + // a create-time setting; a real boolean from a future Coolify (staticNotCompared + // unset) is projected and diffed normally. + staticNotCompared?: boolean; }; export type FieldDiff = { field: string; @@ -245,6 +255,10 @@ export function computeDiff( ): DiffReport { const changes: Change[] = []; const backupsNotCompared: { name: string; reason: string }[] = []; + // is_static is unreadable on Coolify 4.1.2's read path (cast#68); warn once + // per run when the degradation actually bites (a manifest declares `static:` + // on an app whose live value cast could not read), not per application. + let staticWarned = false; for (const d of desired) { const l = live.find((x) => x.kind === d.kind && x.name === d.name); if (!l) { @@ -282,6 +296,26 @@ export function computeDiff( const skipBackup = l.backupNotCompared !== undefined; const fieldDiffs: FieldDiff[] = Object.entries(d.fields) .filter(([field]) => !(skipBackup && field === "backup")) + .filter(([field]) => { + // The unreadable-is_static escape hatch (cast#68), sibling to the + // backup one above. is_static lives on the ApplicationSetting relation, + // which Coolify 4.1.2 never serializes on a read (source-verified), so + // the live projection omits it and Live carries staticNotCompared. Skip + // the comparison rather than diffing the desired value against + // `undefined` forever (a phantom redeploy every run); warn once so the + // degradation is on screen. A real boolean (staticNotCompared unset) + // falls through and diffs normally. + if (field === "is_static" && l.staticNotCompared) { + if (!staticWarned) { + console.warn( + "is_static is set at create time but cannot be read back — it lives on Coolify 4.1.2's ApplicationSetting relation, which no read endpoint serializes (cast#68). So it is not diffed, and changing an EXISTING app's static flag is a Coolify UI act cast cannot reconcile. Ensure it is correct at create time.", + ); + staticWarned = true; + } + return false; + } + return true; + }) .filter(([field, value]) => !eq(value, l.fields[field])) .map(([field, value]) => ({ field, diff --git a/test/wire.test.ts b/test/wire.test.ts index aad03ba..8bceabc 100644 --- a/test/wire.test.ts +++ b/test/wire.test.ts @@ -5,6 +5,7 @@ import { databaseApiFields, databaseVersionFromImage, defaultDatabaseImage, + parseDockerComposeDomains, projectLiveFields, serviceApiFields, } from "../src/cli.js"; @@ -65,6 +66,46 @@ describe("serviceApiFields", () => { }); }); +describe("parseDockerComposeDomains", () => { + // cast#68: the REAL read shape on a live Coolify 4.1.2 — a service-keyed + // object, NOT the array the OpenAPI implies. This exact string came off the + // wire. It must decode into cast's internal { service: string[] } map. + it("parses the real service-keyed object shape", () => { + const raw = + '{"api":{"domain":"https://api.heavyduty.builders"},"admin":{"domain":"https://admin.heavyduty.builders"},"intake":{"domain":"https://apply.heavyduty.builders"}}'; + expect(parseDockerComposeDomains(raw)).toEqual({ + api: ["https://api.heavyduty.builders"], + admin: ["https://admin.heavyduty.builders"], + intake: ["https://apply.heavyduty.builders"], + }); + }); + + it("splits a comma-joined domain string into the internal array", () => { + const raw = + '{"api":{"domain":"https://a.example.com,https://b.example.com"}}'; + expect(parseDockerComposeDomains(raw)).toEqual({ + api: ["https://a.example.com", "https://b.example.com"], + }); + }); + + // The write side (applicationApiFields) still emits the array shape, and the + // vendored OpenAPI documents it — keep tolerating it so the round-trip holds. + it("still parses the legacy array-of-{name,domain} shape", () => { + const raw = '[{"name":"api","domain":"https://api.example.com"}]'; + expect(parseDockerComposeDomains(raw)).toEqual({ + api: ["https://api.example.com"], + }); + }); + + it("collapses a malformed string to undefined rather than throwing", () => { + expect(parseDockerComposeDomains("not json{")).toBeUndefined(); + // A JSON scalar is neither shape. + expect(parseDockerComposeDomains("42")).toBeUndefined(); + expect(parseDockerComposeDomains("")).toBeUndefined(); + expect(parseDockerComposeDomains(null)).toBeUndefined(); + }); +}); + describe("projectLiveFields", () => { it("projects a live application onto the Desired vocabulary", () => { const out = projectLiveFields("application", { @@ -140,16 +181,31 @@ describe("projectLiveFields", () => { expect(out.start_command).toBe("node server.js"); }); - it("always reports is_static as a boolean, tolerating Coolify's 1/0", () => { + it("reports a readable is_static as a boolean, tolerating Coolify's 1/0", () => { expect(projectLiveFields("application", { is_static: 1 }).is_static).toBe( true, ); expect(projectLiveFields("application", { is_static: 0 }).is_static).toBe( false, ); - // Absent on the wire reads as false (its real default), never undefined — - // so it compares against the desired side, which always emits it. - expect(projectLiveFields("application", {}).is_static).toBe(false); + // A real `false` is readable and projected as false, so a UI flip off + // `static` still diffs against a manifest that declares `static: true`. + expect( + projectLiveFields("application", { is_static: false }).is_static, + ).toBe(false); + }); + + // cast#68: Coolify 4.1.2 returns is_static: null on the read path even for a + // genuinely-static app. An unreadable value must be OMITTED, not projected as + // `false` — projecting false diffed false→true and redeployed every run. + it("omits is_static when the live value is unreadable (null/absent)", () => { + expect( + projectLiveFields("application", { is_static: null }), + ).not.toHaveProperty("is_static"); + // Absent on the wire is the same unreadable case, not a real `false`. + expect(projectLiveFields("application", {})).not.toHaveProperty( + "is_static", + ); }); }); @@ -192,6 +248,127 @@ describe("compose app idempotency (review finding #2)", () => { const report = computeDiff(desired, live, "structural"); expect(report.clean).toBe(true); }); + + // cast#68: the idempotency guarantee against the REAL live shape. When + // docker_compose_domains comes back as Coolify 4.1.2's service-keyed object + // string, a matching manifest must still produce ZERO field diff — before the + // fix the object bailed to undefined and cast diffed the map against nothing + // on every apply. + it("produces zero field diffs when live docker_compose_domains is the service-keyed object shape", () => { + const desired = [ + { + kind: "application" as const, + name: "core", + fields: { + git_repository: "acme/widget", + git_branch: "main", + build_pack: "dockercompose", + base_directory: "/", + docker_compose_location: "docker-compose.yaml", + docker_compose_domains: { + api: ["https://api.heavyduty.builders"], + admin: ["https://admin.heavyduty.builders"], + }, + }, + }, + ]; + const liveRaw = { + git_repository: "acme/widget", + git_branch: "main", + build_pack: "dockercompose", + base_directory: "/", + docker_compose_location: "docker-compose.yaml", + docker_compose_domains: + '{"api":{"domain":"https://api.heavyduty.builders"},"admin":{"domain":"https://admin.heavyduty.builders"}}', + }; + const live = [ + { + kind: "application" as const, + name: "core", + uuid: "app-uuid", + fields: projectLiveFields("application", liveRaw), + }, + ]; + const report = computeDiff(desired, live, "structural"); + expect(report.clean).toBe(true); + }); +}); + +// cast#68: is_static is unreadable on Coolify 4.1.2's read path (returns null +// even for a static app). A manifest that declares `static: true` must NOT diff +// against that null forever — the comparison is skipped and a once-per-run warn +// is emitted. A real live boolean still diffs normally. +describe("is_static live-unreadable degradation (cast#68)", () => { + const baseFields = { + git_repository: "acme/site", + git_branch: "main", + build_pack: "static", + base_directory: "/", + }; + + it("does not diff is_static when the live value is unreadable, and warns once", () => { + const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); + const desired = [ + { + kind: "application" as const, + name: "landing", + fields: { ...baseFields, is_static: true }, + }, + ]; + // projectLiveFields omits is_static from a null live read; fetchLive flags + // the resource staticNotCompared (mirrored here). + const liveFields = projectLiveFields("application", { + ...baseFields, + is_static: null, + }); + expect(liveFields).not.toHaveProperty("is_static"); + const live = [ + { + kind: "application" as const, + name: "landing", + uuid: "app-uuid", + fields: liveFields, + staticNotCompared: true, + }, + ]; + const report = computeDiff(desired, live, "structural"); + const change = report.changes.find((c) => c.name === "landing"); + expect(change?.fieldDiffs.some((f) => f.field === "is_static")).toBeFalsy(); + expect(warn).toHaveBeenCalledTimes(1); + warn.mockRestore(); + }); + + it("still diffs is_static when the live value is a real boolean", () => { + const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); + const desired = [ + { + kind: "application" as const, + name: "landing", + fields: { ...baseFields, is_static: true }, + }, + ]; + // A UI flip off static: the live value is a genuine `false`, readable and + // staticNotCompared unset — cast must catch the drift, not suppress it. + const live = [ + { + kind: "application" as const, + name: "landing", + uuid: "app-uuid", + fields: projectLiveFields("application", { + ...baseFields, + is_static: false, + }), + }, + ]; + const report = computeDiff(desired, live, "structural"); + const change = report.changes.find((c) => c.name === "landing"); + const staticDiff = change?.fieldDiffs.find((f) => f.field === "is_static"); + expect(staticDiff).toBeDefined(); + expect(staticDiff?.desired).toBe(true); + expect(staticDiff?.live).toBe(false); + expect(warn).not.toHaveBeenCalled(); + warn.mockRestore(); + }); }); describe("buildExecutor createResource (application, dockercompose)", () => {