docs: bless staging whole-object create + delete (edit/create/delete in one review + save)

[sunib]

Closes #15 (the documentation ask). Leaves the optional PendingMutations helper out of scope on purpose — see below.

What and why

The console this library was extracted from grew whole-object create and delete and wanted them in the same review-and-save pane as field edits. LiveResourceStore is deliberately not the home for create/delete: it keys on metadata.uid (a pending create has none) and its value is the three-way merge (a create has nothing to merge, a delete nothing to reconcile). That read-only stance is correct — what was missing was guidance, so the next consumer doesn't re-derive it.

Commits

1. docs — a "Creating and deleting whole objects" section in client-state-model.md and create/delete siblings of the save handler in saving.md. Both point at adoptSaved / removeResource for reflecting the result before the watch echoes it, and both keep the 204-and-let-the-echo-settle-it recommendation. Two caveats the pattern needs and nobody had written down:

   • a create can only be reflected after the server assigns the uid — never fabricate one for a pending draft;
   • an optimistic delete is not self-healing, because a failed delete produces no watch event.

2. conformance — a new adopt-created-dedup fixture pins the I-IDEMPOTENT contract the docs now lean on: an optimistic adoptSaved and the watch echo of the same create converge to one object by uid, not two. The client loader learns an adopt op (it delivers a save response rather than editing a delivered object, so it carries a body, not a uid/path; the two edit invariants skip it for that reason). The gateway suite is untouched — it treats client as opaque and the fixture has no watch. The delete/recreate identity half was already pinned by delete-recreate-uid.

Deliberately out of scope

The issue's open question — a thin PendingMutations aggregator (edits + creates + deletes → one submit set). Declined for core, for a sharper reason than "reads only": the store holds only objects the stream delivered, and the one knowledge-laden part of aggregation (dedup an optimistic create/delete against its echo by uid) already lives in the store via I-IDEMPOTENT. What's left is knowledge-free page-local bookkeeping with a single known consumer. If a second consumer wants it, it's a natural fit for a write-side companion rather than this read library.

Verification

task lint, task test, and task fixtures-check all green.

Summary by CodeRabbit

• Documentation

  • Expanded guidance on “whole-object” create/delete behavior vs patch/streamed updates, including staging pending changes and handling immediate reflection with idempotent save adoption/removal.
  • Added validation boundaries for create and clarified delete semantics.

• Bug Fixes

  • Prevented optimistic create + watch echo from producing duplicate client objects by reconciling to a single uid.

• Tests

  • Added a new conformance fixture covering create deduplication with saved-object adoption.
  • Updated conformance test harness to support the new adopt edit operation and its expected shape.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 1644ff4b-2612-4a74-a8e7-af575ff657da

📥 Commits

Reviewing files that changed from the base of the PR and between 991b18b and b14707e.

📒 Files selected for processing (2)

• docs/client-state-model.md
• docs/saving.md

🚧 Files skipped from review as they are similar to previous changes (2)

• docs/saving.md
• docs/client-state-model.md

---

📝 Walkthrough

Walkthrough

The changes document host-side create/delete staging and add conformance support for an adopt operation. A new fixture verifies that an adopted created object and its watch echo deduplicate by metadata.uid without dirty state, conflicts, or a follow-up patch.

Changes

Create adoption and host state guidance

Layer / File(s)	Summary
Adopt fixture operation support packages/krm-stream/test/conformance.ts, packages/krm-stream/test/expect.ts, packages/krm-stream/test/conformance.test.ts	Fixture types and execution now support adopt edits carrying a saved object body, while validation excludes them from UID and path assertions.
Create echo deduplication fixture conformance/bodies/cm-created.v1.yaml, conformance/fixtures/adopt-created-dedup.yaml, conformance/README.md	Adds a created ConfigMap body and a fixture asserting that adoptSaved plus the watch echo yields one object by UID with no dirty entries, conflicts, or patch.
Whole-object create and delete guidance docs/client-state-model.md, docs/saving.md	Documents page-local staging for creates and deletes, host-side validation and endpoints, and idempotent adoptSaved and removeResource handling.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

• ConfigButler/krm-stream#1: Introduces the conformance test harness extended here with the adopt fixture operation and body handling.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title is specific and accurately summarizes the docs-focused create/delete staging work.
Linked Issues check	✅ Passed	The docs and conformance changes cover staged whole-object create/delete, host-side writes, adoptSaved/removeResource, and UID-based dedup as requested.
Out of Scope Changes check	✅ Passed	The changes stay focused on the documented create/delete pattern and the supporting conformance fixture/test updates.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch docs/staging-whole-object-create-delete

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 5

🧹 Nitpick comments (1)

docs/client-state-model.md (1)

74-75: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win

Distinguish the local draft key from metadata.uid.

The example says creates are id-less but includes id, which is ambiguous beside the UID-based store identity. Rename it to draftId or explicitly state that it is a client-only key and must never be used as the resource UID.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/client-state-model.md` around lines 74 - 75, Update the pendingCreates
example to use draftId instead of id, and clarify that this is a client-only key
distinct from metadata.uid and must never be used as the resource UID.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/client-state-model.md`:
- Around line 90-92: Update the save flow described by adoptSaved and
removeResource to clear the corresponding entries from pendingCreates and
pendingDeletes after successful create and delete operations. Ensure completed
mutations are removed from the page-local pending lists so they cannot remain in
the review list or be submitted again.
- Around line 62-66: Update the store ownership description near `adoptSaved` to
state that it holds server-identified objects, including projected objects
adopted before their watch echo arrives, rather than only objects delivered by
the stream. Preserve the explanation that pending creates lack a server uid and
therefore are not stored.

In `@docs/saving.md`:
- Around line 121-122: Update the documented client/server flow around the 204
response so the server handler only returns http.StatusNoContent, while the
browser consumer calls store.removeResource(uid) after a successful response
using its tracked UID. Remove the server-side optimistic-removal implication and
preserve the existing deleted-event behavior.
- Around line 97-100: Update the example flow around readObject and the dynamic
client Create call to validate and sanitize the browser-supplied object first.
Enforce the expected GVK, authorized scope and name, and permitted fields, then
pass only the sanitized object to Create; do not rely on Kubernetes admission as
the host-side validation step.

In `@packages/krm-stream/test/conformance.ts`:
- Around line 29-37: The FixtureEdit type in
packages/krm-stream/test/conformance.ts (lines 29-37) must use discriminated
variants: require body and omit uid/path for adopt, while requiring uid/path for
all other operations. In packages/krm-stream/test/expect.ts (lines 27-33),
update the adopt handling to pass e.body directly to body() without a non-null
assertion.

---

Nitpick comments:
In `@docs/client-state-model.md`:
- Around line 74-75: Update the pendingCreates example to use draftId instead of
id, and clarify that this is a client-only key distinct from metadata.uid and
must never be used as the resource UID.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 1570b7bf-88c5-426b-8696-29d4fab175ce

📥 Commits

Reviewing files that changed from the base of the PR and between 3d2309d and 991b18b.

⛔ Files ignored due to path filters (2)

• conformance/gen/bodies.json is excluded by !**/gen/**
• conformance/gen/fixtures.json is excluded by !**/gen/**

📒 Files selected for processing (8)

• conformance/README.md
• conformance/bodies/cm-created.v1.yaml
• conformance/fixtures/adopt-created-dedup.yaml
• docs/client-state-model.md
• docs/saving.md
• packages/krm-stream/test/conformance.test.ts
• packages/krm-stream/test/conformance.ts
• packages/krm-stream/test/expect.ts

[sunib]

Thanks @coderabbitai — addressed in b14707e. Dispositions:

Fixed (4):

• Store ownership too absolute (client-state-model.md) — reworded to "objects with a server identity — a metadata.uid — whether the stream delivered them or adoptSaved inserted one from a save response." You're right that an adopted object enters before its echo.
• Pending-entry cleanup after success (Major) — added: the store primitives update the store, not pendingCreates/pendingDeletes, so a successful write must clear the page-local entry or it can be submitted twice. Good catch.
• Validate the create object inline (Major, security) — the example now calls validateCreate(object, scope) before Create, and the prose no longer lists API-server admission as a host-side check (it's defense in depth behind the write). Agreed this matters for a library whose whole point is the guard.
• id → draftId — renamed and called out as a client-only key distinct from metadata.uid.
• Delete comment — removeResource is now attributed to the browser, not the server handler.

Declined (1):

• Discriminated union for FixtureEdit — keeping the single interface with body? + assertion. Two reasons: the union doesn't narrow through the .filter(e => e.op !== "adopt").map(e => e.path) in conformance.test.ts without adding a type-guard predicate, and the optional-field-plus-! style already matches the sibling ops (e.newKey!). Not worth the extra surface in a test loader.

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

@sunib Thanks for the detailed writeup and for addressing these in b14707e — all four fixes look solid, especially the pending-entry cleanup and inline validateCreate call, both of which close real correctness/security gaps.

On the declined item: agreed, keeping the single FixtureEdit interface with body? is the right call here. A discriminated union would indeed require a type-guard predicate to narrow cleanly through .filter(e => e.op !== "adopt").map(e => e.path) in packages/krm-stream/test/conformance.test.ts, and the optional-field-plus-! convention already matches the other ops (e.newKey!, etc.). Not worth the extra ceremony for a test loader.

Nice work tightening this up.

ʘ‿ʘ

---

✏️ Learnings added

Learnt from: sunib
URL: https://github.com/ConfigButler/krm-stream/pull/16

Timestamp: 2026-07-15T15:05:19.985Z
Learning: In packages/krm-stream/test/conformance.ts, the `FixtureEdit` interface intentionally uses a single shape with optional fields (e.g., `body?` for the `"adopt"` op, `newKey?` for `"renameKey"`) plus non-null assertions at use sites, rather than a discriminated union. This is a deliberate choice because a discriminated union would require adding type-guard predicates to narrow through filter/map chains like `.filter(e => e.op !== "adopt").map(e => e.path)` in packages/krm-stream/test/conformance.test.ts, which isn't worth the added surface for a test fixture loader.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.