promote: foundation and artifact safeguards - #37
Merged
Conversation
fix: close promotion review and foundation gates
Contributor
Author
|
PR #37 CodeRabbit disposition: review run d034f311-c043-4139-8372-ed69b774a83f completed once. All seven inline findings were valid and fixed in focused commits; the disposition is recorded in docs/operations/coderabbit-pr-37-disposition.md. The general walkthrough/docstring coverage warning was not adopted because it is not an enforced repository gate. Scoped and root verification pass (AWS tests, API tests, repo:check, repo:build). No second CodeRabbit run will be triggered. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
services/api/openapi/v1.json (1)
7710-7710: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winMake
maxByteSizean integer in the OpenAPI contract.
AdmitArtifactDto.maxByteSizeremainstype: "number"at Line 7713. The runtime DTO uses@IsInt()for this field. Clients can submit a documented-valid fractional value that the API rejects.Change
maxByteSizetotype: "integer". Add an OpenAPI regression assertion for both byte-size fields.Proposed fix
"actualByteSize": { "type": "integer", "minimum": 0 }, "detectedMediaType": { "type": "string" }, "scanState": { "type": "string", "enum": ["PENDING", "CLEAN", "MALICIOUS", "FAILED"] }, - "maxByteSize": { "type": "number", "minimum": 0 }, + "maxByteSize": { "type": "integer", "minimum": 0 },🤖 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/openapi/v1.json` at line 7710, Update the OpenAPI schema for AdmitArtifactDto.maxByteSize to use type "integer", matching its runtime `@IsInt`() validation, and add a regression assertion covering both maxByteSize and actualByteSize as integer fields.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@services/api/src/features/iae/adapter/prisma-artifact-lineage-repository.adapter.ts`:
- Around line 99-107: The isUniqueConstraintViolation helper currently treats
every Prisma P2002 error as a derived-version conflict. Inspect P2002’s
meta.target to distinguish the id constraint from derivedArtifactVersionId,
route id conflicts through the existing immutable-lineage comparison, and map
only derivedArtifactVersionId conflicts to IAE_DERIVED_LINEAGE_CONFLICT; add a
regression test covering a same-ID, different-payload race with meta.target in
the Prisma error fixture.
---
Outside diff comments:
In `@services/api/openapi/v1.json`:
- Line 7710: Update the OpenAPI schema for AdmitArtifactDto.maxByteSize to use
type "integer", matching its runtime `@IsInt`() validation, and add a regression
assertion covering both maxByteSize and actualByteSize as integer fields.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 6fd78011-f34d-414f-a9a8-2a696e772375
📒 Files selected for processing (12)
docs/operations/coderabbit-pr-37-disposition.mdinfrastructure/aws/README.mdservices/api/openapi/v1.jsonservices/api/src/features/iae/adapter/prisma-artifact-lineage-repository.adapter.tsservices/api/src/features/iae/api/artifact-retention.dto.tsservices/api/src/features/iae/api/inbox-item.dto.tsservices/api/src/features/sa/api/spreadsheet-audit.dto.tsservices/api/test/features/iae/prisma-artifact-lineage-repository.test.tsservices/api/test/openapi.test.tstools/repo-cli/src/check-aws-infrastructure.mjstools/repo-cli/src/validate-aws-opentofu.mjstools/repo-cli/test/aws-infrastructure.test.mjs
🚧 Files skipped from review as they are similar to previous changes (8)
- tools/repo-cli/src/validate-aws-opentofu.mjs
- tools/repo-cli/src/check-aws-infrastructure.mjs
- services/api/src/features/iae/api/inbox-item.dto.ts
- tools/repo-cli/test/aws-infrastructure.test.mjs
- services/api/test/features/iae/prisma-artifact-lineage-repository.test.ts
- services/api/src/features/iae/api/artifact-retention.dto.ts
- infrastructure/aws/README.md
- services/api/src/features/sa/api/spreadsheet-audit.dto.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
services/api/test/features/iae/artifact-upload.service.test.ts (1)
156-163: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAssert transfer compensation.
The test checks the expiration error. It does not check that the service aborts the transfer issued before revalidation. Use
TrackingStorageAdapterand assert oneabortcall.Proposed test update
+ const storage = new TrackingStorageAdapter(); const service = new ArtifactUploadService( new RevalidatingUploadRepository(created.value, expired.value), - new InMemoryArtifactUploadStorageAdapter(), + storage, ); assert.deepEqual(await service.issuePartTransfer(context, created.value.sessionId, 1), { accepted: false, code: 'UPLOAD_SESSION_EXPIRED', }); + assert.equal(storage.abortCalls, 1);🤖 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-upload.service.test.ts` around lines 156 - 163, Update the test around ArtifactUploadService.issuePartTransfer to use TrackingStorageAdapter instead of InMemoryArtifactUploadStorageAdapter, retain the existing expired-session response assertion, and additionally verify the tracking adapter recorded exactly one abort call for the transfer issued before revalidation.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@services/api/test/features/dsm/prisma-dataset-version-repository.test.ts`:
- Around line 99-113: Split this test into two rejection cases: one using the
persisted organizationId and workspaceId with only rowCount changed to -1, and
another using out-of-scope tenant identifiers without combining it with content
tampering. Ensure the rowCount-only case reaches the persisted-domain comparison
in PrismaDatasetVersionRepositoryAdapter.save.
---
Nitpick comments:
In `@services/api/test/features/iae/artifact-upload.service.test.ts`:
- Around line 156-163: Update the test around
ArtifactUploadService.issuePartTransfer to use TrackingStorageAdapter instead of
InMemoryArtifactUploadStorageAdapter, retain the existing expired-session
response assertion, and additionally verify the tracking adapter recorded
exactly one abort call for the transfer issued before revalidation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 9172f529-f74d-4fa1-9c59-84588bb3c138
📒 Files selected for processing (15)
apps/android/app/src/test/java/com/databreeze/android/TelemetryContractTest.ktdocs/operations/coderabbit-pr-37-disposition.mdservices/api/src/features/dsm/adapter/prisma-dataset-profile-repository.adapter.tsservices/api/src/features/dsm/adapter/prisma-dataset-quality-repository.adapter.tsservices/api/src/features/dsm/adapter/prisma-dataset-version-repository.adapter.tsservices/api/src/features/dsm/api/dataset-quality.dto.tsservices/api/src/features/iae/adapter/prisma-artifact-export-repository.adapter.tsservices/api/src/features/iae/adapter/prisma-artifact-lineage-repository.adapter.tsservices/api/src/features/iae/application/artifact-upload.service.tsservices/api/src/platform/prisma-error.tsservices/api/test/features/dsm/dataset-quality.controller.test.tsservices/api/test/features/dsm/prisma-dataset-profile-repository.test.tsservices/api/test/features/dsm/prisma-dataset-quality-repository.test.tsservices/api/test/features/dsm/prisma-dataset-version-repository.test.tsservices/api/test/features/iae/artifact-upload.service.test.ts
🚧 Files skipped from review as they are similar to previous changes (10)
- services/api/test/features/dsm/dataset-quality.controller.test.ts
- services/api/src/features/dsm/adapter/prisma-dataset-profile-repository.adapter.ts
- apps/android/app/src/test/java/com/databreeze/android/TelemetryContractTest.kt
- services/api/src/features/dsm/adapter/prisma-dataset-quality-repository.adapter.ts
- services/api/src/features/iae/adapter/prisma-artifact-lineage-repository.adapter.ts
- services/api/test/features/dsm/prisma-dataset-quality-repository.test.ts
- services/api/test/features/dsm/prisma-dataset-profile-repository.test.ts
- services/api/src/features/dsm/api/dataset-quality.dto.ts
- services/api/src/features/dsm/adapter/prisma-dataset-version-repository.adapter.ts
- services/api/src/features/iae/adapter/prisma-artifact-export-repository.adapter.ts
Comment on lines
+99
to
+113
| const persisted = rows[0]; | ||
| if (!persisted) throw new Error('fixture version was not persisted'); | ||
| await assert.rejects( | ||
| new PrismaDatasetVersionRepositoryAdapter( | ||
| client([ | ||
| { | ||
| ...persisted, | ||
| organizationId: '00000000-0000-4000-8000-000000000821', | ||
| workspaceId: '00000000-0000-4000-8000-000000000822', | ||
| rowCount: -1, | ||
| }, | ||
| ]), | ||
| ).save(tenantContext, created.value), | ||
| /DSM_IMMUTABLE_DATASET_VERSION/u, | ||
| ); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Separate tenant visibility and content tampering cases.
This test changes organizationId, workspaceId, and rowCount together. In services/api/src/features/dsm/adapter/prisma-dataset-version-repository.adapter.ts, Lines [126-145], save checks tenant visibility before comparing the persisted domain value. The changed tenant identifiers can trigger rejection before the rowCount mismatch is evaluated.
Add one case with unchanged tenant identifiers and only rowCount: -1. Keep a separate case for out-of-scope tenant identifiers.
🤖 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/dsm/prisma-dataset-version-repository.test.ts`
around lines 99 - 113, Split this test into two rejection cases: one using the
persisted organizationId and workspaceId with only rowCount changed to -1, and
another using out-of-scope tenant identifiers without combining it with content
tampering. Ensure the rowCount-only case reaches the persisted-domain comparison
in PrismaDatasetVersionRepositoryAdapter.save.
This was referenced Aug 3, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Promotion packet\n\nPromotes the first 58 commits from dev after the reviewed PR #35 slice.\n\n- foundation/local/AWS validation and telemetry safeguards\n- artifact, dataset, spreadsheet-audit, and engine contract hardening\n- IAM/IAE/DSM/SA regression coverage and OpenAPI updates\n- 101 changed files; this packet remains under the 79-commit and 260-file promotion limits\n\n## Verification\n\n- source branch is an ancestor of origin/dev at 8eccfa4\n- git diff --check passes\n- repository gates passed on the merged dev foundation before slicing\n\nThis is a main-bound promotion packet. CodeRabbit should perform its single full review here; do not rerun it after disposition.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation