From 1f7d7a5164b890536cd136b6280f02f323a4517a Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Thu, 16 Apr 2026 16:12:04 -0400 Subject: [PATCH 1/7] fix(pdf): render Tiptap tables in policy PDF exports (CS-221) Policy PDFs stripped table formatting because neither the server-side (trust portal / all-policies bundle) nor the client-side (policy page Download as PDF) renderer handled Tiptap table/tableRow/tableCell/ tableHeader nodes. The server renderer fell through to a default case that rendered cells as stacked paragraphs; the client renderer had no default and dropped tables entirely. Adds a renderTable helper to both renderers that draws a jsPDF grid with borders, a shaded header row, cell padding, text wrapping, and colspan support. Includes unit tests covering header + data rows, colspan, and empty tables. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../policy-pdf-renderer.service.spec.ts | 187 ++++++++++++++++++ .../policy-pdf-renderer.service.ts | 95 +++++++++ apps/app/src/lib/pdf-generator.ts | 97 ++++++++- 3 files changed, 377 insertions(+), 2 deletions(-) diff --git a/apps/api/src/trust-portal/policy-pdf-renderer.service.spec.ts b/apps/api/src/trust-portal/policy-pdf-renderer.service.spec.ts index 4dcc2e8186..accee6bdca 100644 --- a/apps/api/src/trust-portal/policy-pdf-renderer.service.spec.ts +++ b/apps/api/src/trust-portal/policy-pdf-renderer.service.spec.ts @@ -286,6 +286,193 @@ describe('PolicyPdfRendererService', () => { expect(result.length).toBeGreaterThan(0); }); + it('renders tables with header row and data cells (CS-221)', () => { + // Regression test for CS-221: tables in policy content rendered as + // stacked text in PDFs because there was no 'table' case in processContent. + const result = service.renderPoliciesPdfBuffer( + [ + { + name: 'Data Retention Policy', + content: { + type: 'doc', + content: [ + { + type: 'heading', + attrs: { level: 2 }, + content: [{ type: 'text', text: 'Appendix A' }], + }, + { + type: 'table', + content: [ + { + type: 'tableRow', + content: [ + { + type: 'tableHeader', + content: [ + { + type: 'paragraph', + content: [{ type: 'text', text: 'Data Type' }], + }, + ], + }, + { + type: 'tableHeader', + content: [ + { + type: 'paragraph', + content: [ + { type: 'text', text: 'Retention Period' }, + ], + }, + ], + }, + ], + }, + { + type: 'tableRow', + content: [ + { + type: 'tableCell', + content: [ + { + type: 'paragraph', + content: [{ type: 'text', text: 'User logs' }], + }, + ], + }, + { + type: 'tableCell', + content: [ + { + type: 'paragraph', + content: [{ type: 'text', text: '90 days' }], + }, + ], + }, + ], + }, + { + type: 'tableRow', + content: [ + { + type: 'tableCell', + content: [ + { + type: 'paragraph', + content: [ + { type: 'text', text: 'Billing records' }, + ], + }, + ], + }, + { + type: 'tableCell', + content: [ + { + type: 'paragraph', + content: [{ type: 'text', text: '7 years' }], + }, + ], + }, + ], + }, + ], + }, + ], + }, + }, + ], + 'Test Org', + ); + + expect(result).toBeInstanceOf(Buffer); + expect(result.length).toBeGreaterThan(0); + expect(result.subarray(0, 5).toString()).toBe('%PDF-'); + }); + + it('renders tables with cell colspan', () => { + const result = service.renderPoliciesPdfBuffer( + [ + { + name: 'Colspan Policy', + content: { + type: 'doc', + content: [ + { + type: 'table', + content: [ + { + type: 'tableRow', + content: [ + { + type: 'tableHeader', + attrs: { colspan: 2 }, + content: [ + { + type: 'paragraph', + content: [ + { type: 'text', text: 'Merged header' }, + ], + }, + ], + }, + ], + }, + { + type: 'tableRow', + content: [ + { + type: 'tableCell', + content: [ + { + type: 'paragraph', + content: [{ type: 'text', text: 'Left' }], + }, + ], + }, + { + type: 'tableCell', + content: [ + { + type: 'paragraph', + content: [{ type: 'text', text: 'Right' }], + }, + ], + }, + ], + }, + ], + }, + ], + }, + }, + ], + 'Test Org', + ); + + expect(result).toBeInstanceOf(Buffer); + expect(result.length).toBeGreaterThan(0); + }); + + it('handles empty tables without crashing', () => { + const result = service.renderPoliciesPdfBuffer( + [ + { + name: 'Empty Table Policy', + content: { + type: 'doc', + content: [{ type: 'table', content: [] }], + }, + }, + ], + 'Test Org', + ); + + expect(result).toBeInstanceOf(Buffer); + expect(result.length).toBeGreaterThan(0); + }); + it('applies custom primary color', () => { const result = service.renderPoliciesPdfBuffer( [ diff --git a/apps/api/src/trust-portal/policy-pdf-renderer.service.ts b/apps/api/src/trust-portal/policy-pdf-renderer.service.ts index d2bf5e43e2..655db8964d 100644 --- a/apps/api/src/trust-portal/policy-pdf-renderer.service.ts +++ b/apps/api/src/trust-portal/policy-pdf-renderer.service.ts @@ -401,6 +401,10 @@ export class PolicyPdfRendererService { config.yPosition += config.lineHeight; break; + case 'table': + this.renderTable(config, node); + break; + default: if (node.content) { this.processContent(config, node.content); @@ -409,6 +413,97 @@ export class PolicyPdfRendererService { } } + private renderTable(config: PDFConfig, tableNode: JSONContent): void { + const rows = tableNode.content; + if (!rows || rows.length === 0) return; + + const firstRow = rows[0]; + if (!firstRow.content || firstRow.content.length === 0) return; + + // Count columns (including colspans) from the first row + let columnCount = 0; + for (const cell of firstRow.content) { + columnCount += cell.attrs?.colspan ?? 1; + } + if (columnCount === 0) return; + + const colWidth = config.contentWidth / columnCount; + const cellPadding = 2; + const minCellHeight = config.lineHeight + cellPadding * 2; + + config.doc.setFontSize(config.defaultFontSize); + + for (const row of rows) { + if (row.type !== 'tableRow' || !row.content) continue; + + // Pre-compute wrapped lines per cell to determine row height + const cellsInRow: Array<{ + isHeader: boolean; + lines: string[]; + width: number; + }> = []; + + for (const cell of row.content) { + if (cell.type !== 'tableCell' && cell.type !== 'tableHeader') continue; + const isHeader = cell.type === 'tableHeader'; + const colspan = cell.attrs?.colspan ?? 1; + const width = colWidth * colspan; + const rawText = cell.content + ? this.extractTextFromContent(cell.content) + : ''; + const cleanText = this.cleanTextForPDF(rawText); + const lines = config.doc.splitTextToSize( + cleanText || ' ', + width - cellPadding * 2, + ) as string[]; + cellsInRow.push({ isHeader, lines, width }); + } + + if (cellsInRow.length === 0) continue; + + const rowHeight = Math.max( + minCellHeight, + ...cellsInRow.map( + (c) => c.lines.length * config.lineHeight + cellPadding * 2, + ), + ); + + this.checkPageBreak(config, rowHeight); + + const rowY = config.yPosition; + let xOffset = 0; + for (const cell of cellsInRow) { + const x = config.margin + xOffset; + + if (cell.isHeader) { + config.doc.setFillColor(240, 240, 240); + config.doc.rect(x, rowY, cell.width, rowHeight, 'F'); + } + + config.doc.setDrawColor(180, 180, 180); + config.doc.setLineWidth(0.2); + config.doc.rect(x, rowY, cell.width, rowHeight); + + config.doc.setFont('helvetica', cell.isHeader ? 'bold' : 'normal'); + config.doc.setTextColor(0, 0, 0); + cell.lines.forEach((line, li) => { + config.doc.text( + line, + x + cellPadding, + rowY + cellPadding + config.lineHeight * (li + 0.75), + ); + }); + + xOffset += cell.width; + } + + config.doc.setFont('helvetica', 'normal'); + config.yPosition = rowY + rowHeight; + } + + config.yPosition += config.lineHeight * 0.5; + } + renderPoliciesPdfBuffer( policies: PolicyForPDF[], organizationName?: string, diff --git a/apps/app/src/lib/pdf-generator.ts b/apps/app/src/lib/pdf-generator.ts index 00b629541c..a7c07c77ff 100644 --- a/apps/app/src/lib/pdf-generator.ts +++ b/apps/app/src/lib/pdf-generator.ts @@ -271,13 +271,13 @@ const processContent = (config: PDFConfig, content: JSONContent[], level: number if (listItem.type === 'listItem' && listItem.content) { const listText = extractTextFromContent(listItem.content); checkPageBreak(config); - + // Add number with consistent font config.doc.setFontSize(config.defaultFontSize); config.doc.setFont('helvetica', 'normal'); config.doc.setTextColor(0, 0, 0); // Ensure number is black config.doc.text(`${itemNumber}.`, config.margin + level * 10, config.yPosition); - + // Add indented text with proper font reset config.doc.setFontSize(config.defaultFontSize); config.doc.setFont('helvetica', 'normal'); @@ -295,10 +295,103 @@ const processContent = (config: PDFConfig, content: JSONContent[], level: number } } break; + + case 'table': + renderTable(config, item); + break; } } }; +// Render a Tiptap table node as a jsPDF grid with borders and header fill. +// NOTE: Keep in sync with apps/api/src/trust-portal/policy-pdf-renderer.service.ts renderTable +const renderTable = (config: PDFConfig, tableNode: JSONContent) => { + const rows = tableNode.content; + if (!rows || rows.length === 0) return; + + const firstRow = rows[0]; + if (!firstRow.content || firstRow.content.length === 0) return; + + let columnCount = 0; + for (const cell of firstRow.content) { + columnCount += cell.attrs?.colspan ?? 1; + } + if (columnCount === 0) return; + + const colWidth = config.contentWidth / columnCount; + const cellPadding = 2; + const minCellHeight = config.lineHeight + cellPadding * 2; + + config.doc.setFontSize(config.defaultFontSize); + + for (const row of rows) { + if (row.type !== 'tableRow' || !row.content) continue; + + const cellsInRow: Array<{ + isHeader: boolean; + lines: string[]; + width: number; + }> = []; + + for (const cell of row.content) { + if (cell.type !== 'tableCell' && cell.type !== 'tableHeader') continue; + const isHeader = cell.type === 'tableHeader'; + const colspan = cell.attrs?.colspan ?? 1; + const width = colWidth * colspan; + const rawText = cell.content ? extractTextFromContent(cell.content) : ''; + const cleanText = cleanTextForPDF(rawText); + const lines = config.doc.splitTextToSize( + cleanText || ' ', + width - cellPadding * 2, + ) as string[]; + cellsInRow.push({ isHeader, lines, width }); + } + + if (cellsInRow.length === 0) continue; + + const rowHeight = Math.max( + minCellHeight, + ...cellsInRow.map( + (c) => c.lines.length * config.lineHeight + cellPadding * 2, + ), + ); + + checkPageBreak(config, rowHeight); + + const rowY = config.yPosition; + let xOffset = 0; + for (const cell of cellsInRow) { + const x = config.margin + xOffset; + + if (cell.isHeader) { + config.doc.setFillColor(240, 240, 240); + config.doc.rect(x, rowY, cell.width, rowHeight, 'F'); + } + + config.doc.setDrawColor(180, 180, 180); + config.doc.setLineWidth(0.2); + config.doc.rect(x, rowY, cell.width, rowHeight); + + config.doc.setFont('helvetica', cell.isHeader ? 'bold' : 'normal'); + config.doc.setTextColor(0, 0, 0); + cell.lines.forEach((line: string, li: number) => { + config.doc.text( + line, + x + cellPadding, + rowY + cellPadding + config.lineHeight * (li + 0.75), + ); + }); + + xOffset += cell.width; + } + + config.doc.setFont('helvetica', 'normal'); + config.yPosition = rowY + rowHeight; + } + + config.yPosition += config.lineHeight * 0.5; +}; + // Function to add audit logs table const addAuditLogsTable = (config: PDFConfig, auditLogs: AuditLogWithRelations[], isCompact: boolean = false) => { checkPageBreak(config, config.lineHeight * 6); // Ensure we have space for at least the header From 53cf88a2c87d3088e7aa45a9c644baf65cfb684a Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Thu, 16 Apr 2026 16:15:50 -0400 Subject: [PATCH 2/7] fix(api): accept MIME types with parameters or whitespace (CS-217) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit File upload endpoints were rejecting valid Content-Type values like `application/pdf;charset=utf-8` or `application/pdf\n` with a 400 "Invalid MIME type format" error, because each DTO enforced the regex /^[a-zA-Z0-9\-]+\/[a-zA-Z0-9\-\+\.]+$/ with no trim or parameter strip. Extracts a shared `IsMimeTypeField` decorator (+ `normalizeMimeType` helper) that: - strips RFC 7231 media-type parameters (`;charset=...`) and whitespace - lowercases the value before validation and persistence - matches the RFC 6838 restricted-name-chars grammar Applied to all four upload DTOs: - attachments/upload-attachment.dto.ts - tasks/dto/upload-attachment.dto.ts - task-management/dto/upload-task-item-attachment.dto.ts - org-chart/dto/upload-org-chart.dto.ts Also removes dead `BLOCKED_MIME_TYPES` constants from two DTOs — the actual blocklist lives in `attachments.service.ts` and operates on the already-lowercased fileType. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../src/attachments/upload-attachment.dto.ts | 25 +--- .../src/org-chart/dto/upload-org-chart.dto.ts | 9 +- .../dto/upload-task-item-attachment.dto.ts | 9 +- .../src/tasks/dto/upload-attachment.dto.ts | 25 +--- .../api/src/utils/mime-type.validator.spec.ts | 126 ++++++++++++++++++ apps/api/src/utils/mime-type.validator.ts | 32 +++++ 6 files changed, 167 insertions(+), 59 deletions(-) create mode 100644 apps/api/src/utils/mime-type.validator.spec.ts create mode 100644 apps/api/src/utils/mime-type.validator.ts diff --git a/apps/api/src/attachments/upload-attachment.dto.ts b/apps/api/src/attachments/upload-attachment.dto.ts index 2981948c9d..0a2ad097ad 100644 --- a/apps/api/src/attachments/upload-attachment.dto.ts +++ b/apps/api/src/attachments/upload-attachment.dto.ts @@ -6,25 +6,8 @@ import { IsOptional, IsString, MaxLength, - Matches, } from 'class-validator'; - -// Block dangerous MIME types that could execute code -const BLOCKED_MIME_TYPES = [ - 'application/x-msdownload', // .exe - 'application/x-msdos-program', - 'application/x-executable', - 'application/x-sh', // Shell scripts - 'application/x-bat', // Batch files - 'text/x-sh', - 'text/x-python', - 'text/x-perl', - 'text/x-ruby', - 'application/x-httpd-php', // PHP files - 'application/x-javascript', // Executable JS (not JSON) - 'application/javascript', - 'text/javascript', -]; +import { IsMimeTypeField } from '../utils/mime-type.validator'; export class UploadAttachmentDto { @ApiProperty({ @@ -42,11 +25,7 @@ export class UploadAttachmentDto { description: 'MIME type of the file', example: 'application/pdf', }) - @IsString() - @IsNotEmpty() - @Matches(/^[a-zA-Z0-9\-]+\/[a-zA-Z0-9\-\+\.]+$/, { - message: 'Invalid MIME type format', - }) + @IsMimeTypeField() fileType: string; @ApiProperty({ diff --git a/apps/api/src/org-chart/dto/upload-org-chart.dto.ts b/apps/api/src/org-chart/dto/upload-org-chart.dto.ts index 857109bec9..4476026a68 100644 --- a/apps/api/src/org-chart/dto/upload-org-chart.dto.ts +++ b/apps/api/src/org-chart/dto/upload-org-chart.dto.ts @@ -1,5 +1,6 @@ -import { IsNotEmpty, IsString, Matches } from 'class-validator'; +import { IsNotEmpty, IsString } from 'class-validator'; import { ApiProperty } from '@nestjs/swagger'; +import { IsMimeTypeField } from '../../utils/mime-type.validator'; export class UploadOrgChartDto { @ApiProperty({ description: 'Original file name' }) @@ -8,11 +9,7 @@ export class UploadOrgChartDto { fileName: string; @ApiProperty({ description: 'MIME type of the file (e.g. image/png)' }) - @IsString() - @IsNotEmpty() - @Matches(/^[a-zA-Z0-9\-]+\/[a-zA-Z0-9\-\+\.]+$/, { - message: 'Invalid MIME type format', - }) + @IsMimeTypeField() fileType: string; @ApiProperty({ description: 'Base64-encoded file data' }) diff --git a/apps/api/src/task-management/dto/upload-task-item-attachment.dto.ts b/apps/api/src/task-management/dto/upload-task-item-attachment.dto.ts index 1b8c006074..24c892d4ba 100644 --- a/apps/api/src/task-management/dto/upload-task-item-attachment.dto.ts +++ b/apps/api/src/task-management/dto/upload-task-item-attachment.dto.ts @@ -3,13 +3,12 @@ import { Transform } from 'class-transformer'; import { IsBase64, IsNotEmpty, - IsOptional, IsString, MaxLength, - Matches, IsEnum, } from 'class-validator'; import { TaskItemEntityType } from '@db'; +import { IsMimeTypeField } from '../../utils/mime-type.validator'; export class UploadTaskItemAttachmentDto { @ApiProperty({ @@ -27,11 +26,7 @@ export class UploadTaskItemAttachmentDto { description: 'MIME type of the file', example: 'application/pdf', }) - @IsString() - @IsNotEmpty() - @Matches(/^[a-zA-Z0-9\-]+\/[a-zA-Z0-9\-\+\.]+$/, { - message: 'Invalid MIME type format', - }) + @IsMimeTypeField() fileType: string; @ApiProperty({ diff --git a/apps/api/src/tasks/dto/upload-attachment.dto.ts b/apps/api/src/tasks/dto/upload-attachment.dto.ts index c91dc176cc..feac58d064 100644 --- a/apps/api/src/tasks/dto/upload-attachment.dto.ts +++ b/apps/api/src/tasks/dto/upload-attachment.dto.ts @@ -6,25 +6,8 @@ import { IsOptional, IsString, MaxLength, - Matches, } from 'class-validator'; - -// Block dangerous MIME types that could execute code -const BLOCKED_MIME_TYPES = [ - 'application/x-msdownload', // .exe - 'application/x-msdos-program', - 'application/x-executable', - 'application/x-sh', // Shell scripts - 'application/x-bat', // Batch files - 'text/x-sh', - 'text/x-python', - 'text/x-perl', - 'text/x-ruby', - 'application/x-httpd-php', // PHP files - 'application/x-javascript', // Executable JS (not JSON) - 'application/javascript', - 'text/javascript', -]; +import { IsMimeTypeField } from '../../utils/mime-type.validator'; export class UploadAttachmentDto { @ApiProperty({ @@ -42,11 +25,7 @@ export class UploadAttachmentDto { description: 'MIME type of the file', example: 'application/pdf', }) - @IsString() - @IsNotEmpty() - @Matches(/^[a-zA-Z0-9\-]+\/[a-zA-Z0-9\-\+\.]+$/, { - message: 'Invalid MIME type format', - }) + @IsMimeTypeField() fileType: string; @ApiProperty({ diff --git a/apps/api/src/utils/mime-type.validator.spec.ts b/apps/api/src/utils/mime-type.validator.spec.ts new file mode 100644 index 0000000000..cc573fd9b9 --- /dev/null +++ b/apps/api/src/utils/mime-type.validator.spec.ts @@ -0,0 +1,126 @@ +import { plainToInstance } from 'class-transformer'; +import { validate } from 'class-validator'; +import { IsMimeTypeField, normalizeMimeType } from './mime-type.validator'; + +class TestDto { + @IsMimeTypeField() + fileType!: string; +} + +const expectValid = async (input: unknown, expectedNormalized: string) => { + const instance = plainToInstance(TestDto, { fileType: input }); + const errors = await validate(instance); + expect(errors).toEqual([]); + expect(instance.fileType).toBe(expectedNormalized); +}; + +const expectInvalid = async (input: unknown) => { + const instance = plainToInstance(TestDto, { fileType: input }); + const errors = await validate(instance); + expect(errors.length).toBeGreaterThan(0); +}; + +describe('normalizeMimeType', () => { + it('returns non-strings unchanged', () => { + expect(normalizeMimeType(undefined)).toBe(undefined); + expect(normalizeMimeType(null)).toBe(null); + expect(normalizeMimeType(123)).toBe(123); + }); + + it('strips parameters, whitespace, and lowercases', () => { + expect(normalizeMimeType('application/pdf')).toBe('application/pdf'); + expect(normalizeMimeType('application/pdf;charset=utf-8')).toBe( + 'application/pdf', + ); + expect(normalizeMimeType('application/pdf; charset=utf-8')).toBe( + 'application/pdf', + ); + expect(normalizeMimeType(' application/PDF ')).toBe('application/pdf'); + expect(normalizeMimeType('application/pdf\n')).toBe('application/pdf'); + }); +}); + +describe('IsMimeTypeField', () => { + describe('valid inputs (CS-217 regression)', () => { + it('accepts application/pdf', async () => { + await expectValid('application/pdf', 'application/pdf'); + }); + + it('accepts the xlsx vendor type', async () => { + await expectValid( + 'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet', + 'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet', + ); + }); + + it('accepts application/pdf with charset parameter', async () => { + await expectValid( + 'application/pdf;charset=utf-8', + 'application/pdf', + ); + }); + + it('accepts xlsx with charset parameter', async () => { + await expectValid( + 'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet;charset=utf-8', + 'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet', + ); + }); + + it('accepts trailing whitespace', async () => { + await expectValid('application/pdf ', 'application/pdf'); + }); + + it('accepts trailing newline', async () => { + await expectValid('application/pdf\n', 'application/pdf'); + }); + + it('accepts uppercase', async () => { + await expectValid('APPLICATION/PDF', 'application/pdf'); + }); + + it('accepts structured syntax suffix (e.g. +json)', async () => { + await expectValid( + 'application/vnd.api+json', + 'application/vnd.api+json', + ); + }); + + it('accepts image/svg+xml', async () => { + await expectValid('image/svg+xml', 'image/svg+xml'); + }); + }); + + describe('invalid inputs', () => { + it('rejects empty string', async () => { + await expectInvalid(''); + }); + + it('rejects missing slash', async () => { + await expectInvalid('applicationpdf'); + }); + + it('rejects leading slash', async () => { + await expectInvalid('/pdf'); + }); + + it('rejects trailing slash', async () => { + await expectInvalid('application/'); + }); + + it('rejects spaces inside type/subtype', async () => { + await expectInvalid('application /pdf'); + await expectInvalid('application/ pdf'); + }); + + it('rejects multiple slashes', async () => { + await expectInvalid('application/foo/bar'); + }); + + it('rejects non-string values', async () => { + await expectInvalid(123); + await expectInvalid(null); + await expectInvalid(undefined); + }); + }); +}); diff --git a/apps/api/src/utils/mime-type.validator.ts b/apps/api/src/utils/mime-type.validator.ts new file mode 100644 index 0000000000..eee0410e10 --- /dev/null +++ b/apps/api/src/utils/mime-type.validator.ts @@ -0,0 +1,32 @@ +import { applyDecorators } from '@nestjs/common'; +import { Transform } from 'class-transformer'; +import { IsNotEmpty, IsString, Matches } from 'class-validator'; + +// RFC 6838 restricted-name-chars set (alpha, digit, !, #, $, &, -, ^, _, ., +) +// First char must be alpha/digit. +const MIME_TYPE_REGEX = + /^[a-z0-9][a-z0-9!#$&^_.+-]*\/[a-z0-9][a-z0-9!#$&^_.+-]*$/i; + +/** + * Normalize a Content-Type / MIME value to its bare `type/subtype` form: + * strips optional `;parameters` (RFC 7231) and surrounding whitespace, + * and lowercases the result. + */ +export const normalizeMimeType = (value: unknown): unknown => { + if (typeof value !== 'string') return value; + const bare = value.split(';')[0]?.trim().toLowerCase() ?? ''; + return bare; +}; + +/** + * Decorator for DTO fields that accept a file MIME type. + * Normalizes the value (strip parameters, trim, lowercase) before validating + * it against the RFC 6838 restricted-name-chars grammar. + */ +export const IsMimeTypeField = (): PropertyDecorator => + applyDecorators( + IsString(), + IsNotEmpty(), + Transform(({ value }) => normalizeMimeType(value)), + Matches(MIME_TYPE_REGEX, { message: 'Invalid MIME type format' }), + ); From ecc3c0adbe1b07a348890ffb15da48fcc6df738f Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Thu, 16 Apr 2026 16:20:59 -0400 Subject: [PATCH 3/7] fix(integrations): clear stale task status after connection disconnect (CS-166) After a user disconnects an integration (e.g. GitHub), tasks that were marked 'failed' by that connection's check runs stayed stuck on 'failed' forever, because nothing re-evaluates the task's automation state on disconnect. In addition, the task detail "App Automations" panel kept showing historical failed runs from the disconnected connection, and the 12-hour task-schedule job would re-mark done-then-overdue tasks as failed using those stale runs. Three changes, all reading-side filters + one re-evaluation on disconnect: 1. ConnectionService.disconnectConnection and deleteConnection now call reevaluateFailedTasksAfterDisconnect, which re-derives each affected failed task's target status from its remaining non-disconnected automations. Uses a local mirror of the scheduler's getTargetStatus. 2. CheckRunRepository.findByTask excludes runs from disconnected connections so the UI no longer counts stale "failed" history. 3. task-schedule.ts includes the same connection-status filter on its integrationCheckRuns include. Check run rows are preserved for audit trail (matching the existing soft-delete design for IntegrationConnection itself). Co-Authored-By: Claude Opus 4.7 (1M context) --- .../repositories/check-run.repository.ts | 11 +- .../services/connection.service.spec.ts | 211 ++++++++++++++++++ .../services/connection.service.ts | 113 +++++++++- .../src/trigger/tasks/task/task-schedule.ts | 5 +- 4 files changed, 336 insertions(+), 4 deletions(-) create mode 100644 apps/api/src/integration-platform/services/connection.service.spec.ts diff --git a/apps/api/src/integration-platform/repositories/check-run.repository.ts b/apps/api/src/integration-platform/repositories/check-run.repository.ts index 81b955b13f..b5a535412c 100644 --- a/apps/api/src/integration-platform/repositories/check-run.repository.ts +++ b/apps/api/src/integration-platform/repositories/check-run.repository.ts @@ -124,11 +124,18 @@ export class CheckRunRepository { } /** - * Get check runs for a task + * Get check runs for a task. + * + * CS-166: excludes runs from disconnected connections so the task's UI + * panels don't render stale "failed" history after a user disconnects the + * integration. The rows remain in the DB for audit. */ async findByTask(taskId: string, limit = 10) { return db.integrationCheckRun.findMany({ - where: { taskId }, + where: { + taskId, + connection: { status: { not: 'disconnected' } }, + }, include: { results: true, connection: { diff --git a/apps/api/src/integration-platform/services/connection.service.spec.ts b/apps/api/src/integration-platform/services/connection.service.spec.ts new file mode 100644 index 0000000000..8a8872e7f2 --- /dev/null +++ b/apps/api/src/integration-platform/services/connection.service.spec.ts @@ -0,0 +1,211 @@ +import { Test, TestingModule } from '@nestjs/testing'; +import { ConnectionService } from './connection.service'; +import { ConnectionRepository } from '../repositories/connection.repository'; +import { ProviderRepository } from '../repositories/provider.repository'; +import { ConnectionAuthTeardownService } from './connection-auth-teardown.service'; + +jest.mock('@db', () => ({ + db: { + integrationCheckRun: { + findMany: jest.fn(), + }, + task: { + findMany: jest.fn(), + update: jest.fn(), + }, + }, +})); + +jest.mock('@trycompai/integration-platform', () => ({ + getManifest: jest.fn(), +})); + +import { db } from '@db'; + +const findRuns = (db.integrationCheckRun as unknown as { findMany: jest.Mock }) + .findMany; +const findTasks = (db.task as unknown as { findMany: jest.Mock }).findMany; +const updateTask = (db.task as unknown as { update: jest.Mock }).update; + +describe('ConnectionService', () => { + let service: ConnectionService; + + const mockConnectionRepository = { + update: jest.fn(), + }; + const mockProviderRepository = {}; + const mockTeardown = { + teardown: jest.fn(), + }; + + const CONNECTION_ID = 'icn_1'; + + beforeEach(async () => { + jest.clearAllMocks(); + + const module: TestingModule = await Test.createTestingModule({ + providers: [ + ConnectionService, + { provide: ConnectionRepository, useValue: mockConnectionRepository }, + { provide: ProviderRepository, useValue: mockProviderRepository }, + { provide: ConnectionAuthTeardownService, useValue: mockTeardown }, + ], + }).compile(); + + service = module.get(ConnectionService); + + mockConnectionRepository.update.mockResolvedValue({ + id: CONNECTION_ID, + status: 'disconnected', + }); + mockTeardown.teardown.mockResolvedValue(undefined); + }); + + describe('disconnectConnection (CS-166)', () => { + it('re-evaluates failed tasks to "todo" when the only automation source was the disconnected connection', async () => { + findRuns.mockResolvedValue([{ taskId: 'tsk_1' }]); + findTasks.mockResolvedValue([ + { + id: 'tsk_1', + evidenceAutomations: [], + integrationCheckRuns: [], // filtered query returns no active runs + }, + ]); + + await service.disconnectConnection(CONNECTION_ID); + + expect(findRuns).toHaveBeenCalledWith({ + where: { connectionId: CONNECTION_ID, taskId: { not: null } }, + select: { taskId: true }, + distinct: ['taskId'], + }); + expect(findTasks).toHaveBeenCalledWith( + expect.objectContaining({ + where: { id: { in: ['tsk_1'] }, status: 'failed' }, + }), + ); + expect(updateTask).toHaveBeenCalledWith({ + where: { id: 'tsk_1' }, + data: { status: 'todo' }, + }); + }); + + it('re-evaluates failed task to "done" when remaining active automations are passing', async () => { + findRuns.mockResolvedValue([{ taskId: 'tsk_2' }]); + findTasks.mockResolvedValue([ + { + id: 'tsk_2', + evidenceAutomations: [], + integrationCheckRuns: [ + { + checkId: 'other_check', + status: 'success', + createdAt: new Date('2026-04-01'), + }, + ], + }, + ]); + + await service.disconnectConnection(CONNECTION_ID); + + expect(updateTask).toHaveBeenCalledWith({ + where: { id: 'tsk_2' }, + data: { status: 'done' }, + }); + }); + + it('leaves the task at "failed" when another active automation is still failing', async () => { + findRuns.mockResolvedValue([{ taskId: 'tsk_3' }]); + findTasks.mockResolvedValue([ + { + id: 'tsk_3', + evidenceAutomations: [], + integrationCheckRuns: [ + { + checkId: 'other_check', + status: 'failed', + createdAt: new Date('2026-04-01'), + }, + ], + }, + ]); + + await service.disconnectConnection(CONNECTION_ID); + + expect(updateTask).not.toHaveBeenCalled(); + }); + + it('picks the latest run per checkId when multiple exist', async () => { + findRuns.mockResolvedValue([{ taskId: 'tsk_4' }]); + findTasks.mockResolvedValue([ + { + id: 'tsk_4', + evidenceAutomations: [], + // Mock comes pre-sorted desc by createdAt to match the service's + // orderBy. Latest run for check_a is success; check_b is only one + // run and it's success. + integrationCheckRuns: [ + { + checkId: 'check_a', + status: 'success', + createdAt: new Date('2026-04-05'), + }, + { + checkId: 'check_a', + status: 'failed', + createdAt: new Date('2026-04-01'), + }, + { + checkId: 'check_b', + status: 'success', + createdAt: new Date('2026-04-03'), + }, + ], + }, + ]); + + await service.disconnectConnection(CONNECTION_ID); + + expect(updateTask).toHaveBeenCalledWith({ + where: { id: 'tsk_4' }, + data: { status: 'done' }, + }); + }); + + it('does not touch a task that is not currently failed', async () => { + findRuns.mockResolvedValue([{ taskId: 'tsk_5' }]); + // findTasks filters by status: 'failed', so non-failed tasks are not returned + findTasks.mockResolvedValue([]); + + await service.disconnectConnection(CONNECTION_ID); + + expect(updateTask).not.toHaveBeenCalled(); + }); + + it('skips re-evaluation when no runs exist for the connection', async () => { + findRuns.mockResolvedValue([]); + + await service.disconnectConnection(CONNECTION_ID); + + expect(findTasks).not.toHaveBeenCalled(); + expect(updateTask).not.toHaveBeenCalled(); + }); + + it('handles evidenceAutomations — task with failing custom automation stays failed', async () => { + findRuns.mockResolvedValue([{ taskId: 'tsk_6' }]); + findTasks.mockResolvedValue([ + { + id: 'tsk_6', + evidenceAutomations: [ + { runs: [{ evaluationStatus: 'fail' }] }, + ], + integrationCheckRuns: [], + }, + ]); + + await service.disconnectConnection(CONNECTION_ID); + + expect(updateTask).not.toHaveBeenCalled(); + }); + }); +}); diff --git a/apps/api/src/integration-platform/services/connection.service.ts b/apps/api/src/integration-platform/services/connection.service.ts index b3a8d976f2..32ce8dd152 100644 --- a/apps/api/src/integration-platform/services/connection.service.ts +++ b/apps/api/src/integration-platform/services/connection.service.ts @@ -2,8 +2,10 @@ import { Injectable, NotFoundException, ConflictException, + Logger, } from '@nestjs/common'; import { getManifest } from '@trycompai/integration-platform'; +import { db } from '@db'; import { ConnectionRepository } from '../repositories/connection.repository'; import { ProviderRepository } from '../repositories/provider.repository'; import { ConnectionAuthTeardownService } from './connection-auth-teardown.service'; @@ -18,6 +20,8 @@ export interface CreateConnectionInput { @Injectable() export class ConnectionService { + private readonly logger = new Logger(ConnectionService.name); + constructor( private readonly connectionRepository: ConnectionRepository, private readonly providerRepository: ProviderRepository, @@ -135,11 +139,15 @@ export class ConnectionService { ): Promise { await this.connectionAuthTeardownService.teardown({ connectionId }); - return this.connectionRepository.update(connectionId, { + const connection = await this.connectionRepository.update(connectionId, { status: 'disconnected', errorMessage: null, activeCredentialVersionId: null, }); + + await this.reevaluateFailedTasksAfterDisconnect(connectionId); + + return connection; } async deleteConnection(connectionId: string): Promise { @@ -153,6 +161,109 @@ export class ConnectionService { activeCredentialVersionId: null, errorMessage: null, }); + + await this.reevaluateFailedTasksAfterDisconnect(connectionId); + } + + /** + * CS-166: After a connection is disconnected, tasks whose status was set to + * 'failed' by check runs from that connection can end up stuck — the + * historical runs remain in the DB but no new runs will clear them. + * + * We preserve the check run rows for audit (see deleteConnection doc above) + * and instead fix the UX by re-deriving each affected task's target status + * from its remaining, non-disconnected automation state. Only 'failed' tasks + * are considered; 'done' or 'todo' tasks are left alone. + */ + private async reevaluateFailedTasksAfterDisconnect( + connectionId: string, + ): Promise { + const runs = await db.integrationCheckRun.findMany({ + where: { connectionId, taskId: { not: null } }, + select: { taskId: true }, + distinct: ['taskId'], + }); + + const taskIds = runs + .map((r) => r.taskId) + .filter((id): id is string => id !== null); + + if (taskIds.length === 0) return; + + const tasks = await db.task.findMany({ + where: { id: { in: taskIds }, status: 'failed' }, + select: { + id: true, + evidenceAutomations: { + where: { isEnabled: true }, + select: { + runs: { + orderBy: { createdAt: 'desc' }, + take: 1, + select: { evaluationStatus: true }, + }, + }, + }, + integrationCheckRuns: { + where: { connection: { status: { not: 'disconnected' } } }, + orderBy: { createdAt: 'desc' }, + select: { checkId: true, status: true, createdAt: true }, + }, + }, + }); + + for (const task of tasks) { + const target = this.deriveTargetStatusForTask(task); + if (target === 'failed') continue; + await db.task.update({ + where: { id: task.id }, + data: { status: target }, + }); + this.logger.log( + `Task ${task.id} re-evaluated to '${target}' after connection ${connectionId} disconnect`, + ); + } + } + + /** + * Mirror of the scheduler's getTargetStatus (see apps/app/src/trigger/tasks/ + * task/task-schedule-helpers.ts), scoped to the fields we fetch above. + */ + private deriveTargetStatusForTask(task: { + evidenceAutomations: Array<{ + runs: Array<{ evaluationStatus: string | null }>; + }>; + integrationCheckRuns: Array<{ + checkId: string; + status: string; + createdAt: Date; + }>; + }): 'done' | 'todo' | 'failed' { + const hasCustom = task.evidenceAutomations.length > 0; + const customPassing = + hasCustom && + task.evidenceAutomations.every( + (a) => a.runs[0]?.evaluationStatus === 'pass', + ); + + const hasApp = task.integrationCheckRuns.length > 0; + let appPassing = false; + if (hasApp) { + const latestByCheckId = new Map(); + for (const run of task.integrationCheckRuns) { + const existing = latestByCheckId.get(run.checkId); + if (!existing) latestByCheckId.set(run.checkId, run); + } + appPassing = Array.from(latestByCheckId.values()).every( + (r) => r.status === 'success', + ); + } + + if (!hasCustom && !hasApp) return 'todo'; + + const allPassing = + (!hasCustom || customPassing) && (!hasApp || appPassing); + return allPassing ? 'done' : 'failed'; } async updateLastSync(connectionId: string): Promise { diff --git a/apps/app/src/trigger/tasks/task/task-schedule.ts b/apps/app/src/trigger/tasks/task/task-schedule.ts index a9f63ad810..872b3cbd28 100644 --- a/apps/app/src/trigger/tasks/task/task-schedule.ts +++ b/apps/app/src/trigger/tasks/task/task-schedule.ts @@ -69,8 +69,11 @@ export const taskSchedule = schedules.task({ }, }, }, - // Include App Automations (IntegrationCheckRun) - get all runs to group by checkId + // Include App Automations (IntegrationCheckRun) - get all runs to group by checkId. + // CS-166: exclude runs from disconnected connections — their historical + // "failed" state must not drive scheduled status changes. integrationCheckRuns: { + where: { connection: { status: { not: 'disconnected' } } }, orderBy: { createdAt: 'desc' }, select: { checkId: true, From 2548169967cff3480c1258f31b65785fa8c629ce Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Thu, 16 Apr 2026 16:38:01 -0400 Subject: [PATCH 4/7] fix(pdf): separate text from multi-paragraph table cells with newlines Follow-up to the CS-221 review: extractTextFromContent joins nested text nodes with no delimiter, so a cell containing two paragraphs (e.g. "Retention Period" and "30 days") rendered as the single concatenated string "Retention Period30 days". hardBreak nodes had the same problem. Adds an extractCellText helper to both renderers that joins top-level block children with \n and converts hardBreak to \n. splitTextToSize respects embedded newlines, so the cell wraps into separate visual rows. Regression test asserts the concatenated strings do NOT appear in the rendered PDF buffer. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../policy-pdf-renderer.service.spec.ts | 67 +++++++++++++++++++ .../policy-pdf-renderer.service.ts | 30 ++++++++- apps/app/src/lib/pdf-generator.ts | 21 +++++- 3 files changed, 114 insertions(+), 4 deletions(-) diff --git a/apps/api/src/trust-portal/policy-pdf-renderer.service.spec.ts b/apps/api/src/trust-portal/policy-pdf-renderer.service.spec.ts index accee6bdca..42c1d43486 100644 --- a/apps/api/src/trust-portal/policy-pdf-renderer.service.spec.ts +++ b/apps/api/src/trust-portal/policy-pdf-renderer.service.spec.ts @@ -455,6 +455,73 @@ describe('PolicyPdfRendererService', () => { expect(result.length).toBeGreaterThan(0); }); + it('separates text from multi-paragraph cells with newlines', () => { + // Regression test for the CS-221 review comment: cells with multiple + // block children (paragraphs, hardBreaks) used to be concatenated + // without a separator, so "Retention Period" + "30 days" rendered as + // "Retention Period30 days". extractCellText joins top-level blocks + // with \n so splitTextToSize wraps them correctly. + const result = service.renderPoliciesPdfBuffer( + [ + { + name: 'Multi-paragraph Cell Policy', + content: { + type: 'doc', + content: [ + { + type: 'table', + content: [ + { + type: 'tableRow', + content: [ + { + type: 'tableCell', + content: [ + { + type: 'paragraph', + content: [ + { type: 'text', text: 'Retention Period' }, + ], + }, + { + type: 'paragraph', + content: [{ type: 'text', text: '30 days' }], + }, + ], + }, + { + type: 'tableCell', + content: [ + { + type: 'paragraph', + content: [ + { type: 'text', text: 'Line one' }, + { type: 'hardBreak' }, + { type: 'text', text: 'Line two' }, + ], + }, + ], + }, + ], + }, + ], + }, + ], + }, + }, + ], + 'Test Org', + ); + + expect(result).toBeInstanceOf(Buffer); + expect(result.length).toBeGreaterThan(0); + + // The concatenated-without-separator strings must NOT appear in the PDF. + const pdfText = result.toString('latin1'); + expect(pdfText).not.toContain('Retention Period30 days'); + expect(pdfText).not.toContain('Line oneLine two'); + }); + it('handles empty tables without crashing', () => { const result = service.renderPoliciesPdfBuffer( [ diff --git a/apps/api/src/trust-portal/policy-pdf-renderer.service.ts b/apps/api/src/trust-portal/policy-pdf-renderer.service.ts index 655db8964d..d8e54da7fd 100644 --- a/apps/api/src/trust-portal/policy-pdf-renderer.service.ts +++ b/apps/api/src/trust-portal/policy-pdf-renderer.service.ts @@ -448,10 +448,10 @@ export class PolicyPdfRendererService { const isHeader = cell.type === 'tableHeader'; const colspan = cell.attrs?.colspan ?? 1; const width = colWidth * colspan; - const rawText = cell.content - ? this.extractTextFromContent(cell.content) - : ''; + const rawText = this.extractCellText(cell.content ?? []); const cleanText = this.cleanTextForPDF(rawText); + // splitTextToSize respects embedded \n, so multi-paragraph cells + // wrap into separate visual rows within the same cell. const lines = config.doc.splitTextToSize( cleanText || ' ', width - cellPadding * 2, @@ -504,6 +504,30 @@ export class PolicyPdfRendererService { config.yPosition += config.lineHeight * 0.5; } + /** + * Extract display text from a table cell's block-level content. + * + * Tiptap cells wrap their content in block nodes (one `paragraph` per line, + * plus the occasional `hardBreak`). Joining top-level blocks with '\n' + * preserves the visual separation a user sees in the editor so that + * splitTextToSize wraps each intended line separately — without it, two + * paragraphs like "Retention Period" and "30 days" would render as the + * single concatenated string "Retention Period30 days". + */ + private extractCellText(cellContent: JSONContent[]): string { + return cellContent + .map((block) => this.extractInlineText(block)) + .filter((s) => s.length > 0) + .join('\n'); + } + + private extractInlineText(node: JSONContent): string { + if (node.type === 'hardBreak') return '\n'; + if (node.text) return node.text; + if (!node.content) return ''; + return node.content.map((child) => this.extractInlineText(child)).join(''); + } + renderPoliciesPdfBuffer( policies: PolicyForPDF[], organizationName?: string, diff --git a/apps/app/src/lib/pdf-generator.ts b/apps/app/src/lib/pdf-generator.ts index a7c07c77ff..57da34641b 100644 --- a/apps/app/src/lib/pdf-generator.ts +++ b/apps/app/src/lib/pdf-generator.ts @@ -303,6 +303,23 @@ const processContent = (config: PDFConfig, content: JSONContent[], level: number } }; +// Extract display text from a table cell's block-level content. +// See renderTable-in-sync note below: joins top-level blocks with '\n' so +// splitTextToSize wraps multi-paragraph cells correctly instead of producing +// a concatenated "Retention Period30 days". +const extractInlineText = (node: JSONContent): string => { + if (node.type === 'hardBreak') return '\n'; + if (node.text) return node.text; + if (!node.content) return ''; + return node.content.map((child) => extractInlineText(child)).join(''); +}; + +const extractCellText = (cellContent: JSONContent[]): string => + cellContent + .map((block) => extractInlineText(block)) + .filter((s) => s.length > 0) + .join('\n'); + // Render a Tiptap table node as a jsPDF grid with borders and header fill. // NOTE: Keep in sync with apps/api/src/trust-portal/policy-pdf-renderer.service.ts renderTable const renderTable = (config: PDFConfig, tableNode: JSONContent) => { @@ -338,8 +355,10 @@ const renderTable = (config: PDFConfig, tableNode: JSONContent) => { const isHeader = cell.type === 'tableHeader'; const colspan = cell.attrs?.colspan ?? 1; const width = colWidth * colspan; - const rawText = cell.content ? extractTextFromContent(cell.content) : ''; + const rawText = extractCellText(cell.content ?? []); const cleanText = cleanTextForPDF(rawText); + // splitTextToSize respects embedded \n, so multi-paragraph cells + // wrap into separate visual rows within the same cell. const lines = config.doc.splitTextToSize( cleanText || ' ', width - cellPadding * 2, From b8cd6a46dec3a9e8572f803d0f6758d3d23304a1 Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Thu, 16 Apr 2026 16:43:33 -0400 Subject: [PATCH 5/7] =?UTF-8?q?test(pdf):=20harden=20table=20cell=20extrac?= =?UTF-8?q?tion=20=E2=80=94=20lists,=20long=20wrap,=20page=20breaks?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Expands extractCellText to treat bulletList / orderedList / listItem as line-break boundaries, not just top-level block children. Previously a cell whose only block was a bulletList with two items rendered as "AlphaBeta" because recursion concatenated listItem text with no delimiter. Now it renders as "Alpha\nBeta" and splitTextToSize wraps each item to its own visual line. Adds regression tests for: - bullet list items in a cell - cell text longer than the column width (wraps) - 50-row table that forces a mid-table page break - content following a table on the same page (yPosition advances) Co-Authored-By: Claude Opus 4.7 (1M context) --- .../policy-pdf-renderer.service.spec.ts | 241 ++++++++++++++++++ .../policy-pdf-renderer.service.ts | 41 ++- apps/app/src/lib/pdf-generator.ts | 28 +- 3 files changed, 295 insertions(+), 15 deletions(-) diff --git a/apps/api/src/trust-portal/policy-pdf-renderer.service.spec.ts b/apps/api/src/trust-portal/policy-pdf-renderer.service.spec.ts index 42c1d43486..9dad7320bc 100644 --- a/apps/api/src/trust-portal/policy-pdf-renderer.service.spec.ts +++ b/apps/api/src/trust-portal/policy-pdf-renderer.service.spec.ts @@ -522,6 +522,247 @@ describe('PolicyPdfRendererService', () => { expect(pdfText).not.toContain('Line oneLine two'); }); + it('separates bullet list items inside a cell with newlines', () => { + // A cell whose only block is a bulletList used to concatenate items + // (e.g. "AlphaBeta") because extractInlineText didn't recognize list + // containers as line-break boundaries. + const result = service.renderPoliciesPdfBuffer( + [ + { + name: 'List-in-Cell Policy', + content: { + type: 'doc', + content: [ + { + type: 'table', + content: [ + { + type: 'tableRow', + content: [ + { + type: 'tableCell', + content: [ + { + type: 'bulletList', + content: [ + { + type: 'listItem', + content: [ + { + type: 'paragraph', + content: [ + { type: 'text', text: 'Alpha' }, + ], + }, + ], + }, + { + type: 'listItem', + content: [ + { + type: 'paragraph', + content: [ + { type: 'text', text: 'Beta' }, + ], + }, + ], + }, + ], + }, + ], + }, + ], + }, + ], + }, + ], + }, + }, + ], + 'Test Org', + ); + + const pdfText = result.toString('latin1'); + expect(pdfText).not.toContain('AlphaBeta'); + }); + + it('renders very long cell text across wrapped lines', () => { + // Stress test: a single cell with text much longer than the column + // width. Must not throw, must produce a valid PDF, and must grow the + // row height (so lines don't overlap). + const longText = + 'This is a very long cell value that should wrap across multiple lines inside the cell. '.repeat( + 4, + ); + const result = service.renderPoliciesPdfBuffer( + [ + { + name: 'Long Text Policy', + content: { + type: 'doc', + content: [ + { + type: 'table', + content: [ + { + type: 'tableRow', + content: [ + { + type: 'tableCell', + content: [ + { + type: 'paragraph', + content: [{ type: 'text', text: longText }], + }, + ], + }, + { + type: 'tableCell', + content: [ + { + type: 'paragraph', + content: [{ type: 'text', text: 'short' }], + }, + ], + }, + ], + }, + ], + }, + ], + }, + }, + ], + 'Test Org', + ); + + expect(result).toBeInstanceOf(Buffer); + expect(result.length).toBeGreaterThan(0); + expect(result.subarray(0, 5).toString()).toBe('%PDF-'); + }); + + it('inserts a page break when a table row does not fit the current page', () => { + // 50 rows forces at least one page break mid-table. Must not throw. + const rows = Array.from({ length: 50 }, (_, i) => ({ + type: 'tableRow' as const, + content: [ + { + type: 'tableCell' as const, + content: [ + { + type: 'paragraph' as const, + content: [{ type: 'text' as const, text: `Row ${i + 1}` }], + }, + ], + }, + { + type: 'tableCell' as const, + content: [ + { + type: 'paragraph' as const, + content: [{ type: 'text' as const, text: `Value ${i + 1}` }], + }, + ], + }, + ], + })); + + const result = service.renderPoliciesPdfBuffer( + [ + { + name: 'Long Table Policy', + content: { + type: 'doc', + content: [ + { + type: 'table', + content: [ + { + type: 'tableRow', + content: [ + { + type: 'tableHeader', + content: [ + { + type: 'paragraph', + content: [{ type: 'text', text: 'Row' }], + }, + ], + }, + { + type: 'tableHeader', + content: [ + { + type: 'paragraph', + content: [{ type: 'text', text: 'Value' }], + }, + ], + }, + ], + }, + ...rows, + ], + }, + ], + }, + }, + ], + 'Test Org', + ); + + expect(result).toBeInstanceOf(Buffer); + expect(result.length).toBeGreaterThan(0); + }); + + it('renders content that follows a table on the same page', () => { + // yPosition must advance past the table so following content doesn't + // overlap it. + const result = service.renderPoliciesPdfBuffer( + [ + { + name: 'Table Then Paragraph', + content: { + type: 'doc', + content: [ + { + type: 'table', + content: [ + { + type: 'tableRow', + content: [ + { + type: 'tableCell', + content: [ + { + type: 'paragraph', + content: [{ type: 'text', text: 'Cell' }], + }, + ], + }, + ], + }, + ], + }, + { + type: 'paragraph', + content: [ + { + type: 'text', + text: 'Paragraph after the table renders normally.', + }, + ], + }, + ], + }, + }, + ], + 'Test Org', + ); + + expect(result).toBeInstanceOf(Buffer); + expect(result.length).toBeGreaterThan(0); + }); + it('handles empty tables without crashing', () => { const result = service.renderPoliciesPdfBuffer( [ diff --git a/apps/api/src/trust-portal/policy-pdf-renderer.service.ts b/apps/api/src/trust-portal/policy-pdf-renderer.service.ts index d8e54da7fd..33a6ab5b36 100644 --- a/apps/api/src/trust-portal/policy-pdf-renderer.service.ts +++ b/apps/api/src/trust-portal/policy-pdf-renderer.service.ts @@ -507,25 +507,48 @@ export class PolicyPdfRendererService { /** * Extract display text from a table cell's block-level content. * - * Tiptap cells wrap their content in block nodes (one `paragraph` per line, - * plus the occasional `hardBreak`). Joining top-level blocks with '\n' - * preserves the visual separation a user sees in the editor so that - * splitTextToSize wraps each intended line separately — without it, two - * paragraphs like "Retention Period" and "30 days" would render as the - * single concatenated string "Retention Period30 days". + * Tiptap cells wrap their content in block nodes — most commonly one + * `paragraph` per visual line, plus the occasional `hardBreak` inside a + * paragraph or a `bulletList`/`orderedList`. We need to preserve the visual + * line boundaries the user sees in the editor so that `splitTextToSize` + * wraps each intended line separately. Without this, a cell with two + * paragraphs "Retention Period" and "30 days" renders as the single + * concatenated string "Retention Period30 days". + * + * Block boundaries that insert a newline: + * - top-level children of the cell (paragraph, bulletList, etc.) + * - each list item inside a bulletList/orderedList + * - `hardBreak` nodes + * + * Inline marks (bold, italic, link) are flattened to their text — we lose + * the formatting but the content is complete. */ private extractCellText(cellContent: JSONContent[]): string { return cellContent - .map((block) => this.extractInlineText(block)) + .map((block) => this.blockText(block)) .filter((s) => s.length > 0) .join('\n'); } - private extractInlineText(node: JSONContent): string { + private blockText(node: JSONContent): string { + if (node.type === 'bulletList' || node.type === 'orderedList') { + if (!node.content) return ''; + return node.content + .map((item) => this.blockText(item)) + .filter((s) => s.length > 0) + .join('\n'); + } + if (node.type === 'listItem') { + if (!node.content) return ''; + return node.content + .map((child) => this.blockText(child)) + .filter((s) => s.length > 0) + .join('\n'); + } if (node.type === 'hardBreak') return '\n'; if (node.text) return node.text; if (!node.content) return ''; - return node.content.map((child) => this.extractInlineText(child)).join(''); + return node.content.map((child) => this.blockText(child)).join(''); } renderPoliciesPdfBuffer( diff --git a/apps/app/src/lib/pdf-generator.ts b/apps/app/src/lib/pdf-generator.ts index 57da34641b..9466410ee6 100644 --- a/apps/app/src/lib/pdf-generator.ts +++ b/apps/app/src/lib/pdf-generator.ts @@ -304,19 +304,35 @@ const processContent = (config: PDFConfig, content: JSONContent[], level: number }; // Extract display text from a table cell's block-level content. -// See renderTable-in-sync note below: joins top-level blocks with '\n' so -// splitTextToSize wraps multi-paragraph cells correctly instead of producing -// a concatenated "Retention Period30 days". -const extractInlineText = (node: JSONContent): string => { +// Joins top-level blocks and list items with '\n' so splitTextToSize wraps +// multi-paragraph/multi-list-item cells correctly instead of producing a +// concatenated "Retention Period30 days" or "AlphaBeta". +// +// NOTE: Keep in sync with apps/api/src/trust-portal/policy-pdf-renderer.service.ts +const blockText = (node: JSONContent): string => { + if (node.type === 'bulletList' || node.type === 'orderedList') { + if (!node.content) return ''; + return node.content + .map((item) => blockText(item)) + .filter((s) => s.length > 0) + .join('\n'); + } + if (node.type === 'listItem') { + if (!node.content) return ''; + return node.content + .map((child) => blockText(child)) + .filter((s) => s.length > 0) + .join('\n'); + } if (node.type === 'hardBreak') return '\n'; if (node.text) return node.text; if (!node.content) return ''; - return node.content.map((child) => extractInlineText(child)).join(''); + return node.content.map((child) => blockText(child)).join(''); }; const extractCellText = (cellContent: JSONContent[]): string => cellContent - .map((block) => extractInlineText(block)) + .map((block) => blockText(block)) .filter((s) => s.length > 0) .join('\n'); From 9dea98a74e232fccc912cf43084a8ec4f9fb9a1c Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Thu, 16 Apr 2026 16:53:54 -0400 Subject: [PATCH 6/7] fix(pdf): prepend bullet/number markers to list items in table cells MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit List items inside a cell were rendered as indistinguishable plain-text lines, because the cell extractor joined listItem children with \n but never prepended a marker. The top-level list renderer already uses "•" for bulletList and "1.", "2." for orderedList — the cell path now matches so a cell with a list reads as a list, not as unlabeled lines. Adds regression test asserting (a) each item's text is present, (b) the WinAnsi bullet byte (0x95, jsPDF's encoding of U+2022) appears in a bulletList cell, and (c) numbered prefixes appear in an orderedList cell. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../policy-pdf-renderer.service.spec.ts | 120 +++++++++++++++++- .../policy-pdf-renderer.service.ts | 22 +++- apps/app/src/lib/pdf-generator.ts | 22 +++- 3 files changed, 154 insertions(+), 10 deletions(-) diff --git a/apps/api/src/trust-portal/policy-pdf-renderer.service.spec.ts b/apps/api/src/trust-portal/policy-pdf-renderer.service.spec.ts index 9dad7320bc..f1d4c4030f 100644 --- a/apps/api/src/trust-portal/policy-pdf-renderer.service.spec.ts +++ b/apps/api/src/trust-portal/policy-pdf-renderer.service.spec.ts @@ -522,14 +522,111 @@ describe('PolicyPdfRendererService', () => { expect(pdfText).not.toContain('Line oneLine two'); }); - it('separates bullet list items inside a cell with newlines', () => { + it('renders bullet and numbered list items inside a cell with markers', () => { // A cell whose only block is a bulletList used to concatenate items // (e.g. "AlphaBeta") because extractInlineText didn't recognize list - // containers as line-break boundaries. - const result = service.renderPoliciesPdfBuffer( + // containers as line-break boundaries. After the fix, items must + // also carry the same bullet/number prefix as the top-level list + // renderer so they read as a list rather than plain-text lines. + + // Helper: pull every (text)Tj token from a jsPDF buffer, with + // non-ASCII bytes spelled out as \xNN (jsPDF emits the bullet + // character U+2022 as WinAnsi byte 0x95 in its own Tj command). + const tokensFrom = (buf: Buffer): string[] => { + const raw = buf.toString('binary'); + const out: string[] = []; + const re = /\((.*?)\)\s*Tj/g; + let m: RegExpExecArray | null; + while ((m = re.exec(raw)) !== null) { + const bytes = Buffer.from(m[1], 'binary'); + out.push( + Array.from(bytes) + .map((b) => + b < 0x20 || b > 0x7e + ? `\\x${b.toString(16).padStart(2, '0')}` + : String.fromCharCode(b), + ) + .join(''), + ); + } + return out; + }; + + const orderedResult = service.renderPoliciesPdfBuffer( [ { - name: 'List-in-Cell Policy', + name: 'Ordered List in Cell', + content: { + type: 'doc', + content: [ + { + type: 'table', + content: [ + { + type: 'tableRow', + content: [ + { + type: 'tableCell', + content: [ + { + type: 'orderedList', + content: [ + { + type: 'listItem', + content: [ + { + type: 'paragraph', + content: [ + { type: 'text', text: 'First step' }, + ], + }, + ], + }, + { + type: 'listItem', + content: [ + { + type: 'paragraph', + content: [ + { type: 'text', text: 'Second step' }, + ], + }, + ], + }, + ], + }, + ], + }, + ], + }, + ], + }, + ], + }, + }, + ], + 'Test Org', + ); + + expect(orderedResult).toBeInstanceOf(Buffer); + const orderedTokens = tokensFrom(orderedResult); + // Numbered prefixes and their item text must both be present. + // jsPDF may emit the prefix and item text as separate adjacent Tj + // commands (e.g. "1." + "First step"); accept either form. + const orderedHas = (needle: string): boolean => + orderedTokens.some((t) => t.includes(needle)); + expect(orderedHas('1.')).toBe(true); + expect(orderedHas('2.')).toBe(true); + expect(orderedHas('First step')).toBe(true); + expect(orderedHas('Second step')).toBe(true); + // The concatenated-without-markers string must NOT appear. + const orderedRaw = orderedResult.toString('latin1'); + expect(orderedRaw).not.toContain('First stepSecond step'); + + const bulletResult = service.renderPoliciesPdfBuffer( + [ + { + name: 'Bullet List in Cell', content: { type: 'doc', content: [ @@ -582,8 +679,19 @@ describe('PolicyPdfRendererService', () => { 'Test Org', ); - const pdfText = result.toString('latin1'); - expect(pdfText).not.toContain('AlphaBeta'); + const bulletTokens = tokensFrom(bulletResult); + // jsPDF emits the bullet character U+2022 as WinAnsi byte 0x95. The + // whole line '• Alpha' may show up as one token '\x95 Alpha', or as + // two adjacent tokens '\x95' + ' Alpha' depending on jsPDF's text + // layout. Accept both. + const contains = (needle: string): boolean => + bulletTokens.some((t) => t.includes(needle)); + expect(contains('Alpha')).toBe(true); + expect(contains('Beta')).toBe(true); + expect(contains('\\x95')).toBe(true); + // The concatenated-without-separator string must NOT appear. + const bulletRaw = bulletResult.toString('latin1'); + expect(bulletRaw).not.toContain('AlphaBeta'); }); it('renders very long cell text across wrapped lines', () => { diff --git a/apps/api/src/trust-portal/policy-pdf-renderer.service.ts b/apps/api/src/trust-portal/policy-pdf-renderer.service.ts index 33a6ab5b36..da92a19bc1 100644 --- a/apps/api/src/trust-portal/policy-pdf-renderer.service.ts +++ b/apps/api/src/trust-portal/policy-pdf-renderer.service.ts @@ -531,14 +531,22 @@ export class PolicyPdfRendererService { } private blockText(node: JSONContent): string { - if (node.type === 'bulletList' || node.type === 'orderedList') { + if (node.type === 'bulletList') { if (!node.content) return ''; return node.content - .map((item) => this.blockText(item)) + .map((item) => this.renderListItem(item, '•')) + .filter((s) => s.length > 0) + .join('\n'); + } + if (node.type === 'orderedList') { + if (!node.content) return ''; + return node.content + .map((item, i) => this.renderListItem(item, `${i + 1}.`)) .filter((s) => s.length > 0) .join('\n'); } if (node.type === 'listItem') { + // Bare listItem without a parent list (unusual); render without prefix. if (!node.content) return ''; return node.content .map((child) => this.blockText(child)) @@ -551,6 +559,16 @@ export class PolicyPdfRendererService { return node.content.map((child) => this.blockText(child)).join(''); } + private renderListItem(itemNode: JSONContent, prefix: string): string { + if (!itemNode.content) return ''; + const body = itemNode.content + .map((child) => this.blockText(child)) + .filter((s) => s.length > 0) + .join('\n'); + if (!body) return ''; + return `${prefix} ${body}`; + } + renderPoliciesPdfBuffer( policies: PolicyForPDF[], organizationName?: string, diff --git a/apps/app/src/lib/pdf-generator.ts b/apps/app/src/lib/pdf-generator.ts index 9466410ee6..662bed434d 100644 --- a/apps/app/src/lib/pdf-generator.ts +++ b/apps/app/src/lib/pdf-generator.ts @@ -309,15 +309,33 @@ const processContent = (config: PDFConfig, content: JSONContent[], level: number // concatenated "Retention Period30 days" or "AlphaBeta". // // NOTE: Keep in sync with apps/api/src/trust-portal/policy-pdf-renderer.service.ts +const renderListItem = (itemNode: JSONContent, prefix: string): string => { + if (!itemNode.content) return ''; + const body = itemNode.content + .map((child) => blockText(child)) + .filter((s) => s.length > 0) + .join('\n'); + if (!body) return ''; + return `${prefix} ${body}`; +}; + const blockText = (node: JSONContent): string => { - if (node.type === 'bulletList' || node.type === 'orderedList') { + if (node.type === 'bulletList') { + if (!node.content) return ''; + return node.content + .map((item) => renderListItem(item, '•')) + .filter((s) => s.length > 0) + .join('\n'); + } + if (node.type === 'orderedList') { if (!node.content) return ''; return node.content - .map((item) => blockText(item)) + .map((item, i) => renderListItem(item, `${i + 1}.`)) .filter((s) => s.length > 0) .join('\n'); } if (node.type === 'listItem') { + // Bare listItem without a parent list (unusual); render without prefix. if (!node.content) return ''; return node.content .map((child) => blockText(child)) From 9f140e1213f1c3f8335c63733152017e824fce8b Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Thu, 16 Apr 2026 17:01:44 -0400 Subject: [PATCH 7/7] fix(integrations): harden disconnect re-eval (CS-166 review) Two follow-ups to cubic's review on PR #2577: 1. Make the latest-run-per-checkId selection in deriveTargetStatusForTask order-independent: compare run.createdAt instead of trusting the caller's orderBy. Matches the scheduler's getTargetStatus. 2. Wrap reevaluateFailedTasksAfterDisconnect in try/catch at both call sites (disconnectConnection, deleteConnection). The primary disconnect has already persisted by the time re-evaluation runs; a transient DB error during this best-effort cleanup must not surface to the caller. Adds regression tests: (a) reverse-sorted input still picks newest run per checkId, (b) disconnect resolves successfully even when the runs query throws. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../services/connection.service.spec.ts | 50 +++++++++++++++++-- .../services/connection.service.ts | 33 ++++++++++-- 2 files changed, 76 insertions(+), 7 deletions(-) diff --git a/apps/api/src/integration-platform/services/connection.service.spec.ts b/apps/api/src/integration-platform/services/connection.service.spec.ts index 8a8872e7f2..7861af517e 100644 --- a/apps/api/src/integration-platform/services/connection.service.spec.ts +++ b/apps/api/src/integration-platform/services/connection.service.spec.ts @@ -141,9 +141,6 @@ describe('ConnectionService', () => { { id: 'tsk_4', evidenceAutomations: [], - // Mock comes pre-sorted desc by createdAt to match the service's - // orderBy. Latest run for check_a is success; check_b is only one - // run and it's success. integrationCheckRuns: [ { checkId: 'check_a', @@ -172,6 +169,53 @@ describe('ConnectionService', () => { }); }); + it('picks the latest run per checkId even when the input is reverse-sorted', async () => { + // Defensive test: if a future change breaks the query's orderBy, + // the logic must still pick the newest run per checkId. + findRuns.mockResolvedValue([{ taskId: 'tsk_reorder' }]); + findTasks.mockResolvedValue([ + { + id: 'tsk_reorder', + evidenceAutomations: [], + // Oldest first — the opposite of the query's orderBy desc. + integrationCheckRuns: [ + { + checkId: 'check_a', + status: 'failed', + createdAt: new Date('2026-04-01'), + }, + { + checkId: 'check_a', + status: 'success', + createdAt: new Date('2026-04-05'), + }, + ], + }, + ]); + + await service.disconnectConnection(CONNECTION_ID); + + // Latest run for check_a (2026-04-05) is success → task should become + // done. If we naively picked the first-seen run, it would be failed + // and the task would stay at 'failed'. + expect(updateTask).toHaveBeenCalledWith({ + where: { id: 'tsk_reorder' }, + data: { status: 'done' }, + }); + }); + + it('swallows errors from the re-evaluation step so disconnect still succeeds', async () => { + // The primary disconnect has already succeeded by the time re-evaluation + // runs. A DB hiccup in the cleanup path must not surface to the caller. + findRuns.mockRejectedValue(new Error('transient DB failure')); + + await expect( + service.disconnectConnection(CONNECTION_ID), + ).resolves.toEqual( + expect.objectContaining({ id: CONNECTION_ID, status: 'disconnected' }), + ); + }); + it('does not touch a task that is not currently failed', async () => { findRuns.mockResolvedValue([{ taskId: 'tsk_5' }]); // findTasks filters by status: 'failed', so non-failed tasks are not returned diff --git a/apps/api/src/integration-platform/services/connection.service.ts b/apps/api/src/integration-platform/services/connection.service.ts index 32ce8dd152..eafd4023bc 100644 --- a/apps/api/src/integration-platform/services/connection.service.ts +++ b/apps/api/src/integration-platform/services/connection.service.ts @@ -145,7 +145,16 @@ export class ConnectionService { activeCredentialVersionId: null, }); - await this.reevaluateFailedTasksAfterDisconnect(connectionId); + // Best-effort task status cleanup. The primary disconnect already + // succeeded above; a failure here must not surface to the caller. + try { + await this.reevaluateFailedTasksAfterDisconnect(connectionId); + } catch (error) { + this.logger.error( + `Failed to re-evaluate task statuses after disconnecting ${connectionId}`, + error instanceof Error ? error.stack : String(error), + ); + } return connection; } @@ -162,7 +171,15 @@ export class ConnectionService { errorMessage: null, }); - await this.reevaluateFailedTasksAfterDisconnect(connectionId); + // Best-effort task status cleanup (see disconnectConnection for rationale). + try { + await this.reevaluateFailedTasksAfterDisconnect(connectionId); + } catch (error) { + this.logger.error( + `Failed to re-evaluate task statuses after deleting ${connectionId}`, + error instanceof Error ? error.stack : String(error), + ); + } } /** @@ -249,10 +266,18 @@ export class ConnectionService { const hasApp = task.integrationCheckRuns.length > 0; let appPassing = false; if (hasApp) { - const latestByCheckId = new Map(); + // Order-independent latest-per-checkId selection. Matches the scheduler's + // getTargetStatus so behavior doesn't silently depend on whether the + // caller's query ordered runs by createdAt. + const latestByCheckId = new Map< + string, + { status: string; createdAt: Date } + >(); for (const run of task.integrationCheckRuns) { const existing = latestByCheckId.get(run.checkId); - if (!existing) latestByCheckId.set(run.checkId, run); + if (!existing || run.createdAt > existing.createdAt) { + latestByCheckId.set(run.checkId, run); + } } appPassing = Array.from(latestByCheckId.values()).every( (r) => r.status === 'success',