feat: cap table, reports & overview dashboard with live data#561
Open
maxgubitosi wants to merge 3 commits into
Open
feat: cap table, reports & overview dashboard with live data#561maxgubitosi wants to merge 3 commits into
maxgubitosi wants to merge 3 commits into
Conversation
Replace hardcoded demo numbers on the company Overview with values computed server-side from the same tables the dedicated pages list: share issuances, option grants (excluding cancelled/expired), equity plan pools, SAFEs and convertible notes. - src/server/overview.ts: getOverviewData() computes amount raised, fully-diluted shares, stakeholder count, ownership by stakeholder and by share class, and the per-class summary on a fully-diluted basis (issued + granted options + unallocated pool) - donut-card/summary-table now take data as props - show EmptyOverview when no equity is issued, reserved or raised - empty.tsx: fix 'undefined' rendering in welcome title Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Both pages were 'Work in progress' stubs. They are now driven by a shared cap-table engine so they can never disagree with the Overview. - src/server/captable.ts: getCapTable() computes a per-stakeholder x per-share-class fully-diluted matrix (shares, options, equity plan pool, itemized SAFEs/notes, totals) with the same semantics the Overview already shipped with - src/server/overview.ts: getOverviewData() now derives from getCapTable; output verified byte-identical against baselines - Cap table page: summary cards, ownership matrix with per-class columns and totals, equity-plan pool row, convertibles card, empty state; gated behind stakeholder:read like sibling pages - Reports page: four downloadable reports (cap table summary PDF, cap table matrix CSV, securities ledger CSV, stakeholders CSV) generated on demand by /api/reports/[type] with session + RBAC checks; PDF uses the existing @react-pdf/renderer pattern, CSVs use papaparse; no new dependencies Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
On the cap table, shares could not be edited and investors (stakeholders) could not be removed: - The "Update Share" row action was a dead label with no handler, and there was no updateShares procedure (addShares is create-only). - The investor table had no delete action and there was no deleteStakeholder procedure. Changes: - Add securities.updateShares procedure + ZodUpdateShareMutationSchema. - Make the share wizard reusable for editing: AddShareFormProvider accepts defaultValues, each step prefills from them, and the final step branches to updateShares (and no longer forces a new document upload) in edit mode. The share row action now opens this pre-filled wizard. - Add stakeholder.deleteStakeholder procedure + a Delete action in the investor table. The procedure refuses to remove an investor who still holds securities (shares/options/SAFEs/notes/investments) since relationMode= "prisma" provides no DB cascade, and it cleans up data-room / update recipient links. - get-shares now selects stakeholderId/shareClassId (needed to prefill edit). - Fix the share-delete toast that incorrectly said "stakeholder". Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Thank you for following the naming conventions for pull request titles! 🙏 |
📝 WalkthroughWalkthroughPlaceholder dashboard pages for cap table, overview, and reports are replaced with async server components backed by new ChangesCap table dashboard & reports
Share update & stakeholder delete mutations
Sequence Diagram(s)sequenceDiagram
participant Browser
participant ReportCard
participant GETRoute as GET /api/reports/[type]
participant getServerAuthSession
participant REPORTS
participant getCapTable
Browser->>ReportCard: click Download
ReportCard->>GETRoute: fetch /api/reports/${type}
GETRoute->>getServerAuthSession: verify session + companyId
GETRoute->>GETRoute: RBAC check stakeholder:read
GETRoute->>GETRoute: isReportType(params.type)
GETRoute->>REPORTS: generate(companyId)
REPORTS->>getCapTable: fetch cap table data
getCapTable-->>REPORTS: CapTable
REPORTS-->>GETRoute: body, contentType, filename
GETRoute-->>ReportCard: 200 Content-Disposition attachment
ReportCard->>Browser: blob download via anchor
sequenceDiagram
participant ShareTable
participant IssueShareModal
participant AddShareFormProvider
participant DocumentsStep
participant updateShareProcedure
ShareTable->>IssueShareModal: pushModal("IssueShareModal", defaultValues)
IssueShareModal->>AddShareFormProvider: initialize useReducer with defaultValues
AddShareFormProvider->>DocumentsStep: value.id present → isEdit=true
DocumentsStep->>updateShareProcedure: updateShareMutation(id, documents)
updateShareProcedure-->>DocumentsStep: success / failure message
DocumentsStep->>ShareTable: router.refresh()
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/dashboard/overview/empty.tsx (1)
22-27:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAvoid nested interactive controls in the CTA.
<Button>currently wraps<Link>, which creates nested interactive elements and can break accessibility/keyboard behavior. Render the link as the button element instead.Suggested fix
- <Button size="lg"> - <Link href={`/${publicCompanyId}/stakeholders`}> + <Button size="lg" asChild> + <Link href={`/${publicCompanyId}/stakeholders`}> Let{`'`}s get started <RiArrowRightLine className="ml-5 inline-block h-4 w-5" /> </Link> </Button>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/dashboard/overview/empty.tsx` around lines 22 - 27, The Button component wraps a Link component creating nested interactive elements which breaks accessibility and keyboard navigation. Refactor this by using the Button component's asChild prop to render the Link as the button element itself, removing the nested structure. Move the href attribute from the Link to the Button and place the text content directly in the Button, ensuring only a single interactive element is rendered instead of nested ones.
🧹 Nitpick comments (3)
src/components/stakeholder/stakeholder-table.tsx (2)
189-220: ⚖️ Poor tradeoffHooks called inside column cell renderer violate React rules.
useRouter()anduseMutation()are called inside thecellfunction, which is not a React component. While the eslint-disable comments acknowledge this, it can cause bugs if rows are dynamically added/removed mid-render. Consider extracting this into a separateStakeholderActionsCellcomponent that receives the row data as props.This appears to be an existing pattern in the codebase (likely in share-table.tsx as well), so addressing it may require a broader refactor.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/stakeholder/stakeholder-table.tsx` around lines 189 - 220, The cell renderer in the stakeholder table is calling React hooks (useRouter and useMutation) directly inside the column definition, which violates React's rules of hooks. Extract the hook-dependent logic into a separate React component named StakeholderActionsCell that accepts the row data (singleStakeholder) as a prop, then move the useRouter and deleteStakeholder.useMutation calls along with the handleDeleteStakeholder function into this new component. Finally, replace the current cell function with a call to the new StakeholderActionsCell component, passing the necessary row data as props.
248-255: ⚡ Quick winConsider disabling the delete button while the mutation is pending.
The delete action has no loading/disabled state. Users could trigger multiple delete requests by clicking rapidly before the mutation completes.
💡 Example approach
<Allow action="update" subject="stakeholder"> <DropdownMenuItem onSelect={handleDeleteStakeholder} - className="text-red-500" + className="text-red-500" + disabled={deleteStakeholderMutation.isPending} > - Delete + {deleteStakeholderMutation.isPending ? "Deleting..." : "Delete"} </DropdownMenuItem> </Allow>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/stakeholder/stakeholder-table.tsx` around lines 248 - 255, The Delete DropdownMenuItem in the stakeholder-table.tsx file has no disabled state while the deletion mutation is pending, allowing users to trigger multiple delete requests by rapid clicking. Modify the DropdownMenuItem component that calls handleDeleteStakeholder to disable it when the delete mutation is pending. Check if there is a mutation hook being used for the delete operation and extract its pending/loading state, then pass this state to the disabled prop of the DropdownMenuItem to prevent additional clicks until the current mutation completes.src/components/reports/report-card.tsx (1)
15-20: ⚡ Quick winConstrain
typeto known report IDs to prevent UI/API drift.Using
type: stringallows invalid report IDs to compile and fail only at runtime. A literal union here gives immediate compile-time protection.Suggested fix
type ReportCardProps = { title: string; description: string; format: "PDF" | "CSV"; - type: string; + type: + | "captable-summary-pdf" + | "captable-csv" + | "securities-ledger-csv" + | "stakeholders-csv"; };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/reports/report-card.tsx` around lines 15 - 20, The type property in ReportCardProps is defined as a generic string, which allows invalid report IDs to pass type checking and only fail at runtime. Replace the type field definition from a simple string type to a literal union type that includes only the valid report ID values your application supports. This will provide compile-time validation and prevent UI/API drift by catching invalid report IDs immediately during type checking rather than at runtime.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/app/`(authenticated)/(dashboard)/[publicId]/captable/page.tsx:
- Around line 43-45: The current code nests an interactive Link element inside a
Button element, which violates HTML semantics and causes accessibility issues.
Restructure this so that the Link component wraps the Button component instead,
moving the href prop to the Link and the content text inside the Button. This
means the Link with href should be the parent element, and the Button with
size="lg" should be the child containing the "Issue shares" text.
In `@src/app/`(authenticated)/(dashboard)/[publicId]/page.tsx:
- Around line 14-27: The OverviewPage component is missing RBAC permission
checks before accessing sensitive cap-table data, unlike other sibling dashboard
routes. Add a serverAccessControl call after obtaining the session and before
invoking getOverviewData to verify the user has permission to access the
overview. If the user is unauthorized, return an appropriate unauthorized
component or error response to prevent unauthorized data access.
In `@src/components/securities/shares/steps/documents.tsx`:
- Around line 130-137: Remove the DialogClose wrapper around the Button
component in the documents step. Instead, move the modal close logic into the
handleComplete function so that the dialog only closes after the async operation
(upload/update) completes successfully. This prevents the modal from closing
prematurely if the mutation fails, allowing users to retain context and retry
without losing their progress.
In `@src/components/stakeholder/stakeholder-table.tsx`:
- Around line 196-207: The issue is that router.refresh() is called
unconditionally in the onSuccess callback, causing the page to refresh even when
the deletion fails (when success is false). To fix this, move the
router.refresh() call inside the if (success) block so that it only executes
when the deletion actually succeeds, preventing unexpected page refreshes when
an error occurs.
In `@src/pdf-templates/captable-template.tsx`:
- Around line 13-16: The Font.register call for the Oswald family is loading the
font from a remote URL, which creates a dependency on network availability and
external services. Replace the remote URL in the src property with a path to a
local font file that should be bundled with the project. Ensure the local font
file (TTF format) is available in your project's fonts directory and update the
src to reference the local path instead of the external Google Fonts URL.
- Around line 133-138: The numericColumns variable in the matrix width
calculation is incorrectly counting columns. The table actually renders
shareClasses plus 2 numeric columns (for options and fully diluted), but the
variable is set to shareClasses.length + 3, causing column widths to be
miscalculated. Update the numericColumns constant to use + 2 instead of + 3 to
match the actual number of numeric columns rendered in the matrix table.
In `@src/server/captable.ts`:
- Around line 99-114: The db.share.findMany() query in the captable calculations
is missing a filter to exclude inactive share records. Update the where clause
to add a condition that filters out cancelled or voided shares (check the schema
or other similar queries in the codebase to determine the correct status field
and value to exclude). This ensures that only active shares are included in
cap-table calculations for issued totals, fully-diluted percentages, and other
dashboard metrics.
In `@src/server/reports.ts`:
- Around line 130-131: The CSV exports using Papa.unparse are vulnerable to
formula injection attacks because user-controlled text (from stakeholder/company
fields) can start with formula prefixes (=, +, -, @) that execute when opened in
spreadsheet applications. Sanitize all CSV cell values in the three CSV
generators (at the Papa.unparse calls around lines 130-131, 228-229, and
284-285) by prefixing cells that start with these dangerous characters with a
single quote or similar escape mechanism to prevent formula interpretation.
Apply the same sanitization pattern consistently across all three locations.
In `@src/trpc/routers/securities-router/procedures/update-share.ts`:
- Around line 22-47: The update-share procedure accepts stakeholderId and
shareClassId directly from client input without validating that they belong to
the same companyId, which violates tenant isolation. Before constructing the
data object (where stakeholderId and shareClassId are assigned on lines 23-24),
add validation queries using the tx transaction to verify that both the
stakeholder and shareClass records exist and belong to the current companyId.
Only proceed with the share update if both validations pass, otherwise throw an
appropriate error.
In `@src/trpc/routers/securities-router/schema.ts`:
- Around line 87-89: The `id` field in ZodUpdateShareMutationSchema currently
uses z.string() which accepts empty or whitespace-only values, allowing invalid
data to reach the database. Replace z.string() with z.string().trim().min(1) to
ensure the id field rejects empty or whitespace-only values at validation time,
catching issues early before they reach runtime or database operations.
In `@src/trpc/routers/stakeholder-router/procedures/delete-stakeholder.ts`:
- Around line 52-53: The deleteMany operations on dataRoomRecipient and
updateRecipient execute before validating that the stakeholder belongs to the
current company, creating a cross-tenant data deletion vulnerability. Move the
stakeholder ownership authorization check (performed around line 29) to execute
before the deleteMany calls on lines 52-53, or add company ownership constraints
to each deleteMany operation by filtering through the relationship traversal
(e.g., ensuring dataRoom.companyId or update.companyId matches the session
company) to prevent deletion of records from other companies even if a malicious
stakeholderId is supplied.
---
Outside diff comments:
In `@src/components/dashboard/overview/empty.tsx`:
- Around line 22-27: The Button component wraps a Link component creating nested
interactive elements which breaks accessibility and keyboard navigation.
Refactor this by using the Button component's asChild prop to render the Link as
the button element itself, removing the nested structure. Move the href
attribute from the Link to the Button and place the text content directly in the
Button, ensuring only a single interactive element is rendered instead of nested
ones.
---
Nitpick comments:
In `@src/components/reports/report-card.tsx`:
- Around line 15-20: The type property in ReportCardProps is defined as a
generic string, which allows invalid report IDs to pass type checking and only
fail at runtime. Replace the type field definition from a simple string type to
a literal union type that includes only the valid report ID values your
application supports. This will provide compile-time validation and prevent
UI/API drift by catching invalid report IDs immediately during type checking
rather than at runtime.
In `@src/components/stakeholder/stakeholder-table.tsx`:
- Around line 189-220: The cell renderer in the stakeholder table is calling
React hooks (useRouter and useMutation) directly inside the column definition,
which violates React's rules of hooks. Extract the hook-dependent logic into a
separate React component named StakeholderActionsCell that accepts the row data
(singleStakeholder) as a prop, then move the useRouter and
deleteStakeholder.useMutation calls along with the handleDeleteStakeholder
function into this new component. Finally, replace the current cell function
with a call to the new StakeholderActionsCell component, passing the necessary
row data as props.
- Around line 248-255: The Delete DropdownMenuItem in the stakeholder-table.tsx
file has no disabled state while the deletion mutation is pending, allowing
users to trigger multiple delete requests by rapid clicking. Modify the
DropdownMenuItem component that calls handleDeleteStakeholder to disable it when
the delete mutation is pending. Check if there is a mutation hook being used for
the delete operation and extract its pending/loading state, then pass this state
to the disabled prop of the DropdownMenuItem to prevent additional clicks until
the current mutation completes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4e8a02e7-d210-45a6-9ca7-a1c2436244c5
📒 Files selected for processing (29)
src/app/(authenticated)/(dashboard)/[publicId]/captable/page.tsxsrc/app/(authenticated)/(dashboard)/[publicId]/page.tsxsrc/app/(authenticated)/(dashboard)/[publicId]/reports/page.tsxsrc/app/api/(internal)/reports/[type]/route.tssrc/components/captable/captable-matrix.tsxsrc/components/captable/convertibles-card.tsxsrc/components/dashboard/overview/donut-card.tsxsrc/components/dashboard/overview/empty.tsxsrc/components/dashboard/overview/summary-table.tsxsrc/components/modals/issue-share-modal.tsxsrc/components/reports/report-card.tsxsrc/components/securities/shares/share-table.tsxsrc/components/securities/shares/steps/contribution-details.tsxsrc/components/securities/shares/steps/documents.tsxsrc/components/securities/shares/steps/general-details.tsxsrc/components/securities/shares/steps/relevant-dates.tsxsrc/components/stakeholder/stakeholder-table.tsxsrc/pdf-templates/captable-template.tsxsrc/providers/add-share-form-provider.tsxsrc/server/captable.tssrc/server/overview.tssrc/server/reports.tssrc/trpc/routers/securities-router/procedures/get-shares.tssrc/trpc/routers/securities-router/procedures/update-share.tssrc/trpc/routers/securities-router/router.tssrc/trpc/routers/securities-router/schema.tssrc/trpc/routers/stakeholder-router/procedures/delete-stakeholder.tssrc/trpc/routers/stakeholder-router/router.tssrc/trpc/routers/stakeholder-router/schema.ts
Comment on lines
+43
to
+45
| <Button size="lg"> | ||
| <Link href={`/${publicId}/securities/shares`}>Issue shares</Link> | ||
| </Button> |
There was a problem hiding this comment.
Use Button as child wrapper for the link CTA.
This currently nests <Link> inside <Button>, which is invalid interactive nesting and can hurt accessibility/navigation behavior.
Suggested fix
- <Button size="lg">
- <Link href={`/${publicId}/securities/shares`}>Issue shares</Link>
+ <Button size="lg" asChild>
+ <Link href={`/${publicId}/securities/shares`}>Issue shares</Link>
</Button>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <Button size="lg"> | |
| <Link href={`/${publicId}/securities/shares`}>Issue shares</Link> | |
| </Button> | |
| <Button size="lg" asChild> | |
| <Link href={`/${publicId}/securities/shares`}>Issue shares</Link> | |
| </Button> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/app/`(authenticated)/(dashboard)/[publicId]/captable/page.tsx around
lines 43 - 45, The current code nests an interactive Link element inside a
Button element, which violates HTML semantics and causes accessibility issues.
Restructure this so that the Link component wraps the Button component instead,
moving the href prop to the Link and the content text inside the Button. This
means the Link with href should be the parent element, and the Button with
size="lg" should be the child containing the "Issue shares" text.
Comment on lines
+14
to
+27
| const OverviewPage = async ({ | ||
| params: { publicId }, | ||
| }: { | ||
| params: { publicId: string }; | ||
| }) => { | ||
| const session = await withServerComponentSession(); | ||
| const companyId = session?.user?.companyId; | ||
| const firstName = session?.user?.name?.split(" ")[0]; | ||
|
|
||
| const overview = companyId ? await getOverviewData(companyId) : null; | ||
|
|
||
| if (!overview || overview.isEmpty) { | ||
| return <EmptyOverview firstName={firstName} publicCompanyId={publicId} />; | ||
| } |
There was a problem hiding this comment.
Overview page is missing RBAC gating.
This route fetches and renders live cap-table-derived data without a permission check, unlike sibling dashboard routes in this PR. Add serverAccessControl + unauthorized guard before loading overview data.
Suggested fix
import EmptyOverview from "`@/components/dashboard/overview/empty`";
import SummaryTable from "`@/components/dashboard/overview/summary-table`";
import OverviewCard from "`@/components/dashboard/overview/top-card`";
+import { UnAuthorizedState } from "`@/components/ui/un-authorized-state`";
+import { serverAccessControl } from "`@/lib/rbac/access-control`";
import { withServerComponentSession } from "`@/server/auth`";
import { getOverviewData } from "`@/server/overview`";
import type { Metadata } from "next";
@@
const OverviewPage = async ({
params: { publicId },
}: {
params: { publicId: string };
}) => {
+ const { allow } = await serverAccessControl();
+ const canView = allow(true, ["stakeholder", "read"]);
+
+ if (!canView) {
+ return <UnAuthorizedState />;
+ }
+
const session = await withServerComponentSession();
const companyId = session?.user?.companyId;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/app/`(authenticated)/(dashboard)/[publicId]/page.tsx around lines 14 -
27, The OverviewPage component is missing RBAC permission checks before
accessing sensitive cap-table data, unlike other sibling dashboard routes. Add a
serverAccessControl call after obtaining the session and before invoking
getOverviewData to verify the user has permission to access the overview. If the
user is unauthorized, return an appropriate unauthorized component or error
response to prevent unauthorized data access.
Comment on lines
130
to
137
| <DialogClose asChild> | ||
| <Button | ||
| disabled={documentsList.length === 0} | ||
| disabled={!isEdit && documentsList.length === 0} | ||
| onClick={handleComplete} | ||
| > | ||
| Submit | ||
| {isEdit ? "Update share" : "Submit"} | ||
| </Button> | ||
| </DialogClose> |
There was a problem hiding this comment.
Prevent premature modal close during async submit.
DialogClose closes the modal immediately on click, before handleComplete finishes. If upload/update fails, users lose in-progress context and must restart. Close only after successful mutation.
Suggested fix
- <DialogClose asChild>
- <Button
- disabled={!isEdit && documentsList.length === 0}
- onClick={handleComplete}
- >
- {isEdit ? "Update share" : "Submit"}
- </Button>
- </DialogClose>
+ <Button
+ disabled={!isEdit && documentsList.length === 0}
+ onClick={async () => {
+ await handleComplete();
+ // close modal only after success (via your modal manager API)
+ }}
+ >
+ {isEdit ? "Update share" : "Submit"}
+ </Button>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/securities/shares/steps/documents.tsx` around lines 130 - 137,
Remove the DialogClose wrapper around the Button component in the documents
step. Instead, move the modal close logic into the handleComplete function so
that the dialog only closes after the async operation (upload/update) completes
successfully. This prevents the modal from closing prematurely if the mutation
fails, allowing users to retain context and retry without losing their progress.
Comment on lines
+196
to
+207
| onSuccess: ({ success, message }) => { | ||
| if (success) { | ||
| toast.success(message); | ||
| } else { | ||
| toast.error(message); | ||
| } | ||
| router.refresh(); | ||
| }, | ||
| onError: () => { | ||
| toast.error("Failed removing the investor. Please try again."); | ||
| }, | ||
| }); |
There was a problem hiding this comment.
router.refresh() is called even when deletion fails.
When success is false, the error toast is shown but router.refresh() still executes unconditionally. This may cause unexpected UX where the user sees an error but the page refreshes anyway.
🐛 Proposed fix
onSuccess: ({ success, message }) => {
if (success) {
toast.success(message);
+ router.refresh();
} else {
toast.error(message);
}
- router.refresh();
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| onSuccess: ({ success, message }) => { | |
| if (success) { | |
| toast.success(message); | |
| } else { | |
| toast.error(message); | |
| } | |
| router.refresh(); | |
| }, | |
| onError: () => { | |
| toast.error("Failed removing the investor. Please try again."); | |
| }, | |
| }); | |
| onSuccess: ({ success, message }) => { | |
| if (success) { | |
| toast.success(message); | |
| router.refresh(); | |
| } else { | |
| toast.error(message); | |
| } | |
| }, | |
| onError: () => { | |
| toast.error("Failed removing the investor. Please try again."); | |
| }, | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/stakeholder/stakeholder-table.tsx` around lines 196 - 207, The
issue is that router.refresh() is called unconditionally in the onSuccess
callback, causing the page to refresh even when the deletion fails (when success
is false). To fix this, move the router.refresh() call inside the if (success)
block so that it only executes when the deletion actually succeeds, preventing
unexpected page refreshes when an error occurs.
Comment on lines
+13
to
+16
| Font.register({ | ||
| family: "Oswald", | ||
| src: "https://fonts.gstatic.com/s/oswald/v13/Y_TKV6o8WovbUd3m_X9aAA.ttf", | ||
| }); |
There was a problem hiding this comment.
Avoid remote font loading in server-side PDF generation.
Registering the font from an external URL makes report generation dependent on network egress and third-party availability. In locked-down or degraded environments, PDF generation can fail or stall.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/pdf-templates/captable-template.tsx` around lines 13 - 16, The
Font.register call for the Oswald family is loading the font from a remote URL,
which creates a dependency on network availability and external services.
Replace the remote URL in the src property with a path to a local font file that
should be bundled with the project. Ensure the local font file (TTF format) is
available in your project's fonts directory and update the src to reference the
local path instead of the external Google Fonts URL.
Comment on lines
+99
to
+114
| db.share.findMany({ | ||
| where: { companyId }, | ||
| select: { | ||
| quantity: true, | ||
| capitalContribution: true, | ||
| shareClassId: true, | ||
| stakeholder: { | ||
| select: { | ||
| id: true, | ||
| name: true, | ||
| stakeholderType: true, | ||
| currentRelationship: true, | ||
| }, | ||
| }, | ||
| }, | ||
| }), |
There was a problem hiding this comment.
Filter inactive share records out of cap-table calculations.
This query currently includes all shares, while other instruments already exclude inactive statuses. Cancelled/voided shares will inflate issued totals, fully-diluted percentages, and raised-capital figures throughout dashboard/report outputs.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/server/captable.ts` around lines 99 - 114, The db.share.findMany() query
in the captable calculations is missing a filter to exclude inactive share
records. Update the where clause to add a condition that filters out cancelled
or voided shares (check the schema or other similar queries in the codebase to
determine the correct status field and value to exclude). This ensures that only
active shares are included in cap-table calculations for issued totals,
fully-diluted percentages, and other dashboard metrics.
Comment on lines
+130
to
+131
| body: Papa.unparse({ fields, data }), | ||
| contentType: "text/csv", |
There was a problem hiding this comment.
Sanitize CSV cells to prevent formula injection.
These exports include user-controlled text without guarding spreadsheet formula prefixes (=, +, -, @). A crafted stakeholder/company field can execute formulas when the CSV is opened.
Suggested fix pattern
+const sanitizeCsvCell = (value: unknown) => {
+ const s = String(value ?? "");
+ return /^[=+\-@]/.test(s) ? `'${s}` : s;
+};
+
+const sanitizeCsvRows = (rows: unknown[][]) =>
+ rows.map((row) => row.map(sanitizeCsvCell));
- body: Papa.unparse({ fields, data }),
+ body: Papa.unparse({ fields, data: sanitizeCsvRows(data) }),Apply the same sanitization to all three CSV generators.
Also applies to: 228-229, 284-285
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/server/reports.ts` around lines 130 - 131, The CSV exports using
Papa.unparse are vulnerable to formula injection attacks because user-controlled
text (from stakeholder/company fields) can start with formula prefixes (=, +, -,
@) that execute when opened in spreadsheet applications. Sanitize all CSV cell
values in the three CSV generators (at the Papa.unparse calls around lines
130-131, 228-229, and 284-285) by prefixing cells that start with these
dangerous characters with a single quote or similar escape mechanism to prevent
formula interpretation. Apply the same sanitization pattern consistently across
all three locations.
Comment on lines
+22
to
+47
| const data = { | ||
| stakeholderId: input.stakeholderId, | ||
| shareClassId: input.shareClassId, | ||
| status: input.status, | ||
| certificateId: input.certificateId, | ||
| quantity: input.quantity, | ||
| pricePerShare: input.pricePerShare, | ||
| capitalContribution: input.capitalContribution, | ||
| ipContribution: input.ipContribution, | ||
| debtCancelled: input.debtCancelled, | ||
| otherContributions: input.otherContributions, | ||
| cliffYears: input.cliffYears, | ||
| vestingYears: input.vestingYears, | ||
| companyLegends: input.companyLegends, | ||
| issueDate: new Date(input.issueDate), | ||
| rule144Date: new Date(input.rule144Date), | ||
| vestingStartDate: new Date(input.vestingStartDate), | ||
| boardApprovalDate: new Date(input.boardApprovalDate), | ||
| }; | ||
|
|
||
| const share = await tx.share.update({ | ||
| where: { | ||
| id: input.id, | ||
| companyId, | ||
| }, | ||
| data, |
There was a problem hiding this comment.
Enforce tenant ownership checks for reassigned foreign IDs.
stakeholderId and shareClassId are written directly from client input without verifying they belong to the same companyId. That can break tenant isolation by linking a company’s share to another company’s stakeholder/share class (Lines 23-24, 42-47).
Suggested fix
await ctx.db.$transaction(async (tx) => {
const { companyId } = await checkMembership({
session: ctx.session,
tx,
});
+
+ const [stakeholder, shareClass] = await Promise.all([
+ tx.stakeholder.findFirst({
+ where: { id: input.stakeholderId, companyId },
+ select: { id: true },
+ }),
+ tx.shareClass.findFirst({
+ where: { id: input.shareClassId, companyId },
+ select: { id: true },
+ }),
+ ]);
+
+ if (!stakeholder || !shareClass) {
+ throw new Error("Invalid stakeholder or share class for this company");
+ }
const data = {
stakeholderId: input.stakeholderId,
shareClassId: input.shareClassId,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const data = { | |
| stakeholderId: input.stakeholderId, | |
| shareClassId: input.shareClassId, | |
| status: input.status, | |
| certificateId: input.certificateId, | |
| quantity: input.quantity, | |
| pricePerShare: input.pricePerShare, | |
| capitalContribution: input.capitalContribution, | |
| ipContribution: input.ipContribution, | |
| debtCancelled: input.debtCancelled, | |
| otherContributions: input.otherContributions, | |
| cliffYears: input.cliffYears, | |
| vestingYears: input.vestingYears, | |
| companyLegends: input.companyLegends, | |
| issueDate: new Date(input.issueDate), | |
| rule144Date: new Date(input.rule144Date), | |
| vestingStartDate: new Date(input.vestingStartDate), | |
| boardApprovalDate: new Date(input.boardApprovalDate), | |
| }; | |
| const share = await tx.share.update({ | |
| where: { | |
| id: input.id, | |
| companyId, | |
| }, | |
| data, | |
| const [stakeholder, shareClass] = await Promise.all([ | |
| tx.stakeholder.findFirst({ | |
| where: { id: input.stakeholderId, companyId }, | |
| select: { id: true }, | |
| }), | |
| tx.shareClass.findFirst({ | |
| where: { id: input.shareClassId, companyId }, | |
| select: { id: true }, | |
| }), | |
| ]); | |
| if (!stakeholder || !shareClass) { | |
| throw new Error("Invalid stakeholder or share class for this company"); | |
| } | |
| const data = { | |
| stakeholderId: input.stakeholderId, | |
| shareClassId: input.shareClassId, | |
| status: input.status, | |
| certificateId: input.certificateId, | |
| quantity: input.quantity, | |
| pricePerShare: input.pricePerShare, | |
| capitalContribution: input.capitalContribution, | |
| ipContribution: input.ipContribution, | |
| debtCancelled: input.debtCancelled, | |
| otherContributions: input.otherContributions, | |
| cliffYears: input.cliffYears, | |
| vestingYears: input.vestingYears, | |
| companyLegends: input.companyLegends, | |
| issueDate: new Date(input.issueDate), | |
| rule144Date: new Date(input.rule144Date), | |
| vestingStartDate: new Date(input.vestingStartDate), | |
| boardApprovalDate: new Date(input.boardApprovalDate), | |
| }; | |
| const share = await tx.share.update({ | |
| where: { | |
| id: input.id, | |
| companyId, | |
| }, | |
| data, |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/trpc/routers/securities-router/procedures/update-share.ts` around lines
22 - 47, The update-share procedure accepts stakeholderId and shareClassId
directly from client input without validating that they belong to the same
companyId, which violates tenant isolation. Before constructing the data object
(where stakeholderId and shareClassId are assigned on lines 23-24), add
validation queries using the tx transaction to verify that both the stakeholder
and shareClass records exist and belong to the current companyId. Only proceed
with the share update if both validations pass, otherwise throw an appropriate
error.
Comment on lines
+87
to
+89
| export const ZodUpdateShareMutationSchema = ZodAddShareMutationSchema.extend({ | ||
| id: z.string(), | ||
| }); |
There was a problem hiding this comment.
Tighten id validation for update payloads.
id: z.string() accepts empty/whitespace values, which pushes invalid input to runtime/DB errors instead of failing fast at validation (Line 88).
Suggested fix
export const ZodUpdateShareMutationSchema = ZodAddShareMutationSchema.extend({
- id: z.string(),
+ id: z.string().trim().min(1, "Share id is required"),
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| export const ZodUpdateShareMutationSchema = ZodAddShareMutationSchema.extend({ | |
| id: z.string(), | |
| }); | |
| export const ZodUpdateShareMutationSchema = ZodAddShareMutationSchema.extend({ | |
| id: z.string().trim().min(1, "Share id is required"), | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/trpc/routers/securities-router/schema.ts` around lines 87 - 89, The `id`
field in ZodUpdateShareMutationSchema currently uses z.string() which accepts
empty or whitespace-only values, allowing invalid data to reach the database.
Replace z.string() with z.string().trim().min(1) to ensure the id field rejects
empty or whitespace-only values at validation time, catching issues early before
they reach runtime or database operations.
Comment on lines
+52
to
+53
| await tx.dataRoomRecipient.deleteMany({ where: { stakeholderId } }); | ||
| await tx.updateRecipient.deleteMany({ where: { stakeholderId } }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if dataRoomRecipient and updateRecipient models have companyId field
rg -n "model (dataRoomRecipient|updateRecipient|DataRoomRecipient|UpdateRecipient)" -A 30 --type-add 'prisma:*.prisma' --type prismaRepository: captableinc/captable
Length of output: 2874
Cross-tenant data deletion risk: Verify stakeholder ownership before deleteMany operations.
The deleteMany calls on lines 52-53 execute before validating that the stakeholder belongs to the session's company. If an attacker supplies a stakeholderId from a different company, these operations would delete that company's recipient records, with the authorization check failing only on line 55-58.
However, the proposed fix referencing companyId cannot be applied—neither DataRoomRecipient nor UpdateRecipient models contain a companyId field. Instead, validate stakeholder ownership (via the company check already performed on line 29) before executing any deleteMany operations, or add constraint checks that traverse the relationship to the owning company (e.g., via dataRoom.companyId or update.companyId).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/trpc/routers/stakeholder-router/procedures/delete-stakeholder.ts` around
lines 52 - 53, The deleteMany operations on dataRoomRecipient and
updateRecipient execute before validating that the stakeholder belongs to the
current company, creating a cross-tenant data deletion vulnerability. Move the
stakeholder ownership authorization check (performed around line 29) to execute
before the deleteMany calls on lines 52-53, or add company ownership constraints
to each deleteMany operation by filtering through the relationship traversal
(e.g., ensuring dataRoom.companyId or update.companyId matches the session
company) to prevent deletion of records from other companies even if a malicious
stakeholderId is supplied.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR fills in the Cap table management (wip) experience from the README by wiring the dashboard, cap table, and reports to live data. It bundles three closely-related changes:
1. Overview dashboard with live data
feat: wire Overview dashboard to real cap-table datasrc/server/overview.tsto compute real ownership / summary figures from the database.2. Cap table & Reports pages
feat: implement Cap table and Reports pagescaptable-matrix.tsx) and a convertibles card (convertibles-card.tsx), backed bysrc/server/captable.ts.report-card.tsx) backed bysrc/server/reports.ts, an internal API route (/api/reports/[type]), and a PDF export template (src/pdf-templates/captable-template.tsx).3. Edit shares & remove stakeholders
fix(securities): enable updating shares and removing investorsupdate-sharetRPC procedure so existing shares can be edited.delete-stakeholdertRPC procedure so stakeholders/investors can be removed.Notes
Testing
pnpm build(Next.js production build) passes cleanly on top of the latestmain; all routes compile and static pages generate.Summary by CodeRabbit
New Features
Bug Fixes