fix(crew): keep first legacy skipper when several are present

Prefer canonical skipper id and stop overwriting activeSkipperId during
legacy crew migration and read-only share conversion.

Co-authored-by: Cursor <cursoragent@cursor.com>

client/src/components/ReadOnlyViewer.tsx

@@ -7,7 +7,7 @@ import VesselForm from './VesselForm.tsx'
 import LogbookCrewPicker from './LogbookCrewPicker.tsx'
 import type { LogbookCrewSelectionData } from '../types/person.js'
 import { emptyLogbookCrewSelection } from '../types/person.js'
-import { personToSnapshot } from '../utils/personSnapshots.js'
+import { legacyCrewRecordsToLogbookSelection } from '../utils/personSnapshots.js'
 import type { PersonData } from '../types/person.js'
 import LogEntriesList from './LogEntriesList.tsx'
 import { Ship, Users, FileText, Lock, AlertCircle, Globe } from 'lucide-react'
@@ -112,18 +112,7 @@ export default function ReadOnlyViewer({ token, hexKey }: ReadOnlyViewerProps) {
       setLegacyCrews(decCrews)
 
       if (!data.logbookCrewSelection && decCrews.length > 0) {
-        const snapshotsById: LogbookCrewSelectionData['snapshotsById'] = {}
+        setLogbookCrewSelection(legacyCrewRecordsToLogbookSelection(decCrews))
-        let activeSkipperId: string | null = null
-        const activeCrewIds: string[] = []
-        for (const c of decCrews) {
-          snapshotsById[c.payloadId] = personToSnapshot(c.payloadId, c.data)
-          if (c.payloadId === 'skipper' || c.data.role === 'skipper') {
-            activeSkipperId = c.payloadId
-          } else {
-            activeCrewIds.push(c.payloadId)
-          }
-        }
-        setLogbookCrewSelection({ activeSkipperId, activeCrewIds, snapshotsById })
       }
 
       // Decrypt Entries

client/src/services/crewMigration.ts

@@ -3,7 +3,7 @@ import { getActiveMasterKey } from './auth.js'
 import { decryptJson, encryptJson } from './crypto.js'
 import { getLogbookKey } from './logbookKeys.js'
 import type { PersonData } from '../types/person.js'
-import { buildLogbookCrewSelection } from '../utils/personSnapshots.js'
+import { buildLogbookCrewSelection, pickActiveSkipperId } from '../utils/personSnapshots.js'
 import { entryCrewFromLogbookSelection } from '../utils/personSnapshots.js'
 import { saveLogbookCrewSelection } from './logbookCrewSelection.js'
 const MIGRATION_FLAG = 'crew_pool_migration_v1_done'
@@ -24,8 +24,8 @@ export async function migrateLegacyCrewToPoolIfNeeded(): Promise<void> {
       const logbookKey = (await getLogbookKey(logbook.id)) || masterKey
       const legacyCrews = await db.crews.where({ logbookId: logbook.id }).toArray()
 
-      const legacyIds: { skipperId: string | null; crewIds: string[] } = {
+      const legacyIds: { skipperIds: string[]; crewIds: string[] } = {
-        skipperId: null,
+        skipperIds: [],
         crewIds: []
       }
 
@@ -65,14 +65,18 @@ export async function migrateLegacyCrewToPoolIfNeeded(): Promise<void> {
           poolData.set(poolId, personData)
         }
 
-        if (role === 'skipper') legacyIds.skipperId = poolId
+        if (role === 'skipper') {
-        else legacyIds.crewIds.push(poolId)
+          if (!legacyIds.skipperIds.includes(poolId)) legacyIds.skipperIds.push(poolId)
+        } else {
+          legacyIds.crewIds.push(poolId)
+        }
       }
 
+      const activeSkipperId = pickActiveSkipperId(legacyIds.skipperIds)
       const existingSelection = await db.logbookCrewSelections.get(logbook.id)
-      if (!existingSelection && (legacyIds.skipperId || legacyIds.crewIds.length > 0)) {
+      if (!existingSelection && (activeSkipperId || legacyIds.crewIds.length > 0)) {
         const selection = buildLogbookCrewSelection(
-          legacyIds.skipperId,
+          activeSkipperId,
           legacyIds.crewIds,
           poolData
         )

client/src/utils/personSnapshots.test.ts

@@ -0,0 +1,58 @@
+import { describe, expect, it } from 'vitest'
+import type { PersonData } from '../types/person.js'
+import {
+  legacyCrewRecordsToLogbookSelection,
+  pickActiveSkipperId
+} from './personSnapshots.js'
+
+function person(overrides: Partial<PersonData> & { role: PersonData['role'] }): PersonData {
+  return {
+    name: overrides.name ?? 'Test',
+    address: '',
+    birthDate: '',
+    phone: '',
+    nationality: '',
+    passportNumber: '',
+    bloodType: '',
+    allergies: '',
+    diseases: '',
+    role: overrides.role
+  }
+}
+
+describe('pickActiveSkipperId', () => {
+  it('returns null for empty list', () => {
+    expect(pickActiveSkipperId([])).toBeNull()
+  })
+
+  it('prefers canonical skipper payload id', () => {
+    expect(pickActiveSkipperId(['other-skipper', 'skipper', 'third'])).toBe('skipper')
+  })
+
+  it('keeps first skipper when canonical id is absent', () => {
+    expect(pickActiveSkipperId(['alpha', 'beta'])).toBe('alpha')
+  })
+})
+
+describe('legacyCrewRecordsToLogbookSelection', () => {
+  it('does not let a later skipper overwrite the active skipper', () => {
+    const selection = legacyCrewRecordsToLogbookSelection([
+      { payloadId: 'skipper', data: person({ role: 'skipper', name: 'Primary' }) },
+      { payloadId: 'co-skipper', data: person({ role: 'skipper', name: 'Secondary' }) },
+      { payloadId: 'crew-1', data: person({ role: 'crew', name: 'Crew' }) }
+    ])
+
+    expect(selection.activeSkipperId).toBe('skipper')
+    expect(selection.activeCrewIds).toEqual(['crew-1'])
+    expect(Object.keys(selection.snapshotsById)).toEqual(['skipper', 'co-skipper', 'crew-1'])
+  })
+
+  it('uses first skipper when canonical id is missing', () => {
+    const selection = legacyCrewRecordsToLogbookSelection([
+      { payloadId: 'first-skip', data: person({ role: 'skipper', name: 'First' }) },
+      { payloadId: 'second-skip', data: person({ role: 'skipper', name: 'Second' }) }
+    ])
+
+    expect(selection.activeSkipperId).toBe('first-skip')
+  })
+})

client/src/utils/personSnapshots.ts

@@ -1,5 +1,39 @@
 import type { LogbookCrewSelectionData, PersonData, PersonSnapshot } from '../types/person.js'
 
+/** Prefer canonical legacy id `skipper`, otherwise keep the first skipper encountered. */
+export function pickActiveSkipperId(skipperIds: readonly string[]): string | null {
+  if (skipperIds.length === 0) return null
+  return skipperIds.find((id) => id === 'skipper') ?? skipperIds[0]
+}
+
+export function isSkipperRecord(payloadId: string, data: PersonData): boolean {
+  return payloadId === 'skipper' || data.role === 'skipper'
+}
+
+/** Build logbook crew selection from legacy per-logbook crew records (read-only share / migration). */
+export function legacyCrewRecordsToLogbookSelection(
+  crews: Array<{ payloadId: string; data: PersonData }>
+): LogbookCrewSelectionData {
+  const snapshotsById: Record<string, PersonSnapshot> = {}
+  const skipperIds: string[] = []
+  const activeCrewIds: string[] = []
+
+  for (const c of crews) {
+    snapshotsById[c.payloadId] = personToSnapshot(c.payloadId, c.data)
+    if (isSkipperRecord(c.payloadId, c.data)) {
+      if (!skipperIds.includes(c.payloadId)) skipperIds.push(c.payloadId)
+    } else {
+      activeCrewIds.push(c.payloadId)
+    }
+  }
+
+  return {
+    activeSkipperId: pickActiveSkipperId(skipperIds),
+    activeCrewIds,
+    snapshotsById
+  }
+}
+
 export function personToSnapshot(id: string, data: PersonData): PersonSnapshot {
   return {
     id,