feat: complete IAE and DSM foundation - #6
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe pull request adds versioned domain contracts for artifact intake and governance, tenant-scoped IAE and DSM services with Prisma and in-memory adapters, authenticated API routes and OpenAPI schemas, a localized web inbox, and deterministic dataset profiling. ChangesGoverned domain contracts
Persistence and API contracts
IAE runtime
DSM runtime
Web inbox
Dataset profiling
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Browser
participant InboxPage
participant listInbox
participant InboxAPI
Browser->>InboxPage: Open inbox route
InboxPage->>listInbox: Request inbox items
listInbox->>InboxAPI: Send authenticated GET request
InboxAPI-->>listInbox: Return validated inbox items
listInbox-->>InboxPage: Resolve item list or error
InboxPage-->>Browser: Render loading, error, empty, or table state
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Comment |
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
🟠 Major comments (28)
packages/domain/src/artifact/v1.ts-360-364 (1)
360-364: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winValidate
geometryat runtime before using it.Falsy values bypass IAE-006, and malformed truthy bounds such as
'100'orInfinitycan pass through coercive comparisons. Parsegeometryand returnrejected('INVALID_REFERENCE')when it is invalid.🤖 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/src/artifact/v1.ts` around lines 360 - 364, Update the geometry handling in the EvidenceGeometryV1 validation flow to parse and validate input.geometry at runtime before calling validateEvidenceCoordinateV1. Reject falsy, malformed, non-numeric, non-finite, or otherwise invalid bounds with rejected('INVALID_REFERENCE'), and only pass the validated geometry to validateEvidenceCoordinateV1.packages/domain/src/rule-set/v1.ts-105-128 (1)
105-128: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winUnknown parameter keys pass validation for
TYPE,RANGE, andREFERENCErules.The
else ifbranch at Line 126 rejects extra keys only forREQUIREDandUNIQUE. ForTYPE,RANGE, andREFERENCE, the code validates the expected key and ignores every other key. Line 134 then copies all keys into the frozenparametersobject, so arbitrary caller-supplied data reaches the persisted rule set. For example,{ kind: 'TYPE', parameters: { expectedType: 'TEXT', script: 'drop table' } }is accepted. The existing test atpackages/domain/test/mapping-rule-set-v1.test.mjsLine 127 covers only theREQUIREDkind, so this gap is untested.Validate the allowed key set for every kind.
🛠️ Proposed fix
if (typeof parameters !== 'object' || parameters === null || Array.isArray(parameters)) return 'INVALID_PARAMETERS'; + const allowedKeys: Record<string, readonly string[]> = { + REQUIRED: [], + UNIQUE: [], + TYPE: ['expectedType'], + RANGE: ['minimum', 'maximum'], + REFERENCE: ['referenceEntityVersionId'], + }; + const permitted = allowedKeys[kind as string] ?? []; + if (Object.keys(parameters).some((key) => !permitted.includes(key))) + return 'INVALID_PARAMETERS'; if (kind === 'TYPE') {🤖 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/src/rule-set/v1.ts` around lines 105 - 128, Update the rule-parameter validation around the TYPE, RANGE, and REFERENCE branches so each kind rejects any keys outside its defined parameter schema: expectedType, minimum/maximum, and referenceEntityVersionId respectively. Preserve the existing value and range validation, and ensure unknown keys return INVALID_PARAMETERS before parameters are persisted.packages/domain/src/reference-entity/v1.ts-186-195 (1)
186-195: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winReject merges involving non-
ACTIVEparties. If either party has statusMERGEDorINACTIVE, returnrejected('INVALID_STATE')before saving the resolution.🤖 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/src/reference-entity/v1.ts` around lines 186 - 195, Update the resolution validation flow around the source and target entity checks to reject any merge where either party’s status is not ACTIVE, returning rejected('INVALID_STATE') before the resolution is saved. Preserve the existing identifier, scope, same-entity, reason, and timestamp validations.services/api/src/features/dsm/application/mapping.service.ts-26-34 (1)
26-34: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
createthrows instead of returning a rejection envelope.Line 30 throws a raw
Error('DSM_IMMUTABLE_MAPPING'). Every other failure path in this service returnsMappingServiceResultV1.MappingController.createdoes not catch the error, so a client that resends a changed body for an existingversionIdreceives HTTP 500 for a client-caused conflict.A second path reaches the same throw indirectly. If the existing version belongs to a scope that
findcannot see, line 27 returnsundefinedand line 32 callssave. The Prisma adapter then hits the unique constraint onidand surfaces a raw database error.Add a conflict code to
MappingServiceErrorV1and return it.♻️ Proposed fix
-export type MappingServiceErrorV1 = 'VERSION_NOT_FOUND'; +export type MappingServiceErrorV1 = 'VERSION_NOT_FOUND' | 'IMMUTABLE_MAPPING';const existing = await transaction.find(context, created.value.versionId); if (existing) { if (JSON.stringify(existing) === JSON.stringify(created.value)) return created; - throw new Error('DSM_IMMUTABLE_MAPPING'); + return Object.freeze({ + accepted: false as const, + code: 'IMMUTABLE_MAPPING' as const, + }); }🤖 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/dsm/application/mapping.service.ts` around lines 26 - 34, Add a conflict error code to MappingServiceErrorV1 and update create so immutable mapping conflicts return the established MappingServiceResultV1 rejection envelope instead of throwing. In the transaction around repository.find/save, detect an existing versionId conflict—including records hidden by scope—and return the conflict result before save; preserve the idempotent return when the existing mapping matches.services/api/src/features/dsm/adapter/prisma-mapping-repository.adapter.ts-115-127 (1)
115-127: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
saveraces betweenfindUniqueandcreate.Lines 118-126 read the row, then create it. Two concurrent saves of the same
versionIdcan both observenulland both callcreate. The unique constraint onidrejects the second write, so stored data stays correct. The caller then receives a raw Prisma unique-violation error instead ofDSM_IMMUTABLE_MAPPING.Prisma's default
Read Committedisolation does not prevent this, so the transactional path at line 160 has the same exposure.Catch the unique-violation and re-run the immutability comparison.
♻️ Proposed fix
if (existing !== null) { if (JSON.stringify(rowToDomain(existing)) !== JSON.stringify(definition)) throw new Error('DSM_IMMUTABLE_MAPPING'); return; } - await this.client.mappingDefinitionRecord.create({ data: domainToRow(definition) }); + try { + await this.client.mappingDefinitionRecord.create({ data: domainToRow(definition) }); + } catch (error) { + const concurrent = await this.client.mappingDefinitionRecord.findUnique({ + where: { id: definition.versionId }, + }); + if (concurrent === null) throw error; + if (JSON.stringify(rowToDomain(concurrent)) !== JSON.stringify(definition)) + throw new Error('DSM_IMMUTABLE_MAPPING'); + }🤖 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/dsm/adapter/prisma-mapping-repository.adapter.ts` around lines 115 - 127, Update save and the transactional save path to handle Prisma unique-constraint violations from the create operation: when creation races on the same versionId, re-fetch the stored record, compare it with definition using the existing rowToDomain immutability check, and throw DSM_IMMUTABLE_MAPPING for mismatches while treating identical definitions as successful. Preserve propagation of unrelated database errors and reuse the existing transaction logic around the transactional path.services/api/src/features/dsm/adapter/prisma-reference-entity-repository.adapter.ts-235-278 (1)
235-278: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftAlign resolution behavior with the in-memory adapter.
Three differences exist between this adapter and
InMemoryReferenceEntityRepositoryAdapter, which implement the sameReferenceEntityTransactionPortV1:
saveResolutiondoes not verify that the source and target versions exist and are visible. The in-memory adapter throwsDSM_REFERENCE_ENTITY_NOT_FOUND. This adapter persists a resolution that references entities outside the caller scope.saveResolutiondoes not enforce scope narrowing, whilesaveVersiondoes at Line 174.listResolutionsfilters onsourceEntityIdonly. The in-memory adapter matchessourceEntityIdortargetEntityId, so merge history for a merge target is returned in tests but not in production.Add the existence and scope checks, and query both sides of the resolution.
🐛 Proposed direction for the listing fix
public async listResolutions( context: IamTenantContextV1, entityId: BusinessPartyVersionV1['entityId'], ): Promise<readonly BusinessPartyResolutionV1[]> { - const rows = await this.client.referenceEntityResolutionRecord.findMany({ - where: { organizationId: context.tenantScope.organizationId, sourceEntityId: entityId }, - orderBy: { resolvedAt: 'desc' }, - }); - return rows.filter((row) => visible(context.tenantScope, row)).map(rowToResolution); + const [asSource, asTarget] = await Promise.all([ + this.client.referenceEntityResolutionRecord.findMany({ + where: { organizationId: context.tenantScope.organizationId, sourceEntityId: entityId }, + orderBy: { resolvedAt: 'desc' }, + }), + this.client.referenceEntityResolutionRecord.findMany({ + where: { organizationId: context.tenantScope.organizationId, targetEntityId: entityId }, + orderBy: { resolvedAt: 'desc' }, + }), + ]); + const unique = new Map([...asSource, ...asTarget].map((row) => [row.id, row])); + return [...unique.values()] + .filter((row) => visible(context.tenantScope, row)) + .sort((left, right) => right.resolvedAt.getTime() - left.resolvedAt.getTime()) + .map(rowToResolution); }🤖 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/dsm/adapter/prisma-reference-entity-repository.adapter.ts` around lines 235 - 278, Align saveResolution with InMemoryReferenceEntityRepositoryAdapter by applying the same tenant-scope narrowing as saveVersion and validating that both sourceEntityId and targetEntityId resolve to visible versions in the caller’s scope, throwing DSM_REFERENCE_ENTITY_NOT_FOUND otherwise. Update listResolutions to retrieve records where entityId matches either sourceEntityId or targetEntityId, while retaining tenant scoping, visibility filtering, and descending resolvedAt ordering.services/api/src/features/dsm/api/governed-dataset.controller.ts-44-52 (1)
44-52: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winRejected identifiers return HTTP 200 across the DSM controllers. Each handler returns
{ accepted: false, code: 'INVALID_IDENTIFIER' }afterparseStableIdentifierV1rejects the path parameter. NestJS serializes a returned object with the route's success status, so a client that checks the status code treats an invalid identifier as success. The shared root cause is signalling a validation failure through the response body instead of the status code.
services/api/src/features/dsm/api/governed-dataset.controller.ts#L44-L52: throwBadRequestException({ code: 'INVALID_IDENTIFIER' })inlistinstead of returning the result object.services/api/src/features/dsm/api/reference-entity.controller.ts#L45-L52: throwBadRequestException({ code: 'INVALID_IDENTIFIER' })inlistinstead of returning the result object.services/api/src/features/dsm/api/rule-set.controller.ts#L32-L57: throwBadRequestException({ code: 'INVALID_IDENTIFIER' })in bothcreateandlistinstead of returning the result object.If you prefer to keep the result-object style, register one exception filter or interceptor that maps
{ accepted: false }results to the matching status code, and apply it to all four handlers.🤖 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/dsm/api/governed-dataset.controller.ts` around lines 44 - 52, Replace rejected identifier result objects with BadRequestException({ code: 'INVALID_IDENTIFIER' }) so invalid path parameters produce HTTP 400. Apply this in governed-dataset.controller.ts lines 44-52 within list, reference-entity.controller.ts lines 45-52 within list, and rule-set.controller.ts lines 32-57 within both create and list; preserve the existing accepted-identifier flows.services/api/src/features/dsm/adapter/prisma-governed-dataset-repository.adapter.ts-140-149 (1)
140-149: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winThe repository-level
saveruns a check-then-create outside any transaction. Both transaction adapters implementsaveas afindUniquefollowed by acreate. The transactional path throughwithTransactionis safe, but the repository classes call the same method on the base client. Two concurrent saves of the sameversionIdboth observenulland both callcreate. The second call fails with a raw Prisma unique-constraint error rather than the intended immutability error.
services/api/src/features/dsm/adapter/prisma-governed-dataset-repository.adapter.ts#L140-L149: wrapPrismaGovernedDatasetRepositoryAdapter.saveat Line 188 inthis.client.$transaction, so thefindUniqueandcreatein the transaction adapter run atomically.services/api/src/features/dsm/adapter/prisma-rule-set-repository.adapter.ts#L163-L165: wrapPrismaRuleSetRepositoryAdapter.saveinthis.client.$transactionin the same way.Check whether the mapping and reference-entity adapters in the same directory repeat this pattern.
🤖 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/dsm/adapter/prisma-governed-dataset-repository.adapter.ts` around lines 140 - 149, Make repository-level saves atomic by wrapping the findUnique-and-create flow in PrismaGovernedDatasetRepositoryAdapter.save; in services/api/src/features/dsm/adapter/prisma-governed-dataset-repository.adapter.ts lines 140-149, execute the transaction adapter through this.client.$transaction. Apply the same wrapping to PrismaRuleSetRepositoryAdapter.save in services/api/src/features/dsm/adapter/prisma-rule-set-repository.adapter.ts lines 163-165. Inspect the mapping and reference-entity adapters in the same directory for the same check-then-create pattern and apply the equivalent transaction fix where present.services/api/src/features/dsm/application/governed-dataset.service.ts-28-36 (1)
28-36: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winThe immutability conflict escapes the declared result contract.
createdeclaresPromise<GovernedDatasetServiceResultV1<...>>. Every other failure path returns a discriminated result: the domain rejection at Line 27 andVERSION_NOT_FOUNDat Line 47. Line 32 instead throws a rawError.GovernedDatasetController.createdoes not catch it, and no exception filter is registered inDsmModule. A client that posts a duplicateversionIdwith different content receives HTTP 500 for a client-caused conflict.Return a typed result and extend
GovernedDatasetServiceErrorV1.🐛 Proposed fix
-export type GovernedDatasetServiceErrorV1 = 'VERSION_NOT_FOUND'; +export type GovernedDatasetServiceErrorV1 = 'VERSION_NOT_FOUND' | 'IMMUTABLE_DEFINITION';const existing = await transaction.find(context, created.value.versionId); if (existing) { if (JSON.stringify(existing) === JSON.stringify(created.value)) return created; - throw new Error('DSM_IMMUTABLE_DEFINITION'); + return Object.freeze({ accepted: false, code: 'IMMUTABLE_DEFINITION' as const }); }Note that the adapters still throw
DSM_IMMUTABLE_DEFINITIONfromsave. Map that path to the same result or to a 409 response.🤖 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/dsm/application/governed-dataset.service.ts` around lines 28 - 36, Update the create flow in GovernedDatasetService so an existing versionId with different content returns a typed GovernedDatasetServiceErrorV1 result instead of throwing raw Error('DSM_IMMUTABLE_DEFINITION'); extend the error union with the corresponding conflict outcome and ensure the adapter save path that throws the same code is mapped consistently to that result or HTTP 409 by GovernedDatasetController.create.services/api/prisma/migrations/20260802130000_iae_dsm_scope_hardening/migration.sql-48-56 (1)
48-56: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winSome scope combinations match no partial unique index.
The three predicates together do not cover every possible row. A row with
scope_type = 'workspace'andworkspace_id IS NULLmatches none of them. The same applies toscope_type = 'project'with a NULLproject_id, and to anyscope_typevalue outside the three literals. Such a row gets no idempotency-key uniqueness at all, and the failure is silent.Add a
CHECKconstraint that makes the scope columns consistent withscope_type. The three partial indexes then cover every row that the constraint permits.🛡️ Proposed constraint
ALTER TABLE "iae"."inbox_items" ADD CONSTRAINT "inbox_items_scope_shape" CHECK ( ("scope_type" = 'organization' AND "workspace_id" IS NULL AND "project_id" IS NULL) OR ("scope_type" = 'workspace' AND "workspace_id" IS NOT NULL AND "project_id" IS NULL) OR ("scope_type" = 'project' AND "workspace_id" IS NOT NULL AND "project_id" IS NOT NULL) );🤖 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/prisma/migrations/20260802130000_iae_dsm_scope_hardening/migration.sql` around lines 48 - 56, Add a CHECK constraint named inbox_items_scope_shape to the iae.inbox_items table, enforcing that organization scope has no workspace or project, workspace scope has a workspace but no project, and project scope has both identifiers. Place it alongside the three partial unique indexes so every permitted row matches exactly one idempotency-key uniqueness index.services/api/prisma/migrations/20260802100000_iae_dsm_governance/migration.sql-18-19 (1)
18-19: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winA unique constraint over nullable tenant-scope columns does not enforce idempotency. Both sites declare uniqueness on
("organization_id", "workspace_id", "project_id", "idempotency_key"). PostgreSQL treats NULL values as distinct in a unique index, so organization-scoped and workspace-scoped items can repeat an idempotency key. Migration20260802130000_iae_dsm_scope_hardeningadds partial unique indexes that enforce the rule, but neither site is updated to reflect that.
services/api/prisma/migrations/20260802100000_iae_dsm_governance/migration.sql#L18-L19: dropinbox_items_scope_idempotency_keyin the20260802130000_iae_dsm_scope_hardeningmigration, or recreate it as a plain index.services/api/prisma/schema/iae.prisma#L40: change the@@uniqueto@@indexso the model does not claim a constraint that the database does not enforce, and keep the three partial unique indexes as raw SQL.🤖 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/prisma/migrations/20260802100000_iae_dsm_governance/migration.sql` around lines 18 - 19, Update services/api/prisma/migrations/20260802100000_iae_dsm_governance/migration.sql lines 18-19 so inbox_items_scope_idempotency_key is dropped by the 20260802130000_iae_dsm_scope_hardening migration or recreated as a plain index. Update services/api/prisma/schema/iae.prisma line 40 by changing the @@unique declaration to @@index, while preserving the three partial unique indexes as raw SQL.services/api/prisma/migrations/20260802100000_iae_dsm_governance/migration.sql-35-36 (1)
35-36: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winDo not backfill
canonical_hashwith a valid-looking sentinel.
repeat('0', 64)produces a value that satisfies every downstream hash check. It matches the^[0-9a-f]{64}$pattern thatCreateGovernedDatasetDto.canonicalHashdeclares inservices/api/openapi/v1.json. Any equivalence or deduplication comparison then treats all pre-existingdataset_definitionsrows as one identical definition.If
dsm.dataset_definitionscan already hold rows in a deployed environment, compute the real hash in a backfill step. If the table is always empty at this point in the migration history, add the column without a default and state that in a comment.🛠️ Option: add as nullable, backfill, then enforce
ALTER TABLE "dsm"."dataset_definitions" - ADD COLUMN "canonical_hash" CHAR(64) NOT NULL DEFAULT repeat('0', 64); + ADD COLUMN "canonical_hash" CHAR(64); +-- Backfill "canonical_hash" from the canonical serialization of each row, then: +-- ALTER TABLE "dsm"."dataset_definitions" ALTER COLUMN "canonical_hash" SET NOT NULL;🤖 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/prisma/migrations/20260802100000_iae_dsm_governance/migration.sql` around lines 35 - 36, Change the canonical_hash migration for dsm.dataset_definitions so existing rows are not assigned repeat('0', 64) or another hash-shaped sentinel. If rows may exist, add the column nullable, backfill each row with its real canonical hash, then enforce NOT NULL; if the table is guaranteed empty, add it without a default and document that assumption in the migration.services/api/src/features/iae/iae.module.ts-68-71 (1)
68-71: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy liftEvidence grants have no durable adapter.
EVIDENCE_GRANT_REPOSITORY_PORTresolves toInMemoryEvidenceGrantRepositoryAdapterunless the host passes a repository. The other two ports accept a Prisma database client. As a result, in a multi-instance deployment a grant issued on one instance is unknown to the others, and a revocation does not propagate. A restart also drops all active grants.If grants must stay ephemeral and node-local, record that decision in a comment. If not, add a Prisma-backed adapter and an
evidenceGrantDatabaseoption that mirrors the other ports.🤖 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/iae/iae.module.ts` around lines 68 - 71, Update the IAE module’s EVIDENCE_GRANT_REPOSITORY_PORT configuration to use a durable Prisma-backed evidence-grant repository and add an evidenceGrantDatabase option mirroring the existing database-backed ports. Preserve the in-memory adapter only if node-local ephemeral grants are an intentional design decision, and document that decision at the provider.services/api/src/features/iae/adapter/prisma-artifact-repository.adapter.ts-194-216 (1)
194-216: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftHandle the race between
findUniqueandcreate.The check-then-create sequence is not atomic. Two concurrent replays of the same version can both observe
existing === nulland both callcreate. The second call then fails with a raw unique-constraint error instead of the intendedIAE_IMMUTABLE_VERSIONbehaviour, and it is not idempotent. The publicsaveVersionon line 322 also runs outside any transaction, so the database is the only serialization point.Catch the unique-violation error and re-read the row, then apply the same equality check. An
upsertwith an emptyupdateis another option, but it hides content divergence.🔒 Proposed fix sketch
- await this.client.artifactVersion.create({ - data: { - ...databaseScope(version.tenantScope), - id: version.versionId, - ... - }, - }); + try { + await this.client.artifactVersion.create({ data: { /* unchanged */ } }); + } catch (error) { + const replay = await this.client.artifactVersion.findUnique({ + where: { id: version.versionId }, + }); + if (replay === null) throw error; + if (JSON.stringify(rowToVersion(replay)) !== JSON.stringify(version)) + throw new Error('IAE_IMMUTABLE_VERSION'); + }🤖 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/iae/adapter/prisma-artifact-repository.adapter.ts` around lines 194 - 216, Update the version persistence flow around findUnique and artifactVersion.create to handle a concurrent unique-constraint failure: catch the database unique-violation error, re-read the row by version.versionId, compare it with version using the existing rowToVersion equality check, and return only for an identical row; otherwise throw IAE_IMMUTABLE_VERSION. Preserve the current behavior for non-unique database errors and avoid relying on an upsert that could hide content divergence.services/api/test/features/iae/prisma-artifact-repository.test.ts-101-144 (1)
101-144: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winAdd negative scope coverage.
The test name states that the adapter "keeps placement and evidence tenant scoped", but every call uses the same workspace context. No case attempts a read or a write from a sibling workspace. The gap in
savePlacementandsaveEvidencethat I flagged inservices/api/src/features/iae/adapter/prisma-artifact-repository.adapter.tsis not detected by this test for that reason.Add cases that use a sibling workspace context: one that attaches a placement to a version owned by another workspace, and one that lists placements and evidence and expects an empty result.
🤖 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/iae/prisma-artifact-repository.test.ts` around lines 101 - 144, Extend the test around PrismaArtifactRepositoryAdapter with sibling-workspace negative coverage: attempt savePlacement and saveEvidence using a version owned by the original workspace but a sibling workspace context, and assert those writes are rejected or not persisted according to the adapter contract. Also call listPlacements and listEvidence with the sibling context and assert both return empty results, while preserving the existing same-workspace assertions.services/api/test/features/iae/inbox.controller.test.ts-49-52 (1)
49-52: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winProject inbox items through an explicit response DTO.
InboxController.list()returnsInboxItemV1[]directly. Add an allowlistedInboxItemResponseDtoprojection and assert that exact response shape instead of comparing withcreated.value.🤖 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/iae/inbox.controller.test.ts` around lines 49 - 52, Update InboxController.list() to project each InboxItemV1 through an explicit allowlisted InboxItemResponseDto, rather than returning created.value directly. Define the DTO fields from the intended public response contract and update the inbox controller test to assert the exact projected shape while retaining the exclusion checks for opaque, path, byte, and excerpt fields.services/api/src/features/iae/adapter/in-memory-artifact-intake-repository.adapter.ts-53-64 (1)
53-64: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftThe two intake adapters implement
ArtifactIntakeRepositoryPortV1differently, and nothing detects it. The port declares signatures without semantics, and every service test runs against the in-memory adapter only.findByIdempotencytherefore matches any visible scope in memory and the exact caller scope in Prisma, soArtifactIntakeService.createreaches different branches in tests and in production.
services/api/src/features/iae/adapter/in-memory-artifact-intake-repository.adapter.ts#L53-L64: replace thevisible()predicate with exact-scope equality, so the lookup matchesexactScopeWherein the Prisma adapter and the scoped uniqueness constraint in the migrations.services/api/src/features/iae/application/artifact-intake-repository.port.ts#L7-L25: add TSDoc that fixes the scope-matching rule forfindByIdempotency, states whethersaveaccepts a state transition, states whethersavemust enforcecontext.expectedRevision, and lists the required error identifiers.services/api/test/features/iae/artifact-intake.service.test.ts#L53-L56: parameterize the suite over both adapter implementations, using the existing fakeArtifactIntakeDatabaseClientV1for the Prisma entry, so any future divergence fails the build.🤖 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/iae/adapter/in-memory-artifact-intake-repository.adapter.ts` around lines 53 - 64, The in-memory and Prisma intake adapters have divergent scope semantics and the service tests cover only one implementation. In services/api/src/features/iae/adapter/in-memory-artifact-intake-repository.adapter.ts:53-64, update findByIdempotency to use exact tenant-scope equality matching exactScopeWhere; in services/api/src/features/iae/application/artifact-intake-repository.port.ts:7-25, add TSDoc defining findByIdempotency scope matching, save state-transition and expectedRevision rules, and required error identifiers; in services/api/test/features/iae/artifact-intake.service.test.ts:53-56, parameterize the suite over both adapters, using the existing fake ArtifactIntakeDatabaseClientV1 for the Prisma adapter.services/api/src/features/iae/application/artifact-intake.service.ts-34-42 (1)
34-42: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winThe idempotency check ignores
inboxItemIdand returns a different item as a success.The comparison on lines 37-38 covers
artifactVersionIdandtenantScopeonly.inboxItemIdis client-supplied and is not compared.A second request that reuses the idempotency key with a new
inboxItemId, the sameartifactVersionId, and the same scope takes the branch on line 41. The service returns{ accepted: true, value: existing }. The caller receives an item whoseinboxItemIddiffers from the one it sent, and no error indicates the substitution.
InMemoryArtifactIntakeRepositoryAdapter.savetreats that case asIAE_IDEMPOTENCY_CONFLICT. The early return meanssaveis never called, so the adapter guard never runs. The service and the adapter disagree about the same scenario.Include
inboxItemIdin the conflict comparison.🐛 Proposed fix
if (existing) { if ( + existing.inboxItemId !== created.value.inboxItemId || existing.artifactVersionId !== created.value.artifactVersionId || JSON.stringify(existing.tenantScope) !== JSON.stringify(created.value.tenantScope) ) return Object.freeze({ accepted: false, code: 'IDEMPOTENCY_CONFLICT' as const }); return Object.freeze({ accepted: true, value: existing }); }🤖 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/iae/application/artifact-intake.service.ts` around lines 34 - 42, Update the existing idempotency comparison in the artifact-intake service to also compare existing.inboxItemId with created.value.inboxItemId. Preserve the IDEMPOTENCY_CONFLICT response for any mismatch and only return the existing item as accepted when all three fields—artifactVersionId, tenantScope, and inboxItemId—match.services/api/src/features/iae/adapter/prisma-artifact-intake-repository.adapter.ts-195-201 (1)
195-201: 🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy lift
listloads every inbox item in the organization and filters in memory.The query filters only on
organizationId. It has notake, no cursor, and no scope predicate forworkspaceIdorprojectId. Every row of the organization is transferred, thenvisiblerunsparseTenantScopeV1for each row, androwToDomainre-validates each surviving row.
InboxController.listexposes this onGET /v1/artifacts/inboxwith no pagination. For a large organization the request loads the whole table into memory and the latency grows with total intake volume, not with the caller's scope.Push the scope filter into the query and add pagination.
⚡ Push the visibility predicate into the query
public async list(context: IamTenantContextV1): Promise<readonly InboxItemV1[]> { + const scope = databaseScope(context.tenantScope); const rows = await this.client.inboxItem.findMany({ - where: { organizationId: context.tenantScope.organizationId }, + // Ancestor-or-descendant visibility, expressed in SQL. + where: { + organizationId: scope.organizationId, + OR: [ + { workspaceId: null }, + ...(scope.workspaceId === null ? [] : [{ workspaceId: scope.workspaceId }]), + ], + }, orderBy: { createdAt: 'desc' }, + take: limit, }); return rows.filter((row) => visible(context.tenantScope, row)).map(rowToDomain); }The
wheretype onArtifactIntakeDatabaseDelegateV1.findManyisRecord<string, string | null>, so it must widen to acceptORand nested predicates. Paging also needs a matching change on the port, the service, and the controller.🤖 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/iae/adapter/prisma-artifact-intake-repository.adapter.ts` around lines 195 - 201, Update PrismaArtifactIntakeRepositoryAdapter.list to apply tenant visibility directly in the findMany where clause, including organizationId plus the appropriate workspaceId/projectId scope predicates, and add bounded cursor/limit pagination instead of loading the entire organization. Widen ArtifactIntakeDatabaseDelegateV1.findMany’s where type to support OR and nested predicates, then propagate the pagination contract through the repository port, service, and InboxController.list while preserving descending createdAt ordering.services/api/src/features/iae/api/inbox.controller.ts-33-49 (1)
33-49: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftMap result codes to HTTP status codes.
createreturns the service result union directly. A rejected result such as{ accepted: false, code: 'IDEMPOTENCY_CONFLICT' }is serialized with HTTP 200. A client that branches on the status code treats the conflict as a success.The repository adapters also throw plain
Errorvalues forIAE_SCOPE_NARROWING_REQUIRED,IAE_IDEMPOTENCY_CONFLICT,IAE_REVISION_CONFLICT, andIAE_IMMUTABLE_INBOX_ITEM. Nest converts an unrecognizedErrorinto HTTP 500, so an authorization failure and a concurrency conflict both look like a server fault. The raw message is also the only signal available to the caller.Translate the codes at the boundary.
IDEMPOTENCY_CONFLICTandIAE_REVISION_CONFLICTmap to 409.IAE_SCOPE_NARROWING_REQUIREDmaps to 403.INBOX_NOT_FOUNDmaps to 404. Add@ApiResponseentries so the OpenAPI document lists the failure statuses.🐛 Boundary mapping sketch
`@Post`('inbox') `@ApiOperation`({ summary: 'Register a content-free artifact intake item' }) `@ApiBody`({ type: CreateInboxItemDto }) + `@ApiResponse`({ status: 201, description: 'Intake item registered' }) + `@ApiResponse`({ status: 409, description: 'Idempotency or revision conflict' }) + `@ApiResponse`({ status: 403, description: 'Tenant scope does not permit the write' }) async create( `@Req`() request: unknown, `@Headers`('idempotency-key') idempotencyKey: string | undefined, `@Body`() input: CreateInboxItemDto, - ): Promise<ArtifactIntakeServiceResultV1<unknown>> { + ): Promise<InboxItemV1> { const context = await this.requestContext.resolve(request); - return this.intake.create(context, { + const result = await this.intake.create(context, { inboxItemId: input.inboxItemId, tenantScope: context.tenantScope, idempotencyKey: idempotencyKey ?? input.idempotencyKey ?? context.idempotencyKey, artifactVersionId: input.artifactVersionId, createdAt: input.createdAt, - }); + }); + if (result.accepted) return result.value; + if (result.code === 'IDEMPOTENCY_CONFLICT') throw new ConflictException(result.code); + throw new BadRequestException(result.code); }A shared exception filter that maps the
IAE_*error identifiers keeps the thrown adapter errors consistent with this mapping.🤖 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/iae/api/inbox.controller.ts` around lines 33 - 49, Update the create method in the inbox controller to translate rejected service results and adapter errors into the required HTTP statuses: IDEMPOTENCY_CONFLICT and IAE_REVISION_CONFLICT to 409, IAE_SCOPE_NARROWING_REQUIRED to 403, and INBOX_NOT_FOUND to 404. Add a shared exception filter for the IAE_* error identifiers so plain adapter Errors are mapped consistently, then add `@ApiResponse` entries documenting these failure responses.services/api/test/features/iae/artifact-intake.service.test.ts-69-102 (1)
69-102: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winAdd coverage for the quarantine path
The test uses
scanState: 'CLEAN'and assertsROUTED, but its title and[IAE-010]tag claim malicious-content quarantine. Add a case withscanState: 'MALICIOUS'that assertsQUARANTINED, or remove the quarantine claim and tag.🤖 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/iae/artifact-intake.service.test.ts` around lines 69 - 102, Extend the admission test around ArtifactIntakeService.admit to cover malicious content: add an admission using scanState 'MALICIOUS' and assert the resulting item state is 'QUARANTINED'. Keep the existing clean-content assertion for 'ROUTED' and ensure the test title and IAE-010 tag accurately reflect the added quarantine coverage.services/api/src/features/iae/api/inbox-item.dto.ts-18-21 (1)
18-21: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winAdd
@IsOptional()toidempotencyKey. The globalValidationPipedoes not skip undefined properties, so omitting this field fails@MinLength(1). Use@ApiPropertyOptionalfor the OpenAPI schema.🤖 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/iae/api/inbox-item.dto.ts` around lines 18 - 21, Add `@IsOptional`() to idempotencyKey in the DTO so omitted values bypass `@MinLength` validation, and replace `@ApiProperty` with `@ApiPropertyOptional` while preserving the existing length constraints and optional API behavior.services/api/src/features/iae/application/artifact-intake.service.ts-60-71 (1)
60-71: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winReject mismatched artifact versions before admission.
Compare
artifact.versionIdwithitem.artifactVersionIdbefore callingfinalizeArtifactAdmissionV1. Otherwise, evidence for one artifact version can transition another inbox item toROUTED. ReturnARTIFACT_MISMATCHand add a regression 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/src/features/iae/application/artifact-intake.service.ts` around lines 60 - 71, In the transaction flow around `finalizeArtifactAdmissionV1`, compare `artifact.versionId` with `item.artifactVersionId` immediately after the inbox item lookup and before admission; return a frozen `{ accepted: false, code: 'ARTIFACT_MISMATCH' }` result on mismatch. Preserve the existing not-found and admission paths, and add a regression test proving a mismatched artifact cannot transition the inbox item.services/api/src/features/iae/application/artifact-governance.service.ts-12-39 (1)
12-39: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winReturn derived-lineage conflicts as typed results.
registerLineagereturnsArtifactGovernanceServiceResultV1, but conflicting derived versions throwIAE_DERIVED_LINEAGE_CONFLICT.ArtifactGovernanceServiceErrorV1declares only unusedLINEAGE_NOT_FOUND. AddDERIVED_LINEAGE_CONFLICT, return it, removeLINEAGE_NOT_FOUND, and update the test that expects rejection.🤖 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/iae/application/artifact-governance.service.ts` around lines 12 - 39, Update ArtifactGovernanceServiceErrorV1 to replace LINEAGE_NOT_FOUND with DERIVED_LINEAGE_CONFLICT, and change registerLineage to return an accepted:false result with that code instead of throwing when an existing derived lineage differs from created.value. Update the corresponding test to assert the typed rejected result rather than a rejected promise.services/api/src/features/iae/application/evidence-grant.service.ts-129-141 (1)
129-141: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
revokeskips the authorization-epoch check.
issuerejects a staleauthorizationEpochon line 34.revokeperforms no epoch check. A caller holding a stale authorization can therefore still revoke an active grant. Apply the same epoch check thatissueandresolveuse.🤖 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/iae/application/evidence-grant.service.ts` around lines 129 - 141, Update revoke to validate the caller’s authorization epoch using the same check and behavior as issue and resolve before calling transaction.revoke. Preserve the existing identifier validation, grant-not-found handling, and successful revocation flow.services/api/src/features/iae/api/evidence-grant.controller.ts-36-51 (1)
36-51: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winMap rejected service results to HTTP error statuses.
Both handlers return the service result object directly. A rejected result carries
accepted: falseand acode, but the response status stays 201 forissueand 200 forrevoke. Clients then treatGRANT_NOT_FOUND,EPOCH_MISMATCH, andINVALID_IDENTIFIERas success. Translate the discriminated union intoNotFoundException,ForbiddenException, orBadRequestExceptionbefore you return.Also declare the return types instead of
unknown, so the OpenAPI document describes the response bodies.🤖 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/iae/api/evidence-grant.controller.ts` around lines 36 - 51, Update the issue and revoke handlers to inspect the service result’s discriminated `accepted` value before returning: map `GRANT_NOT_FOUND` to `NotFoundException`, `EPOCH_MISMATCH` to `ForbiddenException`, and `INVALID_IDENTIFIER` to `BadRequestException`, while returning accepted results unchanged. Replace both `Promise<unknown>` declarations with the appropriate explicit success-result types so OpenAPI documents the response bodies.services/api/src/features/iae/adapter/in-memory-evidence-grant-repository.adapter.ts-14-16 (1)
14-16: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winDuplicated
visiblepredicate widens read scope in both adapters. Both files define the same helper, which returns true when the record scope contains the caller scope. Each adapter'ssavemethod enforces the opposite, stricter rule:tenantScopeContainsV1(context.tenantScope, record.tenantScope). A workspace-scoped caller can therefore read records that it cannot write. The single root cause is one visibility predicate that does not match the write authorization rule.
services/api/src/features/iae/adapter/in-memory-evidence-grant-repository.adapter.ts#L14-L16: confirm that a narrower caller may read a broader-scoped grant; if not, drop thetenantScopeContainsV1(candidate, context)branch.services/api/src/features/iae/adapter/in-memory-artifact-lineage-repository.adapter.ts#L13-L15: apply the same decision, and move the shared predicate into one module so the two adapters cannot drift.🤖 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/iae/adapter/in-memory-evidence-grant-repository.adapter.ts` around lines 14 - 16, The duplicated visible predicates widen read access beyond the stricter save authorization rule. In services/api/src/features/iae/adapter/in-memory-evidence-grant-repository.adapter.ts lines 14-16 and services/api/src/features/iae/adapter/in-memory-artifact-lineage-repository.adapter.ts lines 13-15, confirm the intended behavior for narrower callers; unless broader records are explicitly readable, remove the tenantScopeContainsV1(candidate, context) branch so visibility matches save, and move the shared predicate into one module reused by both adapters.services/api/src/features/iae/application/derived-artifact.service.ts-69-118 (1)
69-118: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftSource validation and persistence run in separate transactions.
Two problems follow from the split:
- Lines 69-80 read the source versions in a transaction that completes before
validateDerivedArtifactVersionV1runs on line 83. A source version can change between the read and the write on line 104. The policy check then uses stale data, so aDATA_MODE_WIDENINGderivation can pass. Read the sources inside the same transaction that writes the derivative.- Lines 102-103 nest
lineageRepository.withTransactioninsideartifactRepository.withTransaction. These are two independent transactions for the Prisma adapters. If the outer transaction fails after the inner one commits, a lineage record survives without its artifact version and placement. The in-memory adapters do not expose this, because the outer body performs no work after the inner call returns.Persist the artifact records and the lineage record through one transactional boundary, or add a compensating write path.
🤖 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/iae/application/derived-artifact.service.ts` around lines 69 - 118, Restructure the derived-artifact persistence flow around one transactional boundary: move source loading and validateDerivedArtifactVersionV1 into the same transaction that saves the version, placement, evidence, and lineage. Remove the nested independent artifactRepository.withTransaction and lineageRepository.withTransaction calls, ensuring all writes commit or roll back together; otherwise provide a compensating write path for partial commits.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 20b823a3-85ac-40c6-8a19-a4c396f8713d
📒 Files selected for processing (97)
apps/web/src/app/messages.tsapps/web/src/app/router.tsxapps/web/src/features/inbox/inbox-api.tsapps/web/src/features/inbox/inbox-page.tsxapps/web/src/styles.cssapps/web/test/inbox-page.test.tsxpackages/domain/package.jsonpackages/domain/src/artifact-governance/v1.tspackages/domain/src/artifact-intake/v1.tspackages/domain/src/artifact/v1.tspackages/domain/src/dataset-governance/v1.tspackages/domain/src/evidence-grant/v1.tspackages/domain/src/mapping/v1.tspackages/domain/src/reference-entity/v1.tspackages/domain/src/rule-set/v1.tspackages/domain/src/v1.tspackages/domain/test/artifact-governance-v1.test.mjspackages/domain/test/artifact-intake-v1.test.mjspackages/domain/test/artifact-v1.test.mjspackages/domain/test/built-public-api-smoke.mjspackages/domain/test/dataset-governance-v1.test.mjspackages/domain/test/evidence-grant-v1.test.mjspackages/domain/test/mapping-rule-set-v1.test.mjspackages/domain/test/public-api-v1.test.mjspackages/domain/test/reference-entity-v1.test.mjsservices/api/openapi/v1.jsonservices/api/prisma/migrations/20260802100000_iae_dsm_governance/migration.sqlservices/api/prisma/migrations/20260802110000_dsm_mappings_rules/migration.sqlservices/api/prisma/migrations/20260802120000_iae_evidence_grants/migration.sqlservices/api/prisma/migrations/20260802130000_iae_dsm_scope_hardening/migration.sqlservices/api/prisma/schema/dsm.prismaservices/api/prisma/schema/iae.prismaservices/api/scripts/generate-openapi.mjsservices/api/src/app.module.tsservices/api/src/bootstrap.tsservices/api/src/features/dsm/adapter/in-memory-governed-dataset-repository.adapter.tsservices/api/src/features/dsm/adapter/in-memory-mapping-repository.adapter.tsservices/api/src/features/dsm/adapter/in-memory-reference-entity-repository.adapter.tsservices/api/src/features/dsm/adapter/in-memory-rule-set-repository.adapter.tsservices/api/src/features/dsm/adapter/prisma-governed-dataset-repository.adapter.tsservices/api/src/features/dsm/adapter/prisma-mapping-repository.adapter.tsservices/api/src/features/dsm/adapter/prisma-reference-entity-repository.adapter.tsservices/api/src/features/dsm/adapter/prisma-rule-set-repository.adapter.tsservices/api/src/features/dsm/api/governed-dataset.controller.tsservices/api/src/features/dsm/api/governed-dataset.dto.tsservices/api/src/features/dsm/api/mapping.controller.tsservices/api/src/features/dsm/api/mapping.dto.tsservices/api/src/features/dsm/api/reference-entity.controller.tsservices/api/src/features/dsm/api/reference-entity.dto.tsservices/api/src/features/dsm/api/rule-set.controller.tsservices/api/src/features/dsm/application/governed-dataset-repository.port.tsservices/api/src/features/dsm/application/governed-dataset.service.tsservices/api/src/features/dsm/application/mapping-repository.port.tsservices/api/src/features/dsm/application/mapping.service.tsservices/api/src/features/dsm/application/reference-entity-repository.port.tsservices/api/src/features/dsm/application/reference-entity.service.tsservices/api/src/features/dsm/application/rule-set-repository.port.tsservices/api/src/features/dsm/application/rule-set.service.tsservices/api/src/features/dsm/dsm.module.tsservices/api/src/features/iae/adapter/in-memory-artifact-intake-repository.adapter.tsservices/api/src/features/iae/adapter/in-memory-artifact-lineage-repository.adapter.tsservices/api/src/features/iae/adapter/in-memory-evidence-grant-repository.adapter.tsservices/api/src/features/iae/adapter/prisma-artifact-intake-repository.adapter.tsservices/api/src/features/iae/adapter/prisma-artifact-repository.adapter.tsservices/api/src/features/iae/api/evidence-grant.controller.tsservices/api/src/features/iae/api/evidence-grant.dto.tsservices/api/src/features/iae/api/inbox-item.dto.tsservices/api/src/features/iae/api/inbox.controller.tsservices/api/src/features/iae/application/artifact-governance.service.tsservices/api/src/features/iae/application/artifact-intake-repository.port.tsservices/api/src/features/iae/application/artifact-intake.service.tsservices/api/src/features/iae/application/artifact-lineage-repository.port.tsservices/api/src/features/iae/application/derived-artifact.service.tsservices/api/src/features/iae/application/evidence-grant-repository.port.tsservices/api/src/features/iae/application/evidence-grant.service.tsservices/api/src/features/iae/iae.module.tsservices/api/src/platform/http/request-tenant-context.port.tsservices/api/test/features/dsm/governed-dataset.service.test.tsservices/api/test/features/dsm/mapping.service.test.tsservices/api/test/features/dsm/prisma-governed-dataset-repository.test.tsservices/api/test/features/dsm/prisma-mapping-repository.test.tsservices/api/test/features/dsm/prisma-reference-entity-repository.test.tsservices/api/test/features/dsm/prisma-rule-set-repository.test.tsservices/api/test/features/dsm/reference-entity.service.test.tsservices/api/test/features/dsm/rule-set.service.test.tsservices/api/test/features/iae/artifact-governance.service.test.tsservices/api/test/features/iae/artifact-intake.service.test.tsservices/api/test/features/iae/derived-artifact.service.test.tsservices/api/test/features/iae/evidence-grant.service.test.tsservices/api/test/features/iae/inbox.controller.test.tsservices/api/test/features/iae/prisma-artifact-intake-repository.test.tsservices/api/test/features/iae/prisma-artifact-repository.test.tsservices/api/test/openapi-drift.test.tsservices/api/test/openapi.test.tsservices/api/test/prisma-foundation.test.mjsservices/engine/src/databreeze_engine/processors/dataset_profile.pyservices/engine/tests/test_dataset_profile.py
| UPDATE "dsm"."reference_entity_resolutions" AS resolutions | ||
| SET | ||
| "scope_type" = versions."scope_type", | ||
| "organization_id" = versions."organization_id", | ||
| "workspace_id" = versions."workspace_id", | ||
| "project_id" = versions."project_id" | ||
| FROM "dsm"."reference_entity_versions" AS versions | ||
| WHERE resolutions."source_entity_id" = versions."entity_id" | ||
| AND resolutions."organization_id" IS NULL; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🔴 Critical | ⚡ Quick win
This backfill joins against a versioned table and picks an arbitrary row.
dsm.reference_entity_versions holds many rows per entity_id. Its only uniqueness is ("entity_id", "id"). UPDATE ... FROM with a join that matches multiple source rows applies exactly one of them, and PostgreSQL does not define which one. Each resolution row therefore receives a nondeterministic tenant scope. Two runs of the same migration on the same data can produce different organization_id values.
Compare the first backfill at Line 11. That one joins on versions."id", which is the primary key, so it matches at most one row and is deterministic.
Pick the source version explicitly. The example below uses the earliest version per entity.
🐛 Proposed deterministic backfill
UPDATE "dsm"."reference_entity_resolutions" AS resolutions
SET
"scope_type" = versions."scope_type",
"organization_id" = versions."organization_id",
"workspace_id" = versions."workspace_id",
"project_id" = versions."project_id"
-FROM "dsm"."reference_entity_versions" AS versions
-WHERE resolutions."source_entity_id" = versions."entity_id"
- AND resolutions."organization_id" IS NULL;
+FROM (
+ SELECT DISTINCT ON ("entity_id")
+ "entity_id", "scope_type", "organization_id", "workspace_id", "project_id"
+ FROM "dsm"."reference_entity_versions"
+ ORDER BY "entity_id", "created_at", "id"
+) AS versions
+WHERE resolutions."source_entity_id" = versions."entity_id"
+ AND resolutions."organization_id" IS NULL;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| UPDATE "dsm"."reference_entity_resolutions" AS resolutions | |
| SET | |
| "scope_type" = versions."scope_type", | |
| "organization_id" = versions."organization_id", | |
| "workspace_id" = versions."workspace_id", | |
| "project_id" = versions."project_id" | |
| FROM "dsm"."reference_entity_versions" AS versions | |
| WHERE resolutions."source_entity_id" = versions."entity_id" | |
| AND resolutions."organization_id" IS NULL; | |
| UPDATE "dsm"."reference_entity_resolutions" AS resolutions | |
| SET | |
| "scope_type" = versions."scope_type", | |
| "organization_id" = versions."organization_id", | |
| "workspace_id" = versions."workspace_id", | |
| "project_id" = versions."project_id" | |
| FROM ( | |
| SELECT DISTINCT ON ("entity_id") | |
| "entity_id", "scope_type", "organization_id", "workspace_id", "project_id" | |
| FROM "dsm"."reference_entity_versions" | |
| ORDER BY "entity_id", "created_at", "id" | |
| ) AS versions | |
| WHERE resolutions."source_entity_id" = versions."entity_id" | |
| AND resolutions."organization_id" IS NULL; |
🤖 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/prisma/migrations/20260802130000_iae_dsm_scope_hardening/migration.sql`
around lines 26 - 34, Update the backfill UPDATE joining
reference_entity_resolutions to reference_entity_versions so it selects exactly
one deterministic source version per entity_id, using the earliest version
consistently (for example, via an ordered per-entity selection). Preserve the
existing scope fields, NULL organization_id filter, and resolution-to-entity
matching.
| public async revoke(context: IamTenantContextV1, grantId: StableIdentifierV1): Promise<void> { | ||
| const grant = await this.find(context, grantId); | ||
| if (!grant) throw new Error('IAE_GRANT_NOT_FOUND'); | ||
| this.revoked.add(grantId); | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win
Authorize revoke with the narrowing rule, not the read rule.
revoke mutates grant state but authorizes only through find, which uses the bidirectional visible predicate. A caller with a narrower scope can revoke a grant that was issued at a broader scope. save rejects that same caller. Apply the tenantScopeContainsV1(context.tenantScope, grant.tenantScope) check before you record the revocation.
🔒 Proposed fix
public async revoke(context: IamTenantContextV1, grantId: StableIdentifierV1): Promise<void> {
const grant = await this.find(context, grantId);
if (!grant) throw new Error('IAE_GRANT_NOT_FOUND');
+ if (!tenantScopeContainsV1(context.tenantScope, grant.tenantScope))
+ throw new Error('IAE_SCOPE_NARROWING_REQUIRED');
this.revoked.add(grantId);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public async revoke(context: IamTenantContextV1, grantId: StableIdentifierV1): Promise<void> { | |
| const grant = await this.find(context, grantId); | |
| if (!grant) throw new Error('IAE_GRANT_NOT_FOUND'); | |
| this.revoked.add(grantId); | |
| } | |
| public async revoke(context: IamTenantContextV1, grantId: StableIdentifierV1): Promise<void> { | |
| const grant = await this.find(context, grantId); | |
| if (!grant) throw new Error('IAE_GRANT_NOT_FOUND'); | |
| if (!tenantScopeContainsV1(context.tenantScope, grant.tenantScope)) | |
| throw new Error('IAE_SCOPE_NARROWING_REQUIRED'); | |
| this.revoked.add(grantId); | |
| } |
🤖 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/iae/adapter/in-memory-evidence-grant-repository.adapter.ts`
around lines 46 - 50, Update revoke to enforce the narrowing authorization rule
after find returns the grant: validate
tenantScopeContainsV1(context.tenantScope, grant.tenantScope) before adding
grantId to revoked, and reject unauthorized scopes consistently with save while
preserving the existing not-found behavior.
| public async save(context: IamTenantContextV1, item: InboxItemV1): Promise<void> { | ||
| if (!tenantScopeContainsV1(context.tenantScope, item.tenantScope)) { | ||
| throw new Error('IAE_SCOPE_NARROWING_REQUIRED'); | ||
| } | ||
| const existing = await this.client.inboxItem.findUnique({ where: { id: item.inboxItemId } }); | ||
| if (existing !== null) { | ||
| if (JSON.stringify(rowToDomain(existing)) !== JSON.stringify(item)) { | ||
| throw new Error('IAE_IMMUTABLE_INBOX_ITEM'); | ||
| } | ||
| return; | ||
| } | ||
| await this.client.inboxItem.create({ data: domainToCreate(item) }); | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🔴 Critical | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Confirm admit() persists a transitioned item and that no Prisma-backed admit test exists.
set -euo pipefail
echo "=== admit implementation ==="
ast-grep run --pattern 'async admit($$$) { $$$ }' --lang typescript services/api/src || true
rg -nP -C 10 'transitionInboxItemV1' --type=ts services/api/src
echo "=== tests exercising the Prisma intake adapter ==="
fd -t f 'prisma-artifact-intake-repository.test.ts' services/api/test --exec cat -n {}
echo "=== inboxItem model definition ==="
rg -nP -C 20 'model\s+InboxItem' services/api/prismaRepository: DatabreezeService/databreeze-platform
Length of output: 14390
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Prisma adapter ==="
cat -n services/api/src/features/iae/adapter/prisma-artifact-intake-repository.adapter.ts | sed -n '1,240p'
echo "=== In-memory adapter and repository port ==="
rg -n -P -C 18 'class InMemoryArtifactIntakeRepositoryAdapter|expectedRevision|interface ArtifactIntakeRepositoryPortV1|interface ArtifactIntakeDatabaseClientV1|withTransaction' services/api/src/features/iae
echo "=== Prisma adapter usages and transaction wiring ==="
rg -n -P -C 12 'PrismaArtifactIntakeRepositoryAdapter|withTransaction\(|new PrismaArtifactIntake' services/api/src services/api/test
echo "=== Relevant domain transition and context definitions ==="
rg -n -P -C 20 'function transitionInboxItemV1|transitionInboxItemV1|type IamTenantContextV1|expectedRevision' packages servicesRepository: DatabreezeService/databreeze-platform
Length of output: 50394
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Prisma intake adapter ==="
cat -n services/api/src/features/iae/adapter/prisma-artifact-intake-repository.adapter.ts
echo "=== Intake source files ==="
fd -t f . services/api/src/features/iae | sort
echo "=== In-memory save implementation ==="
rg -l 'class InMemoryArtifactIntakeRepositoryAdapter|IAE_REVISION_CONFLICT' services/api/src services/api/test | while read -r f; do
echo "--- $f"
rg -n -C 25 'class InMemoryArtifactIntakeRepositoryAdapter|IAE_REVISION_CONFLICT|async save|save\(' "$f"
done
echo "=== Intake port and context definitions ==="
rg -l 'ArtifactIntakeRepositoryPortV1|IamTenantContextV1' services/api/src packages | while read -r f; do
echo "--- $f"
rg -n -C 12 'ArtifactIntakeRepositoryPortV1|IamTenantContextV1|expectedRevision' "$f"
doneRepository: DatabreezeService/databreeze-platform
Length of output: 50394
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Prisma adapter ==="
cat -n services/api/src/features/iae/adapter/prisma-artifact-intake-repository.adapter.ts
echo "=== Intake adapter files ==="
fd -t f -i '.*artifact.*intake.*(adapter|port).*' services packages | sort
echo "=== Intake save and revision handling ==="
rg -n -C 35 'IAE_REVISION_CONFLICT|class InMemory.*ArtifactIntake|transitionInboxItemV1|expectedRevision' \
services/api/src/features/iae services/api/test packages 2>/dev/null
echo "=== Tenant context ==="
fd -t f 'tenant-context.ts' services packages | xargs -r rg -n -C 20 'expectedRevision|IamTenantContextV1'
echo "=== Transition definition ==="
rg -l 'transitionInboxItemV1' packages services | while read -r f; do
echo "--- $f"
rg -n -C 35 'transitionInboxItemV1' "$f"
doneRepository: DatabreezeService/databreeze-platform
Length of output: 50393
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Admission tests ==="
cat -n services/api/test/features/iae/artifact-intake.service.test.ts | sed -n '45,180p'
echo "=== Intake port ==="
cat -n services/api/src/features/iae/application/artifact-intake-repository.port.ts
echo "=== Tenant context ==="
cat -n services/api/src/features/iam/application/tenant-context.ts | sed -n '1,180p'
echo "=== Transition definition locations ==="
rg -l 'transitionInboxItemV1' . --glob '!services/api/src/features/iae/application/artifact-intake.service.ts' --glob '!services/api/test/**' | sort
echo "=== Transition implementation ==="
for f in $(rg -l 'transitionInboxItemV1' . --glob '!services/api/src/features/iae/application/artifact-intake.service.ts' --glob '!services/api/test/**'); do
echo "--- $f"
rg -n -C 30 'transitionInboxItemV1' "$f"
doneRepository: DatabreezeService/databreeze-platform
Length of output: 13598
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import json
from pathlib import Path
adapter = Path("services/api/src/features/iae/adapter/prisma-artifact-intake-repository.adapter.ts").read_text()
schema = Path("services/api/prisma/schema/iae.prisma").read_text()
existing = {
"schemaVersion": 1,
"inboxItemId": "item-1",
"tenantScope": {"scopeType": "workspace", "organizationId": "org-1", "workspaceId": "ws-1"},
"idempotencyKey": "key-1",
"artifactVersionId": "artifact-1",
"state": "NEW",
"createdAt": "2026-01-01T00:00:00.000Z",
"revision": 1,
}
transitioned = {**existing, "state": "ROUTED", "revision": 2}
assert json.dumps(existing, separators=(",", ":")) != json.dumps(
transitioned, separators=(",", ":")
)
assert "IAE_IMMUTABLE_INBOX_ITEM" in adapter
assert "updateMany" not in adapter
assert "revision" in schema
assert "@@unique([organizationId, workspaceId, projectId, idempotencyKey]" in schema
print("transitioned item differs from persisted item: yes")
print("Prisma save has no updateMany/update path: yes")
print("InboxItem has a composite idempotency uniqueness constraint: yes")
PYRepository: DatabreezeService/databreeze-platform
Length of output: 338
Add an atomic optimistic-concurrency update to save.
ArtifactIntakeService.admit changes an existing item to ROUTED or QUARANTINED and increments revision. This method compares the full item and throws IAE_IMMUTABLE_INBOX_ITEM, so Prisma-backed admission cannot persist valid transitions. It also ignores context.expectedRevision.
Use a conditional update that preserves identity fields and enforces the expected revision. Make creation atomic, or map unique-constraint races to a domain idempotency error instead of exposing Prisma P2002.
🤖 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/iae/adapter/prisma-artifact-intake-repository.adapter.ts`
around lines 155 - 167, The save method currently rejects valid status and
revision transitions and ignores context.expectedRevision. Update
PrismaArtifactIntakeRepositoryAdapter.save to conditionally update existing
records using the expected revision while preserving immutable identity fields,
return successfully for the intended idempotent state, and report a concurrency
mismatch through the domain error; ensure creation is atomic and translate
unique-constraint races into the repository’s domain idempotency error rather
than exposing Prisma P2002.
| const version = await this.client.artifactVersion.findUnique({ | ||
| where: { id: placement.artifactVersionId }, | ||
| }); | ||
| if (version === null) throw new Error('IAE_VERSION_NOT_FOUND'); | ||
| if (!tenantScopeContainsV1(context.tenantScope, placement.tenantScope)) | ||
| throw new Error('IAE_SCOPE_NARROWING_REQUIRED'); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win
Child writes never verify the parent version scope. Both write paths resolve the parent artifact version by id and then check only the child record's own tenantScope against the caller. A caller in one workspace can therefore attach a child record to a version owned by another workspace, and the IAE_VERSION_NOT_FOUND branch discloses whether a version id exists outside the caller's scope. The read paths listPlacements and listEvidence already apply visible to the version row; the write paths must apply the same rule.
services/api/src/features/iae/adapter/prisma-artifact-repository.adapter.ts#L235-L240: after thefindUniquecall, reject whenvisible(context.tenantScope, version)is false, and use the same error as the missing-version branch so the two cases are indistinguishable to the caller.services/api/src/features/iae/adapter/prisma-artifact-repository.adapter.ts#L276-L281: apply the samevisible(context.tenantScope, versionRow)check before theevidenceReference.createcall.
📍 Affects 1 file
services/api/src/features/iae/adapter/prisma-artifact-repository.adapter.ts#L235-L240(this comment)services/api/src/features/iae/adapter/prisma-artifact-repository.adapter.ts#L276-L281
🤖 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/iae/adapter/prisma-artifact-repository.adapter.ts`
around lines 235 - 240, Child write paths do not validate the parent artifact
version’s tenant visibility. In
services/api/src/features/iae/adapter/prisma-artifact-repository.adapter.ts
lines 235-240, update the version lookup in the placement write path to reject
when visible(context.tenantScope, version) is false, using IAE_VERSION_NOT_FOUND
like the null case; at lines 276-281, apply the same
visible(context.tenantScope, versionRow) check before evidenceReference.create
so out-of-scope and missing versions are indistinguishable.
|
Follow-up: GitHub's Python engine check found two Ruff E501 violations in the new deterministic profiler fixture. Fixed in 3021c7d and merged no-ff as 151fbae; local pytest (89), Ruff check, and Ruff format now pass. Local mypy is blocked by this Windows Application Control policy, so the hosted mypy check remains the authoritative verification. CodeRabbit was triggered once via @coderabbitai review at 05:13 UTC and remained pending beyond its 10-minute window; no second review was triggered per repository policy. |
|
Tip For best results, initiate chat on the files or code changes.
The Ruff E501 fix is included in ✏️ Learnings added
You are interacting with an AI system. |
1 participant
Summary
Validation
Delivery
This branch is 46 independently reversible commits ahead of dev and follows the feature-branch/no-ff integration policy. It is ready for the single CodeRabbit review cycle.
Summary by CodeRabbit