diff --git a/assistant/src/__tests__/guardian-principal-id-roundtrip.test.ts b/assistant/src/__tests__/guardian-principal-id-roundtrip.test.ts new file mode 100644 index 00000000000..208a9bd1cbf --- /dev/null +++ b/assistant/src/__tests__/guardian-principal-id-roundtrip.test.ts @@ -0,0 +1,193 @@ +import { mkdtempSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; + +import { afterAll, beforeEach, describe, expect, mock, test } from 'bun:test'; + +const testDir = mkdtempSync(join(tmpdir(), 'guardian-principal-id-roundtrip-test-')); + +mock.module('../util/platform.js', () => ({ + getDataDir: () => testDir, + isMacOS: () => process.platform === 'darwin', + isLinux: () => process.platform === 'linux', + isWindows: () => process.platform === 'win32', + getSocketPath: () => join(testDir, 'test.sock'), + getPidPath: () => join(testDir, 'test.pid'), + getDbPath: () => join(testDir, 'test.db'), + getLogPath: () => join(testDir, 'test.log'), + ensureDataDir: () => {}, +})); + +mock.module('../util/logger.js', () => ({ + getLogger: () => + new Proxy({} as Record, { + get: () => () => {}, + }), +})); + +import { + createCanonicalGuardianRequest, + getCanonicalGuardianRequest, + resolveCanonicalGuardianRequest, + updateCanonicalGuardianRequest, +} from '../memory/canonical-guardian-store.js'; +import { + createBinding, + getActiveBinding, +} from '../memory/guardian-bindings.js'; +import { getDb, initializeDb, resetDb } from '../memory/db.js'; + +initializeDb(); + +function resetTables(): void { + const db = getDb(); + db.run('DELETE FROM canonical_guardian_deliveries'); + db.run('DELETE FROM canonical_guardian_requests'); + db.run('DELETE FROM channel_guardian_bindings'); +} + +describe('guardianPrincipalId roundtrip', () => { + beforeEach(() => { + resetTables(); + }); + + afterAll(() => { + resetDb(); + try { + rmSync(testDir, { recursive: true }); + } catch { + // best-effort cleanup + } + }); + + // ── channel_guardian_bindings ──────────────────────────────────────── + + describe('channel_guardian_bindings', () => { + test('creates binding with guardianPrincipalId and reads it back', () => { + const binding = createBinding({ + assistantId: 'self', + channel: 'telegram', + guardianExternalUserId: 'tg-user-123', + guardianDeliveryChatId: 'tg-chat-123', + guardianPrincipalId: 'principal-abc-def', + }); + + expect(binding.guardianPrincipalId).toBe('principal-abc-def'); + + const fetched = getActiveBinding('self', 'telegram'); + expect(fetched).not.toBeNull(); + expect(fetched!.guardianPrincipalId).toBe('principal-abc-def'); + }); + + test('creates binding without guardianPrincipalId (defaults to null)', () => { + const binding = createBinding({ + assistantId: 'self', + channel: 'sms', + guardianExternalUserId: 'sms-user-456', + guardianDeliveryChatId: 'sms-chat-456', + }); + + expect(binding.guardianPrincipalId).toBeNull(); + + const fetched = getActiveBinding('self', 'sms'); + expect(fetched).not.toBeNull(); + expect(fetched!.guardianPrincipalId).toBeNull(); + }); + }); + + // ── canonical_guardian_requests ────────────────────────────────────── + + describe('canonical_guardian_requests', () => { + test('creates request with guardianPrincipalId and reads it back', () => { + const req = createCanonicalGuardianRequest({ + kind: 'tool_approval', + sourceType: 'channel', + sourceChannel: 'telegram', + guardianExternalUserId: 'guardian-tg-1', + guardianPrincipalId: 'principal-123', + }); + + expect(req.guardianPrincipalId).toBe('principal-123'); + expect(req.decidedByPrincipalId).toBeNull(); + + const fetched = getCanonicalGuardianRequest(req.id); + expect(fetched).not.toBeNull(); + expect(fetched!.guardianPrincipalId).toBe('principal-123'); + expect(fetched!.decidedByPrincipalId).toBeNull(); + }); + + test('creates request without guardianPrincipalId (defaults to null)', () => { + const req = createCanonicalGuardianRequest({ + kind: 'access_request', + sourceType: 'desktop', + }); + + expect(req.guardianPrincipalId).toBeNull(); + expect(req.decidedByPrincipalId).toBeNull(); + }); + + test('creates request with decidedByPrincipalId', () => { + const req = createCanonicalGuardianRequest({ + kind: 'tool_approval', + sourceType: 'voice', + decidedByPrincipalId: 'decider-principal-1', + }); + + expect(req.decidedByPrincipalId).toBe('decider-principal-1'); + }); + + test('updates decidedByPrincipalId via updateCanonicalGuardianRequest', () => { + const req = createCanonicalGuardianRequest({ + kind: 'tool_approval', + sourceType: 'channel', + }); + + const updated = updateCanonicalGuardianRequest(req.id, { + status: 'approved', + decidedByPrincipalId: 'principal-decider-abc', + decidedByExternalUserId: 'ext-user-1', + }); + + expect(updated).not.toBeNull(); + expect(updated!.decidedByPrincipalId).toBe('principal-decider-abc'); + expect(updated!.decidedByExternalUserId).toBe('ext-user-1'); + expect(updated!.status).toBe('approved'); + }); + + test('resolveCanonicalGuardianRequest writes decidedByPrincipalId', () => { + const req = createCanonicalGuardianRequest({ + kind: 'tool_approval', + sourceType: 'voice', + guardianPrincipalId: 'guardian-principal-xyz', + }); + + const resolved = resolveCanonicalGuardianRequest(req.id, 'pending', { + status: 'approved', + answerText: 'Approved', + decidedByExternalUserId: 'guardian-ext-1', + decidedByPrincipalId: 'guardian-principal-xyz', + }); + + expect(resolved).not.toBeNull(); + expect(resolved!.status).toBe('approved'); + expect(resolved!.decidedByPrincipalId).toBe('guardian-principal-xyz'); + expect(resolved!.decidedByExternalUserId).toBe('guardian-ext-1'); + expect(resolved!.guardianPrincipalId).toBe('guardian-principal-xyz'); + }); + + test('resolve without decidedByPrincipalId leaves it null', () => { + const req = createCanonicalGuardianRequest({ + kind: 'tool_approval', + sourceType: 'channel', + }); + + const resolved = resolveCanonicalGuardianRequest(req.id, 'pending', { + status: 'denied', + decidedByExternalUserId: 'guardian-ext-2', + }); + + expect(resolved).not.toBeNull(); + expect(resolved!.decidedByPrincipalId).toBeNull(); + }); + }); +}); diff --git a/assistant/src/memory/canonical-guardian-store.ts b/assistant/src/memory/canonical-guardian-store.ts index b2b388a3fe9..aaedd3b2d91 100644 --- a/assistant/src/memory/canonical-guardian-store.ts +++ b/assistant/src/memory/canonical-guardian-store.ts @@ -31,6 +31,7 @@ export interface CanonicalGuardianRequest { requesterExternalUserId: string | null; requesterChatId: string | null; guardianExternalUserId: string | null; + guardianPrincipalId: string | null; callSessionId: string | null; pendingQuestionId: string | null; questionText: string | null; @@ -40,6 +41,7 @@ export interface CanonicalGuardianRequest { status: CanonicalRequestStatus; answerText: string | null; decidedByExternalUserId: string | null; + decidedByPrincipalId: string | null; followupState: string | null; expiresAt: string | null; createdAt: string; @@ -117,6 +119,7 @@ function rowToRequest(row: typeof canonicalGuardianRequests.$inferSelect): Canon requesterExternalUserId: row.requesterExternalUserId, requesterChatId: row.requesterChatId, guardianExternalUserId: row.guardianExternalUserId, + guardianPrincipalId: row.guardianPrincipalId, callSessionId: row.callSessionId, pendingQuestionId: row.pendingQuestionId, questionText: row.questionText, @@ -126,6 +129,7 @@ function rowToRequest(row: typeof canonicalGuardianRequests.$inferSelect): Canon status: row.status as CanonicalRequestStatus, answerText: row.answerText, decidedByExternalUserId: row.decidedByExternalUserId, + decidedByPrincipalId: row.decidedByPrincipalId, followupState: row.followupState, expiresAt: row.expiresAt, createdAt: row.createdAt, @@ -160,6 +164,7 @@ export interface CreateCanonicalGuardianRequestParams { requesterExternalUserId?: string; requesterChatId?: string; guardianExternalUserId?: string; + guardianPrincipalId?: string; callSessionId?: string; pendingQuestionId?: string; questionText?: string; @@ -169,6 +174,7 @@ export interface CreateCanonicalGuardianRequestParams { status?: CanonicalRequestStatus; answerText?: string; decidedByExternalUserId?: string; + decidedByPrincipalId?: string; followupState?: string; expiresAt?: string; } @@ -187,6 +193,7 @@ export function createCanonicalGuardianRequest(params: CreateCanonicalGuardianRe requesterExternalUserId: params.requesterExternalUserId ?? null, requesterChatId: params.requesterChatId ?? null, guardianExternalUserId: params.guardianExternalUserId ?? null, + guardianPrincipalId: params.guardianPrincipalId ?? null, callSessionId: params.callSessionId ?? null, pendingQuestionId: params.pendingQuestionId ?? null, questionText: params.questionText ?? null, @@ -196,6 +203,7 @@ export function createCanonicalGuardianRequest(params: CreateCanonicalGuardianRe status: params.status ?? ('pending' as const), answerText: params.answerText ?? null, decidedByExternalUserId: params.decidedByExternalUserId ?? null, + decidedByPrincipalId: params.decidedByPrincipalId ?? null, followupState: params.followupState ?? null, expiresAt: params.expiresAt ?? null, createdAt: now, @@ -292,6 +300,7 @@ export interface UpdateCanonicalGuardianRequestParams { status?: CanonicalRequestStatus; answerText?: string; decidedByExternalUserId?: string; + decidedByPrincipalId?: string; followupState?: string | null; expiresAt?: string; } @@ -307,6 +316,7 @@ export function updateCanonicalGuardianRequest( if (updates.status !== undefined) setValues.status = updates.status; if (updates.answerText !== undefined) setValues.answerText = updates.answerText; if (updates.decidedByExternalUserId !== undefined) setValues.decidedByExternalUserId = updates.decidedByExternalUserId; + if (updates.decidedByPrincipalId !== undefined) setValues.decidedByPrincipalId = updates.decidedByPrincipalId; if (updates.followupState !== undefined) setValues.followupState = updates.followupState; if (updates.expiresAt !== undefined) setValues.expiresAt = updates.expiresAt; @@ -322,6 +332,7 @@ export interface ResolveDecision { status: CanonicalRequestStatus; answerText?: string; decidedByExternalUserId?: string; + decidedByPrincipalId?: string; } /** @@ -343,6 +354,7 @@ export function resolveCanonicalGuardianRequest( }; if (decision.answerText !== undefined) setValues.answerText = decision.answerText; if (decision.decidedByExternalUserId !== undefined) setValues.decidedByExternalUserId = decision.decidedByExternalUserId; + if (decision.decidedByPrincipalId !== undefined) setValues.decidedByPrincipalId = decision.decidedByPrincipalId; db.update(canonicalGuardianRequests) .set(setValues) diff --git a/assistant/src/memory/db-init.ts b/assistant/src/memory/db-init.ts index 9827e6d2684..f56487a1e04 100644 --- a/assistant/src/memory/db-init.ts +++ b/assistant/src/memory/db-init.ts @@ -37,6 +37,7 @@ import { migrateNotificationDeliveryThreadDecision, migrateReminderRoutingIntent, migrateSchemaIndexesAndColumns, + migrateGuardianPrincipalIdColumns, migrateVoiceInviteColumns, migrateVoiceInviteDisplayMetadata, recoverCrashedMigrations, @@ -173,5 +174,8 @@ export function initializeDb(): void { // 28. Actor token records (hash-only actor token persistence) createActorTokenRecordsTable(database); + // 29. Guardian principal ID columns on channel_guardian_bindings and canonical_guardian_requests + migrateGuardianPrincipalIdColumns(database); + validateMigrationState(database); } diff --git a/assistant/src/memory/guardian-bindings.ts b/assistant/src/memory/guardian-bindings.ts index 21b7e593cb9..3478ca637de 100644 --- a/assistant/src/memory/guardian-bindings.ts +++ b/assistant/src/memory/guardian-bindings.ts @@ -23,6 +23,7 @@ export interface GuardianBinding { channel: string; guardianExternalUserId: string; guardianDeliveryChatId: string; + guardianPrincipalId: string | null; status: BindingStatus; verifiedAt: number; verifiedVia: string; @@ -42,6 +43,7 @@ function rowToBinding(row: typeof channelGuardianBindings.$inferSelect): Guardia channel: row.channel, guardianExternalUserId: row.guardianExternalUserId, guardianDeliveryChatId: row.guardianDeliveryChatId, + guardianPrincipalId: row.guardianPrincipalId, status: row.status as BindingStatus, verifiedAt: row.verifiedAt, verifiedVia: row.verifiedVia, @@ -60,6 +62,7 @@ export function createBinding(params: { channel: string; guardianExternalUserId: string; guardianDeliveryChatId: string; + guardianPrincipalId?: string | null; verifiedVia?: string; metadataJson?: string | null; }): GuardianBinding { @@ -73,6 +76,7 @@ export function createBinding(params: { channel: params.channel, guardianExternalUserId: params.guardianExternalUserId, guardianDeliveryChatId: params.guardianDeliveryChatId, + guardianPrincipalId: params.guardianPrincipalId ?? null, status: 'active' as const, verifiedAt: now, verifiedVia: params.verifiedVia ?? 'challenge', diff --git a/assistant/src/memory/migrations/125-guardian-principal-id-columns.ts b/assistant/src/memory/migrations/125-guardian-principal-id-columns.ts new file mode 100644 index 00000000000..962b6ccb0bc --- /dev/null +++ b/assistant/src/memory/migrations/125-guardian-principal-id-columns.ts @@ -0,0 +1,19 @@ +import type { DrizzleDb } from '../db-connection.js'; + +/** + * Add guardian_principal_id columns to channel_guardian_bindings and + * canonical_guardian_requests, plus decided_by_principal_id to + * canonical_guardian_requests. + * + * These nullable TEXT columns support the canonical identity binding + * cutover — linking guardian bindings and approval requests to a + * stable principal identity rather than relying solely on + * channel-specific external user IDs. + * + * Uses ALTER TABLE ADD COLUMN with try/catch for idempotency. + */ +export function migrateGuardianPrincipalIdColumns(database: DrizzleDb): void { + try { database.run(/*sql*/ `ALTER TABLE channel_guardian_bindings ADD COLUMN guardian_principal_id TEXT`); } catch { /* already exists */ } + try { database.run(/*sql*/ `ALTER TABLE canonical_guardian_requests ADD COLUMN guardian_principal_id TEXT`); } catch { /* already exists */ } + try { database.run(/*sql*/ `ALTER TABLE canonical_guardian_requests ADD COLUMN decided_by_principal_id TEXT`); } catch { /* already exists */ } +} diff --git a/assistant/src/memory/migrations/index.ts b/assistant/src/memory/migrations/index.ts index a129e637662..876c7422a45 100644 --- a/assistant/src/memory/migrations/index.ts +++ b/assistant/src/memory/migrations/index.ts @@ -65,6 +65,7 @@ export { createCanonicalGuardianTables } from './121-canonical-guardian-requests export { migrateCanonicalGuardianRequesterChatId } from './122-canonical-guardian-requester-chat-id.js'; export { migrateCanonicalGuardianDeliveriesDestinationIndex } from './123-canonical-guardian-deliveries-destination-index.js'; export { migrateVoiceInviteDisplayMetadata } from './124-voice-invite-display-metadata.js'; +export { migrateGuardianPrincipalIdColumns } from './125-guardian-principal-id-columns.js'; export { MIGRATION_REGISTRY, type MigrationRegistryEntry, diff --git a/assistant/src/memory/schema.ts b/assistant/src/memory/schema.ts index 9f1807c25f2..fc49980fea8 100644 --- a/assistant/src/memory/schema.ts +++ b/assistant/src/memory/schema.ts @@ -637,6 +637,7 @@ export const channelGuardianBindings = sqliteTable('channel_guardian_bindings', channel: text('channel').notNull(), guardianExternalUserId: text('guardian_external_user_id').notNull(), guardianDeliveryChatId: text('guardian_delivery_chat_id').notNull(), + guardianPrincipalId: text('guardian_principal_id'), status: text('status').notNull().default('active'), verifiedAt: integer('verified_at').notNull(), verifiedVia: text('verified_via').notNull().default('challenge'), @@ -887,6 +888,7 @@ export const canonicalGuardianRequests = sqliteTable('canonical_guardian_request requesterExternalUserId: text('requester_external_user_id'), requesterChatId: text('requester_chat_id'), guardianExternalUserId: text('guardian_external_user_id'), + guardianPrincipalId: text('guardian_principal_id'), callSessionId: text('call_session_id'), pendingQuestionId: text('pending_question_id'), questionText: text('question_text'), @@ -896,6 +898,7 @@ export const canonicalGuardianRequests = sqliteTable('canonical_guardian_request status: text('status').notNull().default('pending'), answerText: text('answer_text'), decidedByExternalUserId: text('decided_by_external_user_id'), + decidedByPrincipalId: text('decided_by_principal_id'), followupState: text('followup_state'), expiresAt: text('expires_at'), createdAt: text('created_at').notNull(),