Skip to content

feat(asset-registry): close golden POC and generic MCP onboarding - #62

Merged
kl3inIT merged 5 commits into
mainfrom
kl3inIT/asset-registry-pr5-golden-poc-v2
Jul 26, 2026
Merged

feat(asset-registry): close golden POC and generic MCP onboarding#62
kl3inIT merged 5 commits into
mainfrom
kl3inIT/asset-registry-pr5-golden-poc-v2

Conversation

@kl3inIT

@kl3inIT kl3inIT commented Jul 26, 2026

Copy link
Copy Markdown
Owner

Summary

  • prove the L1 support golden flow from governed authoring through second-user Pack completion and close the increment with deterministic evidence
  • add a generic /connect surface for Claude, Codex, and compatible MCP clients
  • enable Keycloak CIMD plus restricted DCR, migrate existing realms idempotently, proxy RFC 9728 discovery, and strengthen production smoke checks
  • add a real Keycloak 26.7 deployment contract covering repeated migration, PKCE/consent, redirect and scope rejection, and dynamic-client cleanup

Verification

  • ./gradlew.bat --no-daemon clean test (147 suites, 0 failed; follow-up test returned BUILD SUCCESSFUL)
  • OpenFGA: 8/8 tests, 66/66 checks, 27/27 ListObjects
  • corepack pnpm -C web lint, typecheck, build, and generated API drift check
  • Playwright: 8/8 Chromium flows
  • shellcheck, actionlint, production Compose interpolation, nginx proxy regression
  • Keycloak 26.7 MCP onboarding contract, including two idempotent migration passes

Summary by CodeRabbit

  • New Features

    • Added an authenticated Connect page with MCP setup guidance for Claude, Codex, and compatible clients.
    • Exposed /connect guidance-backed MCP discovery via the public OAuth-protected resource endpoint.
    • Added asset ownership health badges (“Orphaned” and “Ownership gap”) on asset detail views.
    • Introduced/expanded governed asset registry onboarding fixtures and golden POC coverage.
  • Bug Fixes

    • Improved forwarded-port handling and strengthened public OAuth/MCP smoke checks.
  • Tests

    • Added end-to-end tests for MCP connect and asset-registry golden journeys.
    • Added a new golden end-to-end asset-registry integration scenario.
  • Documentation

    • Documented governed asset registry concepts and added asset-registry fixture README.

@coderabbitai

coderabbitai Bot commented Jul 26, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@kl3inIT, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 50 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 93a9fc0f-5394-4f8f-a0b2-029bbfe61f79

📥 Commits

Reviewing files that changed from the base of the PR and between 75de4e6 and b0ae5e3.

📒 Files selected for processing (1)
  • infrastructure/deployment/scripts/test-keycloak-mcp-onboarding.sh
📝 Walkthrough

Walkthrough

The change adds asset-registry ownership health and golden-flow coverage, configures and validates MCP onboarding through Keycloak and deployment scripts, exposes protected-resource routing, and adds an authenticated MCP connection guide with end-to-end tests.

Changes

Governed asset registry golden flow

Layer / File(s) Summary
Ownership health and registry contracts
ARCHITECTURE.md, core/src/main/java/com/orgmemory/core/assetregistry/*, web/src/features/assets/components/asset-detail-page.tsx
Asset views now report active owner and backup-owner status, with orphaned and continuity-risk badges rendered in the asset header.
Support registry fixtures
demo/fixtures/asset-registry/*
Fixtures define support prompts, work instructions, knowledge, capability-pack requirements, mock tickets, quality checks, and success metrics.
Golden integration workflow
apps/api/src/test/java/com/orgmemory/api/assetregistry/AssetRegistryIntegrationTests.java, apps/api/src/test/resources/db/test-foundation.sql, apps/api/build.gradle.kts
The integration test exercises asset approval, ownership transfer, evaluation, prompt execution, pack completion, release replacement, withdrawal, metrics, and database assertions.
Governance and journey browser coverage
web/test/e2e/asset-registry-golden-poc.spec.ts
The Playwright test mocks owner and support sessions, validates governance and release state, and completes the support journey.

MCP onboarding and connection

Layer / File(s) Summary
MCP architecture and Keycloak contracts
ARCHITECTURE.md, infrastructure/keycloak/*
Architecture records actor-token exchange, while Keycloak configuration adds CIMD support, scopes, client profiles, policies, and DCR constraints.
Keycloak configuration orchestration
infrastructure/deployment/scripts/configure-keycloak-mcp.sh, infrastructure/deployment/scripts/deploy.sh
Deployment configuration bootstraps Keycloak, merges MCP policy definitions, updates registration components, and verifies the resulting state.
Deployment and proxy validation
.github/workflows/ci.yml, infrastructure/deployment/scripts/*, web/nginx.conf
Deployment invokes MCP configuration; smoke, onboarding, and forwarded-port checks validate metadata, registration behavior, protected responses, and proxy headers.
Authenticated connection surface
web/src/features/mcp/*, web/src/routes/_authenticated/connect.tsx, web/src/routeTree.gen.ts, web/src/components/app-shell/app-sidebar.tsx, web/test/e2e/mcp-connect.spec.ts
An authenticated /connect page provides MCP setup instructions, copy controls, capability details, navigation, routing, and browser coverage.

Estimated code review effort: 4 (Complex) | ~60 minutes

Sequence Diagram(s)

MCP onboarding validation

sequenceDiagram
  participant CI
  participant OnboardingTest
  participant Keycloak
  participant Configurator
  CI->>OnboardingTest: Run onboarding validation
  OnboardingTest->>Keycloak: Start container and await metadata
  OnboardingTest->>Configurator: Configure MCP realm
  Configurator->>Keycloak: Apply scopes, profiles, policies, and DCR rules
  OnboardingTest->>Keycloak: Register and inspect valid client
  OnboardingTest->>Keycloak: Reject invalid clients and delete valid client
Loading

Asset registry golden flow

sequenceDiagram
  participant Owner
  participant SupportAgent
  participant IntegrationTest
  participant KnowledgeSearch
  Owner->>IntegrationTest: Approve prompt, instruction, and capability pack
  IntegrationTest->>KnowledgeSearch: Request permission-aware grounding
  IntegrationTest->>SupportAgent: Transfer capability and assign journey
  SupportAgent->>IntegrationTest: Evaluate prompt and complete journey
  IntegrationTest->>IntegrationTest: Publish replacement and withdraw old release
Loading

Possibly related PRs

  • kl3inIT/OrgMemory#14: Adds typed route title support used by the new authenticated Connect route.
  • kl3inIT/OrgMemory#44: Adds deployment and verification framework extended by the MCP onboarding checks.
  • kl3inIT/OrgMemory#60: Overlaps with the asset ownership-health and golden POC test and fixture changes.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title is concise and covers the two main themes: asset-registry golden POC closure and generic MCP onboarding.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch kl3inIT/asset-registry-pr5-golden-poc-v2

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

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 11

🤖 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
`@apps/api/src/test/java/com/orgmemory/api/assetregistry/AssetRegistryIntegrationTests.java`:
- Around line 1089-1090: Remove the static-text assertion in the golden-flow
test around goldenFixture("success-metrics.json"), or replace it with an
assertion against metrics produced by the executed flow. Parse the observed
metrics and verify the evaluation outcome matches evaluation.passed() or
passedCases(), ensuring the test validates system behavior rather than fixture
contents.
- Around line 1332-1343: The goldenFixture method currently resolves fixtures
from the repository working tree; configure demo/fixtures/asset-registry as an
additional test resource directory in the test build, then update goldenFixture
to load the requested fixture through the test classpath using
getResourceAsStream and preserve IOException handling for missing or unreadable
resources.

In `@demo/fixtures/asset-registry/capability-pack-template.json`:
- Around line 29-32: Update the capability-pack completion validation to load
and evaluate the support.triage-quality@1 checklist directly instead of relying
only on MockTicket::rubricPass. Add an explicit assertion that the rubric
passes, plus a negative completion case proving a failed rubric prevents
completion, before treating this journey as deterministic evidence.
- Around line 4-17: Add the required Support Knowledge Space dependency to
capability-pack-template.json as an authored Knowledge Asset/Version item
matching support.sla-and-escalation@1, using the corresponding non-null fixture
IDs. Update the golden mock and completion assertions so the Pack requires that
Knowledge item to be present and completed, rather than allowing completion with
only the two registry releases.

In `@demo/fixtures/asset-registry/prompt-template.json`:
- Around line 24-31: Add a typed accountableTeam field to the required output
contract in demo/fixtures/asset-registry/prompt-template.json at lines 24-31. In
demo/fixtures/asset-registry/mock-tickets.json at lines 2-9, provide expected
accountable-team values for escalated tickets and include assertions that
validate those values during mocked evaluation.

In `@demo/fixtures/asset-registry/quality-checklist.json`:
- Around line 7-10: Extend the asset integration test’s listing-level assertions
to cover exact grounding coordinates, rejected metadata, retention of raw
variable and output values, and prevention of unauthorized metadata leakage.
Reuse the existing support.sla-and-escalation@1 fixture and rubric/checklist
symbols, asserting each required guarantee explicitly rather than relying only
on aggregate LLM scoring.

In `@demo/fixtures/asset-registry/success-metrics.json`:
- Around line 4-9: Document the zero-denominator policy for the ratio metrics in
the success-metrics fixture, specifically first_time_right, view_to_use,
evaluation_pass, reviewer_correction, and owner_coverage. Add the intended
empty-cohort value (null, 0, or “not applicable”) to each metric’s specification
and ensure the corresponding implementation uses that policy instead of
producing undefined rows.

In `@infrastructure/deployment/scripts/configure-keycloak-mcp.sh`:
- Around line 61-71: Update the basic client-scope handling near the existing
awk check so an existing “basic” scope is also re-synced from basic_scope_source
on every run, rather than only creating it when absent. Reuse the existing scope
ID lookup and update mechanism, or add the minimal equivalent, while preserving
creation for missing scopes and matching the idempotent behavior of
merge_client_policy_document.
- Around line 138-158: Update the registration-policy handling around the
provider_id case statement to derive each kcadm -s configuration argument from
the already parsed registration_policy_source JSON instead of hardcoding
trusted-hosts, allowed-client-templates, and max-clients values. Reuse the
existing JSON parsing mechanism and preserve the current provider selection,
kcadm update flow, and unsupported-policy error behavior so the JSON remains the
single source of truth.

In `@infrastructure/keycloak/mcp-client-profiles.json`:
- Around line 6-21: Remove “localhost” and “127.0.0.1” from the
“cimd-allow-permitted-domains” allowlist in the “client-id-metadata-document”
executor configuration, retaining only the trusted hosted domains. Do not
broaden the permitted-domains list; rely on the existing development-only
loopback/permissive SSRF configuration where applicable.

In `@web/test/e2e/asset-registry-golden-poc.spec.ts`:
- Around line 55-59: Update the three progress assertions around the existing
completion clicks to use exact text matching for “0%”, “50%”, and “100%”.
Preserve the current assertion order and completion flow while preventing
substring matches against other percentage 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: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 9830c133-b466-4c42-b253-8ebe74ef733c

📥 Commits

Reviewing files that changed from the base of the PR and between 64639d4 and f648c56.

⛔ Files ignored due to path filters (13)
  • contracts/openapi.json is excluded by !contracts/openapi.json
  • docs/increments/active/README.md is excluded by !docs/**
  • docs/increments/completed/2026-07-25-unified-asset-registry-definition/design.md is excluded by !docs/**
  • docs/increments/completed/2026-07-25-unified-asset-registry-definition/gate-decisions.md is excluded by !docs/**
  • docs/increments/completed/2026-07-25-unified-asset-registry-definition/plan.md is excluded by !docs/**
  • docs/increments/completed/2026-07-25-unified-asset-registry-definition/ui-reference-audit.md is excluded by !docs/**
  • docs/increments/completed/2026-07-25-unified-asset-registry-definition/verification.md is excluded by !docs/**
  • docs/increments/completed/README.md is excluded by !docs/**
  • docs/roadmap.md is excluded by !docs/**
  • docs/runbooks/mcp-asset-delivery.md is excluded by !docs/**
  • docs/specs/domains/asset-registry.md is excluded by !docs/**
  • docs/tests/domains/asset-registry.md is excluded by !docs/**
  • docs/vision.md is excluded by !docs/**
📒 Files selected for processing (32)
  • .github/workflows/ci.yml
  • ARCHITECTURE.md
  • apps/api/src/test/java/com/orgmemory/api/assetregistry/AssetRegistryIntegrationTests.java
  • apps/api/src/test/resources/db/test-foundation.sql
  • core/src/main/java/com/orgmemory/core/assetregistry/AssetRegistryCoordinator.java
  • core/src/main/java/com/orgmemory/core/assetregistry/AssetView.java
  • demo/fixtures/asset-registry/README.md
  • demo/fixtures/asset-registry/capability-pack-template.json
  • demo/fixtures/asset-registry/mock-tickets.json
  • demo/fixtures/asset-registry/prompt-template.json
  • demo/fixtures/asset-registry/quality-checklist.json
  • demo/fixtures/asset-registry/success-metrics.json
  • demo/fixtures/asset-registry/support-sla-and-escalation.md
  • demo/fixtures/asset-registry/work-instruction.json
  • infrastructure/deployment/scripts/configure-keycloak-mcp.sh
  • infrastructure/deployment/scripts/deploy.sh
  • infrastructure/deployment/scripts/smoke-production.sh
  • infrastructure/deployment/scripts/test-keycloak-mcp-onboarding.sh
  • infrastructure/deployment/scripts/test-web-forwarded-port.sh
  • infrastructure/keycloak/Dockerfile
  • infrastructure/keycloak/mcp-basic-client-scope.json
  • infrastructure/keycloak/mcp-client-policies.json
  • infrastructure/keycloak/mcp-client-profiles.json
  • infrastructure/keycloak/mcp-dcr-registration-policy.json
  • web/nginx.conf
  • web/src/components/app-shell/app-sidebar.tsx
  • web/src/features/assets/components/asset-detail-page.tsx
  • web/src/features/mcp/components/mcp-connect-page.tsx
  • web/src/routeTree.gen.ts
  • web/src/routes/_authenticated/connect.tsx
  • web/test/e2e/asset-registry-golden-poc.spec.ts
  • web/test/e2e/mcp-connect.spec.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (3)
  • GitHub Check: PostgreSQL GraphRAG
  • GitHub Check: Backend · Java 25
  • GitHub Check: Deployment contracts
🧰 Additional context used
📓 Path-based instructions (5)
**/*

📄 CodeRabbit inference engine (CLAUDE.md)

**/*: Record current behavior in architecture/specification documentation only after it exists in code; keep intended behavior in vision, roadmap, or an active increment, and do not duplicate state.
Before using unfamiliar Spring Boot 4, Spring Modulith 2, Spring AI 2, Gradle, React, Vite, Tailwind, or TypeScript APIs, consult current official documentation via Context7 and the project orgmemory-* verification skills.
Before retrieval, AI, MCP, permission, upload, graph, or export work, read docs/guidelines/agent-safety.md.
Never commit .env files, provider keys, tokens, or customer data.
Run the relevant verification gates from docs/guidelines/testing-harness.md; use a terminating clean test as the context gate, and do not treat bootRun as verification.

Files:

  • infrastructure/keycloak/mcp-dcr-registration-policy.json
  • web/src/routes/_authenticated/connect.tsx
  • web/test/e2e/mcp-connect.spec.ts
  • demo/fixtures/asset-registry/README.md
  • infrastructure/keycloak/mcp-basic-client-scope.json
  • demo/fixtures/asset-registry/mock-tickets.json
  • infrastructure/keycloak/mcp-client-policies.json
  • demo/fixtures/asset-registry/capability-pack-template.json
  • demo/fixtures/asset-registry/success-metrics.json
  • demo/fixtures/asset-registry/support-sla-and-escalation.md
  • infrastructure/keycloak/Dockerfile
  • demo/fixtures/asset-registry/quality-checklist.json
  • web/src/components/app-shell/app-sidebar.tsx
  • infrastructure/keycloak/mcp-client-profiles.json
  • demo/fixtures/asset-registry/prompt-template.json
  • demo/fixtures/asset-registry/work-instruction.json
  • infrastructure/deployment/scripts/deploy.sh
  • core/src/main/java/com/orgmemory/core/assetregistry/AssetView.java
  • infrastructure/deployment/scripts/smoke-production.sh
  • web/src/features/assets/components/asset-detail-page.tsx
  • web/src/features/mcp/components/mcp-connect-page.tsx
  • infrastructure/deployment/scripts/test-web-forwarded-port.sh
  • web/nginx.conf
  • core/src/main/java/com/orgmemory/core/assetregistry/AssetRegistryCoordinator.java
  • web/test/e2e/asset-registry-golden-poc.spec.ts
  • ARCHITECTURE.md
  • infrastructure/deployment/scripts/test-keycloak-mcp-onboarding.sh
  • infrastructure/deployment/scripts/configure-keycloak-mcp.sh
  • web/src/routeTree.gen.ts
  • apps/api/src/test/java/com/orgmemory/api/assetregistry/AssetRegistryIntegrationTests.java
  • apps/api/src/test/resources/db/test-foundation.sql
**/*.{ts,tsx,js,jsx,css,scss,html}

📄 CodeRabbit inference engine (CLAUDE.md)

For frontend files, run Oxlint, TypeScript typecheck, the production build, and browser tests when the UI flow matters; do not run JetBrains IDE inspection on TypeScript, TSX, or web configuration.

Files:

  • web/src/routes/_authenticated/connect.tsx
  • web/test/e2e/mcp-connect.spec.ts
  • web/src/components/app-shell/app-sidebar.tsx
  • web/src/features/assets/components/asset-detail-page.tsx
  • web/src/features/mcp/components/mcp-connect-page.tsx
  • web/test/e2e/asset-registry-golden-poc.spec.ts
  • web/src/routeTree.gen.ts
web/src/**/*.{ts,tsx}

⚙️ CodeRabbit configuration file

web/src/**/*.{ts,tsx}: OAuth access and refresh tokens must never enter browser JavaScript or
browser storage. Use the HttpOnly BFF session, CSRF-protected mutations,
generated Hey API data clients, accessible states, and both light and
dark themes. Handwritten transport is reserved for documented protocol
flows such as navigation redirects and streaming.

Files:

  • web/src/routes/_authenticated/connect.tsx
  • web/src/components/app-shell/app-sidebar.tsx
  • web/src/features/assets/components/asset-detail-page.tsx
  • web/src/features/mcp/components/mcp-connect-page.tsx
  • web/src/routeTree.gen.ts
.github/**/*.{yml,yaml}

⚙️ CodeRabbit configuration file

.github/**/*.{yml,yaml}: Require least-privilege permissions, explicit release tags for actions,
bounded job timeouts, concurrency cancellation, frozen lockfiles, and no
secrets in pull-request workflows. GitHub Actions are intentionally not
pinned to commit SHAs; Dependabot owns their scheduled version updates.

Files:

  • .github/workflows/ci.yml
**/*.{java,kt}

📄 CodeRabbit inference engine (CLAUDE.md)

JetBrains IDE inspection is a verification gate for the Java backend.

Files:

  • core/src/main/java/com/orgmemory/core/assetregistry/AssetView.java
  • core/src/main/java/com/orgmemory/core/assetregistry/AssetRegistryCoordinator.java
  • apps/api/src/test/java/com/orgmemory/api/assetregistry/AssetRegistryIntegrationTests.java
🧠 Learnings (1)
📚 Learning: 2026-07-24T22:52:57.466Z
Learnt from: kl3inIT
Repo: kl3inIT/OrgMemory PR: 40
File: .github/workflows/ci.yml:126-126
Timestamp: 2026-07-24T22:52:57.466Z
Learning: In this repository’s GitHub Actions workflows, the `uses:` field may intentionally reference GitHub Actions by explicit release tags (not immutable commit SHAs) per the project’s OrgMemory policy. Do not flag tag-based `uses:` references as “unpinned” if they are release-tag-based (e.g., `owner/repovX.Y.Z`) and follow the repo’s Dependabot-owned scheduled updates approach.

Applied to files:

  • .github/workflows/ci.yml
🪛 ast-grep (0.44.1)
infrastructure/deployment/scripts/test-keycloak-mcp-onboarding.sh

[warning] 120-120: Writing to or reading from a hardcoded, predictable path under /tmp is vulnerable to symlink and TOCTOU attacks: a local attacker can pre-create the file (or a symlink pointing elsewhere) and hijack or corrupt the contents. Generate a unique, unpredictable temporary file with mktemp instead, e.g. tmpfile="$(mktemp)" (or mktemp -d for directories) and reference "$tmpfile".
Context: /tmp/orgmemory-mcp-test.config
Note: [CWE-377] Insecure Temporary File.

(predictable-tmp-file-bash)

apps/api/src/test/java/com/orgmemory/api/assetregistry/AssetRegistryIntegrationTests.java

[warning] 864-864: Avoid LDAP injections
Context: knowledgeSearch.search(any(), any(), any(), any())
Note: [CWE-90] Improper Neutralization of Special Elements used in an LDAP Query ('LDAP Injection'). Security best practice.

(ldap-injection-java)

🪛 LanguageTool
demo/fixtures/asset-registry/README.md

[style] ~20-~20: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...dure. - capability-pack-template.json is resolved with exact release UUIDs by th...

(ENGLISH_WORD_REPEAT_BEGINNING_RULE)

🔇 Additional comments (30)
ARCHITECTURE.md (1)

12-12: LGTM!

Also applies to: 42-43, 57-66, 77-94

core/src/main/java/com/orgmemory/core/assetregistry/AssetView.java (1)

19-19: LGTM!

Also applies to: 116-122

core/src/main/java/com/orgmemory/core/assetregistry/AssetRegistryCoordinator.java (2)

602-602: LGTM!

Also applies to: 624-626


628-650: LGTM!

web/src/features/assets/components/asset-detail-page.tsx (1)

194-202: LGTM!

demo/fixtures/asset-registry/work-instruction.json (1)

1-56: LGTM!

apps/api/src/test/java/com/orgmemory/api/assetregistry/AssetRegistryIntegrationTests.java (2)

49-55: LGTM!

Also applies to: 93-96, 115-120, 163-165, 182-184, 855-1088, 1310-1330, 1345-1365


76-77: 📐 Maintainability & Code Quality

No change needed for Jackson 3 new ObjectMapper().

Jackson 3 ObjectMapper construction is still part of the public API, and existing code uses new ObjectMapper() in several modules; the newer JsonMapper.builder().build() idiom is not required here.

web/test/e2e/asset-registry-golden-poc.spec.ts (2)

71-159: LGTM!


1-16: LGTM!

Also applies to: 18-54, 60-69, 161-345

apps/api/src/test/resources/db/test-foundation.sql (1)

72-82: 🗄️ Data Integrity & Integration

No change needed. TEAM_LEAD exists on core/src/main/java/com/orgmemory/core/organization/UserRole.java, and AppUser.role persists UserRole by string name.

demo/fixtures/asset-registry/README.md (1)

1-29: LGTM!

demo/fixtures/asset-registry/support-sla-and-escalation.md (1)

1-25: LGTM!

infrastructure/keycloak/Dockerfile (1)

8-8: LGTM!

Also applies to: 28-28

infrastructure/deployment/scripts/test-keycloak-mcp-onboarding.sh (2)

68-73: Good idempotency coverage.

Running configure-keycloak-mcp.sh twice back-to-back exercises the idempotent-migration requirement called out in the PR objectives.


1-201: LGTM!

infrastructure/deployment/scripts/test-web-forwarded-port.sh (2)

101-110: LGTM!


38-38: 🎯 Functional Correctness

No change needed. The mock backend container still declares --network-alias api, so /api/probe coverage remains resolvable after the added mcp alias/port changes.

web/nginx.conf (1)

97-108: LGTM! The path correctly matches RFC 9728's well-known suffix placement for a resource identifier with a path component (/mcp).

web/src/components/app-shell/app-sidebar.tsx (1)

1-1: LGTM!

Also applies to: 27-27

web/src/features/mcp/components/mcp-connect-page.tsx (1)

1-282: LGTM!

web/src/routeTree.gen.ts (1)

16-16: LGTM! Auto-generated route wiring is consistent with the new /connect route file.

Also applies to: 54-58, 155-155, 176-176, 201-201, 227-227, 248-248, 272-272, 329-335, 459-459, 469-469

web/src/routes/_authenticated/connect.tsx (1)

1-9: LGTM!

web/test/e2e/mcp-connect.spec.ts (1)

1-55: LGTM! Good coverage of tab switching and a browser-error assertion.

infrastructure/keycloak/mcp-basic-client-scope.json (1)

1-36: LGTM!

infrastructure/keycloak/mcp-client-policies.json (1)

1-45: LGTM!

infrastructure/keycloak/mcp-dcr-registration-policy.json (1)

1-31: LGTM!

infrastructure/deployment/scripts/deploy.sh (1)

156-158: LGTM!

.github/workflows/ci.yml (1)

396-398: LGTM!

infrastructure/deployment/scripts/smoke-production.sh (1)

104-123: 🩺 Stability & Availability

No change needed. smoke-production.sh only sets and clears the MCP challenge-header EXIT trap in this block; there are no other EXIT trap registrations to clobber.

Comment thread demo/fixtures/asset-registry/capability-pack-template.json
Comment on lines +29 to +32
"completionCriteria": [
"Both required released items are complete",
"One mock ticket passes support.triage-quality@1",
"The execution trace pins the Prompt release and grounding citation"

@coderabbitai coderabbitai Bot Jul 26, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

Make the declared completion criteria executable.

The pack requires a ticket to pass support.triage-quality@1, but the integration only evaluates prompt cases and trusts MockTicket::rubricPass; it never loads the checklist or verifies that a failed rubric prevents completion. Add a direct rubric assertion and a negative completion case before using this journey as deterministic evidence.

🤖 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 `@demo/fixtures/asset-registry/capability-pack-template.json` around lines 29 -
32, Update the capability-pack completion validation to load and evaluate the
support.triage-quality@1 checklist directly instead of relying only on
MockTicket::rubricPass. Add an explicit assertion that the rubric passes, plus a
negative completion case proving a failed rubric prevents completion, before
treating this journey as deterministic evidence.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in 75de4e6 within the POC contract: rubricPass is no longer trusted as fixture truth. The test evaluates category, SLA, escalation, accountable team, allowed grounding coordinate, and the returned citation; a deliberately wrong/ungrounded result fails before the journey is completed. Pack progress remains an explicit user journey by design rather than introducing a workflow/rubric engine in this increment.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

test_file="$(fd -a '^AssetRegistryIntegrationTests\.java$' apps/api/src/test/java | head -n1)"
printf '%s\n' "== $test_file =="
ast-grep outline "$test_file" --items all --match 'goldenPoc|rubric|journey|ticket' || true
printf '%s\n' '== Relevant assertions and helpers =='
rg -n -i -C 8 \
  'goldenPocTransfersAReleasedSupportCapabilityToASecondUser|rubric|grounding|citation|accountable|journey|complete' \
  "$test_file"

Length of output: 16698


@kl3inIT, the POC-level validation is now present: ticketPassesRubric(...) checks the output fields and grounding/citation evidence, and the deliberately invalid result is asserted to fail before the journey is completed.

However, the negative case does not prevent completion: after that assertion, Lines 1061–1072 still mark every pack item complete unconditionally. The test also does not load quality-checklist.json directly. Since you’ve clarified that pack progress is intentionally an explicit user journey rather than a rubric-gated workflow in this increment, I’ll retain the finding only for the mismatch between the declared completion criterion and its enforceability—not as a request to introduce a workflow engine here.

Comment thread demo/fixtures/asset-registry/prompt-template.json
Comment thread demo/fixtures/asset-registry/success-metrics.json Outdated
Comment thread infrastructure/deployment/scripts/configure-keycloak-mcp.sh
Comment thread infrastructure/deployment/scripts/configure-keycloak-mcp.sh Outdated
Comment thread infrastructure/keycloak/mcp-client-profiles.json
Comment thread web/test/e2e/asset-registry-golden-poc.spec.ts Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 `@infrastructure/deployment/scripts/test-keycloak-mcp-onboarding.sh`:
- Around line 108-111: Update the basic scope assertion near
expected_description to load the expected description from the same
mcp-basic-client-scope.json fixture used by configure-keycloak-mcp.sh, rather
than duplicating its string literal. Keep the assertion against
scope["description"] and preserve the existing fixture-driven validation pattern
used elsewhere for registration-policy settings.
🪄 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: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: d5fcba1e-9b4a-4f03-8265-6f08a1f7b62a

📥 Commits

Reviewing files that changed from the base of the PR and between f648c56 and 75de4e6.

📒 Files selected for processing (11)
  • apps/api/build.gradle.kts
  • apps/api/src/test/java/com/orgmemory/api/assetregistry/AssetRegistryIntegrationTests.java
  • demo/fixtures/asset-registry/capability-pack-template.json
  • demo/fixtures/asset-registry/mock-tickets.json
  • demo/fixtures/asset-registry/prompt-template.json
  • demo/fixtures/asset-registry/quality-checklist.json
  • demo/fixtures/asset-registry/success-metrics.json
  • infrastructure/deployment/scripts/configure-keycloak-mcp.sh
  • infrastructure/deployment/scripts/test-keycloak-mcp-onboarding.sh
  • infrastructure/keycloak/mcp-client-profiles.json
  • web/test/e2e/asset-registry-golden-poc.spec.ts
💤 Files with no reviewable changes (1)
  • infrastructure/keycloak/mcp-client-profiles.json
📜 Review details
⏰ Context from checks skipped due to timeout. (4)
  • GitHub Check: Backend · Java 25
  • GitHub Check: Deployment contracts
  • GitHub Check: Web · Node 24
  • GitHub Check: PostgreSQL GraphRAG
🧰 Additional context used
📓 Path-based instructions (3)
**/*

📄 CodeRabbit inference engine (CLAUDE.md)

**/*: Record current behavior in architecture/specification documentation only after it exists in code; keep intended behavior in vision, roadmap, or an active increment, and do not duplicate state.
Before using unfamiliar Spring Boot 4, Spring Modulith 2, Spring AI 2, Gradle, React, Vite, Tailwind, or TypeScript APIs, consult current official documentation via Context7 and the project orgmemory-* verification skills.
Before retrieval, AI, MCP, permission, upload, graph, or export work, read docs/guidelines/agent-safety.md.
Never commit .env files, provider keys, tokens, or customer data.
Run the relevant verification gates from docs/guidelines/testing-harness.md; use a terminating clean test as the context gate, and do not treat bootRun as verification.

Files:

  • demo/fixtures/asset-registry/mock-tickets.json
  • demo/fixtures/asset-registry/quality-checklist.json
  • demo/fixtures/asset-registry/capability-pack-template.json
  • apps/api/build.gradle.kts
  • demo/fixtures/asset-registry/prompt-template.json
  • demo/fixtures/asset-registry/success-metrics.json
  • web/test/e2e/asset-registry-golden-poc.spec.ts
  • infrastructure/deployment/scripts/configure-keycloak-mcp.sh
  • infrastructure/deployment/scripts/test-keycloak-mcp-onboarding.sh
  • apps/api/src/test/java/com/orgmemory/api/assetregistry/AssetRegistryIntegrationTests.java
**/*.{ts,tsx,js,jsx,css,scss,html}

📄 CodeRabbit inference engine (CLAUDE.md)

For frontend files, run Oxlint, TypeScript typecheck, the production build, and browser tests when the UI flow matters; do not run JetBrains IDE inspection on TypeScript, TSX, or web configuration.

Files:

  • web/test/e2e/asset-registry-golden-poc.spec.ts
**/*.{java,kt}

📄 CodeRabbit inference engine (CLAUDE.md)

JetBrains IDE inspection is a verification gate for the Java backend.

Files:

  • apps/api/src/test/java/com/orgmemory/api/assetregistry/AssetRegistryIntegrationTests.java
🧠 Learnings (1)
📚 Learning: 2026-07-26T09:47:42.916Z
Learnt from: kl3inIT
Repo: kl3inIT/OrgMemory PR: 62
File: demo/fixtures/asset-registry/success-metrics.json:0-0
Timestamp: 2026-07-26T09:47:42.916Z
Learning: In asset-registry fixture JSON files, ratio metrics should set `emptyCohortValue` to `null` (meaning “not applicable”) when the cohort is empty. Do not coerce an empty cohort’s value to `0`; only use `0` when it is explicitly applicable and measured per the measurement policy.

Applied to files:

  • demo/fixtures/asset-registry/mock-tickets.json
  • demo/fixtures/asset-registry/quality-checklist.json
  • demo/fixtures/asset-registry/capability-pack-template.json
  • demo/fixtures/asset-registry/prompt-template.json
  • demo/fixtures/asset-registry/success-metrics.json
🔇 Additional comments (15)
apps/api/src/test/java/com/orgmemory/api/assetregistry/AssetRegistryIntegrationTests.java (2)

1029-1043: Make the checklist gate pack completion.

ticketPassesRubric still hard-codes only part of quality-checklist.json, and the negative helper assertion does not prevent the subsequent completion loop from completing the Pack. Load/evaluate every checklist requirement and prove a failed rubric blocks completion.

Also applies to: 1061-1072, 1408-1423


874-936: 📐 Maintainability & Code Quality

Run the required backend verification gates.

Run JetBrains IDE inspection and the relevant terminating clean test before merge.

As per coding guidelines, “JetBrains IDE inspection is a verification gate for the Java backend” and “use a terminating clean test as the context gate.”

Also applies to: 1002-1072, 1375-1424

Source: Coding guidelines

demo/fixtures/asset-registry/capability-pack-template.json (1)

10-17: LGTM!

Also applies to: 39-41

demo/fixtures/asset-registry/mock-tickets.json (1)

2-9: LGTM!

demo/fixtures/asset-registry/prompt-template.json (1)

24-32: LGTM!

Also applies to: 44-108

demo/fixtures/asset-registry/quality-checklist.json (1)

3-10: LGTM!

demo/fixtures/asset-registry/success-metrics.json (1)

4-12: LGTM!

apps/api/build.gradle.kts (1)

2-2: LGTM!

Also applies to: 47-50

web/test/e2e/asset-registry-golden-poc.spec.ts (1)

57-71: 📐 Maintainability & Code Quality

Run the required frontend verification gates.

Run Oxlint, TypeScript typecheck, the production build, and browser tests for this changed onboarding flow.

As per coding guidelines, “For frontend files, run Oxlint, TypeScript typecheck, the production build, and browser tests when the UI flow matters.”

Also applies to: 75-160, 177-354

Source: Coding guidelines

infrastructure/deployment/scripts/configure-keycloak-mcp.sh (4)

68-102: Idempotent re-sync for "basic" scope now implemented.

This now updates the existing "basic" client scope (preserving id and protocol-mapper ids) instead of skipping when it already exists, resolving the previously flagged gap where drift wasn't corrected.


105-138: LGTM!


170-194: Registration-policy settings now sourced from JSON instead of hardcoded.

policy_settings is now derived from registration_policy_source via the embedded Python script rather than hardcoded per-provider -s flags, resolving the previously flagged duplication with mcp-dcr-registration-policy.json.


219-244: LGTM!

infrastructure/deployment/scripts/test-keycloak-mcp-onboarding.sh (2)

1-67: LGTM!

Also applies to: 114-126, 170-210, 212-231


80-89: 🎯 Functional Correctness

No change needed: -q name=basic is no longer used.

The client-scope lookup fetches the client-scopes list and selects the "basic" scope in Python instead of indexing the filtered result.

Comment thread infrastructure/deployment/scripts/test-keycloak-mcp-onboarding.sh Outdated
@kl3inIT
kl3inIT merged commit 8bf7df4 into main Jul 26, 2026
11 checks passed
@kl3inIT
kl3inIT deleted the kl3inIT/asset-registry-pr5-golden-poc-v2 branch July 26, 2026 10:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant