Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
193 changes: 193 additions & 0 deletions assistant/src/__tests__/guardian-principal-id-roundtrip.test.ts
Original file line number Diff line number Diff line change
@@ -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<string, unknown>, {
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();
});
});
});
12 changes: 12 additions & 0 deletions assistant/src/memory/canonical-guardian-store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -160,6 +164,7 @@ export interface CreateCanonicalGuardianRequestParams {
requesterExternalUserId?: string;
requesterChatId?: string;
guardianExternalUserId?: string;
guardianPrincipalId?: string;
callSessionId?: string;
pendingQuestionId?: string;
questionText?: string;
Expand All @@ -169,6 +174,7 @@ export interface CreateCanonicalGuardianRequestParams {
status?: CanonicalRequestStatus;
answerText?: string;
decidedByExternalUserId?: string;
decidedByPrincipalId?: string;
followupState?: string;
expiresAt?: string;
}
Expand All @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -292,6 +300,7 @@ export interface UpdateCanonicalGuardianRequestParams {
status?: CanonicalRequestStatus;
answerText?: string;
decidedByExternalUserId?: string;
decidedByPrincipalId?: string;
followupState?: string | null;
expiresAt?: string;
}
Expand All @@ -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;

Expand All @@ -322,6 +332,7 @@ export interface ResolveDecision {
status: CanonicalRequestStatus;
answerText?: string;
decidedByExternalUserId?: string;
decidedByPrincipalId?: string;
}

/**
Expand All @@ -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)
Expand Down
4 changes: 4 additions & 0 deletions assistant/src/memory/db-init.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import {
migrateNotificationDeliveryThreadDecision,
migrateReminderRoutingIntent,
migrateSchemaIndexesAndColumns,
migrateGuardianPrincipalIdColumns,
migrateVoiceInviteColumns,
migrateVoiceInviteDisplayMetadata,
recoverCrashedMigrations,
Expand Down Expand Up @@ -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);
}
4 changes: 4 additions & 0 deletions assistant/src/memory/guardian-bindings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ export interface GuardianBinding {
channel: string;
guardianExternalUserId: string;
guardianDeliveryChatId: string;
guardianPrincipalId: string | null;
status: BindingStatus;
verifiedAt: number;
verifiedVia: string;
Expand All @@ -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,
Expand All @@ -60,6 +62,7 @@ export function createBinding(params: {
channel: string;
guardianExternalUserId: string;
guardianDeliveryChatId: string;
guardianPrincipalId?: string | null;
verifiedVia?: string;
metadataJson?: string | null;
}): GuardianBinding {
Expand All @@ -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',
Expand Down
Original file line number Diff line number Diff line change
@@ -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 */ }
}
1 change: 1 addition & 0 deletions assistant/src/memory/migrations/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
3 changes: 3 additions & 0 deletions assistant/src/memory/schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'),
Expand Down Expand Up @@ -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'),
Expand All @@ -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(),
Expand Down