Conversation
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Sorry @Connorbelez, your pull request is larger than the review limit of 150000 diff characters
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
Connorbelez
changed the title
Connorbelez
force-pushed
the
codex/eng-302-explicit-borrower-portal-attribution
branch
from
47e7566 to
4890820
Compare
Connorbelez
changed the base branch from
graphite-base/440
to
codex/eng-301-portal-listings
There was a problem hiding this comment.
Sorry @Connorbelez, you have reached your weekly rate limit of 500000 diff characters.
Please try again later or upgrade to continue using Sourcery
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
There was a problem hiding this comment.
Pull request overview
Implements ENG-302 by introducing explicit portalId attribution on borrowers and onboardingRequests, wiring that attribution through onboarding, origination write paths, seeds, and portal middleware, and adding deterministic backfill + status reporting to support safe cutover.
Changes:
- Add
portalIdfields + indexes toborrowersandonboardingRequests, and enforce borrower access viaborrowers.portalId === ctx.portal.portalId. - Persist trusted portal attribution on onboarding requests and propagate/repair borrower portal attribution through origination + seed paths (fail-closed when unresolved for live origination).
- Add deterministic backfill helpers and extend portal registry backfill/status reporting; update targeted tests and ENG-302 spec artifacts.
Reviewed changes
Copilot reviewed 39 out of 40 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/test/convex/seed/seedAll.test.ts | Verifies seed reruns repair missing portalId on borrowers/onboardingRequests and asserts FairLend portal attribution. |
| src/test/convex/onboarding/onboarding.test.ts | Adds coverage that onboarding requests persist portal attribution derived from trusted users.homePortalId. |
| src/test/convex/engine/transition.test.ts | Updates fixture seeding to include portalId on onboarding requests. |
| src/test/convex/engine/hash-chain-reconciliation.test.ts | Updates onboarding request fixtures to include portalId. |
| src/test/convex/admin/origination/commit.test.ts | Adds regression coverage for stale onboarding vs current broker portal and fail-closed when broker portal missing. |
| convex/seed/seedOnboardingRequest.ts | Seeds/repairs onboarding requests with ensured FairLend portal attribution. |
| convex/seed/seedBorrower.ts | Seeds/repairs borrowers with ensured FairLend portal attribution. |
| convex/schema.ts | Adds optional portalId + portal-scoped indexes for borrowers and onboarding requests. |
| convex/portals/middleware.ts | Cuts borrower portal enforcement over to explicit borrowers.portalId matching. |
| convex/portals/homePortalAssignment.ts | Prefers explicit borrower portalId for home-portal resolution with deterministic org fallback. |
| convex/portals/borrowerPortalAttribution.ts | Introduces shared helpers for deterministic portal attribution (write + backfill). |
| convex/portals/tests/registry.test.ts | Extends registry backfill tests to cover onboarding/borrower portal attribution and unresolved reporting. |
| convex/portals/tests/middleware.test.ts | Updates portal middleware proof coverage to reflect explicit borrower portalId enforcement. |
| convex/onboarding/mutations.ts | Persists onboardingRequests.portalId based on trusted, live users.homePortalId portal. |
| convex/brokers/migrations.ts | Adds backfills for onboardingRequest/borrower portalId, returns richer status snapshot, and reports unresolved IDs. |
| convex/borrowers/resolveOrProvisionForOrigination.ts | Ensures canonical borrower provisioning resolves and persists portal attribution (fail-closed when unresolved). |
| convex/admin/origination/commit.ts | Pins live origination borrower attribution to broker portal and repairs/validates staged borrower links. |
| convex/admin/origination/collections.ts | Plumbs portal attribution into collections-driven borrower creation and fails closed when missing. |
| convex/_generated/api.d.ts | Codegen update to include new portals attribution module exports. |
| specs/ENG-302/* (multiple) | Adds/updates ENG-302 execution artifacts (tasks, status, checklist, chunk docs, audit). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| async function getPortalRegistryBackfillStatusSnapshot(ctx: PortalReaderCtx) { | ||
| const fairLendPortal = await getPortalBySlug(ctx, FAIRLEND_PORTAL_SLUG); | ||
| const brokers = await ctx.db.query("brokers").collect(); | ||
| const onboardingRequests = await ctx.db.query("onboardingRequests").collect(); | ||
| const borrowers = await ctx.db.query("borrowers").collect(); | ||
| const users = await ctx.db.query("users").collect(); | ||
| const portals = await ctx.db.query("portals").collect(); | ||
|
|
||
| const brokerPortal = await getPortalByBrokerId(ctx, broker._id); | ||
| if (!brokerPortal) { | ||
| brokersMissingPortalCount += 1; | ||
| } | ||
| let brokersMissingOrgIdCount = 0; | ||
| let brokersMissingPortalCount = 0; | ||
| for (const broker of brokers) { | ||
| if (!broker.orgId) { | ||
| brokersMissingOrgIdCount += 1; | ||
| continue; | ||
| } | ||
|
|
||
| let usersMissingHomePortalCount = 0; | ||
| for (const user of users) { | ||
| if (user.homePortalId) { | ||
| continue; | ||
| } | ||
| usersMissingHomePortalCount += 1; | ||
| const brokerPortal = await getPortalByBrokerId(ctx, broker._id); | ||
| if (!brokerPortal) { | ||
| brokersMissingPortalCount += 1; | ||
| } | ||
| } | ||
|
|
||
| return { | ||
| fairLendPortalExists: fairLendPortal !== null, | ||
| portalCount: portals.length, | ||
| brokerPortalCount: portals.filter( | ||
| (portal) => portal.portalType === "broker" | ||
| ).length, | ||
| brokersMissingOrgIdCount, | ||
| brokersMissingPortalCount, | ||
| usersMissingHomePortalCount, | ||
| }; | ||
| }) | ||
| const onboardingRequestsMissingPortal = onboardingRequests.filter( | ||
| (request) => !request.portalId | ||
| ); | ||
| const unresolvedOnboardingRequestIds: Id<"onboardingRequests">[] = []; | ||
| for (const onboardingRequest of onboardingRequestsMissingPortal) { | ||
| const resolvedPortalId = await resolveOnboardingRequestPortalIdForBackfill( | ||
| ctx, | ||
| onboardingRequest | ||
| ); | ||
| if (!resolvedPortalId) { | ||
| unresolvedOnboardingRequestIds.push(onboardingRequest._id); | ||
| } | ||
| } | ||
|
|
||
| const borrowersMissingPortal = borrowers.filter( | ||
| (borrower) => !borrower.portalId | ||
| ); | ||
| const unresolvedBorrowerIds: Id<"borrowers">[] = []; | ||
| for (const borrower of borrowersMissingPortal) { | ||
| const resolvedPortalId = await resolveBorrowerPortalIdForBackfill( | ||
| ctx, | ||
| borrower | ||
| ); | ||
| if (!resolvedPortalId) { |
| const attributedRequests = requests | ||
| .filter( | ||
| (request): request is typeof request & { portalId: Id<"portals"> } => | ||
| request.portalId !== undefined | ||
| ) | ||
| .sort((left, right) => { | ||
| if (left.createdAt !== right.createdAt) { | ||
| return right.createdAt - left.createdAt; | ||
| } | ||
| return String(right._id).localeCompare(String(left._id)); | ||
| }); | ||
|
|
||
| return attributedRequests[0]?.portalId; |
| .index("by_user", ["userId"]) | ||
| .index("by_portal", ["portalId"]) | ||
| .index("by_portal_status", ["portalId", "status"]) | ||
| .index("by_status", ["status"]) | ||
| .index("by_user_and_status", ["userId", "status"]), |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
convex/portals/middleware.ts (1)
145-153:⚠️ Potential issue | 🔴 CriticalUse the portal-scoped index to resolve the borrower deterministically.
The current implementation calls
getBorrowerByAuthId, which queriesborrowers.by_userand returns.first(). For a user with multiple borrower rows in different portals, this is non-deterministic and will false-deny access to any portal not matched by the first row. The schema definesborrowers.by_portal_userfor exactly this case. Resolve the borrower directly through the portal-scoped index:Proposed refactor
export async function resolvePortalBorrower( context: PortalAuthedBaseContext & PortalResolvedContext ): Promise<Doc<"borrowers">> { - const borrower = await getBorrowerByAuthId(context, context.viewer.authId); - if (!borrower?.portalId || borrower.portalId !== context.portal.portalId) { + const viewerUser = await getUserByAuthId(context, context.viewer.authId); + if (!viewerUser) { + throw new ConvexError("Forbidden: borrower does not belong to this portal"); + } + + const borrower = await context.db + .query("borrowers") + .withIndex("by_portal_user", (q) => + q.eq("portalId", context.portal.portalId).eq("userId", viewerUser._id) + ) + .first(); + if (!borrower) { throw new ConvexError("Forbidden: borrower does not belong to this portal"); } return borrower; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@convex/portals/middleware.ts` around lines 145 - 153, The resolvePortalBorrower function currently uses getBorrowerByAuthId which queries borrowers.by_user and .first(), causing nondeterministic results when a user has borrower rows in multiple portals; change resolvePortalBorrower to query the portal-scoped index borrowers.by_portal_user (lookup by context.portal.portalId and context.viewer.authId) so you deterministically fetch the borrower for this portal, keep the same Forbidden ConvexError when not found or portal mismatch, and return the borrower document.
🧹 Nitpick comments (5)
src/test/convex/onboarding/onboarding.test.ts (1)
106-129: LGTM — trusted home-portal attribution covered.The new case correctly seeds a portal via
fairLendPortalFields(which setsstatus: "active"andisPublished: true, satisfying the mutation's persistence gate atconvex/onboarding/mutations.ts:115-123) and asserts the request carries the trustedportalId.One small gap worth considering as a follow-up: there is no negative companion test asserting that
portalIdis left undefined when the user'shomePortalIdpoints at an inactive or unpublished portal. That branch is the other half of the gate and is currently uncovered here.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/convex/onboarding/onboarding.test.ts` around lines 106 - 129, Add a negative test that mirrors the existing positive case but seeds a portal that fails the persistence gate (use fairLendPortalFields with status set to an inactive value or isPublished: false), patch the seeded portal id onto the user's homePortalId using seedFromIdentity/MEMBER and the test helper t, call the same mutation api.onboarding.mutations.requestRole via t.withIdentity(MEMBER) and then assert that the persisted request (via ctx.db.get(requestId)) has request.portalId === undefined; reference the existing test structure (createTestConvex, seedFromIdentity, fairLendPortalFields, MEMBER, api.onboarding.mutations.requestRole) to keep the new test consistent with the positive case.convex/seed/seedBorrower.ts (1)
200-222: Seed repair logic looks correct.
ensureFairLendPortalis idempotent (perconvex/portals/homePortalAssignment.ts:61-101), so hoisting it out of the loop is fine, and the conditional patch on!existingBorrower.portalIdproperly repairs legacy rows without churning those already attributed.Minor observation: this assumes every seeded borrower belongs to the FairLend portal. That's true today (all fixtures use
FAIRLEND_STAFF_ORG_ID), but if future fixtures introduce a differentorgId, the repair on reused borrowers would silently attribute them to the FairLend portal regardless of their org. Worth a brief comment anchoring that invariant, or gating the patch onorgId === FAIRLEND_STAFF_ORG_ID.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@convex/seed/seedBorrower.ts` around lines 200 - 222, The loop currently hoists ensureFairLendPortal and unconditionally patches reused borrowers lacking portalId, which can misattribute borrowers if future BORROWER_FIXTURES use a different org; update the repair to only patch when existingBorrower.portalId is falsy AND existingBorrower.orgId === FAIRLEND_STAFF_ORG_ID (or otherwise matches the fixture org), and add a short comment near ensureFairLendPortal / the patch mentioning the invariant that current fixtures use FAIRLEND_STAFF_ORG_ID so only FairLend borrowers are auto-attributed; reference ensureFairLendPortal, BORROWER_FIXTURES, findBorrowerByUserId, existingBorrower.portalId and FAIRLEND_STAFF_ORG_ID to locate the change.convex/portals/__tests__/middleware.test.ts (1)
437-456: Two assertions now collapse to the same code path — consider renaming to clarify intent.Post-cutover, both
MISSING_ORG_BORROWER(noorgId, noportalId) andUNMAPPED_ORG_BORROWER(hasorgIdbut noportalId) exercise exactly the!borrower?.portalIdbranch inresolvePortalBorrower. That's fine and arguably documents "the old org-based fallback no longer grants access", but as-is a reader might think these are two distinct denial reasons.Consider either:
- Renaming the
it(...)block to reflect that both rows lack explicit attribution (e.g. "denies borrowers lacking explicitportalIdregardless of orgId shape"), or- Dropping
UNMAPPED_ORG_BORROWERfrom this file since its signal is now fully captured byMISSING_ORG_BORROWERandMISMATCHED_BORROWER.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@convex/portals/__tests__/middleware.test.ts` around lines 437 - 456, The test name and assertions are confusing because both MISSING_ORG_BORROWER and UNMAPPED_ORG_BORROWER now hit the same !borrower?.portalId branch in resolvePortalBorrower; either rename the it(...) description to reflect that the test asserts denial for borrowers lacking an explicit portalId regardless of orgId shape (e.g. "denies borrowers lacking explicit portalId regardless of orgId") or remove UNMAPPED_ORG_BORROWER from this test and rely on MISSING_ORG_BORROWER plus the existing MISMATCHED_BORROWER case to cover distinct denial reasons so the intent is clear.convex/onboarding/mutations.ts (1)
115-122: LGTM — trusted server-side portal derivation.Resolving
portalIdexclusively fromuser.homePortalId(and only when the portal isactive+isPublished) correctly removes the caller-controlled input surface. The value is consistently threaded into the request and audit payloads.When a user has a
homePortalIdbut the portal isdraft/suspended/archivedor unpublished, the request silently persists without portal attribution rather than failing—consistent with the "fail-open on onboarding, fail-closed on live origination" approach noted in the PR description. If you want visibility into such configuration drift, consider emitting a diagnostic audit field (e.g.,portalResolution: "home_portal_inactive") for observability.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@convex/onboarding/mutations.ts` around lines 115 - 122, Add a diagnostic audit field when a user has a homePortalId but the resolved homePortal is inactive or unpublished: in the portal derivation block (variables homePortal, portalId) set a portalResolution value such as "home_portal_active" when portalId is set, "home_portal_inactive" when homePortal exists but portal.status !== "active" or !homePortal.isPublished, and "none" when no homePortalId; thread that portalResolution into the same request/audit payloads you already pass portalId into so observers can detect configuration drift.convex/admin/origination/commit.ts (1)
416-438:portalIdin commit context is advisory only — consider flagging missing broker portal as a validation error.
getCommitContextresolvesportalIdtonullwhen the broker-of-record has no portal registered, butvalidationErrors(viacollectCommitBlockingErrors) only checks thatbrokerOfRecordIdis present, not that a portal exists for it. The commit will still fail-closed insidebuildBorrowerLinksForCommitwith"Broker of record portal no longer exists", but callers readingcommitContextto drive UI readiness (e.g. thecommitCaseaction usingcommitContext.validationErrors.length) won't surface the issue until the finalize step throws. Surfacing this invalidationErrorswhenbrokerOfRecordId != null && portalId == nullwould give operators a clearer pre-commit signal.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@convex/admin/origination/commit.ts` around lines 416 - 438, getCommitContext currently sets portalId to null when a brokerOfRecordId has no portal, but collectCommitBlockingErrors only checks for brokerOfRecordId presence so UI readiness can miss this failure; update collectCommitBlockingErrors (or add a new check invoked from getCommitContext) to add a validation error when brokerOfRecordId != null && portalId == null (e.g., an error like "Broker of record portal no longer exists") so the returned commit context's validationErrors reflects the missing portal before buildBorrowerLinksForCommit runs; locate getCommitContext, portalId, collectCommitBlockingErrors, and buildBorrowerLinksForCommit in the diff to implement the check and return the error in validationErrors.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@convex/admin/origination/collections.ts`:
- Around line 1247-1269: The code is using ctx.viewer.orgId when creating a
borrower but the commit/case context (commitContext from
internal.admin.origination.commit.getCommitContext) contains the correct portal
and org for the case; update the createCanonicalBorrowerProfileRef call to use
the case/org from commitContext (thread the org id returned by getCommitContext
alongside portalId) instead of ctx.viewer.orgId so the borrower is created in
the case's org while keeping portalId = commitContext.portalId.
In `@convex/admin/origination/commit.ts`:
- Around line 306-320: When handling the staged-borrower branch in
commitOrigination where you call ensureBorrowerPortalAttribution (triggered when
participant.borrowerId and existingBorrower exists), after pushing to
borrowerLinks also call syncUserHomePortalAssignmentByUserId with the
attributedBorrower.userId to refresh the user's homePortalId (mirror what
ensureCanonicalBorrowerForOrigination does); also add the import for
syncUserHomePortalAssignmentByUserId from ../../portals/homePortalAssignment
alongside getPortalByBrokerId so the sync runs immediately after attribution and
avoids stale homePortalId state.
In `@convex/portals/borrowerPortalAttribution.ts`:
- Around line 51-75: When resolving a portal for writes in
resolveBorrowerPortalIdForWrite, don’t fall back to org attribution if a
brokerId is supplied but getPortalByBrokerId(ctx, brokerId) returns no portal —
instead fail closed (return undefined) to avoid misattributing broker-originated
borrowers; additionally, when args.explicitPortalId is provided, validate it by
loading that portal (e.g., via a portal lookup like getPortalById or equivalent)
and ensure the portal’s brokerId or orgId matches the supplied args.brokerId or
args.orgId before accepting it, otherwise reject it (return undefined).
In `@specs/ENG-302/chunks/chunk-01-schema-onboarding/status.md`:
- Around line 15-17: Update the status doc to reflect the final requestRole
mutation contract: remove the stale wording that it "accepts optional trusted
portalId" and change the description so it states requestRole does NOT accept a
portalId argument, that it reads the trusted user.homePortalId server-side, and
only persists that portalId on the onboarding request and audit payloads when
the portal is active and isPublished (i.e., check portal.active &&
portal.isPublished); also change the phrase "`requestRole` currently has no
portal context input" to indicate that this is the settled final state rather
than an open design question.
---
Outside diff comments:
In `@convex/portals/middleware.ts`:
- Around line 145-153: The resolvePortalBorrower function currently uses
getBorrowerByAuthId which queries borrowers.by_user and .first(), causing
nondeterministic results when a user has borrower rows in multiple portals;
change resolvePortalBorrower to query the portal-scoped index
borrowers.by_portal_user (lookup by context.portal.portalId and
context.viewer.authId) so you deterministically fetch the borrower for this
portal, keep the same Forbidden ConvexError when not found or portal mismatch,
and return the borrower document.
---
Nitpick comments:
In `@convex/admin/origination/commit.ts`:
- Around line 416-438: getCommitContext currently sets portalId to null when a
brokerOfRecordId has no portal, but collectCommitBlockingErrors only checks for
brokerOfRecordId presence so UI readiness can miss this failure; update
collectCommitBlockingErrors (or add a new check invoked from getCommitContext)
to add a validation error when brokerOfRecordId != null && portalId == null
(e.g., an error like "Broker of record portal no longer exists") so the returned
commit context's validationErrors reflects the missing portal before
buildBorrowerLinksForCommit runs; locate getCommitContext, portalId,
collectCommitBlockingErrors, and buildBorrowerLinksForCommit in the diff to
implement the check and return the error in validationErrors.
In `@convex/onboarding/mutations.ts`:
- Around line 115-122: Add a diagnostic audit field when a user has a
homePortalId but the resolved homePortal is inactive or unpublished: in the
portal derivation block (variables homePortal, portalId) set a portalResolution
value such as "home_portal_active" when portalId is set, "home_portal_inactive"
when homePortal exists but portal.status !== "active" or
!homePortal.isPublished, and "none" when no homePortalId; thread that
portalResolution into the same request/audit payloads you already pass portalId
into so observers can detect configuration drift.
In `@convex/portals/__tests__/middleware.test.ts`:
- Around line 437-456: The test name and assertions are confusing because both
MISSING_ORG_BORROWER and UNMAPPED_ORG_BORROWER now hit the same
!borrower?.portalId branch in resolvePortalBorrower; either rename the it(...)
description to reflect that the test asserts denial for borrowers lacking an
explicit portalId regardless of orgId shape (e.g. "denies borrowers lacking
explicit portalId regardless of orgId") or remove UNMAPPED_ORG_BORROWER from
this test and rely on MISSING_ORG_BORROWER plus the existing MISMATCHED_BORROWER
case to cover distinct denial reasons so the intent is clear.
In `@convex/seed/seedBorrower.ts`:
- Around line 200-222: The loop currently hoists ensureFairLendPortal and
unconditionally patches reused borrowers lacking portalId, which can
misattribute borrowers if future BORROWER_FIXTURES use a different org; update
the repair to only patch when existingBorrower.portalId is falsy AND
existingBorrower.orgId === FAIRLEND_STAFF_ORG_ID (or otherwise matches the
fixture org), and add a short comment near ensureFairLendPortal / the patch
mentioning the invariant that current fixtures use FAIRLEND_STAFF_ORG_ID so only
FairLend borrowers are auto-attributed; reference ensureFairLendPortal,
BORROWER_FIXTURES, findBorrowerByUserId, existingBorrower.portalId and
FAIRLEND_STAFF_ORG_ID to locate the change.
In `@src/test/convex/onboarding/onboarding.test.ts`:
- Around line 106-129: Add a negative test that mirrors the existing positive
case but seeds a portal that fails the persistence gate (use
fairLendPortalFields with status set to an inactive value or isPublished:
false), patch the seeded portal id onto the user's homePortalId using
seedFromIdentity/MEMBER and the test helper t, call the same mutation
api.onboarding.mutations.requestRole via t.withIdentity(MEMBER) and then assert
that the persisted request (via ctx.db.get(requestId)) has request.portalId ===
undefined; reference the existing test structure (createTestConvex,
seedFromIdentity, fairLendPortalFields, MEMBER,
api.onboarding.mutations.requestRole) to keep the new test consistent with the
positive case.
🪄 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: d3cd3a7f-7fa9-49a9-ac51-11c0bf4cbdfd
⛔ Files ignored due to path filters (1)
convex/_generated/api.d.tsis excluded by!**/_generated/**
📒 Files selected for processing (39)
convex/admin/origination/collections.tsconvex/admin/origination/commit.tsconvex/borrowers/resolveOrProvisionForOrigination.tsconvex/brokers/migrations.tsconvex/onboarding/mutations.tsconvex/portals/__tests__/middleware.test.tsconvex/portals/__tests__/registry.test.tsconvex/portals/borrowerPortalAttribution.tsconvex/portals/homePortalAssignment.tsconvex/portals/middleware.tsconvex/schema.tsconvex/seed/seedBorrower.tsconvex/seed/seedOnboardingRequest.tsspecs/ENG-302/audit.mdspecs/ENG-302/chunks/chunk-01-schema-onboarding/context.mdspecs/ENG-302/chunks/chunk-01-schema-onboarding/status.mdspecs/ENG-302/chunks/chunk-01-schema-onboarding/tasks.mdspecs/ENG-302/chunks/chunk-02-borrower-write-paths/context.mdspecs/ENG-302/chunks/chunk-02-borrower-write-paths/status.mdspecs/ENG-302/chunks/chunk-02-borrower-write-paths/tasks.mdspecs/ENG-302/chunks/chunk-03-backfill-home-portal/context.mdspecs/ENG-302/chunks/chunk-03-backfill-home-portal/status.mdspecs/ENG-302/chunks/chunk-03-backfill-home-portal/tasks.mdspecs/ENG-302/chunks/chunk-04-middleware-tests/context.mdspecs/ENG-302/chunks/chunk-04-middleware-tests/status.mdspecs/ENG-302/chunks/chunk-04-middleware-tests/tasks.mdspecs/ENG-302/chunks/chunk-05-validation-audit/context.mdspecs/ENG-302/chunks/chunk-05-validation-audit/status.mdspecs/ENG-302/chunks/chunk-05-validation-audit/tasks.mdspecs/ENG-302/chunks/manifest.mdspecs/ENG-302/execution-checklist.mdspecs/ENG-302/status.mdspecs/ENG-302/summary.mdspecs/ENG-302/tasks.mdsrc/test/convex/admin/origination/commit.test.tssrc/test/convex/engine/hash-chain-reconciliation.test.tssrc/test/convex/engine/transition.test.tssrc/test/convex/onboarding/onboarding.test.tssrc/test/convex/seed/seedAll.test.ts
| const commitContext: { portalId: Id<"portals"> | null } | null = | ||
| await ctx.runQuery(internal.admin.origination.commit.getCommitContext, { | ||
| caseId: args.caseId, | ||
| viewerAuthId: ctx.viewer.authId, | ||
| viewerIsFairLendAdmin: ctx.viewer.isFairLendAdmin, | ||
| viewerOrgId: ctx.viewer.orgId, | ||
| } | ||
| ); | ||
| }); | ||
| if (!commitContext) { | ||
| throw new ConvexError("Origination case not found"); | ||
| } | ||
| if (!commitContext.portalId) { | ||
| throw new ConvexError( | ||
| "Borrower portal attribution could not be resolved" | ||
| ); | ||
| } | ||
|
|
||
| return ctx.runAction(createCanonicalBorrowerProfileRef, { | ||
| accountNumber: args.accountNumber, | ||
| email: args.email, | ||
| fullName: args.fullName, | ||
| institutionNumber: args.institutionNumber, | ||
| orgId: ctx.viewer.orgId, | ||
| portalId: commitContext.portalId, |
There was a problem hiding this comment.
Use the case/portal org, not the viewer org, when creating the borrower.
portalId is now resolved from the commit/case context, but orgId still comes from ctx.viewer.orgId. For staff or cross-org origination, this can create a borrower in the viewer’s org while attributing it to the broker/case portal. Thread the case org from getCommitContext and use it with the resolved portal.
Proposed direction
- const commitContext: { portalId: Id<"portals"> | null } | null =
+ const commitContext: {
+ orgId?: string;
+ portalId: Id<"portals"> | null;
+ } | null =
await ctx.runQuery(internal.admin.origination.commit.getCommitContext, {
caseId: args.caseId,
viewerAuthId: ctx.viewer.authId,
viewerIsFairLendAdmin: ctx.viewer.isFairLendAdmin,
viewerOrgId: ctx.viewer.orgId,
});
@@
fullName: args.fullName,
institutionNumber: args.institutionNumber,
- orgId: ctx.viewer.orgId,
+ orgId: commitContext.orgId,
portalId: commitContext.portalId,
phone: args.phone,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@convex/admin/origination/collections.ts` around lines 1247 - 1269, The code
is using ctx.viewer.orgId when creating a borrower but the commit/case context
(commitContext from internal.admin.origination.commit.getCommitContext) contains
the correct portal and org for the case; update the
createCanonicalBorrowerProfileRef call to use the case/org from commitContext
(thread the org id returned by getCommitContext alongside portalId) instead of
ctx.viewer.orgId so the borrower is created in the case's org while keeping
portalId = commitContext.portalId.
| if (participant.borrowerId) { | ||
| const existingBorrower = await ctx.db.get(participant.borrowerId); | ||
| if (!existingBorrower) { | ||
| throw new ConvexError("Staged borrower reference no longer exists"); | ||
| } | ||
| const attributedBorrower = await ensureBorrowerPortalAttribution(ctx, { | ||
| borrower: existingBorrower, | ||
| portalId, | ||
| }); | ||
| borrowerLinks.push({ | ||
| borrowerId: participant.borrowerId, | ||
| borrowerId: attributedBorrower._id, | ||
| role: participant.role, | ||
| }); | ||
| continue; | ||
| } |
There was a problem hiding this comment.
Consider syncing user home portal after patching existing borrower's attribution.
When the staged-borrower path patches a previously-unattributed borrower via ensureBorrowerPortalAttribution (which may db.patch a new portalId), this commit path does not call syncUserHomePortalAssignmentByUserId. Compare with ensureCanonicalBorrowerForOrigination (same PR), which always runs that sync after reuse/insert. Given ENG-302 switches resolveUserHomePortalId to prefer explicit borrower.portalId, leaving the user's homePortalId unrefreshed here can result in a borrower whose portal is freshly attributed but whose user's home portal still points at the legacy org-derived fallback until some other event re-syncs it.
Proposed fix
if (participant.borrowerId) {
const existingBorrower = await ctx.db.get(participant.borrowerId);
if (!existingBorrower) {
throw new ConvexError("Staged borrower reference no longer exists");
}
const attributedBorrower = await ensureBorrowerPortalAttribution(ctx, {
borrower: existingBorrower,
portalId,
});
+ await syncUserHomePortalAssignmentByUserId(
+ ctx,
+ attributedBorrower.userId
+ );
borrowerLinks.push({
borrowerId: attributedBorrower._id,
role: participant.role,
});
continue;
}(Requires importing syncUserHomePortalAssignmentByUserId from ../../portals/homePortalAssignment alongside the existing getPortalByBrokerId import.)
📝 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.
| if (participant.borrowerId) { | |
| const existingBorrower = await ctx.db.get(participant.borrowerId); | |
| if (!existingBorrower) { | |
| throw new ConvexError("Staged borrower reference no longer exists"); | |
| } | |
| const attributedBorrower = await ensureBorrowerPortalAttribution(ctx, { | |
| borrower: existingBorrower, | |
| portalId, | |
| }); | |
| borrowerLinks.push({ | |
| borrowerId: participant.borrowerId, | |
| borrowerId: attributedBorrower._id, | |
| role: participant.role, | |
| }); | |
| continue; | |
| } | |
| if (participant.borrowerId) { | |
| const existingBorrower = await ctx.db.get(participant.borrowerId); | |
| if (!existingBorrower) { | |
| throw new ConvexError("Staged borrower reference no longer exists"); | |
| } | |
| const attributedBorrower = await ensureBorrowerPortalAttribution(ctx, { | |
| borrower: existingBorrower, | |
| portalId, | |
| }); | |
| await syncUserHomePortalAssignmentByUserId( | |
| ctx, | |
| attributedBorrower.userId | |
| ); | |
| borrowerLinks.push({ | |
| borrowerId: attributedBorrower._id, | |
| role: participant.role, | |
| }); | |
| continue; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@convex/admin/origination/commit.ts` around lines 306 - 320, When handling the
staged-borrower branch in commitOrigination where you call
ensureBorrowerPortalAttribution (triggered when participant.borrowerId and
existingBorrower exists), after pushing to borrowerLinks also call
syncUserHomePortalAssignmentByUserId with the attributedBorrower.userId to
refresh the user's homePortalId (mirror what
ensureCanonicalBorrowerForOrigination does); also add the import for
syncUserHomePortalAssignmentByUserId from ../../portals/homePortalAssignment
alongside getPortalByBrokerId so the sync runs immediately after attribution and
avoids stale homePortalId state.
| export async function resolveBorrowerPortalIdForWrite( | ||
| ctx: PortalReaderCtx, | ||
| args: { | ||
| brokerId?: Id<"brokers">; | ||
| explicitPortalId?: Id<"portals">; | ||
| orgId?: string; | ||
| userId: Id<"users">; | ||
| } | ||
| ) { | ||
| if (args.explicitPortalId) { | ||
| return args.explicitPortalId; | ||
| } | ||
|
|
||
| if (args.brokerId) { | ||
| const brokerPortal = await getPortalByBrokerId(ctx, args.brokerId); | ||
| if (brokerPortal) { | ||
| return brokerPortal._id; | ||
| } | ||
| } | ||
|
|
||
| if (args.orgId) { | ||
| return getDeterministicPortalIdForOrgId(ctx, args.orgId); | ||
| } | ||
|
|
||
| return undefined; |
There was a problem hiding this comment.
Fail closed for broker-scoped writes instead of falling back to org attribution.
When brokerId is provided and getPortalByBrokerId returns no portal, falling through to orgId can attribute a live broker-originated borrower to the wrong portal. Also validate explicitPortalId against the supplied broker/org before accepting it.
Proposed fix
if (args.explicitPortalId) {
- return args.explicitPortalId;
+ const explicitPortal = await ctx.db.get(args.explicitPortalId);
+ if (
+ !explicitPortal ||
+ explicitPortal.status !== "active" ||
+ !explicitPortal.isPublished ||
+ (args.brokerId && explicitPortal.brokerId !== args.brokerId) ||
+ (args.orgId && explicitPortal.orgId !== args.orgId)
+ ) {
+ return undefined;
+ }
+ return explicitPortal._id;
}
if (args.brokerId) {
const brokerPortal = await getPortalByBrokerId(ctx, args.brokerId);
if (brokerPortal) {
return brokerPortal._id;
}
+ return undefined;
}📝 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.
| export async function resolveBorrowerPortalIdForWrite( | |
| ctx: PortalReaderCtx, | |
| args: { | |
| brokerId?: Id<"brokers">; | |
| explicitPortalId?: Id<"portals">; | |
| orgId?: string; | |
| userId: Id<"users">; | |
| } | |
| ) { | |
| if (args.explicitPortalId) { | |
| return args.explicitPortalId; | |
| } | |
| if (args.brokerId) { | |
| const brokerPortal = await getPortalByBrokerId(ctx, args.brokerId); | |
| if (brokerPortal) { | |
| return brokerPortal._id; | |
| } | |
| } | |
| if (args.orgId) { | |
| return getDeterministicPortalIdForOrgId(ctx, args.orgId); | |
| } | |
| return undefined; | |
| export async function resolveBorrowerPortalIdForWrite( | |
| ctx: PortalReaderCtx, | |
| args: { | |
| brokerId?: Id<"brokers">; | |
| explicitPortalId?: Id<"portals">; | |
| orgId?: string; | |
| userId: Id<"users">; | |
| } | |
| ) { | |
| if (args.explicitPortalId) { | |
| const explicitPortal = await ctx.db.get(args.explicitPortalId); | |
| if ( | |
| !explicitPortal || | |
| explicitPortal.status !== "active" || | |
| !explicitPortal.isPublished || | |
| (args.brokerId && explicitPortal.brokerId !== args.brokerId) || | |
| (args.orgId && explicitPortal.orgId !== args.orgId) | |
| ) { | |
| return undefined; | |
| } | |
| return explicitPortal._id; | |
| } | |
| if (args.brokerId) { | |
| const brokerPortal = await getPortalByBrokerId(ctx, args.brokerId); | |
| if (brokerPortal) { | |
| return brokerPortal._id; | |
| } | |
| return undefined; | |
| } | |
| if (args.orgId) { | |
| return getDeterministicPortalIdForOrgId(ctx, args.orgId); | |
| } | |
| return undefined; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@convex/portals/borrowerPortalAttribution.ts` around lines 51 - 75, When
resolving a portal for writes in resolveBorrowerPortalIdForWrite, don’t fall
back to org attribution if a brokerId is supplied but getPortalByBrokerId(ctx,
brokerId) returns no portal — instead fail closed (return undefined) to avoid
misattributing broker-originated borrowers; additionally, when
args.explicitPortalId is provided, validate it by loading that portal (e.g., via
a portal lookup like getPortalById or equivalent) and ensure the portal’s
brokerId or orgId matches the supplied args.brokerId or args.orgId before
accepting it, otherwise reject it (return undefined).
| - `requestRole` currently has no portal context input. This chunk decides the mutation contract that downstream portal-aware callers and tests will use. | ||
| - Work started on 2026-04-20 after `ready-to-edit` validation passed. | ||
| - The mutation now accepts optional trusted `portalId`, re-resolves it server-side, and persists it on the onboarding request plus audit payloads. |
There was a problem hiding this comment.
Stale note contradicts the final mutation contract.
Line 17 states the mutation "accepts optional trusted portalId, re-resolves it server-side". That is no longer true — per the PR objectives and the shipped requestRole implementation, the mutation does not accept a portalId argument at all; it reads the trusted user.homePortalId and only persists it when the portal is active + isPublished. Likewise, line 15 still says "requestRole currently has no portal context input", which is now the final state rather than an open design question.
Please update to reflect the final contract so this status doc isn't misleading for future readers tracing why the mutation signature diverges from the spec.
📝 Suggested wording
-- `requestRole` currently has no portal context input. This chunk decides the mutation contract that downstream portal-aware callers and tests will use.
+- `requestRole` intentionally takes no caller-provided portal context. Trusted attribution is derived server-side from `users.homePortalId`.
- Work started on 2026-04-20 after `ready-to-edit` validation passed.
-- The mutation now accepts optional trusted `portalId`, re-resolves it server-side, and persists it on the onboarding request plus audit payloads.
+- The mutation resolves `portalId` from the authenticated user's `homePortalId`, gates on `status === "active"` and `isPublished`, and persists it on the onboarding request plus audit payloads.📝 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.
| - `requestRole` currently has no portal context input. This chunk decides the mutation contract that downstream portal-aware callers and tests will use. | |
| - Work started on 2026-04-20 after `ready-to-edit` validation passed. | |
| - The mutation now accepts optional trusted `portalId`, re-resolves it server-side, and persists it on the onboarding request plus audit payloads. | |
| - `requestRole` intentionally takes no caller-provided portal context. Trusted attribution is derived server-side from `users.homePortalId`. | |
| - Work started on 2026-04-20 after `ready-to-edit` validation passed. | |
| - The mutation resolves `portalId` from the authenticated user's `homePortalId`, gates on `status === "active"` and `isPublished`, and persists it on the onboarding request plus audit payloads. |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@specs/ENG-302/chunks/chunk-01-schema-onboarding/status.md` around lines 15 -
17, Update the status doc to reflect the final requestRole mutation contract:
remove the stale wording that it "accepts optional trusted portalId" and change
the description so it states requestRole does NOT accept a portalId argument,
that it reads the trusted user.homePortalId server-side, and only persists that
portalId on the onboarding request and audit payloads when the portal is active
and isPublished (i.e., check portal.active && portal.isPublished); also change
the phrase "`requestRole` currently has no portal context input" to indicate
that this is the settled final state rather than an open design question.
…th. convex/onboarding/mutations.ts (line 42) no longer accepts a caller-controlled portalId; onboarding attribution now comes from trusted users.homePortalId. convex/portals/borrowerPortalAttribution.ts (line 51) stops using onboarding history for live write resolution while keeping onboarding-first behavior for backfill, and convex/admin/origination/commit.ts (line 361), convex/borrowers/resolveOrProvisionForOrigination.ts (line 230), and convex/admin/origination/collections.ts (line 1246) now pin live origination to the current broker/case portal and fail closed when borrower portal attribution cannot be resolved. I also made seeded borrower reuse repair missing portalId on rerun in convex/seed/seedBorrower.ts (line 214). Coverage is up in the exact areas that were weak. src/test/convex/onboarding/onboarding.test.ts (line 106) now proves trusted home-portal attribution, src/test/convex/admin/origination/commit.test.ts (line 1424) covers stale onboarding vs current broker portal and src/test/convex/admin/origination/commit.test.ts (line 1573) covers the missing-broker-portal fail-closed path, and src/test/convex/seed/seedAll.test.ts (line 98) now proves seed reruns repair legacy portal-less borrowers and onboarding requests. I refreshed specs/ENG-302/audit.md (line 1) so the local audit matches the branch.
Connorbelez
force-pushed
the
graphite-base/440
branch
from
599134d to
e61d4c0
Compare
Connorbelez
force-pushed
the
codex/eng-302-explicit-borrower-portal-attribution
branch
from
e1d121f to
f9b2f2d
Compare
Connorbelez
changed the base branch from
graphite-base/440
to
codex/eng-301-portal-listings
Summary by CodeRabbit
New Features
Bug Fixes
Database Schema
portalIdfield to borrowers and onboarding requests.Tests