feat: diff and apply a database's backup schedule (#51) #61

Merged

dan-claude-bot merged 1 commit from feat/backup-schedule-diff into main

2026-07-14 23:12:24 +00:00

dan-claude-bot commented

2026-07-14 22:37:56 +00:00

(Migrated from github.com)

The false premise

docs/semantics.md filed backup schedules under "Known limitations, not defects", on the claim that a manifest database's backup block is kept out of the diff because "live Coolify state doesn't expose it back". The parenthesis was load-bearing and false.

A schedule is not on the database's own GET — but it was never meant to be. It has its own route, GET /databases/{uuid}/backups, and cast had been POSTing to it all along (create branch) without ever reading it. "Coolify doesn't expose it back" meant "it isn't on the resource I happened to read."

The cost was live, not hypothetical:

A database created before its backup: block was declared never got one — apply set the schedule only inside the create branch, so adding backup: to an existing Postgres was a clean run and zero backups.
A schedule deleted or changed in the UI was invisible to diff.
The --full diff that gates a production cutover passed on an unbacked-up production database.

What the source actually says (citations)

I could not probe a live Coolify (no credentials; cast is operator-run against real boxes). The vendored OpenAPI documents the GET body as "Content is very complex. Will be implemented later.", so I read the v4.1.2 source instead:

app/Http/Controllers/Api/DatabasesController.php — database_backup_details_uuid() ends:
```
$backupConfig = ScheduledDatabaseBackup::ownedByCurrentTeamAPI($teamId)
    ->with('executions')->where('database_id', $database->id)->get();
return response()->json($backupConfig);
```
A raw Eloquent collection → a JSON array of ScheduledDatabaseBackup rows (no API resource, no removeSensitiveData), plus an eager-loaded executions array cast ignores. A database with no schedule returns 200 [], never a 404.
app/Models/ScheduledDatabaseBackup.php — $fillable gives the field names: uuid, enabled, save_s3, frequency, database_backup_retention_amount_locally (the retention field cast already POSTs on create), … casts() covers only the two float storage fields, so enabled (tinyint) can serialize as 1/0.
frequency round-trips verbatim: the POST/PATCH handlers validate_cron_expression($request->frequency) (returns a bool) and then store $request->only($backupConfigFields) unchanged — no mutator on the model. "0 3 * * *" reads back as "0 3 * * *"; the preset words (daily, …) read back as themselves. This is what makes the field diffable, and it is exactly what the "spurious drift" fear assumed away without checking.

Fields settled on: frequency (string, verbatim) and retention (= database_backup_retention_amount_locally, integer), plus enabled carried so a disabled schedule cannot read as backed-up.

Branch taken: 2 + 3 (real read), not the branch-4 fallback

The source proves the GET is readable, so this reads it into the live side and diffs frequency + retention like any other field, and makes apply set the schedule on update as well as create (POST or PATCH, decided by a read). Branch 4 ("declared, NOT compared") is kept, but only as the honest degradation for a read that fails — see below.

What changed

resolve.ts — backup is now a normal entry in a database's fields, not a side channel. desiredFromManifest no longer returns backupSchedules.
coolify.ts — parseBackupSchedules (a pure parser) + client.databaseBackups(uuid). Every unreadable answer collapses to undefined ("cannot say"); only a genuine [] means "no schedule". A failed read (including a 404 — which here means database not found, never "no schedules") returns undefined, never "none".
cli.ts — fetchLive(..., { backups: true }) attaches the live schedule for diff/apply (opt-in, so the read-side sweeps don't pay for it); attachBackup maps the four answers. The executor sets the schedule on update via reconcileBackupSchedule (POST if none, PATCH if one), and databaseApiFields strips backup so it never reaches the database's own body.
diff.ts — backup diffs like any field; a backupNotCompared carrier removes it from the comparison entirely (never diffed against undefined) and prints a line.
docs/semantics.md — new Backup schedules section; the :606 "Known limitations" entry struck through and corrected; the :541 disaster-recovery line's reason corrected (the draft path still doesn't read them — noted honestly, see below).

Degrades honestly (mandatory, since no live probe): an unreadable or unrecognized response can only ever print backup schedule for database <name> declared, NOT compared — verify in the Coolify UI — never invented drift, never a clean bill on an unread database. On the write side the same failure raises rather than guessing: POSTing blind would duplicate a schedule that may already exist; skipping is the silent no-op being fixed. >1 schedule on one database is also "NOT compared" (a manifest declares one). Disabled schedules diff as drift and are re-enabled.

OPERATOR ACT — confirm the shape against your own box

cast's read is built to the v4.1.2 source above. Before trusting a backup diff on a live box, confirm that box agrees:

curl -s -H "Authorization: Bearer $COOLIFY_TOKEN" \
  "$COOLIFY_BASE_URL/api/v1/databases/<db-uuid>/backups" | jq .

Expected: a JSON array; each element carries "frequency" (string) and "database_backup_retention_amount_locally" (integer), and "enabled".

If the output disagrees:

Not an array, or the field names differ — cast will read it as unrecognized and print declared, NOT compared on every run (it will not invent drift or a false clean). Open a follow-up with the real shape; the parser (parseBackupSchedules in src/coolify.ts) is the one place to adjust.
enabled is a string like "1"/"0" — already handled.
An empty database returns 404 rather than 200 [] — cast reads any failed GET as "unreadable, NOT compared", so it stays safe; but tell us, because it means the "no schedule → drift" path can't fire and a missing backup would go unreported.

Tests added (+42; 294 → 336, all green)

coolify.test.ts — parseBackupSchedules across every unreadable shape (non-array, the spec's literal placeholder string, bad/partial rows, mixed list), the enabled 1/0/absent cases, and the dangerous-404 client path.
diff.test.ts — clean / drift / no-schedule / disabled, and the two that must not lie: unreadable invents no drift and is not silent.
live-lookup.test.ts — attachBackup's four answers and desired-side key order.
wire.test.ts — the executor: create-on-update (the fix), PATCH-not-duplicate, no empty PATCH on the database, refuse-on-failed-read, refuse-on-multiple, leave-undeclared-alone, still-POST-on-create.
backup-cli.test.ts (new) — end to end through dist/cli.js: the no-schedule case that used to exit 0 now exits 1, and the failed read prints NOT compared while staying clean.

Not done, and why

inventory --emit-draft still does not read backups — draft.ts is outside this issue's file list and a sibling PR touches that area. The DR/UNCAPTURED text is corrected to say the reason is "the draft path hasn't been taught to read them yet", not "they can't be read". Worth a follow-up so a rebuilt-from-draft box carries its schedule.
The --full requirement for reading backups is unchanged. Backups read on diff/apply; a structural diff still can't see them. In scope would be creeping; noted here instead.

Heads up for the merge: this is scheduled last of the parallel set and touches diff.ts / cli.ts, so expect to rebase. The backup-cli.test.ts file is new (no conflict); the diff.ts changes are additive (a new report field + a render block).

Closes #51.

## The false premise `docs/semantics.md` filed backup schedules under **"Known limitations, not defects"**, on the claim that a manifest database's `backup` block is kept out of the diff because *"live Coolify state doesn't expose it back"*. The parenthesis was load-bearing and false. A schedule is not on the database's own `GET` — but it was never meant to be. It has its **own route**, `GET /databases/{uuid}/backups`, and cast had been **POSTing to it all along** (create branch) without ever reading it. "Coolify doesn't expose it back" meant "it isn't on the resource I happened to read." The cost was live, not hypothetical: - A database created before its `backup:` block was declared **never got one** — `apply` set the schedule only inside the create branch, so adding `backup:` to an existing Postgres was a clean run and zero backups. - A schedule deleted or changed in the UI was **invisible** to `diff`. - The `--full` diff that gates a production cutover **passed on an unbacked-up production database**. ## What the source actually says (citations) I could not probe a live Coolify (no credentials; cast is operator-run against real boxes). The vendored OpenAPI documents the GET body as *"Content is very complex. Will be implemented later."*, so I read the v4.1.2 source instead: - **`app/Http/Controllers/Api/DatabasesController.php`** — `database_backup_details_uuid()` ends: ```php $backupConfig = ScheduledDatabaseBackup::ownedByCurrentTeamAPI($teamId) ->with('executions')->where('database_id', $database->id)->get(); return response()->json($backupConfig); ``` A raw Eloquent collection → a **JSON array of `ScheduledDatabaseBackup` rows** (no API resource, no `removeSensitiveData`), plus an eager-loaded `executions` array cast ignores. A database with no schedule returns `200 []`, never a 404. - **`app/Models/ScheduledDatabaseBackup.php`** — `$fillable` gives the field names: `uuid`, `enabled`, `save_s3`, `frequency`, **`database_backup_retention_amount_locally`** (the retention field cast already POSTs on create), … `casts()` covers only the two float storage fields, so `enabled` (tinyint) can serialize as `1`/`0`. - **`frequency` round-trips verbatim**: the POST/PATCH handlers `validate_cron_expression($request->frequency)` (returns a bool) and then store `$request->only($backupConfigFields)` unchanged — no mutator on the model. `"0 3 * * *"` reads back as `"0 3 * * *"`; the preset words (`daily`, …) read back as themselves. This is what makes the field diffable, and it is exactly what the "spurious drift" fear assumed away without checking. **Fields settled on:** `frequency` (string, verbatim) and `retention` (= `database_backup_retention_amount_locally`, integer), plus `enabled` carried so a disabled schedule cannot read as backed-up. ## Branch taken: **2 + 3 (real read)**, not the branch-4 fallback The source proves the GET is readable, so this reads it into the live side and diffs `frequency` + `retention` like any other field, and makes `apply` set the schedule on **update** as well as create (`POST` or `PATCH`, decided by a read). Branch 4 ("declared, NOT compared") is kept, but only as the **honest degradation** for a read that fails — see below. ## What changed - **`resolve.ts`** — `backup` is now a normal entry in a database's `fields`, not a side channel. `desiredFromManifest` no longer returns `backupSchedules`. - **`coolify.ts`** — `parseBackupSchedules` (a pure parser) + `client.databaseBackups(uuid)`. Every unreadable answer collapses to `undefined` ("cannot say"); only a genuine `[]` means "no schedule". A failed read (including a 404 — which here means *database* not found, never "no schedules") returns `undefined`, never "none". - **`cli.ts`** — `fetchLive(..., { backups: true })` attaches the live schedule for `diff`/`apply` (opt-in, so the read-side sweeps don't pay for it); `attachBackup` maps the four answers. The executor sets the schedule on update via `reconcileBackupSchedule` (POST if none, PATCH if one), and `databaseApiFields` strips `backup` so it never reaches the database's own body. - **`diff.ts`** — `backup` diffs like any field; a `backupNotCompared` carrier removes it from the comparison entirely (never diffed against `undefined`) and prints a line. - **`docs/semantics.md`** — new **Backup schedules** section; the `:606` "Known limitations" entry struck through and corrected; the `:541` disaster-recovery line's *reason* corrected (the draft path still doesn't read them — noted honestly, see below). **Degrades honestly (mandatory, since no live probe):** an unreadable or unrecognized response can only ever print `backup schedule for database <name> declared, NOT compared — verify in the Coolify UI` — never invented drift, never a clean bill on an unread database. On the write side the same failure **raises** rather than guessing: POSTing blind would duplicate a schedule that may already exist; skipping is the silent no-op being fixed. `>1` schedule on one database is also "NOT compared" (a manifest declares one). Disabled schedules diff as drift and are re-enabled. ## OPERATOR ACT — confirm the shape against your own box cast's read is built to the v4.1.2 source above. Before trusting a backup diff on a live box, confirm that box agrees: ```bash curl -s -H "Authorization: Bearer $COOLIFY_TOKEN" \ "$COOLIFY_BASE_URL/api/v1/databases/<db-uuid>/backups" | jq . ``` Expected: a JSON **array**; each element carries `"frequency"` (string) and `"database_backup_retention_amount_locally"` (integer), and `"enabled"`. If the output disagrees: - **Not an array, or the field names differ** — cast will read it as unrecognized and print `declared, NOT compared` on every run (it will not invent drift or a false clean). Open a follow-up with the real shape; the parser (`parseBackupSchedules` in `src/coolify.ts`) is the one place to adjust. - **`enabled` is a string like `"1"`/`"0"`** — already handled. - **An empty database returns 404 rather than `200 []`** — cast reads any failed GET as "unreadable, NOT compared", so it stays safe; but tell us, because it means the "no schedule → drift" path can't fire and a missing backup would go unreported. ## Tests added (+42; 294 → 336, all green) - `coolify.test.ts` — `parseBackupSchedules` across every unreadable shape (non-array, the spec's literal placeholder string, bad/partial rows, mixed list), the `enabled` 1/0/absent cases, and the dangerous-404 client path. - `diff.test.ts` — clean / drift / no-schedule / disabled, and the two that must not lie: unreadable invents no drift **and** is not silent. - `live-lookup.test.ts` — `attachBackup`'s four answers and desired-side key order. - `wire.test.ts` — the executor: create-on-update (the fix), PATCH-not-duplicate, no empty PATCH on the database, refuse-on-failed-read, refuse-on-multiple, leave-undeclared-alone, still-POST-on-create. - `backup-cli.test.ts` (new) — end to end through `dist/cli.js`: the no-schedule case that **used to exit 0 now exits 1**, and the failed read prints `NOT compared` while staying clean. ## Not done, and why - **`inventory --emit-draft` still does not read backups** — `draft.ts` is outside this issue's file list and a sibling PR touches that area. The DR/UNCAPTURED text is corrected to say the reason is "the draft path hasn't been taught to read them yet", not "they can't be read". Worth a follow-up so a rebuilt-from-draft box carries its schedule. - **The `--full` requirement for reading backups is unchanged.** Backups read on `diff`/`apply`; a structural diff still can't see them. In scope would be creeping; noted here instead. Heads up for the merge: this is scheduled last of the parallel set and touches `diff.ts` / `cli.ts`, so expect to rebase. The `backup-cli.test.ts` file is new (no conflict); the `diff.ts` changes are additive (a new report field + a render block). Closes #51.

No reviewers

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: heavy-duty/cast#61

No description provided.