Merge upstream v0.28.3 and v0.29.3 into dp-stable

.claude/agents/oauth-expert.md

@@ -46,7 +46,7 @@ Before providing guidance on OAuth/OIDC details, use WebFetch to verify RFC or s
 - `pkg/auth/token.go`: JWT parsing, validation, claims extraction
 - `pkg/auth/middleware.go`: HTTP authentication middleware
 - `pkg/auth/oauth/`: OAuth 2.0 and OIDC client implementations
-- `pkg/auth/tokenexchange/`: RFC 8693 token exchange
+- `pkg/oauthproto/tokenexchange/`: RFC 8693 token exchange
 - `pkg/auth/discovery/`: OAuth/OIDC discovery, RFC 9728 support
 - `pkg/authserver/`: OAuth2 authorization server (Ory Fosite, PKCE, JWT/JWKS)
 

.claude/rules/operator.md

@@ -36,15 +36,27 @@ See `cmd/thv-operator/DESIGN.md` for detailed decision guidelines.
 
 - Always run `task operator-generate` after modifying CRD types
 - Always run `task operator-manifests` after adding kubebuilder markers
-- Always run `task crdref-gen` from `cmd/thv-operator/` after CRD changes to regenerate API docs (uses relative paths)
+- Always run `task crdref-gen` from the repo root after CRD changes to regenerate API docs (running it from `cmd/thv-operator/` fails — the task resolves the config path relative to the repo root)
 - Use `envtest` for integration testing, not real clusters
 - Chainsaw tests require a real Kubernetes cluster
-- Status updates require a separate client patch (`r.Status().Update()`)
+- Status writes must go through `controllerutil.MutateAndPatchStatus` — see the Status Writes section below
 
 ## Status Condition Parity
 
 When adding a status condition to one CRD type, check all parallel types (e.g., `MCPServer` and `VirtualMCPServer`) for the same condition. Conditions that warn about misconfiguration or unsupported states should be consistent across types that share the same feature set — a gap means one type silently accepts invalid config that the other rejects.
 
+## Status Writes
+
+Use `controllerutil.MutateAndPatchStatus` for every status write — not `r.Status().Update` or inline `client.Status().Patch` (see #4633). The helper's doc comment is the authoritative spec.
+
+When adding a status-write call site, check three things:
+
+1. **Caller holds a freshly-`Get`ted object.** Reconciler-start writers do; writers that iterate `List` results (e.g., deletion-path fan-out in `MCPGroupReconciler`) do not and need a fresh `Get` before calling the helper.
+2. **Caller is the sole owner of the entire `Status.Conditions` array.** Per-condition-type ownership is NOT enough. JSON merge-patch replaces the array wholesale for CRDs (the `+listType=map` marker is only honored by strategic-merge-patch), so any concurrent writer whose Patch lands between this caller's Get and Patch — on any condition type, not just the ones this caller touches — will be erased. A fresh `Get` narrows the TOCTOU window but does not eliminate it. If two code paths must write conditions on the same CRD (e.g., operator reconciler + in-pod `K8sReporter`), fix at the design level: consolidate to a single owner, or move one writer to a dedicated status field outside the array.
+3. **Scalar fields the writer touches are not co-owned.** A stale-computed value different from the caller's snapshot will overwrite the live value — the helper cannot defend against this.
+
+Do not use `MutateAndPatchStatus` for spec or metadata writes — those require optimistic locking (`client.MergeFromWithOptions(..., MergeFromWithOptimisticLock{})`). See #4767.
+
 ## Key Operator Commands
 
 ```bash
@@ -53,5 +65,41 @@ task operator-generate        # Generate deepcopy, client code
 task operator-manifests       # Generate CRD YAML, RBAC
 task operator-test            # Run unit tests
 task operator-e2e-test        # Run e2e tests
-task crdref-gen              # Generate CRD API docs (run from cmd/thv-operator/)
+task crdref-gen              # Generate CRD API docs (run from the repo root)
 ```
+
+## Spec / metadata patching
+
+Never use `r.Update` on a CR spec or metadata: `Update` is a full PUT,
+so any field our local copy does not track (e.g. `spec.authzConfig`
+written by a separate authorization controller) gets zeroed on every
+reconcile.
+
+Use `controllerutil.MutateAndPatchSpec` instead. The helper wraps an
+optimistic-lock merge patch: the body only contains fields the caller
+changed, and `MergeFromWithOptimisticLock` sends `resourceVersion` as a
+precondition, so if the server moved between our Get and Patch the
+apiserver returns 409 and controller-runtime requeues with a fresh Get.
+
+This is what protects `metadata.finalizers`. Merge-patch has no
+array-append semantics — arrays are replaced wholesale — so when our
+diff includes `finalizers` (e.g. an `AddFinalizer` call) it must have
+been computed from an up-to-date snapshot. The 409 + requeue is what
+guarantees that: any concurrent finalizer added by another controller
+fails our precondition, and the next reconcile observes it via a fresh
+Get before recomputing the diff.
+
+```go
+if err := ctrlutil.MutateAndPatchSpec(ctx, r.Client, mcpServer, func(m *mcpv1beta1.MCPServer) {
+    controllerutil.AddFinalizer(m, MCPServerFinalizerName)
+}); err != nil {
+    return ctrl.Result{}, err
+}
+```
+
+Expect 409s as routine log noise once the external controller lands —
+the guard doing its job, not a bug.
+
+Status-subresource patching uses the sibling helper
+`controllerutil.MutateAndPatchStatus` (see the "Status Writes" section
+above).

.claude/skills/check-contribution/SKILL.md

@@ -1,6 +1,6 @@
 ---
 name: check-contribution
-description: Validates operator chart contribution practices (helm template, ct lint, docs generation, version bump) before committing changes.
+description: Validates operator chart contribution practices (helm template, ct lint, docs generation) before committing changes.
 allowed-tools: [Bash, Read]
 ---
 
@@ -28,19 +28,14 @@ helm-docs --dry-run
 ```
 Verify that `values.yaml` variables are documented and the generated README.md matches.
 
-### 4. Chart Version Bump
-If chart files changed, verify:
-- `deploy/charts/operator/Chart.yaml` version is bumped for operator changes
-- `deploy/charts/operator-crds/Chart.yaml` version is bumped for CRD changes
-- Version follows [SemVer](https://semver.org/) and bump type matches the change scope
-
 ## Output Format
 
 ```
 ✅ or ❌ Helm template renders successfully
 ✅ or ❌ Chart linting passes
 ✅ or ❌ Documentation up-to-date
-✅ or ❌ Chart version bumped appropriately
+
+Note: Chart version bumps are automated by the release bot. Feature PRs should NOT modify Chart.yaml.
 ```
 
 Include specific errors for any failing checks with actionable remediation commands.