You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
While reviewing sei-k8s-controller PR #216 (replace-pod task for chain-upgrade rollouts), the kubernetes-specialist agent did two full review rounds without catching a cache-lag race that Cursor Bugbot subsequently flagged on its first pass.
The race:
apply-statefulset task SSA-writes the StatefulSet.
Same reconcile, replace-pod (and later observe-image) reads the StatefulSet via the cached client (mgr.GetClient()).
The controller-runtime informer cache lags the apiserver by milliseconds-to-unbounded; the read returns the pre-apply spec/status.
replace-pod sees CurrentRevision == UpdateRevision, no-ops, marks itself complete; rollout deadlocks.
This is a high-severity correctness bug in any executor-driven plan where one task writes a resource and a later task in the same reconcile reads it back. The fix was to plumb mgr.GetAPIReader() through ExecutionConfig and use it for read-after-write reads.
Gap
The agent's current heuristics catch ownership refs, RBAC verbs, generation/observed-generation freshness, multi-replica safety, idempotency. It does not explicitly look for read-after-write patterns through the cached client.
This is a class of bug, not a one-off:
Any executor / plan runner / saga where step N+1 reads what step N just wrote.
Any reconcile loop that issues SSA then re-reads to make a decision.
Any handler that calls client.Update/Patch then immediately client.Get to observe the result.
The cached client is the right default for top-of-reconcile reads (the watch event woke us up, so the cache is already authoritative for that object). It is the wrong choice for reads of objects we just mutated in the same reconcile pass.
Proposed update to .claude/agents/kubernetes-specialist.md
Add a review-checklist section (or extend Domain Expertise) that calls out the two-client model:
controller-runtime client semantics — mgr.GetClient() reads from the informer cache (eventually consistent; lags writes). mgr.GetAPIReader() reads strongly from the apiserver. When reviewing a reconciler or task executor, identify every Get/List. For each one, ask: is this read of a resource that this same reconcile pass already wrote (via Create/Update/Patch/Apply)? If yes, the cached client is unsafe — the read can return the pre-write state. Require APIReader for read-after-write in the same reconcile, or restructure so the read happens on a subsequent reconcile (after the watch event fires).
Concrete checks the agent should run when reviewing a controller PR:
Grep the diff for Apply, Patch, Update, Create, Delete calls — these mark write points.
For each write target, grep for Get/List calls on the same kind in tasks that run after the write within the same reconcile (look at planner ordering, executor loop structure).
Flag any such read-after-write that uses cfg.KubeClient / r.Client rather than an APIReader.
Note that fake-client unit tests cannot catch this — sigs.k8s.io/controller-runtime/pkg/client/fake is synchronous and has no informer cache. Recommend envtest coverage when the fix involves cache semantics.
Bugbot finding: race condition where replace-pod always no-ops due to stale STS status from cached client
Fix commit: plumbed APIReader through ExecutionConfig, switched replace-pod and observe-image STS reads to APIReader.Get
Two prior agent reviews (rounds 2 and 3 of 4) did not surface this; round 4 confirmed the fix only after the user explicitly raised it
Out of scope
Generic stale-cache discussions in pure-watch reconcilers (the cache is correct there by design).
Optimistic-concurrency patches on .status (already covered by the sei-k8s-controller CLAUDE.md).
Acceptance
A future review of a same-reconcile read-after-write through mgr.GetClient() is flagged by the agent on the first pass, with a recommendation to use APIReader or defer the read to the next reconcile.
Background
While reviewing sei-k8s-controller PR #216 (replace-pod task for chain-upgrade rollouts), the
kubernetes-specialistagent did two full review rounds without catching a cache-lag race that Cursor Bugbot subsequently flagged on its first pass.The race:
apply-statefulsettask SSA-writes the StatefulSet.replace-pod(and laterobserve-image) reads the StatefulSet via the cached client (mgr.GetClient()).replace-podseesCurrentRevision == UpdateRevision, no-ops, marks itself complete; rollout deadlocks.This is a high-severity correctness bug in any executor-driven plan where one task writes a resource and a later task in the same reconcile reads it back. The fix was to plumb
mgr.GetAPIReader()throughExecutionConfigand use it for read-after-write reads.Gap
The agent's current heuristics catch ownership refs, RBAC verbs, generation/observed-generation freshness, multi-replica safety, idempotency. It does not explicitly look for read-after-write patterns through the cached client.
This is a class of bug, not a one-off:
client.Update/Patchthen immediatelyclient.Getto observe the result.The cached client is the right default for top-of-reconcile reads (the watch event woke us up, so the cache is already authoritative for that object). It is the wrong choice for reads of objects we just mutated in the same reconcile pass.
Proposed update to
.claude/agents/kubernetes-specialist.mdAdd a review-checklist section (or extend Domain Expertise) that calls out the two-client model:
Concrete checks the agent should run when reviewing a controller PR:
Apply,Patch,Update,Create,Deletecalls — these mark write points.Get/Listcalls on the same kind in tasks that run after the write within the same reconcile (look at planner ordering, executor loop structure).cfg.KubeClient/r.Clientrather than anAPIReader.sigs.k8s.io/controller-runtime/pkg/client/fakeis synchronous and has no informer cache. Recommend envtest coverage when the fix involves cache semantics.Evidence trail
replace-podalways no-ops due to stale STS status from cached clientAPIReaderthroughExecutionConfig, switchedreplace-podandobserve-imageSTS reads toAPIReader.GetOut of scope
.status(already covered by thesei-k8s-controllerCLAUDE.md).Acceptance
A future review of a same-reconcile read-after-write through
mgr.GetClient()is flagged by the agent on the first pass, with a recommendation to useAPIReaderor defer the read to the next reconcile.