From 6e239b9197319e8859feacc2372bb3900076fdca Mon Sep 17 00:00:00 2001 From: Mariano Date: Thu, 7 May 2026 14:03:59 +0100 Subject: [PATCH 1/4] fix: address cubic review findings on onboarding PR P1: Add metadata.set('policies', true) after policy fan-out so the tracker boolean flag is set. P1: Log batchTriggerAndWait failures in vendor/risk mitigation fan-outs instead of silently ignoring them. P2: Strip {{#if}}/{{/if}} markers from mixed-content nodes so template syntax doesn't leak into rendered policies. P2: Fix stale onboardingTriggerJobId locking publish button in ToDoOverview. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../frameworks/frameworks-scores.helper.ts | 4 ++-- .../onboarding/generate-risk-mitigation.ts | 8 +++++++- .../onboarding/generate-vendor-mitigation.ts | 8 +++++++- .../tasks/onboarding/onboard-organization.ts | 1 + .../onboarding/process-policy-template.ts | 19 +++++++++++++++++-- 5 files changed, 34 insertions(+), 6 deletions(-) diff --git a/apps/api/src/frameworks/frameworks-scores.helper.ts b/apps/api/src/frameworks/frameworks-scores.helper.ts index c4cfca9cf..fab0f7144 100644 --- a/apps/api/src/frameworks/frameworks-scores.helper.ts +++ b/apps/api/src/frameworks/frameworks-scores.helper.ts @@ -29,7 +29,7 @@ export async function getOverviewScores(organizationId: string) { }), db.onboarding.findUnique({ where: { organizationId }, - select: { triggerJobId: true }, + select: { triggerJobId: true, triggerJobCompleted: true }, }), db.organization.findUnique({ where: { id: organizationId }, @@ -90,7 +90,7 @@ export async function getOverviewScores(organizationId: string) { incompleteTasks, }, people, - onboardingTriggerJobId: onboarding?.triggerJobId ?? null, + onboardingTriggerJobId: onboarding?.triggerJobCompleted ? null : (onboarding?.triggerJobId ?? null), documents: await computeDocumentsScore(organizationId), findings: await getOrganizationFindings(organizationId), }; diff --git a/apps/app/src/trigger/tasks/onboarding/generate-risk-mitigation.ts b/apps/app/src/trigger/tasks/onboarding/generate-risk-mitigation.ts index 733847323..475bc5ebe 100644 --- a/apps/app/src/trigger/tasks/onboarding/generate-risk-mitigation.ts +++ b/apps/app/src/trigger/tasks/onboarding/generate-risk-mitigation.ts @@ -114,7 +114,7 @@ export const generateRiskMitigationsForOrg = task({ const policies = policyRows.map((p) => ({ name: p.name, description: p.description })); - await tasks.batchTriggerAndWait( + const batchResult = await tasks.batchTriggerAndWait( 'generate-risk-mitigation', risks.map((r) => ({ payload: { @@ -126,6 +126,12 @@ export const generateRiskMitigationsForOrg = task({ options: { concurrencyKey: `${organizationId}:${r.id}` }, })), ); + const failures = batchResult.runs.filter((r) => !r.ok); + if (failures.length > 0) { + logger.error(`${failures.length} risk mitigation(s) failed`, { + failedRunIds: failures.map((r) => r.id), + }); + } // Revalidate the parent risk routes after batch triggering try { diff --git a/apps/app/src/trigger/tasks/onboarding/generate-vendor-mitigation.ts b/apps/app/src/trigger/tasks/onboarding/generate-vendor-mitigation.ts index 8e16b540a..f7f2662fa 100644 --- a/apps/app/src/trigger/tasks/onboarding/generate-vendor-mitigation.ts +++ b/apps/app/src/trigger/tasks/onboarding/generate-vendor-mitigation.ts @@ -116,7 +116,7 @@ export const generateVendorMitigationsForOrg = task({ const policies = policyRows.map((p) => ({ name: p.name, description: p.description })); - await tasks.batchTriggerAndWait( + const batchResult = await tasks.batchTriggerAndWait( 'generate-vendor-mitigation', vendors.map((v) => ({ payload: { @@ -128,6 +128,12 @@ export const generateVendorMitigationsForOrg = task({ options: { concurrencyKey: `${organizationId}:${v.id}` }, })), ); + const failures = batchResult.runs.filter((r) => !r.ok); + if (failures.length > 0) { + logger.error(`${failures.length} vendor mitigation(s) failed`, { + failedRunIds: failures.map((r) => r.id), + }); + } // Revalidate the parent vendors route after batch triggering try { diff --git a/apps/app/src/trigger/tasks/onboarding/onboard-organization.ts b/apps/app/src/trigger/tasks/onboarding/onboard-organization.ts index 478e05b47..e9d697896 100644 --- a/apps/app/src/trigger/tasks/onboarding/onboard-organization.ts +++ b/apps/app/src/trigger/tasks/onboarding/onboard-organization.ts @@ -106,6 +106,7 @@ export const onboardOrganization = task({ const policyCount = policyList.length; metadata.set('currentStep', `Tailoring Policies... (0/${policyCount})`); await updateOrganizationPolicies(payload.organizationId, questionsAndAnswers, frameworks); + metadata.set('policies', true); // Extract vendors + risks in parallel (both are independent LLM calls). metadata.set('currentStep', 'Creating Vendors...'); diff --git a/apps/app/src/trigger/tasks/onboarding/process-policy-template.ts b/apps/app/src/trigger/tasks/onboarding/process-policy-template.ts index 4296331e0..33a571963 100644 --- a/apps/app/src/trigger/tasks/onboarding/process-policy-template.ts +++ b/apps/app/src/trigger/tasks/onboarding/process-policy-template.ts @@ -73,6 +73,19 @@ function processInlineConditionals(text: string, flags: Record) ); } +function stripMarkerText(node: JsonNode, marker: RegExp): JsonNode { + if (typeof node.text === 'string') { + return { ...node, text: node.text.replace(marker, '') }; + } + if (Array.isArray(node.content)) { + return { + ...node, + content: (node.content as JsonNode[]).map((child) => stripMarkerText(child, marker)), + }; + } + return node; +} + function processTextNode(node: JsonNode, vars: Record, flags: Record): JsonNode | null { if (typeof node.text !== 'string') return node; @@ -136,7 +149,8 @@ export function processContentArray( if (!isTrue) { skipDepth++; } else if (!hasOnlyMarker) { - const processed = processNode(node, vars, flags); + const stripped = stripMarkerText(node, /\{\{#if\s+\w+\}\}/g); + const processed = processNode(stripped, vars, flags); if (processed) result.push(processed); } continue; @@ -146,7 +160,8 @@ export function processContentArray( if (skipDepth > 0) { skipDepth--; } else if (!hasOnlyMarker) { - const processed = processNode(node, vars, flags); + const stripped = stripMarkerText(node, /\{\{\/if\}\}/g); + const processed = processNode(stripped, vars, flags); if (processed) result.push(processed); } continue; From cf6115fe4c81b90cd340f9247f60ac5f2f253595 Mon Sep 17 00:00:00 2001 From: Mariano Date: Thu, 7 May 2026 14:08:00 +0100 Subject: [PATCH 2/4] fix: strip only first marker occurrence to preserve nested conditionals The global regex flag in stripMarkerText would remove ALL matching {{#if}}/{{/if}} markers in a subtree, corrupting boundaries of nested conditional blocks. Removed the g flag so only the first occurrence (the one that triggered the match) is stripped. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../src/trigger/tasks/onboarding/process-policy-template.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/app/src/trigger/tasks/onboarding/process-policy-template.ts b/apps/app/src/trigger/tasks/onboarding/process-policy-template.ts index 33a571963..4b8021802 100644 --- a/apps/app/src/trigger/tasks/onboarding/process-policy-template.ts +++ b/apps/app/src/trigger/tasks/onboarding/process-policy-template.ts @@ -149,7 +149,7 @@ export function processContentArray( if (!isTrue) { skipDepth++; } else if (!hasOnlyMarker) { - const stripped = stripMarkerText(node, /\{\{#if\s+\w+\}\}/g); + const stripped = stripMarkerText(node, /\{\{#if\s+\w+\}\}/); const processed = processNode(stripped, vars, flags); if (processed) result.push(processed); } @@ -160,7 +160,7 @@ export function processContentArray( if (skipDepth > 0) { skipDepth--; } else if (!hasOnlyMarker) { - const stripped = stripMarkerText(node, /\{\{\/if\}\}/g); + const stripped = stripMarkerText(node, /\{\{\/if\}\}/); const processed = processNode(stripped, vars, flags); if (processed) result.push(processed); } From da742c9f55237079b36c60c9094ae9e093105520 Mon Sep 17 00:00:00 2001 From: Mariano Date: Thu, 7 May 2026 14:14:07 +0100 Subject: [PATCH 3/4] test(onboarding): add comprehensive tests for policy template processor Covers placeholder replacement, inline/multi-node/nested conditionals, mixed content nodes, edge cases, buildVariables, buildFlags, processTemplate. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../process-policy-template.test.ts | 324 ++++++++++++++++++ 1 file changed, 324 insertions(+) create mode 100644 apps/app/src/trigger/tasks/onboarding/process-policy-template.test.ts diff --git a/apps/app/src/trigger/tasks/onboarding/process-policy-template.test.ts b/apps/app/src/trigger/tasks/onboarding/process-policy-template.test.ts new file mode 100644 index 000000000..e5d3b1db3 --- /dev/null +++ b/apps/app/src/trigger/tasks/onboarding/process-policy-template.test.ts @@ -0,0 +1,324 @@ +import { describe, it, expect } from 'vitest'; +import { processContentArray, buildFlags, buildVariables, processTemplate } from './process-policy-template'; + +const vars = { COMPANY: 'Acme Inc', EMPLOYEES: '50', DATA: 'PII' }; + +function textNode(text: string) { + return { type: 'text', text }; +} + +function paragraph(...children: Record[]) { + return { type: 'paragraph', content: children }; +} + +describe('processContentArray', () => { + describe('placeholder replacement', () => { + it('replaces {{COMPANY}} in text nodes', () => { + const nodes = [paragraph(textNode('Welcome to {{COMPANY}}'))]; + const result = processContentArray(nodes, vars, {}); + expect((result[0] as any).content[0].text).toBe('Welcome to Acme Inc'); + }); + + it('replaces multiple placeholders', () => { + const nodes = [paragraph(textNode('{{COMPANY}} has {{EMPLOYEES}} employees handling {{DATA}}'))]; + const result = processContentArray(nodes, vars, {}); + expect((result[0] as any).content[0].text).toBe('Acme Inc has 50 employees handling PII'); + }); + + it('replaces unknown placeholders with N/A', () => { + const nodes = [paragraph(textNode('Contact {{UNKNOWN}}'))]; + const result = processContentArray(nodes, vars, {}); + expect((result[0] as any).content[0].text).toBe('Contact N/A'); + }); + }); + + describe('inline conditionals (same text node)', () => { + it('keeps content when flag is true', () => { + const nodes = [paragraph(textNode('Before {{#if soc2}}SOC 2 content{{/if}} after'))]; + const result = processContentArray(nodes, vars, { soc2: true }); + expect((result[0] as any).content[0].text).toBe('Before SOC 2 content after'); + }); + + it('removes content when flag is false', () => { + const nodes = [paragraph(textNode('Before {{#if hipaa}}HIPAA content{{/if}} after'))]; + const result = processContentArray(nodes, vars, { hipaa: false }); + expect((result[0] as any).content[0].text).toBe('Before after'); + }); + + it('handles multiple inline conditionals in same text', () => { + const nodes = [paragraph(textNode('{{#if soc2}}SOC2{{/if}} and {{#if hipaa}}HIPAA{{/if}}'))]; + const result = processContentArray(nodes, vars, { soc2: true, hipaa: false }); + expect((result[0] as any).content[0].text).toBe('SOC2 and '); + }); + }); + + describe('multi-node conditionals (marker-only nodes)', () => { + it('keeps block when flag is true', () => { + const nodes = [ + paragraph(textNode('{{#if soc2}}')), + paragraph(textNode('SOC 2 specific content')), + paragraph(textNode('{{/if}}')), + paragraph(textNode('Always visible')), + ]; + const result = processContentArray(nodes, vars, { soc2: true }); + expect(result).toHaveLength(2); + expect((result[0] as any).content[0].text).toBe('SOC 2 specific content'); + expect((result[1] as any).content[0].text).toBe('Always visible'); + }); + + it('removes block when flag is false', () => { + const nodes = [ + paragraph(textNode('{{#if hipaa}}')), + paragraph(textNode('HIPAA specific content')), + paragraph(textNode('More HIPAA content')), + paragraph(textNode('{{/if}}')), + paragraph(textNode('Always visible')), + ]; + const result = processContentArray(nodes, vars, { hipaa: false }); + expect(result).toHaveLength(1); + expect((result[0] as any).content[0].text).toBe('Always visible'); + }); + + it('removes block for unknown flags (defaults to false)', () => { + const nodes = [ + paragraph(textNode('{{#if unknownFramework}}')), + paragraph(textNode('Should be removed')), + paragraph(textNode('{{/if}}')), + ]; + const result = processContentArray(nodes, vars, {}); + expect(result).toHaveLength(0); + }); + }); + + describe('mixed content nodes (marker + text on same node)', () => { + it('strips {{#if}} marker but keeps remaining text when true', () => { + const nodes = [ + paragraph(textNode('{{#if soc2}} SOC 2 intro text')), + paragraph(textNode('More content')), + paragraph(textNode('{{/if}}')), + ]; + const result = processContentArray(nodes, vars, { soc2: true }); + expect(result).toHaveLength(2); + expect((result[0] as any).content[0].text).toBe(' SOC 2 intro text'); + expect((result[1] as any).content[0].text).toBe('More content'); + }); + + it('strips {{/if}} marker but keeps remaining text', () => { + const nodes = [ + paragraph(textNode('{{#if soc2}}')), + paragraph(textNode('Content here')), + paragraph(textNode('End of section {{/if}}')), + ]; + const result = processContentArray(nodes, vars, { soc2: true }); + expect(result).toHaveLength(2); + expect((result[0] as any).content[0].text).toBe('Content here'); + expect((result[1] as any).content[0].text).toBe('End of section '); + }); + + it('removes mixed content node when flag is false', () => { + const nodes = [ + paragraph(textNode('{{#if hipaa}} HIPAA intro')), + paragraph(textNode('HIPAA body')), + paragraph(textNode('{{/if}}')), + ]; + const result = processContentArray(nodes, vars, { hipaa: false }); + expect(result).toHaveLength(0); + }); + }); + + describe('nested conditionals', () => { + it('outer true, inner true: keeps both', () => { + const nodes = [ + paragraph(textNode('{{#if soc2}}')), + paragraph(textNode('SOC 2 content')), + paragraph(textNode('{{#if hipaa}}')), + paragraph(textNode('SOC 2 + HIPAA content')), + paragraph(textNode('{{/if}}')), + paragraph(textNode('{{/if}}')), + ]; + const result = processContentArray(nodes, vars, { soc2: true, hipaa: true }); + expect(result).toHaveLength(2); + expect((result[0] as any).content[0].text).toBe('SOC 2 content'); + expect((result[1] as any).content[0].text).toBe('SOC 2 + HIPAA content'); + }); + + it('outer true, inner false: keeps outer, removes inner', () => { + const nodes = [ + paragraph(textNode('{{#if soc2}}')), + paragraph(textNode('SOC 2 only')), + paragraph(textNode('{{#if hipaa}}')), + paragraph(textNode('Should be removed')), + paragraph(textNode('{{/if}}')), + paragraph(textNode('Still SOC 2')), + paragraph(textNode('{{/if}}')), + ]; + const result = processContentArray(nodes, vars, { soc2: true, hipaa: false }); + expect(result).toHaveLength(2); + expect((result[0] as any).content[0].text).toBe('SOC 2 only'); + expect((result[1] as any).content[0].text).toBe('Still SOC 2'); + }); + + it('outer false: removes everything including true inner', () => { + const nodes = [ + paragraph(textNode('{{#if hipaa}}')), + paragraph(textNode('HIPAA content')), + paragraph(textNode('{{#if soc2}}')), + paragraph(textNode('LEAKED if buggy')), + paragraph(textNode('{{/if}}')), + paragraph(textNode('{{/if}}')), + paragraph(textNode('After block')), + ]; + const result = processContentArray(nodes, vars, { hipaa: false, soc2: true }); + expect(result).toHaveLength(1); + expect((result[0] as any).content[0].text).toBe('After block'); + }); + + it('deeply nested: outer false hides all inner levels', () => { + const nodes = [ + paragraph(textNode('{{#if hipaa}}')), + paragraph(textNode('{{#if soc2}}')), + paragraph(textNode('{{#if gdpr}}')), + paragraph(textNode('Deep content')), + paragraph(textNode('{{/if}}')), + paragraph(textNode('{{/if}}')), + paragraph(textNode('{{/if}}')), + ]; + const result = processContentArray(nodes, vars, { hipaa: false, soc2: true, gdpr: true }); + expect(result).toHaveLength(0); + }); + }); + + describe('placeholder + conditional combined', () => { + it('replaces placeholders inside kept conditional blocks', () => { + const nodes = [ + paragraph(textNode('{{#if soc2}}')), + paragraph(textNode('{{COMPANY}} complies with SOC 2')), + paragraph(textNode('{{/if}}')), + ]; + const result = processContentArray(nodes, vars, { soc2: true }); + expect(result).toHaveLength(1); + expect((result[0] as any).content[0].text).toBe('Acme Inc complies with SOC 2'); + }); + + it('does not process placeholders in removed blocks', () => { + const nodes = [ + paragraph(textNode('{{#if hipaa}}')), + paragraph(textNode('{{COMPANY}} handles PHI')), + paragraph(textNode('{{/if}}')), + ]; + const result = processContentArray(nodes, vars, { hipaa: false }); + expect(result).toHaveLength(0); + }); + }); + + describe('edge cases', () => { + it('empty content array returns empty', () => { + expect(processContentArray([], vars, {})).toEqual([]); + }); + + it('node with no text or content passes through', () => { + const nodes = [{ type: 'hardBreak' }]; + const result = processContentArray(nodes, vars, {}); + expect(result).toHaveLength(1); + expect(result[0]).toEqual({ type: 'hardBreak' }); + }); + + it('removes empty text nodes after placeholder replacement', () => { + const nodes = [paragraph(textNode('{{#if hipaa}}{{/if}}'))]; + const result = processContentArray(nodes, vars, { hipaa: false }); + // Inline conditional removes content, leaving empty string → null → paragraph has no content + expect(result).toHaveLength(0); + }); + + it('preserves node attributes and marks', () => { + const nodes = [{ + type: 'paragraph', + attrs: { textAlign: 'center' }, + content: [{ + type: 'text', + text: '{{COMPANY}} policy', + marks: [{ type: 'bold' }], + }], + }]; + const result = processContentArray(nodes, vars, {}); + const node = result[0] as any; + expect(node.attrs.textAlign).toBe('center'); + expect(node.content[0].text).toBe('Acme Inc policy'); + expect(node.content[0].marks).toEqual([{ type: 'bold' }]); + }); + }); +}); + +describe('buildVariables', () => { + it('maps COMPANY from companyName', () => { + const vars = buildVariables({ companyName: 'TestCo', contextHub: '' }); + expect(vars.COMPANY).toBe('TestCo'); + }); + + it('extracts answers from contextHub Q&A format', () => { + const contextHub = 'What industry is your company in?\nSaaS\nHow many employees do you have?\n50'; + const vars = buildVariables({ companyName: 'X', contextHub }); + expect(vars.INDUSTRY).toBe('SaaS'); + expect(vars.EMPLOYEES).toBe('50'); + }); + + it('handles missing questions gracefully', () => { + const vars = buildVariables({ companyName: 'X', contextHub: 'Random text' }); + expect(vars.INDUSTRY).toBeUndefined(); + }); +}); + +describe('buildFlags', () => { + it('detects SOC 2 framework', () => { + const flags = buildFlags([{ name: 'SOC 2' }]); + expect(flags.soc2).toBe(true); + expect(flags.hipaa).toBe(false); + }); + + it('detects multiple frameworks', () => { + const flags = buildFlags([{ name: 'SOC 2' }, { name: 'HIPAA' }, { name: 'GDPR' }]); + expect(flags.soc2).toBe(true); + expect(flags.hipaa).toBe(true); + expect(flags.gdpr).toBe(true); + expect(flags.pipeda).toBe(false); + }); + + it('detects PIPEDA', () => { + const flags = buildFlags([{ name: 'PIPEDA' }]); + expect(flags.pipeda).toBe(true); + }); +}); + +describe('processTemplate', () => { + it('handles doc-wrapped content', () => { + const content = { + type: 'doc', + content: [paragraph(textNode('{{COMPANY}} policy'))], + }; + const result = processTemplate({ + content, + companyName: 'TestCo', + contextHub: '', + frameworks: [], + }); + expect(result).toHaveLength(1); + expect((result[0] as any).content[0].text).toBe('TestCo policy'); + }); + + it('handles array content', () => { + const content = [paragraph(textNode('Hello {{COMPANY}}'))]; + const result = processTemplate({ + content, + companyName: 'TestCo', + contextHub: '', + frameworks: [], + }); + expect((result[0] as any).content[0].text).toBe('Hello TestCo'); + }); + + it('returns empty for invalid content', () => { + expect(processTemplate({ content: null, companyName: '', contextHub: '', frameworks: [] })).toEqual([]); + expect(processTemplate({ content: 'string', companyName: '', contextHub: '', frameworks: [] })).toEqual([]); + expect(processTemplate({ content: 42, companyName: '', contextHub: '', frameworks: [] })).toEqual([]); + }); +}); From c7173e74146ef7cb8d8fe7c6ee6b715ed90cc4f7 Mon Sep 17 00:00:00 2001 From: Mariano Date: Thu, 7 May 2026 14:21:42 +0100 Subject: [PATCH 4/4] fix(onboarding): treat zero-item steps as complete in tracker When an org has no vendors or risks, `total > 0 && completed >= total` evaluates to false, causing those steps to appear stuck forever. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../src/app/(app)/[orgId]/components/OnboardingTracker.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/apps/app/src/app/(app)/[orgId]/components/OnboardingTracker.tsx b/apps/app/src/app/(app)/[orgId]/components/OnboardingTracker.tsx index f2c5dd82d..207b78a8a 100644 --- a/apps/app/src/app/(app)/[orgId]/components/OnboardingTracker.tsx +++ b/apps/app/src/app/(app)/[orgId]/components/OnboardingTracker.tsx @@ -197,10 +197,10 @@ export const OnboardingTracker = ({ onboarding }: { onboarding: Onboarding }) => return { vendors: meta.vendors === true, risk: meta.risk === true, - policies: pTotal > 0 && pCompleted >= pTotal, + policies: pTotal === 0 || pCompleted >= pTotal, linkage: meta.linkage === true, - vendorMitigations: vTotal > 0 && vCompleted >= vTotal, - riskMitigations: rTotal > 0 && rCompleted >= rTotal, + vendorMitigations: vTotal === 0 || vCompleted >= vTotal, + riskMitigations: rTotal === 0 || rCompleted >= rTotal, currentStep: (meta.currentStep as string) || null, vendorsTotal: (meta.vendorsTotal as number) || 0, vendorsCompleted: (meta.vendorsCompleted as number) || 0,