[HYPERSHELL-49] feat: openshell service accounts

[jsell-rh]

Summary

• Define the gateway-scoped OpenShellGatewayServiceAccount API and its Keycloak confidential-client lifecycle.
• Cap the selected OpenShell role by the creator current gateway binding: owners can select user or admin, while viewers can select only user.
• Add browser-safe role and expiration capabilities, server-side collection behavior, and repeatable non-secret setup metadata.
• Add Service accounts between the existing Connection and Details tabs, with a management table, create flow, and one-time credential handoff.
• Provide safe copyable OpenShell CLI and Client Credentials JWT flows without embedding the client secret or replacing interactive gateway login.
• Specify credential expiration, short-lived access tokens, gateway audience isolation, workspace grants, replacement, revocation, deletion, and audit redaction.

Scope

This specification covers the client-secret flow only. WIF, multi-gateway credentials, configurable scopes, and in-place secret rotation remain deferred.

Validation

• make check
• git diff --check origin/main...HEAD

Jira: https://redhat.atlassian.net/browse/HYPERSHELL-49

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Pro Plus

Run ID: 03214a51-25da-43d2-9d9a-0a8e45224b3b

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

---

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

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[jsell-rh]

Security-sensitive details such as one-time secret redaction, exact gateway binding checks, least-privilege Keycloak roles, strict input parsing, and cache avoidance are handled thoughtfully, but the service-account reconciler is not serialized with foreground lifecycle operations. As written it can re-enable a successfully revoked credential, delete an in-flight create, miss a concurrent create during gateway cleanup, and permanently revoke accounts on transient dependency failures; the console and CLI also have one-time-secret loss paths.

Overall assessment: REQUEST_CHANGES

This is a self-review, so GitHub does not permit the PR author to submit a REQUEST_CHANGES event; the formal review is submitted as COMMENT and the amber/changes-requested label records the assessment.

Blocker

1. components/api-server/plugins/serviceAccounts/service.go:605 — A stale reconciliation snapshot can re-enable a client after Revoke has completed and overwrite Revoked back to Ready. Fix with shared lifecycle serialization plus re-read/CAS before enable and save. Confidence: High (100%).

Major

1. components/api-server/plugins/serviceAccounts/service.go:503 — The reconciler treats every Provisioning row as abandoned and can clean up a live create. Use a shared lock/lease and a proven-stale threshold. Confidence: High (100%).
2. components/api-server/plugins/serviceAccounts/service.go:394 — Gateway cleanup is not serialized with Create, so a client can be provisioned after the cleanup scan and while the gateway row is deleted. Lock cleanup plus deletion and re-read readiness inside Create's lock. Confidence: High (99%).
3. components/api-server/plugins/serviceAccounts/service.go:579 — Internal gateway lookup and temporary OIDC-readiness failures are collapsed into terminal unavailability and permanently revoke accounts. Reserve permanent revocation for confirmed deletion. Confidence: High (100%).
4. components/api-server/pkg/keycloak/service_accounts.go:185 — Every steady-state sweep disables and rebuilds healthy clients; later failures leave Keycloak disabled while persistence still says Ready. Diff before mutation and persist a truthful reconciliation state. Confidence: High (100%).
5. components/api-server/plugins/serviceAccounts/dao.go:129 — A sequential all-status scan cannot bound expiration enforcement to the required one minute. Prioritize due/transitional work and separate bounded drift/orphan scheduling. Confidence: High (97%).
6. packages/gateway-management-ui/src/service-accounts/service-account-create-dialog.tsx:205 — Tab/router/browser navigation bypasses the one-time-secret loss confirmation (UI-INT-04, UI-PERF-04, UI-TRUST-05). Protect navigation and pending completion. Confidence: High (99%).
7. components/cli/cmd/hypershell/create/serviceAccount/cmd.go:86 — The CLI validates the exclusive output file after creation, so predictable local write errors discard the only secret. Reserve the target before POST. Confidence: High (100%).

Verification

• Passed repository policy checks (make check).
• Passed service-account and Keycloak tests under the race detector.
• Passed control-plane, CLI, Go SDK, SDK/CLI generator, and focused API unit suites.
• Passed the complete web check pipeline: formatting, architecture checks, lint, type checks, 344 tests, i18n, production build, and Storybook build.
• The full API integration sweep could not run because the local PostgreSQL service rejected the repository test credentials; unaffected unit packages, including serviceAccounts and keycloak, passed.
• GitHub image builds are successful; enterprise-contract checks are neutral. The PR is also currently draft and conflicts with the advanced main branch.

— Amber

Findings Summary (ordered by severity, highest first):

1. [Blocker] Stale reconciliation can re-enable a successfully revoked credential and restore Ready state - Security / Reconciliation (L605)
2. [Major] Reconciliation can delete a live provisioning request - Concurrency / Reconciliation (L503)
3. [Major] Gateway cleanup can miss a concurrent service-account creation - Lifecycle Consistency (L394)
4. [Major] Transient gateway or OIDC failures permanently revoke credentials - Reliability / Reconciliation (L579, L702)
5. [Major] Routine reconciliation disrupts healthy clients and reports failed clients as Ready - Availability / State Integrity (L185, L436)
6. [Major] The all-status sequential scan cannot guarantee the one-minute expiration deadline - Security / Scheduling (L129)
7. [Major] Console navigation can discard an unacknowledged one-time secret without confirmation - UI Safety (L205)
8. [Major] CLI output-file errors can discard the only secret after account creation - Secret Handoff (L86)

Convention Checklist (omit conventions not applicable to the diff):

Convention	Result
No panic() in production code	Pass
Errors wrapped with fmt.Errorf context	Fail
Typed not-found handling distinguishes 404 from internal failure	Fail
No secrets in logs or error messages	Pass
Input validated	Pass
Reconciliation is convergent and safe to retry	Fail
OpenAPI SDK artifacts generated rather than hand-edited	Pass
Conventional commit intent present in squash title	Pass

[jsell-rh]

The two commits since the previous Amber pass improve the service-account setup UI and move Keycloak administration behind an mTLS control-plane boundary, but all eight prior findings remain unresolved and the moved reconciler can still re-enable a credential after a successful revoke. The new boundary also accepts unscoped Keycloak client UUID mutations and both peers pin cert-manager certificates in memory, so the assessment remains REQUEST_CHANGES.

Overall assessment: REQUEST_CHANGES

This is a self-review, so GitHub does not permit the PR author to submit a REQUEST_CHANGES event; the formal review is submitted as COMMENT and the amber/changes-requested label records the assessment.

Prior-review status

I re-read the formal Amber review at commit 20531fb and all eight inline threads before reviewing commits 55e1170 and faa81fb. All eight threads are unresolved: seven remain current at their existing anchors, while the prior Blocker is outdated only because its call site moved, so I re-anchored that one below and did not duplicate the other active threads.

Blocker

1. components/control-plane/internal/serviceaccountprovisioner/server.go:91; components/control-plane/internal/serviceaccountkeycloak/client.go:214 — The authenticated RPC accepts a caller-supplied Keycloak UUID and the provider verifies only that some client exists before disabling or deleting it. Unlike reconciliation, these methods do not require the HyperShell managed marker or matching gateway and service-account ownership, so a corrupted row or code path with access to the API client can turn the realm-admin boundary into a delete primitive for gateway, console, or unrelated clients. Fix: accept immutable resource IDs and resolve the managed client server-side, or verify the managed marker and exact ownership metadata before either mutation; reject non-managed clients and add negative tests. Confidence: High (98%).
2. components/api-server/plugins/serviceAccounts/service.go:638,653 — Reconciliation still acts on an unlocked snapshot. If Revoke completes while this row is paused, this call enables the client and the final update restores the stale Ready state, allowing a credential reported as revoked to mint tokens again. Fix: share lifecycle serialization across reconciliation and foreground mutations, re-read desired state before enabling, and make the final write a status/version CAS; add a barrier-driven regression test. Confidence: High (100%).

Major

1. components/api-server/plugins/serviceAccounts/service.go:536 — Every Provisioning row is treated as abandoned even though Create persists that state while a live request is still provisioning and delivering its one-time secret. Fix: use the same lifecycle lock or persisted lease in both paths and reclaim only after a proven-stale deadline; test a slow create against reconciliation. Confidence: High (100%).
2. components/api-server/plugins/serviceAccounts/service.go:427; components/api-server/plugins/serviceAccounts/service.go:185 — Gateway cleanup does not share the create lock, while Create checks gateway readiness before taking that lock. Cleanup can finish its scan and a paused create can then provision a credential for a gateway being deleted. Fix: hold a shared per-gateway lock through cleanup and gateway-row deletion, and re-read gateway existence and readiness after Create obtains it. Confidence: High (99%).
3. components/api-server/plugins/serviceAccounts/service.go:612,739 — Any gateway problem becomes terminal revocation, but gateway converts every service error to not-found and temporary or malformed OIDC metadata also produces a problem. Fix: preserve confirmed 404 versus retryable internal and readiness failures; only confirmed deletion should permanently revoke. Test all three paths. Confidence: High (100%).
4. components/control-plane/internal/serviceaccountkeycloak/client.go:185; components/api-server/plugins/serviceAccounts/service.go:473 — Every ordinary sweep disables a healthy client before rebuilding it. A later provider failure leaves Keycloak disabled while the API persists the original Ready state with only LastError changed. Fix: diff actual and desired state before mutation, perform zero writes when converged, and persist a truthful fail-closed reconciliation state when a mutation fails. Confidence: High (100%).
5. components/api-server/plugins/serviceAccounts/dao.go:129 — The oldest 1,000 rows across all statuses are processed sequentially, so due expirations can be starved beyond the specified one-minute disablement bound. Fix: query due and transitional work first, process safety-critical disables with bounded concurrency, and paginate ordinary drift and orphan work separately. Confidence: High (97%).
6. packages/gateway-management-ui/src/service-accounts/service-account-create-dialog.tsx:208; packages/gateway-management-ui/src/pages/gateway-pages.tsx:626 — The loss confirmation protects only modal-close events. Tab selection, route navigation, Back, refresh, and unmount can discard a pending or unacknowledged one-time secret, violating UI-INT-04, UI-PERF-04, and UI-TRUST-05. Fix: guard navigation and beforeunload or hoist the ephemeral handoff above the unmounting tab, then test tab, Back, refresh, and pending-completion paths. Confidence: High (99%).
7. components/cli/cmd/hypershell/create/serviceAccount/cmd.go:86 — The POST happens before the exclusive output target is opened, so an existing or unwritable file discards the only secret after creating a live account. Fix: reserve a mode-0600 target before the request, pass its handle through the write path, and remove the empty reservation if no account is created. Confidence: High (100%).
8. components/api-server/plugins/serviceAccounts/provisioner_client.go:50; components/control-plane/internal/serviceaccountprovisioner/transport.go:60 — Both processes load their leaf certificate and private key only at startup. cert-manager rotates the mounted Secrets, but no TLS callback, watcher, checksum rollout, or reloader consumes the new files, so reconnects eventually fail after the in-memory certificates expire and service-account create, revoke, delete, and expiration work remains unavailable until restart. Fix: dynamically reload certificates with GetCertificate and GetClientCertificate or trigger deterministic rollouts on Secret renewal; test renewal and reconnect without manual restart. Confidence: High (98%).

UI requirement coverage

Requirement IDs	Result	Evidence
UI-INT-04, UI-PERF-04, UI-TRUST-05	FAIL	One-time-secret loss protection does not cover tab, router, browser, or unmount navigation.
UI-A11Y-01 through UI-A11Y-10	NOT_TESTED	No interactive keyboard, screen-reader, zoom/reflow, touch, voice, or disabled-user session was available.
UI-VER-01 through UI-VER-06	PARTIAL	Automated source, unit, build, and Storybook evidence passed; no declared browser or representative-user matrix was executed.
UI-PF-01, UI-PF-02, UI-PF-05, UI-PF-09; UI-CONTENT-04; UI-HEX-01, UI-HEX-02	PASS	Source inspection plus formatting, lint, type, architecture, localization, and component tests passed.

Verification

• Passed repository policy checks with make check and passed git diff --check.
• Passed API service-account and control-plane service-account packages under the Go race detector.
• Passed the complete control-plane test and vet suites, API vet, and the complete CLI test suite.
• Passed the gateway-management UI check pipeline: formatting, lint, type checks, 159 tests, coverage, and build checks.
• Passed the web-console check pipeline: formatting, architecture, lint, type checks, 67 tests, localization, production build, and Storybook build.
• Rendered deploy/base, deploy/kind, deploy/openshift, and deploy/ibm successfully.
• The full API integration sweep remains blocked because local PostgreSQL rejects the repository test credentials; affected unit packages pass. No live Keycloak, certificate-renewal, browser, assistive-technology, or representative-user session was available.
• GitHub image checks are successful and enterprise-contract checks are neutral. The PR is still draft and currently conflicts with main.

— Amber

Findings Summary (ordered by severity, highest first):

1. [Blocker] Privileged UUID mutations can disable or delete non-managed Keycloak clients - Security / Authorization Boundary (L91, L214)
2. [Blocker] Stale reconciliation can re-enable a successfully revoked credential and restore Ready state - Security / Reconciliation (L638, L653)
3. [Major] Reconciliation can delete a live provisioning request - Concurrency / Reconciliation (L536)
4. [Major] Gateway cleanup can miss a concurrent service-account creation - Lifecycle Consistency (L427, L185)
5. [Major] Transient gateway or OIDC failures permanently revoke credentials - Reliability / Reconciliation (L612, L739)
6. [Major] Routine reconciliation disrupts healthy clients and reports failed clients as Ready - Availability / State Integrity (L185, L473)
7. [Major] The all-status sequential scan cannot guarantee the one-minute expiration deadline - Security / Scheduling (L129)
8. [Major] Console navigation can discard an unacknowledged one-time secret without confirmation - UI Safety (L208, L626)
9. [Major] CLI output-file errors can discard the only secret after account creation - Secret Handoff (L86)
10. [Major] cert-manager renewals are ignored until both provisioner peers restart - Availability / Certificate Lifecycle (L50, L60)

Convention Checklist (omit conventions not applicable to the diff):

Convention	Result
No panic() in production code	Pass
Errors wrapped with fmt.Errorf context	Fail
Typed not-found handling distinguishes 404 from internal failure	Fail
No secrets in logs or error messages	Pass
Privileged mutation targets validate managed ownership	Fail
mTLS certificates rotate without manual restart	Fail
Reconciliation is convergent and safe to retry	Fail
SecurityContext present on pod workloads	Pass
Image references are registry-qualified	Pass
Generated OpenAPI and protobuf artifacts are updated	Pass
Conventional commit intent present	Pass

[jsell-rh]

The seven follow-up commits fix the unscoped Keycloak mutation, stale-reconcile, live-provisioning, transient-dependency, steady-state outage, CLI handoff, and leaf-certificate defects from the last Amber round. The assessment remains REQUEST_CHANGES because security-sensitive Keycloak drift can now be accepted as converged, gateway deletion and expiry enforcement still have lifecycle gaps, and the UI, CA rotation, and public-status contract are only partially propagated.

Overall assessment: REQUEST_CHANGES

This is a self-review, so GitHub does not permit the PR author to submit a REQUEST_CHANGES event; the formal review is submitted as COMMENT and the amber/changes-requested label records the assessment.

Prior-review status

I re-read both prior formal Amber reviews and all 11 review threads before evaluating commits b8d6ff1..6fc5b8c.

Previous finding	Current status
Privileged UUID mutation can target non-managed clients	Fixed — destructive operations now require the managed marker and exact stored ownership.
Stale reconcile can re-enable a revoked credential	Fixed — shared locking, a locked re-read, and conditional persistence prevent the stale write.
Reconcile can delete a live Provisioning request	Fixed — Create and reconciliation share the lock, with a 15-minute reclaim threshold.
Gateway cleanup can miss a concurrent Create	PARTIAL — cleanup shares the lock, but gateway-row deletion happens after it is released.
Transient gateway/OIDC failures cause terminal revocation	Fixed — confirmed 404 is now separated from retryable dependency/readiness failures.
Routine reconciliation disables healthy clients and reports failure as Ready	Fixed for the original defect — no-write convergence and degraded state address the outage/state lie; the new convergence predicate has a separate Blocker below.
Expiration scan cannot prove the one-minute bound	PARTIAL — due work is prioritized, but remains capped and sequential.
Navigation can discard the one-time secret	PARTIAL — tab clicks and hard unload are guarded; SPA route/history navigation is not.
CLI output-file failure loses the only secret	Fixed — the exclusive mode-0600 target is reserved before POST.
cert-manager renewal requires restart	PARTIAL — leaf key pairs reload, but CA trust anchors remain static.

Blocker

1. components/control-plane/internal/serviceaccountkeycloak/client.go:238,328 — reconcileConverged cannot see fullScopeAllowed, public-client/service-account/interactive-grant flags, redirect origins, or default scopes because kcClient does not deserialize them. Its mapper comparison also accepts an extra included.custom.audience; the zero-write return therefore preserves security-broadening drift that the product specification explicitly requires reconciliation to remove. Fix: compare an exact normalized security representation and exact mapper configs, including absent custom audiences and disabled device/CIBA grants, and add full-scope/custom-audience drift tests. Confidence: High (100%).

Major

1. components/api-server/plugins/serviceAccounts/service.go:478; components/api-server/plugins/gateways/service.go:213 — CleanupGateway releases the shared lock before gateways.Delete removes the gateway row. A blocked Create can resume in that gap, see the still-healthy gateway, provision a live identity, and then have the gateway deleted. Fix: hold the same lifecycle barrier through final gateway deletion, or persist a deleting state under the barrier that Create rejects; add the exact blocked-Create interleaving test. Confidence: High (100%).
2. components/api-server/plugins/serviceAccounts/dao.go:165; components/api-server/plugins/serviceAccounts/service.go:539 — due rows are now prioritized, but only 1,000 are selected and processed sequentially under a 30-second cycle context. More than one page of due accounts or ordinary provider latency still defeats the specified one-minute disablement guarantee. Fix: drain/page due work independently with bounded concurrency or a deadline-aware worker queue, and prove the bound with a high-latency, multi-gateway test. Confidence: High (98%).
3. packages/gateway-management-ui/src/pages/gateway-pages.tsx:545; components/web-console/app/routes/gateway.tsx:84 — the new leave guard runs only through changeTab. Browser Back/Forward changes the externally controlled activeTab directly, and navigation to another SPA route unmounts the handoff without firing beforeunload, so a pending or unacknowledged secret can still be lost. Fix: integrate a router-level blocker or hoist the handoff above the route boundary, then test Back and another client-side route (UI-INT-04, UI-INT-07, UI-PERF-04, UI-TRUST-05). Confidence: High (100%).
4. components/api-server/plugins/serviceAccounts/provisioner_client.go:103; components/control-plane/internal/serviceaccountprovisioner/transport.go:114 — leaf certificates reload, but RootCAs and ClientCAs are read only at startup. The deployed self-signed CA uses rotationPolicy: Always; after CA rollover and leaf reissuance, the peers reject one another until restart. Fix: reload trust bundles for new handshakes or trigger deterministic rollouts on CA/leaf Secret changes, and add a full CA-rollover handshake test. Confidence: High (99%).
5. components/api-server/openapi/openapi.serviceAccounts.yaml:237 — the public degraded status is incomplete across contract surfaces: the product spec and its polling rule omit it, the web route allowlist omits it, and the checked-in TypeScript SDK omits it. The latter currently breaks the web-console typecheck and generated-SDK drift gate. Fix: define the state semantics in the product spec, update the route parser with a round-trip test, and regenerate every SDK artifact. Confidence: High (100%).

UI requirement coverage

Requirement IDs	Result	Evidence
UI-INT-04, UI-INT-07, UI-PERF-04, UI-TRUST-05	FAIL	SPA history and route navigation can bypass the secret-loss guard.
UI-FND-06	PARTIAL	degraded has inconsistent state semantics across the product spec, OpenAPI clients, and route parser.
UI-A11Y-01 through UI-A11Y-10; UI-FND-01 through UI-FND-02; UI-VER-02, UI-VER-05 through UI-VER-06; UI-PERF-01, UI-PERF-05	NOT_TESTED	No browser/assistive-technology matrix, representative-user evidence, or field performance/monitoring evidence was available.
UI-PF-01, UI-PF-02, UI-PF-05, UI-PF-09; UI-CONTENT-04; UI-I18N-01; UI-HEX-01, UI-HEX-02, UI-HEX-05, UI-HEX-06	PASS	Source inspection and the package formatting, lint, type, localization, architecture, and component-test evidence support these requirements.

Verification

• Passed git diff --check.
• Passed API service-account, control-plane Keycloak/provisioner, and CLI service-account tests under the Go race detector; full go vet ./... passed in all three Go modules.
• Passed the gateway-management UI check pipeline: formatting, lint, type checks, and 163 tests.
• Current GitHub E2E Kind, repository policy, Go lint, gateway-management UI, image builds, and CodeRabbit checks pass.
• The committed head fails the web-console typecheck because the generated TypeScript status union lacks degraded; the OpenAPI SDK drift check fails for the same ungenerated contract change.
• No live Keycloak drift repair, CA rollover, browser Back/route navigation, assistive-technology, or representative-user session was available.

— Amber

Findings Summary (ordered by severity, highest first):

1. [Blocker] The zero-write convergence check preserves security-broadening Keycloak client and mapper drift - Security / Reconciliation (L238, L328)
2. [Major] Gateway cleanup releases its barrier before the gateway row is deleted - Lifecycle Consistency (L478, L213)
3. [Major] The capped sequential due scan still cannot guarantee one-minute expiration enforcement - Security / Scheduling (L165, L539)
4. [Major] SPA route and history navigation can still discard the one-time secret - UI Safety (L545, L84)
5. [Major] CA trust-anchor rotation still requires both provisioner peers to restart - Availability / Certificate Lifecycle (L103, L114)
6. [Major] The new degraded status is incomplete across the specification, SDK, and route contract - Spec / Generated Artifacts (L237)

Convention Checklist (omit conventions not applicable to the diff):

Convention	Result
No panic() in production code	Pass
Errors wrapped with fmt.Errorf context	Fail
Typed not-found handling distinguishes 404 from internal failure	Pass
No secrets in logs or error messages	Pass
Privileged mutation targets validate managed ownership	Pass
Reconciliation removes broader roles, grants, and audiences	Fail
One-minute expiration enforcement is bounded	Fail
mTLS leaf and trust-anchor rotation works without manual restart	Fail
Generated OpenAPI SDK artifacts are current	Fail
One-time UI handoff survives or blocks all navigation paths	Fail
Conventional commit intent is present	Pass

[jsell-rh]

The seven follow-up commits close four of the six findings from the previous Amber round and remove the original security-broadening Keycloak cases. The assessment remains REQUEST_CHANGES because zero-write convergence still accepts behavior-breaking client/mapper drift, and the due-work drain can stop behind a retained first page and leave later expirations unprocessed.

Overall assessment: REQUEST_CHANGES

This is a self-review, so GitHub does not permit the PR author to submit a REQUEST_CHANGES event; the formal review is submitted as COMMENT and the amber/changes-requested label records the assessment.

Prior-review status

I re-read all prior Amber reviews and all 15 review threads before evaluating commits dd8b904..795ec35.

Previous finding	Current status
Zero-write convergence preserves security-broadening Keycloak drift	PARTIAL — full-scope, interactive-flow, injected-scope, and custom-audience drift now trigger repair; behavior-bearing authenticator and role-mapper settings remain unchecked.
Gateway cleanup releases its barrier before gateway-row deletion	Fixed — the final gateway delete now runs inside the shared lifecycle barrier, with an interleaving regression test.
Capped sequential due scan cannot guarantee one-minute expiry	PARTIAL — work is concurrent and successful pages drain, but retained rows can pin the first page and hide later expirations.
SPA route/history navigation can discard the one-time secret	Fixed — a route-level blocker covers links and POP navigation, while the existing guard handles tabs and beforeunload covers hard navigation.
CA trust-anchor rotation requires restart	Fixed — both peers reload trust pools per new handshake and exercise CA-rollover handshakes.
degraded is incomplete across specification, SDK, and route contract	Fixed — the product semantics, generated SDKs, route allowlist, and deep-link round trip are aligned.

Major

1. components/control-plane/internal/serviceaccountkeycloak/client.go:109,393 — The new predicate detects the prior security-broadening fields, but it still accepts behavior-breaking drift: kcClient does not compare the desired protocol/authenticator, and the role mapper can set access.token.claim=false, multivalued=false, or a wrong JSON type while passing convergence. A client can therefore remain reported ready while its stored secret or access-token role claim no longer works. Fix: compare every behavior-bearing client field written by repair and every required mapper config value; add authenticator and role-claim drift regressions. Confidence: High (100%).
2. components/api-server/plugins/serviceAccounts/service.go:583,595 — Re-querying the same ordered LIMIT 1000 page does not advance when attempted rows remain eligible. If the first 1,000 rows fail or intentionally stay transitional, they are returned again, all skipped, and the empty batch exits before later due rows are ever loaded; those later credentials can mint tokens beyond the specified one-minute bound. Fix: use stable keyset pagination or exclusion of attempted IDs, preferably separating due-ready expiration work from other transitions, and test more than one page with a retained first page. Confidence: High (100%).

UI requirement coverage

Requirement IDs	Result	Evidence
UI-INT-04, UI-INT-07, UI-PERF-04, UI-TRUST-05	PASS	Source inspection plus tab, SPA-link, Back/Forward cancel/confirm, and beforeunload tests cover the one-time handoff paths.
UI-FND-06	PASS	degraded now round-trips through the product spec, OpenAPI/SDK types, status presentation, and URL state.
UI-A11Y-01 through UI-A11Y-10; UI-PERF-01, UI-PERF-05	NOT_TESTED	No interactive assistive-technology, representative viewport/input, or field-performance session was available.
UI-VER-01 through UI-VER-06	PARTIAL	Deterministic source, type, component, route, build, and Storybook evidence passed; no declared browser/representative-user matrix was executed.

Verification

• Passed git diff --check and repository policy checks with make check.
• Passed focused API service-account and control-plane Keycloak/provisioner tests under the Go race detector; full go vet ./... passed in both Go modules.
• Passed generated Go SDK tests and the TypeScript SDK check.
• Passed the gateway-management UI check pipeline with 163 tests.
• Passed the complete web-console check pipeline with 73 tests, localization, production build, and Storybook build.
• The API gateway integration package remains locally blocked because PostgreSQL rejects the repository test credentials; the affected service-account package passes independently.
• Current GitHub Konflux checks are pending. The PR remains draft and conflicts with main.
• No live Keycloak drift session, deployed CA rollover, browser assistive-technology session, or representative-user evaluation was available.

— Amber

Findings Summary (ordered by severity, highest first):

1. [Major] Zero-write convergence accepts behavior-breaking client and role-mapper drift - Identity / Reconciliation (L109, L393)
2. [Major] The due-work drain stops behind a retained first page - Security / Scheduling (L583, L595)

Convention Checklist (omit conventions not applicable to the diff):

Convention	Result
No panic() in production code	Pass
Typed not-found handling distinguishes deletion from transient failure	Pass
No secrets in logs or error messages	Pass
Privileged mutations validate exact managed ownership	Pass
Reconciliation compares every behavior-bearing client and mapper setting	Fail
One-minute expiration work advances across every scan page	Fail
mTLS leaf and trust-anchor rotation works without restart	Pass
Generated OpenAPI SDK artifacts are current	Pass
One-time UI handoff blocks tab, route, history, and hard navigation	Pass
Conventional commit intent is present	Pass

[jsell-rh]

The retained-first-page starvation is fixed by the new immutable keyset scan, and the previously cited client-representation and mapper-config fields now converge correctly. I am still requesting changes because mapper-level protocol is written during repair but omitted from the zero-write convergence model; the generated OpenAPI SDK check is also red for DefaultAPI.md drift.

Major

1. Preserve mapper protocol during zero-write convergence — components/control-plane/internal/serviceaccountkeycloak/client.go:417

   parsedProtocolMapper does not deserialize protocol, and protocolMappersConverged consequently compares only name, mapper provider, and required config. A live audience or role mapper changed away from openid-connect can therefore be accepted as converged even though repair pins that field, leaving the resource ready while its required access-token claims may no longer be emitted. Add protocol to the shared desired/live model, compare it to openid-connect, and cover both managed mappers with drift tests. Confidence: High (98%).

Overall assessment

REQUEST_CHANGES. This self-review is submitted as a GitHub comment because GitHub does not allow an author to request changes on their own PR.

UI standards coverage

Requirement	Status	Evidence
UI-HEX-05	PASS	The merge resolution kept generated-SDK normalization inside the API adapter; the latest web-console and gateway-management UI quality gates pass.
UI-VER-09	PASS	Per-change UI checks completed successfully on 3ad4802.
UI-A11Y-01–10; UI-INT-01–10	NOT_TESTED	No behavior-bearing rendered UI changed in the PR-only delta since 795ec35; browser, keyboard, and assistive-technology journeys were not rerun.

Checks and limitations

• go test -race -count=1 ./plugins/serviceAccounts — pass
• go test -race -count=1 ./internal/serviceaccountkeycloak — pass
• go vet ./plugins/serviceAccounts — pass
• go vet ./internal/serviceaccountkeycloak — pass
• Latest GitHub checks — all code, E2E, lint, UI, repository, and build gates pass except Check generated OpenAPI SDK, which reports only components/api-server/pkg/api/openapi/docs/DefaultAPI.md drift. I did not duplicate that already-actionable CI output as an inline finding.
• Scope: re-reviewed 795ec35..3ad4802, the current-base merge resolution, and both prior open Amber findings. No live Keycloak or manual browser/assistive-technology environment was exercised.

Findings Summary (ordered by severity, highest first):

1. [Major] Mapper protocol drift is still accepted as converged - Reconciliation / Spec Consistency (L417-L450)

Convention Checklist (omit conventions not applicable to the diff):

Convention	Result
No panic() in production code	Pass
Errors wrapped with fmt.Errorf context	Pass
No secrets in logs or responses	Pass
Reconcile every repair-controlled behavior field	Fail
Generated OpenAPI artifacts synchronized	Fail
UI API-adapter boundary preserved	Pass

[jsell-rh]

The prior mapper-protocol finding is fixed on d90e5e7: live and desired mapper protocols share one definition, both managed mappers have drift regressions, and the focused race/vet checks pass. I cannot approve this head while the required generated OpenAPI SDK check remains failing on DefaultAPI.md drift, so the overall assessment stays REQUEST_CHANGES.

Prior finding disposition

The Major at components/control-plane/internal/serviceaccountkeycloak/client.go:417 is fixed. managedProtocolMapper.protocol is now the single source used by repair and convergence, the live representation deserializes it, and each managed mapper has a protocol-only drift regression.

Minor

1. Commit the regenerated OpenAPI documentation — components/api-server/pkg/api/openapi/docs/DefaultAPI.md:93 (and repeated generated table/example rows)

   The required Check generated OpenAPI SDK job still regenerates this file and finds drift. The current difference is mechanical trailing whitespace in generated Markdown, but the checked-in artifact does not match the repository generator and the required gate remains red. Run cd components/api-server && make generate generate-sdk, commit the output, and rerun the check. I did not add a duplicate inline comment because the CI job already identifies the exact file and command. Confidence: High (100%).

Overall assessment

REQUEST_CHANGES. This self-review is submitted as a GitHub comment because GitHub does not allow an author to request changes on their own PR.

UI standards coverage

Requirement	Status	Evidence
UI-VER-09	PASS	The latest web-console and gateway-management UI quality gates pass on d90e5e7.
UI-A11Y-01–10; UI-INT-01–10	N/A	The new commit changes only control-plane mapper reconciliation and tests; it introduces no rendered UI behavior.

Checks and limitations

• go test -race -count=1 ./internal/serviceaccountkeycloak — pass
• go vet ./internal/serviceaccountkeycloak — pass
• Latest GitHub checks — code, E2E, lint, UI, repository, and build checks pass; only Check generated OpenAPI SDK fails, solely on DefaultAPI.md.
• Scope: reviewed 3ad4802..d90e5e7, reconciled the previous open thread, and inspected the current CI result. No live Keycloak environment was exercised.

Findings Summary (ordered by severity, highest first):

1. [Minor] Generated DefaultAPI.md remains out of sync with the repository generator - Generated Artifacts (L93 and repeated generated rows)

Convention Checklist (omit conventions not applicable to the diff):

Convention	Result
No panic() in production code	Pass
Errors wrapped with fmt.Errorf context	Pass
No secrets in logs or responses	Pass
Reconcile every repair-controlled behavior field	Pass
Generated OpenAPI artifacts synchronized	Fail

[jsell-rh]

Addressed the Minor from the latest self-review (generated OpenAPI SDK drift) in 5e8f59a. Ran cd components/api-server && make generate generate-sdk and committed the canonical output: DefaultAPI.md now matches the generator (trailing whitespace in generated Markdown table rows), and sdk-go picked up the Gateway created_by field plus the refreshed spec checksum that the main merge introduced but had not regenerated. The Check generated OpenAPI SDK gate is now green; re-running the generator produces no drift.

[jsell-rh]

The generated OpenAPI artifacts are synchronized on 5e8f59a: the prior DefaultAPI.md drift is closed, and the committed Go SDK matches an independent regeneration exactly. No new correctness, security, or UI issue was found in the latest delta and all applicable head checks are green, so the overall assessment is APPROVE.

Prior finding disposition

The Minor generated-artifact finding from the previous review is fixed: DefaultAPI.md is now canonical, the Go SDK contains the generated Gateway.CreatedBy field and refreshed source checksum, and regeneration produces no SDK diff. The earlier mapper-protocol Major remains fixed on d90e5e7; the latest commit does not touch that control-plane code, and all 18 prior review threads remain resolved.

Overall assessment

APPROVE. This self-review is submitted as a GitHub COMMENT because GitHub does not allow an author to approve their own PR.

UI standards coverage

Requirement	Status	Evidence
UI-VER-09	PASS	The web-console and gateway-management UI quality gates pass on 5e8f59a.
UI-A11Y-01–10; UI-INT-01–10	N/A (latest delta)	The latest commit changes generated documentation and the Go SDK only; it introduces no rendered behavior.

Checks and limitations

• Independent Go SDK regeneration into a temporary directory — exact match
• go test -count=1 ./... in components/sdk-go — pass
• go vet ./... in components/sdk-go — pass
• git diff --ignore-all-space d90e5e7..5e8f59a -- DefaultAPI.md — no semantic documentation change
• GitHub head checks — 17 pass, four expected enterprise-contract results are neutral, one unrelated validation job is skipped, and none are failing or pending
• The generic git diff --check reports the pinned generator's trailing spaces in DefaultAPI.md; the authoritative OpenAPI drift and repository-policy gates pass on that exact canonical output.
• Scope: re-reviewed d90e5e7..5e8f59a, the latest discussion, every prior thread, and current head CI. No live Keycloak or manual browser/assistive-technology journey was rerun for this generated-artifact-only delta.

Findings Summary (ordered by severity, highest first):

1. No findings.

Convention Checklist (omit conventions not applicable to the diff):

Convention	Result
OpenAPI clients generated rather than manually edited	Pass
Generated OpenAPI artifacts synchronized	Pass
Generated SDK preserves the source contract	Pass
No secrets in generated output	Pass
Conventional commit message	Pass