feat: add KRM resource streaming library - #1
Merged
Conversation
…e corpus
The client fixtures asserted nothing until now: test/ only checked that the
corpus was well-formed. It now replays each fixture's `events` into a store,
applies `client.edits` at their `after:` positions, and asserts `expect` —
dirty, conflicts, draftSubset, absentPaths, readOnlyPaths, flashed, patch — plus
the checkpoints in conflict-and-converge. All 14 client fixtures pass.
The store is the two layers of spec §4.1, kept apart:
- applyServerEvent REPLACES the authoritative object (a deep-merge cannot
express a removal, so it resurrects the field the server just deleted), then
three-way reconciles the editable regions against the PREVIOUS server object
as base (R-THREEWAY).
- read-only regions follow the server live and flash what moved. They never
grow a draft, a dirty flag or a conflict, and never enter a patch — and a
write to one is refused, not ignored.
- dirtiness is derived on every ask (R-DERIVED); paths are segment arrays
(R-ID); deepEqual is a structural compare, not JSON.stringify.
- patch() is RFC 7386 over the editable changes only; arrays go whole.
Each of the three bugs the inline original shipped is now caught by a mutation:
dropping the base term fails 6 fixtures, keeping a pruned key fails 2, pruning
on reset instead of synced fails 1. Two rules no fixture pins — a REFUSED edit
to status, and key-order-independent equality — are pinned in
test/invariants.test.ts, which is where docs §7 says the property tests live.
No runtime dependencies. The emitted ESM imports nothing but its own relative
./*.js, and a browser can import it with no bundler.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The TS side was two tools short of the Go side, and one of the gaps was a correctness hole rather than a nicety: tsconfig.json includes only src/, so `tsc --noEmit` saw ZERO files under test/ — and `node --test` on Node 22 STRIPS types, it does not check them. The conformance suite, whose entire job is to be the contract check, was the one piece of unverified TypeScript in the repo. tsconfig.test.json (noEmit, src + test) closes that; it needs @types/node, because the tests import node:test. Biome supplies what gofmt + go vet + golangci-lint supply on the other side, in one binary. Both run in `task lint` and in CI; `task fmt-client` fixes what it can. noNonNullAssertion is off, deliberately: with `noUncheckedIndexedAccess` on, `path[i]!` inside a loop bounded by `path.length` is the honest expression of a fact the bound already guarantees, and the alternative is a runtime guard for a branch that cannot happen. Turning OFF noUncheckedIndexedAccess to satisfy the lint rule would have traded a real check for a cosmetic one. CONTRIBUTING now says what the dependency rule actually means: no RUNTIME dependencies, ever — that one is absolute, the bundle is plain ESM a browser imports with no bundler. devDependencies ship nothing, and the three we have each pay for themselves. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… corpus Rung 1 of the e2e ladder: each fixture's `watch:` ops are played through a fake Kubernetes watch and what the gateway puts on the wire is compared to the fixture's `events:` — byte for byte, because `events:` is exactly what the TypeScript store on the other side is fed. All 11 gateway fixtures pass. No cluster: every rule in the corpus is about framing and projection, and a fake watch drives those deterministically in microseconds. The seams (seams.go) are CONTRIBUTING's one rule expressed as Go. The gateway does not know what a tenant is or where a kubeconfig lives; it asks the host, through Authorizer and ClientFor(target, principal), and the host answers. The upstream is behind an interface for the same reason — and that buys the test suite as a side effect: the corpus drives a ScriptedBackend, a real cluster will drive a client-go one, and the loop cannot tell them apart. Watcher is PULL-based, deliberately. "The gateway was handed the last event" and "the gateway finished with it" are the same moment under a pull, and different moments under a channel — so the conformance replay is deterministic with no sleeps. A channel-based client-go watch adapts in ten lines; the reverse does not adapt at all. Two spec rules turned out to be UNTESTABLE by the corpus, and I proved it by mutation: emitting `synced` on every bookmark, and forwarding a partial object with no uid, both leave all 11 fixtures green. A fixture's `watch:` script has no op for a bare BOOKMARK or a degenerate tombstone, so it cannot say those words. They are MUST NOTs (spec §2, §4.2) and rows 7/8 of the gateway's own matrix, so they are pinned in stream_test.go instead — and the fixture format needs three new ops, which is a change to the shared contract and therefore not mine to make unilaterally. Six mutations, six caught: bookmark-as-synced, partial-object-forwarded, monotonicity-removed, tombstone-uid-guessed, secret-policy-off, and empty-named-scope-emits-nothing. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…wrote
Rung 2 of the e2e ladder, and the one that turns "end to end" from a claim into
a fact.
Until now the two implementations shared a DOCUMENT. Go asserted "given this
watch I would emit these events"; TypeScript asserted "given these events I hold
this state"; and nothing, anywhere, checked that the events one side emits are
bytes the other side can read. Two implementations agreeing to disagree is
exactly what that gap permits — and it was not hypothetical, see below.
Three things land:
- gateway/sse.go — the transport. Content-Type, flush-per-frame, a heartbeat
COMMENT every 20s, no `id:` lines ever, and a terminal error as the last
event before the close.
- conformance/gen/sse/*.sse — golden transcripts: the bytes the gateway really
writes, through its real sink, committed and regenerated by `task fixtures`.
They are produced BY the gateway rather than hand-written beside it, because
a transcript written next to the implementation agrees with it by
construction and proves nothing.
- src/sse.ts — the consumer half that never got built: an incremental
SSEDecoder, connectResourceStream (fetch, for bearer tokens) and
connectWithEventSource (cookies, the v1 baseline). The TS suite now replays
the golden bytes ONE BYTE AT A TIME — a frame split down the middle is what a
real network does under exactly the load where you least want to debug it.
And `task e2e-wire`: a real Go gateway on a real socket, read by the real client
over real fetch, 11 fixtures, no cluster. cmd/replay serves the corpus as SSE, so
it is also a cluster you can point a browser at.
The seam caught a genuine divergence the moment it was closed: `terminal` was
omitted from an error event when false (Go's omitempty), so a consumer had to
INFER "do I give up?" from a missing field. That is the same reasoning that made
redactedPaths mandatory, and the failure it prevents is worse — EventSource
reconnects on its own, so a misread terminal error means every open tab hammering
a forbidden scope forever. The field is now always present. Both sides had been
"passing" their tests for a week.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…two bugs they found
Proposal 0001 (docs/proposals/0001-watch-ops.md), grounded in a full read of the
Kubernetes API concepts page, summarized with citations in
docs/facts/kubernetes-api-concepts.md. That file exists because spec/v1.md and
gateway/README.md made claims about the watch API that were written from memory,
and two of them were wrong.
The corpus could not express three of the gateway's own MUST NOTs — proved by
mutation: "emit synced on every bookmark" and "forward a partial object" both
left all 11 fixtures green. So `watch:` grows three ops, each a documented
Kubernetes behaviour:
bookmark a routine BOOKMARK. Kubernetes, verbatim: its object "only includes
a .metadata.resourceVersion field". An object with no uid, no name,
no spec and no status is not an edge case — it is on EVERY stream
that asked for bookmarks, and we must ask, because that is how the
snapshot boundary arrives at all.
partial PartialObjectMetadata delivered as an upsert.
tombstone a DELETED that lost its identity (DeletedFinalStateUnknown).
Two real bugs fell out:
1. resourceVersion OVERFLOW. Kubernetes: "compared as arbitrary bitsize decimal
integers... the bitsize must not be assumed"; its own example is 40 digits.
We compared with strconv.ParseInt — int64, 19 digits. On such a cluster the
parse fails, the monotonicity check gives up, and live updates are SILENTLY
DROPPED — a symptom indistinguishable from "Kubernetes is slow", which is how
a bug like this survives for years. Now compared as the docs prescribe:
longer-is-greater, then lexicographic. A plain lexicographic compare is also
wrong ("9" > "10"), and an extension API server's non-decimal resourceVersion
cannot be ordered at all — so it now drops nothing rather than guessing.
2. The partial-object guard checked the WRONG FIELD. It asked "does it have a
uid?" — but a PartialObjectMetadata has a complete metadata block, uid
included; what it lacks is spec and status. The guard waved it through, and a
consumer whose model is REPLACE would have swapped a live Deployment for a
husk: status blank, the user's spec gone. The honest test is the kind.
spec §6's non-normative note has been corrected — it said "monotonically
increasing integer", which invites exactly the ParseInt that bit us. The
normative rule (resourceVersion is opaque to consumers) is unchanged, and no wire
change: all three ops describe things the gateway must absorb or refuse, so the
correct behaviour emits FEWER events, never different ones.
Four new fixtures. All four catch their mutation; the corpus can now go red for
every rule it claims to defend.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…e, no bundler, no cluster
`task e2e-browser` (and `task demo`, which is the same thing at human speed).
This is the only test of the constraint the whole library is designed around:
that the published ESM imports in a browser with NO BUNDLER. Node importing
dist/index.js proved nothing — Node is not a browser. Here Chromium fetches
index.js, which imports ./store.js, which imports ./merge.js, and one bare
specifier or one missing extension would leave the page blank.
Three more things only a browser can prove, and all three are now asserted:
- native EventSource works at all. It is the same-origin session-cookie path
and the v1 baseline (spec §7); fetch cannot test it, being a different
transport.
- a read-only region actually FLASHES. "Read-only is not ignored" is the
product thesis, and it is a DOM fact, not an array of paths.
- a user can type into a field while the server changes the object underneath
them, and keep what they typed — the three-way merge, from the only end that
matters.
Six specs: the bundler-free import, status-flashes-while-you-edit, a conflict
raised and then clearing on convergence, an unrelated change not disturbing an
edit, a masked Secret with no input to type into, and an absent named object
rendering empty rather than as a ghost. Mutation-checked: deleting the flash from
applyServerEvent turns the status test red.
cmd/replay now serves the example and the built ESM from the SAME ORIGIN as the
stream. Not for convenience — same-origin is the deployment the protocol is
specified around, and a demo on a second port would need CORS and would then be
proving something we do not ship.
Playwright lives in the example's own package, so packages/krm-stream keeps its
three devDependencies and its zero-dependency promise intact.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Each rung replays the same fixtures with one more thing really true. Also states the rule that actually earns the suite its keep: a green test proves nothing until you have watched it go red. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… say so when the upstream lies
You were right that I under-read the page. That "may not parse as a decimal"
caveat is the PRE-1.35 world, and it is scoped to extension/aggregated API
servers. The sentence that actually governs the design is:
"Starting with Kubernetes 1.35, orderability of resource versions for all
Kubernetes types is included in Certified Kubernetes requirements. Base API
objects AND custom resources MUST be orderable as a monotonically increasing
integer for any 1.35+ APIServer implementation in order to pass conformance
tests."
So on a supported cluster an unorderable resourceVersion cannot occur — and my
"return 0, drop nothing" fallback silently degraded on EVERY cluster to
accommodate a case a conformant one cannot produce. Per-object monotonicity is a
promise the protocol makes to consumers (§6); quietly ceasing to keep it is worse
than refusing.
Gateway.Ordering (ResourceVersionOrdering):
OrderingStrict (default, zero value) — this library targets 1.35+. Every
resourceVersion must be an orderable decimal. One that is not means the
upstream is not what we were told it is, so: a TERMINAL INTERNAL error that
NAMES THE ESCAPE HATCH, rather than a silent loss of the guarantee.
OrderingLenient — for the two cases the docs do carve out, and both are real: a
cluster older than 1.35, and an aggregated/extension API server, which is a
third-party implementation the conformance test does not cover. There ordering
is genuinely undefined, so we do not pretend to it and we DROP NOTHING: a
duplicate is harmless (apply is idempotent by construction), a wrongly-dropped
update is data loss.
New fixture resourceversion-unorderable pins the refusal — note it lands AFTER
`reset`, because the gateway only learns what the upstream's resource versions
look like when the first object arrives. That exposed a gap in both framing
tests, which assumed a stream may only end mid-cycle by dying: a TERMINAL error is
by definition the last event on the connection (spec §4.3) and may perfectly well
arrive mid-snapshot. Both loaders now say so, and both now also assert that
nothing follows a terminal error.
Also: five mermaid diagrams — the system as a whole and the snapshot cycle
(README), the test ladder (CONTRIBUTING), and the two that are genuinely hard to
hold in your head as prose (docs/client-state-model): the two layers of
applyServerEvent, and the per-path merge decision, where every branch turns on the
one question the naive implementations never ask — did the SERVER move this? All
five validated against the real mermaid parser in Chromium, which caught one that
GitHub would have rendered as a red error box (semicolons are statement separators
in a sequenceDiagram).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…on a real aggregated API
Two corrections, and the second was a real honesty bug in the corpus.
1. The README now REQUIRES Kubernetes 1.35+, and says why. From 1.35,
resourceVersion orderability is a Certified Kubernetes conformance requirement
— for base objects AND custom resources — and that is what lets the gateway
promise a browser something no naive watch relay can: per-object monotonicity.
It is what makes coalescing safe (a controller rewriting status at 200Hz NEEDS
coalescing) and what lets a stale replay after a relist be dropped instead of
flickering the UI backwards. Ordering is a real dependency, so Gateway.Ordering
is a documented setting rather than an implementation detail.
2. The resourceVersion fixtures used a ConfigMap, and that was teaching a lie.
They now use a Flunder — wardle.example.com/v1alpha1, Kubernetes' own
sample-apiserver, the same aggregated API that gitops-reverser's e2e already
stands up.
Because on a conformant 1.35 cluster:
- a ConfigMap, a Deployment or a CRD CANNOT serve an unorderable
resourceVersion. Orderability is a conformance requirement. A fixture using
one was defending against a scenario that cannot happen.
- kube-apiserver's resourceVersion is an etcd revision: an int64, 19 digits.
You will never meet the docs' 40-digit example there either. A different
backing store is where such a value actually comes from.
- an AGGREGATED / extension API server is a third-party implementation the
conformance test does not cover, and is the only case the docs' equality-only
carve-out is written for. It is the honest home for both fixtures — and
therefore the honest home of OrderingLenient.
A fixture that teaches a real rule with an impossible example is worse than no
fixture: it makes the reader trust a mental model that will mislead them the
next time they meet the real thing.
Both fixtures mutation-checked against a Flunder body: a plain lexicographic
compare (the naive fix once you learn int64 overflows — it reads 41 digits as
OLDER than 40) fails resourceversion-bignum, and a strict mode that degrades
silently instead of refusing fails resourceversion-unorderable.
external/ is gitignored: it is a sibling repo checked out for reference, not ours.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Facts first, backend second. Everything the gateway believes about Kubernetes is currently unverified — most importantly that a streaming list's terminating bookmark really carries k8s.io/initial-events-end, which is the snapshot boundary and appears nowhere in the API docs. So the first deliverable is a fact-finder that writes its answers into docs/facts/, not a test suite. Finding out that assumption is false AFTER building gateway/kube around it would be the expensive way round. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…swer
Phase B of proposal 0002. `task cluster-up` (k3d, k3s v1.36.2, --cluster-init so
the store is real etcd) and `task cluster-facts` — a witness, not a test suite. It
opens real watches, records what actually arrived, and writes the answers into
docs/facts/observed-<version>.md, stamped with a cluster version.
client-go lives in its own module (gateway/kube), so `go get .../gateway` is
still zero-dependency: the protocol core stays clean and the Kubernetes adapter is
opt-in. Verified — the core go.mod has no require block at all.
WHAT THE CLUSTER SAID
F1 ✅ CONFIRMED, and it was the single load-bearing assumption in the gateway: a
streaming list really does end with a BOOKMARK carrying
k8s.io/initial-events-end="true". That annotation appears NOWHERE in the API
documentation. If it had been wrong, `synced` would fire at the wrong moment —
or never, and the browser would never paint. It was faith; now it is a fact.
The docs are also slightly wrong in a way we only learned by looking: they say a
bookmark's object "only includes a .metadata.resourceVersion field", but the real
one also carries metadata.annotations (it must — that is where the marker lives).
What it does NOT carry is a uid, which is what the partial-object guard actually
keys on. The guard is right, for a reason one shade more precise than the
sentence it was written from.
F3 ✅ a 410 arrives on an open watch as watch.Error, code=410 reason="Expired",
message "too old resource version: 1 (2)".
F4 ✅ resourceVersions are orderable decimals (4 digits here — an int64 parse would
have worked, which is exactly the trap: it works until it doesn't).
F5 ✅ a real Deployment controller emits status-only MODIFIEDs — 5 of them, spec
byte-identical every time, readyReplicas climbing to the requested count. The
product's headline is real traffic.
F7 ✅ a kube-apiserver DELETED is complete and carries a uid. The degenerate
tombstone we guard against comes from client-go's INFORMER, not the API server —
which is why that fixture can only ever be a fake-watch one.
F6 ❗ AND THIS ONE CHANGES THE DESIGN. Pointed at a real aggregated API (Kubernetes'
own sample-apiserver — the wardle Flunder our resourceVersion fixtures already
model), the streaming list of gateway/README §3a is REFUSED outright:
sendInitialEvents is forbidden for watch unless the WatchList feature gate is
enabled
An aggregated API server is a separate binary with its own feature gates.
WatchList being on in kube-apiserver says nothing about it. So §3b's
list-then-watch is NOT "a fallback for old clusters" — it is the only way to
serve an aggregated API on a current one, and a gateway implementing only §3a
cannot open a stream for a Flunder at all. §3a and §3b are corrected to say so.
Its resourceVersions also live in their own space, starting at ~1, in an
ephemeral sidecar etcd — so they can go BACKWARDS across a restart. Per-object
monotonicity survives that only because the high-water map is per-CYCLE rather
than per-stream. That decision was made for a different reason; this is the
cluster confirming it.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Phase A of proposal 0002. `gateway/kube` is the only code here that knows what an API server is,
and it stays a separate module so `go get .../krm-stream/gateway` remains genuinely
zero-dependency: the protocol is the product, and client-go is opt-in.
It ships BOTH paths, because the cluster proved that is not a luxury. F6 pointed the §3a streaming
list at a real aggregated API — Kubernetes' own sample-apiserver — and it was refused outright
("sendInitialEvents is forbidden ... unless the WatchList feature gate is enabled"). An aggregated
API server is a separate binary with its own feature gates, so §3b's list-then-watch is not a
fallback for old clusters: it is the ONLY way to serve an aggregated API on a current one. A gateway
implementing only §3a could not open a stream for a Flunder at all — a bug a user would have found,
in their cluster, and we would not.
So the backend DETECTS rather than configures: nobody should have to know which of their APIs is
aggregated in order to watch it. The refusal is remembered per GroupVersion (it is a fact about a
server binary, not about a request), and it is strictly the sendInitialEvents refusal that triggers
the fallback — a Forbidden on Secrets must stay a Forbidden on Secrets, not become a second,
differently-worded denial.
What both paths have in common is the only thing the stream loop cares about: added* terminated by a
bookmark whose InitialEventsEnd is set. On §3a the API server hands us that boundary; on §3b we
synthesize it. That is exactly why the protocol names the boundary and not the mechanism.
Tested three ways, because a stub is a thing we wrote and it will agree with us:
- 8 unit tests against a stubbed dynamic client — chosen over client-go's fake, whose watch reactor
cannot see SendInitialEvents at all, and the exact ListOptions ARE the thing under test;
- both load-bearing behaviours mutation-checked (drop the refusal detection, or open the fallback
watch at the freshest rv instead of the list's, and the suite goes red);
- `task test-cluster`: the REAL stream loop over the REAL backend against a REAL API server, on
both paths. The aggregated test first asserts the API still REFUSES §3a, so a cluster that
quietly gains WatchList cannot make it pass for the wrong reason, and then asserts the gateway
serves the Flunder anyway.
Two things fell out on the way:
- the kube module was in NO ci job and no task — a separate module is exactly what `go test ./...`
does not cross, so the adapter would have shipped untested. Now gated, with the e2e tag vetted
per-PR even though the suite itself runs outside CI: a test suite that has quietly stopped
compiling is one nobody notices has stopped running.
- gosec found the fact-finder building a file path out of a string the SERVER chose (GitVersion).
A cluster we do not own should not get to pick where we write. Sanitized.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe change adds the Go gateway, Kubernetes adapter, TypeScript live store and SSE client, conformance fixtures, replay and browser demos, authorization seams, real-cluster checks, and reusable CI/release workflows. ChangesLive stream and conformance stack
Estimated code review effort: 5 (Critical) | ~120 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
A server could ACCEPT sendInitialEvents and then quietly ignore it — no synthetic ADDEDs, no terminating bookmark, so `synced` never fires and a browser never paints. There is no honest guard: the only one available is a timeout, and "no bookmark in N seconds" makes N a guess that turns a slow cluster into a corrupt one. What we have instead is a stated environment (Kubernetes 1.35+), where the option is not silently droppable. A server that accepts an option and ignores it is broken in a way that is not ours to paper over — and the right answer to a broken upstream is to be diagnosable, not to guess. Say so in the package doc rather than leave the next reader to wonder whether we forgot. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…istry
The browser rung failed on its first-ever CI run, and the reason is worse than a broken build.
Playwright's webServer rebuilds the library before serving it (a stale dist is a test of last week's
library). It did that with a bare `npx tsc`, from a directory that on a CLEAN CHECKOUT has no
node_modules. Bare npx does not fail there. It goes to the registry, downloads whatever is published
under the name `tsc` — a squatter package, not TypeScript — and RUNS it:
[WebServer] npm warn exec The following package was not found and will be installed: tsc@2.0.4
[WebServer] This is not the tsc command you are looking for
So the job did not merely fail to build; it executed an unrelated package off the internet and then
failed to build. It announced itself only because the squatter happens to exit non-zero. A useful one
would have been silent.
This survived because on a developer's machine packages/krm-stream/node_modules is already there —
`task test` puts it there — and because CI ran on main only, where the browser rung does not exist
yet. The first clean checkout to run it found it, which is the argument for opening the PR early.
Two fixes, and the first is the one that matters:
- `npx --no-install` everywhere a build shells out to a tool, so a missing dependency is LOUD
rather than resolved from the network. The repo already used --no-install in lint-client; the
playwright config and build-client are simply where the convention had a hole.
- `task e2e-browser` now depends on build-client, so the library's devDependencies are actually
installed before the webServer tries to compile it.
Verified by reproducing CI's state rather than trusting the diff: rm -rf packages/krm-stream/
{node_modules,dist} && task e2e-browser → 6 passed.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…pt us
Caught in review, reproduced, and it is the worst kind of bug: every test in this repo passed while
`go get github.com/ConfigButler/krm-stream/gateway/kube` was broken for literally everyone else.
gateway/kube/go.mod carried `require .../gateway v0.0.0` plus `replace ... => ../`. A replace applies
ONLY to the main module — a consumer ignores it, resolves the core module at the literal version in
the require line, and gets:
reading github.com/ConfigButler/krm-stream/gateway/go.mod
at revision gateway/v0.0.0: unknown revision gateway/v0.0.0
The core module resolves fine. It is precisely the Kubernetes adapter — the one thing an adopter with
a real cluster actually needs — that could not be fetched. Reproduced from a clean throwaway module
before changing anything, because a bug you cannot reproduce is a bug you cannot claim to have fixed.
The fix is the standard one:
- the require now names a REAL, resolvable version of the core module (a pseudo-version today; it
becomes gateway/v0.1.0 the moment we tag);
- go.work at the root does locally what the replace was doing — and consumers never see a go.work,
which is the entire difference;
- a CI job, `a stranger can go get this`, builds from a clean module in a temp directory OUTSIDE
the workspace, resolving over the network exactly as an adopter would.
That last part is the real repair. Nothing INSIDE this repository could ever have caught this: the
replace hid it from every test we have, by construction. A test that lives in the same tree as the
bug it is meant to catch is not a test. The same job also guards the other half of the promise — that
the core module keeps an empty require block, so `go get .../gateway` stays the zero-dependency
protocol and does not quietly start costing what client-go costs.
Also, per review: the sequence diagram now reads left-to-right from the BROWSER, not from Kubernetes.
The reader of that diagram is a frontend developer deciding whether this library is for them, and
starting at the API server tells the story backwards from where they stand.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (12)
packages/krm-stream/src/store.ts-174-183 (1)
174-183: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winReject renaming onto an existing key.
If
newKeyalready exists, object reconstruction silently overwrites either the renamed value or the destination value depending on insertion order.Proposed guard
const map = get(res.draft, path); if (!isPlainObject(map)) throw new Error(`krm-stream: ${pathKey(path)} is not a map`); + if (!Object.hasOwn(map, oldKey)) throw new Error(`krm-stream: key ${JSON.stringify(oldKey)} does not exist`); + if (oldKey !== newKey && Object.hasOwn(map, newKey)) { + throw new Error(`krm-stream: key ${JSON.stringify(newKey)} already exists`); + }🤖 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 `@packages/krm-stream/src/store.ts` around lines 174 - 183, Update renameKey to validate that newKey does not already exist in the map before reconstructing or mutating it; reject the operation with an error when the destination key exists, while preserving the current rename behavior for absent destinations.docs/facts/observed-v1.36.2+k3s1.md-15-15 (1)
15-15: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winFix the generated Flunder resourceVersion contradiction.
The table and F6 result report
"4", while Line 56 says"2". Update the facts generator so this witness consistently records the observed value.Also applies to: 46-46, 56-57
🤖 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/facts/observed-v1.36.2`+k3s1.md at line 15, Update the facts generator for the F6 Flunder resourceVersion witness so every generated reference consistently records the observed value “4”, including the table entry and the lines currently reporting “2”. Use the existing Flunder witness-generation symbols and preserve the surrounding streaming-list result unchanged.gateway/kube/e2e_test.go-72-85 (1)
72-85: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winCreate a genuinely unique scratch namespace.
The deterministic name can survive an interrupted run or still be terminating. Accepting
AlreadyExiststhen reuses stale state and recreates the teardown race this helper intends to prevent.Proposed fix
func scratchNamespace(t *testing.T, cs kubernetes.Interface) string { t.Helper() - ns := "krm-stream-e2e-" + strings.ToLower(strings.NewReplacer("/", "-", "_", "-").Replace(t.Name())) + prefix := "krm-stream-e2e-" + strings.ToLower(strings.NewReplacer("/", "-", "_", "-").Replace(t.Name())) + "-" ctx := context.Background() - _, err := cs.CoreV1().Namespaces().Create(ctx, &corev1.Namespace{ - ObjectMeta: metav1.ObjectMeta{Name: ns}, + created, err := cs.CoreV1().Namespaces().Create(ctx, &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{GenerateName: prefix}, }, metav1.CreateOptions{}) - if err != nil && !apierrors.IsAlreadyExists(err) { - t.Fatalf("create namespace %s: %v", ns, err) + if err != nil { + t.Fatalf("create namespace with prefix %s: %v", prefix, err) } + ns := created.Name t.Cleanup(func() { _ = cs.CoreV1().Namespaces().Delete(context.Background(), ns, metav1.DeleteOptions{}) })🤖 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 `@gateway/kube/e2e_test.go` around lines 72 - 85, Update scratchNamespace to generate a genuinely unique namespace name for each test run, rather than deriving it solely from t.Name(). Do not accept AlreadyExists as success: create the generated namespace and fail on any creation error, while preserving the existing cleanup behavior.gateway/kube/go.mod-11-17 (1)
11-17: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick winBump the Go toolchain pin
go 1.26.0is already behind the 1.26.x patch line; use the latest 1.26.x release so the build picks up the known security fixes.k8s.io/client-go v0.36.0matches Kubernetes v1.36.0.🤖 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 `@gateway/kube/go.mod` around lines 11 - 17, Update the Go version directive in go.mod from 1.26.0 to the latest available 1.26.x patch release, leaving the Kubernetes dependency versions unchanged.packages/krm-stream/src/index.ts-7-7 (1)
7-7: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRemove the stale “not built yet” note.
Line 7 contradicts the implemented export at Line 22.
Proposed fix
-// connectResourceStream a conforming SSE consumer that feeds a store. (not built yet) +// connectResourceStream a conforming SSE consumer that feeds a store.🤖 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 `@packages/krm-stream/src/index.ts` at line 7, Remove the stale “not built yet” wording from the connectResourceStream comment, keeping the existing description aligned with the implemented export.conformance/README.md-100-100 (1)
100-100: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRoutine bookmarks are permitted, not guaranteed.
Kubernetes does not promise a bookmark on every watch that requests them. The object may also contain type metadata, so “only metadata.resourceVersion” is too strict. Document that routine bookmarks may arrive and must be absorbed.
🤖 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 `@conformance/README.md` at line 100, Update the bookmark row in the conformance documentation to state that routine bookmarks may arrive but are not guaranteed, and that bookmark objects can include type metadata alongside metadata.resourceVersion. Preserve the requirement to absorb bookmarks without forwarding them or treating them as synced.docs/proposals/0002-real-cluster.md-45-82 (1)
45-82: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUpdate the proposal body to match its landed status.
The table still marks F1–F6 as unverified, and Phase A says client-go and the backend do not exist, contradicting Lines 3–27. Mark the observed results and rewrite this section as the landed design—or label the remainder explicitly as historical.
🤖 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/proposals/0002-real-cluster.md` around lines 45 - 82, Update docs/proposals/0002-real-cluster.md to reflect the landed implementation: replace the unverified F1–F6 statuses with the observed results, and revise Phase A to describe the existing client-go KubeBackend and Watch adapter rather than proposing them as missing work. Preserve historical proposal context only if clearly labeled as historical.docs/client-state-model.md-223-224 (1)
223-224: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAlign the array description with the algorithm below.
Arrays are not always atomic here: Lines 236–240 recurse positionally when clean and equal-length, then fall back to atomic handling on dirtiness or length changes. Either qualify this sentence or change the algorithm to be fully atomic.
🤖 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 223 - 224, The array description in section 4.1 conflicts with the positional recursion described in the algorithm. Update the sentence near the array/scalar branch to qualify that arrays are atomic only when dirty or their lengths differ, while clean equal-length arrays recurse positionally; keep the existing algorithm unchanged.conformance/README.md-87-102 (1)
87-102: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDo not characterize every fixture operation as API-server behavior.
disconnectis an SSE/browser condition, whileDeletedFinalStateUnknownis informer-cache behavior rather than an API-server watch event. Reword the introduction to say these operations model conditions handled across the gateway pipeline.🤖 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 `@conformance/README.md` around lines 87 - 102, The introduction before the operation table overstates that every fixture operation is direct API-server behavior. Reword the paragraph to describe the operations as modeling conditions handled across the gateway pipeline, while retaining the existing references to Kubernetes API concepts and the distinction from assumptions based on memory.packages/krm-stream/src/merge.ts-69-74 (1)
69-74: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
mergeContainermishandles non-objecttheirswhenbaseoroursis an object.When
theirsis not a plain object (e.g.,null, a string, or a number) butbaseoroursis, the condition on line 70 evaluates false and the function recurses.recursethen iterates only keys frombase/ours(sinceunionKeysskips non-objects), producing{}or a partial object instead of the server's actual value. On save, the patch would send{}where the server hasnull, potentially creating an unwanted empty object.The fix is to follow the server whenever
theirsis not a plain object — a container with no object from the server has no editable children to recurse into.🐛 Proposed fix for mergeContainer
function mergeContainer(s: MergeState, path: Path, base: unknown, ours: unknown, theirs: unknown): unknown { - if (!isPlainObject(base) && !isPlainObject(ours) && !isPlainObject(theirs)) { - return follow(s, path, base, theirs); - } + if (!isPlainObject(theirs)) return follow(s, path, base, theirs); return recurse(s, path, base, ours, theirs); }🤖 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 `@packages/krm-stream/src/merge.ts` around lines 69 - 74, Update mergeContainer so it immediately follows the server value via follow when theirs is not a plain object, regardless of whether base or ours are objects; only recurse when theirs is a plain object. Preserve the existing follow arguments and recursive merge behavior for object-valued server containers.docs/facts/kubernetes-api-concepts.md-268-268 (1)
268-268: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winDistinguish routine bookmarks from the snapshot-boundary bookmark.
This row says nothing may depend on bookmark arrival, while Lines 142–157 make the requested initial-events bookmark the snapshot boundary. Qualify this as applying to routine bookmarks; otherwise the summary contradicts the protocol design.
🤖 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/facts/kubernetes-api-concepts.md` at line 268, Update the row describing bookmark arrival in the Kubernetes API concepts table to explicitly scope the restriction to routine bookmarks, while preserving the terminating initial-events bookmark as the snapshot boundary defined in the surrounding protocol discussion..github/workflows/ci.yml-193-210 (1)
193-210: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick winSet
persist-credentials: falseon checkouts that don't push (esp. the artifact-uploadingbrowserjob).This job persists the
GITHUB_TOKENin.git/configand later uploadstest-results/as an artifact on failure, which is the exfiltration pattern zizmor flags (artipacked). None of the new jobs push, so credential persistence is unnecessary here and on the other checkouts (Lines 59, 88, 171).🔒 Suggested change
- uses: actions/checkout@v6 + with: + persist-credentials: false🤖 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 @.github/workflows/ci.yml around lines 193 - 210, Update every checkout step in the CI workflow, including the browser job checkout near setup-go/setup-node and the other checkouts referenced at lines 59, 88, and 171, to set actions/checkout’s persist-credentials option to false. Keep the existing checkout behavior otherwise unchanged.Source: Linters/SAST tools
🧹 Nitpick comments (1)
packages/krm-stream/e2e/wire.ts (1)
70-99: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftMake the socket test observe incremental delivery.
A server or proxy buffering every frame until EOF would still pass these assertions. Add a gated second frame and assert the first callback occurs before release/closure; also fragment a frame across writes to exercise chunk boundaries.
Also applies to: 108-119
🤖 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 `@packages/krm-stream/e2e/wire.ts` around lines 70 - 99, Add incremental-delivery coverage to the socket test around connectResourceStream and handle.closed: make the stream emit a second frame behind a controllable gate, assert the first callback occurs before the gate is released or the connection closes, and release it only afterward. Fragment at least one frame across multiple writes so parsing is exercised across chunk boundaries, while preserving the existing fixture assertions.
🤖 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 @.github/workflows/ci.yml:
- Around line 104-133: Update the clean-module go get gate to avoid resolving
fork-only head SHAs from the canonical module path: skip this step for pull
requests whose head repository differs from the base repository, or use the pull
request’s merge commit instead. Preserve the existing validation for
same-repository pull requests and push events.
In `@examples/vanilla-browser/index.html`:
- Around line 143-147: Update the onInput handlers in the editable-value flow to
preserve the original JSON type for arrays, booleans, null, and numbers instead
of storing non-numeric input as strings. Use the existing value and parsing
logic around store.setValue(id, path, next) to convert valid edits back to the
corresponding original type, while preserving the current behavior for text
values and empty or invalid input.
In `@gateway/kube/cmd/facts/main.go`:
- Line 61: Update the namespace lifecycle around the namespace flag and its
creation/cleanup logic: do not accept an AlreadyExists result for the configured
namespace, instead fail on collision or generate a unique namespace name.
Register deferred deletion only after this process successfully creates the
namespace, ensuring pre-existing namespaces and their contents are never
deleted.
- Around line 271-280: Update the watch.Error handling in the facts check to set
f.OK=true only when status.Code is 410 or status.Reason is Expired or Gone. Keep
the existing nil-status response and diagnostic details, but mark unrelated
authorization or server errors as not OK while still returning the finding.
- Around line 489-512: Update the F6 detail construction in the facts command to
derive its narrative from the observed werr, rv, and bookmark values instead of
using fixed claims. Ensure acceptance or rejection of streaming lists, the
returned resourceVersion, and bookmark/reset observations are described
accurately and remain consistent with f.OK and f.Answer.
- Around line 348-353: Update the deployment creation and watch flow around
Deployments(ns).Create and the subsequent Watch call to retain the created
Deployment object. Use its resourceVersion in metav1.ListOptions when starting
the watch, and use its spec as the baseline for validation; preserve existing
AlreadyExists handling while ensuring the watch begins at the creation version.
- Around line 194-207: Update the annotation validation in the finding logic
around InitialEventsAnnotationKey so it only succeeds when the annotation is
present and its value is exactly "true". Treat missing or differently valued
annotations as failure, preserving the existing diagnostic response and bookmark
shape reporting for that path.
In `@gateway/seams.go`:
- Around line 53-59: Update the ClientFor type to accept a context.Context
parameter and propagate that context through every implementation and call site
involved in target resolution or token exchange, allowing cancellation and
deadlines to stop backend acquisition. Preserve the existing target and
principal arguments and Backend/error return contract.
In `@gateway/sse.go`:
- Around line 121-128: Ensure the handler waits for the goroutine started by
Heartbeat before returning: add synchronization around sink.Heartbeat, cancel
its context, and wait for completion after g.Stream returns. Update the defer
flow around stopHeartbeat so the ResponseWriter is never accessed after the
handler exits.
In `@gateway/stream_conformance_test.go`:
- Around line 78-99: Update the stream verification around gw.Stream and
backend.Exhausted so the final connection of a terminal fixture does not pass
merely because Exhausted closes. Track whether this is the last split connection
and fail when it reaches Exhausted, while preserving cancellation and normal
completion checks for earlier connections.
In `@gateway/stream.go`:
- Around line 208-222: Update the WatchDeleted handling around identityOf and
emitted so deletion events pass through isStale using the tombstone
resourceVersion, ignoring stale deletes without emitting or removing state. For
accepted deletions, retain the deletion resourceVersion as the high-water mark
for the rest of the cycle instead of unconditionally deleting emitted[id.UID],
preventing older replays from being accepted.
In `@packages/krm-stream/src/sse.ts`:
- Around line 125-128: Update connectResourceStream and the other stream
connection paths around the referenced signal handling to check
opts.signal.aborted before invoking fetch or constructing EventSource. Abort the
internal controller and prevent stream establishment when the caller’s signal is
already aborted, while preserving the existing one-time abort listener for
signals that abort later.
- Around line 30-39: Update the buffer normalization in the SSE stream
processing loop before frame separator detection so a trailing bare \r at the
end of the current chunk remains buffered until the next chunk arrives, while
still normalizing complete \r\n and standalone \r line endings. Preserve correct
\n\n frame detection and multi-line data event handling in the logic around the
buffer and parseFrame.
In `@packages/krm-stream/src/store.ts`:
- Around line 243-248: Update isEditable and the related editability handling
around the code at lines 278–295 so paths that are ancestors of any redacted
path are also treated as non-editable. Ensure checking `/data` is blocked when
`/data/token` is redacted, preventing edits or generated patches from
overwriting hidden descendants while preserving editability for unrelated paths.
- Around line 134-147: Update adoptSaved to preserve edits made after the save
request was submitted: compare the current draft with the submitted snapshot and
only adopt server values for fields unchanged since submission, retaining newer
local edits and recalculating conflicts accordingly. Use the existing
save-submission state or snapshot mechanism rather than overwriting
existing.draft unconditionally.
---
Minor comments:
In @.github/workflows/ci.yml:
- Around line 193-210: Update every checkout step in the CI workflow, including
the browser job checkout near setup-go/setup-node and the other checkouts
referenced at lines 59, 88, and 171, to set actions/checkout’s
persist-credentials option to false. Keep the existing checkout behavior
otherwise unchanged.
In `@conformance/README.md`:
- Line 100: Update the bookmark row in the conformance documentation to state
that routine bookmarks may arrive but are not guaranteed, and that bookmark
objects can include type metadata alongside metadata.resourceVersion. Preserve
the requirement to absorb bookmarks without forwarding them or treating them as
synced.
- Around line 87-102: The introduction before the operation table overstates
that every fixture operation is direct API-server behavior. Reword the paragraph
to describe the operations as modeling conditions handled across the gateway
pipeline, while retaining the existing references to Kubernetes API concepts and
the distinction from assumptions based on memory.
In `@docs/client-state-model.md`:
- Around line 223-224: The array description in section 4.1 conflicts with the
positional recursion described in the algorithm. Update the sentence near the
array/scalar branch to qualify that arrays are atomic only when dirty or their
lengths differ, while clean equal-length arrays recurse positionally; keep the
existing algorithm unchanged.
In `@docs/facts/kubernetes-api-concepts.md`:
- Line 268: Update the row describing bookmark arrival in the Kubernetes API
concepts table to explicitly scope the restriction to routine bookmarks, while
preserving the terminating initial-events bookmark as the snapshot boundary
defined in the surrounding protocol discussion.
In `@docs/facts/observed-v1.36.2`+k3s1.md:
- Line 15: Update the facts generator for the F6 Flunder resourceVersion witness
so every generated reference consistently records the observed value “4”,
including the table entry and the lines currently reporting “2”. Use the
existing Flunder witness-generation symbols and preserve the surrounding
streaming-list result unchanged.
In `@docs/proposals/0002-real-cluster.md`:
- Around line 45-82: Update docs/proposals/0002-real-cluster.md to reflect the
landed implementation: replace the unverified F1–F6 statuses with the observed
results, and revise Phase A to describe the existing client-go KubeBackend and
Watch adapter rather than proposing them as missing work. Preserve historical
proposal context only if clearly labeled as historical.
In `@gateway/kube/e2e_test.go`:
- Around line 72-85: Update scratchNamespace to generate a genuinely unique
namespace name for each test run, rather than deriving it solely from t.Name().
Do not accept AlreadyExists as success: create the generated namespace and fail
on any creation error, while preserving the existing cleanup behavior.
In `@gateway/kube/go.mod`:
- Around line 11-17: Update the Go version directive in go.mod from 1.26.0 to
the latest available 1.26.x patch release, leaving the Kubernetes dependency
versions unchanged.
In `@packages/krm-stream/src/index.ts`:
- Line 7: Remove the stale “not built yet” wording from the
connectResourceStream comment, keeping the existing description aligned with the
implemented export.
In `@packages/krm-stream/src/merge.ts`:
- Around line 69-74: Update mergeContainer so it immediately follows the server
value via follow when theirs is not a plain object, regardless of whether base
or ours are objects; only recurse when theirs is a plain object. Preserve the
existing follow arguments and recursive merge behavior for object-valued server
containers.
In `@packages/krm-stream/src/store.ts`:
- Around line 174-183: Update renameKey to validate that newKey does not already
exist in the map before reconstructing or mutating it; reject the operation with
an error when the destination key exists, while preserving the current rename
behavior for absent destinations.
---
Nitpick comments:
In `@packages/krm-stream/e2e/wire.ts`:
- Around line 70-99: Add incremental-delivery coverage to the socket test around
connectResourceStream and handle.closed: make the stream emit a second frame
behind a controllable gate, assert the first callback occurs before the gate is
released or the connection closes, and release it only afterward. Fragment at
least one frame across multiple writes so parsing is exercised across chunk
boundaries, while preserving the existing fixture assertions.
🪄 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: ac113ca2-436b-4dc2-88d2-56968b108ad4
⛔ Files ignored due to path filters (22)
conformance/gen/bodies.jsonis excluded by!**/gen/**conformance/gen/fixtures.jsonis excluded by!**/gen/**conformance/gen/sse/bookmark-absorbed.sseis excluded by!**/gen/**conformance/gen/sse/conflict-and-converge.sseis excluded by!**/gen/**conformance/gen/sse/delete-recreate-uid.sseis excluded by!**/gen/**conformance/gen/sse/edit-vs-unrelated-change.sseis excluded by!**/gen/**conformance/gen/sse/key-removed-upstream.sseis excluded by!**/gen/**conformance/gen/sse/named-object-absent.sseis excluded by!**/gen/**conformance/gen/sse/nested-field-removed.sseis excluded by!**/gen/**conformance/gen/sse/partial-object-refused.sseis excluded by!**/gen/**conformance/gen/sse/reconnect-prune.sseis excluded by!**/gen/**conformance/gen/sse/resourceversion-bignum.sseis excluded by!**/gen/**conformance/gen/sse/resourceversion-unorderable.sseis excluded by!**/gen/**conformance/gen/sse/resync-midstream.sseis excluded by!**/gen/**conformance/gen/sse/secret-redaction.sseis excluded by!**/gen/**conformance/gen/sse/snapshot-then-deltas.sseis excluded by!**/gen/**conformance/gen/sse/status-only-churn.sseis excluded by!**/gen/**conformance/gen/sse/tombstone-without-uid.sseis excluded by!**/gen/**examples/vanilla-browser/package-lock.jsonis excluded by!**/package-lock.jsongateway/kube/go.sumis excluded by!**/*.sumgo.workis excluded by!**/*.workpackages/krm-stream/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (63)
.github/workflows/ci.yml.gitignoreCONTRIBUTING.mdREADME.mdTaskfile.ymlconformance/README.mdconformance/bodies/flunder.v1-bignum.yamlconformance/bodies/flunder.v2-bignum-newer.yamlconformance/bodies/flunder.v3-opaque.yamlconformance/fixtures/bookmark-absorbed.yamlconformance/fixtures/partial-object-refused.yamlconformance/fixtures/resourceversion-bignum.yamlconformance/fixtures/resourceversion-unorderable.yamlconformance/fixtures/tombstone-without-uid.yamldocs/client-state-model.mddocs/facts/kubernetes-api-concepts.mddocs/facts/observed-v1.36.2+k3s1.mddocs/proposals/0001-watch-ops.mddocs/proposals/0002-real-cluster.mdexamples/vanilla-browser/README.mdexamples/vanilla-browser/index.htmlexamples/vanilla-browser/package.jsonexamples/vanilla-browser/playwright.config.tsexamples/vanilla-browser/tests/live-krm.spec.tsgateway/README.mdgateway/cmd/replay/main.gogateway/conformance.gogateway/conformance_test.gogateway/event.gogateway/golden_test.gogateway/kube/backend.gogateway/kube/backend_test.gogateway/kube/cmd/facts/main.gogateway/kube/e2e_test.gogateway/kube/go.modgateway/project.gogateway/scripted.gogateway/seams.gogateway/sse.gogateway/stream.gogateway/stream_conformance_test.gogateway/stream_test.gopackages/krm-stream/biome.jsoncpackages/krm-stream/e2e/wire.tspackages/krm-stream/package.jsonpackages/krm-stream/src/deep.tspackages/krm-stream/src/index.tspackages/krm-stream/src/merge.tspackages/krm-stream/src/path.tspackages/krm-stream/src/policy.tspackages/krm-stream/src/sse.tspackages/krm-stream/src/store.tspackages/krm-stream/src/types.tspackages/krm-stream/test/conformance.test.tspackages/krm-stream/test/conformance.tspackages/krm-stream/test/expect.tspackages/krm-stream/test/invariants.test.tspackages/krm-stream/test/store.test.tspackages/krm-stream/test/wire.test.tspackages/krm-stream/tsconfig.test.jsonspec/v1.mdtest/cluster/sample-apiserver/README.mdtest/cluster/sample-apiserver/sample-apiserver.yaml
…write it never had Owner's call, and the repo was quietly disagreeing with itself about it. The corpus declared `gatewayRejects:` in secret-redaction.yaml. conformance.go parsed it into a Reject struct. NOTHING read it. spec §10 made it gateway conformance rule 10 — "it rejects a save that would write a redacted or projection-removed path" — and the README advertised that the library "applies saves as a guarded patch (it will refuse one that touches a redacted path)". None of that was true. There is no write path in this library, and there is not going to be one: a change reaches the cluster through the Kubernetes API from the host's own server, as it always did. krm-stream makes the current state streamable and mergeable; it is not a second, thinner API server. So: the Reject struct, the fixture block, the gateway conformance rule and the README claim are gone. A conformance rule the gateway cannot execute is not a rule, and a struct tag is not an implementation. The RULE did not disappear — it moved to where it can actually be enforced, and it is now stated far more plainly than it was when it was pretending to be ours (spec §3): The endpoint that accepts a save MUST reject any patch touching a redacted or projection-removed path. The consumer never saw the real value of a masked field; it saw a mask. A patch carrying that mask back writes `••••••` OVER the real Secret, and the object is destroyed by a save that looked, from the browser, entirely ordinary. And the part that is easy to get wrong, so it is now written down: a conforming client will not produce such a patch — `patch()` never emits a redacted path, and the browser suite proves it — but that is a NECESSARY condition, not a sufficient one. The client is not the security boundary. A hostile client is just a client. The check belongs on the server that performs the write, and that server is yours. What the corpus can prove, it still proves: the mask is not editable and never reaches the patch. What it cannot prove is someone else's endpoint, and it no longer pretends to. Also per review: the README's architecture diagram drew the browser PATCHing the stream loop, which is precisely the thing that is not true. The save handler is now drawn where it lives — in YOUR server, dashed, going straight to the API server, labelled with the duty it carries. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…of the wire Two review items, and they are one problem: the first ten minutes of an adopter's life. **#4 — the README called an API that did not exist.** The first Go block in the project README was `gateway.Handler(gateway.Options{...})`. Neither identifier was real. What existed was ServeStream(w, r, principal, scope) — a fine seam, but it left every host to write the same four things by hand: parse the scope out of the query, decide what a principal is, check the scope against an allowlist, wire the three together. Four chances to get security-relevant glue subtly wrong, once per adopter. So the library now ships the glue and the README compiles. Handler's one non-obvious decision, now pinned by a test: every refusal — malformed scope, disallowed scope, unidentifiable caller — is a TERMINAL SSE error event over a 200, not an HTTP status code. A 403 is the obvious thing and it is wrong: EventSource cannot read the body of a non-200, so it reaches the page as an `onerror` with no code, no message and no reason. `terminal` is also what stops the browser reconnecting to a scope that can never become valid. ScopePolicy is deny-by-default. The zero value streams NOTHING, so a host that forgets to configure it fails closed — rather than serving Secrets from every namespace in every cluster it can reach, which is what the friendlier default does the first time someone copy-pastes a README. Missing seams PANIC at mount time, on the first line of main(), not on a request from a real user hours later. **#3 — the request half of the wire was never specified.** §8 named the scope's FIELDS and never their ENCODING, so the replay server invented `?group=&version=&resource=`, the client's README invented a URL string, and nothing compared them. Two ends of one repository, free to disagree, with a green suite — which is the exact failure the corpus exists to make impossible for the RESPONSE half. So the request half gets the same treatment: conformance/scopes.yaml is read by BOTH suites. The client asserts resourceStreamURL() produces `canonical`; the gateway asserts ScopeFromQuery() accepts that same `canonical` and yields `scope`. The client's output is fed byte-for-byte into the server's parser, and neither can drift without the other's suite going red. (Mutation-checked: swap two fields in the client's builder — a plausible, harmless-looking reorder — and the TS suite goes red.) Two rules in there do real work: - `resource` and `version` are REQUIRED, never defaulted. A defaulted version streams objects of a shape the caller did not ask for; a defaulted resource streams the wrong objects entirely. - An API-server address is REFUSED, not ignored. It would be easy to satisfy "the browser never supplies an API-server URL" by simply having no parameter for it — but then `?server=https:// 10.0.0.1:6443` is silently dropped and an SSRF probe is indistinguishable from an ordinary request. Refusing loudly turns a security property into an observable one. Ditto kubeconfig, token, endpoint. And `../../kube-system` is not a namespace: every field is validated as the DNS name it actually is, at the edge, before anything downstream is entitled to assume it was. One thing the tests taught me, recorded where the next person will hit it: a stub Watcher that returns ErrWatchClosed immediately makes the gateway spin fresh snapshot cycles forever (a closed watch means "reopen"). It ran for 164 seconds before the test binary was killed. The stream loop was behaving exactly as designed; the stub was the liar. A real API server idles. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Review item #5. Ten tabs on the same namespace were ten watches on the API server, ten snapshots and ten copies of the same object graph — and a reconnect storm (a floor of laptops closing) multiplied it. SharedBackend makes them one watch and one warm cache: a joining subscriber gets its entire reset…synced from memory, and the API server never hears about it. It is a Backend like any other, so it wraps whichever of §3a/§3b is underneath and the stream loop cannot tell the difference. That is what "the protocol is backend-agnostic" has to mean if it means anything — and it is why this could be slid in behind the existing seams rather than through them, exactly as the reviewer said it could be. **It is opt-in, and the reason is not performance.** A shared watch is opened once, so it is opened as ONE identity. Without sharing, ClientFor hands the gateway a client acting AS the caller and Kubernetes' own RBAC is the enforcement — no bug in this library can show someone objects they may not see. With sharing, the upstream runs as your service account and every subscriber reads from its cache, so YOUR Authorizer becomes the only thing between a caller and the objects. A bug there is not a bug, it is a disclosure. That is a choice about someone's threat model, and the library does not get to make it — it only refuses to make it silently. The package comment says so in those words. Three things this had to get right, each one tested and each one a real failure mode: - **A slow subscriber must not stall the pump**, and therefore everyone else. The queue is bounded; overflow resnapshots that consumer from the warm cache (costing the API server nothing) instead of blocking, and instead of the unbounded queue that turns one paused tab into unbounded memory for every other tenant. Slowness degrades into a resnapshot, never into a leak and never a lie. - **A partial object is refused AT THE CACHE**, not only at the wire. The stream loop already guards its own output — but this cache is REPLAYED to every future joiner. A husk forwarded once blanks one consumer's object; a husk cached is served to everybody who arrives later, for as long as the scope lives. Same for a uid-less tombstone: guessing would evict the wrong object from a cache that is then handed to everyone. - **N subscribers resyncing at once must not stampede** the API server into N new watches. They get one. The tests taught me one thing I had wrong, and the fix is now in a comment where the next person will need it: the overflow error was being pushed INTO the subscriber's channel — which cannot work, because the case where we need to send it is exactly the case where the channel is full. The reason was silently dropped and the consumer saw a bare close. It still RECOVERED (a closed watch means resnapshot), so every test passed — but nothing anywhere could say why, which is the difference between a system you can operate and one you can only restart. The reason now lives beside the queue. -race is now a permanent gate, in the Taskfile and in CI. The gateway has a pump goroutine, a per-scope cache and N bounded queues; a data race in that is a browser being shown another tenant's object, not a flaky test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…stays a floor Review item #6, both halves. **The version stamp.** This library is publishable to npm, but it is also VENDORED — copied, as built ESM, into a host that serves it to a browser (that is exactly what the replay server's --dist flag does). A vendored asset drifts, and it drifts SILENTLY: an old client and a new gateway agree about every event they both still understand, right up until the wire changes, and then the failure lands in a user's browser rather than in anybody's test suite. So the bytes now carry their provenance: `VERSION` and `PROTOCOL_VERSION` are exported, and a host can assert the copy it vendored against the gateway it is running. Neither number is hand-maintained in the way that rots. `task test` fails if VERSION drifts from package.json, and PROTOCOL_VERSION is checked against the GO CONSTANT THAT DEFINES IT: the gateway writes it into conformance/gen/protocol.json (published by the side that owns the protocol, not copied by hand into the side that consumes it), and the TypeScript suite reads it back. Neither half of this repo can bump the protocol alone. Both guards mutation-checked: bump either number and the suite goes red. **The Kubernetes floor.** k8s.io/* stays on v0.36.0 — the lowest minor we support — and that is now written down as a rule rather than left as an accident. MVS makes the require line a FLOOR, so a consumer already on v0.36.2 keeps it and nothing drags them backwards; but the asymmetry matters: if we chased patch releases, every adopter would be forced up to whatever we last happened to build against. An adapter has no business dictating which Kubernetes libraries its host runs. We sit on the floor; the consumer picks the ceiling. And one trap, fixed where I fell into it: `task fixtures` did not list Taskfile.yml among its sources, so changing WHICH golden tests the generator runs left it cheerfully reporting "up to date" while the new artifact was never written. The command line of a generator is one of its inputs. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…bug the review found
The reviewer asked for an auth stance doc. Writing it found a bug, because a doc that has to be TRUE
is a test of the code.
**The bug.** Authorize() and ClientFor() were each called exactly ONCE, at stream open — and then the
stream ran for as long as the browser stayed connected, reopening upstream watches internally, for
hours. Two consequences, both real:
- **Revocation was never noticed.** Take away a user's access and their open stream kept delivering
objects, from a watch that had been authorized once, possibly hours earlier.
- **A long stream outlived its short token.** An OIDC access token is good for 5–60 minutes; a
dashboard tab is open all afternoon. The credential captured at open was the credential used
forever, and the host had nowhere to hand us a fresh one.
Both seams are now re-consulted on EVERY SNAPSHOT CYCLE. A cycle is the natural checkpoint — it is
where continuity is re-established anyway, so it is where entitlement should be too. A revoked user's
stream now ends with a TERMINAL Forbidden; terminal because EventSource reconnects on its own, so a
non-terminal refusal would leave a revoked user hammering a forbidden scope forever. And ClientFor is
re-invoked there, which is the seam a host hangs credential refresh on.
Mutation-checked, and the mutant is instructive: restore the old "authorize once at open" and the
revoked-user test does not merely fail, it HANGS — the stream never ends, which is exactly the bug
stated as a symptom.
**The doc** (docs/auth.md) is short and takes one route, as asked: OIDC via Dex.
Everything in it falls out of one physical constraint, stated first because nothing else makes sense
without it: a browser's EventSource CANNOT SEND AN AUTHORIZATION HEADER. So a browser-held token
cannot ride on a native SSE request at all. Therefore the browser logs in to YOUR server, your server
custodies the token, and the SSE request carries nothing but a same-origin HttpOnly cookie — nothing
an XSS can read. ClientFor then opens the watch AS that user, so their own RBAC enforces.
The stance, in one line: krm-stream never holds a credential and is not an authorization boundary —
Kubernetes is. The gateway holds no privileged client of its own, so it cannot bypass RBAC even if a
bug wanted it to. Authorization is not something this library DOES; it is something it structurally
cannot avoid delegating.
Also written down because they are the things people get wrong:
- the Authorizer is fail-fast defence in depth, NOT the boundary;
- a projection is NOT authorization — redaction is a tighter disclosure layer ON TOP OF RBAC, and
must never be relied on to hide what the caller could have read anyway;
- sharing a watch MOVES the boundary onto your Authorizer, which is why it is opt-in;
- bearer token vs impersonation is a real choice, with the blast radius named.
README gets a paragraph and the login flow in the picture — the reader deciding whether this library
is for them should be able to see where the token lives without opening a second file.
Two gaps are listed as gaps rather than quietly omitted: kube.SSARAuthorizer (which would let you
share a watch AND keep Kubernetes as the boundary), and ValidatePatch (the redaction guard for the
host's save endpoint, as a function rather than a paragraph).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…RAuthorizer
The reviewer's best idea, and it closes the one place this library's own principle broke.
The stance is that krm-stream does not authorize: ClientFor opens the watch AS the caller, so the API
server's RBAC decides and no bug in here can talk it round. SharedBackend violated that by
construction — a shared watch is opened ONCE, so it is opened as ONE identity (your service account),
and every subscriber then reads from its cache. At that moment the Authorizer stops being defence in
depth and becomes the only thing between a caller and the objects.
We shipped that with a warning label. A warning label is not a boundary.
kube.SSARAuthorizer gives the boundary back: before a subscriber is served from the shared cache, ASK
THE API SERVER — with a SubjectAccessReview — "may this user list and watch this resource, in this
namespace?" — and let it answer. RBAC decides again, and sharing costs one round-trip per snapshot
cycle instead of the security property. Because the gateway now re-authorizes every cycle, this is
also how a revocation reaches an already-open stream.
`subjectOf` is the host's: Principal is opaque on purpose, so mapping it to the Kubernetes user and
groups RBAC binds against (the OIDC username/groups claims) is a thing only the host can do.
Three ways this could have been accidentally permissive, all now tested, and two of them
mutation-checked:
- **It asks about BOTH `list` and `watch`.** A snapshot cycle is a list THEN a watch — literally so
on the §3b path, where the gateway issues a real LIST — so a caller who may watch but not list
would otherwise be handed, in the snapshot, exactly the objects RBAC refused to let them
enumerate. Checking only `watch` authorizes half of what we are about to do.
- **A review we could not COMPLETE is not an allow.** If the API server cannot tell us whether this
caller may look, the answer is no. The other way round is a disclosure with an excuse.
- **An explicit Denied beats an Allowed** (a webhook authorizer saying "no" is not "no opinion").
It needs `create` on subjectaccessreviews (system:auth-delegator). It does NOT need impersonate
rights — it asks a question ABOUT a user, it does not act AS one, and impersonate is a privilege
whose compromise is total.
docs/auth.md and SharedBackend's own comment now point at it, so the code and the doc say the same
thing. The remaining known gap is ValidatePatch, and it stays listed as undecided rather than
quietly dropped: the hazard is one this library CREATES (the mask exists because we redacted the
field), and it is currently guarded by a paragraph.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…onger true
Asked to explain the ValidatePatch question properly, and writing it down found a lie in our own
code, which is its own small argument for the proposal.
project.go still told the reader that the mask "can never be written back over the truth: the write
path refuses any patch touching a path in redactedPaths." That became FALSE the moment the write path
was removed. There is no write path here; nothing in this library refuses anything. The comment was
describing a guarantee that had been deleted out from under it — which is exactly what happens to a
security property whose only enforcement is prose.
The proposal, stated plainly:
- The projection INVENTS `**REDACTED**`. The browser never saw the real Secret value; it saw a mask
this library made up. A patch carrying that mask back writes the literal string `**REDACTED**`
OVER the real secret, and the token is destroyed by a save that looked, from the browser, like a
green tick. **This failure cannot happen without us.**
- The client already refuses to produce such a patch, and it is tested in a real Chromium. That is a
NECESSARY condition, not a sufficient one — the save endpoint is an HTTP endpoint, and it accepts
whatever bytes are posted to it. The fixture says it in its own words: the client is not the
security boundary; a hostile client is just a client.
- So ship the check as a PURE FUNCTION — ValidatePatch(projection, obj, patch) — that the host calls
inside its own save handler. No client, no connection, no API server, no write. The read-path
stance survives intact; the deleted gatewayRejects: fixtures come back as hostRejects: and drive a
real test of a real function. The fixtures were right; only their placement was wrong.
And the alternative is stated fairly, because it is a real one: if the answer is no, the coherent move
is to STOP MASKING — a projection that removes Secret values entirely creates no overwrite hazard at
all. What is not coherent is inventing the placeholder and leaving the guard to a blockquote.
Nothing is built. The decision is the owner's, and it should be made against the hazard rather than a
summary of it.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…e answer changed mine
"In the end, starting a watch on a Secret is a decision you make yourself." That is right, and
following it honestly turned the proposal inside out.
The premise holds. Streaming a Secret is decided TWICE before this library sees it: the host
allowlists `secrets` in a deny-by-default ScopePolicy, and Kubernetes RBAC grants that user list/watch.
A user who clears both can already run `kubectl get secret -o yaml`. So the mask grants no security
against the user — we say so ourselves in docs/auth.md: a projection is not authorization.
What the mask actually buys is narrower, and worth naming rather than assuming: a browser is an
AMBIENT environment in a way a terminal is not. `kubectl get secret` is one deliberate act by one
person. A dashboard tab is left open — on a second monitor, in a screen share, in a screenshot pasted
into a ticket, in an error reporter that serializes the DOM, on a projector at standup. Same value,
same eyes, different exposure profile. It is why Lens, Headlamp and the OpenShift console all hide
Secret values behind a reveal, though every one of their users could kubectl the value in five
seconds.
But that argues for REDACTION — not for the PLACEHOLDER. And that distinction is the finding:
- `redactedPaths` is authoritative, mandatory on every added/modified, and already carries the fact.
The client marks paths read-only from THAT, not from the value (store.ts).
- spec §3 ALREADY says a consumer must not treat an absent path as deleted if it appears in
`redactedPaths`. Omitting the value is already legal, today, with no spec change.
- So the placeholder is REDUNDANT — and it is the sole source of the hazard. It is the only poisoned
value in the system, and we invented it.
Under the sanctioned write (an RFC 7386 merge patch, which is what patch() builds): with the
placeholder, a consumer sends `**REDACTED**` and the real token is destroyed. With the value omitted,
there is no such value to send — the draft never had one — and the token is untouched.
The hazard does not need a guard. It needs to not be created.
So the proposal now lays out three options and recommends B:
A. keep the placeholder, add ValidatePatch — a guard for a hazard we did not have to create;
B. keep redaction, DROP the placeholder — the hazard cannot arise, keys-only disclosure survives via
redactedPaths, and no spec change is needed;
C. delete redaction entirely — coherent, simpler, and the premise is right; I argue against it only
on the exposure asymmetry, because safe-by-default is worth one field of the wire and the cost of
being wrong is a token in a screenshot you never find out about.
I opened this document recommending A. The change of mind is left in the document rather than tidied
away, because how the answer moved is part of the argument.
Nothing is built. Still the owner's call.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Proposal 0003, option B. The projection no longer invents `**REDACTED**`.
The mask was the only poisoned value in the system, and we made it. A browser holding
`{"token": "**REDACTED**"}` can save it back, and a merge patch carrying that value writes the literal
string OVER the real Secret. The token is destroyed by a save that looked, from the browser, like a
green tick. Guarding that (ValidatePatch, on every adopter's save endpoint, forever, correctly) is a
guard for a hazard we did not have to create. So we stopped creating it.
Now: the value is gone, `data` is left in place but empty, and `redactedPaths` names the keys. Nothing
is lost, because redactedPaths was always authoritative and mandatory — it already carried the one
thing the mask carried, which is that the key EXISTS and is withheld. **The mask is something a UI
draws. It is not something the wire carries.** The demo renders `token ••••••` from
store.redactedPaths(uid), which is the new public accessor and the only place that fact lives.
Spec §3.1 is new and normative: a redacted value MUST NOT appear on the wire, not even as a
placeholder — and it says why, at the point where the next person would otherwise reinvent the mask.
The gateway README's "masked" disclosure policy is deleted for the same reason: it was an option that
created the landmine.
The corpus turned out to be hiding something, and fixing this exposed it. secret-token.v1 was used as
BOTH the gateway's input and its expected output — which only worked because masking an already-masked
value yields the same masked value. The corpus therefore never watched a Secret with a real value in
it, and the projection was never asked to remove anything. There are now two bodies: secret-token.v1
(a real Secret, real base64) and secret-token.v1-wire (data: {}). The difference between the files IS
the projection, and it is a real difference.
The browser test is now the strong one, and it is the assertion that matters: the real value is
nowhere in the page. Not in the DOM, not in the store, not in a screenshot of it — and not even as a
placeholder.
BREAKING CHANGE: `gateway.RedactedPlaceholder` is removed, and a redacted value is no longer present
on the wire in any form. A consumer that rendered the placeholder from the object must render it from
`redactedPaths` instead (the TS client exposes `store.redactedPaths(uid)`). No back-compat shim: there
are no users yet, and keeping the landmine around to be polite to nobody would be the whole mistake
repeated.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ng the truth
The three cache failures had one cause, and it was not a misconfiguration: gateway/go.sum does not
exist and never will, because the core module has ZERO dependencies — the exact invariant the
`consumable` job fails the build over. Asking to cache a file that by design cannot exist warned on
every run. `cache: false` now says so, and the line documents the promise instead of quietly failing.
Same for `consumable` itself, for the opposite reason: it must resolve from the NETWORK, the way an
adopter does, and a warm module cache there is a place for the bug to hide. (Its missing root go.mod
— the repo root holds a go.work — was the fourth warning.)
setup-task v2 ran on a deprecated Node 20; v3 does not.
Beyond the warnings, two changes that the release workflow needs:
- workflow_call, so release.yml can call this pipeline whole. The release tail then runs only
after the exact same jobs a PR gets are green IN THE SAME RUN, which is what keeps a write token
away from code this pipeline has not already validated.
- the `client` job now packs the npm tarball it just tested and hands it over as an artifact, so
what lands on the registry is what this pipeline PROVED — not a second build of the same source
that merely ought to match.
And a new, cheap check: no module may carry a `replace`. The `go get` below it would also catch that,
but only while the version gateway/kube requires already exists as a tag — and inside a release PR it
does not. The grep has no such blind spot, so the mistake that once shipped an unconsumable module
cannot be re-introduced silently.
Actions are pinned by SHA (npm ci, not npm install, for the same reason: the lockfile is the input).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…AG is the release
Nobody types a version number and nobody pushes a tag. release-please reads the conventional commits
since the last release, keeps a release PR open, and merging it IS the act of releasing.
The npm client, the gateway and the adapter release in LOCKSTEP on one number (linked-versions). That
is the same claim this repo makes everywhere else: they are one contract and they move in one commit.
A client that speaks protocol N and a gateway that speaks protocol N have no business carrying
version numbers you need a compatibility table to compare.
The two halves publish in genuinely different ways, and only one of them involves a registry:
npm the tarball ci.yml already packed and tested, pushed over OIDC trusted publishing. There is
NO NPM_TOKEN: npm trades the workflow's short-lived GitHub identity for a publish credential,
so the repo holds no long-lived secret that could leak, and provenance is attached
automatically. (One-time bootstrap on npmjs.com — docs/releasing.md.)
Go nothing is pushed anywhere. A Go module is published by the mere EXISTENCE of a tag named
<module-dir>/vX.Y.Z, which the GitHub release is. So the release ends by resolving that fresh
tag from a clean module, over the network, with no checkout — `a stranger can go get the
release`. Until something outside this repository does that, "published" is a hope. It is the
same check ci.yml runs per-PR, aimed at what an adopter actually types (@v0.1.0, not @<sha>),
and it is the only run of it that CAN exist, because the tag does not exist until now.
version.ts is bumped by release-please rather than by hand, because a vendored build states its own
provenance and version.test.ts already fails if it drifts from package.json.
gateway/kube still requires the core at a pseudo-version, deliberately: pointing it at a tag before
any tag exists would mean release-please rewriting a pseudo-version, and that is not a result worth
guessing at. It is a one-line follow-up once v0.1.0 lands, and docs/releasing.md says exactly what it
is — `consumable` already understands both worlds and defers to the post-tag check when the version
it needs is not tagged yet.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Grouped, so a week of upstream patch releases is one PR to review and not eleven, and `chore(deps):` so routine bumps stay out of a changelog that a consumer of this library reads. Every ecosystem is listed: one that is not listed is one nobody is watching. The core module is absent for the reason it is absent everywhere — it has no dependencies to update. k8s.io/* is explicitly ignored. That requirement is a supported-version FLOOR, not a version to keep current: minimal version selection means an adopter already on a newer client-go keeps it, while chasing patch releases here would force every adopter up to whatever we last built against. Raising it raises the supported minor, which is a deliberate act and not Dependabot's to make. Scorecard is not decoration. A library whose pitch is "import this into your browser and trust what it tells you about your cluster" is asking for a great deal of trust, and it should be able to show its working. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
package.json has always listed README.md in `files`, and there has never been one — so the first publish would have put a package on the registry with nothing on its page. It is deliberately short. This client is one half of a pair, and the documentation belongs in one place rather than being half-told in a second copy that drifts. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
.github/workflows/ci.yml (1)
148-196: 🩺 Stability & Availability | 🟠 MajorFork PRs still break the
go getgate (previously flagged).Line 175 sets
sha="${{ github.event.pull_request.head.sha || github.sha }}". For fork PRs,head.shais a commit in the fork that doesn't exist in the canonical repo, sogo get github.com/ConfigButler/krm-stream/gateway/kube@${sha}at line 194 fails. This was flagged in a prior review and remains unaddressed.Either skip the
go getstep whengithub.event.pull_request.head.repo.full_name != github.repository, or usegithub.sha(the merge commit, which exists in the canonical repo) for allpull_requestevents.🤖 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 @.github/workflows/ci.yml around lines 148 - 196, Update the SHA selection in the “go get the ADAPTER from a clean module” workflow step so fork pull requests do not pass an unresolvable fork-only commit to go get. Use github.sha for all pull_request events, or explicitly skip this step when github.event.pull_request.head.repo.full_name differs from github.repository; preserve the existing SHA behavior for non-fork runs.
🧹 Nitpick comments (1)
.github/workflows/release.yml (1)
92-93: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winConsider pinning the npm upgrade version. In a publish pipeline that leans on trusted-publishing/provenance for supply-chain integrity,
npm install -g npm@latestpulls an unpinned version at each release. Pinning to a known-good floor (e.g.npm@11.5.1, the documented minimum) keeps the release toolchain reproducible and auditable.♻️ Suggested change
- # Trusted publishing needs npm >= 11.5.1; Node 22 ships an older npm. - - run: npm install -g npm@latest + # Trusted publishing needs npm >= 11.5.1; Node 22 ships an older npm. + - run: npm install -g npm@11.5.1🤖 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 @.github/workflows/release.yml around lines 92 - 93, Pin the npm upgrade command in the release workflow to the documented known-good version npm@11.5.1 instead of npm@latest, preserving the existing global installation step and trusted-publishing setup.Source: Linters/SAST tools
🤖 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 @.github/workflows/ci.yml:
- Line 33: Update every actions/checkout step in the CI workflow, including the
checkout steps at the referenced locations, to set persist-credentials to false.
Preserve the existing pinned action versions and all other checkout
configuration.
In `@gateway/kube/authz_test.go`:
- Around line 158-169: Update TestANamedScopeAsksAboutThatName to validate
ResourceAttributes.Name on both SubjectAccessReviews in asked, ensuring each
authorization request uses "app-config" rather than checking only asked[0].
Preserve the existing authorization assertion and expected name.
In `@spec/v1.md`:
- Line 425: Update the normative validation rule in §8 so DNS-style name
validation applies to eligible fields but explicitly excludes labelSelector.
Ensure valid selectors containing operators and separators such as =, commas,
and parentheses remain accepted, consistent with §8’s selector support.
---
Duplicate comments:
In @.github/workflows/ci.yml:
- Around line 148-196: Update the SHA selection in the “go get the ADAPTER from
a clean module” workflow step so fork pull requests do not pass an unresolvable
fork-only commit to go get. Use github.sha for all pull_request events, or
explicitly skip this step when github.event.pull_request.head.repo.full_name
differs from github.repository; preserve the existing SHA behavior for non-fork
runs.
---
Nitpick comments:
In @.github/workflows/release.yml:
- Around line 92-93: Pin the npm upgrade command in the release workflow to the
documented known-good version npm@11.5.1 instead of npm@latest, preserving the
existing global installation step and trusted-publishing setup.
🪄 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: 5a64bee7-4e16-4c62-9620-c1616567cc58
⛔ Files ignored due to path filters (5)
conformance/gen/bodies.jsonis excluded by!**/gen/**conformance/gen/fixtures.jsonis excluded by!**/gen/**conformance/gen/protocol.jsonis excluded by!**/gen/**conformance/gen/scopes.jsonis excluded by!**/gen/**conformance/gen/sse/secret-redaction.sseis excluded by!**/gen/**
📒 Files selected for processing (52)
.devcontainer/Dockerfile.devcontainer/devcontainer-lock.json.github/dependabot.yml.github/workflows/ci.yml.github/workflows/release.yml.github/workflows/scorecard.yml.release-please-manifest.jsonREADME.mdTaskfile.ymlconformance/README.mdconformance/bodies/secret-token.v1-wire.yamlconformance/bodies/secret-token.v1.yamlconformance/fixtures/secret-redaction.yamlconformance/generate.shconformance/scopes.yamldocs/auth.mddocs/client-state-model.mddocs/facts/kubernetes-api-concepts.mddocs/naming.mddocs/proposals/0001-watch-ops.mddocs/proposals/0002-real-cluster.mddocs/proposals/0003-validate-patch.mddocs/releasing.mdexamples/vanilla-browser/index.htmlexamples/vanilla-browser/tests/live-krm.spec.tsgateway/README.mdgateway/auth_test.gogateway/conformance.gogateway/golden_test.gogateway/handler.gogateway/handler_test.gogateway/kube/authz.gogateway/kube/authz_test.gogateway/kube/go.modgateway/project.gogateway/scope.gogateway/scope_conformance_test.gogateway/shared.gogateway/shared_test.gogateway/stream.gogateway/stream_test.gopackages/krm-stream/README.mdpackages/krm-stream/src/index.tspackages/krm-stream/src/store.tspackages/krm-stream/src/url.tspackages/krm-stream/src/version.tspackages/krm-stream/test/conformance.tspackages/krm-stream/test/invariants.test.tspackages/krm-stream/test/scopes.test.tspackages/krm-stream/test/version.test.tsrelease-please-config.jsonspec/v1.md
💤 Files with no reviewable changes (1)
- packages/krm-stream/test/conformance.ts
🚧 Files skipped from review as they are similar to previous changes (14)
- examples/vanilla-browser/index.html
- docs/client-state-model.md
- gateway/kube/go.mod
- docs/proposals/0001-watch-ops.md
- examples/vanilla-browser/tests/live-krm.spec.ts
- docs/proposals/0002-real-cluster.md
- README.md
- conformance/README.md
- docs/facts/kubernetes-api-concepts.md
- gateway/stream.go
- Taskfile.yml
- packages/krm-stream/test/invariants.test.ts
- gateway/stream_test.go
- packages/krm-stream/src/store.ts
The ask: a controller rewrites `status` constantly, and a consumer that only edits `spec` does not care. Today it receives every one of those events in full. The proposal was: hash the interesting parts, put the hashes in metadata, let the consumer choose. Two of those three are right. The third inverts the goal, and it is the finding that reshapes the design: A `status` digest and event-suppression are MUTUALLY EXCLUSIVE. If the gateway emits a digest of status, then every status change changes the digest — so every status change still produces an event. You save the BYTES of the status block and keep every wakeup, every frame, every re-render. For "I don't care about status", the correct traffic is ZERO EVENTS, not a small event per change. The digest actively prevents the thing we want. So the mechanism for "I don't want this" is a VIEW plus SUPPRESSION, and it needs almost no new protocol: a view IS a projection (already named, already server-declared, already on every reset), and suppression is ~20 lines — if the projected object is byte-identical to the last one we sent this consumer, say nothing. Under a no-status view, a Deployment rollout produces N status-only MODIFIEDs upstream and ZERO events downstream. Not smaller events. No events. (One wrinkle, and the fix falls out of an existing rule: metadata.resourceVersion changes on every write, so a naive digest suppresses nothing. Exclude it — a consumer may then hold a stale resourceVersion, which is harmless precisely because spec §6 already forbids them from looking at it.) The general rule behind proposal 0003 is also written down here, because both of this proposal's temptations violate it: THE OBJECT IS A STRICT SUBSET OF WHAT THE API SERVER SENT. The gateway may remove a key. It may never add one, and never change a value. Everything it wants to say ABOUT what it removed lives in the ENVELOPE. That settles `maskedData` (a synthesized field — a consumer round-trips it into a patch and now it is written to your cluster) and, more sharply, a digest in `metadata.annotations` (an annotation IS part of the object; a save persists our private bookkeeping onto the resource, in etcd, forever). It is the mask landmine wearing a different hat. The instinct — be explicit, don't pretend a field is the field — is right; the envelope is where to be explicit, because nothing can round-trip it into a cluster. Digests do pay in one narrow place: "I may not see it, but I need to know it changed" — a rotated Secret. Envelope-only, and it MUST be an HMAC with a per-process key, not a hash: sha256 of a Secret's value is an offline-crackable oracle, and secrets are frequently low-entropy or structured. We would have disclosed the secret we were protecting while believing we had hidden it. So: an opaque `changeToken`, comparable only within one stream. No length, no size (the old README's "mask with length" is exactly the sort of thing you do not publish about a password). And the transport question, answered so it is not relitigated: the cheap wins are enormous and the expensive ones are small. Suppression → views → coalescing → gzip (5–10x on repetitive KRM JSON, invisible to the protocol) → and only then deltas or a new transport. Deltas would trade I-REPLACE — the property the whole convergence proof rests on — for bytes gzip gives us free. WebSocket/gRPC would cost us EventSource, and with it the entire auth model (docs/auth.md hangs off SSE's constraints). The uncomfortable and probably correct summary: the answer to "we are sending too many bytes" is mostly "stop sending events nobody wants", and after that "turn on gzip". Nothing built. Design for discussion. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… it mattered The constant itself went in d0e3140; this fixes the proposal that still claimed otherwise. Its Recommendation read: (`RedactedPlaceholder` stays exported and deprecated, since a consumer may render it.) That was wrong twice over, and leaving it would have been a document quietly disagreeing with the code it describes — which is the exact failure mode this proposal was written about. - There are no consumers yet. Keeping a landmine exported to be polite to nobody would be the whole mistake repeated in miniature. - And a mask is not something the WIRE should carry at all. It is something a UI DRAWS, from `store.redactedPaths(uid)`. Exporting the string would have kept the idea alive that the value belongs in the object. `RedactedPlaceholder` now appears in no source file in the repo — no alias, no shim, no deprecation. What remains is prose explaining why not to reinvent it, and one test that puts the literal string in a ConfigMap value on purpose, to prove redaction is decided by KIND and never by what a value looks like. The historical section is retitled to the past tense: the argument only makes sense against the thing it argued about, so it stays, honestly labelled. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Two corrections and one addition, all from the owner's question: "are you suggesting to remove
resourceVersion from what's sent to browsers then?"
**No — and §3 was ambiguous enough to be read that way.** The suppression digest excludes
resourceVersion from a comparison we make INTERNALLY. The field itself stays on the wire: it is part of
the object, the object is what the API server sent, and deleting a field to stop people misusing it is
how you end up with a protocol full of holes.
**But the question exposed a hazard I under-sold, and it has teeth.** Suppression means the consumer
keeps the object it already had — including its old resourceVersion. Its visible content is correct;
its resourceVersion now trails the cluster, and it trails it PRECISELY in the case that motivated the
proposal: the status-blind editor watching an object whose status churns invisibly.
A host that used that resourceVersion as a save precondition would get a 409 Conflict on every save,
forever, on exactly the objects that churn most — and it would be a FALSE conflict, since nothing the
user could see had changed. Maddening, and it would look like our bug. The protocol already forbids the
cause (§3: no whole-object PUT; saves are a merge patch, which carries no resourceVersion), so the fix
is to say it louder rather than invent anything: a consumer MUST NOT use resourceVersion as a save
precondition. A host that wants optimistic concurrency reads server-side at save time — it has an API
client; the browser does not. The browser's copy is a view, not a transaction handle.
**And a correction on what Kubernetes actually guarantees**, because it is both looser and stricter
than stated, and this repo has the receipts. On kube-apiserver 1.35+ it IS guaranteed orderable as a
monotonically increasing integer — a Certified Kubernetes conformance requirement, for base objects and
CRDs, and the whole basis of our OrderingStrict default. But the guarantee is per-storage: our own
cluster run (F6) found the aggregated API numbering its OWN resourceVersion space from ~1, in an
ephemeral etcd, where a restart sends them BACKWARDS. And the client contract says "opaque" regardless
— a guarantee the SERVER must uphold is not a licence for a CLIENT to depend on it.
**Which is the argument for the envelope `seq`, and it is a good idea — for a better reason than bytes.**
A uint64, per stream, assigned AT EMIT TIME so suppression and coalescing consume no numbers and the
stream is gapless by construction. Three things it buys, and the third is the real one:
1. it gives a consumer an order that is legitimately theirs to use. Today the only number available
is the one thing the spec forbids them to touch — a trap we set;
2. it makes suppression legible: "I dropped 200 status events for you" stays invisible, as it should,
rather than showing up as a suspicious hole;
3. **a gap is proof of loss.** Today, if an intermediary truncates an SSE frame, the consumer applies
what it got, converges WRONG, and never finds out. It looks fine. With a gapless seq it knows, and
can do the one correct thing: reconnect and resnapshot. We currently cannot detect that at all.
It must NOT go in the SSE `id:` field (spec §7 bans id: lines, because EventSource would replay it as
Last-Event-ID and promise a delta resume v1 does not have). It lives in the JSON envelope, which makes
no such promise.
Cost: twenty bytes and a counter. It is free to add NOW and never again — no users, unreleased
protocol — which is the actual argument for doing it first.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…edaction was never a feature
The owner asked whether we could drop the whole redaction apparatus by making the projection smarter,
and get "the secret changed" out of it. Yes — and chewing on it deleted the most dangerous thing in
the earlier draft.
**The realization: the gateway already knows.** §3's suppression makes it stateful per stream — it
must hold the last thing it told each consumer. So it can do the comparison ITSELF. It does not need
to hand the browser a token so the BROWSER can compare. It can simply say: `/data/token` changed.
That deletes the entire cryptographic section: the HMAC, the per-stream key, the key management, the
security review, and the offline-cracking risk (a sha256 of a Secret is a crackable oracle — secrets
are frequently low-entropy or structured, and we would have disclosed the thing we were protecting
while believing we had hidden it).
And it is not merely simpler, it is STRICTLY BETTER. Compared on the same footing, a perfectly-keyed
per-stream token buys:
- "the token was rotated" → same as just saying "changed"
- "it changed while I was disconnected" → NEITHER can do this (new stream, new key)
- "these two Secrets have the same password" → only the token can — and that is a DISCLOSURE we
never wanted to grant
Same power for the legitimate use, plus one illegitimate one, plus a permanent footgun. There is no
version of the crypto that wins.
The shape is a small per-path counter (`withheld[].rev`), not a boolean: a boolean is fragile, because
a UI that coalesces renders can miss the single event carrying `changed: true`. A counter is state, so
a late re-render still sees that it moved.
**And the bigger answer: there is no "redaction feature". There are three verbs.** A projection decides,
per path, exactly one of:
send you get the value
withhold the value never leaves the gateway; you learn the path EXISTS and are woken when it CHANGES
drop the value never leaves the gateway; it is as if it did not exist, and it NEVER wakes you
Everything in the document collapses into that: managedFields is `drop`. A Secret's values are
`withhold` — which is exactly the owner's case. `status` for a status-blind editor is `drop` (zero
traffic); for a dashboard it is `send`. "Redaction" was just `withhold` with a bad name and its own
plumbing, and `redactedPaths` becomes `withheld[]`, carrying the rev for free. One concept, one
envelope field, one code path — instead of a Secret-shaped special case bolted to the side.
The vocabulary also makes the central trade EXPLICIT instead of hidden: `withhold` costs one event per
change (you asked to know), `drop` costs zero. That is the §0 finding, and it is now something a
projection author chooses on purpose rather than discovers in production.
The honest limit, stated: `rev` is scoped to one stream, so you cannot know whether a withheld value
changed while you were DISCONNECTED — and no design can tell you that without publishing a stable,
content-derived identifier, which is precisely what we must never publish. A consumer that cares treats
every `reset` as "this may have changed".
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…haredBackend resync-looped
The test that found it came from outside, and it is a good one: 300 ConfigMaps is a SMALL namespace,
and sharedQueueDepth is 256. It failed immediately.
The bug, and it is worse than a truncated snapshot. subscribe() fills a joiner's queue from the warm
cache while nobody is draining it yet. The 257th object overflows, and the subscriber is told it "fell
too far behind" — so the stream loop does the one thing it knows: it resnapshots. Which fills the
queue from the warm cache again. Which overflows again, at exactly the same object.
**An infinite resync loop, on a 300-object namespace.** SharedBackend could not serve one at all. Both
of my own fan-out tests used two objects, so both were green.
The mistake was conflating two things that were never the same, and the fix is to stop:
- the SNAPSHOT is the consumer's STARTING STATE. It is finite, known in advance, and dropping part
of it is not backpressure — it is a WRONG ANSWER, and the recovery for a wrong answer is to send
it again, forever. It now gets an unbounded slice, drained before any live event.
- LIVE EVENTS are open-ended. A consumer that cannot keep up with THOSE genuinely is falling behind,
and resnapshotting it from the warm cache is exactly right. They keep the bounded channel and the
backpressure it exists for — a slow tab still cannot grow memory for everyone else.
Fixing it exposed a second bug immediately, and the tests caught that one too: with the snapshot no
longer travelling through the channel, a consumer that called Next() BEFORE the upstream finished its
first cycle parked on a channel that would never carry the thing it was waiting for. Every test that
subscribed before the boundary bookmark hung for exactly its timeout. Hence `ready` — a one-slot
wakeup that tells a parked reader the snapshot has landed in `pending`, with Next re-checking `pending`
before parking again, so a missed signal is impossible.
Green under -race, twice.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Owner's review, and both points land. **"Withhold is kind of redact, isn't it?"** Yes, and `redact` was already ours. A redacted document has a BLACK BAR: you can see something was taken, you cannot see what. That is precisely the verb, and it is exactly what `redactedPaths` has always meant. And the silent case already had a word too, in spec §3's own text — a field the gateway REMOVED, "never shown, and never named". So the vocabulary is **send / redact / remove**, all three of them words the spec already uses. `withhold` was inventing a word for a concept that had one. The generalisation was real; the neologism was not. **Three projections, and the status-only one is dropped.** The owner's instinct was right, and for a reason worth writing down: an object with its `spec` removed IS NOT A KRM OBJECT ANY MORE. It is a fragment that looks like a resource and is not one — you cannot round-trip it, you cannot diff it, and every consumer holding one has to know it is holding half a thing. This project's thesis is "the payload is a Kubernetes object, not an abstracted document" (spec §0). A status-only projection quietly abandons that. And its byte argument does not survive §5 either. The case FOR it was: under I-REPLACE every status event re-sends the whole object, including a `spec` that did not change, and a Deployment's spec.template is large. But an SSE stream is ONE GZIP STREAM WITH ONE SLIDING WINDOW — the previous event's spec is still in that window, so re-sending an identical spec costs almost nothing on the wire. Compression eats that problem for free, without inventing a payload that is not a KRM object. So: pay for `remove` when you want ZERO EVENTS (that is krm-spec/v1, a real win compression cannot give you), not merely to shave repeated bytes. krm-raw/v1 send everything, Secret values INCLUDED⚠️ — an operator console meant to disclose krm-full/v1 default. metadata + spec + status, Secrets redacted krm-spec/v1 full minus status — the status-blind editor, and zero traffic under churn One naming note that is a safety property, and it is why I did not take "verbose": the projection that puts Secret values in a browser must have a name that makes a reviewer flinch, because the name is what shows up in the diff. "verbose" sounds like a log level. `raw` sounds like the loaded gun it is. **Custom projections: yes, but only the HOST may define one.** A projection is a SECURITY POLICY — it is the thing that redacts Secrets. If the browser can describe a projection, the browser can describe one that does not redact, and it has just un-redacted your cluster. So: the consumer PICKS a projection from a registered list; it never DESCRIBES one. Agreed it is not the first thing to build, but the seam is reserved now, because retrofitting "the host may define one" onto a hard-coded switch is a refactor and reserving it is free. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…d back
Proposed: send / inform / ignore. The two verbs turn out to be decided by different arguments, so the
answer is a split — and recording why is the point.
**`ignore` wins, and it is better than my `remove`.** `remove` describes what happens to the OBJECT —
the key is gone. But what this proposal is ABOUT is what happens to the EVENTS: a change there must
NEVER WAKE YOU. `remove` merely implies that, by way of suppression. `ignore` states it. It is the
stronger claim and the one a reader actually needs, and it took a moment to see.
**`inform` loses to `redact`, on two counts.** It is ambiguous — read `Inform: ["/data/token"]` cold
and ask "inform whom, of what?", where `Redact: ["/data/token"]` is unmistakable. And, the real reason:
`redact` is a SECURITY TERM OF ART, and that is a feature. The word is what catches a reviewer's eye
scanning a diff; it carries "this is secret" without anybody reading the documentation. `inform` sounds
like a notification preference.
It is the same argument that keeps the disclosing projection called `raw` rather than `verbose`: the
scary word does its work precisely on the person who is NOT reading carefully.
(`inform` does name one real thing `redact` does not — our redaction now tells you WHEN the value
changed, which plain redaction does not imply. But that gap closes for free in the envelope:
`redacted: [{path, rev}]`, counter right there, self-documenting. The security legibility that `inform`
would cost cannot be bought back that way, because its whole value is working on someone who is not
reading.)
So: send / redact / ignore. One verb chosen for what it does to the object's VALUE, one for what it
does to the STREAM, and both for saying so out loud.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…rop it
The owner's reductio settles an open question in one line: under the same logic that keeps `data: {}`,
you would send `status: {}` when status is ignored — which is bullocks.
Correct. And it is sharper than that, because **the rule was already in our own code, four lines above
the block that broke it.** project() removes an `annotations` map it has emptied, with this comment:
// An annotations map that is empty ONLY because we emptied it is our artifact, not the
// server's state. Leaving `annotations: {}` behind would tell the consumer the object
// has an empty annotation map, which is a different fact from having none.
And then the Secret path left `data: {}` behind. We wrote the rule and violated it in the same
function. I even defended it in the proposal ("slightly more informative"), which it is not: `data: {}`
asserts *"this Secret has an empty data map"*, and that is a DIFFERENT FACT from *"this Secret's data
is not yours to see"*. The second is what happened, `redactedPaths` is what says so, and it is the only
thing that does.
So a fully-redacted Secret now carries no `data` key at all. Spec §3 states the rule generally: a
container the projection empties is removed with its contents, never left behind as `{}`.
The other half of the rule, and the half that keeps it honest — now its own test: **we remove what WE
removed, and nothing else.** A Secret that genuinely arrived with an empty `data` map KEEPS it. That
emptiness is the server's fact, not our artifact, and deleting it would report a removal we never made.
The browser suite is the proof that this is safe, and it is worth stating: the demo still renders
`token ••••••` with NO `data` key in the object at all — drawn purely from `redactedPaths`. That is
redaction being genuinely authoritative rather than decorative. If the mask had been quietly depending
on the object's shape, this change would have blanked it, and the test would have caught that.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ug I would have shipped
A user's review of 0004, and the first finding is real. I verified it with code rather than taking it
on faith, because it decides the build order:
before: {"apiVersion":"v1","kind":"Secret","metadata":{…},"type":"Opaque"} redactedPaths=[/data/token]
after: {"apiVersion":"v1","kind":"Secret","metadata":{…},"type":"Opaque"} redactedPaths=[/data/token]
A Secret ROTATION is byte-identical after projection — the projection deletes the value, so there is
nothing left to differ. §3's suppression rule was stated over the projected OBJECT, so the event is
dropped, the `rev` bump from §4 never arrives, and "did my Secret rotate?" returns ZERO EVENTS,
FOREVER. On exactly the case that motivated §4. And it would have looked like the feature working.
The reviewer's fix is right; I have generalised it, because the instance is less interesting than the
class. Suppress on the EVENT, never on the OBJECT: the digest covers everything the consumer would
observe — object AND envelope (`redacted[]`, its rev vector, and whatever we add next) — minus only
what churns without informing (`resourceVersion`, and `seq` itself). Stated over the object, the next
envelope field re-introduces the same bug. Stated over the event, it cannot.
And the build order was wrong in a way that mattered: suppression at step 1, `rev` at step 5, four
steps apart. **They are one change and they ship together** — because the failure is invisible (zero
events on a Secret nobody rotates; zero events on one somebody does).
**§3.1's answer WAS a hand-wave, and the pushback lands.** "A host wanting optimistic concurrency reads
server-side at save time" — a server-side read returns the CURRENT resourceVersion by definition, so
the precondition always matches. It cannot detect that the USER'S VIEW was stale, which is the only
thing OCC is for. It is not OCC; it is a write with a ceremony.
But I push back on `metadata.generation` being THE answer, and the second hole matters more for this
product than the one the reviewer named:
1. we already do better, and it is the product: the store raises a LIVE conflict the moment the
server touches a field the user is editing — before they save. A save-time precondition is a
worse version of a thing this library already does continuously;
2. a merge patch is per-field last-write-wins, and that is INTENDED — two people editing different
fields both succeed, which is why the write is a patch and not a PUT. Say it plainly;
3. and only then: if you want a hard precondition it is `generation`, never `resourceVersion`.
Reviewer's hole: absent on ConfigMap/Secret. **Mine: `generation` does not move on METADATA
writes** — and editing a label is the first thing this library's own fixtures demonstrate. So it
silently misses a concurrent label edit. It is a precondition on `spec`, and must be sold as
exactly that.
**§2.2's Rules shape was kind-blind, and that is a security bug in a doc.** `Redact: ["/data"]` — but
ConfigMap has a `/data` too, and its whole purpose is to be readable. The built-in it replaces reads
`if p == ProjectionEditor && isSecret(out)`: a rule that cannot express `isSecret` cannot replace it.
Rules now match on `(group, kind)` first, then path — group included, because a CRD may define its own
`Secret` kind in its own group.
Also accepted: the gzip argument for killing krm-status/v1 is weaker than stated (32 KB window holds
for one object, degrades on a large scope with many interleaved — brotli's is larger). The CONCLUSION
stands on its own without compression at all: the payload would not be a KRM object. And nobody has
measured it, which in this repo means "go and measure it" (F1–F7).
Housekeeping the reviewer caught: a section duplicated verbatim, §8 sitting between §5 and §6 and
colliding with the gateway README's §8, and a JSON example still saying `redactedPaths` after §2
renamed it.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/proposals/0004-views-and-bytes.md`:
- Around line 434-444: Update the modified Secret envelope example to omit the
object’s data property entirely when all data keys are redacted, while
preserving the existing redacted entries and revision values.
🪄 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: 5ad5c42c-df04-4678-82eb-3a6d65a5f521
⛔ Files ignored due to path filters (2)
conformance/gen/bodies.jsonis excluded by!**/gen/**conformance/gen/sse/secret-redaction.sseis excluded by!**/gen/**
📒 Files selected for processing (8)
conformance/bodies/secret-token.v1-wire.yamldocs/proposals/0004-views-and-bytes.mdgateway/project.gogateway/shared.gogateway/shared_bigcache_test.gogateway/stream_test.gopackages/krm-stream/test/invariants.test.tsspec/v1.md
🚧 Files skipped from review as they are similar to previous changes (5)
- gateway/project.go
- gateway/shared.go
- packages/krm-stream/test/invariants.test.ts
- spec/v1.md
- gateway/stream_test.go
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
gateway/README.md (2)
394-394: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUpdate the JSON Merge Patch citation to RFC 7396. RFC 7386 is obsolete here; both references should point to the current spec.
✏️ Suggested fix
- (RFC 7386, built by the consumer — the reconcile engine's `patch(id)`) and `PATCH` it with the + (RFC 7396, built by the consumer — the reconcile engine's `patch(id)`) and `PATCH` it with the🤖 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 `@gateway/README.md` at line 394, Update the JSON Merge Patch citation in the gateway documentation from RFC 7386 to RFC 7396, including both references mentioned in the surrounding text.
138-138: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAlign the package-shape diagram with the real repo layout.
gateway/README.md:101-117describesapi/,auth/,watcher/,project/,stream/, andwrite/as subpackages, but the implementation is a flatgatewaypackage plusgateway/kube. Either mark this as a conceptual split or update the diagram so readers don’t look for directories that don’t exist.🤖 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 `@gateway/README.md` at line 138, Update the package-shape diagram in gateway/README.md to match the repository’s actual layout: a flat gateway package with the kube submodule containing kube.NewBackend. Either label api, auth, watcher, project, stream, and write as conceptual areas or remove the nonexistent subpackage directories, while preserving accurate package guidance.packages/krm-stream/src/store.ts (1)
179-189: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
renameKeysilently drops data on a key collision.If
newKeyalready exists as a distinct key in the map, theObject.entries(map)rewrite loop writes both the originalnewKeyentry and the renamedoldKeyentry to the samerenamed[newKey]slot; whichever is processed last in iteration order silently overwrites the other, with no error and no signal to the caller.🐛 Proposed fix: reject rename into an existing distinct key
renameKey(id: string, path: Path, oldKey: string, newKey: string): void { const res = this.#editable(id, [...path, oldKey]); this.#editable(id, [...path, newKey]); const map = get(res.draft, path); if (!isPlainObject(map)) throw new Error(`krm-stream: ${pathKey(path)} is not a map`); + if (oldKey !== newKey && Object.hasOwn(map, newKey)) { + throw new Error(`krm-stream: ${pathKey([...path, newKey])} already exists`); + } const renamed: Record<string, unknown> = {}; for (const [k, v] of Object.entries(map)) renamed[k === oldKey ? newKey : k] = v; setAt(res.draft, path, renamed);🤖 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 `@packages/krm-stream/src/store.ts` around lines 179 - 189, Update renameKey to detect when newKey already exists in the map as a distinct key from oldKey, and reject the operation before rewriting or settling the map. Preserve the existing rename behavior when no collision exists.gateway/handler.go (1)
101-118: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winAuthenticate before scope validation.
ScopePolicy.Validatereturns distinct errors for allowlisted targets/resources and namespace semantics, so an unauthenticated caller can enumerate the allowlist by probing query params beforeo.Principal(r)runs. Move the principal check first so unauthenticated requests always get the same refusal.🤖 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 `@gateway/handler.go` around lines 101 - 118, In the handler returned by the visible HTTP HandlerFunc, move the o.Principal(r) authentication check before ScopeFromQuery and o.Scopes.Validate. Ensure any principal error immediately calls refuse with Forbidden("not authenticated") and returns, so unauthenticated requests cannot reach scope validation; preserve the existing scope error handling for authenticated requests.gateway/stream.go (1)
226-236: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winPrune stale
revisionsentries at snapshot boundariesrevisionsis per connection, but only liveWatchDeletedevents remove keys. If an object disappears during a resync, its redaction state can stay resident for the rest of the stream and grow with churn. Clean out UIDs not present inemittedright afterEventSynced.🤖 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 `@gateway/stream.go` around lines 226 - 236, At the InitialEventsEnd branch in the watch stream, immediately after emitting EventSynced, prune the per-connection revisions map by removing every UID not present in emitted. Keep revisions for currently emitted objects and preserve the existing EventSynced observation and error flow.
♻️ Duplicate comments (2)
gateway/stream.go (1)
280-297: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winDeletion events still bypass the monotonicity/staleness check — same issue flagged and marked resolved in a prior review.
WatchDeletedis emitted and prunesemitted/digests/revisionsunconditionally, with noisStalecheck against the tombstone's resourceVersion (when available). A stale/out-of-order delete replayed by the upstream watch can therefore prune state for — and tell the consumer to remove — an object that has since been re-added/modified within the same cycle. This is the exact gap a previous review comment on this range proposed fixing, and that thread is marked "✅ Addressed in commits b3fac4c to dfcf5a0", but the fix is not present in this code.Proposed fix
case WatchDeleted: id := identityOf(ev.Object) if id == nil { return ResyncRequired("deletion tombstone carried no trustworthy uid") } + stale, err := isStale(g.Ordering, emitted, id.UID, ev.Object) + if err != nil { + return err + } + if stale { + continue + } if err := sink.Emit(ctx, Event{Type: EventDeleted, Identity: id}); err != nil { return err }🤖 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 `@gateway/stream.go` around lines 280 - 297, Update the WatchDeleted branch to apply the existing isStale monotonicity check using the tombstone’s resourceVersion when available before emitting EventDeleted or pruning emitted, digests, and revisions. Ignore stale/out-of-order tombstones while preserving the current resync behavior for deletions without a trustworthy identity.packages/krm-stream/src/sse.ts (1)
141-144: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winHandle signals that are already aborted.
AbortSignaldoes not replay itsabortevent to newly registered listeners. Both clients therefore open a stream when passed an already-aborted signal; checksignal.abortedbefore invokingfetchor constructingEventSource.Proposed fix
const controller = new AbortController(); const fetchImpl = opts.fetch ?? globalThis.fetch; - if (opts.signal) opts.signal.addEventListener("abort", () => controller.abort(), { once: true }); + if (opts.signal?.aborted) controller.abort(); + else if (opts.signal) opts.signal.addEventListener("abort", () => controller.abort(), { once: true });export function connectWithEventSource( url: string, store: LiveResourceStore, opts: Omit<StreamOptions, "fetch" | "headers"> = {}, ): StreamHandle { + if (opts.signal?.aborted) { + return { close: () => {}, closed: Promise.resolve() }; + } const es = new EventSource(url, { withCredentials: true });Also applies to: 188-193
🤖 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 `@packages/krm-stream/src/sse.ts` around lines 141 - 144, Update connectResourceStream and the corresponding EventSource client path around the signal-listener setup to check opts.signal.aborted before invoking fetch or constructing EventSource. Immediately abort or return the existing closed-stream behavior for an already-aborted signal, while preserving the current listener wiring for signals that abort later.
🤖 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 `@gateway/README.md`:
- Around line 273-274: The README bullet conflates cluster-scoped resources with
all-namespaces watches. Split the content into separate bullets: one stating
that cluster-scoped resource kinds such as Node and PersistentVolume are
supported and inherently non-namespaced, and another explaining that a
namespaced resource with no namespace is an explicitly allowlisted
all-namespaces watch, not an accidental wildcard.
In `@packages/krm-stream/src/merge.ts`:
- Around line 109-190: Fix keyed-list conflict path indexing in
mergeAssociativeList so entries whose mergeAssociativeEntry result is undefined
do not cause subsequent conflicts to use incorrect positions. Assign final array
indices only after filtering omitted entries, or otherwise track conflicts by
key and remap them to the finalized output positions before returning; preserve
correct revert and takeTheirs behavior for surviving entries.
---
Outside diff comments:
In `@gateway/handler.go`:
- Around line 101-118: In the handler returned by the visible HTTP HandlerFunc,
move the o.Principal(r) authentication check before ScopeFromQuery and
o.Scopes.Validate. Ensure any principal error immediately calls refuse with
Forbidden("not authenticated") and returns, so unauthenticated requests cannot
reach scope validation; preserve the existing scope error handling for
authenticated requests.
In `@gateway/README.md`:
- Line 394: Update the JSON Merge Patch citation in the gateway documentation
from RFC 7386 to RFC 7396, including both references mentioned in the
surrounding text.
- Line 138: Update the package-shape diagram in gateway/README.md to match the
repository’s actual layout: a flat gateway package with the kube submodule
containing kube.NewBackend. Either label api, auth, watcher, project, stream,
and write as conceptual areas or remove the nonexistent subpackage directories,
while preserving accurate package guidance.
In `@gateway/stream.go`:
- Around line 226-236: At the InitialEventsEnd branch in the watch stream,
immediately after emitting EventSynced, prune the per-connection revisions map
by removing every UID not present in emitted. Keep revisions for currently
emitted objects and preserve the existing EventSynced observation and error
flow.
In `@packages/krm-stream/src/store.ts`:
- Around line 179-189: Update renameKey to detect when newKey already exists in
the map as a distinct key from oldKey, and reject the operation before rewriting
or settling the map. Preserve the existing rename behavior when no collision
exists.
---
Duplicate comments:
In `@gateway/stream.go`:
- Around line 280-297: Update the WatchDeleted branch to apply the existing
isStale monotonicity check using the tombstone’s resourceVersion when available
before emitting EventDeleted or pruning emitted, digests, and revisions. Ignore
stale/out-of-order tombstones while preserving the current resync behavior for
deletions without a trustworthy identity.
In `@packages/krm-stream/src/sse.ts`:
- Around line 141-144: Update connectResourceStream and the corresponding
EventSource client path around the signal-listener setup to check
opts.signal.aborted before invoking fetch or constructing EventSource.
Immediately abort or return the existing closed-stream behavior for an
already-aborted signal, while preserving the current listener wiring for signals
that abort later.
🪄 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: 8c77103a-0a4e-4cad-afe8-50cf5d410250
⛔ Files ignored due to path filters (20)
conformance/gen/bodies.jsonis excluded by!**/gen/**conformance/gen/fixtures.jsonis excluded by!**/gen/**conformance/gen/sse/bookmark-absorbed.sseis excluded by!**/gen/**conformance/gen/sse/conflict-and-converge.sseis excluded by!**/gen/**conformance/gen/sse/delete-recreate-uid.sseis excluded by!**/gen/**conformance/gen/sse/edit-vs-unrelated-change.sseis excluded by!**/gen/**conformance/gen/sse/key-removed-upstream.sseis excluded by!**/gen/**conformance/gen/sse/named-object-absent.sseis excluded by!**/gen/**conformance/gen/sse/nested-field-removed.sseis excluded by!**/gen/**conformance/gen/sse/partial-object-refused.sseis excluded by!**/gen/**conformance/gen/sse/reconnect-prune.sseis excluded by!**/gen/**conformance/gen/sse/resourceversion-bignum.sseis excluded by!**/gen/**conformance/gen/sse/resourceversion-unorderable.sseis excluded by!**/gen/**conformance/gen/sse/resync-midstream.sseis excluded by!**/gen/**conformance/gen/sse/secret-redaction.sseis excluded by!**/gen/**conformance/gen/sse/snapshot-then-deltas.sseis excluded by!**/gen/**conformance/gen/sse/status-follow-live.sseis excluded by!**/gen/**conformance/gen/sse/status-only-churn.sseis excluded by!**/gen/**conformance/gen/sse/tombstone-without-uid.sseis excluded by!**/gen/**packages/krm-stream/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (84)
.devcontainer/Dockerfile.devcontainer/devcontainer.json.devcontainer/post-create.sh.github/workflows/ci.yml.github/workflows/release.yml.release-please-manifest.jsonREADME.mdTaskfile.ymlconformance/README.mdconformance/bodies/deploy-web.v1-spec.yamlconformance/bodies/secret-token.v1-wire.yamlconformance/bodies/secret-token.v1.yamlconformance/fixtures/bookmark-absorbed.yamlconformance/fixtures/conflict-and-converge.yamlconformance/fixtures/delete-recreate-uid.yamlconformance/fixtures/edit-vs-unrelated-change.yamlconformance/fixtures/key-removed-upstream.yamlconformance/fixtures/named-object-absent.yamlconformance/fixtures/nested-field-removed.yamlconformance/fixtures/partial-object-refused.yamlconformance/fixtures/reconnect-prune.yamlconformance/fixtures/resourceversion-bignum.yamlconformance/fixtures/resourceversion-unorderable.yamlconformance/fixtures/resync-midstream.yamlconformance/fixtures/secret-redaction.yamlconformance/fixtures/snapshot-then-deltas.yamlconformance/fixtures/status-follow-live.yamlconformance/fixtures/status-only-churn.yamlconformance/fixtures/tombstone-without-uid.yamldocs/adopting.mddocs/client-state-model.mddocs/extraction-plan.mddocs/facts/observed-v1.36.2+k3s1.mddocs/naming.mddocs/operations.mddocs/proposals/0002-real-cluster.mddocs/proposals/0004-views-and-bytes.mddocs/releasing.mddocs/saving.mdexamples/README.mdexamples/vanilla-browser/README.mdexamples/vanilla-browser/index.htmlexamples/vanilla-browser/tests/live-krm.spec.tsgateway/README.mdgateway/conformance.gogateway/conformance_test.gogateway/event.gogateway/handler.gogateway/handler_test.gogateway/observe.gogateway/patch.gogateway/patch_test.gogateway/project.gogateway/projection_policy.gogateway/scope.gogateway/scope_policy_test.gogateway/shared.gogateway/shared_test.gogateway/sse.gogateway/stream.gogateway/stream_conformance_test.gogateway/stream_test.gopackages/krm-stream-compat/README.mdpackages/krm-stream-compat/index.d.tspackages/krm-stream-compat/index.jspackages/krm-stream-compat/package.jsonpackages/krm-stream/README.mdpackages/krm-stream/package.jsonpackages/krm-stream/src/index.tspackages/krm-stream/src/merge.tspackages/krm-stream/src/path.tspackages/krm-stream/src/schema.tspackages/krm-stream/src/sse.tspackages/krm-stream/src/store.tspackages/krm-stream/src/types.tspackages/krm-stream/src/url.tspackages/krm-stream/src/version.tspackages/krm-stream/test/conformance.test.tspackages/krm-stream/test/conformance.tspackages/krm-stream/test/invariants.test.tspackages/krm-stream/test/wire.test.tsrelease-please-config.jsonspec/events.schema.jsonspec/v1.md
🚧 Files skipped from review as they are similar to previous changes (35)
- conformance/fixtures/resourceversion-bignum.yaml
- conformance/bodies/secret-token.v1.yaml
- conformance/fixtures/bookmark-absorbed.yaml
- .release-please-manifest.json
- packages/krm-stream/README.md
- packages/krm-stream/src/version.ts
- docs/facts/observed-v1.36.2+k3s1.md
- packages/krm-stream/src/url.ts
- conformance/bodies/secret-token.v1-wire.yaml
- conformance/fixtures/resourceversion-unorderable.yaml
- release-please-config.json
- gateway/sse.go
- conformance/fixtures/partial-object-refused.yaml
- docs/releasing.md
- packages/krm-stream/test/wire.test.ts
- examples/vanilla-browser/index.html
- conformance/fixtures/tombstone-without-uid.yaml
- gateway/project.go
- examples/vanilla-browser/tests/live-krm.spec.ts
- conformance/README.md
- packages/krm-stream/src/path.ts
- gateway/conformance_test.go
- packages/krm-stream/src/index.ts
- gateway/stream_conformance_test.go
- .github/workflows/release.yml
- gateway/handler_test.go
- packages/krm-stream/package.json
- gateway/shared.go
- .github/workflows/ci.yml
- .devcontainer/Dockerfile
- README.md
- examples/vanilla-browser/README.md
- conformance/fixtures/secret-redaction.yaml
- docs/proposals/0002-real-cluster.md
- Taskfile.yml
sunib
changed the title
Merged
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
1 participant
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
Introduces the first pre-1.0
krm-streamlibrary: a written KRM resource-stream protocol, a Go read gateway, and a dependency-free TypeScript browser store sharing one conformance corpus.Highlights
gateway/kubeadapter, including streaming-list and list-then-watch fallback paths.Adopter Documentation
Validation
task fixtures-checktask test(including Go race tests)task linttask build-clientBoundaries
The library intentionally remains a read gateway and headless client. It does not provide a browser token flow, Kubernetes API proxy, authorization system, or write endpoint.