docs: establish identity product and architecture baseline - #75
Conversation
|
Warning Review limit reached
Next review available in: 47 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the 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 configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughKeyverse의 제품·기술 요구사항, 데이터 모델, 보안·운영 기준, ADR, UML 및 문서 계약 테스트를 추가했다. 문서는 protected main, active PR, Keycloak 소유 데이터와 desired-state reconciliation 경계를 정의한다. Changes제품 계약과 거버넌스
영속성·신원 수명주기
보안·운영·검증
Estimated code review effort: 3 (Moderate) | ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/adr/0006-user-operation-lock.md`:
- Around line 1-6: Include the SCIM patch_user flow in the shared cross-process
user-operation lock boundary. Protect its get_user and deactivate_user
read-modify-write sequence with user_operation_locks.hold(user_id), matching
replace_user; otherwise explicitly exclude PATCH through a separate contract and
update the ADR and merge documentation accordingly.
In `@docs/adr/README.md`:
- Line 19: The documentation contract uses the undefined term “doctoring”;
replace it with the established documentation, doctrine, or actual runbook
terminology and path. Update docs/adr/README.md lines 19-19 and
docs/TEST_STRATEGY.md lines 88-88 consistently, including the
“doctoring/operations links” reference.
In `@docs/ERD.md`:
- Around line 105-111: Update the EXTERNAL_IDENTITY_LINK definition to add a
composite unique constraint on (federation_source_id, external_subject_hash),
preventing one external subject within a federation source from linking to
multiple Keycloak users. Document that federation_source_id defines the
identity-provider scope for this uniqueness rule.
- Around line 31-37: Update the ERD definitions for IDP_CONFIG_ENTRY and the
related entities at the referenced sections so config_key, federation_alias,
directory_alias, client_id, and keycloak_user_uuid are documented as
tenant-scoped composite unique keys with tenant_deployment_id, while retaining
global uniqueness only for UUID identifiers.
- Around line 51-58: Update the three receipt entities, including
FEDERATION_APPLY_RECEIPT, to store the desired-state version or hash alongside
observed_state_hash, outcome, and timestamp. Define documentation rules for
identifying the latest desired version and handling duplicate receipts,
consistent with the versioned contract in THREAT_MODEL.md.
In `@docs/OPERABILITY.md`:
- Line 59: Update the wording around the PR `#72` claim mapper acceptance
statement in OPERABILITY.md to consistently use “Naruon” for the
product/platform and “naruon-web” only for the RP client ID. Clarify whether
authorization readiness refers to the Naruon product or the naruon-web client so
operators know which authentication flow and scope to validate.
In `@docs/TRD.md`:
- Around line 45-49: Synchronize the native loopback redirect exception between
docs/TRD.md and docs/PRD.md: either remove “except separately reviewed native
loopback profile” from the RP clients contract, or, if native loopback is
supported, document its exact exception conditions in PRD-FR-005, the threat
model, test strategy, and traceability documentation.
In `@docs/UML.md`:
- Around line 24-28: Update the UML storage relationships so the Keycloak
PostgreSQL node is explicitly Keycloak-owned and the Keyverse-owned store is
represented as a separate node. Remove the direct ADMIN --> PG relationship,
retain Keycloak’s connection to its own database, and route ADMIN through the
Keycloak Admin API to the Keycloak engine while preserving the existing
deployment relationships.
In `@tests/test_documentation_contract.py`:
- Around line 68-81: Update test_adr_index_contains_governing_identity_decisions
to verify each listed ADR both appears in docs/adr/README.md and exists as a
file at ROOT / "docs" / "adr" / adr, using is_file().
- Around line 49-57: Strengthen test_active_pr_features_are_not_promoted_to_main
so each PR’s identifier is directly associated with active-PR in both
docs/PRD.md and docs/TRACEABILITY.md. Replace the current independent substring
assertions and unrelated RP `#72` exclusion with per-PR checks that validate PR
`#72` and PR `#74` each retain active-PR status in their corresponding documentation
entries.
🪄 Autofix
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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 16a1fb5b-db38-42b8-b622-4be30efcd86c
📒 Files selected for processing (18)
DOCUMENTATION.mddocs/ERD.mddocs/OPERABILITY.mddocs/PRD.mddocs/TEST_STRATEGY.mddocs/THREAT_MODEL.mddocs/TRACEABILITY.mddocs/TRD.mddocs/UML.mddocs/adr/0001-keycloak-hub.mddocs/adr/0002-passwordless-local-accounts.mddocs/adr/0003-identity-matching.mddocs/adr/0004-desired-state-reconciliation.mddocs/adr/0005-secret-ownership.mddocs/adr/0006-user-operation-lock.mddocs/adr/0007-automation-authority.mddocs/adr/README.mdtests/test_documentation_contract.py
|
@coderabbitai review Please re-review current exact head |
|
I will verify the source and documentation contract for:
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/ERD.md (1)
187-189: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winERD의
USER_OPERATION_LOCK_STATE와 구현된 잠금 스키마의 차이를 매핑하세요.
USER_OPERATION_LOCK_STATE가 물리 스키마라면lock_name TEXT PRIMARY KEY와requested_user_ids TEXT가 있는 현재SqliteUserOperationLocks._SCHEMA와 일치하지 않습니다. 논리 모델이라면 SQLite sidecar의 전역 mutex를 ERD의 per-user FK/lease 필드에 어떻게 매핑하는지 문서에 명시하세요. 어용지 구현 없이lease_expires_at동시성 테스트를 검증할 수 없습니다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/ERD.md` around lines 187 - 189, Update the ERD documentation to explicitly map USER_OPERATION_LOCK_STATE to the implementation in SqliteUserOperationLocks._SCHEMA: document whether it is a logical model or physical schema, reconcile lock_name TEXT PRIMARY KEY and requested_user_ids TEXT with the ERD fields, and describe how the SQLite sidecar’s global mutex maps to per-user foreign-key and lease fields, including the lease_expires_at concurrency-test implications.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/ERD.md`:
- Line 154: Update the physical migration constraints for EXTERNAL_IDENTITY_LINK
and ACCOUNT_MERGE_AUDIT so each child record references parent records within
the same TENANT_DEPLOYMENT, using composite foreign keys or an equivalent
database constraint that includes tenant_deployment_id. Ensure cross-tenant
federation-source, Keycloak-user, and merge-audit associations are rejected
regardless of documentation labels.
---
Outside diff comments:
In `@docs/ERD.md`:
- Around line 187-189: Update the ERD documentation to explicitly map
USER_OPERATION_LOCK_STATE to the implementation in
SqliteUserOperationLocks._SCHEMA: document whether it is a logical model or
physical schema, reconcile lock_name TEXT PRIMARY KEY and requested_user_ids
TEXT with the ERD fields, and describe how the SQLite sidecar’s global mutex
maps to per-user foreign-key and lease fields, including the lease_expires_at
concurrency-test implications.
🪄 Autofix
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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 93be661b-de48-455f-941e-b30b02630e44
📒 Files selected for processing (11)
docs/ERD.mddocs/OPERABILITY.mddocs/PRD.mddocs/TEST_STRATEGY.mddocs/TRACEABILITY.mddocs/TRD.mddocs/UML.mddocs/adr/0006-user-operation-lock.mddocs/adr/README.mddocs/merge-unification-flow.mdtests/test_documentation_contract.py
🚧 Files skipped from review as they are similar to previous changes (8)
- docs/PRD.md
- docs/UML.md
- docs/TRD.md
- tests/test_documentation_contract.py
- docs/TEST_STRATEGY.md
- docs/adr/README.md
- docs/OPERABILITY.md
- docs/TRACEABILITY.md
![opencode-agent[bot]](https://avatars.githubusercontent.com/in/1549082?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
OpenCode cannot approve yet because required coverage evidence did not pass.
Review outcome
1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence
-
Problem: The required coverage-evidence job result was
failure, so OpenCode cannot establish approval sufficiency for this head. -
Root cause: Automated approval is only valid when the same-head coverage-evidence job proves supported repository test suites passed and configured docstring gates passed or were advisory, or reports not applicable because no supported source files or package manifests exist. Missing, failed, skipped, unavailable, or unsupported-tooling test evidence is a blocker.
-
Fix: Install or configure the repository test/docstring evidence tooling when source files or package manifests exist, rerun the current-head coverage-evidence job, and approve only after it reports
successwith required evidence or explicit no-source not-applicable evidence. -
Regression test: Keep the approval branch checking
needs.coverage-evidence.result == successbefore posting APPROVE, and publish REQUEST_CHANGES when coverage-evidence blocker states such as cancelled, skipped, failed, unsupported-tooling, or below-100 evidence are present. -
Result: REQUEST_CHANGES
-
Reason: coverage-evidence result was
failure, so required test/docstring evidence was not proven for current head4d2841071e9a8136298bb7198229d47ff406284d. -
Head SHA:
4d2841071e9a8136298bb7198229d47ff406284d -
Workflow run: 31315176332
-
Workflow attempt: 1
Coverage evidence
Coverage evidence job did not run or did not publish coverage evidence.
Changed-File Evidence Map
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Changed file: DOCUMENTATION.md"]
S1 --> I1["repository behavior"]
I1 --> R1["Review risk: Changed file: DOCUMENTATION.md"]
R1 --> V1["required checks"]
Evidence --> S2["Docs (17 files)"]
S2 --> I2["operator or user guidance"]
I2 --> R2["Review risk: Docs (17 files)"]
R2 --> V2["docs review"]
Evidence --> S3["Test: test_documentation_contract.py"]
S3 --> I3["regression suite"]
I3 --> R3["Review risk: Test: test_documentation_contract.py"]
R3 --> V3["targeted test run"]
OpenCode Review Overview
Pull request overviewOpenCode cannot approve yet because required coverage evidence did not pass. Review outcome1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence
Coverage evidenceCoverage evidence job did not run or did not publish coverage evidence. Changed-File Evidence Mapflowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Changed file: DOCUMENTATION.md"]
S1 --> I1["repository behavior"]
I1 --> R1["Review risk: Changed file: DOCUMENTATION.md"]
R1 --> V1["required checks"]
Evidence --> S2["Docs (17 files)"]
S2 --> I2["operator or user guidance"]
I2 --> R2["Review risk: Docs (17 files)"]
R2 --> V2["docs review"]
Evidence --> S3["Test: test_documentation_contract.py"]
S3 --> I3["regression suite"]
I3 --> R3["Review risk: Test: test_documentation_contract.py"]
R3 --> V3["targeted test run"]
|
|
Review follow-up on exact head |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/adr/0008-keyverse-rp-authorization-boundary.md`:
- Around line 15-17: Update the external RP audit documentation to record the
README-referenced revision and each audited RP’s immutable commit SHA, branch,
or equivalent evidence link alongside the audit date and repository paths.
Ensure the authorization status in Lines 21-26 remains reproducible;
alternatively, explicitly label the table as a snapshot and exclude it from
evidence supporting the authorization-ready determination.
- Around line 49-57: Update the Keyverse claim contract in this ADR to
explicitly define required and optional claims, enumerate the allowed bounded
role values and format, and specify tenant/workspace binding and validation
rules for org and workspace. Reconcile the statement that applications need not
support every claim with these requirements, and link the contract to the
relevant acceptance tests so RP implementations use consistent authorization
rules.
🪄 Autofix
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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 5190ffa2-7edc-4af9-b56b-945ec63a593a
📒 Files selected for processing (13)
ARCHITECTURE.mdCHANGELOG.mddocs/ERD.mddocs/OPERABILITY.mddocs/PRD.mddocs/TEST_STRATEGY.mddocs/THREAT_MODEL.mddocs/TRACEABILITY.mddocs/TRD.mddocs/UML.mddocs/adr/0008-keyverse-rp-authorization-boundary.mddocs/adr/README.mdtests/test_documentation_contract.py
🚧 Files skipped from review as they are similar to previous changes (8)
- docs/adr/README.md
- tests/test_documentation_contract.py
- docs/UML.md
- docs/THREAT_MODEL.md
- docs/OPERABILITY.md
- docs/TEST_STRATEGY.md
- docs/ERD.md
- docs/TRACEABILITY.md
|
@opencode-agent @cwl-noema-review Review exact current head |
|
Correction: exact current head is |
Superseded by current head 4da37dc: the predecessor coverage-evidence blocker was resolved, ADR findings were addressed, and all current-head checks pass. This dismissal is not an approval.
1 participant
Purpose
Make Keyverse's cross-cutting product, technical, data, security, operating, and decision memory canonical and machine-checkable instead of reconstructing it from README,
ARCHITECTURE.md, feature specs, doctoring pages, operations pages, and PR bodies.The branch was created from and revalidated against exact protected
mainc8968ec1e68fab16d0ad8216fb5c8fd0b385e95f.Added
DOCUMENTATION.mddocumentation map and maturity vocabularydocs/PRD.mddocs/TRD.mddocs/UML.mddocs/ERD.mdfor Keyverse-owned state while preserving Keycloak schema ownershipdocs/THREAT_MODEL.mddocs/TEST_STRATEGY.mddocs/OPERABILITY.mddocs/TRACEABILITY.mdtests/test_documentation_contract.pyExisting records preserved
ARCHITECTURE.md,docs/topology.md, federation/RP onboarding, merge/unification flow,docs/operations/,docs/doctoring/, anddocs/papers/remain authoritative for their existing slices. This PR consolidates discoverability and cross-cutting invariants rather than duplicating those documents.Truth boundary
The docs describe protected-main passwordless Keycloak hub, account unification, SCIM, SAML/OIDC and LDAP desired-state boundaries, secret-free RP reconciliation, operation locking, deployment-controller ownership, and current quality gates.
PR #72's closed OIDC RP mapper profile and PR #74's hourly GitHub API remediation remain explicitly active-PR, not protected-main/released behavior. Controlled external login/bind/downstream authorization acceptance is kept separate from side-effect-free preflight readiness.
Verification contract
The documentation contract runs in the existing pytest suite and prevents disappearance of canonical records, false ownership of Keycloak internal persistence, missing ADRs, or promotion of active PR work into mainline claims. Exact current-head CI, 100% production statement/branch/docstring gates, package/realm/deployment validation, CodeQL, Semgrep, Security Scan, independent review, and branch protection must pass before merge.
Summary by CodeRabbit
문서화
테스트