From 690c76f77b4ff60d7aae58da79321ad88073a8f3 Mon Sep 17 00:00:00 2001 From: caballeto Date: Wed, 22 Apr 2026 19:23:51 +0200 Subject: [PATCH] feat(state): treat state.json as identity-only; live-API attribute reads at plan time (RFC 0001) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Issue: `devhelm plan` reported phantom diffs and missed real drift on status-page children because the differ compared YAML against stale attributes stored in `state.json` instead of the live API. Most recently surfaced as `defaultOpen` flapping between mini + prod. Architecture (RFC 0001): - state.json bumped to v3; child entries store `apiId` only. v1→v3 and v2→v3 reader-side migrations strip `attributes` silently on load — no user-facing migration step. - Differ now async; `plan`/`deploy` call a new `prefetchChildSnapshots(config, refs, client)` that fetches all status-page groups + components from the API up-front. Tests inject the same `ChildSnapshotMap` instead of mocking HTTP. - `statusPageHandler` declares `fetchChildSnapshots` and a `hasChildChanges` that diffs YAML against the live snapshot, not state. Subsumes the Issue A fix: `pullStatusPageChildren` no longer needs to mirror every server-side default. - `applier`, `child-reconciler`, `state pull`, and `import` no longer write `attributes`. `upsertStateEntry` lost the parameter. - `buildStateV2` kept as a deprecated alias of `buildState` that silently drops attributes — preserves the public name until the next major. Tests: 870/870 green. Highlights: - `state.test.ts` rewritten end-to-end for v3 + migration coverage. - `differ.test.ts` "FIXED:" cases now exercise the live-snapshot path via injected `ChildSnapshotMap`. - `partial-failure-convergence.test.ts` (j) updated to assert identity-only child entries; convergence still holds because the next plan re-reads the API. Docs: `docs/rfcs/0001-state-as-identity-only.md` marked Accepted with a shipped-in-0.5.0 implementation note. Made-with: Cursor --- docs/rfcs/0001-state-as-identity-only.md | 361 ++++++++++++++ docs/rfcs/README.md | 20 + src/commands/deploy/index.ts | 6 +- src/commands/import.ts | 6 +- src/commands/plan.ts | 6 +- src/commands/state/pull.ts | 24 +- src/lib/yaml/applier.ts | 5 - src/lib/yaml/child-reconciler.ts | 25 +- src/lib/yaml/differ.ts | 110 ++++- src/lib/yaml/handlers.ts | 280 +++++++---- src/lib/yaml/index.ts | 8 +- src/lib/yaml/state.ts | 197 +++++--- test/yaml/differ.test.ts | 440 ++++++++---------- test/yaml/handlers.test.ts | 112 ++++- test/yaml/idempotency.test.ts | 60 +-- test/yaml/moved.test.ts | 25 +- test/yaml/partial-failure-convergence.test.ts | 16 +- test/yaml/state.test.ts | 169 ++++--- 18 files changed, 1305 insertions(+), 565 deletions(-) create mode 100644 docs/rfcs/0001-state-as-identity-only.md create mode 100644 docs/rfcs/README.md diff --git a/docs/rfcs/0001-state-as-identity-only.md b/docs/rfcs/0001-state-as-identity-only.md new file mode 100644 index 0000000..6041ca5 --- /dev/null +++ b/docs/rfcs/0001-state-as-identity-only.md @@ -0,0 +1,361 @@ +# RFC 0001: state.json as identity-only; live-API attribute reads at plan time + +| Field | Value | +| --- | --- | +| Status | Accepted (implemented in 0.5.0) | +| Author | DevHelm CLI maintainers | +| Created | 2026-04-22 | +| Accepted | 2026-04-22 | +| Target release | `devhelm` 0.5.0 | +| Related | Issue A (`pullStatusPageChildren` underfills) — subsumed by this RFC | + +## Implementation status + +Shipped in `cli@0.5.0`: + +- `state.json` schema bumped to v3 (identity-only). v1 → v3 and v2 → v3 + reader-side migrations strip `attributes` silently on load; users see + no breakage and no migration command is required. +- `differ.diff()` is now async and accepts a `ChildSnapshotMap`. A new + `prefetchChildSnapshots(config, refs, client)` runs all per-parent + child fetches concurrently before the diff loop. `plan` and `deploy` + call it; tests inject a fake map. +- `statusPageHandler` declares `fetchChildSnapshots` (live API read of + groups + components) and a `hasChildChanges` that compares YAML + against the live snapshot, not state. +- `applier`, `child-reconciler`, `state pull`, and `import` no longer + write `attributes` to state. `upsertStateEntry` lost its `attributes` + parameter. +- `buildStateV2` is kept as a deprecated alias of `buildState` that + silently drops attributes — keeps the public name from breaking until + the next major. + +See the v3 schema in `src/lib/yaml/state.ts` and the diff pipeline in +`src/lib/yaml/differ.ts`. + +## Summary + +Today the `devhelm` CLI compares user YAML against a local `state.json` +snapshot of "what we last applied" instead of against the live API. This +proposal flips that: **`state.json` keeps only the identity mapping +(YAML address → API UUID + minimum disambiguators); attribute values are +always re-read from the live API at plan time**. The result is the same +contract Terraform itself ships with (`refresh` before `plan`), gives us +correct out-of-band drift detection for free, and eliminates an entire +class of bug we keep hitting (most recently `defaultOpen` on status +page groups). + +## Motivation + +### What broke (concrete incidents) + +1. **`defaultOpen` drift between environments (April 2026, mini deploy).** + Production was deployed first (`defaultOpen: true` reached the prod + API and the local `state.json`). The same operator then targeted the + mini API, where the DB still had `defaultOpen: false`. `devhelm plan` + showed *no changes* because the local snapshot already said `true`. + The bug was not in the diff — it was that the diff had no idea what + the mini API actually held. + +2. **`pullStatusPageChildren` underfilled attributes.** `state pull` only + wrote `{name}` into the snapshot for every group/component, ignoring + `defaultOpen`, `description`, `showUptime`, etc. After a `state + pull`, the very next `plan` would falsely report drift on every + field — or, depending on which side of the diff the bug landed, + miss real drift. (Now fixed via single-source `*CurrentSnapshot` + helpers, but the architecture made it inevitable.) + +3. **Manual SQL fixup invisible to plan.** During the same incident an + operator updated `default_open = true` directly via `psql` on the + mini DB. `devhelm plan` did not flag this as drift because the local + `state.json` agreed with the YAML — neither had any window into the + actual API. + +All three share the same root cause: the diff input is local memory +(`state.json`'s `attributes`), not the live API. + +### Why this keeps happening + +The state file currently serves four roles bundled into one document: + +| Role | Why it's needed | Owner of truth | +| --- | --- | --- | +| Identity mapping (`address → apiId`) | Rename detection, child-collection reconciliation | State file | +| Cross-deploy bookkeeping (`serial`, `lastDeployedAt`) | Audit trail | State file | +| Drift baseline (`attributes`) | Compare YAML vs. "what we last shipped" | State file (today) — but morally the API | +| Drift baseline (`children[*].attributes`) | Same, for status page child collections | State file (today) — same problem | + +The first two are inherent to the CLI (no other source has them). The +last two are not — they exist in the API and we already pay the cost of +fetching them at deploy time. Storing a stale shadow copy in +`state.json` and using THAT as the diff baseline trades **drift +correctness** for **fewer HTTP calls at plan time**, which is the wrong +trade for an IaC tool. + +### Precedent: the DevHelm Terraform provider already does this + +Our own Terraform provider (`devhelmhq/terraform-provider-devhelm`) +implements the proposed model. Each `devhelm_status_page_component_group` +resource's `Read()` method always queries the live API +(`internal/provider/resources/status_page_component_group.go:172-205`) +and overwrites the in-memory state with what the API returned. The +TF state file (`terraform.tfstate`) functions as the identity index; +the actual diff input for `terraform plan` is whatever `Read()` just +returned. + +Critically, the TF provider has **never** had a `defaultOpen`-style +drift bug, despite shipping the same field. The same pattern works in +the CLI. + +## Goals + +- `devhelm plan` correctly reports out-of-band changes (DB edits, dashboard + edits, deploys from another machine) without requiring `devhelm state pull` + as a defensive ritual. +- Adding a new field to a managed resource's API DTO requires updating + exactly one place in the CLI (the YAML schema + the YAML→snapshot + function — same as today). It must not require also updating a + separate "what to put in state" path. +- `state.json` becomes small, stable, and human-auditable — the only + values inside are identifiers and operational metadata. +- `devhelm state pull` becomes a no-op for attribute hydration (it + rebuilds identity only). `state pull` remains useful only for + rebuilding a lost state file from scratch. + +## Non-goals + +- Replacing `state.json` with a remote backend. This RFC keeps the file + local + gitignored. +- Changing the YAML schema or any user-facing DSL. +- Changing how child-collection identity is keyed (`groups.`, + `components.` stay). +- Changing concurrency / locking semantics. + +## Proposed design + +### 1. New state schema (v3) + +```jsonc +{ + "version": "3", + "serial": 42, + "lastDeployedAt": "2026-04-22T18:30:00Z", + "resources": { + "monitors.API": { + "apiId": "mon-uuid-1", + "resourceType": "monitor" + }, + "statusPages.public": { + "apiId": "sp-uuid-1", + "resourceType": "statusPage", + "children": { + "groups.Platform": {"apiId": "g-uuid-1"}, + "components.API": {"apiId": "c-uuid-1"} + } + } + } +} +``` + +Differences from v2: + +- `attributes` field removed from `StateEntry` and `ChildStateEntry`. + These were the source of all four drift bugs above. +- `children[*]` becomes `{apiId: string}` only. +- Everything else is unchanged. + +### 2. Plan flow becomes "refresh then diff" + +Today (`plan.ts` → `differ.ts`): +1. Read `state.json`. +2. Fetch all primary resources from the API (`fetchAllRefs`). +3. For each YAML resource, find its API counterpart via state matching. +4. Diff YAML's snapshot vs. **state.json**'s `lastDeployedAttributes` / + children `attributes`. + +Proposed: +1. Read `state.json` (identity only). +2. Fetch all primary resources from the API (unchanged). +3. **For status pages and any other parent with child collections, + fetch children eagerly** (`/api/v1/status-pages/{id}/{groups,components}`). + This is one extra round-trip per status page on plan; status pages are + rare (typically 1 per workspace), so the cost is negligible. +4. Build a "current snapshot" per resource by passing the API DTO + through the existing `toCurrentSnapshot` (now the SOLE path that + produces snapshots — `state pull` calls into the same path). +5. Diff YAML's `toDesiredSnapshot` vs. the freshly-built current + snapshot. Same comparator code as today; only the right-hand-side + source changes. + +### 3. `applier.ts` no longer writes `attributes` to state + +After a successful create/update, the applier writes only `{apiId, resourceType}` +(plus the identity portion of `children`). The next plan will re-read +the API. + +### 4. Child-collection diffing collapses + +`hasStatusPageChildChanges` and `childCollectionDiffers` (`handlers.ts:1113-1150`) +disappear from the plan path. They were the workaround for "we don't +have child API data at plan time"; with eager child fetch, the regular +add/update/delete reconciler runs at both plan-time (read-only, +producing the diff summary) and apply-time (mutating). + +### 5. Migration: silent v2 → v3 read + +`readState()` already supports v1 → v2 silent migration. We add v2 → v3: + +- Read v2 if present. +- Drop all `attributes` fields, keep identity only. +- On the next `deploy`, write v3. + +No user action. No flag day. The first plan after upgrade may show +*real* drift the user didn't know about — that's the feature. + +### 6. `state pull` shrinks + +`pullStatusPageChildren` (and any future per-collection puller) returns +only `{apiId}`. We delete the recently-added `*CurrentSnapshot` calls +in pull (the helpers themselves stay in `handlers.ts`, now used solely +by the plan path). + +`devhelm state pull` continues to exist for genuine +state-file-loss recovery, but its blast radius is now tiny: it cannot +cause phantom diffs because it stores no attributes to compare against. + +## Trade-offs + +### Cost: more HTTP calls at plan time + +- Today: `plan` issues N list calls (one per primary resource type) + + zero per-status-page child fetches. +- Proposed: same N + 2× per status page (groups + components). + Status pages are typically 1–3 per workspace; we go from ~10 list + calls to ~12–16. With local API or DOKS-internal, this is sub-100ms. +- For the production `devhelm.yml` (the dogfood case): currently 1 + status page → 2 extra calls. Negligible. + +If this becomes a problem at very high resource counts, the API has +existing pagination + `?expand=children` patterns we can opt into. Out +of scope for this RFC. + +### Cost: plan now requires API access + +Today, a `devhelm plan` against a stale state file kind-of works +offline (it diffs YAML against last-known attributes). In practice this +is never useful — `plan` already requires API access for primary refs +fetch. No regression. + +### Benefit: third-party edits become first-class + +Edits made via the dashboard, the public API, the Terraform provider, +or direct DB SQL all surface as drift in `devhelm plan` after this RFC. +This is the explicit positive: the CLI stops pretending it's the only +writer. + +### Risk: a future field that's expensive to fetch + +If we ever add a field to a managed resource that's expensive to compute +on the API side (e.g. a derived rollup), eager fetch at plan time would +pay that cost on every plan. Mitigation: the API can move expensive +fields off the default DTO (we already do this for monitor stats), and +the snapshot helpers naturally exclude fields that aren't on the DTO. +This isn't a new problem — `lastDeployedAttributes` would have been +stale anyway. + +### Risk: divergence between deploy-time apply and plan-time refresh + +Today, `applyUpdate` writes attributes back to state immediately, so +plan and apply read the same baseline. Proposed: plan and apply both +read the API; if the API mutates between plan and apply, the user gets +exactly what `terraform apply` does — a possible re-plan prompt or a +benign no-op. We accept this; it is the correct semantic. + +## Alternatives considered + +### A. Keep state attributes, add a `--refresh` flag to `plan` + +Equivalent to `terraform plan -refresh=true`. Cheap to ship, but the +default behavior remains wrong, and the existing failure modes (forgot +to run `state pull`, forgot the flag) persist. We've already lived +with this for ~4 versions; the workaround is exactly what users are +not doing. + +### B. Server-side ETag / version comparison + +API returns `version` per resource; CLI stores only versions in +`state.json` and asks the API "have any of these changed?". Smaller +state, fewer bytes — but requires API-side support, doesn't help when +no version field exists, and still needs full DTOs to compute the diff +display. Not chosen. + +### C. Drop `state.json` entirely; key everything by name + +Equivalent to running `terraform import` on every plan. Breaks rename +detection, makes child collections ambiguous, and we'd need an "I +manage this" flag at the API level. Considered and rejected: the +identity mapping is genuinely useful and small. + +### D. Hybrid — keep attributes but also fetch live and warn on mismatch + +Doubles the work, splits the truth, and exposes operators to "the CLI +says X but the API says Y, who is right?" prompts on every plan. +Strictly worse than (A) or this RFC. + +## Open questions + +- **`pluginManifest`-style cache for primary resource DTOs?** If two + consecutive `plan`s within ~30s could share an in-memory fetch, we + could halve the round-trips during interactive iteration. Probably a + follow-up; out of scope. +- **What about resources with derived attributes the API doesn't echo + back?** None today — every YAML field round-trips through a DTO. If + this changes, those fields go into `state.json` explicitly with a + doc note ("server-derived; CLI-tracked"). +- **Backfill: should the v2→v3 migration warn when it would have + reported drift?** Tempting (helps operators audit). Probably yes, + gated behind a one-shot `--explain-migration` flag so we don't spam + CI. + +## Migration plan + +This RFC is a single PR (post-approval), staged behind a feature flag: + +1. **Phase 0 (already shipped, this branch).** Single-source + `*CurrentSnapshot` helpers + `state pull` parity tests. Removes + the underfilling bug class even before this RFC lands. +2. **Phase 1 — additive.** Add eager child fetch + v3 reader behind + `DEVHELM_STATE_V3=1`. Internal dogfood for one release cycle. +3. **Phase 2 — default on.** Flip the env var default. Both readers + present; writer emits v3. Existing v2 state files silently upgrade + on next deploy. +4. **Phase 3 — cleanup.** Remove v2 writer + `lastDeployedAttributes` + field from `ChildStateEntry`. Remove `childCollectionDiffers`. Ship + in next minor. + +## Acceptance criteria + +The implementation PR is mergeable when: + +- All four reproduction cases above (`defaultOpen` cross-env, + `pullStatusPageChildren` underfill, manual SQL fixup, dashboard + edit) produce a correct diff in `devhelm plan` *without* the user + having run `devhelm state pull`. +- `state.json` no longer contains any field whose source-of-truth lives + in the API. +- Existing 867 unit tests pass unchanged (the diff comparator's input + changes; the comparator itself does not). +- New integration test in `tests/integration/cli/` exercises the + cross-environment scenario end-to-end (deploy A, mutate B's DB, + plan against B, expect drift report). +- Terraform provider's own `Read`-based pattern is referenced in the + shipping documentation as the canonical analog. + +## Out of this RFC's scope (tracked separately) + +- **Plan UX (Issue B).** The diff renderer currently prints opaque + "groups: changed" for child collections. Once attributes are + per-field, the renderer should print field-level diffs. Tracked as a + follow-up since the architectural change here unblocks it. +- **Terraform provider.** Confirmed not affected (it already uses the + proposed model). No work needed. diff --git a/docs/rfcs/README.md b/docs/rfcs/README.md new file mode 100644 index 0000000..bd6c5d9 --- /dev/null +++ b/docs/rfcs/README.md @@ -0,0 +1,20 @@ +# DevHelm CLI RFCs + +Architectural change proposals for the `devhelm` CLI. Each RFC is a +self-contained design doc, opened as a PR and merged once consensus +exists. Implementation PRs reference the RFC number. + +## Index + +| # | Status | Title | +| --- | --- | --- | +| 0001 | Draft | [`state.json` as identity-only; live-API attribute reads at plan time](./0001-state-as-identity-only.md) | + +## Process + +1. Open a PR adding `docs/rfcs/-.md` (next available number). +2. Discussion happens in the PR. +3. Merging the RFC = consensus on the design. Implementation may land in the + same PR (small RFCs) or follow-ups (larger ones). +4. Update the `Status` field in the RFC header as it moves through + `Draft` → `Accepted` → `Shipped` (or `Rejected`/`Superseded`). diff --git a/src/commands/deploy/index.ts b/src/commands/deploy/index.ts index 88bffee..a04ce2b 100644 --- a/src/commands/deploy/index.ts +++ b/src/commands/deploy/index.ts @@ -4,7 +4,7 @@ import {checkedFetch, createApiClient, apiDelete} from '../../lib/api-client.js' import {resolveToken, resolveApiUrl} from '../../lib/auth.js' import {DevhelmApiError, EXIT_CODES} from '../../lib/errors.js' import {urlFlag} from '../../lib/validators.js' -import {loadConfig, validate, validatePlanRefs, fetchAllRefs, registerYamlPendingRefs, diff, formatPlan, changesetToJson, apply, writeState, buildStateV2, readState, emptyState, processMovedBlocks, resourceAddress, StateFileCorruptError} from '../../lib/yaml/index.js' +import {loadConfig, validate, validatePlanRefs, fetchAllRefs, registerYamlPendingRefs, diff, prefetchChildSnapshots, formatPlan, changesetToJson, apply, writeState, buildStateV2, readState, emptyState, processMovedBlocks, resourceAddress, StateFileCorruptError} from '../../lib/yaml/index.js' import {checkEntitlements, formatEntitlementWarnings} from '../../lib/yaml/entitlements.js' const DEFAULT_LOCK_TTL = 30 @@ -152,10 +152,12 @@ export default class Deploy extends Command { this.error('Fix validation errors before deploying', {exit: EXIT_CODES.VALIDATION}) } - const changeset = diff( + const currentChildren = await prefetchChildSnapshots(config, refs, client) + const changeset = await diff( config, refs, {prune: flags.prune || flags['prune-all'], pruneAll: flags['prune-all']}, currentState, + currentChildren, ) const entitlementCheck = await checkEntitlements(client, changeset) diff --git a/src/commands/import.ts b/src/commands/import.ts index 29e30dc..6c5ec90 100644 --- a/src/commands/import.ts +++ b/src/commands/import.ts @@ -111,7 +111,7 @@ export default class Import extends Command { children = await this.fetchStatusPageChildren(client, entry.id) } - upsertStateEntry(state, resourceType, args.name, entry.id, {name: args.name}, children) + upsertStateEntry(state, resourceType, args.name, entry.id, children) writeState(state) const childCount = Object.keys(children).length @@ -131,7 +131,7 @@ export default class Import extends Command { for (const g of groups) { const name = g.name ?? '' if (!name) continue - children[`groups.${name}`] = {apiId: String(g.id ?? ''), attributes: {name}} + children[`groups.${name}`] = {apiId: String(g.id ?? '')} } const components = await fetchPaginated( client, `/api/v1/status-pages/${pageId}/components`, @@ -139,7 +139,7 @@ export default class Import extends Command { for (const c of components) { const name = c.name ?? '' if (!name) continue - children[`components.${name}`] = {apiId: String(c.id ?? ''), attributes: {name}} + children[`components.${name}`] = {apiId: String(c.id ?? '')} } } catch (err) { this.warn(`Failed to fetch status page children: ${err instanceof Error ? err.message : String(err)}`) diff --git a/src/commands/plan.ts b/src/commands/plan.ts index bb7524d..9aa35fb 100644 --- a/src/commands/plan.ts +++ b/src/commands/plan.ts @@ -3,7 +3,7 @@ import {createApiClient} from '../lib/api-client.js' import {resolveToken, resolveApiUrl} from '../lib/auth.js' import {EXIT_CODES} from '../lib/errors.js' import {urlFlag} from '../lib/validators.js' -import {loadConfig, validate, validatePlanRefs, fetchAllRefs, registerYamlPendingRefs, diff, formatPlan, changesetToJson, readState, emptyState, previewMovedBlocks, StateFileCorruptError} from '../lib/yaml/index.js' +import {loadConfig, validate, validatePlanRefs, fetchAllRefs, registerYamlPendingRefs, diff, prefetchChildSnapshots, formatPlan, changesetToJson, readState, emptyState, previewMovedBlocks, StateFileCorruptError} from '../lib/yaml/index.js' import {checkEntitlements, formatEntitlementWarnings} from '../lib/yaml/entitlements.js' export default class Plan extends Command { @@ -126,10 +126,12 @@ export default class Plan extends Command { this.error('Fix validation errors first', {exit: EXIT_CODES.VALIDATION}) } - const changeset = diff( + const currentChildren = await prefetchChildSnapshots(config, refs, client) + const changeset = await diff( config, refs, {prune: flags.prune || flags['prune-all'], pruneAll: flags['prune-all']}, currentState, + currentChildren, ) const entitlementCheck = await checkEntitlements(client, changeset) diff --git a/src/commands/state/pull.ts b/src/commands/state/pull.ts index 42a0ac9..fba1740 100644 --- a/src/commands/state/pull.ts +++ b/src/commands/state/pull.ts @@ -7,7 +7,7 @@ import {urlFlag} from '../../lib/validators.js' import {fetchAllRefs} from '../../lib/yaml/resolver.js' import {allHandlers} from '../../lib/yaml/handlers.js' import {fetchPaginated} from '../../lib/typed-api.js' -import {writeState, buildStateV2, readState, StateFileCorruptError} from '../../lib/yaml/state.js' +import {writeState, buildState, readState, StateFileCorruptError} from '../../lib/yaml/state.js' import type {ChildStateEntry} from '../../lib/yaml/state.js' type Schemas = components['schemas'] @@ -49,10 +49,9 @@ export default class StatePull extends Command { const handlers = allHandlers() const entries: Array<{ - resourceType: Parameters[0][number]['resourceType'] + resourceType: Parameters[0][number]['resourceType'] refKey: string apiId: string - attributes: Record children?: Record }> = [] @@ -66,7 +65,6 @@ export default class StatePull extends Command { resourceType: handler.resourceType, refKey: entry.refKey, apiId: entry.id, - attributes: {name: entry.refKey}, children, }) } @@ -83,7 +81,7 @@ export default class StatePull extends Command { throw err } } - const state = buildStateV2(entries, prevState?.serial ?? 0) + const state = buildState(entries, prevState?.serial ?? 0) if (flags['dry-run']) { this.log(`\nWould write ${entries.length} resources to state:`) @@ -99,9 +97,15 @@ export default class StatePull extends Command { } /** - * Fetch groups and components for a status page and flatten them into the - * `children` shape used by StateEntry. Keys are `groups.` / - * `components.` to match the child-reconciler's identity scheme. + * Fetch identity (id only) for groups and components belonging to a + * status page and flatten them into the `children` shape used by + * StateEntry. Keys are `groups.` / `components.` so the + * child-reconciler's state-aware rename matching keeps working after + * a pull. + * + * As of state v3 (RFC 0001), only the apiId is stored. Attributes are + * always re-fetched from the API at plan time, so there is nothing to + * snapshot here — pull becomes purely an identity-recovery operation. */ private async pullStatusPageChildren( client: ReturnType, @@ -115,7 +119,7 @@ export default class StatePull extends Command { for (const g of groups) { const name = g.name ?? '' if (!name) continue - children[`groups.${name}`] = {apiId: String(g.id ?? ''), attributes: {name}} + children[`groups.${name}`] = {apiId: String(g.id ?? '')} } const components = await fetchPaginated( client, `/api/v1/status-pages/${pageId}/components`, @@ -123,7 +127,7 @@ export default class StatePull extends Command { for (const c of components) { const name = c.name ?? '' if (!name) continue - children[`components.${name}`] = {apiId: String(c.id ?? ''), attributes: {name}} + children[`components.${name}`] = {apiId: String(c.id ?? '')} } } catch (err) { this.warn(`Failed to fetch children for status page ${pageId}: ${err instanceof Error ? err.message : String(err)}`) diff --git a/src/lib/yaml/applier.ts b/src/lib/yaml/applier.ts index 81b4c8c..361533e 100644 --- a/src/lib/yaml/applier.ts +++ b/src/lib/yaml/applier.ts @@ -17,7 +17,6 @@ export interface AppliedStateEntry { resourceType: ResourceType refKey: string apiId: string - attributes: Record children: Record } @@ -89,7 +88,6 @@ export async function apply( resourceType: change.resourceType as ResourceType, refKey: change.refKey, apiId: id, - attributes: {name: change.refKey}, children: outcome?.children ?? {}, }) succeeded.push({action: 'create', resourceType: change.resourceType, refKey: change.refKey, id}) @@ -118,7 +116,6 @@ export async function apply( resourceType: change.resourceType as ResourceType, refKey: change.refKey, apiId: err.partial.id, - attributes: {name: change.refKey}, children: err.partial.children ?? {}, }) } @@ -141,7 +138,6 @@ export async function apply( resourceType: change.resourceType as ResourceType, refKey: change.refKey, apiId: change.existingId, - attributes: {name: change.refKey}, children: outcome.children ?? priorChildren, }) } @@ -155,7 +151,6 @@ export async function apply( resourceType: change.resourceType as ResourceType, refKey: change.refKey, apiId: change.existingId, - attributes: {name: change.refKey}, children: err.partial.children ?? priorChildren, }) } diff --git a/src/lib/yaml/child-reconciler.ts b/src/lib/yaml/child-reconciler.ts index 58dee37..5592fd1 100644 --- a/src/lib/yaml/child-reconciler.ts +++ b/src/lib/yaml/child-reconciler.ts @@ -220,9 +220,7 @@ export async function applyChildDiff( errors.push(`delete ${def.name}.${del.key}: ${errorMessage(err)}`) // Carry the orphan forward in state so the next diff still sees it // (its YAML key is gone, so the next run re-queues the delete). - // Empty attributes keep `hasChildChanges` indifferent — the diff - // logic decides delete vs update from YAML presence, not attrs. - childState[`${def.name}.${del.key}`] = {apiId: del.childId, attributes: {}} + childState[`${def.name}.${del.key}`] = {apiId: del.childId} } } @@ -249,14 +247,12 @@ export async function applyChildDiff( changes.push({action: 'update', childKey: update.key, childId: update.childId}) } catch (err) { errors.push(`update ${def.name}.${update.key}: ${errorMessage(err)}`) - // Record the apiId with empty attributes so the next diff sees the - // child as still drifted (desired snapshot ≠ stored {}) and retries - // the update. Critically, we do NOT store the desired snapshot here - // — that would mark the child as in-sync and the retry would never - // happen. - childState[`${def.name}.${update.key}`] = { - apiId: update.childId, attributes: {}, - } + // Record the apiId so the next deploy still owns the child via state + // for state-aware rename matching. Drift retry no longer relies on + // state attributes (those are gone in v3); the next plan re-fetches + // live API children and will see the desired snapshot ≠ current and + // re-queue the update. + childState[`${def.name}.${update.key}`] = {apiId: update.childId} failedKeys.add(update.key) } } @@ -283,7 +279,7 @@ export async function applyChildDiff( // Build child state for every desired child whose identity is known. // - Skipped: failed creates (no apiId yet — re-run retries as create) // - Skipped: keys already populated above (failed updates/deletes) so we - // don't overwrite their attributes-empty marker with the desired snap. + // keep the failed-marker apiId entry intact. // Renamed children survive because `existingByKey` was populated by // state-aware matching in diff. for (const yaml of desiredYaml) { @@ -293,10 +289,7 @@ export async function applyChildDiff( if (childState[stateKey] !== undefined) continue const apiId = newIds.get(key) ?? existingByKey[key] if (apiId) { - childState[stateKey] = { - apiId, - attributes: def.toDesiredSnapshot(yaml), - } + childState[stateKey] = {apiId} } } diff --git a/src/lib/yaml/differ.ts b/src/lib/yaml/differ.ts index 722ae9e..6af0ebd 100644 --- a/src/lib/yaml/differ.ts +++ b/src/lib/yaml/differ.ts @@ -4,14 +4,20 @@ * * Delegates all per-resource-type semantic comparison to typed handlers * in handlers.ts — no Record anywhere in this file. + * + * **State as identity, not snapshot** (see RFC 0001): for handlers that + * declare `fetchChildSnapshots`, the differ pre-fetches live API children + * at plan time and feeds them into `hasChildChanges`. State is consulted + * only for renames/identity (`apiId`), never as a diff baseline. */ +import type {ApiClient} from '../api-client.js' import type {components} from '../api.generated.js' import type {DevhelmConfig} from './schema.js' import type {ResolvedRefs} from './resolver.js' -import {allHandlers, type ResourceHandler} from './handlers.js' +import {allHandlers, type ResourceHandler, type CurrentChildEntry} from './handlers.js' import type {Change, Changeset, DiffOptions} from './types.js' import {RESOURCE_ORDER} from './types.js' -import type {DeployState, ChildStateEntry} from './state.js' +import type {DeployState} from './state.js' import {resourceAddress} from './state.js' import type {ResourceType} from './types.js' @@ -23,29 +29,47 @@ export type {ChangeAction, ResourceType, Change, DiffOptions, Changeset, Attribu // ── Main diff function ───────────────────────────────────────────────── -export function diff( +/** Pre-fetched live child snapshots, keyed by parent resource-address + * (e.g. `statusPages.platform`). Built by `prefetchChildSnapshots` and + * consumed by `diff`. Empty for parents whose handler doesn't declare + * `fetchChildSnapshots` or that don't yet exist in the API. */ +export type ChildSnapshotMap = Map> + +/** + * Compute the changeset for a deploy/plan run. + * + * **Async because of child collections.** For handlers like `statusPage` + * that declare `fetchChildSnapshots`, the differ needs live API child + * data as the diff baseline (see RFC 0001). Pre-fetching is split into + * `prefetchChildSnapshots` so production callers can run all child + * fetches concurrently up-front, while tests can inject an empty/fake + * map without spinning up an HTTP mock. + * + * `currentChildren` defaults to an empty map. That's the correct + * behavior for configs without child-bearing parents AND for tests that + * only care about parent-field diffing. + */ +export async function diff( config: DevhelmConfig, refs: ResolvedRefs, options: DiffOptions = {}, priorState?: DeployState, -): Changeset { + currentChildren: ChildSnapshotMap = new Map(), +): Promise { const creates: Change[] = [] const updates: Change[] = [] const deletes: Change[] = [] const memberships: Change[] = [] - // Closure so diffSection can resolve child snapshots without threading - // priorState through every helper. Returns {} for handlers / refs with - // no prior tracked state — child-aware handlers treat that as "no children - // tracked" and rely solely on parent-field hasChanged. - function lookupPriorChildren(resourceType: ResourceType, refKey: string): Record { - if (!priorState) return {} + function lookupCurrentChildren( + resourceType: ResourceType, refKey: string, + ): Record { const addr = resourceAddress(resourceType, refKey) - return priorState.resources[addr]?.children ?? {} + return currentChildren.get(addr) ?? {} } for (const handler of allHandlers()) { - diffSection(handler, config[handler.configKey], refs, creates, updates, deletes, options, lookupPriorChildren) + diffSection(handler, config[handler.configKey], refs, creates, updates, deletes, options, lookupCurrentChildren) } diffMemberships(config, refs, memberships, options) @@ -54,9 +78,59 @@ export function diff( updates.sort(byResourceOrder) deletes.sort((a, b) => byResourceOrderIndex(b.resourceType) - byResourceOrderIndex(a.resourceType)) + // Acknowledge unused param: kept in the signature for future drift hooks + // (e.g. detecting parents present in state but absent from both YAML and + // the live API) without forcing every caller to refactor. The differ + // itself reads parent attrs only from `refs` (live API) per RFC 0001. + void priorState + return {creates, updates, deletes, memberships} } +/** + * For every handler that declares `fetchChildSnapshots`, fetch live API + * children for each parent that already exists in the API. Returns a map + * keyed by parent resource-address so the inner diff loop can look up + * children in O(1) without async. + * + * Network errors are NOT swallowed: if the API fails to return children + * for an existing parent, the plan is incorrect by construction (we'd + * silently fall back to "no children" and miss real drift). Better to + * surface the failure and let the caller retry. + */ +export async function prefetchChildSnapshots( + config: DevhelmConfig, + refs: ResolvedRefs, + client: ApiClient, +): Promise { + const out: ChildSnapshotMap = new Map() + const tasks: Promise[] = [] + + for (const handler of allHandlers()) { + if (!handler.fetchChildSnapshots) continue + const items = config[handler.configKey] as unknown[] | undefined + if (!items || items.length === 0) continue + + // Only fetch for parents that actually exist in the API (i.e. have a + // ref entry that isn't a YAML-only pending placeholder). New parents + // skip the fetch — their children are handled by `applyCreate`. + for (const item of items) { + const refKey = handler.getRefKey(item) + const existing = refs.get(handler.refType, refKey) + if (!existing || existing.isPending) continue + const addr = resourceAddress(handler.resourceType as ResourceType, refKey) + tasks.push( + handler.fetchChildSnapshots(existing.id, client, refs).then((snapshots) => { + out.set(addr, snapshots) + }), + ) + } + } + + await Promise.all(tasks) + return out +} + /** * Resource-order comparator. Unknown resource types sort *after* known ones * (using Number.MAX_SAFE_INTEGER) and then by resourceType alphabetically for @@ -85,7 +159,7 @@ function diffSection( updates: Change[], deletes: Change[], options: DiffOptions, - lookupPriorChildren: (resourceType: ResourceType, refKey: string) => Record, + lookupCurrentChildren: (resourceType: ResourceType, refKey: string) => Record, ): void { const desired = new Set() @@ -102,10 +176,12 @@ function diffSection( const parentChanged = handler.hasChanged(item, existing.raw, refs) // hasChildChanges complements parent comparison so the differ can // detect "user added a component to an existing status page" even when - // the page itself is byte-identical. Always queried (cheap, sync) and - // OR'd into the change decision. - const priorChildren = lookupPriorChildren(handler.resourceType as ResourceType, refKey) - const childrenChanged = handler.hasChildChanges?.(item, priorChildren) ?? false + // the page itself is byte-identical. The children map was pre-fetched + // live from the API in prefetchChildSnapshots above, so this OR'd-in + // check correctly catches drift across machines/environments without + // depending on `state.json` having an up-to-date snapshot. + const currentChildren = lookupCurrentChildren(handler.resourceType as ResourceType, refKey) + const childrenChanged = handler.hasChildChanges?.(item, currentChildren) ?? false if (parentChanged || childrenChanged) { const attributeDiffs = handler.computeAttributeDiffs?.(item, existing.raw, refs) updates.push({ diff --git a/src/lib/yaml/handlers.ts b/src/lib/yaml/handlers.ts index f10ae0d..cc40848 100644 --- a/src/lib/yaml/handlers.ts +++ b/src/lib/yaml/handlers.ts @@ -96,6 +96,19 @@ export interface ApplyOutcome { children?: Record } +/** + * Live snapshot of a child resource fetched from the API at plan time. + * + * The differ feeds these to `hasChildChanges` so child drift is detected + * against the API truth, not against `state.json` (which v3 no longer + * stores attributes for — see RFC 0001). `apiId` is preserved alongside + * the snapshot to keep state-aware rename matching working. + */ +export interface CurrentChildEntry { + apiId: string + snapshot: Record +} + export interface ResourceHandler { readonly resourceType: HandledResourceType readonly refType: RefType @@ -115,15 +128,25 @@ export interface ResourceHandler { * only the children differ — without it, statusPageHandler.hasChanged * would compare parent fields only and miss "user added a new component". * - * Compares YAML's children against the prior-state child snapshots that - * `applyChildDiff` wrote on the previous deploy. This is sync — no API - * fetch needed — because state already carries the last-deployed snapshot. - * - * Returns false (no children changed) when the handler doesn't manage - * children, or when `priorChildren` is empty AND the YAML has no children - * declared (first-time deploy path is fully handled by applyCreate). + * **v3 contract**: `currentChildren` is fetched live from the API by the + * differ via `fetchChildSnapshots` immediately before this is called. + * That means cross-environment drift (e.g. a deploy from machine A + * followed by a plan from machine B with no `state pull`) is detected + * correctly without relying on `state.json`'s attributes. + */ + hasChildChanges?(yaml: TYaml, currentChildren: Record): boolean + + /** + * Fetch live child snapshots for a parent. Required if `hasChildChanges` + * is implemented. Called by the differ in parallel for every parent that + * already exists in the API. The returned map is keyed `.` + * (e.g. `groups.Platform`) for parity with the apply path's state keys. */ - hasChildChanges?(yaml: TYaml, priorChildren: Record): boolean + fetchChildSnapshots?( + parentApiId: string, + client: ApiClient, + refs: ResolvedRefs, + ): Promise> fetchAll(client: ApiClient): Promise applyCreate( @@ -159,8 +182,15 @@ interface HandlerDef { toDesiredSnapshot(yaml: TYaml, api: TApiDto, refs: ResolvedRefs): TSnapshot toCurrentSnapshot(api: TApiDto): TSnapshot - /** Optional: report drift inside child collections using prior state. */ - hasChildChanges?(yaml: TYaml, priorChildren: Record): boolean + /** Optional: report drift inside child collections using live API data. */ + hasChildChanges?(yaml: TYaml, currentChildren: Record): boolean + + /** Optional: fetch child snapshots from the API for plan-time diffing. */ + fetchChildSnapshots?( + parentApiId: string, + client: ApiClient, + refs: ResolvedRefs, + ): Promise> fetchAll(client: ApiClient): Promise applyCreate( @@ -212,6 +242,7 @@ function defineHandler( }, hasChildChanges: h.hasChildChanges, + fetchChildSnapshots: h.fetchChildSnapshots, computeAttributeDiffs(yaml: TYaml, api: TApiDto, refs: ResolvedRefs) { const desired = h.toDesiredSnapshot(yaml, api, refs) as Record @@ -926,6 +957,73 @@ export function statusPageComponentDesiredSnapshot( } } +/** + * Translate an API group DTO into the snapshot shape used by the diff + * comparator. This is the single source of truth for "what attributes + * matter on a group" — both the deploy-time collection def and `state pull` + * share this so they cannot drift apart. + * + * Any new field that should participate in drift detection must be added + * here AND in `statusPageGroupDesiredSnapshot` (the YAML-side mirror); the + * shapes must stay structurally identical. + */ +export function statusPageGroupCurrentSnapshot( + api: Schemas['StatusPageComponentGroupDto'], +): Record { + return { + name: api.name ?? '', + description: api.description ?? null, + defaultOpen: api.defaultOpen ?? true, + } +} + +/** + * Translate an API component DTO into the snapshot shape used by the diff + * comparator. Reverse-resolves `groupId` / `monitorId` / `resourceGroupId` + * to the human-readable names used in YAML so desired/current snapshots + * compare as plain strings. + * + * Same drift-detection rules as `statusPageGroupCurrentSnapshot`: every + * field added here must be mirrored in `statusPageComponentDesiredSnapshot`. + */ +export function statusPageComponentCurrentSnapshot( + api: Schemas['StatusPageComponentDto'], + groupNameToId: Map, + refs: ResolvedRefs, +): Record { + let groupName: string | null = null + if (api.groupId) { + for (const [name, id] of groupNameToId) { + if (id === api.groupId) {groupName = name; break} + } + } + let monitorName: string | null = null + if (api.monitorId) { + for (const entry of refs.allEntries('monitors')) { + if (entry.id === api.monitorId) {monitorName = entry.refKey; break} + } + } + let resourceGroupName: string | null = null + if (api.resourceGroupId) { + for (const entry of refs.allEntries('resourceGroups')) { + if (entry.id === String(api.resourceGroupId)) {resourceGroupName = entry.refKey; break} + } + } + return { + name: api.name ?? '', + description: api.description ?? null, + type: api.type ?? 'STATIC', + showUptime: api.showUptime ?? true, + excludeFromOverall: api.excludeFromOverall ?? false, + // API returns an OffsetDateTime (startOfDay UTC); slice back to YYYY-MM-DD + // so desired/current snapshots compare as strings. + startDate: api.startDate ? api.startDate.slice(0, 10) : null, + group: groupName, + monitor: monitorName, + resourceGroup: resourceGroupName, + } +} + function makeGroupCollectionDef( client: ApiClient, ): ChildCollectionDef { @@ -935,11 +1033,7 @@ function makeGroupCollectionDef( apiIdentityKey: (api) => api.name ?? '', apiId: (api) => String(api.id), toDesiredSnapshot: statusPageGroupDesiredSnapshot, - toCurrentSnapshot: (api) => ({ - name: api.name ?? '', - description: api.description ?? null, - defaultOpen: api.defaultOpen ?? true, - }), + toCurrentSnapshot: statusPageGroupCurrentSnapshot, async applyCreate(parentId, yaml, index) { const resp = (await apiPost( client, `/api/v1/status-pages/${parentId}/groups`, @@ -969,40 +1063,7 @@ function makeComponentCollectionDef( apiIdentityKey: (api) => api.name ?? '', apiId: (api) => String(api.id), toDesiredSnapshot: statusPageComponentDesiredSnapshot, - toCurrentSnapshot: (api) => { - // Reverse-resolve groupId/monitorId/resourceGroupId to names for comparison - let groupName: string | null = null - if (api.groupId) { - for (const [name, id] of groupNameToId) { - if (id === api.groupId) {groupName = name; break} - } - } - let monitorName: string | null = null - if (api.monitorId) { - for (const entry of refs.allEntries('monitors')) { - if (entry.id === api.monitorId) {monitorName = entry.refKey; break} - } - } - let resourceGroupName: string | null = null - if (api.resourceGroupId) { - for (const entry of refs.allEntries('resourceGroups')) { - if (entry.id === String(api.resourceGroupId)) {resourceGroupName = entry.refKey; break} - } - } - return { - name: api.name ?? '', - description: api.description ?? null, - type: api.type ?? 'STATIC', - showUptime: api.showUptime ?? true, - excludeFromOverall: api.excludeFromOverall ?? false, - // API returns an OffsetDateTime (startOfDay UTC); slice back to YYYY-MM-DD - // so desired/current snapshots compare as strings. - startDate: api.startDate ? api.startDate.slice(0, 10) : null, - group: groupName, - monitor: monitorName, - resourceGroup: resourceGroupName, - } - }, + toCurrentSnapshot: (api) => statusPageComponentCurrentSnapshot(api, groupNameToId, refs), async applyCreate(parentId, yaml, index) { const body: Record = { name: yaml.name, type: yaml.type, @@ -1057,40 +1118,40 @@ function makeComponentCollectionDef( } /** - * Sync drift detection for status-page child collections. + * Drift detection for status-page child collections (groups + components). * - * Compares the YAML's components/groups against the snapshots stored in the - * prior `state.json` (written by `applyChildDiff` on the previous deploy). - * Returns true if any child was added, removed, renamed, or had any of its - * tracked attributes change. + * **v3 contract**: `currentChildren` is fetched live from the API by the + * differ via `fetchStatusPageChildSnapshots` *before* this is called. Why + * we no longer use prior state for the diff baseline: * - * Why prior state, not the API: - * - hasChanged is sync; fetching live API children would require turning - * the entire diff phase async or pre-fetching every status page's - * children up-front (slow). - * - The prior state IS what we last applied, so a diff of (yaml vs prior - * state) is a complete representation of "user-driven change". External - * drift introduced directly via the API on a child is the concern of - * Tier D (drift recovery) — covered separately and would require an API - * fetch by design. + * - State drift across environments: a deploy from machine A followed + * by a plan from machine B (no `state pull`) would compare YAML + * against B's empty state and conclude "everything must be created" + * — a false positive at best, duplicate-creates at worst. + * - State drift across versions: a CLI release that changes the + * desired-snapshot shape (e.g. the `defaultOpen` rename in v0.4.0) + * would diff every existing parent on every machine until a fresh + * deploy rewrote state. Live API as baseline keeps both sides on + * the same shape forever. See RFC 0001 for the full incident log. * - * Special cases: - * - Empty prior state + no YAML children → no change (handler will still - * get applyCreate which seeds children). - * - YAML omits `components` / `componentGroups` → that collection is left - * alone by `reconcileStatusPageChildren`, so it never reports drift here. + * Special cases (unchanged): + * - Empty current state + no YAML children → no change (applyCreate + * handles the first-time path). + * - YAML omits `components` / `componentGroups` → that collection is + * left alone by `reconcileStatusPageChildren`, so it never reports + * drift here. */ function hasStatusPageChildChanges( yaml: YamlStatusPage, - priorChildren: Record, + currentChildren: Record, ): boolean { if (yaml.componentGroups !== undefined) { - if (childCollectionDiffers(yaml.componentGroups, priorChildren, 'groups.', statusPageGroupDesiredSnapshot)) { + if (childCollectionDiffers(yaml.componentGroups, currentChildren, 'groups.', statusPageGroupDesiredSnapshot)) { return true } } if (yaml.components !== undefined) { - if (childCollectionDiffers(yaml.components, priorChildren, 'components.', statusPageComponentDesiredSnapshot)) { + if (childCollectionDiffers(yaml.components, currentChildren, 'components.', statusPageComponentDesiredSnapshot)) { return true } } @@ -1099,26 +1160,76 @@ function hasStatusPageChildChanges( function childCollectionDiffers( desired: T[], - priorChildren: Record, + currentChildren: Record, prefix: string, toSnapshot: (yaml: T) => Record, ): boolean { - const priorKeys = new Set( - Object.keys(priorChildren).filter((k) => k.startsWith(prefix)).map((k) => k.slice(prefix.length)), + const currentKeys = new Set( + Object.keys(currentChildren).filter((k) => k.startsWith(prefix)).map((k) => k.slice(prefix.length)), ) const desiredKeys = new Set(desired.map((c) => c.name)) - for (const k of desiredKeys) if (!priorKeys.has(k)) return true - for (const k of priorKeys) if (!desiredKeys.has(k)) return true + for (const k of desiredKeys) if (!currentKeys.has(k)) return true + for (const k of currentKeys) if (!desiredKeys.has(k)) return true for (const item of desired) { - const prior = priorChildren[`${prefix}${item.name}`] - if (!prior) return true - if (!isEqual(toSnapshot(item), prior.attributes)) return true + const current = currentChildren[`${prefix}${item.name}`] + if (!current) return true + if (!isEqual(toSnapshot(item), current.snapshot)) return true } return false } +/** + * Live-fetch status page children + project them into snapshots that + * `hasStatusPageChildChanges` can diff against. Used at plan time only; + * the deploy path's `reconcileStatusPageChildren` already does its own + * (richer) live fetch internally. + * + * Component snapshots depend on the group name → id map (so the YAML's + * `group: "Platform"` can be resolved against the API's numeric groupId); + * we build that map from the just-fetched groups before snapshotting + * components. + */ +async function fetchStatusPageChildSnapshots( + pageId: string, + client: ApiClient, + refs: ResolvedRefs, +): Promise> { + const [groups, components] = await Promise.all([ + fetchPaginated( + client, `/api/v1/status-pages/${pageId}/groups`, + ), + fetchPaginated( + client, `/api/v1/status-pages/${pageId}/components`, + ), + ]) + + const out: Record = {} + const groupNameToId = new Map() + + for (const g of groups) { + const name = g.name ?? '' + if (!name) continue + groupNameToId.set(name, String(g.id ?? '')) + out[`groups.${name}`] = { + apiId: String(g.id ?? ''), + snapshot: statusPageGroupCurrentSnapshot(g), + } + } + + for (const c of components) { + const name = c.name ?? '' + if (!name) continue + out[`components.${name}`] = { + apiId: String(c.id ?? ''), + snapshot: statusPageComponentCurrentSnapshot(c, groupNameToId, refs), + } + } + + return out +} + /** * Reconcile groups and components on a status page using the child reconciler. * Replaces the old delete-all/recreate-all approach with individual CRUD. @@ -1284,8 +1395,15 @@ const statusPageHandler = defineHandler fetchPaginated(client, '/api/v1/status-pages'), diff --git a/src/lib/yaml/index.ts b/src/lib/yaml/index.ts index 7e475b9..1840619 100644 --- a/src/lib/yaml/index.ts +++ b/src/lib/yaml/index.ts @@ -6,12 +6,12 @@ export {validate, validatePlanRefs} from './validator.js' export type {ValidationResult, ValidationError} from './validator.js' export {interpolate, interpolateObject, findMissingVariables, findMissingVariablesInObject, findVariablesInObject, InterpolationError} from './interpolation.js' export {fetchAllRefs, registerYamlPendingRefs, ResolvedRefs} from './resolver.js' -export {diff, formatPlan, changesetToJson} from './differ.js' -export type {Changeset, Change, ChangesetJson} from './differ.js' +export {diff, prefetchChildSnapshots, formatPlan, changesetToJson} from './differ.js' +export type {Changeset, Change, ChangesetJson, ChildSnapshotMap} from './differ.js' export {apply} from './applier.js' export type {ApplyResult, AppliedStateEntry} from './applier.js' export {PartialApplyError} from './apply-error.js' export type {PartialOutcome} from './apply-error.js' -export {readState, writeState, buildState, buildStateV2, emptyState, upsertStateEntry, removeStateEntry, lookupByAddress, lookupByApiId, processMovedBlocks, previewMovedBlocks, resourceAddress, parseAddress, parseAndValidateAddress, KNOWN_SECTIONS, migrateV1, StateFileCorruptError} from './state.js' -export type {DeployState, DeployStateV2, StateEntry, ChildStateEntry, DeployStateV1, StateEntryV1} from './state.js' +export {readState, writeState, buildState, buildStateV2, emptyState, upsertStateEntry, removeStateEntry, lookupByAddress, lookupByApiId, processMovedBlocks, previewMovedBlocks, resourceAddress, parseAddress, parseAndValidateAddress, KNOWN_SECTIONS, migrateV1, migrateV2, StateFileCorruptError} from './state.js' +export type {DeployState, DeployStateV3, StateEntry, ChildStateEntry, DeployStateV1, StateEntryV1} from './state.js' export * from './transform.js' diff --git a/src/lib/yaml/state.ts b/src/lib/yaml/state.ts index ca528ac..52208af 100644 --- a/src/lib/yaml/state.ts +++ b/src/lib/yaml/state.ts @@ -1,10 +1,20 @@ /** - * State file v2 for tracking resources managed by `devhelm deploy`. + * State file v3 for tracking resources managed by `devhelm deploy`. * - * The state file is the primary identity source: it maps resource addresses - * (e.g. "monitors.API") to API UUIDs. This enables rename detection via - * `moved` blocks, drift detection via `lastDeployedAttributes`, and child - * resource tracking for status pages and resource groups. + * **Identity-only store.** The state file is the *primary identity source*: + * it maps resource addresses (e.g. `monitors.API`) to API UUIDs and tracks + * the parent → child UUID hierarchy for status pages. Nothing else. + * + * Rationale (see `docs/rfcs/0001-state-as-identity-only.md`): earlier + * versions also stored a snapshot of last-deployed attributes for drift + * detection. That stale snapshot caused multiple incidents — most + * recently the `defaultOpen` cross-environment drift on April 2026 — by + * acting as the diff baseline instead of the live API. v3 removes the + * snapshot, and `devhelm plan` always re-reads attributes from the API. + * + * The historical v1 (flat array, no children) and v2 (with attributes) + * formats are silently migrated by the reader on first load; nothing + * downstream sees them. * * State file: .devhelm/state.json (gitignored by convention) */ @@ -13,28 +23,47 @@ import {join, dirname} from 'node:path' import {z} from 'zod' import type {ResourceType} from './types.js' -// ── V2 types ───────────────────────────────────────────────────────────── +// ── V3 types (current) ─────────────────────────────────────────────────── export interface ChildStateEntry { apiId: string - attributes: Record } export interface StateEntry { apiId: string resourceType: ResourceType - attributes: Record children: Record } -export interface DeployStateV2 { - version: '2' +export interface DeployStateV3 { + version: '3' serial: number lastDeployedAt: string resources: Record } -// ── V1 types (for migration) ───────────────────────────────────────────── +// ── V2 types (for migration only) ──────────────────────────────────────── + +interface ChildStateEntryV2 { + apiId: string + attributes: Record +} + +interface StateEntryV2 { + apiId: string + resourceType: string + attributes: Record + children: Record +} + +interface DeployStateV2 { + version: '2' + serial: number + lastDeployedAt: string + resources: Record +} + +// ── V1 types (for migration only) ──────────────────────────────────────── export interface StateEntryV1 { resourceType: string @@ -49,27 +78,46 @@ export interface DeployStateV1 { resources: StateEntryV1[] } -export type DeployState = DeployStateV2 +export type DeployState = DeployStateV3 // ── Zod schemas for state file validation ──────────────────────────────── -const ChildStateEntrySchema = z.object({ +const ChildStateEntryV3Schema = z.object({ apiId: z.string(), - attributes: z.record(z.unknown()), }) -const StateEntrySchema = z.object({ +const StateEntryV3Schema = z.object({ apiId: z.string(), resourceType: z.string(), - attributes: z.record(z.unknown()), - children: z.record(ChildStateEntrySchema), + children: z.record(ChildStateEntryV3Schema), +}) + +const DeployStateV3Schema = z.object({ + version: z.literal('3'), + serial: z.number(), + lastDeployedAt: z.string(), + resources: z.record(StateEntryV3Schema), +}) + +// V2 schema is permissive — we only need it to validate enough of the shape +// to safely strip out the `attributes` fields during migration. +const ChildStateEntryV2Schema = z.object({ + apiId: z.string(), + attributes: z.record(z.unknown()).optional(), +}) + +const StateEntryV2Schema = z.object({ + apiId: z.string(), + resourceType: z.string(), + attributes: z.record(z.unknown()).optional(), + children: z.record(ChildStateEntryV2Schema).optional(), }) const DeployStateV2Schema = z.object({ version: z.literal('2'), serial: z.number(), lastDeployedAt: z.string(), - resources: z.record(StateEntrySchema), + resources: z.record(StateEntryV2Schema), }) const StateEntryV1Schema = z.object({ @@ -89,7 +137,7 @@ const DeployStateV1Schema = z.object({ const STATE_DIR = '.devhelm' const STATE_FILE = 'state.json' -export const STATE_VERSION = '2' +export const STATE_VERSION = '3' // Resource type → plural section key for address construction const SECTION_NAMES: Record = { @@ -212,12 +260,21 @@ export function readState(cwd: string = process.cwd()): DeployState | undefined return migrateV1(v1.data as DeployStateV1) } - const v2 = DeployStateV2Schema.safeParse(raw) - if (!v2.success) { - const issues = v2.error.issues.map((i) => `${i.path.join('.')}: ${i.message}`).join('; ') - throw new StateFileCorruptError(path, new Error(`invalid v2 state: ${issues}`)) + if (obj.version === '2') { + const v2 = DeployStateV2Schema.safeParse(raw) + if (!v2.success) { + const issues = v2.error.issues.map((i) => `${i.path.join('.')}: ${i.message}`).join('; ') + throw new StateFileCorruptError(path, new Error(`invalid v2 state: ${issues}`)) + } + return migrateV2(v2.data as DeployStateV2) + } + + const v3 = DeployStateV3Schema.safeParse(raw) + if (!v3.success) { + const issues = v3.error.issues.map((i) => `${i.path.join('.')}: ${i.message}`).join('; ') + throw new StateFileCorruptError(path, new Error(`invalid v3 state: ${issues}`)) } - return v2.data as DeployState + return v3.data as DeployState } export function writeState(state: DeployState, cwd: string = process.cwd()): void { @@ -233,7 +290,7 @@ export function writeState(state: DeployState, cwd: string = process.cwd()): voi export function emptyState(): DeployState { return { - version: '2', + version: '3', serial: 0, lastDeployedAt: new Date().toISOString(), resources: {}, @@ -243,13 +300,16 @@ export function emptyState(): DeployState { /** * Create a new state snapshot with incremented serial. * Merges provided entries into a fresh resource map. + * + * Note: callers may still pass `attributes` for backwards compatibility + * with the v2 applier signatures during the transition; they are dropped + * silently. See RFC 0001 for why state no longer stores attributes. */ -export function buildStateV2( +export function buildState( entries: Array<{ resourceType: ResourceType refKey: string apiId: string - attributes?: Record children?: Record }>, previousSerial: number = 0, @@ -260,18 +320,23 @@ export function buildStateV2( resources[addr] = { apiId: e.apiId, resourceType: e.resourceType, - attributes: e.attributes ?? {}, children: e.children ?? {}, } } return { - version: '2', + version: '3', serial: previousSerial + 1, lastDeployedAt: new Date().toISOString(), resources, } } +/** + * @deprecated Use `buildState`. Retained for one release so out-of-tree + * callers (none known) keep building. Drops `attributes` on the floor. + */ +export const buildStateV2 = buildState + /** * Mutate state: set or update a single resource entry. * Increments serial on each call. @@ -281,11 +346,10 @@ export function upsertStateEntry( resourceType: ResourceType, refKey: string, apiId: string, - attributes: Record = {}, children: Record = {}, ): void { const addr = resourceAddress(resourceType, refKey) - state.resources[addr] = {apiId, resourceType, attributes, children} + state.resources[addr] = {apiId, resourceType, children} state.serial++ state.lastDeployedAt = new Date().toISOString() } @@ -396,12 +460,8 @@ export function previewMovedBlocks( addr, { ...entry, - attributes: {...entry.attributes}, children: Object.fromEntries( - Object.entries(entry.children ?? {}).map(([k, v]) => [ - k, - {...v, attributes: {...v.attributes}}, - ]), + Object.entries(entry.children ?? {}).map(([k, v]) => [k, {...v}]), ), }, ]), @@ -411,25 +471,14 @@ export function previewMovedBlocks( return {state: cloned, warnings} } -// ── V1 → V2 migration ─────────────────────────────────────────────────── +// ── Legacy → V3 migration ──────────────────────────────────────────────── /** - * Migrate a v1 state file (flat array, no attributes/children) to v2. - * - * **Synthetic attributes.** v1 stored only the API id and refKey, so we - * cannot reconstruct real attributes here. We seed a placeholder - * `attributes: {name: refKey, _migrated: true}` for two reasons: + * Migrate a v1 state file (flat array, no children) to v3. * - * 1. Some resource types (webhooks, secrets, environments) don't even - * have a `name` field — their refKey is `url`/`key`/`slug`. The - * placeholder is *not* used by the diffing engine (snapshots are - * always rebuilt from the YAML + API DTO via the handler), only as - * filler until the next deploy overwrites it via `buildStateV2`. - * 2. The `_migrated: true` marker makes it trivial to spot a state file - * that has not yet been re-written post-upgrade, both in tooling and - * by humans inspecting `.devhelm/state.json`. - * - * The serial is reset to 1 because v1 didn't track serials. + * v1 stored only `(resourceType, refKey, id)` per entry, which is exactly + * what v3 needs at the parent level — no attributes to drop, no children + * to migrate. The serial is reset to 1 because v1 didn't track serials. */ export function migrateV1(v1: DeployStateV1): DeployState { const resources: Record = {} @@ -438,33 +487,45 @@ export function migrateV1(v1: DeployStateV1): DeployState { resources[addr] = { apiId: entry.id, resourceType: entry.resourceType as ResourceType, - attributes: {name: entry.refKey, _migrated: true}, children: {}, } } return { - version: '2', + version: '3', serial: 1, lastDeployedAt: v1.lastDeployedAt ?? new Date().toISOString(), resources, } } -// ── Backward-compat shim ───────────────────────────────────────────────── +// ── V2 → V3 migration ──────────────────────────────────────────────────── /** - * @deprecated Use buildStateV2. Kept for backward compat with deploy command - * until it's fully migrated to v2 state writes. + * Migrate a v2 state file to v3 by stripping the now-unused `attributes` + * fields at both the parent and child level. Identity (apiId, resourceType, + * children's apiId) is preserved verbatim, so renames and child tracking + * keep working without any user action. + * + * The migration runs silently on first read; the next `deploy` or `state pull` + * persists the v3 shape, and the v2 file is gone. */ -export function buildState( - entries: Array<{resourceType: string; refKey: string; id: string; createdAt: string}>, -): DeployState { - return buildStateV2( - entries.map((e) => ({ - resourceType: e.resourceType as ResourceType, - refKey: e.refKey, - apiId: e.id, - attributes: {name: e.refKey}, - })), - ) +export function migrateV2(v2: DeployStateV2): DeployState { + const resources: Record = {} + for (const [addr, entry] of Object.entries(v2.resources)) { + const children: Record = {} + for (const [key, child] of Object.entries(entry.children ?? {})) { + children[key] = {apiId: child.apiId} + } + resources[addr] = { + apiId: entry.apiId, + resourceType: entry.resourceType as ResourceType, + children, + } + } + return { + version: '3', + serial: v2.serial, + lastDeployedAt: v2.lastDeployedAt, + resources, + } } diff --git a/test/yaml/differ.test.ts b/test/yaml/differ.test.ts index fa1d651..35397d4 100644 --- a/test/yaml/differ.test.ts +++ b/test/yaml/differ.test.ts @@ -9,7 +9,7 @@ function emptyRefs(): ResolvedRefs { describe('differ', () => { describe('diff', () => { - it('detects creates for new resources', () => { + it('detects creates for new resources', async () => { const config: DevhelmConfig = { tags: [{name: 'production', color: '#EF4444'}], monitors: [{ @@ -17,35 +17,35 @@ describe('differ', () => { config: {url: 'https://x.com', method: 'GET'}, }], } - const changeset = diff(config, emptyRefs()) + const changeset = await diff(config, emptyRefs()) expect(changeset.creates).toHaveLength(2) expect(changeset.creates[0].resourceType).toBe('tag') expect(changeset.creates[1].resourceType).toBe('monitor') }) - it('detects updates when field changed', () => { + it('detects updates when field changed', async () => { const refs = new ResolvedRefs() refs.set('tags', 'production', {id: 'tag-1', refKey: 'production', raw: {name: 'production', color: '#000000'}}) const config: DevhelmConfig = { tags: [{name: 'production', color: '#EF4444'}], } - const changeset = diff(config, refs) + const changeset = await diff(config, refs) expect(changeset.updates).toHaveLength(1) expect(changeset.updates[0].existingId).toBe('tag-1') expect(changeset.creates).toHaveLength(0) }) - it('skips update when tag unchanged', () => { + it('skips update when tag unchanged', async () => { const refs = new ResolvedRefs() refs.set('tags', 'production', {id: 'tag-1', refKey: 'production', raw: {name: 'production', color: '#EF4444'}}) const config: DevhelmConfig = { tags: [{name: 'production', color: '#EF4444'}], } - const changeset = diff(config, refs) + const changeset = await diff(config, refs) expect(changeset.updates).toHaveLength(0) }) - it('skips update when monitor unchanged', () => { + it('skips update when monitor unchanged', async () => { const refs = new ResolvedRefs() refs.set('monitors', 'API', {id: 'mon-1', refKey: 'API', raw: { name: 'API', type: 'HTTP', enabled: true, frequencySeconds: 60, @@ -59,11 +59,11 @@ describe('differ', () => { config: {url: 'https://api.com', method: 'GET'}, }], } - const changeset = diff(config, refs) + const changeset = await diff(config, refs) expect(changeset.updates).toHaveLength(0) }) - it('detects update when monitor frequency changed', () => { + it('detects update when monitor frequency changed', async () => { const refs = new ResolvedRefs() refs.set('monitors', 'API', {id: 'mon-1', refKey: 'API', raw: { name: 'API', type: 'HTTP', frequencySeconds: 60, @@ -75,11 +75,11 @@ describe('differ', () => { config: {url: 'https://api.com', method: 'GET'}, }], } - const changeset = diff(config, refs) + const changeset = await diff(config, refs) expect(changeset.updates).toHaveLength(1) }) - it('detects update when monitor config changed', () => { + it('detects update when monitor config changed', async () => { const refs = new ResolvedRefs() refs.set('monitors', 'API', {id: 'mon-1', refKey: 'API', raw: { name: 'API', type: 'HTTP', @@ -91,11 +91,11 @@ describe('differ', () => { config: {url: 'https://api.com/v2', method: 'GET'}, }], } - const changeset = diff(config, refs) + const changeset = await diff(config, refs) expect(changeset.updates).toHaveLength(1) }) - it('detects update when monitor regions changed', () => { + it('detects update when monitor regions changed', async () => { const refs = new ResolvedRefs() refs.set('monitors', 'API', {id: 'mon-1', refKey: 'API', raw: { name: 'API', type: 'HTTP', regions: ['us-east'], @@ -107,11 +107,11 @@ describe('differ', () => { config: {url: 'https://api.com', method: 'GET'}, }], } - const changeset = diff(config, refs) + const changeset = await diff(config, refs) expect(changeset.updates).toHaveLength(1) }) - it('skips update when webhook unchanged', () => { + it('skips update when webhook unchanged', async () => { const refs = new ResolvedRefs() refs.set('webhooks', 'https://hooks.com/x', {id: 'wh-1', refKey: 'https://hooks.com/x', raw: { url: 'https://hooks.com/x', subscribedEvents: ['monitor.down', 'monitor.recovered'], description: 'test', enabled: true, @@ -119,11 +119,11 @@ describe('differ', () => { const config: DevhelmConfig = { webhooks: [{url: 'https://hooks.com/x', subscribedEvents: ['monitor.down', 'monitor.recovered'], description: 'test'}], } - const changeset = diff(config, refs) + const changeset = await diff(config, refs) expect(changeset.updates).toHaveLength(0) }) - it('detects update when webhook events changed', () => { + it('detects update when webhook events changed', async () => { const refs = new ResolvedRefs() refs.set('webhooks', 'https://hooks.com/x', {id: 'wh-1', refKey: 'https://hooks.com/x', raw: { url: 'https://hooks.com/x', subscribedEvents: ['monitor.down'], @@ -131,11 +131,11 @@ describe('differ', () => { const config: DevhelmConfig = { webhooks: [{url: 'https://hooks.com/x', subscribedEvents: ['monitor.down', 'incident.created']}], } - const changeset = diff(config, refs) + const changeset = await diff(config, refs) expect(changeset.updates).toHaveLength(1) }) - it('skips update when environment unchanged', () => { + it('skips update when environment unchanged', async () => { const refs = new ResolvedRefs() refs.set('environments', 'production', {id: 'env-1', refKey: 'production', raw: { name: 'Production', slug: 'production', isDefault: true, @@ -143,21 +143,21 @@ describe('differ', () => { const config: DevhelmConfig = { environments: [{name: 'Production', slug: 'production', isDefault: true}], } - const changeset = diff(config, refs) + const changeset = await diff(config, refs) expect(changeset.updates).toHaveLength(0) }) - it('always updates secrets (value not visible in API)', () => { + it('always updates secrets (value not visible in API)', async () => { const refs = new ResolvedRefs() refs.set('secrets', 'api-key', {id: 'sec-1', refKey: 'api-key', raw: {key: 'api-key'}}) const config: DevhelmConfig = { secrets: [{key: 'api-key', value: 'same-or-different'}], } - const changeset = diff(config, refs) + const changeset = await diff(config, refs) expect(changeset.updates).toHaveLength(1) }) - it('skips update when dependency unchanged', () => { + it('skips update when dependency unchanged', async () => { const refs = new ResolvedRefs() refs.set('dependencies', 'github', {id: 'dep-1', refKey: 'github', raw: { slug: 'github', alertSensitivity: 'INCIDENTS_ONLY', @@ -165,11 +165,11 @@ describe('differ', () => { const config: DevhelmConfig = { dependencies: [{service: 'github', alertSensitivity: 'INCIDENTS_ONLY'}], } - const changeset = diff(config, refs) + const changeset = await diff(config, refs) expect(changeset.updates).toHaveLength(0) }) - it('detects update when dependency alertSensitivity changed', () => { + it('detects update when dependency alertSensitivity changed', async () => { const refs = new ResolvedRefs() refs.set('dependencies', 'github', {id: 'dep-1', refKey: 'github', raw: { slug: 'github', alertSensitivity: 'INCIDENTS_ONLY', @@ -177,11 +177,11 @@ describe('differ', () => { const config: DevhelmConfig = { dependencies: [{service: 'github', alertSensitivity: 'ALL'}], } - const changeset = diff(config, refs) + const changeset = await diff(config, refs) expect(changeset.updates).toHaveLength(1) }) - it('skips update when resource group unchanged', () => { + it('skips update when resource group unchanged', async () => { const refs = new ResolvedRefs() refs.set('resourceGroups', 'API', {id: 'rg-1', refKey: 'API', raw: { name: 'API', description: 'API services', defaultFrequency: 30, @@ -189,117 +189,117 @@ describe('differ', () => { const config: DevhelmConfig = { resourceGroups: [{name: 'API', description: 'API services', defaultFrequency: 30}], } - const changeset = diff(config, refs) + const changeset = await diff(config, refs) expect(changeset.updates).toHaveLength(0) }) - it('detects no changes for empty config sections', () => { + it('detects no changes for empty config sections', async () => { const config: DevhelmConfig = {} - const changeset = diff(config, emptyRefs()) + const changeset = await diff(config, emptyRefs()) expect(changeset.creates).toHaveLength(0) expect(changeset.updates).toHaveLength(0) expect(changeset.deletes).toHaveLength(0) }) - it('detects deletes with prune=true', () => { + it('detects deletes with prune=true', async () => { const refs = new ResolvedRefs() refs.set('monitors', 'old-monitor', { id: 'mon-1', refKey: 'old-monitor', managedBy: 'CLI', raw: {managedBy: 'CLI'}, }) const config: DevhelmConfig = {monitors: []} - const changeset = diff(config, refs, {prune: true}) + const changeset = await diff(config, refs, {prune: true}) expect(changeset.deletes).toHaveLength(1) expect(changeset.deletes[0].refKey).toBe('old-monitor') }) - it('skips non-CLI monitors during prune', () => { + it('skips non-CLI monitors during prune', async () => { const refs = new ResolvedRefs() refs.set('monitors', 'dashboard-monitor', { id: 'mon-1', refKey: 'dashboard-monitor', managedBy: 'DASHBOARD', raw: {managedBy: 'DASHBOARD'}, }) const config: DevhelmConfig = {monitors: []} - const changeset = diff(config, refs, {prune: true}) + const changeset = await diff(config, refs, {prune: true}) expect(changeset.deletes).toHaveLength(0) }) - it('pruneAll deletes non-CLI monitors', () => { + it('pruneAll deletes non-CLI monitors', async () => { const refs = new ResolvedRefs() refs.set('monitors', 'dashboard-monitor', { id: 'mon-1', refKey: 'dashboard-monitor', managedBy: 'DASHBOARD', raw: {managedBy: 'DASHBOARD'}, }) const config: DevhelmConfig = {monitors: []} - const changeset = diff(config, refs, {prune: true, pruneAll: true}) + const changeset = await diff(config, refs, {prune: true, pruneAll: true}) expect(changeset.deletes).toHaveLength(1) expect(changeset.deletes[0].refKey).toBe('dashboard-monitor') }) - it('pruneAll deletes monitors with no managedBy', () => { + it('pruneAll deletes monitors with no managedBy', async () => { const refs = new ResolvedRefs() refs.set('monitors', 'orphan', { id: 'mon-1', refKey: 'orphan', raw: {}, }) const config: DevhelmConfig = {monitors: []} - const changeset = diff(config, refs, {prune: true, pruneAll: true}) + const changeset = await diff(config, refs, {prune: true, pruneAll: true}) expect(changeset.deletes).toHaveLength(1) }) - it('prune: omitted section (undefined) does not delete', () => { + it('prune: omitted section (undefined) does not delete', async () => { const refs = new ResolvedRefs() refs.set('tags', 'T', {id: '1', refKey: 'T', raw: {name: 'T'}}) const config: DevhelmConfig = {} - const changeset = diff(config, refs, {prune: true}) + const changeset = await diff(config, refs, {prune: true}) expect(changeset.deletes).toHaveLength(0) }) - it('prune: empty array deletes all existing', () => { + it('prune: empty array deletes all existing', async () => { const refs = new ResolvedRefs() refs.set('tags', 'T', {id: '1', refKey: 'T', raw: {name: 'T'}}) const config: DevhelmConfig = {tags: []} - const changeset = diff(config, refs, {prune: true}) + const changeset = await diff(config, refs, {prune: true}) expect(changeset.deletes).toHaveLength(1) }) - it('does not delete without prune flag', () => { + it('does not delete without prune flag', async () => { const refs = new ResolvedRefs() refs.set('monitors', 'old', {id: 'mon-1', refKey: 'old', managedBy: 'CLI', raw: {}}) const config: DevhelmConfig = {monitors: []} - const changeset = diff(config, refs, {prune: false}) + const changeset = await diff(config, refs, {prune: false}) expect(changeset.deletes).toHaveLength(0) }) - it('creates in dependency order', () => { + it('creates in dependency order', async () => { const config: DevhelmConfig = { monitors: [{name: 'M', type: 'HTTP', config: {url: 'https://x.com', method: 'GET'}}], tags: [{name: 'T'}], alertChannels: [{name: 'C', config: {channelType: 'slack', webhookUrl: 'url'}}], } - const changeset = diff(config, emptyRefs()) + const changeset = await diff(config, emptyRefs()) const types = changeset.creates.map((c) => c.resourceType) expect(types.indexOf('tag')).toBeLessThan(types.indexOf('alertChannel')) expect(types.indexOf('alertChannel')).toBeLessThan(types.indexOf('monitor')) }) - it('deletes in reverse dependency order', () => { + it('deletes in reverse dependency order', async () => { const refs = new ResolvedRefs() refs.set('tags', 'T', {id: '1', refKey: 'T', raw: {}}) refs.set('monitors', 'M', {id: '2', refKey: 'M', managedBy: 'CLI', raw: {}}) const config: DevhelmConfig = {tags: [], monitors: []} - const changeset = diff(config, refs, {prune: true}) + const changeset = await diff(config, refs, {prune: true}) const types = changeset.deletes.map((c) => c.resourceType) expect(types.indexOf('monitor')).toBeLessThan(types.indexOf('tag')) }) - it('detects group memberships', () => { + it('detects group memberships', async () => { const config: DevhelmConfig = { monitors: [{name: 'M', type: 'HTTP', config: {url: 'https://x.com', method: 'GET'}}], resourceGroups: [{name: 'G', monitors: ['M']}], } - const changeset = diff(config, emptyRefs()) + const changeset = await diff(config, emptyRefs()) expect(changeset.memberships).toHaveLength(1) expect(changeset.memberships[0].refKey).toBe('G → M') }) - it('handles all resource types', () => { + it('handles all resource types', async () => { const config: DevhelmConfig = { tags: [{name: 'T'}], environments: [{name: 'E', slug: 'e'}], @@ -317,13 +317,13 @@ describe('differ', () => { components: [{name: 'API', type: 'STATIC'}], }], } - const changeset = diff(config, emptyRefs()) + const changeset = await diff(config, emptyRefs()) expect(changeset.creates).toHaveLength(10) const types = changeset.creates.map((c) => c.resourceType) expect(types).toContain('statusPage') }) - it('detects update when status page name changes', () => { + it('detects update when status page name changes', async () => { const refs = new ResolvedRefs() refs.set('statusPages', 'public', {id: 'sp-1', refKey: 'public', raw: { id: 'sp-1', name: 'Old Name', slug: 'public', @@ -339,7 +339,7 @@ describe('differ', () => { components: [], }], } - const changeset = diff(config, refs) + const changeset = await diff(config, refs) expect(changeset.updates).toHaveLength(1) expect(changeset.updates[0].refKey).toBe('public') }) @@ -350,7 +350,7 @@ describe('differ', () => { // handler.toDesiredSnapshot / toCurrentSnapshot under the hood, so a // diff() round-trip is the cheapest way to assert end-to-end behavior. - it('detects update when status page branding fields change', () => { + it('detects update when status page branding fields change', async () => { const refs = new ResolvedRefs() refs.set('statusPages', 'sp', {id: 'sp-1', refKey: 'sp', raw: { id: 'sp-1', name: 'P', slug: 'sp', @@ -363,12 +363,12 @@ describe('differ', () => { branding: {brandColor: '#FF0000', theme: 'dark'}, }], } - const changeset = diff(config, refs) + const changeset = await diff(config, refs) expect(changeset.updates).toHaveLength(1) expect(changeset.updates[0].refKey).toBe('sp') }) - it('does NOT update status page when YAML omits branding (preserve current)', () => { + it('does NOT update status page when YAML omits branding (preserve current)', async () => { const refs = new ResolvedRefs() refs.set('statusPages', 'sp', {id: 'sp-1', refKey: 'sp', raw: { id: 'sp-1', name: 'P', slug: 'sp', @@ -379,11 +379,11 @@ describe('differ', () => { const config: DevhelmConfig = { statusPages: [{name: 'P', slug: 'sp'}], } - const changeset = diff(config, refs) + const changeset = await diff(config, refs) expect(changeset.updates).toHaveLength(0) }) - it('detects update when YAML branding shrinks vs API (hidePoweredBy default)', () => { + it('detects update when YAML branding shrinks vs API (hidePoweredBy default)', async () => { // YAML has only brandColor; toBrandingRequest fills the rest with // null/false defaults, so diff against an API with theme=dark must fire. const refs = new ResolvedRefs() @@ -398,31 +398,49 @@ describe('differ', () => { branding: {brandColor: '#FF0000'}, }], } - const changeset = diff(config, refs) + const changeset = await diff(config, refs) expect(changeset.updates).toHaveLength(1) }) - // ── Child-collection drift detection (was: known LIMITATION; now FIXED) ── + // ── Child-collection drift detection (RFC 0001 / Issue C) ───────────── // // statusPageHandler.hasChildChanges compares yaml.components / - // componentGroups against priorState child snapshots so a YAML edit that - // only touches a child attribute (or adds/removes a child) surfaces as a - // page-level update in `devhelm plan` — even if the page's own fields - // are byte-identical. + // componentGroups against the **live API** child snapshots that the + // differ pre-fetches via `prefetchChildSnapshots`. State is no longer + // consulted as a diff baseline — the live API is the source of truth. // - // The conservative no-prior-state path: when the page exists in API but - // priorChildren is empty (fresh state, post-import, etc.) the differ - // queues an update so reconcileStatusPageChildren runs and inspects the - // live children for itself. Prevents silent under-reporting on imports. + // In production, plan/deploy call `prefetchChildSnapshots(config, refs, + // client)` and pass the result as the 5th `diff()` arg. Tests inject + // the same `ChildSnapshotMap` directly to avoid spinning up an HTTP + // mock; the snapshot shape mirrors `statusPageComponentCurrentSnapshot` + // (name, description, type, showUptime, excludeFromOverall, startDate, + // group, monitor, resourceGroup). - it('FIXED: component added in YAML is detected as page-level update', () => { + function makeStatusPageRefs() { const refs = new ResolvedRefs() refs.set('statusPages', 'sp', {id: 'sp-1', refKey: 'sp', raw: { id: 'sp-1', name: 'P', slug: 'sp', branding: {}, componentGroups: [], - components: [{id: 'c-1', name: 'API', type: 'STATIC', showUptime: true}], + components: [], }}) + return refs + } + + function makeChildren(snapshot: Record): Map}>> { + return new Map([['statusPages.sp', { + 'components.API': {apiId: 'c-1', snapshot}, + }]]) + } + + const baseApiComponent = { + name: 'API', description: null, type: 'STATIC', showUptime: true, + excludeFromOverall: false, startDate: null, group: null, + monitor: null, resourceGroup: null, + } + + it('FIXED: component added in YAML is detected as page-level update', async () => { + const refs = makeStatusPageRefs() const config: DevhelmConfig = { statusPages: [{ name: 'P', slug: 'sp', @@ -432,143 +450,65 @@ describe('differ', () => { ], }], } - const priorState = { - version: '2' as const, serial: 1, lastDeployedAt: '2024-01-01T00:00:00Z', - resources: { - 'statusPages.sp': { - apiId: 'sp-1', resourceType: 'statusPage' as const, - attributes: {name: 'P'}, - children: { - 'components.API': {apiId: 'c-1', attributes: { - name: 'API', description: null, type: 'STATIC', showUptime: true, - excludeFromOverall: false, startDate: null, group: null, - monitor: null, resourceGroup: null, - }}, - }, - }, - }, - } - const changeset = diff(config, refs, {}, priorState) + const currentChildren = makeChildren(baseApiComponent) + const changeset = await diff(config, refs, {}, undefined, currentChildren) expect(changeset.updates).toHaveLength(1) }) - it('FIXED: component excludeFromOverall change is detected via prior state', () => { - const refs = new ResolvedRefs() - refs.set('statusPages', 'sp', {id: 'sp-1', refKey: 'sp', raw: { - id: 'sp-1', name: 'P', slug: 'sp', - branding: {}, - componentGroups: [], - components: [{id: 'c-1', name: 'API', type: 'STATIC', excludeFromOverall: false, showUptime: true}], - }}) + it('FIXED: component excludeFromOverall change is detected via live API snapshot', async () => { + const refs = makeStatusPageRefs() const config: DevhelmConfig = { statusPages: [{ name: 'P', slug: 'sp', components: [{name: 'API', type: 'STATIC', excludeFromOverall: true}], }], } - const priorState = { - version: '2' as const, serial: 1, lastDeployedAt: '2024-01-01T00:00:00Z', - resources: { - 'statusPages.sp': { - apiId: 'sp-1', resourceType: 'statusPage' as const, - attributes: {name: 'P'}, - children: { - 'components.API': {apiId: 'c-1', attributes: { - name: 'API', description: null, type: 'STATIC', showUptime: true, - excludeFromOverall: false, startDate: null, group: null, - monitor: null, resourceGroup: null, - }}, - }, - }, - }, - } - const changeset = diff(config, refs, {}, priorState) + const currentChildren = makeChildren({...baseApiComponent, excludeFromOverall: false}) + const changeset = await diff(config, refs, {}, undefined, currentChildren) expect(changeset.updates).toHaveLength(1) }) - it('FIXED: component startDate change is detected via prior state', () => { - const refs = new ResolvedRefs() - refs.set('statusPages', 'sp', {id: 'sp-1', refKey: 'sp', raw: { - id: 'sp-1', name: 'P', slug: 'sp', - branding: {}, - componentGroups: [], - components: [{id: 'c-1', name: 'API', type: 'STATIC', startDate: '2024-01-01T00:00:00Z', showUptime: true}], - }}) + it('FIXED: component startDate change is detected via live API snapshot', async () => { + const refs = makeStatusPageRefs() const config: DevhelmConfig = { statusPages: [{ name: 'P', slug: 'sp', components: [{name: 'API', type: 'STATIC', startDate: '2024-06-01'}], }], } - const priorState = { - version: '2' as const, serial: 1, lastDeployedAt: '2024-01-01T00:00:00Z', - resources: { - 'statusPages.sp': { - apiId: 'sp-1', resourceType: 'statusPage' as const, - attributes: {name: 'P'}, - children: { - 'components.API': {apiId: 'c-1', attributes: { - name: 'API', description: null, type: 'STATIC', showUptime: true, - excludeFromOverall: false, startDate: '2024-01-01', group: null, - monitor: null, resourceGroup: null, - }}, - }, - }, - }, - } - const changeset = diff(config, refs, {}, priorState) + const currentChildren = makeChildren({...baseApiComponent, startDate: '2024-01-01'}) + const changeset = await diff(config, refs, {}, undefined, currentChildren) expect(changeset.updates).toHaveLength(1) }) - it('child-collection diff: clean prior state → no spurious update', () => { - const refs = new ResolvedRefs() - refs.set('statusPages', 'sp', {id: 'sp-1', refKey: 'sp', raw: { - id: 'sp-1', name: 'P', slug: 'sp', - branding: {}, - componentGroups: [], - components: [{id: 'c-1', name: 'API', type: 'STATIC', showUptime: true}], - }}) + it('child-collection diff: live API snapshot matches YAML → no spurious update', async () => { + const refs = makeStatusPageRefs() const config: DevhelmConfig = { statusPages: [{ name: 'P', slug: 'sp', components: [{name: 'API', type: 'STATIC'}], }], } - const priorState = { - version: '2' as const, serial: 1, lastDeployedAt: '2024-01-01T00:00:00Z', - resources: { - 'statusPages.sp': { - apiId: 'sp-1', resourceType: 'statusPage' as const, - attributes: {name: 'P'}, - children: { - 'components.API': {apiId: 'c-1', attributes: { - name: 'API', description: null, type: 'STATIC', showUptime: true, - excludeFromOverall: false, startDate: null, group: null, - monitor: null, resourceGroup: null, - }}, - }, - }, - }, - } - const changeset = diff(config, refs, {}, priorState) + const currentChildren = makeChildren(baseApiComponent) + const changeset = await diff(config, refs, {}, undefined, currentChildren) expect(changeset.updates).toHaveLength(0) }) - it('status page appears in handles all resource types in create ordering', () => { + it('status page appears in handles all resource types in create ordering', async () => { const config: DevhelmConfig = { statusPages: [{ name: 'SP', slug: 'sp', branding: {}, components: [], }], monitors: [{name: 'M', type: 'HTTP', config: {url: 'https://x.com', method: 'GET'}}], } - const changeset = diff(config, emptyRefs()) + const changeset = await diff(config, emptyRefs()) const types = changeset.creates.map((c) => c.resourceType) // Status pages reference monitors through components → monitors must be // created before status pages in the default ordering. expect(types.indexOf('monitor')).toBeLessThan(types.indexOf('statusPage')) }) - it('monitor regions order does not matter', () => { + it('monitor regions order does not matter', async () => { const refs = new ResolvedRefs() refs.set('monitors', 'API', {id: 'mon-1', refKey: 'API', raw: { name: 'API', type: 'HTTP', regions: ['eu-west', 'us-east'], @@ -580,11 +520,11 @@ describe('differ', () => { config: {url: 'https://api.com', method: 'GET'}, }], } - const changeset = diff(config, refs) + const changeset = await diff(config, refs) expect(changeset.updates).toHaveLength(0) }) - it('webhook events order does not matter', () => { + it('webhook events order does not matter', async () => { const refs = new ResolvedRefs() refs.set('webhooks', 'https://x.com', {id: 'wh-1', refKey: 'https://x.com', raw: { url: 'https://x.com', subscribedEvents: ['b', 'a'], enabled: true, @@ -592,11 +532,11 @@ describe('differ', () => { const config: DevhelmConfig = { webhooks: [{url: 'https://x.com', subscribedEvents: ['a', 'b']}], } - const changeset = diff(config, refs) + const changeset = await diff(config, refs) expect(changeset.updates).toHaveLength(0) }) - it('always updates alert channels (discriminated union config)', () => { + it('always updates alert channels (discriminated union config)', async () => { const refs = new ResolvedRefs() refs.set('alertChannels', 'slack-ops', {id: 'ch-1', refKey: 'slack-ops', raw: { id: 'ch-1', name: 'slack-ops', channelType: 'slack', @@ -605,11 +545,11 @@ describe('differ', () => { const config: DevhelmConfig = { alertChannels: [{name: 'slack-ops', config: {channelType: 'slack', webhookUrl: 'https://hooks.slack.com/test'}}], } - const changeset = diff(config, refs) + const changeset = await diff(config, refs) expect(changeset.updates).toHaveLength(1) }) - it('detects update when notification policy escalation changes', () => { + it('detects update when notification policy escalation changes', async () => { const refs = new ResolvedRefs() refs.set('alertChannels', 'ch', {id: 'ch-1', refKey: 'ch', raw: {name: 'ch'}}) refs.set('notificationPolicies', 'critical', {id: 'np-1', refKey: 'critical', raw: { @@ -619,11 +559,11 @@ describe('differ', () => { const config: DevhelmConfig = { notificationPolicies: [{name: 'critical', enabled: true, priority: 0, escalation: {steps: [{channels: ['ch']}]}}], } - const changeset = diff(config, refs) + const changeset = await diff(config, refs) expect(changeset.updates).toHaveLength(1) }) - it('mixed: only changed resources appear as updates', () => { + it('mixed: only changed resources appear as updates', async () => { const refs = new ResolvedRefs() refs.set('tags', 'unchanged', {id: 'tag-1', refKey: 'unchanged', raw: {name: 'unchanged', color: '#FF0000'}}) refs.set('tags', 'changed', {id: 'tag-2', refKey: 'changed', raw: {name: 'changed', color: '#000000'}}) @@ -638,14 +578,14 @@ describe('differ', () => { ], webhooks: [{url: 'https://same.com', subscribedEvents: ['a'], description: 'same'}], } - const changeset = diff(config, refs) + const changeset = await diff(config, refs) expect(changeset.creates).toHaveLength(1) expect(changeset.creates[0].refKey).toBe('brand-new') expect(changeset.updates).toHaveLength(1) expect(changeset.updates[0].refKey).toBe('changed') }) - it('detects update when monitor enabled toggled', () => { + it('detects update when monitor enabled toggled', async () => { const refs = new ResolvedRefs() refs.set('monitors', 'M', {id: 'mon-1', refKey: 'M', raw: { name: 'M', type: 'HTTP', enabled: true, @@ -657,11 +597,11 @@ describe('differ', () => { config: {url: 'https://x.com', method: 'GET'}, }], } - const changeset = diff(config, refs) + const changeset = await diff(config, refs) expect(changeset.updates).toHaveLength(1) }) - it('detects update when environment variables change', () => { + it('detects update when environment variables change', async () => { const refs = new ResolvedRefs() refs.set('environments', 'prod', {id: 'env-1', refKey: 'prod', raw: { name: 'Prod', slug: 'prod', variables: {A: '1'}, @@ -669,11 +609,11 @@ describe('differ', () => { const config: DevhelmConfig = { environments: [{name: 'Prod', slug: 'prod', variables: {A: '1', B: '2'}}], } - const changeset = diff(config, refs) + const changeset = await diff(config, refs) expect(changeset.updates).toHaveLength(1) }) - it('skips update when environment variables same', () => { + it('skips update when environment variables same', async () => { const refs = new ResolvedRefs() refs.set('environments', 'prod', {id: 'env-1', refKey: 'prod', raw: { name: 'Prod', slug: 'prod', variables: {A: '1', B: '2'}, @@ -681,11 +621,11 @@ describe('differ', () => { const config: DevhelmConfig = { environments: [{name: 'Prod', slug: 'prod', variables: {B: '2', A: '1'}}], } - const changeset = diff(config, refs) + const changeset = await diff(config, refs) expect(changeset.updates).toHaveLength(0) }) - it('detects update when resource group health threshold changes', () => { + it('detects update when resource group health threshold changes', async () => { const refs = new ResolvedRefs() refs.set('resourceGroups', 'G', {id: 'rg-1', refKey: 'G', raw: { name: 'G', healthThresholdType: 'PERCENTAGE', healthThresholdValue: 80, @@ -693,11 +633,11 @@ describe('differ', () => { const config: DevhelmConfig = { resourceGroups: [{name: 'G', healthThresholdType: 'PERCENTAGE', healthThresholdValue: 50}], } - const changeset = diff(config, refs) + const changeset = await diff(config, refs) expect(changeset.updates).toHaveLength(1) }) - it('detects update when resource group suppressMemberAlerts changes', () => { + it('detects update when resource group suppressMemberAlerts changes', async () => { const refs = new ResolvedRefs() refs.set('resourceGroups', 'G', {id: 'rg-1', refKey: 'G', raw: { name: 'G', suppressMemberAlerts: false, @@ -705,21 +645,21 @@ describe('differ', () => { const config: DevhelmConfig = { resourceGroups: [{name: 'G', suppressMemberAlerts: true}], } - const changeset = diff(config, refs) + const changeset = await diff(config, refs) expect(changeset.updates).toHaveLength(1) }) - it('tag without color ignores API color (undefined not compared)', () => { + it('tag without color ignores API color (undefined not compared)', async () => { const refs = new ResolvedRefs() refs.set('tags', 'T', {id: 'tag-1', refKey: 'T', raw: {name: 'T', color: '#FF0000'}}) const config: DevhelmConfig = { tags: [{name: 'T'}], } - const changeset = diff(config, refs) + const changeset = await diff(config, refs) expect(changeset.updates).toHaveLength(0) }) - it('config key ordering does not trigger false update', () => { + it('config key ordering does not trigger false update', async () => { const refs = new ResolvedRefs() refs.set('monitors', 'M', {id: 'mon-1', refKey: 'M', raw: { name: 'M', type: 'HTTP', @@ -731,11 +671,11 @@ describe('differ', () => { config: {url: 'https://api.com', method: 'GET', headers: {b: '2', a: '1'}}, }], } - const changeset = diff(config, refs) + const changeset = await diff(config, refs) expect(changeset.updates).toHaveLength(0) }) - it('monitor without frequency set ignores API frequencySeconds', () => { + it('monitor without frequency set ignores API frequencySeconds', async () => { const refs = new ResolvedRefs() refs.set('monitors', 'M', {id: 'mon-1', refKey: 'M', raw: { name: 'M', type: 'HTTP', frequencySeconds: 60, @@ -744,11 +684,11 @@ describe('differ', () => { const config: DevhelmConfig = { monitors: [{name: 'M', type: 'HTTP', config: {url: 'https://x.com', method: 'GET'}}], } - const changeset = diff(config, refs) + const changeset = await diff(config, refs) expect(changeset.updates).toHaveLength(0) }) - it('monitor without regions set ignores API regions', () => { + it('monitor without regions set ignores API regions', async () => { const refs = new ResolvedRefs() refs.set('monitors', 'M', {id: 'mon-1', refKey: 'M', raw: { name: 'M', type: 'HTTP', regions: ['us-east', 'eu-west'], @@ -757,13 +697,13 @@ describe('differ', () => { const config: DevhelmConfig = { monitors: [{name: 'M', type: 'HTTP', config: {url: 'https://x.com', method: 'GET'}}], } - const changeset = diff(config, refs) + const changeset = await diff(config, refs) expect(changeset.updates).toHaveLength(0) }) // ── Monitor tags: YAML uses names, API returns TagDto[] ────────── - it('detects update when monitor tags change', () => { + it('detects update when monitor tags change', async () => { const refs = new ResolvedRefs() refs.set('tags', 'a', {id: 'tag-a', refKey: 'a', raw: {id: 'tag-a', name: 'a'}}) refs.set('monitors', 'M', {id: 'mon-1', refKey: 'M', raw: { @@ -777,11 +717,11 @@ describe('differ', () => { config: {url: 'https://x.com', method: 'GET'}, }], } - const changeset = diff(config, refs) + const changeset = await diff(config, refs) expect(changeset.updates).toHaveLength(1) }) - it('skips update when monitor tags unchanged', () => { + it('skips update when monitor tags unchanged', async () => { const refs = new ResolvedRefs() refs.set('tags', 'a', {id: 'tag-a', refKey: 'a', raw: {id: 'tag-a', name: 'a'}}) refs.set('tags', 'b', {id: 'tag-b', refKey: 'b', raw: {id: 'tag-b', name: 'b'}}) @@ -796,11 +736,11 @@ describe('differ', () => { config: {url: 'https://x.com', method: 'GET'}, }], } - const changeset = diff(config, refs) + const changeset = await diff(config, refs) expect(changeset.updates).toHaveLength(0) }) - it('monitor without tags set ignores API tags', () => { + it('monitor without tags set ignores API tags', async () => { const refs = new ResolvedRefs() refs.set('monitors', 'M', {id: 'mon-1', refKey: 'M', raw: { name: 'M', type: 'HTTP', @@ -810,13 +750,13 @@ describe('differ', () => { const config: DevhelmConfig = { monitors: [{name: 'M', type: 'HTTP', config: {url: 'https://x.com', method: 'GET'}}], } - const changeset = diff(config, refs) + const changeset = await diff(config, refs) expect(changeset.updates).toHaveLength(0) }) // ── Monitor alertChannels: YAML names → API alertChannelIds (UUIDs) ── - it('detects update when monitor alertChannels change', () => { + it('detects update when monitor alertChannels change', async () => { const refs = new ResolvedRefs() refs.set('alertChannels', 'ch1', {id: 'ch-uuid-1', refKey: 'ch1', raw: {id: 'ch-uuid-1', name: 'ch1'}}) refs.set('monitors', 'M', {id: 'mon-1', refKey: 'M', raw: { @@ -829,11 +769,11 @@ describe('differ', () => { config: {url: 'https://x.com', method: 'GET'}, }], } - const changeset = diff(config, refs) + const changeset = await diff(config, refs) expect(changeset.updates).toHaveLength(1) }) - it('skips update when monitor alertChannels unchanged', () => { + it('skips update when monitor alertChannels unchanged', async () => { const refs = new ResolvedRefs() refs.set('alertChannels', 'ch1', {id: 'ch-uuid-1', refKey: 'ch1', raw: {}}) refs.set('alertChannels', 'ch2', {id: 'ch-uuid-2', refKey: 'ch2', raw: {}}) @@ -847,13 +787,13 @@ describe('differ', () => { config: {url: 'https://x.com', method: 'GET'}, }], } - const changeset = diff(config, refs) + const changeset = await diff(config, refs) expect(changeset.updates).toHaveLength(0) }) // ── Monitor environment: YAML slug → API Summary { id, name, slug } ── - it('detects update when monitor environment changes', () => { + it('detects update when monitor environment changes', async () => { const refs = new ResolvedRefs() refs.set('environments', 'prod', {id: 'env-prod', refKey: 'prod', raw: {}}) refs.set('environments', 'staging', {id: 'env-stg', refKey: 'staging', raw: {}}) @@ -868,11 +808,11 @@ describe('differ', () => { config: {url: 'https://x.com', method: 'GET'}, }], } - const changeset = diff(config, refs) + const changeset = await diff(config, refs) expect(changeset.updates).toHaveLength(1) }) - it('skips update when monitor environment unchanged', () => { + it('skips update when monitor environment unchanged', async () => { const refs = new ResolvedRefs() refs.set('environments', 'prod', {id: 'env-prod', refKey: 'prod', raw: {}}) refs.set('monitors', 'M', {id: 'mon-1', refKey: 'M', raw: { @@ -886,13 +826,13 @@ describe('differ', () => { config: {url: 'https://x.com', method: 'GET'}, }], } - const changeset = diff(config, refs) + const changeset = await diff(config, refs) expect(changeset.updates).toHaveLength(0) }) // ── Monitor auth: YAML uses secret name, API uses MonitorAuthDto ── - it('detects update when monitor auth type changes', () => { + it('detects update when monitor auth type changes', async () => { const refs = new ResolvedRefs() refs.set('secrets', 'creds', {id: 'sec-1', refKey: 'creds', raw: {}}) refs.set('monitors', 'M', {id: 'mon-1', refKey: 'M', raw: { @@ -907,11 +847,11 @@ describe('differ', () => { config: {url: 'https://x.com', method: 'GET'}, }], } - const changeset = diff(config, refs) + const changeset = await diff(config, refs) expect(changeset.updates).toHaveLength(1) }) - it('skips update when monitor auth unchanged', () => { + it('skips update when monitor auth unchanged', async () => { const refs = new ResolvedRefs() refs.set('secrets', 'token', {id: 'sec-1', refKey: 'token', raw: {}}) refs.set('monitors', 'M', {id: 'mon-1', refKey: 'M', raw: { @@ -926,13 +866,13 @@ describe('differ', () => { config: {url: 'https://x.com', method: 'GET'}, }], } - const changeset = diff(config, refs) + const changeset = await diff(config, refs) expect(changeset.updates).toHaveLength(0) }) // ── Monitor incidentPolicy ────────────────────────────────────── - it('detects update when monitor incidentPolicy changes', () => { + it('detects update when monitor incidentPolicy changes', async () => { const apiPolicy = { triggerRules: [{ type: 'consecutive_failures', count: 2, scope: 'per_region', severity: 'down', @@ -959,13 +899,13 @@ describe('differ', () => { config: {url: 'https://x.com', method: 'GET'}, }], } - const changeset = diff(config, refs) + const changeset = await diff(config, refs) expect(changeset.updates).toHaveLength(1) }) // ── Resource group: YAML names → API UUIDs ───────────────────── - it('detects update when resource group alertPolicy changes', () => { + it('detects update when resource group alertPolicy changes', async () => { const refs = new ResolvedRefs() refs.set('notificationPolicies', 'old', {id: 'np-old-uuid', refKey: 'old', raw: {}}) refs.set('notificationPolicies', 'new', {id: 'np-new-uuid', refKey: 'new', raw: {}}) @@ -975,11 +915,11 @@ describe('differ', () => { const config: DevhelmConfig = { resourceGroups: [{name: 'G', alertPolicy: 'new'}], } - const changeset = diff(config, refs) + const changeset = await diff(config, refs) expect(changeset.updates).toHaveLength(1) }) - it('detects update when resource group defaultRegions change', () => { + it('detects update when resource group defaultRegions change', async () => { const refs = new ResolvedRefs() refs.set('resourceGroups', 'G', {id: 'rg-1', refKey: 'G', raw: { name: 'G', defaultRegions: ['us-east'], @@ -987,11 +927,11 @@ describe('differ', () => { const config: DevhelmConfig = { resourceGroups: [{name: 'G', defaultRegions: ['us-east', 'eu-west']}], } - const changeset = diff(config, refs) + const changeset = await diff(config, refs) expect(changeset.updates).toHaveLength(1) }) - it('detects update when resource group defaultRetryStrategy changes', () => { + it('detects update when resource group defaultRetryStrategy changes', async () => { const refs = new ResolvedRefs() refs.set('resourceGroups', 'G', {id: 'rg-1', refKey: 'G', raw: { name: 'G', defaultRetryStrategy: {type: 'fixed', maxRetries: 2, interval: 5}, @@ -999,11 +939,11 @@ describe('differ', () => { const config: DevhelmConfig = { resourceGroups: [{name: 'G', defaultRetryStrategy: {type: 'fixed', maxRetries: 3, interval: 5}}], } - const changeset = diff(config, refs) + const changeset = await diff(config, refs) expect(changeset.updates).toHaveLength(1) }) - it('detects update when resource group confirmationDelaySeconds changes', () => { + it('detects update when resource group confirmationDelaySeconds changes', async () => { const refs = new ResolvedRefs() refs.set('resourceGroups', 'G', {id: 'rg-1', refKey: 'G', raw: { name: 'G', confirmationDelaySeconds: 30, @@ -1011,11 +951,11 @@ describe('differ', () => { const config: DevhelmConfig = { resourceGroups: [{name: 'G', confirmationDelaySeconds: 60}], } - const changeset = diff(config, refs) + const changeset = await diff(config, refs) expect(changeset.updates).toHaveLength(1) }) - it('detects update when resource group recoveryCooldownMinutes changes', () => { + it('detects update when resource group recoveryCooldownMinutes changes', async () => { const refs = new ResolvedRefs() refs.set('resourceGroups', 'G', {id: 'rg-1', refKey: 'G', raw: { name: 'G', recoveryCooldownMinutes: 10, @@ -1023,11 +963,11 @@ describe('differ', () => { const config: DevhelmConfig = { resourceGroups: [{name: 'G', recoveryCooldownMinutes: 20}], } - const changeset = diff(config, refs) + const changeset = await diff(config, refs) expect(changeset.updates).toHaveLength(1) }) - it('skips update when all resource group fields match (with resolved refs)', () => { + it('skips update when all resource group fields match (with resolved refs)', async () => { const refs = new ResolvedRefs() refs.set('notificationPolicies', 'critical', {id: 'np-uuid-1', refKey: 'critical', raw: {}}) refs.set('alertChannels', 'ch1', {id: 'ch-uuid-1', refKey: 'ch1', raw: {}}) @@ -1065,11 +1005,11 @@ describe('differ', () => { suppressMemberAlerts: true, }], } - const changeset = diff(config, refs) + const changeset = await diff(config, refs) expect(changeset.updates).toHaveLength(0) }) - it('detects update when webhook API has no subscribedEvents but YAML has events', () => { + it('detects update when webhook API has no subscribedEvents but YAML has events', async () => { const refs = new ResolvedRefs() refs.set('webhooks', 'https://hooks.com/nonevents', {id: 'wh-1', refKey: 'https://hooks.com/nonevents', raw: { url: 'https://hooks.com/nonevents', @@ -1077,11 +1017,11 @@ describe('differ', () => { const config: DevhelmConfig = { webhooks: [{url: 'https://hooks.com/nonevents', subscribedEvents: ['e']}], } - const changeset = diff(config, refs) + const changeset = await diff(config, refs) expect(changeset.updates).toHaveLength(1) }) - it('dependency without alertSensitivity is unchanged', () => { + it('dependency without alertSensitivity is unchanged', async () => { const refs = new ResolvedRefs() refs.set('dependencies', 'gh', {id: 'dep-1', refKey: 'gh', raw: { slug: 'gh', alertSensitivity: 'ALL', @@ -1089,13 +1029,13 @@ describe('differ', () => { const config: DevhelmConfig = { dependencies: [{service: 'gh'}], } - const changeset = diff(config, refs) + const changeset = await diff(config, refs) expect(changeset.updates).toHaveLength(0) }) // ── Dependency component: YAML uses componentId UUID, API has componentId ── - it('detects update when dependency component changes', () => { + it('detects update when dependency component changes', async () => { const refs = new ResolvedRefs() refs.set('dependencies', 'gh', {id: 'dep-1', refKey: 'gh', raw: { slug: 'gh', componentId: 'comp-api', @@ -1103,11 +1043,11 @@ describe('differ', () => { const config: DevhelmConfig = { dependencies: [{service: 'gh', component: 'comp-actions'}], } - const changeset = diff(config, refs) + const changeset = await diff(config, refs) expect(changeset.updates).toHaveLength(1) }) - it('skips update when dependency component unchanged', () => { + it('skips update when dependency component unchanged', async () => { const refs = new ResolvedRefs() refs.set('dependencies', 'gh', {id: 'dep-1', refKey: 'gh', raw: { slug: 'gh', componentId: 'comp-api', alertSensitivity: 'ALL', @@ -1115,11 +1055,11 @@ describe('differ', () => { const config: DevhelmConfig = { dependencies: [{service: 'gh', component: 'comp-api', alertSensitivity: 'ALL'}], } - const changeset = diff(config, refs) + const changeset = await diff(config, refs) expect(changeset.updates).toHaveLength(0) }) - it('dependency without component set ignores API componentId', () => { + it('dependency without component set ignores API componentId', async () => { const refs = new ResolvedRefs() refs.set('dependencies', 'gh', {id: 'dep-1', refKey: 'gh', raw: { slug: 'gh', componentId: 'comp-actions', @@ -1127,19 +1067,19 @@ describe('differ', () => { const config: DevhelmConfig = { dependencies: [{service: 'gh'}], } - const changeset = diff(config, refs) + const changeset = await diff(config, refs) expect(changeset.updates).toHaveLength(0) }) // ── attributeDiffs population ──────────────────────────────────── - it('populates attributeDiffs on updates (tag color change)', () => { + it('populates attributeDiffs on updates (tag color change)', async () => { const refs = new ResolvedRefs() refs.set('tags', 'production', {id: 'tag-1', refKey: 'production', raw: {name: 'production', color: '#000000'}}) const config: DevhelmConfig = { tags: [{name: 'production', color: '#EF4444'}], } - const changeset = diff(config, refs) + const changeset = await diff(config, refs) expect(changeset.updates).toHaveLength(1) const update = changeset.updates[0] expect(update.attributeDiffs).toBeDefined() @@ -1148,7 +1088,7 @@ describe('differ', () => { expect(colorDiff).toMatchObject({field: 'color', old: '#000000', new: '#EF4444'}) }) - it('populates attributeDiffs on updates (monitor frequency)', () => { + it('populates attributeDiffs on updates (monitor frequency)', async () => { const refs = new ResolvedRefs() refs.set('monitors', 'API', {id: 'mon-1', refKey: 'API', raw: { name: 'API', type: 'HTTP', frequencySeconds: 60, @@ -1160,7 +1100,7 @@ describe('differ', () => { config: {url: 'https://api.com', method: 'GET'}, }], } - const changeset = diff(config, refs) + const changeset = await diff(config, refs) expect(changeset.updates).toHaveLength(1) const update = changeset.updates[0] expect(update.attributeDiffs).toBeDefined() @@ -1170,11 +1110,11 @@ describe('differ', () => { expect(freqDiff!.new).toBe(30) }) - it('attributeDiffs is absent or empty for creates', () => { + it('attributeDiffs is absent or empty for creates', async () => { const config: DevhelmConfig = { tags: [{name: 'new', color: '#FF0000'}], } - const changeset = diff(config, emptyRefs()) + const changeset = await diff(config, emptyRefs()) expect(changeset.creates).toHaveLength(1) // Creates should not carry attributeDiffs (no "old" state to diff against) const create = changeset.creates[0] as unknown as {attributeDiffs?: unknown[]} @@ -1183,7 +1123,7 @@ describe('differ', () => { } }) - it('attributeDiffs is present but only for fields that changed', () => { + it('attributeDiffs is present but only for fields that changed', async () => { const refs = new ResolvedRefs() refs.set('monitors', 'M', {id: 'mon-1', refKey: 'M', raw: { name: 'M', type: 'HTTP', enabled: true, frequencySeconds: 60, @@ -1197,7 +1137,7 @@ describe('differ', () => { config: {url: 'https://api.com', method: 'GET'}, }], } - const changeset = diff(config, refs) + const changeset = await diff(config, refs) expect(changeset.updates).toHaveLength(1) const diffs = changeset.updates[0].attributeDiffs ?? [] const changedFields = diffs.map((d) => d.field) @@ -1209,12 +1149,12 @@ describe('differ', () => { }) describe('formatPlan', () => { - it('shows no changes message', () => { + it('shows no changes message', async () => { const result = formatPlan({creates: [], updates: [], deletes: [], memberships: []}) expect(result).toContain('No changes') }) - it('shows create/update/delete counts', () => { + it('shows create/update/delete counts', async () => { const changeset = { creates: [{action: 'create' as const, resourceType: 'monitor' as const, refKey: 'M'}], updates: [{action: 'update' as const, resourceType: 'tag' as const, refKey: 'T', existingId: '1'}], @@ -1230,7 +1170,7 @@ describe('differ', () => { expect(result).toContain('- tag "X"') }) - it('shows memberships', () => { + it('shows memberships', async () => { const changeset = { creates: [], updates: [], @@ -1241,7 +1181,7 @@ describe('differ', () => { expect(result).toContain('→ API → Health Check') }) - it('creates-only plan snapshot', () => { + it('creates-only plan snapshot', async () => { const changeset = { creates: [ {action: 'create' as const, resourceType: 'tag' as const, refKey: 'prod'}, @@ -1255,7 +1195,7 @@ describe('differ', () => { expect(result).toContain('2 to add, 0 to change, 0 to destroy') }) - it('deletes-only plan snapshot', () => { + it('deletes-only plan snapshot', async () => { const changeset = { creates: [], updates: [], @@ -1271,7 +1211,7 @@ describe('differ', () => { expect(result).toContain('0 to add, 0 to change, 2 to destroy') }) - it('mixed plan snapshot', () => { + it('mixed plan snapshot', async () => { const changeset = { creates: [{action: 'create' as const, resourceType: 'tag' as const, refKey: 'new-tag'}], updates: [{action: 'update' as const, resourceType: 'monitor' as const, refKey: 'API', existingId: 'm-1'}], @@ -1286,7 +1226,7 @@ describe('differ', () => { expect(result).toContain('1 to add, 1 to change, 1 to destroy') }) - it('shows attribute diffs for updates', () => { + it('shows attribute diffs for updates', async () => { const changeset = { creates: [], updates: [{ diff --git a/test/yaml/handlers.test.ts b/test/yaml/handlers.test.ts index ec843eb..8ef7296 100644 --- a/test/yaml/handlers.test.ts +++ b/test/yaml/handlers.test.ts @@ -6,9 +6,18 @@ * 3. fetchAll, getRefKey, getApiRefKey, getApiId, deletePath return expected values */ import {describe, it, expect} from 'vitest' -import {HANDLER_MAP, getHandler, allHandlers} from '../../src/lib/yaml/handlers.js' +import { + HANDLER_MAP, + getHandler, + allHandlers, + statusPageGroupDesiredSnapshot, + statusPageGroupCurrentSnapshot, + statusPageComponentDesiredSnapshot, + statusPageComponentCurrentSnapshot, +} from '../../src/lib/yaml/handlers.js' import type {HandledResourceType} from '../../src/lib/yaml/types.js' import {YAML_SECTION_KEYS} from '../../src/lib/yaml/schema.js' +import {ResolvedRefs} from '../../src/lib/yaml/resolver.js' const ALL_HANDLED_TYPES: HandledResourceType[] = [ 'tag', 'environment', 'secret', 'alertChannel', @@ -132,3 +141,104 @@ describe('handler deletePath', () => { expect(getHandler(type).deletePath('id-1', 'ref-1')).toBe(expectedPath) }) }) + +// ── Snapshot parity (regression) ──────────────────────────────────────── +// +// The YAML-side `*DesiredSnapshot` and the API-side `*CurrentSnapshot` +// MUST produce structurally identical objects when given equivalent +// inputs — otherwise `state pull` will write snapshots that don't match +// what the next deploy compares against, producing phantom diffs (or, as +// happened with `defaultOpen`, missing real drift entirely because the +// pulled snapshots only had `{name}` and the diff sub-object check +// triggered on EVERY field instead of just the changed ones). +// +// These tests guard against silent divergence: if anyone adds a field to +// one snapshot helper but not the other, this test fails first and loud. +describe('status-page child snapshot parity', () => { + it('group: desired and current snapshots have identical key sets', () => { + const yamlSnap = statusPageGroupDesiredSnapshot({name: 'Platform'}) + const apiSnap = statusPageGroupCurrentSnapshot({ + id: 'g-1', name: 'Platform', description: null, displayOrder: 0, defaultOpen: true, + }) + expect(Object.keys(apiSnap).sort()).toEqual(Object.keys(yamlSnap).sort()) + }) + + it('group: equivalent yaml + api inputs produce equal snapshots', () => { + const yamlSnap = statusPageGroupDesiredSnapshot({ + name: 'Platform', description: 'Core infra', defaultOpen: false, + }) + const apiSnap = statusPageGroupCurrentSnapshot({ + id: 'g-1', name: 'Platform', description: 'Core infra', + displayOrder: 0, defaultOpen: false, + }) + expect(apiSnap).toEqual(yamlSnap) + }) + + it('group: defaults align (defaultOpen=true, description=null)', () => { + const yamlSnap = statusPageGroupDesiredSnapshot({name: 'Bare'}) + const apiSnap = statusPageGroupCurrentSnapshot({ + id: 'g-1', name: 'Bare', displayOrder: 0, + // Both null and missing should normalize the same way: + description: null, defaultOpen: true, + }) + expect(apiSnap).toEqual(yamlSnap) + expect(yamlSnap.defaultOpen).toBe(true) + expect(yamlSnap.description).toBeNull() + }) + + it('component: desired and current snapshots have identical key sets', () => { + const yamlSnap = statusPageComponentDesiredSnapshot({ + name: 'API', type: 'MONITOR', monitor: 'api', + }) + const apiSnap = statusPageComponentCurrentSnapshot( + {id: 'c-1', name: 'API', type: 'MONITOR'}, + new Map(), + new ResolvedRefs(), + ) + expect(Object.keys(apiSnap).sort()).toEqual(Object.keys(yamlSnap).sort()) + }) + + it('component: equivalent inputs (with group/monitor refs) produce equal snapshots', () => { + const refs = new ResolvedRefs() + refs.set('monitors', 'api-monitor', {refKey: 'api-monitor', id: 'mon-uuid-1'}) + const groupNameToId = new Map([['Platform', 'g-uuid-1']]) + + const yamlSnap = statusPageComponentDesiredSnapshot({ + name: 'API', + type: 'MONITOR', + description: 'Public REST', + showUptime: true, + excludeFromOverall: false, + startDate: '2025-01-15', + group: 'Platform', + monitor: 'api-monitor', + }) + const apiSnap = statusPageComponentCurrentSnapshot( + { + id: 'c-1', + name: 'API', + type: 'MONITOR', + description: 'Public REST', + showUptime: true, + excludeFromOverall: false, + // API returns ISO timestamp at start-of-day UTC; helper slices to YYYY-MM-DD + startDate: '2025-01-15T00:00:00Z', + groupId: 'g-uuid-1', + monitorId: 'mon-uuid-1', + }, + groupNameToId, + refs, + ) + expect(apiSnap).toEqual(yamlSnap) + }) + + it('component: unknown groupId/monitorId reverse-resolve to null (drift surfaces)', () => { + const apiSnap = statusPageComponentCurrentSnapshot( + {id: 'c-1', name: 'API', type: 'MONITOR', groupId: 'unknown-group', monitorId: 'unknown-mon'}, + new Map(), + new ResolvedRefs(), + ) + expect(apiSnap.group).toBeNull() + expect(apiSnap.monitor).toBeNull() + }) +}) diff --git a/test/yaml/idempotency.test.ts b/test/yaml/idempotency.test.ts index 03e8cd8..891f792 100644 --- a/test/yaml/idempotency.test.ts +++ b/test/yaml/idempotency.test.ts @@ -13,7 +13,7 @@ function buildRefs(entries: Array<{type: Parameters[0]; key } describe('idempotency', () => { - it('same YAML + same API state for tags → zero changes', () => { + it('same YAML + same API state for tags → zero changes', async () => { const config: DevhelmConfig = { tags: [{name: 'prod', color: '#EF4444'}, {name: 'staging', color: '#3B82F6'}], } @@ -21,25 +21,25 @@ describe('idempotency', () => { {type: 'tags', key: 'prod', id: 't1', raw: {name: 'prod', color: '#EF4444'}}, {type: 'tags', key: 'staging', id: 't2', raw: {name: 'staging', color: '#3B82F6'}}, ]) - const cs = diff(config, refs) + const cs = await diff(config, refs) expect(cs.creates).toHaveLength(0) expect(cs.updates).toHaveLength(0) expect(cs.deletes).toHaveLength(0) }) - it('same YAML + same API state for environments → zero changes', () => { + it('same YAML + same API state for environments → zero changes', async () => { const config: DevhelmConfig = { environments: [{name: 'Production', slug: 'production', isDefault: true}], } const refs = buildRefs([ {type: 'environments', key: 'production', id: 'e1', raw: {name: 'Production', slug: 'production', isDefault: true}}, ]) - const cs = diff(config, refs) + const cs = await diff(config, refs) expect(cs.creates).toHaveLength(0) expect(cs.updates).toHaveLength(0) }) - it('same YAML + same API state for secrets (hash match) → zero changes', () => { + it('same YAML + same API state for secrets (hash match) → zero changes', async () => { const secretValue = 'super-secret-123' const config: DevhelmConfig = { secrets: [{key: 'API_KEY', value: secretValue}], @@ -47,25 +47,25 @@ describe('idempotency', () => { const refs = buildRefs([ {type: 'secrets', key: 'API_KEY', id: 's1', raw: {key: 'API_KEY', valueHash: sha256Hex(secretValue)}}, ]) - const cs = diff(config, refs) + const cs = await diff(config, refs) expect(cs.creates).toHaveLength(0) expect(cs.updates).toHaveLength(0) }) - it('changed secret value (different hash) → one update', () => { + it('changed secret value (different hash) → one update', async () => { const config: DevhelmConfig = { secrets: [{key: 'API_KEY', value: 'new-secret-456'}], } const refs = buildRefs([ {type: 'secrets', key: 'API_KEY', id: 's1', raw: {key: 'API_KEY', valueHash: sha256Hex('old-secret-123')}}, ]) - const cs = diff(config, refs) + const cs = await diff(config, refs) expect(cs.creates).toHaveLength(0) expect(cs.updates).toHaveLength(1) expect(cs.updates[0].refKey).toBe('API_KEY') }) - it('same YAML + same API state for alert channels (hash match) → zero changes', () => { + it('same YAML + same API state for alert channels (hash match) → zero changes', async () => { const channelConfig = {channelType: 'slack', webhookUrl: 'https://hooks.slack.com/test'} const configHash = sha256Hex(stableStringify(channelConfig)) @@ -80,12 +80,12 @@ describe('idempotency', () => { name: 'Slack Alerts', channelType: 'slack', configHash, }}, ]) - const cs = diff(config, refs) + const cs = await diff(config, refs) expect(cs.creates).toHaveLength(0) expect(cs.updates).toHaveLength(0) }) - it('alert channel config change (different hash) → one update', () => { + it('alert channel config change (different hash) → one update', async () => { const oldConfig = {channelType: 'slack', webhookUrl: 'https://hooks.slack.com/old'} const oldHash = sha256Hex(stableStringify(oldConfig)) @@ -100,13 +100,13 @@ describe('idempotency', () => { name: 'Slack Alerts', channelType: 'slack', configHash: oldHash, }}, ]) - const cs = diff(config, refs) + const cs = await diff(config, refs) expect(cs.creates).toHaveLength(0) expect(cs.updates).toHaveLength(1) expect(cs.updates[0].refKey).toBe('Slack Alerts') }) - it('YAML tags: [] + API monitor with no tags → zero changes (regression)', () => { + it('YAML tags: [] + API monitor with no tags → zero changes (regression)', async () => { // apiTagsToSnapshot used to return {tagIds: null, newTags: []}, while // the desired snapshot built from `tags: []` produced {tagIds: [], ...}, // causing a perpetual update on every plan. @@ -125,12 +125,12 @@ describe('idempotency', () => { alertChannelIds: null, tags: null, }, managedBy: 'CLI'}, ]) - const cs = diff(config, refs) + const cs = await diff(config, refs) expect(cs.creates).toHaveLength(0) expect(cs.updates).toHaveLength(0) }) - it('adding one monitor to existing set → only that monitor in creates', () => { + it('adding one monitor to existing set → only that monitor in creates', async () => { const config: DevhelmConfig = { monitors: [ {name: 'API', type: 'HTTP', config: {url: 'https://api.example.com', method: 'GET'}}, @@ -145,13 +145,13 @@ describe('idempotency', () => { alertChannelIds: null, tagIds: null, }, managedBy: 'CLI'}, ]) - const cs = diff(config, refs) + const cs = await diff(config, refs) expect(cs.creates).toHaveLength(1) expect(cs.creates[0].refKey).toBe('Web') expect(cs.updates).toHaveLength(0) }) - it('removing one monitor → that monitor in deletes (with prune)', () => { + it('removing one monitor → that monitor in deletes (with prune)', async () => { const config: DevhelmConfig = { monitors: [ {name: 'API', type: 'HTTP', config: {url: 'https://api.example.com', method: 'GET'}}, @@ -169,13 +169,13 @@ describe('idempotency', () => { managedBy: 'CLI', }, managedBy: 'CLI'}, ]) - const cs = diff(config, refs, {prune: true}) + const cs = await diff(config, refs, {prune: true}) expect(cs.deletes).toHaveLength(1) expect(cs.deletes[0].refKey).toBe('Web') expect(cs.creates).toHaveLength(0) }) - it('same webhooks → zero changes', () => { + it('same webhooks → zero changes', async () => { const config: DevhelmConfig = { webhooks: [{url: 'https://example.com/webhook', subscribedEvents: ['monitor.down', 'monitor.up']}], } @@ -185,12 +185,12 @@ describe('idempotency', () => { description: null, enabled: true, }}, ]) - const cs = diff(config, refs) + const cs = await diff(config, refs) expect(cs.creates).toHaveLength(0) expect(cs.updates).toHaveLength(0) }) - it('same resource groups → zero changes', () => { + it('same resource groups → zero changes', async () => { const config: DevhelmConfig = { resourceGroups: [{name: 'Backend', description: 'Backend services'}], } @@ -204,12 +204,12 @@ describe('idempotency', () => { recoveryCooldownMinutes: null, }}, ]) - const cs = diff(config, refs) + const cs = await diff(config, refs) expect(cs.creates).toHaveLength(0) expect(cs.updates).toHaveLength(0) }) - it('same dependencies → zero changes', () => { + it('same dependencies → zero changes', async () => { const config: DevhelmConfig = { dependencies: [{service: 'aws-ec2', alertSensitivity: 'ALL'}], } @@ -218,12 +218,12 @@ describe('idempotency', () => { slug: 'aws-ec2', alertSensitivity: 'ALL', componentId: null, }}, ]) - const cs = diff(config, refs) + const cs = await diff(config, refs) expect(cs.creates).toHaveLength(0) expect(cs.updates).toHaveLength(0) }) - it('full stack config unchanged → zero changes across all resource types', () => { + it('full stack config unchanged → zero changes across all resource types', async () => { const secretValue = 'my-api-token' const channelConfig = {channelType: 'slack', webhookUrl: 'https://hooks.slack.com/test'} const channelHash = sha256Hex(stableStringify(channelConfig)) @@ -256,13 +256,13 @@ describe('idempotency', () => { }, managedBy: 'CLI'}, ]) - const cs = diff(config, refs) + const cs = await diff(config, refs) expect(cs.creates).toHaveLength(0) expect(cs.updates).toHaveLength(0) expect(cs.deletes).toHaveLength(0) }) - it('monitor with API-expanded null config fields → zero changes (snapshot null-strip)', () => { + it('monitor with API-expanded null config fields → zero changes (snapshot null-strip)', async () => { // Regression for the A1 BDD failure: API echoes back JSONB monitor // configs with every optional HttpMonitorConfig field expanded to null // (customHeaders, requestBody, contentType, verifyTls). The user's YAML @@ -285,11 +285,11 @@ describe('idempotency', () => { incidentPolicy: null, alertChannelIds: null, tagIds: null, }, managedBy: 'CLI'}, ]) - const cs = diff(config, refs) + const cs = await diff(config, refs) expect(cs.updates.filter(c => c.resourceType === 'monitor')).toHaveLength(0) }) - it('notification policy with matching escalation → zero changes', () => { + it('notification policy with matching escalation → zero changes', async () => { const refs = buildRefs([ {type: 'alertChannels', key: 'Slack', id: 'ac-1', raw: {name: 'Slack', channelType: 'slack'}}, {type: 'notificationPolicies', key: 'Default', id: 'np1', raw: { @@ -311,7 +311,7 @@ describe('idempotency', () => { escalation: {steps: [{channels: ['Slack'], delayMinutes: 0}]}, }], } - const cs = diff(config, refs) + const cs = await diff(config, refs) expect(cs.updates.filter(c => c.resourceType === 'notificationPolicy')).toHaveLength(0) }) }) diff --git a/test/yaml/moved.test.ts b/test/yaml/moved.test.ts index 6bd9056..78bc70b 100644 --- a/test/yaml/moved.test.ts +++ b/test/yaml/moved.test.ts @@ -8,7 +8,7 @@ describe('moved blocks', () => { describe('simple rename', () => { it('renames a monitor address while preserving API ID', () => { const state = emptyState() - upsertStateEntry(state, 'monitor', 'Old API', 'uuid-1', {name: 'Old API', type: 'HTTP'}) + upsertStateEntry(state, 'monitor', 'Old API', 'uuid-1') const warnings = processMovedBlocks(state, [ {from: 'monitors.Old API', to: 'monitors.Core API'}, @@ -19,14 +19,15 @@ describe('moved blocks', () => { const renamed = lookupByAddress(state, 'monitors.Core API') expect(renamed).toBeDefined() expect(renamed!.apiId).toBe('uuid-1') - expect(renamed!.attributes).toEqual({name: 'Old API', type: 'HTTP'}) + // v3: state stores identity only — no attributes carried across moves. + expect(renamed!.resourceType).toBe('monitor') }) it('renames a status page with children preserved', () => { const state = emptyState() - upsertStateEntry(state, 'statusPage', 'old-slug', 'sp-1', {name: 'Old Page'}, { - 'groups.Platform': {apiId: 'g-1', attributes: {name: 'Platform'}}, - 'components.API': {apiId: 'c-1', attributes: {name: 'API'}}, + upsertStateEntry(state, 'statusPage', 'old-slug', 'sp-1', { + 'groups.Platform': {apiId: 'g-1'}, + 'components.API': {apiId: 'c-1'}, }) processMovedBlocks(state, [ @@ -134,7 +135,7 @@ describe('moved blocks', () => { describe('previewMovedBlocks', () => { it('returns a renamed clone without mutating the original state', () => { const original = emptyState() - upsertStateEntry(original, 'monitor', 'Old API', 'uuid-1', {name: 'Old API', type: 'HTTP'}) + upsertStateEntry(original, 'monitor', 'Old API', 'uuid-1') const originalSerial = original.serial const originalLastDeployed = original.lastDeployedAt @@ -153,10 +154,10 @@ describe('moved blocks', () => { expect(original.lastDeployedAt).toBe(originalLastDeployed) }) - it('deep-copies children and attributes so mutations do not leak', () => { + it('deep-copies children so mutations do not leak', () => { const original = emptyState() - upsertStateEntry(original, 'statusPage', 'old-slug', 'sp-1', {name: 'Old Page'}, { - 'components.API': {apiId: 'c-1', attributes: {name: 'API'}}, + upsertStateEntry(original, 'statusPage', 'old-slug', 'sp-1', { + 'components.API': {apiId: 'c-1'}, }) const {state: preview} = previewMovedBlocks(original, [ @@ -164,12 +165,12 @@ describe('moved blocks', () => { ]) const moved = lookupByAddress(preview, 'statusPages.new-slug')! - // Mutate the preview's child attributes - moved.children['components.API'].attributes!.name = 'mutated' + // Mutate the preview's child apiId + moved.children['components.API'].apiId = 'mutated' // Original is unaffected const originalEntry = lookupByAddress(original, 'statusPages.old-slug')! - expect(originalEntry.children['components.API'].attributes!.name).toBe('API') + expect(originalEntry.children['components.API'].apiId).toBe('c-1') }) it('surfaces the same warnings as processMovedBlocks for invalid moves', () => { diff --git a/test/yaml/partial-failure-convergence.test.ts b/test/yaml/partial-failure-convergence.test.ts index bf53913..abb7119 100644 --- a/test/yaml/partial-failure-convergence.test.ts +++ b/test/yaml/partial-failure-convergence.test.ts @@ -35,7 +35,7 @@ import {mkdirSync, rmSync} from 'node:fs' import {join} from 'node:path' import {tmpdir} from 'node:os' import { - apply, buildStateV2, diff, emptyState, fetchAllRefs, + apply, buildStateV2, diff, emptyState, fetchAllRefs, prefetchChildSnapshots, readState, registerYamlPendingRefs, resourceAddress, validate, writeState, type ApplyResult, type DeployState, @@ -474,7 +474,8 @@ async function runDeploy( const client = api.client() as unknown as Parameters[2] const refs = await fetchAllRefs(client, currentState) registerYamlPendingRefs(refs, config) - const changeset = diff(config, refs, {}, currentState) + const currentChildren = await prefetchChildSnapshots(config, refs, client) + const changeset = await diff(config, refs, {}, currentState, currentChildren) const result = await apply(changeset, refs, client, currentState) // Mirror deploy/index.ts state-merge: build fresh state from successful @@ -1105,13 +1106,14 @@ describe('partial-failure convergence (deploy harness)', () => { // Description not applied (PUT failed). expect(api.componentsForPage(page.id)[0].description).toBeNull() - // State: child still tracked by apiId, but attributes are EMPTY - // (not the desired snapshot). This is the critical correctness - // detail — if we'd stored the desired snapshot here, the next - // diff would compare desired-vs-desired and see no drift. + // State: child still tracked by apiId. v3 stores identity only — + // there are no `attributes` on child entries. The next diff + // re-fetches live API children and compares them against the YAML + // snapshot, so drift is detected from the live API regardless of + // what state.json holds. const pageState = second.state.resources['statusPages.public'] expect(pageState.children['components.API']?.apiId).toBe(apiCompId) - expect(pageState.children['components.API']?.attributes).toEqual({}) + expect('attributes' in (pageState.children['components.API'] ?? {})).toBe(false) // Third deploy: clear failure, retry. Update must succeed. api.clearFailures() diff --git a/test/yaml/state.test.ts b/test/yaml/state.test.ts index 0d5cfcd..44b6415 100644 --- a/test/yaml/state.test.ts +++ b/test/yaml/state.test.ts @@ -1,16 +1,16 @@ import {describe, it, expect, beforeEach, afterEach} from 'vitest' -import {mkdirSync, rmSync, existsSync, writeFileSync} from 'node:fs' +import {mkdirSync, rmSync, existsSync, writeFileSync, readFileSync} from 'node:fs' import {join} from 'node:path' import {tmpdir} from 'node:os' import { - readState, writeState, buildStateV2, emptyState, StateFileCorruptError, + readState, writeState, buildState, buildStateV2, emptyState, StateFileCorruptError, upsertStateEntry, removeStateEntry, lookupByAddress, lookupByApiId, processMovedBlocks, resourceAddress, - parseAddress, migrateV1, buildState, + parseAddress, migrateV1, migrateV2, } from '../../src/lib/yaml/state.js' import type {DeployStateV1} from '../../src/lib/yaml/state.js' -describe('state v2', () => { +describe('state v3', () => { let tmpDir: string beforeEach(() => { @@ -28,8 +28,8 @@ describe('state v2', () => { expect(readState(tmpDir)).toBeUndefined() }) - it('writes and reads v2 state', () => { - const state = buildStateV2([ + it('writes and reads v3 state', () => { + const state = buildState([ {resourceType: 'monitor', refKey: 'API', apiId: 'mon-1'}, ]) writeState(state, tmpDir) @@ -37,12 +37,21 @@ describe('state v2', () => { const loaded = readState(tmpDir) expect(loaded).toBeDefined() - expect(loaded!.version).toBe('2') + expect(loaded!.version).toBe('3') expect(loaded!.serial).toBe(1) expect(loaded!.resources['monitors.API']).toBeDefined() expect(loaded!.resources['monitors.API'].apiId).toBe('mon-1') }) + it('does NOT persist `attributes` on disk (v3 is identity-only)', () => { + const state = buildState([ + {resourceType: 'monitor', refKey: 'API', apiId: 'mon-1'}, + ]) + writeState(state, tmpDir) + const raw = readFileSync(join(tmpDir, '.devhelm', 'state.json'), 'utf8') + expect(raw).not.toContain('attributes') + }) + it('creates .devhelm directory if missing', () => { writeState(emptyState(), tmpDir) expect(existsSync(join(tmpDir, '.devhelm'))).toBe(true) @@ -70,10 +79,10 @@ describe('state v2', () => { }) it('overwrites previous state on write', () => { - const s1 = buildStateV2([{resourceType: 'tag', refKey: 'A', apiId: 't-1'}]) + const s1 = buildState([{resourceType: 'tag', refKey: 'A', apiId: 't-1'}]) writeState(s1, tmpDir) - const s2 = buildStateV2([ + const s2 = buildState([ {resourceType: 'tag', refKey: 'A', apiId: 't-1'}, {resourceType: 'monitor', refKey: 'B', apiId: 'm-1'}, ]) @@ -85,16 +94,22 @@ describe('state v2', () => { // ── Serial increment ────────────────────────────────────────────── - it('buildStateV2 increments serial from previous', () => { - const s = buildStateV2([], 5) + it('buildState increments serial from previous', () => { + const s = buildState([], 5) expect(s.serial).toBe(6) }) - it('buildStateV2 defaults serial to 1 when no previous', () => { - const s = buildStateV2([]) + it('buildState defaults serial to 1 when no previous', () => { + const s = buildState([]) expect(s.serial).toBe(1) }) + it('buildStateV2 alias still works (deprecated shim)', () => { + const s = buildStateV2([{resourceType: 'tag', refKey: 'X', apiId: 't-1'}]) + expect(s.version).toBe('3') + expect(s.resources['tags.X'].apiId).toBe('t-1') + }) + // ── Address helpers ─────────────────────────────────────────────── it('resourceAddress builds correct addresses', () => { @@ -124,7 +139,7 @@ describe('state v2', () => { it('upsertStateEntry adds a new entry and increments serial', () => { const state = emptyState() - upsertStateEntry(state, 'monitor', 'API', 'uuid-1', {name: 'API'}) + upsertStateEntry(state, 'monitor', 'API', 'uuid-1') expect(state.resources['monitors.API']).toBeDefined() expect(state.resources['monitors.API'].apiId).toBe('uuid-1') expect(state.serial).toBe(1) @@ -132,16 +147,16 @@ describe('state v2', () => { it('upsertStateEntry overwrites existing entry', () => { const state = emptyState() - upsertStateEntry(state, 'monitor', 'API', 'uuid-1', {name: 'API'}) - upsertStateEntry(state, 'monitor', 'API', 'uuid-1', {name: 'API', url: 'https://new.com'}) - expect(state.resources['monitors.API'].attributes).toEqual({name: 'API', url: 'https://new.com'}) + upsertStateEntry(state, 'monitor', 'API', 'uuid-1') + upsertStateEntry(state, 'monitor', 'API', 'uuid-2') + expect(state.resources['monitors.API'].apiId).toBe('uuid-2') expect(state.serial).toBe(2) }) it('upsertStateEntry with children', () => { const state = emptyState() - upsertStateEntry(state, 'statusPage', 'devhelm', 'sp-1', {name: 'DevHelm'}, { - 'groups.Platform': {apiId: 'g-1', attributes: {name: 'Platform'}}, + upsertStateEntry(state, 'statusPage', 'devhelm', 'sp-1', { + 'groups.Platform': {apiId: 'g-1'}, }) const entry = state.resources['statusPages.devhelm'] expect(entry.children['groups.Platform'].apiId).toBe('g-1') @@ -163,7 +178,7 @@ describe('state v2', () => { // ── Lookup helpers ──────────────────────────────────────────────── it('lookupByAddress finds entry', () => { - const state = buildStateV2([{resourceType: 'monitor', refKey: 'API', apiId: 'uuid-1'}]) + const state = buildState([{resourceType: 'monitor', refKey: 'API', apiId: 'uuid-1'}]) expect(lookupByAddress(state, 'monitors.API')?.apiId).toBe('uuid-1') }) @@ -173,7 +188,7 @@ describe('state v2', () => { }) it('lookupByApiId finds entry across resource types', () => { - const state = buildStateV2([ + const state = buildState([ {resourceType: 'monitor', refKey: 'API', apiId: 'uuid-1'}, {resourceType: 'tag', refKey: 'prod', apiId: 'uuid-2'}, ]) @@ -234,17 +249,17 @@ describe('state v2', () => { it('processMovedBlocks preserves children through rename', () => { const state = emptyState() - upsertStateEntry(state, 'statusPage', 'old-slug', 'sp-1', {name: 'Old'}, { - 'groups.Platform': {apiId: 'g-1', attributes: {name: 'Platform'}}, + upsertStateEntry(state, 'statusPage', 'old-slug', 'sp-1', { + 'groups.Platform': {apiId: 'g-1'}, }) processMovedBlocks(state, [{from: 'statusPages.old-slug', to: 'statusPages.new-slug'}]) const moved = state.resources['statusPages.new-slug'] expect(moved.children['groups.Platform'].apiId).toBe('g-1') }) - // ── V1 → V2 migration ──────────────────────────────────────────── + // ── V1 → V3 migration ──────────────────────────────────────────── - it('migrateV1 converts to v2 format', () => { + it('migrateV1 converts to v3 format', () => { const v1: DeployStateV1 = { version: '1', lastDeployedAt: '2025-01-01T00:00:00Z', @@ -253,14 +268,14 @@ describe('state v2', () => { {resourceType: 'tag', refKey: 'prod', id: 'tag-1', createdAt: '2025-01-01'}, ], } - const v2 = migrateV1(v1) - expect(v2.version).toBe('2') - expect(v2.serial).toBe(1) - expect(v2.resources['monitors.API']).toBeDefined() - expect(v2.resources['monitors.API'].apiId).toBe('mon-1') - expect(v2.resources['monitors.API'].attributes).toEqual({name: 'API', _migrated: true}) - expect(v2.resources['tags.prod']).toBeDefined() - expect(v2.resources['tags.prod'].apiId).toBe('tag-1') + const v3 = migrateV1(v1) + expect(v3.version).toBe('3') + expect(v3.serial).toBe(1) + expect(v3.resources['monitors.API']).toBeDefined() + expect(v3.resources['monitors.API'].apiId).toBe('mon-1') + expect(v3.resources['monitors.API']).not.toHaveProperty('attributes') + expect(v3.resources['tags.prod']).toBeDefined() + expect(v3.resources['tags.prod'].apiId).toBe('tag-1') }) it('readState auto-migrates v1 on disk', () => { @@ -276,34 +291,75 @@ describe('state v2', () => { writeFileSync(join(dir, 'state.json'), JSON.stringify(v1)) const loaded = readState(tmpDir)! - expect(loaded.version).toBe('2') + expect(loaded.version).toBe('3') expect(loaded.serial).toBe(1) expect(loaded.resources['monitors.API'].apiId).toBe('mon-1') }) it('migrateV1 handles empty resources array', () => { const v1: DeployStateV1 = {version: '1', lastDeployedAt: '2025-01-01T00:00:00Z', resources: []} - const v2 = migrateV1(v1) - expect(v2.version).toBe('2') - expect(Object.keys(v2.resources)).toHaveLength(0) - }) - - // ── Backward-compat buildState shim ─────────────────────────────── + const v3 = migrateV1(v1) + expect(v3.version).toBe('3') + expect(Object.keys(v3.resources)).toHaveLength(0) + }) + + // ── V2 → V3 migration ──────────────────────────────────────────── + + it('migrateV2 strips parent and child attributes', () => { + const v2 = { + version: '2' as const, + serial: 7, + lastDeployedAt: '2025-04-22T00:00:00Z', + resources: { + 'statusPages.devhelm': { + apiId: 'sp-1', + resourceType: 'statusPage', + attributes: {name: 'DevHelm', slug: 'devhelm'}, + children: { + 'groups.Platform': {apiId: 'g-1', attributes: {name: 'Platform', defaultOpen: true}}, + 'components.API': {apiId: 'c-1', attributes: {name: 'API'}}, + }, + }, + }, + } + const v3 = migrateV2(v2) + expect(v3.version).toBe('3') + expect(v3.serial).toBe(7) + expect(v3.resources['statusPages.devhelm'].apiId).toBe('sp-1') + expect(v3.resources['statusPages.devhelm']).not.toHaveProperty('attributes') + expect(v3.resources['statusPages.devhelm'].children['groups.Platform'].apiId).toBe('g-1') + expect(v3.resources['statusPages.devhelm'].children['groups.Platform']).not.toHaveProperty('attributes') + expect(v3.resources['statusPages.devhelm'].children['components.API'].apiId).toBe('c-1') + }) + + it('readState auto-migrates v2 on disk silently', () => { + const v2 = { + version: '2', + serial: 3, + lastDeployedAt: '2025-04-22T00:00:00Z', + resources: { + 'monitors.API': { + apiId: 'mon-1', resourceType: 'monitor', + attributes: {name: 'API', frequencySeconds: 60}, + children: {}, + }, + }, + } + const dir = join(tmpDir, '.devhelm') + mkdirSync(dir, {recursive: true}) + writeFileSync(join(dir, 'state.json'), JSON.stringify(v2)) - it('buildState (compat shim) creates v2 state from v1 entries', () => { - const state = buildState([ - {resourceType: 'monitor', refKey: 'API', id: 'mon-1', createdAt: '2025-01-01'}, - ]) - expect(state.version).toBe('2') - expect(state.resources['monitors.API']).toBeDefined() - expect(state.resources['monitors.API'].apiId).toBe('mon-1') + const loaded = readState(tmpDir)! + expect(loaded.version).toBe('3') + expect(loaded.serial).toBe(3) + expect(loaded.resources['monitors.API'].apiId).toBe('mon-1') }) // ── emptyState ──────────────────────────────────────────────────── - it('emptyState creates valid empty v2 state', () => { + it('emptyState creates valid empty v3 state', () => { const state = emptyState() - expect(state.version).toBe('2') + expect(state.version).toBe('3') expect(state.serial).toBe(0) expect(Object.keys(state.resources)).toHaveLength(0) expect(state.lastDeployedAt).toBeTruthy() @@ -311,24 +367,23 @@ describe('state v2', () => { // ── Round-trip ──────────────────────────────────────────────────── - it('full write/read round-trip preserves all data', () => { - const state = buildStateV2([ + it('full write/read round-trip preserves identity (no attributes expected in v3)', () => { + const state = buildState([ { resourceType: 'statusPage', refKey: 'devhelm', apiId: 'sp-1', - attributes: {name: 'DevHelm', slug: 'devhelm'}, children: { - 'groups.Platform': {apiId: 'g-1', attributes: {name: 'Platform'}}, - 'components.API': {apiId: 'c-1', attributes: {name: 'API'}}, + 'groups.Platform': {apiId: 'g-1'}, + 'components.API': {apiId: 'c-1'}, }, }, - {resourceType: 'monitor', refKey: 'API', apiId: 'mon-1', attributes: {name: 'API', type: 'HTTP'}}, + {resourceType: 'monitor', refKey: 'API', apiId: 'mon-1'}, ]) writeState(state, tmpDir) const loaded = readState(tmpDir)! - expect(loaded.version).toBe('2') + expect(loaded.version).toBe('3') expect(loaded.serial).toBe(1) expect(loaded.resources['statusPages.devhelm'].children['groups.Platform'].apiId).toBe('g-1') - expect(loaded.resources['monitors.API'].attributes).toEqual({name: 'API', type: 'HTTP'}) + expect(loaded.resources['monitors.API'].apiId).toBe('mon-1') }) })