feat(#1466 P3b): compliance-grade insurance resolution evidence#1472
Conversation
Advances #837 P2 (documentation) + P3 (declinations). Captures HOW and by WHOM a traveler's insurance resolution was recorded, closing the "rubber-stamp" gap where declined/has_own/selected could be set with no consent or offered- coverage record. Additive/forward-only; ships live on merge. Migration (20260705120000): new enum insurance_resolution_source ('advisor_manual','client_esign') + 4 additive columns on trip_traveler_insurance (resolution_source NOT NULL DEFAULT 'advisor_manual', consent_method, form_token_id FK→form_tokens ON DELETE SET NULL, offered_coverages_snapshot jsonb). The ~93 existing prod rows keep the default with null evidence — a documented pre-P3b audit gap, NOT retro-enforced (flag to Al). Backend: - insurance.service: validateStatusRequirements now REQUIRES a valid consent_method when an advisor records a resolved status (400 otherwise); pending is exempt. create/updateTravelerInsurance FORCE resolution_source='advisor_manual' server-side (mirrors the activity_id-strip; spoofed values stripped) and freeze offered_coverages_snapshot from active packages. A client_esign record is preserved (no consent demanded, evidence not clobbered) when the advisor edits without changing the resolved status; an override flips it to advisor_manual with fresh consent and unlinks the token. formatTravelerInsurance surfaces all 4 fields. - forms.service handleInsuranceWaiverSubmission: stamps resolution_source='client_esign', form_token_id, offered_coverages_snapshot on every write (insert+update, both actions) inside the existing atomic-claim tx. No new send. - Shared insurance-offer-snapshot.mapper (extracted verbatim from queueProposalEmails' contextData snapshot, §7 colocated spec) so the advisor and waiver paths record an identical offer shape. - insurance.queries declines report surfaces resolution_source + consent_method. Frontend (trip-insurance.tsx): - TravelerInsuranceDialog + AssignPackageDialog: required "Confirmed via" selector shown for a resolved status on a non-e-sign record; Save gated until chosen. - Status table + dialog: "View signed waiver" link when form_token_id is present (reuses GET /trips/:id/forms/:formId/pdf — no new endpoint). Shared-types: InsuranceResolutionSource, AdvisorConsentMethod (+ADVISOR_CONSENT_METHODS, excludes client-self-service), InsuranceOfferSnapshotItem; 4 read-DTO fields + consentMethod on Create/Update. STRICT error mode (compliance never best-effort). Money boundary untouched (commissionTotalCents/activityId/mirror/link paths unchanged). No TripsService touch. Tests (all foreground): mapper spec; service evidence spec (consent required per resolved status + exempt for pending; resolution_source always advisor_manual even if spoofed; snapshot frozen; client_esign preserve vs override); forms spec extended (client_esign+form_token_id+snapshot on both branches inside the tx). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis PR adds compliance-grade resolution evidence to trip traveler insurance. A new migration and schema introduce resolution source, consent method, form token, and offer snapshot columns. API services validate/store evidence on create, update, and client e-sign waiver submission; reporting queries expose the fields; admin UI captures consent and shows signed-waiver links. ChangesInsurance Resolution Evidence
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Traveler
participant FormsService
participant Database
participant InsuranceService
participant AdminUI
Traveler->>FormsService: submit signed waiver (formTokenId)
FormsService->>FormsService: build esignEvidence (resolutionSource=client_esign, offeredCoveragesSnapshot, consentMethod=null)
FormsService->>Database: insert/update trip_traveler_insurance with esignEvidence
Database-->>FormsService: confirm write
AdminUI->>InsuranceService: update traveler status (advisor override)
InsuranceService->>InsuranceService: detect material change, require consentMethod
InsuranceService->>Database: write resolutionSource=advisor_manual, consentMethod, clear formTokenId
Database-->>InsuranceService: confirm write
InsuranceService-->>AdminUI: return updated DTO with evidence fields
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
apps/admin/src/app/trips/[id]/_components/trip-insurance.tsxOops! Something went wrong! :( ESLint: 8.57.1 Error: ESLint configuration in --config is invalid:
apps/api/src/forms/__tests__/forms.service.spec.tsOops! Something went wrong! :( ESLint: 8.57.1 Error: ESLint configuration in --config is invalid:
apps/api/src/forms/forms.service.tsOops! Something went wrong! :( ESLint: 8.57.1 Error: ESLint configuration in --config is invalid:
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: 2
🧹 Nitpick comments (1)
packages/database/src/migrations/20260705120000_1466_insurance_resolution_evidence.sql (1)
23-23: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueConsider indexing
form_token_id.
form_token_idhasON DELETE SET NULLbut no index. When aform_tokensrow is deleted, Postgres must scantrip_traveler_insuranceto null out matching rows; without an index this is a sequential scan. Low risk today given the small table (~93 rows per the migration comment), but worth adding proactively since this FK will be hit on every form-token cleanup/expiry going forward.♻️ Optional index addition
ALTER TABLE trip_traveler_insurance ADD COLUMN IF NOT EXISTS resolution_source insurance_resolution_source NOT NULL DEFAULT 'advisor_manual', ADD COLUMN IF NOT EXISTS consent_method varchar(50), ADD COLUMN IF NOT EXISTS form_token_id uuid REFERENCES form_tokens(id) ON DELETE SET NULL, ADD COLUMN IF NOT EXISTS offered_coverages_snapshot jsonb; + +CREATE INDEX IF NOT EXISTS idx_trip_traveler_insurance_form_token_id + ON trip_traveler_insurance (form_token_id) + WHERE form_token_id IS NOT NULL;🤖 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 `@packages/database/src/migrations/20260705120000_1466_insurance_resolution_evidence.sql` at line 23, Add an index for the new trip_traveler_insurance.form_token_id foreign key so deletes from form_tokens don’t require a sequential scan when ON DELETE SET NULL runs. Update the migration that adds form_token_id to also create the appropriate index in the same change set, using the existing migration naming style and keeping it aligned with the new column definition.
🤖 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 `@apps/api/src/forms/forms.service.ts`:
- Around line 571-583: The `esignEvidence` payload in `forms.service.ts` is
persisting `offeredCoveragesSnapshot` directly from `offeredPackages`, which can
carry the old partial token shape. Update the `forms.service.ts` write path to
normalize or validate the snapshot before assigning it, reusing the same shape
produced by `mapPackagesToOfferSnapshot(activePackages)` or equivalent
validation logic. Keep the fix within the client e-sign evidence construction so
every insert/update stores a fully rehydrated compliance snapshot.
In `@apps/api/src/trips/insurance.service.ts`:
- Around line 961-1020: The `updateTravelerInsurance` flow is racing because it
reads the current insurance row, derives
`preserveClientEsign`/`advisorResolutionWrite`, and then writes evidence fields
later without a transaction or row lock. Wrap the read-decision-write sequence
in a transaction and lock the target insurance record first (for example in the
`updateTravelerInsurance` method before computing `materialResolutionChange` and
`effectiveStatus`) so concurrent PATCHes cannot overwrite `resolutionSource`,
`consentMethod`, `offeredCoveragesSnapshot`, or `formTokenId` from stale state.
---
Nitpick comments:
In
`@packages/database/src/migrations/20260705120000_1466_insurance_resolution_evidence.sql`:
- Line 23: Add an index for the new trip_traveler_insurance.form_token_id
foreign key so deletes from form_tokens don’t require a sequential scan when ON
DELETE SET NULL runs. Update the migration that adds form_token_id to also
create the appropriate index in the same change set, using the existing
migration naming style and keeping it aligned with the new column definition.
🪄 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: 57f8f5a2-d23e-4d09-ac00-dddfee6a122c
📒 Files selected for processing (14)
apps/admin/src/app/trips/[id]/_components/trip-insurance.tsxapps/api/src/forms/__tests__/forms.service.spec.tsapps/api/src/forms/forms.service.tsapps/api/src/reporting/queries/insurance.queries.tsapps/api/src/trips/insurance-automation.service.tsapps/api/src/trips/insurance-offer-snapshot.mapper.spec.tsapps/api/src/trips/insurance-offer-snapshot.mapper.tsapps/api/src/trips/insurance.service.evidence.spec.tsapps/api/src/trips/insurance.service.tspackages/database/src/index.tspackages/database/src/migrations/20260705120000_1466_insurance_resolution_evidence.sqlpackages/database/src/migrations/meta/_journal.jsonpackages/database/src/schema/insurance.schema.tspackages/shared-types/src/api/insurance.types.ts
| // refs #1466 P3b — this is a CLIENT E-SIGN resolution: stamp the | ||
| // compliance evidence on every write (insert AND update, both actions). | ||
| // form.id links the signed waiver PDF; offeredPackages freezes what the | ||
| // traveler was shown. Inside the existing atomic-claim tx, no new send. | ||
| const esignEvidence = { | ||
| resolutionSource: 'client_esign' as const, | ||
| formTokenId: form.id, | ||
| offeredCoveragesSnapshot: offeredPackages, | ||
| // Clear any stale advisor consent — this row is now resolved by the | ||
| // client's signed waiver, not an out-of-band advisor confirmation. | ||
| consentMethod: null, | ||
| } | ||
|
|
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the waiver submission flow and the types involved
git ls-files 'apps/api/src/forms/*' 'apps/api/src/**/insurance*' 'apps/api/src/**/forms*' 'apps/api/src/**/shared*' | sed 's#^`#FILE` #'
printf '\n--- forms.service.ts around the cited lines ---\n'
sed -n '430,620p' apps/api/src/forms/forms.service.ts
printf '\n--- search for offeredCoveragesSnapshot / InsuranceOfferSnapshotItem / contextData.packages ---\n'
rg -n "offeredCoveragesSnapshot|InsuranceOfferSnapshotItem|contextData\.packages|mapPackagesToOfferSnapshot|packages:" apps/api/src -g '!**/*.map'
printf '\n--- likely schema/type definitions ---\n'
fd -a 'insurance' apps/api/src
fd -a 'snapshot' apps/api/src
fd -a 'waiver' apps/api/srcRepository: Systemsaholic/tailfire
Length of output: 18129
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n--- insurance-offer-snapshot.mapper.ts ---\n'
sed -n '1,220p' apps/api/src/trips/insurance-offer-snapshot.mapper.ts
printf '\n--- insurance-automation.service.ts around token creation ---\n'
sed -n '280,360p' apps/api/src/trips/insurance-automation.service.ts
printf '\n--- forms.service.spec.ts evidence tests around snapshot payloads ---\n'
sed -n '360,440p' apps/api/src/forms/__tests__/forms.service.spec.ts
printf '\n--- insurance.service.evidence.spec.ts relevant assertions ---\n'
sed -n '1,220p' apps/api/src/trips/insurance.service.evidence.spec.tsRepository: Systemsaholic/tailfire
Length of output: 16417
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n--- search snapshot contract definitions ---\n'
rg -n "offered_coverages_snapshot|offeredCoveragesSnapshot|InsuranceOfferSnapshotItem" . -g '!**/node_modules/**' -g '!**/dist/**' -g '!**/build/**'
printf '\n--- database/schema definitions for trip_traveler_insurance ---\n'
fd -a 'trip.*insurance|insurance.*schema|schema.*ts' apps/api packages | sed -n '1,120p'Repository: Systemsaholic/tailfire
Length of output: 12510
Normalize offeredCoveragesSnapshot before persisting it
apps/api/src/forms/forms.service.ts:571-583 still copies form.contextData.packages directly into evidence. Newly minted tokens use mapPackagesToOfferSnapshot(activePackages), but any older token can still carry the pre-change partial shape and freeze malformed compliance data. Rehydrate or validate the snapshot before writing it.
🤖 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 `@apps/api/src/forms/forms.service.ts` around lines 571 - 583, The
`esignEvidence` payload in `forms.service.ts` is persisting
`offeredCoveragesSnapshot` directly from `offeredPackages`, which can carry the
old partial token shape. Update the `forms.service.ts` write path to normalize
or validate the snapshot before assigning it, reusing the same shape produced by
`mapPackagesToOfferSnapshot(activePackages)` or equivalent validation logic.
Keep the fix within the client e-sign evidence construction so every
insert/update stores a fully rehydrated compliance snapshot.
| // refs #1466 P3b — a resolution captured by the client waiver path | ||
| // (client_esign) already carries its own evidence (signature/IP/signed PDF). | ||
| // It is preserved ONLY for evidence-NEUTRAL edits (notes, premium, policy | ||
| // number). ANY change to the resolution itself — the status OR a status-defining | ||
| // field (package, external policy/provider/coverage, decline reason) — is a | ||
| // material advisor override: re-capture consent, flip to advisor_manual, unlink | ||
| // the signed waiver, and refresh the offer snapshot. | ||
| const differs = (a: unknown, b: unknown) => a !== undefined && a !== b | ||
| const materialResolutionChange = | ||
| differs(dto.status, existing.status) || | ||
| differs(dto.selectedPackageId, existing.selectedPackageId) || | ||
| differs(dto.externalPolicyNumber, existing.externalPolicyNumber) || | ||
| differs(dto.externalProviderName, existing.externalProviderName) || | ||
| differs(dto.externalCoverageDetails, existing.externalCoverageDetails) || | ||
| differs(dto.declinedReason, existing.declinedReason) | ||
| const preserveClientEsign = | ||
| existing.resolutionSource === 'client_esign' && !materialResolutionChange | ||
| const effectiveStatus = dto.status ?? existing.status | ||
| const effectiveResolved = RESOLVED_INSURANCE_STATUSES.has(effectiveStatus) | ||
| // This write establishes an advisor-recorded resolution when we're not | ||
| // preserving an e-sign record and either the status is (re)set or a resolution | ||
| // field materially changed. | ||
| const advisorResolutionWrite = | ||
| !preserveClientEsign && (dto.status !== undefined || materialResolutionChange) | ||
|
|
||
| // Validate status-specific field requirements (only when status is provided, | ||
| // to preserve the existing partial-update contract). | ||
| if (dto.status !== undefined) { | ||
| this.validateStatusRequirements(dto.status, dto) | ||
| } | ||
| // Consent is required whenever this write records an advisor resolution. | ||
| if (advisorResolutionWrite && effectiveResolved) { | ||
| this.assertConsentMethod(dto) | ||
| } | ||
|
|
||
| const updateData: any = { | ||
| ...dto, | ||
| updatedAt: new Date(), | ||
| } | ||
|
|
||
| // refs #1466 P3b — ALL evidence columns are SERVICE-MANAGED; strip them from the | ||
| // incoming body unconditionally (mirror the activity_id-strip pattern) so a | ||
| // spoofed { resolutionSource: 'client_esign' } can't forge provenance AND a | ||
| // neutral PATCH { consentMethod: 'bogus' } can't inject an unvalidated consent | ||
| // value. Consent is re-set ONLY inside the validated advisor-resolution block. | ||
| delete updateData.resolutionSource | ||
| delete updateData.formTokenId | ||
| delete updateData.offeredCoveragesSnapshot | ||
| delete updateData.consentMethod | ||
|
|
||
| if (advisorResolutionWrite) { | ||
| // Advisor is (re)recording the resolution → advisor_manual with fresh | ||
| // evidence; unlink any prior signed-waiver token (this is now the advisor's | ||
| // decision of record). dto.consentMethod was already validated above. | ||
| updateData.resolutionSource = 'advisor_manual' | ||
| updateData.consentMethod = effectiveResolved ? (dto.consentMethod ?? null) : null | ||
| updateData.offeredCoveragesSnapshot = effectiveResolved ? await this.buildOfferSnapshot(tripId) : null | ||
| updateData.formTokenId = null | ||
| } | ||
|
|
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the relevant service methods and schema definitions.
rg -n "getTravelerInsuranceById|updatePackage|FOR UPDATE|transaction\(" apps/api/src/trips -S
# Map the insurance service structure first.
ast-grep outline apps/api/src/trips/insurance.service.ts --view expanded
# Read the relevant sections around the reported lines and the updatePackage comparison point.
sed -n '920,1085p' apps/api/src/trips/insurance.service.ts
printf '\n--- updatePackage ---\n'
sed -n '1,260p' apps/api/src/trips/insurance.service.ts | sed -n '/updatePackage/,/^[[:space:]]*}/p'
# Find the read method implementation and any lock usage in the repository.
rg -n "getTravelerInsuranceById|SELECT.*FOR UPDATE|for update|lock.*tripTravelerInsurance|tripTravelerInsurance" apps/api/src -S
# Read the schema for the insurance row to understand the row identity and update targets.
fd -a ".*trips.*schema.*|.*insurance.*schema.*|.*traveler.*insurance.*" apps/api/srcRepository: Systemsaholic/tailfire
Length of output: 30607
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read the exact methods involved.
sed -n '858,1068p' apps/api/src/trips/insurance.service.ts
printf '\n--- getTravelerInsuranceById ---\n'
sed -n '858,919p' apps/api/src/trips/insurance.service.ts
printf '\n--- updatePackage ---\n'
sed -n '670,798p' apps/api/src/trips/insurance.service.ts
# Inspect the evidence-focused test file for any expected concurrency or locking behavior.
wc -l apps/api/src/trips/insurance.service.evidence.spec.ts
sed -n '1,220p' apps/api/src/trips/insurance.service.evidence.spec.ts
# Check whether updateTravelerInsurance has any dedicated transactional or race-related test coverage.
rg -n "updateTravelerInsurance|client_esign|advisor_manual|FOR UPDATE|transaction\\(" apps/api/src/trips -SRepository: Systemsaholic/tailfire
Length of output: 44622
Lock updateTravelerInsurance before deriving evidence fields
The update path reads the row, decides whether to preserve client_esign or switch to advisor_manual, then writes later with no transaction or FOR UPDATE. Concurrent PATCHes can race and overwrite resolutionSource, consentMethod, offeredCoveragesSnapshot, or formTokenId from stale state. Wrap the read + decision + write in a transaction and lock the insurance row first.
🤖 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 `@apps/api/src/trips/insurance.service.ts` around lines 961 - 1020, The
`updateTravelerInsurance` flow is racing because it reads the current insurance
row, derives `preserveClientEsign`/`advisorResolutionWrite`, and then writes
evidence fields later without a transaction or row lock. Wrap the
read-decision-write sequence in a transaction and lock the target insurance
record first (for example in the `updateTravelerInsurance` method before
computing `materialResolutionChange` and `effectiveStatus`) so concurrent
PATCHes cannot overwrite `resolutionSource`, `consentMethod`,
`offeredCoveragesSnapshot`, or `formTokenId` from stale state.
1 participant
Automation Gate epic #1466 — slice P3b (compliance-grade resolution evidence)
refs #1466. Advances #837 P2 (documentation) + P3 (declinations). Captures how and by whom a traveler's insurance resolution was recorded, closing the "rubber-stamp" gap where
declined/has_own_insurance/selected_packagecould be set with no consent record or offered-coverage snapshot. Additive/forward-only; ships live on merge. Does not rebuild #837 catalog/picker (shipped). STRICT error mode (compliance never best-effort); the money boundary is untouched.Migration (
20260705120000_1466_insurance_resolution_evidence)insurance_resolution_source(advisor_manual|client_esign).trip_traveler_insurance:resolution_source(NOT NULL DEFAULTadvisor_manual),consent_method varchar(50),form_token_id uuidFK→form_tokens(ON DELETE SET NULL),offered_coverages_snapshot jsonb._journal.json(monotonic;migrations:validate-journalpasses).Backend
insurance.service—validateStatusRequirementssplit: field checks stay; consent is a dedicatedassertConsentMethod. A resolved status recorded via the advisor path requires a validconsent_method(400 otherwise);pendingis exempt.create/updateTravelerInsuranceforceresolution_source='advisor_manual'server-side and strip all evidence columns from the DTO (mirrors theactivity_id-strip — spoofedresolution_source/consent_methodcan't forge provenance), and freezeoffered_coverages_snapshotfrom active packages. Aclient_esignrecord is preserved (no consent demanded, evidence intact) for evidence-neutral edits (notes/premium/policy #); a material change (status or a status-defining field — package, external policy/provider/coverage, decline reason) is an advisor override → requires consent, flips toadvisor_manual, unlinks the token, refreshes the snapshot.formatTravelerInsurancesurfaces all 4 fields.forms.servicehandleInsuranceWaiverSubmission— stampsresolution_source='client_esign',form_token_id,offered_coverages_snapshot, and clears stale advisorconsent_methodon every write (insert+update × purchase+decline) inside the existing atomic-claim tx. No new send.insurance-offer-snapshot.mapper— extracted verbatim fromqueueProposalEmails' contextData snapshot (§7 colocated spec) so the advisor and client-waiver paths record an identical offer shape.insurance.queriesdeclines report surfacesresolution_source+consent_method.Frontend (
trip-insurance.tsx)TravelerInsuranceDialog+AssignPackageDialog: required "Confirmed via" selector shown for a resolved status on a non-e-sign record — and, mirroring the server rule, also for a material override of an e-sign record (so the override isn't silently 400'd). Save gated until chosen; threaded into the assign API calls.form_token_idis present (reusesGET /trips/:id/forms/:formId/pdf— no new endpoint).Shared-types
InsuranceResolutionSource,AdvisorConsentMethod(+ADVISOR_CONSENT_METHODS=verbal_phone/email_confirmation/in_person_signature, excludes client-self-service),InsuranceOfferSnapshotItem; 4 read-DTO fields +consentMethodon Create/Update.Tests (all foreground, green)
insurance-offer-snapshot.mapper.spec(field mapping + nullable coalesce).insurance.service.evidence.spec(13): consent required per resolved status + exempt for pending; invalid consent rejected;resolution_sourcealwaysadvisor_manualeven if spoofed; snapshot frozen; client_esign preserve vs material override (incl. package A→B reassignment); neutral-PATCH consent-injection stripped.forms.service.specextended:client_esign+form_token_id+ snapshot + stale-consent-cleared on both insert and update branches inside the tx.Review
Codex (gpt-5.5, xhigh) — reviewed iteratively: r1 BLOCK (e-sign evidence preserved across overrides; stale advisor consent on client update) → fixed; r2 BLOCK (body-mutable
consent_methodon neutral PATCH) → fixed; APPROVE onca801212(final HEAD).🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes