diff --git a/docs/semantics.md b/docs/semantics.md index b52a35f..075613b 100644 --- a/docs/semantics.md +++ b/docs/semantics.md @@ -116,17 +116,18 @@ 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**. 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. +**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. **Hostname overlay, compose apps:** `--hostname-overlay ` accepts a per-service map value for a compose app's entry instead of the plain-app @@ -267,25 +268,8 @@ 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 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 +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 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/scripts/restore-db.sh b/scripts/restore-db.sh index 36f55a5..98831a9 100755 --- a/scripts/restore-db.sh +++ b/scripts/restore-db.sh @@ -3,10 +3,13 @@ # Streams a Coolify Postgres backup artifact into the target container over # tailnet SSH. Refuses to run without explicit confirmation of the target. set -euo pipefail -ARTIFACT="${1:?backup artifact (.sql.gz)}"; HOST="${2:?target host (tailnet name)}"; CONTAINER="${3:?postgres container name}"; DBNAME="${4:?target database name (docker exec psql -U postgres -lqt to list)}" +ARTIFACT="${1:?backup artifact (.sql.gz)}"; HOST="${2:?target host (tailnet name)}"; CONTAINER="${3:?postgres container name}"; DBNAME="${4:?target database name (docker exec env | grep POSTGRES_DB)}" echo "About to RESTORE ${ARTIFACT} into database ${DBNAME} in ${CONTAINER} on ${HOST} — this overwrites that database." read -r -p "Type the target host to confirm: " CONFIRM [ "$CONFIRM" = "$HOST" ] || { echo "confirmation mismatch; aborting"; exit 1; } +# Connect as the container's own superuser: Coolify provisions each Postgres +# resource with a random POSTGRES_USER — "postgres" does not exist. Expand +# $POSTGRES_USER inside the container (single-quoted sh -c), never client-side. # shellcheck disable=SC2029 # intentional: $CONTAINER/$DBNAME are local vars, expand client-side before they reach the remote shell -gunzip -c "$ARTIFACT" | ssh "root@${HOST}" "docker exec -i ${CONTAINER} psql -U postgres -d ${DBNAME} -v ON_ERROR_STOP=1" +gunzip -c "$ARTIFACT" | ssh "root@${HOST}" "docker exec -i ${CONTAINER} sh -c 'psql -U \"\$POSTGRES_USER\" -d ${DBNAME} -v ON_ERROR_STOP=1'" echo "restore complete — run the verification checks in runbooks/restore-drill.md step 4" diff --git a/src/cli.ts b/src/cli.ts index a752788..b2098cb 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -279,22 +279,12 @@ 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 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. +// 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. export function parseDockerComposeDomains( raw: unknown, ): Record | undefined { @@ -305,31 +295,16 @@ export function parseDockerComposeDomains( } catch { return undefined; } + if (!Array.isArray(parsed)) return undefined; const map: Record = {}; - 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); - } + 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; } - 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; + return map; } export function projectLiveFields( @@ -357,33 +332,14 @@ export function projectLiveFields( ? { docker_compose_location: raw.docker_compose_location } : {}), ...(composeDomains ? { docker_compose_domains: composeDomains } : {}), - // 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 }), + // 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, ...(raw.install_command ? { install_command: raw.install_command } : {}), ...(raw.build_command ? { build_command: raw.build_command } : {}), ...(raw.start_command ? { start_command: raw.start_command } : {}), @@ -577,16 +533,6 @@ 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 9d2e937..b7cd8c1 100644 --- a/src/diff.ts +++ b/src/diff.ts @@ -42,16 +42,6 @@ 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; @@ -255,10 +245,6 @@ 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) { @@ -296,26 +282,6 @@ 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 8bceabc..aad03ba 100644 --- a/test/wire.test.ts +++ b/test/wire.test.ts @@ -5,7 +5,6 @@ import { databaseApiFields, databaseVersionFromImage, defaultDatabaseImage, - parseDockerComposeDomains, projectLiveFields, serviceApiFields, } from "../src/cli.js"; @@ -66,46 +65,6 @@ 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", { @@ -181,31 +140,16 @@ describe("projectLiveFields", () => { expect(out.start_command).toBe("node server.js"); }); - it("reports a readable is_static as a boolean, tolerating Coolify's 1/0", () => { + it("always reports 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, ); - // 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", - ); + // 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); }); }); @@ -248,127 +192,6 @@ 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)", () => {