promote: IAM foundation reconciliation to main - #40
Conversation
Merge the verified IAM foundation slice into dev. CodeRabbit is intentionally reserved for the subsequent dev-to-main promotion PR.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (8)
🚧 Files skipped from review as they are similar to previous changes (6)
📝 WalkthroughWalkthroughThe PR adds tenant-scoped IAM hierarchy, membership, and bootstrap capabilities. It adds domain validation, in-memory and Prisma persistence, authenticated API routes, module wiring, OpenAPI contracts, tests, migrations, and foundation evidence updates. ChangesIAM foundation implementation
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant IamHierarchyController
participant IamHierarchyService
participant PrismaIamHierarchyRepositoryAdapter
Client->>IamHierarchyController: send authenticated hierarchy request
IamHierarchyController->>IamHierarchyService: pass tenant context and validated DTO
IamHierarchyService->>PrismaIamHierarchyRepositoryAdapter: validate parents and save identity in transaction
PrismaIamHierarchyRepositoryAdapter-->>IamHierarchyService: return persisted identity or error
IamHierarchyService-->>IamHierarchyController: return application result
IamHierarchyController-->>Client: return API response
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/plans/execution-orchestration.json (1)
162-174: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winKeep
FND-007in the B01 task list.
taskState.FND-007isverified, butB01.taskIdsmoves directly fromFND-006toIAM-001. The ledger omits the handoff task. Any consumer that traversestaskIdscan skip its evidence. AddFND-007afterFND-006, and assert that membership in the orchestration test.Proposed fix
"taskIds": [ "FND-006", + "FND-007", "IAM-001",🤖 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/plans/execution-orchestration.json` around lines 162 - 174, Update the B01 taskIds list in the execution orchestration plan to insert FND-007 immediately after FND-006, preserving the existing order of subsequent tasks. Extend the orchestration test to assert that B01 includes FND-007 at that position.
🧹 Nitpick comments (12)
services/api/src/features/iam/api/bootstrap.dto.ts (1)
84-87: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winDeclare the
apiVersionliteral in the schema.The TypeScript type is the literal
'v1', but@ApiProperty()emits a plain string. The generated contract atservices/api/openapi/v1.jsonline 8671 shows{ "type": "string" }. Add the enum so the published contract matches the type.♻️ Proposed fix
export class BootstrapPlatformDto { - `@ApiProperty`() + `@ApiProperty`({ enum: ['v1'] }) apiVersion!: 'v1'; }Regenerate
services/api/openapi/v1.jsonafter the change.🤖 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 `@services/api/src/features/iam/api/bootstrap.dto.ts` around lines 84 - 87, Update the apiVersion property in BootstrapPlatformDto to declare the allowed literal value in its ApiProperty schema using an enum containing “v1”. Regenerate the OpenAPI contract so the published schema reflects this single permitted value.packages/domain/test/identity-hierarchy-v1.test.mjs (1)
55-90: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a case for
INVALID_STATE.The test name lists "states", but the body asserts only identifier, text, epoch, and kind rejections. Add one case that passes an unsupported
statusvalue so the newactiveOrArchivedguard and the organization status check are covered.♻️ Proposed additional case
{ accepted: false, code: 'INVALID_KIND' }, ); + assert.deepEqual( + createWorkspaceIdentityV1({ + id: ids.workspace, + organizationId: ids.organization, + name: 'Operations', + status: 'DELETED', + createdAt, + }), + { accepted: false, code: 'INVALID_STATE' }, + ); });🤖 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 `@packages/domain/test/identity-hierarchy-v1.test.mjs` around lines 55 - 90, Add an assertion to the existing hierarchy-constructor rejection test covering INVALID_STATE by passing an unsupported status value to the relevant organization identity constructor, exercising the activeOrArchived guard and organization status validation while preserving the expected { accepted: false, code: 'INVALID_STATE' } result.services/api/src/features/iam/application/hierarchy.service.ts (1)
175-181: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider a dedicated name check instead of a placeholder-id pre-check.
This pre-check passes
organizationId.valueas the workspaceidonly to reach the name validation. The intent is not visible from the code, and any future change tocreateWorkspaceIdentityV1identifier rules changes the meaning of this check.createProjectuses the same placeholder pattern at lines 254-262. Consider validating the name with the sharedboundedTextrule, then constructing the identity once inside the transaction.🤖 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 `@services/api/src/features/iam/application/hierarchy.service.ts` around lines 175 - 181, Replace the placeholder-id pre-check in the workspace creation flow with direct name validation using the shared boundedText rule, then construct the workspace identity once inside the transaction. Apply the same change to createProject, removing the duplicate createWorkspaceIdentityV1 validation while preserving the existing rejection behavior and identifier validation.services/api/test/features/iam/hierarchy-controller.test.ts (1)
111-116: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert that the resolved context is the first argument.
The test name states that the controller forwards the authenticated context, but no assertion inspects argument index 0. Add a check so a future change that drops or replaces the context fails this test.
♻️ Proposed additional assertion
assert.equal(calls.length, 7); + for (const call of calls) assert.equal(call[0], context); assert.equal(calls[2]?.[1], ids.organization);🤖 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 `@services/api/test/features/iam/hierarchy-controller.test.ts` around lines 111 - 116, Update the assertions in the hierarchy controller test to verify that the resolved authenticated context is passed as argument index 0 in the relevant recorded call(s). Use the existing expected context value or symbol, while preserving the current assertions for organization, workspace, and other arguments.services/api/src/features/iam/api/membership.dto.ts (2)
35-37: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDeclare each allowed-value list once in the IAM DTOs. Both DTO files repeat an allowed-value list three times: in the
@ApiPropertymetadata, in the@IsInvalidator, and in the TypeScript union. A future value requires three edits per field, and one missed edit makes the OpenAPI schema and the validator disagree.
services/api/src/features/iam/api/membership.dto.ts#L35-L37: declare the role list as oneas constarray and derive@ApiProperty({ enum: ... }),@IsIn(...), and theroleIdtype from it.services/api/src/features/iam/api/hierarchy.dto.ts#L12-L22: declare the project kind list as oneas constarray and derive@ApiProperty({ enum: ... }),@IsIn(...), and thekindtype from it.Prefer the shared constants from
@databreeze/domainif that package already exports the roles and the project kinds.🤖 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 `@services/api/src/features/iam/api/membership.dto.ts` around lines 35 - 37, Declare the allowed values once and reuse them across IAM DTO metadata, validation, and TypeScript types. In services/api/src/features/iam/api/membership.dto.ts:35-37, update the roleId definition to use a shared or local as-const roles array for ApiProperty, IsIn, and the roleId type. In services/api/src/features/iam/api/hierarchy.dto.ts:12-22, apply the same pattern to the project kind list and kind type; prefer existing exports from `@databreeze/domain` when available.
5-23: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAdd cross-field rules for the scope discriminator.
The DTO validates
scopeType,workspaceId, andprojectIdindependently. A body withscopeType: 'project'and noworkspaceIdand noprojectIdpasses validation. A body withscopeType: 'organization'and aprojectIdalso passes. Make the identifier requirements conditional onscopeTypeso the API rejects an inconsistent scope at the request boundary instead of deeper in the domain layer.♻️ Proposed conditional validation
+import { ValidateIf } from 'class-validator'; + export class MembershipScopeDto { `@ApiProperty`({ enum: ['organization', 'workspace', 'project'] }) `@IsIn`(['organization', 'workspace', 'project']) scopeType!: 'organization' | 'workspace' | 'project'; `@ApiProperty`({ format: 'uuid' }) `@IsUUID`() organizationId!: string; `@ApiPropertyOptional`({ format: 'uuid' }) - `@IsOptional`() + `@ValidateIf`((dto: MembershipScopeDto) => dto.scopeType !== 'organization') `@IsUUID`() workspaceId?: string; `@ApiPropertyOptional`({ format: 'uuid' }) - `@IsOptional`() + `@ValidateIf`((dto: MembershipScopeDto) => dto.scopeType === 'project') `@IsUUID`() projectId?: string; }Also reject identifiers that the scope does not use, for example a
projectIdon an organization scope.🤖 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 `@services/api/src/features/iam/api/membership.dto.ts` around lines 5 - 23, Update MembershipScopeDto with cross-field conditional validation tied to scopeType: require workspaceId for workspace scopes, require both workspaceId and projectId for project scopes, and reject identifiers not applicable to organization or workspace scopes. Preserve UUID validation for any supplied identifiers and ensure inconsistent scope combinations fail at the request boundary.services/api/test/features/iam/prisma-iam-repository.test.ts (1)
274-295: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the status and revision changes as well.
The test proves that
startsAtandexpiresAtbecomenull. It does not prove that the transition itself applied. Add assertions forstatusandrevisionso a payload regression on those fields fails the test.♻️ Proposed extra assertions
assert.equal(memberships.get(id('25'))?.startsAt, null); assert.equal(memberships.get(id('25'))?.expiresAt, null); + assert.equal(memberships.get(id('25'))?.status, 'ACTIVE'); + assert.equal(memberships.get(id('25'))?.revision, 2); });🤖 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 `@services/api/test/features/iam/prisma-iam-repository.test.ts` around lines 274 - 295, Extend the test around PrismaIamRepositoryAdapter.saveMembership to also assert that the persisted membership’s status is ACTIVE and revision is 2, alongside the existing cleared startsAt and expiresAt assertions. Use the membership retrieved from memberships.get(id('25')) so payload regressions for the transition fields fail the test.services/api/src/features/iam/adapter/in-memory-iam-hierarchy-repository.adapter.ts (1)
184-222: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReplace
JSON.stringifyequality with field-wise comparison.
JSON.stringifyequality depends on property insertion order. The Prisma adapter compares owned fields withownedFieldsMatch, which is order independent. A record that is semantically identical but built with a different key order producesIAM_HIERARCHY_CONFLICThere and succeeds in Prisma. Use an explicit field comparison to keep both adapters equivalent.♻️ Proposed shared comparison helper
function fieldsMatch<TValue extends object>(existing: TValue, expected: TValue): boolean { return Object.keys(expected).every( (key) => (existing as Record<string, unknown>)[key] === (expected as Record<string, unknown>)[key], ); }- if (existing && JSON.stringify(existing) !== JSON.stringify(value)) + if (existing && !fieldsMatch(existing, value)) throw new Error('IAM_HIERARCHY_CONFLICT');🤖 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 `@services/api/src/features/iam/adapter/in-memory-iam-hierarchy-repository.adapter.ts` around lines 184 - 222, Replace the JSON.stringify comparisons in saveOrganization, saveWorkspace, and saveProject with order-independent field-wise equality using a shared fieldsMatch-style helper. Preserve IAM_HIERARCHY_CONFLICT for differing owned field values while allowing semantically identical records with different property insertion order.services/api/src/features/iam/adapter/prisma-iam-hierarchy-repository.adapter.ts (1)
346-359: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winFilter project rows by tenant scope before you map them.
listProjectsqueries byworkspaceIdonly, then maps every row to a domain identity, and filters by scope last. If one row that belongs to another tenant is malformed,projectFromRowWithDiagnosticsthrowsIAM_PERSISTED_PROJECT_INVALIDand the caller request fails, although that row is not visible to the caller. Add the organization predicate to the query and apply the scope filter before mapping.listWorkspacesat lines 329-331 has the same map-then-filter order; its query already restrictsorganizationId, so the exposure is smaller there.♻️ Proposed change for `listProjects`
public async listProjects( context: IamTenantContextV1, workspaceId: StableIdentifierV1, ): Promise<readonly ProjectIdentityV1[]> { + if (context.tenantScope.scopeType === 'user') return []; const rows = await this.client.projectIdentity.findMany({ - where: { workspaceId }, + where: { workspaceId, organizationId: context.tenantScope.organizationId }, orderBy: { id: 'asc' }, }); return rows - .map((row) => projectFromRowWithDiagnostics(row, this.diagnostics)) - .filter((project) => - projectVisible(context, project.organizationId, project.workspaceId, project.id), - ); + .filter((row) => + projectVisible( + context, + row.organizationId as StableIdentifierV1, + row.workspaceId as StableIdentifierV1, + row.id as StableIdentifierV1, + ), + ) + .map((row) => projectFromRowWithDiagnostics(row, this.diagnostics)); }Adjust the scope guard to the actual
TenantScopeV1variants in@databreeze/domain/tenant-scope/v1.🤖 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 `@services/api/src/features/iam/adapter/prisma-iam-hierarchy-repository.adapter.ts` around lines 346 - 359, Update listProjects to restrict findMany by both workspaceId and the caller’s organization scope, then apply the scope guard to raw row identifiers before projectFromRowWithDiagnostics. Adjust the guard to use the actual TenantScopeV1 variants from `@databreeze/domain/tenant-scope/v1`. Apply the same filter-before-mapping order in listWorkspaces while preserving its existing organizationId query restriction.services/api/test/features/iam/hierarchy-repository.test.ts (1)
159-177: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the workspace rollback too.
The test name states that all staged writes roll back. The test asserts only the organization. Add an assertion for the workspace so the test matches its name.
♻️ Proposed extra assertion
assert.equal( await repository.findOrganization(transactionContext, stable(ids.organization)), undefined, ); + assert.equal(await repository.findWorkspace(transactionContext, stable(ids.workspace)), undefined); });🤖 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 `@services/api/test/features/iam/hierarchy-repository.test.ts` around lines 159 - 177, Extend the rollback test after the existing findOrganization assertion to also call findWorkspace for ids.workspace and assert that it returns undefined, confirming all staged writes are rolled back.services/api/src/features/iam/api/membership.controller.ts (1)
46-107: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftEvery membership endpoint returns HTTP 200, including denials and conflicts.
The application results carry
SCOPE_DENIED,NOT_FOUND,CONFLICT,EXPIRED, andUNAVAILABLE. All five reach the client inside a 200 response. Two effects follow:
- Clients and gateways cannot distinguish success from denial without parsing the body.
- Authorization failures and conflicts do not appear in HTTP-status metrics or alerts.
Map the result codes to HTTP status codes in one shared interceptor or exception filter, and keep the envelope in the body.
🤖 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 `@services/api/src/features/iam/api/membership.controller.ts` around lines 46 - 107, Add one shared interceptor or exception filter for the membership endpoints that maps result codes SCOPE_DENIED, NOT_FOUND, CONFLICT, EXPIRED, and UNAVAILABLE to their corresponding HTTP statuses while preserving the existing response envelope. Apply it to the MembershipController methods list, invite, transition, accept, and transferOwnership, and remove or bypass fixed 200 responses where necessary so mapped statuses reach clients and HTTP metrics.services/api/test/features/iam/bootstrap-controller.test.ts (1)
60-86: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd assertions for the mapped bootstrap response fields.
The test sets
mfaRequired: trueand aworkspacescope. It asserts onlyacceptedanduser.id. The mappeduser.mfaState,session,organizations, andrecentScopesfields stay uncovered. Assert those fields so a change in the mapping inservices/api/src/features/iam/api/bootstrap.controller.tsfails the test.🤖 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 `@services/api/test/features/iam/bootstrap-controller.test.ts` around lines 60 - 86, Extend the bootstrap controller test around IamBootstrapController.bootstrap to assert the mapped response fields user.mfaState, session, organizations, and recentScopes, using expectations derived from the mfaRequired and workspace-scoped context plus bootstrap fixture. Keep the existing accepted, actor propagation, and user.id assertions unchanged.
🤖 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/operations/foundation-handoff-2026-08-03.md`:
- Around line 29-31: Update the Android/Kotlin unit-suite command in the
foundation handoff document to provide separate valid PowerShell and cmd.exe
forms, using each shell’s environment-variable syntax and command invocation
conventions. Replace the current mixed-shell command while preserving its Gradle
tasks and flags.
In `@services/api/src/features/iam/api/bootstrap.controller.ts`:
- Around line 77-83: Update the session construction to preserve the project
identifier when context.tenantScope.scopeType is project, while retaining
workspaceId for workspace scopes and the existing organization behavior. Ensure
the frozen session returned by the bootstrap controller includes projectId so
clients can restore the active project scope.
In `@services/api/src/features/iam/api/hierarchy.controller.ts`:
- Around line 32-42: Update the three hierarchy read handlers, including
getOrganization, to map rejected IamHierarchyApplicationResultV1 values with
code NOT_FOUND to an HTTP 404 response instead of returning the envelope with
HTTP 200. Preserve successful results and existing application-result behavior,
and keep the declared OpenAPI 404 responses aligned with the implemented status
mapping.
In `@services/api/src/features/iam/api/membership.dto.ts`:
- Around line 47-49: Update the ACTIVE transition branch in
IamMembershipService.transition to require the current membership status to be
INVITED only when processing the invited principal’s accept flow, rejecting
administrator attempts to activate an invitation directly. Preserve existing
authorization and valid transition behavior for other statuses.
In `@services/api/src/features/iam/application/membership.service.ts`:
- Around line 250-260: Update the status validation in transition around the
existing current.status guard so transition operations are allowed only when the
membership is already ACTIVE, regardless of statusInput. Ensure INVITED
memberships continue exclusively through accept, preserving its actor-identity
and invitation-expiry checks, and prevent REMOVED memberships from being
reactivated.
- Around line 191-219: Update the membership persistence flow around invite and
saveMembership to enforce uniqueness for the complete principal-and-scope
identity, including nullable scope components, using a database constraint or
equivalent migration rather than relying on the generated id. Handle the
resulting uniqueness conflict in the service by returning the established
duplicate-membership rejection instead of propagating an error.
---
Outside diff comments:
In `@docs/plans/execution-orchestration.json`:
- Around line 162-174: Update the B01 taskIds list in the execution
orchestration plan to insert FND-007 immediately after FND-006, preserving the
existing order of subsequent tasks. Extend the orchestration test to assert that
B01 includes FND-007 at that position.
---
Nitpick comments:
In `@packages/domain/test/identity-hierarchy-v1.test.mjs`:
- Around line 55-90: Add an assertion to the existing hierarchy-constructor
rejection test covering INVALID_STATE by passing an unsupported status value to
the relevant organization identity constructor, exercising the activeOrArchived
guard and organization status validation while preserving the expected {
accepted: false, code: 'INVALID_STATE' } result.
In
`@services/api/src/features/iam/adapter/in-memory-iam-hierarchy-repository.adapter.ts`:
- Around line 184-222: Replace the JSON.stringify comparisons in
saveOrganization, saveWorkspace, and saveProject with order-independent
field-wise equality using a shared fieldsMatch-style helper. Preserve
IAM_HIERARCHY_CONFLICT for differing owned field values while allowing
semantically identical records with different property insertion order.
In
`@services/api/src/features/iam/adapter/prisma-iam-hierarchy-repository.adapter.ts`:
- Around line 346-359: Update listProjects to restrict findMany by both
workspaceId and the caller’s organization scope, then apply the scope guard to
raw row identifiers before projectFromRowWithDiagnostics. Adjust the guard to
use the actual TenantScopeV1 variants from `@databreeze/domain/tenant-scope/v1`.
Apply the same filter-before-mapping order in listWorkspaces while preserving
its existing organizationId query restriction.
In `@services/api/src/features/iam/api/bootstrap.dto.ts`:
- Around line 84-87: Update the apiVersion property in BootstrapPlatformDto to
declare the allowed literal value in its ApiProperty schema using an enum
containing “v1”. Regenerate the OpenAPI contract so the published schema
reflects this single permitted value.
In `@services/api/src/features/iam/api/membership.controller.ts`:
- Around line 46-107: Add one shared interceptor or exception filter for the
membership endpoints that maps result codes SCOPE_DENIED, NOT_FOUND, CONFLICT,
EXPIRED, and UNAVAILABLE to their corresponding HTTP statuses while preserving
the existing response envelope. Apply it to the MembershipController methods
list, invite, transition, accept, and transferOwnership, and remove or bypass
fixed 200 responses where necessary so mapped statuses reach clients and HTTP
metrics.
In `@services/api/src/features/iam/api/membership.dto.ts`:
- Around line 35-37: Declare the allowed values once and reuse them across IAM
DTO metadata, validation, and TypeScript types. In
services/api/src/features/iam/api/membership.dto.ts:35-37, update the roleId
definition to use a shared or local as-const roles array for ApiProperty, IsIn,
and the roleId type. In
services/api/src/features/iam/api/hierarchy.dto.ts:12-22, apply the same pattern
to the project kind list and kind type; prefer existing exports from
`@databreeze/domain` when available.
- Around line 5-23: Update MembershipScopeDto with cross-field conditional
validation tied to scopeType: require workspaceId for workspace scopes, require
both workspaceId and projectId for project scopes, and reject identifiers not
applicable to organization or workspace scopes. Preserve UUID validation for any
supplied identifiers and ensure inconsistent scope combinations fail at the
request boundary.
In `@services/api/src/features/iam/application/hierarchy.service.ts`:
- Around line 175-181: Replace the placeholder-id pre-check in the workspace
creation flow with direct name validation using the shared boundedText rule,
then construct the workspace identity once inside the transaction. Apply the
same change to createProject, removing the duplicate createWorkspaceIdentityV1
validation while preserving the existing rejection behavior and identifier
validation.
In `@services/api/test/features/iam/bootstrap-controller.test.ts`:
- Around line 60-86: Extend the bootstrap controller test around
IamBootstrapController.bootstrap to assert the mapped response fields
user.mfaState, session, organizations, and recentScopes, using expectations
derived from the mfaRequired and workspace-scoped context plus bootstrap
fixture. Keep the existing accepted, actor propagation, and user.id assertions
unchanged.
In `@services/api/test/features/iam/hierarchy-controller.test.ts`:
- Around line 111-116: Update the assertions in the hierarchy controller test to
verify that the resolved authenticated context is passed as argument index 0 in
the relevant recorded call(s). Use the existing expected context value or
symbol, while preserving the current assertions for organization, workspace, and
other arguments.
In `@services/api/test/features/iam/hierarchy-repository.test.ts`:
- Around line 159-177: Extend the rollback test after the existing
findOrganization assertion to also call findWorkspace for ids.workspace and
assert that it returns undefined, confirming all staged writes are rolled back.
In `@services/api/test/features/iam/prisma-iam-repository.test.ts`:
- Around line 274-295: Extend the test around
PrismaIamRepositoryAdapter.saveMembership to also assert that the persisted
membership’s status is ACTIVE and revision is 2, alongside the existing cleared
startsAt and expiresAt assertions. Use the membership retrieved from
memberships.get(id('25')) so payload regressions for the transition fields fail
the test.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 2397e2ad-4258-4b05-9516-0a8b6fb4f39c
📒 Files selected for processing (36)
docs/operations/foundation-handoff-2026-08-03.mddocs/operations/foundation-telemetry-diagnostics-2026-08-03.mddocs/operations/iam-foundation-checkpoint-2026-08-03.mddocs/plans/execution-orchestration.jsonpackages/domain/src/identity/v1.tspackages/domain/test/identity-hierarchy-v1.test.mjsservices/api/openapi/v1.jsonservices/api/src/features/iam/adapter/in-memory-iam-hierarchy-repository.adapter.tsservices/api/src/features/iam/adapter/prisma-iam-hierarchy-repository.adapter.tsservices/api/src/features/iam/adapter/prisma-iam-repository.adapter.tsservices/api/src/features/iam/api/bootstrap.controller.tsservices/api/src/features/iam/api/bootstrap.dto.tsservices/api/src/features/iam/api/hierarchy.controller.tsservices/api/src/features/iam/api/hierarchy.dto.tsservices/api/src/features/iam/api/membership.controller.tsservices/api/src/features/iam/api/membership.dto.tsservices/api/src/features/iam/application/hierarchy-repository.port.tsservices/api/src/features/iam/application/hierarchy.service.tsservices/api/src/features/iam/application/iam-repository.port.tsservices/api/src/features/iam/application/identity-bootstrap.service.tsservices/api/src/features/iam/application/membership.service.tsservices/api/src/features/iam/iam.module.tsservices/api/test/features/foundation-module-composition.test.tsservices/api/test/features/iam/bootstrap-controller.test.tsservices/api/test/features/iam/hierarchy-controller.test.tsservices/api/test/features/iam/hierarchy-repository.test.tsservices/api/test/features/iam/hierarchy-service.test.tsservices/api/test/features/iam/identity-bootstrap.test.tsservices/api/test/features/iam/membership-controller.test.tsservices/api/test/features/iam/membership-service.test.tsservices/api/test/features/iam/prisma-iam-hierarchy-repository.test.tsservices/api/test/features/iam/prisma-iam-repository.test.tsservices/api/test/openapi.test.tstools/repo-cli/test/execution-orchestration.test.mjstools/repo-cli/test/foundation-handoff.test.mjstools/repo-cli/test/foundation-telemetry-reconciliation.test.mjs
| @ApiProperty({ enum: ['ACTIVE', 'SUSPENDED', 'REMOVED'] }) | ||
| @IsIn(['ACTIVE', 'SUSPENDED', 'REMOVED']) | ||
| status!: 'ACTIVE' | 'SUSPENDED' | 'REMOVED'; |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect the membership service transition and accept paths.
fd -t f 'membership.service.ts' services --exec ast-grep outline {} --items all
fd -t f 'membership.service.ts' services --exec rg -n -C 12 'INVITED'Repository: DatabreezeService/databreeze-platform
Length of output: 3650
🏁 Script executed:
#!/bin/bash
set -e
service=$(fd -t f 'membership.service.ts' services | head -n 1)
controller=$(fd -t f 'membership.controller.ts' services | head -n 1)
printf '%s\n' "--- service: $service ---"
cat -n "$service" | sed -n '250,430p'
printf '%s\n' "--- controller candidates ---"
fd -t f -i 'membership' services | sort
printf '%s\n' "--- transition and accept references ---"
rg -n -C 10 'transition|accept' services/api/src/features/iamRepository: DatabreezeService/databreeze-platform
Length of output: 50394
🏁 Script executed:
#!/bin/bash
set -e
service=$(fd -t f 'membership.service.ts' services | head -n 1)
controller=$(fd -t f -i 'membership.controller.ts' services | head -n 1)
printf '%s\n' "--- service: $service ---"
cat -n "$service" | sed -n '250,430p'
printf '%s\n' "--- controller: $controller ---"
cat -n "$controller" | sed -n '1,260p'
printf '%s\n' "--- transition and accept references ---"
rg -n -C 10 'transition|accept' services/api/src/features/iamRepository: DatabreezeService/databreeze-platform
Length of output: 50394
🏁 Script executed:
#!/bin/bash
set -e
service=services/api/src/features/iam/application/membership.service.ts
controller=services/api/src/features/iam/api/membership.controller.ts
printf '%s\n' '--- service method declarations ---'
rg -n '(^|[[:space:]])(public )?async (transition|accept)|transitionMembership|transition\(' "$service"
printf '%s\n' '--- service transition region ---'
start=$(rg -n 'async transition|public transition' "$service" | head -n 1 | cut -d: -f1)
end=$(rg -n 'async accept|public accept' "$service" | head -n 1 | cut -d: -f1)
if [ -n "$start" ] && [ -n "$end" ]; then sed -n "$((start-8)),$((end+55))p" "$service"; fi
printf '%s\n' '--- controller membership routes ---'
rg -n -C 18 '@(Post|Patch|Put|Delete)|membershipId|transition|accept' "$controller"Repository: DatabreezeService/databreeze-platform
Length of output: 11631
🏁 Script executed:
#!/bin/bash
set -e
service=services/api/src/features/iam/application/membership.service.ts
printf '%s\n' '--- authorization implementation ---'
rg -n -C 24 'public async authorize|async authorize|authorize\(' "$service" | head -n 120
printf '%s\n' '--- membership transition tests and status expectations ---'
rg -n -C 8 'INVITED.*ACTIVE|ACTIVE.*INVITED|transition\(.*ACTIVE|/transition|accept' services/api test tests 2>/dev/null | head -n 240 || true
printf '%s\n' '--- behavioral probe for the transition guard ---'
python3 - <<'PY'
statuses = ('INVITED', 'ACTIVE', 'SUSPENDED', 'REMOVED')
for current in statuses:
# This is the exact state guard in IamMembershipService.transition.
allowed = not ('ACTIVE' != 'ACTIVE' and current != 'ACTIVE')
print(f'current={current} requested=ACTIVE transition_guard_allows={allowed}')
PYRepository: DatabreezeService/databreeze-platform
Length of output: 26159
🏁 Script executed:
#!/bin/bash
set -e
service=services/api/src/features/iam/application/membership.service.ts
printf '%s\n' '--- authorization implementation ---'
rg -n -C 24 'public async authorize|async authorize|authorize\(' "$service" | head -n 120
printf '%s\n' '--- membership transition tests and status expectations ---'
rg -n -C 8 'INVITED.*ACTIVE|ACTIVE.*INVITED|transition\(.*ACTIVE|/transition|accept' services/api test tests 2>/dev/null | head -n 240 || true
printf '%s\n' '--- behavioral probe for the transition guard ---'
python3 - <<'PY'
statuses = ('INVITED', 'ACTIVE', 'SUSPENDED', 'REMOVED')
for current in statuses:
# Exact state guard in IamMembershipService.transition.
allowed = not ('ACTIVE' != 'ACTIVE' and current != 'ACTIVE')
print(f'current={current} requested=ACTIVE transition_guard_allows={allowed}')
PYRepository: DatabreezeService/databreeze-platform
Length of output: 24918
Reject INVITED → ACTIVE in IamMembershipService.transition. The ACTIVE branch does not check current.status, so an authorized administrator can activate an invitation without the invited principal using accept.
🤖 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 `@services/api/src/features/iam/api/membership.dto.ts` around lines 47 - 49,
Update the ACTIVE transition branch in IamMembershipService.transition to
require the current membership status to be INVITED only when processing the
invited principal’s accept flow, rejecting administrator attempts to activate an
invitation directly. Preserve existing authorization and valid transition
behavior for other statuses.
| const candidate = validateMembershipV1({ | ||
| id: candidateId, | ||
| principalType: 'USER', | ||
| principalId: principalId.value, | ||
| scope: scope.value, | ||
| roleId: input.roleId, | ||
| status: 'INVITED', | ||
| startsAt: startedAt, | ||
| expiresAt, | ||
| revision: 1, | ||
| }); | ||
| if (!candidate.accepted) { | ||
| if (candidate.code === 'INVALID_IDENTIFIER') return rejected('INVALID_IDENTIFIER'); | ||
| if (candidate.code === 'INVALID_SCOPE') return rejected('INVALID_SCOPE'); | ||
| if (candidate.code === 'INVALID_ROLE') return rejected('INVALID_ROLE'); | ||
| return rejected('INVALID_STATE'); | ||
| } | ||
| const record: IamMembershipRecordV1 = Object.freeze({ | ||
| id: candidate.value.id, | ||
| principalId: candidate.value.principalId, | ||
| scope: candidate.value.scope, | ||
| roleId: candidate.value.roleId, | ||
| status: candidate.value.status, | ||
| ...(candidate.value.startsAt === undefined ? {} : { startsAt: candidate.value.startsAt }), | ||
| ...(candidate.value.expiresAt === undefined ? {} : { expiresAt: candidate.value.expiresAt }), | ||
| revision: candidate.value.revision, | ||
| }); | ||
| try { | ||
| await this.repository.saveMembership(context, record); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find the Prisma schema and inspect membership uniqueness constraints.
fd -H -t f 'schema.prisma' --exec sh -c 'echo "== $1"; cat -n "$1"' _ {} \; | rg -n -C 12 -i 'membershipIdentity|model Membership'Repository: DatabreezeService/databreeze-platform
Length of output: 175
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '== candidate files =='
git ls-files | rg -i '(^|/)(schema\.prisma|membership.*\.(ts|prisma)|.*membership.*repository.*\.(ts|prisma))$' || true
printf '%s\n' '== membership schema declarations =='
rg -n -i -C 15 'model[[:space:]]+Membership|membershipIdentity|@@unique|`@unique`' . --glob '*.prisma' || true
printf '%s\n' '== invite and saveMembership definitions/usages =='
rg -n -C 12 'invite[[:space:]]*\(|saveMembership|validateMembershipV1|candidateId|find.*Membership|membership.*principal' services/api/src --glob '*.{ts,tsx}' || trueRepository: DatabreezeService/databreeze-platform
Length of output: 50394
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '== MembershipIdentity model =='
sed -n '68,94p' services/api/prisma/schema/iam.prisma
printf '%s\n' '== saveMembership symbols =='
rg -n -C 16 'saveMembership|MembershipRepository|membership_identity|membershipIdentity|membership_identities' services/api/src services/api/prisma services/api/test --glob '*.{ts,tsx,sql,prisma}' || true
printf '%s\n' '== invite implementation =='
sed -n '130,235p' services/api/src/features/iam/application/membership.service.ts
printf '%s\n' '== related tests =='
rg -n -C 10 'invite|duplicate|same principal|same scope|already.*membership|MEMBERSHIP' services/api/test/features/iam/membership-service.test.ts || trueRepository: DatabreezeService/databreeze-platform
Length of output: 50394
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '== files defining saveMembership =='
rg -l 'saveMembership' services/api/src --glob '*.ts' | sort
printf '%s\n' '== membership service invite section =='
rg -n -C 8 'async invite|invite\(' services/api/src/features/iam/application/membership.service.ts
sed -n '1,80p' services/api/src/features/iam/application/membership.service.ts
sed -n '180,230p' services/api/src/features/iam/application/membership.service.ts
printf '%s\n' '== adapter implementation =='
for file in $(rg -l 'saveMembership' services/api/src --glob '*.ts'); do
echo "== $file =="
rg -n -C 18 'saveMembership' "$file"
done
printf '%s\n' '== invitation-specific tests =='
rg -n -C 6 'invite\(' services/api/test/features/iam/membership-service.test.tsRepository: DatabreezeService/databreeze-platform
Length of output: 22855
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
from pathlib import Path
import re
schema = Path("services/api/prisma/schema/iam.prisma").read_text()
model = re.search(r"model\s+MembershipIdentity\s*\{(.*?)\n\}", schema, re.S)
assert model, "MembershipIdentity model not found"
body = model.group(1)
uniques = re.findall(r"(?m)^\s*(?:@@unique|@@id)\s*\(([^)]*)\)", body)
single_uniques = re.findall(r"(?m)^\s*\w+\s+\S+\s+[^\\n]*`@unique`", body)
print("MembershipIdentity unique declarations:", uniques + single_uniques)
print("Has principal/scope uniqueness:", any("principalId" in u and "organizationId" in u for u in uniques))
service = Path("services/api/src/features/iam/application/membership.service.ts").read_text()
invite = service[service.index("public async invite("):service.index(" public async transition(")]
print("invite calls findMembership:", bool(re.search(r"\bfindMembership\s*\(", invite)))
print("invite calls saveMembership:", bool(re.search(r"\bsaveMembership\s*\(", invite)))
print("invite generates id:", bool(re.search(r"\bidGenerator\s*\(\)", invite)))
adapter = Path("services/api/src/features/iam/adapter/prisma-iam-repository.adapter.ts").read_text()
save = adapter[adapter.index("public async saveMembership("):adapter.index("export class PrismaIamRepositoryAdapter")]
print("saveMembership looks up by id:", bool(re.search(r"findFirst\s*\(\s*\{\s*where:\s*\{\s*id:\s*membership\.id", save, re.S)))
print("saveMembership creates when id is absent:", bool(re.search(r"if\s*\(!existingRow\).*?membershipIdentity\.create", save, re.S)))
PY
printf '%s\n' '== migration references to memberships =='
git ls-files | rg -i '(^|/)(migration[^/]*/|migrations?/|.*\.sql$)' | xargs -r rg -n -i -C 3 'memberships|membership_identity|principal_id' || trueRepository: DatabreezeService/databreeze-platform
Length of output: 3710
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '== authorization and invite flow =='
sed -n '105,205p' services/api/src/features/iam/application/membership.service.ts
printf '%s\n' '== Prisma membership save path =='
sed -n '180,260p' services/api/src/features/iam/adapter/prisma-iam-repository.adapter.ts
printf '%s\n' '== migration membership table and indexes =='
sed -n '68,90p' services/api/prisma/migrations/20260802000000_iam_identity_foundation/migration.sql
sed -n '172,186p' services/api/prisma/migrations/20260802000000_iam_identity_foundation/migration.sql
python3 - <<'PY'
from pathlib import Path
import re
service = Path("services/api/src/features/iam/application/membership.service.ts").read_text()
invite = service[service.index("public async invite("):service.index(" public async transition(")]
target_lookup = re.search(
r"(?:findMembership|listMemberships)\s*\([^)]*principalId|"
r"(?:findMembership|listMemberships)\s*\([^)]*input\.principalId",
invite,
re.S,
)
print("invite directly looks up target principal:", bool(target_lookup))
print("invite generates a fresh id before save:", bool(re.search(r"candidateId\s*=\s*this\.idGenerator\(\)", invite)))
migration = Path("services/api/prisma/migrations/20260802000000_iam_identity_foundation/migration.sql").read_text()
table = migration[migration.index('CREATE TABLE "iam"."memberships"'):migration.index('-- CreateTable', migration.index('CREATE TABLE "iam"."memberships"') + 1)]
print("membership table has only primary key constraint:", table.count("CONSTRAINT") == 1 and "memberships_pkey" in table)
print("membership migration has a principal/scope unique constraint:", bool(re.search(r"UNIQUE\s*\([^)]*principal_id[^)]*(organization_id|workspace_id|project_id)", migration, re.I | re.S)))
PYRepository: DatabreezeService/databreeze-platform
Length of output: 8855
Prevent duplicate memberships for one principal and scope. MembershipIdentity and its migration enforce uniqueness only on id. invite generates a new ID, and saveMembership checks only that ID before creating a row. Add a database-enforced constraint for the complete principal/scope identity, including nullable scope fields, and handle the conflict.
🤖 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 `@services/api/src/features/iam/application/membership.service.ts` around lines
191 - 219, Update the membership persistence flow around invite and
saveMembership to enforce uniqueness for the complete principal-and-scope
identity, including nullable scope components, using a database constraint or
equivalent migration rather than relying on the generated id. Handle the
resulting uniqueness conflict in the service by returning the established
duplicate-membership rejection instead of propagating an error.
Focused promotion-gate fixes for PR #40. All hosted checks passed; CodeRabbit intentionally skipped on the dev fix PR per policy.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
services/api/test/prisma-foundation.test.mjs (1)
525-537: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAssert
project_idnormalization.Line 537 verifies null normalization for
workspace_idonly. The unique key also requiresCOALESCE("project_id"::text, ''). Add this assertion so a future change cannot remove project-scope normalization while this test still passes.Proposed test update
assert.match(membershipUniquenessMigration, /COALESCE\("workspace_id"::text, ''\)/u); +assert.match(membershipUniquenessMigration, /COALESCE\("project_id"::text, ''\)/u);🤖 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 `@services/api/test/prisma-foundation.test.mjs` around lines 525 - 537, Extend the migration assertions in the membershipUniquenessMigration test to also require COALESCE("project_id"::text, ''). Keep the existing unique-index and workspace_id normalization assertions unchanged.
🤖 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 `@services/api/src/features/iam/api/membership.controller.ts`:
- Around line 72-92: Align applyMembershipOutcomeResponses with the actual
rejected membership response shape: either update the membership handlers used
by preserveMembershipStatus to emit ProblemDetails, or replace the documented
application/problem+json schema with the { accepted: false, code: ... }
rejection envelope for every listed error response. Ensure all membership error
paths use the same documented and emitted body contract.
In `@tools/repo-cli/src/check-execution-orchestration.mjs`:
- Around line 179-184: Validate and normalize batch.handoffTaskIds before
constructing the Set in the handoff-task validation flow, treating non-array
values as malformed and emitting a ledger shape diagnostic instead of throwing
or matching string substrings. Reuse the normalized array in the later
processing around the existing handoffTaskIds logic. Add a regression test
covering object and string values and asserting diagnostics are returned.
---
Nitpick comments:
In `@services/api/test/prisma-foundation.test.mjs`:
- Around line 525-537: Extend the migration assertions in the
membershipUniquenessMigration test to also require COALESCE("project_id"::text,
''). Keep the existing unique-index and workspace_id normalization assertions
unchanged.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: ce0a1c0d-b669-4551-bbd1-b9cad29de291
📒 Files selected for processing (29)
docs/operations/coderabbit-pr-40-disposition.mddocs/operations/foundation-handoff-2026-08-03.mddocs/plans/execution-orchestration.jsonpackages/domain/src/identity/v1.tspackages/domain/test/identity-hierarchy-v1.test.mjsservices/api/openapi/v1.jsonservices/api/prisma/migrations/20260803030000_iam_membership_scope_uniqueness/migration.sqlservices/api/prisma/schema/iam.prismaservices/api/src/features/iam/adapter/in-memory-iam-hierarchy-repository.adapter.tsservices/api/src/features/iam/adapter/in-memory-iam-repository.adapter.tsservices/api/src/features/iam/adapter/prisma-iam-hierarchy-repository.adapter.tsservices/api/src/features/iam/api/bootstrap.controller.tsservices/api/src/features/iam/api/bootstrap.dto.tsservices/api/src/features/iam/api/hierarchy.controller.tsservices/api/src/features/iam/api/hierarchy.dto.tsservices/api/src/features/iam/api/membership.controller.tsservices/api/src/features/iam/api/membership.dto.tsservices/api/src/features/iam/application/hierarchy.service.tsservices/api/src/features/iam/application/membership.service.tsservices/api/test/features/iam/bootstrap-controller.test.tsservices/api/test/features/iam/hierarchy-controller.test.tsservices/api/test/features/iam/hierarchy-repository.test.tsservices/api/test/features/iam/membership-controller.test.tsservices/api/test/features/iam/membership-dto.test.tsservices/api/test/features/iam/membership-service.test.tsservices/api/test/features/iam/prisma-iam-repository.test.tsservices/api/test/prisma-foundation.test.mjstools/repo-cli/src/check-execution-orchestration.mjstools/repo-cli/test/execution-orchestration.test.mjs
🚧 Files skipped from review as they are similar to previous changes (16)
- services/api/test/features/iam/prisma-iam-repository.test.ts
- services/api/test/features/iam/hierarchy-repository.test.ts
- services/api/test/features/iam/hierarchy-controller.test.ts
- docs/operations/foundation-handoff-2026-08-03.md
- services/api/src/features/iam/api/hierarchy.dto.ts
- services/api/src/features/iam/api/membership.dto.ts
- packages/domain/test/identity-hierarchy-v1.test.mjs
- docs/plans/execution-orchestration.json
- services/api/src/features/iam/application/membership.service.ts
- services/api/src/features/iam/adapter/in-memory-iam-hierarchy-repository.adapter.ts
- services/api/src/features/iam/api/bootstrap.dto.ts
- services/api/src/features/iam/adapter/prisma-iam-hierarchy-repository.adapter.ts
- services/api/test/features/iam/membership-controller.test.ts
- packages/domain/src/identity/v1.ts
- services/api/src/features/iam/application/hierarchy.service.ts
- services/api/openapi/v1.json
Focused promotion-gate fixes for PR #40. Hosted checks passed; CodeRabbit skipped on this dev PR per policy.
Promotion scope
Verification
Review protocol
This is the main promotion PR. Request exactly one @coderabbitai full review, wait for the result, validate every claim, fix valid findings in focused commits, document rejected findings, and do not request another review.
Summary by CodeRabbit