diff --git a/docs/operations/coderabbit-pr-40-disposition.md b/docs/operations/coderabbit-pr-40-disposition.md index 3ba7e275..e8e12883 100644 --- a/docs/operations/coderabbit-pr-40-disposition.md +++ b/docs/operations/coderabbit-pr-40-disposition.md @@ -1,13 +1,18 @@ # CodeRabbit disposition for promotion PR 40 Promotion PR [#40](https://github.com/DatabreezeService/databreeze-platform/pull/40) -received exactly one automatic full CodeRabbit review. No manual rerun or second -review was requested. +received one automatic full CodeRabbit review. After the focused fix PR merged +into `dev`, the CodeRabbit integration automatically ran one incremental review +on the new promotion head; no additional review was manually requested or +invoked, and no further run will be requested. - Review ID: `4845720374` - Run ID: `2397e2ad-4258-4b05-9516-0a8b6fb4f39c` - Submitted: `2026-08-03T15:20:07Z` - Reviewed range: `8a4c0af52ed872715103710e3c89ca832f999bd4..f1573921446e9f86313e0f58b926777aed9e1402` +- Automatic incremental review ID: `4846097863` +- Automatic incremental run ID: `ce0a1c0d-b669-4551-bbd1-b9cad29de291` +- Incremental range: `f1573921446e9f86313e0f58b926777aed9e1402..c80994cf4e2cf97be5a9137160417b8feb2b4eb7` ## Valid findings fixed @@ -24,6 +29,9 @@ fixed in focused commits on `fix/coderabbit-pr-40-reconciliation`: | Hierarchy reads and membership outcomes returned denial/not-found/conflict envelopes as HTTP 200. | Accepted. `0689d70` maps hierarchy `NOT_FOUND` to 404 and membership result codes to 400/403/404/409/410/503, with generated OpenAPI and regression tests. | | Windows Android test command mixed PowerShell and cmd.exe syntax. | Accepted. `de3ff3d` documents valid commands for both shells. | | Maintainability and boundary nitpicks (shared DTO constants, cross-field scope validation, identity state coverage, adapter equality/filtering, rollback assertions, and mapped bootstrap assertions). | Accepted. These are covered by `c459a10`, `06588ea`, `0689d70`, and the preceding `37f2289` test changes. | +| The documented membership error body did not match the emitted rejection envelope. | Accepted. `73b6199` documents `MembershipRejectedResponseDto` with `accepted: false` and the bounded error-code enum for every mapped error status. | +| `handoffTaskIds` accepted malformed non-array values and could throw while checking the ledger. | Accepted. `8414b83` normalizes and diagnoses malformed shapes, with object/string regression coverage. | +| The uniqueness migration test omitted `project_id` normalization. | Accepted. `d79c74b` asserts the project `COALESCE` expression alongside the workspace assertion. | ## Rejected findings @@ -34,7 +42,7 @@ correctness, contract, security, or test-coverage improvement in this slice. The focused fixes must pass the affected API/domain tests, OpenAPI drift check, `corepack pnpm repo:check`, `corepack pnpm repo:build`, and the hosted checks on -the follow-up `dev` PR. This document records the single-review disposition; it -does not authorize a second CodeRabbit run. PR #40 remains unmergeable until the -fix PR is merged to `dev`, its promotion checks are green, and all valid findings -are resolved. +the follow-up `dev` PR. This document records both the one full review and the +integration-triggered incremental result; it does not authorize another +CodeRabbit run. PR #40 remains unmergeable until its current checks are green +and all valid findings from both completed results are resolved. diff --git a/services/api/openapi/v1.json b/services/api/openapi/v1.json index c091d3da..98362cd8 100644 --- a/services/api/openapi/v1.json +++ b/services/api/openapi/v1.json @@ -1966,8 +1966,8 @@ "400": { "description": "The request is invalid.", "content": { - "application/problem+json": { - "schema": { "$ref": "#/components/schemas/ProblemDetails" } + "application/json": { + "schema": { "$ref": "#/components/schemas/MembershipRejectedResponseDto" } } }, "headers": { @@ -1984,8 +1984,8 @@ "403": { "description": "The authenticated actor lacks the required scope.", "content": { - "application/problem+json": { - "schema": { "$ref": "#/components/schemas/ProblemDetails" } + "application/json": { + "schema": { "$ref": "#/components/schemas/MembershipRejectedResponseDto" } } }, "headers": { @@ -2002,8 +2002,8 @@ "404": { "description": "The membership is not visible.", "content": { - "application/problem+json": { - "schema": { "$ref": "#/components/schemas/ProblemDetails" } + "application/json": { + "schema": { "$ref": "#/components/schemas/MembershipRejectedResponseDto" } } }, "headers": { @@ -2020,8 +2020,8 @@ "409": { "description": "The membership revision or ownership invariant conflicts.", "content": { - "application/problem+json": { - "schema": { "$ref": "#/components/schemas/ProblemDetails" } + "application/json": { + "schema": { "$ref": "#/components/schemas/MembershipRejectedResponseDto" } } }, "headers": { @@ -2038,8 +2038,8 @@ "410": { "description": "The invitation has expired.", "content": { - "application/problem+json": { - "schema": { "$ref": "#/components/schemas/ProblemDetails" } + "application/json": { + "schema": { "$ref": "#/components/schemas/MembershipRejectedResponseDto" } } }, "headers": { @@ -2074,8 +2074,8 @@ "503": { "description": "Membership persistence is unavailable.", "content": { - "application/problem+json": { - "schema": { "$ref": "#/components/schemas/ProblemDetails" } + "application/json": { + "schema": { "$ref": "#/components/schemas/MembershipRejectedResponseDto" } } }, "headers": { @@ -2128,8 +2128,8 @@ "400": { "description": "The request is invalid.", "content": { - "application/problem+json": { - "schema": { "$ref": "#/components/schemas/ProblemDetails" } + "application/json": { + "schema": { "$ref": "#/components/schemas/MembershipRejectedResponseDto" } } }, "headers": { @@ -2146,8 +2146,8 @@ "403": { "description": "The authenticated actor lacks the required scope.", "content": { - "application/problem+json": { - "schema": { "$ref": "#/components/schemas/ProblemDetails" } + "application/json": { + "schema": { "$ref": "#/components/schemas/MembershipRejectedResponseDto" } } }, "headers": { @@ -2164,8 +2164,8 @@ "404": { "description": "The membership is not visible.", "content": { - "application/problem+json": { - "schema": { "$ref": "#/components/schemas/ProblemDetails" } + "application/json": { + "schema": { "$ref": "#/components/schemas/MembershipRejectedResponseDto" } } }, "headers": { @@ -2182,8 +2182,8 @@ "409": { "description": "The membership revision or ownership invariant conflicts.", "content": { - "application/problem+json": { - "schema": { "$ref": "#/components/schemas/ProblemDetails" } + "application/json": { + "schema": { "$ref": "#/components/schemas/MembershipRejectedResponseDto" } } }, "headers": { @@ -2200,8 +2200,8 @@ "410": { "description": "The invitation has expired.", "content": { - "application/problem+json": { - "schema": { "$ref": "#/components/schemas/ProblemDetails" } + "application/json": { + "schema": { "$ref": "#/components/schemas/MembershipRejectedResponseDto" } } }, "headers": { @@ -2236,8 +2236,8 @@ "503": { "description": "Membership persistence is unavailable.", "content": { - "application/problem+json": { - "schema": { "$ref": "#/components/schemas/ProblemDetails" } + "application/json": { + "schema": { "$ref": "#/components/schemas/MembershipRejectedResponseDto" } } }, "headers": { @@ -2300,8 +2300,8 @@ "400": { "description": "The request is invalid.", "content": { - "application/problem+json": { - "schema": { "$ref": "#/components/schemas/ProblemDetails" } + "application/json": { + "schema": { "$ref": "#/components/schemas/MembershipRejectedResponseDto" } } }, "headers": { @@ -2318,8 +2318,8 @@ "403": { "description": "The authenticated actor lacks the required scope.", "content": { - "application/problem+json": { - "schema": { "$ref": "#/components/schemas/ProblemDetails" } + "application/json": { + "schema": { "$ref": "#/components/schemas/MembershipRejectedResponseDto" } } }, "headers": { @@ -2336,8 +2336,8 @@ "404": { "description": "The membership is not visible.", "content": { - "application/problem+json": { - "schema": { "$ref": "#/components/schemas/ProblemDetails" } + "application/json": { + "schema": { "$ref": "#/components/schemas/MembershipRejectedResponseDto" } } }, "headers": { @@ -2354,8 +2354,8 @@ "409": { "description": "The membership revision or ownership invariant conflicts.", "content": { - "application/problem+json": { - "schema": { "$ref": "#/components/schemas/ProblemDetails" } + "application/json": { + "schema": { "$ref": "#/components/schemas/MembershipRejectedResponseDto" } } }, "headers": { @@ -2372,8 +2372,8 @@ "410": { "description": "The invitation has expired.", "content": { - "application/problem+json": { - "schema": { "$ref": "#/components/schemas/ProblemDetails" } + "application/json": { + "schema": { "$ref": "#/components/schemas/MembershipRejectedResponseDto" } } }, "headers": { @@ -2408,8 +2408,8 @@ "503": { "description": "Membership persistence is unavailable.", "content": { - "application/problem+json": { - "schema": { "$ref": "#/components/schemas/ProblemDetails" } + "application/json": { + "schema": { "$ref": "#/components/schemas/MembershipRejectedResponseDto" } } }, "headers": { @@ -2470,8 +2470,8 @@ "400": { "description": "The request is invalid.", "content": { - "application/problem+json": { - "schema": { "$ref": "#/components/schemas/ProblemDetails" } + "application/json": { + "schema": { "$ref": "#/components/schemas/MembershipRejectedResponseDto" } } }, "headers": { @@ -2488,8 +2488,8 @@ "403": { "description": "The authenticated actor lacks the required scope.", "content": { - "application/problem+json": { - "schema": { "$ref": "#/components/schemas/ProblemDetails" } + "application/json": { + "schema": { "$ref": "#/components/schemas/MembershipRejectedResponseDto" } } }, "headers": { @@ -2506,8 +2506,8 @@ "404": { "description": "The membership is not visible.", "content": { - "application/problem+json": { - "schema": { "$ref": "#/components/schemas/ProblemDetails" } + "application/json": { + "schema": { "$ref": "#/components/schemas/MembershipRejectedResponseDto" } } }, "headers": { @@ -2524,8 +2524,8 @@ "409": { "description": "The membership revision or ownership invariant conflicts.", "content": { - "application/problem+json": { - "schema": { "$ref": "#/components/schemas/ProblemDetails" } + "application/json": { + "schema": { "$ref": "#/components/schemas/MembershipRejectedResponseDto" } } }, "headers": { @@ -2542,8 +2542,8 @@ "410": { "description": "The invitation has expired.", "content": { - "application/problem+json": { - "schema": { "$ref": "#/components/schemas/ProblemDetails" } + "application/json": { + "schema": { "$ref": "#/components/schemas/MembershipRejectedResponseDto" } } }, "headers": { @@ -2578,8 +2578,8 @@ "503": { "description": "Membership persistence is unavailable.", "content": { - "application/problem+json": { - "schema": { "$ref": "#/components/schemas/ProblemDetails" } + "application/json": { + "schema": { "$ref": "#/components/schemas/MembershipRejectedResponseDto" } } }, "headers": { @@ -2642,8 +2642,8 @@ "400": { "description": "The request is invalid.", "content": { - "application/problem+json": { - "schema": { "$ref": "#/components/schemas/ProblemDetails" } + "application/json": { + "schema": { "$ref": "#/components/schemas/MembershipRejectedResponseDto" } } }, "headers": { @@ -2660,8 +2660,8 @@ "403": { "description": "The authenticated actor lacks the required scope.", "content": { - "application/problem+json": { - "schema": { "$ref": "#/components/schemas/ProblemDetails" } + "application/json": { + "schema": { "$ref": "#/components/schemas/MembershipRejectedResponseDto" } } }, "headers": { @@ -2678,8 +2678,8 @@ "404": { "description": "The membership is not visible.", "content": { - "application/problem+json": { - "schema": { "$ref": "#/components/schemas/ProblemDetails" } + "application/json": { + "schema": { "$ref": "#/components/schemas/MembershipRejectedResponseDto" } } }, "headers": { @@ -2696,8 +2696,8 @@ "409": { "description": "The membership revision or ownership invariant conflicts.", "content": { - "application/problem+json": { - "schema": { "$ref": "#/components/schemas/ProblemDetails" } + "application/json": { + "schema": { "$ref": "#/components/schemas/MembershipRejectedResponseDto" } } }, "headers": { @@ -2714,8 +2714,8 @@ "410": { "description": "The invitation has expired.", "content": { - "application/problem+json": { - "schema": { "$ref": "#/components/schemas/ProblemDetails" } + "application/json": { + "schema": { "$ref": "#/components/schemas/MembershipRejectedResponseDto" } } }, "headers": { @@ -2750,8 +2750,8 @@ "503": { "description": "Membership persistence is unavailable.", "content": { - "application/problem+json": { - "schema": { "$ref": "#/components/schemas/ProblemDetails" } + "application/json": { + "schema": { "$ref": "#/components/schemas/MembershipRejectedResponseDto" } } }, "headers": { @@ -9007,6 +9007,28 @@ }, "required": ["kind", "name"] }, + "MembershipRejectedResponseDto": { + "type": "object", + "properties": { + "accepted": { "type": "boolean", "enum": [false], "example": false }, + "code": { + "type": "string", + "enum": [ + "INVALID_IDENTIFIER", + "INVALID_SCOPE", + "INVALID_ROLE", + "INVALID_STATE", + "SCOPE_DENIED", + "NOT_FOUND", + "CONFLICT", + "EXPIRED", + "LAST_OWNER", + "UNAVAILABLE" + ] + } + }, + "required": ["accepted", "code"] + }, "MembershipScopeDto": { "type": "object", "properties": { diff --git a/services/api/src/features/iam/api/membership.controller.ts b/services/api/src/features/iam/api/membership.controller.ts index 72462cdd..64a39a24 100644 --- a/services/api/src/features/iam/api/membership.controller.ts +++ b/services/api/src/features/iam/api/membership.controller.ts @@ -38,6 +38,7 @@ import { import { AcceptMembershipDto, InviteMembershipDto, + MembershipRejectedResponseDto, TransferOwnershipDto, TransitionMembershipDto, } from './membership.dto.js'; @@ -70,24 +71,30 @@ function preserveMembershipStatus(result: TValue, reply?: FastifyReply): } function applyMembershipOutcomeResponses(): MethodDecorator { - const content = { - 'application/problem+json': { schema: { $ref: '#/components/schemas/ProblemDetails' } }, - }; return applyDecorators( - ApiBadRequestResponse({ description: 'The request is invalid.', content }), + ApiBadRequestResponse({ + description: 'The request is invalid.', + type: MembershipRejectedResponseDto, + }), ApiForbiddenResponse({ description: 'The authenticated actor lacks the required scope.', - content, + type: MembershipRejectedResponseDto, + }), + ApiNotFoundResponse({ + description: 'The membership is not visible.', + type: MembershipRejectedResponseDto, }), - ApiNotFoundResponse({ description: 'The membership is not visible.', content }), ApiConflictResponse({ description: 'The membership revision or ownership invariant conflicts.', - content, + type: MembershipRejectedResponseDto, + }), + ApiGoneResponse({ + description: 'The invitation has expired.', + type: MembershipRejectedResponseDto, }), - ApiGoneResponse({ description: 'The invitation has expired.', content }), ApiServiceUnavailableResponse({ description: 'Membership persistence is unavailable.', - content, + type: MembershipRejectedResponseDto, }), ); } diff --git a/services/api/src/features/iam/api/membership.dto.ts b/services/api/src/features/iam/api/membership.dto.ts index 11a28864..3a71d9b2 100644 --- a/services/api/src/features/iam/api/membership.dto.ts +++ b/services/api/src/features/iam/api/membership.dto.ts @@ -22,8 +22,21 @@ const MEMBERSHIP_ROLE_IDS = [ 'approver', 'viewer', ] as const; +const MEMBERSHIP_ERROR_CODES = [ + 'INVALID_IDENTIFIER', + 'INVALID_SCOPE', + 'INVALID_ROLE', + 'INVALID_STATE', + 'SCOPE_DENIED', + 'NOT_FOUND', + 'CONFLICT', + 'EXPIRED', + 'LAST_OWNER', + 'UNAVAILABLE', +] as const; type MembershipScopeTypeDtoV1 = (typeof MEMBERSHIP_SCOPE_TYPES)[number]; type MembershipRoleIdDtoV1 = (typeof MEMBERSHIP_ROLE_IDS)[number]; +type MembershipErrorCodeDtoV1 = (typeof MEMBERSHIP_ERROR_CODES)[number]; @ValidatorConstraint({ name: 'membershipScopeShape', async: false }) class MembershipScopeShapeConstraint implements ValidatorConstraintInterface { @@ -78,6 +91,14 @@ export class InviteMembershipDto { roleId!: MembershipRoleIdDtoV1; } +export class MembershipRejectedResponseDto { + @ApiProperty({ enum: [false], example: false }) + accepted!: false; + + @ApiProperty({ enum: MEMBERSHIP_ERROR_CODES }) + code!: MembershipErrorCodeDtoV1; +} + export class TransitionMembershipDto { @ApiProperty({ minimum: 1, maximum: Number.MAX_SAFE_INTEGER }) @IsInt() diff --git a/services/api/test/openapi.test.ts b/services/api/test/openapi.test.ts index b566bd96..556aa5eb 100644 --- a/services/api/test/openapi.test.ts +++ b/services/api/test/openapi.test.ts @@ -71,6 +71,19 @@ void test('generates deterministic versioned OpenAPI with safe headers, errors, )?.schema?.$ref, '#/components/schemas/BootstrapResponseDto', ); + const membershipResponses = ( + firstDocument.paths['/v1/memberships']?.get as OperationLike | undefined + )?.responses; + for (const status of ['400', '403', '404', '409', '410', '503']) { + assert.equal( + ( + membershipResponses?.[status]?.content?.['application/json'] as + | { readonly schema?: { readonly $ref?: string } } + | undefined + )?.schema?.$ref, + '#/components/schemas/MembershipRejectedResponseDto', + ); + } const paths = Object.keys(firstDocument.paths).sort(); assert.deepEqual(paths, [ diff --git a/services/api/test/prisma-foundation.test.mjs b/services/api/test/prisma-foundation.test.mjs index 67e612aa..49a22a3a 100644 --- a/services/api/test/prisma-foundation.test.mjs +++ b/services/api/test/prisma-foundation.test.mjs @@ -535,4 +535,5 @@ test('the schema diff and centrally ordered migration inventory establish platfo /CREATE UNIQUE INDEX "memberships_principal_scope_identity_key"/u, ); assert.match(membershipUniquenessMigration, /COALESCE\("workspace_id"::text, ''\)/u); + assert.match(membershipUniquenessMigration, /COALESCE\("project_id"::text, ''\)/u); }); diff --git a/tools/repo-cli/src/check-execution-orchestration.mjs b/tools/repo-cli/src/check-execution-orchestration.mjs index 71be74a3..ab7e2b5e 100644 --- a/tools/repo-cli/src/check-execution-orchestration.mjs +++ b/tools/repo-cli/src/check-execution-orchestration.mjs @@ -135,6 +135,7 @@ function validateDeliveryBatches({ ledger, plans, taskIds, taskToPlan, diagnosti const batches = Array.isArray(ledger.deliveryBatches) ? ledger.deliveryBatches : []; const byId = new Map(); const batchByTask = new Map(); + const handoffTaskIdsByBatch = new Map(); for (const batch of batches) { if (typeof batch.batchId !== 'string' || batch.batchId.trim() === '') { @@ -176,7 +177,18 @@ function validateDeliveryBatches({ ledger, plans, taskIds, taskToPlan, diagnosti diagnostics.push(`batch ${batch.batchId} has no tasks`); continue; } - const handoffTaskIds = new Set(batch.handoffTaskIds ?? []); + const rawHandoffTaskIds = batch.handoffTaskIds; + const normalizedHandoffTaskIds = + rawHandoffTaskIds === undefined + ? [] + : Array.isArray(rawHandoffTaskIds) + ? rawHandoffTaskIds + : []; + if (rawHandoffTaskIds !== undefined && !Array.isArray(rawHandoffTaskIds)) { + diagnostics.push(`batch ${batch.batchId} handoffTaskIds must be an array`); + } + handoffTaskIdsByBatch.set(batch.batchId, normalizedHandoffTaskIds); + const handoffTaskIds = new Set(normalizedHandoffTaskIds); for (const taskId of handoffTaskIds) { if (!batch.taskIds.includes(taskId)) { diagnostics.push(`batch ${batch.batchId} handoff task ${taskId} is not in taskIds`); @@ -223,7 +235,7 @@ function validateDeliveryBatches({ ledger, plans, taskIds, taskToPlan, diagnosti .map(([taskId]) => taskId), ); for (const batch of batches) { - for (const taskId of batch.handoffTaskIds ?? []) { + for (const taskId of handoffTaskIdsByBatch.get(batch.batchId) ?? []) { if (!verifiedTasks.has(taskId)) { diagnostics.push(`batch ${batch.batchId} handoff task ${taskId} is not verified`); } @@ -232,8 +244,7 @@ function validateDeliveryBatches({ ledger, plans, taskIds, taskToPlan, diagnosti for (const taskId of taskIds) { if (verifiedTasks.has(taskId)) { const batchId = batchByTask.get(taskId); - const batch = batchId === undefined ? undefined : byId.get(batchId); - if (batchId !== undefined && !batch?.handoffTaskIds?.includes(taskId)) { + if (batchId !== undefined && !handoffTaskIdsByBatch.get(batchId)?.includes(taskId)) { diagnostics.push(`verified task ${taskId} remains batched without handoff declaration`); } } else if (!batchByTask.has(taskId)) { diff --git a/tools/repo-cli/test/execution-orchestration.test.mjs b/tools/repo-cli/test/execution-orchestration.test.mjs index 831e50dd..f960f21b 100644 --- a/tools/repo-cli/test/execution-orchestration.test.mjs +++ b/tools/repo-cli/test/execution-orchestration.test.mjs @@ -321,3 +321,17 @@ test('repository checker rejects delivery-batch drift from the Luna plan', () => }, ); }); + +test('repository checker rejects malformed handoff task lists', () => { + for (const malformed of [{ invalid: true }, 'FND-007']) { + withTemporaryPlans( + ({ ledger }) => { + ledger.deliveryBatches[0].handoffTaskIds = malformed; + }, + (result) => { + assert.notEqual(result.status, 0); + assert.match(result.stderr, /handoffTaskIds must be an array/u); + }, + ); + } +});