feat!: export gateway.Project, and return the whole StreamChange from applyStreamEvent - #10
Conversation
… applyStreamEvent
Items 2 and 3 from the gitops-api adoption report. Both are the same bug in
two languages: the library computed what a host needed and then dropped it
at the seam.
# 2. The save path had no supported shape
store.adoptSaved(object) is the obvious thing to reach for after a save, and
there was no safe way to feed it: project() was unexported, so a host could
either hand the browser a RAW object — managedFields, status, and the Secret
values the projection exists to withhold, delivered through the one endpoint
the stream does not guard — or skip adoptSaved entirely and let the watch
echo settle it.
The second one is right, and nothing said so. Now both halves are fixed:
- saving.md states that 204 + let-the-watch-echo-it is the RECOMMENDED
shape, and says why: the store converges on its own and dirty state is
derived, so there is nothing to adopt. Its example previously ended with
writeResult(w, result, err) — handing back the raw object, i.e.
demonstrating the leak in the recommended code. It answers 204 now.
- gateway.Project(projection, obj) is exported for hosts that must answer
with the object, so adoptSaved can be fed something safe.
- adoptSaved's own doc comment now says its argument MUST be projected.
Project returns []string, not []Redaction: Rev is a counter the STREAM keeps
per uid, incremented when a withheld value changes underneath a consumer who
cannot see it. There is no honest Rev for a lone object, and inventing a zero
would be a field that looks like an answer.
# 3. applyStreamEvent threw away everything but the paths
The store computes ApplyResult{added, structural, flashed, conflicts} and the
seam returned .flashed alone, with no uid. So a UI rendering more than one
resource per stream learned that something moved and not WHAT — and the only
way out was to abandon connectWithEventSource, drive its own EventSource, and
reimplement the event switch. That is the exact work this library exists to do
once, on everyone's behalf.
applyStreamEvent now returns StreamChange{type, uid, added, structural,
flashed, conflicts}, and onChange receives it. Each field answers a question a
renderer actually has: `added` distinguishes an arrival from a change,
`structural` says rebuild-the-rows rather than re-read-the-values, and uid says
which resource. A delete reports structural: true, because a row left.
BREAKING CHANGE: applyStreamEvent returns StreamChange rather than Path[], and
onChange receives a StreamChange rather than Path[]. Read `.flashed` for the
previous value.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 36 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (10)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
1 participant
Items 2 and 3 from the gitops-api adoption report. They turned out to be the same bug in two languages: the library computed exactly what the host needed and then dropped it at the seam.
2. The save path had no supported shape
store.adoptSaved(object)is the obvious thing to reach for after a save, and there was no safe way to feed it —project()was unexported. So a host could either hand the browser a raw object (managedFields, status, and the Secret values the projection exists to withhold, delivered through the one endpoint the stream does not guard), or skipadoptSavedentirely and let the watch echo settle it.You picked the second one by elimination. It's the right answer, and nothing said so.
The doc was worse than silent — it demonstrated the leak. saving.md's recommended example ended with
writeResult(w, result, err): writing the object Kubernetes just returned straight back to the browser. A host copying the recommended code shipped the exact leak the projection exists to prevent. It answers204now, with a section explaining that the write reaches the API server, the watch echoes it back projected and redacted, the store three-way merges it into the draft the user is still holding, and dirty state is derived — so there is nothing to adopt.gateway.Project(projection, obj)is exported for hosts that genuinely must answer with the object, andadoptSaved's doc comment now says its argument must be projected.Why
[]stringand not[]Redaction:Redactioncarries aRev, andRevis not a property of an object — it's a counter the stream keeps per uid, incremented when a withheld value changes underneath a consumer who cannot see it. There is no honestRevfor a lone object, and returning a zero would be a field that looks like an answer.adoptSavedneeds none: it keeps the redactions the store already has.3. applyStreamEvent threw away everything but the paths
Wider than reported. The store computes
ApplyResult{added, structural, flashed, conflicts}and the seam returned.flashedalone, with no uid. So a UI rendering more than one resource per stream learned that something moved and not what — and the only way out was to abandonconnectWithEventSource, drive your ownEventSource, and reimplement the event switch. Which is precisely the work this library exists to do once, correctly, on everyone's behalf.addedandstructuralwere being computed and discarded too, and both change what a renderer does: a new key that is never reported as structural is a row that never gets drawn. Adeletedevent reportsstructural: true, because a row left the collection.Verified
gofmt,go vet. NewTestProjectMakesASavedObjectSafeToReturnasserts the property a host actually relies on — managedFields, the last-applied annotation and the Secret value are gone,/data/tokenis still named in redacted, and the caller's object (which may be an informer's, shared with every other stream on that scope) is not mutated.StreamChangefield, including that a value change is not structural but a new key is.Note on merge order
Both this and #8 touch
examples/vanilla-browser/index.html, in different hunks (theonChangehandler here, the entry-point import there). Whichever lands second may want a trivial rebase.BREAKING CHANGE:
applyStreamEventreturnsStreamChangerather thanPath[];onChangereceives aStreamChange. Read.flashedfor the old value.🤖 Generated with Claude Code