feat(state): identity-only state.json; live-API attribute reads at plan time (RFC 0001)#15
Merged
Merged
Conversation
…ads at plan time (RFC 0001) 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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements RFC 0001. The CLI now diffs YAML against the live API at plan time instead of against a stale
state.jsonsnapshot.state.jsonkeeps only identity (apiId+ childapiIds).This fixes a class of bugs we keep hitting (most recently
defaultOpenflapping between mini and prod when the same operator deployed to both). It also subsumes Issue A (pullStatusPageChildrenunderfilling): the differ never reads child attributes from state, so a partial state pull cannot mislead the next plan.What changed
state.jsonschema bumped to v3 (identity-only). v1→v3 and v2→v3 reader-side migrations stripattributessilently on load — no user migration step.differ.diff()is now async and accepts aChildSnapshotMap. NewprefetchChildSnapshots(config, refs, client)runs all child fetches concurrently up-front.statusPageHandlerdeclaresfetchChildSnapshots(live-API read of groups + components).hasChildChangesdiffs YAML against the live snapshot, not state.applier,child-reconciler,state pull, andimportno longer writeattributes.upsertStateEntrylost the parameter.buildStateV2kept as a deprecated alias ofbuildStatethat silently drops attributes — preserves the public name until the next major.Behavioral impact
devhelm state pullafter manual importTest plan
npm run typecheck— cleannpm run lint— cleannpm test— 870/870 passingtest/yaml/state.test.ts.differ.test.ts"FIXED:" cases now exercise the live-snapshot path via injectedChildSnapshotMap.partial-failure-convergence.test.ts(j) asserts identity-only child entries; convergence still holds because the next plan re-reads the API.devhelm planagainst mini after pulling latest state, confirm no phantom diff on the existingdefaultOpencase.devhelm plan, confirm drift surfaces.Pairs with
devhelmhq/mono#266— dashboard fix to show the real public URL (devhelm.io/sp/<slug>) for status pages. Independent change but shipped together because both surfaced from the same audit.Out of scope (next PR)
<slug>.devhelm.devfirst-party domain — needs domain registration, Cloudflare config, dashboard + web wiring. Tracked separately.