[dev] [carhartlewis] lewis/comp-background-checks#2704
Conversation
…ground-checks # Conflicts: # apps/api/src/frameworks/frameworks-scores.helper.ts # apps/api/src/main.ts # apps/app/.env.example # packages/db/prisma/schema/organization-billing.prisma
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
There was a problem hiding this comment.
13 issues found across 73 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/api/src/main.ts">
<violation number="1" location="apps/api/src/main.ts:103">
P2: Avoid copying the raw request buffer here; `Buffer.from(buf)` duplicates the payload and increases memory pressure for large requests.</violation>
</file>
<file name="apps/app/src/app/(app)/[orgId]/people/all/components/TeamMembers.tsx">
<violation number="1" location="apps/app/src/app/(app)/[orgId]/people/all/components/TeamMembers.tsx:15">
P3: This adds a duplicate `BackgroundCheckStatus` definition instead of reusing a shared source of truth, which increases status-drift risk across background-check flows.</violation>
</file>
<file name="apps/app/src/app/(app)/[orgId]/people/[employeeId]/components/BackgroundCheckStatusView.tsx">
<violation number="1" location="apps/app/src/app/(app)/[orgId]/people/[employeeId]/components/BackgroundCheckStatusView.tsx:47">
P2: Defaulting `customAttachments` to `[]` hides loading/error states and can incorrectly show “Report is still syncing” for completed checks.</violation>
</file>
<file name="apps/api/src/background-checks/background-check-payment.service.spec.ts">
<violation number="1" location="apps/api/src/background-checks/background-check-payment.service.spec.ts:37">
P2: The test asserts only `HttpException` type and does not verify a 402 payment-required status, so it can pass on the wrong error path.</violation>
</file>
<file name="apps/api/src/background-checks/background-check-custom.service.ts">
<violation number="1" location="apps/api/src/background-checks/background-check-custom.service.ts:68">
P1: Don’t set the request to `completed` before the file upload succeeds; a failed upload leaves an incorrect completed/synced background-check record.</violation>
</file>
<file name="apps/app/src/app/(app)/[orgId]/people/[employeeId]/components/BackgroundCheckReport.tsx">
<violation number="1" location="apps/app/src/app/(app)/[orgId]/people/[employeeId]/components/BackgroundCheckReport.tsx:235">
P2: Numeric-string epoch timestamps are parsed as invalid dates, causing audit events to lose their timestamp and sort/display incorrectly.</violation>
</file>
<file name="apps/app/src/app/(app)/[orgId]/people/all/components/MemberRow.tsx">
<violation number="1" location="apps/app/src/app/(app)/[orgId]/people/all/components/MemberRow.tsx:177">
P2: Include the device requirement in task totals while device status is loading; otherwise task progress can be overstated until the device check resolves.</violation>
</file>
<file name="apps/api/src/background-checks/background-check-identity.client.ts">
<violation number="1" location="apps/api/src/background-checks/background-check-identity.client.ts:93">
P2: Add a timeout to Identity API fetch calls so upstream stalls don’t block requests indefinitely.</violation>
</file>
<file name="apps/api/src/background-checks/background-checks.service.ts">
<violation number="1" location="apps/api/src/background-checks/background-checks.service.ts:141">
P1: The catch block refunds on any error in the try block, including local DB write failures after a successful identity creation.</violation>
<violation number="2" location="apps/api/src/background-checks/background-checks.service.ts:246">
P1: Duplicate-event short-circuit returns before state reconciliation, so retries can permanently skip updating the background check after a partial failure.</violation>
</file>
<file name="apps/api/src/background-checks/background-checks.controller.ts">
<violation number="1" location="apps/api/src/background-checks/background-checks.controller.ts:59">
P2: `employeeName` is trimmed after validation, which allows whitespace-only input to become an empty string and be processed as a valid request.</violation>
</file>
<file name="apps/api/src/background-checks/background-check-billing.service.ts">
<violation number="1" location="apps/api/src/background-checks/background-check-billing.service.ts:81">
P1: Verify `session.status === 'complete'` before processing the checkout session. Without this check, a user could manually submit an incomplete session (e.g., failed 3D Secure), bypassing successful payment method authentication.</violation>
<violation number="2" location="apps/api/src/background-checks/background-check-billing.service.ts:195">
P2: Checking `!price.unit_amount` will erroneously reject prices configured with a $0 amount because 0 is falsy. Use `price.unit_amount === null` instead.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
…uploads, and UI P1: Upload file before marking custom background check as completed, scope refund to identity API failures only, reconcile state on duplicate webhook events, and verify checkout session status before processing. P2: Remove unnecessary buffer copy in raw body parsing, track SWR loading state for custom attachments, handle numeric-string epoch timestamps, include device task in totals while loading, add 30s timeout to Identity API calls, assert 402 status in payment test, accept $0 prices in billing, and validate whitespace-only employee names via DTO transform. P3: Deduplicate BackgroundCheckStatus type definition. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ation, and logging P1: Restructure requestForMember to create DB record before charging Stripe, preventing orphaned payments on DB failure and eliminating the TOCTOU race condition on concurrent requests via unique constraint catch. P2: Add @maxlength to base64 fileData field (50MB limit), add @isurl validation to billing redirect DTOs, remove env var names from error messages, and add session metadata org check in handleSetupSuccess. P3: Enhance refund failure logging with structured context for manual intervention. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
2 issues found across 7 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/api/src/attachments/upload-attachment.dto.ts">
<violation number="1" location="apps/api/src/attachments/upload-attachment.dto.ts:38">
P2: This max-length value is applied to base64 characters, which drops the effective binary upload limit to ~48MiB and conflicts with the service’s 100MB file-size policy.</violation>
</file>
<file name="apps/api/src/background-checks/dto/background-check-billing.dto.ts">
<violation number="1" location="apps/api/src/background-checks/dto/background-check-billing.dto.ts:5">
P2: Using `@IsUrl` with default options rejects `http://localhost` redirect URLs used in local/dev flows. Set URL options to allow localhost (e.g. `require_tld: false`) so valid app-origin redirects are not blocked.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
… URL validation Raise base64 @maxlength to 134_217_728 (~100MB binary) to match the service's file-size policy. Set require_tld: false on @isurl so http://localhost redirect URLs used in local/dev flows are not rejected. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/api/src/attachments/upload-attachment.dto.ts">
<violation number="1" location="apps/api/src/attachments/upload-attachment.dto.ts:38">
P2: The base64 max-length value is too low for the service’s 100MB file limit, so valid 96–100MB uploads will be rejected during DTO validation.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
|
🎉 This PR is included in version 3.39.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
This is an automated pull request to merge lewis/comp-background-checks into dev.
It was created by the [Auto Pull Request] action.
Summary by cubic
Adds end-to-end employee background checks with request/invite, Stripe billing (payment method setup and one-off charging), webhook sync, and in‑app report viewing. Stores completed report snapshots and shows a verified tick; supports uploading third‑party reports and a shareable summary.
Bug Fixes
Migration
Written for commit 6d56a2d. Summary will update on new commits. Review in cubic