fix: resolve promotion PR 31 review findings

docs/operations/coderabbit-pr-31-disposition.md

@@ -0,0 +1,50 @@
+# CodeRabbit PR 31 Disposition
+
+Date: 2026-08-03
+Promotion PR: [#31](https://github.com/DatabreezeService/databreeze-platform/pull/31)
+Automatic review ID: `4842552845`
+Reviewed range: `8695eed4bd5b988af9f4bea17e724ef5e1ac101d..688896af0af45281f8d9f379837d95abed04ac6c`
+
+CodeRabbit ran once automatically on the promotion PR. No manual review or rerun was requested. All 32 code findings were reproduced against the current `dev` state: 29 were accepted and fixed with regression coverage, and 3 were rejected after checking the later invariants and public result types.
+
+| ID | Finding | Disposition | Evidence |
+|---|---|---|---|
+| I-01 | Request input could replace the repository-loaded artifact during admission. | Accepted and fixed. The trusted artifact is applied last and a runtime-key injection regression test proves the stored version remains authoritative. | `68e0e4a` |
+| I-02 | XLSX XML members were fully decompressed before the size check. | Accepted and fixed. XML members now use a bounded `ZipExtFile` read and tests reject use of unbounded `ZipFile.read`. | `069c0cd` |
+| O-01 | Prisma intake and export fixtures accepted duplicate primary keys. | Accepted and fixed. Both fixtures now emulate Prisma `P2002` behavior. | `718b406` |
+| M-01 | Sparse quality `stateCounts` could raise `KeyError`. | Accepted and fixed with zero defaults and a sparse-profile regression test. | `5ed2cb4` |
+| M-02 | Spreadsheet `blockedReasons` accepted duplicates. | Accepted and fixed with `ArrayUnique`. | `96553d0` |
+| M-03 | A direct in-memory spreadsheet-audit save could be discarded by transaction rollback. | Accepted and fixed. Public saves use the transaction queue and callbacks use unwrapped helpers. | `6d67783` |
+| M-04 | Spreadsheet-audit `createdAt` accepted non-UTC timestamps. | Accepted and fixed with strict ISO validation and an uppercase-`Z` timestamp pattern. | `8b31681` |
+| M-05 | Several request arrays lacked matching runtime and OpenAPI bounds. | Accepted and fixed for version IDs, fields, mapping steps, rules, artifact inputs, evidence IDs, and findings. | `3bfe600` |
+| M-06 | The intake transition test did not verify the persisted revision. | Accepted and fixed. | `fd508f9` |
+| M-07 | Inbox content-leak assertions were case-sensitive. | Accepted and fixed. | `812e0c5` |
+| M-08 | Readiness 503 responses documented the wrong media type. | Accepted and fixed as `application/problem+json`, with a generated-contract assertion. | `42ff542` |
+| M-09 | Expired upload transfer requests were reported as generic storage unavailability. | Accepted and fixed with `UPLOAD_SESSION_EXPIRED`. | `f4af924` |
+| M-10 | Export processor-version text was validated before normalization and trimming. | Accepted and fixed; empty normalized text is rejected and valid trimmed text is retained. | `e5c4976` |
+| M-11 | Aggregate public API smoke coverage omitted retention and export schema versions. | Accepted and fixed. | `533e7b7` |
+| M-12 | The dataset-profile negative test allegedly mixed a sampling error with its count error. | Rejected. `samplingMethod` is required for both completeness modes; the fixture removes only the sample seed when switching to `COMPLETE`, so the first negative case already isolates `INVALID_COUNT`. Clearing `samplingMethod` would create the ambiguity the comment sought to remove. | `packages/domain/test/dataset-profile-v1.test.mjs` |
+| M-13 | The spreadsheet value-free test inspected the manifest root rather than the finding. | Accepted and fixed. | `eec8df5` |
+| M-14 | Premature upload expiration returned `EXPIRED`. | Accepted and fixed as `INVALID_TIMESTAMP`. | `6173abf` |
+| M-15 | Spreadsheet finding parser errors collapsed into `INVALID_COUNT`. | Accepted and fixed. Coordinate, kind, severity, identifier, and hash errors now retain their structural codes. | `3f769e2` |
+| M-16 | The upload completion test read `.value` without proving acceptance. | Accepted and fixed. | `adb45ef` |
+| M-17 | Premature protected-document expiration returned `EXPIRED`. | Accepted and fixed as `INVALID_STATE`. | `d477368` |
+| M-18 | Dataset profiles allowed `rowCountScanned` above `resourceLimits.maxRows`. | Accepted and fixed. | `afb3fdc` |
+| M-19 | Spreadsheet `maxRow` stopped below the XLSX row limit. | Accepted and fixed across domain validation, DTO validation, and generated OpenAPI at 1,048,576. | `3311f2a` |
+| M-20 | Inbox mutation context contained an unreachable conditional branch. | Accepted and simplified after the existing undefined guard. | `4a4c781` |
+| M-21 | Prisma export saves lacked visibility-safe collision handling, transaction-wrapped direct saves, and create-race translation. | Accepted and fixed with tenant-safe checks and stable immutable-manifest errors. | `34495dd` |
+| M-22 | Artifact-lineage lookup should use `findMany` to select a visible row. | Rejected against current `dev`. Later commits `68e69df` and `6431c9a` enforce one globally unique lineage per derived version; the unique lookup then checks tenant visibility. `findMany` would weaken that invariant and conceal duplicate persisted state. | `services/api/src/features/iae/adapter/prisma-artifact-lineage-repository.adapter.ts` |
+| M-23 | Retention and content-placement service-only error unions omitted domain result codes. | Rejected. `ArtifactRetentionServiceResultV1` already includes `ArtifactRetentionResultV1`, and `ContentPlacementServiceResultV1` already includes `ArtifactResultV1`; both public unions therefore expose the cited codes without duplicating them in their service-only error aliases. | Service result type definitions |
+| M-24 | The default request-tenant-context adapter produced a generic 500. | Accepted and fixed. The shared problem error now maps the unconfigured provider to retryable `AUTHENTICATION_UNAVAILABLE`/503. | `5c0b1d4` |
+| M-25 | Artifact admission accepted fractional byte sizes at the DTO boundary. | Accepted and fixed with integer runtime validation and OpenAPI type. | `b6603eb` |
+| M-26 | Artifact-export controllers returned failed service envelopes with HTTP 200. | Accepted and fixed. Invalid requests map to 400 problems and missing resources to 404 problems. | `c59b5b0` |
+| M-27 | Retention and inbox date-time DTOs accepted date-only or offset values. | Accepted and fixed with strict ISO/UTC validation while preserving nullable inbox `dueAt`. | `ea3c4ed` |
+| M-28 | DSM immutable repositories leaked Prisma create races. | Accepted and fixed for dataset profiles, quality results, and dataset versions by translating `P2002` into their stable immutable error codes. | `bc9e266` |
+| M-29 | Dataset quality safe values accepted objects and arrays despite the scalar OpenAPI contract. | Accepted and fixed with a finite scalar validator and an object-injection regression test. | `e634d65` |
+
+## Release handling
+
+- Fixes are applied through a dedicated PR to `dev`; CodeRabbit is not invoked on that PR.
+- After the fix PR merges, the two critical inline discussions receive the fixing commit references and the promotion PR receives a link to this disposition.
+- PR #31 remains a historical promotion slice. It receives no second CodeRabbit run and is merged only after the repair PR and required checks pass.
+- The generic docstring-coverage warning was not treated as a code finding: it did not identify a changed runtime defect, and bulk comments would add noise without improving the reviewed behavior.

packages/domain/src/artifact-export/v1.ts

@@ -64,11 +64,10 @@ function timestamp(input: unknown): StrictUtcTimestampV1 | undefined {
 }
 
 function text(input: unknown): string | undefined {
-  return typeof input === 'string' &&
-    input.length > 0 &&
-    input.length <= 128 &&
-    !/\p{Cc}/u.test(input)
-    ? input.normalize('NFC').trim()
+  if (typeof input !== 'string') return undefined;
+  const normalized = input.normalize('NFC').trim();
+  return normalized.length > 0 && normalized.length <= 128 && !/\p{Cc}/u.test(normalized)
+    ? normalized
     : undefined;
 }
 

packages/domain/src/artifact-upload/v1.ts

@@ -246,7 +246,8 @@ export function expireArtifactUploadSessionV1(
   const timestampValue = timestamp(now);
   if (!timestampValue) return rejected('INVALID_TIMESTAMP');
   if (session.state !== 'OPEN') return rejected('INVALID_STATE');
-  if (Date.parse(timestampValue) < Date.parse(session.expiresAt)) return rejected('EXPIRED');
+  if (Date.parse(timestampValue) < Date.parse(session.expiresAt))
+    return rejected('INVALID_TIMESTAMP');
   return accepted(
     Object.freeze({ ...session, state: 'EXPIRED' as const, revision: session.revision + 1 }),
   );

packages/domain/src/dataset-profile/v1.ts

@@ -151,6 +151,7 @@ export function createDatasetProfileV1(input: {
   const maxBytes = limit(limitRecord['maxBytes'], 1024 * 1024 * 1024 * 1024);
   const maxDurationMs = limit(limitRecord['maxDurationMs'], 86_400_000);
   if (!maxRows || !maxBytes || !maxDurationMs) return rejected('INVALID_LIMITS');
+  if (rowCountScanned > maxRows) return rejected('INVALID_COUNT');
   if (!profileFingerprint) return rejected('INVALID_HASH');
   if (!createdAt) return rejected('INVALID_TIMESTAMP');
   return accepted(

packages/domain/src/protected-document/v1.ts

@@ -196,7 +196,7 @@ export function expireProtectedDocumentUnlockRequestV1(
   const timestampValue = timestamp(now);
   if (!timestampValue) return rejected('INVALID_TIMESTAMP');
   if (request.state !== 'REQUESTED') return rejected('INVALID_STATE');
-  if (Date.parse(timestampValue) < Date.parse(request.expiresAt)) return rejected('EXPIRED');
+  if (Date.parse(timestampValue) < Date.parse(request.expiresAt)) return rejected('INVALID_STATE');
   return accepted(
     Object.freeze({ ...request, state: 'EXPIRED' as const, revision: request.revision + 1 }),
   );

packages/domain/src/spreadsheet-audit/v1.ts

@@ -119,31 +119,37 @@ function sheet(input: unknown): SpreadsheetAuditSheetV1 | undefined {
     formulaCount === undefined
   )
     return undefined;
-  if (maxRow > 1_000_000 || maxColumn > 16_384 || formulaCount > 1_000_000) return undefined;
+  if (maxRow > 1_048_576 || maxColumn > 16_384 || formulaCount > 1_000_000) return undefined;
   return Object.freeze({ sheetId, name, maxRow, maxColumn, formulaCount });
 }
 
-function finding(input: unknown): SpreadsheetAuditFindingV1 | undefined {
-  if (typeof input !== 'object' || input === null || Array.isArray(input)) return undefined;
+function finding(input: unknown): SpreadsheetAuditResultValidationV1<SpreadsheetAuditFindingV1> {
+  if (typeof input !== 'object' || input === null || Array.isArray(input))
+    return rejected('INVALID_IDENTIFIER');
   const record = input as Record<string, unknown>;
   const findingId = identifier(record['findingId']);
   const sheetId = identifier(record['sheetId']);
   const address = text(record['address'], 16);
   const kind = record['kind'];
   const severity = record['severity'];
   const formulaFingerprint = hash(record['formulaFingerprint']);
-  if (!findingId || !sheetId || !address || !/^[A-Z]{1,3}[1-9][0-9]*$/u.test(address.toUpperCase()))
-    return undefined;
-  if (kind !== 'FORMULA_FAMILY_OUTLIER' && kind !== 'FORMULA_GAP') return undefined;
-  if (severity !== 'INFO' && severity !== 'WARNING' && severity !== 'ERROR') return undefined;
-  if (!formulaFingerprint) return undefined;
+  if (!findingId || !sheetId) return rejected('INVALID_IDENTIFIER');
+  if (!address || !/^[A-Z]{1,3}[1-9][0-9]*$/u.test(address.toUpperCase()))
+    return rejected('INVALID_COORDINATE');
+  if (kind !== 'FORMULA_FAMILY_OUTLIER' && kind !== 'FORMULA_GAP') return rejected('INVALID_KIND');
+  if (severity !== 'INFO' && severity !== 'WARNING' && severity !== 'ERROR')
+    return rejected('INVALID_SEVERITY');
+  if (!formulaFingerprint) return rejected('INVALID_HASH');
   return Object.freeze({
-    findingId,
-    sheetId,
-    address: address.toUpperCase(),
-    kind: kind as SpreadsheetAuditFindingKindV1,
-    severity: severity as SpreadsheetAuditSeverityV1,
-    formulaFingerprint,
+    accepted: true,
+    value: Object.freeze({
+      findingId,
+      sheetId,
+      address: address.toUpperCase(),
+      kind: kind as SpreadsheetAuditFindingKindV1,
+      severity: severity as SpreadsheetAuditSeverityV1,
+      formulaFingerprint,
+    }),
   });
 }
 
@@ -181,10 +187,12 @@ export function createSpreadsheetAuditResultV1(input: {
     return rejected('DUPLICATE_SHEET');
   if (!Array.isArray(input.findings) || input.findings.length > 10_000)
     return rejected('INVALID_COUNT');
-  const findings = input.findings.map(finding);
-  if (findings.some((candidate): candidate is undefined => candidate === undefined))
-    return rejected('INVALID_COUNT');
-  const validFindings = findings as SpreadsheetAuditFindingV1[];
+  const validFindings: SpreadsheetAuditFindingV1[] = [];
+  for (const candidate of input.findings) {
+    const parsed = finding(candidate);
+    if (!parsed.accepted) return parsed;
+    validFindings.push(parsed.value);
+  }
   if (new Set(validFindings.map((candidate) => candidate.findingId)).size !== validFindings.length)
     return rejected('DUPLICATE_IDENTIFIER');
   const sheetsById = new Map(validSheets.map((candidate) => [candidate.sheetId, candidate]));

packages/domain/test/artifact-export-v1.test.mjs

@@ -56,3 +56,33 @@ void test('[IAE-018] export manifests preserve hashes, evidence references, and
     { accepted: false, code: 'DUPLICATE_IDENTIFIER' },
   );
 });
+
+void test('[IAE-018] processor versions are validated after normalization and trimming', () => {
+  const base = {
+    manifestId: '00000000-0000-4000-8000-000000000726',
+    tenantScope: scope,
+    entries: [
+      {
+        versionId: '00000000-0000-4000-8000-000000000727',
+        contentSha256: 'a'.repeat(64),
+        byteSize: 10,
+        evidenceIds: [],
+        processorVersions: ['   '],
+      },
+    ],
+    approvalState: 'PENDING',
+    createdAt: '2026-01-03T00:00:00.000Z',
+    canonicalHash: 'b'.repeat(64),
+  };
+  assert.deepEqual(createArtifactExportManifestV1(base), {
+    accepted: false,
+    code: 'INVALID_ENTRY',
+  });
+
+  const trimmed = createArtifactExportManifestV1({
+    ...base,
+    entries: [{ ...base.entries[0], processorVersions: [`${' '.repeat(128)}v1`] }],
+  });
+  assert.equal(trimmed.accepted, true);
+  if (trimmed.accepted) assert.deepEqual(trimmed.value.entries[0].processorVersions, ['v1']);
+});

packages/domain/test/artifact-upload-v1.test.mjs

@@ -4,6 +4,7 @@ import test from 'node:test';
 import {
   completeArtifactUploadSessionV1,
   createArtifactUploadSessionV1,
+  expireArtifactUploadSessionV1,
   recordArtifactUploadPartV1,
 } from '../dist/artifact-upload/v1.js';
 
@@ -52,11 +53,22 @@ void test('[IAE-014] upload sessions require every bounded part before completio
   });
   assert.equal(second.accepted, true);
   if (!second.accepted) return;
-  assert.equal(
-    completeArtifactUploadSessionV1(second.value, {
-      assembledSha256: base.expectedSha256,
-      expectedRevision: 3,
-    }).value.state,
-    'COMPLETED',
-  );
+  const completed = completeArtifactUploadSessionV1(second.value, {
+    assembledSha256: base.expectedSha256,
+    expectedRevision: 3,
+  });
+  assert.equal(completed.accepted, true);
+  if (!completed.accepted) return;
+  assert.equal(completed.value.state, 'COMPLETED');
+});
+
+void test('[IAE-014] upload sessions reject a premature expiration timestamp', () => {
+  const created = createArtifactUploadSessionV1(base);
+  assert.equal(created.accepted, true);
+  if (!created.accepted) return;
+
+  assert.deepEqual(expireArtifactUploadSessionV1(created.value, base.createdAt), {
+    accepted: false,
+    code: 'INVALID_TIMESTAMP',
+  });
 });

packages/domain/test/built-public-api-smoke.mjs

@@ -64,6 +64,8 @@ const [
 
 assert.equal(aggregate.PERMISSION_SCHEMA_VERSION_V1, 1);
 assert.equal(aggregate.AUTHORIZATION_SCHEMA_VERSION_V1, 1);
+assert.equal(aggregate.ARTIFACT_RETENTION_SCHEMA_VERSION_V1, 1);
+assert.equal(aggregate.ARTIFACT_EXPORT_SCHEMA_VERSION_V1, 1);
 assert.equal(permissions.PERMISSION_SCHEMA_VERSION_V1, 1);
 assert.equal(typeof tenantScope.parseTenantScopeV1, 'function');
 assert.equal(typeof authorization.createScopedAuthorizationEvaluatorV1, 'function');

packages/domain/test/dataset-profile-v1.test.mjs

@@ -56,4 +56,11 @@ void test('[DSM-011] complete profiles reject sample-only fields and impossible
     createDatasetProfileV1({ ...base, completeness: 'COMPLETE', samplingSeed: 'a'.repeat(64) }),
     { accepted: false, code: 'INVALID_SAMPLING' },
   );
+  assert.deepEqual(
+    createDatasetProfileV1({
+      ...base,
+      resourceLimits: { ...base.resourceLimits, maxRows: base.rowCountScanned - 1 },
+    }),
+    { accepted: false, code: 'INVALID_COUNT' },
+  );
 });

packages/domain/test/protected-document-v1.test.mjs

@@ -52,6 +52,10 @@ void test('[IAE-015] device-keychain requests require a device and expire withou
   const created = createProtectedDocumentUnlockRequestV1(base);
   assert.equal(created.accepted, true);
   if (!created.accepted) return;
+  assert.deepEqual(
+    expireProtectedDocumentUnlockRequestV1(created.value, '2026-08-02T00:29:59.999Z'),
+    { accepted: false, code: 'INVALID_STATE' },
+  );
   const expired = expireProtectedDocumentUnlockRequestV1(created.value, '2026-08-02T00:30:00.000Z');
   assert.equal(expired.accepted, true);
   if (expired.accepted) assert.equal(expired.value.state, 'EXPIRED');

packages/domain/test/spreadsheet-audit-v1.test.mjs

@@ -34,8 +34,8 @@ void test('[SA-001, SA-004] audit results retain exact value-free evidence coord
   assert.equal(result.accepted, true);
   if (!result.accepted) return;
   assert.equal(result.value.findings[0]?.address, 'C1');
-  assert.equal(Object.hasOwn(result.value, 'formula'), false);
-  assert.equal(Object.hasOwn(result.value, 'sourceValue'), false);
+  assert.equal(Object.hasOwn(result.value.findings[0], 'formula'), false);
+  assert.equal(Object.hasOwn(result.value.findings[0], 'sourceValue'), false);
 });
 
 void test('[SA-005] findings cannot reference an unknown sheet or duplicate IDs', () => {
@@ -70,4 +70,24 @@ void test('[SA-006] findings must stay inside the exact sheet geometry', () => {
     }),
     { accepted: false, code: 'INVALID_COORDINATE' },
   );
+  assert.equal(
+    createSpreadsheetAuditResultV1({
+      ...base,
+      sheets: [{ ...base.sheets[0], maxRow: 1_048_576 }],
+    }).accepted,
+    true,
+  );
+});
+
+void test('[SA-004] finding validation preserves structural error codes', () => {
+  for (const [finding, code] of [
+    [{ ...base.findings[0], address: 'not-a-cell' }, 'INVALID_COORDINATE'],
+    [{ ...base.findings[0], severity: 'CRITICAL' }, 'INVALID_SEVERITY'],
+    [{ ...base.findings[0], kind: 'UNKNOWN' }, 'INVALID_KIND'],
+  ]) {
+    assert.deepEqual(createSpreadsheetAuditResultV1({ ...base, findings: [finding] }), {
+      accepted: false,
+      code,
+    });
+  }
 });