From 8a33d58c570c307f15682b5638dd69a95ab80b29 Mon Sep 17 00:00:00 2001 From: Simon Koudijs Date: Wed, 15 Jul 2026 13:50:16 +0000 Subject: [PATCH 1/3] docs: bless staging whole-object create + delete MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit client-state-model.md and saving.md covered edit+save but said nothing about whole-object create/delete, so the next consumer re-derives (or mis-derives) it. The store keys on metadata.uid and has no merge for a create or a delete — that is correct and read-only by design — so staging the pending intent is the consumer's job. Document the pattern once: page-local pendingCreates/pendingDeletes aggregated with changes() into one review list under one Save, and the two optimistic primitives (adoptSaved / removeResource) that reflect the result before the watch echoes it, both idempotent with the echo. Two caveats the pattern needs and nobody states: a create can only be reflected after the server assigns the uid (never fabricate one), and an optimistic delete is not self-healing because a failed delete produces no watch event. Refs #15. Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/client-state-model.md | 43 ++++++++++++++++++++++++++++++++++ docs/saving.md | 48 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 91 insertions(+) diff --git a/docs/client-state-model.md b/docs/client-state-model.md index 622ff08..09e3d7b 100644 --- a/docs/client-state-model.md +++ b/docs/client-state-model.md @@ -57,6 +57,49 @@ Redacted paths are not placeholders and do not appear in the object. Render a wi `redactions(id)`, and do not offer an editor for it. The host save endpoint must also call [`gateway.ValidateMergePatch`](../gateway/patch.go) before writing to Kubernetes. +## Creating and deleting whole objects + +The store holds only objects the stream delivered, keyed by `metadata.uid`. That limit is deliberate, +and it is why two operations do not live here: + +- A pending **create** has no server object, so no uid and no key. There is nothing to merge it + against; `changes()`, `patch()`, and `conflicts()` have no meaning for it. +- A **delete** has no fields to reconcile. + +Staging a pending create or delete is therefore the consumer's job, the same way the write itself is +(see [saving edits safely](saving.md)). Keep them in page-local state and render all three sources as +one review list under one Save: + +```ts +const pendingCreates = []; // id-less drafts: { id, name, data } — no server object yet +const pendingDeletes = new Set(); // uids marked for removal + +// One list, one Save: +// pendingCreates → "create " +// pendingDeletes → "delete " +// store.changes(uid) → field edits (skip a uid that is in pendingDeletes) +``` + +### Reflecting the result + +The recommended shape is still 204 and let the watch echo it (see [saving edits safely](saving.md)): a +create arrives as an `added` event, a delete as a `deleted` event, and the store converges on its own. +Two primitives exist for a host that cannot wait for the echo, and both are idempotent with it +(`I-IDEMPOTENT`): + +- `adoptSaved(object)` — insert the created object once the save returns it. The echo that follows is + a no-op, not a second card. +- `removeResource(uid)` — drop a deleted object before its `deleted` event arrives. + +Two caveats keep this honest: + +- **A create reflects only _after_ the server responds.** `adoptSaved` needs the server-assigned uid + and a **projected** object (never a raw Kubernetes object — see [saving edits safely](saving.md)). Do + not fabricate a uid for a pending draft; keep it page-local until the create returns the object. +- **An optimistic delete is not self-healing.** A delete that _fails_ server-side produces no watch + event, so a `removeResource`d object does not reappear until the next snapshot. Re-add it on failure, + or skip the optimism and let the `deleted` echo do it. + ## Arrays and associative lists Arrays are atomic by default. A concurrent array change conflicts with a local array edit, which is diff --git a/docs/saving.md b/docs/saving.md index 6266c4b..1dacd00 100644 --- a/docs/saving.md +++ b/docs/saving.md @@ -79,3 +79,51 @@ intentionally incomplete, and a `PUT` can delete fields the browser never saw. `metadata.resourceVersion` may be stale when `krm-spec/v1` suppresses invisible status churn. Do not use the streamed value as a write precondition. The client-side three-way merge surfaces conflicts in the fields the user can see; send only the user's explicit merge-patch changes. + +## Creating and deleting whole objects + +A create and a delete are host writes exactly as a save is, and they stay host-side for the same +reasons: RBAC, attribution, and — for a create body — validation all live on the server. The client +stages the *intent*; your endpoint performs the *write*. See +[client state model](client-state-model.md#creating-and-deleting-whole-objects) for the client half — +the store keys on uid and has no merge for these, so the consumer aggregates staged create/delete with +`changes()` into one review list. + +```go +// POST /console/configmaps — create +func (s *server) createConfigMap(w http.ResponseWriter, r *http.Request) { + user := userFromSession(r) + scope := authorizedScope(user, r) + object := readObject(r) // the new object the browser assembled + + created, err := s.dynamicFor(user).Resource(configMaps).Namespace(scope.Namespace). + Create(r.Context(), object, metav1.CreateOptions{}) + if err != nil { + http.Error(w, "create failed", http.StatusBadGateway) + return + } + + // 204 and let the watch echo it — the same recommendation as save. To reflect it now instead, + // project it first and return it; the browser calls store.adoptSaved(projected). + _ = created + w.WriteHeader(http.StatusNoContent) +} + +// DELETE /console/configmaps/{name} — delete +func (s *server) deleteConfigMap(w http.ResponseWriter, r *http.Request) { + user := userFromSession(r) + scope := authorizedScope(user, r) + if err := s.dynamicFor(user).Resource(configMaps).Namespace(scope.Namespace). + Delete(r.Context(), scope.Name, metav1.DeleteOptions{}); err != nil { + http.Error(w, "delete failed", http.StatusBadGateway) + return + } + // 204; the `deleted` event prunes it from every open stream. Or store.removeResource(uid) now. + w.WriteHeader(http.StatusNoContent) +} +``` + +`ValidateMergePatch` guards a *patch*. A create sends a whole object, so validate it your own way — +admission, a schema, or an allowlist of the fields a browser may set — before it reaches the API +server; a projected or redacted field must no more ride in on a create body than in a patch. A delete +carries no body to guard. From 991b18b0d41d4c20e472b8f656e4935125e4c817 Mon Sep 17 00:00:00 2001 From: Simon Koudijs Date: Wed, 15 Jul 2026 13:52:15 +0000 Subject: [PATCH 2/3] conformance: pin adoptSaved idempotency with an adopt op MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The docs now bless adoptSaved as the optimistic-create primitive and lean on its I-IDEMPOTENT contract: an optimistic insert and the watch echo of the same create must converge to one object, keyed by uid, not two cards. Nothing pinned that. (The delete/recreate identity half already is, by delete-recreate-uid.) Teach the client loader an `adopt` edit op — it delivers the object a save returned via store.adoptSaved, carrying a bodies/ ref rather than a uid/path, since it is a delivery, not an edit. The two edit invariants (addresses-a-delivered-uid, has-a-path) skip it for that reason. The gateway suite is untouched: it treats `client` as opaque, and the fixture has no watch. Refs #15. Co-Authored-By: Claude Opus 4.8 (1M context) --- conformance/README.md | 1 + conformance/bodies/cm-created.v1.yaml | 12 +++++ conformance/fixtures/adopt-created-dedup.yaml | 26 ++++++++++ conformance/gen/bodies.json | 13 +++++ conformance/gen/fixtures.json | 49 +++++++++++++++++++ packages/krm-stream/test/conformance.test.ts | 6 ++- packages/krm-stream/test/conformance.ts | 6 ++- packages/krm-stream/test/expect.ts | 8 ++- 8 files changed, 118 insertions(+), 3 deletions(-) create mode 100644 conformance/bodies/cm-created.v1.yaml create mode 100644 conformance/fixtures/adopt-created-dedup.yaml diff --git a/conformance/README.md b/conformance/README.md index bc36010..f10eab4 100644 --- a/conformance/README.md +++ b/conformance/README.md @@ -121,6 +121,7 @@ Every fixture names the rule it defends, in `why:`. The ones that catch real bug | `reconnect-prune` | pruning is gated on `synced`; a reconnect removes what vanished while away | | `partial-cycle-no-prune` | a cycle that never reaches `synced` prunes **nothing** | | `delete-recreate-uid` | identity is `uid`, never `name`; no state bleeds across a recreate | +| `adopt-created-dedup` | **I-IDEMPOTENT** — an optimistic `adoptSaved` and the watch echo of the create are one object by uid, not two | | `resync-midstream` | upstream continuity can be lost *without* the SSE connection dropping → a fresh cycle mid-stream | | `nested-field-removed` | `added`/`modified` **replace**; a deep-merge would resurrect a field the server deleted (a ghost) | | `status-follow-live` | `status` is read-only under the full projection: it follows the server live, and never becomes dirty, never conflicts, never enters a patch | diff --git a/conformance/bodies/cm-created.v1.yaml b/conformance/bodies/cm-created.v1.yaml new file mode 100644 index 0000000..4cabe01 --- /dev/null +++ b/conformance/bodies/cm-created.v1.yaml @@ -0,0 +1,12 @@ +# A ConfigMap that did not exist when the stream started: the host created it, the save returned this +# projected object, and the browser called adoptSaved on it. The watch echoes the SAME uid a beat +# later. New uid, new name — nothing about it collides with the baseline app-config. +apiVersion: v1 +kind: ConfigMap +metadata: + uid: cm-created-0007 + name: feature-flags + namespace: app + resourceVersion: "3001" +data: + beta: "true" diff --git a/conformance/fixtures/adopt-created-dedup.yaml b/conformance/fixtures/adopt-created-dedup.yaml new file mode 100644 index 0000000..37cbf74 --- /dev/null +++ b/conformance/fixtures/adopt-created-dedup.yaml @@ -0,0 +1,26 @@ +id: adopt-created-dedup +title: An optimistic create (adoptSaved) and the watch echo of it are the SAME object, by uid. +why: > + I-IDEMPOTENT. A create is host-owned: the store keys on metadata.uid and a pending create has none, + so staging it is the consumer's job (docs/client-state-model.md). Once the save returns the created + object the host may adoptSaved it, so the UI need not wait for the watch. The watch then echoes it as + `added` — and that echo must be a no-op, not a second card. Reconciled by uid, the optimistic insert + and its echo converge to one object, with nothing dirty and nothing to save. +suites: [client] +scope: { target: demo, version: v1, resource: configmaps, namespace: app } +projection: krm-full/v1 + +events: + - { type: reset } + - { type: added, body: cm-app.v1 } + - { type: synced } + - { type: added, body: cm-created.v1 } # the watch catches up to the create + +client: + edits: + - { after: 2, op: adopt, body: cm-created.v1 } # host adopts the save response BEFORE the echo + expect: + uids: [cm-app-0001, cm-created-0007] # two objects, not three — the echo deduped by uid + dirty: [] + conflicts: [] + patch: null diff --git a/conformance/gen/bodies.json b/conformance/gen/bodies.json index 22897c6..7ec49f2 100644 --- a/conformance/gen/bodies.json +++ b/conformance/gen/bodies.json @@ -111,6 +111,19 @@ "log-level": "info" } }, + "cm-created.v1": { + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": { + "uid": "cm-created-0007", + "name": "feature-flags", + "namespace": "app", + "resourceVersion": "3001" + }, + "data": { + "beta": "true" + } + }, "cm-flags.v1": { "apiVersion": "v1", "kind": "ConfigMap", diff --git a/conformance/gen/fixtures.json b/conformance/gen/fixtures.json index a504893..5ccc20b 100644 --- a/conformance/gen/fixtures.json +++ b/conformance/gen/fixtures.json @@ -1,4 +1,53 @@ [ + { + "id": "adopt-created-dedup", + "title": "An optimistic create (adoptSaved) and the watch echo of it are the SAME object, by uid.", + "why": "I-IDEMPOTENT. A create is host-owned: the store keys on metadata.uid and a pending create has none, so staging it is the consumer's job (docs/client-state-model.md). Once the save returns the created object the host may adoptSaved it, so the UI need not wait for the watch. The watch then echoes it as `added` — and that echo must be a no-op, not a second card. Reconciled by uid, the optimistic insert and its echo converge to one object, with nothing dirty and nothing to save.\n", + "suites": [ + "client" + ], + "scope": { + "target": "demo", + "version": "v1", + "resource": "configmaps", + "namespace": "app" + }, + "projection": "krm-full/v1", + "events": [ + { + "type": "reset" + }, + { + "type": "added", + "body": "cm-app.v1" + }, + { + "type": "synced" + }, + { + "type": "added", + "body": "cm-created.v1" + } + ], + "client": { + "edits": [ + { + "after": 2, + "op": "adopt", + "body": "cm-created.v1" + } + ], + "expect": { + "uids": [ + "cm-app-0001", + "cm-created-0007" + ], + "dirty": [], + "conflicts": [], + "patch": null + } + } + }, { "id": "array-atomic-on-change", "title": "When an array's length changes under an edit, the whole array conflicts — atomically.", diff --git a/packages/krm-stream/test/conformance.test.ts b/packages/krm-stream/test/conformance.test.ts index 56f8525..b6b179f 100644 --- a/packages/krm-stream/test/conformance.test.ts +++ b/packages/krm-stream/test/conformance.test.ts @@ -83,6 +83,10 @@ test("a snapshot cycle is reset … added* … synced", () => { test("client fixtures edit objects the stream actually delivered", () => { for (const f of clientFixtures()) { for (const edit of f.client?.edits ?? []) { + // `adopt` is the exception that proves the rule: it delivers an object the stream has NOT yet + // sent (the whole point is that its echo arrives later and must dedup by uid), and it carries a + // `body`, not a `uid`/`path`. The delivered-and-has-a-path checks are for edits, which it is not. + if (edit.op === "adopt") continue; const delivered = deliveredUidsBefore(f, edit.after); assert.ok( delivered.has(edit.uid), @@ -102,7 +106,7 @@ test("paths are segment arrays, never dot-joined strings", () => { ...(f.client?.expect?.absentPaths ?? []), ...(f.client?.expect?.readOnlyPaths ?? []), ...(f.client?.expect?.flashed ?? []), - ...(f.client?.edits ?? []).map((e) => e.path), + ...(f.client?.edits ?? []).filter((e) => e.op !== "adopt").map((e) => e.path), ]; for (const p of paths) { assert.ok(Array.isArray(p), `${f.id}: ${JSON.stringify(p)} must be a segment array`); diff --git a/packages/krm-stream/test/conformance.ts b/packages/krm-stream/test/conformance.ts index 240f893..07e598f 100644 --- a/packages/krm-stream/test/conformance.ts +++ b/packages/krm-stream/test/conformance.ts @@ -26,11 +26,15 @@ const CONFORMANCE = new URL("../../../conformance/", import.meta.url); * A fixture format that could not express that ordering could not test a three-way merge at all. */ export interface FixtureEdit { after: number; - op: "set" | "remove" | "addKey" | "renameKey" | "revert"; + op: "set" | "remove" | "addKey" | "renameKey" | "revert" | "adopt"; uid: string; path: Path; value?: unknown; newKey?: string; + /** `adopt` only: the bodies/ reference to hand `store.adoptSaved`. An adopt is not an edit to a + * delivered object — it IS a delivery, of the object a save returned — so it carries a `body`, not a + * `uid`/`path` like the other ops. */ + body?: string; } export interface FixtureExpect { diff --git a/packages/krm-stream/test/expect.ts b/packages/krm-stream/test/expect.ts index 2417bfa..8c24cb6 100644 --- a/packages/krm-stream/test/expect.ts +++ b/packages/krm-stream/test/expect.ts @@ -19,12 +19,18 @@ import assert from "node:assert/strict"; import type { LiveResourceStore } from "../src/index.ts"; import type { Path } from "../src/types.ts"; -import type { FixtureEdit, FixtureExpect } from "./conformance.ts"; +import { body, type FixtureEdit, type FixtureExpect } from "./conformance.ts"; /** `path` in a fixture edit always addresses the FIELD, not its container — even for the two ops * whose store signature takes the map plus a key. Keeping the fixture format uniform is worth the one * line of translation. */ export function applyEdit(store: LiveResourceStore, e: FixtureEdit): void { + // `adopt` is not an edit to a delivered object — it delivers one, the way a save response does. It + // carries a `body`, not a `uid`/`path`, so it returns before we touch either. + if (e.op === "adopt") { + store.adoptSaved(body(e.body!)); + return; + } const parent = e.path.slice(0, -1); const last = e.path[e.path.length - 1]; switch (e.op) { From b14707edd0628dc2b5451960730f59bf1846452f Mon Sep 17 00:00:00 2001 From: Simon Koudijs Date: Wed, 15 Jul 2026 15:04:41 +0000 Subject: [PATCH 3/3] docs: address PR review on create/delete staging MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Frame store ownership as "has a server identity (uid)", not "the stream delivered it": adoptSaved inserts a save response before its echo, so the stream-only phrasing was too absolute. - Note that a successful write must clear the page-local pendingCreates/ pendingDeletes entry — the store primitives touch the store, not those lists, so a completed mutation left staged can be submitted twice. - Show host-side create validation inline (validateCreate) and stop listing API-server admission as a host-side check — it is defense in depth behind the write, not a substitute for validating before it. - Rename the pendingCreates key id -> draftId and call it a client-only key, distinct from metadata.uid. - Attribute removeResource to the browser in the delete handler comment, not the server handler. Refs #15. Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/client-state-model.md | 11 ++++++++--- docs/saving.md | 19 ++++++++++++++----- 2 files changed, 22 insertions(+), 8 deletions(-) diff --git a/docs/client-state-model.md b/docs/client-state-model.md index 09e3d7b..90f3d2d 100644 --- a/docs/client-state-model.md +++ b/docs/client-state-model.md @@ -59,8 +59,9 @@ Redacted paths are not placeholders and do not appear in the object. Render a wi ## Creating and deleting whole objects -The store holds only objects the stream delivered, keyed by `metadata.uid`. That limit is deliberate, -and it is why two operations do not live here: +The store holds only objects with a server identity — a `metadata.uid` — whether the stream delivered +them or `adoptSaved` inserted one from a save response before its echo arrived. That limit is +deliberate, and it is why two operations do not live here: - A pending **create** has no server object, so no uid and no key. There is nothing to merge it against; `changes()`, `patch()`, and `conflicts()` have no meaning for it. @@ -71,7 +72,7 @@ Staging a pending create or delete is therefore the consumer's job, the same way one review list under one Save: ```ts -const pendingCreates = []; // id-less drafts: { id, name, data } — no server object yet +const pendingCreates = []; // client-only drafts keyed by a local draftId, never a uid: { draftId, name, data } const pendingDeletes = new Set(); // uids marked for removal // One list, one Save: @@ -91,6 +92,10 @@ Two primitives exist for a host that cannot wait for the echo, and both are idem a no-op, not a second card. - `removeResource(uid)` — drop a deleted object before its `deleted` event arrives. +Either way, clear the page-local entry — the `pendingCreates` draft or the `pendingDeletes` uid — when +its write succeeds. Those primitives update the store, not your pending lists; a completed mutation +left staged reappears in the review list and can be submitted twice. + Two caveats keep this honest: - **A create reflects only _after_ the server responds.** `adoptSaved` needs the server-assigned uid diff --git a/docs/saving.md b/docs/saving.md index 1dacd00..3784933 100644 --- a/docs/saving.md +++ b/docs/saving.md @@ -96,6 +96,13 @@ func (s *server) createConfigMap(w http.ResponseWriter, r *http.Request) { scope := authorizedScope(user, r) object := readObject(r) // the new object the browser assembled + // Validate on the host, before the write — pin the GVK, the authorized scope and name, and an + // allowlist of the fields a browser may set. Never trust the assembled object as-is. + if err := validateCreate(object, scope); err != nil { + http.Error(w, err.Error(), http.StatusBadRequest) + return + } + created, err := s.dynamicFor(user).Resource(configMaps).Namespace(scope.Namespace). Create(r.Context(), object, metav1.CreateOptions{}) if err != nil { @@ -118,12 +125,14 @@ func (s *server) deleteConfigMap(w http.ResponseWriter, r *http.Request) { http.Error(w, "delete failed", http.StatusBadGateway) return } - // 204; the `deleted` event prunes it from every open stream. Or store.removeResource(uid) now. + // 204; the `deleted` event prunes it from every open stream. To reflect it now instead, the + // browser calls store.removeResource(uid) with the uid it already tracks. w.WriteHeader(http.StatusNoContent) } ``` -`ValidateMergePatch` guards a *patch*. A create sends a whole object, so validate it your own way — -admission, a schema, or an allowlist of the fields a browser may set — before it reaches the API -server; a projected or redacted field must no more ride in on a create body than in a patch. A delete -carries no body to guard. +`ValidateMergePatch` guards a *patch*. A create sends a whole object, so validate it yourself before +the call — the `validateCreate` above stands in for a schema check or a field allowlist — and pass only +the sanitized object to `Create`. A projected or redacted field must no more ride in on a create body +than in a patch. API-server admission sits behind this as defense in depth, not as a substitute for the +host-side check. A delete carries no body to guard.